Hello,
This patch set fixes a crash in the R-Car DU driver (1/3) and adds two other small cleanups (2/3 and 3/3). Please see individual patches for details.
Laurent Pinchart (3): drm: rcar-du: Fix crash with groups that have less than 9 planes drm: rcar-du: Clarify error message when encoder initialization fails drm: rcar-du: Convert rcar_du_encoders_init_one() return value to 0/<0
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 4 ++-- drivers/gpu/drm/rcar-du/rcar_du_group.h | 2 ++ drivers/gpu/drm/rcar-du/rcar_du_kms.c | 22 ++++++++++++++-------- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 5 ++--- 4 files changed, 20 insertions(+), 13 deletions(-)
Commit 917de180379d ("drm: rcar-du: Implement universal plane support") made the number of planes per group dynamic, but didn't update all loops over the planes array, resulting in out-of-bound accesses on DU instances that have an odd number of CRTCs (such as the R8A7790). Fix it.
Fixes: 917de180379d ("drm: rcar-du: Implement universal plane support") Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 4 ++-- drivers/gpu/drm/rcar-du/rcar_du_group.h | 2 ++ drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 5 ++--- 4 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index a40085806f5b..65d6ba6621ac 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -214,7 +214,7 @@ static void rcar_du_crtc_update_planes(struct rcar_du_crtc *rcrtc) unsigned int i; u32 dspr = 0;
- for (i = 0; i < ARRAY_SIZE(rcrtc->group->planes); ++i) { + for (i = 0; i < rcrtc->group->num_planes; ++i) { struct rcar_du_plane *plane = &rcrtc->group->planes[i]; unsigned int j;
@@ -445,7 +445,7 @@ void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc) rcar_du_crtc_start(rcrtc);
/* Commit the planes state. */ - for (i = 0; i < ARRAY_SIZE(rcrtc->group->planes); ++i) { + for (i = 0; i < rcrtc->group->num_planes; ++i) { struct rcar_du_plane *plane = &rcrtc->group->planes[i];
if (plane->plane.state->crtc != &rcrtc->crtc) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h b/drivers/gpu/drm/rcar-du/rcar_du_group.h index 7b414b31c3be..d7318e1a6b00 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h @@ -30,6 +30,7 @@ struct rcar_du_device; * @used_crtcs: number of CRTCs currently in use * @lock: protects the dptsr_planes field and the DPTSR register * @dptsr_planes: bitmask of planes driven by dot-clock and timing generator 1 + * @num_planes: number of planes in the group * @planes: planes handled by the group */ struct rcar_du_group { @@ -44,6 +45,7 @@ struct rcar_du_group { struct mutex lock; unsigned int dptsr_planes;
+ unsigned int num_planes; struct rcar_du_plane planes[RCAR_DU_NUM_KMS_PLANES]; };
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 20859aae882e..4bb5af4bc474 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -336,7 +336,7 @@ static int rcar_du_atomic_check(struct drm_device *dev, dev_dbg(rcdu->dev, "%s: finding free planes for group %u\n", __func__, index);
- for (i = 0; i < RCAR_DU_NUM_KMS_PLANES; ++i) { + for (i = 0; i < group->num_planes; ++i) { struct rcar_du_plane *plane = &group->planes[i]; struct rcar_du_plane_state *plane_state; struct drm_plane_state *s; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 3e30d84b798f..d90dc428e3fd 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -390,7 +390,6 @@ static const uint32_t formats[] = { int rcar_du_planes_init(struct rcar_du_group *rgrp) { struct rcar_du_device *rcdu = rgrp->dev; - unsigned int num_planes; unsigned int crtcs; unsigned int i; int ret; @@ -398,11 +397,11 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp) /* Create one primary plane per CRTC in this group and seven overlay * planes. */ - num_planes = rgrp->num_crtcs + 7; + rgrp->num_planes = rgrp->num_crtcs + 7;
crtcs = ((1 << rcdu->num_crtcs) - 1) & (3 << (2 * rgrp->index));
- for (i = 0; i < num_planes; ++i) { + for (i = 0; i < rgrp->num_planes; ++i) { enum drm_plane_type type = i < rgrp->num_crtcs ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
A failure to initialize an encoder currently prints an error message in the kernel log without mentioning which encoder failed to initialize. To help debugging initialization issues print the encoder DT node name.
This requires moving the error message to the rcar_du_encoders_init_one function and refactoring it slightly.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 4bb5af4bc474..bca859ad204c 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -639,7 +639,15 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu, of_node_put(encoder); of_node_put(connector);
- return ret < 0 ? ret : 1; + if (!ret) + return 1; + + if (ret != -EPROBE_DEFER) + dev_warn(rcdu->dev, + "failed to initialize encoder %s (%d), skipping\n", + encoder->full_name, ret); + + return 0; }
static int rcar_du_encoders_init(struct rcar_du_device *rcdu) @@ -688,8 +696,6 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu) return ret; }
- dev_info(rcdu->dev, - "encoder initialization failed, skipping\n"); continue; }
The function returns 1 on success, and either 0 or a negative error code on failure. As the 0 and negative values don't need to be differentiated by the caller, convert it to the usual scheme of returning 0 on success and a negative error code on failure.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index bca859ad204c..acbf3087a7f2 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -573,7 +573,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu, if (!entity) { dev_dbg(rcdu->dev, "unconnected endpoint %s, skipping\n", ep->local_node->full_name); - return 0; + return -ENODEV; }
entity_ep_node = of_parse_phandle(ep->local_node, "remote-endpoint", 0); @@ -596,7 +596,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu, encoder->full_name); of_node_put(entity_ep_node); of_node_put(encoder); - return 0; + return -ENODEV; }
break; @@ -625,7 +625,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu, encoder->full_name); of_node_put(encoder); of_node_put(connector); - return 0; + return -EINVAL; } } else { /* @@ -640,14 +640,14 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu, of_node_put(connector);
if (!ret) - return 1; + return 0;
if (ret != -EPROBE_DEFER) dev_warn(rcdu->dev, "failed to initialize encoder %s (%d), skipping\n", encoder->full_name, ret);
- return 0; + return ret; }
static int rcar_du_encoders_init(struct rcar_du_device *rcdu) @@ -699,7 +699,7 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu) continue; }
- num_encoders += ret; + num_encoders++; }
return num_encoders;
On Tue, May 26, 2015 at 5:01 PM, Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com wrote:
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -640,14 +640,14 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu, of_node_put(connector);
if (!ret)
return 1;
return 0; if (ret != -EPROBE_DEFER) dev_warn(rcdu->dev, "failed to initialize encoder %s (%d), skipping\n", encoder->full_name, ret);
return 0;
return ret;
}
Perhaps just
if (ret && ret != -EPROBE_DEFER) dev_warn(...);
return ret;
?
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert,
On Tuesday 26 May 2015 21:24:03 Geert Uytterhoeven wrote:
On Tue, May 26, 2015 at 5:01 PM, Laurent Pinchart wrote:
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -640,14 +640,14 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu, of_node_put(connector);
if (!ret)
return 1;
return 0; if (ret != -EPROBE_DEFER) dev_warn(rcdu->dev, "failed to initialize encoder %s (%d), skipping\n", encoder->full_name, ret);
return 0;
return ret;
}
Perhaps just
if (ret && ret != -EPROBE_DEFER) dev_warn(...); return ret;
?
Good point. I'll fix that.
dri-devel@lists.freedesktop.org