* [PATCH v2] drm/i915: Enable SDVO hotplug interrupts for HDMI and DVI
@ 2011-09-20 10:45 Simon Farnsworth
2011-09-20 11:27 ` Chris Wilson
0 siblings, 1 reply; 3+ messages in thread
From: Simon Farnsworth @ 2011-09-20 10:45 UTC (permalink / raw)
To: Keith Packard; +Cc: intel-gfx
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 <ajax@redhat.com> on IRC.
Signed-off-by: Simon Farnsworth <simon.farnsworth@onelan.co.uk>
---
Changes from first version:
* Don't call intel_sdvo_supports_hotplug twice - pick up the setting
in connector->polled instead (suggested by Keith Packard in review).
* Change subject prefix to drm/i915
drivers/gpu/drm/i915/intel_drv.h | 3 --
drivers/gpu/drm/i915/intel_sdvo.c | 65 ++++++++++++++++---------------------
2 files changed, 28 insertions(+), 40 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..ebce2a5 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1208,74 +1208,55 @@ static bool intel_sdvo_get_capabilities(struct intel_sdvo *intel_sdvo, struct in
return true;
}
-/* No use! */
-#if 0
-struct drm_connector* intel_sdvo_find(struct drm_device *dev, int sdvoB)
+struct drm_connector* intel_sdvo_find_connector(struct intel_sdvo *sdvo)
{
struct drm_connector *connector = NULL;
struct intel_sdvo *iout = NULL;
- struct intel_sdvo *sdvo;
+ struct drm_device *dev;
+
+ dev = sdvo->base.base.dev;
/* find the sdvo connector */
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
- iout = to_intel_sdvo(connector);
-
- if (iout->type != INTEL_OUTPUT_SDVO)
- continue;
-
- sdvo = iout->dev_priv;
-
- if (sdvo->sdvo_reg == SDVOB && sdvoB)
- return connector;
+ iout = intel_attached_sdvo(connector);
- if (sdvo->sdvo_reg == SDVOC && !sdvoB)
+ if (iout == sdvo)
return connector;
-
}
return NULL;
}
-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 +2043,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;
@@ -2528,6 +2512,7 @@ bool intel_sdvo_init(struct drm_device *dev, int sdvo_reg)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_encoder *intel_encoder;
struct intel_sdvo *intel_sdvo;
+ struct drm_connector *connector;
int i;
intel_sdvo = kzalloc(sizeof(struct intel_sdvo), GFP_KERNEL);
@@ -2587,6 +2572,12 @@ bool intel_sdvo_init(struct drm_device *dev, int sdvo_reg)
&intel_sdvo->pixel_clock_max))
goto err;
+ connector = intel_sdvo_find_connector(intel_sdvo);
+ if (connector->polled == DRM_CONNECTOR_POLL_HPD) {
+ 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, "
--
1.7.6
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2] drm/i915: Enable SDVO hotplug interrupts for HDMI and DVI
2011-09-20 10:45 [PATCH v2] drm/i915: Enable SDVO hotplug interrupts for HDMI and DVI Simon Farnsworth
@ 2011-09-20 11:27 ` Chris Wilson
2011-09-20 12:46 ` Simon Farnsworth
0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2011-09-20 11:27 UTC (permalink / raw)
To: Simon Farnsworth, Keith Packard; +Cc: intel-gfx
On Tue, 20 Sep 2011 11:45:00 +0100, Simon Farnsworth <simon.farnsworth@onelan.co.uk> 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 <ajax@redhat.com> on IRC.
>
> Signed-off-by: Simon Farnsworth <simon.farnsworth@onelan.co.uk>
> ---
> Changes from first version:
>
> * Don't call intel_sdvo_supports_hotplug twice - pick up the setting
> in connector->polled instead (suggested by Keith Packard in review).
>
> * Change subject prefix to drm/i915
>
> drivers/gpu/drm/i915/intel_drv.h | 3 --
> drivers/gpu/drm/i915/intel_sdvo.c | 65 ++++++++++++++++---------------------
> 2 files changed, 28 insertions(+), 40 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..ebce2a5 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1208,74 +1208,55 @@ static bool intel_sdvo_get_capabilities(struct intel_sdvo *intel_sdvo, struct in
> return true;
> }
>
> -/* No use! */
> -#if 0
> -struct drm_connector* intel_sdvo_find(struct drm_device *dev, int sdvoB)
> +struct drm_connector* intel_sdvo_find_connector(struct intel_sdvo *sdvo)
> {
> struct drm_connector *connector = NULL;
> struct intel_sdvo *iout = NULL;
> - struct intel_sdvo *sdvo;
> + struct drm_device *dev;
> +
> + dev = sdvo->base.base.dev;
>
> /* find the sdvo connector */
> list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> - iout = to_intel_sdvo(connector);
> -
> - if (iout->type != INTEL_OUTPUT_SDVO)
> - continue;
> -
> - sdvo = iout->dev_priv;
> -
> - if (sdvo->sdvo_reg == SDVOB && sdvoB)
> - return connector;
> + iout = intel_attached_sdvo(connector);
>
> - if (sdvo->sdvo_reg == SDVOC && !sdvoB)
> + if (iout == sdvo)
Let's kill iout as it an ugly name and just do
if (intel_attached_encoder(connector) == &sdvo->base)
return connector;
instead
> return connector;
> -
> }
>
> return NULL;
> }
>
> -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)
We don't set here, so perhaps intel_sdvo_enable_hotplug()? And make it
take the connector so that we don't need the trivial function later, we
only incur the extra pointer dance during init.
> {
> 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)
Should be static, but anyways I'd recommend just skipping this helper.
Couldn't spot anything wrong with the logic, so just style nitpicking.
I'll have to fix the Ubuntu snafu that's preventing my one machine with
a hotpluggable SDVO device from booting before I can test. Also need to
find someone with the SDVO specs to see if there are any known caveats. :|
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] drm/i915: Enable SDVO hotplug interrupts for HDMI and DVI
2011-09-20 11:27 ` Chris Wilson
@ 2011-09-20 12:46 ` Simon Farnsworth
0 siblings, 0 replies; 3+ messages in thread
From: Simon Farnsworth @ 2011-09-20 12:46 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Tuesday 20 September 2011, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Couldn't spot anything wrong with the logic, so just style nitpicking.
> I'll have to fix the Ubuntu snafu that's preventing my one machine with
> a hotpluggable SDVO device from booting before I can test. Also need to
> find someone with the SDVO specs to see if there are any known caveats. :|
> -Chris
v3 patch on the way, with your style nitpicks fixed up. Just needs testing
before I send it to the list.
--
Simon Farnsworth
Software Engineer
ONELAN Limited
http://www.onelan.com/
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-09-20 12:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-20 10:45 [PATCH v2] drm/i915: Enable SDVO hotplug interrupts for HDMI and DVI Simon Farnsworth
2011-09-20 11:27 ` Chris Wilson
2011-09-20 12:46 ` Simon Farnsworth
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.