Re: [PATCH] nightly.conf: Add the xe repo to drm-tip
On Mon, 2024-01-08 at 16:22 -0600, Lucas De Marchi wrote: > On Mon, Jan 08, 2024 at 05:13:51PM -0500, Rodrigo Vivi wrote: > > On Wed, Jan 03, 2024 at 11:59:16PM -0600, Lucas De Marchi wrote: > > > On Wed, Jan 03, 2024 at 02:50:57PM +0100, Thomas Hellström wrote: > > > > On Tue, 2023-12-26 at 13:30 -0500, Rodrigo Vivi wrote: > > > > > On Fri, Dec 22, 2023 at 12:36:39PM +0100, Thomas Hellström > > > > > wrote: > > > > > > Add the xe repo to drm-tip and the dim tools. > > > > > > For now use the sha1 of the first drm-xe-next pull request > > > > > > for drm- > > > > > > tip, > > > > > > since that branch tip is currently adapted for our CI > > > > > > testing. > > > > > > > > > > > > Cc: Rodrigo Vivi > > > > > > Cc: Lucas De Marchi > > > > > > Cc: Oded Gabbay > > > > > > Cc: daniel.vet...@ffwll.ch > > > > > > Cc: Maarten Lankhorst > > > > > > Cc: dim-to...@lists.freedesktop.org > > > > > > Cc: dri-de...@lists.freedesktop.org > > > > > > Cc: intel-gfx@lists.freedesktop.org > > > > > > Signed-off-by: Thomas Hellström > > > > > > > > > > > > --- > > > > > > nightly.conf | 7 +++ > > > > > > 1 file changed, 7 insertions(+) > > > > > > > > > > > > diff --git a/nightly.conf b/nightly.conf > > > > > > index 24126b61b797..accd3ff2cc39 100644 > > > > > > --- a/nightly.conf > > > > > > +++ b/nightly.conf > > > > > > @@ -24,6 +24,10 @@ git://anongit.freedesktop.org/drm-tip > > > > > > https://anongit.freedesktop.org/git/drm/drm-tip > > > > > > https://anongit.freedesktop.org/git/drm/drm-tip.git > > > > > > " > > > > > > +drm_tip_repos[drm-xe]=" > > > > > > +ssh://g...@gitlab.freedesktop.org/drm/xe/kernel.git > > > > > > +https://gitlab.freedesktop.org/drm/xe/kernel.git > > > > > > +" > > > > > > drm_tip_repos[drm-intel]=" > > > > > > ssh://git.freedesktop.org/git/drm/drm-intel > > > > > > ssh://git.freedesktop.org/git/drm-intel > > > > > > @@ -65,14 +69,17 @@ drm_tip_config=( > > > > > > "drmdrm-fixes" > > > > > > "drm-misc drm-misc-fixes" > > > > > > "drm-intel drm-intel-fixes" > > > > > > + "drm-xe drm-xe-fixes" > > > > > > > > > > > > "drmdrm-next" > > > > > > "drm-misc drm-misc-next-fixes" > > > > > > "drm-intel drm-intel-next-fixes" > > > > > > + "drm-xe drm-xe-next-fixes" > > > > > > > > > > > > "drm-misc drm-misc-next" > > > > > > "drm-intel drm-intel-next" > > > > > > "drm-intel drm-intel-gt-next" > > > > > > + "drm-xe drm-xe-next > > > > > > b6e1b7081768" > > > > > > > > > > yeap, up to this commit nothing else should change, but > > > > > then we will need an extra rebase of the rest on top of > > > > > drm/drm-next. > > > > > > > > > > But then we need to decide where these following patches will > > > > > live: > > > > > 880277f31cc69 drm/xe/guc: define LNL FW > > > > > 2cfc5ae1b8267 drm/xe/guc: define PVC FW > > > > > 52383b58eb8cf mei/hdcp: Also enable for XE > > > > > bea27d7369855 mei: gsc: add support for auxiliary device > > > > > created by > > > > > Xe driver > > > > > fcb3410197f05 fault-inject: Include linux/types.h by default. > > > > > 8ebd9cd71f8ac drm/xe: Add PVC's PCI device IDs > > > > > > > > > > > > > > > Will it be the topic/core-for-CI? > > > > > or topic/xe-extras? > > > > > or what? > > > > > > > > This sounds to me like topic/core-for-CI? Or is there any > > > > drawback with > > > > that? > > > > > > I think some of them are not really a "for CI". It's more like > > > the > > > workflow we are adopting e.g. with guc/huc, not sending it to > > > linux-firmware > > > until we are confident on what version we will start officially > > > supporting. > > > > yeap, I kind of agree here, but at the same time it is our way to > > run > > our CI with the firmware blobs that we need while not final, and > > also > > this was already used for i915's MTL firmware. > > ok > > > > > > > > > This one can't go to topic/core-for-CI neither: > > > fcb3410197f05 fault-inject: Include linux/types.h by > > > default. > > > > > > what it would do would be that we would not see the build error > > > anymore, > > > but everyone else would (and it's not a CI-only configuration). > > > Unless it's merged to another branch, we shouldn't merge it. > > > > yeap. it is sad that we were ignored there. let's just drop this > > then. > > our driver is workarounding this bug anyway already. > > agreed, let's drop it. > > > > > > > > > > > "52383b58eb8cf mei/hdcp: Also enable for XE" could be material > > > for > > > topic/core-for-CI and "8ebd9cd71f8ac drm/xe: Add PVC's PCI > > > device IDs" > > > could either be on that branch or another xe-specific one. > > > > yeap. For the MEI we probably need to ping Greg on the original > > submission and ask his ack so we can put that in the final drm-xe- > > next > > for good and not even include in a topic
Re: [PATCH v2 03/13] drm/i915/psr: Do not check alpm on DP or capability change for panel replay
On Thu, 2024-01-11 at 06:14 +, Manna, Animesh wrote: > > > > -Original Message- > > From: Intel-gfx On Behalf > > Of Jouni > > Högander > > Sent: Wednesday, January 10, 2024 6:43 PM > > To: intel-gfx@lists.freedesktop.org > > Subject: [PATCH v2 03/13] drm/i915/psr: Do not check alpm on DP or > > capability change for panel replay > > > > Alpm is eDP specific. Do not check if not eDP. Also panel replay > > doesn't know > > about capability changes -> no need to check that. > > > > Signed-off-by: Jouni Högander > > --- > > drivers/gpu/drm/i915/display/intel_psr.c | 7 +-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > b/drivers/gpu/drm/i915/display/intel_psr.c > > index 3e287a9f0e09..a9421dd092c5 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -2989,8 +2989,11 @@ void intel_psr_short_pulse(struct intel_dp > > *intel_dp) > > /* clear status register */ > > drm_dp_dpcd_writeb(_dp->aux, DP_PSR_ERROR_STATUS, > > error_status); > > > > - psr_alpm_check(intel_dp); > > - psr_capability_changed_check(intel_dp); > > + if (intel_dp_is_edp(intel_dp)) > > + psr_alpm_check(intel_dp); > > + > > + if (!psr->panel_replay_enabled) > > + psr_capability_changed_check(intel_dp); > > There is a CAN_PSR() check starting of the function > intel_psr_short_pulse() which will take care non-edp and panel replay > case, so do you see any gap there? Thank you for pointing this out. We need to run intel_psr_short_pulse for panel replay case as well. I will add that into this patch. BR, Jouni Högander > > Regards, > Animesh > > > > exit: > > mutex_unlock(>lock); > > -- > > 2.34.1 >
RE: [PATCH v2 03/13] drm/i915/psr: Do not check alpm on DP or capability change for panel replay
> -Original Message- > From: Intel-gfx On Behalf Of Jouni > Högander > Sent: Wednesday, January 10, 2024 6:43 PM > To: intel-gfx@lists.freedesktop.org > Subject: [PATCH v2 03/13] drm/i915/psr: Do not check alpm on DP or > capability change for panel replay > > Alpm is eDP specific. Do not check if not eDP. Also panel replay doesn't know > about capability changes -> no need to check that. > > Signed-off-by: Jouni Högander > --- > drivers/gpu/drm/i915/display/intel_psr.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > b/drivers/gpu/drm/i915/display/intel_psr.c > index 3e287a9f0e09..a9421dd092c5 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -2989,8 +2989,11 @@ void intel_psr_short_pulse(struct intel_dp > *intel_dp) > /* clear status register */ > drm_dp_dpcd_writeb(_dp->aux, DP_PSR_ERROR_STATUS, > error_status); > > - psr_alpm_check(intel_dp); > - psr_capability_changed_check(intel_dp); > + if (intel_dp_is_edp(intel_dp)) > + psr_alpm_check(intel_dp); > + > + if (!psr->panel_replay_enabled) > + psr_capability_changed_check(intel_dp); There is a CAN_PSR() check starting of the function intel_psr_short_pulse() which will take care non-edp and panel replay case, so do you see any gap there? Regards, Animesh > > exit: > mutex_unlock(>lock); > -- > 2.34.1
RE: [PATCH v2 02/13] drm/i915/psr: Intel_psr_pause/resume needs to support panel replay
> -Original Message-
> From: Intel-gfx On Behalf Of Jouni
> Högander
> Sent: Wednesday, January 10, 2024 6:43 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [PATCH v2 02/13] drm/i915/psr: Intel_psr_pause/resume needs to
> support panel replay
>
> Currently intel_psr_pause and intel_psr_resume do nothing in case of panel
> replay. Change them to perform pause and return also in case of panel
> replay.
>
> Signed-off-by: Jouni Högander
Changes looks good to me.
Reviewed-by: Animesh Manna
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 9705a75e063a..3e287a9f0e09 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1829,7 +1829,7 @@ void intel_psr_pause(struct intel_dp *intel_dp)
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> struct intel_psr *psr = _dp->psr;
>
> - if (!CAN_PSR(intel_dp))
> + if (!CAN_PSR(intel_dp) && !CAN_PANEL_REPLAY(intel_dp))
> return;
>
> mutex_lock(>lock);
> @@ -1862,7 +1862,7 @@ void intel_psr_resume(struct intel_dp *intel_dp) {
> struct intel_psr *psr = _dp->psr;
>
> - if (!CAN_PSR(intel_dp))
> + if (!CAN_PSR(intel_dp) && !CAN_PANEL_REPLAY(intel_dp))
> return;
>
> mutex_lock(>lock);
> --
> 2.34.1
RE: [PATCH v2 01/13] drm/i915/psr: Disable panel replay for now
> -Original Message-
> From: Intel-gfx On Behalf Of Jouni
> Högander
> Sent: Wednesday, January 10, 2024 6:43 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [PATCH v2 01/13] drm/i915/psr: Disable panel replay for now
>
> Panel replay is not completely validated yet. Let's disable it for now.
Hi,
As I understood currently the feature is not tested due to unavailability of
the panel and at the same time good to check negative testing like if this
feature is causing any regression for other feature like psr/psr2. Instead of
hardcoding Is it ok to have a kernel cmdline parameter to enable/disable this
feature? Can you please share your view.
Regards,
Animesh
>
> Signed-off-by: Jouni Högander
> ---
> drivers/gpu/drm/i915/display/intel_display_types.h | 1 +
> drivers/gpu/drm/i915/display/intel_psr.c | 10 +-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index ae2e8cff9d69..6340fabd045c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1692,6 +1692,7 @@ struct intel_psr {
> #define I915_PSR_DEBUG_ENABLE_SEL_FETCH 0x4
> #define I915_PSR_DEBUG_IRQ 0x10
> #define I915_PSR_DEBUG_SU_REGION_ET_DISABLE 0x20
> +#define I915_PSR_DEBUG_PANEL_REPLAY_DISABLE 0x40
>
> 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 dff21a5edeb7..9705a75e063a 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -214,6 +214,11 @@ static bool psr2_global_enabled(struct intel_dp
> *intel_dp)
> }
> }
>
> +static bool panel_replay_global_enabled(struct intel_dp *intel_dp) {
> + return !(intel_dp->psr.debug &
> I915_PSR_DEBUG_PANEL_REPLAY_DISABLE);
> +}
> +
> static u32 psr_irq_psr_error_bit_get(struct intel_dp *intel_dp) {
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@ -
> 1386,7 +1391,7 @@ void intel_psr_compute_config(struct intel_dp
> *intel_dp,
> }
>
> if (CAN_PANEL_REPLAY(intel_dp))
> - crtc_state->has_panel_replay = true;
> + crtc_state->has_panel_replay =
> panel_replay_global_enabled(intel_dp);
> else
> crtc_state->has_psr = _psr_compute_config(intel_dp,
> crtc_state);
>
> @@ -2845,6 +2850,9 @@ void intel_psr_init(struct intel_dp *intel_dp)
> /* Disable early transport for now */
> intel_dp->psr.debug |= I915_PSR_DEBUG_SU_REGION_ET_DISABLE;
>
> + /* Disable panel replay for now */
> + intel_dp->psr.debug |= I915_PSR_DEBUG_PANEL_REPLAY_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: Restart the heartbeat timer when forcing a pulse
== Series Details == Series: drm/i915/gt: Restart the heartbeat timer when forcing a pulse URL : https://patchwork.freedesktop.org/series/128458/ State : success == Summary == CI Bug Log - changes from CI_DRM_14109 -> Patchwork_128458v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/index.html Participating hosts (36 -> 36) -- Additional (2): bat-kbl-2 bat-mtlp-8 Missing(2): bat-rpls-2 fi-snb-2520m Known issues Here are the changes found in Patchwork_128458v1 that come from known issues: ### CI changes ### Possible fixes * boot: - bat-jsl-1: [FAIL][1] ([i915#8293]) -> [PASS][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14109/bat-jsl-1/boot.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/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_128458v1/bat-mtlp-8/igt@debugfs_t...@basic-hwmon.html - bat-jsl-1: NOTRUN -> [SKIP][4] ([i915#9318]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/bat-jsl-1/igt@debugfs_t...@basic-hwmon.html * igt@fbdev@info: - bat-kbl-2: NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#1849]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/bat-kbl-2/igt@fb...@info.html * igt@gem_huc_copy@huc-copy: - bat-jsl-1: NOTRUN -> [SKIP][6] ([i915#2190]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/bat-jsl-1/igt@gem_huc_c...@huc-copy.html * igt@gem_lmem_swapping@parallel-random-engines: - bat-kbl-2: NOTRUN -> [SKIP][7] ([fdo#109271]) +36 other tests skip [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/bat-kbl-2/igt@gem_lmem_swapp...@parallel-random-engines.html * igt@gem_lmem_swapping@verify-random: - bat-mtlp-8: NOTRUN -> [SKIP][8] ([i915#4613]) +3 other tests skip [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html - bat-jsl-1: NOTRUN -> [SKIP][9] ([i915#4613]) +3 other tests skip [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/bat-jsl-1/igt@gem_lmem_swapp...@verify-random.html * igt@gem_mmap@basic: - bat-mtlp-8: NOTRUN -> [SKIP][10] ([i915#4083]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/bat-mtlp-8/igt@gem_m...@basic.html * igt@gem_mmap_gtt@basic: - bat-mtlp-8: NOTRUN -> [SKIP][11] ([i915#4077]) +2 other tests skip [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/bat-mtlp-8/igt@gem_mmap_...@basic.html * igt@gem_render_tiled_blits@basic: - bat-mtlp-8: NOTRUN -> [SKIP][12] ([i915#4079]) +1 other test skip [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/bat-mtlp-8/igt@gem_render_tiled_bl...@basic.html * igt@i915_pm_rps@basic-api: - bat-mtlp-8: NOTRUN -> [SKIP][13] ([i915#6621]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/bat-mtlp-8/igt@i915_pm_...@basic-api.html * igt@i915_suspend@basic-s3-without-i915: - bat-mtlp-8: NOTRUN -> [SKIP][14] ([i915#6645]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/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][15] ([i915#5190]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/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][16] ([i915#4212]) +8 other tests skip [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/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][17] ([i915#4213]) +1 other test skip [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/bat-mtlp-8/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html - bat-jsl-1: NOTRUN -> [SKIP][18] ([i915#4103]) +1 other test skip [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/bat-jsl-1/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html * igt@kms_dsc@dsc-basic: - bat-mtlp-8: NOTRUN -> [SKIP][19] ([i915#3555] / [i915#3840] / [i915#9159]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128458v1/bat-mtlp-8/igt@kms_...@dsc-basic.html - bat-jsl-1: NOTRUN -> [SKIP][20] ([i915#3555] / [i915#9886]) [20]:
[PATCH] drm/i915/gt: Restart the heartbeat timer when forcing a pulse
From: John Harrison The context persistence code does things like send super high priority heartbeat pulses to ensure any leaked context can still be pre-empted and thus isn't a total denial of service but only a minor denial of service. Unfortunately, it wasn't bothering to restart the heatbeat worker with a fresh timeout. Thus, if a persistent context happened to be closed just before the heartbeat was going to go ping anyway then the forced pulse would get a negligble execution time. And as the forced pulse is super high priority, the worker thread's next step is a reset. Which means a potentially innocent system randomly goes boom when attempting to close a context. So, force a re-schedule of the worker thread with the appropriate timeout. Signed-off-by: John Harrison --- drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c index 1a8e2b7db0138..4ae2fa0b61dd4 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c @@ -290,6 +290,9 @@ static int __intel_engine_pulse(struct intel_engine_cs *engine) heartbeat_commit(rq, ); GEM_BUG_ON(rq->sched.attr.priority < I915_PRIORITY_BARRIER); + /* Ensure the forced pulse gets a full period to execute */ + next_heartbeat(engine); + return 0; } -- 2.43.0
Re: [PATCH 6/6] drm: Add CONFIG_DRM_WERROR
On 1/10/24 12:39, Jani Nikula wrote: Add kconfig to enable -Werror subsystem wide. This is useful for development and CI to keep the subsystem warning free, while avoiding issues outside of the subsystem that kernel wide CONFIG_WERROR=y might hit. Signed-off-by: Jani Nikula Reviewed-by: Hamza Mahfooz --- drivers/gpu/drm/Kconfig | 18 ++ drivers/gpu/drm/Makefile | 3 +++ 2 files changed, 21 insertions(+) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 6ec33d36f3a4..36a00cba2540 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -414,3 +414,21 @@ config DRM_LIB_RANDOM config DRM_PRIVACY_SCREEN bool default n + +config DRM_WERROR + bool "Compile the drm subsystem with warnings as errors" + # As this may inadvertently break the build, only allow the user + # to shoot oneself in the foot iff they aim really hard + depends on EXPERT + # We use the dependency on !COMPILE_TEST to not be enabled in + # allmodconfig or allyesconfig configurations + depends on !COMPILE_TEST + default n + help + A kernel build should not cause any compiler warnings, and this + enables the '-Werror' flag to enforce that rule in the drm subsystem. + + The drm subsystem enables more warnings than the kernel default, so + this config option is disabled by default. + + If in doubt, say N. diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 8b6be830f7c3..b7fd3e58b7af 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -32,6 +32,9 @@ subdir-ccflags-y += -Wno-sign-compare endif # --- end copy-paste +# Enable -Werror in CI and development +subdir-ccflags-$(CONFIG_DRM_WERROR) += -Werror + drm-y := \ drm_aperture.o \ drm_atomic.o \ -- Hamza
Re: [PATCH 5/6] drm: enable (most) W=1 warnings by default across the subsystem
On 1/10/24 12:39, Jani Nikula wrote: At least the i915 and amd drivers enable a bunch more compiler warnings than the kernel defaults. Extend most of the W=1 warnings to the entire drm subsystem by default. Use the copy-pasted warnings from scripts/Makefile.extrawarn with s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and keep up with them in the future. This is similar to the approach currently used in i915. Some of the -Wextra warnings do need to be disabled, just like in Makefile.extrawarn, but take care to not disable them for W=2 or W=3 builds, depending on the warning. There are too many -Wformat-truncation warnings to cleanly fix up front; leave that warning disabled for now. v2: - Drop -Wformat-truncation (too many warnings) - Drop -Wstringop-overflow (enabled by default upstream) Cc: David Airlie Cc: Daniel Vetter Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: Alex Deucher Cc: Christian König Cc: Pan, Xinhui Cc: Karol Herbst Cc: Lyude Paul Cc: Danilo Krummrich Cc: Rob Clark Cc: Abhinav Kumar Cc: Dmitry Baryshkov Cc: Sean Paul Cc: Marijn Suijten Cc: Hamza Mahfooz Acked-by: Javier Martinez Canillas Acked-by: Thomas Zimmermann Acked-by: Sui Jingfeng Signed-off-by: Jani Nikula --- drivers/gpu/drm/Makefile | 27 +++ 1 file changed, 27 insertions(+) diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 104b42df2e95..8b6be830f7c3 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -5,6 +5,33 @@ CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE +# Unconditionally enable W=1 warnings locally +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn +subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter +subdir-ccflags-y += -Wmissing-declarations +subdir-ccflags-y += $(call cc-option, -Wrestrict) It would be safer to do something along the lines of: cond-flags := $(call cc-option, -Wrestrict) \ $(call cc-option, -Wunused-but-set-variable) \ $(call cc-option, -Wunused-const-variable) \ $(call cc-option, -Wunused-const-variable) \ $(call cc-option, -Wformat-overflow) \ $(call cc-option, -Wstringop-truncation) subdir-ccflags-y += $(cond-flags) Otherwise, you will end up breaking `$ make M=drivers/gpu/drm` for a bunch of people. +subdir-ccflags-y += -Wmissing-format-attribute +subdir-ccflags-y += -Wmissing-prototypes +subdir-ccflags-y += -Wold-style-definition +subdir-ccflags-y += -Wmissing-include-dirs +subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable) +subdir-ccflags-y += $(call cc-option, -Wunused-const-variable) +subdir-ccflags-y += $(call cc-option, -Wpacked-not-aligned) +subdir-ccflags-y += $(call cc-option, -Wformat-overflow) +# FIXME: fix -Wformat-truncation warnings and uncomment +#subdir-ccflags-y += $(call cc-option, -Wformat-truncation) +subdir-ccflags-y += $(call cc-option, -Wstringop-truncation) +# The following turn off the warnings enabled by -Wextra +ifeq ($(findstring 2, $(KBUILD_EXTRA_WARN)),) +subdir-ccflags-y += -Wno-missing-field-initializers +subdir-ccflags-y += -Wno-type-limits +subdir-ccflags-y += -Wno-shift-negative-value +endif +ifeq ($(findstring 3, $(KBUILD_EXTRA_WARN)),) +subdir-ccflags-y += -Wno-sign-compare +endif +# --- end copy-paste + drm-y := \ drm_aperture.o \ drm_atomic.o \ -- Hamza
Re: [PATCH] drm/i915/gt: Use rc6.supported flag from intel_gt for rc6_enable sysfs
On Tue, Jan 09, 2024 at 05:03:00PM -0800, Juan Escamilla wrote:
> 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
Reviewed-by: Rodrigo Vivi
and pushing this right now, thanks for your patch
> ---
> 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(>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: [PATCH] drm/xe/display: Disable aux ccs framebuffers
On Tue, Jan 09, 2024 at 08:40:24PM +, Souza, Jose wrote:
> 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.
we can do either:
1. backmerge drm-next into drm-intel-next and merge this as is. (This would be
with
Jani)
2. split in 2 patches, one for drm-intel-next and the other for drm-xe-next.
(This would
be with Jouni)
3. merge this as is in drm-xe-next and deal with the conflicts in a future
backmerge.
Since this is mostly adding a new file I don't believe that it would be a big
deal.
(This would impact myself)
Since next round of drm-intel-next is mine, I'd be okay on handling that and
acking
this approach number 3. But before moving forward with this I'd like to wait for
Jani's and Jouni's opinions.
>
> >
> > >
> > >
> > > 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) &&
✓ Fi.CI.BAT: success for drm: enable W=1 warnings by default across the subsystem (rev2)
== Series Details == Series: drm: enable W=1 warnings by default across the subsystem (rev2) URL : https://patchwork.freedesktop.org/series/127072/ State : success == Summary == CI Bug Log - changes from CI_DRM_14108 -> Patchwork_127072v2 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/index.html Participating hosts (38 -> 37) -- Additional (1): bat-mtlp-8 Missing(2): bat-rpls-2 fi-snb-2520m Known issues Here are the changes found in Patchwork_127072v2 that come from known issues: ### IGT changes ### Issues hit * igt@debugfs_test@basic-hwmon: - bat-mtlp-8: NOTRUN -> [SKIP][1] ([i915#9318]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-mtlp-8/igt@debugfs_t...@basic-hwmon.html * igt@gem_lmem_swapping@verify-random: - bat-mtlp-8: NOTRUN -> [SKIP][2] ([i915#4613]) +3 other tests skip [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html * igt@gem_mmap@basic: - bat-mtlp-8: NOTRUN -> [SKIP][3] ([i915#4083]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-mtlp-8/igt@gem_m...@basic.html * igt@gem_mmap_gtt@basic: - bat-mtlp-8: NOTRUN -> [SKIP][4] ([i915#4077]) +2 other tests skip [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-mtlp-8/igt@gem_mmap_...@basic.html * igt@gem_render_tiled_blits@basic: - bat-mtlp-8: NOTRUN -> [SKIP][5] ([i915#4079]) +1 other test skip [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-mtlp-8/igt@gem_render_tiled_bl...@basic.html * igt@i915_pm_rps@basic-api: - bat-mtlp-8: NOTRUN -> [SKIP][6] ([i915#6621]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-mtlp-8/igt@i915_pm_...@basic-api.html * igt@i915_selftest@live@workarounds: - bat-adlm-1: [PASS][7] -> [INCOMPLETE][8] ([i915#9413]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14108/bat-adlm-1/igt@i915_selftest@l...@workarounds.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-adlm-1/igt@i915_selftest@l...@workarounds.html * igt@i915_suspend@basic-s3-without-i915: - bat-mtlp-8: NOTRUN -> [SKIP][9] ([i915#6645]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/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][10] ([i915#5190]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/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][11] ([i915#4212]) +8 other tests skip [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/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][12] ([i915#4213]) +1 other test skip [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-mtlp-8/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html * igt@kms_dsc@dsc-basic: - bat-mtlp-8: NOTRUN -> [SKIP][13] ([i915#3555] / [i915#3840] / [i915#9159]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-mtlp-8/igt@kms_...@dsc-basic.html * igt@kms_force_connector_basic@force-load-detect: - bat-mtlp-8: NOTRUN -> [SKIP][14] ([fdo#109285]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-mtlp-8/igt@kms_force_connector_ba...@force-load-detect.html * igt@kms_force_connector_basic@prune-stale-modes: - bat-mtlp-8: NOTRUN -> [SKIP][15] ([i915#5274]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-mtlp-8/igt@kms_force_connector_ba...@prune-stale-modes.html * igt@kms_pipe_crc_basic@suspend-read-crc: - bat-atsm-1: NOTRUN -> [SKIP][16] ([i915#1836]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-atsm-1/igt@kms_pipe_crc_ba...@suspend-read-crc.html * igt@kms_setmode@basic-clone-single-crtc: - bat-mtlp-8: NOTRUN -> [SKIP][17] ([i915#3555] / [i915#8809]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-mtlp-8/igt@kms_setm...@basic-clone-single-crtc.html * igt@prime_vgem@basic-fence-mmap: - bat-mtlp-8: NOTRUN -> [SKIP][18] ([i915#3708] / [i915#4077]) +1 other test skip [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127072v2/bat-mtlp-8/igt@prime_v...@basic-fence-mmap.html * igt@prime_vgem@basic-fence-read: - bat-mtlp-8: NOTRUN -> [SKIP][19] ([i915#3708]) +2 other tests skip [19]:
✗ Fi.CI.SPARSE: warning for drm: enable W=1 warnings by default across the subsystem (rev2)
== Series Details == Series: drm: enable W=1 warnings by default across the subsystem (rev2) URL : https://patchwork.freedesktop.org/series/127072/ 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 drm: enable W=1 warnings by default across the subsystem (rev2)
== Series Details == Series: drm: enable W=1 warnings by default across the subsystem (rev2) URL : https://patchwork.freedesktop.org/series/127072/ State : warning == Summary == Error: dim checkpatch failed e933bc518dbe drm/nouveau/acr/ga102: remove unused but set variable 3d19a8353d23 drm/nouveau/svm: remove unused but set variables -:23: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned' #23: FILE: drivers/gpu/drm/nouveau/nouveau_svm.c:115: + unsigned target, cmd; total: 0 errors, 1 warnings, 0 checks, 34 lines checked d3c52590c534 drm/amdgpu: prefer snprintf over sprintf 1a47f78e2cb8 drm/imx: prefer snprintf over sprintf d12ab92b241f drm: enable (most) W=1 warnings by default across the subsystem 9ef4af77b0bf drm: Add CONFIG_DRM_WERROR
[bug report] drm/i915/gt: Use i915_vm_put on ppgtt_create error paths
Hello Chris Wilson,
This is a semi-automatic email about new static checker warnings.
The patch c286558f5853: "drm/i915/gt: Use i915_vm_put on ppgtt_create
error paths" from Sep 26, 2022, leads to the following Smatch
complaint:
drivers/gpu/drm/i915/gt/gen6_ppgtt.c:274 gen6_ppgtt_cleanup()
warn: variable dereferenced before check 'ppgtt->base.pd' (see line 271)
drivers/gpu/drm/i915/gt/gen6_ppgtt.c
256 static void gen6_ppgtt_free_pd(struct gen6_ppgtt *ppgtt)
257 {
258 struct i915_page_directory * const pd = ppgtt->base.pd;
^^
pd is ppgtt->base.pd.
259 struct i915_page_table *pt;
260 u32 pde;
261
262 gen6_for_all_pdes(pt, pd, pde)
^^
There is an unchecked dereference inside this loop macro.
263 if (pt)
264 free_pt(>base.vm, pt);
265 }
266
267 static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
268 {
269 struct gen6_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
270
271 gen6_ppgtt_free_pd(ppgtt);
^^
272 free_scratch(vm);
273
274 if (ppgtt->base.pd)
^^
Checked after an unchecked dereference.
275 free_pd(>base.vm, ppgtt->base.pd);
276
277 mutex_destroy(>flush);
278 }
regards,
dan carpenter
✓ Fi.CI.BAT: success for drm/i915: Rework global state serialization (rev2)
== Series Details == Series: drm/i915: Rework global state serialization (rev2) URL : https://patchwork.freedesktop.org/series/127968/ State : success == Summary == CI Bug Log - changes from CI_DRM_14108 -> Patchwork_127968v2 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v2/index.html Participating hosts (38 -> 34) -- Missing(4): bat-kbl-2 bat-dg2-9 fi-snb-2520m bat-adls-6 Known issues Here are the changes found in Patchwork_127968v2 that come from known issues: ### IGT changes ### Issues hit * igt@kms_pipe_crc_basic@suspend-read-crc: - bat-atsm-1: NOTRUN -> [SKIP][1] ([i915#1836]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v2/bat-atsm-1/igt@kms_pipe_crc_ba...@suspend-read-crc.html Possible fixes * igt@gem_exec_suspend@basic-s3@lmem0: - bat-atsm-1: [ABORT][2] -> [PASS][3] [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14108/bat-atsm-1/igt@gem_exec_suspend@basic...@lmem0.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v2/bat-atsm-1/igt@gem_exec_suspend@basic...@lmem0.html [i915#1836]: https://gitlab.freedesktop.org/drm/intel/issues/1836 Build changes - * Linux: CI_DRM_14108 -> Patchwork_127968v2 CI-20190529: 20190529 CI_DRM_14108: c58d1352dd193d8df380a14e95c05455acaf5b82 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7666: 5f97adfd0e1636788a6af5b592f0d15b4f1027c8 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_127968v2: c58d1352dd193d8df380a14e95c05455acaf5b82 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits a10e407e523c drm/i915: Extract intel_atomic_swap_state() 47b9afbea14a drm/i915: Rework global state serializaiton 16fe755e0644 drm/i915: Compute use_sagv_wm differently == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127968v2/index.html
✗ Fi.CI.SPARSE: warning for drm/i915: Rework global state serialization (rev2)
== Series Details == Series: drm/i915: Rework global state serialization (rev2) URL : https://patchwork.freedesktop.org/series/127968/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
✓ Fi.CI.BAT: success for Panel replay selective update support (rev2)
== Series Details ==
Series: Panel replay selective update support (rev2)
URL : https://patchwork.freedesktop.org/series/128193/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_14108 -> Patchwork_128193v2
Summary
---
**SUCCESS**
No regressions found.
External URL:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v2/index.html
Participating hosts (38 -> 34)
--
Missing(4): bat-kbl-2 bat-dg2-9 fi-snb-2520m fi-bsw-n3050
Known issues
Here are the changes found in Patchwork_128193v2 that come from known issues:
### IGT changes ###
Issues hit
* igt@kms_pipe_crc_basic@suspend-read-crc:
- bat-atsm-1: NOTRUN -> [SKIP][1] ([i915#1836])
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v2/bat-atsm-1/igt@kms_pipe_crc_ba...@suspend-read-crc.html
Possible fixes
* igt@gem_exec_suspend@basic-s3@lmem0:
- bat-atsm-1: [ABORT][2] -> [PASS][3]
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14108/bat-atsm-1/igt@gem_exec_suspend@basic...@lmem0.html
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v2/bat-atsm-1/igt@gem_exec_suspend@basic...@lmem0.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#10022]: https://gitlab.freedesktop.org/drm/intel/issues/10022
[i915#1836]: https://gitlab.freedesktop.org/drm/intel/issues/1836
[i915#9943]: https://gitlab.freedesktop.org/drm/intel/issues/9943
Build changes
-
* Linux: CI_DRM_14108 -> Patchwork_128193v2
CI-20190529: 20190529
CI_DRM_14108: c58d1352dd193d8df380a14e95c05455acaf5b82 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_7666: 5f97adfd0e1636788a6af5b592f0d15b4f1027c8 @
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_128193v2: c58d1352dd193d8df380a14e95c05455acaf5b82 @
git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
5b2af80709ac drm/i915/psr: Add panel replay sel update support to debugfs
interface
f702a2603a88 drm/i915/psr: Modify intel_dp_get_su_granularity to support panel
replay
30d778a29269 drm/panelreplay: dpcd register definition for panelreplay SU
4afdb41b6ac2 drm/i915/psr: Split intel_psr2_config_valid for panel replay
b64834631574 drm/i915/psr: Detect panel replay selective update support
e13ef0e6bf8a drm/i915/psr: Add sink_panel_replay_su_support to intel_psr
1feb8b68b97f drm/i915/psr: Add some documentation of variables used in psr code
43e000242422 drm/i915/psr: Rename psr2_enabled as sel_update_enabled
a1b165fa852d drm/i915/psr: Rename has_psr2 as has_sel_update
245aa06c12de drm/i915/psr: Unify panel replay enable sink
44603b0f21a1 drm/i915/psr: Do not check alpm on DP or capability change for
panel replay
2d7dfe178c9c drm/i915/psr: Intel_psr_pause/resume needs to support panel replay
693d0b295779 drm/i915/psr: Disable panel replay for now
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128193v2/index.html
✗ Fi.CI.CHECKPATCH: warning for Panel replay selective update support (rev2)
== Series Details == Series: Panel replay selective update support (rev2) URL : https://patchwork.freedesktop.org/series/128193/ State : warning == Summary == Error: dim checkpatch failed af6e1b067218 drm/i915/psr: Disable panel replay for now 1ad3b9a2a48a drm/i915/psr: Intel_psr_pause/resume needs to support panel replay f0b97dc1d99b drm/i915/psr: Do not check alpm on DP or capability change for panel replay 424c68f53ab4 drm/i915/psr: Unify panel replay enable sink f0bf5f90363d drm/i915/psr: Rename has_psr2 as has_sel_update ac4958a72f0f drm/i915/psr: Rename psr2_enabled as sel_update_enabled d27945c32bdf drm/i915/psr: Add some documentation of variables used in psr code ac74a6fbe5bd drm/i915/psr: Add sink_panel_replay_su_support to intel_psr f111452b0ef7 drm/i915/psr: Detect panel replay selective update support 65a947a12d8e drm/i915/psr: Split intel_psr2_config_valid for panel replay 9f5ef2469b13 drm/panelreplay: dpcd register definition for panelreplay SU 77a34490833a drm/i915/psr: Modify intel_dp_get_su_granularity to support panel replay ddde1b752ad8 drm/i915/psr: Add panel replay sel update support to debugfs interface -:13: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #13: Sink support: PSR = no, Panel Replay = yes, Panel Replay Selective Update = yes total: 0 errors, 1 warnings, 0 checks, 22 lines checked
✗ Fi.CI.SPARSE: warning for Panel replay selective update support (rev2)
== Series Details == Series: Panel replay selective update support (rev2) URL : https://patchwork.freedesktop.org/series/128193/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
[PATCH 6/6] drm: Add CONFIG_DRM_WERROR
Add kconfig to enable -Werror subsystem wide. This is useful for development and CI to keep the subsystem warning free, while avoiding issues outside of the subsystem that kernel wide CONFIG_WERROR=y might hit. Signed-off-by: Jani Nikula --- drivers/gpu/drm/Kconfig | 18 ++ drivers/gpu/drm/Makefile | 3 +++ 2 files changed, 21 insertions(+) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 6ec33d36f3a4..36a00cba2540 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -414,3 +414,21 @@ config DRM_LIB_RANDOM config DRM_PRIVACY_SCREEN bool default n + +config DRM_WERROR + bool "Compile the drm subsystem with warnings as errors" + # As this may inadvertently break the build, only allow the user + # to shoot oneself in the foot iff they aim really hard + depends on EXPERT + # We use the dependency on !COMPILE_TEST to not be enabled in + # allmodconfig or allyesconfig configurations + depends on !COMPILE_TEST + default n + help + A kernel build should not cause any compiler warnings, and this + enables the '-Werror' flag to enforce that rule in the drm subsystem. + + The drm subsystem enables more warnings than the kernel default, so + this config option is disabled by default. + + If in doubt, say N. diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 8b6be830f7c3..b7fd3e58b7af 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -32,6 +32,9 @@ subdir-ccflags-y += -Wno-sign-compare endif # --- end copy-paste +# Enable -Werror in CI and development +subdir-ccflags-$(CONFIG_DRM_WERROR) += -Werror + drm-y := \ drm_aperture.o \ drm_atomic.o \ -- 2.39.2
[PATCH 5/6] drm: enable (most) W=1 warnings by default across the subsystem
At least the i915 and amd drivers enable a bunch more compiler warnings than the kernel defaults. Extend most of the W=1 warnings to the entire drm subsystem by default. Use the copy-pasted warnings from scripts/Makefile.extrawarn with s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and keep up with them in the future. This is similar to the approach currently used in i915. Some of the -Wextra warnings do need to be disabled, just like in Makefile.extrawarn, but take care to not disable them for W=2 or W=3 builds, depending on the warning. There are too many -Wformat-truncation warnings to cleanly fix up front; leave that warning disabled for now. v2: - Drop -Wformat-truncation (too many warnings) - Drop -Wstringop-overflow (enabled by default upstream) Cc: David Airlie Cc: Daniel Vetter Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: Alex Deucher Cc: Christian König Cc: Pan, Xinhui Cc: Karol Herbst Cc: Lyude Paul Cc: Danilo Krummrich Cc: Rob Clark Cc: Abhinav Kumar Cc: Dmitry Baryshkov Cc: Sean Paul Cc: Marijn Suijten Cc: Hamza Mahfooz Acked-by: Javier Martinez Canillas Acked-by: Thomas Zimmermann Acked-by: Sui Jingfeng Signed-off-by: Jani Nikula --- drivers/gpu/drm/Makefile | 27 +++ 1 file changed, 27 insertions(+) diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 104b42df2e95..8b6be830f7c3 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -5,6 +5,33 @@ CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE +# Unconditionally enable W=1 warnings locally +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn +subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter +subdir-ccflags-y += -Wmissing-declarations +subdir-ccflags-y += $(call cc-option, -Wrestrict) +subdir-ccflags-y += -Wmissing-format-attribute +subdir-ccflags-y += -Wmissing-prototypes +subdir-ccflags-y += -Wold-style-definition +subdir-ccflags-y += -Wmissing-include-dirs +subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable) +subdir-ccflags-y += $(call cc-option, -Wunused-const-variable) +subdir-ccflags-y += $(call cc-option, -Wpacked-not-aligned) +subdir-ccflags-y += $(call cc-option, -Wformat-overflow) +# FIXME: fix -Wformat-truncation warnings and uncomment +#subdir-ccflags-y += $(call cc-option, -Wformat-truncation) +subdir-ccflags-y += $(call cc-option, -Wstringop-truncation) +# The following turn off the warnings enabled by -Wextra +ifeq ($(findstring 2, $(KBUILD_EXTRA_WARN)),) +subdir-ccflags-y += -Wno-missing-field-initializers +subdir-ccflags-y += -Wno-type-limits +subdir-ccflags-y += -Wno-shift-negative-value +endif +ifeq ($(findstring 3, $(KBUILD_EXTRA_WARN)),) +subdir-ccflags-y += -Wno-sign-compare +endif +# --- end copy-paste + drm-y := \ drm_aperture.o \ drm_atomic.o \ -- 2.39.2
[PATCH 4/6] drm/imx: prefer snprintf over sprintf
This will trade the W=1 warning -Wformat-overflow to
-Wformat-truncation. This lets us enable -Wformat-overflow subsystem
wide.
Cc: Philipp Zabel
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/imx/ipuv3/imx-ldb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/imx/ipuv3/imx-ldb.c
b/drivers/gpu/drm/imx/ipuv3/imx-ldb.c
index 53840ab054c7..71d70194fcbd 100644
--- a/drivers/gpu/drm/imx/ipuv3/imx-ldb.c
+++ b/drivers/gpu/drm/imx/ipuv3/imx-ldb.c
@@ -655,7 +655,7 @@ static int imx_ldb_probe(struct platform_device *pdev)
for (i = 0; i < 4; i++) {
char clkname[16];
- sprintf(clkname, "di%d_sel", i);
+ snprintf(clkname, sizeof(clkname), "di%d_sel", i);
imx_ldb->clk_sel[i] = devm_clk_get(imx_ldb->dev, clkname);
if (IS_ERR(imx_ldb->clk_sel[i])) {
ret = PTR_ERR(imx_ldb->clk_sel[i]);
--
2.39.2
[PATCH 1/6] drm/nouveau/acr/ga102: remove unused but set variable
Fix the W=1 warning -Wunused-but-set-variable. Cc: Karol Herbst Cc: Lyude Paul Cc: Danilo Krummrich Cc: nouv...@lists.freedesktop.org Signed-off-by: Jani Nikula --- drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c index f36a359d4531..bd104a030243 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c @@ -218,7 +218,7 @@ nvkm_acr_lsfw_load_sig_image_desc_v2(struct nvkm_subdev *subdev, const struct firmware *hsbl; const struct nvfw_ls_hsbl_bin_hdr *hdr; const struct nvfw_ls_hsbl_hdr *hshdr; - u32 loc, sig, cnt, *meta; + u32 sig, cnt, *meta; ret = nvkm_firmware_load_name(subdev, path, "hs_bl_sig", ver, ); if (ret) @@ -227,7 +227,6 @@ nvkm_acr_lsfw_load_sig_image_desc_v2(struct nvkm_subdev *subdev, hdr = nvfw_ls_hsbl_bin_hdr(subdev, hsbl->data); hshdr = nvfw_ls_hsbl_hdr(subdev, hsbl->data + hdr->header_offset); meta = (u32 *)(hsbl->data + hshdr->meta_data_offset); - loc = *(u32 *)(hsbl->data + hshdr->patch_loc); sig = *(u32 *)(hsbl->data + hshdr->patch_sig); cnt = *(u32 *)(hsbl->data + hshdr->num_sig); -- 2.39.2
[PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf
This will trade the W=1 warning -Wformat-overflow to -Wformat-truncation. This lets us enable -Wformat-overflow subsystem wide. Cc: Alex Deucher Cc: Christian König Cc: Pan, Xinhui Cc: amd-...@lists.freedesktop.org Signed-off-by: Jani Nikula --- drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c index b9674c57c436..82b4b2019fca 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c @@ -329,7 +329,8 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev, ring->eop_gpu_addr = kiq->eop_gpu_addr; ring->no_scheduler = true; - sprintf(ring->name, "kiq_%d.%d.%d.%d", xcc_id, ring->me, ring->pipe, ring->queue); + snprintf(ring->name, sizeof(ring->name), "kiq_%d.%d.%d.%d", +xcc_id, ring->me, ring->pipe, ring->queue); r = amdgpu_ring_init(adev, ring, 1024, irq, AMDGPU_CP_KIQ_IRQ_DRIVER0, AMDGPU_RING_PRIO_DEFAULT, NULL); if (r) -- 2.39.2
[PATCH 2/6] drm/nouveau/svm: remove unused but set variables
Fix the W=1 warning -Wunused-but-set-variable.
Cc: Karol Herbst
Cc: Lyude Paul
Cc: Danilo Krummrich
Cc: nouv...@lists.freedesktop.org
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/nouveau/nouveau_svm.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index cc03e0c22ff3..4d1008915499 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -112,7 +112,7 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
{
struct nouveau_cli *cli = nouveau_cli(file_priv);
struct drm_nouveau_svm_bind *args = data;
- unsigned target, cmd, priority;
+ unsigned target, cmd;
unsigned long addr, end;
struct mm_struct *mm;
@@ -136,9 +136,6 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
return -EINVAL;
}
- priority = args->header >> NOUVEAU_SVM_BIND_PRIORITY_SHIFT;
- priority &= NOUVEAU_SVM_BIND_PRIORITY_MASK;
-
/* FIXME support CPU target ie all target value < GPU_VRAM */
target = args->header >> NOUVEAU_SVM_BIND_TARGET_SHIFT;
target &= NOUVEAU_SVM_BIND_TARGET_MASK;
@@ -926,15 +923,14 @@ nouveau_pfns_map(struct nouveau_svmm *svmm, struct
mm_struct *mm,
unsigned long addr, u64 *pfns, unsigned long npages)
{
struct nouveau_pfnmap_args *args = nouveau_pfns_to_args(pfns);
- int ret;
args->p.addr = addr;
args->p.size = npages << PAGE_SHIFT;
mutex_lock(>mutex);
- ret = nvif_object_ioctl(>vmm->vmm.object, args,
- struct_size(args, p.phys, npages), NULL);
+ nvif_object_ioctl(>vmm->vmm.object, args,
+ struct_size(args, p.phys, npages), NULL);
mutex_unlock(>mutex);
}
--
2.39.2
[PATCH 0/6] drm: enable W=1 warnings by default across the subsystem
This is v2 of [1] to enable most W=1 warnings across the drm subsystem. Some fixes first, and a CONFIG_DRM_WERROR on top. I build tested this for x86 (both gcc and clang), arm and arm64. BR, Jani. [1] https://lore.kernel.org/r/20231129181219.1237887-1-jani.nik...@intel.com Jani Nikula (6): drm/nouveau/acr/ga102: remove unused but set variable drm/nouveau/svm: remove unused but set variables drm/amdgpu: prefer snprintf over sprintf drm/imx: prefer snprintf over sprintf drm: enable (most) W=1 warnings by default across the subsystem drm: Add CONFIG_DRM_WERROR drivers/gpu/drm/Kconfig | 18 +++ drivers/gpu/drm/Makefile | 30 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 3 +- drivers/gpu/drm/imx/ipuv3/imx-ldb.c | 2 +- drivers/gpu/drm/nouveau/nouveau_svm.c | 10 ++- .../gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c| 3 +- 6 files changed, 55 insertions(+), 11 deletions(-) -- 2.39.2
Re: disable large folios for shmem file used by xfs xfile
On Wed, Jan 10, 2024 at 12:37:18PM +, Matthew Wilcox wrote: > On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote: > > Hi all, > > > > Darrick reported that the fairly new XFS xfile code blows up when force > > enabling large folio for shmem. This series fixes this quickly by disabling > > large folios for this particular shmem file for now until it can be fixed > > properly, which will be a lot more invasive. > > > > I've added most of you to the CC list as I suspect most other users of > > shmem_file_setup and friends will have similar issues. > > The graphics users _want_ to use large folios. I'm pretty sure they've > been tested with this. It's just XFS that didn't know about this > feature of shmem. At least sgx has all kinds of PAGE_SIZE assumptions. I haven't audited (and am probably not qualified to) audit that code, so I wanted to send a headsup to everyone.
Re: disable large folios for shmem file used by xfs xfile
On Wed, Jan 10, 2024 at 07:38:43AM -0800, Andrew Morton wrote:
> I assume that kernels which contain 137db333b29186 ("xfs: teach xfile
> to pass back direct-map pages to caller") want this, so a Fixes: that
> and a cc:stable are appropriate?
I think it needs to back all the way back to 3934e8ebb7c
("xfs: create a big array data structure") as that only clears the page
sized chunk of a new allocation and could lead to corruption / or
information leaks.
Re: [PATCH 3/3] drm/i915: Pin error_capture to high end of mappable
On 15.12.2023 12:09, Ville Syrjala wrote:
From: Ville Syrjälä
If we fail to pin error_capture to the start of ggtt (which
is likely given the BIOS fb is usually there), we currently
fall back to pinning it at the next available low address.
This seems somewhat sub-optimal to me in case we later discard
the BIOS fb (fairly likely if there are multiple different sized
displays connected at boot). We are then then left with a
permanenly pinned object somewhere in the middle of the mappable
range of ggtt. It seems more sensible to pin the error capture
object to the end of mappable as a fallback, so even if we discard
the BIOS fb we are left with the mappable region mostly in one
piece (potentially allowing for more/larger objects to be pinned
there later).
Though I suppose we are chopping the ggtt address space as a
whole into two chunks in a slightly different way. Essentially
reducing the size of the second (larger) chunk a bit. So perhaps
pinning truly massive objects (which don't strictly need to
be mappable) could become a bit more difficult.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 21a7e3191c18..f62008962eb5 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -876,7 +876,7 @@ static int init_ggtt(struct i915_ggtt *ggtt)
ggtt->error_capture.size, 0,
ggtt->error_capture.color,
0, ggtt->mappable_end,
- DRM_MM_INSERT_LOW);
+ DRM_MM_INSERT_HIGH);
In this case Chris raised concern that since error_capture must be
mappable we will end up with worse fragmentation in case of
DRM_MM_INSERT_HIGH.
Also see the comment above the change (my concern):
* We strongly prefer taking address 0x0 in order to protect
* other critical buffers against accidental overwrites,
* as writing to address 0 is a very common mistake.
Maybe the ultimate solution would be to relocate BIOS fb vma also if it
is at 0, I don't know.
Regards
Andrzej
}
if (drm_mm_node_allocated(>error_capture)) {
u64 start = ggtt->error_capture.start;
Re: [PATCH 2/3] drm/i915/hdcp: Pin the hdcp gsc message high in ggtt
On 15.12.2023 12:09, Ville Syrjala wrote: From: Ville Syrjälä AFAICS there is no hardware restriction on where in ggtt the hdcp gsc message object needs to be bound. And as it's a regular shmem object we don't need it be in the mappabe range either. So pin it high to make avoid needlessly wasting the precious mappable range for it. Cc: Suraj Kandpal Cc: Alan Previn Cc: Uma Shankar Signed-off-by: Ville Syrjälä Reviewed-by: Andrzej Hajda Regards Andrzej --- drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c index 18117b789b16..302bff75b06c 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c @@ -65,7 +65,7 @@ static int intel_hdcp_gsc_initialize_message(struct drm_i915_private *i915, goto out_unmap; } - err = i915_vma_pin(vma, 0, 0, PIN_GLOBAL); + err = i915_vma_pin(vma, 0, 0, PIN_GLOBAL | PIN_HIGH); if (err) goto out_unmap;
Re: disable large folios for shmem file used by xfs xfile
On Wed, 10 Jan 2024 10:21:07 +0100 Christoph Hellwig wrote:
> Hi all,
>
> Darrick reported that the fairly new XFS xfile code blows up when force
> enabling large folio for shmem. This series fixes this quickly by disabling
> large folios for this particular shmem file for now until it can be fixed
> properly, which will be a lot more invasive.
>
Patches seems reasonable as a short-term only-affects-xfs thing.
I assume that kernels which contain 137db333b29186 ("xfs: teach xfile
to pass back direct-map pages to caller") want this, so a Fixes: that
and a cc:stable are appropriate?
Re: disable large folios for shmem file used by xfs xfile
On Wed, Jan 10, 2024 at 05:28:22PM +0200, Joonas Lahtinen wrote: > Quoting Joonas Lahtinen (2024-01-10 17:20:24) > > However we specifically pass "huge=within_size" to vfs_kern_mount when > > creating a private mount of tmpfs for the purpose of i915 created > > allocations. > > > > Older hardware also had some address hashing bugs where 2M aligned > > memory caused a lot of collisions in TLB so we don't enable it always. > > > > You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function > > i915_gemfs_init for details and references. > > > > So in short, functionality wise we should be fine either default > > for using 2M pages or not. If they become the default, we'd probably > > want an option that would still be able to prevent them for performance > > regression reasons on older hardware. > > To maybe write out my concern better: > > Is there plan to enable huge pages by default in shmem? Not in the next kernel release, but eventually the plan is to allow arbitrary order folios to be used in shmem. So you could ask it to create a 256kB folio for you, if that's the right size to manage memory in. How shmem and its various users go about choosing the right size is not quite clear to me yet. Perhaps somebody else will do it before I get to it; I have a lot of different sub-projects to work on at the moment, and shmem isn't blocking any of them. And I have a sneaking suspicion that more work is needed in the swap code to deal with arbitrary order folios, so that's another reason for me to delay looking at this ;-)
Re: [PATCH 1/3] drm/i915/hdcp: Do intel_hdcp_component_init() much later during init
On 15.12.2023 12:09, Ville Syrjala wrote: From: Ville Syrjälä intel_hdcp_component_init()->...->intel_hdcp_gsc_initialize_message() will allocate ggtt address space for some hdcp gsc message thing. That is currently being done way too early as we haven't even taken over the BIOS fb yet. So this has the potential of corrupting ggtt PTEs that need to be preserved until the the BIOS fb takover is done. Only call intel_hdcp_component_init() once all the BIOS fb takeover, and full ggtt init (which currently also needs to reserve very specific ranges of ggtt, thus assuming that no one else has stolen them yet) is done. Cc: Suraj Kandpal Cc: Alan Previn Cc: Uma Shankar Signed-off-by: Ville Syrjälä Reviewed-by: Andrzej Hajda Regards Andrzej --- drivers/gpu/drm/i915/display/intel_display_driver.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c index 62f7b10484be..b71338b4d793 100644 --- a/drivers/gpu/drm/i915/display/intel_display_driver.c +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c @@ -319,8 +319,6 @@ int intel_display_driver_probe_nogem(struct drm_i915_private *i915) intel_display_driver_init_hw(i915); intel_dpll_update_ref_clks(i915); - intel_hdcp_component_init(i915); - if (i915->display.cdclk.max_cdclk_freq == 0) intel_update_max_cdclk(i915); @@ -360,6 +358,13 @@ int intel_display_driver_probe(struct drm_i915_private *i915) if (!HAS_DISPLAY(i915)) return 0; + /* +* This will bind stuff into ggtt, so it needs to be done after +* the BIOS fb takeover and whatever else magic ggtt reservations +* happen during gem/ggtt init. +*/ + intel_hdcp_component_init(i915); + /* * Force all active planes to recompute their states. So that on * mode_setcrtc after probe, all the intel_plane_state variables
Re: disable large folios for shmem file used by xfs xfile
Quoting Joonas Lahtinen (2024-01-10 17:20:24) > Quoting Matthew Wilcox (2024-01-10 14:37:18) > > On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote: > > > Hi all, > > > > > > Darrick reported that the fairly new XFS xfile code blows up when force > > > enabling large folio for shmem. This series fixes this quickly by > > > disabling > > > large folios for this particular shmem file for now until it can be fixed > > > properly, which will be a lot more invasive. > > > > > > I've added most of you to the CC list as I suspect most other users of > > > shmem_file_setup and friends will have similar issues. > > > > The graphics users _want_ to use large folios. I'm pretty sure they've > > been tested with this. > > Correct. We've done quite a bit of optimization in userspace and > enabling in kernel to take advantage of page sizes of 2M and beyond. > > However we specifically pass "huge=within_size" to vfs_kern_mount when > creating a private mount of tmpfs for the purpose of i915 created > allocations. > > Older hardware also had some address hashing bugs where 2M aligned > memory caused a lot of collisions in TLB so we don't enable it always. > > You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function > i915_gemfs_init for details and references. > > So in short, functionality wise we should be fine either default > for using 2M pages or not. If they become the default, we'd probably > want an option that would still be able to prevent them for performance > regression reasons on older hardware. To maybe write out my concern better: Is there plan to enable huge pages by default in shmem? If not I guess we should be pretty good with the way current code is, force enabling them just might bring out some performance, so we might want to add a warning for that. If there is, then we'll probably want to in sync with those default changes apply a similar call to block them on older HW. Regards, Joonas > > Regards, Joonas > > > It's just XFS that didn't know about this > > feature of shmem.
✓ Fi.CI.BAT: success for drm/xe: ensure fbc cfb size to be page size aligned
== Series Details == Series: drm/xe: ensure fbc cfb size to be page size aligned URL : https://patchwork.freedesktop.org/series/128425/ State : success == Summary == CI Bug Log - changes from CI_DRM_14108 -> Patchwork_128425v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/index.html Participating hosts (38 -> 39) -- Additional (2): bat-dg2-8 bat-mtlp-8 Missing(1): fi-snb-2520m Known issues Here are the changes found in Patchwork_128425v1 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_14108/bat-jsl-1/boot.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/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_128425v1/bat-mtlp-8/igt@debugfs_t...@basic-hwmon.html * igt@gem_lmem_swapping@verify-random: - bat-mtlp-8: NOTRUN -> [SKIP][4] ([i915#4613]) +3 other tests skip [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html * igt@gem_mmap@basic: - bat-mtlp-8: NOTRUN -> [SKIP][5] ([i915#4083]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-mtlp-8/igt@gem_m...@basic.html - bat-dg2-8: NOTRUN -> [SKIP][6] ([i915#4083]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-dg2-8/igt@gem_m...@basic.html * igt@gem_mmap_gtt@basic: - bat-mtlp-8: NOTRUN -> [SKIP][7] ([i915#4077]) +2 other tests skip [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-mtlp-8/igt@gem_mmap_...@basic.html - bat-dg2-8: NOTRUN -> [SKIP][8] ([i915#4077]) +2 other tests skip [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-dg2-8/igt@gem_mmap_...@basic.html * igt@gem_render_tiled_blits@basic: - bat-mtlp-8: NOTRUN -> [SKIP][9] ([i915#4079]) +1 other test skip [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-mtlp-8/igt@gem_render_tiled_bl...@basic.html * igt@gem_tiled_pread_basic: - bat-dg2-8: NOTRUN -> [SKIP][10] ([i915#4079]) +1 other test skip [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-dg2-8/igt@gem_tiled_pread_basic.html * igt@i915_pm_rps@basic-api: - bat-mtlp-8: NOTRUN -> [SKIP][11] ([i915#6621]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-mtlp-8/igt@i915_pm_...@basic-api.html - bat-dg2-8: NOTRUN -> [SKIP][12] ([i915#6621]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-dg2-8/igt@i915_pm_...@basic-api.html * igt@i915_suspend@basic-s3-without-i915: - bat-mtlp-8: NOTRUN -> [SKIP][13] ([i915#6645]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-mtlp-8/igt@i915_susp...@basic-s3-without-i915.html - bat-dg2-8: NOTRUN -> [SKIP][14] ([i915#6645]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-dg2-8/igt@i915_susp...@basic-s3-without-i915.html * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy: - bat-mtlp-8: NOTRUN -> [SKIP][15] ([i915#5190]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-mtlp-8/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html - bat-dg2-8: NOTRUN -> [SKIP][16] ([i915#5190]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-dg2-8/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html * igt@kms_addfb_basic@basic-y-tiled-legacy: - bat-mtlp-8: NOTRUN -> [SKIP][17] ([i915#4212]) +8 other tests skip [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-mtlp-8/igt@kms_addfb_ba...@basic-y-tiled-legacy.html - bat-dg2-8: NOTRUN -> [SKIP][18] ([i915#4215] / [i915#5190]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-dg2-8/igt@kms_addfb_ba...@basic-y-tiled-legacy.html * igt@kms_addfb_basic@framebuffer-vs-set-tiling: - bat-dg2-8: NOTRUN -> [SKIP][19] ([i915#4212]) +7 other tests skip [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128425v1/bat-dg2-8/igt@kms_addfb_ba...@framebuffer-vs-set-tiling.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_128425v1/bat-mtlp-8/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html - bat-dg2-8: NOTRUN -> [SKIP][21] ([i915#4103] /
Re: disable large folios for shmem file used by xfs xfile
Quoting Matthew Wilcox (2024-01-10 14:37:18) > On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote: > > Hi all, > > > > Darrick reported that the fairly new XFS xfile code blows up when force > > enabling large folio for shmem. This series fixes this quickly by disabling > > large folios for this particular shmem file for now until it can be fixed > > properly, which will be a lot more invasive. > > > > I've added most of you to the CC list as I suspect most other users of > > shmem_file_setup and friends will have similar issues. > > The graphics users _want_ to use large folios. I'm pretty sure they've > been tested with this. Correct. We've done quite a bit of optimization in userspace and enabling in kernel to take advantage of page sizes of 2M and beyond. However we specifically pass "huge=within_size" to vfs_kern_mount when creating a private mount of tmpfs for the purpose of i915 created allocations. Older hardware also had some address hashing bugs where 2M aligned memory caused a lot of collisions in TLB so we don't enable it always. You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function i915_gemfs_init for details and references. So in short, functionality wise we should be fine either default for using 2M pages or not. If they become the default, we'd probably want an option that would still be able to prevent them for performance regression reasons on older hardware. Regards, Joonas > It's just XFS that didn't know about this > feature of shmem.
✗ Fi.CI.SPARSE: warning for drm/xe: ensure fbc cfb size to be page size aligned
== Series Details == Series: drm/xe: ensure fbc cfb size to be page size aligned URL : https://patchwork.freedesktop.org/series/128425/ 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:147:1: warning: unreplaced symbol 'return' +./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:155:16: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:155:9: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:173:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:175:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:179:35: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:181:16: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:181:9: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:185:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:187:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:191:35: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:194:16: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:194:9: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:236:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:238:9: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:66:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:92:1: warning: unreplaced symbol 'return' +./drivers/gpu/drm/i915/intel_uncore.h:346:1: warning: trying to copy expression type 31 +./drivers/gpu/drm/i915/intel_uncore.h:351:1: warning: trying to copy expression type 31 +./include/asm-generic/bitops/generic-non-atomic.h:100:17: warning: unreplaced symbol 'old' +./include/asm-generic/bitops/generic-non-atomic.h:100:23: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:100:9: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:105:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:107:9: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:108:9: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:109:9: warning: unreplaced symbol 'old' +./include/asm-generic/bitops/generic-non-atomic.h:111:10: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:111:14: warning: unreplaced symbol 'old' +./include/asm-generic/bitops/generic-non-atomic.h:111:20: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:112:17: warning: unreplaced symbol 'old' +./include/asm-generic/bitops/generic-non-atomic.h:112:23: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:112:9: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:121:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:128:9: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:166:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:168:9: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:169:9: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:170:9: warning: unreplaced symbol 'val' +./include/asm-generic/bitops/generic-non-atomic.h:172:19: warning: unreplaced symbol 'val' +./include/asm-generic/bitops/generic-non-atomic.h:172:25: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:172:9: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:28:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:30:9: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:31:9: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:33:10: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:33:16: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:37:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:39:9: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:40:9: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:42:10: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:42:16: warning: unreplaced symbol 'mask'
✗ Fi.CI.BAT: failure for series starting with [1/2] mm: add a mapping_clear_large_folios helper
== Series Details == Series: series starting with [1/2] mm: add a mapping_clear_large_folios helper URL : https://patchwork.freedesktop.org/series/128420/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14108 -> Patchwork_128420v1 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_128420v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_128420v1, please notify your bug team (i915-ci-in...@lists.freedesktop.org) to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/index.html Participating hosts (38 -> 38) -- Additional (1): bat-mtlp-8 Missing(1): fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_128420v1: ### CI changes ### Possible regressions * boot: - bat-rpls-2: [PASS][1] -> [FAIL][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14108/bat-rpls-2/boot.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-rpls-2/boot.html Known issues Here are the changes found in Patchwork_128420v1 that come from known issues: ### CI changes ### Issues hit * boot: - fi-bsw-n3050: [PASS][3] -> [FAIL][4] ([i915#8293]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14108/fi-bsw-n3050/boot.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/fi-bsw-n3050/boot.html ### IGT changes ### Issues hit * igt@debugfs_test@basic-hwmon: - bat-mtlp-8: NOTRUN -> [SKIP][5] ([i915#9318]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@debugfs_t...@basic-hwmon.html * igt@gem_lmem_swapping@verify-random: - bat-mtlp-8: NOTRUN -> [SKIP][6] ([i915#4613]) +3 other tests skip [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html * igt@gem_mmap@basic: - bat-mtlp-8: NOTRUN -> [SKIP][7] ([i915#4083]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@gem_m...@basic.html * igt@gem_mmap_gtt@basic: - bat-mtlp-8: NOTRUN -> [SKIP][8] ([i915#4077]) +2 other tests skip [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@gem_mmap_...@basic.html * igt@gem_render_tiled_blits@basic: - bat-mtlp-8: NOTRUN -> [SKIP][9] ([i915#4079]) +1 other test skip [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@gem_render_tiled_bl...@basic.html * igt@i915_pm_rps@basic-api: - bat-mtlp-8: NOTRUN -> [SKIP][10] ([i915#6621]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@i915_pm_...@basic-api.html * igt@i915_suspend@basic-s3-without-i915: - bat-mtlp-8: NOTRUN -> [SKIP][11] ([i915#6645]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/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][12] ([i915#5190]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/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][13] ([i915#4212]) +8 other tests skip [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/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][14] ([i915#4213]) +1 other test skip [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html * igt@kms_dsc@dsc-basic: - bat-mtlp-8: NOTRUN -> [SKIP][15] ([i915#3555] / [i915#3840] / [i915#9159]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@kms_...@dsc-basic.html * igt@kms_force_connector_basic@force-load-detect: - bat-mtlp-8: NOTRUN -> [SKIP][16] ([fdo#109285]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@kms_force_connector_ba...@force-load-detect.html * igt@kms_force_connector_basic@prune-stale-modes: - bat-mtlp-8: NOTRUN -> [SKIP][17] ([i915#5274]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@kms_force_connector_ba...@prune-stale-modes.html * igt@kms_pipe_crc_basic@suspend-read-crc: - bat-atsm-1: NOTRUN -> [SKIP][18] ([i915#1836]) [18]:
Re: [PATCH 5/7] drm/uAPI: Add "preferred color format" drm property as setting for userspace
Hi, Am 10.01.24 um 14:42 schrieb Andri Yngvason: mið., 10. jan. 2024 kl. 13:09 skrifaði Werner Sembach: Hi, Am 10.01.24 um 11:11 schrieb Andri Yngvason: Hi, mið., 10. jan. 2024 kl. 09:27 skrifaði Maxime Ripard: On Tue, Jan 09, 2024 at 06:11:02PM +, Andri Yngvason wrote: From: Werner Sembach Add a new general drm property "preferred color format" which can be used by userspace to tell the graphic drivers to which color format to use. Possible options are: - auto (default/current behaviour) - rgb - ycbcr444 - ycbcr422 (not supported by both amdgpu and i915) - ycbcr420 In theory the auto option should choose the best available option for the current setup, but because of bad internal conversion some monitors look better with rgb and some with ycbcr444. I looked at the patch and I couldn't find what is supposed to happen if you set it to something else than auto, and the driver can't match that. Are we supposed to fallback to the "auto" behaviour, or are we suppose to reject the mode entirely? The combination with the active output format property suggests the former, but we should document it explicitly. It is also my understanding that it should fall back to the "auto" behaviour. I will add this to the documentation. Yes, that was the intention, and then userspace can check, but it wasn't well received:https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_964530 Actually a lot of the thoughts that went into the original patch set can be found in that topic. There was another iteration of the patch set that I never finished and sent to the LKML because I got discouraged by this: https://lore.kernel.org/dri-devel/20210623102923.70877c1a@eldfell/ Well, I've implemented this for sway and wlroots now and Simon has reacted positively, so this does appear likely to end up as a feature in wlroots based compositors. I can try to dig it up, but it is completely untested and I don't think I still have the respective TODO list anymore, so I don't know if it is a better or worst starting point than the last iteration I sent to the LKML. You can send the patches to me if you want and I can see if they're useful. I'm really only interested in the color format part though. Alternatively, you can continue your work and post it to LKML and I can focus on the userspace side and testing. By the way, I have an HDMI analyzer that can tell me the actual color format. Searched for what I still had in my private repo, see attachments, filename is the branch name I used and like I said: I don't know which state these branches are in. The hacking_ branch was based on 25fe90f43fa312213b653dc1f12fd2d80f855883 from linux-next and the rejected_ one on 132b189b72a94328f17fd70321bfe63e5b4208e9 from drm-tip. And the rejected_ one is 2 weeks newer. To pick it up again I would first need to allocate some time for it, ... which could take some time. With a HDMI analyzer at hand you are better equipped then me already. I was working with printf statements, Monitor OSD's and test patterns like https://media.extron.com/public/technology/landing/vector4k/img/scalfe-444Chroma.png and http://www.lagom.nl/lcd-test/ while being red blind xD. Thanks, Andri hacking_drm_color_management_no_immutable_flag.tar.gz Description: application/gzip rejected_drm_color_management.tar.gz Description: application/gzip
✗ Fi.CI.SPARSE: warning for series starting with [1/2] mm: add a mapping_clear_large_folios helper
== Series Details == Series: series starting with [1/2] mm: add a mapping_clear_large_folios helper URL : https://patchwork.freedesktop.org/series/128420/ 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/2] mm: add a mapping_clear_large_folios helper
== Series Details == Series: series starting with [1/2] mm: add a mapping_clear_large_folios helper URL : https://patchwork.freedesktop.org/series/128420/ State : warning == Summary == Error: dim checkpatch failed e1a8add4aba0 mm: add a mapping_clear_large_folios helper 0faf35dc1522 xfs: disable large folio support in xfile_create -:14: WARNING:BAD_REPORTED_BY_LINK: Reported-by: should be immediately followed by Closes: with a URL to the report #14: Reported-by: Darrick J. Wong Signed-off-by: Christoph Hellwig total: 0 errors, 1 warnings, 0 checks, 11 lines checked
Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace
Hi,
Am 09.01.24 um 19:10 schrieb Andri Yngvason:
From: Werner Sembach
Add a new general drm property "active color format" which can be used by
graphic drivers to report the used color format back to userspace.
There was no way to check which color format got actually used on a given
monitor. To surely predict this, one must know the exact capabilities of
the monitor, the GPU, and the connection used and what the default
behaviour of the used driver is (e.g. amdgpu prefers YCbCr 4:4:4 while i915
prefers RGB). This property helps eliminating the guessing on this point.
In the future, automatic color calibration for screens might also depend on
this information being available.
Signed-off-by: Werner Sembach
Signed-off-by: Andri Yngvason
Tested-by: Andri Yngvason
One suggestion from back then was, instead picking out singular properties like
"active color format", to just expose whatever the last HDMI or DP metadata
block(s)/frame(s) that was sent over the display wire was to userspace and
accompanying it with a parsing script.
Question: Does the driver really actually know what the GPU is ultimatively
sending out the wire, or is that decided by a final firmware blob we have no
info about?
Greetings
Werner
---
drivers/gpu/drm/drm_connector.c | 63 +
include/drm/drm_connector.h | 10 ++
2 files changed, 73 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index c3725086f4132..30d62e505d188 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1061,6 +1061,14 @@ static const struct drm_prop_enum_list
drm_dp_subconnector_enum_list[] = {
{ DRM_MODE_SUBCONNECTOR_Native, "Native"}, /* DP */
};
+static const struct drm_prop_enum_list drm_active_color_format_enum_list[] = {
+ { 0, "not applicable" },
+ { DRM_COLOR_FORMAT_RGB444, "rgb" },
+ { DRM_COLOR_FORMAT_YCBCR444, "ycbcr444" },
+ { DRM_COLOR_FORMAT_YCBCR422, "ycbcr422" },
+ { DRM_COLOR_FORMAT_YCBCR420, "ycbcr420" },
+};
+
DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name,
drm_dp_subconnector_enum_list)
@@ -1390,6 +1398,15 @@ static const u32 dp_colorspaces =
*drm_connector_attach_max_bpc_property() to create and attach the
*property to the connector during initialization.
*
+ * 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".
+ *
* Connectors also have one standardized atomic property:
*
* CRTC_ID:
@@ -2451,6 +2468,52 @@ int drm_connector_attach_max_bpc_property(struct
drm_connector *connector,
}
EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
+/**
+ * drm_connector_attach_active_color_format_property - attach "active color
format" property
+ * @connector: connector to attach active color format property on.
+ *
+ * This is used to check the applied color format on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_active_color_format_property(struct drm_connector
*connector)
+{
+ struct drm_device *dev = connector->dev;
+ struct drm_property *prop;
+
+ if (!connector->active_color_format_property) {
+ prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
"active color format",
+
drm_active_color_format_enum_list,
+
ARRAY_SIZE(drm_active_color_format_enum_list));
+ if (!prop)
+ return -ENOMEM;
+
+ connector->active_color_format_property = prop;
+ }
+
+ drm_object_attach_property(>base, prop, 0);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_active_color_format_property);
+
+/**
+ * drm_connector_set_active_color_format_property - sets the active color
format property for a
+ * connector
+ * @connector: drm connector
+ * @active_color_format: color format for the connector currently active "on the
cable"
+ *
+ * Should be used by atomic drivers to update the active color format over a
connector.
+ */
+void drm_connector_set_active_color_format_property(struct drm_connector
*connector,
+ u32 active_color_format)
+{
+ drm_object_property_set_value(>base,
connector->active_color_format_property,
+ active_color_format);
+}
+EXPORT_SYMBOL(drm_connector_set_active_color_format_property);
+
/**
*
Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace
Hi, On Wed, 10 Jan 2024 at 10:44, Daniel Vetter wrote: > On Tue, Jan 09, 2024 at 11:12:11PM +, Andri Yngvason wrote: > > ţri., 9. jan. 2024 kl. 22:32 skrifađi Daniel Stone : > > > 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? > > > > > > > Userspace does not determine what's happened without polling. The purpose > > of this property is not for programmatic verification that the preferred > > property was applied. It is my understanding that it's mostly intended for > > debugging purposes. It should only change as a consequence of modesetting, > > although I didn't actually look into what happens if you set the "preferred > > color format" outside of a modeset. > > This feels a bit irky to me, since we don't have any synchronization and > it kinda breaks how userspace gets to know about stuff. > > For context the current immutable properties are all stuff that's derived > from the sink (like edid, or things like that). Userspace is guaranteed to > get a hotplug event (minus driver bugs as usual) if any of these change, > and we've added infrastructure so that the hotplug event even contains the > specific property so that userspace can avoid re-read (which can cause > some costly re-probing) them all. Right. > [...] > > This thing here works entirely differently, and I think we need somewhat > new semantics for this: > > - I agree it should be read-only for userspace, so immutable sounds right. > > - But I also agree with Daniel Stone that this should be tied more > directly to the modeset state. > > So I think the better approach would be to put the output type into > drm_connector_state, require that drivers compute it in their > ->atomic_check code (which in the future would allow us to report it out > for TEST_ONLY commits too), and so guarantee that the value is updated > right after the kms ioctl returns (and not somewhen later for non-blocking > commits). That's exactly the point. Whether userspace gets an explicit notification or it has to 'know' when to read isn't much of an issue - just as long as it's well defined. I think the suggestion of 'do it in atomic_check and then it's guaranteed to be readable when the commit completes' is a good one. I do still have some reservations - for instance, why do we have the fallback to auto when userspace has explicitly requested a certain type? - but they may have been covered previously. Cheers, Daniel
Re: [PATCH 10/12] drm/panelreplay: dpcd register definition for panelreplay SU
On Thu, 2024-01-04 at 12:59 +0200, Dmitry Baryshkov wrote: > On Thu, 4 Jan 2024 at 12:49, Jouni Högander > wrote: > > > > Add definitions for panel replay selective update > > > > Cc: dri-de...@lists.freedesktop.org > > > > 1) This CC should not be necessary. It is already a part of > maintainers entry for this file > > 2) It probably doesn't work as expected. It is separated with the > blank link from the rest of the trailers, so most of the tools will > skip it. > > 3) You have skipped the rest of the maintainers for this file. Please > use ./scripts/get_maintainers.pl and pass corresponding options to > git > send-email. Thank you Dmitry for checking my patch. Sent now version 2. of my patch set. There I took care of your suggestions in patch 11. BR, Jouni Högander > > > Signed-off-by: Jouni Högander > > --- > > include/drm/display/drm_dp.h | 6 ++ > > 1 file changed, 6 insertions(+) > > The patch itself looks good to me. > > > > > diff --git a/include/drm/display/drm_dp.h > > b/include/drm/display/drm_dp.h > > index 3731828825bd..6a59d30b7b25 100644 > > --- a/include/drm/display/drm_dp.h > > +++ b/include/drm/display/drm_dp.h > > @@ -548,6 +548,12 @@ > > # define DP_PANEL_REPLAY_SUPPORT (1 << 0) > > # define DP_PANEL_REPLAY_SU_SUPPORT (1 << 1) > > > > +#define DP_PANEL_PANEL_REPLAY_CAPABILITY 0xb1 > > +# define DP_PANEL_PANEL_REPLAY_SU_GRANULARITY_REQUIRED (1 << 5) > > + > > +#define DP_PANEL_PANEL_REPLAY_X_GRANULARITY 0xb2 > > +#define DP_PANEL_PANEL_REPLAY_Y_GRANULARITY 0xb4 > > + > > /* Link Configuration */ > > #define DP_LINK_BW_SET 0x100 > > # define DP_LINK_RATE_TABLE 0x00 /* eDP 1.4 */ > > -- > > 2.34.1 > > > >
Re: [PATCH 5/7] drm/uAPI: Add "preferred color format" drm property as setting for userspace
Hi, Am 10.01.24 um 11:11 schrieb Andri Yngvason: Hi, mið., 10. jan. 2024 kl. 09:27 skrifaði Maxime Ripard : On Tue, Jan 09, 2024 at 06:11:02PM +, Andri Yngvason wrote: From: Werner Sembach Add a new general drm property "preferred color format" which can be used by userspace to tell the graphic drivers to which color format to use. Possible options are: - auto (default/current behaviour) - rgb - ycbcr444 - ycbcr422 (not supported by both amdgpu and i915) - ycbcr420 In theory the auto option should choose the best available option for the current setup, but because of bad internal conversion some monitors look better with rgb and some with ycbcr444. I looked at the patch and I couldn't find what is supposed to happen if you set it to something else than auto, and the driver can't match that. Are we supposed to fallback to the "auto" behaviour, or are we suppose to reject the mode entirely? The combination with the active output format property suggests the former, but we should document it explicitly. It is also my understanding that it should fall back to the "auto" behaviour. I will add this to the documentation. Yes, that was the intention, and then userspace can check, but it wasn't well received: https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_964530 Actually a lot of the thoughts that went into the original patch set can be found in that topic. There was another iteration of the patch set that I never finished and sent to the LKML because I got discouraged by this: https://lore.kernel.org/dri-devel/20210623102923.70877c1a@eldfell/ I can try to dig it up, but it is completely untested and I don't think I still have the respective TODO list anymore, so I don't know if it is a better or worst starting point than the last iteration I sent to the LKML. Greetings Werner Thanks, Andri
Re: [PATCH 3/7] drm/amd/display: Add handling for new "active color format" property
Hi,
Am 10.01.24 um 14:09 schrieb Daniel Vetter:
On Wed, 10 Jan 2024 at 13:53, Andri Yngvason wrote:
mið., 10. jan. 2024 kl. 11:10 skrifaði Daniel Vetter :
On Tue, Jan 09, 2024 at 06:11:00PM +, Andri Yngvason wrote:
+ /* Extract information from crtc to communicate it to userspace as
connector properties */
+ for_each_new_connector_in_state(state, connector, new_con_state, i) {
+ struct drm_crtc *crtc = new_con_state->crtc;
+ struct dc_stream_state *stream;
+
+ if (crtc) {
+ new_crtc_state = drm_atomic_get_new_crtc_state(state,
crtc);
+ dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+ stream = dm_new_crtc_state->stream;
+
+ if (stream) {
+
drm_connector_set_active_color_format_property(connector,
+
convert_dc_pixel_encoding_into_drm_color_format(
+
dm_new_crtc_state->stream->timing.pixel_encoding));
+ }
+ } else {
+ drm_connector_set_active_color_format_property(connector,
0);
Just realized an even bigger reason why your current design doesn't work:
You don't have locking here.
And you cannot grab the required lock, which is
drm_dev->mode_config.mutex, because that would result in deadlocks. So
this really needs to use the atomic state based design I've described.
Maybe we should just drop "actual color format" and instead fail the
modeset if the "preferred color format" property cannot be satisfied?
It seems like the simplest thing to do here, though it is perhaps less
convenient for userspace. In that case, the "preferred color format"
property should just be called "color format".
Yeah that's more in line with how other atomic properties work. This
way userspace can figure out what works with a TEST_ONLY commit too.
And for this to work you probably want to have an "automatic" setting
too.
-Sima
The problem with TEST_ONLY probing is that color format settings are
interdependent: https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_966634
So changing any other setting may require every color format to be TEST_ONLY
probed again.
Greetings
Werner
[PATCH v2 13/13] drm/i915/psr: Add panel replay sel update support to debugfs interface
Add panel replay selective update support to debugfs status interface. In case of sink supporting panel replay we will print out: Sink support: PSR = no, Panel Replay = yes, Panel Replay Selective Update = yes and PSR mode will look like this if printing out enabled panel replay selective update: PSR mode: Panel Replay Selective Update Enabled Current PSR and panel replay printouts remain same. Cc: Kunal Joshi Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_psr.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 9f5d04261df0..b7a29bdcf668 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -3222,7 +3222,9 @@ static int intel_psr_status(struct seq_file *m, struct intel_dp *intel_dp) if (psr->sink_support) seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]); - seq_printf(m, ", Panel Replay = %s\n", str_yes_no(psr->sink_panel_replay_support)); + seq_printf(m, ", Panel Replay = %s", str_yes_no(psr->sink_panel_replay_support)); + seq_printf(m, ", Panel Replay Selective Update = %s\n", + str_yes_no(psr->sink_panel_replay_su_support)); if (!(psr->sink_support || psr->sink_panel_replay_support)) return 0; @@ -3231,9 +3233,10 @@ static int intel_psr_status(struct seq_file *m, struct intel_dp *intel_dp) mutex_lock(>lock); if (psr->panel_replay_enabled) - status = "Panel Replay Enabled"; + status = psr->sel_update_enabled ? "Panel Replay Selective Update Enabled" : + "Panel Replay Enabled"; else if (psr->enabled) - status = psr->sel_update_enabled ? "PSR2 enabled" : "PSR1 enabled"; + status = psr->sel_update_enabled ? "PSR2" : "PSR1"; else status = "disabled"; seq_printf(m, "PSR mode: %s\n", status); -- 2.34.1
[PATCH v2 12/13] drm/i915/psr: Modify intel_dp_get_su_granularity to support panel replay
Currently intel_dp_get_su_granularity doesn't support panel replay.
This fix modifies it to support panel replay as well.
Signed-off-by: Jouni Högander
---
drivers/gpu/drm/i915/display/intel_psr.c | 59 +---
1 file changed, 53 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
b/drivers/gpu/drm/i915/display/intel_psr.c
index 68da1f284fae..9f5d04261df0 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -465,6 +465,42 @@ static u8 intel_dp_get_sink_sync_latency(struct intel_dp
*intel_dp)
return val;
}
+static u8 intel_dp_get_su_capability(struct intel_dp *intel_dp)
+{
+ u8 su_capability;
+
+ if (intel_dp->psr.sink_panel_replay_su_support)
+ drm_dp_dpcd_read(_dp->aux, DP_PSR2_SU_X_GRANULARITY,
+_capability, 1);
+ else
+ su_capability = intel_dp->psr_dpcd[1];
+
+ return su_capability;
+}
+
+static u8 intel_dp_get_su_ganularity_required_bit(struct intel_dp *intel_dp)
+{
+ return intel_dp->psr.sink_panel_replay_su_support ?
+ DP_PANEL_PANEL_REPLAY_SU_GRANULARITY_REQUIRED :
+ DP_PSR2_SU_GRANULARITY_REQUIRED;
+}
+
+static unsigned int
+intel_dp_get_su_x_granularity_offset(struct intel_dp *intel_dp)
+{
+ return intel_dp->psr.sink_panel_replay_su_support ?
+ DP_PANEL_PANEL_REPLAY_X_GRANULARITY :
+ DP_PSR2_SU_X_GRANULARITY;
+}
+
+static unsigned int
+intel_dp_get_su_y_granularity_offset(struct intel_dp *intel_dp)
+{
+ return intel_dp->psr.sink_panel_replay_su_support ?
+ DP_PANEL_PANEL_REPLAY_Y_GRANULARITY :
+ DP_PSR2_SU_Y_GRANULARITY;
+}
+
static void intel_dp_get_su_granularity(struct intel_dp *intel_dp)
{
struct drm_i915_private *i915 = dp_to_i915(intel_dp);
@@ -472,18 +508,26 @@ static void intel_dp_get_su_granularity(struct intel_dp
*intel_dp)
u16 w;
u8 y;
+ /*
+* TODO: Do we need to take into account panel supporting both PSR and
+* Panel replay?
+*/
+
/* If sink don't have specific granularity requirements set legacy ones
*/
- if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED)) {
+ if (!(intel_dp_get_su_capability(intel_dp) &
+ intel_dp_get_su_ganularity_required_bit(intel_dp))) {
/* As PSR2 HW sends full lines, we do not care about x
granularity */
w = 4;
y = 4;
goto exit;
}
- r = drm_dp_dpcd_read(_dp->aux, DP_PSR2_SU_X_GRANULARITY, , 2);
+ r = drm_dp_dpcd_read(_dp->aux,
+intel_dp_get_su_x_granularity_offset(intel_dp),
+, 2);
if (r != 2)
drm_dbg_kms(>drm,
- "Unable to read DP_PSR2_SU_X_GRANULARITY\n");
+ "Unable to read selective update x granularity\n");
/*
* Spec says that if the value read is 0 the default granularity should
* be used instead.
@@ -491,10 +535,12 @@ static void intel_dp_get_su_granularity(struct intel_dp
*intel_dp)
if (r != 2 || w == 0)
w = 4;
- r = drm_dp_dpcd_read(_dp->aux, DP_PSR2_SU_Y_GRANULARITY, , 1);
+ r = drm_dp_dpcd_read(_dp->aux,
+intel_dp_get_su_y_granularity_offset(intel_dp),
+, 1);
if (r != 1) {
drm_dbg_kms(>drm,
- "Unable to read DP_PSR2_SU_Y_GRANULARITY\n");
+ "Unable to read selective update y granularity\n");
y = 4;
}
if (y == 0)
@@ -587,7 +633,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
if (intel_dp->psr_dpcd[0])
_psr_init_dpcd(intel_dp);
- if (intel_dp->psr.sink_psr2_support)
+ if (intel_dp->psr.sink_psr2_support ||
+ intel_dp->psr.sink_panel_replay_su_support)
intel_dp_get_su_granularity(intel_dp);
}
--
2.34.1
[PATCH v2 11/13] drm/panelreplay: dpcd register definition for panelreplay SU
Add definitions for panel replay selective update v2: Remove unnecessary Cc from commit message Signed-off-by: Jouni Högander --- include/drm/display/drm_dp.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h index 281afff6ee4e..4ebf79948c7f 100644 --- a/include/drm/display/drm_dp.h +++ b/include/drm/display/drm_dp.h @@ -548,6 +548,12 @@ # define DP_PANEL_REPLAY_SUPPORT(1 << 0) # define DP_PANEL_REPLAY_SU_SUPPORT (1 << 1) +#define DP_PANEL_PANEL_REPLAY_CAPABILITY 0xb1 +# define DP_PANEL_PANEL_REPLAY_SU_GRANULARITY_REQUIRED (1 << 5) + +#define DP_PANEL_PANEL_REPLAY_X_GRANULARITY0xb2 +#define DP_PANEL_PANEL_REPLAY_Y_GRANULARITY0xb4 + /* Link Configuration */ #defineDP_LINK_BW_SET 0x100 # define DP_LINK_RATE_TABLE0x00/* eDP 1.4 */ -- 2.34.1
[PATCH v2 09/13] drm/i915/psr: Detect panel replay selective update support
Detect panel replay selective update support and store it into intel_psr->sink_panel_replay_su_support. Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_psr.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index be8b1b7a8025..03e41f10873f 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -519,9 +519,15 @@ static void _panel_replay_init_dpcd(struct intel_dp *intel_dp) return; } - drm_dbg_kms(>drm, - "Panel replay is supported by panel\n"); intel_dp->psr.sink_panel_replay_support = true; + + if (pr_dpcd & DP_PANEL_REPLAY_SU_SUPPORT) + intel_dp->psr.sink_panel_replay_su_support = true; + + drm_dbg_kms(>drm, + "Panel replay %sis supported by panel\n", + intel_dp->psr.sink_panel_replay_su_support ? + "selective_update " : ""); } static void _psr_init_dpcd(struct intel_dp *intel_dp) -- 2.34.1
[PATCH v2 10/13] drm/i915/psr: Split intel_psr2_config_valid for panel replay
Part of intel_psr2_config_valid is valid for panel replay. rename it as
intel_sel_update_config_valid. Split psr2 specific part and name it as
intel_psr2_config_valid.
Signed-off-by: Jouni Högander
---
drivers/gpu/drm/i915/display/intel_psr.c | 60 +++-
1 file changed, 38 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
b/drivers/gpu/drm/i915/display/intel_psr.c
index 03e41f10873f..68da1f284fae 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1286,12 +1286,6 @@ static bool intel_psr2_config_valid(struct intel_dp
*intel_dp,
return false;
}
- if (crtc_state->crc_enabled) {
- drm_dbg_kms(_priv->drm,
- "PSR2 not enabled because it would inhibit pipe CRC
calculation\n");
- return false;
- }
-
if (DISPLAY_VER(dev_priv) >= 12) {
psr_max_h = 5120;
psr_max_v = 3200;
@@ -1342,30 +1336,52 @@ static bool intel_psr2_config_valid(struct intel_dp
*intel_dp,
return false;
}
- if (HAS_PSR2_SEL_FETCH(dev_priv)) {
- if (!intel_psr2_sel_fetch_config_valid(intel_dp, crtc_state) &&
- !HAS_PSR_HW_TRACKING(dev_priv)) {
- drm_dbg_kms(_priv->drm,
- "PSR2 not enabled, selective fetch not
valid and no HW tracking available\n");
- return false;
- }
- }
-
- if (!psr2_granularity_check(intel_dp, crtc_state)) {
- drm_dbg_kms(_priv->drm, "PSR2 not enabled, SU granularity
not compatible\n");
- goto unsupported;
- }
-
if (!crtc_state->enable_psr2_sel_fetch &&
(crtc_hdisplay > psr_max_h || crtc_vdisplay > psr_max_v)) {
drm_dbg_kms(_priv->drm,
"PSR2 not enabled, resolution %dx%d > max supported
%dx%d\n",
crtc_hdisplay, crtc_vdisplay,
psr_max_h, psr_max_v);
- goto unsupported;
+ return false;
}
tgl_dc3co_exitline_compute_config(intel_dp, crtc_state);
+
+ return true;
+}
+
+static bool intel_sel_update_config_valid(struct intel_dp *intel_dp,
+ struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+ if (HAS_PSR2_SEL_FETCH(dev_priv) &&
+ !intel_psr2_sel_fetch_config_valid(intel_dp, crtc_state) &&
+ !HAS_PSR_HW_TRACKING(dev_priv)) {
+ drm_dbg_kms(_priv->drm,
+ "Selective update not enabled, selective fetch not
valid and no HW tracking available\n");
+ goto unsupported;
+ }
+
+ if (crtc_state->has_psr && !intel_psr2_config_valid(intel_dp,
crtc_state))
+ goto unsupported;
+
+ if (crtc_state->has_panel_replay && (DISPLAY_VER(dev_priv) < 14 ||
+
!intel_dp->psr.sink_panel_replay_su_support))
+ goto unsupported;
+
+ if (crtc_state->crc_enabled) {
+ drm_dbg_kms(_priv->drm,
+ "Selective update not enabled because it would
inhibit pipe CRC calculation\n");
+ goto unsupported;
+ }
+
+ if (!psr2_granularity_check(intel_dp, crtc_state)) {
+ drm_dbg_kms(_priv->drm,
+ "Selective update not enabled, SU granularity not
compatible\n");
+ goto unsupported;
+ }
+
return true;
unsupported:
@@ -1435,7 +1451,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
if (!(crtc_state->has_panel_replay || crtc_state->has_psr))
return;
- crtc_state->has_sel_update = intel_psr2_config_valid(intel_dp,
crtc_state);
+ crtc_state->has_sel_update = intel_sel_update_config_valid(intel_dp,
crtc_state);
}
void intel_psr_get_config(struct intel_encoder *encoder,
--
2.34.1
[PATCH v2 08/13] drm/i915/psr: Add sink_panel_replay_su_support to intel_psr
Add new boolean to store panel replay selective update support of sink.
Signed-off-by: Jouni Högander
---
drivers/gpu/drm/i915/display/intel_display_types.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 8315ec307d5f..3151741f49f5 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1721,6 +1721,7 @@ struct intel_psr {
u16 su_y_granularity;
bool source_panel_replay_support;
bool sink_panel_replay_support;
+ bool sink_panel_replay_su_support;
bool panel_replay_enabled;
u32 dc3co_exitline;
u32 dc3co_exit_delay;
--
2.34.1
[PATCH v2 07/13] drm/i915/psr: Add some documentation of variables used in psr code
We are adding more boolean variable into intel_psr and intel_crtc_state structs. Add some documentation about these for sake of clarity. Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_psr.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index cde95cc9925a..be8b1b7a8025 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -171,6 +171,22 @@ * * The rest of the bits are more self-explanatory and/or * irrelevant for normal operation. + * + * Description of intel_crtc_state variables. has_psr, has_panel_replay and + * has_sel_update: + * + * has_psr (alone): PSR1 + * has_psr + has_sel_update: PSR2 + * has_panel_replay: Panel Replay + * has_panel_replay + has_sel_update: Panel Replay Selective Update + * + * Description of some intel_psr enabled, panel_replay_enabled, + * sel_update_enabled + * + * enabled (alone): PSR1 + * enabled + sel_update_enabled: PSR2 + * enabled + panel_replay_enabled:Panel Replay + * enabled + panel_replay_enabled + sel_update_enabled: Panel Replay SU */ bool intel_encoder_can_psr(struct intel_encoder *encoder) -- 2.34.1
[PATCH v2 06/13] drm/i915/psr: Rename psr2_enabled as sel_update_enabled
We are about to reuse psr2_enabled for panel replay as well. Rename
it as sel_update_enabled to avoid confusion.
Signed-off-by: Jouni Högander
---
.../drm/i915/display/intel_display_types.h| 2 +-
drivers/gpu/drm/i915/display/intel_psr.c | 52 +--
2 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 8fdab2f0c546..8315ec307d5f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1706,7 +1706,7 @@ struct intel_psr {
unsigned int busy_frontbuffer_bits;
bool sink_psr2_support;
bool link_standby;
- bool psr2_enabled;
+ bool sel_update_enabled;
bool psr2_sel_fetch_enabled;
bool psr2_sel_fetch_cff_enabled;
bool req_psr2_sdp_prior_scanline;
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
b/drivers/gpu/drm/i915/display/intel_psr.c
index 3d00b4e396de..cde95cc9925a 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -339,12 +339,12 @@ static void psr_irq_control(struct intel_dp *intel_dp)
}
static void psr_event_print(struct drm_i915_private *i915,
- u32 val, bool psr2_enabled)
+ u32 val, bool sel_update_enabled)
{
drm_dbg_kms(>drm, "PSR exit events: 0x%x\n", val);
if (val & PSR_EVENT_PSR2_WD_TIMER_EXPIRE)
drm_dbg_kms(>drm, "\tPSR2 watchdog timer expired\n");
- if ((val & PSR_EVENT_PSR2_DISABLED) && psr2_enabled)
+ if ((val & PSR_EVENT_PSR2_DISABLED) && sel_update_enabled)
drm_dbg_kms(>drm, "\tPSR2 disabled\n");
if (val & PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN)
drm_dbg_kms(>drm, "\tSU dirty FIFO underrun\n");
@@ -372,7 +372,7 @@ static void psr_event_print(struct drm_i915_private *i915,
drm_dbg_kms(>drm, "\tVBI enabled\n");
if (val & PSR_EVENT_LPSP_MODE_EXIT)
drm_dbg_kms(>drm, "\tLPSP mode exited\n");
- if ((val & PSR_EVENT_PSR_DISABLE) && !psr2_enabled)
+ if ((val & PSR_EVENT_PSR_DISABLE) && !sel_update_enabled)
drm_dbg_kms(>drm, "\tPSR disabled\n");
}
@@ -400,7 +400,7 @@ void intel_psr_irq_handler(struct intel_dp *intel_dp, u32
psr_iir)
val = intel_de_rmw(dev_priv, PSR_EVENT(cpu_transcoder),
0, 0);
- psr_event_print(dev_priv, val,
intel_dp->psr.psr2_enabled);
+ psr_event_print(dev_priv, val,
intel_dp->psr.sel_update_enabled);
}
}
@@ -636,7 +636,7 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp)
if (intel_dp->psr.panel_replay_enabled)
return;
- if (intel_dp->psr.psr2_enabled) {
+ if (intel_dp->psr.sel_update_enabled) {
/* Enable ALPM at sink for psr2 */
drm_dp_dpcd_writeb(_dp->aux, DP_RECEIVER_ALPM_CONFIG,
DP_ALPM_ENABLE |
@@ -1446,10 +1446,10 @@ void intel_psr_get_config(struct intel_encoder *encoder,
pipe_config->has_psr = true;
}
- pipe_config->has_sel_update = intel_dp->psr.psr2_enabled;
+ pipe_config->has_sel_update = intel_dp->psr.sel_update_enabled;
pipe_config->infoframes.enable |=
intel_hdmi_infoframe_enable(DP_SDP_VSC);
- if (!intel_dp->psr.psr2_enabled)
+ if (!intel_dp->psr.sel_update_enabled)
goto unlock;
if (HAS_PSR2_SEL_FETCH(dev_priv)) {
@@ -1485,7 +1485,7 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
/* psr1, psr2 and panel-replay are mutually exclusive.*/
if (intel_dp->psr.panel_replay_enabled)
dg2_activate_panel_replay(intel_dp);
- else if (intel_dp->psr.psr2_enabled)
+ else if (intel_dp->psr.sel_update_enabled)
hsw_activate_psr2(intel_dp);
else
hsw_activate_psr1(intel_dp);
@@ -1598,7 +1598,7 @@ static void intel_psr_enable_source(struct intel_dp
*intel_dp,
*/
wm_optimization_wa(intel_dp, crtc_state);
- if (intel_dp->psr.psr2_enabled) {
+ if (intel_dp->psr.sel_update_enabled) {
if (DISPLAY_VER(dev_priv) == 9)
intel_de_rmw(dev_priv, CHICKEN_TRANS(cpu_transcoder), 0,
PSR2_VSC_ENABLE_PROG_HEADER |
@@ -1661,7 +1661,7 @@ static void intel_psr_enable_locked(struct intel_dp
*intel_dp,
drm_WARN_ON(_priv->drm, intel_dp->psr.enabled);
- intel_dp->psr.psr2_enabled = crtc_state->has_sel_update;
+ intel_dp->psr.sel_update_enabled = crtc_state->has_sel_update;
intel_dp->psr.panel_replay_enabled = crtc_state->has_panel_replay;
intel_dp->psr.busy_frontbuffer_bits = 0;
intel_dp->psr.pipe =
[PATCH v2 05/13] drm/i915/psr: Rename has_psr2 as has_sel_update
We are going to reuse has_psr2 for panel_replay as well. Rename it
as has_sel_update to avoid confusion.
Signed-off-by: Jouni Högander
---
drivers/gpu/drm/i915/display/intel_crtc_state_dump.c | 5 +++--
drivers/gpu/drm/i915/display/intel_display.c | 2 +-
drivers/gpu/drm/i915/display/intel_display_types.h | 2 +-
drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
drivers/gpu/drm/i915/display/intel_fbc.c | 2 +-
drivers/gpu/drm/i915/display/intel_psr.c | 8
6 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index 49fd100ec98a..5edbc9b3d766 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -266,9 +266,10 @@ void intel_crtc_state_dump(const struct intel_crtc_state
*pipe_config,
drm_dbg_kms(>drm, "sdp split: %s\n",
str_enabled_disabled(pipe_config->sdp_split_enable));
- drm_dbg_kms(>drm, "psr: %s, psr2: %s, panel replay: %s,
selective fetch: %s\n",
+ drm_dbg_kms(>drm,
+ "psr: %s, selective update: %s, panel replay: %s,
selective fetch: %s\n",
str_enabled_disabled(pipe_config->has_psr),
- str_enabled_disabled(pipe_config->has_psr2),
+ str_enabled_disabled(pipe_config->has_sel_update),
str_enabled_disabled(pipe_config->has_panel_replay),
str_enabled_disabled(pipe_config->enable_psr2_sel_fetch));
}
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index c5de4561f458..433276f24ae4 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5215,7 +5215,7 @@ intel_pipe_config_compare(const struct intel_crtc_state
*current_config,
if (current_config->active_planes) {
PIPE_CONF_CHECK_BOOL(has_psr);
- PIPE_CONF_CHECK_BOOL(has_psr2);
+ PIPE_CONF_CHECK_BOOL(has_sel_update);
PIPE_CONF_CHECK_BOOL(enable_psr2_sel_fetch);
PIPE_CONF_CHECK_I(dc3co_exitline);
}
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 6340fabd045c..8fdab2f0c546 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1211,7 +1211,7 @@ struct intel_crtc_state {
/* PSR is supported but might not be enabled due the lack of enabled
planes */
bool has_psr;
- bool has_psr2;
+ bool has_sel_update;
bool enable_psr2_sel_fetch;
bool enable_psr2_su_region_et;
bool req_psr2_sdp_prior_scanline;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 7e4b7d5606d4..a26db4012172 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2633,7 +2633,7 @@ static void intel_dp_compute_vsc_sdp(struct intel_dp
*intel_dp,
if (intel_dp_needs_vsc_sdp(crtc_state, conn_state)) {
intel_dp_compute_vsc_colorimetry(crtc_state, conn_state,
vsc);
- } else if (crtc_state->has_psr2) {
+ } else if (crtc_state->has_psr && crtc_state->has_sel_update) {
/*
* [PSR2 without colorimetry]
* Prepare VSC Header for SU as per eDP 1.4 spec, Table 6-11
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
b/drivers/gpu/drm/i915/display/intel_fbc.c
index f17a1afb4929..647dd1b56073 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -1235,7 +1235,7 @@ static int intel_fbc_check_plane(struct
intel_atomic_state *state,
* Recommendation is to keep this combination disabled
* Bspec: 50422 HSD: 14010260002
*/
- if (IS_DISPLAY_VER(i915, 12, 14) && crtc_state->has_psr2) {
+ if (IS_DISPLAY_VER(i915, 12, 14) && crtc_state->has_sel_update) {
plane_state->no_fbc_reason = "PSR2 enabled";
return 0;
}
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
b/drivers/gpu/drm/i915/display/intel_psr.c
index 8157a1eac8c2..3d00b4e396de 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1413,7 +1413,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
if (!(crtc_state->has_panel_replay || crtc_state->has_psr))
return;
- crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
+ crtc_state->has_sel_update =
[PATCH v2 04/13] drm/i915/psr: Unify panel replay enable sink
Panel replay enable for a sink is currently done in
intel_ddi.c:intel_ddi_pre_enable_dp. Move it to intel_psr_enable_sink to
unify psr/panel replay paths. Also enable some additional hpd interrupts
for panel replay.
Signed-off-by: Jouni Högander
---
drivers/gpu/drm/i915/display/intel_ddi.c | 7 +--
drivers/gpu/drm/i915/display/intel_psr.c | 19 +--
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 922194b957be..db2a027fc55e 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2800,15 +2800,10 @@ static void intel_ddi_pre_enable_dp(struct
intel_atomic_state *state,
const struct drm_connector_state
*conn_state)
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
- struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
- if (HAS_DP20(dev_priv)) {
+ if (HAS_DP20(dev_priv))
intel_dp_128b132b_sdp_crc16(enc_to_intel_dp(encoder),
crtc_state);
- if (crtc_state->has_panel_replay)
- drm_dp_dpcd_writeb(_dp->aux, PANEL_REPLAY_CONFIG,
- DP_PANEL_REPLAY_ENABLE);
- }
if (DISPLAY_VER(dev_priv) >= 14)
mtl_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
b/drivers/gpu/drm/i915/display/intel_psr.c
index a9421dd092c5..8157a1eac8c2 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -618,6 +618,16 @@ static bool psr2_su_region_et_valid(struct intel_dp
*intel_dp)
return false;
}
+static unsigned int intel_psr_get_enable_sink_offset(struct intel_dp *intel_dp)
+{
+ return intel_dp->psr.panel_replay_enabled ?
+ PANEL_REPLAY_CONFIG : DP_PSR_EN_CFG;
+}
+
+/*
+ * Note: Most of the bits are same in PANEL_REPLAY_CONFIG and DP_PSR_EN_CFG. We
+ * are relying on PSR definitions on these "common" bits.
+ */
static void intel_psr_enable_sink(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
@@ -643,15 +653,20 @@ static void intel_psr_enable_sink(struct intel_dp
*intel_dp)
dpcd_val |= DP_PSR_CRC_VERIFICATION;
}
+ if (intel_dp->psr.panel_replay_enabled)
+ dpcd_val |= DP_PANEL_REPLAY_UNRECOVERABLE_ERROR_EN |
+ DP_PANEL_REPLAY_RFB_STORAGE_ERROR_EN;
+
if (intel_dp->psr.req_psr2_sdp_prior_scanline)
dpcd_val |= DP_PSR_SU_REGION_SCANLINE_CAPTURE;
if (intel_dp->psr.entry_setup_frames > 0)
dpcd_val |= DP_PSR_FRAME_CAPTURE;
- drm_dp_dpcd_writeb(_dp->aux, DP_PSR_EN_CFG, dpcd_val);
+ drm_dp_dpcd_writeb(_dp->aux,
intel_psr_get_enable_sink_offset(intel_dp), dpcd_val);
- drm_dp_dpcd_writeb(_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
+ if (intel_dp_is_edp(intel_dp))
+ drm_dp_dpcd_writeb(_dp->aux, DP_SET_POWER,
DP_SET_POWER_D0);
}
static u32 intel_psr1_get_tp_time(struct intel_dp *intel_dp)
--
2.34.1
[PATCH v2 03/13] drm/i915/psr: Do not check alpm on DP or capability change for panel replay
Alpm is eDP specific. Do not check if not eDP. Also panel replay doesn't know about capability changes -> no need to check that. Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_psr.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 3e287a9f0e09..a9421dd092c5 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -2989,8 +2989,11 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp) /* clear status register */ drm_dp_dpcd_writeb(_dp->aux, DP_PSR_ERROR_STATUS, error_status); - psr_alpm_check(intel_dp); - psr_capability_changed_check(intel_dp); + if (intel_dp_is_edp(intel_dp)) + psr_alpm_check(intel_dp); + + if (!psr->panel_replay_enabled) + psr_capability_changed_check(intel_dp); exit: mutex_unlock(>lock); -- 2.34.1
[PATCH v2 02/13] drm/i915/psr: Intel_psr_pause/resume needs to support panel replay
Currently intel_psr_pause and intel_psr_resume do nothing in case of panel
replay. Change them to perform pause and return also in case of panel
replay.
Signed-off-by: Jouni Högander
---
drivers/gpu/drm/i915/display/intel_psr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
b/drivers/gpu/drm/i915/display/intel_psr.c
index 9705a75e063a..3e287a9f0e09 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1829,7 +1829,7 @@ void intel_psr_pause(struct intel_dp *intel_dp)
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
struct intel_psr *psr = _dp->psr;
- if (!CAN_PSR(intel_dp))
+ if (!CAN_PSR(intel_dp) && !CAN_PANEL_REPLAY(intel_dp))
return;
mutex_lock(>lock);
@@ -1862,7 +1862,7 @@ void intel_psr_resume(struct intel_dp *intel_dp)
{
struct intel_psr *psr = _dp->psr;
- if (!CAN_PSR(intel_dp))
+ if (!CAN_PSR(intel_dp) && !CAN_PANEL_REPLAY(intel_dp))
return;
mutex_lock(>lock);
--
2.34.1
[PATCH v2 01/13] drm/i915/psr: Disable panel replay for now
Panel replay is not completely validated yet. Let's disable it for now.
Signed-off-by: Jouni Högander
---
drivers/gpu/drm/i915/display/intel_display_types.h | 1 +
drivers/gpu/drm/i915/display/intel_psr.c | 10 +-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index ae2e8cff9d69..6340fabd045c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1692,6 +1692,7 @@ struct intel_psr {
#define I915_PSR_DEBUG_ENABLE_SEL_FETCH0x4
#define I915_PSR_DEBUG_IRQ 0x10
#define I915_PSR_DEBUG_SU_REGION_ET_DISABLE0x20
+#define I915_PSR_DEBUG_PANEL_REPLAY_DISABLE0x40
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 dff21a5edeb7..9705a75e063a 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -214,6 +214,11 @@ static bool psr2_global_enabled(struct intel_dp *intel_dp)
}
}
+static bool panel_replay_global_enabled(struct intel_dp *intel_dp)
+{
+ return !(intel_dp->psr.debug & I915_PSR_DEBUG_PANEL_REPLAY_DISABLE);
+}
+
static u32 psr_irq_psr_error_bit_get(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
@@ -1386,7 +1391,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
}
if (CAN_PANEL_REPLAY(intel_dp))
- crtc_state->has_panel_replay = true;
+ crtc_state->has_panel_replay =
panel_replay_global_enabled(intel_dp);
else
crtc_state->has_psr = _psr_compute_config(intel_dp, crtc_state);
@@ -2845,6 +2850,9 @@ void intel_psr_init(struct intel_dp *intel_dp)
/* Disable early transport for now */
intel_dp->psr.debug |= I915_PSR_DEBUG_SU_REGION_ET_DISABLE;
+ /* Disable panel replay for now */
+ intel_dp->psr.debug |= I915_PSR_DEBUG_PANEL_REPLAY_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
[PATCH v2 00/13] Panel replay selective update support
This patch set is implementing panel replay selective update support for Intel hardware. It is also fixing couple of exisiting issues in current panel replay implementation: ALPM status is checked even on DP (non eDP) PSR capability change is checked even when using panel replay Make psr pause/resume to work for panel replay as well Panel replay is disabled by default for now. This is done to minimize possible issues faced by customers as we haven't yet validated the feature completely. v2: Make psr pause/resume to work for panel replay as well Cc: Animesh Manna Cc: Mika Kahola Jouni Högander (13): drm/i915/psr: Disable panel replay for now drm/i915/psr: Intel_psr_pause/resume needs to support panel replay drm/i915/psr: Do not check alpm on DP or capability change for panel replay drm/i915/psr: Unify panel replay enable sink drm/i915/psr: Rename has_psr2 as has_sel_update drm/i915/psr: Rename psr2_enabled as sel_update_enabled drm/i915/psr: Add some documentation of variables used in psr code drm/i915/psr: Add sink_panel_replay_su_support to intel_psr drm/i915/psr: Detect panel replay selective update support drm/i915/psr: Split intel_psr2_config_valid for panel replay drm/panelreplay: dpcd register definition for panelreplay SU drm/i915/psr: Modify intel_dp_get_su_granularity to support panel replay drm/i915/psr: Add panel replay sel update support to debugfs interface .../drm/i915/display/intel_crtc_state_dump.c | 5 +- drivers/gpu/drm/i915/display/intel_ddi.c | 7 +- drivers/gpu/drm/i915/display/intel_display.c | 2 +- .../drm/i915/display/intel_display_types.h| 6 +- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- drivers/gpu/drm/i915/display/intel_psr.c | 244 +- include/drm/display/drm_dp.h | 6 + 8 files changed, 196 insertions(+), 78 deletions(-) -- 2.34.1
Re: [PATCH 3/7] drm/amd/display: Add handling for new "active color format" property
On Wed, 10 Jan 2024 at 13:53, Andri Yngvason wrote:
>
> mið., 10. jan. 2024 kl. 11:10 skrifaði Daniel Vetter :
> >
> > On Tue, Jan 09, 2024 at 06:11:00PM +, Andri Yngvason wrote:
> > > + /* Extract information from crtc to communicate it to userspace as
> > > connector properties */
> > > + for_each_new_connector_in_state(state, connector, new_con_state, i)
> > > {
> > > + struct drm_crtc *crtc = new_con_state->crtc;
> > > + struct dc_stream_state *stream;
> > > +
> > > + if (crtc) {
> > > + new_crtc_state =
> > > drm_atomic_get_new_crtc_state(state, crtc);
> > > + dm_new_crtc_state =
> > > to_dm_crtc_state(new_crtc_state);
> > > + stream = dm_new_crtc_state->stream;
> > > +
> > > + if (stream) {
> > > +
> > > drm_connector_set_active_color_format_property(connector,
> > > +
> > > convert_dc_pixel_encoding_into_drm_color_format(
> > > +
> > > dm_new_crtc_state->stream->timing.pixel_encoding));
> > > + }
> > > + } else {
> > > +
> > > drm_connector_set_active_color_format_property(connector, 0);
> >
> > Just realized an even bigger reason why your current design doesn't work:
> > You don't have locking here.
> >
> > And you cannot grab the required lock, which is
> > drm_dev->mode_config.mutex, because that would result in deadlocks. So
> > this really needs to use the atomic state based design I've described.
> >
>
> Maybe we should just drop "actual color format" and instead fail the
> modeset if the "preferred color format" property cannot be satisfied?
> It seems like the simplest thing to do here, though it is perhaps less
> convenient for userspace. In that case, the "preferred color format"
> property should just be called "color format".
Yeah that's more in line with how other atomic properties work. This
way userspace can figure out what works with a TEST_ONLY commit too.
And for this to work you probably want to have an "automatic" setting
too.
-Sima
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH 2/2] drm/i915/psr: CAN_PSR and CAN_PANEL_REPLAY can be now local defines
On Tue, Jan 09, 2024 at 12:05:17PM +0200, Jouni Högander wrote:
> 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
Reviewed-by: Stanislav Lisovskiy
> ---
> 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
>
Re: [PATCH 1/2] drm/i915/display: No need for full modeset due to psr
On Tue, Jan 09, 2024 at 12:05:16PM +0200, Jouni Högander wrote:
> There is no specific reason to force full modeset if psr is enabled.
>
> Signed-off-by: Jouni Högander
> Tested-by: Paz Zcharya
Reviewed-by: Stanislav Lisovskiy
> ---
> 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(>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
>
Re: disable large folios for shmem file used by xfs xfile
On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote: > Hi all, > > Darrick reported that the fairly new XFS xfile code blows up when force > enabling large folio for shmem. This series fixes this quickly by disabling > large folios for this particular shmem file for now until it can be fixed > properly, which will be a lot more invasive. > > I've added most of you to the CC list as I suspect most other users of > shmem_file_setup and friends will have similar issues. The graphics users _want_ to use large folios. I'm pretty sure they've been tested with this. It's just XFS that didn't know about this feature of shmem.
Re: [RFC 00/15] VBT read Cleanup
On Mon, 08 Jan 2024, Radhakrishna Sripada wrote: > This series does the VBT read cleanup. The series introduces new > intel_vbt structure to cache and collate vbt related info. Vbt > read from different sources viz. firmware/opregion/spi/oprom > needs to be cached for debug purposes and handled accordingly > during cleanup. Mixed feelings. I think the goal is good, not convinced by the implementation. First, i915->display.vbt.data.foo is just too much depth. It was borderline too much before, but now it definitely is. Second, whichever place allocates some stuff should also be responsible for freeing it. I don't like the idea that you have different places allocating and then you have a combined cleanup to take care of the alternatives. Possibly the first thing to do would be to put intel_bios_init() in charge of picking the VBT. Stop looking at opregion directly in intel_bios.c, and instead abstract that away. Also move firmware EDID loading there. Move debugfs there. Etc. The opregion code could still figure out what its idea of VBT is, but intel_bios_init() would the place to ask opregion code about it only if needed. BR, Jani. > > Radhakrishna Sripada (15): > drm/i915: Extract display->vbt_data to a new vbt structure > drm/i915: Move vbt fields from opregion to its own structure > drm/i915: Cache opregion asls pointer > drm/i915: Extract opregion vbt capture to its own function > drm/i915: Init vbt fields when read from oprom/spi > drm/i915: Classify vbt type based on its residence > drm/i915: Collate vbt cleanup for different types > drm/i915: Make intel_bios_init operate on intel_vbt > drm/i915: Move vbt load from opregion to bios init > drm/i915: Move vbt firmware load into intel_bios_init > drm/i915: Make oprom_get_vbt operate on intel_vbt > drm/i915: Make spi_oprom_get_vbt operate on intel_vbt > drm/i915: Make intel_load_vbt_firmware operate on intel_vbt > drm/i915: Kill reduntant vbt_firmware from intel_vbt > drm/i915: Use vbt type to determine its validity > > drivers/gpu/drm/i915/display/intel_bios.c | 348 +++--- > drivers/gpu/drm/i915/display/intel_crt.c | 2 +- > drivers/gpu/drm/i915/display/intel_display.c | 10 +- > .../gpu/drm/i915/display/intel_display_core.h | 16 +- > .../drm/i915/display/intel_display_debugfs.c | 6 +- > drivers/gpu/drm/i915/display/intel_dp.c | 2 +- > drivers/gpu/drm/i915/display/intel_dpll.c | 16 +- > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 19 +- > drivers/gpu/drm/i915/display/intel_dsi.c | 2 +- > drivers/gpu/drm/i915/display/intel_lvds.c | 4 +- > drivers/gpu/drm/i915/display/intel_opregion.c | 165 - > drivers/gpu/drm/i915/display/intel_opregion.h | 13 +- > drivers/gpu/drm/i915/display/intel_panel.c| 2 +- > .../gpu/drm/i915/display/intel_pch_refclk.c | 2 +- > drivers/gpu/drm/i915/display/intel_sdvo.c | 18 +- > drivers/gpu/drm/i915/intel_clock_gating.c | 2 +- > 16 files changed, 348 insertions(+), 279 deletions(-) -- Jani Nikula, Intel
Re: [PATCH v2 04/15] drm/i915: Bypass LMEMBAR/GTTMMADR for MTL stolen memory access
On 1/10/2024 11:49 AM, Nirmoy Das wrote:
Hi Ville,
Apologies, but I lost track of this series after I returned from sick
leave.
Please ignore the uncontextual "but" in the previous response. I need to
disable auto correct options.
Regards,
Nirmoy
On 12/15/2023 11:59 AM, Ville Syrjala wrote:
From: Ville Syrjälä
On MTL accessing stolen memory via the BARs is somehow borked,
and it can hang the machine. As a workaround let's bypass the
BARs and just go straight to DSMBASE/GSMBASE instead.
Note that on every other platform this itself would hang the
machine, but on MTL the system firmware is expected to relax
the access permission guarding stolen memory to enable this
workaround, and thus direct CPU accesses should be fine.
TODO: add w/a numbers and whatnot
Cc: Paz Zcharya
Cc: Nirmoy Das
Cc: Radhakrishna Sripada
Cc: Joonas Lahtinen
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 11 ++-
drivers/gpu/drm/i915/gt/intel_ggtt.c | 13 -
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index ee237043c302..252fe5cd6ede 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -941,7 +941,16 @@ i915_gem_stolen_lmem_setup(struct
drm_i915_private *i915, u16 type,
dsm_size = ALIGN_DOWN(lmem_size - dsm_base, SZ_1M);
}
- if (pci_resource_len(pdev, GEN12_LMEM_BAR) < lmem_size) {
+ if (IS_METEORLAKE(i915)) {
+ /*
+ * Workaround: access via BAR can hang MTL, go directly to DSM.
+ *
+ * Normally this would not work but on MTL the system firmware
+ * should have relaxed the access permissions sufficiently.
+ */
+ io_start = intel_uncore_read64(uncore, GEN12_DSMBASE) &
GEN12_BDSM_MASK;
+ io_size = dsm_size;
This will work well on host driver but I am afraid this will not work
on VM when someone tries to do direct device assignment of the igfx.
GSMBASE/DSMBASE is reserved region so won't show up in VM, last I
checked.
This is an obscure usages but are we suppose to support that? If so
then we need to detect that and fall back to binder approach.
Regards,
Nirmoy
+ } else if (pci_resource_len(pdev, GEN12_LMEM_BAR) < lmem_size) {
io_start = 0;
io_size = 0;
} else {
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 21a7e3191c18..ab71d74ec426 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -24,6 +24,7 @@
#include "intel_ring.h"
#include "i915_drv.h"
#include "i915_pci.h"
+#include "i915_reg.h"
#include "i915_request.h"
#include "i915_scatterlist.h"
#include "i915_utils.h"
@@ -1152,13 +1153,23 @@ static unsigned int gen6_gttadr_offset(struct
drm_i915_private *i915)
static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
{
struct drm_i915_private *i915 = ggtt->vm.i915;
+ struct intel_uncore *uncore = ggtt->vm.gt->uncore;
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
phys_addr_t phys_addr;
u32 pte_flags;
int ret;
GEM_WARN_ON(pci_resource_len(pdev, GEN4_GTTMMADR_BAR) !=
gen6_gttmmadr_size(i915));
- phys_addr = pci_resource_start(pdev, GEN4_GTTMMADR_BAR) +
gen6_gttadr_offset(i915);
+ /*
+ * Workaround: access via BAR can hang MTL, go directly to GSM.
+ *
+ * Normally this would not work but on MTL the system firmware
+ * should have relaxed the access permissions sufficiently.
+ */
+ if (IS_METEORLAKE(i915))
+ phys_addr = intel_uncore_read64(uncore, GEN12_GSMBASE) &
GEN12_BDSM_MASK;
+ else
+ phys_addr = pci_resource_start(pdev, GEN4_GTTMMADR_BAR) +
gen6_gttadr_offset(i915);
if (needs_wc_ggtt_mapping(i915))
ggtt->gsm = ioremap_wc(phys_addr, size);
Re: [PATCH 3/7] drm/amd/display: Add handling for new "active color format" property
On Tue, Jan 09, 2024 at 06:11:00PM +, Andri Yngvason wrote:
> From: Werner Sembach
>
> This commit implements the "active color format" drm property for the AMD
> GPU driver.
>
> Signed-off-by: Werner Sembach
> Signed-off-by: Andri Yngvason
> Tested-by: Andri Yngvason
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 42 ++-
> .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 4 ++
> 2 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 10e041a3b2545..b44d06c3b1706 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6882,6 +6882,24 @@ int convert_dc_color_depth_into_bpc(enum
> dc_color_depth display_color_depth)
> return 0;
> }
>
> +static int convert_dc_pixel_encoding_into_drm_color_format(
> + enum dc_pixel_encoding display_pixel_encoding)
> +{
> + switch (display_pixel_encoding) {
> + case PIXEL_ENCODING_RGB:
> + return DRM_COLOR_FORMAT_RGB444;
> + case PIXEL_ENCODING_YCBCR422:
> + return DRM_COLOR_FORMAT_YCBCR422;
> + case PIXEL_ENCODING_YCBCR444:
> + return DRM_COLOR_FORMAT_YCBCR444;
> + case PIXEL_ENCODING_YCBCR420:
> + return DRM_COLOR_FORMAT_YCBCR420;
> + default:
> + break;
> + }
> + return 0;
> +}
> +
> static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
> struct drm_crtc_state *crtc_state,
> struct drm_connector_state
> *conn_state)
> @@ -7436,8 +7454,10 @@ void amdgpu_dm_connector_init_helper(struct
> amdgpu_display_manager *dm,
> adev->mode_info.underscan_vborder_property,
> 0);
>
> - if (!aconnector->mst_root)
> + if (!aconnector->mst_root) {
> drm_connector_attach_max_bpc_property(>base, 8, 16);
> +
> drm_connector_attach_active_color_format_property(>base);
> + }
>
> aconnector->base.state->max_bpc = 16;
> aconnector->base.state->max_requested_bpc =
> aconnector->base.state->max_bpc;
> @@ -8969,6 +8989,26 @@ static void amdgpu_dm_atomic_commit_tail(struct
> drm_atomic_state *state)
> kfree(dummy_updates);
> }
>
> + /* Extract information from crtc to communicate it to userspace as
> connector properties */
> + for_each_new_connector_in_state(state, connector, new_con_state, i) {
> + struct drm_crtc *crtc = new_con_state->crtc;
> + struct dc_stream_state *stream;
> +
> + if (crtc) {
> + new_crtc_state = drm_atomic_get_new_crtc_state(state,
> crtc);
> + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
> + stream = dm_new_crtc_state->stream;
> +
> + if (stream) {
> +
> drm_connector_set_active_color_format_property(connector,
> +
> convert_dc_pixel_encoding_into_drm_color_format(
> +
> dm_new_crtc_state->stream->timing.pixel_encoding));
> + }
> + } else {
> +
> drm_connector_set_active_color_format_property(connector, 0);
Just realized an even bigger reason why your current design doesn't work:
You don't have locking here.
And you cannot grab the required lock, which is
drm_dev->mode_config.mutex, because that would result in deadlocks. So
this really needs to use the atomic state based design I've described.
A bit a tanget, but it would be really good to add a lockdep assert into
drm_object_property_set_value, that at least for atomic drivers and
connectors the above lock must be held for changing property values. But
it will be quite a bit of audit to make sure all current users obey that
rule.
Cheers, Sima
> + }
> + }
> +
> /**
>* Enable interrupts for CRTCs that are newly enabled or went through
>* a modeset. It was intentionally deferred until after the front end
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 11da0eebee6c4..a4d1b3ea8f81c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -600,6 +600,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr
> *mgr,
> if (connector->max_bpc_property)
> drm_connector_attach_max_bpc_property(connector, 8, 16);
>
> + connector->active_color_format_property =
> master->base.active_color_format_property;
> + if (connector->active_color_format_property)
> +
>
RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
> -Original Message- > From: Nikula, Jani > Sent: Wednesday, January 10, 2024 4:24 PM > To: Murthy, Arun R ; intel-gfx@lists.freedesktop.org; > Deak, Imre > Subject: RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST > > On Wed, 10 Jan 2024, "Murthy, Arun R" wrote: > >> -Original Message- > >> From: Nikula, Jani > >> Sent: Tuesday, January 9, 2024 2:59 PM > >> To: Murthy, Arun R ; > >> intel-gfx@lists.freedesktop.org; Deak, Imre > >> Subject: RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with > >> SST > >> > >> 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? > >> > > Yes, this will be taken care of in a separate patch, tracked as part of > > separate > JIRA. VLK-55112. > > Well, you shouldn't really first open the possibility for a broken config, > and then > say it'll be taken care of in the future. > Sorry, I misread it as single stream sideband msg. As per the transport is considered not more than one stream will be considered(In this case MSTM_CAP=0x00) based on the sink capability. If a hub is connected inbetween the panel and the source, then source reads MSTM_CAP as 1xb and native MST will be enabled. Thanks and Regards, Arun R Murthy > BR, > Jani. > > > -- > Jani Nikula, Intel
[PATCH v1 2/2] drm/xe: Modify the cfb size to be page size aligned for FBC
drm_gem_private_object_init expect the object size be page size aligned. The xe_bo create functions do not update the size for any alignment requirements. So align cfb size to be page size aligned in xe stolen memory handling. Signed-off-by: Vinod Govindapillai --- drivers/gpu/drm/xe/compat-i915-headers/i915_gem_stolen.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/xe/compat-i915-headers/i915_gem_stolen.h b/drivers/gpu/drm/xe/compat-i915-headers/i915_gem_stolen.h index 888e7a87a925..bd233007c1b7 100644 --- a/drivers/gpu/drm/xe/compat-i915-headers/i915_gem_stolen.h +++ b/drivers/gpu/drm/xe/compat-i915-headers/i915_gem_stolen.h @@ -19,6 +19,9 @@ static inline int i915_gem_stolen_insert_node_in_range(struct xe_device *xe, int err; u32 flags = XE_BO_CREATE_PINNED_BIT | XE_BO_CREATE_STOLEN_BIT; + if (align) + size = ALIGN(size, align); + bo = xe_bo_create_locked_range(xe, xe_device_get_root_tile(xe), NULL, size, start, end, ttm_bo_type_kernel, flags); -- 2.34.1
[PATCH v1 1/2] drm/i915/display: use PAGE_SIZE macro for FBC cfb alloc
FBC compressed frame buffer size need to be PAGE_SIZE aligned
and the corresponding the drm_gem functions check the object
size alignment using PAGE_SIZE macro. Use the PAGE_SIZE macro
in the cfb alloc as well instead of the magic number.
Signed-off-by: Vinod Govindapillai
---
drivers/gpu/drm/i915/display/intel_fbc.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
b/drivers/gpu/drm/i915/display/intel_fbc.c
index f17a1afb4929..9b9c8715d664 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -764,13 +764,15 @@ static int find_compression_limit(struct intel_fbc *fbc,
/* Try to over-allocate to reduce reallocations and fragmentation. */
ret = i915_gem_stolen_insert_node_in_range(i915, >compressed_fb,
- size <<= 1, 4096, 0, end);
+ size <<= 1, PAGE_SIZE, 0,
+ end);
if (ret == 0)
return limit;
for (; limit <= intel_fbc_max_limit(i915); limit <<= 1) {
ret = i915_gem_stolen_insert_node_in_range(i915,
>compressed_fb,
- size >>= 1, 4096, 0,
end);
+ size >>= 1,
PAGE_SIZE, 0,
+ end);
if (ret == 0)
return limit;
}
--
2.34.1
[PATCH v1 0/2] drm/xe: ensure fbc cfb size to be page size aligned
DRM gem object handling expet the object size to be page size aligned. Neither the driver or xe stolen memory handlers do that causing BUG_ON being triggered in some cases. Vinod Govindapillai (2): drm/i915/display: use PAGE_SIZE macro for FBC cfb alloc drm/xe: Modify the cfb size to be page size aligned for FBC drivers/gpu/drm/i915/display/intel_fbc.c | 6 -- drivers/gpu/drm/xe/compat-i915-headers/i915_gem_stolen.h | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) -- 2.34.1
RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
On Wed, 10 Jan 2024, "Murthy, Arun R" wrote: >> -Original Message- >> From: Nikula, Jani >> Sent: Tuesday, January 9, 2024 2:59 PM >> To: Murthy, Arun R ; >> intel-gfx@lists.freedesktop.org; >> Deak, Imre >> Subject: RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST >> >> 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? >> > Yes, this will be taken care of in a separate patch, tracked as part of > separate JIRA. VLK-55112. Well, you shouldn't really first open the possibility for a broken config, and then say it'll be taken care of in the future. BR, Jani. -- Jani Nikula, Intel
Re: drm/i915: Add bigjoiner force enable option to debugfs
Hello Stan,
On 11/16/2023 4:07 PM, Stanislav Lisovskiy wrote:
For validation purposes, it might be useful to be able to
force Bigjoiner mode, even if current dotclock/resolution
do not require that.
Lets add such to option to debugfs.
v2: - Apparently intel_dp_need_bigjoiner can't be used, when
debugfs entry is created so lets just check manually
the DISPLAY_VER.
v3: - Switch to intel_connector from drm_connector(Jani Nikula)
- Remove redundant modeset lock(Jani Nikula)
- Use kstrtobool_from_user for boolean value(Jani Nikula)
v4: - Apply the changes to proper function(Jani Nikula)
v5: - Removed unnecessary check from i915_bigjoiner_enable_show
(Ville Syrjälä)
- Added eDP connector check to intel_connector_debugfs_add
(Ville Syrjälä)
- Removed debug message in order to prevent dmesg flooding
(Ville Syrjälä)
Signed-off-by: Stanislav Lisovskiy
---
.../drm/i915/display/intel_display_debugfs.c | 59 +++
.../drm/i915/display/intel_display_types.h| 2 +
drivers/gpu/drm/i915/display/intel_dp.c | 3 +-
3 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 915420d0cef8f..aa95ecf2458ee 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1414,6 +1414,22 @@ out:
drm_modeset_unlock(>mode_config.connection_mutex);
return ret;
}
+static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
+{
+ struct intel_connector *connector = to_intel_connector(m->private);
+ struct intel_encoder *encoder = intel_attached_encoder(connector);
+ struct intel_dp *intel_dp;
+
+ if (!encoder)
+ return -ENODEV;
+
+ intel_dp = enc_to_intel_dp(encoder);
+
+ seq_printf(m, "Bigjoiner enable: %d\n",
intel_dp->force_bigjoiner_enable);
+
+ return 0;
+}
+
static ssize_t i915_dsc_output_format_write(struct file *file,
const char __user *ubuf,
size_t len, loff_t *offp)
@@ -1435,12 +1451,39 @@ static ssize_t i915_dsc_output_format_write(struct file
*file,
return len;
}
+static ssize_t i915_bigjoiner_enable_fops_write(struct file *file,
+ const char __user *ubuf,
+ size_t len, loff_t *offp)
+{
+ struct seq_file *m = file->private_data;
+ struct intel_connector *connector = m->private;
+ struct intel_encoder *encoder = intel_attached_encoder(connector);
+ struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+ bool bigjoiner_en = 0;
+ int ret;
+
+ ret = kstrtobool_from_user(ubuf, len, _en);
+ if (ret < 0)
+ return ret;
+
+ intel_dp->force_bigjoiner_enable = bigjoiner_en;
+ *offp += len;
+
+ return len;
+}
+
static int i915_dsc_output_format_open(struct inode *inode,
struct file *file)
{
return single_open(file, i915_dsc_output_format_show, inode->i_private);
}
+static int i915_bigjoiner_enable_open(struct inode *inode,
+ struct file *file)
+{
+ return single_open(file, i915_bigjoiner_enable_show, inode->i_private);
+}
+
static const struct file_operations i915_dsc_output_format_fops = {
.owner = THIS_MODULE,
.open = i915_dsc_output_format_open,
@@ -1529,6 +1572,15 @@ static const struct file_operations
i915_dsc_fractional_bpp_fops = {
.write = i915_dsc_fractional_bpp_write
};
+static const struct file_operations i915_bigjoiner_enable_fops = {
+ .owner = THIS_MODULE,
+ .open = i915_bigjoiner_enable_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+ .write = i915_bigjoiner_enable_fops_write
+};
+
/*
* Returns the Current CRTC's bpc.
* Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
@@ -1611,6 +1663,13 @@ void intel_connector_debugfs_add(struct intel_connector
*intel_connector)
connector, _dsc_fractional_bpp_fops);
}
+ if (DISPLAY_VER(dev_priv) >= 11 &&
+ (intel_connector->base.connector_type ==
DRM_MODE_CONNECTOR_DisplayPort ||
+intel_connector->base.connector_type == DRM_MODE_CONNECTOR_eDP)) {
+ debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
+ _connector->base,
_bigjoiner_enable_fops);
+ }
Can we force bigjoiner for HDMI?
+
if (connector->connector_type == DRM_MODE_CONNECTOR_DSI ||
connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
diff --git
RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
> -Original Message- > From: Nikula, Jani > Sent: Tuesday, January 9, 2024 2:59 PM > To: Murthy, Arun R ; intel-gfx@lists.freedesktop.org; > Deak, Imre > Subject: RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST > > 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? > Yes, this will be taken care of in a separate patch, tracked as part of separate JIRA. VLK-55112. Thanks and Regards, Arun R Murthy > > > >> > >>> 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(_dp->mst_mgr, > >>> > intel_dp->is_mst); > >>> > >>> -- > >>> Jani Nikula, Intel > > -- > Jani Nikula, Intel
Re: [PATCH v2 04/15] drm/i915: Bypass LMEMBAR/GTTMMADR for MTL stolen memory access
Hi Ville,
Apologies, but I lost track of this series after I returned from sick leave.
On 12/15/2023 11:59 AM, Ville Syrjala wrote:
From: Ville Syrjälä
On MTL accessing stolen memory via the BARs is somehow borked,
and it can hang the machine. As a workaround let's bypass the
BARs and just go straight to DSMBASE/GSMBASE instead.
Note that on every other platform this itself would hang the
machine, but on MTL the system firmware is expected to relax
the access permission guarding stolen memory to enable this
workaround, and thus direct CPU accesses should be fine.
TODO: add w/a numbers and whatnot
Cc: Paz Zcharya
Cc: Nirmoy Das
Cc: Radhakrishna Sripada
Cc: Joonas Lahtinen
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 11 ++-
drivers/gpu/drm/i915/gt/intel_ggtt.c | 13 -
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index ee237043c302..252fe5cd6ede 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -941,7 +941,16 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915,
u16 type,
dsm_size = ALIGN_DOWN(lmem_size - dsm_base, SZ_1M);
}
- if (pci_resource_len(pdev, GEN12_LMEM_BAR) < lmem_size) {
+ if (IS_METEORLAKE(i915)) {
+ /*
+* Workaround: access via BAR can hang MTL, go directly to DSM.
+*
+* Normally this would not work but on MTL the system firmware
+* should have relaxed the access permissions sufficiently.
+*/
+ io_start = intel_uncore_read64(uncore, GEN12_DSMBASE) &
GEN12_BDSM_MASK;
+ io_size = dsm_size;
This will work well on host driver but I am afraid this will not work on
VM when someone tries to do direct device assignment of the igfx.
GSMBASE/DSMBASE is reserved region so won't show up in VM, last I checked.
This is an obscure usages but are we suppose to support that? If so then
we need to detect that and fall back to binder approach.
Regards,
Nirmoy
+ } else if (pci_resource_len(pdev, GEN12_LMEM_BAR) < lmem_size) {
io_start = 0;
io_size = 0;
} else {
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 21a7e3191c18..ab71d74ec426 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -24,6 +24,7 @@
#include "intel_ring.h"
#include "i915_drv.h"
#include "i915_pci.h"
+#include "i915_reg.h"
#include "i915_request.h"
#include "i915_scatterlist.h"
#include "i915_utils.h"
@@ -1152,13 +1153,23 @@ static unsigned int gen6_gttadr_offset(struct
drm_i915_private *i915)
static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
{
struct drm_i915_private *i915 = ggtt->vm.i915;
+ struct intel_uncore *uncore = ggtt->vm.gt->uncore;
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
phys_addr_t phys_addr;
u32 pte_flags;
int ret;
GEM_WARN_ON(pci_resource_len(pdev, GEN4_GTTMMADR_BAR) != gen6_gttmmadr_size(i915));
- phys_addr = pci_resource_start(pdev, GEN4_GTTMMADR_BAR) +
gen6_gttadr_offset(i915);
+ /*
+* Workaround: access via BAR can hang MTL, go directly to GSM.
+*
+* Normally this would not work but on MTL the system firmware
+* should have relaxed the access permissions sufficiently.
+*/
+ if (IS_METEORLAKE(i915))
+ phys_addr = intel_uncore_read64(uncore, GEN12_GSMBASE) &
GEN12_BDSM_MASK;
+ else
+ phys_addr = pci_resource_start(pdev, GEN4_GTTMMADR_BAR) +
gen6_gttadr_offset(i915);
if (needs_wc_ggtt_mapping(i915))
ggtt->gsm = ioremap_wc(phys_addr, size);
Re: [PATCH 2/7] drm/uAPI: Add "active color format" drm property as feedback for userspace
On Tue, Jan 09, 2024 at 11:12:11PM +, Andri Yngvason wrote: > Hi Daniel, > > þri., 9. jan. 2024 kl. 22:32 skrifaði Daniel Stone : > > > 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? > > > > Userspace does not determine what's happened without polling. The purpose > of this property is not for programmatic verification that the preferred > property was applied. It is my understanding that it's mostly intended for > debugging purposes. It should only change as a consequence of modesetting, > although I didn't actually look into what happens if you set the "preferred > color format" outside of a modeset. This feels a bit irky to me, since we don't have any synchronization and it kinda breaks how userspace gets to know about stuff. For context the current immutable properties are all stuff that's derived from the sink (like edid, or things like that). Userspace is guaranteed to get a hotplug event (minus driver bugs as usual) if any of these change, and we've added infrastructure so that the hotplug event even contains the specific property so that userspace can avoid re-read (which can cause some costly re-probing) them all. As an example you can look at drm_connector_set_link_status_property, which drivers follow by a call to drm_kms_helper_connector_hotplug_event to make sure userspace knows about what's up. Could be optimized I think. This thing here works entirely differently, and I think we need somewhat new semantics for this: - I agree it should be read-only for userspace, so immutable sounds right. - But I also agree with Daniel Stone that this should be tied more directly to the modeset state. So I think the better approach would be to put the output type into drm_connector_state, require that drivers compute it in their ->atomic_check code (which in the future would allow us to report it out for TEST_ONLY commits too), and so guarantee that the value is updated right after the kms ioctl returns (and not somewhen later for non-blocking commits). You probably need a bit of work to be able to handle immutable properties with the atomic state infrastructure, but I think otherwise this should fit all rather neatly. Cheers, Sima > > The way I've implemented things in sway, calling the > "preferred_signal_format" command triggers a modeset with the "preferred > color format" set and calling "get_outputs", immediately queries the > "actual color format" and displays it. > > Regards, > Andri -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
> -Original Message- > From: Nikula, Jani > Sent: Tuesday, January 9, 2024 2:58 PM > To: Murthy, Arun R ; intel-gfx@lists.freedesktop.org; > Deak, Imre > Subject: RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST > > 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? > Done! Thanks and Regards, Arun R Murthy --- > > > >> 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(_dp->mst_mgr, > >> > intel_dp->is_mst); > >> > >> -- > >> Jani Nikula, Intel > > -- > Jani Nikula, Intel
RE: ✗ Fi.CI.IGT: failure for Update bxt_sanitize_cdclk() for Xe2_LPD (rev3)
Hi, https://patchwork.freedesktop.org/series/128175/ - Re-reported Thanks, Tejasree -Original Message- From: I915-ci-infra On Behalf Of Gustavo Sousa Sent: Monday, January 8, 2024 11:07 PM To: i915-ci-in...@lists.freedesktop.org; Patchwork ; intel-gfx@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org Subject: Re: ✗ Fi.CI.IGT: failure for Update bxt_sanitize_cdclk() for Xe2_LPD (rev3) Quoting Gustavo Sousa (2024-01-08 14:27:46-03:00) >Quoting Gustavo Sousa (2024-01-08 10:35:56-03:00) >>Quoting Patchwork (2024-01-05 21:14:37-03:00) >>>== Series Details == >>> >>>Series: Update bxt_sanitize_cdclk() for Xe2_LPD (rev3) >>>URL : https://patchwork.freedesktop.org/series/128175/ >>>State : failure >>> >>>== Summary == >>> >>>CI Bug Log - changes from CI_DRM_14080_full -> >>>Patchwork_128175v3_full >>> >>> >>>Summary >>>--- >>> >>> **FAILURE** >>> >>> Serious unknown changes coming with Patchwork_128175v3_full >>> absolutely need to be verified manually. >>> >>> If you think the reported changes have nothing to do with the >>> changes introduced in Patchwork_128175v3_full, please notify your >>> bug team (i915-ci-in...@lists.freedesktop.org) to allow them to document >>> this new failure mode, which will reduce false positives in CI. >>> >>> >>> >>>Participating hosts (8 -> 8) >>>-- >>> >>> No changes in participating hosts >>> >>>Possible new issues >>>--- >>> >>> Here are the unknown changes that may have been introduced in >>> Patchwork_128175v3_full: >>> >>>### IGT changes ### >>> >>> Possible regressions >>> >>> * igt@kms_vblank@query-busy-hang@pipe-c-hdmi-a-2: >>>- shard-glk: [PASS][1] -> [INCOMPLETE][2] >>> [1]: >>> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14080/shard-glk6/igt@kms_vblank@query-busy-h...@pipe-c-hdmi-a-2.html >>> [2]: >>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128175v3/shard-gl >>> k7/igt@kms_vblank@query-busy-h...@pipe-c-hdmi-a-2.html >> >>The dmesg output do not provide conclusive data for the INCOMPLETE >>status and I believe the issue is unrelated, since the real functional >>change is on the driver initialization path. >> >>However, for sanity sake, as GLK takes the bxt_sanitize_cdclk() path >>during initialization, could we re-report, please? > >Re-sending this now as a member of i915-ci-in...@lists.freedesktop.org; >my previous email went into a moderation queue because I was not a member. One last try, as it now complains that the message is too big... -- Gustavo Sousa
Re: [PATCH v2 05/15] drm/i915: Disable the "binder"
On 15.12.2023 11:59, Ville Syrjala wrote:
From: Ville Syrjälä
Now that the GGTT PTE updates go straight to GSMBASE (bypassing
GTTMMADR) there should be no more risk of system hangs? So the
"binder" (ie. update the PTEs via MI_UPDATE_GTT) is no longer
necessary, disable it.
TODO: MI_UPDATE_GTT might be interesting as an optimization
though, so perhaps someone should look into always using it
(assuming the GPU is alive and well)?
Cc: Paz Zcharya
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 86f73fe558ca..5bc7a4fb7485 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -24,7 +24,7 @@
bool i915_ggtt_require_binder(struct drm_i915_private *i915)
{
/* Wa_13010847436 & Wa_14019519902 */
- return MEDIA_VER_FULL(i915) == IP_VER(13, 0);
+ return false && MEDIA_VER_FULL(i915) == IP_VER(13, 0);
I guess this is RFC :)
Maybe revert "drm/i915: Enable GGTT updates with binder in MTL" ???
CC more competent developer.
Nirmoy, any thoughts?
Regards
Andrzej
}
static bool intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
Re: [PATCH v2 15/15] drm/i915: Try to relocate the BIOS fb to the start of ggtt
On 15.12.2023 11:59, Ville Syrjala wrote:
From: Ville Syrjälä
On MTL the GOP (for whatever reason) likes to bind its framebuffer
high up in the ggtt address space. This can conflict with whatever
ggtt_reserve_guc_top() is trying to do, and the result is that
ggtt_reserve_guc_top() fails and then we proceed to explode when
trying to tear down the driver. Thus far I haven't analyzed what
causes the actual fireworks, but it's not super important as even
if it didn't explode we'd still fail the driver load and the user
would be left with an unusable GPU.
To remedy this (without having to figure out exactly what
ggtt_reserve_guc_top() is trying to achieve) we can attempt to
relocate the BIOS framebuffer to a lower ggtt address. We can do
this at this early point in driver init because nothing else is
supposed to be clobbering the ggtt yet. So we simply change where
in the ggtt we pin the vma, the original PTEs will be left as is,
and the new PTEs will get written with the same dma addresses.
The plane will keep on scanning out from the original PTEs until
we are done with the whole process, and at that point we rewrite
the plane's surface address register to point at the new ggtt
address.
Since we don't need a specific ggtt address for the plane
(apart from needing it to land in the mappable region for
normal stolen objects) we'll just try to pin it without a fixed
offset first. It should end up at the lowest available address
(which really should be 0 at this point in the driver init).
If that fails we'll fall back to just pinning it exactly to the
origianal address.
To make sure we don't accidentlally pin it partially over the
original ggtt range (as that would corrupt the original PTEs)
we reserve the original range temporarily during this process.
Cc: Paz Zcharya
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/i9xx_plane.c | 30 ++
drivers/gpu/drm/i915/display/i9xx_plane.h | 7 +++
drivers/gpu/drm/i915/display/intel_display.c | 5 ++
.../gpu/drm/i915/display/intel_display_core.h | 2 +
.../drm/i915/display/intel_plane_initial.c| 57 ++-
.../drm/i915/display/skl_universal_plane.c| 28 +
.../drm/i915/display/skl_universal_plane.h| 2 +
7 files changed, 128 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c
b/drivers/gpu/drm/i915/display/i9xx_plane.c
index 91f2bc405cba..0279c8aabdd1 100644
--- a/drivers/gpu/drm/i915/display/i9xx_plane.c
+++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
@@ -1060,3 +1060,33 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
plane_config->fb = intel_fb;
}
+
+bool i9xx_fixup_initial_plane_config(struct intel_crtc *crtc,
+const struct intel_initial_plane_config
*plane_config)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+ const struct intel_plane_state *plane_state =
+ to_intel_plane_state(plane->base.state);
+ enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
+ u32 base;
+
+ if (!plane_state->uapi.visible)
+ return false;
+
+ base = intel_plane_ggtt_offset(plane_state);
+
+ /*
+* We may have moved the surface to a different
+* part of ggtt, make the plane aware of that.
+*/
+ if (plane_config->base == base)
+ return false;
+
+ if (DISPLAY_VER(dev_priv) >= 4)
+ intel_de_write(dev_priv, DSPSURF(i9xx_plane), base);
+ else
+ intel_de_write(dev_priv, DSPADDR(i9xx_plane), base);
It seems skl_fixup_initial_plane_config is the same, except
intel_de_write part. Couldn't we merge both functions into one and just
add another elseif branch here? Maybe abstracting out somehow surface
registers writes? Just loose ideas.
However I wouldn't be surprised if there is good reason to keep it as is.
Reviewed-by: Andrzej Hajda
Regards
Andrzej
+
+ return true;
+}
diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.h
b/drivers/gpu/drm/i915/display/i9xx_plane.h
index b3d724a144cb..0ca12d1e6839 100644
--- a/drivers/gpu/drm/i915/display/i9xx_plane.h
+++ b/drivers/gpu/drm/i915/display/i9xx_plane.h
@@ -26,6 +26,8 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv,
enum pipe pipe);
void i9xx_get_initial_plane_config(struct intel_crtc *crtc,
struct intel_initial_plane_config
*plane_config);
+bool i9xx_fixup_initial_plane_config(struct intel_crtc *crtc,
+const struct intel_initial_plane_config
*plane_config);
#else
static inline unsigned int i965_plane_max_stride(struct intel_plane *plane,
u32 pixel_format, u64 modifier,
@@ -46,6 +48,11 @@ static inline void i9xx_get_initial_plane_config(struct
intel_crtc *crtc,
Re: [PATCH v2 14/15] drm/i915: Tweak BIOS fb reuse check
On 15.12.2023 11:59, Ville Syrjala wrote:
From: Ville Syrjälä
Currently we assume that we bind the BIOS fb exactly into the same
ggtt address where the BIOS left it. That is about to change, and
in order to keep intel_reuse_initial_plane_obj() working as intended
we need to compare the original ggtt offset (called 'base' here)
as opposed ot the actual vma ggtt offset we selected. Otherwise
s/ot/to/
the first plane could change the ggtt offset, and then subsequent
planes would no longer notice that they are in fact using the same
ggtt offset that the first plane was already using. Thus the reuse
check will fail and we proceed to turn off these subsequent planes.
TODO: would probably make more sense to do the pure readout first
for all the planes, then check for fb reuse, and only then proceed
to pin the object into the final location in the ggtt...
Cc: Paz Zcharya
Signed-off-by: Ville Syrjälä
Reviewed-by: Andrzej Hajda
Regards
Andrzej
---
.../drm/i915/display/intel_plane_initial.c| 34 +++
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c
b/drivers/gpu/drm/i915/display/intel_plane_initial.c
index b7e12b60d68b..82ab98985a09 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
@@ -13,20 +13,21 @@
#include "intel_plane_initial.h"
static bool
-intel_reuse_initial_plane_obj(struct drm_i915_private *i915,
- const struct intel_initial_plane_config
*plane_config,
+intel_reuse_initial_plane_obj(struct intel_crtc *this,
+ const struct intel_initial_plane_config
plane_configs[],
struct drm_framebuffer **fb,
struct i915_vma **vma)
{
+ struct drm_i915_private *i915 = to_i915(this->base.dev);
struct intel_crtc *crtc;
for_each_intel_crtc(>drm, crtc) {
- struct intel_crtc_state *crtc_state =
- to_intel_crtc_state(crtc->base.state);
- struct intel_plane *plane =
+ const struct intel_plane *plane =
to_intel_plane(crtc->base.primary);
- struct intel_plane_state *plane_state =
+ const struct intel_plane_state *plane_state =
to_intel_plane_state(plane->base.state);
+ const struct intel_crtc_state *crtc_state =
+ to_intel_crtc_state(crtc->base.state);
if (!crtc_state->uapi.active)
continue;
@@ -34,7 +35,7 @@ intel_reuse_initial_plane_obj(struct drm_i915_private *i915,
if (!plane_state->ggtt_vma)
continue;
- if (intel_plane_ggtt_offset(plane_state) == plane_config->base) {
+ if (plane_configs[this->pipe].base ==
plane_configs[crtc->pipe].base) {
*fb = plane_state->hw.fb;
*vma = plane_state->ggtt_vma;
return true;
@@ -265,10 +266,11 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
static void
intel_find_initial_plane_obj(struct intel_crtc *crtc,
-struct intel_initial_plane_config *plane_config)
+struct intel_initial_plane_config plane_configs[])
{
- struct drm_device *dev = crtc->base.dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ struct intel_initial_plane_config *plane_config =
+ _configs[crtc->pipe];
struct intel_plane *plane =
to_intel_plane(crtc->base.primary);
struct intel_plane_state *plane_state =
@@ -294,7 +296,7 @@ intel_find_initial_plane_obj(struct intel_crtc *crtc,
* Failed to alloc the obj, check to see if we should share
* an fb with another CRTC instead
*/
- if (intel_reuse_initial_plane_obj(dev_priv, plane_config, , ))
+ if (intel_reuse_initial_plane_obj(crtc, plane_configs, , ))
goto valid_fb;
/*
@@ -359,10 +361,12 @@ static void plane_config_fini(struct
intel_initial_plane_config *plane_config)
void intel_initial_plane_config(struct drm_i915_private *i915)
{
+ struct intel_initial_plane_config plane_configs[I915_MAX_PIPES] = {};
struct intel_crtc *crtc;
for_each_intel_crtc(>drm, crtc) {
- struct intel_initial_plane_config plane_config = {};
+ struct intel_initial_plane_config *plane_config =
+ _configs[crtc->pipe];
if (!to_intel_crtc_state(crtc->base.state)->uapi.active)
continue;
@@ -374,14 +378,14 @@ void intel_initial_plane_config(struct drm_i915_private
*i915)
* can even allow for smooth boot transitions if the BIOS
* fb is large
Re: [PATCH 5/7] drm/uAPI: Add "preferred color format" drm property as setting for userspace
Hi,
On Tue, Jan 09, 2024 at 06:11:02PM +, Andri Yngvason wrote:
> From: Werner Sembach
>
> Add a new general drm property "preferred color format" which can be used
> by userspace to tell the graphic drivers to which color format to use.
>
> Possible options are:
> - auto (default/current behaviour)
> - rgb
> - ycbcr444
> - ycbcr422 (not supported by both amdgpu and i915)
> - ycbcr420
>
> In theory the auto option should choose the best available option for the
> current setup, but because of bad internal conversion some monitors look
> better with rgb and some with ycbcr444.
I looked at the patch and I couldn't find what is supposed to happen if
you set it to something else than auto, and the driver can't match that.
Are we supposed to fallback to the "auto" behaviour, or are we suppose
to reject the mode entirely?
The combination with the active output format property suggests the
former, but we should document it explicitly.
> Also, because of bad shielded connectors and/or cables, it might be
> preferable to use the less bandwidth heavy ycbcr422 and ycbcr420 formats
> for a signal that is less deceptible to interference.
>
> In the future, automatic color calibration for screens might also depend on
> this option being available.
>
> Signed-off-by: Werner Sembach
> Reported-by: Dan Carpenter
> Signed-off-by: Andri Yngvason
> Tested-by: Andri Yngvason
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 4 +++
> drivers/gpu/drm/drm_atomic_uapi.c | 4 +++
> drivers/gpu/drm/drm_connector.c | 50 -
> include/drm/drm_connector.h | 17 ++
> 4 files changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 68ffcc0b00dca..745a43d9c5da3 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -707,6 +707,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> if (old_connector_state->max_requested_bpc !=
> new_connector_state->max_requested_bpc)
> new_crtc_state->connectors_changed = true;
> +
> + if (old_connector_state->preferred_color_format !=
> + new_connector_state->preferred_color_format)
> + new_crtc_state->connectors_changed = true;
> }
>
> if (funcs->atomic_check)
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 98d3b10c08ae1..eba5dea1249e5 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -798,6 +798,8 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> state->max_requested_bpc = val;
> } else if (property == connector->privacy_screen_sw_state_property) {
> state->privacy_screen_sw_state = val;
> + } else if (property == connector->preferred_color_format_property) {
> + state->preferred_color_format = val;
> } else if (connector->funcs->atomic_set_property) {
> return connector->funcs->atomic_set_property(connector,
> state, property, val);
> @@ -881,6 +883,8 @@ drm_atomic_connector_get_property(struct drm_connector
> *connector,
> *val = state->max_requested_bpc;
> } else if (property == connector->privacy_screen_sw_state_property) {
> *val = state->privacy_screen_sw_state;
> + } else if (property == connector->preferred_color_format_property) {
> + *val = state->preferred_color_format;
> } else if (connector->funcs->atomic_get_property) {
> return connector->funcs->atomic_get_property(connector,
> state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 30d62e505d188..4de48a38792cf 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1061,6 +1061,14 @@ static const struct drm_prop_enum_list
> drm_dp_subconnector_enum_list[] = {
> { DRM_MODE_SUBCONNECTOR_Native, "Native"}, /* DP */
> };
>
> +static const struct drm_prop_enum_list
> drm_preferred_color_format_enum_list[] = {
> + { 0, "auto" },
> + { DRM_COLOR_FORMAT_RGB444, "rgb" },
> + { DRM_COLOR_FORMAT_YCBCR444, "ycbcr444" },
> + { DRM_COLOR_FORMAT_YCBCR422, "ycbcr422" },
> + { DRM_COLOR_FORMAT_YCBCR420, "ycbcr420" },
> +};
> +
> static const struct drm_prop_enum_list drm_active_color_format_enum_list[] =
> {
> { 0, "not applicable" },
> { DRM_COLOR_FORMAT_RGB444, "rgb" },
> @@ -1398,11 +1406,20 @@ static const u32 dp_colorspaces =
> * drm_connector_attach_max_bpc_property() to create and attach the
> * property to the connector during initialization.
> *
> + * preferred
Re: [Intel-gfx] [PATCH] drm/i915: Fix bigjoiner case for DP2.0
On Fri, Oct 13, 2023 at 01:43:46PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 13, 2023 at 01:00:00AM +0530, vsrini4 wrote:
> > Patch calculates bigjoiner pipes in mst compute.
> > Patch also passes bigjoiner bool to validate plane
> > max size.
>
> I doubt this is sufficient. The modeset sequence is still all
> wrong for bigjoiner+mst.
Should that be now enough with my series also?
https://patchwork.freedesktop.org/series/128311/
Stan
>
> >
> > Signed-off-by: vsrini4
> > ---
> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 19 ---
> > 1 file changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index e3f176a093d2..f499ce39b2a8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -308,6 +308,7 @@ static int intel_dp_mst_compute_config(struct
> > intel_encoder *encoder,
> >struct drm_connector_state *conn_state)
> > {
> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > + struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
> > struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> > struct intel_dp *intel_dp = _mst->primary->dp;
> > const struct drm_display_mode *adjusted_mode =
> > @@ -318,6 +319,10 @@ static int intel_dp_mst_compute_config(struct
> > intel_encoder *encoder,
> > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> > return -EINVAL;
> >
> > + if (intel_dp_need_bigjoiner(intel_dp, adjusted_mode->crtc_hdisplay,
> > + adjusted_mode->crtc_clock))
> > + pipe_config->bigjoiner_pipes = GENMASK(crtc->pipe + 1,
> > crtc->pipe);
> > +
> > pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> > pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> > pipe_config->has_pch_encoder = false;
> > @@ -936,12 +941,6 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector
> > *connector,
> > if (ret)
> > return ret;
> >
> > - if (mode_rate > max_rate || mode->clock > max_dotclk ||
> > - drm_dp_calc_pbn_mode(mode->clock, min_bpp, false) > port->full_pbn)
> > {
> > - *status = MODE_CLOCK_HIGH;
> > - return 0;
> > - }
> > -
> > if (mode->clock < 1) {
> > *status = MODE_CLOCK_LOW;
> > return 0;
> > @@ -957,6 +956,12 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector
> > *connector,
> > max_dotclk *= 2;
> > }
> >
> > + if (mode_rate > max_rate || mode->clock > max_dotclk ||
> > + drm_dp_calc_pbn_mode(mode->clock, min_bpp, false) > port->full_pbn)
> > {
> > + *status = MODE_CLOCK_HIGH;
> > + return 0;
> > + }
> > +
> > if (DISPLAY_VER(dev_priv) >= 10 &&
> > drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) {
> > /*
> > @@ -994,7 +999,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector
> > *connector,
> > if (mode_rate > max_rate && !dsc)
> > return MODE_CLOCK_HIGH;
> >
> > - *status = intel_mode_valid_max_plane_size(dev_priv, mode, false);
> > + *status = intel_mode_valid_max_plane_size(dev_priv, mode, bigjoiner);
> > return 0;
> > }
> >
> > --
> > 2.33.0
>
> --
> Ville Syrjälä
> Intel
Re: [PATCH 3/3] ASoC: hdmi-codec: drop drm/drm_edid.h include
On Thu, 04 Jan 2024, Jani Nikula wrote: > hdmi-codec.h does not appear to directly need drm/drm_edid.h for > anything. Remove it. Jaroslav, Takashi, ack for merging this via the drm trees, please? BR, Jani. > > There are some files that get drm/drm_edid.h by proxy; include it where > needed. > > v2-v4: Fix build (kernel test robot ) > > Cc: Rob Clark > Cc: Abhinav Kumar > Cc: Dmitry Baryshkov > Cc: Sean Paul > Cc: Marijn Suijten > Cc: linux-arm-...@vger.kernel.org > Cc: freedr...@lists.freedesktop.org > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Robert Foss > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: Jaroslav Kysela > Cc: Takashi Iwai > Cc: linux-so...@vger.kernel.org > Acked-by: Maxime Ripard > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/bridge/lontium-lt9611.c| 1 + > drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 1 + > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 + > drivers/gpu/drm/msm/dp/dp_display.c| 1 + > drivers/gpu/drm/tegra/hdmi.c | 1 + > drivers/gpu/drm/vc4/vc4_hdmi.c | 1 + > include/sound/hdmi-codec.h | 1 - > 7 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c > b/drivers/gpu/drm/bridge/lontium-lt9611.c > index 9663601ce098..b9205d14d943 100644 > --- a/drivers/gpu/drm/bridge/lontium-lt9611.c > +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c > @@ -18,6 +18,7 @@ > > #include > #include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > index e971b75e90ad..f3f130c1ef0a 100644 > --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > @@ -21,6 +21,7 @@ > > #include > #include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 52d91a0df85e..fa63a21bdd1c 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -31,6 +31,7 @@ > #include > #include > #include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index d37d599aec27..c8e1bbebdffe 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > > #include "msm_drv.h" > #include "msm_kms.h" > diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c > index 417fb884240a..09987e372e3e 100644 > --- a/drivers/gpu/drm/tegra/hdmi.c > +++ b/drivers/gpu/drm/tegra/hdmi.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c > index f05e2c95a60d..34f807ed1c31 100644 > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > @@ -35,6 +35,7 @@ > #include > #include > #include > +#include > #include > #include > #include > diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h > index 9b162ac1e08e..5e1a9eafd10f 100644 > --- a/include/sound/hdmi-codec.h > +++ b/include/sound/hdmi-codec.h > @@ -12,7 +12,6 @@ > > #include > #include > -#include > #include > #include > #include -- Jani Nikula, Intel
[PATCH 2/2] xfs: disable large folio support in xfile_create
The xfarray code will crash if large folios are force enabled using: echo force > /sys/kernel/mm/transparent_hugepage/shmem_enabled Fixing this will require a bit of an API change, and prefeably sorting out the hwpoison story for pages vs folio and where it is placed in the shmem API. For now use this one liner to disable large folios. Reported-by: Darrick J. Wong Signed-off-by: Christoph Hellwig --- fs/xfs/scrub/xfile.c | 5 + 1 file changed, 5 insertions(+) diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c index 090c3ead43fdf1..1a8d1bedd0b0dc 100644 --- a/fs/xfs/scrub/xfile.c +++ b/fs/xfs/scrub/xfile.c @@ -94,6 +94,11 @@ xfile_create( lockdep_set_class(>i_rwsem, _i_mutex_key); + /* +* We're not quite ready for large folios yet. +*/ + mapping_clear_large_folios(inode->i_mapping); + trace_xfile_create(xf); *xfilep = xf; -- 2.39.2
[PATCH 1/2] mm: add a mapping_clear_large_folios helper
Users of shmem_kernel_file_setup might not be able to deal with large
folios (yet). Give them a way to disable large folio support on their
mapping.
Signed-off-by: Christoph Hellwig
---
include/linux/pagemap.h | 14 ++
1 file changed, 14 insertions(+)
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 06142ff7f9ce0e..352d1f8423292c 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -343,6 +343,20 @@ static inline void mapping_set_large_folios(struct
address_space *mapping)
__set_bit(AS_LARGE_FOLIO_SUPPORT, >flags);
}
+/**
+ * mapping_clear_large_folios() - Disable large folio support for a mapping
+ * @mapping: The mapping.
+ *
+ * This can be called to undo the effect of mapping_set_large_folios().
+ *
+ * Context: This should not be called while the inode is active as it
+ * is non-atomic.
+ */
+static inline void mapping_clear_large_folios(struct address_space *mapping)
+{
+ __clear_bit(AS_LARGE_FOLIO_SUPPORT, >flags);
+}
+
/*
* Large folio support currently depends on THP. These dependencies are
* being worked on but are not yet fixed.
--
2.39.2
disable large folios for shmem file used by xfs xfile
Hi all, Darrick reported that the fairly new XFS xfile code blows up when force enabling large folio for shmem. This series fixes this quickly by disabling large folios for this particular shmem file for now until it can be fixed properly, which will be a lot more invasive. I've added most of you to the CC list as I suspect most other users of shmem_file_setup and friends will have similar issues.
Re: [PATCH 1/3] drm/nouveau: include drm/drm_edid.h only where needed
On Tue, 09 Jan 2024, Danilo Krummrich wrote: > 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. Thanks for the review, pushed to drm-misc-next. You do still have T: git https://gitlab.freedesktop.org/drm/nouveau.git MAINTAINERS entry, so I thought that's where you apply patches. BR, Jani. -- Jani Nikula, Intel
Re: [PATCH v2 04/15] drm/i915: Bypass LMEMBAR/GTTMMADR for MTL stolen memory access
On 15.12.2023 11:59, Ville Syrjala wrote:
From: Ville Syrjälä
On MTL accessing stolen memory via the BARs is somehow borked,
and it can hang the machine. As a workaround let's bypass the
BARs and just go straight to DSMBASE/GSMBASE instead.
Note that on every other platform this itself would hang the
machine, but on MTL the system firmware is expected to relax
the access permission guarding stolen memory to enable this
workaround, and thus direct CPU accesses should be fine.
TODO: add w/a numbers and whatnot
Cc: Paz Zcharya
Cc: Nirmoy Das
Cc: Radhakrishna Sripada
Cc: Joonas Lahtinen
Signed-off-by: Ville Syrjälä
With w/a id added:
Reviewed-by: Andrzej Hajda
Regards
Andrzej
---
drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 11 ++-
drivers/gpu/drm/i915/gt/intel_ggtt.c | 13 -
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index ee237043c302..252fe5cd6ede 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -941,7 +941,16 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915,
u16 type,
dsm_size = ALIGN_DOWN(lmem_size - dsm_base, SZ_1M);
}
- if (pci_resource_len(pdev, GEN12_LMEM_BAR) < lmem_size) {
+ if (IS_METEORLAKE(i915)) {
+ /*
+* Workaround: access via BAR can hang MTL, go directly to DSM.
+*
+* Normally this would not work but on MTL the system firmware
+* should have relaxed the access permissions sufficiently.
+*/
+ io_start = intel_uncore_read64(uncore, GEN12_DSMBASE) &
GEN12_BDSM_MASK;
+ io_size = dsm_size;
+ } else if (pci_resource_len(pdev, GEN12_LMEM_BAR) < lmem_size) {
io_start = 0;
io_size = 0;
} else {
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 21a7e3191c18..ab71d74ec426 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -24,6 +24,7 @@
#include "intel_ring.h"
#include "i915_drv.h"
#include "i915_pci.h"
+#include "i915_reg.h"
#include "i915_request.h"
#include "i915_scatterlist.h"
#include "i915_utils.h"
@@ -1152,13 +1153,23 @@ static unsigned int gen6_gttadr_offset(struct
drm_i915_private *i915)
static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
{
struct drm_i915_private *i915 = ggtt->vm.i915;
+ struct intel_uncore *uncore = ggtt->vm.gt->uncore;
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
phys_addr_t phys_addr;
u32 pte_flags;
int ret;
GEM_WARN_ON(pci_resource_len(pdev, GEN4_GTTMMADR_BAR) != gen6_gttmmadr_size(i915));
- phys_addr = pci_resource_start(pdev, GEN4_GTTMMADR_BAR) +
gen6_gttadr_offset(i915);
+ /*
+* Workaround: access via BAR can hang MTL, go directly to GSM.
+*
+* Normally this would not work but on MTL the system firmware
+* should have relaxed the access permissions sufficiently.
+*/
+ if (IS_METEORLAKE(i915))
+ phys_addr = intel_uncore_read64(uncore, GEN12_GSMBASE) &
GEN12_BDSM_MASK;
+ else
+ phys_addr = pci_resource_start(pdev, GEN4_GTTMMADR_BAR) +
gen6_gttadr_offset(i915);
if (needs_wc_ggtt_mapping(i915))
ggtt->gsm = ioremap_wc(phys_addr, size);
Re: [PATCH v2 13/15] drm/i915/fbdev: Fix smem_start for LMEMBAR stolen objects
On 15.12.2023 11:59, Ville Syrjala wrote:
From: Ville Syrjälä
The "io" address of an object is its dma address minus the
region.start. Subtract the latter to make smem_start correct.
The current code happens to work for genuine LMEM objects
as LMEM region.start==0, but for LMEMBAR stolen objects
region.start!=0.
TODO: perhaps just set smem_start=0 always as our .fb_mmap()
implementation no longer depends on it? Need to double check
it's not needed for anything else...
Cc: Paz Zcharya
Signed-off-by: Ville Syrjälä
Reviewed-by: Andrzej Hajda
Regards
Andrzej
---
drivers/gpu/drm/i915/display/intel_fbdev_fb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
b/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
index 1ac05d90b2e8..0665f943f65f 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
@@ -79,7 +79,8 @@ int intel_fbdev_fb_fill_info(struct drm_i915_private *i915,
struct fb_info *info
/* Use fbdev's framebuffer from lmem for discrete */
info->fix.smem_start =
(unsigned long)(mem->io.start +
- i915_gem_object_get_dma_address(obj,
0));
+ i915_gem_object_get_dma_address(obj, 0)
-
+ mem->region.start);
info->fix.smem_len = obj->base.size;
} else {
struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
Re: Rework TTMs busy handling
On 2024-01-09 09:34, Christian König wrote: > Am 09.01.24 um 09:14 schrieb Thomas Hellström: >> On Tue, 2024-01-09 at 08:47 +0100, Christian König wrote: >>> >>> 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. >> >> 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. Maybe something like "desired" vs "fallback"? -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [PATCH 1/7] drm: Add eDP 1.5 early transport definition
Hello All, I accidentally pushed this patch into drm-intel/drm-intel-next. Hopefully this doesn't cause problems. I'm very sorry for inconvenience. Best Regards, Jouni Högander On Wed, 2024-01-03 at 10:46 +, 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 > > Cc: dri-de...@lists.freedesktop.org > > Subject: [PATCH 1/7] drm: Add eDP 1.5 early transport definition > > > > Add DP_PSR_ENABLE_SU_REGION_ET to enable panel early transport. > > > > Cc: dri-de...@lists.freedesktop.org > > > > Reviewed-by: Mika Kahola > > > Signed-off-by: Jouni Högander > > --- > > include/drm/display/drm_dp.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/include/drm/display/drm_dp.h > > b/include/drm/display/drm_dp.h index 3731828825bd..281afff6ee4e > > 100644 > > --- a/include/drm/display/drm_dp.h > > +++ b/include/drm/display/drm_dp.h > > @@ -718,6 +718,7 @@ > > # define DP_PSR_SU_REGION_SCANLINE_CAPTURE BIT(4) /* eDP 1.4a > > */ > > # define DP_PSR_IRQ_HPD_WITH_CRC_ERRORSBIT(5) /* > > eDP 1.4a */ > > # define DP_PSR_ENABLE_PSR2BIT(6) /* eDP 1.4a > > */ > > +# define DP_PSR_ENABLE_SU_REGION_ET BIT(7) /* eDP 1.5 > > */ > > > > #define DP_ADAPTER_CTRL 0x1a0 > > # define DP_ADAPTER_CTRL_FORCE_LOAD_SENSE (1 << 0) > > -- > > 2.34.1 >
