Hi Tomi,
Thank you for the patch.
On Wed, Aug 17, 2022 at 04:28:02PM +0300, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>
>
> The rcar DU driver on r8a779a0 has a bug causing some specific colors
> getting converted to transparent colors, which then (usually) show as
> black pixels on the screen.
>
> The reason seems to be that the driver sets PnMR_SPIM_ALP bit in
> PnMR.SPIM field, which is an illegal setting on r8a779a0. The
> PnMR_SPIM_EOR bit also illegal.
>
> Add a new feature flag for this (lack of a) feature and make sure the
> bits are zero on r8a779a0.
Good catch !
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 3 ++-
> drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 +
> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 16 +++++++++++++---
> 3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 541c202c993a..a2776f1d6f2c 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -506,7 +506,8 @@ static const struct rcar_du_device_info rcar_du_r8a7799x_info = {
> static const struct rcar_du_device_info rcar_du_r8a779a0_info = {
> .gen = 3,
> .features = RCAR_DU_FEATURE_CRTC_IRQ
> - | RCAR_DU_FEATURE_VSP1_SOURCE,
> + | RCAR_DU_FEATURE_VSP1_SOURCE
> + | RCAR_DU_FEATURE_NO_BLENDING,
> .channels_mask = BIT(1) | BIT(0),
> .routes = {
> /* R8A779A0 has two MIPI DSI outputs. */
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> index bfad7775d9a1..712389c7b3d0 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -31,6 +31,7 @@ struct rcar_du_device;
> #define RCAR_DU_FEATURE_VSP1_SOURCE BIT(2) /* Has inputs from VSP1 */
> #define RCAR_DU_FEATURE_INTERLACED BIT(3) /* HW supports interlaced */
> #define RCAR_DU_FEATURE_TVM_SYNC BIT(4) /* Has TV switch/sync modes */
> +#define RCAR_DU_FEATURE_NO_BLENDING BIT(5) /* PnMR.SPIM does not have ALP nor EOR bits */
>
> #define RCAR_DU_QUIRK_ALIGN_128B BIT(0) /* Align pitches to 128 bytes */
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> index 9e1f0cbbf642..2103816807cc 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> @@ -506,8 +506,19 @@ static void rcar_du_plane_setup_format_gen3(struct rcar_du_group *rgrp,
> unsigned int index,
> const struct rcar_du_plane_state *state)
> {
> - rcar_du_plane_write(rgrp, index, PnMR,
> - PnMR_SPIM_TP_OFF | state->format->pnmr);
> + struct rcar_du_device *rcdu = rgrp->dev;
> + u32 pnmr;
> +
> + pnmr = state->format->pnmr;
> +
> + if (rcdu->info->features & RCAR_DU_FEATURE_NO_BLENDING) {
> + /* No blending. ALP and EOR are not supported */
> + pnmr &= ~(PnMR_SPIM_ALP | PnMR_SPIM_EOR);
> + }
> +
> + pnmr |= PnMR_SPIM_TP_OFF;
I'd combine this with the initial pnmr assignment. I can handle this
when applying, no need to resubmit.
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> +
> + rcar_du_plane_write(rgrp, index, PnMR, pnmr);
>
> rcar_du_plane_write(rgrp, index, PnDDCR4,
> state->format->edf | PnDDCR4_CODE);
> @@ -521,7 +532,6 @@ static void rcar_du_plane_setup_format_gen3(struct rcar_du_group *rgrp,
> * register to 0 to avoid this.
> */
>
> - /* TODO: Check if alpha-blending should be disabled in PnMR. */
> rcar_du_plane_write(rgrp, index, PnALPHAR, 0);
> }
--
Regards,
Laurent Pinchart
