| Message ID | 503D9810.60104@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 28, 2012 at 11:18:24PM -0500, Ian Pilcher wrote: > On 08/28/2012 07:13 PM, Adam Jackson wrote: > > On 8/28/12 7:33 PM, Ian Pilcher wrote: > >> Actually, I believe that the error is probably in the Intel driver. As > >> I understand it, it shouldn't be sending audio InfoFrames to a non-HDMI > >> display. > > > > If that's the case then I'd still say "we're doing something else wrong > > here". Quirks - at least at the core drm level - are not for working > > around broken drivers, they're for working around broken displays. > > And I'd agree. > > (Although I suppose one *could* argue that the display is broken in 2 > ways -- it reports audio capabilities that aren't really there, and it > gets confused by any InfoFrames -- AVI or audio.) > > I don't have the knowledge or time to fix the Intel driver, but I've > always planned to at least bugzilla the issue. I can't reasonably do > so, however, until the user-defined quirks infrastructure is in place, > so that the behavior can be demonstrated. > > If you prefer to leave the display broken with Intel GPUs, you can > always just remove the EDID_QUIRK_NO_AUDIO flag: Wrt intel infoframes issues: Have you retested on latest 3.6-rc kernels? We've fixed quite a few bugs for our infoframe support recently ... -Daniel > > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -160,6 +160,10 @@ union edid_quirk > edid_quirk_list[EDID_QUIRK_LIST_SIZE] = { > { { { { EDID_MFG_ID('V', 'S', 'C'), cpu_to_le16(5020) } }, > EDID_QUIRK_FORCE_REDUCED_BLANKING } }, > > + /* LG L246WP */ > + { { { { EDID_MFG_ID('G', 'S', 'M'), cpu_to_le16(0x563f) } }, > + EDID_QUIRK_DISABLE_INFOFRAMES } }, > + > /* > * When adding built-in quirks, please adjust EDID_QUIRK_LIST_SIZE to > * provide some room for user-supplied quirks. > > -- > ======================================================================== > Ian Pilcher arequipeno@gmail.com > "If you're going to shift my paradigm ... at least buy me dinner first." > ======================================================================== > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 08/29/2012 02:56 AM, Daniel Vetter wrote: > Wrt intel infoframes issues: Have you retested on latest 3.6-rc kernels? > We've fixed quite a few bugs for our infoframe support recently ... Tested just now. The behavior has changed. I now need to use *both* flags to make my display work with an Intel GPU. (Previously, the NO_AUDIO flag was sufficient to make the display work with Intel graphics.) Presumably, the Intel driver is now sending AVI InfoFrames. Thinking a bit more about this, I'm starting to rethink my assertion that the Intel driver is at fault here. On one hand, it doesn't make any sense to send audio InfoFrames to a non-HDMI display. (BTW, I'm assuming that a display with a DisplayPort port will show up as HDMI.) On the other hand, it can be argued that the DRM layer is giving conflicting information to the driver -- drm_detect_hdmi_monitor is returning FALSE, but drm_detect_monitor_audio is returning TRUE. AFAIK, this doesn't make any sense either. This leads me to propose that drm_detect_monitor_audio return FALSE if either EDID_QUIRK_DISABLE_INFOFRAMES or EDID_QUIRK_NO_AUDIO is set. bool drm_detect_monitor_audio(struct edid *edid) { u8 *edid_ext; int i, j; bool has_audio = false; int start_offset, end_offset; char buf[EDID_DISPLAY_ID_BUF_SIZE]; if (edid_get_quirks(edid) & (EDID_QUIRK_NO_AUDIO | EDID_QUIRK_DISABLE_INFOFRAMES)) { DRM_INFO("Disabling HDMI audio on display %s " "due to EDID quirk\n", drm_edid_display_id_format(edid->display_id, buf, 1)); goto end; } ... EDID_QUIRK_DISABLE_INFOFRAMES would then be sufficient to make the LG L246WP work with both nVidia and Intel GPUs. (ATI GPUs should work as well; I just don't have any hardware to test.) Ajax - Does this address your objections? If so, I'll spin another patch set. Thanks!
On 8/29/12 3:53 PM, Ian Pilcher wrote: > Thinking a bit more about this, I'm starting to rethink my assertion > that the Intel driver is at fault here. On one hand, it doesn't make > any sense to send audio InfoFrames to a non-HDMI display. (BTW, I'm > assuming that a display with a DisplayPort port will show up as HDMI.) Nope. If you connect to the DP port on a monitor, it will claim to be DP: the EDID block version will be 1.4, and the digital display feature byte will indicate that it's DP: http://cgit.freedesktop.org/xorg/app/edid-decode/tree/edid-decode.c#n1145 If you connect to the HDMI port on a monitor, it will claim to be HDMI: the EDID block version will be 1.3 or higher, there will be a CEA extension block, and there will be a vendor-specific data block within that extension with a vendor OUI matching that of the HDMI consortium: http://cgit.freedesktop.org/xorg/app/edid-decode/tree/edid-decode.c#n640 > On the other hand, it can be argued that the DRM layer is giving > conflicting information to the driver -- drm_detect_hdmi_monitor is > returning FALSE, but drm_detect_monitor_audio is returning TRUE. AFAIK, > this doesn't make any sense either. This might be the actual issue. drm_detect_monitor_audio() can return true for several reasons, but it does _not_ make any claim that the monitor is HDMI, only that it has a CEA extension block. Which is certainly ugly, but that's how CEA defined their botch of an extension block. So to me, that says that drivers need to ask _both_ whether the monitor supports audio and whether it's HDMI before sending HDMI-specific audio signalling. - ajax
On 08/29/2012 04:38 PM, Adam Jackson wrote: > So to me, that says that drivers need to ask _both_ whether the monitor > supports audio and whether it's HDMI before sending HDMI-specific audio > signalling. OK, so the burden is on the individual drivers. In terms of moving forward with the rest of the EDID quirk stuff, are you OK with either of these options: * Remove EDID_QUIRK_NO_AUDIO from the flags for the LG L246WP. It won't work "out of the box" with the Intel driver until that driver is fixed to not send audio InfoFrames to non-HDMI displays (but anyone who has the combination will be able to add their own quirk to make it work). * Leave both flags, because both are accurate. The display is confused by any InfoFrames (audio or AVI), and it is reporting non-existent audio capabilities. The fact that the combination of the two flags makes the display work with Intel GPUs is a happy/sad/neutral accident, and the driver's behavior is still considered broken. (Essentially, this would mean just editing the comments to reflect the above reasoning.) If needed, I can easily create a new patch for either option. Just let me know. Thanks!
On Thu, Aug 30, 2012 at 12:23:43AM -0500, Ian Pilcher wrote: > On 08/29/2012 04:38 PM, Adam Jackson wrote: > > So to me, that says that drivers need to ask _both_ whether the monitor > > supports audio and whether it's HDMI before sending HDMI-specific audio > > signalling. > > OK, so the burden is on the individual drivers. > > In terms of moving forward with the rest of the EDID quirk stuff, are > you OK with either of these options: > > * Remove EDID_QUIRK_NO_AUDIO from the flags for the LG L246WP. It won't > work "out of the box" with the Intel driver until that driver is > fixed to not send audio InfoFrames to non-HDMI displays (but anyone > who has the combination will be able to add their own quirk to make > it work). > > * Leave both flags, because both are accurate. The display is confused > by any InfoFrames (audio or AVI), and it is reporting non-existent > audio capabilities. The fact that the combination of the two flags > makes the display work with Intel GPUs is a happy/sad/neutral > accident, and the driver's behavior is still considered broken. > > (Essentially, this would mean just editing the comments to reflect the > above reasoning.) > > If needed, I can easily create a new patch for either option. Just let > me know. I've decided that simply fixing the intel driver is easier. Patch on it's way, please test. Thanks, Daniel
On 08/30/2012 12:23 AM, Ian Pilcher wrote: > > * Remove EDID_QUIRK_NO_AUDIO from the flags for the LG L246WP. It won't > work "out of the box" with the Intel driver until that driver is > fixed to not send audio InfoFrames to non-HDMI displays (but anyone > who has the combination will be able to add their own quirk to make > it work). > > * Leave both flags, because both are accurate. The display is confused > by any InfoFrames (audio or AVI), and it is reporting non-existent > audio capabilities. The fact that the combination of the two flags > makes the display work with Intel GPUs is a happy/sad/neutral > accident, and the driver's behavior is still considered broken. > > (Essentially, this would mean just editing the comments to reflect the > above reasoning.) Ajax - I *really* want to close the loop on this. If you can tell me which of the above options you prefer, I will create a new patch set, including the drm_monitor_has_hdmi_audio() function that you suggested in your note on 8/30. Thanks!
Ping On 09/04/2012 09:02 AM, Ian Pilcher wrote: > On 08/30/2012 12:23 AM, Ian Pilcher wrote: >> >> * Remove EDID_QUIRK_NO_AUDIO from the flags for the LG L246WP. It won't >> work "out of the box" with the Intel driver until that driver is >> fixed to not send audio InfoFrames to non-HDMI displays (but anyone >> who has the combination will be able to add their own quirk to make >> it work). >> >> * Leave both flags, because both are accurate. The display is confused >> by any InfoFrames (audio or AVI), and it is reporting non-existent >> audio capabilities. The fact that the combination of the two flags >> makes the display work with Intel GPUs is a happy/sad/neutral >> accident, and the driver's behavior is still considered broken. >> >> (Essentially, this would mean just editing the comments to reflect the >> above reasoning.) > > Ajax - > > I *really* want to close the loop on this. If you can tell me which of > the above options you prefer, I will create a new patch set, including > the drm_monitor_has_hdmi_audio() function that you suggested in your > note on 8/30. > > Thanks! >
--- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -160,6 +160,10 @@ union edid_quirk edid_quirk_list[EDID_QUIRK_LIST_SIZE] = { { { { { EDID_MFG_ID('V', 'S', 'C'), cpu_to_le16(5020) } }, EDID_QUIRK_FORCE_REDUCED_BLANKING } }, + /* LG L246WP */ + { { { { EDID_MFG_ID('G', 'S', 'M'), cpu_to_le16(0x563f) } }, + EDID_QUIRK_DISABLE_INFOFRAMES } }, + /* * When adding built-in quirks, please adjust EDID_QUIRK_LIST_SIZE to