[PATCH 1/2] Revert "drm/amd: Remove freesync video mode amdgpu parameter"

[Alex Deucher]

On Wed, Feb 28, 2024 at 9:45 AM Christian König
<ckoenig.leichtzumerken at gmail.com> wrote:
>
> Am 28.02.24 um 15:23 schrieb Alex Deucher:
> > On Wed, Feb 28, 2024 at 2:03 AM Christian König
> > <ckoenig.leichtzumerken at gmail.com> wrote:
> >> Am 27.02.24 um 19:48 schrieb Alex Deucher:
> >>> This reverts commit e94e787e37b99645e7c02d20d0a1ba0f8a18a82a.
> >>>
> >>> This conflicts with how compositors want to handle VRR.  Now
> >>> that compositors actually handle VRR, we probably don't need
> >>> freesync video.
> >>>
> >>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2985
> >> Scratching my head what actually happens here? Doesn't the problem then
> >> just depend on a module parameter?
> > Yes.  The problem is that when freesync video is enabled, compositors
> > don't know which modes are actual modes and which are a VRR video
> > mode.  There are still customers that want the vrr video mode smooth
> > video playback, but compositors don't want this by default.  I guess
> > the alternative is to just drop this feature altogether now that
> > compositors and media players are starting to support this properly.
>
> That's what I would suggest as well.
>
> As far as I can see adding those modes is actually buggy behavior and we
> need to avoid it.

Well, they work as expected for the use case they were added for,
video playback with different refresh rates without a modeset.  I'll
apply then as is for now and then work on a set of patches to remove
the functionality.

Alex


>
> Christian.
>
> >
> > Alex
> >
> >> Regards,
> >> Christian.
> >>
> >>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> >>> ---
> >>>    drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
> >>>    drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 27 +++++++++++++++++++++++++
> >>>    2 files changed, 28 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> index 0e365cadcc3fc..925026c183f41 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> @@ -194,6 +194,7 @@ extern int amdgpu_emu_mode;
> >>>    extern uint amdgpu_smu_memory_pool_size;
> >>>    extern int amdgpu_smu_pptable_id;
> >>>    extern uint amdgpu_dc_feature_mask;
> >>> +extern uint amdgpu_freesync_vid_mode;
> >>>    extern uint amdgpu_dc_debug_mask;
> >>>    extern uint amdgpu_dc_visual_confirm;
> >>>    extern int amdgpu_dm_abm_level;
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >>> index 15a8a64fc4e28..82b154b103f43 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >>> @@ -199,6 +199,7 @@ int amdgpu_mes_kiq;
> >>>    int amdgpu_noretry = -1;
> >>>    int amdgpu_force_asic_type = -1;
> >>>    int amdgpu_tmz = -1; /* auto */
> >>> +uint amdgpu_freesync_vid_mode;
> >>>    int amdgpu_reset_method = -1; /* auto */
> >>>    int amdgpu_num_kcq = -1;
> >>>    int amdgpu_smartshift_bias;
> >>> @@ -883,6 +884,32 @@ module_param_named(damageclips, amdgpu_damage_clips, int, 0444);
> >>>    MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto (default), 0 = off, 1 = on)");
> >>>    module_param_named(tmz, amdgpu_tmz, int, 0444);
> >>>
> >>> +/**
> >>> + * DOC: freesync_video (uint)
> >>> + * Enable the optimization to adjust front porch timing to achieve seamless
> >>> + * mode change experience when setting a freesync supported mode for which full
> >>> + * modeset is not needed.
> >>> + *
> >>> + * The Display Core will add a set of modes derived from the base FreeSync
> >>> + * video mode into the corresponding connector's mode list based on commonly
> >>> + * used refresh rates and VRR range of the connected display, when users enable
> >>> + * this feature. From the userspace perspective, they can see a seamless mode
> >>> + * change experience when the change between different refresh rates under the
> >>> + * same resolution. Additionally, userspace applications such as Video playback
> >>> + * can read this modeset list and change the refresh rate based on the video
> >>> + * frame rate. Finally, the userspace can also derive an appropriate mode for a
> >>> + * particular refresh rate based on the FreeSync Mode and add it to the
> >>> + * connector's mode list.
> >>> + *
> >>> + * Note: This is an experimental feature.
> >>> + *
> >>> + * The default value: 0 (off).
> >>> + */
> >>> +MODULE_PARM_DESC(
> >>> +     freesync_video,
> >>> +     "Enable freesync modesetting optimization feature (0 = off (default), 1 = on)");
> >>> +module_param_named(freesync_video, amdgpu_freesync_vid_mode, uint, 0444);
> >>> +
> >>>    /**
> >>>     * DOC: reset_method (int)
> >>>     * GPU reset method (-1 = auto (default), 0 = legacy, 1 = mode0, 2 = mode1, 3 = mode2, 4 = baco)
>