From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.6 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, HTML_MESSAGE,INCLUDES_PATCH,MAILING_LIST_MULTI,MIME_HTML_MOSTLY,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B98FEC3F2D1 for ; Fri, 28 Feb 2020 12:23:46 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9773F246A8 for ; Fri, 28 Feb 2020 12:23:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9773F246A8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9FEE66E174; Fri, 28 Feb 2020 12:23:45 +0000 (UTC) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by gabe.freedesktop.org (Postfix) with ESMTPS id 262E36E172 for ; Fri, 28 Feb 2020 12:23:44 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Feb 2020 04:23:43 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,496,1574150400"; d="scan'208,217";a="238747531" Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66]) by orsmga003.jf.intel.com with ESMTP; 28 Feb 2020 04:23:41 -0800 Received: from irsmsx602.ger.corp.intel.com (163.33.146.8) by IRSMSX152.ger.corp.intel.com (163.33.192.66) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 28 Feb 2020 12:23:40 +0000 Received: from irsmsx604.ger.corp.intel.com (163.33.146.137) by irsmsx602.ger.corp.intel.com (163.33.146.8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Fri, 28 Feb 2020 12:23:40 +0000 Received: from irsmsx604.ger.corp.intel.com ([163.33.146.137]) by IRSMSX604.ger.corp.intel.com ([163.33.146.137]) with mapi id 15.01.1713.004; Fri, 28 Feb 2020 12:23:40 +0000 From: "Lisovskiy, Stanislav" To: =?iso-8859-1?Q?Ville_Syrj=E4l=E4?= Thread-Topic: [PATCH v18 2/8] drm/i915: Introduce skl_plane_wm_level accessor. Thread-Index: AQHV6ygZDq+k27wAR0+wklAiqx1iEagvNVcAgAFWhVY= Date: Fri, 28 Feb 2020 12:23:40 +0000 Message-ID: <4661cc60f2a54e90bca94bb5441ee7c0@intel.com> References: <20200224153240.9047-1-stanislav.lisovskiy@intel.com> <20200224153240.9047-3-stanislav.lisovskiy@intel.com>, <20200227155152.GP13686@intel.com> In-Reply-To: <20200227155152.GP13686@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.253.164] MIME-Version: 1.0 Subject: Re: [Intel-gfx] [PATCH v18 2/8] drm/i915: Introduce skl_plane_wm_level accessor. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "intel-gfx@lists.freedesktop.org" Content-Type: multipart/mixed; boundary="===============1067665903==" Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" --===============1067665903== Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_4661cc60f2a54e90bca94bb5441ee7c0intelcom_" --_000_4661cc60f2a54e90bca94bb5441ee7c0intelcom_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable >> v2: - plane_id -> plane->id(Ville Syrj=E4l=E4) >When did I say that? Can't find a previous review of this patch. >Anywyas, that change seems to cause a lot of needless noise into the >patch, and atm I can't see why we'd require it. Your comment was in https://patchwork.freedesktop.org/patch/345025/?series= =3D68028&rev=3D14, however I seem to have wrongly interpreted it. I think my motivation to swi= tch to plane based iteration was because its way easier to call skl_plane_wm_le= vel function then, because it takes plane itself as a parameter, also as it had= already an id, thought it is also better that way, rather than keeping one more var= iable instead. Whatever.. I'm fine with both, that is not critical anyways. Stan ________________________________ From: Ville Syrj=E4l=E4 Sent: Thursday, February 27, 2020 5:51:52 PM To: Lisovskiy, Stanislav Cc: intel-gfx@lists.freedesktop.org; Ausmus, James; Saarinen, Jani; Roper, = Matthew D Subject: Re: [PATCH v18 2/8] drm/i915: Introduce skl_plane_wm_level accesso= r. On Mon, Feb 24, 2020 at 05:32:34PM +0200, Stanislav Lisovskiy wrote: > For future Gen12 SAGV implementation we need to > seemlessly alter wm levels calculated, depending > on whether we are allowed to enable SAGV or not. > > So this accessor will give additional flexibility > to do that. > > Currently this accessor is still simply working > as "pass-through" function. This will be changed > in next coming patches from this series. > > v2: - plane_id -> plane->id(Ville Syrj=E4l=E4) When did I say that? Can't find a previous review of this patch. Anywyas, that change seems to cause a lot of needless noise into the patch, and atm I can't see why we'd require it. > - Moved wm_level var to have more local scope > (Ville Syrj=E4l=E4) > - Renamed yuv to color_plane(Ville Syrj=E4l=E4) in > skl_plane_wm_level > > Signed-off-by: Stanislav Lisovskiy > --- > drivers/gpu/drm/i915/intel_pm.c | 120 +++++++++++++++++++++----------- > 1 file changed, 81 insertions(+), 39 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel= _pm.c > index d6933e382657..e1d167429489 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4548,6 +4548,18 @@ icl_get_total_relative_data_rate(struct intel_crtc= _state *crtc_state, > return total_data_rate; > } > > +static const struct skl_wm_level * > +skl_plane_wm_level(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state, nit: I'd put the crtc_state as the first parameter as that's the thing we're operating on. The other stuff just specifies which piece we want to dig out. > + int level, > + int color_plane) > +{ > + const struct skl_plane_wm *wm =3D > + &crtc_state->wm.skl.optimal.planes[plane->id]; > + > + return color_plane ? &wm->uv_wm[level] : &wm->wm[level]; > +} > + > static int > skl_allocate_pipe_ddb(struct intel_crtc_state *crtc_state) > { > @@ -4560,7 +4572,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *crtc= _state) > u16 total[I915_MAX_PLANES] =3D {}; > u16 uv_total[I915_MAX_PLANES] =3D {}; > u64 total_data_rate; > - enum plane_id plane_id; > + struct intel_plane *plane; > int num_active; > u64 plane_data_rate[I915_MAX_PLANES] =3D {}; > u64 uv_plane_data_rate[I915_MAX_PLANES] =3D {}; > @@ -4612,22 +4624,28 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cr= tc_state) > */ > for (level =3D ilk_wm_max_level(dev_priv); level >=3D 0; level--) = { > blocks =3D 0; > - for_each_plane_id_on_crtc(intel_crtc, plane_id) { > - const struct skl_plane_wm *wm =3D > - &crtc_state->wm.skl.optimal.planes[plane_id= ]; > > - if (plane_id =3D=3D PLANE_CURSOR) { > - if (wm->wm[level].min_ddb_alloc > total[PLA= NE_CURSOR]) { > + for_each_intel_plane_on_crtc(&dev_priv->drm, intel_crtc, pl= ane) { > + const struct skl_wm_level *wm_level; > + const struct skl_wm_level *wm_uv_level; > + > + wm_level =3D skl_plane_wm_level(plane, crtc_state, > + level, false); > + wm_uv_level =3D skl_plane_wm_level(plane, crtc_stat= e, > + level, true); false/true aren't particularly sensible color plane indices. > + > + if (plane->id =3D=3D PLANE_CURSOR) { > + if (wm_level->min_ddb_alloc > total[PLANE_C= URSOR]) { > drm_WARN_ON(&dev_priv->drm, > - wm->wm[level].min_ddb_a= lloc !=3D U16_MAX); > + wm_level->min_ddb_alloc= !=3D U16_MAX); > blocks =3D U32_MAX; > break; > } > continue; > } > > - blocks +=3D wm->wm[level].min_ddb_alloc; > - blocks +=3D wm->uv_wm[level].min_ddb_alloc; > + blocks +=3D wm_level->min_ddb_alloc; > + blocks +=3D wm_uv_level->min_ddb_alloc; > } > > if (blocks <=3D alloc_size) { > @@ -4649,13 +4667,18 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cr= tc_state) > * watermark level, plus an extra share of the leftover blocks > * proportional to its relative data rate. > */ > - for_each_plane_id_on_crtc(intel_crtc, plane_id) { > - const struct skl_plane_wm *wm =3D > - &crtc_state->wm.skl.optimal.planes[plane_id]; > + for_each_intel_plane_on_crtc(&dev_priv->drm, intel_crtc, plane) { > + const struct skl_wm_level *wm_level; > + const struct skl_wm_level *wm_uv_level; > u64 rate; > u16 extra; > > - if (plane_id =3D=3D PLANE_CURSOR) > + wm_level =3D skl_plane_wm_level(plane, crtc_state, > + level, false); > + wm_uv_level =3D skl_plane_wm_level(plane, crtc_state, > + level, true); > + > + if (plane->id =3D=3D PLANE_CURSOR) > continue; > > /* > @@ -4665,22 +4688,22 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cr= tc_state) > if (total_data_rate =3D=3D 0) > break; > > - rate =3D plane_data_rate[plane_id]; > + rate =3D plane_data_rate[plane->id]; > extra =3D min_t(u16, alloc_size, > DIV64_U64_ROUND_UP(alloc_size * rate, > total_data_rate)); > - total[plane_id] =3D wm->wm[level].min_ddb_alloc + extra; > + total[plane->id] =3D wm_level->min_ddb_alloc + extra; > alloc_size -=3D extra; > total_data_rate -=3D rate; > > if (total_data_rate =3D=3D 0) > break; > > - rate =3D uv_plane_data_rate[plane_id]; > + rate =3D uv_plane_data_rate[plane->id]; > extra =3D min_t(u16, alloc_size, > DIV64_U64_ROUND_UP(alloc_size * rate, > total_data_rate)); > - uv_total[plane_id] =3D wm->uv_wm[level].min_ddb_alloc + ext= ra; > + uv_total[plane->id] =3D wm_uv_level->min_ddb_alloc + extra; > alloc_size -=3D extra; > total_data_rate -=3D rate; > } > @@ -4688,29 +4711,29 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cr= tc_state) > > /* Set the actual DDB start/end points for each plane */ > start =3D alloc->start; > - for_each_plane_id_on_crtc(intel_crtc, plane_id) { > + for_each_intel_plane_on_crtc(&dev_priv->drm, intel_crtc, plane) { > struct skl_ddb_entry *plane_alloc =3D > - &crtc_state->wm.skl.plane_ddb_y[plane_id]; > + &crtc_state->wm.skl.plane_ddb_y[plane->id]; > struct skl_ddb_entry *uv_plane_alloc =3D > - &crtc_state->wm.skl.plane_ddb_uv[plane_id]; > + &crtc_state->wm.skl.plane_ddb_uv[plane->id]; > > - if (plane_id =3D=3D PLANE_CURSOR) > + if (plane->id =3D=3D PLANE_CURSOR) > continue; > > /* Gen11+ uses a separate plane for UV watermarks */ > drm_WARN_ON(&dev_priv->drm, > - INTEL_GEN(dev_priv) >=3D 11 && uv_total[plane_i= d]); > + INTEL_GEN(dev_priv) >=3D 11 && uv_total[plane->= id]); > > /* Leave disabled planes at (0,0) */ > - if (total[plane_id]) { > + if (total[plane->id]) { > plane_alloc->start =3D start; > - start +=3D total[plane_id]; > + start +=3D total[plane->id]; > plane_alloc->end =3D start; > } > > - if (uv_total[plane_id]) { > + if (uv_total[plane->id]) { > uv_plane_alloc->start =3D start; > - start +=3D uv_total[plane_id]; > + start +=3D uv_total[plane->id]; > uv_plane_alloc->end =3D start; > } > } > @@ -4722,9 +4745,16 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *crt= c_state) > * that aren't actually possible. > */ > for (level++; level <=3D ilk_wm_max_level(dev_priv); level++) { > - for_each_plane_id_on_crtc(intel_crtc, plane_id) { > + for_each_intel_plane_on_crtc(&dev_priv->drm, intel_crtc, pl= ane) { > + const struct skl_wm_level *wm_level; > + const struct skl_wm_level *wm_uv_level; > struct skl_plane_wm *wm =3D > - &crtc_state->wm.skl.optimal.planes[plane_id= ]; > + &crtc_state->wm.skl.optimal.planes[plane->i= d]; > + > + wm_level =3D skl_plane_wm_level(plane, crtc_state, > + level, false); > + wm_uv_level =3D skl_plane_wm_level(plane, crtc_stat= e, > + level, true); > > /* > * We only disable the watermarks for each plane i= f > @@ -4738,9 +4768,10 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *crt= c_state) > * planes must be enabled before the level will b= e used." > * So this is actually safe to do. > */ > - if (wm->wm[level].min_ddb_alloc > total[plane_id] |= | > - wm->uv_wm[level].min_ddb_alloc > uv_total[plane= _id]) > - memset(&wm->wm[level], 0, sizeof(wm->wm[lev= el])); > + if (wm_level->min_ddb_alloc > total[plane->id] || > + wm_uv_level->min_ddb_alloc > uv_total[plane->id= ]) > + memset(&wm->wm[level], 0, > + sizeof(struct skl_wm_level)); > > /* > * Wa_1408961008:icl, ehl > @@ -4748,9 +4779,14 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *crt= c_state) > */ > if (IS_GEN(dev_priv, 11) && > level =3D=3D 1 && wm->wm[0].plane_en) { > - wm->wm[level].plane_res_b =3D wm->wm[0].pla= ne_res_b; > - wm->wm[level].plane_res_l =3D wm->wm[0].pla= ne_res_l; > - wm->wm[level].ignore_lines =3D wm->wm[0].ig= nore_lines; > + wm_level =3D skl_plane_wm_level(plane, crtc= _state, > + 0, false); > + wm->wm[level].plane_res_b =3D > + wm_level->plane_res_b; > + wm->wm[level].plane_res_l =3D > + wm_level->plane_res_l; > + wm->wm[level].ignore_lines =3D > + wm_level->ignore_lines; > } > } > } > @@ -4759,11 +4795,11 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cr= tc_state) > * Go back and disable the transition watermark if it turns out we > * don't have enough DDB blocks for it. > */ > - for_each_plane_id_on_crtc(intel_crtc, plane_id) { > + for_each_intel_plane_on_crtc(&dev_priv->drm, intel_crtc, plane) { > struct skl_plane_wm *wm =3D > - &crtc_state->wm.skl.optimal.planes[plane_id]; > + &crtc_state->wm.skl.optimal.planes[plane->id]; > > - if (wm->trans_wm.plane_res_b >=3D total[plane_id]) > + if (wm->trans_wm.plane_res_b >=3D total[plane->id]) > memset(&wm->trans_wm, 0, sizeof(wm->trans_wm)); > } > > @@ -5354,10 +5390,13 @@ void skl_write_plane_wm(struct intel_plane *plane= , > &crtc_state->wm.skl.plane_ddb_y[plane_id]; > const struct skl_ddb_entry *ddb_uv =3D > &crtc_state->wm.skl.plane_ddb_uv[plane_id]; > + const struct skl_wm_level *wm_level; These can be in tighter scope. > > for (level =3D 0; level <=3D max_level; level++) { > + wm_level =3D skl_plane_wm_level(plane, crtc_state, level, f= alse); > + > skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, leve= l), > - &wm->wm[level]); > + wm_level); > } > skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id), > &wm->trans_wm); > @@ -5388,10 +5427,13 @@ void skl_write_cursor_wm(struct intel_plane *plan= e, > &crtc_state->wm.skl.optimal.planes[plane_id]; > const struct skl_ddb_entry *ddb =3D > &crtc_state->wm.skl.plane_ddb_y[plane_id]; > + const struct skl_wm_level *wm_level; > > for (level =3D 0; level <=3D max_level; level++) { > + wm_level =3D skl_plane_wm_level(plane, crtc_state, level, f= alse); > + > skl_write_wm_level(dev_priv, CUR_WM(pipe, level), > - &wm->wm[level]); > + wm_level); > } > skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe), &wm->trans_wm); > > -- > 2.24.1.485.gad05a3d8e5 -- Ville Syrj=E4l=E4 Intel --_000_4661cc60f2a54e90bca94bb5441ee7c0intelcom_ Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable

>> v2: - plane_id -> plane->id(Ville Sy= rj=E4l=E4)

>When did I say that? Can't find a previous review = of this patch.
>Anywyas, that change seems to cause a lot of needl= ess noise into the
>patch, and atm I can't see why we'd require it.


Your comment was in https://patchwork.freedesktop.org/patch/34502= 5/?series=3D68028&rev=3D14,

however I seem to have wrongly interpreted it. I think my motivation to = switch

to plane based iteration was because its way easier to call skl_plane_wm= _level

function then, because it takes plane itself as a parameter, also as it = had already

an id, thought it is also better that way, rather than keeping one more = variable

instead. Whatever.. I'm fine with both, that is not critical anyways.


Stan
From: Ville Syrj=E4l=E4 &= lt;ville.syrjala@linux.intel.com>
Sent: Thursday, February 27, 2020 5:51:52 PM
To: Lisovskiy, Stanislav
Cc: intel-gfx@lists.freedesktop.org; Ausmus, James; Saarinen, Jani; = Roper, Matthew D
Subject: Re: [PATCH v18 2/8] drm/i915: Introduce skl_plane_wm_level = accessor.
 
On Mon, Feb 24, 2020 at 05:32:34PM +0200, Stan= islav Lisovskiy wrote:
> For future Gen12 SAGV implementation we need to
> seemlessly alter wm levels calculated, depending
> on whether we are allowed to enable SAGV or not.
>
> So this accessor will give additional flexibility
> to do that.
>
> Currently this accessor is still simply working
> as "pass-through" function. This will be changed
> in next coming patches from this series.
>
> v2: - plane_id -> plane->id(Ville Syrj=E4l=E4)

When did I say that? Can't find a previous review of this patch.
Anywyas, that change seems to cause a lot of needless noise into the
patch, and atm I can't see why we'd require it.

>     - Moved wm_level var to have more local scope<= br> >       (Ville Syrj=E4l=E4)
>     - Renamed yuv to color_plane(Ville Syrj=E4l=E4= ) in
>       skl_plane_wm_level
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com&g= t;
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 120 +++++&= #43;++++++++++++++&= #43;-----------
>  1 file changed, 81 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/in= tel_pm.c
> index d6933e382657..e1d167429489 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4548,6 +4548,18 @@ icl_get_total_relative_data_rate(struct int= el_crtc_state *crtc_state,
>        return total_data_rate;
>  }
> 
> +static const struct skl_wm_level *
> +skl_plane_wm_level(struct intel_plane *plane,
> +           = ;     const struct intel_crtc_state *crtc_state,

nit: I'd put the crtc_state as the first parameter as that's the thing
we're operating on. The other stuff just specifies which piece we want
to dig out.

> +           = ;     int level,
> +           = ;     int color_plane)
> +{
> +     const struct skl_plane_wm *wm =3D
> +           = ;  &crtc_state->wm.skl.optimal.planes[plane->id];
> +
> +     return color_plane ? &wm->uv_wm[l= evel] : &wm->wm[level];
> +}
> +
>  static int
>  skl_allocate_pipe_ddb(struct intel_crtc_state *crtc_state)
>  {
> @@ -4560,7 +4572,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_stat= e *crtc_state)
>        u16 total[I915_MAX_PLANES] = =3D {};
>        u16 uv_total[I915_MAX_PLANES= ] =3D {};
>        u64 total_data_rate;
> -     enum plane_id plane_id;
> +     struct intel_plane *plane;
>        int num_active;
>        u64 plane_data_rate[I915_MAX= _PLANES] =3D {};
>        u64 uv_plane_data_rate[I915_= MAX_PLANES] =3D {};
> @@ -4612,22 +4624,28 @@ skl_allocate_pipe_ddb(struct intel_crtc_st= ate *crtc_state)
>         */
>        for (level =3D ilk_wm_max_le= vel(dev_priv); level >=3D 0; level--) {
>            = ;    blocks =3D 0;
> -           &nb= sp; for_each_plane_id_on_crtc(intel_crtc, plane_id) {
> -           &nb= sp;         const struct skl_plane_= wm *wm =3D
> -           &nb= sp;            =      &crtc_state->wm.skl.optimal.planes[plane_id= ];
> 
> -           &nb= sp;         if (plane_id =3D=3D PLA= NE_CURSOR) {
> -           &nb= sp;            =      if (wm->wm[level].min_ddb_alloc > total[PLAN= E_CURSOR]) {
> +           = ;  for_each_intel_plane_on_crtc(&dev_priv->drm, intel_crtc, pla= ne) {
> +           = ;          const struct skl_wm= _level *wm_level;
> +           = ;          const struct skl_wm= _level *wm_uv_level;
> +
> +           = ;          wm_level =3D skl_pl= ane_wm_level(plane, crtc_state,
> +           = ;            &n= bsp;            = ;            &n= bsp;  level, false);
> +           = ;          wm_uv_level =3D skl= _plane_wm_level(plane, crtc_state,
> +           = ;            &n= bsp;            = ;            &n= bsp;     level, true);

false/true aren't particularly sensible color plane indices.

> +
> +           = ;          if (plane->id = =3D=3D PLANE_CURSOR) {
> +           = ;            &n= bsp;     if (wm_level->min_ddb_alloc > total[PLAN= E_CURSOR]) {
>            = ;            &n= bsp;            = ;   drm_WARN_ON(&dev_priv->drm,
> -           &nb= sp;            =             &nb= sp;            wm-&g= t;wm[level].min_ddb_alloc !=3D U16_MAX);
> +           = ;            &n= bsp;            = ;             w= m_level->min_ddb_alloc !=3D U16_MAX);
>            = ;            &n= bsp;            = ;   blocks =3D U32_MAX;
>            = ;            &n= bsp;            = ;   break;
>            = ;            &n= bsp;       }
>            = ;            &n= bsp;       continue;
>            = ;            }
> 
> -           &nb= sp;         blocks +=3D wm->= wm[level].min_ddb_alloc;
> -           &nb= sp;         blocks +=3D wm->= uv_wm[level].min_ddb_alloc;
> +           = ;          blocks +=3D wm_= level->min_ddb_alloc;
> +           = ;          blocks +=3D wm_= uv_level->min_ddb_alloc;
>            = ;    }
> 
>            = ;    if (blocks <=3D alloc_size) {
> @@ -4649,13 +4667,18 @@ skl_allocate_pipe_ddb(struct intel_crtc_st= ate *crtc_state)
>         * watermark level, plu= s an extra share of the leftover blocks
>         * proportional to its = relative data rate.
>         */
> -     for_each_plane_id_on_crtc(intel_crtc, plane_= id) {
> -           &nb= sp; const struct skl_plane_wm *wm =3D
> -           &nb= sp;         &crtc_state->wm.= skl.optimal.planes[plane_id];
> +     for_each_intel_plane_on_crtc(&dev_pr= iv->drm, intel_crtc, plane) {
> +           = ;  const struct skl_wm_level *wm_level;
> +           = ;  const struct skl_wm_level *wm_uv_level;
>            = ;    u64 rate;
>            = ;    u16 extra;
> 
> -           &nb= sp; if (plane_id =3D=3D PLANE_CURSOR)
> +           = ;  wm_level =3D skl_plane_wm_level(plane, crtc_state,
> +           = ;            &n= bsp;            = ;       level, false);
> +           = ;  wm_uv_level =3D skl_plane_wm_level(plane, crtc_state,
> +           = ;            &n= bsp;            = ;          level, true);
> +
> +           = ;  if (plane->id =3D=3D PLANE_CURSOR)
>            = ;            continu= e;
> 
>            = ;    /*
> @@ -4665,22 +4688,22 @@ skl_allocate_pipe_ddb(struct intel_crtc_st= ate *crtc_state)
>            = ;    if (total_data_rate =3D=3D 0)
>            = ;            break;<= br> > 
> -           &nb= sp; rate =3D plane_data_rate[plane_id];
> +           = ;  rate =3D plane_data_rate[plane->id];
>            = ;    extra =3D min_t(u16, alloc_size,
>            = ;            &n= bsp;     DIV64_U64_ROUND_UP(alloc_size * rate,
>            = ;            &n= bsp;            = ;            total_d= ata_rate));
> -           &nb= sp; total[plane_id] =3D wm->wm[level].min_ddb_alloc + extra;
> +           = ;  total[plane->id] =3D wm_level->min_ddb_alloc + extra;
>            = ;    alloc_size -=3D extra;
>            = ;    total_data_rate -=3D rate;
> 
>            = ;    if (total_data_rate =3D=3D 0)
>            = ;            break;<= br> > 
> -           &nb= sp; rate =3D uv_plane_data_rate[plane_id];
> +           = ;  rate =3D uv_plane_data_rate[plane->id];
>            = ;    extra =3D min_t(u16, alloc_size,
>            = ;            &n= bsp;     DIV64_U64_ROUND_UP(alloc_size * rate,
>            = ;            &n= bsp;            = ;            total_d= ata_rate));
> -           &nb= sp; uv_total[plane_id] =3D wm->uv_wm[level].min_ddb_alloc + extra; > +           = ;  uv_total[plane->id] =3D wm_uv_level->min_ddb_alloc + extr= a;
>            = ;    alloc_size -=3D extra;
>            = ;    total_data_rate -=3D rate;
>        }
> @@ -4688,29 +4711,29 @@ skl_allocate_pipe_ddb(struct intel_crtc_st= ate *crtc_state)
> 
>        /* Set the actual DDB start/= end points for each plane */
>        start =3D alloc->start; > -     for_each_plane_id_on_crtc(intel_crtc, plane_= id) {
> +     for_each_intel_plane_on_crtc(&dev_pr= iv->drm, intel_crtc, plane) {
>            = ;    struct skl_ddb_entry *plane_alloc =3D
> -           &nb= sp;         &crtc_state->wm.= skl.plane_ddb_y[plane_id];
> +           = ;          &crtc_state->= ;wm.skl.plane_ddb_y[plane->id];
>            = ;    struct skl_ddb_entry *uv_plane_alloc =3D
> -           &nb= sp;         &crtc_state->wm.= skl.plane_ddb_uv[plane_id];
> +           = ;          &crtc_state->= ;wm.skl.plane_ddb_uv[plane->id];
> 
> -           &nb= sp; if (plane_id =3D=3D PLANE_CURSOR)
> +           = ;  if (plane->id =3D=3D PLANE_CURSOR)
>            = ;            continu= e;
> 
>            = ;    /* Gen11+ uses a separate plane for UV watermarks *= /
>            = ;    drm_WARN_ON(&dev_priv->drm,
> -           &nb= sp;            = INTEL_GEN(dev_priv) >=3D 11 && uv_total[plane_id]);
> +           = ;            &n= bsp; INTEL_GEN(dev_priv) >=3D 11 && uv_total[plane->id]);
> 
>            = ;    /* Leave disabled planes at (0,0) */
> -           &nb= sp; if (total[plane_id]) {
> +           = ;  if (total[plane->id]) {
>            = ;            plane_a= lloc->start =3D start;
> -           &nb= sp;         start +=3D total[pl= ane_id];
> +           = ;          start +=3D tota= l[plane->id];
>            = ;            plane_a= lloc->end =3D start;
>            = ;    }
> 
> -           &nb= sp; if (uv_total[plane_id]) {
> +           = ;  if (uv_total[plane->id]) {
>            = ;            uv_plan= e_alloc->start =3D start;
> -           &nb= sp;         start +=3D uv_total= [plane_id];
> +           = ;          start +=3D uv_t= otal[plane->id];
>            = ;            uv_plan= e_alloc->end =3D start;
>            = ;    }
>        }
> @@ -4722,9 +4745,16 @@ skl_allocate_pipe_ddb(struct intel_crtc_sta= te *crtc_state)
>         * that aren't actually= possible.
>         */
>        for (level++; level = <=3D ilk_wm_max_level(dev_priv); level++) {
> -           &nb= sp; for_each_plane_id_on_crtc(intel_crtc, plane_id) {
> +           = ;  for_each_intel_plane_on_crtc(&dev_priv->drm, intel_crtc, pla= ne) {
> +           = ;          const struct skl_wm= _level *wm_level;
> +           = ;          const struct skl_wm= _level *wm_uv_level;
>            = ;            struct = skl_plane_wm *wm =3D
> -           &nb= sp;            =      &crtc_state->wm.skl.optimal.planes[plane_id= ];
> +           = ;            &n= bsp;     &crtc_state->wm.skl.optimal.planes[plan= e->id];
> +
> +           = ;          wm_level =3D skl_pl= ane_wm_level(plane, crtc_state,
> +           = ;            &n= bsp;            = ;            &n= bsp;  level, false);
> +           = ;          wm_uv_level =3D skl= _plane_wm_level(plane, crtc_state,
> +           = ;            &n= bsp;            = ;            &n= bsp;     level, true);
> 
>            = ;            /*
>            = ;             *= We only disable the watermarks for each plane if
> @@ -4738,9 +4768,10 @@ skl_allocate_pipe_ddb(struct intel_crtc_sta= te *crtc_state)
>            = ;             *=   planes must be enabled before the level will be used."
>            = ;             *= So this is actually safe to do.
>            = ;             *= /
> -           &nb= sp;         if (wm->wm[level].mi= n_ddb_alloc > total[plane_id] ||
> -           &nb= sp;            = wm->uv_wm[level].min_ddb_alloc > uv_total[plane_id])
> -           &nb= sp;            =      memset(&wm->wm[level], 0, sizeof(wm->wm[= level]));
> +           = ;          if (wm_level->mi= n_ddb_alloc > total[plane->id] ||
> +           = ;            &n= bsp; wm_uv_level->min_ddb_alloc > uv_total[plane->id])
> +           = ;            &n= bsp;     memset(&wm->wm[level], 0,
> +           = ;            &n= bsp;            size= of(struct skl_wm_level));
> 
>            = ;            /*
>            = ;             *= Wa_1408961008:icl, ehl
> @@ -4748,9 +4779,14 @@ skl_allocate_pipe_ddb(struct intel_crtc_sta= te *crtc_state)
>            = ;             *= /
>            = ;            if (IS_= GEN(dev_priv, 11) &&
>            = ;            &n= bsp;   level =3D=3D 1 && wm->wm[0].plane_en) {
> -           &nb= sp;            =      wm->wm[level].plane_res_b =3D wm->wm[0].plan= e_res_b;
> -           &nb= sp;            =      wm->wm[level].plane_res_l =3D wm->wm[0].plan= e_res_l;
> -           &nb= sp;            =      wm->wm[level].ignore_lines =3D wm->wm[0].ign= ore_lines;
> +           = ;            &n= bsp;     wm_level =3D skl_plane_wm_level(plane, crtc_st= ate,
> +           = ;            &n= bsp;            = ;            &n= bsp;          0, false);
> +           = ;            &n= bsp;     wm->wm[level].plane_res_b =3D
> +           = ;            &n= bsp;            = ; wm_level->plane_res_b;
> +           = ;            &n= bsp;     wm->wm[level].plane_res_l =3D
> +           = ;            &n= bsp;            = ; wm_level->plane_res_l;
> +           = ;            &n= bsp;     wm->wm[level].ignore_lines =3D
> +           = ;            &n= bsp;            = ; wm_level->ignore_lines;
>            = ;            }
>            = ;    }
>        }
> @@ -4759,11 +4795,11 @@ skl_allocate_pipe_ddb(struct intel_crtc_st= ate *crtc_state)
>         * Go back and disable = the transition watermark if it turns out we
>         * don't have enough DD= B blocks for it.
>         */
> -     for_each_plane_id_on_crtc(intel_crtc, plane_= id) {
> +     for_each_intel_plane_on_crtc(&dev_pr= iv->drm, intel_crtc, plane) {
>            = ;    struct skl_plane_wm *wm =3D
> -           &nb= sp;         &crtc_state->wm.= skl.optimal.planes[plane_id];
> +           = ;          &crtc_state->= ;wm.skl.optimal.planes[plane->id];
> 
> -           &nb= sp; if (wm->trans_wm.plane_res_b >=3D total[plane_id])
> +           = ;  if (wm->trans_wm.plane_res_b >=3D total[plane->id])
>            = ;            memset(= &wm->trans_wm, 0, sizeof(wm->trans_wm));
>        }
> 
> @@ -5354,10 +5390,13 @@ void skl_write_plane_wm(struct intel_plane= *plane,
>            = ;    &crtc_state->wm.skl.plane_ddb_y[plane_id];
>        const struct skl_ddb_entry *= ddb_uv =3D
>            = ;    &crtc_state->wm.skl.plane_ddb_uv[plane_id];
> +     const struct skl_wm_level *wm_level;

These can be in tighter scope.
> 
>        for (level =3D 0; level <= =3D max_level; level++) {
> +           = ;  wm_level =3D skl_plane_wm_level(plane, crtc_state, level, false); > +
>            = ;    skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, l= evel),
> -           &nb= sp;            =         &wm->wm[level]);
> +           = ;            &n= bsp;        wm_level);
>        }
>        skl_write_wm_level(dev_priv,= PLANE_WM_TRANS(pipe, plane_id),
>            = ;            &n= bsp;  &wm->trans_wm);
> @@ -5388,10 +5427,13 @@ void skl_write_cursor_wm(struct intel_plan= e *plane,
>            = ;    &crtc_state->wm.skl.optimal.planes[plane_id]; >        const struct skl_ddb_entry *= ddb =3D
>            = ;    &crtc_state->wm.skl.plane_ddb_y[plane_id];
> +     const struct skl_wm_level *wm_level;
> 
>        for (level =3D 0; level <= =3D max_level; level++) {
> +           = ;  wm_level =3D skl_plane_wm_level(plane, crtc_state, level, false); > +
>            = ;    skl_write_wm_level(dev_priv, CUR_WM(pipe, level),
> -           &nb= sp;            =         &wm->wm[level]);
> +           = ;            &n= bsp;        wm_level);
>        }
>        skl_write_wm_level(dev_priv,= CUR_WM_TRANS(pipe), &wm->trans_wm);
> 
> --
> 2.24.1.485.gad05a3d8e5

--
Ville Syrj=E4l=E4
Intel
 --_000_4661cc60f2a54e90bca94bb5441ee7c0intelcom_-- --===============1067665903== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1067665903==--