[PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
On Wed, Nov 30, 2011 at 5:54 AM, Thomas Reim wrote: > Am Dienstag, den 29.11.2011, 15:53 -0500 schrieb Alex Deucher: >> On Tue, Nov 29, 2011 at 1:06 PM, Thomas Reim >> wrote: >> > Dear Alex, >> > >> >> On Mon, Nov 28, 2011 at 11:20 AM, Thomas Reim >> >> wrote: >> >> > ? ? ? ?Extended DDC probe is now default for RADEON chipsets. In case of >> >> > ? ? ? ?HW bugs (e. g. floating connectors), the affected connectors will >> >> > ? ? ? ?not be used, as a valid EDID header can not be detected. Another >> >> > ? ? ? ?patch removed DDC detection and connector status logging during >> >> > ? ? ? ?device setup. So the user is not informed anymore about HW bugs >> >> > ? ? ? ?leading to connectors being unavailable. >> >> > ? ? ? ?Reintroduce one-time logging of connector unavailability status >> >> > ? ? ? ?when probing (single) connector modes during framebuffer >> >> > ? ? ? ?initialisation. >> >> > >> >> > ? ? ? ?For RS690 chipsets DDC detection shall be stopped, if the i2c bus >> >> > ? ? ? ?receives all-0 EDIDs (floating connectors). The introduction of >> >> > ? ? ? ?extended DDC probing prevents the driver from doing this. >> >> > Correctly >> >> > ? ? ? ?relocate the related code. >> >> >> >> Is any of this needed anymore now that we do an extended probe? ?I >> >> think the original null edid patch was just papering over the actual >> >> issue which was the need for a full edid header probe. >> > >> > The difference, at least this is my understanding of Dave's patch, is >> > that we completely stop DDC detection for the famous RS690/RS740 chips >> > with missing or disabled HDMI add-on cards. We decrease latency, as we >> > do not retrieve data from i2c bus anymore. Therefore, I would keep it. >> > >> >> My only concern is that we may disable the i2c bus if there is no >> monitor connected which would prevent edid fetching from working in >> the future. ?Does the problematic ddia i2c line produce the same error >> in the following cases: >> > > The proposed solution differentiates between i2c bus responds, i. e. a > DDC is available, and EDID (header) is valid. Right, but my concern is that if we remove the i2c bus, we no longer get DDC. > >> 1. ddia hdmi connector present, no monitor attached > > DDC is not available: The connector is regarded as being disconnected; > function drm_helper_probe_single_connector_modes() records a debug > message in the logs, i. e. "HDMI-A-1 is disconnected". > > The connector will be probed again, when probing single connector modes > (using function drm_helper_probe_single_connector_modes() about every > second). It's every 5 seconds for polled connectors. If the OEM implements an HPD pin for the connector, it's interrupt driven, so no polling occurs. We get an interrupt when a monitor is connected. > >> 2. ddia hdmi connector present, monitor with faulty edid attached >> > > DDC is available, invalid EDID (header): The connector is regarded as > being disconnected; If it's the first time, that this connector has been > probed, usually during framebuffer initialisation, dump the faulty EDID > to the logs and add an info message, that there is a problem with the > monitor: e. g. "HDMI-A-1: probed a monitor, DDC responded but no|invalid > EDID". In addition, the debug message of case 1 is logged. > > Then we have two subcases: > a) General > The connector will be probed again, when probing single connector modes > (using function drm_helper_probe_single_connector_modes() about every > second). But there will be only the debug message logged. No info > message and no EDID dump. > b) EDID is all zero, i. e. not more than 8 non-zero bytes have been > received (RS690/RS740 chips only) > Floating connector is assumed and an info message is logged: "HDMI-A-1: > detected RS690 floating bus bug, stopping ddc detect". Such a connector > will not be probed again, reducing latency. I believe that this is > rather a theoretical case, as it requires a monitor with an erased > EEPROM. > >> 3. ddia hdmi connector absent > a) Correctly implemented HW > would handle this in the same way as for case 1. The connector is part > of the chipset, i2c bus is correctly terminated and will never be used, > but being probed. This is a drawback of implmentations, that use a > chipset wich provides more connectors than being used by the > implementation. > b) Buggy HW > The connector's i2c has not been terminated correctly. It can't be used, > but is detected. The EDID information will mainly consist of zero bytes, > except of some random byte values. > > DDC is available, invalid EDID (header): The connector is regarded as > being disconnected; If it's the first time, that this connector has been > probed, usually during framebuffer initialisation, dump the received > information > of the not available EDID to the logs and add an info message, that > there is a problem with the connector: e. g. "HDMI-A-1: probed a > monitor, DDC responded but no|invalid EDID". In addition, the debug >
Re: [PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
On Wed, Nov 30, 2011 at 5:54 AM, Thomas Reim rei...@googlemail.com wrote: Am Dienstag, den 29.11.2011, 15:53 -0500 schrieb Alex Deucher: On Tue, Nov 29, 2011 at 1:06 PM, Thomas Reim rei...@googlemail.com wrote: Dear Alex, On Mon, Nov 28, 2011 at 11:20 AM, Thomas Reim rei...@googlemail.com wrote: Extended DDC probe is now default for RADEON chipsets. In case of HW bugs (e. g. floating connectors), the affected connectors will not be used, as a valid EDID header can not be detected. Another patch removed DDC detection and connector status logging during device setup. So the user is not informed anymore about HW bugs leading to connectors being unavailable. Reintroduce one-time logging of connector unavailability status when probing (single) connector modes during framebuffer initialisation. For RS690 chipsets DDC detection shall be stopped, if the i2c bus receives all-0 EDIDs (floating connectors). The introduction of extended DDC probing prevents the driver from doing this. Correctly relocate the related code. Is any of this needed anymore now that we do an extended probe? I think the original null edid patch was just papering over the actual issue which was the need for a full edid header probe. The difference, at least this is my understanding of Dave's patch, is that we completely stop DDC detection for the famous RS690/RS740 chips with missing or disabled HDMI add-on cards. We decrease latency, as we do not retrieve data from i2c bus anymore. Therefore, I would keep it. My only concern is that we may disable the i2c bus if there is no monitor connected which would prevent edid fetching from working in the future. Does the problematic ddia i2c line produce the same error in the following cases: The proposed solution differentiates between i2c bus responds, i. e. a DDC is available, and EDID (header) is valid. Right, but my concern is that if we remove the i2c bus, we no longer get DDC. 1. ddia hdmi connector present, no monitor attached DDC is not available: The connector is regarded as being disconnected; function drm_helper_probe_single_connector_modes() records a debug message in the logs, i. e. HDMI-A-1 is disconnected. The connector will be probed again, when probing single connector modes (using function drm_helper_probe_single_connector_modes() about every second). It's every 5 seconds for polled connectors. If the OEM implements an HPD pin for the connector, it's interrupt driven, so no polling occurs. We get an interrupt when a monitor is connected. 2. ddia hdmi connector present, monitor with faulty edid attached DDC is available, invalid EDID (header): The connector is regarded as being disconnected; If it's the first time, that this connector has been probed, usually during framebuffer initialisation, dump the faulty EDID to the logs and add an info message, that there is a problem with the monitor: e. g. HDMI-A-1: probed a monitor, DDC responded but no|invalid EDID. In addition, the debug message of case 1 is logged. Then we have two subcases: a) General The connector will be probed again, when probing single connector modes (using function drm_helper_probe_single_connector_modes() about every second). But there will be only the debug message logged. No info message and no EDID dump. b) EDID is all zero, i. e. not more than 8 non-zero bytes have been received (RS690/RS740 chips only) Floating connector is assumed and an info message is logged: HDMI-A-1: detected RS690 floating bus bug, stopping ddc detect. Such a connector will not be probed again, reducing latency. I believe that this is rather a theoretical case, as it requires a monitor with an erased EEPROM. 3. ddia hdmi connector absent a) Correctly implemented HW would handle this in the same way as for case 1. The connector is part of the chipset, i2c bus is correctly terminated and will never be used, but being probed. This is a drawback of implmentations, that use a chipset wich provides more connectors than being used by the implementation. b) Buggy HW The connector's i2c has not been terminated correctly. It can't be used, but is detected. The EDID information will mainly consist of zero bytes, except of some random byte values. DDC is available, invalid EDID (header): The connector is regarded as being disconnected; If it's the first time, that this connector has been probed, usually during framebuffer initialisation, dump the received information of the not available EDID to the logs and add an info message, that there is a problem with the connector: e. g. HDMI-A-1: probed a monitor, DDC responded but no|invalid EDID. In addition, the debug message of the first case is logged. Then we have two subcases: (1) RS690/RS740 chips Floating connector is assumed and an info message is logged:
[PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
Am Dienstag, den 29.11.2011, 15:53 -0500 schrieb Alex Deucher: > On Tue, Nov 29, 2011 at 1:06 PM, Thomas Reim wrote: > > Dear Alex, > > > >> On Mon, Nov 28, 2011 at 11:20 AM, Thomas Reim > >> wrote: > >> >Extended DDC probe is now default for RADEON chipsets. In case of > >> >HW bugs (e. g. floating connectors), the affected connectors will > >> >not be used, as a valid EDID header can not be detected. Another > >> >patch removed DDC detection and connector status logging during > >> >device setup. So the user is not informed anymore about HW bugs > >> >leading to connectors being unavailable. > >> >Reintroduce one-time logging of connector unavailability status > >> >when probing (single) connector modes during framebuffer > >> >initialisation. > >> > > >> >For RS690 chipsets DDC detection shall be stopped, if the i2c bus > >> >receives all-0 EDIDs (floating connectors). The introduction of > >> >extended DDC probing prevents the driver from doing this. > >> > Correctly > >> >relocate the related code. > >> > >> Is any of this needed anymore now that we do an extended probe? I > >> think the original null edid patch was just papering over the actual > >> issue which was the need for a full edid header probe. > > > > The difference, at least this is my understanding of Dave's patch, is > > that we completely stop DDC detection for the famous RS690/RS740 chips > > with missing or disabled HDMI add-on cards. We decrease latency, as we > > do not retrieve data from i2c bus anymore. Therefore, I would keep it. > > > > My only concern is that we may disable the i2c bus if there is no > monitor connected which would prevent edid fetching from working in > the future. Does the problematic ddia i2c line produce the same error > in the following cases: > The proposed solution differentiates between i2c bus responds, i. e. a DDC is available, and EDID (header) is valid. > 1. ddia hdmi connector present, no monitor attached DDC is not available: The connector is regarded as being disconnected; function drm_helper_probe_single_connector_modes() records a debug message in the logs, i. e. "HDMI-A-1 is disconnected". The connector will be probed again, when probing single connector modes (using function drm_helper_probe_single_connector_modes() about every second). > 2. ddia hdmi connector present, monitor with faulty edid attached > DDC is available, invalid EDID (header): The connector is regarded as being disconnected; If it's the first time, that this connector has been probed, usually during framebuffer initialisation, dump the faulty EDID to the logs and add an info message, that there is a problem with the monitor: e. g. "HDMI-A-1: probed a monitor, DDC responded but no|invalid EDID". In addition, the debug message of case 1 is logged. Then we have two subcases: a) General The connector will be probed again, when probing single connector modes (using function drm_helper_probe_single_connector_modes() about every second). But there will be only the debug message logged. No info message and no EDID dump. b) EDID is all zero, i. e. not more than 8 non-zero bytes have been received (RS690/RS740 chips only) Floating connector is assumed and an info message is logged: "HDMI-A-1: detected RS690 floating bus bug, stopping ddc detect". Such a connector will not be probed again, reducing latency. I believe that this is rather a theoretical case, as it requires a monitor with an erased EEPROM. > 3. ddia hdmi connector absent a) Correctly implemented HW would handle this in the same way as for case 1. The connector is part of the chipset, i2c bus is correctly terminated and will never be used, but being probed. This is a drawback of implmentations, that use a chipset wich provides more connectors than being used by the implementation. b) Buggy HW The connector's i2c has not been terminated correctly. It can't be used, but is detected. The EDID information will mainly consist of zero bytes, except of some random byte values. DDC is available, invalid EDID (header): The connector is regarded as being disconnected; If it's the first time, that this connector has been probed, usually during framebuffer initialisation, dump the received information of the not available EDID to the logs and add an info message, that there is a problem with the connector: e. g. "HDMI-A-1: probed a monitor, DDC responded but no|invalid EDID". In addition, the debug message of the first case is logged. Then we have two subcases: (1) RS690/RS740 chips Floating connector is assumed and an info message is logged: "HDMI-A-1: detected RS690 floating bus bug, stopping ddc detect". Such a connector will not be probed again, reducing latency. (2) Other chipsets The connector will be probed again, when probing single connector modes (using function drm_helper_probe_single_connector_modes() about every second). But there will be only a
Re: [PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
Am Dienstag, den 29.11.2011, 15:53 -0500 schrieb Alex Deucher: On Tue, Nov 29, 2011 at 1:06 PM, Thomas Reim rei...@googlemail.com wrote: Dear Alex, On Mon, Nov 28, 2011 at 11:20 AM, Thomas Reim rei...@googlemail.com wrote: Extended DDC probe is now default for RADEON chipsets. In case of HW bugs (e. g. floating connectors), the affected connectors will not be used, as a valid EDID header can not be detected. Another patch removed DDC detection and connector status logging during device setup. So the user is not informed anymore about HW bugs leading to connectors being unavailable. Reintroduce one-time logging of connector unavailability status when probing (single) connector modes during framebuffer initialisation. For RS690 chipsets DDC detection shall be stopped, if the i2c bus receives all-0 EDIDs (floating connectors). The introduction of extended DDC probing prevents the driver from doing this. Correctly relocate the related code. Is any of this needed anymore now that we do an extended probe? I think the original null edid patch was just papering over the actual issue which was the need for a full edid header probe. The difference, at least this is my understanding of Dave's patch, is that we completely stop DDC detection for the famous RS690/RS740 chips with missing or disabled HDMI add-on cards. We decrease latency, as we do not retrieve data from i2c bus anymore. Therefore, I would keep it. My only concern is that we may disable the i2c bus if there is no monitor connected which would prevent edid fetching from working in the future. Does the problematic ddia i2c line produce the same error in the following cases: The proposed solution differentiates between i2c bus responds, i. e. a DDC is available, and EDID (header) is valid. 1. ddia hdmi connector present, no monitor attached DDC is not available: The connector is regarded as being disconnected; function drm_helper_probe_single_connector_modes() records a debug message in the logs, i. e. HDMI-A-1 is disconnected. The connector will be probed again, when probing single connector modes (using function drm_helper_probe_single_connector_modes() about every second). 2. ddia hdmi connector present, monitor with faulty edid attached DDC is available, invalid EDID (header): The connector is regarded as being disconnected; If it's the first time, that this connector has been probed, usually during framebuffer initialisation, dump the faulty EDID to the logs and add an info message, that there is a problem with the monitor: e. g. HDMI-A-1: probed a monitor, DDC responded but no|invalid EDID. In addition, the debug message of case 1 is logged. Then we have two subcases: a) General The connector will be probed again, when probing single connector modes (using function drm_helper_probe_single_connector_modes() about every second). But there will be only the debug message logged. No info message and no EDID dump. b) EDID is all zero, i. e. not more than 8 non-zero bytes have been received (RS690/RS740 chips only) Floating connector is assumed and an info message is logged: HDMI-A-1: detected RS690 floating bus bug, stopping ddc detect. Such a connector will not be probed again, reducing latency. I believe that this is rather a theoretical case, as it requires a monitor with an erased EEPROM. 3. ddia hdmi connector absent a) Correctly implemented HW would handle this in the same way as for case 1. The connector is part of the chipset, i2c bus is correctly terminated and will never be used, but being probed. This is a drawback of implmentations, that use a chipset wich provides more connectors than being used by the implementation. b) Buggy HW The connector's i2c has not been terminated correctly. It can't be used, but is detected. The EDID information will mainly consist of zero bytes, except of some random byte values. DDC is available, invalid EDID (header): The connector is regarded as being disconnected; If it's the first time, that this connector has been probed, usually during framebuffer initialisation, dump the received information of the not available EDID to the logs and add an info message, that there is a problem with the connector: e. g. HDMI-A-1: probed a monitor, DDC responded but no|invalid EDID. In addition, the debug message of the first case is logged. Then we have two subcases: (1) RS690/RS740 chips Floating connector is assumed and an info message is logged: HDMI-A-1: detected RS690 floating bus bug, stopping ddc detect. Such a connector will not be probed again, reducing latency. (2) Other chipsets The connector will be probed again, when probing single connector modes (using function drm_helper_probe_single_connector_modes() about every second). But there will be only a debug message (see case 1) logged. No info message and EDID dump. In
[PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
Extended DDC probe is now default for RADEON chipsets. In case of
HW bugs (e. g. floating connectors), the affected connectors will
not be used, as a valid EDID header can not be detected. Another
patch removed DDC detection and connector status logging during
device setup. So the user is not informed anymore about HW bugs
leading to connectors being unavailable.
Reintroduce one-time logging of connector unavailability status
when probing (single) connector modes during framebuffer
initialisation.
For RS690 chipsets DDC detection shall be stopped, if the i2c bus
receives all-0 EDIDs (floating connectors). The introduction of
extended DDC probing prevents the driver from doing this. Correctly
relocate the related code.
Signed-off-by: Thomas Reim
---
drivers/gpu/drm/radeon/radeon_connectors.c | 54 +++-
drivers/gpu/drm/radeon/radeon_i2c.c|7 +++-
drivers/gpu/drm/radeon/radeon_mode.h |3 ++
3 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index e7cb3ab..b06480c 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -662,6 +662,7 @@ radeon_vga_detect(struct drm_connector *connector, bool
force)
struct radeon_connector *radeon_connector =
to_radeon_connector(connector);
struct drm_encoder *encoder;
struct drm_encoder_helper_funcs *encoder_funcs;
+ struct edid *edid = NULL;
bool dret = false;
enum drm_connector_status ret = connector_status_disconnected;
@@ -669,8 +670,24 @@ radeon_vga_detect(struct drm_connector *connector, bool
force)
if (!encoder)
ret = connector_status_disconnected;
- if (radeon_connector->ddc_bus)
+ if (radeon_connector->ddc_bus) {
dret = radeon_ddc_probe(radeon_connector);
+ if (!dret && radeon_connector->broken_edid_header_counter == 1)
{
+ /* Found a broken DDC when probing (single) connector
+ modes during framebuffer initialisation.
+ Clean-up and log the HW bug. */
+ edid = drm_get_edid(_connector->base,
_connector->ddc_bus->adapter);
+ if (!edid) {
+ radeon_connector->broken_edid_header_counter++;
+ DRM_INFO("%s: probed a monitor, DDC responded
but no|invalid EDID\n",
+ drm_get_connector_name(connector));
+ } else {
+ /* We should not get here, as the EDID header
is broken */
+ connector->display_info.raw_edid = NULL;
+ kfree(edid);
+ }
+ }
+ }
if (dret) {
radeon_connector->detected_by_load = false;
if (radeon_connector->edid) {
@@ -847,12 +864,38 @@ radeon_dvi_detect(struct drm_connector *connector, bool
force)
struct drm_encoder *encoder = NULL;
struct drm_encoder_helper_funcs *encoder_funcs;
struct drm_mode_object *obj;
+ struct edid *edid = NULL;
int i;
enum drm_connector_status ret = connector_status_disconnected;
bool dret = false;
- if (radeon_connector->ddc_bus)
+ if (radeon_connector->ddc_bus) {
dret = radeon_ddc_probe(radeon_connector);
+ if (!dret && radeon_connector->broken_edid_header_counter == 1)
{
+ /* Found a broken DDC when probing (single) connector
+ modes during framebuffer initialisation.
+ Clean-up and log the HW bug. */
+ edid = drm_get_edid(_connector->base,
_connector->ddc_bus->adapter);
+ if (!edid) {
+ radeon_connector->broken_edid_header_counter++;
+ DRM_INFO("%s: probed a monitor, DDC responded
but no|invalid EDID\n",
+ drm_get_connector_name(connector));
+ /* rs690 seems to have a problem with
connectors not existing and return
+* random EDIDs mainly with blocks of 0's. If
we see this, just stop
+* polling on this output */
+ if ((rdev->family == CHIP_RS690 || rdev->family
== CHIP_RS740) && radeon_connector->base.null_edid_counter) {
+ ret = connector_status_disconnected;
+ DRM_INFO("%s: detected RS690 floating
bus bug, stopping ddc detect\n",
+
[PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
Dear Alex,
> On Mon, Nov 28, 2011 at 11:20 AM, Thomas Reim
> wrote:
> >Extended DDC probe is now default for RADEON chipsets. In case of
> >HW bugs (e. g. floating connectors), the affected connectors will
> >not be used, as a valid EDID header can not be detected. Another
> >patch removed DDC detection and connector status logging during
> >device setup. So the user is not informed anymore about HW bugs
> >leading to connectors being unavailable.
> >Reintroduce one-time logging of connector unavailability status
> >when probing (single) connector modes during framebuffer
> >initialisation.
> >
> >For RS690 chipsets DDC detection shall be stopped, if the i2c bus
> >receives all-0 EDIDs (floating connectors). The introduction of
> >extended DDC probing prevents the driver from doing this. Correctly
> >relocate the related code.
>
> Is any of this needed anymore now that we do an extended probe? I
> think the original null edid patch was just papering over the actual
> issue which was the need for a full edid header probe.
The difference, at least this is my understanding of Dave's patch, is
that we completely stop DDC detection for the famous RS690/RS740 chips
with missing or disabled HDMI add-on cards. We decrease latency, as we
do not retrieve data from i2c bus anymore. Therefore, I would keep it.
>
> >
> > Signed-off-by: Thomas Reim
> > ---
> > drivers/gpu/drm/radeon/radeon_connectors.c | 54
> > +++-
> > drivers/gpu/drm/radeon/radeon_i2c.c|7 +++-
> > drivers/gpu/drm/radeon/radeon_mode.h |3 ++
> > 3 files changed, 53 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
> > b/drivers/gpu/drm/radeon/radeon_connectors.c
> > index e7cb3ab..a4c0f59 100644
> > --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> > +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> > @@ -662,6 +662,7 @@ radeon_vga_detect(struct drm_connector *connector, bool
> > force)
> >struct radeon_connector *radeon_connector =
> > to_radeon_connector(connector);
> >struct drm_encoder *encoder;
> >struct drm_encoder_helper_funcs *encoder_funcs;
> > + struct edid *edid = NULL;
> >bool dret = false;
> >enum drm_connector_status ret = connector_status_disconnected;
> >
> > @@ -669,8 +670,24 @@ radeon_vga_detect(struct drm_connector *connector,
> > bool force)
> >if (!encoder)
> >ret = connector_status_disconnected;
> >
> > - if (radeon_connector->ddc_bus)
> > + if (radeon_connector->ddc_bus) {
> >dret = radeon_ddc_probe(radeon_connector);
> > + if (!dret && radeon_connector->broken_edid_header_counter
> > == 1) {
> > + /* Found a broken DDC when probing (single)
> > connector
> > + modes during framebuffer initialisation.
> > + Clean-up and log the HW bug. */
> > + edid = drm_get_edid(_connector->base,
> > _connector->ddc_bus->adapter);
> > + if (!edid) {
> > +
> > radeon_connector->broken_edid_header_counter++;
> > + DRM_INFO("%s: probed a monitor, DDC
> > responded but no|invalid EDID\n",
> > +
> > drm_get_connector_name(connector));
> > + } else {
> > + /* We should not get here, as the EDID
> > header is broken */
> > + connector->display_info.raw_edid = NULL;
> > + kfree(edid);
> > + }
> > + }
> > + }
>
> What is this hunk for? It's not related to the problematic DDIA
> interface which is digital only. It seems like it will just spam the
> log for VGA monitors with faulty EDIDs.
The broken_edid_header_counter prevents from spamming. Only the first
time, the extended DDC probe fails due to a broken EDID header (which
may happen also to VGA connectors with a buggy monitor connected; at
least I remember to have seen such dmesg logs in the past) we would
inform users. As the EDID is broken, the screen will stay dark, but
users would have at least the required information in the logs. They can
post them, once they've connected a working monitor.
My ASUS RS690 system suffered from such log spamming. So please believe
me, that I would never ask for a solution that leads us back to this.
>
> >if (dret) {
> >radeon_connector->detected_by_load = false;
> >if (radeon_connector->edid) {
> > @@ -847,12 +864,38 @@ radeon_dvi_detect(struct drm_connector *connector,
> > bool force)
> >struct drm_encoder *encoder = NULL;
> >struct drm_encoder_helper_funcs *encoder_funcs;
> >struct drm_mode_object
[PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
On Tue, Nov 29, 2011 at 1:06 PM, Thomas Reim wrote:
> Dear Alex,
>
>> On Mon, Nov 28, 2011 at 11:20 AM, Thomas Reim
>> wrote:
>> > ? ? ? ?Extended DDC probe is now default for RADEON chipsets. In case of
>> > ? ? ? ?HW bugs (e. g. floating connectors), the affected connectors will
>> > ? ? ? ?not be used, as a valid EDID header can not be detected. Another
>> > ? ? ? ?patch removed DDC detection and connector status logging during
>> > ? ? ? ?device setup. So the user is not informed anymore about HW bugs
>> > ? ? ? ?leading to connectors being unavailable.
>> > ? ? ? ?Reintroduce one-time logging of connector unavailability status
>> > ? ? ? ?when probing (single) connector modes during framebuffer
>> > ? ? ? ?initialisation.
>> >
>> > ? ? ? ?For RS690 chipsets DDC detection shall be stopped, if the i2c bus
>> > ? ? ? ?receives all-0 EDIDs (floating connectors). The introduction of
>> > ? ? ? ?extended DDC probing prevents the driver from doing this. Correctly
>> > ? ? ? ?relocate the related code.
>>
>> Is any of this needed anymore now that we do an extended probe? ?I
>> think the original null edid patch was just papering over the actual
>> issue which was the need for a full edid header probe.
>
> The difference, at least this is my understanding of Dave's patch, is
> that we completely stop DDC detection for the famous RS690/RS740 chips
> with missing or disabled HDMI add-on cards. We decrease latency, as we
> do not retrieve data from i2c bus anymore. Therefore, I would keep it.
>
My only concern is that we may disable the i2c bus if there is no
monitor connected which would prevent edid fetching from working in
the future. Does the problematic ddia i2c line produce the same error
in the following cases:
1. ddia hdmi connector present, no monitor attached
2. ddia hdmi connector present, monitor with faulty edid attached
3. ddia hdmi connector absent
In the third case it makes sense to remove the i2c bus, but in the
first two, we may remove the i2c bus which would prevent future edid
fetches from working.
>>
>> >
>> > Signed-off-by: Thomas Reim
>> > ---
>> > ?drivers/gpu/drm/radeon/radeon_connectors.c | ? 54
>> > +++-
>> > ?drivers/gpu/drm/radeon/radeon_i2c.c ? ? ? ?| ? ?7 +++-
>> > ?drivers/gpu/drm/radeon/radeon_mode.h ? ? ? | ? ?3 ++
>> > ?3 files changed, 53 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
>> > b/drivers/gpu/drm/radeon/radeon_connectors.c
>> > index e7cb3ab..a4c0f59 100644
>> > --- a/drivers/gpu/drm/radeon/radeon_connectors.c
>> > +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
>> > @@ -662,6 +662,7 @@ radeon_vga_detect(struct drm_connector *connector,
>> > bool force)
>> > ? ? ? ?struct radeon_connector *radeon_connector =
>> > to_radeon_connector(connector);
>> > ? ? ? ?struct drm_encoder *encoder;
>> > ? ? ? ?struct drm_encoder_helper_funcs *encoder_funcs;
>> > + ? ? ? struct edid *edid = NULL;
>> > ? ? ? ?bool dret = false;
>> > ? ? ? ?enum drm_connector_status ret = connector_status_disconnected;
>> >
>> > @@ -669,8 +670,24 @@ radeon_vga_detect(struct drm_connector *connector,
>> > bool force)
>> > ? ? ? ?if (!encoder)
>> > ? ? ? ? ? ? ? ?ret = connector_status_disconnected;
>> >
>> > - ? ? ? if (radeon_connector->ddc_bus)
>> > + ? ? ? if (radeon_connector->ddc_bus) {
>> > ? ? ? ? ? ? ? ?dret = radeon_ddc_probe(radeon_connector);
>> > + ? ? ? ? ? ? ? if (!dret && radeon_connector->broken_edid_header_counter
>> > == 1) {
>> > + ? ? ? ? ? ? ? ? ? ? ? /* Found a broken DDC when probing (single)
>> > connector
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ?modes during framebuffer initialisation.
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ?Clean-up and log the HW bug. */
>> > + ? ? ? ? ? ? ? ? ? ? ? edid = drm_get_edid(_connector->base,
>> > _connector->ddc_bus->adapter);
>> > + ? ? ? ? ? ? ? ? ? ? ? if (!edid) {
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?
>> > radeon_connector->broken_edid_header_counter++;
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DRM_INFO("%s: probed a monitor, DDC
>> > responded but no|invalid EDID\n",
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?
>> > drm_get_connector_name(connector));
>> > + ? ? ? ? ? ? ? ? ? ? ? } else {
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? /* We should not get here, as the EDID
>> > header is broken */
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? connector->display_info.raw_edid = NULL;
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? kfree(edid);
>> > + ? ? ? ? ? ? ? ? ? ? ? }
>> > + ? ? ? ? ? ? ? }
>> > + ? ? ? }
>>
>> What is this hunk for? ?It's not related to the problematic DDIA
>> interface which is digital only. ?It seems like it will just spam the
>> log for VGA monitors with faulty EDIDs.
>
> The broken_edid_header_counter prevents from spamming. Only the first
> time, the extended DDC probe fails due to a broken EDID header (which
> may happen also to VGA connectors with a buggy monitor connected; at
> least I remember to have seen such dmesg logs in the past) we would
> inform users. As
[PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
On Mon, Nov 28, 2011 at 11:20 AM, Thomas Reim wrote:
> ? ? ? ?Extended DDC probe is now default for RADEON chipsets. In case of
> ? ? ? ?HW bugs (e. g. floating connectors), the affected connectors will
> ? ? ? ?not be used, as a valid EDID header can not be detected. Another
> ? ? ? ?patch removed DDC detection and connector status logging during
> ? ? ? ?device setup. So the user is not informed anymore about HW bugs
> ? ? ? ?leading to connectors being unavailable.
> ? ? ? ?Reintroduce one-time logging of connector unavailability status
> ? ? ? ?when probing (single) connector modes during framebuffer
> ? ? ? ?initialisation.
>
> ? ? ? ?For RS690 chipsets DDC detection shall be stopped, if the i2c bus
> ? ? ? ?receives all-0 EDIDs (floating connectors). The introduction of
> ? ? ? ?extended DDC probing prevents the driver from doing this. Correctly
> ? ? ? ?relocate the related code.
Is any of this needed anymore now that we do an extended probe? I
think the original null edid patch was just papering over the actual
issue which was the need for a full edid header probe.
>
> Signed-off-by: Thomas Reim
> ---
> ?drivers/gpu/drm/radeon/radeon_connectors.c | ? 54
> +++-
> ?drivers/gpu/drm/radeon/radeon_i2c.c ? ? ? ?| ? ?7 +++-
> ?drivers/gpu/drm/radeon/radeon_mode.h ? ? ? | ? ?3 ++
> ?3 files changed, 53 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
> b/drivers/gpu/drm/radeon/radeon_connectors.c
> index e7cb3ab..a4c0f59 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -662,6 +662,7 @@ radeon_vga_detect(struct drm_connector *connector, bool
> force)
> ? ? ? ?struct radeon_connector *radeon_connector =
> to_radeon_connector(connector);
> ? ? ? ?struct drm_encoder *encoder;
> ? ? ? ?struct drm_encoder_helper_funcs *encoder_funcs;
> + ? ? ? struct edid *edid = NULL;
> ? ? ? ?bool dret = false;
> ? ? ? ?enum drm_connector_status ret = connector_status_disconnected;
>
> @@ -669,8 +670,24 @@ radeon_vga_detect(struct drm_connector *connector, bool
> force)
> ? ? ? ?if (!encoder)
> ? ? ? ? ? ? ? ?ret = connector_status_disconnected;
>
> - ? ? ? if (radeon_connector->ddc_bus)
> + ? ? ? if (radeon_connector->ddc_bus) {
> ? ? ? ? ? ? ? ?dret = radeon_ddc_probe(radeon_connector);
> + ? ? ? ? ? ? ? if (!dret && radeon_connector->broken_edid_header_counter ==
> 1) {
> + ? ? ? ? ? ? ? ? ? ? ? /* Found a broken DDC when probing (single) connector
> + ? ? ? ? ? ? ? ? ? ? ? ? ?modes during framebuffer initialisation.
> + ? ? ? ? ? ? ? ? ? ? ? ? ?Clean-up and log the HW bug. */
> + ? ? ? ? ? ? ? ? ? ? ? edid = drm_get_edid(_connector->base,
> _connector->ddc_bus->adapter);
> + ? ? ? ? ? ? ? ? ? ? ? if (!edid) {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?
> radeon_connector->broken_edid_header_counter++;
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DRM_INFO("%s: probed a monitor, DDC responded
> but no|invalid EDID\n",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? drm_get_connector_name(connector));
> + ? ? ? ? ? ? ? ? ? ? ? } else {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? /* We should not get here, as the EDID header
> is broken */
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? connector->display_info.raw_edid = NULL;
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? kfree(edid);
> + ? ? ? ? ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? }
> + ? ? ? }
What is this hunk for? It's not related to the problematic DDIA
interface which is digital only. It seems like it will just spam the
log for VGA monitors with faulty EDIDs.
> ? ? ? ?if (dret) {
> ? ? ? ? ? ? ? ?radeon_connector->detected_by_load = false;
> ? ? ? ? ? ? ? ?if (radeon_connector->edid) {
> @@ -847,12 +864,38 @@ radeon_dvi_detect(struct drm_connector *connector, bool
> force)
> ? ? ? ?struct drm_encoder *encoder = NULL;
> ? ? ? ?struct drm_encoder_helper_funcs *encoder_funcs;
> ? ? ? ?struct drm_mode_object *obj;
> + ? ? ? struct edid *edid = NULL;
> ? ? ? ?int i;
> ? ? ? ?enum drm_connector_status ret = connector_status_disconnected;
> ? ? ? ?bool dret = false;
>
> - ? ? ? if (radeon_connector->ddc_bus)
> + ? ? ? if (radeon_connector->ddc_bus) {
> ? ? ? ? ? ? ? ?dret = radeon_ddc_probe(radeon_connector);
> + ? ? ? ? ? ? ? if (!dret && radeon_connector->broken_edid_header_counter ==
> 1) {
> + ? ? ? ? ? ? ? ? ? ? ? /* Found a broken DDC when probing (single) connector
> + ? ? ? ? ? ? ? ? ? ? ? ? ?modes during framebuffer initialisation.
> + ? ? ? ? ? ? ? ? ? ? ? ? ?Clean-up and log the HW bug. */
> + ? ? ? ? ? ? ? ? ? ? ? edid = drm_get_edid(_connector->base,
> _connector->ddc_bus->adapter);
> + ? ? ? ? ? ? ? ? ? ? ? if (!edid) {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?
> radeon_connector->broken_edid_header_counter++;
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DRM_INFO("%s: probed a monitor, DDC responded
> but no|invalid EDID\n",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? drm_get_connector_name(connector));
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? /* rs690 seems to have a problem with
> connectors not
Re: [PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
On Mon, Nov 28, 2011 at 11:20 AM, Thomas Reim rei...@googlemail.com wrote:
Extended DDC probe is now default for RADEON chipsets. In case of
HW bugs (e. g. floating connectors), the affected connectors will
not be used, as a valid EDID header can not be detected. Another
patch removed DDC detection and connector status logging during
device setup. So the user is not informed anymore about HW bugs
leading to connectors being unavailable.
Reintroduce one-time logging of connector unavailability status
when probing (single) connector modes during framebuffer
initialisation.
For RS690 chipsets DDC detection shall be stopped, if the i2c bus
receives all-0 EDIDs (floating connectors). The introduction of
extended DDC probing prevents the driver from doing this. Correctly
relocate the related code.
Is any of this needed anymore now that we do an extended probe? I
think the original null edid patch was just papering over the actual
issue which was the need for a full edid header probe.
Signed-off-by: Thomas Reim rei...@gmail.com
---
drivers/gpu/drm/radeon/radeon_connectors.c | 54
+++-
drivers/gpu/drm/radeon/radeon_i2c.c | 7 +++-
drivers/gpu/drm/radeon/radeon_mode.h | 3 ++
3 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index e7cb3ab..a4c0f59 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -662,6 +662,7 @@ radeon_vga_detect(struct drm_connector *connector, bool
force)
struct radeon_connector *radeon_connector =
to_radeon_connector(connector);
struct drm_encoder *encoder;
struct drm_encoder_helper_funcs *encoder_funcs;
+ struct edid *edid = NULL;
bool dret = false;
enum drm_connector_status ret = connector_status_disconnected;
@@ -669,8 +670,24 @@ radeon_vga_detect(struct drm_connector *connector, bool
force)
if (!encoder)
ret = connector_status_disconnected;
- if (radeon_connector-ddc_bus)
+ if (radeon_connector-ddc_bus) {
dret = radeon_ddc_probe(radeon_connector);
+ if (!dret radeon_connector-broken_edid_header_counter ==
1) {
+ /* Found a broken DDC when probing (single) connector
+ modes during framebuffer initialisation.
+ Clean-up and log the HW bug. */
+ edid = drm_get_edid(radeon_connector-base,
radeon_connector-ddc_bus-adapter);
+ if (!edid) {
+
radeon_connector-broken_edid_header_counter++;
+ DRM_INFO(%s: probed a monitor, DDC responded
but no|invalid EDID\n,
+ drm_get_connector_name(connector));
+ } else {
+ /* We should not get here, as the EDID header
is broken */
+ connector-display_info.raw_edid = NULL;
+ kfree(edid);
+ }
+ }
+ }
What is this hunk for? It's not related to the problematic DDIA
interface which is digital only. It seems like it will just spam the
log for VGA monitors with faulty EDIDs.
if (dret) {
radeon_connector-detected_by_load = false;
if (radeon_connector-edid) {
@@ -847,12 +864,38 @@ radeon_dvi_detect(struct drm_connector *connector, bool
force)
struct drm_encoder *encoder = NULL;
struct drm_encoder_helper_funcs *encoder_funcs;
struct drm_mode_object *obj;
+ struct edid *edid = NULL;
int i;
enum drm_connector_status ret = connector_status_disconnected;
bool dret = false;
- if (radeon_connector-ddc_bus)
+ if (radeon_connector-ddc_bus) {
dret = radeon_ddc_probe(radeon_connector);
+ if (!dret radeon_connector-broken_edid_header_counter ==
1) {
+ /* Found a broken DDC when probing (single) connector
+ modes during framebuffer initialisation.
+ Clean-up and log the HW bug. */
+ edid = drm_get_edid(radeon_connector-base,
radeon_connector-ddc_bus-adapter);
+ if (!edid) {
+
radeon_connector-broken_edid_header_counter++;
+ DRM_INFO(%s: probed a monitor, DDC responded
but no|invalid EDID\n,
+ drm_get_connector_name(connector));
+ /* rs690 seems to have a problem with
connectors not existing and return
+ *
Re: [PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
Dear Alex,
On Mon, Nov 28, 2011 at 11:20 AM, Thomas Reim rei...@googlemail.com wrote:
Extended DDC probe is now default for RADEON chipsets. In case of
HW bugs (e. g. floating connectors), the affected connectors will
not be used, as a valid EDID header can not be detected. Another
patch removed DDC detection and connector status logging during
device setup. So the user is not informed anymore about HW bugs
leading to connectors being unavailable.
Reintroduce one-time logging of connector unavailability status
when probing (single) connector modes during framebuffer
initialisation.
For RS690 chipsets DDC detection shall be stopped, if the i2c bus
receives all-0 EDIDs (floating connectors). The introduction of
extended DDC probing prevents the driver from doing this. Correctly
relocate the related code.
Is any of this needed anymore now that we do an extended probe? I
think the original null edid patch was just papering over the actual
issue which was the need for a full edid header probe.
The difference, at least this is my understanding of Dave's patch, is
that we completely stop DDC detection for the famous RS690/RS740 chips
with missing or disabled HDMI add-on cards. We decrease latency, as we
do not retrieve data from i2c bus anymore. Therefore, I would keep it.
Signed-off-by: Thomas Reim rei...@gmail.com
---
drivers/gpu/drm/radeon/radeon_connectors.c | 54
+++-
drivers/gpu/drm/radeon/radeon_i2c.c|7 +++-
drivers/gpu/drm/radeon/radeon_mode.h |3 ++
3 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index e7cb3ab..a4c0f59 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -662,6 +662,7 @@ radeon_vga_detect(struct drm_connector *connector, bool
force)
struct radeon_connector *radeon_connector =
to_radeon_connector(connector);
struct drm_encoder *encoder;
struct drm_encoder_helper_funcs *encoder_funcs;
+ struct edid *edid = NULL;
bool dret = false;
enum drm_connector_status ret = connector_status_disconnected;
@@ -669,8 +670,24 @@ radeon_vga_detect(struct drm_connector *connector,
bool force)
if (!encoder)
ret = connector_status_disconnected;
- if (radeon_connector-ddc_bus)
+ if (radeon_connector-ddc_bus) {
dret = radeon_ddc_probe(radeon_connector);
+ if (!dret radeon_connector-broken_edid_header_counter
== 1) {
+ /* Found a broken DDC when probing (single)
connector
+ modes during framebuffer initialisation.
+ Clean-up and log the HW bug. */
+ edid = drm_get_edid(radeon_connector-base,
radeon_connector-ddc_bus-adapter);
+ if (!edid) {
+
radeon_connector-broken_edid_header_counter++;
+ DRM_INFO(%s: probed a monitor, DDC
responded but no|invalid EDID\n,
+
drm_get_connector_name(connector));
+ } else {
+ /* We should not get here, as the EDID
header is broken */
+ connector-display_info.raw_edid = NULL;
+ kfree(edid);
+ }
+ }
+ }
What is this hunk for? It's not related to the problematic DDIA
interface which is digital only. It seems like it will just spam the
log for VGA monitors with faulty EDIDs.
The broken_edid_header_counter prevents from spamming. Only the first
time, the extended DDC probe fails due to a broken EDID header (which
may happen also to VGA connectors with a buggy monitor connected; at
least I remember to have seen such dmesg logs in the past) we would
inform users. As the EDID is broken, the screen will stay dark, but
users would have at least the required information in the logs. They can
post them, once they've connected a working monitor.
My ASUS RS690 system suffered from such log spamming. So please believe
me, that I would never ask for a solution that leads us back to this.
if (dret) {
radeon_connector-detected_by_load = false;
if (radeon_connector-edid) {
@@ -847,12 +864,38 @@ radeon_dvi_detect(struct drm_connector *connector,
bool force)
struct drm_encoder *encoder = NULL;
struct drm_encoder_helper_funcs *encoder_funcs;
struct drm_mode_object *obj;
+ struct edid *edid = NULL;
int i;
enum drm_connector_status ret = connector_status_disconnected;
[PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
Extended DDC probe is now default for RADEON chipsets. In case of
HW bugs (e. g. floating connectors), the affected connectors will
not be used, as a valid EDID header can not be detected. Another
patch removed DDC detection and connector status logging during
device setup. So the user is not informed anymore about HW bugs
leading to connectors being unavailable.
Reintroduce one-time logging of connector unavailability status
when probing (single) connector modes during framebuffer
initialisation.
For RS690 chipsets DDC detection shall be stopped, if the i2c bus
receives all-0 EDIDs (floating connectors). The introduction of
extended DDC probing prevents the driver from doing this. Correctly
relocate the related code.
Signed-off-by: Thomas Reim rei...@gmail.com
---
drivers/gpu/drm/radeon/radeon_connectors.c | 54 +++-
drivers/gpu/drm/radeon/radeon_i2c.c|7 +++-
drivers/gpu/drm/radeon/radeon_mode.h |3 ++
3 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index e7cb3ab..b06480c 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -662,6 +662,7 @@ radeon_vga_detect(struct drm_connector *connector, bool
force)
struct radeon_connector *radeon_connector =
to_radeon_connector(connector);
struct drm_encoder *encoder;
struct drm_encoder_helper_funcs *encoder_funcs;
+ struct edid *edid = NULL;
bool dret = false;
enum drm_connector_status ret = connector_status_disconnected;
@@ -669,8 +670,24 @@ radeon_vga_detect(struct drm_connector *connector, bool
force)
if (!encoder)
ret = connector_status_disconnected;
- if (radeon_connector-ddc_bus)
+ if (radeon_connector-ddc_bus) {
dret = radeon_ddc_probe(radeon_connector);
+ if (!dret radeon_connector-broken_edid_header_counter == 1)
{
+ /* Found a broken DDC when probing (single) connector
+ modes during framebuffer initialisation.
+ Clean-up and log the HW bug. */
+ edid = drm_get_edid(radeon_connector-base,
radeon_connector-ddc_bus-adapter);
+ if (!edid) {
+ radeon_connector-broken_edid_header_counter++;
+ DRM_INFO(%s: probed a monitor, DDC responded
but no|invalid EDID\n,
+ drm_get_connector_name(connector));
+ } else {
+ /* We should not get here, as the EDID header
is broken */
+ connector-display_info.raw_edid = NULL;
+ kfree(edid);
+ }
+ }
+ }
if (dret) {
radeon_connector-detected_by_load = false;
if (radeon_connector-edid) {
@@ -847,12 +864,38 @@ radeon_dvi_detect(struct drm_connector *connector, bool
force)
struct drm_encoder *encoder = NULL;
struct drm_encoder_helper_funcs *encoder_funcs;
struct drm_mode_object *obj;
+ struct edid *edid = NULL;
int i;
enum drm_connector_status ret = connector_status_disconnected;
bool dret = false;
- if (radeon_connector-ddc_bus)
+ if (radeon_connector-ddc_bus) {
dret = radeon_ddc_probe(radeon_connector);
+ if (!dret radeon_connector-broken_edid_header_counter == 1)
{
+ /* Found a broken DDC when probing (single) connector
+ modes during framebuffer initialisation.
+ Clean-up and log the HW bug. */
+ edid = drm_get_edid(radeon_connector-base,
radeon_connector-ddc_bus-adapter);
+ if (!edid) {
+ radeon_connector-broken_edid_header_counter++;
+ DRM_INFO(%s: probed a monitor, DDC responded
but no|invalid EDID\n,
+ drm_get_connector_name(connector));
+ /* rs690 seems to have a problem with
connectors not existing and return
+* random EDIDs mainly with blocks of 0's. If
we see this, just stop
+* polling on this output */
+ if ((rdev-family == CHIP_RS690 || rdev-family
== CHIP_RS740) radeon_connector-base.null_edid_counter) {
+ ret = connector_status_disconnected;
+ DRM_INFO(%s: detected RS690 floating
bus bug, stopping ddc detect\n,
+
Re: [PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
On Tue, Nov 29, 2011 at 1:06 PM, Thomas Reim rei...@googlemail.com wrote:
Dear Alex,
On Mon, Nov 28, 2011 at 11:20 AM, Thomas Reim rei...@googlemail.com wrote:
Extended DDC probe is now default for RADEON chipsets. In case of
HW bugs (e. g. floating connectors), the affected connectors will
not be used, as a valid EDID header can not be detected. Another
patch removed DDC detection and connector status logging during
device setup. So the user is not informed anymore about HW bugs
leading to connectors being unavailable.
Reintroduce one-time logging of connector unavailability status
when probing (single) connector modes during framebuffer
initialisation.
For RS690 chipsets DDC detection shall be stopped, if the i2c bus
receives all-0 EDIDs (floating connectors). The introduction of
extended DDC probing prevents the driver from doing this. Correctly
relocate the related code.
Is any of this needed anymore now that we do an extended probe? I
think the original null edid patch was just papering over the actual
issue which was the need for a full edid header probe.
The difference, at least this is my understanding of Dave's patch, is
that we completely stop DDC detection for the famous RS690/RS740 chips
with missing or disabled HDMI add-on cards. We decrease latency, as we
do not retrieve data from i2c bus anymore. Therefore, I would keep it.
My only concern is that we may disable the i2c bus if there is no
monitor connected which would prevent edid fetching from working in
the future. Does the problematic ddia i2c line produce the same error
in the following cases:
1. ddia hdmi connector present, no monitor attached
2. ddia hdmi connector present, monitor with faulty edid attached
3. ddia hdmi connector absent
In the third case it makes sense to remove the i2c bus, but in the
first two, we may remove the i2c bus which would prevent future edid
fetches from working.
Signed-off-by: Thomas Reim rei...@gmail.com
---
drivers/gpu/drm/radeon/radeon_connectors.c | 54
+++-
drivers/gpu/drm/radeon/radeon_i2c.c | 7 +++-
drivers/gpu/drm/radeon/radeon_mode.h | 3 ++
3 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index e7cb3ab..a4c0f59 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -662,6 +662,7 @@ radeon_vga_detect(struct drm_connector *connector,
bool force)
struct radeon_connector *radeon_connector =
to_radeon_connector(connector);
struct drm_encoder *encoder;
struct drm_encoder_helper_funcs *encoder_funcs;
+ struct edid *edid = NULL;
bool dret = false;
enum drm_connector_status ret = connector_status_disconnected;
@@ -669,8 +670,24 @@ radeon_vga_detect(struct drm_connector *connector,
bool force)
if (!encoder)
ret = connector_status_disconnected;
- if (radeon_connector-ddc_bus)
+ if (radeon_connector-ddc_bus) {
dret = radeon_ddc_probe(radeon_connector);
+ if (!dret radeon_connector-broken_edid_header_counter
== 1) {
+ /* Found a broken DDC when probing (single)
connector
+ modes during framebuffer initialisation.
+ Clean-up and log the HW bug. */
+ edid = drm_get_edid(radeon_connector-base,
radeon_connector-ddc_bus-adapter);
+ if (!edid) {
+
radeon_connector-broken_edid_header_counter++;
+ DRM_INFO(%s: probed a monitor, DDC
responded but no|invalid EDID\n,
+
drm_get_connector_name(connector));
+ } else {
+ /* We should not get here, as the EDID
header is broken */
+ connector-display_info.raw_edid = NULL;
+ kfree(edid);
+ }
+ }
+ }
What is this hunk for? It's not related to the problematic DDIA
interface which is digital only. It seems like it will just spam the
log for VGA monitors with faulty EDIDs.
The broken_edid_header_counter prevents from spamming. Only the first
time, the extended DDC probe fails due to a broken EDID header (which
may happen also to VGA connectors with a buggy monitor connected; at
least I remember to have seen such dmesg logs in the past) we would
inform users. As the EDID is broken, the screen will stay dark, but
users would have at least the required information in the logs. They can
post them, once they've connected a working monitor.
My ASUS
[PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
Extended DDC probe is now default for RADEON chipsets. In case of
HW bugs (e. g. floating connectors), the affected connectors will
not be used, as a valid EDID header can not be detected. Another
patch removed DDC detection and connector status logging during
device setup. So the user is not informed anymore about HW bugs
leading to connectors being unavailable.
Reintroduce one-time logging of connector unavailability status
when probing (single) connector modes during framebuffer
initialisation.
For RS690 chipsets DDC detection shall be stopped, if the i2c bus
receives all-0 EDIDs (floating connectors). The introduction of
extended DDC probing prevents the driver from doing this. Correctly
relocate the related code.
Signed-off-by: Thomas Reim
---
drivers/gpu/drm/radeon/radeon_connectors.c | 54 +++-
drivers/gpu/drm/radeon/radeon_i2c.c|7 +++-
drivers/gpu/drm/radeon/radeon_mode.h |3 ++
3 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index e7cb3ab..a4c0f59 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -662,6 +662,7 @@ radeon_vga_detect(struct drm_connector *connector, bool
force)
struct radeon_connector *radeon_connector =
to_radeon_connector(connector);
struct drm_encoder *encoder;
struct drm_encoder_helper_funcs *encoder_funcs;
+ struct edid *edid = NULL;
bool dret = false;
enum drm_connector_status ret = connector_status_disconnected;
@@ -669,8 +670,24 @@ radeon_vga_detect(struct drm_connector *connector, bool
force)
if (!encoder)
ret = connector_status_disconnected;
- if (radeon_connector->ddc_bus)
+ if (radeon_connector->ddc_bus) {
dret = radeon_ddc_probe(radeon_connector);
+ if (!dret && radeon_connector->broken_edid_header_counter == 1)
{
+ /* Found a broken DDC when probing (single) connector
+ modes during framebuffer initialisation.
+ Clean-up and log the HW bug. */
+ edid = drm_get_edid(_connector->base,
_connector->ddc_bus->adapter);
+ if (!edid) {
+ radeon_connector->broken_edid_header_counter++;
+ DRM_INFO("%s: probed a monitor, DDC responded
but no|invalid EDID\n",
+ drm_get_connector_name(connector));
+ } else {
+ /* We should not get here, as the EDID header
is broken */
+ connector->display_info.raw_edid = NULL;
+ kfree(edid);
+ }
+ }
+ }
if (dret) {
radeon_connector->detected_by_load = false;
if (radeon_connector->edid) {
@@ -847,12 +864,38 @@ radeon_dvi_detect(struct drm_connector *connector, bool
force)
struct drm_encoder *encoder = NULL;
struct drm_encoder_helper_funcs *encoder_funcs;
struct drm_mode_object *obj;
+ struct edid *edid = NULL;
int i;
enum drm_connector_status ret = connector_status_disconnected;
bool dret = false;
- if (radeon_connector->ddc_bus)
+ if (radeon_connector->ddc_bus) {
dret = radeon_ddc_probe(radeon_connector);
+ if (!dret && radeon_connector->broken_edid_header_counter == 1)
{
+ /* Found a broken DDC when probing (single) connector
+ modes during framebuffer initialisation.
+ Clean-up and log the HW bug. */
+ edid = drm_get_edid(_connector->base,
_connector->ddc_bus->adapter);
+ if (!edid) {
+ radeon_connector->broken_edid_header_counter++;
+ DRM_INFO("%s: probed a monitor, DDC responded
but no|invalid EDID\n",
+ drm_get_connector_name(connector));
+ /* rs690 seems to have a problem with
connectors not existing and return
+* random EDIDs mainly with blocks of 0's. If
we see this, just stop
+* polling on this output */
+ if ((rdev->family == CHIP_RS690 || rdev->family
== CHIP_RS740)) {
+ ret = connector_status_disconnected;
+ DRM_INFO("%s: detected RS690 floating
bus bug, stopping ddc detect\n",
+
drm_get_connector_name(connector));
+
[PATCH 2/2] drm/radeon/kms: wrap-up handling of floating connectors and connector unavailability status logging
Extended DDC probe is now default for RADEON chipsets. In case of
HW bugs (e. g. floating connectors), the affected connectors will
not be used, as a valid EDID header can not be detected. Another
patch removed DDC detection and connector status logging during
device setup. So the user is not informed anymore about HW bugs
leading to connectors being unavailable.
Reintroduce one-time logging of connector unavailability status
when probing (single) connector modes during framebuffer
initialisation.
For RS690 chipsets DDC detection shall be stopped, if the i2c bus
receives all-0 EDIDs (floating connectors). The introduction of
extended DDC probing prevents the driver from doing this. Correctly
relocate the related code.
Signed-off-by: Thomas Reim rei...@gmail.com
---
drivers/gpu/drm/radeon/radeon_connectors.c | 54 +++-
drivers/gpu/drm/radeon/radeon_i2c.c|7 +++-
drivers/gpu/drm/radeon/radeon_mode.h |3 ++
3 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index e7cb3ab..a4c0f59 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -662,6 +662,7 @@ radeon_vga_detect(struct drm_connector *connector, bool
force)
struct radeon_connector *radeon_connector =
to_radeon_connector(connector);
struct drm_encoder *encoder;
struct drm_encoder_helper_funcs *encoder_funcs;
+ struct edid *edid = NULL;
bool dret = false;
enum drm_connector_status ret = connector_status_disconnected;
@@ -669,8 +670,24 @@ radeon_vga_detect(struct drm_connector *connector, bool
force)
if (!encoder)
ret = connector_status_disconnected;
- if (radeon_connector-ddc_bus)
+ if (radeon_connector-ddc_bus) {
dret = radeon_ddc_probe(radeon_connector);
+ if (!dret radeon_connector-broken_edid_header_counter == 1)
{
+ /* Found a broken DDC when probing (single) connector
+ modes during framebuffer initialisation.
+ Clean-up and log the HW bug. */
+ edid = drm_get_edid(radeon_connector-base,
radeon_connector-ddc_bus-adapter);
+ if (!edid) {
+ radeon_connector-broken_edid_header_counter++;
+ DRM_INFO(%s: probed a monitor, DDC responded
but no|invalid EDID\n,
+ drm_get_connector_name(connector));
+ } else {
+ /* We should not get here, as the EDID header
is broken */
+ connector-display_info.raw_edid = NULL;
+ kfree(edid);
+ }
+ }
+ }
if (dret) {
radeon_connector-detected_by_load = false;
if (radeon_connector-edid) {
@@ -847,12 +864,38 @@ radeon_dvi_detect(struct drm_connector *connector, bool
force)
struct drm_encoder *encoder = NULL;
struct drm_encoder_helper_funcs *encoder_funcs;
struct drm_mode_object *obj;
+ struct edid *edid = NULL;
int i;
enum drm_connector_status ret = connector_status_disconnected;
bool dret = false;
- if (radeon_connector-ddc_bus)
+ if (radeon_connector-ddc_bus) {
dret = radeon_ddc_probe(radeon_connector);
+ if (!dret radeon_connector-broken_edid_header_counter == 1)
{
+ /* Found a broken DDC when probing (single) connector
+ modes during framebuffer initialisation.
+ Clean-up and log the HW bug. */
+ edid = drm_get_edid(radeon_connector-base,
radeon_connector-ddc_bus-adapter);
+ if (!edid) {
+ radeon_connector-broken_edid_header_counter++;
+ DRM_INFO(%s: probed a monitor, DDC responded
but no|invalid EDID\n,
+ drm_get_connector_name(connector));
+ /* rs690 seems to have a problem with
connectors not existing and return
+* random EDIDs mainly with blocks of 0's. If
we see this, just stop
+* polling on this output */
+ if ((rdev-family == CHIP_RS690 || rdev-family
== CHIP_RS740)) {
+ ret = connector_status_disconnected;
+ DRM_INFO(%s: detected RS690 floating
bus bug, stopping ddc detect\n,
+
drm_get_connector_name(connector));
+
