[PATCH v2] drm: Only create a cmdline mode if no probed modes match
On Fri, May 22, 2015 at 10:54:12AM +0100, Chris Wilson wrote:
> On Fri, May 22, 2015 at 12:03:27PM +0300, Ville Syrjälä wrote:
> > On Mon, Apr 20, 2015 at 02:28:56PM +0100, Chris Wilson wrote:
> > > The intention of using video=: 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.
> > >
> > > A second issue sneaked into this patch is to add the cmdline mode mode
> > > ahead of the absolute fallback 1024x768 mode. That is if the user has
> > > specified a mode that we create as a fallback, we do not need to add a
> > > second unused fallback mode.
> > >
> > > Fixes regression from
> > >
> > > commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> > > Author: Chris Wilson
> > > 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
> > >
> > > Reported-by: Radek Dostál
> > > Signed-off-by: Chris Wilson
> > > Cc: Radek Dostál
> > > Cc: Jesse Barnes
> > > Cc: Ville Syrjälä
> > > Cc: Daniel Vetter
> > > Cc: dri-devel at lists.freedesktop.org
> > > Cc: Julia Lemire
> > > Cc: Dave Airlie
> > > Cc: stable at vger.kernel.org
> > > ---
> > > drivers/gpu/drm/drm_modes.c| 2 +-
> > > drivers/gpu/drm/drm_probe_helper.c | 39
> > > +++---
> > > 2 files changed, 37 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > > index 213b11ea69b5..13293e009990 100644
> > > --- a/drivers/gpu/drm/drm_modes.c
> > > +++ b/drivers/gpu/drm/drm_modes.c
> > > @@ -1400,7 +1400,7 @@ drm_mode_create_from_cmdline_mode(struct drm_device
> > > *dev,
> > > if (!mode)
> > > return NULL;
> > >
> > > - mode->type |= DRM_MODE_TYPE_USERDEF;
> > > + mode->type |= DRM_MODE_TYPE_USERDEF | DRM_MODE_TYPE_DRIVER;
> >
> > Why do we need the DRIVER flag here?
>
> So we can differentiate it from an equivalent mode added by the user
> later on.
Users can't actually add modes to the connector mode list.
>
> > > + /* Remove the existing fake mode */
> > > + list_for_each_entry(mode, >modes, head) {
> > > + if ((mode->type & (DRM_MODE_TYPE_DRIVER |
> > > DRM_MODE_TYPE_USERDEF)) != (DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_USERDEF))
> > > + continue;
> >
> > Doesn't drm_mode_connector_list_update() kill it from the list
> > eventually if there's no matching mode present on the
> > probed_modes list?
>
> Hmm, that's what I thought I tried at first. If I remember correctly we
> had to set mode->status in order to prune it since
> drm_mode_connector_list_update() itself doesn't do the deletion. Using
> the mode->status was problematic, and the simplest way to do delete the
> original cmdline mode was by explicitly removing it ourselves.
Oh right drm_mode_connector_list_update() only removes the duplicates.
And the the mode->status handling is a bit strange. Not helped by
the fact that MODE_OK is zero so all kzalloced modes start out as
MODE_OK. While I was doing the mode santiy check stuff I did
consider that I should change new modes to be MODE_UNVERIFIED by
default, but I was feeling lazy and decided against it in the end.
So yeah getting the normal prune mechanism to kill the mode would
required some rework to the mode status->handling probably, so
seems like a somewhat bigger effort.
>
> > > @@ -179,9 +212,9 @@ static int
> > > drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
> > > count = (*connector_funcs->get_modes)(connector);
> > > }
> > >
> > > + count += drm_helper_probe_add_cmdline_mode(connector);
> > > if (count == 0 && connector->status == connector_status_connected)
> > > count = drm_add_modes_noedid(connector, 1024, 768);
> > > - count += drm_helper_probe_add_cmdline_mode(connector);
> >
> > Hmm. This means drm_add_modes_noedid() will never be called if the
> > cmdline mode is present, and hence the mode list will only ever have
> >
[PATCH v2] drm: Only create a cmdline mode if no probed modes match
On Mon, Apr 20, 2015 at 02:28:56PM +0100, Chris Wilson wrote:
> The intention of using video=: 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.
>
> A second issue sneaked into this patch is to add the cmdline mode mode
> ahead of the absolute fallback 1024x768 mode. That is if the user has
> specified a mode that we create as a fallback, we do not need to add a
> second unused fallback mode.
>
> Fixes regression from
>
> commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> Author: Chris Wilson
> 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
>
> Reported-by: Radek Dostál
> Signed-off-by: Chris Wilson
> Cc: Radek Dostál
> Cc: Jesse Barnes
> Cc: Ville Syrjälä
> Cc: Daniel Vetter
> Cc: dri-devel at lists.freedesktop.org
> Cc: Julia Lemire
> Cc: Dave Airlie
> Cc: stable at vger.kernel.org
> ---
> drivers/gpu/drm/drm_modes.c| 2 +-
> drivers/gpu/drm/drm_probe_helper.c | 39
> +++---
> 2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 213b11ea69b5..13293e009990 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1400,7 +1400,7 @@ drm_mode_create_from_cmdline_mode(struct drm_device
> *dev,
> if (!mode)
> return NULL;
>
> - mode->type |= DRM_MODE_TYPE_USERDEF;
> + mode->type |= DRM_MODE_TYPE_USERDEF | DRM_MODE_TYPE_DRIVER;
Why do we need the DRIVER flag here?
> drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
> return mode;
> }
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index 63503879a676..2ad8aaf46318 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -79,13 +79,46 @@ 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 = >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, >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;
> +
> + /* Remove the existing fake mode */
> + list_for_each_entry(mode, >modes, head) {
> + if ((mode->type & (DRM_MODE_TYPE_DRIVER |
> DRM_MODE_TYPE_USERDEF)) != (DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_USERDEF))
> + continue;
Doesn't drm_mode_connector_list_update() kill it from the list
eventually if there's no matching mode present on the
probed_modes list?
> +
> + if (mode->hdisplay != cmdline_mode->xres ||
> + mode->vdisplay != cmdline_mode->yres)
> + continue;
> +
> + if (cmdline_mode->refresh_specified &&
> + mode->vrefresh != cmdline_mode->refresh)
> + continue;
> +
> + list_del(>head);
> + drm_mode_destroy(connector->dev, mode);
> + break;
> + }
> +
> + return 0;
> + }
> +
> mode = drm_mode_create_from_cmdline_mode(connector->dev,
> - >cmdline_mode);
> + cmdline_mode);
> if (mode == NULL)
> return 0;
>
> @@ -179,9 +212,9 @@ static int
> drm_helper_probe_single_connector_modes_merge_bits(struct
[PATCH v2] drm: Only create a cmdline mode if no probed modes match
On Fri, May 22, 2015 at 12:03:27PM +0300, Ville Syrjälä wrote:
> On Mon, Apr 20, 2015 at 02:28:56PM +0100, Chris Wilson wrote:
> > The intention of using video=: 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.
> >
> > A second issue sneaked into this patch is to add the cmdline mode mode
> > ahead of the absolute fallback 1024x768 mode. That is if the user has
> > specified a mode that we create as a fallback, we do not need to add a
> > second unused fallback mode.
> >
> > Fixes regression from
> >
> > commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> > Author: Chris Wilson
> > 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
> >
> > Reported-by: Radek Dostál
> > Signed-off-by: Chris Wilson
> > Cc: Radek Dostál
> > Cc: Jesse Barnes
> > Cc: Ville Syrjälä
> > Cc: Daniel Vetter
> > Cc: dri-devel at lists.freedesktop.org
> > Cc: Julia Lemire
> > Cc: Dave Airlie
> > Cc: stable at vger.kernel.org
> > ---
> > drivers/gpu/drm/drm_modes.c| 2 +-
> > drivers/gpu/drm/drm_probe_helper.c | 39
> > +++---
> > 2 files changed, 37 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index 213b11ea69b5..13293e009990 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -1400,7 +1400,7 @@ drm_mode_create_from_cmdline_mode(struct drm_device
> > *dev,
> > if (!mode)
> > return NULL;
> >
> > - mode->type |= DRM_MODE_TYPE_USERDEF;
> > + mode->type |= DRM_MODE_TYPE_USERDEF | DRM_MODE_TYPE_DRIVER;
>
> Why do we need the DRIVER flag here?
So we can differentiate it from an equivalent mode added by the user
later on.
> > + /* Remove the existing fake mode */
> > + list_for_each_entry(mode, >modes, head) {
> > + if ((mode->type & (DRM_MODE_TYPE_DRIVER |
> > DRM_MODE_TYPE_USERDEF)) != (DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_USERDEF))
> > + continue;
>
> Doesn't drm_mode_connector_list_update() kill it from the list
> eventually if there's no matching mode present on the
> probed_modes list?
Hmm, that's what I thought I tried at first. If I remember correctly we
had to set mode->status in order to prune it since
drm_mode_connector_list_update() itself doesn't do the deletion. Using
the mode->status was problematic, and the simplest way to do delete the
original cmdline mode was by explicitly removing it ourselves.
> > @@ -179,9 +212,9 @@ static int
> > drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
> > count = (*connector_funcs->get_modes)(connector);
> > }
> >
> > + count += drm_helper_probe_add_cmdline_mode(connector);
> > if (count == 0 && connector->status == connector_status_connected)
> > count = drm_add_modes_noedid(connector, 1024, 768);
> > - count += drm_helper_probe_add_cmdline_mode(connector);
>
> Hmm. This means drm_add_modes_noedid() will never be called if the
> cmdline mode is present, and hence the mode list will only ever have
> that single mode user specified mode. Not sure if that can be considered
> a real problem or not.
I consider it to a real problem as it goes against my expectations as a
user, that is if I specify a mode to use, I expect that mode to be used.
Doesn't need to be in this patch though.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH v2] drm: Only create a cmdline mode if no probed modes match
On Mon, 20 Apr 2015, Chris Wilson wrote:
> The intention of using video=: 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.
>
> A second issue sneaked into this patch is to add the cmdline mode mode
> ahead of the absolute fallback 1024x768 mode. That is if the user has
> specified a mode that we create as a fallback, we do not need to add a
> second unused fallback mode.
>
> Fixes regression from
>
> commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> Author: Chris Wilson
> 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
>
> Reported-by: Radek Dostál
> Signed-off-by: Chris Wilson
> Cc: Radek Dostál
> Cc: Jesse Barnes
> Cc: Ville Syrjälä
> Cc: Daniel Vetter
> Cc: dri-devel at lists.freedesktop.org
> Cc: Julia Lemire
> Cc: Dave Airlie
> Cc: stable at vger.kernel.org
> ---
> drivers/gpu/drm/drm_modes.c| 2 +-
> drivers/gpu/drm/drm_probe_helper.c | 39
> +++---
> 2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 213b11ea69b5..13293e009990 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1400,7 +1400,7 @@ drm_mode_create_from_cmdline_mode(struct drm_device
> *dev,
> if (!mode)
> return NULL;
>
> - mode->type |= DRM_MODE_TYPE_USERDEF;
> + mode->type |= DRM_MODE_TYPE_USERDEF | DRM_MODE_TYPE_DRIVER;
> drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
> return mode;
> }
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index 63503879a676..2ad8aaf46318 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -79,13 +79,46 @@ 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 = >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, >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;
Would a drm_cmdline_mode_equal() or somesuch be helpful? There's two
copies here, and another variant in drm_pick_cmdline_mode - which also
checks interlace while this one doesn't - why?
> +
> + /* Remove the existing fake mode */
> + list_for_each_entry(mode, >modes, head) {
> + if ((mode->type & (DRM_MODE_TYPE_DRIVER |
> DRM_MODE_TYPE_USERDEF)) != (DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_USERDEF))
> + continue;
> +
> + if (mode->hdisplay != cmdline_mode->xres ||
> + mode->vdisplay != cmdline_mode->yres)
> + continue;
> +
> + if (cmdline_mode->refresh_specified &&
> + mode->vrefresh != cmdline_mode->refresh)
> + continue;
> +
> + list_del(>head);
> + drm_mode_destroy(connector->dev, mode);
drm_mode_remove()
> + break;
> + }
> +
> + return 0;
> + }
> +
> mode = drm_mode_create_from_cmdline_mode(connector->dev,
> - >cmdline_mode);
> + cmdline_mode);
> if (mode == NULL)
> return 0;
>
> @@ -179,9 +212,9 @@ static int
> drm_helper_probe_single_connector_modes_merge_bits(struct
[PATCH v2] drm: Only create a cmdline mode if no probed modes match
On Mon, Apr 20, 2015 at 03:41:48PM +0200, Radek Dostál wrote: > On 04/20/2015 03:28 PM, Chris Wilson wrote: > > The intention of using video=: 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. > > > > A second issue sneaked into this patch is to add the cmdline mode mode > > ahead of the absolute fallback 1024x768 mode. That is if the user has > > specified a mode that we create as a fallback, we do not need to add a > > second unused fallback mode. > > > > Fixes regression from > > > > commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2 > > Author: Chris Wilson > > 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 > > > > Reported-by: Radek Dostál > > Signed-off-by: Chris Wilson > > Cc: Radek Dostál > > Cc: Jesse Barnes > > Cc: Ville Syrjälä > > Cc: Daniel Vetter > > Cc: dri-devel at lists.freedesktop.org > > Cc: Julia Lemire > > Cc: Dave Airlie > > Cc: stable at vger.kernel.org > > works now :) > > Tested-by: Radek Dostál Daniel, Dave do either or you want to pick this up for your fixes tree? -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH v2] drm: Only create a cmdline mode if no probed modes match
On 04/20/2015 03:28 PM, Chris Wilson wrote: > The intention of using video=: 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. > > A second issue sneaked into this patch is to add the cmdline mode mode > ahead of the absolute fallback 1024x768 mode. That is if the user has > specified a mode that we create as a fallback, we do not need to add a > second unused fallback mode. > > Fixes regression from > > commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2 > Author: Chris Wilson > 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 > > Reported-by: Radek Dostál > Signed-off-by: Chris Wilson > Cc: Radek Dostál > Cc: Jesse Barnes > Cc: Ville Syrjälä > Cc: Daniel Vetter > Cc: dri-devel at lists.freedesktop.org > Cc: Julia Lemire > Cc: Dave Airlie > Cc: stable at vger.kernel.org works now :) Tested-by: Radek Dostál Thanks, Radek
[PATCH v2] drm: Only create a cmdline mode if no probed modes match
The intention of using video=: 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.
A second issue sneaked into this patch is to add the cmdline mode mode
ahead of the absolute fallback 1024x768 mode. That is if the user has
specified a mode that we create as a fallback, we do not need to add a
second unused fallback mode.
Fixes regression from
commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
Author: Chris Wilson
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
Reported-by: Radek Dostál
Signed-off-by: Chris Wilson
Cc: Radek Dostál
Cc: Jesse Barnes
Cc: Ville Syrjälä
Cc: Daniel Vetter
Cc: dri-devel at lists.freedesktop.org
Cc: Julia Lemire
Cc: Dave Airlie
Cc: stable at vger.kernel.org
---
drivers/gpu/drm/drm_modes.c| 2 +-
drivers/gpu/drm/drm_probe_helper.c | 39 +++---
2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 213b11ea69b5..13293e009990 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1400,7 +1400,7 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
if (!mode)
return NULL;
- mode->type |= DRM_MODE_TYPE_USERDEF;
+ mode->type |= DRM_MODE_TYPE_USERDEF | DRM_MODE_TYPE_DRIVER;
drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
return mode;
}
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index 63503879a676..2ad8aaf46318 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -79,13 +79,46 @@ 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 = >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, >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;
+
+ /* Remove the existing fake mode */
+ list_for_each_entry(mode, >modes, head) {
+ if ((mode->type & (DRM_MODE_TYPE_DRIVER |
DRM_MODE_TYPE_USERDEF)) != (DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_USERDEF))
+ continue;
+
+ if (mode->hdisplay != cmdline_mode->xres ||
+ mode->vdisplay != cmdline_mode->yres)
+ continue;
+
+ if (cmdline_mode->refresh_specified &&
+ mode->vrefresh != cmdline_mode->refresh)
+ continue;
+
+ list_del(>head);
+ drm_mode_destroy(connector->dev, mode);
+ break;
+ }
+
+ return 0;
+ }
+
mode = drm_mode_create_from_cmdline_mode(connector->dev,
->cmdline_mode);
+cmdline_mode);
if (mode == NULL)
return 0;
@@ -179,9 +212,9 @@ static int
drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
count = (*connector_funcs->get_modes)(connector);
}
+ count += drm_helper_probe_add_cmdline_mode(connector);
if (count == 0 && connector->status == connector_status_connected)
count = drm_add_modes_noedid(connector, 1024, 768);
- count += drm_helper_probe_add_cmdline_mode(connector);
if (count == 0)
goto prune;
