* Re: [PATCH] video: exynos_dp: Clean up SW link training
From: Sean Paul @ 2012-11-01 16:15 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1351702475-31324-1-git-send-email-seanpaul@chromium.org>
On Thu, Nov 1, 2012 at 1:35 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> On Thursday, November 01, 2012 1:55 AM Sean Paul wrote
>>
>> Clean up some of the SW training code to make it more clear and reduce
>> duplicate code.
>>
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> ---
>> drivers/video/exynos/exynos_dp_core.c | 279 +++++++++++++--------------------
>> 1 files changed, 112 insertions(+), 167 deletions(-)
>>
>> Thanks for the pointer. There are still places where the code can be either
>> simplified, or duplication removed.
>
> Removing duplication is good, but don't change the Link training sequence.
> Link training sequence is very sensitive and tricky.
>
I definitely appreciate how tricky it is :) I didn't actually change
any of the functionality from the original code.
I noticed you made a couple of functional changes in your clean-up
patch (http://www.spinics.net/lists/linux-fbdev/msg06849.html). I
assumed that these functional changes were no-ops since bug fixes
would have gone in separate patches.
I've also done a fair bit of testing to ensure it works.
> I will modify your patch and I will submit new patch.
>
More comments below.
> Best regards,
> Jingoo Han
>
>>
>> Below is a rebased patch for your review.
>>
>> Sean
>>
>>
>> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
>> index 44820f2..b126e8a 100644
>> --- a/drivers/video/exynos/exynos_dp_core.c
>> +++ b/drivers/video/exynos/exynos_dp_core.c
>> @@ -276,7 +276,7 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
>>
>> /* Set sink to D0 (Sink Not Ready) mode. */
>> retval = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
>> - DPCD_SET_POWER_STATE_D0);
>> + DPCD_SET_POWER_STATE_D0);
>> if (retval)
>> return retval;
>>
>> @@ -301,17 +301,18 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
>> exynos_dp_set_training_pattern(dp, TRAINING_PTN1);
>>
>> /* Set RX training pattern */
>> - exynos_dp_write_byte_to_dpcd(dp,
>> - DPCD_ADDR_TRAINING_PATTERN_SET,
>> - DPCD_SCRAMBLING_DISABLED |
>> - DPCD_TRAINING_PATTERN_1);
>> + retval = exynos_dp_write_byte_to_dpcd(dp,
>> + DPCD_ADDR_TRAINING_PATTERN_SET,
>> + DPCD_SCRAMBLING_DISABLED | DPCD_TRAINING_PATTERN_1);
>> + if (retval)
>> + return retval;
>>
>> for (lane = 0; lane < lane_count; lane++)
>> buf[lane] = DPCD_PRE_EMPHASIS_PATTERN2_LEVEL0 |
>> DPCD_VOLTAGE_SWING_PATTERN1_LEVEL0;
>> - retval = exynos_dp_write_bytes_to_dpcd(dp,
>> - DPCD_ADDR_TRAINING_LANE0_SET,
>> - lane_count, buf);
>> +
>> + retval = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_TRAINING_LANE0_SET,
>> + lane_count, buf);
>>
>> return retval;
>> }
>> @@ -337,18 +338,17 @@ static int exynos_dp_clock_recovery_ok(u8 link_status[2], int lane_count)
>> return 0;
>> }
>>
>> -static int exynos_dp_channel_eq_ok(u8 link_align[3], int lane_count)
>> +static int exynos_dp_channel_eq_ok(u8 link_status[2], u8 link_align,
>> + int lane_count)
>> {
>> int lane;
>> - u8 lane_align;
>> u8 lane_status;
>>
>> - lane_align = link_align[2];
>> - if ((lane_align & DPCD_INTERLANE_ALIGN_DONE) = 0)
>> + if ((link_align & DPCD_INTERLANE_ALIGN_DONE) = 0)
>> return -EINVAL;
>>
>> for (lane = 0; lane < lane_count; lane++) {
>> - lane_status = exynos_dp_get_lane_status(link_align, lane);
>> + lane_status = exynos_dp_get_lane_status(link_status, lane);
>> lane_status &= DPCD_CHANNEL_EQ_BITS;
>> if (lane_status != DPCD_CHANNEL_EQ_BITS)
>> return -EINVAL;
>> @@ -432,22 +432,47 @@ static void exynos_dp_reduce_link_rate(struct exynos_dp_device *dp)
>> dp->link_train.lt_state = FAILED;
>> }
>>
>> +static void exynos_dp_get_adjust_training_lane(struct exynos_dp_device *dp,
>> + u8 adjust_request[2])
>> +{
>> + int lane, lane_count;
>> + u8 voltage_swing, pre_emphasis, training_lane;
>> +
>> + lane_count = dp->link_train.lane_count;
>> + for (lane = 0; lane < lane_count; lane++) {
>> + voltage_swing = exynos_dp_get_adjust_request_voltage(
>> + adjust_request, lane);
>> + pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
>> + adjust_request, lane);
>> + training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
>> + DPCD_PRE_EMPHASIS_SET(pre_emphasis);
>> +
>> + if (voltage_swing = VOLTAGE_LEVEL_3)
>> + training_lane |= DPCD_MAX_SWING_REACHED;
>> + if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
>> + training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
>> +
>> + dp->link_train.training_lane[lane] = training_lane;
>> + }
>> +}
>> +
>> static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
>> {
>> - u8 link_status[2];
>> int lane, lane_count, retval;
>> -
>> - u8 adjust_request[2];
>> - u8 voltage_swing;
>> - u8 pre_emphasis;
>> - u8 training_lane;
>> + u8 voltage_swing, pre_emphasis, training_lane;
>> + u8 link_status[2], adjust_request[2];
>>
>> usleep_range(100, 101);
>>
>> lane_count = dp->link_train.lane_count;
>>
>> retval = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
>> - 2, link_status);
>> + 2, link_status);
>> + if (retval)
>> + return retval;
>> +
>> + retval = exynos_dp_read_bytes_from_dpcd(dp,
>> + DPCD_ADDR_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
>> if (retval)
>> return retval;
>>
>> @@ -455,43 +480,9 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
>> /* set training pattern 2 for EQ */
>> exynos_dp_set_training_pattern(dp, TRAINING_PTN2);
>>
>> - for (lane = 0; lane < lane_count; lane++) {
>> - retval = exynos_dp_read_bytes_from_dpcd(dp,
>> - DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
>> - 2, adjust_request);
>> - if (retval)
>> - return retval;
>> -
>> - voltage_swing = exynos_dp_get_adjust_request_voltage(
>> - adjust_request, lane);
>> - pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
>> - adjust_request, lane);
>> - training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
>> - DPCD_PRE_EMPHASIS_SET(pre_emphasis);
>> -
>> - if (voltage_swing = VOLTAGE_LEVEL_3)
>> - training_lane |= DPCD_MAX_SWING_REACHED;
>> - if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
>> - training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
>> -
>> - dp->link_train.training_lane[lane] = training_lane;
>> -
>> - exynos_dp_set_lane_link_training(dp,
>> - dp->link_train.training_lane[lane],
>> - lane);
>> - }
>> -
>
> Please don't move it to back.
>
I assume you're talking about the adjust_request read here? I noticed
this was changed in your original clean-up patch
(http://www.spinics.net/lists/linux-fbdev/msg06849.html), but assumed
it was a no-op. What bug does it fix? According to the flowcharts in
the exynos5250 datasheet (figure 49-10 & 49-11), this should be done
*before* setting training pattern 2. Your alteration to my patch will
read it after.
I also noticed that you added back exynos_dp_get_adjust_training_lane
call here, along with setting DPCD_ADDR_TRAINING_LANE0_SET. You'll
notice that this same code is run in the else path of this function.
Hence, I removed the duplication and put it all at the bottom. This
improves readability, matches the flowchart more closely, and removes
duplication.
I'd urge you to please read my patch more carefully and ask questions
if you have any.
Thanks!
Sean
>> retval = exynos_dp_write_byte_to_dpcd(dp,
>> DPCD_ADDR_TRAINING_PATTERN_SET,
>> - DPCD_SCRAMBLING_DISABLED |
>> - DPCD_TRAINING_PATTERN_2);
>> - if (retval)
>> - return retval;
>> -
>> - retval = exynos_dp_write_bytes_to_dpcd(dp,
>> - DPCD_ADDR_TRAINING_LANE0_SET,
>> - lane_count,
>> - dp->link_train.training_lane);
>> + DPCD_SCRAMBLING_DISABLED | DPCD_TRAINING_PATTERN_2);
>> if (retval)
>> return retval;
>>
>> @@ -501,73 +492,49 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
>> for (lane = 0; lane < lane_count; lane++) {
>> training_lane = exynos_dp_get_lane_link_training(
>> dp, lane);
>> - retval = exynos_dp_read_bytes_from_dpcd(dp,
>> - DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
>> - 2, adjust_request);
>> - if (retval)
>> - return retval;
>> -
>> voltage_swing = exynos_dp_get_adjust_request_voltage(
>> adjust_request, lane);
>> pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
>> adjust_request, lane);
>>
>> - if (voltage_swing = VOLTAGE_LEVEL_3 ||
>> - pre_emphasis = PRE_EMPHASIS_LEVEL_3) {
>> - dev_err(dp->dev, "voltage or pre emphasis reached max level\n");
>> - goto reduce_link_rate;
>> - }
>> -
>> - if ((DPCD_VOLTAGE_SWING_GET(training_lane) =
>> - voltage_swing) &&
>> - (DPCD_PRE_EMPHASIS_GET(training_lane) =
>> - pre_emphasis)) {
>> + if (DPCD_VOLTAGE_SWING_GET(training_lane) =
>> + voltage_swing &&
>> + DPCD_PRE_EMPHASIS_GET(training_lane) =
>> + pre_emphasis)
>> dp->link_train.cr_loop[lane]++;
>> - if (dp->link_train.cr_loop[lane] = MAX_CR_LOOP) {
>> - dev_err(dp->dev, "CR Max loop\n");
>> - goto reduce_link_rate;
>> - }
>> - }
>>
>> - training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
>> - DPCD_PRE_EMPHASIS_SET(pre_emphasis);
>> -
>> - if (voltage_swing = VOLTAGE_LEVEL_3)
>> - training_lane |= DPCD_MAX_SWING_REACHED;
>> - if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
>> - training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
>> -
>> - dp->link_train.training_lane[lane] = training_lane;
>> -
>> - exynos_dp_set_lane_link_training(dp,
>> - dp->link_train.training_lane[lane], lane);
>> + if (dp->link_train.cr_loop[lane] = MAX_CR_LOOP ||
>> + voltage_swing = VOLTAGE_LEVEL_3 ||
>> + pre_emphasis = PRE_EMPHASIS_LEVEL_3) {
>> + dev_err(dp->dev, "CR Max reached (%d,%d,%d)\n",
>> + dp->link_train.cr_loop[lane],
>> + voltage_swing, pre_emphasis);
>> + exynos_dp_reduce_link_rate(dp);
>> + return -EIO;
>> + }
>> }
>> + }
>> +
>> + exynos_dp_get_adjust_training_lane(dp, adjust_request);
>>
>> - retval = exynos_dp_write_bytes_to_dpcd(dp,
>> - DPCD_ADDR_TRAINING_LANE0_SET, lane_count,
>> - dp->link_train.training_lane);
>> + for (lane = 0; lane < lane_count; lane++) {
>> + exynos_dp_set_lane_link_training(dp,
>> + dp->link_train.training_lane[lane], lane);
>> + retval = exynos_dp_write_byte_to_dpcd(dp,
>> + DPCD_ADDR_TRAINING_LANE0_SET + lane,
>> + dp->link_train.training_lane[lane]);
>
> The following would be better.
> byte's'_to_dpcd is faster than byte_to_dpcd x 4 times.
>
> for (lane = 0; lane < lane_count; lane++) {
> exynos_dp_set_lane_link_training(dp,
> dp->link_train.training_lane[lane], lane);
> }
>
> retval = exynos_dp_write_bytes_to_dpcd(dp,
> DPCD_ADDR_TRAINING_LANE0_SET, lane_count,
> dp->link_train.training_lane);
>
Makes sense, that's a good change.
>> if (retval)
>> return retval;
>> }
>>
>> return retval;
>> -
>> -reduce_link_rate:
>> - exynos_dp_reduce_link_rate(dp);
>> - return -EIO;
>> }
>>
>> static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
>> {
>> - u8 link_status[2];
>> - u8 link_align[3];
>> int lane, lane_count, retval;
>> u32 reg;
>> -
>> - u8 adjust_request[2];
>> - u8 voltage_swing;
>> - u8 pre_emphasis;
>> - u8 training_lane;
>> + u8 link_align, link_status[2], adjust_request[2];
>>
>> usleep_range(400, 401);
>>
>> @@ -578,85 +545,63 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
>> if (retval)
>> return retval;
>>
>> - if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
>> - link_align[0] = link_status[0];
>> - link_align[1] = link_status[1];
>> -
>> - exynos_dp_read_byte_from_dpcd(dp,
>> - DPCD_ADDR_LANE_ALIGN_STATUS_UPDATED,
>> - &link_align[2]);
>> -
>> - for (lane = 0; lane < lane_count; lane++) {
>> - retval = exynos_dp_read_bytes_from_dpcd(dp,
>> - DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
>> - 2, adjust_request);
>> - if (retval)
>> - return retval;
>> + if (exynos_dp_clock_recovery_ok(link_status, lane_count)) {
>> + exynos_dp_reduce_link_rate(dp);
>> + return -EIO;
>> + }
>>
>> - voltage_swing = exynos_dp_get_adjust_request_voltage(
>> - adjust_request, lane);
>> - pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
>> - adjust_request, lane);
>> - training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
>> - DPCD_PRE_EMPHASIS_SET(pre_emphasis);
>> + retval = exynos_dp_read_bytes_from_dpcd(dp,
>> + DPCD_ADDR_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
>> + if (retval)
>> + return retval;
>>
>> - if (voltage_swing = VOLTAGE_LEVEL_3)
>> - training_lane |= DPCD_MAX_SWING_REACHED;
>> - if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
>> - training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
>> + retval = exynos_dp_read_byte_from_dpcd(dp,
>> + DPCD_ADDR_LANE_ALIGN_STATUS_UPDATED, &link_align);
>> + if (retval)
>> + return retval;
>>
>> - dp->link_train.training_lane[lane] = training_lane;
>> - }
>> + exynos_dp_get_adjust_training_lane(dp, adjust_request);
>>
>> - if (exynos_dp_channel_eq_ok(link_align, lane_count) = 0) {
>> - /* traing pattern Set to Normal */
>> - exynos_dp_training_pattern_dis(dp);
>> + if (!exynos_dp_channel_eq_ok(link_status, link_align, lane_count)) {
>> + /* traing pattern Set to Normal */
>> + exynos_dp_training_pattern_dis(dp);
>>
>> - dev_info(dp->dev, "Link Training success!\n");
>> + dev_info(dp->dev, "Link Training success!\n");
>>
>> - exynos_dp_get_link_bandwidth(dp, ®);
>> - dp->link_train.link_rate = reg;
>> - dev_dbg(dp->dev, "final bandwidth = %.2x\n",
>> - dp->link_train.link_rate);
>> + exynos_dp_get_link_bandwidth(dp, ®);
>> + dp->link_train.link_rate = reg;
>> + dev_dbg(dp->dev, "final bandwidth = %.2x\n",
>> + dp->link_train.link_rate);
>>
>> - exynos_dp_get_lane_count(dp, ®);
>> - dp->link_train.lane_count = reg;
>> - dev_dbg(dp->dev, "final lane count = %.2x\n",
>> - dp->link_train.lane_count);
>> + exynos_dp_get_lane_count(dp, ®);
>> + dp->link_train.lane_count = reg;
>> + dev_dbg(dp->dev, "final lane count = %.2x\n",
>> + dp->link_train.lane_count);
>>
>> - /* set enhanced mode if available */
>> - exynos_dp_set_enhanced_mode(dp);
>> - dp->link_train.lt_state = FINISHED;
>> - } else {
>> - /* not all locked */
>> - dp->link_train.eq_loop++;
>> + /* set enhanced mode if available */
>> + exynos_dp_set_enhanced_mode(dp);
>> + dp->link_train.lt_state = FINISHED;
>>
>> - if (dp->link_train.eq_loop > MAX_EQ_LOOP) {
>> - dev_err(dp->dev, "EQ Max loop\n");
>> - goto reduce_link_rate;
>> - }
>> + return 0;
>> + }
>>
>> - for (lane = 0; lane < lane_count; lane++)
>> - exynos_dp_set_lane_link_training(dp,
>> - dp->link_train.training_lane[lane],
>> - lane);
>> + /* not all locked */
>> + dp->link_train.eq_loop++;
>>
>> - retval = exynos_dp_write_bytes_to_dpcd(dp,
>> - DPCD_ADDR_TRAINING_LANE0_SET,
>> - lane_count,
>> - dp->link_train.training_lane);
>> - if (retval)
>> - return retval;
>> - }
>> - } else {
>> - goto reduce_link_rate;
>> + if (dp->link_train.eq_loop > MAX_EQ_LOOP) {
>> + dev_err(dp->dev, "EQ Max loop\n");
>> + exynos_dp_reduce_link_rate(dp);
>> + return -EIO;
>> }
>>
>> - return 0;
>> + for (lane = 0; lane < lane_count; lane++)
>> + exynos_dp_set_lane_link_training(dp,
>> + dp->link_train.training_lane[lane], lane);
>>
>> -reduce_link_rate:
>> - exynos_dp_reduce_link_rate(dp);
>> - return -EIO;
>> + retval = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_TRAINING_LANE0_SET,
>> + lane_count, dp->link_train.training_lane);
>> +
>> + return retval;
>> }
>>
>> static void exynos_dp_get_max_rx_bandwidth(struct exynos_dp_device *dp,
>> --
>> 1.7.7.3
>
^ permalink raw reply
* Re: [Qemu-devel] [PATCH] add bochs dispi interface framebuffer driver
From: Gerd Hoffmann @ 2012-11-01 13:30 UTC (permalink / raw)
To: Vasilis Liaskovitis; +Cc: linux-fbdev, qemu-devel
In-Reply-To: <20121019103515.GA30341@dhcp-192-168-178-175.profitbricks.localdomain>
On 10/19/12 12:35, Vasilis Liaskovitis wrote:
> Hi,
>
> On Thu, Mar 08, 2012 at 11:13:46AM +0100, Gerd Hoffmann wrote:
>> This patchs adds a frame buffer driver for (virtual/emulated) vga cards
>> implementing the bochs dispi interface. Supported hardware are the
>> bochs vga card with vbe extension and the qemu standard vga.
>>
>> The driver uses a fixed depth of 32bpp. Otherwise it supports the full
>> (but small) feature set of the bochs dispi interface: Resolution
>> switching and display panning. It is tweaked to maximize fbcon speed,
>> so you'll get the comfort of the framebuffer console in kvm guests
>> without performance penalty.
>
> I am testing this driver with qemu-kvm-1.2 or qemu-kvm master (commit)
> and "-std vga". The driver works fine in general.
>
> When I test a guest that runs X (ubuntu-12.04 desktop amd64), sometimes parts of
> the screen and keyboard input is mixed between the X terminal and fbconsole
> terminals. This happens only on the initial X11 login (right after boot or
> reboot) and only sometimes.
Only with bochsfb or with vesafb (+ fbdev xorg driver) too?
> Xorg driver used is fbdev (i can send xorg log), not sure if another driver
> should be used/implemented for the bochsfb.
Yes, that one is fine.
> CONFIG_FB_BOCHS=m
> CONFIG_FB_VESA=y
> # CONFIG_FB_EFI is not set
>
> Should FB_VESA be turned to "not set" for this test? (it's not tristate in Kconfig)
>
> Btw (slightly off-topic) are other framebuffer drivers suitable for the
> standard qemu vga-pci device? Would vesafb or uvesafb work?
Never tried uvesafb. vesafb will work too, but run with a fixed
resolution. bochsfb allows you to change the display resolution at
runtime using fbset. fbcon is faster too because bochsfb supports
display panning.
Latest version of the patch is here:
http://www.kraxel.org/cgit/linux/commit/?h=bochsfb
cheers,
Gerd
^ permalink raw reply
* [PATCH v3 8/8] video: exynos_dp: Enable hotplug interrupts
From: Jingoo Han @ 2012-11-01 10:32 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <012601cdb81b$b9906c10$2cb14430$%han@samsung.com>
From: Sean Paul <seanpaul@chromium.org>
Enable hotplug interrupts and move the hotplug scheduling into the
interrupt handler. This allows us to introduce a screen at any time
while we're running.
[jg1.han@samsung.com: moved the bit masking of hotplug interrupts]
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
Added 'From: Sean Paul <seanpaul@chromium.org>'
drivers/video/exynos/exynos_dp_core.c | 38 ++++++++++++++++++++++++++-------
drivers/video/exynos/exynos_dp_core.h | 9 +++++++
drivers/video/exynos/exynos_dp_reg.c | 38 +++++++++++++++++++++++++++-----
3 files changed, 71 insertions(+), 14 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index ffb6d6d..963e3c7 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -48,10 +48,6 @@ static int exynos_dp_detect_hpd(struct exynos_dp_device *dp)
{
int timeout_loop = 0;
- exynos_dp_init_hpd(dp);
-
- usleep_range(200, 210);
-
while (exynos_dp_get_plug_in_status(dp) != 0) {
timeout_loop++;
if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
@@ -870,7 +866,32 @@ static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
{
struct exynos_dp_device *dp = arg;
- dev_err(dp->dev, "exynos_dp_irq_handler\n");
+ enum dp_irq_type irq_type;
+
+ irq_type = exynos_dp_get_irq_type(dp);
+ switch (irq_type) {
+ case DP_IRQ_TYPE_HP_CABLE_IN:
+ dev_dbg(dp->dev, "Received irq - cable in\n");
+ schedule_work(&dp->hotplug_work);
+ exynos_dp_clear_hotplug_interrupts(dp);
+ break;
+ case DP_IRQ_TYPE_HP_CABLE_OUT:
+ dev_dbg(dp->dev, "Received irq - cable out\n");
+ exynos_dp_clear_hotplug_interrupts(dp);
+ break;
+ case DP_IRQ_TYPE_HP_CHANGE:
+ /*
+ * We get these change notifications once in a while, but there
+ * is nothing we can do with them. Just ignore it for now and
+ * only handle cable changes.
+ */
+ dev_dbg(dp->dev, "Received irq - hotplug change; ignoring.\n");
+ exynos_dp_clear_hotplug_interrupts(dp);
+ break;
+ default:
+ dev_err(dp->dev, "Received irq - unknown type!\n");
+ break;
+ }
return IRQ_HANDLED;
}
@@ -883,7 +904,7 @@ static void exynos_dp_hotplug(struct work_struct *work)
ret = exynos_dp_detect_hpd(dp);
if (ret) {
- dev_err(dp->dev, "unable to detect hpd\n");
+ /* Cable has been disconnected, we're done */
return;
}
@@ -974,7 +995,6 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
exynos_dp_init_dp(dp);
platform_set_drvdata(pdev, dp);
- schedule_work(&dp->hotplug_work);
return 0;
}
@@ -1002,6 +1022,8 @@ static int exynos_dp_suspend(struct device *dev)
struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
struct exynos_dp_device *dp = platform_get_drvdata(pdev);
+ disable_irq(dp->irq);
+
if (work_pending(&dp->hotplug_work))
flush_work(&dp->hotplug_work);
@@ -1026,7 +1048,7 @@ static int exynos_dp_resume(struct device *dev)
exynos_dp_init_dp(dp);
- schedule_work(&dp->hotplug_work);
+ enable_irq(dp->irq);
return 0;
}
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 66f11f4..520486e 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -13,6 +13,13 @@
#ifndef _EXYNOS_DP_CORE_H
#define _EXYNOS_DP_CORE_H
+enum dp_irq_type {
+ DP_IRQ_TYPE_HP_CABLE_IN,
+ DP_IRQ_TYPE_HP_CABLE_OUT,
+ DP_IRQ_TYPE_HP_CHANGE,
+ DP_IRQ_TYPE_UNKNOWN,
+};
+
struct link_train {
int eq_loop;
int cr_loop[4];
@@ -51,6 +58,8 @@ void exynos_dp_set_analog_power_down(struct exynos_dp_device *dp,
bool enable);
void exynos_dp_init_analog_func(struct exynos_dp_device *dp);
void exynos_dp_init_hpd(struct exynos_dp_device *dp);
+enum dp_irq_type exynos_dp_get_irq_type(struct exynos_dp_device *dp);
+void exynos_dp_clear_hotplug_interrupts(struct exynos_dp_device *dp);
void exynos_dp_reset_aux(struct exynos_dp_device *dp);
void exynos_dp_init_aux(struct exynos_dp_device *dp);
int exynos_dp_get_plug_in_status(struct exynos_dp_device *dp);
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index dc09278..af7dae1 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -19,11 +19,11 @@
#include "exynos_dp_core.h"
#include "exynos_dp_reg.h"
-#define COMMON_INT_MASK_1 (0)
-#define COMMON_INT_MASK_2 (0)
-#define COMMON_INT_MASK_3 (0)
-#define COMMON_INT_MASK_4 (0)
-#define INT_STA_MASK (0)
+#define COMMON_INT_MASK_1 0
+#define COMMON_INT_MASK_2 0
+#define COMMON_INT_MASK_3 0
+#define COMMON_INT_MASK_4 (HOTPLUG_CHG | HPD_LOST | PLUG)
+#define INT_STA_MASK INT_HPD
void exynos_dp_enable_video_mute(struct exynos_dp_device *dp, bool enable)
{
@@ -324,7 +324,7 @@ void exynos_dp_init_analog_func(struct exynos_dp_device *dp)
writel(reg, dp->reg_base + EXYNOS_DP_FUNC_EN_2);
}
-void exynos_dp_init_hpd(struct exynos_dp_device *dp)
+void exynos_dp_clear_hotplug_interrupts(struct exynos_dp_device *dp)
{
u32 reg;
@@ -333,12 +333,38 @@ void exynos_dp_init_hpd(struct exynos_dp_device *dp)
reg = INT_HPD;
writel(reg, dp->reg_base + EXYNOS_DP_INT_STA);
+}
+
+void exynos_dp_init_hpd(struct exynos_dp_device *dp)
+{
+ u32 reg;
+
+ exynos_dp_clear_hotplug_interrupts(dp);
reg = readl(dp->reg_base + EXYNOS_DP_SYS_CTL_3);
reg &= ~(F_HPD | HPD_CTRL);
writel(reg, dp->reg_base + EXYNOS_DP_SYS_CTL_3);
}
+enum dp_irq_type exynos_dp_get_irq_type(struct exynos_dp_device *dp)
+{
+ u32 reg;
+
+ /* Parse hotplug interrupt status register */
+ reg = readl(dp->reg_base + EXYNOS_DP_COMMON_INT_STA_4);
+
+ if (reg & PLUG)
+ return DP_IRQ_TYPE_HP_CABLE_IN;
+
+ if (reg & HPD_LOST)
+ return DP_IRQ_TYPE_HP_CABLE_OUT;
+
+ if (reg & HOTPLUG_CHG)
+ return DP_IRQ_TYPE_HP_CHANGE;
+
+ return DP_IRQ_TYPE_UNKNOWN;
+}
+
void exynos_dp_reset_aux(struct exynos_dp_device *dp)
{
u32 reg;
--
1.7.1
^ permalink raw reply related
* [PATCH v3 4/8] video: exynos_dp: Improve EDID error handling
From: Jingoo Han @ 2012-11-01 10:30 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <012001cdb81a$b87fb4d0$297f1e70$%han@samsung.com>
From: Sean Paul <seanpaul@chromium.org>
EDID error handling has 2 problems:
- It doesn't fail as early as it can
- The retry counts for i2c and aux transactions are huge
This patch fails if the initial i2c transaction fails, and reduces the
aux and i2c retry counts down to 3.
[jg1.han@samsung.com: reduced the retry count of exynos_dp_read_byte_from_dpcd()]
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
Added 'From: Sean Paul <seanpaul@chromium.org>'
drivers/video/exynos/exynos_dp_core.c | 13 ++++++++-----
drivers/video/exynos/exynos_dp_reg.c | 11 +++++------
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 878b4b6..ee957e0 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -90,9 +90,11 @@ static int exynos_dp_read_edid(struct exynos_dp_device *dp)
*/
/* Read Extension Flag, Number of 128-byte EDID extension blocks */
- exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
+ retval = exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
EDID_EXTENSION_FLAG,
&extend_block);
+ if (retval)
+ return retval;
if (extend_block > 0) {
dev_dbg(dp->dev, "EDID data includes a single extension!\n");
@@ -181,14 +183,15 @@ static int exynos_dp_handle_edid(struct exynos_dp_device *dp)
int retval;
/* Read DPCD DPCD_ADDR_DPCD_REV~RECEIVE_PORT1_CAP_1 */
- exynos_dp_read_bytes_from_dpcd(dp,
- DPCD_ADDR_DPCD_REV,
- 12, buf);
+ retval = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_DPCD_REV,
+ 12, buf);
+ if (retval)
+ return retval;
/* Read EDID */
for (i = 0; i < 3; i++) {
retval = exynos_dp_read_edid(dp);
- if (retval = 0)
+ if (!retval)
break;
}
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 3f5ca8a..dc09278 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -491,7 +491,7 @@ int exynos_dp_read_byte_from_dpcd(struct exynos_dp_device *dp,
int i;
int retval;
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Clear AUX CH data buffer */
reg = BUF_CLR;
writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
@@ -552,7 +552,7 @@ int exynos_dp_write_bytes_to_dpcd(struct exynos_dp_device *dp,
else
cur_data_count = count - start_offset;
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Select DPCD device address */
reg = AUX_ADDR_7_0(reg_addr + start_offset);
writel(reg, dp->reg_base + EXYNOS_DP_AUX_ADDR_7_0);
@@ -617,7 +617,7 @@ int exynos_dp_read_bytes_from_dpcd(struct exynos_dp_device *dp,
cur_data_count = count - start_offset;
/* AUX CH Request Transaction process */
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Select DPCD device address */
reg = AUX_ADDR_7_0(reg_addr + start_offset);
writel(reg, dp->reg_base + EXYNOS_DP_AUX_ADDR_7_0);
@@ -700,7 +700,7 @@ int exynos_dp_read_byte_from_i2c(struct exynos_dp_device *dp,
int i;
int retval;
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Clear AUX CH data buffer */
reg = BUF_CLR;
writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
@@ -708,7 +708,6 @@ int exynos_dp_read_byte_from_i2c(struct exynos_dp_device *dp,
/* Select EDID device */
retval = exynos_dp_select_i2c_device(dp, device_addr, reg_addr);
if (retval != 0) {
- dev_err(dp->dev, "Select EDID device fail!\n");
continue;
}
@@ -750,7 +749,7 @@ int exynos_dp_read_bytes_from_i2c(struct exynos_dp_device *dp,
int retval = 0;
for (i = 0; i < count; i += 16) {
- for (j = 0; j < 100; j++) {
+ for (j = 0; j < 3; j++) {
/* Clear AUX CH data buffer */
reg = BUF_CLR;
writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
--
1.7.1
^ permalink raw reply related
* [PATCH v3 8/8] video: exynos_dp: Enable hotplug interrupts
From: Jingoo Han @ 2012-11-01 10:29 UTC (permalink / raw)
To: linux-fbdev
Enable hotplug interrupts and move the hotplug scheduling into the
interrupt handler. This allows us to introduce a screen at any time
while we're running.
[jg1.han@samsung.com: moved the bit masking of hotplug interrupts]
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 38 ++++++++++++++++++++++++++-------
drivers/video/exynos/exynos_dp_core.h | 9 +++++++
drivers/video/exynos/exynos_dp_reg.c | 38 +++++++++++++++++++++++++++-----
3 files changed, 71 insertions(+), 14 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index ffb6d6d..963e3c7 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -48,10 +48,6 @@ static int exynos_dp_detect_hpd(struct exynos_dp_device *dp)
{
int timeout_loop = 0;
- exynos_dp_init_hpd(dp);
-
- usleep_range(200, 210);
-
while (exynos_dp_get_plug_in_status(dp) != 0) {
timeout_loop++;
if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
@@ -870,7 +866,32 @@ static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
{
struct exynos_dp_device *dp = arg;
- dev_err(dp->dev, "exynos_dp_irq_handler\n");
+ enum dp_irq_type irq_type;
+
+ irq_type = exynos_dp_get_irq_type(dp);
+ switch (irq_type) {
+ case DP_IRQ_TYPE_HP_CABLE_IN:
+ dev_dbg(dp->dev, "Received irq - cable in\n");
+ schedule_work(&dp->hotplug_work);
+ exynos_dp_clear_hotplug_interrupts(dp);
+ break;
+ case DP_IRQ_TYPE_HP_CABLE_OUT:
+ dev_dbg(dp->dev, "Received irq - cable out\n");
+ exynos_dp_clear_hotplug_interrupts(dp);
+ break;
+ case DP_IRQ_TYPE_HP_CHANGE:
+ /*
+ * We get these change notifications once in a while, but there
+ * is nothing we can do with them. Just ignore it for now and
+ * only handle cable changes.
+ */
+ dev_dbg(dp->dev, "Received irq - hotplug change; ignoring.\n");
+ exynos_dp_clear_hotplug_interrupts(dp);
+ break;
+ default:
+ dev_err(dp->dev, "Received irq - unknown type!\n");
+ break;
+ }
return IRQ_HANDLED;
}
@@ -883,7 +904,7 @@ static void exynos_dp_hotplug(struct work_struct *work)
ret = exynos_dp_detect_hpd(dp);
if (ret) {
- dev_err(dp->dev, "unable to detect hpd\n");
+ /* Cable has been disconnected, we're done */
return;
}
@@ -974,7 +995,6 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
exynos_dp_init_dp(dp);
platform_set_drvdata(pdev, dp);
- schedule_work(&dp->hotplug_work);
return 0;
}
@@ -1002,6 +1022,8 @@ static int exynos_dp_suspend(struct device *dev)
struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
struct exynos_dp_device *dp = platform_get_drvdata(pdev);
+ disable_irq(dp->irq);
+
if (work_pending(&dp->hotplug_work))
flush_work(&dp->hotplug_work);
@@ -1026,7 +1048,7 @@ static int exynos_dp_resume(struct device *dev)
exynos_dp_init_dp(dp);
- schedule_work(&dp->hotplug_work);
+ enable_irq(dp->irq);
return 0;
}
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 66f11f4..520486e 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -13,6 +13,13 @@
#ifndef _EXYNOS_DP_CORE_H
#define _EXYNOS_DP_CORE_H
+enum dp_irq_type {
+ DP_IRQ_TYPE_HP_CABLE_IN,
+ DP_IRQ_TYPE_HP_CABLE_OUT,
+ DP_IRQ_TYPE_HP_CHANGE,
+ DP_IRQ_TYPE_UNKNOWN,
+};
+
struct link_train {
int eq_loop;
int cr_loop[4];
@@ -51,6 +58,8 @@ void exynos_dp_set_analog_power_down(struct exynos_dp_device *dp,
bool enable);
void exynos_dp_init_analog_func(struct exynos_dp_device *dp);
void exynos_dp_init_hpd(struct exynos_dp_device *dp);
+enum dp_irq_type exynos_dp_get_irq_type(struct exynos_dp_device *dp);
+void exynos_dp_clear_hotplug_interrupts(struct exynos_dp_device *dp);
void exynos_dp_reset_aux(struct exynos_dp_device *dp);
void exynos_dp_init_aux(struct exynos_dp_device *dp);
int exynos_dp_get_plug_in_status(struct exynos_dp_device *dp);
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index dc09278..af7dae1 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -19,11 +19,11 @@
#include "exynos_dp_core.h"
#include "exynos_dp_reg.h"
-#define COMMON_INT_MASK_1 (0)
-#define COMMON_INT_MASK_2 (0)
-#define COMMON_INT_MASK_3 (0)
-#define COMMON_INT_MASK_4 (0)
-#define INT_STA_MASK (0)
+#define COMMON_INT_MASK_1 0
+#define COMMON_INT_MASK_2 0
+#define COMMON_INT_MASK_3 0
+#define COMMON_INT_MASK_4 (HOTPLUG_CHG | HPD_LOST | PLUG)
+#define INT_STA_MASK INT_HPD
void exynos_dp_enable_video_mute(struct exynos_dp_device *dp, bool enable)
{
@@ -324,7 +324,7 @@ void exynos_dp_init_analog_func(struct exynos_dp_device *dp)
writel(reg, dp->reg_base + EXYNOS_DP_FUNC_EN_2);
}
-void exynos_dp_init_hpd(struct exynos_dp_device *dp)
+void exynos_dp_clear_hotplug_interrupts(struct exynos_dp_device *dp)
{
u32 reg;
@@ -333,12 +333,38 @@ void exynos_dp_init_hpd(struct exynos_dp_device *dp)
reg = INT_HPD;
writel(reg, dp->reg_base + EXYNOS_DP_INT_STA);
+}
+
+void exynos_dp_init_hpd(struct exynos_dp_device *dp)
+{
+ u32 reg;
+
+ exynos_dp_clear_hotplug_interrupts(dp);
reg = readl(dp->reg_base + EXYNOS_DP_SYS_CTL_3);
reg &= ~(F_HPD | HPD_CTRL);
writel(reg, dp->reg_base + EXYNOS_DP_SYS_CTL_3);
}
+enum dp_irq_type exynos_dp_get_irq_type(struct exynos_dp_device *dp)
+{
+ u32 reg;
+
+ /* Parse hotplug interrupt status register */
+ reg = readl(dp->reg_base + EXYNOS_DP_COMMON_INT_STA_4);
+
+ if (reg & PLUG)
+ return DP_IRQ_TYPE_HP_CABLE_IN;
+
+ if (reg & HPD_LOST)
+ return DP_IRQ_TYPE_HP_CABLE_OUT;
+
+ if (reg & HOTPLUG_CHG)
+ return DP_IRQ_TYPE_HP_CHANGE;
+
+ return DP_IRQ_TYPE_UNKNOWN;
+}
+
void exynos_dp_reset_aux(struct exynos_dp_device *dp)
{
u32 reg;
--
1.7.1
^ permalink raw reply related
* [PATCH v3 7/8] video: exynos_dp: Move hotplug into a workqueue
From: Jingoo Han @ 2012-11-01 10:28 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <012301cdb81b$09cf70f0$1d6e52d0$%han@samsung.com>
From: Sean Paul <seanpaul@chromium.org>
Move the hotplug related code from probe and resume into a workqueue.
This allows us to initialize the DP driver (and resume it) when there
is no monitor connected.
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
Add 'From: Sean Paul <seanpaul@chromium.org>'
drivers/video/exynos/exynos_dp_core.c | 93 +++++++++++++++++---------------
drivers/video/exynos/exynos_dp_core.h | 1 +
2 files changed, 50 insertions(+), 44 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index c2b6bf2..ffb6d6d 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -874,6 +874,45 @@ static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
return IRQ_HANDLED;
}
+static void exynos_dp_hotplug(struct work_struct *work)
+{
+ struct exynos_dp_device *dp;
+ int ret;
+
+ dp = container_of(work, struct exynos_dp_device, hotplug_work);
+
+ ret = exynos_dp_detect_hpd(dp);
+ if (ret) {
+ dev_err(dp->dev, "unable to detect hpd\n");
+ return;
+ }
+
+ ret = exynos_dp_handle_edid(dp);
+ if (ret) {
+ dev_err(dp->dev, "unable to handle edid\n");
+ return;
+ }
+
+ ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
+ dp->video_info->link_rate);
+ if (ret) {
+ dev_err(dp->dev, "unable to do link train\n");
+ return;
+ }
+
+ exynos_dp_enable_scramble(dp, 1);
+ exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
+ exynos_dp_enable_enhanced_mode(dp, 1);
+
+ exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
+ exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
+
+ exynos_dp_init_video(dp);
+ ret = exynos_dp_config_video(dp, dp->video_info);
+ if (ret)
+ dev_err(dp->dev, "unable to config video\n");
+}
+
static int __devinit exynos_dp_probe(struct platform_device *pdev)
{
struct resource *res;
@@ -919,6 +958,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
return -ENODEV;
}
+ INIT_WORK(&dp->hotplug_work, exynos_dp_hotplug);
+
ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
"exynos-dp", dp);
if (ret) {
@@ -932,36 +973,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
exynos_dp_init_dp(dp);
- ret = exynos_dp_detect_hpd(dp);
- if (ret) {
- dev_err(&pdev->dev, "unable to detect hpd\n");
- return ret;
- }
-
- exynos_dp_handle_edid(dp);
-
- ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
- dp->video_info->link_rate);
- if (ret) {
- dev_err(&pdev->dev, "unable to do link train\n");
- return ret;
- }
-
- exynos_dp_enable_scramble(dp, 1);
- exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
- exynos_dp_enable_enhanced_mode(dp, 1);
-
- exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
- exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
-
- exynos_dp_init_video(dp);
- ret = exynos_dp_config_video(dp, dp->video_info);
- if (ret) {
- dev_err(&pdev->dev, "unable to config video\n");
- return ret;
- }
-
platform_set_drvdata(pdev, dp);
+ schedule_work(&dp->hotplug_work);
return 0;
}
@@ -971,6 +984,9 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
struct exynos_dp_device *dp = platform_get_drvdata(pdev);
+ if (work_pending(&dp->hotplug_work))
+ flush_work(&dp->hotplug_work);
+
if (pdata && pdata->phy_exit)
pdata->phy_exit();
@@ -986,6 +1002,9 @@ static int exynos_dp_suspend(struct device *dev)
struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
struct exynos_dp_device *dp = platform_get_drvdata(pdev);
+ if (work_pending(&dp->hotplug_work))
+ flush_work(&dp->hotplug_work);
+
if (pdata && pdata->phy_exit)
pdata->phy_exit();
@@ -1007,21 +1026,7 @@ static int exynos_dp_resume(struct device *dev)
exynos_dp_init_dp(dp);
- exynos_dp_detect_hpd(dp);
- exynos_dp_handle_edid(dp);
-
- exynos_dp_set_link_train(dp, dp->video_info->lane_count,
- dp->video_info->link_rate);
-
- exynos_dp_enable_scramble(dp, 1);
- exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
- exynos_dp_enable_enhanced_mode(dp, 1);
-
- exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
- exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
-
- exynos_dp_init_video(dp);
- exynos_dp_config_video(dp, dp->video_info);
+ schedule_work(&dp->hotplug_work);
return 0;
}
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 57b8a65..66f11f4 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -32,6 +32,7 @@ struct exynos_dp_device {
struct video_info *video_info;
struct link_train link_train;
+ struct work_struct hotplug_work;
};
/* exynos_dp_reg.c */
--
1.7.1
^ permalink raw reply related
* [PATCH v3 6/8] video: exynos_dp: Remove sink control to D0
From: Jingoo Han @ 2012-11-01 10:26 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <012201cdb81a$f3117340$d93459c0$%han@samsung.com>
From: Sean Paul <seanpaul@chromium.org>
According to DP spec, it is not required in the Link Training
procedure.
[jg1.han@samsung.com: modified the commit message]
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
Add 'From: Sean Paul <seanpaul@chromium.org>'
drivers/video/exynos/exynos_dp_core.c | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 8ab95c3..c2b6bf2 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -277,12 +277,6 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
for (lane = 0; lane < lane_count; lane++)
dp->link_train.cr_loop[lane] = 0;
- /* Set sink to D0 (Sink Not Ready) mode. */
- retval = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
- DPCD_SET_POWER_STATE_D0);
- if (retval)
- return retval;
-
/* Set link rate and count as you want to establish*/
exynos_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
exynos_dp_set_lane_count(dp, dp->link_train.lane_count);
--
1.7.1
^ permalink raw reply related
* [PATCH v3 7/8] video: exynos_dp: Move hotplug into a workqueue
From: Jingoo Han @ 2012-11-01 10:24 UTC (permalink / raw)
To: linux-fbdev
Move the hotplug related code from probe and resume into a workqueue.
This allows us to initialize the DP driver (and resume it) when there
is no monitor connected.
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 93 +++++++++++++++++---------------
drivers/video/exynos/exynos_dp_core.h | 1 +
2 files changed, 50 insertions(+), 44 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index c2b6bf2..ffb6d6d 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -874,6 +874,45 @@ static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
return IRQ_HANDLED;
}
+static void exynos_dp_hotplug(struct work_struct *work)
+{
+ struct exynos_dp_device *dp;
+ int ret;
+
+ dp = container_of(work, struct exynos_dp_device, hotplug_work);
+
+ ret = exynos_dp_detect_hpd(dp);
+ if (ret) {
+ dev_err(dp->dev, "unable to detect hpd\n");
+ return;
+ }
+
+ ret = exynos_dp_handle_edid(dp);
+ if (ret) {
+ dev_err(dp->dev, "unable to handle edid\n");
+ return;
+ }
+
+ ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
+ dp->video_info->link_rate);
+ if (ret) {
+ dev_err(dp->dev, "unable to do link train\n");
+ return;
+ }
+
+ exynos_dp_enable_scramble(dp, 1);
+ exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
+ exynos_dp_enable_enhanced_mode(dp, 1);
+
+ exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
+ exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
+
+ exynos_dp_init_video(dp);
+ ret = exynos_dp_config_video(dp, dp->video_info);
+ if (ret)
+ dev_err(dp->dev, "unable to config video\n");
+}
+
static int __devinit exynos_dp_probe(struct platform_device *pdev)
{
struct resource *res;
@@ -919,6 +958,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
return -ENODEV;
}
+ INIT_WORK(&dp->hotplug_work, exynos_dp_hotplug);
+
ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
"exynos-dp", dp);
if (ret) {
@@ -932,36 +973,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
exynos_dp_init_dp(dp);
- ret = exynos_dp_detect_hpd(dp);
- if (ret) {
- dev_err(&pdev->dev, "unable to detect hpd\n");
- return ret;
- }
-
- exynos_dp_handle_edid(dp);
-
- ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
- dp->video_info->link_rate);
- if (ret) {
- dev_err(&pdev->dev, "unable to do link train\n");
- return ret;
- }
-
- exynos_dp_enable_scramble(dp, 1);
- exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
- exynos_dp_enable_enhanced_mode(dp, 1);
-
- exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
- exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
-
- exynos_dp_init_video(dp);
- ret = exynos_dp_config_video(dp, dp->video_info);
- if (ret) {
- dev_err(&pdev->dev, "unable to config video\n");
- return ret;
- }
-
platform_set_drvdata(pdev, dp);
+ schedule_work(&dp->hotplug_work);
return 0;
}
@@ -971,6 +984,9 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
struct exynos_dp_device *dp = platform_get_drvdata(pdev);
+ if (work_pending(&dp->hotplug_work))
+ flush_work(&dp->hotplug_work);
+
if (pdata && pdata->phy_exit)
pdata->phy_exit();
@@ -986,6 +1002,9 @@ static int exynos_dp_suspend(struct device *dev)
struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
struct exynos_dp_device *dp = platform_get_drvdata(pdev);
+ if (work_pending(&dp->hotplug_work))
+ flush_work(&dp->hotplug_work);
+
if (pdata && pdata->phy_exit)
pdata->phy_exit();
@@ -1007,21 +1026,7 @@ static int exynos_dp_resume(struct device *dev)
exynos_dp_init_dp(dp);
- exynos_dp_detect_hpd(dp);
- exynos_dp_handle_edid(dp);
-
- exynos_dp_set_link_train(dp, dp->video_info->lane_count,
- dp->video_info->link_rate);
-
- exynos_dp_enable_scramble(dp, 1);
- exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
- exynos_dp_enable_enhanced_mode(dp, 1);
-
- exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
- exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
-
- exynos_dp_init_video(dp);
- exynos_dp_config_video(dp, dp->video_info);
+ schedule_work(&dp->hotplug_work);
return 0;
}
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 57b8a65..66f11f4 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -32,6 +32,7 @@ struct exynos_dp_device {
struct video_info *video_info;
struct link_train link_train;
+ struct work_struct hotplug_work;
};
/* exynos_dp_reg.c */
--
1.7.1
^ permalink raw reply related
* [PATCH v3 6/8] video: exynos_dp: Remove sink control to D0
From: Jingoo Han @ 2012-11-01 10:23 UTC (permalink / raw)
To: linux-fbdev
According to DP spec, it is not required in the Link Training
procedure.
[jg1.han@samsung.com: modified the commit message]
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 8ab95c3..c2b6bf2 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -277,12 +277,6 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
for (lane = 0; lane < lane_count; lane++)
dp->link_train.cr_loop[lane] = 0;
- /* Set sink to D0 (Sink Not Ready) mode. */
- retval = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
- DPCD_SET_POWER_STATE_D0);
- if (retval)
- return retval;
-
/* Set link rate and count as you want to establish*/
exynos_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
exynos_dp_set_lane_count(dp, dp->link_train.lane_count);
--
1.7.1
^ permalink raw reply related
* [PATCH v3 5/8] video: exynos_dp: Fix bug when checking dp->irq
From: Jingoo Han @ 2012-11-01 10:22 UTC (permalink / raw)
To: linux-fbdev
From: Sean Paul <seanpaul@chromium.org>
Fix a bug where we check !dp->irq instead of the correct check for
-ENXIO.
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index ee957e0..8ab95c3 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -920,7 +920,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
}
dp->irq = platform_get_irq(pdev, 0);
- if (!dp->irq) {
+ if (dp->irq = -ENXIO) {
dev_err(&pdev->dev, "failed to get irq\n");
return -ENODEV;
}
--
1.7.1
^ permalink raw reply related
* [PATCH v3 4/8] video: exynos_dp: Improve EDID error handling
From: Jingoo Han @ 2012-11-01 10:21 UTC (permalink / raw)
To: linux-fbdev
EDID error handling has 2 problems:
- It doesn't fail as early as it can
- The retry counts for i2c and aux transactions are huge
This patch fails if the initial i2c transaction fails, and reduces the
aux and i2c retry counts down to 3.
[jg1.han@samsung.com: reduced the retry count of exynos_dp_read_byte_from_dpcd()]
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 13 ++++++++-----
drivers/video/exynos/exynos_dp_reg.c | 11 +++++------
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 878b4b6..ee957e0 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -90,9 +90,11 @@ static int exynos_dp_read_edid(struct exynos_dp_device *dp)
*/
/* Read Extension Flag, Number of 128-byte EDID extension blocks */
- exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
+ retval = exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
EDID_EXTENSION_FLAG,
&extend_block);
+ if (retval)
+ return retval;
if (extend_block > 0) {
dev_dbg(dp->dev, "EDID data includes a single extension!\n");
@@ -181,14 +183,15 @@ static int exynos_dp_handle_edid(struct exynos_dp_device *dp)
int retval;
/* Read DPCD DPCD_ADDR_DPCD_REV~RECEIVE_PORT1_CAP_1 */
- exynos_dp_read_bytes_from_dpcd(dp,
- DPCD_ADDR_DPCD_REV,
- 12, buf);
+ retval = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_DPCD_REV,
+ 12, buf);
+ if (retval)
+ return retval;
/* Read EDID */
for (i = 0; i < 3; i++) {
retval = exynos_dp_read_edid(dp);
- if (retval = 0)
+ if (!retval)
break;
}
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 3f5ca8a..dc09278 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -491,7 +491,7 @@ int exynos_dp_read_byte_from_dpcd(struct exynos_dp_device *dp,
int i;
int retval;
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Clear AUX CH data buffer */
reg = BUF_CLR;
writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
@@ -552,7 +552,7 @@ int exynos_dp_write_bytes_to_dpcd(struct exynos_dp_device *dp,
else
cur_data_count = count - start_offset;
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Select DPCD device address */
reg = AUX_ADDR_7_0(reg_addr + start_offset);
writel(reg, dp->reg_base + EXYNOS_DP_AUX_ADDR_7_0);
@@ -617,7 +617,7 @@ int exynos_dp_read_bytes_from_dpcd(struct exynos_dp_device *dp,
cur_data_count = count - start_offset;
/* AUX CH Request Transaction process */
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Select DPCD device address */
reg = AUX_ADDR_7_0(reg_addr + start_offset);
writel(reg, dp->reg_base + EXYNOS_DP_AUX_ADDR_7_0);
@@ -700,7 +700,7 @@ int exynos_dp_read_byte_from_i2c(struct exynos_dp_device *dp,
int i;
int retval;
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Clear AUX CH data buffer */
reg = BUF_CLR;
writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
@@ -708,7 +708,6 @@ int exynos_dp_read_byte_from_i2c(struct exynos_dp_device *dp,
/* Select EDID device */
retval = exynos_dp_select_i2c_device(dp, device_addr, reg_addr);
if (retval != 0) {
- dev_err(dp->dev, "Select EDID device fail!\n");
continue;
}
@@ -750,7 +749,7 @@ int exynos_dp_read_bytes_from_i2c(struct exynos_dp_device *dp,
int retval = 0;
for (i = 0; i < count; i += 16) {
- for (j = 0; j < 100; j++) {
+ for (j = 0; j < 3; j++) {
/* Clear AUX CH data buffer */
reg = BUF_CLR;
writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
--
1.7.1
^ permalink raw reply related
* [PATCH v3 3/8] video: exynos_dp: Get pll lock before pattern set
From: Jingoo Han @ 2012-11-01 10:21 UTC (permalink / raw)
To: linux-fbdev
From: Sean Paul <seanpaul@chromium.org>
According to the exynos datasheet (Figure 49-10), we should wait for PLL
lock before programming the training pattern when doing software eDP
link training.
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 13bb10d..878b4b6 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -264,7 +264,7 @@ static void exynos_dp_set_lane_lane_pre_emphasis(struct exynos_dp_device *dp,
static int exynos_dp_link_start(struct exynos_dp_device *dp)
{
u8 buf[4];
- int lane, lane_count, retval;
+ int lane, lane_count, pll_tries, retval;
lane_count = dp->link_train.lane_count;
@@ -297,6 +297,18 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
exynos_dp_set_lane_lane_pre_emphasis(dp,
PRE_EMPHASIS_LEVEL_0, lane);
+ /* Wait for PLL lock */
+ pll_tries = 0;
+ while (exynos_dp_get_pll_lock_status(dp) = PLL_UNLOCKED) {
+ if (pll_tries = DP_TIMEOUT_LOOP_COUNT) {
+ dev_err(dp->dev, "Wait for PLL lock timed out\n");
+ return -ETIMEDOUT;
+ }
+
+ pll_tries++;
+ usleep_range(90, 120);
+ }
+
/* Set training pattern 1 */
exynos_dp_set_training_pattern(dp, TRAINING_PTN1);
--
1.7.1
^ permalink raw reply related
* [PATCH v3 2/8] video: exynos_dp: Clean up SW link training
From: Jingoo Han @ 2012-11-01 10:19 UTC (permalink / raw)
To: linux-fbdev
From: Sean Paul <seanpaul@chromium.org>
Clean up some of the SW training code to make it more clear and reduce
duplicate code.
[jg1.han@samsung.com: modify the procedure of clock recovery]
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 251 +++++++++++++++------------------
1 files changed, 117 insertions(+), 134 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 44820f2..13bb10d 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -276,7 +276,7 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
/* Set sink to D0 (Sink Not Ready) mode. */
retval = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
- DPCD_SET_POWER_STATE_D0);
+ DPCD_SET_POWER_STATE_D0);
if (retval)
return retval;
@@ -301,17 +301,18 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
exynos_dp_set_training_pattern(dp, TRAINING_PTN1);
/* Set RX training pattern */
- exynos_dp_write_byte_to_dpcd(dp,
- DPCD_ADDR_TRAINING_PATTERN_SET,
- DPCD_SCRAMBLING_DISABLED |
- DPCD_TRAINING_PATTERN_1);
+ retval = exynos_dp_write_byte_to_dpcd(dp,
+ DPCD_ADDR_TRAINING_PATTERN_SET,
+ DPCD_SCRAMBLING_DISABLED | DPCD_TRAINING_PATTERN_1);
+ if (retval)
+ return retval;
for (lane = 0; lane < lane_count; lane++)
buf[lane] = DPCD_PRE_EMPHASIS_PATTERN2_LEVEL0 |
DPCD_VOLTAGE_SWING_PATTERN1_LEVEL0;
- retval = exynos_dp_write_bytes_to_dpcd(dp,
- DPCD_ADDR_TRAINING_LANE0_SET,
- lane_count, buf);
+
+ retval = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_TRAINING_LANE0_SET,
+ lane_count, buf);
return retval;
}
@@ -337,18 +338,17 @@ static int exynos_dp_clock_recovery_ok(u8 link_status[2], int lane_count)
return 0;
}
-static int exynos_dp_channel_eq_ok(u8 link_align[3], int lane_count)
+static int exynos_dp_channel_eq_ok(u8 link_status[2], u8 link_align,
+ int lane_count)
{
int lane;
- u8 lane_align;
u8 lane_status;
- lane_align = link_align[2];
- if ((lane_align & DPCD_INTERLANE_ALIGN_DONE) = 0)
+ if ((link_align & DPCD_INTERLANE_ALIGN_DONE) = 0)
return -EINVAL;
for (lane = 0; lane < lane_count; lane++) {
- lane_status = exynos_dp_get_lane_status(link_align, lane);
+ lane_status = exynos_dp_get_lane_status(link_status, lane);
lane_status &= DPCD_CHANNEL_EQ_BITS;
if (lane_status != DPCD_CHANNEL_EQ_BITS)
return -EINVAL;
@@ -432,22 +432,42 @@ static void exynos_dp_reduce_link_rate(struct exynos_dp_device *dp)
dp->link_train.lt_state = FAILED;
}
+static void exynos_dp_get_adjust_training_lane(struct exynos_dp_device *dp,
+ u8 adjust_request[2])
+{
+ int lane, lane_count;
+ u8 voltage_swing, pre_emphasis, training_lane;
+
+ lane_count = dp->link_train.lane_count;
+ for (lane = 0; lane < lane_count; lane++) {
+ voltage_swing = exynos_dp_get_adjust_request_voltage(
+ adjust_request, lane);
+ pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
+ adjust_request, lane);
+ training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
+ DPCD_PRE_EMPHASIS_SET(pre_emphasis);
+
+ if (voltage_swing = VOLTAGE_LEVEL_3)
+ training_lane |= DPCD_MAX_SWING_REACHED;
+ if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
+ training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
+
+ dp->link_train.training_lane[lane] = training_lane;
+ }
+}
+
static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
{
- u8 link_status[2];
int lane, lane_count, retval;
-
- u8 adjust_request[2];
- u8 voltage_swing;
- u8 pre_emphasis;
- u8 training_lane;
+ u8 voltage_swing, pre_emphasis, training_lane;
+ u8 link_status[2], adjust_request[2];
usleep_range(100, 101);
lane_count = dp->link_train.lane_count;
retval = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
- 2, link_status);
+ 2, link_status);
if (retval)
return retval;
@@ -455,43 +475,30 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
/* set training pattern 2 for EQ */
exynos_dp_set_training_pattern(dp, TRAINING_PTN2);
- for (lane = 0; lane < lane_count; lane++) {
- retval = exynos_dp_read_bytes_from_dpcd(dp,
- DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
- 2, adjust_request);
- if (retval)
- return retval;
-
- voltage_swing = exynos_dp_get_adjust_request_voltage(
- adjust_request, lane);
- pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
- adjust_request, lane);
- training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
- DPCD_PRE_EMPHASIS_SET(pre_emphasis);
-
- if (voltage_swing = VOLTAGE_LEVEL_3)
- training_lane |= DPCD_MAX_SWING_REACHED;
- if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
- training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
+ retval = exynos_dp_read_bytes_from_dpcd(dp,
+ DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
+ 2, adjust_request);
+ if (retval)
+ return retval;
- dp->link_train.training_lane[lane] = training_lane;
+ exynos_dp_get_adjust_training_lane(dp, adjust_request);
+ for (lane = 0; lane < lane_count; lane++) {
exynos_dp_set_lane_link_training(dp,
- dp->link_train.training_lane[lane],
- lane);
+ dp->link_train.training_lane[lane], lane);
}
retval = exynos_dp_write_byte_to_dpcd(dp,
- DPCD_ADDR_TRAINING_PATTERN_SET,
- DPCD_SCRAMBLING_DISABLED |
- DPCD_TRAINING_PATTERN_2);
+ DPCD_ADDR_TRAINING_PATTERN_SET,
+ DPCD_SCRAMBLING_DISABLED |
+ DPCD_TRAINING_PATTERN_2);
if (retval)
return retval;
retval = exynos_dp_write_bytes_to_dpcd(dp,
- DPCD_ADDR_TRAINING_LANE0_SET,
- lane_count,
- dp->link_train.training_lane);
+ DPCD_ADDR_TRAINING_LANE0_SET,
+ lane_count,
+ dp->link_train.training_lane);
if (retval)
return retval;
@@ -501,6 +508,7 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
for (lane = 0; lane < lane_count; lane++) {
training_lane = exynos_dp_get_lane_link_training(
dp, lane);
+
retval = exynos_dp_read_bytes_from_dpcd(dp,
DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
2, adjust_request);
@@ -515,18 +523,24 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
if (voltage_swing = VOLTAGE_LEVEL_3 ||
pre_emphasis = PRE_EMPHASIS_LEVEL_3) {
dev_err(dp->dev, "voltage or pre emphasis reached max level\n");
- goto reduce_link_rate;
+ exynos_dp_reduce_link_rate(dp);
+ return -EIO;
}
- if ((DPCD_VOLTAGE_SWING_GET(training_lane) =
- voltage_swing) &&
- (DPCD_PRE_EMPHASIS_GET(training_lane) =
- pre_emphasis)) {
+ if (DPCD_VOLTAGE_SWING_GET(training_lane) =
+ voltage_swing &&
+ DPCD_PRE_EMPHASIS_GET(training_lane) =
+ pre_emphasis)
dp->link_train.cr_loop[lane]++;
- if (dp->link_train.cr_loop[lane] = MAX_CR_LOOP) {
- dev_err(dp->dev, "CR Max loop\n");
- goto reduce_link_rate;
- }
+
+ if (dp->link_train.cr_loop[lane] = MAX_CR_LOOP ||
+ voltage_swing = VOLTAGE_LEVEL_3 ||
+ pre_emphasis = PRE_EMPHASIS_LEVEL_3) {
+ dev_err(dp->dev, "CR Max reached (%d,%d,%d)\n",
+ dp->link_train.cr_loop[lane],
+ voltage_swing, pre_emphasis);
+ exynos_dp_reduce_link_rate(dp);
+ return -EIO;
}
training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
@@ -544,30 +558,21 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
}
retval = exynos_dp_write_bytes_to_dpcd(dp,
- DPCD_ADDR_TRAINING_LANE0_SET, lane_count,
+ DPCD_ADDR_TRAINING_LANE0_SET,
+ lane_count,
dp->link_train.training_lane);
if (retval)
return retval;
}
- return retval;
-
-reduce_link_rate:
- exynos_dp_reduce_link_rate(dp);
- return -EIO;
+ return 0;
}
static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
{
- u8 link_status[2];
- u8 link_align[3];
int lane, lane_count, retval;
u32 reg;
-
- u8 adjust_request[2];
- u8 voltage_swing;
- u8 pre_emphasis;
- u8 training_lane;
+ u8 link_align, link_status[2], adjust_request[2];
usleep_range(400, 401);
@@ -578,85 +583,63 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
if (retval)
return retval;
- if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
- link_align[0] = link_status[0];
- link_align[1] = link_status[1];
-
- exynos_dp_read_byte_from_dpcd(dp,
- DPCD_ADDR_LANE_ALIGN_STATUS_UPDATED,
- &link_align[2]);
-
- for (lane = 0; lane < lane_count; lane++) {
- retval = exynos_dp_read_bytes_from_dpcd(dp,
- DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
- 2, adjust_request);
- if (retval)
- return retval;
+ if (exynos_dp_clock_recovery_ok(link_status, lane_count)) {
+ exynos_dp_reduce_link_rate(dp);
+ return -EIO;
+ }
- voltage_swing = exynos_dp_get_adjust_request_voltage(
- adjust_request, lane);
- pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
- adjust_request, lane);
- training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
- DPCD_PRE_EMPHASIS_SET(pre_emphasis);
+ retval = exynos_dp_read_byte_from_dpcd(dp,
+ DPCD_ADDR_LANE_ALIGN_STATUS_UPDATED, &link_align);
+ if (retval)
+ return retval;
- if (voltage_swing = VOLTAGE_LEVEL_3)
- training_lane |= DPCD_MAX_SWING_REACHED;
- if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
- training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
+ retval = exynos_dp_read_bytes_from_dpcd(dp,
+ DPCD_ADDR_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
+ if (retval)
+ return retval;
- dp->link_train.training_lane[lane] = training_lane;
- }
+ exynos_dp_get_adjust_training_lane(dp, adjust_request);
- if (exynos_dp_channel_eq_ok(link_align, lane_count) = 0) {
- /* traing pattern Set to Normal */
- exynos_dp_training_pattern_dis(dp);
+ if (!exynos_dp_channel_eq_ok(link_status, link_align, lane_count)) {
+ /* traing pattern Set to Normal */
+ exynos_dp_training_pattern_dis(dp);
- dev_info(dp->dev, "Link Training success!\n");
+ dev_info(dp->dev, "Link Training success!\n");
- exynos_dp_get_link_bandwidth(dp, ®);
- dp->link_train.link_rate = reg;
- dev_dbg(dp->dev, "final bandwidth = %.2x\n",
- dp->link_train.link_rate);
+ exynos_dp_get_link_bandwidth(dp, ®);
+ dp->link_train.link_rate = reg;
+ dev_dbg(dp->dev, "final bandwidth = %.2x\n",
+ dp->link_train.link_rate);
- exynos_dp_get_lane_count(dp, ®);
- dp->link_train.lane_count = reg;
- dev_dbg(dp->dev, "final lane count = %.2x\n",
- dp->link_train.lane_count);
+ exynos_dp_get_lane_count(dp, ®);
+ dp->link_train.lane_count = reg;
+ dev_dbg(dp->dev, "final lane count = %.2x\n",
+ dp->link_train.lane_count);
- /* set enhanced mode if available */
- exynos_dp_set_enhanced_mode(dp);
- dp->link_train.lt_state = FINISHED;
- } else {
- /* not all locked */
- dp->link_train.eq_loop++;
+ /* set enhanced mode if available */
+ exynos_dp_set_enhanced_mode(dp);
+ dp->link_train.lt_state = FINISHED;
- if (dp->link_train.eq_loop > MAX_EQ_LOOP) {
- dev_err(dp->dev, "EQ Max loop\n");
- goto reduce_link_rate;
- }
+ return 0;
+ }
- for (lane = 0; lane < lane_count; lane++)
- exynos_dp_set_lane_link_training(dp,
- dp->link_train.training_lane[lane],
- lane);
+ /* not all locked */
+ dp->link_train.eq_loop++;
- retval = exynos_dp_write_bytes_to_dpcd(dp,
- DPCD_ADDR_TRAINING_LANE0_SET,
- lane_count,
- dp->link_train.training_lane);
- if (retval)
- return retval;
- }
- } else {
- goto reduce_link_rate;
+ if (dp->link_train.eq_loop > MAX_EQ_LOOP) {
+ dev_err(dp->dev, "EQ Max loop\n");
+ exynos_dp_reduce_link_rate(dp);
+ return -EIO;
}
- return 0;
+ for (lane = 0; lane < lane_count; lane++)
+ exynos_dp_set_lane_link_training(dp,
+ dp->link_train.training_lane[lane], lane);
-reduce_link_rate:
- exynos_dp_reduce_link_rate(dp);
- return -EIO;
+ retval = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_TRAINING_LANE0_SET,
+ lane_count, dp->link_train.training_lane);
+
+ return retval;
}
static void exynos_dp_get_max_rx_bandwidth(struct exynos_dp_device *dp,
--
1.7.1
^ permalink raw reply related
* [PATCH v3 1/8] video: exynos_dp: Check DPCD return codes
From: Jingoo Han @ 2012-11-01 10:18 UTC (permalink / raw)
To: linux-fbdev
From: Sean Paul <seanpaul@chromium.org>
Add return code checks to the DPCD transactions in the SW link training
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 86 +++++++++++++++++++++-----------
1 files changed, 56 insertions(+), 30 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index d55470e..44820f2 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -261,11 +261,10 @@ static void exynos_dp_set_lane_lane_pre_emphasis(struct exynos_dp_device *dp,
}
}
-static void exynos_dp_link_start(struct exynos_dp_device *dp)
+static int exynos_dp_link_start(struct exynos_dp_device *dp)
{
u8 buf[4];
- int lane;
- int lane_count;
+ int lane, lane_count, retval;
lane_count = dp->link_train.lane_count;
@@ -276,8 +275,10 @@ static void exynos_dp_link_start(struct exynos_dp_device *dp)
dp->link_train.cr_loop[lane] = 0;
/* Set sink to D0 (Sink Not Ready) mode. */
- exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
+ retval = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
DPCD_SET_POWER_STATE_D0);
+ if (retval)
+ return retval;
/* Set link rate and count as you want to establish*/
exynos_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
@@ -286,8 +287,10 @@ static void exynos_dp_link_start(struct exynos_dp_device *dp)
/* Setup RX configuration */
buf[0] = dp->link_train.link_rate;
buf[1] = dp->link_train.lane_count;
- exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_LINK_BW_SET,
+ retval = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_LINK_BW_SET,
2, buf);
+ if (retval)
+ return retval;
/* Set TX pre-emphasis to minimum */
for (lane = 0; lane < lane_count; lane++)
@@ -306,9 +309,11 @@ static void exynos_dp_link_start(struct exynos_dp_device *dp)
for (lane = 0; lane < lane_count; lane++)
buf[lane] = DPCD_PRE_EMPHASIS_PATTERN2_LEVEL0 |
DPCD_VOLTAGE_SWING_PATTERN1_LEVEL0;
- exynos_dp_write_bytes_to_dpcd(dp,
+ retval = exynos_dp_write_bytes_to_dpcd(dp,
DPCD_ADDR_TRAINING_LANE0_SET,
lane_count, buf);
+
+ return retval;
}
static unsigned char exynos_dp_get_lane_status(u8 link_status[2], int lane)
@@ -430,8 +435,7 @@ static void exynos_dp_reduce_link_rate(struct exynos_dp_device *dp)
static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
{
u8 link_status[2];
- int lane;
- int lane_count;
+ int lane, lane_count, retval;
u8 adjust_request[2];
u8 voltage_swing;
@@ -442,17 +446,22 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
lane_count = dp->link_train.lane_count;
- exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
+ retval = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
2, link_status);
+ if (retval)
+ return retval;
if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
/* set training pattern 2 for EQ */
exynos_dp_set_training_pattern(dp, TRAINING_PTN2);
for (lane = 0; lane < lane_count; lane++) {
- exynos_dp_read_bytes_from_dpcd(dp,
+ retval = exynos_dp_read_bytes_from_dpcd(dp,
DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
2, adjust_request);
+ if (retval)
+ return retval;
+
voltage_swing = exynos_dp_get_adjust_request_voltage(
adjust_request, lane);
pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
@@ -472,15 +481,19 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
lane);
}
- exynos_dp_write_byte_to_dpcd(dp,
+ retval = exynos_dp_write_byte_to_dpcd(dp,
DPCD_ADDR_TRAINING_PATTERN_SET,
DPCD_SCRAMBLING_DISABLED |
DPCD_TRAINING_PATTERN_2);
+ if (retval)
+ return retval;
- exynos_dp_write_bytes_to_dpcd(dp,
+ retval = exynos_dp_write_bytes_to_dpcd(dp,
DPCD_ADDR_TRAINING_LANE0_SET,
lane_count,
dp->link_train.training_lane);
+ if (retval)
+ return retval;
dev_info(dp->dev, "Link Training Clock Recovery success\n");
dp->link_train.lt_state = EQUALIZER_TRAINING;
@@ -488,9 +501,12 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
for (lane = 0; lane < lane_count; lane++) {
training_lane = exynos_dp_get_lane_link_training(
dp, lane);
- exynos_dp_read_bytes_from_dpcd(dp,
+ retval = exynos_dp_read_bytes_from_dpcd(dp,
DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
2, adjust_request);
+ if (retval)
+ return retval;
+
voltage_swing = exynos_dp_get_adjust_request_voltage(
adjust_request, lane);
pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
@@ -527,13 +543,14 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
dp->link_train.training_lane[lane], lane);
}
- exynos_dp_write_bytes_to_dpcd(dp,
- DPCD_ADDR_TRAINING_LANE0_SET,
- lane_count,
- dp->link_train.training_lane);
+ retval = exynos_dp_write_bytes_to_dpcd(dp,
+ DPCD_ADDR_TRAINING_LANE0_SET, lane_count,
+ dp->link_train.training_lane);
+ if (retval)
+ return retval;
}
- return 0;
+ return retval;
reduce_link_rate:
exynos_dp_reduce_link_rate(dp);
@@ -544,8 +561,7 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
{
u8 link_status[2];
u8 link_align[3];
- int lane;
- int lane_count;
+ int lane, lane_count, retval;
u32 reg;
u8 adjust_request[2];
@@ -557,8 +573,10 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
lane_count = dp->link_train.lane_count;
- exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
+ retval = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
2, link_status);
+ if (retval)
+ return retval;
if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
link_align[0] = link_status[0];
@@ -569,9 +587,12 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
&link_align[2]);
for (lane = 0; lane < lane_count; lane++) {
- exynos_dp_read_bytes_from_dpcd(dp,
+ retval = exynos_dp_read_bytes_from_dpcd(dp,
DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
2, adjust_request);
+ if (retval)
+ return retval;
+
voltage_swing = exynos_dp_get_adjust_request_voltage(
adjust_request, lane);
pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
@@ -620,10 +641,12 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
dp->link_train.training_lane[lane],
lane);
- exynos_dp_write_bytes_to_dpcd(dp,
- DPCD_ADDR_TRAINING_LANE0_SET,
- lane_count,
- dp->link_train.training_lane);
+ retval = exynos_dp_write_bytes_to_dpcd(dp,
+ DPCD_ADDR_TRAINING_LANE0_SET,
+ lane_count,
+ dp->link_train.training_lane);
+ if (retval)
+ return retval;
}
} else {
goto reduce_link_rate;
@@ -701,16 +724,17 @@ static void exynos_dp_init_training(struct exynos_dp_device *dp,
static int exynos_dp_sw_link_training(struct exynos_dp_device *dp)
{
- int retval = 0;
- int training_finished = 0;
+ int retval = 0, training_finished = 0;
dp->link_train.lt_state = START;
/* Process here */
- while (!training_finished) {
+ while (!retval && !training_finished) {
switch (dp->link_train.lt_state) {
case START:
- exynos_dp_link_start(dp);
+ retval = exynos_dp_link_start(dp);
+ if (retval)
+ dev_err(dp->dev, "LT link start failed!\n");
break;
case CLOCK_RECOVERY:
retval = exynos_dp_process_clock_recovery(dp);
@@ -729,6 +753,8 @@ static int exynos_dp_sw_link_training(struct exynos_dp_device *dp)
return -EREMOTEIO;
}
}
+ if (retval)
+ dev_err(dp->dev, "eDP link training failed (%d)\n", retval);
return retval;
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] video: exynos_dp: Clean up SW link training
From: Jingoo Han @ 2012-11-01 5:35 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1351702475-31324-1-git-send-email-seanpaul@chromium.org>
On Thursday, November 01, 2012 1:55 AM Sean Paul wrote
>
> Clean up some of the SW training code to make it more clear and reduce
> duplicate code.
>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/video/exynos/exynos_dp_core.c | 279 +++++++++++++--------------------
> 1 files changed, 112 insertions(+), 167 deletions(-)
>
> Thanks for the pointer. There are still places where the code can be either
> simplified, or duplication removed.
Removing duplication is good, but don't change the Link training sequence.
Link training sequence is very sensitive and tricky.
I will modify your patch and I will submit new patch.
Best regards,
Jingoo Han
>
> Below is a rebased patch for your review.
>
> Sean
>
>
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 44820f2..b126e8a 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -276,7 +276,7 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
>
> /* Set sink to D0 (Sink Not Ready) mode. */
> retval = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
> - DPCD_SET_POWER_STATE_D0);
> + DPCD_SET_POWER_STATE_D0);
> if (retval)
> return retval;
>
> @@ -301,17 +301,18 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
> exynos_dp_set_training_pattern(dp, TRAINING_PTN1);
>
> /* Set RX training pattern */
> - exynos_dp_write_byte_to_dpcd(dp,
> - DPCD_ADDR_TRAINING_PATTERN_SET,
> - DPCD_SCRAMBLING_DISABLED |
> - DPCD_TRAINING_PATTERN_1);
> + retval = exynos_dp_write_byte_to_dpcd(dp,
> + DPCD_ADDR_TRAINING_PATTERN_SET,
> + DPCD_SCRAMBLING_DISABLED | DPCD_TRAINING_PATTERN_1);
> + if (retval)
> + return retval;
>
> for (lane = 0; lane < lane_count; lane++)
> buf[lane] = DPCD_PRE_EMPHASIS_PATTERN2_LEVEL0 |
> DPCD_VOLTAGE_SWING_PATTERN1_LEVEL0;
> - retval = exynos_dp_write_bytes_to_dpcd(dp,
> - DPCD_ADDR_TRAINING_LANE0_SET,
> - lane_count, buf);
> +
> + retval = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_TRAINING_LANE0_SET,
> + lane_count, buf);
>
> return retval;
> }
> @@ -337,18 +338,17 @@ static int exynos_dp_clock_recovery_ok(u8 link_status[2], int lane_count)
> return 0;
> }
>
> -static int exynos_dp_channel_eq_ok(u8 link_align[3], int lane_count)
> +static int exynos_dp_channel_eq_ok(u8 link_status[2], u8 link_align,
> + int lane_count)
> {
> int lane;
> - u8 lane_align;
> u8 lane_status;
>
> - lane_align = link_align[2];
> - if ((lane_align & DPCD_INTERLANE_ALIGN_DONE) = 0)
> + if ((link_align & DPCD_INTERLANE_ALIGN_DONE) = 0)
> return -EINVAL;
>
> for (lane = 0; lane < lane_count; lane++) {
> - lane_status = exynos_dp_get_lane_status(link_align, lane);
> + lane_status = exynos_dp_get_lane_status(link_status, lane);
> lane_status &= DPCD_CHANNEL_EQ_BITS;
> if (lane_status != DPCD_CHANNEL_EQ_BITS)
> return -EINVAL;
> @@ -432,22 +432,47 @@ static void exynos_dp_reduce_link_rate(struct exynos_dp_device *dp)
> dp->link_train.lt_state = FAILED;
> }
>
> +static void exynos_dp_get_adjust_training_lane(struct exynos_dp_device *dp,
> + u8 adjust_request[2])
> +{
> + int lane, lane_count;
> + u8 voltage_swing, pre_emphasis, training_lane;
> +
> + lane_count = dp->link_train.lane_count;
> + for (lane = 0; lane < lane_count; lane++) {
> + voltage_swing = exynos_dp_get_adjust_request_voltage(
> + adjust_request, lane);
> + pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
> + adjust_request, lane);
> + training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
> + DPCD_PRE_EMPHASIS_SET(pre_emphasis);
> +
> + if (voltage_swing = VOLTAGE_LEVEL_3)
> + training_lane |= DPCD_MAX_SWING_REACHED;
> + if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
> + training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
> +
> + dp->link_train.training_lane[lane] = training_lane;
> + }
> +}
> +
> static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
> {
> - u8 link_status[2];
> int lane, lane_count, retval;
> -
> - u8 adjust_request[2];
> - u8 voltage_swing;
> - u8 pre_emphasis;
> - u8 training_lane;
> + u8 voltage_swing, pre_emphasis, training_lane;
> + u8 link_status[2], adjust_request[2];
>
> usleep_range(100, 101);
>
> lane_count = dp->link_train.lane_count;
>
> retval = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
> - 2, link_status);
> + 2, link_status);
> + if (retval)
> + return retval;
> +
> + retval = exynos_dp_read_bytes_from_dpcd(dp,
> + DPCD_ADDR_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
> if (retval)
> return retval;
>
> @@ -455,43 +480,9 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
> /* set training pattern 2 for EQ */
> exynos_dp_set_training_pattern(dp, TRAINING_PTN2);
>
> - for (lane = 0; lane < lane_count; lane++) {
> - retval = exynos_dp_read_bytes_from_dpcd(dp,
> - DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
> - 2, adjust_request);
> - if (retval)
> - return retval;
> -
> - voltage_swing = exynos_dp_get_adjust_request_voltage(
> - adjust_request, lane);
> - pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
> - adjust_request, lane);
> - training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
> - DPCD_PRE_EMPHASIS_SET(pre_emphasis);
> -
> - if (voltage_swing = VOLTAGE_LEVEL_3)
> - training_lane |= DPCD_MAX_SWING_REACHED;
> - if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
> - training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
> -
> - dp->link_train.training_lane[lane] = training_lane;
> -
> - exynos_dp_set_lane_link_training(dp,
> - dp->link_train.training_lane[lane],
> - lane);
> - }
> -
Please don't move it to back.
> retval = exynos_dp_write_byte_to_dpcd(dp,
> DPCD_ADDR_TRAINING_PATTERN_SET,
> - DPCD_SCRAMBLING_DISABLED |
> - DPCD_TRAINING_PATTERN_2);
> - if (retval)
> - return retval;
> -
> - retval = exynos_dp_write_bytes_to_dpcd(dp,
> - DPCD_ADDR_TRAINING_LANE0_SET,
> - lane_count,
> - dp->link_train.training_lane);
> + DPCD_SCRAMBLING_DISABLED | DPCD_TRAINING_PATTERN_2);
> if (retval)
> return retval;
>
> @@ -501,73 +492,49 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
> for (lane = 0; lane < lane_count; lane++) {
> training_lane = exynos_dp_get_lane_link_training(
> dp, lane);
> - retval = exynos_dp_read_bytes_from_dpcd(dp,
> - DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
> - 2, adjust_request);
> - if (retval)
> - return retval;
> -
> voltage_swing = exynos_dp_get_adjust_request_voltage(
> adjust_request, lane);
> pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
> adjust_request, lane);
>
> - if (voltage_swing = VOLTAGE_LEVEL_3 ||
> - pre_emphasis = PRE_EMPHASIS_LEVEL_3) {
> - dev_err(dp->dev, "voltage or pre emphasis reached max level\n");
> - goto reduce_link_rate;
> - }
> -
> - if ((DPCD_VOLTAGE_SWING_GET(training_lane) =
> - voltage_swing) &&
> - (DPCD_PRE_EMPHASIS_GET(training_lane) =
> - pre_emphasis)) {
> + if (DPCD_VOLTAGE_SWING_GET(training_lane) =
> + voltage_swing &&
> + DPCD_PRE_EMPHASIS_GET(training_lane) =
> + pre_emphasis)
> dp->link_train.cr_loop[lane]++;
> - if (dp->link_train.cr_loop[lane] = MAX_CR_LOOP) {
> - dev_err(dp->dev, "CR Max loop\n");
> - goto reduce_link_rate;
> - }
> - }
>
> - training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
> - DPCD_PRE_EMPHASIS_SET(pre_emphasis);
> -
> - if (voltage_swing = VOLTAGE_LEVEL_3)
> - training_lane |= DPCD_MAX_SWING_REACHED;
> - if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
> - training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
> -
> - dp->link_train.training_lane[lane] = training_lane;
> -
> - exynos_dp_set_lane_link_training(dp,
> - dp->link_train.training_lane[lane], lane);
> + if (dp->link_train.cr_loop[lane] = MAX_CR_LOOP ||
> + voltage_swing = VOLTAGE_LEVEL_3 ||
> + pre_emphasis = PRE_EMPHASIS_LEVEL_3) {
> + dev_err(dp->dev, "CR Max reached (%d,%d,%d)\n",
> + dp->link_train.cr_loop[lane],
> + voltage_swing, pre_emphasis);
> + exynos_dp_reduce_link_rate(dp);
> + return -EIO;
> + }
> }
> + }
> +
> + exynos_dp_get_adjust_training_lane(dp, adjust_request);
>
> - retval = exynos_dp_write_bytes_to_dpcd(dp,
> - DPCD_ADDR_TRAINING_LANE0_SET, lane_count,
> - dp->link_train.training_lane);
> + for (lane = 0; lane < lane_count; lane++) {
> + exynos_dp_set_lane_link_training(dp,
> + dp->link_train.training_lane[lane], lane);
> + retval = exynos_dp_write_byte_to_dpcd(dp,
> + DPCD_ADDR_TRAINING_LANE0_SET + lane,
> + dp->link_train.training_lane[lane]);
The following would be better.
byte's'_to_dpcd is faster than byte_to_dpcd x 4 times.
for (lane = 0; lane < lane_count; lane++) {
exynos_dp_set_lane_link_training(dp,
dp->link_train.training_lane[lane], lane);
}
retval = exynos_dp_write_bytes_to_dpcd(dp,
DPCD_ADDR_TRAINING_LANE0_SET, lane_count,
dp->link_train.training_lane);
> if (retval)
> return retval;
> }
>
> return retval;
> -
> -reduce_link_rate:
> - exynos_dp_reduce_link_rate(dp);
> - return -EIO;
> }
>
> static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
> {
> - u8 link_status[2];
> - u8 link_align[3];
> int lane, lane_count, retval;
> u32 reg;
> -
> - u8 adjust_request[2];
> - u8 voltage_swing;
> - u8 pre_emphasis;
> - u8 training_lane;
> + u8 link_align, link_status[2], adjust_request[2];
>
> usleep_range(400, 401);
>
> @@ -578,85 +545,63 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
> if (retval)
> return retval;
>
> - if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
> - link_align[0] = link_status[0];
> - link_align[1] = link_status[1];
> -
> - exynos_dp_read_byte_from_dpcd(dp,
> - DPCD_ADDR_LANE_ALIGN_STATUS_UPDATED,
> - &link_align[2]);
> -
> - for (lane = 0; lane < lane_count; lane++) {
> - retval = exynos_dp_read_bytes_from_dpcd(dp,
> - DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
> - 2, adjust_request);
> - if (retval)
> - return retval;
> + if (exynos_dp_clock_recovery_ok(link_status, lane_count)) {
> + exynos_dp_reduce_link_rate(dp);
> + return -EIO;
> + }
>
> - voltage_swing = exynos_dp_get_adjust_request_voltage(
> - adjust_request, lane);
> - pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
> - adjust_request, lane);
> - training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
> - DPCD_PRE_EMPHASIS_SET(pre_emphasis);
> + retval = exynos_dp_read_bytes_from_dpcd(dp,
> + DPCD_ADDR_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
> + if (retval)
> + return retval;
>
> - if (voltage_swing = VOLTAGE_LEVEL_3)
> - training_lane |= DPCD_MAX_SWING_REACHED;
> - if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
> - training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
> + retval = exynos_dp_read_byte_from_dpcd(dp,
> + DPCD_ADDR_LANE_ALIGN_STATUS_UPDATED, &link_align);
> + if (retval)
> + return retval;
>
> - dp->link_train.training_lane[lane] = training_lane;
> - }
> + exynos_dp_get_adjust_training_lane(dp, adjust_request);
>
> - if (exynos_dp_channel_eq_ok(link_align, lane_count) = 0) {
> - /* traing pattern Set to Normal */
> - exynos_dp_training_pattern_dis(dp);
> + if (!exynos_dp_channel_eq_ok(link_status, link_align, lane_count)) {
> + /* traing pattern Set to Normal */
> + exynos_dp_training_pattern_dis(dp);
>
> - dev_info(dp->dev, "Link Training success!\n");
> + dev_info(dp->dev, "Link Training success!\n");
>
> - exynos_dp_get_link_bandwidth(dp, ®);
> - dp->link_train.link_rate = reg;
> - dev_dbg(dp->dev, "final bandwidth = %.2x\n",
> - dp->link_train.link_rate);
> + exynos_dp_get_link_bandwidth(dp, ®);
> + dp->link_train.link_rate = reg;
> + dev_dbg(dp->dev, "final bandwidth = %.2x\n",
> + dp->link_train.link_rate);
>
> - exynos_dp_get_lane_count(dp, ®);
> - dp->link_train.lane_count = reg;
> - dev_dbg(dp->dev, "final lane count = %.2x\n",
> - dp->link_train.lane_count);
> + exynos_dp_get_lane_count(dp, ®);
> + dp->link_train.lane_count = reg;
> + dev_dbg(dp->dev, "final lane count = %.2x\n",
> + dp->link_train.lane_count);
>
> - /* set enhanced mode if available */
> - exynos_dp_set_enhanced_mode(dp);
> - dp->link_train.lt_state = FINISHED;
> - } else {
> - /* not all locked */
> - dp->link_train.eq_loop++;
> + /* set enhanced mode if available */
> + exynos_dp_set_enhanced_mode(dp);
> + dp->link_train.lt_state = FINISHED;
>
> - if (dp->link_train.eq_loop > MAX_EQ_LOOP) {
> - dev_err(dp->dev, "EQ Max loop\n");
> - goto reduce_link_rate;
> - }
> + return 0;
> + }
>
> - for (lane = 0; lane < lane_count; lane++)
> - exynos_dp_set_lane_link_training(dp,
> - dp->link_train.training_lane[lane],
> - lane);
> + /* not all locked */
> + dp->link_train.eq_loop++;
>
> - retval = exynos_dp_write_bytes_to_dpcd(dp,
> - DPCD_ADDR_TRAINING_LANE0_SET,
> - lane_count,
> - dp->link_train.training_lane);
> - if (retval)
> - return retval;
> - }
> - } else {
> - goto reduce_link_rate;
> + if (dp->link_train.eq_loop > MAX_EQ_LOOP) {
> + dev_err(dp->dev, "EQ Max loop\n");
> + exynos_dp_reduce_link_rate(dp);
> + return -EIO;
> }
>
> - return 0;
> + for (lane = 0; lane < lane_count; lane++)
> + exynos_dp_set_lane_link_training(dp,
> + dp->link_train.training_lane[lane], lane);
>
> -reduce_link_rate:
> - exynos_dp_reduce_link_rate(dp);
> - return -EIO;
> + retval = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_TRAINING_LANE0_SET,
> + lane_count, dp->link_train.training_lane);
> +
> + return retval;
> }
>
> static void exynos_dp_get_max_rx_bandwidth(struct exynos_dp_device *dp,
> --
> 1.7.7.3
^ permalink raw reply
* Re: [PATCH 08/10] video: exynos_dp: Move hotplug into a workqueue
From: Jingoo Han @ 2012-11-01 2:51 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344398064-13563-9-git-send-email-seanpaul@chromium.org>
On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote
>
> Move the hotplug related code from probe and resume into a workqueue.
> This allows us to initialize the DP driver (and resume it) when there
> is no monitor connected.
>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Olof Johansson <olofj@chromium.org>
> ---
> drivers/video/exynos/exynos_dp_core.c | 94 +++++++++++++++++----------------
> drivers/video/exynos/exynos_dp_core.h | 1 +
> 2 files changed, 50 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 2882362..68ad494 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -834,6 +834,45 @@ static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
> return IRQ_HANDLED;
> }
>
> +static void exynos_dp_hotplug(struct work_struct *work)
> +{
> + struct exynos_dp_device *dp;
> + int ret;
> +
> + dp = container_of(work, struct exynos_dp_device, hotplug_work);
> +
> + ret = exynos_dp_detect_hpd(dp);
> + if (ret) {
> + dev_err(dp->dev, "unable to detect hpd\n");
> + return;
> + }
> +
> + ret = exynos_dp_handle_edid(dp);
> + if (ret) {
> + dev_err(dp->dev, "unable to handle edid\n");
> + return;
> + }
> +
> + ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> + dp->video_info->link_rate);
> + if (ret) {
> + dev_err(dp->dev, "unable to do link train\n");
> + return;
> + }
> +
> + exynos_dp_enable_scramble(dp, 1);
> + exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> + exynos_dp_enable_enhanced_mode(dp, 1);
> +
> + exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> + exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> +
> + exynos_dp_init_video(dp);
> + ret = exynos_dp_config_video(dp, dp->video_info);
> + if (ret)
> + dev_err(&dp->dev, "unable to config video\n");
Please, remove build warnings and re-submit new patch.
drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_hotplug':
drivers/video/exynos/exynos_dp_core.c:875:3: warning: passing argument 1 of 'dev_err' from incompatible pointer type [enabled by
default]
include/linux/device.h:928:5: note: expected 'const struct device *' but argument is of type 'struct device **'
drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_remove':
drivers/video/exynos/exynos_dp_core.c:950:3: warning: 'flush_work_sync' is deprecated (declared at include/linux/workqueue.h:448)
[-Wdeprecated-declarations]
drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_suspend':
drivers/video/exynos/exynos_dp_core.c:968:3: warning: 'flush_work_sync' is deprecated (declared at include/linux/workqueue.h:448)
[-Wdeprecated-declarations]
> +}
> +
> static int __devinit exynos_dp_probe(struct platform_device *pdev)
> {
> struct resource *res;
> @@ -886,6 +925,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> goto err_clock;
> }
>
> + INIT_WORK(&dp->hotplug_work, exynos_dp_hotplug);
> +
> ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
> "exynos-dp", dp);
> if (ret) {
> @@ -899,36 +940,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>
> exynos_dp_init_dp(dp);
>
> - ret = exynos_dp_detect_hpd(dp);
> - if (ret) {
> - dev_err(&pdev->dev, "unable to detect hpd\n");
> - goto err_clock;
> - }
> -
> - exynos_dp_handle_edid(dp);
> -
> - ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> - dp->video_info->link_rate);
> - if (ret) {
> - dev_err(&pdev->dev, "unable to do link train\n");
> - goto err_clock;
> - }
> -
> - exynos_dp_enable_scramble(dp, 1);
> - exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> - exynos_dp_enable_enhanced_mode(dp, 1);
> -
> - exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> - exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> -
> - exynos_dp_init_video(dp);
> - ret = exynos_dp_config_video(dp, dp->video_info);
> - if (ret) {
> - dev_err(&pdev->dev, "unable to config video\n");
> - goto err_clock;
> - }
> -
> platform_set_drvdata(pdev, dp);
> + schedule_work(&dp->hotplug_work);
>
> return 0;
>
> @@ -943,6 +956,9 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
> struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
> struct exynos_dp_device *dp = platform_get_drvdata(pdev);
>
> + if (work_pending(&dp->hotplug_work))
> + flush_work_sync(&dp->hotplug_work);
> +
> if (pdata && pdata->phy_exit)
> pdata->phy_exit();
>
> @@ -959,6 +975,9 @@ static int exynos_dp_suspend(struct device *dev)
> struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
> struct exynos_dp_device *dp = platform_get_drvdata(pdev);
>
> + if (work_pending(&dp->hotplug_work))
> + flush_work_sync(&dp->hotplug_work);
> +
> if (pdata && pdata->phy_exit)
> pdata->phy_exit();
>
> @@ -979,22 +998,7 @@ static int exynos_dp_resume(struct device *dev)
> clk_enable(dp->clock);
>
> exynos_dp_init_dp(dp);
> -
> - exynos_dp_detect_hpd(dp);
> - exynos_dp_handle_edid(dp);
> -
> - exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> - dp->video_info->link_rate);
> -
> - exynos_dp_enable_scramble(dp, 1);
> - exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> - exynos_dp_enable_enhanced_mode(dp, 1);
> -
> - exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> - exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> -
> - exynos_dp_init_video(dp);
> - exynos_dp_config_video(dp, dp->video_info);
> + schedule_work(&dp->hotplug_work);
>
> return 0;
> }
> diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
> index 6431c65..cf1010b 100644
> --- a/drivers/video/exynos/exynos_dp_core.h
> +++ b/drivers/video/exynos/exynos_dp_core.h
> @@ -32,6 +32,7 @@ struct exynos_dp_device {
>
> struct video_info *video_info;
> struct link_train link_train;
> + struct work_struct hotplug_work;
> };
>
> /* exynos_dp_reg.c */
> --
> 1.7.7.3
^ permalink raw reply
* RE: [PATCH 05/10] video: exynos_dp: Remove sink control to D0
From: Jingoo Han @ 2012-11-01 0:45 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344398064-13563-6-git-send-email-seanpaul@chromium.org>
On Thursday, November 01, 2012 2:05 AM Sean Paul wrote
>
> On Mon, Aug 20, 2012 at 5:19 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> > On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote:
> >>
> >> Don't reset the sink power to D0. Removing this for three reasons:
> >>
> >> 1) It's not required in the SW link training documentation
> > OK.
> >> 2) The comment is incorrect, D0 is normal operation, not "power down"
> > OK. As you mentioned, D0 is not 'power down', 'normal operation'.
> > However, if comment is wrong, usually we fix comment, not remove it. :)
> >
>
> The datasheet doesn't mention this step at all. Furthermore, the
> comment explains how it *should* work, so I think it's the code that
> is in error. Do you know why this code is needed?
This is not requirement of the datasheet.
Also, D0 is not necessary for Link training.
Um, this is work-around for initial development.
Newly developed panel did not work, due to pane problem,
so this work-around was added.
However, the panel problem was fixed, so this code is
not necessary.
Please replace comment as below:
According to DP spec, it is not required in the Link Training procedure.
Best regards,
Jingoo Han
>
> >> 3) It seems to change things in the link training that causes glitches
> >
> > Um, it seems that this problem depends on LCD panel.
> > Other LCDs that I have tested do not have this kind of problem.
> > Please, modify this comment.
> >
>
> That was a red herring, it didn't cause the glitches, just made them
> less obvious.
>
> Sean
>
> >>
> >> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> >> ---
> >> drivers/video/exynos/exynos_dp_core.c | 6 ------
> >> 1 files changed, 0 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> >> index 207bd7e..1c998d9 100644
> >> --- a/drivers/video/exynos/exynos_dp_core.c
> >> +++ b/drivers/video/exynos/exynos_dp_core.c
> >> @@ -273,12 +273,6 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
> >> for (lane = 0; lane < lane_count; lane++)
> >> dp->link_train.cr_loop[lane] = 0;
> >>
> >> - /* Set sink to D0 (Sink Not Ready) mode. */
> >> - ret = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
> >> - DPCD_SET_POWER_STATE_D0);
> >> - if (ret)
> >> - return ret;
> >> -
> >> /* Set link rate and count as you want to establish*/
> >> exynos_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
> >> exynos_dp_set_lane_count(dp, dp->link_train.lane_count);
> >> --
> >> 1.7.7.3
> >
^ permalink raw reply
* Re: [PATCH 08/10] video: exynos_dp: Move hotplug into a workqueue
From: Jingoo Han @ 2012-11-01 0:36 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344398064-13563-9-git-send-email-seanpaul@chromium.org>
On Thursday, November 01, 2012 2:20 AM Sean Paul wrote
>
> On Tue, Aug 7, 2012 at 11:54 PM, Sean Paul <seanpaul@chromium.org> wrote:
> > Move the hotplug related code from probe and resume into a workqueue.
> > This allows us to initialize the DP driver (and resume it) when there
> > is no monitor connected.
> >
>
> Comments?
It looks good, but, I have to test this.
>
>
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > Reviewed-by: Olof Johansson <olofj@chromium.org>
> > ---
> > drivers/video/exynos/exynos_dp_core.c | 94 +++++++++++++++++----------------
> > drivers/video/exynos/exynos_dp_core.h | 1 +
> > 2 files changed, 50 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> > index 2882362..68ad494 100644
> > --- a/drivers/video/exynos/exynos_dp_core.c
> > +++ b/drivers/video/exynos/exynos_dp_core.c
> > @@ -834,6 +834,45 @@ static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
> > return IRQ_HANDLED;
> > }
> >
> > +static void exynos_dp_hotplug(struct work_struct *work)
> > +{
> > + struct exynos_dp_device *dp;
> > + int ret;
> > +
> > + dp = container_of(work, struct exynos_dp_device, hotplug_work);
> > +
> > + ret = exynos_dp_detect_hpd(dp);
> > + if (ret) {
> > + dev_err(dp->dev, "unable to detect hpd\n");
> > + return;
> > + }
> > +
> > + ret = exynos_dp_handle_edid(dp);
> > + if (ret) {
> > + dev_err(dp->dev, "unable to handle edid\n");
> > + return;
> > + }
> > +
> > + ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> > + dp->video_info->link_rate);
> > + if (ret) {
> > + dev_err(dp->dev, "unable to do link train\n");
> > + return;
> > + }
> > +
> > + exynos_dp_enable_scramble(dp, 1);
> > + exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> > + exynos_dp_enable_enhanced_mode(dp, 1);
> > +
> > + exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> > + exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> > +
> > + exynos_dp_init_video(dp);
> > + ret = exynos_dp_config_video(dp, dp->video_info);
> > + if (ret)
> > + dev_err(&dp->dev, "unable to config video\n");
> > +}
> > +
> > static int __devinit exynos_dp_probe(struct platform_device *pdev)
> > {
> > struct resource *res;
> > @@ -886,6 +925,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> > goto err_clock;
> > }
> >
> > + INIT_WORK(&dp->hotplug_work, exynos_dp_hotplug);
> > +
> > ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
> > "exynos-dp", dp);
> > if (ret) {
> > @@ -899,36 +940,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >
> > exynos_dp_init_dp(dp);
> >
> > - ret = exynos_dp_detect_hpd(dp);
> > - if (ret) {
> > - dev_err(&pdev->dev, "unable to detect hpd\n");
> > - goto err_clock;
> > - }
> > -
> > - exynos_dp_handle_edid(dp);
> > -
> > - ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> > - dp->video_info->link_rate);
> > - if (ret) {
> > - dev_err(&pdev->dev, "unable to do link train\n");
> > - goto err_clock;
> > - }
> > -
> > - exynos_dp_enable_scramble(dp, 1);
> > - exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> > - exynos_dp_enable_enhanced_mode(dp, 1);
> > -
> > - exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> > - exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> > -
> > - exynos_dp_init_video(dp);
> > - ret = exynos_dp_config_video(dp, dp->video_info);
> > - if (ret) {
> > - dev_err(&pdev->dev, "unable to config video\n");
> > - goto err_clock;
> > - }
> > -
> > platform_set_drvdata(pdev, dp);
> > + schedule_work(&dp->hotplug_work);
> >
> > return 0;
> >
> > @@ -943,6 +956,9 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
> > struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
> > struct exynos_dp_device *dp = platform_get_drvdata(pdev);
> >
> > + if (work_pending(&dp->hotplug_work))
> > + flush_work_sync(&dp->hotplug_work);
> > +
> > if (pdata && pdata->phy_exit)
> > pdata->phy_exit();
> >
> > @@ -959,6 +975,9 @@ static int exynos_dp_suspend(struct device *dev)
> > struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
> > struct exynos_dp_device *dp = platform_get_drvdata(pdev);
> >
> > + if (work_pending(&dp->hotplug_work))
> > + flush_work_sync(&dp->hotplug_work);
> > +
> > if (pdata && pdata->phy_exit)
> > pdata->phy_exit();
> >
> > @@ -979,22 +998,7 @@ static int exynos_dp_resume(struct device *dev)
> > clk_enable(dp->clock);
> >
> > exynos_dp_init_dp(dp);
> > -
> > - exynos_dp_detect_hpd(dp);
> > - exynos_dp_handle_edid(dp);
> > -
> > - exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> > - dp->video_info->link_rate);
> > -
> > - exynos_dp_enable_scramble(dp, 1);
> > - exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> > - exynos_dp_enable_enhanced_mode(dp, 1);
> > -
> > - exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> > - exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> > -
> > - exynos_dp_init_video(dp);
> > - exynos_dp_config_video(dp, dp->video_info);
> > + schedule_work(&dp->hotplug_work);
> >
> > return 0;
> > }
> > diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
> > index 6431c65..cf1010b 100644
> > --- a/drivers/video/exynos/exynos_dp_core.h
> > +++ b/drivers/video/exynos/exynos_dp_core.h
> > @@ -32,6 +32,7 @@ struct exynos_dp_device {
> >
> > struct video_info *video_info;
> > struct link_train link_train;
> > + struct work_struct hotplug_work;
> > };
> >
> > /* exynos_dp_reg.c */
> > --
> > 1.7.7.3
> >
^ permalink raw reply
* Re: [PATCH 08/10] video: exynos_dp: Move hotplug into a workqueue
From: Sean Paul @ 2012-10-31 17:20 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344398064-13563-9-git-send-email-seanpaul@chromium.org>
On Tue, Aug 7, 2012 at 11:54 PM, Sean Paul <seanpaul@chromium.org> wrote:
> Move the hotplug related code from probe and resume into a workqueue.
> This allows us to initialize the DP driver (and resume it) when there
> is no monitor connected.
>
Comments?
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Olof Johansson <olofj@chromium.org>
> ---
> drivers/video/exynos/exynos_dp_core.c | 94 +++++++++++++++++----------------
> drivers/video/exynos/exynos_dp_core.h | 1 +
> 2 files changed, 50 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 2882362..68ad494 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -834,6 +834,45 @@ static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
> return IRQ_HANDLED;
> }
>
> +static void exynos_dp_hotplug(struct work_struct *work)
> +{
> + struct exynos_dp_device *dp;
> + int ret;
> +
> + dp = container_of(work, struct exynos_dp_device, hotplug_work);
> +
> + ret = exynos_dp_detect_hpd(dp);
> + if (ret) {
> + dev_err(dp->dev, "unable to detect hpd\n");
> + return;
> + }
> +
> + ret = exynos_dp_handle_edid(dp);
> + if (ret) {
> + dev_err(dp->dev, "unable to handle edid\n");
> + return;
> + }
> +
> + ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> + dp->video_info->link_rate);
> + if (ret) {
> + dev_err(dp->dev, "unable to do link train\n");
> + return;
> + }
> +
> + exynos_dp_enable_scramble(dp, 1);
> + exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> + exynos_dp_enable_enhanced_mode(dp, 1);
> +
> + exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> + exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> +
> + exynos_dp_init_video(dp);
> + ret = exynos_dp_config_video(dp, dp->video_info);
> + if (ret)
> + dev_err(&dp->dev, "unable to config video\n");
> +}
> +
> static int __devinit exynos_dp_probe(struct platform_device *pdev)
> {
> struct resource *res;
> @@ -886,6 +925,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> goto err_clock;
> }
>
> + INIT_WORK(&dp->hotplug_work, exynos_dp_hotplug);
> +
> ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
> "exynos-dp", dp);
> if (ret) {
> @@ -899,36 +940,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>
> exynos_dp_init_dp(dp);
>
> - ret = exynos_dp_detect_hpd(dp);
> - if (ret) {
> - dev_err(&pdev->dev, "unable to detect hpd\n");
> - goto err_clock;
> - }
> -
> - exynos_dp_handle_edid(dp);
> -
> - ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> - dp->video_info->link_rate);
> - if (ret) {
> - dev_err(&pdev->dev, "unable to do link train\n");
> - goto err_clock;
> - }
> -
> - exynos_dp_enable_scramble(dp, 1);
> - exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> - exynos_dp_enable_enhanced_mode(dp, 1);
> -
> - exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> - exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> -
> - exynos_dp_init_video(dp);
> - ret = exynos_dp_config_video(dp, dp->video_info);
> - if (ret) {
> - dev_err(&pdev->dev, "unable to config video\n");
> - goto err_clock;
> - }
> -
> platform_set_drvdata(pdev, dp);
> + schedule_work(&dp->hotplug_work);
>
> return 0;
>
> @@ -943,6 +956,9 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
> struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
> struct exynos_dp_device *dp = platform_get_drvdata(pdev);
>
> + if (work_pending(&dp->hotplug_work))
> + flush_work_sync(&dp->hotplug_work);
> +
> if (pdata && pdata->phy_exit)
> pdata->phy_exit();
>
> @@ -959,6 +975,9 @@ static int exynos_dp_suspend(struct device *dev)
> struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
> struct exynos_dp_device *dp = platform_get_drvdata(pdev);
>
> + if (work_pending(&dp->hotplug_work))
> + flush_work_sync(&dp->hotplug_work);
> +
> if (pdata && pdata->phy_exit)
> pdata->phy_exit();
>
> @@ -979,22 +998,7 @@ static int exynos_dp_resume(struct device *dev)
> clk_enable(dp->clock);
>
> exynos_dp_init_dp(dp);
> -
> - exynos_dp_detect_hpd(dp);
> - exynos_dp_handle_edid(dp);
> -
> - exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> - dp->video_info->link_rate);
> -
> - exynos_dp_enable_scramble(dp, 1);
> - exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> - exynos_dp_enable_enhanced_mode(dp, 1);
> -
> - exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> - exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> -
> - exynos_dp_init_video(dp);
> - exynos_dp_config_video(dp, dp->video_info);
> + schedule_work(&dp->hotplug_work);
>
> return 0;
> }
> diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
> index 6431c65..cf1010b 100644
> --- a/drivers/video/exynos/exynos_dp_core.h
> +++ b/drivers/video/exynos/exynos_dp_core.h
> @@ -32,6 +32,7 @@ struct exynos_dp_device {
>
> struct video_info *video_info;
> struct link_train link_train;
> + struct work_struct hotplug_work;
> };
>
> /* exynos_dp_reg.c */
> --
> 1.7.7.3
>
^ permalink raw reply
* Re: [PATCH v7 4/8] video: add videomode helpers
From: Laurent Pinchart @ 2012-10-31 17:18 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Thierry Reding, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-5-git-send-email-s.trumtrar@pengutronix.de>
Hi Steffen,
Thanks for the patch.
On Wednesday 31 October 2012 10:28:04 Steffen Trumtrar wrote:
> Add helper functions to convert from display timings to a generic videomode
> structure. This videomode can then be converted to the corresponding
> subsystem mode representation (e.g. fb_videomode).
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
> drivers/video/Kconfig | 6 ++++++
> drivers/video/Makefile | 1 +
> drivers/video/videomode.c | 44 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/videomode.h | 36 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 87 insertions(+)
> create mode 100644 drivers/video/videomode.c
> create mode 100644 include/linux/videomode.h
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 1421fc8..45dd393 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -38,6 +38,12 @@ config DISPLAY_TIMING
> help
> Say Y here, to use the display timing helpers.
>
> +config VIDEOMODE
> + bool "Enable videomode helpers"
Shouldn't this option should be automatically selected through a select
statement in other options that depend on it instead of manually selected ?
Same for the DISPLAY_TIMING option in 1/8.
There's so little code here, do you think it would be a good idea to merge
patches 1/8 and 4/8 and have a single Kconfig option ?
> + help
> + Say Y here, to use the generic videomode helpers. This allows
> + converting from display timings to fb_videomode and drm_display_mode
> +
> menuconfig FB
> tristate "Support for frame buffer devices"
> ---help---
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 552c045..fc30439 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -168,3 +168,4 @@ obj-$(CONFIG_FB_VIRTUAL) += vfb.o
> #video output switch sysfs driver
> obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
> obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
> +obj-$(CONFIG_VIDEOMODE) += videomode.o
> diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
> new file mode 100644
> index 0000000..a9fe010
> --- /dev/null
> +++ b/drivers/video/videomode.c
> @@ -0,0 +1,44 @@
> +/*
> + * generic display timing functions
> + *
> + * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>,
> Pengutronix + *
> + * This file is released under the GPLv2
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/export.h>
> +#include <linux/errno.h>
> +#include <linux/display_timing.h>
> +#include <linux/videomode.h>
As in 1/8, I try to keep #include's sorted alphabetically, but I won't push
for it here either :-)
> +
> +int videomode_from_timing(struct display_timings *disp, struct videomode
> *vm,
> + int index)
unsigned int for index ?
> +{
> + struct display_timing *dt = NULL;
No need to initialize dt to NULL.
> + dt = display_timings_get(disp, index);
> + if (!dt) {
> + pr_err("%s: no signal timings found\n", __func__);
I wonder whether this really deserves a pr_err() here. Would this be a caller
bug, or can there be valid use cases where this function would return an error
?
> + return -EINVAL;
> + }
> +
> + vm->pixelclock = display_timing_get_value(&dt->pixelclock, 0);
> + vm->hactive = display_timing_get_value(&dt->hactive, 0);
> + vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, 0);
> + vm->hback_porch = display_timing_get_value(&dt->hback_porch, 0);
> + vm->hsync_len = display_timing_get_value(&dt->hsync_len, 0);
> +
> + vm->vactive = display_timing_get_value(&dt->vactive, 0);
> + vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, 0);
> + vm->vback_porch = display_timing_get_value(&dt->vback_porch, 0);
> + vm->vsync_len = display_timing_get_value(&dt->vsync_len, 0);
> +
> + vm->vah = dt->vsync_pol_active;
> + vm->hah = dt->hsync_pol_active;
> + vm->interlaced = dt->interlaced;
> + vm->doublescan = dt->doublescan;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(videomode_from_timing);
> diff --git a/include/linux/videomode.h b/include/linux/videomode.h
> new file mode 100644
> index 0000000..f932147
> --- /dev/null
> +++ b/include/linux/videomode.h
> @@ -0,0 +1,36 @@
> +/*
> + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * generic videomode description
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_VIDEOMODE_H
> +#define __LINUX_VIDEOMODE_H
> +
> +#include <linux/display_timing.h>
> +
> +struct videomode {
> + u32 pixelclock;
> + u32 refreshrate;
> +
> + u32 hactive;
> + u32 hfront_porch;
> + u32 hback_porch;
> + u32 hsync_len;
> +
> + u32 vactive;
> + u32 vfront_porch;
> + u32 vback_porch;
> + u32 vsync_len;
> +
> + u32 hah;
> + u32 vah;
> + bool interlaced;
> + bool doublescan;
> +};
> +
> +int videomode_from_timing(struct display_timings *disp, struct videomode
> *vm,
> + int index);
> +#endif
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH] video: exynos_dp: Fix bug when checking dp->irq
From: Sean Paul @ 2012-10-31 17:15 UTC (permalink / raw)
To: linux-fbdev
Fix a bug where we check !dp->irq instead of the correct check for
-ENXIO.
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
---
drivers/video/exynos/exynos_dp_core.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index f761ec2..39271eb 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -882,7 +882,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
}
dp->irq = platform_get_irq(pdev, 0);
- if (!dp->irq) {
+ if (dp->irq = -ENXIO) {
dev_err(&pdev->dev, "failed to get irq\n");
return -ENODEV;
}
--
1.7.7.3
^ permalink raw reply related
* [PATCH] video: exynos_dp: Improve EDID error handling
From: Sean Paul @ 2012-10-31 17:13 UTC (permalink / raw)
To: linux-fbdev
EDID error handling has 2 problems:
- It doesn't fail as early as it can
- The retry counts for i2c and aux transactions are huge
This patch fails if the initial i2c transaction fails, and reduces the
aux and i2c retry counts down to 3.
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/video/exynos/exynos_dp_core.c | 13 ++++++++-----
drivers/video/exynos/exynos_dp_reg.c | 9 ++++-----
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index ef9b003..f761ec2 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -90,9 +90,11 @@ static int exynos_dp_read_edid(struct exynos_dp_device *dp)
*/
/* Read Extension Flag, Number of 128-byte EDID extension blocks */
- exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
+ retval = exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
EDID_EXTENSION_FLAG,
&extend_block);
+ if (retval)
+ return retval;
if (extend_block > 0) {
dev_dbg(dp->dev, "EDID data includes a single extension!\n");
@@ -181,14 +183,15 @@ static int exynos_dp_handle_edid(struct exynos_dp_device *dp)
int retval;
/* Read DPCD DPCD_ADDR_DPCD_REV~RECEIVE_PORT1_CAP_1 */
- exynos_dp_read_bytes_from_dpcd(dp,
- DPCD_ADDR_DPCD_REV,
- 12, buf);
+ retval = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_DPCD_REV, 12,
+ buf);
+ if (retval)
+ return retval;
/* Read EDID */
for (i = 0; i < 3; i++) {
retval = exynos_dp_read_edid(dp);
- if (retval = 0)
+ if (!retval)
break;
}
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 3f5ca8a..9a939aa 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -552,7 +552,7 @@ int exynos_dp_write_bytes_to_dpcd(struct exynos_dp_device *dp,
else
cur_data_count = count - start_offset;
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Select DPCD device address */
reg = AUX_ADDR_7_0(reg_addr + start_offset);
writel(reg, dp->reg_base + EXYNOS_DP_AUX_ADDR_7_0);
@@ -617,7 +617,7 @@ int exynos_dp_read_bytes_from_dpcd(struct exynos_dp_device *dp,
cur_data_count = count - start_offset;
/* AUX CH Request Transaction process */
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Select DPCD device address */
reg = AUX_ADDR_7_0(reg_addr + start_offset);
writel(reg, dp->reg_base + EXYNOS_DP_AUX_ADDR_7_0);
@@ -700,7 +700,7 @@ int exynos_dp_read_byte_from_i2c(struct exynos_dp_device *dp,
int i;
int retval;
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Clear AUX CH data buffer */
reg = BUF_CLR;
writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
@@ -708,7 +708,6 @@ int exynos_dp_read_byte_from_i2c(struct exynos_dp_device *dp,
/* Select EDID device */
retval = exynos_dp_select_i2c_device(dp, device_addr, reg_addr);
if (retval != 0) {
- dev_err(dp->dev, "Select EDID device fail!\n");
continue;
}
@@ -750,7 +749,7 @@ int exynos_dp_read_bytes_from_i2c(struct exynos_dp_device *dp,
int retval = 0;
for (i = 0; i < count; i += 16) {
- for (j = 0; j < 100; j++) {
+ for (j = 0; j < 3; j++) {
/* Clear AUX CH data buffer */
reg = BUF_CLR;
writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
--
1.7.7.3
^ permalink raw reply related
* Re: [PATCH v7 1/8] video: add display_timing struct and helpers
From: Laurent Pinchart @ 2012-10-31 17:09 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Thierry Reding, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-2-git-send-email-s.trumtrar@pengutronix.de>
Hi Steffen,
One more comment.
On Wednesday 31 October 2012 10:28:01 Steffen Trumtrar wrote:
> Add display_timing structure and the according helper functions. This allows
> the description of a display via its supported timing parameters.
>
> Every timing parameter can be specified as a single value or a range
> <min typ max>.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
> drivers/video/Kconfig | 5 +++
> drivers/video/Makefile | 1 +
> drivers/video/display_timing.c | 24 ++++++++++++++
> include/linux/display_timing.h | 69 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 99 insertions(+)
> create mode 100644 drivers/video/display_timing.c
> create mode 100644 include/linux/display_timing.h
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index d08d799..1421fc8 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -33,6 +33,11 @@ config VIDEO_OUTPUT_CONTROL
> This framework adds support for low-level control of the video
> output switch.
>
> +config DISPLAY_TIMING
> + bool "Enable display timings helpers"
> + help
> + Say Y here, to use the display timing helpers.
> +
> menuconfig FB
> tristate "Support for frame buffer devices"
> ---help---
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 23e948e..552c045 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -167,3 +167,4 @@ obj-$(CONFIG_FB_VIRTUAL) += vfb.o
>
> #video output switch sysfs driver
> obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
> +obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
> diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
> new file mode 100644
> index 0000000..9ccfdb3
> --- /dev/null
> +++ b/drivers/video/display_timing.c
> @@ -0,0 +1,24 @@
> +/*
> + * generic display timing functions
> + *
> + * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>,
> Pengutronix + *
> + * This file is released under the GPLv2
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/display_timing.h>
> +
> +void timings_release(struct display_timings *disp)
> +{
> + int i;
> +
> + for (i = 0; i < disp->num_timings; i++)
> + kfree(disp->timings[i]);
> +}
This function doesn't seem to be called externally, you can make it static.
> +void display_timings_release(struct display_timings *disp)
> +{
> + timings_release(disp);
> + kfree(disp->timings);
> +}
> diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h
> new file mode 100644
> index 0000000..aa02a12
> --- /dev/null
> +++ b/include/linux/display_timing.h
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * description of display timings
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_DISPLAY_TIMINGS_H
> +#define __LINUX_DISPLAY_TIMINGS_H
> +
> +#include <linux/types.h>
> +
> +struct timing_entry {
> + u32 min;
> + u32 typ;
> + u32 max;
> +};
> +
> +struct display_timing {
> + struct timing_entry pixelclock;
> +
> + struct timing_entry hactive;
> + struct timing_entry hfront_porch;
> + struct timing_entry hback_porch;
> + struct timing_entry hsync_len;
> +
> + struct timing_entry vactive;
> + struct timing_entry vfront_porch;
> + struct timing_entry vback_porch;
> + struct timing_entry vsync_len;
> +
> + unsigned int vsync_pol_active;
> + unsigned int hsync_pol_active;
> + unsigned int de_pol_active;
> + unsigned int pixelclk_pol;
> + bool interlaced;
> + bool doublescan;
> +};
> +
> +struct display_timings {
> + unsigned int num_timings;
> + unsigned int native_mode;
> +
> + struct display_timing **timings;
> +};
> +
> +/* placeholder function until ranges are really needed */
> +static inline u32 display_timing_get_value(struct timing_entry *te, int
> index) +{
> + return te->typ;
> +}
> +
> +static inline struct display_timing *display_timings_get(struct
> display_timings *disp, + int index)
> +{
> + struct display_timing *dt;
> +
> + if (disp->num_timings > index) {
> + dt = disp->timings[index];
> + return dt;
> + } else
> + return NULL;
> +}
> +void timings_release(struct display_timings *disp);
> +void display_timings_release(struct display_timings *disp);
> +
> +#endif
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 05/10] video: exynos_dp: Remove sink control to D0
From: Sean Paul @ 2012-10-31 17:05 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344398064-13563-6-git-send-email-seanpaul@chromium.org>
On Mon, Aug 20, 2012 at 5:19 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote:
>>
>> Don't reset the sink power to D0. Removing this for three reasons:
>>
>> 1) It's not required in the SW link training documentation
> OK.
>> 2) The comment is incorrect, D0 is normal operation, not "power down"
> OK. As you mentioned, D0 is not 'power down', 'normal operation'.
> However, if comment is wrong, usually we fix comment, not remove it. :)
>
The datasheet doesn't mention this step at all. Furthermore, the
comment explains how it *should* work, so I think it's the code that
is in error. Do you know why this code is needed?
>> 3) It seems to change things in the link training that causes glitches
>
> Um, it seems that this problem depends on LCD panel.
> Other LCDs that I have tested do not have this kind of problem.
> Please, modify this comment.
>
That was a red herring, it didn't cause the glitches, just made them
less obvious.
Sean
>>
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> ---
>> drivers/video/exynos/exynos_dp_core.c | 6 ------
>> 1 files changed, 0 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
>> index 207bd7e..1c998d9 100644
>> --- a/drivers/video/exynos/exynos_dp_core.c
>> +++ b/drivers/video/exynos/exynos_dp_core.c
>> @@ -273,12 +273,6 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
>> for (lane = 0; lane < lane_count; lane++)
>> dp->link_train.cr_loop[lane] = 0;
>>
>> - /* Set sink to D0 (Sink Not Ready) mode. */
>> - ret = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
>> - DPCD_SET_POWER_STATE_D0);
>> - if (ret)
>> - return ret;
>> -
>> /* Set link rate and count as you want to establish*/
>> exynos_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
>> exynos_dp_set_lane_count(dp, dp->link_train.lane_count);
>> --
>> 1.7.7.3
>
^ permalink raw reply
* Re: [PATCH v7 1/8] video: add display_timing struct and helpers
From: Laurent Pinchart @ 2012-10-31 17:04 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Thierry Reding, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-2-git-send-email-s.trumtrar@pengutronix.de>
Hi Steffen,
Thanks for the patch.
As we'll need a v8 anyway due to the comment on patch 5/8, here are a couple
of other small comments.
On Wednesday 31 October 2012 10:28:01 Steffen Trumtrar wrote:
> Add display_timing structure and the according helper functions. This allows
> the description of a display via its supported timing parameters.
>
> Every timing parameter can be specified as a single value or a range
> <min typ max>.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
> drivers/video/Kconfig | 5 +++
> drivers/video/Makefile | 1 +
> drivers/video/display_timing.c | 24 ++++++++++++++
> include/linux/display_timing.h | 69 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 99 insertions(+)
> create mode 100644 drivers/video/display_timing.c
> create mode 100644 include/linux/display_timing.h
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index d08d799..1421fc8 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -33,6 +33,11 @@ config VIDEO_OUTPUT_CONTROL
> This framework adds support for low-level control of the video
> output switch.
>
> +config DISPLAY_TIMING
> + bool "Enable display timings helpers"
> + help
> + Say Y here, to use the display timing helpers.
> +
> menuconfig FB
> tristate "Support for frame buffer devices"
> ---help---
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 23e948e..552c045 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -167,3 +167,4 @@ obj-$(CONFIG_FB_VIRTUAL) += vfb.o
>
> #video output switch sysfs driver
> obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
> +obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
> diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
> new file mode 100644
> index 0000000..9ccfdb3
> --- /dev/null
> +++ b/drivers/video/display_timing.c
> @@ -0,0 +1,24 @@
> +/*
> + * generic display timing functions
> + *
> + * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>,
> Pengutronix + *
> + * This file is released under the GPLv2
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/display_timing.h>
I try to keep #include's sorted alphabetically, but I won't push for that.
> +void timings_release(struct display_timings *disp)
> +{
> + int i;
> +
> + for (i = 0; i < disp->num_timings; i++)
disp->num_timings is an unsigned int, i should be an unsigned int as well to
avoid signed vs. unsigned comparisons.
> + kfree(disp->timings[i]);
> +}
> +
> +void display_timings_release(struct display_timings *disp)
> +{
> + timings_release(disp);
> + kfree(disp->timings);
> +}
> diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h
> new file mode 100644
> index 0000000..aa02a12
> --- /dev/null
> +++ b/include/linux/display_timing.h
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * description of display timings
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_DISPLAY_TIMINGS_H
> +#define __LINUX_DISPLAY_TIMINGS_H
> +
> +#include <linux/types.h>
> +
> +struct timing_entry {
> + u32 min;
> + u32 typ;
> + u32 max;
> +};
> +
> +struct display_timing {
> + struct timing_entry pixelclock;
> +
> + struct timing_entry hactive;
> + struct timing_entry hfront_porch;
> + struct timing_entry hback_porch;
> + struct timing_entry hsync_len;
> +
> + struct timing_entry vactive;
> + struct timing_entry vfront_porch;
> + struct timing_entry vback_porch;
> + struct timing_entry vsync_len;
> +
> + unsigned int vsync_pol_active;
> + unsigned int hsync_pol_active;
> + unsigned int de_pol_active;
> + unsigned int pixelclk_pol;
> + bool interlaced;
> + bool doublescan;
> +};
> +
> +struct display_timings {
> + unsigned int num_timings;
> + unsigned int native_mode;
> +
> + struct display_timing **timings;
> +};
> +
> +/* placeholder function until ranges are really needed */
> +static inline u32 display_timing_get_value(struct timing_entry *te, int
> index)
What is the index parameter for ?
> +{
> + return te->typ;
> +}
> +
> +static inline struct display_timing *display_timings_get(struct
> display_timings *disp,
> + int index)
> +{
> + struct display_timing *dt;
> +
> + if (disp->num_timings > index) {
index should be an unsigned int for the same reason as above.
> + dt = disp->timings[index];
> + return dt;
Maybe just
return disp->timings[index];
?
> + } else
> + return NULL;
> +}
> +
> +void timings_release(struct display_timings *disp);
> +void display_timings_release(struct display_timings *disp);
> +
> +#endif
--
Regards,
Laurent Pinchart
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox