| Message ID | 1436259140-777-1-git-send-email-sonika.jindal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 07, 2015 at 02:22:20PM +0530, Sonika Jindal wrote: > Writing to PCH_PORT_HOTPLUG for each interrupt is not required. > Handle it only if hpd has actually occurred like we handle other > interrupts. > v2: Make few variables local to if block (Ville) > v3: Add check for ibx/cpt both (Ville). > While at it, remove the redundant check for hotplug_trigger from > pch_get_hpd_pins > > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 32 +++++++++++++++++--------------- > 1 file changed, 17 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index a6fbe64..ba2c27c 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1266,9 +1266,6 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask, > *pin_mask = 0; > *long_mask = 0; > > - if (!hotplug_trigger) > - return; > - Looks like BXT still depends on that. So you should change that too, or leave this in until someone else does it. > for_each_hpd_pin(i) { > if ((hpd[i] & hotplug_trigger) == 0) > continue; > @@ -1658,14 +1655,17 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir) > struct drm_i915_private *dev_priv = dev->dev_private; > int pipe; > u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK; > - u32 dig_hotplug_reg; > - u32 pin_mask, long_mask; > > - dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); > - I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); > + if (hotplug_trigger) { > + u32 dig_hotplug_reg, pin_mask, long_mask; > > - pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_ibx); > - intel_hpd_irq_handler(dev, pin_mask, long_mask); > + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); > + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); > + > + pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, > + dig_hotplug_reg, hpd_ibx); Indentation is fubar. > + intel_hpd_irq_handler(dev, pin_mask, long_mask); > + } > > if (pch_iir & SDE_AUDIO_POWER_MASK) { > int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK) >> > @@ -1757,14 +1757,16 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir) > struct drm_i915_private *dev_priv = dev->dev_private; > int pipe; > u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT; > - u32 dig_hotplug_reg; > - u32 pin_mask, long_mask; > > - dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); > - I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); > + if (hotplug_trigger) { > + u32 dig_hotplug_reg, pin_mask, long_mask; > > - pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_cpt); > - intel_hpd_irq_handler(dev, pin_mask, long_mask); > + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); > + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); > + pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, > + dig_hotplug_reg, hpd_cpt); Ditto With those fixed this is Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > + intel_hpd_irq_handler(dev, pin_mask, long_mask); > + } > > if (pch_iir & SDE_AUDIO_POWER_MASK_CPT) { > int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK_CPT) >> > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 7/7/2015 6:51 PM, Ville Syrjälä wrote: > On Tue, Jul 07, 2015 at 02:22:20PM +0530, Sonika Jindal wrote: >> Writing to PCH_PORT_HOTPLUG for each interrupt is not required. >> Handle it only if hpd has actually occurred like we handle other >> interrupts. >> v2: Make few variables local to if block (Ville) >> v3: Add check for ibx/cpt both (Ville). >> While at it, remove the redundant check for hotplug_trigger from >> pch_get_hpd_pins >> >> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> >> --- >> drivers/gpu/drm/i915/i915_irq.c | 32 +++++++++++++++++--------------- >> 1 file changed, 17 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >> index a6fbe64..ba2c27c 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -1266,9 +1266,6 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask, >> *pin_mask = 0; >> *long_mask = 0; >> >> - if (!hotplug_trigger) >> - return; >> - > > Looks like BXT still depends on that. So you should change that too, > or leave this in until someone else does it. No, for bxt, this function is called from bxt_hpd_handler which is called only when "tmp & BXT_DE_PORT_HOTPLUG_MASK" is non-zero. So, it this is fine for bxt. > >> for_each_hpd_pin(i) { >> if ((hpd[i] & hotplug_trigger) == 0) >> continue; >> @@ -1658,14 +1655,17 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir) >> struct drm_i915_private *dev_priv = dev->dev_private; >> int pipe; >> u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK; >> - u32 dig_hotplug_reg; >> - u32 pin_mask, long_mask; >> >> - dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); >> - I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); >> + if (hotplug_trigger) { >> + u32 dig_hotplug_reg, pin_mask, long_mask; >> >> - pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_ibx); >> - intel_hpd_irq_handler(dev, pin_mask, long_mask); >> + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); >> + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); >> + >> + pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, >> + dig_hotplug_reg, hpd_ibx); > > Indentation is fubar. This was to avoid 80 char limit. > >> + intel_hpd_irq_handler(dev, pin_mask, long_mask); >> + } >> >> if (pch_iir & SDE_AUDIO_POWER_MASK) { >> int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK) >> >> @@ -1757,14 +1757,16 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir) >> struct drm_i915_private *dev_priv = dev->dev_private; >> int pipe; >> u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT; >> - u32 dig_hotplug_reg; >> - u32 pin_mask, long_mask; >> >> - dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); >> - I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); >> + if (hotplug_trigger) { >> + u32 dig_hotplug_reg, pin_mask, long_mask; >> >> - pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_cpt); >> - intel_hpd_irq_handler(dev, pin_mask, long_mask); >> + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); >> + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); >> + pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, >> + dig_hotplug_reg, hpd_cpt); > > Ditto > This was to avoid 80 char limit. > With those fixed this is > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> + intel_hpd_irq_handler(dev, pin_mask, long_mask); >> + } >> >> if (pch_iir & SDE_AUDIO_POWER_MASK_CPT) { >> int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK_CPT) >> >> -- >> 1.7.10.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6740
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK 302/302 302/302
SNB 312/316 312/316
IVB 343/343 343/343
BYT -2 287/287 285/287
HSW 380/380 380/380
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*BYT igt@gem_partial_pwrite_pread@reads PASS(1) FAIL(1)
*BYT igt@gem_partial_pwrite_pread@reads-uncached PASS(1) FAIL(1)
Note: You need to pay more attention to line start with '*'
On Wed, Jul 08, 2015 at 10:40:18AM +0530, Jindal, Sonika wrote: > > > On 7/7/2015 6:51 PM, Ville Syrjälä wrote: > > On Tue, Jul 07, 2015 at 02:22:20PM +0530, Sonika Jindal wrote: > >> Writing to PCH_PORT_HOTPLUG for each interrupt is not required. > >> Handle it only if hpd has actually occurred like we handle other > >> interrupts. > >> v2: Make few variables local to if block (Ville) > >> v3: Add check for ibx/cpt both (Ville). > >> While at it, remove the redundant check for hotplug_trigger from > >> pch_get_hpd_pins > >> > >> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_irq.c | 32 +++++++++++++++++--------------- > >> 1 file changed, 17 insertions(+), 15 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > >> index a6fbe64..ba2c27c 100644 > >> --- a/drivers/gpu/drm/i915/i915_irq.c > >> +++ b/drivers/gpu/drm/i915/i915_irq.c > >> @@ -1266,9 +1266,6 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask, > >> *pin_mask = 0; > >> *long_mask = 0; > >> > >> - if (!hotplug_trigger) > >> - return; > >> - > > > > Looks like BXT still depends on that. So you should change that too, > > or leave this in until someone else does it. > No, for bxt, this function is called from bxt_hpd_handler which is > called only when "tmp & BXT_DE_PORT_HOTPLUG_MASK" is non-zero. > So, it this is fine for bxt. Oh, right you are. No problem then. > > > > >> for_each_hpd_pin(i) { > >> if ((hpd[i] & hotplug_trigger) == 0) > >> continue; > >> @@ -1658,14 +1655,17 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir) > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> int pipe; > >> u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK; > >> - u32 dig_hotplug_reg; > >> - u32 pin_mask, long_mask; > >> > >> - dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); > >> - I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); > >> + if (hotplug_trigger) { > >> + u32 dig_hotplug_reg, pin_mask, long_mask; > >> > >> - pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_ibx); > >> - intel_hpd_irq_handler(dev, pin_mask, long_mask); > >> + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); > >> + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); > >> + > >> + pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, > >> + dig_hotplug_reg, hpd_ibx); > > > > Indentation is fubar. > This was to avoid 80 char limit. You have it like this + pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, + dig_hotplug_reg, hpd_ibx); and it should look like this + pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, + dig_hotplug_reg, hpd_ibx); that is the arguments on following lines should line up after the opening '(' on the first line. Sane editors can do this automagically for you. > > > >> + intel_hpd_irq_handler(dev, pin_mask, long_mask); > >> + } > >> > >> if (pch_iir & SDE_AUDIO_POWER_MASK) { > >> int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK) >> > >> @@ -1757,14 +1757,16 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir) > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> int pipe; > >> u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT; > >> - u32 dig_hotplug_reg; > >> - u32 pin_mask, long_mask; > >> > >> - dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); > >> - I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); > >> + if (hotplug_trigger) { > >> + u32 dig_hotplug_reg, pin_mask, long_mask; > >> > >> - pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_cpt); > >> - intel_hpd_irq_handler(dev, pin_mask, long_mask); > >> + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); > >> + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); > >> + pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, > >> + dig_hotplug_reg, hpd_cpt); > > > > Ditto > > > This was to avoid 80 char limit. > > > With those fixed this is > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > >> + intel_hpd_irq_handler(dev, pin_mask, long_mask); > >> + } > >> > >> if (pch_iir & SDE_AUDIO_POWER_MASK_CPT) { > >> int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK_CPT) >> > >> -- > >> 1.7.10.4 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index a6fbe64..ba2c27c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1266,9 +1266,6 @@ static void pch_get_hpd_pins(u32 *pin_mask, u32 *long_mask, *pin_mask = 0; *long_mask = 0; - if (!hotplug_trigger) - return; - for_each_hpd_pin(i) { if ((hpd[i] & hotplug_trigger) == 0) continue; @@ -1658,14 +1655,17 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir) struct drm_i915_private *dev_priv = dev->dev_private; int pipe; u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK; - u32 dig_hotplug_reg; - u32 pin_mask, long_mask; - dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); - I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); + if (hotplug_trigger) { + u32 dig_hotplug_reg, pin_mask, long_mask; - pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_ibx); - intel_hpd_irq_handler(dev, pin_mask, long_mask); + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); + + pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, + dig_hotplug_reg, hpd_ibx); + intel_hpd_irq_handler(dev, pin_mask, long_mask); + } if (pch_iir & SDE_AUDIO_POWER_MASK) { int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK) >> @@ -1757,14 +1757,16 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir) struct drm_i915_private *dev_priv = dev->dev_private; int pipe; u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT; - u32 dig_hotplug_reg; - u32 pin_mask, long_mask; - dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); - I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); + if (hotplug_trigger) { + u32 dig_hotplug_reg, pin_mask, long_mask; - pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd_cpt); - intel_hpd_irq_handler(dev, pin_mask, long_mask); + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); + pch_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, + dig_hotplug_reg, hpd_cpt); + intel_hpd_irq_handler(dev, pin_mask, long_mask); + } if (pch_iir & SDE_AUDIO_POWER_MASK_CPT) { int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK_CPT) >>
Writing to PCH_PORT_HOTPLUG for each interrupt is not required. Handle it only if hpd has actually occurred like we handle other interrupts. v2: Make few variables local to if block (Ville) v3: Add check for ibx/cpt both (Ville). While at it, remove the redundant check for hotplug_trigger from pch_get_hpd_pins Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-)