Re: [Intel-gfx] [PATCH 2/9] drm/i915: Plumb drm_framebuffer into more places

Imre Deak
2017-02-02T13:30:12Z

On Wed, Jan 04, 2017 at 08:42:25PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that framebuffers can be used even before calling
> drm_framebuffer_init() we can start to plumb them into more places,
> instead of passing individual pieces for fb metadata.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
> drivers/gpu/drm/i915/intel_display.c | 127 +++++++++++++++--------------------
> drivers/gpu/drm/i915/intel_drv.h | 11 +--
> drivers/gpu/drm/i915/intel_fbdev.c | 4 +-
> 3 files changed, 57 insertions(+), 85 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e2150a64860c..f0cb80aba89a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2050,10 +2050,13 @@ 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_bytes(const struct drm_i915_private *dev_priv,
> - uint64_t fb_modifier, unsigned int cpp)
> +static unsigned int
> +intel_tile_width_bytes(const struct drm_framebuffer *fb, int plane)
> {
> - switch (fb_modifier) {
> + struct drm_i915_private *dev_priv = to_i915(fb->dev);
> + unsigned int cpp = fb->format->cpp[plane];
> +
> + switch (fb->modifier) {
> case DRM_FORMAT_MOD_NONE:
> return cpp;
> case I915_FORMAT_MOD_X_TILED:
> @@ -2082,41 +2085,38 @@ static unsigned int intel_tile_width_bytes(const struct drm_i915_private *dev_pr
> }
> break;
> default:
> - MISSING_CASE(fb_modifier);
> + MISSING_CASE(fb->modifier);
> return cpp;
> }
> }
>
> -unsigned int intel_tile_height(const struct drm_i915_private *dev_priv,
> - uint64_t fb_modifier, unsigned int cpp)
> +static unsigned int
> +intel_tile_height(const struct drm_framebuffer *fb, int plane)
> {
> - if (fb_modifier == DRM_FORMAT_MOD_NONE)
> + if (fb->modifier == DRM_FORMAT_MOD_NONE)
> return 1;
> else
> - return intel_tile_size(dev_priv) /
> - intel_tile_width_bytes(dev_priv, fb_modifier, cpp);
> + return intel_tile_size(to_i915(fb->dev)) /
> + intel_tile_width_bytes(fb, plane);
> }
>
> /* Return the tile dimensions in pixel units */
> -static void intel_tile_dims(const struct drm_i915_private *dev_priv,
> +static void intel_tile_dims(const struct drm_framebuffer *fb, int plane,
> unsigned int *tile_width,
> - unsigned int *tile_height,
> - uint64_t fb_modifier,
> - unsigned int cpp)
> + unsigned int *tile_height)
> {
> - unsigned int tile_width_bytes =
> - intel_tile_width_bytes(dev_priv, fb_modifier, cpp);
> + unsigned int tile_width_bytes = intel_tile_width_bytes(fb, plane);
> + unsigned int cpp = fb->format->cpp[plane];
>
> *tile_width = tile_width_bytes / cpp;
> - *tile_height = intel_tile_size(dev_priv) / tile_width_bytes;
> + *tile_height = intel_tile_size(to_i915(fb->dev)) / tile_width_bytes;
> }
>
> unsigned int
> -intel_fb_align_height(struct drm_device *dev, unsigned int height,
> - uint32_t pixel_format, uint64_t fb_modifier)
> +intel_fb_align_height(const struct drm_framebuffer *fb,
> + int plane, unsigned int height)
> {
> - unsigned int cpp = drm_format_plane_cpp(pixel_format, 0);
> - unsigned int tile_height = intel_tile_height(to_i915(dev), fb_modifier, cpp);
> + unsigned int tile_height = intel_tile_height(fb, plane);
>
> return ALIGN(height, tile_height);
> }
> @@ -2158,21 +2158,23 @@ static unsigned int intel_linear_alignment(const struct drm_i915_private *dev_pr
> return 0;
> }
>
> -static unsigned int intel_surf_alignment(const struct drm_i915_private *dev_priv,
> - uint64_t fb_modifier)
> +static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
> + int plane)
> {
> - switch (fb_modifier) {
> + struct drm_i915_private *dev_priv = to_i915(fb->dev);
> +
> + switch (fb->modifier) {
> case DRM_FORMAT_MOD_NONE:
> return intel_linear_alignment(dev_priv);
> case I915_FORMAT_MOD_X_TILED:
> - if (INTEL_INFO(dev_priv)->gen >= 9)
> + if (INTEL_GEN(dev_priv) >= 9)
> return 256 * 1024;
> return 0;
> case I915_FORMAT_MOD_Y_TILED:
> case I915_FORMAT_MOD_Yf_TILED:
> return 1 * 1024 * 1024;
> default:
> - MISSING_CASE(fb_modifier);
> + MISSING_CASE(fb->modifier);
> return 0;
> }
> }
> @@ -2189,7 +2191,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> - alignment = intel_surf_alignment(dev_priv, fb->modifier);
> + alignment = intel_surf_alignment(fb, 0);
>
> intel_fill_fb_ggtt_view(&view, fb, rotation);
>
> @@ -2355,8 +2357,7 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
> unsigned int pitch_tiles;
>
> tile_size = intel_tile_size(dev_priv);
> - intel_tile_dims(dev_priv, &tile_width, &tile_height,
> - fb->modifier, cpp);
> + intel_tile_dims(fb, plane, &tile_width, &tile_height);
>
> if (drm_rotation_90_or_270(rotation)) {
> pitch_tiles = pitch / tile_height;
> @@ -2411,8 +2412,7 @@ static u32 _intel_compute_tile_offset(const struct drm_i915_private *dev_priv,
> unsigned int tile_rows, tiles, pitch_tiles;
>
> tile_size = intel_tile_size(dev_priv);
> - intel_tile_dims(dev_priv, &tile_width, &tile_height,
> - fb_modifier, cpp);
> + intel_tile_dims(fb, plane, &tile_width, &tile_height);
>
> if (drm_rotation_90_or_270(rotation)) {
> pitch_tiles = pitch / tile_height;
> @@ -2458,7 +2458,7 @@ u32 intel_compute_tile_offset(int *x, int *y,
> if (fb->format->format == DRM_FORMAT_NV12 && plane == 1)
> alignment = 4096;
> else
> - alignment = intel_surf_alignment(dev_priv, fb->modifier);
> + alignment = intel_surf_alignment(fb, plane);
>
> return _intel_compute_tile_offset(dev_priv, x, y, fb, plane, pitch,
> rotation, alignment);
> @@ -2544,8 +2544,7 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
> unsigned int pitch_tiles;
> struct drm_rect r;
>
> - intel_tile_dims(dev_priv, &tile_width, &tile_height,
> - fb->modifier, cpp);
> + intel_tile_dims(fb, i, &tile_width, &tile_height);
>
> rot_info->plane[i].offset = offset;
> rot_info->plane[i].stride = DIV_ROUND_UP(fb->pitches[i], tile_width * cpp);
> @@ -2873,7 +2872,6 @@ static int skl_max_plane_width(const struct drm_framebuffer *fb, int plane,
>
> static int skl_check_main_surface(struct intel_plane_state *plane_state)
> {
> - const struct drm_i915_private *dev_priv = to_i915(plane_state->base.plane->dev);
> const struct drm_framebuffer *fb = plane_state->base.fb;
> unsigned int rotation = plane_state->base.rotation;
> int x = plane_state->base.src.x1 >> 16;
> @@ -2892,8 +2890,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>
> intel_add_fb_offsets(&x, &y, plane_state, 0);
> offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
> -
> - alignment = intel_surf_alignment(dev_priv, fb->modifier);
> + alignment = intel_surf_alignment(fb, 0);
>
> /*
> * AUX surface offset is specified as the distance from the
> @@ -3210,16 +3207,13 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
> POSTING_READ(reg);
> }
>
> -u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
> - uint64_t fb_modifier, uint32_t pixel_format)
> +static u32
> +intel_fb_stride_alignment(const struct drm_framebuffer *fb, int plane)
> {
> - if (fb_modifier == DRM_FORMAT_MOD_NONE) {
> + if (fb->modifier == DRM_FORMAT_MOD_NONE)
> return 64;
> - } else {
> - int cpp = drm_format_plane_cpp(pixel_format, 0);
> -
> - return intel_tile_width_bytes(dev_priv, fb_modifier, cpp);
> - }
> + else
> + return intel_tile_width_bytes(fb, plane);
> }
>
> u32 intel_fb_gtt_offset(struct drm_framebuffer *fb,
> @@ -3269,21 +3263,16 @@ static void skl_detach_scalers(struct intel_crtc *intel_crtc)
> u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
> unsigned int rotation)
> {
> - const struct drm_i915_private *dev_priv = to_i915(fb->dev);
> u32 stride = intel_fb_pitch(fb, plane, rotation);
>
> /*
> * The stride is either expressed as a multiple of 64 bytes chunks for
> * linear buffers or in number of tiles for tiled buffers.
> */
> - if (drm_rotation_90_or_270(rotation)) {
> - int cpp = fb->format->cpp[plane];
> -
> - stride /= intel_tile_height(dev_priv, fb->modifier, cpp);
> - } else {
> - stride /= intel_fb_stride_alignment(dev_priv, fb->modifier,
> - fb->format->format);
> - }
> + if (drm_rotation_90_or_270(rotation))
> + stride /= intel_tile_height(fb, plane);
> + else
> + stride /= intel_fb_stride_alignment(fb, plane);
>
> return stride;
> }
> @@ -8773,9 +8762,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
> val = I915_READ(DSPSTRIDE(pipe));
> fb->pitches[0] = val & 0xffffffc0;
>
> - aligned_height = intel_fb_align_height(dev, fb->height,
> - fb->format->format,
> - fb->modifier);
> + aligned_height = intel_fb_align_height(fb, 0, fb->height);
>
> plane_config->size = fb->pitches[0] * aligned_height;
>
> @@ -9810,13 +9797,10 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> fb->width = ((val >> 0) & 0x1fff) + 1;
>
> val = I915_READ(PLANE_STRIDE(pipe, 0));
> - stride_mult = intel_fb_stride_alignment(dev_priv, fb->modifier,
> - fb->format->format);
> + stride_mult = intel_fb_stride_alignment(fb, 0);
> fb->pitches[0] = (val & 0x3ff) * stride_mult;
>
> - aligned_height = intel_fb_align_height(dev, fb->height,
> - fb->format->format,
> - fb->modifier);
> + aligned_height = intel_fb_align_height(fb, 0, fb->height);
>
> plane_config->size = fb->pitches[0] * aligned_height;
>
> @@ -9912,9 +9896,7 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
> val = I915_READ(DSPSTRIDE(pipe));
> fb->pitches[0] = val & 0xffffffc0;
>
> - aligned_height = intel_fb_align_height(dev, fb->height,
> - fb->format->format,
> - fb->modifier);
> + aligned_height = intel_fb_align_height(fb, 0, fb->height);
>
> plane_config->size = fb->pitches[0] * aligned_height;
>
> @@ -15967,15 +15949,6 @@ static int intel_framebuffer_init(struct drm_device *dev,
> return -EINVAL;
> }
>
> - stride_alignment = intel_fb_stride_alignment(dev_priv,
> - 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;
> - }
> -
> pitch_limit = intel_fb_pitch_limit(dev_priv, mode_cmd->modifier[0],
> mode_cmd->pixel_format);
> if (mode_cmd->pitches[0] > pitch_limit) {
> @@ -16057,6 +16030,14 @@ static int intel_framebuffer_init(struct drm_device *dev,
> return -EINVAL;
>
> drm_helper_mode_fill_fb_struct(dev, &intel_fb->base, mode_cmd);
> +
> + stride_alignment = intel_fb_stride_alignment(&intel_fb->base, 0);
> + 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;
> + }
> +
> intel_fb->obj = obj;
>
> ret = intel_fill_fb_info(dev_priv, &intel_fb->base);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6b02dac6ea26..e3d19fc6720c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1208,12 +1208,8 @@ void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
> uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
> struct intel_shared_dpll *intel_ddi_get_link_dpll(struct intel_dp *intel_dp,
> int clock);
> -unsigned int intel_fb_align_height(struct drm_device *dev,
> - unsigned int height,
> - uint32_t pixel_format,
> - uint64_t fb_format_modifier);
> -u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
> - uint64_t fb_modifier, uint32_t pixel_format);
> +unsigned int intel_fb_align_height(const struct drm_framebuffer *fb,
> + int plane, unsigned int height);
>
> /* intel_audio.c */
> void intel_init_audio_hooks(struct drm_i915_private *dev_priv);
> @@ -1325,9 +1321,6 @@ 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(const struct drm_i915_private *dev_priv,
> - uint64_t fb_modifier, unsigned int cpp);
> -
> void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,
> enum pipe pipe);
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 73d02d21c768..3716554e32a9 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -631,9 +631,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
> }
>
> cur_size = intel_crtc->config->base.adjusted_mode.crtc_vdisplay;
> - cur_size = intel_fb_align_height(dev, cur_size,
> - fb->base.format->format,
> - fb->base.modifier);
> + cur_size = intel_fb_align_height(&fb->base, 0, cur_size);
> cur_size *= fb->base.pitches[0];
> DRM_DEBUG_KMS("pipe %c area: %dx%d, bpp: %d, size: %d\n",
> pipe_name(intel_crtc->pipe),
> --
> 2.10.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org