On 05/19/2015 11:26 AM, Daniel Vetter wrote: > On Tue, May 19, 2015 at 09:58:52AM +0300, Abdiel Janulgue wrote: >> >> >> On 05/18/2015 07:07 PM, Ville Syrjälä wrote: >>> On Mon, May 18, 2015 at 04:41:51PM +0100, Chris Wilson wrote: >>>> On Mon, May 18, 2015 at 06:36:18PM +0300, Ville Syrjälä wrote: >>>>> On Mon, May 18, 2015 at 11:31:56AM +0300, Abdiel Janulgue wrote: >>>>>> Also clarify comments on context size that the extra state for >>>>>> Resource Streamer is included. >>>>>> >>>>>> Suggested-by: Chris Wilson <ch...@chris-wilson.co.uk> >>>>>> Signed-off-by: Abdiel Janulgue <abdiel.janul...@linux.intel.com> >>>>>> --- >>>>>> drivers/gpu/drm/i915/i915_gem_context.c | 2 +- >>>>>> drivers/gpu/drm/i915/i915_reg.h | 3 ++- >>>>>> 2 files changed, 3 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c >>>>>> b/drivers/gpu/drm/i915/i915_gem_context.c >>>>>> index f3e84c4..1db107a 100644 >>>>>> --- a/drivers/gpu/drm/i915/i915_gem_context.c >>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c >>>>>> @@ -509,7 +509,7 @@ mi_set_context(struct intel_engine_cs *ring, >>>>>> } >>>>>> >>>>>> /* These flags are for resource streamer on HSW+ */ >>>>>> - if (!IS_HASWELL(ring->dev) && INTEL_INFO(ring->dev)->gen < 8) >>>>>> + if (IS_HASWELL(ring->dev)) >>>>>> flags |= (MI_SAVE_EXT_STATE_EN | >>>>>> MI_RESTORE_EXT_STATE_EN); >>>>> >>>>> I don't get it. Previously we told the hardware to save the extended >>>>> context on !hsw, and now we don't. That doesn't seem correct to me. >>>> >>>> We don't use the extended state elsewhere. >>> >>> Umm. On SNB at least 3DSTATE_POLY_STIPPLE_PATTERN seems to be part of >>> the extended state, and on IVB/VLV SOL state is there. Mesa uses all of >>> that. >>> >>>> I'd always been dubious of >>>> the origins/intentions of this line of code since it claims only to be >>>> for enabling RS on HSW... >>>> >>>> i.e. commit e80f14b6d36e3e07111cf2ab084ef8dd5d015ce2 >>>> Author: Ben Widawsky <benjamin.widaw...@intel.com> >>>> Date: Mon Aug 18 10:35:28 2014 -0700 >>>> >>>> drm/i915: Don't save/restore RS when not used >>>> >>>> was backwards. >>> >>> ? It did exactly what it said, ie. avoid setting the RS save/restore >>> bits on HSW+ while leaving the ext save/restore enabled on older >>> platforms. >>> >> >> Another option is to enable extended state save restore for both HSW and >> the older platforms so we would get both features (RS save/restore and >> Extended State Save enable)? >> >> Seems the reason for this confusion is that the we have the exact same >> bit positions in MI_SET_CONTEXT but it got renamed starting from HSW and up. > > If the bit has been renamed it might be good to add at least a new #define > with a _HSw suffix and better name, just to make it clear what's going on. > gcc will see through this and fold down the different conditions to just > one.
I'll do that in the next revision and also add the igt tag you require. In the meantime, please check the igt approach I sent is the correct one. Thanks! -Abdiel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
