[Intel-gfx] [PATCH v4 2/6] drm/i915: tidy up initialisation failure paths (GEM & LRC)
Dave Gordon
david.s.gordon at intel.com
Mon Feb 1 10:45:25 CET 2016
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.
>>>
>>> Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
>>> ---
>>> 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.
.Dave.
More information about the Intel-gfx
mailing list