| Message ID | 04221731ad19461b6a6b9c1a3d84105167aebe47.1607935068.git.michal.simek@xilinx.com |
|---|---|
| State | Superseded |
| Delegated to: | Anatolij Gustschin |
| Headers | show |
| Series | video: seps525: Add new driver for seps525 OLED display | expand |
On Mon, 14 Dec 2020 at 01:38, Michal Simek <michal.simek@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@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@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? > + */ > struct video_ops { > + int (*video_sync)(struct udevice *vid); > }; > > #define video_get_ops(dev) ((struct video_ops *)(dev)->driver->ops) > @@ -155,7 +163,7 @@ int video_clear(struct udevice *dev); > * @force: True to force a sync even if there was one recently (this is > * very expensive on sandbox) > * > - * @return: 0 always > + * @return: 0 on success, error code otherwise > * > * Some frame buffers are cached or have a secondary frame buffer. This > * function syncs these up so that the current contents of the U-Boot frame > -- > 2.29.2 >
On 15. 12. 20 5:00, Simon Glass wrote: > On Mon, 14 Dec 2020 at 01:38, Michal Simek <michal.simek@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@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@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
Hi Michal, On Mon, 14 Dec 2020 at 23:57, Michal Simek <michal.simek@xilinx.com> wrote: > > > > On 15. 12. 20 5:00, Simon Glass wrote: > > On Mon, 14 Dec 2020 at 01:38, Michal Simek <michal.simek@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@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@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
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 + */ struct video_ops { + int (*video_sync)(struct udevice *vid); }; #define video_get_ops(dev) ((struct video_ops *)(dev)->driver->ops) @@ -155,7 +163,7 @@ int video_clear(struct udevice *dev); * @force: True to force a sync even if there was one recently (this is * very expensive on sandbox) * - * @return: 0 always + * @return: 0 on success, error code otherwise * * Some frame buffers are cached or have a secondary frame buffer. This * function syncs these up so that the current contents of the U-Boot frame
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@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(-)