[PATCH 0/7] drm: Fix mode pruning

[PATCH 0/7] drm: Fix mode pruning

[ville.syrjala]

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

Turns out I broke mode pruning when the connector mode lists changes
without the connector getting disconnected in between. Mostly a problem
for i-g-t EDID forcing stuff I suppose, but maybe someone is fast enough
that they can swap cables without the system noticing until the new
cable is plugged in.

Anyway here's the fix, and I also ended up reviewing the way we merge
new and old modes together, and made some changes there.

Ville Syrjälä (7):
  drm: Don't overwrite UNVERFIED mode status to OK
  drm: Rename MODE_UNVERIFIED to MODE_STALE
  drm: Reindent enum drm_mode_status
  drm: Flatten drm_mode_connector_list_update() a bit
  drm: Only merge mode type bits between new probed modes
  drm: Drop drm_helper_probe_single_connector_modes_nomerge()
  drm/sti: Drop bogus drm_mode_sort() call

 drivers/gpu/drm/drm_modes.c              | 56 +++++++++++++---------
 drivers/gpu/drm/drm_probe_helper.c       | 72 ++++++++++------------------
 drivers/gpu/drm/qxl/qxl_display.c        |  2 +-
 drivers/gpu/drm/sti/sti_hda.c            |  2 -
 drivers/gpu/drm/virtio/virtgpu_display.c |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c      |  2 +-
 include/drm/drm_crtc_helper.h            |  4 --
 include/drm/drm_modes.h                  | 80 ++++++++++++++++----------------
 8 files changed, 103 insertions(+), 117 deletions(-)

-- 
2.4.10

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[ville.syrjala]

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

[PATCH 2/7] drm: Rename MODE_UNVERIFIED to MODE_STALE

[ville.syrjala]

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

MODE_UNVERIFIED actually means that the mode came from a previous probe,
and if the new probe doesn't produce a matching mode it will get pruned
from the list. Rename the flag to MODE_STALE to better convey the
meaning.

Cc: Adam Jackson <ajax@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_modes.c        | 2 +-
 drivers/gpu/drm/drm_probe_helper.c | 4 ++--
 include/drm/drm_modes.h            | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 5f79b9334a38..824125b3337f 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1073,7 +1073,7 @@ static const char * const drm_mode_status_names[] = {
 	MODE_STATUS(ONE_SIZE),
 	MODE_STATUS(NO_REDUCED),
 	MODE_STATUS(NO_STEREO),
-	MODE_STATUS(UNVERIFIED),
+	MODE_STATUS(STALE),
 	MODE_STATUS(BAD),
 	MODE_STATUS(ERROR),
 };
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index b9b3bd9349ff..2e0c8bfacd35 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -143,9 +143,9 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
 			connector->name);
-	/* set all modes to the unverified state */
+	/* set all old modes to the stale state */
 	list_for_each_entry(mode, &connector->modes, head)
-		mode->status = MODE_UNVERIFIED;
+		mode->status = MODE_STALE;
 
 	old_status = connector->status;
 
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index f9115aee43f4..71801229ce9d 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -72,7 +72,7 @@ enum drm_mode_status {
     MODE_ONE_SIZE,      /* only one resolution is supported */
     MODE_NO_REDUCED,    /* monitor doesn't accept reduced blanking */
     MODE_NO_STEREO,	/* stereo modes not supported */
-    MODE_UNVERIFIED = -3, /* mode needs to reverified */
+    MODE_STALE = -3,	/* mode has become stale */
     MODE_BAD = -2,	/* unspecified reason */
     MODE_ERROR	= -1	/* error condition */
 };
-- 
2.4.10

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/7] drm: Reindent enum drm_mode_status

[ville.syrjala]

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

There's some random mix of spaces and tabs used within the enum
drm_mode_status definition. Fix it up to use just tabs.

Cc: Adam Jackson <ajax@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 include/drm/drm_modes.h | 78 ++++++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 39 deletions(-)

diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 71801229ce9d..256a1bbb125c 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -36,45 +36,45 @@
  */
 
 enum drm_mode_status {
-    MODE_OK	= 0,	/* Mode OK */
-    MODE_HSYNC,		/* hsync out of range */
-    MODE_VSYNC,		/* vsync out of range */
-    MODE_H_ILLEGAL,	/* mode has illegal horizontal timings */
-    MODE_V_ILLEGAL,	/* mode has illegal horizontal timings */
-    MODE_BAD_WIDTH,	/* requires an unsupported linepitch */
-    MODE_NOMODE,	/* no mode with a matching name */
-    MODE_NO_INTERLACE,	/* interlaced mode not supported */
-    MODE_NO_DBLESCAN,	/* doublescan mode not supported */
-    MODE_NO_VSCAN,	/* multiscan mode not supported */
-    MODE_MEM,		/* insufficient video memory */
-    MODE_VIRTUAL_X,	/* mode width too large for specified virtual size */
-    MODE_VIRTUAL_Y,	/* mode height too large for specified virtual size */
-    MODE_MEM_VIRT,	/* insufficient video memory given virtual size */
-    MODE_NOCLOCK,	/* no fixed clock available */
-    MODE_CLOCK_HIGH,	/* clock required is too high */
-    MODE_CLOCK_LOW,	/* clock required is too low */
-    MODE_CLOCK_RANGE,	/* clock/mode isn't in a ClockRange */
-    MODE_BAD_HVALUE,	/* horizontal timing was out of range */
-    MODE_BAD_VVALUE,	/* vertical timing was out of range */
-    MODE_BAD_VSCAN,	/* VScan value out of range */
-    MODE_HSYNC_NARROW,	/* horizontal sync too narrow */
-    MODE_HSYNC_WIDE,	/* horizontal sync too wide */
-    MODE_HBLANK_NARROW,	/* horizontal blanking too narrow */
-    MODE_HBLANK_WIDE,	/* horizontal blanking too wide */
-    MODE_VSYNC_NARROW,	/* vertical sync too narrow */
-    MODE_VSYNC_WIDE,	/* vertical sync too wide */
-    MODE_VBLANK_NARROW,	/* vertical blanking too narrow */
-    MODE_VBLANK_WIDE,	/* vertical blanking too wide */
-    MODE_PANEL,         /* exceeds panel dimensions */
-    MODE_INTERLACE_WIDTH, /* width too large for interlaced mode */
-    MODE_ONE_WIDTH,     /* only one width is supported */
-    MODE_ONE_HEIGHT,    /* only one height is supported */
-    MODE_ONE_SIZE,      /* only one resolution is supported */
-    MODE_NO_REDUCED,    /* monitor doesn't accept reduced blanking */
-    MODE_NO_STEREO,	/* stereo modes not supported */
-    MODE_STALE = -3,	/* mode has become stale */
-    MODE_BAD = -2,	/* unspecified reason */
-    MODE_ERROR	= -1	/* error condition */
+	MODE_OK	= 0,		/* Mode OK */
+	MODE_HSYNC,		/* hsync out of range */
+	MODE_VSYNC,		/* vsync out of range */
+	MODE_H_ILLEGAL,		/* mode has illegal horizontal timings */
+	MODE_V_ILLEGAL,		/* mode has illegal horizontal timings */
+	MODE_BAD_WIDTH,		/* requires an unsupported linepitch */
+	MODE_NOMODE,		/* no mode with a matching name */
+	MODE_NO_INTERLACE,	/* interlaced mode not supported */
+	MODE_NO_DBLESCAN,	/* doublescan mode not supported */
+	MODE_NO_VSCAN,		/* multiscan mode not supported */
+	MODE_MEM,		/* insufficient video memory */
+	MODE_VIRTUAL_X,		/* mode width too large for specified virtual size */
+	MODE_VIRTUAL_Y,		/* mode height too large for specified virtual size */
+	MODE_MEM_VIRT,		/* insufficient video memory given virtual size */
+	MODE_NOCLOCK,		/* no fixed clock available */
+	MODE_CLOCK_HIGH,	/* clock required is too high */
+	MODE_CLOCK_LOW,		/* clock required is too low */
+	MODE_CLOCK_RANGE,	/* clock/mode isn't in a ClockRange */
+	MODE_BAD_HVALUE,	/* horizontal timing was out of range */
+	MODE_BAD_VVALUE,	/* vertical timing was out of range */
+	MODE_BAD_VSCAN,		/* VScan value out of range */
+	MODE_HSYNC_NARROW,	/* horizontal sync too narrow */
+	MODE_HSYNC_WIDE,	/* horizontal sync too wide */
+	MODE_HBLANK_NARROW,	/* horizontal blanking too narrow */
+	MODE_HBLANK_WIDE,	/* horizontal blanking too wide */
+	MODE_VSYNC_NARROW,	/* vertical sync too narrow */
+	MODE_VSYNC_WIDE,	/* vertical sync too wide */
+	MODE_VBLANK_NARROW,	/* vertical blanking too narrow */
+	MODE_VBLANK_WIDE,	/* vertical blanking too wide */
+	MODE_PANEL,		/* exceeds panel dimensions */
+	MODE_INTERLACE_WIDTH,	/* width too large for interlaced mode */
+	MODE_ONE_WIDTH,		/* only one width is supported */
+	MODE_ONE_HEIGHT,	/* only one height is supported */
+	MODE_ONE_SIZE,		/* only one resolution is supported */
+	MODE_NO_REDUCED,	/* monitor doesn't accept reduced blanking */
+	MODE_NO_STEREO,		/* stereo modes not supported */
+	MODE_STALE = -3,	/* mode has become stale */
+	MODE_BAD = -2,		/* unspecified reason */
+	MODE_ERROR = -1,	/* error condition */
 };
 
 #define DRM_MODE_TYPE_CLOCK_CRTC_C (DRM_MODE_TYPE_CLOCK_C | \
-- 
2.4.10

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/7] drm: Flatten drm_mode_connector_list_update() a bit

[ville.syrjala]

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

Use 'continue' to eliminate one indent level from
drm_mode_connector_list_update(). And while at it,
make 'found_it' bool.

Cc: Adam Jackson <ajax@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_modes.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 824125b3337f..2b94a5c661b0 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1183,30 +1183,30 @@ EXPORT_SYMBOL(drm_mode_sort);
 void drm_mode_connector_list_update(struct drm_connector *connector,
 				    bool merge_type_bits)
 {
-	struct drm_display_mode *mode;
 	struct drm_display_mode *pmode, *pt;
-	int found_it;
 
 	WARN_ON(!mutex_is_locked(&connector->dev->mode_config.mutex));
 
-	list_for_each_entry_safe(pmode, pt, &connector->probed_modes,
-				 head) {
-		found_it = 0;
+	list_for_each_entry_safe(pmode, pt, &connector->probed_modes, head) {
+		struct drm_display_mode *mode;
+		bool found_it = false;
+
 		/* go through current modes checking for the new probed mode */
 		list_for_each_entry(mode, &connector->modes, head) {
-			if (drm_mode_equal(pmode, mode)) {
-				found_it = 1;
-				/* if equal delete the probed mode */
-				mode->status = pmode->status;
-				/* Merge type bits together */
-				if (merge_type_bits)
-					mode->type |= pmode->type;
-				else
-					mode->type = pmode->type;
-				list_del(&pmode->head);
-				drm_mode_destroy(connector->dev, pmode);
-				break;
-			}
+			if (!drm_mode_equal(pmode, mode))
+				continue;
+
+			found_it = true;
+			/* if equal delete the probed mode */
+			mode->status = pmode->status;
+			/* Merge type bits together */
+			if (merge_type_bits)
+				mode->type |= pmode->type;
+			else
+				mode->type = pmode->type;
+			list_del(&pmode->head);
+			drm_mode_destroy(connector->dev, pmode);
+			break;
 		}
 
 		if (!found_it) {
-- 
2.4.10

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/7] drm: Only merge mode type bits between new probed modes

[ville.syrjala]

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

Currently most drivers request that any mode appearing on both the
old mode list and the new probed_modes list get their type bits ORed
together if the modes are deemed to otherwise match each other.

I don't know why anyone would want to merge in the mode type bits
from any mode left over from a previous probe. For instance, you
could never get rid of ther preferred bit if a matching non-preferred
mode is returned by the new probe. So let's not merge anything from
the stale old modes, and just replace them outright with matching new
modes.

If multiple matching modes are produced by the same probe, merging
the type bits between them would seem like a sensible thing to do.
For a bit of extra finesse if two modes are considered equal we can
pick the actual timings from the one marked as preferrred. And if
multiple preferred modes are produced by the same probe somehow, we
can just favor the first one added to the probed_modes list.

You may be asking yourself why we both with the merging at all if
nothing from the old list survives in practice. The only asnwer I have
is "debug output". That is we want to print out a list of pruned modes,
which is why we still want to look for duplicates with the old modes.

There was a previous attempt to get rid of the mode type merging
entirely, but it caused some kind of regression on Daniels's G33
machine. Apparently the hardware has since rotted away, so we don't
have to worry about it anymore. The relevant commits are:
commit 3fbd6439e463 ("drm: copy mode type in drm_mode_connector_list_update()")
commit abce1ec9b08a ("Revert "drm: copy mode type in drm_mode_connector_list_update()"")

It was then decided in
commit b87577b7c768 ("drm: try harder to avoid regression when merging mode bits")
that just qxl virtio are excluded from the merging, while everyone
else does it. That is not changed, although now even qxl and virtio
will be subject to the previously mentioned logic to choose which
actual timings are picked for the new mode.

Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Adam Jackson <ajax@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_modes.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 2b94a5c661b0..8c803e3af1da 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1197,13 +1197,33 @@ void drm_mode_connector_list_update(struct drm_connector *connector,
 				continue;
 
 			found_it = true;
-			/* if equal delete the probed mode */
-			mode->status = pmode->status;
-			/* Merge type bits together */
-			if (merge_type_bits)
-				mode->type |= pmode->type;
-			else
-				mode->type = pmode->type;
+
+			/*
+			 * If the old matching mode is stale (ie. left over
+			 * from a previous probe) just replace it outright.
+			 * Otherwise just merge the type bits between all
+			 * equal probed modes.
+			 *
+			 * If two probed modes are considered equal, pick the
+			 * actual timings from the one that's marked as
+			 * preferred (in case the match isn't 100%). If
+			 * multiple or zero preferred modes are present, favor
+			 * the mode added to the probed_modes list first.
+			 */
+			if (mode->status == MODE_STALE) {
+				drm_mode_copy(mode, pmode);
+			} else if ((mode->type & DRM_MODE_TYPE_PREFERRED) == 0 &&
+				   (pmode->type & DRM_MODE_TYPE_PREFERRED) != 0) {
+				if (merge_type_bits)
+					pmode->type |= mode->type;
+				drm_mode_copy(mode, pmode);
+			} else {
+				if (merge_type_bits)
+					mode->type |= pmode->type;
+				else
+					mode->type = pmode->type;
+			}
+
 			list_del(&pmode->head);
 			drm_mode_destroy(connector->dev, pmode);
 			break;
-- 
2.4.10

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/7] drm: Drop drm_helper_probe_single_connector_modes_nomerge()

[ville.syrjala]

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

Now that the mode type bit merge logic is fixed to only merge
between new probed modes, hopefully we can eliminat the special
case for qxl and virtio. That is make the merge the mode type
bits from all matching new probed modes, just like every other
driver.

qxl and virtio got excluded from the merging in
commit 3fbd6439e463 ("drm: copy mode type in drm_mode_connector_list_update()")
commit abce1ec9b08a ("Revert "drm: copy mode type in drm_mode_connector_list_update()"")
commit b87577b7c768 ("drm: try harder to avoid regression when merging mode bits")

Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Adam Jackson <ajax@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_modes.c              | 12 ++----
 drivers/gpu/drm/drm_probe_helper.c       | 65 +++++++++++---------------------
 drivers/gpu/drm/qxl/qxl_display.c        |  2 +-
 drivers/gpu/drm/virtio/virtgpu_display.c |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c      |  2 +-
 include/drm/drm_crtc_helper.h            |  4 --
 include/drm/drm_modes.h                  |  2 +-
 7 files changed, 28 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 8c803e3af1da..2e86e3412623 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1171,7 +1171,6 @@ EXPORT_SYMBOL(drm_mode_sort);
 /**
  * drm_mode_connector_list_update - update the mode list for the connector
  * @connector: the connector to update
- * @merge_type_bits: whether to merge or overwrite type bits
  *
  * This moves the modes from the @connector probed_modes list
  * to the actual mode list. It compares the probed mode against the current
@@ -1180,8 +1179,7 @@ EXPORT_SYMBOL(drm_mode_sort);
  * This is just a helper functions doesn't validate any modes itself and also
  * doesn't prune any invalid modes. Callers need to do that themselves.
  */
-void drm_mode_connector_list_update(struct drm_connector *connector,
-				    bool merge_type_bits)
+void drm_mode_connector_list_update(struct drm_connector *connector)
 {
 	struct drm_display_mode *pmode, *pt;
 
@@ -1214,14 +1212,10 @@ void drm_mode_connector_list_update(struct drm_connector *connector,
 				drm_mode_copy(mode, pmode);
 			} else if ((mode->type & DRM_MODE_TYPE_PREFERRED) == 0 &&
 				   (pmode->type & DRM_MODE_TYPE_PREFERRED) != 0) {
-				if (merge_type_bits)
-					pmode->type |= mode->type;
+				pmode->type |= mode->type;
 				drm_mode_copy(mode, pmode);
 			} else {
-				if (merge_type_bits)
-					mode->type |= pmode->type;
-				else
-					mode->type = pmode->type;
+				mode->type |= pmode->type;
 			}
 
 			list_del(&pmode->head);
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 2e0c8bfacd35..c4c5730db5d1 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -126,9 +126,26 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_kms_helper_poll_enable_locked);
 
-
-static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connector *connector,
-							      uint32_t maxX, uint32_t maxY, bool merge_type_bits)
+/**
+ * drm_helper_probe_single_connector_modes - get complete set of display modes
+ * @connector: connector to probe
+ * @maxX: max width for modes
+ * @maxY: max height for modes
+ *
+ * Based on the helper callbacks implemented by @connector try to detect all
+ * valid modes.  Modes will first be added to the connector's probed_modes list,
+ * then culled (based on validity and the @maxX, @maxY parameters) and put into
+ * the normal modes list.
+ *
+ * Intended to be use as a generic implementation of the ->fill_modes()
+ * @connector vfunc for drivers that use the crtc helpers for output mode
+ * filtering and detection.
+ *
+ * Returns:
+ * The number of modes found on @connector.
+ */
+int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
+					    uint32_t maxX, uint32_t maxY)
 {
 	struct drm_device *dev = connector->dev;
 	struct drm_display_mode *mode;
@@ -219,7 +236,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
 	if (count == 0)
 		goto prune;
 
-	drm_mode_connector_list_update(connector, merge_type_bits);
+	drm_mode_connector_list_update(connector);
 
 	if (connector->interlace_allowed)
 		mode_flags |= DRM_MODE_FLAG_INTERLACE;
@@ -263,49 +280,9 @@ prune:
 
 	return count;
 }
-
-/**
- * drm_helper_probe_single_connector_modes - get complete set of display modes
- * @connector: connector to probe
- * @maxX: max width for modes
- * @maxY: max height for modes
- *
- * Based on the helper callbacks implemented by @connector try to detect all
- * valid modes.  Modes will first be added to the connector's probed_modes list,
- * then culled (based on validity and the @maxX, @maxY parameters) and put into
- * the normal modes list.
- *
- * Intended to be use as a generic implementation of the ->fill_modes()
- * @connector vfunc for drivers that use the crtc helpers for output mode
- * filtering and detection.
- *
- * Returns:
- * The number of modes found on @connector.
- */
-int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
-					    uint32_t maxX, uint32_t maxY)
-{
-	return drm_helper_probe_single_connector_modes_merge_bits(connector, maxX, maxY, true);
-}
 EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
 
 /**
- * drm_helper_probe_single_connector_modes_nomerge - get complete set of display modes
- * @connector: connector to probe
- * @maxX: max width for modes
- * @maxY: max height for modes
- *
- * This operates like drm_hehlper_probe_single_connector_modes except it
- * replaces the mode bits instead of merging them for preferred modes.
- */
-int drm_helper_probe_single_connector_modes_nomerge(struct drm_connector *connector,
-					    uint32_t maxX, uint32_t maxY)
-{
-	return drm_helper_probe_single_connector_modes_merge_bits(connector, maxX, maxY, false);
-}
-EXPORT_SYMBOL(drm_helper_probe_single_connector_modes_nomerge);
-
-/**
  * drm_kms_helper_hotplug_event - fire off KMS hotplug events
  * @dev: drm_device whose connector state changed
  *
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index cddba079197f..7f9c0eb63169 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -935,7 +935,7 @@ static const struct drm_connector_funcs qxl_connector_funcs = {
 	.save = qxl_conn_save,
 	.restore = qxl_conn_restore,
 	.detect = qxl_conn_detect,
-	.fill_modes = drm_helper_probe_single_connector_modes_nomerge,
+	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = qxl_conn_set_property,
 	.destroy = qxl_conn_destroy,
 };
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index 8e6044d7660a..306a7df7d013 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -412,7 +412,7 @@ static const struct drm_connector_funcs virtio_gpu_connector_funcs = {
 	.save = virtio_gpu_conn_save,
 	.restore = virtio_gpu_conn_restore,
 	.detect = virtio_gpu_conn_detect,
-	.fill_modes = drm_helper_probe_single_connector_modes_nomerge,
+	.fill_modes = drm_helper_probe_single_connector_modes,
 	.destroy = virtio_gpu_conn_destroy,
 	.reset = drm_atomic_helper_connector_reset,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index e38db35132ed..808b247f3cfb 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1554,7 +1554,7 @@ int vmw_du_connector_fill_modes(struct drm_connector *connector,
 		drm_mode_probed_add(connector, mode);
 	}
 
-	drm_mode_connector_list_update(connector, true);
+	drm_mode_connector_list_update(connector);
 	/* Move the prefered mode first, help apps pick the right mode. */
 	drm_mode_sort(&connector->modes);
 
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index e22ab29d2d00..cb7bb91dc036 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -229,10 +229,6 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 extern int drm_helper_probe_single_connector_modes(struct drm_connector
 						   *connector, uint32_t maxX,
 						   uint32_t maxY);
-extern int drm_helper_probe_single_connector_modes_nomerge(struct drm_connector
-							   *connector,
-							   uint32_t maxX,
-							   uint32_t maxY);
 extern void drm_kms_helper_poll_init(struct drm_device *dev);
 extern void drm_kms_helper_poll_fini(struct drm_device *dev);
 extern bool drm_helper_hpd_irq_event(struct drm_device *dev);
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 256a1bbb125c..28023544e048 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -234,7 +234,7 @@ enum drm_mode_status drm_mode_validate_size(const struct drm_display_mode *mode,
 void drm_mode_prune_invalid(struct drm_device *dev,
 			    struct list_head *mode_list, bool verbose);
 void drm_mode_sort(struct list_head *mode_list);
-void drm_mode_connector_list_update(struct drm_connector *connector, bool merge_type_bits);
+void drm_mode_connector_list_update(struct drm_connector *connector);
 
 /* parsing cmdline modes */
 bool
-- 
2.4.10

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 7/7] drm/sti: Drop bogus drm_mode_sort() call

[ville.syrjala]

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

sti seems confused about which mode list is used in its .get_modes()
hook. It adds the modes to the probed_modes list (as is appropriate)
but then for some reason it tries to sort the old mode list.

Just drop the sorting since it does nothing, and let the probe helper
do its thing. It will sort the final mode list after merging in the
modes from the probed_modes list and validating them.

Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Vincent Abriou <vincent.abriou@st.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/sti/sti_hda.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
index d735daccd458..49cce833f2c8 100644
--- a/drivers/gpu/drm/sti/sti_hda.c
+++ b/drivers/gpu/drm/sti/sti_hda.c
@@ -543,8 +543,6 @@ static int sti_hda_connector_get_modes(struct drm_connector *connector)
 		count++;
 	}
 
-	drm_mode_sort(&connector->modes);
-
 	return count;
 }
 
-- 
2.4.10

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

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

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

[Daniel Vetter]

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

Re: [PATCH 2/7] drm: Rename MODE_UNVERIFIED to MODE_STALE

[Daniel Vetter]

On Thu, Dec 03, 2015 at 11:14:10PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> MODE_UNVERIFIED actually means that the mode came from a previous probe,
> and if the new probe doesn't produce a matching mode it will get pruned
> from the list. Rename the flag to MODE_STALE to better convey the
> meaning.
> 
> Cc: Adam Jackson <ajax@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Will conflict with my massive pile of kerneldoc, but makes sense.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_modes.c        | 2 +-
>  drivers/gpu/drm/drm_probe_helper.c | 4 ++--
>  include/drm/drm_modes.h            | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 5f79b9334a38..824125b3337f 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1073,7 +1073,7 @@ static const char * const drm_mode_status_names[] = {
>  	MODE_STATUS(ONE_SIZE),
>  	MODE_STATUS(NO_REDUCED),
>  	MODE_STATUS(NO_STEREO),
> -	MODE_STATUS(UNVERIFIED),
> +	MODE_STATUS(STALE),
>  	MODE_STATUS(BAD),
>  	MODE_STATUS(ERROR),
>  };
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index b9b3bd9349ff..2e0c8bfacd35 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -143,9 +143,9 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
>  			connector->name);
> -	/* set all modes to the unverified state */
> +	/* set all old modes to the stale state */
>  	list_for_each_entry(mode, &connector->modes, head)
> -		mode->status = MODE_UNVERIFIED;
> +		mode->status = MODE_STALE;
>  
>  	old_status = connector->status;
>  
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index f9115aee43f4..71801229ce9d 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -72,7 +72,7 @@ enum drm_mode_status {
>      MODE_ONE_SIZE,      /* only one resolution is supported */
>      MODE_NO_REDUCED,    /* monitor doesn't accept reduced blanking */
>      MODE_NO_STEREO,	/* stereo modes not supported */
> -    MODE_UNVERIFIED = -3, /* mode needs to reverified */
> +    MODE_STALE = -3,	/* mode has become stale */
>      MODE_BAD = -2,	/* unspecified reason */
>      MODE_ERROR	= -1	/* error condition */
>  };
> -- 
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/7] drm: Reindent enum drm_mode_status

[Daniel Vetter]

On Thu, Dec 03, 2015 at 11:14:11PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> There's some random mix of spaces and tabs used within the enum
> drm_mode_status definition. Fix it up to use just tabs.
> 
> Cc: Adam Jackson <ajax@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ok, this will conflict badly, ok if this doesn't go in and I do the
reformatting in my patch to kerneldoc drm_mode_status?
-Daniel

> ---
>  include/drm/drm_modes.h | 78 ++++++++++++++++++++++++-------------------------
>  1 file changed, 39 insertions(+), 39 deletions(-)
> 
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index 71801229ce9d..256a1bbb125c 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -36,45 +36,45 @@
>   */
>  
>  enum drm_mode_status {
> -    MODE_OK	= 0,	/* Mode OK */
> -    MODE_HSYNC,		/* hsync out of range */
> -    MODE_VSYNC,		/* vsync out of range */
> -    MODE_H_ILLEGAL,	/* mode has illegal horizontal timings */
> -    MODE_V_ILLEGAL,	/* mode has illegal horizontal timings */
> -    MODE_BAD_WIDTH,	/* requires an unsupported linepitch */
> -    MODE_NOMODE,	/* no mode with a matching name */
> -    MODE_NO_INTERLACE,	/* interlaced mode not supported */
> -    MODE_NO_DBLESCAN,	/* doublescan mode not supported */
> -    MODE_NO_VSCAN,	/* multiscan mode not supported */
> -    MODE_MEM,		/* insufficient video memory */
> -    MODE_VIRTUAL_X,	/* mode width too large for specified virtual size */
> -    MODE_VIRTUAL_Y,	/* mode height too large for specified virtual size */
> -    MODE_MEM_VIRT,	/* insufficient video memory given virtual size */
> -    MODE_NOCLOCK,	/* no fixed clock available */
> -    MODE_CLOCK_HIGH,	/* clock required is too high */
> -    MODE_CLOCK_LOW,	/* clock required is too low */
> -    MODE_CLOCK_RANGE,	/* clock/mode isn't in a ClockRange */
> -    MODE_BAD_HVALUE,	/* horizontal timing was out of range */
> -    MODE_BAD_VVALUE,	/* vertical timing was out of range */
> -    MODE_BAD_VSCAN,	/* VScan value out of range */
> -    MODE_HSYNC_NARROW,	/* horizontal sync too narrow */
> -    MODE_HSYNC_WIDE,	/* horizontal sync too wide */
> -    MODE_HBLANK_NARROW,	/* horizontal blanking too narrow */
> -    MODE_HBLANK_WIDE,	/* horizontal blanking too wide */
> -    MODE_VSYNC_NARROW,	/* vertical sync too narrow */
> -    MODE_VSYNC_WIDE,	/* vertical sync too wide */
> -    MODE_VBLANK_NARROW,	/* vertical blanking too narrow */
> -    MODE_VBLANK_WIDE,	/* vertical blanking too wide */
> -    MODE_PANEL,         /* exceeds panel dimensions */
> -    MODE_INTERLACE_WIDTH, /* width too large for interlaced mode */
> -    MODE_ONE_WIDTH,     /* only one width is supported */
> -    MODE_ONE_HEIGHT,    /* only one height is supported */
> -    MODE_ONE_SIZE,      /* only one resolution is supported */
> -    MODE_NO_REDUCED,    /* monitor doesn't accept reduced blanking */
> -    MODE_NO_STEREO,	/* stereo modes not supported */
> -    MODE_STALE = -3,	/* mode has become stale */
> -    MODE_BAD = -2,	/* unspecified reason */
> -    MODE_ERROR	= -1	/* error condition */
> +	MODE_OK	= 0,		/* Mode OK */
> +	MODE_HSYNC,		/* hsync out of range */
> +	MODE_VSYNC,		/* vsync out of range */
> +	MODE_H_ILLEGAL,		/* mode has illegal horizontal timings */
> +	MODE_V_ILLEGAL,		/* mode has illegal horizontal timings */
> +	MODE_BAD_WIDTH,		/* requires an unsupported linepitch */
> +	MODE_NOMODE,		/* no mode with a matching name */
> +	MODE_NO_INTERLACE,	/* interlaced mode not supported */
> +	MODE_NO_DBLESCAN,	/* doublescan mode not supported */
> +	MODE_NO_VSCAN,		/* multiscan mode not supported */
> +	MODE_MEM,		/* insufficient video memory */
> +	MODE_VIRTUAL_X,		/* mode width too large for specified virtual size */
> +	MODE_VIRTUAL_Y,		/* mode height too large for specified virtual size */
> +	MODE_MEM_VIRT,		/* insufficient video memory given virtual size */
> +	MODE_NOCLOCK,		/* no fixed clock available */
> +	MODE_CLOCK_HIGH,	/* clock required is too high */
> +	MODE_CLOCK_LOW,		/* clock required is too low */
> +	MODE_CLOCK_RANGE,	/* clock/mode isn't in a ClockRange */
> +	MODE_BAD_HVALUE,	/* horizontal timing was out of range */
> +	MODE_BAD_VVALUE,	/* vertical timing was out of range */
> +	MODE_BAD_VSCAN,		/* VScan value out of range */
> +	MODE_HSYNC_NARROW,	/* horizontal sync too narrow */
> +	MODE_HSYNC_WIDE,	/* horizontal sync too wide */
> +	MODE_HBLANK_NARROW,	/* horizontal blanking too narrow */
> +	MODE_HBLANK_WIDE,	/* horizontal blanking too wide */
> +	MODE_VSYNC_NARROW,	/* vertical sync too narrow */
> +	MODE_VSYNC_WIDE,	/* vertical sync too wide */
> +	MODE_VBLANK_NARROW,	/* vertical blanking too narrow */
> +	MODE_VBLANK_WIDE,	/* vertical blanking too wide */
> +	MODE_PANEL,		/* exceeds panel dimensions */
> +	MODE_INTERLACE_WIDTH,	/* width too large for interlaced mode */
> +	MODE_ONE_WIDTH,		/* only one width is supported */
> +	MODE_ONE_HEIGHT,	/* only one height is supported */
> +	MODE_ONE_SIZE,		/* only one resolution is supported */
> +	MODE_NO_REDUCED,	/* monitor doesn't accept reduced blanking */
> +	MODE_NO_STEREO,		/* stereo modes not supported */
> +	MODE_STALE = -3,	/* mode has become stale */
> +	MODE_BAD = -2,		/* unspecified reason */
> +	MODE_ERROR = -1,	/* error condition */
>  };
>  
>  #define DRM_MODE_TYPE_CLOCK_CRTC_C (DRM_MODE_TYPE_CLOCK_C | \
> -- 
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/7] drm: Flatten drm_mode_connector_list_update() a bit

[Daniel Vetter]

On Thu, Dec 03, 2015 at 11:14:12PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use 'continue' to eliminate one indent level from
> drm_mode_connector_list_update(). And while at it,
> make 'found_it' bool.
> 
> Cc: Adam Jackson <ajax@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/drm_modes.c | 36 ++++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 824125b3337f..2b94a5c661b0 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1183,30 +1183,30 @@ EXPORT_SYMBOL(drm_mode_sort);
>  void drm_mode_connector_list_update(struct drm_connector *connector,
>  				    bool merge_type_bits)
>  {
> -	struct drm_display_mode *mode;
>  	struct drm_display_mode *pmode, *pt;
> -	int found_it;
>  
>  	WARN_ON(!mutex_is_locked(&connector->dev->mode_config.mutex));
>  
> -	list_for_each_entry_safe(pmode, pt, &connector->probed_modes,
> -				 head) {
> -		found_it = 0;
> +	list_for_each_entry_safe(pmode, pt, &connector->probed_modes, head) {
> +		struct drm_display_mode *mode;
> +		bool found_it = false;
> +
>  		/* go through current modes checking for the new probed mode */
>  		list_for_each_entry(mode, &connector->modes, head) {
> -			if (drm_mode_equal(pmode, mode)) {
> -				found_it = 1;
> -				/* if equal delete the probed mode */
> -				mode->status = pmode->status;
> -				/* Merge type bits together */
> -				if (merge_type_bits)
> -					mode->type |= pmode->type;
> -				else
> -					mode->type = pmode->type;
> -				list_del(&pmode->head);
> -				drm_mode_destroy(connector->dev, pmode);
> -				break;
> -			}
> +			if (!drm_mode_equal(pmode, mode))
> +				continue;
> +
> +			found_it = true;
> +			/* if equal delete the probed mode */
> +			mode->status = pmode->status;
> +			/* Merge type bits together */
> +			if (merge_type_bits)
> +				mode->type |= pmode->type;
> +			else
> +				mode->type = pmode->type;
> +			list_del(&pmode->head);
> +			drm_mode_destroy(connector->dev, pmode);
> +			break;
>  		}
>  
>  		if (!found_it) {
> -- 
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/7] drm: Only merge mode type bits between new probed modes

[Daniel Vetter]

On Thu, Dec 03, 2015 at 11:14:13PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently most drivers request that any mode appearing on both the
> old mode list and the new probed_modes list get their type bits ORed
> together if the modes are deemed to otherwise match each other.
> 
> I don't know why anyone would want to merge in the mode type bits
> from any mode left over from a previous probe. For instance, you
> could never get rid of ther preferred bit if a matching non-preferred
> mode is returned by the new probe. So let's not merge anything from
> the stale old modes, and just replace them outright with matching new
> modes.
> 
> If multiple matching modes are produced by the same probe, merging
> the type bits between them would seem like a sensible thing to do.
> For a bit of extra finesse if two modes are considered equal we can
> pick the actual timings from the one marked as preferrred. And if
> multiple preferred modes are produced by the same probe somehow, we
> can just favor the first one added to the probed_modes list.
> 
> You may be asking yourself why we both with the merging at all if
s/both/bother/

> nothing from the old list survives in practice. The only asnwer I have

s/asnwer/answer/

> is "debug output". That is we want to print out a list of pruned modes,
> which is why we still want to look for duplicates with the old modes.

Yup, it's all an elaborate scheme to fill up dmesg ;-)

> There was a previous attempt to get rid of the mode type merging
> entirely, but it caused some kind of regression on Daniels's G33
> machine. Apparently the hardware has since rotted away, so we don't
> have to worry about it anymore. The relevant commits are:
> commit 3fbd6439e463 ("drm: copy mode type in drm_mode_connector_list_update()")
> commit abce1ec9b08a ("Revert "drm: copy mode type in drm_mode_connector_list_update()"")

Slight correction: It's only the sdvo transcoder that rotted, and when I
reported this regression whatever tiny change this causes seems to have
been enough to kill the machine. A few months later it died always. So
really unlucky conincidence that my hw was exactly between dead and alive
when Dave pushed this originally.

> It was then decided in
> commit b87577b7c768 ("drm: try harder to avoid regression when merging mode bits")
> that just qxl virtio are excluded from the merging, while everyone
> else does it. That is not changed, although now even qxl and virtio
> will be subject to the previously mentioned logic to choose which
> actual timings are picked for the new mode.
> 
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Adam Jackson <ajax@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

With the commit message corrected:

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

> ---
>  drivers/gpu/drm/drm_modes.c | 34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 2b94a5c661b0..8c803e3af1da 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1197,13 +1197,33 @@ void drm_mode_connector_list_update(struct drm_connector *connector,
>  				continue;
>  
>  			found_it = true;
> -			/* if equal delete the probed mode */
> -			mode->status = pmode->status;
> -			/* Merge type bits together */
> -			if (merge_type_bits)
> -				mode->type |= pmode->type;
> -			else
> -				mode->type = pmode->type;
> +
> +			/*
> +			 * If the old matching mode is stale (ie. left over
> +			 * from a previous probe) just replace it outright.
> +			 * Otherwise just merge the type bits between all
> +			 * equal probed modes.
> +			 *
> +			 * If two probed modes are considered equal, pick the
> +			 * actual timings from the one that's marked as
> +			 * preferred (in case the match isn't 100%). If
> +			 * multiple or zero preferred modes are present, favor
> +			 * the mode added to the probed_modes list first.
> +			 */
> +			if (mode->status == MODE_STALE) {
> +				drm_mode_copy(mode, pmode);
> +			} else if ((mode->type & DRM_MODE_TYPE_PREFERRED) == 0 &&
> +				   (pmode->type & DRM_MODE_TYPE_PREFERRED) != 0) {
> +				if (merge_type_bits)
> +					pmode->type |= mode->type;
> +				drm_mode_copy(mode, pmode);
> +			} else {
> +				if (merge_type_bits)
> +					mode->type |= pmode->type;
> +				else
> +					mode->type = pmode->type;
> +			}
> +
>  			list_del(&pmode->head);
>  			drm_mode_destroy(connector->dev, pmode);
>  			break;
> -- 
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/7] drm: Drop drm_helper_probe_single_connector_modes_nomerge()

[Daniel Vetter]

On Thu, Dec 03, 2015 at 11:14:14PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that the mode type bit merge logic is fixed to only merge
> between new probed modes, hopefully we can eliminat the special
> case for qxl and virtio. That is make the merge the mode type
> bits from all matching new probed modes, just like every other
> driver.
> 
> qxl and virtio got excluded from the merging in
> commit 3fbd6439e463 ("drm: copy mode type in drm_mode_connector_list_update()")
> commit abce1ec9b08a ("Revert "drm: copy mode type in drm_mode_connector_list_update()"")
> commit b87577b7c768 ("drm: try harder to avoid regression when merging mode bits")
> 
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Adam Jackson <ajax@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/drm_modes.c              | 12 ++----
>  drivers/gpu/drm/drm_probe_helper.c       | 65 +++++++++++---------------------
>  drivers/gpu/drm/qxl/qxl_display.c        |  2 +-
>  drivers/gpu/drm/virtio/virtgpu_display.c |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c      |  2 +-
>  include/drm/drm_crtc_helper.h            |  4 --
>  include/drm/drm_modes.h                  |  2 +-
>  7 files changed, 28 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 8c803e3af1da..2e86e3412623 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1171,7 +1171,6 @@ EXPORT_SYMBOL(drm_mode_sort);
>  /**
>   * drm_mode_connector_list_update - update the mode list for the connector
>   * @connector: the connector to update
> - * @merge_type_bits: whether to merge or overwrite type bits
>   *
>   * This moves the modes from the @connector probed_modes list
>   * to the actual mode list. It compares the probed mode against the current
> @@ -1180,8 +1179,7 @@ EXPORT_SYMBOL(drm_mode_sort);
>   * This is just a helper functions doesn't validate any modes itself and also
>   * doesn't prune any invalid modes. Callers need to do that themselves.
>   */
> -void drm_mode_connector_list_update(struct drm_connector *connector,
> -				    bool merge_type_bits)
> +void drm_mode_connector_list_update(struct drm_connector *connector)
>  {
>  	struct drm_display_mode *pmode, *pt;
>  
> @@ -1214,14 +1212,10 @@ void drm_mode_connector_list_update(struct drm_connector *connector,
>  				drm_mode_copy(mode, pmode);
>  			} else if ((mode->type & DRM_MODE_TYPE_PREFERRED) == 0 &&
>  				   (pmode->type & DRM_MODE_TYPE_PREFERRED) != 0) {
> -				if (merge_type_bits)
> -					pmode->type |= mode->type;
> +				pmode->type |= mode->type;
>  				drm_mode_copy(mode, pmode);
>  			} else {
> -				if (merge_type_bits)
> -					mode->type |= pmode->type;
> -				else
> -					mode->type = pmode->type;
> +				mode->type |= pmode->type;
>  			}
>  
>  			list_del(&pmode->head);
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 2e0c8bfacd35..c4c5730db5d1 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -126,9 +126,26 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev)
>  }
>  EXPORT_SYMBOL(drm_kms_helper_poll_enable_locked);
>  
> -
> -static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connector *connector,
> -							      uint32_t maxX, uint32_t maxY, bool merge_type_bits)
> +/**
> + * drm_helper_probe_single_connector_modes - get complete set of display modes
> + * @connector: connector to probe
> + * @maxX: max width for modes
> + * @maxY: max height for modes
> + *
> + * Based on the helper callbacks implemented by @connector try to detect all
> + * valid modes.  Modes will first be added to the connector's probed_modes list,
> + * then culled (based on validity and the @maxX, @maxY parameters) and put into
> + * the normal modes list.
> + *
> + * Intended to be use as a generic implementation of the ->fill_modes()
> + * @connector vfunc for drivers that use the crtc helpers for output mode
> + * filtering and detection.
> + *
> + * Returns:
> + * The number of modes found on @connector.
> + */
> +int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
> +					    uint32_t maxX, uint32_t maxY)
>  {
>  	struct drm_device *dev = connector->dev;
>  	struct drm_display_mode *mode;
> @@ -219,7 +236,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
>  	if (count == 0)
>  		goto prune;
>  
> -	drm_mode_connector_list_update(connector, merge_type_bits);
> +	drm_mode_connector_list_update(connector);
>  
>  	if (connector->interlace_allowed)
>  		mode_flags |= DRM_MODE_FLAG_INTERLACE;
> @@ -263,49 +280,9 @@ prune:
>  
>  	return count;
>  }
> -
> -/**
> - * drm_helper_probe_single_connector_modes - get complete set of display modes
> - * @connector: connector to probe
> - * @maxX: max width for modes
> - * @maxY: max height for modes
> - *
> - * Based on the helper callbacks implemented by @connector try to detect all
> - * valid modes.  Modes will first be added to the connector's probed_modes list,
> - * then culled (based on validity and the @maxX, @maxY parameters) and put into
> - * the normal modes list.
> - *
> - * Intended to be use as a generic implementation of the ->fill_modes()
> - * @connector vfunc for drivers that use the crtc helpers for output mode
> - * filtering and detection.
> - *
> - * Returns:
> - * The number of modes found on @connector.
> - */
> -int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
> -					    uint32_t maxX, uint32_t maxY)
> -{
> -	return drm_helper_probe_single_connector_modes_merge_bits(connector, maxX, maxY, true);
> -}
>  EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
>  
>  /**
> - * drm_helper_probe_single_connector_modes_nomerge - get complete set of display modes
> - * @connector: connector to probe
> - * @maxX: max width for modes
> - * @maxY: max height for modes
> - *
> - * This operates like drm_hehlper_probe_single_connector_modes except it
> - * replaces the mode bits instead of merging them for preferred modes.
> - */
> -int drm_helper_probe_single_connector_modes_nomerge(struct drm_connector *connector,
> -					    uint32_t maxX, uint32_t maxY)
> -{
> -	return drm_helper_probe_single_connector_modes_merge_bits(connector, maxX, maxY, false);
> -}
> -EXPORT_SYMBOL(drm_helper_probe_single_connector_modes_nomerge);
> -
> -/**
>   * drm_kms_helper_hotplug_event - fire off KMS hotplug events
>   * @dev: drm_device whose connector state changed
>   *
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
> index cddba079197f..7f9c0eb63169 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -935,7 +935,7 @@ static const struct drm_connector_funcs qxl_connector_funcs = {
>  	.save = qxl_conn_save,
>  	.restore = qxl_conn_restore,
>  	.detect = qxl_conn_detect,
> -	.fill_modes = drm_helper_probe_single_connector_modes_nomerge,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.set_property = qxl_conn_set_property,
>  	.destroy = qxl_conn_destroy,
>  };
> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
> index 8e6044d7660a..306a7df7d013 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> @@ -412,7 +412,7 @@ static const struct drm_connector_funcs virtio_gpu_connector_funcs = {
>  	.save = virtio_gpu_conn_save,
>  	.restore = virtio_gpu_conn_restore,
>  	.detect = virtio_gpu_conn_detect,
> -	.fill_modes = drm_helper_probe_single_connector_modes_nomerge,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.destroy = virtio_gpu_conn_destroy,
>  	.reset = drm_atomic_helper_connector_reset,
>  	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index e38db35132ed..808b247f3cfb 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1554,7 +1554,7 @@ int vmw_du_connector_fill_modes(struct drm_connector *connector,
>  		drm_mode_probed_add(connector, mode);
>  	}
>  
> -	drm_mode_connector_list_update(connector, true);
> +	drm_mode_connector_list_update(connector);
>  	/* Move the prefered mode first, help apps pick the right mode. */
>  	drm_mode_sort(&connector->modes);
>  
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index e22ab29d2d00..cb7bb91dc036 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -229,10 +229,6 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>  extern int drm_helper_probe_single_connector_modes(struct drm_connector
>  						   *connector, uint32_t maxX,
>  						   uint32_t maxY);
> -extern int drm_helper_probe_single_connector_modes_nomerge(struct drm_connector
> -							   *connector,
> -							   uint32_t maxX,
> -							   uint32_t maxY);
>  extern void drm_kms_helper_poll_init(struct drm_device *dev);
>  extern void drm_kms_helper_poll_fini(struct drm_device *dev);
>  extern bool drm_helper_hpd_irq_event(struct drm_device *dev);
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index 256a1bbb125c..28023544e048 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -234,7 +234,7 @@ enum drm_mode_status drm_mode_validate_size(const struct drm_display_mode *mode,
>  void drm_mode_prune_invalid(struct drm_device *dev,
>  			    struct list_head *mode_list, bool verbose);
>  void drm_mode_sort(struct list_head *mode_list);
> -void drm_mode_connector_list_update(struct drm_connector *connector, bool merge_type_bits);
> +void drm_mode_connector_list_update(struct drm_connector *connector);
>  
>  /* parsing cmdline modes */
>  bool
> -- 
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 7/7] drm/sti: Drop bogus drm_mode_sort() call

[Daniel Vetter]

On Thu, Dec 03, 2015 at 11:14:15PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> sti seems confused about which mode list is used in its .get_modes()
> hook. It adds the modes to the probed_modes list (as is appropriate)
> but then for some reason it tries to sort the old mode list.
> 
> Just drop the sorting since it does nothing, and let the probe helper
> do its thing. It will sort the final mode list after merging in the
> modes from the probed_modes list and validating them.
> 
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Vincent Abriou <vincent.abriou@st.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/sti/sti_hda.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
> index d735daccd458..49cce833f2c8 100644
> --- a/drivers/gpu/drm/sti/sti_hda.c
> +++ b/drivers/gpu/drm/sti/sti_hda.c
> @@ -543,8 +543,6 @@ static int sti_hda_connector_get_modes(struct drm_connector *connector)
>  		count++;
>  	}
>  
> -	drm_mode_sort(&connector->modes);
> -

Yeah I've just written the kerneldoc for ->get_modes, this is bogus.

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

>  	return count;
>  }
>  
> -- 
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/7] drm: Reindent enum drm_mode_status

[Ville Syrjälä]

On Fri, Dec 04, 2015 at 09:18:57AM +0100, Daniel Vetter wrote:
> On Thu, Dec 03, 2015 at 11:14:11PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > There's some random mix of spaces and tabs used within the enum
> > drm_mode_status definition. Fix it up to use just tabs.
> > 
> > Cc: Adam Jackson <ajax@redhat.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Ok, this will conflict badly, ok if this doesn't go in and I do the
> reformatting in my patch to kerneldoc drm_mode_status?

Fine by me. I just added this when I noticed checkpatch complaining
about the formatting in the previous patch.

> -Daniel
> 
> > ---
> >  include/drm/drm_modes.h | 78 ++++++++++++++++++++++++-------------------------
> >  1 file changed, 39 insertions(+), 39 deletions(-)
> > 
> > diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> > index 71801229ce9d..256a1bbb125c 100644
> > --- a/include/drm/drm_modes.h
> > +++ b/include/drm/drm_modes.h
> > @@ -36,45 +36,45 @@
> >   */
> >  
> >  enum drm_mode_status {
> > -    MODE_OK	= 0,	/* Mode OK */
> > -    MODE_HSYNC,		/* hsync out of range */
> > -    MODE_VSYNC,		/* vsync out of range */
> > -    MODE_H_ILLEGAL,	/* mode has illegal horizontal timings */
> > -    MODE_V_ILLEGAL,	/* mode has illegal horizontal timings */
> > -    MODE_BAD_WIDTH,	/* requires an unsupported linepitch */
> > -    MODE_NOMODE,	/* no mode with a matching name */
> > -    MODE_NO_INTERLACE,	/* interlaced mode not supported */
> > -    MODE_NO_DBLESCAN,	/* doublescan mode not supported */
> > -    MODE_NO_VSCAN,	/* multiscan mode not supported */
> > -    MODE_MEM,		/* insufficient video memory */
> > -    MODE_VIRTUAL_X,	/* mode width too large for specified virtual size */
> > -    MODE_VIRTUAL_Y,	/* mode height too large for specified virtual size */
> > -    MODE_MEM_VIRT,	/* insufficient video memory given virtual size */
> > -    MODE_NOCLOCK,	/* no fixed clock available */
> > -    MODE_CLOCK_HIGH,	/* clock required is too high */
> > -    MODE_CLOCK_LOW,	/* clock required is too low */
> > -    MODE_CLOCK_RANGE,	/* clock/mode isn't in a ClockRange */
> > -    MODE_BAD_HVALUE,	/* horizontal timing was out of range */
> > -    MODE_BAD_VVALUE,	/* vertical timing was out of range */
> > -    MODE_BAD_VSCAN,	/* VScan value out of range */
> > -    MODE_HSYNC_NARROW,	/* horizontal sync too narrow */
> > -    MODE_HSYNC_WIDE,	/* horizontal sync too wide */
> > -    MODE_HBLANK_NARROW,	/* horizontal blanking too narrow */
> > -    MODE_HBLANK_WIDE,	/* horizontal blanking too wide */
> > -    MODE_VSYNC_NARROW,	/* vertical sync too narrow */
> > -    MODE_VSYNC_WIDE,	/* vertical sync too wide */
> > -    MODE_VBLANK_NARROW,	/* vertical blanking too narrow */
> > -    MODE_VBLANK_WIDE,	/* vertical blanking too wide */
> > -    MODE_PANEL,         /* exceeds panel dimensions */
> > -    MODE_INTERLACE_WIDTH, /* width too large for interlaced mode */
> > -    MODE_ONE_WIDTH,     /* only one width is supported */
> > -    MODE_ONE_HEIGHT,    /* only one height is supported */
> > -    MODE_ONE_SIZE,      /* only one resolution is supported */
> > -    MODE_NO_REDUCED,    /* monitor doesn't accept reduced blanking */
> > -    MODE_NO_STEREO,	/* stereo modes not supported */
> > -    MODE_STALE = -3,	/* mode has become stale */
> > -    MODE_BAD = -2,	/* unspecified reason */
> > -    MODE_ERROR	= -1	/* error condition */
> > +	MODE_OK	= 0,		/* Mode OK */
> > +	MODE_HSYNC,		/* hsync out of range */
> > +	MODE_VSYNC,		/* vsync out of range */
> > +	MODE_H_ILLEGAL,		/* mode has illegal horizontal timings */
> > +	MODE_V_ILLEGAL,		/* mode has illegal horizontal timings */
> > +	MODE_BAD_WIDTH,		/* requires an unsupported linepitch */
> > +	MODE_NOMODE,		/* no mode with a matching name */
> > +	MODE_NO_INTERLACE,	/* interlaced mode not supported */
> > +	MODE_NO_DBLESCAN,	/* doublescan mode not supported */
> > +	MODE_NO_VSCAN,		/* multiscan mode not supported */
> > +	MODE_MEM,		/* insufficient video memory */
> > +	MODE_VIRTUAL_X,		/* mode width too large for specified virtual size */
> > +	MODE_VIRTUAL_Y,		/* mode height too large for specified virtual size */
> > +	MODE_MEM_VIRT,		/* insufficient video memory given virtual size */
> > +	MODE_NOCLOCK,		/* no fixed clock available */
> > +	MODE_CLOCK_HIGH,	/* clock required is too high */
> > +	MODE_CLOCK_LOW,		/* clock required is too low */
> > +	MODE_CLOCK_RANGE,	/* clock/mode isn't in a ClockRange */
> > +	MODE_BAD_HVALUE,	/* horizontal timing was out of range */
> > +	MODE_BAD_VVALUE,	/* vertical timing was out of range */
> > +	MODE_BAD_VSCAN,		/* VScan value out of range */
> > +	MODE_HSYNC_NARROW,	/* horizontal sync too narrow */
> > +	MODE_HSYNC_WIDE,	/* horizontal sync too wide */
> > +	MODE_HBLANK_NARROW,	/* horizontal blanking too narrow */
> > +	MODE_HBLANK_WIDE,	/* horizontal blanking too wide */
> > +	MODE_VSYNC_NARROW,	/* vertical sync too narrow */
> > +	MODE_VSYNC_WIDE,	/* vertical sync too wide */
> > +	MODE_VBLANK_NARROW,	/* vertical blanking too narrow */
> > +	MODE_VBLANK_WIDE,	/* vertical blanking too wide */
> > +	MODE_PANEL,		/* exceeds panel dimensions */
> > +	MODE_INTERLACE_WIDTH,	/* width too large for interlaced mode */
> > +	MODE_ONE_WIDTH,		/* only one width is supported */
> > +	MODE_ONE_HEIGHT,	/* only one height is supported */
> > +	MODE_ONE_SIZE,		/* only one resolution is supported */
> > +	MODE_NO_REDUCED,	/* monitor doesn't accept reduced blanking */
> > +	MODE_NO_STEREO,		/* stereo modes not supported */
> > +	MODE_STALE = -3,	/* mode has become stale */
> > +	MODE_BAD = -2,		/* unspecified reason */
> > +	MODE_ERROR = -1,	/* error condition */
> >  };
> >  
> >  #define DRM_MODE_TYPE_CLOCK_CRTC_C (DRM_MODE_TYPE_CLOCK_C | \
> > -- 
> > 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Ville Syrjälä]

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

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

[Ville Syrjälä]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 5/7] drm: Only merge mode type bits between new probed modes

[ville.syrjala]

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

Currently most drivers request that any mode appearing on both the
old mode list and the new probed_modes list get their type bits ORed
together if the modes are deemed to otherwise match each other.

I don't know why anyone would want to merge in the mode type bits
from any mode left over from a previous probe. For instance, you
could never get rid of ther preferred bit if a matching non-preferred
mode is returned by the new probe. So let's not merge anything from
the stale old modes, and just replace them outright with matching new
modes.

If multiple matching modes are produced by the same probe, merging
the type bits between them would seem like a sensible thing to do.
For a bit of extra finesse if two modes are considered equal we can
pick the actual timings from the one marked as preferrred. And if
multiple preferred modes are produced by the same probe somehow, we
can just favor the first one added to the probed_modes list.

You may be asking yourself why we bother with the merging at all if
nothing from the old list survives in practice. The only answer I have
is "debug output". That is we want to print out a list of pruned modes,
which is why we still want to look for duplicates with the old modes.

There was a previous attempt to get rid of the mode type merging
entirely, but it caused some kind of regression on Daniels's G33
machine. Apparently the sdvo transcoder on said machine started to
die at around the same time and has since rotted away totally, so
it may have been a red herring. So we don't have to worry about
it anymore. The relevant commits are:
commit 3fbd6439e463 ("drm: copy mode type in drm_mode_connector_list_update()")
commit abce1ec9b08a ("Revert "drm: copy mode type in drm_mode_connector_list_update()"")

It was then decided in
commit b87577b7c768 ("drm: try harder to avoid regression when merging mode bits")
that just qxl virtio are excluded from the merging, while everyone
else does it. That is not changed, although now even qxl and virtio
will be subject to the previously mentioned logic to choose which
actual timings are picked for the new mode.

v2: Fix typos in commit message, and clarify the details on
    the G33 regression from the previous attempt (Daniel)

Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Adam Jackson <ajax@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_modes.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 2b94a5c661b0..8c803e3af1da 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1197,13 +1197,33 @@ void drm_mode_connector_list_update(struct drm_connector *connector,
 				continue;
 
 			found_it = true;
-			/* if equal delete the probed mode */
-			mode->status = pmode->status;
-			/* Merge type bits together */
-			if (merge_type_bits)
-				mode->type |= pmode->type;
-			else
-				mode->type = pmode->type;
+
+			/*
+			 * If the old matching mode is stale (ie. left over
+			 * from a previous probe) just replace it outright.
+			 * Otherwise just merge the type bits between all
+			 * equal probed modes.
+			 *
+			 * If two probed modes are considered equal, pick the
+			 * actual timings from the one that's marked as
+			 * preferred (in case the match isn't 100%). If
+			 * multiple or zero preferred modes are present, favor
+			 * the mode added to the probed_modes list first.
+			 */
+			if (mode->status == MODE_STALE) {
+				drm_mode_copy(mode, pmode);
+			} else if ((mode->type & DRM_MODE_TYPE_PREFERRED) == 0 &&
+				   (pmode->type & DRM_MODE_TYPE_PREFERRED) != 0) {
+				if (merge_type_bits)
+					pmode->type |= mode->type;
+				drm_mode_copy(mode, pmode);
+			} else {
+				if (merge_type_bits)
+					mode->type |= pmode->type;
+				else
+					mode->type = pmode->type;
+			}
+
 			list_del(&pmode->head);
 			drm_mode_destroy(connector->dev, pmode);
 			break;
-- 
2.4.10

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/7] drm: Rename MODE_UNVERIFIED to MODE_STALE

[ville.syrjala]

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

MODE_UNVERIFIED actually means that the mode came from a previous probe,
and if the new probe doesn't produce a matching mode it will get pruned
from the list. Rename the flag to MODE_STALE to better convey the
meaning.

v2: Rebased due to conflicts with Daniel's doc stuff

Cc: Adam Jackson <ajax@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_modes.c        | 2 +-
 drivers/gpu/drm/drm_probe_helper.c | 4 ++--
 include/drm/drm_modes.h            | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 041138f5acc9..90d8a50dc375 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1074,7 +1074,7 @@ static const char * const drm_mode_status_names[] = {
 	MODE_STATUS(ONE_SIZE),
 	MODE_STATUS(NO_REDUCED),
 	MODE_STATUS(NO_STEREO),
-	MODE_STATUS(UNVERIFIED),
+	MODE_STATUS(STALE),
 	MODE_STATUS(BAD),
 	MODE_STATUS(ERROR),
 };
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index b691253aa752..3bdc8684fe58 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -146,9 +146,9 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
 			connector->name);
-	/* set all modes to the unverified state */
+	/* set all old modes to the stale state */
 	list_for_each_entry(mode, &connector->modes, head)
-		mode->status = MODE_UNVERIFIED;
+		mode->status = MODE_STALE;
 
 	old_status = connector->status;
 
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 9e6d7a1cd19b..d7445ccd958d 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -73,7 +73,7 @@
  * @MODE_ONE_SIZE: only one resolution is supported
  * @MODE_NO_REDUCED: monitor doesn't accept reduced blanking
  * @MODE_NO_STEREO: stereo modes not supported
- * @MODE_UNVERIFIED: mode needs to reverified
+ * @MODE_STALE: mode has become stale
  * @MODE_BAD: unspecified reason
  * @MODE_ERROR: error condition
  *
@@ -117,7 +117,7 @@ enum drm_mode_status {
 	MODE_ONE_SIZE,
 	MODE_NO_REDUCED,
 	MODE_NO_STEREO,
-	MODE_UNVERIFIED = -3,
+	MODE_STALE = -3,
 	MODE_BAD = -2,
 	MODE_ERROR = -1
 };
-- 
2.4.10

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Ville Syrjälä]

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

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

[Ville Syrjälä]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/7] drm: Fix mode pruning

[Daniel Vetter]

On Thu, Dec 03, 2015 at 11:14:08PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Turns out I broke mode pruning when the connector mode lists changes
> without the connector getting disconnected in between. Mostly a problem
> for i-g-t EDID forcing stuff I suppose, but maybe someone is fast enough
> that they can swap cables without the system noticing until the new
> cable is plugged in.
> 
> Anyway here's the fix, and I also ended up reviewing the way we merge
> new and old modes together, and made some changes there.

Pulled in the entire series except patch 3 into drm-misc.

Thanks, Daniel

> 
> Ville Syrjälä (7):
>   drm: Don't overwrite UNVERFIED mode status to OK
>   drm: Rename MODE_UNVERIFIED to MODE_STALE
>   drm: Reindent enum drm_mode_status
>   drm: Flatten drm_mode_connector_list_update() a bit
>   drm: Only merge mode type bits between new probed modes
>   drm: Drop drm_helper_probe_single_connector_modes_nomerge()
>   drm/sti: Drop bogus drm_mode_sort() call
> 
>  drivers/gpu/drm/drm_modes.c              | 56 +++++++++++++---------
>  drivers/gpu/drm/drm_probe_helper.c       | 72 ++++++++++------------------
>  drivers/gpu/drm/qxl/qxl_display.c        |  2 +-
>  drivers/gpu/drm/sti/sti_hda.c            |  2 -
>  drivers/gpu/drm/virtio/virtgpu_display.c |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c      |  2 +-
>  include/drm/drm_crtc_helper.h            |  4 --
>  include/drm/drm_modes.h                  | 80 ++++++++++++++++----------------
>  8 files changed, 103 insertions(+), 117 deletions(-)
> 
> -- 
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel