[PATCH 1/7] drm: Don't overwrite UNVERFIED mode status to OK

stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 1/7] drm: Don't overwrite UNVERFIED mode status to OK
       [not found] <1449177255-9515-1-git-send-email-ville.syrjala@linux.intel.com>
@ 2015-12-03 21:14 ` ville.syrjala
  2015-12-04  8:17   ` Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: ville.syrjala @ 2015-12-03 21:14 UTC (permalink / raw)
  To: dri-devel; +Cc: stable, Adam Jackson

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The way the mode probing works is this:
1. All modes currently on the mode list are marked as UNVERIFIED
2. New modes are on the probed_modes list (they start with
   status OK)
3. Modes are moved from the probed_modes list to the actual
   mode list. If a mode already on the mode list is deemed
   to match one of the probed modes, the duplicate is dropped
   and the mode status updated to OK. After this the
   probed_modes list will be empty.
4. All modes on the mode list are verified to not violate any
   constraints. Any that do are marked as such.
5. Any mode left with a non-OK status is pruned from the list,
   with an appropriate debug message.

What all this means is that any mode on the original list that
didn't have a duplicate on the probed_modes list, should be left
with status UNVERFIED (or previously could have been left with
some other status, but never OK).

I broke that in
commit 05acaec334fc ("drm: Reorganize probed mode validation")
by always assigning something to the mode->status during the validation
step. So any mode from the old list that still passed the validation
would be left on the list with status OK in the end.

Fix this by not doing the basic mode validation unless the mode
already has status OK (meaning it came from the probed_modes list,
or at least a duplicate of it was on that list). This way we will
correctly prune away any mode from the old mode list that didn't
appear on the probed_modes list.

Cc: stable@vger.kernel.org
Cc: Adam Jackson <ajax@redhat.com>
Fixes: 05acaec334fc ("drm: Reorganize probed mode validation")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_probe_helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 94ba39e34299..b9b3bd9349ff 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -229,7 +229,8 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
 		mode_flags |= DRM_MODE_FLAG_3D_MASK;

 	list_for_each_entry(mode, &connector->modes, head) {
-		mode->status = drm_mode_validate_basic(mode);
+		if (mode->status == MODE_OK)
+			mode->status = drm_mode_validate_basic(mode);

 		if (mode->status == MODE_OK)
 			mode->status = drm_mode_validate_size(mode, maxX, maxY);
-- 
2.4.10

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/7] drm: Don't overwrite UNVERFIED mode status to OK
  2015-12-03 21:14 ` [PATCH 1/7] drm: Don't overwrite UNVERFIED mode status to OK ville.syrjala
@ 2015-12-04  8:17   ` Daniel Vetter
  2015-12-04 12:23     ` Ville Syrjälä
  2015-12-10 21:07     ` Ville Syrjälä
  0 siblings, 2 replies; 4+ messages in thread
From: Daniel Vetter @ 2015-12-04  8:17 UTC (permalink / raw)
  To: ville.syrjala; +Cc: dri-devel, stable

On Thu, Dec 03, 2015 at 11:14:09PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjï¿½lï¿½ <ville.syrjala@linux.intel.com>
> 
> The way the mode probing works is this:
> 1. All modes currently on the mode list are marked as UNVERIFIED
> 2. New modes are on the probed_modes list (they start with
>    status OK)
> 3. Modes are moved from the probed_modes list to the actual
>    mode list. If a mode already on the mode list is deemed
>    to match one of the probed modes, the duplicate is dropped
>    and the mode status updated to OK. After this the
>    probed_modes list will be empty.
> 4. All modes on the mode list are verified to not violate any
>    constraints. Any that do are marked as such.
> 5. Any mode left with a non-OK status is pruned from the list,
>    with an appropriate debug message.

This would look really pretty as a kerneldoc addition to
probe_single_connector(). And with asciidoc we can even do pretty ordered
lists like these. Can you please follow-up with a patch for that?

> 
> What all this means is that any mode on the original list that
> didn't have a duplicate on the probed_modes list, should be left
> with status UNVERFIED (or previously could have been left with
> some other status, but never OK).
> 
> I broke that in
> commit 05acaec334fc ("drm: Reorganize probed mode validation")
> by always assigning something to the mode->status during the validation
> step. So any mode from the old list that still passed the validation
> would be left on the list with status OK in the end.
> 
> Fix this by not doing the basic mode validation unless the mode
> already has status OK (meaning it came from the probed_modes list,
> or at least a duplicate of it was on that list). This way we will
> correctly prune away any mode from the old mode list that didn't
> appear on the probed_modes list.
> 
> Cc: stable@vger.kernel.org
> Cc: Adam Jackson <ajax@redhat.com>
> Fixes: 05acaec334fc ("drm: Reorganize probed mode validation")
> Signed-off-by: Ville Syrjï¿½lï¿½ <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_probe_helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 94ba39e34299..b9b3bd9349ff 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -229,7 +229,8 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
>  		mode_flags |= DRM_MODE_FLAG_3D_MASK;
>  
>  	list_for_each_entry(mode, &connector->modes, head) {
> -		mode->status = drm_mode_validate_basic(mode);
> +		if (mode->status == MODE_OK)
> +			mode->status = drm_mode_validate_basic(mode);
>  
>  		if (mode->status == MODE_OK)
>  			mode->status = drm_mode_validate_size(mode, maxX, maxY);
> -- 
> 2.4.10
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/7] drm: Don't overwrite UNVERFIED mode status to OK
  2015-12-04  8:17   ` Daniel Vetter
@ 2015-12-04 12:23     ` Ville Syrjälä
  2015-12-10 21:07     ` Ville Syrjälä
  1 sibling, 0 replies; 4+ messages in thread
From: Ville Syrjälä @ 2015-12-04 12:23 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel, stable

On Fri, Dec 04, 2015 at 09:17:28AM +0100, Daniel Vetter wrote:
> On Thu, Dec 03, 2015 at 11:14:09PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjï¿½lï¿½ <ville.syrjala@linux.intel.com>
> > 
> > The way the mode probing works is this:
> > 1. All modes currently on the mode list are marked as UNVERIFIED
> > 2. New modes are on the probed_modes list (they start with
> >    status OK)
> > 3. Modes are moved from the probed_modes list to the actual
> >    mode list. If a mode already on the mode list is deemed
> >    to match one of the probed modes, the duplicate is dropped
> >    and the mode status updated to OK. After this the
> >    probed_modes list will be empty.
> > 4. All modes on the mode list are verified to not violate any
> >    constraints. Any that do are marked as such.
> > 5. Any mode left with a non-OK status is pruned from the list,
> >    with an appropriate debug message.
> 
> This would look really pretty as a kerneldoc addition to
> probe_single_connector(). And with asciidoc we can even do pretty ordered
> lists like these. Can you please follow-up with a patch for that?

I can try.

> 
> > 
> > What all this means is that any mode on the original list that
> > didn't have a duplicate on the probed_modes list, should be left
> > with status UNVERFIED (or previously could have been left with
> > some other status, but never OK).
> > 
> > I broke that in
> > commit 05acaec334fc ("drm: Reorganize probed mode validation")
> > by always assigning something to the mode->status during the validation
> > step. So any mode from the old list that still passed the validation
> > would be left on the list with status OK in the end.
> > 
> > Fix this by not doing the basic mode validation unless the mode
> > already has status OK (meaning it came from the probed_modes list,
> > or at least a duplicate of it was on that list). This way we will
> > correctly prune away any mode from the old mode list that didn't
> > appear on the probed_modes list.
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Adam Jackson <ajax@redhat.com>
> > Fixes: 05acaec334fc ("drm: Reorganize probed mode validation")
> > Signed-off-by: Ville Syrjï¿½lï¿½ <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > ---
> >  drivers/gpu/drm/drm_probe_helper.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> > index 94ba39e34299..b9b3bd9349ff 100644
> > --- a/drivers/gpu/drm/drm_probe_helper.c
> > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > @@ -229,7 +229,8 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
> >  		mode_flags |= DRM_MODE_FLAG_3D_MASK;
> >  
> >  	list_for_each_entry(mode, &connector->modes, head) {
> > -		mode->status = drm_mode_validate_basic(mode);
> > +		if (mode->status == MODE_OK)
> > +			mode->status = drm_mode_validate_basic(mode);
> >  
> >  		if (mode->status == MODE_OK)
> >  			mode->status = drm_mode_validate_size(mode, maxX, maxY);
> > -- 
> > 2.4.10
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjï¿½lï¿½
Intel OTC

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/7] drm: Don't overwrite UNVERFIED mode status to OK
  2015-12-04  8:17   ` Daniel Vetter
  2015-12-04 12:23     ` Ville Syrjälä
@ 2015-12-10 21:07     ` Ville Syrjälä
  1 sibling, 0 replies; 4+ messages in thread
From: Ville Syrjälä @ 2015-12-10 21:07 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel, stable

On Fri, Dec 04, 2015 at 09:17:28AM +0100, Daniel Vetter wrote:
> On Thu, Dec 03, 2015 at 11:14:09PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjï¿½lï¿½ <ville.syrjala@linux.intel.com>
> > 
> > The way the mode probing works is this:
> > 1. All modes currently on the mode list are marked as UNVERIFIED
> > 2. New modes are on the probed_modes list (they start with
> >    status OK)
> > 3. Modes are moved from the probed_modes list to the actual
> >    mode list. If a mode already on the mode list is deemed
> >    to match one of the probed modes, the duplicate is dropped
> >    and the mode status updated to OK. After this the
> >    probed_modes list will be empty.
> > 4. All modes on the mode list are verified to not violate any
> >    constraints. Any that do are marked as such.
> > 5. Any mode left with a non-OK status is pruned from the list,
> >    with an appropriate debug message.
> 
> This would look really pretty as a kerneldoc addition to
> probe_single_connector(). And with asciidoc we can even do pretty ordered
> lists like these. Can you please follow-up with a patch for that?
> 
> > 
> > What all this means is that any mode on the original list that
> > didn't have a duplicate on the probed_modes list, should be left
> > with status UNVERFIED (or previously could have been left with
> > some other status, but never OK).
> > 
> > I broke that in
> > commit 05acaec334fc ("drm: Reorganize probed mode validation")
> > by always assigning something to the mode->status during the validation
> > step. So any mode from the old list that still passed the validation
> > would be left on the list with status OK in the end.
> > 
> > Fix this by not doing the basic mode validation unless the mode
> > already has status OK (meaning it came from the probed_modes list,
> > or at least a duplicate of it was on that list). This way we will
> > correctly prune away any mode from the old mode list that didn't
> > appear on the probed_modes list.
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Adam Jackson <ajax@redhat.com>
> > Fixes: 05acaec334fc ("drm: Reorganize probed mode validation")
> > Signed-off-by: Ville Syrjï¿½lï¿½ <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Testcase: igt/kms_force_connector_basic/prune-stale-modes
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93332

> 
> > ---
> >  drivers/gpu/drm/drm_probe_helper.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> > index 94ba39e34299..b9b3bd9349ff 100644
> > --- a/drivers/gpu/drm/drm_probe_helper.c
> > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > @@ -229,7 +229,8 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
> >  		mode_flags |= DRM_MODE_FLAG_3D_MASK;
> >  
> >  	list_for_each_entry(mode, &connector->modes, head) {
> > -		mode->status = drm_mode_validate_basic(mode);
> > +		if (mode->status == MODE_OK)
> > +			mode->status = drm_mode_validate_basic(mode);
> >  
> >  		if (mode->status == MODE_OK)
> >  			mode->status = drm_mode_validate_size(mode, maxX, maxY);
> > -- 
> > 2.4.10
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjï¿½lï¿½
Intel OTC

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-12-10 21:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1449177255-9515-1-git-send-email-ville.syrjala@linux.intel.com>
2015-12-03 21:14 ` [PATCH 1/7] drm: Don't overwrite UNVERFIED mode status to OK ville.syrjala
2015-12-04  8:17   ` Daniel Vetter
2015-12-04 12:23     ` Ville Syrjälä
2015-12-10 21:07     ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).