Op 22-04-16 om 01:20 schreef Matt Roper:
> On Thu, Apr 21, 2016 at 02:19:33PM +0200, Maarten Lankhorst wrote:
>> Op 20-04-16 om 04:26 schreef Matt Roper:
>>> Once we move watermark calculation to the atomic check phase, we'll want
>>> to start rejecting display configurations that exceed out watermark
>>> limits. At the moment we just assume that there's always a valid set of
>>> watermarks, even though this may not actually be true. Let's prepare by
>>> passing return codes up through the call stack in preparation.
>>>
>>> Signed-off-by: Matt Roper <matthew.d.ro...@intel.com>
>>> Reviewed-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
>>> ---
>>> drivers/gpu/drm/i915/intel_display.c | 10 ++--
>>> drivers/gpu/drm/i915/intel_pm.c | 90
>>> ++++++++++++++++++++++--------------
>>> 2 files changed, 61 insertions(+), 39 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>>> b/drivers/gpu/drm/i915/intel_display.c
>>> index 80d8f77..7e8fa88 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -13312,7 +13312,7 @@ static int intel_modeset_checks(struct
>>> drm_atomic_state *state)
>>> * phase. The code here should be run after the per-crtc and per-plane
>>> 'check'
>>> * handlers to ensure that all derived state has been updated.
>>> */
>>> -static void calc_watermark_data(struct drm_atomic_state *state)
>>> +static int calc_watermark_data(struct drm_atomic_state *state)
>>> {
>>> struct drm_device *dev = state->dev;
>>> struct drm_i915_private *dev_priv = to_i915(dev);
>>> @@ -13348,7 +13348,9 @@ static void calc_watermark_data(struct
>>> drm_atomic_state *state)
>>>
>>> /* Is there platform-specific watermark information to calculate? */
>>> if (dev_priv->display.compute_global_watermarks)
>>> - dev_priv->display.compute_global_watermarks(state);
>>> + return dev_priv->display.compute_global_watermarks(state);
>>> +
>>> + return 0;
>>> }
>>>
>>> /**
>>> @@ -13432,9 +13434,7 @@ static int intel_atomic_check(struct drm_device
>>> *dev,
>>> return ret;
>>>
>>> intel_fbc_choose_crtc(dev_priv, state);
>>> - calc_watermark_data(state);
>>> -
>>> - return 0;
>>> + return calc_watermark_data(state);
>>> }
>>>
>>> static int intel_atomic_prepare_commit(struct drm_device *dev,
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c
>>> b/drivers/gpu/drm/i915/intel_pm.c
>>> index 21fde05..f07054a 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -3241,13 +3241,14 @@ static bool skl_ddb_allocation_changed(const struct
>>> skl_ddb_allocation *new_ddb,
>>> return false;
>>> }
>>>
>>> -static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>>> - struct intel_crtc_state *cstate,
>>> - struct intel_plane_state *intel_pstate,
>>> - uint16_t ddb_allocation,
>>> - int level,
>>> - uint16_t *out_blocks, /* out */
>>> - uint8_t *out_lines /* out */)
>>> +static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>>> + struct intel_crtc_state *cstate,
>>> + struct intel_plane_state *intel_pstate,
>>> + uint16_t ddb_allocation,
>>> + int level,
>>> + uint16_t *out_blocks, /* out */
>>> + uint8_t *out_lines, /* out */
>>> + bool *enabled /* out */)
>>> {
>>> struct drm_plane_state *pstate = &intel_pstate->base;
>>> struct drm_framebuffer *fb = pstate->fb;
>>> @@ -3259,8 +3260,10 @@ static bool skl_compute_plane_wm(const struct
>>> drm_i915_private *dev_priv,
>>> uint8_t cpp;
>>> uint32_t width = 0, height = 0;
>>>
>>> - if (latency == 0 || !cstate->base.active || !intel_pstate->visible)
>>> - return false;
>>> + if (latency == 0 || !cstate->base.active || !intel_pstate->visible) {
>>> + *enabled = false;
>>> + return 0;
>>> + }
>>>
>>> width = drm_rect_width(&intel_pstate->src) >> 16;
>>> height = drm_rect_height(&intel_pstate->src) >> 16;
>>> @@ -3321,13 +3324,16 @@ static bool skl_compute_plane_wm(const struct
>>> drm_i915_private *dev_priv,
>>> res_blocks++;
>>> }
>>>
>>> - if (res_blocks >= ddb_allocation || res_lines > 31)
>>> - return false;
>>> + if (res_blocks >= ddb_allocation || res_lines > 31) {
>>> + *enabled = false;
>>> + return 0;
>>> + }
>>>
>>> *out_blocks = res_blocks;
>>> *out_lines = res_lines;
>>> + *enabled = true;
>>>
>>> - return true;
>>> + return 0;
>>> }
>>>
>>> static int
>>> @@ -3345,6 +3351,7 @@ skl_compute_wm_level(const struct drm_i915_private
>>> *dev_priv,
>>> struct intel_plane_state *intel_pstate;
>>> uint16_t ddb_blocks;
>>> enum pipe pipe = intel_crtc->pipe;
>>> + int ret;
>>>
>>> for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
>>> int i = skl_wm_plane_id(intel_plane);
>>> @@ -3374,13 +3381,16 @@ skl_compute_wm_level(const struct drm_i915_private
>>> *dev_priv,
>>>
>>> ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][i]);
>>>
>>> - result->plane_en[i] = skl_compute_plane_wm(dev_priv,
>>> - cstate,
>>> - intel_pstate,
>>> - ddb_blocks,
>>> - level,
>>> - &result->plane_res_b[i],
>>> - &result->plane_res_l[i]);
>>> + ret = skl_compute_plane_wm(dev_priv,
>>> + cstate,
>>> + intel_pstate,
>>> + ddb_blocks,
>>> + level,
>>> + &result->plane_res_b[i],
>>> + &result->plane_res_l[i],
>>> + &result->plane_en[i]);
>>> + if (ret)
>>> + return ret;
>>> }
>>>
>>> return 0;
>>> @@ -3417,21 +3427,26 @@ static void skl_compute_transition_wm(struct
>>> intel_crtc_state *cstate,
>>> }
>>> }
>>>
>>> -static void skl_build_pipe_wm(struct intel_crtc_state *cstate,
>>> - struct skl_ddb_allocation *ddb,
>>> - struct skl_pipe_wm *pipe_wm)
>>> +static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
>>> + struct skl_ddb_allocation *ddb,
>>> + struct skl_pipe_wm *pipe_wm)
>>> {
>>> struct drm_device *dev = cstate->base.crtc->dev;
>>> const struct drm_i915_private *dev_priv = dev->dev_private;
>>> int level, max_level = ilk_wm_max_level(dev);
>>> + int ret;
>>>
>>> for (level = 0; level <= max_level; level++) {
>>> - skl_compute_wm_level(dev_priv, ddb, cstate,
>>> - level, &pipe_wm->wm[level]);
>>> + ret = skl_compute_wm_level(dev_priv, ddb, cstate,
>>> + level, &pipe_wm->wm[level]);
>>> + if (ret)
>>> + return ret;
>>> }
>>> pipe_wm->linetime = skl_compute_linetime_wm(cstate);
>>>
>>> skl_compute_transition_wm(cstate, &pipe_wm->trans_wm);
>>> +
>>> + return 0;
>>> }
>>>
>>> static void skl_compute_wm_results(struct drm_device *dev,
>>> @@ -3678,21 +3693,27 @@ static void skl_flush_wm_values(struct
>>> drm_i915_private *dev_priv,
>>> }
>>> }
>>>
>>> -static bool skl_update_pipe_wm(struct drm_crtc_state *cstate,
>>> - struct skl_ddb_allocation *ddb, /* out */
>>> - struct skl_pipe_wm *pipe_wm /* out */)
>>> +static int skl_update_pipe_wm(struct drm_crtc_state *cstate,
>>> + struct skl_ddb_allocation *ddb, /* out */
>>> + struct skl_pipe_wm *pipe_wm, /* out */
>>> + bool *changed /* out */)
>>> {
>>> struct intel_crtc *intel_crtc = to_intel_crtc(cstate->crtc);
>>> struct intel_crtc_state *intel_cstate = to_intel_crtc_state(cstate);
>>> + int ret;
>>>
>>> - skl_build_pipe_wm(intel_cstate, ddb, pipe_wm);
>>> + ret = skl_build_pipe_wm(intel_cstate, ddb, pipe_wm);
>>> + if (ret)
>>> + return ret;
>>>
>>> if (!memcmp(&intel_crtc->wm.active.skl, pipe_wm, sizeof(*pipe_wm)))
>>> - return false;
>>> + *changed = false;
>>> + else
>>> + *changed = true;
>>>
>>> intel_crtc->wm.active.skl = *pipe_wm;
>>>
>>> - return true;
>>> + return 0;
>>> }
>>>
>>> static void skl_update_other_pipe_wm(struct drm_device *dev,
>>> @@ -3725,8 +3746,8 @@ static void skl_update_other_pipe_wm(struct
>>> drm_device *dev,
>>> if (!intel_crtc->active)
>>> continue;
>>>
>>> - wm_changed = skl_update_pipe_wm(intel_crtc->base.state,
>>> - &r->ddb, &pipe_wm);
>>> + skl_update_pipe_wm(intel_crtc->base.state,
>>> + &r->ddb, &pipe_wm, &wm_changed);
>>>
>>> /*
>>> * If we end up re-computing the other pipe WM values, it's
>>> @@ -3827,14 +3848,15 @@ static void skl_update_wm(struct drm_crtc *crtc)
>>> struct skl_wm_values *results = &dev_priv->wm.skl_results;
>>> struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
>>> struct skl_pipe_wm *pipe_wm = &cstate->wm.skl.optimal;
>>> -
>>> + bool wm_changed;
>>>
>>> /* Clear all dirty flags */
>>> results->dirty_pipes = 0;
>>>
>>> skl_clear_wm(results, intel_crtc->pipe);
>>>
>>> - if (!skl_update_pipe_wm(crtc->state, &results->ddb, pipe_wm))
>>> + skl_update_pipe_wm(crtc->state, &results->ddb, pipe_wm, &wm_changed);
>>> + if (!wm_changed)
>>> return;
>>>
>>> skl_compute_wm_results(dev, pipe_wm, results, intel_crtc);
>> With the nitpicks fixed, and with CI happy.
>>
>> Reviewed-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> I'm interpreting this to refer to the entire series (and not just this
> one patch that already had your r-b and no nitpicks); if that's not what
> you meant, please let me know. :-)
>
Oops indeed. That's the case. :-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
