Re: [PATCH v3 3/5] drm/edid: Add support for quirks visible to DRM core and drivers

[Imre Deak <imre.deak@intel.com>]

On Thu, Jun 05, 2025 at 04:07:15PM +0300, Jani Nikula wrote:
> On Thu, 05 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> > Add support for EDID based quirks which can be queried outside of the
> > EDID parser iteself by DRM core and drivers. There are at least two such
> > quirks applicable to all drivers: the DPCD register access probe quirk
> > and the 128b/132b DPRX Lane Count Conversion quirk (see 3.5.2.16.3 in
> > the v2.1a DP Standard). The latter quirk applies to panels with specific
> > EDID panel names, support for defining a quirk this way will be added as
> > a follow-up.
> >
> > v2: Reset global_quirks in drm_reset_display_info().
> > v3: (Jani)
> > - Use one list for both the global and internal quirks.
> > - Drop change for panel name specific quirks.
> > - Add comment about the way quirks should be queried.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/drm_edid.c  | 8 +++++++-
> >  include/drm/drm_connector.h | 4 +++-
> >  include/drm/drm_edid.h      | 5 +++++
> >  3 files changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 857ae0c47a1c3..9cca1e6e4736c 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -68,7 +68,7 @@ static int oui(u8 first, u8 second, u8 third)
> >  
> >  enum drm_edid_internal_quirk {
> >  	/* First detailed mode wrong, use largest 60Hz mode */
> > -	EDID_QUIRK_PREFER_LARGE_60,
> > +	EDID_QUIRK_PREFER_LARGE_60 = DRM_EDID_QUIRK_NUM,
> >  	/* Reported 135MHz pixel clock is too high, needs adjustment */
> >  	EDID_QUIRK_135_CLOCK_TOO_HIGH,
> >  	/* Prefer the largest mode at 75 Hz */
> > @@ -2959,6 +2959,12 @@ static bool drm_edid_has_internal_quirk(struct drm_connector *connector,
> >  	return connector->display_info.quirks & BIT(quirk);
> >  }
> >  
> > +bool drm_edid_has_quirk(struct drm_connector *connector, enum drm_edid_quirk quirk)
> 
> Nitpick, this could've been const.

I'm unsure what rule to follow for atomic object pointers. Ville said
once that unlike atomic state pointers (i.e. drm_connector_state for
instance) these shouldn't be constified. Maybe that applies only to
cases where the object pointer is passed on to another function (so not
in this simple case clearly)?

> Regardless,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> 
> > +{
> > +	return connector->display_info.quirks & BIT(quirk);
> > +}
> > +EXPORT_SYMBOL(drm_edid_has_quirk);
> > +
> >  #define MODE_SIZE(m) ((m)->hdisplay * (m)->vdisplay)
> >  #define MODE_REFRESH_DIFF(c,t) (abs((c) - (t)))
> >  
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 73903c3c842f3..137773dd138ea 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -843,7 +843,9 @@ struct drm_display_info {
> >  	int vics_len;
> >  
> >  	/**
> > -	 * @quirks: EDID based quirks. Internal to EDID parsing.
> > +	 * @quirks: EDID based quirks. DRM core and drivers can query the
> > +	 * @drm_edid_quirk quirks using drm_edid_has_quirk(), the rest of
> > +	 * the quirks also tracked here are internal to EDID parsing.
> >  	 */
> >  	u32 quirks;
> >  
> > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > index b38409670868d..77fd42608e706 100644
> > --- a/include/drm/drm_edid.h
> > +++ b/include/drm/drm_edid.h
> > @@ -109,6 +109,10 @@ struct detailed_data_string {
> >  #define DRM_EDID_CVT_FLAGS_STANDARD_BLANKING (1 << 3)
> >  #define DRM_EDID_CVT_FLAGS_REDUCED_BLANKING  (1 << 4)
> >  
> > +enum drm_edid_quirk {
> > +	DRM_EDID_QUIRK_NUM,
> > +};
> > +
> >  struct detailed_data_monitor_range {
> >  	u8 min_vfreq;
> >  	u8 max_vfreq;
> > @@ -476,5 +480,6 @@ void drm_edid_print_product_id(struct drm_printer *p,
> >  u32 drm_edid_get_panel_id(const struct drm_edid *drm_edid);
> >  bool drm_edid_match(const struct drm_edid *drm_edid,
> >  		    const struct drm_edid_ident *ident);
> > +bool drm_edid_has_quirk(struct drm_connector *connector, enum drm_edid_quirk quirk);
> >  
> >  #endif /* __DRM_EDID_H__ */
> 
> -- 
> Jani Nikula, Intel