Re: [Intel-gfx] [PATCH v2 4/8] drm/i915/gen11: Handle watermarks correctly for separate Y/UV planes, v2.
On Thu, Oct 18, 2018 at 01:51:30PM +0200, Maarten Lankhorst wrote:
> Skylake style watermarks program the UV parameters into wm->uv_wm,
> and have a separate DDB allocation for UV blocks into the same plane.
>
> Gen11 watermarks have a separate plane for Y and UV, with separate
> mechanisms. The simplest way to make it work is to keep the current
> way of programming watermarks and calculate the Y and UV plane
> watermarks from the master plane.
>
> Changes since v1:
> - Constify crtc_state where possible.
> - Make separate paths for planar formats in skl_build_pipe_wm() (Matt)
> - Make separate paths for calculating total data rate. (Matt)
> - Make sure UV watermarks are unused on gen11+ by adding a WARN. (Matt)
This lookd like it should work.
Reviewed-by: Ville Syrjälä
However it's going to conflict pretty badly with Paulo's watermark
series. One of you is going to have to rebase at least, and not sure if
all of Paulo's stuff even makes sense given this patch...
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/intel_pm.c | 301 ++--
> 1 file changed, 207 insertions(+), 94 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d003c08bd9e4..ef95b9d7884b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3814,7 +3814,7 @@ static u16 intel_get_ddb_size(struct drm_i915_private
> *dev_priv,
> }
>
> static void
> -skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
> +skl_ddb_get_pipe_allocation_limits(struct drm_i915_private *dev_priv,
> const struct intel_crtc_state *cstate,
> const u64 total_data_rate,
> struct skl_ddb_allocation *ddb,
> @@ -3823,7 +3823,6 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device
> *dev,
> {
> struct drm_atomic_state *state = cstate->base.state;
> struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> - struct drm_i915_private *dev_priv = to_i915(dev);
> struct drm_crtc *for_crtc = cstate->base.crtc;
> const struct drm_crtc_state *crtc_state;
> const struct drm_crtc *crtc;
> @@ -3945,14 +3944,9 @@ skl_ddb_get_hw_plane_state(struct drm_i915_private
> *dev_priv,
> val & PLANE_CTL_ALPHA_MASK);
>
> val = I915_READ(PLANE_BUF_CFG(pipe, plane_id));
> - /*
> - * FIXME: add proper NV12 support for ICL. Avoid reading unclaimed
> - * registers for now.
> - */
> - if (INTEL_GEN(dev_priv) < 11)
> + if (fourcc == DRM_FORMAT_NV12 && INTEL_GEN(dev_priv) < 11) {
> val2 = I915_READ(PLANE_NV12_BUF_CFG(pipe, plane_id));
>
> - if (fourcc == DRM_FORMAT_NV12) {
> skl_ddb_entry_init_from_hw(dev_priv,
> >plane[pipe][plane_id], val2);
> skl_ddb_entry_init_from_hw(dev_priv,
> @@ -4141,11 +4135,11 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc
> *intel_crtc,
>
> static u64
> skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> - const struct drm_plane_state *pstate,
> + const struct intel_plane_state *intel_pstate,
>const int plane)
> {
> - struct intel_plane *intel_plane = to_intel_plane(pstate->plane);
> - struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
> + struct intel_plane *intel_plane =
> + to_intel_plane(intel_pstate->base.plane);
> uint32_t data_rate;
> uint32_t width = 0, height = 0;
> struct drm_framebuffer *fb;
> @@ -4156,7 +4150,7 @@ skl_plane_relative_data_rate(const struct
> intel_crtc_state *cstate,
> if (!intel_pstate->base.visible)
> return 0;
>
> - fb = pstate->fb;
> + fb = intel_pstate->base.fb;
> format = fb->format->format;
>
> if (intel_plane->id == PLANE_CURSOR)
> @@ -4206,25 +4200,80 @@ skl_get_total_relative_data_rate(struct
> intel_crtc_state *intel_cstate,
> drm_atomic_crtc_state_for_each_plane_state(plane, pstate, cstate) {
> enum plane_id plane_id = to_intel_plane(plane)->id;
> u64 rate;
> + const struct intel_plane_state *intel_pstate =
> + to_intel_plane_state(pstate);
>
> /* packed/y */
> rate = skl_plane_relative_data_rate(intel_cstate,
> - pstate, 0);
> + intel_pstate, 0);
> plane_data_rate[plane_id] = rate;
> -
> total_data_rate += rate;
>
> /* uv-plane */
> rate = skl_plane_relative_data_rate(intel_cstate,
> - pstate, 1);
> + intel_pstate, 1);
>
[Intel-gfx] [PATCH v2 4/8] drm/i915/gen11: Handle watermarks correctly for separate Y/UV planes, v2.
Skylake style watermarks program the UV parameters into wm->uv_wm,
and have a separate DDB allocation for UV blocks into the same plane.
Gen11 watermarks have a separate plane for Y and UV, with separate
mechanisms. The simplest way to make it work is to keep the current
way of programming watermarks and calculate the Y and UV plane
watermarks from the master plane.
Changes since v1:
- Constify crtc_state where possible.
- Make separate paths for planar formats in skl_build_pipe_wm() (Matt)
- Make separate paths for calculating total data rate. (Matt)
- Make sure UV watermarks are unused on gen11+ by adding a WARN. (Matt)
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/intel_pm.c | 301 ++--
1 file changed, 207 insertions(+), 94 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d003c08bd9e4..ef95b9d7884b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3814,7 +3814,7 @@ static u16 intel_get_ddb_size(struct drm_i915_private
*dev_priv,
}
static void
-skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
+skl_ddb_get_pipe_allocation_limits(struct drm_i915_private *dev_priv,
const struct intel_crtc_state *cstate,
const u64 total_data_rate,
struct skl_ddb_allocation *ddb,
@@ -3823,7 +3823,6 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
{
struct drm_atomic_state *state = cstate->base.state;
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
- struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_crtc *for_crtc = cstate->base.crtc;
const struct drm_crtc_state *crtc_state;
const struct drm_crtc *crtc;
@@ -3945,14 +3944,9 @@ skl_ddb_get_hw_plane_state(struct drm_i915_private
*dev_priv,
val & PLANE_CTL_ALPHA_MASK);
val = I915_READ(PLANE_BUF_CFG(pipe, plane_id));
- /*
-* FIXME: add proper NV12 support for ICL. Avoid reading unclaimed
-* registers for now.
-*/
- if (INTEL_GEN(dev_priv) < 11)
+ if (fourcc == DRM_FORMAT_NV12 && INTEL_GEN(dev_priv) < 11) {
val2 = I915_READ(PLANE_NV12_BUF_CFG(pipe, plane_id));
- if (fourcc == DRM_FORMAT_NV12) {
skl_ddb_entry_init_from_hw(dev_priv,
>plane[pipe][plane_id], val2);
skl_ddb_entry_init_from_hw(dev_priv,
@@ -4141,11 +4135,11 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc
*intel_crtc,
static u64
skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
-const struct drm_plane_state *pstate,
+const struct intel_plane_state *intel_pstate,
const int plane)
{
- struct intel_plane *intel_plane = to_intel_plane(pstate->plane);
- struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
+ struct intel_plane *intel_plane =
+ to_intel_plane(intel_pstate->base.plane);
uint32_t data_rate;
uint32_t width = 0, height = 0;
struct drm_framebuffer *fb;
@@ -4156,7 +4150,7 @@ skl_plane_relative_data_rate(const struct
intel_crtc_state *cstate,
if (!intel_pstate->base.visible)
return 0;
- fb = pstate->fb;
+ fb = intel_pstate->base.fb;
format = fb->format->format;
if (intel_plane->id == PLANE_CURSOR)
@@ -4206,25 +4200,80 @@ skl_get_total_relative_data_rate(struct
intel_crtc_state *intel_cstate,
drm_atomic_crtc_state_for_each_plane_state(plane, pstate, cstate) {
enum plane_id plane_id = to_intel_plane(plane)->id;
u64 rate;
+ const struct intel_plane_state *intel_pstate =
+ to_intel_plane_state(pstate);
/* packed/y */
rate = skl_plane_relative_data_rate(intel_cstate,
- pstate, 0);
+ intel_pstate, 0);
plane_data_rate[plane_id] = rate;
-
total_data_rate += rate;
/* uv-plane */
rate = skl_plane_relative_data_rate(intel_cstate,
- pstate, 1);
+ intel_pstate, 1);
uv_plane_data_rate[plane_id] = rate;
-
total_data_rate += rate;
}
return total_data_rate;
}
+static u64
+icl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
+u64 *plane_data_rate)
+{
+ struct drm_crtc_state *cstate = _cstate->base;
+ struct drm_atomic_state *state = cstate->state;
+ struct drm_plane *plane;
+
