Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Mika Kuoppala <mika.kuoppala@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
Date: Thu, 6 Apr 2017 19:25:10 +0300	[thread overview]
Message-ID: <20170406162510.GA30290@intel.com> (raw)
In-Reply-To: <20170406160510.GL10496@nuc-i3427.alporthouse.com>

On Thu, Apr 06, 2017 at 05:05:10PM +0100, Chris Wilson wrote:
> On Thu, Apr 06, 2017 at 06:46:29PM +0300, Ville Syrjälä wrote:
> > On Thu, Apr 06, 2017 at 06:39:42PM +0300, Mika Kuoppala wrote:
> > > +static bool
> > >  check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> > >  {
> > > +	bool ret = false;
> > > +
> > >  	if (HAS_FPGA_DBG_UNCLAIMED(dev_priv))
> > > -		return fpga_check_for_unclaimed_mmio(dev_priv);
> > > +		ret |= fpga_check_for_unclaimed_mmio(dev_priv);
> > >  
> > >  	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > > -		return vlv_check_for_unclaimed_mmio(dev_priv);
> > > +		ret |= vlv_check_for_unclaimed_mmio(dev_priv);
> > >  
> > > -	return false;
> > > +	if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv))
> > > +		ret |= gen6_check_for_fifo_debug(dev_priv);
> > 
> > I'd prefer to keep unclaim vs. wake FIFO separate because the
> > unclaimned stuff is only for display registers. In my plan to
> > split the uncore lock into gt and display locks the unclaimed
> > reg stuff would end up being protected by the display lock rather
> > than the gt lock.
> 
> I don't think it is much of a hindrance, right? We just split it out when
> splitting dpy vs gt.

I suppose. Although if we want to do the FIFO checks for non-GT stuff as
well, then I guess we'd have to risk hitting the FIFO register from
both the display and gt code paths. Not sure if that's safe or not.

> Mika was digging through GTFIFODBG and thought it
> had some bits for a sideband underflow...
> 
> Random thought, would i915->mmio.writeX[reg < 0x40000](i915, reg, val)
> or just push all the decisions to the backend?  I hope gcc would be able
> to automatically store the function pointer i915->mmio.writeX[reg < 0x40000]

I haven't done any real performance analysis on this stuff TBH, apart
from looking at the duration of the atomic commits. I guess separate
function pointers might be nice for code readability at least.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2017-04-06 16:25 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-06  8:44 [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework Mika Kuoppala
2017-04-06  8:44 ` [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo Mika Kuoppala
2017-04-06  9:12   ` Chris Wilson
2017-04-06  9:32     ` Mika Kuoppala
2017-04-06 15:40     ` Mika Kuoppala
2017-04-06 15:46       ` Chris Wilson
2017-05-03  9:54         ` Mika Kuoppala
2017-05-02 14:03       ` Mika Kuoppala
2017-04-06  9:03 ` [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework Chris Wilson
2017-04-06  9:14   ` Chris Wilson
2017-04-06 15:39     ` Mika Kuoppala
2017-04-06 15:46       ` Ville Syrjälä
2017-04-06 16:05         ` Chris Wilson
2017-04-06 16:25           ` Ville Syrjälä [this message]
2017-04-06  9:35 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
2017-04-06 11:13   ` Mika Kuoppala
2017-04-06 11:32     ` Chris Wilson
2017-04-06 16:04 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev2) Patchwork
2017-04-06 16:24 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev3) Patchwork
2017-05-02 14:27 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev4) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170406162510.GA30290@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=mika.kuoppala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox