Hey,
Op 15-09-15 om 04:19 schreef Matt Roper:
> Just pull the info out of the state structures rather than staging
> it in an additional set of structures. To make this more
> straightforward, we change the signature of several internal WM
> functions to take the crtc state as a parameter.
>
> v2:
> - Don't forget to skip cursor planes on a loop in the DDB allocation
> function to match original behavior. (Ander)
> - Change a use of intel_crtc->active to cstate->active. They should
> be identical, but it's better to be consistent. (Ander)
> - Rework more function signatures to pass states rather than crtc for
> consistency. (Ander)
>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 326 +++++++++++++++++++---------------------
> 1 file changed, 151 insertions(+), 175 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 7115368..51dbda7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1787,13 +1787,6 @@ static uint32_t ilk_wm_fbc(uint32_t pri_val, uint32_t horiz_pixels,
> return DIV_ROUND_UP(pri_val * 64, horiz_pixels * bytes_per_pixel) + 2;
> }
>
> -struct skl_pipe_wm_parameters {
> - bool active;
> - uint32_t pipe_htotal;
> - uint32_t pixel_rate; /* in KHz */
> - struct intel_plane_wm_parameters plane[I915_MAX_PLANES];
> -};
> -
> struct ilk_wm_maximums {
> uint16_t pri;
> uint16_t spr;
> @@ -2834,18 +2827,40 @@ static bool ilk_disable_lp_wm(struct drm_device *dev)
> #define SKL_DDB_SIZE 896 /* in blocks */
> #define BXT_DDB_SIZE 512
>
> +/*
> + * Return the index of a plane in the SKL DDB and wm result arrays. Primary
> + * plane is always in slot 0, cursor is always in slot I915_MAX_PLANES-1, and
> + * other universal planes are in indices 1..n. Note that this may leave unused
> + * indices between the top "sprite" plane and the cursor.
> + */
> +static int
> +skl_wm_plane_id(const struct intel_plane *plane)
> +{
> + switch (plane->base.type) {
> + case DRM_PLANE_TYPE_PRIMARY:
> + return 0;
> + case DRM_PLANE_TYPE_CURSOR:
> + return PLANE_CURSOR;
> + case DRM_PLANE_TYPE_OVERLAY:
> + return plane->plane;
Shouldn't this be plane + 1 on skylake? Though it's not the only code in i915 that gets it wrong,
assert_sprites_disabled has the same issue on SKL.. O:-)
> + default:
> + MISSING_CASE(plane->base.type);
> + return plane->plane;
> + }
> +}
> +
> static void
> skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
> - struct drm_crtc *for_crtc,
> + const struct intel_crtc_state *cstate,
> const struct intel_wm_config *config,
> - const struct skl_pipe_wm_parameters *params,
> struct skl_ddb_entry *alloc /* out */)
> {
> + struct drm_crtc *for_crtc = cstate->base.crtc;
> struct drm_crtc *crtc;
> unsigned int pipe_size, ddb_size;
> int nth_active_pipe;
>
> - if (!params->active) {
> + if (!cstate->base.active) {
> alloc->start = 0;
> alloc->end = 0;
> return;
> @@ -2911,19 +2926,29 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
> }
>
> static unsigned int
> -skl_plane_relative_data_rate(const struct intel_plane_wm_parameters *p, int y)
> +skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> + const struct drm_plane_state *pstate,
> + int y)
> {
> + struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> + struct drm_framebuffer *fb = pstate->fb;
>
> /* for planar format */
> - if (p->y_bytes_per_pixel) {
> + if (fb->pixel_format == DRM_FORMAT_NV12) {
> if (y) /* y-plane data rate */
> - return p->horiz_pixels * p->vert_pixels * p->y_bytes_per_pixel;
> + return intel_crtc->config->pipe_src_w *
> + intel_crtc->config->pipe_src_h *
> + drm_format_plane_cpp(fb->pixel_format, 0);
> else /* uv-plane data rate */
> - return (p->horiz_pixels/2) * (p->vert_pixels/2) * p->bytes_per_pixel;
> + return (intel_crtc->config->pipe_src_w/2) *
> + (intel_crtc->config->pipe_src_h/2) *
> + drm_format_plane_cpp(fb->pixel_format, 1);
> }
>
> /* for packed formats */
> - return p->horiz_pixels * p->vert_pixels * p->bytes_per_pixel;
> + return intel_crtc->config->pipe_src_w *
> + intel_crtc->config->pipe_src_h *
> + drm_format_plane_cpp(fb->pixel_format, 1);
^Gives a nasty divide by zero error on my skl when total_data_rate ends up being 0..
You want to change last argument to 0 here. ;-)
> }
>
> /*
> @@ -2932,46 +2957,51 @@ skl_plane_relative_data_rate(const struct intel_plane_wm_parameters *p, int y)
> * 3 * 4096 * 8192 * 4 < 2^32
> */
> static unsigned int
> -skl_get_total_relative_data_rate(struct intel_crtc *intel_crtc,
> - const struct skl_pipe_wm_parameters *params)
> +skl_get_total_relative_data_rate(const struct intel_crtc_state *cstate)
> {
> + struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> + struct drm_device *dev = intel_crtc->base.dev;
> + const struct intel_plane *intel_plane;
> unsigned int total_data_rate = 0;
> - int plane;
>
> - for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) {
> - const struct intel_plane_wm_parameters *p;
> + for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> + const struct drm_plane_state *pstate = intel_plane->base.state;
>
> - p = ¶ms->plane[plane];
> - if (!p->enabled)
> + if (WARN_ON(pstate->fb == NULL))
> continue;
^Why is this a WARN_ON?
All in all, the series looks good.
With those fixes feel free to add this to all patches:
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
