> Subject: Re: [PATCH] drm/i915/display: Use PIPEDMC_FRMTMSTMP on display
> ver >= 30
> 
> Suraj Kandpal <[email protected]> writes:
> 
> > Starting with display version 30, the per-pipe frame timestamp is read
> > from the PIPEDMC register block (PIPEDMC_FRMTMSTMP) instead of the
> > legacy PIPE_FRMTMSTMP MMIO. Extend PIPE_FRMTMSTMP() to take the
> > display and select the appropriate register based on DISPLAY_VER(),
> > and update all callers (intel_vblank, intel_initial_plane) accordingly.
> >
> > Bspec: 79482
> > WA: 14022946399
> 
> Why is this workaround being mentioned here?
> 
> If this is part of the workaround implementation, we should use the proper
> display workaround infra (intel_display_wa.*) and we probably don't need to
> add this commit trailer IMO.
> 
> Is the idea to use the PIPEDMC_FRMTMSTMP register as an alternative for the
> workaround in display IPs that support such a register? If so, I think this
> alternative will not apply to previous display versions, right?

Sure will use the intel_display_wa framework

> 
> Another important question is: is this register updated even when the DMC is
> not loaded?

Its actually does not, I have fixed the issue which will arise from this mainly 
in intel_inital_wait_for_vblank() and will refloat that in the next revision.

Regards,
Suraj Kandpal

> 
> --
> Gustavo Sousa
> 
> > Signed-off-by: Suraj Kandpal <[email protected]>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display_regs.h  | 7 +++++--
> > drivers/gpu/drm/i915/display/intel_initial_plane.c | 4 ++--
> >  drivers/gpu/drm/i915/display/intel_vblank.c        | 4 ++--
> >  3 files changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_regs.h
> > b/drivers/gpu/drm/i915/display/intel_display_regs.h
> > index 4321f8b529da..579f802215d3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_regs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_regs.h
> > @@ -3149,8 +3149,11 @@ enum skl_power_gate {
> >  /* g4x+, except vlv/chv! */
> >  #define _PIPE_FRMTMSTMP_A          0x70048
> >  #define _PIPE_FRMTMSTMP_B          0x71048
> > -#define PIPE_FRMTMSTMP(pipe)               \
> > -   _MMIO_PIPE(pipe, _PIPE_FRMTMSTMP_A, _PIPE_FRMTMSTMP_B)
> > +#define _PIPEDMC_FRMTMSTMP_A               0x5f0ac
> > +#define _PIPEDMC_FRMTMSTMP_B               0x5f4ac
> > +#define PIPE_FRMTMSTMP(display, pipe)      (DISPLAY_VER(display) >= 30 ?
> \
> > +   _MMIO_PIPE(pipe, _PIPEDMC_FRMTMSTMP_A,
> _PIPEDMC_FRMTMSTMP_B) : \
> > +   _MMIO_PIPE(pipe, _PIPE_FRMTMSTMP_A, _PIPE_FRMTMSTMP_B))
> >
> >  /* g4x+, except vlv/chv! */
> >  #define _PIPE_FLIPTMSTMP_A         0x7004C
> > diff --git a/drivers/gpu/drm/i915/display/intel_initial_plane.c
> > b/drivers/gpu/drm/i915/display/intel_initial_plane.c
> > index 034fe199c2a1..004cbdb6be32 100644
> > --- a/drivers/gpu/drm/i915/display/intel_initial_plane.c
> > +++ b/drivers/gpu/drm/i915/display/intel_initial_plane.c
> > @@ -34,9 +34,9 @@ void intel_initial_plane_vblank_wait(struct intel_crtc
> *crtc)
> >             return;
> >     }
> >
> > -   start_ts = intel_de_read(display, PIPE_FRMTMSTMP(crtc->pipe));
> > +   start_ts = intel_de_read(display, PIPE_FRMTMSTMP(display,
> > +crtc->pipe));
> >
> > -   ret = poll_timeout_us(end_ts = intel_de_read(display,
> PIPE_FRMTMSTMP(crtc->pipe)),
> > +   ret = poll_timeout_us(end_ts = intel_de_read(display,
> > +PIPE_FRMTMSTMP(display, crtc->pipe)),
> >                           end_ts != start_ts, 1000, 1000 * 1000, false);
> >     if (ret)
> >             drm_warn(display->drm, "[CRTC:%d:%s] early vblank wait
> timed
> > out\n", diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c
> > b/drivers/gpu/drm/i915/display/intel_vblank.c
> > index 28d81199792e..52ff47936f9e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> > @@ -157,7 +157,7 @@ static u32
> intel_crtc_scanlines_since_frame_timestamp(struct intel_crtc *crtc)
> >              * is sampled at every start of vertical blank.
> >              */
> >             scan_prev_time = intel_de_read_fw(display,
> > -                                             PIPE_FRMTMSTMP(crtc-
> >pipe));
> > +                                             PIPE_FRMTMSTMP(display,
> crtc->pipe));
> >
> >             /*
> >              * The TIMESTAMP_CTR register has the current @@ -166,7
> +166,7 @@
> > static u32 intel_crtc_scanlines_since_frame_timestamp(struct intel_crtc 
> > *crtc)
> >             scan_curr_time = intel_de_read_fw(display,
> IVB_TIMESTAMP_CTR);
> >
> >             scan_post_time = intel_de_read_fw(display,
> > -                                             PIPE_FRMTMSTMP(crtc-
> >pipe));
> > +                                             PIPE_FRMTMSTMP(display,
> crtc->pipe));
> >     } while (scan_post_time != scan_prev_time);
> >
> >     return div_u64(mul_u32_u32(scan_curr_time - scan_prev_time,
> > --
> > 2.34.1

Reply via email to