* [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
@ 2015-07-13 4:17 Sonika Jindal
2015-07-13 6:31 ` Sivakumar Thulasimani
2015-07-13 8:26 ` shuang.he
0 siblings, 2 replies; 31+ messages in thread
From: Sonika Jindal @ 2015-07-13 4:17 UTC (permalink / raw)
To: intel-gfx
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];
+ }
}
/* 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 */
--
1.7.10.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-13 4:17 [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping Sonika Jindal
@ 2015-07-13 6:31 ` Sivakumar Thulasimani
2015-07-13 8:11 ` Jindal, Sonika
2015-07-13 8:40 ` Sonika Jindal
2015-07-13 8:26 ` shuang.he
1 sibling, 2 replies; 31+ messages in thread
From: Sivakumar Thulasimani @ 2015-07-13 6:31 UTC (permalink / raw)
To: Sonika Jindal; +Cc: intel-gfx
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 */
>
>
--
regards,
Sivakumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-13 6:31 ` Sivakumar Thulasimani
@ 2015-07-13 8:11 ` Jindal, Sonika
2015-07-13 8:40 ` Sonika Jindal
1 sibling, 0 replies; 31+ messages in thread
From: Jindal, Sonika @ 2015-07-13 8:11 UTC (permalink / raw)
To: Sivakumar Thulasimani; +Cc: intel-gfx
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 */
>>
>>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-13 4:17 [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping Sonika Jindal
2015-07-13 6:31 ` Sivakumar Thulasimani
@ 2015-07-13 8:26 ` shuang.he
1 sibling, 0 replies; 31+ messages in thread
From: shuang.he @ 2015-07-13 8:26 UTC (permalink / raw)
To: shuang.he, julianx.dumez, christophe.sureau, lei.a.liu, intel-gfx,
sonika.jindal
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 '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-13 6:31 ` Sivakumar Thulasimani
2015-07-13 8:11 ` Jindal, Sonika
@ 2015-07-13 8:40 ` Sonika Jindal
2015-07-13 9:40 ` Daniel Vetter
1 sibling, 1 reply; 31+ messages in thread
From: Sonika Jindal @ 2015-07-13 8:40 UTC (permalink / raw)
To: intel-gfx
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)
v3: Add comment about the WA, move 'hpd' initialization outside for
loop (Siva)
Also, remove few redundant comments from hpd handler (me)
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 46 ++++++++++++++++++++++++++-------------
1 file changed, 31 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a897f68..13cabca 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -88,7 +88,14 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = {
[HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
};
-/* BXT hpd list */
+/*
+ * On BXT A0/A1, sw needs to activate DDIA HPD logic and interrupts to check
+ * the external panel connection. 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
@@ -1967,6 +1974,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;
@@ -1981,7 +1989,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);
}
@@ -3021,31 +3034,34 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
struct intel_encoder *intel_encoder;
u32 hotplug_port = 0;
u32 hotplug_ctrl;
+ const u32 *hpd;
- /* Now, enable HPD */
- for_each_intel_encoder(dev, intel_encoder) {
+ if (INTEL_REVID(dev) < BXT_REVID_B0)
+ hpd = hpd_bxt_a0;
+ else
+ hpd = hpd_bxt;
+
+ 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];
- }
+ hotplug_port |= hpd[intel_encoder->hpd_pin];
- /* Mask all HPD control bits */
hotplug_ctrl = I915_READ(BXT_HOTPLUG_CTL) & ~BXT_HOTPLUG_CTL_MASK;
- /* 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 */
hotplug_ctrl = I915_READ(GEN8_DE_PORT_IMR) & ~hotplug_port;
I915_WRITE(GEN8_DE_PORT_IMR, hotplug_ctrl);
- /* Enable DDI hotplug in IER */
hotplug_ctrl = I915_READ(GEN8_DE_PORT_IER) | hotplug_port;
I915_WRITE(GEN8_DE_PORT_IER, hotplug_ctrl);
POSTING_READ(GEN8_DE_PORT_IER);
--
1.7.10.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-13 8:40 ` Sonika Jindal
@ 2015-07-13 9:40 ` Daniel Vetter
2015-07-13 10:57 ` Sivakumar Thulasimani
0 siblings, 1 reply; 31+ messages in thread
From: Daniel Vetter @ 2015-07-13 9:40 UTC (permalink / raw)
To: Sonika Jindal; +Cc: intel-gfx
On Mon, Jul 13, 2015 at 02:10:09PM +0530, 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)
> v3: Add comment about the WA, move 'hpd' initialization outside for
> loop (Siva)
> Also, remove few redundant comments from hpd handler (me)
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
Can't we do this in two steps:
- Wire up port A hpd in a generic way.
- Add wa for bxt to use port A hpd in the various encoder setup functions
where we assign intel_encoder->hpd_pin.
Currently that switchover is spread all through low-level functions, which
makes this a bit confusion. Imo low-level code shouldn't treat hpd A as
anything but hpd A since that's just confusing. And we already have the
infrastructure for encoders to ask for any hpd pin they want really.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_irq.c | 46 ++++++++++++++++++++++++++-------------
> 1 file changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a897f68..13cabca 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -88,7 +88,14 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = {
> [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
> };
>
> -/* BXT hpd list */
> +/*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and interrupts to check
> + * the external panel connection. 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
> @@ -1967,6 +1974,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;
> @@ -1981,7 +1989,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);
> }
>
> @@ -3021,31 +3034,34 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
> struct intel_encoder *intel_encoder;
> u32 hotplug_port = 0;
> u32 hotplug_ctrl;
> + const u32 *hpd;
>
> - /* Now, enable HPD */
> - for_each_intel_encoder(dev, intel_encoder) {
> + if (INTEL_REVID(dev) < BXT_REVID_B0)
> + hpd = hpd_bxt_a0;
> + else
> + hpd = hpd_bxt;
> +
> + 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];
> - }
> + hotplug_port |= hpd[intel_encoder->hpd_pin];
>
> - /* Mask all HPD control bits */
> hotplug_ctrl = I915_READ(BXT_HOTPLUG_CTL) & ~BXT_HOTPLUG_CTL_MASK;
>
> - /* 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 */
> hotplug_ctrl = I915_READ(GEN8_DE_PORT_IMR) & ~hotplug_port;
> I915_WRITE(GEN8_DE_PORT_IMR, hotplug_ctrl);
>
> - /* Enable DDI hotplug in IER */
> hotplug_ctrl = I915_READ(GEN8_DE_PORT_IER) | hotplug_port;
> I915_WRITE(GEN8_DE_PORT_IER, hotplug_ctrl);
> POSTING_READ(GEN8_DE_PORT_IER);
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-13 9:40 ` Daniel Vetter
@ 2015-07-13 10:57 ` Sivakumar Thulasimani
2015-07-14 5:48 ` [PATCH 1/2] drm/i915/bxt: Add HPD support for DDIA Sonika Jindal
0 siblings, 1 reply; 31+ messages in thread
From: Sivakumar Thulasimani @ 2015-07-13 10:57 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On 7/13/2015 3:10 PM, Daniel Vetter wrote:
> On Mon, Jul 13, 2015 at 02:10:09PM +0530, 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)
>> v3: Add comment about the WA, move 'hpd' initialization outside for
>> loop (Siva)
>> Also, remove few redundant comments from hpd handler (me)
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> Can't we do this in two steps:
> - Wire up port A hpd in a generic way.
> - Add wa for bxt to use port A hpd in the various encoder setup functions
> where we assign intel_encoder->hpd_pin.
>
> Currently that switchover is spread all through low-level functions, which
> makes this a bit confusion. Imo low-level code shouldn't treat hpd A as
> anything but hpd A since that's just confusing. And we already have the
> infrastructure for encoders to ask for any hpd pin they want really.
> -Daniel
>
>> ---
>> drivers/gpu/drm/i915/i915_irq.c | 46 ++++++++++++++++++++++++++-------------
>> 1 file changed, 31 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index a897f68..13cabca 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -88,7 +88,14 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = {
>> [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
>> };
>>
>> -/* BXT hpd list */
>> +/*
>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and interrupts to check
>> + * the external panel connection. 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
>> @@ -1967,6 +1974,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;
>> @@ -1981,7 +1989,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);
>> }
>>
>> @@ -3021,31 +3034,34 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
>> struct intel_encoder *intel_encoder;
>> u32 hotplug_port = 0;
>> u32 hotplug_ctrl;
>> + const u32 *hpd;
>>
>> - /* Now, enable HPD */
>> - for_each_intel_encoder(dev, intel_encoder) {
>> + if (INTEL_REVID(dev) < BXT_REVID_B0)
>> + hpd = hpd_bxt_a0;
>> + else
>> + hpd = hpd_bxt;
>> +
>> + 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];
>> - }
>> + hotplug_port |= hpd[intel_encoder->hpd_pin];
>>
>> - /* Mask all HPD control bits */
>> hotplug_ctrl = I915_READ(BXT_HOTPLUG_CTL) & ~BXT_HOTPLUG_CTL_MASK;
>>
>> - /* 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 */
>> hotplug_ctrl = I915_READ(GEN8_DE_PORT_IMR) & ~hotplug_port;
>> I915_WRITE(GEN8_DE_PORT_IMR, hotplug_ctrl);
>>
>> - /* Enable DDI hotplug in IER */
>> hotplug_ctrl = I915_READ(GEN8_DE_PORT_IER) | hotplug_port;
>> I915_WRITE(GEN8_DE_PORT_IER, hotplug_ctrl);
>> POSTING_READ(GEN8_DE_PORT_IER);
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
Agreed, that is better.
Sonika,
lets split this into two patches,
one : will add HPD support for DDIA like it is already done for DDIB & C.
second, will just update hpd_pin for A0/A1 systems as required and avoid
explicit revision checks in irq.c file.
--
regards,
Sivakumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 1/2] drm/i915/bxt: Add HPD support for DDIA
2015-07-13 10:57 ` Sivakumar Thulasimani
@ 2015-07-14 5:48 ` Sonika Jindal
2015-07-14 5:48 ` [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping Sonika Jindal
0 siblings, 1 reply; 31+ messages in thread
From: Sonika Jindal @ 2015-07-14 5:48 UTC (permalink / raw)
To: intel-gfx
Also remove redundant comments.
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a897f68..63137dd 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -90,6 +90,7 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = {
/* BXT hpd list */
static const u32 hpd_bxt[HPD_NUM_PINS] = {
+ [HPD_PORT_A] = BXT_DE_PORT_HP_DDIA,
[HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
};
@@ -3022,30 +3023,25 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
u32 hotplug_port = 0;
u32 hotplug_ctrl;
- /* 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];
}
- /* Mask all HPD control bits */
hotplug_ctrl = I915_READ(BXT_HOTPLUG_CTL) & ~BXT_HOTPLUG_CTL_MASK;
- /* 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_DDIA)
+ hotplug_ctrl |= BXT_DDIA_HPD_ENABLE;
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 */
hotplug_ctrl = I915_READ(GEN8_DE_PORT_IMR) & ~hotplug_port;
I915_WRITE(GEN8_DE_PORT_IMR, hotplug_ctrl);
- /* Enable DDI hotplug in IER */
hotplug_ctrl = I915_READ(GEN8_DE_PORT_IER) | hotplug_port;
I915_WRITE(GEN8_DE_PORT_IER, hotplug_ctrl);
POSTING_READ(GEN8_DE_PORT_IER);
--
1.7.10.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-14 5:48 ` [PATCH 1/2] drm/i915/bxt: Add HPD support for DDIA Sonika Jindal
@ 2015-07-14 5:48 ` Sonika Jindal
2015-07-14 8:10 ` Daniel Vetter
2015-07-14 14:22 ` Imre Deak
0 siblings, 2 replies; 31+ messages in thread
From: Sonika Jindal @ 2015-07-14 5:48 UTC (permalink / raw)
To: intel-gfx
As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic
and interrupts to check the external panel connection and DDIC HPD
logic for edp panel.
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++--
drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7b54b9d..c32f3d3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5869,10 +5869,24 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
/* Set up the hotplug pin. */
switch (port) {
case PORT_A:
- intel_encoder->hpd_pin = HPD_PORT_A;
+ /*
+ * On BXT A0/A1, sw needs to activate DDIC HPD logic and
+ * interrupts to check the eDP panel connection.
+ */
+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+ intel_encoder->hpd_pin = HPD_PORT_C;
+ else
+ intel_encoder->hpd_pin = HPD_PORT_A;
break;
case PORT_B:
- intel_encoder->hpd_pin = HPD_PORT_B;
+ /*
+ * On BXT A0/A1, sw needs to activate DDIA HPD logic and
+ * interrupts to check the external panel connection.
+ */
+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+ intel_encoder->hpd_pin = HPD_PORT_A;
+ else
+ intel_encoder->hpd_pin = HPD_PORT_B;
break;
case PORT_C:
intel_encoder->hpd_pin = HPD_PORT_C;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 44e7160..121e626 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2011,7 +2011,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
else
intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
- intel_encoder->hpd_pin = HPD_PORT_B;
+ /*
+ * On BXT A0/A1, sw needs to activate DDIA HPD logic and
+ * interrupts to check the external panel connection.
+ */
+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+ intel_encoder->hpd_pin = HPD_PORT_A;
+ else
+ intel_encoder->hpd_pin = HPD_PORT_B;
break;
case PORT_C:
if (IS_BROXTON(dev_priv))
--
1.7.10.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-14 5:48 ` [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping Sonika Jindal
@ 2015-07-14 8:10 ` Daniel Vetter
2015-07-14 14:22 ` Imre Deak
1 sibling, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2015-07-14 8:10 UTC (permalink / raw)
To: Sonika Jindal; +Cc: intel-gfx
On Tue, Jul 14, 2015 at 11:18:50AM +0530, 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 DDIC HPD
> logic for edp panel.
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
Yeah I think this is much clearer. Will pull in as soon as there's an r-b
on these two.
Thanks, Daniel
> ---
> drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++--
> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
> 2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7b54b9d..c32f3d3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5869,10 +5869,24 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> /* Set up the hotplug pin. */
> switch (port) {
> case PORT_A:
> - intel_encoder->hpd_pin = HPD_PORT_A;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIC HPD logic and
> + * interrupts to check the eDP panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_C;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_A;
> break;
> case PORT_B:
> - intel_encoder->hpd_pin = HPD_PORT_B;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_B;
> break;
> case PORT_C:
> intel_encoder->hpd_pin = HPD_PORT_C;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 44e7160..121e626 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2011,7 +2011,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> else
> intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> - intel_encoder->hpd_pin = HPD_PORT_B;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_B;
> break;
> case PORT_C:
> if (IS_BROXTON(dev_priv))
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-14 5:48 ` [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping Sonika Jindal
2015-07-14 8:10 ` Daniel Vetter
@ 2015-07-14 14:22 ` Imre Deak
2015-07-15 6:35 ` Jindal, Sonika
1 sibling, 1 reply; 31+ messages in thread
From: Imre Deak @ 2015-07-14 14:22 UTC (permalink / raw)
To: Sonika Jindal; +Cc: intel-gfx
On ti, 2015-07-14 at 11:18 +0530, 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 DDIC HPD
> logic for edp panel.
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++--
> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
> 2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7b54b9d..c32f3d3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5869,10 +5869,24 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> /* Set up the hotplug pin. */
> switch (port) {
> case PORT_A:
> - intel_encoder->hpd_pin = HPD_PORT_A;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIC HPD logic and
> + * interrupts to check the eDP panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_C;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_A;
> break;
> case PORT_B:
> - intel_encoder->hpd_pin = HPD_PORT_B;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_B;
> break;
> case PORT_C:
> intel_encoder->hpd_pin = HPD_PORT_C;
This won't work for a couple of reasons: atm i915_digport_work_func()
assumes a fixed pin->port mapping, for example it'll call the HPD
handler for the port A encoder for a short/long pulse event triggered
via the HPD_PORT_A pin, whereas after the above patch you want to select
port B's encoder on BXT A0/1. This could be fixed by setting up
hotplug.irq_port in intel_ddi_init() according to the above change, or
not using irq_port at all, but instead looking up the encoder the same
way i915_hotplug_work_func() does using intel_encoder->hpd_pin.
The HPD_PORT_A HPD handling is missing in general, see
for_each_hpd_pin() and intel_hpd_irq_handler()/is_dig_port.
So if going with the above way, these issues need to be addressed first.
--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-14 14:22 ` Imre Deak
@ 2015-07-15 6:35 ` Jindal, Sonika
2015-07-15 8:04 ` Jindal, Sonika
0 siblings, 1 reply; 31+ messages in thread
From: Jindal, Sonika @ 2015-07-15 6:35 UTC (permalink / raw)
To: imre.deak; +Cc: intel-gfx
On 7/14/2015 7:52 PM, Imre Deak wrote:
> On ti, 2015-07-14 at 11:18 +0530, 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 DDIC HPD
>> logic for edp panel.
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++--
>> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
>> 2 files changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 7b54b9d..c32f3d3 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -5869,10 +5869,24 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>> /* Set up the hotplug pin. */
>> switch (port) {
>> case PORT_A:
>> - intel_encoder->hpd_pin = HPD_PORT_A;
>> + /*
>> + * On BXT A0/A1, sw needs to activate DDIC HPD logic and
>> + * interrupts to check the eDP panel connection.
>> + */
>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>> + intel_encoder->hpd_pin = HPD_PORT_C;
>> + else
>> + intel_encoder->hpd_pin = HPD_PORT_A;
>> break;
>> case PORT_B:
>> - intel_encoder->hpd_pin = HPD_PORT_B;
>> + /*
>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>> + * interrupts to check the external panel connection.
>> + */
>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>> + intel_encoder->hpd_pin = HPD_PORT_A;
>> + else
>> + intel_encoder->hpd_pin = HPD_PORT_B;
>> break;
>> case PORT_C:
>> intel_encoder->hpd_pin = HPD_PORT_C;
>
> This won't work for a couple of reasons: atm i915_digport_work_func()
> assumes a fixed pin->port mapping, for example it'll call the HPD
> handler for the port A encoder for a short/long pulse event triggered
> via the HPD_PORT_A pin, whereas after the above patch you want to select
> port B's encoder on BXT A0/1. This could be fixed by setting up
> hotplug.irq_port in intel_ddi_init() according to the above change, or
> not using irq_port at all, but instead looking up the encoder the same
> way i915_hotplug_work_func() does using intel_encoder->hpd_pin.
>
Hmm, i didnt realize that.
Moving this check to intel_ddi_init, will again make the checks spread
all over which we wanted to avoid since hpd_pin was in place.
But looks like hpd_pin is only for i915_hotplug_work_func.
I feel we can move back to the older approach itself
What do you suggest?
Daniel, any comments?
> The HPD_PORT_A HPD handling is missing in general, see
> for_each_hpd_pin() and intel_hpd_irq_handler()/is_dig_port.
>
> So if going with the above way, these issues need to be addressed first.
>
> --Imre
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-15 6:35 ` Jindal, Sonika
@ 2015-07-15 8:04 ` Jindal, Sonika
2015-07-15 9:07 ` Daniel Vetter
0 siblings, 1 reply; 31+ messages in thread
From: Jindal, Sonika @ 2015-07-15 8:04 UTC (permalink / raw)
To: Deak, Imre; +Cc: intel-gfx@lists.freedesktop.org
On 7/15/2015 12:05 PM, Jindal, Sonika wrote:
>
>
> On 7/14/2015 7:52 PM, Imre Deak wrote:
>> On ti, 2015-07-14 at 11:18 +0530, 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 DDIC HPD
>>> logic for edp panel.
>>>
>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++--
>>> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
>>> 2 files changed, 24 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 7b54b9d..c32f3d3 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -5869,10 +5869,24 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>>> /* Set up the hotplug pin. */
>>> switch (port) {
>>> case PORT_A:
>>> - intel_encoder->hpd_pin = HPD_PORT_A;
>>> + /*
>>> + * On BXT A0/A1, sw needs to activate DDIC HPD logic and
>>> + * interrupts to check the eDP panel connection.
>>> + */
>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>>> + intel_encoder->hpd_pin = HPD_PORT_C;
>>> + else
>>> + intel_encoder->hpd_pin = HPD_PORT_A;
>>> break;
>>> case PORT_B:
>>> - intel_encoder->hpd_pin = HPD_PORT_B;
>>> + /*
>>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>>> + * interrupts to check the external panel connection.
>>> + */
>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>>> + intel_encoder->hpd_pin = HPD_PORT_A;
>>> + else
>>> + intel_encoder->hpd_pin = HPD_PORT_B;
>>> break;
>>> case PORT_C:
>>> intel_encoder->hpd_pin = HPD_PORT_C;
>>
>> This won't work for a couple of reasons: atm i915_digport_work_func()
>> assumes a fixed pin->port mapping, for example it'll call the HPD
>> handler for the port A encoder for a short/long pulse event triggered
>> via the HPD_PORT_A pin, whereas after the above patch you want to select
>> port B's encoder on BXT A0/1. This could be fixed by setting up
>> hotplug.irq_port in intel_ddi_init() according to the above change, or
>> not using irq_port at all, but instead looking up the encoder the same
>> way i915_hotplug_work_func() does using intel_encoder->hpd_pin.
>>
> Hmm, i didnt realize that.
> Moving this check to intel_ddi_init, will again make the checks spread
> all over which we wanted to avoid since hpd_pin was in place.
> But looks like hpd_pin is only for i915_hotplug_work_func.
> I feel we can move back to the older approach itself
> What do you suggest?
>
But then we can remove these checks from here, and have it only in
intel_ddi_init, right? So it should look fine.
For HPD_PORT_A, I think we can skip that part as of now.
Please suggest..
> Daniel, any comments?
>
>> The HPD_PORT_A HPD handling is missing in general, see
>> for_each_hpd_pin() and intel_hpd_irq_handler()/is_dig_port.
>>
>
>> So if going with the above way, these issues need to be addressed first.
>>
>> --Imre
>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-15 8:04 ` Jindal, Sonika
@ 2015-07-15 9:07 ` Daniel Vetter
2015-07-17 4:29 ` Jindal, Sonika
0 siblings, 1 reply; 31+ messages in thread
From: Daniel Vetter @ 2015-07-15 9:07 UTC (permalink / raw)
To: Jindal, Sonika; +Cc: intel-gfx@lists.freedesktop.org
On Wed, Jul 15, 2015 at 01:34:29PM +0530, Jindal, Sonika wrote:
>
>
> On 7/15/2015 12:05 PM, Jindal, Sonika wrote:
> >
> >
> >On 7/14/2015 7:52 PM, Imre Deak wrote:
> >>On ti, 2015-07-14 at 11:18 +0530, 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 DDIC HPD
> >>>logic for edp panel.
> >>>
> >>>Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> >>>---
> >>> drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++--
> >>> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
> >>> 2 files changed, 24 insertions(+), 3 deletions(-)
> >>>
> >>>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >>>index 7b54b9d..c32f3d3 100644
> >>>--- a/drivers/gpu/drm/i915/intel_dp.c
> >>>+++ b/drivers/gpu/drm/i915/intel_dp.c
> >>>@@ -5869,10 +5869,24 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> >>> /* Set up the hotplug pin. */
> >>> switch (port) {
> >>> case PORT_A:
> >>>- intel_encoder->hpd_pin = HPD_PORT_A;
> >>>+ /*
> >>>+ * On BXT A0/A1, sw needs to activate DDIC HPD logic and
> >>>+ * interrupts to check the eDP panel connection.
> >>>+ */
> >>>+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> >>>+ intel_encoder->hpd_pin = HPD_PORT_C;
> >>>+ else
> >>>+ intel_encoder->hpd_pin = HPD_PORT_A;
> >>> break;
> >>> case PORT_B:
> >>>- intel_encoder->hpd_pin = HPD_PORT_B;
> >>>+ /*
> >>>+ * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> >>>+ * interrupts to check the external panel connection.
> >>>+ */
> >>>+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> >>>+ intel_encoder->hpd_pin = HPD_PORT_A;
> >>>+ else
> >>>+ intel_encoder->hpd_pin = HPD_PORT_B;
> >>> break;
> >>> case PORT_C:
> >>> intel_encoder->hpd_pin = HPD_PORT_C;
> >>
> >>This won't work for a couple of reasons: atm i915_digport_work_func()
> >>assumes a fixed pin->port mapping, for example it'll call the HPD
> >>handler for the port A encoder for a short/long pulse event triggered
> >>via the HPD_PORT_A pin, whereas after the above patch you want to select
> >>port B's encoder on BXT A0/1. This could be fixed by setting up
> >>hotplug.irq_port in intel_ddi_init() according to the above change, or
> >>not using irq_port at all, but instead looking up the encoder the same
> >>way i915_hotplug_work_func() does using intel_encoder->hpd_pin.
Yeah that's kinda a bug in digport_work_func, but that part is also a
mess. Simplest would be to rename hotplug.irq_port to irq_pin and change
the assignment for that too.
> >Hmm, i didnt realize that.
> >Moving this check to intel_ddi_init, will again make the checks spread
> >all over which we wanted to avoid since hpd_pin was in place.
> >But looks like hpd_pin is only for i915_hotplug_work_func.
> >I feel we can move back to the older approach itself
> >What do you suggest?
> >
> But then we can remove these checks from here, and have it only in
> intel_ddi_init, right? So it should look fine.
>
> For HPD_PORT_A, I think we can skip that part as of now.
>
> Please suggest..
I still think that fake-handling hpd A in the low-level irq handler is
massively confusing. And we need to change all the same parts anyway (i.e.
you'd have to change the digport_work_func too with the old approach).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-15 9:07 ` Daniel Vetter
@ 2015-07-17 4:29 ` Jindal, Sonika
2015-07-17 8:17 ` [PATCH] " Sonika Jindal
0 siblings, 1 reply; 31+ messages in thread
From: Jindal, Sonika @ 2015-07-17 4:29 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx@lists.freedesktop.org
On 7/15/2015 2:37 PM, Daniel Vetter wrote:
> On Wed, Jul 15, 2015 at 01:34:29PM +0530, Jindal, Sonika wrote:
>>
>>
>> On 7/15/2015 12:05 PM, Jindal, Sonika wrote:
>>>
>>>
>>> On 7/14/2015 7:52 PM, Imre Deak wrote:
>>>> On ti, 2015-07-14 at 11:18 +0530, 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 DDIC HPD
>>>>> logic for edp panel.
>>>>>
>>>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>>>> ---
>>>>> drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++--
>>>>> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
>>>>> 2 files changed, 24 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>>>> index 7b54b9d..c32f3d3 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>>> @@ -5869,10 +5869,24 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>>>>> /* Set up the hotplug pin. */
>>>>> switch (port) {
>>>>> case PORT_A:
>>>>> - intel_encoder->hpd_pin = HPD_PORT_A;
>>>>> + /*
>>>>> + * On BXT A0/A1, sw needs to activate DDIC HPD logic and
>>>>> + * interrupts to check the eDP panel connection.
>>>>> + */
>>>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>>>>> + intel_encoder->hpd_pin = HPD_PORT_C;
>>>>> + else
>>>>> + intel_encoder->hpd_pin = HPD_PORT_A;
>>>>> break;
>>>>> case PORT_B:
>>>>> - intel_encoder->hpd_pin = HPD_PORT_B;
>>>>> + /*
>>>>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>>>>> + * interrupts to check the external panel connection.
>>>>> + */
>>>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>>>>> + intel_encoder->hpd_pin = HPD_PORT_A;
>>>>> + else
>>>>> + intel_encoder->hpd_pin = HPD_PORT_B;
>>>>> break;
>>>>> case PORT_C:
>>>>> intel_encoder->hpd_pin = HPD_PORT_C;
>>>>
>>>> This won't work for a couple of reasons: atm i915_digport_work_func()
>>>> assumes a fixed pin->port mapping, for example it'll call the HPD
>>>> handler for the port A encoder for a short/long pulse event triggered
>>>> via the HPD_PORT_A pin, whereas after the above patch you want to select
>>>> port B's encoder on BXT A0/1. This could be fixed by setting up
>>>> hotplug.irq_port in intel_ddi_init() according to the above change, or
>>>> not using irq_port at all, but instead looking up the encoder the same
>>>> way i915_hotplug_work_func() does using intel_encoder->hpd_pin.
>
> Yeah that's kinda a bug in digport_work_func, but that part is also a
> mess. Simplest would be to rename hotplug.irq_port to irq_pin and change
> the assignment for that too.
>
>>> Hmm, i didnt realize that.
>>> Moving this check to intel_ddi_init, will again make the checks spread
>>> all over which we wanted to avoid since hpd_pin was in place.
>>> But looks like hpd_pin is only for i915_hotplug_work_func.
>>> I feel we can move back to the older approach itself
>>> What do you suggest?
>>>
>> But then we can remove these checks from here, and have it only in
>> intel_ddi_init, right? So it should look fine.
>>
>> For HPD_PORT_A, I think we can skip that part as of now.
>>
>> Please suggest..
>
> I still think that fake-handling hpd A in the low-level irq handler is
> massively confusing. And we need to change all the same parts anyway (i.e.
> you'd have to change the digport_work_func too with the old approach).
> -Daniel
So, for now, I will just correct it in intel_ddi_init and leave the
handling of HPD for port A untouched. I will only change the irq_port
for external DP (port B). And the other part with hpd_pin to handle hdmi
hotplug will remain as is.
Please let me know if you see any issue in this.
Regards,
Sonika
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-17 4:29 ` Jindal, Sonika
@ 2015-07-17 8:17 ` Sonika Jindal
2015-07-17 23:47 ` Imre Deak
2015-07-20 5:07 ` shuang.he
0 siblings, 2 replies; 31+ messages in thread
From: Sonika Jindal @ 2015-07-17 8:17 UTC (permalink / raw)
To: intel-gfx
As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic
and interrupts to check the external panel connection and DDIC HPD
logic for edp panel.
v2: For DP, irq_port is used to determine the encoder instead of
hpd_pin and removing the edp HPD logic because port A HPD is not
present(Imre)
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
drivers/gpu/drm/i915/intel_ddi.c | 10 +++++++++-
drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e2c6f73..777e3a3 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3225,7 +3225,15 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
goto err;
intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
- dev_priv->hotplug.irq_port[port] = intel_dig_port;
+ /*
+ * On BXT A0/A1, sw needs to activate DDIA HPD logic and
+ * interrupts to check the external panel connection.
+ */
+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
+ && port == PORT_B)
+ dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
+ else
+ dev_priv->hotplug.irq_port[port] = intel_dig_port;
}
/* In theory we don't need the encoder->type check, but leave it just in
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 70bad5b..94fa716 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
else
intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
- intel_encoder->hpd_pin = HPD_PORT_B;
+ /*
+ * On BXT A0/A1, sw needs to activate DDIA HPD logic and
+ * interrupts to check the external panel connection.
+ */
+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+ intel_encoder->hpd_pin = HPD_PORT_A;
+ else
+ intel_encoder->hpd_pin = HPD_PORT_B;
break;
case PORT_C:
if (IS_BROXTON(dev_priv))
--
1.7.10.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-17 8:17 ` [PATCH] " Sonika Jindal
@ 2015-07-17 23:47 ` Imre Deak
2015-07-20 6:06 ` Jindal, Sonika
2015-07-20 5:07 ` shuang.he
1 sibling, 1 reply; 31+ messages in thread
From: Imre Deak @ 2015-07-17 23:47 UTC (permalink / raw)
To: Sonika Jindal; +Cc: intel-gfx
On Fri, 2015-07-17 at 13:47 +0530, 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 DDIC HPD
> logic for edp panel.
>
> v2: For DP, irq_port is used to determine the encoder instead of
> hpd_pin and removing the edp HPD logic because port A HPD is not
> present(Imre)
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 10 +++++++++-
> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e2c6f73..777e3a3 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3225,7 +3225,15 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> goto err;
>
> intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> - dev_priv->hotplug.irq_port[port] = intel_dig_port;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> + && port == PORT_B)
> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
This happens to work but is confusing. irq_port[PORT_A] will be set here
already and the above will simply overwrite it without explanation. I
would also handle the port == PORT_A case and not set irq_port for it.
The same swapping for hpd_pin is missing from intel_dp_init_connector().
> + else
> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
> }
>
> /* In theory we don't need the encoder->type check, but leave it just in
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 70bad5b..94fa716 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> else
> intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> - intel_encoder->hpd_pin = HPD_PORT_B;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_B;
> break;
> case PORT_C:
> if (IS_BROXTON(dev_priv))
As I earlier pointed out with the above approach, you need to add
support for HPD events on the HPD_PORT_A pin. If you look at the
for_each_hpd_pin() macro and intel_hpd_irq_handler()/is_dig_port you'll
notice that any interrupt event on the HPD_PORT_A pin will be ignored
now.
--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-17 8:17 ` [PATCH] " Sonika Jindal
2015-07-17 23:47 ` Imre Deak
@ 2015-07-20 5:07 ` shuang.he
1 sibling, 0 replies; 31+ messages in thread
From: shuang.he @ 2015-07-20 5:07 UTC (permalink / raw)
To: shuang.he, julianx.dumez, christophe.sureau, lei.a.liu, intel-gfx,
sonika.jindal
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6822
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK 302/302 302/302
SNB 315/315 315/315
IVB 342/342 342/342
BYT -2 285/285 283/285
HSW 378/378 378/378
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*BYT igt@gem_partial_pwrite_pread@reads-display 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 '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-17 23:47 ` Imre Deak
@ 2015-07-20 6:06 ` Jindal, Sonika
2015-07-20 21:43 ` Imre Deak
0 siblings, 1 reply; 31+ messages in thread
From: Jindal, Sonika @ 2015-07-20 6:06 UTC (permalink / raw)
To: imre.deak; +Cc: intel-gfx
On 7/18/2015 5:17 AM, Imre Deak wrote:
> On Fri, 2015-07-17 at 13:47 +0530, 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 DDIC HPD
>> logic for edp panel.
>>
>> v2: For DP, irq_port is used to determine the encoder instead of
>> hpd_pin and removing the edp HPD logic because port A HPD is not
>> present(Imre)
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_ddi.c | 10 +++++++++-
>> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
>> 2 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index e2c6f73..777e3a3 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -3225,7 +3225,15 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>> goto err;
>>
>> intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
>> - dev_priv->hotplug.irq_port[port] = intel_dig_port;
>> + /*
>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>> + * interrupts to check the external panel connection.
>> + */
>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
>> + && port == PORT_B)
>> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
>
> This happens to work but is confusing. irq_port[PORT_A] will be set here
> already and the above will simply overwrite it without explanation. I
> would also handle the port == PORT_A case and not set irq_port for it.
>
> The same swapping for hpd_pin is missing from intel_dp_init_connector().
>
>> + else
>> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
>> }
>>
>> /* In theory we don't need the encoder->type check, but leave it just in
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index 70bad5b..94fa716 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>> intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
>> else
>> intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
>> - intel_encoder->hpd_pin = HPD_PORT_B;
>> + /*
>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>> + * interrupts to check the external panel connection.
>> + */
>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>> + intel_encoder->hpd_pin = HPD_PORT_A;
>> + else
>> + intel_encoder->hpd_pin = HPD_PORT_B;
>> break;
>> case PORT_C:
>> if (IS_BROXTON(dev_priv))
>
> As I earlier pointed out with the above approach, you need to add
> support for HPD events on the HPD_PORT_A pin. If you look at the
> for_each_hpd_pin() macro and intel_hpd_irq_handler()/is_dig_port you'll
> notice that any interrupt event on the HPD_PORT_A pin will be ignored
> now.
Hmm :(. For now, we can fix for_each_hpd_pin and add a check in
intel_hpd_pin_to_port to return PORT_B for pin 0 if A0/A1. Then we can
skip the check in intel_ddi_init. But this again will look very confusing.
So two options:
1. We move back to the older approach where we just use another hpd
ports array in i915_irq.c
2. Go with current approach.
I feel option 1 is more clean and less prone to further issues.
What do you suggest?
Regards,
Sonika
>
> --Imre
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-20 6:06 ` Jindal, Sonika
@ 2015-07-20 21:43 ` Imre Deak
2015-07-22 10:07 ` Sonika Jindal
0 siblings, 1 reply; 31+ messages in thread
From: Imre Deak @ 2015-07-20 21:43 UTC (permalink / raw)
To: Jindal, Sonika; +Cc: intel-gfx
On Mon, 2015-07-20 at 11:36 +0530, Jindal, Sonika wrote:
>
> On 7/18/2015 5:17 AM, Imre Deak wrote:
> > On Fri, 2015-07-17 at 13:47 +0530, 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 DDIC HPD
> >> logic for edp panel.
> >>
> >> v2: For DP, irq_port is used to determine the encoder instead of
> >> hpd_pin and removing the edp HPD logic because port A HPD is not
> >> present(Imre)
> >>
> >> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/intel_ddi.c | 10 +++++++++-
> >> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
> >> 2 files changed, 17 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >> index e2c6f73..777e3a3 100644
> >> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> @@ -3225,7 +3225,15 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> >> goto err;
> >>
> >> intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> >> - dev_priv->hotplug.irq_port[port] = intel_dig_port;
> >> + /*
> >> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> >> + * interrupts to check the external panel connection.
> >> + */
> >> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> >> + && port == PORT_B)
> >> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> >
> > This happens to work but is confusing. irq_port[PORT_A] will be set here
> > already and the above will simply overwrite it without explanation. I
> > would also handle the port == PORT_A case and not set irq_port for it.
> >
> > The same swapping for hpd_pin is missing from intel_dp_init_connector().
> >
> >> + else
> >> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
> >> }
> >>
> >> /* In theory we don't need the encoder->type check, but leave it just in
> >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> >> index 70bad5b..94fa716 100644
> >> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >> @@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >> intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> >> else
> >> intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> >> - intel_encoder->hpd_pin = HPD_PORT_B;
> >> + /*
> >> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> >> + * interrupts to check the external panel connection.
> >> + */
> >> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> >> + intel_encoder->hpd_pin = HPD_PORT_A;
> >> + else
> >> + intel_encoder->hpd_pin = HPD_PORT_B;
> >> break;
> >> case PORT_C:
> >> if (IS_BROXTON(dev_priv))
> >
> > As I earlier pointed out with the above approach, you need to add
> > support for HPD events on the HPD_PORT_A pin. If you look at the
> > for_each_hpd_pin() macro and intel_hpd_irq_handler()/is_dig_port you'll
> > notice that any interrupt event on the HPD_PORT_A pin will be ignored
> > now.
> Hmm :(. For now, we can fix for_each_hpd_pin and add a check in
> intel_hpd_pin_to_port to return PORT_B for pin 0 if A0/A1. Then we can
> skip the check in intel_ddi_init. But this again will look very confusing.
>
> So two options:
> 1. We move back to the older approach where we just use another hpd
> ports array in i915_irq.c
> 2. Go with current approach.
>
> I feel option 1 is more clean and less prone to further issues.
> What do you suggest?
I also think that adding support for HPD on the port A pin is the better
way and this would be needed in any case for eDP short pulse detection
support in the future regardless of this workaround. I'll send a
patchset that does this, I think on top of that we could apply the
current version of your 2 workaround patches (with the comments
addressed).
--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-20 21:43 ` Imre Deak
@ 2015-07-22 10:07 ` Sonika Jindal
2015-07-22 10:33 ` Sivakumar Thulasimani
2015-07-27 5:32 ` Sonika Jindal
0 siblings, 2 replies; 31+ messages in thread
From: Sonika Jindal @ 2015-07-22 10:07 UTC (permalink / raw)
To: intel-gfx
As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic
and interrupts to check the external panel connection and DDIC HPD
logic for edp panel.
v2: For DP, irq_port is used to determine the encoder instead of
hpd_pin and removing the edp HPD logic because port A HPD is not
present(Imre)
v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
Added hpd_pin swapping for intel_dp_init_connector, setting encoder
for PORT_A as per the WA in irq_port (Imre)
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
drivers/gpu/drm/i915/intel_ddi.c | 12 +++++++++++-
drivers/gpu/drm/i915/intel_dp.c | 4 ++++
drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e2c6f73..d5745e2 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3225,7 +3225,17 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
goto err;
intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
- dev_priv->hotplug.irq_port[port] = intel_dig_port;
+ /*
+ * On BXT A0/A1, sw needs to activate DDIA HPD logic and
+ * interrupts to check the external panel connection.
+ */
+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)) {
+ if (port == PORT_B)
+ dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
+ else if (port == PORT_A)
+ dev_priv->hotplug.irq_port[PORT_C] = intel_dig_port;
+ } else
+ dev_priv->hotplug.irq_port[port] = intel_dig_port;
}
/* In theory we don't need the encoder->type check, but leave it just in
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fcc64e5..71679ef 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
switch (port) {
case PORT_A:
intel_encoder->hpd_pin = HPD_PORT_A;
+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+ intel_encoder->hpd_pin = HPD_PORT_C;
break;
case PORT_B:
intel_encoder->hpd_pin = HPD_PORT_B;
+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+ intel_encoder->hpd_pin = HPD_PORT_A;
break;
case PORT_C:
intel_encoder->hpd_pin = HPD_PORT_C;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 70bad5b..94fa716 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
else
intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
- intel_encoder->hpd_pin = HPD_PORT_B;
+ /*
+ * On BXT A0/A1, sw needs to activate DDIA HPD logic and
+ * interrupts to check the external panel connection.
+ */
+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+ intel_encoder->hpd_pin = HPD_PORT_A;
+ else
+ intel_encoder->hpd_pin = HPD_PORT_B;
break;
case PORT_C:
if (IS_BROXTON(dev_priv))
--
1.7.10.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-22 10:07 ` Sonika Jindal
@ 2015-07-22 10:33 ` Sivakumar Thulasimani
2015-07-22 11:09 ` Jindal, Sonika
2015-07-27 5:32 ` Sonika Jindal
1 sibling, 1 reply; 31+ messages in thread
From: Sivakumar Thulasimani @ 2015-07-22 10:33 UTC (permalink / raw)
To: Sonika Jindal; +Cc: intel-gfx
On 7/22/2015 3:37 PM, 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 DDIC HPD
> logic for edp panel.
>
> v2: For DP, irq_port is used to determine the encoder instead of
> hpd_pin and removing the edp HPD logic because port A HPD is not
> present(Imre)
> v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
> Added hpd_pin swapping for intel_dp_init_connector, setting encoder
> for PORT_A as per the WA in irq_port (Imre)
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 12 +++++++++++-
> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
> 3 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e2c6f73..d5745e2 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3225,7 +3225,17 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> goto err;
>
> intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> - dev_priv->hotplug.irq_port[port] = intel_dig_port;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)) {
> + if (port == PORT_B)
> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> + else if (port == PORT_A)
> + dev_priv->hotplug.irq_port[PORT_C] = intel_dig_port;
> + } else
> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
> }
>
> /* In theory we don't need the encoder->type check, but leave it just in
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fcc64e5..71679ef 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> switch (port) {
> case PORT_A:
> intel_encoder->hpd_pin = HPD_PORT_A;
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_C;
> break;
> case PORT_B:
> intel_encoder->hpd_pin = HPD_PORT_B;
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> break;
> case PORT_C:
> intel_encoder->hpd_pin = HPD_PORT_C;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 70bad5b..94fa716 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> else
> intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> - intel_encoder->hpd_pin = HPD_PORT_B;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_B;
> break;
> case PORT_C:
> if (IS_BROXTON(dev_priv))
bxt_hpd_irq_setup is not touched here without this being updated i dont
think HPD can be enabled.
--
regards,
Sivakumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-22 10:33 ` Sivakumar Thulasimani
@ 2015-07-22 11:09 ` Jindal, Sonika
2015-07-22 11:31 ` Sivakumar Thulasimani
0 siblings, 1 reply; 31+ messages in thread
From: Jindal, Sonika @ 2015-07-22 11:09 UTC (permalink / raw)
To: Sivakumar Thulasimani; +Cc: intel-gfx
On 7/22/2015 4:03 PM, Sivakumar Thulasimani wrote:
>
>
> On 7/22/2015 3:37 PM, 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 DDIC HPD
>> logic for edp panel.
>>
>> v2: For DP, irq_port is used to determine the encoder instead of
>> hpd_pin and removing the edp HPD logic because port A HPD is not
>> present(Imre)
>> v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
>> Added hpd_pin swapping for intel_dp_init_connector, setting encoder
>> for PORT_A as per the WA in irq_port (Imre)
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_ddi.c | 12 +++++++++++-
>> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
>> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
>> 3 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>> b/drivers/gpu/drm/i915/intel_ddi.c
>> index e2c6f73..d5745e2 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -3225,7 +3225,17 @@ void intel_ddi_init(struct drm_device *dev,
>> enum port port)
>> goto err;
>> intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
>> - dev_priv->hotplug.irq_port[port] = intel_dig_port;
>> + /*
>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>> + * interrupts to check the external panel connection.
>> + */
>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)) {
>> + if (port == PORT_B)
>> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
>> + else if (port == PORT_A)
>> + dev_priv->hotplug.irq_port[PORT_C] = intel_dig_port;
>> + } else
>> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
>> }
>> /* In theory we don't need the encoder->type check, but leave it
>> just in
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index fcc64e5..71679ef 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct
>> intel_digital_port *intel_dig_port,
>> switch (port) {
>> case PORT_A:
>> intel_encoder->hpd_pin = HPD_PORT_A;
>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>> + intel_encoder->hpd_pin = HPD_PORT_C;
>> break;
>> case PORT_B:
>> intel_encoder->hpd_pin = HPD_PORT_B;
>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>> + intel_encoder->hpd_pin = HPD_PORT_A;
>> break;
>> case PORT_C:
>> intel_encoder->hpd_pin = HPD_PORT_C;
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
>> b/drivers/gpu/drm/i915/intel_hdmi.c
>> index 70bad5b..94fa716 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct
>> intel_digital_port *intel_dig_port,
>> intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
>> else
>> intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
>> - intel_encoder->hpd_pin = HPD_PORT_B;
>> + /*
>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>> + * interrupts to check the external panel connection.
>> + */
>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>> + intel_encoder->hpd_pin = HPD_PORT_A;
>> + else
>> + intel_encoder->hpd_pin = HPD_PORT_B;
>> break;
>> case PORT_C:
>> if (IS_BROXTON(dev_priv))
> bxt_hpd_irq_setup is not touched here without this being updated i dont
> think HPD can be enabled.
Now, we swap the hpd_pin, which is being used to enable HPD in
bxt_hpd_irq_setup.
Regards,
Sonika
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-22 11:09 ` Jindal, Sonika
@ 2015-07-22 11:31 ` Sivakumar Thulasimani
2015-07-22 12:02 ` Jindal, Sonika
0 siblings, 1 reply; 31+ messages in thread
From: Sivakumar Thulasimani @ 2015-07-22 11:31 UTC (permalink / raw)
To: Jindal, Sonika; +Cc: intel-gfx
On 7/22/2015 4:39 PM, Jindal, Sonika wrote:
>
>
> On 7/22/2015 4:03 PM, Sivakumar Thulasimani wrote:
>>
>>
>> On 7/22/2015 3:37 PM, 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 DDIC HPD
>>> logic for edp panel.
>>>
>>> v2: For DP, irq_port is used to determine the encoder instead of
>>> hpd_pin and removing the edp HPD logic because port A HPD is not
>>> present(Imre)
>>> v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
>>> Added hpd_pin swapping for intel_dp_init_connector, setting encoder
>>> for PORT_A as per the WA in irq_port (Imre)
>>>
>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/intel_ddi.c | 12 +++++++++++-
>>> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
>>> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
>>> 3 files changed, 23 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>>> b/drivers/gpu/drm/i915/intel_ddi.c
>>> index e2c6f73..d5745e2 100644
>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>> @@ -3225,7 +3225,17 @@ void intel_ddi_init(struct drm_device *dev,
>>> enum port port)
>>> goto err;
>>> intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
>>> - dev_priv->hotplug.irq_port[port] = intel_dig_port;
>>> + /*
>>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>>> + * interrupts to check the external panel connection.
>>> + */
>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) <
>>> BXT_REVID_B0)) {
>>> + if (port == PORT_B)
>>> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
>>> + else if (port == PORT_A)
>>> + dev_priv->hotplug.irq_port[PORT_C] = intel_dig_port;
>>> + } else
>>> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
>>> }
>>> /* In theory we don't need the encoder->type check, but leave it
>>> just in
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>>> b/drivers/gpu/drm/i915/intel_dp.c
>>> index fcc64e5..71679ef 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct
>>> intel_digital_port *intel_dig_port,
>>> switch (port) {
>>> case PORT_A:
>>> intel_encoder->hpd_pin = HPD_PORT_A;
>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>>> + intel_encoder->hpd_pin = HPD_PORT_C;
>>> break;
>>> case PORT_B:
>>> intel_encoder->hpd_pin = HPD_PORT_B;
>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>>> + intel_encoder->hpd_pin = HPD_PORT_A;
>>> break;
>>> case PORT_C:
>>> intel_encoder->hpd_pin = HPD_PORT_C;
>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
>>> b/drivers/gpu/drm/i915/intel_hdmi.c
>>> index 70bad5b..94fa716 100644
>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>> @@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct
>>> intel_digital_port *intel_dig_port,
>>> intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
>>> else
>>> intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
>>> - intel_encoder->hpd_pin = HPD_PORT_B;
>>> + /*
>>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>>> + * interrupts to check the external panel connection.
>>> + */
>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>>> + intel_encoder->hpd_pin = HPD_PORT_A;
>>> + else
>>> + intel_encoder->hpd_pin = HPD_PORT_B;
>>> break;
>>> case PORT_C:
>>> if (IS_BROXTON(dev_priv))
>> bxt_hpd_irq_setup is not touched here without this being updated i dont
>> think HPD can be enabled.
> Now, we swap the hpd_pin, which is being used to enable HPD in
> bxt_hpd_irq_setup.
>
yes, but the hpd_pin which is updated to HPD_PORT_A when enters
bxt_hpd_setup_irq we are enabling
only HPD B & C. so it will not enable HPDA. unless i am missing some
patch i am not seeing code to
enable HPD A with latest nightly code.
> Regards,
> Sonika
>>
--
regards,
Sivakumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-22 11:31 ` Sivakumar Thulasimani
@ 2015-07-22 12:02 ` Jindal, Sonika
2015-07-22 12:20 ` Sivakumar Thulasimani
0 siblings, 1 reply; 31+ messages in thread
From: Jindal, Sonika @ 2015-07-22 12:02 UTC (permalink / raw)
To: Sivakumar Thulasimani; +Cc: intel-gfx
On 7/22/2015 5:01 PM, Sivakumar Thulasimani wrote:
>
>
> On 7/22/2015 4:39 PM, Jindal, Sonika wrote:
>>
>>
>> On 7/22/2015 4:03 PM, Sivakumar Thulasimani wrote:
>>>
>>>
>>> On 7/22/2015 3:37 PM, 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 DDIC HPD
>>>> logic for edp panel.
>>>>
>>>> v2: For DP, irq_port is used to determine the encoder instead of
>>>> hpd_pin and removing the edp HPD logic because port A HPD is not
>>>> present(Imre)
>>>> v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
>>>> Added hpd_pin swapping for intel_dp_init_connector, setting encoder
>>>> for PORT_A as per the WA in irq_port (Imre)
>>>>
>>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/intel_ddi.c | 12 +++++++++++-
>>>> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
>>>> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
>>>> 3 files changed, 23 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>>>> b/drivers/gpu/drm/i915/intel_ddi.c
>>>> index e2c6f73..d5745e2 100644
>>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>>> @@ -3225,7 +3225,17 @@ void intel_ddi_init(struct drm_device *dev,
>>>> enum port port)
>>>> goto err;
>>>> intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
>>>> - dev_priv->hotplug.irq_port[port] = intel_dig_port;
>>>> + /*
>>>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>>>> + * interrupts to check the external panel connection.
>>>> + */
>>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) <
>>>> BXT_REVID_B0)) {
>>>> + if (port == PORT_B)
>>>> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
>>>> + else if (port == PORT_A)
>>>> + dev_priv->hotplug.irq_port[PORT_C] = intel_dig_port;
>>>> + } else
>>>> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
>>>> }
>>>> /* In theory we don't need the encoder->type check, but leave it
>>>> just in
>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>>>> b/drivers/gpu/drm/i915/intel_dp.c
>>>> index fcc64e5..71679ef 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>> @@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct
>>>> intel_digital_port *intel_dig_port,
>>>> switch (port) {
>>>> case PORT_A:
>>>> intel_encoder->hpd_pin = HPD_PORT_A;
>>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>>>> + intel_encoder->hpd_pin = HPD_PORT_C;
>>>> break;
>>>> case PORT_B:
>>>> intel_encoder->hpd_pin = HPD_PORT_B;
>>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>>>> + intel_encoder->hpd_pin = HPD_PORT_A;
>>>> break;
>>>> case PORT_C:
>>>> intel_encoder->hpd_pin = HPD_PORT_C;
>>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
>>>> b/drivers/gpu/drm/i915/intel_hdmi.c
>>>> index 70bad5b..94fa716 100644
>>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>>> @@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct
>>>> intel_digital_port *intel_dig_port,
>>>> intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
>>>> else
>>>> intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
>>>> - intel_encoder->hpd_pin = HPD_PORT_B;
>>>> + /*
>>>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>>>> + * interrupts to check the external panel connection.
>>>> + */
>>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>>>> + intel_encoder->hpd_pin = HPD_PORT_A;
>>>> + else
>>>> + intel_encoder->hpd_pin = HPD_PORT_B;
>>>> break;
>>>> case PORT_C:
>>>> if (IS_BROXTON(dev_priv))
>>> bxt_hpd_irq_setup is not touched here without this being updated i dont
>>> think HPD can be enabled.
>> Now, we swap the hpd_pin, which is being used to enable HPD in
>> bxt_hpd_irq_setup.
>>
> yes, but the hpd_pin which is updated to HPD_PORT_A when enters
> bxt_hpd_setup_irq we are enabling
> only HPD B & C. so it will not enable HPDA. unless i am missing some
> patch i am not seeing code to
> enable HPD A with latest nightly code.
>
Yes, you are missing the patch 1 of this series :)
[PATCH 1/2] drm/i915/bxt: Add HPD support for DDIA
>> Regards,
>> Sonika
>>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-22 12:02 ` Jindal, Sonika
@ 2015-07-22 12:20 ` Sivakumar Thulasimani
0 siblings, 0 replies; 31+ messages in thread
From: Sivakumar Thulasimani @ 2015-07-22 12:20 UTC (permalink / raw)
To: Jindal, Sonika; +Cc: intel-gfx
On 7/22/2015 5:32 PM, Jindal, Sonika wrote:
>
>
> On 7/22/2015 5:01 PM, Sivakumar Thulasimani wrote:
>>
>>
>> On 7/22/2015 4:39 PM, Jindal, Sonika wrote:
>>>
>>>
>>> On 7/22/2015 4:03 PM, Sivakumar Thulasimani wrote:
>>>>
>>>>
>>>> On 7/22/2015 3:37 PM, 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 DDIC HPD
>>>>> logic for edp panel.
>>>>>
>>>>> v2: For DP, irq_port is used to determine the encoder instead of
>>>>> hpd_pin and removing the edp HPD logic because port A HPD is not
>>>>> present(Imre)
>>>>> v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
>>>>> Added hpd_pin swapping for intel_dp_init_connector, setting encoder
>>>>> for PORT_A as per the WA in irq_port (Imre)
>>>>>
>>>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>>>> ---
>>>>> drivers/gpu/drm/i915/intel_ddi.c | 12 +++++++++++-
>>>>> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
>>>>> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
>>>>> 3 files changed, 23 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>>>>> b/drivers/gpu/drm/i915/intel_ddi.c
>>>>> index e2c6f73..d5745e2 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>>>> @@ -3225,7 +3225,17 @@ void intel_ddi_init(struct drm_device *dev,
>>>>> enum port port)
>>>>> goto err;
>>>>> intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
>>>>> - dev_priv->hotplug.irq_port[port] = intel_dig_port;
>>>>> + /*
>>>>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>>>>> + * interrupts to check the external panel connection.
>>>>> + */
>>>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) <
>>>>> BXT_REVID_B0)) {
>>>>> + if (port == PORT_B)
>>>>> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
>>>>> + else if (port == PORT_A)
>>>>> + dev_priv->hotplug.irq_port[PORT_C] = intel_dig_port;
Please don't enable interrupt for PORT_A :) otherwise we might have to
handle HPD
for each pps on/off like in CHV.
>>>>> + } else
>>>>> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
>>>>> }
>>>>> /* In theory we don't need the encoder->type check, but
>>>>> leave it
>>>>> just in
>>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>>>>> b/drivers/gpu/drm/i915/intel_dp.c
>>>>> index fcc64e5..71679ef 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>>> @@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct
>>>>> intel_digital_port *intel_dig_port,
>>>>> switch (port) {
>>>>> case PORT_A:
>>>>> intel_encoder->hpd_pin = HPD_PORT_A;
>>>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) <
>>>>> BXT_REVID_B0))
>>>>> + intel_encoder->hpd_pin = HPD_PORT_C;
>>>>> break;
>>>>> case PORT_B:
>>>>> intel_encoder->hpd_pin = HPD_PORT_B;
>>>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) <
>>>>> BXT_REVID_B0))
>>>>> + intel_encoder->hpd_pin = HPD_PORT_A;
>>>>> break;
>>>>> case PORT_C:
>>>>> intel_encoder->hpd_pin = HPD_PORT_C;
>>>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
>>>>> b/drivers/gpu/drm/i915/intel_hdmi.c
>>>>> index 70bad5b..94fa716 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>>>> @@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct
>>>>> intel_digital_port *intel_dig_port,
>>>>> intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
>>>>> else
>>>>> intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
>>>>> - intel_encoder->hpd_pin = HPD_PORT_B;
>>>>> + /*
>>>>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>>>>> + * interrupts to check the external panel connection.
>>>>> + */
>>>>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) <
>>>>> BXT_REVID_B0))
>>>>> + intel_encoder->hpd_pin = HPD_PORT_A;
>>>>> + else
>>>>> + intel_encoder->hpd_pin = HPD_PORT_B;
>>>>> break;
>>>>> case PORT_C:
>>>>> if (IS_BROXTON(dev_priv))
>>>> bxt_hpd_irq_setup is not touched here without this being updated i
>>>> dont
>>>> think HPD can be enabled.
>>> Now, we swap the hpd_pin, which is being used to enable HPD in
>>> bxt_hpd_irq_setup.
>>>
>> yes, but the hpd_pin which is updated to HPD_PORT_A when enters
>> bxt_hpd_setup_irq we are enabling
>> only HPD B & C. so it will not enable HPDA. unless i am missing some
>> patch i am not seeing code to
>> enable HPD A with latest nightly code.
>>
> Yes, you are missing the patch 1 of this series :)
> [PATCH 1/2] drm/i915/bxt: Add HPD support for DDIA
>
>>> Regards,
>>> Sonika
>>>>
>>
ok missed that in the thread list :).
--
regards,
Sivakumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-22 10:07 ` Sonika Jindal
2015-07-22 10:33 ` Sivakumar Thulasimani
@ 2015-07-27 5:32 ` Sonika Jindal
2015-07-27 11:46 ` Sivakumar Thulasimani
` (2 more replies)
1 sibling, 3 replies; 31+ messages in thread
From: Sonika Jindal @ 2015-07-27 5:32 UTC (permalink / raw)
To: intel-gfx
WA for BXT A0/A1, where DDIB's HPD pin is swapped to DDIA, so enabling
DDIA HPD pin in place of DDIB.
v2: For DP, irq_port is used to determine the encoder instead of
hpd_pin and removing the edp HPD logic because port A HPD is not
present(Imre)
v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
Added hpd_pin swapping for intel_dp_init_connector, setting encoder
for PORT_A as per the WA in irq_port (Imre)
v4: Dont enable interrupt for edp, also reframe the description (Siva)
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
drivers/gpu/drm/i915/intel_ddi.c | 11 ++++++++++-
drivers/gpu/drm/i915/intel_dp.c | 4 ++++
drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
3 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e2c6f73..8d7ffe0 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3225,7 +3225,16 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
goto err;
intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
- dev_priv->hotplug.irq_port[port] = intel_dig_port;
+ /*
+ * On BXT A0/A1, sw needs to activate DDIA HPD logic and
+ * interrupts to check the external panel connection.
+ */
+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
+ && port == PORT_B)
+ dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
+ /* Dont enable interrupts for edp*/
+ else if (port != PORT_A)
+ dev_priv->hotplug.irq_port[port] = intel_dig_port;
}
/* In theory we don't need the encoder->type check, but leave it just in
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fcc64e5..71679ef 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
switch (port) {
case PORT_A:
intel_encoder->hpd_pin = HPD_PORT_A;
+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+ intel_encoder->hpd_pin = HPD_PORT_C;
break;
case PORT_B:
intel_encoder->hpd_pin = HPD_PORT_B;
+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+ intel_encoder->hpd_pin = HPD_PORT_A;
break;
case PORT_C:
intel_encoder->hpd_pin = HPD_PORT_C;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 70bad5b..94fa716 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
else
intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
- intel_encoder->hpd_pin = HPD_PORT_B;
+ /*
+ * On BXT A0/A1, sw needs to activate DDIA HPD logic and
+ * interrupts to check the external panel connection.
+ */
+ if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+ intel_encoder->hpd_pin = HPD_PORT_A;
+ else
+ intel_encoder->hpd_pin = HPD_PORT_B;
break;
case PORT_C:
if (IS_BROXTON(dev_priv))
--
1.7.10.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-27 5:32 ` Sonika Jindal
@ 2015-07-27 11:46 ` Sivakumar Thulasimani
2015-07-28 8:42 ` shuang.he
2015-08-05 9:53 ` Imre Deak
2 siblings, 0 replies; 31+ messages in thread
From: Sivakumar Thulasimani @ 2015-07-27 11:46 UTC (permalink / raw)
To: Sonika Jindal, intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 3408 bytes --]
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
On 7/27/2015 11:02 AM, Sonika Jindal wrote:
> WA for BXT A0/A1, where DDIB's HPD pin is swapped to DDIA, so enabling
> DDIA HPD pin in place of DDIB.
>
> v2: For DP, irq_port is used to determine the encoder instead of
> hpd_pin and removing the edp HPD logic because port A HPD is not
> present(Imre)
> v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
> Added hpd_pin swapping for intel_dp_init_connector, setting encoder
> for PORT_A as per the WA in irq_port (Imre)
> v4: Dont enable interrupt for edp, also reframe the description (Siva)
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 11 ++++++++++-
> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
> 3 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e2c6f73..8d7ffe0 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3225,7 +3225,16 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> goto err;
>
> intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> - dev_priv->hotplug.irq_port[port] = intel_dig_port;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> + && port == PORT_B)
> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> + /* Dont enable interrupts for edp*/
> + else if (port != PORT_A)
> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
> }
>
> /* In theory we don't need the encoder->type check, but leave it just in
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fcc64e5..71679ef 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> switch (port) {
> case PORT_A:
> intel_encoder->hpd_pin = HPD_PORT_A;
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_C;
> break;
> case PORT_B:
> intel_encoder->hpd_pin = HPD_PORT_B;
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> break;
> case PORT_C:
> intel_encoder->hpd_pin = HPD_PORT_C;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 70bad5b..94fa716 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> else
> intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> - intel_encoder->hpd_pin = HPD_PORT_B;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_B;
> break;
> case PORT_C:
> if (IS_BROXTON(dev_priv))
--
regards,
Sivakumar
[-- Attachment #1.2: Type: text/html, Size: 4210 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-27 5:32 ` Sonika Jindal
2015-07-27 11:46 ` Sivakumar Thulasimani
@ 2015-07-28 8:42 ` shuang.he
2015-08-05 9:53 ` Imre Deak
2 siblings, 0 replies; 31+ messages in thread
From: shuang.he @ 2015-07-28 8:42 UTC (permalink / raw)
To: shuang.he, julianx.dumez, christophe.sureau, lei.a.liu, intel-gfx,
sonika.jindal
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6870
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK -1 296/296 295/296
SNB -1 316/316 315/316
IVB -1 343/343 342/343
BYT -3 286/286 283/286
HSW -1 379/379 378/379
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*ILK igt@gem_reg_read PASS(1) NRUN(1)
*SNB igt@gem_reg_read PASS(1) INIT(1)
*IVB igt@gem_reg_read PASS(1) NRUN(1)
*BYT igt@drm_read@short-buffer-block PASS(1) FAIL(1)
*BYT igt@gem_partial_pwrite_pread@reads-uncached PASS(1) FAIL(1)
*BYT igt@gem_reg_read PASS(1) INIT(1)
*HSW igt@gem_reg_read PASS(1) INIT(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-07-27 5:32 ` Sonika Jindal
2015-07-27 11:46 ` Sivakumar Thulasimani
2015-07-28 8:42 ` shuang.he
@ 2015-08-05 9:53 ` Imre Deak
2015-08-05 10:11 ` Sivakumar Thulasimani
2 siblings, 1 reply; 31+ messages in thread
From: Imre Deak @ 2015-08-05 9:53 UTC (permalink / raw)
To: Sonika Jindal; +Cc: intel-gfx
On Mon, 2015-07-27 at 11:02 +0530, Sonika Jindal wrote:
> WA for BXT A0/A1, where DDIB's HPD pin is swapped to DDIA, so enabling
> DDIA HPD pin in place of DDIB.
>
> v2: For DP, irq_port is used to determine the encoder instead of
> hpd_pin and removing the edp HPD logic because port A HPD is not
> present(Imre)
> v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
> Added hpd_pin swapping for intel_dp_init_connector, setting encoder
> for PORT_A as per the WA in irq_port (Imre)
> v4: Dont enable interrupt for edp, also reframe the description (Siva)
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
This patch is a new version of 2/2 of your patchset at
http://lists.freedesktop.org/archives/intel-gfx/2015-July/071580.html
so this should've been sent either as a reply to the individual v1 patch
there, or just by resending the whole patchset. For now you could just
resend the whole patchset. Also please add the version info to the
subject with git format-patch --subject-prefix.
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 11 ++++++++++-
> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
> 3 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e2c6f73..8d7ffe0 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3225,7 +3225,16 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> goto err;
>
> intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> - dev_priv->hotplug.irq_port[port] = intel_dig_port;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> + && port == PORT_B)
> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> + /* Dont enable interrupts for edp*/
> + else if (port != PORT_A)
> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
Not enabling HPD on eDP is not a BXT specific change (even though it's
disabled now everywhere), so that part should be a separate patch and
ordered before patch 1/2. Also this isn't the proper place to disable
HPD handling; as long as the corresponding intel_encoder->hpd_pin is set
the relevant interrupt will get unmasked and hotplug_work will be
called. See bxt_hpd_irq_setup() and intel_hpd_irq_handler().
> }
>
> /* In theory we don't need the encoder->type check, but leave it just in
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fcc64e5..71679ef 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> switch (port) {
> case PORT_A:
> intel_encoder->hpd_pin = HPD_PORT_A;
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_C;
> break;
> case PORT_B:
> intel_encoder->hpd_pin = HPD_PORT_B;
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> break;
> case PORT_C:
> intel_encoder->hpd_pin = HPD_PORT_C;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 70bad5b..94fa716 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> else
> intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> - intel_encoder->hpd_pin = HPD_PORT_B;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_B;
> break;
> case PORT_C:
> if (IS_BROXTON(dev_priv))
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping
2015-08-05 9:53 ` Imre Deak
@ 2015-08-05 10:11 ` Sivakumar Thulasimani
0 siblings, 0 replies; 31+ messages in thread
From: Sivakumar Thulasimani @ 2015-08-05 10:11 UTC (permalink / raw)
To: imre.deak; +Cc: intel-gfx
On 8/5/2015 3:23 PM, Imre Deak wrote:
> On Mon, 2015-07-27 at 11:02 +0530, Sonika Jindal wrote:
>> WA for BXT A0/A1, where DDIB's HPD pin is swapped to DDIA, so enabling
>> DDIA HPD pin in place of DDIB.
>>
>> v2: For DP, irq_port is used to determine the encoder instead of
>> hpd_pin and removing the edp HPD logic because port A HPD is not
>> present(Imre)
>> v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
>> Added hpd_pin swapping for intel_dp_init_connector, setting encoder
>> for PORT_A as per the WA in irq_port (Imre)
>> v4: Dont enable interrupt for edp, also reframe the description (Siva)
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> This patch is a new version of 2/2 of your patchset at
> http://lists.freedesktop.org/archives/intel-gfx/2015-July/071580.html
>
> so this should've been sent either as a reply to the individual v1 patch
> there, or just by resending the whole patchset. For now you could just
> resend the whole patchset. Also please add the version info to the
> subject with git format-patch --subject-prefix.
>
>
>> ---
>> drivers/gpu/drm/i915/intel_ddi.c | 11 ++++++++++-
>> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
>> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
>> 3 files changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index e2c6f73..8d7ffe0 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -3225,7 +3225,16 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>> goto err;
>>
>> intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
>> - dev_priv->hotplug.irq_port[port] = intel_dig_port;
>> + /*
>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>> + * interrupts to check the external panel connection.
>> + */
>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
>> + && port == PORT_B)
>> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
>> + /* Dont enable interrupts for edp*/
>> + else if (port != PORT_A)
>> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
> Not enabling HPD on eDP is not a BXT specific change (even though it's
> disabled now everywhere), so that part should be a separate patch and
> ordered before patch 1/2. Also this isn't the proper place to disable
> HPD handling; as long as the corresponding intel_encoder->hpd_pin is set
> the relevant interrupt will get unmasked and hotplug_work will be
> called. See bxt_hpd_irq_setup() and intel_hpd_irq_handler().
>
hmm , ok Sonika, please remove HPD_PORT_A being set for PORT_A. here
else if can
be modified to simple else
>> }
>>
>> /* In theory we don't need the encoder->type check, but leave it just in
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index fcc64e5..71679ef 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>> switch (port) {
>> case PORT_A:
>> intel_encoder->hpd_pin = HPD_PORT_A;
>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>> + intel_encoder->hpd_pin = HPD_PORT_C;
>> break;
>> case PORT_B:
>> intel_encoder->hpd_pin = HPD_PORT_B;
>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>> + intel_encoder->hpd_pin = HPD_PORT_A;
>> break;
>> case PORT_C:
>> intel_encoder->hpd_pin = HPD_PORT_C;
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index 70bad5b..94fa716 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>> intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
>> else
>> intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
>> - intel_encoder->hpd_pin = HPD_PORT_B;
>> + /*
>> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>> + * interrupts to check the external panel connection.
>> + */
>> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
>> + intel_encoder->hpd_pin = HPD_PORT_A;
>> + else
>> + intel_encoder->hpd_pin = HPD_PORT_B;
>> break;
>> case PORT_C:
>> if (IS_BROXTON(dev_priv))
>
>
>
--
regards,
Sivakumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2015-08-05 10:11 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-13 4:17 [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping Sonika Jindal
2015-07-13 6:31 ` Sivakumar Thulasimani
2015-07-13 8:11 ` Jindal, Sonika
2015-07-13 8:40 ` Sonika Jindal
2015-07-13 9:40 ` Daniel Vetter
2015-07-13 10:57 ` Sivakumar Thulasimani
2015-07-14 5:48 ` [PATCH 1/2] drm/i915/bxt: Add HPD support for DDIA Sonika Jindal
2015-07-14 5:48 ` [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping Sonika Jindal
2015-07-14 8:10 ` Daniel Vetter
2015-07-14 14:22 ` Imre Deak
2015-07-15 6:35 ` Jindal, Sonika
2015-07-15 8:04 ` Jindal, Sonika
2015-07-15 9:07 ` Daniel Vetter
2015-07-17 4:29 ` Jindal, Sonika
2015-07-17 8:17 ` [PATCH] " Sonika Jindal
2015-07-17 23:47 ` Imre Deak
2015-07-20 6:06 ` Jindal, Sonika
2015-07-20 21:43 ` Imre Deak
2015-07-22 10:07 ` Sonika Jindal
2015-07-22 10:33 ` Sivakumar Thulasimani
2015-07-22 11:09 ` Jindal, Sonika
2015-07-22 11:31 ` Sivakumar Thulasimani
2015-07-22 12:02 ` Jindal, Sonika
2015-07-22 12:20 ` Sivakumar Thulasimani
2015-07-27 5:32 ` Sonika Jindal
2015-07-27 11:46 ` Sivakumar Thulasimani
2015-07-28 8:42 ` shuang.he
2015-08-05 9:53 ` Imre Deak
2015-08-05 10:11 ` Sivakumar Thulasimani
2015-07-20 5:07 ` shuang.he
2015-07-13 8:26 ` shuang.he
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.