Re: [Intel-gfx] [PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config

2013-09-03 Thread Daniel Vetter
On Mon, Sep 02, 2013 at 10:11:43PM +0300, Ville Syrjälä wrote:
 On Mon, Sep 02, 2013 at 08:46:19PM +0200, Daniel Vetter wrote:
  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.
 
 This size isn't the plane size. It's the pipe size. Two different things.

Oops, I guess I'm confused about this. In that case this makes tons of
sense. Now just one of our residential watermarks experts (definitely not
me) needs to double-check that we're always using the right thing ...
-Daniel
-- 
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


Re: [Intel-gfx] [PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config

2013-09-03 Thread Ville Syrjälä
On Tue, Sep 03, 2013 at 09:52:40AM +0200, Daniel Vetter wrote:
 On Mon, Sep 02, 2013 at 10:11:43PM +0300, Ville Syrjälä wrote:
  On Mon, Sep 02, 2013 at 08:46:19PM +0200, Daniel Vetter wrote:
   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.
  
  This size isn't the plane size. It's the pipe size. Two different things.
 
 Oops, I guess I'm confused about this. In that case this makes tons of
 sense. Now just one of our residential watermarks experts (definitely not
 me) needs to double-check that we're always using the right thing ...

Actually we're now using pipe_src_{w,h} always. I was considering adding
primary_{w,h} to pipe config already, but then decided that I could wait
until we split the primary from the crtc.

The reason I need to use pipe_src_{w,h} is the pipe_src_w roudning for
double wide  co. We need to clip the primary to the pipe dimensions, so
the same value works as long as primary planes are fullscreen only.

But if you'd prefer I could still add the explicit primary dimensions
to pipe config.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config

2013-09-02 Thread ville . syrjala
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
---
 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  0x) + 1;
-   pipe_config-requested_mode.hdisplay = ((tmp  16)  0x) + 1;
+   pipe_config-pipe_src_h = (tmp  0x) + 1;
+   pipe_config-pipe_src_w = ((tmp  16)  0x) + 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
@@ 

Re: [Intel-gfx] [PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config

2013-09-02 Thread Daniel Vetter
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  0x) + 1;
 - pipe_config-requested_mode.hdisplay = ((tmp  16)  0x) + 1;
 + pipe_config-pipe_src_h = (tmp  0x) + 1;
 + pipe_config-pipe_src_w = ((tmp  16)  0x) + 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);
   }
  
 - 

Re: [Intel-gfx] [PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config

2013-09-02 Thread Ville Syrjälä
On Mon, Sep 02, 2013 at 08:46:19PM +0200, Daniel Vetter wrote:
 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.

This size isn't the plane size. It's the pipe size. Two different things.

 
 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  0x) + 1;
  -   pipe_config-requested_mode.hdisplay = ((tmp  16)  0x) + 1;
  +   pipe_config-pipe_src_h = (tmp  0x) + 1;
  +   pipe_config-pipe_src_w = ((tmp  16)  0x) + 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 @@