Re: [PATCH 2/7] drm/i915/hdcp: HDCP Capability for the downstream device
On 1/24/2024 6:50 PM, Nautiyal, Ankit K wrote:
On 1/12/2024 1:11 PM, Suraj Kandpal wrote:
Currently we are only checking capability of remote device and not
immediate downstream device but during capability check we need are
concerned with only the HDCP capability of downstream device.
During i915_display_info reporting we need HDCP Capability for both
the monitors and downstream branch device if any this patch adds that.
I agree cases where MST hub/docker and sink are of different
capabilities, this creates a confusion.
with this change, perhaps kms_content_protection IGT can also be
changed to check for MST hub's capability.
Only thing is that for hdmi the 'remote_req' doesnt make sense.
Instead of changing the hdcp_2_2_capable can we just have a separate
function for intel_dp_remote_hdcp2_capable(), which uses aux =
>port->aux.
The common code for reading HDCP2_2 Rx caps can be pulled out in a
separate function, which we can call only in case of MST when we read
remote.
Also we might need to have similar thing for HDCP1.4.
Regards,
Ankit
Signed-off-by: Suraj Kandpal
---
.../drm/i915/display/intel_display_debugfs.c | 19 +++
.../drm/i915/display/intel_display_types.h | 2 +-
drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 4 ++--
drivers/gpu/drm/i915/display/intel_hdcp.c | 6 +++---
drivers/gpu/drm/i915/display/intel_hdcp.h | 2 +-
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
6 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index d951edb36687..457f13357fad 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -210,7 +210,8 @@ static void intel_panel_info(struct seq_file *m,
}
static void intel_hdcp_info(struct seq_file *m,
- struct intel_connector *intel_connector)
+ struct intel_connector *intel_connector,
+ bool remote_req)
{
bool hdcp_cap, hdcp2_cap;
@@ -220,7 +221,7 @@ static void intel_hdcp_info(struct seq_file *m,
}
hdcp_cap = intel_hdcp_capable(intel_connector);
- hdcp2_cap = intel_hdcp2_capable(intel_connector);
+ hdcp2_cap = intel_hdcp2_capable(intel_connector, remote_req);
if (hdcp_cap)
seq_puts(m, "HDCP1.4 ");
@@ -307,7 +308,12 @@ static void intel_connector_info(struct seq_file
*m,
}
seq_puts(m, "\tHDCP version: ");
- intel_hdcp_info(m, intel_connector);
+ intel_hdcp_info(m, intel_connector, true);
+
+ if (intel_encoder_is_mst(encoder)) {
+ seq_puts(m, "\tHDCP Branch Device version: ");
+ intel_hdcp_info(m, intel_connector, false);
+ }
seq_printf(m, "\tmax bpc: %u\n", connector->display_info.bpc);
@@ -1153,7 +1159,12 @@ static int
i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
seq_printf(m, "%s:%d HDCP version: ", connector->base.name,
connector->base.base.id);
- intel_hdcp_info(m, connector);
+ intel_hdcp_info(m, connector, true);
+
+ if (intel_encoder_is_mst(connector->encoder)) {
+ seq_puts(m, "\tHDCP Branch Device version: ");
Perhaps MST HUB HDCP version?
+ intel_hdcp_info(m, connector, false);
+ }
out:
drm_modeset_unlock(>drm.mode_config.connection_mutex);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index ae2e8cff9d69..aa559598f049 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -507,7 +507,7 @@ struct intel_hdcp_shim {
/* Detects whether sink is HDCP2.2 capable */
int (*hdcp_2_2_capable)(struct intel_connector *connector,
- bool *capable);
+ bool *capable, bool remote_req);
/* Write HDCP2.2 messages */
int (*write_2_2_msg)(struct intel_connector *connector,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
index bec49061b2e1..90b027ba3302 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
@@ -649,13 +649,13 @@ int intel_dp_hdcp2_check_link(struct
intel_digital_port *dig_port,
static
int intel_dp_hdcp2_capable(struct intel_connector *connector,
- bool *capable)
+ bool *capable, bool remote_req)
{
struct drm_dp_aux *aux;
u8 rx_caps[3];
int ret;
- aux = intel_dp_hdcp_get_aux(connector, true);
+ aux = intel_dp_hdcp_get_aux(connector, remote_req);
Inline with the comments on the previous patch, this would also be
required to change.
Otherwise the patch looks good to me.
Regards,
Ankit
*capable = false;
ret = drm_dp_dpcd_read(aux,
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
Re: [PATCH] drm/i915/guc: Remove usage of the deprecated ida_simple_xx() API
On Sun, Jan 14, 2024 at 04:15:34PM +0100, Christophe JAILLET wrote:
> ida_alloc() and ida_free() should be preferred to the deprecated
> ida_simple_get() and ida_simple_remove().
>
> Note that the upper limit of ida_simple_get() is exclusive, but the one of
> ida_alloc_range() is inclusive. So a -1 has been added when needed.
>
> Signed-off-by: Christophe JAILLET
Reviewed-by: Matthew Brost
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 13 ++---
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index a259f1118c5a..73ce21ddf682 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -2156,11 +2156,10 @@ static int new_guc_id(struct intel_guc *guc, struct
> intel_context *ce)
>
> order_base_2(ce->parallel.number_children
> + 1));
> else
> - ret = ida_simple_get(>submission_state.guc_ids,
> - NUMBER_MULTI_LRC_GUC_ID(guc),
> - guc->submission_state.num_guc_ids,
> - GFP_KERNEL | __GFP_RETRY_MAYFAIL |
> - __GFP_NOWARN);
> + ret = ida_alloc_range(>submission_state.guc_ids,
> + NUMBER_MULTI_LRC_GUC_ID(guc),
> + guc->submission_state.num_guc_ids - 1,
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL |
> __GFP_NOWARN);
> if (unlikely(ret < 0))
> return ret;
>
> @@ -2183,8 +2182,8 @@ static void __release_guc_id(struct intel_guc *guc,
> struct intel_context *ce)
> + 1));
> } else {
> --guc->submission_state.guc_ids_in_use;
> - ida_simple_remove(>submission_state.guc_ids,
> - ce->guc_id.id);
> + ida_free(>submission_state.guc_ids,
> + ce->guc_id.id);
> }
> clr_ctx_id_mapping(guc, ce->guc_id.id);
> set_context_guc_id_invalid(ce);
> --
> 2.43.0
>
Re: [RFC PATCH] drm/i915: Add GETPARAM for GuC submission version
On Wednesday, January 24, 2024 12:55:56 AM PST Tvrtko Ursulin wrote: > > On 24/01/2024 08:19, Joonas Lahtinen wrote: > > Add reporting of the GuC submissio/VF interface version via GETPARAM > > properties. Mesa intends to use this information to check for old > > firmware versions with known bugs before enabling features like async > > compute. > > There was > https://patchwork.freedesktop.org/patch/560704/?series=124592=1 > which does everything in one go so would be my preference. Joonas's patch posted here is: Reviewed-by: Kenneth Graunke I'm fine with the approach that Tvrtko linked as well, querying all three in one ioctl makes some sense. That particular patch looked like it needed some (trivial) cleaning up before landing, though. Either approach works for me. I agree with John, the submission version should be fine for us. Thanks a lot for taking care of this. signature.asc Description: This is a digitally signed message part.
Re: linux-next: Tree for Jan 23 (drm/xe/)
On 1/24/24 01:17, Jani Nikula wrote: > On Tue, 23 Jan 2024, Randy Dunlap wrote: >> On 1/22/24 18:29, Stephen Rothwell wrote: >>> Hi all, >>> >>> News: there will be no linux-next release on Friday >>> >>> Changes since 20240122: >>> >> >> on ARM64, when >> DRM_I915 is not set >> DRM_XE=m >> DEBUG_FS is not set >> >> ../drivers/gpu/drm/i915/display/intel_display_debugfs.c:1091:6: error: >> redefinition of 'intel_display_debugfs_register' >> 1091 | void intel_display_debugfs_register(struct drm_i915_private *i915) >> | ^~ >> In file included from >> ../drivers/gpu/drm/i915/display/intel_display_debugfs.c:19: > > Does [1] fix the issue? Yes, thanks. > > BR, > Jani. > > > [1] https://lore.kernel.org/r/20240124090515.3363901-1-jani.nik...@intel.com > > -- #Randy
Re: [RFC PATCH] drm/i915: Add GETPARAM for GuC submission version
On 1/24/2024 00:55, Tvrtko Ursulin wrote:
On 24/01/2024 08:19, Joonas Lahtinen wrote:
Add reporting of the GuC submissio/VF interface version via GETPARAM
properties. Mesa intends to use this information to check for old
firmware versions with known bugs before enabling features like async
compute.
There was
https://patchwork.freedesktop.org/patch/560704/?series=124592=1
which does everything in one go so would be my preference.
I also think that the original version is a cleaner implementation.
During the time of that patch there was discussion whether firmware
version or submission version was better. I vaguely remember someone
raised an issue with the latter. Adding John in case he remembers.
The file version number should not be exposed to UMDs, only the
submission version. The whole purpose of the submission version is to
track user facing changes. There was a very, very, very long discussion
about that to which all parties did eventually agree on using the
submission version.
The outstanding issues were simply a) whether UMDs should be tracking
version numbers and all the complications that arise with branching and
non-linear numbering, b) should it just be exposed as a feature flag
instead and c) this will prevent hangs in certain specific situations
but it won't prevent the system running slowly and not using the full
capabilities of the hardware, for that we need to be making sure that
distros actually update to a firmware release that is not ancient.
Signed-off-by: Joonas Lahtinen
Cc: Kenneth Graunke
Cc: Jose Souza
Cc: Sagar Ghuge
Cc: Paulo Zanoni
Cc: John Harrison
Cc: Rodrigo Vivi
Cc: Jani Nikula
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_getparam.c | 12
include/uapi/drm/i915_drm.h | 13 +
2 files changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_getparam.c
b/drivers/gpu/drm/i915/i915_getparam.c
index 5c3fec63cb4c1..f176372debc54 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -113,6 +113,18 @@ int i915_getparam_ioctl(struct drm_device *dev,
void *data,
if (value < 0)
return value;
break;
+ case I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR:
+ case I915_PARAM_GUC_SUBMISSION_VERSION_MINOR:
+ case I915_PARAM_GUC_SUBMISSION_VERSION_PATCH:
+ if (!intel_uc_uses_guc_submission(_gt(i915)->uc))
+ return -ENODEV;
+ if (param->param == I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR)
+ value = to_gt(i915)->uc.guc.submission_version.major;
+ else if (param->param ==
I915_PARAM_GUC_SUBMISSION_VERSION_MINOR)
+ value = to_gt(i915)->uc.guc.submission_version.minor;
+ else
+ value = to_gt(i915)->uc.guc.submission_version.patch;
+ break;
case I915_PARAM_MMAP_GTT_VERSION:
/* Though we've started our numbering from 1, and so class all
* earlier versions as 0, in effect their value is
undefined as
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index fd4f9574d177a..7d5a47f182542 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -806,6 +806,19 @@ typedef struct drm_i915_irq_wait {
*/
#define I915_PARAM_PXP_STATUS 58
+/*
+ * Query for the GuC submission/VF interface version number
What is this VF you speak of? :/
Agreed. There is no SRIOV support in i915 so i915 should not be
mentioning SRIOV specific features.
John.
Regards,
Tvrtko
+ *
+ * -ENODEV is returned if GuC submission is not used
+ *
+ * On success, returns the respective GuC submission/VF interface
major,
+ * minor or patch version as per the requested parameter.
+ *
+ */
+#define I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR 59
+#define I915_PARAM_GUC_SUBMISSION_VERSION_MINOR 60
+#define I915_PARAM_GUC_SUBMISSION_VERSION_PATCH 61
+
/* Must be kept compact -- no holes and well documented */
/**
Re: Re: [PATCH 1/3] bits: introduce fixed-type genmasks
Quoting Yury Norov (2024-01-24 12:27:58-03:00) >On Wed, Jan 24, 2024 at 08:03:53AM -0600, Lucas De Marchi wrote: >> On Wed, Jan 24, 2024 at 09:58:26AM +0200, Jani Nikula wrote: >> > On Tue, 23 Jan 2024, Lucas De Marchi wrote: >> > > From: Yury Norov >> > > >> > > Generalize __GENMASK() to support different types, and implement >> > > fixed-types versions of GENMASK() based on it. The fixed-type version >> > > allows more strict checks to the min/max values accepted, which is >> > > useful for defining registers like implemented by i915 and xe drivers >> > > with their REG_GENMASK*() macros. >> > >> > Mmh, the commit message says the fixed-type version allows more strict >> > checks, but none are actually added. GENMASK_INPUT_CHECK() remains the >> > same. >> > >> > Compared to the i915 and xe versions, this is more lax now. You could >> > specify GENMASK_U32(63,32) without complaints. >> >> Doing this on top of the this series: >> >> -#define XELPDP_PORT_M2P_COMMAND_TYPE_MASKREG_GENMASK(30, 27) >> +#define XELPDP_PORT_M2P_COMMAND_TYPE_MASKREG_GENMASK(62, 32) >> >> and I do get a build failure: >> >> ../drivers/gpu/drm/i915/display/intel_cx0_phy.c: In function >> ‘__intel_cx0_read_once’: >> ../include/linux/bits.h:41:31: error: left shift count >= width of type >> [-Werror=shift-count-overflow] >>41 | (((t)~0ULL - ((t)(1) << (l)) + 1) & \ >> | ^~ > >I would better include this in commit message to avoid people's >confusion. If it comes to v2, can you please do it and mention that >this trick relies on shift-count-overflow compiler check? Wouldn't it be better to have explicit check that l and h are not out of bounds based on BITS_PER_TYPE() than relying on a compiler flag that could be turned off (maybe for some questionable reason, but even so)? -- Gustavo Sousa
Re: Re: [PATCH 1/3] bits: introduce fixed-type genmasks
On Wed, Jan 24, 2024 at 09:58:26AM +0200, Jani Nikula wrote: On Tue, 23 Jan 2024, Lucas De Marchi wrote: From: Yury Norov Generalize __GENMASK() to support different types, and implement fixed-types versions of GENMASK() based on it. The fixed-type version allows more strict checks to the min/max values accepted, which is useful for defining registers like implemented by i915 and xe drivers with their REG_GENMASK*() macros. Mmh, the commit message says the fixed-type version allows more strict checks, but none are actually added. GENMASK_INPUT_CHECK() remains the same. Compared to the i915 and xe versions, this is more lax now. You could specify GENMASK_U32(63,32) without complaints. Doing this on top of the this series: -#define XELPDP_PORT_M2P_COMMAND_TYPE_MASKREG_GENMASK(30, 27) +#define XELPDP_PORT_M2P_COMMAND_TYPE_MASKREG_GENMASK(62, 32) and I do get a build failure: ../drivers/gpu/drm/i915/display/intel_cx0_phy.c: In function ‘__intel_cx0_read_once’: ../include/linux/bits.h:41:31: error: left shift count >= width of type [-Werror=shift-count-overflow] 41 | (((t)~0ULL - ((t)(1) << (l)) + 1) & \ | ^~ I also tested them individually. All of these fail to build for me: 1) -#define XELPDP_PORT_M2P_COMMAND_TYPE_MASKREG_GENMASK(30, 27) +#define XELPDP_PORT_M2P_COMMAND_TYPE_MASKREG_GENMASK(32, 27) 2) -#define XELPDP_PORT_M2P_COMMAND_TYPE_MASKREG_GENMASK(30, 27) +#define XELPDP_PORT_M2P_COMMAND_TYPE_MASKREG_GENMASK(30, 31) 3) -#define XELPDP_PORT_M2P_COMMAND_TYPE_MASKREG_GENMASK(30, 27) +#define XELPDP_PORT_M2P_COMMAND_TYPE_MASKREG_GENMASK(30, -1) Lucas De Marchi
✗ Fi.CI.BAT: failure for drm/i915: Fix VMA UAF on destroy against deactivate race (rev2)
== Series Details == Series: drm/i915: Fix VMA UAF on destroy against deactivate race (rev2) URL : https://patchwork.freedesktop.org/series/129026/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14168 -> Patchwork_129026v2 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_129026v2 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_129026v2, 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_129026v2/index.html Participating hosts (38 -> 35) -- Missing(3): bat-rpls-3 bat-jsl-1 fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_129026v2: ### IGT changes ### Possible regressions * igt@gem_lmem_swapping@random-engines@lmem0: - bat-dg1-7: [PASS][1] -> [ABORT][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/bat-dg1-7/igt@gem_lmem_swapping@random-engi...@lmem0.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129026v2/bat-dg1-7/igt@gem_lmem_swapping@random-engi...@lmem0.html * igt@i915_selftest@live@reset: - bat-adlm-1: [PASS][3] -> [INCOMPLETE][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/bat-adlm-1/igt@i915_selftest@l...@reset.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129026v2/bat-adlm-1/igt@i915_selftest@l...@reset.html * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size: - fi-tgl-1115g4: [PASS][5] -> [SKIP][6] +5 other tests skip [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-flip-after-cursor-varying-size.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129026v2/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-flip-after-cursor-varying-size.html * igt@kms_pipe_crc_basic@hang-read-crc: - fi-tgl-1115g4: NOTRUN -> [SKIP][7] +6 other tests skip [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129026v2/fi-tgl-1115g4/igt@kms_pipe_crc_ba...@hang-read-crc.html Warnings * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - fi-tgl-1115g4: [SKIP][8] ([i915#4103]) -> [SKIP][9] +1 other test skip [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129026v2/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html * igt@kms_dsc@dsc-basic: - fi-tgl-1115g4: [SKIP][10] ([i915#3555] / [i915#3840]) -> [SKIP][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@kms_...@dsc-basic.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129026v2/fi-tgl-1115g4/igt@kms_...@dsc-basic.html Known issues Here are the changes found in Patchwork_129026v2 that come from known issues: ### CI changes ### Issues hit * boot: - bat-rpls-2: [PASS][12] -> [FAIL][13] ([i915#10078]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/bat-rpls-2/boot.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129026v2/bat-rpls-2/boot.html ### IGT changes ### Issues hit * igt@fbdev@info: - fi-tgl-1115g4: [PASS][14] -> [SKIP][15] ([i915#1849] / [i915#2582]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@fb...@info.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129026v2/fi-tgl-1115g4/igt@fb...@info.html * igt@fbdev@nullptr: - fi-tgl-1115g4: [PASS][16] -> [SKIP][17] ([i915#2582]) +3 other tests skip [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@fb...@nullptr.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129026v2/fi-tgl-1115g4/igt@fb...@nullptr.html * igt@gem_lmem_swapping@basic: - fi-apl-guc: NOTRUN -> [SKIP][18] ([fdo#109271] / [i915#4613]) +3 other tests skip [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129026v2/fi-apl-guc/igt@gem_lmem_swapp...@basic.html * igt@gem_lmem_swapping@verify-random: - bat-mtlp-8: NOTRUN -> [SKIP][19] ([i915#4613]) +3 other tests skip [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129026v2/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html * igt@gem_tiled_blits@basic: - fi-pnv-d510:[PASS][20] -> [SKIP][21] ([fdo#109271]) +1 other test skip [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-pnv-d510/igt@gem_tiled_bl...@basic.html [21]:
✗ Fi.CI.SPARSE: warning for drm/i915: Fix VMA UAF on destroy against deactivate race (rev2)
== Series Details == Series: drm/i915: Fix VMA UAF on destroy against deactivate race (rev2) URL : https://patchwork.freedesktop.org/series/129026/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
Re: [RFC PATCH] drm/i915: Add GETPARAM for GuC submission version
On Wed, 2024-01-24 at 10:19 +0200, Joonas Lahtinen wrote:
> Add reporting of the GuC submissio/VF interface version via GETPARAM
> properties. Mesa intends to use this information to check for old
> firmware versions with known bugs before enabling features like async
> compute.
>
Reviewed-by: José Roberto de Souza
> Signed-off-by: Joonas Lahtinen
> Cc: Kenneth Graunke
> Cc: Jose Souza
> Cc: Sagar Ghuge
> Cc: Paulo Zanoni
> Cc: John Harrison
> Cc: Rodrigo Vivi
> Cc: Jani Nikula
> Cc: Tvrtko Ursulin
> ---
> drivers/gpu/drm/i915/i915_getparam.c | 12
> include/uapi/drm/i915_drm.h | 13 +
> 2 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_getparam.c
> b/drivers/gpu/drm/i915/i915_getparam.c
> index 5c3fec63cb4c1..f176372debc54 100644
> --- a/drivers/gpu/drm/i915/i915_getparam.c
> +++ b/drivers/gpu/drm/i915/i915_getparam.c
> @@ -113,6 +113,18 @@ int i915_getparam_ioctl(struct drm_device *dev, void
> *data,
> if (value < 0)
> return value;
> break;
> + case I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR:
> + case I915_PARAM_GUC_SUBMISSION_VERSION_MINOR:
> + case I915_PARAM_GUC_SUBMISSION_VERSION_PATCH:
> + if (!intel_uc_uses_guc_submission(_gt(i915)->uc))
> + return -ENODEV;
> + if (param->param == I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR)
> + value = to_gt(i915)->uc.guc.submission_version.major;
> + else if (param->param ==
> I915_PARAM_GUC_SUBMISSION_VERSION_MINOR)
> + value = to_gt(i915)->uc.guc.submission_version.minor;
> + else
> + value = to_gt(i915)->uc.guc.submission_version.patch;
> + break;
> case I915_PARAM_MMAP_GTT_VERSION:
> /* Though we've started our numbering from 1, and so class all
>* earlier versions as 0, in effect their value is undefined as
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index fd4f9574d177a..7d5a47f182542 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -806,6 +806,19 @@ typedef struct drm_i915_irq_wait {
> */
> #define I915_PARAM_PXP_STATUS 58
>
> +/*
> + * Query for the GuC submission/VF interface version number
> + *
> + * -ENODEV is returned if GuC submission is not used
> + *
> + * On success, returns the respective GuC submission/VF interface major,
> + * minor or patch version as per the requested parameter.
> + *
> + */
> +#define I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR 59
> +#define I915_PARAM_GUC_SUBMISSION_VERSION_MINOR 60
> +#define I915_PARAM_GUC_SUBMISSION_VERSION_PATCH 61
> +
> /* Must be kept compact -- no holes and well documented */
>
> /**
Re: [PATCH] drm/i915: limit eDP MSO pipe only for display version 20 and below
Hi, Luca!
Quoting Luca Coelho (2024-01-24 05:52:29-03:00)
>The pipes that can be used for eDP MSO are limited to pipe A (and
>sometimes also pipe B) only for display version 20 and below.
>
>Modify the function that returns the pipe mask for eDP MSO so that
>these limitations only apply to version 20 and below, enabling all
>pipes otherwise.
>
>Bspec: 68923
>Cc: Jani Nikula
>Cc: James Ausmus
>Signed-off-by: Luca Coelho
>---
> drivers/gpu/drm/i915/display/intel_ddi.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
>b/drivers/gpu/drm/i915/display/intel_ddi.c
>index 922194b957be..5c99ae148213 100644
>--- a/drivers/gpu/drm/i915/display/intel_ddi.c
>+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>@@ -2336,13 +2336,18 @@ static void intel_ddi_power_up_lanes(struct
>intel_encoder *encoder,
> }
> }
>
>-/* Splitter enable for eDP MSO is limited to certain pipes. */
>+/*
>+ * Splitter enable for eDP MSO is limited to certain pipes, on certain
>+ * platforms.
>+ */
> static u8 intel_ddi_splitter_pipe_mask(struct drm_i915_private *i915)
> {
> if (IS_ALDERLAKE_P(i915))
Looks like Xe_LPD+ (MTL's display) and Xe2_LPD (LNL's display) both support both
pipes A and B. For Xe_LPD+ we have that info in BSpec 55473 and for Xe2_LPD, in
BSpec 68923. So, I think we could:
a. OR the condition above with IS_DISPLAY_IP_RANGE(i915, IP_VER(14, 0),
IP_VER(20, 0)), and
b. And make the "else if" below be about display versions below 14.
With those additions,
Reviewed-by: Gustavo Sousa
--
Gustavo Sousa
> return BIT(PIPE_A) | BIT(PIPE_B);
>-else
>+else if (DISPLAY_VER(i915) <= 20)
> return BIT(PIPE_A);
>+
>+return ~0;
> }
>
> static void intel_ddi_mso_get_config(struct intel_encoder *encoder,
>--
>2.39.2
>
Re: [PATCH 2/7] drm/i915/hdcp: HDCP Capability for the downstream device
On 1/12/2024 1:11 PM, Suraj Kandpal wrote:
Currently we are only checking capability of remote device and not
immediate downstream device but during capability check we need are
concerned with only the HDCP capability of downstream device.
During i915_display_info reporting we need HDCP Capability for both
the monitors and downstream branch device if any this patch adds that.
I agree cases where MST hub/docker and sink are of different
capabilities, this creates a confusion.
with this change, perhaps kms_content_protection IGT can also be changed
to check for MST hub's capability.
Only thing is that for hdmi the 'remote_req' doesnt make sense.
Signed-off-by: Suraj Kandpal
---
.../drm/i915/display/intel_display_debugfs.c | 19 +++
.../drm/i915/display/intel_display_types.h| 2 +-
drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 4 ++--
drivers/gpu/drm/i915/display/intel_hdcp.c | 6 +++---
drivers/gpu/drm/i915/display/intel_hdcp.h | 2 +-
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
6 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index d951edb36687..457f13357fad 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -210,7 +210,8 @@ static void intel_panel_info(struct seq_file *m,
}
static void intel_hdcp_info(struct seq_file *m,
- struct intel_connector *intel_connector)
+ struct intel_connector *intel_connector,
+ bool remote_req)
{
bool hdcp_cap, hdcp2_cap;
@@ -220,7 +221,7 @@ static void intel_hdcp_info(struct seq_file *m,
}
hdcp_cap = intel_hdcp_capable(intel_connector);
- hdcp2_cap = intel_hdcp2_capable(intel_connector);
+ hdcp2_cap = intel_hdcp2_capable(intel_connector, remote_req);
if (hdcp_cap)
seq_puts(m, "HDCP1.4 ");
@@ -307,7 +308,12 @@ static void intel_connector_info(struct seq_file *m,
}
seq_puts(m, "\tHDCP version: ");
- intel_hdcp_info(m, intel_connector);
+ intel_hdcp_info(m, intel_connector, true);
+
+ if (intel_encoder_is_mst(encoder)) {
+ seq_puts(m, "\tHDCP Branch Device version: ");
+ intel_hdcp_info(m, intel_connector, false);
+ }
seq_printf(m, "\tmax bpc: %u\n", connector->display_info.bpc);
@@ -1153,7 +1159,12 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
seq_printf(m, "%s:%d HDCP version: ", connector->base.name,
connector->base.base.id);
- intel_hdcp_info(m, connector);
+ intel_hdcp_info(m, connector, true);
+
+ if (intel_encoder_is_mst(connector->encoder)) {
+ seq_puts(m, "\tHDCP Branch Device version: ");
Perhaps MST HUB HDCP version?
+ intel_hdcp_info(m, connector, false);
+ }
out:
drm_modeset_unlock(>drm.mode_config.connection_mutex);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index ae2e8cff9d69..aa559598f049 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -507,7 +507,7 @@ struct intel_hdcp_shim {
/* Detects whether sink is HDCP2.2 capable */
int (*hdcp_2_2_capable)(struct intel_connector *connector,
- bool *capable);
+ bool *capable, bool remote_req);
/* Write HDCP2.2 messages */
int (*write_2_2_msg)(struct intel_connector *connector,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
index bec49061b2e1..90b027ba3302 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
@@ -649,13 +649,13 @@ int intel_dp_hdcp2_check_link(struct intel_digital_port
*dig_port,
static
int intel_dp_hdcp2_capable(struct intel_connector *connector,
- bool *capable)
+ bool *capable, bool remote_req)
{
struct drm_dp_aux *aux;
u8 rx_caps[3];
int ret;
- aux = intel_dp_hdcp_get_aux(connector, true);
+ aux = intel_dp_hdcp_get_aux(connector, remote_req);
Inline with the comments on the previous patch, this would also be
required to change.
Otherwise the patch looks good to me.
Regards,
Ankit
*capable = false;
ret = drm_dp_dpcd_read(aux,
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
b/drivers/gpu/drm/i915/display/intel_hdcp.c
index c3e692e7f790..b88a4713e6a8 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -161,7 +161,7 @@ bool intel_hdcp_capable(struct intel_connector
[PATCH v5 3/3] Revert "drm/i915: Wait for active retire before i915_active_fini()"
This reverts commit 7a2280e8dcd2f1f436db9631287c0b21cf6a92b0, obsoleted by "drm/i915/vma: Fix UAF on destroy against retire race". Signed-off-by: Janusz Krzysztofik Cc: Nirmoy Das --- drivers/gpu/drm/i915/i915_vma.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 604d420b9e1fd..09b8a6b52d065 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -1752,8 +1752,6 @@ static void release_references(struct i915_vma *vma, struct intel_gt *gt, if (vm_ddestroy) i915_vm_resv_put(vma->vm); - /* Wait for async active retire */ - i915_active_wait(>active); i915_active_fini(>active); GEM_WARN_ON(vma->resource); i915_vma_free(vma); -- 2.43.0
[PATCH v5 2/3] drm/i915: Remove extra multi-gt pm-references
There was an attempt to fix an issue of illegal attempts to free a still
active i915 VMA object when parking a GT believed to be idle, reported by
CI on 2-GT Meteor Lake. As a solution, an extra wakeref for a Primary GT
was acquired from i915_gem_do_execbuffer() -- see commit f56fe3e91787
("drm/i915: Fix a VMA UAF for multi-gt platform").
However, that fix occurred insufficient -- the issue was still reported by
CI. That wakeref was released on exit from i915_gem_do_execbuffer(), then
potentially before completion of the request and deactivation of its
associated VMAs. Moreover, CI reports indicated that single-GT platforms
also suffered sporadically from the same race.
Since the issue has now been fixed by a preceding patch "drm/i915/vma: Fix
UAF on destroy against retire race", drop the no longer useful changes
introduced by that insufficient fix.
v2: Avoid the word "revert" in commit message (Rodrigo),
- update commit description reusing relevant chunks dropped from the
description of the proper fix (Rodrigo).
Signed-off-by: Janusz Krzysztofik
Cc: Nirmoy Das
Cc: Rodrigo Vivi
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 17 -
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index d3a771afb083e..cedca8fd8754d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2686,7 +2686,6 @@ static int
eb_select_engine(struct i915_execbuffer *eb)
{
struct intel_context *ce, *child;
- struct intel_gt *gt;
unsigned int idx;
int err;
@@ -2710,17 +2709,10 @@ eb_select_engine(struct i915_execbuffer *eb)
}
}
eb->num_batches = ce->parallel.number_children + 1;
- gt = ce->engine->gt;
for_each_child(ce, child)
intel_context_get(child);
eb->wakeref = intel_gt_pm_get(ce->engine->gt);
- /*
-* Keep GT0 active on MTL so that i915_vma_parked() doesn't
-* free VMAs while execbuf ioctl is validating VMAs.
-*/
- if (gt->info.id)
- eb->wakeref_gt0 = intel_gt_pm_get(to_gt(gt->i915));
if (!test_bit(CONTEXT_ALLOC_BIT, >flags)) {
err = intel_context_alloc_state(ce);
@@ -2759,9 +2751,6 @@ eb_select_engine(struct i915_execbuffer *eb)
return err;
err:
- if (gt->info.id)
- intel_gt_pm_put(to_gt(gt->i915), eb->wakeref_gt0);
-
intel_gt_pm_put(ce->engine->gt, eb->wakeref);
for_each_child(ce, child)
intel_context_put(child);
@@ -2775,12 +2764,6 @@ eb_put_engine(struct i915_execbuffer *eb)
struct intel_context *child;
i915_vm_put(eb->context->vm);
- /*
-* This works in conjunction with eb_select_engine() to prevent
-* i915_vma_parked() from interfering while execbuf validates vmas.
-*/
- if (eb->gt->info.id)
- intel_gt_pm_put(to_gt(eb->gt->i915), eb->wakeref_gt0);
intel_gt_pm_put(eb->context->engine->gt, eb->wakeref);
for_each_child(eb->context, child)
intel_context_put(child);
--
2.43.0
[PATCH v5 1/3] drm/i915/vma: Fix UAF on destroy against retire race
Object debugging tools were sporadically reporting illegal attempts to
free a still active i915 VMA object when parking a GT believed to be idle.
[161.359441] ODEBUG: free active (active state 0) object: 88811643b958
object type: i915_active hint: __i915_vma_active+0x0/0x50 [i915]
[161.360082] WARNING: CPU: 5 PID: 276 at lib/debugobjects.c:514
debug_print_object+0x80/0xb0
...
[161.360304] CPU: 5 PID: 276 Comm: kworker/5:2 Not tainted
6.5.0-rc1-CI_DRM_13375-g003f860e5577+ #1
[161.360314] Hardware name: Intel Corporation Rocket Lake Client
Platform/RocketLake S UDIMM 6L RVP, BIOS RKLSFWI1.R00.3173.A03.2204210138
04/21/2022
[161.360322] Workqueue: i915-unordered __intel_wakeref_put_work [i915]
[161.360592] RIP: 0010:debug_print_object+0x80/0xb0
...
[161.361347] debug_object_free+0xeb/0x110
[161.361362] i915_active_fini+0x14/0x130 [i915]
[161.361866] release_references+0xfe/0x1f0 [i915]
[161.362543] i915_vma_parked+0x1db/0x380 [i915]
[161.363129] __gt_park+0x121/0x230 [i915]
[161.363515] intel_wakeref_put_last+0x1f/0x70 [i915]
That has been tracked down to be happening when another thread is
deactivating the VMA inside __active_retire() helper, after the VMA's
active counter has been already decremented to 0, but before deactivation
of the VMA's object is reported to the object debugging tool.
We could prevent from that race by serializing i915_active_fini() with
__active_retire() via ref->tree_lock, but that wouldn't stop the VMA from
being used, e.g. from __i915_vma_retire() called at the end of
__active_retire(), after that VMA has been already freed by a concurrent
i915_vma_destroy() on return from the i915_active_fini(). Then, we should
rather fix the issue at the VMA level, not in i915_active.
Since __i915_vma_parked() is called from __gt_park() on last put of the
GT's wakeref, the issue could be addressed by holding the GT wakeref long
enough for __active_retire() to complete before that wakeref is released
and the GT parked.
I believe the issue was introduced by commit d93939730347 ("drm/i915:
Remove the vma refcount") which moved a call to i915_active_fini() from
a dropped i915_vma_release(), called on last put of the removed VMA kref,
to i915_vma_parked() processing path called on last put of a GT wakeref.
However, its visibility to the object debugging tool was suppressed by a
bug in i915_active that was fixed two weeks later with commit e92eb246feb9
("drm/i915/active: Fix missing debug object activation").
A VMA associated with a request doesn't acquire a GT wakeref by itself.
Instead, it depends on a wakeref held directly by the request's active
intel_context for a GT associated with its VM, and indirectly on that
intel_context's engine wakeref if the engine belongs to the same GT as the
VMA's VM. Those wakerefs are released asynchronously to VMA deactivation.
Fix the issue by getting a wakeref for the VMA's GT when activating it,
and putting that wakeref only after the VMA is deactivated. However,
exclude global GTT from that processing path, otherwise the GPU never goes
idle. Since __i915_vma_retire() may be called from atomic contexts, use
async variant of wakeref put. Also, to avoid circular locking dependency,
take care of acquiring the wakeref before VM mutex when both are needed.
v5: Replace "tile" with "GT" across commit description (Rodrigo),
- avoid mentioning multi-GT case in commit description (Rodrigo),
- explain why we need to take a temporary wakeref unconditionally inside
i915_vma_pin_ww() (Rodrigo).
v4: Refresh on top of commit 5e4e06e4087e ("drm/i915: Track gt pm
wakerefs") (Andi),
- for more easy backporting, split out removal of former insufficient
workarounds and move them to separate patches (Nirmoy).
- clean up commit message and description a bit.
v3: Identify root cause more precisely, and a commit to blame,
- identify and drop former workarounds,
- update commit message and description.
v2: Get the wakeref before VM mutex to avoid circular locking dependency,
- drop questionable Fixes: tag.
Fixes: d93939730347 ("drm/i915: Remove the vma refcount")
Closes: https://gitlab.freedesktop.org/drm/intel/issues/8875
Signed-off-by: Janusz Krzysztofik
Cc: Thomas Hellström
Cc: Nirmoy Das
Cc: Andi Shyti
Cc: Rodrigo Vivi
Cc: sta...@vger.kernel.org # v5.19+
---
drivers/gpu/drm/i915/i915_vma.c | 26 +++---
drivers/gpu/drm/i915/i915_vma_types.h | 1 +
2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index d09aad34ba37f..604d420b9e1fd 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -34,6 +34,7 @@
#include "gt/intel_engine.h"
#include "gt/intel_engine_heartbeat.h"
#include "gt/intel_gt.h"
+#include "gt/intel_gt_pm.h"
#include "gt/intel_gt_requests.h"
#include "gt/intel_tlb.h"
@@ -103,12 +104,25 @@ static inline struct i915_vma *active_to_vma(struct
i915_active *ref)
static int
[PATCH v5 0/3] drm/i915: Fix VMA UAF on destroy against deactivate race
Object debugging tools were sporadically reporting illegal attempts to
free a still active i915 VMA object when parking a GT believed to be idle.
[161.359441] ODEBUG: free active (active state 0) object: 88811643b958
object type: i915_active hint: __i915_vma_active+0x0/0x50 [i915]
[161.360082] WARNING: CPU: 5 PID: 276 at lib/debugobjects.c:514
debug_print_object+0x80/0xb0
...
[161.360304] CPU: 5 PID: 276 Comm: kworker/5:2 Not tainted
6.5.0-rc1-CI_DRM_13375-g003f860e5577+ #1
[161.360314] Hardware name: Intel Corporation Rocket Lake Client
Platform/RocketLake S UDIMM 6L RVP, BIOS RKLSFWI1.R00.3173.A03.2204210138
04/21/2022
[161.360322] Workqueue: i915-unordered __intel_wakeref_put_work [i915]
[161.360592] RIP: 0010:debug_print_object+0x80/0xb0
...
[161.361347] debug_object_free+0xeb/0x110
[161.361362] i915_active_fini+0x14/0x130 [i915]
[161.361866] release_references+0xfe/0x1f0 [i915]
[161.362543] i915_vma_parked+0x1db/0x380 [i915]
[161.363129] __gt_park+0x121/0x230 [i915]
[161.363515] intel_wakeref_put_last+0x1f/0x70 [i915]
That has been tracked down to be happening when another thread is
deactivating the VMA inside __active_retire() helper, after the VMA's
active counter has been already decremented to 0, but before deactivation
of the VMA's object is reported to the object debugging tool.
We could prevent from that race by serializing i915_active_fini() with
__active_retire() via ref->tree_lock, but that wouldn't stop the VMA from
being used, e.g. from __i915_vma_retire() called at the end of
__active_retire(), after that VMA has been already freed by a concurrent
i915_vma_destroy() on return from the i915_active_fini(). Then, we should
rather fix the issue at the VMA level, not in i915_active.
Since __i915_vma_parked() is called from __gt_park() on last put of the
GT's wakeref, the issue could be addressed by holding the GT wakeref long
enough for __active_retire() to complete before that wakeref is released
and the GT parked.
A VMA associated with a request doesn't acquire a GT wakeref by itself.
Instead, it depends on a wakeref held directly by the request's active
intel_context for a GT associated with its VM, and indirectly on that
intel_context's engine wakeref if the engine belongs to the same GT as the
VMA's VM. Those wakerefs are released asynchronously to VMA deactivation.
In case of single-GT platforms, at least one of those wakerefs is usually
held long enough for the request's VMA to be deactivated on time, before
it is destroyed on last put of its VM GT wakeref. However, on multi-GT
platforms, a request may use a VMA from a GT other than the one that hosts
the request's engine, then it is protected only with the intel_context's
VM GT wakeref.
There was an attempt to fix the issue on 2-GT Meteor Lake by acquiring an
extra wakeref for a Primary GT from i915_gem_do_execbuffer() -- see commit
f56fe3e91787 ("drm/i915: Fix a VMA UAF for multi-gt platform"). However,
that fix occurred insufficient -- the issue was still reported by CI.
That wakeref was released on exit from i915_gem_do_execbuffer(), then
potentially before completion of the request and deactivation of its
associated VMAs. Moreover, CI reports indicated that single-GT platforms
also suffered sporadically from the same race.
I believe the issue was introduced by commit d93939730347 ("drm/i915:
Remove the vma refcount") which moved a call to i915_active_fini() from
a dropped i915_vma_release(), called on last put of the removed VMA kref,
to i915_vma_parked() processing path called on last put of a GT wakeref.
However, its visibility to the object debugging tool was suppressed by a
bug in i915_active that was fixed two weeks later with commit e92eb246feb9
("drm/i915/active: Fix missing debug object activation").
Fix the issue by getting a wakeref for the VMA's GT when activating it,
and putting that wakeref only after the VMA is deactivated. However,
exclude global GTT from that processing path, otherwise the GPU never goes
idle. Since __i915_vma_retire() may be called from atomic contexts, use
async variant of wakeref put. Also, to avoid circular locking dependency,
take care of acquiring the wakeref before VM mutex when both are needed.
Having that fixed, stop explicitly acquiring the extra GT0 wakeref from
inside i915_gem_do_execbuffer(), and also drop an extra call to
i915_active_wait(), introduced by commit 7a2280e8dcd2 ("drm/i915: Wait for
active retire before i915_active_fini()") as another insufficient fix for
this UAF race.
v5: Replace "tile" with "GT" across commit descriptions (Rodrigo),
- reword commit message and description of patch 2 reusing relevant
chunks moved there from commit description of patch 1 (Rodrigo),
- explain why we take a temporary wakeref unconditionally inside
i915_vma_pin_ww() (Rodrigo).
v4: Refresh on top of commit 5e4e06e4087e ("drm/i915: Track gt pm
wakerefs") (Andi),
- for more easy backporting, split out removal of former insufficient
Re: [PATCH 1/7] drm/i915/hdcp: Move to direct reads for HDCP
On 1/12/2024 1:11 PM, Suraj Kandpal wrote:
Even for MST scenarios we need to do direct reads only on the
immediate downstream device the rest of the authentication is taken
care by that device. Remote reads will only be used to check
capability of the monitors in MST topology.
I think it would be good to add fixes tag.
Signed-off-by: Suraj Kandpal
---
drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
index 3a595cd433d4..bec49061b2e1 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
@@ -331,11 +331,11 @@ static const struct hdcp2_dp_msg_data hdcp2_dp_msg_data[]
= {
};
static struct drm_dp_aux *
-intel_dp_hdcp_get_aux(struct intel_connector *connector)
+intel_dp_hdcp_get_aux(struct intel_connector *connector, bool remote_req)
{
struct intel_digital_port *dig_port =
intel_attached_dig_port(connector);
- if (intel_encoder_is_mst(connector->encoder))
+ if (intel_encoder_is_mst(connector->encoder) && remote_req)
return >port->aux;
else
return _port->dp.aux;
For most cases we are not using remote read/writes, so it makes sense to
directly, use dig_port->dp.aux in those places.
Only case where we are interested to remote read is when its its an MST
and we are interested to know the remote sink capability, in
intel_dp_hdcp2_capable.
@@ -346,7 +346,7 @@ intel_dp_hdcp2_read_rx_status(struct intel_connector
*connector,
u8 *rx_status)
{
struct drm_i915_private *i915 = to_i915(connector->base.dev);
- struct drm_dp_aux *aux = intel_dp_hdcp_get_aux(connector);
+ struct drm_dp_aux *aux = intel_dp_hdcp_get_aux(connector, false);
ssize_t ret;
ret = drm_dp_dpcd_read(aux,
@@ -463,7 +463,7 @@ int intel_dp_hdcp2_write_msg(struct intel_connector
*connector,
offset = hdcp2_msg_data->offset;
- aux = intel_dp_hdcp_get_aux(connector);
+ aux = intel_dp_hdcp_get_aux(connector, false);
/* No msg_id in DP HDCP2.2 msgs */
bytes_to_write = size - 1;
@@ -490,7 +490,7 @@ static
ssize_t get_receiver_id_list_rx_info(struct intel_connector *connector,
u32 *dev_cnt, u8 *byte)
{
- struct drm_dp_aux *aux = intel_dp_hdcp_get_aux(connector);
+ struct drm_dp_aux *aux = intel_dp_hdcp_get_aux(connector, false);
ssize_t ret;
u8 *rx_info = byte;
@@ -530,7 +530,7 @@ int intel_dp_hdcp2_read_msg(struct intel_connector *connector,
return -EINVAL;
offset = hdcp2_msg_data->offset;
- aux = intel_dp_hdcp_get_aux(connector);
+ aux = intel_dp_hdcp_get_aux(connector, false);
ret = intel_dp_hdcp2_wait_for_msg(connector, hdcp2_msg_data);
if (ret < 0)
@@ -655,7 +655,7 @@ int intel_dp_hdcp2_capable(struct intel_connector
*connector,
u8 rx_caps[3];
int ret;
- aux = intel_dp_hdcp_get_aux(connector);
+ aux = intel_dp_hdcp_get_aux(connector, true);
As mentioned above lets call this out clearly:
aux = intel_encoder_is_mst(connector->encoder) ? >port->aux :
_port->dp.aux;
Regards,
Ankit
*capable = false;
ret = drm_dp_dpcd_read(aux,
Re: [PATCH] mm: Remove double faults once write a device pfn
"Zhou, Xianrong" writes: > [AMD Official Use Only - General] > >> > The vmf_insert_pfn_prot could cause unnecessary double faults on a >> > device pfn. Because currently the vmf_insert_pfn_prot does not >> > make the pfn writable so the pte entry is normally read-only or >> > dirty catching. >> What? How do you got to this conclusion? >> >>> Sorry. I did not mention that this problem only exists on arm64 platform. >> >> Ok, that makes at least a little bit more sense. >> >> >> >>> Because on arm64 platform the PTE_RDONLY is automatically attached >> >>> to the userspace pte entries even through VM_WRITE + VM_SHARE. >> >>> The PTE_RDONLY needs to be cleared in vmf_insert_pfn_prot. However >> >>> vmf_insert_pfn_prot do not make the pte writable passing false >> >>> @mkwrite to insert_pfn. >> >> Question is why is arm64 doing this? As far as I can see they must >> >> have some hardware reason for that. >> >> >> >> The mkwrite parameter to insert_pfn() was added by commit >> >> b2770da642540 to make insert_pfn() look more like insert_pfn_pmd() so >> >> that the DAX code can insert PTEs which are writable and dirty at the same >> time. >> >> >> > This is one scenario to do so. In fact on arm64 there are many >> > scenarios could be to do so. So we can let vmf_insert_pfn_prot >> > supporting @mkwrite for drivers at core layer and let drivers to >> > decide whether or not to make writable and dirty at one time. The >> > patch did this. Otherwise double faults on arm64 when call >> vmf_insert_pfn_prot. >> >> Well, that doesn't answer my question why arm64 is double faulting in the >> first place,. >> > > > Eh. > > On arm64 When userspace mmap() with PROT_WRITE and MAP_SHARED the > vma->vm_page_prot has the PTE_RDONLY and PTE_WRITE within > PAGE_SHARED_EXEC. (seeing arm64 protection_map) > > When write the userspace virtual address the first fault happen and call > into driver's > .fault->ttm_bo_vm_fault_reserved->vmf_insert_pfn_prot->insert_pfn. > The insert_pfn will establish the pte entry. However the vmf_insert_pfn_prot > pass false @mkwrite to insert_pfn by default and so insert_pfn could not make > the pfn writable and it do not call maybe_mkwrite(pte_mkdirty(entry), vma) > to clear the PTE_RDONLY bit. So the pte entry is actually write protection > for mmu. > So when the first fault return and re-execute the store instruction the second > fault happen again. And in second fault it just only do pte_mkdirty(entry) > which > clear the PTE_RDONLY. It depends if the ARM64 CPU in question supports hardware dirty bit management (DBM). If that is the case and PTE_DBM (ie. PTE_WRITE) is set HW will automatically clear PTE_RDONLY bit to mark the entry dirty instead of raising a write fault. So you shouldn't see a double fault if PTE_DBM/WRITE is set. On ARM64 you can kind of think of PTE_RDONLY as the HW dirty bit and PTE_DBM as the read/write permission bit with SW being responsible for updating PTE_RDONLY via the fault handler if DBM is not supported by HW. At least that's my understanding from having hacked on this in the past. You can see all this weirdness happening in the definitions of pte_dirty() and pte_write() for ARM64. > I think so and hope no wrong. > >> So as long as this isn't sorted out I'm going to reject this patch. >> >> Regards, >> Christian. >> >> > >> >> This is a completely different use case to what you try to use it >> >> here for and that looks extremely fishy to me. >> >> >> >> Regards, >> >> Christian. >> >> >> > The first fault only sets up the pte entry which actually is dirty >> > catching. And the second immediate fault to the pfn due to first >> > dirty catching when the cpu re-execute the store instruction. >> It could be that this is done to work around some hw behavior, but >> not because of dirty catching. >> >> > Normally if the drivers call vmf_insert_pfn_prot and also supply >> > 'pfn_mkwrite' callback within vm_operations_struct which requires >> > the pte to be dirty catching then the vmf_insert_pfn_prot and the >> > double fault are reasonable. It is not a problem. >> Well, as far as I can see that behavior absolutely doesn't make sense. >> >> When pfn_mkwrite is requested then the driver should use PAGE_COPY, >> which is exactly what VMWGFX (the only driver using dirty tracking) >> is >> >> doing. >> Everybody else uses PAGE_SHARED which should make the pte writeable >> immediately. >> >> Regards, >> Christian. >> >> > However the most of drivers calling vmf_insert_pfn_prot do not >> > supply the 'pfn_mkwrite' callback so that the second fault is >> unnecessary. >> > >> > So just like vmf_insert_mixed and vmf_insert_mixed_mkwrite pair, >> > we should also supply vmf_insert_pfn_mkwrite for drivers as well. >> > >> > Signed-off-by: Xianrong Zhou >> > --- >> > arch/x86/entry/vdso/vma.c | 3
✗ Fi.CI.BAT: failure for drm/i915: limit eDP MSO pipe only for display version 20 and below
== Series Details == Series: drm/i915: limit eDP MSO pipe only for display version 20 and below URL : https://patchwork.freedesktop.org/series/129123/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14168 -> Patchwork_129123v1 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_129123v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_129123v1, 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_129123v1/index.html Participating hosts (38 -> 38) -- Additional (1): bat-kbl-2 Missing(1): fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_129123v1: ### IGT changes ### Possible regressions * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size: - fi-tgl-1115g4: [PASS][1] -> [SKIP][2] +5 other tests skip [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-flip-after-cursor-varying-size.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129123v1/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-flip-after-cursor-varying-size.html * igt@kms_pipe_crc_basic@hang-read-crc: - fi-tgl-1115g4: NOTRUN -> [SKIP][3] +6 other tests skip [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129123v1/fi-tgl-1115g4/igt@kms_pipe_crc_ba...@hang-read-crc.html Warnings * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - fi-tgl-1115g4: [SKIP][4] ([i915#4103]) -> [SKIP][5] +1 other test skip [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129123v1/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html * igt@kms_dsc@dsc-basic: - fi-tgl-1115g4: [SKIP][6] ([i915#3555] / [i915#3840]) -> [SKIP][7] [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@kms_...@dsc-basic.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129123v1/fi-tgl-1115g4/igt@kms_...@dsc-basic.html Known issues Here are the changes found in Patchwork_129123v1 that come from known issues: ### IGT changes ### Issues hit * igt@fbdev@info: - bat-kbl-2: NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#1849]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129123v1/bat-kbl-2/igt@fb...@info.html - fi-tgl-1115g4: [PASS][9] -> [SKIP][10] ([i915#1849] / [i915#2582]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@fb...@info.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129123v1/fi-tgl-1115g4/igt@fb...@info.html * igt@fbdev@nullptr: - fi-tgl-1115g4: [PASS][11] -> [SKIP][12] ([i915#2582]) +3 other tests skip [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@fb...@nullptr.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129123v1/fi-tgl-1115g4/igt@fb...@nullptr.html * igt@gem_lmem_swapping@basic: - fi-apl-guc: NOTRUN -> [SKIP][13] ([fdo#109271] / [i915#4613]) +3 other tests skip [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129123v1/fi-apl-guc/igt@gem_lmem_swapp...@basic.html * igt@gem_lmem_swapping@parallel-random-engines: - bat-kbl-2: NOTRUN -> [SKIP][14] ([fdo#109271]) +35 other tests skip [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129123v1/bat-kbl-2/igt@gem_lmem_swapp...@parallel-random-engines.html * igt@gem_lmem_swapping@verify-random: - bat-mtlp-8: NOTRUN -> [SKIP][15] ([i915#4613]) +3 other tests skip [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129123v1/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html * igt@gem_tiled_blits@basic: - fi-pnv-d510:[PASS][16] -> [SKIP][17] ([fdo#109271]) +1 other test skip [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-pnv-d510/igt@gem_tiled_bl...@basic.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129123v1/fi-pnv-d510/igt@gem_tiled_bl...@basic.html * igt@i915_pm_rps@basic-api: - bat-mtlp-8: NOTRUN -> [SKIP][18] ([i915#6621]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129123v1/bat-mtlp-8/igt@i915_pm_...@basic-api.html * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy: - bat-mtlp-8: NOTRUN -> [SKIP][19] ([i915#5190]) [19]:
Re: [PATCH v2 2/4] drm/i915/psr: Add alpm_parameters struct
On Wed, 2024-01-24 at 09:28 +, Murthy, Arun R wrote:
>
>
> > -Original Message-
> > From: Hogander, Jouni
> > Sent: Wednesday, January 24, 2024 2:55 PM
> > To: Murthy, Arun R ;
> > intel-gfx@lists.freedesktop.org
> > Subject: Re: [PATCH v2 2/4] drm/i915/psr: Add alpm_parameters
> > struct
> >
> > On Wed, 2024-01-24 at 09:17 +, Murthy, Arun R wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Intel-gfx On
> > > > Behalf
> > > > Of Jouni Högander
> > > > Sent: Friday, January 5, 2024 7:45 PM
> > > > To: intel-gfx@lists.freedesktop.org
> > > > Subject: [PATCH v2 2/4] drm/i915/psr: Add alpm_parameters
> > > > struct
> > > >
> > > > Add new alpm_parameters struct into intel_psr for all
> > > > calculated
> > > > alpm parameters.
> > > >
> > > > Signed-off-by: Jouni Högander
> > > > ---
> > > > .../drm/i915/display/intel_display_types.h | 8 --
> > > > drivers/gpu/drm/i915/display/intel_psr.c | 28 ++-
> > > >
> > > >
> > > > 2 files changed, 21 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > index b9b9d9f2bc0b..889a8b34b7ac 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > @@ -1677,6 +1677,11 @@ struct intel_pps {
> > > > struct edp_power_seq bios_pps_delays;
> > > > };
> > > >
> > > > +struct alpm_parameters {
> > > > + u8 io_wake_lines;
> > > > + u8 fast_wake_lines;
> > > > +};
> > > Assume that this is being used in PSR as well and can be retained
> > > in
> > > intel_psr struct.
> > > If exclusively used for alpm then having it in a new alpm struct
> > > would
> > > be better.
> >
> > Amount of these parameters are about to grow with AUXless ALPM. I
> > was
> > thinking having own struct would be more clear. Please note that
> > alpm_params
> > is still under struct intel_psr. What do you think?
> >
> Understand that his is quite a big feature and this struct tends to
> grow in the coming patches. But w.r.t this variable since it was used
> in psr and not exclusively for alpm felt so. I would leave it for
> others in the community to add their views on this.
I don't have that strong opinions on this. I can change it back.
BR,
Jouni Högander
>
> Thanks and Regards,
> Arun R Murthy
> ---
> > BR,
> >
> > Jouni Högander
> > >
> > > Thanks and Regards,
> > > Arun R Murthy
> > > ---
> > > > +
> > > > struct intel_psr {
> > > > /* Mutex for PSR state of the transcoder */
> > > > struct mutex lock;
> > > > @@ -1706,8 +1711,6 @@ struct intel_psr {
> > > > bool psr2_sel_fetch_cff_enabled;
> > > > bool req_psr2_sdp_prior_scanline;
> > > > u8 sink_sync_latency;
> > > > - u8 io_wake_lines;
> > > > - u8 fast_wake_lines;
> > > > ktime_t last_entry_attempt;
> > > > ktime_t last_exit;
> > > > bool sink_not_reliable;
> > > > @@ -1721,6 +1724,7 @@ struct intel_psr {
> > > > u32 dc3co_exit_delay;
> > > > struct delayed_work dc3co_work;
> > > > u8 entry_setup_frames;
> > > > + struct alpm_parameters alpm_params;
> > > > };
> > > >
> > > > struct intel_dp {
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 54120b45958b..1709ebb31215 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -759,8 +759,8 @@ static u32 intel_psr2_get_tp_time(struct
> > > > intel_dp
> > > > *intel_dp)
> > > >
> > > > static int psr2_block_count_lines(struct intel_dp *intel_dp)
> > > > {
> > > > - return intel_dp->psr.io_wake_lines < 9 &&
> > > > - intel_dp->psr.fast_wake_lines < 9 ? 8 : 12;
> > > > + return intel_dp->psr.alpm_params.io_wake_lines < 9 &&
> > > > + intel_dp->psr.alpm_params.fast_wake_lines < 9 ?
> > > > 8 :
> > > > 12;
> > > > }
> > > >
> > > > static int psr2_block_count(struct intel_dp *intel_dp) @@ -
> > > > 797,6
> > > > +797,7 @@
> > > > static void dg2_activate_panel_replay(struct intel_dp
> > > > *intel_dp)
> > > > static void hsw_activate_psr2(struct intel_dp *intel_dp) {
> > > > struct drm_i915_private *dev_priv =
> > > > dp_to_i915(intel_dp);
> > > > + struct alpm_parameters *alpm_params = _dp-
> > > > > psr.alpm_params;
> > > > enum transcoder cpu_transcoder = intel_dp-
> > > > >psr.transcoder;
> > > > u32 val = EDP_PSR2_ENABLE;
> > > > u32 psr_val = 0;
> > > > @@ -838,17 +839,17 @@ static void hsw_activate_psr2(struct
> > > > intel_dp
> > > > *intel_dp)
> > > > */
> > > > int tmp;
> > > >
> > > > - tmp = map[intel_dp->psr.io_wake_lines -
> > > >
RE: [PATCH v2 2/4] drm/i915/psr: Add alpm_parameters struct
> -Original Message-
> From: Hogander, Jouni
> Sent: Wednesday, January 24, 2024 2:55 PM
> To: Murthy, Arun R ; intel-gfx@lists.freedesktop.org
> Subject: Re: [PATCH v2 2/4] drm/i915/psr: Add alpm_parameters struct
>
> On Wed, 2024-01-24 at 09:17 +, Murthy, Arun R wrote:
> >
> >
> > > -Original Message-
> > > From: Intel-gfx On Behalf
> > > Of Jouni Högander
> > > Sent: Friday, January 5, 2024 7:45 PM
> > > To: intel-gfx@lists.freedesktop.org
> > > Subject: [PATCH v2 2/4] drm/i915/psr: Add alpm_parameters struct
> > >
> > > Add new alpm_parameters struct into intel_psr for all calculated
> > > alpm parameters.
> > >
> > > Signed-off-by: Jouni Högander
> > > ---
> > > .../drm/i915/display/intel_display_types.h | 8 --
> > > drivers/gpu/drm/i915/display/intel_psr.c | 28 ++-
> > >
> > > 2 files changed, 21 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index b9b9d9f2bc0b..889a8b34b7ac 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -1677,6 +1677,11 @@ struct intel_pps {
> > > struct edp_power_seq bios_pps_delays;
> > > };
> > >
> > > +struct alpm_parameters {
> > > + u8 io_wake_lines;
> > > + u8 fast_wake_lines;
> > > +};
> > Assume that this is being used in PSR as well and can be retained in
> > intel_psr struct.
> > If exclusively used for alpm then having it in a new alpm struct would
> > be better.
>
> Amount of these parameters are about to grow with AUXless ALPM. I was
> thinking having own struct would be more clear. Please note that alpm_params
> is still under struct intel_psr. What do you think?
>
Understand that his is quite a big feature and this struct tends to grow in the
coming patches. But w.r.t this variable since it was used in psr and not
exclusively for alpm felt so. I would leave it for others in the community to
add their views on this.
Thanks and Regards,
Arun R Murthy
---
> BR,
>
> Jouni Högander
> >
> > Thanks and Regards,
> > Arun R Murthy
> > ---
> > > +
> > > struct intel_psr {
> > > /* Mutex for PSR state of the transcoder */
> > > struct mutex lock;
> > > @@ -1706,8 +1711,6 @@ struct intel_psr {
> > > bool psr2_sel_fetch_cff_enabled;
> > > bool req_psr2_sdp_prior_scanline;
> > > u8 sink_sync_latency;
> > > - u8 io_wake_lines;
> > > - u8 fast_wake_lines;
> > > ktime_t last_entry_attempt;
> > > ktime_t last_exit;
> > > bool sink_not_reliable;
> > > @@ -1721,6 +1724,7 @@ struct intel_psr {
> > > u32 dc3co_exit_delay;
> > > struct delayed_work dc3co_work;
> > > u8 entry_setup_frames;
> > > + struct alpm_parameters alpm_params;
> > > };
> > >
> > > struct intel_dp {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 54120b45958b..1709ebb31215 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -759,8 +759,8 @@ static u32 intel_psr2_get_tp_time(struct
> > > intel_dp
> > > *intel_dp)
> > >
> > > static int psr2_block_count_lines(struct intel_dp *intel_dp) {
> > > - return intel_dp->psr.io_wake_lines < 9 &&
> > > - intel_dp->psr.fast_wake_lines < 9 ? 8 : 12;
> > > + return intel_dp->psr.alpm_params.io_wake_lines < 9 &&
> > > + intel_dp->psr.alpm_params.fast_wake_lines < 9 ? 8 :
> > > 12;
> > > }
> > >
> > > static int psr2_block_count(struct intel_dp *intel_dp) @@ -797,6
> > > +797,7 @@
> > > static void dg2_activate_panel_replay(struct intel_dp *intel_dp)
> > > static void hsw_activate_psr2(struct intel_dp *intel_dp) {
> > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > + struct alpm_parameters *alpm_params = _dp-
> > > >psr.alpm_params;
> > > enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
> > > u32 val = EDP_PSR2_ENABLE;
> > > u32 psr_val = 0;
> > > @@ -838,17 +839,17 @@ static void hsw_activate_psr2(struct intel_dp
> > > *intel_dp)
> > > */
> > > int tmp;
> > >
> > > - tmp = map[intel_dp->psr.io_wake_lines -
> > > TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
> > > + tmp = map[alpm_params->io_wake_lines -
> > > +TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
> > > val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(tmp +
> > > TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES);
> > >
> > > - tmp = map[intel_dp->psr.fast_wake_lines -
> > > TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
> > > + tmp = map[alpm_params->fast_wake_lines -
> > > +TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
> > > val |= TGL_EDP_PSR2_FAST_WAKE(tmp +
> > >
Re: [PATCH v2 2/4] drm/i915/psr: Add alpm_parameters struct
On Wed, 2024-01-24 at 09:17 +, Murthy, Arun R wrote:
>
>
> > -Original Message-
> > From: Intel-gfx On Behalf
> > Of Jouni
> > Högander
> > Sent: Friday, January 5, 2024 7:45 PM
> > To: intel-gfx@lists.freedesktop.org
> > Subject: [PATCH v2 2/4] drm/i915/psr: Add alpm_parameters struct
> >
> > Add new alpm_parameters struct into intel_psr for all calculated
> > alpm
> > parameters.
> >
> > Signed-off-by: Jouni Högander
> > ---
> > .../drm/i915/display/intel_display_types.h | 8 --
> > drivers/gpu/drm/i915/display/intel_psr.c | 28 ++-
> >
> > 2 files changed, 21 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index b9b9d9f2bc0b..889a8b34b7ac 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1677,6 +1677,11 @@ struct intel_pps {
> > struct edp_power_seq bios_pps_delays;
> > };
> >
> > +struct alpm_parameters {
> > + u8 io_wake_lines;
> > + u8 fast_wake_lines;
> > +};
> Assume that this is being used in PSR as well and can be retained in
> intel_psr struct.
> If exclusively used for alpm then having it in a new alpm struct
> would be better.
Amount of these parameters are about to grow with AUXless ALPM. I was
thinking having own struct would be more clear. Please note that
alpm_params is still under struct intel_psr. What do you think?
BR,
Jouni Högander
>
> Thanks and Regards,
> Arun R Murthy
> ---
> > +
> > struct intel_psr {
> > /* Mutex for PSR state of the transcoder */
> > struct mutex lock;
> > @@ -1706,8 +1711,6 @@ struct intel_psr {
> > bool psr2_sel_fetch_cff_enabled;
> > bool req_psr2_sdp_prior_scanline;
> > u8 sink_sync_latency;
> > - u8 io_wake_lines;
> > - u8 fast_wake_lines;
> > ktime_t last_entry_attempt;
> > ktime_t last_exit;
> > bool sink_not_reliable;
> > @@ -1721,6 +1724,7 @@ struct intel_psr {
> > u32 dc3co_exit_delay;
> > struct delayed_work dc3co_work;
> > u8 entry_setup_frames;
> > + struct alpm_parameters alpm_params;
> > };
> >
> > struct intel_dp {
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 54120b45958b..1709ebb31215 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -759,8 +759,8 @@ static u32 intel_psr2_get_tp_time(struct
> > intel_dp
> > *intel_dp)
> >
> > static int psr2_block_count_lines(struct intel_dp *intel_dp) {
> > - return intel_dp->psr.io_wake_lines < 9 &&
> > - intel_dp->psr.fast_wake_lines < 9 ? 8 : 12;
> > + return intel_dp->psr.alpm_params.io_wake_lines < 9 &&
> > + intel_dp->psr.alpm_params.fast_wake_lines < 9 ? 8 :
> > 12;
> > }
> >
> > static int psr2_block_count(struct intel_dp *intel_dp) @@ -797,6
> > +797,7 @@
> > static void dg2_activate_panel_replay(struct intel_dp *intel_dp)
> > static void
> > hsw_activate_psr2(struct intel_dp *intel_dp) {
> > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > + struct alpm_parameters *alpm_params = _dp-
> > >psr.alpm_params;
> > enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
> > u32 val = EDP_PSR2_ENABLE;
> > u32 psr_val = 0;
> > @@ -838,17 +839,17 @@ static void hsw_activate_psr2(struct intel_dp
> > *intel_dp)
> > */
> > int tmp;
> >
> > - tmp = map[intel_dp->psr.io_wake_lines -
> > TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
> > + tmp = map[alpm_params->io_wake_lines -
> > +TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
> > val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(tmp +
> > TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES);
> >
> > - tmp = map[intel_dp->psr.fast_wake_lines -
> > TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
> > + tmp = map[alpm_params->fast_wake_lines -
> > +TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
> > val |= TGL_EDP_PSR2_FAST_WAKE(tmp +
> > TGL_EDP_PSR2_FAST_WAKE_MIN_LINES);
> > } else if (DISPLAY_VER(dev_priv) >= 12) {
> > - val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(intel_dp-
> > > psr.io_wake_lines);
> > - val |= TGL_EDP_PSR2_FAST_WAKE(intel_dp-
> > > psr.fast_wake_lines);
> > + val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(alpm_params-
> > > io_wake_lines);
> > + val |= TGL_EDP_PSR2_FAST_WAKE(alpm_params-
> > > fast_wake_lines);
> > } else if (DISPLAY_VER(dev_priv) >= 9) {
> > - val |= EDP_PSR2_IO_BUFFER_WAKE(intel_dp-
> > > psr.io_wake_lines);
> > - val |= EDP_PSR2_FAST_WAKE(intel_dp-
> > >psr.fast_wake_lines);
> > + val |= EDP_PSR2_IO_BUFFER_WAKE(alpm_params-
> > > io_wake_lines);
> >
Re: linux-next: Tree for Jan 23 (drm/xe/)
On Tue, 23 Jan 2024, Randy Dunlap wrote: > On 1/22/24 18:29, Stephen Rothwell wrote: >> Hi all, >> >> News: there will be no linux-next release on Friday >> >> Changes since 20240122: >> > > on ARM64, when > DRM_I915 is not set > DRM_XE=m > DEBUG_FS is not set > > ../drivers/gpu/drm/i915/display/intel_display_debugfs.c:1091:6: error: > redefinition of 'intel_display_debugfs_register' > 1091 | void intel_display_debugfs_register(struct drm_i915_private *i915) > | ^~ > In file included from > ../drivers/gpu/drm/i915/display/intel_display_debugfs.c:19: Does [1] fix the issue? BR, Jani. [1] https://lore.kernel.org/r/20240124090515.3363901-1-jani.nik...@intel.com -- Jani Nikula, Intel
RE: [PATCH v2 2/4] drm/i915/psr: Add alpm_parameters struct
> -Original Message-
> From: Intel-gfx On Behalf Of Jouni
> Högander
> Sent: Friday, January 5, 2024 7:45 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [PATCH v2 2/4] drm/i915/psr: Add alpm_parameters struct
>
> Add new alpm_parameters struct into intel_psr for all calculated alpm
> parameters.
>
> Signed-off-by: Jouni Högander
> ---
> .../drm/i915/display/intel_display_types.h| 8 --
> drivers/gpu/drm/i915/display/intel_psr.c | 28 ++-
> 2 files changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index b9b9d9f2bc0b..889a8b34b7ac 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1677,6 +1677,11 @@ struct intel_pps {
> struct edp_power_seq bios_pps_delays;
> };
>
> +struct alpm_parameters {
> + u8 io_wake_lines;
> + u8 fast_wake_lines;
> +};
Assume that this is being used in PSR as well and can be retained in intel_psr
struct.
If exclusively used for alpm then having it in a new alpm struct would be
better.
Thanks and Regards,
Arun R Murthy
---
> +
> struct intel_psr {
> /* Mutex for PSR state of the transcoder */
> struct mutex lock;
> @@ -1706,8 +1711,6 @@ struct intel_psr {
> bool psr2_sel_fetch_cff_enabled;
> bool req_psr2_sdp_prior_scanline;
> u8 sink_sync_latency;
> - u8 io_wake_lines;
> - u8 fast_wake_lines;
> ktime_t last_entry_attempt;
> ktime_t last_exit;
> bool sink_not_reliable;
> @@ -1721,6 +1724,7 @@ struct intel_psr {
> u32 dc3co_exit_delay;
> struct delayed_work dc3co_work;
> u8 entry_setup_frames;
> + struct alpm_parameters alpm_params;
> };
>
> struct intel_dp {
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 54120b45958b..1709ebb31215 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -759,8 +759,8 @@ static u32 intel_psr2_get_tp_time(struct intel_dp
> *intel_dp)
>
> static int psr2_block_count_lines(struct intel_dp *intel_dp) {
> - return intel_dp->psr.io_wake_lines < 9 &&
> - intel_dp->psr.fast_wake_lines < 9 ? 8 : 12;
> + return intel_dp->psr.alpm_params.io_wake_lines < 9 &&
> + intel_dp->psr.alpm_params.fast_wake_lines < 9 ? 8 : 12;
> }
>
> static int psr2_block_count(struct intel_dp *intel_dp) @@ -797,6 +797,7 @@
> static void dg2_activate_panel_replay(struct intel_dp *intel_dp) static void
> hsw_activate_psr2(struct intel_dp *intel_dp) {
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> + struct alpm_parameters *alpm_params = _dp->psr.alpm_params;
> enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
> u32 val = EDP_PSR2_ENABLE;
> u32 psr_val = 0;
> @@ -838,17 +839,17 @@ static void hsw_activate_psr2(struct intel_dp
> *intel_dp)
>*/
> int tmp;
>
> - tmp = map[intel_dp->psr.io_wake_lines -
> TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
> + tmp = map[alpm_params->io_wake_lines -
> +TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
> val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(tmp +
> TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES);
>
> - tmp = map[intel_dp->psr.fast_wake_lines -
> TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
> + tmp = map[alpm_params->fast_wake_lines -
> +TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
> val |= TGL_EDP_PSR2_FAST_WAKE(tmp +
> TGL_EDP_PSR2_FAST_WAKE_MIN_LINES);
> } else if (DISPLAY_VER(dev_priv) >= 12) {
> - val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(intel_dp-
> >psr.io_wake_lines);
> - val |= TGL_EDP_PSR2_FAST_WAKE(intel_dp-
> >psr.fast_wake_lines);
> + val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(alpm_params-
> >io_wake_lines);
> + val |= TGL_EDP_PSR2_FAST_WAKE(alpm_params-
> >fast_wake_lines);
> } else if (DISPLAY_VER(dev_priv) >= 9) {
> - val |= EDP_PSR2_IO_BUFFER_WAKE(intel_dp-
> >psr.io_wake_lines);
> - val |= EDP_PSR2_FAST_WAKE(intel_dp->psr.fast_wake_lines);
> + val |= EDP_PSR2_IO_BUFFER_WAKE(alpm_params-
> >io_wake_lines);
> + val |= EDP_PSR2_FAST_WAKE(alpm_params->fast_wake_lines);
> }
>
> if (intel_dp->psr.req_psr2_sdp_prior_scanline)
> @@ -1098,10 +1099,11 @@ static bool
> _compute_psr2_sdp_prior_scanline_indication(struct intel_dp *intel_d
> return true;
> }
>
> -static bool _compute_psr2_wake_times(struct intel_dp *intel_dp,
> - struct intel_crtc_state *crtc_state)
> +static bool _compute_alpm_params(struct intel_dp *intel_dp,
> + struct intel_crtc_state *crtc_state)
> {
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + struct
Re: [PATCH] drm/i915/psr: Request modeset on initial commit to compute PSR state
On Tue, 23 Jan 2024, Ville Syrjälä wrote:
> On Tue, Jan 23, 2024 at 07:57:00AM +, Hogander, Jouni wrote:
>> On Tue, 2024-01-23 at 09:41 +0200, Ville Syrjälä wrote:
>> > On Tue, Jan 23, 2024 at 09:11:03AM +0200, Jouni Högander wrote:
>> > > We want to request full modeset in initial fast check to force PSR
>> > > state
>> > > computation. Otherwise PSR is not enabled on initial commit but on
>> > > first
>> > > commit with modeset or fastset. With this change Initial commit
>> > > will still
>> > > end up using fastset (unless something else requires full modeset)
>> > > as PSR
>> > > parameters are not anymore part of intel_pipe_config_compare.
>> >
>> > I think I'd prefer to go the oppostie direction and try to get all
>> > the full modeset stuff out from the initial commit. The only reason
>> > the initial commit was introduced was to compute the plane states
>> > due to lack of readout, and then it got extended due to various other
>> > hacks. Our goal is to inherit the state from the BIOS so ideally
>> > the whole initial_commit thing wouldn't even exist.
>>
>> Bios doesn't enable PSR. Do you think this would be better approach ?:
>>
>> https://patchwork.freedesktop.org/patch/575368/?series=129023=1
>>
>> What we just need is something triggering intel_psr_compute_config +
>> psr enable. Maybe that could be separate function doing both and call
>> that from intel_initial_commit. If/when we get rid of that
>> intel_initial_commit: this function would be called by that new thing.
>
> I don't think we should do anything at all. PSR will get enabled by the
> first proper commit, if possible.
In general, I'm leaning the same way. Priorities:
1) Avoid full modeset at probe if at all possible.
2) Taking the above into account, enable as many power saving
etc. features as possible at probe.
3) If a system needs more power savings (or other fancy features
enabled), a) have them enabled by BIOS/GOP, or b) have the userspace
do a modeset post-probe according to its policy. Don't force that
policy on the kernel.
4) *Maybe* provide a way to force full modeset at probe as a policy
option.
BR,
Jani.
>
>>
>> BR,
>>
>> Jouni Högander
>>
>> >
>> > >
>> > > Cc: Stanislav Lisovskiy
>> > >
>> > > Fixes: a480dd59fe25 ("drm/i915/display: No need for full modeset
>> > > due to psr")
>> > > Signed-off-by: Jouni Högander
>> > > ---
>> > > drivers/gpu/drm/i915/display/intel_dp.c | 8
>> > > drivers/gpu/drm/i915/display/intel_psr.c | 3 ---
>> > > drivers/gpu/drm/i915/display/intel_psr.h | 3 +++
>> > > 3 files changed, 11 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> > > b/drivers/gpu/drm/i915/display/intel_dp.c
>> > > index ab415f41924d..143981b91e8b 100644
>> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> > > @@ -3326,6 +3326,14 @@ 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;
>> > > }
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
>> > > b/drivers/gpu/drm/i915/display/intel_psr.c
>> > > index 1010b8c405df..b6db7dbfaf1a 100644
>> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
>> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>> > > @@ -173,9 +173,6 @@
>> > > * 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)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
>> > > b/drivers/gpu/drm/i915/display/intel_psr.h
>> > > index cde781df84d5..3d9920ebafab 100644
>> > > --- a/drivers/gpu/drm/i915/display/intel_psr.h
>> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
>> > > @@ -21,6 +21,9 @@ 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)
>> > > +
>> > > 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
>> >
>>
--
Jani Nikula, Intel
✗ Fi.CI.BAT: failure for drm/i915/fbc: Allow FBC with CCS modifiers on SKL+ (rev3)
== Series Details == Series: drm/i915/fbc: Allow FBC with CCS modifiers on SKL+ (rev3) URL : https://patchwork.freedesktop.org/series/129075/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14168 -> Patchwork_129075v3 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_129075v3 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_129075v3, 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_129075v3/index.html Participating hosts (38 -> 38) -- Additional (1): bat-kbl-2 Missing(1): fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_129075v3: ### IGT changes ### Possible regressions * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size: - fi-tgl-1115g4: [PASS][1] -> [SKIP][2] +5 other tests skip [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-flip-after-cursor-varying-size.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129075v3/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-flip-after-cursor-varying-size.html * igt@kms_pipe_crc_basic@hang-read-crc: - fi-tgl-1115g4: NOTRUN -> [SKIP][3] +6 other tests skip [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129075v3/fi-tgl-1115g4/igt@kms_pipe_crc_ba...@hang-read-crc.html Warnings * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - fi-tgl-1115g4: [SKIP][4] ([i915#4103]) -> [SKIP][5] +1 other test skip [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129075v3/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html * igt@kms_dsc@dsc-basic: - fi-tgl-1115g4: [SKIP][6] ([i915#3555] / [i915#3840]) -> [SKIP][7] [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@kms_...@dsc-basic.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129075v3/fi-tgl-1115g4/igt@kms_...@dsc-basic.html Known issues Here are the changes found in Patchwork_129075v3 that come from known issues: ### IGT changes ### Issues hit * igt@fbdev@info: - bat-kbl-2: NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#1849]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129075v3/bat-kbl-2/igt@fb...@info.html - fi-tgl-1115g4: [PASS][9] -> [SKIP][10] ([i915#1849] / [i915#2582]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@fb...@info.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129075v3/fi-tgl-1115g4/igt@fb...@info.html * igt@fbdev@nullptr: - fi-tgl-1115g4: [PASS][11] -> [SKIP][12] ([i915#2582]) +3 other tests skip [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@fb...@nullptr.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129075v3/fi-tgl-1115g4/igt@fb...@nullptr.html * igt@gem_lmem_swapping@basic: - fi-apl-guc: NOTRUN -> [SKIP][13] ([fdo#109271] / [i915#4613]) +3 other tests skip [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129075v3/fi-apl-guc/igt@gem_lmem_swapp...@basic.html * igt@gem_lmem_swapping@parallel-random-engines: - bat-kbl-2: NOTRUN -> [SKIP][14] ([fdo#109271]) +35 other tests skip [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129075v3/bat-kbl-2/igt@gem_lmem_swapp...@parallel-random-engines.html * igt@gem_lmem_swapping@verify-random: - bat-mtlp-8: NOTRUN -> [SKIP][15] ([i915#4613]) +3 other tests skip [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129075v3/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html * igt@gem_tiled_blits@basic: - fi-pnv-d510:[PASS][16] -> [SKIP][17] ([fdo#109271]) +1 other test skip [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-pnv-d510/igt@gem_tiled_bl...@basic.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129075v3/fi-pnv-d510/igt@gem_tiled_bl...@basic.html * igt@i915_hangman@error-state-basic: - bat-mtlp-6: [PASS][18] -> [ABORT][19] ([i915#9414]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/bat-mtlp-6/igt@i915_hang...@error-state-basic.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129075v3/bat-mtlp-6/igt@i915_hang...@error-state-basic.html * igt@i915_pm_rps@basic-api: - bat-mtlp-8: NOTRUN -> [SKIP][20] ([i915#6621])
Re: [RFC PATCH] drm/i915: Add GETPARAM for GuC submission version
On 24/01/2024 08:19, Joonas Lahtinen wrote:
Add reporting of the GuC submissio/VF interface version via GETPARAM
properties. Mesa intends to use this information to check for old
firmware versions with known bugs before enabling features like async
compute.
There was
https://patchwork.freedesktop.org/patch/560704/?series=124592=1
which does everything in one go so would be my preference.
During the time of that patch there was discussion whether firmware
version or submission version was better. I vaguely remember someone
raised an issue with the latter. Adding John in case he remembers.
Signed-off-by: Joonas Lahtinen
Cc: Kenneth Graunke
Cc: Jose Souza
Cc: Sagar Ghuge
Cc: Paulo Zanoni
Cc: John Harrison
Cc: Rodrigo Vivi
Cc: Jani Nikula
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_getparam.c | 12
include/uapi/drm/i915_drm.h | 13 +
2 files changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_getparam.c
b/drivers/gpu/drm/i915/i915_getparam.c
index 5c3fec63cb4c1..f176372debc54 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -113,6 +113,18 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
if (value < 0)
return value;
break;
+ case I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR:
+ case I915_PARAM_GUC_SUBMISSION_VERSION_MINOR:
+ case I915_PARAM_GUC_SUBMISSION_VERSION_PATCH:
+ if (!intel_uc_uses_guc_submission(_gt(i915)->uc))
+ return -ENODEV;
+ if (param->param == I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR)
+ value = to_gt(i915)->uc.guc.submission_version.major;
+ else if (param->param ==
I915_PARAM_GUC_SUBMISSION_VERSION_MINOR)
+ value = to_gt(i915)->uc.guc.submission_version.minor;
+ else
+ value = to_gt(i915)->uc.guc.submission_version.patch;
+ break;
case I915_PARAM_MMAP_GTT_VERSION:
/* Though we've started our numbering from 1, and so class all
* earlier versions as 0, in effect their value is undefined as
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index fd4f9574d177a..7d5a47f182542 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -806,6 +806,19 @@ typedef struct drm_i915_irq_wait {
*/
#define I915_PARAM_PXP_STATUS 58
+/*
+ * Query for the GuC submission/VF interface version number
What is this VF you speak of? :/
Regards,
Tvrtko
+ *
+ * -ENODEV is returned if GuC submission is not used
+ *
+ * On success, returns the respective GuC submission/VF interface major,
+ * minor or patch version as per the requested parameter.
+ *
+ */
+#define I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR 59
+#define I915_PARAM_GUC_SUBMISSION_VERSION_MINOR 60
+#define I915_PARAM_GUC_SUBMISSION_VERSION_PATCH 61
+
/* Must be kept compact -- no holes and well documented */
/**
[PATCH] drm/i915: limit eDP MSO pipe only for display version 20 and below
The pipes that can be used for eDP MSO are limited to pipe A (and
sometimes also pipe B) only for display version 20 and below.
Modify the function that returns the pipe mask for eDP MSO so that
these limitations only apply to version 20 and below, enabling all
pipes otherwise.
Bspec: 68923
Cc: Jani Nikula
Cc: James Ausmus
Signed-off-by: Luca Coelho
---
drivers/gpu/drm/i915/display/intel_ddi.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 922194b957be..5c99ae148213 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2336,13 +2336,18 @@ static void intel_ddi_power_up_lanes(struct
intel_encoder *encoder,
}
}
-/* Splitter enable for eDP MSO is limited to certain pipes. */
+/*
+ * Splitter enable for eDP MSO is limited to certain pipes, on certain
+ * platforms.
+ */
static u8 intel_ddi_splitter_pipe_mask(struct drm_i915_private *i915)
{
if (IS_ALDERLAKE_P(i915))
return BIT(PIPE_A) | BIT(PIPE_B);
- else
+ else if (DISPLAY_VER(i915) <= 20)
return BIT(PIPE_A);
+
+ return ~0;
}
static void intel_ddi_mso_get_config(struct intel_encoder *encoder,
--
2.39.2
✗ Fi.CI.BAT: failure for drm/i915: Add GETPARAM for GuC submission version
== Series Details == Series: drm/i915: Add GETPARAM for GuC submission version URL : https://patchwork.freedesktop.org/series/129122/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14168 -> Patchwork_129122v1 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_129122v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_129122v1, 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_129122v1/index.html Participating hosts (38 -> 37) -- Additional (1): bat-kbl-2 Missing(2): fi-snb-2520m fi-pnv-d510 Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_129122v1: ### IGT changes ### Possible regressions * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size: - fi-tgl-1115g4: [PASS][1] -> [SKIP][2] +5 other tests skip [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-flip-after-cursor-varying-size.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129122v1/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-flip-after-cursor-varying-size.html * igt@kms_pipe_crc_basic@hang-read-crc: - fi-tgl-1115g4: NOTRUN -> [SKIP][3] +6 other tests skip [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129122v1/fi-tgl-1115g4/igt@kms_pipe_crc_ba...@hang-read-crc.html Warnings * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - fi-tgl-1115g4: [SKIP][4] ([i915#4103]) -> [SKIP][5] +1 other test skip [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129122v1/fi-tgl-1115g4/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html * igt@kms_dsc@dsc-basic: - fi-tgl-1115g4: [SKIP][6] ([i915#3555] / [i915#3840]) -> [SKIP][7] [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@kms_...@dsc-basic.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129122v1/fi-tgl-1115g4/igt@kms_...@dsc-basic.html Known issues Here are the changes found in Patchwork_129122v1 that come from known issues: ### IGT changes ### Issues hit * igt@fbdev@info: - bat-kbl-2: NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#1849]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129122v1/bat-kbl-2/igt@fb...@info.html - fi-tgl-1115g4: [PASS][9] -> [SKIP][10] ([i915#1849] / [i915#2582]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@fb...@info.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129122v1/fi-tgl-1115g4/igt@fb...@info.html * igt@fbdev@nullptr: - fi-tgl-1115g4: [PASS][11] -> [SKIP][12] ([i915#2582]) +3 other tests skip [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14168/fi-tgl-1115g4/igt@fb...@nullptr.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129122v1/fi-tgl-1115g4/igt@fb...@nullptr.html * igt@gem_lmem_swapping@basic: - fi-apl-guc: NOTRUN -> [SKIP][13] ([fdo#109271] / [i915#4613]) +3 other tests skip [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129122v1/fi-apl-guc/igt@gem_lmem_swapp...@basic.html * igt@gem_lmem_swapping@parallel-random-engines: - bat-kbl-2: NOTRUN -> [SKIP][14] ([fdo#109271]) +35 other tests skip [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129122v1/bat-kbl-2/igt@gem_lmem_swapp...@parallel-random-engines.html * igt@gem_lmem_swapping@verify-random: - bat-mtlp-8: NOTRUN -> [SKIP][15] ([i915#4613]) +3 other tests skip [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129122v1/bat-mtlp-8/igt@gem_lmem_swapp...@verify-random.html * igt@i915_pm_rps@basic-api: - bat-mtlp-8: NOTRUN -> [SKIP][16] ([i915#6621]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129122v1/bat-mtlp-8/igt@i915_pm_...@basic-api.html * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy: - bat-mtlp-8: NOTRUN -> [SKIP][17] ([i915#5190]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129122v1/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][18] ([i915#4212]) +8 other tests skip [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129122v1/bat-mtlp-8/igt@kms_addfb_ba...@basic-y-tiled-legacy.html * igt@kms_busy@basic: - fi-tgl-1115g4: NOTRUN
✗ Fi.CI.SPARSE: warning for drm/i915: Add GETPARAM for GuC submission version
== Series Details == Series: drm/i915: Add GETPARAM for GuC submission version URL : https://patchwork.freedesktop.org/series/129122/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
Re: [Intel-gfx] [PATCH v7] drm/i915: handle uncore spinlock when not available
On 2023-12-01 12:00:32 [+0200], Luca Coelho wrote:
> To handle this, split the spin_lock/unlock_irqsave/restore() into
> spin_lock/unlock() followed by a call to local_irq_save/restore() and
On PREEMPT_RT spinlock_t becomes a sleeping lock so this split is not
working. See Documentation/locking/locktypes.rst
What I don't understand: do you have to keep the interrupts disabled
for some reasons or is it just to avoid using _irqsave() twice?
I do have more i915 related patches in the PREEMPT_RT queue and I can
post them if you folks have time for it. For now, let me show what I did
here:
--->8-
From: Mike Galbraith
Date: Sat, 27 Feb 2016 08:09:11 +0100
Subject: [PATCH 03/10] drm/i915: Use preempt_disable/enable_rt() where
recommended
Mario Kleiner suggest in commit
ad3543ede630f ("drm/intel: Push get_scanout_position() timestamping into kms
driver.")
a spots where preemption should be disabled on PREEMPT_RT. The
difference is that on PREEMPT_RT the intel_uncore::lock disables neither
preemption nor interrupts and so region remains preemptible.
The area covers only register reads and writes. The part that worries me
is:
- __intel_get_crtc_scanline() the worst case is 100us if no match is
found.
- intel_crtc_scanlines_since_frame_timestamp() not sure how long this
may take in the worst case.
It was in the RT queue for a while and nobody complained.
Disable preemption on PREEPMPT_RT during timestamping.
[bigeasy: patch description.]
Cc: Mario Kleiner
Signed-off-by: Mike Galbraith
Signed-off-by: Thomas Gleixner
Signed-off-by: Sebastian Andrzej Siewior
---
drivers/gpu/drm/i915/display/intel_vblank.c | 38
1 file changed, 28 insertions(+), 10 deletions(-)
--- a/drivers/gpu/drm/i915/display/intel_vblank.c
+++ b/drivers/gpu/drm/i915/display/intel_vblank.c
@@ -275,6 +275,26 @@ int intel_crtc_scanline_to_hw(struct int
* all register accesses to the same cacheline to be serialized,
* otherwise they may hang.
*/
+static void intel_vblank_section_enter_irqsave(struct drm_i915_private *i915,
unsigned long *flags)
+ __acquires(i915->uncore.lock)
+{
+#ifdef I915
+ spin_lock_irqsave(>uncore.lock, *flags);
+#else
+ *flags = NULL;
+#endif
+}
+
+static void intel_vblank_section_exit_irqrestore(struct drm_i915_private
*i915, unsigned long flags)
+ __releases(i915->uncore.lock)
+{
+#ifdef I915
+ spin_unlock_irqrestore(>uncore.lock, flags);
+#else
+ if (flags)
+ ;
+#endif
+}
static void intel_vblank_section_enter(struct drm_i915_private *i915)
__acquires(i915->uncore.lock)
{
@@ -332,10 +352,10 @@ static bool i915_get_crtc_scanoutpos(str
* timing critical raw register reads, potentially with
* preemption disabled, so the following code must not block.
*/
- local_irq_save(irqflags);
- intel_vblank_section_enter(dev_priv);
+ intel_vblank_section_enter_irqsave(dev_priv, );
- /* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
+ if (IS_ENABLED(CONFIG_PREEMPT_RT))
+ preempt_disable();
/* Get optional system timestamp before query. */
if (stime)
@@ -399,10 +419,10 @@ static bool i915_get_crtc_scanoutpos(str
if (etime)
*etime = ktime_get();
- /* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */
+ if (IS_ENABLED(CONFIG_PREEMPT_RT))
+ preempt_enable();
- intel_vblank_section_exit(dev_priv);
- local_irq_restore(irqflags);
+ intel_vblank_section_exit_irqrestore(dev_priv, irqflags);
/*
* While in vblank, position will be negative
@@ -440,13 +460,11 @@ int intel_get_crtc_scanline(struct intel
unsigned long irqflags;
int position;
- local_irq_save(irqflags);
- intel_vblank_section_enter(dev_priv);
+ intel_vblank_section_enter_irqsave(dev_priv, );
position = __intel_get_crtc_scanline(crtc);
- intel_vblank_section_exit(dev_priv);
- local_irq_restore(irqflags);
+ intel_vblank_section_exit_irqrestore(dev_priv, irqflags);
return position;
}
[RFC PATCH] drm/i915: Add GETPARAM for GuC submission version
Add reporting of the GuC submissio/VF interface version via GETPARAM
properties. Mesa intends to use this information to check for old
firmware versions with known bugs before enabling features like async
compute.
Signed-off-by: Joonas Lahtinen
Cc: Kenneth Graunke
Cc: Jose Souza
Cc: Sagar Ghuge
Cc: Paulo Zanoni
Cc: John Harrison
Cc: Rodrigo Vivi
Cc: Jani Nikula
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_getparam.c | 12
include/uapi/drm/i915_drm.h | 13 +
2 files changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_getparam.c
b/drivers/gpu/drm/i915/i915_getparam.c
index 5c3fec63cb4c1..f176372debc54 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -113,6 +113,18 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
if (value < 0)
return value;
break;
+ case I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR:
+ case I915_PARAM_GUC_SUBMISSION_VERSION_MINOR:
+ case I915_PARAM_GUC_SUBMISSION_VERSION_PATCH:
+ if (!intel_uc_uses_guc_submission(_gt(i915)->uc))
+ return -ENODEV;
+ if (param->param == I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR)
+ value = to_gt(i915)->uc.guc.submission_version.major;
+ else if (param->param ==
I915_PARAM_GUC_SUBMISSION_VERSION_MINOR)
+ value = to_gt(i915)->uc.guc.submission_version.minor;
+ else
+ value = to_gt(i915)->uc.guc.submission_version.patch;
+ break;
case I915_PARAM_MMAP_GTT_VERSION:
/* Though we've started our numbering from 1, and so class all
* earlier versions as 0, in effect their value is undefined as
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index fd4f9574d177a..7d5a47f182542 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -806,6 +806,19 @@ typedef struct drm_i915_irq_wait {
*/
#define I915_PARAM_PXP_STATUS 58
+/*
+ * Query for the GuC submission/VF interface version number
+ *
+ * -ENODEV is returned if GuC submission is not used
+ *
+ * On success, returns the respective GuC submission/VF interface major,
+ * minor or patch version as per the requested parameter.
+ *
+ */
+#define I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR 59
+#define I915_PARAM_GUC_SUBMISSION_VERSION_MINOR 60
+#define I915_PARAM_GUC_SUBMISSION_VERSION_PATCH 61
+
/* Must be kept compact -- no holes and well documented */
/**
--
2.43.0
Re: [PATCH 3/3] drm/i915: Convert REG_GENMASK* to fixed-width GENMASK_*
On Tue, 23 Jan 2024, Lucas De Marchi wrote: > Now that include/linux/bits.h implements fixed-width GENMASK_*, use them > to implement the i915/xe specific macros. Converting each driver to use > the generic macros are left for later, when/if other driver-specific > macros are also generalized. With the type-specific max checks added to GENMASK_*, this would be great. BR, Jani. > > Signed-off-by: Lucas De Marchi > --- > drivers/gpu/drm/i915/i915_reg_defs.h | 108 +++ > 1 file changed, 11 insertions(+), 97 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h > b/drivers/gpu/drm/i915/i915_reg_defs.h > index a685db1e815d..52f99eb96f86 100644 > --- a/drivers/gpu/drm/i915/i915_reg_defs.h > +++ b/drivers/gpu/drm/i915/i915_reg_defs.h > @@ -9,76 +9,19 @@ > #include > #include > > -/** > - * REG_BIT() - Prepare a u32 bit value > - * @__n: 0-based bit number > - * > - * Local wrapper for BIT() to force u32, with compile time checks. > - * > - * @return: Value with bit @__n set. > - */ > -#define REG_BIT(__n) \ > - ((u32)(BIT(__n) + \ > -BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \ > - ((__n) < 0 || (__n) > 31 > - > -/** > - * REG_BIT8() - Prepare a u8 bit value > - * @__n: 0-based bit number > - * > - * Local wrapper for BIT() to force u8, with compile time checks. > - * > - * @return: Value with bit @__n set. > - */ > -#define REG_BIT8(__n) \ > - ((u8)(BIT(__n) +\ > -BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \ > - ((__n) < 0 || (__n) > 7 > - > -/** > - * REG_GENMASK() - Prepare a continuous u32 bitmask > - * @__high: 0-based high bit > - * @__low: 0-based low bit > - * > - * Local wrapper for GENMASK() to force u32, with compile time checks. > - * > - * @return: Continuous bitmask from @__high to @__low, inclusive. > - */ > -#define REG_GENMASK(__high, __low) \ > - ((u32)(GENMASK(__high, __low) + \ > -BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \ > - __is_constexpr(__low) && \ > - ((__low) < 0 || (__high) > 31 || (__low) > > (__high) > - > -/** > - * REG_GENMASK64() - Prepare a continuous u64 bitmask > - * @__high: 0-based high bit > - * @__low: 0-based low bit > - * > - * Local wrapper for GENMASK_ULL() to force u64, with compile time checks. > - * > - * @return: Continuous bitmask from @__high to @__low, inclusive. > +/* > + * Wrappers over the generic BIT_* and GENMASK_* implementations, > + * for compatibility reasons with previous implementation > */ > -#define REG_GENMASK64(__high, __low) \ > - ((u64)(GENMASK_ULL(__high, __low) + \ > -BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \ > - __is_constexpr(__low) && \ > - ((__low) < 0 || (__high) > 63 || (__low) > > (__high) > +#define REG_GENMASK(__high, __low) GENMASK_U32(__high, __low) > +#define REG_GENMASK64(__high, __low) GENMASK_U64(__high, __low) > +#define REG_GENMASK16(__high, __low) GENMASK_U16(__high, __low) > +#define REG_GENMASK8(__high, __low) GENMASK_U8(__high, __low) > > -/** > - * REG_GENMASK8() - Prepare a continuous u8 bitmask > - * @__high: 0-based high bit > - * @__low: 0-based low bit > - * > - * Local wrapper for GENMASK() to force u8, with compile time checks. > - * > - * @return: Continuous bitmask from @__high to @__low, inclusive. > - */ > -#define REG_GENMASK8(__high, __low) \ > - ((u8)(GENMASK(__high, __low) + \ > -BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \ > - __is_constexpr(__low) && \ > - ((__low) < 0 || (__high) > 7 || (__low) > > (__high) > +#define REG_BIT(__n) BIT_U32(__n) > +#define REG_BIT64(__n) BIT_U64(__n) > +#define REG_BIT16(__n) BIT_U16(__n) > +#define REG_BIT8(__n)BIT_U8(__n) > > /* > * Local integer constant expression version of is_power_of_2(). > @@ -143,35 +86,6 @@ > */ > #define REG_FIELD_GET64(__mask, __val) ((u64)FIELD_GET(__mask, __val)) > > -/** > - * REG_BIT16() - Prepare a u16 bit value > - * @__n: 0-based bit number > - * > - * Local wrapper for BIT() to force u16, with compile time > - * checks. > - * > - * @return: Value with bit @__n set. > - */ > -#define REG_BIT16(__n) \ > - ((u16)(BIT(__n) +
