[PATCH 0/3] add LG panel to dpcd quirk database

[PATCH 0/3] add LG panel to dpcd quirk database

[Lee, Shawn C]

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, it will enter BITS mode
and display black screen. This patch series set constant N
value for specific sink/branch device that would cover
similar issue.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>

Lee, Shawn C (3):
  drm: Add support for device_id based detection.
  drm: Change limited M/N quirk to constant N quirk.
  drm: add LG eDP panel to quirk database

 drivers/gpu/drm/drm_dp_helper.c      | 17 ++++++++++++++++-
 drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++---------------
 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, 38 insertions(+), 27 deletions(-)

-- 
2.7.4

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

[PATCH 1/3] drm: Add support for device_id based detection.

[Lee, Shawn C]

DP quirk list just compare sink or branch device's OUI so far.
That means particular vendor's products will be applied specific
change. This change would confirm device_id the same or not.
Then driver can implement some changes for branch/sink device
that really need additional WA.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 0cccbcb2d03e..22753928af41 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1256,15 +1256,20 @@ 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_LIMITED_M_N) },
 };
 
 #undef OUI
@@ -1283,6 +1288,7 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
 	const struct dpcd_quirk *quirk;
 	u32 quirks = 0;
 	int i;
+	u8 any_device[6] = DEVICE_ID_ANY;
 
 	for (i = 0; i < ARRAY_SIZE(dpcd_quirk_list); i++) {
 		quirk = &dpcd_quirk_list[i];
@@ -1293,12 +1299,19 @@ 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 (memcmp(quirk->device_id, any_device, 6) != 0 &&
+		    memcmp(quirk->device_id, ident->device_id, 6) != 0)
+			continue;
+
 		quirks |= quirk->quirks;
 	}
 
 	return quirks;
 }
 
+#undef DEVICE_ID_ANY
+#undef DEVICE_ID
+
 /**
  * drm_dp_read_desc - read sink/branch descriptor from DPCD
  * @aux: DisplayPort AUX channel
-- 
2.7.4

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

[PATCH 2/3] drm: Change limited M/N quirk to constant N quirk.

[Lee, Shawn C]

Some DP dongles in particular seem to be fussy about too large
link M/N values. Set specific value for N divider can resolve
this issue per dongle vendor's comment. So configure N as
constant value (0x8000) to instead of reduce M/N formula when
specific DP dongle connected.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c      |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++---------------
 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, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 22753928af41..d0c1250975ab 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1269,7 +1269,7 @@ struct dpcd_quirk {
 
 static const struct dpcd_quirk dpcd_quirk_list[] = {
 	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
-	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
+	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
 };
 
 #undef OUI
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ec3e24f07486..b26f4ae60810 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6680,22 +6680,18 @@ 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.
+	 * Several DP dongles in particular seem to be fussy about
+	 * too large *link* M/N * values. Give N value as 0x8000
+	 * that should be acceptable by specific devices.
 	 */
-	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 +6700,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..fce4be57ccc9 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);
+					       costant_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 compatibility.
 	 */
-	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

[PATCH 3/3] 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.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index d0c1250975ab..0ef7c43a9025 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1270,6 +1270,8 @@ struct dpcd_quirk {
 static const struct dpcd_quirk dpcd_quirk_list[] = {
 	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
 	{ 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
-- 
2.7.4

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

✗ Fi.CI.BAT: failure for add LG panel to dpcd quirk database

[Patchwork]

== Series Details ==

Series: add LG panel to dpcd quirk database
URL   : https://patchwork.freedesktop.org/series/49413/
State : failure

== Summary ==

CALL    scripts/checksyscalls.sh
  DESCEND  objtool
  CHK     include/generated/compile.h
  CC [M]  drivers/gpu/drm/i915/intel_dp.o
drivers/gpu/drm/i915/intel_dp.c: In function ‘intel_dp_compute_config’:
drivers/gpu/drm/i915/intel_dp.c:2077:13: error: ‘costant_n’ undeclared (first use in this function); did you mean ‘constant_n’?
             costant_n);
             ^~~~~~~~~
             constant_n
drivers/gpu/drm/i915/intel_dp.c:2077:13: note: each undeclared identifier is reported only once for each function it appears in
scripts/Makefile.build:305: recipe for target 'drivers/gpu/drm/i915/intel_dp.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_dp.o] Error 1
scripts/Makefile.build:546: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:546: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:546: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1060: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

Re: [PATCH 1/3] drm: Add support for device_id based detection.

[Jani Nikula]

On Mon, 10 Sep 2018, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> DP quirk list just compare sink or branch device's OUI so far.
> That means particular vendor's products will be applied specific
> change. This change would confirm device_id the same or not.
> Then driver can implement some changes for branch/sink device
> that really need additional WA.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 0cccbcb2d03e..22753928af41 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1256,15 +1256,20 @@ 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_LIMITED_M_N) },
>  };
>  
>  #undef OUI
> @@ -1283,6 +1288,7 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
>  	const struct dpcd_quirk *quirk;
>  	u32 quirks = 0;
>  	int i;
> +	u8 any_device[6] = DEVICE_ID_ANY;

Please make that any_device[] without the size.

>  
>  	for (i = 0; i < ARRAY_SIZE(dpcd_quirk_list); i++) {
>  		quirk = &dpcd_quirk_list[i];
> @@ -1293,12 +1299,19 @@ 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 (memcmp(quirk->device_id, any_device, 6) != 0 &&
> +		    memcmp(quirk->device_id, ident->device_id, 6) != 0)

Please use sizeof instead of hard coded 6.

With those changes,

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



> +			continue;
> +
>  		quirks |= quirk->quirks;
>  	}
>  
>  	return quirks;
>  }
>  
> +#undef DEVICE_ID_ANY
> +#undef DEVICE_ID
> +
>  /**
>   * drm_dp_read_desc - read sink/branch descriptor from DPCD
>   * @aux: DisplayPort AUX channel

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

Re: [PATCH 2/3] drm: Change limited M/N quirk to constant N quirk.

[Jani Nikula]

On Mon, 10 Sep 2018, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> Some DP dongles in particular seem to be fussy about too large
> link M/N values. Set specific value for N divider can resolve
> this issue per dongle vendor's comment. So configure N as
> constant value (0x8000) to instead of reduce M/N formula when
> specific DP dongle connected.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c      |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++---------------
>  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, 23 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 22753928af41..d0c1250975ab 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1269,7 +1269,7 @@ struct dpcd_quirk {
>  
>  static const struct dpcd_quirk dpcd_quirk_list[] = {
>  	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
> -	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
> +	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
>  };
>  
>  #undef OUI
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ec3e24f07486..b26f4ae60810 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6680,22 +6680,18 @@ 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.
> +	 * Several DP dongles in particular seem to be fussy about
> +	 * too large *link* M/N * values. Give N value as 0x8000
                                ^
Extra *.

> +	 * that should be acceptable by specific devices.

Please also add something like:

	0x8000 is the specified fixed N value for asynchronous clock
	mode, which the devices expect also in synchronous clock mode.


>  	 */
> -	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 +6700,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..fce4be57ccc9 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);
> +					       costant_n);

Typo costant_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 compatibility.
                      ^

Missing space.

With the minor issues fixed,

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



>  	 */
> -	DP_DPCD_QUIRK_LIMITED_M_N,
> +	DP_DPCD_QUIRK_CONSTANT_N,
>  };
>  
>  /**

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

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

[Jani Nikula]

On Mon, 10 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>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>

No access to the panel or its details, so instead of review,

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

> ---
>  drivers/gpu/drm/drm_dp_helper.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index d0c1250975ab..0ef7c43a9025 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1270,6 +1270,8 @@ struct dpcd_quirk {
>  static const struct dpcd_quirk dpcd_quirk_list[] = {
>  	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
>  	{ 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

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

[PATCH v2 0/3] add LG panel to dpcd quirk database

[Lee, Shawn C]

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, it will enter BITS mode
and display black screen. This patch series set constant N
value for specific sink/branch device that would cover
similar issue.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>

Lee, Shawn C (3):
  drm: Add support for device_id based detection.
  drm: Change limited M/N quirk to constant N quirk.
  drm: add LG eDP panel to quirk database

 drivers/gpu/drm/drm_dp_helper.c      | 17 ++++++++++++++++-
 drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++---------------
 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, 40 insertions(+), 27 deletions(-)

-- 
2.7.4

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

[PATCH v2 1/3] drm: Add support for device_id based detection.

[Lee, Shawn C]

DP quirk list just compare sink or branch device's OUI so far.
That means particular vendor's products will be applied specific
change. This change would confirm device_id the same or not.
Then driver can implement some changes for branch/sink device
that really need additional WA.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 0cccbcb2d03e..0362c645d96e 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1256,15 +1256,20 @@ 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_LIMITED_M_N) },
 };
 
 #undef OUI
@@ -1283,6 +1288,7 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
 	const struct dpcd_quirk *quirk;
 	u32 quirks = 0;
 	int i;
+	u8 any_device[] = DEVICE_ID_ANY;
 
 	for (i = 0; i < ARRAY_SIZE(dpcd_quirk_list); i++) {
 		quirk = &dpcd_quirk_list[i];
@@ -1293,12 +1299,19 @@ 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 (memcmp(quirk->device_id, any_device, sizeof(any_device)) != 0 &&
+		    memcmp(quirk->device_id, ident->device_id, sizeof(ident->device_id)) != 0)
+			continue;
+
 		quirks |= quirk->quirks;
 	}
 
 	return quirks;
 }
 
+#undef DEVICE_ID_ANY
+#undef DEVICE_ID
+
 /**
  * drm_dp_read_desc - read sink/branch descriptor from DPCD
  * @aux: DisplayPort AUX channel
-- 
2.7.4

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

[PATCH v2 2/3] drm: Change limited M/N quirk to constant N quirk.

[Lee, Shawn C]

Some DP dongles in particular seem to be fussy about too large
link M/N values. Set specific value for N divider can resolve
this issue per dongle vendor's comment. So configure N as
constant value (0x8000) to instead of reduce M/N formula when
specific DP dongle connected.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c      |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++---------------
 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, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 0362c645d96e..f3a7563eb8a1 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1269,7 +1269,7 @@ struct dpcd_quirk {
 
 static const struct dpcd_quirk dpcd_quirk_list[] = {
 	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
-	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
+	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
 };
 
 #undef OUI
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ec3e24f07486..da0c7fbef3ef 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6680,22 +6680,20 @@ 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.
+	 * Several DP dongles in particular seem to be fussy about
+	 * too large link M/N values. Give N value as 0x8000 that
+	 * should be acceptable by specific devices. 0x8000 is the
+	 * specified fixed N value for asynchronous clock mode,
+	 * which the devices expect also in synchronous clock mode.
 	 */
-	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 +6702,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..f59359e42428 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 compatibility.
 	 */
-	DP_DPCD_QUIRK_LIMITED_M_N,
+	DP_DPCD_QUIRK_CONSTANT_N,
 };
 
 /**
-- 
2.7.4

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

[PATCH v2 3/3] 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.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index f3a7563eb8a1..67d683453f1c 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1270,6 +1270,8 @@ struct dpcd_quirk {
 static const struct dpcd_quirk dpcd_quirk_list[] = {
 	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
 	{ 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
-- 
2.7.4

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

Re: [PATCH v2 0/3] add LG panel to dpcd quirk database

[Alex Deucher]

On Mon, Sep 10, 2018 at 10:55 AM Lee, Shawn C <shawn.c.lee@intel.com> wrote:
>
> 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, it will enter BITS mode
> and display black screen. This patch series set constant N
> value for specific sink/branch device that would cover
> similar issue.

Is this an explicit requirement of the panel itself or just a
workaround for for a PLL limitation on a particular GPU?  For example,
due to hw design and electrical characteristics, certain divider
combinations may not be viable on certain asics.  If that's the case,
shouldn't this be handled in an asic specific manor?  If it's a
requirement of the panel itself, ignore me and carry on.

Alex

>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
>
> Lee, Shawn C (3):
>   drm: Add support for device_id based detection.
>   drm: Change limited M/N quirk to constant N quirk.
>   drm: add LG eDP panel to quirk database
>
>  drivers/gpu/drm/drm_dp_helper.c      | 17 ++++++++++++++++-
>  drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++---------------
>  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, 40 insertions(+), 27 deletions(-)
>
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH v2 0/3] add LG panel to dpcd quirk database

[Lee, Shawn C]

On Mon, Sep 10, 2018 at 10:55 AM Alex Deucher wrote:
>>
>> 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, it will enter BITS mode and display black 
>> screen. This patch series set constant N value for specific 
>> sink/branch device that would cover similar issue.
>
>Is this an explicit requirement of the panel itself or just a workaround for for a PLL limitation on a particular GPU?  For example, due to hw design and electrical characteristics, certain divider combinations may not be viable on certain asics.  If that's the case, shouldn't this be handled in an asic specific manor?  If it's a requirement of the panel itself, ignore me and carry on.
>
>Alex
>

This is a special requirement that panel's tcon ask a particular value of N divider. Tcon needs this value for internal configuration.

>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Cooper Chiou <cooper.chiou@intel.com>
>> Cc: Matt Atwood <matthew.s.atwood@intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Cc: Clint Taylor <clinton.a.taylor@intel.com>
>>
>> Lee, Shawn C (3):
>>   drm: Add support for device_id based detection.
>>   drm: Change limited M/N quirk to constant N quirk.
>>   drm: add LG eDP panel to quirk database
>>
>>  drivers/gpu/drm/drm_dp_helper.c      | 17 ++++++++++++++++-
>>  drivers/gpu/drm/i915/intel_display.c | 28 
>> +++++++++++++---------------  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, 40 insertions(+), 27 deletions(-)
>>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 0/3] add LG panel to dpcd quirk database

[Lee, Shawn C]

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, it will enter BITS mode
and display black screen. This patch series set constant N
value for specific sink/branch device that would cover
similar issue.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>

Lee, Shawn C (3):
  drm: Add support for device_id based detection.
  drm: Change limited M/N quirk to constant N quirk.
  drm: add LG eDP panel to quirk database

 drivers/gpu/drm/drm_dp_helper.c      | 17 ++++++++++++++++-
 drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++---------------
 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, 40 insertions(+), 27 deletions(-)

-- 
2.7.4

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

[PATCH v3 1/3] drm: Add support for device_id based detection.

[Lee, Shawn C]

DP quirk list just compare sink or branch device's OUI so far.
That means particular vendor's products will be applied specific
change. This change would confirm device_id the same or not.
Then driver can implement some changes for branch/sink device
that really need additional WA.

v2: use sizeof instead of hard coded '6'
v3: add lost commit messages back for version 2

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 0cccbcb2d03e..0362c645d96e 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1256,15 +1256,20 @@ 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_LIMITED_M_N) },
 };
 
 #undef OUI
@@ -1283,6 +1288,7 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
 	const struct dpcd_quirk *quirk;
 	u32 quirks = 0;
 	int i;
+	u8 any_device[] = DEVICE_ID_ANY;
 
 	for (i = 0; i < ARRAY_SIZE(dpcd_quirk_list); i++) {
 		quirk = &dpcd_quirk_list[i];
@@ -1293,12 +1299,19 @@ 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 (memcmp(quirk->device_id, any_device, sizeof(any_device)) != 0 &&
+		    memcmp(quirk->device_id, ident->device_id, sizeof(ident->device_id)) != 0)
+			continue;
+
 		quirks |= quirk->quirks;
 	}
 
 	return quirks;
 }
 
+#undef DEVICE_ID_ANY
+#undef DEVICE_ID
+
 /**
  * drm_dp_read_desc - read sink/branch descriptor from DPCD
  * @aux: DisplayPort AUX channel
-- 
2.7.4

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

[PATCH v3 2/3] drm: Change limited M/N quirk to constant N quirk.

[Lee, Shawn C]

Some DP dongles in particular seem to be fussy about too large
link M/N values. Set specific value for N divider can resolve
this issue per dongle vendor's comment. So configure N as
constant value (0x8000) to instead of reduce M/N formula when
specific DP dongle connected.

v2: add more comments for issue description and fix typo.
v3: add lost commit messages back for version 2

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c      |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++---------------
 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, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 0362c645d96e..f3a7563eb8a1 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1269,7 +1269,7 @@ struct dpcd_quirk {
 
 static const struct dpcd_quirk dpcd_quirk_list[] = {
 	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
-	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
+	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
 };
 
 #undef OUI
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ec3e24f07486..da0c7fbef3ef 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6680,22 +6680,20 @@ 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.
+	 * Several DP dongles in particular seem to be fussy about
+	 * too large link M/N values. Give N value as 0x8000 that
+	 * should be acceptable by specific devices. 0x8000 is the
+	 * specified fixed N value for asynchronous clock mode,
+	 * which the devices expect also in synchronous clock mode.
 	 */
-	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 +6702,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..f59359e42428 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

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

[PATCH v3 3/3] 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: no update
v3: add lost commit messages back for version 2

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index f3a7563eb8a1..67d683453f1c 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1270,6 +1270,8 @@ struct dpcd_quirk {
 static const struct dpcd_quirk dpcd_quirk_list[] = {
 	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
 	{ 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
-- 
2.7.4

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

Re: [PATCH v3 0/3] add LG panel to dpcd quirk database

[Jani Nikula]

On Tue, 11 Sep 2018, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> 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, it will enter BITS mode
> and display black screen. This patch series set constant N
> value for specific sink/branch device that would cover
> similar issue.

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

on the series, but please do post the whole thing as a new thread, *not*
in-reply-to the old thread, and *also* to intel-gfx. Patchwork doesn't
seem to have picked this up now, so we're missing CI results.


>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
>
> Lee, Shawn C (3):
>   drm: Add support for device_id based detection.
>   drm: Change limited M/N quirk to constant N quirk.
>   drm: add LG eDP panel to quirk database
>
>  drivers/gpu/drm/drm_dp_helper.c      | 17 ++++++++++++++++-
>  drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++---------------
>  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, 40 insertions(+), 27 deletions(-)

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

Re: [PATCH v3 0/3] add LG panel to dpcd quirk database

[Clint Taylor]

On 09/11/2018 01:56 AM, Lee, Shawn C wrote:
> 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, it will enter BITS mode
> and display black screen. This patch series set constant N
> value for specific sink/branch device that would cover
> similar issue.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
>
> Lee, Shawn C (3):
>    drm: Add support for device_id based detection.
>    drm: Change limited M/N quirk to constant N quirk.
>    drm: add LG eDP panel to quirk database
>
>   drivers/gpu/drm/drm_dp_helper.c      | 17 ++++++++++++++++-
>   drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++---------------
>   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, 40 insertions(+), 27 deletions(-)
>

Tested quirk on Analogix 7737 based USB_C->HDMI dongle that the original 
Limited M/N quirk was designed around. New quirk appears to work correctly.

-Clint

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

Re: [PATCH v2 1/3] drm: Add support for device_id based detection.

[Dhinakaran Pandiyan]

On Mon, 2018-09-10 at 08:26 -0700, Lee, Shawn C wrote:
> DP quirk list just compare sink or branch device's OUI so far.
> That means particular vendor's products will be applied specific
> change. This change would confirm device_id the same or not.
> Then driver can implement some changes for branch/sink device
> that really need additional WA.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c
> b/drivers/gpu/drm/drm_dp_helper.c
> index 0cccbcb2d03e..0362c645d96e 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1256,15 +1256,20 @@ EXPORT_SYMBOL(drm_dp_stop_crc);
>  
>  struct dpcd_quirk {
>  	u8 oui[3];
> +	u8 device_id[6];
>  	bool is_branch;

With device id included, do we still need 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_LIMITED_M_N) },
>  };
>  
>  #undef OUI
> @@ -1283,6 +1288,7 @@ drm_dp_get_quirks(const struct
> drm_dp_dpcd_ident *ident, bool is_branch)
>  	const struct dpcd_quirk *quirk;
>  	u32 quirks = 0;
>  	int i;
> +	u8 any_device[] = DEVICE_ID_ANY;
>  
>  	for (i = 0; i < ARRAY_SIZE(dpcd_quirk_list); i++) {
>  		quirk = &dpcd_quirk_list[i];
> @@ -1293,12 +1299,19 @@ drm_dp_get_quirks(const struct
> drm_dp_dpcd_ident *ident, bool is_branch)

Update documentation that currently says -  
 "* For now, only the OUI (first three bytes) is used, but this may be
extended
 * to device identification string ..."

>  		if (memcmp(quirk->oui, ident->oui, sizeof(ident-
> >oui)) != 0)
>  			continue;
>  
> +		if (memcmp(quirk->device_id, any_device,
> sizeof(any_device)) != 0 &&
> +		    memcmp(quirk->device_id, ident->device_id,
> sizeof(ident->device_id)) != 0)
> +			continue;
> +
>  		quirks |= quirk->quirks;
>  	}
>  
>  	return quirks;
>  }
>  
> +#undef DEVICE_ID_ANY
> +#undef DEVICE_ID
> +
>  /**
>   * drm_dp_read_desc - read sink/branch descriptor from DPCD
>   * @aux: DisplayPort AUX channel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Dhinakaran Pandiyan]

On Mon, 2018-09-10 at 14:43 +0300, Jani Nikula wrote:
> On Mon, 10 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>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Clint Taylor <clinton.a.taylor@intel.com>
> > Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
> 
> No access to the panel or its details, so instead of review,
> 
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> 
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c
> > b/drivers/gpu/drm/drm_dp_helper.c
> > index d0c1250975ab..0ef7c43a9025 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -1270,6 +1270,8 @@ struct dpcd_quirk {
> >  static const struct dpcd_quirk dpcd_quirk_list[] = {
> >  	/* Analogix 7737 needs reduced M and N at HBR2 link rates
> > */
> >  	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, 

Wonder if DEVICE_ID_ANY still the accurate criteria for these dongles
now that we can check against device IDs. I guess, since the quirk
fixes multiple dongles we probably can't check against a single device
ID.


> > BIT(DP_DPCD_QUIRK_CONSTANT_N) },
> > +	/* LG LP140WF6-SPM1 eDP panel */

If you are resending the patches, it might be worth updating the
comment to
/* LG LP140WF6-SPM1 eDP panel needs N value of 0x8000 */

> > +	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a',
> > 'r', 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
> >  };
> >  
> >  #undef OUI
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

RE: [PATCH v3 0/3] add LG panel to dpcd quirk database

[Lee, Shawn C]

On Tue, 11 Sep 2018, Jani Nikula wrote:
>> 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, it will enter BITS mode and display black 
>> screen. This patch series set constant N value for specific 
>> sink/branch device that would cover similar issue.
>
>Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
>on the series, but please do post the whole thing as a new thread, *not* in-reply-to the old thread, and *also* to intel-gfx. Patchwork doesn't seem to have picked this up now, so we're missing CI results.
>

Thanks for reminding. I will commit as a new thread and send "to" intel-gfx as well.

>
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Cooper Chiou <cooper.chiou@intel.com>
>> Cc: Matt Atwood <matthew.s.atwood@intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Cc: Clint Taylor <clinton.a.taylor@intel.com>
>>
>> Lee, Shawn C (3):
>>   drm: Add support for device_id based detection.
>>   drm: Change limited M/N quirk to constant N quirk.
>>   drm: add LG eDP panel to quirk database
>>
>>  drivers/gpu/drm/drm_dp_helper.c      | 17 ++++++++++++++++-
>>  drivers/gpu/drm/i915/intel_display.c | 28 
>> +++++++++++++---------------  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, 40 insertions(+), 27 deletions(-)
>
>--
>Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH v2 1/3] drm: Add support for device_id based detection.

[Lee, Shawn C]

On Wed 9/12/2018 8:12 AM , Dhinakaran Pandiyan wrote:
>> DP quirk list just compare sink or branch device's OUI so far.
>> That means particular vendor's products will be applied specific 
>> change. This change would confirm device_id the same or not.
>> Then driver can implement some changes for branch/sink device that 
>> really need additional WA.
>> 
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Cooper Chiou <cooper.chiou@intel.com>
>> Cc: Matt Atwood <matthew.s.atwood@intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Cc: Clint Taylor <clinton.a.taylor@intel.com>
>> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
>> ---
>>  drivers/gpu/drm/drm_dp_helper.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c 
>> b/drivers/gpu/drm/drm_dp_helper.c index 0cccbcb2d03e..0362c645d96e 
>> 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -1256,15 +1256,20 @@ EXPORT_SYMBOL(drm_dp_stop_crc);
>>  
>>  struct dpcd_quirk {
>>  	u8 oui[3];
>> +	u8 device_id[6];
>>  	bool is_branch;
>
>With device id included, do we still need is_branch? 
>

I think we should keep is_branch here. So far, this WA will be applied for all analogix
branch devices to fix multiple dongles can't display issue.

>>  	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_LIMITED_M_N) },
>>  };
>>  
>>  #undef OUI
>> @@ -1283,6 +1288,7 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident 
>> *ident, bool is_branch)
>>  	const struct dpcd_quirk *quirk;
>>  	u32 quirks = 0;
>>  	int i;
>> +	u8 any_device[] = DEVICE_ID_ANY;
>>  
>>  	for (i = 0; i < ARRAY_SIZE(dpcd_quirk_list); i++) {
>>  		quirk = &dpcd_quirk_list[i];
>> @@ -1293,12 +1299,19 @@ drm_dp_get_quirks(const struct 
>> drm_dp_dpcd_ident *ident, bool is_branch)
>
>Update documentation that currently says -
> "* For now, only the OUI (first three bytes) is used, but this may be extended
> * to device identification string ..."
>
>>  		if (memcmp(quirk->oui, ident->oui, sizeof(ident-
>> >oui)) != 0)
>>  			continue;
>>  
>> +		if (memcmp(quirk->device_id, any_device,
>> sizeof(any_device)) != 0 &&
>> +		    memcmp(quirk->device_id, ident->device_id,
>> sizeof(ident->device_id)) != 0)
>> +			continue;
>> +
>>  		quirks |= quirk->quirks;
>>  	}
>>  
>>  	return quirks;
>>  }
>>  
>> +#undef DEVICE_ID_ANY
>> +#undef DEVICE_ID
>> +
>>  /**
>>   * drm_dp_read_desc - read sink/branch descriptor from DPCD
>>   * @aux: DisplayPort AUX channel
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 0/3] add LG panel to dpcd quirk database

[Jani Nikula]

On Tue, 11 Sep 2018, Clint Taylor <clinton.a.taylor@intel.com> wrote:
> On 09/11/2018 01:56 AM, Lee, Shawn C wrote:
>> 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, it will enter BITS mode
>> and display black screen. This patch series set constant N
>> value for specific sink/branch device that would cover
>> similar issue.
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Cooper Chiou <cooper.chiou@intel.com>
>> Cc: Matt Atwood <matthew.s.atwood@intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Cc: Clint Taylor <clinton.a.taylor@intel.com>
>>
>> Lee, Shawn C (3):
>>    drm: Add support for device_id based detection.
>>    drm: Change limited M/N quirk to constant N quirk.
>>    drm: add LG eDP panel to quirk database
>>
>>   drivers/gpu/drm/drm_dp_helper.c      | 17 ++++++++++++++++-
>>   drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++---------------
>>   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, 40 insertions(+), 27 deletions(-)
>>
>
> Tested quirk on Analogix 7737 based USB_C->HDMI dongle that the original 
> Limited M/N quirk was designed around. New quirk appears to work correctly.

Thanks a lot for this!

BR,
Jani.

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