On Thu, Jun 20, 2019 at 11:46:10PM +0200, Maarten Lankhorst wrote:
> Instead of directly referencing drm_crtc_state, convert to
> intel_ctc_state and use the base struct. This is useful when we're
> making the split between uapi and hw state, and also makes the
> code slightly more readable.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 112 ++++++++++++++------------------
> 1 file changed, 50 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 4116de2a77fd..afa069f0dc70 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3857,8 +3857,8 @@ skl_ddb_get_pipe_allocation_limits(struct drm_i915_private *dev_priv,
> struct drm_atomic_state *state = cstate->base.state;
> struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> struct drm_crtc *for_crtc = cstate->base.crtc;
> - const struct drm_crtc_state *crtc_state;
> - const struct drm_crtc *crtc;
> + const struct intel_crtc_state *crtc_state;
> + const struct intel_crtc *crtc;
> u32 pipe_width = 0, total_width = 0, width_before_pipe = 0;
> enum pipe for_pipe = to_intel_crtc(for_crtc)->pipe;
> u16 ddb_size;
> @@ -3901,16 +3901,16 @@ skl_ddb_get_pipe_allocation_limits(struct drm_i915_private *dev_priv,
> * framebuffer, So instead of allocating DDB equally among pipes
> * distribute DDB based on resolution/width of the display.
> */
> - for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> + for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i) {
> const struct drm_display_mode *adjusted_mode;
> int hdisplay, vdisplay;
> enum pipe pipe;
>
> - if (!crtc_state->enable)
> + if (!crtc_state->base.enable)
> continue;
>
> - pipe = to_intel_crtc(crtc)->pipe;
> - adjusted_mode = &crtc_state->adjusted_mode;
> + pipe = crtc->pipe;
> + adjusted_mode = &crtc_state->base.adjusted_mode;
Those two could be done when declaring the variables.
> drm_mode_get_hv_timing(adjusted_mode, &hdisplay, &vdisplay);
> total_width += hdisplay;
>
> @@ -4139,11 +4139,9 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
> struct intel_crtc_state *cstate)
> {
> struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> - struct drm_crtc_state *crtc_state = &cstate->base;
> - struct drm_atomic_state *state = crtc_state->state;
> + struct drm_atomic_state *state = cstate->base.state;
> struct drm_plane *plane;
> - const struct drm_plane_state *pstate;
> - struct intel_plane_state *intel_pstate;
> + const struct drm_plane_state *drm_pstate;
> int crtc_clock, dotclk;
> u32 pipe_max_pixel_rate;
> uint_fixed_16_16_t pipe_downscale;
> @@ -4152,22 +4150,21 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
> if (!cstate->base.enable)
> return 0;
>
> - drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {
> + drm_atomic_crtc_state_for_each_plane_state(plane, drm_pstate, &cstate->base) {
> uint_fixed_16_16_t plane_downscale;
> uint_fixed_16_16_t fp_9_div_8 = div_fixed16(9, 8);
> int bpp;
> + const struct intel_plane_state *pstate =
> + to_intel_plane_state(drm_pstate);
>
> - if (!intel_wm_plane_visible(cstate,
> - to_intel_plane_state(pstate)))
> + if (!intel_wm_plane_visible(cstate, pstate))
> continue;
>
> - if (WARN_ON(!pstate->fb))
> + if (WARN_ON(!pstate->base.fb))
> return -EINVAL;
>
> - intel_pstate = to_intel_plane_state(pstate);
> - plane_downscale = skl_plane_downscale_amount(cstate,
> - intel_pstate);
> - bpp = pstate->fb->format->cpp[0] * 8;
> + plane_downscale = skl_plane_downscale_amount(cstate, pstate);
> + bpp = pstate->base.fb->format->cpp[0] * 8;
> if (bpp == 64)
> plane_downscale = mul_fixed16(plane_downscale,
> fp_9_div_8);
> @@ -4178,7 +4175,7 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
>
> pipe_downscale = mul_fixed16(pipe_downscale, max_downscale);
>
> - crtc_clock = crtc_state->adjusted_mode.crtc_clock;
> + crtc_clock = cstate->base.adjusted_mode.crtc_clock;
> dotclk = to_intel_atomic_state(state)->cdclk.logical.cdclk;
>
> if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
> @@ -4196,11 +4193,10 @@ 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 intel_plane_state *intel_pstate,
> + const struct intel_plane_state *pstate,
> const int plane)
Hmm. Didn't I rename that 'plane' to 'color_plane'? Maybe it was some
other instance, or the patch never got in. Anyways I'd like to see that
done and then we can use 'plane' for the intel_plane.
> {
> - struct intel_plane *intel_plane =
> - to_intel_plane(intel_pstate->base.plane);
> + struct intel_plane *intel_plane = to_intel_plane(pstate->base.plane);
> u32 data_rate;
> u32 width = 0, height = 0;
> struct drm_framebuffer *fb;
> @@ -4208,10 +4204,10 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> uint_fixed_16_16_t down_scale_amount;
> u64 rate;
>
> - if (!intel_pstate->base.visible)
> + if (!pstate->base.visible)
> return 0;
>
> - fb = intel_pstate->base.fb;
> + fb = pstate->base.fb;
> format = fb->format->format;
>
> if (intel_plane->id == PLANE_CURSOR)
> @@ -4224,8 +4220,8 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> * the 90/270 degree plane rotation cases (to match the
> * GTT mapping), hence no need to account for rotation here.
> */
> - width = drm_rect_width(&intel_pstate->base.src) >> 16;
> - height = drm_rect_height(&intel_pstate->base.src) >> 16;
> + width = drm_rect_width(&pstate->base.src) >> 16;
> + height = drm_rect_height(&pstate->base.src) >> 16;
>
> /* UV plane does 1/2 pixel sub-sampling */
> if (plane == 1 && is_planar_yuv_format(format)) {
> @@ -4235,7 +4231,7 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
>
> data_rate = width * height;
>
> - down_scale_amount = skl_plane_downscale_amount(cstate, intel_pstate);
> + down_scale_amount = skl_plane_downscale_amount(cstate, pstate);
>
> rate = mul_round_up_u32_fixed16(data_rate, down_scale_amount);
>
> @@ -4244,35 +4240,32 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> }
>
> static u64
> -skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
> +skl_get_total_relative_data_rate(struct intel_crtc_state *cstate,
> u64 *plane_data_rate,
> u64 *uv_plane_data_rate)
> {
> - struct drm_crtc_state *cstate = &intel_cstate->base;
> - struct drm_atomic_state *state = cstate->state;
> + struct drm_atomic_state *state = cstate->base.state;
s/cstate/crtc_state/ etc. might be nice in a bunch of these functions.
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> struct drm_plane *plane;
> - const struct drm_plane_state *pstate;
> + const struct drm_plane_state *drm_pstate;
> u64 total_data_rate = 0;
>
> if (WARN_ON(!state))
> return 0;
>
> /* Calculate and cache data rate for each plane */
> - drm_atomic_crtc_state_for_each_plane_state(plane, pstate, cstate) {
> + drm_atomic_crtc_state_for_each_plane_state(plane, drm_pstate, &cstate->base) {
> enum plane_id plane_id = to_intel_plane(plane)->id;
> + const struct intel_plane_state *pstate =
> + to_intel_plane_state(drm_pstate);
> u64 rate;
> - const struct intel_plane_state *intel_pstate =
> - to_intel_plane_state(pstate);
>
> /* packed/y */
> - rate = skl_plane_relative_data_rate(intel_cstate,
> - intel_pstate, 0);
> + rate = skl_plane_relative_data_rate(cstate, pstate, 0);
> plane_data_rate[plane_id] = rate;
> total_data_rate += rate;
>
> /* uv-plane */
> - rate = skl_plane_relative_data_rate(intel_cstate,
> - intel_pstate, 1);
> + rate = skl_plane_relative_data_rate(cstate, pstate, 1);
> uv_plane_data_rate[plane_id] = rate;
> total_data_rate += rate;
> }
> @@ -4281,28 +4274,25 @@ skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
> }
>
> static u64
> -icl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
> +icl_get_total_relative_data_rate(struct intel_crtc_state *cstate,
> u64 *plane_data_rate)
> {
> - struct drm_crtc_state *cstate = &intel_cstate->base;
> - struct drm_atomic_state *state = cstate->state;
> struct drm_plane *plane;
> - const struct drm_plane_state *pstate;
> + const struct drm_plane_state *drm_pstate;
> u64 total_data_rate = 0;
>
> - if (WARN_ON(!state))
> + if (WARN_ON(!cstate->base.state))
> return 0;
>
> /* Calculate and cache data rate for each plane */
> - drm_atomic_crtc_state_for_each_plane_state(plane, pstate, cstate) {
> - const struct intel_plane_state *intel_pstate =
> - to_intel_plane_state(pstate);
> + drm_atomic_crtc_state_for_each_plane_state(plane, drm_pstate, &cstate->base) {
> + const struct intel_plane_state *pstate =
> + to_intel_plane_state(drm_pstate);
> enum plane_id plane_id = to_intel_plane(plane)->id;
> u64 rate;
>
> - if (!intel_pstate->linked_plane) {
> - rate = skl_plane_relative_data_rate(intel_cstate,
> - intel_pstate, 0);
> + if (!pstate->linked_plane) {
> + rate = skl_plane_relative_data_rate(cstate, pstate, 0);
> plane_data_rate[plane_id] = rate;
> total_data_rate += rate;
> } else {
> @@ -4315,18 +4305,16 @@ icl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
> * NULL if we try get_new_plane_state(), so we
> * always calculate from the master.
> */
> - if (intel_pstate->slave)
> + if (pstate->slave)
> continue;
>
> /* Y plane rate is calculated on the slave */
> - rate = skl_plane_relative_data_rate(intel_cstate,
> - intel_pstate, 0);
> - y_plane_id = intel_pstate->linked_plane->id;
> + rate = skl_plane_relative_data_rate(cstate, pstate, 0);
> + y_plane_id = pstate->linked_plane->id;
> plane_data_rate[y_plane_id] = rate;
> total_data_rate += rate;
>
> - rate = skl_plane_relative_data_rate(intel_cstate,
> - intel_pstate, 1);
> + rate = skl_plane_relative_data_rate(cstate, pstate, 1);
> plane_data_rate[plane_id] = rate;
> total_data_rate += rate;
> }
> @@ -5095,9 +5083,8 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate)
> {
> struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> struct skl_pipe_wm *pipe_wm = &cstate->wm.skl.optimal;
> - struct drm_crtc_state *crtc_state = &cstate->base;
> struct drm_plane *plane;
> - const struct drm_plane_state *pstate;
> + const struct drm_plane_state *drm_pstate;
> int ret;
>
> /*
> @@ -5106,14 +5093,15 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate)
> */
> memset(pipe_wm->planes, 0, sizeof(pipe_wm->planes));
>
> - drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {
> - const struct intel_plane_state *intel_pstate =
> - to_intel_plane_state(pstate);
> + drm_atomic_crtc_state_for_each_plane_state(plane, drm_pstate,
> + &cstate->base) {
> + const struct intel_plane_state *pstate =
> + to_intel_plane_state(drm_pstate);
>
> if (INTEL_GEN(dev_priv) >= 11)
> - ret = icl_build_plane_wm(cstate, intel_pstate);
> + ret = icl_build_plane_wm(cstate, pstate);
> else
> - ret = skl_build_plane_wm(cstate, intel_pstate);
> + ret = skl_build_plane_wm(cstate, pstate);
> if (ret)
> return ret;
> }
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
