Re: [PATCH 4.19.y-cip] drm: rcar-du: Fix crash when using LVDS1 clock for CRTC


Nobuhiro Iwamatsu
 

Hi,

-----Original Message-----
From: Biju Das [mailto:biju.das.jz@bp.renesas.com]
Sent: Saturday, April 10, 2021 1:15 AM
To: cip-dev@lists.cip-project.org; iwamatsu nobuhiro(岩松 信洋 □SWC◯ACT)
<nobuhiro1.iwamatsu@toshiba.co.jp>; Pavel Machek <pavel@denx.de>
Cc: Chris Paterson <chris.paterson2@renesas.com>; Biju Das <biju.das.jz@bp.renesas.com>; Prabhakar Mahadev Lad
<prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: [PATCH 4.19.y-cip] drm: rcar-du: Fix crash when using LVDS1 clock for CRTC

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

[ Upstream commit 53ced169373aab52d3b5da0fee6a342002d1876d ]

On D3 and E3 platforms, the LVDS encoder includes a PLL that can
generate a clock for the corresponding CRTC, used even when the CRTC
output to a non-LVDS port. This mechanism is supported by the driver,
but the implementation is broken in dual-link LVDS mode. In that case,
the LVDS1 drm_encoder is skipped, which causes a crash when trying to
access its bridge later on.

Fix this by storing bridge pointers internally instead of retrieving
them from the encoder. The rcar_du_device encoders field isn't used
anymore and can be dropped.

Fixes: 8e8fddab0d0a ("drm: rcar-du: Skip LVDS1 output on Gen3 when using dual-link LVDS mode")
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Commit: 53ced169373aab52d3b5da0fee6a342002d1876d does not have Sasha's singed-off.
# Backported from 5.11 or 5.10?
If you backported from 53ced1693, I don't think this is necessary.

[Biju: Manually applied the changes to adapt with 4.19 kernel]
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
It may be a problem in my environment, I can not apply this patch to linux-4.19.y-cip tree.
And I can not find this mail from cip-dev ML and patchwork.

Best regards,
Nobuhiro

---
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 11 ++++-------
drivers/gpu/drm/rcar-du/rcar_du_drv.h | 6 +++---
drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 5 ++++-
3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index cec4b5c21eea..3362ccced05a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -676,13 +676,11 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
*/
if (rcdu->info->lvds_clk_mask & BIT(rcrtc->index) &&
rstate->outputs == BIT(RCAR_DU_OUTPUT_DPAD0)) {
- struct rcar_du_encoder *encoder =
- rcdu->encoders[RCAR_DU_OUTPUT_LVDS0 + rcrtc->index];
+ struct drm_bridge *bridge = rcdu->lvds[rcrtc->index];
const struct drm_display_mode *mode =
&crtc->state->adjusted_mode;

- rcar_lvds_clk_enable(encoder->base.bridge,
- mode->clock * 1000);
+ rcar_lvds_clk_enable(bridge, mode->clock * 1000);
}

rcar_du_crtc_start(rcrtc);
@@ -700,14 +698,13 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,

if (rcdu->info->lvds_clk_mask & BIT(rcrtc->index) &&
rstate->outputs == BIT(RCAR_DU_OUTPUT_DPAD0)) {
- struct rcar_du_encoder *encoder =
- rcdu->encoders[RCAR_DU_OUTPUT_LVDS0 + rcrtc->index];
+ struct drm_bridge *bridge = rcdu->lvds[rcrtc->index];

/*
* Disable the LVDS clock output, see
* rcar_du_crtc_atomic_enable().
*/
- rcar_lvds_clk_disable(encoder->base.bridge);
+ rcar_lvds_clk_disable(bridge);
}

spin_lock_irq(&crtc->dev->event_lock);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index f16cd263d2c1..558ebefce4ac 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -23,9 +23,9 @@

struct clk;
struct device;
+struct drm_bridge;
struct drm_device;
struct rcar_du_device;
-struct rcar_du_encoder;

#define RCAR_DU_FEATURE_CRTC_IRQ_CLOCK BIT(0) /* Per-CRTC IRQ and clock */
#define RCAR_DU_FEATURE_VSP1_SOURCE BIT(1) /* Has inputs from VSP1 */
@@ -73,6 +73,7 @@ struct rcar_du_device_info {
#define RCAR_DU_MAX_CRTCS 4
#define RCAR_DU_MAX_GROUPS DIV_ROUND_UP(RCAR_DU_MAX_CRTCS, 2)
#define RCAR_DU_MAX_VSPS 4
+#define RCAR_DU_MAX_LVDS 2

struct rcar_du_device {
struct device *dev;
@@ -87,10 +88,9 @@ struct rcar_du_device {
struct rcar_du_crtc crtcs[RCAR_DU_MAX_CRTCS];
unsigned int num_crtcs;

- struct rcar_du_encoder *encoders[RCAR_DU_OUTPUT_MAX];
-
struct rcar_du_group groups[RCAR_DU_MAX_GROUPS];
struct rcar_du_vsp vsps[RCAR_DU_MAX_VSPS];
+ struct drm_bridge *lvds[RCAR_DU_MAX_LVDS];

struct {
struct drm_property *colorkey;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index de8fe74c0362..325e301f97e3 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -47,7 +47,6 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
if (renc == NULL)
return -ENOMEM;

- rcdu->encoders[output] = renc;
renc->output = output;
encoder = rcar_encoder_to_drm_encoder(renc);

@@ -61,6 +60,10 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
goto done;
}

+ if (output == RCAR_DU_OUTPUT_LVDS0 ||
+ output == RCAR_DU_OUTPUT_LVDS1)
+ rcdu->lvds[output - RCAR_DU_OUTPUT_LVDS0] = bridge;
+
/*
* On Gen3 skip the LVDS1 output if the LVDS1 encoder is used as a
* companion for LVDS0 in dual-link mode.
--
2.17.1

Join cip-dev@lists.cip-project.org to automatically receive all group messages.