Re: [PATCH v4 07/27] drm/amd/display: Use bigger VRR range if found in AMD vsdb

[Tomasz Pakuła]

On Thu, 2026-02-26 at 14:36 +0200, Jani Nikula wrote:
> On Mon, 16 Feb 2026, Tomasz Pakuła <tomasz.pakula.oficjalny@xxxxxxxxx> wrote:
> > [Why]
> > Some monitors only expose their full VRR range in AMD vsdb for some
> > reason.
> > 
> > [How]
> > Compare exposed ranges and use the bigger one.
> > Only adjust lower limit if it doesn't support LFC
> > 
> > Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4177
> > Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@xxxxxxxxx>
> > ---
> >  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 31 +++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> > 
> > 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 b3bf5e0c19a5..f36059bb0324 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -13269,6 +13269,34 @@ static bool copy_range_to_amdgpu_connector(struct drm_connector *conn)
> >  	return is_freesync_capable(range);
> >  }
> >  
> > +static void extend_range_from_vsdb(struct drm_display_info *display,
> > +				   const struct amdgpu_hdmi_vsdb_info *vsdb)
> > +{
> > +	u16 vrr_min = display->monitor_range.min_vfreq;
> > +	u16 vrr_max = display->monitor_range.max_vfreq;
> > +
> > +	/* Always extend upper limit */
> > +	if (vsdb->max_refresh_rate_hz > vrr_max)
> > +		vrr_max = vsdb->max_refresh_rate_hz;
> > +
> > +	/*
> > +	 * Only extend lower limit if current one disables LFC.
> > +
> > +	 * During widespread testing, we found that some manufacturers probably
> > +	 * had issues with their monitors' lower VRR boundaries and adjusted
> > +	 * them up (Gigabyte X34GS with official range 48 - 180, AMD vsdb 48 -
> > +	 * 180 yet Monitor Ranges 55 - 180). After setting the lower boundary
> > +	 * from AMD vsdb, such monitors start having blanking issues.
> > +	 *
> > +	 * Work around that by not touching VRR min if it still supports LFC.
> > +	 */
> > +	if (vsdb->min_refresh_rate_hz < vrr_min && (vrr_min * 2 >= vrr_max))
> > +		vrr_min = vsdb->min_refresh_rate_hz;
> > +
> > +	display->monitor_range.min_vfreq = vrr_min;
> > +	display->monitor_range.max_vfreq = vrr_max;
> 
> Random driver code should *not* modify struct drm_display_info,
> especially the fields that drm_edid.c parses. Drivers should cease to
> parse EDID and DisplayID altogether.
> 
> I'm on the verge of NAKing, to the extent that I have control over this,
> any further improvements on driver EDID/DisplayID parsing, with the
> expectation that everything's moved to shared EDID parser in drm_edid.c
> first, and improved there.
> 
> BR,
> Jani.

I completely agree with you, and I too am saddened by how selective AMD
can be when contributing to open source, BUT I'm just a guy who figured
out some fixes neglected by them. Not being an AMD employee, I don't
have access to their plans or what has been their idea behind modifying
drm_display_info (probably just a lack of understanding, like in my
case).

If maybe I could get OK from Alex or Harry to completely rip out
drm_display_info modifications from amdgpu_dm_update_freesync_caps, then
sure, but currently, I'm not in a position to do so. Still, it seems
like amdgpu already relies purely on it's internal structs from there on
out, so it should be pretty straightforward.

Moreover, moving all DisplayID and CTA extension parsing would be
amazing, BUT AMD vsdb specification isn't public. Here, amdgpu even uses
firmware to parse it, which is bonkers IMO, but they want to keep their
secrets (for some reason). There always will be some VRR meddling in the
drivers based on hardware features, though I do think AMD should release
their whole Freesync vsdb to the public as other drivers could parse
this info as well, because we can already see it's needed to properly
support all monitors.

Hell, even some of my fixes could be moved to general drm code to
facilitate proper VRR range detection.

Going back to the issue at hand, if I could get OK from AMD to rip out
all drm_display_info modifications from amdgpu_dm_update_freesync_caps
then it won't be a problem, but until then, I'm dealing with what I've
been given, doing as little functional, changes as possible. We already
know amdgpu code is... unoptimal, and I wouldn't want to add to that :D

Tomasz

> 
> 
> > +}
> > +
> >  /**
> >   * amdgpu_dm_update_freesync_caps - Update Freesync capabilities
> >   *
> > @@ -13339,6 +13367,9 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
> >  		if (is_monitor_range_invalid(connector))
> >  			monitor_range_from_vsdb(&connector->display_info, &vsdb_info);
> >  
> > +		/* Try extending range if found in AMD vsdb */
> > +		extend_range_from_vsdb(&connector->display_info, &vsdb_info);
> > +
> >  		if (dpcd_caps.allow_invalid_MSA_timing_param)
> >  			freesync_capable = copy_range_to_amdgpu_connector(connector);