| Message ID | 1358172910-7820-3-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 --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);