From 6e0c6e1895b9fff3cdb6ef746ee3d8dd4e852f40 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Thu, 14 May 2015 01:08:34 +0300 Subject: [PATCH 1/7] drm: rcar-du: Print the error value when DRM/KMS init fails This helps debugging probe failures. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index da1216a73969..780ca11512ba 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -190,7 +190,7 @@ static int rcar_du_load(struct drm_device *dev, unsigned long flags) /* DRM/KMS objects */ ret = rcar_du_modeset_init(rcdu); if (ret < 0) { - dev_err(&pdev->dev, "failed to initialize DRM/KMS\n"); + dev_err(&pdev->dev, "failed to initialize DRM/KMS (%d)\n", ret); goto done; } From 911316fe2f4113a096b9975f2467fc501864aef0 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Thu, 14 May 2015 15:01:47 +0300 Subject: [PATCH 2/7] drm: rcar-du: Disable all planes when stopping the CRTC The DSnPR plane configuration registers are updated on vblank, and no vblank will occur once the CRTC is stopped. We thus can't only disable planes right before starting the CRTC as it would start scanning out immediately from old frame buffers until the next vblank. Fix the problem by disabling all planes when stopping the CRTC and wait for the change to take effect. This increases the CRTC stop delay, especially when multiple CRTCs are stopped in one operation as we now wait for one vblank per CRTC. Whether this can be improved needs to be researched. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index e6a32c4e4040..a40085806f5b 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -398,6 +398,19 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc) if (!rcrtc->started) return; + /* Disable all planes and wait for the change to take effect. This is + * required as the DSnPR registers are updated on vblank, and no vblank + * will occur once the CRTC is stopped. Disabling planes when starting + * the CRTC thus wouldn't be enough as it would start scanning out + * immediately from old frame buffers until the next vblank. + * + * This increases the CRTC stop delay, especially when multiple CRTCs + * are stopped in one operation as we now wait for one vblank per CRTC. + * Whether this can be improved needs to be researched. + */ + rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0); + drm_crtc_wait_one_vblank(crtc); + /* Disable vertical blanking interrupt reporting. We first need to wait * for page flip completion before stopping the CRTC as userspace * expects page flips to eventually complete. From d6aed57481c5b746f91792c8a977f537c09e52c5 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Mon, 25 May 2015 16:32:45 +0300 Subject: [PATCH 3/7] drm: rcar-du: Fix crash with groups that have less than 9 planes 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") Cc: stable@vger.kernel.org Signed-off-by: Laurent Pinchart --- 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; From 64549cdf85a113270729aa123aa475cec7249a0f Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Tue, 26 May 2015 14:59:42 +0300 Subject: [PATCH 4/7] drm: rcar-du: Clarify error message when encoder initialization fails 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 --- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 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..ab29c179d854 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -639,6 +639,11 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu, of_node_put(encoder); of_node_put(connector); + if (ret && ret != -EPROBE_DEFER) + dev_warn(rcdu->dev, + "failed to initialize encoder %s (%d), skipping\n", + encoder->full_name, ret); + return ret < 0 ? ret : 1; } @@ -688,8 +693,6 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu) return ret; } - dev_info(rcdu->dev, - "encoder initialization failed, skipping\n"); continue; } From 898a2f387d1c663772c630e8f78ef60f1639077e Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Tue, 26 May 2015 15:07:56 +0300 Subject: [PATCH 5/7] drm: rcar-du: Convert rcar_du_encoders_init_one() return value to 0/<0 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 --- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index ab29c179d854..fa185a450aea 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 { /* @@ -644,7 +644,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu, "failed to initialize encoder %s (%d), skipping\n", encoder->full_name, ret); - return ret < 0 ? ret : 1; + return ret; } static int rcar_du_encoders_init(struct rcar_du_device *rcdu) @@ -696,7 +696,7 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu) continue; } - num_encoders += ret; + num_encoders++; } return num_encoders; From 39a3d5706527615db2b7d121b64445e79ca1cc1f Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Wed, 27 May 2015 02:14:37 +0300 Subject: [PATCH 6/7] drm: rcar-du: Clean up planes in the error paths of .atomic_commit() When the .atomic_commit() handler fails, clean up planes previoulsy prepared by drm_atomic_helper_prepare_planes() with a call to drm_atomic_helper_cleanup_planes(). Signed-off-by: Laurent Pinchart --- 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 fa185a450aea..56518eb1269a 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -495,8 +495,10 @@ static int rcar_du_atomic_commit(struct drm_device *dev, /* Allocate the commit object. */ commit = kzalloc(sizeof(*commit), GFP_KERNEL); - if (commit == NULL) - return -ENOMEM; + if (commit == NULL) { + ret = -ENOMEM; + goto error; + } INIT_WORK(&commit->work, rcar_du_atomic_work); commit->dev = dev; @@ -519,7 +521,7 @@ static int rcar_du_atomic_commit(struct drm_device *dev, if (ret) { kfree(commit); - return ret; + goto error; } /* Swap the state, this is the point of no return. */ @@ -531,6 +533,10 @@ static int rcar_du_atomic_commit(struct drm_device *dev, rcar_du_atomic_complete(commit); return 0; + +error: + drm_atomic_helper_cleanup_planes(dev, state); + return ret; } /* ----------------------------------------------------------------------------- From 263b39fe33d69da74ae61e594c94eddf75203f7d Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Wed, 27 May 2015 16:36:29 +0300 Subject: [PATCH 7/7] drm: rcar-du: Use the drm atomic state duplication helpers for planes Ensure that the duplicate and destroy plane state operations will always be in sync with the DRM core implementation of the plane state by using the __drm_atomic_helper_plane_duplicate_state() and __drm_atomic_helper_plane_destroy_state() functions designed especially for this purpose. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index d90dc428e3fd..c66986414bb4 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -302,13 +302,15 @@ rcar_du_plane_atomic_duplicate_state(struct drm_plane *plane) struct rcar_du_plane_state *state; struct rcar_du_plane_state *copy; + if (WARN_ON(!plane->state)) + return NULL; + state = to_rcar_plane_state(plane->state); copy = kmemdup(state, sizeof(*state), GFP_KERNEL); if (copy == NULL) return NULL; - if (copy->state.fb) - drm_framebuffer_reference(copy->state.fb); + __drm_atomic_helper_plane_duplicate_state(plane, ©->state); return ©->state; } @@ -316,9 +318,7 @@ rcar_du_plane_atomic_duplicate_state(struct drm_plane *plane) static void rcar_du_plane_atomic_destroy_state(struct drm_plane *plane, struct drm_plane_state *state) { - if (state->fb) - drm_framebuffer_unreference(state->fb); - + __drm_atomic_helper_plane_destroy_state(plane, state); kfree(to_rcar_plane_state(state)); }