[Intel-gfx] [PATCH v2] drm/i915: Always apply cursor width changes

Antti Koskipää antti.koskipaa at linux.intel.com
Mon Jun 2 12:46:11 CEST 2014


On 06/02/2014 11:49 AM, Daniel Vetter wrote:
> On Fri, May 30, 2014 at 04:35:26PM +0300, Chris Wilson wrote:
>> It is possible for userspace to create a big object large enough for a
>> 256x256, and then switch over to using it as a 64x64 cursor. This
>> requires the cursor update routines to check for a change in width on
>> every update, rather than just when the cursor is originally enabled.
>>
>> This also fixes an issue with 845g/865g which cannot change the base
>> address of the cursor whilst it is active.
>>
>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>> [Antti:rebased, adjusted macro names and moved some lines, no functional
>> changes]
>> Reviewed-by: Antti Koskipaa <antti.koskipaa at linux.intel.com>
>> Tested-by: Antti Koskipaa <antti.koskipaa at linux.intel.com>
> 
> Still missing the igt testcase. Is Antti still working on that one?

The testcase was just posted. It just needed some cleanup.

> And I
> guess now we also need an Cc: stable at vger.kernel.org on this one here.
> -Daniel
> 
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c  |   2 +-
>>  drivers/gpu/drm/i915/intel_display.c | 106 +++++++++++++++++------------------
>>  drivers/gpu/drm/i915/intel_drv.h     |   3 +-
>>  3 files changed, 53 insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 2e5f76a..04d0b7c 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2353,7 +2353,7 @@ static int i915_display_info(struct seq_file *m, void *unused)
>>  
>>  			active = cursor_position(dev, crtc->pipe, &x, &y);
>>  			seq_printf(m, "\tcursor visible? %s, position (%d, %d), addr 0x%08x, active? %s\n",
>> -				   yesno(crtc->cursor_visible),
>> +				   yesno(crtc->cursor_base),
>>  				   x, y, crtc->cursor_addr,
>>  				   yesno(active));
>>  		}
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 731cd01..955f92d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -7872,29 +7872,33 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base)
>>  	struct drm_device *dev = crtc->dev;
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> -	bool visible = base != 0;
>> -	u32 cntl;
>> +	uint32_t cntl;
>>  
>> -	if (intel_crtc->cursor_visible == visible)
>> -		return;
>> -
>> -	cntl = I915_READ(_CURACNTR);
>> -	if (visible) {
>> +	if (base != intel_crtc->cursor_base) {
>>  		/* On these chipsets we can only modify the base whilst
>>  		 * the cursor is disabled.
>>  		 */
>> +		if (intel_crtc->cursor_cntl) {
>> +			I915_WRITE(_CURACNTR, 0);
>> +			POSTING_READ(_CURACNTR);
>> +			intel_crtc->cursor_cntl = 0;
>> +		}
>> +
>>  		I915_WRITE(_CURABASE, base);
>> +		POSTING_READ(_CURABASE);
>> +	}
>>  
>> -		cntl &= ~(CURSOR_FORMAT_MASK);
>> -		/* XXX width must be 64, stride 256 => 0x00 << 28 */
>> -		cntl |= CURSOR_ENABLE |
>> +	/* XXX width must be 64, stride 256 => 0x00 << 28 */
>> +	cntl = 0;
>> +	if (base)
>> +		cntl = (CURSOR_ENABLE |
>>  			CURSOR_GAMMA_ENABLE |
>> -			CURSOR_FORMAT_ARGB;
>> -	} else
>> -		cntl &= ~(CURSOR_ENABLE | CURSOR_GAMMA_ENABLE);
>> -	I915_WRITE(_CURACNTR, cntl);
>> -
>> -	intel_crtc->cursor_visible = visible;
>> +			CURSOR_FORMAT_ARGB);
>> +	if (intel_crtc->cursor_cntl != cntl) {
>> +		I915_WRITE(_CURACNTR, cntl);
>> +		POSTING_READ(_CURACNTR);
>> +		intel_crtc->cursor_cntl = cntl;
>> +	}
>>  }
>>  
>>  static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
>> @@ -7903,16 +7907,12 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>  	int pipe = intel_crtc->pipe;
>> -	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 |= MCURSOR_GAMMA_ENABLE;
>> +	uint32_t cntl;
>>  
>> -			switch (width) {
>> +	cntl = 0;
>> +	if (base) {
>> +		cntl = MCURSOR_GAMMA_ENABLE;
>> +		switch (intel_crtc->cursor_width) {
>>  			case 64:
>>  				cntl |= CURSOR_MODE_64_ARGB_AX;
>>  				break;
>> @@ -7925,18 +7925,16 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
>>  			default:
>>  				WARN_ON(1);
>>  				return;
>> -			}
>> -			cntl |= pipe << 28; /* Connect to correct pipe */
>> -		} else {
>> -			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
>> -			cntl |= CURSOR_MODE_DISABLE;
>>  		}
>> +		cntl |= pipe << 28; /* Connect to correct pipe */
>> +	}
>> +	if (intel_crtc->cursor_cntl != cntl) {
>>  		I915_WRITE(CURCNTR(pipe), cntl);
>> -
>> -		intel_crtc->cursor_visible = visible;
>> +		POSTING_READ(CURCNTR(pipe));
>> +		intel_crtc->cursor_cntl = cntl;
>>  	}
>> +
>>  	/* and commit changes on next vblank */
>> -	POSTING_READ(CURCNTR(pipe));
>>  	I915_WRITE(CURBASE(pipe), base);
>>  	POSTING_READ(CURBASE(pipe));
>>  }
>> @@ -7947,15 +7945,12 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>  	int pipe = intel_crtc->pipe;
>> -	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;
>> -			cntl |= MCURSOR_GAMMA_ENABLE;
>> -			switch (width) {
>> +	uint32_t cntl;
>> +
>> +	cntl = 0;
>> +	if (base) {
>> +		cntl = MCURSOR_GAMMA_ENABLE;
>> +		switch (intel_crtc->cursor_width) {
>>  			case 64:
>>  				cntl |= CURSOR_MODE_64_ARGB_AX;
>>  				break;
>> @@ -7968,21 +7963,18 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
>>  			default:
>>  				WARN_ON(1);
>>  				return;
>> -			}
>> -		} else {
>> -			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
>> -			cntl |= CURSOR_MODE_DISABLE;
>> -		}
>> -		if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>> -			cntl |= CURSOR_PIPE_CSC_ENABLE;
>> -			cntl &= ~CURSOR_TRICKLE_FEED_DISABLE;
>>  		}
>> -		I915_WRITE(CURCNTR(pipe), cntl);
>> +	}
>> +	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>> +		cntl |= CURSOR_PIPE_CSC_ENABLE;
>>  
>> -		intel_crtc->cursor_visible = visible;
>> +	if (intel_crtc->cursor_cntl != cntl) {
>> +		I915_WRITE(CURCNTR(pipe), cntl);
>> +		POSTING_READ(CURCNTR(pipe));
>> +		intel_crtc->cursor_cntl = cntl;
>>  	}
>> +
>>  	/* and commit changes on next vblank */
>> -	POSTING_READ(CURCNTR(pipe));
>>  	I915_WRITE(CURBASE(pipe), base);
>>  	POSTING_READ(CURBASE(pipe));
>>  }
>> @@ -7998,7 +7990,6 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
>>  	int x = intel_crtc->cursor_x;
>>  	int y = intel_crtc->cursor_y;
>>  	u32 base = 0, pos = 0;
>> -	bool visible;
>>  
>>  	if (on)
>>  		base = intel_crtc->cursor_addr;
>> @@ -8027,8 +8018,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
>>  	}
>>  	pos |= y << CURSOR_Y_SHIFT;
>>  
>> -	visible = base != 0;
>> -	if (!visible && !intel_crtc->cursor_visible)
>> +	if (base == 0 && intel_crtc->cursor_base == 0)
>>  		return;
>>  
>>  	I915_WRITE(CURPOS(pipe), pos);
>> @@ -8039,6 +8029,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
>>  		i845_update_cursor(crtc, base);
>>  	else
>>  		i9xx_update_cursor(crtc, base);
>> +	intel_crtc->cursor_base = base;
>>  }
>>  
>>  static int intel_crtc_cursor_set(struct drm_crtc *crtc,
>> @@ -10970,6 +10961,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>>  		intel_crtc->plane = !pipe;
>>  	}
>>  
>> +	intel_crtc->cursor_base = ~0;
>> +	intel_crtc->cursor_cntl = ~0;
>> +
>>  	init_waitqueue_head(&intel_crtc->vbl_wait);
>>  
>>  	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 9bb70dc..9d1ff7b 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -386,7 +386,8 @@ struct intel_crtc {
>>  	uint32_t cursor_addr;
>>  	int16_t cursor_x, cursor_y;
>>  	int16_t cursor_width, cursor_height;
>> -	bool cursor_visible;
>> +	uint32_t cursor_cntl;
>> +	uint32_t cursor_base;
>>  
>>  	struct intel_plane_config plane_config;
>>  	struct intel_crtc_config config;
>> -- 
>> 1.8.3.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

-- 
- Antti




More information about the Intel-gfx mailing list