Re: [PATCH 05/10] drm/i915: Propagating correct error codes to the userspace

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Dec 22, 2015 at 12:02:17PM +0000, Tvrtko Ursulin wrote:
> 
> On 22/12/15 11:29, Ankitprasad Sharma wrote:
> >On Tue, 2015-12-22 at 11:20 +0000, Tvrtko Ursulin wrote:
> >>On 22/12/15 06:20, ankitprasad.r.sharma@intel.com wrote:
> >>>diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >>>index 9968c66..0afb819 100644
> >>>--- a/drivers/gpu/drm/i915/intel_pm.c
> >>>+++ b/drivers/gpu/drm/i915/intel_pm.c
> >>>@@ -5174,10 +5174,11 @@ static void valleyview_setup_pctx(struct drm_device *dev)
> >>>   								      pcbr_offset,
> >>>   								      I915_GTT_OFFSET_NONE,
> >>>   								      pctx_size);
> >>>-		goto out;
> >>>+		if (!IS_ERR(pctx))
> >>>+			goto out;
> >>
> >>This is a change in behaviour, is it OK in this patch or needs to be
> >>split out?
> >>
> >>Previously if i915_gem_object_create_stolen_for_preallocated failed it
> >>would leave it at that. Now it tries a fallback path.
> >I think I should fallback to something like this,
> >
> >if (IS_ERR(pctx))
> >	pctx = NULL;
> >
> >goto out;
> >
> >
> >As keeping pctx as an ERROR POINTER might affect other NULL checks for
> >the pointer, and no change in behaviour. Is this okay?
> 
> I think so, it is then keeping the existing behaviour 1:1.

Afair if the bios allocated the pwrctx on vlv already we can't move it or
reallocate or anything. So if there's indeed a bug and we can't get this
preallocated range then doing a fallback would be a bug.

We probably should put a DRM_ERROR in there if this step fails. Can you
pls do such a follow-up patch?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx