On Tue, Aug 02, 2016 at 03:47:29PM +0100, Chris Wilson wrote:
> On Tue, Aug 02, 2016 at 04:34:25PM +0200, Daniel Vetter wrote:
> > On Fri, Jul 29, 2016 at 12:10:30PM +0100, Chris Wilson wrote:
> > > On Fri, Jul 29, 2016 at 10:57:49AM +0200, Daniel Vetter wrote:
> > > > On Fri, Jul 29, 2016 at 11:42:24AM +0300, Joonas Lahtinen wrote:
> > > > > On pe, 2016-07-29 at 00:45 +0100, Chris Wilson wrote:
> > > > > > It appears that we never told Pineview it has a RENDER_RING. This was
> > > > > > all fine until we started using the ring_mask for determining all the
> > > > > > available rings to initialise for legacy ringbuffer submission in commit
> > > > > > 88d2ba2e95c8 ("drm/i915: Unify engine init loop"). Though really it is a
> > > > > > latent bug since the ring_mask inception in commit 73ae478cdf6a
> > > > > > ("drm/i915: Replace has_bsd/blt/vebox with a mask").
> > > > > >
> > > > > > To prevent similar mishaps in future, add a WARN_ON() if we find
> > > > > > ourselves with a device without any rings.
> > > > > >
> > > > > > Fixes: 73ae478cdf6a ("drm/i915: Replace has_bsd/blt/vebox with a mask")
> > > > > > Fixes: 88d2ba2e95c8 ("drm/i915: Unify engine init loop")
> > > > > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > > > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > > > > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > > > > > Cc: Ben Widawsky <ben@xxxxxxxxxxxx>
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/i915_pci.c | 1 +
> > > > > > drivers/gpu/drm/i915/intel_engine_cs.c | 1 +
> > > > > > 2 files changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> > > > > > index 949c01686a66..2587b1bd41f4 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_pci.c
> > > > > > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > > > > > @@ -173,6 +173,7 @@ static const struct intel_device_info intel_pineview_info = {
> > > > > > .gen = 3, .is_g33 = 1, .is_pineview = 1, .is_mobile = 1, .num_pipes = 2,
> > > > > > .need_gfx_hws = 1, .has_hotplug = 1,
> > > > > > .has_overlay = 1,
> > > > > > + .ring_mask = RENDER_RING,
> > > > > > GEN_DEFAULT_PIPEOFFSETS,
> > > > > > CURSOR_OFFSETS,
> > > > > > };
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> > > > > > index 31d43dfa7469..e6422aac2919 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > > > > > @@ -117,6 +117,7 @@ int intel_engines_init(struct drm_device *dev)
> > > > > > unsigned int i;
> > > > > > int ret;
> > > > > >
> > > > > > + WARN_ON(INTEL_INFO(dev_priv)->ring_mask == 0);
> > > > >
> > > > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> > > >
> > > > dim says this also needs a
> > > >
> > > > Cc: drm-intel-fixes@xxxxxxxxxxxxxxxxxxxxx
> > >
> > > dim says nothing of the sort for me. The bug is only relevant from
> > > 88d2ba2e95c8
> >
> > $ dim fixes 88d2ba2e95c8
>
> What state does dim fixes depend upon? If did not report anything when I
> ran it against the patch before pushing.
3 cases for where the offending commit is:
- It's in a release tag -> cc: stable
- It's in an -rc tag -> cc: fixes
- It tries to make an educate guess for when stuff needs to be
cherry-picked to dinf.
It's not entirely correct. tbh I'm not sure whether dim fixes is a good
idea, or whether we should just pimp the script to figure this out at
cherry-pick time. This is kinda still open and Jani&me are trying to
figure out what is best. Suggestions welcome.
In the end I simply did another scan for Fixes: when cherry-picking to
make sure I didn't miss anything.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
