| Message ID | 20240220141919.3502674-1-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] drm/i915/panelreplay: Panel replay workaround with VRR | expand |
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 <animesh.manna@intel.com> > --- > 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. > + * 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? 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? or perhaps create a specific flag? one extra variable, 3 less comment lines... > + > return 0; > } > > -- > 2.29.0 >
> -----Original Message----- > From: Vivi, Rodrigo <rodrigo.vivi@intel.com> > Sent: Tuesday, February 20, 2024 11:12 PM > To: Manna, Animesh <animesh.manna@intel.com> > Cc: intel-gfx@lists.freedesktop.org; ville.syrjala@linux.intel.com; Hogander, > Jouni <jouni.hogander@intel.com>; Murthy, Arun R > <arun.r.murthy@intel.com> > 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 <animesh.manna@intel.com> > > --- > > 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. Regards, Animesh > > > + > > return 0; > > } > > > > -- > > 2.29.0 > >
On Wed, Feb 21, 2024 at 08:19:35PM +0000, Manna, Animesh wrote: > > > > -----Original Message----- > > From: Vivi, Rodrigo <rodrigo.vivi@intel.com> > > Sent: Tuesday, February 20, 2024 11:12 PM > > To: Manna, Animesh <animesh.manna@intel.com> > > Cc: intel-gfx@lists.freedesktop.org; ville.syrjala@linux.intel.com; Hogander, > > Jouni <jouni.hogander@intel.com>; Murthy, Arun R > > <arun.r.murthy@intel.com> > > 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 <animesh.manna@intel.com> > > > --- > > > 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 + crtc_vblank_start - crtc_vdisplay; -crtc_vblank_start - crtc_vdisplay); +context_latency); Ville, thoughts? > > Regards, > Animesh > > > > > > + > > > return 0; > > > } > > > > > > -- > > > 2.29.0 > > >
On Wed, Feb 21, 2024 at 03:58:48PM -0500, Rodrigo Vivi wrote: > On Wed, Feb 21, 2024 at 08:19:35PM +0000, Manna, Animesh wrote: > > > > > > > -----Original Message----- > > > From: Vivi, Rodrigo <rodrigo.vivi@intel.com> > > > Sent: Tuesday, February 20, 2024 11:12 PM > > > To: Manna, Animesh <animesh.manna@intel.com> > > > Cc: intel-gfx@lists.freedesktop.org; ville.syrjala@linux.intel.com; Hogander, > > > Jouni <jouni.hogander@intel.com>; Murthy, Arun R > > > <arun.r.murthy@intel.com> > > > 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 <animesh.manna@intel.com> > > > > --- > > > > 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 > + crtc_vblank_start - crtc_vdisplay; > > -crtc_vblank_start - crtc_vdisplay); > +context_latency); > > Ville, thoughts? I think what we need is a intel_crtc_vblank_delay() or somesuch thing that accounts for all the things (eg. this w/a, dsb execution latency when we start to use dsb for double buffered registers, etc). And it should probably be called from some central place so that it works for all output types. intel_crtc_compute_config() comes to mind, but I guess we need to first ponder whether there are bits of code being executed prior to that that would need to know the actual vblank_start...
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 +0000, Manna, Animesh wrote: > > > > > > > > > > -----Original Message----- > > > > From: Vivi, Rodrigo <rodrigo.vivi@intel.com> > > > > Sent: Tuesday, February 20, 2024 11:12 PM > > > > To: Manna, Animesh <animesh.manna@intel.com> > > > > Cc: intel-gfx@lists.freedesktop.org; ville.syrjala@linux.intel.com; Hogander, > > > > Jouni <jouni.hogander@intel.com>; Murthy, Arun R > > > > <arun.r.murthy@intel.com> > > > > 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 <animesh.manna@intel.com> > > > > > --- > > > > > 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 > > + crtc_vblank_start - crtc_vdisplay; > > > > -crtc_vblank_start - crtc_vdisplay); > > +context_latency); > > > > Ville, thoughts? > > I think what we need is a intel_crtc_vblank_delay() or somesuch thing > that accounts for all the things (eg. this w/a, dsb execution latency > when we start to use dsb for double buffered registers, etc). > And it should probably be called from some central place so that > it works for all output types. intel_crtc_compute_config() comes to > mind, but I guess we need to first ponder whether there are bits > of code being executed prior to that that would need to know the > actual vblank_start... PSR2 itself might have a chicken vs. egg issue here. Wehether we can enable it not depends on the length of the vblank. Whether that means the full vblank or the pipe's shrunken vblank I don't know. Someone needs to figure that out.
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Thursday, February 22, 2024 2:55 AM > To: Vivi, Rodrigo <rodrigo.vivi@intel.com> > Cc: Manna, Animesh <animesh.manna@intel.com>; intel- > gfx@lists.freedesktop.org; Hogander, Jouni <jouni.hogander@intel.com>; > Murthy, Arun R <arun.r.murthy@intel.com> > 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 +0000, Manna, Animesh wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > From: Vivi, Rodrigo <rodrigo.vivi@intel.com> > > > > > Sent: Tuesday, February 20, 2024 11:12 PM > > > > > To: Manna, Animesh <animesh.manna@intel.com> > > > > > Cc: intel-gfx@lists.freedesktop.org; > > > > > ville.syrjala@linux.intel.com; Hogander, Jouni > > > > > <jouni.hogander@intel.com>; Murthy, Arun R > > > > > <arun.r.murthy@intel.com> > > > > > 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 <animesh.manna@intel.com> > > > > > > --- > > > > > > 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 > > > + crtc_vblank_start - crtc_vdisplay; > > > > > > -crtc_vblank_start - crtc_vdisplay); > > > +context_latency); > > > > > > Ville, thoughts? > > > > I think what we need is a intel_crtc_vblank_delay() or somesuch thing > > that accounts for all the things (eg. this w/a, dsb execution latency > > when we start to use dsb for double buffered registers, etc). > > And it should probably be called from some central place so that it > > works for all output types. intel_crtc_compute_config() comes to mind, > > but I guess we need to first ponder whether there are bits of code > > being executed prior to that that would need to know the actual > > vblank_start... > > PSR2 itself might have a chicken vs. egg issue here. Wehether we can enable > it not depends on the length of the vblank. Whether that means the full > vblank or the pipe's shrunken vblank I don't know. Someone needs to figure > that out. Added a centralized intel_crtc_vblank_delay() in v4 but as this WA is specific for panel replay which will be supported by DP/EDP so called from encoder-compute-config() instead of crtc-compute-config. Regards, Animesh > > -- > Ville Syrjälä > Intel
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 + * 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; + return 0; }
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] Signed-off-by: Animesh Manna <animesh.manna@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)