[PATCH 0/3] R-Car DU fixes

[PATCH 0/3] R-Car DU fixes

[Laurent Pinchart]

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(-)

-- 
Regards,

Laurent Pinchart

[PATCH 1/3] drm: rcar-du: Fix crash with groups that have less than 9 planes

[Laurent Pinchart]

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;
-- 
2.3.6

[PATCH 2/3] drm: rcar-du: Clarify error message when encoder initialization fails

[Laurent Pinchart]

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;
 		}
 
-- 
2.3.6

[PATCH 3/3] drm: rcar-du: Convert rcar_du_encoders_init_one() return value to 0/<0

[Laurent Pinchart]

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;
-- 
2.3.6

Re: [PATCH 3/3] drm: rcar-du: Convert rcar_du_encoders_init_one() return value to 0/<0

[Geert Uytterhoeven]

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

Re: [PATCH 3/3] drm: rcar-du: Convert rcar_du_encoders_init_one() return value to 0/<0

[Laurent Pinchart]

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.

-- 
Regards,

Laurent Pinchart