[PATCH 1/2] dma-buf: return index of the first signaled fence (v2)
Hi Alex, On 07-Nov-2016 11:14 PM, "Alex Deucher" wrote: > > On Fri, Nov 4, 2016 at 6:03 PM, Sumit Semwal wrote: > > Hi Alex, > > > > Thanks for the patches. > > > > On 4 November 2016 at 14:16, Alex Deucher wrote: > >> From: "monk.liu" > >> > >> Return the index of the first signaled fence. This information > >> is useful in some APIs like Vulkan. > >> > >> v2: rebase on drm-next (fence -> dma_fence) > >> > >> Signed-off-by: monk.liu > >> Signed-off-by: Alex Deucher > >> Cc: Sumit Semwal > >> --- > >> > >> This is the same patch set I send out yesterday, I just > >> squashed the amdgpu patches together and rebased everything on > >> the fence -> dma_fence renaming. This is used by our VK driver > >> and we are planning to use it in mesa as well. > >> > > > > Would you be ok if I apply this and the amdgpu patch both together via > > drm-misc, or would you like me to notify you once I merge this for you > > to take the amdgpu patch via your tree? I'm fine either ways, but > > perhaps drm-misc would be a bit neater. > > > > Either way works for me. Whatever is easier for you. > Thanks, will take these and Christian's patches through the drm-misc tree, hopefully today. (returning from LPC and just landed at my home city, 4am here, but will hope to push these today! ) > Alex Best, Sumit. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/0b3ce2ec/attachment.html>
[radeon-alex:drm-next-4.10-wip 33/38] drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c:363:8: error: too few arguments to function 'dma_fence_wait_any_timeout'
tree: git://people.freedesktop.org/~agd5f/linux.git drm-next-4.10-wip
head: 14f8d51910e9d2f2f41164a55fe770cedb4a5585
commit: 45838b82feab1951b6940013d629bcc8a8808e83 [33/38] dma-buf: return index
of the first signaled fence (v2)
config: i386-randconfig-s0-11071731 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout 45838b82feab1951b6940013d629bcc8a8808e83
# save the attached .config to linux build tree
make ARCH=i386
Note: the radeon-alex/drm-next-4.10-wip HEAD
14f8d51910e9d2f2f41164a55fe770cedb4a5585 builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c: In function 'amdgpu_sa_bo_new':
>> drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c:363:8: error: too few arguments to
>> function 'dma_fence_wait_any_timeout'
t = dma_fence_wait_any_timeout(fences, count, false,
^~
In file included from include/drm/drmP.h:60:0,
from drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c:44:
include/linux/dma-fence.h:383:13: note: declared here
signed long dma_fence_wait_any_timeout(struct dma_fence **fences,
^~
vim +/dma_fence_wait_any_timeout +363 drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
6ba60b89 Christian König 2016-03-11 357 for (i = 0, count = 0;
i < AMDGPU_SA_NUM_FENCE_LISTS; ++i)
ee327caf Christian König 2015-10-20 358 if (fences[i])
f54d1867 Chris Wilson2016-10-25 359
fences[count++] = dma_fence_get(fences[i]);
ee327caf Christian König 2015-10-20 360
ee327caf Christian König 2015-10-20 361 if (count) {
d38ceaf9 Alex Deucher2015-04-20 362
spin_unlock(&sa_manager->wq.lock);
f54d1867 Chris Wilson2016-10-25 @363t =
dma_fence_wait_any_timeout(fences, count, false,
ee327caf Christian König 2015-10-20 364
MAX_SCHEDULE_TIMEOUT);
a8d81b36 Nicolai Hähnle 2016-02-05 365 for (i = 0; i <
count; ++i)
f54d1867 Chris Wilson2016-10-25 366
dma_fence_put(fences[i]);
:: The code at line 363 was first introduced by commit
:: f54d1867005c3323f5d8ad83eed823e84226c429 dma-buf: Rename struct fence to
dma_fence
:: TO: Chris Wilson
:: CC: Daniel Vetter
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-- next part --
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 27103 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/ef204d48/attachment-0001.gz>
[PATCH 2/2] drm/amdgpu: add the interface of waiting multiple fences (v4)
2016-11-07 Christian König :
> Am 07.11.2016 um 02:10 schrieb Gustavo Padovan:
> > Hi Alex,
> >
> > 2016-11-04 Alex Deucher :
> >
> > > From: Junwei Zhang
> > >
> > > v2: agd: rebase and squash in all the previous optimizations and
> > > changes so everything compiles.
> > > v3: squash in Slava's 32bit build fix
> > > v4: rebase on drm-next (fence -> dma_fence),
> > > squash in Monk's ioctl update patch
> > >
> > > Signed-off-by: Junwei Zhang
> > > Reviewed-by: Monk Liu
> > > Reviewed-by: Jammy Zhou
> > > Signed-off-by: Alex Deucher
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 173
> > >
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 +
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 2 +-
> > > include/uapi/drm/amdgpu_drm.h | 28 ++
> > > 5 files changed, 205 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > > index dc98ceb..7a94a3c 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > > @@ -1212,6 +1212,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
> > > void *data,
> > > struct drm_file *filp);
> > > int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file
> > > *filp);
> > > int amdgpu_cs_wait_ioctl(struct drm_device *dev, void *data, struct
> > > drm_file *filp);
> > > +int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev, void *data,
> > > + struct drm_file *filp);
> > > int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void *data,
> > > struct drm_file *filp);
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > > index 2728805..2004836 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > > @@ -1130,6 +1130,179 @@ int amdgpu_cs_wait_ioctl(struct drm_device *dev,
> > > void *data,
> > > }
> > > /**
> > > + * amdgpu_cs_get_fence - helper to get fence from drm_amdgpu_fence
> > > + *
> > > + * @adev: amdgpu device
> > > + * @filp: file private
> > > + * @user: drm_amdgpu_fence copied from user space
> > > + */
> > > +static struct dma_fence *amdgpu_cs_get_fence(struct amdgpu_device *adev,
> > > + struct drm_file *filp,
> > > + struct drm_amdgpu_fence *user)
> > > +{
> > > + struct amdgpu_ring *ring;
> > > + struct amdgpu_ctx *ctx;
> > > + struct dma_fence *fence;
> > > + int r;
> > > +
> > > + r = amdgpu_cs_get_ring(adev, user->ip_type, user->ip_instance,
> > > +user->ring, &ring);
> > > + if (r)
> > > + return ERR_PTR(r);
> > > +
> > > + ctx = amdgpu_ctx_get(filp->driver_priv, user->ctx_id);
> > > + if (ctx == NULL)
> > > + return ERR_PTR(-EINVAL);
> > > +
> > > + fence = amdgpu_ctx_get_fence(ctx, ring, user->seq_no);
> > > + amdgpu_ctx_put(ctx);
> > > +
> > > + return fence;
> > > +}
> > > +
> > > +/**
> > > + * amdgpu_cs_wait_all_fence - wait on all fences to signal
> > > + *
> > > + * @adev: amdgpu device
> > > + * @filp: file private
> > > + * @wait: wait parameters
> > > + * @fences: array of drm_amdgpu_fence
> > > + */
> > > +static int amdgpu_cs_wait_all_fences(struct amdgpu_device *adev,
> > > + struct drm_file *filp,
> > > + union drm_amdgpu_wait_fences *wait,
> > > + struct drm_amdgpu_fence *fences)
> > > +{
> > > + uint32_t fence_count = wait->in.fence_count;
> > > + unsigned i;
> > > + long r = 1;
> > > +
> > > + for (i = 0; i < fence_count; i++) {
> > > + struct dma_fence *fence;
> > > + unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns);
> > > +
> > > + fence = amdgpu_cs_get_fence(adev, filp, &fences[i]);
> > > + if (IS_ERR(fence))
> > > + return PTR_ERR(fence);
> > > + else if (!fence)
> > > + continue;
> > > +
> > > + r = dma_fence_wait_timeout(fence, true, timeout);
> > > + if (r < 0)
> > > + return r;
> > > +
> > > + if (r == 0)
> > > + break;
> > > + }
> > > +
> > > + memset(wait, 0, sizeof(*wait));
> > > + wait->out.status = (r > 0);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * amdgpu_cs_wait_any_fence - wait on any fence to signal
> > > + *
> > > + * @adev: amdgpu device
> > > + * @filp: file private
> > > + * @wait: wait parameters
> > > + * @fences: array of drm_amdgpu_fence
> > > + */
> > > +static int amdgpu_cs_wait_any_fence(struct amdgpu_device *adev,
> > > + struct drm_file *filp,
> > > + union drm_amdgpu_wait_fences *wait,
> > > +
[PATCH v3] drm: bridge: add DesignWare HDMI I2S audio support
Hi Russell > > > This is wrong. If platform is NULL, PTR_ERR() will return zero, which > > > will be interpreted as success. Please, avoid using IS_ERR_OR_NULL(), > > > it leads to exactly this kind of cockup - and it's unnecessary here > > > because platform_device_register_full() does not return NULL. > > > > Thank you for your feedback. > > Before sending v4 patch, I would like to confirm. Do you meand this ? > > # use IS_ERR() instead of IS_ERR_OR_NULL() > > > > platform = platform_device_register_full(&pdevinfo); > > if (IS_ERR(platform)) > > return PTR_ERR(platform); > > Yes, that's exactly correct. Thanks ! I will send v4 patch Best regards --- Kuninori Morimoto
[PATCH 4/4] reservation: revert "wait only with non-zero timeout specified (v3)" v2
2016-11-07 Alex Deucher : > From: Christian König > > This reverts commit fb8b7d2b9d80e1e71f379e57355936bd2b024be9. > > Otherwise signaling might never be activated on the fences. This can > result in infinite waiting with hardware which has unreliable interrupts. > > v2: still return one when the timeout is zero and we don't have any fences. > > Reviewed-by: Alex Deucher > Signed-off-by: Christian König > Reviewed-by: Chunming Zhou (v1) > Signed-off-by: Alex Deucher > --- > drivers/dma-buf/reservation.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) You left all my r-b out. For the whole series: Reviewed-by: Gustavo Padovan Gustavo
[PATCH] drm/amdgpu: Attach exclusive fence to prime exported bo's. (v4)
External clients which import our bo's wait only
for exclusive dmabuf-fences, not on shared ones,
ditto for bo's which we import from external
providers and write to.
Therefore attach exclusive fences on prime shared buffers
if our exported buffer gets imported by an external
client, or if we import a buffer from an external
exporter.
See discussion in thread:
https://lists.freedesktop.org/archives/dri-devel/2016-October/122370.html
Prime export tested on Intel iGPU + AMD Tonga dGPU as
DRI3/Present Prime render offload, and with the Tonga
standalone as primary gpu.
v2: Add a wait for all shared fences before prime export,
as suggested by Christian Koenig.
v3: - Mark buffer prime_exported in amdgpu_gem_prime_pin,
so we only use the exclusive fence when exporting a
bo to external clients like a separate iGPU, but not
when exporting/importing from/to ourselves as part of
regular DRI3 fd passing.
- Propagate failure of reservation_object_wait_rcu back
to caller.
v4: - Switch to a prime_shared_count counter instead of a
flag, which gets in/decremented on prime_pin/unpin, so
we can switch back to shared fences if all clients
detach from our exported bo.
- Also switch to exclusive fence for prime imported bo's.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95472
Tested-by: Mike Lothian (v1)
Signed-off-by: Mario Kleiner
Cc: Christian König
Cc: Michel Dänzer
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 19 +++
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 039b57e..496f72b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -459,6 +459,7 @@ struct amdgpu_bo {
u64 metadata_flags;
void*metadata;
u32 metadata_size;
+ unsignedprime_shared_count;
/* list of all virtual address to which this bo
* is associated to
*/
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index 651115d..c02db01f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -132,7 +132,7 @@ static int amdgpu_bo_list_set(struct amdgpu_device *adev,
entry->priority = min(info[i].bo_priority,
AMDGPU_BO_LIST_MAX_PRIORITY);
entry->tv.bo = &entry->robj->tbo;
- entry->tv.shared = true;
+ entry->tv.shared = !entry->robj->prime_shared_count;
if (entry->robj->prefered_domains == AMDGPU_GEM_DOMAIN_GDS)
gds_obj = entry->robj;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
index 7700dc2..31aac7e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
@@ -74,6 +74,7 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev,
if (ret)
return ERR_PTR(ret);
+ bo->prime_shared_count = 1;
return &bo->gem_base;
}
@@ -81,13 +82,29 @@ int amdgpu_gem_prime_pin(struct drm_gem_object *obj)
{
struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
int ret = 0;
+ long lret;
ret = amdgpu_bo_reserve(bo, false);
if (unlikely(ret != 0))
return ret;
+ /*
+* Wait for all shared fences to complete before we switch to future
+* use of exclusive fence on this prime shared bo.
+*/
+ lret = reservation_object_wait_timeout_rcu(bo->tbo.resv, true, false,
+ MAX_SCHEDULE_TIMEOUT);
+ if (unlikely(lret < 0)) {
+ DRM_DEBUG_PRIME("Fence wait failed: %li\n", lret);
+ amdgpu_bo_unreserve(bo);
+ return lret;
+ }
+
/* pin buffer into GTT */
ret = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT, NULL);
+ if (likely(ret == 0))
+ bo->prime_shared_count++;
+
amdgpu_bo_unreserve(bo);
return ret;
}
@@ -102,6 +119,8 @@ void amdgpu_gem_prime_unpin(struct drm_gem_object *obj)
return;
amdgpu_bo_unpin(bo);
+ if (bo->prime_shared_count)
+ bo->prime_shared_count--;
amdgpu_bo_unreserve(bo);
}
--
2.7.4
[PATCH v4] drm: bridge: add DesignWare HDMI I2S audio support
From: Kuninori Morimoto
Current dw-hdmi is supporting sound via AHB bus, but it has
I2S audio feature too. This patch adds I2S audio support to dw-hdmi.
This HDMI I2S is supported by using ALSA SoC common HDMI encoder
driver.
Tested-by: Jose Abreu
Signed-off-by: Kuninori Morimoto
---
v3 -> v4
- use IS_ERR() instead of IS_ERR_OR_NULL() on probe()
drivers/gpu/drm/bridge/Kconfig | 8 ++
drivers/gpu/drm/bridge/Makefile| 1 +
drivers/gpu/drm/bridge/dw-hdmi-audio.h | 7 ++
drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c | 142 +
drivers/gpu/drm/bridge/dw-hdmi.c | 22 -
drivers/gpu/drm/bridge/dw-hdmi.h | 21 +
6 files changed, 199 insertions(+), 2 deletions(-)
create mode 100644 drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 10e12e7..3129f8d 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -39,6 +39,14 @@ config DRM_DW_HDMI_AHB_AUDIO
Designware HDMI block. This is used in conjunction with
the i.MX6 HDMI driver.
+config DRM_DW_HDMI_I2S_AUDIO
+ tristate "Synopsis Designware I2S Audio interface"
+ depends on DRM_DW_HDMI
+ select SND_SOC_HDMI_CODEC
+ help
+ Support the I2S Audio interface which is part of the Synopsis
+ Designware HDMI block.
+
config DRM_NXP_PTN3460
tristate "NXP PTN3460 DP/LVDS bridge"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index cdf3a3c..9a54f2a 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
+obj-$(CONFIG_DRM_DW_HDMI_I2S_AUDIO) += dw-hdmi-i2s-audio.o
obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
obj-$(CONFIG_DRM_SII902X) += sii902x.o
diff --git a/drivers/gpu/drm/bridge/dw-hdmi-audio.h
b/drivers/gpu/drm/bridge/dw-hdmi-audio.h
index 91f631b..fd1f745 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi-audio.h
+++ b/drivers/gpu/drm/bridge/dw-hdmi-audio.h
@@ -11,4 +11,11 @@ struct dw_hdmi_audio_data {
u8 *eld;
};
+struct dw_hdmi_i2s_audio_data {
+ struct dw_hdmi *hdmi;
+
+ void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
+ u8 (*read)(struct dw_hdmi *hdmi, int offset);
+};
+
#endif
diff --git a/drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c
b/drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c
new file mode 100644
index 000..79391c0
--- /dev/null
+++ b/drivers/gpu/drm/bridge/dw-hdmi-i2s-audio.c
@@ -0,0 +1,142 @@
+/*
+ * dw-hdmi-i2s-audio.c
+ *
+ * Copyright (c) 2016 Kuninori Morimoto
+ *
+ * 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 "dw-hdmi.h"
+#include "dw-hdmi-audio.h"
+
+#define DRIVER_NAME "dw-hdmi-i2s-audio"
+
+static inline void hdmi_write(struct dw_hdmi_i2s_audio_data *audio,
+ u8 val, int offset)
+{
+ struct dw_hdmi *hdmi = audio->hdmi;
+
+ audio->write(hdmi, val, offset);
+}
+
+static inline u8 hdmi_read(struct dw_hdmi_i2s_audio_data *audio, int offset)
+{
+ struct dw_hdmi *hdmi = audio->hdmi;
+
+ return audio->read(hdmi, offset);
+}
+
+static int dw_hdmi_i2s_hw_params(struct device *dev, void *data,
+struct hdmi_codec_daifmt *fmt,
+struct hdmi_codec_params *hparms)
+{
+ struct dw_hdmi_i2s_audio_data *audio = data;
+ struct dw_hdmi *hdmi = audio->hdmi;
+ u8 conf0 = 0;
+ u8 conf1 = 0;
+ u8 inputclkfs = 0;
+
+ /* it cares I2S only */
+ if ((fmt->fmt != HDMI_I2S) ||
+ (fmt->bit_clk_master | fmt->frame_clk_master)) {
+ dev_err(dev, "unsupported format/settings\n");
+ return -EINVAL;
+ }
+
+ inputclkfs = HDMI_AUD_INPUTCLKFS_64FS;
+ conf0 = HDMI_AUD_CONF0_I2S_ALL_ENABLE;
+
+ switch (hparms->sample_width) {
+ case 16:
+ conf1 = HDMI_AUD_CONF1_WIDTH_16;
+ break;
+ case 24:
+ case 32:
+ conf1 = HDMI_AUD_CONF1_WIDTH_24;
+ break;
+ }
+
+ dw_hdmi_set_sample_rate(hdmi, hparms->sample_rate);
+
+ hdmi_write(audio, inputclkfs, HDMI_AUD_INPUTCLKFS);
+ hdmi_write(audio, conf0, HDMI_AUD_CONF0);
+ hdmi_write(audio, conf1, HDMI_AUD_CONF1);
+
+ dw_hdmi_audio_enable(hdmi);
+
+ return 0;
+}
+
+static void dw_hdmi_i2s_audio_shutdown(struct device *dev, void *data)
+{
+ struct dw_hdmi_i2s_audio_data *audio = data;
+
[Bug 95306] Random Blank(black) screens on "Carrizo"
https://bugs.freedesktop.org/show_bug.cgi?id=95306 --- Comment #30 from Patrick Laurin --- Indeed, I am currently trying 4.9 rc-3 right now and it has been running for more than 30 minutes now. This is exciting time. Crossing all fingers... On Nov 7, 2016 16:05, wrote: > *Comment # 29 <https://bugs.freedesktop.org/show_bug.cgi?id=95306#c29> on > bug 95306 <https://bugs.freedesktop.org/show_bug.cgi?id=95306> from Tom > * > > Linux 4.9rc4 seems to fix the suspend issue, but booting up is still buggy. > > -- > You are receiving this mail because: > >- You are on the CC list for the bug. > > -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/ec4b5e0c/attachment.html>
linux-next: manual merge of the tip tree with the drm-intel tree
Hi all,
FIXME: Add owner of second tree to To:
Add author(s)/SOB of conflicting commits.
Today's linux-next merge of the tip tree got a conflict in:
drivers/gpu/drm/i915/i915_gem_shrinker.c
between commits:
1233e2db199d ("drm/i915: Move object backing storage manipulation to its own
locking")
from the drm-intel tree and commit:
3ab7c086d5ec ("locking/drm: Kill mutex trickery")
c7faee2109f9 ("locking/drm: Fix i915_gem_shrinker_lock() locking")
from the tip tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/gpu/drm/i915/i915_gem_shrinker.c
index a6fc1bdc48af,e9bd2a81d03a..
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@@ -35,33 -35,6 +35,15 @@@
#include "i915_drv.h"
#include "i915_trace.h"
- static bool mutex_is_locked_by(struct mutex *mutex, struct task_struct *task)
- {
- if (!mutex_is_locked(mutex))
- return false;
-
- #if defined(CONFIG_DEBUG_MUTEXES) || defined(CONFIG_MUTEX_SPIN_ON_OWNER)
- return mutex->owner == task;
- #else
- /* Since UP may be pre-empted, we cannot assume that we own the lock */
- return false;
- #endif
- }
-
+static bool i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
+{
- if (!mutex_trylock(&dev->struct_mutex)) {
- if (!mutex_is_locked_by(&dev->struct_mutex, current))
- return false;
-
- *unlock = false;
- } else {
- *unlock = true;
- }
++ if (!mutex_trylock(&dev->struct_mutex))
++ return false;
+
++ *unlock = true;
+ return true;
+}
+
static bool any_vma_pinned(struct drm_i915_gem_object *obj)
{
struct i915_vma *vma;
[PATCH v2] drm/udl: Ensure channel is selected before using the device.
On 23.08.2016 07:57, Daniel Vetter wrote:
> On Mon, Aug 22, 2016 at 11:17:34PM +0100, Jamie Lentin wrote:
>> Lift configuration command from udlfb. This appears to be essential for
>> at least a Rextron VCUD-60, without which no URB communication occurs.
>>
>> Signed-off-by: Jamie Lentin
>> ---
>> udl_encoder_commit() is too late to do this set up in it seems. This
>> setup doesn't need to be performed again after a suspend, although this
>> is somewhat academic until I send the patch adding suspend and resume
>> functions.
>>
>> Tested with a Rextron VCUD-60 attached to a Thinkpad X201s on Linux 4.7.0
>
> Applied to drm-misc, thanks.
> -Daniel
>
>>
>> Cheers,
>> ---
>> drivers/gpu/drm/udl/udl_main.c | 25 +
>> 1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
>> index 33dbfb2..29f0207 100644
>> --- a/drivers/gpu/drm/udl/udl_main.c
>> +++ b/drivers/gpu/drm/udl/udl_main.c
>> @@ -16,6 +16,8 @@
>> /* -BULK_SIZE as per usb-skeleton. Can we get full page and avoid overhead?
>> */
>> #define BULK_SIZE 512
>>
>> +#define NR_USB_REQUEST_CHANNEL 0x12
>> +
>> #define MAX_TRANSFER (PAGE_SIZE*16 - BULK_SIZE)
>> #define WRITES_IN_FLIGHT (4)
>> #define MAX_VENDOR_DESCRIPTOR_SIZE 256
>> @@ -90,6 +92,26 @@ success:
>> return true;
>> }
>>
>> +/*
>> + * Need to ensure a channel is selected before submitting URBs
>> + */
>> +static int udl_select_std_channel(struct udl_device *udl)
>> +{
>> +int ret;
>> +u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
>> +0x1C, 0x88, 0x5E, 0x15,
>> +0x60, 0xFE, 0xC6, 0x97,
>> +0x16, 0x3D, 0x47, 0xF2};
>> +
>> +ret = usb_control_msg(udl->udev,
>> + usb_sndctrlpipe(udl->udev, 0),
>> + NR_USB_REQUEST_CHANNEL,
>> + (USB_DIR_OUT | USB_TYPE_VENDOR), 0, 0,
>> + set_def_chn, sizeof(set_def_chn),
>> + USB_CTRL_SET_TIMEOUT);
>> +return ret < 0 ? ret : 0;
>> +}
>> +
>> static void udl_release_urb_work(struct work_struct *work)
>> {
>> struct urb_node *unode = container_of(work, struct urb_node,
>> @@ -301,6 +323,9 @@ int udl_driver_load(struct drm_device *dev, unsigned
>> long flags)
>> goto err;
>> }
>>
>> +if (udl_select_std_channel(udl))
>> +DRM_ERROR("Selecting channel failed\n");
>> +
>> if (!udl_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) {
>> DRM_ERROR("udl_alloc_urb_list failed\n");
>> goto err;
>> --
>> 2.8.1
>>
>
It doesn't breaks the device, however under the hood it is boiling,
[ cut here ]
WARNING: CPU: 0 PID: 381 at drivers/usb/core/hcd.c:1584
usb_hcd_map_urb_for_dma+0x49f/0x770
transfer buffer not dma capable
Modules linked in: ... udl(+) ... drm_kms_helper ... drm ...
CPU: 0 PID: 381 Comm: systemd-udevd Not tainted
4.9.0-0.rc4.git0.1.fc26.x86_64+debug #1
...
Call Trace:
[] dump_stack+0x86/0xc3
[] __warn+0xcb/0xf0
[] warn_slowpath_fmt+0x5f/0x80
[] ? debug_dma_mapping_error+0x7b/0x90
[] usb_hcd_map_urb_for_dma+0x49f/0x770
[] ? trace_hardirqs_on_caller+0xf5/0x1b0
[] usb_hcd_submit_urb+0x34d/0xb50
[] ? mark_held_locks+0x76/0xa0
[] ? __raw_spin_lock_init+0x21/0x60
[] ? trace_hardirqs_on_caller+0xf5/0x1b0
[] usb_submit_urb+0x2f4/0x560
[] ? lockdep_init_map+0x61/0x210
[] usb_start_wait_urb+0x74/0x180
[] usb_control_msg+0xdc/0x120
[] udl_driver_load+0x141/0x590 [udl]
[] ? trace_hardirqs_on_caller+0xd1/0x1b0
[] drm_dev_register+0xa9/0xd0 [drm]
[] udl_usb_probe+0x42/0x90 [udl]
[] usb_probe_interface+0x15f/0x2d0
[] driver_probe_device+0x223/0x430
[] __driver_attach+0xe3/0xf0
[] ? driver_probe_device+0x430/0x430
[] bus_for_each_dev+0x73/0xc0
[] driver_attach+0x1e/0x20
[] bus_add_driver+0x173/0x270
[] driver_register+0x60/0xe0
[] usb_register_driver+0xaa/0x160
[] ? 0xc089a000
[] udl_driver_init+0x1e/0x1000 [udl]
[] do_one_initcall+0x50/0x180
[] ? rcu_read_lock_sched_held+0x45/0x80
[] ? kmem_cache_alloc_trace+0x277/0x2d0
[] ? do_init_module+0x27/0x1f1
[] do_init_module+0x5f/0x1f1
[] load_module+0x2401/0x2b40
[] ? __symbol_put+0x70/0x70
[] ? sched_clock_cpu+0x90/0xc0
[] SYSC_init_module+0x19b/0x1c0
[] SyS_init_module+0xe/0x10
[] do_syscall_64+0x6c/0x1f0
[] entry_SYSCALL64_slow_path+0x25/0x25
---[ end trace 1fa5e22a0dcf62da ]---
[drm:udl_driver_load [udl]] *ERROR* Selecting channel failed
udl 1-2:1.0: fb1: udldrmfb frame buffer device
[drm] Initialized udl on minor 1
usbcore: registered new interface driver udl
Is this expected WARN?
Ref.
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/udl?id=d1c151dc
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/hcd.c?id=refs/tags/v4.9-rc4#n1584
[PATCH] drm/udl: make control msg static const.
From: Dave Airlie
Thou shall not send control msg from the stack,
does that mean I can send it from the RO memory area?
Reported-by: poma
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/udl/udl_main.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 29f0207..0798bcc 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -98,10 +98,10 @@ static int udl_parse_vendor_descriptor(struct drm_device
*dev,
static int udl_select_std_channel(struct udl_device *udl)
{
int ret;
- u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
- 0x1C, 0x88, 0x5E, 0x15,
- 0x60, 0xFE, 0xC6, 0x97,
- 0x16, 0x3D, 0x47, 0xF2};
+ static const u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
+0x1C, 0x88, 0x5E, 0x15,
+0x60, 0xFE, 0xC6, 0x97,
+0x16, 0x3D, 0x47, 0xF2};
ret = usb_control_msg(udl->udev,
usb_sndctrlpipe(udl->udev, 0),
--
2.5.5
[PATCH v2] drm/udl: Ensure channel is selected before using the device.
> [drm:udl_driver_load [udl]] *ERROR* Selecting channel failed > udl 1-2:1.0: fb1: udldrmfb frame buffer device > [drm] Initialized udl on minor 1 > usbcore: registered new interface driver udl > > > Is this expected WARN? I've just posted a patch in theory to fix it, please test if oyu have time. Dave.
[PATCH] drm/udl: make control msg static const. (v2)
From: Dave Airlie
Thou shall not send control msg from the stack,
does that mean I can send it from the RO memory area?
and it looks like the answer is no, so here's
v2 which kmemdups.
Reported-by: poma
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/udl/udl_main.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 29f0207..873f010 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -98,17 +98,23 @@ static int udl_parse_vendor_descriptor(struct drm_device
*dev,
static int udl_select_std_channel(struct udl_device *udl)
{
int ret;
- u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
- 0x1C, 0x88, 0x5E, 0x15,
- 0x60, 0xFE, 0xC6, 0x97,
- 0x16, 0x3D, 0x47, 0xF2};
+ static const u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
+0x1C, 0x88, 0x5E, 0x15,
+0x60, 0xFE, 0xC6, 0x97,
+0x16, 0x3D, 0x47, 0xF2};
+ void *sendbuf;
+
+ sendbuf = kmemdup(set_def_chn, sizeof(set_def_chn), GFP_KERNEL);
+ if (!sendbuf)
+ return -ENOMEM;
ret = usb_control_msg(udl->udev,
usb_sndctrlpipe(udl->udev, 0),
NR_USB_REQUEST_CHANNEL,
(USB_DIR_OUT | USB_TYPE_VENDOR), 0, 0,
- set_def_chn, sizeof(set_def_chn),
+ sendbuf, sizeof(set_def_chn),
USB_CTRL_SET_TIMEOUT);
+ kfree(sendbuf);
return ret < 0 ? ret : 0;
}
--
2.5.5
[PATCH v2] drm/udl: Ensure channel is selected before using the device.
On 8 November 2016 at 16:40, Dave Airlie wrote: >> [drm:udl_driver_load [udl]] *ERROR* Selecting channel failed >> udl 1-2:1.0: fb1: udldrmfb frame buffer device >> [drm] Initialized udl on minor 1 >> usbcore: registered new interface driver udl >> >> >> Is this expected WARN? > > I've just posted a patch in theory to fix it, please test if oyu have time. Actually I posted a v2 of it, because the first one didn't work either. Dave.
[PATCH v7 0/3] drm: add explict fencing
From: Gustavo Padovan Hi, This is yet another version of the DRM fences patches. Please refer to the cover letter[1] in a previous version to check for more details. In v7 we now have split most of the out_fences code into prepare_crtc_signaling() and unprepare_crtc_signaling() with improved error handling. More details on the v7 changes are embedded in each commit's message. Robert Foss managed to port Android's drm_hwcomposer to the new HWC2 API and added support to fences. Current patches can be seen here: https://git.collabora.com/cgit/user/robertfoss/drm_hwcomposer.git/log/?h=hwc2_fence_v1 He managed to run AOSP on top of padovan/fences kernel branch with full fence support on qemu/virgl and msm db410c. That means we already have a working open source userspace using the explicit fencing implementation. Also i-g-t testing are available at: https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/log/ Please review! Gustavo [1] https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1253822.html --- Gustavo Padovan (3): drm/fence: add in-fences support drm/fence: add fence timeline to drm_crtc drm/fence: add out-fences support drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_atomic.c| 247 ++-- drivers/gpu/drm/drm_atomic_helper.c | 3 + drivers/gpu/drm/drm_crtc.c | 45 +++ drivers/gpu/drm/drm_plane.c | 1 + include/drm/drm_atomic.h| 1 + include/drm/drm_crtc.h | 55 7 files changed, 311 insertions(+), 42 deletions(-) -- 2.5.5
[PATCH v7 1/3] drm/fence: add in-fences support
From: Gustavo Padovan
There is now a new property called IN_FENCE_FD attached to every plane
state that receives sync_file fds from userspace via the atomic commit
IOCTL.
The fd is then translated to a fence (that may be a fence_array
subclass or just a normal fence) and then used by DRM to fence_wait() for
all fences in the sync_file to signal. So it only commits when all
framebuffers are ready to scanout.
v2: Comments by Daniel Vetter:
- remove set state->fence = NULL in destroy phase
- accept fence -1 as valid and just return 0
- do not call fence_get() - sync_file_fences_get() already calls it
- fence_put() if state->fence is already set, in case userspace
set the property more than once.
v3: WARN_ON if fence is set but state has no FB
v4: Comment from Maarten Lankhorst
- allow set fence with no related fb
v5: rename FENCE_FD to IN_FENCE_FD
v6: Comments by Daniel Vetter:
- rename plane_state->in_fence back to "fence"
- re-introduce WARN_ON if fence set but no fb
- rebase after fence -> dma_fence rename
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/drm_atomic.c| 14 ++
drivers/gpu/drm/drm_atomic_helper.c | 3 +++
drivers/gpu/drm/drm_crtc.c | 6 ++
drivers/gpu/drm/drm_plane.c | 1 +
include/drm/drm_crtc.h | 5 +
6 files changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 483059a..43cb33d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -12,6 +12,7 @@ menuconfig DRM
select I2C
select I2C_ALGOBIT
select DMA_SHARED_BUFFER
+ select SYNC_FILE
help
Kernel-level support for the Direct Rendering Infrastructure (DRI)
introduced in XFree86 4.0. If you say Y here, you need to select
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 5e73954..d1ae463 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include "drm_crtc_internal.h"
@@ -686,6 +687,17 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
drm_atomic_set_fb_for_plane(state, fb);
if (fb)
drm_framebuffer_unreference(fb);
+ } else if (property == config->prop_in_fence_fd) {
+ if (U642I64(val) == -1)
+ return 0;
+
+ if (state->fence)
+ dma_fence_put(state->fence);
+
+ state->fence = sync_file_get_fence(val);
+ if (!state->fence)
+ return -EINVAL;
+
} else if (property == config->prop_crtc_id) {
struct drm_crtc *crtc = drm_crtc_find(dev, val);
return drm_atomic_set_crtc_for_plane(state, crtc);
@@ -747,6 +759,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
if (property == config->prop_fb_id) {
*val = (state->fb) ? state->fb->base.id : 0;
+ } else if (property == config->prop_in_fence_fd) {
+ *val = -1;
} else if (property == config->prop_crtc_id) {
*val = (state->crtc) ? state->crtc->base.id : 0;
} else if (property == config->prop_crtc_x) {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 75ad01d..26a067f 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3114,6 +3114,9 @@ void __drm_atomic_helper_plane_destroy_state(struct
drm_plane_state *state)
{
if (state->fb)
drm_framebuffer_unreference(state->fb);
+
+ if (state->fence)
+ dma_fence_put(state->fence);
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 13441e2..7878bfd 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -397,6 +397,12 @@ static int drm_mode_create_standard_properties(struct
drm_device *dev)
return -ENOMEM;
dev->mode_config.prop_fb_id = prop;
+ prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC,
+ "IN_FENCE_FD", -1, INT_MAX);
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.prop_in_fence_fd = prop;
+
prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
"CRTC_ID", DRM_MODE_OBJECT_CRTC);
if (!prop)
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 249c0ae..3957ef8 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -137,6 +137,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct
drm_plane *plane,
if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
drm_object_attach_property(&plane->ba
[PATCH v7 2/3] drm/fence: add fence timeline to drm_crtc
From: Gustavo Padovan
Create one timeline context for each CRTC to be able to handle out-fences
and signal them. It adds a few members to struct drm_crtc: fence_context,
where we store the context we get from fence_context_alloc(), the
fence seqno and the fence lock, that we pass in fence_init() to be
used by the fence.
v2: Comment by Daniel Stone:
- add BUG_ON() to fence_to_crtc() macro
v3: Comment by Ville Syrjälä
- Use more meaningful name as crtc timeline name
v4: Comments by Brian Starkey
- Use even more meaninful name for the crtc timeline
- add doc for timeline_name
Comment by Daniel Vetter
- use in-line style for comments
- rebase after fence -> dma_fence rename
v5: Comment by Daniel Vetter
- Add doc for drm_crtc_fence_ops
Signed-off-by: Gustavo Padovan
Reviewed-by: Daniel Vetter
---
drivers/gpu/drm/drm_crtc.c | 31 +++
include/drm/drm_crtc.h | 45 +
2 files changed, 76 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 7878bfd..e2a06c8 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -165,6 +165,32 @@ static void drm_crtc_crc_fini(struct drm_crtc *crtc)
#endif
}
+static const char *drm_crtc_fence_get_driver_name(struct dma_fence *fence)
+{
+ struct drm_crtc *crtc = fence_to_crtc(fence);
+
+ return crtc->dev->driver->name;
+}
+
+static const char *drm_crtc_fence_get_timeline_name(struct dma_fence *fence)
+{
+ struct drm_crtc *crtc = fence_to_crtc(fence);
+
+ return crtc->timeline_name;
+}
+
+static bool drm_crtc_fence_enable_signaling(struct dma_fence *fence)
+{
+ return true;
+}
+
+const struct dma_fence_ops drm_crtc_fence_ops = {
+ .get_driver_name = drm_crtc_fence_get_driver_name,
+ .get_timeline_name = drm_crtc_fence_get_timeline_name,
+ .enable_signaling = drm_crtc_fence_enable_signaling,
+ .wait = dma_fence_default_wait,
+};
+
/**
* drm_crtc_init_with_planes - Initialise a new CRTC object with
*specified primary and cursor planes.
@@ -222,6 +248,11 @@ int drm_crtc_init_with_planes(struct drm_device *dev,
struct drm_crtc *crtc,
return -ENOMEM;
}
+ crtc->fence_context = dma_fence_context_alloc(1);
+ spin_lock_init(&crtc->fence_lock);
+ snprintf(crtc->timeline_name, sizeof(crtc->timeline_name),
+"CRTC:%d-%s", crtc->base.id, crtc->name);
+
crtc->base.properties = &crtc->properties;
list_add_tail(&crtc->head, &config->crtc_list);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 719b6a8..30f2401 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -32,6 +32,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -726,9 +728,52 @@ struct drm_crtc {
*/
struct drm_crtc_crc crc;
#endif
+
+ /**
+* @fence_context:
+*
+* timeline context used for fence operations.
+*/
+ unsigned int fence_context;
+
+ /**
+* @fence_lock:
+*
+* spinlock to protect the fences in the fence_context.
+*/
+
+ spinlock_t fence_lock;
+ /**
+* @fence_seqno:
+*
+* Seqno variable used as monotonic counter for the fences
+* created on the CRTC's timeline.
+*/
+ unsigned long fence_seqno;
+
+ /**
+* @timeline_name:
+*
+* The name of the CRTC's fence timeline.
+*/
+ char timeline_name[32];
};
/**
+ * dma_crtc_fence_ops - fence ops for the drm_crtc timeline
+ *
+ * It contains the dma_fence_ops that should be called by the dma_fence
+ * code. CRTC core should use this ops when initializing fences.
+ */
+extern const struct dma_fence_ops drm_crtc_fence_ops;
+
+static inline struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
+{
+ BUG_ON(fence->ops != &drm_crtc_fence_ops);
+ return container_of(fence->lock, struct drm_crtc, fence_lock);
+}
+
+/**
* struct drm_mode_set - new values for a CRTC config change
* @fb: framebuffer to use for new config
* @crtc: CRTC whose configuration we're about to change
--
2.5.5
[PATCH v7 3/3] drm/fence: add out-fences support
From: Gustavo Padovan
Support DRM out-fences by creating a sync_file with a fence for each CRTC
that sets the OUT_FENCE_PTR property.
We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
the sync_file fd back to userspace.
The sync_file and fd are allocated/created before commit, but the
fd_install operation only happens after we know that commit succeed.
v2: Comment by Rob Clark:
- Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here.
Comment by Daniel Vetter:
- Add clean up code for out_fences
v3: Comments by Daniel Vetter:
- create DRM_MODE_ATOMIC_EVENT_MASK
- userspace should fill out_fences_ptr with the crtc_ids for which
it wants fences back.
v4: Create OUT_FENCE_PTR properties and remove old approach.
v5: Comments by Brian Starkey:
- Remove extra fence_get() in atomic_ioctl()
- Check ret before iterating on the crtc_state
- check ret before fd_install
- set fence_state to NULL at the beginning
- check fence_state->out_fence_ptr before put_user()
- change order of fput() and put_unused_fd() on failure
- Add access_ok() check to the out_fence_ptr received
- Rebase after fence -> dma_fence rename
- Store out_fence_ptr in the drm_atomic_state
- Split crtc_setup_out_fence()
- return -1 as out_fence with TEST_ONLY flag
v6: Comments by Daniel Vetter
- Add prepare/unprepare_crtc_signaling()
- move struct drm_out_fence_state to drm_atomic.c
- mark get_crtc_fence() as static
Comments by Brian Starkey
- proper set fence_ptr fence_state array
- isolate fence_idx increment
- improve error handling
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/drm_atomic.c | 233 +++
drivers/gpu/drm/drm_crtc.c | 8 ++
include/drm/drm_atomic.h | 1 +
include/drm/drm_crtc.h | 7 +-
4 files changed, 206 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index d1ae463..9a7d84c 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -289,6 +289,25 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
}
EXPORT_SYMBOL(drm_atomic_get_crtc_state);
+static void
+drm_atomic_set_out_fence_for_crtc(struct drm_atomic_state *state,
+ struct drm_crtc *crtc, u64 __user *fence_ptr)
+{
+ state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
+}
+
+static u64 __user *
+drm_atomic_get_out_fence_for_crtc(struct drm_atomic_state *state,
+struct drm_crtc *crtc)
+{
+ u64 __user *fence_ptr;
+
+ fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
+ state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
+
+ return fence_ptr;
+}
+
/**
* drm_atomic_set_mode_for_crtc - set mode for CRTC
* @state: the CRTC whose incoming state to update
@@ -490,6 +509,12 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
&replaced);
state->color_mgmt_changed |= replaced;
return ret;
+ } else if (property == config->prop_out_fence_ptr) {
+ uint64_t __user *fence_ptr = u64_to_user_ptr(val);
+ if (!access_ok(VERIFY_WRITE, fence_ptr, sizeof(*fence_ptr)))
+ return -EFAULT;
+
+ drm_atomic_set_out_fence_for_crtc(state->state, crtc,
fence_ptr);
} else if (crtc->funcs->atomic_set_property)
return crtc->funcs->atomic_set_property(crtc, state, property,
val);
else
@@ -532,6 +557,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = (state->ctm) ? state->ctm->base.id : 0;
else if (property == config->gamma_lut_property)
*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
+ else if (property == config->prop_out_fence_ptr)
+ *val = 0;
else if (crtc->funcs->atomic_get_property)
return crtc->funcs->atomic_get_property(crtc, state, property,
val);
else
@@ -1506,11 +1533,9 @@ EXPORT_SYMBOL(drm_atomic_nonblocking_commit);
*/
static struct drm_pending_vblank_event *create_vblank_event(
- struct drm_device *dev, struct drm_file *file_priv,
- struct dma_fence *fence, uint64_t user_data)
+ struct drm_device *dev, uint64_t user_data)
{
struct drm_pending_vblank_event *e = NULL;
- int ret;
e = kzalloc(sizeof *e, GFP_KERNEL);
if (!e)
@@ -1520,17 +1545,6 @@ static struct drm_pending_vblank_event
*create_vblank_event(
e->event.base.length = sizeof(e->event);
e->event.user_data = user_data;
- if (file_priv) {
- ret = drm_event_reserve_init(dev, file_priv, &e->base,
-&e->event.base);
-
Enable AMDGPU for CIK by default
On 07/11/16 11:25 PM, Bridgman, John wrote: >> From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On Behalf >> Of Michel Dänzer >> Sent: Monday, November 07, 2016 2:24 AM >> On 07/11/16 03:56 AM, Sandeep wrote: >>> >>> I was wondering when DRM_AMDGPU_CIK would be turned on by default >> in >>> the upstream kernel (or is this upto individual distros?) >>> >>> Is there any work left to be done/bugs to be fixed before it can be >>> enabled by default? >> >> There are still some functional regressions, notably amdgpu doesn't support >> HDMI/DP audio yet. >> >> Also, simply enabling DRM_AMDGPU_CIK by default isn't a good solution, >> since the radeon driver also still supports the CIK devices, and there's no >> good mechanism to choose which driver gets to drive a particular GPU at >> runtime. > > Right... we would need corresponding logic to disable CIK support in radeon. > > Would we need two flags, one for each driver, or could we define a flag at > drivers/gpu/drm level which would choose between radeon and amdgpu for > CIK hardware ? Technically, there's probably nothing preventing the radeon code from using CONFIG_DRM_AMDGPU_CIK/SI as well. > Even as I type that I don't like it... so two flags I guess. It might actually be useful to have support in drivers and be able to choose one or the other at runtime. Somebody "just" needs to come up with a suitable mechanism for that choice. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
[PATCH] drm/udl: make control msg static const.
On 08.11.2016 07:39, Dave Airlie wrote:
> From: Dave Airlie
>
> Thou shall not send control msg from the stack,
> does that mean I can send it from the RO memory area?
>
> Reported-by: poma
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/udl/udl_main.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
> index 29f0207..0798bcc 100644
> --- a/drivers/gpu/drm/udl/udl_main.c
> +++ b/drivers/gpu/drm/udl/udl_main.c
> @@ -98,10 +98,10 @@ static int udl_parse_vendor_descriptor(struct drm_device
> *dev,
> static int udl_select_std_channel(struct udl_device *udl)
> {
> int ret;
> - u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
> - 0x1C, 0x88, 0x5E, 0x15,
> - 0x60, 0xFE, 0xC6, 0x97,
> - 0x16, 0x3D, 0x47, 0xF2};
> + static const u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
> + 0x1C, 0x88, 0x5E, 0x15,
> + 0x60, 0xFE, 0xC6, 0x97,
> + 0x16, 0x3D, 0x47, 0xF2};
>
> ret = usb_control_msg(udl->udev,
> usb_sndctrlpipe(udl->udev, 0),
>
There is a compile time WARN:
$ make drivers/gpu/drm/udl/udl.ko
...
CC [M] drivers/gpu/drm/udl/udl_fb.o
drivers/gpu/drm/udl/udl_main.c: In function âudl_select_std_channelâ:
drivers/gpu/drm/udl/udl_main.c:110:10: warning: passing argument 7 of
âusb_control_msgâ discards âconstâ qualifier from pointer target type
[-Wdiscarded-qualifiers]
set_def_chn, sizeof(set_def_chn),
^~~
In file included from drivers/gpu/drm/udl/udl_drv.h:17:0,
from drivers/gpu/drm/udl/udl_main.c:14:
./include/linux/usb.h:1685:12: note: expected âvoid *â but argument is of
type âconst u8 * {aka const unsigned char *}â
extern int usb_control_msg(struct usb_device *dev, unsigned int pipe,
^~~
CC [M] drivers/gpu/drm/udl/udl_transfer.o
CC [M] drivers/gpu/drm/udl/udl_gem.o
CC [M] drivers/gpu/drm/udl/udl_dmabuf.o
LD [M] drivers/gpu/drm/udl/udl.o
MODPOST 1 modules
CC drivers/gpu/drm/udl/udl.mod.o
LD [M] drivers/gpu/drm/udl/udl.ko
Also there is a flood of waiting for request block:
$ dmesg | grep -P '(?=.*drm)(?=.*urb)'
[ 72.482211] [drm] wait for urb interrupted: ffc2 available: 0
[ 75.552227] [drm] wait for urb interrupted: ffc2 available: 0
[ 76.576146] [drm] wait for urb interrupted: ffc2 available: 0
...
[ 100.192153] [drm] wait for urb interrupted: ffc2 available: 0
[ 101.216166] [drm] wait for urb interrupted: ffc2 available: 0
[ 102.240141] [drm] wait for urb interrupted: ffc2 available: 0
...
[ 200.416146] [drm] wait for urb interrupted: ffc2 available: 0
[ 201.440208] [drm] wait for urb interrupted: ffc2 available: 0
[ 204.384162] [drm] wait for urb interrupted: ffc2 available: 0
...
[ 300.448151] [drm] wait for urb interrupted: ffc2 available: 0
[ 301.920166] [drm] wait for urb interrupted: ffc2 available: 0
[ 304.416147] [drm] wait for urb interrupted: ffc2 available: 0
...
[ 370.016157] [drm] wait for urb interrupted: ffc2 available: 0
...
And finally,
Master Chef's special, the machine is KO'd due to trying out a module reload.
[PATCH] drm/amdgpu: Attach exclusive fence to prime exported bo's. (v4)
Am 08.11.2016 um 04:15 schrieb Mario Kleiner:
> External clients which import our bo's wait only
> for exclusive dmabuf-fences, not on shared ones,
> ditto for bo's which we import from external
> providers and write to.
>
> Therefore attach exclusive fences on prime shared buffers
> if our exported buffer gets imported by an external
> client, or if we import a buffer from an external
> exporter.
>
> See discussion in thread:
> https://lists.freedesktop.org/archives/dri-devel/2016-October/122370.html
>
> Prime export tested on Intel iGPU + AMD Tonga dGPU as
> DRI3/Present Prime render offload, and with the Tonga
> standalone as primary gpu.
>
> v2: Add a wait for all shared fences before prime export,
> as suggested by Christian Koenig.
>
> v3: - Mark buffer prime_exported in amdgpu_gem_prime_pin,
> so we only use the exclusive fence when exporting a
> bo to external clients like a separate iGPU, but not
> when exporting/importing from/to ourselves as part of
> regular DRI3 fd passing.
>
> - Propagate failure of reservation_object_wait_rcu back
> to caller.
>
> v4: - Switch to a prime_shared_count counter instead of a
>flag, which gets in/decremented on prime_pin/unpin, so
>we can switch back to shared fences if all clients
>detach from our exported bo.
>
> - Also switch to exclusive fence for prime imported bo's.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95472
> Tested-by: Mike Lothian (v1)
> Signed-off-by: Mario Kleiner
> Cc: Christian König
> Cc: Michel Dänzer
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 19 +++
> 3 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 039b57e..496f72b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -459,6 +459,7 @@ struct amdgpu_bo {
> u64 metadata_flags;
> void*metadata;
> u32 metadata_size;
> + unsignedprime_shared_count;
> /* list of all virtual address to which this bo
>* is associated to
>*/
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> index 651115d..c02db01f6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> @@ -132,7 +132,7 @@ static int amdgpu_bo_list_set(struct amdgpu_device *adev,
> entry->priority = min(info[i].bo_priority,
> AMDGPU_BO_LIST_MAX_PRIORITY);
> entry->tv.bo = &entry->robj->tbo;
> - entry->tv.shared = true;
> + entry->tv.shared = !entry->robj->prime_shared_count;
>
> if (entry->robj->prefered_domains == AMDGPU_GEM_DOMAIN_GDS)
> gds_obj = entry->robj;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> index 7700dc2..31aac7e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> @@ -74,6 +74,7 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev,
> if (ret)
> return ERR_PTR(ret);
>
> + bo->prime_shared_count = 1;
> return &bo->gem_base;
> }
>
> @@ -81,13 +82,29 @@ int amdgpu_gem_prime_pin(struct drm_gem_object *obj)
> {
> struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
> int ret = 0;
> + long lret;
No need for an extra long sized variable here, just change the type of
the existing one.
The compiler can usually optimize away any size conversions perfectly fine.
With that fixed the patch is Reviewed-by: Christian König
.
Thanks a lot for the help,
Christian.
>
> ret = amdgpu_bo_reserve(bo, false);
> if (unlikely(ret != 0))
> return ret;
>
> + /*
> + * Wait for all shared fences to complete before we switch to future
> + * use of exclusive fence on this prime shared bo.
> + */
> + lret = reservation_object_wait_timeout_rcu(bo->tbo.resv, true, false,
> +MAX_SCHEDULE_TIMEOUT);
> + if (unlikely(lret < 0)) {
> + DRM_DEBUG_PRIME("Fence wait failed: %li\n", lret);
> + amdgpu_bo_unreserve(bo);
> + return lret;
> + }
> +
> /* pin buffer into GTT */
> ret = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT, NULL);
> + if (likely(ret == 0))
> + bo->prime_shared_count++;
> +
> amdgpu_bo_unreserve(bo);
> return ret;
> }
> @@ -102,6 +119,8 @@ void amdgpu_gem_prime_unpin(struct drm_gem_object *obj)
> return;
>
> amdgpu_bo_unpin
[PATCH] drm/tegra: add tiling FB modifiers
Add FB modifiers to allow user-space to specify that a surface is in one
of the two tiling formats supported by Tegra chips, and add support in
the tegradrm driver to handle them properly. This is necessary for the
display controller to directly display buffers generated by the GPU.
This feature is intended to replace the dedicated IOCTL enabled
by TEGRA_STAGING and to provide a non-staging alternative to that
solution.
Signed-off-by: Alexandre Courbot
---
drivers/gpu/drm/tegra/drm.c | 2 ++
drivers/gpu/drm/tegra/fb.c| 23 +++---
include/uapi/drm/drm_fourcc.h | 45 +++
3 files changed, 67 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index a9630c2d6cb3..36b4b30a5164 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -161,6 +161,8 @@ static int tegra_drm_load(struct drm_device *drm, unsigned
long flags)
drm->mode_config.max_width = 4096;
drm->mode_config.max_height = 4096;
+ drm->mode_config.allow_fb_modifiers = true;
+
drm->mode_config.funcs = &tegra_drm_mode_funcs;
err = tegra_drm_fb_prepare(drm);
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index e6d71fa4028e..2fded58b2ca5 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -52,9 +52,26 @@ int tegra_fb_get_tiling(struct drm_framebuffer *framebuffer,
struct tegra_bo_tiling *tiling)
{
struct tegra_fb *fb = to_tegra_fb(framebuffer);
-
- /* TODO: handle YUV formats? */
- *tiling = fb->planes[0]->tiling;
+ uint64_t modifier = fb->base.modifier[0];
+
+ switch (fourcc_mod_tegra_mod(modifier)) {
+ case NV_FORMAT_MOD_TEGRA_TILED:
+ tiling->mode = TEGRA_BO_TILING_MODE_TILED;
+ tiling->value = 0;
+ break;
+
+ case NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(0):
+ tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+ tiling->value = fourcc_mod_tegra_param(modifier);
+ if (tiling->value > 5)
+ return -EINVAL;
+ break;
+
+ default:
+ /* TODO: handle YUV formats? */
+ *tiling = fb->planes[0]->tiling;
+ break;
+ }
return 0;
}
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index a5890bf44c0a..967dfab16881 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -233,6 +233,51 @@ extern "C" {
*/
#define DRM_FORMAT_MOD_SAMSUNG_64_32_TILE fourcc_mod_code(SAMSUNG, 1)
+
+/* NVIDIA Tegra frame buffer modifiers */
+
+/*
+ * Some modifiers take parameters, for example the number of vertical GOBs in
+ * a block. Reserve the lower 32 bits for parameters
+ */
+#define __fourcc_mod_tegra_mode_shift 32
+#define fourcc_mod_tegra_code(val, params) \
+ fourcc_mod_code(NV, __u64)val) << __fourcc_mod_tegra_mode_shift) |
params))
+#define fourcc_mod_tegra_mod(m) \
+ (m & ~((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
+#define fourcc_mod_tegra_param(m) \
+ (m & ((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
+
+/*
+ * Tegra Tiled Layout, used by Tegra 2, 3 and 4.
+ *
+ * Pixels are arranged in simple tiles of 16 x 16 bytes.
+ */
+#define NV_FORMAT_MOD_TEGRA_TILED fourcc_mod_tegra_code(1, 0)
+
+/*
+ * Tegra 16Bx2 Block Linear layout, used by TK1/TX1
+ *
+ * Pixels are arranged in 64x8 Groups Of Bytes (GOBs). GOBs are then stacked
+ * vertically by a power of 2 (1 to 32 GOBs) to form a block.
+ *
+ * Within a GOB, data is ordered as 16B x 2 lines sectors laid in Z-shape.
+ *
+ * Parameter 'v' is the log2 encoding of the number of GOBs stacked vertically.
+ * Valid values are:
+ *
+ * 0 == ONE_GOB
+ * 1 == TWO_GOBS
+ * 2 == FOUR_GOBS
+ * 3 == EIGHT_GOBS
+ * 4 == SIXTEEN_GOBS
+ * 5 == THIRTYTWO_GOBS
+ *
+ * Chapter 20 "Pixel Memory Formats" of the Tegra X1 TRM describes this format
+ * in full detail.
+ */
+#define NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(v) fourcc_mod_tegra_code(2, v)
+
#if defined(__cplusplus)
}
#endif
--
2.10.2
[PATCH] drm/amd/amdgpu : Fix NULL pointer comparison
Am 08.11.2016 um 06:49 schrieb Ravikant Bijendra Sharma:
> From: Ravikant B Sharma
>
> Replace direct comparisons to NULL i.e.
> 'x == NULL' with '!x'. As per coding standard.
>
> Signed-off-by: Ravikant B Sharma
Please stop CCing random people who contributed to the driver in the
past on something as trivial as this.
The maintainers of a driver (in this case Alex and me) should be
perfectly sufficient.
Apart from that the patch is Reviewed-by: Christian König
.
Thanks,
Christian.
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c |2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c |3 +--
> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c|2 +-
> 3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> index 2b6afe1..b7e2762 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> @@ -70,7 +70,7 @@ static bool igp_read_bios_from_vram(struct amdgpu_device
> *adev)
> return false;
> }
> adev->bios = kmalloc(size, GFP_KERNEL);
> - if (adev->bios == NULL) {
> + if (!adev->bios) {
> iounmap(bios);
> return false;
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
> index d8af37a..3ecb083 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
> @@ -327,9 +327,8 @@ int amdgpu_sa_bo_new(struct amdgpu_sa_manager *sa_manager,
> return -EINVAL;
>
> *sa_bo = kmalloc(sizeof(struct amdgpu_sa_bo), GFP_KERNEL);
> - if ((*sa_bo) == NULL) {
> + if (!(*sa_bo))
> return -ENOMEM;
> - }
> (*sa_bo)->manager = sa_manager;
> (*sa_bo)->fence = NULL;
> INIT_LIST_HEAD(&(*sa_bo)->olist);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index ee6a48a..6c020ea7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -3905,7 +3905,7 @@ static int gfx_v8_0_init_save_restore_list(struct
> amdgpu_device *adev)
> int list_size;
> unsigned int *register_list_format =
> kmalloc(adev->gfx.rlc.reg_list_format_size_bytes, GFP_KERNEL);
> - if (register_list_format == NULL)
> + if (!register_list_format)
> return -ENOMEM;
> memcpy(register_list_format, adev->gfx.rlc.register_list_format,
> adev->gfx.rlc.reg_list_format_size_bytes);
[Bug 95306] Random Blank(black) screens on "Carrizo"
https://bugs.freedesktop.org/show_bug.cgi?id=95306 --- Comment #31 from Tom --- The suspend issue is still present, it just doesn't happen so often. I also noticed another thing. It seems to be impossible to cold boot with 4.9rc4, I have to boot some other kernel or Windows and then reboot to successfully boot with 4.9rc4. So far no random blanking, but it already happened before, that it ran for hours without blanking. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/5eace3d2/attachment.html>
[PATCH] drm/udl: make control msg static const. (v2)
On 08.11.2016 07:47, Dave Airlie wrote:
> From: Dave Airlie
>
> Thou shall not send control msg from the stack,
> does that mean I can send it from the RO memory area?
>
> and it looks like the answer is no, so here's
> v2 which kmemdups.
>
> Reported-by: poma
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/udl/udl_main.c | 16 +++-
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
> index 29f0207..873f010 100644
> --- a/drivers/gpu/drm/udl/udl_main.c
> +++ b/drivers/gpu/drm/udl/udl_main.c
> @@ -98,17 +98,23 @@ static int udl_parse_vendor_descriptor(struct drm_device
> *dev,
> static int udl_select_std_channel(struct udl_device *udl)
> {
> int ret;
> - u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
> - 0x1C, 0x88, 0x5E, 0x15,
> - 0x60, 0xFE, 0xC6, 0x97,
> - 0x16, 0x3D, 0x47, 0xF2};
> + static const u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
> + 0x1C, 0x88, 0x5E, 0x15,
> + 0x60, 0xFE, 0xC6, 0x97,
> + 0x16, 0x3D, 0x47, 0xF2};
> + void *sendbuf;
> +
> + sendbuf = kmemdup(set_def_chn, sizeof(set_def_chn), GFP_KERNEL);
> + if (!sendbuf)
> + return -ENOMEM;
>
> ret = usb_control_msg(udl->udev,
> usb_sndctrlpipe(udl->udev, 0),
> NR_USB_REQUEST_CHANNEL,
> (USB_DIR_OUT | USB_TYPE_VENDOR), 0, 0,
> - set_def_chn, sizeof(set_def_chn),
> + sendbuf, sizeof(set_def_chn),
> USB_CTRL_SET_TIMEOUT);
> + kfree(sendbuf);
> return ret < 0 ? ret : 0;
> }
>
>
v2 is OK.
Tested-by: poma
[Intel-gfx] [PATCH] drm: make drm_get_format_name thread-safe
On Thu, Nov 03, 2016 at 02:52:00PM -0400, Rob Clark wrote:
> On Sun, Aug 14, 2016 at 8:02 PM, Eric Engestrom wrote:
> > Signed-off-by: Eric Engestrom
> > ---
> >
> > I moved the main bits to be the first diffs, shouldn't affect anything
> > when applying the patch, but I wanted to ask:
> > I don't like the hard-coded `32` the appears in both kmalloc() and
> > snprintf(), what do you think? If you don't like it either, what would
> > you suggest? Should I #define it?
> >
> > Second question is about the patch mail itself: should I send this kind
> > of patch separated by module, with a note requesting them to be squashed
> > when applying? It has to land as a single patch, but for review it might
> > be easier if people only see the bits they each care about, as well as
> > to collect ack's/r-b's.
> >
> > Cheers,
> > Eric
> >
> > ---
> > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 6 ++--
> > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 6 ++--
> > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 6 ++--
> > drivers/gpu/drm/drm_atomic.c| 5 ++--
> > drivers/gpu/drm/drm_crtc.c | 21 -
> > drivers/gpu/drm/drm_fourcc.c| 17 ++-
> > drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 6 ++--
> > drivers/gpu/drm/i915/i915_debugfs.c | 11 ++-
> > drivers/gpu/drm/i915/intel_atomic_plane.c | 6 ++--
> > drivers/gpu/drm/i915/intel_display.c| 39
> > -
> > drivers/gpu/drm/radeon/atombios_crtc.c | 12 +---
> > include/drm/drm_fourcc.h| 2 +-
> > 12 files changed, 89 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > index 0645c85..38216a1 100644
> > --- a/drivers/gpu/drm/drm_fourcc.c
> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > @@ -39,16 +39,14 @@ static char printable_char(int c)
> > * drm_get_format_name - return a string for drm fourcc format
> > * @format: format to compute name of
> > *
> > - * Note that the buffer used by this function is globally shared and owned
> > by
> > - * the function itself.
> > - *
> > - * FIXME: This isn't really multithreading safe.
> > + * Note that the buffer returned by this function is owned by the caller
> > + * and will need to be freed.
> > */
> > const char *drm_get_format_name(uint32_t format)
> > {
> > - static char buf[32];
> > + char *buf = kmalloc(32, GFP_KERNEL);
>
>
> hmm, I guess I wasn't paying attention at the time this patch came by,
> but unfortunately this makes drm_get_format_name() no longer safe in
> atomic contexts :-/
>
> We should probably either revert this or have two variants of
> drm_get_format_name()? (ie. one that is not thread safe but is good
> enough for debugging)
Where do we need that for atomic contexts? I guess worst-case we could do
an auto-GFP trick using drm_can_sleep or something like that.
-Daniel
>
> BR,
> -R
>
> > - snprintf(buf, sizeof(buf),
> > + snprintf(buf, 32,
> > "%c%c%c%c %s-endian (0x%08x)",
> > printable_char(format & 0xff),
> > printable_char((format >> 8) & 0xff),
> > @@ -73,6 +71,8 @@ EXPORT_SYMBOL(drm_get_format_name);
> > void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> > int *bpp)
> > {
> > + const char *format_name;
> > +
> > switch (format) {
> > case DRM_FORMAT_C8:
> > case DRM_FORMAT_RGB332:
> > @@ -127,8 +127,9 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int
> > *depth,
> > *bpp = 32;
> > break;
> > default:
> > - DRM_DEBUG_KMS("unsupported pixel format %s\n",
> > - drm_get_format_name(format));
> > + format_name = drm_get_format_name(format);
> > + DRM_DEBUG_KMS("unsupported pixel format %s\n", format_name);
> > + kfree(format_name);
> > *depth = 0;
> > *bpp = 0;
> > break;
> > diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> > index 7f90a39..030d22d 100644
> > --- a/include/drm/drm_fourcc.h
> > +++ b/include/drm/drm_fourcc.h
> > @@ -32,6 +32,6 @@ int drm_format_horz_chroma_subsampling(uint32_t format);
> > int drm_format_vert_chroma_subsampling(uint32_t format);
> > int drm_format_plane_width(int width, uint32_t format, int plane);
> > int drm_format_plane_height(int height, uint32_t format, int plane);
> > -const char *drm_get_format_name(uint32_t format);
> > +const char *drm_get_format_name(uint32_t format) __malloc;
> >
> > #endif /* __DRM_FOURCC_H__ */
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> > b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> > index c1b04e9..0bf8959 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> >
[Intel-gfx] [PATCH 01/19] drm/atomic: Add new iterators over all state
On Tue, Nov 01, 2016 at 02:34:00PM +0100, Maarten Lankhorst wrote:
> Op 01-11-16 om 14:09 schreef Ville Syrjälä:
> > On Mon, Oct 17, 2016 at 02:37:00PM +0200, Maarten Lankhorst wrote:
> >> Add for_each_(old)(new)_(plane,connector,crtc)_in_state iterators to
> >> replace the old for_each_xxx_in_state ones. This is useful for >1 flip
> >> depth and getting rid of all xxx->state dereferences.
> >>
> >> Signed-off-by: Maarten Lankhorst
> >> ---
> >> drivers/gpu/drm/drm_atomic.c | 6 +++
> >> drivers/gpu/drm/drm_atomic_helper.c | 52 +--
> >> drivers/gpu/drm/i915/intel_display.c | 11 ++---
> >> include/drm/drm_atomic.h | 81
> >> ++--
> >> include/drm/drm_atomic_helper.h | 3 ++
> >> 5 files changed, 142 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> >> index 5dd70540219c..120775fcfb70 100644
> >> --- a/drivers/gpu/drm/drm_atomic.c
> >> +++ b/drivers/gpu/drm/drm_atomic.c
> >> @@ -278,6 +278,8 @@ drm_atomic_get_crtc_state(struct drm_atomic_state
> >> *state,
> >>return ERR_PTR(-ENOMEM);
> >>
> >>state->crtcs[index].state = crtc_state;
> >> + state->crtcs[index].old_state = crtc->state;
> >> + state->crtcs[index].new_state = crtc_state;
> >>state->crtcs[index].ptr = crtc;
> >>crtc_state->state = state;
> >>
> >> @@ -640,6 +642,8 @@ drm_atomic_get_plane_state(struct drm_atomic_state
> >> *state,
> >>
> >>state->planes[index].state = plane_state;
> >>state->planes[index].ptr = plane;
> >> + state->planes[index].old_state = plane->state;
> >> + state->planes[index].new_state = plane_state;
> >>plane_state->state = state;
> >>
> >>DRM_DEBUG_ATOMIC("Added [PLANE:%d:%s] %p state to %p\n",
> >> @@ -931,6 +935,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state
> >> *state,
> >>
> >>drm_connector_reference(connector);
> >>state->connectors[index].state = connector_state;
> >> + state->connectors[index].old_state = connector->state;
> >> + state->connectors[index].new_state = connector_state;
> >>state->connectors[index].ptr = connector;
> >>connector_state->state = state;
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> >> b/drivers/gpu/drm/drm_atomic_helper.c
> >> index 07b432f43b98..fcb6e5b55217 100644
> >> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >> @@ -2495,7 +2495,7 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_all);
> >> *
> >> * See also:
> >> * drm_atomic_helper_duplicate_state(), drm_atomic_helper_disable_all(),
> >> - * drm_atomic_helper_resume()
> >> + * drm_atomic_helper_resume(), drm_atomic_helper_commit_duplicated_state()
> >> */
> >> struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev)
> >> {
> >> @@ -2536,6 +2536,52 @@ unlock:
> >> EXPORT_SYMBOL(drm_atomic_helper_suspend);
> >>
> >> /**
> >> + * drm_atomic_helper_commit_duplicated_state - commit duplicated state
> >> + * @state: duplicated atomic state to commit
> >> + * @ctx: pointer to acquire_ctx to use for commit.
> >> + * @nonblock: commit the state non-blocking.
> >> + *
> >> + * The state returned by drm_atomic_helper_duplicate_state() and
> >> + * drm_atomic_helper_suspend() is partially invalid, and needs to
> >> + * be fixed up before commit.
> > So the old_state pointers are potentially invalid because whatever->state
> > may have changed since we duplicated the state?
>
> Yes when you use drm_atomic_helper_duplicate_state, and commit a different
> state before committing the duplicated state.
> This only happens during suspend/resume and gpu reset.
Kerneldoc should imo mention that suspend/resume is the only case this is
valid, and even then it depends upon the driver. Proper atomic commits
never keep drm_atomic_state around when dropping locks, so this can't
happen with an atomic ioctl. Please also directly cross-reference all
these functions.
-Daniel
>
> ~Maarten
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Intel-gfx] [PATCH 05/19] drm/atomic: Make add_affected_connectors look at crtc_state.
On Thu, Nov 03, 2016 at 05:32:42PM +0200, Ville Syrjälä wrote:
> On Mon, Oct 17, 2016 at 02:37:04PM +0200, Maarten Lankhorst wrote:
> > This kills another dereference of connector->state. connector_mask
> > holds all unchanged connectors at least and any changed connectors
> > are already in state anyway.
> >
> > Signed-off-by: Maarten Lankhorst
> > ---
> > drivers/gpu/drm/drm_atomic.c | 11 ---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 120775fcfb70..1915ec44f7eb 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1230,8 +1230,13 @@ drm_atomic_add_affected_connectors(struct
> > drm_atomic_state *state,
> > struct drm_mode_config *config = &state->dev->mode_config;
> > struct drm_connector *connector;
> > struct drm_connector_state *conn_state;
> > + struct drm_crtc_state *crtc_state;
> > int ret;
> >
> > + crtc_state = drm_atomic_get_crtc_state(state, crtc);
> > + if (IS_ERR(crtc_state))
> > + return PTR_ERR(crtc_state);
> > +
> > ret = drm_modeset_lock(&config->connection_mutex, state->acquire_ctx);
> > if (ret)
> > return ret;
> > @@ -1240,11 +1245,11 @@ drm_atomic_add_affected_connectors(struct
> > drm_atomic_state *state,
> > crtc->base.id, crtc->name, state);
> >
> > /*
> > -* Changed connectors are already in @state, so only need to look at the
> > -* current configuration.
> > +* Changed connectors are already in @state, so only need to look
> > +* at the connector_mask in crtc_state.
> > */
> > drm_for_each_connector(connector, state->dev) {
> > - if (connector->state->crtc != crtc)
> > + if (!(crtc_state->connector_mask & (1 <<
> > drm_connector_index(connector
> > continue;
>
> So this being the new crtc state, connector_mask will include all newly
> enabled
> connectors (these will have been already added to the top level state), and
> also any connector that was already enabled and isn't getting disabled (these
> are the ones we need to explicitly add to the top level state here).
>
> /me wishes the top level state would be renamed to drm_atomic_transaction or
> something...
+1 on drm_atomic_transaction, that's an awesome name. Not sure how bad it
would be to roll this out though ...
-Daniel
>
> Any connector that is getting disabled will already have been added to
> the top level state as well, so them not being included in the new crtc
> state's connectors_mask is fine.
>
> Reviewed-by: Ville Syrjälä
>
> >
> > conn_state = drm_atomic_get_connector_state(state, connector);
> > --
> > 2.7.4
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Intel-gfx] [PATCH 07/19] drm/atomic: Fix atomic helpers to use the new iterator macros.
On Thu, Nov 03, 2016 at 06:22:05PM +0200, Ville Syrjälä wrote: > This thing could use a commit message I think. Could at least lay out > the basic rules on the foo->state/foo_state vs. old_state/new_state > replacements. Might help someone understand it who doesn't know so much > about the current state swapping mechanism. Yup. I know Maarten doesn't like typing them, but except for trival&obvious stuff they're needed. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 5/5] drm/sun4i: Add support for the overscan profiles
On Tue, Oct 18, 2016 at 10:29:38AM +0200, Maxime Ripard wrote:
> Create overscan profiles reducing the displayed zone.
>
> For each TV standard (PAL and NTSC so far), we create 4 more reduced modes
> by steps of 5% that the user will be able to select.
>
> Signed-off-by: Maxime Ripard
tbh I think if we agree to do this (and that still seems an open question)
I think there should be a generic helper to add these overscan modes with
increased porches. Anything that only depends upon the sink (and
overscanning is something the sink does) should imo be put into a suitable
helper library for everyone to share.
Or maybe even stash it into the probe helpers and call it for all TV
connectors. Definitely not a driver-private thing.
-Daniel
> ---
> drivers/gpu/drm/sun4i/sun4i_tv.c | 60 +++--
> 1 file changed, 36 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c
> b/drivers/gpu/drm/sun4i/sun4i_tv.c
> index f99886462cb8..9ee03ba086b6 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
> @@ -301,27 +301,33 @@ static const struct tv_mode
> *sun4i_tv_find_tv_by_mode(const struct drm_display_m
> DRM_DEBUG_DRIVER("Comparing mode %s vs %s",
>mode->name, tv_mode->name);
>
> - if (!strcmp(mode->name, tv_mode->name))
> + if (!strncmp(mode->name, tv_mode->name, strlen(tv_mode->name)))
> return tv_mode;
> }
>
> /* Then by number of lines */
> for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
> const struct tv_mode *tv_mode = &tv_modes[i];
> + int j;
>
> - DRM_DEBUG_DRIVER("Comparing mode %s vs %s (X: %d vs %d)",
> - mode->name, tv_mode->name,
> - mode->vdisplay, tv_mode->vdisplay);
> + for (j = 0; j < 20; j += 5) {
> + u32 vdisplay = tv_mode->vdisplay * (100 - j) / 100;
>
> - if (mode->vdisplay == tv_mode->vdisplay)
> - return tv_mode;
> + DRM_DEBUG_DRIVER("Comparing mode with %s (%d) (X: %d vs
> %d)",
> + tv_mode->name, j,
> + vdisplay, tv_mode->vdisplay);
> +
> + if (vdisplay == tv_mode->vdisplay)
> + return tv_mode;
> + }
> }
>
> return NULL;
> }
>
> static void sun4i_tv_mode_to_drm_mode(const struct tv_mode *tv_mode,
> - struct drm_display_mode *mode)
> + struct drm_display_mode *mode,
> + int overscan)
> {
> DRM_DEBUG_DRIVER("Creating mode %s\n", mode->name);
>
> @@ -329,12 +335,12 @@ static void sun4i_tv_mode_to_drm_mode(const struct
> tv_mode *tv_mode,
> mode->clock = 13500;
> mode->flags = DRM_MODE_FLAG_INTERLACE;
>
> - mode->hdisplay = tv_mode->hdisplay;
> + mode->hdisplay = tv_mode->hdisplay * (100 - overscan) / 100;
> mode->hsync_start = mode->hdisplay + tv_mode->hfront_porch;
> mode->hsync_end = mode->hsync_start + tv_mode->hsync_len;
> mode->htotal = mode->hsync_end + tv_mode->hback_porch;
>
> - mode->vdisplay = tv_mode->vdisplay;
> + mode->vdisplay = tv_mode->vdisplay * (100 - overscan) / 100;
> mode->vsync_start = mode->vdisplay + tv_mode->vfront_porch;
> mode->vsync_end = mode->vsync_start + tv_mode->vsync_len;
> mode->vtotal = mode->vsync_end + tv_mode->vback_porch;
> @@ -352,10 +358,10 @@ static int sun4i_tv_atomic_check(struct drm_encoder
> *encoder,
> return -EINVAL;
>
> state->display_x_size = tv_mode->hdisplay;
> - state->plane_x_offset = 0;
> + state->plane_x_offset = (tv_mode->hdisplay - mode->hdisplay) / 2;
>
> state->display_y_size = tv_mode->vdisplay;
> - state->plane_y_offset = 0;
> + state->plane_y_offset = (tv_mode->vdisplay - mode->vdisplay) / 2;
>
> return 0;
> }
> @@ -404,7 +410,7 @@ static void sun4i_tv_mode_set(struct drm_encoder *encoder,
> struct drm_display_mode tv_drm_mode = { 0 };
>
> strcpy(tv_drm_mode.name, "TV");
> - sun4i_tv_mode_to_drm_mode(tv_mode, &tv_drm_mode);
> + sun4i_tv_mode_to_drm_mode(tv_mode, &tv_drm_mode, 0);
> drm_mode_set_crtcinfo(&tv_drm_mode, CRTC_INTERLACE_HALVE_V);
>
> sun4i_tcon1_mode_set(tcon, &tv_drm_mode);
> @@ -526,22 +532,28 @@ static int sun4i_tv_comp_get_modes(struct drm_connector
> *connector)
> int i;
>
> for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
> - struct drm_display_mode *mode;
> const struct tv_mode *tv_mode = &tv_modes[i];
> -
> - mode = drm_mode_create(connector->dev);
> - if (!mode) {
> - DRM_ERROR("Failed to create a new display mode\n");
> -
[PATCH] drm/tegra: add tiling FB modifiers
On Tue, Nov 8, 2016 at 8:50 AM, Alexandre Courbot
wrote:
> Add FB modifiers to allow user-space to specify that a surface is in one
> of the two tiling formats supported by Tegra chips, and add support in
> the tegradrm driver to handle them properly. This is necessary for the
> display controller to directly display buffers generated by the GPU.
>
> This feature is intended to replace the dedicated IOCTL enabled
> by TEGRA_STAGING and to provide a non-staging alternative to that
> solution.
>
> Signed-off-by: Alexandre Courbot
> ---
> drivers/gpu/drm/tegra/drm.c | 2 ++
> drivers/gpu/drm/tegra/fb.c| 23 +++---
> include/uapi/drm/drm_fourcc.h | 45
> +++
> 3 files changed, 67 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index a9630c2d6cb3..36b4b30a5164 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -161,6 +161,8 @@ static int tegra_drm_load(struct drm_device *drm,
> unsigned long flags)
> drm->mode_config.max_width = 4096;
> drm->mode_config.max_height = 4096;
>
> + drm->mode_config.allow_fb_modifiers = true;
> +
> drm->mode_config.funcs = &tegra_drm_mode_funcs;
>
> err = tegra_drm_fb_prepare(drm);
> diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
> index e6d71fa4028e..2fded58b2ca5 100644
> --- a/drivers/gpu/drm/tegra/fb.c
> +++ b/drivers/gpu/drm/tegra/fb.c
> @@ -52,9 +52,26 @@ int tegra_fb_get_tiling(struct drm_framebuffer
> *framebuffer,
> struct tegra_bo_tiling *tiling)
> {
> struct tegra_fb *fb = to_tegra_fb(framebuffer);
> -
> - /* TODO: handle YUV formats? */
> - *tiling = fb->planes[0]->tiling;
> + uint64_t modifier = fb->base.modifier[0];
> +
> + switch (fourcc_mod_tegra_mod(modifier)) {
> + case NV_FORMAT_MOD_TEGRA_TILED:
> + tiling->mode = TEGRA_BO_TILING_MODE_TILED;
> + tiling->value = 0;
> + break;
> +
> + case NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(0):
> + tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
> + tiling->value = fourcc_mod_tegra_param(modifier);
> + if (tiling->value > 5)
> + return -EINVAL;
Shouldn't this contain some hardware-check for the support? AFAIK, not
all Tegras support all block-heights (if even this mode at all?)...
[PATCH drm/qxl v4 1/7] qxl: Mark some internal functions as static
They are not used outside of their respective source file
Signed-off-by: Christophe Fergeau
Acked-by: Frediano Ziglio
---
drivers/gpu/drm/qxl/qxl_cmd.c | 2 +-
drivers/gpu/drm/qxl/qxl_display.c | 4 ++--
drivers/gpu/drm/qxl/qxl_drv.h | 3 ---
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index 04270f5..74fc936 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -578,7 +578,7 @@ int qxl_hw_surface_dealloc(struct qxl_device *qdev,
return 0;
}
-int qxl_update_surface(struct qxl_device *qdev, struct qxl_bo *surf)
+static int qxl_update_surface(struct qxl_device *qdev, struct qxl_bo *surf)
{
struct qxl_rect rect;
int ret;
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 3aef127..8cf5177 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -36,7 +36,7 @@ static bool qxl_head_enabled(struct qxl_head *head)
return head->width && head->height;
}
-void qxl_alloc_client_monitors_config(struct qxl_device *qdev, unsigned count)
+static void qxl_alloc_client_monitors_config(struct qxl_device *qdev, unsigned
count)
{
if (qdev->client_monitors_config &&
count > qdev->client_monitors_config->count) {
@@ -559,7 +559,7 @@ static bool qxl_crtc_mode_fixup(struct drm_crtc *crtc,
return true;
}
-void
+static void
qxl_send_monitors_config(struct qxl_device *qdev)
{
int i;
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 8e633ca..a3c1131 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -394,13 +394,11 @@ qxl_framebuffer_init(struct drm_device *dev,
struct drm_gem_object *obj,
const struct drm_framebuffer_funcs *funcs);
void qxl_display_read_client_monitors_config(struct qxl_device *qdev);
-void qxl_send_monitors_config(struct qxl_device *qdev);
int qxl_create_monitors_object(struct qxl_device *qdev);
int qxl_destroy_monitors_object(struct qxl_device *qdev);
/* used by qxl_debugfs only */
void qxl_crtc_set_from_monitors_config(struct qxl_device *qdev);
-void qxl_alloc_client_monitors_config(struct qxl_device *qdev, unsigned count);
/* qxl_gem.c */
int qxl_gem_init(struct qxl_device *qdev);
@@ -573,6 +571,5 @@ int qxl_bo_check_id(struct qxl_device *qdev, struct qxl_bo
*bo);
struct qxl_drv_surface *
qxl_surface_lookup(struct drm_device *dev, int surface_id);
void qxl_surface_evict(struct qxl_device *qdev, struct qxl_bo *surf, bool
freeing);
-int qxl_update_surface(struct qxl_device *qdev, struct qxl_bo *surf);
#endif
--
2.9.3
[PATCH drm/qxl v4 5/7] qxl: Remove qxl_bo_init() return value
It's always returning 0, and it's always ignored.
Signed-off-by: Christophe Fergeau
Acked-by: Frediano Ziglio
---
drivers/gpu/drm/qxl/qxl_drv.h | 2 +-
drivers/gpu/drm/qxl/qxl_gem.c | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 5a4720a..feac7e6 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -398,7 +398,7 @@ int qxl_create_monitors_object(struct qxl_device *qdev);
int qxl_destroy_monitors_object(struct qxl_device *qdev);
/* qxl_gem.c */
-int qxl_gem_init(struct qxl_device *qdev);
+void qxl_gem_init(struct qxl_device *qdev);
void qxl_gem_fini(struct qxl_device *qdev);
int qxl_gem_object_create(struct qxl_device *qdev, int size,
int alignment, int initial_domain,
diff --git a/drivers/gpu/drm/qxl/qxl_gem.c b/drivers/gpu/drm/qxl/qxl_gem.c
index d9746e9..3f185c4 100644
--- a/drivers/gpu/drm/qxl/qxl_gem.c
+++ b/drivers/gpu/drm/qxl/qxl_gem.c
@@ -111,10 +111,9 @@ void qxl_gem_object_close(struct drm_gem_object *obj,
{
}
-int qxl_gem_init(struct qxl_device *qdev)
+void qxl_gem_init(struct qxl_device *qdev)
{
INIT_LIST_HEAD(&qdev->gem.objects);
- return 0;
}
void qxl_gem_fini(struct qxl_device *qdev)
--
2.9.3
[PATCH drm/qxl v4 2/7] qxl: Remove unused prototype
qxl_crtc_set_from_monitors_config() is defined in qxl_drv.h but never implemented. Signed-off-by: Christophe Fergeau Acked-by: Frediano Ziglio --- drivers/gpu/drm/qxl/qxl_drv.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h index a3c1131..5a4720a 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.h +++ b/drivers/gpu/drm/qxl/qxl_drv.h @@ -397,9 +397,6 @@ void qxl_display_read_client_monitors_config(struct qxl_device *qdev); int qxl_create_monitors_object(struct qxl_device *qdev); int qxl_destroy_monitors_object(struct qxl_device *qdev); -/* used by qxl_debugfs only */ -void qxl_crtc_set_from_monitors_config(struct qxl_device *qdev); - /* qxl_gem.c */ int qxl_gem_init(struct qxl_device *qdev); void qxl_gem_fini(struct qxl_device *qdev); -- 2.9.3
[PATCH drm/qxl v4 7/7] qxl: Allow resolution which are not multiple of 8
The use of drm_cvt_mode() in qxl_add_monitors_config_modes() means that the resolutions we are going to present to user-space are going to be rounded down to a multiple of 8. In the QXL arbitrary resolution case, this is not useful. This commit forces the actual width/height that was requested by the client in the drm_display_mode structure rather than keeping the rounded version. Signed-off-by: Christophe Fergeau --- drivers/gpu/drm/qxl/qxl_display.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index 518333c..fa99481 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -199,6 +199,9 @@ static int qxl_add_monitors_config_modes(struct drm_connector *connector, mode = drm_cvt_mode(dev, head->width, head->height, 60, false, false, false); mode->type |= DRM_MODE_TYPE_PREFERRED; + mode->hdisplay = head->width; + mode->vdisplay = head->height; + drm_mode_set_name(mode); *pwidth = head->width; *pheight = head->height; drm_mode_probed_add(connector, mode); -- 2.9.3
[PATCH drm/qxl v4 3/7] qxl: Add missing '\n' to qxl_io_log() call
The message has to be terminated by a newline as it's not going to get added automatically. Signed-off-by: Christophe Fergeau Acked-by: Frediano Ziglio --- drivers/gpu/drm/qxl/qxl_fb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c index 28c1423..969c7c1 100644 --- a/drivers/gpu/drm/qxl/qxl_fb.c +++ b/drivers/gpu/drm/qxl/qxl_fb.c @@ -198,7 +198,7 @@ static int qxlfb_framebuffer_dirty(struct drm_framebuffer *fb, /* * we are using a shadow draw buffer, at qdev->surface0_shadow */ - qxl_io_log(qdev, "dirty x[%d, %d], y[%d, %d]", clips->x1, clips->x2, + qxl_io_log(qdev, "dirty x[%d, %d], y[%d, %d]\n", clips->x1, clips->x2, clips->y1, clips->y2); image->dx = clips->x1; image->dy = clips->y1; -- 2.9.3
[PATCH drm/qxl v4 4/7] qxl: Call qxl_gem_{init,fini}
qdev->gem.objects was initialized directly in qxl_device_init() rather than going through qxl_gem_init(), and qxl_gem_fini() was never called. Signed-off-by: Christophe Fergeau --- drivers/gpu/drm/qxl/qxl_kms.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c index e642242..af685f1 100644 --- a/drivers/gpu/drm/qxl/qxl_kms.c +++ b/drivers/gpu/drm/qxl/qxl_kms.c @@ -131,7 +131,7 @@ static int qxl_device_init(struct qxl_device *qdev, mutex_init(&qdev->update_area_mutex); mutex_init(&qdev->release_mutex); mutex_init(&qdev->surf_evict_mutex); - INIT_LIST_HEAD(&qdev->gem.objects); + qxl_gem_init(qdev); qdev->rom_base = pci_resource_start(pdev, 2); qdev->rom_size = pci_resource_len(pdev, 2); @@ -273,6 +273,7 @@ static void qxl_device_fini(struct qxl_device *qdev) qxl_ring_free(qdev->command_ring); qxl_ring_free(qdev->cursor_ring); qxl_ring_free(qdev->release_ring); + qxl_gem_fini(qdev); qxl_bo_fini(qdev); io_mapping_free(qdev->surface_mapping); io_mapping_free(qdev->vram_mapping); -- 2.9.3
[PATCH drm/qxl v4 6/7] qxl: Don't notify userspace when monitors config is unchanged
When the QXL driver receives a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG interrupt,
we currently always notify userspace that there was some hotplug event.
However, gnome-shell/mutter is reacting to this event by attempting a
resolution change, which it does by issueing drmModeRmFB, drmModeAddFB,
and then drmModeSetCrtc. This has the side-effect of causing
qxl_crtc_mode_set() to tell the QXL virtual hardware that a primary
surface was destroyed and created. After going through QEMU and then the
remote SPICE client, a new identical monitors config message will be
sent, resulting in a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG interrupt to
be emitted, and the same scenario occurring again.
As destroying/creating the primary surface causes a visible screen
flicker, this makes the guest hard to use (
https://bugzilla.redhat.com/show_bug.cgi?id=1266484 ).
This commit checks if the screen configuration we received is the same
one as the current one, and does not notify userspace about it if that's
the case.
Signed-off-by: Christophe Fergeau
---
drivers/gpu/drm/qxl/qxl_display.c | 62 ---
1 file changed, 52 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 8cf5177..518333c 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -57,11 +57,18 @@ static void qxl_alloc_client_monitors_config(struct
qxl_device *qdev, unsigned c
qdev->client_monitors_config->count = count;
}
+enum {
+ MONITORS_CONFIG_MODIFIED,
+ MONITORS_CONFIG_UNCHANGED,
+ MONITORS_CONFIG_BAD_CRC,
+};
+
static int qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
{
int i;
int num_monitors;
uint32_t crc;
+ int status = MONITORS_CONFIG_UNCHANGED;
num_monitors = qdev->rom->client_monitors_config.count;
crc = crc32(0, (const uint8_t *)&qdev->rom->client_monitors_config,
@@ -70,7 +77,7 @@ static int qxl_display_copy_rom_client_monitors_config(struct
qxl_device *qdev)
qxl_io_log(qdev, "crc mismatch: have %X (%zd) != %X\n", crc,
sizeof(qdev->rom->client_monitors_config),
qdev->rom->client_monitors_config_crc);
- return 1;
+ return MONITORS_CONFIG_BAD_CRC;
}
if (num_monitors > qdev->monitors_config->max_allowed) {
DRM_DEBUG_KMS("client monitors list will be truncated: %d <
%d\n",
@@ -79,6 +86,10 @@ static int
qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
} else {
num_monitors = qdev->rom->client_monitors_config.count;
}
+ if (qdev->client_monitors_config
+ && (num_monitors != qdev->client_monitors_config->count)) {
+ status = MONITORS_CONFIG_MODIFIED;
+ }
qxl_alloc_client_monitors_config(qdev, num_monitors);
/* we copy max from the client but it isn't used */
qdev->client_monitors_config->max_allowed =
@@ -88,17 +99,39 @@ static int
qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
&qdev->rom->client_monitors_config.heads[i];
struct qxl_head *client_head =
&qdev->client_monitors_config->heads[i];
- client_head->x = c_rect->left;
- client_head->y = c_rect->top;
- client_head->width = c_rect->right - c_rect->left;
- client_head->height = c_rect->bottom - c_rect->top;
- client_head->surface_id = 0;
- client_head->id = i;
- client_head->flags = 0;
+ if (client_head->x != c_rect->left) {
+ client_head->x = c_rect->left;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->y != c_rect->top) {
+ client_head->y = c_rect->top;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->width != c_rect->right - c_rect->left) {
+ client_head->width = c_rect->right - c_rect->left;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->height != c_rect->bottom - c_rect->top) {
+ client_head->height = c_rect->bottom - c_rect->top;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->surface_id != 0) {
+ client_head->surface_id = 0;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->id != i) {
+ client_head->id = i;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->flags != 0) {
+ client_head->flags = 0;
+
[PATCH drm/qxl v4 0/7] qxl: Various fixes
Hey, Here is what should hopefully be the final version of this series, only change since v3 is that I made sure to keep "PATCH" in the subject line, and I added missing Acked-by/Signed-of-by annotations to the logs. Christophe
[PATCH 2/4] drm/i2c: tda998x: Remove obsolete drm_connector_register() call
On Mon, Oct 31, 2016 at 12:09:23AM +, Russell King - ARM Linux wrote: > On Mon, Oct 24, 2016 at 08:53:04AM +0200, Daniel Vetter wrote: > > On Mon, Oct 24, 2016 at 11:58:00AM +0530, Archit Taneja wrote: > > > On 10/22/2016 03:25 PM, Russell King - ARM Linux wrote: > > > > Looking at drm_bridge_disable() and drm_bridge_enable(), the control > > > > model appears to be: > > > > > > > > crtc -> encoder -> connector > > > > `-> bridge > > > > `-> bridge > > > > `-> bridge > > > > > > > > Bridges are always attached to an encoder, and there can be multiple > > > > bridges attached to one encoder. Bridges can't be attached to the > > > > connector. > > > > In helpers connectors are no-op objects. We _never_ call any connector > > callbacks when doing a modeset. Connectors are only used to probe output > > state, and as the userspace-visisble endpoint representation. Hence the > > real graph is > > > > crtc -> encoder [ -> bridge [ -> bridge [...]]] -> connector > > > > with the last bridge owning the connector. And that last bridge probably > > needs to store a pointer to its connector(s). > > That model can't work for TDA998x if the TDA998x is followed by > another "bridge" (eg, to convert the TDMS signals to something else) > unless there's some way to tell a bridge that it isn't the last in > the chain. > > However, my graph is accurate as it's reflecting the software > modelling - it reflects how the various objects are bound together in > DRM. The DRM encoder has pointers to the DRM bridge, which has a > pointer to the next DRM bridge. The DRM connector doesn't have any > pointers to the connector, only to the DRM encoder. So, DRM bridges > are childs of the encoder, and the encoder (and attached encoder > bridge chain) can be selected by the DRM connector. Small note: The connector -> encoder pointer is only used for legacy modesetting drivers. In atomic we shoveled it into drm_connector_state as as derived state of the connector->crtc link (which is what setCrtc and atomic ioctl set). > However, you are correct that for different "tasks" like mode setting, > or output probing, the representation is somewhat different - that's > not really what I was talking about though, and I certainly was not > talking about the userspace representation. > > What I'm 100% concerned about is how this stuff looks in kernel space > and what the driver(s) should look like. Ah, I missed that. Some shared code and pointers in generic drivers to untangle which exact drm_bridge owns the connector would certainly be useful. Otoh I'm not aware of any real-world chaining existing yet, I guess that's why this is unsolved. > > > > So, in the case of TDA998x, we end up with the TDA998x providing a > > > > connector, because it has connector functionality, and providing a > > > > bridge. The encoder is left to the KMS driver, which adds additional > > > > complexity (100+ lines) to each and every KMS driver, requiring the > > > > KMS driver to have much more knowledge of what's attached to the > > > > "CRTC", so it can create these encoders itself. I still think this > > > > is a backwards step - maybe one step forwards, two backwards. > > > > We've stubbed out everything that's in an encoder, you definitely don't > > need hundreds of lines to write one any more. If there's still a callback > > left around drm_encoder which has not yet suitable default handling, then > > that's a bug. > > Sorry, but I do need exactly what I've written above, I can talk rather > definitively because I've actually got the code in front of me. Most of > the additional lines is due to the complexity added to the KMS driver to > locate (actually for a third time) all the components in the system, > specifically parsing the DT tree to find the "encoders" (or rather the > TDA998x in this case), creating the DRM encoder objects, and binding the > TDA998x bridge. > > Here's the _exact_ diffstat for the hacky conversion so far (including > something like the 10 patches I posted last weekend, which haven't had > any comments yet): > > drivers/gpu/drm/armada/armada_drv.c | 125 + > drivers/gpu/drm/i2c/tda998x_drv.c | 904 > +--- > 2 files changed, 560 insertions(+), 469 deletions(-) > > The actual bridge conversion on its own is: > > drivers/gpu/drm/armada/armada_drv.c | 125 > drivers/gpu/drm/i2c/tda998x_drv.c | 139 > ++-- > 2 files changed, 180 insertions(+), 84 deletions(-) Hm, this doesn't look good indeed ... > > Note: Only applies to atomic though, I'm not going to bother with old > > legacy crtc helpers. I guess armada needs to switch to atomic, otherwise > > encoders are indeed a bit a pain. ... but I can't justify the effort really in making non-atomic drivers look good personally. Not going to reject patches from others of course. > That's not
[PATCH] drm/tegra: add tiling FB modifiers
On 11/08/2016 06:07 PM, Erik Faye-Lund wrote:
> On Tue, Nov 8, 2016 at 8:50 AM, Alexandre Courbot
> wrote:
>> Add FB modifiers to allow user-space to specify that a surface is in one
>> of the two tiling formats supported by Tegra chips, and add support in
>> the tegradrm driver to handle them properly. This is necessary for the
>> display controller to directly display buffers generated by the GPU.
>>
>> This feature is intended to replace the dedicated IOCTL enabled
>> by TEGRA_STAGING and to provide a non-staging alternative to that
>> solution.
>>
>> Signed-off-by: Alexandre Courbot
>> ---
>> drivers/gpu/drm/tegra/drm.c | 2 ++
>> drivers/gpu/drm/tegra/fb.c| 23 +++---
>> include/uapi/drm/drm_fourcc.h | 45
>> +++
>> 3 files changed, 67 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
>> index a9630c2d6cb3..36b4b30a5164 100644
>> --- a/drivers/gpu/drm/tegra/drm.c
>> +++ b/drivers/gpu/drm/tegra/drm.c
>> @@ -161,6 +161,8 @@ static int tegra_drm_load(struct drm_device *drm,
>> unsigned long flags)
>> drm->mode_config.max_width = 4096;
>> drm->mode_config.max_height = 4096;
>>
>> + drm->mode_config.allow_fb_modifiers = true;
>> +
>> drm->mode_config.funcs = &tegra_drm_mode_funcs;
>>
>> err = tegra_drm_fb_prepare(drm);
>> diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
>> index e6d71fa4028e..2fded58b2ca5 100644
>> --- a/drivers/gpu/drm/tegra/fb.c
>> +++ b/drivers/gpu/drm/tegra/fb.c
>> @@ -52,9 +52,26 @@ int tegra_fb_get_tiling(struct drm_framebuffer
>> *framebuffer,
>> struct tegra_bo_tiling *tiling)
>> {
>> struct tegra_fb *fb = to_tegra_fb(framebuffer);
>> -
>> - /* TODO: handle YUV formats? */
>> - *tiling = fb->planes[0]->tiling;
>> + uint64_t modifier = fb->base.modifier[0];
>> +
>> + switch (fourcc_mod_tegra_mod(modifier)) {
>> + case NV_FORMAT_MOD_TEGRA_TILED:
>> + tiling->mode = TEGRA_BO_TILING_MODE_TILED;
>> + tiling->value = 0;
>> + break;
>> +
>> + case NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(0):
>> + tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
>> + tiling->value = fourcc_mod_tegra_param(modifier);
>> + if (tiling->value > 5)
>> + return -EINVAL;
>
> Shouldn't this contain some hardware-check for the support? AFAIK, not
> all Tegras support all block-heights (if even this mode at all?)...
tegra_dc_setup_window does that check later (check the test on
dc->soc->supports_block_linear). At the moment no error message is
displayed though (and it seems like we are writing a stale value in
DC_WIN_BUFFER_ADDR_MODE if the SoC doesn't support block linear and the
tiling mode is TEGRA_BO_TILING_MODE_BLOCK?)
[PATCH] drm: tda998x: mali-dp: hdlcd: refactor connector registration
On Mon, Oct 31, 2016 at 08:58:43AM +, Russell King - ARM Linux wrote:
> On Mon, Oct 24, 2016 at 10:24:42PM +0200, Daniel Vetter wrote:
> > On Mon, Oct 24, 2016 at 4:52 PM, Brian Starkey
> > wrote:
> > >>
> > >>> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
> > >>> b/drivers/gpu/drm/i2c/tda998x_drv.c
> > >>> index f4315bc..6e6fca2 100644
> > >>> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> > >>> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> > >>> @@ -1369,7 +1369,6 @@ const struct drm_connector_helper_funcs
> > >>> tda998x_connector_helper_funcs = {
> > >>>
> > >>> static void tda998x_connector_destroy(struct drm_connector *connector)
> > >>> {
> > >>> - drm_connector_unregister(connector);
> > >>> drm_connector_cleanup(connector);
> > >>> }
> > >>>
> > >>> @@ -1441,16 +1440,10 @@ static int tda998x_bind(struct device *dev,
> > >>> struct device *master, void *data)
> > >>> if (ret)
> > >>> goto err_connector;
> > >>>
> > >>> - ret = drm_connector_register(&priv->connector);
> > >>> - if (ret)
> > >>> - goto err_sysfs;
> > >>> -
> > >>
> > >>
> > >> Instead of smashing all these patches into one, what about checking here
> > >> for midlayer driver set with:
> > >>
> > >> /* register here for drivers still using midlayer load/unload */
> > >> if (dev->driver->load)
> > >> drm_connector_register(connector),
> > >>
> > >> Similar in other places. That way we wouldn't need to switch the world in
> > >> one patch.
> > >
> > >
> > > I don't think that helps. If we do that in isolation (first), then
> > > mali-dp and hdlcd won't get their connectors registered because their
> > > bind order is:
> > >
> > > drm_dev_register();
> > > component_bind_all();
> > >
> > > If we change the mali-dp/hdlcd bind order first, then tda998x will
> > > explode on drm_connector_register() until it's patched to remove that.
> > >
> > > As I mentioned in my mail to Russell, the only way I can see to avoid
> > > patching all three drivers in one go is:
> > > 1) Add (probably open-coded) drm_connector_register_all() to the end
> > > of bind in hdlcd and mali-dp
> > > 2) Patch tda998x to remove drm_connector_register()
> > > 3) Reorder hdlcd/mali-dp bind and remove the connector registration
> > > added in 1)
> > >
> > > We can do that, but it's extra churn for the same result, and none of
> > > the 5 patches will really make sense in isolation anyway.
> >
> > I thought there's also armada to take care of, which this patch would
> > break?
>
> NO NO NO NO NO. I've said this several times. Let's try it again,
> and see if it sticks.
>
> Because Armada has not been converted from a mid-layered driver, it
> is _IMMUNE_ from any patch removing the drm_connector_register() call
> in TDA998x. It does _NOT_ break in any way.
>
> Only those drivers which are de-mid-layered, and worked around the
> drm_connector_register() call inside TDA998x (eg, mali) break, because
> of the order in which they are _forced_ to call stuff.
>
> In a de-mid-layered driver, with the drm_connector_register() call in
> place in TDA998x, drm_dev_register() _MUST_ be called prior to
> component_bind_all(), otherwise you get a WARN_ON() dump from the
> kobject code. With the drm_connector_register() call removed,
> drm_dev_register() _MUST_ be called after component_bind_all() so that
> the connector is registered.
>
> It's the de-mid-layered drivers which are the problem here, not the
> mid-layered ones like Armada.
>
> > Maybe even another driver, so the hack would still be useful
> > for those other drivers. And it would have been useful if malidp/hdlcd
> > wouldn't have started out with the wrong init ordering ;-)
>
> It's forced into the "wrong init ordering" due to the kobject WARN_ON.
Hm, I entirely missed that part of the troubles. Anyway, if you all agree
on a patch I certainly won't block it, feel free to merge through suitable
trees (or I can smash it into drm-misc if that's wanted).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH 5/5] reservation: revert "wait only with non-zero timeout specified (v3)" v2
On Mon, Nov 07, 2016 at 12:55:30PM -0500, Alex Deucher wrote: > On Sun, Nov 6, 2016 at 8:07 PM, Gustavo Padovan > wrote: > > Hi Christian, > > > > 2016-10-20 Christian König : > > > >> From: Christian König > >> > >> This reverts commit fb8b7d2b9d80e1e71f379e57355936bd2b024be9. > >> > >> Otherwise signaling might never be activated on the fences. This can > >> result in infinite waiting with hardware which has unreliable interrupts. > >> > >> v2: still return one when the timeout is zero and we don't have any fences. > >> > >> Signed-off-by: Christian König > >> Reviewed-by: Chunming Zhou (v1) > >> --- > >> drivers/dma-buf/reservation.c | 5 + > >> 1 file changed, 1 insertion(+), 4 deletions(-) > > > > Reviewed-by: Gustavo Padovan > > I've rebased these patches based on the fence renaming in drm-next. > Should I pull these in through my tree or should they go in through > drm-misc or the dma-buf tree? If the later, I'll send out the rebased > patches. If there's no amdgpu deps I think best to merge through drm-misc (which now also contains dma-buf stuff, maintainers patch should go out as soon as the new drm-misc.git is live). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[Bug 97857] card detects non-existent monitor on display port
https://bugs.freedesktop.org/show_bug.cgi?id=97857 Michel Dänzer changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #12 from Michel Dänzer --- Fixed in https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=9dc79965b21967caebde575f5f5d8bf1aa2c23ab . -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/7b3e1c20/attachment.html>
[PATCH v2] drm: Track drm_mm allocators and show leaks on shutdown
On Mon, Oct 31, 2016 at 09:08:06AM +, Chris Wilson wrote:
> We can use the kernel's stack tracer and depot to record the allocation
> site of every drm_mm user. Then on shutdown, as well as warning that
> allocated nodes still reside with the drm_mm range manager, we can
> display who allocated them to aide tracking down the leak.
>
> v2: Move Kconfig around so it lies underneath the DRM options submenu.
>
> Signed-off-by: Chris Wilson
> Reviewed-by: Christian König
Applied to drm-misc, thanks.
-Daniel
> ---
> drivers/gpu/drm/Kconfig | 13 +
> drivers/gpu/drm/drm_mm.c | 74
> ++--
> include/drm/drm_mm.h | 6
> 3 files changed, 90 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 483059a22b1b..25e8e3793d29 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -33,6 +33,19 @@ config DRM_DP_AUX_CHARDEV
> read and write values to arbitrary DPCD registers on the DP aux
> channel.
>
> +config DRM_DEBUG_MM
> + bool "Insert extra checks and debug info into the DRM range managers"
> + default n
> + depends on DRM
> + select STACKDEPOT
> + help
> + Enable allocation tracking of memory manager and leak detection on
> + shutdown.
> +
> + Recommended for driver developers only.
> +
> + If in doubt, say "N".
> +
> config DRM_KMS_HELPER
> tristate
> depends on DRM
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 11d44a1e0ab3..89b891a4847f 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -104,6 +104,66 @@ static struct drm_mm_node
> *drm_mm_search_free_in_range_generic(const struct drm_
> u64 end,
> enum drm_mm_search_flags flags);
>
> +#ifdef CONFIG_DRM_DEBUG_MM
> +#define STACKDEPTH 32
> +#define BUFSZ 4096
> +
> +static noinline void save_stack(struct drm_mm_node *node)
> +{
> + unsigned long entries[STACKDEPTH];
> + struct stack_trace trace = {
> + .entries = entries,
> + .max_entries = STACKDEPTH,
> + .skip = 1
> + };
> +
> + save_stack_trace(&trace);
> + if (trace.nr_entries != 0 &&
> + trace.entries[trace.nr_entries-1] == ULONG_MAX)
> + trace.nr_entries--;
> +
> + /* May be called under spinlock, so avoid sleeping */
> + node->stack = depot_save_stack(&trace, GFP_NOWAIT);
> +}
> +
> +static void show_leaks(struct drm_mm *mm)
> +{
> + struct drm_mm_node *node;
> + unsigned long entries[STACKDEPTH];
> + char *buf;
> +
> + buf = kmalloc(BUFSZ, GFP_KERNEL);
> + if (!buf)
> + return;
> +
> + list_for_each_entry(node, &mm->head_node.node_list, node_list) {
> + struct stack_trace trace = {
> + .entries = entries,
> + .max_entries = STACKDEPTH
> + };
> +
> + if (!node->stack) {
> + DRM_ERROR("node [%08llx + %08llx]: unknown owner\n",
> + node->start, node->size);
> + continue;
> + }
> +
> + depot_fetch_stack(node->stack, &trace);
> + snprint_stack_trace(buf, BUFSZ, &trace, 0);
> + DRM_ERROR("node [%08llx + %08llx]: inserted at\n%s",
> + node->start, node->size, buf);
> + }
> +
> + kfree(buf);
> +}
> +
> +#undef STACKDEPTH
> +#undef BUFSZ
> +#else
> +static void save_stack(struct drm_mm_node *node) { }
> +static void show_leaks(struct drm_mm *mm) { }
> +#endif
> +
> #define START(node) ((node)->start)
> #define LAST(node) ((node)->start + (node)->size - 1)
>
> @@ -228,6 +288,8 @@ static void drm_mm_insert_helper(struct drm_mm_node
> *hole_node,
> list_add(&node->hole_stack, &mm->hole_stack);
> node->hole_follows = 1;
> }
> +
> + save_stack(node);
> }
>
> /**
> @@ -293,6 +355,8 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct
> drm_mm_node *node)
> node->hole_follows = 1;
> }
>
> + save_stack(node);
> +
> return 0;
> }
> EXPORT_SYMBOL(drm_mm_reserve_node);
> @@ -397,6 +461,8 @@ static void drm_mm_insert_helper_range(struct drm_mm_node
> *hole_node,
> list_add(&node->hole_stack, &mm->hole_stack);
> node->hole_follows = 1;
> }
> +
> + save_stack(node);
> }
>
> /**
> @@ -861,10 +927,12 @@ EXPORT_SYMBOL(drm_mm_init);
> * Note that it is a bug to call this function on an allocator which is not
> * clean.
> */
> -void drm_mm_takedown(struct drm_mm * mm)
> +void drm_mm_takedown(struct drm_mm *mm)
> {
> - WARN(!list_empty(&mm->head_node.node_list),
> - "Memory manager not clean during takedown.\n");
> + if (WARN(!list_empty(&mm->head_node.node_list),
> + "Memo
[PATCH 2/2] drm/i915: Enable drm_mm debug when enabling DRM_I915_DEBUG
On Sat, Oct 29, 2016 at 07:42:14PM +0100, Chris Wilson wrote: > A frequent issue that arises on shutdown is the drm_mm range manager > complaining of a leak. To aide debugging those, drm can now track the > allocation callsite and print those for the leaks. > > Signed-off-by: Chris Wilson Makes sense, and applied to drm-misc for easier merging. -Daniel > --- > drivers/gpu/drm/i915/Kconfig.debug | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/Kconfig.debug > b/drivers/gpu/drm/i915/Kconfig.debug > index cee87bfd10c4..69edf196ed03 100644 > --- a/drivers/gpu/drm/i915/Kconfig.debug > +++ b/drivers/gpu/drm/i915/Kconfig.debug > @@ -21,6 +21,7 @@ config DRM_I915_DEBUG > select PREEMPT_COUNT > select X86_MSR # used by igt/pm_rpm > select DRM_VGEM # used by igt/prime_vgem (dmabuf interop checks) > +select DRM_DEBUG_MM > default n > help >Choose this option to turn on extra driver debugging that may > affect > -- > 2.10.1 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm: update the documentation of drm_framebuffer_unregister_private
On Mon, Oct 31, 2016 at 07:59:56PM +0800, Rongrong Zou wrote:
> Add obvious description to drm_framebuffer_unregister_private()
> to explain it is deprecated.
>
> Signed-off-by: Rongrong Zou
> ---
> drivers/gpu/drm/drm_framebuffer.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_framebuffer.c
> b/drivers/gpu/drm/drm_framebuffer.c
> index 398efd6..d2b0507 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -751,6 +751,11 @@ struct drm_framebuffer *drm_framebuffer_lookup(struct
> drm_device *dev,
> * those used for fbdev. Note that the caller must hold a reference of it's
> own,
> * i.e. the object may not be destroyed through this call (since it'll lead
> to a
> * locking inversion).
> + *
> + * NOTE: This function is deprecated. For driver-private framebuffers it is
> not
> + * recommended to embed a framebuffer struct info fbdev struct, instead, a
> + * framebuffer pointer is preferred and drm_framebuffer_unreference() should
> be
> + * called when the framebuffer is to be cleaned up.
Awesome, kerneldoc updates for stuff we discussed, I really like this.
Thanks for submitting your patch, applied to drm-misc.
-Daniel
> */
> void drm_framebuffer_unregister_private(struct drm_framebuffer *fb)
> {
> --
> 1.9.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm/atomic-helper: fix reference to drm_atomic_helper_commit_planes
On Mon, Oct 31, 2016 at 10:36:46AM -0700, Stefan Agner wrote:
> The kernel-doc references drm_atomic_commit_planes() which does not
> exist. The functions name is drm_atomic_helper_commit_planes().
>
> Signed-off-by: Stefan Agner
Thanks a lot for fixing this, applied to drm-misc.
-Daniel
> ---
> include/drm/drm_modeset_helper_vtables.h | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/include/drm/drm_modeset_helper_vtables.h
> b/include/drm/drm_modeset_helper_vtables.h
> index 10e449c..72478cf 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -361,8 +361,8 @@ struct drm_crtc_helper_funcs {
>*
>* Note that the power state of the display pipe when this function is
>* called depends upon the exact helpers and calling sequence the driver
> - * has picked. See drm_atomic_commit_planes() for a discussion of the
> - * tradeoffs and variants of plane commit helpers.
> + * has picked. See drm_atomic_helper_commit_planes() for a discussion of
> + * the tradeoffs and variants of plane commit helpers.
>*
>* This callback is used by the atomic modeset helpers and by the
>* transitional plane helpers, but it is optional.
> @@ -385,8 +385,8 @@ struct drm_crtc_helper_funcs {
>*
>* Note that the power state of the display pipe when this function is
>* called depends upon the exact helpers and calling sequence the driver
> - * has picked. See drm_atomic_commit_planes() for a discussion of the
> - * tradeoffs and variants of plane commit helpers.
> + * has picked. See drm_atomic_helper_commit_planes() for a discussion of
> + * the tradeoffs and variants of plane commit helpers.
>*
>* This callback is used by the atomic modeset helpers and by the
>* transitional plane helpers, but it is optional.
> @@ -940,8 +940,8 @@ struct drm_plane_helper_funcs {
>*
>* Note that the power state of the display pipe when this function is
>* called depends upon the exact helpers and calling sequence the driver
> - * has picked. See drm_atomic_commit_planes() for a discussion of the
> - * tradeoffs and variants of plane commit helpers.
> + * has picked. See drm_atomic_helper_commit_planes() for a discussion of
> + * the tradeoffs and variants of plane commit helpers.
>*
>* This callback is used by the atomic modeset helpers and by the
>* transitional plane helpers, but it is optional.
> @@ -963,8 +963,8 @@ struct drm_plane_helper_funcs {
>*
>* Note that the power state of the display pipe when this function is
>* called depends upon the exact helpers and calling sequence the driver
> - * has picked. See drm_atomic_commit_planes() for a discussion of the
> - * tradeoffs and variants of plane commit helpers.
> + * has picked. See drm_atomic_helper_commit_planes() for a discussion of
> + * the tradeoffs and variants of plane commit helpers.
>*
>* This callback is used by the atomic modeset helpers and by the
>* transitional plane helpers, but it is optional.
> --
> 2.10.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Spice-devel] [PATCH drm/qxl v4 6/7] qxl: Don't notify userspace when monitors config is unchanged
> When the QXL driver receives a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG
> interrupt,
> we currently always notify userspace that there was some hotplug event.
>
> However, gnome-shell/mutter is reacting to this event by attempting a
> resolution change, which it does by issueing drmModeRmFB, drmModeAddFB,
> and then drmModeSetCrtc. This has the side-effect of causing
> qxl_crtc_mode_set() to tell the QXL virtual hardware that a primary
> surface was destroyed and created. After going through QEMU and then the
> remote SPICE client, a new identical monitors config message will be
> sent, resulting in a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG interrupt to
> be emitted, and the same scenario occurring again.
>
> As destroying/creating the primary surface causes a visible screen
> flicker, this makes the guest hard to use (
> https://bugzilla.redhat.com/show_bug.cgi?id=1266484 ).
>
> This commit checks if the screen configuration we received is the same
> one as the current one, and does not notify userspace about it if that's
> the case.
>
> Signed-off-by: Christophe Fergeau
> ---
> drivers/gpu/drm/qxl/qxl_display.c | 62
> ---
> 1 file changed, 52 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c
> b/drivers/gpu/drm/qxl/qxl_display.c
> index 8cf5177..518333c 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -57,11 +57,18 @@ static void qxl_alloc_client_monitors_config(struct
> qxl_device *qdev, unsigned c
> qdev->client_monitors_config->count = count;
> }
>
> +enum {
> + MONITORS_CONFIG_MODIFIED,
> + MONITORS_CONFIG_UNCHANGED,
> + MONITORS_CONFIG_BAD_CRC,
> +};
> +
> static int qxl_display_copy_rom_client_monitors_config(struct qxl_device
> *qdev)
> {
> int i;
> int num_monitors;
> uint32_t crc;
> + int status = MONITORS_CONFIG_UNCHANGED;
>
> num_monitors = qdev->rom->client_monitors_config.count;
> crc = crc32(0, (const uint8_t *)&qdev->rom->client_monitors_config,
> @@ -70,7 +77,7 @@ static int
> qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
> qxl_io_log(qdev, "crc mismatch: have %X (%zd) != %X\n", crc,
> sizeof(qdev->rom->client_monitors_config),
> qdev->rom->client_monitors_config_crc);
> - return 1;
> + return MONITORS_CONFIG_BAD_CRC;
> }
> if (num_monitors > qdev->monitors_config->max_allowed) {
> DRM_DEBUG_KMS("client monitors list will be truncated: %d <
> %d\n",
> @@ -79,6 +86,10 @@ static int
> qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
> } else {
> num_monitors = qdev->rom->client_monitors_config.count;
> }
> + if (qdev->client_monitors_config
> + && (num_monitors != qdev->client_monitors_config->count)) {
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> qxl_alloc_client_monitors_config(qdev, num_monitors);
> /* we copy max from the client but it isn't used */
> qdev->client_monitors_config->max_allowed =
> @@ -88,17 +99,39 @@ static int
> qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
> &qdev->rom->client_monitors_config.heads[i];
> struct qxl_head *client_head =
> &qdev->client_monitors_config->heads[i];
> - client_head->x = c_rect->left;
> - client_head->y = c_rect->top;
> - client_head->width = c_rect->right - c_rect->left;
> - client_head->height = c_rect->bottom - c_rect->top;
> - client_head->surface_id = 0;
> - client_head->id = i;
> - client_head->flags = 0;
> + if (client_head->x != c_rect->left) {
> + client_head->x = c_rect->left;
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> + if (client_head->y != c_rect->top) {
> + client_head->y = c_rect->top;
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> + if (client_head->width != c_rect->right - c_rect->left) {
> + client_head->width = c_rect->right - c_rect->left;
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> + if (client_head->height != c_rect->bottom - c_rect->top) {
> + client_head->height = c_rect->bottom - c_rect->top;
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> + if (client_head->surface_id != 0) {
> + client_head->surface_id = 0;
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> + if (client_head->id != i) {
> + client_head->id = i;
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> +
[PATCH] drm/gma500: make function static to eliminate compiling warning
On Tue, Nov 01, 2016 at 11:49:45AM +0800, Jiang Biao wrote:
> psb_gtt_remove is only used in this file, and make it static to
> eliminate missing-prototypes compiling warning.
>
> Signed-off-by: Jiang Biao
Applied to drm-misc, thanks.
-Daniel
> ---
> drivers/gpu/drm/gma500/gtt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
> index 8f69225..e505f94 100644
> --- a/drivers/gpu/drm/gma500/gtt.c
> +++ b/drivers/gpu/drm/gma500/gtt.c
> @@ -130,7 +130,7 @@ static int psb_gtt_insert(struct drm_device *dev, struct
> gtt_range *r,
> * page table entries with the dummy page. This is protected via the gtt
> * mutex which the caller must hold.
> */
> -void psb_gtt_remove(struct drm_device *dev, struct gtt_range *r)
> +static void psb_gtt_remove(struct drm_device *dev, struct gtt_range *r)
> {
> struct drm_psb_private *dev_priv = dev->dev_private;
> u32 __iomem *gtt_slot;
> --
> 2.1.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm: move check for min/max width/height for atomic drivers
On Wed, Nov 02, 2016 at 10:27:44AM -0400, Rob Clark wrote:
> drm-hwc + android tries to create an fb for the wallpaper layer, which
> is larger than the screen resolution, and potentially larger than
> mode_config->max_{width,height}. But the plane src_w/src_h is within
> the max limits, so it is something the hardware can otherwise do.
>
> For atomic drivers, defer the check to drm_atomic_plane_check().
>
> Signed-off-by: Rob Clark
Top-posting since the discussion kinda disintegrated, but I think an
optional max_fb_w/h (if 0 we use the screen limits) would be the better
approach. More common code ftw at least.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic.c | 17 +
> drivers/gpu/drm/drm_crtc.c | 26 +-
> 2 files changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 34edd4f..fb0f07ce 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -846,7 +846,9 @@ plane_switching_crtc(struct drm_atomic_state *state,
> static int drm_atomic_plane_check(struct drm_plane *plane,
> struct drm_plane_state *state)
> {
> + struct drm_mode_config *config = &plane->dev->mode_config;
> unsigned int fb_width, fb_height;
> + unsigned int min_width, max_width, min_height, max_height;
> int ret;
>
> /* either *both* CRTC and FB must be set, or neither */
> @@ -909,6 +911,21 @@ static int drm_atomic_plane_check(struct drm_plane
> *plane,
> return -ENOSPC;
> }
>
> + min_width = config->min_width << 16;
> + max_width = config->max_width << 16;
> + min_height = config->min_height << 16;
> + max_height = config->max_height << 16;
> +
> + /* Make sure source dimensions are within bounds. */
> + if (min_width > state->src_w || state->src_w > max_width ||
> + min_height > state->src_h || state->src_h > max_height) {
> + DRM_DEBUG_ATOMIC("Invalid source size "
> + "%u.%06ux%u.%06u\n",
> + state->src_w >> 16, ((state->src_w & 0x) *
> 15625) >> 10,
> + state->src_h >> 16, ((state->src_h & 0x) *
> 15625) >> 10);
> + return -ERANGE;
> + }
> +
> if (plane_switching_crtc(state->state, plane, state)) {
> DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
>plane->base.id, plane->name);
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index b4b973f..7294bde 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -3369,15 +3369,23 @@ internal_framebuffer_create(struct drm_device *dev,
> return ERR_PTR(-EINVAL);
> }
>
> - if ((config->min_width > r->width) || (r->width > config->max_width)) {
> - DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <=
> %d\n",
> - r->width, config->min_width, config->max_width);
> - return ERR_PTR(-EINVAL);
> - }
> - if ((config->min_height > r->height) || (r->height >
> config->max_height)) {
> - DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <=
> %d\n",
> - r->height, config->min_height, config->max_height);
> - return ERR_PTR(-EINVAL);
> + /* for atomic drivers, we check the src dimensions in
> + * drm_atomic_plane_check().. allow creation of a fb
> + * that is larger than what can be scanned out, as
> + * long as userspace doesn't try to scanout a portion
> + * of the fb that is too large.
> + */
> + if (!file_priv->atomic) {
> + if ((config->min_width > r->width) || (r->width >
> config->max_width)) {
> + DRM_DEBUG_KMS("bad framebuffer width %d, should be >=
> %d && <= %d\n",
> + r->width, config->min_width,
> config->max_width);
> + return ERR_PTR(-EINVAL);
> + }
> + if ((config->min_height > r->height) || (r->height >
> config->max_height)) {
> + DRM_DEBUG_KMS("bad framebuffer height %d, should be >=
> %d && <= %d\n",
> + r->height, config->min_height,
> config->max_height);
> + return ERR_PTR(-EINVAL);
> + }
> }
>
> if (r->flags & DRM_MODE_FB_MODIFIERS &&
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[git pull] drm fixes for 4.9-rc4
On Fri, Nov 04, 2016 at 05:58:55PM -0400, Theodore Ts'o wrote: > On Fri, Nov 04, 2016 at 01:38:25PM -0700, Linus Torvalds wrote: > > On Wed, Nov 2, 2016 at 5:31 PM, Dave Airlie wrote: > > > > > > There are a set of fixes for an oops we've been seeing around MST > > > display unplug, > > > > Side note: I heard from a couple of people at the KS that said that > > they had had problems with suspend/resume after plugging in to a 4k > > display (but _only_ a 4k display - apparently normal FHD displays > > didn't show this). I think at least one was USB3/Thunderbolt. Ted with > > a Lenovo laptop (intel GPU) was one, I forget who else mentioned this. > > Actually, it's after a unplugging from a Dell 30" monitor with a 3k > display (2560 x 1920). This is after I've carefully deactivated the > video output to the Dell 30" monitor, unplugged the Dell 30" monitor > (at which point the system becomes non-responsive for 2-3 seconds for > reasons unknown), and only suspending after the system has recovered > from the unplug. > > At that point, it's a 20-30% chance that the system will never come > back after a suspend. So I have to make a point of saving all of my > editor buffers, etc., since I never can know whether my laptop will > come back. > > This was happening for years and years on the T540p laptop, as well as > my new T460 laptop. I've complained about this in the past, and > gotten no response, and I've just gotten used to the fact that if I'm > transitioning from home (where I have the 30" display) to work, > there's a good chance the resume will lock up, and I will be forced to > push the power button for 8 seconds to forcibly power down the laptop > to recover from the suspend. :-( > > I agree with Linus's suspicion that I probably need to bite the bullet > and just buy a new SST monitor, and that will probably make the > problem go away. But if the bug can be fixed, that would be really > great. This pull contains a bunch of fixes for oopses on mst connector unplugging, and iirc those started popping up with the connector refcounting we landed in 4.8. Definitely should retest with this before spending more time debugging. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 1/2] drm/edid: Add the missing "Hz" to VIC 58,59 comment
On Fri, Nov 04, 2016 at 08:29:11AM +0100, Andrzej Hajda wrote:
> On 03.11.2016 13:53, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > All the VICs apart from 58 and 59 have the word "Hz" included in the
> > comment. Include it for 59 and 59 as well.
> >
> > Cc: Shashank Sharma
> > Cc: Andrzej Hajda
> > Signed-off-by: Ville Syrjälä
> Reviewed-by: Andrzej Hajda
Applied to drm-misc. Can you pls also review patch 2/2?
Thanks, Daniel
> --
> Regards
> Andrzej
>
> > ---
> > drivers/gpu/drm/drm_edid.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 9506933b41cd..7eec18925b70 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -957,13 +957,13 @@ static const struct drm_display_mode edid_cea_modes[]
> > = {
> >798, 858, 0, 480, 489, 495, 525, 0,
> >DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
> > .vrefresh = 240, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
> > - /* 58 - 720(1440)x480i at 240 */
> > + /* 58 - 720(1440)x480i at 240Hz */
> > { DRM_MODE("720x480i", DRM_MODE_TYPE_DRIVER, 54000, 720, 739,
> >801, 858, 0, 480, 488, 494, 525, 0,
> >DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC |
> > DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK),
> > .vrefresh = 240, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3, },
> > - /* 59 - 720(1440)x480i at 240 */
> > + /* 59 - 720(1440)x480i at 240Hz */
> > { DRM_MODE("720x480i", DRM_MODE_TYPE_DRIVER, 54000, 720, 739,
> >801, 858, 0, 480, 488, 494, 525, 0,
> >DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC |
>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm/uapi: Add a warning that mode flags must match the xrandr definitions
On Thu, Nov 03, 2016 at 04:10:01PM +0200, ville.syrjala at linux.intel.com
wrote:
> From: Ville Syrjälä
>
> Existing userspace expected the mode flags to match the xrandr
> definitions 1:1, and even adding new flags in he previously unused
> bits is likely to break existing userspace. Add a comment warning
> people about this potential trap.
>
> Cc: Shashank Sharma
> Cc: "Lin, Jia"
> Cc: Akashdeep Sharma
> Cc: Jim Bride
> Cc: Jose Abreu
> Cc: Daniel Vetter
> Cc: Emil Velikov
> Signed-off-by: Ville Syrjälä
Applied to drm-misc, thanks.
-Daniel
> ---
> include/uapi/drm/drm_mode.h | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 084b50a02dc5..01000c9f7c2c 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -47,7 +47,15 @@ extern "C" {
> #define DRM_MODE_TYPE_DRIVER (1<<6)
>
> /* Video mode flags */
> -/* bit compatible with the xorg definitions. */
> +/* bit compatible with the xrandr RR_ definitions (bits 0-13)
> + *
> + * ABI warning: Existing userspace really expects
> + * the mode flags to match the xrandr definitions. Any
> + * changes that don't match the xrandr definitions will
> + * likely need a new client cap or some other mechanism
> + * to avoid breaking existing userspace. This includes
> + * allocating new flags in the previously unused bits!
> + */
> #define DRM_MODE_FLAG_PHSYNC (1<<0)
> #define DRM_MODE_FLAG_NHSYNC (1<<1)
> #define DRM_MODE_FLAG_PVSYNC (1<<2)
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm/amdgpu: Attach exclusive fence to prime exported bo's. (v3)
Am 07.11.2016 um 20:20 schrieb Mario Kleiner: > On 11/07/2016 08:55 AM, Christian König wrote: >> Am 07.11.2016 um 04:29 schrieb Michel Dänzer: >> Also, I think we should set bo->prime_exported (prime_shared?) in >> amdgpu_gem_prime_import_sg_table as well, so that we'll also set >> exclusive fences on BOs imported from other GPUs. >> Yes, really good idea. >> > > Ok. I guess we don't need to wait for fences there before setting the > flag/counter, as we can't have done anything yet with the bo just > created from the imported sg_table? Correct, waiting at this point shouldn't be necessary. Regards, Christian.
[Intel-gfx] [PATCH v2] drm: move allocation out of drm_get_format_name()
On Mon, Nov 07, 2016 at 07:38:25PM +0200, Jani Nikula wrote:
> On Mon, 07 Nov 2016, Eric Engestrom wrote:
> > On Monday, 2016-11-07 10:10:13 +0200, Jani Nikula wrote:
> >> On Mon, 07 Nov 2016, Eric Engestrom wrote:
> >> > Fixes: 90844f00049e9f42573fd31d7c32e8fd31d3fd07
> >> >
> >> > drm: make drm_get_format_name thread-safe
> >> >
> >> > Signed-off-by: Eric Engestrom
> >> > [danvet: Clarify that the returned pointer must be freed with
> >> > kfree().]
> >> > Signed-off-by: Daniel Vetter
> >>
> >> The Fixes: format is:
> >>
> >> Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe")
> >>
> >> But is this a fix, really, or just an improvement? What exactly is the
> >> bug being fixed? The commit message is not sufficient.
> >
> > "The function's behaviour was changed in 90844f00049e, without changing
> > its signature, causing people to keep using it the old way without
> > realising they were now leaking memory.
> > Rob Clark also noticed it was also allocating GFP_KERNEL memory in
> > atomic contexts, breaking them.
> >
> > Instead of having to allocate GFP_ATOMIC memory and fixing the callers
> > to make them cleanup the memory afterwards, let's change the function's
> > signature by having the caller take care of the memory and passing it to
> > the function.
> > The new parameter is a single-field struct in order to enforce the size
> > of its buffer and help callers to correctly manage their memory."
> >
> > Does this sound good?
>
> It's fine; no need to go overboard. ;)
Can you pls resend with that and corrected Fixes and all the acks
collected?
-Daniel
>
> BR,
> Jani.
>
> >
> >> > @@ -54,6 +62,6 @@ int drm_format_horz_chroma_subsampling(uint32_t
> >> > format);
> >> > int drm_format_vert_chroma_subsampling(uint32_t format);
> >> > int drm_format_plane_width(int width, uint32_t format, int plane);
> >> > int drm_format_plane_height(int height, uint32_t format, int plane);
> >> > -char *drm_get_format_name(uint32_t format) __malloc;
> >> > +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf
> >> > *buf);
> >>
> >> I wonder if it would be better to make that return "const char *". If
> >> the user really wants to look under the hood, there's buf->str. *shrug*
> >
> > Good idea, I'll do that in v3 with the proper commit msg and tags. It'll
> > have to wait another day though, -ENOTIME and all.
> >
> >>
> >> With the commit message improved,
> >>
> >> Reviewed-by: Jani Nikula
> >
> > Cheers :)
> > Eric
>
> --
> Jani Nikula, Intel Open Source Technology Center
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm: don't override possible_crtcs for primary/cursor planes
On Sat, Nov 05, 2016 at 10:52:01AM -0400, Rob Clark wrote: > It is kind of a pointless restriction. If userspace does silly things > like using crtcA's cursor plane on crtcB, and then setcursor on crtcA, > it will end up with the overlay disabled on crtcB. But userspace is > allowed to shoot itself like this. > > v2: don't WARN_ON() if caller did not set ->possible_crtcs. This keeps > the existing behavior by default, if caller does not set the > ->possible_crtcs. > > Signed-off-by: Rob Clark Applied to drm-misc, thanks. Should we maybe also update the kerneldoc a bit in struct drm_plane? -Daniel > --- > drivers/gpu/drm/drm_crtc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 13441e2..ce274ed 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -229,9 +229,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, > struct drm_crtc *crtc, > > crtc->primary = primary; > crtc->cursor = cursor; > - if (primary) > + if (primary && !primary->possible_crtcs) > primary->possible_crtcs = 1 << drm_crtc_index(crtc); > - if (cursor) > + if (cursor && !cursor->possible_crtcs) > cursor->possible_crtcs = 1 << drm_crtc_index(crtc); > > ret = drm_crtc_crc_init(crtc); > -- > 2.7.4 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH v2 4/4] drm/plane: add inline doc for struct drm_plane
On Mon, Nov 07, 2016 at 07:03:33PM +0900, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Some of the members of struct drm_plane had extra comments so for these
> add inline kernel comment to consolidate all documentation in one place.
>
> Signed-off-by: Gustavo Padovan
For in-line kerneldoc I think we want real paragraphs with real sentences,
just looks better. I bikesheded your patch to suit and applied the entire
series to drm-misc.
Thanks, Daniel
> ---
> include/drm/drm_plane.h | 61
> +++--
> 1 file changed, 49 insertions(+), 12 deletions(-)
>
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 68f6d22..683b170 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -32,11 +32,6 @@ struct drm_crtc;
> /**
> * struct drm_plane_state - mutable plane state
> * @plane: backpointer to the plane
> - * @crtc: currently bound CRTC, NULL if disabled
> - * @fb: currently bound framebuffer
> - * @fence: optional fence to wait for before scanning out @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
> @@ -51,18 +46,56 @@ struct drm_crtc;
> * where N is the number of active planes for given crtc
> * @src: clipped source coordinates of the plane (in 16.16)
> * @dst: clipped destination coordinates of the plane
> - * @visible: visibility of the plane
> * @state: backpointer to global drm_atomic_state
> */
> struct drm_plane_state {
> struct drm_plane *plane;
>
> - struct drm_crtc *crtc; /* do not write directly, use
> drm_atomic_set_crtc_for_plane() */
> - struct drm_framebuffer *fb; /* do not write directly, use
> drm_atomic_set_fb_for_plane() */
> - struct dma_fence *fence; /* do not write directly, use
> drm_atomic_set_fence_for_plane() */
> + /**
> + * @crtc:
> + *
> + * currently bound CRTC, NULL if disabled
> + *
> + * do not write directly, use drm_atomic_set_crtc_for_plane()
> + */
> + struct drm_crtc *crtc;
> +
> + /**
> + * @fb:
> + *
> + * currently bound framebuffer
> + *
> + * do not write directly, use drm_atomic_set_fb_for_plane()
> + */
> + struct drm_framebuffer *fb;
> +
> + /**
> + * @fence:
> + *
> + * optional fence to wait for before scanning out @fb
> + *
> + * do not write directly, use drm_atomic_set_fence_for_plane()
> + */
> + struct dma_fence *fence;
> +
> + /**
> + * @crtc_x:
> + *
> + * left position of visible portion of plane on crtc
> + *
> + * Signed dest location allows it to be partially off screen.
> + */
> +
> + int32_t crtc_x;
> + /**
> + * @crtc_y:
> + *
> + * upper position of visible portion of plane on crtc
> + *
> + * Signed dest location allows it to be partially off screen.
> + */
> + int32_t crtc_y;
>
> - /* Signed dest location allows it to be partially off screen */
> - int32_t crtc_x, crtc_y;
> uint32_t crtc_w, crtc_h;
>
> /* Source values are 16.16 fixed point */
> @@ -79,7 +112,11 @@ struct drm_plane_state {
> /* Clipped coordinates */
> struct drm_rect src, dst;
>
> - /*
> + /**
> + * @visible:
> + *
> + * visibility of the plane
> + *
>* Is the plane actually visible? Can be false even
>* if fb!=NULL and crtc!=NULL, due to clipping.
>*/
> --
> 2.5.5
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm/tegra: add tiling FB modifiers
On Tue, Nov 08, 2016 at 04:50:42PM +0900, Alexandre Courbot wrote:
> Add FB modifiers to allow user-space to specify that a surface is in one
> of the two tiling formats supported by Tegra chips, and add support in
> the tegradrm driver to handle them properly. This is necessary for the
> display controller to directly display buffers generated by the GPU.
>
> This feature is intended to replace the dedicated IOCTL enabled
> by TEGRA_STAGING and to provide a non-staging alternative to that
> solution.
>
> Signed-off-by: Alexandre Courbot
Ack on the drm_fourcc.h part, I think that's the amount of detail in
comments that's reasonable. Feel free to merge through tegra trees.
-Daniel
> ---
> drivers/gpu/drm/tegra/drm.c | 2 ++
> drivers/gpu/drm/tegra/fb.c| 23 +++---
> include/uapi/drm/drm_fourcc.h | 45
> +++
> 3 files changed, 67 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index a9630c2d6cb3..36b4b30a5164 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -161,6 +161,8 @@ static int tegra_drm_load(struct drm_device *drm,
> unsigned long flags)
> drm->mode_config.max_width = 4096;
> drm->mode_config.max_height = 4096;
>
> + drm->mode_config.allow_fb_modifiers = true;
> +
> drm->mode_config.funcs = &tegra_drm_mode_funcs;
>
> err = tegra_drm_fb_prepare(drm);
> diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
> index e6d71fa4028e..2fded58b2ca5 100644
> --- a/drivers/gpu/drm/tegra/fb.c
> +++ b/drivers/gpu/drm/tegra/fb.c
> @@ -52,9 +52,26 @@ int tegra_fb_get_tiling(struct drm_framebuffer
> *framebuffer,
> struct tegra_bo_tiling *tiling)
> {
> struct tegra_fb *fb = to_tegra_fb(framebuffer);
> -
> - /* TODO: handle YUV formats? */
> - *tiling = fb->planes[0]->tiling;
> + uint64_t modifier = fb->base.modifier[0];
> +
> + switch (fourcc_mod_tegra_mod(modifier)) {
> + case NV_FORMAT_MOD_TEGRA_TILED:
> + tiling->mode = TEGRA_BO_TILING_MODE_TILED;
> + tiling->value = 0;
> + break;
> +
> + case NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(0):
> + tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
> + tiling->value = fourcc_mod_tegra_param(modifier);
> + if (tiling->value > 5)
> + return -EINVAL;
> + break;
> +
> + default:
> + /* TODO: handle YUV formats? */
> + *tiling = fb->planes[0]->tiling;
> + break;
> + }
>
> return 0;
> }
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index a5890bf44c0a..967dfab16881 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -233,6 +233,51 @@ extern "C" {
> */
> #define DRM_FORMAT_MOD_SAMSUNG_64_32_TILEfourcc_mod_code(SAMSUNG, 1)
>
> +
> +/* NVIDIA Tegra frame buffer modifiers */
> +
> +/*
> + * Some modifiers take parameters, for example the number of vertical GOBs in
> + * a block. Reserve the lower 32 bits for parameters
> + */
> +#define __fourcc_mod_tegra_mode_shift 32
> +#define fourcc_mod_tegra_code(val, params) \
> + fourcc_mod_code(NV, __u64)val) << __fourcc_mod_tegra_mode_shift) |
> params))
> +#define fourcc_mod_tegra_mod(m) \
> + (m & ~((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
> +#define fourcc_mod_tegra_param(m) \
> + (m & ((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
> +
> +/*
> + * Tegra Tiled Layout, used by Tegra 2, 3 and 4.
> + *
> + * Pixels are arranged in simple tiles of 16 x 16 bytes.
> + */
> +#define NV_FORMAT_MOD_TEGRA_TILED fourcc_mod_tegra_code(1, 0)
> +
> +/*
> + * Tegra 16Bx2 Block Linear layout, used by TK1/TX1
> + *
> + * Pixels are arranged in 64x8 Groups Of Bytes (GOBs). GOBs are then stacked
> + * vertically by a power of 2 (1 to 32 GOBs) to form a block.
> + *
> + * Within a GOB, data is ordered as 16B x 2 lines sectors laid in Z-shape.
> + *
> + * Parameter 'v' is the log2 encoding of the number of GOBs stacked
> vertically.
> + * Valid values are:
> + *
> + * 0 == ONE_GOB
> + * 1 == TWO_GOBS
> + * 2 == FOUR_GOBS
> + * 3 == EIGHT_GOBS
> + * 4 == SIXTEEN_GOBS
> + * 5 == THIRTYTWO_GOBS
> + *
> + * Chapter 20 "Pixel Memory Formats" of the Tegra X1 TRM describes this
> format
> + * in full detail.
> + */
> +#define NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(v) fourcc_mod_tegra_code(2, v)
> +
> #if defined(__cplusplus)
> }
> #endif
> --
> 2.10.2
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH v7 0/3] drm: add explict fencing
On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: > From: Gustavo Padovan > > Hi, > > This is yet another version of the DRM fences patches. Please refer > to the cover letter[1] in a previous version to check for more details. Explicit fencing is not a superset of the implicit fences. The driver may be using implicit fences (on a reservation object) to serialise asynchronous operations wrt to each other (such as dispatching threads to flush cpu caches to memory, manipulating page tables and the like before the flip). Since the user doesn't know about these operations, they are not included in the explicit fence they provide, at which point we can't trust their fence to the exclusion of the implicit fences... -Chris -- Chris Wilson, Intel Open Source Technology Centre
linux-next: manual merge of the tip tree with the drm-intel tree
On Tue, Nov 08, 2016 at 03:25:41PM +1100, Stephen Rothwell wrote:
> Hi all,
>
> FIXME: Add owner of second tree to To:
>Add author(s)/SOB of conflicting commits.
>
> Today's linux-next merge of the tip tree got a conflict in:
>
> drivers/gpu/drm/i915/i915_gem_shrinker.c
>
> between commits:
>
> 1233e2db199d ("drm/i915: Move object backing storage manipulation to its
> own locking")
>
> from the drm-intel tree and commit:
>
> 3ab7c086d5ec ("locking/drm: Kill mutex trickery")
> c7faee2109f9 ("locking/drm: Fix i915_gem_shrinker_lock() locking")
Hm, this seems to be the older versions that nuke the recursive locking
trickery entirely, I thought we had version in-flight that kept that? I
know that the i915 (and msm locking fwiw) is horrible since essentially
it's a recursive BKL, and we're working (slowly, after all getting rid of
the BKL wasn't simple either) to fix this. But meanwhile I'm assuming that
we'll still need this to be able to get out of low memory situations in
i915. Has that part simply not yet landed?
Thanks, Daniel
>
> from the tip tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc drivers/gpu/drm/i915/i915_gem_shrinker.c
> index a6fc1bdc48af,e9bd2a81d03a..
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@@ -35,33 -35,6 +35,15 @@@
> #include "i915_drv.h"
> #include "i915_trace.h"
>
> - static bool mutex_is_locked_by(struct mutex *mutex, struct task_struct
> *task)
> - {
> - if (!mutex_is_locked(mutex))
> - return false;
> -
> - #if defined(CONFIG_DEBUG_MUTEXES) || defined(CONFIG_MUTEX_SPIN_ON_OWNER)
> - return mutex->owner == task;
> - #else
> - /* Since UP may be pre-empted, we cannot assume that we own the lock */
> - return false;
> - #endif
> - }
> -
> +static bool i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
> +{
> - if (!mutex_trylock(&dev->struct_mutex)) {
> - if (!mutex_is_locked_by(&dev->struct_mutex, current))
> - return false;
> -
> - *unlock = false;
> - } else {
> - *unlock = true;
> - }
> ++if (!mutex_trylock(&dev->struct_mutex))
> ++return false;
> +
> ++*unlock = true;
> +return true;
> +}
> +
> static bool any_vma_pinned(struct drm_i915_gem_object *obj)
> {
> struct i915_vma *vma;
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Bug 98638] Panic on shutdown with AMDGPU and Ubuntu Plymouth
https://bugs.freedesktop.org/show_bug.cgi?id=98638 Bug ID: 98638 Summary: Panic on shutdown with AMDGPU and Ubuntu Plymouth Product: DRI Version: DRI git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel at lists.freedesktop.org Reporter: ernstp at gmail.com Hi, I think this is another exciting shutdown panic. I happens with 4.9-rc4 and drm-next-4.10-wip from right now. It only happens when I have the Ubuntu bootsplash enabled, the screen stays frozen. I managed to capture a few lines with netconsole: [ 352.263493] systemd-shutdow: 31 output lines suppressed due to ratelimiting [ 352.533484] sd 4:0:0:0: [sdd] Synchronizing SCSI cache [ 352.733464] sd 0:0:0:0: [sda] Synchronizing SCSI cache [ 352.791916] [drm] amdgpu: finishing device. mac_hid psmouse sysfillrect sysimgblt fb_sys_fops e1000e drm fjes ptp wmi pps_core[ 353.908045] CPU: 1 PID: 2072 Comm: plymouthd Not tainted 4.9.0-rc4+ #63 [ 353.908045] Hardware name: System manufacturer System Product Name/P8P67 PRO REV 3.1, BIOS 1704 06/08/2011 [ 353.908046] a86f010f7538 [ 353.908047] 81615882 a86f010f7588 a86f010f7578 [ 353.908049] 812834eb 016f 97bc34cce400 97bc3001df68 [ 353.908051] 97bc2d670e00[ 353.908126] [] __die+0xa0/0xe0 [ 353.908295] [] ? drm_mode_getcrtc+0x140/0x140 [drm] 8b That's all I got. I have 2 4K monitors connected to a Fiji Fury card. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/9e61cd6c/attachment.html>
[linux-sunxi] [PATCH v5 4/7] ASoC: sunxi: Add sun8i I2S driver
On Mon, 7 Nov 2016 21:05:05 +0100 Maxime Ripard wrote: > Hi, > > On Sun, Nov 06, 2016 at 07:02:48PM +0100, Jean-Francois Moine wrote: > > On Sun, 23 Oct 2016 09:33:16 +0800 > > Chen-Yu Tsai wrote: > > > > > On Fri, Oct 21, 2016 at 4:36 PM, Jean-Francois Moine > > > wrote: > > > > This patch adds I2S support to sun8i SoCs as the A83T and H3. > > > > > > > > Signed-off-by: Jean-Francois Moine > > > > --- > > > > Note: This driver is closed to the sun4i-i2s except that: > > > > - it handles the H3 > > > > > > If it's close to sun4i-i2s, you should probably rework that one to support > > > the newer SoCs. > > > > I started to add the H3 into the sun4i-i2s, but I am blocked with > > regmap. > > Many H3 registers are common with the A10, but some of them have more > > or less fields, the fields may be at different offsets. And, finally, > > some registers are completely different. > > This would not raise any problem, except with regmap which is really > > painful. > > That's weird, because regmap's regmap_field should make that much > easier. #define field_relaxed(addr, mask, val) \ writel_relaxed((readl_relaxed(addr) & mask) | val, addr) > > As I may understood, regmap is used to simplify suspend/resume, but, is > > it useful to save the I2S register on suspend? > > Practically, I am streaming some tune on my device. I suspend it for > > any reason. The next morning, I resume it. Are you sure I want to > > continue to hear the end of the tune? > > > > I better think that streaming should be simply stopped on suspend. > > You're mistaken. The code in there is for *runtime* suspend, ie when > the device is no longer used, so that case shouldn't even happen at > all. > > (And real suspend isn't supported anyway) Is it time to remove this useless code? > > Then, there is no need to save the playing registers, and, here I am, > > there is no need to use regmap. > > > > May I go this way? > > No, please don't. regmap is also providing very useful features, such > as access to all the registers through debugfs, or tracing. What > exactly feels painful to you? When the I/O registers are in memory (that's the case), you may access them (read and write) thru /dev/mem. Also, is a register access trace really needed in this driver? The pain is to define the regmap_config (which registers can be read/write/volatile and which can be the values the u-boot let us in the registers at startup time), and the lot of code which is run instead of simple load/store machine instructions. -- Ken ar c'hentañ| ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/
[PATCH v7 1/3] drm/fence: add in-fences support
On Tue, Nov 08, 2016 at 03:54:48PM +0900, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> There is now a new property called IN_FENCE_FD attached to every plane
> state that receives sync_file fds from userspace via the atomic commit
> IOCTL.
>
> The fd is then translated to a fence (that may be a fence_array
> subclass or just a normal fence) and then used by DRM to fence_wait() for
> all fences in the sync_file to signal. So it only commits when all
> framebuffers are ready to scanout.
>
> v2: Comments by Daniel Vetter:
> - remove set state->fence = NULL in destroy phase
> - accept fence -1 as valid and just return 0
> - do not call fence_get() - sync_file_fences_get() already calls it
> - fence_put() if state->fence is already set, in case userspace
> set the property more than once.
>
> v3: WARN_ON if fence is set but state has no FB
>
> v4: Comment from Maarten Lankhorst
> - allow set fence with no related fb
>
> v5: rename FENCE_FD to IN_FENCE_FD
>
> v6: Comments by Daniel Vetter:
> - rename plane_state->in_fence back to "fence"
> - re-introduce WARN_ON if fence set but no fb
>
> - rebase after fence -> dma_fence rename
>
> Signed-off-by: Gustavo Padovan
> ---
> drivers/gpu/drm/Kconfig | 1 +
> drivers/gpu/drm/drm_atomic.c| 14 ++
> drivers/gpu/drm/drm_atomic_helper.c | 3 +++
> drivers/gpu/drm/drm_crtc.c | 6 ++
> drivers/gpu/drm/drm_plane.c | 1 +
> include/drm/drm_crtc.h | 5 +
> 6 files changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 483059a..43cb33d 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -12,6 +12,7 @@ menuconfig DRM
> select I2C
> select I2C_ALGOBIT
> select DMA_SHARED_BUFFER
> + select SYNC_FILE
> help
> Kernel-level support for the Direct Rendering Infrastructure (DRI)
> introduced in XFree86 4.0. If you say Y here, you need to select
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5e73954..d1ae463 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
>
> #include "drm_crtc_internal.h"
>
> @@ -686,6 +687,17 @@ int drm_atomic_plane_set_property(struct drm_plane
> *plane,
> drm_atomic_set_fb_for_plane(state, fb);
> if (fb)
> drm_framebuffer_unreference(fb);
> + } else if (property == config->prop_in_fence_fd) {
> + if (U642I64(val) == -1)
> + return 0;
> +
> + if (state->fence)
> + dma_fence_put(state->fence);
> +
> + state->fence = sync_file_get_fence(val);
> + if (!state->fence)
> + return -EINVAL;
Might have missed it, but your igt only seems to test for -1 (i.e. no
fence), not for an entirely invalid fence. I think we need another
testcase which opens some file (/dev/null maybe) and tries to use that as
fence.
Another corner case is error unrolling. I think we need 3 cases for full
coverage:
- Early error unrolling while setting properties. Best way imo would be 1)
set an in-fence (correctly), then 2) set an invalid property (just set
the CRTC plane property to something invalid like ~0ULL).
- Late unrolling (i.e. atomic_check fails). I think the reliable way here
to provoke this on all drivers would be: Atomic commit with in-fence and
otherwise everything the same (i.e. known to work), but changing the
crtc's ACTIVE property from false to true without setting ALLOW_MODESET.
- And then also a successful TEST_ONLY commit.
Same is required for out-fences ofc too.
Patch itself looks good, and I couldn't poke any holes into it.
Reviewed-by: Daniel Vetter
> +
> } else if (property == config->prop_crtc_id) {
> struct drm_crtc *crtc = drm_crtc_find(dev, val);
> return drm_atomic_set_crtc_for_plane(state, crtc);
> @@ -747,6 +759,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>
> if (property == config->prop_fb_id) {
> *val = (state->fb) ? state->fb->base.id : 0;
> + } else if (property == config->prop_in_fence_fd) {
> + *val = -1;
> } else if (property == config->prop_crtc_id) {
> *val = (state->crtc) ? state->crtc->base.id : 0;
> } else if (property == config->prop_crtc_x) {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 75ad01d..26a067f 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3114,6 +3114,9 @@ void __drm_atomic_helper_plane_destroy_state(struct
> drm_plane_state *state)
> {
> if (state->fb)
> drm_framebuffer_unreference(state->fb);
> +
> + if (state->fence)
> +
[PATCH libdrm] xf86drm: Parse the separate files to retrieve the vendor/device info
On 1 November 2016 at 18:47, Mauro Santos wrote:
> On 01-11-2016 18:13, Emil Velikov wrote:
>> From: Emil Velikov
>>
>> Parsing config sysfs file wakes up the device. The latter of which may
>> be slow and isn't required to begin with.
>>
>> Reading through config is/was required since the revision is not
>> available by other means, although with a kernel patch in the way we can
>> 'cheat' temporarily.
>>
>> That should be fine, since no open-source project has ever used the
>> value.
>>
>> Cc: Michel Dänzer
>> Cc: Mauro Santos
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502
>> Signed-off-by: Emil Velikov
>> ---
>> Mauro can you apply this against libdrm and rebuild it. You do _not_
>> need to rebuild mesa afterwords.
>>
>> Thanks
>> ---
>> xf86drm.c | 50 +++---
>> 1 file changed, 35 insertions(+), 15 deletions(-)
>>
>> diff --git a/xf86drm.c b/xf86drm.c
>> index 52add5e..5a5100c 100644
>> --- a/xf86drm.c
>> +++ b/xf86drm.c
>> @@ -2950,25 +2950,45 @@ static int drmParsePciDeviceInfo(const char *d_name,
>> drmPciDeviceInfoPtr device)
>> {
>> #ifdef __linux__
>> +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
>> +static const char *attrs[] = {
>> + "revision", /* XXX: make sure it's always first, see note below */
>> + "vendor",
>> + "device",
>> + "subsystem_vendor",
>> + "subsystem_device",
>> +};
>> char path[PATH_MAX + 1];
>> -unsigned char config[64];
>> -int fd, ret;
>> +unsigned int data[ARRAY_SIZE(attrs)];
>> +FILE *fp;
>> +int ret;
>>
>> -snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/config", d_name);
>> -fd = open(path, O_RDONLY);
>> -if (fd < 0)
>> -return -errno;
>> +for (unsigned i = 0; i < ARRAY_SIZE(attrs); i++) {
>> +snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/%s",
>> + d_name, attrs[i]);
>> +fp = fopen(path, "r");
>> +if (!fp) {
>> +/* Note: First we check the revision, since older kernels
>> + * may not have it. Default to zero in such cases. */
>> +if (i == 0) {
>> +data[i] = 0;
>> +continue;
>> +}
>> +return -errno;
>> +}
>>
>> -ret = read(fd, config, sizeof(config));
>> -close(fd);
>> -if (ret < 0)
>> -return -errno;
>> +ret = fscanf(fp, "%x", &data[i]);
>> +fclose(fp);
>> +if (ret != 1)
>> +return -errno;
>> +
>> +}
>>
>> -device->vendor_id = config[0] | (config[1] << 8);
>> -device->device_id = config[2] | (config[3] << 8);
>> -device->revision_id = config[8];
>> -device->subvendor_id = config[44] | (config[45] << 8);
>> -device->subdevice_id = config[46] | (config[47] << 8);
>> +device->revision_id = data[0] & 0xff;
>> +device->vendor_id = data[1] & 0x;
>> +device->device_id = data[2] & 0x;
>> +device->subvendor_id = data[3] & 0x;
>> +device->subdevice_id = data[4] & 0x;
>>
>> return 0;
>> #else
>>
>
> I have applied this against libdrm 2.4.71 and I don't see any delays
> when starting firefox/chromium/thunderbird/glxgears.
>
> There is also no indication in dmesg that the dGPU is being
> reinitialized when starting the programs where I've detected the problem.
>
Thanks Mauro. Can you give this a try alongside the kernel fix [1] ?
I'd love to get the latter merged soon(ish).
Independent of the kernel side, I might need to go another way for
libdrm/mesa so I'll CC you on future patches.
Your help is greatly appreciated !
Thanks
Emil
[1] http://patchwork.ozlabs.org/patch/689975/
[PATCH] PCI: create revision file in sysfs
[Dropping Jammy since his email bounces] On 1 November 2016 at 15:47, Alex Deucher wrote: > On Tue, Nov 1, 2016 at 11:42 AM, Emil Velikov > wrote: >> From: Emil Velikov >> >> Currently the revision isn't available via sysfs/libudev thus if one >> wants to know the value they need to read through the config file. >> >> This in itself wakes/powers up the device, causing unwanted delay >> since it can be quite costly. >> >> Expose the revision as a separate file, just like we do for the device, >> vendor, their subsystem version and class. >> >> Cc: Jammy Zhou >> Cc: Michel Dänzer >> Cc: Bjorn Helgaas >> Cc: linux-pci at vger.kernel.org >> Cc: linux-kernel at vger.kernel.org >> Signed-off-by: Emil Velikov > > Reviewed-by: Alex Deucher > Thanks Alex. Gents, to elaborate a bit: When using the Mesa drivers alongside firefox [1] (since Mesa 13.0), glxinfo (Mesa 10.0) and others, all the GPUs* will be awaken, causing unwanted delays and increased power usage. >From the userspace POV we have two distinct users who require the revision file - libdrm and libpciaccess. * The latter would even wake up _all_ the devices located on a PCI bus ! Let me know if you'd like the above in the patch summary, meanwhile I'll poke and collect a few more ack/r-b/t-b. Thanks Emil [1] https://bugs.freedesktop.org/show_bug.cgi?id=98502
[PATCH] drm: tda998x: mali-dp: hdlcd: refactor connector registration
On Tue, Nov 08, 2016 at 10:59:43AM +, Russell King - ARM Linux wrote: > On Tue, Nov 08, 2016 at 10:25:52AM +0100, Daniel Vetter wrote: > > Hm, I entirely missed that part of the troubles. Anyway, if you all agree > > on a patch I certainly won't block it, feel free to merge through suitable > > trees (or I can smash it into drm-misc if that's wanted). > > I think those who are interested in seeing the drm_connector_register() > call disappear from tda998x only care about that happening, but not how > it happens. > > We have agreement between myself, Brian and Liviu on this approach, and > I think everyone else is waiting for me to push out the commit so it can > be used as the basis for their work. I think everyone else is waiting > for me to push something out which gets us past this log-jam. > > I don't understand the connectivity between drm-misc and David's drm > tree - so I'm going to let you make the decision on whether to merge > this into drm-misc. I normally send my pull requests for Armada and > TDA998x changes to David, which means when I send my other TDA998x > changes, the mali/tda998x commit will be included in that pull > request too. So I'm wondering whether it would make more sense for > me to send it to David instead, or whether I need to send my other > changes through drm-misc instead. I find the whole drm vs drm-misc > thing rather confusing. > > I think we should get this accepted into drm trees before anyone bases > their work on this commit (which is why I've been holding off during > the last week, waiting for DRM folk to get back from Santa Fe and > readjust to the higher atmospheric pressure!) > > Anyway, here is my pull request for the mali/hdlcd/tda998x commit which > I'd normally send to David - I don't mind which tree it goes into as > long as things work out nicely. drm-misc is just the collector for when it doesn't make sense to have a driver or topic/feature pull request (or there isn't really a permanent driver tree). Pull to Dave directly makes sense. -Daniel > 8<=== > > David, > > Please incorporate the latest TDA998x I2C driver (drm-tda998x-mali > branch), which can be found at: > > git://git.armlinux.org.uk/~rmk/linux-arm.git drm-tda998x-mali > > with SHA1 90731c24d2db7ec04df43ddbcee9605183d05187. > > This change removes the call to drm_connector_register() which has been > blocking the proper de-midlayer conversion of other DRM drivers. > Unfortunately, hdlcd and mali have intimate dependencies on this change, > which is why these drivers need to be fixed up in the same commit - they > can't be separate commits without these drivers breaking. All other > DRM drivers which make use of tda998x (to my knowledge - armada, tilcdc) > cope with this change. > > This will update the following files: > > drivers/gpu/drm/arm/hdlcd_drv.c | 19 +++ > drivers/gpu/drm/arm/malidp_drv.c | 18 +++--- > drivers/gpu/drm/i2c/tda998x_drv.c | 8 > 3 files changed, 22 insertions(+), 23 deletions(-) > > through these changes: > > Brian Starkey (1): > drm/i2c: tda998x: mali-dp: hdlcd: refactor connector registration > > Many thanks. > > -- > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net. -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm: panel: simple-panel: get the enable gpio as-is
On Mon, Nov 07, 2016 at 03:26:56PM +0100, Philipp Zabel wrote: > Am Montag, den 07.11.2016, 14:17 +0100 schrieb Thierry Reding: > > On Mon, Nov 07, 2016 at 06:12:43PM +0800, Chen-Yu Tsai wrote: > > > On Sun, Nov 6, 2016 at 7:09 PM, Icenowy Zheng wrote: > > > > The enable gpio of simple-panel may be used by a simplefb or other > > > > driver on the panel's display before the KMS driver get load. > > > > > > > > Get the GPIO as-is, so the panel won't be disabled, and the simplefb > > > > can work. > > > > > > > > Signed-off-by: Icenowy Zheng > > > > --- > > > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > > > b/drivers/gpu/drm/panel/panel-simple.c > > > > index 113db3c..ccee4c1 100644 > > > > --- a/drivers/gpu/drm/panel/panel-simple.c > > > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > > > @@ -312,7 +312,7 @@ static int panel_simple_probe(struct device *dev, > > > > const struct panel_desc *desc) > > > > return PTR_ERR(panel->supply); > > > > > > > > panel->enable_gpio = devm_gpiod_get_optional(dev, "enable", > > > > -GPIOD_OUT_LOW); > > > > +GPIOD_ASIS); > > > > > > The GPIO requested as-is might be in input mode. You should change the > > > gpiod_set_value calls to gpiod_direction_output calls. The later also > > > allows you to give an initial value. Not sure if it checks for cansleep > > > like the set_value calls though. > > > > I'd prefer not to add gpiod_direction_output() calls outside of > > ->probe(). Instead, could we make this patch be smart about taking over > > from an earlier user? Could it read the current direction and value of > > the GPIO and not disable it if it had previously been enabled? > > Seconded, the PWM backlight driver in drivers/video/backlight/pwm_bl.c > already does a similar thing. > > > And even if we go this extra mile there's a possibility that the GPIO > > was just left dangling by earlier software (or hardware) and leaving it > > on would actually be worse than turning the panel off. > > Is this something the encoder driver should communicate to the panel? > That one will know whether its atomic_reset state is enabled or > disabled. At that point it's already too late. Configuration of the GPIO here is within the panel's ->probe() implementation, which is before it can ever even get attached to an encoder/connector. I can't think of a proper solution for this, but perhaps the best heuristic would be to request with GPIOD_ASIS and then query the direction. If it's configured as output we could assume that somebody's configured it explicitly and it has the correct value. That could still break existing use-cases, but it would at least allow basic hand-over. Also, if leaving the GPIO configured as-is causes glitches or other issues, then these are observable with the current code as well, until the panel driver takes over and disables everything. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/e0f3dae7/attachment.sig>
[Bug 98523] Can't run without "nomodeset" on MacPro6,1 with two AMD R9 280X Tahiti video cards
https://bugs.freedesktop.org/show_bug.cgi?id=98523 --- Comment #1 from Heigh --- Hi Michel, Thank you for looking into this, and for assigning the bug. In the meantime, I have found a workaround or a solution. The system needs to be booted with systemd-boot bootloader (or perhaps any other supporting UEFI boot) When I boot Ubuntu with standard, installed during installation grub2, radeon driver would't detect external displays connected via Display Ports. But when I boot system using systemd-boot installed by Arch Linux (or KaOS), it detects displays normally, and works correctly, supporting all external displays as expected. I think this is related to some aspects of the hardware not exposed in BIOS boot mode, and exposed in EFI/UEFI boot mode. Preferred solution would be working radeon driver in BIOS mode if possible, or, if that is not possible, some meaningful message in the system journal explaining that certain features (e.g. external monitors via Display Port) will not be supported unless the system is booted in EFI/UEFI mode. Thank you, Heigh -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/47509cee/attachment.html>
[PATCH v7 0/3] drm: add explict fencing
On Tue, Nov 08, 2016 at 10:35:08AM +, Chris Wilson wrote: > On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > Hi, > > > > This is yet another version of the DRM fences patches. Please refer > > to the cover letter[1] in a previous version to check for more details. > > Explicit fencing is not a superset of the implicit fences. The driver > may be using implicit fences (on a reservation object) to serialise > asynchronous operations wrt to each other (such as dispatching threads > to flush cpu caches to memory, manipulating page tables and the like > before the flip). Since the user doesn't know about these operations, > they are not included in the explicit fence they provide, at which point > we can't trust their fence to the exclusion of the implicit fences... My thoughts are that in atomic_check drivers just fill in the fence from the reservation_object (i.e. the uapi implicit fencing part). If there's any additional work that's queued up in ->prepare_fb then I guess the driver needs to track that internally, but _only_ for kernel-internally queued work. The reason for that is that with explicit fencing we want to allow userspace to overwrite any existing implicit fences that might hang around. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[drm-intel:topic/drm-misc 1/13] ERROR: "depot_save_stack" [drivers/gpu/drm/drm.ko] undefined!
tree: git://anongit.freedesktop.org/drm-intel topic/drm-misc head: 3835b46e5535d9ad534776bc93670db097682556 commit: 5705670d0463423337c82d00877989e7df8b169d [1/13] drm: Track drm_mm allocators and show leaks on shutdown config: i386-randconfig-i1-201645 (attached as .config) compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4 reproduce: git checkout 5705670d0463423337c82d00877989e7df8b169d # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): >> ERROR: "depot_save_stack" [drivers/gpu/drm/drm.ko] undefined! >> ERROR: "depot_fetch_stack" [drivers/gpu/drm/drm.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation -- next part -- A non-text attachment was scrubbed... Name: .config.gz Type: application/gzip Size: 33705 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/953192f4/attachment-0001.gz>
[PATCH 2/2] drm/edid: Consider alternate cea timings to be the same VIC
On 03.11.2016 13:53, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä
>
> CEA-861 specifies that the vertical front porch may vary by one or two
> lines for specific VICs. Up to now we've only considered a mode to match
> the VIC if it matched the shortest possible vertical front porch length
> (as that is the variant we store in cea_modes[]). Let's allow our VIC
> matching to work with the other timings variants as well so that that
> we'll send out the correct VIC if the variant actually used isn't the
> one with the shortest vertical front porch.
>
> Cc: Shashank Sharma
> Cc: Andrzej Hajda
> Cc: Adam Jackson
> Signed-off-by: Ville Syrjälä
I have checked against specification and it looks OK.
I have few nitpicks below regarding implementation, but this could be
matter of taste, so:
Reviewed-by: Andrzej Hajda
> ---
> drivers/gpu/drm/drm_edid.c | 66
> +-
> 1 file changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 7eec18925b70..728990fee4ef 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2613,6 +2613,41 @@ cea_mode_alternate_clock(const struct drm_display_mode
> *cea_mode)
> return clock;
> }
>
> +static bool
> +cea_mode_alternate_timings(u8 vic, struct drm_display_mode *mode)
> +{
> + /*
> + * For certain VICs the spec allows the vertical
> + * front porch to vary by one or two lines.
> + *
> + * cea_modes[] stores the variant with the shortest
> + * vertical front porch. We can adjust the mode to
> + * get the other variants by simply increasing the
> + * vertical front porch length.
> + */
> + BUILD_BUG_ON(edid_cea_modes[8].vtotal != 262 ||
> + edid_cea_modes[9].vtotal != 262 ||
> + edid_cea_modes[12].vtotal != 262 ||
> + edid_cea_modes[13].vtotal != 262 ||
> + edid_cea_modes[23].vtotal != 312 ||
> + edid_cea_modes[24].vtotal != 312 ||
> + edid_cea_modes[27].vtotal != 312 ||
> + edid_cea_modes[28].vtotal != 312);
I am not sure if this compile check is really necessary,
I would rather put comment before edid_cea_modes array
which mode should be put into array in multi-mode case.
> +
> + if (((vic == 8 || vic == 9 ||
> + vic == 12 || vic == 13) && mode->vtotal < 263) ||
> + ((vic == 23 || vic == 24 ||
> + vic == 27 || vic == 28) && mode->vtotal < 314)) {
I wonder if it wouldn't be better to mark somehow these modes
in the array as alternating ones. This way all info about cea modes
will be in one place. For example by (ab)using private_flags:
/* 8 - 720(1440)x240 at 60Hz */
{ DRM_MODE("720x240", DRM_MODE_TYPE_DRIVER, 13500, 720, 739,
801, 858, 0, 240, 244, 247, 262, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC |
DRM_MODE_FLAG_DBLCLK),
.vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
.private_flags = CEA_MODE_FLAG_VFP2},
This is of course just an idea, I am not sure if not overkill :)
> + mode->vsync_start++;
> + mode->vsync_end++;
> + mode->vtotal++;
> +
> + return true;
> + }
> +
> + return false;
> +}
> +
> static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode
> *to_match,
>unsigned int clock_tolerance)
> {
> @@ -2622,19 +2657,21 @@ static u8 drm_match_cea_mode_clock_tolerance(const
> struct drm_display_mode *to_m
> return 0;
>
> for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> - const struct drm_display_mode *cea_mode = &edid_cea_modes[vic];
> + struct drm_display_mode cea_mode = edid_cea_modes[vic];
> unsigned int clock1, clock2;
>
> /* Check both 60Hz and 59.94Hz */
> - clock1 = cea_mode->clock;
> - clock2 = cea_mode_alternate_clock(cea_mode);
> + clock1 = cea_mode.clock;
> + clock2 = cea_mode_alternate_clock(&cea_mode);
>
> if (abs(to_match->clock - clock1) > clock_tolerance &&
> abs(to_match->clock - clock2) > clock_tolerance)
> continue;
>
> - if (drm_mode_equal_no_clocks(to_match, cea_mode))
> - return vic;
> + do {
> + if (drm_mode_equal_no_clocks_no_stereo(to_match,
> &cea_mode))
> + return vic;
> + } while (cea_mode_alternate_timings(vic, &cea_mode));
> }
>
> return 0;
> @@ -2655,18 +2692,23 @@ u8 drm_match_cea_mode(const struct drm_display_mode
> *to_match)
> return 0;
>
> for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> - const struct drm_display_mode *cea_mode
[PATCH v7 0/3] drm: add explict fencing
On Tue, Nov 08, 2016 at 12:32:56PM +0100, Daniel Vetter wrote: > On Tue, Nov 08, 2016 at 10:35:08AM +, Chris Wilson wrote: > > On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: > > > From: Gustavo Padovan > > > > > > Hi, > > > > > > This is yet another version of the DRM fences patches. Please refer > > > to the cover letter[1] in a previous version to check for more details. > > > > Explicit fencing is not a superset of the implicit fences. The driver > > may be using implicit fences (on a reservation object) to serialise > > asynchronous operations wrt to each other (such as dispatching threads > > to flush cpu caches to memory, manipulating page tables and the like > > before the flip). Since the user doesn't know about these operations, > > they are not included in the explicit fence they provide, at which point > > we can't trust their fence to the exclusion of the implicit fences... > > My thoughts are that in atomic_check drivers just fill in the fence from > the reservation_object (i.e. the uapi implicit fencing part). If there's > any additional work that's queued up in ->prepare_fb then I guess the > driver needs to track that internally, but _only_ for kernel-internally > queued work. That's not a trivial task to work out which of the fence contexts within the reservation object are required and which are to be replaced by the explicit fence, esp. when you have to consider external fences. > The reason for that is that with explicit fencing we want to allow > userspace to overwrite any existing implicit fences that might hang > around. I'm just suggesting the danger of that when userspace doesn't know everything and the current interfaces do not allow for userspace to know, we only tell userspace about its own action (more or less). -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH v9 09/11] drm/i915: add dev.i915.oa_max_sample_rate sysctl
DRM_ERROR("Minimum OA sampling exponent is
> 6 without root privileges\n");
> > + if (oa_period <= NSEC_PER_SEC) {
> > + u64 tmp = NSEC_PER_SEC;
> > + do_div(tmp, oa_period);
> > + oa_freq_hz = tmp;
> > + } else
> > + oa_freq_hz = 0;
> > +
> > + if (oa_freq_hz > i915_oa_max_sample_rate &&
> > + !capable(CAP_SYS_ADMIN)) {
> > + DRM_ERROR("OA exponent would exceed the
> max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without
> root privileges\n",
> > + i915_oa_max_sample_rate);
> > return -EACCES;
> > }
> >
> > @@ -1481,6 +1511,15 @@ static struct ctl_table oa_table[] = {
> >.extra1 = &zero,
> >.extra2 = &one,
> >},
> > + {
> > + .procname = "oa_max_sample_rate",
> > + .data = &i915_oa_max_sample_rate,
> > + .maxlen = sizeof(i915_oa_max_sample_rate),
> > + .mode = 0644,
> > + .proc_handler = proc_dointvec_minmax,
> > + .extra1 = &zero,
> > + .extra2 = &oa_sample_rate_hard_limit,
> > + },
> > {}
> > };
> >
>
>
>
-- next part --
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/367f6684/attachment-0001.html>
[PATCH] drm: Add stackdepot include for DRM_DEBUG_MM
0day found that stackdepot.h doesn't get automatically included on all
architectures, so remember to add our #include.
Reported-by: kbuild test robot
Fixes: 5705670d0463 ("drm: Track drm_mm allocators and show leaks on shutdown")
Signed-off-by: Chris Wilson
Cc: Daniel Vetter
---
drivers/gpu/drm/drm_mm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 89b891a4847f..632473beb40c 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -105,6 +105,8 @@ static struct drm_mm_node
*drm_mm_search_free_in_range_generic(const struct drm_
enum drm_mm_search_flags flags);
#ifdef CONFIG_DRM_DEBUG_MM
+#include
+
#define STACKDEPTH 32
#define BUFSZ 4096
--
2.10.2
[GIT PULL][drm-next] drm/mali-dp fixes and updates
Hi Dave, Here is the list of fixes that I have for drm/mali-dp. They've been on the mailing lists for a while and merged into linux-next for a few weeks, but due to holiday and travel to Linux Plumbers I did not send the pull request earlier. I don't know if these patches can be pulled into v4.9 still (they will conflict with Ville Syrjälä's cleanup of DRM_ROTATE series that is already in drm-next), but if you do that would be great. The following changes since commit fb422950c6cd726fd36eb72a7cf84583440a18a2: Merge branch 'linux-4.9' of git://github.com/skeggsb/linux into drm-next (2016-10-28 14:24:56 +1000) are available in the git repository at: git://linux-arm.org/linux-ld.git for-upstream/mali-dp for you to fetch changes up to e64053f05eb924db45f90a1556a200d1acb4b01e: drm: mali-dp: Clear CVAL when leaving config mode (2016-11-08 11:40:02 +) Baoyou Xie (1): drm/arm: mark symbols static where possible Brian Starkey (8): drm: mali-dp: Add pitch alignment check function drm: mali-dp: Add pitch alignment check for planes arm: mali-dp: Extract mode_config cleanup into malidp_fini drm: mali-dp: Refactor plane initialisation drm: mali-dp: Enable alpha blending drm: mali-dp: Store internal format and n_planes in plane state drm: mali-dp: Don't set DRM_PLANE_COMMIT_ACTIVE_ONLY drm: mali-dp: Clear CVAL when leaving config mode Liviu Dudau (3): drm: mali-dp: Clear the config_valid flag before using it in wait_event. drm: mali-dp: Set the drm->irq_enabled flag to match driver's state. drm: mali-dp: Add support for setting plane's rotation property from userspace. drivers/gpu/drm/arm/malidp_drv.c| 19 +--- drivers/gpu/drm/arm/malidp_drv.h| 3 ++ drivers/gpu/drm/arm/malidp_hw.c | 5 ++ drivers/gpu/drm/arm/malidp_hw.h | 9 drivers/gpu/drm/arm/malidp_planes.c | 96 - 5 files changed, 92 insertions(+), 40 deletions(-) Many thanks, Liviu -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ã)_/¯
[PATCH v7 0/3] drm: add explict fencing
On Tue, Nov 08, 2016 at 11:45:51AM +, Chris Wilson wrote: > On Tue, Nov 08, 2016 at 12:32:56PM +0100, Daniel Vetter wrote: > > On Tue, Nov 08, 2016 at 10:35:08AM +, Chris Wilson wrote: > > > On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: > > > > From: Gustavo Padovan > > > > > > > > Hi, > > > > > > > > This is yet another version of the DRM fences patches. Please refer > > > > to the cover letter[1] in a previous version to check for more details. > > > > > > Explicit fencing is not a superset of the implicit fences. The driver > > > may be using implicit fences (on a reservation object) to serialise > > > asynchronous operations wrt to each other (such as dispatching threads > > > to flush cpu caches to memory, manipulating page tables and the like > > > before the flip). Since the user doesn't know about these operations, > > > they are not included in the explicit fence they provide, at which point > > > we can't trust their fence to the exclusion of the implicit fences... > > > > My thoughts are that in atomic_check drivers just fill in the fence from > > the reservation_object (i.e. the uapi implicit fencing part). If there's > > any additional work that's queued up in ->prepare_fb then I guess the > > driver needs to track that internally, but _only_ for kernel-internally > > queued work. > > That's not a trivial task to work out which of the fence contexts within > the reservation object are required and which are to be replaced by the > explicit fence, esp. when you have to consider external fences. Hm, what kind of async kernel tasks are you thinking off? Atm I don't know of anyone who does e.g. clflush through the gpu. And ttm bo placement moves for display should be explicit enough that drivers will deal with them correctly. At least that seems to have been the conclusion from the long amdgpu thread. > > The reason for that is that with explicit fencing we want to allow > > userspace to overwrite any existing implicit fences that might hang > > around. > > I'm just suggesting the danger of that when userspace doesn't know > everything and the current interfaces do not allow for userspace to know, > we only tell userspace about its own action (more or less). tools for fools, but yes userspace is expected to get this 100% right (for any userspace-issued cs at least), and eat the fallout if it doesn't. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH v7 0/3] drm: add explict fencing
Am 08.11.2016 um 12:45 schrieb Chris Wilson: > On Tue, Nov 08, 2016 at 12:32:56PM +0100, Daniel Vetter wrote: >> On Tue, Nov 08, 2016 at 10:35:08AM +, Chris Wilson wrote: >>> On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: From: Gustavo Padovan Hi, This is yet another version of the DRM fences patches. Please refer to the cover letter[1] in a previous version to check for more details. >>> Explicit fencing is not a superset of the implicit fences. The driver >>> may be using implicit fences (on a reservation object) to serialise >>> asynchronous operations wrt to each other (such as dispatching threads >>> to flush cpu caches to memory, manipulating page tables and the like >>> before the flip). Since the user doesn't know about these operations, >>> they are not included in the explicit fence they provide, at which point >>> we can't trust their fence to the exclusion of the implicit fences... >> My thoughts are that in atomic_check drivers just fill in the fence from >> the reservation_object (i.e. the uapi implicit fencing part). If there's >> any additional work that's queued up in ->prepare_fb then I guess the >> driver needs to track that internally, but _only_ for kernel-internally >> queued work. > That's not a trivial task to work out which of the fence contexts within > the reservation object are required and which are to be replaced by the > explicit fence, esp. when you have to consider external fences. > >> The reason for that is that with explicit fencing we want to allow >> userspace to overwrite any existing implicit fences that might hang >> around. > I'm just suggesting the danger of that when userspace doesn't know > everything and the current interfaces do not allow for userspace to know, > we only tell userspace about its own action (more or less). It's even worse than that. See the kernel can for example swap out objects any time it wants. Userspace doesn't know about such operations and so can't provide them as explicit fence. Same is true for example in situations where one userspace process doesn't know about operations another process does. E.g. for backward compatibility with DRI2/3 for example. So we will always have a mixture of implicit fences and explicit fences. The approach we used for amdgpu is that we implicit wait for all fences which the initiator of an operation can't know about (e.g. from another process or kernel internally) and explicitly wait for all additional fences provided by the initiator or an operation. Regards, Christian. > -Chris >
[PATCH] drm: Add stackdepot include for DRM_DEBUG_MM
Am 08.11.2016 um 12:56 schrieb Chris Wilson:
> 0day found that stackdepot.h doesn't get automatically included on all
> architectures, so remember to add our #include.
>
> Reported-by: kbuild test robot
> Fixes: 5705670d0463 ("drm: Track drm_mm allocators and show leaks on
> shutdown")
> Signed-off-by: Chris Wilson
> Cc: Daniel Vetter
Reviewed-by: Christian König .
> ---
> drivers/gpu/drm/drm_mm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 89b891a4847f..632473beb40c 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -105,6 +105,8 @@ static struct drm_mm_node
> *drm_mm_search_free_in_range_generic(const struct drm_
> enum drm_mm_search_flags flags);
>
> #ifdef CONFIG_DRM_DEBUG_MM
> +#include
> +
> #define STACKDEPTH 32
> #define BUFSZ 4096
>
[PATCH] drm: Add stackdepot include for DRM_DEBUG_MM
On Tue, Nov 08, 2016 at 11:56:01AM +, Chris Wilson wrote:
> 0day found that stackdepot.h doesn't get automatically included on all
> architectures, so remember to add our #include.
>
> Reported-by: kbuild test robot
> Fixes: 5705670d0463 ("drm: Track drm_mm allocators and show leaks on
> shutdown")
> Signed-off-by: Chris Wilson
> Cc: Daniel Vetter
Thx for the fast fixup, applied to drm-misc.
-Daniel
> ---
> drivers/gpu/drm/drm_mm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 89b891a4847f..632473beb40c 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -105,6 +105,8 @@ static struct drm_mm_node
> *drm_mm_search_free_in_range_generic(const struct drm_
> enum drm_mm_search_flags flags);
>
> #ifdef CONFIG_DRM_DEBUG_MM
> +#include
> +
> #define STACKDEPTH 32
> #define BUFSZ 4096
>
> --
> 2.10.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[GIT PULL][drm-next] drm/mali-dp fixes and updates
On Tue, Nov 08, 2016 at 12:26:19PM +, Liviu Dudau wrote: > Hi Dave, > > Here is the list of fixes that I have for drm/mali-dp. They've been on the > mailing > lists for a while and merged into linux-next for a few weeks, but due to > holiday and > travel to Linux Plumbers I did not send the pull request earlier. I don't > know if > these patches can be pulled into v4.9 still (they will conflict with Ville > Syrjälä's > cleanup of DRM_ROTATE series that is already in drm-next), but if you do that > would > be great. > > The following changes since commit fb422950c6cd726fd36eb72a7cf84583440a18a2: > > Merge branch 'linux-4.9' of git://github.com/skeggsb/linux into drm-next > (2016-10-28 14:24:56 +1000) > > are available in the git repository at: > > git://linux-arm.org/linux-ld.git for-upstream/mali-dp > > for you to fetch changes up to e64053f05eb924db45f90a1556a200d1acb4b01e: > > drm: mali-dp: Clear CVAL when leaving config mode (2016-11-08 11:40:02 > +) > > > Baoyou Xie (1): > drm/arm: mark symbols static where possible > > Brian Starkey (8): > drm: mali-dp: Add pitch alignment check function > drm: mali-dp: Add pitch alignment check for planes > arm: mali-dp: Extract mode_config cleanup into malidp_fini > drm: mali-dp: Refactor plane initialisation > drm: mali-dp: Enable alpha blending > drm: mali-dp: Store internal format and n_planes in plane state > drm: mali-dp: Don't set DRM_PLANE_COMMIT_ACTIVE_ONLY > drm: mali-dp: Clear CVAL when leaving config mode > > Liviu Dudau (3): > drm: mali-dp: Clear the config_valid flag before using it in wait_event. > drm: mali-dp: Set the drm->irq_enabled flag to match driver's state. > drm: mali-dp: Add support for setting plane's rotation property from > userspace. Quick check says these patches haven't shown on up a mailing list. Please submit first, do review in public and then send the pull request. There's generally a lot of cross-driver learning possible with this kind of stuff. Imo splitting drivers into their own mailing list and community only makes sense once there's just too much traffic, like with the big ones (intel, amd and nouveau because it's developed in userspace entirely). Thanks, Daniel > > drivers/gpu/drm/arm/malidp_drv.c| 19 +--- > drivers/gpu/drm/arm/malidp_drv.h| 3 ++ > drivers/gpu/drm/arm/malidp_hw.c | 5 ++ > drivers/gpu/drm/arm/malidp_hw.h | 9 > drivers/gpu/drm/arm/malidp_planes.c | 96 > - > 5 files changed, 92 insertions(+), 40 deletions(-) > > > Many thanks, > Liviu > > -- > > | I would like to | > | fix the world, | > | but they're not | > | giving me the | > \ source code! / > --- > ¯\_(ã)_/¯ > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH v2] drm/i915: don't whitelist oacontrol in cmd parser
This v2 patch bumps the command parser version so it can be referenced in
corresponding i-g-t gem_exec_parse changes.
--- >8 ---
Being able to program OACONTROL from a non-privileged batch buffer is
not sufficient to be able to configure the OA unit. This was originally
allowed to help enable Mesa to expose OA counters via the
INTEL_performance_query extension, but the current implementation based
on programming OACONTROL via a batch buffer isn't able to report useable
data without a more complete OA unit configuration. Mesa handles the
possibility that writes to OACONTROL may not be allowed and so only
advertises the extension after explicitly testing that a write to
OACONTROL succeeds. Based on this; removing OACONTROL from the whitelist
should be ok for userspace.
Removing this simplifies adding a new kernel api for configuring the OA
unit without needing to consider the possibility that userspace might
trample on OACONTROL state which we'd like to start managing within
the kernel instead. In particular running any Mesa based GL application
currently results in clearing OACONTROL when initializing which would
disable the capturing of metrics.
v2:
This bumps the command parser version from 8 to 9, as the change is
visible to userspace.
Signed-off-by: Robert Bragg
Reviewed-by: Matthew Auld
Reviewed-by: Sourab Gupta
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 42 --
1 file changed, 5 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index c9d2ecd..f5762cd 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -450,7 +450,6 @@ static const struct drm_i915_reg_descriptor
gen7_render_regs[] = {
REG64(PS_INVOCATION_COUNT),
REG64(PS_DEPTH_COUNT),
REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
- REG32(GEN7_OACONTROL), /* Only allowed for LRI and SRM. See below. */
REG64(MI_PREDICATE_SRC0),
REG64(MI_PREDICATE_SRC1),
REG32(GEN7_3DPRIM_END_OFFSET),
@@ -1060,8 +1059,7 @@ bool intel_engine_needs_cmd_parser(struct intel_engine_cs
*engine)
static bool check_cmd(const struct intel_engine_cs *engine,
const struct drm_i915_cmd_descriptor *desc,
const u32 *cmd, u32 length,
- const bool is_master,
- bool *oacontrol_set)
+ const bool is_master)
{
if (desc->flags & CMD_DESC_SKIP)
return true;
@@ -1099,31 +1097,6 @@ static bool check_cmd(const struct intel_engine_cs
*engine,
}
/*
-* OACONTROL requires some special handling for
-* writes. We want to make sure that any batch which
-* enables OA also disables it before the end of the
-* batch. The goal is to prevent one process from
-* snooping on the perf data from another process. To do
-* that, we need to check the value that will be written
-* to the register. Hence, limit OACONTROL writes to
-* only MI_LOAD_REGISTER_IMM commands.
-*/
- if (reg_addr == i915_mmio_reg_offset(GEN7_OACONTROL)) {
- if (desc->cmd.value == MI_LOAD_REGISTER_MEM) {
- DRM_DEBUG_DRIVER("CMD: Rejected LRM to
OACONTROL\n");
- return false;
- }
-
- if (desc->cmd.value == MI_LOAD_REGISTER_REG) {
- DRM_DEBUG_DRIVER("CMD: Rejected LRR to
OACONTROL\n");
- return false;
- }
-
- if (desc->cmd.value == MI_LOAD_REGISTER_IMM(1))
- *oacontrol_set = (cmd[offset + 1] != 0);
- }
-
- /*
* Check the value written to the register against the
* allowed mask/value pair given in the whitelist entry.
*/
@@ -1214,7 +1187,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs
*engine,
u32 *cmd, *batch_end;
struct drm_i915_cmd_descriptor default_desc = noop_desc;
const struct drm_i915_cmd_descriptor *desc = &default_desc;
- bool oacontrol_set = false; /* OACONTROL tracking. See check_cmd() */
bool needs_clflush_after = false;
int ret = 0;
@@ -1270,8 +1242,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs
*engine,
break;
}
- if (!check_cmd(engine, desc, cmd, length, is_master,
- &oacontrol_set)) {
+
[PATCH v7 0/3] drm: add explict fencing
On Tue, Nov 08, 2016 at 01:44:34PM +0100, Christian König wrote: > Am 08.11.2016 um 12:45 schrieb Chris Wilson: > > On Tue, Nov 08, 2016 at 12:32:56PM +0100, Daniel Vetter wrote: > > > On Tue, Nov 08, 2016 at 10:35:08AM +, Chris Wilson wrote: > > > > On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: > > > > > From: Gustavo Padovan > > > > > > > > > > Hi, > > > > > > > > > > This is yet another version of the DRM fences patches. Please refer > > > > > to the cover letter[1] in a previous version to check for more > > > > > details. > > > > Explicit fencing is not a superset of the implicit fences. The driver > > > > may be using implicit fences (on a reservation object) to serialise > > > > asynchronous operations wrt to each other (such as dispatching threads > > > > to flush cpu caches to memory, manipulating page tables and the like > > > > before the flip). Since the user doesn't know about these operations, > > > > they are not included in the explicit fence they provide, at which point > > > > we can't trust their fence to the exclusion of the implicit fences... > > > My thoughts are that in atomic_check drivers just fill in the fence from > > > the reservation_object (i.e. the uapi implicit fencing part). If there's > > > any additional work that's queued up in ->prepare_fb then I guess the > > > driver needs to track that internally, but _only_ for kernel-internally > > > queued work. > > That's not a trivial task to work out which of the fence contexts within > > the reservation object are required and which are to be replaced by the > > explicit fence, esp. when you have to consider external fences. > > > The reason for that is that with explicit fencing we want to allow > > > userspace to overwrite any existing implicit fences that might hang > > > around. > > I'm just suggesting the danger of that when userspace doesn't know > > everything and the current interfaces do not allow for userspace to know, > > we only tell userspace about its own action (more or less). > > It's even worse than that. See the kernel can for example swap out objects > any time it wants. > > Userspace doesn't know about such operations and so can't provide them as > explicit fence. > > Same is true for example in situations where one userspace process doesn't > know about operations another process does. E.g. for backward compatibility > with DRI2/3 for example. > > So we will always have a mixture of implicit fences and explicit fences. > > The approach we used for amdgpu is that we implicit wait for all fences > which the initiator of an operation can't know about (e.g. from another > process or kernel internally) and explicitly wait for all additional fences > provided by the initiator or an operation. On android userspace is also supposed to know about explicit fences from other users, i.e. shared buffers. So there you shouldn't wait for implicit fences from other processes, only for kernel-internal stuff. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH v7 0/3] drm: add explict fencing
On Tue, Nov 08, 2016 at 01:43:40PM +0100, Daniel Vetter wrote: > On Tue, Nov 08, 2016 at 11:45:51AM +, Chris Wilson wrote: > > On Tue, Nov 08, 2016 at 12:32:56PM +0100, Daniel Vetter wrote: > > > On Tue, Nov 08, 2016 at 10:35:08AM +, Chris Wilson wrote: > > > > On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: > > > > > From: Gustavo Padovan > > > > > > > > > > Hi, > > > > > > > > > > This is yet another version of the DRM fences patches. Please refer > > > > > to the cover letter[1] in a previous version to check for more > > > > > details. > > > > > > > > Explicit fencing is not a superset of the implicit fences. The driver > > > > may be using implicit fences (on a reservation object) to serialise > > > > asynchronous operations wrt to each other (such as dispatching threads > > > > to flush cpu caches to memory, manipulating page tables and the like > > > > before the flip). Since the user doesn't know about these operations, > > > > they are not included in the explicit fence they provide, at which point > > > > we can't trust their fence to the exclusion of the implicit fences... > > > > > > My thoughts are that in atomic_check drivers just fill in the fence from > > > the reservation_object (i.e. the uapi implicit fencing part). If there's > > > any additional work that's queued up in ->prepare_fb then I guess the > > > driver needs to track that internally, but _only_ for kernel-internally > > > queued work. > > > > That's not a trivial task to work out which of the fence contexts within > > the reservation object are required and which are to be replaced by the > > explicit fence, esp. when you have to consider external fences. > > Hm, what kind of async kernel tasks are you thinking off? Atm I don't know > of anyone who does e.g. clflush through the gpu. And ttm bo placement > moves for display should be explicit enough that drivers will deal with > them correctly. At least that seems to have been the conclusion from the > long amdgpu thread. Now that we (i915) serialise on an reservation_object (obj->resv), we have floated ideas to use that to serialise async tasks (such as offloading the 100ms clflush to a (cpu) worker, a gpu task would pose a similar problem with a fence inserted that is not exposed to userspace). Also tempted to look at using async tasks + fences to do GTT updates but that is not a common pain point at the moment, and cases where it is the GTT thrashing itself is the issue. So how does i915 deal with ttm bo fences? -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH v3 08/11] drm/edid: Remove drm_select_eld
The only user was i915, which is now gone.
Signed-off-by: Maarten Lankhorst
Reviewed-by: Ville Syrjälä
Acked-by: Dave Airlie #irc
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c | 26 --
include/drm/drm_edid.h | 1 -
2 files changed, 27 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9506933b41cd..1801e9c0e41b 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3611,32 +3611,6 @@ int drm_av_sync_delay(struct drm_connector *connector,
EXPORT_SYMBOL(drm_av_sync_delay);
/**
- * drm_select_eld - select one ELD from multiple HDMI/DP sinks
- * @encoder: the encoder just changed display mode
- *
- * It's possible for one encoder to be associated with multiple HDMI/DP sinks.
- * The policy is now hard coded to simply use the first HDMI/DP sink's ELD.
- *
- * Return: The connector associated with the first HDMI/DP sink that has ELD
- * attached to it.
- */
-struct drm_connector *drm_select_eld(struct drm_encoder *encoder)
-{
- struct drm_connector *connector;
- struct drm_device *dev = encoder->dev;
-
- WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
- WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
-
- drm_for_each_connector(connector, dev)
- if (connector->encoder == encoder && connector->eld[0])
- return connector;
-
- return NULL;
-}
-EXPORT_SYMBOL(drm_select_eld);
-
-/**
* drm_detect_hdmi_monitor - detect whether monitor is HDMI
* @edid: monitor EDID information
*
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index c3a7d440bc11..38eabf65f19d 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -330,7 +330,6 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad
**sads);
int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb);
int drm_av_sync_delay(struct drm_connector *connector,
const struct drm_display_mode *mode);
-struct drm_connector *drm_select_eld(struct drm_encoder *encoder);
#ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
int drm_load_edid_firmware(struct drm_connector *connector);
--
2.7.4
[PATCH 2/2] drm/edid: Consider alternate cea timings to be the same VIC
On Tue, Nov 08, 2016 at 12:43:55PM +0100, Andrzej Hajda wrote:
> On 03.11.2016 13:53, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > CEA-861 specifies that the vertical front porch may vary by one or two
> > lines for specific VICs. Up to now we've only considered a mode to match
> > the VIC if it matched the shortest possible vertical front porch length
> > (as that is the variant we store in cea_modes[]). Let's allow our VIC
> > matching to work with the other timings variants as well so that that
> > we'll send out the correct VIC if the variant actually used isn't the
> > one with the shortest vertical front porch.
> >
> > Cc: Shashank Sharma
> > Cc: Andrzej Hajda
> > Cc: Adam Jackson
> > Signed-off-by: Ville Syrjälä
> I have checked against specification and it looks OK.
> I have few nitpicks below regarding implementation, but this could be
> matter of taste, so:
>
> Reviewed-by: Andrzej Hajda
>
>
> > ---
> > drivers/gpu/drm/drm_edid.c | 66
> > +-
> > 1 file changed, 54 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 7eec18925b70..728990fee4ef 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -2613,6 +2613,41 @@ cea_mode_alternate_clock(const struct
> > drm_display_mode *cea_mode)
> > return clock;
> > }
> >
> > +static bool
> > +cea_mode_alternate_timings(u8 vic, struct drm_display_mode *mode)
> > +{
> > + /*
> > +* For certain VICs the spec allows the vertical
> > +* front porch to vary by one or two lines.
> > +*
> > +* cea_modes[] stores the variant with the shortest
> > +* vertical front porch. We can adjust the mode to
> > +* get the other variants by simply increasing the
> > +* vertical front porch length.
> > +*/
> > + BUILD_BUG_ON(edid_cea_modes[8].vtotal != 262 ||
> > +edid_cea_modes[9].vtotal != 262 ||
> > +edid_cea_modes[12].vtotal != 262 ||
> > +edid_cea_modes[13].vtotal != 262 ||
> > +edid_cea_modes[23].vtotal != 312 ||
> > +edid_cea_modes[24].vtotal != 312 ||
> > +edid_cea_modes[27].vtotal != 312 ||
> > +edid_cea_modes[28].vtotal != 312);
>
> I am not sure if this compile check is really necessary,
> I would rather put comment before edid_cea_modes array
> which mode should be put into array in multi-mode case.
Comments can't guarantee this wouldn't break. Doesn't mean
we can't have a comment in addition though.
>
> > +
> > + if (((vic == 8 || vic == 9 ||
> > + vic == 12 || vic == 13) && mode->vtotal < 263) ||
> > + ((vic == 23 || vic == 24 ||
> > + vic == 27 || vic == 28) && mode->vtotal < 314)) {
>
> I wonder if it wouldn't be better to mark somehow these modes
> in the array as alternating ones. This way all info about cea modes
> will be in one place. For example by (ab)using private_flags:
> /* 8 - 720(1440)x240 at 60Hz */
> { DRM_MODE("720x240", DRM_MODE_TYPE_DRIVER, 13500, 720, 739,
>801, 858, 0, 240, 244, 247, 262, 0,
>DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC |
> DRM_MODE_FLAG_DBLCLK),
> .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
> .private_flags = CEA_MODE_FLAG_VFP2},
>
> This is of course just an idea, I am not sure if not overkill :)
Sounds potentially sensible. Should do the same for the alternate clock
thing as well then I suppose.
I was pondering whether I should just convert the cea mode array into
some kind of other data structure that could store multiple variants for
each mode. But then I figured it's maybe a bit too much work for just
these few excepttions.
Another thought that occurred to me recently is that the cea mode list
is getting rather long, so I was wondering if could speed up the lookups
somehow. Currently we just do a linear search through the whole thing.
A simple bsearch() might not work out since the modes aren't necessarily
sorted in any useful order, so we might need some kind of a hash or
something, But then I figured that we shouldn't do these lookups too
often so it might not be worth the effort to optimize it.
>
>
> > + mode->vsync_start++;
> > + mode->vsync_end++;
> > + mode->vtotal++;
> > +
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode
> > *to_match,
> > unsigned int clock_tolerance)
> > {
> > @@ -2622,19 +2657,21 @@ static u8 drm_match_cea_mode_clock_tolerance(const
> > struct drm_display_mode *to_m
> > return 0;
> >
> > for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> > - const struct drm_display_mode *cea_mode = &edid_cea_modes[vic];
> > + struct drm_d
[PATCH] drm: Add stackdepot include for DRM_DEBUG_MM
On Tue, Nov 08, 2016 at 01:46:15PM +0100, Daniel Vetter wrote:
> On Tue, Nov 08, 2016 at 11:56:01AM +, Chris Wilson wrote:
> > 0day found that stackdepot.h doesn't get automatically included on all
> > architectures, so remember to add our #include.
> >
> > Reported-by: kbuild test robot
> > Fixes: 5705670d0463 ("drm: Track drm_mm allocators and show leaks on
> > shutdown")
> > Signed-off-by: Chris Wilson
> > Cc: Daniel Vetter
>
> Thx for the fast fixup, applied to drm-misc.
And I misread the kbuild result. Apparently we all build drm.ko as a
builtin and kbuild doesn't.
depot_save_stack is not exported :(
Is
- depends on DRM
+ depends on DRM=y
a good temporary? Will have to munge i915 similarly. :(
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH v7 3/3] drm/fence: add out-fences support
On Tue, Nov 08, 2016 at 03:54:50PM +0900, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Support DRM out-fences by creating a sync_file with a fence for each CRTC
> that sets the OUT_FENCE_PTR property.
>
> We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
> the sync_file fd back to userspace.
>
> The sync_file and fd are allocated/created before commit, but the
> fd_install operation only happens after we know that commit succeed.
>
> v2: Comment by Rob Clark:
> - Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here.
>
> Comment by Daniel Vetter:
> - Add clean up code for out_fences
>
> v3: Comments by Daniel Vetter:
> - create DRM_MODE_ATOMIC_EVENT_MASK
> - userspace should fill out_fences_ptr with the crtc_ids for which
> it wants fences back.
>
> v4: Create OUT_FENCE_PTR properties and remove old approach.
>
> v5: Comments by Brian Starkey:
> - Remove extra fence_get() in atomic_ioctl()
> - Check ret before iterating on the crtc_state
> - check ret before fd_install
> - set fence_state to NULL at the beginning
> - check fence_state->out_fence_ptr before put_user()
> - change order of fput() and put_unused_fd() on failure
>
> - Add access_ok() check to the out_fence_ptr received
> - Rebase after fence -> dma_fence rename
> - Store out_fence_ptr in the drm_atomic_state
> - Split crtc_setup_out_fence()
> - return -1 as out_fence with TEST_ONLY flag
>
> v6: Comments by Daniel Vetter
> - Add prepare/unprepare_crtc_signaling()
> - move struct drm_out_fence_state to drm_atomic.c
> - mark get_crtc_fence() as static
>
> Comments by Brian Starkey
> - proper set fence_ptr fence_state array
> - isolate fence_idx increment
>
> - improve error handling
>
> Signed-off-by: Gustavo Padovan
Found a bunch more nitpicks, and wishlist items for igt coverage.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic.c | 233
> +++
> drivers/gpu/drm/drm_crtc.c | 8 ++
> include/drm/drm_atomic.h | 1 +
> include/drm/drm_crtc.h | 7 +-
> 4 files changed, 206 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index d1ae463..9a7d84c 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -289,6 +289,25 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
> }
> EXPORT_SYMBOL(drm_atomic_get_crtc_state);
>
> +static void
> +drm_atomic_set_out_fence_for_crtc(struct drm_atomic_state *state,
> + struct drm_crtc *crtc, u64 __user *fence_ptr)
> +{
> + state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
> +}
> +
> +static u64 __user *
> +drm_atomic_get_out_fence_for_crtc(struct drm_atomic_state *state,
> + struct drm_crtc *crtc)
Since these two are static I'd drop the long prefix to make the code
easier to read.
> +{
> + u64 __user *fence_ptr;
> +
> + fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
> + state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
> +
> + return fence_ptr;
> +}
> +
> /**
> * drm_atomic_set_mode_for_crtc - set mode for CRTC
> * @state: the CRTC whose incoming state to update
> @@ -490,6 +509,12 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> &replaced);
> state->color_mgmt_changed |= replaced;
> return ret;
> + } else if (property == config->prop_out_fence_ptr) {
> + uint64_t __user *fence_ptr = u64_to_user_ptr(val);
Don't we need to filter out NULL/0 here like we filter out -1 for the
in-fences? Just in case some userspace samples/restores all properties.
Sounds like an igt is missing for this ...
Maybe we even need a special igt which samples _all_ current atomic
properties, and then does an atomic commit which changes none of them
(without setting the ALLOW_MODESET flag ofc). That /should/ work, and
would catch such bugs in the future.
> + if (!access_ok(VERIFY_WRITE, fence_ptr, sizeof(*fence_ptr)))
> + return -EFAULT;
Same comment about igt coverage I made for patch 1, but with
s/in-fence/out-fence/, and s/~0ULL/8/. I picked 8 as an invalid address !=
NULL.
And the testcase need to cover all possible combinations of output event
generation, i.e. out-fence, event and out-fence+event. So 3x3=9 testcases
for this I think.
> +
> + drm_atomic_set_out_fence_for_crtc(state->state, crtc,
> fence_ptr);
> } else if (crtc->funcs->atomic_set_property)
> return crtc->funcs->atomic_set_property(crtc, state, property,
> val);
> else
> @@ -532,6 +557,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> *val = (state->ctm) ? state->ctm->base.id : 0;
> else if (property == config->gamma_lut_property)
[PATCH v7 0/3] drm: add explict fencing
On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: > From: Gustavo Padovan > > Hi, > > This is yet another version of the DRM fences patches. Please refer > to the cover letter[1] in a previous version to check for more details. > > In v7 we now have split most of the out_fences code into > prepare_crtc_signaling() and unprepare_crtc_signaling() with improved error > handling. More details on the v7 changes are embedded in each commit's > message. > > Robert Foss managed to port Android's drm_hwcomposer to the new HWC2 API and > added support to fences. Current patches can be seen here: > > https://git.collabora.com/cgit/user/robertfoss/drm_hwcomposer.git/log/?h=hwc2_fence_v1 > > He managed to run AOSP on top of padovan/fences kernel branch with full fence > support on qemu/virgl and msm db410c. That means we already have a working > open source userspace using the explicit fencing implementation. > > Also i-g-t testing are available at: > > https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/log/ > > Please review! I think we're getting there. Found some issues with patch 3 still. Besides that I think we need: - r-b/ack from Sean Paul, as an ack that he's happy with what this means for drm_hwcomposer (and that the hwc patches look ok, too). - acks/t-b from everyone who's run this, the more the better. This is a big uabi extension, making the effort by everyone explicit is important. - ack from Brian that he can use the out-fence stuff for his writeback support. Probably need to add a for_each_connector loop to prepare/complete_signalling (and drop the crtc_ in there, but Brian can do that). Cheers, Daniel > > Gustavo > > [1] https://www.mail-archive.com/linux-kernel at > vger.kernel.org/msg1253822.html > > --- > Gustavo Padovan (3): > drm/fence: add in-fences support > drm/fence: add fence timeline to drm_crtc > drm/fence: add out-fences support > > drivers/gpu/drm/Kconfig | 1 + > drivers/gpu/drm/drm_atomic.c| 247 > ++-- > drivers/gpu/drm/drm_atomic_helper.c | 3 + > drivers/gpu/drm/drm_crtc.c | 45 +++ > drivers/gpu/drm/drm_plane.c | 1 + > include/drm/drm_atomic.h| 1 + > include/drm/drm_crtc.h | 55 > 7 files changed, 311 insertions(+), 42 deletions(-) > > -- > 2.5.5 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 1/2] drm/i915: Restrict DRM_DEBUG_MM automatic selection
DRM_DEBUG_MM is only valid if the DRM.ko is builtin as currently
depot_save_stack is not exported.
Fixes: 5c7fcf2db027 ("drm/i915: Enable drm_mm debug when enabling
DRM_I915_DEBUG")
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/Kconfig.debug | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/Kconfig.debug
b/drivers/gpu/drm/i915/Kconfig.debug
index 69edf196ed03..51ba630a134b 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -21,7 +21,7 @@ config DRM_I915_DEBUG
select PREEMPT_COUNT
select X86_MSR # used by igt/pm_rpm
select DRM_VGEM # used by igt/prime_vgem (dmabuf interop checks)
-select DRM_DEBUG_MM
+select DRM_DEBUG_MM if DRM=y
default n
help
Choose this option to turn on extra driver debugging that may affect
--
2.10.2
[PATCH 2/2] drm: Restrict stackdepot usage to builtin drm.ko
I misread the kbuild result thinking that we had missed the include
(which we had for completeness anyway), what kbuild was actually warning
me about was that depot_save_stack was not exported.
Temporarily fix this by only selecting STACKDEPOT iff drm.ko is builtin
Reported-by: kbuild test robot
Fixes: 5705670d0463 ("drm: Track drm_mm allocators and show leaks on shutdown")
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 25e8e3793d29..d6ee0592b625 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -36,7 +36,7 @@ config DRM_DP_AUX_CHARDEV
config DRM_DEBUG_MM
bool "Insert extra checks and debug info into the DRM range managers"
default n
- depends on DRM
+ depends on DRM=y
select STACKDEPOT
help
Enable allocation tracking of memory manager and leak detection on
--
2.10.2
[PATCH RFC 00/12] tda998x updates
Hi Russell, On 08/11/16 12:24, Russell King - ARM Linux wrote: > As no one responded to the previous round, I'm not spending soo much > time writing up a description of these changes again. It's also been > quite a long time, so I've forgotten all the details of the changes, > so I'll do my best. > > Changes from the previous series include: > - reorder the initial three patches > - change the (now third patch)... I think to increase the size of the > locked region. > - fix edid parsing for infoframe generation - as was pointed out for > dw-hdmi, parsing the EDID in get_modes() is incorrect, as that method > will not be called when an override-edid is in effect. We need to > parse the override-edid. Moreover, infoframe generation should not > be keyed to whether the monitor is HDMI or not, CEA-861B allows non- > HDMI to send infoframes. > - only send audio if audio and infoframes are supported. > > Otherwise, these are very much like the previous posting of the series, > except rebased upon the mali/hdlcd/tda998x change to remove the > drm_connector_register() call. > > https://www.spinics.net/lists/dri-devel/msg121495.html > > It'd be nice to have other tda998x users ack and test these patches, I've just merged your drm-tda998x-devel branch and booted it on my Juno, and I can at least confirm that DVI still seems to still work as before. Anything in particular I should be looking out for? (Unfortunately the only handy HDMI monitor is one of those slightly-out-of-spec ones which has never really worked with the pixel clocks Juno provides) Robin. > I've tried to test on Juno, but the Juno situation seems to be a huge > fail. (HBI0282B completely fails with latest firmware - (a) FPGA image > incompatibilities io_b118 causes all FPGA AMBA devices to vanish, (b) > seems no way to get SCPI support on it - adding the BL0 executable > start address in the SCC registers seems to be incompatible with the > devchip, causing the PLLs to fail. In discussion with Sudeep over > these issues, but no idea where things are with it at the moment, other > than Sudeep needs to investigate. All Linaro firmware releases are > broken on HBI0282B.) > > drivers/gpu/drm/i2c/tda998x_drv.c | 826 > -- > 1 file changed, 429 insertions(+), 397 deletions(-) >
[Bug 95306] Random Blank(black) screens on "Carrizo"
https://bugs.freedesktop.org/show_bug.cgi?id=95306 --- Comment #32 from Patrick Laurin --- I confirm still present on rc4. Guess I only got lucky on rc3 for 45 minutes. On Nov 8, 2016 03:10, wrote: > *Comment # 31 <https://bugs.freedesktop.org/show_bug.cgi?id=95306#c31> on > bug 95306 <https://bugs.freedesktop.org/show_bug.cgi?id=95306> from Tom > * > > The suspend issue is still present, it just doesn't happen so often. > I also noticed another thing. It seems to be impossible to cold boot with > 4.9rc4, I have to boot some other kernel or Windows and then reboot to > successfully boot with 4.9rc4. > So far no random blanking, but it already happened before, that it ran for > hours without blanking. > > -- > You are receiving this mail because: > >- You are on the CC list for the bug. > > -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/337c2123/attachment.html>
[Bug 95306] Random Blank(black) screens on "Carrizo"
https://bugs.freedesktop.org/show_bug.cgi?id=95306 --- Comment #33 from Patrick Laurin --- Issue == screen turning black after a while. On Nov 8, 2016 08:35, "Patrick Laurin" wrote: > I confirm still present on rc4. Guess I only got lucky on rc3 for 45 > minutes. > > On Nov 8, 2016 03:10, wrote: > >> *Comment # 31 <https://bugs.freedesktop.org/show_bug.cgi?id=95306#c31> on >> bug 95306 <https://bugs.freedesktop.org/show_bug.cgi?id=95306> from Tom >> * >> >> The suspend issue is still present, it just doesn't happen so often. >> I also noticed another thing. It seems to be impossible to cold boot with >> 4.9rc4, I have to boot some other kernel or Windows and then reboot to >> successfully boot with 4.9rc4. >> So far no random blanking, but it already happened before, that it ran for >> hours without blanking. >> >> -- >> You are receiving this mail because: >> >>- You are on the CC list for the bug. >> >> -- You are receiving this mail because: You are the assignee for the bug. ------ next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/b5efdd79/attachment-0001.html>
[GIT PULL][drm-next] drm/mali-dp fixes and updates
On Tue, Nov 08, 2016 at 01:49:55PM +0100, Daniel Vetter wrote: > On Tue, Nov 08, 2016 at 12:26:19PM +, Liviu Dudau wrote: > > Hi Dave, > > > > Here is the list of fixes that I have for drm/mali-dp. They've been on the > > mailing > > lists for a while and merged into linux-next for a few weeks, but due to > > holiday and > > travel to Linux Plumbers I did not send the pull request earlier. I don't > > know if > > these patches can be pulled into v4.9 still (they will conflict with Ville > > Syrjälä's > > cleanup of DRM_ROTATE series that is already in drm-next), but if you do > > that would > > be great. > > > > The following changes since commit fb422950c6cd726fd36eb72a7cf84583440a18a2: > > > > Merge branch 'linux-4.9' of git://github.com/skeggsb/linux into drm-next > > (2016-10-28 14:24:56 +1000) > > > > are available in the git repository at: > > > > git://linux-arm.org/linux-ld.git for-upstream/mali-dp > > > > for you to fetch changes up to e64053f05eb924db45f90a1556a200d1acb4b01e: > > > > drm: mali-dp: Clear CVAL when leaving config mode (2016-11-08 11:40:02 > > +) > > > > > > Baoyou Xie (1): > > drm/arm: mark symbols static where possible > > > > Brian Starkey (8): > > drm: mali-dp: Add pitch alignment check function > > drm: mali-dp: Add pitch alignment check for planes > > arm: mali-dp: Extract mode_config cleanup into malidp_fini > > drm: mali-dp: Refactor plane initialisation > > drm: mali-dp: Enable alpha blending > > drm: mali-dp: Store internal format and n_planes in plane state > > drm: mali-dp: Don't set DRM_PLANE_COMMIT_ACTIVE_ONLY > > drm: mali-dp: Clear CVAL when leaving config mode > > > > Liviu Dudau (3): > > drm: mali-dp: Clear the config_valid flag before using it in > > wait_event. > > drm: mali-dp: Set the drm->irq_enabled flag to match driver's state. > > drm: mali-dp: Add support for setting plane's rotation property from > > userspace. > > Quick check says these patches haven't shown on up a mailing list. Please > submit first, do review in public and then send the pull request. There's > generally a lot of cross-driver learning possible with this kind of stuff. > Imo splitting drivers into their own mailing list and community only makes > sense once there's just too much traffic, like with the big ones (intel, > amd and nouveau because it's developed in userspace entirely). Actually, all except the last patch from me and last from Brian have been on the dri-devel mailing list, I believe you have even commented on one of them (drm->irq_enabled). For my patches, first two start here [1]. For Brian's patches, his series starts here [2] (also contains my patches) and one before last is here [3] [1] https://lists.freedesktop.org/archives/dri-devel/2016-July/114302.html [2] https://lists.freedesktop.org/archives/dri-devel/2016-October/120486.html [3] https://lists.freedesktop.org/archives/dri-devel/2016-October/121849.html I appologise for missing to send the last two patches, they were in the public tree but did not sent them for some reason to the public list. Will do that now. Best regards, Liviu > > Thanks, Daniel > > > > drivers/gpu/drm/arm/malidp_drv.c| 19 +--- > > drivers/gpu/drm/arm/malidp_drv.h| 3 ++ > > drivers/gpu/drm/arm/malidp_hw.c | 5 ++ > > drivers/gpu/drm/arm/malidp_hw.h | 9 > > drivers/gpu/drm/arm/malidp_planes.c | 96 > > - > > 5 files changed, 92 insertions(+), 40 deletions(-) > > > > > > Many thanks, > > Liviu > > > > -- > > > > | I would like to | > > | fix the world, | > > | but they're not | > > | giving me the | > > \ source code! / > > --- > > ¯\_(ã)_/¯ > > ___ > > dri-devel mailing list > > dri-devel at lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ã)_/¯
[PATCH] drm: mali-dp: Clear CVAL when leaving config mode
It's possible for CVAL to get set whilst we are in config mode. If this
happens, afer we leave config mode the HW will latch whatever
configuration is in the registers at the next vsync. Most likely this
will be a partial configuration, as we'll be racing against the ongoing
atomic_commit.
To avoid this, clear CVAL before leaving config mode.
Signed-off-by: Brian Starkey
---
drivers/gpu/drm/arm/malidp_hw.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 7f4a0bd..65e667b 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -125,6 +125,7 @@ static void malidp500_leave_config_mode(struct
malidp_hw_device *hwdev)
{
u32 status, count = 100;
+ malidp_hw_clearbits(hwdev, MALIDP_CFG_VALID, MALIDP500_CONFIG_VALID);
malidp_hw_clearbits(hwdev, MALIDP500_DC_CONFIG_REQ,
MALIDP500_DC_CONTROL);
while (count) {
status = malidp_hw_read(hwdev, hwdev->map.dc_base +
MALIDP_REG_STATUS);
@@ -271,6 +272,7 @@ static void malidp550_leave_config_mode(struct
malidp_hw_device *hwdev)
{
u32 status, count = 100;
+ malidp_hw_clearbits(hwdev, MALIDP_CFG_VALID, MALIDP550_CONFIG_VALID);
malidp_hw_clearbits(hwdev, MALIDP550_DC_CONFIG_REQ,
MALIDP550_DC_CONTROL);
while (count) {
status = malidp_hw_read(hwdev, hwdev->map.dc_base +
MALIDP_REG_STATUS);
--
1.7.9.5
[PATCH] drm/amdgpu/powerplay/smu7: fix unintialized data usage
A recent bugfix replaced an out-of-bounds access with direct
use of unintialized data:
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c: In function
'smu7_patch_limits_vddc':
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c:2033:6: error: 'vddc' may be
used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c:2146:11: note: 'vddc' was
declared here
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c:2033:6: error: 'vddci' may be
used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c:2146:17: note: 'vddci' was
declared here
uint32_t vddc, vddci;
This initializes the data as before using the correct type.
Fixes: 77f7f71f5be1 ("drm/amdgpu/powerplay/smu7: fix static checker warning")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 9e49f2777143..b2d61d043d52 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -2147,9 +2147,11 @@ static int smu7_patch_limits_vddc(struct pp_hwmgr *hwmgr,
struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
if (tab) {
+ vddc = tab->vddc;
smu7_patch_ppt_v0_with_vdd_leakage(hwmgr, &vddc,
&data->vddc_leakage);
tab->vddc = vddc;
+ vddci = tab->vddci;
smu7_patch_ppt_v0_with_vdd_leakage(hwmgr, &vddci,
&data->vddci_leakage);
tab->vddci = vddci;
--
2.9.0
[PATCH] drm/nouveau: fix LEDS_CLASS=m configuration
The newly introduced LED handling for nouveau fails to link when the
driver is built-in but the LED subsystem is a loadable module:
drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_suspend':
tvnv17.c:(.text.nouveau_do_suspend+0x10): undefined reference to
`nouveau_led_suspend'
drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_resume':
tvnv17.c:(.text.nouveau_do_resume+0xf0): undefined reference to
`nouveau_led_resume'
drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_unload':
tvnv17.c:(.text.nouveau_drm_unload+0x34): undefined reference to
`nouveau_led_fini'
drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_load':
tvnv17.c:(.text.nouveau_drm_load+0x7d0): undefined reference to
`nouveau_led_init'
This adds a separate Kconfig symbol for the LED support that
correctly tracks the dependencies.
Fixes: 8d021d71b324 ("drm/nouveau/drm/nouveau: add a LED driver for the NVIDIA
logo")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/nouveau/Kbuild| 2 +-
drivers/gpu/drm/nouveau/Kconfig | 8
drivers/gpu/drm/nouveau/nouveau_led.h | 2 +-
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/Kbuild b/drivers/gpu/drm/nouveau/Kbuild
index fde6e3656636..5e00e911daa6 100644
--- a/drivers/gpu/drm/nouveau/Kbuild
+++ b/drivers/gpu/drm/nouveau/Kbuild
@@ -22,7 +22,7 @@ nouveau-$(CONFIG_DEBUG_FS) += nouveau_debugfs.o
nouveau-y += nouveau_drm.o
nouveau-y += nouveau_hwmon.o
nouveau-$(CONFIG_COMPAT) += nouveau_ioc32.o
-nouveau-$(CONFIG_LEDS_CLASS) += nouveau_led.o
+nouveau-$(CONFIG_DRM_NOUVEAU_LED) += nouveau_led.o
nouveau-y += nouveau_nvif.o
nouveau-$(CONFIG_NOUVEAU_PLATFORM_DRIVER) += nouveau_platform.o
nouveau-y += nouveau_usif.o # userspace <-> nvif
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 78631fb61adf..715cd6f4dc31 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -46,6 +46,14 @@ config NOUVEAU_DEBUG
The paranoia and spam levels will add a lot of extra checks which
may potentially slow down driver operation.
+config DRM_NOUVEAU_LED
+ bool "Support for logo LED"
+ depends on DRM_NOUVEAU && LEDS_CLASS
+ depends on !(DRM_NOUVEAU=y && LEDS_CLASS=m)
+ help
+ Say Y here to enabling controlling the brightness of the
+ LED behind NVIDIA logo on certain Titan cards.
+
config NOUVEAU_DEBUG_DEFAULT
int "Default debug level"
depends on DRM_NOUVEAU
diff --git a/drivers/gpu/drm/nouveau/nouveau_led.h
b/drivers/gpu/drm/nouveau/nouveau_led.h
index 187ecdb82002..9c9bb6ac938e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_led.h
+++ b/drivers/gpu/drm/nouveau/nouveau_led.h
@@ -42,7 +42,7 @@ nouveau_led(struct drm_device *dev)
}
/* nouveau_led.c */
-#if IS_ENABLED(CONFIG_LEDS_CLASS)
+#if IS_ENABLED(CONFIG_DRM_NOUVEAU_LED)
int nouveau_led_init(struct drm_device *dev);
void nouveau_led_suspend(struct drm_device *dev);
void nouveau_led_resume(struct drm_device *dev);
--
2.9.0
[PATCH] drm/i915: avoid harmless empty-body warning
The newly added assert_kernel_context_is_current introduces a warning
when built with W=1:
drivers/gpu/drm/i915/i915_gem.c: In function
âassert_kernel_context_is_currentâ:
drivers/gpu/drm/i915/i915_gem.c:4417:63: error: suggest braces around empty
body in an âelseâ statement [-Werror=empty-body]
Changing the GEM_BUG_ON() macro from an empty definition to "do { } while (0)"
makes the macro more robust to use and avoids the warning.
Fixes: 3033acab07f9 ("drm/i915: Queue the idling context switch after all other
timelines")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/i915/i915_gem.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 735580d72eb1..51ec793f2e20 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -28,7 +28,7 @@
#ifdef CONFIG_DRM_I915_DEBUG_GEM
#define GEM_BUG_ON(expr) BUG_ON(expr)
#else
-#define GEM_BUG_ON(expr)
+#define GEM_BUG_ON(expr) do { } while (0)
#endif
#define I915_NUM_ENGINES 5
--
2.9.0
[Bug 98523] Can't run without "nomodeset" on MacPro6,1 with two AMD R9 280X Tahiti video cards
https://bugs.freedesktop.org/show_bug.cgi?id=98523 Alex Deucher changed: What|Removed |Added Resolution|--- |NOTOURBUG Status|NEW |RESOLVED --- Comment #2 from Alex Deucher --- The problem is Macs expose the vbios for some cards in a proprietary way. IIRC, the vbios is only available via EFI prior to the OS loading. The bootloader needs to snag the vbios and then make it available to the OS when it loads. I'm not really a Mac expert so I don't remember all the details. There's no way to work around it in the driver. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/650e3675/attachment.html>
