Re: [PATCH 7/7] drm/i915/psr: Disable early transport by default

[Hogander, Jouni]

On Tue, 2024-01-09 at 12:57 +, Kahola, Mika wrote:
> > -Original Message-
> > From: Intel-gfx  On Behalf
> > Of Jouni Högander
> > Sent: Monday, December 18, 2023 7:50 PM
> > To: intel-gfx@lists.freedesktop.org
> > Subject: [PATCH 7/7] drm/i915/psr: Disable early transport by
> > default
> > 
> > Early transport validation is currently incomplete. Due to this
> > disable the feature by default.
> > 
> 
> Reviewed-by: Mika Kahola 

Thank you Mika for reviewing my patches. This whole set is now pushed
to drm-intel-next.

BR,

Jouni Högander

> 
> > Signed-off-by: Jouni Högander 
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index c29fd708608a..7835afee4283 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -2851,6 +2851,9 @@ void intel_psr_init(struct intel_dp
> > *intel_dp)
> > else
> > intel_dp->psr.source_support = true;
> > 
> > +   /* Disable early transport for now */
> > +   intel_dp->psr.debug |= I915_PSR_DEBUG_SU_REGION_ET_DISABLE;
> > +
> > /* Set link_standby x link_off defaults */
> > if (DISPLAY_VER(dev_priv) < 12)
> > /* For new platforms up to TGL let's respect VBT
> > back again */
> > --
> > 2.34.1
> 

✓ Fi.CI.BAT: success for drm/i915/gt: Use rc6.supported flag from intel_gt for rc6_enable sysfs (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/gt: Use rc6.supported flag from intel_gt for rc6_enable sysfs 
(rev2)
URL   : https://patchwork.freedesktop.org/series/128343/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_14103 -> Patchwork_128343v2


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128343v2/index.html

Participating hosts (37 -> 36)
--

  Additional (1): bat-kbl-2 
  Missing(2): bat-rpls-2 fi-snb-2520m 

Known issues


  Here are the changes found in Patchwork_128343v2 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@fbdev@info:
- bat-kbl-2:  NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#1849])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128343v2/bat-kbl-2/igt@fb...@info.html

  * igt@gem_exec_suspend@basic-s0@smem:
- bat-dg2-9:  [PASS][2] -> [INCOMPLETE][3] ([i915#9275])
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14103/bat-dg2-9/igt@gem_exec_suspend@basic...@smem.html
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128343v2/bat-dg2-9/igt@gem_exec_suspend@basic...@smem.html

  * igt@gem_lmem_swapping@parallel-random-engines:
- bat-kbl-2:  NOTRUN -> [SKIP][4] ([fdo#109271]) +36 other tests 
skip
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128343v2/bat-kbl-2/igt@gem_lmem_swapp...@parallel-random-engines.html

  * igt@i915_suspend@basic-s3-without-i915:
- bat-adlp-6: [PASS][5] -> [INCOMPLETE][6] ([i915#7443])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14103/bat-adlp-6/igt@i915_susp...@basic-s3-without-i915.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128343v2/bat-adlp-6/igt@i915_susp...@basic-s3-without-i915.html

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-dp-1:
- bat-dg2-8:  [PASS][7] -> [INCOMPLETE][8] ([i915#1982] / 
[i915#9280])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14103/bat-dg2-8/igt@kms_pipe_crc_basic@suspend-read-...@pipe-a-dp-1.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128343v2/bat-dg2-8/igt@kms_pipe_crc_basic@suspend-read-...@pipe-a-dp-1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
  the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#5591]: https://gitlab.freedesktop.org/drm/intel/issues/5591
  [i915#7443]: https://gitlab.freedesktop.org/drm/intel/issues/7443
  [i915#9275]: https://gitlab.freedesktop.org/drm/intel/issues/9275
  [i915#9280]: https://gitlab.freedesktop.org/drm/intel/issues/9280


Build changes
-

  * Linux: CI_DRM_14103 -> Patchwork_128343v2

  CI-20190529: 20190529
  CI_DRM_14103: e727f148b7ccd42555b101f35fe28b0d0b1d34f0 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7664: 5b8a8dcca7711c73f9cfeb66b5deb27751290fb4 @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_128343v2: e727f148b7ccd42555b101f35fe28b0d0b1d34f0 @ 
git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

0cba1204bc6e drm/i915/gt: Use rc6.supported flag from intel_gt for rc6_enable 
sysfs

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128343v2/index.html

[PATCH] drm/i915/gt: Use rc6.supported flag from intel_gt for rc6_enable sysfs

[Juan Escamilla]

Currently if rc6 is supported, it gets enabled and the sysfs files for
rc6_enable_show and rc6_enable_dev_show uses masks to check information
from drm_i915_private.

However rc6_support functions take more variables and conditions into
consideration and thus these masks are not enough for most of the modern
hardware and it is simpley lyting to the user.

Let's fix it by at least use the rc6.supported flag from intel_gt
information.

Signed-off-by: Juan Escamilla 
---
 drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c 
b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
index f0dea54880af..2d3c4dab6d21 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
@@ -176,27 +176,13 @@ static u32 get_residency(struct intel_gt *gt, enum 
intel_rc6_res_type id)
return DIV_ROUND_CLOSEST_ULL(res, 1000);
 }
 
-static u8 get_rc6_mask(struct intel_gt *gt)
-{
-   u8 mask = 0;
-
-   if (HAS_RC6(gt->i915))
-   mask |= BIT(0);
-   if (HAS_RC6p(gt->i915))
-   mask |= BIT(1);
-   if (HAS_RC6pp(gt->i915))
-   mask |= BIT(2);
-
-   return mask;
-}
-
 static ssize_t rc6_enable_show(struct kobject *kobj,
   struct kobj_attribute *attr,
   char *buff)
 {
struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
 
-   return sysfs_emit(buff, "%x\n", get_rc6_mask(gt));
+   return sysfs_emit(buff, "%x\n", gt->rc6.supported);
 }
 
 static ssize_t rc6_enable_dev_show(struct device *dev,
@@ -205,7 +191,7 @@ static ssize_t rc6_enable_dev_show(struct device *dev,
 {
struct intel_gt *gt = intel_gt_sysfs_get_drvdata(&dev->kobj, 
attr->attr.name);
 
-   return sysfs_emit(buff, "%x\n", get_rc6_mask(gt));
+   return sysfs_emit(buff, "%x\n", gt->rc6.supported);
 }
 
 static u32 __rc6_residency_ms_show(struct intel_gt *gt)
-- 
2.43.0

Re: ✗ Fi.CI.IGT: failure for Resolve suspend-resume racing with GuC destroy-context-worker (rev13)

[Rodrigo Vivi]

On Thu, Jan 04, 2024 at 05:39:16PM +, Teres Alexis, Alan Previn wrote:
> On Thu, 2024-01-04 at 10:57 +, Patchwork wrote:
> > Patch Details
> > Series: Resolve suspend-resume racing with GuC destroy-context-worker 
> > (rev13)
> > URL:https://patchwork.freedesktop.org/series/121916/
> > State:  failure
> > Details:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121916v13/index.html
> > CI Bug Log - changes from CI_DRM_14076_full -> Patchwork_121916v13_full
> > Summary
> > 
> > FAILURE
> alan:snip
> 
> 
> > Here are the unknown changes that may have been introduced in 
> > Patchwork_121916v13_full:
> > 
> > IGT changes
> > Possible regressions
> > 
> >   *   igt@gem_eio@wait-wedge-immediate:
> >  *   shard-mtlp: 
> > PASS
> >  -> 
> > ABORT
> > 
> alan: from the code and dmesg, this is unrelated to guc context destruction 
> flows.
> Its reading an MCR register that times out. Additionally, i believe this 
> error is occuring during post-reset-init flows.
> So its definitely not doing any context destruction at this point (as reset 
> would have happenned sooner).

yeap, it is indeed happening once in a while:

https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7659/shard-mtlp-4/igt@gem_...@wait-wedge-immediate.html

I was going to merge the series now, but then I noticed that Matt had taken 
care of that.
Thank you all.

> > Known issues
> > 
> 

Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace

[Daniel Stone]

Hi,

On Tue, 9 Jan 2024 at 18:12, Andri Yngvason  wrote:
> + * active color format:
> + * This read-only property tells userspace the color format actually used
> + * by the hardware display engine "on the cable" on a connector. The 
> chosen
> + * value depends on hardware capabilities, both display engine and
> + * connected monitor. Drivers shall use
> + * drm_connector_attach_active_color_format_property() to install this
> + * property. Possible values are "not applicable", "rgb", "ycbcr444",
> + * "ycbcr422", and "ycbcr420".

How does userspace determine what's happened without polling? Will it
only change after an `ALLOW_MODESET` commit, and be guaranteed to be
updated after the commit has completed and the event being sent?
Should it send a HOTPLUG event? Other?

Cheers,
Daniel

Re: [PATCH] drm/xe/display: Disable aux ccs framebuffers

[Souza, Jose]

On Mon, 2024-01-08 at 17:18 -0500, Rodrigo Vivi wrote:
> On Tue, Jan 02, 2024 at 09:44:48PM +0200, Jani Nikula wrote:
> > On Tue, 02 Jan 2024, Juha-Pekka Heikkila  
> > wrote:
> > > Aux ccs framebuffers don't work on Xe driver hence disable them
> > > from plane capabilities until they are fixed. Flat ccs framebuffers
> > > work and they are left enabled. Here is separated plane capabilities
> > > check on i915 so it can behave differencly depending on the driver.
> > 
> > Cc: Rodrigo and xe maintainers
> > 
> > We need to figure out the proper workflow, the mailing lists to use, the
> > subject prefix to use, the acks to require, etc, for changes touching
> > both xe and i915.
> > 
> > I'd very much prefer changes to i915 display to be merged via
> > drm-intel-next as always. For one thing, it'll take a while to sync
> > stuff back from drm-xe-next to drm-intel-next, and most display
> > development still happens on drm-intel-next.
> 
> I fully agree with you.
> 
> > 
> > But this patch can't be applied to drm-intel-next, because xe doesn't
> > even exist on drm-intel-next yet...
> 
> should we do a backmerge of drm-next already, or too early for that?

Can we split it into 2 patches and merge it?
This is necessary to fix Wayland compositors on ADL and newer.

> 
> > 
> > 
> > BR,
> > Jani.
> > 
> > 
> > > 
> > > Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/933
> > > Signed-off-by: Juha-Pekka Heikkila 
> > > ---
> > >  drivers/gpu/drm/i915/Makefile |  1 +
> > >  .../gpu/drm/i915/display/intel_plane_caps.c   | 68 +++
> > >  .../gpu/drm/i915/display/intel_plane_caps.h   | 14 
> > >  .../drm/i915/display/skl_universal_plane.c| 61 +
> > >  drivers/gpu/drm/xe/display/xe_plane_initial.c | 23 +++
> > >  5 files changed, 107 insertions(+), 60 deletions(-)
> > >  create mode 100644 drivers/gpu/drm/i915/display/intel_plane_caps.c
> > >  create mode 100644 drivers/gpu/drm/i915/display/intel_plane_caps.h
> > > 
> > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > > index e777686190ca..c5e3c2dd0a01 100644
> > > --- a/drivers/gpu/drm/i915/Makefile
> > > +++ b/drivers/gpu/drm/i915/Makefile
> > > @@ -302,6 +302,7 @@ i915-y += \
> > >   display/intel_overlay.o \
> > >   display/intel_pch_display.o \
> > >   display/intel_pch_refclk.o \
> > > + display/intel_plane_caps.o \
> > >   display/intel_plane_initial.o \
> > >   display/intel_pmdemand.o \
> > >   display/intel_psr.o \
> > > diff --git a/drivers/gpu/drm/i915/display/intel_plane_caps.c 
> > > b/drivers/gpu/drm/i915/display/intel_plane_caps.c
> > > new file mode 100644
> > > index ..6206ae11f296
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/display/intel_plane_caps.c
> > > @@ -0,0 +1,68 @@
> > > +// SPDX-License-Identifier: MIT
> > > +/*
> > > + * Copyright © 2024 Intel Corporation
> > > + */
> > > +
> > > +#include "i915_drv.h"
> > > +#include "intel_fb.h"
> > > +#include "intel_plane_caps.h"
> > > +
> > > +static bool skl_plane_has_rc_ccs(struct drm_i915_private *i915,
> > > +  enum pipe pipe, enum plane_id plane_id)
> > > +{
> > > + /* Wa_22011186057 */
> > > + if (IS_ALDERLAKE_P(i915) && IS_DISPLAY_STEP(i915, STEP_A0, STEP_B0))
> > > + return false;
> > > +
> > > + if (DISPLAY_VER(i915) >= 11)
> > > + return true;
> > > +
> > > + if (IS_GEMINILAKE(i915))
> > > + return pipe != PIPE_C;
> > > +
> > > + return pipe != PIPE_C &&
> > > + (plane_id == PLANE_PRIMARY ||
> > > +  plane_id == PLANE_SPRITE0);
> > > +}
> > > +
> > > +static bool gen12_plane_has_mc_ccs(struct drm_i915_private *i915,
> > > +enum plane_id plane_id)
> > > +{
> > > + if (DISPLAY_VER(i915) < 12)
> > > + return false;
> > > +
> > > + /* Wa_14010477008 */
> > > + if (IS_DG1(i915) || IS_ROCKETLAKE(i915) ||
> > > + (IS_TIGERLAKE(i915) && IS_DISPLAY_STEP(i915, STEP_A0, STEP_D0)))
> > > + return false;
> > > +
> > > + /* Wa_22011186057 */
> > > + if (IS_ALDERLAKE_P(i915) && IS_DISPLAY_STEP(i915, STEP_A0, STEP_B0))
> > > + return false;
> > > +
> > > + return plane_id < PLANE_SPRITE4;
> > > +}
> > > +
> > > +u8 skl_get_plane_caps(struct drm_i915_private *i915,
> > > +   enum pipe pipe, enum plane_id plane_id)
> > > +{
> > > + u8 caps = INTEL_PLANE_CAP_TILING_X;
> > > +
> > > + if (DISPLAY_VER(i915) < 13 || IS_ALDERLAKE_P(i915))
> > > + caps |= INTEL_PLANE_CAP_TILING_Y;
> > > + if (DISPLAY_VER(i915) < 12)
> > > + caps |= INTEL_PLANE_CAP_TILING_Yf;
> > > + if (HAS_4TILE(i915))
> > > + caps |= INTEL_PLANE_CAP_TILING_4;
> > > +
> > > + if (skl_plane_has_rc_ccs(i915, pipe, plane_id)) {
> > > + caps |= INTEL_PLANE_CAP_CCS_RC;
> > > + if (DISPLAY_VER(i915) >= 12)
> > > + caps |= INTEL_PLANE_CAP_CCS_RC_CC;
> > > + }
> > > +
> > > + if (gen12_plane_has_mc_ccs(i915, plane_id))
> > > + caps |= INTE

Re: [PATCH] drm/xe/display: Disable aux ccs framebuffers

[Souza, Jose]

On Tue, 2024-01-02 at 20:24 +0200, Juha-Pekka Heikkila wrote:
> Aux ccs framebuffers don't work on Xe driver hence disable them
> from plane capabilities until they are fixed. Flat ccs framebuffers
> work and they are left enabled. Here is separated plane capabilities
> check on i915 so it can behave differencly depending on the driver.
> 

Reviewed-by: José Roberto de Souza 

> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/933
> Signed-off-by: Juha-Pekka Heikkila 
> ---
>  drivers/gpu/drm/i915/Makefile |  1 +
>  .../gpu/drm/i915/display/intel_plane_caps.c   | 68 +++
>  .../gpu/drm/i915/display/intel_plane_caps.h   | 14 
>  .../drm/i915/display/skl_universal_plane.c| 61 +
>  drivers/gpu/drm/xe/display/xe_plane_initial.c | 23 +++
>  5 files changed, 107 insertions(+), 60 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_plane_caps.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_plane_caps.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index e777686190ca..c5e3c2dd0a01 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -302,6 +302,7 @@ i915-y += \
>   display/intel_overlay.o \
>   display/intel_pch_display.o \
>   display/intel_pch_refclk.o \
> + display/intel_plane_caps.o \
>   display/intel_plane_initial.o \
>   display/intel_pmdemand.o \
>   display/intel_psr.o \
> diff --git a/drivers/gpu/drm/i915/display/intel_plane_caps.c 
> b/drivers/gpu/drm/i915/display/intel_plane_caps.c
> new file mode 100644
> index ..6206ae11f296
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_plane_caps.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#include "i915_drv.h"
> +#include "intel_fb.h"
> +#include "intel_plane_caps.h"
> +
> +static bool skl_plane_has_rc_ccs(struct drm_i915_private *i915,
> +  enum pipe pipe, enum plane_id plane_id)
> +{
> + /* Wa_22011186057 */
> + if (IS_ALDERLAKE_P(i915) && IS_DISPLAY_STEP(i915, STEP_A0, STEP_B0))
> + return false;
> +
> + if (DISPLAY_VER(i915) >= 11)
> + return true;
> +
> + if (IS_GEMINILAKE(i915))
> + return pipe != PIPE_C;
> +
> + return pipe != PIPE_C &&
> + (plane_id == PLANE_PRIMARY ||
> +  plane_id == PLANE_SPRITE0);
> +}
> +
> +static bool gen12_plane_has_mc_ccs(struct drm_i915_private *i915,
> +enum plane_id plane_id)
> +{
> + if (DISPLAY_VER(i915) < 12)
> + return false;
> +
> + /* Wa_14010477008 */
> + if (IS_DG1(i915) || IS_ROCKETLAKE(i915) ||
> + (IS_TIGERLAKE(i915) && IS_DISPLAY_STEP(i915, STEP_A0, STEP_D0)))
> + return false;
> +
> + /* Wa_22011186057 */
> + if (IS_ALDERLAKE_P(i915) && IS_DISPLAY_STEP(i915, STEP_A0, STEP_B0))
> + return false;
> +
> + return plane_id < PLANE_SPRITE4;
> +}
> +
> +u8 skl_get_plane_caps(struct drm_i915_private *i915,
> +   enum pipe pipe, enum plane_id plane_id)
> +{
> + u8 caps = INTEL_PLANE_CAP_TILING_X;
> +
> + if (DISPLAY_VER(i915) < 13 || IS_ALDERLAKE_P(i915))
> + caps |= INTEL_PLANE_CAP_TILING_Y;
> + if (DISPLAY_VER(i915) < 12)
> + caps |= INTEL_PLANE_CAP_TILING_Yf;
> + if (HAS_4TILE(i915))
> + caps |= INTEL_PLANE_CAP_TILING_4;
> +
> + if (skl_plane_has_rc_ccs(i915, pipe, plane_id)) {
> + caps |= INTEL_PLANE_CAP_CCS_RC;
> + if (DISPLAY_VER(i915) >= 12)
> + caps |= INTEL_PLANE_CAP_CCS_RC_CC;
> + }
> +
> + if (gen12_plane_has_mc_ccs(i915, plane_id))
> + caps |= INTEL_PLANE_CAP_CCS_MC;
> +
> + return caps;
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_plane_caps.h 
> b/drivers/gpu/drm/i915/display/intel_plane_caps.h
> new file mode 100644
> index ..60a941c76f23
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_plane_caps.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#ifndef __INTEL_PLANE_CAPS_H__
> +#define __INTEL_PLANE_CAPS_H__
> +
> +#include "intel_display_types.h"
> +
> +u8 skl_get_plane_caps(struct drm_i915_private *i915,
> +   enum pipe pipe, enum plane_id plane_id);
> +
> +#endif /* __INTEL_PLANE_CAPS_H__ */
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c 
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 511dc1544854..f2fd3833c61d 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -17,6 +17,7 @@
>  #include "intel_fb.h"
>  #include "intel_fbc.h"
>  #include "intel_frontbuffer.h"
> +#include "intel_plane_caps.h"
>  #include "intel_psr.h"
>  #include "intel_psr_regs

Re: ✗ Fi.CI.IGT: failure for Resolve suspend-resume racing with GuC destroy-context-worker (rev13)

[Matt Roper]

On Thu, Jan 04, 2024 at 05:39:16PM +, Teres Alexis, Alan Previn wrote:
> On Thu, 2024-01-04 at 10:57 +, Patchwork wrote:
> > Patch Details
> > Series: Resolve suspend-resume racing with GuC destroy-context-worker 
> > (rev13)
> > URL:https://patchwork.freedesktop.org/series/121916/
> > State:  failure
> > Details:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_121916v13/index.html
> > CI Bug Log - changes from CI_DRM_14076_full -> Patchwork_121916v13_full
> > Summary
> > 
> > FAILURE
> alan:snip
> 
> 
> > Here are the unknown changes that may have been introduced in 
> > Patchwork_121916v13_full:
> > 
> > IGT changes
> > Possible regressions
> > 
> >   *   igt@gem_eio@wait-wedge-immediate:
> >  *   shard-mtlp: 
> > PASS
> >  -> 
> > ABORT
> > 
> alan: from the code and dmesg, this is unrelated to guc context destruction 
> flows.
> Its reading an MCR register that times out. Additionally, i believe this 
> error is occuring during post-reset-init flows.
> So its definitely not doing any context destruction at this point (as reset 
> would have happenned sooner).

Yeah, the MCR timeouts are due to these CI machines running an outdated
IFWI, so they're missing an important workaround in the firmware.

Series applies to drm-intel-gt-next.  Thanks for the patches and
reviews.


Matt

> > Known issues
> > 
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Re: [PATCH 3/5] drm/ttm: replace busy placement with flags v5

[Zack Rusin]

On Tue, Jan 9, 2024 at 2:47 AM Christian König
 wrote:
>
> From: Somalapuram Amaranath 
>
> Instead of a list of separate busy placement add flags which indicate
> that a placement should only be used when there is room or if we need to
> evict.
>
> v2: add missing TTM_PL_FLAG_IDLE for i915
> v3: fix auto build test ERROR on drm-tip/drm-tip
> v4: fix some typos pointed out by checkpatch
> v5: cleanup some rebase problems with VMWGFX
>
> Signed-off-by: Christian König 
> Signed-off-by: Somalapuram Amaranath 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  6 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 11 +---
>  drivers/gpu/drm/drm_gem_vram_helper.c  |  2 -
>  drivers/gpu/drm/i915/gem/i915_gem_ttm.c| 37 +--
>  drivers/gpu/drm/loongson/lsdc_ttm.c|  2 -
>  drivers/gpu/drm/nouveau/nouveau_bo.c   | 59 +++--
>  drivers/gpu/drm/nouveau/nouveau_bo.h   |  1 -
>  drivers/gpu/drm/qxl/qxl_object.c   |  2 -
>  drivers/gpu/drm/qxl/qxl_ttm.c  |  2 -
>  drivers/gpu/drm/radeon/radeon_object.c |  2 -
>  drivers/gpu/drm/radeon/radeon_ttm.c|  8 +--
>  drivers/gpu/drm/radeon/radeon_uvd.c|  1 -
>  drivers/gpu/drm/ttm/ttm_bo.c   | 21 ---
>  drivers/gpu/drm/ttm/ttm_resource.c | 73 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c |  2 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  4 --
>  include/drm/ttm/ttm_placement.h| 10 +--
>  include/drm/ttm/ttm_resource.h |  8 +--
>  18 files changed, 79 insertions(+), 172 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index cef920a93924..aa0dd6dad068 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -220,9 +220,6 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo 
> *abo, u32 domain)
>
> placement->num_placement = c;
> placement->placement = places;
> -
> -   placement->num_busy_placement = c;
> -   placement->busy_placement = places;
>  }
>
>  /**
> @@ -1406,8 +1403,7 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct 
> ttm_buffer_object *bo)
> AMDGPU_GEM_DOMAIN_GTT);
>
> /* Avoid costly evictions; only set GTT as a busy placement */
> -   abo->placement.num_busy_placement = 1;
> -   abo->placement.busy_placement = &abo->placements[1];
> +   abo->placements[0].flags |= TTM_PL_FLAG_IDLE;
>
> r = ttm_bo_validate(bo, &abo->placement, &ctx);
> if (unlikely(r == -EBUSY || r == -ERESTARTSYS))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 05991c5c8ddb..9a6a00b1af40 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -102,23 +102,19 @@ static void amdgpu_evict_flags(struct ttm_buffer_object 
> *bo,
> /* Don't handle scatter gather BOs */
> if (bo->type == ttm_bo_type_sg) {
> placement->num_placement = 0;
> -   placement->num_busy_placement = 0;
> return;
> }
>
> /* Object isn't an AMDGPU object so ignore */
> if (!amdgpu_bo_is_amdgpu_bo(bo)) {
> placement->placement = &placements;
> -   placement->busy_placement = &placements;
> placement->num_placement = 1;
> -   placement->num_busy_placement = 1;
> return;
> }
>
> abo = ttm_to_amdgpu_bo(bo);
> if (abo->flags & AMDGPU_GEM_CREATE_DISCARDABLE) {
> placement->num_placement = 0;
> -   placement->num_busy_placement = 0;
> return;
> }
>
> @@ -128,13 +124,13 @@ static void amdgpu_evict_flags(struct ttm_buffer_object 
> *bo,
> case AMDGPU_PL_OA:
> case AMDGPU_PL_DOORBELL:
> placement->num_placement = 0;
> -   placement->num_busy_placement = 0;
> return;
>
> case TTM_PL_VRAM:
> if (!adev->mman.buffer_funcs_enabled) {
> /* Move to system memory */
> amdgpu_bo_placement_from_domain(abo, 
> AMDGPU_GEM_DOMAIN_CPU);
> +
> } else if (!amdgpu_gmc_vram_full_visible(&adev->gmc) &&
>!(abo->flags & 
> AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) &&
>amdgpu_bo_in_cpu_visible_vram(abo)) {
> @@ -149,8 +145,7 @@ static void amdgpu_evict_flags(struct ttm_buffer_object 
> *bo,
> 
> AMDGPU_GEM_DOMAIN_CPU);
> abo->placements[0].fpfn = adev->gmc.visible_vram_size 
> >> PAGE_SHIFT;
> abo->placements[0].lpfn = 0;
> -   abo->placement.busy_placement = &abo->placements[1];
> -   abo->placeme

Re: [PATCH 1/3] drm/nouveau: include drm/drm_edid.h only where needed

[Danilo Krummrich]

On 1/9/24 10:59, Jani Nikula wrote:

On Mon, 08 Jan 2024, Danilo Krummrich  wrote:

On 1/4/24 21:16, Jani Nikula wrote:

Including drm_edid.h from nouveau_connector.h causes the rebuild of 15
files when drm_edid.h is modified, while there are only a few files that
actually need to include drm_edid.h.

Cc: Karol Herbst 
Cc: Lyude Paul 
Cc: Danilo Krummrich 
Cc: nouv...@lists.freedesktop.org
Signed-off-by: Jani Nikula 


Reviewed-by: Danilo Krummrich 


Are you going to pick this up via the nouveau tree, or shall I apply it
to drm-misc-next?


We don't maintain a separate tree, hence feel free to apply it to drm-misc-next.

- Danilo



BR,
Jani.




---
   drivers/gpu/drm/nouveau/dispnv50/head.c | 1 +
   drivers/gpu/drm/nouveau/nouveau_connector.h | 2 +-
   2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c 
b/drivers/gpu/drm/nouveau/dispnv50/head.c
index 5f490fbf1877..83355dbc15ee 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -32,6 +32,7 @@
   
   #include 

   #include 
+#include 
   #include 
   #include "nouveau_connector.h"
   
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h b/drivers/gpu/drm/nouveau/nouveau_connector.h

index a2df4918340c..0608cabed058 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -35,7 +35,6 @@
   
   #include 

   #include 
-#include 
   #include 
   #include 
   
@@ -44,6 +43,7 @@
   
   struct nvkm_i2c_port;

   struct dcb_output;
+struct edid;
   
   #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT

   struct nouveau_backlight {

Re: [PATCH] drm/i915/guc: Avoid circular locking issue on busyness flush

[Daniel Vetter]

On Tue, Dec 19, 2023 at 11:59:57AM -0800, john.c.harri...@intel.com wrote:
> From: John Harrison 
> 
> Avoid the following lockdep complaint:
> <4> [298.856498] ==
> <4> [298.856500] WARNING: possible circular locking dependency detected
> <4> [298.856503] 6.7.0-rc5-CI_DRM_14017-g58ac4ffc75b6+ #1 Tainted: G
> N
> <4> [298.856505] --
> <4> [298.856507] kworker/4:1H/190 is trying to acquire lock:
> <4> [298.856509] 8881103e9978 (>->reset.backoff_srcu){}-{0:0}, at:
> _intel_gt_reset_lock+0x35/0x380 [i915]
> <4> [298.856661]
> but task is already holding lock:
> <4> [298.856663] c900013f7e58
> ((work_completion)(&(&guc->timestamp.work)->work)){+.+.}-{0:0}, at:
> process_scheduled_works+0x264/0x530
> <4> [298.856671]
> which lock already depends on the new lock.
> 
> The complaint is not actually valid. The busyness worker thread does
> indeed hold the worker lock and then attempt to acquire the reset lock
> (which may have happened in reverse order elsewhere). However, it does
> so with a trylock that exits if the reset lock is not available
> (specifically to prevent this and other similar deadlocks).
> Unfortunately, lockdep does not understand the trylock semantics (the
> lock is an i915 specific custom implementation for resets).
> 
> Not doing a synchronous flush of the worker thread when a reset is in
> progress resolves the lockdep splat by never even attempting to grab
> the lock in this particular scenario.
> 
> There are situatons where a synchronous cancel is required, however.
> So, always do the synchronous cancel if not in reset. And add an extra
> synchronous cancel to the end of the reset flow to account for when a
> reset is occurring at driver shutdown and the cancel is no longer
> synchronous but could lead to unallocated memory accesses if the
> worker is not stopped.
> 
> Signed-off-by: Zhanjun Dong 
> Signed-off-by: John Harrison 
> Cc: Andi Shyti 
> Cc: Daniel Vetter 
> ---
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 48 ++-
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c |  2 +-
>  2 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index a259f1118c5ab..0228a77d456ed 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1362,7 +1362,45 @@ static void guc_enable_busyness_worker(struct 
> intel_guc *guc)
>  
>  static void guc_cancel_busyness_worker(struct intel_guc *guc)
>  {
> - cancel_delayed_work_sync(&guc->timestamp.work);
> + /*
> +  * There are many different call stacks that can get here. Some of them
> +  * hold the reset mutex. The busyness worker also attempts to acquire 
> the
> +  * reset mutex. Synchronously flushing a worker thread requires 
> acquiring
> +  * the worker mutex. Lockdep sees this as a conflict. It thinks that the
> +  * flush can deadlock because it holds the worker mutex while waiting 
> for
> +  * the reset mutex, but another thread is holding the reset mutex and 
> might
> +  * attempt to use other worker functions.
> +  *
> +  * In practice, this scenario does not exist because the busyness worker
> +  * does not block waiting for the reset mutex. It does a try-lock on it 
> and
> +  * immediately exits if the lock is already held. Unfortunately, the 
> mutex
> +  * in question (I915_RESET_BACKOFF) is an i915 implementation which has 
> lockdep
> +  * annotation but not to the extent of explaining the 'might lock' is 
> also a
> +  * 'does not need to lock'. So one option would be to add more complex 
> lockdep
> +  * annotations to ignore the issue (if at all possible). A simpler 
> option is to
> +  * just not flush synchronously when a rest in progress. Given that the 
> worker
> +  * will just early exit and re-schedule itself anyway, there is no 
> advantage
> +  * to running it immediately.
> +  *
> +  * If a reset is not in progress, then the synchronous flush may be 
> required.
> +  * As noted many call stacks lead here, some during suspend and driver 
> unload
> +  * which do require a synchronous flush to make sure the worker is 
> stopped
> +  * before memory is freed.
> +  *
> +  * Trying to pass a 'need_sync' or 'in_reset' flag all the way down 
> through
> +  * every possible call stack is unfeasible. It would be too intrusive 
> to many
> +  * areas that really don't care about the GuC backend. However, there 
> is the
> +  * 'reset_in_progress' flag available, so just use that.
> +  *
> +  * And note that in the case of a reset occurring during driver unload
> +  * (wedge_on_fini), skipping the cancel in _prepare (when the reset 
> flag is set
> +  * is fine because the

✓ Fi.CI.BAT: success for Allow PSR mode changes without full modeset

[Patchwork]

== Series Details ==

Series: Allow PSR mode changes without full modeset
URL   : https://patchwork.freedesktop.org/series/128360/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_14100 -> Patchwork_128360v1


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/index.html

Participating hosts (38 -> 21)
--

  Additional (1): bat-dg2-9 
  Missing(18): bat-mtlp-8 bat-dg1-7 bat-kbl-2 bat-adlp-9 bat-dg2-8 
fi-skl-guc fi-tgl-1115g4 fi-snb-2520m bat-adlp-6 fi-kbl-guc fi-glk-j4005 
fi-kbl-x1275 fi-ivb-3770 fi-elk-e7500 bat-rplp-1 fi-blb-e6850 bat-jsl-1 
fi-skl-6600u 

Known issues


  Here are the changes found in Patchwork_128360v1 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@gem_mmap@basic:
- bat-dg2-9:  NOTRUN -> [SKIP][1] ([i915#4083])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@gem_m...@basic.html

  * igt@gem_mmap_gtt@basic:
- bat-dg2-9:  NOTRUN -> [SKIP][2] ([i915#4077]) +2 other tests skip
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@gem_mmap_...@basic.html

  * igt@gem_render_tiled_blits@basic:
- bat-dg2-9:  NOTRUN -> [SKIP][3] ([i915#4079]) +1 other test skip
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@gem_render_tiled_bl...@basic.html

  * igt@i915_pm_rps@basic-api:
- bat-dg2-9:  NOTRUN -> [SKIP][4] ([i915#6621])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@i915_pm_...@basic-api.html

  * igt@i915_suspend@basic-s3-without-i915:
- bat-atsm-1: NOTRUN -> [SKIP][5] ([i915#6645])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-atsm-1/igt@i915_susp...@basic-s3-without-i915.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
- bat-dg2-9:  NOTRUN -> [SKIP][6] ([i915#5190])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
- bat-dg2-9:  NOTRUN -> [SKIP][7] ([i915#4215] / [i915#5190])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@kms_addfb_ba...@basic-y-tiled-legacy.html

  * igt@kms_addfb_basic@framebuffer-vs-set-tiling:
- bat-dg2-9:  NOTRUN -> [SKIP][8] ([i915#4212]) +7 other tests skip
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@kms_addfb_ba...@framebuffer-vs-set-tiling.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- bat-dg2-9:  NOTRUN -> [SKIP][9] ([i915#4103] / [i915#4213]) +1 
other test skip
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_force_connector_basic@force-load-detect:
- bat-dg2-9:  NOTRUN -> [SKIP][10] ([fdo#109285])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@kms_force_connector_ba...@force-load-detect.html

  * igt@kms_force_connector_basic@prune-stale-modes:
- bat-dg2-9:  NOTRUN -> [SKIP][11] ([i915#5274])
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@kms_force_connector_ba...@prune-stale-modes.html

  * igt@kms_pipe_crc_basic@suspend-read-crc:
- bat-atsm-1: NOTRUN -> [SKIP][12] ([i915#1836])
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-atsm-1/igt@kms_pipe_crc_ba...@suspend-read-crc.html

  * igt@kms_pm_backlight@basic-brightness:
- bat-dg2-9:  NOTRUN -> [SKIP][13] ([i915#5354])
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@kms_pm_backli...@basic-brightness.html

  * igt@kms_setmode@basic-clone-single-crtc:
- bat-dg2-9:  NOTRUN -> [SKIP][14] ([i915#3555])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@kms_setm...@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
- bat-dg2-9:  NOTRUN -> [SKIP][15] ([i915#3708])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@prime_v...@basic-fence-flip.html

  * igt@prime_vgem@basic-fence-mmap:
- bat-dg2-9:  NOTRUN -> [SKIP][16] ([i915#3708] / [i915#4077]) +1 
other test skip
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@prime_v...@basic-fence-mmap.html

  * igt@prime_vgem@basic-write:
- bat-dg2-9:  NOTRUN -> [SKIP][17] ([i915#3291] / [i915#3708]) +2 
other tests skip
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128360v1/bat-dg2-9/igt@prime_v...@basic-write.html

  
 Possible fixes 

  * igt@i915_selftest@live@hangcheck:
- 

✗ Fi.CI.CHECKPATCH: warning for Allow PSR mode changes without full modeset

[Patchwork]

== Series Details ==

Series: Allow PSR mode changes without full modeset
URL   : https://patchwork.freedesktop.org/series/128360/
State : warning

== Summary ==

Error: dim checkpatch failed
6dc8ca5bdd65 drm/i915/display: No need for full modeset due to psr
d034710b2ab1 drm/i915/psr: CAN_PSR and CAN_PANEL_REPLAY can be now local defines
-:23: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'intel_dp' - possible 
side-effects?
#23: FILE: drivers/gpu/drm/i915/display/intel_psr.c:176:
+#define CAN_PSR(intel_dp) ((intel_dp)->psr.sink_support && \
+  (intel_dp)->psr.source_support)

-:26: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'intel_dp' - possible 
side-effects?
#26: FILE: drivers/gpu/drm/i915/display/intel_psr.c:179:
+#define CAN_PANEL_REPLAY(intel_dp) ((intel_dp)->psr.sink_panel_replay_support 
&& \
+   (intel_dp)->psr.source_panel_replay_support)

total: 0 errors, 0 warnings, 2 checks, 24 lines checked

RE: [PATCH 7/7] drm/i915/psr: Disable early transport by default

[Kahola, Mika]

> -Original Message-
> From: Intel-gfx  On Behalf Of Jouni 
> Högander
> Sent: Monday, December 18, 2023 7:50 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [PATCH 7/7] drm/i915/psr: Disable early transport by default
> 
> Early transport validation is currently incomplete. Due to this disable the 
> feature by default.
> 

Reviewed-by: Mika Kahola 

> Signed-off-by: Jouni Högander 
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c 
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index c29fd708608a..7835afee4283 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2851,6 +2851,9 @@ void intel_psr_init(struct intel_dp *intel_dp)
>   else
>   intel_dp->psr.source_support = true;
> 
> + /* Disable early transport for now */
> + intel_dp->psr.debug |= I915_PSR_DEBUG_SU_REGION_ET_DISABLE;
> +
>   /* Set link_standby x link_off defaults */
>   if (DISPLAY_VER(dev_priv) < 12)
>   /* For new platforms up to TGL let's respect VBT back again */
> --
> 2.34.1

RE: [PATCH 6/7] drm/i915/psr: Enable psr2 early transport as possible

[Kahola, Mika]

> -Original Message-
> From: Intel-gfx  On Behalf Of Jouni 
> Högander
> Sent: Monday, December 18, 2023 7:50 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [PATCH 6/7] drm/i915/psr: Enable psr2 early transport as possible
> 
> Check source and sink support for psr2 early transport and enable it if not 
> disabled by debug flag.
> 
> Bspec: 68934
> 

Reviewed-by: Mika Kahola 

> Signed-off-by: Jouni Högander 
> ---
>  .../drm/i915/display/intel_display_types.h| 16 --
>  drivers/gpu/drm/i915/display/intel_psr.c  | 22 ++-
>  drivers/gpu/drm/i915/display/intel_psr_regs.h |  1 +
>  3 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 05dd8decd610..ca8bc909ea35 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1215,6 +1215,7 @@ struct intel_crtc_state {
>   bool has_psr;
>   bool has_psr2;
>   bool enable_psr2_sel_fetch;
> + bool enable_psr2_su_region_et;
>   bool req_psr2_sdp_prior_scanline;
>   bool has_panel_replay;
>   bool wm_level_disabled;
> @@ -1686,13 +1687,14 @@ struct intel_psr {
>   /* Mutex for PSR state of the transcoder */
>   struct mutex lock;
> 
> -#define I915_PSR_DEBUG_MODE_MASK 0x0f
> -#define I915_PSR_DEBUG_DEFAULT   0x00
> -#define I915_PSR_DEBUG_DISABLE   0x01
> -#define I915_PSR_DEBUG_ENABLE0x02
> -#define I915_PSR_DEBUG_FORCE_PSR10x03
> -#define I915_PSR_DEBUG_ENABLE_SEL_FETCH  0x4
> -#define I915_PSR_DEBUG_IRQ   0x10
> +#define I915_PSR_DEBUG_MODE_MASK 0x0f
> +#define I915_PSR_DEBUG_DEFAULT   0x00
> +#define I915_PSR_DEBUG_DISABLE   0x01
> +#define I915_PSR_DEBUG_ENABLE0x02
> +#define I915_PSR_DEBUG_FORCE_PSR10x03
> +#define I915_PSR_DEBUG_ENABLE_SEL_FETCH  0x4
> +#define I915_PSR_DEBUG_IRQ   0x10
> +#define I915_PSR_DEBUG_SU_REGION_ET_DISABLE  0x20
> 
>   u32 debug;
>   bool sink_support;
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c 
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 67f68c26a312..c29fd708608a 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -528,7 +528,7 @@ static void _psr_init_dpcd(struct intel_dp *intel_dp)
>   intel_dp_get_sink_sync_latency(intel_dp);
> 
>   if (DISPLAY_VER(i915) >= 9 &&
> - intel_dp->psr_dpcd[0] == DP_PSR2_WITH_Y_COORD_IS_SUPPORTED) {
> + intel_dp->psr_dpcd[0] >= DP_PSR2_WITH_Y_COORD_IS_SUPPORTED) {
>   bool y_req = intel_dp->psr_dpcd[1] &
>DP_PSR2_SU_Y_COORDINATE_REQUIRED;
>   bool alpm = intel_dp_get_alpm_status(intel_dp);
> @@ -604,6 +604,18 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
>  aux_ctl);
>  }
> 
> +static bool psr2_su_region_et_valid(struct intel_dp *intel_dp) {
> + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +
> + if (DISPLAY_VER(i915) >= 20 &&
> + intel_dp->psr_dpcd[0] == DP_PSR2_WITH_Y_COORD_ET_SUPPORTED &&
> + !(intel_dp->psr.debug & I915_PSR_DEBUG_SU_REGION_ET_DISABLE))
> + return true;
> +
> + return false;
> +}
> +
>  static void intel_psr_enable_sink(struct intel_dp *intel_dp)  {
>   struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@ -619,6 
> +631,8 @@ static void
> intel_psr_enable_sink(struct intel_dp *intel_dp)
>  DP_ALPM_LOCK_ERROR_IRQ_HPD_ENABLE);
> 
>   dpcd_val |= DP_PSR_ENABLE_PSR2 | DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
> + if (psr2_su_region_et_valid(intel_dp))
> + dpcd_val |= DP_PSR_ENABLE_SU_REGION_ET;
>   } else {
>   if (intel_dp->psr.link_standby)
>   dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE; @@ -869,6 +883,9 
> @@ static void hsw_activate_psr2(struct
> intel_dp *intel_dp)
>   intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(cpu_transcoder), 0);
>   }
> 
> + if (psr2_su_region_et_valid(intel_dp))
> + val |= LNL_EDP_PSR2_SU_REGION_ET_ENABLE;
> +
>   /*
>* PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and BSpec is
>* recommending keep this bit unset while PSR2 is enabled.
> @@ -1031,6 +1048,9 @@ static bool intel_psr2_sel_fetch_config_valid(struct 
> intel_dp *intel_dp,
>   return false;
>   }
> 
> + if (psr2_su_region_et_valid(intel_dp))
> + crtc_state->enable_psr2_su_region_et = true;
> +
>   return crtc_state->enable_psr2_sel_fetch = true;  }
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr_regs.h 
> b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> index ceefcc70e8f9..bc252f38239e 100644
> -

RE: [PATCH 5/7] drm/i915/psr: Configure PIPE_SRCSZ_ERLY_TPT for psr2 early transport

[Kahola, Mika]

> -Original Message-
> From: Intel-gfx  On Behalf Of Jouni 
> Högander
> Sent: Monday, December 18, 2023 7:50 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [PATCH 5/7] drm/i915/psr: Configure PIPE_SRCSZ_ERLY_TPT for psr2 
> early transport
> 
> There is a new register used to configure selective update area size for 
> early transport.
> 
> Configure PIPE_SRCSZ_ERLY_TPT using calculated selective update area carried 
> in crtc_state->su_area.
> 
> Bspec: 68927
> 

Reviewed-by: Mika Kahola 

> Signed-off-by: Jouni Högander 
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  | 10 ++  
> drivers/gpu/drm/i915/display/intel_psr_regs.h |  5 +
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> b/drivers/gpu/drm/i915/display/intel_display.c
> index f1e58163277d..8d25020a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -104,6 +104,7 @@
>  #include "intel_pmdemand.h"
>  #include "intel_pps.h"
>  #include "intel_psr.h"
> +#include "intel_psr_regs.h"
>  #include "intel_sdvo.h"
>  #include "intel_snps_phy.h"
>  #include "intel_tc.h"
> @@ -2706,6 +2707,15 @@ static void intel_set_pipe_src_size(const struct 
> intel_crtc_state *crtc_state)
>*/
>   intel_de_write(dev_priv, PIPESRC(pipe),
>  PIPESRC_WIDTH(width - 1) | PIPESRC_HEIGHT(height - 1));
> +
> + if (!crtc_state->enable_psr2_su_region_et)
> + return;
> +
> + width = drm_rect_width(&crtc_state->psr2_su_area);
> + height = drm_rect_height(&crtc_state->psr2_su_area);
> +
> + intel_de_write(dev_priv, PIPE_SRCSZ_ERLY_TPT(pipe),
> +PIPESRC_WIDTH(width - 1) | PIPESRC_HEIGHT(height - 1));
>  }
> 
>  static bool intel_pipe_is_interlaced(const struct intel_crtc_state 
> *crtc_state) diff --git
> a/drivers/gpu/drm/i915/display/intel_psr_regs.h 
> b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> index efe4306b37e0..ceefcc70e8f9 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> @@ -245,6 +245,11 @@
>  #define  ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME  REG_BIT(14)
>  #define  ADLP_PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME   
> REG_BIT(13)
> 
> +/* PSR2 Early transport */
> +#define _PIPE_SRCSZ_ERLY_TPT_A   0x70074
> +
> +#define PIPE_SRCSZ_ERLY_TPT(trans)   _MMIO_TRANS2(trans, 
> _PIPE_SRCSZ_ERLY_TPT_A)
> +
>  #define _SEL_FETCH_PLANE_BASE_1_A0x70890
>  #define _SEL_FETCH_PLANE_BASE_2_A0x708B0
>  #define _SEL_FETCH_PLANE_BASE_3_A0x708D0
> --
> 2.34.1

✓ Fi.CI.BAT: success for drm/i915: Cursor vblank evasion (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Cursor vblank evasion (rev2)
URL   : https://patchwork.freedesktop.org/series/127744/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_14100 -> Patchwork_127744v2


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/index.html

Participating hosts (38 -> 39)
--

  Additional (2): bat-rpls-2 bat-dg2-9 
  Missing(1): fi-snb-2520m 

Known issues


  Here are the changes found in Patchwork_127744v2 that come from known issues:

### CI changes ###

 Issues hit 

  * boot:
- fi-bsw-n3050:   [PASS][1] -> [FAIL][2] ([i915#8293])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14100/fi-bsw-n3050/boot.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/fi-bsw-n3050/boot.html

  
 Possible fixes 

  * boot:
- bat-jsl-1:  [FAIL][3] ([i915#8293]) -> [PASS][4]
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14100/bat-jsl-1/boot.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-jsl-1/boot.html

  

### IGT changes ###

 Issues hit 

  * igt@debugfs_test@basic-hwmon:
- bat-rpls-2: NOTRUN -> [SKIP][5] ([i915#9318])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-rpls-2/igt@debugfs_t...@basic-hwmon.html
- bat-jsl-1:  NOTRUN -> [SKIP][6] ([i915#9318])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-jsl-1/igt@debugfs_t...@basic-hwmon.html

  * igt@gem_exec_suspend@basic-s3@smem:
- bat-dg2-8:  [PASS][7] -> [INCOMPLETE][8] ([i915#9275])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14100/bat-dg2-8/igt@gem_exec_suspend@basic...@smem.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-dg2-8/igt@gem_exec_suspend@basic...@smem.html

  * igt@gem_huc_copy@huc-copy:
- bat-jsl-1:  NOTRUN -> [SKIP][9] ([i915#2190])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-jsl-1/igt@gem_huc_c...@huc-copy.html

  * igt@gem_lmem_swapping@verify-random:
- bat-jsl-1:  NOTRUN -> [SKIP][10] ([i915#4613]) +3 other tests skip
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-jsl-1/igt@gem_lmem_swapp...@verify-random.html

  * igt@gem_mmap@basic:
- bat-dg2-9:  NOTRUN -> [SKIP][11] ([i915#4083])
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-dg2-9/igt@gem_m...@basic.html

  * igt@gem_mmap_gtt@basic:
- bat-dg2-9:  NOTRUN -> [SKIP][12] ([i915#4077]) +2 other tests skip
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-dg2-9/igt@gem_mmap_...@basic.html

  * igt@gem_render_tiled_blits@basic:
- bat-dg2-9:  NOTRUN -> [SKIP][13] ([i915#4079]) +1 other test skip
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-dg2-9/igt@gem_render_tiled_bl...@basic.html

  * igt@gem_tiled_pread_basic:
- bat-rpls-2: NOTRUN -> [SKIP][14] ([i915#3282])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-rpls-2/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_rps@basic-api:
- bat-dg2-9:  NOTRUN -> [SKIP][15] ([i915#6621])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-dg2-9/igt@i915_pm_...@basic-api.html

  * igt@i915_suspend@basic-s3-without-i915:
- bat-atsm-1: NOTRUN -> [SKIP][16] ([i915#6645])
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-atsm-1/igt@i915_susp...@basic-s3-without-i915.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
- bat-dg2-9:  NOTRUN -> [SKIP][17] ([i915#5190])
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-dg2-9/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
- bat-dg2-9:  NOTRUN -> [SKIP][18] ([i915#4215] / [i915#5190])
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-dg2-9/igt@kms_addfb_ba...@basic-y-tiled-legacy.html

  * igt@kms_addfb_basic@framebuffer-vs-set-tiling:
- bat-dg2-9:  NOTRUN -> [SKIP][19] ([i915#4212]) +7 other tests skip
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-dg2-9/igt@kms_addfb_ba...@framebuffer-vs-set-tiling.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- bat-dg2-9:  NOTRUN -> [SKIP][20] ([i915#4103] / [i915#4213]) +1 
other test skip
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-dg2-9/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html
- bat-rpls-2: NOTRUN -> [SKIP][21] ([i915#4103]) +1 other test skip
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127744v2/bat-rpls-2/igt@kms_cursor_leg...@basic-busy-flip-b

✗ Fi.CI.SPARSE: warning for drm/i915: Cursor vblank evasion (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Cursor vblank evasion (rev2)
URL   : https://patchwork.freedesktop.org/series/127744/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit'
+

RE: [PATCH 4/7] drm/i915/psr: Calculate and configure CUR_POS_ERLY_TPT

[Kahola, Mika]

> -Original Message-
> From: Intel-gfx  On Behalf Of Jouni 
> Högander
> Sent: Monday, December 18, 2023 7:50 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [PATCH 4/7] drm/i915/psr: Calculate and configure CUR_POS_ERLY_TPT
> 
> New register CUR_POS_ERLY_TPT related to early transport is supposed to be 
> configured when early transport is in use.
> 
> This register is used to configure cursor vertical postion from beginning of 
> selective update area.
> 
> Bspec: 68927
> 

Reviewed-by: Mika Kahola 

> Signed-off-by: Jouni Högander 
> ---
>  drivers/gpu/drm/i915/display/intel_cursor.c | 32 -
>  drivers/gpu/drm/i915/i915_reg.h |  2 ++
>  2 files changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c 
> b/drivers/gpu/drm/i915/display/intel_cursor.c
> index 926e2de00eb5..ecff90e233f0 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -47,12 +47,23 @@ static u32 intel_cursor_base(const struct 
> intel_plane_state *plane_state)
>   return base + plane_state->view.color_plane[0].offset;
>  }
> 
> -static u32 intel_cursor_position(const struct intel_plane_state *plane_state)
> +static u32 intel_cursor_position(const struct intel_crtc_state *crtc_state,
> +  const struct intel_plane_state *plane_state,
> +  bool early_tpt)
>  {
>   int x = plane_state->uapi.dst.x1;
>   int y = plane_state->uapi.dst.y1;
>   u32 pos = 0;
> 
> + /*
> +  * Formula from Bspec:
> +  * MAX(-1 *  +  * select setting> + 1, CUR_POS Y Position - Update region Y position
> +  */
> + if (early_tpt)
> + y = max(-1 * drm_rect_height(&plane_state->uapi.dst) + 1,
> + y - crtc_state->psr2_su_area.y1);
> +
>   if (x < 0) {
>   pos |= CURSOR_POS_X_SIGN;
>   x = -x;
> @@ -274,7 +285,7 @@ static void i845_cursor_update_arm(struct intel_plane 
> *plane,
>   size = CURSOR_HEIGHT(height) | CURSOR_WIDTH(width);
> 
>   base = intel_cursor_base(plane_state);
> - pos = intel_cursor_position(plane_state);
> + pos = intel_cursor_position(crtc_state, plane_state, false);
>   }
> 
>   /* On these chipsets we can only modify the base/size/stride @@ -503,17 
> +514,24 @@ static void
> i9xx_cursor_update_sel_fetch_arm(struct intel_plane *plane,
>const struct intel_crtc_state 
> *crtc_state,
>const struct intel_plane_state 
> *plane_state)  {
> - struct drm_i915_private *i915 = to_i915(plane->base.dev);
> + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>   enum pipe pipe = plane->pipe;
> 
>   if (!crtc_state->enable_psr2_sel_fetch)
>   return;
> 
> - if (drm_rect_height(&plane_state->psr2_sel_fetch_area) > 0)
> - intel_de_write_fw(i915, PLANE_SEL_FETCH_CTL(pipe, plane->id),
> + if (drm_rect_height(&plane_state->psr2_sel_fetch_area) > 0) {
> + if (crtc_state->enable_psr2_su_region_et) {
> + u32 val = intel_cursor_position(crtc_state, plane_state,
> + true);
> + intel_de_write_fw(dev_priv, CURPOS_ERLY_TPT(pipe), val);
> + }
> +
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, 
> plane->id),
> plane_state->ctl);
> - else
> + } else {
>   i9xx_cursor_disable_sel_fetch_arm(plane, crtc_state);
> + }
>  }
> 
>  /* TODO: split into noarm+arm pair */
> @@ -536,7 +554,7 @@ static void i9xx_cursor_update_arm(struct intel_plane 
> *plane,
>   fbc_ctl = CUR_FBC_EN | CUR_FBC_HEIGHT(height - 1);
> 
>   base = intel_cursor_base(plane_state);
> - pos = intel_cursor_position(plane_state);
> + pos = intel_cursor_position(crtc_state, plane_state, false);
>   }
> 
>   /*
> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> b/drivers/gpu/drm/i915/i915_reg.h index 27dc903f0553..d6f7e68ba308 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3059,6 +3059,7 @@
>  #define   MCURSOR_MODE_64_ARGB_AX(0x20 | MCURSOR_MODE_64_32B_AX)
>  #define _CURABASE0x70084
>  #define _CURAPOS 0x70088
> +#define _CURAPOS_ERLY_TPT0x7008c
>  #define   CURSOR_POS_Y_SIGN  REG_BIT(31)
>  #define   CURSOR_POS_Y_MASK  REG_GENMASK(30, 16)
>  #define   CURSOR_POS_Y(y)REG_FIELD_PREP(CURSOR_POS_Y_MASK, (y))
> @@ -3087,6 +3088,7 @@
>  #define CURCNTR(pipe) _MMIO_CURSOR2(pipe, _CURACNTR)  #define CURBASE(pipe) 
> _MMIO_CURSOR2(pipe, _CURABASE)
> #define CURPOS(pipe) _MMIO_CURSOR2(pipe, _CURAPOS)
> +#define CURPOS_ERLY_TPT(pipe) _MMIO_CURSOR2(pipe, _CURAPOS_ERLY_TPT)
>  #define CURSIZE

Re: [PATCH 9/9] Revert "drm/i915/xe2lpd: Treat cursor plane as regular plane for DDB allocation"

[Lisovskiy, Stanislav]

On Fri, Dec 15, 2023 at 01:15:16PM +0200, Ville Syrjälä wrote:
> On Wed, Dec 13, 2023 at 05:29:12PM +0200, Ville Syrjälä wrote:
> > On Wed, Dec 13, 2023 at 05:15:06PM +0200, Ville Syrjälä wrote:
> > > On Wed, Dec 13, 2023 at 01:28:15PM +0200, Lisovskiy, Stanislav wrote:
> > > > On Wed, Dec 13, 2023 at 12:25:19PM +0200, Ville Syrjala wrote:
> > > > > From: Ville Syrjälä 
> > > > > 
> > > > > This reverts commit cfeff354f70bb1d0deb0279506e3f7989bc16e28.
> > > > > 
> > > > > A core design consideration with legacy cursor updates is that the
> > > > > cursor must not touch any other plane, even if we were to force it
> > > > > to take the slow path. That is the real reason why the cursor uses
> > > > > a fixed ddb allocation, not because bspec says so.
> > > > > 
> > > > > Treating cursors as any other plane during ddb allocation
> > > > > violates that, which means we can now pull other planes into
> > > > > fully unsynced legacy cursor mailbox commits. That is
> > > > > definitely not something we've ever considered when designing
> > > > > the rest of the code. The noarm+arm register write split in
> > > > > particular makes that dangerous as previous updates can get
> > > > > disarmed pretty much at any random time, and not necessarily
> > > > > in an order that is actually safe (eg. against ddb overlaps).
> > > > > 
> > > > > So if we were to do this then:
> > > > > - someone needs to expend the appropriate amount of brain
> > > > >   cells thinking through all the tricky details
> > > > 
> > > > So question is how can we avoid pulling other planes to the commit?..
> > > 
> > > By preallocating the ddb, as we do already.
> > 
> > I guess one thing we could consider is allcating the cursor ddb
> > based on the cursors real size (as opposed to always allocating for
> > 256x256 cursor), and not doing a mailbox update when changing size.
> > But as we learn in https://gitlab.freedesktop.org/drm/intel/-/issues/7687:
> > - current userspace always uses 256x256 until the PLANE_SIZE_HINTS
> >   stuff (or something similar) lands, so there is no point
> > - it would lead to worse power consumption with smaller cursors
> >   as there isn't enough extra ddb. The fact that we currently 
> >   allocate the minimum for 256x256 means smallers cursor sizes
> >   are more efficient. Some tests I did also indicated that the
> >   current code does not give optimal values even if we let it
> >   fully calculate the extra ddb like in the reverted commit here.
> >   We really need someone to take a proper look at how to tune
> >   the ddb allocation for optimal power consumption...
> 
> Oh, and another random idea I keep having occasionally would
> be to by default assume that legacy cursor uapi wouldn't be
> used, and then massage stuff sufficiently when the first use
> appears to make it work well from that point onwards. That 
> way we could try to be a bit more optimal with ddb/other
> stuff for userspace that never uses the legacy cursor uapi.
> But haven't really thought through the details on this one.

Reviewed-by: Stanislav Lisovskiy 

> 
> -- 
> Ville Syrjälä
> Intel

[PATCH 1/2] drm/i915/display: No need for full modeset due to psr

[Jouni Högander]

There is no specific reason to force full modeset if psr is enabled.

Signed-off-by: Jouni Högander 
Tested-by: Paz Zcharya 
---
 drivers/gpu/drm/i915/display/intel_display.c | 7 ---
 drivers/gpu/drm/i915/display/intel_dp.c  | 7 ---
 2 files changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 31a6a82c1261..0cccf6df6718 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5202,13 +5202,6 @@ intel_pipe_config_compare(const struct intel_crtc_state 
*current_config,
 
PIPE_CONF_CHECK_CSC(csc);
PIPE_CONF_CHECK_CSC(output_csc);
-
-   if (current_config->active_planes) {
-   PIPE_CONF_CHECK_BOOL(has_psr);
-   PIPE_CONF_CHECK_BOOL(has_psr2);
-   PIPE_CONF_CHECK_BOOL(enable_psr2_sel_fetch);
-   PIPE_CONF_CHECK_I(dc3co_exitline);
-   }
}
 
PIPE_CONF_CHECK_BOOL(double_wide);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 7e4b7d5606d4..ab415f41924d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -3326,13 +3326,6 @@ bool intel_dp_initial_fastset_check(struct intel_encoder 
*encoder,
fastset = false;
}
 
-   if (CAN_PSR(intel_dp)) {
-   drm_dbg_kms(&i915->drm, "[ENCODER:%d:%s] Forcing full modeset 
to compute PSR state\n",
-   encoder->base.base.id, encoder->base.name);
-   crtc_state->uapi.mode_changed = true;
-   fastset = false;
-   }
-
return fastset;
 }
 
-- 
2.34.1

[PATCH 2/2] drm/i915/psr: CAN_PSR and CAN_PANEL_REPLAY can be now local defines

[Jouni Högander]

CAN_PSR and CAN_PANEL_REPLAY are not used outside intel_psr.c anymore. Make
them as intel_psr.c local defines.

Signed-off-by: Jouni Högander 
---
 drivers/gpu/drm/i915/display/intel_psr.c | 6 ++
 drivers/gpu/drm/i915/display/intel_psr.h | 6 --
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c 
b/drivers/gpu/drm/i915/display/intel_psr.c
index 54120b45958b..34c0a5a14455 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -173,6 +173,12 @@
  * irrelevant for normal operation.
  */
 
+#define CAN_PSR(intel_dp) ((intel_dp)->psr.sink_support && \
+  (intel_dp)->psr.source_support)
+
+#define CAN_PANEL_REPLAY(intel_dp) ((intel_dp)->psr.sink_panel_replay_support 
&& \
+   (intel_dp)->psr.source_panel_replay_support)
+
 bool intel_encoder_can_psr(struct intel_encoder *encoder)
 {
if (intel_encoder_is_dp(encoder) || encoder->type == 
INTEL_OUTPUT_DP_MST)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.h 
b/drivers/gpu/drm/i915/display/intel_psr.h
index 143e0595c097..cde781df84d5 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.h
+++ b/drivers/gpu/drm/i915/display/intel_psr.h
@@ -21,12 +21,6 @@ struct intel_encoder;
 struct intel_plane;
 struct intel_plane_state;
 
-#define CAN_PSR(intel_dp) ((intel_dp)->psr.sink_support && \
-  (intel_dp)->psr.source_support)
-
-#define CAN_PANEL_REPLAY(intel_dp) ((intel_dp)->psr.sink_panel_replay_support 
&& \
-   (intel_dp)->psr.source_panel_replay_support)
-
 bool intel_encoder_can_psr(struct intel_encoder *encoder);
 void intel_psr_init_dpcd(struct intel_dp *intel_dp);
 void intel_psr_pre_plane_update(struct intel_atomic_state *state,
-- 
2.34.1

[PATCH 0/2] Allow PSR mode changes without full modeset

[Jouni Högander]

Currently PSR mode changes are triggering full modeset. This patch set
allows changes to PSR mode using fast modeset.

Jouni Högander (2):
  drm/i915/display: No need for full modeset due to psr
  drm/i915/psr: CAN_PSR and CAN_PANEL_REPLAY can be now local defines

 drivers/gpu/drm/i915/display/intel_display.c | 7 ---
 drivers/gpu/drm/i915/display/intel_dp.c  | 7 ---
 drivers/gpu/drm/i915/display/intel_psr.c | 6 ++
 drivers/gpu/drm/i915/display/intel_psr.h | 6 --
 4 files changed, 6 insertions(+), 20 deletions(-)

-- 
2.34.1

Re: [PATCH 1/3] drm/nouveau: include drm/drm_edid.h only where needed

[Jani Nikula]

On Mon, 08 Jan 2024, Danilo Krummrich  wrote:
> On 1/4/24 21:16, Jani Nikula wrote:
>> Including drm_edid.h from nouveau_connector.h causes the rebuild of 15
>> files when drm_edid.h is modified, while there are only a few files that
>> actually need to include drm_edid.h.
>> 
>> Cc: Karol Herbst 
>> Cc: Lyude Paul 
>> Cc: Danilo Krummrich 
>> Cc: nouv...@lists.freedesktop.org
>> Signed-off-by: Jani Nikula 
>
> Reviewed-by: Danilo Krummrich 

Are you going to pick this up via the nouveau tree, or shall I apply it
to drm-misc-next?

BR,
Jani.

>
>> ---
>>   drivers/gpu/drm/nouveau/dispnv50/head.c | 1 +
>>   drivers/gpu/drm/nouveau/nouveau_connector.h | 2 +-
>>   2 files changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c 
>> b/drivers/gpu/drm/nouveau/dispnv50/head.c
>> index 5f490fbf1877..83355dbc15ee 100644
>> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
>> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
>> @@ -32,6 +32,7 @@
>>   
>>   #include 
>>   #include 
>> +#include 
>>   #include 
>>   #include "nouveau_connector.h"
>>   
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h 
>> b/drivers/gpu/drm/nouveau/nouveau_connector.h
>> index a2df4918340c..0608cabed058 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_connector.h
>> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
>> @@ -35,7 +35,6 @@
>>   
>>   #include 
>>   #include 
>> -#include 
>>   #include 
>>   #include 
>>   
>> @@ -44,6 +43,7 @@
>>   
>>   struct nvkm_i2c_port;
>>   struct dcb_output;
>> +struct edid;
>>   
>>   #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
>>   struct nouveau_backlight {
>

-- 
Jani Nikula, Intel

RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Jani Nikula]

On Tue, 09 Jan 2024, Jani Nikula  wrote:
> On Tue, 09 Jan 2024, "Murthy, Arun R"  wrote:
>>> -Original Message-
>>> From: Nikula, Jani 
>>> Sent: Monday, January 8, 2024 7:01 PM
>>> To: Murthy, Arun R ; 
>>> intel-gfx@lists.freedesktop.org;
>>> Deak, Imre 
>>> Cc: Murthy, Arun R 
>>> Subject: Re: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
>>>
>>> On Wed, 03 Jan 2024, Arun R Murthy  wrote:
>>> > With a value of '0' read from MSTM_CAP register MST to be enabled.
>>> > DP2.1 SCR updates the spec for 128/132b DP capable supporting only one
>>> > stream and not supporting single stream sideband MSG.
>>> > The underlying protocol will be MST to enable use of MTP.
>>> >
>>> > Signed-off-by: Arun R Murthy 
>>> > ---
>>> >  drivers/gpu/drm/i915/display/intel_dp.c | 9 +++--
>>> >  1 file changed, 7 insertions(+), 2 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>>> > b/drivers/gpu/drm/i915/display/intel_dp.c
>>> > index 9ff0cbd9c0df..40d3280f8d98 100644
>>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>> > @@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
>>> > if (!intel_dp_mst_source_support(intel_dp))
>>> > return;
>>> >
>>> > -   intel_dp->is_mst = sink_can_mst &&
>>> > -   i915->display.params.enable_dp_mst;
>>> > +   /*
>>> > +* Even if dpcd reg MSTM_CAP is 0, if the sink supports UHBR rates
>>> then
>>> > +* DP2.1 can be enabled with underlying protocol using MST for MTP
>>> > +*/
>>> > +   intel_dp->is_mst = (sink_can_mst ||
>>> > +
>>> drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
>>> > +   && i915->display.params.enable_dp_mst;
>>>
>>> We use drm_dp_is_uhbr_rate() in intel_dp_is_uhbr() to determine whether the
>>> link rate in the *crtc state* is uhbr, and by proxy whether the link in the 
>>> *crtc
>>> state* is 128b/132b.
>>>
>> Yes! If the link rate in crtc_state is not uhbr then we dont enable 
>> 128/132b. Also the return from drm_dp_read_mst_cap() return 0 or 1 and if 
>> not mst then 128/132b is not enabled. We need to deviate here and a value of 
>> bit[0]=0, bit[1]=0 in MSTM_CAP register is 128/132b with SST. Hence this 
>> hack is added to see if the return from MSTM_CAP is 0x00 and if uhbr rates 
>> are supported then enable 128/132b.
>
> Per spec it depends on intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] &
> DP_CAP_ANSI_128B132B, why not use that here too?

Also, shouldn't we ensure we don't try to do more than one stream?

>
>>
>>> There, we've already decided to use uhbr and 128b/132b.
>>>
>>> This one here is different, and I think it's taking us to the wrong 
>>> direction. For
>>> example, it should be possible to downgrade the link from uhbr to non-uhbr 
>>> on
>>> link failures. We don't have that yet. But this makes untangling that even
>>> harder.
>>
>> Yes upon having the fallback, I think we will have to handle fallback in 
>> this case separately. Change might be required in drm core, 
>> drm_dp_read_mst_cap() should consider the DP2.1 changes to accommodate this 
>> 0x00 value.
>>
>> Will be floating the fallback patches very soon.
>>
>> Thanks and Regards,
>> Arun R Murthy
>> 
>>>
>>>
>>> BR,
>>> Jani.
>>>
>>>
>>> >
>>> > drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
>>> > intel_dp->is_mst);
>>>
>>> --
>>> Jani Nikula, Intel

-- 
Jani Nikula, Intel

RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Jani Nikula]

On Tue, 09 Jan 2024, "Murthy, Arun R"  wrote:
>> -Original Message-
>> From: Nikula, Jani 
>> Sent: Monday, January 8, 2024 7:01 PM
>> To: Murthy, Arun R ; 
>> intel-gfx@lists.freedesktop.org;
>> Deak, Imre 
>> Cc: Murthy, Arun R 
>> Subject: Re: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
>>
>> On Wed, 03 Jan 2024, Arun R Murthy  wrote:
>> > With a value of '0' read from MSTM_CAP register MST to be enabled.
>> > DP2.1 SCR updates the spec for 128/132b DP capable supporting only one
>> > stream and not supporting single stream sideband MSG.
>> > The underlying protocol will be MST to enable use of MTP.
>> >
>> > Signed-off-by: Arun R Murthy 
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_dp.c | 9 +++--
>> >  1 file changed, 7 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> > b/drivers/gpu/drm/i915/display/intel_dp.c
>> > index 9ff0cbd9c0df..40d3280f8d98 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> > @@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
>> > if (!intel_dp_mst_source_support(intel_dp))
>> > return;
>> >
>> > -   intel_dp->is_mst = sink_can_mst &&
>> > -   i915->display.params.enable_dp_mst;
>> > +   /*
>> > +* Even if dpcd reg MSTM_CAP is 0, if the sink supports UHBR rates
>> then
>> > +* DP2.1 can be enabled with underlying protocol using MST for MTP
>> > +*/
>> > +   intel_dp->is_mst = (sink_can_mst ||
>> > +
>> drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
>> > +   && i915->display.params.enable_dp_mst;
>>
>> We use drm_dp_is_uhbr_rate() in intel_dp_is_uhbr() to determine whether the
>> link rate in the *crtc state* is uhbr, and by proxy whether the link in the 
>> *crtc
>> state* is 128b/132b.
>>
> Yes! If the link rate in crtc_state is not uhbr then we dont enable 128/132b. 
> Also the return from drm_dp_read_mst_cap() return 0 or 1 and if not mst then 
> 128/132b is not enabled. We need to deviate here and a value of bit[0]=0, 
> bit[1]=0 in MSTM_CAP register is 128/132b with SST. Hence this hack is added 
> to see if the return from MSTM_CAP is 0x00 and if uhbr rates are supported 
> then enable 128/132b.

Per spec it depends on intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] &
DP_CAP_ANSI_128B132B, why not use that here too?

>
>> There, we've already decided to use uhbr and 128b/132b.
>>
>> This one here is different, and I think it's taking us to the wrong 
>> direction. For
>> example, it should be possible to downgrade the link from uhbr to non-uhbr on
>> link failures. We don't have that yet. But this makes untangling that even
>> harder.
>
> Yes upon having the fallback, I think we will have to handle fallback in this 
> case separately. Change might be required in drm core, drm_dp_read_mst_cap() 
> should consider the DP2.1 changes to accommodate this 0x00 value.
>
> Will be floating the fallback patches very soon.
>
> Thanks and Regards,
> Arun R Murthy
> 
>>
>>
>> BR,
>> Jani.
>>
>>
>> >
>> > drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
>> > intel_dp->is_mst);
>>
>> --
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel

Re: [PATCH 2/5] drm/ttm: return ENOSPC from ttm_bo_mem_space

[Thomas Hellström]

On Tue, 2024-01-09 at 08:47 +0100, Christian König wrote:
> Only convert it to ENOMEM in ttm_bo_validate.
> 

Could we have a more elaborate commit description here, (why is this
change needed)?

> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
> b/drivers/gpu/drm/ttm/ttm_bo.c
> index edf10618fe2b..8c1eaa74fa21 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -830,7 +830,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object
> *bo,
>   goto error;
>   }
>  
> - ret = -ENOMEM;
> + ret = -ENOSPC;
>   if (!type_found) {
>   pr_err(TTM_PFX "No compatible memory type found\n");
>   ret = -EINVAL;
> @@ -916,6 +916,9 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
>   return -EINVAL;
>  
>   ret = ttm_bo_move_buffer(bo, placement, ctx);
> + /* For backward compatibility with userspace */
> + if (ret == -ENOSPC)
> + return -ENOMEM;
>   if (ret)
>   return ret;
>  

Re: [PATCH v2 1/3] drm/i915/gt: Support fixed CCS mode

[Tvrtko Ursulin]

On 08/01/2024 15:13, Joonas Lahtinen wrote:

Quoting Tvrtko Ursulin (2024-01-05 12:39:31)


On 04/01/2024 21:23, Andi Shyti wrote:





+void intel_gt_apply_ccs_mode(struct intel_gt *gt)
+{
+   mutex_lock(>->ccs.mutex);
+   __intel_gt_apply_ccs_mode(gt);
+   mutex_unlock(>->ccs.mutex);
+}
+
+void intel_gt_init_ccs_mode(struct intel_gt *gt)
+{
+   mutex_init(>->ccs.mutex);
+   gt->ccs.mode = 1;


What is '1'? And this question carries over to the sysfs interface in the
following patch - who will use it and where it is documented how to use it?


The value '1' is explained in the comment above[1] and in the


Do you mean this is mode '1':

   * With 1 engine (ccs0):
   *   slice 0, 1, 2, 3: ccs0

?

But I don't see where it says what do different modes mean on different
SKU configurations.

It also does not say what should the num_slices sysfs file be used for.

Does "mode N" mean "assign each command streamer N compute slices"? Or
"assign each compute slice N command streamers"?

I wonder if we should add something user friendly into
Documentation/ABI/*/sysfs-... Joonas your thoughts?


We definitely should always properly document all sysfs additions, just
seems like we less frequently remember to do so. So yeah, this should be
documented just like other uAPI.

I also like the idea of not exposing the the file at all if the value
can't be modified.

The ccs_mode is just supposed to allow user to select how many CCS
engines they want to expose, and always make an even split of slices
between them, nothing more nothing less.


Hmm I can't see that the series changes anywhere what command streamers 
will get reported as available.


Regards,

Tvrtko

Re: Rework TTMs busy handling

[Christian König]

Am 09.01.24 um 09:14 schrieb Thomas Hellström:

Hi, Christian

On Tue, 2024-01-09 at 08:47 +0100, Christian König wrote:

Hi guys,

I'm trying to make this functionality a bit more useful for years now
since we multiple reports that behavior of drivers can be suboptimal
when multiple placements be given.

So basically instead of hacking around the TTM behavior in the driver
once more I've gone ahead and changed the idle/busy placement list
into idle/busy placement flags. This not only saves a bunch of code,
but also allows setting some placements as fallback which are used if
allocating from the preferred ones didn't worked.

Zack pointed out that some removed VMWGFX code was brought back
because
of rebasing, fixed in this version.

Intel CI seems to be happy with those patches, so any more comments?

Looks like Xe changes are missing? (xe is now in drm-tip).

I also have some doubts about the naming "idle" vs "busy", since an
elaborate eviction mechanism would probably at some point want to check
for gpu idle vs gpu busy, and this might create some confusion moving
forward for people confusing busy as in memory overcommit with busy as
in gpu activity.

I can't immediately think of something better, though.


Yeah, I was wondering about that as well. Especially since I wanted to 
add some more flags in the future when for example a bandwidth quota how 
much memory can be moved in/out is exceeded.


Something like phase1, phase2, phase3 etc..., but that's also not very 
descriptive either.


Going to take a look at XE as well, thanks for the notice.

Regards,
Christian.



/Thomas



Regards,
Christian.

✓ Fi.CI.BAT: success for series starting with [1/5] drm/vmwgfx: remove vmw_vram_gmr_placement

[Patchwork]

== Series Details ==

Series: series starting with [1/5] drm/vmwgfx: remove vmw_vram_gmr_placement
URL   : https://patchwork.freedesktop.org/series/128355/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_14099 -> Patchwork_128355v1


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/index.html

Participating hosts (34 -> 36)
--

  Additional (3): bat-kbl-2 bat-adlm-1 bat-mtlp-8 
  Missing(1): fi-snb-2520m 

Known issues


  Here are the changes found in Patchwork_128355v1 that come from known issues:

### CI changes ###

 Issues hit 

  * boot:
- bat-jsl-1:  [PASS][1] -> [FAIL][2] ([i915#8293])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14099/bat-jsl-1/boot.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-jsl-1/boot.html

  

### IGT changes ###

 Issues hit 

  * igt@debugfs_test@basic-hwmon:
- bat-mtlp-8: NOTRUN -> [SKIP][3] ([i915#9318])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-mtlp-8/igt@debugfs_t...@basic-hwmon.html
- bat-adlm-1: NOTRUN -> [SKIP][4] ([i915#3826])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-adlm-1/igt@debugfs_t...@basic-hwmon.html

  * igt@fbdev@eof:
- bat-adlm-1: NOTRUN -> [SKIP][5] ([i915#2582]) +3 other tests skip
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-adlm-1/igt@fb...@eof.html

  * igt@fbdev@info:
- bat-kbl-2:  NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#1849])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-kbl-2/igt@fb...@info.html
- bat-adlm-1: NOTRUN -> [SKIP][7] ([i915#1849] / [i915#2582])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-adlm-1/igt@fb...@info.html

  * igt@gem_lmem_swapping@parallel-random-engines:
- bat-kbl-2:  NOTRUN -> [SKIP][8] ([fdo#109271]) +36 other tests 
skip
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-kbl-2/igt@gem_lmem_swapp...@parallel-random-engines.html
- bat-adlm-1: NOTRUN -> [SKIP][9] ([i915#4613]) +3 other tests skip
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-adlm-1/igt@gem_lmem_swapp...@parallel-random-engines.html

  * igt@gem_lmem_swapping@verify-random:
- bat-mtlp-8: NOTRUN -> [SKIP][10] ([i915#4613]) +3 other tests skip
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html

  * igt@gem_mmap@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][11] ([i915#4083])
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-mtlp-8/igt@gem_m...@basic.html

  * igt@gem_mmap_gtt@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][12] ([i915#4077]) +2 other tests skip
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-mtlp-8/igt@gem_mmap_...@basic.html

  * igt@gem_render_tiled_blits@basic:
- bat-mtlp-8: NOTRUN -> [SKIP][13] ([i915#4079]) +1 other test skip
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-mtlp-8/igt@gem_render_tiled_bl...@basic.html

  * igt@gem_tiled_pread_basic:
- bat-adlm-1: NOTRUN -> [SKIP][14] ([i915#3282])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-adlm-1/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_rps@basic-api:
- bat-mtlp-8: NOTRUN -> [SKIP][15] ([i915#6621])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-mtlp-8/igt@i915_pm_...@basic-api.html
- bat-adlm-1: NOTRUN -> [SKIP][16] ([i915#6621])
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-adlm-1/igt@i915_pm_...@basic-api.html

  * igt@i915_suspend@basic-s3-without-i915:
- bat-mtlp-8: NOTRUN -> [SKIP][17] ([i915#6645])
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-mtlp-8/igt@i915_susp...@basic-s3-without-i915.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][18] ([i915#5190])
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-mtlp-8/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][19] ([i915#4212]) +8 other tests skip
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-mtlp-8/igt@kms_addfb_ba...@basic-y-tiled-legacy.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- bat-mtlp-8: NOTRUN -> [SKIP][20] ([i915#4213]) +1 other test skip
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128355v1/bat-mtlp-8/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_cursor_legacy@basi

✗ Fi.CI.SPARSE: warning for series starting with [1/5] drm/vmwgfx: remove vmw_vram_gmr_placement

[Patchwork]

== Series Details ==

Series: series starting with [1/5] drm/vmwgfx: remove vmw_vram_gmr_placement
URL   : https://patchwork.freedesktop.org/series/128355/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/vmwgfx: remove vmw_vram_gmr_placement

[Patchwork]

== Series Details ==

Series: series starting with [1/5] drm/vmwgfx: remove vmw_vram_gmr_placement
URL   : https://patchwork.freedesktop.org/series/128355/
State : warning

== Summary ==

Error: dim checkpatch failed
3215387b42ab drm/vmwgfx: remove vmw_vram_gmr_placement
-:70: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address 
mismatch: 'From: "Christian König" ' != 
'Signed-off-by: Christian König '

total: 0 errors, 1 warnings, 0 checks, 47 lines checked
6c54d65338a8 drm/ttm: return ENOSPC from ttm_bo_mem_space
-:35: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address 
mismatch: 'From: "Christian König" ' != 
'Signed-off-by: Christian König '

total: 0 errors, 1 warnings, 0 checks, 17 lines checked
649aee04eb37 drm/ttm: replace busy placement with flags v5
14e27651bb55 drm/ttm: improve idle/busy handling v2
-:347: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address 
mismatch: 'From: "Christian König" ' != 
'Signed-off-by: Christian König '

total: 0 errors, 1 warnings, 0 checks, 284 lines checked
224e2d78eae5 drm/amdgpu: use GTT only as fallback for VRAM|GTT
-:32: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address 
mismatch: 'From: "Christian König" ' != 
'Signed-off-by: Christian König '

total: 0 errors, 1 warnings, 0 checks, 12 lines checked

Re: Rework TTMs busy handling

[Thomas Hellström]

Hi, Christian

On Tue, 2024-01-09 at 08:47 +0100, Christian König wrote:
> Hi guys,
> 
> I'm trying to make this functionality a bit more useful for years now
> since we multiple reports that behavior of drivers can be suboptimal
> when multiple placements be given.
> 
> So basically instead of hacking around the TTM behavior in the driver
> once more I've gone ahead and changed the idle/busy placement list
> into idle/busy placement flags. This not only saves a bunch of code,
> but also allows setting some placements as fallback which are used if
> allocating from the preferred ones didn't worked.
> 
> Zack pointed out that some removed VMWGFX code was brought back
> because
> of rebasing, fixed in this version.
> 
> Intel CI seems to be happy with those patches, so any more comments?

Looks like Xe changes are missing? (xe is now in drm-tip).

I also have some doubts about the naming "idle" vs "busy", since an
elaborate eviction mechanism would probably at some point want to check
for gpu idle vs gpu busy, and this might create some confusion moving
forward for people confusing busy as in memory overcommit with busy as
in gpu activity.

I can't immediately think of something better, though.

/Thomas


> 
> Regards,
> Christian.
> 
>