[PATCH v3 0/2] Enhance EDID quirks to allow forcing a mode

[PATCH v3 0/2] Enhance EDID quirks to allow forcing a mode

[Dylan Semler]

Changes in this version
 * Uses drm_cvt_mode() instead of drm_gtf_mode() to build modeline
 * Adds bool to specify reduced blanking to edid_quirk_force_mode
 * Removes preferred bit from all other modes

There is at least one monitor that doesn't report its native resolution
in its EDID block.  This enhancement extends the EDID quirk logic to
make monitors like this "just work".

The first patch in this series sets up a new quirk list where monitors'
correct width, height, refresh rate, and reduced blanking parameters are
specified.  When a matching monitor is attached the full mode is
calculated with drm_cvt_mode() and added to the connector.  The
DRM_MODE_TYPE_PREFERRED bit is set on the new mode and unset from all
other modes.

The first patch also defines a new quirk bit: EDID_QUIRK_FORCE_MODE.
This bit needs to be set for the new quirk list discribed above to be
checked.

The second patch adds the offending monitor to the quirk lists.

Dylan Semler (2):
  drm: Enhance EDID quirks to explicitly set a mode
  drm: Add EDID force quirk for MMT Monitor2Go HD+

 drivers/gpu/drm/drm_edid.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

-- 
1.7.11.7

[PATCH v3 1/2] drm: Enhance EDID quirks to explicitly set a mode

[Dylan Semler]

There is at least one monitor that doesn't report its native resolution
in its EDID block.  This enhancement extends the EDID quirk logic to
make monitors like this "just work".

This patch sets up a new quirk list where monitors' correct width,
height, refresh rate, and reduced blanking parameters are specified.
When a matching monitor is attached the full mode is calculated with
drm_cvt_mode() and added to the connector.  The DRM_MODE_TYPE_PREFERRED
bit is set on the new mode and unset from all other modes.

The patch also defines a new quirk bit: EDID_QUIRK_FORCE_MODE.  This
bit needs to be set for the new quirk list discribed above to be
checked.

Signed-off-by: Dylan Semler <dylan.semler@gmail.com>
---
 drivers/gpu/drm/drm_edid.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index c194f4e..38b8641 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -68,6 +68,8 @@
 #define EDID_QUIRK_DETAILED_SYNC_PP		(1 << 6)
 /* Force reduced-blanking timings for detailed modes */
 #define EDID_QUIRK_FORCE_REDUCED_BLANKING	(1 << 7)
+/* Force specific mode for monitors that don't report correct EDIDs */
+#define EDID_QUIRK_FORCE_MODE			(1 << 8)
 
 struct detailed_mode_closure {
 	struct drm_connector *connector;
@@ -127,6 +129,16 @@ static struct edid_quirk {
 	{ "VSC", 5020, EDID_QUIRK_FORCE_REDUCED_BLANKING },
 };
 
+static struct edid_quirk_force_mode {
+	char vendor[4];
+	int product_id;
+	int hdisplay;
+	int vdisplay;
+	int vrefresh;
+	bool reduced;
+} edid_quirk_force_mode_list[] = {
+};
+
 /*
  * Autogenerated from the DMT spec.
  * This table is copied from xfree86/modes/xf86EdidModes.c.
@@ -2219,6 +2231,70 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
 	return closure.modes;
 }
 
+/* Add an explicit mode based on a quirk
+ */
+static int
+do_force_quirk_modes(struct drm_connector *connector, int hdisplay,
+		     int vdisplay, int vrefresh, bool reduced)
+{
+	struct drm_display_mode *mode, *t, *cur_mode;
+	struct drm_device *dev = connector->dev;
+	int num_modes = 0;
+
+	/* sanity check display parameters */
+	if (hdisplay < 0)
+		return 0;
+	if (vdisplay < 0)
+		return 0;
+	if (vrefresh < 0)
+		return 0;
+
+	/* loop through the probed modes and clear the preferred bit */
+	list_for_each_entry_safe(cur_mode, t, &connector->probed_modes, head)
+		cur_mode->type &= ~DRM_MODE_TYPE_PREFERRED;
+
+	mode = drm_cvt_mode(dev, hdisplay, vdisplay, vrefresh, reduced, 0, 0);
+
+	if (mode) {
+		mode->type |= DRM_MODE_TYPE_PREFERRED;
+		drm_mode_probed_add(connector, mode);
+		num_modes++;
+	}
+	return num_modes;
+}
+
+/*
+ * add_force_quirk_modes - Add modes based on monitor's EDID quirks
+ * @connector: attached connector
+ * @edid: EDID block to scan
+ * @quirks: quirks to apply
+ *
+ * At least one monitor doesn't report its native resolution in its EDID block.
+ * Here we add the native mode according to this quirk
+ */
+static int
+add_force_quirk_modes(struct drm_connector *connector, struct edid *edid,
+		      u32 quirks)
+{
+	struct edid_quirk_force_mode *quirk_mode;
+	int i, num_modes = 0;
+
+	for (i = 0; i < ARRAY_SIZE(edid_quirk_force_mode_list); i++) {
+		quirk_mode = &edid_quirk_force_mode_list[i];
+
+		if (edid_vendor(edid, quirk_mode->vendor) &&
+		    (EDID_PRODUCT_ID(edid) == quirk_mode->product_id)) {
+			num_modes = do_force_quirk_modes(connector,
+					quirk_mode->hdisplay,
+					quirk_mode->vdisplay,
+					quirk_mode->vrefresh,
+					quirk_mode->reduced);
+		}
+	}
+	return num_modes;
+
+}
+
 #define HDMI_IDENTIFIER 0x000C03
 #define AUDIO_BLOCK	0x01
 #define VIDEO_BLOCK     0x02
@@ -2803,6 +2879,8 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
 
 	if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75))
 		edid_fixup_preferred(connector, quirks);
+	if (quirks & EDID_QUIRK_FORCE_MODE)
+		num_modes += add_force_quirk_modes(connector, edid, quirks);
 
 	drm_add_display_info(edid, &connector->display_info);
 
-- 
1.7.11.7

[PATCH v3 2/2] drm: Add EDID force quirk for MMT Monitor2Go HD+

[Dylan Semler]

Set the new EDID_QUIRK_FORCE_MODE bit for the MMT Monitor2Go HD+ monitor
and add it to the edid_quirk_force_mode_list.

Signed-off-by: Dylan Semler <dylan.semler@gmail.com>
---
 drivers/gpu/drm/drm_edid.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 38b8641..2f30be4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -109,6 +109,9 @@ static struct edid_quirk {
 	{ "FCM", 13600, EDID_QUIRK_PREFER_LARGE_75 |
 	  EDID_QUIRK_DETAILED_IN_CM },
 
+	/* Mobile Monitor Technologies LLC, Monitor2Go HD+ */
+	{ "LLP", 0x4c54, EDID_QUIRK_FORCE_MODE},
+
 	/* LG Philips LCD LP154W01-A5 */
 	{ "LPL", 0, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE },
 	{ "LPL", 0x2a00, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE },
@@ -137,6 +140,8 @@ static struct edid_quirk_force_mode {
 	int vrefresh;
 	bool reduced;
 } edid_quirk_force_mode_list[] = {
+	/* Mobile Monitor Technologies LLC, Monitor2Go HD+ */
+	{ "LLP", 0x4c54, 1600, 900, 60, 1 },
 };
 
 /*
-- 
1.7.11.7

Re: [PATCH v3 0/2] Enhance EDID quirks to allow forcing a mode

[Daniel Vetter]

On Fri, Mar 22, 2013 at 07:08:05PM -0400, Dylan Semler wrote:
> Changes in this version
>  * Uses drm_cvt_mode() instead of drm_gtf_mode() to build modeline
>  * Adds bool to specify reduced blanking to edid_quirk_force_mode
>  * Removes preferred bit from all other modes
> 
> There is at least one monitor that doesn't report its native resolution
> in its EDID block.  This enhancement extends the EDID quirk logic to
> make monitors like this "just work".
> 
> The first patch in this series sets up a new quirk list where monitors'
> correct width, height, refresh rate, and reduced blanking parameters are
> specified.  When a matching monitor is attached the full mode is
> calculated with drm_cvt_mode() and added to the connector.  The
> DRM_MODE_TYPE_PREFERRED bit is set on the new mode and unset from all
> other modes.
> 
> The first patch also defines a new quirk bit: EDID_QUIRK_FORCE_MODE.
> This bit needs to be set for the new quirk list discribed above to be
> checked.
> 
> The second patch adds the offending monitor to the quirk lists.
> 
> Dylan Semler (2):
>   drm: Enhance EDID quirks to explicitly set a mode
>   drm: Add EDID force quirk for MMT Monitor2Go HD+

Ah, missed this resend of your patches. Looks good to me, but I guess Dave
want's to have Ajax's ack, too. Anyway for the series:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH v3 1/2] drm: Enhance EDID quirks to explicitly set a mode

[Jani Nikula]

Hi, please find some review comments inline.

BR,
Jani.

On Sat, 23 Mar 2013, Dylan Semler <dylan.semler@gmail.com> wrote:
> There is at least one monitor that doesn't report its native resolution
> in its EDID block.  This enhancement extends the EDID quirk logic to
> make monitors like this "just work".
>
> This patch sets up a new quirk list where monitors' correct width,
> height, refresh rate, and reduced blanking parameters are specified.
> When a matching monitor is attached the full mode is calculated with
> drm_cvt_mode() and added to the connector.  The DRM_MODE_TYPE_PREFERRED
> bit is set on the new mode and unset from all other modes.
>
> The patch also defines a new quirk bit: EDID_QUIRK_FORCE_MODE.  This
> bit needs to be set for the new quirk list discribed above to be
> checked.
>
> Signed-off-by: Dylan Semler <dylan.semler@gmail.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 78 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index c194f4e..38b8641 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -68,6 +68,8 @@
>  #define EDID_QUIRK_DETAILED_SYNC_PP		(1 << 6)
>  /* Force reduced-blanking timings for detailed modes */
>  #define EDID_QUIRK_FORCE_REDUCED_BLANKING	(1 << 7)
> +/* Force specific mode for monitors that don't report correct EDIDs */
> +#define EDID_QUIRK_FORCE_MODE			(1 << 8)
>  
>  struct detailed_mode_closure {
>  	struct drm_connector *connector;
> @@ -127,6 +129,16 @@ static struct edid_quirk {
>  	{ "VSC", 5020, EDID_QUIRK_FORCE_REDUCED_BLANKING },
>  };
>  
> +static struct edid_quirk_force_mode {
> +	char vendor[4];
> +	int product_id;
> +	int hdisplay;
> +	int vdisplay;
> +	int vrefresh;
> +	bool reduced;
> +} edid_quirk_force_mode_list[] = {
> +};
> +
>  /*
>   * Autogenerated from the DMT spec.
>   * This table is copied from xfree86/modes/xf86EdidModes.c.
> @@ -2219,6 +2231,70 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
>  	return closure.modes;
>  }
>  
> +/* Add an explicit mode based on a quirk
> + */
> +static int
> +do_force_quirk_modes(struct drm_connector *connector, int hdisplay,
> +		     int vdisplay, int vrefresh, bool reduced)

Nitpick: This adds one mode, not many _modes_.

> +{
> +	struct drm_display_mode *mode, *t, *cur_mode;
> +	struct drm_device *dev = connector->dev;
> +	int num_modes = 0;
> +
> +	/* sanity check display parameters */
> +	if (hdisplay < 0)
> +		return 0;
> +	if (vdisplay < 0)
> +		return 0;
> +	if (vrefresh < 0)
> +		return 0;
> +
> +	/* loop through the probed modes and clear the preferred bit */
> +	list_for_each_entry_safe(cur_mode, t, &connector->probed_modes, head)
> +		cur_mode->type &= ~DRM_MODE_TYPE_PREFERRED;

You're not deleting entries, so list_for_each_entry() would suffice,
getting rid of the temp variable.

> +
> +	mode = drm_cvt_mode(dev, hdisplay, vdisplay, vrefresh, reduced, 0, 0);

You could

	if (!mode)
		return 0;

here and get rid of the num_modes variable.

> +
> +	if (mode) {
> +		mode->type |= DRM_MODE_TYPE_PREFERRED;
> +		drm_mode_probed_add(connector, mode);
> +		num_modes++;
> +	}
> +	return num_modes;
> +}
> +
> +/*
> + * add_force_quirk_modes - Add modes based on monitor's EDID quirks
> + * @connector: attached connector
> + * @edid: EDID block to scan
> + * @quirks: quirks to apply
> + *
> + * At least one monitor doesn't report its native resolution in its EDID block.
> + * Here we add the native mode according to this quirk
> + */
> +static int
> +add_force_quirk_modes(struct drm_connector *connector, struct edid *edid,
> +		      u32 quirks)
> +{
> +	struct edid_quirk_force_mode *quirk_mode;
> +	int i, num_modes = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(edid_quirk_force_mode_list); i++) {
> +		quirk_mode = &edid_quirk_force_mode_list[i];
> +
> +		if (edid_vendor(edid, quirk_mode->vendor) &&
> +		    (EDID_PRODUCT_ID(edid) == quirk_mode->product_id)) {
> +			num_modes = do_force_quirk_modes(connector,
> +					quirk_mode->hdisplay,
> +					quirk_mode->vdisplay,
> +					quirk_mode->vrefresh,
> +					quirk_mode->reduced);

I was wondering why you don't bail out here. Maybe you want to be able
to add more than one quirk mode? In that case you should +=, not = the
num_modes.

(Note that then do_force_quirk_modes makes the last one in the array the
preferred mode, clearing the DRM_MODE_TYPE_PREFERRED from the previously
added quirk modes.)

> +		}
> +	}
> +	return num_modes;
> +
> +}
> +
>  #define HDMI_IDENTIFIER 0x000C03
>  #define AUDIO_BLOCK	0x01
>  #define VIDEO_BLOCK     0x02
> @@ -2803,6 +2879,8 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
>  
>  	if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75))
>  		edid_fixup_preferred(connector, quirks);
> +	if (quirks & EDID_QUIRK_FORCE_MODE)
> +		num_modes += add_force_quirk_modes(connector, edid, quirks);

You don't use quirks within add_force_quirk_modes() for anything.

>  
>  	drm_add_display_info(edid, &connector->display_info);
>  
> -- 
> 1.7.11.7
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/2] drm: Enhance EDID quirks to explicitly set a mode

[Dylan Semler]

[-- Attachment #1.1: Type: text/plain, Size: 3541 bytes --]

On Mon, Mar 25, 2013 at 9:41 AM, Jani Nikula <jani.nikula@linux.intel.com>
wrote:
>
>
> Hi, please find some review comments inline.
>

You make some good points, thanks.

> On Sat, 23 Mar 2013, Dylan Semler <dylan.semler@gmail.com> wrote:
> >
> > +/* Add an explicit mode based on a quirk
> > + */
> > +static int
> > +do_force_quirk_modes(struct drm_connector *connector, int hdisplay,
> > +                  int vdisplay, int vrefresh, bool reduced)
>
> Nitpick: This adds one mode, not many _modes_.

sure, I'll fix.


> > +
> > +     /* loop through the probed modes and clear the preferred bit */
> > +     list_for_each_entry_safe(cur_mode, t, &connector->probed_modes,
head)
> > +             cur_mode->type &= ~DRM_MODE_TYPE_PREFERRED;
>
> You're not deleting entries, so list_for_each_entry() would suffice,
> getting rid of the temp variable.
>

I was going to use this but I wasn't sure if I understood all of the
differences between the two.  I opted for _safe because I noticed it's used
in
edid_fixup_preferred() even though that routine doesn't remove entries
either.
I'll fix it.

> > +
> > +     mode = drm_cvt_mode(dev, hdisplay, vdisplay, vrefresh, reduced,
0, 0);
>
> You could
>
>         if (!mode)
>                 return 0;
>
> here and get rid of the num_modes variable.

Yes, you're right.  I went with

if (mode) {
    ...
    return 1;
}
else
    return 0;

I would naively expect this to be better.  Is there a reason to prefer if
(!mode)?

> > +static int
> > +add_force_quirk_modes(struct drm_connector *connector, struct edid
*edid,
> > +                   u32 quirks)
> > +{
> > +     struct edid_quirk_force_mode *quirk_mode;
> > +     int i, num_modes = 0;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(edid_quirk_force_mode_list); i++) {
> > +             quirk_mode = &edid_quirk_force_mode_list[i];
> > +
> > +             if (edid_vendor(edid, quirk_mode->vendor) &&
> > +                 (EDID_PRODUCT_ID(edid) == quirk_mode->product_id)) {
> > +                     num_modes = do_force_quirk_modes(connector,
> > +                                     quirk_mode->hdisplay,
> > +                                     quirk_mode->vdisplay,
> > +                                     quirk_mode->vrefresh,
> > +                                     quirk_mode->reduced);
>
> I was wondering why you don't bail out here. Maybe you want to be able
> to add more than one quirk mode? In that case you should +=, not = the
> num_modes.
>
> (Note that then do_force_quirk_modes makes the last one in the array the
> preferred mode, clearing the DRM_MODE_TYPE_PREFERRED from the previously
> added quirk modes.)

I thought about whether multiple forced modes would ever be needed.  I can't
imagine it would but it's easy enough to allow for it so we might as well
implement it now.

I think we'd want for it to be clear which of the forced modes will be used
and
so the current behavior of making the last one in the array preferred may be
the best.  I will add a comment by the edid_quirk_force_mode_list describing
this.


> > @@ -2803,6 +2879,8 @@ int drm_add_edid_modes(struct drm_connector
*connector, struct edid *edid)
> >
> >       if (quirks & (EDID_QUIRK_PREFER_LARGE_60 |
EDID_QUIRK_PREFER_LARGE_75))
> >               edid_fixup_preferred(connector, quirks);
> > +     if (quirks & EDID_QUIRK_FORCE_MODE)
> > +             num_modes += add_force_quirk_modes(connector, edid,
quirks);
>
> You don't use quirks within add_force_quirk_modes() for anything.

good point.  I'll remove it.


Thanks,
Dylan

[-- Attachment #1.2: Type: text/html, Size: 5401 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/2] drm: Enhance EDID quirks to explicitly set a mode

[Jani Nikula]

On Mon, 25 Mar 2013, Dylan Semler <dylan.semler@gmail.com> wrote:
> On Mon, Mar 25, 2013 at 9:41 AM, Jani Nikula <jani.nikula@linux.intel.com>
> wrote:
>>
>>
>> Hi, please find some review comments inline.
>>
>
> You make some good points, thanks.
>
>> On Sat, 23 Mar 2013, Dylan Semler <dylan.semler@gmail.com> wrote:
>> >
>> > +/* Add an explicit mode based on a quirk
>> > + */
>> > +static int
>> > +do_force_quirk_modes(struct drm_connector *connector, int hdisplay,
>> > +                  int vdisplay, int vrefresh, bool reduced)
>>
>> Nitpick: This adds one mode, not many _modes_.
>
> sure, I'll fix.
>
>
>> > +
>> > +     /* loop through the probed modes and clear the preferred bit */
>> > +     list_for_each_entry_safe(cur_mode, t, &connector->probed_modes,
> head)
>> > +             cur_mode->type &= ~DRM_MODE_TYPE_PREFERRED;
>>
>> You're not deleting entries, so list_for_each_entry() would suffice,
>> getting rid of the temp variable.
>>
>
> I was going to use this but I wasn't sure if I understood all of the
> differences between the two.  I opted for _safe because I noticed it's used
> in
> edid_fixup_preferred() even though that routine doesn't remove entries
> either.
> I'll fix it.
>
>> > +
>> > +     mode = drm_cvt_mode(dev, hdisplay, vdisplay, vrefresh, reduced,
> 0, 0);
>>
>> You could
>>
>>         if (!mode)
>>                 return 0;
>>
>> here and get rid of the num_modes variable.
>
> Yes, you're right.  I went with
>
> if (mode) {
>     ...
>     return 1;
> }
> else
>     return 0;

Note that you should use braces in all branches if one branch requires
them. See Documentation/CodingStyle.

>
> I would naively expect this to be better.  Is there a reason to prefer if
> (!mode)?

Just a personal preference of checking errors and bailing out early, and
handling the happy day scenario at the top indentation level. I won't
insist.


BR,
Jani.


>
>> > +static int
>> > +add_force_quirk_modes(struct drm_connector *connector, struct edid
> *edid,
>> > +                   u32 quirks)
>> > +{
>> > +     struct edid_quirk_force_mode *quirk_mode;
>> > +     int i, num_modes = 0;
>> > +
>> > +     for (i = 0; i < ARRAY_SIZE(edid_quirk_force_mode_list); i++) {
>> > +             quirk_mode = &edid_quirk_force_mode_list[i];
>> > +
>> > +             if (edid_vendor(edid, quirk_mode->vendor) &&
>> > +                 (EDID_PRODUCT_ID(edid) == quirk_mode->product_id)) {
>> > +                     num_modes = do_force_quirk_modes(connector,
>> > +                                     quirk_mode->hdisplay,
>> > +                                     quirk_mode->vdisplay,
>> > +                                     quirk_mode->vrefresh,
>> > +                                     quirk_mode->reduced);
>>
>> I was wondering why you don't bail out here. Maybe you want to be able
>> to add more than one quirk mode? In that case you should +=, not = the
>> num_modes.
>>
>> (Note that then do_force_quirk_modes makes the last one in the array the
>> preferred mode, clearing the DRM_MODE_TYPE_PREFERRED from the previously
>> added quirk modes.)
>
> I thought about whether multiple forced modes would ever be needed.  I can't
> imagine it would but it's easy enough to allow for it so we might as well
> implement it now.
>
> I think we'd want for it to be clear which of the forced modes will be used
> and
> so the current behavior of making the last one in the array preferred may be
> the best.  I will add a comment by the edid_quirk_force_mode_list describing
> this.
>
>
>> > @@ -2803,6 +2879,8 @@ int drm_add_edid_modes(struct drm_connector
> *connector, struct edid *edid)
>> >
>> >       if (quirks & (EDID_QUIRK_PREFER_LARGE_60 |
> EDID_QUIRK_PREFER_LARGE_75))
>> >               edid_fixup_preferred(connector, quirks);
>> > +     if (quirks & EDID_QUIRK_FORCE_MODE)
>> > +             num_modes += add_force_quirk_modes(connector, edid,
> quirks);
>>
>> You don't use quirks within add_force_quirk_modes() for anything.
>
> good point.  I'll remove it.
>
>
> Thanks,
> Dylan