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@xxxxxxxxx> > >>>--- > >>> 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@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx