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

[Michal Simek]

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.

Thanks,
Michal