* Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Archit Taneja @ 2012-11-02 11:56 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <5093AE79.9010603@ti.com>
On Friday 02 November 2012 04:58 PM, Tomi Valkeinen wrote:
> On 2012-11-02 13:09, Archit Taneja wrote:
>> On Friday 02 November 2012 04:19 PM, Tomi Valkeinen wrote:
>>> On 2012-11-02 12:44, Archit Taneja wrote:
>>>
>>>> Hmm, that makes sense. Anyway, I don't think it's really bad if we refer
>>>> to dssdev->channel for now.
>>>
>>> It is, because dssdev->channel doesn't exist with DT.
>>>
>>> With DT we either need to figure out the channel in omapdss at runtime,
>>> or add a property to the DT data telling the channel. And adding such a
>>> property is not correct, as DT should be about describing the HW.
>>
>> Ok.
>>
>> I don't totally agree with your idea of figuring out the manager in
>> panel the panel's probe. If it's done in the panel driver's probe
>> itself, then by this point of time we have already set
>> mgr->output->device links. If omapdss only does this stuff, then
>
> Hmm, I'm not sure I understand what's your point above? If figuring out
> the mgr is done in panel's probe, the mgr->output link is not yet made
> before that time.
My point is that we are trying to find a manager at panel's probe
itself. It think that's what we do now. But one of your recent patch
moves that to omapfb.
>
>> omapfb/omapdrm have just the job of connecting the overlays to the
>> manager. Do you think that's okay?
>
> Yes, that's how I think it should be. I don't see why omapfb/omapdrm
> should care about which manager is being used for the output, it doesn't
> really matter as long there is one and it works.
>
> Then again, I don't have anything against omapfb/omapdrm choosing the
> manager, but I don't see how they would have any better idea of which
> manager to use than omapdss.
>
> But as doing the connections at probe time is a bit problematic, perhaps
> we should have a new step in this whole sequence. Something like
> "connect" or whatever, which would lock the required blocks in the whole
> pipeline, and acquire the required resources that couldn't be gotten at
> probe time.
>
> But even then, choosing the manager is not easy, as whoever chooses the
> manager needs to observe all the possible displays used at the same time...
Right. I was wondering if omapfb/omapdrm could understand the 'all
possible displays information' better compared to a panel's probe.
Even omapdrm/omafb can't be perfect because we could insert a panel
driver module at any time, and omapfb/omapdrm may miss that out.
Archit
^ permalink raw reply
* Re: [Qemu-devel] [PATCH] add bochs dispi interface framebuffer driver
From: Vasilis Liaskovitis @ 2012-11-02 11:53 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: linux-fbdev, qemu-devel
In-Reply-To: <5092797B.5090403@redhat.com>
On Thu, Nov 01, 2012 at 02:30:35PM +0100, Gerd Hoffmann wrote:
> 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?
vt-switching with vesafb/X11 works fine on a grml 64-bit image. However, xorg
uses vesa driver in this case, not fbdev (fbdev / fbdevhw xorg modules are
initially loaded but then unloaded). X11 uses 1280x768 and vesafb uses 1024x768
according to dmesg.
But i haven't been able to test ubuntu+vesafb. Ubuntu kernels use efifb
(CONFIG_FB_EFI=y) and fbconsoles don't work at all with this driver +
qemu/seabios/vgastd.
I have tried using a custom kernel (CONFIG_FB_EFI not set, CONFIG_FB_VESA=y) but
for some reason I can't load vesafb on ubuntu desktop. No fb drivers are
blacklisted, but no fb driver is loaded if I specify a vga text mode with "vga="
in the kernel command line. X11 still uses 1280x768 resolution here.
Anyway, these are screenshots of the original problem (messed up output with
bochsfb + fbdev-xorg on ubuntu 12.04 startup):
vt7 http://picpaste.de/bochsfb-badstart-AirrXZuF.png
vt1 http://www.picpaste.de/bochsfb-badstart-f1-EO10MVdF.png
it still happens with the latest bochsfb driver (tested with 3.6.0 though, not
3.7.0-rc3 yet)
>
> > 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.
I assume bochsfb is the way we want to go. I can send more detailed info on the
uvesafb issue if needed.
thanks,
- Vasilis
^ permalink raw reply
* Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Tomi Valkeinen @ 2012-11-02 11:28 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <5093A9E8.70106@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1948 bytes --]
On 2012-11-02 13:09, Archit Taneja wrote:
> On Friday 02 November 2012 04:19 PM, Tomi Valkeinen wrote:
>> On 2012-11-02 12:44, Archit Taneja wrote:
>>
>>> Hmm, that makes sense. Anyway, I don't think it's really bad if we refer
>>> to dssdev->channel for now.
>>
>> It is, because dssdev->channel doesn't exist with DT.
>>
>> With DT we either need to figure out the channel in omapdss at runtime,
>> or add a property to the DT data telling the channel. And adding such a
>> property is not correct, as DT should be about describing the HW.
>
> Ok.
>
> I don't totally agree with your idea of figuring out the manager in
> panel the panel's probe. If it's done in the panel driver's probe
> itself, then by this point of time we have already set
> mgr->output->device links. If omapdss only does this stuff, then
Hmm, I'm not sure I understand what's your point above? If figuring out
the mgr is done in panel's probe, the mgr->output link is not yet made
before that time.
> omapfb/omapdrm have just the job of connecting the overlays to the
> manager. Do you think that's okay?
Yes, that's how I think it should be. I don't see why omapfb/omapdrm
should care about which manager is being used for the output, it doesn't
really matter as long there is one and it works.
Then again, I don't have anything against omapfb/omapdrm choosing the
manager, but I don't see how they would have any better idea of which
manager to use than omapdss.
But as doing the connections at probe time is a bit problematic, perhaps
we should have a new step in this whole sequence. Something like
"connect" or whatever, which would lock the required blocks in the whole
pipeline, and acquire the required resources that couldn't be gotten at
probe time.
But even then, choosing the manager is not easy, as whoever chooses the
manager needs to observe all the possible displays used at the same time...
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Archit Taneja @ 2012-11-02 11:21 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <5093A530.5050302@ti.com>
On Friday 02 November 2012 04:19 PM, Tomi Valkeinen wrote:
> On 2012-11-02 12:44, Archit Taneja wrote:
>
>> Hmm, that makes sense. Anyway, I don't think it's really bad if we refer
>> to dssdev->channel for now.
>
> It is, because dssdev->channel doesn't exist with DT.
>
> With DT we either need to figure out the channel in omapdss at runtime,
> or add a property to the DT data telling the channel. And adding such a
> property is not correct, as DT should be about describing the HW.
Ok.
I don't totally agree with your idea of figuring out the manager in
panel the panel's probe. If it's done in the panel driver's probe
itself, then by this point of time we have already set
mgr->output->device links. If omapdss only does this stuff, then
omapfb/omapdrm have just the job of connecting the overlays to the
manager. Do you think that's okay?
Archit
^ permalink raw reply
* Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Archit Taneja @ 2012-11-02 10:56 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <50939B84.6090602@ti.com>
On Friday 02 November 2012 03:38 PM, Tomi Valkeinen wrote:
> On 2012-10-31 09:26, Archit Taneja wrote:
>> On Tuesday 30 October 2012 09:40 PM, Tomi Valkeinen wrote:
>
>>> - if (dpi_use_dsi_pll(dssdev)) {
>>> - enum omap_dss_clk_source dispc_fclk_src >>> - dssdev->clocks.dispc.dispc_fclk_src;
>>> - dpi.dsidev = dpi_get_dsidev(dispc_fclk_src);
>>> + /*
>>> + * XXX We shouldn't need dssdev->channel for this. The dsi pll clock
>>> + * source for DPI is SoC integration detail, not something that
>>> should
>>> + * be configured in the dssdev
>>> + */
>>
>> It is SoC integration detail, but it's flexible, it depends on which
>> manager is connected to DPI output. If it's connected to LCD1, the
>
> Hmm, yes, the comment is a bit misleading. The DSI PLL is not used for
> DPI, but for DISPC's LCD output. And DPI uses one of those LCD outputs.
>
>> source can be DSI1 PLL, if it's LCD2, it's source can be DSI2 PLL, if
>> it's connected to TV manager, it's source "has to be" HDMI PLL. And
>> these connections vary based on which OMAP revision we are on. We can
>> only be certain on OMAP3 that the source for DPI pixel clock can be
>> either PRCM or DSI PLL.
>
> On OMAP2 we can be certain the clock is PRCM =).
>
>> At the point of probe, we really don't know which manager is the DPI
>> output connected to. Hence, we sort of use dssdev->channel to make a
>> guess what manager we connect to in the future.
>
> Yep. My point was mainly that dssdev needs to go away, and we should
> somehow decide the used channel internally.
>
>> The right approach would be to figure this out at the time of enable,
>> where we know which manager the DPI output is connected to. We could
>> probably move the verification there too.
>
> Who chooses which manager to use for DPI?
If you are asking in terms of HW. The value in the DSS_CTRL bitfield
decides which manager to use.
If you meant who/how should we choose this in software, then I don't
know either.
>
> I'm not sure... I would really like to manage the basic setup, acquiring
> the resources, etc. at probe time, and enable would only enable the display.
One thing we could do is to grab all the possible resources that DPI can
use for its pixel clock, and when it's enable time, see what all options
it has. So, for example, for DPI on omap4, we could try to grab and
verify all DSI PLL, HDMI PLL and PRCM. And then later choose the most
appropriate one while enabling.
>
> That means that we should somehow get a manager for DPI at probe time,
> which may be a bit difficult, as we don't know what other displays there
> will be. So if, say, DPI can use LCD1 or LCD2, but DSI can only use
> LCD2, and at DPI's probe (presuming it's before DSI) we pick LCD2, DSI
> won't work.
Yes, it's not easy to know this at probe time. We could try to allocate
resources at the time mgr->set_output() is called. We could have an
output specific op. dss_mgr_set_output() could look like:
dss_mgr_set_output(mgr, output)
{
/* Do the older stuff */
...
...
output->get_and_config_resources(output);
}
In dpi.c:
dss_dpi_get_and_config_resources(output)
{
switch (output->manager->id) {
case LCD:
Get DSI 1 PLL;
case LCD2:
Get DSI2 PLL;
case TV:
Get HDMI PLL;
}
/*
* Also set the DSS_CTRL bits here to tell which manager
* we need to connect to
*/
dss_select_dpi_manager(output->manager->id);
}
omapdss_output_ops dpi_ops = {
.get_and_config_resources = dss_dpi_get_and_config_resources,
...
};
However, even though this approach might be correct in the sense that we
confiugre dpi when we know what manager we are connected to, I have to
say that its not nice at all. Especially because setting manager output
links is very omapdss-apply-ish. But I feel we would need to do
something similar in omapdrm too.
>
> Anyway, while I'm not sure how to solve the above problem, I think we
> should improve our init a bit. For DPI there are the following steps
> done, in order:
>
> - DPI device added
> - DPI driver probe
> - DPI panel device added
> - DPI panel driver probe
>
> We currently add the panel device in DPI driver's probe, and figure out
> the DSI PLL at the same time. I think that should happen only when the
> panel driver probe happens. The panel driver should call something like
> dpi_get_output() or whatever, which acquires the DPI bus for the panel
> driver, and this would probably also choose the manager.
Hmm, that makes sense. Anyway, I don't think it's really bad if we refer
to dssdev->channel for now.
I think we could have a clearer picture of this when we understand how
omapdrm sets the links between its entities and how CPF would link the
output-panel side of things.
Archit
^ permalink raw reply
* Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Tomi Valkeinen @ 2012-11-02 10:49 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <5093A3FB.1060806@ti.com>
[-- Attachment #1: Type: text/plain, Size: 437 bytes --]
On 2012-11-02 12:44, Archit Taneja wrote:
> Hmm, that makes sense. Anyway, I don't think it's really bad if we refer
> to dssdev->channel for now.
It is, because dssdev->channel doesn't exist with DT.
With DT we either need to figure out the channel in omapdss at runtime,
or add a property to the DT data telling the channel. And adding such a
property is not correct, as DT should be about describing the HW.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Tomi Valkeinen @ 2012-11-02 10:08 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <5090D28E.6050703@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2827 bytes --]
On 2012-10-31 09:26, Archit Taneja wrote:
> On Tuesday 30 October 2012 09:40 PM, Tomi Valkeinen wrote:
>> - if (dpi_use_dsi_pll(dssdev)) {
>> - enum omap_dss_clk_source dispc_fclk_src =
>> - dssdev->clocks.dispc.dispc_fclk_src;
>> - dpi.dsidev = dpi_get_dsidev(dispc_fclk_src);
>> + /*
>> + * XXX We shouldn't need dssdev->channel for this. The dsi pll clock
>> + * source for DPI is SoC integration detail, not something that
>> should
>> + * be configured in the dssdev
>> + */
>
> It is SoC integration detail, but it's flexible, it depends on which
> manager is connected to DPI output. If it's connected to LCD1, the
Hmm, yes, the comment is a bit misleading. The DSI PLL is not used for
DPI, but for DISPC's LCD output. And DPI uses one of those LCD outputs.
> source can be DSI1 PLL, if it's LCD2, it's source can be DSI2 PLL, if
> it's connected to TV manager, it's source "has to be" HDMI PLL. And
> these connections vary based on which OMAP revision we are on. We can
> only be certain on OMAP3 that the source for DPI pixel clock can be
> either PRCM or DSI PLL.
On OMAP2 we can be certain the clock is PRCM =).
> At the point of probe, we really don't know which manager is the DPI
> output connected to. Hence, we sort of use dssdev->channel to make a
> guess what manager we connect to in the future.
Yep. My point was mainly that dssdev needs to go away, and we should
somehow decide the used channel internally.
> The right approach would be to figure this out at the time of enable,
> where we know which manager the DPI output is connected to. We could
> probably move the verification there too.
Who chooses which manager to use for DPI?
I'm not sure... I would really like to manage the basic setup, acquiring
the resources, etc. at probe time, and enable would only enable the display.
That means that we should somehow get a manager for DPI at probe time,
which may be a bit difficult, as we don't know what other displays there
will be. So if, say, DPI can use LCD1 or LCD2, but DSI can only use
LCD2, and at DPI's probe (presuming it's before DSI) we pick LCD2, DSI
won't work.
Anyway, while I'm not sure how to solve the above problem, I think we
should improve our init a bit. For DPI there are the following steps
done, in order:
- DPI device added
- DPI driver probe
- DPI panel device added
- DPI panel driver probe
We currently add the panel device in DPI driver's probe, and figure out
the DSI PLL at the same time. I think that should happen only when the
panel driver probe happens. The panel driver should call something like
dpi_get_output() or whatever, which acquires the DPI bus for the panel
driver, and this would probably also choose the manager.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [PATCH] video: exynos_dp: Clean up SW link training
From: Jingoo Han @ 2012-11-02 0:48 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1351702475-31324-1-git-send-email-seanpaul@chromium.org>
On Friday, November 02, 2012 1:15 AM Sean Paul wrote
>
> 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.
No, you changed the procedure when exynos_dp_clock_recovery_ok() fails.
It is not the same with exynos_dp_get_adjust_training_lane().
So, the else path at exynos_dp_process_clock_recovery() should not be
changed.
>
> 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.
Yes, I know.
But, most panels does NOT make the problem,
even though there is a bug.
With your panel, exynos_dp_clock_recovery_ok() does not fail,
so, the problem does NOT happen.
>
> > 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.
No, it is not same.
1) your patch
exynos_dp_read_bytes_from_dpcd(dp,
DPCD_ADDR_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
...
} else {
for (lane = 0; lane < lane_count; lane++) {
training_lane = exynos_dp_get_lane_link_training()
voltage_swing = exynos_dp_get_adjust_request_voltage()
pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis()
...
2) my patch
if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
...
} else {
for (lane = 0; lane < lane_count; lane++) {
training_lane = exynos_dp_get_lane_link_training()
exynos_dp_read_bytes_from_dpcd(dp,
DPCD_ADDR_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
voltage_swing = exynos_dp_get_adjust_request_voltage()
pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis()
...
When the place of reading DPCD_ADDR_ADJUST_REQUEST_LANE0_1 is changed,
it makes the Link Fail problem when exynos_dp_clock_recovery_ok() fails.
In the case of most panels, exynos_dp_clock_recovery_ok() does not fail.
But, some panels failed at exynos_dp_clock_recovery_ok(), and makes the problem
if your patch is used.
Also, your patch calls exynos_dp_get_adjust_request_voltage() and
exynos_dp_get_adjust_request_pre_emphasis() twice,
when exynos_dp_clock_recovery_ok() fails. Previously, exynos_dp_clock_recovery_ok()
and exynos_dp_get_adjust_request_pre_emphasis() are called only onetime
when exynos_dp_clock_recovery_ok() fails.
There is no need to call exynos_dp_get_adjust_request_voltage() and
exynos_dp_get_adjust_request_pre_emphasis() 'TWICE'.
Anyway, your patch is good and readability is improved.
So, I went the extra mile to accept your original patch and
add it to v3 patch that I sent.
(http://www.spinics.net/lists/linux-fbdev/msg08548.html)
But, it is necessary to be careful with some error paths,
that is not used at most eDP panels.
Best regards,
Jingoo Han
> 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: [PATCH v7 3/8] of: add generic videomode description
From: Thierry Reding @ 2012-11-01 20:17 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Philipp Zabel, Rob Herring, linux-fbdev,
dri-devel, Laurent Pinchart, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-4-git-send-email-s.trumtrar@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 362 bytes --]
On Wed, Oct 31, 2012 at 10:28:03AM +0100, Steffen Trumtrar wrote:
[...]
> +config OF_VIDEOMODE
> + def_bool y
> + depends on VIDEOMODE
> + help
> + helper to get videomodes from the devicetree
> +
I think patches 3 and 4 need to be swapped, since patch 4 introduces the
VIDEOMODE Kconfig symbol (as well as the videomode.h helper) that patch
3 uses.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v7 2/8] of: add helper to parse display timings
From: Thierry Reding @ 2012-11-01 20:15 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-3-git-send-email-s.trumtrar@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]
On Wed, Oct 31, 2012 at 10:28:02AM +0100, Steffen Trumtrar wrote:
[...]
> diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt
[...]
> @@ -0,0 +1,139 @@
> +display-timings bindings
> +==================
> +
> +display-timings-node
> +------------
Maybe extend the underline to the length of the section and subsection
titles respectively?
> +struct display_timing
> +===================
Same here.
> +config OF_DISPLAY_TIMINGS
> + def_bool y
> + depends on DISPLAY_TIMING
Maybe this should be called OF_DISPLAY_TIMING to match DISPLAY_TIMING,
or rename DISPLAY_TIMING to DISPLAY_TIMINGS for the sake of consistency?
> +/**
> + * of_get_display_timing_list - parse all display_timing entries from a device_node
> + * @np: device_node with the subnodes
> + **/
> +struct display_timings *of_get_display_timing_list(struct device_node *np)
Perhaps this would better be named of_get_display_timings() to match the
return type?
> + disp = kzalloc(sizeof(*disp), GFP_KERNEL);
Shouldn't you be checking this for allocation failures?
> + disp->timings = kzalloc(sizeof(struct display_timing *)*disp->num_timings,
> + GFP_KERNEL);
Same here.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v7 1/8] video: add display_timing struct and helpers
From: Thierry Reding @ 2012-11-01 20:08 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1351675689-26814-2-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]
On Wed, Oct 31, 2012 at 10:28:01AM +0100, Steffen Trumtrar wrote:
[...]
> +void timings_release(struct display_timings *disp)
> +{
> + int i;
> +
> + for (i = 0; i < disp->num_timings; i++)
> + kfree(disp->timings[i]);
> +}
> +
> +void display_timings_release(struct display_timings *disp)
> +{
> + timings_release(disp);
> + kfree(disp->timings);
> +}
I'm not quite sure I understand how these are supposed to be used. The
only use-case where a struct display_timings is dynamically allocated is
for the OF helpers. In that case, wouldn't it be more useful to have a
function that frees the complete structure, including the struct
display_timings itself? Something like this, which has all of the above
rolled into one:
void display_timings_free(struct display_timings *disp)
{
if (disp->timings) {
unsigned int i;
for (i = 0; i < disp->num_timings; i++)
kfree(disp->timings[i]);
}
kfree(disp->timings);
kfree(disp);
}
Is there a use-case where a struct display_timings is not dynamically
allocated? The only one I can think of is where it is defined as
platform data, but in that case you don't want to be calling
display_timing_release() on it anyway.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v7 2/8] of: add helper to parse display timings
From: Stephen Warren @ 2012-11-01 17:52 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
linux-media, Tomi Valkeinen, kernel
In-Reply-To: <1351675689-26814-3-git-send-email-s.trumtrar@pengutronix.de>
On 10/31/2012 03:28 AM, Steffen Trumtrar wrote:
Patch description? The patch defines the DT binding as well, which isn't
mentioned in the patch subject.
> new file mode 100644
> index 0000000..04c94a3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/display-timings.txt
> +Usage in backend
> +========
Everything before this point in the binding docs looks reasonable to me.
Everything after this point is Linux-specific/internal implementation
detail, and hence shouldn't be in the binding document.
I only read the DT binding.
^ permalink raw reply
* 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
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