[PATCH 2/5] video: Allow deferring and retrying driver-specific video sync

[Simon Glass]

Hi Alper,

On Mon, 21 Aug 2023 at 12:13, Alper Nebi Yasak <alpernebiyasak at gmail.com> wrote:
>
> The sandbox SDL driver has some code in the video uclass to rate limit
> video syncs by postponing them, and forcing a sync nonetheless with a
> "force" argument.
>
> Add infrastructure for doing this through driver ops, where the driver
> can request to defer a sync with -EAGAIN, and the uclass can force it by
> calling the op again it until it does something.
>
> Signed-off-by: Alper Nebi Yasak <alpernebiyasak at gmail.com>
> ---
> Alternatively, add "force" argument into the driver ops->video_sync().
> But I think it should go away instead. The problem is video_sync() being
> called irregularly and too frequently, maybe we can call it only from
> cyclic at the hardware refresh rate?

Yes I like that idea better. Calling it in a tight loop until it does
something seems wrong to me.

>
>  drivers/video/video-uclass.c | 2 ++
>  include/video.h              | 3 ++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> index 7cec6362570f..accf486509cb 100644
> --- a/drivers/video/video-uclass.c
> +++ b/drivers/video/video-uclass.c
> @@ -446,6 +446,8 @@ int video_sync(struct udevice *vid, bool force)
>
>         if (ops && ops->video_sync) {
>                 ret = ops->video_sync(vid);
> +               while (force && ret == -EAGAIN)
> +                       ret = ops->video_sync(vid);
>                 if (ret)
>                         return ret;
>         }
> diff --git a/include/video.h b/include/video.h
> index 42e57b44188d..5c4327d4e455 100644
> --- a/include/video.h
> +++ b/include/video.h
> @@ -137,7 +137,8 @@ struct video_priv {
>   *             displays needs synchronization when data in an FB is available.
>   *             For these devices implement video_sync hook to call a sync
>   *             function. vid is pointer to video device udevice. Function
> - *             should return 0 on success video_sync and error code otherwise
> + *             should return 0 on success video_sync, -EAGAIN if it was
> + *             deferred and should be tried again, and error code otherwise
>   */
>  struct video_ops {
>         int (*video_sync)(struct udevice *vid);
> --
> 2.40.1
>

Regards,
Simon