From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 2/5] drm/i915: Add has_hdmi_monitor to intel_hdmi
Date: Mon, 14 Jan 2013 17:25:01 +0200 [thread overview]
Message-ID: <20130114152501.GA3489@intel.com> (raw)
In-Reply-To: <20130114150939.GA9361@phenom.ffwll.local>
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
--
Ville Syrjälä
Intel OTC
next prev parent reply other threads:[~2013-01-14 15:25 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-14 14:15 drm/i915: RGB quantization range stuff ville.syrjala
2013-01-14 14:15 ` [PATCH v3 1/5] drm/i915: Fix RGB color range property for PCH platforms ville.syrjala
2013-01-14 14:15 ` [PATCH 2/5] drm/i915: Add has_hdmi_monitor to intel_hdmi ville.syrjala
2013-01-14 15:09 ` Daniel Vetter
2013-01-14 15:25 ` Ville Syrjälä [this message]
2013-01-14 14:15 ` [PATCH 3/5] drm/i915: Add "Automatic" mode for the "Broadcast RGB" property ville.syrjala
2013-01-14 14:15 ` [PATCH 4/5] drm/edid: Add drm_rgb_quant_range_selectable() ville.syrjala
2013-01-14 14:15 ` [PATCH 5/5] drm/i915: Provide the quantization range in the AVI infoframe ville.syrjala
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130114152501.GA3489@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).