diff mbox series

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

Message ID 20220423073728.111808-1-biju.das.jz@bp.renesas.com (mailing list archive)
State Superseded, archived
Delegated to: Kieran Bingham
Headers show
Series drm: rcar-du: Add setting to PnALPHAR register on Gen3 | expand

Commit Message

Biju Das April 23, 2022, 7:37 a.m. UTC 
From: LUU HOAI <hoai.luu.ub@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@renesas.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
This patch is based on [1]
[1] https://github.com/renesas-rcar/linux-bsp/commit/fcb34fe338cbde0a64919430733541035f20a784

Not sure this patches has to go with Fixes tag for stable??

Tested the changes on RZ/G2M board

root@hihope-rzg2m:/cip-test-scripts#  modetest -M rcar-du -w 54:alpha:55555
root@hihope-rzg2m:/cip-test-scripts# modetest -M rcar-du -s "93@90:1024x768@AR24" -d -P "54@90:400x300+200+200@XR24"
setting mode 1024x768-75Hz@AR24 on connectors 93, crtc 90
testing 400x300@XR24 overlay plane 54
---
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Laurent Pinchart April 24, 2022, 3:31 p.m. UTC | #1 
Hi Biju,

Thank you for the patch.

On Sat, Apr 23, 2022 at 08:37:28AM +0100, Biju Das wrote:
> From: LUU HOAI <hoai.luu.ub@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@renesas.com>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> This patch is based on [1]
> [1] https://github.com/renesas-rcar/linux-bsp/commit/fcb34fe338cbde0a64919430733541035f20a784
> 
> Not sure this patches has to go with Fixes tag for stable??
> 
> Tested the changes on RZ/G2M board
> 
> root@hihope-rzg2m:/cip-test-scripts#  modetest -M rcar-du -w 54:alpha:55555
> root@hihope-rzg2m:/cip-test-scripts# modetest -M rcar-du -s "93@90:1024x768@AR24" -d -P "54@90:400x300+200+200@XR24"
> setting mode 1024x768-75Hz@AR24 on connectors 93, crtc 90
> testing 400x300@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:

	/*
	 * 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.

Furthermore, is this really the right fix, shouldn't we instead avoid
enabling alpha-blending in PnMR on Gen3 ?

> +	rcar_du_plane_write(rgrp, index, PnALPHAR, 0x00000000);
>  }
>  
>  static void rcar_du_plane_setup_format(struct rcar_du_group *rgrp,
Biju Das April 24, 2022, 4:12 p.m. UTC | #2 
Hi Laurent,

Thanks for the feedback.

> Subject: Re: [PATCH] drm: rcar-du: Add setting to PnALPHAR register on
> Gen3
> 
> Hi Biju,
> 
> Thank you for the patch.
> 
> On Sat, Apr 23, 2022 at 08:37:28AM +0100, Biju Das wrote:
> > From: LUU HOAI <hoai.luu.ub@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@renesas.com>
> > Signed-off-by: Biju Das <biju.das.jz@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@hihope-rzg2m:/cip-test-scripts#  modetest -M rcar-du -w
> > 54:alpha:55555 root@hihope-rzg2m:/cip-test-scripts# modetest -M rcar-du
> -s "93@90:1024x768@AR24" -d -P "54@90:400x300+200+200@XR24"
> > setting mode 1024x768-75Hz@AR24 on connectors 93, crtc 90 testing
> > 400x300@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?

Here the issue they mentioned as "register is not initialized by reset"

Cheers,
Biju

> 
> > +	rcar_du_plane_write(rgrp, index, PnALPHAR, 0x00000000);
> >  }
> >
> >  static void rcar_du_plane_setup_format(struct rcar_du_group *rgrp,
> 
> --
> Regards,
> 
> Laurent Pinchart
Laurent Pinchart April 24, 2022, 11:25 p.m. UTC | #3 
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@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@renesas.com>
> > > Signed-off-by: Biju Das <biju.das.jz@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@hihope-rzg2m:/cip-test-scripts#  modetest -M rcar-du -w
> > > 54:alpha:55555 root@hihope-rzg2m:/cip-test-scripts# modetest -M rcar-du
> > -s "93@90:1024x768@AR24" -d -P "54@90:400x300+200+200@XR24"
> > > setting mode 1024x768-75Hz@AR24 on connectors 93, crtc 90 testing
> > > 400x300@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.
  */

> 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 ?

> > >  }
> > >
> > >  static void rcar_du_plane_setup_format(struct rcar_du_group *rgrp,
Biju Das April 25, 2022, 6:19 a.m. UTC | #4 
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@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@renesas.com>
> > > > Signed-off-by: Biju Das <biju.das.jz@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@hihope-rzg2m:/cip-test-scripts#  modetest -M rcar-du -w
> > > > 54:alpha:55555 root@hihope-rzg2m:/cip-test-scripts# modetest -M
> > > > rcar-du
> > > -s "93@90:1024x768@AR24" -d -P "54@90:400x300+200+200@XR24"
> > > > setting mode 1024x768-75Hz@AR24 on connectors 93, crtc 90 testing
> > > > 400x300@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
diff mbox series

Patch

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
+	 */
+	rcar_du_plane_write(rgrp, index, PnALPHAR, 0x00000000);
 }
 
 static void rcar_du_plane_setup_format(struct rcar_du_group *rgrp,