Re: [PATCH] drm/radeon: Fix Asus M2A-VM HDMI EDID error flooding problem

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 24, 2011 at 12:02 AM, Thomas Reim <thomas.reim@xxxxxxxxxx> wrote:
> Hi Alex,
>
> thank you for your feedback.
>
>> Thinking about it more, it might be cleaner to just make the extended
>> mode a flag, e.g.,
>> dret = radeon_ddc_probe_extended(radeon_connector,
>> radeon_connector->requires_extended_probe);
>> and handle the extended fetch in the same probe function.
>>
>
> OK. That makes also sense. I will adapt again the patch.
>
>> > +       /* RS690 HDMI DDC quirk:
>> > +        * Some integrated ATI Radeon chipset implementations (e. g.
>> > +        * Asus M2A-VM HDMI) indicate the availability of a DDC even
>> > +        * when there's no monitor connected to HDMI. For HDMI
>> > +        * connectors we check for the availability of EDID with
>> > +        * at least a correct EDID header and EDID version/revision
>> > +        * information. Only then, DDC is assumed to be available.
>> > +        * This prevents drm_get_edid() and drm_edid_block_valid() of
>> > +        * periodically dumping data and kernel errors into the logs
>> > +        * and onto the terminal, which would lead to an unacceptable
>> > +        * system behaviour */
>> > +       if (connector_type == DRM_MODE_CONNECTOR_HDMIA &&
>> > +               (rdev->family == CHIP_RS690 ||
>> > +                rdev->family == CHIP_RS740 ||
>> > +                rdev->family == CHIP_RV630))
>> This seems like an arbitrary selection of chips.  I haven't heard of
>> any problems related to ddc on rv630.  Also I think we should limit it
>> to the specific connector that is problematic rather than all hdmi
>> ports.  In the case of your board, it seems the hdmi port on the
>> add-in card is the only problematic one.  Lots of rs690 motherboards
>> have hdmi ports on the motherboard itself that work fine.  I'd prefer
>> to match based on the pci device and subsytem ids and the
>> supported_device and connector type.  See radeon_atom_apply_quirks()
>> in radeon_atombios.c for an example.  Something like:
>>
>> radeon_connector->requires_extended_probe =
>>         radeon_connector_needs_extended_probe(rdev, supported_dev,
>> connector_type);
>
> I've added RS740 because linux uses the same firmware and this chip was
> also part of the other patch you mentioned in your first e-mail. RV630
> was added because I checked freedesktop bug 31943. The problem described
> there is different from the one here, but I saw logs, when no monitor
> was connected, and for this situation the patch would help.

I'd rather add quirks on a case by case bases rather than speculating
on mailing list reports.

>
> With regard to moving away from connector type and chip family to pci
> devices I'm not really sure. I remember complaints from people a year
> ago, that used the RS690 on a laptop and had the same problem. I just
> can't find the related messages. Don't you believe that this could be to
> focused? Especially, if we compare patch
> http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=commitdiff;h=4a9a8b71e12d41abb71c4e741bff524f016cfef4?
>

We should probably revert or rework that patch when we apply yours
otherwise we may end up removing i2c buses unnecessarily in some
cases.  I think this is a better approach.

> I felt rather safe with the above approach, as nothing will go wrong, if
> we check the HDMIA type connectors also RS690 of another manufacturers.
> We just check for a valid first six bytes set of the EDID header now.

As I said above, lets just apply this to the specific board and
connector that is problematic.  I seems to only affect the hdmi port
on the add-in cards, so there's no need to penalize all hdmi ports.
If we get enough quirks down the road, we can make it a general rule.

>
>> > @@ -777,8 +780,17 @@ static int radeon_ddc_dump(struct drm_connector *connector)
>> >        if (!radeon_connector->ddc_bus)
>> >                return -1;
>> >        edid = drm_get_edid(connector, &radeon_connector->ddc_bus->adapter);
>> > +       /* Log EDID retrieval status here. In particular with regard to
>> > +        * connectors with requires_extended_probe flag set, that will prevent
>> > +        * function radeon_dvi_detect() to fetch EDID on this connector,
>> > +        * as long as there is no valid EDID header found */
>> >        if (edid) {
>> > +               DRM_INFO("Radeon display connector %s: Found valid EDID",
>> > +                               drm_get_connector_name(connector));
>> >                kfree(edid);
>> > +       } else {
>> > +               DRM_INFO("Radeon display connector %s: No monitor connected or invalid EDID",
>> > +                               drm_get_connector_name(connector));
>>
>> These will just spam the log as well.
>
> Here's the log:
> kernel: [   14.912386] [drm] Radeon Display Connectors
> kernel: [   14.912389] [drm] Connector 0:
> kernel: [   14.912391] [drm]   VGA
> kernel: [   14.912394] [drm]   DDC: 0x7e50 0x7e40 0x7e54 0x7e44 0x7e58
> 0x7e48 0x7e5c 0x7e4c
> kernel: [   14.912397] [drm]   Encoders:
> kernel: [   14.912398] [drm]     CRT1: INTERNAL_KLDSCP_DAC1
> kernel: [   14.912401] [drm] Connector 1:
> kernel: [   14.912402] [drm]   S-video
> kernel: [   14.912404] [drm]   Encoders:
> kernel: [   14.912405] [drm]     TV1: INTERNAL_KLDSCP_DAC1
> kernel: [   14.912407] [drm] Connector 2:
> kernel: [   14.912409] [drm]   HDMI-A
> kernel: [   14.912410] [drm]   HPD2
> kernel: [   14.912413] [drm]   DDC: 0x7e40 0x7e60 0x7e44 0x7e64 0x7e48
> 0x7e68 0x7e4c 0x7e6c
> kernel: [   14.912415] [drm]   Encoders:
> kernel: [   14.912417] [drm]     DFP2: INTERNAL_DDI
> kernel: [   14.912419] [drm] Connector 3:
> kernel: [   14.912421] [drm]   DVI-D
> kernel: [   14.912423] [drm]   DDC: 0x7e40 0x7e50 0x7e44 0x7e54 0x7e48
> 0x7e58 0x7e4c 0x7e5c
> kernel: [   14.912425] [drm]   Encoders:
> kernel: [   14.912427] [drm]     DFP3: INTERNAL_LVTM1
> kernel: [   15.071566] HDA Intel 0000:00:14.2: PCI INT A -> GSI 16
> (level, low) -> IRQ 16
> kernel: [   15.071645] HDA Intel 0000:00:14.2: irq 42 for MSI/MSI-X
> kernel: [   15.072678] [drm] Radeon display connector VGA-1: No display
> connected or invalid EDID
> kernel: [   15.470734] Raw EDID:
> kernel: [   15.470745] <3>00 00 00 00 00 00 00 00 00 00 00 07 00 00 00
> 00  ................
> kernel: [   15.470748] <3>00 00 00 00 00 00 00 00 00 00 07 00 00 00 00
> 00  ................
> kernel: [   15.470751] <3>00 00 00 00 00 00 00 00 00 00 7f 00 00 00 00
> 00  ................
> kernel: [   15.470754] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00  ................
> kernel: [   15.470757] <3>00 00 00 00 1f 00 00 00 00 00 00 00 00 00 00
> 00  ................
> kernel: [   15.470760] <3>00 00 00 00 00 07 00 00 00 00 00 7f 00 00 00
> 00  ................
> kernel: [   15.470762] <3>00 00 00 00 00 00 07 00 00 00 00 00 00 00 00
> 00  ................
> kernel: [   15.470765] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 01  ................
> kernel: [   15.470767]
> kernel: [   15.779568] Raw EDID:
> kernel: [   15.779578] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00  ................
> kernel: [   15.779581] <3>00 00 00 00 00 7f 00 00 00 00 03 00 00 00 00
> 00  ................
> kernel: [   15.779584] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00  ................
> kernel: [   15.779587] <3>00 00 00 00 ff 00 00 00 00 00 00 00 00 00 00
> 00  ................
> kernel: [   15.779590] <3>00 00 00 00 00 00 00 00 ff 00 00 00 00 00 00
> 00  ................
> kernel: [   15.779593] <3>00 00 00 00 00 00 00 00 00 01 00 00 00 00 00
> 00  ................
> kernel: [   15.779596] <3>00 00 00 7f 00 00 00 00 00 03 07 00 00 00 00
> 00  ................
> kernel: [   15.779598] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00  ................
> kernel: [   15.779600]
> kernel: [   16.151817] Raw EDID:
> kernel: [   16.151827] <3>00 00 00 00 00 00 1f 00 00 00 00 00 00 00 00
> 00  ................
> kernel: [   16.151830] <3>00 00 00 00 00 00 00 00 00 01 00 00 00 00 00
> 00  ................
> kernel: [   16.151833] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00  ................
> kernel: [   16.151836] <3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00  ................
> kernel: [   16.151839] <3>00 00 1f 00 00 00 00 00 00 00 00 00 00 00 0f
> 00  ................
> kernel: [   16.151842] <3>01 00 00 00 00 00 00 00 01 00 00 00 00 07 00
> ff  ................
> kernel: [   16.151844] <3>00 00 00 00 00 00 ff 00 00 00 00 00 00 00 7f
> 00  ................
> kernel: [   16.151847] <3>00 0f 00 00 00 00 00 00 00 3f 00 00 00 00 00
> 00  .........?......
> kernel: [   16.151849]
> kernel: [   16.444209] Raw EDID:
> kernel: [   16.444220] <3>00 07 00 00 00 00 00 00 ff 00 00 00 00 ff 00
> 00  ................
> kernel: [   16.444223] <3>00 00 3f 00 00 00 00 00 00 00 00 00 00 ff 00
> 00  ..?.............
> kernel: [   16.444226] <3>00 00 00 00 00 00 00 00 00 07 00 00 00 01 0f
> 00  ................
> kernel: [   16.444229] <3>00 07 00 00 00 00 00 00 00 00 01 07 00 00 00
> 00  ................
> kernel: [   16.444231] <3>00 00 00 00 00 00 00 00 7f 00 00 00 00 1f 00
> 00  ................
> kernel: [   16.444234] <3>00 00 03 00 00 00 00 3f 00 03 00 00 00 00 00
> 00  .......?........
> kernel: [   16.444237] <3>7f 00 00 1f 00 00 00 00 00 00 00 00 0f 07 00
> 00  ................
> kernel: [   16.444240] <3>00 00 00 00 00 00 00 00 00 00 03 00 00 00 00
> 00  ................
> kernel: [   16.444242]
> kernel: [   16.444248] radeon 0000:01:05.0: HDMI-A-1: EDID block 0
> invalid.
> kernel: [   16.444252] [drm] Radeon display connector HDMI-A-1: No
> display connected or invalid EDID
> kernel: [   16.762734] [drm] Radeon display connector DVI-D-1: Found
> valid EDID
>
> Just one info entry per connector will be added during connector setup.
> The EDID dump was already there and comes from the DDC probing during
> connector setup. After that, there are no more entries in the log with
> regard to the buggy HDMI connector. From a user point of view I would
> prefer to have the two log entries:
> radeon 0000:01:05.0: HDMI-A-1: EDID block 0 invalid.
> [drm] Radeon display connector HDMI-A-1: No display connected or invalid
> EDID.
> One from the probing, which is a little bit cryptic, and the explaining
> one which prelude the silence for this connector.

ok, that makes sense.

>
>> > diff --git a/drivers/gpu/drm/radeon/radeon_i2c.c b/drivers/gpu/drm/radeon/radeon_i2c.c
>> > index 781196d..7e93cf9 100644
>> > --- a/drivers/gpu/drm/radeon/radeon_i2c.c
>> > +++ b/drivers/gpu/drm/radeon/radeon_i2c.c
>> > @@ -34,7 +34,7 @@
>> >  */
>> >  bool radeon_ddc_probe(struct radeon_connector *radeon_connector)
>> >  {
>> > -       u8 out_buf[] = { 0x0, 0x0};
>> > +       u8 out = 0x0;
>> >        u8 buf[2];
>> >        int ret;
>> >        struct i2c_msg msgs[] = {
>> > @@ -42,7 +42,7 @@ bool radeon_ddc_probe(struct radeon_connector *radeon_connector)
>> >                        .addr = 0x50,
>> >                        .flags = 0,
>> >                        .len = 1,
>> > -                       .buf = out_buf,
>> > +                       .buf = &out,
>> >                },
>> >                {
>> >                        .addr = 0x50,
>>
>>
>> The change above doesn't seem to be related.
>
> This was a comment from Jean who complained about the ineffective usage
> of the i2c bus. But I can also restore the old code. What's your
> preference?

Ah, I missed that.  Let's make that a separate patch, or fix it when
you add support for the extended edid check.

Thanks for fixing this up.

Alex

>
> Best regards
>
> Thomas
>
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux