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@xxxxxxxxx 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@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
