[PATCH v2 3/5] video: Introduce video_sync operation

[Simon Glass]

Hi Michal,

On Mon, 14 Dec 2020 at 23:57, Michal Simek <michal.simek at xilinx.com> wrote:
>
>
>
> On 15. 12. 20 5:00, Simon Glass wrote:
> > On Mon, 14 Dec 2020 at 01:38, Michal Simek <michal.simek at xilinx.com> wrote:
> >>
> >> Some drivers like LCD connected via SPI requires explicit sync function
> >> which copy framebuffer content over SPI to controller to display.
> >> This hook doesn't exist yet that's why introduce it via video operations.
> >>
> >> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> >> ---
> >>
> >> Changes in v2:
> >> - Add support for returning value
> >> - Update kernel-doc format to pass kernel-doc script
> >> - Update patch subject s/call/operation/
> >>
> >> Simon: Please review this. I didn't find existing way how this can be done
> >> that's why I am introducing this hook. Also maybe name can be named a
> >> little bit differently. That's why waiting for better suggestion.
> >>
> >> ---
> >>  drivers/video/video-uclass.c |  9 +++++++++
> >>  include/video.h              | 12 ++++++++++--
> >>  2 files changed, 19 insertions(+), 2 deletions(-)
> >
> > Reviewed-by: Simon Glass <sjg at chromium.org>
> >
> >>
> >> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> >> index 6fc412801714..938e7d371311 100644
> >> --- a/drivers/video/video-uclass.c
> >> +++ b/drivers/video/video-uclass.c
> >> @@ -175,6 +175,15 @@ void video_set_default_colors(struct udevice *dev, bool invert)
> >>  /* Flush video activity to the caches */
> >>  int video_sync(struct udevice *vid, bool force)
> >>  {
> >> +       struct video_ops *ops = video_get_ops(vid);
> >> +       int ret;
> >> +
> >> +       if (ops && ops->video_sync) {
> >> +               ret = ops->video_sync(vid);
> >> +               if (ret)
> >> +                       return ret;
> >> +       }
> >> +
> >>         /*
> >>          * flush_dcache_range() is declared in common.h but it seems that some
> >>          * architectures do not actually implement it. Is there a way to find
> >> diff --git a/include/video.h b/include/video.h
> >> index 1bfe6843a805..12fc525ab4ef 100644
> >> --- a/include/video.h
> >> +++ b/include/video.h
> >> @@ -114,8 +114,16 @@ struct video_priv {
> >>         u8 bg_col_idx;
> >>  };
> >>
> >> -/* Placeholder - there are no video operations at present */
> >> +/**
> >> + * struct video_ops - structure for keeping video operations
> >> + * @video_sync: Synchronize FB with device. Some device like SPI based LCD
> >> + *             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
> >
> > Can this go before the video_sync, below?
>
> I was checking some files in Linux how they are dealing with it to pass
> kernel-doc and all of them used this format.
> Also if I look at include/dm/uclass.h  - struct uclass_driver
> it is done in the same way.
>
> Maybe we can ask Linux guys if there is a better way than this one.

Yes I see that a lot in linux and it is unfortunate.

+Heinrich Schuchardt probably knows

Regards,
Simon