Re: [PATCH 2/2] drm/i915: Ignore long hpds on eDP ports

[Todd Previte <tprevite@gmail.com>]

On 10/16/2014 10:46 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Turning vdd on/off can generate a long hpd pulse on eDP ports. In order
> to handle hpd we would need to turn on vdd to perform aux transfers.
> This would lead to an endless cycle of
> "vdd off -> long hpd -> vdd on -> detect -> vdd off -> ..."
>
> So ignore long hpd pulses on eDP ports. eDP panels should be physically
> tied to the machine anyway so they should not actually disappear and
> thus don't need long hpd handling. Short hpds are still needed for link
> re-train and whatnot so we can't just turn off the hpd interrupt
> entirely for eDP ports. Perhaps we could turn it off whenever the panel
> is disabled, but just ignoring the long hpd seems sufficient.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f07f02c..4455009 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4611,6 +4611,18 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>   	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP)
>   		intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT;
>   
> +	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
> +		/*
> +		 * vdd off can generate a long pulse on eDP which
> +		 * would require vdd on to handle it, and thus we
> +		 * would end up in an endless cycle of
> +		 * "vdd off -> long hpd -> vdd on -> detect -> vdd off -> ..."
> +		 */
> +		DRM_DEBUG_KMS("ignoring long hpd on eDP port %c\n",
> +			      port_name(intel_dig_port->port));
> +		return false;
> +	}
> +
>   	DRM_DEBUG_KMS("got hpd irq on port %c - %s\n",
>   		      port_name(intel_dig_port->port),
>   		      long_hpd ? "long" : "short");
I'm not sure that ignoring a long pulse is the best way to handle it. 
eDP does not appear to differentiate between short and long pulses per 
the specification (not to mention that HPD support for eDP is optional 
in the first place). It seems to me that it would probably be better to 
handle them as a normal (short) HPD pulse and just do the regular link 
maintenance stuff. As I said, the spec doesn't differentiate between the 
long and short pulses for eDP so it's a safer bet to handle them as a 
short pulse than to ignore them entirely.

-T
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx