On Fri, 2024-12-20 at 11:34 +0200, Ville Syrjälä wrote:
> On Fri, Dec 13, 2024 at 08:35:23AM +0200, Jouni Högander wrote:
> > Bspec says this for SFF/CFF:
> >
> > "Hardware samples this bit on the start of every V. Blank Guardband
> > region. For deterministic behavior, Software should ensure it is
> > not
> > changing this bit around the sample point."
> >
> > This can be achieved by using intel_vblank_evade.
> >
> > Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_psr.c | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index c074de6b6c12..8aa2574ba5c9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -26,6 +26,7 @@
> > #include <drm/drm_atomic_helper.h>
> > #include <drm/drm_damage_helper.h>
> > #include <drm/drm_debugfs.h>
> > +#include <drm/drm_vblank.h>
> >
> > #include "i915_drv.h"
> > #include "i915_reg.h"
> > @@ -44,6 +45,7 @@
> > #include "intel_psr.h"
> > #include "intel_psr_regs.h"
> > #include "intel_snps_phy.h"
> > +#include "intel_vblank.h"
> > #include "skl_universal_plane.h"
> >
> > /**
> > @@ -3114,16 +3116,31 @@ static void
> > intel_psr_configure_full_frame_update(struct intel_dp *intel_dp)
> > {
> > struct intel_display *display =
> > to_intel_display(intel_dp);
> > enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
> > + struct intel_crtc *crtc = intel_crtc_for_pipe(display,
> > intel_dp->psr.pipe);
> > + struct intel_crtc_state *crtc_state =
> > to_intel_crtc_state(crtc->base.state);
>
> You can't use that without proper locking.
I did some experiments with this and started thinking I will drop this
patch from the series. frontbuffer callbacks in gem code are quite a
rabbit hole what comes to locking. This is anyways addressing existing
issue not introduced by this patch series. One idea I have is to
implement evasion only for ORIGIN_DIRTYFB and ORIGIN_CURSOR_UPDATE. Do
you have any better ideas?
BR,
Jouni Högander
>
> > + struct intel_vblank_evade_ctx evade;
> >
> > if (!intel_dp->psr.psr2_sel_fetch_enabled)
> > return;
> >
> > + intel_vblank_evade_init(crtc_state, crtc_state, &evade);
> > +
> > + drm_crtc_vblank_get(&crtc->base);
> > +
> > + local_irq_disable();
> > +
> > + intel_vblank_evade(&evade);
> > +
> > + drm_crtc_vblank_put(&crtc->base);
> > +
> > intel_de_write(display,
> > PSR2_MAN_TRK_CTL(display, cpu_transcoder),
> > man_trk_ctl_enable_bit_get(display) |
> > man_trk_ctl_partial_frame_bit_get(display)
> > |
> >
> > man_trk_ctl_single_full_frame_bit_get(display) |
> > man_trk_ctl_continuos_full_frame(display));
> > +
> > + local_irq_enable();
> > }
> >
> > static void _psr_invalidate_handle(struct intel_dp *intel_dp)
> > --
> > 2.34.1
>
