diff mbox

[2/5] drm/i915: Add has_hdmi_monitor to intel_hdmi

Message ID 1358172910-7820-3-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Jan. 14, 2013, 2:15 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

intel_hdmi.has_hdmi_sink is tied into the force audio property, so it
doesn't seem like the correct way to detect HDMI monitors. Instead add
a new has_hdmi_monitor member which simply tells whether a HDMI monitor
was detected.

Not sure if this is the best way, or if we should just use has_hdmi_sink
for the CEA-861 automatic color range property... At least the infoframe
stuff already depends on has_hdmi_sink.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h  |    1 +
 drivers/gpu/drm/i915/intel_hdmi.c |    6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

Comments

Daniel Vetter Jan. 14, 2013, 3:09 p.m. UTC | #1
On Mon, Jan 14, 2013 at 04:15:07PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> intel_hdmi.has_hdmi_sink is tied into the force audio property, so it
> doesn't seem like the correct way to detect HDMI monitors. Instead add
> a new has_hdmi_monitor member which simply tells whether a HDMI monitor
> was detected.
> 
> Not sure if this is the best way, or if we should just use has_hdmi_sink
> for the CEA-861 automatic color range property... At least the infoframe
> stuff already depends on has_hdmi_sink.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Imo it looks simpler if we just reuse the has_hdmi_sink bool. That already
controls infoframes and similar stuff, so if users would force dvi mode
this would automatically also disable all broadcast range magic (if that's
in the default value). Users could still override the broadcast stuff
explictly.

I guess it'd have been better if we have a a force-dvi master switch
outside of the audio property to control all things hdmi. But alas ...
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_drv.h  |    1 +
>  drivers/gpu/drm/i915/intel_hdmi.c |    6 ++++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4df47be..aed7478 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -344,6 +344,7 @@ struct intel_hdmi {
>  	int ddc_bus;
>  	uint32_t color_range;
>  	bool has_hdmi_sink;
> +	bool has_hdmi_monitor;
>  	bool has_audio;
>  	enum hdmi_force_audio force_audio;
>  	void (*write_infoframe)(struct drm_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index f194d75..e663dec 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -815,6 +815,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>  		 return status;
>  
>  	intel_hdmi->has_hdmi_sink = false;
> +	intel_hdmi->has_hdmi_monitor = false;
>  	intel_hdmi->has_audio = false;
>  	edid = drm_get_edid(connector,
>  			    intel_gmbus_get_adapter(dev_priv,
> @@ -823,9 +824,10 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>  	if (edid) {
>  		if (edid->input & DRM_EDID_INPUT_DIGITAL) {
>  			status = connector_status_connected;
> +			intel_hdmi->has_hdmi_monitor =
> +				drm_detect_hdmi_monitor(edid);
>  			if (intel_hdmi->force_audio != HDMI_AUDIO_OFF_DVI)
> -				intel_hdmi->has_hdmi_sink =
> -						drm_detect_hdmi_monitor(edid);
> +				intel_hdmi->has_hdmi_sink = intel_hdmi->has_hdmi_monitor;
>  			intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
>  		}
>  		kfree(edid);
> -- 
> 1.7.8.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Jan. 14, 2013, 3:25 p.m. UTC | #2
On Mon, Jan 14, 2013 at 04:09:39PM +0100, Daniel Vetter wrote:
> On Mon, Jan 14, 2013 at 04:15:07PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > intel_hdmi.has_hdmi_sink is tied into the force audio property, so it
> > doesn't seem like the correct way to detect HDMI monitors. Instead add
> > a new has_hdmi_monitor member which simply tells whether a HDMI monitor
> > was detected.
> > 
> > Not sure if this is the best way, or if we should just use has_hdmi_sink
> > for the CEA-861 automatic color range property... At least the infoframe
> > stuff already depends on has_hdmi_sink.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Imo it looks simpler if we just reuse the has_hdmi_sink bool. That already
> controls infoframes and similar stuff, so if users would force dvi mode
> this would automatically also disable all broadcast range magic (if that's
> in the default value). Users could still override the broadcast stuff
> explictly.

OK. I can do that.

> I guess it'd have been better if we have a a force-dvi master switch
> outside of the audio property to control all things hdmi. But alas
> ...

Yeah it feels a bit weird that some audio prop controls it all.

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_drv.h  |    1 +
> >  drivers/gpu/drm/i915/intel_hdmi.c |    6 ++++--
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 4df47be..aed7478 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -344,6 +344,7 @@ struct intel_hdmi {
> >  	int ddc_bus;
> >  	uint32_t color_range;
> >  	bool has_hdmi_sink;
> > +	bool has_hdmi_monitor;
> >  	bool has_audio;
> >  	enum hdmi_force_audio force_audio;
> >  	void (*write_infoframe)(struct drm_encoder *encoder,
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index f194d75..e663dec 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -815,6 +815,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >  		 return status;
> >  
> >  	intel_hdmi->has_hdmi_sink = false;
> > +	intel_hdmi->has_hdmi_monitor = false;
> >  	intel_hdmi->has_audio = false;
> >  	edid = drm_get_edid(connector,
> >  			    intel_gmbus_get_adapter(dev_priv,
> > @@ -823,9 +824,10 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >  	if (edid) {
> >  		if (edid->input & DRM_EDID_INPUT_DIGITAL) {
> >  			status = connector_status_connected;
> > +			intel_hdmi->has_hdmi_monitor =
> > +				drm_detect_hdmi_monitor(edid);
> >  			if (intel_hdmi->force_audio != HDMI_AUDIO_OFF_DVI)
> > -				intel_hdmi->has_hdmi_sink =
> > -						drm_detect_hdmi_monitor(edid);
> > +				intel_hdmi->has_hdmi_sink = intel_hdmi->has_hdmi_monitor;
> >  			intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
> >  		}
> >  		kfree(edid);
> > -- 
> > 1.7.8.6
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4df47be..aed7478 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -344,6 +344,7 @@  struct intel_hdmi {
 	int ddc_bus;
 	uint32_t color_range;
 	bool has_hdmi_sink;
+	bool has_hdmi_monitor;
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
 	void (*write_infoframe)(struct drm_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index f194d75..e663dec 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -815,6 +815,7 @@  intel_hdmi_detect(struct drm_connector *connector, bool force)
 		 return status;
 
 	intel_hdmi->has_hdmi_sink = false;
+	intel_hdmi->has_hdmi_monitor = false;
 	intel_hdmi->has_audio = false;
 	edid = drm_get_edid(connector,
 			    intel_gmbus_get_adapter(dev_priv,
@@ -823,9 +824,10 @@  intel_hdmi_detect(struct drm_connector *connector, bool force)
 	if (edid) {
 		if (edid->input & DRM_EDID_INPUT_DIGITAL) {
 			status = connector_status_connected;
+			intel_hdmi->has_hdmi_monitor =
+				drm_detect_hdmi_monitor(edid);
 			if (intel_hdmi->force_audio != HDMI_AUDIO_OFF_DVI)
-				intel_hdmi->has_hdmi_sink =
-						drm_detect_hdmi_monitor(edid);
+				intel_hdmi->has_hdmi_sink = intel_hdmi->has_hdmi_monitor;
 			intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
 		}
 		kfree(edid);