On Wed, Oct 14, 2015 at 07:28:56PM +0300, [email protected] wrote: > From: Ville Syrjälä <[email protected]> > > I find more usual to think about tile widths than heights, so changing > the intel_tile_height() to calculate the tile height as > tile_size/tile_width is easier than the opposite to the poor brain. > > v2: Reorder arguments for consistency > Constify dev_priv arguments > > Signed-off-by: Ville Syrjälä <[email protected]> > --- > drivers/gpu/drm/i915/intel_display.c | 88 > ++++++++++++------------------------ > drivers/gpu/drm/i915/intel_drv.h | 5 +- > drivers/gpu/drm/i915/intel_sprite.c | 4 +- > 3 files changed, 34 insertions(+), 63 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index c31fe47..d028326 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2214,6 +2214,11 @@ static bool need_vtd_wa(struct drm_device *dev) > return false; > } > > +static unsigned int intel_tile_size(const struct drm_i915_private *dev_priv) > +{ > + return IS_GEN2(dev_priv) ? 2048 : 4096; > +} > + > static unsigned int intel_tile_width(const struct drm_i915_private *dev_priv, > uint64_t fb_modifier, unsigned int cpp) > { > @@ -2249,67 +2254,34 @@ static unsigned int intel_tile_width(const struct > drm_i915_private *dev_priv, > } > } > > -unsigned int > -intel_tile_height(struct drm_device *dev, uint32_t pixel_format, > - uint64_t fb_format_modifier, unsigned int plane) > +unsigned int intel_tile_height(const struct drm_i915_private *dev_priv, > + uint64_t fb_modifier, unsigned int cpp) > { > - unsigned int tile_height; > - uint32_t pixel_bytes; > - > - switch (fb_format_modifier) { > - case DRM_FORMAT_MOD_NONE: > - tile_height = 1; > - break; > - case I915_FORMAT_MOD_X_TILED: > - tile_height = IS_GEN2(dev) ? 16 : 8; > - break; > - case I915_FORMAT_MOD_Y_TILED: > - tile_height = 32; > - break; > - case I915_FORMAT_MOD_Yf_TILED: > - pixel_bytes = drm_format_plane_cpp(pixel_format, plane); > - switch (pixel_bytes) { > - default: > - case 1: > - tile_height = 64; > - break; > - case 2: > - case 4: > - tile_height = 32; > - break; > - case 8: > - tile_height = 16;
Minus math fail, this here seems to match what I've come up with. Given that's fixed this is Reviewed-by: Daniel Vetter <[email protected]> One thing to consider is to shovel all the tile helpers into i915_gem_tiling.c and add some kerneldoc. Just to avoid intel_display.c becoming even more a dumpster pile of random things. One more bikeshed comment below. > - break; > - case 16: > - WARN_ONCE(1, > - "128-bit pixels are not supported for > display!"); > - tile_height = 16; > - break; > - } > - break; > - default: > - MISSING_CASE(fb_format_modifier); > - tile_height = 1; > - break; > - } > - > - return tile_height; > + if (fb_modifier == DRM_FORMAT_MOD_NONE) > + return 1; > + else > + return intel_tile_size(dev_priv) / > + intel_tile_width(dev_priv, fb_modifier, cpp); > } > > unsigned int > intel_fb_align_height(struct drm_device *dev, unsigned int height, > - uint32_t pixel_format, uint64_t fb_format_modifier) > + uint32_t pixel_format, uint64_t fb_modifier) > { > - return ALIGN(height, intel_tile_height(dev, pixel_format, > - fb_format_modifier, 0)); > + unsigned int cpp = drm_format_plane_cpp(pixel_format, 0); > + unsigned int tile_height = intel_tile_height(to_i915(dev), fb_modifier, > cpp); > + > + return ALIGN(height, tile_height); > } > > static int > intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer > *fb, > const struct drm_plane_state *plane_state) > { > + struct drm_i915_private *dev_priv = to_i915(fb->dev); > struct intel_rotation_info *info = &view->rotation_info; > unsigned int tile_height, tile_pitch; > + unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, 0); > > *view = i915_ggtt_view_normal; > > @@ -2327,22 +2299,19 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, > struct drm_framebuffer *fb, > info->uv_offset = fb->offsets[1]; > info->fb_modifier = fb->modifier[0]; > > - tile_height = intel_tile_height(fb->dev, fb->pixel_format, > - fb->modifier[0], 0); > + tile_height = intel_tile_height(dev_priv, fb->modifier[0], cpp); > tile_pitch = PAGE_SIZE / tile_height; > info->width_pages = DIV_ROUND_UP(fb->pitches[0], tile_pitch); > info->height_pages = DIV_ROUND_UP(fb->height, tile_height); > info->size = info->width_pages * info->height_pages * PAGE_SIZE; > > if (info->pixel_format == DRM_FORMAT_NV12) { > - tile_height = intel_tile_height(fb->dev, fb->pixel_format, > - fb->modifier[0], 1); > + cpp = drm_format_plane_cpp(fb->pixel_format, 1); > + tile_height = intel_tile_height(dev_priv, fb->modifier[1], cpp); > tile_pitch = PAGE_SIZE / tile_height; > - info->width_pages_uv = DIV_ROUND_UP(fb->pitches[0], tile_pitch); > - info->height_pages_uv = DIV_ROUND_UP(fb->height / 2, > - tile_height); > - info->size_uv = info->width_pages_uv * info->height_pages_uv * > - PAGE_SIZE; > + info->width_pages_uv = DIV_ROUND_UP(fb->pitches[1], tile_pitch); > + info->height_pages_uv = DIV_ROUND_UP(fb->height / 2, > tile_height); I guess a follow-up patch will emplouy your shiny new helpers here? Cheers, Daniel > + info->size_uv = info->width_pages_uv * info->height_pages_uv * > PAGE_SIZE; > } > > return 0; > @@ -3102,6 +3071,7 @@ static void skylake_update_primary_plane(struct > drm_crtc *crtc, > int src_x = 0, src_y = 0, src_w = 0, src_h = 0; > int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0; > int scaler_id = -1; > + int pixel_size; > > plane_state = to_intel_plane_state(plane->state); > > @@ -3112,6 +3082,8 @@ static void skylake_update_primary_plane(struct > drm_crtc *crtc, > return; > } > > + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); > + > plane_ctl = PLANE_CTL_ENABLE | > PLANE_CTL_PIPE_GAMMA_ENABLE | > PLANE_CTL_PIPE_CSC_ENABLE; > @@ -3144,8 +3116,8 @@ static void skylake_update_primary_plane(struct > drm_crtc *crtc, > > if (intel_rotation_90_or_270(rotation)) { > /* stride = Surface height in tiles */ > - tile_height = intel_tile_height(dev, fb->pixel_format, > - fb->modifier[0], 0); > + tile_height = intel_tile_height(dev_priv, fb->modifier[0], > + pixel_size); > stride = DIV_ROUND_UP(fb->height, tile_height); > x_offset = stride * tile_height - y - src_h; > y_offset = x; > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 98a8d31..429f744 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1095,9 +1095,8 @@ int intel_plane_atomic_set_property(struct drm_plane > *plane, > int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, > struct drm_plane_state *plane_state); > > -unsigned int > -intel_tile_height(struct drm_device *dev, uint32_t pixel_format, > - uint64_t fb_format_modifier, unsigned int plane); > +unsigned int intel_tile_height(const struct drm_i915_private *dev_priv, > + uint64_t fb_modifier, unsigned int cpp); > > static inline bool > intel_rotation_90_or_270(unsigned int rotation) > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > b/drivers/gpu/drm/i915/intel_sprite.c > index 3d96871..4a1a5f4 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -241,8 +241,8 @@ skl_update_plane(struct drm_plane *drm_plane, struct > drm_crtc *crtc, > > if (intel_rotation_90_or_270(rotation)) { > /* stride: Surface height in tiles */ > - tile_height = intel_tile_height(dev, fb->pixel_format, > - fb->modifier[0], 0); > + tile_height = intel_tile_height(dev_priv, fb->modifier[0], > + pixel_size); > stride = DIV_ROUND_UP(fb->height, tile_height); > plane_size = (src_w << 16) | src_h; > x_offset = stride * tile_height - y - (src_h + 1); > -- > 2.4.9 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
