[PATCH v2 5/5] video: Fill video handoff in video post probe

[Simon Glass]

Hi Devarsh,

On Tue, 31 Oct 2023 at 13:12, Devarsh Thakkar <devarsht at ti.com> wrote:
>
> Fill video handoff fields in video_post_probe
> as at this point we have full framebuffer related

framebuffer-related

> information.
>
> Also fill all the fields available in video hand-off
> struct as those were missing earlier and u-boot

U-Boot

> framework expects them to be filled for some of the
> functionalities.
>
> Reported-by: Simon Glass <sjg at chromium.org>
> Signed-off-by: Devarsh Thakkar <devarsht at ti.com>
> ---
> V2: No change
> ---
>  drivers/video/video-uclass.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> index 335a1a1828..635b70f668 100644
> --- a/drivers/video/video-uclass.c
> +++ b/drivers/video/video-uclass.c
> @@ -153,16 +153,6 @@ int video_reserve(ulong *addrp)
>         debug("Video frame buffers from %lx to %lx\n", gd->video_bottom,
>               gd->video_top);
>
> -       if (spl_phase() == PHASE_SPL && CONFIG_IS_ENABLED(VIDEO_HANDOFF)) {
> -               struct video_handoff *ho;
> -
> -               ho = bloblist_add(BLOBLISTT_U_BOOT_VIDEO, sizeof(*ho), 0);
> -               if (!ho)
> -                       return log_msg_ret("blf", -ENOENT);
> -               ho->fb = *addrp;
> -               ho->size = size;

You also need to fill out the other fields in there:

struct video_handoff {
   u64 fb;
   u32 size;
   u16 xsize;
   u16 ysize;
   u32 line_length;
   u8 bpix;
};


> -       }
> -
>         return 0;
>  }
>
> @@ -558,6 +548,21 @@ static int video_post_probe(struct udevice *dev)
>
>         priv->fb_size = priv->line_length * priv->ysize;
>
> +       /* Set up video handoff fields for passing video blob to next stage */
> +       if (spl_phase() == PHASE_SPL && CONFIG_IS_ENABLED(BLOBLIST)) {
> +               struct video_handoff *ho;
> +
> +               ho = bloblist_add(BLOBLISTT_U_BOOT_VIDEO, sizeof(*ho), 0);
> +               if (!ho)
> +                       return log_msg_ret("blf", -ENOENT);
> +               ho->fb = gd->video_bottom;

This is a bit odd, since it assumes there is a single video
framebuffer. I suppose that is normally the case, but please add a
comment saying that only one is supported.

> +               ho->size = gd->video_top - gd->video_bottom;
> +               ho->xsize = priv->xsize;
> +               ho->ysize = priv->ysize;
> +               ho->line_length = priv->line_length;
> +               ho->bpix = priv->bpix;
> +       }
> +
>         if (IS_ENABLED(CONFIG_VIDEO_COPY) && plat->copy_base)
>                 priv->copy_fb = map_sysmem(plat->copy_base, plat->size);
>
> --
> 2.34.1
>

Please can you add some documentation to doc/develop/spl.rst or similar?

Regards,
Simon