| Message ID | 20230821181339.4135800-3-alpernebiyasak@gmail.com |
|---|---|
| State | Under Review |
| Delegated to: | Anatolij Gustschin |
| Headers | show |
| Series | sandbox: video: Refactor out of uclass, try partial screen updates | expand |
Hi Alper, On Mon, 21 Aug 2023 at 12:13, Alper Nebi Yasak <alpernebiyasak@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@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
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);
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@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? drivers/video/video-uclass.c | 2 ++ include/video.h | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-)