diff mbox series

[07/11] drm/edid: Use GTF2 for inferred modes

Message ID 20220826213501.31490-8-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/edid: Range descriptor stuff | expand

Commit Message

Ville Syrjala Aug. 26, 2022, 9:34 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

For some resaon we only use the secondary GTF curve for the
standard timings. Use it for inferred modes as well.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

Comments

Jani Nikula Sept. 2, 2022, 12:25 p.m. UTC | #1 
On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> For some resaon we only use the secondary GTF curve for the
> standard timings. Use it for inferred modes as well.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 0c7cbe9b44f5..fed2bdd55c09 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3546,6 +3546,35 @@ static int drm_gtf_modes_for_range(struct drm_connector *connector,
>  	return modes;
>  }
>  
> +static int drm_gtf2_modes_for_range(struct drm_connector *connector,
> +				    const struct drm_edid *drm_edid,
> +				    const struct detailed_timing *timing)
> +{
> +	int i, modes = 0;
> +	struct drm_display_mode *newmode;
> +	struct drm_device *dev = connector->dev;
> +
> +	for (i = 0; i < ARRAY_SIZE(extra_modes); i++) {
> +		const struct minimode *m = &extra_modes[i];
> +
> +		newmode = drm_gtf2_mode(dev, drm_edid, m->w, m->h, m->r);
> +		if (!newmode)
> +			return modes;
> +
> +		drm_mode_fixup_1366x768(newmode);
> +		if (!mode_in_range(newmode, drm_edid, timing) ||
> +		    !valid_inferred_mode(connector, newmode)) {
> +			drm_mode_destroy(dev, newmode);
> +			continue;
> +		}
> +
> +		drm_mode_probed_add(connector, newmode);
> +		modes++;
> +	}
> +
> +	return modes;
> +}
> +
>  static int drm_cvt_modes_for_range(struct drm_connector *connector,
>  				   const struct drm_edid *drm_edid,
>  				   const struct detailed_timing *timing)
> @@ -3594,7 +3623,11 @@ do_inferred_modes(const struct detailed_timing *timing, void *c)
>  		return; /* GTF not defined yet */
>  
>  	switch (range->flags) {
> -	case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG: /* XXX could do more */
> +	case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG:
> +		closure->modes += drm_gtf2_modes_for_range(closure->connector,
> +							   closure->drm_edid,
> +							   timing);
> +		break;
>  	case DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG:

Additionally, per spec:

* Default GTF supported if bit 0 in Feature Support Byte at address 18h = 1

* Secondary GTF supported --- requires support for Default GTF

So I guess both of these would need the edid->features &
DRM_EDID_FEATURE_DEFAULT_GTF check?

Other than that,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>



>  		closure->modes += drm_gtf_modes_for_range(closure->connector,
>  							  closure->drm_edid,
Ville Syrjala Sept. 2, 2022, 12:45 p.m. UTC | #2 
On Fri, Sep 02, 2022 at 03:25:39PM +0300, Jani Nikula wrote:
> On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > For some resaon we only use the secondary GTF curve for the
> > standard timings. Use it for inferred modes as well.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_edid.c | 35 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 0c7cbe9b44f5..fed2bdd55c09 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -3546,6 +3546,35 @@ static int drm_gtf_modes_for_range(struct drm_connector *connector,
> >  	return modes;
> >  }
> >  
> > +static int drm_gtf2_modes_for_range(struct drm_connector *connector,
> > +				    const struct drm_edid *drm_edid,
> > +				    const struct detailed_timing *timing)
> > +{
> > +	int i, modes = 0;
> > +	struct drm_display_mode *newmode;
> > +	struct drm_device *dev = connector->dev;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(extra_modes); i++) {
> > +		const struct minimode *m = &extra_modes[i];
> > +
> > +		newmode = drm_gtf2_mode(dev, drm_edid, m->w, m->h, m->r);
> > +		if (!newmode)
> > +			return modes;
> > +
> > +		drm_mode_fixup_1366x768(newmode);
> > +		if (!mode_in_range(newmode, drm_edid, timing) ||
> > +		    !valid_inferred_mode(connector, newmode)) {
> > +			drm_mode_destroy(dev, newmode);
> > +			continue;
> > +		}
> > +
> > +		drm_mode_probed_add(connector, newmode);
> > +		modes++;
> > +	}
> > +
> > +	return modes;
> > +}
> > +
> >  static int drm_cvt_modes_for_range(struct drm_connector *connector,
> >  				   const struct drm_edid *drm_edid,
> >  				   const struct detailed_timing *timing)
> > @@ -3594,7 +3623,11 @@ do_inferred_modes(const struct detailed_timing *timing, void *c)
> >  		return; /* GTF not defined yet */
> >  
> >  	switch (range->flags) {
> > -	case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG: /* XXX could do more */
> > +	case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG:
> > +		closure->modes += drm_gtf2_modes_for_range(closure->connector,
> > +							   closure->drm_edid,
> > +							   timing);
> > +		break;
> >  	case DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG:
> 
> Additionally, per spec:
> 
> * Default GTF supported if bit 0 in Feature Support Byte at address 18h = 1
> 
> * Secondary GTF supported --- requires support for Default GTF
> 
> So I guess both of these would need the edid->features &
> DRM_EDID_FEATURE_DEFAULT_GTF check?

There is one actually
	if (drm_edid->edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
	                num_modes += add_inferred_modes(connector, drm_edid);

Though as I think I mentioned in some of these patches a lot of
real world EDIDs don't set the default gtf/continuous frequency
bit but still include a range descriptor. While illegal, I think
a reasonable interpretation might be that they want us to use 
the formula specified in the range descriptor for the non-DMT
standard timings, while still indicating that other timings
generated by said formula are not supported. 

> 
> Other than that,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> 
> >  		closure->modes += drm_gtf_modes_for_range(closure->connector,
> >  							  closure->drm_edid,
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0c7cbe9b44f5..fed2bdd55c09 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3546,6 +3546,35 @@  static int drm_gtf_modes_for_range(struct drm_connector *connector,
 	return modes;
 }
 
+static int drm_gtf2_modes_for_range(struct drm_connector *connector,
+				    const struct drm_edid *drm_edid,
+				    const struct detailed_timing *timing)
+{
+	int i, modes = 0;
+	struct drm_display_mode *newmode;
+	struct drm_device *dev = connector->dev;
+
+	for (i = 0; i < ARRAY_SIZE(extra_modes); i++) {
+		const struct minimode *m = &extra_modes[i];
+
+		newmode = drm_gtf2_mode(dev, drm_edid, m->w, m->h, m->r);
+		if (!newmode)
+			return modes;
+
+		drm_mode_fixup_1366x768(newmode);
+		if (!mode_in_range(newmode, drm_edid, timing) ||
+		    !valid_inferred_mode(connector, newmode)) {
+			drm_mode_destroy(dev, newmode);
+			continue;
+		}
+
+		drm_mode_probed_add(connector, newmode);
+		modes++;
+	}
+
+	return modes;
+}
+
 static int drm_cvt_modes_for_range(struct drm_connector *connector,
 				   const struct drm_edid *drm_edid,
 				   const struct detailed_timing *timing)
@@ -3594,7 +3623,11 @@  do_inferred_modes(const struct detailed_timing *timing, void *c)
 		return; /* GTF not defined yet */
 
 	switch (range->flags) {
-	case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG: /* XXX could do more */
+	case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG:
+		closure->modes += drm_gtf2_modes_for_range(closure->connector,
+							   closure->drm_edid,
+							   timing);
+		break;
 	case DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG:
 		closure->modes += drm_gtf_modes_for_range(closure->connector,
 							  closure->drm_edid,