linux-kernel - Re: [PATCH] drm/sun4i: Fix DE2 YVU handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <10671571.W45mMbEh4O@kista>
Date:   Wed, 02 Sep 2020 18:51:15 +0200
From:   Jernej Škrabec <jernej.skrabec@...l.net>
To:     Roman Stratiienko <r.stratiienko@...il.com>
Cc:     mripard@...nel.org, wens@...e.org, irlied@...ux.ie,
        daniel@...ll.ch, dri-devel@...ts.freedesktop.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/sun4i: Fix DE2 YVU handling

Dne sreda, 02. september 2020 ob 09:01:17 CEST je Roman Stratiienko 
napisal(a):
> ср, 2 сент. 2020 г. в 00:58, Jernej Skrabec <jernej.skrabec@...l.net>:
> > Function sun8i_vi_layer_get_csc_mode() is supposed to return CSC mode
> > but due to inproper return type (bool instead of u32) it returns just 0
> > or 1. Colors are wrong for YVU formats because of that.
> > 
> > Fixes: daab3d0e8e2b ("drm/sun4i: de2: csc_mode in de2 format struct is
> > mostly redundant") Reported-by: Roman Stratiienko
> > <r.stratiienko@...il.com>
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@...l.net>
> > ---
> > 
> >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index 22c8c5375d0d..c0147af6a840
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > @@ -211,7 +211,7 @@ static int sun8i_vi_layer_update_coord(struct
> > sun8i_mixer *mixer, int channel,> 
> >         return 0;
> >  
> >  }
> > 
> > -static bool sun8i_vi_layer_get_csc_mode(const struct drm_format_info
> > *format) +static u32 sun8i_vi_layer_get_csc_mode(const struct
> > drm_format_info *format)> 
> >  {
> >  
> >         if (!format->is_yuv)
> >         
> >                 return SUN8I_CSC_MODE_OFF;
> > 
> > --
> > 2.28.0
> 
> Hi Jernej,
> 
> Thank you for the fix.
> I can confirm this patch fixes the issue with wrong colors.

Thanks! Can I assume that this means your Tested-by tag can be added?

> 
> Let me share my thoughts:
> I've looked into csc code, and it seems to me reordering U, V offsets
> should be a much simpler solution than applying
> color transformation matrices.It should also simplify adding more
> color encodings in the future.

Switching offsets assumes that you have separate planes for U and V which may 
not be true in the future. I agree that CSC matrices are needlessly duplicated 
for handling U/V switch. I have a patch which reorganize matrix on the fly when 
coefficients are written in registers but since it's a part of a bigger, 
unfinished series, I didn't sent it out yet. Only difference in YUV and YVU CSC 
matrices are switched 2nd and 3rd column.

Best regards,
Jernej

> 
> Regards,
> Roman