Re: [PATCH 0/3] drm/exynos: add support for dynamic zpos in DECON and FIMD
18. 12. 11. 오후 4:49에 Andrzej Hajda 이(가) 쓴 글: > On 11.12.2018 00:45, Inki Dae wrote: >> Hi Andrzej, >> >> 18. 12. 10. 오후 4:35에 Andrzej Hajda 이(가) 쓴 글: >>> Hi Inki, >>> >>> On 10.12.2018 03:25, Inki Dae wrote: Hi Andrzej, 18. 12. 6. 오후 6:38에 Andrzej Hajda 이(가) 쓴 글: > Hi Inki, > > This small patchset adds dynamic zpos support for DECON and FIMD. This patch will allow user space to change zpos. However, DECON and FIMD devices have fixed priority of HW overlays. This would mean that zpos change by user space doesn't guarantee correct HW overlay priority. Why do you want to support mutable zpos? >>> >>> Practically you have patches which proves it works, you can see that >>> changing zpos value results in adequate change in displayed image. >>> >>> Conceptually it is just a matter of disconnecting hardware windows >>> present in DECON and FIMD from DRM planes which are software entities. >>> >>> You can reason about it this way: >>> >>> - drm plane is a framebuffer plus informations how it should be >>> transformed/displayed on DECON/FIMD, >>> >>> - hw window in DECON/FIMD is just a channel through which plane is send >>> to the display. >>> >>> DECON and FIMD has fixed hw windows order - windows with lower numbers >>> are displayed below windows with higher number. To display planes in >>> given z-order you just need to send them via windows with appropriate >>> index - farthest plane should go always via win0, closer one via win1, >>> ..., till the last plane. >>> >>> So for example if you have three planes and want to display them in >>> following order (first one farthest, last one closest): >>> >>> plane2, plane1, plane3 >>> >>> you should map them to planes as follow: >>> >>> plane2 -> win0, plane1 -> win1, plane3 -> win2 >>> >>> then for example plane2 is disabled, we will have following mapping: >>> >>> plane1 -> win0, plane3 -> win1, win2 - disabled >> Plane1 is displayed below Plane3. > > > And this is what we wanted, the initial order was: plane2, plane1, > plane3, first farthest (or lowest if you prefer this naming schema). > > >> >>> then if you change zorder of planes to: plane3, plane1: >>> >>> plane3 -> win0, plane1 -> win1 >> Plane3 is displayed below Plane1 because DECON/FIMD HW aren't able to change >> HW overlay priroty. > > > No, plane3 is displayed below plane1 because we sent it via win0, if we > want inverse we can send plane3 via win1 and plane1 via win0. > > >> However, user space wanted that Plane1 is displayed below Plane3. Like this, >> zpos change by user space doesn't guarantee correct HW overlay priority. > > > As I said before in this example userspace wanted plane3 below plane1, > and as I wrote in comment above any order of planes user want driver is > able to realize with this patch. > > >> And there is no any reason to change zpos in user space excepting Mixer >> device which supports HW overlay priority change. > > > Lack of special registry for manipulating windows order does not mean it > cannot support dynamic zpos. > Why changing zpos in user space is required? > >> In fact, we supported mutable zpos before but changed it to immutable with >> same reason. > > > It was just broken if I remember correctly. > No, I worked well and real user used it I remember. > >> >> Lastly, your patch made real user broken. > > > Who? How? Window server of Tizen didn't work and you can test it with below image - I didn't check why it doesn't. Anyway, we don't have to break real user. http://download.tizen.org/snapshots/tizen/unified/latest/images/standard/mobile-wayland-armv7l-tm2/ Thanks, Inki Dae > > > Regards > > Andrzej > > >> >> Thanks, >> Inki Dae >> >>> >>> As you see there is no relation between plane number and window number, >>> but window number is equal to plane's position in zpos-ordered list of >>> planes and this is exact value of normalized_zpos field in drm_plane_state. >>> >>> >>> I hope this extended explanation will clarify things. >>> >>> >>> Regards >>> >>> Andrzej >>> >>> >>> Thanks, Inki Dae > It was tested on tm2 and trats2. > > Regards > Andrzej > > > Andrzej Hajda (3): > drm/exynos/decon5433: add dynamic zpos support > drm/exynos/fimd: create local helper for disabling hardware window > drm/exynos/fimd: add dynamic zpos support > > drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 23 +- > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 42 --- > 2 files changed, 29 insertions(+), 36 deletions(-) > >>> >>> >> > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 22/29] drm/omap: Move DISPC timing checks to CRTC .mode_valid() operation
Hi Sebastian, (CC'ing Daniel) On Tuesday, 11 December 2018 03:07:45 EET Sebastian Reichel wrote: > On Mon, Dec 10, 2018 at 02:14:01PM +0100, Sebastian Reichel wrote: > > On Mon, Dec 10, 2018 at 09:50:08AM +0200, Laurent Pinchart wrote: > >> On Monday, 10 December 2018 00:07:55 EET Sebastian Reichel wrote: > >>> On Wed, Dec 05, 2018 at 05:00:15PM +0200, Laurent Pinchart wrote: > The DISPC timings checks relate to the CRTC, but they're performed > in the encoder and connector .atomic_check() and .mode_valid() > operations. Move them to the CRTC .mode_valid() operation. > > Signed-off-by: Laurent Pinchart > --- > >>> > >>> Reviewed-by: Sebastian Reichel > >>> > >>> This should also fix the issue with DSI in a less ugly way: > >>> > >>> https://lists.freedesktop.org/archives/dri-devel/2018-November/196622. > >>> html > >> > >> Should I add the > >> > >> Fixes: 7c27fa57ef31 ("drm/omap: Call dispc timings check operation > >> directly") > >> > >> tag to this patch ? > > > > I plan to test this series in combination with my DSI patches today > > or tomorrow evening. If it works, probably the following should be > > added to the patch description: > > > > > > As a side-effect this fixes DSI, which uses slightly different mode > > configuration for the DSI encoder and the DISPC. > > > > Fixes: 7c27fa57ef31 ("drm/omap: Call dispc timings check operation > > directly") > > > > I merged this series with my remaining DSI command mode patches > (btw. this series does not apply cleanly to master due to a recent > patch from Tomi). I'll fix that, thank you for the notice. > Unfortunately this patch does not fix the issue. After this patch the > mgr_check_timings() call in omap_crtc_mode_valid() will fail. > > The good news: The Droid 4 display worked again after dropping the > mgr_check_timings call as a quick hack. So the other patches from > this set are > > Tested-by: Sebastian Reichel Thank you for testing this. > The problem is, that DISPC timings for DSI are not directly > calculated from mode. Instead DSI code does some modifications > before applying the DISPC settings. I wonder what the best way to fix this would be. DSI isn't so special in this regard, any encoder can modify modes, resulting in the CRTC outputting a mode significantly different than the mode being displayed. This is handled by the DRM helpers through the .mode_fixup() operations, which can mangle modes along the pipeline. At atomic commit time the atomic helpers call .mode_valid() first, followed by .mode_fixup(). An easy fix would be to remove our .mode_valid() handlers completely and perform both fixup and validation in the .mode_fixup() handlers. However, the .mode_valid() operations are also used in drm_helper_probe_single_connector_modes(), where no mode fixup is performed. We can't drop .mode_valid() for that code path (at least not as anything else than a hack). Daniel, what's the expected way to handle this ? -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] [RFC] MAINTAINERS: Daniel for drm co-maintainer
On Mon, Dec 10, 2018 at 11:30:01AM +0100, Daniel Vetter wrote: > lkml and Linus gained a CoC, and it's serious this time. Which means > my no 1 reason for declining to officially step up as drm maintainer > is gone, and I didn't find any new good excuse. > > I chatted with a few people in private already, and the biggest > concern is that I mislay my community hat and start running around > with my intel hat only. Or some other convenient abuse of trust. > > That's why this patch doesn't just need a lot of acks that mean "yeah > seems fine to me", but a lot of acks that mean "yeah we'll tell you > when you're over the line and usurp you from that comfy chair if you > don't get it". Which I think we've been done a fairly good job here at > dri-devel in general, but better to be clear. > > Rough idea is that I'll do this for maybe 2-3 years, helping Dave > figure out a group model for drm overall. And getting the tooling and > infrastructure for that off the ground. Then step down again because > some other shiny thing that needs chasing. Of course as plans tend to > do, this one will probably pan out a bit different in reality. > > Cc: David Airlie > Cc: Linus Torvalds > Signed-off-by: Daniel Vetter Acked-by: Gerd Hoffmann > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7e05aa20b0ab..2c4cd038df2a 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4849,6 +4849,7 @@ F: include/uapi/drm/vmwgfx_drm.h > > DRM DRIVERS > M: David Airlie > +M: Daniel Vetter > L: dri-devel@lists.freedesktop.org > T: git git://anongit.freedesktop.org/drm/drm > B: https://bugs.freedesktop.org/ > -- > 2.20.0.rc1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm/exynos: add support for dynamic zpos in DECON and FIMD
On 11.12.2018 09:01, Inki Dae wrote: > > 18. 12. 11. 오후 4:49에 Andrzej Hajda 이(가) 쓴 글: >> On 11.12.2018 00:45, Inki Dae wrote: >>> Hi Andrzej, >>> >>> 18. 12. 10. 오후 4:35에 Andrzej Hajda 이(가) 쓴 글: Hi Inki, On 10.12.2018 03:25, Inki Dae wrote: > Hi Andrzej, > > 18. 12. 6. 오후 6:38에 Andrzej Hajda 이(가) 쓴 글: >> Hi Inki, >> >> This small patchset adds dynamic zpos support for DECON and FIMD. > This patch will allow user space to change zpos. However, DECON and FIMD > devices have fixed priority of HW overlays. > This would mean that zpos change by user space doesn't guarantee correct > HW overlay priority. > > Why do you want to support mutable zpos? Practically you have patches which proves it works, you can see that changing zpos value results in adequate change in displayed image. Conceptually it is just a matter of disconnecting hardware windows present in DECON and FIMD from DRM planes which are software entities. You can reason about it this way: - drm plane is a framebuffer plus informations how it should be transformed/displayed on DECON/FIMD, - hw window in DECON/FIMD is just a channel through which plane is send to the display. DECON and FIMD has fixed hw windows order - windows with lower numbers are displayed below windows with higher number. To display planes in given z-order you just need to send them via windows with appropriate index - farthest plane should go always via win0, closer one via win1, ..., till the last plane. So for example if you have three planes and want to display them in following order (first one farthest, last one closest): plane2, plane1, plane3 you should map them to planes as follow: plane2 -> win0, plane1 -> win1, plane3 -> win2 then for example plane2 is disabled, we will have following mapping: plane1 -> win0, plane3 -> win1, win2 - disabled >>> Plane1 is displayed below Plane3. >> >> And this is what we wanted, the initial order was: plane2, plane1, >> plane3, first farthest (or lowest if you prefer this naming schema). >> >> then if you change zorder of planes to: plane3, plane1: plane3 -> win0, plane1 -> win1 >>> Plane3 is displayed below Plane1 because DECON/FIMD HW aren't able to >>> change HW overlay priroty. >> >> No, plane3 is displayed below plane1 because we sent it via win0, if we >> want inverse we can send plane3 via win1 and plane1 via win0. >> >> >>> However, user space wanted that Plane1 is displayed below Plane3. Like >>> this, zpos change by user space doesn't guarantee correct HW overlay >>> priority. >> >> As I said before in this example userspace wanted plane3 below plane1, >> and as I wrote in comment above any order of planes user want driver is >> able to realize with this patch. >> >> >>> And there is no any reason to change zpos in user space excepting Mixer >>> device which supports HW overlay priority change. >> >> Lack of special registry for manipulating windows order does not mean it >> cannot support dynamic zpos. >> > Why changing zpos in user space is required? > >>> In fact, we supported mutable zpos before but changed it to immutable with >>> same reason. >> >> It was just broken if I remember correctly. >> > No, I worked well and real user used it I remember. > >>> Lastly, your patch made real user broken. >> >> Who? How? > Window server of Tizen didn't work and you can test it with below image - I > didn't check why it doesn't. Anyway, we don't have to break real user. > http://download.tizen.org/snapshots/tizen/unified/latest/images/standard/mobile-wayland-armv7l-tm2/ Are you saying it works with latest mainline without my patches? Regards Andrzej > > Thanks, > Inki Dae > >> >> Regards >> >> Andrzej >> >> >>> Thanks, >>> Inki Dae >>> As you see there is no relation between plane number and window number, but window number is equal to plane's position in zpos-ordered list of planes and this is exact value of normalized_zpos field in drm_plane_state. I hope this extended explanation will clarify things. Regards Andrzej > Thanks, > Inki Dae > >> It was tested on tm2 and trats2. >> >> Regards >> Andrzej >> >> >> Andrzej Hajda (3): >> drm/exynos/decon5433: add dynamic zpos support >> drm/exynos/fimd: create local helper for disabling hardware window >> drm/exynos/fimd: add dynamic zpos support >> >> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 23 +- >> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 42 --- >> 2 files changed, 29 insertions(+), 36 deletions(-) >> >> >> > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/
[PATCH v5 03/17] drm/sun4i: sun6i_mipi_dsi: Add Allwinner A64 MIPI DSI support
The MIPI DSI controller on Allwinner A64 is similar to Allwinner A31 without support of DSI mod clock(CLK_DSI_SCLK) So, alter has_mod_clk bool via driver data for respective SoC's compatible. Signed-off-by: Jagan Teki --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 561de393ea23..50f535ae57e9 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -1120,11 +1120,18 @@ static const struct sun6i_dsi_variant sun6i_a31_dsi = { .has_mod_clk = true, }; +static const struct sun6i_dsi_variant sun50i_a64_dsi = { +}; + static const struct of_device_id sun6i_dsi_of_table[] = { { .compatible = "allwinner,sun6i-a31-mipi-dsi", .data = &sun6i_a31_dsi, }, + { + .compatible = "allwinner,sun50i-a64-mipi-dsi", + .data = &sun50i_a64_dsi, + }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table); -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 04/17] dt-bindings: sun6i-dsi: Add compatible for A64 MIPI DSI
The MIPI DSI controller on Allwinner A64 is similar to Allwinner A31 without support of DSI mod clock. Signed-off-by: Jagan Teki Reviewed-by: Rob Herring --- Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt index 6a6cf5de08b0..9fa6e7a758ad 100644 --- a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt +++ b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt @@ -12,6 +12,7 @@ The DSI Encoder generates the DSI signal from the TCON's. Required properties: - compatible: value must be one of: * allwinner,sun6i-a31-mipi-dsi +* allwinner,sun50i-a64-mipi-dsi - reg: base address and size of memory-mapped region - interrupts: interrupt associated to this IP - clocks: phandles to the clocks feeding the DSI encoder -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v1] drm/tegra: vic: Defer firmware loading until it is really needed
DRM driver fails to load if VIC firmware is missing, let's defer the firmware loading until it is really needed. This eliminates the need to have initrd with the firmware if DRM driver is compiled as built-in. Signed-off-by: Dmitry Osipenko --- drivers/gpu/drm/tegra/vic.c | 43 +++-- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c index d47983deb1cf..efb75f290ff0 100644 --- a/drivers/gpu/drm/tegra/vic.c +++ b/drivers/gpu/drm/tegra/vic.c @@ -96,6 +96,34 @@ static int vic_runtime_suspend(struct device *dev) return 0; } +static int vic_load_firmware(struct vic *vic) +{ + struct host1x_client *client = &vic->client.base; + struct drm_device *dev = dev_get_drvdata(client->parent); + struct tegra_drm *tegra = dev->dev_private; + int err; + + if (vic->falcon.data) + return 0; + + vic->falcon.data = tegra; + + err = falcon_read_firmware(&vic->falcon, vic->config->firmware); + if (err < 0) + goto err_cleanup; + + err = falcon_load_firmware(&vic->falcon); + if (err < 0) + goto err_cleanup; + + return 0; + +err_cleanup: + vic->falcon.data = NULL; + + return err; +} + static int vic_boot(struct vic *vic) { u32 fce_ucode_size, fce_bin_data_offset; @@ -105,6 +133,10 @@ static int vic_boot(struct vic *vic) if (vic->booted) return 0; + err = vic_load_firmware(vic); + if (err < 0) + return err; + /* setup clockgating registers */ vic_writel(vic, CG_IDLE_CG_DLY_CNT(4) | CG_IDLE_CG_EN | @@ -181,13 +213,6 @@ static int vic_init(struct host1x_client *client) vic->domain = tegra->domain; } - if (!vic->falcon.data) { - vic->falcon.data = tegra; - err = falcon_load_firmware(&vic->falcon); - if (err < 0) - goto detach; - } - vic->channel = host1x_channel_request(client->dev); if (!vic->channel) { err = -ENOMEM; @@ -372,10 +397,6 @@ static int vic_probe(struct platform_device *pdev) if (err < 0) return err; - err = falcon_read_firmware(&vic->falcon, vic->config->firmware); - if (err < 0) - goto exit_falcon; - platform_set_drvdata(pdev, vic); INIT_LIST_HEAD(&vic->client.base.list); -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: TK1: DRM, Nouveau and VIC
Hi Thierry On Mon, 2018-12-10 at 11:21 +0100, Thierry Reding wrote: > On Sat, Dec 08, 2018 at 02:54:45PM +, Marcel Ziswiler wrote: > > Hi Thierry et al. > > > > I noticed that since commit 3dde5a2342cd ("ARM: tegra: Add VIC on > > Tegra124") graphics on Apalis TK1 is broken. During boot it fails > > loading the vic firmware: > > > > [1.595824] tegra-vic 5434.vic: Direct firmware load for > > nvidia/tegra124/vic03_ucode.bin failed with error -2 > > [1.606140] tegra-vic: probe of 5434.vic failed with error > > -2 > > > > Subsequently Tegra HDMI seems to fail completely: > > > > [2.379860] tegra-hdmi 5428.hdmi: failed to get PLL > > regulator > > > > And finally, Nouveau even crashes: > > > > [8.241115] nouveau 5700.gpu: Linked as a consumer to > > regulator.31 > > [8.247889] nouveau 5700.gpu: NVIDIA GK20A (0ea000a1) > > [8.253396] nouveau 5700.gpu: imem: using IOMMU > > [8.270210] Unable to handle kernel NULL pointer dereference at > > virtual address 006c > > [8.278340] pgd = (ptrval) > > [8.281250] [006c] *pgd= > > [8.284944] Internal error: Oops: 5 [#1] PREEMPT SMP ARM > > [8.290260] Modules linked in: nouveau(+) ttm > > [8.294625] CPU: 2 PID: 203 Comm: systemd-udevd Not tainted > > 4.20.0- > > rc5-next-20181207-8-g85b0f8e25f86-dirty #110 > > [8.305055] Hardware name: NVIDIA Tegra SoC (Flattened Device > > Tree) > > [8.311331] PC is at drm_plane_register_all+0x18/0x50 > > [8.316373] LR is at drm_modeset_register_all+0xc/0x70 > > [8.321513] pc : []lr : []psr: > > a0060013 > > [8.327768] sp : ed527c70 ip : ecc43ec0 fp : > > [8.332993] r10: 0016 r9 : ecc43e80 r8 : > > [8.338209] r7 : bf182c80 r6 : r5 : ed61b24c r4 : > > fffc > > [8.344735] r3 : 0002f000 r2 : r1 : 2e124000 r0 : > > ed61b000 > > [8.351260] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA > > ARM Segment none > > [8.358383] Control: 10c5387d Table: ad64c06a DAC: 0051 > > [8.364127] Process systemd-udevd (pid: 203, stack limit = > > 0x(ptrval)) > > [8.370654] Stack: (0xed527c70 to 0xed528000) > > [8.375004] 7c60: ed61b000 > > ed61b000 c0564cc8 > > [8.383177] 7c80: ed61b000 c054b5b8 0001 > > 0001 > > [8.391355] 7ca0: ed527cc0 c0f08c48 ed61b000 > > bf180c5c bf0dc900 > > [8.399531] 7cc0: eda29208 5dfe844b ee9f2a10 > > bf180c5c c05a9328 > > [8.407695] 7ce0: c1006828 ee9f2a10 c100682c > > c05a744c ee9f2a10 bf180c5c > > [8.415871] 7d00: ee9f2a44 c05a77a8 c0f08c48 bf182980 > > c05a769c eefd14d0 c05a77a8 > > [8.424048] 7d20: ee9f2a10 bf180c5c ee9f2a44 c05a77a8 > > c0f08c48 bf182980 > > [8.432226] 7d40: c05a7884 ee9ebfb4 c0f08c48 bf180c5c > > c05a5790 ee88135c > > [8.440405] 7d60: ee9ebfb4 5dfe844b c0f71168 bf180c5c ee379e80 > > c0f71168 c05a692c > > [8.448570] 7d80: bf15dc00 bf180ac8 e000 bf180c5c bf180ac8 > > e000 bf1aa000 c05a84a0 > > [8.456746] 7da0: bf182b80 bf180ac8 e000 bf1aa170 c0fbd220 > > c0f08c48 e000 c0102ed0 > > [8.464924] 7dc0: ed53f4c0 006000c0 c01b3d98 000c 6113 > > bf182980 0040 c02592d0 > > [8.473102] 7de0: eda60200 2e124000 ee80 006000c0 006000c0 > > c01b3d98 000c c025a8cc > > [8.481281] 7e00: c024ce54 a113 bf182980 5dfe844b bf182980 > > 0002 ed53f4c0 0002 > > [8.489459] 7e20: eceba000 c01b3dd4 c0f08c48 bf182980 > > ed527f40 0002 eceb9fc0 > > [8.497625] 7e40: 0002 c01b61a4 bf18298c 7fff bf182980 > > c01b2f88 c01b279c > > [8.505800] 7e60: bf1829c8 bf182a80 bf182b6c bf182ab0 c0b03ab0 > > c0d58964 c0ca726c c0ca7278 > > [8.513978] 7e80: c0ca72d0 c0f08c48 c02654a0 > > e000 bf00 > > [8.522157] 7ea0: > > 6e72656b 6c65 > > [8.530336] 7ec0: > > > > [8.538502] 7ee0: > > 5dfe844b 7fff c0f08c48 > > [8.546677] 7f00: 000f b6f761cc c0101204 ed526000 > > 017b 004a3270 c01b66a4 > > [8.554855] 7f20: 7fff 0003 0001 004a3270 > > f0ced000 06e8994c > > [8.563032] 7f40: f0e37f3a f0e50a40 f0ced000 06e8994c f7b75f9c > > f7b75d34 f63e62dc 0016b000 > > [8.571209] 7f60: 0017f6f0 00050a48 > > 003b 003c 0023 > > [8.579388] 7f80: 0014 5dfe844b > > 004c0ec0 0001 > > [8.587554] 7fa0: 017b c0101000 004c0ec0 000f > > b6f761cc 0002 > > [8.595730] 7fc0: 004c0ec0 0001 017b 0048
[RESEND PATCH v3] backlight: pwm_bl: Fix brightness levels for non-DT case.
Commit '88ba95bedb79 ("backlight: pwm_bl: Compute brightness of LED linearly to human eye")' allows the possibility to compute a default brightness table when there isn't the brightness-levels property in the DT. Unfortunately the changes made broke the pwm backlight for the non-DT boards. Usually, the non-DT boards don't pass the brightness levels via platform data, instead, it sets the max_brightness in their platform data and the driver calculates the level without a table. The offending patch assumed that when there is no brightness levels table we should create one, but this is clearly wrong for the non-DT case. After this patch the code handles the DT and the non-DT case taking in consideration also if max_brightness is set or not. Fixes: 88ba95bedb79 ("backlight: pwm_bl: Compute brightness of LED linearly to human eye") Cc: Reported-by: Robert Jarzmik Signed-off-by: Enric Balletbo i Serra Tested-by: Robert Jarzmik Acked-by: Daniel Thompson --- Changes in v3: - Fixed some typos in commit message. - Removed ' in Fixes tag. Changes in v2: - Rebase on top of mainline - Add Tested-by and Acked-by tags. drivers/video/backlight/pwm_bl.c | 41 +++- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index b7b5b31f3824..feb90764a811 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -568,7 +568,30 @@ static int pwm_backlight_probe(struct platform_device *pdev) goto err_alloc; } - if (!data->levels) { + if (data->levels) { + /* +* For the DT case, only when brightness levels is defined +* data->levels is filled. For the non-DT case, data->levels +* can come from platform data, however is not usual. +*/ + for (i = 0; i <= data->max_brightness; i++) { + if (data->levels[i] > pb->scale) + pb->scale = data->levels[i]; + + pb->levels = data->levels; + } + } else if (!data->max_brightness) { + /* +* If no brightness levels are provided and max_brightness is +* not set, use the default brightness table. For the DT case, +* max_brightness is set to 0 when brightness levels is not +* specified. For the non-DT case, max_brightness is usually +* set to some value. +*/ + + /* Get the PWM period (in nanoseconds) */ + pwm_get_state(pb->pwm, &state); + ret = pwm_backlight_brightness_default(&pdev->dev, data, state.period); if (ret < 0) { @@ -576,13 +599,19 @@ static int pwm_backlight_probe(struct platform_device *pdev) "failed to setup default brightness table\n"); goto err_alloc; } - } - for (i = 0; i <= data->max_brightness; i++) { - if (data->levels[i] > pb->scale) - pb->scale = data->levels[i]; + for (i = 0; i <= data->max_brightness; i++) { + if (data->levels[i] > pb->scale) + pb->scale = data->levels[i]; - pb->levels = data->levels; + pb->levels = data->levels; + } + } else { + /* +* That only happens for the non-DT case, where platform data +* sets the max_brightness value. +*/ + pb->scale = data->max_brightness; } pb->lth_brightness = data->lth_brightness * (state.period / pb->scale); -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 30/29] drm/omap: Merge omap_dss_device type and output_type fields
Hi, On Mon, Dec 10, 2018 at 02:28:43PM +0200, Laurent Pinchart wrote: > The omap_dss_device type and output_type fields differ mostly for > historical reasons. The output_type field is required for all devices > but the display at the end of the pipeline, and must be set to > OMAP_DISPLAY_TYPE_NONE for the latter. The type field is required for > all devices but the internal encoder, for which it is ignored. > > The only reason why the output_type field must be set to > OMAP_DISPLAY_TYPE_NONE for the display at the end of the pipeline is to > identify omap_dss_device instances corresponding to displays. This is > not documented and confusing. > > Clean the code by adding a new display field to the omap_dss_device > structure to identify displays, and merge the type and output_type > fields. > > Signed-off-by: Laurent Pinchart > --- Reviewed-by: Sebastian Reichel -- Sebastian > .../gpu/drm/omapdrm/displays/connector-analog-tv.c | 1 + > drivers/gpu/drm/omapdrm/displays/connector-dvi.c | 1 + > drivers/gpu/drm/omapdrm/displays/connector-hdmi.c | 1 + > drivers/gpu/drm/omapdrm/displays/encoder-opa362.c | 1 - > drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c | 1 - > .../gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 1 - > drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 1 + > drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c| 1 + > .../omapdrm/displays/panel-lgphilips-lb035q02.c| 1 + > .../drm/omapdrm/displays/panel-nec-nl8048hl11.c| 1 + > .../drm/omapdrm/displays/panel-sharp-ls037v7dw01.c | 1 + > .../drm/omapdrm/displays/panel-sony-acx565akm.c| 1 + > .../drm/omapdrm/displays/panel-tpo-td028ttec1.c| 1 + > .../drm/omapdrm/displays/panel-tpo-td043mtea1.c| 1 + > drivers/gpu/drm/omapdrm/dss/base.c | 2 +- > drivers/gpu/drm/omapdrm/dss/dpi.c | 2 +- > drivers/gpu/drm/omapdrm/dss/dsi.c | 2 +- > drivers/gpu/drm/omapdrm/dss/hdmi4.c| 2 +- > drivers/gpu/drm/omapdrm/dss/hdmi5.c| 2 +- > drivers/gpu/drm/omapdrm/dss/omapdss.h | 14 +- > drivers/gpu/drm/omapdrm/dss/output.c | 2 +- > drivers/gpu/drm/omapdrm/dss/sdi.c | 2 +- > drivers/gpu/drm/omapdrm/dss/venc.c | 2 +- > drivers/gpu/drm/omapdrm/omap_crtc.c| 2 +- > drivers/gpu/drm/omapdrm/omap_encoder.c | 4 ++-- > 25 files changed, 31 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/displays/connector-analog-tv.c > b/drivers/gpu/drm/omapdrm/displays/connector-analog-tv.c > index 1503563117f3..6c0561101874 100644 > --- a/drivers/gpu/drm/omapdrm/displays/connector-analog-tv.c > +++ b/drivers/gpu/drm/omapdrm/displays/connector-analog-tv.c > @@ -56,6 +56,7 @@ static int tvc_probe(struct platform_device *pdev) > dssdev->ops = &tvc_ops; > dssdev->dev = &pdev->dev; > dssdev->type = OMAP_DISPLAY_TYPE_VENC; > + dssdev->display = true; > dssdev->owner = THIS_MODULE; > dssdev->of_ports = BIT(0); > > diff --git a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c > b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c > index bf5ee50ce5fe..fa3a69bf8a04 100644 > --- a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c > +++ b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c > @@ -239,6 +239,7 @@ static int dvic_probe(struct platform_device *pdev) > dssdev->ops = &dvic_ops; > dssdev->dev = &pdev->dev; > dssdev->type = OMAP_DISPLAY_TYPE_DVI; > + dssdev->display = true; > dssdev->owner = THIS_MODULE; > dssdev->of_ports = BIT(0); > > diff --git a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c > b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c > index 797da4a3f22e..68d6f6e44b03 100644 > --- a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c > +++ b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c > @@ -140,6 +140,7 @@ static int hdmic_probe(struct platform_device *pdev) > dssdev->ops = &hdmic_ops; > dssdev->dev = &pdev->dev; > dssdev->type = OMAP_DISPLAY_TYPE_HDMI; > + dssdev->display = true; > dssdev->owner = THIS_MODULE; > dssdev->of_ports = BIT(0); > dssdev->ops_flags = ddata->hpd_gpio > diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c > b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c > index fc5e0c47054d..29a5a130ebd1 100644 > --- a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c > +++ b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c > @@ -88,7 +88,6 @@ static int opa362_probe(struct platform_device *pdev) > dssdev->ops = &opa362_ops; > dssdev->dev = &pdev->dev; > dssdev->type = OMAP_DISPLAY_TYPE_VENC; > - dssdev->output_type = OMAP_DISPLAY_TYPE_VENC; > dssdev->owner = THIS_MODULE; > dssdev->of_ports = BIT(1) | BIT(0); > > diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c > b/drivers/gpu/drm/omapdrm/displays/encoder-t
[PATCH v5 00/17] drm/sun4i: Allwinner A64 MIPI-DSI support
This series fixed the issues related to work DSI on 2-lane panel which is reported on previous version[1][2][3] This supposed to be a clean series, where it support Allwinner A64 MIPI-DSI support for 4-lane, 2-lane DSI panels. This series fixed all previous series comments along with checkpatch warnings/error. Changes for v5: - collect Rob, Acked-by - droped "Fix VBP size calculation" patch - updated vblk timing calculation. - droped techstar, bananapi dsi panel drivers which may require bridge or other setup. it's under discussion. Changes for v4: - droppoed untested CCU_FEATURE_FIXED_POSTDIV check code in nkm min, max rate patches - create two patches for "Add Allwinner A64 MIPI DSI support" one for has_mod_clk quirk and other one for A64 support - use existing driver code construct for hblk computation - dropped "Increase hfp packet overhead" patch [2], though BSP added this but we have no issues as of now. (no issues on panel side w/o this change) - create separate function for vblk computation - enable vcc-dsi regulator in dsi_runtime_resume - collect Rob, Acked-by - update MAINTAINERS file for panel drivers - cleanup commit messages - fixed checkpatch warnings/errors [3] https://patchwork.kernel.org/cover/10680247/ [2] https://patchwork.kernel.org/patch/10657541/ [1] https://patchwork.kernel.org/patch/10657619/ Note: the respetive dts consumer for dsi will send once the panel driver finalized or in burst mode patch series. Any inputs, Jagan. Jagan Teki (17): clk: sunxi-ng: Add check for minimal rate to NKM PLLs drm/sun4i: sun6i_mipi_dsi: Add has_mod_clk quirk drm/sun4i: sun6i_mipi_dsi: Add Allwinner A64 MIPI DSI support dt-bindings: sun6i-dsi: Add compatible for A64 MIPI DSI drm/sun4i: sun6i_mipi_dsi: Add DSI Generic short write 2 param transfer drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay drm/sun4i: sun6i_mipi_dsi: Fix DSI hbp timing value drm/sun4i: sun6i_mipi_dsi: Fix DSI hblk timing calculation drm/sun4i: sun6i_mipi_dsi: Add DSI hblk packet overhead drm/sun4i: sun6i_mipi_dsi: Fix DSI hfp timing value drm/sun4i: sun6i_mipi_dsi: Set proper vblk timing calculation drm/sun4i: sun6i_mipi_dsi: Add support for VCC-DSI voltage regulator dt-bindings: sun6i-dsi: Add VCC-DSI supply property clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI dt-bindings: sun6i-dsi: Add A64 DPHY compatible (w/ A31 fallback) arm64: dts: allwinner: a64: Add DSI pipeline .../bindings/display/sunxi/sun6i-dsi.txt | 5 + arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 45 +++ drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 1 + drivers/clk/sunxi-ng/ccu_nkm.c| 5 + drivers/clk/sunxi-ng/ccu_nkm.h| 1 + drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c| 118 ++ drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h| 8 ++ 7 files changed, 159 insertions(+), 24 deletions(-) -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/meson: remove firmware framebuffers
On Mon, Dec 10, 2018 at 10:05 AM Maxime Jourdan wrote: > > In case we are using simplefb or another conflicting framebuffer, make > the call to drm_fb_helper_remove_conflicting_framebuffers() > > Signed-off-by: Maxime Jourdan > --- > drivers/gpu/drm/meson/meson_drv.c | 19 +++ > 1 file changed, 19 insertions(+) > Woops, some spaces made it in there.. will resend, sorry about that. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RESEND] drm/meson: remove firmware framebuffers
In case we are using simplefb or another conflicting framebuffer, make the call to drm_fb_helper_remove_conflicting_framebuffers() Signed-off-by: Maxime Jourdan --- drivers/gpu/drm/meson/meson_drv.c | 20 1 file changed, 20 insertions(+) diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c index d3443125e661..afbb3d707d15 100644 --- a/drivers/gpu/drm/meson/meson_drv.c +++ b/drivers/gpu/drm/meson/meson_drv.c @@ -159,6 +159,23 @@ static void meson_vpu_init(struct meson_drm *priv) writel_relaxed(0x2, priv->io_base + _REG(VPU_WRARB_MODE_L2C1)); } +static void meson_remove_framebuffers(void) +{ + struct apertures_struct *ap; + + ap = alloc_apertures(1); + if (!ap) + return; + + /* The framebuffer can be located anywhere in RAM */ + ap->ranges[0].base = 0; + ap->ranges[0].size = ~0; + + drm_fb_helper_remove_conflicting_framebuffers(ap, "meson-drm-fb", + false); + kfree(ap); +} + static int meson_drv_bind_master(struct device *dev, bool has_components) { struct platform_device *pdev = to_platform_device(dev); @@ -242,6 +259,9 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) if (ret) goto free_drm; + /* Remove early framebuffers (ie. simplefb) */ + meson_remove_framebuffers(); + drm_mode_config_init(drm); drm->mode_config.max_width = 3840; drm->mode_config.max_height = 2160; -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/5] clk: meson: meson8b: use a separate clock table for Meson8
On 08/12/2018 18:12, Martin Blumenstingl wrote: > The Meson8 SoC is slightly different compared to Meson8b and Meson8m2 > because it does not have the glitch-free Mali GPU clock mux. For Meson8b > and Meson8m2 there are currently no known differences. > > Add a separate clk_hw_onecell_data table for Meson8 so these differences > can be implemented. For now meson8_hw_onecell_data is a clone of our > existing meson8b_hw_onecell_data. > > Signed-off-by: Martin Blumenstingl > --- > drivers/clk/meson/meson8b.c | 203 ++-- > 1 file changed, 197 insertions(+), 6 deletions(-) > > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > index 950d0e548c75..0b9353d8d4fd 100644 > --- a/drivers/clk/meson/meson8b.c > +++ b/drivers/clk/meson/meson8b.c > @@ -1659,6 +1659,185 @@ static MESON_GATE(meson8b_ao_ahb_sram, HHI_GCLK_AO, > 1); > static MESON_GATE(meson8b_ao_ahb_bus, HHI_GCLK_AO, 2); > static MESON_GATE(meson8b_ao_iface, HHI_GCLK_AO, 3); > > +static struct clk_hw_onecell_data meson8_hw_onecell_data = { > + .hws = { > + [CLKID_XTAL] = &meson8b_xtal.hw, > + [CLKID_PLL_FIXED] = &meson8b_fixed_pll.hw, > + [CLKID_PLL_VID] = &meson8b_vid_pll.hw, > + [CLKID_PLL_SYS] = &meson8b_sys_pll.hw, > + [CLKID_FCLK_DIV2] = &meson8b_fclk_div2.hw, > + [CLKID_FCLK_DIV3] = &meson8b_fclk_div3.hw, > + [CLKID_FCLK_DIV4] = &meson8b_fclk_div4.hw, > + [CLKID_FCLK_DIV5] = &meson8b_fclk_div5.hw, > + [CLKID_FCLK_DIV7] = &meson8b_fclk_div7.hw, > + [CLKID_CPUCLK] = &meson8b_cpu_clk.hw, > + [CLKID_MPEG_SEL] = &meson8b_mpeg_clk_sel.hw, > + [CLKID_MPEG_DIV] = &meson8b_mpeg_clk_div.hw, > + [CLKID_CLK81] = &meson8b_clk81.hw, > + [CLKID_DDR] = &meson8b_ddr.hw, > + [CLKID_DOS] = &meson8b_dos.hw, > + [CLKID_ISA] = &meson8b_isa.hw, > + [CLKID_PL301] = &meson8b_pl301.hw, > + [CLKID_PERIPHS] = &meson8b_periphs.hw, > + [CLKID_SPICC] = &meson8b_spicc.hw, > + [CLKID_I2C] = &meson8b_i2c.hw, > + [CLKID_SAR_ADC] = &meson8b_sar_adc.hw, > + [CLKID_SMART_CARD] = &meson8b_smart_card.hw, > + [CLKID_RNG0]= &meson8b_rng0.hw, > + [CLKID_UART0] = &meson8b_uart0.hw, > + [CLKID_SDHC]= &meson8b_sdhc.hw, > + [CLKID_STREAM] = &meson8b_stream.hw, > + [CLKID_ASYNC_FIFO] = &meson8b_async_fifo.hw, > + [CLKID_SDIO]= &meson8b_sdio.hw, > + [CLKID_ABUF]= &meson8b_abuf.hw, > + [CLKID_HIU_IFACE] = &meson8b_hiu_iface.hw, > + [CLKID_ASSIST_MISC] = &meson8b_assist_misc.hw, > + [CLKID_SPI] = &meson8b_spi.hw, > + [CLKID_I2S_SPDIF] = &meson8b_i2s_spdif.hw, > + [CLKID_ETH] = &meson8b_eth.hw, > + [CLKID_DEMUX] = &meson8b_demux.hw, > + [CLKID_AIU_GLUE]= &meson8b_aiu_glue.hw, > + [CLKID_IEC958] = &meson8b_iec958.hw, > + [CLKID_I2S_OUT] = &meson8b_i2s_out.hw, > + [CLKID_AMCLK] = &meson8b_amclk.hw, > + [CLKID_AIFIFO2] = &meson8b_aififo2.hw, > + [CLKID_MIXER] = &meson8b_mixer.hw, > + [CLKID_MIXER_IFACE] = &meson8b_mixer_iface.hw, > + [CLKID_ADC] = &meson8b_adc.hw, > + [CLKID_BLKMV] = &meson8b_blkmv.hw, > + [CLKID_AIU] = &meson8b_aiu.hw, > + [CLKID_UART1] = &meson8b_uart1.hw, > + [CLKID_G2D] = &meson8b_g2d.hw, > + [CLKID_USB0]= &meson8b_usb0.hw, > + [CLKID_USB1]= &meson8b_usb1.hw, > + [CLKID_RESET] = &meson8b_reset.hw, > + [CLKID_NAND]= &meson8b_nand.hw, > + [CLKID_DOS_PARSER] = &meson8b_dos_parser.hw, > + [CLKID_USB] = &meson8b_usb.hw, > + [CLKID_VDIN1] = &meson8b_vdin1.hw, > + [CLKID_AHB_ARB0]= &meson8b_ahb_arb0.hw, > + [CLKID_EFUSE] = &meson8b_efuse.hw, > + [CLKID_BOOT_ROM]= &meson8b_boot_rom.hw, > + [CLKID_AHB_DATA_BUS]= &meson8b_ahb_data_bus.hw, > + [CLKID_AHB_CTRL_BUS]= &meson8b_ahb_ctrl_bus.hw, > + [CLKID_HDMI_INTR_SYNC] = &meson8b_hdmi_intr_sync.hw, > + [CLKID_HDMI_PCLK] = &meson8b_hdmi_pclk.hw,
[PATCH v5 01/17] clk: sunxi-ng: Add check for minimal rate to NKM PLLs
Some NKM PLLs doesn't work well when their output clock rate is set below certain rate. So, add support for minimal rate for relevant PLLs. Signed-off-by: Jagan Teki Acked-by: Stephen Boyd --- drivers/clk/sunxi-ng/ccu_nkm.c | 5 + drivers/clk/sunxi-ng/ccu_nkm.h | 1 + 2 files changed, 6 insertions(+) diff --git a/drivers/clk/sunxi-ng/ccu_nkm.c b/drivers/clk/sunxi-ng/ccu_nkm.c index 841840e35e61..096ff4f4839a 100644 --- a/drivers/clk/sunxi-ng/ccu_nkm.c +++ b/drivers/clk/sunxi-ng/ccu_nkm.c @@ -125,6 +125,11 @@ static unsigned long ccu_nkm_round_rate(struct ccu_mux_internal *mux, if (nkm->common.features & CCU_FEATURE_FIXED_POSTDIV) rate *= nkm->fixed_post_div; + if (rate < nkm->min_rate) { + rate = nkm->min_rate; + return rate; + } + ccu_nkm_find_best(*parent_rate, rate, &_nkm); rate = *parent_rate * _nkm.n * _nkm.k / _nkm.m; diff --git a/drivers/clk/sunxi-ng/ccu_nkm.h b/drivers/clk/sunxi-ng/ccu_nkm.h index cc6efb70a102..ff5bd00f429f 100644 --- a/drivers/clk/sunxi-ng/ccu_nkm.h +++ b/drivers/clk/sunxi-ng/ccu_nkm.h @@ -35,6 +35,7 @@ struct ccu_nkm { struct ccu_mux_internal mux; unsigned intfixed_post_div; + unsigned intmin_rate; struct ccu_common common; }; -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 15/17] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
Minimum PLL used for MIPI is 500MHz, as per manual, but lowering the min rate by 300MHz can result proper working nkms divider with the help of desired dclock rate from panel driver. Signed-off-by: Jagan Teki Acked-by: Stephen Boyd --- drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c index 181b599dc163..b623c8150b4f 100644 --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c @@ -183,6 +183,7 @@ static struct ccu_nkm pll_mipi_clk = { .n = _SUNXI_CCU_MULT(8, 4), .k = _SUNXI_CCU_MULT_MIN(4, 2, 2), .m = _SUNXI_CCU_DIV(0, 4), + .min_rate = 3,/* Actual rate is 500MHz */ .common = { .reg= 0x040, .hw.init= CLK_HW_INIT("pll-mipi", "pll-video0", -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm/nouveau: tegra: Call nouveau_drm_device_init()
On Fri, 2018-11-23 at 13:11 +0100, Thierry Reding wrote: > From: Thierry Reding > > As part of commit cfea88a4d866 ("drm/nouveau: Start using new drm_dev > initialization helpers"), the initialization of the Nouveau DRM > device > was reworked and along the way the platform driver initialization was > left incomplete. Add a call to nouveau_drm_device_init() to make sure > all of the structures are properly initialized. > > Signed-off-by: Thierry Reding > Reviewed-by: Lyude Paul Tested-by: Marcel Ziswiler > --- > drivers/gpu/drm/nouveau/nouveau_drm.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c > b/drivers/gpu/drm/nouveau/nouveau_drm.c > index 2b2baf6e0e0d..d2928d43f29a 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > @@ -1171,10 +1171,16 @@ nouveau_platform_device_create(const struct > nvkm_device_tegra_func *func, > goto err_free; > } > > + err = nouveau_drm_device_init(drm); > + if (err) > + goto err_put; > + > platform_set_drvdata(pdev, drm); > > return drm; > > +err_put: > + drm_dev_put(drm); > err_free: > nvkm_device_del(pdevice); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 06/17] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits
TCON DRQ set bits for non-burst DSI mode can computed via horizontal front porch instead of front porch + sync timings. BSP code form BPI-M64-bsp is computing TCON DRQ set bits for non-burts as (from linux-sunxi/ drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) => panel->lcd_ht -panel->lcd_x - panel->lcd_hbp => (timmings->hor_front_porch + panel->lcd_hbp + panel->lcd_x) - panel->lcd_x - panel->hbp => timmings->hor_front_porch => mode->hsync_start - mode->hdisplay So, update the DRQ set bits accordingly. Signed-off-by: Jagan Teki --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index cdd44a1307b3..c9b0222ebcd4 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -367,9 +367,9 @@ static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi, struct mipi_dsi_device *device = dsi->device; u32 val = 0; - if ((mode->hsync_end - mode->hdisplay) > 20) { + if ((mode->hsync_start - mode->hdisplay) > 20) { /* Maagic */ - u16 drq = (mode->hsync_end - mode->hdisplay) - 20; + u16 drq = (mode->hsync_start - mode->hdisplay) - 20; drq *= mipi_dsi_pixel_format_to_bpp(device->format); drq /= 32; -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 24/29] drm/omap: Factor out common mode validation code
Hi Laurent, On Mon, Dec 10, 2018 at 10:27:05AM +0200, Laurent Pinchart wrote: > On Monday, 10 December 2018 00:19:22 EET Sebastian Reichel wrote: > > On Wed, Dec 05, 2018 at 05:00:17PM +0200, Laurent Pinchart wrote: > > > The encoder .atomic_check() and connector .mode_valid() operations both > > > walk through the dss devices in the pipeline to validate the mode. > > > Factor out the common code in a new omap_drm_connector_mode_fixup() > > > function. > > > > > > Signed-off-by: Laurent Pinchart > > > --- > > > > This is a bit tricky to review. It would probably be easier to > > review, if the changes were split into two commits: > > > > 1. introduce common function > > 2. move drm_display_mode/videomode conversion further up the stack > > I agree that the connector changes are not very nicely formatted. I however > don't like splitting patches, especially when small, in such a way. > > One issue preventing a better diff formatting is the change from int to enum > drm_mode_status for the omap_connector_mode_valid() function. Without that > change, the patch can be better formatted with > > git diff --anchored="static int omap_connector_mode_valid" > > I agree it's still not perfect. > > If you think the above change is worth it I'll submit a split version, > otherwise I'll leave it as is. You don't have to change it for me. I just asked for better formating in future patches. -- Sebastian signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: TK1: DRM, Nouveau and VIC
Hi Thierry On Mon, 2018-12-10 at 17:23 +0100, Thierry Reding wrote: Snip. > > Looks like with pci_disable_device() it may take a rather strange > > path... > > Yikes... it has no business at all calling pci_disable_device() on > Tegra. Unless if you happen to have a GPU plugged into the PCIe slot. > I'm assuming that's not what you're doing? Nope, I only have a Wi-Fi card behind a PCIe switch though (;-p). > I'll see if I can reproduce (and fix) that crash on unload. > Admittedly > it's not something that I regularly test. Perhaps that's something > that > I should change... Don't worry. After a couple of years working on this I happen to try this the first time myself just now (;-p). > Thierry Cheers Marcel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 05/17] drm/sun4i: sun6i_mipi_dsi: Add DSI Generic short write 2 param transfer
Short transfer write support for DCS and Generic transfer types share similar way to process command sequence in DSI block so add generic write 2 param transfer type macro so-that the panels which are requesting similar transfer type may process properly. Signed-off-by: Jagan Teki --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 50f535ae57e9..cdd44a1307b3 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -871,6 +871,7 @@ static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host, switch (msg->type) { case MIPI_DSI_DCS_SHORT_WRITE: case MIPI_DSI_DCS_SHORT_WRITE_PARAM: + case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM: ret = sun6i_dsi_dcs_write_short(dsi, msg); break; -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/8] drm/msm/dsi: 28nm PHY: Get ref clock from the DT
Quoting Matthias Kaehlcke (2018-12-04 14:42:29) > Get the ref clock of the PHY from the device tree instead of > hardcoding its name and rate. > > Signed-off-by: Matthias Kaehlcke > --- Reviewed-by: Stephen Boyd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 09/17] drm/sun4i: sun6i_mipi_dsi: Fix DSI hblk timing calculation
hblk is adding line with all porch timing values, or timings values from htotal without sync time. Current driver is subtracting htotal with hsa, but the hsa is bounded with packet overhead. For real hblk calculation needed by subtracting htotal with back and front porch values and BSP code BPI-M64-bsp is eventually following the same. BPI-M64-bsp is computing hbp as (from linux-sunxi/ drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2); => (timmings->hor_total_time - timmings->hor_sync_time) => (mode->htotal - (mode->hsync_end - mode->hsync_start)) So, update the DSI hblk timing accordingly. Tested on 2-lane, 4-lane MIPI-DSI LCD panels. Signed-off-by: Jagan Teki --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 81151d7633f9..4c95b3384ed9 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -495,7 +495,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, /* * hblk seems to be the line + porches length. */ - hblk = mode->htotal * Bpp - hsa; + hblk = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp; /* * And I'm not entirely sure what vblk is about. The driver in -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 22/29] drm/omap: Move DISPC timing checks to CRTC .mode_valid() operation
Hi, On Mon, Dec 10, 2018 at 09:50:08AM +0200, Laurent Pinchart wrote: > Hi Sebastian, > > On Monday, 10 December 2018 00:07:55 EET Sebastian Reichel wrote: > > On Wed, Dec 05, 2018 at 05:00:15PM +0200, Laurent Pinchart wrote: > > > The DISPC timings checks relate to the CRTC, but they're performed in > > > the encoder and connector .atomic_check() and .mode_valid() operations. > > > Move them to the CRTC .mode_valid() operation. > > > > > > Signed-off-by: Laurent Pinchart > > > --- > > > > Reviewed-by: Sebastian Reichel > > > > This should also fix the issue with DSI in a less ugly way: > > > > https://lists.freedesktop.org/archives/dri-devel/2018-November/196622.html > > Should I add the > > Fixes: 7c27fa57ef31 ("drm/omap: Call dispc timings check operation directly") > > tag to this patch ? I plan to test this series in combination with my DSI patches today or tomorrow evening. If it works, probably the following should be added to the patch description: As a side-effect this fixes DSI, which uses slightly different mode configuration for the DSI encoder and the DISPC. Fixes: 7c27fa57ef31 ("drm/omap: Call dispc timings check operation directly") -- Sebastian > > > drivers/gpu/drm/omapdrm/omap_connector.c | 6 -- > > > drivers/gpu/drm/omapdrm/omap_crtc.c | 9 + > > > drivers/gpu/drm/omapdrm/omap_encoder.c | 6 -- > > > 3 files changed, 9 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c > > > b/drivers/gpu/drm/omapdrm/omap_connector.c index > > > 8db1f2fbcf43..dd1e0f2e8ffc 100644 > > > --- a/drivers/gpu/drm/omapdrm/omap_connector.c > > > +++ b/drivers/gpu/drm/omapdrm/omap_connector.c > > > @@ -245,8 +245,6 @@ static int omap_connector_mode_valid(struct > > > drm_connector *connector,> > > >struct drm_display_mode *mode) > > > > > > { > > > > > > struct omap_connector *omap_connector = to_omap_connector(connector); > > > > > > - enum omap_channel channel = omap_connector->output->dispc_channel; > > > - struct omap_drm_private *priv = connector->dev->dev_private; > > > > > > struct omap_dss_device *dssdev; > > > struct videomode vm = {0}; > > > struct drm_device *dev = connector->dev; > > > > > > @@ -256,10 +254,6 @@ static int omap_connector_mode_valid(struct > > > drm_connector *connector,> > > > drm_display_mode_to_videomode(mode, &vm); > > > mode->vrefresh = drm_mode_vrefresh(mode); > > > > > > - r = priv->dispc_ops->mgr_check_timings(priv->dispc, channel, &vm); > > > - if (r) > > > - goto done; > > > - > > > > > > for (dssdev = omap_connector->output; dssdev; dssdev = dssdev->next) { > > > > > > if (!dssdev->ops->check_timings) > > > > > > continue; > > > > > > diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c > > > b/drivers/gpu/drm/omapdrm/omap_crtc.c index caffc547ef97..51036e6fca1c > > > 100644 > > > --- a/drivers/gpu/drm/omapdrm/omap_crtc.c > > > +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c > > > @@ -391,6 +391,15 @@ static enum drm_mode_status > > > omap_crtc_mode_valid(struct drm_crtc *crtc,> > > > const struct drm_display_mode *mode) > > > > > > { > > > > > > struct omap_drm_private *priv = crtc->dev->dev_private; > > > > > > + struct omap_crtc *omap_crtc = to_omap_crtc(crtc); > > > + struct videomode vm = {0}; > > > + int r; > > > + > > > + drm_display_mode_to_videomode(mode, &vm); > > > + r = priv->dispc_ops->mgr_check_timings(priv->dispc, omap_crtc->channel, > > > +&vm); > > > + if (r) > > > + return r; > > > > > > /* Check for bandwidth limit */ > > > if (priv->max_bandwidth) { > > > > > > diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c > > > b/drivers/gpu/drm/omapdrm/omap_encoder.c index 79ee927cd905..7d01df6fa723 > > > 100644 > > > --- a/drivers/gpu/drm/omapdrm/omap_encoder.c > > > +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c > > > @@ -198,19 +198,13 @@ static int omap_encoder_atomic_check(struct > > > drm_encoder *encoder,> > > >struct drm_connector_state *conn_state) > > > > > > { > > > > > > struct omap_encoder *omap_encoder = to_omap_encoder(encoder); > > > > > > - enum omap_channel channel = omap_encoder->output->dispc_channel; > > > > > > struct drm_device *dev = encoder->dev; > > > > > > - struct omap_drm_private *priv = dev->dev_private; > > > > > > struct omap_dss_device *dssdev; > > > struct videomode vm = { 0 }; > > > int ret; > > > > > > drm_display_mode_to_videomode(&crtc_state->mode, &vm); > > > > > > - ret = priv->dispc_ops->mgr_check_timings(priv->dispc, channel, &vm); > > > - if (ret) > > > - goto done; > > > - > > > > > > for (dssdev = omap_encoder->output; dssdev; dssdev = dssdev->next) { > > > > > > if (!dssdev->ops->check_timings) > > > > > >
Re: [linux-sunxi] Re: [PATCH v4 08/26] drm/sun4i: sun6i_mipi_dsi: Fix VBP size calculation
On Fri, Dec 7, 2018 at 6:51 PM Maxime Ripard wrote: > > On Tue, Nov 27, 2018 at 04:34:35PM +0530, Jagan Teki wrote: > > On Tue, Nov 27, 2018 at 3:55 PM Maxime Ripard > > wrote: > > > > > > On Tue, Nov 20, 2018 at 09:55:42PM +0530, Jagan Teki wrote: > > > > On Tue, Nov 20, 2018 at 9:27 PM Maxime Ripard > > > > wrote: > > > > > > > > > > On Thu, Nov 15, 2018 at 11:19:53PM +0530, Jagan Teki wrote: > > > > > > On Thu, Nov 15, 2018 at 3:26 PM Maxime Ripard > > > > > > wrote: > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > On Tue, Nov 13, 2018 at 04:46:15PM +0530, Jagan Teki wrote: > > > > > > > > The horizontal and vertical back porch calculation in BSP > > > > > > > > code is simply following the Linux drm comment diagram, in > > > > > > > > include/drm/drm_modes.h which is > > > > > > > > > > > > > > > > [hv]back porch = [hv]total - [hv]sync_end > > > > > > > > > > > > > > > > BSP code form BPI-M64-bsp is calculating vertical back porch as > > > > > > > > (from linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c) > > > > > > > > > > > > > > > > timmings->ver_sync_time= panel_info->lcd_vspw; > > > > > > > > timmings->ver_back_porch= > > > > > > > > panel_info->lcd_vbp-panel_info->lcd_vspw; > > > > > > > > > > > > > > > > vbp = panel->lcd_vbp; > > > > > > > > vspw = panel->lcd_vspw; > > > > > > > > dsi_dev[sel]->dsi_basic_size0.bits.vbp = vbp-vspw; > > > > > > > > dsi_dev[sel]->dsi_basic_size0.bits.vbp = panel->lcd_vbp - > > > > > > > > panel->lcd_vspw; > > > > > > > > => timmings->ver_back_porch + panel_info->lcd_vspw - > > > > > > > > panel_info->lcd_vspw > > > > > > > > => timmings->ver_back_porch > > > > > > > > => mode->vtotal - mode->end > > > > > > > > > > > > > > > > Which evatually same as mode->vtotal - mode->vsync_end so > > > > > > > > update the > > > > > > > > same in SUN6I_DSI_BASIC_SIZE0_VBP > > > > > > > > > > > > > > > > On the information note, existing SUN6I_DSI_BASIC_SIZE0_VSA is > > > > > > > > proper > > > > > > > > value. > > > > > > > > > > > > > > > > Signed-off-by: Jagan Teki > > > > > > > > > > > > > > I've tested your changes on my A33 board, and this commit will > > > > > > > break > > > > > > > it. > > > > > > > > > > > > > > It creates vblank timeouts, and visual artifacts at the bottom of > > > > > > > the > > > > > > > display. > > > > > > > > > > > > Strange, VBP is earlier gives front porch which is anyway wrong. > > > > > > > > > > > > > > > > > > > > Later commits seem to fix the issue, but will create some > > > > > > > blanking on > > > > > > > the upper third of the display. > > > > > > > > > > > > > > Since the documentation is quite sparse, and a MIPI-DSI analyzer > > > > > > > is > > > > > > > way too expensive, I'd really like to have at least what each of > > > > > > > these > > > > > > > commits are actually fixing, and what symptoms each of these were > > > > > > > causing, and not just "the BSP does it". > > > > > > > > > > > > W/o this 2-lane panel is breaking, same vblank timeout and visual > > > > > > artifacts at the bottom of the panel. though the commits may > > > > > > reference > > > > > > BSP, I have at-least tested on 3 different panels for us to prove > > > > > > its > > > > > > working. > > > > > > > > > > > > > Having some datasheet for the panels you had working would help > > > > > > > too. > > > > > > > > > > > > Unfortunately datasheet doesn't have any required information what > > > > > > we > > > > > > actually looking for. > > > > > > > > > > Not even the timings? How did you get that information then? > > > > > > > > datasheet has timing values, but this changes need controller > > > > information about VBP register that I don't have. But again existing > > > > VBP is not back porch for real, it's front porch. > > > > > > Yet, this breaks the existing setup. So again: > > > > Was it with 4-lane or 2-lane panel? can you test it with 2-lane panel? > > I can see the issue on 2-lane but the 4-lane working fine with this > > patch even. > > It's a 4 lane display. Thanks, look like my panel timings seems reverse b/w BP and FP. It work irrespective of this change after proper update. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 11/12] drm/panel: Add Feiyang FY07024DI26A30-D MIPI-DSI LCD panel
Hi Thierry and David, On Fri, Nov 16, 2018 at 10:10 PM Jagan Teki wrote: > > Feiyang FY07024DI26A30-D is 1024x600, 4-lane MIPI-DSI LCD panel. > > Add panel driver for it. > > Signed-off-by: Jagan Teki > --- > MAINTAINERS | 6 + > drivers/gpu/drm/panel/Kconfig | 9 + > drivers/gpu/drm/panel/Makefile| 1 + > .../drm/panel/panel-feiyang-fy07024di26a30d.c | 286 ++ > 4 files changed, 302 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 3dac08d0b3cb..40c8bfc974f4 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4620,6 +4620,12 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > S: Maintained > F: drivers/gpu/drm/tve200/ > > +DRM DRIVER FOR FEIYANG FY07024DI26A30-D MIPI-DSI LCD PANELS > +M: Jagan Teki > +S: Maintained > +F: drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c > +F: > Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.txt > + > DRM DRIVER FOR ILITEK ILI9225 PANELS > M: David Lechner > S: Maintained > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > index d0d4e60f5153..bc70896fe43c 100644 > --- a/drivers/gpu/drm/panel/Kconfig > +++ b/drivers/gpu/drm/panel/Kconfig > @@ -47,6 +47,15 @@ config DRM_PANEL_SIMPLE > that it can be automatically turned off when the panel goes into a > low power state. > > +config DRM_PANEL_FEIYANG_FY07024DI26A30D > + tristate "Feiyang FY07024DI26A30-D MIPI-DSI LCD panel" > + depends on OF > + depends on DRM_MIPI_DSI > + depends on BACKLIGHT_CLASS_DEVICE > + help > + Say Y if you want to enable support for panels based on the > + Feiyang FY07024DI26A30-D MIPI-DSI interface. > + Any comments on this? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 12/17] drm/sun4i: sun6i_mipi_dsi: Set proper vblk timing calculation
Unlike hblk, the vblk timings should follow an equation to compute the desired value for lane 4 devices and rest of devices it would be 0. BSP code from BPI-M64-bsp is computing vblk as for 4-lane devices (from linux-sunxi drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) tmp = (ht*dsi_pixel_bits[format]/8)*vt-(4+dsi_hblk+2); dsi_vblk = (lane-tmp%lane); So, update the vblk timing calculation accordingly. Tested on 2-lane, 4-lane MIPI-DSI LCD panels. Signed-off-by: Jagan Teki --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 29 +++--- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index d8947be92f9d..cbcef7bf7681 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -355,6 +355,27 @@ static void sun6i_dsi_inst_init(struct sun6i_dsi *dsi, SUN6I_DSI_INST_JUMP_CFG_NUM(1)); }; +static u16 sun6i_dsi_get_timings_vblk(struct sun6i_dsi *dsi, + struct drm_display_mode *mode, u16 hblk) +{ + struct mipi_dsi_device *device = dsi->device; + unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8; + int tmp; + + if (device->lanes != 4) + return 0; + + /* +* The vertical blank is set using a blanking packet (4 bytes + +* payload + 2 bytes). Its minimal size is therefore 6 bytes +*/ +#define VBLK_PACKET_OVERHEAD 6 + tmp = (mode->htotal * Bpp) * mode->vtotal - + (hblk + VBLK_PACKET_OVERHEAD); + + return (device->lanes - tmp % device->lanes); +} + static u16 sun6i_dsi_get_video_start_delay(struct sun6i_dsi *dsi, struct drm_display_mode *mode) { @@ -503,13 +524,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp - HBLK_PACKET_OVERHEAD); - /* -* And I'm not entirely sure what vblk is about. The driver in -* Allwinner BSP is using a rather convoluted calculation -* there only for 4 lanes. However, using 0 (the !4 lanes -* case) even with a 4 lanes screen seems to work... -*/ - vblk = 0; + vblk = sun6i_dsi_get_timings_vblk(dsi, mode, hblk); /* How many bytes do we need to send all payloads? */ bytes = max_t(size_t, max(max(hfp, hblk), max(hsa, hbp)), vblk); -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 13/17] drm/sun4i: sun6i_mipi_dsi: Add support for VCC-DSI voltage regulator
Some boards have VCC-DSI pin connected to voltage regulator which may not be turned on by default. Add support for such boards by adding voltage regulator handling code to MIPI DSI driver. Signed-off-by: Jagan Teki --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 14 ++ drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 3 +++ 2 files changed, 17 insertions(+) diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index cbcef7bf7681..a87b65fff0e0 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -1023,6 +1023,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev) return PTR_ERR(base); } + dsi->regulator = devm_regulator_get(dev, "vcc-dsi"); + if (IS_ERR(dsi->regulator)) { + dev_err(dev, "Couldn't get VCC-DSI supply\n"); + return PTR_ERR(dsi->regulator); + } + dsi->regs = devm_regmap_init_mmio_clk(dev, "bus", base, &sun6i_dsi_regmap_config); if (IS_ERR(dsi->regs)) { @@ -1104,6 +1110,13 @@ static int sun6i_dsi_remove(struct platform_device *pdev) static int __maybe_unused sun6i_dsi_runtime_resume(struct device *dev) { struct sun6i_dsi *dsi = dev_get_drvdata(dev); + int err; + + err = regulator_enable(dsi->regulator); + if (err) { + dev_err(dsi->dev, "failed to enable VCC-DSI supply: %d\n", err); + return err; + } reset_control_deassert(dsi->reset); if (dsi->variant->has_mod_clk) @@ -1138,6 +1151,7 @@ static int __maybe_unused sun6i_dsi_runtime_suspend(struct device *dev) if (dsi->variant->has_mod_clk) clk_disable_unprepare(dsi->mod_clk); reset_control_assert(dsi->reset); + regulator_disable(dsi->regulator); return 0; } diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h index 597b62227019..0df60f84bab3 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h @@ -13,6 +13,8 @@ #include #include +#include + struct sun6i_dphy { struct clk *bus_clk; struct clk *mod_clk; @@ -32,6 +34,7 @@ struct sun6i_dsi { struct clk *bus_clk; struct clk *mod_clk; struct regmap *regs; + struct regulator*regulator; struct reset_control*reset; struct sun6i_dphy *dphy; -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 2/8] drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT
Quoting Matthias Kaehlcke (2018-12-04 14:42:28) > Get the ref clock of the PHY from the device tree instead of > hardcoding its name and rate. > > Signed-off-by: Matthias Kaehlcke > --- Reviewed-by: Stephen Boyd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/tegra: Refactor CEC support
On 12/10/18 5:36 PM, Hans Verkuil wrote: > On 12/10/18 5:34 PM, Thierry Reding wrote: >> From: Thierry Reding >> >> Most of the CEC support code already lives in the "output" library code. >> Move registration and unregistration to the library code as well to make >> use of the same code with HDMI on Tegra210 and later via the SOR. >> >> Signed-off-by: Thierry Reding > > Reviewed-by: Hans Verkuil But read my reply to "[PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194". You may want to take a different approach here. Regards, Hans > > Thanks! > > Hans > >> --- >> drivers/gpu/drm/tegra/drm.h| 2 +- >> drivers/gpu/drm/tegra/hdmi.c | 9 - >> drivers/gpu/drm/tegra/output.c | 11 +-- >> 3 files changed, 10 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h >> index 019862a41cb4..dbc9e11b0aec 100644 >> --- a/drivers/gpu/drm/tegra/drm.h >> +++ b/drivers/gpu/drm/tegra/drm.h >> @@ -132,7 +132,7 @@ struct tegra_output { >> struct drm_panel *panel; >> struct i2c_adapter *ddc; >> const struct edid *edid; >> -struct cec_notifier *notifier; >> +struct cec_notifier *cec; >> unsigned int hpd_irq; >> int hpd_gpio; >> enum of_gpio_flags hpd_gpio_flags; >> diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c >> index 0082468f703c..d19973945614 100644 >> --- a/drivers/gpu/drm/tegra/hdmi.c >> +++ b/drivers/gpu/drm/tegra/hdmi.c >> @@ -22,8 +22,6 @@ >> >> #include >> >> -#include >> - >> #include "hdmi.h" >> #include "drm.h" >> #include "dc.h" >> @@ -1709,10 +1707,6 @@ static int tegra_hdmi_probe(struct platform_device >> *pdev) >> return PTR_ERR(hdmi->vdd); >> } >> >> -hdmi->output.notifier = cec_notifier_get(&pdev->dev); >> -if (hdmi->output.notifier == NULL) >> -return -ENOMEM; >> - >> hdmi->output.dev = &pdev->dev; >> >> err = tegra_output_probe(&hdmi->output); >> @@ -1771,9 +1765,6 @@ static int tegra_hdmi_remove(struct platform_device >> *pdev) >> >> tegra_output_remove(&hdmi->output); >> >> -if (hdmi->output.notifier) >> -cec_notifier_put(hdmi->output.notifier); >> - >> return 0; >> } >> >> diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c >> index c662efc7e413..9c2b9dad55c3 100644 >> --- a/drivers/gpu/drm/tegra/output.c >> +++ b/drivers/gpu/drm/tegra/output.c >> @@ -36,7 +36,7 @@ int tegra_output_connector_get_modes(struct drm_connector >> *connector) >> else if (output->ddc) >> edid = drm_get_edid(connector, output->ddc); >> >> -cec_notifier_set_phys_addr_from_edid(output->notifier, edid); >> +cec_notifier_set_phys_addr_from_edid(output->cec, edid); >> drm_connector_update_edid_property(connector, edid); >> >> if (edid) { >> @@ -73,7 +73,7 @@ tegra_output_connector_detect(struct drm_connector >> *connector, bool force) >> } >> >> if (status != connector_status_connected) >> -cec_notifier_phys_addr_invalidate(output->notifier); >> +cec_notifier_phys_addr_invalidate(output->cec); >> >> return status; >> } >> @@ -174,11 +174,18 @@ int tegra_output_probe(struct tegra_output *output) >> disable_irq(output->hpd_irq); >> } >> >> +output->cec = cec_notifier_get(output->dev); >> +if (!output->cec) >> +return -ENOMEM; >> + >> return 0; >> } >> >> void tegra_output_remove(struct tegra_output *output) >> { >> +if (output->cec) >> +cec_notifier_put(output->cec); >> + >> if (gpio_is_valid(output->hpd_gpio)) { >> free_irq(output->hpd_irq, output); >> gpio_free(output->hpd_gpio); >> > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/8] drm/msm/dsi: 10nm PHY: Get ref clock from the DT
Quoting Matthias Kaehlcke (2018-12-04 14:42:31) > Get the ref clock of the PHY from the device tree instead of > hardcoding its name and rate. > > Note: This change could break old out-of-tree DTS files that > use the 10nm PHY > > Signed-off-by: Matthias Kaehlcke > Reviewed-by: Douglas Anderson > --- > Changes in v4: > - none > > Changes in v3: > - fixed check for EPROBE_DEFER > - added note to commit message about breaking old DTS files > - added 'Reviewed-by: Douglas Anderson ' tag > > Changes in v2: > - remove anonymous array in clk_init_data assignment > - log error code if devm_clk_get() fails > - don't log devm_clk_get() failures for -EPROBE_DEFER > - updated commit message > --- > drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c | 13 - > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c > b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c > index 4c03f0b7343ed..2d23372acd20d 100644 > --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c > +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c > @@ -91,6 +91,7 @@ struct dsi_pll_10nm { > void __iomem *phy_cmn_mmio; > void __iomem *mmio; > > + struct clk *vco_ref_clk; Same comment, and also get the name in probe or pass the clk pointer around instead of storing it forever. > u64 vco_ref_clk_rate; > u64 vco_current_rate; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 16/17] dt-bindings: sun6i-dsi: Add A64 DPHY compatible (w/ A31 fallback)
The MIPI DSI PHY HDMI controller on Allwinner A64 is similar on the one on A31. Add A64 compatible and append A31 compatible as fallback. Signed-off-by: Jagan Teki Reviewed-by: Rob Herring --- Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt index adc7cdf129dd..08f1f57abff5 100644 --- a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt +++ b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt @@ -40,6 +40,7 @@ D-PHY Required properties: - compatible: value must be one of: * allwinner,sun6i-a31-mipi-dphy +* "allwinner,sun50i-a64-mipi-dphy", "allwinner,sun6i-a31-mipi-dphy" - reg: base address and size of memory-mapped region - clocks: phandles to the clocks feeding the DSI encoder * bus: the DSI interface clock -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 11/17] drm/sun4i: sun6i_mipi_dsi: Fix DSI hfp timing value
Current driver is calculating hfp maximum value by subtracting htotal with hsync_end which is front back value, but the hpp refers to front porch. Front porch value is calculating by subtracting hsync_start with hdisplay as per drm_mode timings, and BSP code from BPI-M64-bsp is eventually following the same. BPI-M64-bsp is computing hfp as (from linux-sunxi/ drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) dsi_hbp = (hbp-hspw)*dsi_pixel_bits[format]/8 - (4+2); dsi_hact = x * dsi_pixel_bits[format]/8; dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2); dsi_hfp = dsi_hblk - (4+dsi_hact+2) - (4+dsi_hbp+2); Example, u32 fmt = dsi_pixel_bits[format]/8; => ((ht-hspw)*fmt - 10) - (6 + x * fmt) - (6 + (hbp-hspw)*fmt - 6) => (ht - hspw - x - (hbp - hspw)) * fmt - 16 => (ht - x - hbp) * fmt - 16 => (ht - x - (timmings->hor_total_time - timmings->hor_front_porch - x) * fmt - 16 => (timmings->hor_total_time - x - timmings->hor_total_time + timmings->hor_front_porch + x) * fmt - 16 => timmings->hor_front_porch * fmt - 16 So, update the DSI hfp timing accordingly. Tested on 2-lane, 4-lane MIPI-DSI LCD panels. Signed-off-by: Jagan Teki --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 07eba9ec469b..d8947be92f9d 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -490,7 +490,8 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, */ #define HFP_PACKET_OVERHEAD6 hfp = max((unsigned int)HFP_PACKET_OVERHEAD, - (mode->htotal - mode->hsync_end) * Bpp - HFP_PACKET_OVERHEAD); + (mode->hsync_start - mode->hdisplay) * Bpp - + HFP_PACKET_OVERHEAD); /* * hblk seems to be the line + porches length. -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 08/17] drm/sun4i: sun6i_mipi_dsi: Fix DSI hbp timing value
Current driver is calculating hbp maximum value by subtracting hsync_start with hdisplay which is front porch value, but the hbp refers to back porch. Back porch value is calculating by subtracting htotal with hsync_end as per drm_mode timings, and BSP code from BPI-M64-bsp is eventually following the same. BPI-M64-bsp is computing hbp as (in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) dsi_hbp = (hbp-hspw)*dsi_pixel_bits[format]/8 - (4+2); => (panel->lcd_hbp - timmings->hor_sync_time) => (timmings->hor_back_porch + timmings->hor_sync_time - timmings->hor_sync_time) => timmings->hor_back_porch => mode->htotal - mode->hsync_end So, update the MIPI-DSI hbp value accordingly. Tested on 2-lane, 4-lane DSI LCD panels. Signed-off-by: Jagan Teki --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index cb41fea4f3ee..81151d7633f9 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -482,7 +482,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, */ #define HBP_PACKET_OVERHEAD6 hbp = max((unsigned int)HBP_PACKET_OVERHEAD, - (mode->hsync_start - mode->hdisplay) * Bpp - HBP_PACKET_OVERHEAD); + (mode->htotal - mode->hsync_end) * Bpp - HBP_PACKET_OVERHEAD); /* * The frontporch is set using a blanking packet (4 bytes + -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 14/17] dt-bindings: sun6i-dsi: Add VCC-DSI supply property
Most of the Allwinner MIPI DSI controllers are supply with VCC-DSI pin. which need to supply for some of the boards to trigger the power. So, document the supply property so-that the required board can eable it via device tree. Signed-off-by: Jagan Teki Reviewed-by: Rob Herring --- Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt index 9fa6e7a758ad..adc7cdf129dd 100644 --- a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt +++ b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt @@ -28,6 +28,9 @@ Required properties: first port should be the input endpoint, usually coming from the associated TCON. +Optional properties: + - vcc-dsi-supply: the VCC-DSI power supply of the DSI encoder + Any MIPI-DSI device attached to this should be described according to the bindings defined in ../mipi-dsi-bus.txt -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/tegra: Refactor CEC support
On 12/10/18 5:34 PM, Thierry Reding wrote: > From: Thierry Reding > > Most of the CEC support code already lives in the "output" library code. > Move registration and unregistration to the library code as well to make > use of the same code with HDMI on Tegra210 and later via the SOR. > > Signed-off-by: Thierry Reding Reviewed-by: Hans Verkuil Thanks! Hans > --- > drivers/gpu/drm/tegra/drm.h| 2 +- > drivers/gpu/drm/tegra/hdmi.c | 9 - > drivers/gpu/drm/tegra/output.c | 11 +-- > 3 files changed, 10 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h > index 019862a41cb4..dbc9e11b0aec 100644 > --- a/drivers/gpu/drm/tegra/drm.h > +++ b/drivers/gpu/drm/tegra/drm.h > @@ -132,7 +132,7 @@ struct tegra_output { > struct drm_panel *panel; > struct i2c_adapter *ddc; > const struct edid *edid; > - struct cec_notifier *notifier; > + struct cec_notifier *cec; > unsigned int hpd_irq; > int hpd_gpio; > enum of_gpio_flags hpd_gpio_flags; > diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c > index 0082468f703c..d19973945614 100644 > --- a/drivers/gpu/drm/tegra/hdmi.c > +++ b/drivers/gpu/drm/tegra/hdmi.c > @@ -22,8 +22,6 @@ > > #include > > -#include > - > #include "hdmi.h" > #include "drm.h" > #include "dc.h" > @@ -1709,10 +1707,6 @@ static int tegra_hdmi_probe(struct platform_device > *pdev) > return PTR_ERR(hdmi->vdd); > } > > - hdmi->output.notifier = cec_notifier_get(&pdev->dev); > - if (hdmi->output.notifier == NULL) > - return -ENOMEM; > - > hdmi->output.dev = &pdev->dev; > > err = tegra_output_probe(&hdmi->output); > @@ -1771,9 +1765,6 @@ static int tegra_hdmi_remove(struct platform_device > *pdev) > > tegra_output_remove(&hdmi->output); > > - if (hdmi->output.notifier) > - cec_notifier_put(hdmi->output.notifier); > - > return 0; > } > > diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c > index c662efc7e413..9c2b9dad55c3 100644 > --- a/drivers/gpu/drm/tegra/output.c > +++ b/drivers/gpu/drm/tegra/output.c > @@ -36,7 +36,7 @@ int tegra_output_connector_get_modes(struct drm_connector > *connector) > else if (output->ddc) > edid = drm_get_edid(connector, output->ddc); > > - cec_notifier_set_phys_addr_from_edid(output->notifier, edid); > + cec_notifier_set_phys_addr_from_edid(output->cec, edid); > drm_connector_update_edid_property(connector, edid); > > if (edid) { > @@ -73,7 +73,7 @@ tegra_output_connector_detect(struct drm_connector > *connector, bool force) > } > > if (status != connector_status_connected) > - cec_notifier_phys_addr_invalidate(output->notifier); > + cec_notifier_phys_addr_invalidate(output->cec); > > return status; > } > @@ -174,11 +174,18 @@ int tegra_output_probe(struct tegra_output *output) > disable_irq(output->hpd_irq); > } > > + output->cec = cec_notifier_get(output->dev); > + if (!output->cec) > + return -ENOMEM; > + > return 0; > } > > void tegra_output_remove(struct tegra_output *output) > { > + if (output->cec) > + cec_notifier_put(output->cec); > + > if (gpio_is_valid(output->hpd_gpio)) { > free_irq(output->hpd_irq, output); > gpio_free(output->hpd_gpio); > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 17/17] arm64: dts: allwinner: a64: Add DSI pipeline
The A64 has a MIPI-DSI block which is similar to A31 without mod clock. So, add dsi node with A64 compatible, dphy node with A31 compatible and finally connect dsi to tcon0 to make proper DSI pipeline. Signed-off-by: Jagan Teki --- arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 45 +++ 1 file changed, 45 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi index dd5740bc3fc9..dd5c7ad55149 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi @@ -344,6 +344,12 @@ #address-cells = <1>; #size-cells = <0>; reg = <1>; + + tcon0_out_dsi: endpoint@1 { + reg = <1>; + remote-endpoint = <&dsi_in_tcon0>; + allwinner,tcon-channel = <1>; + }; }; }; }; @@ -910,6 +916,45 @@ status = "disabled"; }; + dsi: dsi@1ca { + compatible = "allwinner,sun50i-a64-mipi-dsi"; + reg = <0x01ca 0x1000>; + interrupts = ; + clocks = <&ccu CLK_BUS_MIPI_DSI>; + clock-names = "bus"; + resets = <&ccu RST_BUS_MIPI_DSI>; + phys = <&dphy>; + phy-names = "dphy"; + status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + + dsi_in_tcon0: endpoint { + remote-endpoint = <&tcon0_out_dsi>; + }; + }; + }; + }; + + dphy: d-phy@1ca1000 { + compatible = "allwinner,sun50i-a64-mipi-dphy", +"allwinner,sun6i-a31-mipi-dphy"; + reg = <0x01ca1000 0x1000>; + clocks = <&ccu CLK_BUS_MIPI_DSI>, +<&ccu CLK_DSI_DPHY>; + clock-names = "bus", "mod"; + resets = <&ccu RST_BUS_MIPI_DSI>; + status = "disabled"; + #phy-cells = <0>; + }; + csi: csi@1cb { compatible = "allwinner,sun50i-a64-csi"; reg = <0x01cb 0x1000>; -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 02/17] drm/sun4i: sun6i_mipi_dsi: Add has_mod_clk quirk
Mod clock is not mandatory for all Allwinner MIPI DSI controllers, it is connected as CLK_DSI_SCLK for A31 and not available in A64. So add has_mod_clk quirk and process the clk accordingly. Signed-off-by: Jagan Teki --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 39 ++ drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 5 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index e3b34a345546..561de393ea23 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -981,6 +982,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev) dsi->host.ops = &sun6i_dsi_host_ops; dsi->host.dev = dev; + dsi->variant = of_device_get_match_data(dev); + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); base = devm_ioremap_resource(dev, res); if (IS_ERR(base)) { @@ -1001,17 +1004,20 @@ static int sun6i_dsi_probe(struct platform_device *pdev) return PTR_ERR(dsi->reset); } - dsi->mod_clk = devm_clk_get(dev, "mod"); - if (IS_ERR(dsi->mod_clk)) { - dev_err(dev, "Couldn't get the DSI mod clock\n"); - return PTR_ERR(dsi->mod_clk); + if (dsi->variant->has_mod_clk) { + dsi->mod_clk = devm_clk_get(dev, "mod"); + if (IS_ERR(dsi->mod_clk)) { + dev_err(dev, "Couldn't get the DSI mod clock\n"); + return PTR_ERR(dsi->mod_clk); + } } /* * In order to operate properly, that clock seems to be always * set to 297MHz. */ - clk_set_rate_exclusive(dsi->mod_clk, 29700); + if (dsi->variant->has_mod_clk) + clk_set_rate_exclusive(dsi->mod_clk, 29700); dphy_node = of_parse_phandle(dev->of_node, "phys", 0); ret = sun6i_dphy_probe(dsi, dphy_node); @@ -1043,7 +1049,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev) pm_runtime_disable(dev); sun6i_dphy_remove(dsi); err_unprotect_clk: - clk_rate_exclusive_put(dsi->mod_clk); + if (dsi->variant->has_mod_clk) + clk_rate_exclusive_put(dsi->mod_clk); return ret; } @@ -1056,7 +1063,8 @@ static int sun6i_dsi_remove(struct platform_device *pdev) mipi_dsi_host_unregister(&dsi->host); pm_runtime_disable(dev); sun6i_dphy_remove(dsi); - clk_rate_exclusive_put(dsi->mod_clk); + if (dsi->variant->has_mod_clk) + clk_rate_exclusive_put(dsi->mod_clk); return 0; } @@ -1066,7 +1074,8 @@ static int __maybe_unused sun6i_dsi_runtime_resume(struct device *dev) struct sun6i_dsi *dsi = dev_get_drvdata(dev); reset_control_deassert(dsi->reset); - clk_prepare_enable(dsi->mod_clk); + if (dsi->variant->has_mod_clk) + clk_prepare_enable(dsi->mod_clk); /* * Enable the DSI block. @@ -1094,7 +1103,8 @@ static int __maybe_unused sun6i_dsi_runtime_suspend(struct device *dev) { struct sun6i_dsi *dsi = dev_get_drvdata(dev); - clk_disable_unprepare(dsi->mod_clk); + if (dsi->variant->has_mod_clk) + clk_disable_unprepare(dsi->mod_clk); reset_control_assert(dsi->reset); return 0; @@ -1106,9 +1116,16 @@ static const struct dev_pm_ops sun6i_dsi_pm_ops = { NULL) }; +static const struct sun6i_dsi_variant sun6i_a31_dsi = { + .has_mod_clk = true, +}; + static const struct of_device_id sun6i_dsi_of_table[] = { - { .compatible = "allwinner,sun6i-a31-mipi-dsi" }, - { } + { + .compatible = "allwinner,sun6i-a31-mipi-dsi", + .data = &sun6i_a31_dsi, + }, + { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table); diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h index dbbc5b3ecbda..597b62227019 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h @@ -20,6 +20,10 @@ struct sun6i_dphy { struct reset_control*reset; }; +struct sun6i_dsi_variant { + boolhas_mod_clk; +}; + struct sun6i_dsi { struct drm_connectorconnector; struct drm_encoder encoder; @@ -35,6 +39,7 @@ struct sun6i_dsi { struct sun4i_drv*drv; struct mipi_dsi_device *device; struct drm_panel*panel; + const struct sun6i_dsi_variant *variant; }; static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host) -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-
Re: TK1: DRM, Nouveau and VIC
Hi Thierry On Mon, 2018-12-10 at 12:00 +0100, Thierry Reding wrote: > On Mon, Dec 10, 2018 at 11:21:47AM +0100, Thierry Reding wrote: > > On Sat, Dec 08, 2018 at 02:54:45PM +, Marcel Ziswiler wrote: > > > Hi Thierry et al. > > > > > > I noticed that since commit 3dde5a2342cd ("ARM: tegra: Add VIC on > > > Tegra124") graphics on Apalis TK1 is broken. During boot it fails > > > loading the vic firmware: > > > > > > [1.595824] tegra-vic 5434.vic: Direct firmware load for > > > nvidia/tegra124/vic03_ucode.bin failed with error -2 > > > [1.606140] tegra-vic: probe of 5434.vic failed with error > > > -2 > > > > > > Subsequently Tegra HDMI seems to fail completely: > > > > > > [2.379860] tegra-hdmi 5428.hdmi: failed to get PLL > > > regulator > > > > > > And finally, Nouveau even crashes: > > > > > > [8.241115] nouveau 5700.gpu: Linked as a consumer to > > > regulator.31 > > > [8.247889] nouveau 5700.gpu: NVIDIA GK20A (0ea000a1) > > > [8.253396] nouveau 5700.gpu: imem: using IOMMU > > > [8.270210] Unable to handle kernel NULL pointer dereference > > > at > > > virtual address 006c > > > [8.278340] pgd = (ptrval) > > > [8.281250] [006c] *pgd= > > > [8.284944] Internal error: Oops: 5 [#1] PREEMPT SMP ARM > > > [8.290260] Modules linked in: nouveau(+) ttm > > > [8.294625] CPU: 2 PID: 203 Comm: systemd-udevd Not tainted > > > 4.20.0- > > > rc5-next-20181207-8-g85b0f8e25f86-dirty #110 > > > [8.305055] Hardware name: NVIDIA Tegra SoC (Flattened Device > > > Tree) > > > [8.311331] PC is at drm_plane_register_all+0x18/0x50 > > > [8.316373] LR is at drm_modeset_register_all+0xc/0x70 > > > [8.321513] pc : []lr : []psr: > > > a0060013 > > > [8.327768] sp : ed527c70 ip : ecc43ec0 fp : > > > [8.332993] r10: 0016 r9 : ecc43e80 r8 : > > > [8.338209] r7 : bf182c80 r6 : r5 : ed61b24c r4 : > > > fffc > > > [8.344735] r3 : 0002f000 r2 : r1 : 2e124000 r0 : > > > ed61b000 > > > [8.351260] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA > > > ARM Segment none > > > [8.358383] Control: 10c5387d Table: ad64c06a DAC: 0051 > > > [8.364127] Process systemd-udevd (pid: 203, stack limit = > > > 0x(ptrval)) > > > [8.370654] Stack: (0xed527c70 to 0xed528000) > > > [8.375004] 7c60: ed61b000 > > > ed61b000 c0564cc8 > > > [8.383177] 7c80: ed61b000 c054b5b8 0001 > > > 0001 > > > [8.391355] 7ca0: ed527cc0 c0f08c48 ed61b000 > > > bf180c5c bf0dc900 > > > [8.399531] 7cc0: eda29208 5dfe844b ee9f2a10 > > > bf180c5c c05a9328 > > > [8.407695] 7ce0: c1006828 ee9f2a10 c100682c > > > c05a744c ee9f2a10 bf180c5c > > > [8.415871] 7d00: ee9f2a44 c05a77a8 c0f08c48 bf182980 > > > c05a769c eefd14d0 c05a77a8 > > > [8.424048] 7d20: ee9f2a10 bf180c5c ee9f2a44 c05a77a8 > > > c0f08c48 bf182980 > > > [8.432226] 7d40: c05a7884 ee9ebfb4 c0f08c48 bf180c5c > > > c05a5790 ee88135c > > > [8.440405] 7d60: ee9ebfb4 5dfe844b c0f71168 bf180c5c ee379e80 > > > c0f71168 c05a692c > > > [8.448570] 7d80: bf15dc00 bf180ac8 e000 bf180c5c bf180ac8 > > > e000 bf1aa000 c05a84a0 > > > [8.456746] 7da0: bf182b80 bf180ac8 e000 bf1aa170 c0fbd220 > > > c0f08c48 e000 c0102ed0 > > > [8.464924] 7dc0: ed53f4c0 006000c0 c01b3d98 000c 6113 > > > bf182980 0040 c02592d0 > > > [8.473102] 7de0: eda60200 2e124000 ee80 006000c0 006000c0 > > > c01b3d98 000c c025a8cc > > > [8.481281] 7e00: c024ce54 a113 bf182980 5dfe844b bf182980 > > > 0002 ed53f4c0 0002 > > > [8.489459] 7e20: eceba000 c01b3dd4 c0f08c48 bf182980 > > > ed527f40 0002 eceb9fc0 > > > [8.497625] 7e40: 0002 c01b61a4 bf18298c 7fff bf182980 > > > c01b2f88 c01b279c > > > [8.505800] 7e60: bf1829c8 bf182a80 bf182b6c bf182ab0 c0b03ab0 > > > c0d58964 c0ca726c c0ca7278 > > > [8.513978] 7e80: c0ca72d0 c0f08c48 c02654a0 > > > e000 bf00 > > > [8.522157] 7ea0: > > > 6e72656b 6c65 > > > [8.530336] 7ec0: > > > > > > [8.538502] 7ee0: > > > 5dfe844b 7fff c0f08c48 > > > [8.546677] 7f00: 000f b6f761cc c0101204 ed526000 > > > 017b 004a3270 c01b66a4 > > > [8.554855] 7f20: 7fff 0003 0001 004a3270 > > > f0ced000 06e8994c > > > [8.563032] 7f40: f0e37f3a f0e50a40 f0ced000 06e8994c f7b75f9c > > > f7b75d34 f63e62dc 0016b000 > > > [8.571209] 7f60: 0017f6f0 00050a48 > > > 003b 003c
Re: [PATCH v1 8/9] drm/doc: Add initial komeda driver documentation
On 12/10/18 3:47 AM, james qian wang (Arm Technology China) wrote: > On Wed, Dec 05, 2018 at 06:08:38PM -0800, Randy Dunlap wrote: >> On 12/5/18 2:25 AM, james qian wang (Arm Technology China) wrote: >>> Signed-off-by: James (Qian) Wang >>> --- >>> Documentation/gpu/drivers.rst| 1 + >>> Documentation/gpu/komeda-kms.rst | 483 +++ >>> 2 files changed, 484 insertions(+) >>> create mode 100644 Documentation/gpu/komeda-kms.rst >> >> Hi, >> >> I have some editing changes for you to consider, although I did have a few >> problems with reading the text. >> > > Thank you Randy, I'll correct the doc according to your comments in the next > version. > Hi James, >>> diff --git a/Documentation/gpu/komeda-kms.rst >>> b/Documentation/gpu/komeda-kms.rst >>> new file mode 100644 >>> index ..8af925ca0869 >>> --- /dev/null >>> +++ b/Documentation/gpu/komeda-kms.rst >>> @@ -0,0 +1,483 @@ >>> +.. SPDX-License-Identifier: GPL-2.0 >>> + >>> +== >>> + drm/komeda ARM display driver >>> +== >>> + >>> +The drm/komeda driver supports for the ARM display processor D71 and later >> >> driver supports the ARM >> >>> +IPs, this document is for giving a brief overview of driver design: how it >> >>IPs. This document gives a brief overview of the driver design: >> >> (although I hate using "IPs" like that.) > > How about "Product" or "Chipset" Yes, much better IMO. >>> +works and why design it like that. >>> + >>> +Overview of D71 like display IPs >>> + >>> + >>> +From D71, Arm display IP begins to adopt a flexible and modularized >> >> ARM > > Sorry my fault, I used "ARM" in the initial lines which misleads you, but > after > Arm reband last year, the "Arm" is the right version. > > And I'll unify all the usage to "Arm" in the next version. I thought that you would know more about that than I do. [snip] >> So above, I tried to write what I thought you meant, but saying that >> Layer_Split >> needs to be handled in user mode ... and then saying that if it is handled >> in the >> kernel, it would be smooth and simple -- that seems to be contradictory to >> me, >> so I guess I didn't understand it very well. >> > > Sorry, I need to reorganize my sentence. maybe like below: > > Layer_Split is quite complicated feature, which splits a big image into > two parts and handle it by two layers and scalers individually. handles > But it imports a edge problem or effect in the middle of image after the > split. an middle of the image > To avoid such problem, need a complicated Split calculation and some special such a problem, it needs a complicated > configurationes to the layer and scaler. configurations > We'd better hide such HW related complexity to user mode. thanks, -- ~Randy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1.1 04/29] drm/omap: Use atomic suspend/resume helpers
Hi, On Mon, Dec 10, 2018 at 10:01:19AM +0200, Laurent Pinchart wrote: > Instead of rolling out custom suspend/resume implementations based on > state information stored in the driver's data structures, use the atomic > suspend/resume helpers that rely on a DRM atomic state object. > > Signed-off-by: Laurent Pinchart > --- Reviewed-by: Sebastian Reichel -- Sebastian > Changes since v1: > > - Use drm_mode_config_helper_suspend() > --- > drivers/gpu/drm/omapdrm/dss/omapdss.h | 3 -- > drivers/gpu/drm/omapdrm/omap_drv.c| 50 ++- > 2 files changed, 2 insertions(+), 51 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h > b/drivers/gpu/drm/omapdrm/dss/omapdss.h > index d3b576e6edf5..149a0f09adbc 100644 > --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h > +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h > @@ -432,9 +432,6 @@ struct omap_dss_device { > unsigned long ops_flags; > unsigned long bus_flags; > > - /* helper variable for driver suspend/resume */ > - bool activate_after_resume; > - > enum omap_display_caps caps; > > enum omap_dss_display_state state; > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c > b/drivers/gpu/drm/omapdrm/omap_drv.c > index 5e67d58cbc28..8a4dd737d88c 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > @@ -685,54 +685,12 @@ static int pdev_remove(struct platform_device *pdev) > } > > #ifdef CONFIG_PM_SLEEP > -static int omap_drm_suspend_all_displays(struct drm_device *ddev) > -{ > - struct omap_drm_private *priv = ddev->dev_private; > - int i; > - > - for (i = 0; i < priv->num_pipes; i++) { > - struct omap_dss_device *display = priv->pipes[i].display; > - > - if (display->state == OMAP_DSS_DISPLAY_ACTIVE) { > - display->ops->disable(display); > - display->activate_after_resume = true; > - } else { > - display->activate_after_resume = false; > - } > - } > - > - return 0; > -} > - > -static int omap_drm_resume_all_displays(struct drm_device *ddev) > -{ > - struct omap_drm_private *priv = ddev->dev_private; > - int i; > - > - for (i = 0; i < priv->num_pipes; i++) { > - struct omap_dss_device *display = priv->pipes[i].display; > - > - if (display->activate_after_resume) { > - display->ops->enable(display); > - display->activate_after_resume = false; > - } > - } > - > - return 0; > -} > - > static int omap_drm_suspend(struct device *dev) > { > struct omap_drm_private *priv = dev_get_drvdata(dev); > struct drm_device *drm_dev = priv->ddev; > > - drm_kms_helper_poll_disable(drm_dev); > - > - drm_modeset_lock_all(drm_dev); > - omap_drm_suspend_all_displays(drm_dev); > - drm_modeset_unlock_all(drm_dev); > - > - return 0; > + return drm_mode_config_helper_suspend(drm_dev); > } > > static int omap_drm_resume(struct device *dev) > @@ -740,11 +698,7 @@ static int omap_drm_resume(struct device *dev) > struct omap_drm_private *priv = dev_get_drvdata(dev); > struct drm_device *drm_dev = priv->ddev; > > - drm_modeset_lock_all(drm_dev); > - omap_drm_resume_all_displays(drm_dev); > - drm_modeset_unlock_all(drm_dev); > - > - drm_kms_helper_poll_enable(drm_dev); > + drm_mode_config_helper_resume(drm_dev); > > return omap_gem_resume(drm_dev); > } > -- > Regards, > > Laurent Pinchart > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: TK1: DRM, Nouveau and VIC
On 10.12.2018 13:21, Thierry Reding wrote: > On Sat, Dec 08, 2018 at 02:54:45PM +, Marcel Ziswiler wrote: >> Hi Thierry et al. >> >> I noticed that since commit 3dde5a2342cd ("ARM: tegra: Add VIC on >> Tegra124") graphics on Apalis TK1 is broken. During boot it fails >> loading the vic firmware: >> >> [1.595824] tegra-vic 5434.vic: Direct firmware load for >> nvidia/tegra124/vic03_ucode.bin failed with error -2 >> [1.606140] tegra-vic: probe of 5434.vic failed with error -2 >> >> Subsequently Tegra HDMI seems to fail completely: >> >> [2.379860] tegra-hdmi 5428.hdmi: failed to get PLL regulator >> >> And finally, Nouveau even crashes: >> >> [8.241115] nouveau 5700.gpu: Linked as a consumer to >> regulator.31 >> [8.247889] nouveau 5700.gpu: NVIDIA GK20A (0ea000a1) >> [8.253396] nouveau 5700.gpu: imem: using IOMMU >> [8.270210] Unable to handle kernel NULL pointer dereference at >> virtual address 006c >> [8.278340] pgd = (ptrval) >> [8.281250] [006c] *pgd= >> [8.284944] Internal error: Oops: 5 [#1] PREEMPT SMP ARM >> [8.290260] Modules linked in: nouveau(+) ttm >> [8.294625] CPU: 2 PID: 203 Comm: systemd-udevd Not tainted 4.20.0- >> rc5-next-20181207-8-g85b0f8e25f86-dirty #110 >> [8.305055] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) >> [8.311331] PC is at drm_plane_register_all+0x18/0x50 >> [8.316373] LR is at drm_modeset_register_all+0xc/0x70 >> [8.321513] pc : []lr : []psr: a0060013 >> [8.327768] sp : ed527c70 ip : ecc43ec0 fp : >> [8.332993] r10: 0016 r9 : ecc43e80 r8 : >> [8.338209] r7 : bf182c80 r6 : r5 : ed61b24c r4 : >> fffc >> [8.344735] r3 : 0002f000 r2 : r1 : 2e124000 r0 : >> ed61b000 >> [8.351260] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA >> ARM Segment none >> [8.358383] Control: 10c5387d Table: ad64c06a DAC: 0051 >> [8.364127] Process systemd-udevd (pid: 203, stack limit = >> 0x(ptrval)) >> [8.370654] Stack: (0xed527c70 to 0xed528000) >> [8.375004] 7c60: ed61b000 >> ed61b000 c0564cc8 >> [8.383177] 7c80: ed61b000 c054b5b8 0001 >> 0001 >> [8.391355] 7ca0: ed527cc0 c0f08c48 ed61b000 >> bf180c5c bf0dc900 >> [8.399531] 7cc0: eda29208 5dfe844b ee9f2a10 >> bf180c5c c05a9328 >> [8.407695] 7ce0: c1006828 ee9f2a10 c100682c >> c05a744c ee9f2a10 bf180c5c >> [8.415871] 7d00: ee9f2a44 c05a77a8 c0f08c48 bf182980 >> c05a769c eefd14d0 c05a77a8 >> [8.424048] 7d20: ee9f2a10 bf180c5c ee9f2a44 c05a77a8 >> c0f08c48 bf182980 >> [8.432226] 7d40: c05a7884 ee9ebfb4 c0f08c48 bf180c5c >> c05a5790 ee88135c >> [8.440405] 7d60: ee9ebfb4 5dfe844b c0f71168 bf180c5c ee379e80 >> c0f71168 c05a692c >> [8.448570] 7d80: bf15dc00 bf180ac8 e000 bf180c5c bf180ac8 >> e000 bf1aa000 c05a84a0 >> [8.456746] 7da0: bf182b80 bf180ac8 e000 bf1aa170 c0fbd220 >> c0f08c48 e000 c0102ed0 >> [8.464924] 7dc0: ed53f4c0 006000c0 c01b3d98 000c 6113 >> bf182980 0040 c02592d0 >> [8.473102] 7de0: eda60200 2e124000 ee80 006000c0 006000c0 >> c01b3d98 000c c025a8cc >> [8.481281] 7e00: c024ce54 a113 bf182980 5dfe844b bf182980 >> 0002 ed53f4c0 0002 >> [8.489459] 7e20: eceba000 c01b3dd4 c0f08c48 bf182980 >> ed527f40 0002 eceb9fc0 >> [8.497625] 7e40: 0002 c01b61a4 bf18298c 7fff bf182980 >> c01b2f88 c01b279c >> [8.505800] 7e60: bf1829c8 bf182a80 bf182b6c bf182ab0 c0b03ab0 >> c0d58964 c0ca726c c0ca7278 >> [8.513978] 7e80: c0ca72d0 c0f08c48 c02654a0 >> e000 bf00 >> [8.522157] 7ea0: >> 6e72656b 6c65 >> [8.530336] 7ec0: >> >> [8.538502] 7ee0: >> 5dfe844b 7fff c0f08c48 >> [8.546677] 7f00: 000f b6f761cc c0101204 ed526000 >> 017b 004a3270 c01b66a4 >> [8.554855] 7f20: 7fff 0003 0001 004a3270 >> f0ced000 06e8994c >> [8.563032] 7f40: f0e37f3a f0e50a40 f0ced000 06e8994c f7b75f9c >> f7b75d34 f63e62dc 0016b000 >> [8.571209] 7f60: 0017f6f0 00050a48 >> 003b 003c 0023 >> [8.579388] 7f80: 0014 5dfe844b >> 004c0ec0 0001 >> [8.587554] 7fa0: 017b c0101000 004c0ec0 000f >> b6f761cc 0002 >> [8.595730] 7fc0: 004c0ec0 0001 017b 0048e114 >> 004a3270 >> [8.603908] 7fe0: bea8f990 bea8f980 b6f71269 b6e9f6c0 400d0010 >> 000f >> [8.612096]
Re: [PATCH v4 4/8] drm/msm/dsi: 14nm PHY: Get ref clock from the DT
Quoting Matthias Kaehlcke (2018-12-04 14:42:30) > diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c > b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c > index 71fe60e5f01f1..032bf3e8614bd 100644 > --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c > +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c > @@ -40,7 +40,6 @@ > > #define NUM_PROVIDED_CLKS 2 > > -#define VCO_REF_CLK_RATE 1920 > #define VCO_MIN_RATE 13UL > #define VCO_MAX_RATE 26UL > > @@ -139,6 +138,7 @@ struct dsi_pll_14nm { > /* protects REG_DSI_14nm_PHY_CMN_CLK_CFG0 register */ > spinlock_t postdiv_lock; > > + struct clk *vco_ref_clk; Is there any need to keep it in the struct? Or just get the clk, find the rate, and then put the clk and call pll_14nm_postdiv_register()? > u64 vco_current_rate; > u64 vco_ref_clk_rate; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 10/17] drm/sun4i: sun6i_mipi_dsi: Add DSI hblk packet overhead
Add 10 bytes packet overhead for hblk where blank is set using a blanking packet like (4 bytes + 4 bytes + payload + 2 bytes) This is according to BSP code from BPI-M64-bsp (from linux-sunxi/ drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2); So, add 10 bytes packet overhead for DSI hblk. Tested on 2-lane, 4-lane MIPI-DSI LCD panels. Signed-off-by: Jagan Teki --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 4c95b3384ed9..07eba9ec469b 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -494,8 +494,13 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, /* * hblk seems to be the line + porches length. +* The blank is set using a blanking packet (4 bytes + 4 bytes + +* payload + 2 bytes). So minimal size is 10 bytes */ - hblk = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp; +#define HBLK_PACKET_OVERHEAD 10 + hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, + (mode->htotal - (mode->hsync_end - mode->hsync_start)) * + Bpp - HBLK_PACKET_OVERHEAD); /* * And I'm not entirely sure what vblk is about. The driver in -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 22/29] drm/omap: Move DISPC timing checks to CRTC .mode_valid() operation
Hi, On Mon, Dec 10, 2018 at 02:14:01PM +0100, Sebastian Reichel wrote: > On Mon, Dec 10, 2018 at 09:50:08AM +0200, Laurent Pinchart wrote: > > Hi Sebastian, > > > > On Monday, 10 December 2018 00:07:55 EET Sebastian Reichel wrote: > > > On Wed, Dec 05, 2018 at 05:00:15PM +0200, Laurent Pinchart wrote: > > > > The DISPC timings checks relate to the CRTC, but they're performed in > > > > the encoder and connector .atomic_check() and .mode_valid() operations. > > > > Move them to the CRTC .mode_valid() operation. > > > > > > > > Signed-off-by: Laurent Pinchart > > > > --- > > > > > > Reviewed-by: Sebastian Reichel > > > > > > This should also fix the issue with DSI in a less ugly way: > > > > > > https://lists.freedesktop.org/archives/dri-devel/2018-November/196622.html > > > > Should I add the > > > > Fixes: 7c27fa57ef31 ("drm/omap: Call dispc timings check operation > > directly") > > > > tag to this patch ? > > I plan to test this series in combination with my DSI patches today > or tomorrow evening. If it works, probably the following should be > added to the patch description: > > > As a side-effect this fixes DSI, which uses slightly different mode > configuration for the DSI encoder and the DISPC. > > Fixes: 7c27fa57ef31 ("drm/omap: Call dispc timings check operation directly") > I merged this series with my remaining DSI command mode patches (btw. this series does not apply cleanly to master due to a recent patch from Tomi). Unfortunately this patch does not fix the issue. After this patch the mgr_check_timings() call in omap_crtc_mode_valid() will fail. The good news: The Droid 4 display worked again after dropping the mgr_check_timings call as a quick hack. So the other patches from this set are Tested-by: Sebastian Reichel The problem is, that DISPC timings for DSI are not directly calculated from mode. Instead DSI code does some modifications before applying the DISPC settings. -- Sebastian signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 07/17] drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay
Video start delay can be computed by subtracting total vertical timing with front porch timing and with adding 1 delay line for TCON. BSP code form BPI-M64-bsp is computing video start delay as (from linux-sunxi/ drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) u32 vfp = panel->lcd_vt - panel->lcd_y - panel->lcd_vbp; => (panel->lcd_vt) - panel->lcd_y - (panel->lcd_vbp) => (timmings->ver_front_porch + panel->lcd_vbp + panel->lcd_y) - panel->lcd_y - (panel->lcd_vbp) => timmings->ver_front_porch + panel->lcd_vbp + panel->lcd_y - panel->lcd_y - panel->lcd_vbp => timmings->ver_front_porch So, update the start delay computation accordingly. Signed-off-by: Jagan Teki --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index c9b0222ebcd4..cb41fea4f3ee 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -358,7 +358,17 @@ static void sun6i_dsi_inst_init(struct sun6i_dsi *dsi, static u16 sun6i_dsi_get_video_start_delay(struct sun6i_dsi *dsi, struct drm_display_mode *mode) { - return mode->vtotal - (mode->vsync_end - mode->vdisplay) + 1; + u32 vfp = mode->vsync_start - mode->vdisplay; + u32 start_delay; + + start_delay = mode->vtotal - vfp + 1; + if (start_delay > mode->vtotal) + start_delay -= mode->vtotal; + + if (!start_delay) + start_delay = 1; + + return start_delay; } static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi, -- 2.18.0.321.gffc6fa0e3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/display: Pass app_tf by value rather than by reference
Clang warns when an expression that equals zero is used as a null pointer constant (in lieu of NULL): drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4435:3: warning: expression which evaluates to zero treated as a null pointer constant of type 'const enum color_transfer_func *' [-Wnon-literal-null-conversion] TRANSFER_FUNC_UNKNOWN, ^ 1 warning generated. This warning is caused by commit bb47de736661 ("drm/amdgpu: Set FreeSync state using drm VRR properties") and it could be solved by using NULL instead of TRANSFER_FUNC_UNKNOWN or casting TRANSFER_FUNC_UNKNOWN as a pointer. However, after looking into it, there doesn't appear to be a good reason to pass app_tf by reference as it is never mutated along the way. This is the only code path in which app_tf is used: mod_freesync_build_vrr_infopacket -> build_vrr_infopacket_v2 -> build_vrr_infopacket_fs2_data Neither mod_freesync_build_vrr_infopacket or build_vrr_infopacket_v2 modify app_tf's value and build_vrr_infopacket_fs2_data expects just the value so we can avoid dereferencing anything by just passing in app_tf's value to mod_freesync_build_vrr_infopacket and build_vrr_infopacket_v2. There is no functional change because build_vrr_infopacket_fs2_data doesn't do anything if TRANSFER_FUNC_UNKNOWN is passed to it, the same as not calling build_vrr_infopacket_fs2_data at all like before this change when NULL was used for app_tf. Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/amd/display/modules/freesync/freesync.c | 7 +++ drivers/gpu/drm/amd/display/modules/inc/mod_freesync.h | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c index 620a171620ee..520665a9d81a 100644 --- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c +++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c @@ -656,7 +656,7 @@ static void build_vrr_infopacket_v1(enum signal_type signal, static void build_vrr_infopacket_v2(enum signal_type signal, const struct mod_vrr_params *vrr, - const enum color_transfer_func *app_tf, + enum color_transfer_func app_tf, struct dc_info_packet *infopacket) { unsigned int payload_size = 0; @@ -664,8 +664,7 @@ static void build_vrr_infopacket_v2(enum signal_type signal, build_vrr_infopacket_header_v2(signal, infopacket, &payload_size); build_vrr_infopacket_data(vrr, infopacket); - if (app_tf != NULL) - build_vrr_infopacket_fs2_data(*app_tf, infopacket); + build_vrr_infopacket_fs2_data(app_tf, infopacket); build_vrr_infopacket_checksum(&payload_size, infopacket); @@ -676,7 +675,7 @@ void mod_freesync_build_vrr_infopacket(struct mod_freesync *mod_freesync, const struct dc_stream_state *stream, const struct mod_vrr_params *vrr, enum vrr_packet_type packet_type, - const enum color_transfer_func *app_tf, + enum color_transfer_func app_tf, struct dc_info_packet *infopacket) { /* SPD info packet for FreeSync */ diff --git a/drivers/gpu/drm/amd/display/modules/inc/mod_freesync.h b/drivers/gpu/drm/amd/display/modules/inc/mod_freesync.h index 949a8b62aa98..063af6258fd9 100644 --- a/drivers/gpu/drm/amd/display/modules/inc/mod_freesync.h +++ b/drivers/gpu/drm/amd/display/modules/inc/mod_freesync.h @@ -145,7 +145,7 @@ void mod_freesync_build_vrr_infopacket(struct mod_freesync *mod_freesync, const struct dc_stream_state *stream, const struct mod_vrr_params *vrr, enum vrr_packet_type packet_type, - const enum color_transfer_func *app_tf, + enum color_transfer_func app_tf, struct dc_info_packet *infopacket); void mod_freesync_build_vrr_params(struct mod_freesync *mod_freesync, -- 2.20.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/meson: remove firmware framebuffers
In case we are using simplefb or another conflicting framebuffer, make the call to drm_fb_helper_remove_conflicting_framebuffers() Signed-off-by: Maxime Jourdan --- drivers/gpu/drm/meson/meson_drv.c | 19 +++ 1 file changed, 19 insertions(+) diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c index d3443125e661..82fbd7ae80e4 100644 --- a/drivers/gpu/drm/meson/meson_drv.c +++ b/drivers/gpu/drm/meson/meson_drv.c @@ -159,6 +159,22 @@ static void meson_vpu_init(struct meson_drm *priv) writel_relaxed(0x2, priv->io_base + _REG(VPU_WRARB_MODE_L2C1)); } +static void meson_remove_framebuffers(void) +{ +struct apertures_struct *ap; + +ap = alloc_apertures(1); +if (!ap) +return; + +/* The framebuffer can be located anywhere in RAM */ +ap->ranges[0].base = 0; +ap->ranges[0].size = ~0; + +drm_fb_helper_remove_conflicting_framebuffers(ap, "meson-drm-fb", false); +kfree(ap); +} + static int meson_drv_bind_master(struct device *dev, bool has_components) { struct platform_device *pdev = to_platform_device(dev); @@ -242,6 +258,9 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) if (ret) goto free_drm; + /* Remove early framebuffers (ie. simplefb) */ + meson_remove_framebuffers(); + drm_mode_config_init(drm); drm->mode_config.max_width = 3840; drm->mode_config.max_height = 2160; -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] clk: meson: meson8b: add the GPU clock tree
On 08/12/2018 18:12, Martin Blumenstingl wrote: > Add the GPU clock tree on Meson8, Meson8b and Meson8m2. > > The GPU clock tree on Meson8b and Meson8m2 is almost identical to the > one one GXBB: > - there's a glitch-free mux at HHI_MALI_CLK_CNTL[31] > - there are two identical parents for this mux: mali_0 and mali_1, each > with a gate, divider and mux > - the parents of mali_0_sel and mali_1_sel are identical to GXBB except > there's no GP0_PLL on these 32-bit SoCs > > Meson8 is different because it does not have the glitch-free mux. > Instead if only has the mali_0 clock tree. The parents of mali_0_sel are > identical to the ones on Meson8b and Meson8m2. > > Signed-off-by: Martin Blumenstingl > --- > drivers/clk/meson/meson8b.c | 146 > drivers/clk/meson/meson8b.h | 9 ++- > 2 files changed, 154 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > index 0b9353d8d4fd..748552c5f6c8 100644 > --- a/drivers/clk/meson/meson8b.c > +++ b/drivers/clk/meson/meson8b.c > @@ -1573,6 +1573,135 @@ static struct clk_regmap meson8b_hdmi_sys = { > }, > }; > > +/* > + * The MALI IP is clocked by two identical clocks (mali_0 and mali_1) > + * muxed by a glitch-free switch on Meson8b and Meson8m2. Meson8 only > + * has mali_0 and no glitch-free mux. > + */ > +static const char * const meson8b_mali_0_1_parent_names[] = { > + "xtal", "mpll2", "mpll1", "fclk_div7", "fclk_div4", "fclk_div3", > + "fclk_div5" > +}; > + > +static u32 meson8b_mali_0_1_mux_table[] = { 0, 2, 3, 4, 5, 6, 7 }; > + > +static struct clk_regmap meson8b_mali_0_sel = { > + .data = &(struct clk_regmap_mux_data){ > + .offset = HHI_MALI_CLK_CNTL, > + .mask = 0x7, > + .shift = 9, > + .table = meson8b_mali_0_1_mux_table, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "mali_0_sel", > + .ops = &clk_regmap_mux_ops, > + .parent_names = meson8b_mali_0_1_parent_names, > + .num_parents = ARRAY_SIZE(meson8b_mali_0_1_parent_names), > + /* > + * Don't propagate rate changes up because the only changeable > + * parents are mpll1 and mpll2 but we need those for audio and > + * RGMII (Ethernet). We don't want to change the audio or > + * Ethernet clocks when setting the GPU frequency. > + */ > + .flags = 0, > + }, > +}; > + > +static struct clk_regmap meson8b_mali_0_div = { > + .data = &(struct clk_regmap_div_data){ > + .offset = HHI_MALI_CLK_CNTL, > + .shift = 0, > + .width = 7, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "mali_0_div", > + .ops = &clk_regmap_divider_ops, > + .parent_names = (const char *[]){ "mali_0_sel" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_mali_0 = { > + .data = &(struct clk_regmap_gate_data){ > + .offset = HHI_MALI_CLK_CNTL, > + .bit_idx = 8, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "mali_0", > + .ops = &clk_regmap_gate_ops, > + .parent_names = (const char *[]){ "mali_0_div" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_mali_1_sel = { > + .data = &(struct clk_regmap_mux_data){ > + .offset = HHI_MALI_CLK_CNTL, > + .mask = 0x7, > + .shift = 25, > + .table = meson8b_mali_0_1_mux_table, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "mali_1_sel", > + .ops = &clk_regmap_mux_ops, > + .parent_names = meson8b_mali_0_1_parent_names, > + .num_parents = ARRAY_SIZE(meson8b_mali_0_1_parent_names), > + /* > + * Don't propagate rate changes up because the only changeable > + * parents are mpll1 and mpll2 but we need those for audio and > + * RGMII (Ethernet). We don't want to change the audio or > + * Ethernet clocks when setting the GPU frequency. > + */ > + .flags = 0, > + }, > +}; > + > +static struct clk_regmap meson8b_mali_1_div = { > + .data = &(struct clk_regmap_div_data){ > + .offset = HHI_MALI_CLK_CNTL, > + .shift = 16, > + .width = 7, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "mali_1_div", > + .ops = &clk_regmap_divider_ops, > + .parent_names = (const char *[]){ "mali_1_sel" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_mali_1 = { > + .data = &(struct
Re: [PATCH] host1x: cdma: use completion instead of semaphore
On Mon, Dec 10, 2018 at 10:51:04PM +0100, Arnd Bergmann wrote: > In this usage, the two are completely equivalent, but the > completion documents better what is going on, and we generally > try to avoid semaphores these days. > > Signed-off-by: Arnd Bergmann > --- > drivers/gpu/host1x/cdma.c | 6 +++--- > drivers/gpu/host1x/cdma.h | 4 ++-- > 2 files changed, 5 insertions(+), 5 deletions(-) My understanding is that potentially many userspace processes could be blocking on this, which I think is the reason for it being a semaphore. Is the completion going to work for those cases as well? Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] host1x: cdma: use completion instead of semaphore
On Tue, Dec 11, 2018 at 11:08 AM Thierry Reding wrote: > > On Mon, Dec 10, 2018 at 10:51:04PM +0100, Arnd Bergmann wrote: > > In this usage, the two are completely equivalent, but the > > completion documents better what is going on, and we generally > > try to avoid semaphores these days. > > > > Signed-off-by: Arnd Bergmann > > --- > > drivers/gpu/host1x/cdma.c | 6 +++--- > > drivers/gpu/host1x/cdma.h | 4 ++-- > > 2 files changed, 5 insertions(+), 5 deletions(-) > > My understanding is that potentially many userspace processes could be > blocking on this, which I think is the reason for it being a semaphore. > Is the completion going to work for those cases as well? Yes, it behaves the exact same way here. Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v2 2/2] drm/msm/a6xx: Fix NULL dereference during crashstate capture
The gpu crashstate's base objects registers pointer can be NULL if the target implementation decides to capture the register dump on its own. This patch simply checks for NULL before dereferencing. Signed-off-by: Sharat Masetty --- Changes from v1: Addressed comments from Jordan Crouse drivers/gpu/drm/msm/adreno/adreno_gpu.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 40bcf32..56a63c4 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -415,6 +415,9 @@ void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state) } } + if (!adreno_gpu->registers) + return; + /* Count the number of registers */ for (i = 0; adreno_gpu->registers[i] != ~0; i += 2) count += adreno_gpu->registers[i + 1] - @@ -550,9 +553,10 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state, } } - drm_puts(p, "registers:\n"); - for (i = 0; i < state->nr_registers; i++) { + if (i == 0) + drm_puts(p, "registers:\n"); + drm_printf(p, " - { offset: 0x%04x, value: 0x%08x }\n", state->registers[i * 2] << 2, state->registers[(i * 2) + 1]); -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v2 1/2] drm/msm/adreno: Make adreno_gpu_state_get() return void
We are not really checking the state of the adreno_gpu_state_get() function at the callers and in addition the state capture is mostly a best effort service, so make the function return void. Signed-off-by: Sharat Masetty --- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4 +--- drivers/gpu/drm/msm/adreno/adreno_gpu.h | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 1ca4bea..40bcf32 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -380,7 +380,7 @@ bool adreno_idle(struct msm_gpu *gpu, struct msm_ringbuffer *ring) return false; } -int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state) +void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state) { struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); int i, count = 0; @@ -437,8 +437,6 @@ int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state) state->nr_registers = count; } - - return 0; } void adreno_gpu_state_destroy(struct msm_gpu_state *state) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h index 4973c8c..d4834b3 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h @@ -235,7 +235,7 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, void adreno_gpu_state_destroy(struct msm_gpu_state *state); -int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state); +void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state); int adreno_gpu_state_put(struct msm_gpu_state *state); /* ringbuffer helpers (the parts that are adreno specific) */ -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] host1x: cdma: use completion instead of semaphore
On Tue, Dec 11, 2018 at 11:11:38AM +0100, Arnd Bergmann wrote: > On Tue, Dec 11, 2018 at 11:08 AM Thierry Reding > wrote: > > > > On Mon, Dec 10, 2018 at 10:51:04PM +0100, Arnd Bergmann wrote: > > > In this usage, the two are completely equivalent, but the > > > completion documents better what is going on, and we generally > > > try to avoid semaphores these days. > > > > > > Signed-off-by: Arnd Bergmann > > > --- > > > drivers/gpu/host1x/cdma.c | 6 +++--- > > > drivers/gpu/host1x/cdma.h | 4 ++-- > > > 2 files changed, 5 insertions(+), 5 deletions(-) > > > > My understanding is that potentially many userspace processes could be > > blocking on this, which I think is the reason for it being a semaphore. > > Is the completion going to work for those cases as well? > > Yes, it behaves the exact same way here. Great, I'll queue this for v4.22. Thanks, Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 02/10] drm/syncobj: remove drm_syncobj_cb and cleanup
From: Christian König This completes "drm/syncobj: Drop add/remove_callback from driver interface" and cleans up the implementation a bit. Signed-off-by: Christian König --- drivers/gpu/drm/drm_syncobj.c | 91 --- include/drm/drm_syncobj.h | 21 2 files changed, 30 insertions(+), 82 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index db30a0e89db8..e19525af0cce 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -56,6 +56,16 @@ #include "drm_internal.h" #include +struct syncobj_wait_entry { + struct list_head node; + struct task_struct *task; + struct dma_fence *fence; + struct dma_fence_cb fence_cb; +}; + +static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj, + struct syncobj_wait_entry *wait); + /** * drm_syncobj_find - lookup and reference a sync object. * @file_private: drm file private pointer @@ -82,58 +92,33 @@ struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private, } EXPORT_SYMBOL(drm_syncobj_find); -static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj, - struct drm_syncobj_cb *cb, - drm_syncobj_func_t func) +static void drm_syncobj_fence_add_wait(struct drm_syncobj *syncobj, + struct syncobj_wait_entry *wait) { - cb->func = func; - list_add_tail(&cb->node, &syncobj->cb_list); -} - -static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj, -struct dma_fence **fence, -struct drm_syncobj_cb *cb, -drm_syncobj_func_t func) -{ - int ret; - - *fence = drm_syncobj_fence_get(syncobj); - if (*fence) - return 1; + if (wait->fence) + return; spin_lock(&syncobj->lock); /* We've already tried once to get a fence and failed. Now that we * have the lock, try one more time just to be sure we don't add a * callback when a fence has already been set. */ - if (syncobj->fence) { - *fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, - lockdep_is_held(&syncobj->lock))); - ret = 1; - } else { - *fence = NULL; - drm_syncobj_add_callback_locked(syncobj, cb, func); - ret = 0; - } + if (syncobj->fence) + wait->fence = dma_fence_get( + rcu_dereference_protected(syncobj->fence, 1)); + else + list_add_tail(&wait->node, &syncobj->cb_list); spin_unlock(&syncobj->lock); - - return ret; } -void drm_syncobj_add_callback(struct drm_syncobj *syncobj, - struct drm_syncobj_cb *cb, - drm_syncobj_func_t func) +static void drm_syncobj_remove_wait(struct drm_syncobj *syncobj, + struct syncobj_wait_entry *wait) { - spin_lock(&syncobj->lock); - drm_syncobj_add_callback_locked(syncobj, cb, func); - spin_unlock(&syncobj->lock); -} + if (!wait->node.next) + return; -void drm_syncobj_remove_callback(struct drm_syncobj *syncobj, -struct drm_syncobj_cb *cb) -{ spin_lock(&syncobj->lock); - list_del_init(&cb->node); + list_del_init(&wait->node); spin_unlock(&syncobj->lock); } @@ -148,7 +133,7 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, struct dma_fence *fence) { struct dma_fence *old_fence; - struct drm_syncobj_cb *cur, *tmp; + struct syncobj_wait_entry *cur, *tmp; if (fence) dma_fence_get(fence); @@ -162,7 +147,7 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, if (fence != old_fence) { list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) { list_del_init(&cur->node); - cur->func(syncobj, cur); + syncobj_wait_syncobj_func(syncobj, cur); } } @@ -608,13 +593,6 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, &args->handle); } -struct syncobj_wait_entry { - struct task_struct *task; - struct dma_fence *fence; - struct dma_fence_cb fence_cb; - struct drm_syncobj_cb syncobj_cb; -}; - static void syncobj_wait_fence_func(struct dma_fence *fence, struct dma_fence_cb *cb) { @@ -625,11 +603,8 @@ static void syncobj_wait_fence_func(struct dma_fence *
[PATCH 01/10] dma-buf: add new dma_fence_chain container v4
From: Christian König Lockless container implementation similar to a dma_fence_array, but with only two elements per node and automatic garbage collection. v2: properly document dma_fence_chain_for_each, add dma_fence_chain_find_seqno, drop prev reference during garbage collection if it's not a chain fence. v3: use head and iterator for dma_fence_chain_for_each v4: fix reference count in dma_fence_chain_enable_signaling Signed-off-by: Christian König --- drivers/dma-buf/Makefile | 3 +- drivers/dma-buf/dma-fence-chain.c | 241 ++ include/linux/dma-fence-chain.h | 81 ++ 3 files changed, 324 insertions(+), 1 deletion(-) create mode 100644 drivers/dma-buf/dma-fence-chain.c create mode 100644 include/linux/dma-fence-chain.h diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index 0913a6ccab5a..1f006e083eb9 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -1,4 +1,5 @@ -obj-y := dma-buf.o dma-fence.o dma-fence-array.o reservation.o seqno-fence.o +obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \ +reservation.o seqno-fence.o obj-$(CONFIG_SYNC_FILE)+= sync_file.o obj-$(CONFIG_SW_SYNC) += sw_sync.o sync_debug.o obj-$(CONFIG_UDMABUF) += udmabuf.o diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c new file mode 100644 index ..0c5e3c902fa0 --- /dev/null +++ b/drivers/dma-buf/dma-fence-chain.c @@ -0,0 +1,241 @@ +/* + * fence-chain: chain fences together in a timeline + * + * Copyright (C) 2018 Advanced Micro Devices, Inc. + * Authors: + * Christian König + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include + +static bool dma_fence_chain_enable_signaling(struct dma_fence *fence); + +/** + * dma_fence_chain_get_prev - use RCU to get a reference to the previous fence + * @chain: chain node to get the previous node from + * + * Use dma_fence_get_rcu_safe to get a reference to the previous fence of the + * chain node. + */ +static struct dma_fence *dma_fence_chain_get_prev(struct dma_fence_chain *chain) +{ + struct dma_fence *prev; + + rcu_read_lock(); + prev = dma_fence_get_rcu_safe(&chain->prev); + rcu_read_unlock(); + return prev; +} + +/** + * dma_fence_chain_walk - chain walking function + * @fence: current chain node + * + * Walk the chain to the next node. Returns the next fence or NULL if we are at + * the end of the chain. Garbage collects chain nodes which are already + * signaled. + */ +struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence) +{ + struct dma_fence_chain *chain, *prev_chain; + struct dma_fence *prev, *replacement, *tmp; + + chain = to_dma_fence_chain(fence); + if (!chain) { + dma_fence_put(fence); + return NULL; + } + + while ((prev = dma_fence_chain_get_prev(chain))) { + + prev_chain = to_dma_fence_chain(prev); + if (prev_chain) { + if (!dma_fence_is_signaled(prev_chain->fence)) + break; + + replacement = dma_fence_chain_get_prev(prev_chain); + } else { + if (!dma_fence_is_signaled(prev)) + break; + + replacement = NULL; + } + + tmp = cmpxchg(&chain->prev, prev, replacement); + if (tmp == prev) + dma_fence_put(tmp); + else + dma_fence_put(replacement); + dma_fence_put(prev); + } + + dma_fence_put(fence); + return prev; +} +EXPORT_SYMBOL(dma_fence_chain_walk); + +/** + * dma_fence_chain_find_seqno - find fence chain node by seqno + * @pfence: pointer to the chain node where to start + * @seqno: the sequence number to search for + * + * Advance the fence pointer to the chain node which will signal this sequence + * number. If no sequence number is provided then this is a no-op. + * + * Returns EINVAL if the fence is not a chain node or the sequence number has + * not yet advanced far enough. + */ +int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno) +{ + struct dma_fence_chain *chain; + + if (!seqno) + return 0; + + chain = to_dma_fence_chain(*pfence); + if (!chain || chain->base.seqno < seqno) + return -EINVAL; + + dma_fence_chain_for_each(*pfence, &chain->base) { + if ((*pf
[PATCH 04/10] drm/syncobj: add support for timeline point wait v8
points array is one-to-one match with syncobjs array. v2: add seperate ioctl for timeline point wait, otherwise break uapi. v3: userspace can specify two kinds waits:: a. Wait for time point to be completed. b. and wait for time point to become available v4: rebase v5: add comment for xxx_WAIT_AVAILABLE v6: rebase and rework on new container v7: drop _WAIT_COMPLETED, it is the default anyway v8: correctly handle garbage collected fences Signed-off-by: Chunming Zhou Signed-off-by: Christian König Cc: Daniel Rakos Cc: Jason Ekstrand Cc: Bas Nieuwenhuizen Cc: Dave Airlie Cc: Chris Wilson --- drivers/gpu/drm/drm_internal.h | 2 + drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_syncobj.c | 153 ++--- include/uapi/drm/drm.h | 15 4 files changed, 143 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index c7a7d7ce5d1c..18b41e10195c 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -178,6 +178,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); +int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 94bd872d56c4..a9a17ed35cc4 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -675,6 +675,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, drm_syncobj_timeline_wait_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 2b8a744fbbb6..532d7d3c98cd 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -61,6 +61,7 @@ struct syncobj_wait_entry { struct task_struct *task; struct dma_fence *fence; struct dma_fence_cb fence_cb; + u64point; }; static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj, @@ -95,6 +96,8 @@ EXPORT_SYMBOL(drm_syncobj_find); static void drm_syncobj_fence_add_wait(struct drm_syncobj *syncobj, struct syncobj_wait_entry *wait) { + struct dma_fence *fence; + if (wait->fence) return; @@ -103,11 +106,15 @@ static void drm_syncobj_fence_add_wait(struct drm_syncobj *syncobj, * have the lock, try one more time just to be sure we don't add a * callback when a fence has already been set. */ - if (syncobj->fence) - wait->fence = dma_fence_get( - rcu_dereference_protected(syncobj->fence, 1)); - else + fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, 1)); + if (!fence || dma_fence_chain_find_seqno(&fence, wait->point)) { + dma_fence_put(fence); list_add_tail(&wait->node, &syncobj->cb_list); + } else if (!fence) { + wait->fence = dma_fence_get_stub(); + } else { + wait->fence = fence; + } spin_unlock(&syncobj->lock); } @@ -147,10 +154,8 @@ void drm_syncobj_add_point(struct drm_syncobj *syncobj, dma_fence_chain_init(chain, prev, fence, point); rcu_assign_pointer(syncobj->fence, &chain->base); - list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) { - list_del_init(&cur->node); + list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) syncobj_wait_syncobj_func(syncobj, cur); - } spin_unlock(&syncobj->lock); /* Walk the chain once to trigger garbage collection */ @@ -182,10 +187,8 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, rcu_assign_pointer(syncobj->fence, fence); if (fence != old_fence) { - list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) { - list_del_init(&cur->node); + list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) syncobj_wait_syncobj_func(syncobj, cur); - } } spin_
[PATCH 05/10] drm/syncobj: add timeline payload query ioctl v4
user mode can query timeline payload. v2: check return value of copy_to_user v3: handle querying entry by entry v4: rebase on new chain container, simplify interface Signed-off-by: Chunming Zhou Cc: Daniel Rakos Cc: Jason Ekstrand Cc: Bas Nieuwenhuizen Cc: Dave Airlie Cc: Christian König Cc: Chris Wilson --- drivers/gpu/drm/drm_internal.h | 2 ++ drivers/gpu/drm/drm_ioctl.c| 2 ++ drivers/gpu/drm/drm_syncobj.c | 43 ++ include/uapi/drm/drm.h | 10 4 files changed, 57 insertions(+) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 18b41e10195c..dab4d5936441 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -184,6 +184,8 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); +int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); /* drm_framebuffer.c */ void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index a9a17ed35cc4..7578ef6dc1d1 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -681,6 +681,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_UNLOCKED), diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 532d7d3c98cd..76ce13dafc4d 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1061,3 +1061,46 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, return ret; } + +int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private) +{ + struct drm_syncobj_timeline_array *args = data; + struct drm_syncobj **syncobjs; + uint64_t __user *points = u64_to_user_ptr(args->points); + uint32_t i; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) + return -ENODEV; + + if (args->pad != 0) + return -EINVAL; + + if (args->count_handles == 0) + return -EINVAL; + + ret = drm_syncobj_array_find(file_private, +u64_to_user_ptr(args->handles), +args->count_handles, +&syncobjs); + if (ret < 0) + return ret; + + for (i = 0; i < args->count_handles; i++) { + struct dma_fence_chain *chain; + struct dma_fence *fence; + uint64_t point; + + fence = drm_syncobj_fence_get(syncobjs[i]); + chain = to_dma_fence_chain(fence); + point = chain ? fence->seqno : 0; + ret = copy_to_user(&points[i], &point, sizeof(uint64_t)); + ret = ret ? -EFAULT : 0; + if (ret) + break; + } + drm_syncobj_array_free(syncobjs, args->count_handles); + + return ret; +} diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 0092111d002c..b2c36f2b2599 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -767,6 +767,14 @@ struct drm_syncobj_array { __u32 pad; }; +struct drm_syncobj_timeline_array { + __u64 handles; + __u64 points; + __u32 count_handles; + __u32 pad; +}; + + /* Query current scanout sequence number */ struct drm_crtc_get_sequence { __u32 crtc_id; /* requested crtc_id */ @@ -924,6 +932,8 @@ extern "C" { #define DRM_IOCTL_MODE_REVOKE_LEASEDRM_IOWR(0xC9, struct drm_mode_revoke_lease) #define DRM_IOCTL_SYNCOBJ_TIMELINE_WAITDRM_IOWR(0xCA, struct drm_syncobj_timeline_wait) +#define DRM_IOCTL_SYNCOBJ_QUERYDRM_IOWR(0xCB, struct drm_syncobj_timeline_array) + /** * Device specific ioctls should only be in their respective headers * The device specific ioctl range is from 0x40 to 0x9f. -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 03/10] drm/syncobj: add new drm_syncobj_add_point interface v3
From: Christian König Use the dma_fence_chain object to create a timeline of fence objects instead of just replacing the existing fence. v2: rebase and cleanup v3: fix garbage collection parameters Signed-off-by: Christian König --- drivers/gpu/drm/drm_syncobj.c | 37 +++ include/drm/drm_syncobj.h | 5 + 2 files changed, 42 insertions(+) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index e19525af0cce..2b8a744fbbb6 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -122,6 +122,43 @@ static void drm_syncobj_remove_wait(struct drm_syncobj *syncobj, spin_unlock(&syncobj->lock); } +/** + * drm_syncobj_add_point - add new timeline point to the syncobj + * @syncobj: sync object to add timeline point do + * @chain: chain node to use to add the point + * @fence: fence to encapsulate in the chain node + * @point: sequence number to use for the point + * + * Add the chain node as new timeline point to the syncobj. + */ +void drm_syncobj_add_point(struct drm_syncobj *syncobj, + struct dma_fence_chain *chain, + struct dma_fence *fence, + uint64_t point) +{ + struct syncobj_wait_entry *cur, *tmp; + struct dma_fence *prev; + + dma_fence_get(fence); + + spin_lock(&syncobj->lock); + + prev = drm_syncobj_fence_get(syncobj); + dma_fence_chain_init(chain, prev, fence, point); + rcu_assign_pointer(syncobj->fence, &chain->base); + + list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) { + list_del_init(&cur->node); + syncobj_wait_syncobj_func(syncobj, cur); + } + spin_unlock(&syncobj->lock); + + /* Walk the chain once to trigger garbage collection */ + dma_fence_chain_for_each(fence, prev); + dma_fence_put(prev); +} +EXPORT_SYMBOL(drm_syncobj_add_point); + /** * drm_syncobj_replace_fence - replace fence in a sync object. * @syncobj: Sync object to replace fence in diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 7c6ed845c70d..8acb4ae4f311 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -27,6 +27,7 @@ #define __DRM_SYNCOBJ_H__ #include "linux/dma-fence.h" +#include "linux/dma-fence-chain.h" /** * struct drm_syncobj - sync object. @@ -110,6 +111,10 @@ drm_syncobj_fence_get(struct drm_syncobj *syncobj) struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private, u32 handle); +void drm_syncobj_add_point(struct drm_syncobj *syncobj, + struct dma_fence_chain *chain, + struct dma_fence *fence, + uint64_t point); void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, struct dma_fence *fence); int drm_syncobj_find_fence(struct drm_file *file_private, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/10] drm/amdgpu: update version for timeline syncobj support in amdgpu
Signed-off-by: Chunming Zhou --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 8de55f7f1a3a..cafafdb1d03f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -71,9 +71,10 @@ * - 3.25.0 - Add support for sensor query info (stable pstate sclk/mclk). * - 3.26.0 - GFX9: Process AMDGPU_IB_FLAG_TC_WB_NOT_INVALIDATE. * - 3.27.0 - Add new chunk to to AMDGPU_CS to enable BO_LIST creation. + * - 3.28.0 - Add syncobj timeline support to AMDGPU_CS. */ #define KMS_DRIVER_MAJOR 3 -#define KMS_DRIVER_MINOR 27 +#define KMS_DRIVER_MINOR 28 #define KMS_DRIVER_PATCHLEVEL 0 int amdgpu_vram_limit = 0; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/10] drm/amdgpu: add timeline support in amdgpu CS v2
syncobj wait/signal operation is appending in command submission. v2: separate to two kinds in/out_deps functions Signed-off-by: Chunming Zhou Cc: Daniel Rakos Cc: Jason Ekstrand Cc: Bas Nieuwenhuizen Cc: Dave Airlie Cc: Christian König Cc: Chris Wilson --- drivers/gpu/drm/amd/amdgpu/amdgpu.h| 10 +- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 147 + include/uapi/drm/amdgpu_drm.h | 8 ++ 3 files changed, 140 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 42f882c633ee..f9160ea1396a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -545,6 +545,12 @@ struct amdgpu_cs_chunk { void*kdata; }; +struct amdgpu_cs_post_dep { + struct drm_syncobj *syncobj; + struct dma_fence_chain *chain; + u64 point; +}; + struct amdgpu_cs_parser { struct amdgpu_device*adev; struct drm_file *filp; @@ -574,8 +580,8 @@ struct amdgpu_cs_parser { /* user fence */ struct amdgpu_bo_list_entry uf_entry; - unsigned num_post_dep_syncobjs; - struct drm_syncobj **post_dep_syncobjs; + unsignednum_post_deps; + struct amdgpu_cs_post_dep *post_deps; }; static inline u32 amdgpu_get_ib_value(struct amdgpu_cs_parser *p, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index dc54e9efd910..580f1ea27157 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -213,6 +213,8 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs case AMDGPU_CHUNK_ID_DEPENDENCIES: case AMDGPU_CHUNK_ID_SYNCOBJ_IN: case AMDGPU_CHUNK_ID_SYNCOBJ_OUT: + case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT: + case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL: break; default: @@ -792,9 +794,11 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, ttm_eu_backoff_reservation(&parser->ticket, &parser->validated); - for (i = 0; i < parser->num_post_dep_syncobjs; i++) - drm_syncobj_put(parser->post_dep_syncobjs[i]); - kfree(parser->post_dep_syncobjs); + for (i = 0; i < parser->num_post_deps; i++) { + drm_syncobj_put(parser->post_deps[i].syncobj); + kfree(parser->post_deps[i].chain); + } + kfree(parser->post_deps); dma_fence_put(parser->fence); @@ -1100,13 +1104,18 @@ static int amdgpu_cs_process_fence_dep(struct amdgpu_cs_parser *p, } static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p, -uint32_t handle) +uint32_t handle, u64 point, +u64 flags) { - int r; struct dma_fence *fence; - r = drm_syncobj_find_fence(p->filp, handle, 0, 0, &fence); - if (r) + int r; + + r = drm_syncobj_find_fence(p->filp, handle, point, flags, &fence); + if (r) { + DRM_ERROR("syncobj %u failed to find fence @ %llu (%d)!\n", + handle, point, r); return r; + } r = amdgpu_sync_fence(p->adev, &p->job->sync, fence, true); dma_fence_put(fence); @@ -1117,46 +1126,115 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p, static int amdgpu_cs_process_syncobj_in_dep(struct amdgpu_cs_parser *p, struct amdgpu_cs_chunk *chunk) { + struct drm_amdgpu_cs_chunk_sem *deps; unsigned num_deps; int i, r; - struct drm_amdgpu_cs_chunk_sem *deps; deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; num_deps = chunk->length_dw * 4 / sizeof(struct drm_amdgpu_cs_chunk_sem); + for (i = 0; i < num_deps; ++i) { + r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle, + 0, 0); + if (r) + return r; + } + + return 0; +} + +static int amdgpu_cs_process_syncobj_timeline_in_dep(struct amdgpu_cs_parser *p, +struct amdgpu_cs_chunk *chunk) +{ + struct drm_amdgpu_cs_chunk_syncobj *syncobj_deps; + unsigned num_deps; + int i, r; + + syncobj_deps = (struct drm_amdgpu_cs_chunk_syncobj *)chunk->kdata; + num_deps = chunk->length_dw * 4 / + sizeof(struct drm_amdgpu_cs_chunk_syncobj); for (i = 0; i < num_deps; ++i) { - r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle)
[PATCH 06/10] drm/syncobj: use the timeline point in drm_syncobj_find_fence v3
From: Christian König Implement finding the right timeline point in drm_syncobj_find_fence. v2: return -EINVAL when the point is not submitted yet. v3: fix reference counting bug, add flags handling as well Signed-off-by: Christian König --- drivers/gpu/drm/drm_syncobj.c | 43 --- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 76ce13dafc4d..d964b348ecba 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -231,16 +231,53 @@ int drm_syncobj_find_fence(struct drm_file *file_private, struct dma_fence **fence) { struct drm_syncobj *syncobj = drm_syncobj_find(file_private, handle); - int ret = 0; + struct syncobj_wait_entry wait; + int ret; if (!syncobj) return -ENOENT; *fence = drm_syncobj_fence_get(syncobj); - if (!*fence) { + drm_syncobj_put(syncobj); + + if (*fence) { + ret = dma_fence_chain_find_seqno(fence, point); + if (!ret) + return 0; + dma_fence_put(*fence); + } else { ret = -EINVAL; } - drm_syncobj_put(syncobj); + + if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)) + return ret; + + memset(&wait, 0, sizeof(wait)); + wait.task = current; + wait.point = point; + drm_syncobj_fence_add_wait(syncobj, &wait); + + do { + set_current_state(TASK_INTERRUPTIBLE); + if (wait.fence) { + ret = 0; + break; + } + + if (signal_pending(current)) { + ret = -ERESTARTSYS; + break; + } + + schedule(); + } while (1); + + __set_current_state(TASK_RUNNING); + *fence = wait.fence; + + if (wait.node.next) + drm_syncobj_remove_wait(syncobj, &wait); + return ret; } EXPORT_SYMBOL(drm_syncobj_find_fence); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 09/10] drm/syncobj: add timeline signal ioctl for syncobj v2
v2: individually allocate chain array, since chain node is free independently. Signed-off-by: Chunming Zhou --- drivers/gpu/drm/drm_internal.h | 2 + drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_syncobj.c | 81 ++ include/uapi/drm/drm.h | 1 + 4 files changed, 86 insertions(+) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 06c2adc4950e..d7a19bd89cb2 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -186,6 +186,8 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); +int drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index e9d4bed12783..a50dc62dc87b 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -683,6 +683,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL, drm_syncobj_timeline_signal_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 3bba86c8160d..961f6d343564 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1173,6 +1173,87 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, return ret; } +int +drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private) +{ + struct drm_syncobj_timeline_array *args = data; + struct drm_syncobj **syncobjs; + struct dma_fence_chain **chains; + uint64_t *points; + uint32_t i, j, timeline_count = 0; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) + return -EOPNOTSUPP; + + if (args->pad != 0) + return -EINVAL; + + if (args->count_handles == 0) + return -EINVAL; + + ret = drm_syncobj_array_find(file_private, +u64_to_user_ptr(args->handles), +args->count_handles, +&syncobjs); + if (ret < 0) + return ret; + + points = kmalloc_array(args->count_handles, sizeof(*points), + GFP_KERNEL); + if (!points) { + ret = -ENOMEM; + goto out; + } + if (!u64_to_user_ptr(args->points)) { + memset(points, 0, args->count_handles * sizeof(uint64_t)); + } else if (copy_from_user(points, u64_to_user_ptr(args->points), + sizeof(uint64_t) * args->count_handles)) { + ret = -EFAULT; + goto err_points; + } + + + for (i = 0; i < args->count_handles; i++) { + if (points[i]) + timeline_count++; + } + chains = kmalloc_array(timeline_count, sizeof(void *), GFP_KERNEL); + if (!chains) { + ret = -ENOMEM; + goto err_points; + } + for (i = 0; i < timeline_count; i++) { + chains[i] = kzalloc(sizeof(struct dma_fence_chain), GFP_KERNEL); + if (!chains[i]) { + for (j = 0; j < i; j++) + kfree(chains[j]); + ret = -ENOMEM; + goto err_chains; + } + } + + for (i = 0, j = 0; i < args->count_handles; i++) { + if (points[i]) { + struct dma_fence *fence = dma_fence_get_stub(); + + drm_syncobj_add_point(syncobjs[i], chains[j++], + fence, points[i]); + dma_fence_put(fence); + } else + drm_syncobj_assign_null_handle(syncobjs[i]); + } +err_chains: + kfree(chains); +err_points: + kfree(points); +out: + drm_syncobj_array_free(syncobjs, args->count_handles); + + return ret; +} + int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, struct d
[PATCH 08/10] drm/syncobj: add transition iotcls between binary and timeline v2
we need to import/export timeline point. v2: unify to one transfer ioctl Signed-off-by: Chunming Zhou --- drivers/gpu/drm/drm_internal.h | 2 + drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_syncobj.c | 74 ++ include/uapi/drm/drm.h | 10 + 4 files changed, 88 insertions(+) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index dab4d5936441..06c2adc4950e 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -176,6 +176,8 @@ int drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); +int drm_syncobj_transfer_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 7578ef6dc1d1..e9d4bed12783 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -673,6 +673,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE, drm_syncobj_fd_to_handle_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TRANSFER, drm_syncobj_transfer_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, drm_syncobj_timeline_wait_ioctl, diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index d964b348ecba..3bba86c8160d 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -670,6 +670,80 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data, &args->handle); } +static int drm_syncobj_transfer_to_timeline(struct drm_file *file_private, + struct drm_syncobj_transfer *args) +{ + struct drm_syncobj *timeline_syncobj = NULL; + struct dma_fence *fence; + struct dma_fence_chain *chain; + int ret; + + timeline_syncobj = drm_syncobj_find(file_private, args->dst_handle); + if (!timeline_syncobj) { + return -ENOENT; + } + ret = drm_syncobj_find_fence(file_private, args->src_handle, +args->src_point, args->flags, +&fence); + if (ret) + goto err; + chain = kzalloc(sizeof(struct dma_fence_chain), GFP_KERNEL); + if (!chain) { + ret = -ENOMEM; + goto err1; + } + drm_syncobj_add_point(timeline_syncobj, chain, fence, args->dst_point); +err1: + dma_fence_put(fence); +err: + drm_syncobj_put(timeline_syncobj); + + return ret; +} + +static int +drm_syncobj_transfer_to_binary(struct drm_file *file_private, + struct drm_syncobj_transfer *args) +{ + struct drm_syncobj *binary_syncobj = NULL; + struct dma_fence *fence; + int ret; + + binary_syncobj = drm_syncobj_find(file_private, args->dst_handle); + if (!binary_syncobj) + return -ENOENT; + ret = drm_syncobj_find_fence(file_private, args->src_handle, +args->src_point, args->flags, &fence); + if (ret) + goto err; + drm_syncobj_replace_fence(binary_syncobj, fence); + dma_fence_put(fence); +err: + drm_syncobj_put(binary_syncobj); + + return ret; +} +int +drm_syncobj_transfer_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private) +{ + struct drm_syncobj_transfer *args = data; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) + return -ENODEV; + + if (args->pad) + return -EINVAL; + + if (args->dst_point) + ret = drm_syncobj_transfer_to_timeline(file_private, args); + else + ret = drm_syncobj_transfer_to_binary(file_private, args); + + return ret; +} + static void syncobj_wait_fence_func(struct dma_fence *fence, struct dma_fence_cb *cb) { diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index b2c36f2b2599..4c1e2e6579fa 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -735,6 +735,15 @@ struct drm_syncobj_handle { __u32 pad; }; +struct drm_syncobj_transfer
[PATCH libdrm 1/8] new syncobj extension v3
v2: drop not implemented IOCTLs and flags v3: add transfer/signal ioctls Signed-off-by: Chunming Zhou Signed-off-by: Christian König --- include/drm/drm.h | 35 +++ 1 file changed, 35 insertions(+) diff --git a/include/drm/drm.h b/include/drm/drm.h index 85c685a2..26f51bca 100644 --- a/include/drm/drm.h +++ b/include/drm/drm.h @@ -729,8 +729,18 @@ struct drm_syncobj_handle { __u32 pad; }; +struct drm_syncobj_transfer { +__u32 src_handle; +__u32 dst_handle; +__u64 src_point; +__u64 dst_point; +__u32 flags; +__u32 pad; +}; + #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0) #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1) +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE (1 << 2) struct drm_syncobj_wait { __u64 handles; /* absolute timeout */ @@ -741,12 +751,31 @@ struct drm_syncobj_wait { __u32 pad; }; +struct drm_syncobj_timeline_wait { +__u64 handles; +/* wait on specific timeline point for every handles*/ +__u64 points; +/* absolute timeout */ +__s64 timeout_nsec; +__u32 count_handles; +__u32 flags; +__u32 first_signaled; /* only valid when not waiting all */ +__u32 pad; +}; + struct drm_syncobj_array { __u64 handles; __u32 count_handles; __u32 pad; }; +struct drm_syncobj_timeline_array { +__u64 handles; +__u64 points; +__u32 count_handles; +__u32 pad; +}; + /* Query current scanout sequence number */ struct drm_crtc_get_sequence { __u32 crtc_id; /* requested crtc_id */ @@ -903,6 +932,12 @@ extern "C" { #define DRM_IOCTL_MODE_GET_LEASE DRM_IOWR(0xC8, struct drm_mode_get_lease) #define DRM_IOCTL_MODE_REVOKE_LEASEDRM_IOWR(0xC9, struct drm_mode_revoke_lease) +#define DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT DRM_IOWR(0xCA, struct drm_syncobj_timeline_wait) +#define DRM_IOCTL_SYNCOBJ_QUERY DRM_IOWR(0xCB, struct drm_syncobj_timeline_array) +#define DRM_IOCTL_SYNCOBJ_TRANSFER DRM_IOWR(0xCC, struct drm_syncobj_transfer) +#define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL DRM_IOWR(0xCD, struct drm_syncobj_timeline_array) + + /** * Device specific ioctls should only be in their respective headers * The device specific ioctl range is from 0x40 to 0x9f. -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 8/8] add syncobj timeline tests v3
v2: drop DRM_SYNCOBJ_CREATE_TYPE_TIMELINE, fix timeout calculation, fix some warnings v3: add export/import and cpu signal testing cases Signed-off-by: Chunming Zhou Signed-off-by: Christian König --- tests/amdgpu/Makefile.am | 3 +- tests/amdgpu/amdgpu_test.c | 12 ++ tests/amdgpu/amdgpu_test.h | 21 +++ tests/amdgpu/meson.build | 2 +- tests/amdgpu/syncobj_tests.c | 290 +++ 5 files changed, 326 insertions(+), 2 deletions(-) create mode 100644 tests/amdgpu/syncobj_tests.c diff --git a/tests/amdgpu/Makefile.am b/tests/amdgpu/Makefile.am index 447ff217..d3fbe2bb 100644 --- a/tests/amdgpu/Makefile.am +++ b/tests/amdgpu/Makefile.am @@ -33,4 +33,5 @@ amdgpu_test_SOURCES = \ vcn_tests.c \ uve_ib.h \ deadlock_tests.c \ - vm_tests.c + vm_tests.c \ + syncobj_tests.c diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c index ebf44098..ff1448f3 100644 --- a/tests/amdgpu/amdgpu_test.c +++ b/tests/amdgpu/amdgpu_test.c @@ -56,6 +56,7 @@ #define UVD_ENC_TESTS_STR "UVD ENC Tests" #define DEADLOCK_TESTS_STR "Deadlock Tests" #define VM_TESTS_STR "VM Tests" +#define SYNCOBJ_TIMELINE_TESTS_STR "SYNCOBJ TIMELINE Tests" /** * Open handles for amdgpu devices @@ -116,6 +117,12 @@ static CU_SuiteInfo suites[] = { .pCleanupFunc = suite_vm_tests_clean, .pTests = vm_tests, }, + { + .pName = SYNCOBJ_TIMELINE_TESTS_STR, + .pInitFunc = suite_syncobj_timeline_tests_init, + .pCleanupFunc = suite_syncobj_timeline_tests_clean, + .pTests = syncobj_timeline_tests, + }, CU_SUITE_INFO_NULL, }; @@ -165,6 +172,11 @@ static Suites_Active_Status suites_active_stat[] = { .pName = VM_TESTS_STR, .pActive = suite_vm_tests_enable, }, + { + .pName = SYNCOBJ_TIMELINE_TESTS_STR, + .pActive = suite_syncobj_timeline_tests_enable, + }, + }; diff --git a/tests/amdgpu/amdgpu_test.h b/tests/amdgpu/amdgpu_test.h index af81eea8..24d64b64 100644 --- a/tests/amdgpu/amdgpu_test.h +++ b/tests/amdgpu/amdgpu_test.h @@ -194,6 +194,27 @@ CU_BOOL suite_vm_tests_enable(void); */ extern CU_TestInfo vm_tests[]; +/** + * Initialize syncobj timeline test suite + */ +int suite_syncobj_timeline_tests_init(); + +/** + * Deinitialize syncobj timeline test suite + */ +int suite_syncobj_timeline_tests_clean(); + +/** + * Decide if the suite is enabled by default or not. + */ +CU_BOOL suite_syncobj_timeline_tests_enable(void); + +/** + * Tests in syncobj timeline test suite + */ +extern CU_TestInfo syncobj_timeline_tests[]; + + /** * Helper functions */ diff --git a/tests/amdgpu/meson.build b/tests/amdgpu/meson.build index 4c1237c6..3ceec715 100644 --- a/tests/amdgpu/meson.build +++ b/tests/amdgpu/meson.build @@ -24,7 +24,7 @@ if dep_cunit.found() files( 'amdgpu_test.c', 'basic_tests.c', 'bo_tests.c', 'cs_tests.c', 'vce_tests.c', 'uvd_enc_tests.c', 'vcn_tests.c', 'deadlock_tests.c', - 'vm_tests.c', + 'vm_tests.c', 'syncobj_tests.c', ), dependencies : [dep_cunit, dep_threads], include_directories : [inc_root, inc_drm, include_directories('../../amdgpu')], diff --git a/tests/amdgpu/syncobj_tests.c b/tests/amdgpu/syncobj_tests.c new file mode 100644 index ..a0c627d7 --- /dev/null +++ b/tests/amdgpu/syncobj_tests.c @@ -0,0 +1,290 @@ +/* + * Copyright 2017 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * +*/ + +#include "CUnit/Basic.h" + +#include "amdgpu_test.h" +#include "amdgpu_drm.h" +#include "amdgpu_internal.h" +#include + +static amdgpu_device_handle device_handle; +static uint32_t major_version; +static uint32_t minor_version; + +static void amdgpu_syncobj_timeline_test(v
[PATCH libdrm 2/8] addr cs chunk for syncobj timeline
Signed-off-by: Chunming Zhou --- include/drm/amdgpu_drm.h | 9 + 1 file changed, 9 insertions(+) diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h index 1ceec56d..a3c067dd 100644 --- a/include/drm/amdgpu_drm.h +++ b/include/drm/amdgpu_drm.h @@ -517,6 +517,8 @@ struct drm_amdgpu_gem_va { #define AMDGPU_CHUNK_ID_SYNCOBJ_IN 0x04 #define AMDGPU_CHUNK_ID_SYNCOBJ_OUT 0x05 #define AMDGPU_CHUNK_ID_BO_HANDLES 0x06 +#define AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT0x07 +#define AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL 0x08 struct drm_amdgpu_cs_chunk { __u32 chunk_id; @@ -592,6 +594,13 @@ struct drm_amdgpu_cs_chunk_sem { __u32 handle; }; +struct drm_amdgpu_cs_chunk_syncobj { + __u32 handle; + __u32 flags; + __u64 point; +}; + + #define AMDGPU_FENCE_TO_HANDLE_GET_SYNCOBJ 0 #define AMDGPU_FENCE_TO_HANDLE_GET_SYNCOBJ_FD 1 #define AMDGPU_FENCE_TO_HANDLE_GET_SYNC_FILE_FD2 -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 4/8] wrap syncobj timeline query/wait APIs for amdgpu v3
v2: symbos are stored in lexical order. v3: drop export/import and extra query indirection Signed-off-by: Chunming Zhou Signed-off-by: Christian König --- amdgpu/amdgpu-symbol-check | 2 ++ amdgpu/amdgpu.h| 39 ++ amdgpu/amdgpu_cs.c | 23 ++ 3 files changed, 64 insertions(+) diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check index 6f5e0f95..4553736f 100755 --- a/amdgpu/amdgpu-symbol-check +++ b/amdgpu/amdgpu-symbol-check @@ -49,8 +49,10 @@ amdgpu_cs_submit amdgpu_cs_submit_raw amdgpu_cs_syncobj_export_sync_file amdgpu_cs_syncobj_import_sync_file +amdgpu_cs_syncobj_query amdgpu_cs_syncobj_reset amdgpu_cs_syncobj_signal +amdgpu_cs_syncobj_timeline_wait amdgpu_cs_syncobj_wait amdgpu_cs_wait_fences amdgpu_cs_wait_semaphore diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h index dc51659a..330658a0 100644 --- a/amdgpu/amdgpu.h +++ b/amdgpu/amdgpu.h @@ -1489,6 +1489,45 @@ int amdgpu_cs_syncobj_wait(amdgpu_device_handle dev, int64_t timeout_nsec, unsigned flags, uint32_t *first_signaled); +/** + * Wait for one or all sync objects on their points to signal. + * + * \param dev- \c [in] self-explanatory + * \param handles - \c [in] array of sync object handles + * \param points - \c [in] array of sync points to wait + * \param num_handles - \c [in] self-explanatory + * \param timeout_nsec - \c [in] self-explanatory + * \param flags - \c [in] a bitmask of DRM_SYNCOBJ_WAIT_FLAGS_* + * \param first_signaled - \c [in] self-explanatory + * + * \return 0 on success\n + * -ETIME - Timeout + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_timeline_wait(amdgpu_device_handle dev, + uint32_t *handles, uint64_t *points, + unsigned num_handles, + int64_t timeout_nsec, unsigned flags, + uint32_t *first_signaled); +/** + * Query sync objects payloads. + * + * \param dev- \c [in] self-explanatory + * \param handles - \c [in] array of sync object handles + * \param points - \c [out] array of sync points returned, which presents + * syncobj payload. + * \param num_handles - \c [in] self-explanatory + * + * \return 0 on success\n + * -ETIME - Timeout + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_query(amdgpu_device_handle dev, + uint32_t *handles, uint64_t *points, + unsigned num_handles); + /** * Export kernel sync object to shareable fd. * diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c index 3b8231aa..e4a547c6 100644 --- a/amdgpu/amdgpu_cs.c +++ b/amdgpu/amdgpu_cs.c @@ -661,6 +661,29 @@ drm_public int amdgpu_cs_syncobj_wait(amdgpu_device_handle dev, flags, first_signaled); } +drm_public int amdgpu_cs_syncobj_timeline_wait(amdgpu_device_handle dev, + uint32_t *handles, uint64_t *points, + unsigned num_handles, + int64_t timeout_nsec, unsigned flags, + uint32_t *first_signaled) +{ + if (NULL == dev) + return -EINVAL; + + return drmSyncobjTimelineWait(dev->fd, handles, points, num_handles, + timeout_nsec, flags, first_signaled); +} + +drm_public int amdgpu_cs_syncobj_query(amdgpu_device_handle dev, + uint32_t *handles, uint64_t *points, + unsigned num_handles) +{ + if (NULL == dev) + return -EINVAL; + + return drmSyncobjQuery(dev->fd, handles, points, num_handles); +} + drm_public int amdgpu_cs_export_syncobj(amdgpu_device_handle dev, uint32_t handle, int *shared_fd) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 3/8] add timeline wait/query ioctl v2
v2: drop export/import Signed-off-by: Chunming Zhou --- xf86drm.c | 44 xf86drm.h | 6 ++ 2 files changed, 50 insertions(+) diff --git a/xf86drm.c b/xf86drm.c index 71ad54ba..9816b3b2 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -4277,3 +4277,47 @@ drm_public int drmSyncobjSignal(int fd, const uint32_t *handles, ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_SIGNAL, &args); return ret; } + +drm_public int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t *points, + unsigned num_handles, + int64_t timeout_nsec, unsigned flags, + uint32_t *first_signaled) +{ +struct drm_syncobj_timeline_wait args; +int ret; + +memclear(args); +args.handles = (uintptr_t)handles; +args.points = (uint64_t)(uintptr_t)points; +args.timeout_nsec = timeout_nsec; +args.count_handles = num_handles; +args.flags = flags; + +ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, &args); +if (ret < 0) +return -errno; + +if (first_signaled) +*first_signaled = args.first_signaled; +return ret; +} + + +drm_public int drmSyncobjQuery(int fd, uint32_t *handles, uint64_t *points, + uint32_t handle_count) +{ +struct drm_syncobj_timeline_array args; +int ret; + +memclear(args); +args.handles = (uintptr_t)handles; +args.points = (uint64_t)(uintptr_t)points; +args.count_handles = handle_count; + +ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_QUERY, &args); +if (ret) +return ret; +return 0; +} + + diff --git a/xf86drm.h b/xf86drm.h index 7773d71a..49a40633 100644 --- a/xf86drm.h +++ b/xf86drm.h @@ -875,6 +875,12 @@ extern int drmSyncobjWait(int fd, uint32_t *handles, unsigned num_handles, uint32_t *first_signaled); extern int drmSyncobjReset(int fd, const uint32_t *handles, uint32_t handle_count); extern int drmSyncobjSignal(int fd, const uint32_t *handles, uint32_t handle_count); +extern int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t *points, + unsigned num_handles, + int64_t timeout_nsec, unsigned flags, + uint32_t *first_signaled); +extern int drmSyncobjQuery(int fd, uint32_t *handles, uint64_t *points, + uint32_t handle_count); #if defined(__cplusplus) } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 5/8] add timeline signal/transfer ioctls v2
v2: use one transfer ioctl Signed-off-by: Chunming Zhou --- xf86drm.c | 33 + xf86drm.h | 6 ++ 2 files changed, 39 insertions(+) diff --git a/xf86drm.c b/xf86drm.c index 9816b3b2..2a089616 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -4278,6 +4278,21 @@ drm_public int drmSyncobjSignal(int fd, const uint32_t *handles, return ret; } +drm_public int drmSyncobjTimelineSignal(int fd, const uint32_t *handles, + uint64_t *points, uint32_t handle_count) +{ +struct drm_syncobj_timeline_array args; +int ret; + +memclear(args); +args.handles = (uintptr_t)handles; +args.points = (uint64_t)(uintptr_t)points; +args.count_handles = handle_count; + +ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL, &args); +return ret; +} + drm_public int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t *points, unsigned num_handles, int64_t timeout_nsec, unsigned flags, @@ -4320,4 +4335,22 @@ drm_public int drmSyncobjQuery(int fd, uint32_t *handles, uint64_t *points, return 0; } +drm_public int drmSyncobjTransfer(int fd, + uint32_t dst_handle, uint64_t dst_point, + uint32_t src_handle, uint64_t src_point, + uint32_t flags) +{ +struct drm_syncobj_transfer args; +int ret; + +memclear(args); +args.src_handle = src_handle; +args.dst_handle = dst_handle; +args.src_point = src_point; +args.dst_point = dst_point; +args.flags = flags; + +ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TRANSFER, &args); +return ret; +} diff --git a/xf86drm.h b/xf86drm.h index 49a40633..799d9b9e 100644 --- a/xf86drm.h +++ b/xf86drm.h @@ -875,12 +875,18 @@ extern int drmSyncobjWait(int fd, uint32_t *handles, unsigned num_handles, uint32_t *first_signaled); extern int drmSyncobjReset(int fd, const uint32_t *handles, uint32_t handle_count); extern int drmSyncobjSignal(int fd, const uint32_t *handles, uint32_t handle_count); +extern int drmSyncobjTimelineSignal(int fd, const uint32_t *handles, + uint64_t *points, uint32_t handle_count); extern int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t *points, unsigned num_handles, int64_t timeout_nsec, unsigned flags, uint32_t *first_signaled); extern int drmSyncobjQuery(int fd, uint32_t *handles, uint64_t *points, uint32_t handle_count); +extern int drmSyncobjTransfer(int fd, + uint32_t dst_handle, uint64_t dst_point, + uint32_t src_handle, uint64_t src_point, + uint32_t flags); #if defined(__cplusplus) } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 7/8] wrap transfer interfaces
Signed-off-by: Chunming Zhou --- amdgpu/amdgpu.h| 22 ++ amdgpu/amdgpu_cs.c | 16 2 files changed, 38 insertions(+) diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h index 5536d2d5..48e28aef 100644 --- a/amdgpu/amdgpu.h +++ b/amdgpu/amdgpu.h @@ -1638,6 +1638,28 @@ int amdgpu_cs_syncobj_import_sync_file2(amdgpu_device_handle dev, uint32_t syncobj, uint64_t point, int sync_file_fd); + +/** + * transfer between syncbojs. + * + * \param dev- \c [in] device handle + * \param dst_handle - \c [in] sync object handle + * \param dst_point - \c [in] timeline point, 0 presents dst is binary + * \param src_handle - \c [in] sync object handle + * \param src_point - \c [in] timeline point, 0 presents src is binary + * \param flags - \c [in] flags + * + * \return 0 on success\n + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_transfer(amdgpu_device_handle dev, + uint32_t dst_handle, + uint64_t dst_point, + uint32_t src_handle, + uint64_t src_point, + uint32_t flags); + /** * Export an amdgpu fence as a handle (syncobj or fd). * diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c index 399aa66e..62a2f3de 100644 --- a/amdgpu/amdgpu_cs.c +++ b/amdgpu/amdgpu_cs.c @@ -792,6 +792,22 @@ out: return ret; } +drm_public int amdgpu_cs_syncobj_transfer(amdgpu_device_handle dev, + uint32_t dst_handle, + uint64_t dst_point, + uint32_t src_handle, + uint64_t src_point, + uint32_t flags) +{ + if (NULL == dev) + return -EINVAL; + + return drmSyncobjTransfer(dev->fd, + dst_handle, dst_point, + src_handle, src_point, + flags); +} + drm_public int amdgpu_cs_submit_raw(amdgpu_device_handle dev, amdgpu_context_handle context, amdgpu_bo_list_handle bo_list_handle, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 6/8] expose timeline signal/export/import interfaces v2
v2: adapt to new one transfer ioctl Signed-off-by: Chunming Zhou --- amdgpu/amdgpu-symbol-check | 3 ++ amdgpu/amdgpu.h| 51 amdgpu/amdgpu_cs.c | 68 ++ 3 files changed, 122 insertions(+) diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check index 4553736f..255d25e5 100755 --- a/amdgpu/amdgpu-symbol-check +++ b/amdgpu/amdgpu-symbol-check @@ -48,10 +48,13 @@ amdgpu_cs_signal_semaphore amdgpu_cs_submit amdgpu_cs_submit_raw amdgpu_cs_syncobj_export_sync_file +amdgpu_cs_syncobj_export_sync_file2 amdgpu_cs_syncobj_import_sync_file +amdgpu_cs_syncobj_import_sync_file2 amdgpu_cs_syncobj_query amdgpu_cs_syncobj_reset amdgpu_cs_syncobj_signal +amdgpu_cs_syncobj_timeline_signal amdgpu_cs_syncobj_timeline_wait amdgpu_cs_syncobj_wait amdgpu_cs_wait_fences diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h index 330658a0..5536d2d5 100644 --- a/amdgpu/amdgpu.h +++ b/amdgpu/amdgpu.h @@ -1469,6 +1469,23 @@ int amdgpu_cs_syncobj_reset(amdgpu_device_handle dev, int amdgpu_cs_syncobj_signal(amdgpu_device_handle dev, const uint32_t *syncobjs, uint32_t syncobj_count); +/** + * Signal kernel timeline sync objects. + * + * \param dev - \c [in] device handle + * \param syncobjs - \c [in] array of sync object handles + * \param points - \c [in] array of timeline points + * \param syncobj_count - \c [in] number of handles in syncobjs + * + * \return 0 on success\n + * <0 - Negative POSIX Error code + * +*/ +int amdgpu_cs_syncobj_timeline_signal(amdgpu_device_handle dev, + const uint32_t *syncobjs, + uint64_t *points, + uint32_t syncobj_count); + /** * Wait for one or all sync objects to signal. * @@ -1586,7 +1603,41 @@ int amdgpu_cs_syncobj_export_sync_file(amdgpu_device_handle dev, int amdgpu_cs_syncobj_import_sync_file(amdgpu_device_handle dev, uint32_t syncobj, int sync_file_fd); +/** + * Export kernel timeline sync object to a sync_file. + * + * \param dev- \c [in] device handle + * \param syncobj- \c [in] sync object handle + * \param point - \c [in] timeline point + * \param flags - \c [in] flags + * \param sync_file_fd - \c [out] sync_file file descriptor. + * + * \return 0 on success\n + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_export_sync_file2(amdgpu_device_handle dev, + uint32_t syncobj, + uint64_t point, + uint32_t flags, + int *sync_file_fd); +/** + * Import kernel timeline sync object from a sync_file. + * + * \param dev- \c [in] device handle + * \param syncobj- \c [in] sync object handle + * \param point - \c [in] timeline point + * \param sync_file_fd - \c [in] sync_file file descriptor. + * + * \return 0 on success\n + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_import_sync_file2(amdgpu_device_handle dev, + uint32_t syncobj, + uint64_t point, + int sync_file_fd); /** * Export an amdgpu fence as a handle (syncobj or fd). * diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c index e4a547c6..399aa66e 100644 --- a/amdgpu/amdgpu_cs.c +++ b/amdgpu/amdgpu_cs.c @@ -649,6 +649,18 @@ drm_public int amdgpu_cs_syncobj_signal(amdgpu_device_handle dev, return drmSyncobjSignal(dev->fd, syncobjs, syncobj_count); } +drm_public int amdgpu_cs_syncobj_timeline_signal(amdgpu_device_handle dev, +const uint32_t *syncobjs, +uint64_t *points, +uint32_t syncobj_count) +{ + if (NULL == dev) + return -EINVAL; + + return drmSyncobjTimelineSignal(dev->fd, syncobjs, + points, syncobj_count); +} + drm_public int amdgpu_cs_syncobj_wait(amdgpu_device_handle dev, uint32_t *handles, unsigned num_handles, int64_t timeout_nsec, unsigned flags, @@ -724,6 +736,62 @@ drm_public int amdgpu_cs_syncobj_import_sync_file(amdgpu_device_handle dev, return drmSyncobjImportSyncFile(dev->fd, syncobj, sync_file_fd); } +drm_public int amdgpu_cs_syncobj_export_sync_file2(amdgpu_device_handle dev, + uint32_t syncobj, + uint64_t point, + uint32_t flags
[PATCH i-g-t] igt: add timeline test cases v2
v2: adapt to new transfer ioctl Signed-off-by: Chunming Zhou --- include/drm-uapi/drm.h | 33 ++ lib/igt_syncobj.c| 206 lib/igt_syncobj.h| 19 + tests/meson.build|1 + tests/syncobj_timeline.c | 1032 ++ 5 files changed, 1291 insertions(+) create mode 100644 tests/syncobj_timeline.c diff --git a/include/drm-uapi/drm.h b/include/drm-uapi/drm.h index 85c685a2..dcd245d9 100644 --- a/include/drm-uapi/drm.h +++ b/include/drm-uapi/drm.h @@ -731,6 +731,8 @@ struct drm_syncobj_handle { #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0) #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1) +/* wait for time point to become available */ +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE (1 << 2) struct drm_syncobj_wait { __u64 handles; /* absolute timeout */ @@ -741,11 +743,38 @@ struct drm_syncobj_wait { __u32 pad; }; +struct drm_syncobj_timeline_wait { +__u64 handles; +/* wait on specific timeline point for every handles*/ +__u64 points; +/* absolute timeout */ +__s64 timeout_nsec; +__u32 count_handles; +__u32 flags; +__u32 first_signaled; /* only valid when not waiting all */ +__u32 pad; +}; + struct drm_syncobj_array { __u64 handles; __u32 count_handles; __u32 pad; }; +struct drm_syncobj_timeline_array { + __u64 handles; + __u64 points; + __u32 count_handles; + __u32 pad; +}; + +struct drm_syncobj_transfer { +__u32 src_handle; +__u32 dst_handle; +__u64 src_point; +__u64 dst_point; +__u32 flags; +__u32 pad; +}; /* Query current scanout sequence number */ struct drm_crtc_get_sequence { @@ -902,6 +931,10 @@ extern "C" { #define DRM_IOCTL_MODE_LIST_LESSEESDRM_IOWR(0xC7, struct drm_mode_list_lessees) #define DRM_IOCTL_MODE_GET_LEASE DRM_IOWR(0xC8, struct drm_mode_get_lease) #define DRM_IOCTL_MODE_REVOKE_LEASEDRM_IOWR(0xC9, struct drm_mode_revoke_lease) +#define DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT DRM_IOWR(0xCA, struct drm_syncobj_timeline_wait) +#define DRM_IOCTL_SYNCOBJ_QUERY DRM_IOWR(0xCB, struct drm_syncobj_timeline_array) +#define DRM_IOCTL_SYNCOBJ_TRANSFERDRM_IOWR(0xCC, struct drm_syncobj_transfer) +#define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNALDRM_IOWR(0xCD, struct drm_syncobj_timeline_array) /** * Device specific ioctls should only be in their respective headers diff --git a/lib/igt_syncobj.c b/lib/igt_syncobj.c index d9114ca8..efa2adc4 100644 --- a/lib/igt_syncobj.c +++ b/lib/igt_syncobj.c @@ -286,3 +286,209 @@ syncobj_signal(int fd, uint32_t *handles, uint32_t count) { igt_assert_eq(__syncobj_signal(fd, handles, count), 0); } + +static int +__syncobj_timeline_signal(int fd, uint32_t *handles, uint64_t *points, uint32_t count) +{ + struct drm_syncobj_timeline_array array = { 0 }; + int err = 0; + + array.handles = to_user_pointer(handles); + array.points = to_user_pointer(points); + array.count_handles = count; + if (drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL, &array)) + err = -errno; + return err; +} + +/** + * syncobj_signal: + * @fd: The DRM file descriptor. + * @handles: Array of syncobj handles to signal + * @points: List of point of handles to signal. + * @count: Count of syncobj handles. + * + * Signal a set of syncobjs. + */ +void +syncobj_timeline_signal(int fd, uint32_t *handles, uint64_t *points, uint32_t count) +{ + igt_assert_eq(__syncobj_timeline_signal(fd, handles, points, count), 0); +} +int +__syncobj_timeline_wait_ioctl(int fd, struct drm_syncobj_timeline_wait *args) +{ + int err = 0; + if (drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, args)) + err = -errno; + return err; +} +static int +__syncobj_timeline_wait(int fd, uint32_t *handles, uint64_t *points, + unsigned num_handles, + int64_t timeout_nsec, unsigned flags, + uint32_t *first_signaled) +{ + struct drm_syncobj_timeline_wait args; + int ret; + + args.handles = to_user_pointer(handles); + args.points = (uint64_t)to_user_pointer(points); + args.timeout_nsec = timeout_nsec; + args.count_handles = num_handles; + args.flags = flags; + args.first_signaled = 0; + args.pad = 0; + + ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, &args); + if (ret < 0) + return -errno; + + if (first_signaled) + *first_signaled = args.first_signaled; + + return ret; +} +int +syncobj_timeline_wait_err(int fd, uint32_t *handles, uint64_t *points, + unsigned num_handles, + int64_t timeout_nsec, unsigned flags) +{ + return __syncobj_timeline_wait(fd, handles, points, num_handles, +
Re: [PATCH] drm/sun4i: fix HSYNC and VSYNC polarity
Hi Giulio, On Thu, 6 Dec 2018 at 22:00, Giulio Benetti wrote: > > Hi Jonathan, > > Il 06/12/2018 08:29, Jonathan Liu ha scritto: > > Hi Giulio, > > > > On Thu, 15 Feb 2018 at 17:54, Giulio Benetti > > wrote: > >> > >> Differently from other Lcd signals, HSYNC and VSYNC signals > >> result inverted if their bits are cleared to 0. > >> > >> Invert their settings of IO_POL register. > >> > >> Signed-off-by: Giulio Benetti > >> --- > >> drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> b/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> index 3c15cf2..aaf911a 100644 > >> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> @@ -389,10 +389,10 @@ static void sun4i_tcon0_mode_set_rgb(struct > >> sun4i_tcon *tcon, > >> SUN4I_TCON0_BASIC3_H_SYNC(hsync)); > >> > >> /* Setup the polarity of the various signals */ > >> - if (!(mode->flags & DRM_MODE_FLAG_PHSYNC)) > >> + if (mode->flags & DRM_MODE_FLAG_PHSYNC) > >> val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE; > >> > >> - if (!(mode->flags & DRM_MODE_FLAG_PVSYNC)) > >> + if (mode->flags & DRM_MODE_FLAG_PVSYNC) > >> val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE; > >> > >> regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG, > > > > I am running Linux 4.19.6 and noticed with Olimex LCD-OLinuXino-7TS 7" > > LCD touchscreen (Innolux AT070TN92) connected to Olimex > > A20-OLinuXino-MICRO that the image does not display correctly after > > this change. > > The image is shifted to the right. > > > > Reverting the change results in the image being displayed correctly on > > the screen. > > > > I have in the device tree a "panel" node with compatible = > > "innolux,at070tn92" which uses the timings in > > drivers/gpu/drm/panel/panel-simple.c. > > > > Any ideas? > > Checking Display Datasheet: > https://www.olimex.com/Products/Retired/A13-LCD7-TS/resources/S700-AT070TN92.pdf > > Page 13 section 3.3.2 you can see it needs active low VS and HS. > > You can refer to this Thread and check scope captures about VS and HS > versus TCON0_IOPOL register: > https://lists.freedesktop.org/archives/dri-devel/2018-January/163874.html > > There should be something that wrongly sets one of these or both: > mode->flags |= DRM_MODE_FLAG_PHSYNC; > and/or > mode->flags |= DRM_MODE_FLAG_PVSYNC; > > Checked in panel-simple.c but it's not there. flags is 0 because it is not assigned in the struct definition for the panel. Before this change, TCON0_IO_POL_REG would be 0x0300 (both bits set to 1) and image displays correctly. After this change, TCON0_IO_POL_REG is 0x (both bits set to 0) and image doesn't display correctly. Checked using "devmem2 0x01c0c088" on A20-OLinuXino-MICRO Rev J. > > At the moment I don't have a board to check it with me, I'll try to do > it soon, but I'm full with other work at the moment. > > If you have the chance to debug you could try to find in which point > mode->flags gets changed with those 2 flags. > > When the problem came out I've noticed the same thing for u-boot too but > it's been decided to not patch it because in that case every single > sunxi defconfig had to be changed from: > CONFIG_VIDEO_LCD_MODE="...,sync:3,..." > to: > CONFIG_VIDEO_LCD_MODE="...,sync:0,..." > and it would have been a great mess, probably not worth it: > https://lists.denx.de/pipermail/u-boot/2018-February/321405.html > > So keep in mind that TCON driver works differently for HS and > VS(inverted) polarity in u-boot and linux. > > Hope to work this out soon. Regards, Jonathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108919] Parkitect (Unity Game) dispalys artifacts and black screens with Vega hardware
https://bugs.freedesktop.org/show_bug.cgi?id=108919 --- Comment #9 from Alexander Walker --- Created attachment 142775 --> https://bugs.freedesktop.org/attachment.cgi?id=142775&action=edit renderdoc capture -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] [RFC] MAINTAINERS: Daniel for drm co-maintainer
Hi Daniel, On Mon, Dec 10, 2018 at 11:30:01AM +0100, Daniel Vetter wrote: > lkml and Linus gained a CoC, and it's serious this time. Which means > my no 1 reason for declining to officially step up as drm maintainer > is gone, and I didn't find any new good excuse. I wish you best of luck in the new role! > > I chatted with a few people in private already, and the biggest > concern is that I mislay my community hat and start running around > with my intel hat only. Or some other convenient abuse of trust. > > That's why this patch doesn't just need a lot of acks that mean "yeah > seems fine to me", but a lot of acks that mean "yeah we'll tell you > when you're over the line and usurp you from that comfy chair if you > don't get it". Which I think we've been done a fairly good job here at > dri-devel in general, but better to be clear. Maybe a good step would be to arrange for you (and Dave?) to get some non-Intel hardware that you could play with? > > Rough idea is that I'll do this for maybe 2-3 years, helping Dave > figure out a group model for drm overall. And getting the tooling and > infrastructure for that off the ground. Then step down again because > some other shiny thing that needs chasing. Of course as plans tend to > do, this one will probably pan out a bit different in reality. > > Cc: David Airlie > Cc: Linus Torvalds > Signed-off-by: Daniel Vetter Before I sign off anything I would like to get some clarity on these email addresses. :) Which one is the primary one and are you using it for signing off? Best regards, Liviu > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7e05aa20b0ab..2c4cd038df2a 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4849,6 +4849,7 @@ F: include/uapi/drm/vmwgfx_drm.h > > DRM DRIVERS > M: David Airlie > +M: Daniel Vetter > L: dri-devel@lists.freedesktop.org > T: git git://anongit.freedesktop.org/drm/drm > B: https://bugs.freedesktop.org/ > -- > 2.20.0.rc1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- With enough courage, you can do without a reputation. -- Rhett Butler ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108919] Parkitect (Unity Game) dispalys artifacts and black screens with Vega hardware
https://bugs.freedesktop.org/show_bug.cgi?id=108919 --- Comment #10 from Alexander Walker --- (In reply to Timothy Arceri from comment #8) > Do you think you could try grabbing a capture in renderdoc [1]? Once you > unzip it somewhere you can add the following to the launch options for the > game in steam: > > LD_PRELOAD=/your path to renderdoc here/renderdoc_1.2/lib/librenderdoc.so > %command% > > When you lanuch the game you should see some text overlayed at the top of > the screen saying you can create a capture by pressing F12. Once you have a > capture you can find it in a folder in the /tmp/ directory. > > [1] https://renderdoc.org/ I've attached a capture. It's a flickering issue so hopefully it was captured in that 1 frame. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 102646] Screen flickering under amdgpu-experimental [buggy auto power profile]
https://bugs.freedesktop.org/show_bug.cgi?id=102646 --- Comment #58 from tempel.jul...@gmail.com --- I've written a small script to only write into pp_dpm_mclk when it's not forced into pstate 2: #!/bin/bash if ! cat /sys/class/drm/card0/device/pp_dpm_mclk | grep -xqFe "2: 2250Mhz *" then echo "2" > /sys/class/drm/card0/device/pp_dpm_mclk fi --- Can be restarted every 1s via systemd: [Unit] Description=watch-pp_dpm_mclk StartLimitIntervalSec=0 [Service] ExecStart=.../watch-pp_dpm_mclk.sh Restart=always RestartSec=1 [Install] WantedBy=multi-user.target --- Seems to do the trick without nasty side effects. Real fixes for the memory clock change flicker and the not sticking pp_dpm_mclk value would of course be better. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/a5xx: Build a5xx_gpu_state_(get/put) under the right conditionals
Build the GPU crashstate capture functions only if either of CONFIG_DEBUG_FS, CONFIG_DEV_COREDUMP is defined. Signed-off-by: Sharat Masetty --- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c index d5f5e56..81014b5 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c @@ -1328,6 +1328,7 @@ static void a5xx_gpu_state_get_hlsq_regs(struct msm_gpu *gpu, msm_gem_kernel_put(dumper.bo, gpu->aspace, true); } +#if defined(CONFIG_DEBUG_FS) || defined(CONFIG_DEV_COREDUMP) static struct msm_gpu_state *a5xx_gpu_state_get(struct msm_gpu *gpu) { struct a5xx_gpu_state *a5xx_state = kzalloc(sizeof(*a5xx_state), @@ -1373,8 +1374,6 @@ int a5xx_gpu_state_put(struct msm_gpu_state *state) return kref_put(&state->ref, a5xx_gpu_state_destroy); } - -#if defined(CONFIG_DEBUG_FS) || defined(CONFIG_DEV_COREDUMP) void a5xx_show(struct msm_gpu *gpu, struct msm_gpu_state *state, struct drm_printer *p) { @@ -1456,13 +1455,13 @@ static unsigned long a5xx_gpu_busy(struct msm_gpu *gpu) .destroy = a5xx_destroy, #if defined(CONFIG_DEBUG_FS) || defined(CONFIG_DEV_COREDUMP) .show = a5xx_show, + .gpu_state_get = a5xx_gpu_state_get, + .gpu_state_put = a5xx_gpu_state_put, #endif #if defined(CONFIG_DEBUG_FS) .debugfs_init = a5xx_debugfs_init, #endif .gpu_busy = a5xx_gpu_busy, - .gpu_state_get = a5xx_gpu_state_get, - .gpu_state_put = a5xx_gpu_state_put, }, .get_timestamp = a5xx_get_timestamp, }; -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108919] Parkitect (Unity Game) dispalys artifacts and black screens with Vega hardware
https://bugs.freedesktop.org/show_bug.cgi?id=108919 --- Comment #11 from Alexander Walker --- Created attachment 142776 --> https://bugs.freedesktop.org/attachment.cgi?id=142776&action=edit renderdoc black screen from toggling vsync -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm/bridge: ti-tfp410: Set connector type based on DT connector node
On 06/12/2018 22:26, Laurent Pinchart wrote: > The TI TFP410 is a DVI encoder, not a full HDMI encoder. Its output can > be routed to a DVI-D connector, even if in many cases embedded systems > will use an HDMI connector to carry the DVI signals. > > Instead of hardcoding the connector type to HDMI, retrieve the connector > type from its DT node. > > Signed-off-by: Laurent Pinchart > --- Reviewed-by: Jyri Sarha > drivers/gpu/drm/bridge/ti-tfp410.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c > b/drivers/gpu/drm/bridge/ti-tfp410.c > index c3e32138c6bb..e4280f5af9f5 100644 > --- a/drivers/gpu/drm/bridge/ti-tfp410.c > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c > @@ -27,6 +27,7 @@ > struct tfp410 { > struct drm_bridge bridge; > struct drm_connectorconnector; > + unsigned intconnector_type; > > struct i2c_adapter *ddc; > struct gpio_desc*hpd; > @@ -126,7 +127,7 @@ static int tfp410_attach(struct drm_bridge *bridge) > drm_connector_helper_add(&dvi->connector, >&tfp410_con_helper_funcs); > ret = drm_connector_init(bridge->dev, &dvi->connector, > - &tfp410_con_funcs, DRM_MODE_CONNECTOR_HDMIA); > + &tfp410_con_funcs, dvi->connector_type); > if (ret) { > dev_err(dvi->dev, "drm_connector_init() failed: %d\n", ret); > return ret; > @@ -172,6 +173,11 @@ static int tfp410_get_connector_properties(struct tfp410 > *dvi) > if (!connector_node) > return -ENODEV; > > + if (of_device_is_compatible(connector_node, "hdmi-connector")) > + dvi->connector_type = DRM_MODE_CONNECTOR_HDMIA; > + else > + dvi->connector_type = DRM_MODE_CONNECTOR_DVID; > + > dvi->hpd = fwnode_get_named_gpiod(&connector_node->fwnode, > "hpd-gpios", 0, GPIOD_IN, "hpd"); > if (IS_ERR(dvi->hpd)) { > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm/bridge: ti-tfp410: Add support for the powerdown GPIO
On 06/12/2018 22:26, Laurent Pinchart wrote: > The TFP410 has a powerdown pin that can be connected to a GPIO to > control power saving. The DT bindings define a corresponding property, > but the driver doesn't implement support for it. Fix that. > > Signed-off-by: Laurent Pinchart > --- I find the naming of the the gpio bit backwards, but I guess it si fine since it is also the name of the pin. Reviewed-by: Jyri Sarha > drivers/gpu/drm/bridge/ti-tfp410.c | 24 > 1 file changed, 24 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c > b/drivers/gpu/drm/bridge/ti-tfp410.c > index e4280f5af9f5..d25d23cfe3f5 100644 > --- a/drivers/gpu/drm/bridge/ti-tfp410.c > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c > @@ -32,6 +32,7 @@ struct tfp410 { > struct i2c_adapter *ddc; > struct gpio_desc*hpd; > struct delayed_work hpd_work; > + struct gpio_desc*powerdown; > > struct device *dev; > }; > @@ -139,8 +140,24 @@ static int tfp410_attach(struct drm_bridge *bridge) > return 0; > } > > +static void tfp410_enable(struct drm_bridge *bridge) > +{ > + struct tfp410 *dvi = drm_bridge_to_tfp410(bridge); > + > + gpiod_set_value_cansleep(dvi->powerdown, 0); > +} > + > +static void tfp410_disable(struct drm_bridge *bridge) > +{ > + struct tfp410 *dvi = drm_bridge_to_tfp410(bridge); > + > + gpiod_set_value_cansleep(dvi->powerdown, 1); > +} > + > static const struct drm_bridge_funcs tfp410_bridge_funcs = { > .attach = tfp410_attach, > + .enable = tfp410_enable, > + .disable= tfp410_disable, > }; > > static void tfp410_hpd_work_func(struct work_struct *work) > @@ -229,6 +246,13 @@ static int tfp410_init(struct device *dev) > if (ret) > goto fail; > > + dvi->powerdown = devm_gpiod_get_optional(dev, "powerdown", > + GPIOD_OUT_HIGH); > + if (IS_ERR(dvi->powerdown)) { > + dev_err(dev, "failed to parse powerdown gpio\n"); > + return PTR_ERR(dvi->powerdown); > + } > + > if (dvi->hpd) { > INIT_DELAYED_WORK(&dvi->hpd_work, tfp410_hpd_work_func); > > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/vc4: Add a debugfs entry to disable/enable the load tracker
On Fri, 7 Dec 2018 14:47:57 +0100 Paul Kocialkowski wrote: > diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c > index f3fd34faa812..e80b1ad5c938 100644 > --- a/drivers/gpu/drm/vc4/vc4_kms.c > +++ b/drivers/gpu/drm/vc4/vc4_kms.c > @@ -479,6 +479,7 @@ static const struct drm_private_state_funcs > vc4_load_tracker_state_funcs = { > static int > vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) > { > + struct vc4_dev *vc4 = to_vc4_dev(dev); > int ret; > > ret = vc4_ctm_atomic_check(dev, state); > @@ -489,7 +490,10 @@ vc4_atomic_check(struct drm_device *dev, struct > drm_atomic_state *state) > if (ret) > return ret; > > - return vc4_load_tracker_atomic_check(state); > + if (vc4->load_tracker_enabled) > + return vc4_load_tracker_atomic_check(state); Even if we don't return an error when ->load_tracker_enabled is false, we should keep updating the ->{hvs,membus}_load state, otherwise those fields will contain bad values if we activate the load tracker after the planes have been enabled. I suggest moving this test in vc4_load_tracker_atomic_check() just before if (load_state->membus_load > SZ_1G + SZ_512M) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm/exynos: add support for dynamic zpos in DECON and FIMD
Hi again, It seems I have missed two questions: On 11.12.2018 09:36, Andrzej Hajda wrote: > On 11.12.2018 09:01, Inki Dae wrote: >> 18. 12. 11. 오후 4:49에 Andrzej Hajda 이(가) 쓴 글: >>> On 11.12.2018 00:45, Inki Dae wrote: Hi Andrzej, 18. 12. 10. 오후 4:35에 Andrzej Hajda 이(가) 쓴 글: > Hi Inki, > > On 10.12.2018 03:25, Inki Dae wrote: >> Hi Andrzej, >> >> 18. 12. 6. 오후 6:38에 Andrzej Hajda 이(가) 쓴 글: >>> Hi Inki, >>> >>> This small patchset adds dynamic zpos support for DECON and FIMD. >> This patch will allow user space to change zpos. However, DECON and FIMD >> devices have fixed priority of HW overlays. >> This would mean that zpos change by user space doesn't guarantee correct >> HW overlay priority. >> >> Why do you want to support mutable zpos? > Practically you have patches which proves it works, you can see that > changing zpos value results in adequate change in displayed image. > > Conceptually it is just a matter of disconnecting hardware windows > present in DECON and FIMD from DRM planes which are software entities. > > You can reason about it this way: > > - drm plane is a framebuffer plus informations how it should be > transformed/displayed on DECON/FIMD, > > - hw window in DECON/FIMD is just a channel through which plane is send > to the display. > > DECON and FIMD has fixed hw windows order - windows with lower numbers > are displayed below windows with higher number. To display planes in > given z-order you just need to send them via windows with appropriate > index - farthest plane should go always via win0, closer one via win1, > ..., till the last plane. > > So for example if you have three planes and want to display them in > following order (first one farthest, last one closest): > > plane2, plane1, plane3 > > you should map them to planes as follow: > > plane2 -> win0, plane1 -> win1, plane3 -> win2 > > then for example plane2 is disabled, we will have following mapping: > > plane1 -> win0, plane3 -> win1, win2 - disabled Plane1 is displayed below Plane3. >>> And this is what we wanted, the initial order was: plane2, plane1, >>> plane3, first farthest (or lowest if you prefer this naming schema). >>> >>> > then if you change zorder of planes to: plane3, plane1: > > plane3 -> win0, plane1 -> win1 Plane3 is displayed below Plane1 because DECON/FIMD HW aren't able to change HW overlay priroty. >>> No, plane3 is displayed below plane1 because we sent it via win0, if we >>> want inverse we can send plane3 via win1 and plane1 via win0. >>> >>> However, user space wanted that Plane1 is displayed below Plane3. Like this, zpos change by user space doesn't guarantee correct HW overlay priority. >>> As I said before in this example userspace wanted plane3 below plane1, >>> and as I wrote in comment above any order of planes user want driver is >>> able to realize with this patch. >>> >>> And there is no any reason to change zpos in user space excepting Mixer device which supports HW overlay priority change. >>> Lack of special registry for manipulating windows order does not mean it >>> cannot support dynamic zpos. >>> >> Why changing zpos in user space is required? I didn't say it is required, but usually it is better if the driver supports more features than less. Regards Andrzej >> In fact, we supported mutable zpos before but changed it to immutable with same reason. >>> It was just broken if I remember correctly. >>> >> No, I worked well and real user used it I remember. I tried to find the code in git history, I have found some pre-atomic patches for zpos. It is not clear how it worked and why mutable zpos has been removed. Do you remember reasons? Regards Andrzej >> Lastly, your patch made real user broken. >>> Who? How? >> Window server of Tizen didn't work and you can test it with below image - I >> didn't check why it doesn't. Anyway, we don't have to break real user. >> http://download.tizen.org/snapshots/tizen/unified/latest/images/standard/mobile-wayland-armv7l-tm2/ > > Are you saying it works with latest mainline without my patches? > > > Regards > > Andrzej > > >> Thanks, >> Inki Dae >> >>> Regards >>> >>> Andrzej >>> >>> Thanks, Inki Dae > As you see there is no relation between plane number and window number, > but window number is equal to plane's position in zpos-ordered list of > planes and this is exact value of normalized_zpos field in > drm_plane_state. > > > I hope this extended explanation will clarify things. > > > Regards > > Andrzej > > > >> Thanks, >> Inki Dae >> >>> It was tested on tm2 and trats2. >>> >>> Regards >>> Andrzej >>> >>> >>> Andrzej Hajda (3): >>> drm/e
Re: [PATCH] drm/dp: Set the MOT bit for Write_Status_Update_Request transactions
On Mon, Dec 10, 2018 at 02:15:11PM -0800, Dhinakaran Pandiyan wrote: > On Mon, 2018-12-10 at 23:29 +0200, Ville Syrjälä wrote: > > On Mon, Dec 10, 2018 at 01:07:49PM -0800, Dhinakaran Pandiyan wrote: > > > The Write_Status_Update_Request I2C transaction requires the MOT > > > bit to > > > be set, Change the logical AND to OR to fix what looks like a typo. > > > > It's not a type. We're just preserving MOT. What makes you think it > > should always be set? > > > The table defining request commands (2-148) has the MOT bit set for > Write_Status_Update_Request, doesn't make it look like an option when > querying the status. Checking the callers again, I see that we could > get a defer when ending an i2c transaction and that will require a > Write_Status_Update_Request with MOT unset. Sorry for the noise. Or a short reply. Admittedly the spec is a bit vague on this topic, but after trawling it thorougly again I spotted that table 2-151 #9 does have an example of write_status_update with mot==0. So I guess my original interpretation was in fact correct. Phew :) > > > > > > > > > > Cc: dri-devel@lists.freedesktop.org > > > Cc: Jani Nikula > > > Cc: Ville Syrjälä > > > Fixes: 68ec2a2a2481 ("drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain > > > partial I2C_WRITE requests") > > > Signed-off-by: Dhinakaran Pandiyan > > > --- > > > drivers/gpu/drm/drm_dp_helper.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/drm_dp_helper.c > > > b/drivers/gpu/drm/drm_dp_helper.c > > > index 2d6c491a0542..d98805b517f0 100644 > > > --- a/drivers/gpu/drm/drm_dp_helper.c > > > +++ b/drivers/gpu/drm/drm_dp_helper.c > > > @@ -677,7 +677,7 @@ static void > > > drm_dp_i2c_msg_write_status_update(struct drm_dp_aux_msg *msg) > > >* rest of the message > > >*/ > > > if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE) { > > > - msg->request &= DP_AUX_I2C_MOT; > > > + msg->request |= DP_AUX_I2C_MOT; > > > msg->request |= DP_AUX_I2C_WRITE_STATUS_UPDATE; > > > } > > > } > > > -- > > > 2.17.1 > > > > -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/6] sparc: merge 32-bit and 64-bit version of pci.h
I've pulled this into the dma-mapping for-next tree now. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm/bridge: ti-tfp410: Add support for the powerdown GPIO
Hi Jyri, On Tuesday, 11 December 2018 15:01:52 EET Jyri Sarha wrote: > On 06/12/2018 22:26, Laurent Pinchart wrote: > > The TFP410 has a powerdown pin that can be connected to a GPIO to > > control power saving. The DT bindings define a corresponding property, > > but the driver doesn't implement support for it. Fix that. > > > > Signed-off-by: Laurent Pinchart > > --- > > I find the naming of the the gpio bit backwards, but I guess it si fine > since it is also the name of the pin. I would have gone for "enable-gpios", but given that we have DTs in the wild using the "powerdown-gpios" property (with the omapdrm-specific TFP410 driver which uses the same compatible string as the drm_bridge-based driver) I think we need to stick to it. As you mentioned this is the pin's name, so it's not too big of a deal. I'll now wait for your review of patch 5/5 before merging these changes :-) The plan is to get them in drm-misc for v4.22, please let me know if that causes any problem for you. Also, please feel free to review 1/5 and 2/5 if you have time. > Reviewed-by: Jyri Sarha > > > drivers/gpu/drm/bridge/ti-tfp410.c | 24 > > 1 file changed, 24 insertions(+) > > > > diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c > > b/drivers/gpu/drm/bridge/ti-tfp410.c index e4280f5af9f5..d25d23cfe3f5 > > 100644 > > --- a/drivers/gpu/drm/bridge/ti-tfp410.c > > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c > > @@ -32,6 +32,7 @@ struct tfp410 { > > struct i2c_adapter *ddc; > > struct gpio_desc*hpd; > > struct delayed_work hpd_work; > > + struct gpio_desc*powerdown; > > struct device *dev; > > }; > > > > @@ -139,8 +140,24 @@ static int tfp410_attach(struct drm_bridge *bridge) > > return 0; > > } > > > > +static void tfp410_enable(struct drm_bridge *bridge) > > +{ > > + struct tfp410 *dvi = drm_bridge_to_tfp410(bridge); > > + > > + gpiod_set_value_cansleep(dvi->powerdown, 0); > > +} > > + > > +static void tfp410_disable(struct drm_bridge *bridge) > > +{ > > + struct tfp410 *dvi = drm_bridge_to_tfp410(bridge); > > + > > + gpiod_set_value_cansleep(dvi->powerdown, 1); > > +} > > + > > static const struct drm_bridge_funcs tfp410_bridge_funcs = { > > .attach = tfp410_attach, > > + .enable = tfp410_enable, > > + .disable= tfp410_disable, > > }; > > > > static void tfp410_hpd_work_func(struct work_struct *work) > > @@ -229,6 +246,13 @@ static int tfp410_init(struct device *dev) > > if (ret) > > goto fail; > > > > + dvi->powerdown = devm_gpiod_get_optional(dev, "powerdown", > > +GPIOD_OUT_HIGH); > > + if (IS_ERR(dvi->powerdown)) { > > + dev_err(dev, "failed to parse powerdown gpio\n"); > > + return PTR_ERR(dvi->powerdown); > > + } > > + > > if (dvi->hpd) { > > INIT_DELAYED_WORK(&dvi->hpd_work, tfp410_hpd_work_func); -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v3 11/19] kunit: add Python libraries for handing KUnit config and kernel
On Tue, 11 Dec 2018 15:09:26 +0100 Petr Mladek wrote: > > We have liburcu already, which is good. The main sticking points are: > > > > - printk has started adding a lot of %pX enhancements which printf > >obviously doesn't know about. > > I wonder how big problem it is and if it is worth using another > approach. No, please do not change the %pX approach. > > An alternative would be to replace them with helper functions > the would produce the same string. The meaning would be easier > to understand. But concatenating with the surrounding text > would be less elegant. People might start using pr_cont() > that is problematic (mixed lines). > > Also the %pX formats are mostly used to print context of some > structures. Even the helper functions would need some maintenance > to keep them compatible. > > BTW: The printk() feature has been introduced 10 years ago by > the commit 4d8a743cdd2690c0bc8 ("vsprintf: add infrastructure > support for extended '%p' specifiers"). trace-cmd and perf know about most of the %pX data and how to read it. Perhaps we can extend the libtraceevent library to export a generic way to read data from printk() output for other tools to use. -- Steve ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/atomic: integrate modeset lock with private objects
On Mon, 22 Oct 2018 14:31:22 +0200 Boris Brezillon wrote: > From: Rob Clark > > Follow the same pattern of locking as with other state objects. This > avoids boilerplate in the driver. > > Signed-off-by: Rob Clark > Signed-off-by: Boris Brezillon > Reviewed-by: Daniel Vetter Queued to drm-misc-next. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] drm/bridge: ti-tfp410: Report input bus config through bridge timings
On 06/12/2018 22:26, Laurent Pinchart wrote: > The TFP410 supports configurable pixel clock sampling edge and data > de-skew adjustments. The configuration can be set through I2C or > dedicated chip pins. > > Report the configuration through the drm_bridge timings. As the > ti-tftp410 driver doesn't support configuring the chip through I2C, we > simply use the default configuration in that case. When the chip is > configured through dedicated pins, we parse the configuration from DT. > > Signed-off-by: Laurent Pinchart > --- > drivers/gpu/drm/bridge/ti-tfp410.c | 77 -- > 1 file changed, 74 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c > b/drivers/gpu/drm/bridge/ti-tfp410.c > index d25d23cfe3f5..48ec647a7526 100644 > --- a/drivers/gpu/drm/bridge/ti-tfp410.c > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c > @@ -34,6 +34,8 @@ struct tfp410 { > struct delayed_work hpd_work; > struct gpio_desc*powerdown; > > + struct drm_bridge_timings timings; > + > struct device *dev; > }; > > @@ -180,6 +182,70 @@ static irqreturn_t tfp410_hpd_irq_thread(int irq, void > *arg) > return IRQ_HANDLED; > } > > +static const struct drm_bridge_timings tfp410_default_timings = { > + .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE > + | DRM_BUS_FLAG_DE_HIGH, > + .setup_time_ps = 1200, > + .hold_time_ps = 1300, > +}; > + > +static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c) > +{ > + struct drm_bridge_timings *timings = &dvi->timings; > + struct device_node *ep; > + u32 pclk_sample = 0; > + s32 deskew = 0; > + > + /* Start with defaults. */ > + *timings = tfp410_default_timings; > + > + if (i2c) > + /* > + * In I2C mode timings are configured through the I2C interface. > + * As the driver doesn't support I2C configuration yet, we just > + * go with the defaults (BSEL=1, DSEL=1, DKEN=0, EDGE=1). > + */ > + return 0; > + > + /* > + * In non-I2C mode, timings are configured through the BSEL, DSEL, DKEN > + * and EDGE pins. They are specified in DT through endpoint properties > + * and vendor-specific properties. > + */ > + ep = of_graph_get_endpoint_by_regs(dvi->dev->of_node, 0, 0); > + if (!ep) > + return -EINVAL; > + > + /* Get the sampling edge from the endpoint. */ > + of_property_read_u32(ep, "pclk-sample", &pclk_sample); > + of_node_put(ep); > + > + timings->input_bus_flags = DRM_BUS_FLAG_DE_HIGH; > + > + switch (pclk_sample) { > + case 0: > + timings->input_bus_flags |= DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE > + | DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE; > + break; > + case 1: > + timings->input_bus_flags |= DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE > + | DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE; > + break; > + default: > + return -EINVAL; > + } > + > + /* Get the setup and hold time from vendor-specific properties. */ > + of_property_read_u32(dvi->dev->of_node, "ti,deskew", (u32 *)&deskew); > + if (deskew < -4 || deskew > 3) > + return -EINVAL; Is "ti,deskew" property documented somewhere? If it is, I could not find it. Could it be added to the tfp410 binding document, or somewhere else? Othewise: Reviewed-by: Jyri Sarha > + > + timings->setup_time_ps = min(0, 1200 - 350 * deskew); > + timings->hold_time_ps = min(0, 1300 + 350 * deskew); > + > + return 0; > +} > + > static int tfp410_get_connector_properties(struct tfp410 *dvi) > { > struct device_node *connector_node, *ddc_phandle; > @@ -223,7 +289,7 @@ static int tfp410_get_connector_properties(struct tfp410 > *dvi) > return ret; > } > > -static int tfp410_init(struct device *dev) > +static int tfp410_init(struct device *dev, bool i2c) > { > struct tfp410 *dvi; > int ret; > @@ -240,8 +306,13 @@ static int tfp410_init(struct device *dev) > > dvi->bridge.funcs = &tfp410_bridge_funcs; > dvi->bridge.of_node = dev->of_node; > + dvi->bridge.timings = &dvi->timings; > dvi->dev = dev; > > + ret = tfp410_parse_timings(dvi, i2c); > + if (ret) > + goto fail; > + > ret = tfp410_get_connector_properties(dvi); > if (ret) > goto fail; > @@ -294,7 +365,7 @@ static int tfp410_fini(struct device *dev) > > static int tfp410_probe(struct platform_device *pdev) > { > - return tfp410_init(&pdev->dev); > + return tfp410_init(&pdev->dev, false); > } > > static int tfp410_remove(struct platform_device *pdev) > @@ -331,7 +402,7 @@ static int tfp410_i2c_probe(struct i2c_client *client, > return -ENXIO; > } > > - return tfp410_init(&client->dev); > + return tfp410_ini
[Bug 201957] amdgpu: ring gfx timeout
https://bugzilla.kernel.org/show_bug.cgi?id=201957 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) --- This is more likely a mesa issue than a kernel issue. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency
Hi Laurent, On 04/12/2018 16:36, Laurent Pinchart wrote: > Implement a .mode_valid() handler in the R-Car glue layer to reject > modes with an unsupported clock frequency. Thank you, I believe my concerns were addressed; (and my misunderstandings are now understandings) > Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham > --- > drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c | 15 +++ > 1 file changed, 15 insertions(+) > > Changes since v1: > > - Add a comment to explain where the limit comes from > > diff --git a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c > b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c > index 75490a3e0a2a..603bb340e8cf 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c > +++ b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c > @@ -35,6 +35,20 @@ static const struct rcar_hdmi_phy_params > rcar_hdmi_phy_params[] = { > { ~0UL, 0x, 0x, 0x }, > }; > > +static enum drm_mode_status > +rcar_hdmi_mode_valid(struct drm_connector *connector, > + const struct drm_display_mode *mode) > +{ > + /* > + * The maximum supported clock frequency is 297 MHz, as shown in the PHY > + * parameters table. > + */ > + if (mode->clock > 297000) It's still a bit of a shame that it's not clear that the mode->clock is expressed in KHz (unless you already know that, which I didn't) - but I'll let that slide for now. -- KB > + return MODE_CLOCK_HIGH; > + > + return MODE_OK; > +} > + > static int rcar_hdmi_phy_configure(struct dw_hdmi *hdmi, > const struct dw_hdmi_plat_data *pdata, > unsigned long mpixelclock) > @@ -59,6 +73,7 @@ static int rcar_hdmi_phy_configure(struct dw_hdmi *hdmi, > } > > static const struct dw_hdmi_plat_data rcar_dw_hdmi_plat_data = { > + .mode_valid = rcar_hdmi_mode_valid, > .configure_phy = rcar_hdmi_phy_configure, > }; > > -- Regards -- Kieran ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] drm/bridge: ti-tfp410: Report input bus config through bridge timings
On 11/12/18 16:48, Jyri Sarha wrote: >> +/* Get the setup and hold time from vendor-specific properties. */ >> +of_property_read_u32(dvi->dev->of_node, "ti,deskew", (u32 *)&deskew); >> +if (deskew < -4 || deskew > 3) >> +return -EINVAL; > > Is "ti,deskew" property documented somewhere? If it is, I could not find > it. Could it be added to the tfp410 binding document, or somewhere else? Patch 2 of this series. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH for-4.20] Revert "drm/rockchip: Allow driver to be shutdown on reboot/kexec"
Am Mittwoch, 5. Dezember 2018, 19:16:57 CET schrieb Brian Norris: > This reverts commit 7f3ef5dedb146e3d5063b6845781ad1bb59b92b5. > > It causes new warnings [1] on shutdown when running the Google Kevin or > Scarlet (RK3399) boards under Chrome OS. Presumably our usage of DRM is > different than what Marc and Heiko test. > > We're looking at a different approach (e.g., [2]) to replace this, but > IMO the revert should be taken first, as it already propagated to > -stable. > > [1] Report here: > http://lkml.kernel.org/lkml/20181205030127.ga200...@google.com > > WARNING: CPU: 4 PID: 2035 at drivers/gpu/drm/drm_mode_config.c:477 > drm_mode_config_cleanup+0x1c4/0x294 > ... > Call trace: > drm_mode_config_cleanup+0x1c4/0x294 > rockchip_drm_unbind+0x4c/0x8c > component_master_del+0x88/0xb8 > rockchip_drm_platform_remove+0x2c/0x44 > rockchip_drm_platform_shutdown+0x20/0x2c > platform_drv_shutdown+0x2c/0x38 > device_shutdown+0x164/0x1b8 > kernel_restart_prepare+0x40/0x48 > kernel_restart+0x20/0x68 > ... > Memory manager not clean during takedown. > WARNING: CPU: 4 PID: 2035 at drivers/gpu/drm/drm_mm.c:950 > drm_mm_takedown+0x34/0x44 > ... > drm_mm_takedown+0x34/0x44 > rockchip_drm_unbind+0x64/0x8c > component_master_del+0x88/0xb8 > rockchip_drm_platform_remove+0x2c/0x44 > rockchip_drm_platform_shutdown+0x20/0x2c > platform_drv_shutdown+0x2c/0x38 > device_shutdown+0x164/0x1b8 > kernel_restart_prepare+0x40/0x48 > kernel_restart+0x20/0x68 > ... > > [2] https://patchwork.kernel.org/patch/10556151/ > https://www.spinics.net/lists/linux-rockchip/msg21342.html > [PATCH] drm/rockchip: shutdown drm subsystem on shutdown > > Fixes: 7f3ef5dedb14 ("drm/rockchip: Allow driver to be shutdown on > reboot/kexec") > Cc: Jeffy Chen > Cc: Robin Murphy > Cc: Vicente Bergas > Cc: Marc Zyngier > Cc: Heiko Stuebner > Cc: sta...@vger.kernel.org > Signed-off-by: Brian Norris So, I've applied this revert to drm-misc-fixes now, so it should still land in 4.20-rc. I'll resurrect Vicentes patch for a later regular inclusion as well, but with more testing involved. Heiko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] drm/bridge: ti-tfp410: Report input bus config through bridge timings
On 11/12/2018 17:19, Tomi Valkeinen wrote: > On 11/12/18 16:48, Jyri Sarha wrote: > >>> + /* Get the setup and hold time from vendor-specific properties. */ >>> + of_property_read_u32(dvi->dev->of_node, "ti,deskew", (u32 *)&deskew); >>> + if (deskew < -4 || deskew > 3) >>> + return -EINVAL; >> >> Is "ti,deskew" property documented somewhere? If it is, I could not find >> it. Could it be added to the tfp410 binding document, or somewhere else? > > Patch 2 of this series. > Hrrm, ok then. Should have applied the patches. Then just: Reviewed-by: Jyri Sarha -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] drm/bridge: ti-tfp410: Report input bus config through bridge timings
Hi Jyri, On Tuesday, 11 December 2018 16:48:16 EET Jyri Sarha wrote: > On 06/12/2018 22:26, Laurent Pinchart wrote: > > The TFP410 supports configurable pixel clock sampling edge and data > > de-skew adjustments. The configuration can be set through I2C or > > dedicated chip pins. > > > > Report the configuration through the drm_bridge timings. As the > > ti-tftp410 driver doesn't support configuring the chip through I2C, we > > simply use the default configuration in that case. When the chip is > > configured through dedicated pins, we parse the configuration from DT. > > > > Signed-off-by: Laurent Pinchart > > --- > > > > drivers/gpu/drm/bridge/ti-tfp410.c | 77 -- > > 1 file changed, 74 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c > > b/drivers/gpu/drm/bridge/ti-tfp410.c index d25d23cfe3f5..48ec647a7526 > > 100644 > > --- a/drivers/gpu/drm/bridge/ti-tfp410.c > > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c > > @@ -34,6 +34,8 @@ struct tfp410 { > > struct delayed_work hpd_work; > > struct gpio_desc*powerdown; > > > > + struct drm_bridge_timings timings; > > + > > struct device *dev; > > }; > > > > @@ -180,6 +182,70 @@ static irqreturn_t tfp410_hpd_irq_thread(int irq, > > void *arg) > > return IRQ_HANDLED; > > } > > > > +static const struct drm_bridge_timings tfp410_default_timings = { > > + .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE > > +| DRM_BUS_FLAG_DE_HIGH, > > + .setup_time_ps = 1200, > > + .hold_time_ps = 1300, > > +}; > > + > > +static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c) > > +{ > > + struct drm_bridge_timings *timings = &dvi->timings; > > + struct device_node *ep; > > + u32 pclk_sample = 0; > > + s32 deskew = 0; > > + > > + /* Start with defaults. */ > > + *timings = tfp410_default_timings; > > + > > + if (i2c) > > + /* > > +* In I2C mode timings are configured through the I2C interface. > > +* As the driver doesn't support I2C configuration yet, we just > > +* go with the defaults (BSEL=1, DSEL=1, DKEN=0, EDGE=1). > > +*/ > > + return 0; > > + > > + /* > > +* In non-I2C mode, timings are configured through the BSEL, DSEL, DKEN > > +* and EDGE pins. They are specified in DT through endpoint properties > > +* and vendor-specific properties. > > +*/ > > + ep = of_graph_get_endpoint_by_regs(dvi->dev->of_node, 0, 0); > > + if (!ep) > > + return -EINVAL; > > + > > + /* Get the sampling edge from the endpoint. */ > > + of_property_read_u32(ep, "pclk-sample", &pclk_sample); > > + of_node_put(ep); > > + > > + timings->input_bus_flags = DRM_BUS_FLAG_DE_HIGH; > > + > > + switch (pclk_sample) { > > + case 0: > > + timings->input_bus_flags |= DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE > > +| DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE; > > + break; > > + case 1: > > + timings->input_bus_flags |= DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE > > +| DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + /* Get the setup and hold time from vendor-specific properties. */ > > + of_property_read_u32(dvi->dev->of_node, "ti,deskew", (u32 *)&deskew); > > + if (deskew < -4 || deskew > 3) > > + return -EINVAL; > > Is "ti,deskew" property documented somewhere? If it is, I could not find > it. Could it be added to the tfp410 binding document, or somewhere else? To satisfy your request, I've taken my time machine to rewrite history, and this patch series now includes the bindings documentation update in patch 2/5 ;-) > Othewise: > > Reviewed-by: Jyri Sarha > > > + > > + timings->setup_time_ps = min(0, 1200 - 350 * deskew); > > + timings->hold_time_ps = min(0, 1300 + 350 * deskew); > > + > > + return 0; > > +} > > + > > static int tfp410_get_connector_properties(struct tfp410 *dvi) > > { > struct device_node *connector_node, *ddc_phandle; > > @@ -223,7 +289,7 @@ static int tfp410_get_connector_properties(struct > > tfp410 *dvi) > > return ret; > > } > > > > -static int tfp410_init(struct device *dev) > > +static int tfp410_init(struct device *dev, bool i2c) > > { > > struct tfp410 *dvi; > > int ret; > > @@ -240,8 +306,13 @@ static int tfp410_init(struct device *dev) > > > > dvi->bridge.funcs = &tfp410_bridge_funcs; > > dvi->bridge.of_node = dev->of_node; > > + dvi->bridge.timings = &dvi->timings; > > dvi->dev = dev; > > > > + ret = tfp410_parse_timings(dvi, i2c); > > + if (ret) > > + goto fail; > > + > > ret = tfp410_get_connector_properties(dvi); > > if (ret) > > goto fail; > > @@ -294,7 +365,7 @@ static int tfp410_fini(struct device *dev) > > > > static int tfp410_probe(struct platform_
Re: [PATCH 4/7] drm: Move the legacy kms disable_all helper to crtc helpers
On Mon, Dec 10, 2018 at 10:58:20AM -0500, Alex Deucher wrote: > On Mon, Dec 10, 2018 at 5:04 AM Daniel Vetter wrote: > > > > It's not a core function, and the matching atomic functions are also > > not in the core. Plus the suspend/resume helper is also already there. > > > > Needs a tiny bit of open-coding, but less midlayer beats that I think. > > > > Cc: Sam Bobroff > > Signed-off-by: Daniel Vetter > > Cc: Maarten Lankhorst > > Cc: Maxime Ripard > > Cc: Sean Paul > > Cc: David Airlie > > Cc: Ben Skeggs > > Cc: Alex Deucher > > Cc: "Christian König" > > Cc: "David (ChunMing) Zhou" > > Cc: Rex Zhu > > Cc: Andrey Grodzovsky > > Cc: Huang Rui > > Cc: Shaoyun Liu > > Cc: Monk Liu > > Cc: nouv...@lists.freedesktop.org > > Cc: amd-...@lists.freedesktop.org > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +- > > drivers/gpu/drm/drm_crtc.c | 31 --- > > drivers/gpu/drm/drm_crtc_helper.c | 35 ++ > > drivers/gpu/drm/nouveau/nouveau_display.c | 2 +- > > drivers/gpu/drm/radeon/radeon_display.c| 2 +- > > include/drm/drm_crtc.h | 2 -- > > include/drm/drm_crtc_helper.h | 1 + > > 7 files changed, 39 insertions(+), 36 deletions(-) > > /snip > > diff --git a/drivers/gpu/drm/drm_crtc_helper.c > > b/drivers/gpu/drm/drm_crtc_helper.c > > index a3c81850e755..23159eb494f1 100644 > > --- a/drivers/gpu/drm/drm_crtc_helper.c > > +++ b/drivers/gpu/drm/drm_crtc_helper.c > > @@ -984,3 +984,38 @@ void drm_helper_resume_force_mode(struct drm_device > > *dev) > > drm_modeset_unlock_all(dev); > > } > > EXPORT_SYMBOL(drm_helper_resume_force_mode); > > + > > +/** > > + * drm_helper_force_disable_all - Forcibly turn off all enabled CRTCs > > + * @dev: DRM device whose CRTCs to turn off > > + * > > + * Drivers may want to call this on unload to ensure that all displays are > > + * unlit and the GPU is in a consistent, low power state. Takes modeset > > locks. > > + * > > + * Note: This should only be used by non-atomic legacy drivers. For an > > atomic > > + * version look at drm_atomic_helper_shutdown(). > > + * > > + * Returns: > > + * Zero on success, error code on failure. > > + */ > > +int drm_helper_force_disable_all(struct drm_device *dev) > > Maybe put crtc somewhere in the function name so it's clear what we > are disabling. FWIW, I think it's more clear this way. set_config_internal will turn off everything attached to the crtc, so _everything_ will be disabled in this case. Either way, Reviewed-by: Sean Paul Sean > With that fixed: > Reviewed-by: Alex Deucher > > > +{ > > + struct drm_crtc *crtc; > > + int ret = 0; > > + > > + drm_modeset_lock_all(dev); > > + drm_for_each_crtc(crtc, dev) > > + if (crtc->enabled) { > > + struct drm_mode_set set = { > > + .crtc = crtc, > > + }; > > + > > + ret = drm_mode_set_config_internal(&set); > > + if (ret) > > + goto out; > > + } > > +out: > > + drm_modeset_unlock_all(dev); > > + return ret; > > +} > > +EXPORT_SYMBOL(drm_helper_force_disable_all); > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c > > b/drivers/gpu/drm/nouveau/nouveau_display.c > > index f326ffd86766..5d273a655479 100644 > > --- a/drivers/gpu/drm/nouveau/nouveau_display.c > > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c > > @@ -453,7 +453,7 @@ nouveau_display_fini(struct drm_device *dev, bool > > suspend, bool runtime) > > if (drm_drv_uses_atomic_modeset(dev)) > > drm_atomic_helper_shutdown(dev); > > else > > - drm_crtc_force_disable_all(dev); > > + drm_helper_force_disable_all(dev); > > } > > > > /* disable flip completion events */ > > diff --git a/drivers/gpu/drm/radeon/radeon_display.c > > b/drivers/gpu/drm/radeon/radeon_display.c > > index e6912eb99b42..92332226e5cf 100644 > > --- a/drivers/gpu/drm/radeon/radeon_display.c > > +++ b/drivers/gpu/drm/radeon/radeon_display.c > > @@ -1643,7 +1643,7 @@ void radeon_modeset_fini(struct radeon_device *rdev) > > if (rdev->mode_info.mode_config_initialized) { > > drm_kms_helper_poll_fini(rdev->ddev); > > radeon_hpd_fini(rdev); > > - drm_crtc_force_disable_all(rdev->ddev); > > + drm_helper_force_disable_all(rdev->ddev); > > radeon_fbdev_fini(rdev); > > radeon_afmt_fini(rdev); > > drm_mode_config_cleanup(rdev->ddev); > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > > index b45bec0b7a9c..85abd3fe9e83 100644 > > --- a/include/drm/drm_crtc.h > > +++ b/include/drm/drm_crtc.h > > @@ -1149,8 +1149,6 @@ static inline uint32_t drm_crtc_mask(const struct > > drm_crt
Re: [PATCH 4/7] drm: Move the legacy kms disable_all helper to crtc helpers
On Tue, Dec 11, 2018 at 10:53 AM Sean Paul wrote: > > On Mon, Dec 10, 2018 at 10:58:20AM -0500, Alex Deucher wrote: > > On Mon, Dec 10, 2018 at 5:04 AM Daniel Vetter > > wrote: > > > > > > It's not a core function, and the matching atomic functions are also > > > not in the core. Plus the suspend/resume helper is also already there. > > > > > > Needs a tiny bit of open-coding, but less midlayer beats that I think. > > > > > > Cc: Sam Bobroff > > > Signed-off-by: Daniel Vetter > > > Cc: Maarten Lankhorst > > > Cc: Maxime Ripard > > > Cc: Sean Paul > > > Cc: David Airlie > > > Cc: Ben Skeggs > > > Cc: Alex Deucher > > > Cc: "Christian König" > > > Cc: "David (ChunMing) Zhou" > > > Cc: Rex Zhu > > > Cc: Andrey Grodzovsky > > > Cc: Huang Rui > > > Cc: Shaoyun Liu > > > Cc: Monk Liu > > > Cc: nouv...@lists.freedesktop.org > > > Cc: amd-...@lists.freedesktop.org > > > --- > > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +- > > > drivers/gpu/drm/drm_crtc.c | 31 --- > > > drivers/gpu/drm/drm_crtc_helper.c | 35 ++ > > > drivers/gpu/drm/nouveau/nouveau_display.c | 2 +- > > > drivers/gpu/drm/radeon/radeon_display.c| 2 +- > > > include/drm/drm_crtc.h | 2 -- > > > include/drm/drm_crtc_helper.h | 1 + > > > 7 files changed, 39 insertions(+), 36 deletions(-) > > > > > /snip > > > > diff --git a/drivers/gpu/drm/drm_crtc_helper.c > > > b/drivers/gpu/drm/drm_crtc_helper.c > > > index a3c81850e755..23159eb494f1 100644 > > > --- a/drivers/gpu/drm/drm_crtc_helper.c > > > +++ b/drivers/gpu/drm/drm_crtc_helper.c > > > @@ -984,3 +984,38 @@ void drm_helper_resume_force_mode(struct drm_device > > > *dev) > > > drm_modeset_unlock_all(dev); > > > } > > > EXPORT_SYMBOL(drm_helper_resume_force_mode); > > > + > > > +/** > > > + * drm_helper_force_disable_all - Forcibly turn off all enabled CRTCs > > > + * @dev: DRM device whose CRTCs to turn off > > > + * > > > + * Drivers may want to call this on unload to ensure that all displays > > > are > > > + * unlit and the GPU is in a consistent, low power state. Takes modeset > > > locks. > > > + * > > > + * Note: This should only be used by non-atomic legacy drivers. For an > > > atomic > > > + * version look at drm_atomic_helper_shutdown(). > > > + * > > > + * Returns: > > > + * Zero on success, error code on failure. > > > + */ > > > +int drm_helper_force_disable_all(struct drm_device *dev) > > > > Maybe put crtc somewhere in the function name so it's clear what we > > are disabling. > > FWIW, I think it's more clear this way. set_config_internal will turn off > everything attached to the crtc, so _everything_ will be disabled in this > case. I'm not pressed. RB either way for me as well. Alex > > Either way, > > Reviewed-by: Sean Paul > > Sean > > > With that fixed: > > Reviewed-by: Alex Deucher > > > > > +{ > > > + struct drm_crtc *crtc; > > > + int ret = 0; > > > + > > > + drm_modeset_lock_all(dev); > > > + drm_for_each_crtc(crtc, dev) > > > + if (crtc->enabled) { > > > + struct drm_mode_set set = { > > > + .crtc = crtc, > > > + }; > > > + > > > + ret = drm_mode_set_config_internal(&set); > > > + if (ret) > > > + goto out; > > > + } > > > +out: > > > + drm_modeset_unlock_all(dev); > > > + return ret; > > > +} > > > +EXPORT_SYMBOL(drm_helper_force_disable_all); > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c > > > b/drivers/gpu/drm/nouveau/nouveau_display.c > > > index f326ffd86766..5d273a655479 100644 > > > --- a/drivers/gpu/drm/nouveau/nouveau_display.c > > > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c > > > @@ -453,7 +453,7 @@ nouveau_display_fini(struct drm_device *dev, bool > > > suspend, bool runtime) > > > if (drm_drv_uses_atomic_modeset(dev)) > > > drm_atomic_helper_shutdown(dev); > > > else > > > - drm_crtc_force_disable_all(dev); > > > + drm_helper_force_disable_all(dev); > > > } > > > > > > /* disable flip completion events */ > > > diff --git a/drivers/gpu/drm/radeon/radeon_display.c > > > b/drivers/gpu/drm/radeon/radeon_display.c > > > index e6912eb99b42..92332226e5cf 100644 > > > --- a/drivers/gpu/drm/radeon/radeon_display.c > > > +++ b/drivers/gpu/drm/radeon/radeon_display.c > > > @@ -1643,7 +1643,7 @@ void radeon_modeset_fini(struct radeon_device *rdev) > > > if (rdev->mode_info.mode_config_initialized) { > > > drm_kms_helper_poll_fini(rdev->ddev); > > > radeon_hpd_fini(rdev); > > > - drm_crtc_force_disable_all(rdev->ddev); > > > + drm_helper_force_disable_all(rdev->ddev); > > > radeon_fbdev_fini(
Re: [PATCH v3 2/2] drm/sched: Rework HW fence processing.
A I understand you say that by the time the fence callback runs the job might have already been released, but how so if the job gets released from drm_sched_job_finish work handler in the normal flow - so, after the HW fence (s_fence->parent) cb is executed. Other 2 flows are error use cases where amdgpu_job_free is called directly in which cases I assume the job wasn't submitted to HW. Last flow I see is drm_sched_entity_kill_jobs_cb and here I actually see a problem with the code as it's today - drm_sched_fence_finished is called which will trigger s_fence->finished callback to run which today schedules drm_sched_job_finish which releases the job, but we don't even wait for that and call free_job cb directly from after that which seems wrong to me. Andrey On 12/10/2018 09:45 PM, Zhou, David(ChunMing) wrote: > I don't think adding cb to sched job would work as soon as their lifetime is > different with fence. > Unless you make the sched job reference, otherwise we will get trouble sooner > or later. > > -David > >> -Original Message- >> From: amd-gfx On Behalf Of >> Andrey Grodzovsky >> Sent: Tuesday, December 11, 2018 5:44 AM >> To: dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org; >> ckoenig.leichtzumer...@gmail.com; e...@anholt.net; >> etna...@lists.freedesktop.org >> Cc: Zhou, David(ChunMing) ; Liu, Monk >> ; Grodzovsky, Andrey >> >> Subject: [PATCH v3 2/2] drm/sched: Rework HW fence processing. >> >> Expedite job deletion from ring mirror list to the HW fence signal callback >> instead from finish_work, together with waiting for all such fences to >> signal in >> drm_sched_stop we garantee that already signaled job will not be processed >> twice. >> Remove the sched finish fence callback and just submit finish_work directly >> from the HW fence callback. >> >> v2: Fix comments. >> >> v3: Attach hw fence cb to sched_job >> >> Suggested-by: Christian Koenig >> Signed-off-by: Andrey Grodzovsky >> --- >> drivers/gpu/drm/scheduler/sched_main.c | 58 -- >> >> include/drm/gpu_scheduler.h| 6 ++-- >> 2 files changed, 30 insertions(+), 34 deletions(-) >> >> diff --git a/drivers/gpu/drm/scheduler/sched_main.c >> b/drivers/gpu/drm/scheduler/sched_main.c >> index cdf95e2..f0c1f32 100644 >> --- a/drivers/gpu/drm/scheduler/sched_main.c >> +++ b/drivers/gpu/drm/scheduler/sched_main.c >> @@ -284,8 +284,6 @@ static void drm_sched_job_finish(struct work_struct >> *work) >> cancel_delayed_work_sync(&sched->work_tdr); >> >> spin_lock_irqsave(&sched->job_list_lock, flags); >> -/* remove job from ring_mirror_list */ >> -list_del_init(&s_job->node); >> /* queue TDR for next job */ >> drm_sched_start_timeout(sched); >> spin_unlock_irqrestore(&sched->job_list_lock, flags); @@ -293,22 >> +291,11 @@ static void drm_sched_job_finish(struct work_struct *work) >> sched->ops->free_job(s_job); >> } >> >> -static void drm_sched_job_finish_cb(struct dma_fence *f, >> -struct dma_fence_cb *cb) >> -{ >> -struct drm_sched_job *job = container_of(cb, struct drm_sched_job, >> - finish_cb); >> -schedule_work(&job->finish_work); >> -} >> - >> static void drm_sched_job_begin(struct drm_sched_job *s_job) { >> struct drm_gpu_scheduler *sched = s_job->sched; >> unsigned long flags; >> >> -dma_fence_add_callback(&s_job->s_fence->finished, &s_job- >>> finish_cb, >> - drm_sched_job_finish_cb); >> - >> spin_lock_irqsave(&sched->job_list_lock, flags); >> list_add_tail(&s_job->node, &sched->ring_mirror_list); >> drm_sched_start_timeout(sched); >> @@ -359,12 +346,11 @@ void drm_sched_stop(struct drm_gpu_scheduler >> *sched, struct drm_sched_job *bad, >> list_for_each_entry_reverse(s_job, &sched->ring_mirror_list, node) >> { >> if (s_job->s_fence->parent && >> dma_fence_remove_callback(s_job->s_fence->parent, >> - &s_job->s_fence->cb)) { >> + &s_job->cb)) { >> dma_fence_put(s_job->s_fence->parent); >> s_job->s_fence->parent = NULL; >> atomic_dec(&sched->hw_rq_count); >> -} >> -else { >> +} else { >> /* TODO Is it get/put neccessey here ? */ >> dma_fence_get(&s_job->s_fence->finished); >> list_add(&s_job->finish_node, &wait_list); @@ - >> 417,31 +403,34 @@ EXPORT_SYMBOL(drm_sched_stop); void >> drm_sched_start(struct drm_gpu_scheduler *sched, bool unpark_only) { >> struct drm_sched_job *s_job, *tmp; >> -unsigned long flags; >> int r; >> >> if (unpark_only) >> goto unpark; >> >> -spin_lock_irqsave(&sched->job_list_lock, flags); >> +/* >> + * Locking the list is not
Re: [PATCH v3 2/2] drm/sched: Rework HW fence processing.
Yeah, completely correct explained. I was unfortunately really busy today, but going to give that a look as soon as I have time. Christian. Am 11.12.18 um 17:01 schrieb Grodzovsky, Andrey: A I understand you say that by the time the fence callback runs the job might have already been released, but how so if the job gets released from drm_sched_job_finish work handler in the normal flow - so, after the HW fence (s_fence->parent) cb is executed. Other 2 flows are error use cases where amdgpu_job_free is called directly in which cases I assume the job wasn't submitted to HW. Last flow I see is drm_sched_entity_kill_jobs_cb and here I actually see a problem with the code as it's today - drm_sched_fence_finished is called which will trigger s_fence->finished callback to run which today schedules drm_sched_job_finish which releases the job, but we don't even wait for that and call free_job cb directly from after that which seems wrong to me. Andrey On 12/10/2018 09:45 PM, Zhou, David(ChunMing) wrote: I don't think adding cb to sched job would work as soon as their lifetime is different with fence. Unless you make the sched job reference, otherwise we will get trouble sooner or later. -David -Original Message- From: amd-gfx On Behalf Of Andrey Grodzovsky Sent: Tuesday, December 11, 2018 5:44 AM To: dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org; ckoenig.leichtzumer...@gmail.com; e...@anholt.net; etna...@lists.freedesktop.org Cc: Zhou, David(ChunMing) ; Liu, Monk ; Grodzovsky, Andrey Subject: [PATCH v3 2/2] drm/sched: Rework HW fence processing. Expedite job deletion from ring mirror list to the HW fence signal callback instead from finish_work, together with waiting for all such fences to signal in drm_sched_stop we garantee that already signaled job will not be processed twice. Remove the sched finish fence callback and just submit finish_work directly from the HW fence callback. v2: Fix comments. v3: Attach hw fence cb to sched_job Suggested-by: Christian Koenig Signed-off-by: Andrey Grodzovsky --- drivers/gpu/drm/scheduler/sched_main.c | 58 -- include/drm/gpu_scheduler.h| 6 ++-- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index cdf95e2..f0c1f32 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -284,8 +284,6 @@ static void drm_sched_job_finish(struct work_struct *work) cancel_delayed_work_sync(&sched->work_tdr); spin_lock_irqsave(&sched->job_list_lock, flags); - /* remove job from ring_mirror_list */ - list_del_init(&s_job->node); /* queue TDR for next job */ drm_sched_start_timeout(sched); spin_unlock_irqrestore(&sched->job_list_lock, flags); @@ -293,22 +291,11 @@ static void drm_sched_job_finish(struct work_struct *work) sched->ops->free_job(s_job); } -static void drm_sched_job_finish_cb(struct dma_fence *f, - struct dma_fence_cb *cb) -{ - struct drm_sched_job *job = container_of(cb, struct drm_sched_job, -finish_cb); - schedule_work(&job->finish_work); -} - static void drm_sched_job_begin(struct drm_sched_job *s_job) { struct drm_gpu_scheduler *sched = s_job->sched; unsigned long flags; - dma_fence_add_callback(&s_job->s_fence->finished, &s_job- finish_cb, - drm_sched_job_finish_cb); - spin_lock_irqsave(&sched->job_list_lock, flags); list_add_tail(&s_job->node, &sched->ring_mirror_list); drm_sched_start_timeout(sched); @@ -359,12 +346,11 @@ void drm_sched_stop(struct drm_gpu_scheduler *sched, struct drm_sched_job *bad, list_for_each_entry_reverse(s_job, &sched->ring_mirror_list, node) { if (s_job->s_fence->parent && dma_fence_remove_callback(s_job->s_fence->parent, - &s_job->s_fence->cb)) { + &s_job->cb)) { dma_fence_put(s_job->s_fence->parent); s_job->s_fence->parent = NULL; atomic_dec(&sched->hw_rq_count); - } - else { + } else { /* TODO Is it get/put neccessey here ? */ dma_fence_get(&s_job->s_fence->finished); list_add(&s_job->finish_node, &wait_list); @@ - 417,31 +403,34 @@ EXPORT_SYMBOL(drm_sched_stop); void drm_sched_start(struct drm_gpu_scheduler *sched, bool unpark_only) { struct drm_sched_job *s_job, *tmp; - unsigned long flags; int r; if (unpark_only) goto unpark; - spin_lock_irqsave(&sched->job_list_lock, flags); + /* +* Lo