Re: [PATCH v4 2/6] drm/i915: tidy up initialisation failure paths (GEM & LRC)

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

 



On 11/02/16 08:47, Daniel Vetter wrote:
On Mon, Feb 01, 2016 at 09:45:25AM +0000, Dave Gordon wrote:
On 30/01/16 11:28, Chris Wilson wrote:
On Sat, Jan 30, 2016 at 10:56:27AM +0000, Chris Wilson wrote:
On Fri, Jan 29, 2016 at 07:19:27PM +0000, Dave Gordon wrote:
1. add call to i915_gem_context_fini() to deallocate the default
    context(s) if the call to init_rings() fails, so that we don't
    leak the context in that situation.

2. remove useless code in intel_logical_ring_cleanup(), presumably
    copypasted from legacy ringbuffer version at creation.


If your commit message has a list of independent changes ...

Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_gem.c  |  5 ++++-
  drivers/gpu/drm/i915/intel_lrc.c | 10 ++--------
  2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a928823..5a4d468 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4986,8 +4986,11 @@ int i915_gem_init(struct drm_device *dev)
  		goto out_unlock;

  	ret = dev_priv->gt.init_rings(dev);
-	if (ret)
+	if (ret) {
+		i915_gem_context_fini(dev);
+		/* XXX: anything else to be undone here? */

Yes. Make this a separate patch and begin the onion unwind.

Hmm. Actually, we have to make sure that we can still modeset if this
function fails - that is anything but utter catastrophe should just
result in loss of functionality (no stolen, no GEM execution etc) but we
can still drive the displays so the user can see how bad the damage is.
-Chris

Yes, Mika said that's why (he thought) there wasn't a complete reversal of
everything the driver has done up to this point.

The addition of the context_fini() seems reasonable, that's going to make it
leak a bit less, while still leaving basic framebuffer working.

Could be a separate patch if you like, but hardly seems worth splitting from
the other chunk, which after all only replaces unreachable code with a
WARNing.

... it should be split. As a rule of thumb at least. I don't really care
all that much here though, so jut for the future.
-Daniel

That was already done in the updated (v5) patchset posted 2016-02-05.
This (v4) sequence is therefore already obsolete.

.Dave.

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux