On Tue, Jul 09, 2024 at 11:32:11AM +0200, Daniel Vetter wrote:
> On Mon, Jul 08, 2024 at 04:29:07PM -0400, Hamza Mahfooz wrote:
> > Hook up drm_crtc_set_vblank_offdelay() in amdgpu_dm, so that we can
> > enable PSR more quickly for displays that support it.
> >
> > Signed-off-by: Hamza Mahfooz <hamza.mahfooz@xxxxxxx>
> > ---
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 30 ++++++++++++++-----
> > 1 file changed, 22 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index fdbc9b57a23d..ee6c31e9d3c4 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -8231,7 +8231,7 @@ static int amdgpu_dm_encoder_init(struct drm_device *dev,
> >
> > static void manage_dm_interrupts(struct amdgpu_device *adev,
> > struct amdgpu_crtc *acrtc,
> > - bool enable)
> > + struct dm_crtc_state *acrtc_state)
> > {
> > /*
> > * We have no guarantee that the frontend index maps to the same
> > @@ -8239,12 +8239,25 @@ static void manage_dm_interrupts(struct amdgpu_device *adev,
> > *
> > * TODO: Use a different interrupt or check DC itself for the mapping.
> > */
> > - int irq_type =
> > - amdgpu_display_crtc_idx_to_irq_type(
> > - adev,
> > - acrtc->crtc_id);
> > + int irq_type = amdgpu_display_crtc_idx_to_irq_type(adev,
> > + acrtc->crtc_id);
> > + struct dc_crtc_timing *timing;
> > + int offdelay;
> > +
> > + if (acrtc_state) {
> > + timing = &acrtc_state->stream->timing;
> > +
> > + /* at least 2 frames */
> > + offdelay = 2000 / div64_u64(div64_u64((timing->pix_clk_100hz *
> > + (uint64_t)100),
> > + timing->v_total),
> > + timing->h_total) + 1;
>
> Yeah, _especially_ when you have a this short timeout your really have to
> instead fix the vblank driver code properly so you can enable
> vblank_disable_immediate. This is just cheating :-)
Michel mentioned on irc that DC had immediate vblank disabling, but this
was reverted with f64e6e0b6afe ("Revert "drm/amdgpu/display: set
vblank_disable_immediate for DC"").
I haven't looked at the details of the bug report, but stuttering is
exactly what happens when the driver's vblank code has these races. Going
for a very low timeout instead of zero just means it's a bit harder to hit
the issue, and much, much harder to debug properly.
So yeah even more reasons to look at the underlying root-cause here I
think.
-Sima
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
