[Intel-gfx] [PATCH 1/2] drm/i915: Modify DP set clock to accomodate more eDP timings v2
eDP 1.4 supports 4-5 extra link rates in additional to current 2 link
rate. Create a structure to store the DPLL divisor data to improve
readability.
v2: Fix the gen4_dpll/pch_dpll initialization to C99
designated initializers, and use a single loop for all platforms. (Jani and
Daniel)
Signed-off-by: Chon Ming Lee chon.ming@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 65 ++-
1 files changed, 37 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2151d13..fd09058 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,6 +38,27 @@
#define DP_LINK_CHECK_TIMEOUT (10 * 1000)
+struct dp_link_dpll {
+ int link_bw;
+ struct dpll dpll;
+};
+
+static const struct dp_link_dpll gen4_dpll[] =
+{
+ { DP_LINK_BW_1_62,
+ { .p1 = 2, .p2 = 10, .n = 2, .m1 = 23, .m2 = 8 }},
+ { DP_LINK_BW_2_7,
+ { .p1 = 1, .p2 = 10, .n = 1, .m1 = 14, .m2 = 2 }}
+};
+
+static const struct dp_link_dpll pch_dpll[] =
+{
+ { DP_LINK_BW_1_62,
+ { .p1 = 2, .p2 = 10, .n = 1, .m1 = 12, .m2 = 9 }},
+ { DP_LINK_BW_2_7,
+ { .p1 = 1, .p2 = 10, .n = 2, .m1 = 14, .m2 = 8 }}
+};
+
/**
* is_edp - is the given port attached to an eDP panel (either CPU or PCH)
* @intel_dp: DP struct
@@ -649,42 +670,30 @@ intel_dp_set_clock(struct intel_encoder *encoder,
struct intel_crtc_config *pipe_config, int link_bw)
{
struct drm_device *dev = encoder-base.dev;
+ const struct dp_link_dpll *divisor = NULL;
+ int i, count = 0;
if (IS_G4X(dev)) {
- if (link_bw == DP_LINK_BW_1_62) {
- pipe_config-dpll.p1 = 2;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.n = 2;
- pipe_config-dpll.m1 = 23;
- pipe_config-dpll.m2 = 8;
- } else {
- pipe_config-dpll.p1 = 1;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.n = 1;
- pipe_config-dpll.m1 = 14;
- pipe_config-dpll.m2 = 2;
- }
- pipe_config-clock_set = true;
+ divisor = gen4_dpll;
+ count = ARRAY_SIZE(gen4_dpll);
} else if (IS_HASWELL(dev)) {
/* Haswell has special-purpose DP DDI clocks. */
} else if (HAS_PCH_SPLIT(dev)) {
- if (link_bw == DP_LINK_BW_1_62) {
- pipe_config-dpll.n = 1;
- pipe_config-dpll.p1 = 2;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.m1 = 12;
- pipe_config-dpll.m2 = 9;
- } else {
- pipe_config-dpll.n = 2;
- pipe_config-dpll.p1 = 1;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.m1 = 14;
- pipe_config-dpll.m2 = 8;
- }
- pipe_config-clock_set = true;
+ divisor = pch_dpll;
+ count = ARRAY_SIZE(pch_dpll);
} else if (IS_VALLEYVIEW(dev)) {
/* FIXME: Need to figure out optimized DP clocks for vlv. */
}
+
+ if (divisor count) {
+ for (i = 0; i count; i++) {
+ if (link_bw == divisor[i].link_bw) {
+ pipe_config-dpll = divisor[i].dpll;
+ pipe_config-clock_set = true;
+ break;
+ }
+ }
+ }
}
bool
--
1.7.7.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Modify DP set clock to accomodate more eDP timings.
On 08/30 10:28, Jani Nikula wrote:
On Fri, 30 Aug 2013, Chon Ming Lee chon.ming@intel.com wrote:
eDP 1.4 supports 4-5 extra link rates in additional to current 2 link
rate. Create a structure to store the DPLL divisor data to improve
readability.
DP 1.2 already supports 3 link rates, right?
Yes, 3 link rate for DP 1.2, but most of the older intel gfx doesn't support the
highest 5.4Gbps link rate yet.
Signed-off-by: Chon Ming Lee chon.ming@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 48
+++---
1 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index 2151d13..ab8a5ff 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,6 +38,19 @@
#define DP_LINK_CHECK_TIMEOUT (10 * 1000)
+struct dp_link_dpll{
Missing space before {.
+ int link_bw;
+ struct dpll dpll;
+};
+
+static const struct dp_link_dpll gen4_dpll[] =
+ {{ DP_LINK_BW_1_62, {2,23,8,2,10,0,0,0,0}},
+ { DP_LINK_BW_2_7, {1,14,2,1,10,0,0,0,0}}};
+
+static const struct dp_link_dpll pch_dpll[] =
+ {{ DP_LINK_BW_1_62, {1,12,9,2,10,0,0,0,0}},
+ { DP_LINK_BW_2_7, {2,14,8,1,10,0,0,0,0}}};
+
Please switch these to use C99 designated initializers for clarity,
something like this:
static const struct dp_link_dpll gen4_dpll[] = {
{
.link_bw = DP_LINK_BW_1_62,
.dpll = {
.n = 2,
.m1 = 23, m2 = 8,
p1 = 2, p2 = 10,
},
}, {
.link_bw = DP_LINK_BW_2_7,
.dpll = {
.n = 1,
.m1 = 14, m2 = 2,
p1 = 1, p2 = 10,
},
}
};
Thanks, will make the correction.
/**
* is_edp - is the given port attached to an eDP panel (either CPU or PCH)
* @intel_dp: DP struct
@@ -649,37 +662,24 @@ intel_dp_set_clock(struct intel_encoder *encoder,
struct intel_crtc_config *pipe_config, int link_bw)
{
struct drm_device *dev = encoder-base.dev;
+ int i;
if (IS_G4X(dev)) {
- if (link_bw == DP_LINK_BW_1_62) {
- pipe_config-dpll.p1 = 2;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.n = 2;
- pipe_config-dpll.m1 = 23;
- pipe_config-dpll.m2 = 8;
- } else {
- pipe_config-dpll.p1 = 1;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.n = 1;
- pipe_config-dpll.m1 = 14;
- pipe_config-dpll.m2 = 2;
+ for(i = 0; i sizeof(gen4_dpll) / sizeof(struct dp_link_dpll);
i++) {
Please use i ARRAY_SIZE(gen4_dpll).
Already make this change. After sent out the patch, realize I should this
ARRAY_SIZE. Thanks to point this out.
+ if (link_bw == gen4_dpll[i].link_bw){
+ pipe_config-dpll = gen4_dpll[i].dpll;
+ break;
+ }
}
The old if-else used some values even if link_bw was bogus. I'm not sure
how likely that is. But if the link_bw is not found in the table for
some obscure reason (e.g. the hw doesn't support the link rate), this
fails. Perhaps we should have a test for i == ARRAY_SIZE(gen4_dpll) here
and complain loudly if we hit that, and perhaps use a fallback value.
In intel_dp_compute_config, it will only assign either one of two link rates
into link_bw before calling this function. The link_bw won't be out of range.
I would think the checking should be done prior to calling this function. For
example, in eDP 1.4, instead of supporting more link rates, there is possible to
use single lane, but choose the largest link rate, or use 4 lanes, with lower
link rate. If fail out here, might be too late to recover.
pipe_config-clock_set = true;
} else if (IS_HASWELL(dev)) {
/* Haswell has special-purpose DP DDI clocks. */
} else if (HAS_PCH_SPLIT(dev)) {
- if (link_bw == DP_LINK_BW_1_62) {
- pipe_config-dpll.n = 1;
- pipe_config-dpll.p1 = 2;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.m1 = 12;
- pipe_config-dpll.m2 = 9;
- } else {
- pipe_config-dpll.n = 2;
- pipe_config-dpll.p1 = 1;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.m1 = 14;
- pipe_config-dpll.m2 = 8;
+ for(i = 0; i sizeof(pch_dpll) / sizeof(struct dp_link_dpll);
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Modify DP set clock to accomodate more eDP timings.
On Sat, Aug 31, 2013 at 01:57:44AM +0800, Chon Ming Lee wrote:
eDP 1.4 supports 4-5 extra link rates in additional to current 2 link
rate. Create a structure to store the DPLL divisor data to improve
readability.
Signed-off-by: Chon Ming Lee chon.ming@intel.com
This is a neat idea and I agree it'll be much better when we add edp link
rates. Small comments below:
---
drivers/gpu/drm/i915/intel_dp.c | 48 +++---
1 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2151d13..ab8a5ff 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,6 +38,19 @@
#define DP_LINK_CHECK_TIMEOUT(10 * 1000)
+struct dp_link_dpll{
+ int link_bw;
+ struct dpll dpll;
+};
+
+static const struct dp_link_dpll gen4_dpll[] =
+ {{ DP_LINK_BW_1_62, {2,23,8,2,10,0,0,0,0}},
+ { DP_LINK_BW_2_7, {1,14,2,1,10,0,0,0,0}}};
Usually we only indent by one tab here. Also please use c99 initializers
and you don't need to explicitly initialize to 0. And a bit more space
helps readbility further imo:
static const struct dp_link_dpll gen4_dpll[] =
{
{ DP_LINK_BW_1_62,
{ .p1 = 2, .p2 = 10, .n = 2, .m1 = 23, .m2 = 8 }},
/* more ... */
};
+
+static const struct dp_link_dpll pch_dpll[] =
+ {{ DP_LINK_BW_1_62, {1,12,9,2,10,0,0,0,0}},
+ { DP_LINK_BW_2_7, {2,14,8,1,10,0,0,0,0}}};
+
/**
* is_edp - is the given port attached to an eDP panel (either CPU or PCH)
* @intel_dp: DP struct
@@ -649,37 +662,24 @@ intel_dp_set_clock(struct intel_encoder *encoder,
struct intel_crtc_config *pipe_config, int link_bw)
{
struct drm_device *dev = encoder-base.dev;
+ int i;
if (IS_G4X(dev)) {
- if (link_bw == DP_LINK_BW_1_62) {
- pipe_config-dpll.p1 = 2;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.n = 2;
- pipe_config-dpll.m1 = 23;
- pipe_config-dpll.m2 = 8;
- } else {
- pipe_config-dpll.p1 = 1;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.n = 1;
- pipe_config-dpll.m1 = 14;
- pipe_config-dpll.m2 = 2;
+ for(i = 0; i sizeof(gen4_dpll) / sizeof(struct dp_link_dpll);
i++) {
+ if (link_bw == gen4_dpll[i].link_bw){
+ pipe_config-dpll = gen4_dpll[i].dpll;
+ break;
+ }
}
pipe_config-clock_set = true;
} else if (IS_HASWELL(dev)) {
/* Haswell has special-purpose DP DDI clocks. */
} else if (HAS_PCH_SPLIT(dev)) {
- if (link_bw == DP_LINK_BW_1_62) {
- pipe_config-dpll.n = 1;
- pipe_config-dpll.p1 = 2;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.m1 = 12;
- pipe_config-dpll.m2 = 9;
- } else {
- pipe_config-dpll.n = 2;
- pipe_config-dpll.p1 = 1;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.m1 = 14;
- pipe_config-dpll.m2 = 8;
+ for(i = 0; i sizeof(pch_dpll) / sizeof(struct dp_link_dpll);
i++) {
+ if (link_bw == pch_dpll[i].link_bw){
+ pipe_config-dpll = pch_dpll[i].dpll;
+ break;
+ }
I'd add temporary variables here to first select the right platform (and
size of the array) and then only have one for loop to fish out the right
value.
Cheers, Daniel
}
pipe_config-clock_set = true;
} else if (IS_VALLEYVIEW(dev)) {
--
1.7.7.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Modify DP set clock to accomodate more eDP timings.
On Fri, 30 Aug 2013, Chon Ming Lee chon.ming@intel.com wrote:
eDP 1.4 supports 4-5 extra link rates in additional to current 2 link
rate. Create a structure to store the DPLL divisor data to improve
readability.
DP 1.2 already supports 3 link rates, right?
Signed-off-by: Chon Ming Lee chon.ming@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 48 +++---
1 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2151d13..ab8a5ff 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,6 +38,19 @@
#define DP_LINK_CHECK_TIMEOUT(10 * 1000)
+struct dp_link_dpll{
Missing space before {.
+ int link_bw;
+ struct dpll dpll;
+};
+
+static const struct dp_link_dpll gen4_dpll[] =
+ {{ DP_LINK_BW_1_62, {2,23,8,2,10,0,0,0,0}},
+ { DP_LINK_BW_2_7, {1,14,2,1,10,0,0,0,0}}};
+
+static const struct dp_link_dpll pch_dpll[] =
+ {{ DP_LINK_BW_1_62, {1,12,9,2,10,0,0,0,0}},
+ { DP_LINK_BW_2_7, {2,14,8,1,10,0,0,0,0}}};
+
Please switch these to use C99 designated initializers for clarity,
something like this:
static const struct dp_link_dpll gen4_dpll[] = {
{
.link_bw = DP_LINK_BW_1_62,
.dpll = {
.n = 2,
.m1 = 23, m2 = 8,
p1 = 2, p2 = 10,
},
}, {
.link_bw = DP_LINK_BW_2_7,
.dpll = {
.n = 1,
.m1 = 14, m2 = 2,
p1 = 1, p2 = 10,
},
}
};
/**
* is_edp - is the given port attached to an eDP panel (either CPU or PCH)
* @intel_dp: DP struct
@@ -649,37 +662,24 @@ intel_dp_set_clock(struct intel_encoder *encoder,
struct intel_crtc_config *pipe_config, int link_bw)
{
struct drm_device *dev = encoder-base.dev;
+ int i;
if (IS_G4X(dev)) {
- if (link_bw == DP_LINK_BW_1_62) {
- pipe_config-dpll.p1 = 2;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.n = 2;
- pipe_config-dpll.m1 = 23;
- pipe_config-dpll.m2 = 8;
- } else {
- pipe_config-dpll.p1 = 1;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.n = 1;
- pipe_config-dpll.m1 = 14;
- pipe_config-dpll.m2 = 2;
+ for(i = 0; i sizeof(gen4_dpll) / sizeof(struct dp_link_dpll);
i++) {
Please use i ARRAY_SIZE(gen4_dpll).
+ if (link_bw == gen4_dpll[i].link_bw){
+ pipe_config-dpll = gen4_dpll[i].dpll;
+ break;
+ }
}
The old if-else used some values even if link_bw was bogus. I'm not sure
how likely that is. But if the link_bw is not found in the table for
some obscure reason (e.g. the hw doesn't support the link rate), this
fails. Perhaps we should have a test for i == ARRAY_SIZE(gen4_dpll) here
and complain loudly if we hit that, and perhaps use a fallback value.
pipe_config-clock_set = true;
} else if (IS_HASWELL(dev)) {
/* Haswell has special-purpose DP DDI clocks. */
} else if (HAS_PCH_SPLIT(dev)) {
- if (link_bw == DP_LINK_BW_1_62) {
- pipe_config-dpll.n = 1;
- pipe_config-dpll.p1 = 2;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.m1 = 12;
- pipe_config-dpll.m2 = 9;
- } else {
- pipe_config-dpll.n = 2;
- pipe_config-dpll.p1 = 1;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.m1 = 14;
- pipe_config-dpll.m2 = 8;
+ for(i = 0; i sizeof(pch_dpll) / sizeof(struct dp_link_dpll);
i++) {
+ if (link_bw == pch_dpll[i].link_bw){
+ pipe_config-dpll = pch_dpll[i].dpll;
+ break;
+ }
}
Same here.
BR,
Jani.
pipe_config-clock_set = true;
} else if (IS_VALLEYVIEW(dev)) {
--
1.7.7.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Modify DP set clock to accomodate more eDP timings.
eDP 1.4 supports 4-5 extra link rates in additional to current 2 link
rate. Create a structure to store the DPLL divisor data to improve
readability.
Signed-off-by: Chon Ming Lee chon.ming@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 48 +++---
1 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2151d13..ab8a5ff 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,6 +38,19 @@
#define DP_LINK_CHECK_TIMEOUT (10 * 1000)
+struct dp_link_dpll{
+ int link_bw;
+ struct dpll dpll;
+};
+
+static const struct dp_link_dpll gen4_dpll[] =
+ {{ DP_LINK_BW_1_62, {2,23,8,2,10,0,0,0,0}},
+ { DP_LINK_BW_2_7, {1,14,2,1,10,0,0,0,0}}};
+
+static const struct dp_link_dpll pch_dpll[] =
+ {{ DP_LINK_BW_1_62, {1,12,9,2,10,0,0,0,0}},
+ { DP_LINK_BW_2_7, {2,14,8,1,10,0,0,0,0}}};
+
/**
* is_edp - is the given port attached to an eDP panel (either CPU or PCH)
* @intel_dp: DP struct
@@ -649,37 +662,24 @@ intel_dp_set_clock(struct intel_encoder *encoder,
struct intel_crtc_config *pipe_config, int link_bw)
{
struct drm_device *dev = encoder-base.dev;
+ int i;
if (IS_G4X(dev)) {
- if (link_bw == DP_LINK_BW_1_62) {
- pipe_config-dpll.p1 = 2;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.n = 2;
- pipe_config-dpll.m1 = 23;
- pipe_config-dpll.m2 = 8;
- } else {
- pipe_config-dpll.p1 = 1;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.n = 1;
- pipe_config-dpll.m1 = 14;
- pipe_config-dpll.m2 = 2;
+ for(i = 0; i sizeof(gen4_dpll) / sizeof(struct dp_link_dpll);
i++) {
+ if (link_bw == gen4_dpll[i].link_bw){
+ pipe_config-dpll = gen4_dpll[i].dpll;
+ break;
+ }
}
pipe_config-clock_set = true;
} else if (IS_HASWELL(dev)) {
/* Haswell has special-purpose DP DDI clocks. */
} else if (HAS_PCH_SPLIT(dev)) {
- if (link_bw == DP_LINK_BW_1_62) {
- pipe_config-dpll.n = 1;
- pipe_config-dpll.p1 = 2;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.m1 = 12;
- pipe_config-dpll.m2 = 9;
- } else {
- pipe_config-dpll.n = 2;
- pipe_config-dpll.p1 = 1;
- pipe_config-dpll.p2 = 10;
- pipe_config-dpll.m1 = 14;
- pipe_config-dpll.m2 = 8;
+ for(i = 0; i sizeof(pch_dpll) / sizeof(struct dp_link_dpll);
i++) {
+ if (link_bw == pch_dpll[i].link_bw){
+ pipe_config-dpll = pch_dpll[i].dpll;
+ break;
+ }
}
pipe_config-clock_set = true;
} else if (IS_VALLEYVIEW(dev)) {
--
1.7.7.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
