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);
?
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
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
