Re: [PATCH v2 11/14] drm/i915: Add explicit pipe src size to pipe config

Damien Lespiau
2013-09-10T17:03:11Z

On Wed, Sep 04, 2013 at 06:25:28PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Rather that mess about with hdisplay/vdisplay from requested_mode, add
> explicit pipe src size information to pipe config.
>
> Now requested_mode is only really relevant for dvo/sdvo output timings.
> For everything else either adjusted_mode or pipe src size should be
> used.
>
> In many places where we end up using pipe source size, we should
> actually use the primary plane size, but we don't currently store
> that information explicitly. As long as we treat primaries as full
> screen only, we can get away with this. Eventually when we move
> primaries over to drm_plane, we need to fix it all up.
>
> v2: Add a comment to explain what pipe_src_{w,h} are
> Add a note about primary planes to commit message
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

--
Damien

> ---
> drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++------
> drivers/gpu/drm/i915/intel_drv.h | 6 ++++
> drivers/gpu/drm/i915/intel_panel.c | 56 +++++++++++++++++-------------------
> drivers/gpu/drm/i915/intel_pm.c | 33 ++++++++++-----------
> drivers/gpu/drm/i915/intel_sprite.c | 4 +--
> 5 files changed, 67 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 71c7763..8d64d54 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4673,7 +4673,6 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
> enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
> struct drm_display_mode *adjusted_mode =
> &intel_crtc->config.adjusted_mode;
> - struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
> uint32_t vsyncshift, crtc_vtotal, crtc_vblank_end;
>
> /* We need to be careful not to changed the adjusted mode, for otherwise
> @@ -4726,7 +4725,8 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
> * always be the user's requested size.
> */
> I915_WRITE(PIPESRC(pipe),
> - ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
> + ((intel_crtc->config.pipe_src_w - 1) << 16) |
> + (intel_crtc->config.pipe_src_h - 1));
> }
>
> static void intel_get_pipe_timings(struct intel_crtc *crtc,
> @@ -4764,8 +4764,11 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
> }
>
> tmp = I915_READ(PIPESRC(crtc->pipe));
> - pipe_config->requested_mode.vdisplay = (tmp & 0xffff) + 1;
> - pipe_config->requested_mode.hdisplay = ((tmp >> 16) & 0xffff) + 1;
> + pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> + pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
> +
> + pipe_config->requested_mode.vdisplay = pipe_config->pipe_src_h;
> + pipe_config->requested_mode.hdisplay = pipe_config->pipe_src_w;
> }
>
> static void intel_crtc_mode_from_pipe_config(struct intel_crtc *intel_crtc,
> @@ -4861,7 +4864,6 @@ static int i9xx_crtc_mode_set(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 drm_display_mode *mode = &intel_crtc->config.requested_mode;
> int pipe = intel_crtc->pipe;
> int plane = intel_crtc->plane;
> int refclk, num_connectors = 0;
> @@ -4960,8 +4962,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> * which should always be the user's requested size.
> */
> I915_WRITE(DSPSIZE(plane),
> - ((mode->vdisplay - 1) << 16) |
> - (mode->hdisplay - 1));
> + ((intel_crtc->config.pipe_src_h - 1) << 16) |
> + (intel_crtc->config.pipe_src_w - 1));
> I915_WRITE(DSPPOS(plane), 0);
>
> i9xx_set_pipeconf(intel_crtc);
> @@ -8281,6 +8283,8 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
> DRM_DEBUG_KMS("adjusted mode:\n");
> drm_mode_debug_printmodeline(&pipe_config->adjusted_mode);
> DRM_DEBUG_KMS("port clock: %d\n", pipe_config->port_clock);
> + DRM_DEBUG_KMS("pipe src size: %dx%d\n",
> + pipe_config->pipe_src_w, pipe_config->pipe_src_h);
> DRM_DEBUG_KMS("gmch pfit: control: 0x%08x, ratios: 0x%08x, lvds border: 0x%08x\n",
> pipe_config->gmch_pfit.control,
> pipe_config->gmch_pfit.pgm_ratios,
> @@ -8332,6 +8336,10 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>
> drm_mode_copy(&pipe_config->adjusted_mode, mode);
> drm_mode_copy(&pipe_config->requested_mode, mode);
> +
> + pipe_config->pipe_src_w = mode->hdisplay;
> + pipe_config->pipe_src_h = mode->vdisplay;
> +
> pipe_config->cpu_transcoder =
> (enum transcoder) to_intel_crtc(crtc)->pipe;
> pipe_config->shared_dpll = DPLL_ID_PRIVATE;
> @@ -8681,8 +8689,8 @@ intel_pipe_config_compare(struct drm_device *dev,
> DRM_MODE_FLAG_NVSYNC);
> }
>
> - PIPE_CONF_CHECK_I(requested_mode.hdisplay);
> - PIPE_CONF_CHECK_I(requested_mode.vdisplay);
> + PIPE_CONF_CHECK_I(pipe_src_w);
> + PIPE_CONF_CHECK_I(pipe_src_h);
>
> PIPE_CONF_CHECK_I(gmch_pfit.control);
> /* pfit ratios are autocomputed by the hw on gen4+ */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 59f0ec7..9d30909 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -215,6 +215,12 @@ struct intel_crtc_config {
> /* Actual pipe timings ie. what we program into the pipe timing
> * registers. adjusted_mode.clock is the pipe pixel clock. */
> struct drm_display_mode adjusted_mode;
> +
> + /* Pipe source size (ie. panel fitter input size)
> + * All planes will be positioned inside this space,
> + * and get clipped at the edges. */
> + int pipe_src_w, pipe_src_h;
> +
> /* Whether to set up the PCH/FDI. Note that we never allow sharing
> * between pch encoders and cpu encoders. */
> bool has_pch_encoder;
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 42114ec..c9dba46 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -50,23 +50,22 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> struct intel_crtc_config *pipe_config,
> int fitting_mode)
> {
> - struct drm_display_mode *mode, *adjusted_mode;
> + struct drm_display_mode *adjusted_mode;
> int x, y, width, height;
>
> - mode = &pipe_config->requested_mode;
> adjusted_mode = &pipe_config->adjusted_mode;
>
> x = y = width = height = 0;
>
> /* Native modes don't need fitting */
> - if (adjusted_mode->hdisplay == mode->hdisplay &&
> - adjusted_mode->vdisplay == mode->vdisplay)
> + if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> + adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> goto done;
>
> switch (fitting_mode) {
> case DRM_MODE_SCALE_CENTER:
> - width = mode->hdisplay;
> - height = mode->vdisplay;
> + width = pipe_config->pipe_src_w;
> + height = pipe_config->pipe_src_h;
> x = (adjusted_mode->hdisplay - width + 1)/2;
> y = (adjusted_mode->vdisplay - height + 1)/2;
> break;
> @@ -74,17 +73,17 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> case DRM_MODE_SCALE_ASPECT:
> /* Scale but preserve the aspect ratio */
> {
> - u32 scaled_width = adjusted_mode->hdisplay * mode->vdisplay;
> - u32 scaled_height = mode->hdisplay * adjusted_mode->vdisplay;
> + u32 scaled_width = adjusted_mode->hdisplay * pipe_config->pipe_src_h;
> + u32 scaled_height = pipe_config->pipe_src_w * adjusted_mode->vdisplay;
> if (scaled_width > scaled_height) { /* pillar */
> - width = scaled_height / mode->vdisplay;
> + width = scaled_height / pipe_config->pipe_src_h;
> if (width & 1)
> width++;
> x = (adjusted_mode->hdisplay - width + 1) / 2;
> y = 0;
> height = adjusted_mode->vdisplay;
> } else if (scaled_width < scaled_height) { /* letter */
> - height = scaled_width / mode->hdisplay;
> + height = scaled_width / pipe_config->pipe_src_w;
> if (height & 1)
> height++;
> y = (adjusted_mode->vdisplay - height + 1) / 2;
> @@ -176,14 +175,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> {
> struct drm_device *dev = intel_crtc->base.dev;
> u32 pfit_control = 0, pfit_pgm_ratios = 0, border = 0;
> - struct drm_display_mode *mode, *adjusted_mode;
> + struct drm_display_mode *adjusted_mode;
>
> - mode = &pipe_config->requested_mode;
> adjusted_mode = &pipe_config->adjusted_mode;
>
> /* Native modes don't need fitting */
> - if (adjusted_mode->hdisplay == mode->hdisplay &&
> - adjusted_mode->vdisplay == mode->vdisplay)
> + if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> + adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> goto out;
>
> switch (fitting_mode) {
> @@ -192,16 +190,16 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> * For centered modes, we have to calculate border widths &
> * heights and modify the values programmed into the CRTC.
> */
> - centre_horizontally(adjusted_mode, mode->hdisplay);
> - centre_vertically(adjusted_mode, mode->vdisplay);
> + centre_horizontally(adjusted_mode, pipe_config->pipe_src_w);
> + centre_vertically(adjusted_mode, pipe_config->pipe_src_h);
> border = LVDS_BORDER_ENABLE;
> break;
> case DRM_MODE_SCALE_ASPECT:
> /* Scale but preserve the aspect ratio */
> if (INTEL_INFO(dev)->gen >= 4) {
> u32 scaled_width = adjusted_mode->hdisplay *
> - mode->vdisplay;
> - u32 scaled_height = mode->hdisplay *
> + pipe_config->pipe_src_h;
> + u32 scaled_height = pipe_config->pipe_src_w *
> adjusted_mode->vdisplay;
>
> /* 965+ is easy, it does everything in hw */
> @@ -211,12 +209,12 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> else if (scaled_width < scaled_height)
> pfit_control |= PFIT_ENABLE |
> PFIT_SCALING_LETTER;
> - else if (adjusted_mode->hdisplay != mode->hdisplay)
> + else if (adjusted_mode->hdisplay != pipe_config->pipe_src_w)
> pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
> } else {
> u32 scaled_width = adjusted_mode->hdisplay *
> - mode->vdisplay;
> - u32 scaled_height = mode->hdisplay *
> + pipe_config->pipe_src_h;
> + u32 scaled_height = pipe_config->pipe_src_w *
> adjusted_mode->vdisplay;
> /*
> * For earlier chips we have to calculate the scaling
> @@ -226,11 +224,11 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> if (scaled_width > scaled_height) { /* pillar */
> centre_horizontally(adjusted_mode,
> scaled_height /
> - mode->vdisplay);
> + pipe_config->pipe_src_h);
>
> border = LVDS_BORDER_ENABLE;
> - if (mode->vdisplay != adjusted_mode->vdisplay) {
> - u32 bits = panel_fitter_scaling(mode->vdisplay, adjusted_mode->vdisplay);
> + if (pipe_config->pipe_src_h != adjusted_mode->vdisplay) {
> + u32 bits = panel_fitter_scaling(pipe_config->pipe_src_h, adjusted_mode->vdisplay);
> pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
> bits << PFIT_VERT_SCALE_SHIFT);
> pfit_control |= (PFIT_ENABLE |
> @@ -240,11 +238,11 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> } else if (scaled_width < scaled_height) { /* letter */
> centre_vertically(adjusted_mode,
> scaled_width /
> - mode->hdisplay);
> + pipe_config->pipe_src_w);
>
> border = LVDS_BORDER_ENABLE;
> - if (mode->hdisplay != adjusted_mode->hdisplay) {
> - u32 bits = panel_fitter_scaling(mode->hdisplay, adjusted_mode->hdisplay);
> + if (pipe_config->pipe_src_w != adjusted_mode->hdisplay) {
> + u32 bits = panel_fitter_scaling(pipe_config->pipe_src_w, adjusted_mode->hdisplay);
> pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
> bits << PFIT_VERT_SCALE_SHIFT);
> pfit_control |= (PFIT_ENABLE |
> @@ -265,8 +263,8 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> * Full scaling, even if it changes the aspect ratio.
> * Fortunately this is all done for us in hw.
> */
> - if (mode->vdisplay != adjusted_mode->vdisplay ||
> - mode->hdisplay != adjusted_mode->hdisplay) {
> + if (pipe_config->pipe_src_h != adjusted_mode->vdisplay ||
> + pipe_config->pipe_src_w != adjusted_mode->hdisplay) {
> pfit_control |= PFIT_ENABLE;
> if (INTEL_INFO(dev)->gen >= 4)
> pfit_control |= PFIT_SCALING_AUTO;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3ba412c..3823d63 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -450,9 +450,8 @@ void intel_update_fbc(struct drm_device *dev)
> struct drm_framebuffer *fb;
> struct intel_framebuffer *intel_fb;
> struct drm_i915_gem_object *obj;
> - const struct drm_display_mode *mode;
> const struct drm_display_mode *adjusted_mode;
> - unsigned int max_hdisplay, max_vdisplay;
> + unsigned int max_width, max_height;
>
> if (!I915_HAS_FBC(dev)) {
> set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED);
> @@ -496,7 +495,6 @@ void intel_update_fbc(struct drm_device *dev)
> fb = crtc->fb;
> intel_fb = to_intel_framebuffer(fb);
> obj = intel_fb->obj;
> - mode = &intel_crtc->config.requested_mode;
> adjusted_mode = &intel_crtc->config.adjusted_mode;
>
> if (i915_enable_fbc < 0 &&
> @@ -519,14 +517,14 @@ void intel_update_fbc(struct drm_device *dev)
> }
>
> if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
> - max_hdisplay = 4096;
> - max_vdisplay = 2048;
> + max_width = 4096;
> + max_height = 2048;
> } else {
> - max_hdisplay = 2048;
> - max_vdisplay = 1536;
> + max_width = 2048;
> + max_height = 1536;
> }
> - if ((mode->hdisplay > max_hdisplay) ||
> - (mode->vdisplay > max_vdisplay)) {
> + if (intel_crtc->config.pipe_src_w > max_width ||
> + intel_crtc->config.pipe_src_h > max_height) {
> if (set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE))
> DRM_DEBUG_KMS("mode too large for compression, disabling\n");
> goto out_disable;
> @@ -1177,7 +1175,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
> adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> clock = adjusted_mode->clock;
> htotal = adjusted_mode->htotal;
> - hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> + hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
> pixel_size = crtc->fb->bits_per_pixel / 8;
>
> /* Use the small buffer method to calculate plane watermark */
> @@ -1264,7 +1262,7 @@ static bool g4x_compute_srwm(struct drm_device *dev,
> adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> clock = adjusted_mode->clock;
> htotal = adjusted_mode->htotal;
> - hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> + hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
> pixel_size = crtc->fb->bits_per_pixel / 8;
>
> line_time_us = (htotal * 1000) / clock;
> @@ -1492,7 +1490,7 @@ static void i965_update_wm(struct drm_device *dev)
> &to_intel_crtc(crtc)->config.adjusted_mode;
> int clock = adjusted_mode->clock;
> int htotal = adjusted_mode->htotal;
> - int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> + int hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
> int pixel_size = crtc->fb->bits_per_pixel / 8;
> unsigned long line_time_us;
> int entries;
> @@ -1613,7 +1611,7 @@ static void i9xx_update_wm(struct drm_device *dev)
> &to_intel_crtc(enabled)->config.adjusted_mode;
> int clock = adjusted_mode->clock;
> int htotal = adjusted_mode->htotal;
> - int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> + int hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
> int pixel_size = enabled->fb->bits_per_pixel / 8;
> unsigned long line_time_us;
> int entries;
> @@ -1762,7 +1760,7 @@ static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
> adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> clock = adjusted_mode->clock;
> htotal = adjusted_mode->htotal;
> - hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> + hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
> pixel_size = crtc->fb->bits_per_pixel / 8;
>
> line_time_us = (htotal * 1000) / clock;
> @@ -2114,8 +2112,8 @@ static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
> if (pfit_size) {
> uint64_t pipe_w, pipe_h, pfit_w, pfit_h;
>
> - pipe_w = intel_crtc->config.requested_mode.hdisplay;
> - pipe_h = intel_crtc->config.requested_mode.vdisplay;
> + pipe_w = intel_crtc->config.pipe_src_w;
> + pipe_h = intel_crtc->config.pipe_src_h;
> pfit_w = (pfit_size >> 16) & 0xFFFF;
> pfit_h = pfit_size & 0xFFFF;
> if (pipe_w < pfit_w)
> @@ -2640,8 +2638,7 @@ static void hsw_compute_wm_parameters(struct drm_device *dev,
> p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
> p->pri.bytes_per_pixel = crtc->fb->bits_per_pixel / 8;
> p->cur.bytes_per_pixel = 4;
> - p->pri.horiz_pixels =
> - intel_crtc->config.requested_mode.hdisplay;
> + p->pri.horiz_pixels = intel_crtc->config.pipe_src_w;
> p->cur.horiz_pixels = 64;
> /* TODO: for now, assume primary and cursor planes are always enabled. */
> p->pri.enabled = true;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 753cef3..71717e2 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -652,8 +652,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> .y2 = crtc_y + crtc_h,
> };
> const struct drm_rect clip = {
> - .x2 = intel_crtc->config.requested_mode.hdisplay,
> - .y2 = intel_crtc->config.requested_mode.vdisplay,
> + .x2 = intel_crtc->config.pipe_src_w,
> + .y2 = intel_crtc->config.pipe_src_h,
> };
>
> intel_fb = to_intel_framebuffer(fb);
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx