On Wed, Jun 08, 2016 at 12:49:14PM +0100, Tvrtko Ursulin wrote:
>
> On 08/06/16 12:10, Chris Wilson wrote:
> >On Wed, Jun 08, 2016 at 11:18:59AM +0100, Tvrtko Ursulin wrote:
> >>
> >>On 08/06/16 11:01, Chris Wilson wrote:
> >>>On Tue, Jun 07, 2016 at 01:46:53PM +0100, Tvrtko Ursulin wrote:
> >>>>
> >>>>On 03/06/16 17:08, Chris Wilson wrote:
> >>>>>With only a single callsite for intel_engine_cs->irq_get and ->irq_put,
> >>>>>we can reduce the code size by moving the common preamble into the
> >>>>>caller, and we can also eliminate the reference counting.
> >>>>>
> >>>>>For completeness, as we are no longer doing reference counting on irq,
> >>>>>rename the get/put vfunctions to enable/disable respectively.
> >>>>>
> >>>>>Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >>>>>---
> >>>>> drivers/gpu/drm/i915/i915_irq.c | 8 +-
> >>>>> drivers/gpu/drm/i915/intel_breadcrumbs.c | 10 +-
> >>>>> drivers/gpu/drm/i915/intel_lrc.c | 34 +---
> >>>>> drivers/gpu/drm/i915/intel_ringbuffer.c | 269 ++++++++++---------------------
> >>>>> drivers/gpu/drm/i915/intel_ringbuffer.h | 5 +-
> >>>>> 5 files changed, 108 insertions(+), 218 deletions(-)
> >>>>>
> >>>>>diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >>>>>index 14b3d65bb604..5bdb433dde8c 100644
> >>>>>--- a/drivers/gpu/drm/i915/i915_irq.c
> >>>>>+++ b/drivers/gpu/drm/i915/i915_irq.c
> >>>>>@@ -259,12 +259,12 @@ static void ilk_update_gt_irq(struct drm_i915_private *dev_priv,
> >>>>> dev_priv->gt_irq_mask &= ~interrupt_mask;
> >>>>> dev_priv->gt_irq_mask |= (~enabled_irq_mask & interrupt_mask);
> >>>>> I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
> >>>>>- POSTING_READ(GTIMR);
> >>>>> }
> >>>>>
> >>>>> void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask)
> >>>>> {
> >>>>> ilk_update_gt_irq(dev_priv, mask, mask);
> >>>>>+ POSTING_READ_FW(GTIMR);
> >>>>> }
> >>>>
> >>>>Unrelated hunks?
> >>>>
> >>>>How is POSTING_READ_FW correct?
> >>>
> >>>The requirement here is an uncached read of the mmio register in order
> >>>to flush the previous write to hw. A grander scheme would be to convert
> >>>all posting reads, but that requires double checking to see if anyone
> >>>has been cheating!
> >>
> >>So what prevents to force-wake for getting released between the
> >>I915_WRITE and POSTING_READ_FW ?
> >
> >Nothing. The point is that the FW is not required for the correctness or
> >operation of the POSTING_READ as a barrier to hardware enabling the
> >interrupt.
>
> So sleeping hardware is OK with being read from? It won't hang or
> anything, just provide bad data?
Just garbage.
> Why not change POSTING_READ to be I915_READ_FW always then?
First plan was to purge all the posting-reads. That proves some are
required. So now, if it appears in a profile, it is asked to justify its
existence.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
