Re: [Intel-gfx] [PATCH] drm/i915: Give the sprite width to the WM computation
On Mon, Mar 17, 2014 at 05:58:14PM +, Damien Lespiau wrote:
> In the future, we'll need the height of the fb we're fetching from
> memory for WM computation.
>
> At some point in the future, it'd be nice to give a pointer to a
> mystical plane_config structure instead of chaplet of parameters.
Given that we kinda have such a beast now (but somewhere totally else
unforutnately), should we just start using that one? Or do you think it's
better to add parameters until we're clearer on where plane_config needs
to go to?
Imo the pile of parameters we now have in these functions cross the chasm
into "I can't read this any more without constantly jumping back&forth".
Ville, Paulo, any opinions on how to move forward here with the least
amount of madness?
-Daniel
>
> Signed-off-by: Damien Lespiau
> ---
> drivers/gpu/drm/i915/i915_drv.h | 4 ++--
> drivers/gpu/drm/i915/intel_drv.h| 5 -
> drivers/gpu/drm/i915/intel_pm.c | 17 +++--
> drivers/gpu/drm/i915/intel_sprite.c | 15 +--
> 4 files changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 70fbe90..057873e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -438,8 +438,8 @@ struct drm_i915_display_funcs {
> void (*update_wm)(struct drm_crtc *crtc);
> void (*update_sprite_wm)(struct drm_plane *plane,
>struct drm_crtc *crtc,
> - uint32_t sprite_width, int pixel_size,
> - bool enable, bool scaled);
> + uint32_t sprite_width, uint32_t sprite_height,
> + int pixel_size, bool enable, bool scaled);
> void (*modeset_global_resources)(struct drm_device *dev);
> /* Returns the active state of the crtc, and if the crtc is active,
>* fills out the pipe-config with the hw state. */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index e0064a1..8e6f971 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -392,6 +392,7 @@ struct intel_crtc {
>
> struct intel_plane_wm_parameters {
> uint32_t horiz_pixels;
> + uint32_t vert_pixels;
> uint8_t bytes_per_pixel;
> bool enabled;
> bool scaled;
> @@ -873,7 +874,9 @@ void intel_suspend_hw(struct drm_device *dev);
> void intel_update_watermarks(struct drm_crtc *crtc);
> void intel_update_sprite_watermarks(struct drm_plane *plane,
> struct drm_crtc *crtc,
> - uint32_t sprite_width, int pixel_size,
> + uint32_t sprite_width,
> + uint32_t sprite_height,
> + int pixel_size,
> bool enabled, bool scaled);
> void intel_init_pm(struct drm_device *dev);
> void intel_pm_setup(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ad58ce3..6dbaf66 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2579,10 +2579,11 @@ static void ilk_update_wm(struct drm_crtc *crtc)
> ilk_write_wm_values(dev_priv, &results);
> }
>
> -static void ilk_update_sprite_wm(struct drm_plane *plane,
> - struct drm_crtc *crtc,
> - uint32_t sprite_width, int pixel_size,
> - bool enabled, bool scaled)
> +static void
> +ilk_update_sprite_wm(struct drm_plane *plane,
> + struct drm_crtc *crtc,
> + uint32_t sprite_width, uint32_t sprite_height,
> + int pixel_size, bool enabled, bool scaled)
> {
> struct drm_device *dev = plane->dev;
> struct intel_plane *intel_plane = to_intel_plane(plane);
> @@ -2590,6 +2591,7 @@ static void ilk_update_sprite_wm(struct drm_plane
> *plane,
> intel_plane->wm.enabled = enabled;
> intel_plane->wm.scaled = scaled;
> intel_plane->wm.horiz_pixels = sprite_width;
> + intel_plane->wm.vert_pixels = sprite_width;
> intel_plane->wm.bytes_per_pixel = pixel_size;
>
> /*
> @@ -2720,13 +2722,16 @@ void intel_update_watermarks(struct drm_crtc *crtc)
>
> void intel_update_sprite_watermarks(struct drm_plane *plane,
> struct drm_crtc *crtc,
> - uint32_t sprite_width, int pixel_size,
> + uint32_t sprite_width,
> + uint32_t sprite_height,
> + int pixel_size,
> bool enabled, bool scaled)
> {
> struct drm_i915_private *dev_priv = plane->dev->dev_private;
>
> if (dev_priv->display.update_sprite_wm)
> -
[Intel-gfx] [PATCH] drm/i915: Give the sprite width to the WM computation
In the future, we'll need the height of the fb we're fetching from
memory for WM computation.
At some point in the future, it'd be nice to give a pointer to a
mystical plane_config structure instead of chaplet of parameters.
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
drivers/gpu/drm/i915/intel_drv.h| 5 -
drivers/gpu/drm/i915/intel_pm.c | 17 +++--
drivers/gpu/drm/i915/intel_sprite.c | 15 +--
4 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 70fbe90..057873e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -438,8 +438,8 @@ struct drm_i915_display_funcs {
void (*update_wm)(struct drm_crtc *crtc);
void (*update_sprite_wm)(struct drm_plane *plane,
struct drm_crtc *crtc,
-uint32_t sprite_width, int pixel_size,
-bool enable, bool scaled);
+uint32_t sprite_width, uint32_t sprite_height,
+int pixel_size, bool enable, bool scaled);
void (*modeset_global_resources)(struct drm_device *dev);
/* Returns the active state of the crtc, and if the crtc is active,
* fills out the pipe-config with the hw state. */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e0064a1..8e6f971 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -392,6 +392,7 @@ struct intel_crtc {
struct intel_plane_wm_parameters {
uint32_t horiz_pixels;
+ uint32_t vert_pixels;
uint8_t bytes_per_pixel;
bool enabled;
bool scaled;
@@ -873,7 +874,9 @@ void intel_suspend_hw(struct drm_device *dev);
void intel_update_watermarks(struct drm_crtc *crtc);
void intel_update_sprite_watermarks(struct drm_plane *plane,
struct drm_crtc *crtc,
- uint32_t sprite_width, int pixel_size,
+ uint32_t sprite_width,
+ uint32_t sprite_height,
+ int pixel_size,
bool enabled, bool scaled);
void intel_init_pm(struct drm_device *dev);
void intel_pm_setup(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ad58ce3..6dbaf66 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2579,10 +2579,11 @@ static void ilk_update_wm(struct drm_crtc *crtc)
ilk_write_wm_values(dev_priv, &results);
}
-static void ilk_update_sprite_wm(struct drm_plane *plane,
-struct drm_crtc *crtc,
-uint32_t sprite_width, int pixel_size,
-bool enabled, bool scaled)
+static void
+ilk_update_sprite_wm(struct drm_plane *plane,
+struct drm_crtc *crtc,
+uint32_t sprite_width, uint32_t sprite_height,
+int pixel_size, bool enabled, bool scaled)
{
struct drm_device *dev = plane->dev;
struct intel_plane *intel_plane = to_intel_plane(plane);
@@ -2590,6 +2591,7 @@ static void ilk_update_sprite_wm(struct drm_plane *plane,
intel_plane->wm.enabled = enabled;
intel_plane->wm.scaled = scaled;
intel_plane->wm.horiz_pixels = sprite_width;
+ intel_plane->wm.vert_pixels = sprite_width;
intel_plane->wm.bytes_per_pixel = pixel_size;
/*
@@ -2720,13 +2722,16 @@ void intel_update_watermarks(struct drm_crtc *crtc)
void intel_update_sprite_watermarks(struct drm_plane *plane,
struct drm_crtc *crtc,
- uint32_t sprite_width, int pixel_size,
+ uint32_t sprite_width,
+ uint32_t sprite_height,
+ int pixel_size,
bool enabled, bool scaled)
{
struct drm_i915_private *dev_priv = plane->dev->dev_private;
if (dev_priv->display.update_sprite_wm)
- dev_priv->display.update_sprite_wm(plane, crtc, sprite_width,
+ dev_priv->display.update_sprite_wm(plane, crtc,
+ sprite_width, sprite_height,
pixel_size, enabled, scaled);
}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 336ae6c..844bccd 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -115,7 +115,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
sprctl |= SP_ENABLE;
- intel_upda
