From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Farnsworth Subject: Re: [PATCH] i915: Enable SDVO hotplug interrupts for HDMI and DVI Date: Mon, 19 Sep 2011 10:41:27 +0100 Message-ID: <201109191041.27317.simon.farnsworth@onelan.co.uk> References: <1313574741-31193-1-git-send-email-simon.farnsworth@onelan.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from claranet-outbound-smtp06.uk.clara.net (claranet-outbound-smtp06.uk.clara.net [195.8.89.39]) by gabe.freedesktop.org (Postfix) with ESMTP id 704ED9EEF5 for ; Mon, 19 Sep 2011 02:41:34 -0700 (PDT) In-Reply-To: <1313574741-31193-1-git-send-email-simon.farnsworth@onelan.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Keith Packard Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org Hello Keith, I've not seen any replies to this patch in the last month, nor can I find it in your tree at http://cgit.freedesktop.org/~keithp/linux/ (I'd check kernel.org as well, but it's still down). Is there anything I need to do to make this patch acceptable? Or is it just a case of waiting for you to catch up on post-XDC madness? -- Simon On Wednesday 17 August 2011, Simon Farnsworth wrote: > I was seeing a nasty 5 frame glitch every 10 seconds, caused by the > poll for connection on DVI attached by SDVO. > > As my SDVO DVI supports hotplug detect interrupts, the fix is to > enable them, and hook them in to the various bits of driver > infrastructure so that they work reliably. > > With lots of help from Adam Jackson on IRC. > > Signed-off-by: Simon Farnsworth > --- > I'm not entirely confident that I'm treating the result of > SDVO_CMD_GET_HOT_PLUG_SUPPORT correctly for multifunction SDVO > devices. I've assumed it's a bitmask, but I have no documents to tell > me that that's right. Review from someone who understands SDVO would > be appreciated. > > Note that I've only wired up hotplug fully for SDVO TMDS > outputs. While we will get hotplug interrupts for other SDVO types, we > won't do anything with them; if someone with another SDVO type wants > to wire it up and test it, look at how intel_sdvo_dvi_init sets > connector->polled. > > However, this does work well for my hardware, which is all > single-function SDVO DVI-D ports, both on Atom N270 and 965GME. > > drivers/gpu/drm/i915/intel_drv.h | 3 -- > drivers/gpu/drm/i915/intel_sdvo.c | 43 > ++++++++++++++++++------------------- 2 files changed, 21 insertions(+), > 25 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h index 7b330e7..94bb596 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -336,9 +336,6 @@ extern void intel_release_load_detect_pipe(struct > intel_encoder *intel_encoder, struct drm_connector *connector, > struct intel_load_detect_pipe *old); > > -extern struct drm_connector* intel_sdvo_find(struct drm_device *dev, int > sdvoB); -extern int intel_sdvo_supports_hotplug(struct drm_connector > *connector); -extern void intel_sdvo_set_hotplug(struct drm_connector > *connector, int enable); extern void intelfb_restore(void); > extern void intel_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 > green, u16 blue, int regno); > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c > b/drivers/gpu/drm/i915/intel_sdvo.c index 30fe554..9404388 100644 > --- a/drivers/gpu/drm/i915/intel_sdvo.c > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > @@ -1235,47 +1235,38 @@ struct drm_connector* intel_sdvo_find(struct > drm_device *dev, int sdvoB) > > return NULL; > } > +#endif > > -int intel_sdvo_supports_hotplug(struct drm_connector *connector) > +static int intel_sdvo_supports_hotplug(struct intel_sdvo *intel_sdvo) > { > u8 response[2]; > - u8 status; > - struct intel_sdvo *intel_sdvo; > - DRM_DEBUG_KMS("\n"); > - > - if (!connector) > - return 0; > - > - intel_sdvo = to_intel_sdvo(connector); > > return intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_HOT_PLUG_SUPPORT, > &response, 2) && response[0]; > } > > -void intel_sdvo_set_hotplug(struct drm_connector *connector, int on) > +static void intel_sdvo_set_hotplug(struct intel_sdvo *intel_sdvo) > { > u8 response[2]; > u8 status; > - struct intel_sdvo *intel_sdvo = to_intel_sdvo(connector); > > intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_ACTIVE_HOT_PLUG, NULL, 0); > intel_sdvo_read_response(intel_sdvo, &response, 2); > > - 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); - } else { > - response[0] = 0; > - response[1] = 0; > - intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_ACTIVE_HOT_PLUG, > &response, 2); - } > + intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_ACTIVE_HOT_PLUG, &response, > 2); > > intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_ACTIVE_HOT_PLUG, NULL, 0); > intel_sdvo_read_response(intel_sdvo, &response, 2); > } > -#endif > + > +void intel_sdvo_do_hotplug(struct intel_encoder *encoder) > +{ > + struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base); > + intel_sdvo_set_hotplug(intel_sdvo); > +} > > static bool > intel_sdvo_multifunc_encoder(struct intel_sdvo *intel_sdvo) > @@ -2062,7 +2053,10 @@ 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; > + else > + connector->polled = DRM_CONNECTOR_POLL_CONNECT | > DRM_CONNECTOR_POLL_DISCONNECT; encoder->encoder_type = > DRM_MODE_ENCODER_TMDS; > connector->connector_type = DRM_MODE_CONNECTOR_DVID; > > @@ -2587,6 +2581,11 @@ bool intel_sdvo_init(struct drm_device *dev, int > sdvo_reg) &intel_sdvo->pixel_clock_max)) > goto err; > > + if (intel_sdvo_supports_hotplug(intel_sdvo)) { > + intel_encoder->hot_plug = intel_sdvo_do_hotplug; > + intel_sdvo_set_hotplug(intel_sdvo); > + } > + > DRM_DEBUG_KMS("%s device VID/DID: %02X:%02X.%02X, " > "clock range %dMHz - %dMHz, " > "input 1: %c, input 2: %c, " -- Simon Farnsworth Software Engineer ONELAN Limited http://www.onelan.com/