[drm-intel:topic/drm-misc 25/25] htmldocs: drivers/gpu/drm/drm_prime.c:298: warning: No description found for parameter 'dev'
tree: git://anongit.freedesktop.org/drm-intel topic/drm-misc
head: a4fce9cb782ad340ee5576a38e934e5e75832dc6
commit: a4fce9cb782ad340ee5576a38e934e5e75832dc6 [25/25] drm/prime: Take a ref
on the drm_dev when exporting a dma_buf
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
include/drm/drm_fb_helper.h:222: warning: Cannot understand *
@DRM_FB_HELPER_DEFAULT_OPS:
on line 222 - I thought it was a doc line
>> drivers/gpu/drm/drm_prime.c:298: warning: No description found for parameter
>> 'dev'
>> drivers/gpu/drm/drm_prime.c:298: warning: No description found for parameter
>> 'exp_info'
>> drivers/gpu/drm/drm_prime.c:298: warning: Excess function parameter
>> 'dma_buf' description in 'drm_gem_dmabuf_export'
drivers/gpu/drm/drm_prime.c:299: warning: No description found for parameter
'dev'
drivers/gpu/drm/drm_prime.c:299: warning: No description found for parameter
'exp_info'
drivers/gpu/drm/drm_prime.c:299: warning: Excess function parameter
'dma_buf' description in 'drm_gem_dmabuf_export'
vim +/dev +298 drivers/gpu/drm/drm_prime.c
282 {
283 /* nothing to be done here */
284 }
285
286 /**
287 * drm_gem_dmabuf_export - dma_buf export implementation for GEM
288 * @dma_buf: buffer to be exported
289 *
290 * This wraps dma_buf_export() for use by generic GEM drivers that are
using
291 * drm_gem_dmabuf_release(). In addition to calling dma_buf_export(),
we take
292 * a reference to the drm_device which is released by
drm_gem_dmabuf_release().
293 *
294 * Returns the new dmabuf.
295 */
296 struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
297struct dma_buf_export_info
*exp_info)
> 298 {
299 struct dma_buf *dma_buf;
300
301 dma_buf = dma_buf_export(exp_info);
302 if (!IS_ERR(dma_buf))
303 drm_dev_ref(dev);
304
305 return dma_buf;
306 }
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-- next part --
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 6406 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161006/8b405779/attachment.gz>
[Bug 98105] date not accepting
https://bugs.freedesktop.org/show_bug.cgi?id=98105 Bug ID: 98105 Summary: date not accepting Product: Mesa Version: 12.0 Hardware: Other OS: Windows (All) Status: NEW Severity: minor Priority: medium Component: Drivers/DRI/i810 Assignee: dri-devel at lists.freedesktop.org Reporter: microprocessor51293 at gmail.com QA Contact: dri-devel at lists.freedesktop.org while entering date its disabling keyboard inputs -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161006/f6a284a7/attachment.html>
[Bug 98105] date not accepting
https://bugs.freedesktop.org/show_bug.cgi?id=98105 Mahesh Salvi changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161006/6742298b/attachment.html>
[Bug 98105] date not accepting
https://bugs.freedesktop.org/show_bug.cgi?id=98105 --- Comment #1 from Mahesh Salvi --- jkhkjmgh -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161006/9225f74b/attachment.html>
[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support
Hi Archit, On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote: > Hi Maxime, > > On 09/30/2016 08:07 PM, Maxime Ripard wrote: > >Some boards have an entirely passive RGB to VGA bridge, based on either > >DACs or resistor ladders. > > > >Those might or might not have an i2c bus routed to the VGA connector in > >order to access the screen EDIDs. > > > >Add a bridge that doesn't do anything but expose the modes available on the > >screen, either based on the EDIDs if available, or based on the XGA > >standards. > > > >Acked-by: Rob Herring > >Signed-off-by: Maxime Ripard > >--- > > .../bindings/display/bridge/rgb-to-vga-bridge.txt | 48 + > > drivers/gpu/drm/bridge/Kconfig | 7 + > > drivers/gpu/drm/bridge/Makefile| 1 + > > drivers/gpu/drm/bridge/rgb-to-vga.c| 229 > > + > > 4 files changed, 285 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt > > create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c > > > >diff --git > >a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt > >b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt > >new file mode 100644 > >index ..a8375bc1f9cb > >--- /dev/null > >+++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt > >@@ -0,0 +1,48 @@ > >+Dumb RGB to VGA bridge > >+-- > >+ > >+This binding is aimed for dumb RGB to VGA bridges that do not require > >+any configuration. > >+ > >+Required properties: > >+ > >+- compatible: Must be "rgb-to-vga-bridge" > > I'd talked to Laurent on IRC if he's okay with this. And I guess you to > had discussed it during XDC too. He's suggested that it'd be better to > have the compatible string as "simple-vga-dac". I just wished this bikeshedding had taken place publicly and be actually part of that discussion, but yeah, ok. > Some of the reasons behind having this: > > - We don't need to specify "rgb" in the compatible string since most > simple VGA DACs can only work with an RGB input. Ok. > - Also, with "dac" specified in the string, we don't need to > specifically mention "bridge" in the string. Also, bridge is a drm > specific term. > > - "simple" is considered because it's an unconfigurable bridge, and it > might be misleading for other VGA DACs to not use "vga-dac". All those "simple" bindings are just the biggest lie we ever told. It's simple when you introduce it, and then grows into something much more complicated than a non-simple implementation. > What do you think about this? If you think it's good, would it be > possible for you to change this? I guess it's okay for the rest of > the patch to stay the same. I'll update and respin the serie. Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161006/701c158a/attachment.sig>
[PATCH v6 0/5] drm: Add Support for Passive RGB to VGA bridges
Hi, This serie is about adding support for the RGB to VGA bridge found in the A13-Olinuxino and the CHIP VGA adapter. Both these boards rely on an entirely passive bridge made out of resitor ladders that do not require any initialisation. The only thing needed is to get the timings from the screen if available (and if not, fall back on XGA standards), set up the display pipeline to output on the RGB bus with the proper timings, and you're done. This serie also fixes a bunch of bugs uncovered when trying to increase the resolution, and hence the pixel clock, of our pipeline. It also fixes a few bugs in the DRM driver itself that went unnoticed before. Let me know what you think, Maxime Changes from v5: - Renamed to simple-vga-dac Changes from v4: - Removed unused functions Changes from v3: - Depends on OF in Kconfig - Fixed typos in the driver comments - Removed the mention of a "passive" bridge in the bindings doc - Made the strcuture const - Removed the nops and best_encoders implementations - Removed the call to drm_bridge_enable in the sun4i driver Changes from v2: - Changed the compatible as suggested - Rebased on top 4.8 Changes from v1: - Switch to using a vga-connector - Use drm_encoder bridge pointer instead of doing our own - Report the connector status as unknown instead of connected by default, and as connected only if we can retrieve the EDID. - Switch to of_i2c_get_adapter by node, and put the reference when done - Rebased on linux-next Maxime Ripard (5): drm/sun4i: rgb: Remove the bridge enable/disable functions drm/bridge: Add RGB to VGA bridge support ARM: sun5i: a13-olinuxino: Enable VGA bridge ARM: multi_v7: enable VGA bridge ARM: sunxi: Enable VGA bridge .../bindings/display/bridge/rgb-to-vga-bridge.txt | 48 + arch/arm/boot/dts/sun5i-a13-olinuxino.dts | 54 + arch/arm/configs/multi_v7_defconfig| 1 + arch/arm/configs/sunxi_defconfig | 1 + drivers/gpu/drm/bridge/Kconfig | 7 + drivers/gpu/drm/bridge/Makefile| 1 + drivers/gpu/drm/bridge/rgb-to-vga.c| 229 + drivers/gpu/drm/sun4i/sun4i_rgb.c | 6 - 8 files changed, 341 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c -- 2.9.3 Maxime Ripard (5): drm/sun4i: rgb: Remove the bridge enable/disable functions drm/bridge: Add RGB to VGA bridge support ARM: sun5i: a13-olinuxino: Enable VGA bridge ARM: multi_v7: enable VGA bridge ARM: sunxi: Enable VGA bridge Documentation/devicetree/bindings/display/bridge/simple-vga-dac.txt | 48 +++- arch/arm/boot/dts/sun5i-a13-olinuxino.dts | 54 +- arch/arm/configs/multi_v7_defconfig | 1 +- arch/arm/configs/sunxi_defconfig| 1 +- drivers/gpu/drm/bridge/Kconfig | 7 ++- drivers/gpu/drm/bridge/Makefile | 1 +- drivers/gpu/drm/bridge/simple-vga-dac.c | 229 - drivers/gpu/drm/sun4i/sun4i_rgb.c | 6 +-- 8 files changed, 341 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/bridge/simple-vga-dac.txt create mode 100644 drivers/gpu/drm/bridge/simple-vga-dac.c -- git-series 0.8.10
[PATCH v6 4/5] ARM: multi_v7: enable VGA bridge
Enable the RGB to VGA bridge driver in the defconfig Signed-off-by: Maxime Ripard --- arch/arm/configs/multi_v7_defconfig | 1 + 1 file changed, 1 insertion(+), 0 deletions(-) diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index 2c8665cd9dc5..ae1879a61bbe 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -567,6 +567,7 @@ CONFIG_DRM=y CONFIG_DRM_I2C_ADV7511=m # CONFIG_DRM_I2C_CH7006 is not set # CONFIG_DRM_I2C_SIL164 is not set +CONFIG_DRM_SIMPLE_VGA_DAC=m CONFIG_DRM_NXP_PTN3460=m CONFIG_DRM_PARADE_PS8622=m CONFIG_DRM_NOUVEAU=m -- git-series 0.8.10
[PATCH v6 2/5] drm/bridge: Add RGB to VGA bridge support
Some boards have an entirely passive RGB to VGA bridge, based on either
DACs or resistor ladders.
Those might or might not have an i2c bus routed to the VGA connector in
order to access the screen EDIDs.
Add a bridge that doesn't do anything but expose the modes available on the
screen, either based on the EDIDs if available, or based on the XGA
standards.
Acked-by: Rob Herring
Signed-off-by: Maxime Ripard
---
Documentation/devicetree/bindings/display/bridge/simple-vga-dac.txt | 48
+++-
drivers/gpu/drm/bridge/Kconfig | 7 ++-
drivers/gpu/drm/bridge/Makefile | 1 +-
drivers/gpu/drm/bridge/simple-vga-dac.c | 229
-
4 files changed, 285 insertions(+), 0 deletions(-)
create mode 100644
Documentation/devicetree/bindings/display/bridge/simple-vga-dac.txt
create mode 100644 drivers/gpu/drm/bridge/simple-vga-dac.c
diff --git
a/Documentation/devicetree/bindings/display/bridge/simple-vga-dac.txt
b/Documentation/devicetree/bindings/display/bridge/simple-vga-dac.txt
new file mode 100644
index ..7143c54ea88c
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/simple-vga-dac.txt
@@ -0,0 +1,48 @@
+Dumb RGB to VGA bridge
+--
+
+This binding is aimed for dumb RGB to VGA bridges that do not require
+any configuration.
+
+Required properties:
+
+- compatible: Must be "simple-vga-dac"
+
+Required nodes:
+
+This device has two video ports. Their connections are modeled using the OF
+graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for RGB input
+- Video port 1 for VGA output
+
+
+Example
+---
+
+bridge {
+ compatible = "simple-vga-dac";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port at 0 {
+ reg = <0>;
+
+ vga_bridge_in: endpoint {
+ remote-endpoint = <&tcon0_out_vga>;
+ };
+ };
+
+ port at 1 {
+ reg = <1>;
+
+ vga_bridge_out: endpoint {
+ remote-endpoint = <&vga_con_in>;
+ };
+ };
+ };
+};
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index b590e678052d..5fe8c7829052 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -58,6 +58,13 @@ config DRM_SII902X
---help---
Silicon Image sii902x bridge chip driver.
+config DRM_SIMPLE_VGA_DAC
+ tristate "Simple VGA DAC support"
+ depends on OF
+ select DRM_KMS_HELPER
+ ---help---
+ Support for RGB to VGA bridges
+
config DRM_TOSHIBA_TC358767
tristate "Toshiba TC358767 eDP bridge"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index efdb07e878f5..2cdd99035f38 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
obj-$(CONFIG_DRM_SII902X) += sii902x.o
+obj-$(CONFIG_DRM_SIMPLE_VGA_DAC) += simple-vga-dac.o
obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
diff --git a/drivers/gpu/drm/bridge/simple-vga-dac.c
b/drivers/gpu/drm/bridge/simple-vga-dac.c
new file mode 100644
index ..c81a25ab0b0d
--- /dev/null
+++ b/drivers/gpu/drm/bridge/simple-vga-dac.c
@@ -0,0 +1,229 @@
+/*
+ * Copyright (C) 2015-2016 Free Electrons
+ * Copyright (C) 2015-2016 NextThing Co
+ *
+ * Maxime Ripard
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+struct simple_vga {
+ struct drm_bridge bridge;
+ struct drm_connectorconnector;
+
+ struct i2c_adapter *ddc;
+};
+
+static inline struct simple_vga *
+drm_bridge_to_simple_vga(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct simple_vga, bridge);
+}
+
+static inline struct simple_vga *
+drm_connector_to_simple_vga(struct drm_connector *connector)
+{
+ return container_of(connector, struct simple_vga, connector);
+}
+
+static int simple_vga_get_modes(struct drm_connector *connector)
+{
+ struct simple_vga *vga = drm_connector_to_simple_vga(connector);
+ struct edid *edid;
+ int ret
[PATCH v6 1/5] drm/sun4i: rgb: Remove the bridge enable/disable functions
The atomic helpers already call the drm_bridge_enable on our behalf, there's no need to do it a second time. Reported-by: Sean Paul Signed-off-by: Maxime Ripard --- drivers/gpu/drm/sun4i/sun4i_rgb.c | 6 -- 1 file changed, 0 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c index 4e4bea6f395c..d198ad7e5323 100644 --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c @@ -155,9 +155,6 @@ static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder) if (!IS_ERR(tcon->panel)) drm_panel_prepare(tcon->panel); - /* encoder->bridge can be NULL; drm_bridge_enable checks for it */ - drm_bridge_enable(encoder->bridge); - sun4i_tcon_channel_enable(tcon, 0); if (!IS_ERR(tcon->panel)) @@ -177,9 +174,6 @@ static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder) sun4i_tcon_channel_disable(tcon, 0); - /* encoder->bridge can be NULL; drm_bridge_disable checks for it */ - drm_bridge_disable(encoder->bridge); - if (!IS_ERR(tcon->panel)) drm_panel_unprepare(tcon->panel); } -- git-series 0.8.10
[PATCH v6 3/5] ARM: sun5i: a13-olinuxino: Enable VGA bridge
Now that we have support for the VGA bridges using our DRM driver, enable
the display engine for the Olimex A13-Olinuxino.
Signed-off-by: Maxime Ripard
Acked-by: Chen-Yu Tsai
---
arch/arm/boot/dts/sun5i-a13-olinuxino.dts | 54 -
1 file changed, 54 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
b/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
index b3c234c65ea1..c69e0b0b7b55 100644
--- a/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
+++ b/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
@@ -72,6 +72,47 @@
default-state = "on";
};
};
+
+ bridge {
+ compatible = "simple-vga-dac";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port at 0 {
+ reg = <0>;
+
+ vga_bridge_in: endpoint {
+ remote-endpoint = <&tcon0_out_vga>;
+ };
+ };
+
+ port at 1 {
+ reg = <1>;
+
+ vga_bridge_out: endpoint {
+ remote-endpoint = <&vga_con_in>;
+ };
+ };
+ };
+ };
+
+ vga {
+ compatible = "vga-connector";
+
+ port {
+ vga_con_in: endpoint {
+ remote-endpoint = <&vga_bridge_out>;
+ };
+ };
+ };
+};
+
+&be0 {
+ status = "okay";
};
&ehci0 {
@@ -211,6 +252,19 @@
status = "okay";
};
+&tcon0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&lcd_rgb666_pins>;
+ status = "okay";
+};
+
+&tcon0_out {
+ tcon0_out_vga: endpoint at 0 {
+ reg = <0>;
+ remote-endpoint = <&vga_bridge_in>;
+ };
+};
+
&uart1 {
pinctrl-names = "default";
pinctrl-0 = <&uart1_pins_b>;
--
git-series 0.8.10
[PATCH v6 5/5] ARM: sunxi: Enable VGA bridge
Enable the VGA bridge used on the A13-Olinuxino in the sunxi defconfig Signed-off-by: Maxime Ripard --- arch/arm/configs/sunxi_defconfig | 1 + 1 file changed, 1 insertion(+), 0 deletions(-) diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig index 714da336ec86..33b8c3308335 100644 --- a/arch/arm/configs/sunxi_defconfig +++ b/arch/arm/configs/sunxi_defconfig @@ -98,6 +98,7 @@ CONFIG_MEDIA_RC_SUPPORT=y CONFIG_RC_DEVICES=y CONFIG_IR_SUNXI=y CONFIG_DRM=y +CONFIG_DRM_SIMPLE_VGA_DAC=y CONFIG_DRM_SUN4I=y CONFIG_FB=y CONFIG_FB_SIMPLE=y -- git-series 0.8.10
[Bug 98028] Guns of Icarus Online segfaults on startup since AMDGPU: Partially fix control flow at -O0
https://bugs.freedesktop.org/show_bug.cgi?id=98028 --- Comment #8 from Nicolai Hähnle --- Careful inspection of the commit you bisected this to has lead me to a smoking gun. Could you please check whether the patch at https://reviews.llvm.org/D25306 fixes this for you? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161006/8753fb64/attachment.html>
[PATCH v9 0/4] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are exected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. In this v9, there are build fixes for !CONFIG_DEBUG_FS and a fix so we don't break probing of drivers that still use the .load callback (tested on Tegra124). Thanks, Tomeu Tomeu Vizoso (4): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API drm/i915: Put "cooked" vlank counters in frame CRC lines Documentation/gpu/drm-uapi.rst|6 + drivers/gpu/drm/Makefile |3 +- drivers/gpu/drm/drm_crtc.c| 34 +- drivers/gpu/drm/drm_debugfs.c | 34 +- drivers/gpu/drm/drm_debugfs_crc.c | 351 drivers/gpu/drm/drm_internal.h| 16 + drivers/gpu/drm/i915/Makefile |2 +- drivers/gpu/drm/i915/i915_debugfs.c | 886 +--- drivers/gpu/drm/i915/i915_drv.h |1 + drivers/gpu/drm/i915/i915_irq.c | 83 ++- drivers/gpu/drm/i915/intel_display.c |1 + drivers/gpu/drm/i915/intel_drv.h | 13 + drivers/gpu/drm/i915/intel_pipe_crc.c | 1013 + include/drm/drm_crtc.h| 41 ++ include/drm/drm_debugfs_crc.h | 73 +++ 15 files changed, 1645 insertions(+), 912 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c create mode 100644 include/drm/drm_debugfs_crc.h -- 2.7.4
[PATCH v9 1/4] drm/i915/debugfs: Move out pipe CRC code
In preparation to using a generic API in the DRM core for continuous CRC
generation, move the related code out of i915_debugfs.c into a new file.
Eventually, only the Intel-specific code will remain in this new file.
v2: Rebased.
v6: Rebased.
v7: Fix whitespace issue.
v9: Have intel_display_crc_init accept a drm_i915_private instead.
Signed-off-by: Tomeu Vizoso
Reviewed-by: Emil Velikov
---
drivers/gpu/drm/i915/Makefile | 2 +-
drivers/gpu/drm/i915/i915_debugfs.c | 886 +---
drivers/gpu/drm/i915/intel_drv.h | 5 +
drivers/gpu/drm/i915/intel_pipe_crc.c | 943 ++
4 files changed, 953 insertions(+), 883 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index a998c2bce70a..e6fe0040fe64 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -24,7 +24,7 @@ i915-y := i915_drv.o \
intel_runtime_pm.o
i915-$(CONFIG_COMPAT) += i915_ioc32.o
-i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
+i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
# GEM code
i915-y += i915_cmd_parser.o \
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index da7141382b00..4fb9d829884e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -26,19 +26,9 @@
*
*/
-#include
-#include
-#include
#include
-#include
-#include
#include
-#include
-#include
#include "intel_drv.h"
-#include "intel_ringbuffer.h"
-#include
-#include "i915_drv.h"
static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
{
@@ -3414,12 +3404,6 @@ static int i915_drrs_status(struct seq_file *m, void
*unused)
return 0;
}
-struct pipe_crc_info {
- const char *name;
- struct drm_i915_private *dev_priv;
- enum pipe pipe;
-};
-
static int i915_dp_mst_info(struct seq_file *m, void *unused)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -3449,848 +3433,6 @@ static int i915_dp_mst_info(struct seq_file *m, void
*unused)
return 0;
}
-static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
-{
- struct pipe_crc_info *info = inode->i_private;
- struct drm_i915_private *dev_priv = info->dev_priv;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
-
- if (info->pipe >= INTEL_INFO(dev_priv)->num_pipes)
- return -ENODEV;
-
- spin_lock_irq(&pipe_crc->lock);
-
- if (pipe_crc->opened) {
- spin_unlock_irq(&pipe_crc->lock);
- return -EBUSY; /* already open */
- }
-
- pipe_crc->opened = true;
- filep->private_data = inode->i_private;
-
- spin_unlock_irq(&pipe_crc->lock);
-
- return 0;
-}
-
-static int i915_pipe_crc_release(struct inode *inode, struct file *filep)
-{
- struct pipe_crc_info *info = inode->i_private;
- struct drm_i915_private *dev_priv = info->dev_priv;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
-
- spin_lock_irq(&pipe_crc->lock);
- pipe_crc->opened = false;
- spin_unlock_irq(&pipe_crc->lock);
-
- return 0;
-}
-
-/* (6 fields, 8 chars each, space separated (5) + '\n') */
-#define PIPE_CRC_LINE_LEN (6 * 8 + 5 + 1)
-/* account for \'0' */
-#define PIPE_CRC_BUFFER_LEN(PIPE_CRC_LINE_LEN + 1)
-
-static int pipe_crc_data_count(struct intel_pipe_crc *pipe_crc)
-{
- assert_spin_locked(&pipe_crc->lock);
- return CIRC_CNT(pipe_crc->head, pipe_crc->tail,
- INTEL_PIPE_CRC_ENTRIES_NR);
-}
-
-static ssize_t
-i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
- loff_t *pos)
-{
- struct pipe_crc_info *info = filep->private_data;
- struct drm_i915_private *dev_priv = info->dev_priv;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
- char buf[PIPE_CRC_BUFFER_LEN];
- int n_entries;
- ssize_t bytes_read;
-
- /*
-* Don't allow user space to provide buffers not big enough to hold
-* a line of data.
-*/
- if (count < PIPE_CRC_LINE_LEN)
- return -EINVAL;
-
- if (pipe_crc->source == INTEL_PIPE_CRC_SOURCE_NONE)
- return 0;
-
- /* nothing to read */
- spin_lock_irq(&pipe_crc->lock);
- while (pipe_crc_data_count(pipe_crc) == 0) {
- int ret;
-
- if (filep->f_flags & O_NONBLOCK) {
- spin_unlock_irq(&pipe_crc->lock);
- return -EAGAIN;
- }
-
- ret = wait_event_interruptible_lock_irq(pipe_crc->wq,
- pipe_crc_data_count(pipe_crc), pipe_crc->lock);
- if (ret) {
- spin_unlock_irq(&pipe_crc->lock);
-
[PATCH v9 2/4] drm: Add API for capturing frame CRCs
Adds files and directories to debugfs for controlling and reading frame
CRCs, per CRTC:
dri/0/crtc-0/crc
dri/0/crtc-0/crc/control
dri/0/crtc-0/crc/data
Drivers can implement the set_crc_source callback() in drm_crtc_funcs to
start and stop generating frame CRCs and can add entries to the output
by calling drm_crtc_add_crc_entry.
v2:
- Lots of good fixes suggested by Thierry.
- Added documentation.
- Changed the debugfs layout.
- Moved to allocate the entries circular queue once when frame
generation gets enabled for the first time.
v3:
- Use the control file just to select the source, and start and stop
capture when the data file is opened and closed, respectively.
- Make variable the number of CRC values per entry, per source.
- Allocate entries queue each time we start capturing as now there
isn't a fixed number of CRC values per entry.
- Store the frame counter in the data file as a 8-digit hex number.
- For sources that cannot provide useful frame numbers, place
in the frame field.
v4:
- Build only if CONFIG_DEBUG_FS is enabled.
- Use memdup_user_nul.
- Consolidate calculation of the size of an entry in a helper.
- Add 0x prefix to hex numbers in the data file.
- Remove unnecessary snprintf and strlen usage in read callback.
v5:
- Made the crcs array in drm_crtc_crc_entry fixed-size
- Lots of other smaller improvements suggested by Emil Velikov
v7:
- Move definition of drm_debugfs_crtc_crc_add to drm_internal.h
v8:
- Call debugfs_remove_recursive when we fail to create the minor
device
v9:
- Register the debugfs directory for a crtc from
drm_crtc_register_all()
Signed-off-by: Tomeu Vizoso
---
Documentation/gpu/drm-uapi.rst| 6 +
drivers/gpu/drm/Makefile | 3 +-
drivers/gpu/drm/drm_crtc.c| 34 +++-
drivers/gpu/drm/drm_debugfs.c | 34 +++-
drivers/gpu/drm/drm_debugfs_crc.c | 351 ++
drivers/gpu/drm/drm_internal.h| 16 ++
include/drm/drm_crtc.h| 41 +
include/drm/drm_debugfs_crc.h | 73
8 files changed, 555 insertions(+), 3 deletions(-)
create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c
create mode 100644 include/drm/drm_debugfs_crc.h
diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 1ba301cebe16..de3ac9f90f8f 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -216,3 +216,9 @@ interfaces. Especially since all hardware-acceleration
interfaces to
userspace are driver specific for efficiency and other reasons these
interfaces can be rather substantial. Hence every driver has its own
chapter.
+
+Testing and validation
+==
+
+.. kernel-doc:: drivers/gpu/drm/drm_debugfs_crc.c
+ :doc: CRC ABI
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 25c720454017..74579d2e796e 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -9,7 +9,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \
drm_scatter.o drm_pci.o \
drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
- drm_info.o drm_debugfs.o drm_encoder_slave.o \
+ drm_info.o drm_encoder_slave.o \
drm_trace_points.o drm_global.o drm_prime.o \
drm_rect.o drm_vma_manager.o drm_flip_work.o \
drm_modeset_lock.o drm_atomic.o drm_bridge.o \
@@ -23,6 +23,7 @@ drm-$(CONFIG_PCI) += ati_pcigart.o
drm-$(CONFIG_DRM_PANEL) += drm_panel.o
drm-$(CONFIG_OF) += drm_of.o
drm-$(CONFIG_AGP) += drm_agpsupport.o
+drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 2d7bedf28647..151ff9805de1 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -40,7 +40,7 @@
#include
#include
#include
-#include
+#include
#include "drm_crtc_internal.h"
#include "drm_internal.h"
@@ -126,6 +126,10 @@ static int drm_crtc_register_all(struct drm_device *dev)
ret = crtc->funcs->late_register(crtc);
if (ret)
return ret;
+
+ ret = drm_debugfs_crtc_add(crtc);
+ if (ret)
+ return ret;
}
return 0;
@@ -136,11 +140,31 @@ static void drm_crtc_unregister_all(struct drm_device
*dev)
struct drm_crtc *crtc;
drm_for_each_crtc(crtc, dev) {
+ drm_debugfs_crtc_remove(crtc);
if (crtc->funcs->early_unregister)
crtc->funcs->early_unregister(crtc);
}
}
+static int drm_crtc_crc_init(struct drm_crtc *crtc)
+{
+#ifdef CON
[PATCH v9 3/4] drm/i915: Use new CRC debugfs API
The core provides now an ABI to userspace for generation of frame CRCs,
so implement the ->set_crc_source() callback and reuse as much code as
possible with the previous ABI implementation.
When handling the pageflip interrupt, we skip 1 or 2 frames depending on
the HW because they contain wrong values. For the legacy ABI for
generating frame CRCs, this was done in userspace but now that we have a
generic ABI it's better if it's not exposed by the kernel.
v2:
- Leave the legacy implementation in place as the ABI implementation
in the core is incompatible with it.
v3:
- Use the "cooked" vblank counter so we have a whole 32 bits.
- Make sure we don't mess with the state of the legacy CRC capture
ABI implementation.
v4:
- Keep use of get_vblank_counter as in the legacy code, will be
changed in a followup commit.
v5:
- Skip first frame or two as it's known that they contain wrong
data.
- A few fixes suggested by Emil Velikov.
v6:
- Rework programming of the HW registers to preserve previous
behavior.
v7:
- Address whitespace issue.
- Added a comment on why in the implementation of the new ABI we
skip the 1st or 2nd frames.
v9:
- Add stub for intel_crtc_set_crc_source.
Signed-off-by: Tomeu Vizoso
Reviewed-by: Emil Velikov
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 83 +--
drivers/gpu/drm/i915/intel_display.c | 1 +
drivers/gpu/drm/i915/intel_drv.h | 8 +++
drivers/gpu/drm/i915/intel_pipe_crc.c | 94 ++-
5 files changed, 149 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f8c66eea06bc..20522f0a4c57 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1713,6 +1713,7 @@ struct intel_pipe_crc {
enum intel_pipe_crc_source source;
int head, tail;
wait_queue_head_t wq;
+ int skipped;
};
struct i915_frontbuffer_tracking {
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index bd6c8b0eeaef..1549cc4f88ec 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1491,41 +1491,72 @@ static void display_pipe_crc_irq_handler(struct
drm_i915_private *dev_priv,
{
struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
struct intel_pipe_crc_entry *entry;
- int head, tail;
+ struct drm_crtc *crtc = intel_get_crtc_for_pipe(&dev_priv->drm, pipe);
+ struct drm_driver *driver = dev_priv->drm.driver;
+ uint32_t crcs[5];
+ int head, tail, ret;
+ u32 frame;
spin_lock(&pipe_crc->lock);
+ if (pipe_crc->source) {
+ if (!pipe_crc->entries) {
+ spin_unlock(&pipe_crc->lock);
+ DRM_DEBUG_KMS("spurious interrupt\n");
+ return;
+ }
- if (!pipe_crc->entries) {
- spin_unlock(&pipe_crc->lock);
- DRM_DEBUG_KMS("spurious interrupt\n");
- return;
- }
-
- head = pipe_crc->head;
- tail = pipe_crc->tail;
+ head = pipe_crc->head;
+ tail = pipe_crc->tail;
- if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
- spin_unlock(&pipe_crc->lock);
- DRM_ERROR("CRC buffer overflowing\n");
- return;
- }
+ if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
+ spin_unlock(&pipe_crc->lock);
+ DRM_ERROR("CRC buffer overflowing\n");
+ return;
+ }
- entry = &pipe_crc->entries[head];
+ entry = &pipe_crc->entries[head];
- entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm,
-pipe);
- entry->crc[0] = crc0;
- entry->crc[1] = crc1;
- entry->crc[2] = crc2;
- entry->crc[3] = crc3;
- entry->crc[4] = crc4;
+ entry->frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
+ entry->crc[0] = crc0;
+ entry->crc[1] = crc1;
+ entry->crc[2] = crc2;
+ entry->crc[3] = crc3;
+ entry->crc[4] = crc4;
- head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
- pipe_crc->head = head;
+ head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
+ pipe_crc->head = head;
- spin_unlock(&pipe_crc->lock);
+ spin_unlock(&pipe_crc->lock);
- wake_up_interruptible(&pipe_crc->wq);
+ wake_up_interruptible(&pipe_crc->wq);
+ } else {
+ /*
+* For some not yet identified reason, the first CRC is
+* bonkers. So let's just wait for the next vblank and rea
[PATCH v9 4/4] drm/i915: Put "cooked" vlank counters in frame CRC lines
Use drm_accurate_vblank_count so we have the full 32 bit to represent
the frame counter and userspace has a simpler way of knowing when the
counter wraps around.
Signed-off-by: Tomeu Vizoso
Reviewed-by: Emil Velikov
---
drivers/gpu/drm/i915/i915_irq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1549cc4f88ec..238a353454e9 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1495,7 +1495,6 @@ static void display_pipe_crc_irq_handler(struct
drm_i915_private *dev_priv,
struct drm_driver *driver = dev_priv->drm.driver;
uint32_t crcs[5];
int head, tail, ret;
- u32 frame;
spin_lock(&pipe_crc->lock);
if (pipe_crc->source) {
@@ -1551,8 +1550,9 @@ static void display_pipe_crc_irq_handler(struct
drm_i915_private *dev_priv,
crcs[2] = crc2;
crcs[3] = crc3;
crcs[4] = crc4;
- frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
- ret = drm_crtc_add_crc_entry(crtc, true, frame, crcs);
+ ret = drm_crtc_add_crc_entry(crtc, true,
+drm_accurate_vblank_count(crtc),
+crcs);
spin_unlock(&crtc->crc.lock);
if (!ret)
wake_up_interruptible(&crtc->crc.wq);
--
2.7.4
[PATCH v3 3/3] drm/bridge: add Silicon Image SiI8620 driver
Hi Daniel, Archit, On 30.09.2016 12:33, Andrzej Hajda wrote: > On 30.09.2016 12:07, Daniel Vetter wrote: >> On Fri, Sep 30, 2016 at 09:30:16AM +0530, Archit Taneja wrote: >>> Hi Andrezj, >>> >>> On 09/26/2016 07:10 PM, Andrzej Hajda wrote: SiI8620 transmitter converts eTMDS/HDMI signal to MHL 3.0. It is controlled via I2C bus. Its interaction with other devices in video pipeline is performed mainly on HW level. The only interaction it does on device driver level is filtering-out unsupported video modes, it exposes drm_bridge interface to perform this operation. >>> The patchset looks good to me. Is the MHL header patch >>> accepted? I was wondering how we pull this in. >>> >>> +Daniel >> I think someone with real clue about what MHL is needs to review that >> header. Also I have no idea why that's under video/, is there another >> driver in media we want to share this with? >> -Daniel >> > I have put it into include/linux as MHL could be used to transmit: > - video, > - audio, > - remote control protocol (input device), > - ... embed other protocols (USB for example), > > But since I am not aware of other MHL users in near future > I can put the header together with the bridge driver. These patches are hanging on the list for almost year, since Archit decided to review it (thanks Archit), I would like to end this process. The options I see: 1. Leave it as is, mhl.h is like hdmi.h - it can server for multiple subsystems. I guess it can be hard to find MHL specialist to review it as it does not seems to be popular subject, on the other side it is only in-kernel header so it should pose serious danger. 2. Move the header to some of drm dirs: a) drivers/gpu/drm/bridge/ b) include/drm/bridge/ c) include/drm/ ... 3. Incorporate it into drivers/gpu/drm/bridge/sil-sii8620.h This is the least problematic solution, but possible future abstraction of MHL will be more noisy. Daniel, which option do you prefer? For me any option is OK, I just want to end this little bit frustrating process. Regards Andrzej
[PATCH 2/3] drm/fb_cma_helper: Add panic handling
On Wed, Oct 05, 2016 at 09:36:17PM +0200, Noralf Trønnes wrote:
>
> Den 05.10.2016 15:22, skrev Laurent Pinchart:
> > Hi Noralf,
> >
> > Thank you for the patch.
> >
> > On Sunday 11 Sep 2016 20:47:41 Noralf Trønnes wrote:
> > > This enables panic message output for fb cma helper framebuffers.
> > >
> > > Signed-off-by: Noralf Trønnes
> > > ---
> > > drivers/gpu/drm/drm_fb_cma_helper.c | 10 ++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > b/drivers/gpu/drm/drm_fb_cma_helper.c index 1fd6eac..2f1b012 100644
> > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > @@ -126,9 +126,19 @@ int drm_fb_cma_create_handle(struct drm_framebuffer
> > > *fb, }
> > > EXPORT_SYMBOL(drm_fb_cma_create_handle);
> > >
> > > +static void *drm_fb_cma_panic_vmap(struct drm_framebuffer *fb)
> > > +{
> > > + struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> > > + struct drm_gem_cma_object *cma_obj = fb_cma->obj[0];
> > > +
> > > + /* A PRIME imported buffer will not have it's vaddr set. */
> > Does this mean that, if the framebuffer that happens to be displayed when a
> > panic occurs is imported, no message will be printed ? I understand how
> > supporting such cases is difficult, but I'm wondering how we could proceed
> > to
> > ensure that a panic can be displayed in most (if not all) cases.
>
> If we can vmap all cma buffers, then it's simple:
> - Add dma_buf_vmap() call to drm_gem_cma_prime_import_sg_table()
> - Add dma_buf_vunmap() call to drm_gem_cma_free_object()
>
> If not then it looks more complicated, since this is atomic context.
> There is dma_buf_kmap_atomic(), but there are no users.
> And drm_gem_prime_dmabuf_ops doesn't support .kmap_atomic either
> (returns NULL).
>
> omapdrm is the only dma_buf provider I can find that actually uses
> kmap_atomic() instead of just returning NULL or relying on an existing
> virtual address. It has it's own .gem_prime_import/export functions to
> accomplish this.
dma_buf's kmap_atomic is a bit ill-defined, since atm it's not clear how
to get the buffer pinned in the first place. Originally the plan was to
use begin/end_cpu_access for this (which again can block, so not suitable
for panic context). But that was kinda reused as the coherence control
interface, and doesn't really pin anything. I guess we should probably
just nuke the kmap interface part of dma_buf, the idea was to use that for
ttm, but that never really happened.
> > Similarly, it looks like your code only handles single-planar formats, but
> > there's no explicit check for that in patch 1/3. The easiest fix is to
> > reject
> > multi-planar framebuffers, but that would again result in silent panics in
> > some cases.
>
> That's correct if we talk about the default panic_draw_xy() function:
> drm_framebuffer_panic_draw_xy(). Most likely this function can be extended
> to support multi-planar formats, but Daniel said we could wait with that.
> And the driver can also implement it's own .panic_draw_xy() function.
>
> > > + return cma_obj ? cma_obj->vaddr : NULL;
> > Can cma_obj be NULL here ? I thought that framebuffer objects were always
> > created with at least one GEM object.
>
> I was trying to be very careful since a panic is about something gone
> very wrong. But in that case I should have checked that fb_cma isn't NULL
> also before dereferencing it.
> Maybe I'm over the top paranoid :-)
Yeah I think this is a bit too much paranoia. I think you can remove these
NULL checks here, it's truly impossible. Where we really need to be
careful is with locking, both to make sure we exclusively use trylocks for
everything, and that we do grab all the locks (to avoid disaster when the
part that panicked is kms itself).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH v3 3/3] drm/bridge: add Silicon Image SiI8620 driver
On Thu, Oct 06, 2016 at 11:02:58AM +0200, Andrzej Hajda wrote: > Hi Daniel, Archit, > > On 30.09.2016 12:33, Andrzej Hajda wrote: > > On 30.09.2016 12:07, Daniel Vetter wrote: > >> On Fri, Sep 30, 2016 at 09:30:16AM +0530, Archit Taneja wrote: > >>> Hi Andrezj, > >>> > >>> On 09/26/2016 07:10 PM, Andrzej Hajda wrote: > SiI8620 transmitter converts eTMDS/HDMI signal to MHL 3.0. > It is controlled via I2C bus. Its interaction with other > devices in video pipeline is performed mainly on HW level. > The only interaction it does on device driver level is > filtering-out unsupported video modes, it exposes drm_bridge > interface to perform this operation. > >>> The patchset looks good to me. Is the MHL header patch > >>> accepted? I was wondering how we pull this in. > >>> > >>> +Daniel > >> I think someone with real clue about what MHL is needs to review that > >> header. Also I have no idea why that's under video/, is there another > >> driver in media we want to share this with? > >> -Daniel > >> > > I have put it into include/linux as MHL could be used to transmit: > > - video, > > - audio, > > - remote control protocol (input device), > > - ... embed other protocols (USB for example), > > > > But since I am not aware of other MHL users in near future > > I can put the header together with the bridge driver. > > These patches are hanging on the list for almost year, > since Archit decided to review it (thanks Archit), I would > like to end this process. > > The options I see: > 1. Leave it as is, mhl.h is like hdmi.h - it can server for > multiple subsystems. I guess it can be hard to find > MHL specialist to review it as it does not seems > to be popular subject, on the other side it is only > in-kernel header so it should pose serious danger. > 2. Move the header to some of drm dirs: > a) drivers/gpu/drm/bridge/ > b) include/drm/bridge/ > c) include/drm/ > ... > 3. Incorporate it into drivers/gpu/drm/bridge/sil-sii8620.h > This is the least problematic solution, but possible > future abstraction of MHL will be more noisy. > > Daniel, which option do you prefer? For me any option > is OK, I just want to end this little bit frustrating process. I just brought this up as a question, I don't personally care all that much. Except for option 2a) I think they are all ok (we only have internal headers outside of include/). Whatever you&Archit can agree on is fine with me (and then Archit can all push it directly to drm-misc). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm/fb-helper: fix sphinx markup for DRM_FB_HELPER_DEFAULT_OPS
On Wed, Oct 05, 2016 at 08:34:14PM +0200, Stefan Christ wrote:
> Fix invalid sphinx markup in the comment for the newly added
> DRM_FB_HELPER_DEFAULT_OPS.
>
> Signed-off-by: Stefan Christ
> ---
> Hi,
>
> > > If I'm not mistaken v1 of this patch is already in drm-misc so you may
> > > want to send a patch that fix just the line above.
> >
> > Yup, I need an incremental patch which applies on top of drm-misc or
> > linux-next. Sorry if this wasn't clear.
> > -Daniel
>
> Thanks for the head-ups. Here is a fix patch for it. Actually I should have
> come up with the same idea myself seeing the patch queued for the next pull
> request.
>
> I have not added a "Fixes:" trailer in the commit message. If this is
> necessary
> I can resend it, when I see the offending patch in Linus master tree with a
> commit id.
drm-misc has stable sha1, which means you can cite them already when they
show up in linux-next. But for a minor fix like this no big deal really.
Applied to drm-misc, thanks.
-Daniel
>
> Kind regards,
> Stefan Christ
> ---
> include/drm/drm_fb_helper.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 3c5f599..ed8edfe 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -218,7 +218,7 @@ struct drm_fb_helper {
> };
>
> /**
> - * @DRM_FB_HELPER_DEFAULT_OPS:
> + * define DRM_FB_HELPER_DEFAULT_OPS - helper define for drm drivers
> *
> * Helper define to register default implementations of drm_fb_helper
> * functions. To be used in struct fb_ops of drm drivers.
> --
> 2.1.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm/bridge: Drop drm_connector_unregister and call drm_connector_cleanup directly
On Wed, Oct 05, 2016 at 04:31:33PM +0200, Marek Vasut wrote:
> Drop unneeded drm_connector_unregister() and remove the unnecessary
> wrapper functions around drm_connector_cleanup().
>
> Signed-off-by: Marek Vasut
> Cc: Daniel Vetter
Yeah, since 4.8 the only connectors you need to register/unregister
explicitly are hotpluggeable ones like DP mst. Applied to drm-misc, thanks
for this neat little cleanup.
-Daniel
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 9 +
> drivers/gpu/drm/bridge/dw-hdmi.c | 8 +---
> drivers/gpu/drm/bridge/tc358767.c | 8 +---
> 3 files changed, 3 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 001b075..6e0447f 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -999,18 +999,11 @@ analogix_dp_detect(struct drm_connector *connector,
> bool force)
> return status;
> }
>
> -static void analogix_dp_connector_destroy(struct drm_connector *connector)
> -{
> - drm_connector_unregister(connector);
> - drm_connector_cleanup(connector);
> -
> -}
> -
> static const struct drm_connector_funcs analogix_dp_connector_funcs = {
> .dpms = drm_atomic_helper_connector_dpms,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .detect = analogix_dp_detect,
> - .destroy = analogix_dp_connector_destroy,
> + .destroy = drm_connector_cleanup,
> .reset = drm_atomic_helper_connector_reset,
> .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index 66ad8e6..ab7023e 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -1477,12 +1477,6 @@ dw_hdmi_connector_mode_valid(struct drm_connector
> *connector,
> return mode_status;
> }
>
> -static void dw_hdmi_connector_destroy(struct drm_connector *connector)
> -{
> - drm_connector_unregister(connector);
> - drm_connector_cleanup(connector);
> -}
> -
> static void dw_hdmi_connector_force(struct drm_connector *connector)
> {
> struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
> @@ -1499,7 +1493,7 @@ static const struct drm_connector_funcs
> dw_hdmi_connector_funcs = {
> .dpms = drm_atomic_helper_connector_dpms,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .detect = dw_hdmi_connector_detect,
> - .destroy = dw_hdmi_connector_destroy,
> + .destroy = drm_connector_cleanup,
> .force = dw_hdmi_connector_force,
> .reset = drm_atomic_helper_connector_reset,
> .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> diff --git a/drivers/gpu/drm/bridge/tc358767.c
> b/drivers/gpu/drm/bridge/tc358767.c
> index a09825d..44d476e 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1165,17 +1165,11 @@ static const struct drm_connector_helper_funcs
> tc_connector_helper_funcs = {
> .best_encoder = tc_connector_best_encoder,
> };
>
> -static void tc_connector_destroy(struct drm_connector *connector)
> -{
> - drm_connector_unregister(connector);
> - drm_connector_cleanup(connector);
> -}
> -
> static const struct drm_connector_funcs tc_connector_funcs = {
> .dpms = drm_atomic_helper_connector_dpms,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .detect = tc_connector_detect,
> - .destroy = tc_connector_destroy,
> + .destroy = drm_connector_cleanup,
> .reset = drm_atomic_helper_connector_reset,
> .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> --
> 2.9.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm: Fix up kerneldoc for new drm_gem_dmabuf_export()
On Wed, Oct 05, 2016 at 06:40:56PM +0100, Chris Wilson wrote:
> I hit send before completing a make htmldoc, and lo I forgot to fix up
> the cut'n'paste.
>
> Fixes: a4fce9cb782a ("drm/prime: Take a ref on the drm_dev when exporting...")
> Reported-by: kbuild test robot
> Signed-off-by: Chris Wilson
> Cc: Daniel Vetter
> Cc: stable at vger.kernel.org
Thanks for the quick fix, applied to drm-misc.
-Daniel
> ---
> drivers/gpu/drm/drm_prime.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 875df8d719fb..b22a94dd7b53 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -285,7 +285,8 @@ static void drm_gem_unmap_dma_buf(struct
> dma_buf_attachment *attach,
>
> /**
> * drm_gem_dmabuf_export - dma_buf export implementation for GEM
> - * @dma_buf: buffer to be exported
> + * @dev: parent device for the exported dmabuf
> + * @exp_info: the export information used by dma_buf_export()
> *
> * This wraps dma_buf_export() for use by generic GEM drivers that are using
> * drm_gem_dmabuf_release(). In addition to calling dma_buf_export(), we take
> --
> 2.9.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Bug 98105] date not accepting
https://bugs.freedesktop.org/show_bug.cgi?id=98105 Vedran MiletiÄ changed: What|Removed |Added Resolution|FIXED |INVALID -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161006/02ccc9ec/attachment.html>
[Bug 98105] date not accepting
https://bugs.freedesktop.org/show_bug.cgi?id=98105 Vedran MiletiÄ changed: What|Removed |Added Version|12.0|unspecified QA Contact|dri-devel at lists.freedesktop | |.org| Component|Drivers/DRI/i810|/dev/null Product|Mesa|a big freedesktop.org fly ||ribbon Assignee|dri-devel at lists.freedesktop |devnull at freedesktop.org |.org| -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161006/12719b9e/attachment.html>
[PATCH v2 07/15] drm/omap: Use per-plane rotation property
On 26/09/16 19:30, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä
>
> The global mode_config.rotation_property is going away, switch over to
> per-plane rotation_property.
>
> Not sure I got the annoying crtc rotation_property handling right.
> Might work, or migth not.
>
> v2: Drop the BIT()
> Don't create rotation property twice for each primary plane
>
> Cc: Tomi Valkeinen
> Cc: Rob Clark
> Signed-off-by: Ville Syrjälä
> ---
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c
> index 6ddaa5ea4b6b..b272f810989e 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -211,9 +211,16 @@ void omap_plane_install_properties(struct drm_plane
> *plane,
> struct omap_drm_private *priv = dev->dev_private;
>
> if (priv->has_dmm) {
> - struct drm_property *prop = dev->mode_config.rotation_property;
> -
> - drm_object_attach_property(obj, prop, DRM_ROTATE_0);
> + if (!plane->rotation_property)
> + drm_plane_create_rotation_property(plane,
> +DRM_ROTATE_0,
> +DRM_ROTATE_0 |
> DRM_ROTATE_90 |
> +DRM_ROTATE_180 |
> DRM_ROTATE_270 |
> +DRM_REFLECT_X |
> DRM_REFLECT_Y);
> +
> + if (plane->rotation_property && obj != &plane->base)
> + drm_object_attach_property(obj,
> plane->rotation_property,
> +DRM_ROTATE_0);
I think this could use a short comment, as it's not obvious wth is going
on here =).
Otherwise both omap patches look fine, and test fine.
Reviewed-by: Tomi Valkeinen
Tomi
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161006/308052f4/attachment-0001.sig>
[PATCH v9 17/19] drm/virtio: kconfig: Fix recursive dependency issue.
Hi Bjorn,
On 27 September 2016 at 18:01, Bjorn Andersson
wrote:
> On Wed 21 Sep 05:09 PDT 2016, Emil Velikov wrote:
>
>> On 20 September 2016 at 09:32, Peter Griffin
>> wrote:
>> > Hi Emil,
>> >
>> > On Tue, 20 Sep 2016, Emil Velikov wrote:
>> >
>> >> On 5 September 2016 at 14:16, Peter Griffin
>> >> wrote:
>> >> > ST_SLIM_REMOTEPROC must select REMOTEPROC, which exposes the following
>> >> > recursive dependency.
>> >
>> >
>> >> >
>> >> From a humble skim through remoteproc, drm and a few other subsystems
>> >> I think the above is wrong. All the drivers (outside of remoteproc),
>> >> that I've seen, depend on the core component, they don't select it.
>> >
>> > I will let Bjorn comment on the remoteproc subsystem Kconfig design, and
>> > why it is like it is.
>> >
>> > For this particular SLIM_RPROC I have added it to Kconfig in keeping with
>> > all
>> > the other drivers in the remoteproc subsystem which has exposed this
>> > recursive
>> > dependency issue.
>> >
>> > For this particular kconfig symbol a quick grep reveals more drivers in
>> > the kernel using 'select', than 'depend on'
>> >
>> > git grep "select VIRTIO" | wc -l
>> > 14
>> >
>> > git grep "depends on VIRTIO" | wc -l
>> > 10
>> >
>> Might be worth taking a closer look into these at some point.
>>
>
> The general idea here is that VIRTIO provides the "framework" and as
> such drivers implementing VIRTIO do select and drivers using virtio use
> depends.
>
> This is found in several places around the kernel.
>
>> >
>> >> Furthermore most places explicitly hide the drivers from the menu if
>> >> the core component isn't enabled.
>> >
>> > Remoteproc subsystem takes a different approach, the core code is only
>> > enabled
>> > if a driver which relies on it is enabled. This IMHO makes sense, as
>> > remoteproc is not widely used (only a few particular ARM SoC's).
>> >
>> > It is true that for subsystems which rely on the core component being
>> > explicitly enabled, they often tend to hide drivers which depend on it
>> > from the menu unless it is. This also makes sense.
>> >
>> >>
>> >> Is there something that requires such a different/unusual behaviour in
>> >> remoteproc ?
>> >>
>
> There's nothing unusual in remoteproc that forces us to stay with this
> model; however the parts related to the REMOTEPROC config is useless by
> themselves.
>
I'm afraid that the "supporting" arguments you're using are generic
and not specific to remoteproc. Although as Jani mentioned and pointed
to the documentation, remoteproc is {mis,ab}using 'select' leading to
issues elsewhere.
In the long term we might want to switch to 'select' and attribute
kconfig like Jani suggested. But in the short term we want to avoid
select-ing things just for our "convenience".
Thanks
Emil
[PATCH 2/3] drm/fb_cma_helper: Add panic handling
Hi Daniel,
On Thursday 06 Oct 2016 11:12:40 Daniel Vetter wrote:
> On Wed, Oct 05, 2016 at 09:36:17PM +0200, Noralf Trønnes wrote:
> > Den 05.10.2016 15:22, skrev Laurent Pinchart:
> > > On Sunday 11 Sep 2016 20:47:41 Noralf Trønnes wrote:
> > > > This enables panic message output for fb cma helper framebuffers.
> > > >
> > > > Signed-off-by: Noralf Trønnes
> > > > ---
> > > >
> > > > drivers/gpu/drm/drm_fb_cma_helper.c | 10 ++
> > > > 1 file changed, 10 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > b/drivers/gpu/drm/drm_fb_cma_helper.c index 1fd6eac..2f1b012 100644
> > > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > @@ -126,9 +126,19 @@ int drm_fb_cma_create_handle(struct
> > > > drm_framebuffer
> > > > *fb, }
> > > >
> > > > EXPORT_SYMBOL(drm_fb_cma_create_handle);
> > > >
> > > > +static void *drm_fb_cma_panic_vmap(struct drm_framebuffer *fb)
> > > > +{
> > > > + struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> > > > + struct drm_gem_cma_object *cma_obj = fb_cma->obj[0];
> > > > +
> > > > + /* A PRIME imported buffer will not have it's vaddr set. */
> > >
> > > Does this mean that, if the framebuffer that happens to be displayed
> > > when a panic occurs is imported, no message will be printed ? I
> > > understand how supporting such cases is difficult, but I'm wondering how
> > > we could proceed to ensure that a panic can be displayed in most (if not
> > > all) cases.
> >
> > If we can vmap all cma buffers, then it's simple:
> > - Add dma_buf_vmap() call to drm_gem_cma_prime_import_sg_table()
> > - Add dma_buf_vunmap() call to drm_gem_cma_free_object()
> >
> > If not then it looks more complicated, since this is atomic context.
> > There is dma_buf_kmap_atomic(), but there are no users.
> > And drm_gem_prime_dmabuf_ops doesn't support .kmap_atomic either
> > (returns NULL).
> >
> > omapdrm is the only dma_buf provider I can find that actually uses
> > kmap_atomic() instead of just returning NULL or relying on an existing
> > virtual address. It has it's own .gem_prime_import/export functions to
> > accomplish this.
>
> dma_buf's kmap_atomic is a bit ill-defined, since atm it's not clear how
> to get the buffer pinned in the first place. Originally the plan was to
> use begin/end_cpu_access for this (which again can block, so not suitable
> for panic context). But that was kinda reused as the coherence control
> interface, and doesn't really pin anything. I guess we should probably
> just nuke the kmap interface part of dma_buf, the idea was to use that for
> ttm, but that never really happened.
I agree with you, and I'd go one step further: we need to define very
precisely the semantics of the dmabuf operations. Drivers cache various
information in different ways, making interoperability very fragile.
> > > Similarly, it looks like your code only handles single-planar formats,
> > > but there's no explicit check for that in patch 1/3. The easiest fix is
> > > to reject multi-planar framebuffers, but that would again result in
> > > silent panics in some cases.
> >
> > That's correct if we talk about the default panic_draw_xy() function:
> > drm_framebuffer_panic_draw_xy(). Most likely this function can be extended
> > to support multi-planar formats, but Daniel said we could wait with that.
> > And the driver can also implement it's own .panic_draw_xy() function.
> >
> > > > + return cma_obj ? cma_obj->vaddr : NULL;
> > >
> > > Can cma_obj be NULL here ? I thought that framebuffer objects were
> > > always created with at least one GEM object.
> >
> > I was trying to be very careful since a panic is about something gone
> > very wrong. But in that case I should have checked that fb_cma isn't NULL
> > also before dereferencing it.
> > Maybe I'm over the top paranoid :-)
>
> Yeah I think this is a bit too much paranoia. I think you can remove these
> NULL checks here, it's truly impossible. Where we really need to be
> careful is with locking, both to make sure we exclusively use trylocks for
> everything, and that we do grab all the locks (to avoid disaster when the
> part that panicked is kms itself).
--
Regards,
Laurent Pinchart
[PATCH v2 07/15] drm/omap: Use per-plane rotation property
On Thu, Oct 06, 2016 at 12:59:17PM +0300, Tomi Valkeinen wrote:
>
> On 26/09/16 19:30, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > The global mode_config.rotation_property is going away, switch over to
> > per-plane rotation_property.
> >
> > Not sure I got the annoying crtc rotation_property handling right.
> > Might work, or migth not.
> >
> > v2: Drop the BIT()
> > Don't create rotation property twice for each primary plane
> >
> > Cc: Tomi Valkeinen
> > Cc: Rob Clark
> > Signed-off-by: Ville Syrjälä
> > ---
>
>
> > diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> > b/drivers/gpu/drm/omapdrm/omap_plane.c
> > index 6ddaa5ea4b6b..b272f810989e 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> > @@ -211,9 +211,16 @@ void omap_plane_install_properties(struct drm_plane
> > *plane,
> > struct omap_drm_private *priv = dev->dev_private;
> >
> > if (priv->has_dmm) {
> > - struct drm_property *prop = dev->mode_config.rotation_property;
> > -
> > - drm_object_attach_property(obj, prop, DRM_ROTATE_0);
> > + if (!plane->rotation_property)
> > + drm_plane_create_rotation_property(plane,
> > + DRM_ROTATE_0,
> > + DRM_ROTATE_0 |
> > DRM_ROTATE_90 |
> > + DRM_ROTATE_180 |
> > DRM_ROTATE_270 |
> > + DRM_REFLECT_X |
> > DRM_REFLECT_Y);
> > +
> > + if (plane->rotation_property && obj != &plane->base)
> > + drm_object_attach_property(obj,
> > plane->rotation_property,
> > + DRM_ROTATE_0);
>
> I think this could use a short comment, as it's not obvious wth is going
> on here =).
/* Attach the rotation property also to the crtc object */ ?
>
> Otherwise both omap patches look fine, and test fine.
>
> Reviewed-by: Tomi Valkeinen
>
> Tomi
>
--
Ville Syrjälä
Intel OTC
[PATCH v2 07/15] drm/omap: Use per-plane rotation property
On 06/10/16 13:30, Ville Syrjälä wrote:
> On Thu, Oct 06, 2016 at 12:59:17PM +0300, Tomi Valkeinen wrote:
>>
>> On 26/09/16 19:30, ville.syrjala at linux.intel.com wrote:
>>> From: Ville Syrjälä
>>>
>>> The global mode_config.rotation_property is going away, switch over to
>>> per-plane rotation_property.
>>>
>>> Not sure I got the annoying crtc rotation_property handling right.
>>> Might work, or migth not.
>>>
>>> v2: Drop the BIT()
>>> Don't create rotation property twice for each primary plane
>>>
>>> Cc: Tomi Valkeinen
>>> Cc: Rob Clark
>>> Signed-off-by: Ville Syrjälä
>>> ---
>>
>>
>>> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
>>> b/drivers/gpu/drm/omapdrm/omap_plane.c
>>> index 6ddaa5ea4b6b..b272f810989e 100644
>>> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
>>> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
>>> @@ -211,9 +211,16 @@ void omap_plane_install_properties(struct drm_plane
>>> *plane,
>>> struct omap_drm_private *priv = dev->dev_private;
>>>
>>> if (priv->has_dmm) {
>>> - struct drm_property *prop = dev->mode_config.rotation_property;
>>> -
>>> - drm_object_attach_property(obj, prop, DRM_ROTATE_0);
>>> + if (!plane->rotation_property)
>>> + drm_plane_create_rotation_property(plane,
>>> + DRM_ROTATE_0,
>>> + DRM_ROTATE_0 |
>>> DRM_ROTATE_90 |
>>> + DRM_ROTATE_180 |
>>> DRM_ROTATE_270 |
>>> + DRM_REFLECT_X |
>>> DRM_REFLECT_Y);
>>> +
>>> + if (plane->rotation_property && obj != &plane->base)
>>> + drm_object_attach_property(obj,
>>> plane->rotation_property,
>>> + DRM_ROTATE_0);
>>
>> I think this could use a short comment, as it's not obvious wth is going
>> on here =).
>
> /* Attach the rotation property also to the crtc object */ ?
Yes, sounds fine to me.
Tomi
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161006/9d0bd3d7/attachment.sig>
[Intel-gfx] [PATCH 4/6] drm/i915/gen9: Make skl_wm_level per-plane
Op 05-10-16 om 22:33 schreef Paulo Zanoni:
> Em Qua, 2016-10-05 Ã s 11:33 -0400, Lyude escreveu:
>> Having skl_wm_level contain all of the watermarks for each plane is
>> annoying since it prevents us from having any sort of object to
>> represent a single watermark level, something we take advantage of in
>> the next commit to cut down on all of the copy paste code in here.
> I'd like to start my review pointing that I really like this patch. I
> agree the current form is annoying.
>
> See below for some details.
>
>
>> Signed-off-by: Lyude
>> Cc: Maarten Lankhorst
>> Cc: Ville Syrjälä
>> Cc: Matt Roper
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 6 +-
>> drivers/gpu/drm/i915/intel_drv.h | 6 +-
>> drivers/gpu/drm/i915/intel_pm.c | 208 +--
>>
>> 3 files changed, 100 insertions(+), 120 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index d26e5999..0f97d43 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1648,9 +1648,9 @@ struct skl_wm_values {
>> };
>>
>> struct skl_wm_level {
>> -bool plane_en[I915_MAX_PLANES];
>> -uint16_t plane_res_b[I915_MAX_PLANES];
>> -uint8_t plane_res_l[I915_MAX_PLANES];
>> +bool plane_en;
>> +uint16_t plane_res_b;
>> +uint8_t plane_res_l;
>> };
>>
>> /*
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index 35ba282..d684f4f 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -468,9 +468,13 @@ struct intel_pipe_wm {
>> bool sprites_scaled;
>> };
>>
>> -struct skl_pipe_wm {
>> +struct skl_plane_wm {
>> struct skl_wm_level wm[8];
>> struct skl_wm_level trans_wm;
>> +};
>> +
>> +struct skl_pipe_wm {
>> +struct skl_plane_wm planes[I915_MAX_PLANES];
>> uint32_t linetime;
>> };
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c
>> b/drivers/gpu/drm/i915/intel_pm.c
>> index af96888..250f12d 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -3668,67 +3668,52 @@ static int
>> skl_compute_wm_level(const struct drm_i915_private *dev_priv,
>> struct skl_ddb_allocation *ddb,
>> struct intel_crtc_state *cstate,
>> + struct intel_plane *intel_plane,
>> int level,
>> struct skl_wm_level *result)
>> {
>> struct drm_atomic_state *state = cstate->base.state;
>> struct intel_crtc *intel_crtc = to_intel_crtc(cstate-
>>> base.crtc);
>> -struct drm_plane *plane;
>> -struct intel_plane *intel_plane;
>> -struct intel_plane_state *intel_pstate;
>> +struct drm_plane *plane = &intel_plane->base;
>> +struct intel_plane_state *intel_pstate = NULL;
>> uint16_t ddb_blocks;
>> enum pipe pipe = intel_crtc->pipe;
>> int ret;
>> +int i = skl_wm_plane_id(intel_plane);
>> +
>> +if (state)
>> +intel_pstate =
>> +intel_atomic_get_existing_plane_state(state,
>> + intel_
>> plane);
>>
>> /*
>> - * We'll only calculate watermarks for planes that are
>> actually
>> - * enabled, so make sure all other planes are set as
>> disabled.
>> + * Note: If we start supporting multiple pending atomic
>> commits against
>> + * the same planes/CRTC's in the future, plane->state will
>> no longer be
>> + * the correct pre-state to use for the calculations here
>> and we'll
>> + * need to change where we get the 'unchanged' plane data
>> from.
>> + *
>> + * For now this is fine because we only allow one queued
>> commit against
>> + * a CRTC. Even if the plane isn't modified by this
>> transaction and we
>> + * don't have a plane lock, we still have the CRTC's lock,
>> so we know
>> + * that no other transactions are racing with us to update
>> it.
>> */
>> -memset(result, 0, sizeof(*result));
>> -
>> -for_each_intel_plane_mask(&dev_priv->drm,
>> - intel_plane,
>> - cstate->base.plane_mask) {
>> -int i = skl_wm_plane_id(intel_plane);
>> -
>> -plane = &intel_plane->base;
>> -intel_pstate = NULL;
>> -if (state)
>> -intel_pstate =
>> -intel_atomic_get_existing_plane_stat
>> e(state,
>> -
>> intel_plane);
>> +if (!intel_pstate)
>> +intel_pstate = to_intel_plane_state(plane->state);
>>
>> -/*
>> - * Note: If we start supporting multiple pending
>> atomic commits
>> - * against the same planes/CRTC's in the future,
>> plane->state
>> - * will no longer be the correct pre-state to use
>> for the
>> - * calculations here
[PATCH v9 17/19] drm/virtio: kconfig: Fix recursive dependency issue.
On 6 October 2016 at 10:37, Peter Griffin wrote:
> In fact the help text for VIRTIO even states this option should be selected
> by any driver which implements virtio.
>
Almost but not quite. It says:
"This option is selected by any driver which implements the virtio _bus_"
REMOTEPROC obviously does that while the ST SLIM driver does not. Thus
the latter should _not_ select, be that explicitly or implicitly via
REMOTEPROC, the symbol.
>>
>> People tend to abuse select because it's "convenient". If you depend,
>> but some of your dependencies aren't met, you're in for some digging
>> through Kconfig to find the missing deps. Just to make the option you
>> want visible in menuconfig. If you instead select something with
>> dependencies, it'll be right most of the time, and it's "convenient",
>> until it breaks. (And hey, it usually breaks for someone else with some
>> other config, so it's still convenient for you.)
>
> I'm sure they do but in this case it is actually the use of 'depends on'
> which has caused the breakage and inconvenience for somebody else and sadly
> this
> inconvienice is still on-going due to this patch not being applied or getting
> an
> acked-by from the appropriate maintainers.
>
Surely you're not saying that pre-existing driver following the
documentation, is 'causing breakage' for a new driver {ab,mis}using a
feature ?
This reminds me an old saying: "If the shoe doesnât fit, it doesnât
mean there is anything wrong with your feet."
You seem to be suggesting the opposite ?
>>
>> Perhaps kconfig should complain about selecting visible symbols and
>> symbols with dependencies.
>
> That sounds like it would be a useful addition.
>
> Is it possible to get this patch applied or an acked-by to avoid further delay
> to the fdma series?
>
Please don't apply duct tape, especially where it's _not_ needed.
$ sed -i s/select REMOTEPROC/depends on REMOTEPROC/ drivers/remoteproc/Kconfig
... will resolve things in the right place. The alternative will lead
to random issues in other subsystems.
Regards,
Emil
[PATCH v9 2/4] drm: Add API for capturing frame CRCs
On 6 October 2016 at 09:56, Tomeu Vizoso wrote: > Adds files and directories to debugfs for controlling and reading frame > CRCs, per CRTC: > > dri/0/crtc-0/crc > dri/0/crtc-0/crc/control > dri/0/crtc-0/crc/data > > Drivers can implement the set_crc_source callback() in drm_crtc_funcs to > start and stop generating frame CRCs and can add entries to the output > by calling drm_crtc_add_crc_entry. > > v2: > - Lots of good fixes suggested by Thierry. > - Added documentation. > - Changed the debugfs layout. > - Moved to allocate the entries circular queue once when frame > generation gets enabled for the first time. > v3: > - Use the control file just to select the source, and start and stop > capture when the data file is opened and closed, respectively. > - Make variable the number of CRC values per entry, per source. > - Allocate entries queue each time we start capturing as now there > isn't a fixed number of CRC values per entry. > - Store the frame counter in the data file as a 8-digit hex number. > - For sources that cannot provide useful frame numbers, place > in the frame field. > > v4: > - Build only if CONFIG_DEBUG_FS is enabled. > - Use memdup_user_nul. > - Consolidate calculation of the size of an entry in a helper. > - Add 0x prefix to hex numbers in the data file. > - Remove unnecessary snprintf and strlen usage in read callback. > > v5: > - Made the crcs array in drm_crtc_crc_entry fixed-size > - Lots of other smaller improvements suggested by Emil Velikov > > v7: > - Move definition of drm_debugfs_crtc_crc_add to drm_internal.h > > v8: > - Call debugfs_remove_recursive when we fail to create the minor > device > > v9: > - Register the debugfs directory for a crtc from > drm_crtc_register_all() > > Signed-off-by: Tomeu Vizoso > --- > > Documentation/gpu/drm-uapi.rst| 6 + > drivers/gpu/drm/Makefile | 3 +- > drivers/gpu/drm/drm_crtc.c| 34 +++- > drivers/gpu/drm/drm_debugfs.c | 34 +++- > drivers/gpu/drm/drm_debugfs_crc.c | 351 > ++ > drivers/gpu/drm/drm_internal.h| 16 ++ > include/drm/drm_crtc.h| 41 + > include/drm/drm_debugfs_crc.h | 73 > 8 files changed, 555 insertions(+), 3 deletions(-) > create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c > create mode 100644 include/drm/drm_debugfs_crc.h > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > index 1ba301cebe16..de3ac9f90f8f 100644 > --- a/Documentation/gpu/drm-uapi.rst > +++ b/Documentation/gpu/drm-uapi.rst > @@ -216,3 +216,9 @@ interfaces. Especially since all hardware-acceleration > interfaces to > userspace are driver specific for efficiency and other reasons these > interfaces can be rather substantial. Hence every driver has its own > chapter. > + > +Testing and validation > +== > + > +.. kernel-doc:: drivers/gpu/drm/drm_debugfs_crc.c > + :doc: CRC ABI > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 25c720454017..74579d2e796e 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -9,7 +9,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \ > drm_scatter.o drm_pci.o \ > drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \ > drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \ > - drm_info.o drm_debugfs.o drm_encoder_slave.o \ > + drm_info.o drm_encoder_slave.o \ > drm_trace_points.o drm_global.o drm_prime.o \ > drm_rect.o drm_vma_manager.o drm_flip_work.o \ > drm_modeset_lock.o drm_atomic.o drm_bridge.o \ > @@ -23,6 +23,7 @@ drm-$(CONFIG_PCI) += ati_pcigart.o > drm-$(CONFIG_DRM_PANEL) += drm_panel.o > drm-$(CONFIG_OF) += drm_of.o > drm-$(CONFIG_AGP) += drm_agpsupport.o > +drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o > > drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ > drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \ > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 2d7bedf28647..151ff9805de1 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -40,7 +40,7 @@ > #include > #include > #include > -#include > +#include > > #include "drm_crtc_internal.h" > #include "drm_internal.h" > @@ -126,6 +126,10 @@ static int drm_crtc_register_all(struct drm_device *dev) > ret = crtc->funcs->late_register(crtc); > if (ret) Here we want to teardown the already created debugfs entries. > return ret; > + > + ret = drm_debugfs_crtc_add(crtc); > + if (ret) > + return ret; IIRC v8 made sure we don't error out if adding debugfs entries fails. Pretty su
[PATCH 0/6] Start of skl watermark cleanup
Op 05-10-16 om 17:33 schreef Lyude: > While it (mostly) works, the code for handling watermarks on Skylake has been > kind of ugly for a while. As well a lot of it isn't that friendly to atomic > transactions, Lots of copy paste, redundant wm values, etc. While this isn't a > full cleanup, it's a good start. As well, we add a couple of features for > making debugging watermarks a little easier. It's a good start, with the review comments addressed and rebased I think it's good to go. :) Feel free to add my r-b. ~Maarten
[PATCH v3 3/3] drm/bridge: add Silicon Image SiI8620 driver
On 10/06/2016 02:47 PM, Daniel Vetter wrote: > On Thu, Oct 06, 2016 at 11:02:58AM +0200, Andrzej Hajda wrote: >> Hi Daniel, Archit, >> >> On 30.09.2016 12:33, Andrzej Hajda wrote: >>> On 30.09.2016 12:07, Daniel Vetter wrote: On Fri, Sep 30, 2016 at 09:30:16AM +0530, Archit Taneja wrote: > Hi Andrezj, > > On 09/26/2016 07:10 PM, Andrzej Hajda wrote: >> SiI8620 transmitter converts eTMDS/HDMI signal to MHL 3.0. >> It is controlled via I2C bus. Its interaction with other >> devices in video pipeline is performed mainly on HW level. >> The only interaction it does on device driver level is >> filtering-out unsupported video modes, it exposes drm_bridge >> interface to perform this operation. > The patchset looks good to me. Is the MHL header patch > accepted? I was wondering how we pull this in. > > +Daniel I think someone with real clue about what MHL is needs to review that header. Also I have no idea why that's under video/, is there another driver in media we want to share this with? -Daniel >>> I have put it into include/linux as MHL could be used to transmit: >>> - video, >>> - audio, >>> - remote control protocol (input device), >>> - ... embed other protocols (USB for example), >>> >>> But since I am not aware of other MHL users in near future >>> I can put the header together with the bridge driver. >> >> These patches are hanging on the list for almost year, >> since Archit decided to review it (thanks Archit), I would >> like to end this process. >> >> The options I see: >> 1. Leave it as is, mhl.h is like hdmi.h - it can server for >> multiple subsystems. I guess it can be hard to find >> MHL specialist to review it as it does not seems >> to be popular subject, on the other side it is only >> in-kernel header so it should pose serious danger. >> 2. Move the header to some of drm dirs: >> a) drivers/gpu/drm/bridge/ >> b) include/drm/bridge/ >> c) include/drm/ >> ... >> 3. Incorporate it into drivers/gpu/drm/bridge/sil-sii8620.h >> This is the least problematic solution, but possible >> future abstraction of MHL will be more noisy. >> >> Daniel, which option do you prefer? For me any option >> is OK, I just want to end this little bit frustrating process. > > I just brought this up as a question, I don't personally care all that > much. Except for option 2a) I think they are all ok (we only have internal > headers outside of include/). Whatever you&Archit can agree on is fine > with me (and then Archit can all push it directly to drm-misc). I think we can go with 2b), and later move it elsewhere if other places need it. Thanks, Archit > -Daniel > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v9 17/19] drm/virtio: kconfig: Fix recursive dependency issue.
Hi Peter,
On 6 October 2016 at 11:48, Peter Griffin wrote:
> Hi Emil,
>
> On Wed, 21 Sep 2016, Emil Velikov wrote:
>
>> On 20 September 2016 at 09:32, Peter Griffin
>> wrote:
>> > Hi Emil,
>> >
>> > On Tue, 20 Sep 2016, Emil Velikov wrote:
>> >
>> >> On 5 September 2016 at 14:16, Peter Griffin
>> >> wrote:
>> >> > ST_SLIM_REMOTEPROC must select REMOTEPROC, which exposes the following
>> >> > recursive dependency.
>> >
>> >
>> >> >
>> >> From a humble skim through remoteproc, drm and a few other subsystems
>> >> I think the above is wrong. All the drivers (outside of remoteproc),
>> >> that I've seen, depend on the core component, they don't select it.
>> >
>> > I will let Bjorn comment on the remoteproc subsystem Kconfig design, and
>> > why it is like it is.
>> >
>> > For this particular SLIM_RPROC I have added it to Kconfig in keeping with
>> > all
>> > the other drivers in the remoteproc subsystem which has exposed this
>> > recursive
>> > dependency issue.
>> >
>> > For this particular kconfig symbol a quick grep reveals more drivers in
>> > the kernel using 'select', than 'depend on'
>> >
>> > git grep "select VIRTIO" | wc -l
>> > 14
>> >
>> > git grep "depends on VIRTIO" | wc -l
>> > 10
>> >
>> Might be worth taking a closer look into these at some point.
>
> VIRTIO has no dependencies, and is a non visible symbol. Its Kconfig help also
> mentions that drivers should select it.
>
This is a (un)fortunate detail cannot/should not overrule the other
arguments I've mentioned, should it ?
>>
>> >
>> >> Furthermore most places explicitly hide the drivers from the menu if
>> >> the core component isn't enabled.
>> >
>> > Remoteproc subsystem takes a different approach, the core code is only
>> > enabled
>> > if a driver which relies on it is enabled. This IMHO makes sense, as
>> > remoteproc is not widely used (only a few particular ARM SoC's).
>> >
>> > It is true that for subsystems which rely on the core component being
>> > explicitly enabled, they often tend to hide drivers which depend on it
>> > from the menu unless it is. This also makes sense.
>> >
>> >>
>> >> Is there something that requires such a different/unusual behaviour in
>> >> remoteproc ?
>> >>
>> >
>> > I'm not sure it is that unusual...looking at config USB, it selects
>> > USB_COMMON in
>> > mfd subsystem, client drivers select MFD_CORE.
>> >
>> On the USB case I'm not sure what the reasoning behind the USB vs
>> USB_COMMON split. In seems that one could just fold them, but that's
>> another topic. On the MFD side... it follows the select {,mis,ab}use.
>> With one (the only one?) MFD driver not using/selecting MFD_CORE doing
>> it's own version of mfd_add_devices... which could be reworked,
>> possibly.
>
> Much like selecting VIRTIO in this patch, MFD_CORE is a non visible symbol
> with no dependencies so it matches the documentation Jani referenced.
>
> I personally think the approach taken makes sense, as why would you want to
> have
> a CONFIG_MFD_CORE=y symbol being enabled unless you actually have a MFD device
> which uses it also enabled in your kernel?
>
> It seems to me a good solution to make the client drivers select the core
> component so that it only gets enabled if at least one driver requires it.
> This avoids having useless core code which will never be used compiled into
> the
> kernel and in the end a smaller kernel size for a given kernel configuration
> (better
> cache use etc etc).
>
>> > I've added Arnd to this thread, as I've seen lots of Kconfig patches from
>> > him
>> > recently, maybe he has some thoughts on whether this is the correct fix or
>> > not?
>> >
>> Ack. Fwiw, I believe that the reasoning put by Jani is perfeclty
>> reasonable, but it'll be great to hear others as well.
>
> Yes me to. However I don't think anything in this patch is at odds with the
> documentation Jani has referenced.
>
It case it's not clear, let me elaborate:
Yes, using depend might not be the most user-friendly thing to do and
in the long term we might want to move to select.
Yes, I agree with the argument about symbol visibility but that is not
the only contributing factor.
If one wants to pick on specific users which opt for $driver select
$core they must do the same for $driver depends on $core. Check the
number 'in favour" of each case and draw their conclusions ;-)
In particular: both MFD_CORE and USB_COMMON, are _optional_ as only
some drivers depends on them. Thus giving them as an example is
wrong/irrelevant, I'm afraid.
Thanks
Emil
[PATCH v9 2/4] drm: Add API for capturing frame CRCs
On 10/06/2016 01:13 PM, Emil Velikov wrote: > On 6 October 2016 at 09:56, Tomeu Vizoso > wrote: >> Adds files and directories to debugfs for controlling and reading frame >> CRCs, per CRTC: >> >> dri/0/crtc-0/crc >> dri/0/crtc-0/crc/control >> dri/0/crtc-0/crc/data >> >> Drivers can implement the set_crc_source callback() in drm_crtc_funcs to >> start and stop generating frame CRCs and can add entries to the output >> by calling drm_crtc_add_crc_entry. >> >> v2: >> - Lots of good fixes suggested by Thierry. >> - Added documentation. >> - Changed the debugfs layout. >> - Moved to allocate the entries circular queue once when frame >> generation gets enabled for the first time. >> v3: >> - Use the control file just to select the source, and start and stop >> capture when the data file is opened and closed, respectively. >> - Make variable the number of CRC values per entry, per source. >> - Allocate entries queue each time we start capturing as now there >> isn't a fixed number of CRC values per entry. >> - Store the frame counter in the data file as a 8-digit hex number. >> - For sources that cannot provide useful frame numbers, place >> in the frame field. >> >> v4: >> - Build only if CONFIG_DEBUG_FS is enabled. >> - Use memdup_user_nul. >> - Consolidate calculation of the size of an entry in a helper. >> - Add 0x prefix to hex numbers in the data file. >> - Remove unnecessary snprintf and strlen usage in read callback. >> >> v5: >> - Made the crcs array in drm_crtc_crc_entry fixed-size >> - Lots of other smaller improvements suggested by Emil Velikov >> >> v7: >> - Move definition of drm_debugfs_crtc_crc_add to drm_internal.h >> >> v8: >> - Call debugfs_remove_recursive when we fail to create the minor >> device >> >> v9: >> - Register the debugfs directory for a crtc from >> drm_crtc_register_all() >> >> Signed-off-by: Tomeu Vizoso >> --- >> >> Documentation/gpu/drm-uapi.rst| 6 + >> drivers/gpu/drm/Makefile | 3 +- >> drivers/gpu/drm/drm_crtc.c| 34 +++- >> drivers/gpu/drm/drm_debugfs.c | 34 +++- >> drivers/gpu/drm/drm_debugfs_crc.c | 351 >> ++ >> drivers/gpu/drm/drm_internal.h| 16 ++ >> include/drm/drm_crtc.h| 41 + >> include/drm/drm_debugfs_crc.h | 73 >> 8 files changed, 555 insertions(+), 3 deletions(-) >> create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c >> create mode 100644 include/drm/drm_debugfs_crc.h >> >> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst >> index 1ba301cebe16..de3ac9f90f8f 100644 >> --- a/Documentation/gpu/drm-uapi.rst >> +++ b/Documentation/gpu/drm-uapi.rst >> @@ -216,3 +216,9 @@ interfaces. Especially since all hardware-acceleration >> interfaces to >> userspace are driver specific for efficiency and other reasons these >> interfaces can be rather substantial. Hence every driver has its own >> chapter. >> + >> +Testing and validation >> +== >> + >> +.. kernel-doc:: drivers/gpu/drm/drm_debugfs_crc.c >> + :doc: CRC ABI >> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile >> index 25c720454017..74579d2e796e 100644 >> --- a/drivers/gpu/drm/Makefile >> +++ b/drivers/gpu/drm/Makefile >> @@ -9,7 +9,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \ >> drm_scatter.o drm_pci.o \ >> drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \ >> drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \ >> - drm_info.o drm_debugfs.o drm_encoder_slave.o \ >> + drm_info.o drm_encoder_slave.o \ >> drm_trace_points.o drm_global.o drm_prime.o \ >> drm_rect.o drm_vma_manager.o drm_flip_work.o \ >> drm_modeset_lock.o drm_atomic.o drm_bridge.o \ >> @@ -23,6 +23,7 @@ drm-$(CONFIG_PCI) += ati_pcigart.o >> drm-$(CONFIG_DRM_PANEL) += drm_panel.o >> drm-$(CONFIG_OF) += drm_of.o >> drm-$(CONFIG_AGP) += drm_agpsupport.o >> +drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o >> >> drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ >> drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o >> \ >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >> index 2d7bedf28647..151ff9805de1 100644 >> --- a/drivers/gpu/drm/drm_crtc.c >> +++ b/drivers/gpu/drm/drm_crtc.c >> @@ -40,7 +40,7 @@ >> #include >> #include >> #include >> -#include >> +#include >> >> #include "drm_crtc_internal.h" >> #include "drm_internal.h" >> @@ -126,6 +126,10 @@ static int drm_crtc_register_all(struct drm_device *dev) >> ret = crtc->funcs->late_register(crtc); >> if (ret) > Here we want to teardown the already created debugfs entries. Yeah, I was a bit puzzled by the existing behaviour of drm_modeset
[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support
On 10/06/2016 12:51 PM, Maxime Ripard wrote: > Hi Archit, > > On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote: >> Hi Maxime, >> >> On 09/30/2016 08:07 PM, Maxime Ripard wrote: >>> Some boards have an entirely passive RGB to VGA bridge, based on either >>> DACs or resistor ladders. >>> >>> Those might or might not have an i2c bus routed to the VGA connector in >>> order to access the screen EDIDs. >>> >>> Add a bridge that doesn't do anything but expose the modes available on the >>> screen, either based on the EDIDs if available, or based on the XGA >>> standards. >>> >>> Acked-by: Rob Herring >>> Signed-off-by: Maxime Ripard >>> --- >>> .../bindings/display/bridge/rgb-to-vga-bridge.txt | 48 + >>> drivers/gpu/drm/bridge/Kconfig | 7 + >>> drivers/gpu/drm/bridge/Makefile| 1 + >>> drivers/gpu/drm/bridge/rgb-to-vga.c| 229 >>> + >>> 4 files changed, 285 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt >>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c >>> >>> diff --git >>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt >>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt >>> new file mode 100644 >>> index ..a8375bc1f9cb >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt >>> @@ -0,0 +1,48 @@ >>> +Dumb RGB to VGA bridge >>> +-- >>> + >>> +This binding is aimed for dumb RGB to VGA bridges that do not require >>> +any configuration. >>> + >>> +Required properties: >>> + >>> +- compatible: Must be "rgb-to-vga-bridge" >> >> I'd talked to Laurent on IRC if he's okay with this. And I guess you to >> had discussed it during XDC too. He's suggested that it'd be better to >> have the compatible string as "simple-vga-dac". > > I just wished this bikeshedding had taken place publicly and be > actually part of that discussion, but yeah, ok. Sorry about that. I'd pinged him for an Ack, the discussion went more than that :) > >> Some of the reasons behind having this: >> >> - We don't need to specify "rgb" in the compatible string since most >> simple VGA DACs can only work with an RGB input. > > Ok. > >> - Also, with "dac" specified in the string, we don't need to >> specifically mention "bridge" in the string. Also, bridge is a drm >> specific term. >> >> - "simple" is considered because it's an unconfigurable bridge, and it >> might be misleading for other VGA DACs to not use "vga-dac". > > All those "simple" bindings are just the biggest lie we ever > told. It's simple when you introduce it, and then grows into something > much more complicated than a non-simple implementation. "simple" here is supposed to mean that it's an unconfigurable RGB to VGA DAC. This isn't supposed to follow the simple-panel model, where you add the "simple-panel" string in the compatible node, along with you chip specific compatible string. In other words, this driver shouldn't be touched again in the future :) If someone wants to write a RGB to VGA driver which is even slightly configurable, they'll need to write a new bridge driver. Thanks, Archit > >> What do you think about this? If you think it's good, would it be >> possible for you to change this? I guess it's okay for the rest of >> the patch to stay the same. > > I'll update and respin the serie. > > Thanks, > Maxime > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v10 0/3] Secure Memory Allocation Framework
When using dmabuf the devices have to attach themselves on the buffer before map it so you can know which devices will use the buffer before allocate it when the first dma_buf_map_attachment() is called (defered allocation) Split dmabuf attach/map_attachment is not really done in drm or v4l2 but patch like this one are going in this direction: http://www.spinics.net/lists/linux-media/msg104480.html I agree that device constraint enumeration is still missing, for smaf I have just to test by using device dma_coherent_mask field to check if cma allocator can be used for allocation. I guess that if you can export more device information to userland we can get access to them in kernel too. 2016-10-05 15:43 GMT+02:00 Daniel Vetter : > On Wed, Oct 05, 2016 at 03:40:14PM +0200, Benjamin Gaignard wrote: >> because with ion it is up to userland to decide which heap to use >> and until now userland doesn't have any way to get device constraints... >> >> I will prefer let a central allocator (in kernel) decide from the >> attached devices >> which allocator is the best. It is what I have implemented in smaf. > > And how does that work? Atm there's no interfaces at all in the kernel to > allocate a buffer suitable for 2 devices at the same time. Seems > incomplete if this is the direction you want to go to. Also, it's against > the direction we all discussed ad XDC, where the clear consensus was to > have most of that haggling in userspace (with the kernel exporting a > little bit more information to userspace than what it does now). > -Daniel > >> >> Benjamin >> >> >> 2016-10-05 15:19 GMT+02:00 Daniel Vetter : >> > On Tue, Oct 04, 2016 at 01:47:21PM +0200, Benjamin Gaignard wrote: >> >> version 10 changes: >> >> - rebased on kernel 4.8 tag >> >> - minor typo fix >> >> >> >> version 9 changes: >> >> - rebased on 4.8-rc5 >> >> - struct dma_attrs doesn't exist anymore so update CMA allocator >> >>to compile with new dma_*_attr functions >> >> - add example SMAF use case in cover letter >> >> >> >> version 8 changes: >> >> - rework of the structures used within ioctl >> >>by adding a version field and padding to be futur proof >> >> - rename fake secure moduel to test secure module >> >> - fix the various remarks done on the previous patcheset >> >> >> >> version 7 changes: >> >> - rebased on kernel 4.6-rc7 >> >> - simplify secure module API >> >> - add vma ops to be able to detect mmap/munmap calls >> >> - add ioctl to get number and allocator names >> >> - update libsmaf with adding tests >> >>https://git.linaro.org/people/benjamin.gaignard/libsmaf.git >> >> - add debug log in fake secure module >> >> >> >> version 6 changes: >> >> - rebased on kernel 4.5-rc4 >> >> - fix mmapping bug while requested allocation size isn't a a multiple of >> >>PAGE_SIZE (add a test for this in libsmaf) >> >> >> >> version 5 changes: >> >> - rebased on kernel 4.3-rc6 >> >> - rework locking schema and make handle status use an atomic_t >> >> - add a fake secure module to allow performing tests without trusted >> >>environment >> >> >> >> version 4 changes: >> >> - rebased on kernel 4.3-rc3 >> >> - fix missing EXPORT_SYMBOL for smaf_create_handle() >> >> >> >> version 3 changes: >> >> - Remove ioctl for allocator selection instead provide the name of >> >>the targeted allocator with allocation request. >> >>Selecting allocator from userland isn't the prefered way of working >> >>but is needed when the first user of the buffer is a software >> >> component. >> >> - Fix issues in case of error while creating smaf handle. >> >> - Fix module license. >> >> - Update libsmaf and tests to care of the SMAF API evolution >> >>https://git.linaro.org/people/benjamin.gaignard/libsmaf.git >> >> >> >> version 2 changes: >> >> - Add one ioctl to allow allocator selection from userspace. >> >>This is required for the uses case where the first user of >> >>the buffer is a software IP which can't perform dma_buf attachement. >> >> - Add name and ranking to allocator structure to be able to sort them. >> >> - Create a tiny library to test SMAF: >> >>https://git.linaro.org/people/benjamin.gaignard/libsmaf.git >> >> - Fix one issue when try to secure buffer without secure module >> >> registered >> >> >> >> SMAF aim to solve two problems: allocating memory that fit with hardware >> >> IPs >> >> constraints and secure those data from bus point of view. >> >> >> >> One example of SMAF usage is camera preview: on SoC you may use either an >> >> USB >> >> webcam or the built-in camera interface and the frames could be send >> >> directly >> >> to the dipslay Ip or handle by GPU. >> >> Most of USB interfaces and GPU have mmu but almost all built-in camera >> >> interace and display Ips don't have mmu so when selecting how allocate >> >> buffer you need to be aware of each devices constraints (contiguous >> >> memroy, >> >> stride, boundary, alignment ...). >> >> ION has solve this p
[PATCH v9 2/4] drm: Add API for capturing frame CRCs
On 6 October 2016 at 12:33, Tomeu Vizoso wrote: > On 10/06/2016 01:13 PM, Emil Velikov wrote: >> On 6 October 2016 at 09:56, Tomeu Vizoso >> wrote: >>> Adds files and directories to debugfs for controlling and reading frame >>> CRCs, per CRTC: >>> >>> dri/0/crtc-0/crc >>> dri/0/crtc-0/crc/control >>> dri/0/crtc-0/crc/data >>> >>> Drivers can implement the set_crc_source callback() in drm_crtc_funcs to >>> start and stop generating frame CRCs and can add entries to the output >>> by calling drm_crtc_add_crc_entry. >>> >>> v2: >>> - Lots of good fixes suggested by Thierry. >>> - Added documentation. >>> - Changed the debugfs layout. >>> - Moved to allocate the entries circular queue once when frame >>> generation gets enabled for the first time. >>> v3: >>> - Use the control file just to select the source, and start and stop >>> capture when the data file is opened and closed, respectively. >>> - Make variable the number of CRC values per entry, per source. >>> - Allocate entries queue each time we start capturing as now there >>> isn't a fixed number of CRC values per entry. >>> - Store the frame counter in the data file as a 8-digit hex number. >>> - For sources that cannot provide useful frame numbers, place >>> in the frame field. >>> >>> v4: >>> - Build only if CONFIG_DEBUG_FS is enabled. >>> - Use memdup_user_nul. >>> - Consolidate calculation of the size of an entry in a helper. >>> - Add 0x prefix to hex numbers in the data file. >>> - Remove unnecessary snprintf and strlen usage in read callback. >>> >>> v5: >>> - Made the crcs array in drm_crtc_crc_entry fixed-size >>> - Lots of other smaller improvements suggested by Emil Velikov >>> >>> v7: >>> - Move definition of drm_debugfs_crtc_crc_add to drm_internal.h >>> >>> v8: >>> - Call debugfs_remove_recursive when we fail to create the minor >>> device >>> >>> v9: >>> - Register the debugfs directory for a crtc from >>> drm_crtc_register_all() >>> >>> Signed-off-by: Tomeu Vizoso >>> --- >>> >>> Documentation/gpu/drm-uapi.rst| 6 + >>> drivers/gpu/drm/Makefile | 3 +- >>> drivers/gpu/drm/drm_crtc.c| 34 +++- >>> drivers/gpu/drm/drm_debugfs.c | 34 +++- >>> drivers/gpu/drm/drm_debugfs_crc.c | 351 >>> ++ >>> drivers/gpu/drm/drm_internal.h| 16 ++ >>> include/drm/drm_crtc.h| 41 + >>> include/drm/drm_debugfs_crc.h | 73 >>> 8 files changed, 555 insertions(+), 3 deletions(-) >>> create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c >>> create mode 100644 include/drm/drm_debugfs_crc.h >>> >>> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst >>> index 1ba301cebe16..de3ac9f90f8f 100644 >>> --- a/Documentation/gpu/drm-uapi.rst >>> +++ b/Documentation/gpu/drm-uapi.rst >>> @@ -216,3 +216,9 @@ interfaces. Especially since all hardware-acceleration >>> interfaces to >>> userspace are driver specific for efficiency and other reasons these >>> interfaces can be rather substantial. Hence every driver has its own >>> chapter. >>> + >>> +Testing and validation >>> +== >>> + >>> +.. kernel-doc:: drivers/gpu/drm/drm_debugfs_crc.c >>> + :doc: CRC ABI >>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile >>> index 25c720454017..74579d2e796e 100644 >>> --- a/drivers/gpu/drm/Makefile >>> +++ b/drivers/gpu/drm/Makefile >>> @@ -9,7 +9,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \ >>> drm_scatter.o drm_pci.o \ >>> drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \ >>> drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \ >>> - drm_info.o drm_debugfs.o drm_encoder_slave.o \ >>> + drm_info.o drm_encoder_slave.o \ >>> drm_trace_points.o drm_global.o drm_prime.o \ >>> drm_rect.o drm_vma_manager.o drm_flip_work.o \ >>> drm_modeset_lock.o drm_atomic.o drm_bridge.o \ >>> @@ -23,6 +23,7 @@ drm-$(CONFIG_PCI) += ati_pcigart.o >>> drm-$(CONFIG_DRM_PANEL) += drm_panel.o >>> drm-$(CONFIG_OF) += drm_of.o >>> drm-$(CONFIG_AGP) += drm_agpsupport.o >>> +drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o >>> >>> drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ >>> drm_plane_helper.o drm_dp_mst_topology.o >>> drm_atomic_helper.o \ >>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >>> index 2d7bedf28647..151ff9805de1 100644 >>> --- a/drivers/gpu/drm/drm_crtc.c >>> +++ b/drivers/gpu/drm/drm_crtc.c >>> @@ -40,7 +40,7 @@ >>> #include >>> #include >>> #include >>> -#include >>> +#include >>> >>> #include "drm_crtc_internal.h" >>> #include "drm_internal.h" >>> @@ -126,6 +126,10 @@ static int drm_crtc_register_all(struct drm_device >>> *dev) >>> ret = crtc->funcs->late_
[PATCH] exynos-drm: Fix error messages to print flags and size
Hello,
I think this patch was never picked up. So just a short 'ping' from my side.
With best wishes,
Tobias
Shuah Khan wrote:
> Fix exynos_drm_gem_create() error messages to include flags and size when
> flags and size are invalid.
>
> Signed-off-by: Shuah Khan
> ---
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index cdf9f1a..4c4cb0e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -231,12 +231,12 @@ struct exynos_drm_gem *exynos_drm_gem_create(struct
> drm_device *dev,
> int ret;
>
> if (flags & ~(EXYNOS_BO_MASK)) {
> - DRM_ERROR("invalid flags.\n");
> + DRM_ERROR("invalid GEM buffer flags: %u\n", flags);
> return ERR_PTR(-EINVAL);
> }
>
> if (!size) {
> - DRM_ERROR("invalid size.\n");
> + DRM_ERROR("invalid GEM buffer size: %lu\n", size);
> return ERR_PTR(-EINVAL);
> }
>
>
[PATCH] *-symbol-check: Don't hard-code nm executable
Hi Heiko, On 3 October 2016 at 18:26, Heiko Becker wrote: > Helpful if your nm executable has a prefix based on the > architecture, for example. > Thanks for the patch. Currently one can run the tests as standalone. Personally I don't mind if we "loose" that functionality or not, but in either case we want some sane default is NM is not set. The decision whether to error/skip out or use a default value, I'll leave to you ;-) Thanks Emil
[Bug 98037] HD6450 KMS failure
https://bugs.freedesktop.org/show_bug.cgi?id=98037 --- Comment #7 from Will Lewis --- Recent test results: Kernel 3.13.6 - DVI output after boot, but with vertical, inch-wide blue-green stripes. No HDMI output. X unusable on either screen. Kernel 4.0 - DVI output after boot. No HDMI output. X works on DVI only. Kernel 4.7.2 - No DVI or HDMI output after boot. X works on DVI only. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161006/bdc8c6a4/attachment-0001.html>
[PATCH] libdrm: Update plane type enum definitions to correspond to kernel change
Hi Dhinakaran, On 23 September 2016 at 07:33, Dhinakaran Pandiyan wrote: > The positions of primary and overlay plane type enums in the kernel were > updated in > > drm/doc: Polish for drm_plane.[hc] > > So, making the change here as well. > Obviously the issue is/has been fixed elsewhere, so just a general comment: One must _not_ break API/ABI without bumping the library major. Unless you hate your users and you want them to suffer, that is ;-) Regards, Emil
[PATCH libdrm] modetest: Allow the user to specify the plane ID
On 28 September 2016 at 15:27, wrote: > From: Ville Syrjälä > > Devices can have multiple planes, so allow the user to choose between > them. > > Signed-off-by: Ville Syrjälä In the long term I'm wondering if we don't want to nuke/deprecate the clunky modetest and use a slimmed down atomic one. Either way, the patch is great but we want to update modeprint to provide plane(s) info. Otherwise one has no way of knowing which plane_id to feed without explicit knowledge about the driver/hardware. Thanks Emil
[Bug 97524] Invalid sampler settings cause full GPU reset
https://bugs.freedesktop.org/show_bug.cgi?id=97524 Nicolai Hähnle changed: What|Removed |Added QA Contact|dri-devel at lists.freedesktop |mesa-dev at lists.freedesktop. |.org|org Component|Drivers/Gallium/radeonsi|Mesa core Assignee|dri-devel at lists.freedesktop |mesa-dev at lists.freedesktop. |.org|org --- Comment #10 from Nicolai Hähnle --- I can reproduce this now. It really seems like this should be fixed in Mesa main, though: there is already code that checks for this condition when it affects a single program stage (in _mesa_update_shader_textures_used) and when it affects a SSO pipeline (in _mesa_sampler_uniforms_pipeline_are_valid). This code needs to be hooked into the non-SSO case as well. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161006/8fe8d219/attachment.html>
[Bug 98028] Guns of Icarus Online segfaults on startup since AMDGPU: Partially fix control flow at -O0
https://bugs.freedesktop.org/show_bug.cgi?id=98028 --- Comment #9 from Daniel Scharrer --- Your patch from D25306 fixes the crash for me. Thanks for looking into this. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161006/3c34b509/attachment.html>
[PATCH v10 0/4] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are exected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. In this v10 debugfs creation failures don't abort CRTC registration, as suggested by Emil Velikov. Thanks, Tomeu Tomeu Vizoso (4): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API drm/i915: Put "cooked" vlank counters in frame CRC lines Documentation/gpu/drm-uapi.rst|6 + drivers/gpu/drm/Makefile |3 +- drivers/gpu/drm/drm_crtc.c| 34 +- drivers/gpu/drm/drm_debugfs.c | 34 +- drivers/gpu/drm/drm_debugfs_crc.c | 351 drivers/gpu/drm/drm_internal.h| 16 + drivers/gpu/drm/i915/Makefile |2 +- drivers/gpu/drm/i915/i915_debugfs.c | 886 +--- drivers/gpu/drm/i915/i915_drv.h |1 + drivers/gpu/drm/i915/i915_irq.c | 83 ++- drivers/gpu/drm/i915/intel_display.c |1 + drivers/gpu/drm/i915/intel_drv.h | 13 + drivers/gpu/drm/i915/intel_pipe_crc.c | 1013 + include/drm/drm_crtc.h| 41 ++ include/drm/drm_debugfs_crc.h | 73 +++ 15 files changed, 1645 insertions(+), 912 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c create mode 100644 include/drm/drm_debugfs_crc.h -- 2.7.4
[PATCH v10 2/4] drm: Add API for capturing frame CRCs
Adds files and directories to debugfs for controlling and reading frame
CRCs, per CRTC:
dri/0/crtc-0/crc
dri/0/crtc-0/crc/control
dri/0/crtc-0/crc/data
Drivers can implement the set_crc_source callback() in drm_crtc_funcs to
start and stop generating frame CRCs and can add entries to the output
by calling drm_crtc_add_crc_entry.
v2:
- Lots of good fixes suggested by Thierry.
- Added documentation.
- Changed the debugfs layout.
- Moved to allocate the entries circular queue once when frame
generation gets enabled for the first time.
v3:
- Use the control file just to select the source, and start and stop
capture when the data file is opened and closed, respectively.
- Make variable the number of CRC values per entry, per source.
- Allocate entries queue each time we start capturing as now there
isn't a fixed number of CRC values per entry.
- Store the frame counter in the data file as a 8-digit hex number.
- For sources that cannot provide useful frame numbers, place
in the frame field.
v4:
- Build only if CONFIG_DEBUG_FS is enabled.
- Use memdup_user_nul.
- Consolidate calculation of the size of an entry in a helper.
- Add 0x prefix to hex numbers in the data file.
- Remove unnecessary snprintf and strlen usage in read callback.
v5:
- Made the crcs array in drm_crtc_crc_entry fixed-size
- Lots of other smaller improvements suggested by Emil Velikov
v7:
- Move definition of drm_debugfs_crtc_crc_add to drm_internal.h
v8:
- Call debugfs_remove_recursive when we fail to create the minor
device
v9:
- Register the debugfs directory for a crtc from
drm_crtc_register_all()
v10:
- Don't let debugfs failures interrupt CRTC registration (Emil
Velikov)
Signed-off-by: Tomeu Vizoso
---
Documentation/gpu/drm-uapi.rst| 6 +
drivers/gpu/drm/Makefile | 3 +-
drivers/gpu/drm/drm_crtc.c| 34 +++-
drivers/gpu/drm/drm_debugfs.c | 34 +++-
drivers/gpu/drm/drm_debugfs_crc.c | 351 ++
drivers/gpu/drm/drm_internal.h| 16 ++
include/drm/drm_crtc.h| 41 +
include/drm/drm_debugfs_crc.h | 73
8 files changed, 555 insertions(+), 3 deletions(-)
create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c
create mode 100644 include/drm/drm_debugfs_crc.h
diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 1ba301cebe16..de3ac9f90f8f 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -216,3 +216,9 @@ interfaces. Especially since all hardware-acceleration
interfaces to
userspace are driver specific for efficiency and other reasons these
interfaces can be rather substantial. Hence every driver has its own
chapter.
+
+Testing and validation
+==
+
+.. kernel-doc:: drivers/gpu/drm/drm_debugfs_crc.c
+ :doc: CRC ABI
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 25c720454017..74579d2e796e 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -9,7 +9,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \
drm_scatter.o drm_pci.o \
drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
- drm_info.o drm_debugfs.o drm_encoder_slave.o \
+ drm_info.o drm_encoder_slave.o \
drm_trace_points.o drm_global.o drm_prime.o \
drm_rect.o drm_vma_manager.o drm_flip_work.o \
drm_modeset_lock.o drm_atomic.o drm_bridge.o \
@@ -23,6 +23,7 @@ drm-$(CONFIG_PCI) += ati_pcigart.o
drm-$(CONFIG_DRM_PANEL) += drm_panel.o
drm-$(CONFIG_OF) += drm_of.o
drm-$(CONFIG_AGP) += drm_agpsupport.o
+drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 2d7bedf28647..b9891a724437 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -40,7 +40,7 @@
#include
#include
#include
-#include
+#include
#include "drm_crtc_internal.h"
#include "drm_internal.h"
@@ -122,6 +122,10 @@ static int drm_crtc_register_all(struct drm_device *dev)
int ret = 0;
drm_for_each_crtc(crtc, dev) {
+ if (drm_debugfs_crtc_add(crtc)) {
+ DRM_ERROR("Failed to initialize debugfs entry for CRTC
'%s'.\n",
+ crtc->name);
+
if (crtc->funcs->late_register)
ret = crtc->funcs->late_register(crtc);
if (ret)
@@ -138,9 +142,29 @@ static void drm_crtc_unregister_all(struct drm_device *dev)
drm_for_each_crtc(crtc, dev) {
if (crtc->funcs->early_unregister)
[PATCH v10 4/4] drm/i915: Put "cooked" vlank counters in frame CRC lines
Use drm_accurate_vblank_count so we have the full 32 bit to represent
the frame counter and userspace has a simpler way of knowing when the
counter wraps around.
Signed-off-by: Tomeu Vizoso
Reviewed-by: Emil Velikov
---
drivers/gpu/drm/i915/i915_irq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1549cc4f88ec..238a353454e9 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1495,7 +1495,6 @@ static void display_pipe_crc_irq_handler(struct
drm_i915_private *dev_priv,
struct drm_driver *driver = dev_priv->drm.driver;
uint32_t crcs[5];
int head, tail, ret;
- u32 frame;
spin_lock(&pipe_crc->lock);
if (pipe_crc->source) {
@@ -1551,8 +1550,9 @@ static void display_pipe_crc_irq_handler(struct
drm_i915_private *dev_priv,
crcs[2] = crc2;
crcs[3] = crc3;
crcs[4] = crc4;
- frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
- ret = drm_crtc_add_crc_entry(crtc, true, frame, crcs);
+ ret = drm_crtc_add_crc_entry(crtc, true,
+drm_accurate_vblank_count(crtc),
+crcs);
spin_unlock(&crtc->crc.lock);
if (!ret)
wake_up_interruptible(&crtc->crc.wq);
--
2.7.4
[PATCH v10 1/4] drm/i915/debugfs: Move out pipe CRC code
In preparation to using a generic API in the DRM core for continuous CRC
generation, move the related code out of i915_debugfs.c into a new file.
Eventually, only the Intel-specific code will remain in this new file.
v2: Rebased.
v6: Rebased.
v7: Fix whitespace issue.
v9: Have intel_display_crc_init accept a drm_i915_private instead.
Signed-off-by: Tomeu Vizoso
Reviewed-by: Emil Velikov
---
drivers/gpu/drm/i915/Makefile | 2 +-
drivers/gpu/drm/i915/i915_debugfs.c | 886 +---
drivers/gpu/drm/i915/intel_drv.h | 5 +
drivers/gpu/drm/i915/intel_pipe_crc.c | 943 ++
4 files changed, 953 insertions(+), 883 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index a998c2bce70a..e6fe0040fe64 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -24,7 +24,7 @@ i915-y := i915_drv.o \
intel_runtime_pm.o
i915-$(CONFIG_COMPAT) += i915_ioc32.o
-i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
+i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
# GEM code
i915-y += i915_cmd_parser.o \
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index da7141382b00..4fb9d829884e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -26,19 +26,9 @@
*
*/
-#include
-#include
-#include
#include
-#include
-#include
#include
-#include
-#include
#include "intel_drv.h"
-#include "intel_ringbuffer.h"
-#include
-#include "i915_drv.h"
static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
{
@@ -3414,12 +3404,6 @@ static int i915_drrs_status(struct seq_file *m, void
*unused)
return 0;
}
-struct pipe_crc_info {
- const char *name;
- struct drm_i915_private *dev_priv;
- enum pipe pipe;
-};
-
static int i915_dp_mst_info(struct seq_file *m, void *unused)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -3449,848 +3433,6 @@ static int i915_dp_mst_info(struct seq_file *m, void
*unused)
return 0;
}
-static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
-{
- struct pipe_crc_info *info = inode->i_private;
- struct drm_i915_private *dev_priv = info->dev_priv;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
-
- if (info->pipe >= INTEL_INFO(dev_priv)->num_pipes)
- return -ENODEV;
-
- spin_lock_irq(&pipe_crc->lock);
-
- if (pipe_crc->opened) {
- spin_unlock_irq(&pipe_crc->lock);
- return -EBUSY; /* already open */
- }
-
- pipe_crc->opened = true;
- filep->private_data = inode->i_private;
-
- spin_unlock_irq(&pipe_crc->lock);
-
- return 0;
-}
-
-static int i915_pipe_crc_release(struct inode *inode, struct file *filep)
-{
- struct pipe_crc_info *info = inode->i_private;
- struct drm_i915_private *dev_priv = info->dev_priv;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
-
- spin_lock_irq(&pipe_crc->lock);
- pipe_crc->opened = false;
- spin_unlock_irq(&pipe_crc->lock);
-
- return 0;
-}
-
-/* (6 fields, 8 chars each, space separated (5) + '\n') */
-#define PIPE_CRC_LINE_LEN (6 * 8 + 5 + 1)
-/* account for \'0' */
-#define PIPE_CRC_BUFFER_LEN(PIPE_CRC_LINE_LEN + 1)
-
-static int pipe_crc_data_count(struct intel_pipe_crc *pipe_crc)
-{
- assert_spin_locked(&pipe_crc->lock);
- return CIRC_CNT(pipe_crc->head, pipe_crc->tail,
- INTEL_PIPE_CRC_ENTRIES_NR);
-}
-
-static ssize_t
-i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
- loff_t *pos)
-{
- struct pipe_crc_info *info = filep->private_data;
- struct drm_i915_private *dev_priv = info->dev_priv;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
- char buf[PIPE_CRC_BUFFER_LEN];
- int n_entries;
- ssize_t bytes_read;
-
- /*
-* Don't allow user space to provide buffers not big enough to hold
-* a line of data.
-*/
- if (count < PIPE_CRC_LINE_LEN)
- return -EINVAL;
-
- if (pipe_crc->source == INTEL_PIPE_CRC_SOURCE_NONE)
- return 0;
-
- /* nothing to read */
- spin_lock_irq(&pipe_crc->lock);
- while (pipe_crc_data_count(pipe_crc) == 0) {
- int ret;
-
- if (filep->f_flags & O_NONBLOCK) {
- spin_unlock_irq(&pipe_crc->lock);
- return -EAGAIN;
- }
-
- ret = wait_event_interruptible_lock_irq(pipe_crc->wq,
- pipe_crc_data_count(pipe_crc), pipe_crc->lock);
- if (ret) {
- spin_unlock_irq(&pipe_crc->lock);
-
[PATCH v10 3/4] drm/i915: Use new CRC debugfs API
The core provides now an ABI to userspace for generation of frame CRCs,
so implement the ->set_crc_source() callback and reuse as much code as
possible with the previous ABI implementation.
When handling the pageflip interrupt, we skip 1 or 2 frames depending on
the HW because they contain wrong values. For the legacy ABI for
generating frame CRCs, this was done in userspace but now that we have a
generic ABI it's better if it's not exposed by the kernel.
v2:
- Leave the legacy implementation in place as the ABI implementation
in the core is incompatible with it.
v3:
- Use the "cooked" vblank counter so we have a whole 32 bits.
- Make sure we don't mess with the state of the legacy CRC capture
ABI implementation.
v4:
- Keep use of get_vblank_counter as in the legacy code, will be
changed in a followup commit.
v5:
- Skip first frame or two as it's known that they contain wrong
data.
- A few fixes suggested by Emil Velikov.
v6:
- Rework programming of the HW registers to preserve previous
behavior.
v7:
- Address whitespace issue.
- Added a comment on why in the implementation of the new ABI we
skip the 1st or 2nd frames.
v9:
- Add stub for intel_crtc_set_crc_source.
Signed-off-by: Tomeu Vizoso
Reviewed-by: Emil Velikov
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 83 +--
drivers/gpu/drm/i915/intel_display.c | 1 +
drivers/gpu/drm/i915/intel_drv.h | 8 +++
drivers/gpu/drm/i915/intel_pipe_crc.c | 94 ++-
5 files changed, 149 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f8c66eea06bc..20522f0a4c57 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1713,6 +1713,7 @@ struct intel_pipe_crc {
enum intel_pipe_crc_source source;
int head, tail;
wait_queue_head_t wq;
+ int skipped;
};
struct i915_frontbuffer_tracking {
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index bd6c8b0eeaef..1549cc4f88ec 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1491,41 +1491,72 @@ static void display_pipe_crc_irq_handler(struct
drm_i915_private *dev_priv,
{
struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
struct intel_pipe_crc_entry *entry;
- int head, tail;
+ struct drm_crtc *crtc = intel_get_crtc_for_pipe(&dev_priv->drm, pipe);
+ struct drm_driver *driver = dev_priv->drm.driver;
+ uint32_t crcs[5];
+ int head, tail, ret;
+ u32 frame;
spin_lock(&pipe_crc->lock);
+ if (pipe_crc->source) {
+ if (!pipe_crc->entries) {
+ spin_unlock(&pipe_crc->lock);
+ DRM_DEBUG_KMS("spurious interrupt\n");
+ return;
+ }
- if (!pipe_crc->entries) {
- spin_unlock(&pipe_crc->lock);
- DRM_DEBUG_KMS("spurious interrupt\n");
- return;
- }
-
- head = pipe_crc->head;
- tail = pipe_crc->tail;
+ head = pipe_crc->head;
+ tail = pipe_crc->tail;
- if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
- spin_unlock(&pipe_crc->lock);
- DRM_ERROR("CRC buffer overflowing\n");
- return;
- }
+ if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
+ spin_unlock(&pipe_crc->lock);
+ DRM_ERROR("CRC buffer overflowing\n");
+ return;
+ }
- entry = &pipe_crc->entries[head];
+ entry = &pipe_crc->entries[head];
- entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm,
-pipe);
- entry->crc[0] = crc0;
- entry->crc[1] = crc1;
- entry->crc[2] = crc2;
- entry->crc[3] = crc3;
- entry->crc[4] = crc4;
+ entry->frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
+ entry->crc[0] = crc0;
+ entry->crc[1] = crc1;
+ entry->crc[2] = crc2;
+ entry->crc[3] = crc3;
+ entry->crc[4] = crc4;
- head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
- pipe_crc->head = head;
+ head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
+ pipe_crc->head = head;
- spin_unlock(&pipe_crc->lock);
+ spin_unlock(&pipe_crc->lock);
- wake_up_interruptible(&pipe_crc->wq);
+ wake_up_interruptible(&pipe_crc->wq);
+ } else {
+ /*
+* For some not yet identified reason, the first CRC is
+* bonkers. So let's just wait for the next vblank and rea
[PATCH v11 0/4] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are exected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. Sorry about that, but there was a dangling brace in v10 breaking the build so here is this v11. Thanks, Tomeu Tomeu Vizoso (4): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API drm/i915: Put "cooked" vlank counters in frame CRC lines Documentation/gpu/drm-uapi.rst|6 + drivers/gpu/drm/Makefile |3 +- drivers/gpu/drm/drm_crtc.c| 34 +- drivers/gpu/drm/drm_debugfs.c | 34 +- drivers/gpu/drm/drm_debugfs_crc.c | 351 drivers/gpu/drm/drm_internal.h| 16 + drivers/gpu/drm/i915/Makefile |2 +- drivers/gpu/drm/i915/i915_debugfs.c | 886 +--- drivers/gpu/drm/i915/i915_drv.h |1 + drivers/gpu/drm/i915/i915_irq.c | 83 ++- drivers/gpu/drm/i915/intel_display.c |1 + drivers/gpu/drm/i915/intel_drv.h | 13 + drivers/gpu/drm/i915/intel_pipe_crc.c | 1013 + include/drm/drm_crtc.h| 41 ++ include/drm/drm_debugfs_crc.h | 73 +++ 15 files changed, 1645 insertions(+), 912 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c create mode 100644 include/drm/drm_debugfs_crc.h -- 2.7.4
[PATCH v11 1/4] drm/i915/debugfs: Move out pipe CRC code
In preparation to using a generic API in the DRM core for continuous CRC
generation, move the related code out of i915_debugfs.c into a new file.
Eventually, only the Intel-specific code will remain in this new file.
v2: Rebased.
v6: Rebased.
v7: Fix whitespace issue.
v9: Have intel_display_crc_init accept a drm_i915_private instead.
Signed-off-by: Tomeu Vizoso
Reviewed-by: Emil Velikov
---
drivers/gpu/drm/i915/Makefile | 2 +-
drivers/gpu/drm/i915/i915_debugfs.c | 886 +---
drivers/gpu/drm/i915/intel_drv.h | 5 +
drivers/gpu/drm/i915/intel_pipe_crc.c | 943 ++
4 files changed, 953 insertions(+), 883 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index a998c2bce70a..e6fe0040fe64 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -24,7 +24,7 @@ i915-y := i915_drv.o \
intel_runtime_pm.o
i915-$(CONFIG_COMPAT) += i915_ioc32.o
-i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
+i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
# GEM code
i915-y += i915_cmd_parser.o \
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index da7141382b00..4fb9d829884e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -26,19 +26,9 @@
*
*/
-#include
-#include
-#include
#include
-#include
-#include
#include
-#include
-#include
#include "intel_drv.h"
-#include "intel_ringbuffer.h"
-#include
-#include "i915_drv.h"
static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
{
@@ -3414,12 +3404,6 @@ static int i915_drrs_status(struct seq_file *m, void
*unused)
return 0;
}
-struct pipe_crc_info {
- const char *name;
- struct drm_i915_private *dev_priv;
- enum pipe pipe;
-};
-
static int i915_dp_mst_info(struct seq_file *m, void *unused)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -3449,848 +3433,6 @@ static int i915_dp_mst_info(struct seq_file *m, void
*unused)
return 0;
}
-static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
-{
- struct pipe_crc_info *info = inode->i_private;
- struct drm_i915_private *dev_priv = info->dev_priv;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
-
- if (info->pipe >= INTEL_INFO(dev_priv)->num_pipes)
- return -ENODEV;
-
- spin_lock_irq(&pipe_crc->lock);
-
- if (pipe_crc->opened) {
- spin_unlock_irq(&pipe_crc->lock);
- return -EBUSY; /* already open */
- }
-
- pipe_crc->opened = true;
- filep->private_data = inode->i_private;
-
- spin_unlock_irq(&pipe_crc->lock);
-
- return 0;
-}
-
-static int i915_pipe_crc_release(struct inode *inode, struct file *filep)
-{
- struct pipe_crc_info *info = inode->i_private;
- struct drm_i915_private *dev_priv = info->dev_priv;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
-
- spin_lock_irq(&pipe_crc->lock);
- pipe_crc->opened = false;
- spin_unlock_irq(&pipe_crc->lock);
-
- return 0;
-}
-
-/* (6 fields, 8 chars each, space separated (5) + '\n') */
-#define PIPE_CRC_LINE_LEN (6 * 8 + 5 + 1)
-/* account for \'0' */
-#define PIPE_CRC_BUFFER_LEN(PIPE_CRC_LINE_LEN + 1)
-
-static int pipe_crc_data_count(struct intel_pipe_crc *pipe_crc)
-{
- assert_spin_locked(&pipe_crc->lock);
- return CIRC_CNT(pipe_crc->head, pipe_crc->tail,
- INTEL_PIPE_CRC_ENTRIES_NR);
-}
-
-static ssize_t
-i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
- loff_t *pos)
-{
- struct pipe_crc_info *info = filep->private_data;
- struct drm_i915_private *dev_priv = info->dev_priv;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
- char buf[PIPE_CRC_BUFFER_LEN];
- int n_entries;
- ssize_t bytes_read;
-
- /*
-* Don't allow user space to provide buffers not big enough to hold
-* a line of data.
-*/
- if (count < PIPE_CRC_LINE_LEN)
- return -EINVAL;
-
- if (pipe_crc->source == INTEL_PIPE_CRC_SOURCE_NONE)
- return 0;
-
- /* nothing to read */
- spin_lock_irq(&pipe_crc->lock);
- while (pipe_crc_data_count(pipe_crc) == 0) {
- int ret;
-
- if (filep->f_flags & O_NONBLOCK) {
- spin_unlock_irq(&pipe_crc->lock);
- return -EAGAIN;
- }
-
- ret = wait_event_interruptible_lock_irq(pipe_crc->wq,
- pipe_crc_data_count(pipe_crc), pipe_crc->lock);
- if (ret) {
- spin_unlock_irq(&pipe_crc->lock);
-
[PATCH v11 2/4] drm: Add API for capturing frame CRCs
Adds files and directories to debugfs for controlling and reading frame
CRCs, per CRTC:
dri/0/crtc-0/crc
dri/0/crtc-0/crc/control
dri/0/crtc-0/crc/data
Drivers can implement the set_crc_source callback() in drm_crtc_funcs to
start and stop generating frame CRCs and can add entries to the output
by calling drm_crtc_add_crc_entry.
v2:
- Lots of good fixes suggested by Thierry.
- Added documentation.
- Changed the debugfs layout.
- Moved to allocate the entries circular queue once when frame
generation gets enabled for the first time.
v3:
- Use the control file just to select the source, and start and stop
capture when the data file is opened and closed, respectively.
- Make variable the number of CRC values per entry, per source.
- Allocate entries queue each time we start capturing as now there
isn't a fixed number of CRC values per entry.
- Store the frame counter in the data file as a 8-digit hex number.
- For sources that cannot provide useful frame numbers, place
in the frame field.
v4:
- Build only if CONFIG_DEBUG_FS is enabled.
- Use memdup_user_nul.
- Consolidate calculation of the size of an entry in a helper.
- Add 0x prefix to hex numbers in the data file.
- Remove unnecessary snprintf and strlen usage in read callback.
v5:
- Made the crcs array in drm_crtc_crc_entry fixed-size
- Lots of other smaller improvements suggested by Emil Velikov
v7:
- Move definition of drm_debugfs_crtc_crc_add to drm_internal.h
v8:
- Call debugfs_remove_recursive when we fail to create the minor
device
v9:
- Register the debugfs directory for a crtc from
drm_crtc_register_all()
v10:
- Don't let debugfs failures interrupt CRTC registration (Emil
Velikov)
v11:
- Remove extra brace that broke compilation. Sorry!
Signed-off-by: Tomeu Vizoso
---
Documentation/gpu/drm-uapi.rst| 6 +
drivers/gpu/drm/Makefile | 3 +-
drivers/gpu/drm/drm_crtc.c| 34 +++-
drivers/gpu/drm/drm_debugfs.c | 34 +++-
drivers/gpu/drm/drm_debugfs_crc.c | 351 ++
drivers/gpu/drm/drm_internal.h| 16 ++
include/drm/drm_crtc.h| 41 +
include/drm/drm_debugfs_crc.h | 73
8 files changed, 555 insertions(+), 3 deletions(-)
create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c
create mode 100644 include/drm/drm_debugfs_crc.h
diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 1ba301cebe16..de3ac9f90f8f 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -216,3 +216,9 @@ interfaces. Especially since all hardware-acceleration
interfaces to
userspace are driver specific for efficiency and other reasons these
interfaces can be rather substantial. Hence every driver has its own
chapter.
+
+Testing and validation
+==
+
+.. kernel-doc:: drivers/gpu/drm/drm_debugfs_crc.c
+ :doc: CRC ABI
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 25c720454017..74579d2e796e 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -9,7 +9,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \
drm_scatter.o drm_pci.o \
drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
- drm_info.o drm_debugfs.o drm_encoder_slave.o \
+ drm_info.o drm_encoder_slave.o \
drm_trace_points.o drm_global.o drm_prime.o \
drm_rect.o drm_vma_manager.o drm_flip_work.o \
drm_modeset_lock.o drm_atomic.o drm_bridge.o \
@@ -23,6 +23,7 @@ drm-$(CONFIG_PCI) += ati_pcigart.o
drm-$(CONFIG_DRM_PANEL) += drm_panel.o
drm-$(CONFIG_OF) += drm_of.o
drm-$(CONFIG_AGP) += drm_agpsupport.o
+drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 2d7bedf28647..60403bf7a4ff 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -40,7 +40,7 @@
#include
#include
#include
-#include
+#include
#include "drm_crtc_internal.h"
#include "drm_internal.h"
@@ -122,6 +122,10 @@ static int drm_crtc_register_all(struct drm_device *dev)
int ret = 0;
drm_for_each_crtc(crtc, dev) {
+ if (drm_debugfs_crtc_add(crtc))
+ DRM_ERROR("Failed to initialize debugfs entry for CRTC
'%s'.\n",
+ crtc->name);
+
if (crtc->funcs->late_register)
ret = crtc->funcs->late_register(crtc);
if (ret)
@@ -138,9 +142,29 @@ static void drm_crtc_unregister_all(struct drm_device *dev)
drm_for_each_crtc(crtc, dev) {
[PATCH v11 3/4] drm/i915: Use new CRC debugfs API
The core provides now an ABI to userspace for generation of frame CRCs,
so implement the ->set_crc_source() callback and reuse as much code as
possible with the previous ABI implementation.
When handling the pageflip interrupt, we skip 1 or 2 frames depending on
the HW because they contain wrong values. For the legacy ABI for
generating frame CRCs, this was done in userspace but now that we have a
generic ABI it's better if it's not exposed by the kernel.
v2:
- Leave the legacy implementation in place as the ABI implementation
in the core is incompatible with it.
v3:
- Use the "cooked" vblank counter so we have a whole 32 bits.
- Make sure we don't mess with the state of the legacy CRC capture
ABI implementation.
v4:
- Keep use of get_vblank_counter as in the legacy code, will be
changed in a followup commit.
v5:
- Skip first frame or two as it's known that they contain wrong
data.
- A few fixes suggested by Emil Velikov.
v6:
- Rework programming of the HW registers to preserve previous
behavior.
v7:
- Address whitespace issue.
- Added a comment on why in the implementation of the new ABI we
skip the 1st or 2nd frames.
v9:
- Add stub for intel_crtc_set_crc_source.
Signed-off-by: Tomeu Vizoso
Reviewed-by: Emil Velikov
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 83 +--
drivers/gpu/drm/i915/intel_display.c | 1 +
drivers/gpu/drm/i915/intel_drv.h | 8 +++
drivers/gpu/drm/i915/intel_pipe_crc.c | 94 ++-
5 files changed, 149 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f8c66eea06bc..20522f0a4c57 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1713,6 +1713,7 @@ struct intel_pipe_crc {
enum intel_pipe_crc_source source;
int head, tail;
wait_queue_head_t wq;
+ int skipped;
};
struct i915_frontbuffer_tracking {
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index bd6c8b0eeaef..1549cc4f88ec 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1491,41 +1491,72 @@ static void display_pipe_crc_irq_handler(struct
drm_i915_private *dev_priv,
{
struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
struct intel_pipe_crc_entry *entry;
- int head, tail;
+ struct drm_crtc *crtc = intel_get_crtc_for_pipe(&dev_priv->drm, pipe);
+ struct drm_driver *driver = dev_priv->drm.driver;
+ uint32_t crcs[5];
+ int head, tail, ret;
+ u32 frame;
spin_lock(&pipe_crc->lock);
+ if (pipe_crc->source) {
+ if (!pipe_crc->entries) {
+ spin_unlock(&pipe_crc->lock);
+ DRM_DEBUG_KMS("spurious interrupt\n");
+ return;
+ }
- if (!pipe_crc->entries) {
- spin_unlock(&pipe_crc->lock);
- DRM_DEBUG_KMS("spurious interrupt\n");
- return;
- }
-
- head = pipe_crc->head;
- tail = pipe_crc->tail;
+ head = pipe_crc->head;
+ tail = pipe_crc->tail;
- if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
- spin_unlock(&pipe_crc->lock);
- DRM_ERROR("CRC buffer overflowing\n");
- return;
- }
+ if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
+ spin_unlock(&pipe_crc->lock);
+ DRM_ERROR("CRC buffer overflowing\n");
+ return;
+ }
- entry = &pipe_crc->entries[head];
+ entry = &pipe_crc->entries[head];
- entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm,
-pipe);
- entry->crc[0] = crc0;
- entry->crc[1] = crc1;
- entry->crc[2] = crc2;
- entry->crc[3] = crc3;
- entry->crc[4] = crc4;
+ entry->frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
+ entry->crc[0] = crc0;
+ entry->crc[1] = crc1;
+ entry->crc[2] = crc2;
+ entry->crc[3] = crc3;
+ entry->crc[4] = crc4;
- head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
- pipe_crc->head = head;
+ head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
+ pipe_crc->head = head;
- spin_unlock(&pipe_crc->lock);
+ spin_unlock(&pipe_crc->lock);
- wake_up_interruptible(&pipe_crc->wq);
+ wake_up_interruptible(&pipe_crc->wq);
+ } else {
+ /*
+* For some not yet identified reason, the first CRC is
+* bonkers. So let's just wait for the next vblank and rea
[PATCH v11 4/4] drm/i915: Put "cooked" vlank counters in frame CRC lines
Use drm_accurate_vblank_count so we have the full 32 bit to represent
the frame counter and userspace has a simpler way of knowing when the
counter wraps around.
Signed-off-by: Tomeu Vizoso
Reviewed-by: Emil Velikov
---
drivers/gpu/drm/i915/i915_irq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1549cc4f88ec..238a353454e9 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1495,7 +1495,6 @@ static void display_pipe_crc_irq_handler(struct
drm_i915_private *dev_priv,
struct drm_driver *driver = dev_priv->drm.driver;
uint32_t crcs[5];
int head, tail, ret;
- u32 frame;
spin_lock(&pipe_crc->lock);
if (pipe_crc->source) {
@@ -1551,8 +1550,9 @@ static void display_pipe_crc_irq_handler(struct
drm_i915_private *dev_priv,
crcs[2] = crc2;
crcs[3] = crc3;
crcs[4] = crc4;
- frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
- ret = drm_crtc_add_crc_entry(crtc, true, frame, crcs);
+ ret = drm_crtc_add_crc_entry(crtc, true,
+drm_accurate_vblank_count(crtc),
+crcs);
spin_unlock(&crtc->crc.lock);
if (!ret)
wake_up_interruptible(&crtc->crc.wq);
--
2.7.4
[PATCH v10 0/3] Secure Memory Allocation Framework
so there is discussion about a "central userspace allocator" (ie. more
like a common userspace API that could be implemented on top of
various devices/APIs) to decide in a generic way which device could
allocate.
https://github.com/cubanismo/allocator
and I wrote up some rough thoughts/proposal about how the usage might
look.. just rough, so don't try to compile it or anything, and not
consensus yet so it will probably change/evolve..
https://github.com/robclark/allocator/blob/master/USAGE.md
I think ion could be just another device to share buffers with, which
happens to not impose any specific constraints. How "liballoc-ion.so"
backend figures out how to map constraints/usage to a heap is a bit
hand-wavey at the moment.
BR,
-R
On Wed, Oct 5, 2016 at 9:40 AM, Benjamin Gaignard
wrote:
> because with ion it is up to userland to decide which heap to use
> and until now userland doesn't have any way to get device constraints...
>
> I will prefer let a central allocator (in kernel) decide from the
> attached devices
> which allocator is the best. It is what I have implemented in smaf.
>
> Benjamin
>
>
> 2016-10-05 15:19 GMT+02:00 Daniel Vetter :
>> On Tue, Oct 04, 2016 at 01:47:21PM +0200, Benjamin Gaignard wrote:
>>> version 10 changes:
>>> - rebased on kernel 4.8 tag
>>> - minor typo fix
>>>
>>> version 9 changes:
>>> - rebased on 4.8-rc5
>>> - struct dma_attrs doesn't exist anymore so update CMA allocator
>>>to compile with new dma_*_attr functions
>>> - add example SMAF use case in cover letter
>>>
>>> version 8 changes:
>>> - rework of the structures used within ioctl
>>>by adding a version field and padding to be futur proof
>>> - rename fake secure moduel to test secure module
>>> - fix the various remarks done on the previous patcheset
>>>
>>> version 7 changes:
>>> - rebased on kernel 4.6-rc7
>>> - simplify secure module API
>>> - add vma ops to be able to detect mmap/munmap calls
>>> - add ioctl to get number and allocator names
>>> - update libsmaf with adding tests
>>>https://git.linaro.org/people/benjamin.gaignard/libsmaf.git
>>> - add debug log in fake secure module
>>>
>>> version 6 changes:
>>> - rebased on kernel 4.5-rc4
>>> - fix mmapping bug while requested allocation size isn't a a multiple of
>>>PAGE_SIZE (add a test for this in libsmaf)
>>>
>>> version 5 changes:
>>> - rebased on kernel 4.3-rc6
>>> - rework locking schema and make handle status use an atomic_t
>>> - add a fake secure module to allow performing tests without trusted
>>>environment
>>>
>>> version 4 changes:
>>> - rebased on kernel 4.3-rc3
>>> - fix missing EXPORT_SYMBOL for smaf_create_handle()
>>>
>>> version 3 changes:
>>> - Remove ioctl for allocator selection instead provide the name of
>>>the targeted allocator with allocation request.
>>>Selecting allocator from userland isn't the prefered way of working
>>>but is needed when the first user of the buffer is a software component.
>>> - Fix issues in case of error while creating smaf handle.
>>> - Fix module license.
>>> - Update libsmaf and tests to care of the SMAF API evolution
>>>https://git.linaro.org/people/benjamin.gaignard/libsmaf.git
>>>
>>> version 2 changes:
>>> - Add one ioctl to allow allocator selection from userspace.
>>>This is required for the uses case where the first user of
>>>the buffer is a software IP which can't perform dma_buf attachement.
>>> - Add name and ranking to allocator structure to be able to sort them.
>>> - Create a tiny library to test SMAF:
>>>https://git.linaro.org/people/benjamin.gaignard/libsmaf.git
>>> - Fix one issue when try to secure buffer without secure module registered
>>>
>>> SMAF aim to solve two problems: allocating memory that fit with hardware IPs
>>> constraints and secure those data from bus point of view.
>>>
>>> One example of SMAF usage is camera preview: on SoC you may use either an
>>> USB
>>> webcam or the built-in camera interface and the frames could be send
>>> directly
>>> to the dipslay Ip or handle by GPU.
>>> Most of USB interfaces and GPU have mmu but almost all built-in camera
>>> interace and display Ips don't have mmu so when selecting how allocate
>>> buffer you need to be aware of each devices constraints (contiguous memroy,
>>> stride, boundary, alignment ...).
>>> ION has solve this problem by let userland decide which allocator (heap) to
>>> use
>>> but this require to adapt userland for each platform and sometime for each
>>> use case.
>>>
>>> To be sure to select the best allocation method for devices SMAF implement
>>> deferred allocation mechanism: memory allocation is only done when the first
>>> device effectively required it.
>>> Allocator modules have to implement a match() to let SMAF know if they are
>>> compatibles with devices needs.
>>> This patch set provide an example of allocator module which use
>>> dma_{alloc/free/mmap}_attrs() and check if at least one device have
>>> coherent_dma_m
[pull] radeon and amdgpu drm-next-4.9
Hi Dave, Just some misc bug fixes for 4.9. The following changes since commit c2cbc38b9715bd8318062e600668fc30e5a3fbfa: drm: virtio: reinstate drm_virtio_set_busid() (2016-10-04 13:10:30 +1000) are available in the git repository at: git://people.freedesktop.org/~agd5f/linux drm-next-4.9 for you to fetch changes up to 8036617e92e3fad49eef9bbe868b661c58249aff: drm/amdgpu: revert "use more than 64KB fragment size if possible" (2016-10-06 12:39:04 -0400) Alex Deucher (3): drm/amdgpu: remove DRM_AMD_POWERPLAY drm/amdgpu/vce: add support for hw config packet (v2) drm/amdgpu/virtual_dce: adjust config ifdef Christian König (1): drm/amdgpu: revert "use more than 64KB fragment size if possible" Grazvydas Ignotas (3): drm/amdgpu: also track late init state drm/amdgpu/dce11: add missing drm_mode_config_cleanup call drm/amdgpu: warn if dp aux is still attached on free Huang Rui (1): drm/amdgpu: clean up to set fw_offset as 0 twice Mario Kleiner (2): drm/radeon: Slightly more robust flip completion handling for < DCE-4 drm/radeon: Prevent races on pre DCE4 between flip submission and completion. drivers/gpu/drm/amd/amdgpu/Kconfig | 1 - drivers/gpu/drm/amd/amdgpu/Makefile| 4 --- drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 4 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 2 -- drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_powerplay.c | 13 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c| 14 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 12 +++ drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 1 + drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 4 +-- drivers/gpu/drm/amd/powerplay/Kconfig | 6 drivers/gpu/drm/radeon/atombios_crtc.c | 4 +-- drivers/gpu/drm/radeon/radeon_display.c| 47 ++ drivers/gpu/drm/radeon/rv515.c | 3 +- 17 files changed, 67 insertions(+), 55 deletions(-) delete mode 100644 drivers/gpu/drm/amd/powerplay/Kconfig
[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support
Hello, On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote: > On 10/06/2016 12:51 PM, Maxime Ripard wrote: > > On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote: > >> On 09/30/2016 08:07 PM, Maxime Ripard wrote: > >>> Some boards have an entirely passive RGB to VGA bridge, based on either > >>> DACs or resistor ladders. > >>> > >>> Those might or might not have an i2c bus routed to the VGA connector in > >>> order to access the screen EDIDs. > >>> > >>> Add a bridge that doesn't do anything but expose the modes available on > >>> the screen, either based on the EDIDs if available, or based on the XGA > >>> standards. > >>> > >>> Acked-by: Rob Herring > >>> Signed-off-by: Maxime Ripard > >>> --- > >>> .../bindings/display/bridge/rgb-to-vga-bridge.txt | 48 + > >>> drivers/gpu/drm/bridge/Kconfig | 7 + > >>> drivers/gpu/drm/bridge/Makefile| 1 + > >>> drivers/gpu/drm/bridge/rgb-to-vga.c| 229 +++ > >>> 4 files changed, 285 insertions(+) > >>> create mode 100644 > >>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt > >>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c > >>> > >>> diff --git > >>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx > >>> t > >>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx > >>> t new file mode 100644 > >>> index ..a8375bc1f9cb > >>> --- /dev/null > >>> +++ > >>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx > >>> t @@ -0,0 +1,48 @@ > >>> +Dumb RGB to VGA bridge > >>> +-- > >>> + > >>> +This binding is aimed for dumb RGB to VGA bridges that do not require > >>> +any configuration. > >>> + > >>> +Required properties: > >>> + > >>> +- compatible: Must be "rgb-to-vga-bridge" > >> > >> I'd talked to Laurent on IRC if he's okay with this. And I guess you to > >> had discussed it during XDC too. He's suggested that it'd be better to > >> have the compatible string as "simple-vga-dac". > > > > I just wished this bikeshedding had taken place publicly and be > > actually part of that discussion, but yeah, ok. > > Sorry about that. I'd pinged him for an Ack, the discussion went > more than that :) > > >> Some of the reasons behind having this: > >> > >> - We don't need to specify "rgb" in the compatible string since most > >> simple VGA DACs can only work with an RGB input. > > > > Ok. > > > >> - Also, with "dac" specified in the string, we don't need to > >> specifically mention "bridge" in the string. Also, bridge is a drm > >> specific term. > >> > >> - "simple" is considered because it's an unconfigurable bridge, and it > >> might be misleading for other VGA DACs to not use "vga-dac". > > > > All those "simple" bindings are just the biggest lie we ever > > told. It's simple when you introduce it, and then grows into something > > much more complicated than a non-simple implementation. > > "simple" here is supposed to mean that it's an unconfigurable RGB to > VGA DAC. This isn't supposed to follow the simple-panel model, where > you add the "simple-panel" string in the compatible node, along with > you chip specific compatible string. I agree with Maxime, I don't like the word "simple". My preference would be "vga-dac" for a lack of a better qualifier than "simple" to describe the fact that the device requires no configuration. My only concern with "vga-dac" is that we would restrict usage of that compatible string for a subset of VGA DACs, with more complex devices not being compatible with "vga-dac" even though they are VGA DACs. That's a problem I can live with though. > In other words, this driver shouldn't be touched again in the future :) > If someone wants to write a RGB to VGA driver which is even > slightly configurable, they'll need to write a new bridge driver. I'm sure that won't be true. I can certainly foresee the addition of regulators support for instance. It's unfortunately never black and white. > >> What do you think about this? If you think it's good, would it be > >> possible for you to change this? I guess it's okay for the rest of > >> the patch to stay the same. > > > > I'll update and respin the serie. -- Regards, Laurent Pinchart
[PATCH] drm/amd/amdgpu: default to zero number of states if not enabled
From: Colin Ian King Currently, if adev->pp_enabled is false then the pp_stats_info data is not read and hence a garbage number of states from the stack is used to dump out the number of states. Given data.nums could be any random value, this could easily lead to read outside the data.states array. Fix this by setting data.nums to zero if adev->pp_enabled is false. Signed-off-by: Colin Ian King --- drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index accc908..808d788 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -195,6 +195,8 @@ static ssize_t amdgpu_get_pp_num_states(struct device *dev, if (adev->pp_enabled) amdgpu_dpm_get_pp_num_states(adev, &data); + else + data.nums = 0; buf_len = snprintf(buf, PAGE_SIZE, "states: %d\n", data.nums); for (i = 0; i < data.nums; i++) -- 2.9.3
[PATCH] drm/amd/amdgpu: default to zero number of states if not enabled
On Thu, Oct 6, 2016 at 2:02 PM, Colin King wrote: > From: Colin Ian King > > Currently, if adev->pp_enabled is false then the pp_stats_info data > is not read and hence a garbage number of states from the stack > is used to dump out the number of states. Given data.nums could be > any random value, this could easily lead to read outside the > data.states array. Fix this by setting data.nums to zero if > adev->pp_enabled is false. Are you actually seeing a problem? The pp_num_states attribute only gets added in the first place if pp_enabled is true. Alex > > Signed-off-by: Colin Ian King > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > index accc908..808d788 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > @@ -195,6 +195,8 @@ static ssize_t amdgpu_get_pp_num_states(struct device > *dev, > > if (adev->pp_enabled) > amdgpu_dpm_get_pp_num_states(adev, &data); > + else > + data.nums = 0; > > buf_len = snprintf(buf, PAGE_SIZE, "states: %d\n", data.nums); > for (i = 0; i < data.nums; i++) > -- > 2.9.3 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/amdgpu: default to zero number of states if not enabled
On 06/10/16 19:32, Alex Deucher wrote: > On Thu, Oct 6, 2016 at 2:02 PM, Colin King > wrote: >> From: Colin Ian King >> >> Currently, if adev->pp_enabled is false then the pp_stats_info data >> is not read and hence a garbage number of states from the stack >> is used to dump out the number of states. Given data.nums could be >> any random value, this could easily lead to read outside the >> data.states array. Fix this by setting data.nums to zero if >> adev->pp_enabled is false. > > Are you actually seeing a problem? Nope. > The pp_num_states attribute only > gets added in the first place if pp_enabled is true. Does that mean that the check on adev->pp_enabled is redundant then? > > Alex > >> >> Signed-off-by: Colin Ian King >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c >> index accc908..808d788 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c >> @@ -195,6 +195,8 @@ static ssize_t amdgpu_get_pp_num_states(struct device >> *dev, >> >> if (adev->pp_enabled) >> amdgpu_dpm_get_pp_num_states(adev, &data); >> + else >> + data.nums = 0; >> >> buf_len = snprintf(buf, PAGE_SIZE, "states: %d\n", data.nums); >> for (i = 0; i < data.nums; i++) >> -- >> 2.9.3 >> >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/amdgpu: default to zero number of states if not enabled
On 06/10/16 20:04, Deucher, Alexander wrote: >> -Original Message- >> From: Colin Ian King [mailto:colin.king at canonical.com] >> Sent: Thursday, October 06, 2016 3:04 PM >> To: Alex Deucher >> Cc: Deucher, Alexander; Koenig, Christian; David Airlie; Huang, JinHuiEric; >> Zhu, Rex; Zhou, Jammy; StDenis, Tom; Dan Carpenter; Maling list - DRI >> developers; LKML >> Subject: Re: [PATCH] drm/amd/amdgpu: default to zero number of states if >> not enabled >> >> On 06/10/16 19:32, Alex Deucher wrote: >>> On Thu, Oct 6, 2016 at 2:02 PM, Colin King >> wrote: From: Colin Ian King Currently, if adev->pp_enabled is false then the pp_stats_info data is not read and hence a garbage number of states from the stack is used to dump out the number of states. Given data.nums could be any random value, this could easily lead to read outside the data.states array. Fix this by setting data.nums to zero if adev->pp_enabled is false. >>> >>> Are you actually seeing a problem? >> >> Nope. >> >>> The pp_num_states attribute only >>> gets added in the first place if pp_enabled is true. >> >> Does that mean that the check on adev->pp_enabled is redundant then? > > Yes, I think so. OK, in which case it's probably extraneous code that could be removed. > > Alex > >> >>> >>> Alex >> >>> Signed-off-by: Colin Ian King --- drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index accc908..808d788 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -195,6 +195,8 @@ static ssize_t amdgpu_get_pp_num_states(struct >> device *dev, if (adev->pp_enabled) amdgpu_dpm_get_pp_num_states(adev, &data); + else + data.nums = 0; buf_len = snprintf(buf, PAGE_SIZE, "states: %d\n", data.nums); for (i = 0; i < data.nums; i++) -- 2.9.3 ___ dri-devel mailing list dri-devel at lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel >
[PATCH] drm/amd/amdgpu: default to zero number of states if not enabled
> -Original Message- > From: Colin Ian King [mailto:colin.king at canonical.com] > Sent: Thursday, October 06, 2016 3:04 PM > To: Alex Deucher > Cc: Deucher, Alexander; Koenig, Christian; David Airlie; Huang, JinHuiEric; > Zhu, Rex; Zhou, Jammy; StDenis, Tom; Dan Carpenter; Maling list - DRI > developers; LKML > Subject: Re: [PATCH] drm/amd/amdgpu: default to zero number of states if > not enabled > > On 06/10/16 19:32, Alex Deucher wrote: > > On Thu, Oct 6, 2016 at 2:02 PM, Colin King > wrote: > >> From: Colin Ian King > >> > >> Currently, if adev->pp_enabled is false then the pp_stats_info data > >> is not read and hence a garbage number of states from the stack > >> is used to dump out the number of states. Given data.nums could be > >> any random value, this could easily lead to read outside the > >> data.states array. Fix this by setting data.nums to zero if > >> adev->pp_enabled is false. > > > > Are you actually seeing a problem? > > Nope. > > > The pp_num_states attribute only > > gets added in the first place if pp_enabled is true. > > Does that mean that the check on adev->pp_enabled is redundant then? Yes, I think so. Alex > > > > > Alex > > > > >> > >> Signed-off-by: Colin Ian King > >> --- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > >> index accc908..808d788 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > >> @@ -195,6 +195,8 @@ static ssize_t amdgpu_get_pp_num_states(struct > device *dev, > >> > >> if (adev->pp_enabled) > >> amdgpu_dpm_get_pp_num_states(adev, &data); > >> + else > >> + data.nums = 0; > >> > >> buf_len = snprintf(buf, PAGE_SIZE, "states: %d\n", data.nums); > >> for (i = 0; i < data.nums; i++) > >> -- > >> 2.9.3 > >> > >> ___ > >> dri-devel mailing list > >> dri-devel at lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 1/5] drm/sun4i: rgb: Remove the bridge enable/disable functions
On Thu, Oct 6, 2016 at 3:57 AM, Maxime Ripard wrote: > The atomic helpers already call the drm_bridge_enable on our behalf, > there's no need to do it a second time. > > Reported-by: Sean Paul Reviewed-by: Sean Paul > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/sun4i/sun4i_rgb.c | 6 -- > 1 file changed, 0 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c > b/drivers/gpu/drm/sun4i/sun4i_rgb.c > index 4e4bea6f395c..d198ad7e5323 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c > +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c > @@ -155,9 +155,6 @@ static void sun4i_rgb_encoder_enable(struct drm_encoder > *encoder) > if (!IS_ERR(tcon->panel)) > drm_panel_prepare(tcon->panel); > > - /* encoder->bridge can be NULL; drm_bridge_enable checks for it */ > - drm_bridge_enable(encoder->bridge); > - > sun4i_tcon_channel_enable(tcon, 0); > > if (!IS_ERR(tcon->panel)) > @@ -177,9 +174,6 @@ static void sun4i_rgb_encoder_disable(struct drm_encoder > *encoder) > > sun4i_tcon_channel_disable(tcon, 0); > > - /* encoder->bridge can be NULL; drm_bridge_disable checks for it */ > - drm_bridge_disable(encoder->bridge); > - > if (!IS_ERR(tcon->panel)) > drm_panel_unprepare(tcon->panel); > } > -- > git-series 0.8.10 > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support
On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart wrote: > Hello, > > On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote: >> On 10/06/2016 12:51 PM, Maxime Ripard wrote: >> > On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote: >> >> On 09/30/2016 08:07 PM, Maxime Ripard wrote: >> >>> Some boards have an entirely passive RGB to VGA bridge, based on either >> >>> DACs or resistor ladders. >> >>> >> >>> Those might or might not have an i2c bus routed to the VGA connector in >> >>> order to access the screen EDIDs. >> >>> >> >>> Add a bridge that doesn't do anything but expose the modes available on >> >>> the screen, either based on the EDIDs if available, or based on the XGA >> >>> standards. >> >>> >> >>> Acked-by: Rob Herring >> >>> Signed-off-by: Maxime Ripard >> >>> --- >> >>> .../bindings/display/bridge/rgb-to-vga-bridge.txt | 48 + >> >>> drivers/gpu/drm/bridge/Kconfig | 7 + >> >>> drivers/gpu/drm/bridge/Makefile| 1 + >> >>> drivers/gpu/drm/bridge/rgb-to-vga.c| 229 +++ >> >>> 4 files changed, 285 insertions(+) >> >>> create mode 100644 >> >>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt >> >>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c >> >>> >> >>> diff --git >> >>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx >> >>> t >> >>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx >> >>> t new file mode 100644 >> >>> index ..a8375bc1f9cb >> >>> --- /dev/null >> >>> +++ >> >>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx >> >>> t @@ -0,0 +1,48 @@ >> >>> +Dumb RGB to VGA bridge >> >>> +-- >> >>> + >> >>> +This binding is aimed for dumb RGB to VGA bridges that do not require >> >>> +any configuration. >> >>> + >> >>> +Required properties: >> >>> + >> >>> +- compatible: Must be "rgb-to-vga-bridge" >> >> >> >> I'd talked to Laurent on IRC if he's okay with this. And I guess you to >> >> had discussed it during XDC too. He's suggested that it'd be better to >> >> have the compatible string as "simple-vga-dac". >> > >> > I just wished this bikeshedding had taken place publicly and be >> > actually part of that discussion, but yeah, ok. >> >> Sorry about that. I'd pinged him for an Ack, the discussion went >> more than that :) >> >> >> Some of the reasons behind having this: >> >> >> >> - We don't need to specify "rgb" in the compatible string since most >> >> simple VGA DACs can only work with an RGB input. >> > >> > Ok. >> > >> >> - Also, with "dac" specified in the string, we don't need to >> >> specifically mention "bridge" in the string. Also, bridge is a drm >> >> specific term. >> >> >> >> - "simple" is considered because it's an unconfigurable bridge, and it >> >> might be misleading for other VGA DACs to not use "vga-dac". >> > >> > All those "simple" bindings are just the biggest lie we ever >> > told. It's simple when you introduce it, and then grows into something >> > much more complicated than a non-simple implementation. >> >> "simple" here is supposed to mean that it's an unconfigurable RGB to >> VGA DAC. This isn't supposed to follow the simple-panel model, where >> you add the "simple-panel" string in the compatible node, along with >> you chip specific compatible string. > > I agree with Maxime, I don't like the word "simple". My preference would be > "vga-dac" for a lack of a better qualifier than "simple" to describe the fact > that the device requires no configuration. My only concern with "vga-dac" is > that we would restrict usage of that compatible string for a subset of VGA > DACs, with more complex devices not being compatible with "vga-dac" even > though they are VGA DACs. That's a problem I can live with though. While we're bikeshedding (feel free to ignore my input on this), I think Maxime's initial "dumb" qualifier was better than "simple". I think "passive" also gets the point across better than "simple", which we've already established as something else in drm. Now that I've gotten that out of the way, this patch looks good to me regardless of the name. Reviewed-by: Sean Paul Sean > >> In other words, this driver shouldn't be touched again in the future :) >> If someone wants to write a RGB to VGA driver which is even >> slightly configurable, they'll need to write a new bridge driver. > > I'm sure that won't be true. I can certainly foresee the addition of > regulators support for instance. It's unfortunately never black and white. > >> >> What do you think about this? If you think it's good, would it be >> >> possible for you to change this? I guess it's okay for the rest of >> >> the patch to stay the same. >> > >> > I'll update and respin the serie. > > -- > Regards, > > Laurent Pinchart > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/
[PATCH v5 0/3] drm/nouveau: set DMA mask before mapping scratch page
This v4 is now a 3 piece series (since v4), after Alexandre pointed out that
both GF 100 and NV50 are affected by the same issue, and that a related issue
has been solved already for Tegra in commit 9d0394c6bed5
("drm/nouveau/instmem/gk20a: set DMA mask early").
The issue that this series addresses is the fact that the Nouveau driver
invokes the DMA API before setting the DMA mask. In both cases addressed
here, these are simply static bidirectional mappings of scratch pages whose
purpose is not well understood, and in most cases, it does not matter that
these pages are always allocated below 4 GB even if the hardware can access
memory much higher up.
However, on platforms without any RAM below 4 GB, the preliminary DMA mask
of 32 is preventing the nouveau driver from loading on GF100 and NV50
hardware with an error like the following one:
nouveau :02:00.0: enabling device ( -> 0003)
nouveau :02:00.0: NVIDIA GT218 (0a8280b1)
nouveau :02:00.0: bios: version 70.18.a6.00.00
nouveau :02:00.0: fb ctor failed, -14
nouveau: probe of :02:00.0 failed with error -14
So fix this by setting a preliminary DMA mask based on the MMU device 'dma_bits'
property (patch #1), and postpone mapping the scratch pages to the respective
FB .init() hooks. (#2 and #3)
v5: move setting of preliminary DMA mask to nvkm_device_pci_new() (#1)
move allocation and DMA mapping of scratch pages to .oneinit hooks (#2, #3)
v4: split and move dma_set_mask to probe hook (Alexander)
v3: rework code to get rid of DMA_ERROR_CODE references, which is not
defined on all architectures
v2: replace incorrect comparison of dma_addr_t type var against NULL
Ard Biesheuvel (3):
drm/nouveau: set streaming DMA mask early
drm/nouveau/fb/gf100: defer DMA mapping of scratch page to oneinit()
hook
drm/nouveau/fb/nv50: defer DMA mapping of scratch page to oneinit()
hook
drivers/gpu/drm/nouveau/nvkm/engine/device/pci.c | 37 ++--
drivers/gpu/drm/nouveau/nvkm/subdev/fb/gf100.c | 31 +---
drivers/gpu/drm/nouveau/nvkm/subdev/fb/nv50.c| 33 +++--
3 files changed, 69 insertions(+), 32 deletions(-)
--
2.7.4
[PATCH v5 2/3] drm/nouveau/fb/gf100: defer DMA mapping of scratch page to oneinit() hook
The 100c10 scratch page is mapped using dma_map_page() before the TTM
layer has had a chance to set the DMA mask. This means we are still
running with the default of 32 when this code executes, and this causes
problems for platforms with no memory below 4 GB (such as AMD Seattle)
So move the dma_map_page() to the .oneinit hook, which executes after the
DMA mask has been set.
Signed-off-by: Ard Biesheuvel
---
drivers/gpu/drm/nouveau/nvkm/subdev/fb/gf100.c | 31
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gf100.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gf100.c
index 76433cc66fff..c1995c0024ef 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gf100.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gf100.c
@@ -50,24 +50,39 @@ gf100_fb_intr(struct nvkm_fb *base)
}
int
-gf100_fb_oneinit(struct nvkm_fb *fb)
+gf100_fb_oneinit(struct nvkm_fb *base)
{
- struct nvkm_device *device = fb->subdev.device;
+ struct gf100_fb *fb = gf100_fb(base);
+ struct nvkm_device *device = fb->base.subdev.device;
int ret, size = 0x1000;
size = nvkm_longopt(device->cfgopt, "MmuDebugBufferSize", size);
size = min(size, 0x1000);
ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST, size, 0x1000,
- false, &fb->mmu_rd);
+ false, &base->mmu_rd);
if (ret)
return ret;
ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST, size, 0x1000,
- false, &fb->mmu_wr);
+ false, &base->mmu_wr);
if (ret)
return ret;
+ fb->r100c10_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+ if (!fb->r100c10_page) {
+ nvkm_error(&fb->base.subdev, "failed 100c10 page alloc\n");
+ return -ENOMEM;
+ }
+
+ fb->r100c10 = dma_map_page(device->dev, fb->r100c10_page, 0, PAGE_SIZE,
+ DMA_BIDIRECTIONAL);
+ if (dma_mapping_error(device->dev, fb->r100c10)) {
+ nvkm_error(&fb->base.subdev, "failed to map 100c10 page\n");
+ __free_page(fb->r100c10_page);
+ return -EFAULT;
+ }
+
return 0;
}
@@ -123,14 +138,6 @@ gf100_fb_new_(const struct nvkm_fb_func *func, struct
nvkm_device *device,
nvkm_fb_ctor(func, device, index, &fb->base);
*pfb = &fb->base;
- fb->r100c10_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
- if (fb->r100c10_page) {
- fb->r100c10 = dma_map_page(device->dev, fb->r100c10_page, 0,
- PAGE_SIZE, DMA_BIDIRECTIONAL);
- if (dma_mapping_error(device->dev, fb->r100c10))
- return -EFAULT;
- }
-
return 0;
}
--
2.7.4
[Nouveau] [PATCH v4 1/3] drm/nouveau: set streaming DMA mask early
On 3 October 2016 at 06:39, Alexandre Courbot wrote: > On Mon, Sep 26, 2016 at 9:32 PM, Ard Biesheuvel > wrote: >> Some subdevices (i.e., fb/nv50.c and fb/gf100.c) map a scratch page using >> dma_map_page() way before the TTM layer has had a chance to set the DMA >> mask. This may prevent the driver from loading at all on platforms whose >> system memory is not covered by the default DMA mask of 32-bit (i.e., when >> all RAM is above 4 GB). >> >> So set a preliminary DMA mask right after constructing the PCI device, and >> base it on the .dma_bits member of the MMU subdevice, which is what the TTM >> layer will base the DMA mask on as well. >> >> Signed-off-by: Ard Biesheuvel >> --- >> drivers/gpu/drm/nouveau/nouveau_drm.c | 11 +++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c >> b/drivers/gpu/drm/nouveau/nouveau_drm.c >> index 652ab111dd74..e61e9a0adb51 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c >> @@ -361,6 +361,17 @@ static int nouveau_drm_probe(struct pci_dev *pdev, >> >> pci_set_master(pdev); >> >> + /* >> +* Set a preliminary DMA mask based on the .dma_bits member of the >> +* MMU subdevice. This allows other subdevices to create DMA mappings >> +* in their init() function, which are called before the TTM layer >> sets >> +* the DMA mask definitively. >> +* This is necessary for platforms where the default DMA mask of 32 >> +* does not cover any system memory, i.e., when all RAM is > 4 GB. >> +*/ >> + dma_set_mask_and_coherent(device->dev, >> + DMA_BIT_MASK(device->mmu->dma_bits)); > > I would just move this to nvkm_device_pci_new() so that it perfectly > mirrors the same call done in nvkm_device_tegra_new(), which was done > for the same purpose. Otherwise, looks good to me. OK, will do that.
[PATCH v5 3/3] drm/nouveau/fb/nv50: defer DMA mapping of scratch page to oneinit() hook
The 100c08 scratch page is mapped using dma_map_page() before the TTM
layer has had a chance to set the DMA mask. This means we are still
running with the default of 32 when this code executes, and this causes
problems for platforms with no memory below 4 GB (such as AMD Seattle)
So move the dma_map_page() to the .oneinit hook, which executes after the
DMA mask has been set.
Signed-off-by: Ard Biesheuvel
---
drivers/gpu/drm/nouveau/nvkm/subdev/fb/nv50.c | 33 ++--
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/nv50.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/nv50.c
index 1b5fb02eab2a..d9bc4d11f145 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/nv50.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/nv50.c
@@ -210,6 +210,28 @@ nv50_fb_intr(struct nvkm_fb *base)
nvkm_fifo_chan_put(fifo, flags, &chan);
}
+static int
+nv50_fb_oneinit(struct nvkm_fb *base)
+{
+ struct nv50_fb *fb = nv50_fb(base);
+
+ fb->r100c08_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+ if (!fb->r100c08_page) {
+ nvkm_error(&fb->base.subdev, "failed 100c08 page alloc\n");
+ return -ENOMEM;
+ }
+
+ fb->r100c08 = dma_map_page(device->dev, fb->r100c08_page, 0, PAGE_SIZE,
+ DMA_BIDIRECTIONAL);
+ if (dma_mapping_error(device->dev, fb->r100c08)) {
+ nvkm_error(&fb->base.subdev, "failed to map 100c08 page\n");
+ __free_page(fb->r100c08_page);
+ return -EFAULT;
+ }
+
+ return 0;
+}
+
static void
nv50_fb_init(struct nvkm_fb *base)
{
@@ -245,6 +267,7 @@ nv50_fb_dtor(struct nvkm_fb *base)
static const struct nvkm_fb_func
nv50_fb_ = {
.dtor = nv50_fb_dtor,
+ .oneinit = nv50_fb_oneinit,
.init = nv50_fb_init,
.intr = nv50_fb_intr,
.ram_new = nv50_fb_ram_new,
@@ -263,16 +286,6 @@ nv50_fb_new_(const struct nv50_fb_func *func, struct
nvkm_device *device,
fb->func = func;
*pfb = &fb->base;
- fb->r100c08_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
- if (fb->r100c08_page) {
- fb->r100c08 = dma_map_page(device->dev, fb->r100c08_page, 0,
- PAGE_SIZE, DMA_BIDIRECTIONAL);
- if (dma_mapping_error(device->dev, fb->r100c08))
- return -EFAULT;
- } else {
- nvkm_warn(&fb->base.subdev, "failed 100c08 page alloc\n");
- }
-
return 0;
}
--
2.7.4
[PATCH v9 17/19] drm/virtio: kconfig: Fix recursive dependency issue.
Hi Jani, Sorry for the delay, I've been travelling last week. On Tue, 20 Sep 2016, Jani Nikula wrote: > On Tue, 20 Sep 2016, Peter Griffin wrote: > > Hi Emil, > > > > On Tue, 20 Sep 2016, Emil Velikov wrote: > > > >> On 5 September 2016 at 14:16, Peter Griffin > >> wrote: > >> > ST_SLIM_REMOTEPROC must select REMOTEPROC, which exposes the following > >> > recursive dependency. > > > > > >> > > >> From a humble skim through remoteproc, drm and a few other subsystems > >> I think the above is wrong. All the drivers (outside of remoteproc), > >> that I've seen, depend on the core component, they don't select it. > > > > I will let Bjorn comment on the remoteproc subsystem Kconfig design, and > > why it is like it is. > > > > For this particular SLIM_RPROC I have added it to Kconfig in keeping with > > all > > the other drivers in the remoteproc subsystem which has exposed this > > recursive > > dependency issue. > > > > For this particular kconfig symbol a quick grep reveals more drivers in > > the kernel using 'select', than 'depend on' > > > > git grep "select VIRTIO" | wc -l > > 14 > > > > git grep "depends on VIRTIO" | wc -l > > 10 > > > > > >> Furthermore most places explicitly hide the drivers from the menu if > >> the core component isn't enabled. > > > > Remoteproc subsystem takes a different approach, the core code is only > > enabled > > if a driver which relies on it is enabled. This IMHO makes sense, as > > remoteproc is not widely used (only a few particular ARM SoC's). > > > > It is true that for subsystems which rely on the core component being > > explicitly enabled, they often tend to hide drivers which depend on it > > from the menu unless it is. This also makes sense. > > > >> > >> Is there something that requires such a different/unusual behaviour in > >> remoteproc ? > >> > > > > I'm not sure it is that unusual...looking at config USB, it selects > > USB_COMMON in > > mfd subsystem, client drivers select MFD_CORE. > > > > I've added Arnd to this thread, as I've seen lots of Kconfig patches from > > him > > recently, maybe he has some thoughts on whether this is the correct fix or > > not? > > > Documentation/kbuild/kconfig-language.txt: > > Note: > select should be used with care. select will force > a symbol to a value without visiting the dependencies. > By abusing select you are able to select a symbol FOO even > if FOO depends on BAR that is not set. > In general use select only for non-visible symbols > (no prompts anywhere) and for symbols with no dependencies. > That will limit the usefulness but on the other hand avoid > the illegal configurations all over. Thanks for the documentation link. I believe this proves this patch is an appropriate fix as VIRTIO is a non-visible symbol, and has no dependencies. In fact the help text for VIRTIO even states this option should be selected by any driver which implements virtio. > > People tend to abuse select because it's "convenient". If you depend, > but some of your dependencies aren't met, you're in for some digging > through Kconfig to find the missing deps. Just to make the option you > want visible in menuconfig. If you instead select something with > dependencies, it'll be right most of the time, and it's "convenient", > until it breaks. (And hey, it usually breaks for someone else with some > other config, so it's still convenient for you.) I'm sure they do but in this case it is actually the use of 'depends on' which has caused the breakage and inconvenience for somebody else and sadly this inconvienice is still on-going due to this patch not being applied or getting an acked-by from the appropriate maintainers. > > Perhaps kconfig should complain about selecting visible symbols and > symbols with dependencies. That sounds like it would be a useful addition. Is it possible to get this patch applied or an acked-by to avoid further delay to the fdma series? regards, Peter.
[PATCH v9 17/19] drm/virtio: kconfig: Fix recursive dependency issue.
Hi Emil,
On Wed, 21 Sep 2016, Emil Velikov wrote:
> On 20 September 2016 at 09:32, Peter Griffin
> wrote:
> > Hi Emil,
> >
> > On Tue, 20 Sep 2016, Emil Velikov wrote:
> >
> >> On 5 September 2016 at 14:16, Peter Griffin
> >> wrote:
> >> > ST_SLIM_REMOTEPROC must select REMOTEPROC, which exposes the following
> >> > recursive dependency.
> >
> >
> >> >
> >> From a humble skim through remoteproc, drm and a few other subsystems
> >> I think the above is wrong. All the drivers (outside of remoteproc),
> >> that I've seen, depend on the core component, they don't select it.
> >
> > I will let Bjorn comment on the remoteproc subsystem Kconfig design, and
> > why it is like it is.
> >
> > For this particular SLIM_RPROC I have added it to Kconfig in keeping with
> > all
> > the other drivers in the remoteproc subsystem which has exposed this
> > recursive
> > dependency issue.
> >
> > For this particular kconfig symbol a quick grep reveals more drivers in
> > the kernel using 'select', than 'depend on'
> >
> > git grep "select VIRTIO" | wc -l
> > 14
> >
> > git grep "depends on VIRTIO" | wc -l
> > 10
> >
> Might be worth taking a closer look into these at some point.
VIRTIO has no dependencies, and is a non visible symbol. Its Kconfig help also
mentions that drivers should select it.
>
> >
> >> Furthermore most places explicitly hide the drivers from the menu if
> >> the core component isn't enabled.
> >
> > Remoteproc subsystem takes a different approach, the core code is only
> > enabled
> > if a driver which relies on it is enabled. This IMHO makes sense, as
> > remoteproc is not widely used (only a few particular ARM SoC's).
> >
> > It is true that for subsystems which rely on the core component being
> > explicitly enabled, they often tend to hide drivers which depend on it
> > from the menu unless it is. This also makes sense.
> >
> >>
> >> Is there something that requires such a different/unusual behaviour in
> >> remoteproc ?
> >>
> >
> > I'm not sure it is that unusual...looking at config USB, it selects
> > USB_COMMON in
> > mfd subsystem, client drivers select MFD_CORE.
> >
> On the USB case I'm not sure what the reasoning behind the USB vs
> USB_COMMON split. In seems that one could just fold them, but that's
> another topic. On the MFD side... it follows the select {,mis,ab}use.
> With one (the only one?) MFD driver not using/selecting MFD_CORE doing
> it's own version of mfd_add_devices... which could be reworked,
> possibly.
Much like selecting VIRTIO in this patch, MFD_CORE is a non visible symbol
with no dependencies so it matches the documentation Jani referenced.
I personally think the approach taken makes sense, as why would you want to have
a CONFIG_MFD_CORE=y symbol being enabled unless you actually have a MFD device
which uses it also enabled in your kernel?
It seems to me a good solution to make the client drivers select the core
component so that it only gets enabled if at least one driver requires it.
This avoids having useless core code which will never be used compiled into the
kernel and in the end a smaller kernel size for a given kernel configuration
(better
cache use etc etc).
> > I've added Arnd to this thread, as I've seen lots of Kconfig patches from
> > him
> > recently, maybe he has some thoughts on whether this is the correct fix or
> > not?
> >
> Ack. Fwiw, I believe that the reasoning put by Jani is perfeclty
> reasonable, but it'll be great to hear others as well.
Yes me to. However I don't think anything in this patch is at odds with the
documentation Jani has referenced.
regards,
Peter.
[PATCH v5 1/3] drm/nouveau: set streaming DMA mask early
Some subdevices (i.e., fb/nv50.c and fb/gf100.c) map a scratch page using dma_map_page() way before the TTM layer has had a chance to set the DMA mask. This may prevent the driver from loading at all on platforms whose system memory is not covered by the default DMA mask of 32-bit (i.e., when all RAM is above 4 GB). So set a preliminary DMA mask right after constructing the PCI device, and base it on the .dma_bits member of the MMU subdevice, which is what the TTM layer will base the DMA mask on as well. Signed-off-by: Ard Biesheuvel --- drivers/gpu/drm/nouveau/nvkm/engine/device/pci.c | 37 ++-- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/pci.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/pci.c index 62ad0300cfa5..0030cd9543b2 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/pci.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/pci.c @@ -1665,14 +1665,31 @@ nvkm_device_pci_new(struct pci_dev *pci_dev, const char *cfg, const char *dbg, *pdevice = &pdev->device; pdev->pdev = pci_dev; - return nvkm_device_ctor(&nvkm_device_pci_func, quirk, &pci_dev->dev, - pci_is_pcie(pci_dev) ? NVKM_DEVICE_PCIE : - pci_find_capability(pci_dev, PCI_CAP_ID_AGP) ? - NVKM_DEVICE_AGP : NVKM_DEVICE_PCI, - (u64)pci_domain_nr(pci_dev->bus) << 32 | -pci_dev->bus->number << 16 | -PCI_SLOT(pci_dev->devfn) << 8 | -PCI_FUNC(pci_dev->devfn), name, - cfg, dbg, detect, mmio, subdev_mask, - &pdev->device); + ret = nvkm_device_ctor(&nvkm_device_pci_func, quirk, &pci_dev->dev, + pci_is_pcie(pci_dev) ? NVKM_DEVICE_PCIE : + pci_find_capability(pci_dev, PCI_CAP_ID_AGP) ? + NVKM_DEVICE_AGP : NVKM_DEVICE_PCI, + (u64)pci_domain_nr(pci_dev->bus) << 32 | + pci_dev->bus->number << 16 | + PCI_SLOT(pci_dev->devfn) << 8 | + PCI_FUNC(pci_dev->devfn), name, + cfg, dbg, detect, mmio, subdev_mask, + &pdev->device); + + if (ret) + return ret; + + /* +* Set a preliminary DMA mask based on the .dma_bits member of the +* MMU subdevice. This allows other subdevices to create DMA mappings +* in their init() or oneinit() methods, which may be called before the +* TTM layer sets the DMA mask definitively. +* This is necessary for platforms where the default DMA mask of 32 +* does not cover any system memory, i.e., when all RAM is > 4 GB. +*/ + if (subdev_mask & BIT(NVKM_SUBDEV_MMU)) + dma_set_mask_and_coherent(&pci_dev->dev, + DMA_BIT_MASK(pdev->device.mmu->dma_bits)); + + return 0; } -- 2.7.4
[GIT PULL] drm-vc4-next-2016-10-06
These are fixes that have been on the list for 1-3 weeks that didn't make it into 4.9. I've been running most of them most of the time, some have been merged downstream, and some have also been merged to the Fedora kernel build. This is about as much testing as we ever get on vc4, so I feel pretty good about them. The branch base is your current -next, because I wanted the merge forward of drm-vc4-fixes to avoid conflicts. The following changes since commit c2cbc38b9715bd8318062e600668fc30e5a3fbfa: drm: virtio: reinstate drm_virtio_set_busid() (2016-10-04 13:10:30 +1000) are available in the git repository at: https://github.com/anholt/linux tags/drm-vc4-next-2016-10-06 for you to fetch changes up to dfccd937deec9283d6ced73e138808e62bec54e8: drm/vc4: Add support for double-clocked modes. (2016-10-06 11:58:28 -0700) This pull request brings in several fixes for drm-next, mostly for HDMI. Eric Anholt (7): drm/vc4: Fix races when the CS reads from render targets. drm/vc4: Enable limited range RGB output on HDMI with CEA modes. drm/vc4: Fall back to using an EDID probe in the absence of a GPIO. drm/vc4: Increase timeout for HDMI_SCHEDULER_CONTROL changes. drm/vc4: Fix support for interlaced modes on HDMI. drm/vc4: Set up the AVI and SPD infoframes. drm/vc4: Add support for double-clocked modes. Masahiro Yamada (1): drm/vc4: cleanup with list_first_entry_or_null() drivers/gpu/drm/vc4/vc4_crtc.c | 64 +- drivers/gpu/drm/vc4/vc4_drv.h | 30 +++-- drivers/gpu/drm/vc4/vc4_gem.c | 13 ++ drivers/gpu/drm/vc4/vc4_hdmi.c | 231 +--- drivers/gpu/drm/vc4/vc4_regs.h | 19 ++- drivers/gpu/drm/vc4/vc4_render_cl.c | 21 +++- drivers/gpu/drm/vc4/vc4_validate.c | 17 ++- 7 files changed, 306 insertions(+), 89 deletions(-)
