Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200901220305.6809-1-jernej.skrabec@siol.net>
 <CAGphcdnT4U8AztL_B_B2HYskQSAYKRgCQcnO3Q8Qj+UFO082hQ@mail.gmail.com> <10671571.W45mMbEh4O@kista>
In-Reply-To: <10671571.W45mMbEh4O@kista>
From:   Roman Stratiienko <r.stratiienko@gmail.com>
Date:   Wed, 2 Sep 2020 19:55:42 +0300
Message-ID: <CAGphcdmfb0Ha0h2UX_Ez7049r7sm2Np6amg_paTcqfPB42Ga9w@mail.gmail.com>
Subject: Re: [PATCH] drm/sun4i: Fix DE2 YVU handling
To:     =?UTF-8?Q?Jernej_=C5=A0krabec?= <jernej.skrabec@siol.net>
Cc:     mripard@kernel.org, wens@csie.org, irlied@linux.ie,
        Daniel Vetter <daniel@ffwll.ch>,
        dri-devel@lists.freedesktop.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

=D1=81=D1=80, 2 =D1=81=D0=B5=D0=BD=D1=82. 2020 =D0=B3. =D0=B2 19:46, Jernej=
 =C5=A0krabec <jernej.skrabec@siol.net>:
>
> Dne sreda, 02. september 2020 ob 09:01:17 CEST je Roman Stratiienko
> napisal(a):
> > =D1=81=D1=80, 2 =D1=81=D0=B5=D0=BD=D1=82. 2020 =D0=B3. =D0=B2 00:58, Je=
rnej Skrabec <jernej.skrabec@siol.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 i=
s
> > > mostly redundant") Reported-by: Roman Stratiienko
> > > <r.stratiienko@gmail.com>
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.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..c0147af6=
a840
> > > 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?

Sure.

Tested-by: Roman Stratiienko <r.stratiienko@gmail.com>

>
> >
> > 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 dupli=
cated
> for handling U/V switch. I have a patch which reorganize matrix on the fl=
y 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 Y=
VU CSC
> matrices are switched 2nd and 3rd column.

This sounds reasonable. Thank you for your explanation.

Regards,
Roman

>
> Best regards,
> Jernej
>
> >
> > Regards,
> > Roman
>
>
>
>