[PATCH v3] drm: Only create a cmdline mode if no probed modes match

[PATCH v3] drm: Only create a cmdline mode if no probed modes match

[Chris Wilson]

The intention of using video=<connector>:<mode> is primarily to select
the user's preferred resolution at startup. Currently we always create a
new mode irrespective of whether the monitor has a native mode at the
desired resolution. This has the issue that we may then select the fake
mode rather the native mode during fb_helper->inital_config() and so
if the fake mode is invalid we then end up with a loss of signal. Oops.
This invalid fake mode would also be exported to userspace, who
potentially may make the same mistake.

To avoid this issue, we filter out the added command line mode if we
detect the desired resolution (and clock if specified) amongst the
probed modes. This fixes the immediate problem of adding a duplicate
mode, but perhaps more generically we should avoid adding a GTF mode if
the monitor has an EDID that is not GTF-compatible, or similarly for
CVT.

Fixes regression from

commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Aug 6 10:08:32 2014 +0200

    drm: Perform cmdline mode parsing during connector initialisation

that breaks HDMI output on BeagleBone Black with LG TV (model 19LS4R-ZA).

v2: Explicitly delete our earlier cmdline mode
v3: Mode pruning should now be sufficient to delete stale cmdline modes

Reported-by: Radek Dostál <rd@radekdostal.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Radek Dostál <rd@radekdostal.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org
Cc: Julia Lemire <jlemire@matrox.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/drm_probe_helper.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 0329080d7f7c..a705ed12c062 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -82,13 +82,28 @@ drm_mode_validate_flag(const struct drm_display_mode *mode,
 
 static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
 {
+	struct drm_cmdline_mode *cmdline_mode;
 	struct drm_display_mode *mode;
 
-	if (!connector->cmdline_mode.specified)
+	cmdline_mode = &connector->cmdline_mode;
+	if (!cmdline_mode->specified)
 		return 0;
 
+	/* Only add a GTF mode if we find no matching probed modes */
+	list_for_each_entry(mode, &connector->probed_modes, head) {
+		if (mode->hdisplay != cmdline_mode->xres ||
+		    mode->vdisplay != cmdline_mode->yres)
+			continue;
+
+		if (cmdline_mode->refresh_specified &&
+		    mode->vrefresh != cmdline_mode->refresh)
+			continue;
+
+		return 0;
+	}
+
 	mode = drm_mode_create_from_cmdline_mode(connector->dev,
-						 &connector->cmdline_mode);
+						 cmdline_mode);
 	if (mode == NULL)
 		return 0;
 
-- 
2.8.1

Re: [PATCH v3] drm: Only create a cmdline mode if no probed modes match

[Ville Syrjälä]

On Wed, Jun 01, 2016 at 10:34:36AM +0100, Chris Wilson wrote:
> The intention of using video=<connector>:<mode> is primarily to select
> the user's preferred resolution at startup. Currently we always create a
> new mode irrespective of whether the monitor has a native mode at the
> desired resolution. This has the issue that we may then select the fake
> mode rather the native mode during fb_helper->inital_config() and so
> if the fake mode is invalid we then end up with a loss of signal. Oops.
> This invalid fake mode would also be exported to userspace, who
> potentially may make the same mistake.
> 
> To avoid this issue, we filter out the added command line mode if we
> detect the desired resolution (and clock if specified) amongst the
> probed modes. This fixes the immediate problem of adding a duplicate
> mode, but perhaps more generically we should avoid adding a GTF mode if
> the monitor has an EDID that is not GTF-compatible, or similarly for
> CVT.
> 
> Fixes regression from
> 
> commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Wed Aug 6 10:08:32 2014 +0200
> 
>     drm: Perform cmdline mode parsing during connector initialisation
> 
> that breaks HDMI output on BeagleBone Black with LG TV (model 19LS4R-ZA).
> 
> v2: Explicitly delete our earlier cmdline mode
> v3: Mode pruning should now be sufficient to delete stale cmdline modes
> 
> Reported-by: Radek Dost�l <rd@radekdostal.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Radek Dost�l <rd@radekdostal.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Julia Lemire <jlemire@matrox.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/drm_probe_helper.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 0329080d7f7c..a705ed12c062 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -82,13 +82,28 @@ drm_mode_validate_flag(const struct drm_display_mode *mode,
>  
>  static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
>  {
> +	struct drm_cmdline_mode *cmdline_mode;
>  	struct drm_display_mode *mode;
>  
> -	if (!connector->cmdline_mode.specified)
> +	cmdline_mode = &connector->cmdline_mode;
> +	if (!cmdline_mode->specified)
>  		return 0;
>  
> +	/* Only add a GTF mode if we find no matching probed modes */
> +	list_for_each_entry(mode, &connector->probed_modes, head) {
> +		if (mode->hdisplay != cmdline_mode->xres ||
> +		    mode->vdisplay != cmdline_mode->yres)
> +			continue;
> +
> +		if (cmdline_mode->refresh_specified &&
> +		    mode->vrefresh != cmdline_mode->refresh)

I think we might not have .vrefresh populated for the probed modes.
We update .vrefresh only for the modes left on the real mode list in the
end.

> +			continue;
> +
> +		return 0;
> +	}
> +
>  	mode = drm_mode_create_from_cmdline_mode(connector->dev,
> -						 &connector->cmdline_mode);
> +						 cmdline_mode);
>  	if (mode == NULL)
>  		return 0;
>  
> -- 
> 2.8.1

-- 
Ville Syrj�l�
Intel OTC

Re: [PATCH v3] drm: Only create a cmdline mode if no probed modes match

[Chris Wilson]

On Wed, Jun 01, 2016 at 12:43:53PM +0300, Ville Syrj�l� wrote:
> On Wed, Jun 01, 2016 at 10:34:36AM +0100, Chris Wilson wrote:
> > The intention of using video=<connector>:<mode> is primarily to select
> > the user's preferred resolution at startup. Currently we always create a
> > new mode irrespective of whether the monitor has a native mode at the
> > desired resolution. This has the issue that we may then select the fake
> > mode rather the native mode during fb_helper->inital_config() and so
> > if the fake mode is invalid we then end up with a loss of signal. Oops.
> > This invalid fake mode would also be exported to userspace, who
> > potentially may make the same mistake.
> > 
> > To avoid this issue, we filter out the added command line mode if we
> > detect the desired resolution (and clock if specified) amongst the
> > probed modes. This fixes the immediate problem of adding a duplicate
> > mode, but perhaps more generically we should avoid adding a GTF mode if
> > the monitor has an EDID that is not GTF-compatible, or similarly for
> > CVT.
> > 
> > Fixes regression from
> > 
> > commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Wed Aug 6 10:08:32 2014 +0200
> > 
> >     drm: Perform cmdline mode parsing during connector initialisation
> > 
> > that breaks HDMI output on BeagleBone Black with LG TV (model 19LS4R-ZA).
> > 
> > v2: Explicitly delete our earlier cmdline mode
> > v3: Mode pruning should now be sufficient to delete stale cmdline modes
> > 
> > Reported-by: Radek Dost�l <rd@radekdostal.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Radek Dost�l <rd@radekdostal.com>
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: Julia Lemire <jlemire@matrox.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: stable@vger.kernel.org
> > ---
> >  drivers/gpu/drm/drm_probe_helper.c | 19 +++++++++++++++++--
> >  1 file changed, 17 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> > index 0329080d7f7c..a705ed12c062 100644
> > --- a/drivers/gpu/drm/drm_probe_helper.c
> > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > @@ -82,13 +82,28 @@ drm_mode_validate_flag(const struct drm_display_mode *mode,
> >  
> >  static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
> >  {
> > +	struct drm_cmdline_mode *cmdline_mode;
> >  	struct drm_display_mode *mode;
> >  
> > -	if (!connector->cmdline_mode.specified)
> > +	cmdline_mode = &connector->cmdline_mode;
> > +	if (!cmdline_mode->specified)
> >  		return 0;
> >  
> > +	/* Only add a GTF mode if we find no matching probed modes */
> > +	list_for_each_entry(mode, &connector->probed_modes, head) {
> > +		if (mode->hdisplay != cmdline_mode->xres ||
> > +		    mode->vdisplay != cmdline_mode->yres)
> > +			continue;
> > +
> > +		if (cmdline_mode->refresh_specified &&
> > +		    mode->vrefresh != cmdline_mode->refresh)
> 
> I think we might not have .vrefresh populated for the probed modes.
> We update .vrefresh only for the modes left on the real mode list in the
> end.

In that case, if cmdline_mode->refresh_specified keep and leave a
comment suggesting we might be able to do better :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH v3] drm: Only create a cmdline mode if no probed modes match

[Chris Wilson]

On Wed, Jun 01, 2016 at 12:43:53PM +0300, Ville Syrj�l� wrote:
> On Wed, Jun 01, 2016 at 10:34:36AM +0100, Chris Wilson wrote:
> > The intention of using video=<connector>:<mode> is primarily to select
> > the user's preferred resolution at startup. Currently we always create a
> > new mode irrespective of whether the monitor has a native mode at the
> > desired resolution. This has the issue that we may then select the fake
> > mode rather the native mode during fb_helper->inital_config() and so
> > if the fake mode is invalid we then end up with a loss of signal. Oops.
> > This invalid fake mode would also be exported to userspace, who
> > potentially may make the same mistake.
> > 
> > To avoid this issue, we filter out the added command line mode if we
> > detect the desired resolution (and clock if specified) amongst the
> > probed modes. This fixes the immediate problem of adding a duplicate
> > mode, but perhaps more generically we should avoid adding a GTF mode if
> > the monitor has an EDID that is not GTF-compatible, or similarly for
> > CVT.
> > 
> > Fixes regression from
> > 
> > commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Wed Aug 6 10:08:32 2014 +0200
> > 
> >     drm: Perform cmdline mode parsing during connector initialisation
> > 
> > that breaks HDMI output on BeagleBone Black with LG TV (model 19LS4R-ZA).
> > 
> > v2: Explicitly delete our earlier cmdline mode
> > v3: Mode pruning should now be sufficient to delete stale cmdline modes
> > 
> > Reported-by: Radek Dost�l <rd@radekdostal.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Radek Dost�l <rd@radekdostal.com>
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: Julia Lemire <jlemire@matrox.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: stable@vger.kernel.org
> > ---
> >  drivers/gpu/drm/drm_probe_helper.c | 19 +++++++++++++++++--
> >  1 file changed, 17 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> > index 0329080d7f7c..a705ed12c062 100644
> > --- a/drivers/gpu/drm/drm_probe_helper.c
> > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > @@ -82,13 +82,28 @@ drm_mode_validate_flag(const struct drm_display_mode *mode,
> >  
> >  static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
> >  {
> > +	struct drm_cmdline_mode *cmdline_mode;
> >  	struct drm_display_mode *mode;
> >  
> > -	if (!connector->cmdline_mode.specified)
> > +	cmdline_mode = &connector->cmdline_mode;
> > +	if (!cmdline_mode->specified)
> >  		return 0;
> >  
> > +	/* Only add a GTF mode if we find no matching probed modes */
> > +	list_for_each_entry(mode, &connector->probed_modes, head) {
> > +		if (mode->hdisplay != cmdline_mode->xres ||
> > +		    mode->vdisplay != cmdline_mode->yres)
> > +			continue;
> > +
> > +		if (cmdline_mode->refresh_specified &&
> > +		    mode->vrefresh != cmdline_mode->refresh)
> 
> I think we might not have .vrefresh populated for the probed modes.
> We update .vrefresh only for the modes left on the real mode list in the
> end.

Or drm_mode_vrefresh() ?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH v4] drm: Only create a cmdline mode if no probed modes match

[Chris Wilson]

The intention of using video=<connector>:<mode> is primarily to select
the user's preferred resolution at startup. Currently we always create a
new mode irrespective of whether the monitor has a native mode at the
desired resolution. This has the issue that we may then select the fake
mode rather the native mode during fb_helper->inital_config() and so
if the fake mode is invalid we then end up with a loss of signal. Oops.
This invalid fake mode would also be exported to userspace, who
potentially may make the same mistake.

To avoid this issue, we filter out the added command line mode if we
detect the desired resolution (and clock if specified) amongst the
probed modes. This fixes the immediate problem of adding a duplicate
mode, but perhaps more generically we should avoid adding a GTF mode if
the monitor has an EDID that is not GTF-compatible, or similarly for
CVT.

Fixes regression from

commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Aug 6 10:08:32 2014 +0200

    drm: Perform cmdline mode parsing during connector initialisation

that breaks HDMI output on BeagleBone Black with LG TV (model 19LS4R-ZA).

v2: Explicitly delete our earlier cmdline mode
v3: Mode pruning should now be sufficient to delete stale cmdline modes
v4: Compute the vrefresh for the probed mode

Reported-by: Radek Dostál <rd@radekdostal.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Radek Dostál <rd@radekdostal.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org
Cc: Julia Lemire <jlemire@matrox.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/drm_probe_helper.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 0329080d7f7c..a0df377d7d1c 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -82,13 +82,30 @@ drm_mode_validate_flag(const struct drm_display_mode *mode,
 
 static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
 {
+	struct drm_cmdline_mode *cmdline_mode;
 	struct drm_display_mode *mode;
 
-	if (!connector->cmdline_mode.specified)
+	cmdline_mode = &connector->cmdline_mode;
+	if (!cmdline_mode->specified)
 		return 0;
 
+	/* Only add a GTF mode if we find no matching probed modes */
+	list_for_each_entry(mode, &connector->probed_modes, head) {
+		if (mode->hdisplay != cmdline_mode->xres ||
+		    mode->vdisplay != cmdline_mode->yres)
+			continue;
+
+		if (cmdline_mode->refresh_specified) {
+			/* The probed mode's vrefresh is set until later */
+			if (drm_mode_vrefresh(mode) != cmdline_mode->refresh)
+				continue;
+		}
+
+		return 0;
+	}
+
 	mode = drm_mode_create_from_cmdline_mode(connector->dev,
-						 &connector->cmdline_mode);
+						 cmdline_mode);
 	if (mode == NULL)
 		return 0;
 
-- 
2.8.1

Re: [PATCH v3] drm: Only create a cmdline mode if no probed modes match

[Ville Syrjälä]

On Wed, Jun 01, 2016 at 10:47:51AM +0100, Chris Wilson wrote:
> On Wed, Jun 01, 2016 at 12:43:53PM +0300, Ville Syrj�l� wrote:
> > On Wed, Jun 01, 2016 at 10:34:36AM +0100, Chris Wilson wrote:
> > > The intention of using video=<connector>:<mode> is primarily to select
> > > the user's preferred resolution at startup. Currently we always create a
> > > new mode irrespective of whether the monitor has a native mode at the
> > > desired resolution. This has the issue that we may then select the fake
> > > mode rather the native mode during fb_helper->inital_config() and so
> > > if the fake mode is invalid we then end up with a loss of signal. Oops.
> > > This invalid fake mode would also be exported to userspace, who
> > > potentially may make the same mistake.
> > > 
> > > To avoid this issue, we filter out the added command line mode if we
> > > detect the desired resolution (and clock if specified) amongst the
> > > probed modes. This fixes the immediate problem of adding a duplicate
> > > mode, but perhaps more generically we should avoid adding a GTF mode if
> > > the monitor has an EDID that is not GTF-compatible, or similarly for
> > > CVT.
> > > 
> > > Fixes regression from
> > > 
> > > commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> > > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > > Date:   Wed Aug 6 10:08:32 2014 +0200
> > > 
> > >     drm: Perform cmdline mode parsing during connector initialisation
> > > 
> > > that breaks HDMI output on BeagleBone Black with LG TV (model 19LS4R-ZA).
> > > 
> > > v2: Explicitly delete our earlier cmdline mode
> > > v3: Mode pruning should now be sufficient to delete stale cmdline modes
> > > 
> > > Reported-by: Radek Dost�l <rd@radekdostal.com>
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Radek Dost�l <rd@radekdostal.com>
> > > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: Julia Lemire <jlemire@matrox.com>
> > > Cc: Dave Airlie <airlied@redhat.com>
> > > Cc: stable@vger.kernel.org
> > > ---
> > >  drivers/gpu/drm/drm_probe_helper.c | 19 +++++++++++++++++--
> > >  1 file changed, 17 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> > > index 0329080d7f7c..a705ed12c062 100644
> > > --- a/drivers/gpu/drm/drm_probe_helper.c
> > > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > > @@ -82,13 +82,28 @@ drm_mode_validate_flag(const struct drm_display_mode *mode,
> > >  
> > >  static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
> > >  {
> > > +	struct drm_cmdline_mode *cmdline_mode;
> > >  	struct drm_display_mode *mode;
> > >  
> > > -	if (!connector->cmdline_mode.specified)
> > > +	cmdline_mode = &connector->cmdline_mode;
> > > +	if (!cmdline_mode->specified)
> > >  		return 0;
> > >  
> > > +	/* Only add a GTF mode if we find no matching probed modes */
> > > +	list_for_each_entry(mode, &connector->probed_modes, head) {
> > > +		if (mode->hdisplay != cmdline_mode->xres ||
> > > +		    mode->vdisplay != cmdline_mode->yres)
> > > +			continue;
> > > +
> > > +		if (cmdline_mode->refresh_specified &&
> > > +		    mode->vrefresh != cmdline_mode->refresh)
> > 
> > I think we might not have .vrefresh populated for the probed modes.
> > We update .vrefresh only for the modes left on the real mode list in the
> > end.
> 
> Or drm_mode_vrefresh() ?

That should work.

-- 
Ville Syrj�l�
Intel OTC

Re: [PATCH v4] drm: Only create a cmdline mode if no probed modes match

[Alex Deucher]

On Wed, Jun 1, 2016 at 5:50 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> The intention of using video=<connector>:<mode> is primarily to select
> the user's preferred resolution at startup. Currently we always create a
> new mode irrespective of whether the monitor has a native mode at the
> desired resolution. This has the issue that we may then select the fake
> mode rather the native mode during fb_helper->inital_config() and so
> if the fake mode is invalid we then end up with a loss of signal. Oops.
> This invalid fake mode would also be exported to userspace, who
> potentially may make the same mistake.
>
> To avoid this issue, we filter out the added command line mode if we
> detect the desired resolution (and clock if specified) amongst the
> probed modes. This fixes the immediate problem of adding a duplicate
> mode, but perhaps more generically we should avoid adding a GTF mode if
> the monitor has an EDID that is not GTF-compatible, or similarly for
> CVT.
>
> Fixes regression from
>
> commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Wed Aug 6 10:08:32 2014 +0200
>
>     drm: Perform cmdline mode parsing during connector initialisation
>
> that breaks HDMI output on BeagleBone Black with LG TV (model 19LS4R-ZA).
>
> v2: Explicitly delete our earlier cmdline mode
> v3: Mode pruning should now be sufficient to delete stale cmdline modes
> v4: Compute the vrefresh for the probed mode

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>
> Reported-by: Radek Dostál <rd@radekdostal.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Radek Dostál <rd@radekdostal.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Julia Lemire <jlemire@matrox.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/drm_probe_helper.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 0329080d7f7c..a0df377d7d1c 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -82,13 +82,30 @@ drm_mode_validate_flag(const struct drm_display_mode *mode,
>
>  static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
>  {
> +       struct drm_cmdline_mode *cmdline_mode;
>         struct drm_display_mode *mode;
>
> -       if (!connector->cmdline_mode.specified)
> +       cmdline_mode = &connector->cmdline_mode;
> +       if (!cmdline_mode->specified)
>                 return 0;
>
> +       /* Only add a GTF mode if we find no matching probed modes */
> +       list_for_each_entry(mode, &connector->probed_modes, head) {
> +               if (mode->hdisplay != cmdline_mode->xres ||
> +                   mode->vdisplay != cmdline_mode->yres)
> +                       continue;
> +
> +               if (cmdline_mode->refresh_specified) {
> +                       /* The probed mode's vrefresh is set until later */
> +                       if (drm_mode_vrefresh(mode) != cmdline_mode->refresh)
> +                               continue;
> +               }
> +
> +               return 0;
> +       }
> +
>         mode = drm_mode_create_from_cmdline_mode(connector->dev,
> -                                                &connector->cmdline_mode);
> +                                                cmdline_mode);
>         if (mode == NULL)
>                 return 0;
>
> --
> 2.8.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4] drm: Only create a cmdline mode if no probed modes match

[Radek Dostál]

On 06/01/2016 11:50 AM, Chris Wilson wrote:
> Fixes regression from
>
> commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> Author: Chris Wilson<chris@chris-wilson.co.uk>
> Date:   Wed Aug 6 10:08:32 2014 +0200
>
>      drm: Perform cmdline mode parsing during connector initialisation
>
> that breaks HDMI output on BeagleBone Black with LG TV (model 19LS4R-ZA).

please remove this from the commit message. The original bug is no 
longer reproducible with 4.7-rc1

Thanks,
Radek

Re: [PATCH v4] drm: Only create a cmdline mode if no probed modes match

[Chris Wilson]

On Thu, Jun 02, 2016 at 11:38:26AM +0200, Radek Dost�l wrote:
> On 06/01/2016 11:50 AM, Chris Wilson wrote:
> >Fixes regression from
> >
> >commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> >Author: Chris Wilson<chris@chris-wilson.co.uk>
> >Date:   Wed Aug 6 10:08:32 2014 +0200
> >
> >     drm: Perform cmdline mode parsing during connector initialisation
> >
> >that breaks HDMI output on BeagleBone Black with LG TV (model 19LS4R-ZA).
> 
> please remove this from the commit message. The original bug is no
> longer reproducible with 4.7-rc1

If there's no motivation for the patch anymore, it can just wither away
in one of my old trees.

Does anyone care about pruning the autogenerated video= mode if a probed
one matches? Presumably, it is still visible to userspace and switching
to it will cause the same issue as before? Or was it always a driver
bug (failing to set the mode)?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH v4] drm: Only create a cmdline mode if no probed modes match

[Ville Syrjälä]

On Thu, Jun 02, 2016 at 11:52:17AM +0100, Chris Wilson wrote:
> On Thu, Jun 02, 2016 at 11:38:26AM +0200, Radek Dost�l wrote:
> > On 06/01/2016 11:50 AM, Chris Wilson wrote:
> > >Fixes regression from
> > >
> > >commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> > >Author: Chris Wilson<chris@chris-wilson.co.uk>
> > >Date:   Wed Aug 6 10:08:32 2014 +0200
> > >
> > >     drm: Perform cmdline mode parsing during connector initialisation
> > >
> > >that breaks HDMI output on BeagleBone Black with LG TV (model 19LS4R-ZA).
> > 
> > please remove this from the commit message. The original bug is no
> > longer reproducible with 4.7-rc1
> 
> If there's no motivation for the patch anymore, it can just wither away
> in one of my old trees.
> 
> Does anyone care about pruning the autogenerated video= mode if a probed
> one matches? Presumably, it is still visible to userspace and switching
> to it will cause the same issue as before? Or was it always a driver
> bug (failing to set the mode)?

IMO the patch makes total sense even if it's not needed for this
particular bug. Feel free to add

Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>

-- 
Ville Syrj�l�
Intel OTC

Re: [PATCH v4] drm: Only create a cmdline mode if no probed modes match

[Radek Dostál]

On 06/02/2016 01:30 PM, Ville Syrj�l� wrote:
> IMO the patch makes total sense even if it's not needed for this
> particular bug. Feel free to add

I agree and additionally can confirm, that with this patch BBB still 
works as expected with LG 19LS4R-ZA.

Thanks,
Radek

Re: [PATCH v4] drm: Only create a cmdline mode if no probed modes match

[Daniel Vetter]

On Thu, Jun 02, 2016 at 02:30:40PM +0300, Ville Syrj�l� wrote:
> On Thu, Jun 02, 2016 at 11:52:17AM +0100, Chris Wilson wrote:
> > On Thu, Jun 02, 2016 at 11:38:26AM +0200, Radek Dost�l wrote:
> > > On 06/01/2016 11:50 AM, Chris Wilson wrote:
> > > >Fixes regression from
> > > >
> > > >commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> > > >Author: Chris Wilson<chris@chris-wilson.co.uk>
> > > >Date:   Wed Aug 6 10:08:32 2014 +0200
> > > >
> > > >     drm: Perform cmdline mode parsing during connector initialisation
> > > >
> > > >that breaks HDMI output on BeagleBone Black with LG TV (model 19LS4R-ZA).
> > > 
> > > please remove this from the commit message. The original bug is no
> > > longer reproducible with 4.7-rc1
> > 
> > If there's no motivation for the patch anymore, it can just wither away
> > in one of my old trees.
> > 
> > Does anyone care about pruning the autogenerated video= mode if a probed
> > one matches? Presumably, it is still visible to userspace and switching
> > to it will cause the same issue as before? Or was it always a driver
> > bug (failing to set the mode)?
> 
> IMO the patch makes total sense even if it's not needed for this
> particular bug. Feel free to add
> 
> Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>

Agreed. I dropped the cc: stable and adjusted the commit message to
explain the situation. Merged to drm-misc, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch