[PATCH v3 0/3] drm: Introduce drm_crtc_index() and drm_crtc_mask()

[PATCH v3 0/3] drm: Introduce drm_crtc_index() and drm_crtc_mask()

[Thierry Reding]

This series is a continuation of a patch posted earlier by Russell. It
addresses some comments from the initial posting as well as from some
discussion on IRC.

Patch 2 moves the drm_encoder_crtc_ok() function from the CRTC helpers
into the core so that patch 3 can reuse it to get rid of the duplicate
implementation in the i915 driver.

Note that I have a pair of patches that rely on this to fix a bug in the
Tegra driver that was exposed by the addition of the panel support, so I
would like to take these two patches through the Tegra tree.

Thierry

Russell King (1):
  drm: provide a helper for the encoder possible_crtcs mask

Thierry Reding (2):
  drm: Move drm_encoder_crtc_ok() to core
  drm/i915: Use drm_encoder_crtc_ok()

 drivers/gpu/drm/drm_crtc.c           | 23 +++++++++++++++++++++++
 drivers/gpu/drm/drm_crtc_helper.c    | 29 -----------------------------
 drivers/gpu/drm/i915/intel_display.c | 26 ++------------------------
 include/drm/drm_crtc.h               | 26 ++++++++++++++++++++++++++
 4 files changed, 51 insertions(+), 53 deletions(-)

-- 
1.8.4.2

[PATCH v3 1/3] drm: provide a helper for the encoder possible_crtcs mask

[Thierry Reding]

From: Russell King <rmk+kernel@arm.linux.org.uk>

The encoder possible_crtcs mask identifies which CRTCs can be bound to
a particular encoder.  Each bit from bit 0 defines an index in the list
of CRTCs held in the DRM mode_config crtc_list.  Rather than having
drivers trying to track the position of their CRTCs in the list, expose
the code which already exists for calculating the appropriate mask bit
for a CRTC.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
[treding@nvidia.com: add drm_crtc_index(), move to core]
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v3:
- move drm_crtc_mask() into include/drm/drm_crtc.h and make it static inline
  as suggested by Russell King
- BUG() instead of returning -1 if a CRTC isn't registered (David Herrmann)
- drop WARN(!crtc, "...") since code will oops anyway (Jani Nikula)

Changes in v2:
- further extract a new drm_crtc_index() function
- move to DRM core (drm_crtc.c)

 drivers/gpu/drm/drm_crtc.c        | 23 +++++++++++++++++++++++
 drivers/gpu/drm/drm_crtc_helper.c | 18 +-----------------
 include/drm/drm_crtc.h            | 13 +++++++++++++
 3 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 266a01d7f635..3b7d32da1604 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -675,6 +675,29 @@ void drm_crtc_cleanup(struct drm_crtc *crtc)
 EXPORT_SYMBOL(drm_crtc_cleanup);
 
 /**
+ * drm_crtc_index - find the index of a registered CRTC
+ * @crtc: CRTC to find index for
+ *
+ * Given a registered CRTC, return the index of that CRTC within a DRM
+ * device's list of CRTCs.
+ */
+unsigned int drm_crtc_index(struct drm_crtc *crtc)
+{
+	unsigned int index = 0;
+	struct drm_crtc *tmp;
+
+	list_for_each_entry(tmp, &crtc->dev->mode_config.crtc_list, head) {
+		if (tmp == crtc)
+			return index;
+
+		index++;
+	}
+
+	BUG();
+}
+EXPORT_SYMBOL(drm_crtc_index);
+
+/**
  * drm_mode_probed_add - add a mode to a connector's probed mode list
  * @connector: connector the new mode
  * @mode: mode data
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 01361aba033b..940c678cf012 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -334,23 +334,7 @@ EXPORT_SYMBOL(drm_helper_disable_unused_functions);
 static bool drm_encoder_crtc_ok(struct drm_encoder *encoder,
 				struct drm_crtc *crtc)
 {
-	struct drm_device *dev;
-	struct drm_crtc *tmp;
-	int crtc_mask = 1;
-
-	WARN(!crtc, "checking null crtc?\n");
-
-	dev = crtc->dev;
-
-	list_for_each_entry(tmp, &dev->mode_config.crtc_list, head) {
-		if (tmp == crtc)
-			break;
-		crtc_mask <<= 1;
-	}
-
-	if (encoder->possible_crtcs & crtc_mask)
-		return true;
-	return false;
+	return !!(encoder->possible_crtcs & drm_crtc_mask(crtc));
 }
 
 /*
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index f32c5cd51f41..4f2e3e82f014 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -929,6 +929,19 @@ extern int drm_crtc_init(struct drm_device *dev,
 			 struct drm_crtc *crtc,
 			 const struct drm_crtc_funcs *funcs);
 extern void drm_crtc_cleanup(struct drm_crtc *crtc);
+extern unsigned int drm_crtc_index(struct drm_crtc *crtc);
+
+/**
+ * drm_crtc_mask - find the mask of a registered CRTC
+ * @crtc: CRTC to find mask for
+ *
+ * Given a registered CRTC, return the mask bit of that CRTC for an
+ * encoder's possible_crtcs field.
+ */
+static inline uint32_t drm_crtc_mask(struct drm_crtc *crtc)
+{
+	return 1 << drm_crtc_index(crtc);
+}
 
 extern void drm_connector_ida_init(void);
 extern void drm_connector_ida_destroy(void);
-- 
1.8.4.2

[PATCH v3 2/3] drm: Move drm_encoder_crtc_ok() to core

[Thierry Reding]

Using the new drm_crtc_mask() function, drm_encoder_crtc_ok() can now be
written in a significantly shorter way, so it can be moved to a header
file and be made static inline.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v3:
- new patch

 drivers/gpu/drm/drm_crtc_helper.c | 13 -------------
 include/drm/drm_crtc.h            | 13 +++++++++++++
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 940c678cf012..48c90cbceb06 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -324,19 +324,6 @@ void drm_helper_disable_unused_functions(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_helper_disable_unused_functions);
 
-/**
- * drm_encoder_crtc_ok - can a given crtc drive a given encoder?
- * @encoder: encoder to test
- * @crtc: crtc to test
- *
- * Return false if @encoder can't be driven by @crtc, true otherwise.
- */
-static bool drm_encoder_crtc_ok(struct drm_encoder *encoder,
-				struct drm_crtc *crtc)
-{
-	return !!(encoder->possible_crtcs & drm_crtc_mask(crtc));
-}
-
 /*
  * Check the CRTC we're going to map each output to vs. its current
  * CRTC.  If they don't match, we have to disable the output and the CRTC
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 4f2e3e82f014..b3865a0e39f4 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -963,6 +963,19 @@ extern int drm_encoder_init(struct drm_device *dev,
 			    const struct drm_encoder_funcs *funcs,
 			    int encoder_type);
 
+/**
+ * drm_encoder_crtc_ok - can a given crtc drive a given encoder?
+ * @encoder: encoder to test
+ * @crtc: crtc to test
+ *
+ * Return false if @encoder can't be driven by @crtc, true otherwise.
+ */
+static inline bool drm_encoder_crtc_ok(struct drm_encoder *encoder,
+				       struct drm_crtc *crtc)
+{
+	return !!(encoder->possible_crtcs & drm_crtc_mask(crtc));
+}
+
 extern int drm_plane_init(struct drm_device *dev,
 			  struct drm_plane *plane,
 			  unsigned long possible_crtcs,
-- 
1.8.4.2

[PATCH v3 3/3] drm/i915: Use drm_encoder_crtc_ok()

[Thierry Reding]

The intel_encoder_crtc_ok() is a duplicate of the drm_encoder_crtc_ok()
function that used to be only available in the DRM CRTC helpers. It has
recently been moved to the core, so the duplicate can now be dropped.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v3:
- replace intel_encoder_crtc_ok() by drm_encoder_crtc_ok()

 drivers/gpu/drm/i915/intel_display.c | 26 ++------------------------
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e77d4b8856a7..7c245185c71f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8720,28 +8720,6 @@ static struct drm_crtc_helper_funcs intel_helper_funcs = {
 	.load_lut = intel_crtc_load_lut,
 };
 
-static bool intel_encoder_crtc_ok(struct drm_encoder *encoder,
-				  struct drm_crtc *crtc)
-{
-	struct drm_device *dev;
-	struct drm_crtc *tmp;
-	int crtc_mask = 1;
-
-	WARN(!crtc, "checking null crtc?\n");
-
-	dev = crtc->dev;
-
-	list_for_each_entry(tmp, &dev->mode_config.crtc_list, head) {
-		if (tmp == crtc)
-			break;
-		crtc_mask <<= 1;
-	}
-
-	if (encoder->possible_crtcs & crtc_mask)
-		return true;
-	return false;
-}
-
 /**
  * intel_modeset_update_staged_output_state
  *
@@ -9923,8 +9901,8 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 		}
 
 		/* Make sure the new CRTC will work with the encoder */
-		if (!intel_encoder_crtc_ok(&connector->new_encoder->base,
-					   new_crtc)) {
+		if (!drm_encoder_crtc_ok(&connector->new_encoder->base,
+					 new_crtc)) {
 			return -EINVAL;
 		}
 		connector->encoder->new_crtc = to_intel_crtc(new_crtc);
-- 
1.8.4.2

Re: [PATCH v3 3/3] drm/i915: Use drm_encoder_crtc_ok()

[Jani Nikula]

On Mon, 13 Jan 2014, Thierry Reding <thierry.reding@gmail.com> wrote:
> The intel_encoder_crtc_ok() is a duplicate of the drm_encoder_crtc_ok()
> function that used to be only available in the DRM CRTC helpers. It has
> recently been moved to the core, so the duplicate can now be dropped.
>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
> Changes in v3:
> - replace intel_encoder_crtc_ok() by drm_encoder_crtc_ok()
>
>  drivers/gpu/drm/i915/intel_display.c | 26 ++------------------------
>  1 file changed, 2 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e77d4b8856a7..7c245185c71f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8720,28 +8720,6 @@ static struct drm_crtc_helper_funcs intel_helper_funcs = {
>  	.load_lut = intel_crtc_load_lut,
>  };
>  
> -static bool intel_encoder_crtc_ok(struct drm_encoder *encoder,
> -				  struct drm_crtc *crtc)
> -{
> -	struct drm_device *dev;
> -	struct drm_crtc *tmp;
> -	int crtc_mask = 1;
> -
> -	WARN(!crtc, "checking null crtc?\n");
> -
> -	dev = crtc->dev;
> -
> -	list_for_each_entry(tmp, &dev->mode_config.crtc_list, head) {
> -		if (tmp == crtc)
> -			break;
> -		crtc_mask <<= 1;
> -	}
> -
> -	if (encoder->possible_crtcs & crtc_mask)
> -		return true;
> -	return false;
> -}
> -
>  /**
>   * intel_modeset_update_staged_output_state
>   *
> @@ -9923,8 +9901,8 @@ intel_modeset_stage_output_state(struct drm_device *dev,
>  		}
>  
>  		/* Make sure the new CRTC will work with the encoder */
> -		if (!intel_encoder_crtc_ok(&connector->new_encoder->base,
> -					   new_crtc)) {
> +		if (!drm_encoder_crtc_ok(&connector->new_encoder->base,
> +					 new_crtc)) {
>  			return -EINVAL;
>  		}
>  		connector->encoder->new_crtc = to_intel_crtc(new_crtc);
> -- 
> 1.8.4.2
>

-- 
Jani Nikula, Intel Open Source Technology Center