[PATCH 4/4] ARM: dts: Use OF graph for DP to panel connection in exynos5800-peach-pi
Hello Krzysztof, On 12/07/2015 09:48 PM, Krzysztof Kozlowski wrote: > On 08.12.2015 00:36, Inki Dae wrote: >> Hi Javier, >> >> 2015-12-07 22:41 GMT+09:00 Javier Martinez Canillas > osg.samsung.com>: >>> Hello Inki, >>> >>> On 12/07/2015 09:52 AM, Inki Dae wrote: From: Javier Martinez Canillas >>> >>> Thanks a lot for posting this patch. >>> The DT binding for the Exynos DRM Display Port (DP) driver isn't consistent since it uses a phandle to describe the connection between the DP port and the display panel but uses the OF graph ports and endpoints to describe the connection betwen the DP port, a bridge chip and the panel. The Exynos DP driver and the DT binding have been changed to allow also to describe the DP port to panel connection using ports / endpoints (OF graph) so this patch changes the Exynos5800 Peach Pi DT to make it consistent with the Exynos5420 Peach Pit that has a eDP to LVDS chip and uses OF graph too. Signed-off-by: Javier Martinez Canillas Tested-by: Javier Martinez Canillas >>> >>> This tag was not in my original patch, it's true that I tested >>> it but will someone believe me? ;) >> >> Oops. I confused you spread Reviewed-by and Tested-by here and there. >> Don't worry about that. Will remove it if you don't give me Tested-by. >> :) > > Actually authorship (the "From") in this case means Tested-by. Author > always tests the patch so it would look weird if we start adding > tested-by to our own patches, right? > Exactly, that's what I tried to say. It's implied that the author tested her/his own patch in the best possible way. > Dear Inki, > However the patch misses your SoB. You touched and sent it so please > extend the SoB chain-of-blame. > Right, I missed that. > Best regards, > Krzysztof > Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America
[PATCH 4/4] ARM: dts: Use OF graph for DP to panel connection in exynos5800-peach-pi
Hello Krzysztof, On 12/07/2015 09:45 PM, Krzysztof Kozlowski wrote: > On 07.12.2015 21:52, Inki Dae wrote: >> From: Javier Martinez Canillas >> >> The DT binding for the Exynos DRM Display Port (DP) driver isn't consistent >> since it uses a phandle to describe the connection between the DP port and >> the display panel but uses the OF graph ports and endpoints to describe the >> connection betwen the DP port, a bridge chip and the panel. >> >> The Exynos DP driver and the DT binding have been changed to allow also to >> describe the DP port to panel connection using ports / endpoints (OF graph) >> so this patch changes the Exynos5800 Peach Pi DT to make it consistent with >> the Exynos5420 Peach Pit that has a eDP to LVDS chip and uses OF graph too. >> >> Signed-off-by: Javier Martinez Canillas >> Tested-by: Javier Martinez Canillas >> Reviewed-by: Inki Dae >> --- >> arch/arm/boot/dts/exynos5800-peach-pi.dts | 15 ++- >> 1 file changed, 14 insertions(+), 1 deletion(-) >> > > Looks sensible: > Reviewed-by: Krzysztof Kozlowski > > Dependencies are not mentioned, does it depend on patch 1? > Yes, it depends on patch 1/4 so it should be merged through the Exynos DRM tree to maintain bisectability. Inki's patch maintains the DT ABI backward compatibility though so another option is to wait until the DRM change hit mainline and then pick $SUBJECT. > Best regards, > Krzysztof > Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America
[Bug 93264] Tonga VM Faults since llvm ScheduleDAGInstrs: Rework schedule graph builder.
https://bugs.freedesktop.org/show_bug.cgi?id=93264 --- Comment #2 from Andy Furniss --- Maybe there is some mesa that doesn't do it I suppose. I don't update llvm as often as mesa so could have missed that. When I noticed this I reset mesa back to where the old bug fixes went in, as I knew that used to be good. It was still bad, then I tried it on older kernels, still bad so I got back on head and started testing llvm. On mesa head the llvm bisect does seem good - I haven't had much time, but a few runs with llvm sitting on the bad were all bad and a few on the one before all good. It was only a quick test - I didn't throw cpufreq into the mix. When I saw the result of the bisect I did think red herring as it wasn't AMD - but then was slightly relieved when AMD got a mention in the commit message. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/70d8bebc/attachment.html>
[Bug 93217] [tonga] [powerplay] Radon M395X isn't initialised with the powerplay branch
https://bugs.freedesktop.org/show_bug.cgi?id=93217 Mike Lothian changed: What|Removed |Added Attachment #120392|0 |1 is obsolete|| --- Comment #7 from Mike Lothian --- Created attachment 120399 --> https://bugs.freedesktop.org/attachment.cgi?id=120399&action=edit Dmesg with patch applied -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/9a2a4d17/attachment.html>
[Bug 93217] [tonga] [powerplay] Radon M395X isn't initialised with the powerplay branch
https://bugs.freedesktop.org/show_bug.cgi?id=93217 --- Comment #6 from Alex Deucher --- Created attachment 120397 --> https://bugs.freedesktop.org/attachment.cgi?id=120397&action=edit add debugging output Please apply this patch and attach the output. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/b30e37ad/attachment.html>
[PATCH] drm/exynos: decon: remove unused variables
This patch just removes unused variables, i and ret.
Signed-off-by: Inki Dae
---
drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index edfd6e3..2ac1d4d 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -377,8 +377,6 @@ static void decon_swreset(struct decon_context *ctx)
static void decon_enable(struct exynos_drm_crtc *crtc)
{
struct decon_context *ctx = crtc->ctx;
- int ret;
- int i;
if (!test_and_clear_bit(BIT_SUSPENDED, &ctx->flags))
return;
--
1.9.1
[PATCH 4/4] ARM: dts: Use OF graph for DP to panel connection in exynos5800-peach-pi
From: Javier Martinez Canillas
The DT binding for the Exynos DRM Display Port (DP) driver isn't consistent
since it uses a phandle to describe the connection between the DP port and
the display panel but uses the OF graph ports and endpoints to describe the
connection betwen the DP port, a bridge chip and the panel.
The Exynos DP driver and the DT binding have been changed to allow also to
describe the DP port to panel connection using ports / endpoints (OF graph)
so this patch changes the Exynos5800 Peach Pi DT to make it consistent with
the Exynos5420 Peach Pit that has a eDP to LVDS chip and uses OF graph too.
Signed-off-by: Javier Martinez Canillas
Tested-by: Javier Martinez Canillas
Reviewed-by: Inki Dae
---
arch/arm/boot/dts/exynos5800-peach-pi.dts | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts
b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index 49a4f43..1cc2e95 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -122,6 +122,12 @@
compatible = "auo,b133htn01";
power-supply = <&tps65090_fet6>;
backlight = <&backlight>;
+
+ port {
+ panel_in: endpoint {
+ remote-endpoint = <&dp_out>;
+ };
+ };
};
mmc1_pwrseq: mmc1_pwrseq {
@@ -148,7 +154,14 @@
samsung,link-rate = <0x0a>;
samsung,lane-count = <2>;
samsung,hpd-gpio = <&gpx2 6 GPIO_ACTIVE_HIGH>;
- panel = <&panel>;
+
+ ports {
+ port {
+ dp_out: endpoint {
+ remote-endpoint = <&panel_in>;
+ };
+ };
+ };
};
&fimd {
--
1.9.1
[PATCH v2 3/4] dt-bindings: exynos-dp: update ports node binding for panel
This patch updates a ports node binding for panel.
With this, dp node can have a ports node which describes
a remote endpoint node that can be connected to panel or bridge
node.
Changelog v2:
- remove unnecessary properties and numbering.
- update description about eDP device.
Signed-off-by: Inki Dae
Reviewed-by: Javier Martinez Canillas
---
.../bindings/display/exynos/exynos_dp.txt | 41 +++---
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
b/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
index 64693f2..22efeba 100644
--- a/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
+++ b/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
@@ -1,3 +1,20 @@
+Device-Tree bindings for Samsung Exynos Embedded DisplayPort Transmitter(eDP)
+
+DisplayPort is industry standard to accommodate the growing board adoption
+of digital display technology within the PC and CE industries.
+It consolidates the internal and external connection methods to reduce device
+complexity and cost. It also supports necessary features for important cross
+industry applications and provides performance scalability to enable the next
+generation of displays that feature higher color depths, refresh rates, and
+display resolutions.
+
+eDP (embedded display port) device is compliant with Embedded DisplayPort
+standard as follows,
+- DisplayPort standard 1.1a for Exynos5250 and Exynos5260.
+- DisplayPort standard 1.3 for Exynos5422s and Exynos5800.
+
+eDP resides between FIMD and panel or FIMD and bridge such as LVDS.
+
The Exynos display port interface should be configured based on
the type of panel connected to it.
@@ -66,8 +83,15 @@ Optional properties for dp-controller:
Hotplug detect GPIO.
Indicates which GPIO should be used for hotplug
detection
- -video interfaces: Device node can contain video interface port
- nodes according to [1].
+Video interfaces:
+ Device node can contain video interface port nodes according to [1].
+ The following are properties specific to those nodes:
+
+ endpoint node connected to bridge or panel node:
+ - remote-endpoint: specifies the endpoint in panel or bridge node.
+ This node is required in all kinds of exynos dp
+ to represent the connection between dp and bridge
+ or dp and panel.
[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -111,9 +135,18 @@ Board Specific portion:
};
ports {
- port at 0 {
+ port {
dp_out: endpoint {
- remote-endpoint = <&bridge_in>;
+ remote-endpoint = <&dp_in>;
+ };
+ };
+ };
+
+ panel {
+ ...
+ port {
+ dp_in: endpoint {
+ remote-endpoint = <&dp_out>;
};
};
};
--
1.9.1
[PATCH v2 2/4] drm/exynos: dp: fix wrong return type
This patch fixes wrong return type when dt binding of bridge device
failed.
If a board has a bridge device then of_graph_get_remote_port_parent
function shouldn't be NULL. So this patch will return a proper error
type so that the deferred probe isn't triggered.
Changelog v2:
- return -EINVAL if getting a port node failed.
Signed-off-by: Inki Dae
Reviewed-by: Javier Martinez Canillas
---
drivers/gpu/drm/exynos/exynos_dp_core.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
b/drivers/gpu/drm/exynos/exynos_dp_core.c
index 60260a0..aeee60a 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -1437,8 +1437,10 @@ static int exynos_dp_probe(struct platform_device *pdev)
of_node_put(bridge_node);
if (!dp->ptn_bridge)
return -EPROBE_DEFER;
- } else
- return -EPROBE_DEFER;
+ } else {
+ DRM_ERROR("no port node for bridge device.\n");
+ return -EINVAL;
+ }
}
out:
--
1.9.1
[PATCH v3 1/4] drm/exynos: dp: add of_graph dt binding support for panel
This patch adds of_graph dt binding support for panel device
and also keeps the backward compatibility.
i.e.,
The dts file for Exynos5800 based peach pi board
has a panel property so we need to keep the backward compatibility.
Changelog v3:
- bind only one of two nodes outbound - panel or bridge.
Changelog v2:
- return -EINVAL if getting a port node failed.
Signed-off-by: Inki Dae
Reviewed-by: Javier Martinez Canillas
---
drivers/gpu/drm/exynos/exynos_dp_core.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
b/drivers/gpu/drm/exynos/exynos_dp_core.c
index 94f02a0..60260a0 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -1392,7 +1392,7 @@ static const struct component_ops exynos_dp_ops = {
static int exynos_dp_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct device_node *panel_node, *bridge_node, *endpoint;
+ struct device_node *panel_node = NULL, *bridge_node, *endpoint = NULL;
struct exynos_dp_device *dp;
int ret;
@@ -1403,14 +1403,32 @@ static int exynos_dp_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, dp);
+ /* This is for the backward compatibility. */
panel_node = of_parse_phandle(dev->of_node, "panel", 0);
if (panel_node) {
dp->panel = of_drm_find_panel(panel_node);
of_node_put(panel_node);
if (!dp->panel)
return -EPROBE_DEFER;
+ } else {
+ endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
+ if (endpoint) {
+ panel_node = of_graph_get_remote_port_parent(endpoint);
+ if (panel_node) {
+ dp->panel = of_drm_find_panel(panel_node);
+ of_node_put(panel_node);
+ if (!dp->panel)
+ return -EPROBE_DEFER;
+ } else {
+ DRM_ERROR("no port node for panel device.\n");
+ return -EINVAL;
+ }
+ }
}
+ if (endpoint)
+ goto out;
+
endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
if (endpoint) {
bridge_node = of_graph_get_remote_port_parent(endpoint);
@@ -1423,6 +1441,7 @@ static int exynos_dp_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
+out:
pm_runtime_enable(dev);
ret = component_add(&pdev->dev, &exynos_dp_ops);
--
1.9.1
[PATCH v2 0/4] drm/exynos: dp: consider port node outbound for panel
This patch series considers a port node outbound for panel device node, including dt binding for it. And also it fixes a wrong error type. Changelog v2: - add a patch from Javier, which allows dp to connect panel using of graph. - remove unnecessary properties and numbering pointed out by Rob and Javier. - update description about eDP device. Thanks, Inki Dae Inki Dae (3): drm/exynos: dp: add of_graph dt binding support for panel drm/exynos: dp: fix wrong return type dt-bindings: exynos-dp: update ports node binding for panel Javier Martinez Canillas (1): ARM: dts: Use OF graph for DP to panel connection in exynos5800-peach-pi .../bindings/display/exynos/exynos_dp.txt | 41 +++--- arch/arm/boot/dts/exynos5800-peach-pi.dts | 15 +++- drivers/gpu/drm/exynos/exynos_dp_core.c| 27 -- 3 files changed, 75 insertions(+), 8 deletions(-) -- 1.9.1
[PATCH v3 00/15] ASoC: hdac_hdmi: Add DP & notification support
On Tue, Dec 08, 2015 at 02:47:58AM +0530, Subhransu S. Prusty wrote: > This patch series adds DP audio and hotplug notification support. Not related to the code but there seems to be something wrong with the time configuration on your system which is causing all your patches to be submitted quite a few hours in the future which confuses time based mailbox sorting - might be worth looking at that. -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/0a39e1f9/attachment.sig>
[PATCH 3/9] drm/vc4: Add create and map BO ioctls.
Michel Dänzer writes: > On 08.12.2015 10:16, Eric Anholt wrote: >> Emil Velikov writes: >> >>> Hi Eric, >>> >>> A couple of suggestions/requests on the UAPI header side >>> >>> On 1 December 2015 at 20:35, Eric Anholt wrote: >>> >>>> --- /dev/null >>>> +++ b/include/uapi/drm/vc4_drm.h >>> >>>> +#include >>>> + >>> Can we make this a "drm.h" ? >> >> Nope. >> >> include/uapi/drm/vc4_drm.h:27:17: fatal error: drm.h: No such file or >> directory > > What happened to include/uapi/drm/drm.h in that tree? Looks like it's just versus "drm.h" failure. I've changed over to "drm.h" -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 818 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/1f60b1ff/attachment-0001.sig>
[PATCH 2/3] radeon: Fix VCE ring test for Big-Endian systems
On Mon, Dec 7, 2015 at 9:51 AM, Michel Dänzer wrote:
> On 05.12.2015 06:09, Oded Gabbay wrote:
>> This patch fixes the VCE ring test when running on Big-Endian machines.
>> Every write to the ring needs to be translated to little-endian.
>>
>> Signed-off-by: Oded Gabbay
>> Cc: stable at vger.kernel.org
>> ---
>> drivers/gpu/drm/radeon/radeon_vce.c | 32
>> 1 file changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_vce.c
>> b/drivers/gpu/drm/radeon/radeon_vce.c
>> index 574f62b..86f57e4 100644
>> --- a/drivers/gpu/drm/radeon/radeon_vce.c
>> +++ b/drivers/gpu/drm/radeon/radeon_vce.c
>> @@ -699,12 +699,12 @@ bool radeon_vce_semaphore_emit(struct radeon_device
>> *rdev,
>> {
>> uint64_t addr = semaphore->gpu_addr;
>>
>> - radeon_ring_write(ring, VCE_CMD_SEMAPHORE);
>> - radeon_ring_write(ring, (addr >> 3) & 0x000F);
>> - radeon_ring_write(ring, (addr >> 23) & 0x000F);
>> - radeon_ring_write(ring, 0x01003000 | (emit_wait ? 1 : 0));
>> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_SEMAPHORE));
>> + radeon_ring_write(ring, cpu_to_le32((addr >> 3) & 0x000F));
>> + radeon_ring_write(ring, cpu_to_le32((addr >> 23) & 0x000F));
>> + radeon_ring_write(ring, cpu_to_le32(0x01003000 | (emit_wait ? 1 : 0)));
>> if (!emit_wait)
>> - radeon_ring_write(ring, VCE_CMD_END);
>> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END));
>>
>> return true;
>> }
>> @@ -719,10 +719,10 @@ bool radeon_vce_semaphore_emit(struct radeon_device
>> *rdev,
>> void radeon_vce_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib)
>> {
>> struct radeon_ring *ring = &rdev->ring[ib->ring];
>> - radeon_ring_write(ring, VCE_CMD_IB);
>> - radeon_ring_write(ring, ib->gpu_addr);
>> - radeon_ring_write(ring, upper_32_bits(ib->gpu_addr));
>> - radeon_ring_write(ring, ib->length_dw);
>> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_IB));
>> + radeon_ring_write(ring, cpu_to_le32(ib->gpu_addr));
>> + radeon_ring_write(ring, cpu_to_le32(upper_32_bits(ib->gpu_addr)));
>> + radeon_ring_write(ring, cpu_to_le32(ib->length_dw));
>> }
>>
>> /**
>> @@ -738,12 +738,12 @@ void radeon_vce_fence_emit(struct radeon_device *rdev,
>> struct radeon_ring *ring = &rdev->ring[fence->ring];
>> uint64_t addr = rdev->fence_drv[fence->ring].gpu_addr;
>>
>> - radeon_ring_write(ring, VCE_CMD_FENCE);
>> - radeon_ring_write(ring, addr);
>> - radeon_ring_write(ring, upper_32_bits(addr));
>> - radeon_ring_write(ring, fence->seq);
>> - radeon_ring_write(ring, VCE_CMD_TRAP);
>> - radeon_ring_write(ring, VCE_CMD_END);
>> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_FENCE));
>> + radeon_ring_write(ring, cpu_to_le32(addr));
>> + radeon_ring_write(ring, cpu_to_le32(upper_32_bits(addr)));
>> + radeon_ring_write(ring, cpu_to_le32(fence->seq));
>> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_TRAP));
>> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END));
>> }
>>
>> /**
>> @@ -765,7 +765,7 @@ int radeon_vce_ring_test(struct radeon_device *rdev,
>> struct radeon_ring *ring)
>> ring->idx, r);
>> return r;
>> }
>> - radeon_ring_write(ring, VCE_CMD_END);
>> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END));
>> radeon_ring_unlock_commit(rdev, ring, false);
>>
>> for (i = 0; i < rdev->usec_timeout; i++) {
>>
>
> A new helper function such as
>
> static inline void radeon_ring_write_le(struct radeon_ring *ring, uint32_t v)
> {
> radeon_ring_write(ring, cpu_to_le32(v));
> }
>
> might be nice for this.
>
>
> --
> Earthling Michel Dänzer | http://www.amd.com
> Libre software enthusiast | Mesa and X developer
IMHO, I don't think this gives any benefit.
You would just need to replace every:
radeon_ring_write(ring, cpu_to_le32(SOME_DEFINE));
with
radeon_ring_write_le(ring, SOME_DEFINE);
So no reduce in code size. Also, if you change it in my code, I think
you need to change it in the entire driver for consistency.
What's even more important, is that when I look at the above, it seems
to me this change even makes the code *less* clear as you now need to
go into radeon_ring_write_le to actually understand how the value is
written.
Oded
[Bug 93217] [tonga] [powerplay] Radon M395X isn't initialised with the powerplay branch
https://bugs.freedesktop.org/show_bug.cgi?id=93217 Mike Lothian changed: What|Removed |Added Hardware|Other |x86-64 (AMD64) OS|All |Linux (All) -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/cab0a9a2/attachment.html>
[Bug 93217] [tonga] [powerplay] Radon M395X isn't initialised with the powerplay branch
https://bugs.freedesktop.org/show_bug.cgi?id=93217 Mike Lothian changed: What|Removed |Added Attachment #120284|0 |1 is obsolete|| --- Comment #5 from Mike Lothian --- Created attachment 120392 --> https://bugs.freedesktop.org/attachment.cgi?id=120392&action=edit Updated dmesg with drm.debug=0xf -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/f494f948/attachment.html>
[PATCH 4/4] fbdev: Debug knob to register without holding console_lock
On 25/08/15 16:45, Daniel Vetter wrote: > When the usual fbcon legacy options are enabled we have > ->register_framebuffer > ->fb notifier chain calls into fbcon > ->fbcon sets up console on new fbi > ->fbi->set_par > ->drm_fb_helper_set_par exercises full kms api > > And because of locking inversion hilarity all of register_framebuffer > is done with the console lock held. Which means that the first time on > driver load we exercise _all_ the kms code (all probe paths and > modeset paths for everything connected) is under the console lock. > That means if anything goes belly-up in that big pile of code nothing > ever reaches logfiles (and the machine is dead). > > Usual tactic to debug that is to temporarily remove those console_lock > calls to be able to capture backtraces. I'm fed up writing this patch > and recompiling kernels. Hence this patch here to add an unsafe, > kernel-taining option to do this at runtime. I think this was never merged. This was part 4 of 4, were there dependencies or...? Should I apply this for 4.5? But then... I think my issues with console lock have been later at runtime, not at register. Maybe we need a module option to disable the console lock altogether. I wonder how much havoc that might create, though. Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: OpenPGP digital signature URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/b622758e/attachment.sig>
[PATCH] drm: do not use device name as a format string
On 12/07/2015 01:31 PM, Thierry Reding wrote:
> On Mon, Dec 07, 2015 at 12:46:52PM +0100, Daniel Vetter wrote:
>> On Mon, Dec 07, 2015 at 11:53:01AM +0200, Jani Nikula wrote:
>>> On Mon, 07 Dec 2015, Daniel Vetter wrote:
On Sun, Dec 06, 2015 at 11:16:32AM +0100, Nicolas Iooss wrote:
> On 12/06/2015 10:35 AM, Daniel Vetter wrote:
>>> On 11/18/2015 06:58 PM, Nicolas Iooss wrote:
drm_dev_set_unique() formats its parameter using kvasprintf() but many
of its callers directly pass dev_name(dev) as printf format string,
without any format parameter. This can cause some issues when the
device name contains '%' characters.
To avoid any potential issue, always use "%s" when using
drm_dev_set_unique() with dev_name().
>>
>> Not sure this is worth it really, normally people don't place %
>> characters
>> into their device names, ever. And if they do it'll blow up. There's also
>> no security issue here since userspace can't set this name.
>>
>> If the maintainers of the affected drivers don't want this I won't merge
>> this patch.
>
> Actually I had the same opinion before I began to add __printf
> attributes and "%s" in several places in the kernel to make
> -Wformat-security useful. This led me to discover some funny issues
> like the one fixed by commit 3958b79266b1 ("configfs: fix kernel
> infoleak through user-controlled format string",
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=3958b79266b14729edd61daf9dfb84de45f4ec6d
> ). The patch I sent is in fact a very small step towards making
> -Wformat-security useful again to detect "real" issues.
>
> Of course, if you do not feel it is worth it and believe that dev_name
> is fully controlled by trusted sources which will never introduce any %
> character, I understand your will of not merging my patch.
Ah, that's the context I was missing, that really should be in the commit
message. If this is part of an overall effort to enable something useful
it makes more sense to get it in.
On the patch itself it feels rather funny to do a "%s", str); combo, maybe
we should have a drm_dev_set_unique_static instead? Including kerneldoc
explaining why there's too.
>>>
>>> No caller of drm_dev_set_unique() actually uses the formatting for
>>> anything... so you'd end up with drm_dev_set_unique_static() and an
>>> orphaned drm_dev_set_unique()...
>>
>> Ok, then I guess we can just ditch the printf stuff from set_unique.
>> Nicolas, you're up for that?
>
> Looking at all the callsites of drm_dev_set_unique() it seems like all
> of the drivers (with the exception of vgem) use dev_name() on the same
> device that's already passed into drm_dev_alloc(), so perhaps another
> alternative would be to have drm_dev_alloc() set the unique name by
> default and keep the function for cases where it needs to be set
> explicitly (like for vgem). vgem passes drm_dev_alloc() a NULL device,
> so that could serve as condition.
I have written a patch which removes the printf format processing from
drm_dev_set_unique(). I will test it as soon as possible and depending
on the results, send it or explain what went wrong. If no one does the
work before me, I'll also take a look at calling drm_dev_set_unique() in
drm_dev_alloc(), and this would be an other patch.
Thanks,
Nicolas
[PATCH 1/4] component: remove old add_components method
On Mon, Nov 23, 2015 at 04:02:37PM +, Russell King wrote:
> Now that drivers create an array of component matches at probe time, we
> can retire the old methods. This involves removing the add_components
> master method, and removing component_master_add_child() from public
> view. We also remove component_add_master() as that interface is no
> longer useful.
>
> Signed-off-by: Russell King
I've been using this patch for a couple of weeks
Tested-by: Andrew Lunn
Andrew
> ---
> drivers/base/component.c | 31 +--
> include/linux/component.h | 5 -
> 2 files changed, 5 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> index f748430bb654..2ca22738ae92 100644
> --- a/drivers/base/component.c
> +++ b/drivers/base/component.c
> @@ -84,7 +84,7 @@ static void component_detach_master(struct master *master,
> struct component *c)
> * function and compare data. This is safe to call for duplicate matches
> * and will not result in the same component being added multiple times.
> */
> -int component_master_add_child(struct master *master,
> +static int component_master_add_child(struct master *master,
> int (*compare)(struct device *, void *), void *compare_data)
> {
> struct component *c;
> @@ -104,7 +104,6 @@ int component_master_add_child(struct master *master,
>
> return ret;
> }
> -EXPORT_SYMBOL_GPL(component_master_add_child);
>
> static int find_components(struct master *master)
> {
> @@ -112,14 +111,6 @@ static int find_components(struct master *master)
> size_t i;
> int ret = 0;
>
> - if (!match) {
> - /*
> - * Search the list of components, looking for components that
> - * belong to this master, and attach them to the master.
> - */
> - return master->ops->add_components(master->dev, master);
> - }
> -
> /*
>* Scan the array of match functions and attach
>* any components which are found to this master.
> @@ -290,15 +281,10 @@ int component_master_add_with_match(struct device *dev,
> struct master *master;
> int ret;
>
> - if (ops->add_components && match)
> - return -EINVAL;
> -
> - if (match) {
> - /* Reallocate the match array for its true size */
> - match = component_match_realloc(dev, match, match->num);
> - if (IS_ERR(match))
> - return PTR_ERR(match);
> - }
> + /* Reallocate the match array for its true size */
> + match = component_match_realloc(dev, match, match->num);
> + if (IS_ERR(match))
> + return PTR_ERR(match);
>
> master = kzalloc(sizeof(*master), GFP_KERNEL);
> if (!master)
> @@ -326,13 +312,6 @@ int component_master_add_with_match(struct device *dev,
> }
> EXPORT_SYMBOL_GPL(component_master_add_with_match);
>
> -int component_master_add(struct device *dev,
> - const struct component_master_ops *ops)
> -{
> - return component_master_add_with_match(dev, ops, NULL);
> -}
> -EXPORT_SYMBOL_GPL(component_master_add);
> -
> void component_master_del(struct device *dev,
> const struct component_master_ops *ops)
> {
> diff --git a/include/linux/component.h b/include/linux/component.h
> index c00dcc302611..71c434a6a5ee 100644
> --- a/include/linux/component.h
> +++ b/include/linux/component.h
> @@ -17,18 +17,13 @@ void component_unbind_all(struct device *, void *);
> struct master;
>
> struct component_master_ops {
> - int (*add_components)(struct device *, struct master *);
> int (*bind)(struct device *);
> void (*unbind)(struct device *);
> };
>
> -int component_master_add(struct device *, const struct component_master_ops
> *);
> void component_master_del(struct device *,
> const struct component_master_ops *);
>
> -int component_master_add_child(struct master *master,
> - int (*compare)(struct device *, void *), void *compare_data);
> -
> struct component_match;
>
> int component_master_add_with_match(struct device *,
> --
> 2.1.0
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[PATCH] drm/omap: Use platform_register/unregister_drivers()
On 02/12/15 18:23, Thierry Reding wrote: > From: Thierry Reding > > These new helpers simplify implementing multi-driver modules and > properly handle failure to register one driver by unregistering all > previously registered drivers. > > Signed-off-by: Thierry Reding > --- > drivers/gpu/drm/omapdrm/omap_drv.c | 26 +++--- > 1 file changed, 7 insertions(+), 19 deletions(-) Thanks, applying for 4.5. Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: OpenPGP digital signature URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/76473065/attachment.sig>
[PATCH 02/12] drm/etnaviv: add devicetree bindings
On 05.12.2015 19:12, Daniel Vetter wrote: > On Fri, Dec 04, 2015 at 05:43:33PM -0500, Ilia Mirkin wrote: >> On Fri, Dec 4, 2015 at 5:05 PM, Russell King - ARM Linux >> wrote: >>> On Fri, Dec 04, 2015 at 03:42:47PM -0500, Ilia Mirkin wrote: On Fri, Dec 4, 2015 at 3:31 PM, Russell King - ARM Linux wrote: > Moreover, DRI3 is not yet available for Gallium, so if we're talking > about Xorg, then functional DRI2 is a requirement, and that _needs_ > to have a single device for the rendering instances. Xorg has no way > to pass multiple render nodes to client over DRI2. Just to correct... DRI3 has been available on gallium [at least in the context of st/mesa] basically since DRI3 was introduced. Not sure what issue you're fighting with, but it's definitely not a gallium limitation... could be something related to platform devices. >>> >>> Well, my statement is based on the fact that there's nothing in >>> src/gallium/state-tracker/dri which hints at being DRI3. Maybe it's >>> implemented differently, I don't know. The DRI state tracker code in src/gallium/state_trackers/dri/ supports DRI1-3. There's almost no explicit mention of DRI3 in it because DRI3 and DRI2 work mostly the same as far as this code is concerned. >>> I think it's a DRI3 limitation. The issue with the DRI3 design is that: >>> >>> * The client has access to the GPU render nodes only, but not the >>> corresponding KMS node. >>> * Buffers in DRI3 are allocated from the GPU render nodes. >>> * The Xorg Present protocol is then used to manage the vblank >>> synchonisation and page flips. >>> >>> Now, the KMS scanout hardware typically does not support any kind of >>> scatter-gather: the buffers it has must be contiguous. These can be >>> allocated from the KMS DRM device. >>> >>> However, the DRI3 client has no access to the KMS DRM device to allocate >>> linear buffers from, and GPUs typically don't have dumb buffer support. >>> Hence, the client can't pass a suitable buffer to the present layer. > > Oh right, buffer alloc if you have special constraints won't work with > DRI3 as-is. For that we probably need something like DRI2 for buffer alloc > + Present (like DRI3 does) for flipping them. FWIW, that's basically possible already. E.g. the Gallium nine state tracker can use Present even with DRI2. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
[PATCH v3 1/3] modetest: introduce get_prop_info() for getting property id and type
On Fri, Nov 27, 2015 at 11:37:33AM +0900, Hyungwon Hwang wrote: > Hello all, > > +To Thierry Reding > > On Thu, 26 Nov 2015 16:42:01 + > Emil Velikov wrote: > > > Hi Hyungwon > > > > I completely missed out that you've sent an updated version of the > > series. I will take a look at them later on tonight. Meanwhile have > > you looked at the atomic tests/helpers work from Thierry [1] ? It > > creates nice simple helpers (as opposed to a single massive C file > > like modetest) and I'm leaning that there is quite a lot of code that > > can be reused from there. > > > > Speaking of which Thierry, > > What is happening with these patches ? Last time you've respun them > > I've pinged you to send them over for inclusion. There was a trivial > > fix needed here or there but they were in pretty good shape. > > Yes. I checked it now. It would be more clear concise, if they comes. > Would it be better to wait Thierry for sending them again? Can you > afford to do that, Thierry? Yeah, I think I can do that. It'll probably take until tomorrow for me to find a free slot to rebase and send out again. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/303446e5/attachment.sig>
[PATCH 3/9] drm/vc4: Add create and map BO ioctls.
Emil Velikov writes:
> Hi Eric,
>
> A couple of suggestions/requests on the UAPI header side
>
> On 1 December 2015 at 20:35, Eric Anholt wrote:
>
>> --- /dev/null
>> +++ b/include/uapi/drm/vc4_drm.h
>
>> +#include
>> +
> Can we make this a "drm.h" ?
Nope.
include/uapi/drm/vc4_drm.h:27:17: fatal error: drm.h: No such file or directory
and none of the drivers do, either.
>> +struct drm_vc4_create_bo {
>> + uint32_t size;
>> + uint32_t flags;
>> + /** Returned GEM handle for the BO. */
>> + uint32_t handle;
>> + uint32_t pad;
> ... and not use the stdint.h types but __{s,u}* through the file.
>
> I'd rather than not nag why we want/need those, but if you prefer I
> can repeat myself/others.
I've switched to using the special types.
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/cdc87ba5/attachment.sig>
[PATCH 2/3] radeon: Fix VCE ring test for Big-Endian systems
On 05.12.2015 06:09, Oded Gabbay wrote:
> This patch fixes the VCE ring test when running on Big-Endian machines.
> Every write to the ring needs to be translated to little-endian.
>
> Signed-off-by: Oded Gabbay
> Cc: stable at vger.kernel.org
> ---
> drivers/gpu/drm/radeon/radeon_vce.c | 32
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_vce.c
> b/drivers/gpu/drm/radeon/radeon_vce.c
> index 574f62b..86f57e4 100644
> --- a/drivers/gpu/drm/radeon/radeon_vce.c
> +++ b/drivers/gpu/drm/radeon/radeon_vce.c
> @@ -699,12 +699,12 @@ bool radeon_vce_semaphore_emit(struct radeon_device
> *rdev,
> {
> uint64_t addr = semaphore->gpu_addr;
>
> - radeon_ring_write(ring, VCE_CMD_SEMAPHORE);
> - radeon_ring_write(ring, (addr >> 3) & 0x000F);
> - radeon_ring_write(ring, (addr >> 23) & 0x000F);
> - radeon_ring_write(ring, 0x01003000 | (emit_wait ? 1 : 0));
> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_SEMAPHORE));
> + radeon_ring_write(ring, cpu_to_le32((addr >> 3) & 0x000F));
> + radeon_ring_write(ring, cpu_to_le32((addr >> 23) & 0x000F));
> + radeon_ring_write(ring, cpu_to_le32(0x01003000 | (emit_wait ? 1 : 0)));
> if (!emit_wait)
> - radeon_ring_write(ring, VCE_CMD_END);
> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END));
>
> return true;
> }
> @@ -719,10 +719,10 @@ bool radeon_vce_semaphore_emit(struct radeon_device
> *rdev,
> void radeon_vce_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib)
> {
> struct radeon_ring *ring = &rdev->ring[ib->ring];
> - radeon_ring_write(ring, VCE_CMD_IB);
> - radeon_ring_write(ring, ib->gpu_addr);
> - radeon_ring_write(ring, upper_32_bits(ib->gpu_addr));
> - radeon_ring_write(ring, ib->length_dw);
> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_IB));
> + radeon_ring_write(ring, cpu_to_le32(ib->gpu_addr));
> + radeon_ring_write(ring, cpu_to_le32(upper_32_bits(ib->gpu_addr)));
> + radeon_ring_write(ring, cpu_to_le32(ib->length_dw));
> }
>
> /**
> @@ -738,12 +738,12 @@ void radeon_vce_fence_emit(struct radeon_device *rdev,
> struct radeon_ring *ring = &rdev->ring[fence->ring];
> uint64_t addr = rdev->fence_drv[fence->ring].gpu_addr;
>
> - radeon_ring_write(ring, VCE_CMD_FENCE);
> - radeon_ring_write(ring, addr);
> - radeon_ring_write(ring, upper_32_bits(addr));
> - radeon_ring_write(ring, fence->seq);
> - radeon_ring_write(ring, VCE_CMD_TRAP);
> - radeon_ring_write(ring, VCE_CMD_END);
> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_FENCE));
> + radeon_ring_write(ring, cpu_to_le32(addr));
> + radeon_ring_write(ring, cpu_to_le32(upper_32_bits(addr)));
> + radeon_ring_write(ring, cpu_to_le32(fence->seq));
> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_TRAP));
> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END));
> }
>
> /**
> @@ -765,7 +765,7 @@ int radeon_vce_ring_test(struct radeon_device *rdev,
> struct radeon_ring *ring)
> ring->idx, r);
> return r;
> }
> - radeon_ring_write(ring, VCE_CMD_END);
> + radeon_ring_write(ring, cpu_to_le32(VCE_CMD_END));
> radeon_ring_unlock_commit(rdev, ring, false);
>
> for (i = 0; i < rdev->usec_timeout; i++) {
>
A new helper function such as
static inline void radeon_ring_write_le(struct radeon_ring *ring, uint32_t v)
{
radeon_ring_write(ring, cpu_to_le32(v));
}
might be nice for this.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
[PATCH 26/28] drm: Move drm_display_mode an related docs into kerneldoc
On Mon, Dec 07, 2015 at 04:02:38PM +0100, Thierry Reding wrote:
> On Fri, Dec 04, 2015 at 09:46:07AM +0100, Daniel Vetter wrote:
> > @@ -140,12 +352,48 @@ struct drm_display_mode {
> > int crtc_vsync_end;
> > int crtc_vtotal;
> >
> > - /* Driver private mode info */
> > + /**
> > +* @private:
> > +*
> > +* Pointer for driver private data. This can only be used for mode
> > +* objects passed to drivers in modeset operations. It shouldn't be used
> > +* by atomic drivers since they can store any additional data by
> > +* subclassing state structures.
> > +*/
> > int *private;
>
> Off-topic: Any reasons why this is int * and not void *?
Was added like that years ago. Iirc no one ever used this at all, so maybe
we should just nuke it. With atomic state structures we have a much better
solution now.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH v3] staging/android: add TODO to de-stage android sync framework
Hi, any comments/update on this? Thanks Gustavo 2015-11-26 Gustavo Padovan : > From: Gustavo Padovan > > - remove CONFIG_SW_SYNC_USER, it is used only for testing/debugging and >should not be upstreamed. > - port CONFIG_SW_SYNC_USER tests interfaces to use debugfs somehow > - port libsync tests to kselftest > - clean up and ABI check for security issues > - move the sync framework to drivers/base/dma-buf > > Cc: Arve Hjønnevåg > Cc: Riley Andrews > Cc: Daniel Vetter > Cc: Rob Clark > Cc: Greg Hackmann > Cc: John Harrison > Signed-off-by: Gustavo Padovan > --- > drivers/staging/android/TODO | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO > index 8f3ac37..64d8c87 100644 > --- a/drivers/staging/android/TODO > +++ b/drivers/staging/android/TODO > @@ -25,5 +25,13 @@ ion/ > exposes existing cma regions and doesn't reserve unecessarily memory when > booting a system which doesn't use ion. > > +sync framework: > + - remove CONFIG_SW_SYNC_USER, it is used only for testing/debugging and > + should not be upstreamed. > + - port CONFIG_SW_SYNC_USER tests interfaces to use debugfs somehow > + - port libsync tests to kselftest > + - clean up and ABI check for security issues > + - move it to drivers/base/dma-buf > + > Please send patches to Greg Kroah-Hartman and Cc: > Arve Hjønnevåg and Riley Andrews android.com> > -- > 2.1.0 >
[PATCH 24/28] drm: Document drm_connector_helper_funcs
On Mon, Dec 07, 2015 at 03:48:37PM +0100, Daniel Vetter wrote:
> On Mon, Dec 07, 2015 at 03:42:22PM +0100, Thierry Reding wrote:
> > On Fri, Dec 04, 2015 at 09:46:05AM +0100, Daniel Vetter wrote:
> > > Nothing special, except the somewhat awkard split in probe helper
> >
> > "awkward"
> >
> > > callbacks between here and drm_crtc_funcs.
> > >
> > > Signed-off-by: Daniel Vetter
> > > ---
> > > include/drm/drm_modeset_helper_vtables.h | 106
> > > +--
> > > 1 file changed, 101 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/include/drm/drm_modeset_helper_vtables.h
> > > b/include/drm/drm_modeset_helper_vtables.h
> > > index 66b78c14154e..22cc51b278fb 100644
> > > --- a/include/drm/drm_modeset_helper_vtables.h
> > > +++ b/include/drm/drm_modeset_helper_vtables.h
> > > @@ -264,18 +264,114 @@ static inline void drm_encoder_helper_add(struct
> > > drm_encoder *encoder,
> > >
> > > /**
> > > * struct drm_connector_helper_funcs - helper operations for connectors
> > > - * @get_modes: get mode list for this connector
> > > - * @mode_valid: is this mode valid on the given connector? (optional)
> > > - * @best_encoder: return the preferred encoder for this connector
> > > - * @atomic_best_encoder: atomic version of @best_encoder
> > > *
> > > - * The helper operations are called by the mid-layer CRTC helper.
> > > + * These functions are used by the atomic and legacy modeset helpers and
> > > by the
> > > + * probe helpers.
> > > */
> > > struct drm_connector_helper_funcs {
> > > + /**
> > > + * @get_modes:
> > > + *
> > > + * This function should fill in all modes currently valid for the sink
> > > + * into the connector->probe_modes function. It should also update the
> >
> > What's probe_modes? I've never heard of it. Did you mean ->fill_modes()?
> > Also it's strange to say "fill into the ... function". Perhaps "pass
> > into the ... function" instead?
>
> connector->probe*d*_modes *list* is what it should read. Fixed.
As with all the other patches, with this and the nits fixed:
Reviewed-by: Thierry Reding
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/1deb1759/attachment.sig>
[PATCH 28/28] drm/atomic-helper: Reject legacy flips on a disabled pipe
On Fri, Dec 04, 2015 at 09:46:09AM +0100, Daniel Vetter wrote:
> We want this for consistency with existing page_flip semantics.
>
> Since this spurred quite a discussion on IRC also document why we
> reject even generation when the pipe is off: It's not that it's hard
> to implement, but userspace has a track recording proofing that it's
> way too easy to accidentally abuse and cause havoc. We want to make
> sure userspace doesn't get away with that.
>
> Cc: Daniel Stone
> Cc: Ville Syrjälä
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_atomic.c| 9 +
> drivers/gpu/drm/drm_atomic_helper.c | 8
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 7426d40017a0..06cdb52907da 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1265,6 +1265,15 @@ int drm_atomic_check_only(struct drm_atomic_state
> *state)
> if (config->funcs->atomic_check)
> ret = config->funcs->atomic_check(state->dev, state);
>
> + /*
> + * Reject event generation for when a CRTC is off and stays off. It
> + * wouldn't be hard to implement this, but userspace has a track record
> + * of happily burning through 100% cpu (or worse, crash) when the
> + * display pipe is suspended. To avoid all that fun just reject updates
> + * that ask for events since likely that indicates a bug in the
> + * compositors drawing loop. This is consistent with the vblank ioctl
"compositor's".
> + * which also rejects service on a disabled pipe.
> + */
> if (!state->allow_modeset) {
> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 110f3db8dd05..8e281a96c35f 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2283,6 +2283,14 @@ retry:
> goto fail;
> drm_atomic_set_fb_for_plane(plane_state, fb);
>
> + state->allow_modeset = false;
Perhaps explain the reason for setting this?
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/5681a3ff/attachment.sig>
[PATCH 27/28] drm: Document drm_encoder/crtc_helper_funcs
/* atomic helpers */ > + /** > + * @atomic_check: > + * > + * This callback is used to validate encoder state for atomic drivers. > + * Since the encoder is the object connecting the CRTC and connector it > + * gets passed both states, to be able to validate interactions and > + * update the CRTC to match what the encoder needs for the requested > + * connector. > + * > + * This function is used by the atomic helpers, but it is optional. > + * > + * NOTE: > + * > + * This function is called in the check phase of an atomic update. The > + * driver is not allowed to change anything outside of the free-standing > + * state objects passed-in or assembled in the overall &drm_atomic_state > + * update tracking structure. > + * > + * RETURNS: > + * > + * 0 on success, -EINVAL if the state or the transition can't be > + * support, -ENOMEM on memory allocation failure and -EDEADLK if an "supported" Thanks for writing this up, it's great documentation. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/6204ad1b/attachment.sig>
[PATCH 26/28] drm: Move drm_display_mode an related docs into kerneldoc
vestige
> + *from older kms designs where userspace had to first add a custom
> + *mode to the kernel's mode list before it could use it. Don't use.
> + */
> unsigned int type;
>
> - /* Proposed mode values */
> + /**
> + * @clock:
> + *
> + * Pixel clock in kHz.
> + */
> int clock; /* in kHz */
> int hdisplay;
> int hsync_start;
> @@ -118,14 +270,74 @@ struct drm_display_mode {
> int vsync_end;
> int vtotal;
> int vscan;
I'm thinking that these could all use "unsigned", but that's definitely
something for a separate patch.
> + /**
> + * @flags:
> + *
> + * Sync and timing flags:
> + *
> + * - DRM_MODE_FLAG_PHSYNC: horizontal sync is active high.
> + * - DRM_MODE_FLAG_NHSYNC: horizontal sync is active low.
> + * - DRM_MODE_FLAG_PVSYNC: vertical sync is active high.
> + * - DRM_MODE_FLAG_NVSYNC: vertical sync is active low.
> + * - DRM_MODE_FLAG_INTERLACE: mode is interlaced.
> + * - DRM_MODE_FLAG_DBLSCAN: mode uses doublescan.
> + * - DRM_MODE_FLAG_CSYNC: mode uses composite sync.
> + * - DRM_MODE_FLAG_PCSYNC: composite sync is active high.
> + * - DRM_MODE_FLAG_NCSYNC: composite sync is active low.
> + * - DRM_MODE_FLAG_HSKEW: hskew provided (not used?).
> + * - DRM_MODE_FLAG_BCAST: not used?
> + * - DRM_MODE_FLAG_PIXMUX: not used?
> + * - DRM_MODE_FLAG_DBLCLK: double-clocked mode.
> + * - DRM_MODE_FLAG_CLKDIV2: half-clocked mode.
> + *
> + * Additionally there's flags to specify how 3D modes are packed:
> + *
> + * - DRM_MODE_FLAG_3D_NONE: normal, non-3D mode.
> + * - DRM_MODE_FLAG_3D_FRAME_PACKING: 2 full frames for left and right.
> + * - DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE: interleaved like fields.
> + * - DRM_MODE_FLAG_3D_LINE_ALTERNATIVE: interleaved lines.
> + * - DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL: side-by-side full frames.
> + * - DRM_MODE_FLAG_3D_L_DEPTH: ?
^
Stray tab.
> + * Note that with digital outputs like HDMI or DP there's usually a
> + * massive confusion between the dot clock and the signal clock at the
> + * bit encoding level. Especially when a 8b/10b encoding is used and the
> + * differences is exactly a factor of 10.
"difference"
> + */
> + int crtc_clock;
> int crtc_hdisplay;
> int crtc_hblank_start;
> int crtc_hblank_end;
> @@ -140,12 +352,48 @@ struct drm_display_mode {
> int crtc_vsync_end;
> int crtc_vtotal;
>
> - /* Driver private mode info */
> + /**
> + * @private:
> + *
> + * Pointer for driver private data. This can only be used for mode
> + * objects passed to drivers in modeset operations. It shouldn't be used
> + * by atomic drivers since they can store any additional data by
> + * subclassing state structures.
> + */
> int *private;
Off-topic: Any reasons why this is int * and not void *?
> +
> + /**
> + * @private_flags:
> + *
> + * Similar to @private, but just an integer.
> + */
> int private_flags;
>
> - int vrefresh; /* in Hz */
> - int hsync; /* in kHz */
> + /**
> + * @vrefresh:
> + *
> + * Vertical refresh rate, for debug output in human readable form. Not
> + * used in a functional way.
> + *
> + * This value is in Hz.
> + */
> + int vrefresh;
> +
> + /**
> + * @hsync:
> + *
> + * Horizontal refresh rate, for debug output in human readable form. Not
> + * used in a functional way.
> + *
> + * This value is in kHz.
> + */
> + int hsync;
> +
> + /**
> + * @picture_aspect_ratio:
> + *
> + * Filed for setting the HDMI picture aspect ratio of a mode.
"Field".
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/398427ab/attachment-0001.sig>
[PATCH v11 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory
Split the dp core driver from exynos directory to bridge directory,
and rename the core driver to analogix_dp_*, rename the platform
code to exynos_dp.
Beside the new analogix_dp driver would export six hooks.
"analogix_dp_bind()" and "analogix_dp_unbind()"
"analogix_dp_suspned()" and "analogix_dp_resume()"
"analogix_dp_detect()" and "analogix_dp_get_modes()"
The bind/unbind symbols is used for analogix platform driver to connect
with analogix_dp core driver. And the detect/get_modes is used for analogix
platform driver to init the connector.
They reason why connector need register in helper driver is rockchip drm
haven't implement the atomic API, but Exynos drm have implement it, so
there would need two different connector helper functions, that's why we
leave the connector register in helper driver.
Signed-off-by: Yakir Yang
Tested-by: Javier Martinez Canillas
---
Changes in v11:
- Fix compiled error in analogix_dp_unbind()
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6:
- Fix the Kconfig recursive dependency (Javier)
Changes in v5:
- Correct the check condition of gpio_is_valid when driver try to get
the "hpd-gpios" DT propery. (Heiko)
- Move the platform attach callback in the front of core driver bridge
attch function. Cause once platform failed at attach, core driver should
still failed, so no need to init connector before platform attached
(Krzysztof)
- Keep code style no changes with the previous exynos_dp_code.c in this
patch, and update commit message about the new export symbol (Krzysztof)
- Gather the device type patch (v4 11/16) into this one. (Krzysztof)
- leave out the connector registration to analogix platform driver. (Thierry)
Changes in v4:
- Update "analogix,hpd-gpios" to "hpd-gpios" DT propery. (Rob)
- Rename "analogix_dp-exynos.c" file name to "exynos_dp.c" (Jingoo)
- Create a separate folder for analogix code in bridge/ (Archit)
Changes in v3:
- Move exynos's video_timing code to analogix_dp-exynos platform driver,
add get_modes method to struct analogix_dp_plat_data. (Thierry)
- Rename some "samsung*" dts propery to "analogix*". (Heiko)
Changes in v2:
- Remove new copyright (Jingoo)
- Fix compiled failed due to analogix_dp_device misspell
drivers/gpu/drm/bridge/Kconfig |2 +
drivers/gpu/drm/bridge/Makefile|1 +
drivers/gpu/drm/bridge/analogix/Kconfig|3 +
drivers/gpu/drm/bridge/analogix/Makefile |1 +
.../analogix/analogix_dp_core.c} | 799 +
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 277 +
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 1263
.../analogix/analogix_dp_reg.h}| 258 ++--
drivers/gpu/drm/exynos/Kconfig |3 +-
drivers/gpu/drm/exynos/Makefile|2 +-
drivers/gpu/drm/exynos/exynos_dp.c | 384 ++
drivers/gpu/drm/exynos/exynos_dp_core.h| 282 -
drivers/gpu/drm/exynos/exynos_dp_reg.c | 1263
include/drm/bridge/analogix_dp.h | 41 +
14 files changed, 2399 insertions(+), 2180 deletions(-)
create mode 100644 drivers/gpu/drm/bridge/analogix/Kconfig
create mode 100644 drivers/gpu/drm/bridge/analogix/Makefile
rename drivers/gpu/drm/{exynos/exynos_dp_core.c =>
bridge/analogix/analogix_dp_core.c} (50%)
create mode 100644 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
create mode 100644 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
rename drivers/gpu/drm/{exynos/exynos_dp_reg.h =>
bridge/analogix/analogix_dp_reg.h} (64%)
create mode 100644 drivers/gpu/drm/exynos/exynos_dp.c
delete mode 100644 drivers/gpu/drm/exynos/exynos_dp_core.h
delete mode 100644 drivers/gpu/drm/exynos/exynos_dp_reg.c
create mode 100644 include/drm/bridge/analogix_dp.h
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 27e2022..efd94e0 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -40,4 +40,6 @@ config DRM_PARADE_PS8622
---help---
Parade eDP-LVDS bridge chip driver.
+source "drivers/gpu/drm/bridge/analogix/Kconfig"
+
endmenu
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index f13c33d..ff821f4 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
+obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig
b/drivers/gpu/drm/bridge/analogix/Kconfig
new file mode 100644
index 000..80f286f
--- /dev/null
+++ b/drivers/gpu/drm/bridge/analogix/Kconfig
@@ -0,0 +1,3 @@
+config DRM_ANALOGIX_DP
+ tristate
+ depends on
[PATCH 24/28] drm: Document drm_connector_helper_funcs
On Mon, Dec 07, 2015 at 03:42:22PM +0100, Thierry Reding wrote:
> On Fri, Dec 04, 2015 at 09:46:05AM +0100, Daniel Vetter wrote:
> > Nothing special, except the somewhat awkard split in probe helper
>
> "awkward"
>
> > callbacks between here and drm_crtc_funcs.
> >
> > Signed-off-by: Daniel Vetter
> > ---
> > include/drm/drm_modeset_helper_vtables.h | 106
> > +--
> > 1 file changed, 101 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/drm/drm_modeset_helper_vtables.h
> > b/include/drm/drm_modeset_helper_vtables.h
> > index 66b78c14154e..22cc51b278fb 100644
> > --- a/include/drm/drm_modeset_helper_vtables.h
> > +++ b/include/drm/drm_modeset_helper_vtables.h
> > @@ -264,18 +264,114 @@ static inline void drm_encoder_helper_add(struct
> > drm_encoder *encoder,
> >
> > /**
> > * struct drm_connector_helper_funcs - helper operations for connectors
> > - * @get_modes: get mode list for this connector
> > - * @mode_valid: is this mode valid on the given connector? (optional)
> > - * @best_encoder: return the preferred encoder for this connector
> > - * @atomic_best_encoder: atomic version of @best_encoder
> > *
> > - * The helper operations are called by the mid-layer CRTC helper.
> > + * These functions are used by the atomic and legacy modeset helpers and
> > by the
> > + * probe helpers.
> > */
> > struct drm_connector_helper_funcs {
> > + /**
> > +* @get_modes:
> > +*
> > +* This function should fill in all modes currently valid for the sink
> > +* into the connector->probe_modes function. It should also update the
>
> What's probe_modes? I've never heard of it. Did you mean ->fill_modes()?
> Also it's strange to say "fill into the ... function". Perhaps "pass
> into the ... function" instead?
connector->probe*d*_modes *list* is what it should read. Fixed.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH 01/20] drm: use __u{32,64} instead of uint{32,64}_t in virtgpu_drm.h
On Mon, Dec 07, 2015 at 01:29:41PM +, Emil Velikov wrote: > On 5 December 2015 at 21:03, Dave Airlie wrote: > > On 5 December 2015 at 00:22, Emil Velikov > > wrote: > >> On 30 November 2015 at 14:10, Gabriel Laskar > >> wrote: > >>> Signed-off-by: Gabriel Laskar > >>> CC: Emil Velikov > >>> CC: Mikko Rapeli > >>> > >>> --- > >>> include/uapi/drm/virtgpu_drm.h | 98 > >>> +- > >>> 1 file changed, 49 insertions(+), 49 deletions(-) > >>> > >> For the series > >> Reviewed-by: Emil Velikov > >> > >> Dave would you have any comments wrt this and the remainder of Mikko's > >> series ? Alternatively what can we do to get those merged (would you > >> like a branch/pull request) ? > > > > Yeah a git pull for these would be good, it's about all I can do to > > care about them. > > > >From your earlier reply I got the impression that you'll pick Mikko's > work. Either way, glad to see some progress on the topic. > > Mikko, Gabriel, > > Will you guys be so kind to send pull requests or shall I ? Go a head and send one. I guess you are talking about the drm specific patches. I tested Gabriel's changes too and found one more userspace compilation problem from the now exported AMD header. You can pick my patches from emails or latest draft is at https://github.com/mcfrisk/linux/tree/headers_test_v05 I'm following Linus'es tree and dropping changes which get merged from my development branch. -Mikko
[PATCH 25/28] drm/atomic-helper: Mention the new system/resume helpers the docs
On Fri, Dec 04, 2015 at 09:46:06AM +0100, Daniel Vetter wrote: > They have pretty kerneldoc already, but better to link to that in > one of the overview sections. > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_atomic_helper.c | 6 ++ > 1 file changed, 6 insertions(+) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/4a171ee4/attachment.sig>
[PATCH 23/28] drm: Document drm_plane_helper_funcs
On Mon, Dec 07, 2015 at 03:27:59PM +0100, Thierry Reding wrote: > On Fri, Dec 04, 2015 at 09:46:04AM +0100, Daniel Vetter wrote: > > +* mus ensure that drm_atomic_helper_check_modeset() has been called > > +* beforehand. > > Perhaps mention that the default drm_atomic_helper_check() > implementation calls them in the right order? Good idea, added a sentence with reference to drm_atomic_helper_check(). > > > +* > > +* When using drm_atomic_helper_check_planes CRTC's ->atomic_check() > > Parentheses after the function name? Also, technically I think it would > need to be "CRTCs'" since it relates to multiple CRTCs. Perhaps just > leaving out the 's would work as well. I went with CRTCs'. > > void (*atomic_begin)(struct drm_crtc *crtc, > > struct drm_crtc_state *old_crtc_state); > > + /** > > +* @atomic_flush: > > +* > > +* Drivers should prepare for an atomic update of multiple planes on > > +* a CRTC in this hook. Depending upon hardware this might include > > The first sentence here is the exact same as for ->atomic_begin(), but > it clearly has a different purpose. Perhaps: > > "Drivers should finalize an atomic update of multiple planes on >a CRTC in this hook. ..." > > ? Sounds good, simply forgotten to update that part. All other suggestions applied, thanks a lot for all your careful comments. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 28/28] drm/atomic-helper: Reject legacy flips on a disabled pipe
On Fri, Dec 04, 2015 at 09:46:09AM +0100, Daniel Vetter wrote:
> We want this for consistency with existing page_flip semantics.
>
> Since this spurred quite a discussion on IRC also document why we
> reject even generation when the pipe is off: It's not that it's hard
> to implement, but userspace has a track recording proofing that it's
> way too easy to accidentally abuse and cause havoc. We want to make
> sure userspace doesn't get away with that.
>
> Cc: Daniel Stone
> Cc: Ville Syrjälä
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_atomic.c| 9 +
> drivers/gpu/drm/drm_atomic_helper.c | 8
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 7426d40017a0..06cdb52907da 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1265,6 +1265,15 @@ int drm_atomic_check_only(struct drm_atomic_state
> *state)
> if (config->funcs->atomic_check)
> ret = config->funcs->atomic_check(state->dev, state);
>
> + /*
> + * Reject event generation for when a CRTC is off and stays off. It
> + * wouldn't be hard to implement this, but userspace has a track record
> + * of happily burning through 100% cpu (or worse, crash) when the
> + * display pipe is suspended. To avoid all that fun just reject updates
> + * that ask for events since likely that indicates a bug in the
> + * compositors drawing loop. This is consistent with the vblank ioctl
> + * which also rejects service on a disabled pipe.
> + */
> if (!state->allow_modeset) {
> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 110f3db8dd05..8e281a96c35f 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2283,6 +2283,14 @@ retry:
Argh! Once again that stupid 'put goto labes at col 0' rule making
it impossible to review a patch. Could we *please* change that to
put the labels at col 1 instead?
> goto fail;
> drm_atomic_set_fb_for_plane(plane_state, fb);
>
> + state->allow_modeset = false;
> + if (!crtc_state->active) {
> + DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n",
> + crtc->base.id);
> + ret = -EINVAL;
> + goto fail;
> + }
> +
> ret = drm_atomic_async_commit(state);
> if (ret != 0)
> goto fail;
> --
> 2.5.1
--
Ville Syrjälä
Intel OTC
[PATCH 24/28] drm: Document drm_connector_helper_funcs
On Fri, Dec 04, 2015 at 09:46:05AM +0100, Daniel Vetter wrote:
> Nothing special, except the somewhat awkard split in probe helper
"awkward"
> callbacks between here and drm_crtc_funcs.
>
> Signed-off-by: Daniel Vetter
> ---
> include/drm/drm_modeset_helper_vtables.h | 106
> +--
> 1 file changed, 101 insertions(+), 5 deletions(-)
>
> diff --git a/include/drm/drm_modeset_helper_vtables.h
> b/include/drm/drm_modeset_helper_vtables.h
> index 66b78c14154e..22cc51b278fb 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -264,18 +264,114 @@ static inline void drm_encoder_helper_add(struct
> drm_encoder *encoder,
>
> /**
> * struct drm_connector_helper_funcs - helper operations for connectors
> - * @get_modes: get mode list for this connector
> - * @mode_valid: is this mode valid on the given connector? (optional)
> - * @best_encoder: return the preferred encoder for this connector
> - * @atomic_best_encoder: atomic version of @best_encoder
> *
> - * The helper operations are called by the mid-layer CRTC helper.
> + * These functions are used by the atomic and legacy modeset helpers and by
> the
> + * probe helpers.
> */
> struct drm_connector_helper_funcs {
> + /**
> + * @get_modes:
> + *
> + * This function should fill in all modes currently valid for the sink
> + * into the connector->probe_modes function. It should also update the
What's probe_modes? I've never heard of it. Did you mean ->fill_modes()?
Also it's strange to say "fill into the ... function". Perhaps "pass
into the ... function" instead?
> + * EDID property by calling drm_mode_connector_update_edid_property().
> + *
> + * The usual way to implement this is to cache the EDID retrieved in the
> + * probe callback somewhere in the driver-private connector structure.
> + * In this function drivers then parse the modes in the EDID and add it
"add them"?
> + * by calling drm_add_edid_modes(). But connectors that driver a fixed
"drive"
> + * panel can also manually add specific modes using
> + * drm_mode_probed_add(). Finally drivers that support audio propably
"probably"
> + /**
> + * @mode_valid:
> + *
> + * Callback to validate a mode for a connector, irrespective of the
> + * specific display configuration.
> + *
> + * This callback is used by the probe helpers to filter the mode list
> + * (which is usually derived from the EDID data block from the sink).
> + * See e.g. drm_helper_probe_single_connector_modes().
> + *
> + * NOTE:
> + *
> + * This only filters the mode list supplied to userspace in the
> + * GETCONNECOTR ioctl. Userspace is free to create modes of its own and
"GETCONNECTOR IOCTL"
> + * ask the kernel to use it. It this case the atomic helpers or legacy
"to use them"
> + * CRTC heleprs will not call this function. Drivers therefore must
"helpers"
> + * still fully validate any mode passed in in a modeset request.
> + *
> + * RETURNS:
> + *
> + * Either DRM_MODE_OK or one of the failure reasons in enum
The enum value is MODE_OK, without the DRM_ prefix.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/b9ff3659/attachment-0001.sig>
[PATCH 26/28] drm: Move drm_display_mode an related docs into kerneldoc
On Fri, Dec 04, 2015 at 09:46:07AM +0100, Daniel Vetter wrote: > This was in the documentation for modeset helper hooks, where it is a > bit misplaced. > > v2: Reindent the drm_mode_status enum, inspired by Ville. > > Cc: ville.syrjala at linux.intel.com > Signed-off-by: Daniel Vetter > --- > Documentation/DocBook/gpu.tmpl | 192 --- > drivers/gpu/drm/drm_modes.c| 3 +- > include/drm/drm_modes.h| 342 > +++-- > 3 files changed, 297 insertions(+), 240 deletions(-) > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > index 5312f5a05798..86e5b12a49ba 100644 > --- a/Documentation/DocBook/gpu.tmpl > +++ b/Documentation/DocBook/gpu.tmpl > @@ -1758,198 +1758,6 @@ void intel_crt_init(struct drm_device *dev) > drm_add_edid_modes manually in that case. > > > -When adding modes manually the driver creates each mode with a > call to > -drm_mode_create and must fill the following > fields. > - > - > -__u32 type; > - > - Mode type bitmask, a combination of > - > - > - DRM_MODE_TYPE_BUILTIN > - not used? > - > - > - DRM_MODE_TYPE_CLOCK_C > - not used? > - > - > - DRM_MODE_TYPE_CRTC_C > - not used? > - > - > - > -DRM_MODE_TYPE_PREFERRED - The preferred mode for the connector > - > - > -not used? > - > - > - > - DRM_MODE_TYPE_DEFAULT > - not used? > - > - > - DRM_MODE_TYPE_USERDEF > - not used? > - > - > - DRM_MODE_TYPE_DRIVER > - > - > - The mode has been created by the driver (as > opposed to > - to user-created modes). > - > - > - > - > - Drivers must set the DRM_MODE_TYPE_DRIVER bit for all > modes they > - create, and set the DRM_MODE_TYPE_PREFERRED bit for the > preferred > - mode. > - > - > - > -__u32 clock; > -Pixel clock frequency in kHz unit > - > - > -__u16 hdisplay, hsync_start, hsync_end, htotal; > -__u16 vdisplay, vsync_start, vsync_end, vtotal; > -Horizontal and vertical timing information > - > - > - > -__u16 hskew; > -__u16 vscan; > -Unknown > - > - > -__u32 flags; > - > - Mode flags, a combination of > - > - > - DRM_MODE_FLAG_PHSYNC > - > -Horizontal sync is active high > - > - > - > - DRM_MODE_FLAG_NHSYNC > - > -Horizontal sync is active low > - > - > - > - DRM_MODE_FLAG_PVSYNC > - > -Vertical sync is active high > - > - > - > - DRM_MODE_FLAG_NVSYNC > - > -Vertical sync is active low > - > - > - > - DRM_MODE_FLAG_INTERLACE > - > -Mode is interlaced > - > - > - > - DRM_MODE_FLAG_DBLSCAN > - > -Mode uses doublescan > - > - > - > - DRM_MODE_FLAG_CSYNC > - > -Mode uses composite sync > - > - > - > - DRM_MODE_FLAG_PCSYNC > - > -Composite sync is active high > - > - > - > -
[PATCH v10 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory
On 12/07/2015 02:38 PM, Yakir Yang wrote:
> Split the dp core driver from exynos directory to bridge directory,
> and rename the core driver to analogix_dp_*, rename the platform
> code to exynos_dp.
>
> Beside the new analogix_dp driver would export six hooks.
> "analogix_dp_bind()" and "analogix_dp_unbind()"
> "analogix_dp_suspned()" and "analogix_dp_resume()"
> "analogix_dp_detect()" and "analogix_dp_get_modes()"
>
> The bind/unbind symbols is used for analogix platform driver to connect
> with analogix_dp core driver. And the detect/get_modes is used for analogix
> platform driver to init the connector.
>
> They reason why connector need register in helper driver is rockchip drm
> haven't implement the atomic API, but Exynos drm have implement it, so
> there would need two different connector helper functions, that's why we
> leave the connector register in helper driver.
>
> Signed-off-by: Yakir Yang
> Tested-by: Javier Martinez Canillas
> ---
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> similarity index 50%
> rename from drivers/gpu/drm/exynos/exynos_dp_core.c
> rename to drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index aee3262..cd86413 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>
> -static int exynos_dp_remove(struct platform_device *pdev)
> +void analogix_dp_unbind(struct device *dev, struct device *master,
> + void *data)
> {
> - pm_runtime_disable(&pdev->dev);
> - component_del(&pdev->dev, &exynos_dp_ops);
> + struct analogix_dp_device *dp = dev_get_drvdata(dev);
>
> - return 0;
> + analogix_dp_bridge_disable(dp->bridge);
> + pm_runtime_disable(&pdev->dev);
> }
> +EXPORT_SYMBOL_GPL(analogix_dp_unbind);
>
Sorry to introduce an compiled error, my test 4.4-rc3 kernel have some
different with linux-next kernel, so i used to write the patches on next
kernel,
and back the changes to my 4.4 kernel to test.
I already found this error when I'm testing, but forget to reserver this
changes
to linux-next kernel, I would send a new version to fix this.
- Yakir
[Intel-gfx] [PATCH 28/28] drm/atomic-helper: Reject legacy flips on a disabled pipe
Hi,
On 4 December 2015 at 08:46, Daniel Vetter wrote:
> + /*
> +* Reject event generation for when a CRTC is off and stays off. It
> +* wouldn't be hard to implement this, but userspace has a track
> record
> +* of happily burning through 100% cpu (or worse, crash) when the
> +* display pipe is suspended. To avoid all that fun just reject
> updates
> +* that ask for events since likely that indicates a bug in the
> +* compositors drawing loop. This is consistent with the vblank ioctl
> +* which also rejects service on a disabled pipe.
> +*/
Sorry to keep whingeing, but this isn't actually related to event
generation. To the best of my knowledge, this change does a few
things. Firstly, comments the check above (enforcing that (flags &
ALLOW_MODESET) == {crtcs}->allow_modeset), which is good. But the
comment is incorrect, because whether or not an event is requested is
wholly unrelated. Secondly, it disables allow_modeset on pageflip,
which shouldn't be changing DPMS stage (good!). Thirdly, it enforces
something like the above statement on pageflips, except again it has
no regard to events: it just enforces the no-DPMS-on-flip rule, for
which event generation is a subset.
If you fix the above comment to more accurately note that this just
enforces that you need the ALLOW_MODESET flag to change active, mode
or connector routing, and (as Thierry asked), add a comment below to
note that we enforce existing no-DPMS-on-flip semantics in the helper,
then you're welcome to my R-b. But please don't mention events in the
new comment. :)
Cheers,
Daniel
[PATCH 23/28] drm: Document drm_plane_helper_funcs
optional. > + */ > void (*cleanup_fb)(struct drm_plane *plane, > const struct drm_plane_state *old_state); > > + /** > + * @atomic_check: > + * > + * Drivers should check plane specific constraints in this hook. > + * > + * When using drm_atomic_helper_check_planes plane's ->atomic_check() Parentheses after the function name. > + * hooks are called before the ones for CRTCs, which allows drivers to > + * request shared resources that the CRTC controls here. For more > + * complicated depencies the driver can call the provided check helpers "dependencies" > + * multiple times until the computed state has a final configuration and > + * everything has been checked. > + * > + * This function is also allowed to inspect any other object's state and > + * can add more state objects to the atomic commit if needed. Care must > + * be taken though to ensure that state check&compute functions for > + * these added states are all called, and derived state in other objects > + * all updated. Again the recommendation is to just call check helpers > + * until a maximal configuration is reached. > + * > + * This callback is used by the atomic modeset helpers and by the > + * transitional plane helpers, but it is optional. > + * > + * NOTE: > + * > + * This function is called in the check phase of an atomic update. The > + * driver is not allowed to change anything outside of the free-standing > + * state objects passed-in or assembled in the overall &drm_atomic_state > + * update tracking structure. > + * > + * RETURNS: > + * > + * 0 on success, -EINVAL if the state or the transition can't be > + * support, -ENOMEM on memory allocation failure and -EDEADLK if an "supported" > + * attempt to obtain another state object ran into a &drm_modeset_lock > + * deadlock. > + */ > int (*atomic_check)(struct drm_plane *plane, > struct drm_plane_state *state); > + > + /** > + * @atomic_update: > + * > + * Drivers should use this function to update the plane state. This > + * hook is called in-between the ->atomic_begin and "->atomic_begin()" > + * ->atomic_flush() of &drm_crtc_helper_funcs. > + * > + * Note that the power state of the display pipe when this function is > + * called depends upon the exact helpers and calling sequence the driver > + * has picked. See drm_atomic_commit_planes for a discussion of the Parentheses. > + * tradeoffs and variants of plane commit helpers. > + * > + * This callback is used by the atomic modeset helpers and by the > + * transitional plane helpers, but it is optional. > + */ > void (*atomic_update)(struct drm_plane *plane, > struct drm_plane_state *old_state); > + /** > + * @atomic_disable: > + * > + * Drivers should use this function to unconditionally disable a plane. > + * This hook is called in-between the ->atomic_begin and > + * ->atomic_flush() of &drm_crtc_helper_funcs. It is an alternative to > + * @atomic_update, which will be called for disabling planes, too, if > + * the @atomic_disable hook isn't implemented. > + * > + * This hook is also useful to disable planes in preration of a modeset, "preparation" > + * by calling drm_atomic_helper_disable_planes_on_crtc() from the > + * ->disable hook in &drm_crtc_helper_funcs. "->disable()" > + * > + * Note that the power state of the display pipe when this function is > + * called depends upon the exact helpers and calling sequence the driver > + * has picked. See drm_atomic_commit_planes for a discussion of the Parentheses. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/ae8681f8/attachment.sig>
[PATCH 0/4] Component helper updates
Given the lack of interest in these patches, I've put these into my "for-next" branch so that they can get some exposure in linux-next. On Mon, Nov 23, 2015 at 04:02:11PM +, Russell King - ARM Linux wrote: > Greg, > > These four patches update the component helper by: > * Removing the legacy matching code with the .add_components method > in the master's operation structure. Nothing in -rc1 appears to be > using the legacy functions or method, according to my git greps. > > * A slight code reorganisation which results in slightly easier to read > code. > > * Switch to tracking components via an array rather than a list, which > allows us to keep the matching and matched components together, and > more importantly allows us to reduce the amount of matching - with > this structure, we can incrementally add the component devices as > they become available, rather than re-running the list of matches > each time something changes. > > * Fix the lack of match release functionality, which Liviu Dudau > reminded me was missing. This allows people who want to pass > device_node structures in to (correctly) retain the reference to the > node, and drop the node when the need to do matches is no longer > required. > > The first three patches have been well tested over the last year as I've > had them in my tree that long. I hadn't considered them important enough > to send as they're only removing and cleaning up functionality. However, > with the need to fix something, it now makes sense to get them merged. > > The last patch has been tested with etnaviv DRM to prove that the match > release works by unloading the module. On unload, the release function > is correctly called. > > This is intended for the next merge window. Please apply. > > drivers/base/component.c | 281 > -- > include/linux/component.h | 33 -- > 2 files changed, 167 insertions(+), 147 deletions(-) > > -- > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net. > > ___ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[PATCH v3 2/5] drm/dsi: Try to match non-DT dsi devices
On 12/07/2015 02:40 PM, Jani Nikula wrote:
> On Mon, 07 Dec 2015, Archit Taneja wrote:
>> On 12/07/2015 02:15 PM, Jani Nikula wrote:
>>> On Mon, 07 Dec 2015, Archit Taneja wrote:
Hi,
On 11/30/2015 06:15 PM, kbuild test robot wrote:
> Hi Archit,
>
> [auto build test ERROR on: v4.4-rc3]
> [also build test ERROR on: next-20151127]
>
> url:
> https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
> config: x86_64-allyesdebian (attached as .config)
> reproduce:
># save the attached .config to linux build tree
>make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
> drivers/gpu/drm/drm_mipi_dsi.c: In function
> 'of_mipi_dsi_device_add':
>>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of
>>> function 'of_modalias_node' [-Werror=implicit-function-declaration]
> if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
depend on CONFIG_OF?
>>>
>>> Please don't.
>>
>> Just curious, how did x86 use DSI if the only way to create DSI devices
>> until now was via DT?
>
> Oh, you want the gory details... we use the DSI code as a library for
> abstraction and helpers, without actually creating or registering the
> devices.
Okay, got it. I'll go with the IS_ENABLED(CONFIG_OF) approach.
Humble request: Next time if I share something that doesn't make sense,
please reply with something more than a "Please don't". That just sounds
condescending and doesn't really help me with my cause either.
Thanks,
Archit
>
> BR,
> Jani.
>
>
>>
>> Archit
>>
>>>
>>> BR,
>>> Jani.
>>>
>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, hosted by The Linux Foundation
[PATCH v10 17/17] drm: bridge: analogix/dp: expand the look time for waiting AUX CH reply
After test on rockchiop platform, i found sometims driver would failed
at reading EDID message. After debugging more, i found that it's okay
to read_a byte from i2c, but it would failed at AUX transcation if we
try to ready multi-bytes from i2c.
Driver just can't received the AUX CH reply command, even no AUX error
code. I may guess that the AUX wait time is not enough, so I try to
expand the AUX wait time, and i do see this could fix the EDID read
failed at rockchip platform.
And I thought that expand the wait time won't hurt Exynos platform too
much, cause Exynos didn't have this problem, then driver would received
the reply command very soon, so no more additional wait time would bring
to Exynos platform.
Signed-off-by: Yakir Yang
---
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index c7e2959..dc376bd 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -482,7 +482,7 @@ int analogix_dp_start_aux_transaction(struct
analogix_dp_device *dp)
reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
while (!(reg & RPLY_RECEIV)) {
timeout_loop++;
- if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
+ if (DP_TIMEOUT_LOOP_COUNT * 10 < timeout_loop) {
dev_err(dp->dev, "AUX CH command reply failed!\n");
return -ETIMEDOUT;
}
--
1.9.1
[PATCH v10 16/17] drm: bridge: analogix/dp: add edid modes parse in get_modes method
Display Port monitor could support kinds of mode which indicate
in monitor edid, not just one single display resolution which
defined in panel or devivetree property display timing.
Note: Gustavo Padovan try to remove the controller and phy
power on function in bind time at bellow commit:
drm/exynos: do not start enabling DP at bind() phase
But for now driver need to read edid message in .get_modes()
function, so controller must be inited in bind time, so we
need to add controller init back.
Signed-off-by: Yakir Yang
Tested-by: Javier Martinez Canillas
---
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4:
- Call drm_panel_prepare() in .get_modes function, ensure panel should
power on before driver try to read edid message.
Changes in v3:
- Add edid modes parse support
Changes in v2: None
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 26
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 46 +++---
2 files changed, 42 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 0f42d73..aa1a9be 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -107,7 +107,7 @@ static unsigned char
analogix_dp_calc_edid_check_sum(unsigned char *edid_data)
static int analogix_dp_read_edid(struct analogix_dp_device *dp)
{
- unsigned char edid[EDID_BLOCK_LENGTH * 2];
+ unsigned char *edid = dp->edid;
unsigned int extend_block = 0;
unsigned char sum;
unsigned char test_vector;
@@ -901,12 +901,6 @@ static void analogix_dp_commit(struct analogix_dp_device
*dp)
DRM_ERROR("failed to disable the panel\n");
}
- ret = analogix_dp_handle_edid(dp);
- if (ret) {
- dev_err(dp->dev, "unable to handle edid\n");
- return;
- }
-
ret = analogix_dp_set_link_train(dp, dp->video_info.max_lane_count,
dp->video_info.max_link_rate);
if (ret) {
@@ -947,8 +941,24 @@ EXPORT_SYMBOL_GPL(analogix_dp_detect);
int analogix_dp_get_modes(struct device *dev)
{
struct analogix_dp_device *dp = dev_get_drvdata(dev);
+ struct edid *edid = (struct edid *)dp->edid;
int num_modes = 0;
+ if (dp->plat_data && dp->plat_data->panel) {
+ if (drm_panel_prepare(dp->plat_data->panel)) {
+ DRM_ERROR("failed to setup the panel\n");
+ return -EINVAL;
+ }
+ }
+
+ if (analogix_dp_handle_edid(dp)) {
+ dev_err(dp->dev, "unable to handle edid\n");
+ return -EINVAL;
+ }
+
+ drm_mode_connector_update_edid_property(dp->connector, edid);
+ num_modes += drm_add_edid_modes(dp->connector, edid);
+
if (dp->plat_data->panel)
num_modes += drm_panel_get_modes(dp->plat_data->panel);
@@ -1309,6 +1319,8 @@ int analogix_dp_bind(struct device *dev, struct
drm_device *drm_dev,
phy_power_on(dp->phy);
+ analogix_dp_init_dp(dp);
+
ret = devm_request_irq(&pdev->dev, dp->irq, analogix_dp_irq_handler,
irq_flags, "analogix-dp", dp);
if (ret) {
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index d3c7e0a..2bd2e0d 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -20,6 +20,28 @@
#define MAX_CR_LOOP 5
#define MAX_EQ_LOOP 5
+/* I2C EDID Chip ID, Slave Address */
+#define I2C_EDID_DEVICE_ADDR 0x50
+#define I2C_E_EDID_DEVICE_ADDR 0x30
+
+#define EDID_BLOCK_LENGTH 0x80
+#define EDID_HEADER_PATTERN0x00
+#define EDID_EXTENSION_FLAG0x7e
+#define EDID_CHECKSUM 0x7f
+
+/* DP_MAX_LANE_COUNT */
+#define DPCD_ENHANCED_FRAME_CAP(x) (((x) >> 7) & 0x1)
+#define DPCD_MAX_LANE_COUNT(x) ((x) & 0x1f)
+
+/* DP_LANE_COUNT_SET */
+#define DPCD_LANE_COUNT_SET(x) ((x) & 0x1f)
+
+/* DP_TRAINING_LANE0_SET */
+#define DPCD_PRE_EMPHASIS_SET(x) (((x) & 0x3) << 3)
+#define DPCD_PRE_EMPHASIS_GET(x) (((x) >> 3) & 0x3)
+#define DPCD_VOLTAGE_SWING_SET(x) (((x) & 0x3) << 0)
+#define DPCD_VOLTAGE_SWING_GET(x) (((x) >> 0) & 0x3)
+
enum link_rate_type {
LINK_RATE_1_62GBPS = DP_LINK_BW_1_62,
LINK_RATE_2_70GBPS = DP_LINK_BW_2_7,
@@ -161,6 +183,7 @@ struct analogix_dp_device {
int dpms_mode;
int hpd_gpio;
boolneed_force_hpd;
+ unsigned char edid[EDID_B
[PATCH v10 15/17] drm: bridge: analogix/dp: move hpd detect to connector detect function
This change just make a little clean to make code more like
drm core expect, move hdp detect code from bridge->enable(),
and place them into connector->detect().
Note: Gustavo Padovan try to remove the controller and phy
power on function in bind time at bellow commit:
drm/exynos: do not start enabling DP at bind() phase
But for now the connector status don't hardcode to connected,
need to operate dp phy in .detect function, so we need to revert
parts if Gustavo Padovan's changes, add phy poweron
function in bind time.
Signed-off-by: Yakir Yang
Tested-by: Javier Martinez Canillas
---
Changes in v10:
- Revert parts of Gustavo Padovan's changes in commit:
drm/exynos: do not start enabling DP at bind() phase
Add dp phy poweron function in bind time.
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4:
- Take Jingoo suggest, add commit messages.
Changes in v3:
- move dp hpd detect to connector detect function.
Changes in v2: None
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 94968e4..0f42d73 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -901,12 +901,6 @@ static void analogix_dp_commit(struct analogix_dp_device
*dp)
DRM_ERROR("failed to disable the panel\n");
}
- ret = analogix_dp_detect_hpd(dp);
- if (ret) {
- /* Cable has been disconnected, we're done */
- return;
- }
-
ret = analogix_dp_handle_edid(dp);
if (ret) {
dev_err(dp->dev, "unable to handle edid\n");
@@ -941,6 +935,11 @@ static void analogix_dp_commit(struct analogix_dp_device
*dp)
enum drm_connector_status analogix_dp_detect(struct device *dev, bool force)
{
+ struct analogix_dp_device *dp = dev_get_drvdata(dev);
+
+ if (analogix_dp_detect_hpd(dp))
+ return connector_status_disconnected;
+
return connector_status_connected;
}
EXPORT_SYMBOL_GPL(analogix_dp_detect);
@@ -1308,6 +1307,8 @@ int analogix_dp_bind(struct device *dev, struct
drm_device *drm_dev,
pm_runtime_enable(dev);
+ phy_power_on(dp->phy);
+
ret = devm_request_irq(&pdev->dev, dp->irq, analogix_dp_irq_handler,
irq_flags, "analogix-dp", dp);
if (ret) {
--
1.9.1
[PATCH 22/28] drm/atomic-helper: Reject attempts at re-stealing encoders
On Mon, Dec 07, 2015 at 02:26:04PM +0100, Thierry Reding wrote:
> On Fri, Dec 04, 2015 at 09:46:03AM +0100, Daniel Vetter wrote:
> > This can happen when we run out of encoders for a multi-crtc modeset,
> > or also when userspace is silly and tries to clone multiple connectors
> > that need the same encoder on the same crtc.
> >
> > Reported-and-Tested-and-Reviewed-by: Maarten Lankhorst > at linux.intel.com>
> > Cc: Maarten Lankhorst
> > Signed-off-by: Daniel Vetter
Oops that patch shouldn't have been in here. And it's already merged -
somehow I botched the rebase before sending out this series and it didn't
drop out. I'll address your feedback with a follow-up patch.
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 28
> > 1 file changed, 28 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 2cf8ab7dbc8c..ab275499d2a3 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -86,6 +86,27 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state
> > *state,
> > }
> > }
> >
> > +static bool
> > +check_pending_encoder_assignment(struct drm_atomic_state *state,
> > +struct drm_encoder *new_encoder,
> > +struct drm_connector *new_connector)
>
> new_connector seems to be unused.
>
> > +{
> > + struct drm_connector *connector;
> > + struct drm_connector_state *conn_state;
> > + int i;
> > +
> > + for_each_connector_in_state(state, connector, conn_state, i) {
> > + if (conn_state->best_encoder != new_encoder)
> > + continue;
> > +
> > + /* encoder already assigned and we're trying to re-steal it! */
> > + if (connector->state->best_encoder != conn_state->best_encoder)
>
> Was this supposed to be new_connector->state->best_encoder?
Nah, new_connector was just leftovers from an earlier, broken version of
this. The function really only checks whether the given encoder has
alreaddy been reassigned in this update. That's enough to decide that
there's a conflict and reject the modeset. I'll do a patch to drop the
superflous argument.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH v10 14/17] drm: bridge: analogix/dp: try force hpd after plug in lookup failed
Some edp screen do not have hpd signal, so we can't just return
failed when hpd plug in detect failed.
This is an hardware property, so we need add a devicetree property
"analogix,need-force-hpd" to indicate this sutiation.
Signed-off-by: Yakir Yang
Tested-by: Javier Martinez Canillas
---
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3:
- Add "analogix,need-force-hpd" to indicate whether driver need foce
hpd when hpd detect failed.
Changes in v2: None
.../bindings/display/bridge/analogix_dp.txt| 4 ++-
.../bindings/display/exynos/exynos_dp.txt | 1 +
.../display/rockchip/analogix_dp-rockchip.txt | 1 +
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 36 +++---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 ++
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 9 ++
6 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
index 7659a7a..74f0e80 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
+++ b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
@@ -22,6 +22,9 @@ Required properties for dp-controller:
from general PHY binding: Should be "dp".
Optional properties for dp-controller:
+ -analogix,need-force-hpd:
+ Indicate driver need force hpd when hpd detect failed, this
+ is used for some eDP screen which don't have hpd signal.
-hpd-gpios:
Hotplug detect GPIO.
Indicates which GPIO should be used for hotplug detection
@@ -31,7 +34,6 @@ Optional properties for dp-controller:
* Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
*
Documentation/devicetree/bindings/video/analogix_dp-rockchip.txt
-
[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
---
diff --git a/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
b/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
index 9905081..8800164 100644
--- a/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
+++ b/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
@@ -41,6 +41,7 @@ For the below properties, please refer to Analogix DP binding
document:
-phys (required)
-phy-names (required)
-hpd-gpios (optional)
+ -analogix,need-force-hpd (optional)
-video interfaces (optional)
Deprecated properties for DisplayPort:
diff --git
a/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
b/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
index dae86c4..1f1e594 100644
---
a/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
+++
b/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
@@ -32,6 +32,7 @@ For the below properties, please refer to Analogix DP binding
document:
- phys (required)
- phy-names (required)
- hpd-gpios (optional)
+- analogix,need-force-hpd (optional)
---
Example:
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index a11504b..94968e4 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -59,15 +59,38 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device
*dp)
{
int timeout_loop = 0;
- while (analogix_dp_get_plug_in_status(dp) != 0) {
+ while (timeout_loop < DP_TIMEOUT_LOOP_COUNT) {
+ if (analogix_dp_get_plug_in_status(dp) == 0)
+ return 0;
+
timeout_loop++;
- if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
- dev_err(dp->dev, "failed to get hpd plug status\n");
- return -ETIMEDOUT;
- }
usleep_range(10, 11);
}
+ /*
+* Some edp screen do not have hpd signal, so we can't just
+* return failed when hpd plug in detect failed, DT property
+* "need-force-hpd" would indicate whether driver need this.
+*/
+ if (!dp->need_force_hpd)
+ return -ETIMEDOUT;
+
+ /*
+* The eDP TRM indicate that if HPD_STATUS(RO) is 0, AUX CH
+* will not work, so we need to give a force hpd action to
+* set HPD_STATUS manually.
+*/
+ dev_dbg(dp->dev, "failed to get hpd plug status, try to force hpd\n");
+
+ analogix_dp_force_hpd(dp);
+
+ if (analogix_dp_get_plug_in_status(dp) != 0) {
+ dev_err(dp->dev,
[PATCH v10 13/17] drm: bridge: analogix/dp: add max link rate and lane count limit for RK3288
There are some IP limit on rk3288 that only support 4 physical lanes
of 2.7/1.6 Gbps/lane, so seprate them out by device_type flag.
Signed-off-by: Yakir Yang
Tested-by: Javier Martinez Canillas
---
Changes in v10:
- Remove the surplus "plat_data" check. (Heiko)
- switch (dp->plat_data && dp->plat_data->dev_type) {
+ switch (dp->plat_data->dev_type) {
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4:
- Seprate the link-rate and lane-count limit out with the device_type
flag. (Thierry)
Changes in v3: None
Changes in v2: None
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 33 ++
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 4 +--
2 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 5f542b7..a11504b 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -890,8 +890,8 @@ static void analogix_dp_commit(struct analogix_dp_device
*dp)
return;
}
- ret = analogix_dp_set_link_train(dp, dp->video_info.lane_count,
-dp->video_info.link_rate);
+ ret = analogix_dp_set_link_train(dp, dp->video_info.max_lane_count,
+dp->video_info.max_link_rate);
if (ret) {
dev_err(dp->dev, "unable to do link train\n");
return;
@@ -1158,16 +1158,25 @@ static int analogix_dp_dt_parse_pdata(struct
analogix_dp_device *dp)
struct device_node *dp_node = dp->dev->of_node;
struct video_info *video_info = &dp->video_info;
- if (of_property_read_u32(dp_node, "samsung,link-rate",
-&video_info->link_rate)) {
- dev_err(dp->dev, "failed to get link-rate\n");
- return -EINVAL;
- }
-
- if (of_property_read_u32(dp_node, "samsung,lane-count",
-&video_info->lane_count)) {
- dev_err(dp->dev, "failed to get lane-count\n");
- return -EINVAL;
+ switch (dp->plat_data->dev_type) {
+ case RK3288_DP:
+ /*
+* Like Rk3288 DisplayPort TRM indicate that "Main link
+* containing 4 physical lanes of 2.7/1.62 Gbps/lane".
+*/
+ video_info->max_link_rate = 0x0A;
+ video_info->max_lane_count = 0x04;
+ break;
+ case EXYNOS_DP:
+ /*
+* NOTE: those property parseing code is used for
+* providing backward compatibility for samsung platform.
+*/
+ of_property_read_u32(dp_node, "samsung,link-rate",
+&video_info->max_link_rate);
+ of_property_read_u32(dp_node, "samsung,lane-count",
+&video_info->max_lane_count);
+ break;
}
return 0;
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index e37cef6..e6f8243 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -129,8 +129,8 @@ struct video_info {
enum color_coefficient ycbcr_coeff;
enum color_depth color_depth;
- enum link_rate_type link_rate;
- enum link_lane_count_type lane_count;
+ enum link_rate_type max_link_rate;
+ enum link_lane_count_type max_lane_count;
};
struct link_train {
--
1.9.1
[PATCH v10 12/17] drm: bridge: analogix/dp: add some rk3288 special registers setting
RK3288 need some special registers setting, we can separate
them out by the dev_type of plat_data.
Signed-off-by: Yakir Yang
Tested-by: Javier Martinez Canillas
---
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Fix compile failed dut to phy_pd_addr variable misspell error
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 76 ++-
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h | 12
2 files changed, 60 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 861097a..21a3287 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -15,6 +15,8 @@
#include
#include
+#include
+
#include "analogix_dp_core.h"
#include "analogix_dp_reg.h"
@@ -72,6 +74,14 @@ void analogix_dp_init_analog_param(struct analogix_dp_device
*dp)
reg = SEL_24M | TX_DVDD_BIT_1_0625V;
writel(reg, dp->reg_base + ANALOGIX_DP_ANALOG_CTL_2);
+ if (dp->plat_data && (dp->plat_data->dev_type == RK3288_DP)) {
+ writel(REF_CLK_24M, dp->reg_base + ANALOGIX_DP_PLL_REG_1);
+ writel(0x95, dp->reg_base + ANALOGIX_DP_PLL_REG_2);
+ writel(0x40, dp->reg_base + ANALOGIX_DP_PLL_REG_3);
+ writel(0x58, dp->reg_base + ANALOGIX_DP_PLL_REG_4);
+ writel(0x22, dp->reg_base + ANALOGIX_DP_PLL_REG_5);
+ }
+
reg = DRIVE_DVDD_BIT_1_0625V | VCO_BIT_600_MICRO;
writel(reg, dp->reg_base + ANALOGIX_DP_ANALOG_CTL_3);
@@ -206,81 +216,85 @@ void analogix_dp_set_analog_power_down(struct
analogix_dp_device *dp,
bool enable)
{
u32 reg;
+ u32 phy_pd_addr = ANALOGIX_DP_PHY_PD;
+
+ if (dp->plat_data && (dp->plat_data->dev_type == RK3288_DP))
+ phy_pd_addr = ANALOGIX_DP_PD;
switch (block) {
case AUX_BLOCK:
if (enable) {
- reg = readl(dp->reg_base + ANALOGIX_DP_PHY_PD);
+ reg = readl(dp->reg_base + phy_pd_addr);
reg |= AUX_PD;
- writel(reg, dp->reg_base + ANALOGIX_DP_PHY_PD);
+ writel(reg, dp->reg_base + phy_pd_addr);
} else {
- reg = readl(dp->reg_base + ANALOGIX_DP_PHY_PD);
+ reg = readl(dp->reg_base + phy_pd_addr);
reg &= ~AUX_PD;
- writel(reg, dp->reg_base + ANALOGIX_DP_PHY_PD);
+ writel(reg, dp->reg_base + phy_pd_addr);
}
break;
case CH0_BLOCK:
if (enable) {
- reg = readl(dp->reg_base + ANALOGIX_DP_PHY_PD);
+ reg = readl(dp->reg_base + phy_pd_addr);
reg |= CH0_PD;
- writel(reg, dp->reg_base + ANALOGIX_DP_PHY_PD);
+ writel(reg, dp->reg_base + phy_pd_addr);
} else {
- reg = readl(dp->reg_base + ANALOGIX_DP_PHY_PD);
+ reg = readl(dp->reg_base + phy_pd_addr);
reg &= ~CH0_PD;
- writel(reg, dp->reg_base + ANALOGIX_DP_PHY_PD);
+ writel(reg, dp->reg_base + phy_pd_addr);
}
break;
case CH1_BLOCK:
if (enable) {
- reg = readl(dp->reg_base + ANALOGIX_DP_PHY_PD);
+ reg = readl(dp->reg_base + phy_pd_addr);
reg |= CH1_PD;
- writel(reg, dp->reg_base + ANALOGIX_DP_PHY_PD);
+ writel(reg, dp->reg_base + phy_pd_addr);
} else {
- reg = readl(dp->reg_base + ANALOGIX_DP_PHY_PD);
+ reg = readl(dp->reg_base + phy_pd_addr);
reg &= ~CH1_PD;
- writel(reg, dp->reg_base + ANALOGIX_DP_PHY_PD);
+ writel(reg, dp->reg_base + phy_pd_addr);
}
break;
case CH2_BLOCK:
if (enable) {
- reg = readl(dp->reg_base + ANALOGIX_DP_PHY_PD);
+ reg = readl(dp->reg_base + phy_pd_addr);
reg |= CH2_PD;
- writel(reg, dp->reg_base + ANALOGIX_DP_PHY_PD);
+ writel(reg, dp->reg_base + phy_pd_addr);
} else {
- reg = readl(dp->reg_base + ANALOGIX_DP_PHY_PD);
+ reg = readl(dp->reg_base + phy_pd_addr);
reg &= ~CH2_PD;
- writel(reg, dp->reg_base + ANALOGIX_DP_PHY_PD);
+ writel(reg, dp->r
[PATCH v10 11/17] drm: rockchip: vop: add bpc and color mode setting
From: Mark Yao
Add bpc and color mode setting in rockchip_drm_vop driver, so
connector could try to use the edid drm_display_info to config
vop output mode.
Signed-off-by: Mark Yao
Signed-off-by: Yakir Yang
---
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5:
- Fix compiled error (Heiko)
- Using the connector display info message to configure eDP driver input
video mode, but hard code CRTC video output mode to RGBaaa.
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 25 +++
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 2 +-
drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 +-
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 32 ++---
4 files changed, 51 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 2c82a9a..3990951 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -180,14 +180,29 @@ static void rockchip_dp_drm_encoder_mode_set(struct
drm_encoder *encoder,
static void rockchip_dp_drm_encoder_prepare(struct drm_encoder *encoder)
{
struct rockchip_dp_device *dp = to_dp(encoder);
+ struct drm_connector *cn = &dp->connector;
+ int ret = -1;
u32 val;
- int ret;
- ret = rockchip_drm_crtc_mode_config(encoder->crtc,
- DRM_MODE_CONNECTOR_eDP,
- ROCKCHIP_OUT_MODE_);
+ /*
+* FIXME(Yakir): driver should configure the CRTC output video
+* mode with the display information which indicated the monitor
+* support colorimetry.
+*
+* But don't know why the CRTC driver seems could only output the
+* RGBaaa rightly. For example, if connect the "innolux,n116bge"
+* eDP screen, EDID would indicated that screen only accepted the
+* 6bpc mode. But if I configure CRTC to RGB666 output, then eDP
+* screen would show a blue picture (RGB888 show a green picture).
+* But if I configure CTRC to RGBaaa, and eDP driver still keep
+* RGB666 input video mode, then screen would works prefect.
+*/
+ if (cn->display_info.color_formats & DRM_COLOR_FORMAT_RGB444)
+ ret = rockchip_drm_crtc_mode_config(encoder->crtc,
+ DRM_MODE_CONNECTOR_eDP,
+ 10, DRM_COLOR_FORMAT_RGB444);
if (ret < 0) {
- dev_err(dp->dev, "Could not set crtc mode config: %d.\n", ret);
+ dev_err(dp->dev, "Could not set crtc mode config (%d)\n", ret);
return;
}
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 80d6fc8..428a3c1 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -215,7 +215,7 @@ static void dw_hdmi_rockchip_encoder_commit(struct
drm_encoder *encoder)
static void dw_hdmi_rockchip_encoder_prepare(struct drm_encoder *encoder)
{
rockchip_drm_crtc_mode_config(encoder->crtc, DRM_MODE_CONNECTOR_HDMIA,
- ROCKCHIP_OUT_MODE_);
+ 10, DRM_COLOR_FORMAT_RGB444);
}
static struct drm_encoder_helper_funcs dw_hdmi_rockchip_encoder_helper_funcs =
{
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index dc4e5f0..ef1d7fb 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -59,7 +59,7 @@ void rockchip_unregister_crtc_funcs(struct drm_device *dev,
int pipe);
int rockchip_drm_encoder_get_mux_id(struct device_node *node,
struct drm_encoder *encoder);
int rockchip_drm_crtc_mode_config(struct drm_crtc *crtc, int connector_type,
- int out_mode);
+ int bpc, int color);
int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
struct device *dev);
void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 5d8ae5e..9ef4a1f 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1062,14 +1062,40 @@ static const struct drm_plane_funcs vop_plane_funcs = {
int rockchip_drm_crtc_mode_config(struct drm_crtc *crtc,
int connector_type,
- int out_mode)
+ int bpc, int color)
{
struct vop *vop = to_vop(crtc);
+ /*
+* RK3288 vo
[PATCH v10 10/17] dt-bindings: add document for rockchip dp phy
Add dt binding documentation for rockchip display port PHY.
Signed-off-by: Yakir Yang
Reviewed-by: Heiko Stuebner
---
Changes in v10: None
Changes in v9: None
Changes in v8:
- Remove the specific address in the example node name. (Heiko)
Changes in v7:
- Simplify the commit message. (Kishon)
Changes in v6: None
Changes in v5:
- Split binding doc's from driver changes. (Rob)
- Update the rockchip,grf explain in document, and correct the clock required
elemets in document. (Rob & Heiko)
Changes in v4: None
Changes in v3: None
Changes in v2: None
.../devicetree/bindings/phy/rockchip-dp-phy.txt| 22 ++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/rockchip-dp-phy.txt
diff --git a/Documentation/devicetree/bindings/phy/rockchip-dp-phy.txt
b/Documentation/devicetree/bindings/phy/rockchip-dp-phy.txt
new file mode 100644
index 000..00902cb
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/rockchip-dp-phy.txt
@@ -0,0 +1,22 @@
+Rockchip Soc Seroes Display Port PHY
+
+
+Required properties:
+- compatible : should be one of the following supported values:
+- "rockchip.rk3288-dp-phy"
+- clocks: from common clock binding: handle to dp clock.
+ of memory mapped region.
+- clock-names: from common clock binding:
+ Required elements: "24m"
+- rockchip,grf: phandle to the syscon managing the "general register files"
+- #phy-cells : from the generic PHY bindings, must be 0;
+
+Example:
+
+edp_phy: edp-phy {
+ compatible = "rockchip,rk3288-dp-phy";
+ rockchip,grf = <&grf>;
+ clocks = <&cru SCLK_EDP_24M>;
+ clock-names = "24m";
+ #phy-cells = <0>;
+};
--
1.9.1
[PATCH v10 09/17] phy: Add driver for rockchip Display Port PHY
Add phy driver for the Rockchip DisplayPort PHY module. This
is required to get DisplayPort working in Rockchip SoCs.
Signed-off-by: Yakir Yang
Reviewed-by: Heiko Stuebner
---
Changes in v10:
- Fix the wrong macro value of GRF_EDP_REF_CLK_SEL_INTER_HIWORD_MASK
BIT(4) -> BIT(20)
Changes in v9:
- Removed the unused the variable "res" in probe function. (Heiko)
- Removed the unused head file.
Changes in v8:
- Fix the mixed spacers on macro definitions. (Heiko)
- Remove the unnecessary empty line after clk_prepare_enable. (Heiko)
Changes in v7:
- Simply the commit message. (Kishon)
- Symmetrical enable/disbale the phy clock and power. (Kishon)
Changes in v6: None
Changes in v5:
- Remove "reg" DT property, cause driver could poweron/poweroff phy via
the exist "grf" syscon already. And rename the example DT node from
"edp_phy: phy at ff770274" to "edp_phy: edp-phy" directly. (Heiko)
- Add deivce_node at the front of driver, update phy_ops type from "static
struct" to "static const struct". And correct the input paramters of
devm_phy_create() interfaces. (Heiko)
Changes in v4:
- Add commit message, and remove the redundant rockchip_dp_phy_init()
function, move those code to probe() method. And remove driver .owner
number. (Kishon)
Changes in v3:
- Suggest, add rockchip dp phy driver, collect the phy clocks and
power control. (Heiko)
Changes in v2: None
drivers/phy/Kconfig | 7 ++
drivers/phy/Makefile | 1 +
drivers/phy/phy-rockchip-dp.c | 151 ++
3 files changed, 159 insertions(+)
create mode 100644 drivers/phy/phy-rockchip-dp.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 7eb5859d..7355819 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -319,6 +319,13 @@ config PHY_ROCKCHIP_USB
help
Enable this to support the Rockchip USB 2.0 PHY.
+config PHY_ROCKCHIP_DP
+ tristate "Rockchip Display Port PHY Driver"
+ depends on ARCH_ROCKCHIP && OF
+ select GENERIC_PHY
+ help
+ Enable this to support the Rockchip Display Port PHY.
+
config PHY_ST_SPEAR1310_MIPHY
tristate "ST SPEAR1310-MIPHY driver"
select GENERIC_PHY
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 075db1a..b1700cd 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -35,6 +35,7 @@ phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)+=
phy-s5pv210-usb2.o
obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o
obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)+= phy-qcom-apq8064-sata.o
obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
+obj-$(CONFIG_PHY_ROCKCHIP_DP) += phy-rockchip-dp.o
obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)+= phy-qcom-ipq806x-sata.o
obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY) += phy-spear1310-miphy.o
obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY) += phy-spear1340-miphy.o
diff --git a/drivers/phy/phy-rockchip-dp.c b/drivers/phy/phy-rockchip-dp.c
new file mode 100644
index 000..3cb3bf8
--- /dev/null
+++ b/drivers/phy/phy-rockchip-dp.c
@@ -0,0 +1,151 @@
+/*
+ * Rockchip DP PHY driver
+ *
+ * Copyright (C) 2015 FuZhou Rockchip Co., Ltd.
+ * Author: Yakir Yang
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define GRF_SOC_CON12 0x0274
+
+#define GRF_EDP_REF_CLK_SEL_INTER_HIWORD_MASK BIT(20)
+#define GRF_EDP_REF_CLK_SEL_INTER BIT(4)
+
+#define GRF_EDP_PHY_SIDDQ_HIWORD_MASK BIT(21)
+#define GRF_EDP_PHY_SIDDQ_ON0
+#define GRF_EDP_PHY_SIDDQ_OFF BIT(5)
+
+struct rockchip_dp_phy {
+ struct device *dev;
+ struct regmap *grf;
+ struct clk *phy_24m;
+};
+
+static int rockchip_set_phy_state(struct phy *phy, bool enable)
+{
+ struct rockchip_dp_phy *dp = phy_get_drvdata(phy);
+ int ret;
+
+ if (enable) {
+ ret = regmap_write(dp->grf, GRF_SOC_CON12,
+ GRF_EDP_PHY_SIDDQ_HIWORD_MASK |
+ GRF_EDP_PHY_SIDDQ_ON);
+ if (ret < 0) {
+ dev_err(dp->dev, "Can't enable PHY power %d\n", ret);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(dp->phy_24m);
+ } else {
+ clk_disable_unprepare(dp->phy_24m);
+
+ ret = regmap_write(dp->grf, GRF_SOC_CON12,
+ GRF_EDP_PHY_SIDDQ_HIWORD_MASK |
+ GRF_EDP_PHY_SIDDQ_OFF);
+ }
+
+ return ret;
+}
+
+static int rockchip_dp_phy_power_on(struct phy *phy)
+{
+ return rockchip_set_phy_state(phy, true);
+}
+
+static int rockchip_dp_phy_power_off(struct phy *phy)
+{
+ re
[PATCH v10 08/17] dt-bindings: add document for rockchip variant of analogix_dp
Rockchip DP driver is a helper driver of analogix_dp coder driver,
so most of the DT property should be descriped in analogix_dp document.
Signed-off-by: Yakir Yang
Reviewed-by: Heiko Stuebner
---
Changes in v10: None
Changes in v9:
- Document more details for 'ports' property.
Changes in v8:
- Modify the commit subject name. (Heiko)
Changes in v7: None
Changes in v6: None
Changes in v5:
- Split binding doc's from driver changes. (Rob)
- Add eDP hotplug pinctrl property. (Heiko)
Changes in v4: None
Changes in v3: None
Changes in v2: None
.../display/rockchip/analogix_dp-rockchip.txt | 91 ++
1 file changed, 91 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
diff --git
a/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
b/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
new file mode 100644
index 000..dae86c4
--- /dev/null
+++
b/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
@@ -0,0 +1,91 @@
+Rockchip RK3288 specific extensions to the Analogix Display Port
+
+
+Required properties:
+- compatible: "rockchip,rk3288-edp";
+
+- reg: physical base address of the controller and length
+
+- clocks: from common clock binding: handle to dp clock.
+ of memory mapped region.
+
+- clock-names: from common clock binding:
+ Required elements: "dp" "pclk"
+
+- resets: Must contain an entry for each entry in reset-names.
+ See ../reset/reset.txt for details.
+
+- pinctrl-names: Names corresponding to the chip hotplug pinctrl states.
+- pinctrl-0: pin-control mode. should be <&edp_hpd>
+
+- reset-names: Must include the name "dp"
+
+- rockchip,grf: this soc should set GRF regs, so need get grf here.
+
+- ports: there are 2 port nodes with endpoint definitions as defined in
+ Documentation/devicetree/bindings/media/video-interfaces.txt.
+Port 0: contained 2 endpoints, connecting to the ouput of vop.
+Port 1: contained 1 endpoint, connecting to the input of panel.
+
+For the below properties, please refer to Analogix DP binding document:
+ * Documentation/devicetree/bindings/drm/bridge/analogix_dp.txt
+- phys (required)
+- phy-names (required)
+- hpd-gpios (optional)
+---
+
+Example:
+ dp-controller: dp at ff97 {
+ compatible = "rockchip,rk3288-dp";
+ reg = <0xff97 0x4000>;
+ interrupts = ;
+ clocks = <&cru SCLK_EDP>, <&cru PCLK_EDP_CTRL>;
+ clock-names = "dp", "pclk";
+ phys = <&dp_phy>;
+ phy-names = "dp";
+
+ rockchip,grf = <&grf>;
+ resets = <&cru 111>;
+ reset-names = "dp";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&edp_hpd>;
+
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ edp_in: port at 0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ edp_in_vopb: endpoint at 0 {
+ reg = <0>;
+ remote-endpoint = <&vopb_out_edp>;
+ };
+ edp_in_vopl: endpoint at 1 {
+ reg = <1>;
+ remote-endpoint = <&vopl_out_edp>;
+ };
+ };
+
+ edp_out: port at 1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ edp_out_panel: endpoint {
+ reg = <0>;
+ remote-endpoint = <&panel_in_edp>
+ };
+ };
+ };
+ };
+
+ pinctrl {
+ edp {
+ edp_hpd: edp-hpd {
+ rockchip,pins = <7 11 RK_FUNC_2
&pcfg_pull_none>;
+ };
+ };
+ };
--
1.9.1
[PATCH v10 07/17] drm: rockchip: dp: add rockchip platform dp driver
Rockchip have three clocks for dp controller, we leave pclk_edp
to analogix_dp driver control, and keep the sclk_edp_24m and
sclk_edp in platform driver.
Signed-off-by: Yakir Yang
Tested-by: Javier Martinez Canillas
---
Changes in v10:
- Correct the ROCKCHIP_ANALOGIX_DP indentation in Kconfig to tabs here (Heiko)
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5:
- Remove the empty line at the end of document, and correct the endpoint
numbers in the example DT node, and remove the regulator iomux setting
in driver code while using the pinctl in devicetree instead. (Heiko)
- Add device type declared, cause the previous "platform device type
support (v4 11/16)" already merge into (v5 02/14).
- Implement connector registration code. (Thierry)
Changes in v4:
- Remove some deprecated DT properties in rockchip dp document.
Changes in v3:
- Leave "sclk_edp_24m" to rockchip dp phy driver which name to "24m",
and leave "sclk_edp" to analogix dp core driver which name to "dp",
and leave "pclk_edp" to rockchip dp platform driver which name to
"pclk". (Thierry & Heiko)
- Add devicetree binding document. (Heiko)
- Remove "rockchip,panel" DT property, take use of remote point to get panel
node. (Heiko)
- Add the new function point dp_platdata->get_modes() init.
Changes in v2:
- Get panel node with remote-endpoint method, and create devicetree binding
for driver. (Heiko)
- Remove the clock enable/disbale with "sclk_edp" & "sclk_edp_24m",
leave those clock to rockchip dp phy driver.
drivers/gpu/drm/rockchip/Kconfig| 9 +
drivers/gpu/drm/rockchip/Makefile | 1 +
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 442
include/drm/bridge/analogix_dp.h| 1 +
4 files changed, 453 insertions(+)
create mode 100644 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 35215f6..686cb49 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -25,3 +25,12 @@ config ROCKCHIP_DW_HDMI
for the Synopsys DesignWare HDMI driver. If you want to
enable HDMI on RK3288 based SoC, you should selet this
option.
+
+config ROCKCHIP_ANALOGIX_DP
+ tristate "Rockchip specific extensions for Analogix DP driver"
+ depends on DRM_ROCKCHIP
+ select DRM_ANALOGIX_DP
+ help
+ This selects support for Rockchip SoC specific extensions
+ for the Analogix Core DP driver. If you want to enable DP
+ on RK3288 based SoC, you should selet this option.
diff --git a/drivers/gpu/drm/rockchip/Makefile
b/drivers/gpu/drm/rockchip/Makefile
index f3d8a19..8ad01fb 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -6,5 +6,6 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o
rockchip_drm_fbdev.o \
rockchip_drm_gem.o
obj-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
+obj-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o rockchip_drm_vop.o
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
new file mode 100644
index 000..2c82a9a
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -0,0 +1,442 @@
+/*
+ * Rockchip SoC DP (Display Port) interface driver.
+ *
+ * Copyright (C) Fuzhou Rockchip Electronics Co., Ltd.
+ * Author: Andy Yan
+ * Yakir Yang
+ * Jeff Chen
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+
+#include "rockchip_drm_drv.h"
+#include "rockchip_drm_vop.h"
+
+#define to_dp(nm) container_of(nm, struct rockchip_dp_device, nm)
+
+/* dp grf register offset */
+#define GRF_SOC_CON60x025c
+#define GRF_EDP_LCD_SEL_MASKBIT(5)
+#define GRF_EDP_SEL_VOP_LIT BIT(5)
+#define GRF_EDP_SEL_VOP_BIG 0
+
+struct rockchip_dp_device {
+ struct drm_device*drm_dev;
+ struct device*dev;
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+ struct drm_display_mode mode;
+
+ struct clk *pclk;
+ struct regmap*grf;
+ struct reset_control *rst;
+
+ struct analogix_dp_plat_data plat_data;
+};
+
+static int rockchip_dp_pre_init(struct rockchip_dp_device *dp)
+{
+ reset_control_assert(dp->rst);
+ usleep_range(10, 20);
+ reset_control_deas
[PATCH v10 06/17] ARM: dts: exynos/dp: remove some properties that deprecated by analogix_dp driver
After exynos_dp have been split the common IP code into analogix_dp driver,
the analogix_dp driver have deprecated some Samsung platform properties which
could be dynamically parsed from EDID/MODE/DPCD message, so this is an update
for Exynos DTS file for dp-controller.
Beside the backward compatibility is fully preserved, so there are no
bisectability break that make this change in a separate patch.
Signed-off-by: Yakir Yang
Reviewed-by: Krzysztof Kozlowski
Tested-by: Javier Martinez Canillas
---
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6:
- Fix Peach Pit hpd property name error:
- hpd-gpio = <&gpx2 6 0>;
+ hpd-gpios = <&gpx2 6 0>;
Changes in v5:
- Correct the misspell in commit message. (Krzysztof)
Changes in v4:
- Separate all DTS changes to a separate patch. (Krzysztof)
Changes in v3: None
Changes in v2: None
arch/arm/boot/dts/exynos5250-arndale.dts | 2 --
arch/arm/boot/dts/exynos5250-smdk5250.dts | 2 --
arch/arm/boot/dts/exynos5250-snow-common.dtsi | 4 +---
arch/arm/boot/dts/exynos5250-spring.dts | 4 +---
arch/arm/boot/dts/exynos5420-peach-pit.dts| 4 +---
arch/arm/boot/dts/exynos5420-smdk5420.dts | 2 --
arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 +---
7 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts
b/arch/arm/boot/dts/exynos5250-arndale.dts
index c000532..b1790cf 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -124,8 +124,6 @@
&dp {
status = "okay";
samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x0a>;
samsung,lane-count = <4>;
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts
b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index 0f5dcd4..f30c2db 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -80,8 +80,6 @@
&dp {
samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x0a>;
samsung,lane-count = <4>;
diff --git a/arch/arm/boot/dts/exynos5250-snow-common.dtsi
b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
index 5cb33ba..b96624b 100644
--- a/arch/arm/boot/dts/exynos5250-snow-common.dtsi
+++ b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
@@ -236,12 +236,10 @@
pinctrl-names = "default";
pinctrl-0 = <&dp_hpd>;
samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x0a>;
samsung,lane-count = <2>;
- samsung,hpd-gpio = <&gpx0 7 GPIO_ACTIVE_HIGH>;
+ hpd-gpios = <&gpx0 7 GPIO_ACTIVE_HIGH>;
ports {
port at 0 {
diff --git a/arch/arm/boot/dts/exynos5250-spring.dts
b/arch/arm/boot/dts/exynos5250-spring.dts
index c1edd6d..91881d7 100644
--- a/arch/arm/boot/dts/exynos5250-spring.dts
+++ b/arch/arm/boot/dts/exynos5250-spring.dts
@@ -74,12 +74,10 @@
pinctrl-names = "default";
pinctrl-0 = <&dp_hpd_gpio>;
samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x0a>;
samsung,lane-count = <1>;
- samsung,hpd-gpio = <&gpc3 0 GPIO_ACTIVE_HIGH>;
+ hpd-gpios = <&gpc3 0 GPIO_ACTIVE_HIGH>;
};
&ehci {
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts
b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 35cfb07..2f37c87 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -148,12 +148,10 @@
pinctrl-names = "default";
pinctrl-0 = <&dp_hpd_gpio>;
samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x06>;
samsung,lane-count = <2>;
- samsung,hpd-gpio = <&gpx2 6 GPIO_ACTIVE_HIGH>;
+ hpd-gpios = <&gpx2 6 GPIO_ACTIVE_HIGH>;
ports {
port at 0 {
diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts
b/arch/arm/boot/dts/exynos5420-smdk5420.dts
index ac35aef..f67344f 100644
--- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
+++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
@@ -93,8 +93,6 @@
pinctrl-names = "default";
pinctrl-0 = <&dp_hpd>;
samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x0a>;
samsung,lane-count = <4>;
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts
b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index 7b018e4..363c95f 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
++
[PATCH v10 05/17] dt-bindings: add document for analogix display port driver
Analogix dp driver is split from exynos dp driver, so we just
make an copy of exynos_dp.txt, and then simplify exynos_dp.txt
Beside update some exynos dtsi file with the latest change
according to the devicetree binding documents.
Signed-off-by: Yakir Yang
Tested-by: Javier Martinez Canillas
---
Changes in v10: None
Changes in v9: None
Changes in v8:
- Correct the right document path of display-timing.txt (Heiko)
- Correct the misspell of 'from' to 'frm'. (Heiko)
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4:
- Split all DTS changes, and provide backward compatibility. Mark old
properties as deprecated but still support them. (Krzysztof)
- Update "analogix,hpd-gpio" to "hpd-gpios" prop name. (Rob)
- Deprecated some properties which could parsed from Edid/Mode/DPCD. (Thierry)
"analogix,color-space" & "analogix,color-depth" &
"analogix,link-rate" & "analogix,lane-count"&
"analogix,ycbcr-coeff" & "analogix,dynamic-range" &
"vsync-active-high"& "hsync-active-high" & "interlaces"
Changes in v3:
- Add devicetree binding documents. (Heiko)
- Remove sync pol & colorimetry properies from the new analogix dp driver
devicetree binding. (Thierry)
- Update the exist exynos dtsi file with the latest DP DT properies.
Changes in v2: None
.../bindings/display/bridge/analogix_dp.txt| 50 +
.../bindings/display/exynos/exynos_dp.txt | 65 --
2 files changed, 72 insertions(+), 43 deletions(-)
create mode 100644
Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
diff --git a/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
new file mode 100644
index 000..7659a7a
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
@@ -0,0 +1,50 @@
+Analogix Display Port bridge bindings
+
+Required properties for dp-controller:
+ -compatible:
+ platform specific such as:
+* "samsung,exynos5-dp"
+* "rockchip,rk3288-dp"
+ -reg:
+ physical base address of the controller and length
+ of memory mapped region.
+ -interrupts:
+ interrupt combiner values.
+ -clocks:
+ from common clock binding: handle to dp clock.
+ -clock-names:
+ from common clock binding: Shall be "dp".
+ -interrupt-parent:
+ phandle to Interrupt combiner node.
+ -phys:
+ from general PHY binding: the phandle for the PHY device.
+ -phy-names:
+ from general PHY binding: Should be "dp".
+
+Optional properties for dp-controller:
+ -hpd-gpios:
+ Hotplug detect GPIO.
+ Indicates which GPIO should be used for hotplug detection
+ -port@[X]: SoC specific port nodes with endpoint definitions as defined
+ in Documentation/devicetree/bindings/media/video-interfaces.txt,
+ please refer to the SoC specific binding document:
+ * Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
+ *
Documentation/devicetree/bindings/video/analogix_dp-rockchip.txt
+
+
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
+---
+
+Example:
+
+ dp-controller {
+ compatible = "samsung,exynos5-dp";
+ reg = <0x145b 0x1>;
+ interrupts = <10 3>;
+ interrupt-parent = <&combiner>;
+ clocks = <&clock 342>;
+ clock-names = "dp";
+
+ phys = <&dp_phy>;
+ phy-names = "dp";
+ };
diff --git a/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
b/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
index 64693f2..9905081 100644
--- a/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
+++ b/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
@@ -31,45 +31,31 @@ Required properties for dp-controller:
from general PHY binding: the phandle for the PHY device.
-phy-names:
from general PHY binding: Should be "dp".
- -samsung,color-space:
- input video data format.
- COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2
- -samsung,dynamic-range:
- dynamic range for input video data.
- VESA = 0, CEA = 1
- -samsung,ycbcr-coeff:
- YCbCr co-efficients for input video.
- COLOR_YCBCR601 = 0, COLOR_YCBCR709 = 1
- -samsung,color-depth:
- number of bits per colour component.
- COLOR_6 = 0, COLOR_8 = 1, COLOR_10 = 2, COLOR_12 = 3
- -samsung,link-rate:
- link rate supported by the panel.
-
[PATCH v10 04/17] drm: bridge: analogix/dp: dynamic parse sync_pol & interlace & dynamic_range
Both hsync/vsync polarity and interlace mode can be parsed from
drm display mode, and dynamic_range and ycbcr_coeff can be judge
by the video code.
But presumably Exynos still relies on the DT properties, so take
good use of mode_fixup() in to achieve the compatibility hacks.
Signed-off-by: Yakir Yang
Reviewed-by: Krzysztof Kozlowski
Tested-by: Javier Martinez Canillas
---
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7:
- Back to use the of_property_read_bool() interfacs to provoid backward
compatibility of "hsync-active-high" "vsync-active-high" "interlaced"
to avoid -EOVERFLOW error (Krzysztof)
Changes in v6: None
Changes in v5:
- Switch video timing type to "u32", so driver could use "of_property_read_u32"
to get the backword timing values. Krzysztof suggest me that driver could use
the "of_property_read_bool" to get backword timing values, but that interfacs
would modify the original drm_display_mode timing directly (whether those
properties exists or not).
Changes in v4:
- Provide backword compatibility with samsung. (Krzysztof)
Changes in v3:
- Dynamic parse video timing info from struct drm_display_mode and
struct drm_display_info. (Thierry)
Changes in v2: None
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 148 +
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 +-
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 14 +-
3 files changed, 103 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 1f66deb..5f542b7 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -890,8 +890,8 @@ static void analogix_dp_commit(struct analogix_dp_device
*dp)
return;
}
- ret = analogix_dp_set_link_train(dp, dp->video_info->lane_count,
-dp->video_info->link_rate);
+ ret = analogix_dp_set_link_train(dp, dp->video_info.lane_count,
+dp->video_info.link_rate);
if (ret) {
dev_err(dp->dev, "unable to do link train\n");
return;
@@ -1032,6 +1032,85 @@ static void analogix_dp_bridge_disable(struct drm_bridge
*bridge)
dp->dpms_mode = DRM_MODE_DPMS_OFF;
}
+static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge,
+ struct drm_display_mode *orig_mode,
+ struct drm_display_mode *mode)
+{
+ struct analogix_dp_device *dp = bridge->driver_private;
+ struct drm_display_info *display_info = &dp->connector->display_info;
+ struct video_info *video = &dp->video_info;
+ struct device_node *dp_node = dp->dev->of_node;
+ int vic;
+
+ /* Input video interlaces & hsync pol & vsync pol */
+ video->interlaced = !!(mode->flags & DRM_MODE_FLAG_INTERLACE);
+ video->v_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NVSYNC);
+ video->h_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NHSYNC);
+
+ /* Input video dynamic_range & colorimetry */
+ vic = drm_match_cea_mode(mode);
+ if ((vic == 6) || (vic == 7) || (vic == 21) || (vic == 22) ||
+ (vic == 2) || (vic == 3) || (vic == 17) || (vic == 18)) {
+ video->dynamic_range = CEA;
+ video->ycbcr_coeff = COLOR_YCBCR601;
+ } else if (vic) {
+ video->dynamic_range = CEA;
+ video->ycbcr_coeff = COLOR_YCBCR709;
+ } else {
+ video->dynamic_range = VESA;
+ video->ycbcr_coeff = COLOR_YCBCR709;
+ }
+
+ /* Input vide bpc and color_formats */
+ switch (display_info->bpc) {
+ case 12:
+ video->color_depth = COLOR_12;
+ break;
+ case 10:
+ video->color_depth = COLOR_10;
+ break;
+ case 8:
+ video->color_depth = COLOR_8;
+ break;
+ case 6:
+ video->color_depth = COLOR_6;
+ break;
+ default:
+ video->color_depth = COLOR_8;
+ break;
+ }
+ if (display_info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
+ video->color_space = COLOR_YCBCR444;
+ else if (display_info->color_formats & DRM_COLOR_FORMAT_YCRCB422)
+ video->color_space = COLOR_YCBCR422;
+ else if (display_info->color_formats & DRM_COLOR_FORMAT_RGB444)
+ video->color_space = COLOR_RGB;
+ else
+ video->color_space = COLOR_RGB;
+
+ /*
+* NOTE: those property parsing code is used for providing backward
+* compatibility for samsung platform.
+* Due to we used the "of_property_read_u32" interfaces, when this
+* property isn't present, the "video_info" can keep the original
+* v
[PATCH v10 03/17] drm: bridge: analogix/dp: remove duplicate configuration of link rate and link count
link_rate and lane_count already configured in analogix_dp_set_link_train(),
so we don't need to config those repeatly after training finished, just
remove them out.
Beside Display Port 1.2 already support 5.4Gbps link rate, the maximum sets
would change from {1.62Gbps, 2.7Gbps} to {1.62Gbps, 2.7Gbps, 5.4Gbps}.
Signed-off-by: Yakir Yang
Tested-by: Javier Martinez Canillas
---
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4:
- Update commit message more readable. (Jingoo)
- Adjust the order from 05 to 04
Changes in v3:
- The link_rate and lane_count shouldn't config to the DT property value
directly, but we can take those as hardware limite. For example, RK3288
only support 4 physical lanes of 2.7/1.62 Gbps/lane, so DT property would
like "link-rate = 0x0a" "lane-count = 4". (Thierry)
Changes in v2: None
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 8
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 5 +++--
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 039aaab..1f66deb 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -624,6 +624,8 @@ static void analogix_dp_get_max_rx_bandwidth(struct
analogix_dp_device *dp,
/*
* For DP rev.1.1, Maximum link rate of Main Link lanes
* 0x06 = 1.62 Gbps, 0x0a = 2.7 Gbps
+* For DP rev.1.2, Maximum link rate of Main Link lanes
+* 0x06 = 1.62 Gbps, 0x0a = 2.7 Gbps, 0x14 = 5.4Gbps
*/
analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LINK_RATE, &data);
*bandwidth = data;
@@ -657,7 +659,8 @@ static void analogix_dp_init_training(struct
analogix_dp_device *dp,
analogix_dp_get_max_rx_lane_count(dp, &dp->link_train.lane_count);
if ((dp->link_train.link_rate != LINK_RATE_1_62GBPS) &&
- (dp->link_train.link_rate != LINK_RATE_2_70GBPS)) {
+ (dp->link_train.link_rate != LINK_RATE_2_70GBPS) &&
+ (dp->link_train.link_rate != LINK_RATE_5_40GBPS)) {
dev_err(dp->dev, "Rx Max Link Rate is abnormal :%x !\n",
dp->link_train.link_rate);
dp->link_train.link_rate = LINK_RATE_1_62GBPS;
@@ -898,9 +901,6 @@ static void analogix_dp_commit(struct analogix_dp_device
*dp)
analogix_dp_enable_rx_to_enhanced_mode(dp, 1);
analogix_dp_enable_enhanced_mode(dp, 1);
- analogix_dp_set_lane_count(dp, dp->video_info->lane_count);
- analogix_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
-
analogix_dp_init_video(dp);
ret = analogix_dp_config_video(dp);
if (ret)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index 14d20be..9a90a18 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -21,8 +21,9 @@
#define MAX_EQ_LOOP 5
enum link_rate_type {
- LINK_RATE_1_62GBPS = 0x06,
- LINK_RATE_2_70GBPS = 0x0a
+ LINK_RATE_1_62GBPS = DP_LINK_BW_1_62,
+ LINK_RATE_2_70GBPS = DP_LINK_BW_2_7,
+ LINK_RATE_5_40GBPS = DP_LINK_BW_5_4,
};
enum link_lane_count_type {
--
1.9.1
[PATCH v10 02/17] drm: bridge: analogix/dp: fix some obvious code style
Fix some obvious alignment problems, like alignment and line
over 80 characters problems, make this easy to be maintained
later.
Signed-off-by: Yakir Yang
Reviewed-by: Krzysztof Kozlowski
Tested-by: Javier Martinez Canillas
---
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5:
- Resequence this patch after analogix_dp driver have been split
from exynos_dp code, and rephrase reasonable commit message, and
remove some controversial style (Krzysztof)
- analogix_dp_write_byte_to_dpcd(
- dp, DP_TEST_RESPONSE,
+ analogix_dp_write_byte_to_dpcd(dp,
+ DP_TEST_RESPONSE,
DP_TEST_EDID_CHECKSUM_WRITE);
Changes in v4: None
Changes in v3: None
Changes in v2:
- Improved commit message more readable, and avoid using some
uncommon style like bellow: (Joe Preches)
- retval = exynos_dp_read_bytes_from_i2c(...
...);
+ retval =
+ exynos_dp_read_bytes_from_i2c(..);
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 129 ++---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 72 ++--
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 124 ++--
3 files changed, 163 insertions(+), 162 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index cd86413..039aaab 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -61,7 +61,7 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device
*dp)
while (analogix_dp_get_plug_in_status(dp) != 0) {
timeout_loop++;
- if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
+ if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
dev_err(dp->dev, "failed to get hpd plug status\n");
return -ETIMEDOUT;
}
@@ -98,8 +98,8 @@ static int analogix_dp_read_edid(struct analogix_dp_device
*dp)
/* Read Extension Flag, Number of 128-byte EDID extension blocks */
retval = analogix_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
- EDID_EXTENSION_FLAG,
- &extend_block);
+ EDID_EXTENSION_FLAG,
+ &extend_block);
if (retval)
return retval;
@@ -107,7 +107,8 @@ static int analogix_dp_read_edid(struct analogix_dp_device
*dp)
dev_dbg(dp->dev, "EDID data includes a single extension!\n");
/* Read EDID data */
- retval = analogix_dp_read_bytes_from_i2c(dp,
I2C_EDID_DEVICE_ADDR,
+ retval = analogix_dp_read_bytes_from_i2c(dp,
+ I2C_EDID_DEVICE_ADDR,
EDID_HEADER_PATTERN,
EDID_BLOCK_LENGTH,
&edid[EDID_HEADER_PATTERN]);
@@ -138,7 +139,7 @@ static int analogix_dp_read_edid(struct analogix_dp_device
*dp)
}
analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
- &test_vector);
+ &test_vector);
if (test_vector & DP_TEST_LINK_EDID_READ) {
analogix_dp_write_byte_to_dpcd(dp,
DP_TEST_EDID_CHECKSUM,
@@ -152,10 +153,8 @@ static int analogix_dp_read_edid(struct analogix_dp_device
*dp)
/* Read EDID data */
retval = analogix_dp_read_bytes_from_i2c(dp,
- I2C_EDID_DEVICE_ADDR,
- EDID_HEADER_PATTERN,
- EDID_BLOCK_LENGTH,
- &edid[EDID_HEADER_PATTERN]);
+ I2C_EDID_DEVICE_ADDR, EDID_HEADER_PATTERN,
+ EDID_BLOCK_LENGTH, &edid[EDID_HEADER_PATTERN]);
if (retval != 0) {
dev_err(dp->dev, "EDID Read failed!\n");
return -EIO;
@@ -166,16 +165,13 @@ static int analogix_dp_read_edid(struct
analogix_dp_device *dp)
return -EIO;
}
- analogix_dp_read_byte_from_dpcd(dp,
- DP_TEST_REQUEST,
- &test_vector);
+ analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
+ &test_vector);
if (test_vector & DP_TEST_LINK_EDID_READ) {
analogix_dp_write_byte_to_dpcd(dp,
- DP_TEST_EDID_CHECKSUM,
-
[PATCH v10 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory
Split the dp core driver from exynos directory to bridge directory,
and rename the core driver to analogix_dp_*, rename the platform
code to exynos_dp.
Beside the new analogix_dp driver would export six hooks.
"analogix_dp_bind()" and "analogix_dp_unbind()"
"analogix_dp_suspned()" and "analogix_dp_resume()"
"analogix_dp_detect()" and "analogix_dp_get_modes()"
The bind/unbind symbols is used for analogix platform driver to connect
with analogix_dp core driver. And the detect/get_modes is used for analogix
platform driver to init the connector.
They reason why connector need register in helper driver is rockchip drm
haven't implement the atomic API, but Exynos drm have implement it, so
there would need two different connector helper functions, that's why we
leave the connector register in helper driver.
Signed-off-by: Yakir Yang
Tested-by: Javier Martinez Canillas
---
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6:
- Fix the Kconfig recursive dependency (Javier)
Changes in v5:
- Correct the check condition of gpio_is_valid when driver try to get
the "hpd-gpios" DT propery. (Heiko)
- Move the platform attach callback in the front of core driver bridge
attch function. Cause once platform failed at attach, core driver should
still failed, so no need to init connector before platform attached
(Krzysztof)
- Keep code style no changes with the previous exynos_dp_code.c in this
patch, and update commit message about the new export symbol (Krzysztof)
- Gather the device type patch (v4 11/16) into this one. (Krzysztof)
- leave out the connector registration to analogix platform driver. (Thierry)
Changes in v4:
- Update "analogix,hpd-gpios" to "hpd-gpios" DT propery. (Rob)
- Rename "analogix_dp-exynos.c" file name to "exynos_dp.c" (Jingoo)
- Create a separate folder for analogix code in bridge/ (Archit)
Changes in v3:
- Move exynos's video_timing code to analogix_dp-exynos platform driver,
add get_modes method to struct analogix_dp_plat_data. (Thierry)
- Rename some "samsung*" dts propery to "analogix*". (Heiko)
Changes in v2:
- Remove new copyright (Jingoo)
- Fix compiled failed due to analogix_dp_device misspell
drivers/gpu/drm/bridge/Kconfig |2 +
drivers/gpu/drm/bridge/Makefile|1 +
drivers/gpu/drm/bridge/analogix/Kconfig|3 +
drivers/gpu/drm/bridge/analogix/Makefile |1 +
.../analogix/analogix_dp_core.c} | 799 +
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 277 +
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 1263
.../analogix/analogix_dp_reg.h}| 258 ++--
drivers/gpu/drm/exynos/Kconfig |3 +-
drivers/gpu/drm/exynos/Makefile|2 +-
drivers/gpu/drm/exynos/exynos_dp.c | 384 ++
drivers/gpu/drm/exynos/exynos_dp_core.h| 282 -
drivers/gpu/drm/exynos/exynos_dp_reg.c | 1263
include/drm/bridge/analogix_dp.h | 41 +
14 files changed, 2399 insertions(+), 2180 deletions(-)
create mode 100644 drivers/gpu/drm/bridge/analogix/Kconfig
create mode 100644 drivers/gpu/drm/bridge/analogix/Makefile
rename drivers/gpu/drm/{exynos/exynos_dp_core.c =>
bridge/analogix/analogix_dp_core.c} (50%)
create mode 100644 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
create mode 100644 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
rename drivers/gpu/drm/{exynos/exynos_dp_reg.h =>
bridge/analogix/analogix_dp_reg.h} (64%)
create mode 100644 drivers/gpu/drm/exynos/exynos_dp.c
delete mode 100644 drivers/gpu/drm/exynos/exynos_dp_core.h
delete mode 100644 drivers/gpu/drm/exynos/exynos_dp_reg.c
create mode 100644 include/drm/bridge/analogix_dp.h
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 27e2022..efd94e0 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -40,4 +40,6 @@ config DRM_PARADE_PS8622
---help---
Parade eDP-LVDS bridge chip driver.
+source "drivers/gpu/drm/bridge/analogix/Kconfig"
+
endmenu
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index f13c33d..ff821f4 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
+obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig
b/drivers/gpu/drm/bridge/analogix/Kconfig
new file mode 100644
index 000..80f286f
--- /dev/null
+++ b/drivers/gpu/drm/bridge/analogix/Kconfig
@@ -0,0 +1,3 @@
+config DRM_ANALOGIX_DP
+ tristate
+ depends on DRM
diff --git a/drivers/gpu/drm/bridge/analogix/Makefile
b/
[PATCH v10 0/17] Add Analogix Core Display Port Driver
Hi all,
The Samsung Exynos eDP controller and Rockchip RK3288 eDP controller
share the same IP, so a lot of parts can be re-used. I split the common
code into bridge directory, then rk3288 and exynos only need to keep
some platform code. Cause I can't find the exact IP name of exynos dp
controller, so I decide to name dp core driver with "analogix" which I
find in rk3288 eDP TRM
But there are still three light registers setting differents bewteen
exynos and rk3288.
1. RK3288 have five special pll resigters which not indicata in exynos
dp controller.
2. The address of DP_PHY_PD(dp phy power manager register) are different
between rk3288 and exynos.
3. Rk3288 and exynos have different setting with AUX_HW_RETRY_CTL(dp debug
register).
This series have been well tested on Rockchip platform with eDP panel on
Jerry Chromebook and Display Port Monitor on RK3288 board. Also I have
tested on Samsung Snow and Peach Pit Chromebooks, and thanks to Javier at
Samsung
help to retest the whole series on Samsung Exynos5800 Peach Pi Chromebook,
glad to say that things works rightlly.
Thanks,
- Yakir
Changes in v10:
- Correct the ROCKCHIP_ANALOGIX_DP indentation in Kconfig to tabs here (Heiko)
- Fix the wrong macro value of GRF_EDP_REF_CLK_SEL_INTER_HIWORD_MASK
BIT(4) -> BIT(20)
- Remove the surplus "plat_data" check. (Heiko)
- switch (dp->plat_data && dp->plat_data->dev_type) {
+ switch (dp->plat_data->dev_type) {
- Revert parts of Gustavo Padovan's changes in commit:
drm/exynos: do not start enabling DP at bind() phase
Add dp phy poweron function in bind time.
Changes in v9:
- Document more details for 'ports' property.
- Removed the unused the variable "res" in probe function. (Heiko)
- Removed the unused head file.
Changes in v8:
- Correct the right document path of display-timing.txt (Heiko)
- Correct the misspell of 'from' to 'frm'. (Heiko)
- Modify the commit subject name. (Heiko)
- Fix the mixed spacers on macro definitions. (Heiko)
- Remove the unnecessary empty line after clk_prepare_enable. (Heiko)
- Remove the specific address in the example node name. (Heiko)
Changes in v7:
- Back to use the of_property_read_bool() interfacs to provoid backward
compatibility of "hsync-active-high" "vsync-active-high" "interlaced"
to avoid -EOVERFLOW error (Krzysztof)
- Simply the commit message. (Kishon)
- Symmetrical enable/disbale the phy clock and power. (Kishon)
- Simplify the commit message. (Kishon)
Changes in v6:
- Fix the Kconfig recursive dependency (Javier)
- Fix Peach Pit hpd property name error:
- hpd-gpio = <&gpx2 6 0>;
+ hpd-gpios = <&gpx2 6 0>;
Changes in v5:
- Correct the check condition of gpio_is_valid when driver try to get
the "hpd-gpios" DT propery. (Heiko)
- Move the platform attach callback in the front of core driver bridge
attch function. Cause once platform failed at attach, core driver should
still failed, so no need to init connector before platform attached
(Krzysztof)
- Keep code style no changes with the previous exynos_dp_code.c in this
patch, and update commit message about the new export symbol (Krzysztof)
- Gather the device type patch (v4 11/16) into this one. (Krzysztof)
- leave out the connector registration to analogix platform driver. (Thierry)
- Resequence this patch after analogix_dp driver have been split
from exynos_dp code, and rephrase reasonable commit message, and
remove some controversial style (Krzysztof)
- analogix_dp_write_byte_to_dpcd(
- dp, DP_TEST_RESPONSE,
+ analogix_dp_write_byte_to_dpcd(dp,
+ DP_TEST_RESPONSE,
DP_TEST_EDID_CHECKSUM_WRITE);
- Switch video timing type to "u32", so driver could use "of_property_read_u32"
to get the backword timing values. Krzysztof suggest me that driver could use
the "of_property_read_bool" to get backword timing values, but that interfacs
would modify the original drm_display_mode timing directly (whether those
properties exists or not).
- Correct the misspell in commit message. (Krzysztof)
- Remove the empty line at the end of document, and correct the endpoint
numbers in the example DT node, and remove the regulator iomux setting
in driver code while using the pinctl in devicetree instead. (Heiko)
- Add device type declared, cause the previous "platform device type
support (v4 11/16)" already merge into (v5 02/14).
- Implement connector registration code. (Thierry)
- Split binding doc's from driver changes. (Rob)
- Add eDP hotplug pinctrl property. (Heiko)
- Remove "reg" DT property, cause driver could poweron/poweroff phy via
the exist "grf" syscon already. And rename the example DT node from
"edp_phy: phy at ff770274" to "edp_phy: edp-phy" directly. (Heiko)
- Add deivce_node at the front of driver, update phy_ops type from "static
struct" to "static const struct". And correct the input paramters of
devm_phy_create() i
[PATCH 21/28] drm: Kerneldoc for drm_mode_config_funcs
On Mon, Dec 07, 2015 at 02:14:48PM +0100, Thierry Reding wrote: > On Fri, Dec 04, 2015 at 09:46:02AM +0100, Daniel Vetter wrote: > > +* This list is exhaustive. Specifically this hook is not allowed to > > +* return -EINVAL (any invalid requests should be caught in > > +* @atomic_check) or -EDEADLK (this function must not acquire > > +* additional modeset locks). The core will also reject any async > > +* atomic flips with -EINVAL already (for matching semantics in this > > +* case with legacy page flips). > > Can you elaborate on this last assertion? Why does the core reject async > atomic flips? I don't see where it does that. Was an ongoing discussion with Daniels Stone about what exactly we should do here. Separate patch, not yet resolved so I dropped that sentence for now - currently core code does indeed not do that. All other suggestions applied, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH v3 2/5] drm/dsi: Try to match non-DT dsi devices
On 12/07/2015 02:15 PM, Jani Nikula wrote:
> On Mon, 07 Dec 2015, Archit Taneja wrote:
>> Hi,
>>
>> On 11/30/2015 06:15 PM, kbuild test robot wrote:
>>> Hi Archit,
>>>
>>> [auto build test ERROR on: v4.4-rc3]
>>> [also build test ERROR on: next-20151127]
>>>
>>> url:
>>> https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
>>> config: x86_64-allyesdebian (attached as .config)
>>> reproduce:
>>> # save the attached .config to linux build tree
>>> make ARCH=x86_64
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>> drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of
> function 'of_modalias_node' [-Werror=implicit-function-declaration]
>>>if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
>>
>> Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
>> depend on CONFIG_OF?
>
> Please don't.
Just curious, how did x86 use DSI if the only way to create DSI devices
until now was via DT?
Archit
>
> BR,
> Jani.
>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, hosted by The Linux Foundation
[PATCH 22/28] drm/atomic-helper: Reject attempts at re-stealing encoders
On Fri, Dec 04, 2015 at 09:46:03AM +0100, Daniel Vetter wrote:
> This can happen when we run out of encoders for a multi-crtc modeset,
> or also when userspace is silly and tries to clone multiple connectors
> that need the same encoder on the same crtc.
>
> Reported-and-Tested-and-Reviewed-by: Maarten Lankhorst linux.intel.com>
> Cc: Maarten Lankhorst
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 28
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 2cf8ab7dbc8c..ab275499d2a3 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -86,6 +86,27 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state
> *state,
> }
> }
>
> +static bool
> +check_pending_encoder_assignment(struct drm_atomic_state *state,
> + struct drm_encoder *new_encoder,
> + struct drm_connector *new_connector)
new_connector seems to be unused.
> +{
> + struct drm_connector *connector;
> + struct drm_connector_state *conn_state;
> + int i;
> +
> + for_each_connector_in_state(state, connector, conn_state, i) {
> + if (conn_state->best_encoder != new_encoder)
> + continue;
> +
> + /* encoder already assigned and we're trying to re-steal it! */
> + if (connector->state->best_encoder != conn_state->best_encoder)
Was this supposed to be new_connector->state->best_encoder?
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/aec91ff4/attachment.sig>
DRM i2c module or bridge ?
On 11/12/2015 07:20 PM, Emil Velikov wrote: > On 12 November 2015 at 13:18, Thierry Reding > wrote: >> On Thu, Nov 12, 2015 at 12:48:51PM +, Emil Velikov wrote: >>> Hello Thierry, all, >>> >>> Inspired by a recent discussion I was started wondering - where is the >>> cut between DRM i2c modules (most of which encoders/transmitters) and >>> bridge drivers (again some of which i2c encoders) ? Does anyone has >>> some pointers on the topic ? >> >> DRM bridge is a superset of I2C encoders, so everything that I2C encoder >> drivers do they should be able to do with DRM bridges, and more. There >> isn't a strict guideline here, but I think there's general agreement >> that new drivers should be using the DRM bridge framework. The primary >> reason is that bridges integrate seamlessly with the driver model, that >> is, the drivers that implement them are regular drivers that register >> with the corresponding bus and get bound to a device, whereas the I2C >> encoder infrastructure is mostly about manually instantiating devices. >> >> For existing drivers I guess they could all be converted, but doing so >> may require a bit of work. They also tend to work as-is, so finding >> volunteers to do the conversion is probably going to be difficult given >> the lack of motivation. >> >> Thierry >> >>> Based on the above I did a very quick search for third party IP >>> modules in the DRM subsystem: >>> >>> * i915 >>> dvo_ch7017.c >>> dvo_ch7xxx.c >>> dvo_ivch.c >>> dvo_ns2501.c >>> dvo_sil164.c >>> dvo_tfp410.c >> >> It looks like these use some framework that's custom to the i915 driver >> but could otherwise easily be DRM bridges. >> >>> * gma500 >>> tc35876x-dsi-lvds.c >> >> This seems to be some sort of hybrid bridge and panel driver. >> >>> * sti >>> sti_hdmi_tx3g0c55phy.c >>> sti_hdmi_tx3g4c28phy.c >> >> These seem to implement some sort of PHY interface, but from a quick >> look moving these to the PHY framework seems overkill. They seem no good >> fit for DRM bridge because they are not separate devices, but rather the >> SoC generation specific bits of the STi HDMI driver. >> >>> (and for posterity) >>> * i2c >>> adv7511.c >>> ch7006_drv.c >>> sil164_drv.c >>> tda998x_drv.c >>> >>> >>> * bridge >>> dw_hdmi.c >>> nxp-ptn3460.c >>> parade-ps8622.c >>> >>> >>> By the looks of it, we can move rework (some of?) the above into >>> i2c/bridge modules and in other cases (sil164) just use the existing >>> one ? I'm neither volunteering nor suggesting people must work of >>> these, merely curious. >> >> My take on this is that it's probably best to keep the above in their >> current form. If they need to be shared across multiple hardware setups >> it might make sense to convert them to DRM bridge drivers. >> >> For new drivers it's probably best to make them bridge drivers from the >> start. >> > Thanks for the comprehensive reply Thierry. Pretty sure there are > other people wondering about these - this should straighten things > out. Please refer to the following thread for a similar discussion: http://lists.freedesktop.org/archives/dri-devel/2015-July/087097.html > > Just a small note: considering that most desktop GPUs are moving (have > moved ?) away from third party encoders/transmitters I doubt we'll be > seeing any movements on that front. We still have a requirement for such encoders in the SoC world. A SoC may provide a particular kind of encoder output, but we might need to convert that into another type of encoded output. There are multiple reasons why we might want to do this (SoC limitations, support old encoded formats like LVDS, weird requirements on some boards). There is also a trend of re-use of the same third party encoder IPs across multiple SoCs. Having bridges for such IPs is helpful too. Archit > > Cheers, > Emil > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
[PATCH 21/28] drm: Kerneldoc for drm_mode_config_funcs
*support a queue of outstanding updates, but currently no driver > + *supports that. Note that drivers must wait for preceding updates > + *to complete if a synchronous update is requested, they are not > + *allowed to fail the commit in that case. > + * > + * - -ENOMEM, if the driver failed to allocate memory. Specifically > + *this can happen when trying to pin framebuffers, which must only > + *be done when committing the state. > + * > + * - -ENOSPC, as a refinement of the more generic -ENOMEM to indicate > + *that the driver has run out of vram, iommu space or similar gpu > + *address space needed for framebuffer. > + * > + * - -EIO, if the hardware completely died. > + * > + * - -EINTR, -EAGAIN or -ERESTARTSYS, if the ioctl should be restarted. s/ioctl/IOCTL/ > + *This can either be due to a pending signal, or because the driver > + *needs to completely bail out to recover from an exceptional > + *situation like a gpu hang. From a userspace point of view all > errors are s/gpu/GPU/ > + *treated equally. > + * > + * This list is exhaustive. Specifically this hook is not allowed to > + * return -EINVAL (any invalid requests should be caught in > + * @atomic_check) or -EDEADLK (this function must not acquire > + * additional modeset locks). The core will also reject any async > + * atomic flips with -EINVAL already (for matching semantics in this > + * case with legacy page flips). Can you elaborate on this last assertion? Why does the core reject async atomic flips? I don't see where it does that. > + */ > int (*atomic_commit)(struct drm_device *dev, >struct drm_atomic_state *a, Why is the state variable called "a" here? Why not "state"? Same for ->atomic_check() above. > + /** > + * @atomic_state_alloc: > + * > + * This optional hook can be used by drivers who want to subclass struct "... drivers that want ..." > + * &drm_atomic_state to be able to track their own driver-private global > + * state easily. If this hook is implemented, drivers must also > + * implement @atomic_state_clear and @atomic_state_free. > + * > + * RETURNS: > + * > + * A new &drm_atomic_state on success or NULL on failure. > + */ > struct drm_atomic_state *(*atomic_state_alloc)(struct drm_device *dev); > + > + /** > + * @atomic_state_clear: > + * > + * This hook must clear any driver private state duplicated into the > + * passed-in &drm_atomic_state. This hook is called when the caller > + * encountered a &drm_modeset_lock deadlock and needs to drop all > + * already acquired locks as part of the deadlock avoidance dance > + * implemented in drm_modeset_lock_backoff(). > + * > + * Any duplicated state must be invalidated since a concurrent atomic > + * update might change it, and the drm atomic interfaces always apply > + * updates as relative changes to the current state. > + * > + * Drivers who implement this must call drm_atomic_state_default_clear() "Drivers that implement ..." > + * to clear common state. > + */ > void (*atomic_state_clear)(struct drm_atomic_state *state); > + > + /** > + * @atomic_state_free: > + * > + * This hook needs driver private resources and the &drm_atomic_state Did you mean "This hook frees ..."? > + * itself. Note that the core first calls drm_atomic_state_clear to Parentheses after drm_atomic_state_clear? Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/4f151480/attachment-0001.sig>
[PATCH 08/15] drm: omapdrm: gem: Mask out private flags passed from userspace
On 4 December 2015 at 22:27, Laurent Pinchart
wrote:
> The 8 high order bits of the buffer flags are reserved for internal use.
> Mask them out from the flags passed by userspace.
>
Ouch... I know that the Intel guys are pretty rigorous about input
validation, although I wonder how many drivers are (partially or not)
missing these.
> Signed-off-by: Laurent Pinchart
> ---
> drivers/gpu/drm/omapdrm/omap_drv.c | 9 ++---
> include/uapi/drm/omap_drm.h| 1 +
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c
> index 83d63d71062c..e405ab23d7a6 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -551,10 +551,13 @@ static int ioctl_gem_new(struct drm_device *dev, void
> *data,
> struct drm_file *file_priv)
> {
> struct drm_omap_gem_new *args = data;
> + u32 flags = args->flags & OMAP_BO_USER_MASK;
> +
> VERB("%p:%p: size=0x%08x, flags=%08x", dev, file_priv,
> - args->size.bytes, args->flags);
> - return omap_gem_new_handle(dev, file_priv, args->size,
> - args->flags, &args->handle);
> +args->size.bytes, flags);
> +
> + return omap_gem_new_handle(dev, file_priv, args->size, flags,
> + &args->handle);
> }
>
> static int ioctl_gem_cpu_prep(struct drm_device *dev, void *data,
> diff --git a/include/uapi/drm/omap_drm.h b/include/uapi/drm/omap_drm.h
> index 1d0b1172664e..6852c26e8f78 100644
> --- a/include/uapi/drm/omap_drm.h
> +++ b/include/uapi/drm/omap_drm.h
> @@ -36,6 +36,7 @@ struct drm_omap_param {
> #define OMAP_BO_SCANOUT0x0001 /* scanout capable
> (phys contiguous) */
> #define OMAP_BO_CACHE_MASK 0x0006 /* cache type mask, see cache
> modes */
> #define OMAP_BO_TILED_MASK 0x0f00 /* tiled mapping mask, see
> tiled modes */
> +#define OMAP_BO_USER_MASK 0x00ff /* flags settable by
> userspace */
>
Fwiw I'm not too sure that adding the mask here (UAPI) is a good
idea. If you like to keep it, I'd suggest that it encapsulates only
what's currently available. Might even want to move the individual
masks in the respective sections/hunks. Speaking of which -
OMAP_BO_TILED_MASK should be 0x0300, shouldn't it ?
Regards,
Emil
[PATCH v5 4/4] MAINTAINERS: Add Liviu Dudau as maintainer for ARM HDLCD driver.
On Mon, Dec 07, 2015 at 01:25:10PM +, Emil Velikov wrote: > On 7 December 2015 at 12:11, Liviu Dudau wrote: > > Update MAINTAINERS file for HDLCD driver. > > > > Cc: Andrew Morton > > Cc: Arnd Bergmann > > Cc: Mauro Carvalho Chehab > > Cc: Greg KH > > Cc: Joe Perches > > Cc: Jiri Slaby > > > > Signed-off-by: Liviu Dudau > > --- > > MAINTAINERS | 6 ++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index cba790b..8a637e3 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -820,6 +820,12 @@ S: Maintained > > F: drivers/net/arcnet/ > > F: include/uapi/linux/if_arcnet.h > > > > +ARM HDLCD DRM DRIVER > > +M: Liviu Dudau > > +S: Maintained > You surely meant Supported, didn't you ? Well it is a very fine line in corporate world, when a new product comes the next year, to justify spending time (or money) on last year's (or 5) product. I am payed to do work now in the display drivers, so yes, is Supported. As for the HDLCD, it could soon turn into a "legacy" device as the new products come online having the Mali DP engine. I'm not sure it makes a difference, but I can go with Supported for now. Thanks for spotting it! Best regards, Liviu > > Regards, > Emil > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ã)_/¯
[PATCH 07/28] drm: Update drm_plane_funcs kerneldoc
On Mon, Dec 07, 2015 at 01:43:49PM +0100, Thierry Reding wrote: > On Mon, Dec 07, 2015 at 01:34:16PM +0100, Daniel Vetter wrote: > > On Mon, Dec 07, 2015 at 12:46:38PM +0100, Thierry Reding wrote: > > > On Fri, Dec 04, 2015 at 09:45:48AM +0100, Daniel Vetter wrote: > > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > > > [...] > > > > + /** > > > > +* @destroy: > > > > +* > > > > +* Clean up CRTC resources. This is only called at driver > > > > unload time > > > > > > Perhaps drop "only" because there are more than a single potential > > > usage? > > > > It's for driver unload only since you can't hotplug planes. The only > > hotpluggable thing in drm atm are connectors, and I've added these blurbs > > to highlight that. > > The complete hunk is this: > > +* Clean up CRTC resources. This is only called at driver unload time > +* through drm_mode_config_cleanup(), but can also be called at > runtime > +* when a CRTC is being hot-unplugged. > > If you say CRTCs aren't hotpluggable then the above is very confusing. > So either it is only called at driver unload time (which is what you're > saying, since CRTCs aren't hotpluggable) or it can be called in other > circumstances, in which case "only" is wrong. > > So I think either drop the last subsentence to reflect the current > capabilities of the subsystem, or make it something like the following > to clarify: > > The DRM subsystem doesn't currently support hotplugging CRTCs, > but eventually that feature might be added, at which point this > callback will be called when a CRTC in hot-unplugged. > > But since nobody knows when this will happen it's somewhat premature, in > my opinion, to have such documentation. kerneldoc should document facts, > not the roadmap. Oh dear I've been blind. Copy-pasta fail, the hotplugging stuff should only be for connectors. Everything else (crtc, plane & encoder) is not hopluggable. I also inserted a missing "a" while at it in these paragraphs. I made a total mess between plane, CRTC & connector here ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 2/2] drm/dp/mst: save vcpi with payloads
This makes it possibly for drivers to find the associated
mst_port by looking at the payload allocation table.
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 64a0a3729643..3b6627dde9ff 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1673,6 +1673,7 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
if (mgr->proposed_vcpis[i]) {
port = container_of(mgr->proposed_vcpis[i], struct
drm_dp_mst_port, vcpi);
req_payload.num_slots =
mgr->proposed_vcpis[i]->num_slots;
+ req_payload.vcpi = mgr->proposed_vcpis[i]->vcpi;
} else {
port = NULL;
req_payload.num_slots = 0;
@@ -1688,6 +1689,7 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
if (req_payload.num_slots) {
drm_dp_create_payload_step1(mgr,
mgr->proposed_vcpis[i]->vcpi, &req_payload);
mgr->payloads[i].num_slots =
req_payload.num_slots;
+ mgr->payloads[i].vcpi = req_payload.vcpi;
} else if (mgr->payloads[i].num_slots) {
mgr->payloads[i].num_slots = 0;
drm_dp_destroy_payload_step1(mgr, port,
port->vcpi.vcpi, &mgr->payloads[i]);
--
2.1.4
[PATCH 1/2] drm/dp/mst: reply with ACK for UP reqs
From: Mykola Lysenko
Currently we reply with NACK to UP requests which might
confuse receivers. We haven't seen any actual issues with
this but should still respond to UP requests correctly.
Signed-off-by: Mykola Lysenko
---
drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 809959d56d78..64a0a3729643 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1823,7 +1823,7 @@ static int drm_dp_encode_up_ack_reply(struct
drm_dp_sideband_msg_tx *msg, u8 req
{
struct drm_dp_sideband_msg_reply_body reply;
- reply.reply_type = 1;
+ reply.reply_type = 0;
reply.req_type = req_type;
drm_dp_encode_sideband_reply(&reply, msg);
return 0;
--
2.1.4
[PATCH 0/2] Two small patches for MST
Two minor patches for MST here. We were replying NACK to UP requests when we intended to ACK them. We were also not filling in the vcpi field for mst_mgr->payloads although it's defined. Saving the vcpi simplifies the new amdgpu MST implementation that we currently work on. Harry Wentland (1): drm/dp/mst: save vcpi with payloads Mykola Lysenko (1): drm/dp/mst: reply with ACK for UP reqs drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- 2.1.4
[PATCH 16/28] drm: Document drm_atomic_*_get_property
On Mon, Dec 07, 2015 at 01:01:35PM +0100, Thierry Reding wrote: > On Fri, Dec 04, 2015 at 09:45:57AM +0100, Daniel Vetter wrote: > > Yes these are internal functions and not exported and we generally > > don't document them. But for symmetry with the _set_property functions > > (which are exported for the atomic helpers) I'd like to document them. > > Upcoming vtable kerneldoc will reference both the set and get_property > > functions. > > > > Signed-off-by: Daniel Vetter > > --- > > Documentation/DocBook/gpu.tmpl | 1 + > > drivers/gpu/drm/drm_atomic.c | 33 ++--- > > 2 files changed, 31 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > > index 23ad100c2bf5..02c7d44f517c 100644 > > --- a/Documentation/DocBook/gpu.tmpl > > +++ b/Documentation/DocBook/gpu.tmpl > > @@ -946,6 +946,7 @@ int max_width, max_height; > > > >Atomic Mode Setting Function Reference > > !Edrivers/gpu/drm/drm_atomic.c > > +!Idrivers/gpu/drm/drm_atomic.c > > > > > >Frame Buffer Creation > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > index ef5f7663a718..7426d40017a0 100644 > > --- a/drivers/gpu/drm/drm_atomic.c > > +++ b/drivers/gpu/drm/drm_atomic.c > > @@ -429,11 +429,20 @@ int drm_atomic_crtc_set_property(struct drm_crtc > > *crtc, > > } > > EXPORT_SYMBOL(drm_atomic_crtc_set_property); > > > > -/* > > +/** > > + * drm_atomic_crtc_get_property - get property value from CRTC state > > + * @crtc: the drm CRTC to set a property on > > + * @state: the state object to get the property value from > > + * @property: the property to set > > + * @val: pointer to where the value should be written to > > Nit: I find this difficult to write and read, and I prefer the wording > "return location for the property value". But either way: Yeah that reads much better. Changed all 3 places in drm_atomic. We might want to have a style guide for DRM docs for these things ... -Daniel > > Reviewed-by: Thierry Reding -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 07/28] drm: Update drm_plane_funcs kerneldoc
On Mon, Dec 07, 2015 at 01:34:16PM +0100, Daniel Vetter wrote: > On Mon, Dec 07, 2015 at 12:46:38PM +0100, Thierry Reding wrote: > > On Fri, Dec 04, 2015 at 09:45:48AM +0100, Daniel Vetter wrote: > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > > [...] > > > + /** > > > + * @destroy: > > > + * > > > + * Clean up CRTC resources. This is only called at driver unload time > > > > Perhaps drop "only" because there are more than a single potential > > usage? > > It's for driver unload only since you can't hotplug planes. The only > hotpluggable thing in drm atm are connectors, and I've added these blurbs > to highlight that. The complete hunk is this: +* Clean up CRTC resources. This is only called at driver unload time +* through drm_mode_config_cleanup(), but can also be called at runtime +* when a CRTC is being hot-unplugged. If you say CRTCs aren't hotpluggable then the above is very confusing. So either it is only called at driver unload time (which is what you're saying, since CRTCs aren't hotpluggable) or it can be called in other circumstances, in which case "only" is wrong. So I think either drop the last subsentence to reflect the current capabilities of the subsystem, or make it something like the following to clarify: The DRM subsystem doesn't currently support hotplugging CRTCs, but eventually that feature might be added, at which point this callback will be called when a CRTC in hot-unplugged. But since nobody knows when this will happen it's somewhat premature, in my opinion, to have such documentation. kerneldoc should document facts, not the roadmap. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/9c3cf3f5/attachment.sig>
DRM i2c module or bridge ?
Hi Archit, On 7 December 2015 at 08:47, Archit Taneja wrote: > On 11/12/2015 07:20 PM, Emil Velikov wrote: >> On 12 November 2015 at 13:18, Thierry Reding >> wrote: >>> >>> On Thu, Nov 12, 2015 at 12:48:51PM +, Emil Velikov wrote: Hello Thierry, all, Inspired by a recent discussion I was started wondering - where is the cut between DRM i2c modules (most of which encoders/transmitters) and bridge drivers (again some of which i2c encoders) ? Does anyone has some pointers on the topic ? >>> >>> >>> DRM bridge is a superset of I2C encoders, so everything that I2C encoder >>> drivers do they should be able to do with DRM bridges, and more. There >>> isn't a strict guideline here, but I think there's general agreement >>> that new drivers should be using the DRM bridge framework. The primary >>> reason is that bridges integrate seamlessly with the driver model, that >>> is, the drivers that implement them are regular drivers that register >>> with the corresponding bus and get bound to a device, whereas the I2C >>> encoder infrastructure is mostly about manually instantiating devices. >>> >>> For existing drivers I guess they could all be converted, but doing so >>> may require a bit of work. They also tend to work as-is, so finding >>> volunteers to do the conversion is probably going to be difficult given >>> the lack of motivation. >>> >>> Thierry >>> Based on the above I did a very quick search for third party IP modules in the DRM subsystem: * i915 dvo_ch7017.c dvo_ch7xxx.c dvo_ivch.c dvo_ns2501.c dvo_sil164.c dvo_tfp410.c >>> >>> >>> It looks like these use some framework that's custom to the i915 driver >>> but could otherwise easily be DRM bridges. >>> * gma500 tc35876x-dsi-lvds.c >>> >>> >>> This seems to be some sort of hybrid bridge and panel driver. >>> * sti sti_hdmi_tx3g0c55phy.c sti_hdmi_tx3g4c28phy.c >>> >>> >>> These seem to implement some sort of PHY interface, but from a quick >>> look moving these to the PHY framework seems overkill. They seem no good >>> fit for DRM bridge because they are not separate devices, but rather the >>> SoC generation specific bits of the STi HDMI driver. >>> (and for posterity) * i2c adv7511.c ch7006_drv.c sil164_drv.c tda998x_drv.c * bridge dw_hdmi.c nxp-ptn3460.c parade-ps8622.c By the looks of it, we can move rework (some of?) the above into i2c/bridge modules and in other cases (sil164) just use the existing one ? I'm neither volunteering nor suggesting people must work of these, merely curious. >>> >>> >>> My take on this is that it's probably best to keep the above in their >>> current form. If they need to be shared across multiple hardware setups >>> it might make sense to convert them to DRM bridge drivers. >>> >>> For new drivers it's probably best to make them bridge drivers from the >>> start. >>> >> Thanks for the comprehensive reply Thierry. Pretty sure there are >> other people wondering about these - this should straighten things >> out. > > > Please refer to the following thread for a similar discussion: > > http://lists.freedesktop.org/archives/dri-devel/2015-July/087097.html > Thanks for the link, I've already seen the discussion. Yet I did not find it as "clear cut" as Thierry's answer. >> >> Just a small note: considering that most desktop GPUs are moving (have >> moved ?) away from third party encoders/transmitters I doubt we'll be >> seeing any movements on that front. > > > We still have a requirement for such encoders in the SoC world. A SoC > may provide a particular kind of encoder output, but we might need to > convert that into another type of encoded output. There are multiple > reasons why we might want to do this (SoC limitations, support old > encoded formats like LVDS, weird requirements on some boards). > > There is also a trend of re-use of the same third party encoder IPs > across multiple SoCs. Having bridges for such IPs is helpful too. > Reusing things drivers multiple SoC is great imho. As Rob mentioned in another thread - albeit (slightly) awkward one could port the i2c driver to a bridge one, as as al the users of the former are converted it can be nuked. We'll get there one day :-) -Emil
[PATCH 20/28] drm: Add kerneldoc for drm_framebuffer_funcs
On Fri, Dec 04, 2015 at 09:46:01AM +0100, Daniel Vetter wrote:
> While typing these I noticed that ->dirty is a bit a can of worms
> and even supports blt/fill semantics ... shocked me a bit.
>
> Oh well it's defined in a way that nothing bad (just a bit of inefficiency)
> will happen for drivers which supports this. So I didn't bother copying
> the detailed spec into the new kerneldoc.
>
> Signed-off-by: Daniel Vetter
> ---
> include/drm/drm_crtc.h | 50
> +-
> 1 file changed, 41 insertions(+), 9 deletions(-)
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 72a7e096dd42..551484fb55e5 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -162,23 +162,55 @@ struct drm_tile_group {
> u8 group_data[8];
> };
>
> +/**
> + * struct drm_framebuffer_funcs - framebuffer hooks
> + */
> struct drm_framebuffer_funcs {
> - /* note: use drm_framebuffer_remove() */
> + /**
> + * @destroy:
> + *
> + * Clean up framebuffer resources, specifically also unreference the
> + * backing storage. The core guarantees to call this function for every
> + * framebuffer succesfully created by fb_create in
"successfully" and "->fb_create()"?
> + /**
> + * @create_handle:
> + *
> + * Create a buffer handle in the driver-specific buffer manager (either
> + * GEM or TTM) valid for the passed-in struct &drm_file. This is used by
> + * the core to implement the GETFB ioctl, which returns (for
s/ioctl/IOCTL/?
> + * sufficiently priviledged user) also a native buffer handle. This can
"users"
> + * be used for seamless transitions between modesetting clients by
> + * copying the current screen contents to a private buffer and blending
> + * between that and the new contents.
> + *
> + * RETURNS:
> + *
> + * 0 on success or a negative error code on failure.
> + */
> int (*create_handle)(struct drm_framebuffer *fb,
>struct drm_file *file_priv,
>unsigned int *handle);
> - /*
> + /**
> + * @dirty:
> + *
>* Optional callback for the dirty fb ioctl.
>*
> - * Userspace can notify the driver via this callback
> - * that a area of the framebuffer has changed and should
> - * be flushed to the display hardware.
> + * Userspace can notify the driver via this callback that a area of the
"an area"
> + * framebuffer has changed and should be flushed to the display
> + * hardware. This can also be used internally, e.g. by the fbdev
> + * emulation, though that's not (yet) the case currently.
Nit: I'd drop "(yet) " because of "currently".
> + *
> + * See documentation in drm_mode.h for the struct drm_mode_fb_dirty_cmd
> + * for more information as all the semantics and arguments have a one to
> + * one mapping on this function.
Perhaps "for more information on struct drm_mode_fb_dirty_cmd as all the
...". Oh and I guess also &drm_mode_fb_dirty_cmd for the cross-reference?
Again, fine if this is all fixed-up in a follow-up patch, since this is
all from moving the documentation:
Reviewed-by: Thierry Reding
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/a73493a4/attachment.sig>
[PATCH 07/28] drm: Update drm_plane_funcs kerneldoc
On Mon, Dec 07, 2015 at 12:46:38PM +0100, Thierry Reding wrote:
> On Fri, Dec 04, 2015 at 09:45:48AM +0100, Daniel Vetter wrote:
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> [...]
> > + /**
> > +* @destroy:
> > +*
> > +* Clean up CRTC resources. This is only called at driver unload time
>
> Perhaps drop "only" because there are more than a single potential
> usage?
It's for driver unload only since you can't hotplug planes. The only
hotpluggable thing in drm atm are connectors, and I've added these blurbs
to highlight that.
>
> > + /**
> > +* @set_property:
> > +*
> > +* This is the legacy entry point to update a property attach to the
>
> "attached to"
>
> > - /* atomic update handling */
> > + /**
> > +* @atomic_duplicate_state:
> > +*
> > +* Duplicate the current atomic state for this CRTC and return it.
> > +* The core and helpers gurantee that any atomic state duplicated with
> > +* this hook and still owned by the caller (i.e. not transferred to the
> > +* driver by calling ->atomic_commit() from struct
> > +* &drm_mode_config_funcs) will be cleaned up by calling the
> > +* @atomic_destroy_state hook in this structure.
> > +*
> > +* Atomic drivers which don't subclass struct &drm_crtc should use
> > +* drm_atomic_helper_crtc_duplicate_state(). Drivers that subclass the
> > +* state structure to extend it with driver-private state should use
> > +* __drm_atomic_helper_crtc_duplicate_state() to make sure shared state
> > is
> > +* duplicated in a consisten fashion across drivers.
>
> "consistent"
>
> > + /**
> > +* @atomic_set_property:
> > +*
> > +* Decode a driver-private property value and store the decoded value
> > +* into the passed-in state structure. Since the atomic core decodes all
> > +* standardized properties (even for extensions beyond the core set of
> > +* properties which might not be implemented by all drivers) this
> > +* requires drivers to subclass the state structure.
> > +*
> > +* Such driver-private properties should really only implemented for
>
> "be implemented"
>
> > +* This function is called in the state assembly phase of atomic
> > +* modesets, which can be aborted for any reason (including on
> > +* userspace's request to just check whether a configuration would be
> > +* possible). Drivers MUST NOT touch any persistent state (hardware or
> > +* software) or data structures except the passed in adjusted_mode
>
> Copy-paste error? Should be "... the passed in @state parameter."
>
> > +*
> > +* Also since userspace controls in which order properties are set this
> > +* function must not do any input validation (since the state update is
> > +* incompletely and hence likely inconsistent). Instead any such input
>
> "incomplete"
>
> > +* validation must be done in the various atomic_check callbacks.
> > +*
> > +* RETURNS:
> > +*
> > +* 0 if the property has been found, -EINVAL if the property isn't
> > +* implemented by the driver (which shouldn't ever happen, the core only
>
> s/shouldn't ever/should never/?
>
> > +* asks for properties attached to this CRTC). No other
>
> Seems like you could put more text on the above line.
>
> > +* validation is allowed by the driver. The core checks that the
> > +* property value is within the range (integer, valid enum value, ...)
> > +* the driver set when registering the property already.
>
> I'd put the "already" after "The core", otherwise it reads weird in my
> opinion.
>
> > +*/
> > int (*atomic_set_property)(struct drm_crtc *crtc,
> >struct drm_crtc_state *state,
> >struct drm_property *property,
> >uint64_t val);
> > + /**
> > +* @atomic_get_property:
> > +*
> > +* Reads out the decoded driver-private property. This is used to
> > +* implement the GETCRTC ioctl.
> > +*
> > +* Do not call this function directly, use
> > +* drm_atomic_crtc_get_property() instead.
> > +*
> > +* This callback is optional if the driver does not support any
> > +* driver-private atomic properties.
> > +*
> > +* RETURNS:
> > +*
> > +* 0 on success, -EINVAL if ther property isn't implemented by the
>
> s/ther/the/
>
> > +* driver (which shouldn't ever happen, the core only asks for
>
> s/shouldn't ever/should never/?
>
> > +* properties attached on this CRTC).
>
> "attached to"
>
> > @@ -539,19 +662,142 @@ struct drm_connector_funcs {
> > enum drm_connector_status (*detect)(struct drm_connector *connector,
> > bool force);
> > int (*fill_modes)(struct drm_connector *connector, uint32_t max_width,
> > uint32_t max_height);
> > +
> > + /**
> > +* @set_property:
> > +*
> > +* This
[PATCH] drm: do not use device name as a format string
On Mon, Dec 07, 2015 at 12:46:52PM +0100, Daniel Vetter wrote:
> On Mon, Dec 07, 2015 at 11:53:01AM +0200, Jani Nikula wrote:
> > On Mon, 07 Dec 2015, Daniel Vetter wrote:
> > > On Sun, Dec 06, 2015 at 11:16:32AM +0100, Nicolas Iooss wrote:
> > >> On 12/06/2015 10:35 AM, Daniel Vetter wrote:
> > >> >> On 11/18/2015 06:58 PM, Nicolas Iooss wrote:
> > >> >>> drm_dev_set_unique() formats its parameter using kvasprintf() but
> > >> >>> many
> > >> >>> of its callers directly pass dev_name(dev) as printf format string,
> > >> >>> without any format parameter. This can cause some issues when the
> > >> >>> device name contains '%' characters.
> > >> >>>
> > >> >>> To avoid any potential issue, always use "%s" when using
> > >> >>> drm_dev_set_unique() with dev_name().
> > >> >
> > >> > Not sure this is worth it really, normally people don't place %
> > >> > characters
> > >> > into their device names, ever. And if they do it'll blow up. There's
> > >> > also
> > >> > no security issue here since userspace can't set this name.
> > >> >
> > >> > If the maintainers of the affected drivers don't want this I won't
> > >> > merge
> > >> > this patch.
> > >>
> > >> Actually I had the same opinion before I began to add __printf
> > >> attributes and "%s" in several places in the kernel to make
> > >> -Wformat-security useful. This led me to discover some funny issues
> > >> like the one fixed by commit 3958b79266b1 ("configfs: fix kernel
> > >> infoleak through user-controlled format string",
> > >> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=3958b79266b14729edd61daf9dfb84de45f4ec6d
> > >> ). The patch I sent is in fact a very small step towards making
> > >> -Wformat-security useful again to detect "real" issues.
> > >>
> > >> Of course, if you do not feel it is worth it and believe that dev_name
> > >> is fully controlled by trusted sources which will never introduce any %
> > >> character, I understand your will of not merging my patch.
> > >
> > > Ah, that's the context I was missing, that really should be in the commit
> > > message. If this is part of an overall effort to enable something useful
> > > it makes more sense to get it in.
> > >
> > > On the patch itself it feels rather funny to do a "%s", str); combo, maybe
> > > we should have a drm_dev_set_unique_static instead? Including kerneldoc
> > > explaining why there's too.
> >
> > No caller of drm_dev_set_unique() actually uses the formatting for
> > anything... so you'd end up with drm_dev_set_unique_static() and an
> > orphaned drm_dev_set_unique()...
>
> Ok, then I guess we can just ditch the printf stuff from set_unique.
> Nicolas, you're up for that?
Looking at all the callsites of drm_dev_set_unique() it seems like all
of the drivers (with the exception of vgem) use dev_name() on the same
device that's already passed into drm_dev_alloc(), so perhaps another
alternative would be to have drm_dev_alloc() set the unique name by
default and keep the function for cases where it needs to be set
explicitly (like for vgem). vgem passes drm_dev_alloc() a NULL device,
so that could serve as condition.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/ca73220d/attachment.sig>
[PATCH 01/20] drm: use __u{32, 64} instead of uint{32, 64}_t in virtgpu_drm.h
On 5 December 2015 at 21:03, Dave Airlie wrote: > On 5 December 2015 at 00:22, Emil Velikov wrote: >> On 30 November 2015 at 14:10, Gabriel Laskar wrote: >>> Signed-off-by: Gabriel Laskar >>> CC: Emil Velikov >>> CC: Mikko Rapeli >>> >>> --- >>> include/uapi/drm/virtgpu_drm.h | 98 >>> +- >>> 1 file changed, 49 insertions(+), 49 deletions(-) >>> >> For the series >> Reviewed-by: Emil Velikov >> >> Dave would you have any comments wrt this and the remainder of Mikko's >> series ? Alternatively what can we do to get those merged (would you >> like a branch/pull request) ? > > Yeah a git pull for these would be good, it's about all I can do to > care about them. > >From your earlier reply I got the impression that you'll pick Mikko's work. Either way, glad to see some progress on the topic. Mikko, Gabriel, Will you guys be so kind to send pull requests or shall I ? Thanks Emil
[PATCH 03/28] drm: Reorganize helper vtables and their docs
On Mon, Dec 07, 2015 at 12:59:33PM +0100, Daniel Vetter wrote: > On Mon, Dec 07, 2015 at 12:00:09PM +0100, Thierry Reding wrote: > > On Fri, Dec 04, 2015 at 09:45:44AM +0100, Daniel Vetter wrote: > > [...] > > > diff --git a/drivers/gpu/drm/drm_crtc_helper.c > > > b/drivers/gpu/drm/drm_crtc_helper.c > > > index 10d0989db273..077e48d3cac2 100644 > > > --- a/drivers/gpu/drm/drm_crtc_helper.c > > > +++ b/drivers/gpu/drm/drm_crtc_helper.c > > > @@ -62,6 +62,11 @@ > > > * converting to the plane helpers). New drivers must not use these > > > functions > > > * but need to implement the atomic interface instead, potentially using > > > the > > > * atomic helpers for that. > > > + * > > > + * The these legacy modeset helpers use the same function table > > > structures as > > > > s/The these/The/ > > > > > diff --git a/include/drm/drm_modeset_helper_vtables.h > > > b/include/drm/drm_modeset_helper_vtables.h > > > new file mode 100644 > > > index ..35c5a1c4e7ba > > > --- /dev/null > > > +++ b/include/drm/drm_modeset_helper_vtables.h > > > @@ -0,0 +1,252 @@ > > > +/* > > > + * Copyright © 2015 Intel Corporation > > > + * Daniel Vetter > > > > Perhaps inherit the copyright statements from the includes that you > > extracted these from? > > Done for the above two - all the stuff below is just moved and would > conflict massively with later patches. So left that as per our irc > discussion. For the record, I'm fine with leave the below as-is and fix it up in a follow-up patch. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/39b60c5e/attachment.sig>
[PATCH] drm: do not use device name as a format string
On Wed, 18 Nov 2015 18:58:18 +0100 Nicolas Iooss wrote: > drm_dev_set_unique() formats its parameter using kvasprintf() but many > of its callers directly pass dev_name(dev) as printf format string, > without any format parameter. This can cause some issues when the > device name contains '%' characters. > > To avoid any potential issue, always use "%s" when using > drm_dev_set_unique() with dev_name(). > > Signed-off-by: Nicolas Iooss I'll let Daniel decide whether it's relevant or not to add a new function/macro to hide this, but I'm fine with the current patch. [for the atmel-hlcdc driver] Acked-by: Boris Brezillon > --- > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c| 2 +- > drivers/gpu/drm/tegra/drm.c | 2 +- > drivers/gpu/drm/vc4/vc4_drv.c| 2 +- > include/drm/drmP.h | 1 + > 5 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > index 244df0a440b7..0d720d3a7ee0 100644 > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > @@ -733,7 +733,7 @@ static int atmel_hlcdc_dc_drm_probe(struct > platform_device *pdev) > if (!ddev) > return -ENOMEM; > > - ret = drm_dev_set_unique(ddev, dev_name(ddev->dev)); > + ret = drm_dev_set_unique(ddev, "%s", dev_name(ddev->dev)); > if (ret) > goto err_unref; > > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > index 1930234ba5f1..947d75f59881 100644 > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > @@ -363,7 +363,7 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) > fsl_dev->np = dev->of_node; > drm->dev_private = fsl_dev; > dev_set_drvdata(dev, fsl_dev); > - drm_dev_set_unique(drm, dev_name(dev)); > + drm_dev_set_unique(drm, "%s", dev_name(dev)); > > ret = drm_dev_register(drm, 0); > if (ret < 0) > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c > index 159ef515cab1..b278f60f4376 100644 > --- a/drivers/gpu/drm/tegra/drm.c > +++ b/drivers/gpu/drm/tegra/drm.c > @@ -991,7 +991,7 @@ static int host1x_drm_probe(struct host1x_device *dev) > if (!drm) > return -ENOMEM; > > - drm_dev_set_unique(drm, dev_name(&dev->dev)); > + drm_dev_set_unique(drm, "%s", dev_name(&dev->dev)); > dev_set_drvdata(&dev->dev, drm); > > err = drm_dev_register(drm, 0); > diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c > index 6e730605edcc..c90a451aaa79 100644 > --- a/drivers/gpu/drm/vc4/vc4_drv.c > +++ b/drivers/gpu/drm/vc4/vc4_drv.c > @@ -168,7 +168,7 @@ static int vc4_drm_bind(struct device *dev) > vc4->dev = drm; > drm->dev_private = vc4; > > - drm_dev_set_unique(drm, dev_name(dev)); > + drm_dev_set_unique(drm, "%s", dev_name(dev)); > > drm_mode_config_init(drm); > if (ret) > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 0b921ae06cd8..995fb96cb740 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -1049,6 +1049,7 @@ void drm_dev_ref(struct drm_device *dev); > void drm_dev_unref(struct drm_device *dev); > int drm_dev_register(struct drm_device *dev, unsigned long flags); > void drm_dev_unregister(struct drm_device *dev); > +__printf(2, 3) > int drm_dev_set_unique(struct drm_device *dev, const char *fmt, ...); > > struct drm_minor *drm_minor_acquire(unsigned int minor_id); -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[PATCH 19/28] drm: document drm_crtc_funcs
t (yet) take care of that. Therefore drivers > + * currently must clean up and release pending events in their > + * ->preclose driver function. > + * > + * This callback is optional. > + * > + * NOTE: > + * > + * Very early versions of the KMS ABI mandated that the driver must > + * block (but not reject) any rendering to the old framebuffer until the > + * flip operation has completed and the old framebuffer is not longer "no longer" > + * visible. This requirement has been lifted, and userspace is instead > + * expected to request delivery of a event and wait with recycling old "an event" Otherwise: Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/c0b3d804/attachment.sig>
[PATCH v5 4/4] MAINTAINERS: Add Liviu Dudau as maintainer for ARM HDLCD driver.
On 7 December 2015 at 12:11, Liviu Dudau wrote: > Update MAINTAINERS file for HDLCD driver. > > Cc: Andrew Morton > Cc: Arnd Bergmann > Cc: Mauro Carvalho Chehab > Cc: Greg KH > Cc: Joe Perches > Cc: Jiri Slaby > > Signed-off-by: Liviu Dudau > --- > MAINTAINERS | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index cba790b..8a637e3 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -820,6 +820,12 @@ S: Maintained > F: drivers/net/arcnet/ > F: include/uapi/linux/if_arcnet.h > > +ARM HDLCD DRM DRIVER > +M: Liviu Dudau > +S: Maintained You surely meant Supported, didn't you ? Regards, Emil
[PATCH v5 4/4] MAINTAINERS: Add Liviu Dudau as maintainer for ARM HDLCD driver.
On 12/07/2015, 01:11 PM, Liviu Dudau wrote: > Update MAINTAINERS file for HDLCD driver. > > Cc: Andrew Morton > Cc: Arnd Bergmann > Cc: Mauro Carvalho Chehab > Cc: Greg KH > Cc: Joe Perches > Cc: Jiri Slaby Please drop all of us who edited MAINTAINERS in the last decade from your CC list in next submissions. Naturally, we do not care about your changes in MAINTAINERS at all. get_maintainers perhaps should not return 'git log' people for the MAINTAINERS file? > Signed-off-by: Liviu Dudau > --- > MAINTAINERS | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index cba790b..8a637e3 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -820,6 +820,12 @@ S: Maintained > F: drivers/net/arcnet/ > F: include/uapi/linux/if_arcnet.h > > +ARM HDLCD DRM DRIVER > +M: Liviu Dudau > +S: Maintained > +F: drivers/gpu/drm/arm/ > +F: Documentation/devicetree/bindings/display/arm,hdlcd.txt > + > ARM MFM AND FLOPPY DRIVERS > M: Ian Molton > S: Maintained > thanks, -- js suse labs
[PATCH 18/28] drm: connector->dpms is not optional
On Fri, Dec 04, 2015 at 09:45:59AM +0100, Daniel Vetter wrote: > We always register the DPMS property, it's really a fundamental > part of a display driver. So don't check whether the vfunc is there, > it's non-optional > > Yes I've audited all the almost 100 drm_connector_funcs we have, no > one botched this ;-) > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_crtc.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/90ce6cc7/attachment.sig>
[PATCH 17/28] drm: Document drm_connector_funcs
On Fri, Dec 04, 2015 at 09:45:58AM +0100, Daniel Vetter wrote: > The special case here is that both ->detect and ->force are actually > functions only called by the probe helpers and hence really shouldn't > be here. But since they've used by pretty much every driver I figured > it's better to just document this for now instead of holding this doc > patch hostage until that's all fixed. For that reason also group force > right next to detect. > > v2: Use FIXME comments to annotate where we should move a hook to > helpers. > > Signed-off-by: Daniel Vetter > --- > include/drm/drm_crtc.h | 84 > -- > 1 file changed, 74 insertions(+), 10 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/9482bfd2/attachment.sig>
[PATCH 16/28] drm: Document drm_atomic_*_get_property
On Fri, Dec 04, 2015 at 09:45:57AM +0100, Daniel Vetter wrote: > Yes these are internal functions and not exported and we generally > don't document them. But for symmetry with the _set_property functions > (which are exported for the atomic helpers) I'd like to document them. > Upcoming vtable kerneldoc will reference both the set and get_property > functions. > > Signed-off-by: Daniel Vetter > --- > Documentation/DocBook/gpu.tmpl | 1 + > drivers/gpu/drm/drm_atomic.c | 33 ++--- > 2 files changed, 31 insertions(+), 3 deletions(-) > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > index 23ad100c2bf5..02c7d44f517c 100644 > --- a/Documentation/DocBook/gpu.tmpl > +++ b/Documentation/DocBook/gpu.tmpl > @@ -946,6 +946,7 @@ int max_width, max_height; > >Atomic Mode Setting Function Reference > !Edrivers/gpu/drm/drm_atomic.c > +!Idrivers/gpu/drm/drm_atomic.c > > >Frame Buffer Creation > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index ef5f7663a718..7426d40017a0 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -429,11 +429,20 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, > } > EXPORT_SYMBOL(drm_atomic_crtc_set_property); > > -/* > +/** > + * drm_atomic_crtc_get_property - get property value from CRTC state > + * @crtc: the drm CRTC to set a property on > + * @state: the state object to get the property value from > + * @property: the property to set > + * @val: pointer to where the value should be written to Nit: I find this difficult to write and read, and I prefer the wording "return location for the property value". But either way: Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/7425a4b7/attachment.sig>
[PATCH] drm: Move encoder->save/restore into nouveau
On Fri, Dec 04, 2015 at 05:14:07PM +0100, Daniel Vetter wrote: > Nouveau is the only user, and atomic drivers should do state > save/restoring differently. So move it into noveau. > > Saves me typing some kerneldoc, too ;-) > > v2: Move misplaced hunk into earlier nouveau patch. > > Cc: Ilia Mirkin > Cc: Ben Skeggs > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/nouveau/dispnv04/dac.c| 7 +++ > drivers/gpu/drm/nouveau/dispnv04/dfp.c| 7 +++ > drivers/gpu/drm/nouveau/dispnv04/disp.c | 32 > --- > drivers/gpu/drm/nouveau/dispnv04/tvnv04.c | 5 +++-- > drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 5 +++-- > drivers/gpu/drm/nouveau/nouveau_encoder.h | 3 +++ > include/drm/drm_modeset_helper_vtables.h | 4 > 7 files changed, 27 insertions(+), 36 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/ed1fe78f/attachment.sig>
[PATCH 03/28] drm: Reorganize helper vtables and their docs
On Mon, Dec 07, 2015 at 12:00:09PM +0100, Thierry Reding wrote: > On Fri, Dec 04, 2015 at 09:45:44AM +0100, Daniel Vetter wrote: > [...] > > diff --git a/drivers/gpu/drm/drm_crtc_helper.c > > b/drivers/gpu/drm/drm_crtc_helper.c > > index 10d0989db273..077e48d3cac2 100644 > > --- a/drivers/gpu/drm/drm_crtc_helper.c > > +++ b/drivers/gpu/drm/drm_crtc_helper.c > > @@ -62,6 +62,11 @@ > > * converting to the plane helpers). New drivers must not use these > > functions > > * but need to implement the atomic interface instead, potentially using > > the > > * atomic helpers for that. > > + * > > + * The these legacy modeset helpers use the same function table structures > > as > > s/The these/The/ > > > diff --git a/include/drm/drm_modeset_helper_vtables.h > > b/include/drm/drm_modeset_helper_vtables.h > > new file mode 100644 > > index ..35c5a1c4e7ba > > --- /dev/null > > +++ b/include/drm/drm_modeset_helper_vtables.h > > @@ -0,0 +1,252 @@ > > +/* > > + * Copyright © 2015 Intel Corporation > > + * Daniel Vetter > > Perhaps inherit the copyright statements from the includes that you > extracted these from? Done for the above two - all the stuff below is just moved and would conflict massively with later patches. So left that as per our irc discussion. -Daniel > > > +/** > > + * DOC: overview > > + * > > + * The DRM mode setting helper functions are common code for drivers to > > use if > > + * they wish. Drivers are not forced to use this code in their > > + * implementations but it would be useful if the code they do use at least > > + * provides a consistent interface and operation to userspace. Therefore > > it is > > + * highly recommended to use the provided helpers as much as possible. > > + * > > + * Because there is only one pointer per modeset object to hold a vfunc > > table > > + * for helper libraries they are by necessity shared among the different > > + * helpers. > > + * > > + * To make this clear all the helper vtables are pulled together in this > > location here. > > Perhaps drop the "here" at the end of that sentence. Also maybe wrap the > last line because it stands out as much longer than the above. > > > + */ > > + > > +enum mode_set_atomic; > > + > > +/** > > + * struct drm_crtc_helper_funcs - helper operations for CRTCs > > + * @dpms: set power state > > + * @prepare: prepare the CRTC, called before @mode_set > > + * @commit: commit changes to CRTC, called after @mode_set > > + * @mode_fixup: try to fixup proposed mode for this CRTC > > + * @mode_set: set this mode > > + * @mode_set_nofb: set mode only (no scanout buffer attached) > > + * @mode_set_base: update the scanout buffer > > + * @mode_set_base_atomic: non-blocking mode set (used for kgdb support) > > + * @load_lut: load color palette > > + * @disable: disable CRTC when no longer in use > > + * @enable: enable CRTC > > + * @atomic_check: check for validity of an atomic state > > + * @atomic_begin: begin atomic update > > + * @atomic_flush: flush atomic update > > + * > > + * The helper operations are called by the mid-layer CRTC helper. > > + * > > + * Note that with atomic helpers @dpms, @prepare and @commit hooks are > > + * deprecated. Used @enable and @disable instead exclusively. > > I /think/ it would be more correct to say: "Use @enable and @disable > exclusively instead." > > > + * > > + * With legacy crtc helpers there's a big semantic difference between > > @disable > > s/crtc/CRTC/, there's a couple more places where the casing is > inconsistent, I'll refrain from pointing them out explicitly since your > editor will be much better at finding them. > > > +/** > > + * struct drm_encoder_helper_funcs - helper operations for encoders > > + * @dpms: set power state > > + * @save: save connector state > > + * @restore: restore connector state > > + * @mode_fixup: try to fixup proposed mode for this connector > > + * @prepare: part of the disable sequence, called before the CRTC modeset > > + * @commit: called after the CRTC modeset > > + * @mode_set: set this mode, optional for atomic helpers > > + * @get_crtc: return CRTC that the encoder is currently attached to > > + * @detect: connection status detection > > + * @disable: disable encoder when not in use (overrides DPMS off) > > + * @enable: enable encoder > > + * @atomic_check: check for validity of an atomic update > > + * > > + * The helper operations are called by the mid-layer CRTC helper. > > + * > > + * Note that with atomic helpers @dpms, @prepare and @commit hooks are > > + * deprecated. Used @enable and @disable instead exclusively. > > Same comment as for the CRTC helper functions. > > Thierry -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 14/28] drm: Remove crtc/connector->save/restore hooks
On Fri, Dec 04, 2015 at 09:45:55AM +0100, Daniel Vetter wrote: > They're not how system suspend/resume should be done with atomic > (there's new helpers for that developed by Thierry Redding), and for s/Redding/Reding/ =) > legacy drivers this really should be a helper hook and not a core one. > > But there's not even helper code to use them, and only 2 drivers > (which now have their own private hooks) set them. Ditch them. > > Saves me typing some kerneldoc, too ;-) > > Signed-off-by: Daniel Vetter > --- > include/drm/drm_crtc.h | 11 --- > 1 file changed, 11 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/bfa5b8d4/attachment-0001.sig>
[PATCH 01/28] drm: Polish fbdev helper struct docs
On Mon, Dec 07, 2015 at 12:50:24PM +0100, Daniel Vetter wrote:
> On Mon, Dec 07, 2015 at 11:45:22AM +0100, Thierry Reding wrote:
> > On Fri, Dec 04, 2015 at 09:45:42AM +0100, Daniel Vetter wrote:
> > > Mostly this is just adding extensive docs for the callbacks, but also
> > > a few other additions.
> > >
> > > v2: Use FIXME comments to annotate helper hooks that should be
> > > replaced.
> > >
> > > Signed-off-by: Daniel Vetter
> > > ---
> > > include/drm/drm_fb_helper.h | 92
> > > ++---
> > > 1 file changed, 79 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> > > index 87b090c4b730..5ce033e36039 100644
> > > --- a/include/drm/drm_fb_helper.h
> > > +++ b/include/drm/drm_fb_helper.h
> > > @@ -74,25 +74,76 @@ struct drm_fb_helper_surface_size {
> > >
> > > /**
> > > * struct drm_fb_helper_funcs - driver callbacks for the fbdev emulation
> > > library
> > > - * @gamma_set: Set the given gamma lut register on the given crtc.
> > > - * @gamma_get: Read the given gamma lut register on the given crtc, used
> > > to
> > > - * save the current lut when force-restoring the fbdev for
> > > e.g.
> > > - * kdbg.
> > > - * @fb_probe: Driver callback to allocate and initialize the fbdev info
> > > - *structure. Furthermore it also needs to allocate the drm
> > > - *framebuffer used to back the fbdev.
> > > - * @initial_config: Setup an initial fbdev display configuration
> > > *
> > > * Driver callbacks used by the fbdev emulation helper library.
> > > */
> > > struct drm_fb_helper_funcs {
> > > + /**
> > > + * @gamma_set:
> > > + *
> > > + * Set the given gamma lut register on the given crtc.
> > > + *
> > > + * This callback is optional.
> > > + *
> > > + * FIXME:
> > > + *
> > > + * This callback is functionally redundant with the core gamma table
> > > + * support and simply exists because the fbdev hasn't yet been
> > > + * refactored to use the core gamma table interfaces.
> > > + */
> > > void (*gamma_set)(struct drm_crtc *crtc, u16 red, u16 green,
> > > u16 blue, int regno);
> >
> > Pardon my ignorance, but is there a way to document parameters with this
> > new syntax?
>
> Unfortunately not. Doing that would be quite a bit more rework of the
> entire kerneldoc toolchain I think.
Yes, that was my suspicion as well.
> > > + /**
> > > + * @gamma_get:
> > > + *
> > > + * Read the given gamma lut register on the given crtc, used to save the
> > > + * current lut when force-restoring the fbdev for e.g. kdbg.
> > > + *
> > > + * This callback is optional.
> > > + *
> > > + * FIXME:
> > > + *
> > > + * This callback is functionally redundant with the core gamma table
> > > + * support and simply exists because the fbdev hasn't yet been
> > > + * refactored to use the core gamma table interfaces.
> > > + */
> > > void (*gamma_get)(struct drm_crtc *crtc, u16 *red, u16 *green,
> > > u16 *blue, int regno);
> >
> > Nit: While at it, perhaps (here and in the @gamma_set documentation):
> > s/lut/LUT/ and s/crtc/CRTC/?
>
> Yeah I thought about our inconsistency wrt upper-case of abbrevations in
> the docs. I think we should do this as a trivial patch thing for newbies.
Fair enough.
Thierry
> > > * @fb: Scanout framebuffer object
> > > * @dev: DRM device
> >
> > There seems to be an extra space between the : and the description. That
> > was already there, but maybe worth a follow-up.
>
> I think fix that up while applying, same for the others.
Okay, either way, this is a good improvement, so:
Reviewed-by: Thierry Reding
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/e9e3f4a6/attachment.sig>
[PATCH 12/28] drm/gma500: Move to private save/restore hooks
On Fri, Dec 04, 2015 at 09:45:53AM +0100, Daniel Vetter wrote: > I want to remove the core ones since with atomic drivers system > suspend/resume is solved much differently. And there's only 2 drivers > (nouveau besides gma500) really using them. > > Cc: Patrik Jakobsson > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/gma500/cdv_device.c| 2 ++ > drivers/gpu/drm/gma500/cdv_intel_display.c | 2 -- > drivers/gpu/drm/gma500/cdv_intel_hdmi.c| 5 +++-- > drivers/gpu/drm/gma500/cdv_intel_lvds.c| 4 ++-- > drivers/gpu/drm/gma500/mdfld_device.c | 2 ++ > drivers/gpu/drm/gma500/mdfld_dsi_output.c | 5 +++-- > drivers/gpu/drm/gma500/oaktrail_device.c | 2 ++ > drivers/gpu/drm/gma500/psb_device.c| 22 -- > drivers/gpu/drm/gma500/psb_drv.h | 2 ++ > drivers/gpu/drm/gma500/psb_intel_display.c | 2 -- > drivers/gpu/drm/gma500/psb_intel_drv.h | 3 +++ > drivers/gpu/drm/gma500/psb_intel_lvds.c| 5 +++-- > drivers/gpu/drm/gma500/psb_intel_sdvo.c| 5 +++-- > drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 -- > 14 files changed, 37 insertions(+), 26 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/fb911cb4/attachment.sig>
[PATCH] drm/nouveau: Use private save/restore hooks for CRTCs
On Fri, Dec 04, 2015 at 05:13:38PM +0100, Daniel Vetter wrote: > I want to remove the core ones since with atomic drivers system > suspend/resume is solved much differently. And there's only 2 drivers > (gma500 besides nouveau) really using them. > > v2: Fixup bugs Ilia spotted. > > Cc: Ben Skeggs > Cc: Ilia Mirkin > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/nouveau/dispnv04/crtc.c | 5 +++-- > drivers/gpu/drm/nouveau/dispnv04/disp.c | 11 ++- > drivers/gpu/drm/nouveau/nouveau_crtc.h | 3 +++ > 3 files changed, 12 insertions(+), 7 deletions(-) Looks good to me: Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/e8286750/attachment.sig>
[PATCH 01/28] drm: Polish fbdev helper struct docs
On Mon, Dec 07, 2015 at 11:45:22AM +0100, Thierry Reding wrote:
> On Fri, Dec 04, 2015 at 09:45:42AM +0100, Daniel Vetter wrote:
> > Mostly this is just adding extensive docs for the callbacks, but also
> > a few other additions.
> >
> > v2: Use FIXME comments to annotate helper hooks that should be
> > replaced.
> >
> > Signed-off-by: Daniel Vetter
> > ---
> > include/drm/drm_fb_helper.h | 92
> > ++---
> > 1 file changed, 79 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> > index 87b090c4b730..5ce033e36039 100644
> > --- a/include/drm/drm_fb_helper.h
> > +++ b/include/drm/drm_fb_helper.h
> > @@ -74,25 +74,76 @@ struct drm_fb_helper_surface_size {
> >
> > /**
> > * struct drm_fb_helper_funcs - driver callbacks for the fbdev emulation
> > library
> > - * @gamma_set: Set the given gamma lut register on the given crtc.
> > - * @gamma_get: Read the given gamma lut register on the given crtc, used to
> > - * save the current lut when force-restoring the fbdev for e.g.
> > - * kdbg.
> > - * @fb_probe: Driver callback to allocate and initialize the fbdev info
> > - *structure. Furthermore it also needs to allocate the drm
> > - *framebuffer used to back the fbdev.
> > - * @initial_config: Setup an initial fbdev display configuration
> > *
> > * Driver callbacks used by the fbdev emulation helper library.
> > */
> > struct drm_fb_helper_funcs {
> > + /**
> > +* @gamma_set:
> > +*
> > +* Set the given gamma lut register on the given crtc.
> > +*
> > +* This callback is optional.
> > +*
> > +* FIXME:
> > +*
> > +* This callback is functionally redundant with the core gamma table
> > +* support and simply exists because the fbdev hasn't yet been
> > +* refactored to use the core gamma table interfaces.
> > +*/
> > void (*gamma_set)(struct drm_crtc *crtc, u16 red, u16 green,
> > u16 blue, int regno);
>
> Pardon my ignorance, but is there a way to document parameters with this
> new syntax?
Unfortunately not. Doing that would be quite a bit more rework of the
entire kerneldoc toolchain I think.
>
> > + /**
> > +* @gamma_get:
> > +*
> > +* Read the given gamma lut register on the given crtc, used to save the
> > +* current lut when force-restoring the fbdev for e.g. kdbg.
> > +*
> > +* This callback is optional.
> > +*
> > +* FIXME:
> > +*
> > +* This callback is functionally redundant with the core gamma table
> > +* support and simply exists because the fbdev hasn't yet been
> > +* refactored to use the core gamma table interfaces.
> > +*/
> > void (*gamma_get)(struct drm_crtc *crtc, u16 *red, u16 *green,
> > u16 *blue, int regno);
>
> Nit: While at it, perhaps (here and in the @gamma_set documentation):
> s/lut/LUT/ and s/crtc/CRTC/?
Yeah I thought about our inconsistency wrt upper-case of abbrevations in
the docs. I think we should do this as a trivial patch thing for newbies.
> > + /**
> > +* @fb_probe:
> > +*
> > +* Driver callback to allocate and initialize the fbdev info structure.
> > +* Furthermore it also needs to allocate the drm framebuffer used to
> > +* back the fbdev.
> > +*
> > +* This callback is mandatory.
> > +*
> > +* RETURNS:
> > +*
> > +* The driver should return 0 on success and a negative error code on
> > +* failure.
> > +*/
> > int (*fb_probe)(struct drm_fb_helper *helper,
> > struct drm_fb_helper_surface_size *sizes);
>
> Nit: s/drm/DRM/
>
> > /**
> > - * struct drm_fb_helper - helper to emulate fbdev on top of kms
> > + * struct drm_fb_helper - main structure to emulate fbdev on top of kms
>
> s/kms/KMS
>
> > * @fb: Scanout framebuffer object
> > * @dev: DRM device
>
> There seems to be an extra space between the : and the description. That
> was already there, but maybe worth a follow-up.
I think fix that up while applying, same for the others.
-Daniel
>
> > * @crtc_count: number of possible CRTCs
> > * @crtc_info: per-CRTC helper state (mode, x/y offset, etc)
> > * @connector_count: number of connected connectors
> > * @connector_info_alloc_count: size of connector_info
> > + * @connector_info: array of per-connector information
> > * @funcs: driver callbacks for fb helper
> > * @fbdev: emulated fbdev device info struct
> > * @pseudo_palette: fake palette of 16 colors
> > - * @kernel_fb_list: list_head in kernel_fb_helper_list
> > - * @delayed_hotplug: was there a hotplug while kms master active?
> > + *
> > + * This is the main structure used by the fbdev helpers. Drivers supporting
> > + * fbdev emulation should embedded this into their overall driver
> > structure.
> > + * Drivers must also fill out a struct &drm_fb_helper_funcs with a few
> > + * opera
[PATCH 11/28] drm/vmwgfx: Drop dummy save/restore hooks
On Fri, Dec 04, 2015 at 09:45:52AM +0100, Daniel Vetter wrote: > These hooks will be gone soon. > > Cc: Thomas Hellstrom > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 16 > drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 4 > drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 4 > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 > 4 files changed, 28 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/ba80bd63/attachment-0001.sig>
[PATCH 10/28] drm/virtio: Drop dummy save/restore functions
On Fri, Dec 04, 2015 at 09:45:51AM +0100, Daniel Vetter wrote: > These hooks will be gone soon. > > Cc: Dave Airlie > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/virtio/virtgpu_display.c | 12 > 1 file changed, 12 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/f413072f/attachment.sig>
[PATCH 09/28] drm/qxl: Drop dummy save/restore hooks
On Fri, Dec 04, 2015 at 09:45:50AM +0100, Daniel Vetter wrote: > These hooks will be gone soon. > > Cc: Dave Airlie > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/qxl/qxl_display.c | 12 > 1 file changed, 12 deletions(-) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/166271e7/attachment.sig>
[PATCH 08/28] drm/noveau: Ditch NULL save/restore hook assignments
On Fri, Dec 04, 2015 at 09:45:49AM +0100, Daniel Vetter wrote: > gcc does this for us, and these hooks will be gone soon. In the subject: s/noveau/nouveau/, otherwise: Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/6b8fe600/attachment.sig>
[PATCH] drm: do not use device name as a format string
On Mon, Dec 07, 2015 at 11:53:01AM +0200, Jani Nikula wrote:
> On Mon, 07 Dec 2015, Daniel Vetter wrote:
> > On Sun, Dec 06, 2015 at 11:16:32AM +0100, Nicolas Iooss wrote:
> >> On 12/06/2015 10:35 AM, Daniel Vetter wrote:
> >> >> On 11/18/2015 06:58 PM, Nicolas Iooss wrote:
> >> >>> drm_dev_set_unique() formats its parameter using kvasprintf() but many
> >> >>> of its callers directly pass dev_name(dev) as printf format string,
> >> >>> without any format parameter. This can cause some issues when the
> >> >>> device name contains '%' characters.
> >> >>>
> >> >>> To avoid any potential issue, always use "%s" when using
> >> >>> drm_dev_set_unique() with dev_name().
> >> >
> >> > Not sure this is worth it really, normally people don't place %
> >> > characters
> >> > into their device names, ever. And if they do it'll blow up. There's also
> >> > no security issue here since userspace can't set this name.
> >> >
> >> > If the maintainers of the affected drivers don't want this I won't merge
> >> > this patch.
> >>
> >> Actually I had the same opinion before I began to add __printf
> >> attributes and "%s" in several places in the kernel to make
> >> -Wformat-security useful. This led me to discover some funny issues
> >> like the one fixed by commit 3958b79266b1 ("configfs: fix kernel
> >> infoleak through user-controlled format string",
> >> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=3958b79266b14729edd61daf9dfb84de45f4ec6d
> >> ). The patch I sent is in fact a very small step towards making
> >> -Wformat-security useful again to detect "real" issues.
> >>
> >> Of course, if you do not feel it is worth it and believe that dev_name
> >> is fully controlled by trusted sources which will never introduce any %
> >> character, I understand your will of not merging my patch.
> >
> > Ah, that's the context I was missing, that really should be in the commit
> > message. If this is part of an overall effort to enable something useful
> > it makes more sense to get it in.
> >
> > On the patch itself it feels rather funny to do a "%s", str); combo, maybe
> > we should have a drm_dev_set_unique_static instead? Including kerneldoc
> > explaining why there's too.
>
> No caller of drm_dev_set_unique() actually uses the formatting for
> anything... so you'd end up with drm_dev_set_unique_static() and an
> orphaned drm_dev_set_unique()...
Ok, then I guess we can just ditch the printf stuff from set_unique.
Nicolas, you're up for that?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
