Re: [PATCH] drm/i915: Introduce intel_hdp_pin.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2017-08-10 at 14:23 -0700, Rodrigo Vivi wrote:
> On Wed, Aug 9, 2017 at 9:44 PM, Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@xxxxxxxxx> wrote:
> > On Wed, 2017-08-02 at 12:26 -0700, Rodrigo Vivi wrote:
> >> The idea is to have an unique place to decide the pin-port
> >> per platform.
> >>
> >> So let's create this function now without any functional
> >> change.
> >
> > This seems to change the behavior when port A is not eDP.
> >
> >> Just adding together code from hdmi and dp together.
> >>
> >> v2: Add missing pin for port A.
> >>
> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> >> ---
> >>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
> >>  drivers/gpu/drm/i915/intel_dp.c      |  8 ++------
> >>  drivers/gpu/drm/i915/intel_hdmi.c    | 19 +------------------
> >>  drivers/gpu/drm/i915/intel_hotplug.c | 26 ++++++++++++++++++++++++++
> >>  4 files changed, 30 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 5c2c7a677e96..c038717ad739 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -3148,6 +3148,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv);
> >>  void intel_hpd_init_work(struct drm_i915_private *dev_priv);
> >>  void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
> >>  enum port intel_hpd_port(enum hpd_pin pin);
> >> +enum hpd_pin intel_hpd_pin(enum port port);
> >>  bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin);
> >>  void intel_hpd_enable(struct drm_i915_private *dev_priv, enum hpd_pin pin);
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index d4e6128f3b3a..126783752c6d 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -5906,26 +5906,22 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
> >>       struct intel_encoder *encoder = &intel_dig_port->base;
> >>       struct intel_dp *intel_dp = &intel_dig_port->dp;
> >>
> >> +     encoder->hpd_pin = intel_hpd_pin(intel_dig_port->port);
> >> +
> >>       switch (intel_dig_port->port) {
> >>       case PORT_A:
> >> -             encoder->hpd_pin = HPD_PORT_A;
> >>               intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A;
> >>               break;
> >>       case PORT_B:
> >> -             encoder->hpd_pin = HPD_PORT_B;
> >>               intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B;
> >>               break;
> >>       case PORT_C:
> >> -             encoder->hpd_pin = HPD_PORT_C;
> >>               intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C;
> >>               break;
> >>       case PORT_D:
> >> -             encoder->hpd_pin = HPD_PORT_D;
> >>               intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
> >>               break;
> >>       case PORT_E:
> >> -             encoder->hpd_pin = HPD_PORT_E;
> >> -
> >>               /* FIXME: Check VBT for actual wiring of PORT E */
> >>               intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
> >>               break;
> >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> >> index 5609976539bf..dcd14c71a989 100644
> >> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >> @@ -1921,24 +1921,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >>               connector->ycbcr_420_allowed = true;
> >>
> >>       intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> >> -
> >> -     switch (port) {
> >> -     case PORT_B:
> >> -             intel_encoder->hpd_pin = HPD_PORT_B;
> >> -             break;
> >> -     case PORT_C:
> >> -             intel_encoder->hpd_pin = HPD_PORT_C;
> >> -             break;
> >> -     case PORT_D:
> >> -             intel_encoder->hpd_pin = HPD_PORT_D;
> >> -             break;
> >> -     case PORT_E:
> >> -             intel_encoder->hpd_pin = HPD_PORT_E;
> >> -             break;
> >> -     default:
> >> -             MISSING_CASE(port);
> >> -             return;
> >> -     }
> >> +     intel_encoder->hpd_pin = intel_hpd_pin(port);
> >
> > Instead of returning early for port A, this will go ahead and attach the
> > HDMI encoder to the connector.
> 
> hmm! indeed!
> this also explains how I end up forgetting the PORT_A on v1...
> 
> > However, I don't know likely  we'll have
> > a vbt that says port A supports both DP and HDMI but not eDP.
> 
> If we have a VBT stating that or it is a current bug we have or if
> we have a VBT that lies to that point we have probably bigger issues,
> but I understand that so far this option is working stable out there and
> we don't want to take the risk, so what about a"
> 
> + if(WARN_ON(port == PORT_A)) return;
> + intel_encoder->hpd_pin = intel_hpd_pin(port);
> 

Looks good to me with this change
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>


> ?
> or just change the commit message explaining this case?
> 
> >
> >>
> >>       if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> >>               intel_hdmi->write_infoframe = vlv_write_infoframe;
> >> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> >> index 5982c47586e7..7b1cf30a3e9b 100644
> >> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> >> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> >> @@ -101,6 +101,32 @@ enum port intel_hpd_port(enum hpd_pin pin)
> >>       }
> >>  }
> >>
> >> +/**
> >> + * intel_hpd_pin - return pin hard associated with certain port.
> >> + * @port: the hpd port to get associated pin
> >> + *
> >> + * Return pin that is associatade with @port and HDP_NONE if no pin is
> >> + * hard associated with that @port.
> >> + */
> >> +enum hpd_pin intel_hpd_pin(enum port port)
> >> +{
> >> +     switch (port) {
> >> +     case PORT_A:
> >> +             return HPD_PORT_A;
> >> +     case PORT_B:
> >> +             return HPD_PORT_B;
> >> +     case PORT_C:
> >> +             return HPD_PORT_C;
> >> +     case PORT_D:
> >> +             return HPD_PORT_D;
> >> +     case PORT_E:
> >> +             return HPD_PORT_E;
> >> +     default:
> >> +             MISSING_CASE(port);
> >> +             return HPD_NONE;
> >> +     }
> >> +}
> >> +
> >>  #define HPD_STORM_DETECT_PERIOD              1000
> >>  #define HPD_STORM_REENABLE_DELAY     (2 * 60 * 1000)
> >>
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux