Re: [PATCH v3 05/14] drm: bridge/analogix_dp: fix link_rate lane_count bug

2015-08-20 Thread Yakir Yang

Hi Jingoo,

On 08/20/2015 02:22 AM, Jingoo Han wrote:

On 2015. 8. 19., at PM 11:50, Yakir Yang y...@rock-chips.com wrote:

link_rate and lane_count already configed in analogix_dp_set_link_train(),

s/configed/configured

Also, the commit name such as fix ... bug is not good.
How about following?

drm: bridge/analogix_dp: remove duplicate configuration of link rate and link 
count


Thanks, done, it's more readable.

- Yakir

Best regards,
Jingoo Han


so we don't need to config those repeatly after training finished, just
remove them out.

Beside Display Port 1.2 already support 5.4Gbps link rate, the maximum sets
would change from {1.62Gbps, 2.7Gbps} to {1.62Gbps, 2.7Gbps, 5.4Gbps}.

Signed-off-by: Yakir Yang y...@rock-chips.com
---
Changes in v3:
- Take Thierry Reding suggest, link_rate and lane_count shouldn't config to
  the DT property value directly, but we can take those as hardware limite.
  For example, RK3288 only support 4 physical lanes of 2.7/1.62 Gbps/lane,
  so DT property would like link-rate = 0x0a lane-count = 4.

Changes in v2: None

drivers/gpu/drm/bridge/analogix_dp_core.c | 16 
drivers/gpu/drm/bridge/analogix_dp_core.h |  9 +
2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix_dp_core.c
index 480cc13..1778e0a 100644
--- a/drivers/gpu/drm/bridge/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix_dp_core.c
@@ -635,6 +635,8 @@ static void analogix_dp_get_max_rx_bandwidth(struct 
analogix_dp_device *dp,
/*
 * For DP rev.1.1, Maximum link rate of Main Link lanes
 * 0x06 = 1.62 Gbps, 0x0a = 2.7 Gbps
+ * For DP rev.1.2, Maximum link rate of Main Link lanes
+ * 0x06 = 1.62 Gbps, 0x0a = 2.7 Gbps, 0x14 = 5.4Gbps
 */
analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LINK_RATE, data);
*bandwidth = data;
@@ -668,7 +670,8 @@ static void analogix_dp_init_training(struct 
analogix_dp_device *dp,
analogix_dp_get_max_rx_lane_count(dp, dp-link_train.lane_count);

if ((dp-link_train.link_rate != LINK_RATE_1_62GBPS) 
-(dp-link_train.link_rate != LINK_RATE_2_70GBPS)) {
+(dp-link_train.link_rate != LINK_RATE_2_70GBPS) 
+(dp-link_train.link_rate != LINK_RATE_5_40GBPS)) {
dev_err(dp-dev, Rx Max Link Rate is abnormal :%x !\n,
dp-link_train.link_rate);
dp-link_train.link_rate = LINK_RATE_1_62GBPS;
@@ -901,8 +904,8 @@ static void analogix_dp_commit(struct analogix_dp_device 
*dp)
return;
}

-ret = analogix_dp_set_link_train(dp, dp-video_info-lane_count,
- dp-video_info-link_rate);
+ret = analogix_dp_set_link_train(dp, dp-video_info-max_lane_count,
+ dp-video_info-max_link_rate);
if (ret) {
dev_err(dp-dev, unable to do link train\n);
return;
@@ -912,9 +915,6 @@ static void analogix_dp_commit(struct analogix_dp_device 
*dp)
analogix_dp_enable_rx_to_enhanced_mode(dp, 1);
analogix_dp_enable_enhanced_mode(dp, 1);

-analogix_dp_set_lane_count(dp, dp-video_info-lane_count);
-analogix_dp_set_link_bandwidth(dp, dp-video_info-link_rate);
-
analogix_dp_init_video(dp);
ret = analogix_dp_config_video(dp);
if (ret)
@@ -1198,13 +1198,13 @@ static struct video_info 
*analogix_dp_dt_parse_pdata(struct device *dev)
}

if (of_property_read_u32(dp_node, analogix,link-rate,
- dp_video_config-link_rate)) {
+ dp_video_config-max_link_rate)) {
dev_err(dev, failed to get link-rate\n);
return ERR_PTR(-EINVAL);
}

if (of_property_read_u32(dp_node, analogix,lane-count,
- dp_video_config-lane_count)) {
+ dp_video_config-max_lane_count)) {
dev_err(dev, failed to get lane-count\n);
return ERR_PTR(-EINVAL);
}
diff --git a/drivers/gpu/drm/bridge/analogix_dp_core.h 
b/drivers/gpu/drm/bridge/analogix_dp_core.h
index 2cefde9..941b34f 100644
--- a/drivers/gpu/drm/bridge/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix_dp_core.h
@@ -21,8 +21,9 @@
#define MAX_EQ_LOOP 5

enum link_rate_type {
-LINK_RATE_1_62GBPS = 0x06,
-LINK_RATE_2_70GBPS = 0x0a
+LINK_RATE_1_62GBPS = DP_LINK_BW_1_62,
+LINK_RATE_2_70GBPS = DP_LINK_BW_2_7,
+LINK_RATE_5_40GBPS = DP_LINK_BW_5_4,
};

enum link_lane_count_type {
@@ -128,8 +129,8 @@ struct video_info {
enum color_coefficient ycbcr_coeff;
enum color_depth color_depth;

-enum link_rate_type link_rate;
-enum link_lane_count_type lane_count;
+enum link_rate_type   max_link_rate;
+enum link_lane_count_type max_lane_count;
};

struct link_train {
--
1.9.1








--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 05/14] drm: bridge/analogix_dp: fix link_rate lane_count bug

2015-08-20 Thread Jingoo Han
On 2015. 8. 19., at PM 11:50, Yakir Yang y...@rock-chips.com wrote:
 
 link_rate and lane_count already configed in analogix_dp_set_link_train(),

s/configed/configured

Also, the commit name such as fix ... bug is not good.
How about following?

drm: bridge/analogix_dp: remove duplicate configuration of link rate and link 
count

Best regards,
Jingoo Han

 so we don't need to config those repeatly after training finished, just
 remove them out.
 
 Beside Display Port 1.2 already support 5.4Gbps link rate, the maximum sets
 would change from {1.62Gbps, 2.7Gbps} to {1.62Gbps, 2.7Gbps, 5.4Gbps}.
 
 Signed-off-by: Yakir Yang y...@rock-chips.com
 ---
 Changes in v3:
 - Take Thierry Reding suggest, link_rate and lane_count shouldn't config to
  the DT property value directly, but we can take those as hardware limite.
  For example, RK3288 only support 4 physical lanes of 2.7/1.62 Gbps/lane,
  so DT property would like link-rate = 0x0a lane-count = 4.
 
 Changes in v2: None
 
 drivers/gpu/drm/bridge/analogix_dp_core.c | 16 
 drivers/gpu/drm/bridge/analogix_dp_core.h |  9 +
 2 files changed, 13 insertions(+), 12 deletions(-)
 
 diff --git a/drivers/gpu/drm/bridge/analogix_dp_core.c 
 b/drivers/gpu/drm/bridge/analogix_dp_core.c
 index 480cc13..1778e0a 100644
 --- a/drivers/gpu/drm/bridge/analogix_dp_core.c
 +++ b/drivers/gpu/drm/bridge/analogix_dp_core.c
 @@ -635,6 +635,8 @@ static void analogix_dp_get_max_rx_bandwidth(struct 
 analogix_dp_device *dp,
/*
 * For DP rev.1.1, Maximum link rate of Main Link lanes
 * 0x06 = 1.62 Gbps, 0x0a = 2.7 Gbps
 + * For DP rev.1.2, Maximum link rate of Main Link lanes
 + * 0x06 = 1.62 Gbps, 0x0a = 2.7 Gbps, 0x14 = 5.4Gbps
 */
analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LINK_RATE, data);
*bandwidth = data;
 @@ -668,7 +670,8 @@ static void analogix_dp_init_training(struct 
 analogix_dp_device *dp,
analogix_dp_get_max_rx_lane_count(dp, dp-link_train.lane_count);
 
if ((dp-link_train.link_rate != LINK_RATE_1_62GBPS) 
 -(dp-link_train.link_rate != LINK_RATE_2_70GBPS)) {
 +(dp-link_train.link_rate != LINK_RATE_2_70GBPS) 
 +(dp-link_train.link_rate != LINK_RATE_5_40GBPS)) {
dev_err(dp-dev, Rx Max Link Rate is abnormal :%x !\n,
dp-link_train.link_rate);
dp-link_train.link_rate = LINK_RATE_1_62GBPS;
 @@ -901,8 +904,8 @@ static void analogix_dp_commit(struct analogix_dp_device 
 *dp)
return;
}
 
 -ret = analogix_dp_set_link_train(dp, dp-video_info-lane_count,
 - dp-video_info-link_rate);
 +ret = analogix_dp_set_link_train(dp, dp-video_info-max_lane_count,
 + dp-video_info-max_link_rate);
if (ret) {
dev_err(dp-dev, unable to do link train\n);
return;
 @@ -912,9 +915,6 @@ static void analogix_dp_commit(struct analogix_dp_device 
 *dp)
analogix_dp_enable_rx_to_enhanced_mode(dp, 1);
analogix_dp_enable_enhanced_mode(dp, 1);
 
 -analogix_dp_set_lane_count(dp, dp-video_info-lane_count);
 -analogix_dp_set_link_bandwidth(dp, dp-video_info-link_rate);
 -
analogix_dp_init_video(dp);
ret = analogix_dp_config_video(dp);
if (ret)
 @@ -1198,13 +1198,13 @@ static struct video_info 
 *analogix_dp_dt_parse_pdata(struct device *dev)
}
 
if (of_property_read_u32(dp_node, analogix,link-rate,
 - dp_video_config-link_rate)) {
 + dp_video_config-max_link_rate)) {
dev_err(dev, failed to get link-rate\n);
return ERR_PTR(-EINVAL);
}
 
if (of_property_read_u32(dp_node, analogix,lane-count,
 - dp_video_config-lane_count)) {
 + dp_video_config-max_lane_count)) {
dev_err(dev, failed to get lane-count\n);
return ERR_PTR(-EINVAL);
}
 diff --git a/drivers/gpu/drm/bridge/analogix_dp_core.h 
 b/drivers/gpu/drm/bridge/analogix_dp_core.h
 index 2cefde9..941b34f 100644
 --- a/drivers/gpu/drm/bridge/analogix_dp_core.h
 +++ b/drivers/gpu/drm/bridge/analogix_dp_core.h
 @@ -21,8 +21,9 @@
 #define MAX_EQ_LOOP 5
 
 enum link_rate_type {
 -LINK_RATE_1_62GBPS = 0x06,
 -LINK_RATE_2_70GBPS = 0x0a
 +LINK_RATE_1_62GBPS = DP_LINK_BW_1_62,
 +LINK_RATE_2_70GBPS = DP_LINK_BW_2_7,
 +LINK_RATE_5_40GBPS = DP_LINK_BW_5_4,
 };
 
 enum link_lane_count_type {
 @@ -128,8 +129,8 @@ struct video_info {
enum color_coefficient ycbcr_coeff;
enum color_depth color_depth;
 
 -enum link_rate_type link_rate;
 -enum link_lane_count_type lane_count;
 +enum link_rate_type   max_link_rate;
 +enum link_lane_count_type max_lane_count;
 };
 
 struct link_train {
 -- 
 1.9.1
 
 
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 05/14] drm: bridge/analogix_dp: fix link_rate lane_count bug

2015-08-19 Thread Yakir Yang
link_rate and lane_count already configed in analogix_dp_set_link_train(),
so we don't need to config those repeatly after training finished, just
remove them out.

Beside Display Port 1.2 already support 5.4Gbps link rate, the maximum sets
would change from {1.62Gbps, 2.7Gbps} to {1.62Gbps, 2.7Gbps, 5.4Gbps}.

Signed-off-by: Yakir Yang y...@rock-chips.com
---
Changes in v3:
- Take Thierry Reding suggest, link_rate and lane_count shouldn't config to
  the DT property value directly, but we can take those as hardware limite.
  For example, RK3288 only support 4 physical lanes of 2.7/1.62 Gbps/lane,
  so DT property would like link-rate = 0x0a lane-count = 4.

Changes in v2: None

 drivers/gpu/drm/bridge/analogix_dp_core.c | 16 
 drivers/gpu/drm/bridge/analogix_dp_core.h |  9 +
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix_dp_core.c
index 480cc13..1778e0a 100644
--- a/drivers/gpu/drm/bridge/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix_dp_core.c
@@ -635,6 +635,8 @@ static void analogix_dp_get_max_rx_bandwidth(struct 
analogix_dp_device *dp,
/*
 * For DP rev.1.1, Maximum link rate of Main Link lanes
 * 0x06 = 1.62 Gbps, 0x0a = 2.7 Gbps
+* For DP rev.1.2, Maximum link rate of Main Link lanes
+* 0x06 = 1.62 Gbps, 0x0a = 2.7 Gbps, 0x14 = 5.4Gbps
 */
analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LINK_RATE, data);
*bandwidth = data;
@@ -668,7 +670,8 @@ static void analogix_dp_init_training(struct 
analogix_dp_device *dp,
analogix_dp_get_max_rx_lane_count(dp, dp-link_train.lane_count);
 
if ((dp-link_train.link_rate != LINK_RATE_1_62GBPS) 
-   (dp-link_train.link_rate != LINK_RATE_2_70GBPS)) {
+   (dp-link_train.link_rate != LINK_RATE_2_70GBPS) 
+   (dp-link_train.link_rate != LINK_RATE_5_40GBPS)) {
dev_err(dp-dev, Rx Max Link Rate is abnormal :%x !\n,
dp-link_train.link_rate);
dp-link_train.link_rate = LINK_RATE_1_62GBPS;
@@ -901,8 +904,8 @@ static void analogix_dp_commit(struct analogix_dp_device 
*dp)
return;
}
 
-   ret = analogix_dp_set_link_train(dp, dp-video_info-lane_count,
-dp-video_info-link_rate);
+   ret = analogix_dp_set_link_train(dp, dp-video_info-max_lane_count,
+dp-video_info-max_link_rate);
if (ret) {
dev_err(dp-dev, unable to do link train\n);
return;
@@ -912,9 +915,6 @@ static void analogix_dp_commit(struct analogix_dp_device 
*dp)
analogix_dp_enable_rx_to_enhanced_mode(dp, 1);
analogix_dp_enable_enhanced_mode(dp, 1);
 
-   analogix_dp_set_lane_count(dp, dp-video_info-lane_count);
-   analogix_dp_set_link_bandwidth(dp, dp-video_info-link_rate);
-
analogix_dp_init_video(dp);
ret = analogix_dp_config_video(dp);
if (ret)
@@ -1198,13 +1198,13 @@ static struct video_info 
*analogix_dp_dt_parse_pdata(struct device *dev)
}
 
if (of_property_read_u32(dp_node, analogix,link-rate,
-dp_video_config-link_rate)) {
+dp_video_config-max_link_rate)) {
dev_err(dev, failed to get link-rate\n);
return ERR_PTR(-EINVAL);
}
 
if (of_property_read_u32(dp_node, analogix,lane-count,
-dp_video_config-lane_count)) {
+dp_video_config-max_lane_count)) {
dev_err(dev, failed to get lane-count\n);
return ERR_PTR(-EINVAL);
}
diff --git a/drivers/gpu/drm/bridge/analogix_dp_core.h 
b/drivers/gpu/drm/bridge/analogix_dp_core.h
index 2cefde9..941b34f 100644
--- a/drivers/gpu/drm/bridge/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix_dp_core.h
@@ -21,8 +21,9 @@
 #define MAX_EQ_LOOP 5
 
 enum link_rate_type {
-   LINK_RATE_1_62GBPS = 0x06,
-   LINK_RATE_2_70GBPS = 0x0a
+   LINK_RATE_1_62GBPS = DP_LINK_BW_1_62,
+   LINK_RATE_2_70GBPS = DP_LINK_BW_2_7,
+   LINK_RATE_5_40GBPS = DP_LINK_BW_5_4,
 };
 
 enum link_lane_count_type {
@@ -128,8 +129,8 @@ struct video_info {
enum color_coefficient ycbcr_coeff;
enum color_depth color_depth;
 
-   enum link_rate_type link_rate;
-   enum link_lane_count_type lane_count;
+   enum link_rate_type   max_link_rate;
+   enum link_lane_count_type max_lane_count;
 };
 
 struct link_train {
-- 
1.9.1


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html