| Message ID | 1436761070-29138-1-git-send-email-sonika.jindal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 7/13/2015 9:47 AM, Sonika Jindal wrote: > As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic > and interrupts to check the external panel connection. > And remove the redundant comment. > > v2: Remove redundant IS_BROXTON check, Add comment about port C not > connected, and rephrase the commit message to include only what we > are doing here (Imre) > > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 38 +++++++++++++++++++++++++++++--------- > 1 file changed, 29 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index eb52a03..760539c 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -88,7 +88,11 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = { > [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS > }; > > -/* BXT hpd list */ > +/* Port C is not connected on bxt A0/A1 */ > +static const u32 hpd_bxt_a0[HPD_NUM_PINS] = { > + [HPD_PORT_B] = BXT_DE_PORT_HP_DDIA > +}; > + > static const u32 hpd_bxt[HPD_NUM_PINS] = { > [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB, > [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC > @@ -2257,6 +2261,7 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status) > struct drm_i915_private *dev_priv = dev->dev_private; > u32 hp_control, hp_trigger; > u32 pin_mask, long_mask; > + const u32 *hpd; > > /* Get the status */ > hp_trigger = iir_status & BXT_DE_PORT_HOTPLUG_MASK; > @@ -2271,7 +2276,12 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status) > /* Clear sticky bits in hpd status */ > I915_WRITE(BXT_HOTPLUG_CTL, hp_control); > > - pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, hpd_bxt); > + if (INTEL_REVID(dev) < BXT_REVID_B0) > + hpd = hpd_bxt_a0; > + else > + hpd = hpd_bxt; > + > + pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, hpd); > intel_hpd_irq_handler(dev, pin_mask, long_mask); > } > > @@ -3315,8 +3325,15 @@ static void bxt_hpd_irq_setup(struct drm_device *dev) > /* Now, enable HPD */ > for_each_intel_encoder(dev, intel_encoder) { > if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state > - == HPD_ENABLED) > - hotplug_port |= hpd_bxt[intel_encoder->hpd_pin]; > + == HPD_ENABLED) { > + const u32 *hpd; > + > + if (INTEL_REVID(dev) < BXT_REVID_B0) > + hpd = hpd_bxt_a0; > + else > + hpd = hpd_bxt; > + hotplug_port |= hpd[intel_encoder->hpd_pin]; > + } > } > hpd initialization can be moved out so it is done once instead of being repeated for each encoder. > /* Mask all HPD control bits */ > @@ -3324,11 +3341,14 @@ static void bxt_hpd_irq_setup(struct drm_device *dev) > > /* Enable requested port in hotplug control */ > /* TODO: implement (short) HPD support on port A */ > - WARN_ON_ONCE(hotplug_port & BXT_DE_PORT_HP_DDIA); > - if (hotplug_port & BXT_DE_PORT_HP_DDIB) > - hotplug_ctrl |= BXT_DDIB_HPD_ENABLE; > - if (hotplug_port & BXT_DE_PORT_HP_DDIC) > - hotplug_ctrl |= BXT_DDIC_HPD_ENABLE; > + if (INTEL_REVID(dev) < BXT_REVID_B0 && (hotplug_port & BXT_DE_PORT_HP_DDIA)) > + hotplug_ctrl |= BXT_DDIA_HPD_ENABLE; can you add a comment here stating the swap in hpd pins ? as i am not sure not everyone will first check the commit message for the change here. > + else { > + if (hotplug_port & BXT_DE_PORT_HP_DDIB) > + hotplug_ctrl |= BXT_DDIB_HPD_ENABLE; > + if (hotplug_port & BXT_DE_PORT_HP_DDIC) > + hotplug_ctrl |= BXT_DDIC_HPD_ENABLE; > + } > I915_WRITE(BXT_HOTPLUG_CTL, hotplug_ctrl); > > /* Unmask DDI hotplug in IMR */ > >
On 7/13/2015 12:01 PM, Sivakumar Thulasimani wrote: > > > On 7/13/2015 9:47 AM, Sonika Jindal wrote: >> As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic >> and interrupts to check the external panel connection. >> And remove the redundant comment. >> >> v2: Remove redundant IS_BROXTON check, Add comment about port C not >> connected, and rephrase the commit message to include only what we >> are doing here (Imre) >> >> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> >> --- >> drivers/gpu/drm/i915/i915_irq.c | 38 >> +++++++++++++++++++++++++++++--------- >> 1 file changed, 29 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c >> b/drivers/gpu/drm/i915/i915_irq.c >> index eb52a03..760539c 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -88,7 +88,11 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = { >> [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS >> }; >> -/* BXT hpd list */ >> +/* Port C is not connected on bxt A0/A1 */ >> +static const u32 hpd_bxt_a0[HPD_NUM_PINS] = { >> + [HPD_PORT_B] = BXT_DE_PORT_HP_DDIA >> +}; >> + >> static const u32 hpd_bxt[HPD_NUM_PINS] = { >> [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB, >> [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC >> @@ -2257,6 +2261,7 @@ static void bxt_hpd_handler(struct drm_device >> *dev, uint32_t iir_status) >> struct drm_i915_private *dev_priv = dev->dev_private; >> u32 hp_control, hp_trigger; >> u32 pin_mask, long_mask; >> + const u32 *hpd; >> /* Get the status */ >> hp_trigger = iir_status & BXT_DE_PORT_HOTPLUG_MASK; >> @@ -2271,7 +2276,12 @@ static void bxt_hpd_handler(struct drm_device >> *dev, uint32_t iir_status) >> /* Clear sticky bits in hpd status */ >> I915_WRITE(BXT_HOTPLUG_CTL, hp_control); >> - pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, >> hpd_bxt); >> + if (INTEL_REVID(dev) < BXT_REVID_B0) >> + hpd = hpd_bxt_a0; >> + else >> + hpd = hpd_bxt; >> + >> + pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, >> hpd); >> intel_hpd_irq_handler(dev, pin_mask, long_mask); >> } >> @@ -3315,8 +3325,15 @@ static void bxt_hpd_irq_setup(struct drm_device >> *dev) >> /* Now, enable HPD */ >> for_each_intel_encoder(dev, intel_encoder) { >> if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state >> - == HPD_ENABLED) >> - hotplug_port |= hpd_bxt[intel_encoder->hpd_pin]; >> + == HPD_ENABLED) { >> + const u32 *hpd; >> + >> + if (INTEL_REVID(dev) < BXT_REVID_B0) >> + hpd = hpd_bxt_a0; >> + else >> + hpd = hpd_bxt; >> + hotplug_port |= hpd[intel_encoder->hpd_pin]; >> + } >> } > hpd initialization can be moved out so it is done once instead of being > repeated for each encoder. Sure. > >> /* Mask all HPD control bits */ >> @@ -3324,11 +3341,14 @@ static void bxt_hpd_irq_setup(struct >> drm_device *dev) >> /* Enable requested port in hotplug control */ >> /* TODO: implement (short) HPD support on port A */ >> - WARN_ON_ONCE(hotplug_port & BXT_DE_PORT_HP_DDIA); >> - if (hotplug_port & BXT_DE_PORT_HP_DDIB) >> - hotplug_ctrl |= BXT_DDIB_HPD_ENABLE; >> - if (hotplug_port & BXT_DE_PORT_HP_DDIC) >> - hotplug_ctrl |= BXT_DDIC_HPD_ENABLE; >> + if (INTEL_REVID(dev) < BXT_REVID_B0 && (hotplug_port & >> BXT_DE_PORT_HP_DDIA)) >> + hotplug_ctrl |= BXT_DDIA_HPD_ENABLE; > can you add a comment here stating the swap in hpd pins ? > as i am not sure not everyone will first check the commit message for > the change here. Ok, I'l add >> + else { >> + if (hotplug_port & BXT_DE_PORT_HP_DDIB) >> + hotplug_ctrl |= BXT_DDIB_HPD_ENABLE; >> + if (hotplug_port & BXT_DE_PORT_HP_DDIC) >> + hotplug_ctrl |= BXT_DDIC_HPD_ENABLE; >> + } >> I915_WRITE(BXT_HOTPLUG_CTL, hotplug_ctrl); >> /* Unmask DDI hotplug in IMR */ >> >> > >
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6777
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK 303/303 303/303
SNB +3 309/316 312/316
IVB 343/343 343/343
BYT -1 285/285 284/285
HSW +13 367/381 380/381
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*SNB igt@kms_mmio_vs_cs_flip@setcrtc_vs_cs_flip DMESG_WARN(1) PASS(1)
*SNB igt@kms_mmio_vs_cs_flip@setplane_vs_cs_flip DMESG_WARN(1) PASS(1)
*SNB igt@pm_rpm@cursor DMESG_WARN(1) PASS(1)
*SNB igt@pm_rpm@cursor-dpms DMESG_FAIL(1) FAIL(1)
*BYT igt@gem_tiled_partial_pwrite_pread@reads PASS(1) FAIL(1)
*HSW igt@kms_mmio_vs_cs_flip@setplane_vs_cs_flip DMESG_WARN(1) PASS(1)
*HSW igt@pm_lpsp@non-edp DMESG_WARN(1) PASS(1)
*HSW igt@pm_rpm@debugfs-read DMESG_WARN(1) PASS(1)
*HSW igt@pm_rpm@gem-idle DMESG_WARN(1) PASS(1)
*HSW igt@pm_rpm@gem-mmap-gtt DMESG_WARN(1) PASS(1)
*HSW igt@pm_rpm@gem-pread DMESG_WARN(1) PASS(1)
*HSW igt@pm_rpm@i2c DMESG_WARN(1) PASS(1)
*HSW igt@pm_rpm@modeset-non-lpsp DMESG_WARN(1) PASS(1)
*HSW igt@pm_rpm@modeset-non-lpsp-stress-no-wait DMESG_WARN(1) PASS(1)
*HSW igt@pm_rpm@pci-d3-state DMESG_WARN(1) PASS(1)
*HSW igt@pm_rpm@reg-read-ioctl DMESG_WARN(1) PASS(1)
*HSW igt@pm_rpm@rte DMESG_WARN(1) PASS(1)
*HSW igt@pm_rpm@sysfs-read DMESG_WARN(1) PASS(1)
Note: You need to pay more attention to line start with '*'
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index eb52a03..760539c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -88,7 +88,11 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = { [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS }; -/* BXT hpd list */ +/* Port C is not connected on bxt A0/A1 */ +static const u32 hpd_bxt_a0[HPD_NUM_PINS] = { + [HPD_PORT_B] = BXT_DE_PORT_HP_DDIA +}; + static const u32 hpd_bxt[HPD_NUM_PINS] = { [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB, [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC @@ -2257,6 +2261,7 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status) struct drm_i915_private *dev_priv = dev->dev_private; u32 hp_control, hp_trigger; u32 pin_mask, long_mask; + const u32 *hpd; /* Get the status */ hp_trigger = iir_status & BXT_DE_PORT_HOTPLUG_MASK; @@ -2271,7 +2276,12 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status) /* Clear sticky bits in hpd status */ I915_WRITE(BXT_HOTPLUG_CTL, hp_control); - pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, hpd_bxt); + if (INTEL_REVID(dev) < BXT_REVID_B0) + hpd = hpd_bxt_a0; + else + hpd = hpd_bxt; + + pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, hpd); intel_hpd_irq_handler(dev, pin_mask, long_mask); } @@ -3315,8 +3325,15 @@ static void bxt_hpd_irq_setup(struct drm_device *dev) /* Now, enable HPD */ for_each_intel_encoder(dev, intel_encoder) { if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state - == HPD_ENABLED) - hotplug_port |= hpd_bxt[intel_encoder->hpd_pin]; + == HPD_ENABLED) { + const u32 *hpd; + + if (INTEL_REVID(dev) < BXT_REVID_B0) + hpd = hpd_bxt_a0; + else + hpd = hpd_bxt; + hotplug_port |= hpd[intel_encoder->hpd_pin]; + } } /* Mask all HPD control bits */ @@ -3324,11 +3341,14 @@ static void bxt_hpd_irq_setup(struct drm_device *dev) /* Enable requested port in hotplug control */ /* TODO: implement (short) HPD support on port A */ - WARN_ON_ONCE(hotplug_port & BXT_DE_PORT_HP_DDIA); - if (hotplug_port & BXT_DE_PORT_HP_DDIB) - hotplug_ctrl |= BXT_DDIB_HPD_ENABLE; - if (hotplug_port & BXT_DE_PORT_HP_DDIC) - hotplug_ctrl |= BXT_DDIC_HPD_ENABLE; + if (INTEL_REVID(dev) < BXT_REVID_B0 && (hotplug_port & BXT_DE_PORT_HP_DDIA)) + hotplug_ctrl |= BXT_DDIA_HPD_ENABLE; + else { + if (hotplug_port & BXT_DE_PORT_HP_DDIB) + hotplug_ctrl |= BXT_DDIB_HPD_ENABLE; + if (hotplug_port & BXT_DE_PORT_HP_DDIC) + hotplug_ctrl |= BXT_DDIC_HPD_ENABLE; + } I915_WRITE(BXT_HOTPLUG_CTL, hotplug_ctrl); /* Unmask DDI hotplug in IMR */
As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic and interrupts to check the external panel connection. And remove the redundant comment. v2: Remove redundant IS_BROXTON check, Add comment about port C not connected, and rephrase the commit message to include only what we are doing here (Imre) Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-)