On Mon, Feb 23, 2015 at 03:55:56PM +0000, Tvrtko Ursulin wrote:
> From: Damien Lespiau <damien.lesp...@intel.com>
>
> Skylake is able to scannout those tiling formats. We need to allow them
> in the ADDFB ioctl and tell the harware about it.
>
> v2: Rebased for addfb2 interface. (Tvrtko Ursulin)
> v3: Rebased for fb modifier changes. (Tvrtko Ursulin)
> v4: Don't allow Y tiled fbs just yet. (Tvrtko Ursulin)
> v5: Check for stride alignment and max pitch. (Tvrtko Ursulin)
> v6: Simplify maximum pitch check. (Ville Syrjälä)
> v7: Drop the gen9 check since requirements are no different. (Ville Syrjälä)
>
> Signed-off-by: Damien Lespiau <damien.lesp...@intel.com>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 115
> ++++++++++++++++++++++++-----------
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> drivers/gpu/drm/i915/intel_sprite.c | 18 ++++--
> 3 files changed, 95 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 6e70748..a523d84 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2728,6 +2728,34 @@ static void ironlake_update_primary_plane(struct
> drm_crtc *crtc,
> POSTING_READ(reg);
> }
>
> +u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
> + uint32_t pixel_format)
> +{
> + u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, 0) * 8;
> +
> + /*
> + * The stride is either expressed as a multiple of 64 bytes
> + * chunks for linear buffers or in number of tiles for tiled
> + * buffers.
> + */
> + switch (fb_modifier) {
> + case DRM_FORMAT_MOD_NONE:
> + return 64;
> + case I915_FORMAT_MOD_X_TILED:
if (gen2)
return 128;
> + return 512;
> + case I915_FORMAT_MOD_Y_TILED:
> + return 128;
In theory we could check gen2 and HAS_128_BYTE_Y_TILING() here, but
since old platforms didn't do Y tiled scanout anyway that's not really
needed. But maybe toss in a comment about that so that people don't
start wondering why this doesn't care about that stuff?
> + case I915_FORMAT_MOD_Yf_TILED:
> + if (bits_per_pixel == 8)
> + return 64;
> + else
> + return 128;
> + default:
> + MISSING_CASE(fb_modifier);
> + return 64;
> + }
> +}
> +
> static void skylake_update_primary_plane(struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> int x, int y)
> @@ -2735,10 +2763,9 @@ static void skylake_update_primary_plane(struct
> drm_crtc *crtc,
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_framebuffer *intel_fb;
> struct drm_i915_gem_object *obj;
> int pipe = intel_crtc->pipe;
> - u32 plane_ctl, stride;
> + u32 plane_ctl, stride_div;
>
> if (!intel_crtc->primary_enabled) {
> I915_WRITE(PLANE_CTL(pipe, 0), 0);
> @@ -2782,29 +2809,30 @@ static void skylake_update_primary_plane(struct
> drm_crtc *crtc,
> BUG();
> }
>
> - intel_fb = to_intel_framebuffer(fb);
> - obj = intel_fb->obj;
> -
> - /*
> - * The stride is either expressed as a multiple of 64 bytes chunks for
> - * linear buffers or in number of tiles for tiled buffers.
> - */
> switch (fb->modifier[0]) {
> case DRM_FORMAT_MOD_NONE:
> - stride = fb->pitches[0] >> 6;
> break;
> case I915_FORMAT_MOD_X_TILED:
> plane_ctl |= PLANE_CTL_TILED_X;
> - stride = fb->pitches[0] >> 9;
> + break;
> + case I915_FORMAT_MOD_Y_TILED:
> + plane_ctl |= PLANE_CTL_TILED_Y;
> + break;
> + case I915_FORMAT_MOD_Yf_TILED:
> + plane_ctl |= PLANE_CTL_TILED_YF;
> break;
> default:
> - BUG();
> + MISSING_CASE(fb->modifier[0]);
> }
>
> plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
> if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180))
> plane_ctl |= PLANE_CTL_ROTATE_180;
>
> + obj = intel_fb_obj(fb);
> + stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
> + fb->pixel_format);
> +
> I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
>
> DRM_DEBUG_KMS("Writing base %08lX %d,%d,%d,%d pitch=%d\n",
> @@ -2817,7 +2845,7 @@ static void skylake_update_primary_plane(struct
> drm_crtc *crtc,
> I915_WRITE(PLANE_SIZE(pipe, 0),
> (intel_crtc->config->pipe_src_h - 1) << 16 |
> (intel_crtc->config->pipe_src_w - 1));
> - I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
> + I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div);
> I915_WRITE(PLANE_SURF(pipe, 0), i915_gem_obj_ggtt_offset(obj));
>
> POSTING_READ(PLANE_SURF(pipe, 0));
> @@ -12657,14 +12685,43 @@ static const struct drm_framebuffer_funcs
> intel_fb_funcs = {
> .create_handle = intel_user_framebuffer_create_handle,
> };
>
> +static
> +u32 intel_fb_pitch_limit(struct drm_device *dev, uint64_t fb_modifier,
> + uint32_t pixel_format)
> +{
> + u32 gen = INTEL_INFO(dev)->gen;
> +
> + if (gen >= 9) {
> + /* "The stride in bytes must not exceed the of the size of 8K
> + * pixels and 32K bytes."
> + */
> + return min(8192*drm_format_plane_cpp(pixel_format, 0), 32768);
> + } else if (gen >= 5 && !IS_VALLEYVIEW(dev)) {
> + return 32*1024;
> + } else if (gen >= 4) {
> + if (fb_modifier == I915_FORMAT_MOD_X_TILED)
> + return 16*1024;
> + else
> + return 32*1024;
> + } else if (gen >= 3) {
> + if (fb_modifier == I915_FORMAT_MOD_X_TILED)
> + return 8*1024;
> + else
> + return 16*1024;
> + } else {
> + /* XXX DSPC is limited to 4k tiled */
> + return 8*1024;
> + }
> +}
> +
> static int intel_framebuffer_init(struct drm_device *dev,
> struct intel_framebuffer *intel_fb,
> struct drm_mode_fb_cmd2 *mode_cmd,
> struct drm_i915_gem_object *obj)
> {
> int aligned_height;
> - int pitch_limit;
> int ret;
> + u32 pitch_limit, stride_alignment;
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> @@ -12690,31 +12747,19 @@ static int intel_framebuffer_init(struct drm_device
> *dev,
> return -EINVAL;
> }
>
> - if (mode_cmd->pitches[0] & 63) {
> - DRM_DEBUG("pitch (%d) must be at least 64 byte aligned\n",
> - mode_cmd->pitches[0]);
> + stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[0],
> + mode_cmd->pixel_format);
> + if (mode_cmd->pitches[0] & (stride_alignment - 1)) {
> + DRM_DEBUG("pitch (%d) must be at least %u byte aligned\n",
> + mode_cmd->pitches[0], stride_alignment);
> return -EINVAL;
> }
>
> - if (INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev)) {
> - pitch_limit = 32*1024;
> - } else if (INTEL_INFO(dev)->gen >= 4) {
> - if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)
> - pitch_limit = 16*1024;
> - else
> - pitch_limit = 32*1024;
> - } else if (INTEL_INFO(dev)->gen >= 3) {
> - if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)
> - pitch_limit = 8*1024;
> - else
> - pitch_limit = 16*1024;
> - } else
> - /* XXX DSPC is limited to 4k tiled */
> - pitch_limit = 8*1024;
> -
> + pitch_limit = intel_fb_pitch_limit(dev, mode_cmd->modifier[0],
> + mode_cmd->pixel_format);
We seem to alternate between "stride" and "pitch" somewhat randomly.
Should maybe stick to one or other :)
Anyways this looks pretty good to me. With the gen2 X tiled stride
alignment fixed this is
Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> if (mode_cmd->pitches[0] > pitch_limit) {
> - DRM_DEBUG("%s pitch (%d) must be at less than %d\n",
> - mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED ?
> + DRM_DEBUG("%s pitch (%u) must be at less than %d\n",
> + mode_cmd->modifier[0] != DRM_FORMAT_MOD_NONE ?
> "tiled" : "linear",
> mode_cmd->pitches[0], pitch_limit);
> return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 1de8e20..399d2b2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -883,6 +883,8 @@ int intel_fb_align_height(struct drm_device *dev, int
> height,
> uint64_t fb_format_modifier);
> void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire);
>
> +u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
> + uint32_t pixel_format);
>
> /* intel_audio.c */
> void intel_init_audio(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 5ae56ec..b7103bd 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -189,7 +189,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct
> drm_crtc *crtc,
> struct intel_plane *intel_plane = to_intel_plane(drm_plane);
> const int pipe = intel_plane->pipe;
> const int plane = intel_plane->plane + 1;
> - u32 plane_ctl, stride;
> + u32 plane_ctl, stride_div;
> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>
> plane_ctl = I915_READ(PLANE_CTL(pipe, plane));
> @@ -247,15 +247,20 @@ skl_update_plane(struct drm_plane *drm_plane, struct
> drm_crtc *crtc,
>
> switch (fb->modifier[0]) {
> case DRM_FORMAT_MOD_NONE:
> - stride = fb->pitches[0] >> 6;
> break;
> case I915_FORMAT_MOD_X_TILED:
> plane_ctl |= PLANE_CTL_TILED_X;
> - stride = fb->pitches[0] >> 9;
> + break;
> + case I915_FORMAT_MOD_Y_TILED:
> + plane_ctl |= PLANE_CTL_TILED_Y;
> + break;
> + case I915_FORMAT_MOD_Yf_TILED:
> + plane_ctl |= PLANE_CTL_TILED_YF;
> break;
> default:
> - BUG();
> + MISSING_CASE(fb->modifier[0]);
> }
> +
> if (drm_plane->state->rotation == BIT(DRM_ROTATE_180))
> plane_ctl |= PLANE_CTL_ROTATE_180;
>
> @@ -266,6 +271,9 @@ skl_update_plane(struct drm_plane *drm_plane, struct
> drm_crtc *crtc,
> pixel_size, true,
> src_w != crtc_w || src_h != crtc_h);
>
> + stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
> + fb->pixel_format);
> +
> /* Sizes are 0 based */
> src_w--;
> src_h--;
> @@ -273,7 +281,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct
> drm_crtc *crtc,
> crtc_h--;
>
> I915_WRITE(PLANE_OFFSET(pipe, plane), (y << 16) | x);
> - I915_WRITE(PLANE_STRIDE(pipe, plane), stride);
> + I915_WRITE(PLANE_STRIDE(pipe, plane), fb->pitches[0] / stride_div);
> I915_WRITE(PLANE_POS(pipe, plane), (crtc_y << 16) | crtc_x);
> I915_WRITE(PLANE_SIZE(pipe, plane), (crtc_h << 16) | crtc_w);
> I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
> --
> 2.3.0
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
