[PATCH v3 6/7] video: Only dcache flush damaged lines

[Simon Glass]

Hi,

On Tue, 3 Jan 2023 at 13:25, Alexander Graf <agraf at csgraf.de> wrote:
>
>
> On 30.12.22 22:12, Heinrich Schuchardt wrote:
> > On 12/30/22 20:58, Alexander Graf wrote:
> >> Now that we have a damage area tells us which parts of the frame buffer
> >> actually need updating, let's only dcache flush those on video_sync()
> >> calls. With this optimization in place, frame buffer updates -
> >> especially
> >> on large screen such as 4k displays - speed up significantly.
> >>
> >> Signed-off-by: Alexander Graf <agraf at csgraf.de>
> >> Reported-by: Da Xue <da at libre.computer>
> >>
> >> ---
> >>
> >> v1 -> v2:
> >>
> >>    - Fix dcache range; we were flushing too much before
> >>    - Remove ifdefs
> >> ---
> >>   drivers/video/video-uclass.c | 51 ++++++++++++++++++++++++++++++------
> >>   1 file changed, 43 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> >> index f1fbeaadcb..bf93f524c7 100644
> >> --- a/drivers/video/video-uclass.c
> >> +++ b/drivers/video/video-uclass.c
> >> @@ -297,9 +297,45 @@ int video_damage(struct udevice *vid, int x, int
> >> y, int width, int height)
> >>       return 0;
> >>   }
> >>
> >> +#if defined(CONFIG_ARM) && !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
> >
> > ARM isn't the only architecture implementing flush_dcache_range().
> > So this condition needs to be fixed as well as the one in video_sync.
> >
> > Why don't you simply rely on priv->flash_dcache irrespective of the
> > architecture? Let the video drivers decide if they need it.
>
>
> This is code that already was #ifdef'ed before. The way I understand its
> history, we couldn't decisively ensure that flush_dcache_range() exists,
> so we couldn't build the code always.
>
> I agree that it looks weird and error prone though. I believe you're
> trying to say we should just make sure there's always a
> flush_dcache_range() function and keep the rest to the compiler?
>
Can  / should we use the dma_map stuff here?

Regards,
Simon