| Message ID | 1441970912-5961-1-git-send-email-sonika.jindal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 11, 2015 at 4:40 AM Sonika Jindal <sonika.jindal@intel.com> wrote: > Using intel_encoder's hpd_pin to check the live status > because of BXT A0/A1 WA for HPD pins and hpd_pin contains the > updated pin for the corresponding port. > It makes sense, but is it always true? or we should have a fallback to intel_dig_port->port in some case or in some stepping after A0/A1? With this clarified feel free to use: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > b/drivers/gpu/drm/i915/intel_dp.c > index 796f930..bf17030 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4663,11 +4663,14 @@ static bool vlv_digital_port_connected(struct > drm_i915_private *dev_priv, > } > > static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, > - struct intel_digital_port *port) > + struct intel_digital_port > *intel_dig_port) > { > + struct intel_encoder *intel_encoder = &intel_dig_port->base; > + enum port port; > u32 bit; > > - switch (port->port) { > + intel_hpd_pin_to_port(intel_encoder->hpd_pin, &port); > + switch (port) { > case PORT_A: > bit = BXT_DE_PORT_HP_DDIA; > break; > @@ -4678,7 +4681,7 @@ static bool bxt_digital_port_connected(struct > drm_i915_private *dev_priv, > bit = BXT_DE_PORT_HP_DDIC; > break; > default: > - MISSING_CASE(port->port); > + MISSING_CASE(port); > return false; > } > > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Fri, Sep 11, 2015 at 04:58:32PM +0530, Sonika Jindal wrote: > Using intel_encoder's hpd_pin to check the live status > because of BXT A0/A1 WA for HPD pins and hpd_pin contains the > updated pin for the corresponding port. > > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 796f930..bf17030 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4663,11 +4663,14 @@ static bool vlv_digital_port_connected(struct drm_i915_private *dev_priv, > } > > static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, > - struct intel_digital_port *port) > + struct intel_digital_port *intel_dig_port) > { > + struct intel_encoder *intel_encoder = &intel_dig_port->base; > + enum port port; > u32 bit; > > - switch (port->port) { > + intel_hpd_pin_to_port(intel_encoder->hpd_pin, &port); This looks very wrong - the function you're calling here maps from hpd_pin to the port, but what you actually want is to map from port to pin. That mapping is in intel_encoder->hpd_pin. -Daniel > + switch (port) { > case PORT_A: > bit = BXT_DE_PORT_HP_DDIA; > break; > @@ -4678,7 +4681,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, > bit = BXT_DE_PORT_HP_DDIC; > break; > default: > - MISSING_CASE(port->port); > + MISSING_CASE(port); > return false; > } > > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 9/14/2015 2:04 PM, Daniel Vetter wrote: > On Fri, Sep 11, 2015 at 04:58:32PM +0530, Sonika Jindal wrote: >> Using intel_encoder's hpd_pin to check the live status >> because of BXT A0/A1 WA for HPD pins and hpd_pin contains the >> updated pin for the corresponding port. >> >> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 796f930..bf17030 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -4663,11 +4663,14 @@ static bool vlv_digital_port_connected(struct drm_i915_private *dev_priv, >> } >> >> static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, >> - struct intel_digital_port *port) >> + struct intel_digital_port *intel_dig_port) >> { >> + struct intel_encoder *intel_encoder = &intel_dig_port->base; >> + enum port port; >> u32 bit; >> >> - switch (port->port) { >> + intel_hpd_pin_to_port(intel_encoder->hpd_pin, &port); > > This looks very wrong - the function you're calling here maps from hpd_pin > to the port, but what you actually want is to map from port to pin. That > mapping is in intel_encoder->hpd_pin. > -Daniel No, I want to find out the correct port whose hpd bit we need to check. That port we are finding based upon the hpd_pin we have set for that encoder. This was the whole point of using hpd_pin for the BXT A0/A1 WA. That instead of checking different HPD bit based upon the port, just change the hpd_pin itself in intel_encoder. Regards, Sonika > >> + switch (port) { >> case PORT_A: >> bit = BXT_DE_PORT_HP_DDIA; >> break; >> @@ -4678,7 +4681,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, >> bit = BXT_DE_PORT_HP_DDIC; >> break; >> default: >> - MISSING_CASE(port->port); >> + MISSING_CASE(port); >> return false; >> } >> >> -- >> 1.7.10.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Mon, Sep 14, 2015 at 02:08:41PM +0530, Jindal, Sonika wrote: > > > On 9/14/2015 2:04 PM, Daniel Vetter wrote: > >On Fri, Sep 11, 2015 at 04:58:32PM +0530, Sonika Jindal wrote: > >>Using intel_encoder's hpd_pin to check the live status > >>because of BXT A0/A1 WA for HPD pins and hpd_pin contains the > >>updated pin for the corresponding port. > >> > >>Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> > >>--- > >> drivers/gpu/drm/i915/intel_dp.c | 9 ++++++--- > >> 1 file changed, 6 insertions(+), 3 deletions(-) > >> > >>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > >>index 796f930..bf17030 100644 > >>--- a/drivers/gpu/drm/i915/intel_dp.c > >>+++ b/drivers/gpu/drm/i915/intel_dp.c > >>@@ -4663,11 +4663,14 @@ static bool vlv_digital_port_connected(struct drm_i915_private *dev_priv, > >> } > >> > >> static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, > >>- struct intel_digital_port *port) > >>+ struct intel_digital_port *intel_dig_port) > >> { > >>+ struct intel_encoder *intel_encoder = &intel_dig_port->base; > >>+ enum port port; > >> u32 bit; > >> > >>- switch (port->port) { > >>+ intel_hpd_pin_to_port(intel_encoder->hpd_pin, &port); > > > >This looks very wrong - the function you're calling here maps from hpd_pin > >to the port, but what you actually want is to map from port to pin. That > >mapping is in intel_encoder->hpd_pin. > >-Daniel > No, I want to find out the correct port whose hpd bit we need to check. > That port we are finding based upon the hpd_pin we have set for that > encoder. This was the whole point of using hpd_pin for the BXT A0/A1 WA. > That instead of checking different HPD bit based upon the port, just change > the hpd_pin itself in intel_encoder. Ah right I was getting confused. Patch applied now, thanks for clarifying. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 796f930..bf17030 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4663,11 +4663,14 @@ static bool vlv_digital_port_connected(struct drm_i915_private *dev_priv, } static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, - struct intel_digital_port *port) + struct intel_digital_port *intel_dig_port) { + struct intel_encoder *intel_encoder = &intel_dig_port->base; + enum port port; u32 bit; - switch (port->port) { + intel_hpd_pin_to_port(intel_encoder->hpd_pin, &port); + switch (port) { case PORT_A: bit = BXT_DE_PORT_HP_DDIA; break; @@ -4678,7 +4681,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, bit = BXT_DE_PORT_HP_DDIC; break; default: - MISSING_CASE(port->port); + MISSING_CASE(port); return false; }
Using intel_encoder's hpd_pin to check the live status because of BXT A0/A1 WA for HPD pins and hpd_pin contains the updated pin for the corresponding port. Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)