On Fri, Oct 25, 2013 at 12:27:42AM +0000, Liu, Chuansheng wrote: > Hello Chris and Ben, > > > -----Original Message----- > > From: Ben Widawsky [mailto:ben@xxxxxxxxxxxx] > > Sent: Friday, October 25, 2013 4:57 AM > > To: Chris Wilson; Liu, Chuansheng; daniel.vetter@xxxxxxxx; airlied@xxxxxxxx; > > intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; > > linux-kernel@xxxxxxxxxxxxxxx; Li, Fei > > Subject: Re: [Intel-gfx] drm/i915: Avoid accessing the stolen address when it is > > unavailable > > > > On Thu, Oct 24, 2013 at 01:17:06PM +0100, Chris Wilson wrote: > > > On Fri, Oct 25, 2013 at 12:33:47AM +0800, Chuansheng Liu wrote: > > > > > > > > In our platform, we hit the the stolen region initialization failure case, > > > > such as below log: > > > > [drm:i915_stolen_to_physical] *ERROR* conflict detected with stolen > > region: [0x7b000000] > > > > > > > > And it causes the dev_priv->mm.stolen_base is NULL, in this case, we > > should > > > > avoid accessing it any more. > > > > > > > > Here is possible call trace: > > > > intel_enable_gt_powersave -- > > > > > valleyview_enable_rps -- > > > > > valleyview_setup_pctx > > > > > > The two create_stolen routines are no-ops in that case so all that > > > happens instead is that we read VLV_PCBR. However, really if > > > i915_gem_object_create_stolen_for_preallocated() fails we should abort > > > loading the driver as it means we have a hardware conflict and undefined > > > behaviour. > In case of dev_priv->mm.stolen_base == NULL, and the valleyview_setup_pctx() is called > at the first time, it will call i915_gem_object_create_stolen_for_preallocated(), which should > should return NULL always due to (!drm_mm_initialized(&dev_priv->mm.stolen)). > > After that, every time specially when doing pm operation, the above scenario will > be called again and again. > > Here this patch is to save some time for PM operation, we do not need to read > VLV_PCBR and pcbr_offset calculation in case of stolen_base == NULL. > > Is it making sense? Thanks. I see. No, it is a pointless optimisation that leaks knowledge about internals of another subsystem to paper over a kernel bug. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
