✗ Fi.CI.CHECKPATCH: warning for drm/i915: add LG eDP panel to quirk database

✗ Fi.CI.CHECKPATCH: warning for drm/i915: add LG eDP panel to quirk database

[Patchwork]

== Series Details ==

Series: drm/i915: add LG eDP panel to quirk database
URL   : https://patchwork.freedesktop.org/series/49251/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
e5c2ff993f03 drm/i915: add LG eDP panel to quirk database
-:36: WARNING:LONG_LINE: line over 100 characters
#36: FILE: drivers/gpu/drm/drm_dp_helper.c:1271:
+	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N), DEVICE_ID(0, 0, 0, 0, 0, 0) },

-:38: WARNING:LONG_LINE: line over 100 characters
#38: FILE: drivers/gpu/drm/drm_dp_helper.c:1273:
+	{ OUI(0x00, 0x22, 0xb9), false, BIT(DP_DPCD_QUIRK_CONSTANT_N), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T') },

total: 0 errors, 2 warnings, 0 checks, 127 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for drm/i915: add LG eDP panel to quirk database

[Patchwork]

== Series Details ==

Series: drm/i915: add LG eDP panel to quirk database
URL   : https://patchwork.freedesktop.org/series/49251/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: add LG eDP panel to quirk database
-O:drivers/gpu/drm/i915/intel_display.c:6702:18: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_display.c:6705:26: warning: expression using sizeof(void)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: add LG eDP panel to quirk database

[Lee, Shawn C]

The N value was computed by kernel driver that based on synchronous clock
mode. But only specific N value (0x8000) would be acceptable for
LG LP140WF6-SPM1 eDP panel which is running at asynchronous clock mode.
With the other N value, Tcon will enter BITS mode and display black screen.
Add this panel into quirk database and give particular N value when
calculate M/N divider.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c      | 10 +++++++++-
 drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++------
 drivers/gpu/drm/i915/intel_display.h |  3 ++-
 drivers/gpu/drm/i915/intel_dp.c      |  8 ++++++--
 drivers/gpu/drm/i915/intel_dp_mst.c  |  2 +-
 include/drm/drm_dp_helper.h          |  1 +
 6 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 0cccbcb2d03e..a0259bbbe9df 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1258,13 +1258,18 @@ struct dpcd_quirk {
 	u8 oui[3];
 	bool is_branch;
 	u32 quirks;
+	u8 dev_id[6];
 };
 
 #define OUI(first, second, third) { (first), (second), (third) }
+#define DEVICE_ID(first, second, third, fourth, fifth, sixth) \
+	{ (first), (second), (third), (fourth), (fifth), (sixth) }
 
 static const struct dpcd_quirk dpcd_quirk_list[] = {
 	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
-	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
+	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N), DEVICE_ID(0, 0, 0, 0, 0, 0) },
+	/* LG LP140WF6-SPM1 eDP panel */
+	{ OUI(0x00, 0x22, 0xb9), false, BIT(DP_DPCD_QUIRK_CONSTANT_N), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T') },
 };
 
 #undef OUI
@@ -1293,6 +1298,9 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
 		if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
 			continue;
 
+		if (!is_branch && memcmp(quirk->dev_id, ident->device_id, 6) != 0)
+			continue;
+
 		quirks |= quirk->quirks;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ec3e24f07486..ff01a742b054 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6465,7 +6465,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
 	pipe_config->fdi_lanes = lane;
 
 	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
-			       link_bw, &pipe_config->fdi_m_n, false);
+			       link_bw, &pipe_config->fdi_m_n, false, false);
 
 	ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
 	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
@@ -6680,7 +6680,7 @@ intel_reduce_m_n_ratio(uint32_t *num, uint32_t *den)
 
 static void compute_m_n(unsigned int m, unsigned int n,
 			uint32_t *ret_m, uint32_t *ret_n,
-			bool reduce_m_n)
+			bool reduce_m_n, bool constant_n)
 {
 	/*
 	 * Reduce M/N as much as possible without loss in precision. Several DP
@@ -6695,7 +6695,11 @@ static void compute_m_n(unsigned int m, unsigned int n,
 		}
 	}
 
-	*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
+	if (constant_n)
+		*ret_n = 0x8000;
+	else
+		*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
+
 	*ret_m = div_u64((uint64_t) m * *ret_n, n);
 	intel_reduce_m_n_ratio(ret_m, ret_n);
 }
@@ -6704,18 +6708,18 @@ void
 intel_link_compute_m_n(int bits_per_pixel, int nlanes,
 		       int pixel_clock, int link_clock,
 		       struct intel_link_m_n *m_n,
-		       bool reduce_m_n)
+		       bool reduce_m_n, bool constant_n)
 {
 	m_n->tu = 64;
 
 	compute_m_n(bits_per_pixel * pixel_clock,
 		    link_clock * nlanes * 8,
 		    &m_n->gmch_m, &m_n->gmch_n,
-		    reduce_m_n);
+		    reduce_m_n, constant_n);
 
 	compute_m_n(pixel_clock, link_clock,
 		    &m_n->link_m, &m_n->link_n,
-		    reduce_m_n);
+		    reduce_m_n, constant_n);
 }
 
 static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index 43f080c6538d..dde7d73bd36d 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -379,7 +379,8 @@ struct intel_link_m_n {
 void intel_link_compute_m_n(int bpp, int nlanes,
 			    int pixel_clock, int link_clock,
 			    struct intel_link_m_n *m_n,
-			    bool reduce_m_n);
+			    bool reduce_m_n,
+			    bool constant_n);
 
 bool is_ccs_modifier(u64 modifier);
 #endif
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 436c22de33b6..95edbac24919 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2000,6 +2000,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		to_intel_digital_connector_state(conn_state);
 	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
 					   DP_DPCD_QUIRK_LIMITED_M_N);
+	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
+					   DP_DPCD_QUIRK_CONSTANT_N);
 
 	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
 		pipe_config->has_pch_encoder = true;
@@ -2064,7 +2066,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 			       adjusted_mode->crtc_clock,
 			       pipe_config->port_clock,
 			       &pipe_config->dp_m_n,
-			       reduce_m_n);
+			       reduce_m_n,
+			       constant_n);
 
 	if (intel_connector->panel.downclock_mode != NULL &&
 		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
@@ -2074,7 +2077,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 					       intel_connector->panel.downclock_mode->clock,
 					       pipe_config->port_clock,
 					       &pipe_config->dp_m2_n2,
-					       reduce_m_n);
+					       reduce_m_n,
+					       constant_n);
 	}
 
 	if (!HAS_DDI(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 352e5216cc65..ac23982fa335 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -87,7 +87,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 			       adjusted_mode->crtc_clock,
 			       pipe_config->port_clock,
 			       &pipe_config->dp_m_n,
-			       reduce_m_n);
+			       reduce_m_n, false);
 
 	pipe_config->dp_m_n.tu = slots;
 
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 05cc31b5db16..b1b2fd609c1c 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1266,6 +1266,7 @@ enum drm_dp_quirk {
 	 * to 16 bits.
 	 */
 	DP_DPCD_QUIRK_LIMITED_M_N,
+	DP_DPCD_QUIRK_CONSTANT_N,
 };
 
 /**
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: add LG eDP panel to quirk database

[Patchwork]

== Series Details ==

Series: drm/i915: add LG eDP panel to quirk database
URL   : https://patchwork.freedesktop.org/series/49251/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4776 -> Patchwork_10105 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49251/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10105 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@read-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     INCOMPLETE (fdo#102657) -> PASS

    
  fdo#102657 https://bugs.freedesktop.org/show_bug.cgi?id=102657
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362


== Participating hosts (53 -> 47) ==

  Missing    (6): fi-hsw-4770r fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_4776 -> Patchwork_10105

  CI_DRM_4776: 18e1a6da2ae33504c83d1e5dfc7a08a3940e7b58 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4631: 8884101aa01aedee01b2c3d0ac075473384551b7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10105: e5c2ff993f03a1fc46874810f8c730f222d9e7ca @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e5c2ff993f03 drm/i915: add LG eDP panel to quirk database

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10105/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: add LG eDP panel to quirk database

[Jani Nikula]

On Thu, 06 Sep 2018, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> The N value was computed by kernel driver that based on synchronous clock
> mode. But only specific N value (0x8000) would be acceptable for
> LG LP140WF6-SPM1 eDP panel which is running at asynchronous clock mode.

As I wrote before, it's the DP source that determines between
synchronous and asynchronous clock mode, not the sink. The sink in
question appears to expect a constant N related to asynchronous clock
mode even when we're using and advertising synchronous clock mode in DP
Main Stream Attributes.

(It's possible a certain other OS uses the same constant N regardless of
clock mode, leading to the panel working by coincidence.)

> With the other N value, Tcon will enter BITS mode and display black screen.
> Add this panel into quirk database and give particular N value when
> calculate M/N divider.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c      | 10 +++++++++-
>  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++------
>  drivers/gpu/drm/i915/intel_display.h |  3 ++-
>  drivers/gpu/drm/i915/intel_dp.c      |  8 ++++++--
>  drivers/gpu/drm/i915/intel_dp_mst.c  |  2 +-
>  include/drm/drm_dp_helper.h          |  1 +
>  6 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 0cccbcb2d03e..a0259bbbe9df 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1258,13 +1258,18 @@ struct dpcd_quirk {
>  	u8 oui[3];
>  	bool is_branch;
>  	u32 quirks;
> +	u8 dev_id[6];

Your commit message says nothing about the need to extend the quirk
mechanism to use the device ID. Do we really need it?

If we do need it, please name it device_id like it is in struct
drm_dp_dpcd_ident, and place it after oui.

>  };
>  
>  #define OUI(first, second, third) { (first), (second), (third) }
> +#define DEVICE_ID(first, second, third, fourth, fifth, sixth) \
> +	{ (first), (second), (third), (fourth), (fifth), (sixth) }
>  
>  static const struct dpcd_quirk dpcd_quirk_list[] = {
>  	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
> -	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
> +	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N), DEVICE_ID(0, 0, 0, 0, 0, 0) },

Maybe add #define DEVICE_ID_ANY to initialize all to zero.

> +	/* LG LP140WF6-SPM1 eDP panel */
> +	{ OUI(0x00, 0x22, 0xb9), false, BIT(DP_DPCD_QUIRK_CONSTANT_N), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T') },
>  };
>  
>  #undef OUI

#undef DEVICE_ID
#undef DEVICE_ID_ANY

> @@ -1293,6 +1298,9 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
>  		if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
>  			continue;
>  
> +		if (!is_branch && memcmp(quirk->dev_id, ident->device_id, 6) != 0)
> +			continue;
> +

Why not branch devices? This is really a trick to avoid matching on the
Analogix device which you set to zeros, and now you require device id
match for all non-branch quirks. Not good. Please check if the quirk
table has all zeros, and compare if not.

>  		quirks |= quirk->quirks;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ec3e24f07486..ff01a742b054 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6465,7 +6465,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
>  	pipe_config->fdi_lanes = lane;
>  
>  	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> -			       link_bw, &pipe_config->fdi_m_n, false);
> +			       link_bw, &pipe_config->fdi_m_n, false, false);

This is getting pretty ugly, to be honest. Which is why I was wondering
if we could actually turn the Analogix quirk into a constant N...

>  
>  	ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
>  	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
> @@ -6680,7 +6680,7 @@ intel_reduce_m_n_ratio(uint32_t *num, uint32_t *den)
>  
>  static void compute_m_n(unsigned int m, unsigned int n,
>  			uint32_t *ret_m, uint32_t *ret_n,
> -			bool reduce_m_n)
> +			bool reduce_m_n, bool constant_n)
>  {
>  	/*
>  	 * Reduce M/N as much as possible without loss in precision. Several DP
> @@ -6695,7 +6695,11 @@ static void compute_m_n(unsigned int m, unsigned int n,
>  		}
>  	}
>  
> -	*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
> +	if (constant_n)
> +		*ret_n = 0x8000;
> +	else
> +		*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
> +
>  	*ret_m = div_u64((uint64_t) m * *ret_n, n);
>  	intel_reduce_m_n_ratio(ret_m, ret_n);
>  }
> @@ -6704,18 +6708,18 @@ void
>  intel_link_compute_m_n(int bits_per_pixel, int nlanes,
>  		       int pixel_clock, int link_clock,
>  		       struct intel_link_m_n *m_n,
> -		       bool reduce_m_n)
> +		       bool reduce_m_n, bool constant_n)
>  {
>  	m_n->tu = 64;
>  
>  	compute_m_n(bits_per_pixel * pixel_clock,
>  		    link_clock * nlanes * 8,
>  		    &m_n->gmch_m, &m_n->gmch_n,
> -		    reduce_m_n);
> +		    reduce_m_n, constant_n);
>  
>  	compute_m_n(pixel_clock, link_clock,
>  		    &m_n->link_m, &m_n->link_n,
> -		    reduce_m_n);
> +		    reduce_m_n, constant_n);
>  }
>  
>  static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> index 43f080c6538d..dde7d73bd36d 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -379,7 +379,8 @@ struct intel_link_m_n {
>  void intel_link_compute_m_n(int bpp, int nlanes,
>  			    int pixel_clock, int link_clock,
>  			    struct intel_link_m_n *m_n,
> -			    bool reduce_m_n);
> +			    bool reduce_m_n,
> +			    bool constant_n);
>  
>  bool is_ccs_modifier(u64 modifier);
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 436c22de33b6..95edbac24919 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2000,6 +2000,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  		to_intel_digital_connector_state(conn_state);
>  	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
>  					   DP_DPCD_QUIRK_LIMITED_M_N);
> +	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
> +					   DP_DPCD_QUIRK_CONSTANT_N);
>  
>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
>  		pipe_config->has_pch_encoder = true;
> @@ -2064,7 +2066,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  			       adjusted_mode->crtc_clock,
>  			       pipe_config->port_clock,
>  			       &pipe_config->dp_m_n,
> -			       reduce_m_n);
> +			       reduce_m_n,
> +			       constant_n);
>  
>  	if (intel_connector->panel.downclock_mode != NULL &&
>  		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> @@ -2074,7 +2077,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  					       intel_connector->panel.downclock_mode->clock,
>  					       pipe_config->port_clock,
>  					       &pipe_config->dp_m2_n2,
> -					       reduce_m_n);
> +					       reduce_m_n,
> +					       constant_n);
>  	}
>  
>  	if (!HAS_DDI(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 352e5216cc65..ac23982fa335 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -87,7 +87,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  			       adjusted_mode->crtc_clock,
>  			       pipe_config->port_clock,
>  			       &pipe_config->dp_m_n,
> -			       reduce_m_n);
> +			       reduce_m_n, false);
>  
>  	pipe_config->dp_m_n.tu = slots;
>  
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 05cc31b5db16..b1b2fd609c1c 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1266,6 +1266,7 @@ enum drm_dp_quirk {
>  	 * to 16 bits.
>  	 */
>  	DP_DPCD_QUIRK_LIMITED_M_N,
> +	DP_DPCD_QUIRK_CONSTANT_N,

Documentation missing. See the comment for limited M/N.

BR,
Jani.

>  };
>  
>  /**

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: add LG eDP panel to quirk database

[Jani Nikula]

On Thu, 06 Sep 2018, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> The N value was computed by kernel driver that based on synchronous clock
> mode. But only specific N value (0x8000) would be acceptable for
> LG LP140WF6-SPM1 eDP panel which is running at asynchronous clock mode.
> With the other N value, Tcon will enter BITS mode and display black screen.
> Add this panel into quirk database and give particular N value when
> calculate M/N divider.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c      | 10 +++++++++-

PS. This is not drm/i915. This is DRM.

>  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++------
>  drivers/gpu/drm/i915/intel_display.h |  3 ++-
>  drivers/gpu/drm/i915/intel_dp.c      |  8 ++++++--
>  drivers/gpu/drm/i915/intel_dp_mst.c  |  2 +-
>  include/drm/drm_dp_helper.h          |  1 +
>  6 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 0cccbcb2d03e..a0259bbbe9df 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1258,13 +1258,18 @@ struct dpcd_quirk {
>  	u8 oui[3];
>  	bool is_branch;
>  	u32 quirks;
> +	u8 dev_id[6];
>  };
>  
>  #define OUI(first, second, third) { (first), (second), (third) }
> +#define DEVICE_ID(first, second, third, fourth, fifth, sixth) \
> +	{ (first), (second), (third), (fourth), (fifth), (sixth) }
>  
>  static const struct dpcd_quirk dpcd_quirk_list[] = {
>  	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
> -	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
> +	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N), DEVICE_ID(0, 0, 0, 0, 0, 0) },
> +	/* LG LP140WF6-SPM1 eDP panel */
> +	{ OUI(0x00, 0x22, 0xb9), false, BIT(DP_DPCD_QUIRK_CONSTANT_N), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T') },
>  };
>  
>  #undef OUI
> @@ -1293,6 +1298,9 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
>  		if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
>  			continue;
>  
> +		if (!is_branch && memcmp(quirk->dev_id, ident->device_id, 6) != 0)
> +			continue;
> +
>  		quirks |= quirk->quirks;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ec3e24f07486..ff01a742b054 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6465,7 +6465,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
>  	pipe_config->fdi_lanes = lane;
>  
>  	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> -			       link_bw, &pipe_config->fdi_m_n, false);
> +			       link_bw, &pipe_config->fdi_m_n, false, false);
>  
>  	ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
>  	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
> @@ -6680,7 +6680,7 @@ intel_reduce_m_n_ratio(uint32_t *num, uint32_t *den)
>  
>  static void compute_m_n(unsigned int m, unsigned int n,
>  			uint32_t *ret_m, uint32_t *ret_n,
> -			bool reduce_m_n)
> +			bool reduce_m_n, bool constant_n)
>  {
>  	/*
>  	 * Reduce M/N as much as possible without loss in precision. Several DP
> @@ -6695,7 +6695,11 @@ static void compute_m_n(unsigned int m, unsigned int n,
>  		}
>  	}
>  
> -	*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
> +	if (constant_n)
> +		*ret_n = 0x8000;
> +	else
> +		*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
> +
>  	*ret_m = div_u64((uint64_t) m * *ret_n, n);
>  	intel_reduce_m_n_ratio(ret_m, ret_n);
>  }
> @@ -6704,18 +6708,18 @@ void
>  intel_link_compute_m_n(int bits_per_pixel, int nlanes,
>  		       int pixel_clock, int link_clock,
>  		       struct intel_link_m_n *m_n,
> -		       bool reduce_m_n)
> +		       bool reduce_m_n, bool constant_n)
>  {
>  	m_n->tu = 64;
>  
>  	compute_m_n(bits_per_pixel * pixel_clock,
>  		    link_clock * nlanes * 8,
>  		    &m_n->gmch_m, &m_n->gmch_n,
> -		    reduce_m_n);
> +		    reduce_m_n, constant_n);
>  
>  	compute_m_n(pixel_clock, link_clock,
>  		    &m_n->link_m, &m_n->link_n,
> -		    reduce_m_n);
> +		    reduce_m_n, constant_n);
>  }
>  
>  static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> index 43f080c6538d..dde7d73bd36d 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -379,7 +379,8 @@ struct intel_link_m_n {
>  void intel_link_compute_m_n(int bpp, int nlanes,
>  			    int pixel_clock, int link_clock,
>  			    struct intel_link_m_n *m_n,
> -			    bool reduce_m_n);
> +			    bool reduce_m_n,
> +			    bool constant_n);
>  
>  bool is_ccs_modifier(u64 modifier);
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 436c22de33b6..95edbac24919 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2000,6 +2000,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  		to_intel_digital_connector_state(conn_state);
>  	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
>  					   DP_DPCD_QUIRK_LIMITED_M_N);
> +	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
> +					   DP_DPCD_QUIRK_CONSTANT_N);
>  
>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
>  		pipe_config->has_pch_encoder = true;
> @@ -2064,7 +2066,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  			       adjusted_mode->crtc_clock,
>  			       pipe_config->port_clock,
>  			       &pipe_config->dp_m_n,
> -			       reduce_m_n);
> +			       reduce_m_n,
> +			       constant_n);
>  
>  	if (intel_connector->panel.downclock_mode != NULL &&
>  		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> @@ -2074,7 +2077,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  					       intel_connector->panel.downclock_mode->clock,
>  					       pipe_config->port_clock,
>  					       &pipe_config->dp_m2_n2,
> -					       reduce_m_n);
> +					       reduce_m_n,
> +					       constant_n);
>  	}
>  
>  	if (!HAS_DDI(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 352e5216cc65..ac23982fa335 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -87,7 +87,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  			       adjusted_mode->crtc_clock,
>  			       pipe_config->port_clock,
>  			       &pipe_config->dp_m_n,
> -			       reduce_m_n);
> +			       reduce_m_n, false);
>  
>  	pipe_config->dp_m_n.tu = slots;
>  
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 05cc31b5db16..b1b2fd609c1c 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1266,6 +1266,7 @@ enum drm_dp_quirk {
>  	 * to 16 bits.
>  	 */
>  	DP_DPCD_QUIRK_LIMITED_M_N,
> +	DP_DPCD_QUIRK_CONSTANT_N,
>  };
>  
>  /**

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for drm/i915: add LG eDP panel to quirk database

[Patchwork]

== Series Details ==

Series: drm/i915: add LG eDP panel to quirk database
URL   : https://patchwork.freedesktop.org/series/49251/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4776_full -> Patchwork_10105_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_10105_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10105_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10105_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-blt:
      shard-kbl:          PASS -> DMESG-FAIL

    
== Known issues ==

  Here are the changes found in Patchwork_10105_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@preempt-queue-contexts-chain-blt:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    igt@kms_cursor_legacy@cursor-vs-flip-toggle:
      shard-hsw:          PASS -> FAIL (fdo#103355)

    
    ==== Possible fixes ====

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_frontbuffer_tracking@fbc-badstride:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_setmode@basic:
      shard-kbl:          FAIL (fdo#99912) -> PASS

    igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4776 -> Patchwork_10105

  CI_DRM_4776: 18e1a6da2ae33504c83d1e5dfc7a08a3940e7b58 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4631: 8884101aa01aedee01b2c3d0ac075473384551b7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10105: e5c2ff993f03a1fc46874810f8c730f222d9e7ca @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10105/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: add LG eDP panel to quirk database

[Lee, Shawn C]

On Thu, 06 Sep 2018, Jani Nikula wrote:
>> The N value was computed by kernel driver that based on synchronous 
>> clock mode. But only specific N value (0x8000) would be acceptable for 
>> LG LP140WF6-SPM1 eDP panel which is running at asynchronous clock mode.
>
>As I wrote before, it's the DP source that determines between synchronous and asynchronous clock mode, not the sink. The sink in question appears to expect a
>constant N related to asynchronous clock mode even when we're using and advertising synchronous clock mode in DP Main Stream Attributes.
>
> (It's possible a certain other OS uses the same constant N regardless of clock mode, leading to the panel working by coincidence.)
>

Yes, this sink device should not expect source will give specific N value. I have no idea what the other OS to calculate M/V value but vendor said they don't have similar issue before.

>> With the other N value, Tcon will enter BITS mode and display black screen.
>> Add this panel into quirk database and give particular N value when 
>> calculate M/N divider.
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
>> Cc: Matt Atwood <matthew.s.atwood@intel.com>
>> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
>> ---
>>  drivers/gpu/drm/drm_dp_helper.c      | 10 +++++++++-
>>  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++------  
>>  drivers/gpu/drm/i915/intel_display.h |  3 ++-
>>  drivers/gpu/drm/i915/intel_dp.c      |  8 ++++++--
>>  drivers/gpu/drm/i915/intel_dp_mst.c  |  2 +-
>>  include/drm/drm_dp_helper.h          |  1 +
>>  6 files changed, 29 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c 
>> b/drivers/gpu/drm/drm_dp_helper.c index 0cccbcb2d03e..a0259bbbe9df 
>> 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -1258,13 +1258,18 @@ struct dpcd_quirk {
>>  	u8 oui[3];
>>  	bool is_branch;
>>  	u32 quirks;
>> +	u8 dev_id[6]; 
>
>Your commit message says nothing about the need to extend the quirk mechanism to use the device ID. Do we really need it?
>
>If we do need it, please name it device_id like it is in struct drm_dp_dpcd_ident, and place it after oui.
>

If we just recognize OUI. And didn't use device_id to identify which sink/branch device need specific M/N value. 
That means all the sink/branch device with Analogix OUI will apply this work around.

00-22-B9   (hex)		Analogix Seminconductor, Inc

>>  };
>>  
>>  #define OUI(first, second, third) { (first), (second), (third) }
>> +#define DEVICE_ID(first, second, third, fourth, fifth, sixth) \
>> +	{ (first), (second), (third), (fourth), (fifth), (sixth) }
>>  
>>  static const struct dpcd_quirk dpcd_quirk_list[] = {
>>  	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
>> -	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
>> +	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N), 
>> +DEVICE_ID(0, 0, 0, 0, 0, 0) },
>
>Maybe add #define DEVICE_ID_ANY to initialize all to zero.
>
>> +	/* LG LP140WF6-SPM1 eDP panel */
>> +	{ OUI(0x00, 0x22, 0xb9), false, BIT(DP_DPCD_QUIRK_CONSTANT_N), 
>> +DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T') },
>>  };
>>  
>>  #undef OUI
>
>#undef DEVICE_ID
>#undef DEVICE_ID_ANY
>
>> @@ -1293,6 +1298,9 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
>>  		if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
>>  			continue;
>>  
>> +		if (!is_branch && memcmp(quirk->dev_id, ident->device_id, 6) != 0)
>> +			continue;
>> +
>
>Why not branch devices? This is really a trick to avoid matching on the Analogix device which you set to zeros, and now you require device id match for all non-branch quirks. Not good. Please check if the >quirk table has all zeros, and compare if not.
>

It will not compare device_id for branch device because of I don't have the device_id for Analogix 7737 branch device you debug before. 
So I set it to zero for default setup.

>>  		quirks |= quirk->quirks;
>>  	}
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index ec3e24f07486..ff01a742b054 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -6465,7 +6465,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
>>  	pipe_config->fdi_lanes = lane;
>>  
>>  	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
>> -			       link_bw, &pipe_config->fdi_m_n, false);
>> +			       link_bw, &pipe_config->fdi_m_n, false, false);
>
>This is getting pretty ugly, to be honest. Which is why I was wondering if we could actually turn the Analogix quirk into a constant N...
>

Seems Anaglogix 7737 can't handle full 24-bit main link Mdiv and Ndiv main link attributes properly. 
Using constant N (0x8000) value should be acceptable. It would not exceed the Analogix 7737 limitation.

>>  
>>  	ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
>>  	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) { @@ -6680,7 
>> +6680,7 @@ intel_reduce_m_n_ratio(uint32_t *num, uint32_t *den)
>>  
>>  static void compute_m_n(unsigned int m, unsigned int n,
>>  			uint32_t *ret_m, uint32_t *ret_n,
>> -			bool reduce_m_n)
>> +			bool reduce_m_n, bool constant_n)
>>  {
>>  	/*
>>  	 * Reduce M/N as much as possible without loss in precision. Several 
>> DP @@ -6695,7 +6695,11 @@ static void compute_m_n(unsigned int m, unsigned int n,
>>  		}
>>  	}
>>  
>> -	*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
>> +	if (constant_n)
>> +		*ret_n = 0x8000;
>> +	else
>> +		*ret_n = min_t(unsigned int, roundup_pow_of_two(n), 
>> +DATA_LINK_N_MAX);
>> +
>>  	*ret_m = div_u64((uint64_t) m * *ret_n, n);
>>  	intel_reduce_m_n_ratio(ret_m, ret_n);  } @@ -6704,18 +6708,18 @@ 
>> void  intel_link_compute_m_n(int bits_per_pixel, int nlanes,
>>  		       int pixel_clock, int link_clock,
>>  		       struct intel_link_m_n *m_n,
>> -		       bool reduce_m_n)
>> +		       bool reduce_m_n, bool constant_n)
>>  {
>>  	m_n->tu = 64;
>>  
>>  	compute_m_n(bits_per_pixel * pixel_clock,
>>  		    link_clock * nlanes * 8,
>>  		    &m_n->gmch_m, &m_n->gmch_n,
>> -		    reduce_m_n);
>> +		    reduce_m_n, constant_n);
>>  
>>  	compute_m_n(pixel_clock, link_clock,
>>  		    &m_n->link_m, &m_n->link_n,
>> -		    reduce_m_n);
>> +		    reduce_m_n, constant_n);
>>  }
>>  
>>  static inline bool intel_panel_use_ssc(struct drm_i915_private 
>> *dev_priv) diff --git a/drivers/gpu/drm/i915/intel_display.h 
>> b/drivers/gpu/drm/i915/intel_display.h
>> index 43f080c6538d..dde7d73bd36d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.h
>> +++ b/drivers/gpu/drm/i915/intel_display.h
>> @@ -379,7 +379,8 @@ struct intel_link_m_n {  void 
>> intel_link_compute_m_n(int bpp, int nlanes,
>>  			    int pixel_clock, int link_clock,
>>  			    struct intel_link_m_n *m_n,
>> -			    bool reduce_m_n);
>> +			    bool reduce_m_n,
>> +			    bool constant_n);
>>  
>>  bool is_ccs_modifier(u64 modifier);
>>  #endif
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> b/drivers/gpu/drm/i915/intel_dp.c index 436c22de33b6..95edbac24919 
>> 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -2000,6 +2000,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>  		to_intel_digital_connector_state(conn_state);
>>  	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
>>  					   DP_DPCD_QUIRK_LIMITED_M_N);
>> +	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
>> +					   DP_DPCD_QUIRK_CONSTANT_N);
>>  
>>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
>>  		pipe_config->has_pch_encoder = true; @@ -2064,7 +2066,8 @@ 
>> intel_dp_compute_config(struct intel_encoder *encoder,
>>  			       adjusted_mode->crtc_clock,
>>  			       pipe_config->port_clock,
>>  			       &pipe_config->dp_m_n,
>> -			       reduce_m_n);
>> +			       reduce_m_n,
>> +			       constant_n);
>>  
>>  	if (intel_connector->panel.downclock_mode != NULL &&
>>  		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) { @@ -2074,7 +2077,8 
>> @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>  					       intel_connector->panel.downclock_mode->clock,
>>  					       pipe_config->port_clock,
>>  					       &pipe_config->dp_m2_n2,
>> -					       reduce_m_n);
>> +					       reduce_m_n,
>> +					       constant_n);
>>  	}
>>  
>>  	if (!HAS_DDI(dev_priv))
>> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
>> b/drivers/gpu/drm/i915/intel_dp_mst.c
>> index 352e5216cc65..ac23982fa335 100644
>> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
>> @@ -87,7 +87,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>>  			       adjusted_mode->crtc_clock,
>>  			       pipe_config->port_clock,
>>  			       &pipe_config->dp_m_n,
>> -			       reduce_m_n);
>> +			       reduce_m_n, false);
>>  
>>  	pipe_config->dp_m_n.tu = slots;
>>  
>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h 
>> index 05cc31b5db16..b1b2fd609c1c 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -1266,6 +1266,7 @@ enum drm_dp_quirk {
>>  	 * to 16 bits.
>>  	 */
>>  	DP_DPCD_QUIRK_LIMITED_M_N,
>> +	DP_DPCD_QUIRK_CONSTANT_N,
>
>Documentation missing. See the comment for limited M/N.
>

I think we can remain DP_DPCD_QUIRK_LIMITED_M_N only and give these 2 device the same N value.

>BR,
>Jani.
>
>>  };
>>  
>>  /**
>
>--
>Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: add LG eDP panel to quirk database

[Dhinakaran Pandiyan]

On Thu, 2018-09-06 at 14:19 +0000, Lee, Shawn C wrote:
> On Thu, 06 Sep 2018, Jani Nikula wrote:
> > > The N value was computed by kernel driver that based on
> > > synchronous 
> > > clock mode. But only specific N value (0x8000) would be
> > > acceptable for 
> > > LG LP140WF6-SPM1 eDP panel which is running at asynchronous clock
> > > mode.
> > 
> > As I wrote before, it's the DP source that determines between
> > synchronous and asynchronous clock mode, not the sink. The sink in
> > question appears to expect a
> > constant N related to asynchronous clock mode even when we're using
> > and advertising synchronous clock mode in DP Main Stream
> > Attributes.
> > 
> > (It's possible a certain other OS uses the same constant N
> > regardless of clock mode, leading to the panel working by
> > coincidence.)
> > 
> 
> Yes, this sink device should not expect source will give specific N
> value. I have no idea what the other OS to calculate M/V value but
> vendor said they don't have similar issue before.
> 
> > > With the other N value, Tcon will enter BITS mode and display
> > > black screen.
> > > Add this panel into quirk database and give particular N value
> > > when 
> > > calculate M/N divider.
> > > 
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > 
> > Cc: Cooper Chiou <cooper.chiou@intel.com>
> > > Cc: Matt Atwood <matthew.s.atwood@intel.com>
> > > Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_dp_helper.c      | 10 +++++++++-
> > >  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++------  
> > >  drivers/gpu/drm/i915/intel_display.h |  3 ++-
> > >  drivers/gpu/drm/i915/intel_dp.c      |  8 ++++++--
> > >  drivers/gpu/drm/i915/intel_dp_mst.c  |  2 +-
> > >  include/drm/drm_dp_helper.h          |  1 +
> > >  6 files changed, 29 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_helper.c 
> > > b/drivers/gpu/drm/drm_dp_helper.c index
> > > 0cccbcb2d03e..a0259bbbe9df 
> > > 100644
> > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > @@ -1258,13 +1258,18 @@ struct dpcd_quirk {
> > >  	u8 oui[3];
> > >  	bool is_branch;
> > >  	u32 quirks;
> > > +	u8 dev_id[6]; 
> > 
> > Your commit message says nothing about the need to extend the quirk
> > mechanism to use the device ID. Do we really need it?
> > 
> > If we do need it, please name it device_id like it is in struct
> > drm_dp_dpcd_ident, and place it after oui.
> > 
> 
> If we just recognize OUI. And didn't use device_id to identify which
> sink/branch device need specific M/N value. 
> That means all the sink/branch device with Analogix OUI will apply
> this work around.
> 
> 00-22-B9   (hex)		Analogix Seminconductor, Inc
> 
> > >  };
> > >  
> > >  #define OUI(first, second, third) { (first), (second), (third) }
> > > +#define DEVICE_ID(first, second, third, fourth, fifth, sixth) \
> > > +	{ (first), (second), (third), (fourth), (fifth), (sixth)
> > > }
> > >  
> > >  static const struct dpcd_quirk dpcd_quirk_list[] = {
> > >  	/* Analogix 7737 needs reduced M and N at HBR2 link
> > > rates */
> > > -	{ OUI(0x00, 0x22, 0xb9), true,
> > > BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
> > > +	{ OUI(0x00, 0x22, 0xb9), true,
> > > BIT(DP_DPCD_QUIRK_LIMITED_M_N), 
> > > +DEVICE_ID(0, 0, 0, 0, 0, 0) },
> > 
> > Maybe add #define DEVICE_ID_ANY to initialize all to zero.
> > 
> > > +	/* LG LP140WF6-SPM1 eDP panel */
> > > +	{ OUI(0x00, 0x22, 0xb9), false,
> > > BIT(DP_DPCD_QUIRK_CONSTANT_N), 
> > > +DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T') },
> > >  };
> > >  
> > >  #undef OUI
> > 
> > #undef DEVICE_ID
> > #undef DEVICE_ID_ANY
> > 
> > > @@ -1293,6 +1298,9 @@ drm_dp_get_quirks(const struct
> > > drm_dp_dpcd_ident *ident, bool is_branch)
> > >  		if (memcmp(quirk->oui, ident->oui, sizeof(ident-
> > > >oui)) != 0)
> > >  			continue;
> > >  
> > > +		if (!is_branch && memcmp(quirk->dev_id, ident-
> > > >device_id, 6) != 0)
> > > +			continue;
> > > +
> > 
> > Why not branch devices? This is really a trick to avoid matching on
> > the Analogix device which you set to zeros, and now you require
> > device id match for all non-branch quirks. Not good. Please check
> > if the >quirk table has all zeros, and compare if not.
> > 
> 
> It will not compare device_id for branch device because of I don't
> have the device_id for Analogix 7737 branch device you debug before. 
> So I set it to zero for default setup.

Cc'ing: Clint

> 
> > >  		quirks |= quirk->quirks;
> > >  	}
> > >  
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index ec3e24f07486..ff01a742b054 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -6465,7 +6465,7 @@ static int
> > > ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> > >  	pipe_config->fdi_lanes = lane;
> > >  
> > >  	intel_link_compute_m_n(pipe_config->pipe_bpp, lane,
> > > fdi_dotclock,
> > > -			       link_bw, &pipe_config->fdi_m_n,
> > > false);
> > > +			       link_bw, &pipe_config->fdi_m_n,
> > > false, false);
> > 
> > This is getting pretty ugly, to be honest. Which is why I was
> > wondering if we could actually turn the Analogix quirk into a
> > constant N...
> > 
> 
> Seems Anaglogix 7737 can't handle full 24-bit main link Mdiv and Ndiv
> main link attributes properly. 
> Using constant N (0x8000) value should be acceptable. It would not
> exceed the Analogix 7737 limitation.
> 
> > >  
> > >  	ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe,
> > > pipe_config);
> > >  	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) { @@
> > > -6680,7 
> > > +6680,7 @@ intel_reduce_m_n_ratio(uint32_t *num, uint32_t *den)
> > >  
> > >  static void compute_m_n(unsigned int m, unsigned int n,
> > >  			uint32_t *ret_m, uint32_t *ret_n,
> > > -			bool reduce_m_n)
> > > +			bool reduce_m_n, bool constant_n)
> > >  {
> > >  	/*
> > >  	 * Reduce M/N as much as possible without loss in
> > > precision. Several 
> > > DP @@ -6695,7 +6695,11 @@ static void compute_m_n(unsigned int m,
> > > unsigned int n,
> > >  		}
> > >  	}
> > >  
> > > -	*ret_n = min_t(unsigned int, roundup_pow_of_two(n),
> > > DATA_LINK_N_MAX);
> > > +	if (constant_n)
> > > +		*ret_n = 0x8000;
> > > +	else
> > > +		*ret_n = min_t(unsigned int,
> > > roundup_pow_of_two(n), 
> > > +DATA_LINK_N_MAX);
> > > +
> > >  	*ret_m = div_u64((uint64_t) m * *ret_n, n);
> > >  	intel_reduce_m_n_ratio(ret_m, ret_n);  } @@ -6704,18
> > > +6708,18 @@ 
> > > void  intel_link_compute_m_n(int bits_per_pixel, int nlanes,
> > >  		       int pixel_clock, int link_clock,
> > >  		       struct intel_link_m_n *m_n,
> > > -		       bool reduce_m_n)
> > > +		       bool reduce_m_n, bool constant_n)
> > >  {
> > >  	m_n->tu = 64;
> > >  
> > >  	compute_m_n(bits_per_pixel * pixel_clock,
> > >  		    link_clock * nlanes * 8,
> > >  		    &m_n->gmch_m, &m_n->gmch_n,
> > > -		    reduce_m_n);
> > > +		    reduce_m_n, constant_n);
> > >  
> > >  	compute_m_n(pixel_clock, link_clock,
> > >  		    &m_n->link_m, &m_n->link_n,
> > > -		    reduce_m_n);
> > > +		    reduce_m_n, constant_n);
> > >  }
> > >  
> > >  static inline bool intel_panel_use_ssc(struct drm_i915_private 
> > > *dev_priv) diff --git a/drivers/gpu/drm/i915/intel_display.h 
> > > b/drivers/gpu/drm/i915/intel_display.h
> > > index 43f080c6538d..dde7d73bd36d 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > @@ -379,7 +379,8 @@ struct intel_link_m_n {  void 
> > > intel_link_compute_m_n(int bpp, int nlanes,
> > >  			    int pixel_clock, int link_clock,
> > >  			    struct intel_link_m_n *m_n,
> > > -			    bool reduce_m_n);
> > > +			    bool reduce_m_n,
> > > +			    bool constant_n);
> > >  
> > >  bool is_ccs_modifier(u64 modifier);
> > >  #endif
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > > b/drivers/gpu/drm/i915/intel_dp.c index
> > > 436c22de33b6..95edbac24919 
> > > 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -2000,6 +2000,8 @@ intel_dp_compute_config(struct
> > > intel_encoder *encoder,
> > >  		to_intel_digital_connector_state(conn_state);
> > >  	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
> > >  					   DP_DPCD_QUIRK_LIMITED
> > > _M_N);
> > > +	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
> > > +					   DP_DPCD_QUIRK_CONSTAN
> > > T_N);
> > >  
> > >  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) &&
> > > port != PORT_A)
> > >  		pipe_config->has_pch_encoder = true; @@ -2064,7
> > > +2066,8 @@ 
> > > intel_dp_compute_config(struct intel_encoder *encoder,
> > >  			       adjusted_mode->crtc_clock,
> > >  			       pipe_config->port_clock,
> > >  			       &pipe_config->dp_m_n,
> > > -			       reduce_m_n);
> > > +			       reduce_m_n,
> > > +			       constant_n);
> > >  
> > >  	if (intel_connector->panel.downclock_mode != NULL &&
> > >  		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> > > @@ -2074,7 +2077,8 
> > > @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > >  					       intel_connector-
> > > >panel.downclock_mode->clock,
> > >  					       pipe_config-
> > > >port_clock,
> > >  					       &pipe_config-
> > > >dp_m2_n2,
> > > -					       reduce_m_n);
> > > +					       reduce_m_n,
> > > +					       constant_n);
> > >  	}
> > >  
> > >  	if (!HAS_DDI(dev_priv))
> > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
> > > b/drivers/gpu/drm/i915/intel_dp_mst.c
> > > index 352e5216cc65..ac23982fa335 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > > @@ -87,7 +87,7 @@ static bool intel_dp_mst_compute_config(struct
> > > intel_encoder *encoder,
> > >  			       adjusted_mode->crtc_clock,
> > >  			       pipe_config->port_clock,
> > >  			       &pipe_config->dp_m_n,
> > > -			       reduce_m_n);
> > > +			       reduce_m_n, false);
> > >  
> > >  	pipe_config->dp_m_n.tu = slots;
> > >  
> > > diff --git a/include/drm/drm_dp_helper.h
> > > b/include/drm/drm_dp_helper.h 
> > > index 05cc31b5db16..b1b2fd609c1c 100644
> > > --- a/include/drm/drm_dp_helper.h
> > > +++ b/include/drm/drm_dp_helper.h
> > > @@ -1266,6 +1266,7 @@ enum drm_dp_quirk {
> > >  	 * to 16 bits.
> > >  	 */
> > >  	DP_DPCD_QUIRK_LIMITED_M_N,
> > > +	DP_DPCD_QUIRK_CONSTANT_N,
> > 
> > Documentation missing. See the comment for limited M/N.
> > 
> 
> I think we can remain DP_DPCD_QUIRK_LIMITED_M_N only and give these 2
> device the same N value.
> 
> > BR,
> > Jani.
> > 
> > >  };
> > >  
> > >  /**
> > 
> > --
> > Jani Nikula, Intel Open Source Graphics Center
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for drm/i915: add LG eDP panel to quirk database (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: add LG eDP panel to quirk database (rev2)
URL   : https://patchwork.freedesktop.org/series/49251/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
d5816afc0b2c drm: add LG eDP panel to quirk database
-:47: WARNING:LONG_LINE: line over 100 characters
#47: FILE: drivers/gpu/drm/drm_dp_helper.c:1275:
+	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },

-:203: WARNING:TYPO_SPELLING: 'compatability' may be misspelled - perhaps 'compatibility'?
#203: FILE: include/drm/drm_dp_helper.h:1267:
+	 * to 16 bits.So will give a constant value (0x8000) for compatability.

total: 0 errors, 2 warnings, 0 checks, 152 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for drm/i915: add LG eDP panel to quirk database (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: add LG eDP panel to quirk database (rev2)
URL   : https://patchwork.freedesktop.org/series/49251/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm: add LG eDP panel to quirk database
-O:drivers/gpu/drm/i915/intel_display.c:6702:18: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_display.c:6692:26: warning: expression using sizeof(void)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm: add LG eDP panel to quirk database

[Lee, Shawn C]

The N value was computed by kernel driver that based on synchronous clock
mode. But only specific N value (0x8000) would be acceptable for
LG LP140WF6-SPM1 eDP panel which is running at asynchronous clock mode.
With the other N value, Tcon will enter BITS mode and display black screen.
Add this panel into quirk database and give particular N value when
calculate M/N divider.

v2:
- Use CONSTANT_N instead of LIMITED_M_N and set N as 0x8000 for particular
  branch/sink device.
- Fix typo.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c      | 14 +++++++++++++-
 drivers/gpu/drm/i915/intel_display.c | 25 ++++++++-----------------
 drivers/gpu/drm/i915/intel_display.h |  2 +-
 drivers/gpu/drm/i915/intel_dp.c      |  8 ++++----
 drivers/gpu/drm/i915/intel_dp_mst.c  |  6 +++---
 include/drm/drm_dp_helper.h          |  6 +++---
 6 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 0cccbcb2d03e..eb9d97456db1 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1256,18 +1256,27 @@ EXPORT_SYMBOL(drm_dp_stop_crc);
 
 struct dpcd_quirk {
 	u8 oui[3];
+	u8 device_id[6];
 	bool is_branch;
 	u32 quirks;
 };
 
 #define OUI(first, second, third) { (first), (second), (third) }
+#define DEVICE_ID(first, second, third, fourth, fifth, sixth) \
+	{ (first), (second), (third), (fourth), (fifth), (sixth) }
+
+#define DEVICE_ID_ANY	DEVICE_ID(0, 0, 0, 0, 0, 0)
 
 static const struct dpcd_quirk dpcd_quirk_list[] = {
 	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
-	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
+	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
+	/* LG LP140WF6-SPM1 eDP panel */
+	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
 };
 
 #undef OUI
+#undef DEVICE_ID
+#undef DEVICE_ID_ANY
 
 /*
  * Get a bit mask of DPCD quirks for the sink/branch device identified by
@@ -1293,6 +1302,9 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
 		if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
 			continue;
 
+		if (!is_branch && memcmp(quirk->device_id, ident->device_id, 6) != 0)
+			continue;
+
 		quirks |= quirk->quirks;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ec3e24f07486..268b1f546b77 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6680,22 +6680,13 @@ intel_reduce_m_n_ratio(uint32_t *num, uint32_t *den)
 
 static void compute_m_n(unsigned int m, unsigned int n,
 			uint32_t *ret_m, uint32_t *ret_n,
-			bool reduce_m_n)
+			bool constant_n)
 {
-	/*
-	 * Reduce M/N as much as possible without loss in precision. Several DP
-	 * dongles in particular seem to be fussy about too large *link* M/N
-	 * values. The passed in values are more likely to have the least
-	 * significant bits zero than M after rounding below, so do this first.
-	 */
-	if (reduce_m_n) {
-		while ((m & 1) == 0 && (n & 1) == 0) {
-			m >>= 1;
-			n >>= 1;
-		}
-	}
+	if (constant_n)
+		*ret_n = 0x8000;
+	else
+		*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
 
-	*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
 	*ret_m = div_u64((uint64_t) m * *ret_n, n);
 	intel_reduce_m_n_ratio(ret_m, ret_n);
 }
@@ -6704,18 +6695,18 @@ void
 intel_link_compute_m_n(int bits_per_pixel, int nlanes,
 		       int pixel_clock, int link_clock,
 		       struct intel_link_m_n *m_n,
-		       bool reduce_m_n)
+		       bool constant_n)
 {
 	m_n->tu = 64;
 
 	compute_m_n(bits_per_pixel * pixel_clock,
 		    link_clock * nlanes * 8,
 		    &m_n->gmch_m, &m_n->gmch_n,
-		    reduce_m_n);
+		    constant_n);
 
 	compute_m_n(pixel_clock, link_clock,
 		    &m_n->link_m, &m_n->link_n,
-		    reduce_m_n);
+		    constant_n);
 }
 
 static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index 43f080c6538d..8e8bd5eed2c2 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -379,7 +379,7 @@ struct intel_link_m_n {
 void intel_link_compute_m_n(int bpp, int nlanes,
 			    int pixel_clock, int link_clock,
 			    struct intel_link_m_n *m_n,
-			    bool reduce_m_n);
+			    bool constant_n);
 
 bool is_ccs_modifier(u64 modifier);
 #endif
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 436c22de33b6..6b4c19123f2a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1998,8 +1998,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
 	struct intel_digital_connector_state *intel_conn_state =
 		to_intel_digital_connector_state(conn_state);
-	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
-					   DP_DPCD_QUIRK_LIMITED_M_N);
+	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
+					   DP_DPCD_QUIRK_CONSTANT_N);
 
 	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
 		pipe_config->has_pch_encoder = true;
@@ -2064,7 +2064,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 			       adjusted_mode->crtc_clock,
 			       pipe_config->port_clock,
 			       &pipe_config->dp_m_n,
-			       reduce_m_n);
+			       constant_n);
 
 	if (intel_connector->panel.downclock_mode != NULL &&
 		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
@@ -2074,7 +2074,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 					       intel_connector->panel.downclock_mode->clock,
 					       pipe_config->port_clock,
 					       &pipe_config->dp_m2_n2,
-					       reduce_m_n);
+					       constant_n);
 	}
 
 	if (!HAS_DDI(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 352e5216cc65..184023435b08 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -45,8 +45,8 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	int lane_count, slots;
 	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	int mst_pbn;
-	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
-					   DP_DPCD_QUIRK_LIMITED_M_N);
+	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
+					   DP_DPCD_QUIRK_CONSTANT_N);
 
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return false;
@@ -87,7 +87,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 			       adjusted_mode->crtc_clock,
 			       pipe_config->port_clock,
 			       &pipe_config->dp_m_n,
-			       reduce_m_n);
+			       constant_n);
 
 	pipe_config->dp_m_n.tu = slots;
 
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 05cc31b5db16..ab0914ef5e38 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1260,12 +1260,12 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
  */
 enum drm_dp_quirk {
 	/**
-	 * @DP_DPCD_QUIRK_LIMITED_M_N:
+	 * @DP_DPCD_QUIRK_CONSTANT_N:
 	 *
 	 * The device requires main link attributes Mvid and Nvid to be limited
-	 * to 16 bits.
+	 * to 16 bits.So will give a constant value (0x8000) for compatability.
 	 */
-	DP_DPCD_QUIRK_LIMITED_M_N,
+	DP_DPCD_QUIRK_CONSTANT_N,
 };
 
 /**
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: add LG eDP panel to quirk database (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: add LG eDP panel to quirk database (rev2)
URL   : https://patchwork.freedesktop.org/series/49251/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4785 -> Patchwork_10118 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49251/revisions/2/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10118 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-cnl-psr:         PASS -> DMESG-WARN (fdo#104951)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#107139, fdo#105128) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362


== Participating hosts (53 -> 49) ==

  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4785 -> Patchwork_10118

  CI_DRM_4785: 7fddb79b0908d5c08efc93656fd10a761c9d14ca @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4634: 7d89cc39dde3b4881d85ace45d504cc098fa3684 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10118: d5816afc0b2c2379bb638d5dee916fe1adb92448 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d5816afc0b2c drm: add LG eDP panel to quirk database

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10118/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: add LG eDP panel to quirk database (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: add LG eDP panel to quirk database (rev2)
URL   : https://patchwork.freedesktop.org/series/49251/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4785_full -> Patchwork_10118_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

  Here are the changes found in Patchwork_10118_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_workarounds@suspend-resume-context:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    
    ==== Possible fixes ====

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363, fdo#102887) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS
      shard-kbl:          FAIL (fdo#99912) -> PASS

    igt@perf@polling:
      shard-hsw:          FAIL (fdo#102252) -> PASS

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4785 -> Patchwork_10118

  CI_DRM_4785: 7fddb79b0908d5c08efc93656fd10a761c9d14ca @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4634: 7d89cc39dde3b4881d85ace45d504cc098fa3684 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10118: d5816afc0b2c2379bb638d5dee916fe1adb92448 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10118/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm: add LG eDP panel to quirk database

[Jani Nikula]

On Thu, 06 Sep 2018, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> The N value was computed by kernel driver that based on synchronous clock
> mode. But only specific N value (0x8000) would be acceptable for
> LG LP140WF6-SPM1 eDP panel which is running at asynchronous clock mode.
> With the other N value, Tcon will enter BITS mode and display black screen.
> Add this panel into quirk database and give particular N value when
> calculate M/N divider.
>
> v2:
> - Use CONSTANT_N instead of LIMITED_M_N and set N as 0x8000 for particular
>   branch/sink device.
> - Fix typo.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c      | 14 +++++++++++++-
>  drivers/gpu/drm/i915/intel_display.c | 25 ++++++++-----------------
>  drivers/gpu/drm/i915/intel_display.h |  2 +-
>  drivers/gpu/drm/i915/intel_dp.c      |  8 ++++----
>  drivers/gpu/drm/i915/intel_dp_mst.c  |  6 +++---
>  include/drm/drm_dp_helper.h          |  6 +++---
>  6 files changed, 32 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 0cccbcb2d03e..eb9d97456db1 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1256,18 +1256,27 @@ EXPORT_SYMBOL(drm_dp_stop_crc);
>  
>  struct dpcd_quirk {
>  	u8 oui[3];
> +	u8 device_id[6];
>  	bool is_branch;
>  	u32 quirks;
>  };
>  
>  #define OUI(first, second, third) { (first), (second), (third) }
> +#define DEVICE_ID(first, second, third, fourth, fifth, sixth) \
> +	{ (first), (second), (third), (fourth), (fifth), (sixth) }
> +
> +#define DEVICE_ID_ANY	DEVICE_ID(0, 0, 0, 0, 0, 0)
>  
>  static const struct dpcd_quirk dpcd_quirk_list[] = {
>  	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
> -	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
> +	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
> +	/* LG LP140WF6-SPM1 eDP panel */
> +	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
>  };
>  
>  #undef OUI
> +#undef DEVICE_ID
> +#undef DEVICE_ID_ANY
>  
>  /*
>   * Get a bit mask of DPCD quirks for the sink/branch device identified by
> @@ -1293,6 +1302,9 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
>  		if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
>  			continue;
>  
> +		if (!is_branch && memcmp(quirk->device_id, ident->device_id, 6) != 0)
> +			continue;
> +

Again, this makes it impossible to add device_id specific quirks for
branch devices later. Instead of !is_branch, check for quirk->device_id
is nonzero.

Other than that, I think the changes looks good. However, I'm starting
to think this needs to be split to three patches:

1/3: Add support for device_id based detection.

2/3: Change limited M/N quirk to constant N quirk.

3/3: Add LG quirk to the table.

The changes touch drm helpers so please Cc: dri-devel too.

BR,
Jani.


>  		quirks |= quirk->quirks;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ec3e24f07486..268b1f546b77 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6680,22 +6680,13 @@ intel_reduce_m_n_ratio(uint32_t *num, uint32_t *den)
>  
>  static void compute_m_n(unsigned int m, unsigned int n,
>  			uint32_t *ret_m, uint32_t *ret_n,
> -			bool reduce_m_n)
> +			bool constant_n)
>  {
> -	/*
> -	 * Reduce M/N as much as possible without loss in precision. Several DP
> -	 * dongles in particular seem to be fussy about too large *link* M/N
> -	 * values. The passed in values are more likely to have the least
> -	 * significant bits zero than M after rounding below, so do this first.
> -	 */
> -	if (reduce_m_n) {
> -		while ((m & 1) == 0 && (n & 1) == 0) {
> -			m >>= 1;
> -			n >>= 1;
> -		}
> -	}
> +	if (constant_n)
> +		*ret_n = 0x8000;
> +	else
> +		*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
>  
> -	*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
>  	*ret_m = div_u64((uint64_t) m * *ret_n, n);
>  	intel_reduce_m_n_ratio(ret_m, ret_n);
>  }
> @@ -6704,18 +6695,18 @@ void
>  intel_link_compute_m_n(int bits_per_pixel, int nlanes,
>  		       int pixel_clock, int link_clock,
>  		       struct intel_link_m_n *m_n,
> -		       bool reduce_m_n)
> +		       bool constant_n)
>  {
>  	m_n->tu = 64;
>  
>  	compute_m_n(bits_per_pixel * pixel_clock,
>  		    link_clock * nlanes * 8,
>  		    &m_n->gmch_m, &m_n->gmch_n,
> -		    reduce_m_n);
> +		    constant_n);
>  
>  	compute_m_n(pixel_clock, link_clock,
>  		    &m_n->link_m, &m_n->link_n,
> -		    reduce_m_n);
> +		    constant_n);
>  }
>  
>  static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> index 43f080c6538d..8e8bd5eed2c2 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -379,7 +379,7 @@ struct intel_link_m_n {
>  void intel_link_compute_m_n(int bpp, int nlanes,
>  			    int pixel_clock, int link_clock,
>  			    struct intel_link_m_n *m_n,
> -			    bool reduce_m_n);
> +			    bool constant_n);
>  
>  bool is_ccs_modifier(u64 modifier);
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 436c22de33b6..6b4c19123f2a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1998,8 +1998,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	struct intel_connector *intel_connector = intel_dp->attached_connector;
>  	struct intel_digital_connector_state *intel_conn_state =
>  		to_intel_digital_connector_state(conn_state);
> -	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
> -					   DP_DPCD_QUIRK_LIMITED_M_N);
> +	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
> +					   DP_DPCD_QUIRK_CONSTANT_N);
>  
>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
>  		pipe_config->has_pch_encoder = true;
> @@ -2064,7 +2064,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  			       adjusted_mode->crtc_clock,
>  			       pipe_config->port_clock,
>  			       &pipe_config->dp_m_n,
> -			       reduce_m_n);
> +			       constant_n);
>  
>  	if (intel_connector->panel.downclock_mode != NULL &&
>  		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> @@ -2074,7 +2074,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  					       intel_connector->panel.downclock_mode->clock,
>  					       pipe_config->port_clock,
>  					       &pipe_config->dp_m2_n2,
> -					       reduce_m_n);
> +					       constant_n);
>  	}
>  
>  	if (!HAS_DDI(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 352e5216cc65..184023435b08 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -45,8 +45,8 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	int lane_count, slots;
>  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>  	int mst_pbn;
> -	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
> -					   DP_DPCD_QUIRK_LIMITED_M_N);
> +	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
> +					   DP_DPCD_QUIRK_CONSTANT_N);
>  
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return false;
> @@ -87,7 +87,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  			       adjusted_mode->crtc_clock,
>  			       pipe_config->port_clock,
>  			       &pipe_config->dp_m_n,
> -			       reduce_m_n);
> +			       constant_n);
>  
>  	pipe_config->dp_m_n.tu = slots;
>  
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 05cc31b5db16..ab0914ef5e38 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1260,12 +1260,12 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
>   */
>  enum drm_dp_quirk {
>  	/**
> -	 * @DP_DPCD_QUIRK_LIMITED_M_N:
> +	 * @DP_DPCD_QUIRK_CONSTANT_N:
>  	 *
>  	 * The device requires main link attributes Mvid and Nvid to be limited
> -	 * to 16 bits.
> +	 * to 16 bits.So will give a constant value (0x8000) for compatability.
>  	 */
> -	DP_DPCD_QUIRK_LIMITED_M_N,
> +	DP_DPCD_QUIRK_CONSTANT_N,
>  };
>  
>  /**

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm: add LG eDP panel to quirk database

[Lee, Shawn C]

Thanks for review! I will split this patch to 3 and commit again.

On Thu, 06 Sep 2018, Jani Nikula wrote:
>> The N value was computed by kernel driver that based on synchronous 
>> clock mode. But only specific N value (0x8000) would be acceptable for 
>> LG LP140WF6-SPM1 eDP panel which is running at asynchronous clock mode.
>> With the other N value, Tcon will enter BITS mode and display black screen.
>> Add this panel into quirk database and give particular N value when 
>> calculate M/N divider.
>>
>> v2:
>> - Use CONSTANT_N instead of LIMITED_M_N and set N as 0x8000 for particular
>>   branch/sink device.
>> - Fix typo.
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Cooper Chiou <cooper.chiou@intel.com>
>> Cc: Matt Atwood <matthew.s.atwood@intel.com>
>> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
>> ---
>>  drivers/gpu/drm/drm_dp_helper.c      | 14 +++++++++++++-
>>  drivers/gpu/drm/i915/intel_display.c | 25 ++++++++-----------------  
>> drivers/gpu/drm/i915/intel_display.h |  2 +-
>>  drivers/gpu/drm/i915/intel_dp.c      |  8 ++++----
>>  drivers/gpu/drm/i915/intel_dp_mst.c  |  6 +++---
>>  include/drm/drm_dp_helper.h          |  6 +++---
>>  6 files changed, 32 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c 
>> b/drivers/gpu/drm/drm_dp_helper.c index 0cccbcb2d03e..eb9d97456db1 
>> 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -1256,18 +1256,27 @@ EXPORT_SYMBOL(drm_dp_stop_crc);
>>  
>>  struct dpcd_quirk {
>>  	u8 oui[3];
>> +	u8 device_id[6];
>>  	bool is_branch;
>>  	u32 quirks;
>>  };
>>  
>>  #define OUI(first, second, third) { (first), (second), (third) }
>> +#define DEVICE_ID(first, second, third, fourth, fifth, sixth) \
>> +	{ (first), (second), (third), (fourth), (fifth), (sixth) }
>> +
>> +#define DEVICE_ID_ANY	DEVICE_ID(0, 0, 0, 0, 0, 0)
>>  
>>  static const struct dpcd_quirk dpcd_quirk_list[] = {
>>  	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
>> -	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
>> +	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
>> +	/* LG LP140WF6-SPM1 eDP panel */
>> +	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'), 
>> +false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
>>  };
>>  
>>  #undef OUI
>> +#undef DEVICE_ID
>> +#undef DEVICE_ID_ANY
>>  
>>  /*
>>   * Get a bit mask of DPCD quirks for the sink/branch device 
>> identified by @@ -1293,6 +1302,9 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
>>  		if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
>>  			continue;
>>  
>> +		if (!is_branch && memcmp(quirk->device_id, ident->device_id, 6) != 0)
>> +			continue;
>> +
>
>Again, this makes it impossible to add device_id specific quirks for branch devices later. Instead of !is_branch, check for quirk->device_id is nonzero.
>
>Other than that, I think the changes looks good. However, I'm starting to think this needs to be split to three patches:
>
>1/3: Add support for device_id based detection.
>
>2/3: Change limited M/N quirk to constant N quirk.
>
>3/3: Add LG quirk to the table.
>
>The changes touch drm helpers so please Cc: dri-devel too.
>
>BR,
>Jani.
>
>
>>  		quirks |= quirk->quirks;
>>  	}
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index ec3e24f07486..268b1f546b77 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -6680,22 +6680,13 @@ intel_reduce_m_n_ratio(uint32_t *num, uint32_t 
>> *den)
>>  
>>  static void compute_m_n(unsigned int m, unsigned int n,
>>  			uint32_t *ret_m, uint32_t *ret_n,
>> -			bool reduce_m_n)
>> +			bool constant_n)
>>  {
>> -	/*
>> -	 * Reduce M/N as much as possible without loss in precision. Several DP
>> -	 * dongles in particular seem to be fussy about too large *link* M/N
>> -	 * values. The passed in values are more likely to have the least
>> -	 * significant bits zero than M after rounding below, so do this first.
>> -	 */
>> -	if (reduce_m_n) {
>> -		while ((m & 1) == 0 && (n & 1) == 0) {
>> -			m >>= 1;
>> -			n >>= 1;
>> -		}
>> -	}
>> +	if (constant_n)
>> +		*ret_n = 0x8000;
>> +	else
>> +		*ret_n = min_t(unsigned int, roundup_pow_of_two(n), 
>> +DATA_LINK_N_MAX);
>>  
>> -	*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
>>  	*ret_m = div_u64((uint64_t) m * *ret_n, n);
>>  	intel_reduce_m_n_ratio(ret_m, ret_n);  } @@ -6704,18 +6695,18 @@ 
>> void  intel_link_compute_m_n(int bits_per_pixel, int nlanes,
>>  		       int pixel_clock, int link_clock,
>>  		       struct intel_link_m_n *m_n,
>> -		       bool reduce_m_n)
>> +		       bool constant_n)
>>  {
>>  	m_n->tu = 64;
>>  
>>  	compute_m_n(bits_per_pixel * pixel_clock,
>>  		    link_clock * nlanes * 8,
>>  		    &m_n->gmch_m, &m_n->gmch_n,
>> -		    reduce_m_n);
>> +		    constant_n);
>>  
>>  	compute_m_n(pixel_clock, link_clock,
>>  		    &m_n->link_m, &m_n->link_n,
>> -		    reduce_m_n);
>> +		    constant_n);
>>  }
>>  
>>  static inline bool intel_panel_use_ssc(struct drm_i915_private 
>> *dev_priv) diff --git a/drivers/gpu/drm/i915/intel_display.h 
>> b/drivers/gpu/drm/i915/intel_display.h
>> index 43f080c6538d..8e8bd5eed2c2 100644
>> --- a/drivers/gpu/drm/i915/intel_display.h
>> +++ b/drivers/gpu/drm/i915/intel_display.h
>> @@ -379,7 +379,7 @@ struct intel_link_m_n {  void 
>> intel_link_compute_m_n(int bpp, int nlanes,
>>  			    int pixel_clock, int link_clock,
>>  			    struct intel_link_m_n *m_n,
>> -			    bool reduce_m_n);
>> +			    bool constant_n);
>>  
>>  bool is_ccs_modifier(u64 modifier);
>>  #endif
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> b/drivers/gpu/drm/i915/intel_dp.c index 436c22de33b6..6b4c19123f2a 
>> 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1998,8 +1998,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>  	struct intel_connector *intel_connector = intel_dp->attached_connector;
>>  	struct intel_digital_connector_state *intel_conn_state =
>>  		to_intel_digital_connector_state(conn_state);
>> -	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
>> -					   DP_DPCD_QUIRK_LIMITED_M_N);
>> +	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
>> +					   DP_DPCD_QUIRK_CONSTANT_N);
>>  
>>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
>>  		pipe_config->has_pch_encoder = true; @@ -2064,7 +2064,7 @@ 
>> intel_dp_compute_config(struct intel_encoder *encoder,
>>  			       adjusted_mode->crtc_clock,
>>  			       pipe_config->port_clock,
>>  			       &pipe_config->dp_m_n,
>> -			       reduce_m_n);
>> +			       constant_n);
>>  
>>  	if (intel_connector->panel.downclock_mode != NULL &&
>>  		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) { @@ -2074,7 +2074,7 
>> @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>  					       intel_connector->panel.downclock_mode->clock,
>>  					       pipe_config->port_clock,
>>  					       &pipe_config->dp_m2_n2,
>> -					       reduce_m_n);
>> +					       constant_n);
>>  	}
>>  
>>  	if (!HAS_DDI(dev_priv))
>> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
>> b/drivers/gpu/drm/i915/intel_dp_mst.c
>> index 352e5216cc65..184023435b08 100644
>> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
>> @@ -45,8 +45,8 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>>  	int lane_count, slots;
>>  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>>  	int mst_pbn;
>> -	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
>> -					   DP_DPCD_QUIRK_LIMITED_M_N);
>> +	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
>> +					   DP_DPCD_QUIRK_CONSTANT_N);
>>  
>>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>>  		return false;
>> @@ -87,7 +87,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>>  			       adjusted_mode->crtc_clock,
>>  			       pipe_config->port_clock,
>>  			       &pipe_config->dp_m_n,
>> -			       reduce_m_n);
>> +			       constant_n);
>>  
>>  	pipe_config->dp_m_n.tu = slots;
>>  
>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h 
>> index 05cc31b5db16..ab0914ef5e38 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -1260,12 +1260,12 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
>>   */
>>  enum drm_dp_quirk {
>>  	/**
>> -	 * @DP_DPCD_QUIRK_LIMITED_M_N:
>> +	 * @DP_DPCD_QUIRK_CONSTANT_N:
>>  	 *
>>  	 * The device requires main link attributes Mvid and Nvid to be limited
>> -	 * to 16 bits.
>> +	 * to 16 bits.So will give a constant value (0x8000) for compatability.
>>  	 */
>> -	DP_DPCD_QUIRK_LIMITED_M_N,
>> +	DP_DPCD_QUIRK_CONSTANT_N,
>>  };
>>  
>>  /**
>
>--
>Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx