From: "Antti Koskipää" <antti.koskipaa@linux.intel.com>
To: "daniel@ffwll.ch" <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2] drm/i915: Always apply cursor width changes
Date: Mon, 02 Jun 2014 13:46:11 +0300 [thread overview]
Message-ID: <538C55F3.3030402@linux.intel.com> (raw)
In-Reply-To: <20140602084956.GP19050@phenom.ffwll.local>
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@chris-wilson.co.uk>
>> [Antti:rebased, adjusted macro names and moved some lines, no functional
>> changes]
>> Reviewed-by: Antti Koskipaa <antti.koskipaa@linux.intel.com>
>> Tested-by: Antti Koskipaa <antti.koskipaa@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@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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
--
- Antti
next prev parent reply other threads:[~2014-06-02 10:45 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-30 13:35 [PATCH v2] drm/i915: Always apply cursor width changes Chris Wilson
2014-06-02 8:49 ` Daniel Vetter
2014-06-02 10:46 ` Antti Koskipää [this message]
2014-06-02 15:34 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=538C55F3.3030402@linux.intel.com \
--to=antti.koskipaa@linux.intel.com \
--cc=daniel@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.