[Intel-gfx] [PATCH] drm/i915/dsb: fix cmd_buf being wrongly set

Lucas De Marchi lucas.demarchi at intel.com
Wed Nov 27 22:11:21 UTC 2019


The "err" label is not really "err", but rather "out" since the return
path is shared between error condition and normal path. This broke when
commit 03cea61076f0 ("drm/i915/dsb: fix extra warning on error path
handling") added a "dsb->cmd_buf = NULL;" there, making DSB to stop
working since now all writes would pass-through via mmio.

Remove the set to NULL since it's actually not needed: we only set it if
all steps are succesful. While at it, rename the label so this confusion
doesn't happen again.

Fixes: 03cea61076f0 ("drm/i915/dsb: fix extra warning on error path handling"
Resolves: https://gitlab.freedesktop.org/drm/intel/issues/8
Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
---

Right now I don't have access to the hw to reproduce it, so this is
build-tested only.

 drivers/gpu/drm/i915/display/intel_dsb.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 5bf67bdc8182..ada006a690df 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -116,34 +116,34 @@ intel_dsb_get(struct intel_crtc *crtc)
 	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
 	if (IS_ERR(obj)) {
 		DRM_ERROR("Gem object creation failed\n");
-		goto err;
+		goto out;
 	}
 
 	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
 	if (IS_ERR(vma)) {
 		DRM_ERROR("Vma creation failed\n");
 		i915_gem_object_put(obj);
-		goto err;
+		goto out;
 	}
 
 	buf = i915_gem_object_pin_map(vma->obj, I915_MAP_WC);
 	if (IS_ERR(buf)) {
 		DRM_ERROR("Command buffer creation failed\n");
-		goto err;
+		goto out;
 	}
 
 	dsb->id = DSB1;
 	dsb->vma = vma;
 	dsb->cmd_buf = buf;
 
-err:
+out:
 	/*
-	 * Set cmd_buf to NULL so the writes pass-through, but leave the
-	 * dangling refcount to be removed later by the corresponding
-	 * intel_dsb_put(): the important error message will already be
-	 * logged above.
+	 * On error dsb->cmd_buf will continue to be NULL, making the writes
+	 * pass-through. Leave the dangling ref to be removed later by the
+	 * corresponding intel_dsb_put(): the important error message will
+	 * already be logged above.
 	 */
-	dsb->cmd_buf = NULL;
+
 	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 
 	return dsb;
-- 
2.24.0



More information about the Intel-gfx mailing list