| Message ID | 20190705124325.14270-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/gt: Apply RCS workarounds to the render class | expand |
On Fri, 2019-07-05 at 13:43 +0100, Chris Wilson wrote: > We now track features correctly instead of probing i915->engine[RCS0] > which is much more flexible and avoids any nasty surprises. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > index df5932f5f578..bdf279fa3b2e 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > @@ -448,12 +448,6 @@ int intel_engines_init_mmio(struct > drm_i915_private *i915) > if (WARN_ON(mask != engine_mask)) > device_info->engine_mask = mask; > > - /* We always presume we have at least RCS available for later > probing */ > - if (WARN_ON(!HAS_ENGINE(i915, RCS0))) { > - err = -ENODEV; > - goto cleanup; > - } > - Just going by the series here, we have quite a few other place we are touching RCS0 specifically during driver load. Do we really want to get rid of this? Or is there an alternative if RCS0 isn't present for some reason? Thanks, Stuart > RUNTIME_INFO(i915)->num_engines = hweight32(mask); > > intel_gt_check_and_clear_faults(&i915->gt);
Quoting Summers, Stuart (2019-07-08 22:11:15) > On Fri, 2019-07-05 at 13:43 +0100, Chris Wilson wrote: > > We now track features correctly instead of probing i915->engine[RCS0] > > which is much more flexible and avoids any nasty surprises. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > --- > > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 6 ------ > > 1 file changed, 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > index df5932f5f578..bdf279fa3b2e 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > @@ -448,12 +448,6 @@ int intel_engines_init_mmio(struct > > drm_i915_private *i915) > > if (WARN_ON(mask != engine_mask)) > > device_info->engine_mask = mask; > > > > - /* We always presume we have at least RCS available for later > > probing */ > > - if (WARN_ON(!HAS_ENGINE(i915, RCS0))) { > > - err = -ENODEV; > > - goto cleanup; > > - } > > - > > Just going by the series here, we have quite a few other place we are > touching RCS0 specifically during driver load. Do we really want to get > rid of this? Or is there an alternative if RCS0 isn't present for some > reason? Outside of gvt/ (which I don't dare to try and verify), the only places where we now mention RCS0 are in direct hw mappings to that engine (e.g. interrupts and mmio setup). [Excluding selftests/ which are mostly converted and really just a matter of generalising if applicable or marking as "no really, this only applies to RCS0".] Assuming the other couple of patches also land. -Chris
On 05/07/2019 13:43, Chris Wilson wrote: > We now track features correctly instead of probing i915->engine[RCS0] > which is much more flexible and avoids any nasty surprises. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > index df5932f5f578..bdf279fa3b2e 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > @@ -448,12 +448,6 @@ int intel_engines_init_mmio(struct drm_i915_private *i915) > if (WARN_ON(mask != engine_mask)) > device_info->engine_mask = mask; > > - /* We always presume we have at least RCS available for later probing */ > - if (WARN_ON(!HAS_ENGINE(i915, RCS0))) { > - err = -ENODEV; > - goto cleanup; > - } > - > RUNTIME_INFO(i915)->num_engines = hweight32(mask); > > intel_gt_check_and_clear_faults(&i915->gt); > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
On Mon, 2019-07-08 at 22:16 +0100, Chris Wilson wrote: > Quoting Summers, Stuart (2019-07-08 22:11:15) > > On Fri, 2019-07-05 at 13:43 +0100, Chris Wilson wrote: > > > We now track features correctly instead of probing i915- > > > >engine[RCS0] > > > which is much more flexible and avoids any nasty surprises. > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > > --- > > > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 6 ------ > > > 1 file changed, 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > > b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > > index df5932f5f578..bdf279fa3b2e 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > > @@ -448,12 +448,6 @@ int intel_engines_init_mmio(struct > > > drm_i915_private *i915) > > > if (WARN_ON(mask != engine_mask)) > > > device_info->engine_mask = mask; > > > > > > - /* We always presume we have at least RCS available for > > > later > > > probing */ > > > - if (WARN_ON(!HAS_ENGINE(i915, RCS0))) { > > > - err = -ENODEV; > > > - goto cleanup; > > > - } > > > - > > > > Just going by the series here, we have quite a few other place we > > are > > touching RCS0 specifically during driver load. Do we really want to > > get > > rid of this? Or is there an alternative if RCS0 isn't present for > > some > > reason? > > Outside of gvt/ (which I don't dare to try and verify), the only > places > where we now mention RCS0 are in direct hw mappings to that engine > (e.g. interrupts and mmio setup). [Excluding selftests/ which are > mostly > converted and really just a matter of generalising if applicable or > marking as "no really, this only applies to RCS0".] Assuming the > other > couple of patches also land. Worst case, we don't have any platforms without RCS0, so getting rid of this shouldn't cause harm. We can always work through issues if we ever have a platform without this for some reason. Reviewed-by: Stuart Summers <stuart.summers@intel.com> Thanks, Stuart > -Chris
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index df5932f5f578..bdf279fa3b2e 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -448,12 +448,6 @@ int intel_engines_init_mmio(struct drm_i915_private *i915) if (WARN_ON(mask != engine_mask)) device_info->engine_mask = mask; - /* We always presume we have at least RCS available for later probing */ - if (WARN_ON(!HAS_ENGINE(i915, RCS0))) { - err = -ENODEV; - goto cleanup; - } - RUNTIME_INFO(i915)->num_engines = hweight32(mask); intel_gt_check_and_clear_faults(&i915->gt);
We now track features correctly instead of probing i915->engine[RCS0] which is much more flexible and avoids any nasty surprises. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 6 ------ 1 file changed, 6 deletions(-)