[Bug 66963] Rv6xx dpm problems
https://bugs.freedesktop.org/show_bug.cgi?id=66963 --- Comment #249 from Kajzer --- Patch is working fine, had zero issues. However, it still hangs on boot sometimes, it doesn't actually hang, it looks like it's crashed for a couple of seconds, monitor flashes on and off, then it boots but without dpm. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/67bfc131/attachment.html>
[PATCH] drm/atomic-helper: implementatations for legacy interfaces
On Wed, Nov 05, 2014 at 02:48:48PM -0500, Sean Paul wrote: > > + if (!crtc && crtc != set->crtc) > > I think this should be an || Hm. My idea idea was actually something along the lines of diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 4f80885de3f6..077c792c46e0 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1271,7 +1271,14 @@ static int update_output_state(struct drm_atomic_state *state, struct drm_crtc *crtc = state->crtcs[i]; struct drm_crtc_state *crtc_state = state->crtc_states[i]; - if (!crtc && crtc != set->crtc) + if (!crtc) + continue; + + /* Don't update ->enable for the CRTC in the set_config request, +* since a mismatch would indicate a bug in the upper layers. +* The actual modeset code later on will catch any +* inconsistencies here. */ + if (crtc == set->crtc) continue; crtc_state->enable = I.e. that we don't recompute the new enable state for set->crtc so that we can catch bug in the helper function or core drm code which maps the legacy ->set_config to the atomic interface. Still r-b with that change applied, or want to take a deeper look again? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 10/17] drm: Atomic crtc/connector updates using crtc/plane helper interfaces
I've applied all the other nits, replies to the more interesting bits
below.
On Wed, Nov 05, 2014 at 01:53:48PM -0500, Sean Paul wrote:
> On Sun, Nov 02, 2014 at 02:19:23PM +0100, Daniel Vetter wrote:
> > + if (new_encoder != connector_state->best_encoder) {
>
> nit: If you just returned early when the encoder doesn't change, you can save
> indentation and line breaks.
Hm, that would mean either a goto (I don't like that for non-exceptional
control flow) or duplicating the debug output. I'll go with the latter and
frob it a bit.
> > + return ret;
> > +
> > + has_connectors = drm_atomic_connectors_for_crtc(state,
> > + crtc);
> > +
> > + if (crtc_state->enable != has_connectors) {
>
> This makes me a little nervous. Even though has_connectors is a bool,
> drm_atomic_connectors_for_crtc returns an int, and this seems like something
> that someone might "fix" in the future.
>
> [PATCH] drm/atomic: Use proper type for drm_atomic_connectors_for_crtc
Hm, yeah. I'll rename to num_connectors and add a !!. The idea of
returning an int and not just a bool is that drivers might care if there's
more than one, i.e. for cloned setups. At least i915 has some special
considerations for that in some cases.
> > + ret = drm_crtc_vblank_get(crtc);
> > + if (ret != 0)
> > + continue;
> > +
> > + old_crtc_state->enable = true;
> > + old_crtc_state->last_vblank_count = drm_vblank_count(dev, i);
>
> I think you should collect last_vblank_count before drm_crtc_vblank_get
vblank_get should block on anything, and I'm not sure whether the
drm_vblank_count can't just fall over if vblank_get fails (since
vblank_get before vblank_count is the pattern drm_irq.c uses iirc). So I'd
prefer it this way round just for paranoia. So I think I'll stick with
this scheme. The waiting is only done once we've grabbed all the vblank
count, and that's the important part to ensure we don't wait for too long.
This really is just all part of the "generic code has no idea about the
dpms state of a crtc". I'll plan to fix that with the missing dpms on
atomic pieces.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Bug#768157: machine hangs when loading cirrus with modeset=1 while booted with vga=791
Control: tag -1 upstream confirmed On Wed, 2014-11-05 at 15:02 +0100, Evgeni Golov wrote: > Package: src:linux > Version: 3.16.7-1 > Severity: normal > > Hi, > > while testing the new Grml release, I noticed an interesting hang of QEMU when > the machine is booted. The initial testing was done using [1] in QEMU from Sid > (2.1+dfsg-5), but I also could reproduce it with the same QEMU and a fresh > Jessie install with both, 3.16.5-1 and 3.16.7-1. > > To reproduce: > * boot a fresh Jessie with 3.16 kernel in QEMU and vga=791 as bootparam > * modprobe cirrus modeset=1 > * machine hangs > > I am not sure this will happen on real cirrus HW, as I don't have any, but > QEMU > is quite widespread and cirrus is the default VGA driver. [...] The cirrus KMS driver only binds to QEMU Cirrus devices, not real hardware. It has been working for me, but I can reproduce the apparent hang with the 'vga=791' parameter. It's not actually hanging, though - using a serial console, I see the error messages: [1.976874] [drm:cirrus_vram_init] *ERROR* can't reserve VRAM [1.978067] cirrus :00:02.0: Fatal error during GPU init: -6 and I can log in there. But, as the driver detects this error after the point of no return (remove_conflicting_framebuffers()), the display remains broken. Ben. -- Ben Hutchings The program is absolutely right; therefore, the computer must be wrong. -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 811 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/50d0a3a2/attachment.sig>
[PATCH V2 1/2] imx-drm: imx-hdmi: split imx soc specific code from imx-hdmi
imx6 and rockchip rk3288 and JZ4780 (Ingenic Xburst/MIPS)
use the interface compatible Designware HDMI IP, but they
also have some lightly difference, such as phy pll configuration,
register width(imx hdmi register is one byte, but rk3288 is 4
bytes width), 4K support(imx6 doesn't support 4k, but rk3288 does),
clk useage,and the crtc mux configuration is also platform specific.
To reuse the imx hdmi driver, split the platform specific code out
to dw_hdmi-imx.c.
Signed-off-by: Andy Yan
---
drivers/staging/imx-drm/Makefile | 2 +-
drivers/staging/imx-drm/dw_hdmi-imx.c | 214 ++
drivers/staging/imx-drm/imx-hdmi.c| 726 ++
include/drm/bridge/dw_hdmi.h | 114 ++
4 files changed, 634 insertions(+), 422 deletions(-)
create mode 100644 drivers/staging/imx-drm/dw_hdmi-imx.c
create mode 100644 include/drm/bridge/dw_hdmi.h
diff --git a/drivers/staging/imx-drm/Makefile b/drivers/staging/imx-drm/Makefile
index 582c438..809027d 100644
--- a/drivers/staging/imx-drm/Makefile
+++ b/drivers/staging/imx-drm/Makefile
@@ -9,4 +9,4 @@ obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
imx-ipuv3-crtc-objs := ipuv3-crtc.o ipuv3-plane.o
obj-$(CONFIG_DRM_IMX_IPUV3)+= imx-ipuv3-crtc.o
-obj-$(CONFIG_DRM_IMX_HDMI) += imx-hdmi.o
+obj-$(CONFIG_DRM_IMX_HDMI) += imx-hdmi.o dw_hdmi-imx.o
diff --git a/drivers/staging/imx-drm/dw_hdmi-imx.c
b/drivers/staging/imx-drm/dw_hdmi-imx.c
new file mode 100644
index 000..71ac859
--- /dev/null
+++ b/drivers/staging/imx-drm/dw_hdmi-imx.c
@@ -0,0 +1,214 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "imx-drm.h"
+
+struct imx_hdmi_priv {
+ struct device *dev;
+ struct clk *isfr_clk;
+ struct clk *iahb_clk;
+ struct regmap *regmap;
+};
+
+static const struct mpll_config imx_mpll_cfg[] = {
+ {
+ 4525, {
+ { 0x01e0, 0x },
+ { 0x21e1, 0x },
+ { 0x41e2, 0x }
+ },
+ }, {
+ 9250, {
+ { 0x0140, 0x0005 },
+ { 0x2141, 0x0005 },
+ { 0x4142, 0x0005 },
+ },
+ }, {
+ 14850, {
+ { 0x00a0, 0x000a },
+ { 0x20a1, 0x000a },
+ { 0x40a2, 0x000a },
+ },
+ }, {
+ ~0UL, {
+ { 0x00a0, 0x000a },
+ { 0x2001, 0x000f },
+ { 0x4002, 0x000f },
+ },
+ }
+};
+
+static const struct curr_ctrl imx_cur_ctr[] = {
+ /* pixelclk bpp8bpp10 bpp12 */
+ {
+ 5400, { 0x091c, 0x091c, 0x06dc },
+ }, {
+ 5840, { 0x091c, 0x06dc, 0x06dc },
+ }, {
+ 7200, { 0x06dc, 0x06dc, 0x091c },
+ }, {
+ 7425, { 0x06dc, 0x0b5c, 0x091c },
+ }, {
+ 11880, { 0x091c, 0x091c, 0x06dc },
+ }, {
+ 21600, { 0x06dc, 0x0b5c, 0x091c },
+ }
+};
+
+static int imx_hdmi_parse_dt(struct imx_hdmi_priv *hdmi)
+{
+ struct device_node *np = hdmi->dev->of_node;
+
+ hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "gpr");
+ if (IS_ERR(hdmi->regmap)) {
+ dev_err(hdmi->dev, "Unable to get gpr\n");
+ return PTR_ERR(hdmi->regmap);
+ }
+
+ hdmi->isfr_clk = devm_clk_get(hdmi->dev, "isfr");
+ if (IS_ERR(hdmi->isfr_clk)) {
+ dev_err(hdmi->dev, "Unable to get HDMI isfr clk\n");
+ return PTR_ERR(hdmi->isfr_clk);
+ }
+
+ hdmi->iahb_clk = devm_clk_get(hdmi->dev, "iahb");
+ if (IS_ERR(hdmi->iahb_clk)) {
+ dev_err(hdmi->dev, "Unable to get HDMI iahb clk\n");
+ return PTR_ERR(hdmi->iahb_clk);
+ }
+
+ return 0;
+}
+
+static void *imx_hdmi_imx_setup(struct platform_device *pdev)
+{
+ struct imx_hdmi_priv *hdmi;
+ int ret;
+
+ hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
+ if (!hdmi)
+ return ERR_PTR(-ENOMEM);
+ hdmi->dev = &pdev->dev;
+
+ ret = imx_hdmi_parse_dt(hdmi);
+ if (ret < 0)
+ return ERR_PTR(ret);
+ ret = clk_prepare_enable(hdmi->isfr_clk);
+ if (ret) {
+ dev_err(hdmi->dev,
+ "Cannot enable HDMI isfr clock: %d\n", ret);
+ return ERR_PTR(ret);
+ }
+
+ ret = clk_prepare_enable(hdmi->iahb_clk);
+ if (ret) {
+ dev_err(hdmi->dev,
+ "Cannot enable HDMI iahb clock: %d\n", ret);
+ return ERR_PTR(ret);
+ }
+
+ r
[PATCH V2 2/2] move imx-hdmi to bridge/dw-hdmi
the original imx hdmi driver is under staging/imx-drm,
which depends on imx-drm, so move the imx hdmi drvier out
to drm/bridge and rename imx-hdmi to dw-hdmi
Signed-off-by: Andy Yan
---
drivers/gpu/drm/bridge/Kconfig | 5 +
drivers/gpu/drm/bridge/Makefile| 1 +
.../imx-hdmi.c => gpu/drm/bridge/dw_hdmi.c}| 292 ++---
.../imx-hdmi.h => gpu/drm/bridge/dw_hdmi.h}| 0
drivers/staging/imx-drm/Kconfig| 1 +
drivers/staging/imx-drm/Makefile | 2 +-
drivers/staging/imx-drm/dw_hdmi-imx.c | 66 ++---
include/drm/bridge/dw_hdmi.h | 26 +-
8 files changed, 200 insertions(+), 193 deletions(-)
rename drivers/{staging/imx-drm/imx-hdmi.c => gpu/drm/bridge/dw_hdmi.c} (83%)
rename drivers/{staging/imx-drm/imx-hdmi.h => gpu/drm/bridge/dw_hdmi.h} (100%)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 884923f..26162ef 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -3,3 +3,8 @@ config DRM_PTN3460
depends on DRM
select DRM_KMS_HELPER
---help---
+
+config DRM_DW_HDMI
+ bool "Synopsys DesignWare High-Definition Multimedia Interface"
+ depends on DRM
+ select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index b4733e1..d8a8cfd 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,3 +1,4 @@
ccflags-y := -Iinclude/drm
obj-$(CONFIG_DRM_PTN3460) += ptn3460.o
+obj-$(CONFIG_DRM_DW_HDMI) += dw_hdmi.o
diff --git a/drivers/staging/imx-drm/imx-hdmi.c
b/drivers/gpu/drm/bridge/dw_hdmi.c
similarity index 83%
rename from drivers/staging/imx-drm/imx-hdmi.c
rename to drivers/gpu/drm/bridge/dw_hdmi.c
index 138706c..9c6aa88 100644
--- a/drivers/staging/imx-drm/imx-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -25,7 +25,7 @@
#include
#include
-#include "imx-hdmi.h"
+#include "dw_hdmi.h"
#define RGB0
#define YCBCR444 1
@@ -80,17 +80,17 @@ static const u16 csc_coeff_rgb_in_eitu709[3][4] = {
/*On rockchip platform, no-word access to the hdmi
* register will causes an imprecise external abort
*/
-static inline void hdmi_writel(struct imx_hdmi *hdmi, u32 val, int offset)
+static inline void hdmi_writel(struct dw_hdmi *hdmi, u32 val, int offset)
{
writel(val, hdmi->regs + (offset << 2));
}
-static inline u32 hdmi_readl(struct imx_hdmi *hdmi, int offset)
+static inline u32 hdmi_readl(struct dw_hdmi *hdmi, int offset)
{
return readl(hdmi->regs + (offset << 2));
}
-static void hdmi_modl(struct imx_hdmi *hdmi, u32 data, u32 mask, unsigned reg)
+static void hdmi_modl(struct dw_hdmi *hdmi, u32 data, u32 mask, unsigned reg)
{
u32 val = hdmi_readl(hdmi, reg) & ~mask;
@@ -98,23 +98,23 @@ static void hdmi_modl(struct imx_hdmi *hdmi, u32 data, u32
mask, unsigned reg)
hdmi_writel(hdmi, val, reg);
}
-static void hdmi_mask_writel(struct imx_hdmi *hdmi, u32 data, unsigned int reg,
+static void hdmi_mask_writel(struct dw_hdmi *hdmi, u32 data, unsigned int reg,
u32 shift, u32 mask)
{
hdmi_modl(hdmi, data << shift, mask, reg);
}
-static inline void hdmi_writeb(struct imx_hdmi *hdmi, u32 val, int offset)
+static inline void hdmi_writeb(struct dw_hdmi *hdmi, u32 val, int offset)
{
writeb(val, hdmi->regs + offset);
}
-static inline u32 hdmi_readb(struct imx_hdmi *hdmi, int offset)
+static inline u32 hdmi_readb(struct dw_hdmi *hdmi, int offset)
{
return readb(hdmi->regs + offset);
}
-static void hdmi_modb(struct imx_hdmi *hdmi, u32 data, u32 mask, unsigned reg)
+static void hdmi_modb(struct dw_hdmi *hdmi, u32 data, u32 mask, unsigned reg)
{
u8 val = hdmi_readb(hdmi, reg) & ~mask;
@@ -122,13 +122,13 @@ static void hdmi_modb(struct imx_hdmi *hdmi, u32 data,
u32 mask, unsigned reg)
hdmi_writeb(hdmi, val, reg);
}
-static void hdmi_mask_writeb(struct imx_hdmi *hdmi, u32 data, unsigned int reg,
+static void hdmi_mask_writeb(struct dw_hdmi *hdmi, u32 data, unsigned int reg,
u32 shift, u32 mask)
{
hdmi_modb(hdmi, data << shift, mask, reg);
}
-static void hdmi_set_clock_regenerator_n(struct imx_hdmi *hdmi,
+static void hdmi_set_clock_regenerator_n(struct dw_hdmi *hdmi,
unsigned int value)
{
hdmi->write(hdmi, value & 0xff, HDMI_AUD_N1);
@@ -139,7 +139,7 @@ static void hdmi_set_clock_regenerator_n(struct imx_hdmi
*hdmi,
hdmi->mod(hdmi, 0, HDMI_AUD_CTS3_N_SHIFT_MASK, HDMI_AUD_CTS3);
}
-static void hdmi_regenerate_cts(struct imx_hdmi *hdmi, unsigned int cts)
+static void hdmi_regenerate_cts(struct dw_hdmi *hdmi, unsigned int cts)
{
/* Must be set/cleared first */
hdmi->mod(hdmi, 0, HDMI_AUD_CTS3_CTS_MANUAL, HDMI_AUD_CTS3);
@@ -283,11 +283,11 @@ static u
[PATCH 1/2] imx-drm: imx-hdmi: split imx soc specific code from imx-hdmi
imx6 and rockchip rk3288 and JZ4780 (Ingenic Xburst/MIPS)
use the interface compatible Designware HDMI IP, but they
also have some lightly difference, such as phy pll configuration,
register width(imx hdmi register is one byte, but rk3288 is 4
bytes width), 4K support(imx6 doesn't support 4k, but rk3288 does),
clk useage,and the crtc mux configuration is also platform specific.
To reuse the imx hdmi driver, split the platform specific code out
to dw_hdmi-imx.c.
Signed-off-by: Andy Yan
---
drivers/staging/imx-drm/Makefile | 2 +-
drivers/staging/imx-drm/dw_hdmi-imx.c | 214 ++
drivers/staging/imx-drm/imx-hdmi.c| 726 ++
include/drm/bridge/dw_hdmi.h | 114 ++
4 files changed, 634 insertions(+), 422 deletions(-)
create mode 100644 drivers/staging/imx-drm/dw_hdmi-imx.c
create mode 100644 include/drm/bridge/dw_hdmi.h
diff --git a/drivers/staging/imx-drm/Makefile b/drivers/staging/imx-drm/Makefile
index 582c438..809027d 100644
--- a/drivers/staging/imx-drm/Makefile
+++ b/drivers/staging/imx-drm/Makefile
@@ -9,4 +9,4 @@ obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
imx-ipuv3-crtc-objs := ipuv3-crtc.o ipuv3-plane.o
obj-$(CONFIG_DRM_IMX_IPUV3)+= imx-ipuv3-crtc.o
-obj-$(CONFIG_DRM_IMX_HDMI) += imx-hdmi.o
+obj-$(CONFIG_DRM_IMX_HDMI) += imx-hdmi.o dw_hdmi-imx.o
diff --git a/drivers/staging/imx-drm/dw_hdmi-imx.c
b/drivers/staging/imx-drm/dw_hdmi-imx.c
new file mode 100644
index 000..71ac859
--- /dev/null
+++ b/drivers/staging/imx-drm/dw_hdmi-imx.c
@@ -0,0 +1,214 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "imx-drm.h"
+
+struct imx_hdmi_priv {
+ struct device *dev;
+ struct clk *isfr_clk;
+ struct clk *iahb_clk;
+ struct regmap *regmap;
+};
+
+static const struct mpll_config imx_mpll_cfg[] = {
+ {
+ 4525, {
+ { 0x01e0, 0x },
+ { 0x21e1, 0x },
+ { 0x41e2, 0x }
+ },
+ }, {
+ 9250, {
+ { 0x0140, 0x0005 },
+ { 0x2141, 0x0005 },
+ { 0x4142, 0x0005 },
+ },
+ }, {
+ 14850, {
+ { 0x00a0, 0x000a },
+ { 0x20a1, 0x000a },
+ { 0x40a2, 0x000a },
+ },
+ }, {
+ ~0UL, {
+ { 0x00a0, 0x000a },
+ { 0x2001, 0x000f },
+ { 0x4002, 0x000f },
+ },
+ }
+};
+
+static const struct curr_ctrl imx_cur_ctr[] = {
+ /* pixelclk bpp8bpp10 bpp12 */
+ {
+ 5400, { 0x091c, 0x091c, 0x06dc },
+ }, {
+ 5840, { 0x091c, 0x06dc, 0x06dc },
+ }, {
+ 7200, { 0x06dc, 0x06dc, 0x091c },
+ }, {
+ 7425, { 0x06dc, 0x0b5c, 0x091c },
+ }, {
+ 11880, { 0x091c, 0x091c, 0x06dc },
+ }, {
+ 21600, { 0x06dc, 0x0b5c, 0x091c },
+ }
+};
+
+static int imx_hdmi_parse_dt(struct imx_hdmi_priv *hdmi)
+{
+ struct device_node *np = hdmi->dev->of_node;
+
+ hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "gpr");
+ if (IS_ERR(hdmi->regmap)) {
+ dev_err(hdmi->dev, "Unable to get gpr\n");
+ return PTR_ERR(hdmi->regmap);
+ }
+
+ hdmi->isfr_clk = devm_clk_get(hdmi->dev, "isfr");
+ if (IS_ERR(hdmi->isfr_clk)) {
+ dev_err(hdmi->dev, "Unable to get HDMI isfr clk\n");
+ return PTR_ERR(hdmi->isfr_clk);
+ }
+
+ hdmi->iahb_clk = devm_clk_get(hdmi->dev, "iahb");
+ if (IS_ERR(hdmi->iahb_clk)) {
+ dev_err(hdmi->dev, "Unable to get HDMI iahb clk\n");
+ return PTR_ERR(hdmi->iahb_clk);
+ }
+
+ return 0;
+}
+
+static void *imx_hdmi_imx_setup(struct platform_device *pdev)
+{
+ struct imx_hdmi_priv *hdmi;
+ int ret;
+
+ hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
+ if (!hdmi)
+ return ERR_PTR(-ENOMEM);
+ hdmi->dev = &pdev->dev;
+
+ ret = imx_hdmi_parse_dt(hdmi);
+ if (ret < 0)
+ return ERR_PTR(ret);
+ ret = clk_prepare_enable(hdmi->isfr_clk);
+ if (ret) {
+ dev_err(hdmi->dev,
+ "Cannot enable HDMI isfr clock: %d\n", ret);
+ return ERR_PTR(ret);
+ }
+
+ ret = clk_prepare_enable(hdmi->iahb_clk);
+ if (ret) {
+ dev_err(hdmi->dev,
+ "Cannot enable HDMI iahb clock: %d\n", ret);
+ return ERR_PTR(ret);
+ }
+
+ r
[PATCH V2 0/2] make imx hdmi publicly used by dw hdmi compatible platform
We found freescale imx6 and rockchip rk3288 and Ingenic JZ4780 (Xburst/MIPS)
use the interface compatible Designware HDMI IP, but they also have some
lightly difference, such as phy pll configuration, register width(imx hdmi
register is one byte, but rk3288 is 4 bytes width and can only access by word),
4K support(imx6 doesn't support 4k, but rk3288 does).
To reuse the imx-hdmi driver, we do this patch set:
patch (1): split out imx-soc code from imx-hdmi to dw_hdmi-imx.c
patch (2): move imx-hdmi to bridge/, and rename to dw-hdmi to
make this driver indepent of drm-imx . And we will add rockchip
platform specific code dw_hdmi-rockchip.c later, this is depend
on drm-rockchip.
Changes in V2:
- use git format-patch -M to generate these patch
- remove change-id
- remove from
Andy Yan (2):
imx-drm: imx-hdmi: split imx soc specific code from imx-hdmi
move imx-hdmi to bridge/dw-hdmi
drivers/gpu/drm/bridge/Kconfig | 5 +
drivers/gpu/drm/bridge/Makefile| 1 +
.../imx-hdmi.c => gpu/drm/bridge/dw_hdmi.c}| 964 +
.../imx-hdmi.h => gpu/drm/bridge/dw_hdmi.h}| 0
drivers/staging/imx-drm/Kconfig| 1 +
drivers/staging/imx-drm/Makefile | 2 +-
drivers/staging/imx-drm/dw_hdmi-imx.c | 214 +
include/drm/bridge/dw_hdmi.h | 114 +++
8 files changed, 760 insertions(+), 541 deletions(-)
rename drivers/{staging/imx-drm/imx-hdmi.c => gpu/drm/bridge/dw_hdmi.c} (57%)
rename drivers/{staging/imx-drm/imx-hdmi.h => gpu/drm/bridge/dw_hdmi.h} (100%)
create mode 100644 drivers/staging/imx-drm/dw_hdmi-imx.c
create mode 100644 include/drm/bridge/dw_hdmi.h
--
1.9.1
exynos-drm registration: infinite loop
2014-11-05 17:38 GMT+03:00 Thierry Reding : > On Tue, Nov 04, 2014 at 09:18:46PM +0400, Matwey V. Kornilov wrote: >> Hi, >> >> I run 3.18-rc3 kernel on BeagleBone Black. It doesn't have Exynos DRM >> of course, but I run multi-platform kernel where CONFIG_DRM_EXYNOS is >> set to 'y'. >> The issue here is that the platform probe/init goes to infinite loop >> as the following: >> >> [5.717343] platform exynos-drm: Driver exynos-drm requests probe deferral >> [5.726848] exynos-drm-ipp exynos-drm-ipp: drm ipp registered >> successfully. >> [5.734700] platform exynos-drm: Driver exynos-drm requests probe deferral >> [5.744044] exynos-drm-ipp exynos-drm-ipp: drm ipp registered >> successfully. >> [5.752010] platform exynos-drm: Driver exynos-drm requests probe deferral >> [5.761377] exynos-drm-ipp exynos-drm-ipp: drm ipp registered >> successfully. >> [5.769291] platform exynos-drm: Driver exynos-drm requests probe deferral >> >> It is quite unexpectable behavior. I would expect that the exynos-drm >> failed to initialize with 'no device' error. >> >> See also for the reference: https://bugzilla.kernel.org/show_bug.cgi?id=87691 > > The reason for this seems to be that Exynos DRM instantiates a dummy > device that it can bind to (see exynos_drm_init()). Now this code is > executed unconditonally, so the device will be created whether or not > the kernel/module runs on an Exynos SoC. The driver should really bind > to some real device rather than instantiating a dummy. Or if it must > bind to a dummy then the code needs to at least check that it's running > on an Exynos SoC before instantiating the dummy. > > Thierry Could it be fixed somehow? By the way, I am not sure that exynos_drm_match_add behaves correctly. It returns EPROBE_DEFER when there are no matches, but it doesn't check (probably can not check) whether all the probes have been run. -- With best regards, Matwey V. Kornilov http://blog.matwey.name xmpp://0x2207 at jabber.ru
[PATCH 6/7] drm/rcar: gem: dumb: pitch is an output
Hi Thierry, Thank you for the patch. On Wednesday 05 November 2014 14:25:18 Thierry Reding wrote: > From: Thierry Reding > > When creating a dumb buffer object using the DRM_IOCTL_MODE_CREATE_DUMB > IOCTL, only the width, height, bpp and flags fields are inputs. The > caller is not guaranteed to zero out or set handle, pitch and size. > Drivers must not treat these values as possible inputs, otherwise they > may use uninitialized memory during the computation of the framebuffer > size. > > The R-Car DU driver treats the pitch passed in from userspace as minimum > and will only overwrite it when the driver-computed pitch is larger, > allowing userspace to, intentionally or not, overallocate framebuffers. As discussed on IRC, my concern with this is that some userspace applications might be relying on this behaviour. I'm not personally aware of any though, so I'm not opposed to this patch, but I can't vouch it won't cause any userspace breakage. (On a side note I believe treating the pitch and size arguments as inputs could be a worthwhile extension to the API, but given that we haven't rejected incorrect values in the past we're pretty much stuck). > Signed-off-by: Thierry Reding > --- > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c > b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 5329491e32c3..6289e3797bc5 > 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c > @@ -126,7 +126,7 @@ int rcar_du_dumb_create(struct drm_file *file, struct > drm_device *dev, else > align = 16 * args->bpp / 8; > > - args->pitch = roundup(max(args->pitch, min_pitch), align); > + args->pitch = roundup(min_pitch, align); > > return drm_gem_cma_dumb_create_internal(file, dev, args); > } -- Regards, Laurent Pinchart
[PATCH] drm/exynos: add has_vtsel flag
The exynos fimd provides video type selection bits from system register
but exynos3 series don't has it, so needs has_vtsel flag and we can
distinguish whether set video type selection bits.
Signed-off-by: Joonyoung Shim
---
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 085b066..1b32771 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -96,6 +96,7 @@ struct fimd_driver_data {
unsigned int has_clksel:1;
unsigned int has_limited_fmt:1;
unsigned int has_vidoutcon:1;
+ unsigned int has_vtsel:1;
};
static struct fimd_driver_data s3c64xx_fimd_driver_data = {
@@ -118,6 +119,7 @@ static struct fimd_driver_data exynos4_fimd_driver_data = {
.lcdblk_vt_shift = 10,
.lcdblk_bypass_shift = 1,
.has_shadowcon = 1,
+ .has_vtsel = 1,
};
static struct fimd_driver_data exynos5_fimd_driver_data = {
@@ -127,6 +129,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = {
.lcdblk_bypass_shift = 15,
.has_shadowcon = 1,
.has_vidoutcon = 1,
+ .has_vtsel = 1,
};
struct fimd_win_data {
@@ -343,7 +346,8 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
writel(0, timing_base + I80IFCONFBx(0));
/* set video type selection to I80 interface */
- if (ctx->sysreg && regmap_update_bits(ctx->sysreg,
+ if (driver_data->has_vtsel && ctx->sysreg &&
+ regmap_update_bits(ctx->sysreg,
driver_data->lcdblk_offset,
0x3 << driver_data->lcdblk_vt_shift,
0x1 << driver_data->lcdblk_vt_shift)) {
--
1.9.1
[PATCH] drm/exynos: add has_vtsel flag
The exynos fimd provides video type selection bits from system register
but exynos3 series don't has it, so needs has_vtsel flag and we can
distinguish whether set video type selection bits.
Signed-off-by: Joonyoung Shim
---
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 085b066..1b32771 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -96,6 +96,7 @@ struct fimd_driver_data {
unsigned int has_clksel:1;
unsigned int has_limited_fmt:1;
unsigned int has_vidoutcon:1;
+ unsigned int has_vtsel:1;
};
static struct fimd_driver_data s3c64xx_fimd_driver_data = {
@@ -118,6 +119,7 @@ static struct fimd_driver_data exynos4_fimd_driver_data = {
.lcdblk_vt_shift = 10,
.lcdblk_bypass_shift = 1,
.has_shadowcon = 1,
+ .has_vtsel = 1,
};
static struct fimd_driver_data exynos5_fimd_driver_data = {
@@ -127,6 +129,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = {
.lcdblk_bypass_shift = 15,
.has_shadowcon = 1,
.has_vidoutcon = 1,
+ .has_vtsel = 1,
};
struct fimd_win_data {
@@ -343,7 +346,8 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
writel(0, timing_base + I80IFCONFBx(0));
/* set video type selection to I80 interface */
- if (ctx->sysreg && regmap_update_bits(ctx->sysreg,
+ if (driver_data->has_vtsel && ctx->sysreg &&
+ regmap_update_bits(ctx->sysreg,
driver_data->lcdblk_offset,
0x3 << driver_data->lcdblk_vt_shift,
0x1 << driver_data->lcdblk_vt_shift)) {
--
1.9.1
[Bug 87791] radeonsi lockup and oops
https://bugzilla.kernel.org/show_bug.cgi?id=87791 --- Comment #3 from aCaB --- (In reply to Alex Deucher from comment #2) > This sounds like a mesa regression rather than a kernel driver bug. Can you > bisect mesa? I understand mesa may be sending crap to the kernel space but that doesn't sound like a good reason to deref a NULL. As for bisecting mesa, I am certainly willing to do that but I need a reliable way to trigger the lockup rather than just log in and wait for it to occour. Will see if I get more hints over then next few days. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH] drm/exynos: add has_vtsel flag
The exynos fimd provides video type selection bits from system register
but exynos3 series don't has it, so needs has_vtsel flag and we can
distinguish whether set video type selection bits.
Signed-off-by: Joonyoung Shim
---
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 085b066..1b32771 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -96,6 +96,7 @@ struct fimd_driver_data {
unsigned int has_clksel:1;
unsigned int has_limited_fmt:1;
unsigned int has_vidoutcon:1;
+ unsigned int has_vtsel:1;
};
static struct fimd_driver_data s3c64xx_fimd_driver_data = {
@@ -118,6 +119,7 @@ static struct fimd_driver_data exynos4_fimd_driver_data = {
.lcdblk_vt_shift = 10,
.lcdblk_bypass_shift = 1,
.has_shadowcon = 1,
+ .has_vtsel = 1,
};
static struct fimd_driver_data exynos5_fimd_driver_data = {
@@ -127,6 +129,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = {
.lcdblk_bypass_shift = 15,
.has_shadowcon = 1,
.has_vidoutcon = 1,
+ .has_vtsel = 1,
};
struct fimd_win_data {
@@ -343,7 +346,8 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
writel(0, timing_base + I80IFCONFBx(0));
/* set video type selection to I80 interface */
- if (ctx->sysreg && regmap_update_bits(ctx->sysreg,
+ if (driver_data->has_vtsel && ctx->sysreg &&
+ regmap_update_bits(ctx->sysreg,
driver_data->lcdblk_offset,
0x3 << driver_data->lcdblk_vt_shift,
0x1 << driver_data->lcdblk_vt_shift)) {
--
1.9.1
[Bug 87791] radeonsi lockup and oops
https://bugzilla.kernel.org/show_bug.cgi?id=87791 Alex Deucher changed: What|Removed |Added CC||alexdeucher at gmail.com --- Comment #2 from Alex Deucher --- (In reply to aCaB from comment #0) > Created attachment 156761 [details] > lockup example (no oops) > > After upgrading mesa from mesa-10.3.0 to mesa-10.3.1 the Radeon HD 7700 card > locks up several times a day without any specific trigger (or reliable way > to reproduce it). > This sounds like a mesa regression rather than a kernel driver bug. Can you bisect mesa? -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 87791] radeonsi lockup and oops
https://bugzilla.kernel.org/show_bug.cgi?id=87791 aCaB changed: What|Removed |Added Attachment #156761|application/octet-stream|text/plain mime type|| -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 87791] radeonsi lockup and oops
https://bugzilla.kernel.org/show_bug.cgi?id=87791 aCaB changed: What|Removed |Added Attachment #156771|application/octet-stream|text/plain mime type|| -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 87791] radeonsi lockup and oops
https://bugzilla.kernel.org/show_bug.cgi?id=87791 --- Comment #1 from aCaB --- Created attachment 156771 --> https://bugzilla.kernel.org/attachment.cgi?id=156771&action=edit lockup example (with oops) -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 87791] New: radeonsi lockup and oops
https://bugzilla.kernel.org/show_bug.cgi?id=87791 Bug ID: 87791 Summary: radeonsi lockup and oops Product: Drivers Version: 2.5 Kernel Version: 3.17 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-dri at kernel-bugs.osdl.org Reporter: acab at digitalfuture.it Regression: No Created attachment 156761 --> https://bugzilla.kernel.org/attachment.cgi?id=156761&action=edit lockup example (no oops) After upgrading mesa from mesa-10.3.0 to mesa-10.3.1 the Radeon HD 7700 card locks up several times a day without any specific trigger (or reliable way to reproduce it). Xorg appears frozen with just the mouse pointer moving. It's not even possible to switch to a VT, however everything else works just fine. At any given time the X bt looks like this: Thread 2 (Thread 0x7fe8147be700 (LWP 2415)): #0 0x7fe81b4d911c in pthread_cond_wait () from /lib64/libpthread.so.0 #1 0x7fe816a807a3 in ?? () from /usr/lib64/dri/radeonsi_dri.so #2 0x7fe816a7ffc7 in ?? () from /usr/lib64/dri/radeonsi_dri.so #3 0x7fe81b4d5083 in start_thread () from /lib64/libpthread.so.0 #4 0x7fe81b9da3ad in clone () from /lib64/libc.so.6 Thread 1 (Thread 0x7fe81d607880 (LWP 2380)): #0 0x7fe81b9836e9 in __memcpy_sse2_unaligned () from /lib64/libc.so.6 #1 0x7fe816857a46 in ?? () from /usr/lib64/dri/radeonsi_dri.so #2 0x7fe816858742 in ?? () from /usr/lib64/dri/radeonsi_dri.so #3 0x7fe816859452 in ?? () from /usr/lib64/dri/radeonsi_dri.so #4 0x7fe8168abf13 in ?? () from /usr/lib64/dri/radeonsi_dri.so #5 0x7fe8168ac993 in ?? () from /usr/lib64/dri/radeonsi_dri.so #6 0x7fe81684ad74 in ?? () from /usr/lib64/dri/radeonsi_dri.so #7 0x7fe81684c510 in ?? () from /usr/lib64/dri/radeonsi_dri.so #8 0x7fe81a58b883 in ?? () from /usr/lib64/xorg/modules/libglamoregl.so #9 0x7fe81a58c64e in ?? () from /usr/lib64/xorg/modules/libglamoregl.so #10 0x7fe81a58d160 in ?? () from /usr/lib64/xorg/modules/libglamoregl.so #11 0x7fe81a58d81c in ?? () from /usr/lib64/xorg/modules/libglamoregl.so #12 0x7fe81a56c674 in ?? () from /usr/lib64/xorg/modules/libglamoregl.so #13 0x7fe81a56cd84 in ?? () from /usr/lib64/xorg/modules/libglamoregl.so #14 0x7fe81a56dd5e in ?? () from /usr/lib64/xorg/modules/libglamoregl.so #15 0x7fe81a56e6ba in ?? () from /usr/lib64/xorg/modules/libglamoregl.so #16 0x00563e2d in miCopyRegion () #17 0x005643b6 in miDoCopy () #18 0x7fe81a56e6fd in ?? () from /usr/lib64/xorg/modules/libglamoregl.so #19 0x00511828 in ?? () #20 0x00432291 in ?? () #21 0x00435d3e in ?? () #22 0x00439b6a in ?? () #23 0x7fe81b913a65 in __libc_start_main () from /lib64/libc.so.6 #24 0x0042531e in _start () -- You are receiving this mail because: You are watching the assignee of the bug.
[lm-sensors] Kaveri [and Radeon] temps
> "GR" == Guenter Roeck writes: ... GR> AMD CPUs are known report wildly off temperatures especially at GR> low temperatures. Thanks. After some further digging, I wonder whether Aravind's recent patch might help? The apu does have both PCI_DEVICE_ID_AMD_16H_M30H_NB_F3 and PCI_DEVICE_ID_AMD_16H_M30H_NB_F4 and lspci reports no driver for the F4 device. GR> Does the temperature increase with load ? It seems to max out w/in a kelvin or two of the room's ambient temp. -JimC -- James Cloos OpenPGP: 0x997A9F17ED7DAEA6
[RFC 1/2] core: Add generic object registry implementation
On Wed, Nov 05, 2014 at 10:13:53AM +0100, Thierry Reding wrote:
> On Tue, Nov 04, 2014 at 08:38:45AM -0800, Greg Kroah-Hartman wrote:
> > On Tue, Nov 04, 2014 at 05:29:27PM +0100, Thierry Reding wrote:
> [...]
> > > diff --git a/drivers/base/registry.c b/drivers/base/registry.c
> [...]
> > > +/**
> > > + * registry_record_ref - reference on the registry record
> > > + * @record: record to reference
> > > + *
> > > + * Increases the reference count on the record and returns a pointer to
> > > it.
> > > + *
> > > + * Return: A pointer to the record on success or NULL on failure.
> > > + */
> > > +struct registry_record *registry_record_ref(struct registry_record
> > > *record)
> > > +{
> > > + if (!record)
> > > + return NULL;
> > > +
> > > + /*
> > > + * Refuse to give out any more references if the module owning the
> > > + * record is being removed.
> > > + */
> > > + if (!try_module_get(record->owner))
> > > + return NULL;
> > > +
> > > + kref_get(&record->kref);
> >
> > You "protect" from a module being removed, but not from someone else
> > releasing the kref at the same time. Where is the lock that prevents
> > this from happening?
>
> You're right, we need a record lock to serialize ref/unref.
>
> > And do we really care about module reference counts here?
>
> We need this so that the code of the .release() callback stays in
> memory as long as there are any references.
>
> Also we need the module reference count for registries because they
> would typically be statically allocated and go away along with a module
> when it is unloaded.
Never use a 'static' variable as a dynamic one with a kref, that's just
asking for trouble.
> > > +/**
> > > + * registry_add - add a record to a registry
> > > + * @registry: registry to add the record to
> > > + * @record: record to add
> > > + *
> > > + * Tries to increase the reference count of the module owning the
> > > registry. If
> > > + * successful adds the new record to the registry.
> > > + *
> > > + * Return: 0 on success or a negative error code on failure.
> > > + */
> > > +int registry_add(struct registry *registry, struct registry_record
> > > *record)
> > > +{
> > > + if (!try_module_get(registry->owner))
> > > + return -ENODEV;
> > > +
> > > + mutex_lock(®istry->lock);
> > > + list_add_tail(&record->list, ®istry->list);
> > > + mutex_unlock(®istry->lock);
> >
> > No incrementing of the reference of the record at all?
>
> I'm not sure if we really need one. Drivers will have to remove the
> record from the registry before dropping their reference. I guess we
> could throw in another kref_get() here (and a kref_put() in
> registry_remove()) for good measure to prevent breakage with buggy
> drivers.
Or at least warn people that they need to clean stuff up properly if
they do not, otherwise they will get it wrong. You need to make it very
hard to get wrong.
> > You seem to be using 2 reference counts for the record / registry, a
> > module count, and a kref count, which can cause confusion...
>
> There is one reference count (kref actually) per registry record and one
> module count for the record owner and the registry owner.
But both counts are in the same structure, which is a mess.
> Can you elaborate what you think is confusing? Perhaps I can add more
> kerneldoc to clarify.
You have 2 references in the same structure, with different lifecycles,
causing confusion, and odds are, bugs...
Sure, document it better if you want, but I think something needs to be
done differently if at all possible.
thanks,
greg k-h
[PATCH] drm: Global atomic state handling
Some differences compared to Rob's patches again: - Dropped the committed and checked booleans. Checking will be internally enforced by always calling ->atomic_check before ->atomic_commit. And async handling needs to be solved differently because the current scheme completely side-steps ww mutex deadlock avoidance (and so either reinvents a new deadlock avoidance wheel or like the current code just deadlocks). - State for connectors needed to be added, since now they have a full-blown drm_connector_state (so that drivers have something to attach their own stuff to). - Refcounting is gone. I plane to solve async updates differently, since the lock-passing scheme doesn't cut it (since it abuses ww mutexes). Essentially what we need for async is a simple ownership transfer from the caller to the driver. That doesn't need full-blown refcounting. - The acquire ctx is a pointer. Real atomic callers should have that on their stack, legacy entry points need to put the right one (obtained by drm_modeset_legacy_acuire_ctx) in there. - I've dropped all hooks except check/commit. All the begin/end handling is done by core functions and is the same. - commit/check are just thin wrappers that ensure that ->check is always called. - To help out with locking in the legacy implementations I've added a helper to just grab all locks in the backoff case. v2: Add notices that check/commit can fail with EDEADLK. v3: - More consistent naming for state_alloc. - Add state_clear which is needed for backoff and retry. v4: Planes/connectors can switch between crtcs, and we need to be careful that we grab the state (and locks) for both the old and new crtc. Improve the interface functions to ensure this. v5: Add functions to grab affected connectors for a crtc and to recompute the crtc->enable state. This is useful for both helper and atomic ioctl code when e.g. removing a connector. v6: Squash in fixup from Fengguang to use ERR_CAST. v7: Add debug output. v8: Make checkpatch happy about kcalloc argument ordering. v9: Improve kerneldoc in drm_crtc.h v10: - Fix another kcalloc argument misorder I've missed. - More polish for kerneldoc. v11: Clarify the ownership rules for the state object. The new rule is that a successful drm_atomic_commit (whether synchronous or asnyc) always inherits the state and is responsible for the clean-up. That way async and sync ->commit functions are more similar. v12: A few bugfixes: - Assign state->state pointers correctly when grabbing state objects - we need to link them up with the global state. - Handle a NULL crtc in set_crtc_for_plane to simplify code flow a bit for the callers of this function. v13: Review from Sean: - kerneldoc spelling fixes - Don't overallocate states->planes. - Handle NULL crtc in set_crtc_for_connector. v14: Sprinkle __must_check over all functions which do wait/wound locking to make sure callers don't forget this. Since I have ;-) v15: Be more explicit in the kerneldoc when functions can return -EDEADLK what to do. And that every other -errno is fatal. v16: Indent with tabs instead of space, spotted by Ander. v17: Review from Thierry, small kerneldoc and other naming polish. Cc: Thierry Reding Cc: Ander Conselvan de Oliveira Cc: Daniel Thompson Cc: Fengguang Wu Cc: Sean Paul Cc: Matt Roper Reviewed-by: Sean Paul Signed-off-by: Daniel Vetter --- Documentation/DocBook/drm.tmpl | 4 + drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_atomic.c | 600 + include/drm/drm_atomic.h | 65 + include/drm/drm_crtc.h | 50 +++- 5 files changed, 712 insertions(+), 9 deletions(-) create mode 100644 drivers/gpu/drm/drm_atomic.c create mode 100644 include/drm/drm_atomic.h diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index f83d3b6ea4e5..748513b34025 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -996,6 +996,10 @@ int max_width, max_height; !Edrivers/gpu/drm/drm_modes.c + Atomic Mode Setting Function Reference +!Edrivers/gpu/drm/drm_atomic.c + + Frame Buffer Creation struct drm_framebuffer *(*fb_create)(struct drm_device *dev, struct drm_file *file_priv, diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 9292a761ea6d..c5e37dc459ee 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -14,7 +14,7 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \ drm_info.o drm_debugfs.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_modeset_lock.o drm_atomic.o drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/
[PATCH 05/17] drm: Add atomic driver interface definitions for objects
On Wed, Nov 5, 2014 at 5:26 PM, Thierry Reding
wrote:
>> +struct drm_plane_state {
>> + struct drm_crtc *crtc;
>> + struct drm_framebuffer *fb;
>> +
>> + /* Signed dest location allows it to be partially off screen */
>> + int32_t crtc_x, crtc_y;
>> + uint32_t crtc_w, crtc_h;
>
> Should these perhaps be unsized types? For the same reasons you argued
> the other day.
>
>> +
>> + /* Source values are 16.16 fixed point */
>> + uint32_t src_x, src_y;
>> + uint32_t src_h, src_w;
>
> Do we really use the 16.16 fixed point format for these? Maybe now would
> be a good time to get rid of that if we don't need it. If they're not a
> 16.16 fixed point format they could also be unsized.
The samantics and types intentionally and precisely match what we
currently pass in through the set_plane ioctl. And yeah most drivers
can't do subpixel precise upscaling, but some hardware can do that. So
I don't see a point in changing the interface here.
I've implemented all the other stuff you've spotted.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/plane-helper: transitional atomic plane helpers
Converting a driver to the atomic interface can be a daunting
undertaking. One of the prerequisites is to have full universal planes
support.
To make that transition a bit easier this patch provides plane helpers
which use the new atomic helper callbacks just only for the plane
changes. This way the plane update functionality can be tested without
being forced to convert everything at once.
Of course a real atomic update capable driver will implement the
all plane properties through the atomic interface, so these helpers
are mostly transitional. But they can be used to enable proper
universal plane support, especially once the crtc helpers have also
been adapted.
v2: Use ->atomic_duplicate_state if available.
v3: Don't forget to call ->atomic_destroy_state if available.
v4: Fixup kerneldoc, reported by Paulo.
v5: Extract a common plane_commit helper and fix some bugs in the
plane_state setup of the plane_disable implementation.
v6: Fix issues with the cleanup of the old fb. Since transitional
helpers can be mixed we need to assume that the old fb has been set up
by a legacy path (e.g. set_config or page_flip when the primary plane
is converted to use these functions already). Hence pass an additional
old_fb parameter to plane_commit to do that cleanup work correctly.
v7:
- Fix spurious WARNING (crtc helpers really love to disable stuff
harder) and fix array index bonghits.
- Correctly handle the lack of plane->state object, necessary for
transitional use.
- Don't indicate failure if drm_vblank_get doesn't work - that's
expected when the pipe is in dpms off mode.
v8: Review from Sean:
- s/fail/out/ to make the meaning of a label more clear.
- spelling fix in the commit message.
Cc: Paulo Zanoni
Cc: Sean Paul
Reviewed-by: Sean Paul
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_plane_helper.c | 172 -
include/drm/drm_plane_helper.h | 8 ++
2 files changed, 179 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index 827ec1a3040b..a202b89e14eb 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -27,7 +27,7 @@
#include
#include
#include
-#include
+#include
#define SUBPIXEL_MASK 0x
@@ -369,3 +369,173 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc
*crtc,
return drm_crtc_init_with_planes(dev, crtc, primary, NULL, funcs);
}
EXPORT_SYMBOL(drm_crtc_init);
+
+static int
+plane_commit(struct drm_plane *plane, struct drm_plane_state *plane_state,
+struct drm_framebuffer *old_fb)
+{
+ struct drm_plane_helper_funcs *plane_funcs;
+ struct drm_crtc *crtc[2];
+ struct drm_crtc_helper_funcs *crtc_funcs[2];
+ int i, ret = 0;
+
+ plane_funcs = plane->helper_private;
+
+ /* Since this is a transitional helper we can't assume that plane->state
+* is always valid. Hence we need to use plane->crtc instead of
+* plane->state->crtc as the old crtc. */
+ crtc[0] = plane->crtc;
+ crtc[1] = crtc[0] != plane_state->crtc ? plane_state->crtc : NULL;
+
+ for (i = 0; i < 2; i++)
+ crtc_funcs[i] = crtc[i] ? crtc[i]->helper_private : NULL;
+
+ if (plane_funcs->atomic_check) {
+ ret = plane_funcs->atomic_check(plane, plane_state);
+ if (ret)
+ goto out;
+ }
+
+ if (plane_funcs->prepare_fb && plane_state->fb) {
+ ret = plane_funcs->prepare_fb(plane, plane_state->fb);
+ if (ret)
+ goto out;
+ }
+
+ /* Point of no return, commit sw state. */
+ swap(plane->state, plane_state);
+
+ for (i = 0; i < 2; i++) {
+ if (crtc_funcs[i] && crtc_funcs[i]->atomic_begin)
+ crtc_funcs[i]->atomic_begin(crtc[i]);
+ }
+
+ plane_funcs->atomic_update(plane);
+
+ for (i = 0; i < 2; i++) {
+ if (crtc_funcs[i] && crtc_funcs[i]->atomic_flush)
+ crtc_funcs[i]->atomic_flush(crtc[i]);
+ }
+
+ for (i = 0; i < 2; i++) {
+ if (!crtc[i])
+ continue;
+
+ /* There's no other way to figure out whether the crtc is
running. */
+ ret = drm_crtc_vblank_get(crtc[i]);
+ if (ret == 0) {
+ drm_crtc_wait_one_vblank(crtc[i]);
+ drm_crtc_vblank_put(crtc[i]);
+ }
+
+ ret = 0;
+ }
+
+ if (plane_funcs->cleanup_fb && old_fb)
+ plane_funcs->cleanup_fb(plane, old_fb);
+out:
+ if (plane_state) {
+ if (plane->funcs->atomic_destroy_state)
+ plane->funcs->atomic_destroy_state(plane, plane_state);
+ else
+ kfree(plane_state);
+ }
+
+ return ret;
+}
+
+/**
+ * drm_plane_helper_update() - Helper for
[PATCH 08/17] drm/plane-helper: transitional atomic plane helpers
On Wed, Nov 5, 2014 at 5:45 PM, Sean Paul wrote:
>> + /* There's no other way to figure out whether the crtc is running. */
>> + ret = drm_crtc_vblank_get(crtc[i]);
>> + if (ret == 0) {
>> + drm_crtc_wait_one_vblank(crtc[i]);
>> + drm_crtc_vblank_put(crtc[i]);
>> + }
>
> This will be good motivation for driver writers to convert to universal
> planes!
The problem here is actually dpms. Our current dpms interface sucks
badly, since from generic code it's completely impossible to figure
out whether the pipe is still on or somehow disabled due to dpms. I'm
pondering ideas and designs right now, but my current dpms plan for
atomic includes a fix for this. So with that I'd replace this hack
with crtc->state->active check and add a WARN_ON to make sure the
drm_vblank_get doesn't fail when the pipe is supposed to be on.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 05/17] drm: Add atomic driver interface definitions for objects
On Sun, Nov 02, 2014 at 02:19:18PM +0100, Daniel Vetter wrote:
[...]
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index a68e02be7e37..9847009ad451 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -224,6 +224,25 @@ struct drm_encoder;
> struct drm_pending_vblank_event;
> struct drm_plane;
> struct drm_bridge;
> +struct drm_atomic_state;
> +
> +/**
> + * struct drm_crtc_state - mutable crtc state
> + * @enable: whether the CRTC should be enabled, gates all other state
You use a mix of "crtc" and "CRTC" here. Since it's an abbreviation it
should really be "CRTC" consistently. The commit message also suffers
from this.
> + * @mode: current mode timings
> + * @event: optional pointer to a DRM event to signal upon completion of the
> + * state update
> + * @state: backpointer to global drm_atomic_state
> + */
> +struct drm_crtc_state {
> + bool enable: 1;
I thought there had been a recent thread about bitfields being
discouraged.
> @@ -288,6 +310,15 @@ struct drm_crtc_funcs {
>
> int (*set_property)(struct drm_crtc *crtc,
> struct drm_property *property, uint64_t val);
> +
> + /* atomic update handling */
> + struct drm_crtc_state *(*atomic_duplicate_state)(struct drm_crtc *crtc);
> + void (*atomic_destroy_state)(struct drm_crtc *crtc,
> + struct drm_crtc_state *cstate);
Why is the second parameter named "cstate"? Other functions use simply
"state".
> +/**
> + * struct drm_connector_state - mutable connector state
> + * @crtc: crtc to connect connector to, NULL if disabled
s/crtc/CRTC/
> + * @state: backpointer to global drm_atomic_state
> + */
> +struct drm_connector_state {
> + struct drm_crtc *crtc;
> +
> + struct drm_atomic_state *state;
> +};
>
> /**
> * struct drm_connector_funcs - control connectors on a given device
> @@ -393,6 +437,10 @@ struct drm_crtc {
> * @set_property: property for this connector may need an update
> * @destroy: make object go away
> * @force: notify the driver that the connector is forced on
> + * @atomic_duplicate_state: duplicate the atomic state for this connector
> + * @atomic_destroy_state: destroy an atomic state for this connector
> + * @atomic_set_property: set a property on an atomic state for this connector
> + *
The last line here seems gratuituous.
> +/**
> + * struct drm_plane_state - mutable plane state
> + * @crtc: currently bound CRTC, NULL if disabled
> + * @fb: currently bound fb
Nit: For consistency with the spelling for CRTC this should be FB.
> + * @crtc_x: left position of visible portion of plane on crtc
> + * @crtc_y: upper position of visible portion of plane on crtc
> + * @crtc_w: width of visible portion of plane on crtc
> + * @crtc_h: height of visible portion of plane on crtc
> + * @src_x: left position of visible portion of plane within
> + * plane (in 16.16)
> + * @src_y: upper position of visible portion of plane within
> + * plane (in 16.16)
> + * @src_w: width of visible portion of plane (in 16.16)
> + * @src_h: height of visible portion of plane (in 16.16)
> + * @state: backpointer to global drm_atomic_state
> + */
> +struct drm_plane_state {
> + struct drm_crtc *crtc;
> + struct drm_framebuffer *fb;
> +
> + /* Signed dest location allows it to be partially off screen */
> + int32_t crtc_x, crtc_y;
> + uint32_t crtc_w, crtc_h;
Should these perhaps be unsized types? For the same reasons you argued
the other day.
> +
> + /* Source values are 16.16 fixed point */
> + uint32_t src_x, src_y;
> + uint32_t src_h, src_w;
Do we really use the 16.16 fixed point format for these? Maybe now would
be a good time to get rid of that if we don't need it. If they're not a
16.16 fixed point format they could also be unsized.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/dd908b27/attachment-0001.sig>
[PATCH 04/17] drm/modeset_lock: document trylock_only in kerneldoc
On Sun, Nov 02, 2014 at 02:19:17PM +0100, Daniel Vetter wrote: > I've forgotten to do this in: > > commit cb597bb3a2fbfc871cc1c703fb330d247bd21394 > Author: Daniel Vetter > Date: Sun Jul 27 19:09:33 2014 +0200 > > drm: trylock modest locking for fbdev panics > > Oops, fix this asap. > > In my defense kerneldoc is really awful and there's no way it can pick > up structured comments per struct member. Which means we need both > since people won't scroll up even a few lines. > > Signed-off-by: Daniel Vetter > --- > include/drm/drm_modeset_lock.h | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Thierry Reding -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/f63342a9/attachment.sig>
[Intel-gfx] [PATCH 05/17] drm: Add atomic driver interface definitions for objects
On Wed, Nov 05, 2014 at 06:04:59PM +0100, Daniel Vetter wrote:
> On Wed, Nov 5, 2014 at 5:26 PM, Thierry Reding
> wrote:
> >> +struct drm_plane_state {
> >> + struct drm_crtc *crtc;
> >> + struct drm_framebuffer *fb;
> >> +
> >> + /* Signed dest location allows it to be partially off screen */
> >> + int32_t crtc_x, crtc_y;
> >> + uint32_t crtc_w, crtc_h;
> >
> > Should these perhaps be unsized types? For the same reasons you argued
> > the other day.
> >
> >> +
> >> + /* Source values are 16.16 fixed point */
> >> + uint32_t src_x, src_y;
> >> + uint32_t src_h, src_w;
> >
> > Do we really use the 16.16 fixed point format for these? Maybe now would
> > be a good time to get rid of that if we don't need it. If they're not a
> > 16.16 fixed point format they could also be unsized.
>
> The samantics and types intentionally and precisely match what we
> currently pass in through the set_plane ioctl. And yeah most drivers
> can't do subpixel precise upscaling, but some hardware can do that. So
> I don't see a point in changing the interface here.
>
> I've implemented all the other stuff you've spotted.
Just a pass by comment, it'd be awesome to have a fixed point 16.16 type
at some point so we don't have to always specify that an uint32_t
variable happens to be 16.16.
--
Damien
[RFC 1/2] core: Add generic object registry implementation
On 11/05/2014 03:04 PM, Thierry Reding wrote: > On Wed, Nov 05, 2014 at 01:36:24PM +0100, Andrzej Hajda wrote: >> On 11/04/2014 05:29 PM, Thierry Reding wrote: >>> From: Thierry Reding >>> >>> Add a generic implementation of an object registry. This targets drivers >>> and subsystems that provide auxiliary objects that other drivers need to >>> look up. The goal is to put the difficult parts (keep object references, >>> module usage count, ...) into core code so that individual subsystems do >>> not have to deal with them. >>> >>> The intention is for subsystems to instantiate a struct registry and use >>> a struct registry_record embedded into a subsystem-specific structure to >>> provide a subsystem-specific API around that. >> >> >> As I understand you want to use this registry for panels and bridges. >> Could you explain the idea and describe example scenario when these >> refcountings are useful. I guess it should be when panel attached to >> drmdrv want to disappear. > > Correct. When a panel driver is unloaded it frees memory associated with > the panel. The goal of this registry is for the panel object to stay > around until all references are gone. > >> Real lifetime of panel is limited by probe/remove callbacks of panel >> driver, do you want to prolong it behind these limits? > > Yes. > >> Do you want to have zombie panels, without hardware they abstract? For >> what purpose? > > So that display drivers don't try to access objects that have been > freed. Why do not just release panel references from drm_dev, I have successfully implemented dsi panels this way, thanks to dsi bus specific attach/detach callbacks and drm hotplug mechansim. My point is we do not need to make the whole tricky double refcounting, with total redesign of panels, revoke, zombies, etc It is enough to have just hot plug/unplug callbacks. This is why I have proposed few months ago interface_tracker framework. It can add hot(un)plug capability in a generic way to any framework. Regards Andrzej > >> What do you want to do with panel ops? Do they need support both life >> states? > > That's a slightly separate issue for which David Herrmann had a solution > in mind. As I understand it, the idea would be for these objects to gain > revoke support. The goal is that once the underlying object disappears, > calling any of the operations involved would fail (with something like a > -ENODEV). It's a little more complicated than that because the device > could disappear right in the middle of such an operation, but that's > precisely what revoke should allow us to do. Readding David for > visibility. > >> Anyway implementation currently seems to be broken, > > DRM panels are currently completely broken. If you remove the driver for > a panel the display driver that uses this panel will simply crash the > next time it tries to do anything with the panel. This type of registry > is the first step in trying to fix this. > >> you try to refcount >> objects which are usually embedded in driver priv data, which disappears >> during remove callback of the driver. > > A second step in fixing this will be to convert drivers to no longer > free the panel but rather drop their reference to the panel that they've > registered. This will make sure that memory doesn't get freed until all > references are gone. > > Note that this means that drivers will also need to be converted not to > use devm_* helpers since that conflicts with the reference counted life- > times of these record objects. > > Thierry > > > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
[Bug 76130] Radeon HD 4570 set dpm state fails after suspend
https://bugs.freedesktop.org/show_bug.cgi?id=76130 --- Comment #6 from Alex Deucher --- Created attachment 108972 --> https://bugs.freedesktop.org/attachment.cgi?id=108972&action=edit possible fix Does this patch help? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/c7200ca8/attachment.html>
[PATCH 3/7] drm/doc: Add GEM/CMA helpers to kerneldoc
On Wed, Nov 05, 2014 at 04:04:55PM +0100, Daniel Vetter wrote: > On Wed, Nov 05, 2014 at 04:01:26PM +0100, Thierry Reding wrote: > > On Wed, Nov 05, 2014 at 03:34:40PM +0100, Daniel Vetter wrote: > > > On Wed, Nov 05, 2014 at 02:25:15PM +0100, Thierry Reding wrote: > > > > + * Return: A struct drm_gem_cma_object * on success or an > > > > ERR_PTR()-encoded > > > > > > Same bikeshed about "Returns:\n" as with the panel kerneldoc patch. > > > > I've been following the style described in the kernel-doc nano-HOWTO, > > which says that: > > > > The return value, if any, should be described in a dedicated > > section named "Return". > > > > There are other things in that document that we don't follow in DRM, so > > I wonder if we should just consider it as guidelines rather than actual > > rules (they aren't enforced anyway) or perhaps make a pass over existing > > kerneldoc and convert it to the rules described in that document. > > > > That document is the only reference for the kerneldoc syntax (that I > > know of), so I had always thought that we should be following it. > > We've started out with all-uppercase RETURNS from userspace libdrm iirc. I > don't care really, but iirc the Returns: is the common one we use. Imo it > also reads better in English, but not native speaker here ;-) Alright, I'll go with the variant that you proposed for the sake of consistency. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/fd7d4352/attachment.sig>
[PATCH 3/7] drm/doc: Add GEM/CMA helpers to kerneldoc
On Wed, Nov 05, 2014 at 04:01:26PM +0100, Thierry Reding wrote: > On Wed, Nov 05, 2014 at 03:34:40PM +0100, Daniel Vetter wrote: > > On Wed, Nov 05, 2014 at 02:25:15PM +0100, Thierry Reding wrote: > > > + * Return: A struct drm_gem_cma_object * on success or an > > > ERR_PTR()-encoded > > > > Same bikeshed about "Returns:\n" as with the panel kerneldoc patch. > > I've been following the style described in the kernel-doc nano-HOWTO, > which says that: > > The return value, if any, should be described in a dedicated > section named "Return". > > There are other things in that document that we don't follow in DRM, so > I wonder if we should just consider it as guidelines rather than actual > rules (they aren't enforced anyway) or perhaps make a pass over existing > kerneldoc and convert it to the rules described in that document. > > That document is the only reference for the kerneldoc syntax (that I > know of), so I had always thought that we should be following it. We've started out with all-uppercase RETURNS from userspace libdrm iirc. I don't care really, but iirc the Returns: is the common one we use. Imo it also reads better in English, but not native speaker here ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 0/7] drm: Sanitize DRM_IOCTL_MODE_CREATE_DUMB input
On Wed, Nov 05, 2014 at 02:24:42PM +, Russell King - ARM Linux wrote: > On Wed, Nov 05, 2014 at 02:25:12PM +0100, Thierry Reding wrote: > > Discussion on IRC lead to the conclusion that new IOCTLs should have > > input validation and require userspace to zero out output parameters to > > avoid this kind of mess in the future. In order to help avoid this kind > > of ambiguity it would be a good idea to start documenting IOCTLs more > > officially (e.g. in the DRM DocBook). > > That is a good point, however not everyone can build the documentation. > I tried a while back and the whole documentation subsystem is /soo/ > fragile its untrue. You have to have the exact right versions installed > and hope that the exact right versions have configured themselves to > work together correctly, otherwise you get cryptic unsolvable error > messages. Tex is notorious for these problems. > > When I tried to generate the docbook stuff, after spending a significant > amount of time trying to debug it, I just gave up completely with any > ideas about ever adding to the documentation. I've since decided that > I am /never/ going to do anything past adding docbook comments to files. > > If someone cares about linking them into the rest of the docbook system, > and they have the capability to generate the docbook output, then that's > fine and dandy, but I believe that requiring everyone to have that > capability is asking far too much. Ime all the kerneldoc and tex stuff is hopeless. The only thing I use is $ make htmldocs That works somewhat after a bit of wrestling. But note that it doesn't handle file renames gracefully and make clean won't get you out of that fuzz. So my build-docs script also has a $ git clean -dfx Documentation/DocBook in there. Not pretty I agree. Add to that that kerneldoc is a feature poor markup language (no lists, hyperlinks, chapters or anything really). I'm trying really hard to get someone at intel (or paid by intel) to fix this mess up though. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 3/7] drm/doc: Add GEM/CMA helpers to kerneldoc
ce-specific quirks. > > -/* > > - * drm_gem_cma_mmap - (struct file_operation)->mmap callback function > > +/** > > + * drm_gem_cma_mmap - memory-map a CMA GEM object > > + * @filp: file object > > + * @vma: VMA for the area to be mapped > > Imo the real value of kerneldoc is the blabla here that describes what > this should be used for and when. E.g. here > > "This function implements an augmented version of the gem DRM file mmap > operation for cma objects: In addition to the usual gem vma setup it > immediately faults in the entire object instead of using on-demand > faulting. Drivers which employ the cma helpers should use this function as > their ->mmap handler for the DRM device file's file_operation structure." I've taken the easy route and simply copied your example. > So requires grepping each function and hunting for the usual pattern (and > noticing tons of crazy stuff, usually). > > Same for all the functions below. Okay, I'll see what I can come up with. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/13ffdda2/attachment.sig>
[lm-sensors] Kaveri [and Radeon] temps
On Wed, Nov 05, 2014 at 06:34:20PM -0500, James Cloos wrote: > > "GR" == Guenter Roeck writes: > > ... > GR> AMD CPUs are known report wildly off temperatures especially at > GR> low temperatures. > > Thanks. After some further digging, I wonder whether Aravind's recent > patch might help? The apu does have both PCI_DEVICE_ID_AMD_16H_M30H_NB_F3 > and PCI_DEVICE_ID_AMD_16H_M30H_NB_F4 and lspci reports no driver for the > F4 device. > Aravind would have to comment; my understanding is that the F4 device reports the power, not the temperature. You could try to apply the patch and see if it makes a difference. Worst case there will be no change ;-). > GR> Does the temperature increase with load ? > > It seems to max out w/in a kelvin or two of the room's ambient temp. > Even under load ? Sounds weird. Question of course is if the chip really reports a useful temperature in the first place; this is something I can not answer, since I have neither a datasheet nor a system with that CPU. Guenter
[PATCH 0/7] drm: Sanitize DRM_IOCTL_MODE_CREATE_DUMB input
On Wed, Nov 05, 2014 at 02:24:42PM +, Russell King - ARM Linux wrote: > On Wed, Nov 05, 2014 at 02:25:12PM +0100, Thierry Reding wrote: > > Discussion on IRC lead to the conclusion that new IOCTLs should have > > input validation and require userspace to zero out output parameters to > > avoid this kind of mess in the future. In order to help avoid this kind > > of ambiguity it would be a good idea to start documenting IOCTLs more > > officially (e.g. in the DRM DocBook). > > That is a good point, however not everyone can build the documentation. > I tried a while back and the whole documentation subsystem is /soo/ > fragile its untrue. You have to have the exact right versions installed > and hope that the exact right versions have configured themselves to > work together correctly, otherwise you get cryptic unsolvable error > messages. Tex is notorious for these problems. I usually only build the HTML documentation which tends to be less prone to the issues that you're describing. > When I tried to generate the docbook stuff, after spending a significant > amount of time trying to debug it, I just gave up completely with any > ideas about ever adding to the documentation. I've since decided that > I am /never/ going to do anything past adding docbook comments to files. > > If someone cares about linking them into the rest of the docbook system, > and they have the capability to generate the docbook output, then that's > fine and dandy, but I believe that requiring everyone to have that > capability is asking far too much. It looks as if perhaps running the xmldocs target would at least run kerneldoc on the files even if it doesn't produce anything very human readable. If that works for you it'd at least give you a means to check that the kerneldoc comments you've added are consistent. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/a246e612/attachment.sig>
[PATCH 7/7] drm: Sanitize DRM_IOCTL_MODE_CREATE_DUMB input
On Wed, Nov 05, 2014 at 02:25:19PM +0100, Thierry Reding wrote: > From: Thierry Reding > > Some drivers treat the pitch and size fields as inputs and will use them > as minima provided by userspace so that they are only overwritten if the > minimal requirements of the driver exceed them. > > This can cause strange behaviour when applications don't zero out these > fields, causing whatever was on the stack to be passed to the IOCTL. In > a typical case this would become visible as a failed allocation if the > pitch or size were unusually high. But this could also cause more subtle > bugs like overallocating dumb framebuffers. > > To prevent drivers from misusing these values, make the DRM core zero > out the pitch and size fields before passing the structure to the driver > implementation. > > While at it, also set the output handle field to zero for good measure, > even though it's less likely to be abused. > > Signed-off-by: Thierry Reding > --- > drivers/gpu/drm/drm_crtc.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 0f3c24c0981b..6aceb689ccea 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -4755,6 +4755,14 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev, > if (PAGE_ALIGN(size) == 0) > return -EINVAL; > > + /* > + * handle, pitch and size are output parameters. Zero them out to > + * prevent drivers from accidentally using uninitialized data. Maybe add: Unfortunately we can't reject ioctls with garbage in them since existing userspace is not clearing these fields properly. With that comment: Reviewed-by: Daniel Vetter That way it's clear that we can never reuse these fields for flags or anything at all. Also a good reminder for folks that they really should have if (args->foo) return -EINVAL for any reserved, unused or output-only fields. -Daniel > + */ > + args->handle = 0; > + args->pitch = 0; > + args->size = 0; > + > return dev->driver->dumb_create(file_priv, dev, args); > } > > -- > 2.1.3 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 6/7] drm/rcar: gem: dumb: pitch is an output
On Wed, Nov 05, 2014 at 02:25:18PM +0100, Thierry Reding wrote: > From: Thierry Reding > > When creating a dumb buffer object using the DRM_IOCTL_MODE_CREATE_DUMB > IOCTL, only the width, height, bpp and flags fields are inputs. The > caller is not guaranteed to zero out or set handle, pitch and size. > Drivers must not treat these values as possible inputs, otherwise they > may use uninitialized memory during the computation of the framebuffer > size. > > The R-Car DU driver treats the pitch passed in from userspace as minimum > and will only overwrite it when the driver-computed pitch is larger, > allowing userspace to, intentionally or not, overallocate framebuffers. > > Signed-off-by: Thierry Reding Reviewed-by: Daniel Vetter Aside: I tend to put the maintainer Cc: into the patch so that they're on the permanent record. That will also make sure that when this patch gets resend (e.g. stable backport) maintainers are still included properly. -Daniel > --- > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c > b/drivers/gpu/drm/rcar-du/rcar_du_kms.c > index 5329491e32c3..6289e3797bc5 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c > @@ -126,7 +126,7 @@ int rcar_du_dumb_create(struct drm_file *file, struct > drm_device *dev, > else > align = 16 * args->bpp / 8; > > - args->pitch = roundup(max(args->pitch, min_pitch), align); > + args->pitch = roundup(min_pitch, align); > > return drm_gem_cma_dumb_create_internal(file, dev, args); > } > -- > 2.1.3 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH v4 16/16] drm/panel: Add Sharp LQ101R1SX01 support
On 11/04/2014 02:29 PM, Thierry Reding wrote:
> On Tue, Nov 04, 2014 at 11:41:15AM +0100, Andrzej Hajda wrote:
>> On 11/03/2014 10:13 AM, Thierry Reding wrote:
> [...]
>>> diff --git a/Documentation/devicetree/bindings/panel/sharp,lq101r1sx01.txt
>>> b/Documentation/devicetree/bindings/panel/sharp,lq101r1sx01.txt
> [...]
>>> +Example:
>>> +
>>> + dsi at 5430 {
>>> + panel: panel at 0 {
>>> + compatible = "sharp,lq101r1sx01";
>>> + reg = <0>;
>>> +
>>> + link2 = <&secondary>;
>>> +
>>> + power-supply = <...>;
>>> + backlight = <...>;
>>> + };
>>> + };
>>> +
>>> + dsi at 5440 {
>>> + secondary: panel at 0 {
>>> + compatible = "sharp,lq101r1sx01";
>>> + reg = <0>;
>>> + };
>>> + };
>>
>> The bindings above and the implementation below clearly shows
>> that the secondary node is just a dummy dsi device.
>
> It's not only a dummy device. Binding it to the device's compatible
> string allows the driver to properly set up the DSI device (flags,
> number of lanes, ...).
This is why I called it dummy DSI device :)
>
>> Hiding this behind conditionals in both docs and code does not look good
>> to me. On the other side having common dummy dsi driver
>> would allow to reuse it in other dual-dsi devices.
>> So instead of 2nd node you would have:
>> dsi at 5440 {
>> secondary: panel at 0 {
>> compatible = "dsi-dummy-whatever";
>> reg = <0>;
>> };
>> };
>>
>> Or just:
>> dsi2: dsi at 5440 {
>> }
>>
>> with phandle to dsi2 in 1st node, in such case 2nd dsi dev would be
>> created dynamically like with i2c_new_dummy.
>
> I don't think that's technically valid DT. Every device must have a
> compatible property. And the compatible property should have an entry
> for the specific model of the device. "dummy" doesn't qualify for that
One could ask if it is technically valid to represent one hw device via
two separate nodes. Each choice scarifies something.
>
> Hopefully when more dual-channel DSI devices get supported some kind of
> pattern will emerge. For now I'll go with what I have in this patch. It
> is as accurate as I can think of.
Agreed
>
>>> diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
>>> b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> [...]
>>> +struct sharp_panel {
>>> + struct drm_panel base;
>>> + /* the datasheet refers to them as DSI-LINK1 and DSI-LINK2 */
>>> + struct mipi_dsi_device *link1;
>>> + struct mipi_dsi_device *link2;
>>> +
>>> + struct backlight_device *backlight;
>>> + struct regulator *supply;
>>> +
>>> + bool prepared;
>>> + bool enabled;
>>
>> Nitpick.
>> As I wrote in review to previous version it is up to panel client
>> (usually connector) to call panel callbacks properly, we should not add
>> additional checks to panels. If call order is incorrect then the client
>> should be fixed.
>
> There are no such guarantees today. But hopefully this will change with
> atomic modesetting.
I do not understand that. It is the connector code who calls these
callbacks. Why cannot you force it to call them in proper order?
>
>>> +static int sharp_panel_remove(struct mipi_dsi_device *dsi)
>>> +{
>>> + struct sharp_panel *sharp = mipi_dsi_get_drvdata(dsi);
>>> + int err;
>>> +
>>> + /* only detach from host for the DSI-LINK2 interface */
>>> + if (!sharp) {
>>> + mipi_dsi_detach(dsi);
>>
>> Quotation from previous review:
>>
There is no locking mechanism here, so it is possible that
everything can crash if someone unbind driver from LINK2 and then try to
enable the panel.
>>>
>>> I don't think so. Since we're not doing anything with the DSI-LINK2
>>> device anymore it's irrelevant whether it is bound to the driver or
>>> not.
>>>
>>
>> Please consider following scenario:
>> 1. panel link2 is probed
>> 2. panel link1 is probed
>> 3. panel link2 is unbound (for example by: echo link2
>>> /sys/bus/dsi/drivers/*lq101*/unbind)
>> 4. drm is bound:
>>- during sharp_setup_symmetrical_split you will call transfer
>> but there is no device attached to dsi2.
>>
>> Maybe it will not cause any troubles but it seems incorrect.
>
> The device is still there. The driver may not be bound to it, but we
> don't need that for the transfers anyway.
And you do not want to call it dummy device :)
>
>> I guess simple workaround is to do device_lock and check if device is
>> bound every time you want to access link2 device.
>
> I don't see where the problem is. We do keep a reference to the LINK2
> device from the first, so it can't just disappear. The only way it could
> disappear is if the host controller that provides its bus goes away, but
> there's code at the DSI host controller level to deal with that.
>
As I understand it is important to you that ho
[PATCH 5/7] drm/omap: gem: dumb: pitch is an output
On Wed, Nov 05, 2014 at 02:25:17PM +0100, Thierry Reding wrote:
> From: Thierry Reding
>
> When creating a dumb buffer object using the DRM_IOCTL_MODE_CREATE_DUMB
> IOCTL, only the width, height, bpp and flags fields are inputs. The
> caller is not guaranteed to zero out or set handle, pitch and size.
> Drivers must not treat these values as possible inputs, otherwise they
> may use uninitialized memory during the computation of the framebuffer
> size.
>
> The OMAP driver uses the pitch field passed in by userspace as a minimum
> and only override it if the driver-computed pitch is larger than what
> userspace provided. To prevent this from causing overallocation, fix the
> minimum pitch to 0 to enforce the driver-computed pitch.
>
> Signed-off-by: Thierry Reding
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/omapdrm/omap_gem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
> b/drivers/gpu/drm/omapdrm/omap_gem.c
> index e4849413ee80..bff60b73995b 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -613,7 +613,7 @@ int omap_gem_dumb_create(struct drm_file *file, struct
> drm_device *dev,
> union omap_gem_size gsize;
>
> /* in case someone tries to feed us a completely bogus stride: */
> - args->pitch = align_pitch(args->pitch, args->width, args->bpp);
> + args->pitch = align_pitch(0, args->width, args->bpp);
> args->size = PAGE_ALIGN(args->pitch * args->height);
>
> gsize = (union omap_gem_size){
> --
> 2.1.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
exynos-drm registration: infinite loop
On Tue, Nov 04, 2014 at 09:18:46PM +0400, Matwey V. Kornilov wrote: > Hi, > > I run 3.18-rc3 kernel on BeagleBone Black. It doesn't have Exynos DRM > of course, but I run multi-platform kernel where CONFIG_DRM_EXYNOS is > set to 'y'. > The issue here is that the platform probe/init goes to infinite loop > as the following: > > [5.717343] platform exynos-drm: Driver exynos-drm requests probe deferral > [5.726848] exynos-drm-ipp exynos-drm-ipp: drm ipp registered successfully. > [5.734700] platform exynos-drm: Driver exynos-drm requests probe deferral > [5.744044] exynos-drm-ipp exynos-drm-ipp: drm ipp registered successfully. > [5.752010] platform exynos-drm: Driver exynos-drm requests probe deferral > [5.761377] exynos-drm-ipp exynos-drm-ipp: drm ipp registered successfully. > [5.769291] platform exynos-drm: Driver exynos-drm requests probe deferral > > It is quite unexpectable behavior. I would expect that the exynos-drm > failed to initialize with 'no device' error. > > See also for the reference: https://bugzilla.kernel.org/show_bug.cgi?id=87691 The reason for this seems to be that Exynos DRM instantiates a dummy device that it can bind to (see exynos_drm_init()). Now this code is executed unconditonally, so the device will be created whether or not the kernel/module runs on an Exynos SoC. The driver should really bind to some real device rather than instantiating a dummy. Or if it must bind to a dummy then the code needs to at least check that it's running on an Exynos SoC before instantiating the dummy. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/dfa3a341/attachment.sig>
[PATCH 4/7] drm/cma: Introduce drm_gem_cma_dumb_create_internal()
On Wed, Nov 05, 2014 at 02:25:16PM +0100, Thierry Reding wrote:
> From: Thierry Reding
>
> This function is similar to drm_gem_cma_dumb_create() but targetted at
> kernel internal users so that they can override the pitch and size
> requirements of the dumb buffer.
>
> It is important to make this difference because the IOCTL says that the
> pitch and size fields are to be considered outputs and therefore should
> not be used in computations of the framebuffer size. Internal users may
> still want to use this code to avoid duplication and at the same time
> pass on additional, driver-specific restrictions on the pitch and size.
>
> While at it, convert the R-Car DU driver, the single user that overrides
> the pitch, to use the new internal helper.
>
> Signed-off-by: Thierry Reding
Some comments about the kerneldoc, with that address (needs the alignment
with the previous patch essentially) this is
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_gem_cma_helper.c | 33 -
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
> include/drm/drm_gem_cma_helper.h | 5 +
> 3 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> b/drivers/gpu/drm/drm_gem_cma_helper.c
> index 55306be1f8f7..c55986566f0c 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -189,7 +189,7 @@ void drm_gem_cma_free_object(struct drm_gem_object
> *gem_obj)
> EXPORT_SYMBOL_GPL(drm_gem_cma_free_object);
>
> /**
> - * drm_gem_cma_dumb_create - create a dumb buffer object
> + * drm_gem_cma_dumb_create_internal - create a dumb buffer object
> * @file_priv: DRM file-private structure to create the dumb buffer for
> * @drm: DRM device
> * @args: IOCTL data
> @@ -199,12 +199,12 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_free_object);
Kerneldoc should be update to mention what exactly this is useful for, and
how it differs from the drm_gem_cma_dumb_create().
> *
> * Return: 0 on success or a negative error code on failure.
> */
> -int drm_gem_cma_dumb_create(struct drm_file *file_priv,
> - struct drm_device *drm,
> - struct drm_mode_create_dumb *args)
> +int drm_gem_cma_dumb_create_internal(struct drm_file *file_priv,
> + struct drm_device *drm,
> + struct drm_mode_create_dumb *args)
> {
> + unsigned int min_pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> struct drm_gem_cma_object *cma_obj;
> - int min_pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
>
> if (args->pitch < min_pitch)
> args->pitch = min_pitch;
> @@ -216,6 +216,29 @@ int drm_gem_cma_dumb_create(struct drm_file *file_priv,
>&args->handle);
> return PTR_ERR_OR_ZERO(cma_obj);
> }
> +EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create_internal);
> +
> +/**
> + * drm_gem_cma_dumb_create - create a dumb buffer object
> + * @file_priv: DRM file-private structure to create the dumb buffer for
> + * @drm: DRM device
> + * @args: IOCTL data
Same here (and raised in the previous kerneldoc patch already), this needs
some comments on what it's useful for and what's the difference with the
_internal variant.
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int drm_gem_cma_dumb_create(struct drm_file *file_priv,
> + struct drm_device *drm,
> + struct drm_mode_create_dumb *args)
> +{
> + struct drm_gem_cma_object *cma_obj;
> +
> + args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> + args->size = args->pitch * args->height;
> +
> + cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size,
> + &args->handle);
> + return PTR_ERR_OR_ZERO(cma_obj);
> +}
> EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create);
>
> /**
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index 6c24ad7d03ef..5329491e32c3 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -128,7 +128,7 @@ int rcar_du_dumb_create(struct drm_file *file, struct
> drm_device *dev,
>
> args->pitch = roundup(max(args->pitch, min_pitch), align);
>
> - return drm_gem_cma_dumb_create(file, dev, args);
> + return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
>
> static struct drm_framebuffer *
> diff --git a/include/drm/drm_gem_cma_helper.h
> b/include/drm/drm_gem_cma_helper.h
> index 873d4eb7f125..acd6af8a8e67 100644
> --- a/include/drm/drm_gem_cma_helper.h
> +++ b/include/drm/drm_gem_cma_helper.h
> @@ -30,6 +30,11 @@ to_drm_gem_cma_obj(struct drm_gem_object *gem_obj)
> void drm_gem_cma_free_object(struct drm_gem_object *gem_obj);
>
> /* create memory region for DRM framebuffer */
> +int drm_gem_cma_dumb_
[PATCH 3/7] drm/doc: Add GEM/CMA helpers to kerneldoc
On Wed, Nov 05, 2014 at 02:25:15PM +0100, Thierry Reding wrote:
> From: Thierry Reding
>
> Most of the functions already have the beginnings of kerneldoc comments
> but are using the wrong opening marker. Use the correct opening marker
> and flesh out the comments so that they can be integrated with the DRM
> DocBook document.
>
> Signed-off-by: Thierry Reding
Bunch of comments inline below.
-Daniel
> ---
> Documentation/DocBook/drm.tmpl | 6 ++
> drivers/gpu/drm/drm_gem_cma_helper.c | 155
> ++-
> include/drm/drm_gem_cma_helper.h | 25 --
> 3 files changed, 139 insertions(+), 47 deletions(-)
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 18025496a736..666a210af121 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -963,6 +963,12 @@ struct drm_gem_object * (*gem_prime_import)(struct
> drm_device *dev,
> !Edrivers/gpu/drm/drm_mm.c
> !Iinclude/drm/drm_mm.h
>
> +
> + CMA Helper Functions Reference
> +!Pdrivers/gpu/drm/drm_gem_cma_helper.c cma helpers
> +!Edrivers/gpu/drm/drm_gem_cma_helper.c
> +!Iinclude/drm/drm_gem_cma_helper.h
> +
>
>
>
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> b/drivers/gpu/drm/drm_gem_cma_helper.c
> index 0316310e2cc4..55306be1f8f7 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -29,18 +29,30 @@
> #include
> #include
>
> -/*
> +/**
> + * DOC: cma helpers
> + *
> + * The Contiguous Memory Allocator reserves a pool of memory at early boot
> + * that is used to service requests for large blocks of contiguous memory.
> + *
> + * The DRM GEM/CMA helpers use this allocator as a means to provide buffer
> + * objects that are physically contiguous in memory. This is useful for
> + * display drivers that are unable to map scattered buffers via an IOMMU.
> + */
> +
> +/**
> * __drm_gem_cma_create - Create a GEM CMA object without allocating memory
> - * @drm: The drm device
> - * @size: The GEM object size
> + * @drm: DRM device
> + * @size: size of the object to allocate
> *
> - * This function creates and initializes a GEM CMA object of the given size,
> but
> - * doesn't allocate any memory to back the object.
> + * This function creates and initializes a GEM CMA object of the given size,
> + * but doesn't allocate any memory to back the object.
> *
> - * Return a struct drm_gem_cma_object* on success or ERR_PTR values on
> failure.
> + * Return: A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded
Same bikeshed about "Returns:\n" as with the panel kerneldoc patch.
> + * negative error code on failure.
> */
> static struct drm_gem_cma_object *
> -__drm_gem_cma_create(struct drm_device *drm, unsigned int size)
> +__drm_gem_cma_create(struct drm_device *drm, size_t size)
> {
> struct drm_gem_cma_object *cma_obj;
> struct drm_gem_object *gem_obj;
> @@ -69,14 +81,16 @@ error:
> return ERR_PTR(ret);
> }
>
> -/*
> +/**
> * drm_gem_cma_create - allocate an object with the given size
> + * @drm: DRM device
> + * @size: size of the object to allocate
> *
> - * returns a struct drm_gem_cma_object* on success or ERR_PTR values
> - * on failure.
> + * Return: A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded
> + * negative error code on failure.
> */
> struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> - unsigned int size)
> + size_t size)
> {
> struct drm_gem_cma_object *cma_obj;
> int ret;
> @@ -104,17 +118,21 @@ error:
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_create);
>
> -/*
> - * drm_gem_cma_create_with_handle - allocate an object with the given
> - * size and create a gem handle on it
> +/**
> + * drm_gem_cma_create_with_handle - allocate an object with the given size
> and
> + * return a GEM handle to it
> + * @file_priv: DRM file-private structure to register the handle for
> + * @drm: DRM device
> + * @size: size of the object to allocate
> + * @handle: return location for the GEM handle
> *
> - * returns a struct drm_gem_cma_object* on success or ERR_PTR values
> - * on failure.
> + * Return: A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded
> + * negative error code on failure.
> */
> -static struct drm_gem_cma_object *drm_gem_cma_create_with_handle(
> - struct drm_file *file_priv,
> - struct drm_device *drm, unsigned int size,
> - unsigned int *handle)
> +static struct drm_gem_cma_object *
> +drm_gem_cma_create_with_handle(struct drm_file *file_priv,
> +struct drm_device *drm, size_t size,
> +uint32_t *handle)
> {
> struct drm_gem_cma_object *cma_obj;
> struct drm_gem_object *gem_obj;
> @@ -145,9 +163,9 @@ err_handle_create:
> return ERR_PTR(ret);
> }
>
> -/*
> - * dr
[PATCH] drm: Global atomic state handling
Some differences compared to Rob's patches again: - Dropped the committed and checked booleans. Checking will be internally enforced by always calling ->atomic_check before ->atomic_commit. And async handling needs to be solved differently because the current scheme completely side-steps ww mutex deadlock avoidance (and so either reinvents a new deadlock avoidance wheel or like the current code just deadlocks). - State for connectors needed to be added, since now they have a full-blown drm_connector_state (so that drivers have something to attach their own stuff to). - Refcounting is gone. I plane to solve async updates differently, since the lock-passing scheme doesn't cut it (since it abuses ww mutexes). Essentially what we need for async is a simple ownership transfer from the caller to the driver. That doesn't need full-blown refcounting. - The acquire ctx is a pointer. Real atomic callers should have that on their stack, legacy entry points need to put the right one (obtained by drm_modeset_legacy_acuire_ctx) in there. - I've dropped all hooks except check/commit. All the begin/end handling is done by core functions and is the same. - commit/check are just thin wrappers that ensure that ->check is always called. - To help out with locking in the legacy implementations I've added a helper to just grab all locks in the backoff case. v2: Add notices that check/commit can fail with EDEADLK. v3: - More consistent naming for state_alloc. - Add state_clear which is needed for backoff and retry. v4: Planes/connectors can switch between crtcs, and we need to be careful that we grab the state (and locks) for both the old and new crtc. Improve the interface functions to ensure this. v5: Add functions to grab affected connectors for a crtc and to recompute the crtc->enable state. This is useful for both helper and atomic ioctl code when e.g. removing a connector. v6: Squash in fixup from Fengguang to use ERR_CAST. v7: Add debug output. v8: Make checkpatch happy about kcalloc argument ordering. v9: Improve kerneldoc in drm_crtc.h v10: - Fix another kcalloc argument misorder I've missed. - More polish for kerneldoc. v11: Clarify the ownership rules for the state object. The new rule is that a successful drm_atomic_commit (whether synchronous or asnyc) always inherits the state and is responsible for the clean-up. That way async and sync ->commit functions are more similar. v12: A few bugfixes: - Assign state->state pointers correctly when grabbing state objects - we need to link them up with the global state. - Handle a NULL crtc in set_crtc_for_plane to simplify code flow a bit for the callers of this function. v13: Review from Sean: - kerneldoc spelling fixes - Don't overallocate states->planes. - Handle NULL crtc in set_crtc_for_connector. v14: Sprinkle __must_check over all functions which do wait/wound locking to make sure callers don't forget this. Since I have ;-) v15: Be more explicit in the kerneldoc when functions can return -EDEADLK what to do. And that every other -errno is fatal. v20: Indent with tabs instead of space, spotted by Ander. Cc: Ander Conselvan de Oliveira Cc: Daniel Thompson Cc: Fengguang Wu Cc: Sean Paul Cc: Matt Roper Reviewed-by: Sean Paul Signed-off-by: Daniel Vetter --- Documentation/DocBook/drm.tmpl | 4 + drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_atomic.c | 600 + include/drm/drm_atomic.h | 65 + include/drm/drm_crtc.h | 35 +++ 5 files changed, 705 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_atomic.c create mode 100644 include/drm/drm_atomic.h diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index f83d3b6ea4e5..748513b34025 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -996,6 +996,10 @@ int max_width, max_height; !Edrivers/gpu/drm/drm_modes.c + Atomic Mode Setting Function Reference +!Edrivers/gpu/drm/drm_atomic.c + + Frame Buffer Creation struct drm_framebuffer *(*fb_create)(struct drm_device *dev, struct drm_file *file_priv, diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 9292a761ea6d..c5e37dc459ee 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -14,7 +14,7 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \ drm_info.o drm_debugfs.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_modeset_lock.o drm_atomic.o drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c new file mode 100644 index ..ad15a88c0f74 --- /dev/null +++ b
[lm-sensors] Kaveri [and Radeon] temps
On Wed, Nov 05, 2014 at 04:15:54PM -0500, James Cloos wrote: > I'm getting odd temp readings on an A8-7600 with 3.3.4 and Linus' > current kernel. > > Eg, the gpu temp shows up when (mostly) idle as: > > radeon-pci-0008 > Adapter: PCI adapter > temp1: -1.0°C (crit = +120.0°C, hyst = +90.0°C) > The driver for the Radeon chips is maintained by the radeon driver maintainers, so you might want to ask at dri-devel at lists.freedesktop.org (copied with this reply). The driver supports different chips with different methods to read the temperature, so it might help to know which chip is in your system (lspci -nn should tell you). > aka: > > radeon-pci-0008 > Adapter: PCI adapter > temp1:+32.0°F (crit = +248.0°F, hyst = +194.0°F) > > > And the cpu temp is similar: > > k10temp-pci-00c3 > Adapter: PCI adapter > temp1: +0.0°C (high = +70.0°C) > (crit = +80.0°C, hyst = +79.0°C) > aka: > > k10temp-pci-00c3 > Adapter: PCI adapter > temp1:+32.0°F (high = +158.0°F) > (crit = +176.0°F, hyst = +174.2°F) > AMD CPUs are known report wildly off temperatures especially at low temperatures. Does the temperature increase with load ? Thanks, Guenter
[PATCH 2/7] drm/doc: mm: Fix indentation
On Wed, Nov 05, 2014 at 02:25:14PM +0100, Thierry Reding wrote:
> From: Thierry Reding
>
> Use spaces consistently for indentation in the memory-management
> section.
>
> Signed-off-by: Thierry Reding
Acked-by: Daniel Vetter
> ---
> Documentation/DocBook/drm.tmpl | 268
> -
> 1 file changed, 134 insertions(+), 134 deletions(-)
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index f6a9d7b21380..18025496a736 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -492,10 +492,10 @@ char *date;
>
>The Translation Table Manager (TTM)
>
> - TTM design background and information belongs here.
> +TTM design background and information belongs here.
>
>
> - TTM initialization
> +TTM initialization
> This section is outdated.
>
>Drivers wishing to support TTM must fill out a drm_bo_driver
> @@ -503,42 +503,42 @@ char *date;
>pointers for initializing the TTM, allocating and freeing memory,
>waiting for command completion and fence synchronization, and
> memory
>migration. See the radeon_ttm.c file for an example of usage.
> -
> -
> - The ttm_global_reference structure is made up of several fields:
> -
> -
> - struct ttm_global_reference {
> - enum ttm_global_types global_type;
> - size_t size;
> - void *object;
> - int (*init) (struct ttm_global_reference *);
> - void (*release) (struct ttm_global_reference *);
> - };
> -
> -
> - There should be one global reference structure for your memory
> - manager as a whole, and there will be others for each object
> - created by the memory manager at runtime. Your global TTM should
> - have a type of TTM_GLOBAL_TTM_MEM. The size field for the global
> - object should be sizeof(struct ttm_mem_global), and the init and
> - release hooks should point at your driver-specific init and
> - release routines, which probably eventually call
> - ttm_mem_global_init and ttm_mem_global_release, respectively.
> -
> -
> - Once your global TTM accounting structure is set up and initialized
> - by calling ttm_global_item_ref() on it,
> - you need to create a buffer object TTM to
> - provide a pool for buffer object allocation by clients and the
> - kernel itself. The type of this object should be TTM_GLOBAL_TTM_BO,
> - and its size should be sizeof(struct ttm_bo_global). Again,
> - driver-specific init and release functions may be provided,
> - likely eventually calling ttm_bo_global_init() and
> - ttm_bo_global_release(), respectively. Also, like the previous
> - object, ttm_global_item_ref() is used to create an initial reference
> - count for the TTM, which will call your initialization function.
> -
> +
> +
> + The ttm_global_reference structure is made up of several fields:
> +
> +
> + struct ttm_global_reference {
> + enum ttm_global_types global_type;
> + size_t size;
> + void *object;
> + int (*init) (struct ttm_global_reference *);
> + void (*release) (struct ttm_global_reference *);
> + };
> +
> +
> + There should be one global reference structure for your memory
> + manager as a whole, and there will be others for each object
> + created by the memory manager at runtime. Your global TTM should
> + have a type of TTM_GLOBAL_TTM_MEM. The size field for the global
> + object should be sizeof(struct ttm_mem_global), and the init and
> + release hooks should point at your driver-specific init and
> + release routines, which probably eventually call
> + ttm_mem_global_init and ttm_mem_global_release, respectively.
> +
> +
> + Once your global TTM accounting structure is set up and initialized
> + by calling ttm_global_item_ref() on it,
> + you need to create a buffer object TTM to
> + provide a pool for buffer object allocation by clients and the
> + kernel itself. The type of this object should be
> TTM_GLOBAL_TTM_BO,
> + and its size should be sizeof(struct ttm_bo_global). Again,
> + driver-specific init and release functions may be provided,
> + likely eventually calling ttm_bo_global_init() and
> + ttm_bo_global_release(), respectively. Also, like the previous
> + object, ttm_global_item_ref() is used to create an initial
> reference
> + count for the TTM, which will call your initialization function.
> +
>
>
>
> @@ -566,19 +5
[PATCH 1/7] drm/gem: Fix a few kerneldoc typos
On Wed, Nov 05, 2014 at 02:25:13PM +0100, Thierry Reding wrote:
> From: Thierry Reding
>
> While at it, adjust the drm_gem_handle_create() function declaration to
> be more consistent with other functions in the file.
>
> Signed-off-by: Thierry Reding
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_gem.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index f6ca51259fa3..597318ae307e 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -309,7 +309,7 @@ EXPORT_SYMBOL(drm_gem_dumb_destroy);
> * drm_gem_handle_create_tail - internal functions to create a handle
> * @file_priv: drm file-private structure to register the handle for
> * @obj: object to register
> - * @handlep: pionter to return the created handle to the caller
> + * @handlep: pointer to return the created handle to the caller
> *
> * This expects the dev->object_name_lock to be held already and will drop it
> * before returning. Used to avoid races in establishing new handles when
> @@ -362,7 +362,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
> }
>
> /**
> - * gem_handle_create - create a gem handle for an object
> + * drm_gem_handle_create - create a gem handle for an object
> * @file_priv: drm file-private structure to register the handle for
> * @obj: object to register
> * @handlep: pionter to return the created handle to the caller
> @@ -371,10 +371,9 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
> * to the object, which includes a regular reference count. Callers
> * will likely want to dereference the object afterwards.
> */
> -int
> -drm_gem_handle_create(struct drm_file *file_priv,
> -struct drm_gem_object *obj,
> -u32 *handlep)
> +int drm_gem_handle_create(struct drm_file *file_priv,
> + struct drm_gem_object *obj,
> + u32 *handlep)
> {
> mutex_lock(&obj->dev->object_name_lock);
>
> --
> 2.1.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 81644] Random crashes on RadeonSI with Chromium.
https://bugs.freedesktop.org/show_bug.cgi?id=81644 --- Comment #148 from Gedalya --- See https://code.google.com/p/chromium/issues/detail?id=404357 In my case it was definitely crashing with mesa 10.2. In all cases it seemed to be triggered by chromium. Bug 85647 is talking about something that started with mesa 10.3. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/5819663d/attachment-0001.html>
[PATCH] drm: Global atomic state handling
On 11/05/2014 12:07 AM, Daniel Vetter wrote:
> /**
> + * struct struct drm_atomic_state - the global state object for atomic
> updates
> + * @dev: parent DRM device
> + * @flags: state flags like async update
> + * @planes: pointer to array of plane pointers
> + * @plane_states: pointer to array of plane states pointers
> + * @crtcs: pointer to array of CRTC pointers
> + * @crtc_states: pointer to array of CRTC states pointers
> + * @connectors: pointer to array of connector pointers
> + * @connector_states: pointer to array of connector states pointers
> + * @acquire_ctx: acquire context for this atomic modeset state update
> + */
> +struct drm_atomic_state {
> + struct drm_device *dev;
> + uint32_t flags;
> + struct drm_plane **planes;
> + struct drm_plane_state **plane_states;
> + struct drm_crtc **crtcs;
> + struct drm_crtc_state **crtc_states;
> + struct drm_connector **connectors;
> + struct drm_connector_state **connector_states;
> +
> + struct drm_modeset_acquire_ctx *acquire_ctx;
> +};
The lines above are indented with spaces instead of tabs.
Ander
[RFC 1/2] core: Add generic object registry implementation
On Wed, Nov 05, 2014 at 01:36:24PM +0100, Andrzej Hajda wrote: > On 11/04/2014 05:29 PM, Thierry Reding wrote: > > From: Thierry Reding > > > > Add a generic implementation of an object registry. This targets drivers > > and subsystems that provide auxiliary objects that other drivers need to > > look up. The goal is to put the difficult parts (keep object references, > > module usage count, ...) into core code so that individual subsystems do > > not have to deal with them. > > > > The intention is for subsystems to instantiate a struct registry and use > > a struct registry_record embedded into a subsystem-specific structure to > > provide a subsystem-specific API around that. > > > As I understand you want to use this registry for panels and bridges. > Could you explain the idea and describe example scenario when these > refcountings are useful. I guess it should be when panel attached to > drmdrv want to disappear. Correct. When a panel driver is unloaded it frees memory associated with the panel. The goal of this registry is for the panel object to stay around until all references are gone. > Real lifetime of panel is limited by probe/remove callbacks of panel > driver, do you want to prolong it behind these limits? Yes. > Do you want to have zombie panels, without hardware they abstract? For > what purpose? So that display drivers don't try to access objects that have been freed. > What do you want to do with panel ops? Do they need support both life > states? That's a slightly separate issue for which David Herrmann had a solution in mind. As I understand it, the idea would be for these objects to gain revoke support. The goal is that once the underlying object disappears, calling any of the operations involved would fail (with something like a -ENODEV). It's a little more complicated than that because the device could disappear right in the middle of such an operation, but that's precisely what revoke should allow us to do. Readding David for visibility. > Anyway implementation currently seems to be broken, DRM panels are currently completely broken. If you remove the driver for a panel the display driver that uses this panel will simply crash the next time it tries to do anything with the panel. This type of registry is the first step in trying to fix this. > you try to refcount > objects which are usually embedded in driver priv data, which disappears > during remove callback of the driver. A second step in fixing this will be to convert drivers to no longer free the panel but rather drop their reference to the panel that they've registered. This will make sure that memory doesn't get freed until all references are gone. Note that this means that drivers will also need to be converted not to use devm_* helpers since that conflicts with the reference counted life- times of these record objects. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/4e6c84bb/attachment-0001.sig>
[PATCH] drm/atomic-helper: implementatations for legacy interfaces
On Wed, Nov 05, 2014 at 02:46:10PM +0100, Daniel Vetter wrote:
> Well, except page_flip since that requires async commit, which isn't
> there yet.
>
> For the functions which changes planes there's a bit of trickery
> involved to keep the fb refcounting working. But otherwise fairly
> straight-forward atomic updates.
>
> The property setting functions are still a bit incomplete. Once we
> have generic properties (e.g. rotation, but also all the properties
> needed by the atomic ioctl) we need to filter those out and parse them
> in the helper. Preferrably with the same function as used by the real
> atomic ioctl implementation.
>
> v2: Fixup kerneldoc, reported by Paulo.
>
> v3: Add missing EXPORT_SYMBOL.
>
> v4: We need to look at the crtc of the modeset, not some random
> leftover one from a previous loop when udpating the connector->crtc
> routing. Also push some local variables into inner loops to avoid
> these kinds of bugs.
>
> v5: Adjust semantics - drivers now own the atomic state upon
> successfully synchronous commit.
>
> v6: Use the set_crtc_for_plane function to assign the crtc, since
> otherwise the book-keeping is off.
>
> v7:
> - Improve comments.
> - Filter out the crtc of the ->set_config call when recomputing
> crtc_state->enabled: We should compute the same state, but not doing
> so will give us a good chance to catch bugs and inconsistencies -
> the atomic helper's atomic_check function re-validates this again.
> - Fix the set_config implementation logic when disabling the crtc: We
> still need to update the output routing to disable all the
> connectors properly in the state. Caught by the atomic_check
> functions, so at least that part worked ;-) Also add some WARN_ONs
> to ensure ->set_config preconditions all apply.
>
> v8: Fixup an embarrassing h/vdisplay mixup.
>
> v9: Shuffled bad squash to the right patch, spotted by Daniel
>
> v10: Use set_crtc_for_connector as suggested by Sean.
>
> v11: Daniel Thompson noticed that my error handling is inconsistent
> and that in a few cases I didn't handle fatal errors (i.e. not
> -EDEADLK). Fix this by consolidate the ww mutex backoff handling
> into one check in the fail: block and flatten the error control
> flow everywhere else.
>
> Cc: Daniel Thompson
> Cc: Sean Paul
> Cc: Daniel Thompson
> Cc: Paulo Zanoni
> Signed-off-by: Daniel Vetter
Looks good to me, a couple nits.
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 504
> +++-
> include/drm/drm_atomic_helper.h | 21 ++
> 2 files changed, 524 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 887e1971c915..b84581f8c8fc 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -86,6 +86,7 @@ steal_encoder(struct drm_atomic_state *state,
> struct drm_crtc_state *crtc_state;
> struct drm_connector *connector;
> struct drm_connector_state *connector_state;
> + int ret;
>
> /*
> * We can only steal an encoder coming from a connector, which means we
> @@ -116,7 +117,9 @@ steal_encoder(struct drm_atomic_state *state,
> if (IS_ERR(connector_state))
> return PTR_ERR(connector_state);
>
> - connector_state->crtc = NULL;
> + ret = drm_atomic_set_crtc_for_connector(connector_state, NULL);
> + if (ret)
> + return ret;
> connector_state->best_encoder = NULL;
> }
>
> @@ -1045,3 +1048,502 @@ void drm_atomic_helper_swap_state(struct drm_device
> *dev,
> }
> }
> EXPORT_SYMBOL(drm_atomic_helper_swap_state);
> +
> +/**
> + * drm_atomic_helper_update_plane - Helper for primary plane update using
> atomic
> + * @plane: plane object to update
> + * @crtc: owning CRTC of owning plane
> + * @fb: framebuffer to flip onto plane
> + * @crtc_x: x offset of primary plane on crtc
> + * @crtc_y: y offset of primary plane on crtc
> + * @crtc_w: width of primary plane rectangle on crtc
> + * @crtc_h: height of primary plane rectangle on crtc
> + * @src_x: x offset of @fb for panning
> + * @src_y: y offset of @fb for panning
> + * @src_w: width of source rectangle in @fb
> + * @src_h: height of source rectangle in @fb
> + *
> + * Provides a default plane update handler using the atomic driver interface.
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_helper_update_plane(struct drm_plane *plane,
> + struct drm_crtc *crtc,
> + struct drm_framebuffer *fb,
> + int crtc_x, int crtc_y,
> + unsigned int crtc_w, unsigned int crtc_h,
> + uint32_t src_x, uint32_t src_y,
> + uint32_t src_w, uint32_t src_h)
> +{
> + struct drm_atomic_state *state;
> + struct drm_plane_state *plane_state;
> + int ret = 0;
> +
> + state = drm_atomic_state_alloc(plane->dev);
> + if (!state)
> + return -ENOMEM;
> +
> + state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> +retry:
> + plane_state = drm_atomic_get_plane_state(state, plane);
> + if (IS_ERR(plane_state))
[PATCH] drm/atomic-helper: implement ->page_flip
Currently there is no way to implement async flips using atomic, that
essentially requires us to be able to cancel pending requests
mid-flight.
To be able to do that (and I guess we want this since vblank synced
updates whic opportunistically cancel still pending updates seem to be
wanted) we'd need to add a mandatory cancellation mode. Depending upon
the exact semantics we decide upon that could mean that userspace will
not get completion events, or will get them all stacked up.
So reject async updates for now. Also async updates usually means not
vblank synced at all, and I guess for drivers which want to support
this they should simply add a special pageflip handler (since usually
you need a special flip cmd to achieve this). That kind of async flip
is pretty much exclusively just used for games and benchmarks where
dropping just one frame means you'll get a headshot or something bad
like that ... And so slight amounts of tearing is acceptable.
v2: Fixup kerneldoc, reported by Paulo.
v3: Use the set_crtc_for_plane function to assign the crtc, since
otherwise the book-keeping is off.
v4: Update crtc->primary->fb since ->page_flip is the only driver
callback where the core won't do this itself. We might want to fix
this inconsistency eventually.
v5: Use set_crtc_for_connector as suggested by Sean.
v6: Daniel Thompson noticed that my error handling is inconsistent
and that in a few cases I didn't handle fatal errors (i.e. not
-EDEADLK). Fix this by consolidate the ww mutex backoff handling
into one check in the fail: block and flatten the error control
flow everywhere else.
Cc: Daniel Thompson
Cc: Sean Paul
Cc: Paulo Zanoni
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_atomic_helper.c | 86 +
include/drm/drm_atomic_helper.h | 5 +++
2 files changed, 91 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index c7d1554b7ce1..6b490571c803 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1608,3 +1608,89 @@ backoff:
goto retry;
}
EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
+
+/**
+ * drm_atomic_helper_page_flip - execute a legacy page flip
+ * @crtc: DRM crtc
+ * @fb: DRM framebuffer
+ * @event: optional DRM event to signal upon completion
+ * @flags: flip flags for non-vblank sync'ed updates
+ *
+ * Provides a default page flip implementation using the atomic driver
interface.
+ *
+ * Note that for now so called async page flips (i.e. updates which are not
+ * synchronized to vblank) are not supported, since the atomic interfaces have
+ * no provisions for this yet.
+ *
+ * Returns:
+ * Returns 0 on success, negative errno numbers on failure.
+ */
+int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ struct drm_pending_vblank_event *event,
+ uint32_t flags)
+{
+ struct drm_plane *plane = crtc->primary;
+ struct drm_atomic_state *state;
+ struct drm_plane_state *plane_state;
+ struct drm_crtc_state *crtc_state;
+ int ret = 0;
+
+ if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
+ return -EINVAL;
+
+ state = drm_atomic_state_alloc(plane->dev);
+ if (!state)
+ return -ENOMEM;
+
+ state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+retry:
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state)) {
+ ret = PTR_ERR(crtc_state);
+ goto fail;
+ }
+ crtc_state->event = event;
+
+ plane_state = drm_atomic_get_plane_state(state, plane);
+ if (IS_ERR(plane_state)) {
+ ret = PTR_ERR(plane_state);
+ goto fail;
+ }
+
+ ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
+ if (ret != 0)
+ goto fail;
+ plane_state->fb = fb;
+
+ ret = drm_atomic_async_commit(state);
+ if (ret != 0)
+ goto fail;
+
+ /* TODO: ->page_flip is the only driver callback where the core
+* doesn't update plane->fb. For now patch it up here. */
+ plane->fb = plane->state->fb;
+
+ /* Driver takes ownership of state on successful async commit. */
+ return 0;
+fail:
+ if (ret == -EDEADLK)
+ goto backoff;
+
+ drm_atomic_state_free(state);
+
+ return ret;
+backoff:
+ drm_atomic_legacy_backoff(state);
+ drm_atomic_state_clear(state);
+
+ /*
+* Someone might have exchanged the framebuffer while we dropped locks
+* in the backoff code. We need to fix up the fb refcount tracking the
+* core does for us.
+*/
+ plane->old_fb = plane->fb;
+
+ goto retry;
+}
+EXPORT_SYMBOL(drm_atomic_helper_page_flip);
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index
[PATCH] drm/atomic-helper: implementatations for legacy interfaces
Well, except page_flip since that requires async commit, which isn't
there yet.
For the functions which changes planes there's a bit of trickery
involved to keep the fb refcounting working. But otherwise fairly
straight-forward atomic updates.
The property setting functions are still a bit incomplete. Once we
have generic properties (e.g. rotation, but also all the properties
needed by the atomic ioctl) we need to filter those out and parse them
in the helper. Preferrably with the same function as used by the real
atomic ioctl implementation.
v2: Fixup kerneldoc, reported by Paulo.
v3: Add missing EXPORT_SYMBOL.
v4: We need to look at the crtc of the modeset, not some random
leftover one from a previous loop when udpating the connector->crtc
routing. Also push some local variables into inner loops to avoid
these kinds of bugs.
v5: Adjust semantics - drivers now own the atomic state upon
successfully synchronous commit.
v6: Use the set_crtc_for_plane function to assign the crtc, since
otherwise the book-keeping is off.
v7:
- Improve comments.
- Filter out the crtc of the ->set_config call when recomputing
crtc_state->enabled: We should compute the same state, but not doing
so will give us a good chance to catch bugs and inconsistencies -
the atomic helper's atomic_check function re-validates this again.
- Fix the set_config implementation logic when disabling the crtc: We
still need to update the output routing to disable all the
connectors properly in the state. Caught by the atomic_check
functions, so at least that part worked ;-) Also add some WARN_ONs
to ensure ->set_config preconditions all apply.
v8: Fixup an embarrassing h/vdisplay mixup.
v9: Shuffled bad squash to the right patch, spotted by Daniel
v10: Use set_crtc_for_connector as suggested by Sean.
v11: Daniel Thompson noticed that my error handling is inconsistent
and that in a few cases I didn't handle fatal errors (i.e. not
-EDEADLK). Fix this by consolidate the ww mutex backoff handling
into one check in the fail: block and flatten the error control
flow everywhere else.
Cc: Daniel Thompson
Cc: Sean Paul
Cc: Daniel Thompson
Cc: Paulo Zanoni
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_atomic_helper.c | 504 +++-
include/drm/drm_atomic_helper.h | 21 ++
2 files changed, 524 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 887e1971c915..b84581f8c8fc 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -86,6 +86,7 @@ steal_encoder(struct drm_atomic_state *state,
struct drm_crtc_state *crtc_state;
struct drm_connector *connector;
struct drm_connector_state *connector_state;
+ int ret;
/*
* We can only steal an encoder coming from a connector, which means we
@@ -116,7 +117,9 @@ steal_encoder(struct drm_atomic_state *state,
if (IS_ERR(connector_state))
return PTR_ERR(connector_state);
- connector_state->crtc = NULL;
+ ret = drm_atomic_set_crtc_for_connector(connector_state, NULL);
+ if (ret)
+ return ret;
connector_state->best_encoder = NULL;
}
@@ -1045,3 +1048,502 @@ void drm_atomic_helper_swap_state(struct drm_device
*dev,
}
}
EXPORT_SYMBOL(drm_atomic_helper_swap_state);
+
+/**
+ * drm_atomic_helper_update_plane - Helper for primary plane update using
atomic
+ * @plane: plane object to update
+ * @crtc: owning CRTC of owning plane
+ * @fb: framebuffer to flip onto plane
+ * @crtc_x: x offset of primary plane on crtc
+ * @crtc_y: y offset of primary plane on crtc
+ * @crtc_w: width of primary plane rectangle on crtc
+ * @crtc_h: height of primary plane rectangle on crtc
+ * @src_x: x offset of @fb for panning
+ * @src_y: y offset of @fb for panning
+ * @src_w: width of source rectangle in @fb
+ * @src_h: height of source rectangle in @fb
+ *
+ * Provides a default plane update handler using the atomic driver interface.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+int drm_atomic_helper_update_plane(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h)
+{
+ struct drm_atomic_state *state;
+ struct drm_plane_state *plane_state;
+ int ret = 0;
+
+ state = drm_atomic_state_alloc(plane->dev);
+ if (!state)
+ return -ENOMEM;
+
+ state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+retry:
+ plane_state = drm_atomic_get_plane_state(state, pla
[PATCH] drm: Global atomic state handling
Some differences compared to Rob's patches again: - Dropped the committed and checked booleans. Checking will be internally enforced by always calling ->atomic_check before ->atomic_commit. And async handling needs to be solved differently because the current scheme completely side-steps ww mutex deadlock avoidance (and so either reinvents a new deadlock avoidance wheel or like the current code just deadlocks). - State for connectors needed to be added, since now they have a full-blown drm_connector_state (so that drivers have something to attach their own stuff to). - Refcounting is gone. I plane to solve async updates differently, since the lock-passing scheme doesn't cut it (since it abuses ww mutexes). Essentially what we need for async is a simple ownership transfer from the caller to the driver. That doesn't need full-blown refcounting. - The acquire ctx is a pointer. Real atomic callers should have that on their stack, legacy entry points need to put the right one (obtained by drm_modeset_legacy_acuire_ctx) in there. - I've dropped all hooks except check/commit. All the begin/end handling is done by core functions and is the same. - commit/check are just thin wrappers that ensure that ->check is always called. - To help out with locking in the legacy implementations I've added a helper to just grab all locks in the backoff case. v2: Add notices that check/commit can fail with EDEADLK. v3: - More consistent naming for state_alloc. - Add state_clear which is needed for backoff and retry. v4: Planes/connectors can switch between crtcs, and we need to be careful that we grab the state (and locks) for both the old and new crtc. Improve the interface functions to ensure this. v5: Add functions to grab affected connectors for a crtc and to recompute the crtc->enable state. This is useful for both helper and atomic ioctl code when e.g. removing a connector. v6: Squash in fixup from Fengguang to use ERR_CAST. v7: Add debug output. v8: Make checkpatch happy about kcalloc argument ordering. v9: Improve kerneldoc in drm_crtc.h v10: - Fix another kcalloc argument misorder I've missed. - More polish for kerneldoc. v11: Clarify the ownership rules for the state object. The new rule is that a successful drm_atomic_commit (whether synchronous or asnyc) always inherits the state and is responsible for the clean-up. That way async and sync ->commit functions are more similar. v12: A few bugfixes: - Assign state->state pointers correctly when grabbing state objects - we need to link them up with the global state. - Handle a NULL crtc in set_crtc_for_plane to simplify code flow a bit for the callers of this function. v13: Review from Sean: - kerneldoc spelling fixes - Don't overallocate states->planes. - Handle NULL crtc in set_crtc_for_connector. v14: Sprinkle __must_check over all functions which do wait/wound locking to make sure callers don't forget this. Since I have ;-) v15: Be more explicit in the kerneldoc when functions can return -EDEADLK what to do. And that every other -errno is fatal. Cc: Daniel Thompson Cc: Fengguang Wu Cc: Sean Paul Cc: Matt Roper Reviewed-by: Sean Paul Signed-off-by: Daniel Vetter --- Documentation/DocBook/drm.tmpl | 4 + drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_atomic.c | 600 + include/drm/drm_atomic.h | 65 + include/drm/drm_crtc.h | 35 +++ 5 files changed, 705 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_atomic.c create mode 100644 include/drm/drm_atomic.h diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index f83d3b6ea4e5..748513b34025 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -996,6 +996,10 @@ int max_width, max_height; !Edrivers/gpu/drm/drm_modes.c + Atomic Mode Setting Function Reference +!Edrivers/gpu/drm/drm_atomic.c + + Frame Buffer Creation struct drm_framebuffer *(*fb_create)(struct drm_device *dev, struct drm_file *file_priv, diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 9292a761ea6d..c5e37dc459ee 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -14,7 +14,7 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \ drm_info.o drm_debugfs.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_modeset_lock.o drm_atomic.o drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c new file mode 100644 index ..ad15a88c0f74 --- /dev/null +++ b/drivers/gpu/drm/drm_atomic.c @@ -0,0 +1,600 @@ +/* + * Copyright (C) 2014 Red Hat + * Copyr
[PATCH 1/2] imx-drm: imx-hdmi: split imx soc specific code from imx-hdmi
Hi Andy,
I think separating the core from the SoC specific part is a good step
into the right direction.
Am Mittwoch, den 05.11.2014, 20:55 +0800 schrieb Andy Yan:
> imx6 and rockchip rk3288 and JZ4780 (Ingenic Xburst/MIPS)
> use the interface compatible Designware HDMI IP, but they
> also have some lightly difference, such as phy pll configuration,
> register width(imx hdmi register is one byte, but rk3288 is 4
> bytes width), 4K support(imx6 doesn't support 4k, but rk3288 does),
> clk useage,and the crtc mux configuration is also platform specific.
>
> To reuse the imx hdmi driver, split the platform specific code out
> to dw_hdmi-imx.c.
>
> Signed-off-by: Andy Yan
[...]
> +static int imx_hdmi_parse_dt(struct imx_hdmi_priv *hdmi)
> +{
> + struct device_node *np = hdmi->dev->of_node;
> +
> + hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "gpr");
> + if (IS_ERR(hdmi->regmap)) {
> + dev_err(hdmi->dev, "Unable to get gpr\n");
> + return PTR_ERR(hdmi->regmap);
> + }
> +
> + hdmi->isfr_clk = devm_clk_get(hdmi->dev, "isfr");
> + if (IS_ERR(hdmi->isfr_clk)) {
> + dev_err(hdmi->dev, "Unable to get HDMI isfr clk\n");
> + return PTR_ERR(hdmi->isfr_clk);
> + }
> +
> + hdmi->iahb_clk = devm_clk_get(hdmi->dev, "iahb");
> + if (IS_ERR(hdmi->iahb_clk)) {
> + dev_err(hdmi->dev, "Unable to get HDMI iahb clk\n");
> + return PTR_ERR(hdmi->iahb_clk);
> + }
Surely this IP core needs to be clocked regardless of the SoC? How are
clocks controlled on rk3288 and jz4780?
[...]
> +/*On rockchip platform, no-word access to the hdmi
> + * register will causes an imprecise external abort
> + */
> +static inline void hdmi_writel(struct imx_hdmi *hdmi, u32 val, int offset)
> +{
> + writel(val, hdmi->regs + (offset << 2));
> +}
>
> - bool phy_enabled;
> - struct drm_display_mode previous_mode;
> +static inline u32 hdmi_readl(struct imx_hdmi *hdmi, int offset)
> +{
> + return readl(hdmi->regs + (offset << 2));
> +}
>
> - struct regmap *regmap;
> - struct i2c_adapter *ddc;
> - void __iomem *regs;
> +static void hdmi_modl(struct imx_hdmi *hdmi, u32 data, u32 mask, unsigned
> reg)
> +{
> + u32 val = hdmi_readl(hdmi, reg) & ~mask;
>
> - unsigned int sample_rate;
> - int ratio;
> -};
> + val |= data & mask;
> + hdmi_writel(hdmi, val, reg);
> +}
>
> -static void imx_hdmi_set_ipu_di_mux(struct imx_hdmi *hdmi, int ipu_di)
> +static void hdmi_mask_writel(struct imx_hdmi *hdmi, u32 data, unsigned int
> reg,
> + u32 shift, u32 mask)
> {
> - regmap_update_bits(hdmi->regmap, IOMUXC_GPR3,
> -IMX6Q_GPR3_HDMI_MUX_CTL_MASK,
> -ipu_di << IMX6Q_GPR3_HDMI_MUX_CTL_SHIFT);
> + hdmi_modl(hdmi, data << shift, mask, reg);
> }
>
> -static inline void hdmi_writeb(struct imx_hdmi *hdmi, u8 val, int offset)
> +static inline void hdmi_writeb(struct imx_hdmi *hdmi, u32 val, int offset)
> {
> writeb(val, hdmi->regs + offset);
> }
>
> -static inline u8 hdmi_readb(struct imx_hdmi *hdmi, int offset)
> +static inline u32 hdmi_readb(struct imx_hdmi *hdmi, int offset)
> {
> return readb(hdmi->regs + offset);
> }
>
> -static void hdmi_modb(struct imx_hdmi *hdmi, u8 data, u8 mask, unsigned reg)
> +static void hdmi_modb(struct imx_hdmi *hdmi, u32 data, u32 mask, unsigned
> reg)
> {
> u8 val = hdmi_readb(hdmi, reg) & ~mask;
Do you really need to use readl instead of readb? In my opinion it would
be better then to convert the driver to use regmap for register access
(in a separate patch) and then let the SoC specific driver extension
provide the regmap to the core driver.
[...]
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> new file mode 100644
> index 000..e7e8285
> --- /dev/null
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -0,0 +1,114 @@
> +/*
> + * Copyright (C) 2011 Freescale Semiconductor, Inc.
> + *
> + * 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.
> + */
> +
> +#ifndef __DW_HDMI_H__
> +#define __DW_HDMI_H__
> +
> +#include
> +
> +#define HDMI_EDID_LEN 512
> +
> +enum {
> + RES_8,
> + RES_10,
> + RES_12,
> + RES_MAX,
> +};
> +
> +enum imx_hdmi_devtype {
> + IMX6Q_HDMI,
> + IMX6DL_HDMI,
> +};
> +
> +struct mpll_config {
> + unsigned long mpixelclock;
> + struct {
> + u16 cpce;
> + u16 gmp;
> + } res[RES_MAX];
> +};
> +
> +struct curr_ctrl {
> + unsigned long mpixelclock;
> + u16 curr[RES_MAX];
> +};
For a header file in include/drm/bridge maybe these struct names are a
bit too generic now.
regards
Philipp
[PATCH v4 01/16] drm/dsi: Add message to packet translator
On 11/04/2014 03:39 PM, Thierry Reding wrote:
> On Tue, Nov 04, 2014 at 03:21:14PM +0100, Andrzej Hajda wrote:
>> On 11/04/2014 02:58 PM, Thierry Reding wrote:
>>> On Tue, Nov 04, 2014 at 12:43:21PM +0100, Andrzej Hajda wrote:
On 11/03/2014 10:13 AM, Thierry Reding wrote:
> [...]
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c
> b/drivers/gpu/drm/drm_mipi_dsi.c
> [...]
> + packet->header[1] = (msg->tx_len >> 0) & 0xff;
> + packet->header[2] = (msg->tx_len >> 8) & 0xff;
> +
> + packet->payload_length = msg->tx_len;
> + packet->payload = tx;
> + } else {
> + packet->header[1] = (msg->tx_len > 0) ? tx[0] : 0;
> + packet->header[2] = (msg->tx_len > 1) ? tx[1] : 0;
> + }
> +
> + packet->size = sizeof(packet->header) + packet->payload_length;
size seems to be completely useless.
>>> It's not. Tegra has two FIFOs that can be selected depending on the size
>>> of a transfer. I use this field to detect which FIFO needs to be
>>> selected.
>> But size is always equal payload_length + 4, so it does not carry any
>> additional information.
> Right, but it's out of convenience to prevent every driver from doing
> this again. It's part of the help that the helper provides. =)
>
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(mipi_dsi_create_packet);
> +
> +/**
> * mipi_dsi_dcs_write - send DCS write command
> * @dsi: DSI device
> * @data: pointer to the command followed by parameters
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 8569dc5a1026..663aa68826f4 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -44,6 +44,24 @@ struct mipi_dsi_msg {
> };
>
> /**
> + * struct mipi_dsi_packet - represents a MIPI DSI packet in protocol
> format
> + * @size: size (in bytes) of the packet
> + * @header: the four bytes that make up the header (Data ID, Word Count
> or
> + * Packet Data, and ECC)
> + * @payload_length: number of bytes in the payload
> + * @payload: a pointer to a buffer containing the payload, if any
> + */
> +struct mipi_dsi_packet {
> + size_t size;
> + u8 header[4];
I wonder if it wouldn't be good to make it u32 or at least anonymous union:
union {
u8 header[4];
u32 header32;
};
>>> I'm not sure this is very useful. It's pretty trivial how you
>>> concatenate the individual bytes and it actually remove any ambiguity
>>> about the endianness.
>> This looks better:
>>
>> val = le16_to_cpu(pkt->header32);
>> writel(val, REG);
>>
>> than this:
>>
>> val = pkt->header[2] << 16 | pkt->header[1] << 8 | pkt->header[0];
>> writel(val, REG);
> I disagree. =) Having the individual bytes makes their ordering very
> explicit.
>
Wow, you want to keep size field to prevent drivers from adding
sometimes 4 to payload,
but you do not want to simplify header calculation that is much more
complicated :)
Regards
Andrzej
[PATCH 7/7] drm: Sanitize DRM_IOCTL_MODE_CREATE_DUMB input
From: Thierry Reding Some drivers treat the pitch and size fields as inputs and will use them as minima provided by userspace so that they are only overwritten if the minimal requirements of the driver exceed them. This can cause strange behaviour when applications don't zero out these fields, causing whatever was on the stack to be passed to the IOCTL. In a typical case this would become visible as a failed allocation if the pitch or size were unusually high. But this could also cause more subtle bugs like overallocating dumb framebuffers. To prevent drivers from misusing these values, make the DRM core zero out the pitch and size fields before passing the structure to the driver implementation. While at it, also set the output handle field to zero for good measure, even though it's less likely to be abused. Signed-off-by: Thierry Reding --- drivers/gpu/drm/drm_crtc.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 0f3c24c0981b..6aceb689ccea 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -4755,6 +4755,14 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev, if (PAGE_ALIGN(size) == 0) return -EINVAL; + /* +* handle, pitch and size are output parameters. Zero them out to +* prevent drivers from accidentally using uninitialized data. +*/ + args->handle = 0; + args->pitch = 0; + args->size = 0; + return dev->driver->dumb_create(file_priv, dev, args); } -- 2.1.3
[PATCH 6/7] drm/rcar: gem: dumb: pitch is an output
From: Thierry Reding When creating a dumb buffer object using the DRM_IOCTL_MODE_CREATE_DUMB IOCTL, only the width, height, bpp and flags fields are inputs. The caller is not guaranteed to zero out or set handle, pitch and size. Drivers must not treat these values as possible inputs, otherwise they may use uninitialized memory during the computation of the framebuffer size. The R-Car DU driver treats the pitch passed in from userspace as minimum and will only overwrite it when the driver-computed pitch is larger, allowing userspace to, intentionally or not, overallocate framebuffers. Signed-off-by: Thierry Reding --- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 5329491e32c3..6289e3797bc5 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -126,7 +126,7 @@ int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev, else align = 16 * args->bpp / 8; - args->pitch = roundup(max(args->pitch, min_pitch), align); + args->pitch = roundup(min_pitch, align); return drm_gem_cma_dumb_create_internal(file, dev, args); } -- 2.1.3
[PATCH 5/7] drm/omap: gem: dumb: pitch is an output
From: Thierry Reding
When creating a dumb buffer object using the DRM_IOCTL_MODE_CREATE_DUMB
IOCTL, only the width, height, bpp and flags fields are inputs. The
caller is not guaranteed to zero out or set handle, pitch and size.
Drivers must not treat these values as possible inputs, otherwise they
may use uninitialized memory during the computation of the framebuffer
size.
The OMAP driver uses the pitch field passed in by userspace as a minimum
and only override it if the driver-computed pitch is larger than what
userspace provided. To prevent this from causing overallocation, fix the
minimum pitch to 0 to enforce the driver-computed pitch.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/omapdrm/omap_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
b/drivers/gpu/drm/omapdrm/omap_gem.c
index e4849413ee80..bff60b73995b 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -613,7 +613,7 @@ int omap_gem_dumb_create(struct drm_file *file, struct
drm_device *dev,
union omap_gem_size gsize;
/* in case someone tries to feed us a completely bogus stride: */
- args->pitch = align_pitch(args->pitch, args->width, args->bpp);
+ args->pitch = align_pitch(0, args->width, args->bpp);
args->size = PAGE_ALIGN(args->pitch * args->height);
gsize = (union omap_gem_size){
--
2.1.3
[PATCH 4/7] drm/cma: Introduce drm_gem_cma_dumb_create_internal()
From: Thierry Reding
This function is similar to drm_gem_cma_dumb_create() but targetted at
kernel internal users so that they can override the pitch and size
requirements of the dumb buffer.
It is important to make this difference because the IOCTL says that the
pitch and size fields are to be considered outputs and therefore should
not be used in computations of the framebuffer size. Internal users may
still want to use this code to avoid duplication and at the same time
pass on additional, driver-specific restrictions on the pitch and size.
While at it, convert the R-Car DU driver, the single user that overrides
the pitch, to use the new internal helper.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/drm_gem_cma_helper.c | 33 -
drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
include/drm/drm_gem_cma_helper.h | 5 +
3 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
b/drivers/gpu/drm/drm_gem_cma_helper.c
index 55306be1f8f7..c55986566f0c 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -189,7 +189,7 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj)
EXPORT_SYMBOL_GPL(drm_gem_cma_free_object);
/**
- * drm_gem_cma_dumb_create - create a dumb buffer object
+ * drm_gem_cma_dumb_create_internal - create a dumb buffer object
* @file_priv: DRM file-private structure to create the dumb buffer for
* @drm: DRM device
* @args: IOCTL data
@@ -199,12 +199,12 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_free_object);
*
* Return: 0 on success or a negative error code on failure.
*/
-int drm_gem_cma_dumb_create(struct drm_file *file_priv,
- struct drm_device *drm,
- struct drm_mode_create_dumb *args)
+int drm_gem_cma_dumb_create_internal(struct drm_file *file_priv,
+struct drm_device *drm,
+struct drm_mode_create_dumb *args)
{
+ unsigned int min_pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
struct drm_gem_cma_object *cma_obj;
- int min_pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
if (args->pitch < min_pitch)
args->pitch = min_pitch;
@@ -216,6 +216,29 @@ int drm_gem_cma_dumb_create(struct drm_file *file_priv,
&args->handle);
return PTR_ERR_OR_ZERO(cma_obj);
}
+EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create_internal);
+
+/**
+ * drm_gem_cma_dumb_create - create a dumb buffer object
+ * @file_priv: DRM file-private structure to create the dumb buffer for
+ * @drm: DRM device
+ * @args: IOCTL data
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int drm_gem_cma_dumb_create(struct drm_file *file_priv,
+ struct drm_device *drm,
+ struct drm_mode_create_dumb *args)
+{
+ struct drm_gem_cma_object *cma_obj;
+
+ args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
+ args->size = args->pitch * args->height;
+
+ cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size,
+&args->handle);
+ return PTR_ERR_OR_ZERO(cma_obj);
+}
EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create);
/**
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 6c24ad7d03ef..5329491e32c3 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -128,7 +128,7 @@ int rcar_du_dumb_create(struct drm_file *file, struct
drm_device *dev,
args->pitch = roundup(max(args->pitch, min_pitch), align);
- return drm_gem_cma_dumb_create(file, dev, args);
+ return drm_gem_cma_dumb_create_internal(file, dev, args);
}
static struct drm_framebuffer *
diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
index 873d4eb7f125..acd6af8a8e67 100644
--- a/include/drm/drm_gem_cma_helper.h
+++ b/include/drm/drm_gem_cma_helper.h
@@ -30,6 +30,11 @@ to_drm_gem_cma_obj(struct drm_gem_object *gem_obj)
void drm_gem_cma_free_object(struct drm_gem_object *gem_obj);
/* create memory region for DRM framebuffer */
+int drm_gem_cma_dumb_create_internal(struct drm_file *file_priv,
+struct drm_device *drm,
+struct drm_mode_create_dumb *args);
+
+/* create memory region for DRM framebuffer */
int drm_gem_cma_dumb_create(struct drm_file *file_priv,
struct drm_device *drm,
struct drm_mode_create_dumb *args);
--
2.1.3
[PATCH 3/7] drm/doc: Add GEM/CMA helpers to kerneldoc
From: Thierry Reding
Most of the functions already have the beginnings of kerneldoc comments
but are using the wrong opening marker. Use the correct opening marker
and flesh out the comments so that they can be integrated with the DRM
DocBook document.
Signed-off-by: Thierry Reding
---
Documentation/DocBook/drm.tmpl | 6 ++
drivers/gpu/drm/drm_gem_cma_helper.c | 155 ++-
include/drm/drm_gem_cma_helper.h | 25 --
3 files changed, 139 insertions(+), 47 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 18025496a736..666a210af121 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -963,6 +963,12 @@ struct drm_gem_object * (*gem_prime_import)(struct
drm_device *dev,
!Edrivers/gpu/drm/drm_mm.c
!Iinclude/drm/drm_mm.h
+
+ CMA Helper Functions Reference
+!Pdrivers/gpu/drm/drm_gem_cma_helper.c cma helpers
+!Edrivers/gpu/drm/drm_gem_cma_helper.c
+!Iinclude/drm/drm_gem_cma_helper.h
+
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
b/drivers/gpu/drm/drm_gem_cma_helper.c
index 0316310e2cc4..55306be1f8f7 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -29,18 +29,30 @@
#include
#include
-/*
+/**
+ * DOC: cma helpers
+ *
+ * The Contiguous Memory Allocator reserves a pool of memory at early boot
+ * that is used to service requests for large blocks of contiguous memory.
+ *
+ * The DRM GEM/CMA helpers use this allocator as a means to provide buffer
+ * objects that are physically contiguous in memory. This is useful for
+ * display drivers that are unable to map scattered buffers via an IOMMU.
+ */
+
+/**
* __drm_gem_cma_create - Create a GEM CMA object without allocating memory
- * @drm: The drm device
- * @size: The GEM object size
+ * @drm: DRM device
+ * @size: size of the object to allocate
*
- * This function creates and initializes a GEM CMA object of the given size,
but
- * doesn't allocate any memory to back the object.
+ * This function creates and initializes a GEM CMA object of the given size,
+ * but doesn't allocate any memory to back the object.
*
- * Return a struct drm_gem_cma_object* on success or ERR_PTR values on failure.
+ * Return: A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded
+ * negative error code on failure.
*/
static struct drm_gem_cma_object *
-__drm_gem_cma_create(struct drm_device *drm, unsigned int size)
+__drm_gem_cma_create(struct drm_device *drm, size_t size)
{
struct drm_gem_cma_object *cma_obj;
struct drm_gem_object *gem_obj;
@@ -69,14 +81,16 @@ error:
return ERR_PTR(ret);
}
-/*
+/**
* drm_gem_cma_create - allocate an object with the given size
+ * @drm: DRM device
+ * @size: size of the object to allocate
*
- * returns a struct drm_gem_cma_object* on success or ERR_PTR values
- * on failure.
+ * Return: A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded
+ * negative error code on failure.
*/
struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
- unsigned int size)
+ size_t size)
{
struct drm_gem_cma_object *cma_obj;
int ret;
@@ -104,17 +118,21 @@ error:
}
EXPORT_SYMBOL_GPL(drm_gem_cma_create);
-/*
- * drm_gem_cma_create_with_handle - allocate an object with the given
- * size and create a gem handle on it
+/**
+ * drm_gem_cma_create_with_handle - allocate an object with the given size and
+ * return a GEM handle to it
+ * @file_priv: DRM file-private structure to register the handle for
+ * @drm: DRM device
+ * @size: size of the object to allocate
+ * @handle: return location for the GEM handle
*
- * returns a struct drm_gem_cma_object* on success or ERR_PTR values
- * on failure.
+ * Return: A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded
+ * negative error code on failure.
*/
-static struct drm_gem_cma_object *drm_gem_cma_create_with_handle(
- struct drm_file *file_priv,
- struct drm_device *drm, unsigned int size,
- unsigned int *handle)
+static struct drm_gem_cma_object *
+drm_gem_cma_create_with_handle(struct drm_file *file_priv,
+ struct drm_device *drm, size_t size,
+ uint32_t *handle)
{
struct drm_gem_cma_object *cma_obj;
struct drm_gem_object *gem_obj;
@@ -145,9 +163,9 @@ err_handle_create:
return ERR_PTR(ret);
}
-/*
- * drm_gem_cma_free_object - (struct drm_driver)->gem_free_object callback
- * function
+/**
+ * drm_gem_cma_free_object - free resources associated with a CMA GEM object
+ * @gem_obj: GEM object to free
*/
void drm_gem_cma_free_object(struct drm_gem_object *gem_obj)
{
@@ -170,15 +188,20 @@ void drm_gem_cma_free_object(struct drm_gem_object
*gem_obj)
}
EXPORT_SYMBOL_GPL(drm_gem_cma_free_object);
-/*
- * drm_gem_cma_dumb_c
[PATCH 2/7] drm/doc: mm: Fix indentation
From: Thierry Reding
Use spaces consistently for indentation in the memory-management
section.
Signed-off-by: Thierry Reding
---
Documentation/DocBook/drm.tmpl | 268 -
1 file changed, 134 insertions(+), 134 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index f6a9d7b21380..18025496a736 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -492,10 +492,10 @@ char *date;
The Translation Table Manager (TTM)
- TTM design background and information belongs here.
+TTM design background and information belongs here.
- TTM initialization
+TTM initialization
This section is outdated.
Drivers wishing to support TTM must fill out a drm_bo_driver
@@ -503,42 +503,42 @@ char *date;
pointers for initializing the TTM, allocating and freeing memory,
waiting for command completion and fence synchronization, and memory
migration. See the radeon_ttm.c file for an example of usage.
-
-
- The ttm_global_reference structure is made up of several fields:
-
-
- struct ttm_global_reference {
- enum ttm_global_types global_type;
- size_t size;
- void *object;
- int (*init) (struct ttm_global_reference *);
- void (*release) (struct ttm_global_reference *);
- };
-
-
- There should be one global reference structure for your memory
- manager as a whole, and there will be others for each object
- created by the memory manager at runtime. Your global TTM should
- have a type of TTM_GLOBAL_TTM_MEM. The size field for the global
- object should be sizeof(struct ttm_mem_global), and the init and
- release hooks should point at your driver-specific init and
- release routines, which probably eventually call
- ttm_mem_global_init and ttm_mem_global_release, respectively.
-
-
- Once your global TTM accounting structure is set up and initialized
- by calling ttm_global_item_ref() on it,
- you need to create a buffer object TTM to
- provide a pool for buffer object allocation by clients and the
- kernel itself. The type of this object should be TTM_GLOBAL_TTM_BO,
- and its size should be sizeof(struct ttm_bo_global). Again,
- driver-specific init and release functions may be provided,
- likely eventually calling ttm_bo_global_init() and
- ttm_bo_global_release(), respectively. Also, like the previous
- object, ttm_global_item_ref() is used to create an initial reference
- count for the TTM, which will call your initialization function.
-
+
+
+ The ttm_global_reference structure is made up of several fields:
+
+
+ struct ttm_global_reference {
+ enum ttm_global_types global_type;
+ size_t size;
+ void *object;
+ int (*init) (struct ttm_global_reference *);
+ void (*release) (struct ttm_global_reference *);
+ };
+
+
+ There should be one global reference structure for your memory
+ manager as a whole, and there will be others for each object
+ created by the memory manager at runtime. Your global TTM should
+ have a type of TTM_GLOBAL_TTM_MEM. The size field for the global
+ object should be sizeof(struct ttm_mem_global), and the init and
+ release hooks should point at your driver-specific init and
+ release routines, which probably eventually call
+ ttm_mem_global_init and ttm_mem_global_release, respectively.
+
+
+ Once your global TTM accounting structure is set up and initialized
+ by calling ttm_global_item_ref() on it,
+ you need to create a buffer object TTM to
+ provide a pool for buffer object allocation by clients and the
+ kernel itself. The type of this object should be TTM_GLOBAL_TTM_BO,
+ and its size should be sizeof(struct ttm_bo_global). Again,
+ driver-specific init and release functions may be provided,
+ likely eventually calling ttm_bo_global_init() and
+ ttm_bo_global_release(), respectively. Also, like the previous
+ object, ttm_global_item_ref() is used to create an initial reference
+ count for the TTM, which will call your initialization function.
+
@@ -566,19 +566,19 @@ char *date;
using driver-specific ioctls.
- On a fundamental level, GEM involves several operations:
-
- Memory allocation and freeing
- Command execution
- Aperture man
[PATCH 1/7] drm/gem: Fix a few kerneldoc typos
From: Thierry Reding
While at it, adjust the drm_gem_handle_create() function declaration to
be more consistent with other functions in the file.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/drm_gem.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index f6ca51259fa3..597318ae307e 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -309,7 +309,7 @@ EXPORT_SYMBOL(drm_gem_dumb_destroy);
* drm_gem_handle_create_tail - internal functions to create a handle
* @file_priv: drm file-private structure to register the handle for
* @obj: object to register
- * @handlep: pionter to return the created handle to the caller
+ * @handlep: pointer to return the created handle to the caller
*
* This expects the dev->object_name_lock to be held already and will drop it
* before returning. Used to avoid races in establishing new handles when
@@ -362,7 +362,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
}
/**
- * gem_handle_create - create a gem handle for an object
+ * drm_gem_handle_create - create a gem handle for an object
* @file_priv: drm file-private structure to register the handle for
* @obj: object to register
* @handlep: pionter to return the created handle to the caller
@@ -371,10 +371,9 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
* to the object, which includes a regular reference count. Callers
* will likely want to dereference the object afterwards.
*/
-int
-drm_gem_handle_create(struct drm_file *file_priv,
- struct drm_gem_object *obj,
- u32 *handlep)
+int drm_gem_handle_create(struct drm_file *file_priv,
+ struct drm_gem_object *obj,
+ u32 *handlep)
{
mutex_lock(&obj->dev->object_name_lock);
--
2.1.3
[PATCH 0/7] drm: Sanitize DRM_IOCTL_MODE_CREATE_DUMB input
From: Thierry Reding Drivers currently treat the IOCTL data for DRM_IOCTL_MODE_CREATE_DUMB inconsistently. While the documentation clearly states that the handle, pitch and size fields are outputs, some drivers use their values as a minimum hint from userspace, presumably to allow userspace to over- allocate buffers on purpose (for example after negotiating pitch alignment requirements with other devices). Most of userspace is well-behaved in that in zeros out the output fields which works well with drivers that use them as minima. However, some userspace (like Mesa's GBM DRI backend) only initializes the inputs, so that the driver will end up using uninitialized data from the stack in the computations. This can cause the driver to attempt very large allocations, resulting in failure, or silently use excessively large buffers. This series tries to fix this by sanitizing the IOCTL data (setting the output fields to 0 in the kernel) so that drivers can no longer use uninitialized data. While at it, it also fixes existing drivers to not treat output fields as input/output for consistency. Note that this is technically breaking ABI since overallocating will no longer be possible after this series is applied. However, the public DRM headers have always documented the fields as being outputs, so any application relying on them being inputs could be considered buggy. The hope is that no such applications exist in the wild. Discussion on IRC lead to the conclusion that new IOCTLs should have input validation and require userspace to zero out output parameters to avoid this kind of mess in the future. In order to help avoid this kind of ambiguity it would be a good idea to start documenting IOCTLs more officially (e.g. in the DRM DocBook). I'm adding a bunch of people on Cc to get feedback about what userspace people know might be relying on the current behaviour. Drivers that are impacted are OMAP, R-Car and any that use GEM CMA helpers. This series also contains a couple of preparatory patches that add the GEM CMA helpers to our DocBook. Thierry Thierry Reding (7): drm/gem: Fix a few kerneldoc typos drm/doc: mm: Fix indentation drm/doc: Add GEM/CMA helpers to kerneldoc drm/cma: Introduce drm_gem_cma_dumb_create_internal() drm/omap: gem: dumb: pitch is an output drm/rcar: gem: dumb: pitch is an output drm: Sanitize DRM_IOCTL_MODE_CREATE_DUMB input Documentation/DocBook/drm.tmpl| 274 +- drivers/gpu/drm/drm_crtc.c| 8 + drivers/gpu/drm/drm_gem.c | 11 +- drivers/gpu/drm/drm_gem_cma_helper.c | 182 +- drivers/gpu/drm/omapdrm/omap_gem.c| 2 +- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 4 +- include/drm/drm_gem_cma_helper.h | 30 +++- 7 files changed, 319 insertions(+), 192 deletions(-) -- 2.1.3
[PATCH 0/7] drm: Sanitize DRM_IOCTL_MODE_CREATE_DUMB input
On Wed, Nov 05, 2014 at 02:25:12PM +0100, Thierry Reding wrote: > Discussion on IRC lead to the conclusion that new IOCTLs should have > input validation and require userspace to zero out output parameters to > avoid this kind of mess in the future. In order to help avoid this kind > of ambiguity it would be a good idea to start documenting IOCTLs more > officially (e.g. in the DRM DocBook). That is a good point, however not everyone can build the documentation. I tried a while back and the whole documentation subsystem is /soo/ fragile its untrue. You have to have the exact right versions installed and hope that the exact right versions have configured themselves to work together correctly, otherwise you get cryptic unsolvable error messages. Tex is notorious for these problems. When I tried to generate the docbook stuff, after spending a significant amount of time trying to debug it, I just gave up completely with any ideas about ever adding to the documentation. I've since decided that I am /never/ going to do anything past adding docbook comments to files. If someone cares about linking them into the rest of the docbook system, and they have the capability to generate the docbook output, then that's fine and dandy, but I believe that requiring everyone to have that capability is asking far too much. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net.
[PULL] topic/core-stuff
Hi Dave, Just various stuff all over from a bunch of people. Shortlog gives a beter overview, it's really all misc drm patches. Cheers, Daniel The following changes since commit 1bcecfacde6269dc6cee9a098bc454222d441ff9: drm/core: use helper to check driver features (2014-10-03 10:38:56 +0200) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/topic/core-stuff-2014-11-05 for you to fetch changes up to babc94936b7a48f0411c85a63074c0e378b55761: drm/edid: add #defines and helpers for ELD (2014-11-05 14:03:44 +0100) Chris Wilson (1): drm: Implement O_NONBLOCK support on /dev/dri/cardN Chuck Ebbert (2): drm/crtc: Fix two typos drm/crtc: Remove duplicated ioctl code Damien Lespiau (3): drm: Add a note to drm_property_create() about property lifetime drm/i915: Don't destroy DRM properties in the driver drm/gma500: Don't destroy DRM properties in the driver Daniel Vetter (1): drm/dp-helper: Move the legacy helpers to gma500 Jani Nikula (1): drm/edid: add #defines and helpers for ELD Joe Perches (1): drm: drm_err: Remove unnecessary __func__ argument Masanari Iida (2): gpu:drm: Fix typo in Documentation/DocBook/drm.xml gpu: drm: Fix warning caused by a parameter description in drm_crtc.c Peter Hurley (2): drm: Fix DRM_FORCE_ON_DIGITAL use drm: Remove compiler BUG_ON() test Rickard Strandqvist (1): gpu: drm: drm_dp_mst_topology.c: Fix improper use of strncat Todd Previte (1): drm/dp: Add counters in the drm_dp_aux struct for I2C NACKs and DEFERs drivers/gpu/drm/drm_crtc.c | 40 +++ drivers/gpu/drm/drm_dp_helper.c | 194 +-- drivers/gpu/drm/drm_dp_mst_topology.c | 15 +-- drivers/gpu/drm/drm_drv.c | 5 +- drivers/gpu/drm/drm_fb_helper.c | 1 - drivers/gpu/drm/drm_fops.c | 12 +- drivers/gpu/drm/drm_irq.c | 4 +- drivers/gpu/drm/drm_modes.c | 2 +- drivers/gpu/drm/drm_prime.c | 4 +- drivers/gpu/drm/drm_probe_helper.c | 3 +- drivers/gpu/drm/gma500/cdv_intel_dp.c | 195 drivers/gpu/drm/gma500/psb_intel_sdvo.c | 49 drivers/gpu/drm/i915/i915_cmd_parser.c | 4 +- drivers/gpu/drm/i915/i915_reg.h | 2 +- drivers/gpu/drm/i915/intel_lrc.c| 2 +- drivers/gpu/drm/i915/intel_sdvo.c | 47 include/drm/drmP.h | 8 +- include/drm/drm_dp_helper.h | 21 +--- include/drm/drm_dp_mst_helper.h | 2 +- include/drm/drm_edid.h | 102 + 20 files changed, 347 insertions(+), 365 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/panel: Flesh out kerneldoc
On Tue, Nov 04, 2014 at 05:26:13PM +0100, Thierry Reding wrote:
> From: Thierry Reding
>
> Write more complete kerneldoc comments for the DRM panel API and
> integrate the helpers in the DRM DocBook reference.
>
> Signed-off-by: Thierry Reding
A few comments below. With thos addressed this is
Reviewed-by: Daniel Vetter
And feel free the ignore the bikeshed one of course ;-)
Cheers, Daniel
> ---
> Documentation/DocBook/drm.tmpl | 6 +
> drivers/gpu/drm/drm_panel.c| 61
> ++
> include/drm/drm_panel.h| 59
> 3 files changed, 126 insertions(+)
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index d89ca5830697..8737f03d9a75 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2379,6 +2379,12 @@ void intel_crt_init(struct drm_device *dev)
>Plane Helper Reference
> !Edrivers/gpu/drm/drm_plane_helper.c Plane Helpers
>
> +
> + Panel Helper Reference
> +!Iinclude/drm/drm_panel.h
> +!Edrivers/gpu/drm/drm_panel.c
> +!Pdrivers/gpu/drm/drm_panel.c drm panel
> +
>
>
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 2ef988e037b7..3dfe3c886502 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -30,12 +30,36 @@
> static DEFINE_MUTEX(panel_lock);
> static LIST_HEAD(panel_list);
>
> +/**
> + * DOC: drm panel
> + *
> + * The DRM panel helpers allow drivers to register panel objects with a
> + * central registry and provide functions to retrieve those panels in display
> + * drivers.
I think a few more words here would be useful.
"The goal is to be able to abstract away panel specific quirks and then
share panel drivers between different display drivers.""
> + */
> +
> +/**
> + * drm_panel_init - initialize a panel
> + * @panel: DRM panel
> + *
> + * Sets up internal fields of the panel so that it can subsequently be added
> + * to the registry.
> + */
> void drm_panel_init(struct drm_panel *panel)
> {
> INIT_LIST_HEAD(&panel->list);
> }
> EXPORT_SYMBOL(drm_panel_init);
>
> +/**
> + * drm_panel_add - add a panel to the global registry
> + * @panel: panel to add
> + *
> + * Add a panel to the global registry so that it can be looked up by display
> + * drivers.
> + *
> + * Return: 0 on success or a negative error code on failure.
Bikeshed: I think the usual layout for return value sections in drm is
* Returns:
* blabla
i.e. with s at the end and newline before the blabla.
> + */
> int drm_panel_add(struct drm_panel *panel)
> {
> mutex_lock(&panel_lock);
> @@ -46,6 +70,12 @@ int drm_panel_add(struct drm_panel *panel)
> }
> EXPORT_SYMBOL(drm_panel_add);
>
> +/**
> + * drm_panel_remove - remove a panel from the global registry
> + * @panel: DRM panel
> + *
> + * Removes a panel from the global registry.
> + */
> void drm_panel_remove(struct drm_panel *panel)
> {
> mutex_lock(&panel_lock);
> @@ -54,6 +84,18 @@ void drm_panel_remove(struct drm_panel *panel)
> }
> EXPORT_SYMBOL(drm_panel_remove);
>
> +/**
> + * drm_panel_attach - attach a panel to a connector
> + * @panel: DRM panel
> + * @connector: DRM connector
> + *
> + * After obtaining a pointer to a DRM panel a display driver calls this
> + * function to attach a panel to a connector.
> + *
> + * An error is returned if the panel is already attached to another
> connector.
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> int drm_panel_attach(struct drm_panel *panel, struct drm_connector
> *connector)
> {
> if (panel->connector)
> @@ -66,6 +108,15 @@ int drm_panel_attach(struct drm_panel *panel, struct
> drm_connector *connector)
> }
> EXPORT_SYMBOL(drm_panel_attach);
>
> +/**
> + * drm_panel_detach - detach a panel from a connector
> + * @panel: DRM panel
> + *
> + * Detaches a panel from the connector it is attached to. If a panel is not
> + * attached to any connector this is effectively a no-op.
> + *
> + * Return: 0 on success or a negative error code on failure.
This never fails. I think it's better to switch the return value to void
instead of documenting something that never actually happens. This kind of
interface polish is one of the main reasons I think doing kerneldoc is
useful
> + */
> int drm_panel_detach(struct drm_panel *panel)
> {
> panel->connector = NULL;
> @@ -76,6 +127,16 @@ int drm_panel_detach(struct drm_panel *panel)
> EXPORT_SYMBOL(drm_panel_detach);
>
> #ifdef CONFIG_OF
> +/**
> + * of_drm_find_panel - look up a panel using a device tree node
> + * @np: device tree node of the panel
> + *
> + * Searches the set of registered panels for one that matches the given
> device
> + * tree node. If a matching panel is found, return a pointer to it.
> + *
> + * Return: A pointer to the panel registered for the specified device tree
> + * node or NULL if no panel matching t
[PATCH 10/17] drm: Atomic crtc/connector updates using crtc/plane helper interfaces
On Sun, Nov 02, 2014 at 02:19:23PM +0100, Daniel Vetter wrote: > So this is finally the integration of the crtc and plane helper > interfaces into the atomic helper functions. > > In the check function we now have a few steps: > > - First we update the output routing and figure out which crtcs need a > full mode set. Suitable encoders are selected using ->best_encoder, > with the same semantics as the crtc helpers of implicitly disabling > all connectors currently using the encoder. > > - Then we pull all other connectors into the state update which feed > from a crtc which changes. This must be done do catch mode changes > and similar updates - atomic updates are differences on top of the > current state. > > - Then we call all the various ->mode_fixup to compute the adjusted > mode. Note that here we have a slight semantic difference compared > to the crtc helpers: We have not yet updated the encoder->crtc link > when calling the encoder's ->mode_fixup function. But that's a > requirement when converting to atomic since we want to prepare the > entire state completely contained with the over drm_atomic_state > structure. So this must be carefully checked when converting drivers > over to atomic helpers. > > - Finally we do call the atomic_check functions on planes and crtcs. > > The commit function is also quite a beast: > > - The only step that can fail is done first, namely pinning the > framebuffers. After that we cross the point of no return, an async > commit would push all that into the worker thread. > > - The disabling of encoders and connectors is a bit tricky, since > depending upon the final state we need to select different crtc > helper functions. > > - Software tracking is a bit clarified compared to the crtc helpers: > We commit the software state before starting to touch the hardware, > like crtc helpers. But since we just swap them we still have the old > state (i.e. the current hw state) around, which is really handy to > write simple disable functions. So no more > drm_crtc_helper_disable_all_unused_functions kind of fun because > we're leaving unused crtcs/encoders behind. Everything gets shut > down in-order now, which is one of the key differences of the i915 > helpers compared to crtc helpers and a really nice additional > guarantee. > > - Like with the plane helpers the atomic commit function waits for one > vblank to pass before calling the framebuffer cleanup function. > > Compared to Rob's helper approach there's a bunch of upsides: > > - All the interfaces which can fail are called in the ->check hook > (i.e. ->best_match and the various ->mode_fixup hooks). This means > that drivers can just reuse those functions and don't need to move > everything into ->atomic_check callbacks. If drivers have no need > for additional constraint checking beyong their existing crtc s/beyong/beyond/ > helper callbacks they don't need to do anything. > > - The actual commit operation is properly stage: First we prepare > framebuffers, which can potentially still fail (due to memory > exhausting). This is important for the async case, where this must > be done synchronously to correctly return errors. > > - The output configuration changes (done with crtc helper functions) > and the plane update (using atomic plane helpers) are correctly > interleaved: First we shut down any crtcs that need changing, then > we update planes and finally we enable everything again. Hardware > without GO bits must be more careful with ordering, which this > sequence enables. > > - Also for hardware with shared output resources (like display PLLs) > we first must shut down the old configuration before we can enable > the new one. Otherwise we can hit an impossible intermediate state > where there's not enough PLLs (which is the point behind atomic > updates). > > v2: > - Ensure that users of ->check update crtc_state->enable correctly. > - Update the legacy state in crtc/plane structures. Eventually we want > to remove that, but for now the drm core still expects this (especially > the plane->fb pointer). > > v3: A few changes for better async handling: > > - Reorder the software side state commit so that it happens all before > we touch the hardware. This way async support becomes very easy > since we can punt all the actual hw touching to a worker thread. And > as long as we synchronize with that thread (flushing or cancelling, > depending upon what the driver can handle) before we commit the next > software state there's no need for any locking in the worker thread > at all. Which greatly simplifies things. > > And as long as we synchronize with all relevant threads we can have > a lot of them (e.g. per-crtc for per-crtc updates) running in > parallel. > > - Expose pre/post plane commit steps separately. We need to expose the > actual hw commit step anyway for drivers to be able to implement > async
[RFC 1/2] core: Add generic object registry implementation
On 11/04/2014 05:29 PM, Thierry Reding wrote:
> From: Thierry Reding
>
> Add a generic implementation of an object registry. This targets drivers
> and subsystems that provide auxiliary objects that other drivers need to
> look up. The goal is to put the difficult parts (keep object references,
> module usage count, ...) into core code so that individual subsystems do
> not have to deal with them.
>
> The intention is for subsystems to instantiate a struct registry and use
> a struct registry_record embedded into a subsystem-specific structure to
> provide a subsystem-specific API around that.
As I understand you want to use this registry for panels and bridges.
Could you explain the idea and describe example scenario when these
refcountings are useful. I guess it should be when panel attached to
drmdrv want to disappear.
Real lifetime of panel is limited by probe/remove callbacks of panel
driver, do you want to prolong it behind these limits?
Do you want to have zombie panels, without hardware they abstract? For
what purpose?
What do you want to do with panel ops? Do they need support both life
states?
Anyway implementation currently seems to be broken, you try to refcount
objects which are usually embedded in driver priv data, which disappears
during remove callback of the driver.
Regards
Andrzej
>
> Signed-off-by: Thierry Reding
> ---
> drivers/base/Makefile| 2 +-
> drivers/base/registry.c | 147
> +++
> include/linux/registry.h | 62
> 3 files changed, 210 insertions(+), 1 deletion(-)
> create mode 100644 drivers/base/registry.c
> create mode 100644 include/linux/registry.h
>
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 53c3fe1aeb29..250262d1af2c 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -4,7 +4,7 @@ obj-y := component.o core.o bus.o dd.o
> syscore.o \
> driver.o class.o platform.o \
> cpu.o firmware.o init.o map.o devres.o \
> attribute_container.o transport_class.o \
> -topology.o container.o property.o
> +topology.o container.o property.o registry.o
> obj-$(CONFIG_DEVTMPFS) += devtmpfs.o
> obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
> obj-y+= power/
> diff --git a/drivers/base/registry.c b/drivers/base/registry.c
> new file mode 100644
> index ..9f510f6237b7
> --- /dev/null
> +++ b/drivers/base/registry.c
> @@ -0,0 +1,147 @@
> +/*
> + * Copyright (C) 2014, NVIDIA Corporation. All rights reserved.
> + *
> + * This file is released under the GPL v2.
> + */
> +
> +#include
> +#include
> +#include
> +
> +static inline struct registry_record *to_registry_record(struct kref *kref)
> +{
> + return container_of(kref, struct registry_record, kref);
> +}
> +
> +static void registry_record_release(struct kref *kref)
> +{
> + struct registry_record *record = to_registry_record(kref);
> +
> + record->release(record);
> +}
> +
> +/**
> + * registry_record_init - initialize a registry record
> + * @record: record to initialize
> + *
> + * Sets up internal fields of the registry record so that it can subsequently
> + * be added to a registry.
> + */
> +void registry_record_init(struct registry_record *record)
> +{
> + INIT_LIST_HEAD(&record->list);
> + kref_init(&record->kref);
> +}
> +
> +/**
> + * registry_record_ref - reference on the registry record
> + * @record: record to reference
> + *
> + * Increases the reference count on the record and returns a pointer to it.
> + *
> + * Return: A pointer to the record on success or NULL on failure.
> + */
> +struct registry_record *registry_record_ref(struct registry_record *record)
> +{
> + if (!record)
> + return NULL;
> +
> + /*
> + * Refuse to give out any more references if the module owning the
> + * record is being removed.
> + */
> + if (!try_module_get(record->owner))
> + return NULL;
> +
> + kref_get(&record->kref);
> +
> + return record;
> +}
> +
> +/**
> + * registry_record_unref - drop a reference to a registry record
> + * @record: record to unreference
> + *
> + * Decreases the reference count on the record. When the reference count
> + * reaches zero the record will be destroyed.
> + */
> +void registry_record_unref(struct registry_record *record)
> +{
> + if (record) {
> + /*
> + * Keep a copy of the module owner since the record may
> + * disappear during the kref_put().
> + */
> + struct module *owner = record->owner;
> +
> + kref_put(&record->kref, registry_record_release);
> + module_put(owner);
> + }
> +}
> +
> +/**
> + * registry_add - add a record to a registry
> + * @registry: registry to add the record to
> + * @record: record to add
> + *
> + * Tr
[PATCH V2 1/2] imx-drm: imx-hdmi: split imx soc specific code from imx-hdmi
This one patch does too much to be reviewed easily.
One patch is supposed to modify/add one thing at a time in the kernel.
Separating platform specific code from imx-drm/imx-hdmi is one thing.
Adding support for multi-byte register access is something different.
i.e. Something like.
1/3 split platform specific code out.
2/3 move/rename imx-hdmi outside the folder
3/3 add support for multi byte register width access.
If there are other things that are not directly relevant to the patch,
it goes in a different patch. Bug fixes are also separate.
This should result in readable patches that can be reviewed easily.
Also, the approach with 4 byte access is ok. But you could use reg-shifts as
well perhaps.
Then you won't have to change so much of the code.
static inline void hdmi_writeb(struct dwc_hdmi *hdmi, u8 val, int offset)
+{
+ writeb(val, hdmi->regs + (offset << hdmi->reg_shift));
+}
+
+static inline u8 hdmi_readb(struct dwc_hdmi *hdmi, int offset)
+{
+ return readb(hdmi->regs + (offset << hdmi->reg_shift));
+}
And then in probe
+hdmi->reg_shift = 0;
+
+ if (of_property_read_u32(np, "reg-shift", &hdmi->reg_shift))
+ dev_warn(hdmi->dev, "No reg-shift\n");
This way the reg-shift property can be defined using DT
Cheers,
ZubairLK
On 05/11/14 12:59, Andy Yan wrote:
> imx6 and rockchip rk3288 and JZ4780 (Ingenic Xburst/MIPS)
> use the interface compatible Designware HDMI IP, but they
> also have some lightly difference, such as phy pll configuration,
> register width(imx hdmi register is one byte, but rk3288 is 4
> bytes width), 4K support(imx6 doesn't support 4k, but rk3288 does),
> clk useage,and the crtc mux configuration is also platform specific.
>
> To reuse the imx hdmi driver, split the platform specific code out
> to dw_hdmi-imx.c.
>
> Signed-off-by: Andy Yan
> ---
> drivers/staging/imx-drm/Makefile | 2 +-
> drivers/staging/imx-drm/dw_hdmi-imx.c | 214 ++
> drivers/staging/imx-drm/imx-hdmi.c| 726
> ++
> include/drm/bridge/dw_hdmi.h | 114 ++
> 4 files changed, 634 insertions(+), 422 deletions(-)
> create mode 100644 drivers/staging/imx-drm/dw_hdmi-imx.c
> create mode 100644 include/drm/bridge/dw_hdmi.h
i915/dri hang
I filed this with bugzilla @ kernel.org (87741) Not sure if I need to send here. Long running stable systems exhibit the following with all mods removed, as distributed. (I use my own scheduler and file system - the kernels below use the stock ext4 file system): Kernel Command line: BOOT_IMAGE=/boot/vmlinuz-3.18.0-0-reaper root=UUID=8536097e-c02a-4a8c-9cdf-b40ba4e3b74d ro crashkernel=384M-2G:64M,2G-:128M drm_kms_helper.edid_firmware=edid/1280x1024_75.bin thermal.crt=75 thermal.nocrt=70 quiet splash vt.handoff=7 CPU: Intel(R) Pentium(R) D CPU 3.40GHz (fam: 0f, model: 06, stepping: 04) Memory: 2G MB DMI: ECS 945GCT-M2/945GCT-M2, BIOS 080012 07/18/2008 Intel graphics stolen memory is: 0x7f80-0x7fff V3.18~rc1 will reward you with the following upon initiation of an OpenGL consumer: Oct 31 12:28:43 Aesop kernel: [ 653.551243] [ cut here ] Oct 31 12:28:43 Aesop kernel: [ 653.551334] WARNING: CPU: 0 PID: 3766 at /home/jim/software/ubuntu/linux-3.18-rc1/drivers/gpu/drm/i915/intel_display.c:9917 intel_check_page_flip+0xb8/0xc1 [i915]() Oct 31 12:28:43 Aesop kernel: [ 653.551337] Kicking stuck page flip: queued at 48001, now 48002 Oct 31 12:28:43 Aesop kernel: [ 653.551340] Modules linked in: snd_hrtimer cpufreq_conservative cpufreq_powersave nf_log_ipv4 nf_log_common xt_tcpudp ip6table_mangle iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat xt_TCPMSS xt_LOG ipt_REJECT iptable_mangle xt_multiport xt_state xt_limit xt_conntrack nf_conntrack_ftp nf_conntrack ip6table_filter ip6_tables iptable_filter ip_tables x_tables bnep rfcomm lp bluetooth ipv6 arc4 rt2800usb rt2800lib crc_ccitt rt2x00usb rt2x00lib mac80211 gspca_zc3xx gspca_main cfg80211 videodev snd_hda_codec_idt snd_hda_codec_generic snd_hda_intel snd_hda_controller snd_hda_codec snd_hwdep snd_pcm_oss snd_mixer_oss snd_pcm snd_seq_dummy snd_seq_oss snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq snd_timer gpio_ich snd_seq_device ppdev snd soundcore serio_raw parport_pc parport i915 drm_kms_helper lpc_ich mfd_core it87 hwmon_vid 8139too 8139cp mii Oct 31 12:28:43 Aesop kernel: [ 653.551404] CPU: 0 PID: 3766 Comm: balazar_brother Not tainted 3.18.0-0-reaper #1~rc1 Oct 31 12:28:43 Aesop kernel: [ 653.551408] Hardware name: ECS 945GCT-M2/945GCT-M2, BIOS 080012 07/18/2008 Oct 31 12:28:43 Aesop kernel: [ 653.551411] 26bd 88007f403b90 816730f0 0007 Oct 31 12:28:43 Aesop kernel: [ 653.551415] 88007f403be0 88007f403bd0 8104a027 88007f403c00 Oct 31 12:28:43 Aesop kernel: [ 653.551419] 8800378ca008 88007be17858 0082 88007be176b8 Oct 31 12:28:43 Aesop kernel: [ 653.551423] Call Trace: Oct 31 12:28:43 Aesop kernel: [ 653.551426] [] dump_stack+0x46/0x58 Oct 31 12:28:43 Aesop kernel: [ 653.551443] [] warn_slowpath_common+0x81/0x9f Oct 31 12:28:43 Aesop kernel: [ 653.551447] [] warn_slowpath_fmt+0x46/0x48 Oct 31 12:28:43 Aesop kernel: [ 653.551478] [] intel_check_page_flip+0xb8/0xc1 [i915] Oct 31 12:28:43 Aesop kernel: [ 653.551502] [] i915_handle_vblank+0x53/0xab [i915] Oct 31 12:28:43 Aesop kernel: [ 653.551509] [] ? usb_submit_urb.part.8+0x1f1/0x559 Oct 31 12:28:43 Aesop kernel: [ 653.551534] [] i915_irq_handler+0x236/0x375 [i915] Oct 31 12:28:43 Aesop kernel: [ 653.551540] [] ? check_preempt_wakeup+0x119/0x1bc Oct 31 12:28:43 Aesop kernel: [ 653.551545] [] handle_irq_event_percpu+0x56/0x1b4 Oct 31 12:28:43 Aesop kernel: [ 653.551548] [] handle_irq_event+0x3a/0x61 Oct 31 12:28:43 Aesop kernel: [ 653.551553] [] handle_fasteoi_irq+0x7a/0xdc Oct 31 12:28:43 Aesop kernel: [ 653.551558] [] handle_irq+0x22/0x3c Oct 31 12:28:43 Aesop kernel: [ 653.551562] [] do_IRQ+0x53/0xf0 Oct 31 12:28:43 Aesop kernel: [ 653.551568] [] common_interrupt+0x6a/0x6a Oct 31 12:28:43 Aesop kernel: [ 653.551572] [] ? lapic_next_event+0x1d/0x21 Oct 31 12:28:43 Aesop kernel: [ 653.551576] [] ? __do_softirq+0x80/0x2a2 Oct 31 12:28:43 Aesop kernel: [ 653.551580] [] ? __do_softirq+0x20/0x2a2 Oct 31 12:28:43 Aesop kernel: [ 653.551583] [] irq_exit+0x7e/0x9b Oct 31 12:28:43 Aesop kernel: [ 653.551587] [] smp_apic_timer_interrupt+0x4a/0x5a Oct 31 12:28:43 Aesop kernel: [ 653.551590] [] apic_timer_interrupt+0x6a/0x70 Oct 31 12:28:43 Aesop kernel: [ 653.551592] [] ? schedule+0x29/0x5e Oct 31 12:28:43 Aesop kernel: [ 653.551599] [] ? _raw_spin_unlock_irqrestore+0xe/0x10 Oct 31 12:28:43 Aesop kernel: [ 653.551606] [] slob_free+0x129/0x485 Oct 31 12:28:43 Aesop kernel: [ 653.551612] [] ? skb_release_data+0xc5/0x13d Oct 31 12:28:43 Aesop kernel: [ 653.551615] [] kfree+0xde/0xed Oct 31 12:28:43 Aesop kernel: [ 653.551619] [] skb_release_data+0xc5/0x13d Oct 31 12:28:43 Aesop kernel: [ 653.551623] [] __kfree_skb+0x28/0x37 Oct 31 12:28:43 Aesop kernel: [ 653.551626] [] consume_skb+0x30/0x7c Oct 31 12:28:43 Aesop kernel: [ 653.551633] [] unix_stream_recvmsg+0x3c5/0x6cd Oct 31 12:28:43 Aesop kernel: [ 653.551639]
[PATCH 09/17] drm/crtc-helper: Transitional functions using atomic plane helpers
On Sun, Nov 02, 2014 at 02:19:22PM +0100, Daniel Vetter wrote:
> These two functions allow drivers to reuse their atomic plane helpers
> functions for the primary plane to implement the interfaces required
> by the crtc helpers for the legacy ->set_config callback.
>
> This is purely transitional and won't be used once the driver is fully
> converted. But it allows partial conversions to the atomic plane
> helpers which are functional.
>
> v2:
> - Use ->atomic_duplicate_state if available.
> - Don't forget to run crtc_funcs->atomic_check.
>
> v3: Shift source coordinates correctly for 16.16 fixed point.
>
> v4: Don't forget to call ->atomic_destroy_state if available.
>
> v5: Fixup kerneldoc.
>
> v6: Reuse the plane_commit function from the transitional plane
> helpers to avoid too much duplication.
>
> v7:
> - Remove some stale comment.
> - Correctly handle the lack of plane->state object, necessary for
> transitional use.
>
> v8: Fixup an embarrassing h/vdisplay mixup.
>
> Signed-off-by: Daniel Vetter
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/drm_crtc_helper.c | 110
> +
> drivers/gpu/drm/drm_plane_helper.c | 10 ++--
> include/drm/drm_crtc.h | 4 ++
> include/drm/drm_crtc_helper.h | 7 +++
> include/drm/drm_plane_helper.h | 4 ++
> 5 files changed, 130 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> b/drivers/gpu/drm/drm_crtc_helper.c
> index 6c65a0a28fbd..95ecbb131053 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -38,6 +38,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> MODULE_AUTHOR("David Airlie, Jesse Barnes");
> @@ -888,3 +889,112 @@ void drm_helper_resume_force_mode(struct drm_device
> *dev)
> drm_modeset_unlock_all(dev);
> }
> EXPORT_SYMBOL(drm_helper_resume_force_mode);
> +
> +/**
> + * drm_helper_crtc_mode_set - mode_set implementation for atomic plane
> helpers
> + * @crtc: DRM CRTC
> + * @mode: DRM display mode which userspace requested
> + * @adjusted_mode: DRM display mode adjusted by ->mode_fixup callbacks
> + * @x: x offset of the CRTC scanout area on the underlying framebuffer
> + * @y: y offset of the CRTC scanout area on the underlying framebuffer
> + * @old_fb: previous framebuffer
> + *
> + * This function implements a callback useable as the ->mode_set callback
> + * required by the crtc helpers. Besides the atomic plane helper functions
> for
> + * the primary plane the driver must also provide the ->mode_set_nofb
> callback
> + * to set up the crtc.
> + *
> + * This is a transitional helper useful for converting drivers to the atomic
> + * interfaces.
> + */
> +int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode
> *mode,
> + struct drm_display_mode *adjusted_mode, int x, int y,
> + struct drm_framebuffer *old_fb)
> +{
> + struct drm_crtc_state *crtc_state;
> + struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
> + int ret;
> +
> + if (crtc->funcs->atomic_duplicate_state)
> + crtc_state = crtc->funcs->atomic_duplicate_state(crtc);
> + else if (crtc->state)
> + crtc_state = kmemdup(crtc->state, sizeof(*crtc_state),
> + GFP_KERNEL);
> + else
> + crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> + if (!crtc_state)
> + return -ENOMEM;
> +
> + crtc_state->enable = true;
> + crtc_state->planes_changed = true;
> + drm_mode_copy(&crtc_state->mode, mode);
> + drm_mode_copy(&crtc_state->adjusted_mode, adjusted_mode);
> +
> + if (crtc_funcs->atomic_check) {
> + ret = crtc_funcs->atomic_check(crtc, crtc_state);
> + if (ret) {
> + kfree(crtc_state);
> +
> + return ret;
> + }
> + }
> +
> + swap(crtc->state, crtc_state);
> +
> + crtc_funcs->mode_set_nofb(crtc);
> +
> + if (crtc_state) {
> + if (crtc->funcs->atomic_destroy_state)
> + crtc->funcs->atomic_destroy_state(crtc, crtc_state);
> + else
> + kfree(crtc_state);
> + }
> +
> + return drm_helper_crtc_mode_set_base(crtc, x, y, old_fb);
> +}
> +EXPORT_SYMBOL(drm_helper_crtc_mode_set);
> +
> +/**
> + * drm_helper_crtc_mode_set_base - mode_set_base implementation for atomic
> plane helpers
> + * @crtc: DRM CRTC
> + * @x: x offset of the CRTC scanout area on the underlying framebuffer
> + * @y: y offset of the CRTC scanout area on the underlying framebuffer
> + * @old_fb: previous framebuffer
> + *
> + * This function implements a callback useable as the ->mode_set_base used
> + * required by the crtc helpers. The driver must provide the atomic plane
> helper
> + * functions for the primary plane.
> + *
> + * This is a transitional helper useful for converting drivers to the atomic
> + * interfaces.
> + */
> +int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
> + struct drm_framebuffer *old_fb)
> +{
> + struct drm_plane_state *plane_state;
> + struct drm_plane *plane = crtc->primary;
> +
> + if (plane->funcs->atomic_duplicate_state)
> + plane_state = plane->funcs->atomic_dup
[PATCH 2/2] move imx-hdmi to bridge/dw-hdmi
On Tue, Nov 04, 2014 at 09:39:58PM +0800, Andy Yan wrote: > From: Andy yan Leave this out. It's for when you are forwarding a patch from someone else. Also it has a typo and your email From: header is correct. > > the original imx hdmi driver is under staging/imx-drm, > which depends on imx-drm, so move the imx hdmi drvier out > to drm/bridge and rename imx-hdmi to dw-hdmi > No Signed-off-by: line. regards, dan carpenter
[PATCH 08/17] drm/plane-helper: transitional atomic plane helpers
On Sun, Nov 02, 2014 at 02:19:21PM +0100, Daniel Vetter wrote:
> Converting a driver to the atomic interface can be a daunting
> undertaking. One of the prerequisites is to have full universal planes
> support.
>
> To make that transition a bit easier this pathc provides plane helpers
s/pathc/patch/
> which use the new atomic helper callbacks just only for the plane
> changes. This way the plane update functionality can be tested without
> being forced to convert everything at once.
>
> Of course a real atomic update capable driver will implement the
> all plane properties through the atomic interface, so these helpers
> are mostly transitional. But they can be used to enable proper
> universal plane support, especially once the crtc helpers have also
> been adapted.
>
> v2: Use ->atomic_duplicate_state if available.
>
> v3: Don't forget to call ->atomic_destroy_state if available.
>
> v4: Fixup kerneldoc, reported by Paulo.
>
> v5: Extract a common plane_commit helper and fix some bugs in the
> plane_state setup of the plane_disable implementation.
>
> v6: Fix issues with the cleanup of the old fb. Since transitional
> helpers can be mixed we need to assume that the old fb has been set up
> by a legacy path (e.g. set_config or page_flip when the primary plane
> is converted to use these functions already). Hence pass an additional
> old_fb parameter to plane_commit to do that cleanup work correctly.
>
> v7:
> - Fix spurious WARNING (crtc helpers really love to disable stuff
> harder) and fix array index bonghits.
> - Correctly handle the lack of plane->state object, necessary for
> transitional use.
> - Don't indicate failure if drm_vblank_get doesn't work - that's
> expected when the pipe is in dpms off mode.
>
> Cc: Paulo Zanoni
> Signed-off-by: Daniel Vetter
Just a few nits to prove that I read the whole thing ;-).
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/drm_plane_helper.c | 172
> -
> include/drm/drm_plane_helper.h | 8 ++
> 2 files changed, 179 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_plane_helper.c
> b/drivers/gpu/drm/drm_plane_helper.c
> index 827ec1a3040b..45aa8c98e3fb 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -27,7 +27,7 @@
> #include
> #include
> #include
> -#include
> +#include
>
> #define SUBPIXEL_MASK 0x
>
> @@ -369,3 +369,173 @@ int drm_crtc_init(struct drm_device *dev, struct
> drm_crtc *crtc,
> return drm_crtc_init_with_planes(dev, crtc, primary, NULL, funcs);
> }
> EXPORT_SYMBOL(drm_crtc_init);
> +
> +static int
> +plane_commit(struct drm_plane *plane, struct drm_plane_state *plane_state,
> + struct drm_framebuffer *old_fb)
> +{
> + struct drm_plane_helper_funcs *plane_funcs;
> + struct drm_crtc *crtc[2];
> + struct drm_crtc_helper_funcs *crtc_funcs[2];
> + int i, ret = 0;
> +
> + plane_funcs = plane->helper_private;
> +
> + /* Since this is a transitional helper we can't assume that plane->state
> + * is always valid. Hence we need to use plane->crtc instead of
> + * plane->state->crtc as the old crtc. */
> + crtc[0] = plane->crtc;
> + crtc[1] = crtc[0] != plane_state->crtc ? plane_state->crtc : NULL;
> +
> + for (i = 0; i < 2; i++)
> + crtc_funcs[i] = crtc[i] ? crtc[i]->helper_private : NULL;
> +
> + if (plane_funcs->atomic_check) {
> + ret = plane_funcs->atomic_check(plane, plane_state);
> + if (ret)
> + goto fail;
> + }
> +
> + if (plane_funcs->prepare_fb && plane_state->fb) {
> + ret = plane_funcs->prepare_fb(plane, plane_state->fb);
> + if (ret)
> + goto fail;
> + }
> +
> + /* Point of no return, commit sw state. */
> + swap(plane->state, plane_state);
> +
> + for (i = 0; i < 2; i++) {
> + if (crtc_funcs[i] && crtc_funcs[i]->atomic_begin)
> + crtc_funcs[i]->atomic_begin(crtc[i]);
> + }
> +
> + plane_funcs->atomic_update(plane);
> +
> + for (i = 0; i < 2; i++) {
> + if (crtc_funcs[i] && crtc_funcs[i]->atomic_flush)
> + crtc_funcs[i]->atomic_flush(crtc[i]);
> + }
> +
> + for (i = 0; i < 2; i++) {
> + if (!crtc[i])
> + continue;
> +
> + /* There's no other way to figure out whether the crtc is running. */
> + ret = drm_crtc_vblank_get(crtc[i]);
> + if (ret == 0) {
> + drm_crtc_wait_one_vblank(crtc[i]);
> + drm_crtc_vblank_put(crtc[i]);
> + }
This will be good motivation for driver writers to convert to universal
planes!
> +
> + ret = 0;
> + }
> +
> + if (plane_funcs->cleanup_fb && old_fb)
> + plane_funcs->cleanup_fb(plane, old_fb);
> +fail:
minor nit: Might be better to rename this "out", so it's clear that it's
intended to run even in the successful case.
> + if (plane_state) {
> + if (plane->funcs->atomic_destroy_state)
> + plane->funcs->atomic_destroy_state(plane, plane_state);
> + else
> + kfree(plane_state);
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * drm_plane_helper_update() - Helper for primary plane update
> + * @plane: plane object to update
> + * @crtc: owning CRTC of owning plane
> + * @fb: framebuf
[PATCH] drm/atomic-helper: implement ->page_flip
On 04/11/14 22:09, Daniel Vetter wrote:
> Currently there is no way to implement async flips using atomic, that
> essentially requires us to be able to cancel pending requests
> mid-flight.
>
> To be able to do that (and I guess we want this since vblank synced
> updates whic opportunistically cancel still pending updates seem to be
> wanted) we'd need to add a mandatory cancellation mode. Depending upon
> the exact semantics we decide upon that could mean that userspace will
> not get completion events, or will get them all stacked up.
>
> So reject async updates for now. Also async updates usually means not
> vblank synced at all, and I guess for drivers which want to support
> this they should simply add a special pageflip handler (since usually
> you need a special flip cmd to achieve this). That kind of async flip
> is pretty much exclusively just used for games and benchmarks where
> dropping just one frame means you'll get a headshot or something bad
> like that ... And so slight amounts of tearing is acceptable.
>
> v2: Fixup kerneldoc, reported by Paulo.
>
> v3: Use the set_crtc_for_plane function to assign the crtc, since
> otherwise the book-keeping is off.
>
> v4: Update crtc->primary->fb since ->page_flip is the only driver
> callback where the core won't do this itself. We might want to fix
> this inconsistency eventually.
>
> v5: Use set_crtc_for_connector as suggested by Sean.
>
> Cc: Sean Paul
> Cc: Paulo Zanoni
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 92
> +
> include/drm/drm_atomic_helper.h | 5 ++
> 2 files changed, 97 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 790edc18b2cb..36ccc42d43db 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1633,3 +1633,95 @@ backoff:
> goto retry;
> }
> EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
> +
> +/**
> + * drm_atomic_helper_page_flip - execute a legacy page flip
> + * @crtc: DRM crtc
> + * @fb: DRM framebuffer
> + * @event: optional DRM event to signal upon completion
> + * @flags: flip flags for non-vblank sync'ed updates
> + *
> + * Provides a default page flip implementation using the atomic driver
> interface.
> + *
> + * Note that for now so called async page flips (i.e. updates which are not
> + * synchronized to vblank) are not supported, since the atomic interfaces
> have
> + * no provisions for this yet.
> + *
> + * Returns:
> + * Returns 0 on success, negative errno numbers on failure.
> + */
> +int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
> + struct drm_framebuffer *fb,
> + struct drm_pending_vblank_event *event,
> + uint32_t flags)
> +{
> + struct drm_plane *plane = crtc->primary;
> + struct drm_atomic_state *state;
> + struct drm_plane_state *plane_state;
> + struct drm_crtc_state *crtc_state;
> + int ret = 0;
> +
> + if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> + return -EINVAL;
> +
> + state = drm_atomic_state_alloc(plane->dev);
> + if (!state)
> + return -ENOMEM;
> +
> + state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> +retry:
> + crtc_state = drm_atomic_get_crtc_state(state, crtc);
> + if (IS_ERR(crtc_state)) {
> + ret = PTR_ERR(crtc_state);
> + if (ret == -EDEADLK)
> + goto backoff;
> + else
> + goto fail;
> + }
> + crtc_state->event = event;
> +
> + plane_state = drm_atomic_get_plane_state(state, plane);
> + if (IS_ERR(plane_state)) {
> + ret = PTR_ERR(plane_state);
> + if (ret == -EDEADLK)
> + goto backoff;
> + else
> + goto fail;
> + }
> +
> + ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> + if (ret == -EDEADLK)
> + goto backoff;
The kerneldoc for this function states that it can return -ENOMEM too
and, even if it didn't, this approach to error handling is potentially
fragile as the kernel changes.
> + plane_state->fb = fb;
> +
> + ret = drm_atomic_async_commit(state);
> + if (ret == -EDEADLK)
> + goto backoff;
A fussy point but this would be easier to read if it followed the same
pattern as the above error checks with a "goto fail". The the success
path below can be outdented and made more obvious.
> +
> + /* Driver takes ownership of state on successful async commit. */
> + if (ret == 0) {
> + /* TODO: ->page_flip is the only driver callback where the core
> + * doesn't update plane->fb. For now patch it up here. */
> + plane->fb = plane->state->fb;
> +
> + return 0;
> + }
> +
> +fail:
> + drm_atomic_state_free(state);
> +
> + retu
[PATCH] drm: drop README.drm, ancient scrolls
From: Dave Airlie This stuff is ancient, we have docs now in the kernel, lets just drop it. Pointed out by Glenn Signed-off-by: Dave Airlie --- drivers/gpu/drm/README.drm | 43 --- 1 file changed, 43 deletions(-) delete mode 100644 drivers/gpu/drm/README.drm diff --git a/drivers/gpu/drm/README.drm b/drivers/gpu/drm/README.drm deleted file mode 100644 index b5b3327..000 --- a/drivers/gpu/drm/README.drm +++ /dev/null @@ -1,43 +0,0 @@ - -* For the very latest on DRI development, please see: * -* http://dri.freedesktop.org/ * - - -The Direct Rendering Manager (drm) is a device-independent kernel-level -device driver that provides support for the XFree86 Direct Rendering -Infrastructure (DRI). - -The DRM supports the Direct Rendering Infrastructure (DRI) in four major -ways: - -1. The DRM provides synchronized access to the graphics hardware via - the use of an optimized two-tiered lock. - -2. The DRM enforces the DRI security policy for access to the graphics - hardware by only allowing authenticated X11 clients access to - restricted regions of memory. - -3. The DRM provides a generic DMA engine, complete with multiple - queues and the ability to detect the need for an OpenGL context - switch. - -4. The DRM is extensible via the use of small device-specific modules - that rely extensively on the API exported by the DRM module. - - -Documentation on the DRI is available from: -http://dri.freedesktop.org/wiki/Documentation -http://sourceforge.net/project/showfiles.php?group_id=387 -http://dri.sourceforge.net/doc/ - -For specific information about kernel-level support, see: - -The Direct Rendering Manager, Kernel Support for the Direct Rendering -Infrastructure -http://dri.sourceforge.net/doc/drm_low_level.html - -Hardware Locking for the Direct Rendering Infrastructure -http://dri.sourceforge.net/doc/hardware_locking_low_level.html - -A Security Analysis of the Direct Rendering Infrastructure -http://dri.sourceforge.net/doc/security_low_level.html -- 1.9.3
[RFC 1/2] core: Add generic object registry implementation
On Tue, Nov 04, 2014 at 08:38:45AM -0800, Greg Kroah-Hartman wrote:
> On Tue, Nov 04, 2014 at 05:29:27PM +0100, Thierry Reding wrote:
[...]
> > diff --git a/drivers/base/registry.c b/drivers/base/registry.c
[...]
> > +/**
> > + * registry_record_ref - reference on the registry record
> > + * @record: record to reference
> > + *
> > + * Increases the reference count on the record and returns a pointer to it.
> > + *
> > + * Return: A pointer to the record on success or NULL on failure.
> > + */
> > +struct registry_record *registry_record_ref(struct registry_record *record)
> > +{
> > + if (!record)
> > + return NULL;
> > +
> > + /*
> > +* Refuse to give out any more references if the module owning the
> > +* record is being removed.
> > +*/
> > + if (!try_module_get(record->owner))
> > + return NULL;
> > +
> > + kref_get(&record->kref);
>
> You "protect" from a module being removed, but not from someone else
> releasing the kref at the same time. Where is the lock that prevents
> this from happening?
You're right, we need a record lock to serialize ref/unref.
> And do we really care about module reference counts here?
We need this so that the code of the .release() callback stays in
memory as long as there are any references.
Also we need the module reference count for registries because they
would typically be statically allocated and go away along with a module
when it is unloaded.
> > +/**
> > + * registry_add - add a record to a registry
> > + * @registry: registry to add the record to
> > + * @record: record to add
> > + *
> > + * Tries to increase the reference count of the module owning the
> > registry. If
> > + * successful adds the new record to the registry.
> > + *
> > + * Return: 0 on success or a negative error code on failure.
> > + */
> > +int registry_add(struct registry *registry, struct registry_record *record)
> > +{
> > + if (!try_module_get(registry->owner))
> > + return -ENODEV;
> > +
> > + mutex_lock(®istry->lock);
> > + list_add_tail(&record->list, ®istry->list);
> > + mutex_unlock(®istry->lock);
>
> No incrementing of the reference of the record at all?
I'm not sure if we really need one. Drivers will have to remove the
record from the registry before dropping their reference. I guess we
could throw in another kref_get() here (and a kref_put() in
registry_remove()) for good measure to prevent breakage with buggy
drivers.
> You seem to be using 2 reference counts for the record / registry, a
> module count, and a kref count, which can cause confusion...
There is one reference count (kref actually) per registry record and one
module count for the record owner and the registry owner.
Can you elaborate what you think is confusing? Perhaps I can add more
kerneldoc to clarify.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/00a5f89f/attachment.sig>
[PATCH 0/2] make imx hdmi publicly used by dw hdmi compatible platform
On 2014å¹´11æ04æ¥ 22:29, Russell King - ARM Linux wrote: > On Tue, Nov 04, 2014 at 09:33:10PM +0800, Andy Yan wrote: >> From: Andy yan >> >> We found freescale imx6 and rockchip rk3288 and Ingenic JZ4780 (Xburst/MIPS) >> use the interface compatible Designware HDMI IP, but they also have some >> lightly difference, such as phy pll configuration, register width(imx hdmi >> register is one byte, but rk3288 is 4 bytes width and can only access by >> word), >> 4K support(imx6 doesn't support 4k, but rk3288 does). >> >> To reuse the imx-hdmi driver, we do this patch set: >> patch (1): split out imx-soc code from imx-hdmi to dw_hdmi-imx.c >> patch (2): move imx-hdmi to bridge/, and rename to dw-hdmi to >> make this driver indepent of drm-imx . And we will add rockchip >> platform specific code dw_hdmi-rockchip.c later, this is depend >> on drm-rockchip. > Great - I fully agree that this needs to be renamed as it is a Designware > IP. > > Should it be moved into bridge/ ? It isn't implemented as a DRM bridge > driver at present, so this seems illogical. Is the longer term plan to > convert it to be a DRM bridge? We have plan to convert it to DRM bridge. > > Secondly, if you want HDMI audio support, you may find the patches I > maintain for the SolidRun devices useful, which add this as a standard > ALSA device. I also have CEC support for it as well. If you're > interested, I'll email those. > It's very great if you email those, we are also working on audio and CEC.
i915/dri hang
On Wed, 05 Nov 2014, Jim McDevitt wrote: > I filed this with bugzilla @ kernel.org (87741) Not sure if I need to > send here. Thanks for the report; for drm/i915 we do track the bugs at kernel.org and freedesktop.org bugzillas, so let's have the follow-ups in https://bugzilla.kernel.org/show_bug.cgi?id=87741 Thanks, Jani. -- Jani Nikula, Intel Open Source Technology Center
[Bug 85647] Random radeonsi crashes with mesa 10.3.x
https://bugs.freedesktop.org/show_bug.cgi?id=85647 --- Comment #14 from Michel Dänzer --- Created attachment 108943 --> https://bugs.freedesktop.org/attachment.cgi?id=108943&action=edit radeonsi: Disable asynchronous DMA Does this patch avoid the lockups? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/da24c2b8/attachment.html>
[PATCH 2/2] move imx-hdmi to bridge/dw-hdmi
Hi ZubairLK: On 2014å¹´11æ04æ¥ 21:50, Zubair Lutfullah Kakakhel wrote: > Hi, > > On 04/11/14 13:39, Andy Yan wrote: >> From: Andy yan >> >> the original imx hdmi driver is under staging/imx-drm, >> which depends on imx-drm, so move the imx hdmi drvier out >> to drm/bridge and rename imx-hdmi to dw-hdmi >> >> Change-Id: I5f417372f256aa26cd00a3cd0160741680afd39d >> --- >> drivers/gpu/drm/bridge/Kconfig|5 + >> drivers/gpu/drm/bridge/Makefile |1 + >> drivers/gpu/drm/bridge/dw_hdmi.c | 1651 >> + >> drivers/gpu/drm/bridge/dw_hdmi.h | 1032 + >> drivers/staging/imx-drm/Kconfig |1 + >> drivers/staging/imx-drm/Makefile |2 +- >> drivers/staging/imx-drm/dw_hdmi-imx.c | 66 +- >> drivers/staging/imx-drm/imx-hdmi.c| 1651 >> - >> drivers/staging/imx-drm/imx-hdmi.h| 1032 - >> include/drm/bridge/dw_hdmi.h | 26 +- >> 10 files changed, 2737 insertions(+), 2730 deletions(-) >> create mode 100644 drivers/gpu/drm/bridge/dw_hdmi.c >> create mode 100644 drivers/gpu/drm/bridge/dw_hdmi.h >> delete mode 100644 drivers/staging/imx-drm/imx-hdmi.c >> delete mode 100644 drivers/staging/imx-drm/imx-hdmi.h >> > > Could you try generating this patch with the -M option? > > git format-patch should then understand its a rename. > Then it should generate a smaller patch that is possible to review easily. > > Thanks > ZubairLK > > > ok, I will do it .
[Bug 85647] Random radeonsi crashes with mesa 10.3.x
https://bugs.freedesktop.org/show_bug.cgi?id=85647 --- Comment #13 from Hannu --- Created attachment 108940 --> https://bugs.freedesktop.org/attachment.cgi?id=108940&action=edit journalctl -b after crash Got something new, I updated xserver to 1.16.1.901-1 and linux to 3.18.0-rc3. Crashed same way as before. After crash no error message in Xorg.0.log but found this with journalctl -b: Nov 05 10:24:10 kernel: radeon :01:00.0: ring 3 stalled for more than 10020msec Nov 05 10:24:10 kernel: radeon :01:00.0: GPU lockup (current fence id 0x2757 last fence id 0x2788 on ring 3) Nov 05 10:24:10 kernel: radeon :01:00.0: Saved 2028 dwords of commands on ring 0. Nov 05 10:24:10 kernel: radeon :01:00.0: GPU softreset: 0x006C Nov 05 10:24:10 kernel: radeon :01:00.0: GRBM_STATUS = 0xA0003028 Nov 05 10:24:10 kernel: radeon :01:00.0: GRBM_STATUS_SE0 = 0x0006 and so on. Relevant parts attached, error message at end. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/a80ce9b8/attachment.html>
[PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld()
Reviewed-by: Rodrigo Vivi
On Tue, Oct 28, 2014 at 7:20 AM, Jani Nikula wrote:
> The Baseline_ELD_Len field does not include ELD Header Block size.
>
> From High Definition Audio Specification, Revision 1.0a:
>
> The header block is a fixed size of 4 bytes. The baseline block
> is variable size in multiple of 4 bytes, and its size is defined
> in the header block Baseline_ELD_Len field (in number of
> DWords).
>
> Do not include the header size in Baseline_ELD_Len field. Fix all known
> users of eld[2].
>
> While at it, switch to DIV_ROUND_UP instead of open coding it.
>
> Signed-off-by: Jani Nikula
>
> ---
>
> This is based on an audio rework series which is mid-way being merged to
> i915. I don't think this should be cc: stable worthy, as, AFAICT, we
> don't use the vendor block, and anyone reading SADs respecting SAD_Count
> should stop at the same offset regardless of this patch. So I propose
> this gets eventually merged via i915 without a rush.
> ---
> drivers/gpu/drm/drm_edid.c | 7 +--
> drivers/gpu/drm/i915/intel_audio.c | 16
> drivers/gpu/drm/nouveau/nv50_display.c | 3 ++-
> 3 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3bf999134bcc..45aaa6f5ef36 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3128,9 +3128,12 @@ void drm_edid_to_eld(struct drm_connector *connector,
> struct edid *edid)
> }
> }
> eld[5] |= sad_count << 4;
> - eld[2] = (20 + mnl + sad_count * 3 + 3) / 4;
>
> - DRM_DEBUG_KMS("ELD size %d, SAD count %d\n", (int)eld[2], sad_count);
> + eld[DRM_ELD_BASELINE_ELD_LEN] =
> + DIV_ROUND_UP(drm_eld_calc_baseline_block_size(eld), 4);
> +
> + DRM_DEBUG_KMS("ELD size %d, SAD count %d\n",
> + drm_eld_size(eld), sad_count);
> }
> EXPORT_SYMBOL(drm_edid_to_eld);
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 20af973d7cba..439fa4afa18b 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -107,7 +107,7 @@ static bool intel_eld_uptodate(struct drm_connector
> *connector,
> tmp &= ~bits_elda;
> I915_WRITE(reg_elda, tmp);
>
> - for (i = 0; i < eld[2]; i++)
> + for (i = 0; i < drm_eld_size(eld) / 4; i++)
> if (I915_READ(reg_edid) != *((uint32_t *)eld + i))
> return false;
>
> @@ -162,7 +162,7 @@ static void g4x_audio_codec_enable(struct drm_connector
> *connector,
> len = (tmp >> 9) & 0x1f;/* ELD buffer size */
> I915_WRITE(G4X_AUD_CNTL_ST, tmp);
>
> - len = min_t(int, eld[2], len);
> + len = min(drm_eld_size(eld) / 4, len);
> DRM_DEBUG_DRIVER("ELD size %d\n", len);
> for (i = 0; i < len; i++)
> I915_WRITE(G4X_HDMIW_HDMIEDID, *((uint32_t *)eld + i));
> @@ -209,7 +209,7 @@ static void hsw_audio_codec_enable(struct drm_connector
> *connector,
> int len, i;
>
> DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
> - pipe_name(pipe), eld[2]);
> + pipe_name(pipe), drm_eld_size(eld));
>
> /* Enable audio presence detect, invalidate ELD */
> tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> @@ -225,8 +225,8 @@ static void hsw_audio_codec_enable(struct drm_connector
> *connector,
> I915_WRITE(HSW_AUD_DIP_ELD_CTRL(pipe), tmp);
>
> /* Up to 84 bytes of hw ELD buffer */
> - len = min_t(int, eld[2], 21);
> - for (i = 0; i < len; i++)
> + len = min(drm_eld_size(eld), 84);
> + for (i = 0; i < len / 4; i++)
> I915_WRITE(HSW_AUD_EDID_DATA(pipe), *((uint32_t *)eld + i));
>
> /* ELD valid */
> @@ -315,7 +315,7 @@ static void ilk_audio_codec_enable(struct drm_connector
> *connector,
> int aud_cntrl_st2;
>
> DRM_DEBUG_KMS("Enable audio codec on port %c, pipe %c, %u bytes
> ELD\n",
> - port_name(port), pipe_name(pipe), eld[2]);
> + port_name(port), pipe_name(pipe), drm_eld_size(eld));
>
> /* XXX: vblank wait here */
>
> @@ -354,8 +354,8 @@ static void ilk_audio_codec_enable(struct drm_connector
> *connector,
> I915_WRITE(aud_cntl_st, tmp);
>
> /* Up to 84 bytes of hw ELD buffer */
> - len = min_t(int, eld[2], 21);
> - for (i = 0; i < len; i++)
> + len = min(drm_eld_size(eld), 84);
> + for (i = 0; i < len / 4; i++)
> I915_WRITE(hdmiw_hdmiedid, *((uint32_t *)eld + i));
>
> /* ELD valid */
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c
> b/drivers/gpu/drm/nouveau/nv50_display.c
> index ae873d1a8d46..d92c11484bd9 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -1680,7 +1
[Bug 85866] Desktop environment becomes unresponsive, but mouse can move after distro update
https://bugs.freedesktop.org/show_bug.cgi?id=85866 Michel Dänzer changed: What|Removed |Added Component|Driver/radeonhd |Drivers/Gallium/radeonsi Assignee|eich at pdx.freedesktop.org|dri-devel at lists.freedesktop ||.org Product|xorg|Mesa QA Contact|xorg-team at lists.x.org | --- Comment #2 from Michel Dänzer --- Please attach the output of dmesg (preferably from after the problem occurred) and glxinfo. What versions of the kernel and Mesa were you using before the update? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141105/afc83609/attachment.html>
[PATCH] drm: drop README.drm, ancient scrolls
On November 5, 2014 2:03:16 AM Dave Airlie wrote: > From: Dave Airlie > > This stuff is ancient, we have docs now in the kernel, > lets just drop it. > > Pointed out by Glenn > > Signed-off-by: Dave Airlie > --- > drivers/gpu/drm/README.drm | 43 --- > 1 file changed, 43 deletions(-) > delete mode 100644 drivers/gpu/drm/README.drm I was thinking exactly the same thing just the other day: Reviewed-by: Thierry Reding
[Bug 68571] GPU lockup on AMD Radeon HD6850 with DPM=1
https://bugzilla.kernel.org/show_bug.cgi?id=68571 --- Comment #72 from prettyvanilla at posteo.at --- Created attachment 156581 --> https://bugzilla.kernel.org/attachment.cgi?id=156581&action=edit vbios of the sapphire radeon hd 6870 dirt3 edition -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 68571] GPU lockup on AMD Radeon HD6850 with DPM=1
https://bugzilla.kernel.org/show_bug.cgi?id=68571 --- Comment #71 from prettyvanilla at posteo.at --- Created attachment 156571 --> https://bugzilla.kernel.org/attachment.cgi?id=156571&action=edit dmesg after (uvd related) gpu lockup and successful reset -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 68571] GPU lockup on AMD Radeon HD6850 with DPM=1
https://bugzilla.kernel.org/show_bug.cgi?id=68571 --- Comment #70 from prettyvanilla at posteo.at --- I just had my Sapphire HD 6870 (Dirt3 Edition) successfully reset itself after a lockup for the first time (as opposed to the usual black or funnily patterned screen), so I can post my dmesg. Gaming seems to work without a hitch (played through The Swapper in one go), but doing anything which uses the UVD seems to be a ticking time bomb. (Yes, Magic SysRq keys work usually after a lockup.) If there is any additional data that would help to debug this, please tell me how to provide it... -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH 07/17] drm: Add atomic/plane helpers
On Tue, Nov 4, 2014 at 11:30 PM, Sean Paul wrote: >> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl >> index 89829ae58e97..ea0ef43b19e1 100644 >> --- a/Documentation/DocBook/drm.tmpl >> +++ b/Documentation/DocBook/drm.tmpl >> @@ -996,6 +996,10 @@ int max_width, max_height; >> !Edrivers/gpu/drm/drm_modes.c >> >> >> + Atomic Mode Setting Function Reference >> +!Edrivers/gpu/drm/drm_atomic.c >> + >> + > > This change should probably be in the previous patch. Yeah, fixed locally. Thanks for the review so far. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
