From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [Intel-gfx] [PATCH 1/4] drm/edid: add helper function to detect monitor audio capability Date: Sun, 19 Sep 2010 08:38:07 +0100 Message-ID: <89k77n$p02t9j@fmsmga001.fm.intel.com> References: <1284879129-19720-1-git-send-email-zhenyuw@linux.intel.com> <1284879129-19720-2-git-send-email-zhenyuw@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1284879129-19720-2-git-send-email-zhenyuw@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Zhenyu Wang , dri-devel@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org, fengguang.wu@intel.com List-Id: dri-devel@lists.freedesktop.org [Lets see if I have a working vpn connection today...] On Sun, 19 Sep 2010 14:52:06 +0800, Zhenyu Wang wrote: > To help to determine if digital display port needs to enable > audio output or not. This one adds a helper to get monitor's > audio capability via EDID CEA extension block. > > Tested-by: Wu Fengguang > Signed-off-by: Zhenyu Wang This should be cc'ed for Adam Jackson's attention as well. > --- > drivers/gpu/drm/drm_edid.c | 92 +++++++++++++++++++++++++++++++++++++------- > include/drm/drm_crtc.h | 1 + > 2 files changed, 79 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 96e9631..7f356af 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -1268,34 +1268,51 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, > } > > #define HDMI_IDENTIFIER 0x000C03 > +#define AUDIO_BLOCK 0x01 > #define VENDOR_BLOCK 0x03 > +#define EDID_BASIC_AUDIO (1 << 6) > + > /** > + * drm_detect_monitor_audio - check monitor audio capability > + * > + * Monitor should have CEA extension block. > + * If monitor has 'basic audio', but no CEA audio blocks, it's 'basic > + * audio' only. If there is any audio extension block and supported > + * audio format, assume at least 'basic audio' support, even if 'basic > + * audio' is not defined in EDID. > + * > + */ > +bool drm_detect_monitor_audio(struct edid *edid) drm_edid_has_monitor_audio()? That is a little more self-descriptive. (I also think drm_detect_hdmi_monitor is poorly named.) > +{ > + u8 *edid_ext; > + int i, j; > + bool has_audio = false; > + int start_offset, end_offset; > + > + edid_ext = drm_find_cea_extension(edid); > + if (!edid_ext) > + goto end; > + > + has_audio = ((edid_ext[3] & EDID_BASIC_AUDIO) != 0); Too many brackets do not lead to code clarity. ;-) > + > + if (has_audio) { The last time Adam had a chance to comment on this EDID check, he suggested that we cannot rely on has_audio being a reliable indicator of the sink's properties. If the EDID has no audio support, then return early, otherwise perform the secondary check that extension block contains audio data. -Chris -- Chris Wilson, Intel Open Source Technology Centre