[Intel-gfx] [PATCH] Revert "drm/i915/skl: New ddb allocation algorithm"

Jani Nikula jani.nikula at linux.intel.com
Thu Jun 15 20:13:01 UTC 2017


On Thu, 15 Jun 2017, Ander Conselvan De Oliveira <conselvan2 at gmail.com> wrote:
> On Thu, 2017-06-15 at 09:44 +0530, Mahesh Kumar wrote:
>> Hi Ander,
>> 
>> 
>> On Wednesday 14 June 2017 05:17 PM, Ander Conselvan De Oliveira wrote:
>> > On Tue, 2017-06-13 at 12:06 -0700, Rodrigo Vivi wrote:
>> > > On Tue, Jun 13, 2017 at 11:07 AM, Matt Roper <matthew.d.roper at intel.com> wrote:
>> > > > On Tue, Jun 13, 2017 at 10:52:30AM -0700, Rodrigo Vivi wrote:
>> > > > > This reverts commit bb9d85f6e9de8fef5236c076530eab67a2f2431b.
>> > > > > 
>> > > > > New ddb allocation algorithm is a show stopper on my SKL system.
>> > > > > 
>> > > > > Besides not be able to get external DP 4k at 60 (through USB type C),
>> > > > > It fully hang my screen when unplugging the USB type C.
>> > > > > 
>> > > > > Cc: Mahesh Kumar <mahesh1.kumar at intel.com>
>> > > > > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> > > > > Cc: Matt Roper <matthew.d.roper at intel.com>
>> > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> > > > 
>> > > > Is there a bugzilla entry with details/logs from the failure we can
>> > > > refer to?
>> > > 
>> > > nope, since I know it was working few weeks ago and then stopped
>> > > working I went directly
>> > > to bisect so I could get my system usable quickly again.
>> > > 
>> > > But yes, I need to open a proper bugzilla and provide logs.
>> > > Should we revert first and grab logs latter or we are going to try to
>> > > analyse logs and fix?
>> > > 
>> > > Anyways this is a big show stopper so I believe we revert at least
>> > > before next pull request.
>> > 
>> > This patch also causes artifacts in dual screen setups. All the scanlines below
>> > the mouse cursor are wrong, sometimes containing a shade of gray and others
>> > repeating the image above. I've seen this in APL and GLK using DSI+HDMI,
>> > eDP+HDMI and DSI+DP. In the DP case, however, the screen goes blank and reports
>> > lack of signal.
>> > 
>> > I've noticed fifo underrun messages. I compared the ddb allocation in GLK with a
>> > working and non-working kernel, but they were the same.
>> 
>> your observation looks like one we observed in one of the Linux variant 
>> program.
>> There was an configuration where lower WM-level having higher DDB 
>> requirement than higher levels. Which was leading to lower WM levels 
>> getting disabled.
>> 
>> Can you please try with following patch to see if it's solving the problem.
>> 
>> https://patchwork.freedesktop.org/patch/161571/
>
> I tested that in my GLK, but it didn't help. Still getting artifacts following
> the mouse cursor.

If the revert helps the GLK case as well, please let's revert. We
generally waste too much time trying to get things fixed when the revert
and return to the drawing board is the way to go.

Rodrigo, please add the Bugzilla: reference and proper Fixes: line.

BR,
Jani.


>
> Ander
>
>> Rodrigo,
>> 
>> Can you please also check if this is the same reason for your failure. 
>> I'm arranging for a system with SKL+USB type-C DP 4K at 60 & eDP. will 
>> debug this further.
>> 
>> -Mahesh
>> 
>> > 
>> > Ander
>> > > When you say you can't get DP 4k at 60, are you seeing messages
>> > > > in dmesg about the setup being outside system watermark limitations or
>> > > > are you just getting a black screen?
>> > > 
>> > > just getting a blank screen on DP when plugging usb-c and a locked-up
>> > > screen on eDP when removing usb-c.
>> > > 
>> > > > 
>> > > > Matt
>> > > > 
>> > > > > ---
>> > > > >   drivers/gpu/drm/i915/intel_pm.c | 257 +++++++++++++++-------------------------
>> > > > >   1 file changed, 98 insertions(+), 159 deletions(-)
>> > > > > 
>> > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> > > > > index aa9d8ce..dc4275d 100644
>> > > > > --- a/drivers/gpu/drm/i915/intel_pm.c
>> > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c
>> > > > > @@ -4140,41 +4140,13 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
>> > > > >        minimum[PLANE_CURSOR] = skl_cursor_allocation(num_active);
>> > > > >   }
>> > > > > 
>> > > > > -static void
>> > > > > -skl_enable_plane_wm_levels(const struct drm_i915_private *dev_priv,
>> > > > > -                        uint16_t plane_ddb,
>> > > > > -                        uint16_t max_level,
>> > > > > -                        struct skl_plane_wm *wm)
>> > > > > -{
>> > > > > -     int level;
>> > > > > -     /*
>> > > > > -      * Now enable all levels in WM structure which can be enabled
>> > > > > -      * using current DDB allocation
>> > > > > -      */
>> > > > > -     for (level = ilk_wm_max_level(dev_priv); level >= 0; level--) {
>> > > > > -             struct skl_wm_level *level_wm = &wm->wm[level];
>> > > > > -
>> > > > > -             if (level > max_level || level_wm->plane_res_b == 0
>> > > > > -                                   || level_wm->plane_res_l >= 31
>> > > > > -                                   || level_wm->plane_res_b >= plane_ddb) {
>> > > > > -                     level_wm->plane_en = false;
>> > > > > -                     level_wm->plane_res_b = 0;
>> > > > > -                     level_wm->plane_res_l = 0;
>> > > > > -             } else {
>> > > > > -                     level_wm->plane_en = true;
>> > > > > -             }
>> > > > > -     }
>> > > > > -}
>> > > > > -
>> > > > >   static int
>> > > > >   skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>> > > > > -                   struct skl_pipe_wm *pipe_wm,
>> > > > >                      struct skl_ddb_allocation *ddb /* out */)
>> > > > >   {
>> > > > >        struct drm_atomic_state *state = cstate->base.state;
>> > > > >        struct drm_crtc *crtc = cstate->base.crtc;
>> > > > >        struct drm_device *dev = crtc->dev;
>> > > > > -     struct drm_i915_private *dev_priv = to_i915(dev);
>> > > > >        struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> > > > >        enum pipe pipe = intel_crtc->pipe;
>> > > > >        struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
>> > > > > @@ -4187,9 +4159,6 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
>> > > > >        unsigned plane_data_rate[I915_MAX_PLANES] = {};
>> > > > >        unsigned plane_y_data_rate[I915_MAX_PLANES] = {};
>> > > > >        uint16_t total_min_blocks = 0;
>> > > > > -     uint16_t total_level_ddb;
>> > > > > -     uint16_t plane_blocks = 0;
>> > > > > -     int max_level, level;
>> > > > > 
>> > > > >        /* Clear the partitioning for disabled planes. */
>> > > > >        memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
>> > > > > @@ -4228,48 +4197,10 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
>> > > > >                return -EINVAL;
>> > > > >        }
>> > > > > 
>> > > > > -     alloc_size -= minimum[PLANE_CURSOR];
>> > > > > -     ddb->plane[pipe][PLANE_CURSOR].start = alloc->end -
>> > > > > -                                                     minimum[PLANE_CURSOR];
>> > > > > +     alloc_size -= total_min_blocks;
>> > > > > +     ddb->plane[pipe][PLANE_CURSOR].start = alloc->end - minimum[PLANE_CURSOR];
>> > > > >        ddb->plane[pipe][PLANE_CURSOR].end = alloc->end;
>> > > > > 
>> > > > > -     for (level = ilk_wm_max_level(dev_priv); level >= 0; level--) {
>> > > > > -             total_level_ddb = 0;
>> > > > > -             for_each_plane_id_on_crtc(intel_crtc, plane_id) {
>> > > > > -                     /*
>> > > > > -                      * TODO: We should calculate watermark values for Y/UV
>> > > > > -                      * plane both in case of NV12 format and use both values
>> > > > > -                      * for ddb calculation. NV12 is disabled as of now, So
>> > > > > -                      * using only single/UV plane value here.
>> > > > > -                      */
>> > > > > -                     struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
>> > > > > -                     uint16_t plane_res_b = wm->wm[level].plane_res_b;
>> > > > > -                     uint16_t min = minimum[plane_id] + y_minimum[plane_id];
>> > > > > -
>> > > > > -                     if (plane_id == PLANE_CURSOR)
>> > > > > -                             continue;
>> > > > > -
>> > > > > -                     total_level_ddb += max(plane_res_b, min);
>> > > > > -             }
>> > > > > -
>> > > > > -             /*
>> > > > > -              * If This level can successfully be enabled with the
>> > > > > -              * pipe's current DDB allocation, then all lower levels are
>> > > > > -              * guaranteed to succeed as well.
>> > > > > -              */
>> > > > > -             if (total_level_ddb <= alloc_size)
>> > > > > -                     break;
>> > > > > -     }
>> > > > > -
>> > > > > -     if ((level < 0) || (total_min_blocks > alloc_size)) {
>> > > > > -             DRM_DEBUG_KMS("Requested display configuration exceeds system DDB limitations");
>> > > > > -             DRM_DEBUG_KMS("minimum required %d/%d\n", (level < 0) ?
>> > > > > -                             total_level_ddb : total_min_blocks, alloc_size);
>> > > > > -             return -EINVAL;
>> > > > > -     }
>> > > > > -     max_level = level;
>> > > > > -     alloc_size -= total_level_ddb;
>> > > > > -
>> > > > >        /*
>> > > > >         * 2. Distribute the remaining space in proportion to the amount of
>> > > > >         * data each plane needs to fetch from memory.
>> > > > > @@ -4279,24 +4210,13 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
>> > > > >        total_data_rate = skl_get_total_relative_data_rate(cstate,
>> > > > >                                                           plane_data_rate,
>> > > > >                                                           plane_y_data_rate);
>> > > > > -     /*
>> > > > > -      * PLANE_CURSOR data rate is not included in total_data_rate.
>> > > > > -      * If only cursor plane is enabled we have to enable its WM levels
>> > > > > -      * explicitly before returning. Cursor has fixed ddb allocation,
>> > > > > -      * So it's ok to always check cursor WM enabling before return.
>> > > > > -      */
>> > > > > -     plane_blocks = skl_ddb_entry_size(&ddb->plane[pipe][PLANE_CURSOR]);
>> > > > > -     skl_enable_plane_wm_levels(dev_priv, plane_blocks, max_level,
>> > > > > -                                &pipe_wm->planes[PLANE_CURSOR]);
>> > > > >        if (total_data_rate == 0)
>> > > > >                return 0;
>> > > > > 
>> > > > >        start = alloc->start;
>> > > > >        for_each_plane_id_on_crtc(intel_crtc, plane_id) {
>> > > > >                unsigned int data_rate, y_data_rate;
>> > > > > -             uint16_t plane_blocks = 0, y_plane_blocks = 0;
>> > > > > -             struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
>> > > > > -             uint16_t plane_res_b = wm->wm[max_level].plane_res_b;
>> > > > > +             uint16_t plane_blocks, y_plane_blocks = 0;
>> > > > > 
>> > > > >                if (plane_id == PLANE_CURSOR)
>> > > > >                        continue;
>> > > > > @@ -4308,36 +4228,33 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
>> > > > >                 * promote the expression to 64 bits to avoid overflowing, the
>> > > > >                 * result is < available as data_rate / total_data_rate < 1
>> > > > >                 */
>> > > > > +             plane_blocks = minimum[plane_id];
>> > > > > +             plane_blocks += div_u64((uint64_t)alloc_size * data_rate,
>> > > > > +                                     total_data_rate);
>> > > > > 
>> > > > >                /* Leave disabled planes at (0,0) */
>> > > > >                if (data_rate) {
>> > > > > -                     plane_blocks = max(minimum[plane_id], plane_res_b);
>> > > > > -                     plane_blocks += div_u64((uint64_t)alloc_size *
>> > > > > -                                     data_rate, total_data_rate);
>> > > > >                        ddb->plane[pipe][plane_id].start = start;
>> > > > >                        ddb->plane[pipe][plane_id].end = start + plane_blocks;
>> > > > > -                     start += plane_blocks;
>> > > > >                }
>> > > > > 
>> > > > > +             start += plane_blocks;
>> > > > > +
>> > > > >                /*
>> > > > >                 * allocation for y_plane part of planar format:
>> > > > > -              * TODO: Once we start calculating watermark values for Y/UV
>> > > > > -              * plane both consider it for initial allowed wm blocks.
>> > > > >                 */
>> > > > >                y_data_rate = plane_y_data_rate[plane_id];
>> > > > > 
>> > > > > +             y_plane_blocks = y_minimum[plane_id];
>> > > > > +             y_plane_blocks += div_u64((uint64_t)alloc_size * y_data_rate,
>> > > > > +                                     total_data_rate);
>> > > > > +
>> > > > >                if (y_data_rate) {
>> > > > > -                     y_plane_blocks = y_minimum[plane_id];
>> > > > > -                     y_plane_blocks += div_u64((uint64_t)alloc_size *
>> > > > > -                                     y_data_rate, total_data_rate);
>> > > > >                        ddb->y_plane[pipe][plane_id].start = start;
>> > > > >                        ddb->y_plane[pipe][plane_id].end = start + y_plane_blocks;
>> > > > > -                     start += y_plane_blocks;
>> > > > >                }
>> > > > > -             skl_enable_plane_wm_levels(dev_priv,
>> > > > > -                                        plane_blocks,
>> > > > > -                                        max_level,
>> > > > > -                                        wm);
>> > > > > +
>> > > > > +             start += y_plane_blocks;
>> > > > >        }
>> > > > > 
>> > > > >        return 0;
>> > > > > @@ -4427,9 +4344,11 @@ static uint_fixed_16_16_t skl_wm_method2(uint32_t pixel_rate,
>> > > > >   static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>> > > > >                                struct intel_crtc_state *cstate,
>> > > > >                                const struct intel_plane_state *intel_pstate,
>> > > > > +                             uint16_t ddb_allocation,
>> > > > >                                int level,
>> > > > >                                uint16_t *out_blocks, /* out */
>> > > > > -                             uint8_t *out_lines /* out */)
>> > > > > +                             uint8_t *out_lines, /* out */
>> > > > > +                             bool *enabled /* out */)
>> > > > >   {
>> > > > >        struct intel_plane *plane = to_intel_plane(intel_pstate->base.plane);
>> > > > >        const struct drm_plane_state *pstate = &intel_pstate->base;
>> > > > > @@ -4452,8 +4371,10 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>> > > > >        bool y_tiled, x_tiled;
>> > > > > 
>> > > > >        if (latency == 0 ||
>> > > > > -         !intel_wm_plane_visible(cstate, intel_pstate))
>> > > > > +         !intel_wm_plane_visible(cstate, intel_pstate)) {
>> > > > > +             *enabled = false;
>> > > > >                return 0;
>> > > > > +     }
>> > > > > 
>> > > > >        y_tiled = fb->modifier == I915_FORMAT_MOD_Y_TILED ||
>> > > > >                  fb->modifier == I915_FORMAT_MOD_Yf_TILED;
>> > > > > @@ -4541,6 +4462,9 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>> > > > >                if ((cpp * cstate->base.adjusted_mode.crtc_htotal / 512 < 1) &&
>> > > > >                    (plane_bytes_per_line / 512 < 1))
>> > > > >                        selected_result = method2;
>> > > > > +             else if ((ddb_allocation && ddb_allocation /
>> > > > > +                     fixed_16_16_to_u32_round_up(plane_blocks_per_line)) >= 1)
>> > > > > +                     selected_result = min_fixed_16_16(method1, method2);
>> > > > >                else if (latency >= linetime_us)
>> > > > >                        selected_result = min_fixed_16_16(method1, method2);
>> > > > >                else
>> > > > > @@ -4560,42 +4484,64 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>> > > > >                }
>> > > > >        }
>> > > > > 
>> > > > > -     if (res_lines >= 31 && level == 0) {
>> > > > > -             struct drm_plane *plane = pstate->plane;
>> > > > > +     if (res_blocks >= ddb_allocation || res_lines > 31) {
>> > > > > +             *enabled = false;
>> > > > > 
>> > > > > -             DRM_DEBUG_KMS("Requested display configuration exceeds system watermark limitations\n");
>> > > > > -             DRM_DEBUG_KMS("[PLANE:%d:%s] lines required = %u/31\n",
>> > > > > -                             plane->base.id, plane->name, res_lines);
>> > > > > -             return -EINVAL;
>> > > > > +             /*
>> > > > > +              * If there are no valid level 0 watermarks, then we can't
>> > > > > +              * support this display configuration.
>> > > > > +              */
>> > > > > +             if (level) {
>> > > > > +                     return 0;
>> > > > > +             } else {
>> > > > > +                     struct drm_plane *plane = pstate->plane;
>> > > > > +
>> > > > > +                     DRM_DEBUG_KMS("Requested display configuration exceeds system watermark limitations\n");
>> > > > > +                     DRM_DEBUG_KMS("[PLANE:%d:%s] blocks required = %u/%u, lines required = %u/31\n",
>> > > > > +                                   plane->base.id, plane->name,
>> > > > > +                                   res_blocks, ddb_allocation, res_lines);
>> > > > > +                     return -EINVAL;
>> > > > > +             }
>> > > > >        }
>> > > > > 
>> > > > >        *out_blocks = res_blocks;
>> > > > >        *out_lines = res_lines;
>> > > > > +     *enabled = true;
>> > > > > 
>> > > > >        return 0;
>> > > > >   }
>> > > > > 
>> > > > >   static int
>> > > > >   skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
>> > > > > +                   struct skl_ddb_allocation *ddb,
>> > > > >                      struct intel_crtc_state *cstate,
>> > > > >                      const struct intel_plane_state *intel_pstate,
>> > > > >                      struct skl_plane_wm *wm)
>> > > > >   {
>> > > > > +     struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
>> > > > > +     struct drm_plane *plane = intel_pstate->base.plane;
>> > > > > +     struct intel_plane *intel_plane = to_intel_plane(plane);
>> > > > > +     uint16_t ddb_blocks;
>> > > > > +     enum pipe pipe = intel_crtc->pipe;
>> > > > >        int level, max_level = ilk_wm_max_level(dev_priv);
>> > > > >        int ret;
>> > > > > 
>> > > > >        if (WARN_ON(!intel_pstate->base.fb))
>> > > > >                return -EINVAL;
>> > > > > 
>> > > > > +     ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][intel_plane->id]);
>> > > > > +
>> > > > >        for (level = 0; level <= max_level; level++) {
>> > > > >                struct skl_wm_level *result = &wm->wm[level];
>> > > > > 
>> > > > >                ret = skl_compute_plane_wm(dev_priv,
>> > > > >                                           cstate,
>> > > > >                                           intel_pstate,
>> > > > > +                                        ddb_blocks,
>> > > > >                                           level,
>> > > > >                                           &result->plane_res_b,
>> > > > > -                                        &result->plane_res_l);
>> > > > > +                                        &result->plane_res_l,
>> > > > > +                                        &result->plane_en);
>> > > > >                if (ret)
>> > > > >                        return ret;
>> > > > >        }
>> > > > > @@ -4661,7 +4607,8 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
>> > > > > 
>> > > > >                wm = &pipe_wm->planes[plane_id];
>> > > > > 
>> > > > > -             ret = skl_compute_wm_levels(dev_priv, cstate, intel_pstate, wm);
>> > > > > +             ret = skl_compute_wm_levels(dev_priv, ddb, cstate,
>> > > > > +                                         intel_pstate, wm);
>> > > > >                if (ret)
>> > > > >                        return ret;
>> > > > >                skl_compute_transition_wm(cstate, &wm->trans_wm);
>> > > > > @@ -4774,45 +4721,6 @@ bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry **entries,
>> > > > >        return false;
>> > > > >   }
>> > > > > 
>> > > > > -static int
>> > > > > -skl_ddb_add_affected_planes(struct intel_crtc_state *cstate,
>> > > > > -                         const struct skl_pipe_wm *old_pipe_wm,
>> > > > > -                         const struct skl_pipe_wm *pipe_wm)
>> > > > > -{
>> > > > > -     struct drm_atomic_state *state = cstate->base.state;
>> > > > > -     struct drm_device *dev = state->dev;
>> > > > > -     struct drm_crtc *crtc = cstate->base.crtc;
>> > > > > -     struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> > > > > -     struct drm_i915_private *dev_priv = to_i915(dev);
>> > > > > -     struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
>> > > > > -     struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
>> > > > > -     struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl_hw.ddb;
>> > > > > -     struct drm_plane_state *plane_state;
>> > > > > -     struct drm_plane *plane;
>> > > > > -     enum pipe pipe = intel_crtc->pipe;
>> > > > > -
>> > > > > -     WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
>> > > > > -
>> > > > > -     drm_for_each_plane_mask(plane, dev, cstate->base.plane_mask) {
>> > > > > -             enum plane_id plane_id = to_intel_plane(plane)->id;
>> > > > > -             const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
>> > > > > -             const struct skl_plane_wm *old_wm = &old_pipe_wm->planes[plane_id];
>> > > > > -
>> > > > > -             if ((skl_ddb_entry_equal(&cur_ddb->plane[pipe][plane_id],
>> > > > > -                                     &new_ddb->plane[pipe][plane_id]) &&
>> > > > > -                 skl_ddb_entry_equal(&cur_ddb->y_plane[pipe][plane_id],
>> > > > > -                                     &new_ddb->y_plane[pipe][plane_id])) &&
>> > > > > -                 !memcmp(wm, old_wm, sizeof(struct skl_plane_wm)))
>> > > > > -                     continue;
>> > > > > -
>> > > > > -             plane_state = drm_atomic_get_plane_state(state, plane);
>> > > > > -             if (IS_ERR(plane_state))
>> > > > > -                     return PTR_ERR(plane_state);
>> > > > > -     }
>> > > > > -
>> > > > > -     return 0;
>> > > > > -}
>> > > > > -
>> > > > >   static int skl_update_pipe_wm(struct drm_crtc_state *cstate,
>> > > > >                              const struct skl_pipe_wm *old_pipe_wm,
>> > > > >                              struct skl_pipe_wm *pipe_wm, /* out */
>> > > > > @@ -4826,17 +4734,6 @@ static int skl_update_pipe_wm(struct drm_crtc_state *cstate,
>> > > > >        if (ret)
>> > > > >                return ret;
>> > > > > 
>> > > > > -     ret = skl_allocate_pipe_ddb(intel_cstate, pipe_wm, ddb);
>> > > > > -     if (ret)
>> > > > > -             return ret;
>> > > > > -     /*
>> > > > > -      * TODO: Planes are included in state to arm WM registers.
>> > > > > -      * Scope to optimize further, by just rewriting plane surf register.
>> > > > > -      */
>> > > > > -     ret = skl_ddb_add_affected_planes(intel_cstate, old_pipe_wm, pipe_wm);
>> > > > > -     if (ret)
>> > > > > -             return ret;
>> > > > > -
>> > > > >        if (!memcmp(old_pipe_wm, pipe_wm, sizeof(*pipe_wm)))
>> > > > >                *changed = false;
>> > > > >        else
>> > > > > @@ -4859,7 +4756,41 @@ static int skl_update_pipe_wm(struct drm_crtc_state *cstate,
>> > > > >   }
>> > > > > 
>> > > > >   static int
>> > > > > -skl_include_affected_crtcs(struct drm_atomic_state *state)
>> > > > > +skl_ddb_add_affected_planes(struct intel_crtc_state *cstate)
>> > > > > +{
>> > > > > +     struct drm_atomic_state *state = cstate->base.state;
>> > > > > +     struct drm_device *dev = state->dev;
>> > > > > +     struct drm_crtc *crtc = cstate->base.crtc;
>> > > > > +     struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> > > > > +     struct drm_i915_private *dev_priv = to_i915(dev);
>> > > > > +     struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
>> > > > > +     struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
>> > > > > +     struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl_hw.ddb;
>> > > > > +     struct drm_plane_state *plane_state;
>> > > > > +     struct drm_plane *plane;
>> > > > > +     enum pipe pipe = intel_crtc->pipe;
>> > > > > +
>> > > > > +     WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
>> > > > > +
>> > > > > +     drm_for_each_plane_mask(plane, dev, cstate->base.plane_mask) {
>> > > > > +             enum plane_id plane_id = to_intel_plane(plane)->id;
>> > > > > +
>> > > > > +             if (skl_ddb_entry_equal(&cur_ddb->plane[pipe][plane_id],
>> > > > > +                                     &new_ddb->plane[pipe][plane_id]) &&
>> > > > > +                 skl_ddb_entry_equal(&cur_ddb->y_plane[pipe][plane_id],
>> > > > > +                                     &new_ddb->y_plane[pipe][plane_id]))
>> > > > > +                     continue;
>> > > > > +
>> > > > > +             plane_state = drm_atomic_get_plane_state(state, plane);
>> > > > > +             if (IS_ERR(plane_state))
>> > > > > +                     return PTR_ERR(plane_state);
>> > > > > +     }
>> > > > > +
>> > > > > +     return 0;
>> > > > > +}
>> > > > > +
>> > > > > +static int
>> > > > > +skl_compute_ddb(struct drm_atomic_state *state)
>> > > > >   {
>> > > > >        struct drm_device *dev = state->dev;
>> > > > >        struct drm_i915_private *dev_priv = to_i915(dev);
>> > > > > @@ -4923,6 +4854,14 @@ static int skl_update_pipe_wm(struct drm_crtc_state *cstate,
>> > > > >                cstate = intel_atomic_get_crtc_state(state, intel_crtc);
>> > > > >                if (IS_ERR(cstate))
>> > > > >                        return PTR_ERR(cstate);
>> > > > > +
>> > > > > +             ret = skl_allocate_pipe_ddb(cstate, ddb);
>> > > > > +             if (ret)
>> > > > > +                     return ret;
>> > > > > +
>> > > > > +             ret = skl_ddb_add_affected_planes(cstate);
>> > > > > +             if (ret)
>> > > > > +                     return ret;
>> > > > >        }
>> > > > > 
>> > > > >        return 0;
>> > > > > @@ -5012,7 +4951,7 @@ static int skl_update_pipe_wm(struct drm_crtc_state *cstate,
>> > > > >        /* Clear all dirty flags */
>> > > > >        results->dirty_pipes = 0;
>> > > > > 
>> > > > > -     ret = skl_include_affected_crtcs(state);
>> > > > > +     ret = skl_compute_ddb(state);
>> > > > >        if (ret)
>> > > > >                return ret;
>> > > > > 
>> > > > > --
>> > > > > 1.9.1
>> > > > > 
>> > > > 
>> > > > --
>> > > > Matt Roper
>> > > > Graphics Software Engineer
>> > > > IoTG Platform Enabling & Development
>> > > > Intel Corporation
>> > > > (916) 356-2795
>> > > > _______________________________________________
>> > > > Intel-gfx mailing list
>> > > > Intel-gfx at lists.freedesktop.org
>> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> > > 
>> > > 
>> > 
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center


More information about the Intel-gfx mailing list