Re: [PATCH 2/2] drm/i915/dsb: fix extra warning on error path handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Nov 15, 2019 at 12:52:49PM -0800, Matt Roper wrote:
On Mon, Nov 11, 2019 at 12:50:25PM -0800, Lucas De Marchi wrote:
When we call intel_dsb_get(), the dsb initialization may fail for
various reasons. We already log the error message in that path, making
it unnecessary to trigger a warning that refcount == 0 when calling
intel_dsb_put().

So here we simplify the logic and do lazy shutdown: leaving the extra
refcount alive so when we call intel_dsb_put() we end up calling
i915_vma_unpin_and_release().

Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>

Due to the lack of any actual concurrency, it seems like we could
eventually get rid of the whole get/put design and just allocate the
buffer once (and pin it during the prepare step).  But this seems good

I assumed this was designed to accept the pattern

intel_dsb_get();
intel_dsb_get();
intel_dsb_put();
intel_dsb_put();

enough for now.

Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>

thanks
Lucas De Marchi



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

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 4feebbeb0b0c..858af6be9c36 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -102,6 +102,7 @@ intel_dsb_get(struct intel_crtc *crtc)
 	struct intel_dsb *dsb = &crtc->dsb;
 	struct drm_i915_gem_object *obj;
 	struct i915_vma *vma;
+	u32 *buf;
 	intel_wakeref_t wakeref;

 	if (!HAS_DSB(i915))
@@ -110,7 +111,6 @@ intel_dsb_get(struct intel_crtc *crtc)
 	if (++dsb->refcount != 1)
 		return dsb;

-	dsb->id = DSB1;
 	wakeref = intel_runtime_pm_get(&i915->runtime_pm);

 	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
@@ -123,22 +123,29 @@ intel_dsb_get(struct intel_crtc *crtc)
 	if (IS_ERR(vma)) {
 		DRM_ERROR("Vma creation failed\n");
 		i915_gem_object_put(obj);
-		dsb->refcount--;
 		goto err;
 	}

-	dsb->cmd_buf = i915_gem_object_pin_map(vma->obj, I915_MAP_WC);
-	if (IS_ERR(dsb->cmd_buf)) {
+	buf = i915_gem_object_pin_map(vma->obj, I915_MAP_WC);
+	if (IS_ERR(buf)) {
 		DRM_ERROR("Command buffer creation failed\n");
-		i915_vma_unpin_and_release(&vma, 0);
-		dsb->cmd_buf = NULL;
-		dsb->refcount--;
 		goto err;
 	}
+
+	dsb->id = DSB1;
 	dsb->vma = vma;
+	dsb->cmd_buf = buf;

 err:
+	/*
+	 * 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.
+	 */
+	dsb->cmd_buf = NULL;
 	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
+
 	return dsb;
 }

--
2.24.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux