On Mon, Sep 02, 2013 at 09:13:38PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrj...@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.
> 
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

Not sold on this - imo it makes more sense to keep track of this in a new
plane config structure or something similar which ties the fb + metadata
to the crtc. Then we could move a bunch of things we currently have in the
pipe config or someplaces else even (fbc, ips, ...) over there.

So I'm voting for keeping the mess a bit longer until we can do the real
thing ...
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++------
>  drivers/gpu/drm/i915/intel_drv.h     |  3 ++
>  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, 64 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index f49dbe8..17fe7ed 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4692,7 +4692,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
> @@ -4745,7 +4744,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,
> @@ -4783,8 +4783,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,
> @@ -4880,7 +4883,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;
> @@ -4978,8 +4980,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);
> @@ -8255,6 +8257,8 @@ static void intel_dump_pipe_config(struct intel_crtc 
> *crtc,
>       drm_mode_debug_printmodeline(&pipe_config->requested_mode);
>       DRM_DEBUG_KMS("adjusted mode:\n");
>       drm_mode_debug_printmodeline(&pipe_config->adjusted_mode);
> +     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,
> @@ -8306,6 +8310,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;
> @@ -8649,8 +8657,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 e017c30..594d9f4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -213,6 +213,9 @@ struct intel_crtc_config {
>  
>       struct drm_display_mode requested_mode;
>       struct drm_display_mode adjusted_mode;
> +
> +     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 913cb9d..c361f04 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -60,23 +60,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;
> @@ -84,17 +83,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;
> @@ -186,14 +185,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) {
> @@ -202,16 +200,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 */
> @@ -221,12 +219,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
> @@ -236,11 +234,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 |
> @@ -250,11 +248,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 |
> @@ -275,8 +273,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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to