Re: [Intel-gfx] [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
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.jin...@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
Re: [Intel-gfx] [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
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.jin...@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
Re: [Intel-gfx] [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
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.jin...@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
Re: [Intel-gfx] [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
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.jin...@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
Re: [Intel-gfx] [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
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.jin...@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
Re: [Intel-gfx] [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
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.jin...@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
[Intel-gfx] [PATCH 2/2] drm/i915/bxt: WA for swapped HPD pins in A stepping
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.jin...@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