Re: [Intel-gfx] [PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config
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ä
> > > >
> > > > 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ä
> > >
> > > 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
Re: [Intel-gfx] [PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config
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ä > > > > > > 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ä > > > > 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
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ä
> >
> > 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ä
>
> 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;
> > + p
Re: [Intel-gfx] [PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config
On Mon, Sep 02, 2013 at 09:13:38PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> 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ä
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_
[Intel-gfx] [PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config
From: Ville Syrjälä
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ä
---
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
