On Tue, Aug 12, 2014 at 07:39:55PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> 
> 845/865 support different cursor sizes as well, albeit a bit differently
> than later platforms. Add the necessary code to make them work.
> 
> Untested due to lack of hardware.
> 
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  3 +-
>  drivers/gpu/drm/i915/intel_display.c | 86 
> +++++++++++++++++++++++++-----------
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  3 files changed, 64 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f79c20d..203062e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4128,7 +4128,8 @@ enum punit_power_well {
>  /* Old style CUR*CNTR flags (desktop 8xx) */
>  #define   CURSOR_ENABLE              0x80000000
>  #define   CURSOR_GAMMA_ENABLE        0x40000000
> -#define   CURSOR_STRIDE_MASK 0x30000000
> +#define   CURSOR_STRIDE_SHIFT        28
> +#define   CURSOR_STRIDE(x)   ((ffs(x)-9) << CURSOR_STRIDE_SHIFT) /* 
> 256,512,1k,2k */
>  #define   CURSOR_PIPE_CSC_ENABLE (1<<24)
>  #define   CURSOR_FORMAT_SHIFT        24
>  #define   CURSOR_FORMAT_MASK (0x07 << CURSOR_FORMAT_SHIFT)
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index a1cf052..db10870 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8005,30 +8005,53 @@ 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);
> -     uint32_t cntl;
> +     uint32_t cntl = 0, size = 0;
>  
> -     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;
> +     if (base) {
> +             unsigned int width = intel_crtc->cursor_width;
> +             unsigned int height = intel_crtc->cursor_height;
> +             unsigned int stride = roundup_pow_of_two(width) * 4;
> +
> +             switch (stride) {
> +             case 256:
> +             case 512:
> +             case 1024:
> +             case 2048:
> +                     cntl |= CURSOR_STRIDE(stride);
> +                     break;
> +             default:
> +                     WARN_ON(1);
> +                     return;

Hmm, this is just a programming error. I would rather

switch (stride) {
default:
        WARN_ONCE(1,
                  "Invalid cursor width/stride, width=%d, stride=%d\n",
                  width, stride));
        stride = 256;
        /* fallthrough */
case 256:
case 512:
case 1024:
case 2048:
        break;
}

You get the warning about the programming bug, the user keeps his cursor
(even a corrupt cursor is better than none, and is much easier to
spot!).

Sadly, I only have 845g and so since I only use square cursors in the
ddx I don't actually have the test case I thought I did.

Still,
Reviewed-by: Chris Wilson <ch...@chris-wilson.co.uk>
(preferrably with the minor suggestions taken into account, if not acted
upon :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to