RE: [PATCH v3] drm/i915/panelreplay: Panel replay workaround with VRR
> -Original Message- > From: Ville Syrjälä > Sent: Thursday, February 22, 2024 2:55 AM > To: Vivi, Rodrigo > Cc: Manna, Animesh ; intel- > g...@lists.freedesktop.org; Hogander, Jouni ; > Murthy, Arun R > Subject: Re: [PATCH v3] drm/i915/panelreplay: Panel replay workaround with > VRR > > On Wed, Feb 21, 2024 at 11:08:18PM +0200, Ville Syrjälä wrote: > > On Wed, Feb 21, 2024 at 03:58:48PM -0500, Rodrigo Vivi wrote: > > > On Wed, Feb 21, 2024 at 08:19:35PM +, Manna, Animesh wrote: > > > > > > > > > > > > > -Original Message- > > > > > From: Vivi, Rodrigo > > > > > Sent: Tuesday, February 20, 2024 11:12 PM > > > > > To: Manna, Animesh > > > > > Cc: intel-gfx@lists.freedesktop.org; > > > > > ville.syrj...@linux.intel.com; Hogander, Jouni > > > > > ; Murthy, Arun R > > > > > > > > > > Subject: Re: [PATCH v3] drm/i915/panelreplay: Panel replay > > > > > workaround with VRR > > > > > > > > > > On Tue, Feb 20, 2024 at 07:49:19PM +0530, Animesh Manna wrote: > > > > > > Panel Replay VSC SDP not getting sent when VRR is enabled and > > > > > > W1 and > > > > > > W2 are 0. So Program Set Context Latency in > > > > > TRANS_SET_CONTEXT_LATENCY > > > > > > register to at least a value of 1. > > > > > > > > > > > > HSD: 14015406119 > > > > > > > > > > Unnecessary mark since the wa_name already is a pointer to the HSD. > > > > > > > > > > > > > > > > > v1: Initial version. > > > > > > v2: Update timings stored in adjusted_mode struct. [Ville] > > > > > > v3: Add WA in compute_config(). [Ville] > > > > > > > > > > > > Signed-off-by: Animesh Manna > > > > > > --- > > > > > > drivers/gpu/drm/i915/display/intel_dp.c | 12 > > > > > > 1 file changed, 12 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > > index 217196196e50..eb0fa513cd0f 100644 > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > > @@ -2948,6 +2948,18 @@ intel_dp_compute_config(struct > > > > > > intel_encoder > > > > > *encoder, > > > > > > intel_dp_compute_vsc_sdp(intel_dp, pipe_config, > conn_state); > > > > > > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, > > > > > pipe_config, > > > > > > conn_state); > > > > > > > > > > > > + /* > > > > > > +* WA: HSD-14015406119 > > > > > > > > > > this is not the right one. You should use the lineage one and > > > > > then mark the platforms. > > > > > > > > > > /* wa_14015401596: xe_lpd, xe_hpd */ > > > > > > > > > > or perhaps > > > > > > > > > > /* wa_14015401596: display versions: 13, 14 */ > > > > > > > > > > and also add a check for the display version with it. > > > > > > > > Sure. > > > > > > > > > > > > > > > +* Program Set Context Latency in > TRANS_SET_CONTEXT_LATENCY > > > > > register > > > > > > +* to at least a value of 1 when Panel Replay is enabled with > VRR. > > > > > > +* Value for TRANS_SET_CONTEXT_LATENCY is calculated by > > > > > substracting > > > > > > +* crtc_vdisplay from crtc_vblank_start, so incrementing > > > > > crtc_vblank_start > > > > > > +* by 1 if both are equal. > > > > > > +*/ > > > > > > + if (pipe_config->vrr.enable && pipe_config- > >has_panel_replay && > > > > > > + adjusted_mode->crtc_vblank_start == adjusted_mode- > > > > > >crtc_vdisplay) > > > > > > + adjusted_mode->crtc_vblank_start += 1; > > > > > > > > > > why to mess with the vblank start instead of going to > > > > > intel_set_transcoder_timings() and change directly what is > > > > > getting written to the register when the register gets written? > > > > > > > > I have done in previous version of this patch. But as per review > feedback, added now here. > > > > https://patchwork.freedesktop.org/series/129632/#rev1 > > > > https://patchwork.freedesktop.org/series/129632/#rev2 > > > > > > > > > > > > > > In case the answer is becasue by then we don't have the > > > > > vrr.enable or something like that, then we should consider move > > > > > around when we set that register? > > > > > > > > This was not acceptable in earlier versions. As per feedback instead of > atomic-commit need to add in compute-config phase. > > > > > > > > > > > > > > or perhaps create a specific flag? one extra variable, 3 less comment > lines... > > > > > > > > The above comment is not clear to me, can you please elaborate more > here. > > > > > > something like: > > > > > > @intel_dp_compute_config() > > > > > > +if (pipe_config->vrr.enable && pipe_config->has_panel_replay && > > > + adjusted_mode->crtc_vblank_start == adjusted_mode-crtc_vdisplay) > > > + pipe_config->mode_flags = > > > +I915_MODE_FLAG_MIN_TRANS_CONTEXT_LATENCY_1 > > > > > > then > > > @intel_set_transcoder_timings() > > > +u32 context_latency; > > > > > > +if (crtc_state->mode_flags & > I915_MODE_FLAG_MIN_TRANS_CONTEXT_LATENCY_1) > > > + context_latency = 1; > > > +else > > > +
RE: [PATCH v5 0/5] ALPM AUX-Less
> -Original Message- > From: Hogander, Jouni > Sent: Friday, March 22, 2024 4:00 PM > To: intel-gfx@lists.freedesktop.org > Cc: Ville Syrjälä ; Manna, Animesh > ; Murthy, Arun R ; > Hogander, Jouni > Subject: [PATCH v5 0/5] ALPM AUX-Less > > This patch set is implementing calculation of ALPM AUX-Less parameters for > Intel HW and writing them in case of AUX-Less is enabled. It is also enabling > ALPM AUX-Less for eDP Panel Replay. Current code is not allowing Panel > Replay on eDP. Patches for this are coming later. > > This implementation is only for Panel Replay usage. LOBF (Link Off Between > Active Frames) usage needs more work. > > v5: > - mention AUX Less enable is only on source side in commit message > v4: > - drop patch adding AUX LESS dpcd defines > - re-use fast_wake_lines to store aux_less_wake_lines > - add comment explaining why AUX less is enabled on eDP panel replay > without any extra checks > v3: > - use definitions instead of numbers for max values > - do not use alpm_ctl as uninitialized variable > v2: > - use variables instead of values directly > - fix several max values > - move converting port clock to Mhz into _lnl_compute_* > - do not set AUX-Wake related bits for AUX-Less case > - do not write ALPM configuration for DP2.0 Panel Replay or PSR1 > > Jouni Högander (5): > drm/i915/psr: Add missing ALPM AUX-Less register definitions > drm/i915/psr: Calculate aux less wake time > drm/i915/psr: Silence period and lfps half cycle > drm/i915/psr: Enable ALPM on source side for eDP Panel replay > drm/i915/psr: Do not write ALPM configuration for PSR1 or DP2.0 Panel > Replay The above patches LGTM. For whole patch series: Reviewed-by: Animesh Manna > > .../drm/i915/display/intel_display_types.h| 2 + > drivers/gpu/drm/i915/display/intel_psr.c | 188 +- > drivers/gpu/drm/i915/display/intel_psr_regs.h | 12 +- > 3 files changed, 193 insertions(+), 9 deletions(-) > > -- > 2.34.1
RE: [PATCH] drm/xe/display: fix potential overflow when multiplying 2 u32
Any comments? Thanks and Regards, Arun R Murthy > -Original Message- > From: Murthy, Arun R > Sent: Monday, March 18, 2024 4:31 PM > To: intel-gfx@lists.freedesktop.org; intel...@lists.freedesktop.org > Cc: Murthy, Arun R > Subject: [PATCH] drm/xe/display: fix potential overflow when multiplying 2 u32 > > Multiplying XE_PAGE_SIZE with another u32 and the product stored in > u64 can potentially lead to overflow, use mul_u32_u32 instead. > > Signed-off-by: Arun R Murthy > --- > drivers/gpu/drm/xe/display/xe_fb_pin.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c > b/drivers/gpu/drm/xe/display/xe_fb_pin.c > index 722c84a56607..e0b511ff7eab 100644 > --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c > +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c > @@ -29,7 +29,7 @@ write_dpt_rotated(struct xe_bo *bo, struct iosys_map > *map, u32 *dpt_ofs, u32 bo_ > u32 src_idx = src_stride * (height - 1) + column + bo_ofs; > > for (row = 0; row < height; row++) { > - u64 pte = ggtt->pt_ops->pte_encode_bo(bo, src_idx * > XE_PAGE_SIZE, > + u64 pte = ggtt->pt_ops->pte_encode_bo(bo, > mul_u32_u32(src_idx, > +XE_PAGE_SIZE), > xe- > >pat.idx[XE_CACHE_WB]); > > iosys_map_wr(map, *dpt_ofs, u64, pte); @@ -61,7 > +61,7 @@ write_dpt_remapped(struct xe_bo *bo, struct iosys_map *map, u32 > *dpt_ofs, > > for (column = 0; column < width; column++) { > iosys_map_wr(map, *dpt_ofs, u64, > - pte_encode_bo(bo, src_idx * XE_PAGE_SIZE, > + pte_encode_bo(bo, mul_u32_u32(src_idx, > XE_PAGE_SIZE), >xe->pat.idx[XE_CACHE_WB])); > > *dpt_ofs += 8; > @@ -118,7 +118,7 @@ static int __xe_pin_fb_vma_dpt(struct > intel_framebuffer *fb, > u32 x; > > for (x = 0; x < size / XE_PAGE_SIZE; x++) { > - u64 pte = ggtt->pt_ops->pte_encode_bo(bo, x * > XE_PAGE_SIZE, > + u64 pte = ggtt->pt_ops->pte_encode_bo(bo, > mul_u32_u32(x, > +XE_PAGE_SIZE), > xe- > >pat.idx[XE_CACHE_WB]); > > iosys_map_wr(>vmap, x * 8, u64, pte); @@ - > 164,7 +164,7 @@ write_ggtt_rotated(struct xe_bo *bo, struct xe_ggtt *ggtt, > u32 *ggtt_ofs, u32 bo > u32 src_idx = src_stride * (height - 1) + column + bo_ofs; > > for (row = 0; row < height; row++) { > - u64 pte = ggtt->pt_ops->pte_encode_bo(bo, src_idx * > XE_PAGE_SIZE, > + u64 pte = ggtt->pt_ops->pte_encode_bo(bo, > mul_u32_u32(src_idx, > +XE_PAGE_SIZE), > xe- > >pat.idx[XE_CACHE_WB]); > > xe_ggtt_set_pte(ggtt, *ggtt_ofs, pte); @@ -381,4 > +381,4 @@ struct i915_address_space *intel_dpt_create(struct > intel_framebuffer *fb) void intel_dpt_destroy(struct i915_address_space *vm) > { > return; > -} > \ No newline at end of file > +} > -- > 2.25.1
[PATCH v4] drm/i915/panelreplay: Panel replay workaround with VRR
Panel Replay VSC SDP not getting sent when VRR is enabled and W1 and W2 are 0. So Program Set Context Latency in TRANS_SET_CONTEXT_LATENCY register to at least a value of 1. HSD: 14015406119 v1: Initial version. v2: Update timings stored in adjusted_mode struct. [Ville] v3: Add WA in compute_config(). [Ville] v4: - Add DISPLAY_VER() check and improve code comment. [Rodrigo] - Introduce centralized intel_crtc_vblank_delay(). [Ville] Signed-off-by: Animesh Manna --- drivers/gpu/drm/i915/display/intel_display.c | 17 + drivers/gpu/drm/i915/display/intel_display.h | 1 + drivers/gpu/drm/i915/display/intel_psr.c | 4 3 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 00ac65a14029..7f5c42a14196 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -3840,6 +3840,23 @@ bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state) return true; } +void intel_crtc_vblank_delay(struct intel_crtc_state *crtc_state) +{ + struct drm_display_mode *adjusted_mode = _state->hw.adjusted_mode; + + /* +* wa_14015401596 for display versions >= 13. +* Program Set Context Latency in TRANS_SET_CONTEXT_LATENCY register +* to at least a value of 1 when Panel Replay is enabled with VRR. +* Value for TRANS_SET_CONTEXT_LATENCY is calculated by substracting +* crtc_vdisplay from crtc_vblank_start, so incrementing crtc_vblank_start +* by 1 if both are equal. +*/ + if (crtc_state->vrr.enable && crtc_state->has_panel_replay && + adjusted_mode->crtc_vblank_start == adjusted_mode->crtc_vdisplay) + adjusted_mode->crtc_vblank_start += 1; +} + int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n) { diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h index f4a0773f0fca..23315eced41e 100644 --- a/drivers/gpu/drm/i915/display/intel_display.h +++ b/drivers/gpu/drm/i915/display/intel_display.h @@ -413,6 +413,7 @@ bool intel_crtc_is_bigjoiner_master(const struct intel_crtc_state *crtc_state); u8 intel_crtc_bigjoiner_slave_pipes(const struct intel_crtc_state *crtc_state); struct intel_crtc *intel_master_crtc(const struct intel_crtc_state *crtc_state); bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state); +void intel_crtc_vblank_delay(struct intel_crtc_state *crtc_state); bool intel_pipe_config_compare(const struct intel_crtc_state *current_config, const struct intel_crtc_state *pipe_config, bool fastset); diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 72cadad09db5..fccef5434e9c 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1430,6 +1430,10 @@ void intel_psr_compute_config(struct intel_dp *intel_dp, if (!(crtc_state->has_panel_replay || crtc_state->has_psr)) return; + /* wa_14015401596: display versions 13, 14 */ + if (DISPLAY_VER(dev_priv) >= 13) + intel_crtc_vblank_delay(crtc_state); + crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state); } -- 2.29.0
Re: [PATCH 10/11] drm/i915/dp_mst: Make HBLANK expansion quirk work for logical ports
On 3/27/2024 7:49 PM, Imre Deak wrote: On Wed, Mar 27, 2024 at 01:40:58PM +0530, Nautiyal, Ankit K wrote: On 3/21/2024 1:41 AM, Imre Deak wrote: The DPCD OUI of the logical port on a Dell UHBR monitor - on which the AUX device is used to enable DSC - is all 0. To detect if the HBLANK expansion quirk is required for this monitor use the OUI of the port's parent instead. Since in the above case the DPCD of both the logical port and the parent port reports being a sink device (vs. branch device) type, read the proper sink/branch OUI based on the DPCD device type. This is required by a follow-up patch enabling the quirk for the above Dell monitor. Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/display/intel_dp_mst.c | 22 +++-- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 516b00f791420..76a8fb21b8e52 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1512,23 +1512,33 @@ intel_dp_mst_read_decompression_port_dsc_caps(struct intel_dp *intel_dp, static bool detect_dsc_hblank_expansion_quirk(const struct intel_connector *connector) { struct drm_i915_private *i915 = to_i915(connector->base.dev); + struct drm_dp_aux *aux = connector->dp.dsc_decompression_aux; struct drm_dp_desc desc; u8 dpcd[DP_RECEIVER_CAP_SIZE]; - if (!connector->dp.dsc_decompression_aux) + if (!aux) return false; - if (drm_dp_read_desc(connector->dp.dsc_decompression_aux, -, true) < 0) + /* +* A logical port's OUI (at least for affected sinks) is all 0, so +* instead of that the parent port's OUI is used for identification. +*/ + if (drm_dp_mst_port_is_logical(connector->port)) { + aux = drm_dp_mst_aux_for_parent(connector->port); + if (!aux) + aux = >mst_port->aux; As I understand, we are setting connector->mst_port as intel_dp, in the caller intel_dp_add_mst_connector so its unlikely that aux can be NULL, but do you see if we need to check for aux? Yes, intel_connector::mst_port (always) points to the intel_dp of the MST root port, and aux will be always initialized for all the registered DP encoders/intel_dps; so mst_port->aux will always point to a valid/non-NULL AUX device. (In any case above we take the address of intel_dp::aux, which can't be NULL.) Agreed. The change LGTM. Reviewed-by: Ankit Nautiyal Regards, Ankit + } + + if (drm_dp_read_dpcd_caps(aux, dpcd) < 0) + return false; + + if (drm_dp_read_desc(aux, , drm_dp_is_branch(dpcd)) < 0) return false; if (!drm_dp_has_quirk(, DP_DPCD_QUIRK_HBLANK_EXPANSION_REQUIRES_DSC)) return false; - if (drm_dp_read_dpcd_caps(connector->dp.dsc_decompression_aux, dpcd) < 0) - return false; - if (!(dpcd[DP_RECEIVE_PORT_0_CAP_0] & DP_HBLANK_EXPANSION_CAPABLE)) return false;
Re: [PATCH 09/11] drm/dp_mst: Add drm_dp_mst_aux_for_parent()
On 3/27/2024 7:55 PM, Imre Deak wrote: On Wed, Mar 27, 2024 at 02:30:53PM +0530, Nautiyal, Ankit K wrote: On 3/21/2024 1:41 AM, Imre Deak wrote: Add a function to get the AUX device of the parent of an MST port, used by a follow-up i915 patch in the patchset. Cc: Lyude Paul Cc: dri-de...@lists.freedesktop.org Signed-off-by: Imre Deak --- drivers/gpu/drm/display/drm_dp_mst_topology.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index 6bd471a2266ce..d70f7de644371 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -6004,6 +6004,22 @@ static bool drm_dp_mst_is_virtual_dpcd(struct drm_dp_mst_port *port) return false; } +/** + * drm_dp_mst_aux_for_parent() - Get the AUX device for an MST port's parent + * @port: MST port whose parent's AUX device is returned + * + * Return the AUX device for @port's parent or NULL if port's parent is the + * root port. + */ +struct drm_dp_aux *drm_dp_mst_aux_for_parent(struct drm_dp_mst_port *port) +{ + if (!port->parent || !port->parent->port_parent) + return NULL; + + return >parent->port_parent->aux; +} +EXPORT_SYMBOL(drm_dp_mst_aux_for_parent); As mentioned in previous patch, the declaration of this in the header, got included in previous patch. Yes thanks, the header change should've been in this patch, will move it here (while applying the patches if nothing else comes up). With the above fixed, this is: Reviewed-by: Ankit Nautiyal Regards, Ankit + /** * drm_dp_mst_dsc_aux_for_port() - Find the correct aux for DSC * @port: The port to check. A leaf of the MST tree with an attached display.
✗ Fi.CI.SPARSE: warning for drm/i915: A few bigjoiner fixes
== Series Details == Series: drm/i915: A few bigjoiner fixes URL : https://patchwork.freedesktop.org/series/131711/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:155:9: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:173:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:175:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:179:35: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:181:16: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:181:9: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:185:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:187:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:191:35: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:194:16: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:194:9: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:236:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:238:9: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:66:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:92:1: warning: unreplaced symbol 'return' +drivers/gpu/drm/i915/display/intel_display_types.h:1955:9: warning: too many warnings +drivers/gpu/drm/i915/display/intel_display_types.h:1981:9: warning: unreplaced symbol 'intel_encoder' +drivers/gpu/drm/i915/display/intel_display_types.h:2028:24: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/display/intel_display_types.h:2028:24: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/display/intel_display_types.h:2093:16: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/display/intel_display_types.h:2101:16: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/display/intel_display_types.h:2101:16: warning: unreplaced symbol 'crtc' +drivers/gpu/drm/i915/display/intel_display_types.h:2101:16: warning: unreplaced symbol 'return' +drivers/gpu/drm/i915/display/intel_display_types.h:2101:16: warning: unreplaced symbol 'state' +./include/asm-generic/bitops/generic-non-atomic.h:100:17: warning: unreplaced symbol 'old' +./include/asm-generic/bitops/generic-non-atomic.h:100:23: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:100:9: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:105:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:107:9: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:108:9: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:109:9: warning: unreplaced symbol 'old' +./include/asm-generic/bitops/generic-non-atomic.h:111:10: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:111:14: warning: unreplaced symbol 'old' +./include/asm-generic/bitops/generic-non-atomic.h:111:20: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:112:17: warning: unreplaced symbol 'old' +./include/asm-generic/bitops/generic-non-atomic.h:112:23: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:112:9: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:121:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:128:9: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:166:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:168:9: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:169:9: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:170:9: warning: unreplaced symbol 'val' +./include/asm-generic/bitops/generic-non-atomic.h:172:19: warning: unreplaced symbol 'val' +./include/asm-generic/bitops/generic-non-atomic.h:172:25: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:172:9: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:28:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:30:9: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:31:9: warning:
✗ Fi.CI.SPARSE: warning for drm/i915/gt: Limit the reserved VM space to only the platforms that need it
== Series Details == Series: drm/i915/gt: Limit the reserved VM space to only the platforms that need it URL : https://patchwork.freedesktop.org/series/131707/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: Limit the reserved VM space to only the platforms that need it
== Series Details == Series: drm/i915/gt: Limit the reserved VM space to only the platforms that need it URL : https://patchwork.freedesktop.org/series/131707/ State : warning == Summary == Error: dim checkpatch failed 438bf8f0a744 drm/i915/gt: Limit the reserved VM space to only the platforms that need it -:72: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'engine' - possible side-effects? #72: FILE: drivers/gpu/drm/i915/gt/intel_gt.h:93: +#define NEEDS_FASTCOLOR_BLT_WABB(engine) ( \ + intel_gt_needs_wa_16018031267(engine->gt) && \ + engine->class == COPY_ENGINE_CLASS && engine->instance == 0) -:72: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'engine' may be better as '(engine)' to avoid precedence issues #72: FILE: drivers/gpu/drm/i915/gt/intel_gt.h:93: +#define NEEDS_FASTCOLOR_BLT_WABB(engine) ( \ + intel_gt_needs_wa_16018031267(engine->gt) && \ + engine->class == COPY_ENGINE_CLASS && engine->instance == 0) total: 0 errors, 0 warnings, 2 checks, 43 lines checked
✗ Fi.CI.SPARSE: warning for drm/i915: Implemnt vblank sycnhronized mbus joining changes
== Series Details == Series: drm/i915: Implemnt vblank sycnhronized mbus joining changes URL : https://patchwork.freedesktop.org/series/131700/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately.
Re: [PATCH i-g-t] lib/kunit: Read results from debugfs
On Wednesday, 27 March 2024 18:27:50 CET Kamil Konieczny wrote: > Hi Janusz, > On 2024-03-27 at 12:22:54 +0100, Janusz Krzysztofik wrote: > > KUnit can provide KTAP reports from test modules via debugfs files, one > > per test suite. Using that source of test results instead of extracting > > them from dmesg, where they may be interleaved with other kernel messages, > > seems more easy to handle and less error prone. Switch to it. > > > > If KUnit debugfs support is found not configured then fall back to legacy > > processing path. > > > > Signed-off-by: Janusz Krzysztofik > > --- > > lib/igt_kmod.c | 143 - > > 1 file changed, 105 insertions(+), 38 deletions(-) > > > > diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c > > index 1ec9c8a602..a5b170ca9c 100644 > > --- a/lib/igt_kmod.c > > +++ b/lib/igt_kmod.c > > @@ -28,6 +28,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -39,6 +40,7 @@ > > > > #include "igt_aux.h" > > #include "igt_core.h" > > +#include "igt_debugfs.h" > > #include "igt_kmod.h" > > #include "igt_ktap.h" > > #include "igt_sysfs.h" > > @@ -864,6 +866,31 @@ static int open_parameters(const char *module_name) > > return open(path, O_RDONLY); > > } > > > > +static DIR *kunit_debugfs_open(void) > > +{ > > + const char *debugfs_path = igt_debugfs_mount(); > > + int debugfs_fd, kunit_fd; > > + DIR *kunit_dir; > > + > > + if (igt_debug_on(!debugfs_path)) > > + return NULL; > > + > > + debugfs_fd = open(debugfs_path, O_DIRECTORY); > > + if (igt_debug_on(debugfs_fd < 0)) > > + return NULL; > > + > > + kunit_fd = openat(debugfs_fd, "kunit", O_DIRECTORY); > > + close(debugfs_fd); > > + if (igt_debug_on(kunit_fd < 0)) > > + return NULL; > > + > > + kunit_dir = fdopendir(kunit_fd); > > + if (igt_debug_on(!kunit_dir)) > > + close(kunit_fd); > > imho here: > close(kunit_fd); > igt_debug_on(!kunit_dir); No, we are not allowed to close it. fdopendir(3) states: "After a successful call to fdopendir(), fd is used internally by the implementation, and should not otherwise be used by the application." > > > + > > + return kunit_dir; > > +} > > + > > static bool kunit_set_filtering(const char *filter_glob, const char > > *filter, > > const char *filter_action) > > { > > @@ -1071,23 +1098,48 @@ static void kunit_results_free(struct igt_list_head > > *results, > > free(*suite_name); > > } > > > > -static int kunit_get_results(struct igt_list_head *results, int kmsg_fd, > > -struct igt_ktap_results **ktap) > > +static int kunit_get_results(struct igt_list_head *results, int debugfs_fd, > > +const char *suite, struct igt_ktap_results **ktap) > > { > > - int err; > > + FILE *results_stream; > > + int ret, results_fd; > > + char *buf = NULL; > > + size_t size = 0; > > + ssize_t len; > > + > > + if (igt_debug_on((ret = openat(debugfs_fd, suite, O_DIRECTORY), ret < > > 0))) > > + return ret; > > + > > + results_fd = openat(ret, "results", O_RDONLY); > > + close(ret); > > + if (igt_debug_on(results_fd < 0)) > > + return results_fd; > > + > > + results_stream = fdopen(results_fd, "r"); > > + if (igt_debug_on(!results_stream)) { > > + close(results_fd); > > + return -errno; > > + } > > > > *ktap = igt_ktap_alloc(results); > > - if (igt_debug_on(!*ktap)) > > - return -ENOMEM; > > + if (igt_debug_on(!*ktap)) { > > + ret = -ENOMEM; > > + goto out_fclose; > > + } > > + > > + while (len = getline(, , results_stream), len > 0) { > > + ret = igt_ktap_parse(buf, *ktap); > > + if (ret != -EINPROGRESS) > > + break; > > + } > > > > - do > > - igt_debug_on((err = kunit_kmsg_result_get(results, NULL, > > kmsg_fd, *ktap), > > - err && err != -EINPROGRESS)); > > - while (err == -EINPROGRESS); > > + free(buf); > > > > igt_ktap_free(ktap); > > +out_fclose: > > + fclose(results_stream); > > > > - return err; > > + return ret; > > } > > > > static void __igt_kunit_legacy(struct igt_ktest *tst, > > @@ -1101,7 +1153,13 @@ static void __igt_kunit_legacy(struct igt_ktest *tst, > > pthread_mutexattr_t attr; > > IGT_LIST_HEAD(results); > > unsigned long taints; > > - int ret; > > + int flags, ret; > > + > > + igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n"); > > + > > + igt_skip_on((flags = fcntl(tst->kmsg, F_GETFL, 0), flags < 0)); > > + igt_skip_on_f(fcntl(tst->kmsg, F_SETFL, flags & ~O_NONBLOCK) == -1, > > + "Could not set /dev/kmsg to blocking mode\n"); > > > > igt_skip_on(lseek(tst->kmsg, 0, SEEK_END) < 0); > > > > @@ -1224,30 +1282,20 @@ static void __igt_kunit_legacy(struct
Re: [PATCH v7 2/3] drm/i915/gt: Do not generate the command streamer for all the CCS
On Wed, Mar 27, 2024 at 04:56:18PM +0100, Andi Shyti wrote: > We want a fixed load CCS balancing consisting in all slices > sharing one single user engine. For this reason do not create the > intel_engine_cs structure with its dedicated command streamer for > CCS slices beyond the first. > > Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement") > Signed-off-by: Andi Shyti > Cc: Chris Wilson > Cc: Joonas Lahtinen > Cc: Matt Roper > Cc: # v6.2+ > Acked-by: Michal Mrozek > --- > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 15 +++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > index f553cf4e6449..47c4a69e854c 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > @@ -908,6 +908,21 @@ static intel_engine_mask_t init_engine_mask(struct > intel_gt *gt) > info->engine_mask &= ~BIT(GSC0); > } > > + /* > + * Do not create the command streamer for CCS slices beyond the first. > + * All the workload submitted to the first engine will be shared among > + * all the slices. > + * > + * Once the user will be allowed to customize the CCS mode, then this > + * check needs to be removed. > + */ > + if (IS_DG2(gt->i915)) { > + intel_engine_mask_t first_ccs = BIT((CCS0 + > __ffs(CCS_MASK(gt; > + intel_engine_mask_t all_ccs = CCS_MASK(gt) << CCS0; > + > + info->engine_mask &= ~(all_ccs &= ~first_ccs); Shouldn't the second "&=" just be an "&" since there's no need to modify the all_ccs variable that never gets used again? In fact since this is DG2-specific, it seems like it might be more intuitive to just write the whole thing more directly as if (IS_DG2(gt->i915)) { int first_ccs = __ffs(CCS_MASK(gt)); info->engine_mask &= ~GENMASK(CCS3, CCS0); info->engine_mask |= BIT(_CCS(first_ccs)); } But up to you; if you just want to remove the unnecessary "=" that's fine too. Either way, Reviewed-by: Matt Roper Matt > + } > + > return info->engine_mask; > } > > -- > 2.43.0 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation
Re: [PATCH i-g-t] lib/kunit: Read results from debugfs
On Wednesday, 27 March 2024 17:03:01 CET Lucas De Marchi wrote: > On Wed, Mar 27, 2024 at 12:22:54PM +0100, Janusz Krzysztofik wrote: > >KUnit can provide KTAP reports from test modules via debugfs files, one > >per test suite. Using that source of test results instead of extracting > >them from dmesg, where they may be interleaved with other kernel messages, > >seems more easy to handle and less error prone. Switch to it. > > > >If KUnit debugfs support is found not configured then fall back to legacy > >processing path. > > > >Signed-off-by: Janusz Krzysztofik > >--- > > lib/igt_kmod.c | 143 - > > 1 file changed, 105 insertions(+), 38 deletions(-) > > > >diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c > >index 1ec9c8a602..a5b170ca9c 100644 > >--- a/lib/igt_kmod.c > >+++ b/lib/igt_kmod.c > >@@ -28,6 +28,7 @@ > > #include > > #include > > #include > >+#include > > #include > > #include > > #include > >@@ -39,6 +40,7 @@ > > > > #include "igt_aux.h" > > #include "igt_core.h" > >+#include "igt_debugfs.h" > > #include "igt_kmod.h" > > #include "igt_ktap.h" > > #include "igt_sysfs.h" > >@@ -864,6 +866,31 @@ static int open_parameters(const char *module_name) > > return open(path, O_RDONLY); > > } > > > >+static DIR *kunit_debugfs_open(void) > >+{ > >+const char *debugfs_path = igt_debugfs_mount(); > >+int debugfs_fd, kunit_fd; > >+DIR *kunit_dir; > >+ > >+if (igt_debug_on(!debugfs_path)) > >+return NULL; > >+ > >+debugfs_fd = open(debugfs_path, O_DIRECTORY); > >+if (igt_debug_on(debugfs_fd < 0)) > >+return NULL; > >+ > >+kunit_fd = openat(debugfs_fd, "kunit", O_DIRECTORY); > >+close(debugfs_fd); > >+if (igt_debug_on(kunit_fd < 0)) > >+return NULL; > >+ > >+kunit_dir = fdopendir(kunit_fd); > >+if (igt_debug_on(!kunit_dir)) > >+close(kunit_fd); > >+ > >+return kunit_dir; > > > any reason not to use strcat() + return fopen() To me the code looked simpler than if I copied and concatenated strings to a local buffer of fixed size with potential truncation handling. I guess you prefer your pattern over mine, but you haven't explained why. Would you like to convince me? > > >+} > >+ > > static bool kunit_set_filtering(const char *filter_glob, const char *filter, > > const char *filter_action) > > { > >@@ -1071,23 +1098,48 @@ static void kunit_results_free(struct igt_list_head > >*results, > > free(*suite_name); > > } > > > >-static int kunit_get_results(struct igt_list_head *results, int kmsg_fd, > >- struct igt_ktap_results **ktap) > >+static int kunit_get_results(struct igt_list_head *results, int debugfs_fd, > >+ const char *suite, struct igt_ktap_results **ktap) > > { > >-int err; > >+FILE *results_stream; > >+int ret, results_fd; > >+char *buf = NULL; > >+size_t size = 0; > >+ssize_t len; > >+ > >+if (igt_debug_on((ret = openat(debugfs_fd, suite, O_DIRECTORY), ret < > >0))) > > a little odd to return on any value != 0, but log on < 0. did you mean > to compare < 0 in the first arg?. I'm not able to recall what I could mean, but anyway, you are right, if (igt_debug_on((ret = openat(...)) < 0)) will be more correct. > > >+return ret; > >+ > >+results_fd = openat(ret, "results", O_RDONLY); > >+close(ret); > >+if (igt_debug_on(results_fd < 0)) > >+return results_fd; > >+ > >+results_stream = fdopen(results_fd, "r"); > >+if (igt_debug_on(!results_stream)) { > >+close(results_fd); > >+return -errno; > >+} > > > > *ktap = igt_ktap_alloc(results); > >-if (igt_debug_on(!*ktap)) > >-return -ENOMEM; > >+if (igt_debug_on(!*ktap)) { > >+ret = -ENOMEM; > >+goto out_fclose; > >+} > >+ > >+while (len = getline(, , results_stream), len > 0) { > >+ret = igt_ktap_parse(buf, *ktap); > >+if (ret != -EINPROGRESS) > >+break; > >+} > > > >-do > >-igt_debug_on((err = kunit_kmsg_result_get(results, NULL, > >kmsg_fd, *ktap), > >- err && err != -EINPROGRESS)); > >-while (err == -EINPROGRESS); > >+free(buf); > > > > igt_ktap_free(ktap); > >+out_fclose: > >+fclose(results_stream); > > > >-return err; > >+return ret; > > } > > > > static void __igt_kunit_legacy(struct igt_ktest *tst, > >@@ -1101,7 +1153,13 @@ static void __igt_kunit_legacy(struct igt_ktest *tst, > > pthread_mutexattr_t attr; > > IGT_LIST_HEAD(results); > > unsigned long taints; > >-int ret; > >+int flags, ret; > >+ > >+igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n"); > >+ > >+igt_skip_on((flags = fcntl(tst->kmsg, F_GETFL, 0), flags < 0)); > >+igt_skip_on_f(fcntl(tst->kmsg, F_SETFL, flags & ~O_NONBLOCK) == -1, > >+
Re: [PATCH] drm/i915/gt: Limit the reserved VM space to only the platforms that need it
Hi, On Wed, Mar 27, 2024 at 09:05:46PM +0100, Andi Shyti wrote: > Commit 9bb66c179f50 ("drm/i915: Reserve some kernel space per > vm") reduces the available VM space of one page in order to apply > Wa_16018031267 and Wa_16018063123. > > This page was reserved indiscrimitely in all platforms even when > not needed. Limit it to DG2 onwards. > > Fixes: 9bb66c179f50 ("drm/i915: Reserve some kernel space per vm") > Signed-off-by: Andi Shyti > Cc: Andrzej Hajda > Cc: Chris Wilson > Cc: Jonathan Cavitt > Cc: Nirmoy Das I forgot to add stable here: Cc: # v6.8+ Sorry about that! Andi
Re: [PATCH] drm/i915/hwmon: Remove i915_hwmon_unregister() during driver unbind
On Wed, 27 Mar 2024 02:15:27 -0700, Krzysztofik, Janusz wrote: > Hi Janusz, > For me, that still doesn't explain why you think that i915->hwmon reset to > NULL on i915 driver unregister can be the root cause of the reported UAF in > hwmon sysfs and this patch is going to fix that UAF issue. I can see no > references to i915->hwmon in that code, and even then, that's not NULL what is > reported here as non-canonical address. The code is using a no longer valid > pointer to an already freed (and poisoned) memory. Correct, I said basically the same thing in my reply to the patch. That the patch doesn't explain that ddat seems to have been freed and poisoned. > > > I think that instead of dropping i915_hwmon_unregister() we have to > > > actually > > > unregister hwmon in the body of that function, which is called from > > > i915_driver_unregister() intended for closing user interfaces, then called > > > relatively early during driver unbind, so hwmon sysfs entries disappear > > > before > > > i915 structures, especially uncore used by hwmon code, are freed. > > > > You mean uncore are freed before hwmon get unregistered, I don't think > > so. uncore is also drm device managed resource so in sequence I think it > > should be freed after i915 dev managed resources freed. > > If both uncore and hwmon are managed resources of i915 device then how can you > predict which of them is released first? Look at __hwmon_device_register. Here we see: hdev->parent = dev So the hwmon device is a child device of the drm device (against which ddat is devm_kzalloc'd). Since a child device holds a reference against the parent (device_add() has get_device(dev->parent)), I would expect the hwmon device to disappear before the parent drm device. And I am assuming hwmon sysfs is linked to the hwmon device, so sysfs should disappear before ddat getting freed. But apparently this is not what is happening, so there's still something we are missing. Thanks. -- Ashutosh
[PATCH 2/2] drm/i915: Fix intel_modeset_pipe_config_late() for bigjoiner
From: Ville Syrjälä Currently intel_modeset_pipe_config_late() is called after the bigjoiner state copy, and it will actually not do anything for bigjoiner slaves. This can lead to a mismatched state between the master and slave. The two things that we do in the encoder .compute_config_late() hook are mst master transcoder and port sync master transcoder elections. So if either of either MST or port sync is combined with bigjoiner then we can see the mismatch. Currently this problem is more or less theoretical; MST+bigjoiner has not been implemented yet, and port sync+bigjoiner would require a tiled display with >5k tiles (or a very high dotclock per tile). Although we do have kms_tiled_display in igt which can fake a tiled display, and we can now force bigjoiner via debugfs, so it is possible to trigger this if you try hard enough. Reorder the code such that intel_modeset_pipe_config_late() will be called before the bigjoiner state copy happens so that both pipes will end up with the same state. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 46 ++-- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 4d6668a5f1ab..e22326362ccb 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -4762,8 +4762,6 @@ intel_modeset_pipe_config_late(struct intel_atomic_state *state, struct drm_connector *connector; int i; - intel_bigjoiner_adjust_pipe_src(crtc_state); - for_each_new_connector_in_state(>base, connector, conn_state, i) { struct intel_encoder *encoder = @@ -6257,27 +6255,37 @@ static int intel_atomic_check_config(struct intel_atomic_state *state, continue; } - if (intel_crtc_is_bigjoiner_slave(new_crtc_state)) { - drm_WARN_ON(>drm, new_crtc_state->uapi.enable); + if (drm_WARN_ON(>drm, intel_crtc_is_bigjoiner_slave(new_crtc_state))) continue; - } ret = intel_crtc_prepare_cleared_state(state, crtc); if (ret) - break; + goto fail; if (!new_crtc_state->hw.enable) continue; ret = intel_modeset_pipe_config(state, crtc, limits); if (ret) - break; + goto fail; + } - ret = intel_atomic_check_bigjoiner(state, crtc); + for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { + if (!intel_crtc_needs_modeset(new_crtc_state)) + continue; + + if (drm_WARN_ON(>drm, intel_crtc_is_bigjoiner_slave(new_crtc_state))) + continue; + + if (!new_crtc_state->hw.enable) + continue; + + ret = intel_modeset_pipe_config_late(state, crtc); if (ret) - break; + goto fail; } +fail: if (ret) *failed_pipe = crtc->pipe; @@ -6373,16 +6381,26 @@ int intel_atomic_check(struct drm_device *dev, if (ret) goto fail; + for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { + if (!intel_crtc_needs_modeset(new_crtc_state)) + continue; + + if (intel_crtc_is_bigjoiner_slave(new_crtc_state)) { + drm_WARN_ON(_priv->drm, new_crtc_state->uapi.enable); + continue; + } + + ret = intel_atomic_check_bigjoiner(state, crtc); + if (ret) + goto fail; + } + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { if (!intel_crtc_needs_modeset(new_crtc_state)) continue; - if (new_crtc_state->hw.enable) { - ret = intel_modeset_pipe_config_late(state, crtc); - if (ret) - goto fail; - } + intel_bigjoiner_adjust_pipe_src(new_crtc_state); intel_crtc_check_fastset(old_crtc_state, new_crtc_state); } -- 2.43.2
[PATCH 1/2] drm/i915: Disable port sync when bigjoiner is used
From: Ville Syrjälä The current modeset sequence can't handle port sync and bigjoiner at the same time. Refuse port sync when bigjoiner is needed, at least until we fix the modeset sequence. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_ddi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index a3d3d4942eb1..fa6fe9ec8027 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -4244,6 +4244,7 @@ static bool crtcs_port_sync_compatible(const struct intel_crtc_state *crtc_state const struct intel_crtc_state *crtc_state2) { return crtc_state1->hw.active && crtc_state2->hw.active && + !crtc_state1->bigjoiner_pipes && !crtc_state2->bigjoiner_pipes && crtc_state1->output_types == crtc_state2->output_types && crtc_state1->output_format == crtc_state2->output_format && crtc_state1->lane_count == crtc_state2->lane_count && -- 2.43.2
[PATCH 0/2] drm/i915: A few bigjoiner fixes
From: Ville Syrjälä Refuse bigjoiner+port sync as it's completely broken, and also fix a potential state mismatch once we do get MST/port sync + bigjoiner support. Ville Syrjälä (2): drm/i915: Disable port sync when bigjoiner is used drm/i915: Fix intel_modeset_pipe_config_late() for bigjoiner drivers/gpu/drm/i915/display/intel_ddi.c | 1 + drivers/gpu/drm/i915/display/intel_display.c | 46 ++-- 2 files changed, 33 insertions(+), 14 deletions(-) -- 2.43.2
[PATCH] drm/i915/gt: Limit the reserved VM space to only the platforms that need it
Commit 9bb66c179f50 ("drm/i915: Reserve some kernel space per vm") reduces the available VM space of one page in order to apply Wa_16018031267 and Wa_16018063123. This page was reserved indiscrimitely in all platforms even when not needed. Limit it to DG2 onwards. Fixes: 9bb66c179f50 ("drm/i915: Reserve some kernel space per vm") Signed-off-by: Andi Shyti Cc: Andrzej Hajda Cc: Chris Wilson Cc: Jonathan Cavitt Cc: Nirmoy Das --- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 3 +++ drivers/gpu/drm/i915/gt/intel_gt.c | 6 ++ drivers/gpu/drm/i915/gt/intel_gt.h | 9 + 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index 1bd0e041e15c..398d60a66410 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -961,6 +961,9 @@ static int gen8_init_rsvd(struct i915_address_space *vm) struct i915_vma *vma; int ret; + if (!intel_gt_needs_wa_16018031267(vm->gt)) + return 0; + /* The memory will be used only by GPU. */ obj = i915_gem_object_create_lmem(i915, PAGE_SIZE, I915_BO_ALLOC_VOLATILE | diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 2c6d31b8fc1a..580b5141ce1e 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -1024,6 +1024,12 @@ enum i915_map_type intel_gt_coherent_map_type(struct intel_gt *gt, return I915_MAP_WC; } +bool intel_gt_needs_wa_16018031267(struct intel_gt *gt) +{ + /* Wa_16018031267, Wa_16018063123 */ + return IS_GFX_GT_IP_RANGE(gt, IP_VER(12, 55), IP_VER(12, 71)); +} + bool intel_gt_needs_wa_22016122933(struct intel_gt *gt) { return MEDIA_VER_FULL(gt->i915) == IP_VER(13, 0) && gt->type == GT_MEDIA; diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h index 6e7cab60834c..b5e114d284ad 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.h +++ b/drivers/gpu/drm/i915/gt/intel_gt.h @@ -82,17 +82,18 @@ struct drm_printer; ##__VA_ARGS__); \ } while (0) -#define NEEDS_FASTCOLOR_BLT_WABB(engine) ( \ - IS_GFX_GT_IP_RANGE(engine->gt, IP_VER(12, 55), IP_VER(12, 71)) && \ - engine->class == COPY_ENGINE_CLASS && engine->instance == 0) - static inline bool gt_is_root(struct intel_gt *gt) { return !gt->info.id; } +bool intel_gt_needs_wa_16018031267(struct intel_gt *gt); bool intel_gt_needs_wa_22016122933(struct intel_gt *gt); +#define NEEDS_FASTCOLOR_BLT_WABB(engine) ( \ + intel_gt_needs_wa_16018031267(engine->gt) && \ + engine->class == COPY_ENGINE_CLASS && engine->instance == 0) + static inline struct intel_gt *uc_to_gt(struct intel_uc *uc) { return container_of(uc, struct intel_gt, uc); -- 2.43.0
Re: [PATCH] drm/i915/hwmon: Remove i915_hwmon_unregister() during driver unbind
On Tue, 26 Mar 2024 05:48:38 -0700, Badal Nilawar wrote: > Hi Badal, > i915_hwmon and its resources are managed resources of i915 dev. > During i915 driver unregister flow the function i915_hwmon_unregister() > explicitly makes i915_hwmon resource NULL. This happen before > hwmon is actually unregistered. Doing so may cause UAF if hwmon > sysfs is being accessed: I don't agree with this patch. For even if we remove setting to NULL, it doesn't explain what we see, which is that the devm_kzalloc'd i915->hwmon has been *freed* (since ddat (i915->hwmon->ddat) is 0x6b6b6b6b6b6b6dbf): (gdb) list *hwm_energy+0x2b 0x161f8b is in hwm_energy (drivers/gpu/drm/i915/i915_hwmon.c:134). 129 struct hwm_energy_info *ei = >ei; 130 intel_wakeref_t wakeref; 131 i915_reg_t rgaddr; 132 u32 reg_val; 133 134 if (ddat->gt_n >= 0) 135 rgaddr = hwmon->rg.energy_status_tile; 136 else 137 rgaddr = hwmon->rg.energy_status_all; 138 If we don't have i915_hwmon_unregister equivalent in xe, that's because xe has not implemented equivalents of i915_hwmon_power_max_disable and i915_hwmon_power_max_restore, where the check is used. Also, we should verify any fix before sending a patch out, either via local repro (say use a script to read hwmon energy file while running the selftests) or CI. Though in this case CI doesn't show any failures but I am wondering if that is only by chance. Fwiu, the issue is somehow being caused by ddat being *freed* before hwmon sysfs has disappeared, at module unload time (since selftests use module unload), allowing prometheus-node process to call into the energy read sysfs and cause the crash. Thanks. -- Ashutosh > > <7> [518.386591] i915 :03:00.0: [drm] intel_gt_set_wedged called from > intel_gt_set_wedged_on_fini+0xd/0x30 [i915] > <7> [518.471128] i915 :03:00.0: [drm:drm_client_release] drm_fb_helper > <4> [518.501476] general protection fault, probably for non-canonical address > 0x6b6b6b6b6b6b6dbf: [#1] PREEMPT SMP NOPTI > <4> [518.512264] CPU: 6 PID: 679 Comm: prometheus-node Tainted: G U > 6.9.0-rc1-CI_DRM_14482-g4a8fabcf2f1a+ #1 > <4> [518.522951] Hardware name: Intel Corporation Raptor Lake Client > Platform/RPL-S ADP-S DDR5 UDIMM CRB, BIOS RPLSFWI1.R00.4221.A00.2305271351 > 05/27/2023 > <4> [518.536217] RIP: 0010:hwm_energy+0x2b/0x100 [i915] > <4> [518.541159] Code: 48 89 e5 41 57 41 56 41 55 41 54 53 48 89 fb 48 83 e4 > f0 48 83 ec 10 4c 8b 77 08 4c 8b 2f 8b 7f 34 48 89 74 24 08 85 ff 78 2b <45> > 8b bd 54 02 00 00 49 8b 7e 18 e8 35 e4 ea ff 49 89 c4 48 85 c0 > <4> [518.559746] RSP: 0018:c900077efd00 EFLAGS: 00010202 > <4> [518.564943] RAX: RBX: 8881e3078428 RCX: > > <4> [518.572025] RDX: 0001 RSI: c900077efda0 RDI: > 6b6b6b6b > <4> [518.579107] RBP: c900077efd40 R08: c900077efda0 R09: > 0001 > <4> [518.586186] R10: 0001 R11: 88810c19aa00 R12: > 888243e1a010 > <4> [518.593264] R13: 6b6b6b6b6b6b6b6b R14: 6b6b6b6b6b6b6b6b R15: > 8881e3078428 > <4> [518.600343] FS: 7f9def400700() GS:8d10() > knlGS: > <4> [518.608373] CS: 0010 DS: ES: CR0: 80050033 > <4> [518.614077] CR2: 564f19bff288 CR3: 000119f94000 CR4: > 00f50ef0 > <4> [518.621158] PKRU: 5554 > <4> [518.623858] Call Trace: > <4> [518.626303] > <4> [518.628400] ? __die_body+0x1a/0x60 > <4> [518.631881] ? die_addr+0x38/0x60 > <4> [518.635182] ? exc_general_protection+0x1a1/0x400 > <4> [518.639862] ? asm_exc_general_protection+0x26/0x30 > <4> [518.644710] ? hwm_energy+0x2b/0x100 [i915] > <4> [518.649007] hwm_read+0x9a/0x310 [i915] > <4> [518.652953] hwmon_attr_show+0x36/0x140 > <4> [518.656775] dev_attr_show+0x15/0x60 > > Fixes: b3b088e28183 ("drm/i915/hwmon: Add HWMON infrastructure") > Closes: https://gitlab.freedesktop.org/drm/intel/issues/10366 > Cc: Ashutosh Dixit > Cc: Janusz Krzysztofik > Signed-off-by: Badal Nilawar > --- > drivers/gpu/drm/i915/i915_driver.c | 2 -- > drivers/gpu/drm/i915/i915_hwmon.c | 5 - > 2 files changed, 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_driver.c > b/drivers/gpu/drm/i915/i915_driver.c > index 4b9233c07a22..a95b455964b7 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -660,8 +660,6 @@ static void i915_driver_unregister(struct > drm_i915_private *dev_priv) > for_each_gt(gt, dev_priv, i) > intel_gt_driver_unregister(gt); > > - i915_hwmon_unregister(dev_priv); > - > i915_perf_unregister(dev_priv); > i915_pmu_unregister(dev_priv); > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c > b/drivers/gpu/drm/i915/i915_hwmon.c > index c9169e56b9a1..91f171752d34
[PATCH 13/13] drm/i915: Optimize out redundant dbuf slice updates
From: Ville Syrjälä if the new dbuf slices are a superset of the old dbuf slices then we don't have to do anything in intel_dbuf_post_plane_update(). Restructure the code to skip such redundant dbuf slice updates. The main benefit is slightly less confusing logs. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/skl_watermark.c | 27 +--- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 1b48009efe2b..50ec51065118 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3788,16 +3788,20 @@ void intel_dbuf_pre_plane_update(struct intel_atomic_state *state) intel_atomic_get_new_dbuf_state(state); const struct intel_dbuf_state *old_dbuf_state = intel_atomic_get_old_dbuf_state(state); + u8 old_slices, new_slices; - if (!new_dbuf_state || - new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices) + if (!new_dbuf_state) + return; + + old_slices = old_dbuf_state->enabled_slices; + new_slices = old_dbuf_state->enabled_slices | new_dbuf_state->enabled_slices; + + if (old_slices == new_slices) return; WARN_ON(!new_dbuf_state->base.changed); - gen9_dbuf_slices_update(i915, - old_dbuf_state->enabled_slices | - new_dbuf_state->enabled_slices); + gen9_dbuf_slices_update(i915, new_slices); } void intel_dbuf_post_plane_update(struct intel_atomic_state *state) @@ -3807,15 +3811,20 @@ void intel_dbuf_post_plane_update(struct intel_atomic_state *state) intel_atomic_get_new_dbuf_state(state); const struct intel_dbuf_state *old_dbuf_state = intel_atomic_get_old_dbuf_state(state); + u8 old_slices, new_slices; - if (!new_dbuf_state || - new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices) + if (!new_dbuf_state) + return; + + old_slices = old_dbuf_state->enabled_slices | new_dbuf_state->enabled_slices; + new_slices = new_dbuf_state->enabled_slices; + + if (old_slices == new_slices) return; WARN_ON(!new_dbuf_state->base.changed); - gen9_dbuf_slices_update(i915, - new_dbuf_state->enabled_slices); + gen9_dbuf_slices_update(i915, new_slices); } static int skl_watermark_ipc_status_show(struct seq_file *m, void *data) -- 2.43.2
[PATCH 12/13] drm/i915: Use a plain old int for the cdclk/mdclk ratio
From: Ville Syrjälä No point in throwing around u8 when we're dealing with just an integer. Use a plain old boring 'int'. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 6 +++--- drivers/gpu/drm/i915/display/intel_cdclk.h | 4 ++-- drivers/gpu/drm/i915/display/skl_watermark.c | 6 -- drivers/gpu/drm/i915/display/skl_watermark.h | 6 -- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 66c161d7b485..5cba0d08189b 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1893,8 +1893,8 @@ static u32 xe2lpd_mdclk_source_sel(struct drm_i915_private *i915) return MDCLK_SOURCE_SEL_CD2XCLK; } -u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, - const struct intel_cdclk_config *cdclk_config) +int intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, + const struct intel_cdclk_config *cdclk_config) { if (mdclk_source_is_cdclk_pll(i915)) return DIV_ROUND_UP(cdclk_config->vco, cdclk_config->cdclk); @@ -3321,7 +3321,7 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state) if (intel_mdclk_cdclk_ratio(dev_priv, _cdclk_state->actual) != intel_mdclk_cdclk_ratio(dev_priv, _cdclk_state->actual)) { - u8 ratio = intel_mdclk_cdclk_ratio(dev_priv, _cdclk_state->actual); + int ratio = intel_mdclk_cdclk_ratio(dev_priv, _cdclk_state->actual); ret = intel_dbuf_state_set_mdclk_cdclk_ratio(state, ratio); if (ret) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h index 5d4faf401774..cfdcdec07a4d 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.h +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h @@ -67,8 +67,8 @@ void intel_update_cdclk(struct drm_i915_private *dev_priv); u32 intel_read_rawclk(struct drm_i915_private *dev_priv); bool intel_cdclk_clock_changed(const struct intel_cdclk_config *a, const struct intel_cdclk_config *b); -u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, - const struct intel_cdclk_config *cdclk_config); +int intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, + const struct intel_cdclk_config *cdclk_config); bool intel_cdclk_is_decreasing_later(struct intel_atomic_state *state); void intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state); void intel_set_cdclk_post_plane_update(struct intel_atomic_state *state); diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index ca0f1f89e6d9..1b48009efe2b 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3616,7 +3616,8 @@ static void intel_mbus_dbox_update(struct intel_atomic_state *state) } } -int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio) +int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, + int ratio) { struct intel_dbuf_state *dbuf_state; @@ -3629,7 +3630,8 @@ int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 return intel_atomic_lock_global_state(_state->base); } -void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio, bool joined_mbus) +void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, +int ratio, bool joined_mbus) { enum dbuf_slice slice; diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h index 3323a1d973f9..ef1a008466be 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.h +++ b/drivers/gpu/drm/i915/display/skl_watermark.h @@ -74,11 +74,13 @@ intel_atomic_get_dbuf_state(struct intel_atomic_state *state); to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state, _i915(state->base.dev)->display.dbuf.obj)) int intel_dbuf_init(struct drm_i915_private *i915); -int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio); +int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, + int ratio); void intel_dbuf_pre_plane_update(struct intel_atomic_state *state); void intel_dbuf_post_plane_update(struct intel_atomic_state *state); -void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio, bool joined_mbus); +void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, +int ratio, bool joined_mbus); void intel_dbuf_mbus_pre_ddb_update(struct intel_atomic_state *state); void
[PATCH 11/13] drm/i915: Implement vblank synchronized MBUS join changes
From: Stanislav Lisovskiy Currently we can't change MBUS join status without doing a modeset, because we are lacking mechanism to synchronize those with vblank. However then this means that we can't do a fastset, if there is a need to change MBUS join state. Fix that by implementing such change. We already call correspondent check and update at pre_plane dbuf update, so the only thing left is to have a non-modeset version of that. If active pipes stay the same then fastset is possible and only MBUS join state/ddb allocation updates would be committed. The full mbus/cdclk sequence will look as follows: 1. disable pipes 2. increase cdclk if necessary 2.1 reprogram cdclk 2.2 update dbuf tracker value 3. enable mbus joining if necessary 3.1 update mbus_ctl 3.2 update dbuf tracker value 4. reallocate dbuf for planes on active pipes 5. disable mbus joining if necessary 5.1 update dbuf tracker value 5.2 update mbus_ctl 6. enable pipes 7. decrease cdclk if necessary 7.1 update dbuf tracker value 7.2 reprogram cdclk And in order to keep things in sync we need: Step 2: - mbus_join == old - mdclk/cdclk ratio == new Step 3: - mbus_join == new - mdclk/cdclk ratio == old when cdclk is changing in step 7 - mdclk/cdclk ratio == new when cdclk is changing in step 2 Step 5: - mbus_join == new - mdclk/cdclk ratio == old when cdclk is changing in step 7 - mdclk/cdclk ratio == new when cdclk is changing in step 2 Step 7: - mbus_join == new - mdclk/cdclk ratio == new v2: - Removed redundant parentheses(Ville Syrjälä) - Constified new_crtc_state in intel_mbus_joined_pipe(Ville Syrjälä) - Removed pipe_select variable(Ville Syrjälä) [v3: vsyrjala: Correctly sequence vs. cdclk updates, properly describe the full sequence, shuffle code around to make the diff more legible, streamline a few things] Signed-off-by: Stanislav Lisovskiy Co-developed-by: Ville Syrjälä Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 11 ++ drivers/gpu/drm/i915/display/intel_cdclk.h | 1 + drivers/gpu/drm/i915/display/intel_display.c | 5 +- drivers/gpu/drm/i915/display/skl_watermark.c | 141 --- drivers/gpu/drm/i915/display/skl_watermark.h | 3 +- 5 files changed, 112 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 4024118a7ffb..66c161d7b485 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2576,6 +2576,17 @@ static void intel_cdclk_pcode_post_notify(struct intel_atomic_state *state) update_cdclk, update_pipe_count); } +bool intel_cdclk_is_decreasing_later(struct intel_atomic_state *state) +{ + const struct intel_cdclk_state *old_cdclk_state = + intel_atomic_get_old_cdclk_state(state); + const struct intel_cdclk_state *new_cdclk_state = + intel_atomic_get_new_cdclk_state(state); + + return new_cdclk_state && !new_cdclk_state->disable_pipes && + new_cdclk_state->actual.cdclk < old_cdclk_state->actual.cdclk; +} + /** * intel_set_cdclk_pre_plane_update - Push the CDCLK state to the hardware * @state: intel atomic state diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h index 2843fc091086..5d4faf401774 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.h +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h @@ -69,6 +69,7 @@ bool intel_cdclk_clock_changed(const struct intel_cdclk_config *a, const struct intel_cdclk_config *b); u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915, const struct intel_cdclk_config *cdclk_config); +bool intel_cdclk_is_decreasing_later(struct intel_atomic_state *state); void intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state); void intel_set_cdclk_post_plane_update(struct intel_atomic_state *state); void intel_cdclk_dump_config(struct drm_i915_private *i915, diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 4d6668a5f1ab..023cf4a77e6f 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -6915,6 +6915,8 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state) intel_pre_update_crtc(state, crtc); } + intel_dbuf_mbus_pre_ddb_update(state); + while (update_pipes) { for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { @@ -6945,6 +6947,8 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state) } } + intel_dbuf_mbus_post_ddb_update(state); + update_pipes = modeset_pipes; /* @@ -7191,7 +7195,6 @@
[PATCH 10/13] drm/i915: Use old mbus_join value when increasing CDCLK
From: Stanislav Lisovskiy In order to make sure we are not breaking the proper sequence lets to updates step by step and don't change MBUS join value during MDCLK/CDCLK programming stage. MBUS join programming would be taken care by pre/post ddb hooks. v2: - Reworded comment about using old mbus_join value in intel_set_cdclk(Ville Syrjälä) Signed-off-by: Stanislav Lisovskiy [v3: vsyrjala: rebase on top of cdclk changes, reword a bit more] Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 98546f384023..4024118a7ffb 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2612,6 +2612,12 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state) old_cdclk_state->actual.voltage_level); } + /* +* mbus joining will be changed later by +* intel_dbuf_mbus_{pre,post}_ddb_update() +*/ + cdclk_config.joined_mbus = old_cdclk_state->actual.joined_mbus; + drm_WARN_ON(>drm, !new_cdclk_state->base.changed); intel_set_cdclk(i915, _config, new_cdclk_state->pipe, -- 2.43.2
[PATCH 08/13] drm/i915: Extract intel_dbuf_mdclk_min_tracker_update()
From: Ville Syrjälä Extact the stuff that writes the dbuf/mbus ration stuff into its own function. Will help with correctly sequencing the operations done during mbus programming. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/skl_watermark.c | 43 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index f7e03078bd43..7767c5eada36 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3653,6 +3653,30 @@ void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio DBUF_MIN_TRACKER_STATE_SERVICE(ratio - 1)); } +static void intel_dbuf_mdclk_min_tracker_update(struct intel_atomic_state *state) +{ + struct drm_i915_private *i915 = to_i915(state->base.dev); + const struct intel_dbuf_state *old_dbuf_state = + intel_atomic_get_old_dbuf_state(state); + const struct intel_dbuf_state *new_dbuf_state = + intel_atomic_get_new_dbuf_state(state); + + if (DISPLAY_VER(i915) >= 20 && + old_dbuf_state->mdclk_cdclk_ratio != new_dbuf_state->mdclk_cdclk_ratio) { + /* +* For Xe2LPD and beyond, when there is a change in the ratio +* between MDCLK and CDCLK, updates to related registers need to +* happen at a specific point in the CDCLK change sequence. In +* that case, we defer to the call to +* intel_dbuf_mdclk_cdclk_ratio_update() to the CDCLK logic. +*/ + return; + } + + intel_dbuf_mdclk_cdclk_ratio_update(i915, new_dbuf_state->mdclk_cdclk_ratio, + new_dbuf_state->joined_mbus); +} + static void intel_dbuf_mbus_join_update(struct intel_atomic_state *state) { struct drm_i915_private *i915 = to_i915(state->base.dev); @@ -3683,10 +3707,6 @@ static void intel_dbuf_mbus_join_update(struct intel_atomic_state *state) static void update_mbus_pre_enable(struct intel_atomic_state *state) { struct drm_i915_private *i915 = to_i915(state->base.dev); - const struct intel_dbuf_state *old_dbuf_state = - intel_atomic_get_old_dbuf_state(state); - const struct intel_dbuf_state *new_dbuf_state = - intel_atomic_get_new_dbuf_state(state); if (!HAS_MBUS_JOINING(i915)) return; @@ -3697,20 +3717,7 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state) */ intel_dbuf_mbus_join_update(state); - if (DISPLAY_VER(i915) >= 20 && - old_dbuf_state->mdclk_cdclk_ratio != new_dbuf_state->mdclk_cdclk_ratio) { - /* -* For Xe2LPD and beyond, when there is a change in the ratio -* between MDCLK and CDCLK, updates to related registers need to -* happen at a specific point in the CDCLK change sequence. In -* that case, we defer to the call to -* intel_dbuf_mdclk_cdclk_ratio_update() to the CDCLK logic. -*/ - return; - } - - intel_dbuf_mdclk_cdclk_ratio_update(i915, new_dbuf_state->mdclk_cdclk_ratio, - new_dbuf_state->joined_mbus); + intel_dbuf_mdclk_min_tracker_update(state); } void intel_dbuf_pre_plane_update(struct intel_atomic_state *state) -- 2.43.2
[PATCH 09/13] drm/i915: Add debugs for mbus joining and dbuf ratio programming
From: Ville Syrjälä Add some debugs so that we can actually observe what is actually happening during the mbus/dbuf programming steps. We can just shove them into fairly low level functions as none of them are called during any critical sections/etc. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/skl_watermark.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 7767c5eada36..a118ecf9e532 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3647,6 +3647,9 @@ void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio if (joined_mbus) ratio *= 2; + drm_dbg_kms(>drm, "Updating dbuf ratio to %d (mbus joined: %s)\n", + ratio, str_yes_no(joined_mbus)); + for_each_dbuf_slice(i915, slice) intel_de_rmw(i915, DBUF_CTL_S(slice), DBUF_MIN_TRACKER_STATE_SERVICE_MASK, @@ -3680,10 +3683,16 @@ static void intel_dbuf_mdclk_min_tracker_update(struct intel_atomic_state *state static void intel_dbuf_mbus_join_update(struct intel_atomic_state *state) { struct drm_i915_private *i915 = to_i915(state->base.dev); + const struct intel_dbuf_state *old_dbuf_state = + intel_atomic_get_old_dbuf_state(state); const struct intel_dbuf_state *new_dbuf_state = intel_atomic_get_new_dbuf_state(state); u32 mbus_ctl; + drm_dbg_kms(>drm, "Changing mbus joined: %s -> %s\n", + str_yes_no(old_dbuf_state->joined_mbus), + str_yes_no(new_dbuf_state->joined_mbus)); + /* * TODO: Implement vblank synchronized MBUS joining changes. * Must be properly coordinated with dbuf reprogramming. -- 2.43.2
[PATCH 07/13] drm/i915: Extract intel_dbuf_mbus_join_update()
From: Ville Syrjälä Extact the stuff that writes the joining bits in MBUS_CTL into its own function. Will help with correctly sequencing the operations done during mbus programming. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/skl_watermark.c | 37 +--- 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 6bd3fec0aa56..f7e03078bd43 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3653,21 +3653,12 @@ void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio DBUF_MIN_TRACKER_STATE_SERVICE(ratio - 1)); } -/* - * Configure MBUS_CTL and all DBUF_CTL_S of each slice to join_mbus state before - * update the request state of all DBUS slices. - */ -static void update_mbus_pre_enable(struct intel_atomic_state *state) +static void intel_dbuf_mbus_join_update(struct intel_atomic_state *state) { struct drm_i915_private *i915 = to_i915(state->base.dev); + const struct intel_dbuf_state *new_dbuf_state = + intel_atomic_get_new_dbuf_state(state); u32 mbus_ctl; - const struct intel_dbuf_state *old_dbuf_state = - intel_atomic_get_old_dbuf_state(state); - const struct intel_dbuf_state *new_dbuf_state = - intel_atomic_get_new_dbuf_state(state); - - if (!HAS_MBUS_JOINING(i915)) - return; /* * TODO: Implement vblank synchronized MBUS joining changes. @@ -3683,6 +3674,28 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state) intel_de_rmw(i915, MBUS_CTL, MBUS_HASHING_MODE_MASK | MBUS_JOIN | MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl); +} + +/* + * Configure MBUS_CTL and all DBUF_CTL_S of each slice to join_mbus state before + * update the request state of all DBUS slices. + */ +static void update_mbus_pre_enable(struct intel_atomic_state *state) +{ + struct drm_i915_private *i915 = to_i915(state->base.dev); + const struct intel_dbuf_state *old_dbuf_state = + intel_atomic_get_old_dbuf_state(state); + const struct intel_dbuf_state *new_dbuf_state = + intel_atomic_get_new_dbuf_state(state); + + if (!HAS_MBUS_JOINING(i915)) + return; + + /* +* TODO: Implement vblank synchronized MBUS joining changes. +* Must be properly coordinated with dbuf reprogramming. +*/ + intel_dbuf_mbus_join_update(state); if (DISPLAY_VER(i915) >= 20 && old_dbuf_state->mdclk_cdclk_ratio != new_dbuf_state->mdclk_cdclk_ratio) { -- 2.43.2
[PATCH 06/13] drm/i915: Relocate intel_mbus_dbox_update()
From: Ville Syrjälä intel_mbus_dbox_update() will become static soon. Relocate it into a place that avoids having to add a forward declaration for it. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/skl_watermark.c | 166 +-- 1 file changed, 83 insertions(+), 83 deletions(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index f582992592c1..6bd3fec0aa56 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3540,6 +3540,89 @@ int intel_dbuf_init(struct drm_i915_private *i915) return 0; } +static bool xelpdp_is_only_pipe_per_dbuf_bank(enum pipe pipe, u8 active_pipes) +{ + switch (pipe) { + case PIPE_A: + return !(active_pipes & BIT(PIPE_D)); + case PIPE_D: + return !(active_pipes & BIT(PIPE_A)); + case PIPE_B: + return !(active_pipes & BIT(PIPE_C)); + case PIPE_C: + return !(active_pipes & BIT(PIPE_B)); + default: /* to suppress compiler warning */ + MISSING_CASE(pipe); + break; + } + + return false; +} + +void intel_mbus_dbox_update(struct intel_atomic_state *state) +{ + struct drm_i915_private *i915 = to_i915(state->base.dev); + const struct intel_dbuf_state *new_dbuf_state, *old_dbuf_state; + const struct intel_crtc *crtc; + u32 val = 0; + + if (DISPLAY_VER(i915) < 11) + return; + + new_dbuf_state = intel_atomic_get_new_dbuf_state(state); + old_dbuf_state = intel_atomic_get_old_dbuf_state(state); + if (!new_dbuf_state || + (new_dbuf_state->joined_mbus == old_dbuf_state->joined_mbus && +new_dbuf_state->active_pipes == old_dbuf_state->active_pipes)) + return; + + if (DISPLAY_VER(i915) >= 14) + val |= MBUS_DBOX_I_CREDIT(2); + + if (DISPLAY_VER(i915) >= 12) { + val |= MBUS_DBOX_B2B_TRANSACTIONS_MAX(16); + val |= MBUS_DBOX_B2B_TRANSACTIONS_DELAY(1); + val |= MBUS_DBOX_REGULATE_B2B_TRANSACTIONS_EN; + } + + if (DISPLAY_VER(i915) >= 14) + val |= new_dbuf_state->joined_mbus ? MBUS_DBOX_A_CREDIT(12) : +MBUS_DBOX_A_CREDIT(8); + else if (IS_ALDERLAKE_P(i915)) + /* Wa_22010947358:adl-p */ + val |= new_dbuf_state->joined_mbus ? MBUS_DBOX_A_CREDIT(6) : +MBUS_DBOX_A_CREDIT(4); + else + val |= MBUS_DBOX_A_CREDIT(2); + + if (DISPLAY_VER(i915) >= 14) { + val |= MBUS_DBOX_B_CREDIT(0xA); + } else if (IS_ALDERLAKE_P(i915)) { + val |= MBUS_DBOX_BW_CREDIT(2); + val |= MBUS_DBOX_B_CREDIT(8); + } else if (DISPLAY_VER(i915) >= 12) { + val |= MBUS_DBOX_BW_CREDIT(2); + val |= MBUS_DBOX_B_CREDIT(12); + } else { + val |= MBUS_DBOX_BW_CREDIT(1); + val |= MBUS_DBOX_B_CREDIT(8); + } + + for_each_intel_crtc_in_pipe_mask(>drm, crtc, new_dbuf_state->active_pipes) { + u32 pipe_val = val; + + if (DISPLAY_VER(i915) >= 14) { + if (xelpdp_is_only_pipe_per_dbuf_bank(crtc->pipe, + new_dbuf_state->active_pipes)) + pipe_val |= MBUS_DBOX_BW_8CREDITS_MTL; + else + pipe_val |= MBUS_DBOX_BW_4CREDITS_MTL; + } + + intel_de_write(i915, PIPE_MBUS_DBOX_CTL(crtc->pipe), pipe_val); + } +} + int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio) { struct intel_dbuf_state *dbuf_state; @@ -3657,89 +3740,6 @@ void intel_dbuf_post_plane_update(struct intel_atomic_state *state) new_dbuf_state->enabled_slices); } -static bool xelpdp_is_only_pipe_per_dbuf_bank(enum pipe pipe, u8 active_pipes) -{ - switch (pipe) { - case PIPE_A: - return !(active_pipes & BIT(PIPE_D)); - case PIPE_D: - return !(active_pipes & BIT(PIPE_A)); - case PIPE_B: - return !(active_pipes & BIT(PIPE_C)); - case PIPE_C: - return !(active_pipes & BIT(PIPE_B)); - default: /* to suppress compiler warning */ - MISSING_CASE(pipe); - break; - } - - return false; -} - -void intel_mbus_dbox_update(struct intel_atomic_state *state) -{ - struct drm_i915_private *i915 = to_i915(state->base.dev); - const struct intel_dbuf_state *new_dbuf_state, *old_dbuf_state; - const struct intel_crtc *crtc; - u32 val = 0; - - if (DISPLAY_VER(i915) < 11) - return; - -
[PATCH 05/13] drm/i915: Loop over all active pipes in intel_mbus_dbox_update
From: Stanislav Lisovskiy We need to loop through all active pipes, not just the ones, that are in current state, because disabling and enabling even a particular pipe affects credits in another one. Signed-off-by: Stanislav Lisovskiy Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/skl_watermark.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index bc341abcab2f..f582992592c1 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -3680,10 +3680,8 @@ void intel_mbus_dbox_update(struct intel_atomic_state *state) { struct drm_i915_private *i915 = to_i915(state->base.dev); const struct intel_dbuf_state *new_dbuf_state, *old_dbuf_state; - const struct intel_crtc_state *new_crtc_state; const struct intel_crtc *crtc; u32 val = 0; - int i; if (DISPLAY_VER(i915) < 11) return; @@ -3727,12 +3725,9 @@ void intel_mbus_dbox_update(struct intel_atomic_state *state) val |= MBUS_DBOX_B_CREDIT(8); } - for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { + for_each_intel_crtc_in_pipe_mask(>drm, crtc, new_dbuf_state->active_pipes) { u32 pipe_val = val; - if (!new_crtc_state->hw.active) - continue; - if (DISPLAY_VER(i915) >= 14) { if (xelpdp_is_only_pipe_per_dbuf_bank(crtc->pipe, new_dbuf_state->active_pipes)) -- 2.43.2
[PATCH 04/13] drm/i915/cdclk: Indicate whether CDCLK change happens during pre or post plane update
From: Ville Syrjälä Currently we just get a plain "Changing CDCLK to ..." in the logs. It would actually be interesting to see whether we're doing the programming during the pre or post plane phase of the commit. Include that information in the debug message. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 19 ++- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 99d2657f29a7..98546f384023 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2434,18 +2434,9 @@ static void intel_pcode_notify(struct drm_i915_private *i915, ret); } -/** - * intel_set_cdclk - Push the CDCLK configuration to the hardware - * @dev_priv: i915 device - * @cdclk_config: new CDCLK configuration - * @pipe: pipe with which to synchronize the update - * - * Program the hardware based on the passed in CDCLK state, - * if necessary. - */ static void intel_set_cdclk(struct drm_i915_private *dev_priv, const struct intel_cdclk_config *cdclk_config, - enum pipe pipe) + enum pipe pipe, const char *context) { struct intel_encoder *encoder; @@ -2455,7 +2446,7 @@ static void intel_set_cdclk(struct drm_i915_private *dev_priv, if (drm_WARN_ON_ONCE(_priv->drm, !dev_priv->display.funcs.cdclk->set_cdclk)) return; - intel_cdclk_dump_config(dev_priv, cdclk_config, "Changing CDCLK to"); + intel_cdclk_dump_config(dev_priv, cdclk_config, context); for_each_intel_encoder_with_psr(_priv->drm, encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); @@ -2623,7 +2614,8 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state) drm_WARN_ON(>drm, !new_cdclk_state->base.changed); - intel_set_cdclk(i915, _config, new_cdclk_state->pipe); + intel_set_cdclk(i915, _config, new_cdclk_state->pipe, + "Pre changing CDCLK to"); } /** @@ -2651,7 +2643,8 @@ intel_set_cdclk_post_plane_update(struct intel_atomic_state *state) drm_WARN_ON(>drm, !new_cdclk_state->base.changed); - intel_set_cdclk(i915, _cdclk_state->actual, new_cdclk_state->pipe); + intel_set_cdclk(i915, _cdclk_state->actual, new_cdclk_state->pipe, + "Post changing CDCLK to"); } static int intel_pixel_rate_to_cdclk(const struct intel_crtc_state *crtc_state) -- 2.43.2
[PATCH 02/13] drm/i915/cdclk: Fix voltage_level programming edge case
From: Ville Syrjälä Currently we only consider the relationship of the old and new CDCLK frequencies when determining whether to do the repgramming from intel_set_cdclk_pre_plane_update() or intel_set_cdclk_post_plane_update(). It is technically possible to have a situation where the CDCLK frequency is decreasing, but the voltage_level is increasing due a DDI port. In this case we should bump the voltage level already in intel_set_cdclk_pre_plane_update() (so that the voltage_level will have been increased by the time the port gets enabled), while leaving the CDCLK frequency unchanged (as active planes/etc. may still depend on it). We can then reduce the CDCLK frequency to its final value from intel_set_cdclk_post_plane_update(). In order to handle that correctly we shall construct a suitable amalgam of the old and new cdclk states in intel_set_cdclk_pre_plane_update(). And we can simply call intel_set_cdclk() unconditionally in both places as it will not do anything if nothing actually changes vs. the current hw state. v2: Handle cdclk_state->disable_pipes Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 27 +- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 619529dba095..504c5cbbcfff 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2600,6 +2600,7 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state) intel_atomic_get_old_cdclk_state(state); const struct intel_cdclk_state *new_cdclk_state = intel_atomic_get_new_cdclk_state(state); + struct intel_cdclk_config cdclk_config; if (!intel_cdclk_changed(_cdclk_state->actual, _cdclk_state->actual)) @@ -2608,13 +2609,21 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state) if (IS_DG2(i915)) intel_cdclk_pcode_pre_notify(state); - if (new_cdclk_state->disable_pipes || - old_cdclk_state->actual.cdclk <= new_cdclk_state->actual.cdclk) { - drm_WARN_ON(>drm, !new_cdclk_state->base.changed); + if (new_cdclk_state->disable_pipes) { + cdclk_config = new_cdclk_state->actual; + } else { + if (new_cdclk_state->actual.cdclk >= old_cdclk_state->actual.cdclk) + cdclk_config = new_cdclk_state->actual; + else + cdclk_config = old_cdclk_state->actual; - intel_set_cdclk(i915, _cdclk_state->actual, - new_cdclk_state->pipe); + cdclk_config.voltage_level = max(new_cdclk_state->actual.voltage_level, + old_cdclk_state->actual.voltage_level); } + + drm_WARN_ON(>drm, !new_cdclk_state->base.changed); + + intel_set_cdclk(i915, _config, new_cdclk_state->pipe); } /** @@ -2640,13 +2649,9 @@ intel_set_cdclk_post_plane_update(struct intel_atomic_state *state) if (IS_DG2(i915)) intel_cdclk_pcode_post_notify(state); - if (!new_cdclk_state->disable_pipes && - old_cdclk_state->actual.cdclk > new_cdclk_state->actual.cdclk) { - drm_WARN_ON(>drm, !new_cdclk_state->base.changed); + drm_WARN_ON(>drm, !new_cdclk_state->base.changed); - intel_set_cdclk(i915, _cdclk_state->actual, - new_cdclk_state->pipe); - } + intel_set_cdclk(i915, _cdclk_state->actual, new_cdclk_state->pipe); } static int intel_pixel_rate_to_cdclk(const struct intel_crtc_state *crtc_state) -- 2.43.2
[PATCH 03/13] drm/i915/cdclk: Drop tgl/dg2 cdclk bump hacks
From: Ville Syrjälä No ever figured out why bumping the cdclk helped with whatever issue we were having at the time. Remove the hacks and start from scratch so that we can actually see if any problems still remain. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 19 --- 1 file changed, 19 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 504c5cbbcfff..99d2657f29a7 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2802,25 +2802,6 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) if (crtc_state->dsc.compression_enable) min_cdclk = max(min_cdclk, intel_vdsc_min_cdclk(crtc_state)); - /* -* HACK. Currently for TGL/DG2 platforms we calculate -* min_cdclk initially based on pixel_rate divided -* by 2, accounting for also plane requirements, -* however in some cases the lowest possible CDCLK -* doesn't work and causing the underruns. -* Explicitly stating here that this seems to be currently -* rather a Hack, than final solution. -*/ - if (IS_TIGERLAKE(dev_priv) || IS_DG2(dev_priv)) { - /* -* Clamp to max_cdclk_freq in case pixel rate is higher, -* in order not to break an 8K, but still leave W/A at place. -*/ - min_cdclk = max_t(int, min_cdclk, - min_t(int, crtc_state->pixel_rate, - dev_priv->display.cdclk.max_cdclk_freq)); - } - return min_cdclk; } -- 2.43.2
[PATCH 01/13] drm/i915/cdclk: Fix CDCLK programming order when pipes are active
From: Ville Syrjälä Currently we always reprogram CDCLK from the intel_set_cdclk_pre_plane_update() when using squahs/crawl. The code only works correctly for the cd2x update or full modeset cases, and it was simply never updated to deal with squash/crawl. If the CDCLK frequency is increasing we must reprogram it before we do anything else that might depend on the new higher frequency, and conversely we must not decrease the frequency until everything that might still depend on the old higher frequency has been dealt with. Since cdclk_state->pipe is only relevant when doing a cd2x update we can't use it to determine the correct sequence during squash/crawl. To that end introduce cdclk_state->disable_pipes which simply indicates that we must perform the update while the pipes are disable (ie. during intel_set_cdclk_pre_plane_update()). Otherwise we use the same old vs. new CDCLK frequency comparsiong as for cd2x updates. The only remaining problem case is when the voltage_level needs to increase due to a DDI port, but the CDCLK frequency is decreasing (and not all pipes are being disabled). The current approach will not bump the voltage level up until after the port has already been enabled, which is too late. But we'll take care of that case separately. v2: Don't break the "must disable pipes case" Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 15 +-- drivers/gpu/drm/i915/display/intel_cdclk.h | 3 +++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 31aaa9780dfc..619529dba095 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2600,7 +2600,6 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state) intel_atomic_get_old_cdclk_state(state); const struct intel_cdclk_state *new_cdclk_state = intel_atomic_get_new_cdclk_state(state); - enum pipe pipe = new_cdclk_state->pipe; if (!intel_cdclk_changed(_cdclk_state->actual, _cdclk_state->actual)) @@ -2609,11 +2608,12 @@ intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state) if (IS_DG2(i915)) intel_cdclk_pcode_pre_notify(state); - if (pipe == INVALID_PIPE || + if (new_cdclk_state->disable_pipes || old_cdclk_state->actual.cdclk <= new_cdclk_state->actual.cdclk) { drm_WARN_ON(>drm, !new_cdclk_state->base.changed); - intel_set_cdclk(i915, _cdclk_state->actual, pipe); + intel_set_cdclk(i915, _cdclk_state->actual, + new_cdclk_state->pipe); } } @@ -2632,7 +2632,6 @@ intel_set_cdclk_post_plane_update(struct intel_atomic_state *state) intel_atomic_get_old_cdclk_state(state); const struct intel_cdclk_state *new_cdclk_state = intel_atomic_get_new_cdclk_state(state); - enum pipe pipe = new_cdclk_state->pipe; if (!intel_cdclk_changed(_cdclk_state->actual, _cdclk_state->actual)) @@ -2641,11 +2640,12 @@ intel_set_cdclk_post_plane_update(struct intel_atomic_state *state) if (IS_DG2(i915)) intel_cdclk_pcode_post_notify(state); - if (pipe != INVALID_PIPE && + if (!new_cdclk_state->disable_pipes && old_cdclk_state->actual.cdclk > new_cdclk_state->actual.cdclk) { drm_WARN_ON(>drm, !new_cdclk_state->base.changed); - intel_set_cdclk(i915, _cdclk_state->actual, pipe); + intel_set_cdclk(i915, _cdclk_state->actual, + new_cdclk_state->pipe); } } @@ -3124,6 +3124,7 @@ static struct intel_global_state *intel_cdclk_duplicate_state(struct intel_globa return NULL; cdclk_state->pipe = INVALID_PIPE; + cdclk_state->disable_pipes = false; return _state->base; } @@ -3316,6 +3317,8 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state) if (ret) return ret; + new_cdclk_state->disable_pipes = true; + drm_dbg_kms(_priv->drm, "Modeset required for cdclk change\n"); } diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h index bc8f86e292d8..2843fc091086 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.h +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h @@ -53,6 +53,9 @@ struct intel_cdclk_state { /* bitmask of active pipes */ u8 active_pipes; + + /* update cdclk with pipes disabled */ + bool disable_pipes; }; int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state); -- 2.43.2
[PATCH 00/13] drm/i915: Implemnt vblank sycnhronized mbus joining changes
From: Ville Syrjälä Get rid of the full modeset requirement for changing mbus joining. Things got quite a bit more complicated than originally envisioned due to the dynamic cdclk/mdclk ratio. Sadly we have to do a fairly careful dance between the dbuf and cdclk code to make sure everything is programmed in the correct sequence. Stan did the grunt work, but the sequence vs. cdclk updates was still not right so I finished that part. I also reorganized the code quite a bit to make the resulting patches more legible. And I tossed in more debugs and whatnot so we can actually observe what it's doing. Quickly smoke tested on tgl and adl, and things seem pretty decent. Unfortunately I don't have a LNL on me right now so I haven't fully tested the mdclk/cdclk ratio changes on real hw, but I did hack my adl to pretend that the ratio changes with cdclk and double checked that the logs look sensible for all the combinations of cdclk increase/decrease and mbus join enable/disable. So should work (tm) on real hw too. Stanislav Lisovskiy (3): drm/i915: Loop over all active pipes in intel_mbus_dbox_update drm/i915: Use old mbus_join value when increasing CDCLK drm/i915: Implement vblank synchronized MBUS join changes Ville Syrjälä (10): drm/i915/cdclk: Fix CDCLK programming order when pipes are active drm/i915/cdclk: Fix voltage_level programming edge case drm/i915/cdclk: Drop tgl/dg2 cdclk bump hacks drm/i915/cdclk: Indicate whether CDCLK change happens during pre or post plane update drm/i915: Relocate intel_mbus_dbox_update() drm/i915: Extract intel_dbuf_mbus_join_update() drm/i915: Extract intel_dbuf_mdclk_min_tracker_update() drm/i915: Add debugs for mbus joining and dbuf ratio programming drm/i915: Use a plain old int for the cdclk/mdclk ratio drm/i915: Optimize out redundant dbuf slice updates drivers/gpu/drm/i915/display/intel_cdclk.c | 85 +++-- drivers/gpu/drm/i915/display/intel_cdclk.h | 8 +- drivers/gpu/drm/i915/display/intel_display.c | 5 +- drivers/gpu/drm/i915/display/skl_watermark.c | 344 --- drivers/gpu/drm/i915/display/skl_watermark.h | 9 +- 5 files changed, 271 insertions(+), 180 deletions(-) -- 2.43.2
Re: ✗ Fi.CI.BAT: failure for drm/i915: Add new PCI IDs to DG2 platform in driver
On Wed, Mar 27, 2024 at 12:52:49AM -, Patchwork wrote: > == Series Details == > > Series: drm/i915: Add new PCI IDs to DG2 platform in driver > URL : https://patchwork.freedesktop.org/series/131625/ > State : failure > > == Summary == > > CI Bug Log - changes from CI_DRM_14489 -> Patchwork_131625v1 > > > Summary > --- > > **FAILURE** > > Serious unknown changes coming with Patchwork_131625v1 absolutely need to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_131625v1, 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_131625v1/index.html > > Participating hosts (38 -> 36) > -- > > Additional (2): bat-dg1-7 fi-kbl-8809g > Missing(4): bat-mtlp-8 fi-glk-j4005 fi-bsw-nick fi-snb-2520m > > Possible new issues > --- > > Here are the unknown changes that may have been introduced in > Patchwork_131625v1: > > ### IGT changes ### > > Possible regressions > > * igt@kms_pm_rpm@basic-rte: > - bat-jsl-3: [PASS][1] -> [FAIL][2] >[1]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-jsl-3/igt@kms_pm_...@basic-rte.html >[2]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131625v1/bat-jsl-3/igt@kms_pm_...@basic-rte.html Jasper Lake failing to enter runtime suspend state isn't related to the DG2 PCI IDs being added here. Applied to drm-intel-next. Thanks for the patch. Matt > > > Suppressed > > The following results come from untrusted machines, tests, or statuses. > They do not affect the overall result. > > * igt@i915_selftest@live@active: > - {bat-arls-4}: [PASS][3] -> [DMESG-WARN][4] >[3]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-arls-4/igt@i915_selftest@l...@active.html >[4]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131625v1/bat-arls-4/igt@i915_selftest@l...@active.html > > > Known issues > > > Here are the changes found in Patchwork_131625v1 that come from known > issues: > > ### CI changes ### > > Issues hit > > * boot: > - bat-arls-3: [PASS][5] -> [FAIL][6] ([i915#10234]) >[5]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-arls-3/boot.html >[6]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131625v1/bat-arls-3/boot.html > - bat-jsl-1: [PASS][7] -> [FAIL][8] ([i915#8293]) >[7]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-jsl-1/boot.html >[8]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131625v1/bat-jsl-1/boot.html > - fi-kbl-8809g: NOTRUN -> [FAIL][9] ([i915#8293]) >[9]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131625v1/fi-kbl-8809g/boot.html > > > > ### IGT changes ### > > Issues hit > > * igt@gem_mmap@basic: > - bat-dg1-7: NOTRUN -> [SKIP][10] ([i915#4083]) >[10]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131625v1/bat-dg1-7/igt@gem_m...@basic.html > > * igt@gem_tiled_fence_blits@basic: > - bat-dg1-7: NOTRUN -> [SKIP][11] ([i915#4077]) +2 other tests > skip >[11]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131625v1/bat-dg1-7/igt@gem_tiled_fence_bl...@basic.html > > * igt@gem_tiled_pread_basic: > - bat-dg1-7: NOTRUN -> [SKIP][12] ([i915#4079]) +1 other test > skip >[12]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131625v1/bat-dg1-7/igt@gem_tiled_pread_basic.html > > * igt@i915_pm_rps@basic-api: > - bat-dg1-7: NOTRUN -> [SKIP][13] ([i915#6621]) >[13]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131625v1/bat-dg1-7/igt@i915_pm_...@basic-api.html > > * igt@kms_addfb_basic@addfb25-x-tiled-mismatch-legacy: > - bat-dg1-7: NOTRUN -> [SKIP][14] ([i915#4212]) +7 other tests > skip >[14]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131625v1/bat-dg1-7/igt@kms_addfb_ba...@addfb25-x-tiled-mismatch-legacy.html > > * igt@kms_addfb_basic@basic-y-tiled-legacy: > - bat-dg1-7: NOTRUN -> [SKIP][15] ([i915#4215]) >[15]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131625v1/bat-dg1-7/igt@kms_addfb_ba...@basic-y-tiled-legacy.html > > * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy: > - bat-dg1-7: NOTRUN -> [SKIP][16] ([i915#4103] / [i915#4213]) +1 > other test skip >[16]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131625v1/bat-dg1-7/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html > > * igt@kms_dsc@dsc-basic: > - bat-dg1-7: NOTRUN -> [SKIP][17] ([i915#3555] / [i915#3840]) >[17]: >
Re: [PATCH i-g-t] lib/kunit: Read results from debugfs
Hi Janusz, On 2024-03-27 at 12:22:54 +0100, Janusz Krzysztofik wrote: > KUnit can provide KTAP reports from test modules via debugfs files, one > per test suite. Using that source of test results instead of extracting > them from dmesg, where they may be interleaved with other kernel messages, > seems more easy to handle and less error prone. Switch to it. > > If KUnit debugfs support is found not configured then fall back to legacy > processing path. > > Signed-off-by: Janusz Krzysztofik > --- > lib/igt_kmod.c | 143 - > 1 file changed, 105 insertions(+), 38 deletions(-) > > diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c > index 1ec9c8a602..a5b170ca9c 100644 > --- a/lib/igt_kmod.c > +++ b/lib/igt_kmod.c > @@ -28,6 +28,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -39,6 +40,7 @@ > > #include "igt_aux.h" > #include "igt_core.h" > +#include "igt_debugfs.h" > #include "igt_kmod.h" > #include "igt_ktap.h" > #include "igt_sysfs.h" > @@ -864,6 +866,31 @@ static int open_parameters(const char *module_name) > return open(path, O_RDONLY); > } > > +static DIR *kunit_debugfs_open(void) > +{ > + const char *debugfs_path = igt_debugfs_mount(); > + int debugfs_fd, kunit_fd; > + DIR *kunit_dir; > + > + if (igt_debug_on(!debugfs_path)) > + return NULL; > + > + debugfs_fd = open(debugfs_path, O_DIRECTORY); > + if (igt_debug_on(debugfs_fd < 0)) > + return NULL; > + > + kunit_fd = openat(debugfs_fd, "kunit", O_DIRECTORY); > + close(debugfs_fd); > + if (igt_debug_on(kunit_fd < 0)) > + return NULL; > + > + kunit_dir = fdopendir(kunit_fd); > + if (igt_debug_on(!kunit_dir)) > + close(kunit_fd); imho here: close(kunit_fd); igt_debug_on(!kunit_dir); > + > + return kunit_dir; > +} > + > static bool kunit_set_filtering(const char *filter_glob, const char *filter, > const char *filter_action) > { > @@ -1071,23 +1098,48 @@ static void kunit_results_free(struct igt_list_head > *results, > free(*suite_name); > } > > -static int kunit_get_results(struct igt_list_head *results, int kmsg_fd, > - struct igt_ktap_results **ktap) > +static int kunit_get_results(struct igt_list_head *results, int debugfs_fd, > + const char *suite, struct igt_ktap_results **ktap) > { > - int err; > + FILE *results_stream; > + int ret, results_fd; > + char *buf = NULL; > + size_t size = 0; > + ssize_t len; > + > + if (igt_debug_on((ret = openat(debugfs_fd, suite, O_DIRECTORY), ret < > 0))) > + return ret; > + > + results_fd = openat(ret, "results", O_RDONLY); > + close(ret); > + if (igt_debug_on(results_fd < 0)) > + return results_fd; > + > + results_stream = fdopen(results_fd, "r"); > + if (igt_debug_on(!results_stream)) { > + close(results_fd); > + return -errno; > + } > > *ktap = igt_ktap_alloc(results); > - if (igt_debug_on(!*ktap)) > - return -ENOMEM; > + if (igt_debug_on(!*ktap)) { > + ret = -ENOMEM; > + goto out_fclose; > + } > + > + while (len = getline(, , results_stream), len > 0) { > + ret = igt_ktap_parse(buf, *ktap); > + if (ret != -EINPROGRESS) > + break; > + } > > - do > - igt_debug_on((err = kunit_kmsg_result_get(results, NULL, > kmsg_fd, *ktap), > - err && err != -EINPROGRESS)); > - while (err == -EINPROGRESS); > + free(buf); > > igt_ktap_free(ktap); > +out_fclose: > + fclose(results_stream); > > - return err; > + return ret; > } > > static void __igt_kunit_legacy(struct igt_ktest *tst, > @@ -1101,7 +1153,13 @@ static void __igt_kunit_legacy(struct igt_ktest *tst, > pthread_mutexattr_t attr; > IGT_LIST_HEAD(results); > unsigned long taints; > - int ret; > + int flags, ret; > + > + igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n"); > + > + igt_skip_on((flags = fcntl(tst->kmsg, F_GETFL, 0), flags < 0)); > + igt_skip_on_f(fcntl(tst->kmsg, F_SETFL, flags & ~O_NONBLOCK) == -1, > + "Could not set /dev/kmsg to blocking mode\n"); > > igt_skip_on(lseek(tst->kmsg, 0, SEEK_END) < 0); > > @@ -1224,30 +1282,20 @@ static void __igt_kunit_legacy(struct igt_ktest *tst, > igt_skip_on_f(ret, "KTAP parser failed\n"); > } > > -static void kunit_get_tests_timeout(int signal) > -{ > - igt_skip("Timed out while trying to extract a list of KUnit test cases > from /dev/kmsg\n"); > -} > - > static bool kunit_get_tests(struct igt_list_head *tests, > struct igt_ktest *tst, > const char *suite, > const
Re: [RESEND v3 2/2] drm: Add CONFIG_DRM_WERROR
On Wed, Mar 27, 2024 at 09:59:01AM +0200, Jani Nikula wrote: > On Wed, 27 Mar 2024, Maxime Ripard wrote: > > On Tue, Mar 26, 2024 at 03:56:50PM -0700, Nathan Chancellor wrote: > >> On Tue, Mar 05, 2024 at 11:07:36AM +0200, Jani Nikula wrote: > >> > Add kconfig to enable -Werror subsystem wide. This is useful for > >> > development and CI to keep the subsystem warning free, while avoiding > >> > issues outside of the subsystem that kernel wide CONFIG_WERROR=y might > >> > hit. > >> > > >> > v2: Don't depend on COMPILE_TEST > >> > > >> > Reviewed-by: Hamza Mahfooz # v1 > >> > Signed-off-by: Jani Nikula > >> > --- > >> > drivers/gpu/drm/Kconfig | 13 + > >> > drivers/gpu/drm/Makefile | 3 +++ > >> > 2 files changed, 16 insertions(+) > >> > > >> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > >> > index 6e853acf15da..c08e18108c2a 100644 > >> > --- a/drivers/gpu/drm/Kconfig > >> > +++ b/drivers/gpu/drm/Kconfig > >> > @@ -416,3 +416,16 @@ config DRM_LIB_RANDOM > >> > config DRM_PRIVACY_SCREEN > >> > bool > >> > default n > >> > + > >> > +config DRM_WERROR > >> > +bool "Compile the drm subsystem with warnings as errors" > >> > +depends on EXPERT > >> > +default n > >> > +help > >> > + A kernel build should not cause any compiler warnings, and > >> > this > >> > + enables the '-Werror' flag to enforce that rule in the drm > >> > subsystem. > >> > + > >> > + The drm subsystem enables more warnings than the kernel > >> > default, so > >> > + this config option is disabled by default. > >> > + > >> > + If in doubt, say N. > >> > >> While I understand the desire for an easy switch that maintainers and > >> developers can use to ensure that their changes are warning free for the > >> drm subsystem specifically, I think subsystem specific configuration > >> options like this are actively detrimental to developers and continuous > >> integration systems that build test the entire kernel. For example, we > >> turned off CONFIG_WERROR for our Hexagon builds because of warnings that > >> appear with -Wextra that are legitimate but require treewide changes to > >> resolve in a manner sufficient for Linus: > >> > >> https://github.com/ClangBuiltLinux/linux/issues/1285 > >> https://lore.kernel.org/all/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/ > >> https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/ > >> > >> But now, due to CONFIG_DRM_WERROR getting enabled by all{mod,yes}config > >> and -Wextra being unconditionally enabled for DRM, those warnings hard > >> break the build despite CONFIG_WERROR=n... > > > > Would making DRM_WERROR depends on WERROR address your concerns? > > But then what would be the point of having DRM_WERROR at all? For me the > point is, "werror in drm, ignore the rest, they're someone else's > problem". Right, I do think this is a valid view point and one I am sympathetic to, especially since it is in the pursuit of increased code quality. I do not want to disrupt that. > An alternative would be to "depends on !COMPILE_TEST" that we have in > i915, but then some folks want to have COMPILE_TEST in drm, because some > drivers are otherwise hard for people to build. Right. I think it is unfortunate how (at least in my opinion) CONFIG_COMPILE_TEST has two meanings: genuinely just compile testing or "allmodconfig". For the first case, we would want CONFIG_DRM_WERROR=y but for the second case, it would be nice for CONFIG_DRM_WERROR to default to off (because CONFIG_WERROR is enabled) but allow developers to turn it on explicitly. Another lofty/wistful idea to solve this would be to implement something similar to compiler diagnostic groups for Kconfig, where there would be a hierarchy like CONFIG_WERROR CONFIG_DRM_WERROR CONFIG_SUBSYSTEM_A_WERROR CONFIG_SUBSYSTEM_B_WERROR where the value of CONFIG_WERROR is the same value for all subconfigurations under it but they could still be enabled individually without any additional dependencies (ala something like '-Wno-unused -Wunused-variable'), which would allow my use case of CONFIG_WERROR=n removing all instances of -Werror to continue to work but allow other developers and CI systems to just set their specific -Werror configuration and be done with it. I don't think something like that exists but maybe I don't know Kconfig as well as I think I do :) > Nathan, we do want to fix any issues switfly. Are you hitting specific > build problems? Yes, I see three distinct set of problems from our CI as a direct result of this series. I already covered two in the prior mail but I'll be a little more expansive below. 1. Instances of -Wunused-but-set-variable from variables that only have unary operations applied to them. Clang can warn in this case where GCC cannot: https://godbolt.org/z/d368q3coP int main(void) { int a = 0;
✗ Fi.CI.SPARSE: warning for Disable automatic load CCS load balancing (rev12)
== Series Details == Series: Disable automatic load CCS load balancing (rev12) URL : https://patchwork.freedesktop.org/series/129951/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. +./arch/x86/include/asm/bitops.h:116:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:147:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:149:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:153:26: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:155:16: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:155:9: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:173:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:175:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:179:35: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:181:16: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:181:9: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:185:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:187:9: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:191:35: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:194:16: warning: unreplaced symbol 'oldbit' +./arch/x86/include/asm/bitops.h:194:9: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:236:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:238:9: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:66:1: warning: unreplaced symbol 'return' +./arch/x86/include/asm/bitops.h:92:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:100:17: warning: unreplaced symbol 'old' +./include/asm-generic/bitops/generic-non-atomic.h:100:23: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:100:9: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:105:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:107:9: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:108:9: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:109:9: warning: unreplaced symbol 'old' +./include/asm-generic/bitops/generic-non-atomic.h:111:10: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:111:14: warning: unreplaced symbol 'old' +./include/asm-generic/bitops/generic-non-atomic.h:111:20: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:112:17: warning: unreplaced symbol 'old' +./include/asm-generic/bitops/generic-non-atomic.h:112:23: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:112:9: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:121:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:128:9: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:166:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:168:9: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:169:9: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:170:9: warning: unreplaced symbol 'val' +./include/asm-generic/bitops/generic-non-atomic.h:172:19: warning: unreplaced symbol 'val' +./include/asm-generic/bitops/generic-non-atomic.h:172:25: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:172:9: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:28:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:30:9: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:31:9: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:33:10: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:33:16: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:37:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:39:9: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:40:9: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:42:10: warning: unreplaced symbol 'p' +./include/asm-generic/bitops/generic-non-atomic.h:42:16: warning: unreplaced symbol 'mask' +./include/asm-generic/bitops/generic-non-atomic.h:55:1: warning: unreplaced symbol 'return' +./include/asm-generic/bitops/generic-non-atomic.h:57:9: warning: unreplaced symbol 'mask'
✗ Fi.CI.CHECKPATCH: warning for Disable automatic load CCS load balancing (rev12)
== Series Details == Series: Disable automatic load CCS load balancing (rev12) URL : https://patchwork.freedesktop.org/series/129951/ State : warning == Summary == Error: dim checkpatch failed 6f6385b0a101 drm/i915/gt: Disable HW load balancing for CCS 9e580948c216 drm/i915/gt: Do not generate the command streamer for all the CCS f630774aa552 drm/i915/gt: Enable only one CCS for compute workload Traceback (most recent call last): File "scripts/spdxcheck.py", line 6, in from ply import lex, yacc ModuleNotFoundError: No module named 'ply' Traceback (most recent call last): File "scripts/spdxcheck.py", line 6, in from ply import lex, yacc ModuleNotFoundError: No module named 'ply' -:37: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #37: new file mode 100644 -:111: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'cslice' may be better as '(cslice)' to avoid precedence issues #111: FILE: drivers/gpu/drm/i915/gt/intel_gt_regs.h:1433: +#define XEHP_CCS_MODE_CSLICE(cslice, ccs)(ccs << (cslice * XEHP_CCS_MODE_CSLICE_WIDTH)) -:111: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'ccs' may be better as '(ccs)' to avoid precedence issues #111: FILE: drivers/gpu/drm/i915/gt/intel_gt_regs.h:1433: +#define XEHP_CCS_MODE_CSLICE(cslice, ccs)(ccs << (cslice * XEHP_CCS_MODE_CSLICE_WIDTH)) total: 0 errors, 1 warnings, 2 checks, 89 lines checked
✓ Fi.CI.BAT: success for QGV/SAGV related fixes (rev9)
== Series Details == Series: QGV/SAGV related fixes (rev9) URL : https://patchwork.freedesktop.org/series/126962/ State : success == Summary == CI Bug Log - changes from CI_DRM_14489 -> Patchwork_126962v9 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/index.html Participating hosts (38 -> 36) -- Additional (2): bat-dg1-7 fi-kbl-8809g Missing(4): bat-arls-4 bat-mtlp-8 fi-glk-j4005 fi-snb-2520m Known issues Here are the changes found in Patchwork_126962v9 that come from known issues: ### CI changes ### Issues hit * boot: - bat-arls-3: [PASS][1] -> [FAIL][2] ([i915#10234]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-arls-3/boot.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-arls-3/boot.html Possible fixes * boot: - bat-dg2-11: [FAIL][3] ([i915#10491]) -> [PASS][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-11/boot.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-11/boot.html ### IGT changes ### Issues hit * igt@gem_mmap@basic: - bat-dg1-7: NOTRUN -> [SKIP][5] ([i915#4083]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg1-7/igt@gem_m...@basic.html - bat-dg2-11: NOTRUN -> [SKIP][6] ([i915#4083]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-11/igt@gem_m...@basic.html * igt@gem_tiled_fence_blits@basic: - bat-dg1-7: NOTRUN -> [SKIP][7] ([i915#4077]) +2 other tests skip [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg1-7/igt@gem_tiled_fence_bl...@basic.html - bat-dg2-11: NOTRUN -> [SKIP][8] ([i915#4077]) +2 other tests skip [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-11/igt@gem_tiled_fence_bl...@basic.html * igt@gem_tiled_pread_basic: - bat-dg1-7: NOTRUN -> [SKIP][9] ([i915#4079]) +1 other test skip [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg1-7/igt@gem_tiled_pread_basic.html - bat-dg2-11: NOTRUN -> [SKIP][10] ([i915#4079]) +1 other test skip [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-11/igt@gem_tiled_pread_basic.html * igt@i915_pm_rps@basic-api: - bat-dg1-7: NOTRUN -> [SKIP][11] ([i915#6621]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg1-7/igt@i915_pm_...@basic-api.html - bat-dg2-11: NOTRUN -> [SKIP][12] ([i915#6621]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-11/igt@i915_pm_...@basic-api.html * igt@i915_selftest@live@gt_heartbeat: - bat-dg2-9: [PASS][13] -> [INCOMPLETE][14] ([i915#10556]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-9/igt@i915_selftest@live@gt_heartbeat.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-9/igt@i915_selftest@live@gt_heartbeat.html * igt@i915_selftest@live@gt_mocs: - bat-adln-1: [PASS][15] -> [INCOMPLETE][16] ([i915#10072]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-adln-1/igt@i915_selftest@live@gt_mocs.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-adln-1/igt@i915_selftest@live@gt_mocs.html * igt@i915_selftest@live@gt_tlb: - bat-dg2-14: [PASS][17] -> [ABORT][18] ([i915#10592]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-14/igt@i915_selftest@live@gt_tlb.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-14/igt@i915_selftest@live@gt_tlb.html * igt@kms_addfb_basic@addfb25-x-tiled-mismatch-legacy: - bat-dg1-7: NOTRUN -> [SKIP][19] ([i915#4212]) +7 other tests skip [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg1-7/igt@kms_addfb_ba...@addfb25-x-tiled-mismatch-legacy.html - bat-dg2-11: NOTRUN -> [SKIP][20] ([i915#4212]) +7 other tests skip [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-11/igt@kms_addfb_ba...@addfb25-x-tiled-mismatch-legacy.html * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy: - bat-dg2-11: NOTRUN -> [SKIP][21] ([i915#5190]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-11/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html * igt@kms_addfb_basic@basic-y-tiled-legacy: - bat-dg1-7: NOTRUN -> [SKIP][22] ([i915#4215]) [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg1-7/igt@kms_addfb_ba...@basic-y-tiled-legacy.html - bat-dg2-11: NOTRUN -> [SKIP][23] ([i915#4215] / [i915#5190]) [23]:
Re: [PATCH i-g-t] lib/kunit: Read results from debugfs
On Wed, Mar 27, 2024 at 12:22:54PM +0100, Janusz Krzysztofik wrote: KUnit can provide KTAP reports from test modules via debugfs files, one per test suite. Using that source of test results instead of extracting them from dmesg, where they may be interleaved with other kernel messages, seems more easy to handle and less error prone. Switch to it. If KUnit debugfs support is found not configured then fall back to legacy processing path. Signed-off-by: Janusz Krzysztofik --- lib/igt_kmod.c | 143 - 1 file changed, 105 insertions(+), 38 deletions(-) diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c index 1ec9c8a602..a5b170ca9c 100644 --- a/lib/igt_kmod.c +++ b/lib/igt_kmod.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -39,6 +40,7 @@ #include "igt_aux.h" #include "igt_core.h" +#include "igt_debugfs.h" #include "igt_kmod.h" #include "igt_ktap.h" #include "igt_sysfs.h" @@ -864,6 +866,31 @@ static int open_parameters(const char *module_name) return open(path, O_RDONLY); } +static DIR *kunit_debugfs_open(void) +{ + const char *debugfs_path = igt_debugfs_mount(); + int debugfs_fd, kunit_fd; + DIR *kunit_dir; + + if (igt_debug_on(!debugfs_path)) + return NULL; + + debugfs_fd = open(debugfs_path, O_DIRECTORY); + if (igt_debug_on(debugfs_fd < 0)) + return NULL; + + kunit_fd = openat(debugfs_fd, "kunit", O_DIRECTORY); + close(debugfs_fd); + if (igt_debug_on(kunit_fd < 0)) + return NULL; + + kunit_dir = fdopendir(kunit_fd); + if (igt_debug_on(!kunit_dir)) + close(kunit_fd); + + return kunit_dir; any reason not to use strcat() + return fopen() +} + static bool kunit_set_filtering(const char *filter_glob, const char *filter, const char *filter_action) { @@ -1071,23 +1098,48 @@ static void kunit_results_free(struct igt_list_head *results, free(*suite_name); } -static int kunit_get_results(struct igt_list_head *results, int kmsg_fd, -struct igt_ktap_results **ktap) +static int kunit_get_results(struct igt_list_head *results, int debugfs_fd, +const char *suite, struct igt_ktap_results **ktap) { - int err; + FILE *results_stream; + int ret, results_fd; + char *buf = NULL; + size_t size = 0; + ssize_t len; + + if (igt_debug_on((ret = openat(debugfs_fd, suite, O_DIRECTORY), ret < 0))) a little odd to return on any value != 0, but log on < 0. did you mean to compare < 0 in the first arg?. + return ret; + + results_fd = openat(ret, "results", O_RDONLY); + close(ret); + if (igt_debug_on(results_fd < 0)) + return results_fd; + + results_stream = fdopen(results_fd, "r"); + if (igt_debug_on(!results_stream)) { + close(results_fd); + return -errno; + } *ktap = igt_ktap_alloc(results); - if (igt_debug_on(!*ktap)) - return -ENOMEM; + if (igt_debug_on(!*ktap)) { + ret = -ENOMEM; + goto out_fclose; + } + + while (len = getline(, , results_stream), len > 0) { + ret = igt_ktap_parse(buf, *ktap); + if (ret != -EINPROGRESS) + break; + } - do - igt_debug_on((err = kunit_kmsg_result_get(results, NULL, kmsg_fd, *ktap), - err && err != -EINPROGRESS)); - while (err == -EINPROGRESS); + free(buf); igt_ktap_free(ktap); +out_fclose: + fclose(results_stream); - return err; + return ret; } static void __igt_kunit_legacy(struct igt_ktest *tst, @@ -1101,7 +1153,13 @@ static void __igt_kunit_legacy(struct igt_ktest *tst, pthread_mutexattr_t attr; IGT_LIST_HEAD(results); unsigned long taints; - int ret; + int flags, ret; + + igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n"); + + igt_skip_on((flags = fcntl(tst->kmsg, F_GETFL, 0), flags < 0)); + igt_skip_on_f(fcntl(tst->kmsg, F_SETFL, flags & ~O_NONBLOCK) == -1, + "Could not set /dev/kmsg to blocking mode\n"); igt_skip_on(lseek(tst->kmsg, 0, SEEK_END) < 0); @@ -1224,30 +1282,20 @@ static void __igt_kunit_legacy(struct igt_ktest *tst, igt_skip_on_f(ret, "KTAP parser failed\n"); } -static void kunit_get_tests_timeout(int signal) -{ - igt_skip("Timed out while trying to extract a list of KUnit test cases from /dev/kmsg\n"); -} - static bool kunit_get_tests(struct igt_list_head *tests, struct igt_ktest *tst, const char *suite, const char *opts, + DIR *debugfs_dir, struct
[PATCH v7 3/3] drm/i915/gt: Enable only one CCS for compute workload
Enable only one CCS engine by default with all the compute sices allocated to it. While generating the list of UABI engines to be exposed to the user, exclude any additional CCS engines beyond the first instance. This change can be tested with igt i915_query. Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement") Signed-off-by: Andi Shyti Cc: Chris Wilson Cc: Joonas Lahtinen Cc: Matt Roper Cc: # v6.2+ Reviewed-by: Matt Roper Acked-by: Michal Mrozek --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c | 39 + drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.h | 13 +++ drivers/gpu/drm/i915/gt/intel_gt_regs.h | 5 +++ drivers/gpu/drm/i915/gt/intel_workarounds.c | 7 5 files changed, 65 insertions(+) create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.h diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 3ef6ed41e62b..a6885a1d41a1 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -118,6 +118,7 @@ gt-y += \ gt/intel_ggtt_fencing.o \ gt/intel_gt.o \ gt/intel_gt_buffer_pool.o \ + gt/intel_gt_ccs_mode.o \ gt/intel_gt_clock_utils.o \ gt/intel_gt_debugfs.o \ gt/intel_gt_engines_debugfs.o \ diff --git a/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c b/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c new file mode 100644 index ..044219c5960a --- /dev/null +++ b/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2024 Intel Corporation + */ + +#include "i915_drv.h" +#include "intel_gt.h" +#include "intel_gt_ccs_mode.h" +#include "intel_gt_regs.h" + +void intel_gt_apply_ccs_mode(struct intel_gt *gt) +{ + int cslice; + u32 mode = 0; + int first_ccs = __ffs(CCS_MASK(gt)); + + if (!IS_DG2(gt->i915)) + return; + + /* Build the value for the fixed CCS load balancing */ + for (cslice = 0; cslice < I915_MAX_CCS; cslice++) { + if (CCS_MASK(gt) & BIT(cslice)) + /* +* If available, assign the cslice +* to the first available engine... +*/ + mode |= XEHP_CCS_MODE_CSLICE(cslice, first_ccs); + + else + /* +* ... otherwise, mark the cslice as +* unavailable if no CCS dispatches here +*/ + mode |= XEHP_CCS_MODE_CSLICE(cslice, +XEHP_CCS_MODE_CSLICE_MASK); + } + + intel_uncore_write(gt->uncore, XEHP_CCS_MODE, mode); +} diff --git a/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.h b/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.h new file mode 100644 index ..9e5549caeb26 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2024 Intel Corporation + */ + +#ifndef __INTEL_GT_CCS_MODE_H__ +#define __INTEL_GT_CCS_MODE_H__ + +struct intel_gt; + +void intel_gt_apply_ccs_mode(struct intel_gt *gt); + +#endif /* __INTEL_GT_CCS_MODE_H__ */ diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h index 31b102604e3d..743fe3566722 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h @@ -1480,6 +1480,11 @@ #define XEHP_RCU_MODE_FIXED_SLICE_CCS_MODE REG_BIT(1) #define GEN12_RCU_MODE_CCS_ENABLEREG_BIT(0) +#define XEHP_CCS_MODE _MMIO(0x14804) +#define XEHP_CCS_MODE_CSLICE_MASKREG_GENMASK(2, 0) /* CCS0-3 + rsvd */ +#define XEHP_CCS_MODE_CSLICE_WIDTH ilog2(XEHP_CCS_MODE_CSLICE_MASK + 1) +#define XEHP_CCS_MODE_CSLICE(cslice, ccs)(ccs << (cslice * XEHP_CCS_MODE_CSLICE_WIDTH)) + #define CHV_FUSE_GT_MMIO(VLV_GUNIT_BASE + 0x2168) #define CHV_FGT_DISABLE_SS0 (1 << 10) #define CHV_FGT_DISABLE_SS1 (1 << 11) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 9963e5725ae5..8188c9f0b5ce 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -10,6 +10,7 @@ #include "intel_engine_regs.h" #include "intel_gpu_commands.h" #include "intel_gt.h" +#include "intel_gt_ccs_mode.h" #include "intel_gt_mcr.h" #include "intel_gt_print.h" #include "intel_gt_regs.h" @@ -2869,6 +2870,12 @@ static void ccs_engine_wa_mode(struct intel_engine_cs *engine, struct i915_wa_li * made to completely disable automatic CCS load balancing. */ wa_masked_en(wal, GEN12_RCU_MODE, XEHP_RCU_MODE_FIXED_SLICE_CCS_MODE); + +
[PATCH v7 2/3] drm/i915/gt: Do not generate the command streamer for all the CCS
We want a fixed load CCS balancing consisting in all slices sharing one single user engine. For this reason do not create the intel_engine_cs structure with its dedicated command streamer for CCS slices beyond the first. Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement") Signed-off-by: Andi Shyti Cc: Chris Wilson Cc: Joonas Lahtinen Cc: Matt Roper Cc: # v6.2+ Acked-by: Michal Mrozek --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index f553cf4e6449..47c4a69e854c 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -908,6 +908,21 @@ static intel_engine_mask_t init_engine_mask(struct intel_gt *gt) info->engine_mask &= ~BIT(GSC0); } + /* +* Do not create the command streamer for CCS slices beyond the first. +* All the workload submitted to the first engine will be shared among +* all the slices. +* +* Once the user will be allowed to customize the CCS mode, then this +* check needs to be removed. +*/ + if (IS_DG2(gt->i915)) { + intel_engine_mask_t first_ccs = BIT((CCS0 + __ffs(CCS_MASK(gt; + intel_engine_mask_t all_ccs = CCS_MASK(gt) << CCS0; + + info->engine_mask &= ~(all_ccs &= ~first_ccs); + } + return info->engine_mask; } -- 2.43.0
[PATCH v7 1/3] drm/i915/gt: Disable HW load balancing for CCS
The hardware should not dynamically balance the load between CCS engines. Wa_14019159160 recommends disabling it across all platforms. Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement") Signed-off-by: Andi Shyti Cc: Chris Wilson Cc: Joonas Lahtinen Cc: Matt Roper Cc: # v6.2+ Reviewed-by: Matt Roper Acked-by: Michal Mrozek --- drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 + drivers/gpu/drm/i915/gt/intel_workarounds.c | 23 +++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h index 50962cfd1353..31b102604e3d 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h @@ -1477,6 +1477,7 @@ #define ECOBITS_PPGTT_CACHE4B(0 << 8) #define GEN12_RCU_MODE _MMIO(0x14800) +#define XEHP_RCU_MODE_FIXED_SLICE_CCS_MODE REG_BIT(1) #define GEN12_RCU_MODE_CCS_ENABLEREG_BIT(0) #define CHV_FUSE_GT_MMIO(VLV_GUNIT_BASE + 0x2168) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index b079cbbc1897..9963e5725ae5 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -51,7 +51,8 @@ * registers belonging to BCS, VCS or VECS should be implemented in * xcs_engine_wa_init(). Workarounds for registers not belonging to a specific * engine's MMIO range but that are part of of the common RCS/CCS reset domain - * should be implemented in general_render_compute_wa_init(). + * should be implemented in general_render_compute_wa_init(). The settings + * about the CCS load balancing should be added in ccs_engine_wa_mode(). * * - GT workarounds: the list of these WAs is applied whenever these registers * revert to their default values: on GPU reset, suspend/resume [1]_, etc. @@ -2854,6 +2855,22 @@ add_render_compute_tuning_settings(struct intel_gt *gt, wa_write_clr(wal, GEN8_GARBCNTL, GEN12_BUS_HASH_CTL_BIT_EXC); } +static void ccs_engine_wa_mode(struct intel_engine_cs *engine, struct i915_wa_list *wal) +{ + struct intel_gt *gt = engine->gt; + + if (!IS_DG2(gt->i915)) + return; + + /* +* Wa_14019159160: This workaround, along with others, leads to +* significant challenges in utilizing load balancing among the +* CCS slices. Consequently, an architectural decision has been +* made to completely disable automatic CCS load balancing. +*/ + wa_masked_en(wal, GEN12_RCU_MODE, XEHP_RCU_MODE_FIXED_SLICE_CCS_MODE); +} + /* * The workarounds in this function apply to shared registers in * the general render reset domain that aren't tied to a @@ -3000,8 +3017,10 @@ engine_init_workarounds(struct intel_engine_cs *engine, struct i915_wa_list *wal * to a single RCS/CCS engine's workaround list since * they're reset as part of the general render domain reset. */ - if (engine->flags & I915_ENGINE_FIRST_RENDER_COMPUTE) + if (engine->flags & I915_ENGINE_FIRST_RENDER_COMPUTE) { general_render_compute_wa_init(engine, wal); + ccs_engine_wa_mode(engine, wal); + } if (engine->class == COMPUTE_CLASS) ccs_engine_wa_init(engine, wal); -- 2.43.0
[PATCH v7 0/3] Disable automatic load CCS load balancing
Hi, this series does basically two things: 1. Disables automatic load balancing as adviced by the hardware workaround. 2. Assigns all the CCS slices to one single user engine. The user will then be able to query only one CCS engine >From v5 I have created a new file, gt/intel_gt_ccs_mode.c where I added the intel_gt_apply_ccs_mode(). In the upcoming patches, this file will contain the implementation for dynamic CCS mode setting. Thanks Tvrtko, Matt, John and Joonas for your reviews! Andi Changelog = v6 -> v7 - find a more appropriate place where to remove the CCS engines: remove them in init_engine_mask() instead of intel_engines_init_mmio(). (Thanks, Matt) - Add Michal's ACK, thanks Michal! v5 -> v6 (thanks Matt for the suggestions in v6) - Remove the refactoring and the for_each_available_engine() macro and instead do not create the intel_engine_cs structure at all. - In patch 1 just a trivial reordering of the bit definitions. v4 -> v5 - Use the workaround framework to do all the CCS balancing settings in order to always apply the modes also when the engine resets. Put everything in its own specific function to be executed for the first CCS engine encountered. (Thanks Matt) - Calculate the CCS ID for the CCS mode as the first available CCS among all the engines (Thanks Matt) - create the intel_gt_ccs_mode.c function to host the CCS configuration. We will have it ready for the next series. - Fix a selftest that was failing because could not set CCS2. - Add the for_each_available_engine() macro to exclude CCS1+ and start using it in the hangcheck selftest. v3 -> v4 - Reword correctly the comment in the workaround - Fix a buffer overflow (Thanks Joonas) - Handle properly the fused engines when setting the CCS mode. v2 -> v3 - Simplified the algorithm for creating the list of the exported uabi engines. (Patch 1) (Thanks, Tvrtko) - Consider the fused engines when creating the uabi engine list (Patch 2) (Thanks, Matt) - Patch 4 now uses a the refactoring from patch 1, in a cleaner outcome. v1 -> v2 - In Patch 1 use the correct workaround number (thanks Matt). - In Patch 2 do not add the extra CCS engines to the exposed UABI engine list and adapt the engine counting accordingly (thanks Tvrtko). - Reword the commit of Patch 2 (thanks John). Andi Shyti (3): drm/i915/gt: Disable HW load balancing for CCS drm/i915/gt: Do not generate the command streamer for all the CCS drm/i915/gt: Enable only one CCS for compute workload drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gt/intel_engine_cs.c | 15 drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c | 39 + drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.h | 13 +++ drivers/gpu/drm/i915/gt/intel_gt_regs.h | 6 drivers/gpu/drm/i915/gt/intel_workarounds.c | 30 ++-- 6 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.c create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_ccs_mode.h -- 2.43.0
Re: [PATCH 06/11] drm/i915/dp_mst: Sanitize calculating the DSC DPT bpp limit
On Tue, Mar 26, 2024 at 01:13:38PM -0700, Manasi Navare wrote: > Hi Imre, > > Would this impact/fix DSC functionality on ADL based platforms as well > or will this change only impact platforms that support UHBR? The related DPT limit changes in this patchset make a difference only on platforms supporting UHBR, so DG2 and MTL+. > Manasi > > On Tue, Mar 26, 2024 at 5:55 AM Nautiyal, Ankit K > wrote: > > > > > > On 3/21/2024 1:41 AM, Imre Deak wrote: > > > Instead of checking each compressed bpp value against the maximum > > > DSC/DPT bpp, simplify things by calculating the maximum bpp upfront and > > > limiting the range of bpps looped over using this maximum. > > > > > > While at it add a comment about the origin of the DSC/DPT bpp limit. > > > > > > Bspec: 49259, 68912 > > > > > > Signed-off-by: Imre Deak > > > > LGTM. > > > > Reviewed-by: Ankit Nautiyal > > > > > --- > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 45 +++-- > > > 1 file changed, 23 insertions(+), 22 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > index 40660dc5edb45..516b00f791420 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > @@ -51,27 +51,24 @@ > > > #include "intel_vdsc.h" > > > #include "skl_scaler.h" > > > > > > -static int intel_dp_mst_check_constraints(struct drm_i915_private *i915, > > > int bpp, > > > - const struct drm_display_mode > > > *adjusted_mode, > > > - struct intel_crtc_state > > > *crtc_state, > > > - bool dsc) > > > +static int intel_dp_mst_max_dpt_bpp(const struct intel_crtc_state > > > *crtc_state, > > > + bool dsc) > > > { > > > - if (intel_dp_is_uhbr(crtc_state) && DISPLAY_VER(i915) < 20 && dsc) { > > > - int output_bpp = bpp; > > > - int symbol_clock = > > > intel_dp_link_symbol_clock(crtc_state->port_clock); > > > - int available_bw = mul_u32_u32(symbol_clock * 72, > > > - > > > drm_dp_bw_channel_coding_efficiency(true)) / > > > -100; > > > + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev); > > > + const struct drm_display_mode *adjusted_mode = > > > + _state->hw.adjusted_mode; > > > > > > - if (output_bpp * adjusted_mode->crtc_clock > > > > - available_bw) { > > > - drm_dbg_kms(>drm, "UHBR check failed(required > > > bw %d available %d)\n", > > > - output_bpp * adjusted_mode->crtc_clock, > > > available_bw); > > > - return -EINVAL; > > > - } > > > - } > > > + if (!intel_dp_is_uhbr(crtc_state) || DISPLAY_VER(i915) >= 20 || > > > !dsc) > > > + return INT_MAX; > > > > > > - return 0; > > > + /* > > > + * DSC->DPT interface width: > > > + * ICL-MTL: 72 bits (each branch has 72 bits, only left branch is > > > used) > > > + * LNL+:144 bits (not a bottleneck in any config) > > > + */ > > > + return > > > div64_u64(mul_u32_u32(intel_dp_link_symbol_clock(crtc_state->port_clock) > > > * 72, > > > + > > > drm_dp_bw_channel_coding_efficiency(true)), > > > + mul_u32_u32(adjusted_mode->crtc_clock, 100)); > > > } > > > > > > static int intel_dp_mst_bw_overhead(const struct intel_crtc_state > > > *crtc_state, > > > @@ -160,6 +157,7 @@ static int > > > intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > > > const struct drm_display_mode *adjusted_mode = > > > _state->hw.adjusted_mode; > > > int bpp, slots = -EINVAL; > > > + int max_dpt_bpp; > > > int ret = 0; > > > > > > mst_state = drm_atomic_get_mst_topology_state(state, > > > _dp->mst_mgr); > > > @@ -180,6 +178,13 @@ static int > > > intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > > > > > > crtc_state->port_clock, > > > > > > crtc_state->lane_count); > > > > > > + max_dpt_bpp = intel_dp_mst_max_dpt_bpp(crtc_state, dsc); > > > + if (max_bpp > max_dpt_bpp) { > > > + drm_dbg_kms(>drm, "Limiting bpp to max DPT bpp (%d -> > > > %d)\n", > > > + max_bpp, max_dpt_bpp); > > > + max_bpp = max_dpt_bpp; > > > + } > > > + > > > drm_dbg_kms(>drm, "Looking for slots in range min bpp %d max > > > bpp %d\n", > > > min_bpp, max_bpp); > > > > > > @@ -191,10 +196,6 @@ static int > > > intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > > > > > >
Re: [PATCH 01/11] drm/i915/dp: Fix DSC line buffer depth programming
On Tue, Mar 26, 2024 at 12:50:17PM -0700, Manasi Navare wrote: Hi, > Hi Imre, > > Thanks for the DSC fixes. > > Would the line buf depth calculation that was getting set to 0 impact > DSC on all platforms or was this issue only specific to MTL and was > getting set correctly with older platforms? Only those configs are affected where both the source and the sink supports DSC1.2, so ADL is not affected. > We didnt notice any DSC issues/corruptions with ADL based systems. Yes, that makes sense. > The actual change makes sense, just want to confirm if this applies to > all platforms or any particular? The change will make a difference only on MTL+. > With that clarification: > > Reviewed-by: Manasi Navare Thanks. > Regards > Manasi > > On Tue, Mar 26, 2024 at 3:01 AM Nautiyal, Ankit K > wrote: > > > > > > On 3/21/2024 1:41 AM, Imre Deak wrote: > > > Fix the calculation of the DSC line buffer depth. This is limited both > > > by the source's and sink's maximum line buffer depth, but the former one > > > was not taken into account. On all Intel platform's the source's maximum > > > buffer depth is 13, so the overall limit is simply the minimum of the > > > source/sink's limit, regardless of the DSC version. > > > > > > This leaves the DSI DSC line buffer depth calculation as-is, trusting > > > VBT. > > > > > > On DSC version 1.2 for sinks reporting a maximum line buffer depth of 16 > > > the line buffer depth was incorrectly programmed as 0, leading to a > > > corruption in color gradients / lines on the decompressed screen image. > > > > > > Cc: dri-de...@lists.freedesktop.org > > > Signed-off-by: Imre Deak > > > > LGTM. > > > > Reviewed-by: Ankit Nautiyal > > > > > --- > > > drivers/gpu/drm/i915/display/intel_dp.c | 16 ++-- > > > include/drm/display/drm_dsc.h | 3 --- > > > 2 files changed, 6 insertions(+), 13 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > index af7ca00e9bc0a..dbe65651bf277 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > @@ -89,6 +89,9 @@ > > > #define DP_DSC_MAX_ENC_THROUGHPUT_0 34 > > > #define DP_DSC_MAX_ENC_THROUGHPUT_1 40 > > > > > > +/* Max DSC line buffer depth supported by HW. */ > > > +#define INTEL_DP_DSC_MAX_LINE_BUF_DEPTH 13 > > > + > > > /* DP DSC FEC Overhead factor in ppm = 1/(0.972261) = 1.028530 */ > > > #define DP_DSC_FEC_OVERHEAD_FACTOR 1028530 > > > > > > @@ -1703,7 +1706,6 @@ static int intel_dp_dsc_compute_params(const struct > > > intel_connector *connector, > > > { > > > struct drm_i915_private *i915 = to_i915(connector->base.dev); > > > struct drm_dsc_config *vdsc_cfg = _state->dsc.config; > > > - u8 line_buf_depth; > > > int ret; > > > > > > /* > > > @@ -1732,20 +1734,14 @@ static int intel_dp_dsc_compute_params(const > > > struct intel_connector *connector, > > > connector->dp.dsc_dpcd[DP_DSC_DEC_COLOR_FORMAT_CAP > > > - DP_DSC_SUPPORT] & > > > DP_DSC_RGB; > > > > > > - line_buf_depth = > > > drm_dp_dsc_sink_line_buf_depth(connector->dp.dsc_dpcd); > > > - if (!line_buf_depth) { > > > + vdsc_cfg->line_buf_depth = min(INTEL_DP_DSC_MAX_LINE_BUF_DEPTH, > > > + > > > drm_dp_dsc_sink_line_buf_depth(connector->dp.dsc_dpcd)); > > > + if (!vdsc_cfg->line_buf_depth) { > > > drm_dbg_kms(>drm, > > > "DSC Sink Line Buffer Depth invalid\n"); > > > return -EINVAL; > > > } > > > > > > - if (vdsc_cfg->dsc_version_minor == 2) > > > - vdsc_cfg->line_buf_depth = (line_buf_depth == > > > DSC_1_2_MAX_LINEBUF_DEPTH_BITS) ? > > > - DSC_1_2_MAX_LINEBUF_DEPTH_VAL : line_buf_depth; > > > - else > > > - vdsc_cfg->line_buf_depth = (line_buf_depth > > > > DSC_1_1_MAX_LINEBUF_DEPTH_BITS) ? > > > - DSC_1_1_MAX_LINEBUF_DEPTH_BITS : line_buf_depth; > > > - > > > vdsc_cfg->block_pred_enable = > > > connector->dp.dsc_dpcd[DP_DSC_BLK_PREDICTION_SUPPORT - > > > DP_DSC_SUPPORT] & > > > DP_DSC_BLK_PREDICTION_IS_SUPPORTED; > > > diff --git a/include/drm/display/drm_dsc.h b/include/drm/display/drm_dsc.h > > > index bc90273d06a62..bbbe7438473d3 100644 > > > --- a/include/drm/display/drm_dsc.h > > > +++ b/include/drm/display/drm_dsc.h > > > @@ -40,9 +40,6 @@ > > > #define DSC_PPS_RC_RANGE_MINQP_SHIFT11 > > > #define DSC_PPS_RC_RANGE_MAXQP_SHIFT6 > > > #define DSC_PPS_NATIVE_420_SHIFT1 > > > -#define DSC_1_2_MAX_LINEBUF_DEPTH_BITS 16 > > > -#define DSC_1_2_MAX_LINEBUF_DEPTH_VAL0 > > > -#define DSC_1_1_MAX_LINEBUF_DEPTH_BITS 13 > > > > > > /** > > >* struct
Re: [PATCH 09/11] drm/dp_mst: Add drm_dp_mst_aux_for_parent()
On Wed, Mar 27, 2024 at 02:30:53PM +0530, Nautiyal, Ankit K wrote: > > On 3/21/2024 1:41 AM, Imre Deak wrote: > > Add a function to get the AUX device of the parent of an MST port, used > > by a follow-up i915 patch in the patchset. > > > > Cc: Lyude Paul > > Cc: dri-de...@lists.freedesktop.org > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/display/drm_dp_mst_topology.c | 16 > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c > > b/drivers/gpu/drm/display/drm_dp_mst_topology.c > > index 6bd471a2266ce..d70f7de644371 100644 > > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c > > @@ -6004,6 +6004,22 @@ static bool drm_dp_mst_is_virtual_dpcd(struct > > drm_dp_mst_port *port) > > return false; > > } > > +/** > > + * drm_dp_mst_aux_for_parent() - Get the AUX device for an MST port's > > parent > > + * @port: MST port whose parent's AUX device is returned > > + * > > + * Return the AUX device for @port's parent or NULL if port's parent is the > > + * root port. > > + */ > > +struct drm_dp_aux *drm_dp_mst_aux_for_parent(struct drm_dp_mst_port *port) > > +{ > > + if (!port->parent || !port->parent->port_parent) > > + return NULL; > > + > > + return >parent->port_parent->aux; > > +} > > +EXPORT_SYMBOL(drm_dp_mst_aux_for_parent); > > As mentioned in previous patch, the declaration of this in the header, > got included in previous patch. Yes thanks, the header change should've been in this patch, will move it here (while applying the patches if nothing else comes up). > Regards, > > Ankit > > > + > > /** > >* drm_dp_mst_dsc_aux_for_port() - Find the correct aux for DSC > >* @port: The port to check. A leaf of the MST tree with an attached > > display.
Re: [PATCH 10/11] drm/i915/dp_mst: Make HBLANK expansion quirk work for logical ports
On Wed, Mar 27, 2024 at 01:40:58PM +0530, Nautiyal, Ankit K wrote: > > On 3/21/2024 1:41 AM, Imre Deak wrote: > > The DPCD OUI of the logical port on a Dell UHBR monitor - on which the > > AUX device is used to enable DSC - is all 0. To detect if the HBLANK > > expansion quirk is required for this monitor use the OUI of the port's > > parent instead. > > > > Since in the above case the DPCD of both the logical port and the parent > > port reports being a sink device (vs. branch device) type, read the > > proper sink/branch OUI based on the DPCD device type. > > > > This is required by a follow-up patch enabling the quirk for the above > > Dell monitor. > > > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 22 +++-- > > 1 file changed, 16 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > index 516b00f791420..76a8fb21b8e52 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > @@ -1512,23 +1512,33 @@ > > intel_dp_mst_read_decompression_port_dsc_caps(struct intel_dp *intel_dp, > > static bool detect_dsc_hblank_expansion_quirk(const struct > > intel_connector *connector) > > { > > struct drm_i915_private *i915 = to_i915(connector->base.dev); > > + struct drm_dp_aux *aux = connector->dp.dsc_decompression_aux; > > struct drm_dp_desc desc; > > u8 dpcd[DP_RECEIVER_CAP_SIZE]; > > - if (!connector->dp.dsc_decompression_aux) > > + if (!aux) > > return false; > > - if (drm_dp_read_desc(connector->dp.dsc_decompression_aux, > > -, true) < 0) > > + /* > > +* A logical port's OUI (at least for affected sinks) is all 0, so > > +* instead of that the parent port's OUI is used for identification. > > +*/ > > + if (drm_dp_mst_port_is_logical(connector->port)) { > > + aux = drm_dp_mst_aux_for_parent(connector->port); > > + if (!aux) > > + aux = >mst_port->aux; > > As I understand, we are setting connector->mst_port as intel_dp, in the > caller intel_dp_add_mst_connector so its unlikely that aux can be NULL, but > do you see if we need to check for aux? Yes, intel_connector::mst_port (always) points to the intel_dp of the MST root port, and aux will be always initialized for all the registered DP encoders/intel_dps; so mst_port->aux will always point to a valid/non-NULL AUX device. (In any case above we take the address of intel_dp::aux, which can't be NULL.) > Regards, > > Ankit > > > + } > > + > > + if (drm_dp_read_dpcd_caps(aux, dpcd) < 0) > > + return false; > > + > > + if (drm_dp_read_desc(aux, , drm_dp_is_branch(dpcd)) < 0) > > return false; > > if (!drm_dp_has_quirk(, > > DP_DPCD_QUIRK_HBLANK_EXPANSION_REQUIRES_DSC)) > > return false; > > - if (drm_dp_read_dpcd_caps(connector->dp.dsc_decompression_aux, dpcd) < > > 0) > > - return false; > > - > > if (!(dpcd[DP_RECEIVE_PORT_0_CAP_0] & DP_HBLANK_EXPANSION_CAPABLE)) > > return false;
RE: [PATCH 2/5] drm/i915/psr: Move writing early transport pipe src
> -Original Message- > From: Hogander, Jouni > Sent: Tuesday, March 19, 2024 2:33 PM > To: intel-gfx@lists.freedesktop.org > Cc: Kahola, Mika ; Hogander, Jouni > > Subject: [PATCH 2/5] drm/i915/psr: Move writing early transport pipe src > > Currently PIPE_SRCSZ_ERLY_TPT is written in > intel_display.c:intel_set_pipe_src_size. This doesn't work as > intel_set_pipe_src_size > is called only on modeset. > > Bspec: 68927 > > Fixes: 3291bbb93e16 ("drm/i915/psr: Configure PIPE_SRCSZ_ERLY_TPT for psr2 > early transport") Reviewed-by: Mika Kahola > Signed-off-by: Jouni Högander > --- > drivers/gpu/drm/i915/display/intel_display.c | 9 - > drivers/gpu/drm/i915/display/intel_psr.c | 7 +++ > 2 files changed, 7 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index d366a103a707..55c2a0fbd797 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2709,15 +2709,6 @@ static void intel_set_pipe_src_size(const struct > intel_crtc_state *crtc_state) >*/ > intel_de_write(dev_priv, PIPESRC(pipe), > PIPESRC_WIDTH(width - 1) | PIPESRC_HEIGHT(height - 1)); > - > - if (!crtc_state->enable_psr2_su_region_et) > - return; > - > - width = drm_rect_width(_state->psr2_su_area); > - height = drm_rect_height(_state->psr2_su_area); > - > - intel_de_write(dev_priv, PIPE_SRCSZ_ERLY_TPT(pipe), > -PIPESRC_WIDTH(width - 1) | PIPESRC_HEIGHT(height - 1)); > } > > static bool intel_pipe_is_interlaced(const struct intel_crtc_state > *crtc_state) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > b/drivers/gpu/drm/i915/display/intel_psr.c > index cbf9495c7072..961f92d10241 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -2018,6 +2018,7 @@ static void psr_force_hw_tracking_exit(struct intel_dp > *intel_dp) > > void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state > *crtc_state) { > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev); > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > struct intel_encoder *encoder; > @@ -2037,6 +2038,12 @@ void intel_psr2_program_trans_man_trk_ctl(const struct > intel_crtc_state *crtc_st > > intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(cpu_transcoder), > crtc_state->psr2_man_track_ctl); > + > + if (!crtc_state->enable_psr2_su_region_et) > + return; > + > + intel_de_write(dev_priv, PIPE_SRCSZ_ERLY_TPT(crtc->pipe), > +crtc_state->pipe_srcsz_early_tpt); > } > > static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state, > -- > 2.34.1
RE: [PATCH 1/5] drm/i915/psr: Calculate PIPE_SRCSZ_ERLY_TPT value
> -Original Message- > From: Hogander, Jouni > Sent: Tuesday, March 19, 2024 2:33 PM > To: intel-gfx@lists.freedesktop.org > Cc: Kahola, Mika ; Hogander, Jouni > > Subject: [PATCH 1/5] drm/i915/psr: Calculate PIPE_SRCSZ_ERLY_TPT value > > When early transport is enabled we need to write PIPE_SRCSZ_ERLY_TPT on every > flip doing selective update. This patch > calculates PIPE_SRCSZ_ERLY_TPT same way as is done for PSR2_MAN_TRK_CTL value > and stores i in intel_crtc_state- > >pipe_srcsz_early_tpt to be written later during flip. > > Bspec: 68927 > Reviewed-by: Mika Kahola > Signed-off-by: Jouni Högander > --- > .../gpu/drm/i915/display/intel_display_types.h | 2 ++ > drivers/gpu/drm/i915/display/intel_psr.c | 16 > 2 files changed, 18 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > b/drivers/gpu/drm/i915/display/intel_display_types.h > index 8b9860cefaae..ba573490fd87 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1423,6 +1423,8 @@ struct intel_crtc_state { > > u32 psr2_man_track_ctl; > > + u32 pipe_srcsz_early_tpt; > + > struct drm_rect psr2_su_area; > > /* Variable Refresh Rate state */ > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > b/drivers/gpu/drm/i915/display/intel_psr.c > index 747761efa4be..cbf9495c7072 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -2075,6 +2075,20 @@ static void psr2_man_trk_ctl_calc(struct > intel_crtc_state *crtc_state, > crtc_state->psr2_man_track_ctl = val; > } > > +static u32 psr2_pipe_srcsz_early_tpt_calc(struct intel_crtc_state > *crtc_state, > + bool full_update) > +{ > + int width, height; > + > + if (!crtc_state->enable_psr2_su_region_et || full_update) > + return 0; > + > + width = drm_rect_width(_state->psr2_su_area); > + height = drm_rect_height(_state->psr2_su_area); > + > + return PIPESRC_WIDTH(width - 1) | PIPESRC_HEIGHT(height - 1); } > + > static void clip_area_update(struct drm_rect *overlap_damage_area, >struct drm_rect *damage_area, >struct drm_rect *pipe_src) > @@ -2362,6 +2376,8 @@ int intel_psr2_sel_fetch_update(struct > intel_atomic_state *state, > > skip_sel_fetch_set_loop: > psr2_man_trk_ctl_calc(crtc_state, full_update); > + crtc_state->pipe_srcsz_early_tpt = > + psr2_pipe_srcsz_early_tpt_calc(crtc_state, full_update); > return 0; > } > > -- > 2.34.1
FAILED: Patch "drm/i915/dp: Limit SST link rate to <=8.1Gbps" failed to apply to 4.19-stable tree
The patch below does not apply to the 4.19-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From 6061811d72e14f41f71b6a025510920b187bfcca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Thu, 8 Feb 2024 17:45:52 +0200 Subject: [PATCH] drm/i915/dp: Limit SST link rate to <=8.1Gbps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Limit the link rate to HBR3 or below (<=8.1Gbps) in SST mode. UHBR (10Gbps+) link rates require 128b/132b channel encoding which we have not yet hooked up into the SST/no-sideband codepaths. Cc: sta...@vger.kernel.org Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240208154552.14545-1-ville.syrj...@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_dp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index ab415f41924d7..5045c34a16be1 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2356,6 +2356,9 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp, limits->min_rate = intel_dp_common_rate(intel_dp, 0); limits->max_rate = intel_dp_max_link_rate(intel_dp); + /* FIXME 128b/132b SST support missing */ + limits->max_rate = min(limits->max_rate, 81); + limits->min_lane_count = 1; limits->max_lane_count = intel_dp_max_lane_count(intel_dp); -- 2.43.0
FAILED: Patch "Revert "drm/i915/dsi: Do display on sequence later on icl+"" failed to apply to 4.19-stable tree
The patch below does not apply to the 4.19-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From dc524d05974f615b145404191fcf91b478950499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Tue, 16 Jan 2024 23:08:21 +0200 Subject: [PATCH] Revert "drm/i915/dsi: Do display on sequence later on icl+" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 88b065943cb583e890324d618e8d4b23460d51a3. Lenovo 82TQ is unhappy if we do the display on sequence this late. The display output shows severe corruption. It's unclear if this is a failure on our part (perhaps something to do with sending commands in LP mode after HS /video mode transmission has been started? Though the backlight on command at least seems to work) or simply that there are some commands in the sequence that are needed to be done earlier (eg. could be some DSC init stuff?). If the latter then I don't think the current Windows code would work either, but maybe this was originally tested with an older driver, who knows. Root causing this fully would likely require a lot of experimentation which isn't really feasible without direct access to the machine, so let's just accept failure and go back to the original sequence. Cc: sta...@vger.kernel.org Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10071 Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240116210821.30194-1-ville.syrj...@linux.intel.com Acked-by: Jani Nikula --- drivers/gpu/drm/i915/display/icl_dsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c index ac456a2275dba..eda4a8b885904 100644 --- a/drivers/gpu/drm/i915/display/icl_dsi.c +++ b/drivers/gpu/drm/i915/display/icl_dsi.c @@ -1155,6 +1155,7 @@ static void gen11_dsi_powerup_panel(struct intel_encoder *encoder) } intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_INIT_OTP); + intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON); /* ensure all panel commands dispatched before enabling transcoder */ wait_for_cmds_dispatched_to_panel(encoder); @@ -1255,8 +1256,6 @@ static void gen11_dsi_enable(struct intel_atomic_state *state, /* step6d: enable dsi transcoder */ gen11_dsi_enable_transcoder(encoder); - intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON); - /* step7: enable backlight */ intel_backlight_enable(crtc_state, conn_state); intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON); -- 2.43.0
FAILED: Patch "drm/i915/dp: Limit SST link rate to <=8.1Gbps" failed to apply to 5.4-stable tree
The patch below does not apply to the 5.4-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From 6061811d72e14f41f71b6a025510920b187bfcca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Thu, 8 Feb 2024 17:45:52 +0200 Subject: [PATCH] drm/i915/dp: Limit SST link rate to <=8.1Gbps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Limit the link rate to HBR3 or below (<=8.1Gbps) in SST mode. UHBR (10Gbps+) link rates require 128b/132b channel encoding which we have not yet hooked up into the SST/no-sideband codepaths. Cc: sta...@vger.kernel.org Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240208154552.14545-1-ville.syrj...@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_dp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index ab415f41924d7..5045c34a16be1 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2356,6 +2356,9 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp, limits->min_rate = intel_dp_common_rate(intel_dp, 0); limits->max_rate = intel_dp_max_link_rate(intel_dp); + /* FIXME 128b/132b SST support missing */ + limits->max_rate = min(limits->max_rate, 81); + limits->min_lane_count = 1; limits->max_lane_count = intel_dp_max_lane_count(intel_dp); -- 2.43.0
FAILED: Patch "Revert "drm/i915/dsi: Do display on sequence later on icl+"" failed to apply to 5.4-stable tree
The patch below does not apply to the 5.4-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From dc524d05974f615b145404191fcf91b478950499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Tue, 16 Jan 2024 23:08:21 +0200 Subject: [PATCH] Revert "drm/i915/dsi: Do display on sequence later on icl+" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 88b065943cb583e890324d618e8d4b23460d51a3. Lenovo 82TQ is unhappy if we do the display on sequence this late. The display output shows severe corruption. It's unclear if this is a failure on our part (perhaps something to do with sending commands in LP mode after HS /video mode transmission has been started? Though the backlight on command at least seems to work) or simply that there are some commands in the sequence that are needed to be done earlier (eg. could be some DSC init stuff?). If the latter then I don't think the current Windows code would work either, but maybe this was originally tested with an older driver, who knows. Root causing this fully would likely require a lot of experimentation which isn't really feasible without direct access to the machine, so let's just accept failure and go back to the original sequence. Cc: sta...@vger.kernel.org Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10071 Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240116210821.30194-1-ville.syrj...@linux.intel.com Acked-by: Jani Nikula --- drivers/gpu/drm/i915/display/icl_dsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c index ac456a2275dba..eda4a8b885904 100644 --- a/drivers/gpu/drm/i915/display/icl_dsi.c +++ b/drivers/gpu/drm/i915/display/icl_dsi.c @@ -1155,6 +1155,7 @@ static void gen11_dsi_powerup_panel(struct intel_encoder *encoder) } intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_INIT_OTP); + intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON); /* ensure all panel commands dispatched before enabling transcoder */ wait_for_cmds_dispatched_to_panel(encoder); @@ -1255,8 +1256,6 @@ static void gen11_dsi_enable(struct intel_atomic_state *state, /* step6d: enable dsi transcoder */ gen11_dsi_enable_transcoder(encoder); - intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON); - /* step7: enable backlight */ intel_backlight_enable(crtc_state, conn_state); intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON); -- 2.43.0
FAILED: Patch "drm/i915/dp: Limit SST link rate to <=8.1Gbps" failed to apply to 5.10-stable tree
The patch below does not apply to the 5.10-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From 6061811d72e14f41f71b6a025510920b187bfcca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Thu, 8 Feb 2024 17:45:52 +0200 Subject: [PATCH] drm/i915/dp: Limit SST link rate to <=8.1Gbps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Limit the link rate to HBR3 or below (<=8.1Gbps) in SST mode. UHBR (10Gbps+) link rates require 128b/132b channel encoding which we have not yet hooked up into the SST/no-sideband codepaths. Cc: sta...@vger.kernel.org Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240208154552.14545-1-ville.syrj...@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_dp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index ab415f41924d7..5045c34a16be1 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2356,6 +2356,9 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp, limits->min_rate = intel_dp_common_rate(intel_dp, 0); limits->max_rate = intel_dp_max_link_rate(intel_dp); + /* FIXME 128b/132b SST support missing */ + limits->max_rate = min(limits->max_rate, 81); + limits->min_lane_count = 1; limits->max_lane_count = intel_dp_max_lane_count(intel_dp); -- 2.43.0
FAILED: Patch "Revert "drm/i915/dsi: Do display on sequence later on icl+"" failed to apply to 5.10-stable tree
The patch below does not apply to the 5.10-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From dc524d05974f615b145404191fcf91b478950499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Tue, 16 Jan 2024 23:08:21 +0200 Subject: [PATCH] Revert "drm/i915/dsi: Do display on sequence later on icl+" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 88b065943cb583e890324d618e8d4b23460d51a3. Lenovo 82TQ is unhappy if we do the display on sequence this late. The display output shows severe corruption. It's unclear if this is a failure on our part (perhaps something to do with sending commands in LP mode after HS /video mode transmission has been started? Though the backlight on command at least seems to work) or simply that there are some commands in the sequence that are needed to be done earlier (eg. could be some DSC init stuff?). If the latter then I don't think the current Windows code would work either, but maybe this was originally tested with an older driver, who knows. Root causing this fully would likely require a lot of experimentation which isn't really feasible without direct access to the machine, so let's just accept failure and go back to the original sequence. Cc: sta...@vger.kernel.org Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10071 Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240116210821.30194-1-ville.syrj...@linux.intel.com Acked-by: Jani Nikula --- drivers/gpu/drm/i915/display/icl_dsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c index ac456a2275dba..eda4a8b885904 100644 --- a/drivers/gpu/drm/i915/display/icl_dsi.c +++ b/drivers/gpu/drm/i915/display/icl_dsi.c @@ -1155,6 +1155,7 @@ static void gen11_dsi_powerup_panel(struct intel_encoder *encoder) } intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_INIT_OTP); + intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON); /* ensure all panel commands dispatched before enabling transcoder */ wait_for_cmds_dispatched_to_panel(encoder); @@ -1255,8 +1256,6 @@ static void gen11_dsi_enable(struct intel_atomic_state *state, /* step6d: enable dsi transcoder */ gen11_dsi_enable_transcoder(encoder); - intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON); - /* step7: enable backlight */ intel_backlight_enable(crtc_state, conn_state); intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON); -- 2.43.0
FAILED: Patch "drm/i915/dp: Limit SST link rate to <=8.1Gbps" failed to apply to 5.15-stable tree
The patch below does not apply to the 5.15-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From 6061811d72e14f41f71b6a025510920b187bfcca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Thu, 8 Feb 2024 17:45:52 +0200 Subject: [PATCH] drm/i915/dp: Limit SST link rate to <=8.1Gbps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Limit the link rate to HBR3 or below (<=8.1Gbps) in SST mode. UHBR (10Gbps+) link rates require 128b/132b channel encoding which we have not yet hooked up into the SST/no-sideband codepaths. Cc: sta...@vger.kernel.org Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240208154552.14545-1-ville.syrj...@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_dp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index ab415f41924d7..5045c34a16be1 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2356,6 +2356,9 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp, limits->min_rate = intel_dp_common_rate(intel_dp, 0); limits->max_rate = intel_dp_max_link_rate(intel_dp); + /* FIXME 128b/132b SST support missing */ + limits->max_rate = min(limits->max_rate, 81); + limits->min_lane_count = 1; limits->max_lane_count = intel_dp_max_lane_count(intel_dp); -- 2.43.0
FAILED: Patch "drm/i915: Check before removing mm notifier" failed to apply to 5.15-stable tree
The patch below does not apply to the 5.15-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From db7bbd13f08774cde0332c705f042e327fe21e73 Mon Sep 17 00:00:00 2001 From: Nirmoy Das Date: Mon, 19 Feb 2024 13:50:47 +0100 Subject: [PATCH] drm/i915: Check before removing mm notifier Error in mmu_interval_notifier_insert() can leave a NULL notifier.mm pointer. Catch that and return early. Fixes: ed29c2691188 ("drm/i915: Fix userptr so we do not have to worry about obj->mm.lock, v7.") Cc: # v5.13+ [tursulin: Added Fixes and cc stable.] Cc: Andi Shyti Cc: Shawn Lee Signed-off-by: Nirmoy Das Reviewed-by: Rodrigo Vivi Link: https://patchwork.freedesktop.org/patch/msgid/20240219125047.28906-1-nirmoy@intel.com Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c index 0e21ce9d3e5ac..61abfb505766d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c @@ -349,6 +349,9 @@ i915_gem_userptr_release(struct drm_i915_gem_object *obj) { GEM_WARN_ON(obj->userptr.page_ref); + if (!obj->userptr.notifier.mm) + return; + mmu_interval_notifier_remove(>userptr.notifier); obj->userptr.notifier.mm = NULL; } -- 2.43.0
FAILED: Patch "Revert "drm/i915/dsi: Do display on sequence later on icl+"" failed to apply to 5.15-stable tree
The patch below does not apply to the 5.15-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From dc524d05974f615b145404191fcf91b478950499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Tue, 16 Jan 2024 23:08:21 +0200 Subject: [PATCH] Revert "drm/i915/dsi: Do display on sequence later on icl+" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 88b065943cb583e890324d618e8d4b23460d51a3. Lenovo 82TQ is unhappy if we do the display on sequence this late. The display output shows severe corruption. It's unclear if this is a failure on our part (perhaps something to do with sending commands in LP mode after HS /video mode transmission has been started? Though the backlight on command at least seems to work) or simply that there are some commands in the sequence that are needed to be done earlier (eg. could be some DSC init stuff?). If the latter then I don't think the current Windows code would work either, but maybe this was originally tested with an older driver, who knows. Root causing this fully would likely require a lot of experimentation which isn't really feasible without direct access to the machine, so let's just accept failure and go back to the original sequence. Cc: sta...@vger.kernel.org Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10071 Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240116210821.30194-1-ville.syrj...@linux.intel.com Acked-by: Jani Nikula --- drivers/gpu/drm/i915/display/icl_dsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c index ac456a2275dba..eda4a8b885904 100644 --- a/drivers/gpu/drm/i915/display/icl_dsi.c +++ b/drivers/gpu/drm/i915/display/icl_dsi.c @@ -1155,6 +1155,7 @@ static void gen11_dsi_powerup_panel(struct intel_encoder *encoder) } intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_INIT_OTP); + intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON); /* ensure all panel commands dispatched before enabling transcoder */ wait_for_cmds_dispatched_to_panel(encoder); @@ -1255,8 +1256,6 @@ static void gen11_dsi_enable(struct intel_atomic_state *state, /* step6d: enable dsi transcoder */ gen11_dsi_enable_transcoder(encoder); - intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON); - /* step7: enable backlight */ intel_backlight_enable(crtc_state, conn_state); intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON); -- 2.43.0
FAILED: Patch "drm/i915/dp: Limit SST link rate to <=8.1Gbps" failed to apply to 6.1-stable tree
The patch below does not apply to the 6.1-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From 6061811d72e14f41f71b6a025510920b187bfcca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Thu, 8 Feb 2024 17:45:52 +0200 Subject: [PATCH] drm/i915/dp: Limit SST link rate to <=8.1Gbps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Limit the link rate to HBR3 or below (<=8.1Gbps) in SST mode. UHBR (10Gbps+) link rates require 128b/132b channel encoding which we have not yet hooked up into the SST/no-sideband codepaths. Cc: sta...@vger.kernel.org Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240208154552.14545-1-ville.syrj...@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_dp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index ab415f41924d7..5045c34a16be1 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2356,6 +2356,9 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp, limits->min_rate = intel_dp_common_rate(intel_dp, 0); limits->max_rate = intel_dp_max_link_rate(intel_dp); + /* FIXME 128b/132b SST support missing */ + limits->max_rate = min(limits->max_rate, 81); + limits->min_lane_count = 1; limits->max_lane_count = intel_dp_max_lane_count(intel_dp); -- 2.43.0
FAILED: Patch "drm/i915: Check before removing mm notifier" failed to apply to 6.1-stable tree
The patch below does not apply to the 6.1-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From db7bbd13f08774cde0332c705f042e327fe21e73 Mon Sep 17 00:00:00 2001 From: Nirmoy Das Date: Mon, 19 Feb 2024 13:50:47 +0100 Subject: [PATCH] drm/i915: Check before removing mm notifier Error in mmu_interval_notifier_insert() can leave a NULL notifier.mm pointer. Catch that and return early. Fixes: ed29c2691188 ("drm/i915: Fix userptr so we do not have to worry about obj->mm.lock, v7.") Cc: # v5.13+ [tursulin: Added Fixes and cc stable.] Cc: Andi Shyti Cc: Shawn Lee Signed-off-by: Nirmoy Das Reviewed-by: Rodrigo Vivi Link: https://patchwork.freedesktop.org/patch/msgid/20240219125047.28906-1-nirmoy@intel.com Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c index 0e21ce9d3e5ac..61abfb505766d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c @@ -349,6 +349,9 @@ i915_gem_userptr_release(struct drm_i915_gem_object *obj) { GEM_WARN_ON(obj->userptr.page_ref); + if (!obj->userptr.notifier.mm) + return; + mmu_interval_notifier_remove(>userptr.notifier); obj->userptr.notifier.mm = NULL; } -- 2.43.0
FAILED: Patch "Revert "drm/i915/dsi: Do display on sequence later on icl+"" failed to apply to 6.1-stable tree
The patch below does not apply to the 6.1-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From dc524d05974f615b145404191fcf91b478950499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Tue, 16 Jan 2024 23:08:21 +0200 Subject: [PATCH] Revert "drm/i915/dsi: Do display on sequence later on icl+" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 88b065943cb583e890324d618e8d4b23460d51a3. Lenovo 82TQ is unhappy if we do the display on sequence this late. The display output shows severe corruption. It's unclear if this is a failure on our part (perhaps something to do with sending commands in LP mode after HS /video mode transmission has been started? Though the backlight on command at least seems to work) or simply that there are some commands in the sequence that are needed to be done earlier (eg. could be some DSC init stuff?). If the latter then I don't think the current Windows code would work either, but maybe this was originally tested with an older driver, who knows. Root causing this fully would likely require a lot of experimentation which isn't really feasible without direct access to the machine, so let's just accept failure and go back to the original sequence. Cc: sta...@vger.kernel.org Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10071 Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240116210821.30194-1-ville.syrj...@linux.intel.com Acked-by: Jani Nikula --- drivers/gpu/drm/i915/display/icl_dsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c index ac456a2275dba..eda4a8b885904 100644 --- a/drivers/gpu/drm/i915/display/icl_dsi.c +++ b/drivers/gpu/drm/i915/display/icl_dsi.c @@ -1155,6 +1155,7 @@ static void gen11_dsi_powerup_panel(struct intel_encoder *encoder) } intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_INIT_OTP); + intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON); /* ensure all panel commands dispatched before enabling transcoder */ wait_for_cmds_dispatched_to_panel(encoder); @@ -1255,8 +1256,6 @@ static void gen11_dsi_enable(struct intel_atomic_state *state, /* step6d: enable dsi transcoder */ gen11_dsi_enable_transcoder(encoder); - intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON); - /* step7: enable backlight */ intel_backlight_enable(crtc_state, conn_state); intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON); -- 2.43.0
FAILED: Patch "drm/i915/dp: Limit SST link rate to <=8.1Gbps" failed to apply to 6.6-stable tree
The patch below does not apply to the 6.6-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From 6061811d72e14f41f71b6a025510920b187bfcca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Thu, 8 Feb 2024 17:45:52 +0200 Subject: [PATCH] drm/i915/dp: Limit SST link rate to <=8.1Gbps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Limit the link rate to HBR3 or below (<=8.1Gbps) in SST mode. UHBR (10Gbps+) link rates require 128b/132b channel encoding which we have not yet hooked up into the SST/no-sideband codepaths. Cc: sta...@vger.kernel.org Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240208154552.14545-1-ville.syrj...@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_dp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index ab415f41924d7..5045c34a16be1 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2356,6 +2356,9 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp, limits->min_rate = intel_dp_common_rate(intel_dp, 0); limits->max_rate = intel_dp_max_link_rate(intel_dp); + /* FIXME 128b/132b SST support missing */ + limits->max_rate = min(limits->max_rate, 81); + limits->min_lane_count = 1; limits->max_lane_count = intel_dp_max_lane_count(intel_dp); -- 2.43.0
FAILED: Patch "drm/i915: Check before removing mm notifier" failed to apply to 6.6-stable tree
The patch below does not apply to the 6.6-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From db7bbd13f08774cde0332c705f042e327fe21e73 Mon Sep 17 00:00:00 2001 From: Nirmoy Das Date: Mon, 19 Feb 2024 13:50:47 +0100 Subject: [PATCH] drm/i915: Check before removing mm notifier Error in mmu_interval_notifier_insert() can leave a NULL notifier.mm pointer. Catch that and return early. Fixes: ed29c2691188 ("drm/i915: Fix userptr so we do not have to worry about obj->mm.lock, v7.") Cc: # v5.13+ [tursulin: Added Fixes and cc stable.] Cc: Andi Shyti Cc: Shawn Lee Signed-off-by: Nirmoy Das Reviewed-by: Rodrigo Vivi Link: https://patchwork.freedesktop.org/patch/msgid/20240219125047.28906-1-nirmoy@intel.com Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c index 0e21ce9d3e5ac..61abfb505766d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c @@ -349,6 +349,9 @@ i915_gem_userptr_release(struct drm_i915_gem_object *obj) { GEM_WARN_ON(obj->userptr.page_ref); + if (!obj->userptr.notifier.mm) + return; + mmu_interval_notifier_remove(>userptr.notifier); obj->userptr.notifier.mm = NULL; } -- 2.43.0
FAILED: Patch "drm/i915: Check before removing mm notifier" failed to apply to 6.7-stable tree
The patch below does not apply to the 6.7-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From db7bbd13f08774cde0332c705f042e327fe21e73 Mon Sep 17 00:00:00 2001 From: Nirmoy Das Date: Mon, 19 Feb 2024 13:50:47 +0100 Subject: [PATCH] drm/i915: Check before removing mm notifier Error in mmu_interval_notifier_insert() can leave a NULL notifier.mm pointer. Catch that and return early. Fixes: ed29c2691188 ("drm/i915: Fix userptr so we do not have to worry about obj->mm.lock, v7.") Cc: # v5.13+ [tursulin: Added Fixes and cc stable.] Cc: Andi Shyti Cc: Shawn Lee Signed-off-by: Nirmoy Das Reviewed-by: Rodrigo Vivi Link: https://patchwork.freedesktop.org/patch/msgid/20240219125047.28906-1-nirmoy@intel.com Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c index 0e21ce9d3e5ac..61abfb505766d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c @@ -349,6 +349,9 @@ i915_gem_userptr_release(struct drm_i915_gem_object *obj) { GEM_WARN_ON(obj->userptr.page_ref); + if (!obj->userptr.notifier.mm) + return; + mmu_interval_notifier_remove(>userptr.notifier); obj->userptr.notifier.mm = NULL; } -- 2.43.0
✗ Fi.CI.BAT: failure for QGV/SAGV related fixes (rev9)
== Series Details == Series: QGV/SAGV related fixes (rev9) URL : https://patchwork.freedesktop.org/series/126962/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14489 -> Patchwork_126962v9 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_126962v9 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_126962v9, 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_126962v9/index.html Participating hosts (38 -> 36) -- Additional (2): bat-dg1-7 fi-kbl-8809g Missing(4): bat-arls-4 bat-mtlp-8 fi-glk-j4005 fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_126962v9: ### IGT changes ### Possible regressions * igt@i915_selftest@live@gt_heartbeat: - bat-dg2-9: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-9/igt@i915_selftest@live@gt_heartbeat.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-9/igt@i915_selftest@live@gt_heartbeat.html Known issues Here are the changes found in Patchwork_126962v9 that come from known issues: ### CI changes ### Issues hit * boot: - bat-arls-3: [PASS][3] -> [FAIL][4] ([i915#10234]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-arls-3/boot.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-arls-3/boot.html Possible fixes * boot: - bat-dg2-11: [FAIL][5] ([i915#10491]) -> [PASS][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-11/boot.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-11/boot.html ### IGT changes ### Issues hit * igt@gem_mmap@basic: - bat-dg1-7: NOTRUN -> [SKIP][7] ([i915#4083]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg1-7/igt@gem_m...@basic.html - bat-dg2-11: NOTRUN -> [SKIP][8] ([i915#4083]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-11/igt@gem_m...@basic.html * igt@gem_tiled_fence_blits@basic: - bat-dg1-7: NOTRUN -> [SKIP][9] ([i915#4077]) +2 other tests skip [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg1-7/igt@gem_tiled_fence_bl...@basic.html - bat-dg2-11: NOTRUN -> [SKIP][10] ([i915#4077]) +2 other tests skip [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-11/igt@gem_tiled_fence_bl...@basic.html * igt@gem_tiled_pread_basic: - bat-dg1-7: NOTRUN -> [SKIP][11] ([i915#4079]) +1 other test skip [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg1-7/igt@gem_tiled_pread_basic.html - bat-dg2-11: NOTRUN -> [SKIP][12] ([i915#4079]) +1 other test skip [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-11/igt@gem_tiled_pread_basic.html * igt@i915_pm_rps@basic-api: - bat-dg1-7: NOTRUN -> [SKIP][13] ([i915#6621]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg1-7/igt@i915_pm_...@basic-api.html - bat-dg2-11: NOTRUN -> [SKIP][14] ([i915#6621]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-11/igt@i915_pm_...@basic-api.html * igt@i915_selftest@live@gt_mocs: - bat-adln-1: [PASS][15] -> [INCOMPLETE][16] ([i915#10072]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-adln-1/igt@i915_selftest@live@gt_mocs.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-adln-1/igt@i915_selftest@live@gt_mocs.html * igt@i915_selftest@live@gt_tlb: - bat-dg2-14: [PASS][17] -> [ABORT][18] ([i915#10592]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-14/igt@i915_selftest@live@gt_tlb.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-14/igt@i915_selftest@live@gt_tlb.html * igt@kms_addfb_basic@addfb25-x-tiled-mismatch-legacy: - bat-dg1-7: NOTRUN -> [SKIP][19] ([i915#4212]) +7 other tests skip [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg1-7/igt@kms_addfb_ba...@addfb25-x-tiled-mismatch-legacy.html - bat-dg2-11: NOTRUN -> [SKIP][20] ([i915#4212]) +7 other tests skip [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126962v9/bat-dg2-11/igt@kms_addfb_ba...@addfb25-x-tiled-mismatch-legacy.html * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy: - bat-dg2-11: NOTRUN ->
FAILED: Patch "drm/i915/psr: Only allow PSR in LPSP mode on HSW non-ULT" failed to apply to 6.8-stable tree
The patch below does not apply to the 6.8-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From 94501c3ca6400e463ff6cc0c9cf4a2feb6a9205d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Thu, 18 Jan 2024 23:21:31 +0200 Subject: [PATCH] drm/i915/psr: Only allow PSR in LPSP mode on HSW non-ULT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On HSW non-ULT (or at least on Dell Latitude E6540) external displays start to flicker when we enable PSR on the eDP. We observe a much higher SR and PC6 residency than should be possible with an external display, and indeen much higher than what we observe with eDP disabled and only the external display enabled. Looks like the hardware is somehow ignoring the fact that the external display is active during PSR. I wasn't able to redproduce this on my HSW ULT machine, or BDW. So either there's something specific about this particular laptop (eg. some unknown firmware thing) or the issue is limited to just non-ULT HSW systems. All known registers that could affect this look perfectly reasonable on the affected machine. As a workaround let's unmask the LPSP event to prevent PSR entry except while in LPSP mode (only pipe A + eDP active). This will prevent PSR entry entirely when multiple pipes are active. The one slight downside is that we now also prevent PSR entry when driving eDP with pipe B or C, but I think that's a reasonable tradeoff to avoid having to implement a more complex workaround. Cc: sta...@vger.kernel.org Fixes: 783d8b80871f ("drm/i915/psr: Re-enable PSR1 on hsw/bdw") Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10092 Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240118212131.31868-1-ville.syrj...@linux.intel.com Reviewed-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_psr.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 696d5d32ca9db..1010b8c405df2 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1544,8 +1544,18 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, * can rely on frontbuffer tracking. */ mask = EDP_PSR_DEBUG_MASK_MEMUP | - EDP_PSR_DEBUG_MASK_HPD | - EDP_PSR_DEBUG_MASK_LPSP; + EDP_PSR_DEBUG_MASK_HPD; + + /* +* For some unknown reason on HSW non-ULT (or at least on +* Dell Latitude E6540) external displays start to flicker +* when PSR is enabled on the eDP. SR/PC6 residency is much +* higher than should be possible with an external display. +* As a workaround leave LPSP unmasked to prevent PSR entry +* when external displays are active. +*/ + if (DISPLAY_VER(dev_priv) >= 8 || IS_HASWELL_ULT(dev_priv)) + mask |= EDP_PSR_DEBUG_MASK_LPSP; if (DISPLAY_VER(dev_priv) < 20) mask |= EDP_PSR_DEBUG_MASK_MAX_SLEEP; -- 2.43.0
FAILED: Patch "drm/i915/dp: Limit SST link rate to <=8.1Gbps" failed to apply to 6.8-stable tree
The patch below does not apply to the 6.8-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From 6061811d72e14f41f71b6a025510920b187bfcca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Thu, 8 Feb 2024 17:45:52 +0200 Subject: [PATCH] drm/i915/dp: Limit SST link rate to <=8.1Gbps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Limit the link rate to HBR3 or below (<=8.1Gbps) in SST mode. UHBR (10Gbps+) link rates require 128b/132b channel encoding which we have not yet hooked up into the SST/no-sideband codepaths. Cc: sta...@vger.kernel.org Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240208154552.14545-1-ville.syrj...@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_dp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index ab415f41924d7..5045c34a16be1 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2356,6 +2356,9 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp, limits->min_rate = intel_dp_common_rate(intel_dp, 0); limits->max_rate = intel_dp_max_link_rate(intel_dp); + /* FIXME 128b/132b SST support missing */ + limits->max_rate = min(limits->max_rate, 81); + limits->min_lane_count = 1; limits->max_lane_count = intel_dp_max_lane_count(intel_dp); -- 2.43.0
FAILED: Patch "drm/i915: Check before removing mm notifier" failed to apply to 6.8-stable tree
The patch below does not apply to the 6.8-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From db7bbd13f08774cde0332c705f042e327fe21e73 Mon Sep 17 00:00:00 2001 From: Nirmoy Das Date: Mon, 19 Feb 2024 13:50:47 +0100 Subject: [PATCH] drm/i915: Check before removing mm notifier Error in mmu_interval_notifier_insert() can leave a NULL notifier.mm pointer. Catch that and return early. Fixes: ed29c2691188 ("drm/i915: Fix userptr so we do not have to worry about obj->mm.lock, v7.") Cc: # v5.13+ [tursulin: Added Fixes and cc stable.] Cc: Andi Shyti Cc: Shawn Lee Signed-off-by: Nirmoy Das Reviewed-by: Rodrigo Vivi Link: https://patchwork.freedesktop.org/patch/msgid/20240219125047.28906-1-nirmoy@intel.com Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c index 0e21ce9d3e5ac..61abfb505766d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c @@ -349,6 +349,9 @@ i915_gem_userptr_release(struct drm_i915_gem_object *obj) { GEM_WARN_ON(obj->userptr.page_ref); + if (!obj->userptr.notifier.mm) + return; + mmu_interval_notifier_remove(>userptr.notifier); obj->userptr.notifier.mm = NULL; } -- 2.43.0
FAILED: Patch "Revert "drm/i915/dsi: Do display on sequence later on icl+"" failed to apply to 6.8-stable tree
The patch below does not apply to the 6.8-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to . Thanks, Sasha -- original commit in Linus's tree -- >From dc524d05974f615b145404191fcf91b478950499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Tue, 16 Jan 2024 23:08:21 +0200 Subject: [PATCH] Revert "drm/i915/dsi: Do display on sequence later on icl+" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 88b065943cb583e890324d618e8d4b23460d51a3. Lenovo 82TQ is unhappy if we do the display on sequence this late. The display output shows severe corruption. It's unclear if this is a failure on our part (perhaps something to do with sending commands in LP mode after HS /video mode transmission has been started? Though the backlight on command at least seems to work) or simply that there are some commands in the sequence that are needed to be done earlier (eg. could be some DSC init stuff?). If the latter then I don't think the current Windows code would work either, but maybe this was originally tested with an older driver, who knows. Root causing this fully would likely require a lot of experimentation which isn't really feasible without direct access to the machine, so let's just accept failure and go back to the original sequence. Cc: sta...@vger.kernel.org Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10071 Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20240116210821.30194-1-ville.syrj...@linux.intel.com Acked-by: Jani Nikula --- drivers/gpu/drm/i915/display/icl_dsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c index ac456a2275dba..eda4a8b885904 100644 --- a/drivers/gpu/drm/i915/display/icl_dsi.c +++ b/drivers/gpu/drm/i915/display/icl_dsi.c @@ -1155,6 +1155,7 @@ static void gen11_dsi_powerup_panel(struct intel_encoder *encoder) } intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_INIT_OTP); + intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON); /* ensure all panel commands dispatched before enabling transcoder */ wait_for_cmds_dispatched_to_panel(encoder); @@ -1255,8 +1256,6 @@ static void gen11_dsi_enable(struct intel_atomic_state *state, /* step6d: enable dsi transcoder */ gen11_dsi_enable_transcoder(encoder); - intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON); - /* step7: enable backlight */ intel_backlight_enable(crtc_state, conn_state); intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON); -- 2.43.0
[PATCH i-g-t] lib/kunit: Read results from debugfs
KUnit can provide KTAP reports from test modules via debugfs files, one per test suite. Using that source of test results instead of extracting them from dmesg, where they may be interleaved with other kernel messages, seems more easy to handle and less error prone. Switch to it. If KUnit debugfs support is found not configured then fall back to legacy processing path. Signed-off-by: Janusz Krzysztofik --- lib/igt_kmod.c | 143 - 1 file changed, 105 insertions(+), 38 deletions(-) diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c index 1ec9c8a602..a5b170ca9c 100644 --- a/lib/igt_kmod.c +++ b/lib/igt_kmod.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -39,6 +40,7 @@ #include "igt_aux.h" #include "igt_core.h" +#include "igt_debugfs.h" #include "igt_kmod.h" #include "igt_ktap.h" #include "igt_sysfs.h" @@ -864,6 +866,31 @@ static int open_parameters(const char *module_name) return open(path, O_RDONLY); } +static DIR *kunit_debugfs_open(void) +{ + const char *debugfs_path = igt_debugfs_mount(); + int debugfs_fd, kunit_fd; + DIR *kunit_dir; + + if (igt_debug_on(!debugfs_path)) + return NULL; + + debugfs_fd = open(debugfs_path, O_DIRECTORY); + if (igt_debug_on(debugfs_fd < 0)) + return NULL; + + kunit_fd = openat(debugfs_fd, "kunit", O_DIRECTORY); + close(debugfs_fd); + if (igt_debug_on(kunit_fd < 0)) + return NULL; + + kunit_dir = fdopendir(kunit_fd); + if (igt_debug_on(!kunit_dir)) + close(kunit_fd); + + return kunit_dir; +} + static bool kunit_set_filtering(const char *filter_glob, const char *filter, const char *filter_action) { @@ -1071,23 +1098,48 @@ static void kunit_results_free(struct igt_list_head *results, free(*suite_name); } -static int kunit_get_results(struct igt_list_head *results, int kmsg_fd, -struct igt_ktap_results **ktap) +static int kunit_get_results(struct igt_list_head *results, int debugfs_fd, +const char *suite, struct igt_ktap_results **ktap) { - int err; + FILE *results_stream; + int ret, results_fd; + char *buf = NULL; + size_t size = 0; + ssize_t len; + + if (igt_debug_on((ret = openat(debugfs_fd, suite, O_DIRECTORY), ret < 0))) + return ret; + + results_fd = openat(ret, "results", O_RDONLY); + close(ret); + if (igt_debug_on(results_fd < 0)) + return results_fd; + + results_stream = fdopen(results_fd, "r"); + if (igt_debug_on(!results_stream)) { + close(results_fd); + return -errno; + } *ktap = igt_ktap_alloc(results); - if (igt_debug_on(!*ktap)) - return -ENOMEM; + if (igt_debug_on(!*ktap)) { + ret = -ENOMEM; + goto out_fclose; + } + + while (len = getline(, , results_stream), len > 0) { + ret = igt_ktap_parse(buf, *ktap); + if (ret != -EINPROGRESS) + break; + } - do - igt_debug_on((err = kunit_kmsg_result_get(results, NULL, kmsg_fd, *ktap), - err && err != -EINPROGRESS)); - while (err == -EINPROGRESS); + free(buf); igt_ktap_free(ktap); +out_fclose: + fclose(results_stream); - return err; + return ret; } static void __igt_kunit_legacy(struct igt_ktest *tst, @@ -1101,7 +1153,13 @@ static void __igt_kunit_legacy(struct igt_ktest *tst, pthread_mutexattr_t attr; IGT_LIST_HEAD(results); unsigned long taints; - int ret; + int flags, ret; + + igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n"); + + igt_skip_on((flags = fcntl(tst->kmsg, F_GETFL, 0), flags < 0)); + igt_skip_on_f(fcntl(tst->kmsg, F_SETFL, flags & ~O_NONBLOCK) == -1, + "Could not set /dev/kmsg to blocking mode\n"); igt_skip_on(lseek(tst->kmsg, 0, SEEK_END) < 0); @@ -1224,30 +1282,20 @@ static void __igt_kunit_legacy(struct igt_ktest *tst, igt_skip_on_f(ret, "KTAP parser failed\n"); } -static void kunit_get_tests_timeout(int signal) -{ - igt_skip("Timed out while trying to extract a list of KUnit test cases from /dev/kmsg\n"); -} - static bool kunit_get_tests(struct igt_list_head *tests, struct igt_ktest *tst, const char *suite, const char *opts, + DIR *debugfs_dir, struct igt_ktap_results **ktap) { - struct sigaction sigalrm = { .sa_handler = kunit_get_tests_timeout, }, -*saved; struct igt_ktap_result *r, *rn; + struct dirent *subdir;
Re: [RFC 0/5] Introduce drm sharpening property
On Wed, 27 Mar 2024 07:11:48 + "Garg, Nemesa" wrote: > > -Original Message- > > From: Pekka Paalanen > > Sent: Wednesday, March 13, 2024 3:07 PM > > To: Garg, Nemesa > > Cc: Simon Ser ; intel-gfx@lists.freedesktop.org; dri- > > de...@lists.freedesktop.org; G M, Adarsh > > Subject: Re: [RFC 0/5] Introduce drm sharpening property > > > > On Tue, 12 Mar 2024 16:26:00 +0200 > > Pekka Paalanen wrote: > > > > > On Tue, 12 Mar 2024 08:30:34 + > > > "Garg, Nemesa" wrote: > > > > > > > This KMS property is not implementing any formula > > > > > > Sure it is. Maybe Intel just does not want to tell what the algorithm > > > is, or maybe it's even patented. > > > > > > > and the values > > > > that are being used are based on empirical analysis and certain > > > > experiments done on the hardware. These values are fixed and is not > > > > expected to change and this can change from vendor to vendor. The > > > > client can choose any sharpness value on the scale and on the basis > > > > of it the sharpness will be set. The sharpness effect can be changed > > > > from content to content and from display to display so user needs to > > > > adjust the optimum intensity value so as to get good experience on > > > > the screen. > > > > > > > > > > IOW, it's an opaque box operation, and there is no way to reproduce > > > its results without the specific Intel hardware. Definitely no way to > > > reproduce its results in free open source software alone. > > > > > > Such opaque box operations can only occur after KMS blending, at the > > > CRTC or later stage. They cannot appear before blending, not in the > > > new KMS color pipeline design at least. The reason is that the modern > > > way to use KMS planes is opportunistic composition off-loading. > > > Opportunistic means that userspace decides from time to time whether > > > it composes the final picture using KMS or some other rendering method > > > (usually GPU and shaders). Since userspace will arbitrarily switch > > > between KMS and render composition, both must result in the exact same > > > image, or end users will observe unwanted flicker. > > > > > > Such opaque box operations are fine after blending, because there they > > > can be configured once and remain on forever. No switching, no flicker. > > > > If you want to see how sharpness property would apply in Wayland design, it > > would be in step 5 "Adjust (settings UI)" of > > https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/color- > > management-model.md#compositor-color-management-model > > > > To relate that diagram to KMS color processing, you can identify step 3 > > "Compose" > > as the KMS blending step. Everything before step 3 happens in KMS plane > > color > > processing, and steps 4-5 happen in KMS CRTC color processing. > > > > Sharpening would essentially be a "compositor color effect", it just > > happens to be > > implementable only by specific Intel hardware. > > > > If a color effect is dynamic or content-dependant, it will preclude > > colorimetric > > monitor calibration. > > > > > > Thanks, > > pq > > > > > > > Where does "sharpeness" operation occur in the Intel color processing > > > chain? Is it before or after blending? > > > > Thank you for detail explanation and link. > Sharpness operation occur post blending in CRTC ie on the final > composed output after blending . Yes Pekka you are right as per the > diagram it is done at step 5 "Adjust (settings UI)"). I will also document > this thing > along with documentation change. > > > > What kind of transfer characteristics does it expect from the image, > > > and can those be realized with KMS CRTC properties if KMS is > > > configured such that the blending happens using some other > > > characteristics > > (e.g. > > > blending in optical space)? > > > > The filter values are not dependent/calculated on the inputs of > image but depending on the blending space and other inputs the > blended output gets changed and the sharpness is applied post > blending so according to the content user needs to adjust the > strength value to get the better visual effect. So tuning of sharpness > strength > may be needed by user based on the input contents and blending policy > to get the desired experience. > > > > What about SDR vs. HDR imagery? > > > > The interface can be used for both HDR and SDR. The effect is more prominent > for SDR use cases. > For HDR filter values and tap value may change. Who will be providing these values? The kernel driver cannot know if it is dealing with SDR or HDR or which transfer function is in effect at that point of the post-blending color pipeline. If the UAPI is one "strength" value, then how can it work? Maybe the UAPI needs more controls, if not providing all "filter and tap" values directly. Maybe all the filter and tap values should be provided by userspace? Thanks, pq pgplA16SRjlrS.pgp Description: OpenPGP
✗ Fi.CI.BAT: failure for drm/i915/gt: Report full vm address range (rev5)
== Series Details == Series: drm/i915/gt: Report full vm address range (rev5) URL : https://patchwork.freedesktop.org/series/131095/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14489 -> Patchwork_131095v5 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_131095v5 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_131095v5, 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_131095v5/index.html Participating hosts (38 -> 35) -- Additional (1): fi-blb-e6850 Missing(4): bat-arls-4 bat-kbl-2 fi-bsw-nick fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_131095v5: ### IGT changes ### Possible regressions * igt@gem_busy@busy@all-engines: - fi-blb-e6850: NOTRUN -> [ABORT][1] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131095v5/fi-blb-e6850/igt@gem_busy@b...@all-engines.html - fi-pnv-d510:[PASS][2] -> [ABORT][3] [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/fi-pnv-d510/igt@gem_busy@b...@all-engines.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131095v5/fi-pnv-d510/igt@gem_busy@b...@all-engines.html - fi-ivb-3770:[PASS][4] -> [ABORT][5] [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/fi-ivb-3770/igt@gem_busy@b...@all-engines.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131095v5/fi-ivb-3770/igt@gem_busy@b...@all-engines.html - fi-elk-e7500: [PASS][6] -> [ABORT][7] [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/fi-elk-e7500/igt@gem_busy@b...@all-engines.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131095v5/fi-elk-e7500/igt@gem_busy@b...@all-engines.html - fi-ilk-650: [PASS][8] -> [ABORT][9] [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/fi-ilk-650/igt@gem_busy@b...@all-engines.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131095v5/fi-ilk-650/igt@gem_busy@b...@all-engines.html * igt@gem_softpin@allocator-basic: - bat-dg2-14: [PASS][10] -> [FAIL][11] +3 other tests fail [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-14/igt@gem_soft...@allocator-basic.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131095v5/bat-dg2-14/igt@gem_soft...@allocator-basic.html * igt@gem_softpin@allocator-basic-reserve: - bat-atsm-1: [PASS][12] -> [FAIL][13] +2 other tests fail [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-atsm-1/igt@gem_soft...@allocator-basic-reserve.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131095v5/bat-atsm-1/igt@gem_soft...@allocator-basic-reserve.html - bat-dg2-9: [PASS][14] -> [FAIL][15] +2 other tests fail [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-9/igt@gem_soft...@allocator-basic-reserve.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131095v5/bat-dg2-9/igt@gem_soft...@allocator-basic-reserve.html - bat-dg2-8: [PASS][16] -> [FAIL][17] +2 other tests fail [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-8/igt@gem_soft...@allocator-basic-reserve.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131095v5/bat-dg2-8/igt@gem_soft...@allocator-basic-reserve.html Known issues Here are the changes found in Patchwork_131095v5 that come from known issues: ### IGT changes ### Issues hit * igt@debugfs_test@basic-hwmon: - fi-blb-e6850: NOTRUN -> [SKIP][18] [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131095v5/fi-blb-e6850/igt@debugfs_t...@basic-hwmon.html Possible fixes * igt@i915_selftest@live@ring_submission: - bat-dg2-14: [ABORT][19] ([i915#10366]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-14/igt@i915_selftest@live@ring_submission.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131095v5/bat-dg2-14/igt@i915_selftest@live@ring_submission.html [i915#10366]: https://gitlab.freedesktop.org/drm/intel/issues/10366 Build changes - * Linux: CI_DRM_14489 -> Patchwork_131095v5 CI-20190529: 20190529 CI_DRM_14489: f9c56f1a03b5c35488671e4ffe61e28b12ffe163 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7785: 7785 Patchwork_131095v5: f9c56f1a03b5c35488671e4ffe61e28b12ffe163 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits 65268abd84f6 drm/i915/gt: Report full vm address range == Logs == For more
Re: [PATCH] drm/i915/hwmon: Remove i915_hwmon_unregister() during driver unbind
Hi Badal, On Wednesday, 27 March 2024 05:30:17 CET Nilawar, Badal wrote: > > On 27-03-2024 02:58, Krzysztofik, Janusz wrote: > > On Tuesday, 26 March 2024 13:48:38 CET Badal Nilawar wrote: > >> i915_hwmon and its resources are managed resources of i915 dev. > >> During i915 driver unregister flow the function i915_hwmon_unregister() > >> explicitly makes i915_hwmon resource NULL. This happen before > >> hwmon is actually unregistered. Doing so may cause UAF if hwmon > >> sysfs is being accessed: > >> > >> <7> [518.386591] i915 :03:00.0: [drm] intel_gt_set_wedged called from > >> intel_gt_set_wedged_on_fini+0xd/0x30 [i915] > >> <7> [518.471128] i915 :03:00.0: [drm:drm_client_release] drm_fb_helper > >> <4> [518.501476] general protection fault, probably for non-canonical > >> address 0x6b6b6b6b6b6b6dbf: [#1] PREEMPT SMP NOPTI > >> <4> [518.512264] CPU: 6 PID: 679 Comm: prometheus-node Tainted: G U > >> 6.9.0-rc1-CI_DRM_14482-g4a8fabcf2f1a+ #1 > >> <4> [518.522951] Hardware name: Intel Corporation Raptor Lake Client > >> Platform/RPL-S ADP-S DDR5 UDIMM CRB, BIOS RPLSFWI1.R00.4221.A00.2305271351 > >> 05/27/2023 > >> <4> [518.536217] RIP: 0010:hwm_energy+0x2b/0x100 [i915] > >> <4> [518.541159] Code: 48 89 e5 41 57 41 56 41 55 41 54 53 48 89 fb 48 83 > >> e4 f0 48 83 ec 10 4c 8b 77 08 4c 8b 2f 8b 7f 34 48 89 74 24 08 85 ff 78 2b > >> <45> 8b bd 54 02 00 00 49 8b 7e 18 e8 35 e4 ea ff 49 89 c4 48 85 c0 > >> <4> [518.559746] RSP: 0018:c900077efd00 EFLAGS: 00010202 > >> <4> [518.564943] RAX: RBX: 8881e3078428 RCX: > >> > >> <4> [518.572025] RDX: 0001 RSI: c900077efda0 RDI: > >> 6b6b6b6b > >> <4> [518.579107] RBP: c900077efd40 R08: c900077efda0 R09: > >> 0001 > >> <4> [518.586186] R10: 0001 R11: 88810c19aa00 R12: > >> 888243e1a010 > >> <4> [518.593264] R13: 6b6b6b6b6b6b6b6b R14: 6b6b6b6b6b6b6b6b R15: > >> 8881e3078428 > >> <4> [518.600343] FS: 7f9def400700() GS:8d10() > >> knlGS: > >> <4> [518.608373] CS: 0010 DS: ES: CR0: 80050033 > >> <4> [518.614077] CR2: 564f19bff288 CR3: 000119f94000 CR4: > >> 00f50ef0 > >> <4> [518.621158] PKRU: 5554 > >> <4> [518.623858] Call Trace: > >> <4> [518.626303] > >> <4> [518.628400] ? __die_body+0x1a/0x60 > >> <4> [518.631881] ? die_addr+0x38/0x60 > >> <4> [518.635182] ? exc_general_protection+0x1a1/0x400 > >> <4> [518.639862] ? asm_exc_general_protection+0x26/0x30 > >> <4> [518.644710] ? hwm_energy+0x2b/0x100 [i915] > >> <4> [518.649007] hwm_read+0x9a/0x310 [i915] > >> <4> [518.652953] hwmon_attr_show+0x36/0x140 > >> <4> [518.656775] dev_attr_show+0x15/0x60 > > > > I don't think that's a good example of what you are talking about in your > > commit description. I haven't looked throughout the i915 code to find out > > who > > actually uses that i915->hwmon pointer and when, but while looking at the > > hwmon code that now fails on sysfs access, I haven't noticed any use of > > i915->hwmon. > > i915_hwmon is main structure and I think issue is ddat is invalid. > > struct hwm_drvdata { > struct i915_hwmon *hwmon; > struct intel_uncore *uncore; > struct device *hwmon_dev; > struct hwm_energy_info ei; /* Energy info for energy1_input */ > char name[12]; > int gt_n; > bool reset_in_progress; > wait_queue_head_t waitq; > }; > > struct i915_hwmon { > struct hwm_drvdata ddat; > struct hwm_drvdata ddat_gt[I915_MAX_GT]; > struct mutex hwmon_lock;/* counter overflow logic and rmw */ > struct hwm_reg rg; > int scl_shift_power; > int scl_shift_energy; > int scl_shift_time; > }; > > (gdb) list *hwm_energy+0x2b > 0x161f8b is in hwm_energy (drivers/gpu/drm/i915/i915_hwmon.c:134). > 129 struct hwm_energy_info *ei = >ei; > 130 intel_wakeref_t wakeref; > 131 i915_reg_t rgaddr; > 132 u32 reg_val; > 133 > 134 if (ddat->gt_n >= 0) > 135 rgaddr = hwmon->rg.energy_status_tile; > 136 else > 137 rgaddr = hwmon->rg.energy_status_all; > 138 For me, that still doesn't explain why you think that i915->hwmon reset to NULL on i915 driver unregister can be the root cause of the reported UAF in hwmon sysfs and this patch is going to fix that UAF issue. I can see no references to i915->hwmon in that code, and even then, that's not NULL what is reported here as non-canonical address. The code is using a no longer valid pointer to an already freed (and poisoned) memory. > > > > > I think that instead of dropping i915_hwmon_unregister() we have to actually > > unregister hwmon in the body of that function, which is called from > > i915_driver_unregister() intended for closing user interfaces, then called > >
Re: [PATCH 09/11] drm/dp_mst: Add drm_dp_mst_aux_for_parent()
On 3/21/2024 1:41 AM, Imre Deak wrote: Add a function to get the AUX device of the parent of an MST port, used by a follow-up i915 patch in the patchset. Cc: Lyude Paul Cc: dri-de...@lists.freedesktop.org Signed-off-by: Imre Deak --- drivers/gpu/drm/display/drm_dp_mst_topology.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index 6bd471a2266ce..d70f7de644371 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -6004,6 +6004,22 @@ static bool drm_dp_mst_is_virtual_dpcd(struct drm_dp_mst_port *port) return false; } +/** + * drm_dp_mst_aux_for_parent() - Get the AUX device for an MST port's parent + * @port: MST port whose parent's AUX device is returned + * + * Return the AUX device for @port's parent or NULL if port's parent is the + * root port. + */ +struct drm_dp_aux *drm_dp_mst_aux_for_parent(struct drm_dp_mst_port *port) +{ + if (!port->parent || !port->parent->port_parent) + return NULL; + + return >parent->port_parent->aux; +} +EXPORT_SYMBOL(drm_dp_mst_aux_for_parent); As mentioned in previous patch, the declaration of this in the header, got included in previous patch. Regards, Ankit + /** * drm_dp_mst_dsc_aux_for_port() - Find the correct aux for DSC * @port: The port to check. A leaf of the MST tree with an attached display.
Re: [PATCH 11/11] drm/i915/dp_mst: Enable HBLANK expansion quirk for UHBR rates
On 3/21/2024 1:41 AM, Imre Deak wrote: Enabling the 5k@60Hz uncompressed mode on the MediaTek/Dell U3224KBA monitor results in a blank screen, at least on MTL platforms on UHBR link rates with some (<30) uncompressed bpp values. Enabling compression fixes the problem, so do that for now. Windows enables DSC always if the sink supports it and forcing it to enable the mode without compression leads to the same problem above (which suggests a panel issue with uncompressed mode). The same 5k mode on non-UHBR link rates is not affected and lower resolution modes are not affected either. The problem is similar to the one fixed by the HBLANK expansion quirk on Synaptics hubs, with the difference that the problematic mode has a longer HBLANK duration. Also the monitor doesn't report supporting HBLANK expansion; either its internal MST hub does the expansion internally - similarly to the Synaptics hub - or the issue has another root cause, but still related to the mode's short HBLANK duration. Enable the quirk for the monitor adjusting the detection for the above differences. Cc: dri-de...@lists.freedesktop.org Signed-off-by: Imre Deak LGTM. Reviewed-by: Ankit Nautiyal --- drivers/gpu/drm/display/drm_dp_helper.c | 2 ++ drivers/gpu/drm/i915/display/intel_dp_mst.c | 22 + 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c index f5d4be8978660..3e8e1bb59dea3 100644 --- a/drivers/gpu/drm/display/drm_dp_helper.c +++ b/drivers/gpu/drm/display/drm_dp_helper.c @@ -2281,6 +2281,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = { { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) }, /* Synaptics DP1.4 MST hubs require DSC for some modes on which it applies HBLANK expansion. */ { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_HBLANK_EXPANSION_REQUIRES_DSC) }, + /* MediaTek panels (at least in U3224KBA) require DSC for modes with a short HBLANK on UHBR links. */ + { OUI(0x00, 0x0C, 0xE7), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_HBLANK_EXPANSION_REQUIRES_DSC) }, /* Apple MacBookPro 2017 15 inch eDP Retina panel reports too low DP_MAX_LINK_RATE */ { OUI(0x00, 0x10, 0xfa), DEVICE_ID(101, 68, 21, 101, 98, 97), false, BIT(DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS) }, }; diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 76a8fb21b8e52..b5224fe6cc16b 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -407,15 +407,22 @@ static int mode_hblank_period_ns(const struct drm_display_mode *mode) static bool hblank_expansion_quirk_needs_dsc(const struct intel_connector *connector, -const struct intel_crtc_state *crtc_state) +const struct intel_crtc_state *crtc_state, +const struct link_config_limits *limits) { const struct drm_display_mode *adjusted_mode = _state->hw.adjusted_mode; + bool is_uhbr_sink = connector->mst_port && + drm_dp_uhbr_channel_coding_supported(connector->mst_port->dpcd); + int hblank_limit = is_uhbr_sink ? 500 : 300; if (!connector->dp.dsc_hblank_expansion_quirk) return false; - if (mode_hblank_period_ns(adjusted_mode) > 300) + if (is_uhbr_sink && !drm_dp_is_uhbr_rate(limits->max_rate)) + return false; + + if (mode_hblank_period_ns(adjusted_mode) > hblank_limit) return false; return true; @@ -431,7 +438,7 @@ adjust_limits_for_dsc_hblank_expansion_quirk(const struct intel_connector *conne const struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); int min_bpp_x16 = limits->link.min_bpp_x16; - if (!hblank_expansion_quirk_needs_dsc(connector, crtc_state)) + if (!hblank_expansion_quirk_needs_dsc(connector, crtc_state, limits)) return true; if (!dsc) { @@ -1539,7 +1546,14 @@ static bool detect_dsc_hblank_expansion_quirk(const struct intel_connector *conn DP_DPCD_QUIRK_HBLANK_EXPANSION_REQUIRES_DSC)) return false; - if (!(dpcd[DP_RECEIVE_PORT_0_CAP_0] & DP_HBLANK_EXPANSION_CAPABLE)) + /* +* UHBR (MST sink) devices requiring this quirk doesn't advertise the +* HBLANK expansion support. Presuming that they perform HBLANK +* expansion internally, or are affected by this issue on modes with a +* short HBLANK for other reasons. +*/ + if (!drm_dp_uhbr_channel_coding_supported(dpcd) && + !(dpcd[DP_RECEIVE_PORT_0_CAP_0] & DP_HBLANK_EXPANSION_CAPABLE)) return false; drm_dbg_kms(>drm,
✓ Fi.CI.BAT: success for drm/i915/dp: Fix the computation for compressed_bpp for DISPLAY < 13 (rev2)
== Series Details == Series: drm/i915/dp: Fix the computation for compressed_bpp for DISPLAY < 13 (rev2) URL : https://patchwork.freedesktop.org/series/130710/ State : success == Summary == CI Bug Log - changes from CI_DRM_14489 -> Patchwork_130710v2 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/index.html Participating hosts (38 -> 38) -- Additional (3): fi-blb-e6850 bat-dg1-7 fi-kbl-8809g Missing(3): bat-arls-4 bat-kbl-2 fi-snb-2520m Known issues Here are the changes found in Patchwork_130710v2 that come from known issues: ### CI changes ### Issues hit * boot: - fi-cfl-8109u: [PASS][1] -> [FAIL][2] ([i915#8293]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/fi-cfl-8109u/boot.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/fi-cfl-8109u/boot.html - fi-kbl-8809g: NOTRUN -> [FAIL][3] ([i915#8293]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/fi-kbl-8809g/boot.html Possible fixes * boot: - bat-dg2-11: [FAIL][4] ([i915#10491]) -> [PASS][5] [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-11/boot.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg2-11/boot.html ### IGT changes ### Issues hit * igt@gem_mmap@basic: - bat-dg1-7: NOTRUN -> [SKIP][6] ([i915#4083]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg1-7/igt@gem_m...@basic.html - bat-dg2-11: NOTRUN -> [SKIP][7] ([i915#4083]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg2-11/igt@gem_m...@basic.html * igt@gem_tiled_fence_blits@basic: - bat-dg1-7: NOTRUN -> [SKIP][8] ([i915#4077]) +2 other tests skip [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg1-7/igt@gem_tiled_fence_bl...@basic.html - bat-dg2-11: NOTRUN -> [SKIP][9] ([i915#4077]) +2 other tests skip [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg2-11/igt@gem_tiled_fence_bl...@basic.html * igt@gem_tiled_pread_basic: - bat-dg1-7: NOTRUN -> [SKIP][10] ([i915#4079]) +1 other test skip [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg1-7/igt@gem_tiled_pread_basic.html - bat-dg2-11: NOTRUN -> [SKIP][11] ([i915#4079]) +1 other test skip [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg2-11/igt@gem_tiled_pread_basic.html * igt@i915_pm_rpm@module-reload: - fi-blb-e6850: NOTRUN -> [SKIP][12] +32 other tests skip [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/fi-blb-e6850/igt@i915_pm_...@module-reload.html * igt@i915_pm_rps@basic-api: - bat-dg1-7: NOTRUN -> [SKIP][13] ([i915#6621]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg1-7/igt@i915_pm_...@basic-api.html - bat-dg2-11: NOTRUN -> [SKIP][14] ([i915#6621]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg2-11/igt@i915_pm_...@basic-api.html * igt@i915_selftest@live@active: - bat-dg2-8: [PASS][15] -> [ABORT][16] ([i915#10366]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-8/igt@i915_selftest@l...@active.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg2-8/igt@i915_selftest@l...@active.html * igt@i915_selftest@live@slpc: - bat-dg2-9: [PASS][17] -> [ABORT][18] ([i915#10366]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-9/igt@i915_selftest@l...@slpc.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg2-9/igt@i915_selftest@l...@slpc.html * igt@kms_addfb_basic@addfb25-x-tiled-mismatch-legacy: - bat-dg1-7: NOTRUN -> [SKIP][19] ([i915#4212]) +7 other tests skip [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg1-7/igt@kms_addfb_ba...@addfb25-x-tiled-mismatch-legacy.html - bat-dg2-11: NOTRUN -> [SKIP][20] ([i915#4212]) +7 other tests skip [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg2-11/igt@kms_addfb_ba...@addfb25-x-tiled-mismatch-legacy.html * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy: - bat-dg2-11: NOTRUN -> [SKIP][21] ([i915#5190]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg2-11/igt@kms_addfb_ba...@addfb25-y-tiled-small-legacy.html * igt@kms_addfb_basic@basic-y-tiled-legacy: - bat-dg1-7: NOTRUN -> [SKIP][22] ([i915#4215]) [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130710v2/bat-dg1-7/igt@kms_addfb_ba...@basic-y-tiled-legacy.html - bat-dg2-11: NOTRUN -> [SKIP][23] ([i915#4215] /
Re: [PATCH 10/11] drm/i915/dp_mst: Make HBLANK expansion quirk work for logical ports
On 3/21/2024 1:41 AM, Imre Deak wrote: The DPCD OUI of the logical port on a Dell UHBR monitor - on which the AUX device is used to enable DSC - is all 0. To detect if the HBLANK expansion quirk is required for this monitor use the OUI of the port's parent instead. Since in the above case the DPCD of both the logical port and the parent port reports being a sink device (vs. branch device) type, read the proper sink/branch OUI based on the DPCD device type. This is required by a follow-up patch enabling the quirk for the above Dell monitor. Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/display/intel_dp_mst.c | 22 +++-- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 516b00f791420..76a8fb21b8e52 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1512,23 +1512,33 @@ intel_dp_mst_read_decompression_port_dsc_caps(struct intel_dp *intel_dp, static bool detect_dsc_hblank_expansion_quirk(const struct intel_connector *connector) { struct drm_i915_private *i915 = to_i915(connector->base.dev); + struct drm_dp_aux *aux = connector->dp.dsc_decompression_aux; struct drm_dp_desc desc; u8 dpcd[DP_RECEIVER_CAP_SIZE]; - if (!connector->dp.dsc_decompression_aux) + if (!aux) return false; - if (drm_dp_read_desc(connector->dp.dsc_decompression_aux, -, true) < 0) + /* +* A logical port's OUI (at least for affected sinks) is all 0, so +* instead of that the parent port's OUI is used for identification. +*/ + if (drm_dp_mst_port_is_logical(connector->port)) { + aux = drm_dp_mst_aux_for_parent(connector->port); + if (!aux) + aux = >mst_port->aux; As I understand, we are setting connector->mst_port as intel_dp, in the caller intel_dp_add_mst_connector so its unlikely that aux can be NULL, but do you see if we need to check for aux? Regards, Ankit + } + + if (drm_dp_read_dpcd_caps(aux, dpcd) < 0) + return false; + + if (drm_dp_read_desc(aux, , drm_dp_is_branch(dpcd)) < 0) return false; if (!drm_dp_has_quirk(, DP_DPCD_QUIRK_HBLANK_EXPANSION_REQUIRES_DSC)) return false; - if (drm_dp_read_dpcd_caps(connector->dp.dsc_decompression_aux, dpcd) < 0) - return false; - if (!(dpcd[DP_RECEIVE_PORT_0_CAP_0] & DP_HBLANK_EXPANSION_CAPABLE)) return false;
Re: [RESEND v3 2/2] drm: Add CONFIG_DRM_WERROR
On Wed, 27 Mar 2024, Maxime Ripard wrote: > Hi, > > On Tue, Mar 26, 2024 at 03:56:50PM -0700, Nathan Chancellor wrote: >> On Tue, Mar 05, 2024 at 11:07:36AM +0200, Jani Nikula wrote: >> > Add kconfig to enable -Werror subsystem wide. This is useful for >> > development and CI to keep the subsystem warning free, while avoiding >> > issues outside of the subsystem that kernel wide CONFIG_WERROR=y might >> > hit. >> > >> > v2: Don't depend on COMPILE_TEST >> > >> > Reviewed-by: Hamza Mahfooz # v1 >> > Signed-off-by: Jani Nikula >> > --- >> > drivers/gpu/drm/Kconfig | 13 + >> > drivers/gpu/drm/Makefile | 3 +++ >> > 2 files changed, 16 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >> > index 6e853acf15da..c08e18108c2a 100644 >> > --- a/drivers/gpu/drm/Kconfig >> > +++ b/drivers/gpu/drm/Kconfig >> > @@ -416,3 +416,16 @@ config DRM_LIB_RANDOM >> > config DRM_PRIVACY_SCREEN >> >bool >> >default n >> > + >> > +config DRM_WERROR >> > + bool "Compile the drm subsystem with warnings as errors" >> > + depends on EXPERT >> > + default n >> > + help >> > +A kernel build should not cause any compiler warnings, and this >> > +enables the '-Werror' flag to enforce that rule in the drm subsystem. >> > + >> > +The drm subsystem enables more warnings than the kernel default, so >> > +this config option is disabled by default. >> > + >> > +If in doubt, say N. >> >> While I understand the desire for an easy switch that maintainers and >> developers can use to ensure that their changes are warning free for the >> drm subsystem specifically, I think subsystem specific configuration >> options like this are actively detrimental to developers and continuous >> integration systems that build test the entire kernel. For example, we >> turned off CONFIG_WERROR for our Hexagon builds because of warnings that >> appear with -Wextra that are legitimate but require treewide changes to >> resolve in a manner sufficient for Linus: >> >> https://github.com/ClangBuiltLinux/linux/issues/1285 >> https://lore.kernel.org/all/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/ >> https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/ >> >> But now, due to CONFIG_DRM_WERROR getting enabled by all{mod,yes}config >> and -Wextra being unconditionally enabled for DRM, those warnings hard >> break the build despite CONFIG_WERROR=n... > > Would making DRM_WERROR depends on WERROR address your concerns? But then what would be the point of having DRM_WERROR at all? For me the point is, "werror in drm, ignore the rest, they're someone else's problem". An alternative would be to "depends on !COMPILE_TEST" that we have in i915, but then some folks want to have COMPILE_TEST in drm, because some drivers are otherwise hard for people to build. Nathan, we do want to fix any issues switfly. Are you hitting specific build problems? BR, Jani. > > Maxime -- Jani Nikula, Intel
Re: [RESEND v3 2/2] drm: Add CONFIG_DRM_WERROR
Hi, On Tue, Mar 26, 2024 at 03:56:50PM -0700, Nathan Chancellor wrote: > On Tue, Mar 05, 2024 at 11:07:36AM +0200, Jani Nikula wrote: > > Add kconfig to enable -Werror subsystem wide. This is useful for > > development and CI to keep the subsystem warning free, while avoiding > > issues outside of the subsystem that kernel wide CONFIG_WERROR=y might > > hit. > > > > v2: Don't depend on COMPILE_TEST > > > > Reviewed-by: Hamza Mahfooz # v1 > > Signed-off-by: Jani Nikula > > --- > > drivers/gpu/drm/Kconfig | 13 + > > drivers/gpu/drm/Makefile | 3 +++ > > 2 files changed, 16 insertions(+) > > > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > > index 6e853acf15da..c08e18108c2a 100644 > > --- a/drivers/gpu/drm/Kconfig > > +++ b/drivers/gpu/drm/Kconfig > > @@ -416,3 +416,16 @@ config DRM_LIB_RANDOM > > config DRM_PRIVACY_SCREEN > > bool > > default n > > + > > +config DRM_WERROR > > + bool "Compile the drm subsystem with warnings as errors" > > + depends on EXPERT > > + default n > > + help > > + A kernel build should not cause any compiler warnings, and this > > + enables the '-Werror' flag to enforce that rule in the drm subsystem. > > + > > + The drm subsystem enables more warnings than the kernel default, so > > + this config option is disabled by default. > > + > > + If in doubt, say N. > > While I understand the desire for an easy switch that maintainers and > developers can use to ensure that their changes are warning free for the > drm subsystem specifically, I think subsystem specific configuration > options like this are actively detrimental to developers and continuous > integration systems that build test the entire kernel. For example, we > turned off CONFIG_WERROR for our Hexagon builds because of warnings that > appear with -Wextra that are legitimate but require treewide changes to > resolve in a manner sufficient for Linus: > > https://github.com/ClangBuiltLinux/linux/issues/1285 > https://lore.kernel.org/all/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/ > https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/ > > But now, due to CONFIG_DRM_WERROR getting enabled by all{mod,yes}config > and -Wextra being unconditionally enabled for DRM, those warnings hard > break the build despite CONFIG_WERROR=n... Would making DRM_WERROR depends on WERROR address your concerns? Maxime signature.asc Description: PGP signature
RE: [RFC 0/5] Introduce drm sharpening property
> -Original Message- > From: Pekka Paalanen > Sent: Wednesday, March 13, 2024 3:07 PM > To: Garg, Nemesa > Cc: Simon Ser ; intel-gfx@lists.freedesktop.org; dri- > de...@lists.freedesktop.org; G M, Adarsh > Subject: Re: [RFC 0/5] Introduce drm sharpening property > > On Tue, 12 Mar 2024 16:26:00 +0200 > Pekka Paalanen wrote: > > > On Tue, 12 Mar 2024 08:30:34 + > > "Garg, Nemesa" wrote: > > > > > This KMS property is not implementing any formula > > > > Sure it is. Maybe Intel just does not want to tell what the algorithm > > is, or maybe it's even patented. > > > > > and the values > > > that are being used are based on empirical analysis and certain > > > experiments done on the hardware. These values are fixed and is not > > > expected to change and this can change from vendor to vendor. The > > > client can choose any sharpness value on the scale and on the basis > > > of it the sharpness will be set. The sharpness effect can be changed > > > from content to content and from display to display so user needs to > > > adjust the optimum intensity value so as to get good experience on > > > the screen. > > > > > > > IOW, it's an opaque box operation, and there is no way to reproduce > > its results without the specific Intel hardware. Definitely no way to > > reproduce its results in free open source software alone. > > > > Such opaque box operations can only occur after KMS blending, at the > > CRTC or later stage. They cannot appear before blending, not in the > > new KMS color pipeline design at least. The reason is that the modern > > way to use KMS planes is opportunistic composition off-loading. > > Opportunistic means that userspace decides from time to time whether > > it composes the final picture using KMS or some other rendering method > > (usually GPU and shaders). Since userspace will arbitrarily switch > > between KMS and render composition, both must result in the exact same > > image, or end users will observe unwanted flicker. > > > > Such opaque box operations are fine after blending, because there they > > can be configured once and remain on forever. No switching, no flicker. > > If you want to see how sharpness property would apply in Wayland design, it > would be in step 5 "Adjust (settings UI)" of > https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/color- > management-model.md#compositor-color-management-model > > To relate that diagram to KMS color processing, you can identify step 3 > "Compose" > as the KMS blending step. Everything before step 3 happens in KMS plane color > processing, and steps 4-5 happen in KMS CRTC color processing. > > Sharpening would essentially be a "compositor color effect", it just happens > to be > implementable only by specific Intel hardware. > > If a color effect is dynamic or content-dependant, it will preclude > colorimetric > monitor calibration. > > > Thanks, > pq > > > > Where does "sharpeness" operation occur in the Intel color processing > > chain? Is it before or after blending? > > Thank you for detail explanation and link. Sharpness operation occur post blending in CRTC ie on the final composed output after blending . Yes Pekka you are right as per the diagram it is done at step 5 "Adjust (settings UI)"). I will also document this thing along with documentation change. > > What kind of transfer characteristics does it expect from the image, > > and can those be realized with KMS CRTC properties if KMS is > > configured such that the blending happens using some other characteristics > (e.g. > > blending in optical space)? > > The filter values are not dependent/calculated on the inputs of image but depending on the blending space and other inputs the blended output gets changed and the sharpness is applied post blending so according to the content user needs to adjust the strength value to get the better visual effect. So tuning of sharpness strength may be needed by user based on the input contents and blending policy to get the desired experience. > > What about SDR vs. HDR imagery? > > The interface can be used for both HDR and SDR. The effect is more prominent for SDR use cases. For HDR filter values and tap value may change. Thanks, Nemesa > > > > Thanks, > > pq > > > > > > -Original Message- > > > > From: dri-devel On > > > > Behalf Of Simon Ser > > > > Sent: Monday, March 4, 2024 7:46 PM > > > > To: Garg, Nemesa > > > > Cc: Pekka Paalanen ; intel- > > > > g...@lists.freedesktop.org; dri-de...@lists.freedesktop.org; G M, > > > > Adarsh > > > > Subject: RE: [RFC 0/5] Introduce drm sharpening property > > > > > > > > On Monday, March 4th, 2024 at 15:04, Garg, Nemesa > > > > > > > > wrote: > > > > > > > > > This is generic as sharpness effect is applied post blending. > > > > > Depending on the color gamut, pixel format and other inputs the > > > > > image gets blended and once we get blended output it can be sharpened > based > > > > > on strength value
✗ Fi.CI.BAT: failure for Fix UBSAN warning in hdcp_info
== Series Details == Series: Fix UBSAN warning in hdcp_info URL : https://patchwork.freedesktop.org/series/131673/ State : failure == Summary == CI Bug Log - changes from CI_DRM_14489 -> Patchwork_131673v1 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_131673v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_131673v1, 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_131673v1/index.html Participating hosts (38 -> 36) -- Additional (2): bat-dg1-7 fi-kbl-8809g Missing(4): bat-arls-4 bat-mtlp-8 bat-kbl-2 fi-snb-2520m Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_131673v1: ### IGT changes ### Possible regressions * igt@i915_selftest@live@sanitycheck: - bat-atsm-1: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-atsm-1/igt@i915_selftest@l...@sanitycheck.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-atsm-1/igt@i915_selftest@l...@sanitycheck.html Known issues Here are the changes found in Patchwork_131673v1 that come from known issues: ### CI changes ### Issues hit * boot: - bat-arls-3: [PASS][3] -> [FAIL][4] ([i915#10234]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-arls-3/boot.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-arls-3/boot.html Possible fixes * boot: - bat-dg2-11: [FAIL][5] ([i915#10491]) -> [PASS][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-11/boot.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-dg2-11/boot.html ### IGT changes ### Issues hit * igt@gem_lmem_swapping@basic@lmem0: - bat-dg2-11: NOTRUN -> [FAIL][7] ([i915#10378]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-dg2-11/igt@gem_lmem_swapping@ba...@lmem0.html * igt@gem_mmap@basic: - bat-dg1-7: NOTRUN -> [SKIP][8] ([i915#4083]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-dg1-7/igt@gem_m...@basic.html - bat-dg2-11: NOTRUN -> [SKIP][9] ([i915#4083]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-dg2-11/igt@gem_m...@basic.html * igt@gem_tiled_fence_blits@basic: - bat-dg1-7: NOTRUN -> [SKIP][10] ([i915#4077]) +2 other tests skip [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-dg1-7/igt@gem_tiled_fence_bl...@basic.html - bat-dg2-11: NOTRUN -> [SKIP][11] ([i915#4077]) +2 other tests skip [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-dg2-11/igt@gem_tiled_fence_bl...@basic.html * igt@gem_tiled_pread_basic: - bat-dg1-7: NOTRUN -> [SKIP][12] ([i915#4079]) +1 other test skip [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-dg1-7/igt@gem_tiled_pread_basic.html - bat-dg2-11: NOTRUN -> [SKIP][13] ([i915#4079]) +1 other test skip [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-dg2-11/igt@gem_tiled_pread_basic.html * igt@i915_pm_rps@basic-api: - bat-dg1-7: NOTRUN -> [SKIP][14] ([i915#6621]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-dg1-7/igt@i915_pm_...@basic-api.html - bat-dg2-11: NOTRUN -> [SKIP][15] ([i915#6621]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-dg2-11/igt@i915_pm_...@basic-api.html * igt@i915_selftest@live@uncore: - bat-dg2-9: [PASS][16] -> [ABORT][17] ([i915#10366]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-9/igt@i915_selftest@l...@uncore.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-dg2-9/igt@i915_selftest@l...@uncore.html * igt@i915_selftest@live@vma: - bat-dg2-8: [PASS][18] -> [ABORT][19] ([i915#10366]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14489/bat-dg2-8/igt@i915_selftest@l...@vma.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-dg2-8/igt@i915_selftest@l...@vma.html * igt@kms_addfb_basic@addfb25-x-tiled-mismatch-legacy: - bat-dg1-7: NOTRUN -> [SKIP][20] ([i915#4212]) +7 other tests skip [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_131673v1/bat-dg1-7/igt@kms_addfb_ba...@addfb25-x-tiled-mismatch-legacy.html - bat-dg2-11: NOTRUN -> [SKIP][21] ([i915#4212]) +7 other tests skip [21]: