Re: [Intel-gfx] [PATCH v4 14/23] drm/tilcdc: Provide ddc symlink in connector sysfs directory
Hi,
I'm glad to see this work moving forward!
On Wed, 2019-07-24 at 10:01 +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 23.07.19 um 14:44 schrieb Andrzej Pietrasiewicz:
> > Hi Sam,
> >
> > W dniu 23.07.2019 o 11:05, Sam Ravnborg pisze:
> > > Hi Andrzej
> > >
> > > On Thu, Jul 11, 2019 at 01:26:41PM +0200, Andrzej Pietrasiewicz wrote:
> > > > Use the ddc pointer provided by the generic connector.
> > > >
> > > > Signed-off-by: Andrzej Pietrasiewicz
> > > > ---
> > > > drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> > > > b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> > > > index 62d014c20988..c373edb95666 100644
> > > > --- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> > > > +++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> > > > @@ -219,6 +219,7 @@ static struct drm_connector
> > > > *tfp410_connector_create(struct drm_device *dev,
> > > > tfp410_connector->mod = mod;
> > > > connector = _connector->base;
> > > > +connector->ddc = mod->i2c;
> > > > drm_connector_init(dev, connector, _connector_funcs,
> > > > DRM_MODE_CONNECTOR_DVID);
> > >
> > > When reading this code, it looks strange that we set connector->ddc
> > > *before* the call to init the connector.
> > > One could risk that drm_connector_init() used memset(..) to clear all
> > > fields or so, and it would break this order.
> >
> > I verified the code of drm_connector_init() and cannot find any memset()
> > invocations there. What is your actual concern?
>
> I think this echoes my concern about the implicit order of operation. It
> seems too easy to get this wrong. If you don't want to add an additional
> interface for setting the ddc field, why not add a dedicated initializer
> function that sets the ddc field? Something like this.
>
> int drm_connector_init_with_ddc(connector, funcs, ..., ddc)
> {
> ret = drm_connector_init(connector, funcs, ...);
> if (ret)
> return ret;
>
> if (!ddc)
> return 0;
>
> connector->ddc = ddc;
> /* set up sysfs */
>
I know this comment comes late to the party, but I'm a slightly
suprised to see the above instead of implementing drm_connector_init
in terms of drm_connector_init_with_ddc, as we typically do.
Namely, something along these lines (code might not even build!):
--8<-
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index d49e19f3de3a..dbd095933175 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -179,11 +179,12 @@ void drm_connector_free_work_fn(struct work_struct *work)
}
/**
- * drm_connector_init - Init a preallocated connector
+ * drm_connector_init_with_ddc - Init a preallocated connector
* @dev: DRM device
* @connector: the connector to init
* @funcs: callbacks for this connector
* @connector_type: user visible type of the connector
+ * @ddc: pointer to the associated ddc adapter (optional)
*
* Initialises a preallocated connector. Connectors should be
* subclassed as part of driver connector objects.
@@ -191,10 +192,11 @@ void drm_connector_free_work_fn(struct work_struct *work)
* Returns:
* Zero on success, error code on failure.
*/
-int drm_connector_init(struct drm_device *dev,
- struct drm_connector *connector,
- const struct drm_connector_funcs *funcs,
- int connector_type)
+int drm_connector_init_with_ddc(struct drm_device *dev,
+ struct drm_connector *connector,
+ const struct drm_connector_funcs *funcs,
+ int connector_type,
+ struct i2c_adapter *ddc)
{
struct drm_mode_config *config = >mode_config;
int ret;
@@ -215,6 +217,9 @@ int drm_connector_init(struct drm_device *dev,
connector->dev = dev;
connector->funcs = funcs;
+ /* provide ddc symlink in sysfs */
+ connector->ddc = ddc;
+
/* connector index is used with 32bit bitmasks */
ret = ida_simple_get(>connector_ida, 0, 32, GFP_KERNEL);
if (ret < 0) {
@@ -295,41 +300,6 @@ int drm_connector_init(struct drm_device *dev,
return ret;
}
-EXPORT_SYMBOL(drm_connector_init);
-
-/**
- * drm_connector_init_with_ddc - Init a preallocated connector
- * @dev: DRM device
- * @connector: the connector to init
- * @funcs: callbacks for this connector
- * @connector_type: user visible type of the connector
- * @ddc: pointer to the associated ddc adapter
- *
- * Initialises a preallocated connector. Connectors should be
- * subclassed as part of driver connector objects.
- *
- * Ensures that the ddc field of the connector is correctly set.
- *
- * Returns:
- * Zero on success, error code on failure.
- */
-int
Re: [Intel-gfx] [v7 00/16] Add Plane Color Properties
On Wed, 2019-06-19 at 18:03 +0300, Ville Syrjälä wrote: > On Wed, Jun 19, 2019 at 10:18:18AM -0300, Ezequiel Garcia wrote: > > On Wed, 2019-06-19 at 06:20 +, Shankar, Uma wrote: > > > > -Original Message- > > > > From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On > > > > Behalf Of > > > > Ezequiel Garcia > > > > Sent: Friday, June 14, 2019 9:48 PM > > > > To: Shankar, Uma > > > > Cc: Emil Velikov ; > > > > intel-gfx@lists.freedesktop.org; Syrjala, > > > > Ville ; Lankhorst, Maarten > > > > ; > > > > dri-devel > > > > Subject: Re: [v7 00/16] Add Plane Color Properties > > > > > > > > On Thu, 28 Mar 2019 at 16:50, Uma Shankar wrote: > > > > > This is how a typical display color hardware pipeline looks like: > > > > > +---+ > > > > > |RAM| > > > > > | +--++-++-+ | > > > > > | | FB 1 || FB 2 || FB N| | > > > > > | +--++-++-+ | > > > > > +---+ > > > > >| Plane Color Hardware Block | > > > > > ++ > > > > > | +---v-+ +---v---+ +---v--+ | > > > > > | | Plane A | | Plane B | | Plane N | | > > > > > | | DeGamma | | Degamma | | Degamma | | > > > > > | +---+-+ +---+---+ +---+--+ | > > > > > | | | || > > > > > | +---v-+ +---v---+ +---v--+ | > > > > > | |Plane A | | Plane B | | Plane N | | > > > > > | |CSC/CTM | | CSC/CTM | | CSC/CTM | | > > > > > | +---+-+ ++--+ ++-+ | > > > > > | | | | | > > > > > | +---v-+ +v--+ +v-+ | > > > > > | | Plane A | | Plane B | | Plane N | | > > > > > | | Gamma | | Gamma | | Gamma| | > > > > > | +---+-+ ++--+ ++-+ | > > > > > | | | | | > > > > > ++ > > > > > +--v--v---v---| > > > > > > > || > > > > > > > Pipe Blender|| > > > > > +++ > > > > > >|| > > > > > >+---v--+ | > > > > > >| Pipe DeGamma| | > > > > > >| | | > > > > > >+---+--+ | > > > > > >|Pipe Color | > > > > > >+---v--+ Hardware| > > > > > >| Pipe CSC/CTM| | > > > > > >| | | > > > > > >+---+--+ | > > > > > >|| > > > > > >+---v--+ | > > > > > >| Pipe Gamma | | > > > > > >| | | > > > > > >+---+--+ | > > > > > >|| > > > > > +-+ > > > > > | > > > > > v > > > > >Pipe Output > > > > > > > > > > This patch series adds properties for plane color features. It adds > > > > > properties for degamma used to linearize data, CSC used for gamut > > > > > conversion, and gamma used to again non-linearize data as per panel > > > > > supported color space. These can be utilize by user space to convert > > > > > planes from one format to another, one color space to another etc. > > > > > > > > > > Usersapce
Re: [Intel-gfx] [v7 00/16] Add Plane Color Properties
On Wed, 2019-06-19 at 06:20 +, Shankar, Uma wrote: > > -Original Message- > > From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf > > Of > > Ezequiel Garcia > > Sent: Friday, June 14, 2019 9:48 PM > > To: Shankar, Uma > > Cc: Emil Velikov ; > > intel-gfx@lists.freedesktop.org; Syrjala, > > Ville ; Lankhorst, Maarten > > ; > > dri-devel > > Subject: Re: [v7 00/16] Add Plane Color Properties > > > > On Thu, 28 Mar 2019 at 16:50, Uma Shankar wrote: > > > This is how a typical display color hardware pipeline looks like: > > > +---+ > > > |RAM| > > > | +--++-++-+ | > > > | | FB 1 || FB 2 || FB N| | > > > | +--++-++-+ | > > > +---+ > > >| Plane Color Hardware Block | > > > ++ > > > | +---v-+ +---v---+ +---v--+ | > > > | | Plane A | | Plane B | | Plane N | | > > > | | DeGamma | | Degamma | | Degamma | | > > > | +---+-+ +---+---+ +---+--+ | > > > | | | || > > > | +---v-+ +---v---+ +---v--+ | > > > | |Plane A | | Plane B | | Plane N | | > > > | |CSC/CTM | | CSC/CTM | | CSC/CTM | | > > > | +---+-+ ++--+ ++-+ | > > > | | | | | > > > | +---v-+ +v--+ +v-+ | > > > | | Plane A | | Plane B | | Plane N | | > > > | | Gamma | | Gamma | | Gamma| | > > > | +---+-+ ++--+ ++-+ | > > > | | | | | > > > ++ > > > +--v--v---v---| > > > > > || > > > > > Pipe Blender|| > > > +++ > > > >|| > > > >+---v--+ | > > > >| Pipe DeGamma| | > > > >| | | > > > >+---+--+ | > > > >|Pipe Color | > > > >+---v--+ Hardware| > > > >| Pipe CSC/CTM| | > > > >| | | > > > >+---+--+ | > > > >|| > > > >+---v--+ | > > > >| Pipe Gamma | | > > > >| | | > > > >+---+--+ | > > > >|| > > > +-+ > > > | > > > v > > >Pipe Output > > > > > > This patch series adds properties for plane color features. It adds > > > properties for degamma used to linearize data, CSC used for gamut > > > conversion, and gamma used to again non-linearize data as per panel > > > supported color space. These can be utilize by user space to convert > > > planes from one format to another, one color space to another etc. > > > > > > Usersapce can take smart blending decisions and utilize these hardware > > > supported plane color features to get accurate color profile. The same > > > can help in consistent color quality from source to panel taking > > > advantage of advanced color features in hardware. > > > > > > These patches just add the property interfaces and enable helper > > > functions. > > > > > > This series adds Intel Gen9 specific plane gamma feature. We can build > > > up and add other platform/hardware specific implementation on top of > > > this series > > > > > > Note: This is just to get a design feedback whether these interfaces > > > look ok. Based on community feedback on interfaces, we will implement > > > IGT tests to validate plane color feature
Re: [Intel-gfx] [v7 00/16] Add Plane Color Properties
(+ Boris, + Sean) On Fri, 2019-06-14 at 13:17 -0300, Ezequiel Garcia wrote: > On Thu, 28 Mar 2019 at 16:50, Uma Shankar wrote: > > This is how a typical display color hardware pipeline looks like: > > +---+ > > |RAM| > > | +--++-++-+ | > > | | FB 1 || FB 2 || FB N| | > > | +--++-++-+ | > > +---+ > >| Plane Color Hardware Block | > > ++ > > | +---v-+ +---v---+ +---v--+ | > > | | Plane A | | Plane B | | Plane N | | > > | | DeGamma | | Degamma | | Degamma | | > > | +---+-+ +---+---+ +---+--+ | > > | | | || > > | +---v-+ +---v---+ +---v--+ | > > | |Plane A | | Plane B | | Plane N | | > > | |CSC/CTM | | CSC/CTM | | CSC/CTM | | > > | +---+-+ ++--+ ++-+ | > > | | | | | > > | +---v-+ +v--+ +v-+ | > > | | Plane A | | Plane B | | Plane N | | > > | | Gamma | | Gamma | | Gamma| | > > | +---+-+ ++--+ ++-+ | > > | | | | | > > ++ > > +--v--v---v---| > > > > || > > > > Pipe Blender|| > > +++ > > >|| > > >+---v--+ | > > >| Pipe DeGamma| | > > >| | | > > >+---+--+ | > > >|Pipe Color | > > >+---v--+ Hardware| > > >| Pipe CSC/CTM| | > > >| | | > > >+---+--+ | > > >|| > > >+---v--+ | > > >| Pipe Gamma | | > > >| | | > > >+---+--+ | > > >|| > > +-+ > > | > > v > >Pipe Output > > > > This patch series adds properties for plane color features. It adds > > properties for degamma used to linearize data, CSC used for gamut > > conversion, and gamma used to again non-linearize data as per panel > > supported color space. These can be utilize by user space to convert > > planes from one format to another, one color space to another etc. > > > > Usersapce can take smart blending decisions and utilize these hardware > > supported plane color features to get accurate color profile. The same > > can help in consistent color quality from source to panel taking > > advantage of advanced color features in hardware. > > > > These patches just add the property interfaces and enable helper > > functions. > > > > This series adds Intel Gen9 specific plane gamma feature. We can > > build up and add other platform/hardware specific implementation > > on top of this series > > > > Note: This is just to get a design feedback whether these interfaces > > look ok. Based on community feedback on interfaces, we will implement > > IGT tests to validate plane color features. This is un-tested currently. > > > > Userspace implementation using these properties have been done in drm > > hwcomposer by "Alexandru-Cosmin Gheorghe alexandru-cosmin.gheor...@arm.com" > > from ARM. A merge request has been opened by Alexandru for drm_hwcomposer, > > implementing the property changes for the same. Please review that as well: > > https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/merge_requests/25 > > > > v2: Dropped legacy gamma table for plane as suggested by Maarten. Added > > Gen9/BDW plane gamma feature and rebase on tot. > > > > v3: Added a new drm_color_lut_ext structure to accommodate 32 bit precision > > entries, pointed to by Brian, Starkey for HDR usecases. Addre
Re: [Intel-gfx] [v7 00/16] Add Plane Color Properties
On Thu, 28 Mar 2019 at 16:50, Uma Shankar wrote: > > This is how a typical display color hardware pipeline looks like: > +---+ > |RAM| > | +--++-++-+ | > | | FB 1 || FB 2 || FB N| | > | +--++-++-+ | > +---+ >| Plane Color Hardware Block | > ++ > | +---v-+ +---v---+ +---v--+ | > | | Plane A | | Plane B | | Plane N | | > | | DeGamma | | Degamma | | Degamma | | > | +---+-+ +---+---+ +---+--+ | > | | | || > | +---v-+ +---v---+ +---v--+ | > | |Plane A | | Plane B | | Plane N | | > | |CSC/CTM | | CSC/CTM | | CSC/CTM | | > | +---+-+ ++--+ ++-+ | > | | | | | > | +---v-+ +v--+ +v-+ | > | | Plane A | | Plane B | | Plane N | | > | | Gamma | | Gamma | | Gamma| | > | +---+-+ ++--+ ++-+ | > | | | | | > ++ > +--v--v---v---| > || || > || Pipe Blender|| > +++ > ||| > |+---v--+ | > || Pipe DeGamma| | > || | | > |+---+--+ | > ||Pipe Color | > |+---v--+ Hardware| > || Pipe CSC/CTM| | > || | | > |+---+--+ | > ||| > |+---v--+ | > || Pipe Gamma | | > || | | > |+---+--+ | > ||| > +-+ > | > v >Pipe Output > > This patch series adds properties for plane color features. It adds > properties for degamma used to linearize data, CSC used for gamut > conversion, and gamma used to again non-linearize data as per panel > supported color space. These can be utilize by user space to convert > planes from one format to another, one color space to another etc. > > Usersapce can take smart blending decisions and utilize these hardware > supported plane color features to get accurate color profile. The same > can help in consistent color quality from source to panel taking > advantage of advanced color features in hardware. > > These patches just add the property interfaces and enable helper > functions. > > This series adds Intel Gen9 specific plane gamma feature. We can > build up and add other platform/hardware specific implementation > on top of this series > > Note: This is just to get a design feedback whether these interfaces > look ok. Based on community feedback on interfaces, we will implement > IGT tests to validate plane color features. This is un-tested currently. > > Userspace implementation using these properties have been done in drm > hwcomposer by "Alexandru-Cosmin Gheorghe alexandru-cosmin.gheor...@arm.com" > from ARM. A merge request has been opened by Alexandru for drm_hwcomposer, > implementing the property changes for the same. Please review that as well: > https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/merge_requests/25 > > v2: Dropped legacy gamma table for plane as suggested by Maarten. Added > Gen9/BDW plane gamma feature and rebase on tot. > > v3: Added a new drm_color_lut_ext structure to accommodate 32 bit precision > entries, pointed to by Brian, Starkey for HDR usecases. Addressed Sean,Paul > comments and moved plane color properties to drm_plane instead of > mode_config. Added property documentation as suggested by Daniel, Vetter. > Fixed a rebase fumble which occurred in v2, pointed by Emil Velikov. > > v4: Rebase > > v5: Added "Display Color Hardware Pipeline" flow to kernel > documentation as suggested by "Ville Syrjala" and "Brian Starkey". > Moved the property creation to drm_color_mgmt.c file to consolidate > all color operations at one place. Addressed Alexandru's review comments. > > v6: Rebase. Added support for ICL Color features. Enhanced Lut precision to > accept input values in u32.32 format. This is needed for higher precision > required in HDR data processing. > > v7: Fixed Lut roundup and extraction function in patch 1 and address > definitions for Degamma index in patch
Re: [Intel-gfx] Kernel stability on baytrail machines
Hi Alan, (Adding interested people to this thread) On 09 Apr 08:14 PM, One Thousand Gnomes wrote: > > > I do feel that the importance of the mentioned bug is currently > > > underestimated. Can anyone here give a note, how much current linux > > > kernel is supposed to be stable on general baytrail machines? > > > > If you did not get any replies... you might want to check MAINTAINERS file, > > and > > put Intel x86 maintainers on Cc list. > > > > I'm sure someone cares :-). > > Yes we care, and there are people looking at the various reports. > Are there any updates on the status of this issue? The current bugzilla report [1] marks this as a power management issue. However, many reports indicate that it would only freeze when running X, so it's not completely clear if it's related to the gfx driver too. Also, do we know which CPUs are affect by this issue? and which are NOT affected :) - would be quite relevant in picking a CPU for a product. [1] https://bugzilla.kernel.org/show_bug.cgi?id=109051 -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: i915: Improve behavior in case of broken HDMI EDID
On 22 April 2016 at 14:02, Daniel Vetter <dan...@ffwll.ch> wrote:
> On Fri, Apr 22, 2016 at 12:18:07PM -0300, Ezequiel Garcia wrote:
>> On 22 Apr 10:15 AM, Daniel Vetter wrote:
>> > On Thu, Apr 21, 2016 at 03:13:51PM -0300, Ezequiel Garcia wrote:
>> > > Daniel,
>> > >
>> > > Thanks a lot for the quick reply!
>> > >
>> > > On 20 Apr 01:34 PM, Daniel Vetter wrote:
>> > > > On Tue, Apr 19, 2016 at 02:31:13PM -0300, Ezequiel Garcia wrote:
>> > > > > Currently, our implementation of drm_connector_funcs.detect is
>> > > > > based on getting a valid EDID.
>> > > > >
>> > > > > This requirement makes the driver fail to detect connected
>> > > > > connectors in case of EDID corruption, which in turn prevents
>> > > > > from falling back to modes provided by builtin or user-provided
>> > > > > EDIDs.
>> > > >
>> > > > Imo, this should be fixed in the probe helpers. Something like the
>> > > > below
>> > > > might make sense:
>> > > >
>> > > >
>> > > > diff --git a/drivers/gpu/drm/drm_probe_helper.c
>> > > > b/drivers/gpu/drm/drm_probe_helper.c
>> > > > index e714b5a7955f..d3b9dc7535da 100644
>> > > > --- a/drivers/gpu/drm/drm_probe_helper.c
>> > > > +++ b/drivers/gpu/drm/drm_probe_helper.c
>> > > > @@ -214,7 +214,10 @@ int
>> > > > drm_helper_probe_single_connector_modes(struct drm_connector
>> > > > *connector,
>> > > > else
>> > > > connector->status =
>> > > > connector_status_disconnected;
>> > > > if (connector->funcs->force)
>> > > > - connector->funcs->force(connector);
>> > > > + connector->funcs->force(connector);
>> > > > + } else if (connector->override_edid){
>> > > > + connector->status = connector_status_connected;
>> > > > + connector->funcs->force(connector);
>> > > > } else {
>> > > > connector->status =
>> > > > connector->funcs->detect(connector, true);
>> > > > }
>> > > >
>> > > >
>> > > > It should do what you want it to do, still allow us to override force
>> > > > state manually and also fix things up for every, not just i915-hdmi.
>> > > > Also,
>> > > > much smaller patch.
>> > > >
>> > >
>> > > The patch you are proposing doesn't seem to be related
>> > > to the issue I want to fix, so maybe my explanation is still
>> > > unclear. After re-reading my commit log, I came to realize
>> > > I'm still not explaining the issue properly.
>> > >
>> > > Let me try again :-)
>> > >
>> > > A user can connect any kind of HDMI monitor to the box, and
>> > > the kernel should be able to output some video, even when the
>> > > HDMI monitor is a piece of crap and sends completely broken EDID.
>> > >
>> > > Currently, the i915 driver uses the return value of intel_hdmi_set_edid()
>> > > to set the connector status. IOW, the connector status is set to
>> > > "connected"
>> > > *only* if the EDID is correct, and is left as "disconnected" if the EDID
>> > > is corrupt.
>> > >
>> > > However, the connector status can be detected by just probing
>> > > the I2C bus (drm_probe_ddc).
>> > >
>> > > The DRM probe helper relies on the .detect callback to decide if
>> > > the modes' fallbacks, EDID provided modes, etc are going to be set.
>> > >
>> > > It only set those modes if the .detect callback handler returns
>> > > a "connected" status and does nothing if .detect returns
>> > > "disconnected".
>> > >
>> > > If the connector is reported as "disconnected" it will skip it and
>> > > the user won't be able to use it (except if the state is forced
>> > > with a parameter).
>> > >
>> > > I am currently shipping intel boxes without monitors, and the
>> > > user can connect its own monitor. I can't know before hand
>> > > what dev
Re: [Intel-gfx] [PATCH] drm: i915: Improve behavior in case of broken HDMI EDID
On 22 Apr 10:15 AM, Daniel Vetter wrote:
> On Thu, Apr 21, 2016 at 03:13:51PM -0300, Ezequiel Garcia wrote:
> > Daniel,
> >
> > Thanks a lot for the quick reply!
> >
> > On 20 Apr 01:34 PM, Daniel Vetter wrote:
> > > On Tue, Apr 19, 2016 at 02:31:13PM -0300, Ezequiel Garcia wrote:
> > > > Currently, our implementation of drm_connector_funcs.detect is
> > > > based on getting a valid EDID.
> > > >
> > > > This requirement makes the driver fail to detect connected
> > > > connectors in case of EDID corruption, which in turn prevents
> > > > from falling back to modes provided by builtin or user-provided
> > > > EDIDs.
> > >
> > > Imo, this should be fixed in the probe helpers. Something like the below
> > > might make sense:
> > >
> > >
> > > diff --git a/drivers/gpu/drm/drm_probe_helper.c
> > > b/drivers/gpu/drm/drm_probe_helper.c
> > > index e714b5a7955f..d3b9dc7535da 100644
> > > --- a/drivers/gpu/drm/drm_probe_helper.c
> > > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > > @@ -214,7 +214,10 @@ int drm_helper_probe_single_connector_modes(struct
> > > drm_connector *connector,
> > > else
> > > connector->status = connector_status_disconnected;
> > > if (connector->funcs->force)
> > > - connector->funcs->force(connector);
> > > + connector->funcs->force(connector);
> > > + } else if (connector->override_edid){
> > > + connector->status = connector_status_connected;
> > > + connector->funcs->force(connector);
> > > } else {
> > > connector->status = connector->funcs->detect(connector, true);
> > > }
> > >
> > >
> > > It should do what you want it to do, still allow us to override force
> > > state manually and also fix things up for every, not just i915-hdmi. Also,
> > > much smaller patch.
> > >
> >
> > The patch you are proposing doesn't seem to be related
> > to the issue I want to fix, so maybe my explanation is still
> > unclear. After re-reading my commit log, I came to realize
> > I'm still not explaining the issue properly.
> >
> > Let me try again :-)
> >
> > A user can connect any kind of HDMI monitor to the box, and
> > the kernel should be able to output some video, even when the
> > HDMI monitor is a piece of crap and sends completely broken EDID.
> >
> > Currently, the i915 driver uses the return value of intel_hdmi_set_edid()
> > to set the connector status. IOW, the connector status is set to "connected"
> > *only* if the EDID is correct, and is left as "disconnected" if the EDID
> > is corrupt.
> >
> > However, the connector status can be detected by just probing
> > the I2C bus (drm_probe_ddc).
> >
> > The DRM probe helper relies on the .detect callback to decide if
> > the modes' fallbacks, EDID provided modes, etc are going to be set.
> >
> > It only set those modes if the .detect callback handler returns
> > a "connected" status and does nothing if .detect returns
> > "disconnected".
> >
> > If the connector is reported as "disconnected" it will skip it and
> > the user won't be able to use it (except if the state is forced
> > with a parameter).
> >
> > I am currently shipping intel boxes without monitors, and the
> > user can connect its own monitor. I can't know before hand
> > what device is going to be plugged (neither on which output
> > connector, HDMI, DVI, etc)... so I am not able to force any state.
> >
> > The patch I am proposing makes the kernel work without any
> > user intervention, in the face of corrupted EDID coming out of
> > a monitor. This works because the .detect logic after my patch
> > just checks if a I2C device is present in the bus to mark the
> > connector as "connected" and does not use the EDID parsing for that.
> >
> > In fact, the EDID parsing is moved to .get_modes() since they're
> > not really used before. This at the very least, is consistent with
> > how other drivers work (I'm not inventing anything).
> >
> > Maybe the following commit log is better. How does it look now?
>
> But in that case the only thing you get is the 1024x756 fallback mode.
> You're users are happy with that?
Well, users are happy when things Just Work :-)
> I thought your use-case was
Re: [Intel-gfx] [PATCH] drm: i915: Improve behavior in case of broken HDMI EDID
Daniel,
Thanks a lot for the quick reply!
On 20 Apr 01:34 PM, Daniel Vetter wrote:
> On Tue, Apr 19, 2016 at 02:31:13PM -0300, Ezequiel Garcia wrote:
> > Currently, our implementation of drm_connector_funcs.detect is
> > based on getting a valid EDID.
> >
> > This requirement makes the driver fail to detect connected
> > connectors in case of EDID corruption, which in turn prevents
> > from falling back to modes provided by builtin or user-provided
> > EDIDs.
>
> Imo, this should be fixed in the probe helpers. Something like the below
> might make sense:
>
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index e714b5a7955f..d3b9dc7535da 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -214,7 +214,10 @@ int drm_helper_probe_single_connector_modes(struct
> drm_connector *connector,
> else
> connector->status = connector_status_disconnected;
> if (connector->funcs->force)
> - connector->funcs->force(connector);
> + connector->funcs->force(connector);
> + } else if (connector->override_edid){
> + connector->status = connector_status_connected;
> + connector->funcs->force(connector);
> } else {
> connector->status = connector->funcs->detect(connector, true);
> }
>
>
> It should do what you want it to do, still allow us to override force
> state manually and also fix things up for every, not just i915-hdmi. Also,
> much smaller patch.
>
The patch you are proposing doesn't seem to be related
to the issue I want to fix, so maybe my explanation is still
unclear. After re-reading my commit log, I came to realize
I'm still not explaining the issue properly.
Let me try again :-)
A user can connect any kind of HDMI monitor to the box, and
the kernel should be able to output some video, even when the
HDMI monitor is a piece of crap and sends completely broken EDID.
Currently, the i915 driver uses the return value of intel_hdmi_set_edid()
to set the connector status. IOW, the connector status is set to "connected"
*only* if the EDID is correct, and is left as "disconnected" if the EDID
is corrupt.
However, the connector status can be detected by just probing
the I2C bus (drm_probe_ddc).
The DRM probe helper relies on the .detect callback to decide if
the modes' fallbacks, EDID provided modes, etc are going to be set.
It only set those modes if the .detect callback handler returns
a "connected" status and does nothing if .detect returns
"disconnected".
If the connector is reported as "disconnected" it will skip it and
the user won't be able to use it (except if the state is forced
with a parameter).
I am currently shipping intel boxes without monitors, and the
user can connect its own monitor. I can't know before hand
what device is going to be plugged (neither on which output
connector, HDMI, DVI, etc)... so I am not able to force any state.
The patch I am proposing makes the kernel work without any
user intervention, in the face of corrupted EDID coming out of
a monitor. This works because the .detect logic after my patch
just checks if a I2C device is present in the bus to mark the
connector as "connected" and does not use the EDID parsing for that.
In fact, the EDID parsing is moved to .get_modes() since they're
not really used before. This at the very least, is consistent with
how other drivers work (I'm not inventing anything).
Maybe the following commit log is better. How does it look now?
--8<--
drm: i915: Fix HDMI connector status detection in case of broken EDID
The i915 DRM driver attempts to parse the EDID in the HDMI connector
.detect callback and use the return status of intel_hdmi_set_edid()
to decide if the connector status is connected or disconnected.
The problem is that intel_hdmi_set_edid() fails if the EDID is not
correct (i.e: corrupted) and so .detect will wrongly report to the
DRM core that the connector is disconnected.
It's totally acceptable to use a HDMI connector even in the case of
a broken EDID, by using the CONFIG_DRM_LOAD_EDID_FIRMWARE
and noedid fallback options. The only thing that .detect should
check is that there is a device answering in the correct I2C address
and bus.
So this patch changes the .detect logic to just check the DDC presence
to decide the connector status, so the core can set the EDID fallbacks.
Also, checking for the EDID in the .connect callback doesn't seem to be
correct since that should only check that the connector has been hooked
so this patch also moves the EDID parsing logic to the .get_modes handler
when the modes are actually needed and filled to report to user-space.
--
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm: i915: Improve behavior in case of broken HDMI EDID
Currently, our implementation of drm_connector_funcs.detect is
based on getting a valid EDID.
This requirement makes the driver fail to detect connected
connectors in case of EDID corruption, which in turn prevents
from falling back to modes provided by builtin or user-provided
EDIDs.
Let's fix this by calling drm_probe_ddc in drm_connector_funcs.detect,
and do the EDID full reading and parsing in
drm_connector_helper_funcs.get_modes, when it's actually needed.
This patch allows i915 to take advantage of the DRM_LOAD_EDID_FIRMWARE
infrastructure.
Without this patch, any device that fails to provide a valid
EDID will be reported as disconnected (unless the state is forced)
and thus the kernel won't allow to use such device with any mode,
either builtin, user-provided, or the 1024x768 noedid fallback.
Signed-off-by: Ezequiel Garcia <ezequ...@vanguardiasur.com.ar>
---
This patch supersedes: "drm/i915/hdmi: Fix weak connector detection",
https://patchwork.freedesktop.org/patch/79098/.
drivers/gpu/drm/i915/intel_hdmi.c | 59 ---
1 file changed, 36 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 616108c4bc3e..aa2f2271394a 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1392,36 +1392,17 @@ intel_hdmi_detect(struct drm_connector *connector, bool
force)
enum drm_connector_status status;
struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
struct drm_i915_private *dev_priv = to_i915(connector->dev);
- bool live_status = false;
- unsigned int try;
-
- DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
- connector->base.id, connector->name);
+ struct i2c_adapter *adap;
intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
- for (try = 0; !live_status && try < 9; try++) {
- if (try)
- msleep(10);
- live_status = intel_digital_port_connected(dev_priv,
- hdmi_to_dig_port(intel_hdmi));
- }
-
- if (!live_status)
- DRM_DEBUG_KMS("Live status not up!");
-
- intel_hdmi_unset_edid(connector);
-
- if (intel_hdmi_set_edid(connector, live_status)) {
- struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
-
- hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
+ adap = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
+ if (drm_probe_ddc(adap))
status = connector_status_connected;
- } else
+ else
status = connector_status_disconnected;
intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
-
return status;
}
@@ -1442,10 +1423,42 @@ intel_hdmi_force(struct drm_connector *connector)
hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
}
+static void intel_hdmi_detect_edid(struct drm_connector *connector)
+{
+ struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+ struct drm_i915_private *dev_priv = to_i915(connector->dev);
+ bool live_status = false;
+ unsigned int try;
+
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
+ connector->base.id, connector->name);
+
+ intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
+
+ for (try = 0; !live_status && try < 9; try++) {
+ if (try)
+ msleep(10);
+ live_status = intel_digital_port_connected(dev_priv,
+ hdmi_to_dig_port(intel_hdmi));
+ }
+
+ if (!live_status)
+ DRM_DEBUG_KMS("Live status not up!");
+
+ intel_hdmi_unset_edid(connector);
+ if (intel_hdmi_set_edid(connector, live_status))
+ hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
+
+ intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
+}
+
static int intel_hdmi_get_modes(struct drm_connector *connector)
{
struct edid *edid;
+ if (!to_intel_connector(connector)->detect_edid)
+ intel_hdmi_detect_edid(connector);
+
edid = to_intel_connector(connector)->detect_edid;
if (edid == NULL)
return 0;
--
2.7.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/hdmi: Fix weak connector detection
On 5 April 2016 at 11:54, Ezequiel Garcia <ezequ...@vanguardiasur.com.ar> wrote:
> (Adding Jani again, who got dropped for some reason)
>
> On 1 April 2016 at 16:50, Ezequiel Garcia <ezequ...@vanguardiasur.com.ar>
> wrote:
>> On 01 Apr 06:46 PM, Ville Syrjälä wrote:
>>> On Fri, Apr 01, 2016 at 12:38:11PM -0300, Ezequiel Garcia wrote:
>>> > El abr. 1, 2016 11:47 AM, "Ville Syrjälä" <ville.syrj...@linux.intel.com>
>>> > escribió:
>>> > >
>>> > > On Thu, Mar 31, 2016 at 05:55:03PM -0300, Ezequiel Garcia wrote:
>>> > > > Currently, our implementation of drm_connector_funcs.detect is
>>> > > > based on getting a valid EDID.
>>> > > >
>>> > > > This requirement makes the driver fail to detect connected
>>> > > > connectors in case of EDID corruption, which prevents from falling
>>> > > > back to modes provided by builtin or user-provided EDIDs.
>>> > >
>>> > > So why are you getting corrupted EDIDs?
>>> > >
>>> >
>>> > Does it matter?
>>>
>>> Yes. We should fix the real cause (if possible) instead of adding
>>> more duct tape.
>>>
>>
>> So, there are two things involved in this patch:
>>
>> 1.
>> There are several reasons why EDID can get screwed, this is
>> documented at length [1], and it's the motivation for
>> CONFIG_DRM_LOAD_EDID_FIRMWARE to exist.
>>
>> You can find lots of reports on the internet of people getting
>> corrupt EDID from their monitors. For instance, here's one [2].
>>
>> And even if no firmware is provided using CONFIG_DRM_LOAD_EDID_FIRMWARE,
>> the DRM core will provide a 1024x768 fallback mode:
>>
>> int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>> uint32_t maxX, uint32_t maxY)
>> {
>> [..]
>> if (count == 0 && connector->status == connector_status_connected)
>> count = drm_add_modes_noedid(connector, 1024, 768);
>>
>> But, this only works if the connector is detected.
>>
>> Since I'm interested in backporting this patch to apply it on the kernels
>> I maintain (which are currently deployed on hundreds of machines), I tried
>> to find a simple solution. Hence, this patch.
>>
>> There's no issue to fix here, because broken hardware is a fact of life,
>> and not something we can fix or ignore [3].
>>
>> 2.
>> On the other side, the i915 implementation looks suspicious. IMHO,
>> drm_connector_funcs.detect should not try to read a valid EDID,
>> and just try to detect if the connector is connected or disconnected.
>>
>> The EDID can be read in drm_connector_helper_funcs.get_modes, as other
>> drm/connector drivers are doing (tda998x, tfp410, tegra).
>>
>> However, I think it's safer to get a simple fix now, and do this
>> as follow-up patches.
>>
>> How does it sound?
>>
Are there any other comments regarding this patch?
If at all possible, I'd like to see this merged, or otherwise
a proposal for an alternative solution.
>> [1] Documentation/EDID/HOWTO.txt
>> [2]
>> http://www.blaicher.com/2012/06/howto-fixing-a-broken-edid-eeprom-with-a-bus-pirate-v4/
>> [3] https://marc.info/?l=linux-kernel=112838038415265=4
Thanks,
--
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/hdmi: Fix weak connector detection
(Adding Jani again, who got dropped for some reason)
On 1 April 2016 at 16:50, Ezequiel Garcia <ezequ...@vanguardiasur.com.ar> wrote:
> On 01 Apr 06:46 PM, Ville Syrjälä wrote:
>> On Fri, Apr 01, 2016 at 12:38:11PM -0300, Ezequiel Garcia wrote:
>> > El abr. 1, 2016 11:47 AM, "Ville Syrjälä" <ville.syrj...@linux.intel.com>
>> > escribió:
>> > >
>> > > On Thu, Mar 31, 2016 at 05:55:03PM -0300, Ezequiel Garcia wrote:
>> > > > Currently, our implementation of drm_connector_funcs.detect is
>> > > > based on getting a valid EDID.
>> > > >
>> > > > This requirement makes the driver fail to detect connected
>> > > > connectors in case of EDID corruption, which prevents from falling
>> > > > back to modes provided by builtin or user-provided EDIDs.
>> > >
>> > > So why are you getting corrupted EDIDs?
>> > >
>> >
>> > Does it matter?
>>
>> Yes. We should fix the real cause (if possible) instead of adding
>> more duct tape.
>>
>
> So, there are two things involved in this patch:
>
> 1.
> There are several reasons why EDID can get screwed, this is
> documented at length [1], and it's the motivation for
> CONFIG_DRM_LOAD_EDID_FIRMWARE to exist.
>
> You can find lots of reports on the internet of people getting
> corrupt EDID from their monitors. For instance, here's one [2].
>
> And even if no firmware is provided using CONFIG_DRM_LOAD_EDID_FIRMWARE,
> the DRM core will provide a 1024x768 fallback mode:
>
> int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
> uint32_t maxX, uint32_t maxY)
> {
> [..]
> if (count == 0 && connector->status == connector_status_connected)
> count = drm_add_modes_noedid(connector, 1024, 768);
>
> But, this only works if the connector is detected.
>
> Since I'm interested in backporting this patch to apply it on the kernels
> I maintain (which are currently deployed on hundreds of machines), I tried
> to find a simple solution. Hence, this patch.
>
> There's no issue to fix here, because broken hardware is a fact of life,
> and not something we can fix or ignore [3].
>
> 2.
> On the other side, the i915 implementation looks suspicious. IMHO,
> drm_connector_funcs.detect should not try to read a valid EDID,
> and just try to detect if the connector is connected or disconnected.
>
> The EDID can be read in drm_connector_helper_funcs.get_modes, as other
> drm/connector drivers are doing (tda998x, tfp410, tegra).
>
> However, I think it's safer to get a simple fix now, and do this
> as follow-up patches.
>
> How does it sound?
>
> [1] Documentation/EDID/HOWTO.txt
> [2]
> http://www.blaicher.com/2012/06/howto-fixing-a-broken-edid-eeprom-with-a-bus-pirate-v4/
> [3] https://marc.info/?l=linux-kernel=112838038415265=4
> --
> Ezequiel Garcia, VanguardiaSur
> www.vanguardiasur.com.ar
--
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/hdmi: Fix weak connector detection
On 01 Apr 06:46 PM, Ville Syrjälä wrote:
> On Fri, Apr 01, 2016 at 12:38:11PM -0300, Ezequiel Garcia wrote:
> > El abr. 1, 2016 11:47 AM, "Ville Syrjälä" <ville.syrj...@linux.intel.com>
> > escribió:
> > >
> > > On Thu, Mar 31, 2016 at 05:55:03PM -0300, Ezequiel Garcia wrote:
> > > > Currently, our implementation of drm_connector_funcs.detect is
> > > > based on getting a valid EDID.
> > > >
> > > > This requirement makes the driver fail to detect connected
> > > > connectors in case of EDID corruption, which prevents from falling
> > > > back to modes provided by builtin or user-provided EDIDs.
> > >
> > > So why are you getting corrupted EDIDs?
> > >
> >
> > Does it matter?
>
> Yes. We should fix the real cause (if possible) instead of adding
> more duct tape.
>
So, there are two things involved in this patch:
1.
There are several reasons why EDID can get screwed, this is
documented at length [1], and it's the motivation for
CONFIG_DRM_LOAD_EDID_FIRMWARE to exist.
You can find lots of reports on the internet of people getting
corrupt EDID from their monitors. For instance, here's one [2].
And even if no firmware is provided using CONFIG_DRM_LOAD_EDID_FIRMWARE,
the DRM core will provide a 1024x768 fallback mode:
int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
uint32_t maxX, uint32_t maxY)
{
[..]
if (count == 0 && connector->status == connector_status_connected)
count = drm_add_modes_noedid(connector, 1024, 768);
But, this only works if the connector is detected.
Since I'm interested in backporting this patch to apply it on the kernels
I maintain (which are currently deployed on hundreds of machines), I tried
to find a simple solution. Hence, this patch.
There's no issue to fix here, because broken hardware is a fact of life,
and not something we can fix or ignore [3].
2.
On the other side, the i915 implementation looks suspicious. IMHO,
drm_connector_funcs.detect should not try to read a valid EDID,
and just try to detect if the connector is connected or disconnected.
The EDID can be read in drm_connector_helper_funcs.get_modes, as other
drm/connector drivers are doing (tda998x, tfp410, tegra).
However, I think it's safer to get a simple fix now, and do this
as follow-up patches.
How does it sound?
[1] Documentation/EDID/HOWTO.txt
[2]
http://www.blaicher.com/2012/06/howto-fixing-a-broken-edid-eeprom-with-a-bus-pirate-v4/
[3] https://marc.info/?l=linux-kernel=112838038415265=4
--
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/hdmi: Fix weak connector detection
El abr. 1, 2016 11:47 AM, "Ville Syrjälä" <ville.syrj...@linux.intel.com> escribió: > > On Thu, Mar 31, 2016 at 05:55:03PM -0300, Ezequiel Garcia wrote: > > Currently, our implementation of drm_connector_funcs.detect is > > based on getting a valid EDID. > > > > This requirement makes the driver fail to detect connected > > connectors in case of EDID corruption, which prevents from falling > > back to modes provided by builtin or user-provided EDIDs. > > So why are you getting corrupted EDIDs? > Does it matter? > > > > Let's fix this by improving the detection, with a DDC probe, > > if the current EDID-based detection failed. > > > > Note that a better way of dealing with this could calling > > drm_probe_ddc in drm_connector_funcs.detect, and do the > > EDID full reading and parsing in drm_connector_helper_funcs.get_modes, > > when it's actually needed. > > > > However, this would be more invasive and thus more error-prone. > > The current commit is an attempt to get some uninvasive fix, > > and allow for easier backporting. > > > > Signed-off-by: Ezequiel Garcia <ezequ...@vanguardiasur.com.ar> > > --- > > drivers/gpu/drm/i915/intel_hdmi.c | 14 ++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > > index a0d8daed2470..c079206e6681 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -1428,6 +1428,20 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) > > } else > > status = connector_status_disconnected; > > > > + /* > > + * The above call to intel_hdmi_set_edid() checked for a valid EDID. > > + * However, the EDID can get corrupted for several reasons, resulting > > + * in a disconnected status despite the connector being connected. > > + * Hence, let's try one more time, by only probing the DDC. > > + * > > + * This allows the DRM core to fallback to builtin or user-provided > > + * EDID firmware, e.g. in drm_helper_probe_single_connector_modes. > > + */ > > + if (status == connector_status_disconnected) > > + if (drm_probe_ddc(intel_gmbus_get_adapter(dev_priv, > > + intel_hdmi->ddc_bus))) > > + status = connector_status_connected; > > + > > intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); > > > > return status; > > -- > > 2.7.0 > > > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/hdmi: Fix weak connector detection
Currently, our implementation of drm_connector_funcs.detect is based on getting a valid EDID. This requirement makes the driver fail to detect connected connectors in case of EDID corruption, which prevents from falling back to modes provided by builtin or user-provided EDIDs. Let's fix this by improving the detection, with a DDC probe, if the current EDID-based detection failed. Note that a better way of dealing with this could calling drm_probe_ddc in drm_connector_funcs.detect, and do the EDID full reading and parsing in drm_connector_helper_funcs.get_modes, when it's actually needed. However, this would be more invasive and thus more error-prone. The current commit is an attempt to get some uninvasive fix, and allow for easier backporting. Signed-off-by: Ezequiel Garcia <ezequ...@vanguardiasur.com.ar> --- drivers/gpu/drm/i915/intel_hdmi.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index a0d8daed2470..c079206e6681 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1428,6 +1428,20 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) } else status = connector_status_disconnected; + /* +* The above call to intel_hdmi_set_edid() checked for a valid EDID. +* However, the EDID can get corrupted for several reasons, resulting +* in a disconnected status despite the connector being connected. +* Hence, let's try one more time, by only probing the DDC. +* +* This allows the DRM core to fallback to builtin or user-provided +* EDID firmware, e.g. in drm_helper_probe_single_connector_modes. +*/ + if (status == connector_status_disconnected) + if (drm_probe_ddc(intel_gmbus_get_adapter(dev_priv, + intel_hdmi->ddc_bus))) + status = connector_status_connected; + intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); return status; -- 2.7.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
