On Thu, 10 Oct 2024, "Hogander, Jouni" <jouni.hogander@xxxxxxxxx> wrote:
> On Thu, 2024-10-10 at 11:03 +0300, Jani Nikula wrote:
>> On Thu, 10 Oct 2024, Jouni Högander <jouni.hogander@xxxxxxxxx> wrote:
>> > Add own interface for PSR usage to perform push on frontbuffer
>> > tracking
>> > invalidate and flush call backs. Use this new interface from PSR
>> > code.
>> >
>> > Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx>
>> > ---
>> > drivers/gpu/drm/i915/display/intel_psr.c | 7 ++++++-
>> > drivers/gpu/drm/i915/display/intel_vrr.c | 18 ++++++++++++++++++
>> > drivers/gpu/drm/i915/display/intel_vrr.h | 4 ++++
>> > 3 files changed, 28 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
>> > b/drivers/gpu/drm/i915/display/intel_psr.c
>> > index 5be8076475f0b..7959a33771b13 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>> > @@ -2326,8 +2326,13 @@ static void psr_force_exit(struct intel_dp
>> > *intel_dp)
>> > * This workaround do not exist for platforms with display
>> > 10 or newer
>> > * but testing proved that it works for up display 13, for
>> > newer
>> > * than that testing will be needed.
>> > + *
>> > + * In Lunarlake we can use TRANS_PUSH mechanism to force
>> > sending update
>> > + * to sink.
>> > */
>> > - intel_de_write(display, CURSURFLIVE(display, intel_dp-
>> > >psr.pipe), 0);
>> > + DISPLAY_VER(display) >= 20 ?
>> > + intel_vrr_psr_send_push(display, cpu_transcoder) :
>> > + intel_de_write(display, CURSURFLIVE(display,
>> > intel_dp->psr.pipe), 0);
>> > }
>> >
>> > void intel_psr2_program_trans_man_trk_ctl(const struct
>> > intel_crtc_state *crtc_state)
>> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
>> > b/drivers/gpu/drm/i915/display/intel_vrr.c
>> > index 5925ade4591d4..d51830d173b61 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> > @@ -328,6 +328,24 @@ void intel_vrr_send_push(const struct
>> > intel_crtc_state *crtc_state)
>> > trans_push_val);
>> > }
>> >
>> > +/**
>> > + * intel_vrr_psr_send_push - Send push interface for PSR code
>> > + * @display: Intel display
>> > + * @cpu_transcoder: cpu_transcode
>> > + *
>> > + * This is for PSR usage to perform push on frontbuffer tracking
>> > invalidate and
>> > + * flush call back. PSR mutex should be taken by caller.
>> > + */
>> > +void intel_vrr_psr_send_push(struct intel_display *display,
>> > + enum transcoder cpu_transcoder)
>> > +{
>> > + if (DISPLAY_VER(display) < 20)
>> > + return;
>> > +
>> > + intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder),
>> > 0,
>> > + TRANS_PUSH_SEND | LNL_TRANS_PUSH_PSR_PR_EN);
>> > +}
>> > +
>> > bool intel_vrr_is_push_sent(const struct intel_crtc_state
>> > *crtc_state)
>> > {
>> > struct intel_display *display =
>> > to_intel_display(crtc_state);
>> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h
>> > b/drivers/gpu/drm/i915/display/intel_vrr.h
>> > index a75f159168c11..3da7ba12697ff 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_vrr.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
>> > @@ -12,6 +12,8 @@ struct drm_connector_state;
>> > struct intel_atomic_state;
>> > struct intel_connector;
>> > struct intel_crtc_state;
>> > +struct intel_display;
>> > +enum transcoder;
>> >
>> > bool intel_vrr_is_capable(struct intel_connector *connector);
>> > bool intel_vrr_is_in_range(struct intel_connector *connector, int
>> > vrefresh);
>> > @@ -25,6 +27,8 @@ bool intel_vrr_is_push_sent(const struct
>> > intel_crtc_state *crtc_state);
>> > void intel_vrr_disable(const struct intel_crtc_state
>> > *old_crtc_state);
>> > void intel_vrr_psr_frame_change_enable(const struct
>> > intel_crtc_state *crtc_state);
>> > void intel_vrr_psr_frame_change_disable(const struct
>> > intel_crtc_state *crtc_state);
>> > +void intel_vrr_psr_send_push(struct intel_display *display,
>> > + enum transcoder cpu_transcoder);
>>
>> Nitpick, why not just make that parameter crtc_state like for all the
>> other functions?
>
> This is about to be used from frontbuffer flush/invalidate path as
> well. There we do not have crtc_state directly. I'll guess we could use
> current state of crtc. Do you think that would be better?
I wasn't aware of the potential future use. Just stick with this, at
least for now.
BR,
Jani.
>
> BR,
>
> Jouni Högander
>
>>
>> BR,
>> Jani.
>>
>> > void intel_vrr_get_config(struct intel_crtc_state *crtc_state);
>> > int intel_vrr_vmax_vblank_start(const struct intel_crtc_state
>> > *crtc_state);
>> > int intel_vrr_vmin_vblank_start(const struct intel_crtc_state
>> > *crtc_state);
>>
>
--
Jani Nikula, Intel
