On Tue, Feb 25, 2025 at 11:39:04PM +0530, Chaitanya Kumar Borah wrote:
> >From PTL, LUT registers are made double buffered. This helps us
> to program them in the active region without any concern of tearing.
> This particulary helps in case of displays with high refresh rates
> where vblank periods are shorter.
>
> This patch makes the following changes
>
> - Adds the macro HAS_DOUBLE_BUFFERED_LUT() to distinguish
> platforms that have double buffered LUT registers.
>
> - Program LUT values in active region through
> intel_pre_update_crtc()
>
> - Disable updating of LUT values during vblank.
>
> - Disable pre-loading of LUT values as they are no longer
> single buffered.
>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_color.c | 4 ++++
> drivers/gpu/drm/i915/display/intel_crtc.c | 4 +++-
> drivers/gpu/drm/i915/display/intel_display.c | 6 +++++-
> drivers/gpu/drm/i915/display/intel_display_device.h | 1 +
> 4 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index cfe14162231d..c3ee34b96c15 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -2022,6 +2022,10 @@ static bool intel_can_preload_luts(struct intel_atomic_state *state,
> {
> const struct intel_crtc_state *old_crtc_state =
> intel_atomic_get_old_crtc_state(state, crtc);
> + struct intel_display *display = to_intel_display(crtc);
> +
> + if (HAS_DOUBLE_BUFFERED_LUT(display))
> + return false;
>
> return !old_crtc_state->post_csc_lut &&
> !old_crtc_state->pre_csc_lut;
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
> index 5b2603ef2ff7..927f9acf61c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -432,10 +432,12 @@ static void intel_crtc_vblank_work(struct kthread_work *base)
> struct intel_crtc_state *crtc_state =
> container_of(work, typeof(*crtc_state), vblank_work);
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> + struct intel_display *display = to_intel_display(crtc_state);
>
> trace_intel_crtc_vblank_work_start(crtc);
>
> - intel_color_load_luts(crtc_state);
> + if (!HAS_DOUBLE_BUFFERED_LUT(display))
Wrong place. You don't even want to schedule the vblank worker
for this.
> + intel_color_load_luts(crtc_state);
>
> if (crtc_state->uapi.event) {
> spin_lock_irq(&crtc->base.dev->event_lock);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 065fdf6dbb88..919e236a9650 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6879,9 +6879,13 @@ static void intel_pre_update_crtc(struct intel_atomic_state *state,
>
> if (!modeset &&
> intel_crtc_needs_color_update(new_crtc_state) &&
> - !new_crtc_state->use_dsb)
> + !new_crtc_state->use_dsb) {
> intel_color_commit_noarm(NULL, new_crtc_state);
>
> + if (HAS_DOUBLE_BUFFERED_LUT(display))
> + intel_color_load_luts(new_crtc_state);
Explanation missing on the double buffering behaviour of the LUT.
This now assumes that it's not self-arming, and therefore some
other register must be the arming register. Which register is it?
> + }
> +
> if (!new_crtc_state->use_dsb)
> intel_crtc_planes_update_noarm(NULL, state, crtc);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> index fc33791f02b9..419d0213e412 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -155,6 +155,7 @@ struct intel_display_platforms {
> #define HAS_DMC(__display) (DISPLAY_RUNTIME_INFO(__display)->has_dmc)
> #define HAS_DMC_WAKELOCK(__display) (DISPLAY_VER(__display) >= 20)
> #define HAS_DOUBLE_BUFFERED_M_N(__display) (DISPLAY_VER(__display) >= 9 || (__display)->platform.broadwell)
> +#define HAS_DOUBLE_BUFFERED_LUT(__display) (DISPLAY_VER(__display) >= 30)
> #define HAS_DOUBLE_WIDE(__display) (DISPLAY_VER(__display) < 4)
> #define HAS_DP_MST(__display) (DISPLAY_INFO(__display)->has_dp_mst)
> #define HAS_DP20(__display) ((__display)->platform.dg2 || DISPLAY_VER(__display) >= 14)
> --
> 2.25.1
--
Ville Syrjälä
Intel
