[PATCH 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[PATCH 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[sagar.a.kamble]

From: Sagar Kamble <sagar.a.kamble@intel.com>

With this patch we allow larger cursor planes of sizes 128x128
and 256x256. Planning to extend kms_cursor_crc test for verifying
these larger planes.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: G, Pallavi <pallavi.g@intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
 drivers/gpu/drm/i915/intel_display.c | 25 ++++++++++++++++++++-----
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2f564ce..2fee3a2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3522,7 +3522,11 @@
 /* New style CUR*CNTR flags */
 #define   CURSOR_MODE		0x27
 #define   CURSOR_MODE_DISABLE   0x00
+#define   CURSOR_MODE_128_32B_AX 0x02
+#define   CURSOR_MODE_256_32B_AX 0x03
 #define   CURSOR_MODE_64_32B_AX 0x07
+#define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
+#define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
 #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
 #define   MCURSOR_PIPE_SELECT	(1 << 28)
 #define   MCURSOR_PIPE_A	0x00
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f19e6ea..00b51f3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7411,10 +7411,18 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
 	bool visible = base != 0;
 
 	if (intel_crtc->cursor_visible != visible) {
+		int16_t width = intel_crtc->cursor_width;
 		uint32_t cntl = I915_READ(CURCNTR(pipe));
 		if (base) {
 			cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
-			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+
+			if (width == 64)
+				cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+			else if (width == 128)
+				cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+			else if (width == 256)
+				cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+
 			cntl |= pipe << 28; /* Connect to correct pipe */
 		} else {
 			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
@@ -7439,10 +7447,17 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
 	bool visible = base != 0;
 
 	if (intel_crtc->cursor_visible != visible) {
+		int16_t width = intel_crtc->cursor_width;
 		uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
 		if (base) {
 			cntl &= ~CURSOR_MODE;
-			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+
+			if (width == 64)
+				cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+			else if (width == 128)
+				cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+			else if (width == 256)
+				cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
 		} else {
 			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
 			cntl |= CURSOR_MODE_DISABLE;
@@ -7538,9 +7553,9 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 		goto finish;
 	}
 
-	/* Currently we only support 64x64 cursors */
-	if (width != 64 || height != 64) {
-		DRM_ERROR("we currently only support 64x64 cursors\n");
+	/* Check for which cursor types we support */
+	if (width > 256 || height > 256) {
+		DRM_ERROR("We currently only support 64x64, 128x128, 256x256 cursors\n");
 		return -EINVAL;
 	}
 
-- 
1.8.5

Re: [PATCH 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Ville Syrjälä]

On Tue, Feb 18, 2014 at 03:39:46PM +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> With this patch we allow larger cursor planes of sizes 128x128
> and 256x256. Planning to extend kms_cursor_crc test for verifying
> these larger planes.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org

Let's not spam everyone. Just intel-gfx is enough for such patches.
Well, maybe keep dri-devel too since cursor size seems to be a hot
topic across other drivers currently.

> Signed-off-by: G, Pallavi <pallavi.g@intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
>  drivers/gpu/drm/i915/intel_display.c | 25 ++++++++++++++++++++-----
>  2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2f564ce..2fee3a2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3522,7 +3522,11 @@
>  /* New style CUR*CNTR flags */
>  #define   CURSOR_MODE		0x27
>  #define   CURSOR_MODE_DISABLE   0x00
> +#define   CURSOR_MODE_128_32B_AX 0x02
> +#define   CURSOR_MODE_256_32B_AX 0x03
>  #define   CURSOR_MODE_64_32B_AX 0x07
> +#define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
> +#define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
>  #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
>  #define   MCURSOR_PIPE_SELECT	(1 << 28)
>  #define   MCURSOR_PIPE_A	0x00
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f19e6ea..00b51f3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7411,10 +7411,18 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
>  	bool visible = base != 0;
>  
>  	if (intel_crtc->cursor_visible != visible) {
> +		int16_t width = intel_crtc->cursor_width;
>  		uint32_t cntl = I915_READ(CURCNTR(pipe));
>  		if (base) {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> -			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +
> +			if (width == 64)
> +				cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			else if (width == 128)
> +				cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			else if (width == 256)
> +				cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +
>  			cntl |= pipe << 28; /* Connect to correct pipe */
>  		} else {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> @@ -7439,10 +7447,17 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
>  	bool visible = base != 0;
>  
>  	if (intel_crtc->cursor_visible != visible) {
> +		int16_t width = intel_crtc->cursor_width;
>  		uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
>  		if (base) {
>  			cntl &= ~CURSOR_MODE;
> -			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +
> +			if (width == 64)
> +				cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			else if (width == 128)
> +				cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			else if (width == 256)
> +				cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
>  		} else {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
>  			cntl |= CURSOR_MODE_DISABLE;
> @@ -7538,9 +7553,9 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
>  		goto finish;
>  	}
>  
> -	/* Currently we only support 64x64 cursors */
> -	if (width != 64 || height != 64) {
> -		DRM_ERROR("we currently only support 64x64 cursors\n");
> +	/* Check for which cursor types we support */
> +	if (width > 256 || height > 256) {

This has to check explicitly for 64x64, 128x128, or 256x256. Any other
combination of width and height is illegal.

Additionally gen2 supports only 64x64, so that has to be checked also.

Another issue is how to tell userspace about the cursor size. There was
a patch on dri-devel recently adding cursor width/height capability
queries to drm. I guess that should be enough for xorg since AFAICS it
only uses a single fixed cursor size. The downside is that if the actual
cursor image would fit one of the smaller sizes, we end up doing needless
memory fetches for the extra pixels.

Once we have drm_planes for cursors, I was thinking we might add some kind
of enum property that lists all the supported sizes for the plane.

> +		DRM_ERROR("We currently only support 64x64, 128x128, 256x256 cursors\n");
>  		return -EINVAL;
>  	}
>  
> -- 
> 1.8.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

[PATCH v2 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[sagar.a.kamble]

From: Sagar Kamble <sagar.a.kamble@intel.com>

With this patch we allow larger cursor planes of sizes 128x128
and 256x256.

v2: Added more precise check on size while setting cursor plane.

Testcase: igt/kms_cursor_crc
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: G, Pallavi <pallavi.g@intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
 drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++++++++-----
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2f564ce..2fee3a2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3522,7 +3522,11 @@
 /* New style CUR*CNTR flags */
 #define   CURSOR_MODE		0x27
 #define   CURSOR_MODE_DISABLE   0x00
+#define   CURSOR_MODE_128_32B_AX 0x02
+#define   CURSOR_MODE_256_32B_AX 0x03
 #define   CURSOR_MODE_64_32B_AX 0x07
+#define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
+#define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
 #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
 #define   MCURSOR_PIPE_SELECT	(1 << 28)
 #define   MCURSOR_PIPE_A	0x00
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f19e6ea..d036b41 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7411,10 +7411,18 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
 	bool visible = base != 0;
 
 	if (intel_crtc->cursor_visible != visible) {
+		int16_t width = intel_crtc->cursor_width;
 		uint32_t cntl = I915_READ(CURCNTR(pipe));
 		if (base) {
 			cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
-			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+
+			if (width == 64)
+				cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+			else if (width == 128)
+				cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+			else if (width == 256)
+				cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+
 			cntl |= pipe << 28; /* Connect to correct pipe */
 		} else {
 			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
@@ -7439,10 +7447,17 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
 	bool visible = base != 0;
 
 	if (intel_crtc->cursor_visible != visible) {
+		int16_t width = intel_crtc->cursor_width;
 		uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
 		if (base) {
 			cntl &= ~CURSOR_MODE;
-			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+
+			if (width == 64)
+				cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+			else if (width == 128)
+				cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+			else if (width == 256)
+				cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
 		} else {
 			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
 			cntl |= CURSOR_MODE_DISABLE;
@@ -7538,9 +7553,12 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 		goto finish;
 	}
 
-	/* Currently we only support 64x64 cursors */
-	if (width != 64 || height != 64) {
-		DRM_ERROR("we currently only support 64x64 cursors\n");
+	/* Check for which cursor types we support */
+	if ((!(width == 64 && height == 64) && IS_GEN2(dev)) ||
+	    (!(width == 64 && height == 64)
+	     && !(width == 128 && height == 128)
+	     && !(width == 256 && height == 256))) {
+		DRM_ERROR("Cursor dimension is not supported\n");
 		return -EINVAL;
 	}
 
-- 
1.8.5

Re: [PATCH v2 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Ville Syrjälä]

On Mon, Feb 24, 2014 at 09:11:43PM +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> With this patch we allow larger cursor planes of sizes 128x128
> and 256x256.
> 
> v2: Added more precise check on size while setting cursor plane.
> 
> Testcase: igt/kms_cursor_crc
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: G, Pallavi <pallavi.g@intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
>  drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++++++++-----
>  2 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2f564ce..2fee3a2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3522,7 +3522,11 @@
>  /* New style CUR*CNTR flags */
>  #define   CURSOR_MODE		0x27
>  #define   CURSOR_MODE_DISABLE   0x00
> +#define   CURSOR_MODE_128_32B_AX 0x02
> +#define   CURSOR_MODE_256_32B_AX 0x03
>  #define   CURSOR_MODE_64_32B_AX 0x07
> +#define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
> +#define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
>  #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
>  #define   MCURSOR_PIPE_SELECT	(1 << 28)
>  #define   MCURSOR_PIPE_A	0x00
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f19e6ea..d036b41 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7411,10 +7411,18 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
>  	bool visible = base != 0;
>  
>  	if (intel_crtc->cursor_visible != visible) {
> +		int16_t width = intel_crtc->cursor_width;
>  		uint32_t cntl = I915_READ(CURCNTR(pipe));
>  		if (base) {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> -			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +
> +			if (width == 64)
> +				cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			else if (width == 128)
> +				cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			else if (width == 256)
> +				cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;

A switch statement might be a bit clearer.

> +
>  			cntl |= pipe << 28; /* Connect to correct pipe */
>  		} else {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> @@ -7439,10 +7447,17 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
>  	bool visible = base != 0;
>  
>  	if (intel_crtc->cursor_visible != visible) {
> +		int16_t width = intel_crtc->cursor_width;
>  		uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
>  		if (base) {
>  			cntl &= ~CURSOR_MODE;
> -			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +
> +			if (width == 64)
> +				cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			else if (width == 128)
> +				cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			else if (width == 256)
> +				cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;

ditto.

>  		} else {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
>  			cntl |= CURSOR_MODE_DISABLE;
> @@ -7538,9 +7553,12 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
>  		goto finish;
>  	}
>  
> -	/* Currently we only support 64x64 cursors */
> -	if (width != 64 || height != 64) {
> -		DRM_ERROR("we currently only support 64x64 cursors\n");
> +	/* Check for which cursor types we support */
> +	if ((!(width == 64 && height == 64) && IS_GEN2(dev)) ||
> +	    (!(width == 64 && height == 64)
> +	     && !(width == 128 && height == 128)
> +	     && !(width == 256 && height == 256))) {

I'd make this something like:

if (!((width == 64 && height == 64) ||
      (width == 128 && height == 128 && !IS_GEN2(dev)) ||
      (width == 256 && height == 256 && !IS_GEN2(dev))))

> +		DRM_ERROR("Cursor dimension is not supported\n");

Should be DRM_DEBUG() or something. Otherwise the user can spam dmesg
by just issuing invalid cursor ioctls.

I see we have a bunch of other DRM_ERROR()s for similar stuff in the
cursor code. Those should be converted to debugs as well. Maybe you
can whip together another patch for that?

>  		return -EINVAL;
>  	}
>  
> -- 
> 1.8.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

[PATCH v3 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[sagar.a.kamble]

From: Sagar Kamble <sagar.a.kamble@intel.com>

With this patch we allow larger cursor planes of sizes 128x128
and 256x256.

v2: Added more precise check on size while setting cursor plane.

v3: Changes related to restructuring cursor size restrictions
and DRM_DEBUG usage.

Testcase: igt/kms_cursor_crc
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: G, Pallavi <pallavi.g@intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
 drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++++++++++++-----
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 146609a..aee8258 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3551,7 +3551,11 @@ enum punit_power_well {
 /* New style CUR*CNTR flags */
 #define   CURSOR_MODE		0x27
 #define   CURSOR_MODE_DISABLE   0x00
+#define   CURSOR_MODE_128_32B_AX 0x02
+#define   CURSOR_MODE_256_32B_AX 0x03
 #define   CURSOR_MODE_64_32B_AX 0x07
+#define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
+#define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
 #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
 #define   MCURSOR_PIPE_SELECT	(1 << 28)
 #define   MCURSOR_PIPE_A	0x00
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0868afb..e59e8fd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7440,10 +7440,22 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
 	bool visible = base != 0;
 
 	if (intel_crtc->cursor_visible != visible) {
+		int16_t width = intel_crtc->cursor_width;
 		uint32_t cntl = I915_READ(CURCNTR(pipe));
 		if (base) {
 			cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
-			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+
+			switch (width) {
+				case 64:
+					cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+					break;
+				case 128:
+					cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+					break;
+				case 256:
+					cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+					break;
+			}
 			cntl |= pipe << 28; /* Connect to correct pipe */
 		} else {
 			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
@@ -7468,10 +7480,22 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
 	bool visible = base != 0;
 
 	if (intel_crtc->cursor_visible != visible) {
+		int16_t width = intel_crtc->cursor_width;
 		uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
 		if (base) {
 			cntl &= ~CURSOR_MODE;
-			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+
+			switch (width) {
+				case 64:
+					cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+					break;
+				case 128:
+					cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+					break;
+				case 256:
+					cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+					break;
+			}
 		} else {
 			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
 			cntl |= CURSOR_MODE_DISABLE;
@@ -7567,9 +7591,11 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 		goto finish;
 	}
 
-	/* Currently we only support 64x64 cursors */
-	if (width != 64 || height != 64) {
-		DRM_ERROR("we currently only support 64x64 cursors\n");
+	/* Check for which cursor types we support */
+	if (!((width == 64 && height == 64) ||
+			(width == 128 && height == 128 && !IS_GEN2(dev)) ||
+			(width == 256 && height == 256 && !IS_GEN2(dev)))) {
+		DRM_DEBUG("Cursor dimension not supported\n");
 		return -EINVAL;
 	}
 
-- 
1.8.5

Re: [PATCH v3 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Alex Deucher]

On Sat, Mar 8, 2014 at 1:49 PM,  <sagar.a.kamble@intel.com> wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
>
> With this patch we allow larger cursor planes of sizes 128x128
> and 256x256.
>
> v2: Added more precise check on size while setting cursor plane.
>
> v3: Changes related to restructuring cursor size restrictions
> and DRM_DEBUG usage.
>

I'm not sure how useful it is since you appear to be able to
selectively adjust the cursor size, but I recently added support for
exposing the cursor size as a drm cap:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8716ed4e7bed4e4c7e3f37940e950ddc0362f450

Alex

> Testcase: igt/kms_cursor_crc
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: G, Pallavi <pallavi.g@intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
>  drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++++++++++++-----
>  2 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 146609a..aee8258 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3551,7 +3551,11 @@ enum punit_power_well {
>  /* New style CUR*CNTR flags */
>  #define   CURSOR_MODE          0x27
>  #define   CURSOR_MODE_DISABLE   0x00
> +#define   CURSOR_MODE_128_32B_AX 0x02
> +#define   CURSOR_MODE_256_32B_AX 0x03
>  #define   CURSOR_MODE_64_32B_AX 0x07
> +#define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
> +#define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
>  #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
>  #define   MCURSOR_PIPE_SELECT  (1 << 28)
>  #define   MCURSOR_PIPE_A       0x00
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0868afb..e59e8fd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7440,10 +7440,22 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
>         bool visible = base != 0;
>
>         if (intel_crtc->cursor_visible != visible) {
> +               int16_t width = intel_crtc->cursor_width;
>                 uint32_t cntl = I915_READ(CURCNTR(pipe));
>                 if (base) {
>                         cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> -                       cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +
> +                       switch (width) {
> +                               case 64:
> +                                       cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +                                       break;
> +                               case 128:
> +                                       cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +                                       break;
> +                               case 256:
> +                                       cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +                                       break;
> +                       }
>                         cntl |= pipe << 28; /* Connect to correct pipe */
>                 } else {
>                         cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> @@ -7468,10 +7480,22 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
>         bool visible = base != 0;
>
>         if (intel_crtc->cursor_visible != visible) {
> +               int16_t width = intel_crtc->cursor_width;
>                 uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
>                 if (base) {
>                         cntl &= ~CURSOR_MODE;
> -                       cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +
> +                       switch (width) {
> +                               case 64:
> +                                       cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +                                       break;
> +                               case 128:
> +                                       cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +                                       break;
> +                               case 256:
> +                                       cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +                                       break;
> +                       }
>                 } else {
>                         cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
>                         cntl |= CURSOR_MODE_DISABLE;
> @@ -7567,9 +7591,11 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
>                 goto finish;
>         }
>
> -       /* Currently we only support 64x64 cursors */
> -       if (width != 64 || height != 64) {
> -               DRM_ERROR("we currently only support 64x64 cursors\n");
> +       /* Check for which cursor types we support */
> +       if (!((width == 64 && height == 64) ||
> +                       (width == 128 && height == 128 && !IS_GEN2(dev)) ||
> +                       (width == 256 && height == 256 && !IS_GEN2(dev)))) {
> +               DRM_DEBUG("Cursor dimension not supported\n");
>                 return -EINVAL;
>         }
>
> --
> 1.8.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Sagar Arun Kamble]

On Sat, 2014-03-08 at 13:51 -0500, Alex Deucher wrote:
> On Sat, Mar 8, 2014 at 1:49 PM,  <sagar.a.kamble@intel.com> wrote:
> > From: Sagar Kamble <sagar.a.kamble@intel.com>
> >
> > With this patch we allow larger cursor planes of sizes 128x128
> > and 256x256.
> >
> > v2: Added more precise check on size while setting cursor plane.
> >
> > v3: Changes related to restructuring cursor size restrictions
> > and DRM_DEBUG usage.
> >
> 
> I'm not sure how useful it is since you appear to be able to
> selectively adjust the cursor size, but I recently added support for
> exposing the cursor size as a drm cap:
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8716ed4e7bed4e4c7e3f37940e950ddc0362f450
> 
> Alex
Thanks Alex. This is useful for cursor test to enumerate sub-tests based
on these caps.
> 
> > Testcase: igt/kms_cursor_crc
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: G, Pallavi <pallavi.g@intel.com>
> > Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
> >  drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++++++++++++-----
> >  2 files changed, 35 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 146609a..aee8258 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3551,7 +3551,11 @@ enum punit_power_well {
> >  /* New style CUR*CNTR flags */
> >  #define   CURSOR_MODE          0x27
> >  #define   CURSOR_MODE_DISABLE   0x00
> > +#define   CURSOR_MODE_128_32B_AX 0x02
> > +#define   CURSOR_MODE_256_32B_AX 0x03
> >  #define   CURSOR_MODE_64_32B_AX 0x07
> > +#define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
> > +#define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
> >  #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
> >  #define   MCURSOR_PIPE_SELECT  (1 << 28)
> >  #define   MCURSOR_PIPE_A       0x00
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 0868afb..e59e8fd 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -7440,10 +7440,22 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> >         bool visible = base != 0;
> >
> >         if (intel_crtc->cursor_visible != visible) {
> > +               int16_t width = intel_crtc->cursor_width;
> >                 uint32_t cntl = I915_READ(CURCNTR(pipe));
> >                 if (base) {
> >                         cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> > -                       cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > +
> > +                       switch (width) {
> > +                               case 64:
> > +                                       cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > +                                       break;
> > +                               case 128:
> > +                                       cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > +                                       break;
> > +                               case 256:
> > +                                       cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > +                                       break;
> > +                       }
> >                         cntl |= pipe << 28; /* Connect to correct pipe */
> >                 } else {
> >                         cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> > @@ -7468,10 +7480,22 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> >         bool visible = base != 0;
> >
> >         if (intel_crtc->cursor_visible != visible) {
> > +               int16_t width = intel_crtc->cursor_width;
> >                 uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
> >                 if (base) {
> >                         cntl &= ~CURSOR_MODE;
> > -                       cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > +
> > +                       switch (width) {
> > +                               case 64:
> > +                                       cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > +                                       break;
> > +                               case 128:
> > +                                       cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > +                                       break;
> > +                               case 256:
> > +                                       cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > +                                       break;
> > +                       }
> >                 } else {
> >                         cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> >                         cntl |= CURSOR_MODE_DISABLE;
> > @@ -7567,9 +7591,11 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
> >                 goto finish;
> >         }
> >
> > -       /* Currently we only support 64x64 cursors */
> > -       if (width != 64 || height != 64) {
> > -               DRM_ERROR("we currently only support 64x64 cursors\n");
> > +       /* Check for which cursor types we support */
> > +       if (!((width == 64 && height == 64) ||
> > +                       (width == 128 && height == 128 && !IS_GEN2(dev)) ||
> > +                       (width == 256 && height == 256 && !IS_GEN2(dev)))) {
> > +               DRM_DEBUG("Cursor dimension not supported\n");
> >                 return -EINVAL;
> >         }
> >
> > --
> > 1.8.5
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Sagar Arun Kamble]

On Sun, 2014-03-09 at 00:34 +0530, Sagar Arun Kamble wrote:
> On Sat, 2014-03-08 at 13:51 -0500, Alex Deucher wrote:
> > On Sat, Mar 8, 2014 at 1:49 PM,  <sagar.a.kamble@intel.com> wrote:
> > > From: Sagar Kamble <sagar.a.kamble@intel.com>
> > >
> > > With this patch we allow larger cursor planes of sizes 128x128
> > > and 256x256.
> > >
> > > v2: Added more precise check on size while setting cursor plane.
> > >
> > > v3: Changes related to restructuring cursor size restrictions
> > > and DRM_DEBUG usage.
> > >
> > 
> > I'm not sure how useful it is since you appear to be able to
> > selectively adjust the cursor size, but I recently added support for
> > exposing the cursor size as a drm cap:
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8716ed4e7bed4e4c7e3f37940e950ddc0362f450
> > 
> > Alex
> Thanks Alex. This is useful for cursor test to enumerate sub-tests based
> on these caps.
Realized after sending mail that we just get to know current cursor
width and height. Can we have capability that exposes all supported
cursor sizes?
> > 
> > > Testcase: igt/kms_cursor_crc
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Cc: David Airlie <airlied@linux.ie>
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: linux-kernel@vger.kernel.org
> > > Signed-off-by: G, Pallavi <pallavi.g@intel.com>
> > > Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
> > >  drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++++++++++++-----
> > >  2 files changed, 35 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 146609a..aee8258 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -3551,7 +3551,11 @@ enum punit_power_well {
> > >  /* New style CUR*CNTR flags */
> > >  #define   CURSOR_MODE          0x27
> > >  #define   CURSOR_MODE_DISABLE   0x00
> > > +#define   CURSOR_MODE_128_32B_AX 0x02
> > > +#define   CURSOR_MODE_256_32B_AX 0x03
> > >  #define   CURSOR_MODE_64_32B_AX 0x07
> > > +#define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
> > > +#define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
> > >  #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
> > >  #define   MCURSOR_PIPE_SELECT  (1 << 28)
> > >  #define   MCURSOR_PIPE_A       0x00
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 0868afb..e59e8fd 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -7440,10 +7440,22 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> > >         bool visible = base != 0;
> > >
> > >         if (intel_crtc->cursor_visible != visible) {
> > > +               int16_t width = intel_crtc->cursor_width;
> > >                 uint32_t cntl = I915_READ(CURCNTR(pipe));
> > >                 if (base) {
> > >                         cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> > > -                       cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > > +
> > > +                       switch (width) {
> > > +                               case 64:
> > > +                                       cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > > +                                       break;
> > > +                               case 128:
> > > +                                       cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > > +                                       break;
> > > +                               case 256:
> > > +                                       cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > > +                                       break;
> > > +                       }
> > >                         cntl |= pipe << 28; /* Connect to correct pipe */
> > >                 } else {
> > >                         cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> > > @@ -7468,10 +7480,22 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> > >         bool visible = base != 0;
> > >
> > >         if (intel_crtc->cursor_visible != visible) {
> > > +               int16_t width = intel_crtc->cursor_width;
> > >                 uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
> > >                 if (base) {
> > >                         cntl &= ~CURSOR_MODE;
> > > -                       cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > > +
> > > +                       switch (width) {
> > > +                               case 64:
> > > +                                       cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > > +                                       break;
> > > +                               case 128:
> > > +                                       cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > > +                                       break;
> > > +                               case 256:
> > > +                                       cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> > > +                                       break;
> > > +                       }
> > >                 } else {
> > >                         cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> > >                         cntl |= CURSOR_MODE_DISABLE;
> > > @@ -7567,9 +7591,11 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
> > >                 goto finish;
> > >         }
> > >
> > > -       /* Currently we only support 64x64 cursors */
> > > -       if (width != 64 || height != 64) {
> > > -               DRM_ERROR("we currently only support 64x64 cursors\n");
> > > +       /* Check for which cursor types we support */
> > > +       if (!((width == 64 && height == 64) ||
> > > +                       (width == 128 && height == 128 && !IS_GEN2(dev)) ||
> > > +                       (width == 256 && height == 256 && !IS_GEN2(dev)))) {
> > > +               DRM_DEBUG("Cursor dimension not supported\n");
> > >                 return -EINVAL;
> > >         }
> > >
> > > --
> > > 1.8.5
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [Intel-gfx] [PATCH v3 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Chris Wilson]

On Sun, Mar 09, 2014 at 12:37:49AM +0530, Sagar Arun Kamble wrote:
> On Sun, 2014-03-09 at 00:34 +0530, Sagar Arun Kamble wrote:
> > On Sat, 2014-03-08 at 13:51 -0500, Alex Deucher wrote:
> > > On Sat, Mar 8, 2014 at 1:49 PM,  <sagar.a.kamble@intel.com> wrote:
> > > > From: Sagar Kamble <sagar.a.kamble@intel.com>
> > > >
> > > > With this patch we allow larger cursor planes of sizes 128x128
> > > > and 256x256.
> > > >
> > > > v2: Added more precise check on size while setting cursor plane.
> > > >
> > > > v3: Changes related to restructuring cursor size restrictions
> > > > and DRM_DEBUG usage.
> > > >
> > > 
> > > I'm not sure how useful it is since you appear to be able to
> > > selectively adjust the cursor size, but I recently added support for
> > > exposing the cursor size as a drm cap:
> > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8716ed4e7bed4e4c7e3f37940e950ddc0362f450
> > > 
> > > Alex
> > Thanks Alex. This is useful for cursor test to enumerate sub-tests based
> > on these caps.
> Realized after sending mail that we just get to know current cursor
> width and height. Can we have capability that exposes all supported
> cursor sizes?

The cap exposes the max cursor size. Knowing our hardware we can infer
that pot sizes from 64 to max are supported, but it would be better if
we did expose that information through the cap as well.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [Intel-gfx] [PATCH v3 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Daniel Vetter]

On Mon, Mar 10, 2014 at 10:29 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> Realized after sending mail that we just get to know current cursor
>> width and height. Can we have capability that exposes all supported
>> cursor sizes?
>
> The cap exposes the max cursor size. Knowing our hardware we can infer
> that pot sizes from 64 to max are supported, but it would be better if
> we did expose that information through the cap as well.

I think the right approach here is to expose this through the
cursor-as-real-plane interface, which kinda has this already. On top
of that we can then add a few fourcc enumerations of the fixed rgba
cursor layouts like 64x64, 128x128, ... This helps if the plane is a
general one with very high limits, but also with special support for
cursor formats.

If anyone wants to go crazy we could then also add new fourccs for all
the other cursor layouts - atm only rgba with fixed dimensions can be
support with the current cursor ioctl.

So reviewing the overall situation I actually _don't_ want a new
cap/ioctl/prop here just for now. As long as we need to go with intel
specific hacks userspace might as well probe things manually and act
upon the -EINVAL.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH v3 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Chris Wilson]

On Mon, Mar 10, 2014 at 10:55:40AM +0100, Daniel Vetter wrote:
> On Mon, Mar 10, 2014 at 10:29 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >> Realized after sending mail that we just get to know current cursor
> >> width and height. Can we have capability that exposes all supported
> >> cursor sizes?
> >
> > The cap exposes the max cursor size. Knowing our hardware we can infer
> > that pot sizes from 64 to max are supported, but it would be better if
> > we did expose that information through the cap as well.
> 
> I think the right approach here is to expose this through the
> cursor-as-real-plane interface, which kinda has this already. On top
> of that we can then add a few fourcc enumerations of the fixed rgba
> cursor layouts like 64x64, 128x128, ... This helps if the plane is a
> general one with very high limits, but also with special support for
> cursor formats.
> 
> If anyone wants to go crazy we could then also add new fourccs for all
> the other cursor layouts - atm only rgba with fixed dimensions can be
> support with the current cursor ioctl.
> 
> So reviewing the overall situation I actually _don't_ want a new
> cap/ioctl/prop here just for now. As long as we need to go with intel
> specific hacks userspace might as well probe things manually and act
> upon the -EINVAL.

Adding the cap allows existing userspace to dtrt...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [Intel-gfx] [PATCH v3 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Daniel Vetter]

On Mon, Mar 10, 2014 at 11:04 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > The cap exposes the max cursor size. Knowing our hardware we can infer
>> > that pot sizes from 64 to max are supported, but it would be better if
>> > we did expose that information through the cap as well.
>>
>> I think the right approach here is to expose this through the
>> cursor-as-real-plane interface, which kinda has this already. On top
>> of that we can then add a few fourcc enumerations of the fixed rgba
>> cursor layouts like 64x64, 128x128, ... This helps if the plane is a
>> general one with very high limits, but also with special support for
>> cursor formats.
>>
>> If anyone wants to go crazy we could then also add new fourccs for all
>> the other cursor layouts - atm only rgba with fixed dimensions can be
>> support with the current cursor ioctl.
>>
>> So reviewing the overall situation I actually _don't_ want a new
>> cap/ioctl/prop here just for now. As long as we need to go with intel
>> specific hacks userspace might as well probe things manually and act
>> upon the -EINVAL.
>
> Adding the cap allows existing userspace to dtrt...

1. Move cursor off-screen.
2. Check desired cursor layout with set_cursor.
3. Pick real cursor, set it.

Or

1. Use set_cursor on disabled crtc.

Both aren't pretty, but that ugly should help to move
cursors-as-real-planes forward ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH v3 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Daniel Vetter]

On Mon, Mar 10, 2014 at 11:07 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Mon, Mar 10, 2014 at 11:04 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>> > The cap exposes the max cursor size. Knowing our hardware we can infer
>>> > that pot sizes from 64 to max are supported, but it would be better if
>>> > we did expose that information through the cap as well.
>>>
>>> I think the right approach here is to expose this through the
>>> cursor-as-real-plane interface, which kinda has this already. On top
>>> of that we can then add a few fourcc enumerations of the fixed rgba
>>> cursor layouts like 64x64, 128x128, ... This helps if the plane is a
>>> general one with very high limits, but also with special support for
>>> cursor formats.
>>>
>>> If anyone wants to go crazy we could then also add new fourccs for all
>>> the other cursor layouts - atm only rgba with fixed dimensions can be
>>> support with the current cursor ioctl.
>>>
>>> So reviewing the overall situation I actually _don't_ want a new
>>> cap/ioctl/prop here just for now. As long as we need to go with intel
>>> specific hacks userspace might as well probe things manually and act
>>> upon the -EINVAL.
>>
>> Adding the cap allows existing userspace to dtrt...
>
> 1. Move cursor off-screen.
> 2. Check desired cursor layout with set_cursor.
> 3. Pick real cursor, set it.
>
> Or
>
> 1. Use set_cursor on disabled crtc.
>
> Both aren't pretty, but that ugly should help to move
> cursors-as-real-planes forward ;-)

I stand corrected, there's now a cursor size cap in the drm-next
branch (which is included in -nightly), so we need support this and
also make sure it works correctly in the igt.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH v3 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Daniel Vetter]

On Sun, Mar 09, 2014 at 12:19:06AM +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> With this patch we allow larger cursor planes of sizes 128x128
> and 256x256.
> 
> v2: Added more precise check on size while setting cursor plane.
> 
> v3: Changes related to restructuring cursor size restrictions
> and DRM_DEBUG usage.
> 
> Testcase: igt/kms_cursor_crc
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: G, Pallavi <pallavi.g@intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
>  drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++++++++++++-----
>  2 files changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 146609a..aee8258 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3551,7 +3551,11 @@ enum punit_power_well {
>  /* New style CUR*CNTR flags */
>  #define   CURSOR_MODE		0x27
>  #define   CURSOR_MODE_DISABLE   0x00
> +#define   CURSOR_MODE_128_32B_AX 0x02
> +#define   CURSOR_MODE_256_32B_AX 0x03
>  #define   CURSOR_MODE_64_32B_AX 0x07
> +#define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
> +#define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
>  #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
>  #define   MCURSOR_PIPE_SELECT	(1 << 28)
>  #define   MCURSOR_PIPE_A	0x00
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0868afb..e59e8fd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7440,10 +7440,22 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
>  	bool visible = base != 0;
>  
>  	if (intel_crtc->cursor_visible != visible) {
> +		int16_t width = intel_crtc->cursor_width;
>  		uint32_t cntl = I915_READ(CURCNTR(pipe));
>  		if (base) {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> -			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +
> +			switch (width) {
> +				case 64:
> +					cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +					break;

We don't needlessly indent case 64: and the following code. Even with that
this indents a bit too far, so I suggest you extract the width->flags
computation into a little helper and use it in both places. Also please
extract the GAMMA flag, it's something else. Finally please add a default:
case and put a WARN_ON(1) in there, to make sure our code doesn't let
invalid cases slip through.
-Daniel

> +				case 128:
> +					cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +					break;
> +				case 256:
> +					cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +					break;
> +			}
>  			cntl |= pipe << 28; /* Connect to correct pipe */
>  		} else {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> @@ -7468,10 +7480,22 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
>  	bool visible = base != 0;
>  
>  	if (intel_crtc->cursor_visible != visible) {
> +		int16_t width = intel_crtc->cursor_width;
>  		uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
>  		if (base) {
>  			cntl &= ~CURSOR_MODE;
> -			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +
> +			switch (width) {
> +				case 64:
> +					cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +					break;
> +				case 128:
> +					cntl |= CURSOR_MODE_128_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +					break;
> +				case 256:
> +					cntl |= CURSOR_MODE_256_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +					break;
> +			}
>  		} else {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
>  			cntl |= CURSOR_MODE_DISABLE;
> @@ -7567,9 +7591,11 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
>  		goto finish;
>  	}
>  
> -	/* Currently we only support 64x64 cursors */
> -	if (width != 64 || height != 64) {
> -		DRM_ERROR("we currently only support 64x64 cursors\n");
> +	/* Check for which cursor types we support */
> +	if (!((width == 64 && height == 64) ||
> +			(width == 128 && height == 128 && !IS_GEN2(dev)) ||
> +			(width == 256 && height == 256 && !IS_GEN2(dev)))) {
> +		DRM_DEBUG("Cursor dimension not supported\n");
>  		return -EINVAL;
>  	}
>  
> -- 
> 1.8.5
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH v4 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[sagar.a.kamble]

From: Sagar Kamble <sagar.a.kamble@intel.com>

With this patch we allow larger cursor planes of sizes 128x128
and 256x256.

v2: Added more precise check on size while setting cursor plane.

v3: Changes related to restructuring cursor size restrictions
and DRM_DEBUG usage.

v4: Indentation related changes for setting cursor control and
implementing DRM_CAP_CURSOR_WIDTH and DRM_CAP_CURSOR_HEIGHT

Testcase: igt/kms_cursor_crc
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: G, Pallavi <pallavi.g@intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  4 +++
 drivers/gpu/drm/i915/intel_display.c | 53 ++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_drv.h     |  7 +++++
 3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 146609a..aee8258 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3551,7 +3551,11 @@ enum punit_power_well {
 /* New style CUR*CNTR flags */
 #define   CURSOR_MODE		0x27
 #define   CURSOR_MODE_DISABLE   0x00
+#define   CURSOR_MODE_128_32B_AX 0x02
+#define   CURSOR_MODE_256_32B_AX 0x03
 #define   CURSOR_MODE_64_32B_AX 0x07
+#define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
+#define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
 #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
 #define   MCURSOR_PIPE_SELECT	(1 << 28)
 #define   MCURSOR_PIPE_A	0x00
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0868afb..ec6a073 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7440,10 +7440,26 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
 	bool visible = base != 0;
 
 	if (intel_crtc->cursor_visible != visible) {
+		int16_t width = intel_crtc->cursor_width;
 		uint32_t cntl = I915_READ(CURCNTR(pipe));
 		if (base) {
 			cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
-			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+			cntl |= MCURSOR_GAMMA_ENABLE;
+
+			switch (width) {
+			case 64:
+				cntl |= CURSOR_MODE_64_ARGB_AX;
+				break;
+			case 128:
+				cntl |= CURSOR_MODE_128_ARGB_AX;
+				break;
+			case 256:
+				cntl |= CURSOR_MODE_256_ARGB_AX;
+				break;
+			default:
+				WARN_ON(1);
+				return;
+			}
 			cntl |= pipe << 28; /* Connect to correct pipe */
 		} else {
 			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
@@ -7468,10 +7484,25 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
 	bool visible = base != 0;
 
 	if (intel_crtc->cursor_visible != visible) {
+		int16_t width = intel_crtc->cursor_width;
 		uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
 		if (base) {
 			cntl &= ~CURSOR_MODE;
-			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+			cntl |= MCURSOR_GAMMA_ENABLE;
+			switch (width) {
+			case 64:
+				cntl |= CURSOR_MODE_64_ARGB_AX;
+				break;
+			case 128:
+				cntl |= CURSOR_MODE_128_ARGB_AX;
+				break;
+			case 256:
+				cntl |= CURSOR_MODE_256_ARGB_AX;
+				break;
+			default:
+				WARN_ON(1);
+				return;
+			}
 		} else {
 			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
 			cntl |= CURSOR_MODE_DISABLE;
@@ -7567,9 +7598,11 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 		goto finish;
 	}
 
-	/* Currently we only support 64x64 cursors */
-	if (width != 64 || height != 64) {
-		DRM_ERROR("we currently only support 64x64 cursors\n");
+	/* Check for which cursor types we support */
+	if (!((width == 64 && height == 64) ||
+			(width == 128 && height == 128 && !IS_GEN2(dev)) ||
+			(width == 256 && height == 256 && !IS_GEN2(dev)))) {
+		DRM_DEBUG("Cursor dimension not supported\n");
 		return -EINVAL;
 	}
 
@@ -10331,6 +10364,16 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 
 	drm_crtc_init(dev, &intel_crtc->base, &intel_crtc_funcs);
 
+	if (IS_GEN2(dev)) {
+		intel_crtc->max_cursor_width = GEN2_CURSOR_WIDTH;
+		intel_crtc->max_cursor_height = GEN2_CURSOR_HEIGHT;
+	} else {
+		intel_crtc->max_cursor_width = CURSOR_WIDTH;
+		intel_crtc->max_cursor_height = CURSOR_HEIGHT;
+	}
+	dev->mode_config.cursor_width = intel_crtc->max_cursor_width;
+	dev->mode_config.cursor_height = intel_crtc->max_cursor_height;
+
 	drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256);
 	for (i = 0; i < 256; i++) {
 		intel_crtc->lut_r[i] = i;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9c70905..eca4a0a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -78,6 +78,12 @@
 #define MAX_OUTPUTS 6
 /* maximum connectors per crtcs in the mode set */
 
+/* Maximum cursor sizes */
+#define GEN2_CURSOR_WIDTH 64
+#define GEN2_CURSOR_HEIGHT 64
+#define CURSOR_WIDTH 256
+#define CURSOR_HEIGHT 256
+
 #define INTEL_I2C_BUS_DVO 1
 #define INTEL_I2C_BUS_SDVO 2
 
@@ -364,6 +370,7 @@ struct intel_crtc {
 	uint32_t cursor_addr;
 	int16_t cursor_x, cursor_y;
 	int16_t cursor_width, cursor_height;
+	int16_t max_cursor_width, max_cursor_height;
 	bool cursor_visible;
 
 	struct intel_crtc_config config;
-- 
1.8.5

Re: [PATCH v4 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Sagar Arun Kamble]

Gentle reminder for reviewing this and i-g-t patch.

On Mon, 2014-03-10 at 17:06 +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> With this patch we allow larger cursor planes of sizes 128x128
> and 256x256.
> 
> v2: Added more precise check on size while setting cursor plane.
> 
> v3: Changes related to restructuring cursor size restrictions
> and DRM_DEBUG usage.
> 
> v4: Indentation related changes for setting cursor control and
> implementing DRM_CAP_CURSOR_WIDTH and DRM_CAP_CURSOR_HEIGHT
> 
> Testcase: igt/kms_cursor_crc
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: G, Pallavi <pallavi.g@intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  4 +++
>  drivers/gpu/drm/i915/intel_display.c | 53 ++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_drv.h     |  7 +++++
>  3 files changed, 59 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 146609a..aee8258 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3551,7 +3551,11 @@ enum punit_power_well {
>  /* New style CUR*CNTR flags */
>  #define   CURSOR_MODE		0x27
>  #define   CURSOR_MODE_DISABLE   0x00
> +#define   CURSOR_MODE_128_32B_AX 0x02
> +#define   CURSOR_MODE_256_32B_AX 0x03
>  #define   CURSOR_MODE_64_32B_AX 0x07
> +#define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
> +#define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
>  #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
>  #define   MCURSOR_PIPE_SELECT	(1 << 28)
>  #define   MCURSOR_PIPE_A	0x00
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0868afb..ec6a073 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7440,10 +7440,26 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
>  	bool visible = base != 0;
>  
>  	if (intel_crtc->cursor_visible != visible) {
> +		int16_t width = intel_crtc->cursor_width;
>  		uint32_t cntl = I915_READ(CURCNTR(pipe));
>  		if (base) {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> -			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			cntl |= MCURSOR_GAMMA_ENABLE;
> +
> +			switch (width) {
> +			case 64:
> +				cntl |= CURSOR_MODE_64_ARGB_AX;
> +				break;
> +			case 128:
> +				cntl |= CURSOR_MODE_128_ARGB_AX;
> +				break;
> +			case 256:
> +				cntl |= CURSOR_MODE_256_ARGB_AX;
> +				break;
> +			default:
> +				WARN_ON(1);
> +				return;
> +			}
>  			cntl |= pipe << 28; /* Connect to correct pipe */
>  		} else {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> @@ -7468,10 +7484,25 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
>  	bool visible = base != 0;
>  
>  	if (intel_crtc->cursor_visible != visible) {
> +		int16_t width = intel_crtc->cursor_width;
>  		uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
>  		if (base) {
>  			cntl &= ~CURSOR_MODE;
> -			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			cntl |= MCURSOR_GAMMA_ENABLE;
> +			switch (width) {
> +			case 64:
> +				cntl |= CURSOR_MODE_64_ARGB_AX;
> +				break;
> +			case 128:
> +				cntl |= CURSOR_MODE_128_ARGB_AX;
> +				break;
> +			case 256:
> +				cntl |= CURSOR_MODE_256_ARGB_AX;
> +				break;
> +			default:
> +				WARN_ON(1);
> +				return;
> +			}
>  		} else {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
>  			cntl |= CURSOR_MODE_DISABLE;
> @@ -7567,9 +7598,11 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
>  		goto finish;
>  	}
>  
> -	/* Currently we only support 64x64 cursors */
> -	if (width != 64 || height != 64) {
> -		DRM_ERROR("we currently only support 64x64 cursors\n");
> +	/* Check for which cursor types we support */
> +	if (!((width == 64 && height == 64) ||
> +			(width == 128 && height == 128 && !IS_GEN2(dev)) ||
> +			(width == 256 && height == 256 && !IS_GEN2(dev)))) {
> +		DRM_DEBUG("Cursor dimension not supported\n");
>  		return -EINVAL;
>  	}
>  
> @@ -10331,6 +10364,16 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  
>  	drm_crtc_init(dev, &intel_crtc->base, &intel_crtc_funcs);
>  
> +	if (IS_GEN2(dev)) {
> +		intel_crtc->max_cursor_width = GEN2_CURSOR_WIDTH;
> +		intel_crtc->max_cursor_height = GEN2_CURSOR_HEIGHT;
> +	} else {
> +		intel_crtc->max_cursor_width = CURSOR_WIDTH;
> +		intel_crtc->max_cursor_height = CURSOR_HEIGHT;
> +	}
> +	dev->mode_config.cursor_width = intel_crtc->max_cursor_width;
> +	dev->mode_config.cursor_height = intel_crtc->max_cursor_height;
> +
>  	drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256);
>  	for (i = 0; i < 256; i++) {
>  		intel_crtc->lut_r[i] = i;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9c70905..eca4a0a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -78,6 +78,12 @@
>  #define MAX_OUTPUTS 6
>  /* maximum connectors per crtcs in the mode set */
>  
> +/* Maximum cursor sizes */
> +#define GEN2_CURSOR_WIDTH 64
> +#define GEN2_CURSOR_HEIGHT 64
> +#define CURSOR_WIDTH 256
> +#define CURSOR_HEIGHT 256
> +
>  #define INTEL_I2C_BUS_DVO 1
>  #define INTEL_I2C_BUS_SDVO 2
>  
> @@ -364,6 +370,7 @@ struct intel_crtc {
>  	uint32_t cursor_addr;
>  	int16_t cursor_x, cursor_y;
>  	int16_t cursor_width, cursor_height;
> +	int16_t max_cursor_width, max_cursor_height;
>  	bool cursor_visible;
>  
>  	struct intel_crtc_config config;

Re: [PATCH v4 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Imre Deak]

[-- Attachment #1: Type: text/plain, Size: 5932 bytes --]

On Mon, 2014-03-10 at 17:06 +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> With this patch we allow larger cursor planes of sizes 128x128
> and 256x256.
> 
> v2: Added more precise check on size while setting cursor plane.
> 
> v3: Changes related to restructuring cursor size restrictions
> and DRM_DEBUG usage.
> 
> v4: Indentation related changes for setting cursor control and
> implementing DRM_CAP_CURSOR_WIDTH and DRM_CAP_CURSOR_HEIGHT
> 
> Testcase: igt/kms_cursor_crc
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: G, Pallavi <pallavi.g@intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>

Looks ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  4 +++
>  drivers/gpu/drm/i915/intel_display.c | 53 ++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_drv.h     |  7 +++++
>  3 files changed, 59 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 146609a..aee8258 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3551,7 +3551,11 @@ enum punit_power_well {
>  /* New style CUR*CNTR flags */
>  #define   CURSOR_MODE		0x27
>  #define   CURSOR_MODE_DISABLE   0x00
> +#define   CURSOR_MODE_128_32B_AX 0x02
> +#define   CURSOR_MODE_256_32B_AX 0x03
>  #define   CURSOR_MODE_64_32B_AX 0x07
> +#define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
> +#define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
>  #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
>  #define   MCURSOR_PIPE_SELECT	(1 << 28)
>  #define   MCURSOR_PIPE_A	0x00
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0868afb..ec6a073 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7440,10 +7440,26 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
>  	bool visible = base != 0;
>  
>  	if (intel_crtc->cursor_visible != visible) {
> +		int16_t width = intel_crtc->cursor_width;
>  		uint32_t cntl = I915_READ(CURCNTR(pipe));
>  		if (base) {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> -			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			cntl |= MCURSOR_GAMMA_ENABLE;
> +
> +			switch (width) {
> +			case 64:
> +				cntl |= CURSOR_MODE_64_ARGB_AX;
> +				break;
> +			case 128:
> +				cntl |= CURSOR_MODE_128_ARGB_AX;
> +				break;
> +			case 256:
> +				cntl |= CURSOR_MODE_256_ARGB_AX;
> +				break;
> +			default:
> +				WARN_ON(1);
> +				return;
> +			}
>  			cntl |= pipe << 28; /* Connect to correct pipe */
>  		} else {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> @@ -7468,10 +7484,25 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
>  	bool visible = base != 0;
>  
>  	if (intel_crtc->cursor_visible != visible) {
> +		int16_t width = intel_crtc->cursor_width;
>  		uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
>  		if (base) {
>  			cntl &= ~CURSOR_MODE;
> -			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			cntl |= MCURSOR_GAMMA_ENABLE;
> +			switch (width) {
> +			case 64:
> +				cntl |= CURSOR_MODE_64_ARGB_AX;
> +				break;
> +			case 128:
> +				cntl |= CURSOR_MODE_128_ARGB_AX;
> +				break;
> +			case 256:
> +				cntl |= CURSOR_MODE_256_ARGB_AX;
> +				break;
> +			default:
> +				WARN_ON(1);
> +				return;
> +			}
>  		} else {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
>  			cntl |= CURSOR_MODE_DISABLE;
> @@ -7567,9 +7598,11 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
>  		goto finish;
>  	}
>  
> -	/* Currently we only support 64x64 cursors */
> -	if (width != 64 || height != 64) {
> -		DRM_ERROR("we currently only support 64x64 cursors\n");
> +	/* Check for which cursor types we support */
> +	if (!((width == 64 && height == 64) ||
> +			(width == 128 && height == 128 && !IS_GEN2(dev)) ||
> +			(width == 256 && height == 256 && !IS_GEN2(dev)))) {
> +		DRM_DEBUG("Cursor dimension not supported\n");
>  		return -EINVAL;
>  	}
>  
> @@ -10331,6 +10364,16 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  
>  	drm_crtc_init(dev, &intel_crtc->base, &intel_crtc_funcs);
>  
> +	if (IS_GEN2(dev)) {
> +		intel_crtc->max_cursor_width = GEN2_CURSOR_WIDTH;
> +		intel_crtc->max_cursor_height = GEN2_CURSOR_HEIGHT;
> +	} else {
> +		intel_crtc->max_cursor_width = CURSOR_WIDTH;
> +		intel_crtc->max_cursor_height = CURSOR_HEIGHT;
> +	}
> +	dev->mode_config.cursor_width = intel_crtc->max_cursor_width;
> +	dev->mode_config.cursor_height = intel_crtc->max_cursor_height;
> +
>  	drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256);
>  	for (i = 0; i < 256; i++) {
>  		intel_crtc->lut_r[i] = i;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9c70905..eca4a0a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -78,6 +78,12 @@
>  #define MAX_OUTPUTS 6
>  /* maximum connectors per crtcs in the mode set */
>  
> +/* Maximum cursor sizes */
> +#define GEN2_CURSOR_WIDTH 64
> +#define GEN2_CURSOR_HEIGHT 64
> +#define CURSOR_WIDTH 256
> +#define CURSOR_HEIGHT 256
> +
>  #define INTEL_I2C_BUS_DVO 1
>  #define INTEL_I2C_BUS_SDVO 2
>  
> @@ -364,6 +370,7 @@ struct intel_crtc {
>  	uint32_t cursor_addr;
>  	int16_t cursor_x, cursor_y;
>  	int16_t cursor_width, cursor_height;
> +	int16_t max_cursor_width, max_cursor_height;
>  	bool cursor_visible;
>  
>  	struct intel_crtc_config config;


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH v4 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Daniel Vetter]

On Thu, Mar 20, 2014 at 05:30:43PM +0200, Imre Deak wrote:
> On Mon, 2014-03-10 at 17:06 +0530, sagar.a.kamble@intel.com wrote:
> > From: Sagar Kamble <sagar.a.kamble@intel.com>
> > 
> > With this patch we allow larger cursor planes of sizes 128x128
> > and 256x256.
> > 
> > v2: Added more precise check on size while setting cursor plane.
> > 
> > v3: Changes related to restructuring cursor size restrictions
> > and DRM_DEBUG usage.
> > 
> > v4: Indentation related changes for setting cursor control and
> > implementing DRM_CAP_CURSOR_WIDTH and DRM_CAP_CURSOR_HEIGHT
> > 
> > Testcase: igt/kms_cursor_crc
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: G, Pallavi <pallavi.g@intel.com>
> > Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> Looks ok:
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH v4 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Imre Deak]

[-- Attachment #1: Type: text/plain, Size: 5932 bytes --]

On Mon, 2014-03-10 at 17:06 +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> With this patch we allow larger cursor planes of sizes 128x128
> and 256x256.
> 
> v2: Added more precise check on size while setting cursor plane.
> 
> v3: Changes related to restructuring cursor size restrictions
> and DRM_DEBUG usage.
> 
> v4: Indentation related changes for setting cursor control and
> implementing DRM_CAP_CURSOR_WIDTH and DRM_CAP_CURSOR_HEIGHT
> 
> Testcase: igt/kms_cursor_crc
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: G, Pallavi <pallavi.g@intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>

Looks ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  4 +++
>  drivers/gpu/drm/i915/intel_display.c | 53 ++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_drv.h     |  7 +++++
>  3 files changed, 59 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 146609a..aee8258 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3551,7 +3551,11 @@ enum punit_power_well {
>  /* New style CUR*CNTR flags */
>  #define   CURSOR_MODE		0x27
>  #define   CURSOR_MODE_DISABLE   0x00
> +#define   CURSOR_MODE_128_32B_AX 0x02
> +#define   CURSOR_MODE_256_32B_AX 0x03
>  #define   CURSOR_MODE_64_32B_AX 0x07
> +#define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
> +#define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
>  #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
>  #define   MCURSOR_PIPE_SELECT	(1 << 28)
>  #define   MCURSOR_PIPE_A	0x00
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0868afb..ec6a073 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7440,10 +7440,26 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
>  	bool visible = base != 0;
>  
>  	if (intel_crtc->cursor_visible != visible) {
> +		int16_t width = intel_crtc->cursor_width;
>  		uint32_t cntl = I915_READ(CURCNTR(pipe));
>  		if (base) {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> -			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			cntl |= MCURSOR_GAMMA_ENABLE;
> +
> +			switch (width) {
> +			case 64:
> +				cntl |= CURSOR_MODE_64_ARGB_AX;
> +				break;
> +			case 128:
> +				cntl |= CURSOR_MODE_128_ARGB_AX;
> +				break;
> +			case 256:
> +				cntl |= CURSOR_MODE_256_ARGB_AX;
> +				break;
> +			default:
> +				WARN_ON(1);
> +				return;
> +			}
>  			cntl |= pipe << 28; /* Connect to correct pipe */
>  		} else {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> @@ -7468,10 +7484,25 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
>  	bool visible = base != 0;
>  
>  	if (intel_crtc->cursor_visible != visible) {
> +		int16_t width = intel_crtc->cursor_width;
>  		uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
>  		if (base) {
>  			cntl &= ~CURSOR_MODE;
> -			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
> +			cntl |= MCURSOR_GAMMA_ENABLE;
> +			switch (width) {
> +			case 64:
> +				cntl |= CURSOR_MODE_64_ARGB_AX;
> +				break;
> +			case 128:
> +				cntl |= CURSOR_MODE_128_ARGB_AX;
> +				break;
> +			case 256:
> +				cntl |= CURSOR_MODE_256_ARGB_AX;
> +				break;
> +			default:
> +				WARN_ON(1);
> +				return;
> +			}
>  		} else {
>  			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
>  			cntl |= CURSOR_MODE_DISABLE;
> @@ -7567,9 +7598,11 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
>  		goto finish;
>  	}
>  
> -	/* Currently we only support 64x64 cursors */
> -	if (width != 64 || height != 64) {
> -		DRM_ERROR("we currently only support 64x64 cursors\n");
> +	/* Check for which cursor types we support */
> +	if (!((width == 64 && height == 64) ||
> +			(width == 128 && height == 128 && !IS_GEN2(dev)) ||
> +			(width == 256 && height == 256 && !IS_GEN2(dev)))) {
> +		DRM_DEBUG("Cursor dimension not supported\n");
>  		return -EINVAL;
>  	}
>  
> @@ -10331,6 +10364,16 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  
>  	drm_crtc_init(dev, &intel_crtc->base, &intel_crtc_funcs);
>  
> +	if (IS_GEN2(dev)) {
> +		intel_crtc->max_cursor_width = GEN2_CURSOR_WIDTH;
> +		intel_crtc->max_cursor_height = GEN2_CURSOR_HEIGHT;
> +	} else {
> +		intel_crtc->max_cursor_width = CURSOR_WIDTH;
> +		intel_crtc->max_cursor_height = CURSOR_HEIGHT;
> +	}
> +	dev->mode_config.cursor_width = intel_crtc->max_cursor_width;
> +	dev->mode_config.cursor_height = intel_crtc->max_cursor_height;
> +
>  	drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256);
>  	for (i = 0; i < 256; i++) {
>  		intel_crtc->lut_r[i] = i;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9c70905..eca4a0a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -78,6 +78,12 @@
>  #define MAX_OUTPUTS 6
>  /* maximum connectors per crtcs in the mode set */
>  
> +/* Maximum cursor sizes */
> +#define GEN2_CURSOR_WIDTH 64
> +#define GEN2_CURSOR_HEIGHT 64
> +#define CURSOR_WIDTH 256
> +#define CURSOR_HEIGHT 256
> +
>  #define INTEL_I2C_BUS_DVO 1
>  #define INTEL_I2C_BUS_SDVO 2
>  
> @@ -364,6 +370,7 @@ struct intel_crtc {
>  	uint32_t cursor_addr;
>  	int16_t cursor_x, cursor_y;
>  	int16_t cursor_width, cursor_height;
> +	int16_t max_cursor_width, max_cursor_height;
>  	bool cursor_visible;
>  
>  	struct intel_crtc_config config;


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1463 bytes --]

On Tue, Feb 18, 2014 at 03:13:33PM +0200, Ville Syrjälä wrote:
[...]
> Once we have drm_planes for cursors, I was thinking we might add some kind
> of enum property that lists all the supported sizes for the plane.

This comment was intriguing, so I was wondering whether you could
elaborate a little on this. The reason why I ask is that we have a
fairly similar issue on Tegra, where recent hardware supports 128x128
and 256x256 hardware cursors, whereas older generations support only
32x32 and 64x64. Also very early generations don't support ARGB cursors,
but a somewhat funky pixel format (2 bpp, background, foreground,
transparent, invert).

With the current set of cursor IOCTLs it isn't really practical to
support the legacy kind of cursors, but representing the cursor as a
plane would have the benefit of attaching a number of supported formats
as well as resolutions to it. That would make it a whole lot easier to
support these additional modes.

As for the supported sizes enumeration, how do you intend to handle the
correlation between horizontal and vertical sizes, since an enumeration
property is only a single 32-bit value. I suppose if we limit ourselves
to supporting only cursors with 64Kx64K we could stash horizontal and
vertical dimensions into a single 32-bit value.

Also, is there a plan to add a type field to the planes so that
userspace can determine if it's a proper overlay or a cursor?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

[Ville Syrjälä]

On Tue, Feb 25, 2014 at 12:35:21PM +0100, Thierry Reding wrote:
> On Tue, Feb 18, 2014 at 03:13:33PM +0200, Ville Syrjälä wrote:
> [...]
> > Once we have drm_planes for cursors, I was thinking we might add some kind
> > of enum property that lists all the supported sizes for the plane.
> 
> This comment was intriguing, so I was wondering whether you could
> elaborate a little on this. The reason why I ask is that we have a
> fairly similar issue on Tegra, where recent hardware supports 128x128
> and 256x256 hardware cursors, whereas older generations support only
> 32x32 and 64x64. Also very early generations don't support ARGB cursors,
> but a somewhat funky pixel format (2 bpp, background, foreground,
> transparent, invert).

Yeah there a a bunch of legacy cursor modes on most hardware which
might be supportable. Whether or not they would actually get used is
another matter though. I think these days everyone expects the
cursor to have alpha at least. I guess userspace could figure out if
the legacy formats are enough to represent the cursor image, and maybe
even decide to take a small quality hit by converting the ARGB image
to one of the legacy formats if it's "close enough".

I think for i915 we'll just not bother with that since all our hardware
has ARGB cursor support.

> 
> With the current set of cursor IOCTLs it isn't really practical to
> support the legacy kind of cursors, but representing the cursor as a
> plane would have the benefit of attaching a number of supported formats
> as well as resolutions to it. That would make it a whole lot easier to
> support these additional modes.
> 
> As for the supported sizes enumeration, how do you intend to handle the
> correlation between horizontal and vertical sizes, since an enumeration
> property is only a single 32-bit value. I suppose if we limit ourselves
> to supporting only cursors with 64Kx64K we could stash horizontal and
> vertical dimensions into a single 32-bit value.

Prop values are 64bit so you could stick a 32x32 dimensions into a
single value. Or you could even ignore the actual value, and just
stick the dimensions to the enum name as "%ux%u". But userspace
would need to parse that, so maybe doing both is the best option.

The only issue here is that enum properties must have a current
value. That doesn't really make sense in this case. So maybe we
actually want to come up with some kind of plane caps mechanism
that would be more fitting for this purpose. OTOH I'm not sure
adding yet another mechanism is worth it.

> 
> Also, is there a plan to add a type field to the planes so that
> userspace can determine if it's a proper overlay or a cursor?

First you have to define what's the differnece between a cursor
and a proper overlay.

-- 
Ville Syrjälä
Intel OTC