[PATCH] drm: rcar-du: Add setting to PnALPHAR register on Gen3

[Biju Das]

Hi Laurent,

Thanks for the feedback.

> Subject: Re: [PATCH] drm: rcar-du: Add setting to PnALPHAR register on
> Gen3
> 
> Hi Biju,
> 
> On Sun, Apr 24, 2022 at 04:12:08PM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH] drm: rcar-du: Add setting to PnALPHAR register
> > > on Gen3 On Sat, Apr 23, 2022 at 08:37:28AM +0100, Biju Das wrote:
> > > > From: LUU HOAI <hoai.luu.ub at renesas.com>
> > > >
> > > > In Gen3, when Alpha blend is enabled in the PnMR register,
> > > > depending on the initial value of the PnALPHAR register, either
> > > > channel of DU might be black screen.
> > > > Therefore, this patch prevents the black screen by setting the
> > > > PnALPHAR register to all 0.
> > > >
> > > > In addition, PnALPHAR register will be released in the R-Car Gen3
> > > > Hardware Manual Rev 2.4 (Sep. 2021).
> > > >
> > > > Signed-off-by: LUU HOAI <hoai.luu.ub at renesas.com>
> > > > Signed-off-by: Biju Das <biju.das.jz at bp.renesas.com>
> > > > ---
> > > > This patch is based on [1]
> > > > [1]
> > > >
> > > > Not sure this patches has to go with Fixes tag for stable??
> > > >
> > > > Tested the changes on RZ/G2M board
> > > >
> > > > root at hihope-rzg2m:/cip-test-scripts#  modetest -M rcar-du -w
> > > > 54:alpha:55555 root at hihope-rzg2m:/cip-test-scripts# modetest -M
> > > > rcar-du
> > > -s "93 at 90:1024x768 at AR24" -d -P "54 at 90:400x300+200+200 at XR24"
> > > > setting mode 1024x768-75Hz at AR24 on connectors 93, crtc 90 testing
> > > > 400x300 at XR24 overlay plane 54
> > > > ---
> > > >  drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > > > b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > > > index 5c1c7bb04f3f..aff39b9253f8 100644
> > > > --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > > > +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > > > @@ -510,6 +510,12 @@ static void
> > > > rcar_du_plane_setup_format_gen3(struct rcar_du_group *rgrp,
> > > >
> > > >  	rcar_du_plane_write(rgrp, index, PnDDCR4,
> > > >  			    state->format->edf | PnDDCR4_CODE);
> > > > +
> > > > +	/* In Gen3, PnALPHAR register need to be set to 0
> > > > +	 * to avoid black screen issue when alpha blend is enable
> > > > +	 * on DU module
> > > > +	 */
> > >
> > > Comments should start with /* on a line of its own, and you can also
> > > reflow the text to 80 columns:
> >
> > OK.
> >
> > > 	/*
> > > 	 * In Gen3, PnALPHAR register need to be set to 0 to avoid black
> screen
> > > 	 * issue when alpha blend is enable on DU module.
> > > 	 */
> > >
> > > It would however be nicer to document the exact behaviour, but the
> > > latest version of the documentation I have access to is rev 2.3 and
> > > it lists PnALPHAR as not available on Gen3.
> >
> > I don't have access to rev 2.4, but I got access to
> > "R-Car-Gen3_Common_OPC_Customer_Notifications_V30.1.pdf"
> > where it is mentioned about this issue and solution for fix which is
> > inline with the patch from R-Car BSP.
> >
> > "The reason is that a register is not initialized by reset.
> > This could lead to output wrong image data of other plane or wrong
> > color set from BPOR (Background plane output register)."
> >
> > > Furthermore, is this really the right fix, shouldn't we instead
> > > avoid enabling alpha-blending in PnMR on Gen3 ?
> >
> > Avoid enabling alpha-blending in PnMR on Gen3, will it help here?
> 
> It's hard to tell without knowing the exact cause of the issue. Clearing
> PnALPHAR probably makes sense on Gen3 if the register exists,
> independently from disabling alpha blending in PnMR. It would be nice if
> the commit messsage could reference the issue described in R-Car-
> Gen3_Common_OPC_Customer_Notifications_V30.1.pdf. I would also expand the
> comment a little bit:
> 
>  /*
>   * On Gen3, some DU channels have two planes, each being wired to a
> separate
>   * VSPD instance. The DU can then blend two two planes. While this
> feature
>   * isn't used by the driver, issues related to alpha blending (such as
>   * incorrect colors or planes being invisible) may still occur if the
> PnALPHAR
>   * register has a stale value. Set the register to 0 to avoid this.
>   */

OK.

> 
> > Here the issue they mentioned as "register is not initialized by reset"
> >
> > > > +	rcar_du_plane_write(rgrp, index, PnALPHAR, 0x00000000);
> 
> I'd write 0 instead of 0x00000000 to match the rest of the driver.
> 
> Would you mind sending a v2 with these changed and an expanded commit
> message ?

OK, Will send v2 with these changes.

Cheers,
Biju

> 
> > > >  }
> > > >
> > > >  static void rcar_du_plane_setup_format(struct rcar_du_group
> > > > *rgrp,
> 
> --
> Regards,
> 
> Laurent Pinchart