[Intel-gfx] [PATCH] drm/i915: Enable SDVO hotplug interrupts for HDMI and DVI

[Simon Farnsworth]

On Tuesday 20 September 2011, Keith Packard <keithp at keithp.com> wrote:
> On Tue, 20 Sep 2011 15:17:38 +0100, Simon Farnsworth 
<simon.farnsworth at onelan.co.uk> wrote:
> > -	if (on) {
> > -		intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_HOT_PLUG_SUPPORT, 
NULL,
> > 0); -		status = intel_sdvo_read_response(intel_sdvo, &response, 2);
> > +	intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_HOT_PLUG_SUPPORT, NULL,
> > 0); +	status = intel_sdvo_read_response(intel_sdvo, &response, 2);
> 
> ...
> 
> > +	intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_ACTIVE_HOT_PLUG,
> > &response, 2);
> 
> You are unconditionally enabling hotplug on all devices, I think you
> want to take the desired device as an argument to this function and add
> it to the set of active hotplug devices instead. You've just gotten the
> active hotplug value above, so removing the call to GET_HOT_PLUG_SUPPORT
> and doing:
> 
>   	intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_ACTIVE_HOT_PLUG, NULL, 0);
>   	intel_sdvo_read_response(intel_sdvo, &response, 2);
> 
>         response[0] |= (1 << device);
> 
> 	intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_ACTIVE_HOT_PLUG, &response,
> 2);
> 
> should do what you want.

This interacts badly with the need I've found to call this function every time 
a hotplug interrupt comes through (explained below) - I'm not sure how to 
track back from the encoder (which gets the callback) to the SDVO device 
number.

> 
> > @@ -2062,7 +2020,13 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo,
> > int device)
> > 
> >  	intel_connector = &intel_sdvo_connector->base;
> >  	connector = &intel_connector->base;
> > 
> > -	connector->polled = DRM_CONNECTOR_POLL_CONNECT |
> > DRM_CONNECTOR_POLL_DISCONNECT; +	if
> > (intel_sdvo_supports_hotplug(intel_sdvo) & (1 << device)) {
> > +		connector->polled = DRM_CONNECTOR_POLL_HPD;
> > +		intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
> 
> The encoder->hot_plug function is what is called when the hotplug
> interrupt is detected. The only current user is the DisplayPort driver
> which uses this to signal link retraining. You definitely don't want or
> need to call intel_sdvo_enable_hotplug at that point.

If I don't call intel_sdvo_enable_hotplug at this point, I only get the one 
hotplug interrupt; it appears that the SDVO devices I have send their 
interrupt, then expect you to re-enable HPD interrupts before they will send 
another interrupt.

I'm not sure how best to handle this; my gut feeling is that there's no real 
harm enabling HPD interrupts on all ports that support it, even though I'm 
only processing HPD interrupts from DVI and HDMI ports. If you have better 
ideas, I'm happy to try them.
-- 
Simon Farnsworth
Software Engineer
ONELAN Limited
http://www.onelan.com/