Re: [Intel-gfx] [PATCH 0/3] drm/i915: Drop legacy IOCTLs on new HW
On Mon, 15 Mar 2021 07:34:25 -0700, Jason Ekstrand wrote: > > These three patches exist to clean up some of our IOCTL mess in i915. > We've got more clean-up we should do eventually, but these are some of the > easiest to drop and most egregious cases. > > Test-with: 20210121083742.46592-1-ashutosh.di...@intel.com Hi Jason, Sorry the above IGT build is too old and has been discarded so no tests were running on actual HW as is mentioned here: https://intel-gfx-ci.01.org/test-with.html I resubmitted the IGT patch today so we have a newer IGT build and have just resubmitted this series with that IGT build. There are no other changes with the actual patches themselves. Thanks. -- Ashutosh > > Ashutosh Dixit (1): > drm/i915: Disable pread/pwrite ioctl's for future platforms (v3) > > Jason Ekstrand (2): > drm/i915/gem: Drop legacy execbuffer support (v2) > drm/i915/gem: Drop relocation support on all new hardware (v5) > > .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 113 ++ > drivers/gpu/drm/i915/gem/i915_gem_ioctls.h| 2 - > drivers/gpu/drm/i915/i915_drv.c | 2 +- > drivers/gpu/drm/i915/i915_gem.c | 14 +++ > include/uapi/drm/i915_drm.h | 1 + > 5 files changed, 26 insertions(+), 106 deletions(-) > > -- > 2.29.2 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] dma-buf: add dma_resv_get_singleton_rcu (v3)
Add a helper function to get a single fence representing
all fences in a dma_resv object.
This fence is either the only one in the object or all not
signaled fences of the object in a flatted out dma_fence_array.
v2 (Jason Ekstrand):
- Take reference of fences both for creating the dma_fence_array and in
the case where we return one fence.
- Handle the case where dma_resv_get_list() returns NULL
v3 (Jason Ekstrand):
- Add an _rcu suffix because it is read-only
- Rewrite to use dma_resv_get_fences_rcu so it's RCU-safe
- Add an EXPORT_SYMBOL_GPL declaration
- Re-author the patch to Jason since very little is left of Christian
König's original patch
Signed-off-by: Jason Ekstrand
---
drivers/dma-buf/dma-resv.c | 104 +
include/linux/dma-resv.h | 2 +
2 files changed, 106 insertions(+)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index 6ddbeb5dfbf65..5dd4c38bd9cb4 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -33,6 +33,8 @@
*/
#include
+#include
+#include
#include
#include
#include
@@ -49,6 +51,19 @@
* write-side updates.
*/
+/**
+ * dma_fence_deep_dive_for_each - deep dive into the fence containers
+ * @fence: resulting fence
+ * @chain: variable for a dma_fence_chain
+ * @index: index into a dma_fence_array
+ * @head: starting point
+ *
+ * Helper to deep dive into the fence containers for flattening them.
+ */
+#define dma_fence_deep_dive_for_each(fence, chain, index, head)\
+ dma_fence_chain_for_each(chain, head) \
+ dma_fence_array_for_each(fence, index, chain)
+
DEFINE_WD_CLASS(reservation_ww_class);
EXPORT_SYMBOL(reservation_ww_class);
@@ -517,6 +532,95 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
}
EXPORT_SYMBOL_GPL(dma_resv_get_fences_rcu);
+/**
+ * dma_resv_get_singleton - get a single fence for the dma_resv object
+ * @obj: the reservation object
+ * @extra: extra fence to add to the resulting array
+ * @result: resulting dma_fence
+ *
+ * Get a single fence representing all unsignaled fences in the dma_resv object
+ * plus the given extra fence. If we got only one fence return a new
+ * reference to that, otherwise return a dma_fence_array object.
+ *
+ * RETURNS
+ * Returns -NOMEM if allocations fail, zero otherwise.
+ */
+int dma_resv_get_singleton_rcu(struct dma_resv *obj, struct dma_fence **result)
+{
+ struct dma_fence **resv_fences, *fence, *chain, **fences;
+ struct dma_fence_array *array;
+ unsigned int num_resv_fences, num_fences;
+ unsigned int ret, i, j;
+
+ ret = dma_resv_get_fences_rcu(obj, NULL, _resv_fences,
_fences);
+ if (ret)
+ return ret;
+
+ num_fences = 0;
+ *result = NULL;
+
+ if (num_resv_fences == 0)
+ return 0;
+
+ for (i = 0; i < num_resv_fences; ++i) {
+ dma_fence_deep_dive_for_each(fence, chain, j, resv_fences[i]) {
+ if (dma_fence_is_signaled(fence))
+ continue;
+
+ *result = fence;
+ ++num_fences;
+ }
+ }
+
+ if (num_fences <= 1) {
+ *result = dma_fence_get(*result);
+ goto put_resv_fences;
+ }
+
+ fences = kmalloc_array(num_fences, sizeof(struct dma_fence*),
+ GFP_KERNEL);
+ if (!fences) {
+ *result = NULL;
+ ret = -ENOMEM;
+ goto put_resv_fences;
+ }
+
+ num_fences = 0;
+ for (i = 0; i < num_resv_fences; ++i) {
+ dma_fence_deep_dive_for_each(fence, chain, j, resv_fences[i]) {
+ if (!dma_fence_is_signaled(fence))
+ fences[num_fences++] = dma_fence_get(fence);
+ }
+ }
+
+ if (num_fences <= 1) {
+ *result = num_fences ? fences[0] : NULL;
+ kfree(fences);
+ goto put_resv_fences;
+ }
+
+ array = dma_fence_array_create(num_fences, fences,
+ dma_fence_context_alloc(1),
+ 1, false);
+ if (array) {
+ *result = >base;
+ } else {
+ *result = NULL;
+ while (num_fences--)
+ dma_fence_put(fences[num_fences]);
+ kfree(fences);
+ ret = -ENOMEM;
+ }
+
+put_resv_fences:
+ while (num_resv_fences--)
+ dma_fence_put(resv_fences[num_resv_fences]);
+ kfree(resv_fences);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dma_resv_get_singleton_rcu);
+
/**
* dma_resv_wait_timeout_rcu - Wait on reservation's objects
* shared and/or exclusive fences.
diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
index d44a77e8a7e34..5f82894fed0b9 100644
--- a/include/linux/dma-resv.h
+++
[PATCH 3/3] dma-buf: Add an API for exporting sync files (v7)
Modern userspace APIs like Vulkan are built on an explicit
synchronization model. This doesn't always play nicely with the
implicit synchronization used in the kernel and assumed by X11 and
Wayland. The client -> compositor half of the synchronization isn't too
bad, at least on intel, because we can control whether or not i915
synchronizes on the buffer and whether or not it's considered written.
The harder part is the compositor -> client synchronization when we get
the buffer back from the compositor. We're required to be able to
provide the client with a VkSemaphore and VkFence representing the point
in time where the window system (compositor and/or display) finished
using the buffer. With current APIs, it's very hard to do this in such
a way that we don't get confused by the Vulkan driver's access of the
buffer. In particular, once we tell the kernel that we're rendering to
the buffer again, any CPU waits on the buffer or GPU dependencies will
wait on some of the client rendering and not just the compositor.
This new IOCTL solves this problem by allowing us to get a snapshot of
the implicit synchronization state of a given dma-buf in the form of a
sync file. It's effectively the same as a poll() or I915_GEM_WAIT only,
instead of CPU waiting directly, it encapsulates the wait operation, at
the current moment in time, in a sync_file so we can check/wait on it
later. As long as the Vulkan driver does the sync_file export from the
dma-buf before we re-introduce it for rendering, it will only contain
fences from the compositor or display. This allows to accurately turn
it into a VkFence or VkSemaphore without any over- synchronization.
v2 (Jason Ekstrand):
- Use a wrapper dma_fence_array of all fences including the new one
when importing an exclusive fence.
v3 (Jason Ekstrand):
- Lock around setting shared fences as well as exclusive
- Mark SIGNAL_SYNC_FILE as a read-write ioctl.
- Initialize ret to 0 in dma_buf_wait_sync_file
v4 (Jason Ekstrand):
- Use the new dma_resv_get_singleton helper
v5 (Jason Ekstrand):
- Rename the IOCTLs to import/export rather than wait/signal
- Drop the WRITE flag and always get/set the exclusive fence
v6 (Jason Ekstrand):
- Drop the sync_file import as it was all-around sketchy and not nearly
as useful as import.
- Re-introduce READ/WRITE flag support for export
- Rework the commit message
v7 (Jason Ekstrand):
- Require at least one sync flag
- Fix a refcounting bug: dma_resv_get_excl() doesn't take a reference
- Use _rcu helpers since we're accessing the dma_resv read-only
Signed-off-by: Jason Ekstrand
---
drivers/dma-buf/dma-buf.c| 58
include/uapi/linux/dma-buf.h | 6
2 files changed, 64 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index f264b70c383eb..69200d019ac90 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -362,6 +363,60 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const
char __user *buf)
return ret;
}
+static long dma_buf_export_sync_file(struct dma_buf *dmabuf,
+void __user *user_data)
+{
+ struct dma_buf_sync_file arg;
+ struct dma_fence *fence = NULL;
+ struct sync_file *sync_file;
+ int fd, ret;
+
+ if (copy_from_user(, user_data, sizeof(arg)))
+ return -EFAULT;
+
+ if (arg.flags & ~DMA_BUF_SYNC_RW)
+ return -EINVAL;
+
+ if ((arg.flags & DMA_BUF_SYNC_RW) == 0)
+ return -EINVAL;
+
+ fd = get_unused_fd_flags(O_CLOEXEC);
+ if (fd < 0)
+ return fd;
+
+ if (arg.flags & DMA_BUF_SYNC_WRITE) {
+ ret = dma_resv_get_singleton_rcu(dmabuf->resv, );
+ if (ret)
+ goto err_put_fd;
+ } else if (arg.flags & DMA_BUF_SYNC_READ) {
+ fence = dma_resv_get_excl_rcu(dmabuf->resv);
+ }
+
+ if (!fence)
+ fence = dma_fence_get_stub();
+
+ sync_file = sync_file_create(fence);
+
+ dma_fence_put(fence);
+
+ if (!sync_file) {
+ ret = -EINVAL;
+ goto err_put_fd;
+ }
+
+ fd_install(fd, sync_file->file);
+
+ arg.fd = fd;
+ if (copy_to_user(user_data, , sizeof(arg)))
+ return -EFAULT;
+
+ return 0;
+
+err_put_fd:
+ put_unused_fd(fd);
+ return ret;
+}
+
static long dma_buf_ioctl(struct file *file,
unsigned int cmd, unsigned long arg)
{
@@ -405,6 +460,9 @@ static long dma_buf_ioctl(struct file *file,
case DMA_BUF_SET_NAME_B:
return dma_buf_set_name(dmabuf, (const char __user *)arg);
+ case DMA_BUF_IOCTL_EXPORT_SYNC_FILE:
+ return dma_buf_export_sync_file(dmabuf, (void __user *)arg);
+
default:
[PATCH 0/3] dma-buf: Add an API for exporting sync files (v7)
Modern userspace APIs like Vulkan are built on an explicit synchronization model. This doesn't always play nicely with the implicit synchronization used in the kernel and assumed by X11 and Wayland. The client -> compositor half of the synchronization isn't too bad, at least on intel, because we can control whether or not i915 synchronizes on the buffer and whether or not it's considered written. The harder part is the compositor -> client synchronization when we get the buffer back from the compositor. We're required to be able to provide the client with a VkSemaphore and VkFence representing the point in time where the window system (compositor and/or display) finished using the buffer. With current APIs, it's very hard to do this in such a way that we don't get confused by the Vulkan driver's access of the buffer. In particular, once we tell the kernel that we're rendering to the buffer again, any CPU waits on the buffer or GPU dependencies will wait on some of the client rendering and not just the compositor. This new IOCTL solves this problem by allowing us to get a snapshot of the implicit synchronization state of a given dma-buf in the form of a sync file. It's effectively the same as a poll() or I915_GEM_WAIT only, instead of CPU waiting directly, it encapsulates the wait operation, at the current moment in time, in a sync_file so we can check/wait on it later. As long as the Vulkan driver does the sync_file export from the dma-buf before we re-introduce it for rendering, it will only contain fences from the compositor or display. This allows to accurately turn it into a VkFence or VkSemaphore without any over- synchronization. The only remaining TODO item as far as I know is that some kernel CI system is sending me nastygrams about build system problems. With this change, dma-fence now has a dependency on sync_file and I really don't know how best to solve it. Should we make sync_file no longer optional? Should I hide the new ioctl behind a #define? If so, what #define? I'm a bit lost when it comes to KConfig, I'm afraid. Mesa MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 IGT tests: https://lists.freedesktop.org/archives/igt-dev/2021-March/029825.html Cc: Christian König Cc: Michel Dänzer Cc: Dave Airlie Cc: Bas Nieuwenhuizen Cc: Daniel Stone Christian König (2): dma-buf: add dma_fence_array_for_each (v2) dma-buf: add dma_resv_get_singleton (v2) Jason Ekstrand (1): dma-buf: Add an API for exporting sync files (v6) drivers/dma-buf/dma-buf.c | 55 ++ drivers/dma-buf/dma-fence-array.c | 27 +++ drivers/dma-buf/dma-resv.c| 118 ++ include/linux/dma-fence-array.h | 17 + include/linux/dma-resv.h | 3 + include/uapi/linux/dma-buf.h | 6 ++ 6 files changed, 226 insertions(+) -- 2.29.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] dma-buf: add dma_fence_array_for_each (v2)
From: Christian König
Add a helper to iterate over all fences in a dma_fence_array object.
v2 (Jason Ekstrand)
- Return NULL from dma_fence_array_first if head == NULL. This matches
the iterator behavior of dma_fence_chain_for_each in that it iterates
zero times if head == NULL.
- Return NULL from dma_fence_array_next if index > array->num_fences.
Signed-off-by: Jason Ekstrand
---
drivers/dma-buf/dma-fence-array.c | 27 +++
include/linux/dma-fence-array.h | 17 +
2 files changed, 44 insertions(+)
diff --git a/drivers/dma-buf/dma-fence-array.c
b/drivers/dma-buf/dma-fence-array.c
index d3fbd950be944..2ac1afc697d0f 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -201,3 +201,30 @@ bool dma_fence_match_context(struct dma_fence *fence, u64
context)
return true;
}
EXPORT_SYMBOL(dma_fence_match_context);
+
+struct dma_fence *dma_fence_array_first(struct dma_fence *head)
+{
+ struct dma_fence_array *array;
+
+ if (!head)
+ return NULL;
+
+ array = to_dma_fence_array(head);
+ if (!array)
+ return head;
+
+ return array->fences[0];
+}
+EXPORT_SYMBOL(dma_fence_array_first);
+
+struct dma_fence *dma_fence_array_next(struct dma_fence *head,
+ unsigned int index)
+{
+ struct dma_fence_array *array = to_dma_fence_array(head);
+
+ if (!array || index >= array->num_fences)
+ return NULL;
+
+ return array->fences[index];
+}
+EXPORT_SYMBOL(dma_fence_array_next);
diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h
index 303dd712220fd..588ac8089dd61 100644
--- a/include/linux/dma-fence-array.h
+++ b/include/linux/dma-fence-array.h
@@ -74,6 +74,19 @@ to_dma_fence_array(struct dma_fence *fence)
return container_of(fence, struct dma_fence_array, base);
}
+/**
+ * dma_fence_array_for_each - iterate over all fences in array
+ * @fence: current fence
+ * @index: index into the array
+ * @head: potential dma_fence_array object
+ *
+ * Test if @array is a dma_fence_array object and if yes iterate over all
fences
+ * in the array. If not just iterate over the fence in @array itself.
+ */
+#define dma_fence_array_for_each(fence, index, head) \
+ for (index = 0, fence = dma_fence_array_first(head); fence; \
+++(index), fence = dma_fence_array_next(head, index))
+
struct dma_fence_array *dma_fence_array_create(int num_fences,
struct dma_fence **fences,
u64 context, unsigned seqno,
@@ -81,4 +94,8 @@ struct dma_fence_array *dma_fence_array_create(int num_fences,
bool dma_fence_match_context(struct dma_fence *fence, u64 context);
+struct dma_fence *dma_fence_array_first(struct dma_fence *head);
+struct dma_fence *dma_fence_array_next(struct dma_fence *head,
+ unsigned int index);
+
#endif /* __LINUX_DMA_FENCE_ARRAY_H */
--
2.29.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/komeda: Fix off-by-1 when with readback conn due to rounding
On Fri, Mar 12, 2021 at 10:55:21AM +, Brian Starkey wrote: > (Adding back James again - did you use get_maintainer.pl?) > > On Thu, Mar 11, 2021 at 12:08:46PM +, carsten.haitz...@foss.arm.com wrote: > > From: Carsten Haitzler > > > > When setting up a readback connector that writes data back to memory > > rather than to an actual output device (HDMI etc.), rounding was set > > to round. As the DPU uses a higher internal number of bits when generating > > a color value, this round-down back to 8bit ended up with everything > > being off-by one. e.g. #fefefe became #ff. This sets > > Perhaps overly pedantic, but now we've tracked down what was actually > happening I think we can be more precise here. Not _everything_ is > off-by-one, it's just rounding in the standard sense - if the most > significant bit-to-be-discarded is set, the value is rounded up to > minimise the absolute error introduced by bit-depth reduction. > > > rounding to "round-down" so things end up correct by turning on the LW_TRC > > round down flag. > > Can we call it "truncate" rather than round down? I think it makes > "TRC" a bit more understandable. > > > > > Signed-off-by: Carsten Haitzler > > --- > > drivers/gpu/drm/arm/display/komeda/d71/d71_component.c | 7 ++- > > drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h | 1 + > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c > > b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c > > index 8a02ade369db..e97acc5519d1 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c > > @@ -468,7 +468,12 @@ static void d71_wb_layer_update(struct > > komeda_component *c, > > struct komeda_layer_state *st = to_layer_st(state); > > struct drm_connector_state *conn_st = state->wb_conn->state; > > struct komeda_fb *kfb = to_kfb(conn_st->writeback_job->fb); > > - u32 ctrl = L_EN | LW_OFM, mask = L_EN | LW_OFM | LW_TBU_EN; > > + /* LW_TRC sets rounding to truncate not round which is needed for > > +* the output of writeback to match the input in the most common > > +* use cases like RGB888 -> RGB888, so set this bit by default > > +*/ > > Hm, not sure why this file uses "net/" style comments, but as you > said, this is in-keeping with the rest of the file, so meh :-) > > > + u32 ctrl = LW_TRC | L_EN | LW_OFM; > > + u32 mask = LW_TRC | L_EN | LW_OFM | LW_TBU_EN; > > If you were aiming for matching register order, this should be: > > L_EN | LW_TRC | LW_OFM | LW_TBU_EN > > > I think it'd be nice to have the exact behaviour in the commit > message, but either way this seems OK as a pragmatic fix so: > > Reviewed-by: Brian Starkey > > Thanks, > -Brian > > > u32 __iomem *reg = c->reg; > > > > d71_layer_update_fb(c, kfb, st->addr); > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h > > b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h > > index e80172a0b320..a8036689d721 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h > > @@ -321,6 +321,7 @@ > > #define LAYER_WR_FORMAT0x0D8 > > > > /* Layer_WR control bits */ > > +#define LW_TRC BIT(1) > > #define LW_OFM BIT(4) > > #define LW_LALPHA(x) (((x) & 0xFF) << 8) > > #define LW_A_WCACHE(x) (((x) & 0xF) << 28) > > -- > > 2.30.0 > > Acked-by: James Qian Wang Thanks James > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] dma-buf: Add an API for exporting sync files (v6)
Hi Jason, Thank you for the patch! Yet something to improve: [auto build test ERROR on tegra-drm/drm/tegra/for-next] [also build test ERROR on linus/master v5.12-rc3 next-20210315] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Jason-Ekstrand/dma-buf-add-dma_fence_array_for_each-v2/20210316-060509 base: git://anongit.freedesktop.org/tegra/linux.git drm/tegra/for-next config: parisc-randconfig-s031-20210315 (attached as .config) compiler: hppa-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.3-277-gc089cd2d-dirty # https://github.com/0day-ci/linux/commit/570269f5d3ec3a13936fa2224682d39c8a037126 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Jason-Ekstrand/dma-buf-add-dma_fence_array_for_each-v2/20210316-060509 git checkout 570269f5d3ec3a13936fa2224682d39c8a037126 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=parisc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): hppa-linux-ld: drivers/dma-buf/dma-buf.o: in function `dma_buf_ioctl': >> (.text+0x1944): undefined reference to `sync_file_create' --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[GIT FIXES FOR v5.12] R-Car DU fix
The following changes since commit 4042160c2e5433e0759782c402292a90b5bf458d: drm/nouveau: fix dma syncing for loops (v2) (2021-03-12 11:21:47 +1000) are available in the Git repository at: git://linuxtv.org/pinchartl/media.git tags/du-fixes-20210316 for you to fetch changes up to 7a1adbd2399023177508836c2b13a6c723035409: drm: rcar-du: Use drmm_encoder_alloc() to manage encoder (2021-03-16 03:12:36 +0200) R-Car DU v5.12 fix Kieran Bingham (1): drm: rcar-du: Use drmm_encoder_alloc() to manage encoder drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 31 ++- 1 file changed, 6 insertions(+), 25 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] dma-buf: Add an API for exporting sync files (v6)
I-G-T tests: https://lists.freedesktop.org/archives/igt-dev/2021-March/029825.html On Mon, Mar 15, 2021 at 4:04 PM Jason Ekstrand wrote: > > Modern userspace APIs like Vulkan are built on an explicit > synchronization model. This doesn't always play nicely with the > implicit synchronization used in the kernel and assumed by X11 and > Wayland. The client -> compositor half of the synchronization isn't too > bad, at least on intel, because we can control whether or not i915 > synchronizes on the buffer and whether or not it's considered written. > > The harder part is the compositor -> client synchronization when we get > the buffer back from the compositor. We're required to be able to > provide the client with a VkSemaphore and VkFence representing the point > in time where the window system (compositor and/or display) finished > using the buffer. With current APIs, it's very hard to do this in such > a way that we don't get confused by the Vulkan driver's access of the > buffer. In particular, once we tell the kernel that we're rendering to > the buffer again, any CPU waits on the buffer or GPU dependencies will > wait on some of the client rendering and not just the compositor. > > This new IOCTL solves this problem by allowing us to get a snapshot of > the implicit synchronization state of a given dma-buf in the form of a > sync file. It's effectively the same as a poll() or I915_GEM_WAIT only, > instead of CPU waiting directly, it encapsulates the wait operation, at > the current moment in time, in a sync_file so we can check/wait on it > later. As long as the Vulkan driver does the sync_file export from the > dma-buf before we re-introduce it for rendering, it will only contain > fences from the compositor or display. This allows to accurately turn > it into a VkFence or VkSemaphore without any over- synchronization. > > Mesa MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 > > Cc: Christian König > Cc: Michel Dänzer > Cc: Dave Airlie > Cc: Bas Nieuwenhuizen > Cc: Daniel Stone > > Christian König (2): > dma-buf: add dma_fence_array_for_each (v2) > dma-buf: add dma_resv_get_singleton (v2) > > Jason Ekstrand (1): > dma-buf: Add an API for exporting sync files (v6) > > drivers/dma-buf/dma-buf.c | 55 ++ > drivers/dma-buf/dma-fence-array.c | 27 +++ > drivers/dma-buf/dma-resv.c| 118 ++ > include/linux/dma-fence-array.h | 17 + > include/linux/dma-resv.h | 3 + > include/uapi/linux/dma-buf.h | 6 ++ > 6 files changed, 226 insertions(+) > > -- > 2.29.2 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] dma-buf: Add an API for exporting sync files (v6)
Hi Jason, Thank you for the patch! Yet something to improve: [auto build test ERROR on tegra-drm/drm/tegra/for-next] [also build test ERROR on linus/master v5.12-rc3 next-20210315] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Jason-Ekstrand/dma-buf-add-dma_fence_array_for_each-v2/20210316-060509 base: git://anongit.freedesktop.org/tegra/linux.git drm/tegra/for-next config: xtensa-randconfig-r022-20210315 (attached as .config) compiler: xtensa-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/570269f5d3ec3a13936fa2224682d39c8a037126 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Jason-Ekstrand/dma-buf-add-dma_fence_array_for_each-v2/20210316-060509 git checkout 570269f5d3ec3a13936fa2224682d39c8a037126 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): xtensa-linux-ld: drivers/dma-buf/dma-buf.o: in function `dma_buf_dynamic_attach': >> dma-buf.c:(.text+0xf4c): undefined reference to `sync_file_create' xtensa-linux-ld: drivers/dma-buf/dma-buf.o: in function `dma_buf_unmap_attachment': dma-buf.c:(.text+0x10b0): undefined reference to `sync_file_create' --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] dma-buf: Add an API for exporting sync files (v6)
On Mon, Mar 15, 2021 at 4:05 PM Jason Ekstrand wrote:
>
> Modern userspace APIs like Vulkan are built on an explicit
> synchronization model. This doesn't always play nicely with the
> implicit synchronization used in the kernel and assumed by X11 and
> Wayland. The client -> compositor half of the synchronization isn't too
> bad, at least on intel, because we can control whether or not i915
> synchronizes on the buffer and whether or not it's considered written.
>
> The harder part is the compositor -> client synchronization when we get
> the buffer back from the compositor. We're required to be able to
> provide the client with a VkSemaphore and VkFence representing the point
> in time where the window system (compositor and/or display) finished
> using the buffer. With current APIs, it's very hard to do this in such
> a way that we don't get confused by the Vulkan driver's access of the
> buffer. In particular, once we tell the kernel that we're rendering to
> the buffer again, any CPU waits on the buffer or GPU dependencies will
> wait on some of the client rendering and not just the compositor.
>
> This new IOCTL solves this problem by allowing us to get a snapshot of
> the implicit synchronization state of a given dma-buf in the form of a
> sync file. It's effectively the same as a poll() or I915_GEM_WAIT only,
> instead of CPU waiting directly, it encapsulates the wait operation, at
> the current moment in time, in a sync_file so we can check/wait on it
> later. As long as the Vulkan driver does the sync_file export from the
> dma-buf before we re-introduce it for rendering, it will only contain
> fences from the compositor or display. This allows to accurately turn
> it into a VkFence or VkSemaphore without any over- synchronization.
>
> v2 (Jason Ekstrand):
> - Use a wrapper dma_fence_array of all fences including the new one
>when importing an exclusive fence.
>
> v3 (Jason Ekstrand):
> - Lock around setting shared fences as well as exclusive
> - Mark SIGNAL_SYNC_FILE as a read-write ioctl.
> - Initialize ret to 0 in dma_buf_wait_sync_file
>
> v4 (Jason Ekstrand):
> - Use the new dma_resv_get_singleton helper
>
> v5 (Jason Ekstrand):
> - Rename the IOCTLs to import/export rather than wait/signal
> - Drop the WRITE flag and always get/set the exclusive fence
>
> v6 (Jason Ekstrand):
> - Drop the sync_file import as it was all-around sketchy and not nearly
>as useful as import.
> - Re-introduce READ/WRITE flag support for export
> - Rework the commit message
>
> Signed-off-by: Jason Ekstrand
> ---
> drivers/dma-buf/dma-buf.c| 55
> include/uapi/linux/dma-buf.h | 6
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index f264b70c383eb..e7f9dd62c19a9 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -362,6 +363,57 @@ static long dma_buf_set_name(struct dma_buf *dmabuf,
> const char __user *buf)
> return ret;
> }
>
> +static long dma_buf_export_sync_file(struct dma_buf *dmabuf,
> +void __user *user_data)
> +{
> + struct dma_buf_sync_file arg;
> + struct dma_fence *fence = NULL;
> + struct sync_file *sync_file;
> + int fd, ret;
> +
> + if (copy_from_user(, user_data, sizeof(arg)))
> + return -EFAULT;
> +
> + if (arg.flags & ~DMA_BUF_SYNC_RW)
> + return -EINVAL;
> +
> + fd = get_unused_fd_flags(O_CLOEXEC);
> + if (fd < 0)
> + return fd;
> +
> + if (arg.flags & DMA_BUF_SYNC_WRITE) {
> + ret = dma_resv_get_singleton(dmabuf->resv, NULL, );
> + if (ret)
> + goto err_put_fd;
> + } else if (arg.flags & DMA_BUF_SYNC_READ) {
> + fence = dma_resv_get_excl(dmabuf->resv);
> + }
> +
> + if (!fence)
> + fence = dma_fence_get_stub();
> +
> + sync_file = sync_file_create(fence);
> +
> + dma_fence_put(fence);
> +
> + if (!sync_file) {
> + ret = -EINVAL;
Should this be -EINVAL or -ENOMEM?
> + goto err_put_fd;
> + }
> +
> + fd_install(fd, sync_file->file);
> +
> + arg.fd = fd;
> + if (copy_to_user(user_data, , sizeof(arg)))
> + return -EFAULT;
> +
> + return 0;
> +
> +err_put_fd:
> + put_unused_fd(fd);
> + return ret;
> +}
> +
> static long dma_buf_ioctl(struct file *file,
> unsigned int cmd, unsigned long arg)
> {
> @@ -405,6 +457,9 @@ static long dma_buf_ioctl(struct file *file,
> case DMA_BUF_SET_NAME_B:
> return dma_buf_set_name(dmabuf, (const char __user *)arg);
>
> + case DMA_BUF_IOCTL_EXPORT_SYNC_FILE:
> + return
Re: vmwgfx leaking bo pins?
On 3/15/21 9:38 PM, Daniel Vetter wrote: On Mon, Mar 15, 2021 at 6:57 PM Zack Rusin wrote: On 3/12/21 5:06 AM, Thomas Hellström (Intel) wrote: On 3/12/21 12:02 AM, Zack Rusin wrote: On Mar 11, 2021, at 17:35, Thomas Hellström (Intel) wrote: Hi, Zack On 3/11/21 10:07 PM, Zack Rusin wrote: On Mar 11, 2021, at 05:46, Thomas Hellström (Intel) wrote: Hi, I tried latest drm-fixes today and saw a lot of these: Fallout from ttm rework? Yes, I fixed this in d1a73c641afd2617bd80bce8b71a096fc5b74b7e it was in drm-misc-next in the drm-misc tree for a while but hasn’t been merged for 5.12. z Hmm, yes but doesn't that fix trip the ttm_bo_unpin() dma_resv_assert_held(bo->base.resv)? No, doesn’t seem to. TBH I’m not sure why myself, but it seems to be working fine. With CONFIG_PROVE_LOCKING=y I see this: [7.117145] [drm] FIFO at 0xfe00 size is 8192 kiB [7.117284] [drm] VRAM at 0xe800 size is 131072 kiB [7.117291] INFO: trying to register non-static key. [7.117295] the code is fine but needs lockdep annotation. [7.117298] turning off the locking correctness validator Which will probably mask that dma_resv_assert_held(bo->base.resv) Ah, yes, you're right. After fixing that I do see the dma_resv_assert_held triggered. Technically trivially fixable with ttm_bo_reserve/ttm_bo_unreserve around ttm_bo_unpin but it's a little ugly that some places e.g. vmw_bo_unreference will require ttm_bo_reserve/ttm_bo_unreserve around ttm_bo_unpin but some won't e.g. vmw_mob_destroy won't because its lock is held by ttm_bo_delayed_delete without a very clear indication within the function which is which. It looks like, like Daniel hints below, for the mob pagetable bos since they are pinned and hence not on a LRU list, the parent bo is holding the only reference, which is utilized in vmw_mob_unbind() to make sure the tryreserve always succeeds. (unpin could be called in vmw_mob_unbind for the pagetable bo just after fencing if necessary), and similarly for the other vmwgfx_mob error paths, but in that case one should probably keep the bo pointers in stack variables until you know you don't have to unpin. Then it should be ok to tryreserve around unpinning in the error paths. But it's constructs like that, that really makes me think we shouldn't need to reserve to unpin. Not sure it applies here, but if the refcount is down to 0 we know we're in destruction code and can't race with anything anymore, so maybe we can lift the debug check. Otoh I think at that point we might still be on lru lists, so the rules become rather tricky whether it's really always legal to skip the dma_resv_lock. But we could perhaps figure out something if it's too annoying to have a consistent calling context in drivers. I'm not a huge fan of dropping the requirement from unpin and switching to atomic_t for the pin count, since pin/unpin is an extremely slow path, adding complexity in how we protect stuff for a function that's maybe called 60/s (for page flipping we pin/unpin) doesn't strike me as the right balance. *If* we can protect the bo LRU state with the lru lock instead of with reservation it shuld probably only be a matter of extending the lru lock critical section over a couple of assignments. If we change the bo lru state we'd need to grab the lru lock sooner or later anyway, so I think the added complexity should be minimal. /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] RFC: dma-buf: Add an API for importing and exporting sync files (v5)
On Mon, Mar 15, 2021 at 10:11 PM Jason Ekstrand wrote: > > On Wed, Sep 30, 2020 at 4:55 AM Daniel Vetter wrote: > > > > On Wed, Sep 30, 2020 at 11:39:06AM +0200, Michel Dänzer wrote: > > > On 2020-03-17 10:21 p.m., Jason Ekstrand wrote: > > > > Explicit synchronization is the future. At least, that seems to be what > > > > most userspace APIs are agreeing on at this point. However, most of our > > > > Linux APIs (both userspace and kernel UAPI) are currently built around > > > > implicit synchronization with dma-buf. While work is ongoing to change > > > > many of the userspace APIs and protocols to an explicit synchronization > > > > model, switching over piecemeal is difficult due to the number of > > > > potential components involved. On the kernel side, many drivers use > > > > dma-buf including GPU (3D/compute), display, v4l, and others. In > > > > userspace, we have X11, several Wayland compositors, 3D drivers, compute > > > > drivers (OpenCL etc.), media encode/decode, and the list goes on. > > > > > > > > This patch provides a path forward by allowing userspace to manually > > > > manage the fences attached to a dma-buf. Alternatively, one can think > > > > of this as making dma-buf's implicit synchronization simply a carrier > > > > for an explicit fence. This is accomplished by adding two IOCTLs to > > > > dma-buf for importing and exporting a sync file to/from the dma-buf. > > > > This way a userspace component which is uses explicit synchronization, > > > > such as a Vulkan driver, can manually set the write fence on a buffer > > > > before handing it off to an implicitly synchronized component such as a > > > > Wayland compositor or video encoder. In this way, each of the different > > > > components can be upgraded to an explicit synchronization model one at a > > > > time as long as the userspace pieces connecting them are aware of it and > > > > import/export fences at the right times. > > > > > > > > There is a potential race condition with this API if userspace is not > > > > careful. A typical use case for implicit synchronization is to wait for > > > > the dma-buf to be ready, use it, and then signal it for some other > > > > component. Because a sync_file cannot be created until it is guaranteed > > > > to complete in finite time, userspace can only signal the dma-buf after > > > > it has already submitted the work which uses it to the kernel and has > > > > received a sync_file back. There is no way to atomically submit a > > > > wait-use-signal operation. This is not, however, really a problem with > > > > this API so much as it is a problem with explicit synchronization > > > > itself. The way this is typically handled is to have very explicit > > > > ownership transfer points in the API or protocol which ensure that only > > > > one component is using it at any given time. Both X11 (via the PRESENT > > > > extension) and Wayland provide such ownership transfer points via > > > > explicit present and idle messages. > > > > > > > > The decision was intentionally made in this patch to make the import and > > > > export operations IOCTLs on the dma-buf itself rather than as a DRM > > > > IOCTL. This makes it the import/export operation universal across all > > > > components which use dma-buf including GPU, display, v4l, and others. > > > > It also means that a userspace component can do the import/export > > > > without access to the DRM fd which may be tricky to get in cases where > > > > the client communicates with DRM via a userspace API such as OpenGL or > > > > Vulkan. At a future date we may choose to add direct import/export APIs > > > > to components such as drm_syncobj to avoid allocating a file descriptor > > > > and going through two ioctls. However, that seems to be something of a > > > > micro-optimization as import/export operations are likely to happen at a > > > > rate of a few per frame of rendered or decoded video. > > > > > > > > v2 (Jason Ekstrand): > > > > - Use a wrapper dma_fence_array of all fences including the new one > > > > when importing an exclusive fence. > > > > > > > > v3 (Jason Ekstrand): > > > > - Lock around setting shared fences as well as exclusive > > > > - Mark SIGNAL_SYNC_FILE as a read-write ioctl. > > > > - Initialize ret to 0 in dma_buf_wait_sync_file > > > > > > > > v4 (Jason Ekstrand): > > > > - Use the new dma_resv_get_singleton helper > > > > > > > > v5 (Jason Ekstrand): > > > > - Rename the IOCTLs to import/export rather than wait/signal > > > > - Drop the WRITE flag and always get/set the exclusive fence > > > > > > > > Signed-off-by: Jason Ekstrand > > > > > > What's the status of this? DMA_BUF_IOCTL_EXPORT_SYNC_FILE would be useful > > > for Wayland compositors to wait for client buffers to become ready without > > > being prone to getting delayed by later HW access to them, so it would be > > > nice to merge that at least (if DMA_BUF_IOCTL_IMPORT_SYNC_FILE is still > > > controversial). > > >
Re: [PATCH 3/3] RFC: dma-buf: Add an API for importing and exporting sync files (v5)
On Wed, Sep 30, 2020 at 4:55 AM Daniel Vetter wrote: > > On Wed, Sep 30, 2020 at 11:39:06AM +0200, Michel Dänzer wrote: > > On 2020-03-17 10:21 p.m., Jason Ekstrand wrote: > > > Explicit synchronization is the future. At least, that seems to be what > > > most userspace APIs are agreeing on at this point. However, most of our > > > Linux APIs (both userspace and kernel UAPI) are currently built around > > > implicit synchronization with dma-buf. While work is ongoing to change > > > many of the userspace APIs and protocols to an explicit synchronization > > > model, switching over piecemeal is difficult due to the number of > > > potential components involved. On the kernel side, many drivers use > > > dma-buf including GPU (3D/compute), display, v4l, and others. In > > > userspace, we have X11, several Wayland compositors, 3D drivers, compute > > > drivers (OpenCL etc.), media encode/decode, and the list goes on. > > > > > > This patch provides a path forward by allowing userspace to manually > > > manage the fences attached to a dma-buf. Alternatively, one can think > > > of this as making dma-buf's implicit synchronization simply a carrier > > > for an explicit fence. This is accomplished by adding two IOCTLs to > > > dma-buf for importing and exporting a sync file to/from the dma-buf. > > > This way a userspace component which is uses explicit synchronization, > > > such as a Vulkan driver, can manually set the write fence on a buffer > > > before handing it off to an implicitly synchronized component such as a > > > Wayland compositor or video encoder. In this way, each of the different > > > components can be upgraded to an explicit synchronization model one at a > > > time as long as the userspace pieces connecting them are aware of it and > > > import/export fences at the right times. > > > > > > There is a potential race condition with this API if userspace is not > > > careful. A typical use case for implicit synchronization is to wait for > > > the dma-buf to be ready, use it, and then signal it for some other > > > component. Because a sync_file cannot be created until it is guaranteed > > > to complete in finite time, userspace can only signal the dma-buf after > > > it has already submitted the work which uses it to the kernel and has > > > received a sync_file back. There is no way to atomically submit a > > > wait-use-signal operation. This is not, however, really a problem with > > > this API so much as it is a problem with explicit synchronization > > > itself. The way this is typically handled is to have very explicit > > > ownership transfer points in the API or protocol which ensure that only > > > one component is using it at any given time. Both X11 (via the PRESENT > > > extension) and Wayland provide such ownership transfer points via > > > explicit present and idle messages. > > > > > > The decision was intentionally made in this patch to make the import and > > > export operations IOCTLs on the dma-buf itself rather than as a DRM > > > IOCTL. This makes it the import/export operation universal across all > > > components which use dma-buf including GPU, display, v4l, and others. > > > It also means that a userspace component can do the import/export > > > without access to the DRM fd which may be tricky to get in cases where > > > the client communicates with DRM via a userspace API such as OpenGL or > > > Vulkan. At a future date we may choose to add direct import/export APIs > > > to components such as drm_syncobj to avoid allocating a file descriptor > > > and going through two ioctls. However, that seems to be something of a > > > micro-optimization as import/export operations are likely to happen at a > > > rate of a few per frame of rendered or decoded video. > > > > > > v2 (Jason Ekstrand): > > > - Use a wrapper dma_fence_array of all fences including the new one > > > when importing an exclusive fence. > > > > > > v3 (Jason Ekstrand): > > > - Lock around setting shared fences as well as exclusive > > > - Mark SIGNAL_SYNC_FILE as a read-write ioctl. > > > - Initialize ret to 0 in dma_buf_wait_sync_file > > > > > > v4 (Jason Ekstrand): > > > - Use the new dma_resv_get_singleton helper > > > > > > v5 (Jason Ekstrand): > > > - Rename the IOCTLs to import/export rather than wait/signal > > > - Drop the WRITE flag and always get/set the exclusive fence > > > > > > Signed-off-by: Jason Ekstrand > > > > What's the status of this? DMA_BUF_IOCTL_EXPORT_SYNC_FILE would be useful > > for Wayland compositors to wait for client buffers to become ready without > > being prone to getting delayed by later HW access to them, so it would be > > nice to merge that at least (if DMA_BUF_IOCTL_IMPORT_SYNC_FILE is still > > controversial). > > I think the missing bits are just the usual stuff > - igt testcases > - userspace using the new ioctls > - review of the entire pile > > I don't think there's any fundamental objections aside from "no one ever > pushed
[Bug 212279] AMDGPU doesn’t expose any way of setting the full RGB range [ryzen+ mobile]
https://bugzilla.kernel.org/show_bug.cgi?id=212279 --- Comment #3 from Neil (ledu...@hotmail.com) --- Created attachment 295875 --> https://bugzilla.kernel.org/attachment.cgi?id=295875=edit edited edid with wxedid Got it! I failed at editing the file first because it was a 0 byte file (didn't had my TV connected lol!) So. I followed this video https://www.youtube.com/watch?v=tYYMiX7dlak And basically did the following in my debian sid partition (I will describe it for anyone interested): To find my TV edid $ find /sys/devices/pci*/ -name edid Grab/copy the desired edid file to my home folder $ cp /sys/devices/pci:00/:00:08.1/:05:00.0/drm/card0/card0-HDMI-A-1/edid ~/edid.bin Then edited that file with wxedid to change the following: SPF: Supported features = vsig_format to 0b00 CHD: CEA-861 header = YCbCr 4:2:2 to 0 CHD: CEA-861 header = YCbCr 4:4:4 to 0 VSD: Vendor Specific Data Block = DC_Y444 to 0 Then before saving the file I clicked Options > Recalc checksum (otherwise the system will not use your edited edid!), after that I did saved the file, and copied that new modified edid file to /lib/firmware/edid/edid.bin To use the new edid, I added 'drm_kms_helper.edid_firmware=HDMI-A-1:edid/edid.bin' to /etc/default/grub (to the line GRUB_CMDLINE_LINUX="foo") updated initramfs with 'sudo update-initramfs -u' and then updated grub as well with 'sudo update-grub'. Note: I compiled the latest wxedid from sourceforge, and, 'options > recalc checksum' option is MISSING at least for me. That's why the process was failing for me. Luckily for me debian has a wxedid in their repo that has the recalc checksum option. Now... bit of work ain't it? I don't know if this happens because of my TV, I haven't tried with other monitors/televisions. But a bit of improvement in the matter would be GREAT. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] dma-buf: add dma_resv_get_singleton (v2)
From: Christian König
Add a helper function to get a single fence representing
all fences in a dma_resv object.
This fence is either the only one in the object or all not
signaled fences of the object in a flatted out dma_fence_array.
v2 (Jason Ekstrand):
- Take reference of fences both for creating the dma_fence_array and in
the case where we return one fence.
- Handle the case where dma_resv_get_list() returns NULL
Signed-off-by: Jason Ekstrand
---
drivers/dma-buf/dma-resv.c | 118 +
include/linux/dma-resv.h | 3 +
2 files changed, 121 insertions(+)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index 6ddbeb5dfbf65..1733f1ec86a44 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -33,6 +33,8 @@
*/
#include
+#include
+#include
#include
#include
#include
@@ -49,6 +51,19 @@
* write-side updates.
*/
+/**
+ * dma_fence_deep_dive_for_each - deep dive into the fence containers
+ * @fence: resulting fence
+ * @chain: variable for a dma_fence_chain
+ * @index: index into a dma_fence_array
+ * @head: starting point
+ *
+ * Helper to deep dive into the fence containers for flattening them.
+ */
+#define dma_fence_deep_dive_for_each(fence, chain, index, head)\
+ dma_fence_chain_for_each(chain, head) \
+ dma_fence_array_for_each(fence, index, chain)
+
DEFINE_WD_CLASS(reservation_ww_class);
EXPORT_SYMBOL(reservation_ww_class);
@@ -517,6 +532,109 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
}
EXPORT_SYMBOL_GPL(dma_resv_get_fences_rcu);
+/**
+ * dma_resv_get_singleton - get a single fence for the dma_resv object
+ * @obj: the reservation object
+ * @extra: extra fence to add to the resulting array
+ * @result: resulting dma_fence
+ *
+ * Get a single fence representing all unsignaled fences in the dma_resv object
+ * plus the given extra fence. If we got only one fence return a new
+ * reference to that, otherwise return a dma_fence_array object.
+ *
+ * RETURNS
+ * Returns -NOMEM if allocations fail, zero otherwise.
+ */
+int dma_resv_get_singleton(struct dma_resv *obj, struct dma_fence *extra,
+ struct dma_fence **result)
+{
+ struct dma_resv_list *fobj = dma_resv_get_list(obj);
+ struct dma_fence *excl = dma_resv_get_excl(obj);
+ struct dma_fence *fence, *chain, **fences;
+ struct dma_fence_array *array;
+ unsigned int num_fences, shared_count;
+ unsigned int i, j;
+
+ num_fences = 0;
+ *result = NULL;
+
+ dma_fence_deep_dive_for_each(fence, chain, i, extra) {
+ if (dma_fence_is_signaled(fence))
+ continue;
+
+ *result = fence;
+ ++num_fences;
+ }
+
+ dma_fence_deep_dive_for_each(fence, chain, i, excl) {
+ if (dma_fence_is_signaled(fence))
+ continue;
+
+ *result = fence;
+ ++num_fences;
+ }
+
+ shared_count = fobj ? fobj->shared_count : 0;
+ for (i = 0; i < shared_count; ++i) {
+ struct dma_fence *f;
+
+ f = rcu_dereference_protected(fobj->shared[i],
+ dma_resv_held(obj));
+ dma_fence_deep_dive_for_each(fence, chain, j, f) {
+ if (dma_fence_is_signaled(fence))
+ continue;
+
+ *result = fence;
+ ++num_fences;
+ }
+ }
+
+ if (num_fences <= 1) {
+ *result = dma_fence_get(*result);
+ return 0;
+ }
+
+ fences = kmalloc_array(num_fences, sizeof(struct dma_fence*),
+ GFP_KERNEL);
+ if (!fences)
+ return -ENOMEM;
+
+ num_fences = 0;
+
+ dma_fence_deep_dive_for_each(fence, chain, i, extra)
+ if (!dma_fence_is_signaled(fence))
+ fences[num_fences++] = dma_fence_get(fence);
+
+ dma_fence_deep_dive_for_each(fence, chain, i, excl)
+ if (!dma_fence_is_signaled(fence))
+ fences[num_fences++] = dma_fence_get(fence);
+
+ for (i = 0; i < shared_count; ++i) {
+ struct dma_fence *f;
+
+ f = rcu_dereference_protected(fobj->shared[i],
+ dma_resv_held(obj));
+ dma_fence_deep_dive_for_each(fence, chain, j, f)
+ if (!dma_fence_is_signaled(fence))
+ fences[num_fences++] = dma_fence_get(fence);
+ }
+
+ array = dma_fence_array_create(num_fences, fences,
+ dma_fence_context_alloc(1),
+ 1, false);
+ if (!array)
+ goto error_free;
+
+ *result = >base;
+ return 0;
+
+error_free:
+ while
[PATCH 3/3] dma-buf: Add an API for exporting sync files (v6)
Modern userspace APIs like Vulkan are built on an explicit
synchronization model. This doesn't always play nicely with the
implicit synchronization used in the kernel and assumed by X11 and
Wayland. The client -> compositor half of the synchronization isn't too
bad, at least on intel, because we can control whether or not i915
synchronizes on the buffer and whether or not it's considered written.
The harder part is the compositor -> client synchronization when we get
the buffer back from the compositor. We're required to be able to
provide the client with a VkSemaphore and VkFence representing the point
in time where the window system (compositor and/or display) finished
using the buffer. With current APIs, it's very hard to do this in such
a way that we don't get confused by the Vulkan driver's access of the
buffer. In particular, once we tell the kernel that we're rendering to
the buffer again, any CPU waits on the buffer or GPU dependencies will
wait on some of the client rendering and not just the compositor.
This new IOCTL solves this problem by allowing us to get a snapshot of
the implicit synchronization state of a given dma-buf in the form of a
sync file. It's effectively the same as a poll() or I915_GEM_WAIT only,
instead of CPU waiting directly, it encapsulates the wait operation, at
the current moment in time, in a sync_file so we can check/wait on it
later. As long as the Vulkan driver does the sync_file export from the
dma-buf before we re-introduce it for rendering, it will only contain
fences from the compositor or display. This allows to accurately turn
it into a VkFence or VkSemaphore without any over- synchronization.
v2 (Jason Ekstrand):
- Use a wrapper dma_fence_array of all fences including the new one
when importing an exclusive fence.
v3 (Jason Ekstrand):
- Lock around setting shared fences as well as exclusive
- Mark SIGNAL_SYNC_FILE as a read-write ioctl.
- Initialize ret to 0 in dma_buf_wait_sync_file
v4 (Jason Ekstrand):
- Use the new dma_resv_get_singleton helper
v5 (Jason Ekstrand):
- Rename the IOCTLs to import/export rather than wait/signal
- Drop the WRITE flag and always get/set the exclusive fence
v6 (Jason Ekstrand):
- Drop the sync_file import as it was all-around sketchy and not nearly
as useful as import.
- Re-introduce READ/WRITE flag support for export
- Rework the commit message
Signed-off-by: Jason Ekstrand
---
drivers/dma-buf/dma-buf.c| 55
include/uapi/linux/dma-buf.h | 6
2 files changed, 61 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index f264b70c383eb..e7f9dd62c19a9 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -362,6 +363,57 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const
char __user *buf)
return ret;
}
+static long dma_buf_export_sync_file(struct dma_buf *dmabuf,
+void __user *user_data)
+{
+ struct dma_buf_sync_file arg;
+ struct dma_fence *fence = NULL;
+ struct sync_file *sync_file;
+ int fd, ret;
+
+ if (copy_from_user(, user_data, sizeof(arg)))
+ return -EFAULT;
+
+ if (arg.flags & ~DMA_BUF_SYNC_RW)
+ return -EINVAL;
+
+ fd = get_unused_fd_flags(O_CLOEXEC);
+ if (fd < 0)
+ return fd;
+
+ if (arg.flags & DMA_BUF_SYNC_WRITE) {
+ ret = dma_resv_get_singleton(dmabuf->resv, NULL, );
+ if (ret)
+ goto err_put_fd;
+ } else if (arg.flags & DMA_BUF_SYNC_READ) {
+ fence = dma_resv_get_excl(dmabuf->resv);
+ }
+
+ if (!fence)
+ fence = dma_fence_get_stub();
+
+ sync_file = sync_file_create(fence);
+
+ dma_fence_put(fence);
+
+ if (!sync_file) {
+ ret = -EINVAL;
+ goto err_put_fd;
+ }
+
+ fd_install(fd, sync_file->file);
+
+ arg.fd = fd;
+ if (copy_to_user(user_data, , sizeof(arg)))
+ return -EFAULT;
+
+ return 0;
+
+err_put_fd:
+ put_unused_fd(fd);
+ return ret;
+}
+
static long dma_buf_ioctl(struct file *file,
unsigned int cmd, unsigned long arg)
{
@@ -405,6 +457,9 @@ static long dma_buf_ioctl(struct file *file,
case DMA_BUF_SET_NAME_B:
return dma_buf_set_name(dmabuf, (const char __user *)arg);
+ case DMA_BUF_IOCTL_EXPORT_SYNC_FILE:
+ return dma_buf_export_sync_file(dmabuf, (void __user *)arg);
+
default:
return -ENOTTY;
}
diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h
index 7f30393b92c3b..9bce1e8bd31d3 100644
--- a/include/uapi/linux/dma-buf.h
+++ b/include/uapi/linux/dma-buf.h
@@ -37,6 +37,11 @@ struct dma_buf_sync {
#define DMA_BUF_NAME_LEN
[PATCH 1/3] dma-buf: add dma_fence_array_for_each (v2)
From: Christian König
Add a helper to iterate over all fences in a dma_fence_array object.
v2 (Jason Ekstrand)
- Return NULL from dma_fence_array_first if head == NULL. This matches
the iterator behavior of dma_fence_chain_for_each in that it iterates
zero times if head == NULL.
- Return NULL from dma_fence_array_next if index > array->num_fences.
Signed-off-by: Jason Ekstrand
---
drivers/dma-buf/dma-fence-array.c | 27 +++
include/linux/dma-fence-array.h | 17 +
2 files changed, 44 insertions(+)
diff --git a/drivers/dma-buf/dma-fence-array.c
b/drivers/dma-buf/dma-fence-array.c
index d3fbd950be944..2ac1afc697d0f 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -201,3 +201,30 @@ bool dma_fence_match_context(struct dma_fence *fence, u64
context)
return true;
}
EXPORT_SYMBOL(dma_fence_match_context);
+
+struct dma_fence *dma_fence_array_first(struct dma_fence *head)
+{
+ struct dma_fence_array *array;
+
+ if (!head)
+ return NULL;
+
+ array = to_dma_fence_array(head);
+ if (!array)
+ return head;
+
+ return array->fences[0];
+}
+EXPORT_SYMBOL(dma_fence_array_first);
+
+struct dma_fence *dma_fence_array_next(struct dma_fence *head,
+ unsigned int index)
+{
+ struct dma_fence_array *array = to_dma_fence_array(head);
+
+ if (!array || index >= array->num_fences)
+ return NULL;
+
+ return array->fences[index];
+}
+EXPORT_SYMBOL(dma_fence_array_next);
diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h
index 303dd712220fd..588ac8089dd61 100644
--- a/include/linux/dma-fence-array.h
+++ b/include/linux/dma-fence-array.h
@@ -74,6 +74,19 @@ to_dma_fence_array(struct dma_fence *fence)
return container_of(fence, struct dma_fence_array, base);
}
+/**
+ * dma_fence_array_for_each - iterate over all fences in array
+ * @fence: current fence
+ * @index: index into the array
+ * @head: potential dma_fence_array object
+ *
+ * Test if @array is a dma_fence_array object and if yes iterate over all
fences
+ * in the array. If not just iterate over the fence in @array itself.
+ */
+#define dma_fence_array_for_each(fence, index, head) \
+ for (index = 0, fence = dma_fence_array_first(head); fence; \
+++(index), fence = dma_fence_array_next(head, index))
+
struct dma_fence_array *dma_fence_array_create(int num_fences,
struct dma_fence **fences,
u64 context, unsigned seqno,
@@ -81,4 +94,8 @@ struct dma_fence_array *dma_fence_array_create(int num_fences,
bool dma_fence_match_context(struct dma_fence *fence, u64 context);
+struct dma_fence *dma_fence_array_first(struct dma_fence *head);
+struct dma_fence *dma_fence_array_next(struct dma_fence *head,
+ unsigned int index);
+
#endif /* __LINUX_DMA_FENCE_ARRAY_H */
--
2.29.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/3] dma-buf: Add an API for exporting sync files (v6)
Modern userspace APIs like Vulkan are built on an explicit synchronization model. This doesn't always play nicely with the implicit synchronization used in the kernel and assumed by X11 and Wayland. The client -> compositor half of the synchronization isn't too bad, at least on intel, because we can control whether or not i915 synchronizes on the buffer and whether or not it's considered written. The harder part is the compositor -> client synchronization when we get the buffer back from the compositor. We're required to be able to provide the client with a VkSemaphore and VkFence representing the point in time where the window system (compositor and/or display) finished using the buffer. With current APIs, it's very hard to do this in such a way that we don't get confused by the Vulkan driver's access of the buffer. In particular, once we tell the kernel that we're rendering to the buffer again, any CPU waits on the buffer or GPU dependencies will wait on some of the client rendering and not just the compositor. This new IOCTL solves this problem by allowing us to get a snapshot of the implicit synchronization state of a given dma-buf in the form of a sync file. It's effectively the same as a poll() or I915_GEM_WAIT only, instead of CPU waiting directly, it encapsulates the wait operation, at the current moment in time, in a sync_file so we can check/wait on it later. As long as the Vulkan driver does the sync_file export from the dma-buf before we re-introduce it for rendering, it will only contain fences from the compositor or display. This allows to accurately turn it into a VkFence or VkSemaphore without any over- synchronization. Mesa MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 Cc: Christian König Cc: Michel Dänzer Cc: Dave Airlie Cc: Bas Nieuwenhuizen Cc: Daniel Stone Christian König (2): dma-buf: add dma_fence_array_for_each (v2) dma-buf: add dma_resv_get_singleton (v2) Jason Ekstrand (1): dma-buf: Add an API for exporting sync files (v6) drivers/dma-buf/dma-buf.c | 55 ++ drivers/dma-buf/dma-fence-array.c | 27 +++ drivers/dma-buf/dma-resv.c| 118 ++ include/linux/dma-fence-array.h | 17 + include/linux/dma-resv.h | 3 + include/uapi/linux/dma-buf.h | 6 ++ 6 files changed, 226 insertions(+) -- 2.29.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: vmwgfx leaking bo pins?
On Mon, Mar 15, 2021 at 6:57 PM Zack Rusin wrote: > > On 3/12/21 5:06 AM, Thomas Hellström (Intel) wrote: > > > > On 3/12/21 12:02 AM, Zack Rusin wrote: > >> > >>> On Mar 11, 2021, at 17:35, Thomas Hellström (Intel) > >>> wrote: > >>> > >>> Hi, Zack > >>> > >>> On 3/11/21 10:07 PM, Zack Rusin wrote: > > On Mar 11, 2021, at 05:46, Thomas Hellström (Intel) > > wrote: > > > > Hi, > > > > I tried latest drm-fixes today and saw a lot of these: Fallout from > > ttm rework? > Yes, I fixed this in d1a73c641afd2617bd80bce8b71a096fc5b74b7e it was > in drm-misc-next in the drm-misc tree for a while but hasn’t been > merged for 5.12. > > z > > >>> Hmm, yes but doesn't that fix trip the ttm_bo_unpin() > >>> dma_resv_assert_held(bo->base.resv)? > >> No, doesn’t seem to. TBH I’m not sure why myself, but it seems to be > >> working fine. > >> > >> > > With CONFIG_PROVE_LOCKING=y I see this: > > > > [7.117145] [drm] FIFO at 0xfe00 size is 8192 kiB > > [7.117284] [drm] VRAM at 0xe800 size is 131072 kiB > > [7.117291] INFO: trying to register non-static key. > > [7.117295] the code is fine but needs lockdep annotation. > > [7.117298] turning off the locking correctness validator > > > > Which will probably mask that dma_resv_assert_held(bo->base.resv) > > > > Ah, yes, you're right. After fixing that I do see the > dma_resv_assert_held triggered. Technically trivially fixable with > ttm_bo_reserve/ttm_bo_unreserve around ttm_bo_unpin but it's a little > ugly that some places e.g. vmw_bo_unreference will require > ttm_bo_reserve/ttm_bo_unreserve around ttm_bo_unpin but some won't e.g. > vmw_mob_destroy won't because its lock is held by ttm_bo_delayed_delete > without a very clear indication within the function which is which. Not sure it applies here, but if the refcount is down to 0 we know we're in destruction code and can't race with anything anymore, so maybe we can lift the debug check. Otoh I think at that point we might still be on lru lists, so the rules become rather tricky whether it's really always legal to skip the dma_resv_lock. But we could perhaps figure out something if it's too annoying to have a consistent calling context in drivers. I'm not a huge fan of dropping the requirement from unpin and switching to atomic_t for the pin count, since pin/unpin is an extremely slow path, adding complexity in how we protect stuff for a function that's maybe called 60/s (for page flipping we pin/unpin) doesn't strike me as the right balance. And atomic commit helpers are explicitly designed to allow drivers to grab dma_resv_lock in their ->cleanup_fb hook, since that part is _not_ in the dma_fence signalling critical path for the atomic flip. But if all else fails I guess that's an option too. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/ttm: switch to per device LRU lock
Hi "Christian,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next
tegra-drm/drm/tegra/for-next linus/master drm/drm-next v5.12-rc3 next-20210315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Christian-K-nig/drm-ttm-move-swapout-logic-around/20210316-000551
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-m001-20210315 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
smatch warnings:
drivers/gpu/drm/ttm/ttm_device.c:158 ttm_device_swapout() warn: inconsistent
returns '>lru_lock'.
drivers/gpu/drm/ttm/ttm_bo.c:665 ttm_mem_evict_first() error: we previously
assumed 'bo' could be null (see line 662)
vim +158 drivers/gpu/drm/ttm/ttm_device.c
70ae63f3a85b97 Christian König 2021-03-15 123
70ae63f3a85b97 Christian König 2021-03-15 124 long ttm_device_swapout(struct
ttm_device *bdev, struct ttm_operation_ctx *ctx,
70ae63f3a85b97 Christian König 2021-03-15 125 gfp_t
gfp_flags)
70ae63f3a85b97 Christian König 2021-03-15 126 {
70ae63f3a85b97 Christian König 2021-03-15 127 struct
ttm_resource_manager *man;
824dca26fe3958 Christian König 2021-03-15 128 struct
ttm_buffer_object *bo;
70ae63f3a85b97 Christian König 2021-03-15 129 unsigned i, j;
824dca26fe3958 Christian König 2021-03-15 130 int ret;
824dca26fe3958 Christian König 2021-03-15 131
1ed8d8fc515b90 Christian König 2021-03-15 132
spin_lock(>lru_lock);
70ae63f3a85b97 Christian König 2021-03-15 133 for (i = TTM_PL_SYSTEM;
i < TTM_NUM_MEM_TYPES; ++i) {
70ae63f3a85b97 Christian König 2021-03-15 134 man =
ttm_manager_type(bdev, i);
70ae63f3a85b97 Christian König 2021-03-15 135 if (!man ||
!man->use_tt)
70ae63f3a85b97 Christian König 2021-03-15 136
continue;
70ae63f3a85b97 Christian König 2021-03-15 137
70ae63f3a85b97 Christian König 2021-03-15 138 for (j = 0; j <
TTM_MAX_BO_PRIORITY; ++j) {
70ae63f3a85b97 Christian König 2021-03-15 139
list_for_each_entry(bo, >lru[j], lru) {
70ae63f3a85b97 Christian König 2021-03-15 140
long num_pages;
824dca26fe3958 Christian König 2021-03-15 141
70ae63f3a85b97 Christian König 2021-03-15 142
if (!bo->ttm ||
70ae63f3a85b97 Christian König 2021-03-15 143
bo->ttm->page_flags & TTM_PAGE_FLAG_SG ||
70ae63f3a85b97 Christian König 2021-03-15 144
bo->ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)
70ae63f3a85b97 Christian König 2021-03-15 145
continue;
70ae63f3a85b97 Christian König 2021-03-15 146
70ae63f3a85b97 Christian König 2021-03-15 147
num_pages = bo->ttm->num_pages;
824dca26fe3958 Christian König 2021-03-15 148
ret = ttm_bo_swapout(bo, ctx, gfp_flags);
824dca26fe3958 Christian König 2021-03-15 149
/* ttm_bo_swapout has dropped the lru_lock */
824dca26fe3958 Christian König 2021-03-15 150
if (!ret)
824dca26fe3958 Christian König 2021-03-15 151
return num_pages;
824dca26fe3958 Christian König 2021-03-15 152
if (ret != -EBUSY)
824dca26fe3958 Christian König 2021-03-15 153
return ret;
824dca26fe3958 Christian König 2021-03-15 154 }
824dca26fe3958 Christian König 2021-03-15 155 }
70ae63f3a85b97 Christian König 2021-03-15 156 }
1ed8d8fc515b90 Christian König 2021-03-15 157
spin_unlock(>lru_lock);
824dca26fe3958 Christian König 2021-03-15 @158 return 0;
824dca26fe3958 Christian König 2021-03-15 159 }
70ae63f3a85b97 Christian König 2021-03-15 160
EXPORT_SYMBOL(ttm_device_swapout);
824dca26fe3958 Christian König 2021-03-15 161
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v8 3/3] drm: Add GUD USB Display driver
Hi Noralf,
super fair call with the BE testing, let's hope for some testing soonish.
I was thinking about my device doing protocol STALL when I try to
return 0 bytes, and while it *is* a bug in my device, from a standards
point of view it's actually completely valid, if not expected:
--8<-- usb_20.pdf 8.5.3.4 STALL Handshakes Returned by Control Pipes
If the device is unable to complete a command, it returns a STALL in the
Data and/or Status stages of the control transfer. Unlike the case of a
functional stall, protocol stall does not indicate an error with the device.
-->8--
I think it's fair to say that a device can't complete the command
when it has no data to return.
So how about allowing STALL for optional GUD_REQ_GET_:s to mean the same
as a 0 byte response? Should I propose a separate patch for it later?
Noralf Trønnes wrote:
> +++ b/drivers/gpu/drm/gud/gud_connector.c
..
> +static int gud_connector_get_modes(struct drm_connector *connector)
..
> + ret = gud_usb_get(gdrm, GUD_REQ_GET_CONNECTOR_EDID, connector->index,
> + edid_ctx.buf, GUD_CONNECTOR_MAX_EDID_LEN);
if (ret == -EPIPE)
ret = 0;
> + if (ret > 0 && ret % EDID_LENGTH) {
> + gud_conn_err(connector, "Invalid EDID size", ret);
> + } else if (ret > 0) {
> + edid_ctx.len = ret;
> + edid = drm_do_get_edid(connector, gud_connector_get_edid_block,
> _ctx);
> + }
> +static int gud_connector_add_properties(struct gud_device *gdrm, struct
> gud_connector *gconn)
..
> + ret = gud_usb_get(gdrm, GUD_REQ_GET_CONNECTOR_PROPERTIES,
> connector->index,
> + properties, GUD_CONNECTOR_PROPERTIES_MAX_NUM *
> sizeof(*properties));
if (ret == -EPIPE)
ret = 0;
> + if (ret <= 0)
> + goto out;
> + if (ret % sizeof(*properties)) {
> + ret = -EIO;
> + goto out;
> + }
> +++ b/drivers/gpu/drm/gud/gud_drv.c
..
..
> +static int gud_get_properties(struct gud_device *gdrm)
..
> + ret = gud_usb_get(gdrm, GUD_REQ_GET_PROPERTIES, 0,
> + properties, GUD_PROPERTIES_MAX_NUM *
> sizeof(*properties));
if (ret == -EPIPE)
ret = 0;
> + if (ret <= 0)
> + goto out;
> + if (ret % sizeof(*properties)) {
> + ret = -EIO;
> + goto out;
> + }
Then I looked whether a device could cause trouble in the driver by
returning complex/unexpected data, and found this:
> +static int gud_probe(struct usb_interface *intf, const struct usb_device_id
> *id)
..
> + /* Add room for emulated XRGB */
> + formats = devm_kmalloc_array(dev, GUD_FORMATS_MAX_NUM + 1,
> sizeof(*formats), GFP_KERNEL);
It looks like this +1 and the way xrgb_emulation_format works means
that an interface will not always work correctly if multiple emulated
formats (R1, XRGB, RGB565) are returned, because only one emulated
mode is added after the loop, with struct drm_format_info for the last
emulated format returned by the device. So userspace would only see the
last emulated mode and the bulk output would only ever use that
particular pixel format, any earlier ones would be unavailable?
If this is EWONTFIX then how about adding an error message if multiple
emulated modes are returned and ignore all but the first, rather than
all but the last?
Related: Can userspace find out which GUD_PIXEL_FORMAT_* is behind an
emulated format? It's needed to decide how the emulated framebuffer
should be used, in particular to not use G or B if GUD_PIXEL_FORMAT_R1.
> + switch (format) {
> + case GUD_DRM_FORMAT_R1:
> + fallthrough;
> + case GUD_DRM_FORMAT_XRGB:
> + if (!xrgb_emulation_format)
> + xrgb_emulation_format = info;
> + break;
> + case DRM_FORMAT_RGB565:
> + rgb565_supported = true;
> + if (!xrgb_emulation_format)
> + xrgb_emulation_format = info;
> + break;
Could RGB565 go before XRGB111 (or R1) and also fallthrough; in this
construct? Not terribly important, but the repetition caught my eye.
Then, in gud_connector.c I saw this, which surprised me:
+int gud_connector_fill_properties(struct drm_connector_state *connector_state,
..
+ if (prop == GUD_PROPERTY_BACKLIGHT_BRIGHTNESS) {
+ val = connector_state->tv.brightness;
+ } else {
Why is this using tv.brightness rather than say gconn->initial_brightness?
It looks like the end result might be the same because tv.brightness is
set to gconn->initial_brightness in gud_connector_reset() but it's a
little confusing to me, since a GUD backlight isn't a drm/TV thing?
Thanks a lot
//Peter
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
Re: [PATCH 2/3] drm/ttm: remove swap LRU v2
Am 15.03.21 um 19:54 schrieb Matthew Auld:
On Mon, 15 Mar 2021 at 16:04, Christian König
wrote:
[SNIP]
@@ -1193,6 +1164,10 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct
ttm_operation_ctx *ctx,
bool locked;
int ret;
+ if (!bo->ttm || bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
+ TTM_PAGE_FLAG_SWAPPED))
+ return false;
+
return 0; ?
Seems inconsistent to return zero here and not drop the lru lock? Or
maybe turn this into a programmer error, since the current caller
already checks for the above?
Thanks, that is just an artifact from rebasing and should be removed.
[SNIP]
@@ -109,27 +106,60 @@ static int ttm_global_init(void)
long ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags)
{
struct ttm_global *glob = _glob;
+ struct ttm_device *bdev;
+ int ret = -EBUSY;
+
+ mutex_lock(_global_mutex);
+ list_for_each_entry(bdev, >device_list, device_list) {
+ ret = ttm_device_swapout(bdev, ctx, gfp_flags);
Mixing int and long for num_pages.
Does ttm enforce a maximum page count somewhere for object sizes?
We should use 32 bit values for the number of pages in TTM, even signed
values allow for 8TB large BOs.
And I really hope that we can get rid of the BO approach in general
before we ever come close to that limit.
Something like INT_MAX, since it doesn't look like ttm is consistently
using the same type(unsigned long?) when representing the number of
pages for an object?
I should probably add a check for that in the tt code, yes.
[SNIP]
static void ttm_init_sysman(struct ttm_device *bdev)
{
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index b991422e156c..0e82b0662d9e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1371,7 +1371,7 @@ static int vmw_pm_freeze(struct device *kdev)
vmw_execbuf_release_pinned_bo(dev_priv);
vmw_resource_evict_all(dev_priv);
vmw_release_device_early(dev_priv);
- while (ttm_global_swapout(, GFP_KERNEL) > 0);
+ while (ttm_device_swapout(_priv->bdev, , GFP_KERNEL) == 0);
Is this the intended behaviour? ttm_device_swapout() still just
returns num_pages if it swapped something out. I assume this wants to
keep swapping stuff out, until it can't anymore. Or am I missing
something?
Indeed that's a mix up. Thanks for pointing that out.
Christian.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/vmwgfx: clean up vmw_move_notify v2
Instead of swapping bo->mem just give old and new as parameters.
Also drop unused parameters and code.
v2: cleanup stale documentation as well.
Signed-off-by: Christian König
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h| 3 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 +---
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 27 +++---
3 files changed, 16 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index bb2ce6327944..7e6518709e14 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -775,7 +775,8 @@ extern void vmw_resource_unreserve(struct vmw_resource *res,
struct vmw_buffer_object *new_backup,
unsigned long new_backup_offset);
extern void vmw_query_move_notify(struct ttm_buffer_object *bo,
- struct ttm_resource *mem);
+ struct ttm_resource *old_mem,
+ struct ttm_resource *new_mem);
extern int vmw_query_readback_all(struct vmw_buffer_object *dx_query_mob);
extern void vmw_resource_evict_all(struct vmw_private *dev_priv);
extern void vmw_resource_unbind_list(struct vmw_buffer_object *vbo);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index c3a724e37104..35f02958ee2c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -847,13 +847,15 @@ int vmw_query_readback_all(struct vmw_buffer_object
*dx_query_mob)
* vmw_query_move_notify - Read back cached query states
*
* @bo: The TTM buffer object about to move.
- * @mem: The memory region @bo is moving to.
+ * @old_mem: The memory region @bo is moving from.
+ * @new_mem: The memory region @bo is moving to.
*
* Called before the query MOB is swapped out to read back cached query
* states from the device.
*/
void vmw_query_move_notify(struct ttm_buffer_object *bo,
- struct ttm_resource *mem)
+ struct ttm_resource *old_mem,
+ struct ttm_resource *new_mem)
{
struct vmw_buffer_object *dx_query_mob;
struct ttm_device *bdev = bo->bdev;
@@ -871,7 +873,8 @@ void vmw_query_move_notify(struct ttm_buffer_object *bo,
}
/* If BO is being moved from MOB to system memory */
- if (mem->mem_type == TTM_PL_SYSTEM && bo->mem.mem_type == VMW_PL_MOB) {
+ if (new_mem->mem_type == TTM_PL_SYSTEM &&
+ old_mem->mem_type == VMW_PL_MOB) {
struct vmw_fence_obj *fence;
(void) vmw_query_readback_all(dx_query_mob);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 63f10c865061..2dc031fe4a90 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -691,21 +691,19 @@ static int vmw_ttm_io_mem_reserve(struct ttm_device
*bdev, struct ttm_resource *
* vmw_move_notify - TTM move_notify_callback
*
* @bo: The TTM buffer object about to move.
- * @evict: Unused
- * @mem: The struct ttm_resource indicating to what memory
+ * @old_mem: The old memory where we move from
+ * @new_mem: The struct ttm_resource indicating to what memory
* region the move is taking place.
*
* Calls move_notify for all subsystems needing it.
* (currently only resources).
*/
static void vmw_move_notify(struct ttm_buffer_object *bo,
- bool evict,
- struct ttm_resource *mem)
+ struct ttm_resource *old_mem,
+ struct ttm_resource *new_mem)
{
- if (!mem)
- return;
- vmw_bo_move_notify(bo, mem);
- vmw_query_move_notify(bo, mem);
+ vmw_bo_move_notify(bo, new_mem);
+ vmw_query_move_notify(bo, old_mem, new_mem);
}
@@ -736,7 +734,7 @@ static int vmw_move(struct ttm_buffer_object *bo,
return ret;
}
- vmw_move_notify(bo, evict, new_mem);
+ vmw_move_notify(bo, >mem, new_mem);
if (old_man->use_tt && new_man->use_tt) {
if (bo->mem.mem_type == TTM_PL_SYSTEM) {
@@ -758,18 +756,10 @@ static int vmw_move(struct ttm_buffer_object *bo,
}
return 0;
fail:
- swap(*new_mem, bo->mem);
- vmw_move_notify(bo, false, new_mem);
- swap(*new_mem, bo->mem);
+ vmw_move_notify(bo, new_mem, >mem);
return ret;
}
-static void
-vmw_delete_mem_notify(struct ttm_buffer_object *bo)
-{
- vmw_move_notify(bo, false, NULL);
-}
-
struct ttm_device_funcs vmw_bo_driver = {
.ttm_tt_create = _ttm_tt_create,
.ttm_tt_populate = _ttm_populate,
@@ -779,7 +769,6 @@ struct ttm_device_funcs vmw_bo_driver = {
.evict_flags = vmw_evict_flags,
.move = vmw_move,
[PATCH 1/3] drm/qxl: clean up qxl_bo_move_notify
Remove the unused evict parameter and drop swapping bo->mem.
Signed-off-by: Christian König
---
drivers/gpu/drm/qxl/qxl_ttm.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index b7f77eb685cb..47afe95d04a1 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -121,7 +121,6 @@ static struct ttm_tt *qxl_ttm_tt_create(struct
ttm_buffer_object *bo,
}
static void qxl_bo_move_notify(struct ttm_buffer_object *bo,
- bool evict,
struct ttm_resource *new_mem)
{
struct qxl_bo *qbo;
@@ -144,29 +143,22 @@ static int qxl_bo_move(struct ttm_buffer_object *bo, bool
evict,
struct ttm_resource *old_mem = >mem;
int ret;
- qxl_bo_move_notify(bo, evict, new_mem);
+ qxl_bo_move_notify(bo, new_mem);
ret = ttm_bo_wait_ctx(bo, ctx);
if (ret)
- goto out;
+ return ret;
if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) {
ttm_bo_move_null(bo, new_mem);
return 0;
}
- ret = ttm_bo_move_memcpy(bo, ctx, new_mem);
-out:
- if (ret) {
- swap(*new_mem, bo->mem);
- qxl_bo_move_notify(bo, false, new_mem);
- swap(*new_mem, bo->mem);
- }
- return ret;
+ return ttm_bo_move_memcpy(bo, ctx, new_mem);
}
static void qxl_bo_delete_mem_notify(struct ttm_buffer_object *bo)
{
- qxl_bo_move_notify(bo, false, NULL);
+ qxl_bo_move_notify(bo, NULL);
}
static struct ttm_device_funcs qxl_bo_driver = {
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/nouveau: clean up nouveau_bo_move_ntfy
Just another leftover from a TTM cleanup.
Signed-off-by: Christian König
---
drivers/gpu/drm/nouveau/nouveau_bo.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index ca2a8ae1938e..9bb8cee3df40 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -861,9 +861,8 @@ nouveau_bo_move_init(struct nouveau_drm *drm)
NV_INFO(drm, "MM: using %s for buffer copies\n", name);
}
-static void
-nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool evict,
-struct ttm_resource *new_reg)
+static void nouveau_bo_move_ntfy(struct ttm_buffer_object *bo,
+struct ttm_resource *new_reg)
{
struct nouveau_mem *mem = new_reg ? nouveau_mem(new_reg) : NULL;
struct nouveau_bo *nvbo = nouveau_bo(bo);
@@ -949,7 +948,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
return ret;
}
- nouveau_bo_move_ntfy(bo, evict, new_reg);
+ nouveau_bo_move_ntfy(bo, new_reg);
ret = ttm_bo_wait_ctx(bo, ctx);
if (ret)
goto out_ntfy;
@@ -1014,9 +1013,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
}
out_ntfy:
if (ret) {
- swap(*new_reg, bo->mem);
- nouveau_bo_move_ntfy(bo, false, new_reg);
- swap(*new_reg, bo->mem);
+ nouveau_bo_move_ntfy(bo, >mem);
}
return ret;
}
@@ -1290,7 +1287,7 @@ nouveau_bo_fence(struct nouveau_bo *nvbo, struct
nouveau_fence *fence, bool excl
static void
nouveau_bo_delete_mem_notify(struct ttm_buffer_object *bo)
{
- nouveau_bo_move_ntfy(bo, false, NULL);
+ nouveau_bo_move_ntfy(bo, NULL);
}
struct ttm_device_funcs nouveau_bo_driver = {
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[drm-intel:topic/core-for-CI 20/30] include/linux/kconfig.h:7:10: fatal error: generated/autoconf.h: No such file or directory
tree: git://anongit.freedesktop.org/drm-intel topic/core-for-CI head: 785b066b32b630e4735ca6e51acbb067703e2d3a commit: 98a674f9cd22c2cf6f507ccc1f26fae3c91d1271 [20/30] Revert "drm/i915: Don't select BROKEN" config: alpha-randconfig-r033-20210315 (attached as .config) compiler: alpha-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git remote add drm-intel git://anongit.freedesktop.org/drm-intel git fetch --no-tags drm-intel topic/core-for-CI git checkout 98a674f9cd22c2cf6f507ccc1f26fae3c91d1271 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): In file included from scripts/selinux/mdp/mdp.c:22: >> include/linux/kconfig.h:7:10: fatal error: generated/autoconf.h: No such >> file or directory 7 | #include | ^~ compilation terminated. -- Makefile arch include kernel scripts source usr [scripts/kconfig/Makefile:63: syncconfig] Error 1 Makefile arch include kernel scripts source usr [Makefile:600: syncconfig] Error 2 Makefile arch include kernel scripts source usr [Makefile:709: include/config/auto.conf.cmd] Error 2 Failed to remake makefile 'include/config/auto.conf.cmd'. Failed to remake makefile 'include/config/auto.conf'. In file included from scripts/selinux/mdp/mdp.c:22: >> include/linux/kconfig.h:7:10: fatal error: generated/autoconf.h: No such >> file or directory 7 | #include | ^~ compilation terminated. Makefile arch include kernel scripts source usr [scripts/Makefile.host:95: scripts/selinux/mdp/mdp] Error 1 Target '__build' not remade because of errors. Makefile arch include kernel scripts source usr [scripts/Makefile.build:514: scripts/selinux/mdp] Error 2 Target '__build' not remade because of errors. Makefile arch include kernel scripts source usr [scripts/Makefile.build:514: scripts/selinux] Error 2 Target '__build' not remade because of errors. Makefile arch include kernel scripts source usr [Makefile:1217: scripts] Error 2 Target 'modules_prepare' not remade because of errors. make: Makefile arch include kernel scripts source usr [Makefile:215: __sub-make] Error 2 make: Target 'modules_prepare' not remade because of errors. -- Makefile arch include kernel scripts source usr [scripts/kconfig/Makefile:63: syncconfig] Error 1 Makefile arch include kernel scripts source usr [Makefile:600: syncconfig] Error 2 Makefile arch include kernel scripts source usr [Makefile:709: include/config/auto.conf.cmd] Error 2 Failed to remake makefile 'include/config/auto.conf.cmd'. Failed to remake makefile 'include/config/auto.conf'. In file included from scripts/selinux/mdp/mdp.c:22: >> include/linux/kconfig.h:7:10: fatal error: generated/autoconf.h: No such >> file or directory 7 | #include | ^~ compilation terminated. Makefile arch include kernel scripts source usr [scripts/Makefile.host:95: scripts/selinux/mdp/mdp] Error 1 Target '__build' not remade because of errors. Makefile arch include kernel scripts source usr [scripts/Makefile.build:514: scripts/selinux/mdp] Error 2 Target '__build' not remade because of errors. Makefile arch include kernel scripts source usr [scripts/Makefile.build:514: scripts/selinux] Error 2 Target '__build' not remade because of errors. Makefile arch include kernel scripts source usr [Makefile:1217: scripts] Error 2 Target 'prepare' not remade because of errors. make: Makefile arch include kernel scripts source usr [Makefile:215: __sub-make] Error 2 make: Target 'prepare' not remade because of errors. vim +7 include/linux/kconfig.h 8b59cd81dc5e72 Masahiro Yamada 2020-04-23 6 2a11c8ea20bf85 Michal Marek2011-07-20 @7 #include 2a11c8ea20bf85 Michal Marek2011-07-20 8 :: The code at line 7 was first introduced by commit :: 2a11c8ea20bf850b3a2c60db8c2e7497d28aba99 kconfig: Introduce IS_ENABLED(), IS_BUILTIN() and IS_MODULE() :: TO: Michal Marek :: CC: Michal Marek --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: make ttm_bo_unpin more defensive
On 3/15/21 7:47 PM, Christian König wrote:
Am 15.03.21 um 18:08 schrieb Thomas Hellström (Intel):
On 3/15/21 11:26 AM, Christian König wrote:
Am 13.03.21 um 19:29 schrieb Thomas Hellström (Intel):
Hi, Christian
On 3/12/21 10:38 AM, Christian König wrote:
We seem to have some more driver bugs than thought.
Signed-off-by: Christian König
---
include/drm/ttm/ttm_bo_api.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/drm/ttm/ttm_bo_api.h
b/include/drm/ttm/ttm_bo_api.h
index 4fb523dfab32..df9fe596e7c5 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -603,9 +603,11 @@ static inline void ttm_bo_pin(struct
ttm_buffer_object *bo)
static inline void ttm_bo_unpin(struct ttm_buffer_object *bo)
{
dma_resv_assert_held(bo->base.resv);
- WARN_ON_ONCE(!bo->pin_count);
WARN_ON_ONCE(!kref_read(>kref));
- --bo->pin_count;
+ if (bo->pin_count)
+ --bo->pin_count;
+ else
+ WARN_ON_ONCE(true);
}
int ttm_mem_evict_first(struct ttm_device *bdev,
Since I now have been staring for half a year at the code of the
driver that made pinning an art, I have a couple of suggestions
here, Could we use an atomic for pin_count, allowing unlocked
unpinning but require the lock only for pin_count transition 0->1,
(but unlocked for pin_if_already_pinned). In particular I think
vmwgfx would benefit from unlocked unpins. Also if the atomic were
a refcount_t, that would probably give you the above behaviour?
Nope, I've considered this as well while moving the pin count into TTM.
The problem is that you not only need the BO reserved for 0->1
transitions, but also for 1->0 transitions to move the BO on the LRU
correctly.
Ah, and there is no way to have us know the correct LRU list without
reservation?
Not really, there is always the chance that CPU A is reducing the
count from 1->0 while CPU B is doing 0->1 and you end up with a LRU
status which doesn't match the pin count.
We could try to do something like a loop updating the LRU status until
it matches the pin count, but the implications of that are usually
really nasty.
OK, yeah I was more thinking along the lines of protecting the LRU
status with the global lru lock and unpin would then be
if (refcount_dec_and_lock(>pin_count, _glob.lru_lock)) {
add_to_relevant_lrus(bo, bo->lru_status);
spin_unlock(_glob.lru_lock);
}
But looking at ttm_bo_move_to_lru_tail() I realize that's not really
trivial anymore. I hope it's doable at some point though.
But meanwhile, perhaps TTM needs to accept and pave over that drivers
are in fact destroying pinned buffers?
/Thomas
Christian.
/Thomas
Christian.
/Thomas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/ttm: remove swap LRU v2
Hi "Christian,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next
tegra-drm/drm/tegra/for-next linus/master drm/drm-next v5.12-rc3 next-20210315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Christian-K-nig/drm-ttm-move-swapout-logic-around/20210316-000551
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: i386-randconfig-s002-20210315 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-277-gc089cd2d-dirty
#
https://github.com/0day-ci/linux/commit/70ae63f3a85b9791dfcf38034c304aedda122e7b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Christian-K-nig/drm-ttm-move-swapout-logic-around/20210316-000551
git checkout 70ae63f3a85b9791dfcf38034c304aedda122e7b
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
"sparse warnings: (new ones prefixed by >>)"
drivers/gpu/drm/ttm/ttm_device.c:42:1: sparse: sparse: symbol
'ttm_global_mutex' was not declared. Should it be static?
drivers/gpu/drm/ttm/ttm_device.c:43:10: sparse: sparse: symbol
'ttm_glob_use_count' was not declared. Should it be static?
>> drivers/gpu/drm/ttm/ttm_device.c:125:6: sparse: sparse: context imbalance in
>> 'ttm_device_swapout' - wrong count at exit
vim +/ttm_device_swapout +125 drivers/gpu/drm/ttm/ttm_device.c
124
> 125 long ttm_device_swapout(struct ttm_device *bdev, struct
> ttm_operation_ctx *ctx,
126 gfp_t gfp_flags)
127 {
128 struct ttm_global *glob = _glob;
129 struct ttm_resource_manager *man;
130 struct ttm_buffer_object *bo;
131 unsigned i, j;
132 int ret;
133
134 spin_lock(>lru_lock);
135 for (i = TTM_PL_SYSTEM; i < TTM_NUM_MEM_TYPES; ++i) {
136 man = ttm_manager_type(bdev, i);
137 if (!man || !man->use_tt)
138 continue;
139
140 for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
141 list_for_each_entry(bo, >lru[j], lru) {
142 long num_pages;
143
144 if (!bo->ttm ||
145 bo->ttm->page_flags &
TTM_PAGE_FLAG_SG ||
146 bo->ttm->page_flags &
TTM_PAGE_FLAG_SWAPPED)
147 continue;
148
149 num_pages = bo->ttm->num_pages;
150 ret = ttm_bo_swapout(bo, ctx,
gfp_flags);
151 /* ttm_bo_swapout has dropped the
lru_lock */
152 if (!ret)
153 return num_pages;
154 if (ret != -EBUSY)
155 return ret;
156 }
157 }
158 }
159 spin_unlock(>lru_lock);
160 return 0;
161 }
162 EXPORT_SYMBOL(ttm_device_swapout);
163
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/ttm: remove swap LRU v2
On Mon, 15 Mar 2021 at 16:04, Christian König
wrote:
>
> Instead evict round robin from each devices SYSTEM and TT domain.
>
> v2: reorder num_pages access reported by Dan's script
>
> Signed-off-by: Christian König
> ---
> drivers/gpu/drm/ttm/ttm_bo.c| 33 ++--
> drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
> drivers/gpu/drm/ttm/ttm_device.c| 60 +
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
> include/drm/ttm/ttm_bo_api.h| 1 -
> include/drm/ttm/ttm_bo_driver.h | 1 -
> include/drm/ttm/ttm_device.h| 7 +---
> 7 files changed, 52 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 56d2e38af273..a1be88be357b 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -73,7 +73,6 @@ static void ttm_bo_del_from_lru(struct ttm_buffer_object
> *bo)
> {
> struct ttm_device *bdev = bo->bdev;
>
> - list_del_init(>swap);
> list_del_init(>lru);
>
> if (bdev->funcs->del_from_lru_notify)
> @@ -104,16 +103,6 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object
> *bo,
>
> man = ttm_manager_type(bdev, mem->mem_type);
> list_move_tail(>lru, >lru[bo->priority]);
> - if (man->use_tt && bo->ttm &&
> - !(bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
> -TTM_PAGE_FLAG_SWAPPED))) {
> - struct list_head *swap;
> -
> - swap = _glob.swap_lru[bo->priority];
> - list_move_tail(>swap, swap);
> - } else {
> - list_del_init(>swap);
> - }
>
> if (bdev->funcs->del_from_lru_notify)
> bdev->funcs->del_from_lru_notify(bo);
> @@ -128,9 +117,6 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
> ttm_bo_bulk_move_set_pos(>vram[bo->priority],
> bo);
> break;
> }
> - if (bo->ttm && !(bo->ttm->page_flags &
> -(TTM_PAGE_FLAG_SG | TTM_PAGE_FLAG_SWAPPED)))
> - ttm_bo_bulk_move_set_pos(>swap[bo->priority],
> bo);
> }
> }
> EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
> @@ -168,20 +154,6 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move
> *bulk)
> list_bulk_move_tail(>lru[i], >first->lru,
> >last->lru);
> }
> -
> - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
> - struct ttm_lru_bulk_move_pos *pos = >swap[i];
> - struct list_head *lru;
> -
> - if (!pos->first)
> - continue;
> -
> - dma_resv_assert_held(pos->first->base.resv);
> - dma_resv_assert_held(pos->last->base.resv);
> -
> - lru = _glob.swap_lru[i];
> - list_bulk_move_tail(lru, >first->swap, >last->swap);
> - }
> }
> EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
>
> @@ -1058,7 +1030,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
> kref_init(>kref);
> INIT_LIST_HEAD(>lru);
> INIT_LIST_HEAD(>ddestroy);
> - INIT_LIST_HEAD(>swap);
> bo->bdev = bdev;
> bo->type = type;
> bo->mem.mem_type = TTM_PL_SYSTEM;
> @@ -1193,6 +1164,10 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo,
> struct ttm_operation_ctx *ctx,
> bool locked;
> int ret;
>
> + if (!bo->ttm || bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
> + TTM_PAGE_FLAG_SWAPPED))
> + return false;
> +
return 0; ?
Seems inconsistent to return zero here and not drop the lru lock? Or
maybe turn this into a programmer error, since the current caller
already checks for the above?
> if (!ttm_bo_evict_swapout_allowable(bo, ctx, , NULL))
> return -EBUSY;
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 031e5819fec4..a2a17c84ceb3 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -303,7 +303,6 @@ static int ttm_buffer_object_transfer(struct
> ttm_buffer_object *bo,
> atomic_inc(_glob.bo_count);
> INIT_LIST_HEAD(>base.ddestroy);
> INIT_LIST_HEAD(>base.lru);
> - INIT_LIST_HEAD(>base.swap);
> fbo->base.moving = NULL;
> drm_vma_node_reset(>base.base.vma_node);
>
> diff --git a/drivers/gpu/drm/ttm/ttm_device.c
> b/drivers/gpu/drm/ttm/ttm_device.c
> index dfc2a7e4e490..2c280fb1e992 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -67,7 +67,6 @@ static int ttm_global_init(void)
> unsigned long num_pages;
> struct sysinfo si;
> int ret = 0;
> - unsigned i;
>
> mutex_lock(_global_mutex);
> if (++ttm_glob_use_count > 1)
> @@ -90,8 +89,6 @@ static int ttm_global_init(void)
>
Re: [PATCH 1/3] drm/ttm: move swapout logic around
Hi "Christian,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next
tegra-drm/drm/tegra/for-next linus/master drm/drm-next v5.12-rc3 next-20210315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Christian-K-nig/drm-ttm-move-swapout-logic-around/20210316-000551
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
#
https://github.com/0day-ci/linux/commit/824dca26fe395899b41d9790944ddea345f7a6fd
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Christian-K-nig/drm-ttm-move-swapout-logic-around/20210316-000551
git checkout 824dca26fe395899b41d9790944ddea345f7a6fd
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
drivers/gpu/drm/ttm/ttm_device.c:42: warning: Function parameter or member
'ttm_global_mutex' not described in 'DEFINE_MUTEX'
drivers/gpu/drm/ttm/ttm_device.c:42: warning: expecting prototype for
ttm_global_mutex(). Prototype was for DEFINE_MUTEX() instead
drivers/gpu/drm/ttm/ttm_device.c:110: warning: Function parameter or member
'ctx' not described in 'ttm_global_swapout'
drivers/gpu/drm/ttm/ttm_device.c:110: warning: Function parameter or member
'gfp_flags' not described in 'ttm_global_swapout'
>> drivers/gpu/drm/ttm/ttm_device.c:110: warning: expecting prototype for A
>> buffer object shrink method that tries to swap out the first(). Prototype
>> was for ttm_global_swapout() instead
vim +110 drivers/gpu/drm/ttm/ttm_device.c
104
105 /**
106 * A buffer object shrink method that tries to swap out the first
107 * buffer object on the global::swap_lru list.
108 */
109 long ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags)
> 110 {
111 struct ttm_global *glob = _glob;
112 struct ttm_buffer_object *bo;
113 unsigned i;
114 int ret;
115
116 spin_lock(>lru_lock);
117 for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
118 list_for_each_entry(bo, >swap_lru[i], swap) {
119 uint32_t num_pages = bo->ttm->num_pages;
120
121 ret = ttm_bo_swapout(bo, ctx, gfp_flags);
122 /* ttm_bo_swapout has dropped the lru_lock */
123 if (!ret)
124 return num_pages;
125 if (ret != -EBUSY)
126 return ret;
127 }
128 }
129 spin_unlock(>lru_lock);
130 return 0;
131 }
132 EXPORT_SYMBOL(ttm_global_swapout);
133
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
2021 X.Org Foundation Election Candidates
To all X.Org Foundation Members: The election for the X.Org Foundation Board of Directors will begin on 22 March 2021. We have 6 candidates who are running for 4 seats. They are (in alphabetical order): Samuel Iglesias Gonsálvez Manasi Navare Lyude Paul Rodrigo Siqueira Lucas Stach Daniel Vetter Attached below are the Personal Statements each candidate submitted for your consideration along with their Statements of Contribution that they submitted with the membership application. Please review each of the candidates' statements to help you decide whom to vote for during the upcoming election. If you have questions of the candidates, you should feel free to ask them here on the mailing list. The election committee will provide detailed instructions on how the voting system will work when the voting period begins. ** Election Schedule ** Nomination period Start: Mon 22nd February Nomination period End: Sun 7th March Publication of Candidates & start of Candidate QA: Mon 15th March Deadline of X.Org membership application or renewal: Thu 18th March Election Planned Start: Mon 22nd March anywhere on earth Election Planned End: Sun 4th April anywhere on earth ** Election Committee ** * Eric Anholt * Mark Filion * Keith Packard * Harry Wentland Thanks, Harry Wentland, on behalf of the X.Org elections committee ** Nominees ** ## Samuel Iglesias Gonsálvez __Current Affiliation:__ Igalia __Personal Statement:__ I have been contributing to Mesa and piglit for 7 years improving open-source drivers for both OpenGL and Vulkan. During my time on the board, I have become the XDC organization coordinator and the XDC CFP committee chair, due to my experience organizing the XDC 2018 in A Coruña, Spain. Thanks to these experiences, I have been deeply involved in the XDC organization process, where I have helped make a great and welcoming conference every year. If I am elected, I plan to continue leading both the XDC organization process and the XDC CFP committee. ## Manasi Navare __Current Affiliation:__ Intel __Statement of contribution:__ I am a lead contributor to Intel’s Open source graphics kernel driver i915 as well as to the Linux Kernel DRM subsystem. One of my most widely used contributions is the Display Port Compliance code in i915, DRM as well as in Xserver and IGT to make the entire graphics stack Display Port compliant and reward the end users with black screen free displays. I have also enabled features like Display stream compression across DRM and i915 as per VESA’s DSC specification for Display Port. Most recently I have been working with both the DRM and i915 community as well as the AMD developers to implement and enable adaptive sync feature for variable refresh rate on display drivers for enhanced gaming experience. I also have commit rights to several upstream projects like drm-intel, drm-misc and Intel GPU Tools. I have served on X.org board of directors for last 2 years organizing XDC conferences, being on Code of Conduct committee for X.org and Freedesktop and taking over the treasurer responsibility since September 2020. __Personal Statement:__ I have been a Linux Open Source contributor for last 7 years since I joined Intel's Open source technology center. My major contributions are in enabling display interfaces and develop display features in upcoming display specifications in i915 and Linux DRM subsystem. I have presented several talks at Linux Graphics conferences like Embedded Linux Conference, XDC and FOSDEM on several graphics display features like Display Port compliance and Display Stream Compression, Tiled display support, Adaptive Sync or Variable Refresh rate kernel support for gaming and I have been already actively involved in IRC discussions with DRM and i915 maintainers to constantly provide any solution on display port questions and work on improving the kernel documentation and code quality. I have served on the X.org board of directors since 2019 helping in XDC 2019 and XDC 2020 organization and for ensuring the Code of Conduct on the X.org community and during XDC events. I have previously mentored for the KMS project in Outreachy 2018 winter program as well as administered the Google summer of code 2019 program and will continue to do so whenever I get an opportunity. I joined the Code of Conduct Freedesktop committee as well to ensure it is followed on all the Freedesktop projects and open source channels. I have recently stepped up to take over the treasurer responsibility as part of the X.org board and working with all our sponsors to get the invoicing for the X.org events. If I get elected I would like to continue being a treasurer managing the XDC sponsorship and invoicing responsibilities as well as continue serving on the Code of conduct committee for X.org and Freedesktop. In addition to this I would like to help out on XDC event website organization and
Re: [PATCH] drm/ttm: make ttm_bo_unpin more defensive
Am 15.03.21 um 18:08 schrieb Thomas Hellström (Intel):
On 3/15/21 11:26 AM, Christian König wrote:
Am 13.03.21 um 19:29 schrieb Thomas Hellström (Intel):
Hi, Christian
On 3/12/21 10:38 AM, Christian König wrote:
We seem to have some more driver bugs than thought.
Signed-off-by: Christian König
---
include/drm/ttm/ttm_bo_api.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/drm/ttm/ttm_bo_api.h
b/include/drm/ttm/ttm_bo_api.h
index 4fb523dfab32..df9fe596e7c5 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -603,9 +603,11 @@ static inline void ttm_bo_pin(struct
ttm_buffer_object *bo)
static inline void ttm_bo_unpin(struct ttm_buffer_object *bo)
{
dma_resv_assert_held(bo->base.resv);
- WARN_ON_ONCE(!bo->pin_count);
WARN_ON_ONCE(!kref_read(>kref));
- --bo->pin_count;
+ if (bo->pin_count)
+ --bo->pin_count;
+ else
+ WARN_ON_ONCE(true);
}
int ttm_mem_evict_first(struct ttm_device *bdev,
Since I now have been staring for half a year at the code of the
driver that made pinning an art, I have a couple of suggestions
here, Could we use an atomic for pin_count, allowing unlocked
unpinning but require the lock only for pin_count transition 0->1,
(but unlocked for pin_if_already_pinned). In particular I think
vmwgfx would benefit from unlocked unpins. Also if the atomic were a
refcount_t, that would probably give you the above behaviour?
Nope, I've considered this as well while moving the pin count into TTM.
The problem is that you not only need the BO reserved for 0->1
transitions, but also for 1->0 transitions to move the BO on the LRU
correctly.
Ah, and there is no way to have us know the correct LRU list without
reservation?
Not really, there is always the chance that CPU A is reducing the count
from 1->0 while CPU B is doing 0->1 and you end up with a LRU status
which doesn't match the pin count.
We could try to do something like a loop updating the LRU status until
it matches the pin count, but the implications of that are usually
really nasty.
Christian.
/Thomas
Christian.
/Thomas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v15 1/2] drm/tegra: dc: Support memory bandwidth management
15.03.2021 01:31, Michał Mirosław пишет:
> On Thu, Mar 11, 2021 at 08:22:54PM +0300, Dmitry Osipenko wrote:
>> Display controller (DC) performs isochronous memory transfers, and thus,
>> has a requirement for a minimum memory bandwidth that shall be fulfilled,
>> otherwise framebuffer data can't be fetched fast enough and this results
>> in a DC's data-FIFO underflow that follows by a visual corruption.
> [...]
>> +static unsigned long
>> +tegra_plane_overlap_mask(struct drm_crtc_state *state,
>> + const struct drm_plane_state *plane_state)
>> +{
>> +const struct drm_plane_state *other_state;
>> +const struct tegra_plane *tegra;
>> +unsigned long overlap_mask = 0;
>> +struct drm_plane *plane;
>> +struct drm_rect rect;
>> +
>> +if (!plane_state->visible || !plane_state->fb)
>> +return 0;
>> +
>> +drm_atomic_crtc_state_for_each_plane_state(plane, other_state, state) {
> [...]
>> +/*
>> + * Data-prefetch FIFO will easily help to overcome temporal memory
>> + * pressure if other plane overlaps with the cursor plane.
>> + */
>> +if (tegra_plane_is_cursor(plane_state) && overlap_mask)
>> +return 0;
>> +
>> +return overlap_mask;
>> +}
>
> Since for cursor plane this always returns 0, you could test
> tegra_plane_is_cursor() at the start of the function.
Yes, thanks.
>> +static int tegra_crtc_calculate_memory_bandwidth(struct drm_crtc *crtc,
>> + struct drm_atomic_state *state)
> [...]
>> +/*
>> + * For overlapping planes pixel's data is fetched for each plane at
>> + * the same time, hence bandwidths are accumulated in this case.
>> + * This needs to be taken into account for calculating total bandwidth
>> + * consumed by all planes.
>> + *
>> + * Here we get the overlapping state of each plane, which is a
>> + * bitmask of plane indices telling with what planes there is an
>> + * overlap. Note that bitmask[plane] includes BIT(plane) in order
>> + * to make further code nicer and simpler.
>> + */
>> +drm_atomic_crtc_state_for_each_plane_state(plane, plane_state,
>> new_state) {
>> +tegra_state = to_const_tegra_plane_state(plane_state);
>> +tegra = to_tegra_plane(plane);
>> +
>> +if (WARN_ON_ONCE(tegra->index >= TEGRA_DC_LEGACY_PLANES_NUM))
>> +return -EINVAL;
>> +
>> +plane_peak_bw[tegra->index] =
>> tegra_state->peak_memory_bandwidth;
>> +mask = tegra_plane_overlap_mask(new_state, plane_state);
>> +overlap_mask[tegra->index] = mask;
>> +
>> +if (hweight_long(mask) != 3)
>> +all_planes_overlap_simultaneously = false;
>> +}
>> +
>> +old_state = drm_atomic_get_old_crtc_state(state, crtc);
>> +old_dc_state = to_const_dc_state(old_state);
>> +
>> +/*
>> + * Then we calculate maximum bandwidth of each plane state.
>> + * The bandwidth includes the plane BW + BW of the "simultaneously"
>> + * overlapping planes, where "simultaneously" means areas where DC
>> + * fetches from the planes simultaneously during of scan-out process.
>> + *
>> + * For example, if plane A overlaps with planes B and C, but B and C
>> + * don't overlap, then the peak bandwidth will be either in area where
>> + * A-and-B or A-and-C planes overlap.
>> + *
>> + * The plane_peak_bw[] contains peak memory bandwidth values of
>> + * each plane, this information is needed by interconnect provider
>> + * in order to set up latency allowness based on the peak BW, see
>> + * tegra_crtc_update_memory_bandwidth().
>> + */
>> +for (i = 0; i < ARRAY_SIZE(plane_peak_bw); i++) {
>> +overlap_bw = 0;
>> +
>> +for_each_set_bit(k, _mask[i], 3) {
>> +if (k == i)
>> +continue;
>> +
>> +if (all_planes_overlap_simultaneously)
>> +overlap_bw += plane_peak_bw[k];
>> +else
>> +overlap_bw = max(overlap_bw, plane_peak_bw[k]);
>> +}
>> +
>> +new_dc_state->plane_peak_bw[i] = plane_peak_bw[i] + overlap_bw;
>> +
>> +/*
>> + * If plane's peak bandwidth changed (for example plane isn't
>> + * overlapped anymore) and plane isn't in the atomic state,
>> + * then add plane to the state in order to have the bandwidth
>> + * updated.
>> + */
>> +if (old_dc_state->plane_peak_bw[i] !=
>> +new_dc_state->plane_peak_bw[i]) {
>> +plane = tegra_crtc_get_plane_by_index(crtc, i);
>> +if (!plane)
>> +continue;
>> +
>> +plane_state = drm_atomic_get_plane_state(state, plane);
>> +if (IS_ERR(plane_state))
>> +
Re: [PATCH] drm/amd/display: Remove unnecessary conversion to bool
Applied. Thanks! Alex On Mon, Mar 15, 2021 at 4:22 AM Jiapeng Chong wrote: > > Fix the following coccicheck warnings: > > ./drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c:358:69-74: WARNING: > conversion to bool not needed here. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong > --- > drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c > b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c > index 3e6f760..e153109 100644 > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c > @@ -355,7 +355,7 @@ void mpc3_set_output_gamma( > next_mode = LUT_RAM_A; > > mpc3_power_on_ogam_lut(mpc, mpcc_id, true); > - mpc3_configure_ogam_lut(mpc, mpcc_id, next_mode == LUT_RAM_A ? > true:false); > + mpc3_configure_ogam_lut(mpc, mpcc_id, next_mode == LUT_RAM_A); > > if (next_mode == LUT_RAM_A) > mpc3_program_luta(mpc, mpcc_id, params); > -- > 1.8.3.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V2] drm: amd: pm: Mundane typo fixes in the file amdgpu_pm.c
On Sun, Mar 14, 2021 at 11:22 PM Bhaskar Chowdhury wrote: > > > s/"an minimum"/"a minimum"/ > s/"an maxmum"/"a maximum"/ > > Signed-off-by: Bhaskar Chowdhury Applied. Thanks! Alex > --- > Changes from V1: > Randy's suggestion to adjust the subject line text > And missed out a spell too,which now included > > drivers/gpu/drm/amd/pm/amdgpu_pm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c > b/drivers/gpu/drm/amd/pm/amdgpu_pm.c > index 5fa65f191a37..308249ae1a22 100644 > --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c > +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c > @@ -3315,9 +3315,9 @@ static ssize_t amdgpu_hwmon_show_mclk_label(struct > device *dev, > * > * - pwm1_max: pulse width modulation fan control maximum level (255) > * > - * - fan1_min: an minimum value Unit: revolution/min (RPM) > + * - fan1_min: a minimum value Unit: revolution/min (RPM) > * > - * - fan1_max: an maxmum value Unit: revolution/max (RPM) > + * - fan1_max: a maximum value Unit: revolution/max (RPM) > * > * - fan1_input: fan speed in RPM > * > -- > 2.30.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 212279] AMDGPU doesn’t expose any way of setting the full RGB range [ryzen+ mobile]
https://bugzilla.kernel.org/show_bug.cgi?id=212279 --- Comment #2 from Neil (ledu...@hotmail.com) --- Created attachment 295873 --> https://bugzilla.kernel.org/attachment.cgi?id=295873=edit edid-decode Here is the edid file for my TV (works full rgb range with windows 10) I will try to edit the file with wxEDID now, but, it would be less tedious if it were automatic as you say. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/display: remove redundant initialization of variable result
Applied. Thanks!
Alex
On Thu, Mar 11, 2021 at 11:34 AM Colin King wrote:
>
> From: Colin Ian King
>
> The variable result is being initialized with a value that is
> never read and it is being updated later with a new value. The
> initialization is redundant and can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King
> ---
> drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> index 099f43709060..47e6c33f73cb 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> @@ -4281,7 +4281,7 @@ void dp_set_panel_mode(struct dc_link *link, enum
> dp_panel_mode panel_mode)
>
> if (edp_config_set.bits.PANEL_MODE_EDP
> != panel_mode_edp) {
> - enum dc_status result = DC_ERROR_UNEXPECTED;
> + enum dc_status result;
>
> edp_config_set.bits.PANEL_MODE_EDP =
> panel_mode_edp;
> --
> 2.30.2
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/amd/pm: Fix spelling mistake "disble" -> "disable"
Applied. Thanks! Alex On Fri, Mar 12, 2021 at 5:08 AM Colin King wrote: > > From: Colin Ian King > > There is a spelling mistake in an assert message. Fix it. > > Signed-off-by: Colin Ian King > --- > drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c > b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c > index 0d725b66fb78..7edafef095a3 100644 > --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c > +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c > @@ -1300,7 +1300,7 @@ static int smu7_start_dpm(struct pp_hwmgr *hwmgr) > (0 == smum_send_msg_to_smc(hwmgr, > PPSMC_MSG_PCIeDPM_Disable, > NULL)), > - "Failed to disble pcie DPM during DPM Start > Function!", > + "Failed to disable pcie DPM during DPM Start > Function!", > return -EINVAL); > } > > -- > 2.30.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 212259] Entire graphics stack locks up when running SteamVR and sometimes Sway; is sometimes unrecoverable
https://bugzilla.kernel.org/show_bug.cgi?id=212259 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) --- What chip is this? Can you attach your full dmesg output? Does updating mesa help? -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 212281] AMDGPU warning stack trace in dmesg (dcn20_validate_bandwidth_fp)
https://bugzilla.kernel.org/show_bug.cgi?id=212281 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) --- Please attach your full dmesg output and xorg log (if using X). -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 212279] AMDGPU doesn’t expose any way of setting the full RGB range [ryzen+ mobile]
https://bugzilla.kernel.org/show_bug.cgi?id=212279 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) --- IIRC, the driver sets full or limited range automatically based on the EDID. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: vmwgfx leaking bo pins?
On 3/12/21 5:06 AM, Thomas Hellström (Intel) wrote: On 3/12/21 12:02 AM, Zack Rusin wrote: On Mar 11, 2021, at 17:35, Thomas Hellström (Intel) wrote: Hi, Zack On 3/11/21 10:07 PM, Zack Rusin wrote: On Mar 11, 2021, at 05:46, Thomas Hellström (Intel) wrote: Hi, I tried latest drm-fixes today and saw a lot of these: Fallout from ttm rework? Yes, I fixed this in d1a73c641afd2617bd80bce8b71a096fc5b74b7e it was in drm-misc-next in the drm-misc tree for a while but hasn’t been merged for 5.12. z Hmm, yes but doesn't that fix trip the ttm_bo_unpin() dma_resv_assert_held(bo->base.resv)? No, doesn’t seem to. TBH I’m not sure why myself, but it seems to be working fine. With CONFIG_PROVE_LOCKING=y I see this: [ 7.117145] [drm] FIFO at 0xfe00 size is 8192 kiB [ 7.117284] [drm] VRAM at 0xe800 size is 131072 kiB [ 7.117291] INFO: trying to register non-static key. [ 7.117295] the code is fine but needs lockdep annotation. [ 7.117298] turning off the locking correctness validator Which will probably mask that dma_resv_assert_held(bo->base.resv) Ah, yes, you're right. After fixing that I do see the dma_resv_assert_held triggered. Technically trivially fixable with ttm_bo_reserve/ttm_bo_unreserve around ttm_bo_unpin but it's a little ugly that some places e.g. vmw_bo_unreference will require ttm_bo_reserve/ttm_bo_unreserve around ttm_bo_unpin but some won't e.g. vmw_mob_destroy won't because its lock is held by ttm_bo_delayed_delete without a very clear indication within the function which is which. z ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [RFC 1/6] drm/i915: Individual request cancellation
On 12/03/2021 15:46, Tvrtko Ursulin wrote:
From: Chris Wilson
Currently, we cancel outstanding requests within a context when the
context is closed. We may also want to cancel individual requests using
the same graceful preemption mechanism.
v2 (Tvrtko):
* Cancel waiters carefully considering no timeline lock and RCU.
* Fixed selftests.
Signed-off-by: Chris Wilson
Signed-off-by: Tvrtko Ursulin
[snip]
+void i915_request_cancel(struct i915_request *rq, int error)
+{
+ if (!i915_request_set_error_once(rq, error))
+ return;
+
+ set_bit(I915_FENCE_FLAG_SENTINEL, >fence.flags);
+
+ if (i915_sw_fence_signaled(>submit)) {
+ struct i915_dependency *p;
+
+restart:
+ rcu_read_lock();
+ for_each_waiter(p, rq) {
+ struct i915_request *w =
+ container_of(p->waiter, typeof(*w), sched);
+
+ if (__i915_request_is_complete(w) ||
+ fatal_error(w->fence.error))
+ continue;
+
+ w = i915_request_get(w);
+ rcu_read_unlock();
+ /* Recursion bound by the number of engines */
+ i915_request_cancel(w, error);
+ i915_request_put(w);
+
+ /* Restart after having to drop rcu lock. */
+ goto restart;
+ }
So I need to fix this error propagation to waiters in order to avoid
potential stack overflow caught in shards (gem_ctx_ringsize).
Or alternatively we decide not to propagate fence errors. Not sure that
consequences either way are particularly better or worse. Things will
break anyway since what userspace inspects for unexpected fence errors?!
So rendering corruption more or less. Can it cause a further stream of
GPU hangs I am not sure. Only if there is a inter-engine data dependency
involving data more complex than images/textures.
Regards,
Tvrtko
+ rcu_read_unlock();
+ }
+
+ __cancel_request(rq);
+}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 212293] [amdgpu] divide error: 0000 on resume from S3
https://bugzilla.kernel.org/show_bug.cgi?id=212293 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #2 from Alex Deucher (alexdeuc...@gmail.com) --- Can you bisect? https://www.kernel.org/doc/html/latest/admin-guide/bug-bisect.html -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: make ttm_bo_unpin more defensive
On 3/15/21 8:48 AM, Thomas Zimmermann wrote:
Hi
Am 13.03.21 um 19:29 schrieb Thomas Hellström (Intel):
Hi, Christian
On 3/12/21 10:38 AM, Christian König wrote:
We seem to have some more driver bugs than thought.
Signed-off-by: Christian König
---
include/drm/ttm/ttm_bo_api.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/drm/ttm/ttm_bo_api.h
b/include/drm/ttm/ttm_bo_api.h
index 4fb523dfab32..df9fe596e7c5 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -603,9 +603,11 @@ static inline void ttm_bo_pin(struct
ttm_buffer_object *bo)
static inline void ttm_bo_unpin(struct ttm_buffer_object *bo)
{
dma_resv_assert_held(bo->base.resv);
- WARN_ON_ONCE(!bo->pin_count);
WARN_ON_ONCE(!kref_read(>kref));
- --bo->pin_count;
+ if (bo->pin_count)
+ --bo->pin_count;
+ else
+ WARN_ON_ONCE(true);
}
int ttm_mem_evict_first(struct ttm_device *bdev,
Since I now have been staring for half a year at the code of the
driver that made pinning an art, I have a couple of suggestions here,
Could we use an atomic for pin_count, allowing unlocked unpinning but
require the lock only for pin_count transition 0->1, (but unlocked
for pin_if_already_pinned). In particular I think vmwgfx would
benefit from unlocked unpins. Also if the atomic were a refcount_t,
that would probably give you the above behaviour?
What's the benefit?
I'm asking because, there's been talk about streamlining all the GEM
locking and actually allowing dma-buf resv locking in pin and vmap
operations. Atomic ops might not contradict this, but also might not
be useful in the long term.
The benefit would be that at unpinning time it might be tricky to take
the reservation lock, because you might be already in a ww transaction.
But Christian pointed out the LRU complications...
/Thomas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: make ttm_bo_unpin more defensive
On 3/15/21 11:26 AM, Christian König wrote:
Am 13.03.21 um 19:29 schrieb Thomas Hellström (Intel):
Hi, Christian
On 3/12/21 10:38 AM, Christian König wrote:
We seem to have some more driver bugs than thought.
Signed-off-by: Christian König
---
include/drm/ttm/ttm_bo_api.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/drm/ttm/ttm_bo_api.h
b/include/drm/ttm/ttm_bo_api.h
index 4fb523dfab32..df9fe596e7c5 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -603,9 +603,11 @@ static inline void ttm_bo_pin(struct
ttm_buffer_object *bo)
static inline void ttm_bo_unpin(struct ttm_buffer_object *bo)
{
dma_resv_assert_held(bo->base.resv);
- WARN_ON_ONCE(!bo->pin_count);
WARN_ON_ONCE(!kref_read(>kref));
- --bo->pin_count;
+ if (bo->pin_count)
+ --bo->pin_count;
+ else
+ WARN_ON_ONCE(true);
}
int ttm_mem_evict_first(struct ttm_device *bdev,
Since I now have been staring for half a year at the code of the
driver that made pinning an art, I have a couple of suggestions here,
Could we use an atomic for pin_count, allowing unlocked unpinning but
require the lock only for pin_count transition 0->1, (but unlocked
for pin_if_already_pinned). In particular I think vmwgfx would
benefit from unlocked unpins. Also if the atomic were a refcount_t,
that would probably give you the above behaviour?
Nope, I've considered this as well while moving the pin count into TTM.
The problem is that you not only need the BO reserved for 0->1
transitions, but also for 1->0 transitions to move the BO on the LRU
correctly.
Ah, and there is no way to have us know the correct LRU list without
reservation?
/Thomas
Christian.
/Thomas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 212293] [amdgpu] divide error: 0000 on resume from S3
https://bugzilla.kernel.org/show_bug.cgi?id=212293 --- Comment #1 from Sefa Eyeoglu (cont...@scrumplex.net) --- ADDITIONAL SYSTEM INFO OS: Arch Linux (with testing repos) Kernels with this issue: 5.11.6.arch1, 5.11.6.zen1, 5.12rc2 (built from Arch Linux User Repository) Kernels without this issue: 5.10.23-1-lts -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 212293] New: [amdgpu] divide error: 0000 on resume from S3
https://bugzilla.kernel.org/show_bug.cgi?id=212293 Bug ID: 212293 Summary: [amdgpu] divide error: on resume from S3 Product: Drivers Version: 2.5 Kernel Version: 5.11.6 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: cont...@scrumplex.net Regression: No Created attachment 295869 --> https://bugzilla.kernel.org/attachment.cgi?id=295869=edit kernel log since resume My system experiences a kernel panic when resuming from S3, coming from amdgpu. The GPU has to be in a specific state for this to happen. Mainly when my desktop environment turns off the screens after some inactivity, and subsequently suspends the system. This issue only occurs with kernel versions 5.11.x. I could only reproduce this with KDE Plasma / KWin on Wayland, while testing KDE Plasma / KWin on Xorg and on Wayland (Xorg seems to work fine). REPRODUCTION 1. Start KDE Plasma / KWin on Wayland 2. Set Screen Energy Saving "Switch off after" to a low value like 1min 3. Wait until Plasma has turned off screens 4. Suspend the system (via SSH for example) 5. Try to wake from sleep SYSTEM INFO CPU: AMD Ryzen 9 3900X Mainboard: ASUS ROG STRIX B450-F GAMING II GPU: GIGABYTE Radeon RX VEGA 56 GAMING OC 8G ATTACHMENTS I attached the kernel panic I could capture via ttyS0. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/bridge: ti-sn65dsi86: Move code in prep for EDID read fix
Hi Doug,
On Mon, Mar 15, 2021 at 09:31:41AM -0700, Doug Anderson wrote:
> On Sat, Mar 13, 2021 at 1:13 PM Laurent Pinchart wrote:
> > On Thu, Mar 04, 2021 at 03:52:00PM -0800, Douglas Anderson wrote:
> > > This patch is _only_ code motion to prepare for the patch
> > > ("drm/bridge: ti-sn65dsi86: Properly get the EDID, but only if
> > > refclk") and make it easier to understand.
> >
> > s/make/makes/
>
> I was never an expert at grammar, but I think either "make" or "makes"
> are fine. Simple version with parenthesis:
>
> Mine:
>
> This patch is to (prepare for the patch ) and (make it
> easier to understand).
>
> Yours:
>
> This patch is (to prepare for the patch ) and (makes it
> easier to understand).
>
> I suppose also valid would be:
>
> This patch is (to prepare for the patch ) and (to make it
> easier to understand).
Your absolutely right. Both versions are fine, and your preferred
version is best :-)
> In any case if/when I spin this patch I'm fine changing it to your
> version just because (as I understand) it's equally valid and maybe
> looks slightly better?
>
> > > Signed-off-by: Douglas Anderson
> >
> > Reviewed-by: Laurent Pinchart
>
> Thanks for the reviews!
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/bridge: ti-sn65dsi86: Properly get the EDID, but only if refclk
Hi,
On Sat, Mar 13, 2021 at 1:17 PM Laurent Pinchart
wrote:
>
> Hi Doug,
>
> Thank you for the patch.
>
> On Thu, Mar 04, 2021 at 03:52:01PM -0800, Douglas Anderson wrote:
> > In commit 58074b08c04a ("drm/bridge: ti-sn65dsi86: Read EDID blob over
> > DDC") we attempted to make the ti-sn65dsi86 bridge properly read the
> > EDID from the panel. That commit kinda worked but it had some serious
> > problems.
> >
> > The problems all stem from the fact that userspace wants to be able to
> > read the EDID before it explicitly enables the panel. For eDP panels,
> > though, we don't actually power the panel up until the pre-enable
> > stage and the pre-enable call happens right before the enable call
> > with no way to interject in-between. For eDP panels, you can't read
> > the EDID until you power the panel. The result was that
> > ti_sn_bridge_connector_get_modes() was always failing to read the EDID
> > (falling back to what drm_panel_get_modes() returned) until _after_
> > the EDID was needed.
> >
> > To make it concrete, on my system I saw this happen:
> > 1. We'd attach the bridge.
> > 2. Userspace would ask for the EDID (several times). We'd try but fail
> >to read the EDID over and over again and fall back to the hardcoded
> >modes.
> > 3. Userspace would decide on a mode based only on the hardcoded modes.
> > 4. Userspace would ask to turn the panel on.
> > 5. Userspace would (eventually) check the modes again (in Chrome OS
> >this happens on the handoff from the boot splash screen to the
> >browser). Now we'd read them properly and, if they were different,
> >userspace would request to change the mode.
> >
> > The fact that userspace would always end up using the hardcoded modes
> > at first significantly decreases the benefit of the EDID
> > reading. Also: if the modes were even a tiny bit different we'd end up
> > doing a wasteful modeset and at boot.
>
> s/and at/at/ ?
Sure, I can correct if/when I respin or it can be corrected when landed.
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index 491c9c4f32d1..af3fb4657af6 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -16,6 +16,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include
> >
> > @@ -130,6 +131,12 @@
> > * @ln_assign:Value to program to the LN_ASSIGN register.
> > * @ln_polrs: Value for the 4-bit LN_POLRS field of SN_ENH_FRAME_REG.
> > *
> > + * @pre_enabled_early: If true we did an early pre_enable at attach.
> > + * @pre_enable_timeout_work: Delayed work to undo the pre_enable from
> > attach
> > + * if a normal pre_enable never came.
>
> Could we simplify this by using the runtime PM autosuspend feature ? The
> configuration of the bridge would be moved from pre_enable to the PM
> runtime resume handler, the clk_disable_unprepare() call moved from
> post_disable to the runtime suspend handler, and the work queue replaced
> by usage of pm_runtime_put_autosuspend().
It's an interesting idea but I don't think I can make it work, at
least not in a generic enough way. Specifically we can also use this
bridge chip as a generic GPIO provider in Linux. When someone asks us
to read a GPIO then we have to power the bridge on
(pm_runtime_get_sync()) and when someone asks us to configure a GPIO
as an output then we actually leave the bridge powered until they stop
requesting it as an output. At the moment the only user of this
functionality (that I know of) is for the HPD pin on trogdor boards
(long story about why we don't use the dedicated HPD) but the API
supports using these GPIOs for anything and I've tested that it works.
It wouldn't be great to have to keep the panel on in order to access
the GPIOs.
The other problem is that I think the time scales are different. At
boot time I think we'd want to leave the panel on for tens of seconds
to give userspace a chance to start up and configure the panel. After
userspace starts up I think we'd want autosuspend to be much faster.
This could probably be solved by tweaking the runtime delay in code
but I didn't fully dig because of the above problem.
-Doug
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/bridge: ti-sn65dsi86: Move code in prep for EDID read fix
Hi,
On Sat, Mar 13, 2021 at 1:13 PM Laurent Pinchart
wrote:
>
> Hi Douglas,
>
> Thank you for the patch.
>
> On Thu, Mar 04, 2021 at 03:52:00PM -0800, Douglas Anderson wrote:
> > This patch is _only_ code motion to prepare for the patch
> > ("drm/bridge: ti-sn65dsi86: Properly get the EDID, but only if
> > refclk") and make it easier to understand.
>
> s/make/makes/
I was never an expert at grammar, but I think either "make" or "makes"
are fine. Simple version with parenthesis:
Mine:
This patch is to (prepare for the patch ) and (make it
easier to understand).
Yours:
This patch is (to prepare for the patch ) and (makes it
easier to understand).
I suppose also valid would be:
This patch is (to prepare for the patch ) and (to make it
easier to understand).
In any case if/when I spin this patch I'm fine changing it to your
version just because (as I understand) it's equally valid and maybe
looks slightly better?
> > Signed-off-by: Douglas Anderson
>
> Reviewed-by: Laurent Pinchart
Thanks for the reviews!
-Doug
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/ttm: move swapout logic around
Move the iteration of the global lru into the new function
ttm_global_swapout() and use that instead in drivers.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c| 57 -
drivers/gpu/drm/ttm/ttm_device.c| 29 +++
drivers/gpu/drm/ttm/ttm_tt.c| 2 +-
drivers/gpu/drm/vmwgfx/ttm_memory.c | 3 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
include/drm/ttm/ttm_bo_api.h| 3 +-
include/drm/ttm/ttm_device.h| 2 +
7 files changed, 53 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index a08dec7281fc..56d2e38af273 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1186,56 +1186,35 @@ int ttm_bo_wait(struct ttm_buffer_object *bo,
}
EXPORT_SYMBOL(ttm_bo_wait);
-/*
- * A buffer object shrink method that tries to swap out the first
- * buffer object on the bo_global::swap_lru list.
- */
-int ttm_bo_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags)
+int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
+ gfp_t gfp_flags)
{
struct ttm_global *glob = _glob;
- struct ttm_buffer_object *bo;
- int ret = -EBUSY;
bool locked;
- unsigned i;
-
- spin_lock(>lru_lock);
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- list_for_each_entry(bo, >swap_lru[i], swap) {
- if (!ttm_bo_evict_swapout_allowable(bo, ctx, ,
- NULL))
- continue;
-
- if (!ttm_bo_get_unless_zero(bo)) {
- if (locked)
- dma_resv_unlock(bo->base.resv);
- continue;
- }
+ int ret;
- ret = 0;
- break;
- }
- if (!ret)
- break;
- }
+ if (!ttm_bo_evict_swapout_allowable(bo, ctx, , NULL))
+ return -EBUSY;
- if (ret) {
- spin_unlock(>lru_lock);
- return ret;
+ if (!ttm_bo_get_unless_zero(bo)) {
+ if (locked)
+ dma_resv_unlock(bo->base.resv);
+ return -EBUSY;
}
if (bo->deleted) {
- ret = ttm_bo_cleanup_refs(bo, false, false, locked);
+ ttm_bo_cleanup_refs(bo, false, false, locked);
ttm_bo_put(bo);
- return ret;
+ return 0;
}
ttm_bo_del_from_lru(bo);
+ /* TODO: Cleanup the locking */
spin_unlock(>lru_lock);
- /**
+ /*
* Move to system cached
*/
-
if (bo->mem.mem_type != TTM_PL_SYSTEM) {
struct ttm_operation_ctx ctx = { false, false };
struct ttm_resource evict_mem;
@@ -1255,29 +1234,26 @@ int ttm_bo_swapout(struct ttm_operation_ctx *ctx, gfp_t
gfp_flags)
}
}
- /**
+ /*
* Make sure BO is idle.
*/
-
ret = ttm_bo_wait(bo, false, false);
if (unlikely(ret != 0))
goto out;
ttm_bo_unmap_virtual(bo);
- /**
+ /*
* Swap out. Buffer will be swapped in again as soon as
* anyone tries to access a ttm page.
*/
-
if (bo->bdev->funcs->swap_notify)
bo->bdev->funcs->swap_notify(bo);
ret = ttm_tt_swapout(bo->bdev, bo->ttm, gfp_flags);
out:
- /**
-*
+ /*
* Unreserve without putting on LRU to avoid swapping out an
* already swapped buffer.
*/
@@ -1286,7 +1262,6 @@ int ttm_bo_swapout(struct ttm_operation_ctx *ctx, gfp_t
gfp_flags)
ttm_bo_put(bo);
return ret;
}
-EXPORT_SYMBOL(ttm_bo_swapout);
void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
{
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index 95e1b7b1f2e6..dfc2a7e4e490 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -102,6 +102,35 @@ static int ttm_global_init(void)
return ret;
}
+/**
+ * A buffer object shrink method that tries to swap out the first
+ * buffer object on the global::swap_lru list.
+ */
+long ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags)
+{
+ struct ttm_global *glob = _glob;
+ struct ttm_buffer_object *bo;
+ unsigned i;
+ int ret;
+
+ spin_lock(>lru_lock);
+ for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
+ list_for_each_entry(bo, >swap_lru[i], swap) {
+ uint32_t num_pages = bo->ttm->num_pages;
+
+ ret = ttm_bo_swapout(bo, ctx, gfp_flags);
+ /* ttm_bo_swapout has dropped the lru_lock */
+ if (!ret)
+
[PATCH 3/3] drm/ttm: switch to per device LRU lock
Instead of having a global lock.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 ++---
drivers/gpu/drm/qxl/qxl_release.c | 5 +--
drivers/gpu/drm/ttm/ttm_bo.c | 49 --
drivers/gpu/drm/ttm/ttm_device.c | 12 +++
drivers/gpu/drm/ttm/ttm_execbuf_util.c | 8 ++---
drivers/gpu/drm/ttm/ttm_resource.c | 9 +++--
include/drm/ttm/ttm_bo_driver.h| 4 +--
include/drm/ttm/ttm_device.h | 4 +--
8 files changed, 43 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 9d19078246c8..ae18c0e32347 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -638,15 +638,15 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device
*adev,
struct amdgpu_vm_bo_base *bo_base;
if (vm->bulk_moveable) {
- spin_lock(_glob.lru_lock);
+ spin_lock(>mman.bdev.lru_lock);
ttm_bo_bulk_move_lru_tail(>lru_bulk_move);
- spin_unlock(_glob.lru_lock);
+ spin_unlock(>mman.bdev.lru_lock);
return;
}
memset(>lru_bulk_move, 0, sizeof(vm->lru_bulk_move));
- spin_lock(_glob.lru_lock);
+ spin_lock(>mman.bdev.lru_lock);
list_for_each_entry(bo_base, >idle, vm_status) {
struct amdgpu_bo *bo = bo_base->bo;
@@ -660,7 +660,7 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
>shadow->tbo.mem,
>lru_bulk_move);
}
- spin_unlock(_glob.lru_lock);
+ spin_unlock(>mman.bdev.lru_lock);
vm->bulk_moveable = true;
}
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index f5845c96d414..b19f2f00b215 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -426,16 +426,13 @@ void qxl_release_fence_buffer_objects(struct qxl_release
*release)
release->id | 0xf000, release->base.seqno);
trace_dma_fence_emit(>base);
- spin_lock(_glob.lru_lock);
-
list_for_each_entry(entry, >bos, head) {
bo = entry->bo;
dma_resv_add_shared_fence(bo->base.resv, >base);
- ttm_bo_move_to_lru_tail(bo, >mem, NULL);
+ ttm_bo_move_to_lru_tail_unlocked(bo);
dma_resv_unlock(bo->base.resv);
}
- spin_unlock(_glob.lru_lock);
ww_acquire_fini(>ticket);
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index a1be88be357b..a8103c8718a3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -242,9 +242,9 @@ static int ttm_bo_individualize_resv(struct
ttm_buffer_object *bo)
* reference it any more. The only tricky case is the trylock on
* the resv object while holding the lru_lock.
*/
- spin_lock(_glob.lru_lock);
+ spin_lock(>bdev->lru_lock);
bo->base.resv = >base._resv;
- spin_unlock(_glob.lru_lock);
+ spin_unlock(>bdev->lru_lock);
}
return r;
@@ -303,7 +303,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
if (unlock_resv)
dma_resv_unlock(bo->base.resv);
- spin_unlock(_glob.lru_lock);
+ spin_unlock(>bdev->lru_lock);
lret = dma_resv_wait_timeout_rcu(resv, true, interruptible,
30 * HZ);
@@ -313,7 +313,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
else if (lret == 0)
return -EBUSY;
- spin_lock(_glob.lru_lock);
+ spin_lock(>bdev->lru_lock);
if (unlock_resv && !dma_resv_trylock(bo->base.resv)) {
/*
* We raced, and lost, someone else holds the
reservation now,
@@ -323,7 +323,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
* delayed destruction would succeed, so just return
success
* here.
*/
- spin_unlock(_glob.lru_lock);
+ spin_unlock(>bdev->lru_lock);
return 0;
}
ret = 0;
@@ -332,13 +332,13 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object
*bo,
if (ret || unlikely(list_empty(>ddestroy))) {
if (unlock_resv)
dma_resv_unlock(bo->base.resv);
- spin_unlock(_glob.lru_lock);
+ spin_unlock(>bdev->lru_lock);
return ret;
}
ttm_bo_del_from_lru(bo);
list_del_init(>ddestroy);
-
[PATCH 2/3] drm/ttm: remove swap LRU v2
Instead evict round robin from each devices SYSTEM and TT domain.
v2: reorder num_pages access reported by Dan's script
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c| 33 ++--
drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
drivers/gpu/drm/ttm/ttm_device.c| 60 +
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
include/drm/ttm/ttm_bo_api.h| 1 -
include/drm/ttm/ttm_bo_driver.h | 1 -
include/drm/ttm/ttm_device.h| 7 +---
7 files changed, 52 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 56d2e38af273..a1be88be357b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -73,7 +73,6 @@ static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
{
struct ttm_device *bdev = bo->bdev;
- list_del_init(>swap);
list_del_init(>lru);
if (bdev->funcs->del_from_lru_notify)
@@ -104,16 +103,6 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
man = ttm_manager_type(bdev, mem->mem_type);
list_move_tail(>lru, >lru[bo->priority]);
- if (man->use_tt && bo->ttm &&
- !(bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
-TTM_PAGE_FLAG_SWAPPED))) {
- struct list_head *swap;
-
- swap = _glob.swap_lru[bo->priority];
- list_move_tail(>swap, swap);
- } else {
- list_del_init(>swap);
- }
if (bdev->funcs->del_from_lru_notify)
bdev->funcs->del_from_lru_notify(bo);
@@ -128,9 +117,6 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
ttm_bo_bulk_move_set_pos(>vram[bo->priority], bo);
break;
}
- if (bo->ttm && !(bo->ttm->page_flags &
-(TTM_PAGE_FLAG_SG | TTM_PAGE_FLAG_SWAPPED)))
- ttm_bo_bulk_move_set_pos(>swap[bo->priority], bo);
}
}
EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
@@ -168,20 +154,6 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move
*bulk)
list_bulk_move_tail(>lru[i], >first->lru,
>last->lru);
}
-
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- struct ttm_lru_bulk_move_pos *pos = >swap[i];
- struct list_head *lru;
-
- if (!pos->first)
- continue;
-
- dma_resv_assert_held(pos->first->base.resv);
- dma_resv_assert_held(pos->last->base.resv);
-
- lru = _glob.swap_lru[i];
- list_bulk_move_tail(lru, >first->swap, >last->swap);
- }
}
EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
@@ -1058,7 +1030,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
kref_init(>kref);
INIT_LIST_HEAD(>lru);
INIT_LIST_HEAD(>ddestroy);
- INIT_LIST_HEAD(>swap);
bo->bdev = bdev;
bo->type = type;
bo->mem.mem_type = TTM_PL_SYSTEM;
@@ -1193,6 +1164,10 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct
ttm_operation_ctx *ctx,
bool locked;
int ret;
+ if (!bo->ttm || bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
+ TTM_PAGE_FLAG_SWAPPED))
+ return false;
+
if (!ttm_bo_evict_swapout_allowable(bo, ctx, , NULL))
return -EBUSY;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 031e5819fec4..a2a17c84ceb3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -303,7 +303,6 @@ static int ttm_buffer_object_transfer(struct
ttm_buffer_object *bo,
atomic_inc(_glob.bo_count);
INIT_LIST_HEAD(>base.ddestroy);
INIT_LIST_HEAD(>base.lru);
- INIT_LIST_HEAD(>base.swap);
fbo->base.moving = NULL;
drm_vma_node_reset(>base.base.vma_node);
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index dfc2a7e4e490..2c280fb1e992 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -67,7 +67,6 @@ static int ttm_global_init(void)
unsigned long num_pages;
struct sysinfo si;
int ret = 0;
- unsigned i;
mutex_lock(_global_mutex);
if (++ttm_glob_use_count > 1)
@@ -90,8 +89,6 @@ static int ttm_global_init(void)
goto out;
}
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
- INIT_LIST_HEAD(>swap_lru[i]);
INIT_LIST_HEAD(>device_list);
atomic_set(>bo_count, 0);
@@ -109,27 +106,60 @@ static int ttm_global_init(void)
long ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags)
{
struct ttm_global *glob = _glob;
+ struct ttm_device *bdev;
+ int ret = -EBUSY;
+
+ mutex_lock(_global_mutex);
+
[PATCH 1/2] drm/amdgpu: fix compile error on architecture s390
ioremap_cache is not supported on some architecture such as s390. Put the codes into a #ifdef to fix some compile error reported by test robot. Signed-off-by: Oak Zeng Reported-by: Kernel test robot --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 37751e7..1091585 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1817,7 +1817,7 @@ int amdgpu_ttm_init(struct amdgpu_device *adev) /* Change the size here instead of the init above so only lpfn is affected */ amdgpu_ttm_set_buffer_funcs_status(adev, false); -#ifdef CONFIG_64BIT +#ifdef CONFIG_X86 if (adev->gmc.xgmi.connected_to_cpu) adev->mman.aper_base_kaddr = ioremap_cache(adev->gmc.aper_base, adev->gmc.visible_vram_size); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/i915: Disable pread/pwrite ioctl's for future platforms (v3)
From: Ashutosh Dixit
The rationale for this change is roughly as follows:
1. The functionality can be done entirely in userspace with a
combination of mmap + memcpy
2. The only reason anyone in userspace is still using it is because
someone implemented bo_subdata that way in libdrm ages ago and
they're all too lazy to write the 5 lines of code to do a map.
3. This falls cleanly into the category of things which will only get
more painful with local memory support.
These ioctls aren't used much anymore by "real" userspace drivers.
Vulkan has never used them and neither has the iris GL driver. The old
i965 GL driver does use PWRITE for glBufferSubData but it only supports
up through Gen11; Gen12 was never enabled in i965. The compute driver
has never used PREAD/PWRITE. The only remaining user is the media
driver which uses it exactly twice and they're easily removed [1] so
expecting them to drop it going forward is reasonable.
IGT changes which handle this kernel change have also been submitted [2].
[1] https://github.com/intel/media-driver/pull/1160
[2] https://patchwork.freedesktop.org/series/81384/
v2 (Jason Ekstrand):
- Improved commit message with the status of all usermode drivers
- A more future-proof platform check
v3 (Jason Ekstrand):
- Drop the HAS_LMEM checks as they're already covered by the version
checks
Signed-off-by: Ashutosh Dixit
Signed-off-by: Jason Ekstrand
Reviewed-by: Jason Ekstrand
---
drivers/gpu/drm/i915/i915_gem.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b2e3b5cfccb4a..80915467e0d84 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -374,10 +374,17 @@ int
i915_gem_pread_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
{
+ struct drm_i915_private *i915 = to_i915(dev);
struct drm_i915_gem_pread *args = data;
struct drm_i915_gem_object *obj;
int ret;
+ /* PREAD is disallowed for all platforms after TGL-LP. This also
+* covers all platforms with local memory.
+*/
+ if (INTEL_GEN(i915) >= 12 && !IS_TIGERLAKE(i915))
+ return -EOPNOTSUPP;
+
if (args->size == 0)
return 0;
@@ -675,10 +682,17 @@ int
i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
{
+ struct drm_i915_private *i915 = to_i915(dev);
struct drm_i915_gem_pwrite *args = data;
struct drm_i915_gem_object *obj;
int ret;
+ /* PWRITE is disallowed for all platforms after TGL-LP. This also
+* covers all platforms with local memory.
+*/
+ if (INTEL_GEN(i915) >= 12 && !IS_TIGERLAKE(i915))
+ return -EOPNOTSUPP;
+
if (args->size == 0)
return 0;
--
2.29.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/i915/gem: Drop relocation support on all new hardware (v5)
The Vulkan driver in Mesa for Intel hardware never uses relocations if
it's running on a version of i915 that supports at least softpin which
all versions of i915 supporting Gen12 do. On the OpenGL side, Gen12+ is
only supported by iris which never uses relocations. The older i965
driver in Mesa does use relocations but it only supports Intel hardware
through Gen11 and has been deprecated for all hardware Gen9+. The
compute driver also never uses relocations. This only leaves the media
driver which is supposed to be switching to softpin going forward.
Making softpin a requirement for all future hardware seems reasonable.
There is one piece of hardware enabled by default in i915: RKL which was
enabled by e22fa6f0a976 which has not yet landed in drm-next so this
almost but not really a userspace API change for RKL. If it becomes a
problem, we can always add !IS_ROCKETLAKE(eb->i915) to the condition.
Rejecting relocations starting with newer Gen12 platforms has the
benefit that we don't have to bother supporting it on platforms with
local memory. Given how much CPU touching of memory is required for
relocations, not having to do so on platforms where not all memory is
directly CPU-accessible carries significant advantages.
v2 (Jason Ekstrand):
- Allow TGL-LP platforms as they've already shipped
v3 (Jason Ekstrand):
- WARN_ON platforms with LMEM support in case the check is wrong
v4 (Jason Ekstrand):
- Call out Rocket Lake in the commit message
v5 (Jason Ekstrand):
- Drop the HAS_LMEM check as it's already covered by the version check
Signed-off-by: Jason Ekstrand
Reviewed-by: Zbigniew Kempczyński
Reviewed-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 99772f37bff60..f66cff2943baa 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1764,7 +1764,8 @@ eb_relocate_vma_slow(struct i915_execbuffer *eb, struct
eb_vma *ev)
return err;
}
-static int check_relocations(const struct drm_i915_gem_exec_object2 *entry)
+static int check_relocations(const struct i915_execbuffer *eb,
+const struct drm_i915_gem_exec_object2 *entry)
{
const char __user *addr, *end;
unsigned long size;
@@ -1774,6 +1775,12 @@ static int check_relocations(const struct
drm_i915_gem_exec_object2 *entry)
if (size == 0)
return 0;
+ /* Relocations are disallowed for all platforms after TGL-LP. This
+* also covers all platforms with local memory.
+*/
+ if (INTEL_GEN(eb->i915) >= 12 && !IS_TIGERLAKE(eb->i915))
+ return -EINVAL;
+
if (size > N_RELOC(ULONG_MAX))
return -EINVAL;
@@ -1807,7 +1814,7 @@ static int eb_copy_relocations(const struct
i915_execbuffer *eb)
if (nreloc == 0)
continue;
- err = check_relocations(>exec[i]);
+ err = check_relocations(eb, >exec[i]);
if (err)
goto err;
@@ -1880,7 +1887,7 @@ static int eb_prefault_relocations(const struct
i915_execbuffer *eb)
for (i = 0; i < count; i++) {
int err;
- err = check_relocations(>exec[i]);
+ err = check_relocations(eb, >exec[i]);
if (err)
return err;
}
--
2.29.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/i915/gem: Drop legacy execbuffer support (v2)
libdrm has supported the newer execbuffer2 ioctl and using it by default
when it exists since libdrm commit b50964027bef which landed Mar 2, 2010.
The i915 and i965 drivers in Mesa at the time both used libdrm and so
did the Intel X11 back-end. The SNA back-end for X11 has always used
execbuffer2.
v2 (Jason Ekstrand):
- Add a comment saying what Linux version it's being removed in.
Signed-off-by: Jason Ekstrand
Acked-by: Keith Packard
Acked-by: Dave Airlie
---
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 100 --
drivers/gpu/drm/i915/gem/i915_gem_ioctls.h| 2 -
drivers/gpu/drm/i915/i915_drv.c | 2 +-
include/uapi/drm/i915_drm.h | 1 +
4 files changed, 2 insertions(+), 103 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index fe170186dd428..99772f37bff60 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -3394,106 +3394,6 @@ static bool check_buffer_count(size_t count)
return !(count < 1 || count > INT_MAX || count > SIZE_MAX / sz - 1);
}
-/*
- * Legacy execbuffer just creates an exec2 list from the original exec object
- * list array and passes it to the real function.
- */
-int
-i915_gem_execbuffer_ioctl(struct drm_device *dev, void *data,
- struct drm_file *file)
-{
- struct drm_i915_private *i915 = to_i915(dev);
- struct drm_i915_gem_execbuffer *args = data;
- struct drm_i915_gem_execbuffer2 exec2;
- struct drm_i915_gem_exec_object *exec_list = NULL;
- struct drm_i915_gem_exec_object2 *exec2_list = NULL;
- const size_t count = args->buffer_count;
- unsigned int i;
- int err;
-
- if (!check_buffer_count(count)) {
- drm_dbg(>drm, "execbuf2 with %zd buffers\n", count);
- return -EINVAL;
- }
-
- exec2.buffers_ptr = args->buffers_ptr;
- exec2.buffer_count = args->buffer_count;
- exec2.batch_start_offset = args->batch_start_offset;
- exec2.batch_len = args->batch_len;
- exec2.DR1 = args->DR1;
- exec2.DR4 = args->DR4;
- exec2.num_cliprects = args->num_cliprects;
- exec2.cliprects_ptr = args->cliprects_ptr;
- exec2.flags = I915_EXEC_RENDER;
- i915_execbuffer2_set_context_id(exec2, 0);
-
- err = i915_gem_check_execbuffer();
- if (err)
- return err;
-
- /* Copy in the exec list from userland */
- exec_list = kvmalloc_array(count, sizeof(*exec_list),
- __GFP_NOWARN | GFP_KERNEL);
-
- /* Allocate extra slots for use by the command parser */
- exec2_list = kvmalloc_array(count + 2, eb_element_size(),
- __GFP_NOWARN | GFP_KERNEL);
- if (exec_list == NULL || exec2_list == NULL) {
- drm_dbg(>drm,
- "Failed to allocate exec list for %d buffers\n",
- args->buffer_count);
- kvfree(exec_list);
- kvfree(exec2_list);
- return -ENOMEM;
- }
- err = copy_from_user(exec_list,
-u64_to_user_ptr(args->buffers_ptr),
-sizeof(*exec_list) * count);
- if (err) {
- drm_dbg(>drm, "copy %d exec entries failed %d\n",
- args->buffer_count, err);
- kvfree(exec_list);
- kvfree(exec2_list);
- return -EFAULT;
- }
-
- for (i = 0; i < args->buffer_count; i++) {
- exec2_list[i].handle = exec_list[i].handle;
- exec2_list[i].relocation_count = exec_list[i].relocation_count;
- exec2_list[i].relocs_ptr = exec_list[i].relocs_ptr;
- exec2_list[i].alignment = exec_list[i].alignment;
- exec2_list[i].offset = exec_list[i].offset;
- if (INTEL_GEN(to_i915(dev)) < 4)
- exec2_list[i].flags = EXEC_OBJECT_NEEDS_FENCE;
- else
- exec2_list[i].flags = 0;
- }
-
- err = i915_gem_do_execbuffer(dev, file, , exec2_list);
- if (exec2.flags & __EXEC_HAS_RELOC) {
- struct drm_i915_gem_exec_object __user *user_exec_list =
- u64_to_user_ptr(args->buffers_ptr);
-
- /* Copy the new buffer offsets back to the user's exec list. */
- for (i = 0; i < args->buffer_count; i++) {
- if (!(exec2_list[i].offset & UPDATE))
- continue;
-
- exec2_list[i].offset =
- gen8_canonical_addr(exec2_list[i].offset &
PIN_OFFSET_MASK);
- exec2_list[i].offset &= PIN_OFFSET_MASK;
- if (__copy_to_user(_exec_list[i].offset,
-
[PATCH 0/3] drm/i915: Drop legacy IOCTLs on new HW
These three patches exist to clean up some of our IOCTL mess in i915. We've got more clean-up we should do eventually, but these are some of the easiest to drop and most egregious cases. Test-with: 20210121083742.46592-1-ashutosh.di...@intel.com Ashutosh Dixit (1): drm/i915: Disable pread/pwrite ioctl's for future platforms (v3) Jason Ekstrand (2): drm/i915/gem: Drop legacy execbuffer support (v2) drm/i915/gem: Drop relocation support on all new hardware (v5) .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 113 ++ drivers/gpu/drm/i915/gem/i915_gem_ioctls.h| 2 - drivers/gpu/drm/i915/i915_drv.c | 2 +- drivers/gpu/drm/i915/i915_gem.c | 14 +++ include/uapi/drm/i915_drm.h | 1 + 5 files changed, 26 insertions(+), 106 deletions(-) -- 2.29.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/2] drm/i915/dp_link_training: Add newlines to debug messages
On Thu, 11 Mar 2021, Ville Syrjälä wrote:
> On Wed, Mar 10, 2021 at 04:47:56PM -0500, Sean Paul wrote:
>> From: Sean Paul
>>
>> This patch adds some newlines which are missing from debug messages.
>> This will prevent logs from being stacked up in dmesg.
>>
>> Signed-off-by: Sean Paul
>> ---
>> drivers/gpu/drm/i915/display/intel_dp_link_training.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> index 892d7db7d94f..ad02d493ec16 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> @@ -29,7 +29,7 @@ static void
>> intel_dp_dump_link_status(const u8 link_status[DP_LINK_STATUS_SIZE])
>> {
>>
>> -DRM_DEBUG_KMS("ln0_1:0x%x ln2_3:0x%x align:0x%x sink:0x%x
>> adj_req0_1:0x%x adj_req2_3:0x%x",
>> +DRM_DEBUG_KMS("ln0_1:0x%x ln2_3:0x%x align:0x%x sink:0x%x
>> adj_req0_1:0x%x adj_req2_3:0x%x\n",
>>link_status[0], link_status[1], link_status[2],
>>link_status[3], link_status[4], link_status[5]);
>> }
>> @@ -731,7 +731,7 @@ intel_dp_link_train_phy(struct intel_dp *intel_dp,
>>
>> out:
>> drm_dbg_kms(_to_i915(intel_dp)->drm,
>> -"[CONNECTOR:%d:%s] Link Training %s at link rate = %d, lane
>> count = %d, at %s",
>> +"[CONNECTOR:%d:%s] Link Training %s at link rate = %d, lane
>> count = %d, at %s\n",
>
> Looking through some ci logs we do get the newline here somehow. A bit
> weird.
>
> Reviewed-by: Ville Syrjälä
Thanks for the patches and review, pushed to din.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5.11 094/306] drm/fb-helper: only unmap if buffer not null
From: Greg Kroah-Hartman
From: Tong Zhang
commit 874a52f9b693ed8bf7a92b3592a547ce8a684e6f upstream.
drm_fbdev_cleanup() can be called when fb_helper->buffer is null, hence
fb_helper->buffer should be checked before calling
drm_client_buffer_vunmap(). This buffer is also checked in
drm_client_framebuffer_delete(), so we should also do the same thing for
drm_client_buffer_vunmap().
[ 199.128742] RIP: 0010:drm_client_buffer_vunmap+0xd/0x20
[ 199.129031] Code: 43 18 48 8b 53 20 49 89 45 00 49 89 55 08 5b 44 89 e0 41
5c 41 5d 41 5e 5d
c3 0f 1f 00 53 48 89 fb 48 8d 7f 10 e8 73 7d a1 ff <48> 8b 7b 10 48 8d 73 18 5b
e9 75 53 fc ff 0
f 1f 44 00 00 48 b8 00
[ 199.130041] RSP: 0018:888103f3fc88 EFLAGS: 00010282
[ 199.130329] RAX: 0001 RBX: RCX: 8214d46d
[ 199.130733] RDX: 1079c6b9 RSI: 0246 RDI: 83ce35c8
[ 199.131119] RBP: 888103d25458 R08: 0001 R09: fbfff0791761
[ 199.131505] R10: 83c8bb07 R11: fbfff0791760 R12:
[ 199.131891] R13: 888103d25468 R14: 888103d25418 R15: 888103f18120
[ 199.132277] FS: 7f36fdcbb6a0() GS:88815b40()
knlGS:
[ 199.132721] CS: 0010 DS: ES: CR0: 80050033
[ 199.133033] CR2: 0010 CR3: 000103d26000 CR4: 06f0
[ 199.133420] DR0: DR1: DR2:
[ 199.133807] DR3: DR6: fffe0ff0 DR7: 0400
[ 199.134195] Call Trace:
[ 199.134333] drm_fbdev_cleanup+0x179/0x1a0
[ 199.134562] drm_fbdev_client_unregister+0x2b/0x40
[ 199.134828] drm_client_dev_unregister+0xa8/0x180
[ 199.135088] drm_dev_unregister+0x61/0x110
[ 199.135315] mgag200_pci_remove+0x38/0x52 [mgag200]
[ 199.135586] pci_device_remove+0x62/0xe0
[ 199.135806] device_release_driver_internal+0x148/0x270
[ 199.136094] driver_detach+0x76/0xe0
[ 199.136294] bus_remove_driver+0x7e/0x100
[ 199.136521] pci_unregister_driver+0x28/0xf0
[ 199.136759] __x64_sys_delete_module+0x268/0x300
[ 199.137016] ? __ia32_sys_delete_module+0x300/0x300
[ 199.137285] ? call_rcu+0x3e4/0x580
[ 199.137481] ? fpregs_assert_state_consistent+0x4d/0x60
[ 199.137767] ? exit_to_user_mode_prepare+0x2f/0x130
[ 199.138037] do_syscall_64+0x33/0x40
[ 199.138237] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 199.138517] RIP: 0033:0x7f36fdc3dcf7
Signed-off-by: Tong Zhang
Fixes: 763aea17bf57 ("drm/fb-helper: Unmap client buffer during shutdown")
Cc: Thomas Zimmermann
Cc: Sam Ravnborg
Cc: Maxime Ripard
Cc: Maarten Lankhorst
Cc: David Airlie
Cc: Daniel Vetter
Cc: dri-devel@lists.freedesktop.org
Cc: # v5.11+
Signed-off-by: Thomas Zimmermann
Link:
https://patchwork.freedesktop.org/patch/msgid/20210228044625.171151-1-ztong0...@gmail.com
Signed-off-by: Maarten Lankhorst
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/drm_fb_helper.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2043,7 +2043,7 @@ static void drm_fbdev_cleanup(struct drm
if (shadow)
vfree(shadow);
- else
+ else if (fb_helper->buffer)
drm_client_buffer_vunmap(fb_helper->buffer);
drm_client_framebuffer_delete(fb_helper->buffer);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/imx/dcss: Use device_get_match_data()
The retrieval of driver data can be a bit simplified by using
device_get_match_data(), so switch to it.
Signed-off-by: Fabio Estevam
---
drivers/gpu/drm/imx/dcss/dcss-dev.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/imx/dcss/dcss-dev.c
b/drivers/gpu/drm/imx/dcss/dcss-dev.c
index c849533ca83e..de0f02de94c4 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-dev.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-dev.c
@@ -168,13 +168,6 @@ struct dcss_dev *dcss_dev_create(struct device *dev, bool
hdmi_output)
int ret;
struct resource *res;
struct dcss_dev *dcss;
- const struct dcss_type_data *devtype;
-
- devtype = of_device_get_match_data(dev);
- if (!devtype) {
- dev_err(dev, "no device match found\n");
- return ERR_PTR(-ENODEV);
- }
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -187,7 +180,7 @@ struct dcss_dev *dcss_dev_create(struct device *dev, bool
hdmi_output)
return ERR_PTR(-ENOMEM);
dcss->dev = dev;
- dcss->devtype = devtype;
+ dcss->devtype = device_get_match_data(dev);
dcss->hdmi_output = hdmi_output;
ret = dcss_clks_init(dcss);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: qcom-wled: Use sink_addr for sync toggle
On Sun, Mar 14, 2021 at 11:11:10AM +0100, Marijn Suijten wrote: > From: Obeida Shamoun > > WLED3_SINK_REG_SYNC is, as the name implies, a sink register offset. > Therefore, use the sink address as base instead of the ctrl address. > > This fixes the sync toggle on wled4, which can be observed by the fact > that adjusting brightness now works. > > It has no effect on wled3 because sink and ctrl base addresses are the > same. This allows adjusting the brightness without having to disable > then reenable the module. > > Signed-off-by: Obeida Shamoun > Signed-off-by: Konrad Dybcio > Signed-off-by: Marijn Suijten LGTM, although an acked-by from Kiran would be nice to have: Reviewed-by: Daniel Thompson Daniel. > --- > drivers/video/backlight/qcom-wled.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/backlight/qcom-wled.c > b/drivers/video/backlight/qcom-wled.c > index 091f07e7c145..fc8b443d10fd 100644 > --- a/drivers/video/backlight/qcom-wled.c > +++ b/drivers/video/backlight/qcom-wled.c > @@ -336,13 +336,13 @@ static int wled3_sync_toggle(struct wled *wled) > unsigned int mask = GENMASK(wled->max_string_count - 1, 0); > > rc = regmap_update_bits(wled->regmap, > - wled->ctrl_addr + WLED3_SINK_REG_SYNC, > + wled->sink_addr + WLED3_SINK_REG_SYNC, > mask, mask); > if (rc < 0) > return rc; > > rc = regmap_update_bits(wled->regmap, > - wled->ctrl_addr + WLED3_SINK_REG_SYNC, > + wled->sink_addr + WLED3_SINK_REG_SYNC, > mask, WLED3_SINK_REG_SYNC_CLEAR); > > return rc; > -- > 2.30.2 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/bridge: sii9234: sil-sii8620.c: move to use request_irq by IRQF_NO_AUTOEN flag
After this patch cbe16f35bee68 genirq: Add IRQF_NO_AUTOEN for
request_irq/nmi() is merged. request_irq() after setting
IRQ_NOAUTOEN as below
irq_set_status_flags(irq, IRQ_NOAUTOEN);
request_irq(dev, irq...);
can be replaced by request_irq() with IRQF_NO_AUTOEN flag.
v2:
Fix the problem of using wrong flags
Signed-off-by: Tian Tao
---
drivers/gpu/drm/bridge/sii9234.c | 4 ++--
drivers/gpu/drm/bridge/sil-sii8620.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
index 15c98a7..54b5097 100644
--- a/drivers/gpu/drm/bridge/sii9234.c
+++ b/drivers/gpu/drm/bridge/sii9234.c
@@ -911,10 +911,10 @@ static int sii9234_probe(struct i2c_client *client,
return -EINVAL;
}
- irq_set_status_flags(client->irq, IRQ_NOAUTOEN);
ret = devm_request_threaded_irq(dev, client->irq, NULL,
sii9234_irq_thread,
- IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT |
+ IRQF_NO_AUTOEN,
"sii9234", ctx);
if (ret < 0) {
dev_err(dev, "failed to install IRQ handler\n");
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
b/drivers/gpu/drm/bridge/sil-sii8620.c
index 843265d..4133f6e 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -2308,10 +2308,10 @@ static int sii8620_probe(struct i2c_client *client,
dev_err(dev, "no irq provided\n");
return -EINVAL;
}
- irq_set_status_flags(client->irq, IRQ_NOAUTOEN);
ret = devm_request_threaded_irq(dev, client->irq, NULL,
sii8620_irq_thread,
- IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT |
+ IRQF_NO_AUTOEN,
"sii8620", ctx);
if (ret < 0)
return dev_err_probe(dev, ret,
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/5] drm: Add and export function drm_gem_cma_sync_data
Hi Thomas,
Le lun. 15 mars 2021 à 8:43, Thomas Zimmermann a
écrit :
Hi
Am 11.03.21 um 13:33 schrieb Paul Cercueil:
Le jeu. 11 mars 2021 à 12:28, Christoph Hellwig
a écrit :
On Sun, Mar 07, 2021 at 08:28:34PM +, Paul Cercueil wrote:
+drm_atomic_for_each_plane_damage(, ) {
+for (i = 0; i < finfo->num_planes; i++) {
+daddr = drm_fb_cma_get_gem_addr(state->fb, state, i);
+
+/* Ignore x1/x2 values, invalidate complete lines */
+offset = clip.y1 * state->fb->pitches[i];
+
+dma_sync_single_for_device(dev, daddr + offset,
+ (clip.y2 - clip.y1) *
state->fb->pitches[i],
+ DMA_TO_DEVICE);
Are these helpers only ever used to transfer data to the device and
never from it? If so please clearly document that.
Yes. In the DRM world, are there cases where we transfer data from
the device? I assume these cases are handled by v4l2 instead.
Software rendering (i.e., anything wrt dumb buffers) likely reads
back framebuffer content during blit operations. For devices where
this is a slow operation (e.g., PCI read) we set struct
drm_mode_config.prefer_shadow to hint renderers to use shadow
buffering.
This has been brought up a few times already. I answered that in the
cover letter. In my case, *writes* (e.g. dumb memcpy) are also slower
with a write-combine buffer than with a non-coherent buffer + cache
sync. So a shadow buffer does nothing for me.
Cheers,
-Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/exynos: move to use request_irq by IRQF_NO_AUTOEN flag
After this patch cbe16f35bee68 genirq: Add IRQF_NO_AUTOEN for
request_irq/nmi() is merged. request_irq() after setting
IRQ_NOAUTOEN as below
irq_set_status_flags(irq, IRQ_NOAUTOEN);
request_irq(dev, irq...);
can be replaced by request_irq() with IRQF_NO_AUTOEN flag.
v2:
Fix the problem of using wrong flags
Signed-off-by: Tian Tao
---
drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 4 ++--
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 7 +++
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 1f79bc2..c277d2f 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -775,8 +775,8 @@ static int decon_conf_irq(struct decon_context *ctx, const
char *name,
return irq;
}
}
- irq_set_status_flags(irq, IRQ_NOAUTOEN);
- ret = devm_request_irq(ctx->dev, irq, handler, flags, "drm_decon", ctx);
+ ret = devm_request_irq(ctx->dev, irq, handler,
+ flags | IRQF_NO_AUTOEN, "drm_decon", ctx);
if (ret < 0) {
dev_err(ctx->dev, "IRQ %s request failed\n", name);
return ret;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 83ab6b3..44e402b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1352,10 +1352,9 @@ static int exynos_dsi_register_te_irq(struct exynos_dsi
*dsi,
}
te_gpio_irq = gpio_to_irq(dsi->te_gpio);
- irq_set_status_flags(te_gpio_irq, IRQ_NOAUTOEN);
ret = request_threaded_irq(te_gpio_irq, exynos_dsi_te_irq_handler, NULL,
- IRQF_TRIGGER_RISING, "TE", dsi);
+ IRQF_TRIGGER_RISING | IRQF_NO_AUTOEN, "TE",
dsi);
if (ret) {
dev_err(dsi->dev, "request interrupt failed with %d\n", ret);
gpio_free(dsi->te_gpio);
@@ -1802,9 +1801,9 @@ static int exynos_dsi_probe(struct platform_device *pdev)
if (dsi->irq < 0)
return dsi->irq;
- irq_set_status_flags(dsi->irq, IRQ_NOAUTOEN);
ret = devm_request_threaded_irq(dev, dsi->irq, NULL,
- exynos_dsi_irq, IRQF_ONESHOT,
+ exynos_dsi_irq,
+ IRQF_ONESHOT | IRQF_NO_AUTOEN,
dev_name(dev), dsi);
if (ret) {
dev_err(dev, "failed to request dsi irq\n");
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 203905] amdgpu:actual_brightness has unreal/wrong value
https://bugzilla.kernel.org/show_bug.cgi?id=203905 --- Comment #25 from m11.1l1@gmail.com --- I found another workaround and it's easier than EVER and it's done via mkinitcpio First, what's mkinitcpio(https://wiki.archlinux.org/index.php/Mkinitcpio)? The initial ramdisk is in essence a very small environment (early userspace) which loads various kernel modules and sets up necessary things before handing over control to init. This makes it possible to have, for example, encrypted root file systems and root file systems on a software RAID array. mkinitcpio allows for easy extension with custom hooks, has autodetection at runtime, and many other features. So go ahead and edit with `sudo nano /etc/mkinitcpio.conf` Then add the "amdgpu" in the modules section like this: `MODULES=(amdgpu)` After that, simply run `sudo mkinitcpio -P` so that it regenerates mkinitcpio for all kernels This solved my issue, I hope it works for others too :)_ -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: make ttm_bo_unpin more defensive
Am 13.03.21 um 19:29 schrieb Thomas Hellström (Intel):
Hi, Christian
On 3/12/21 10:38 AM, Christian König wrote:
We seem to have some more driver bugs than thought.
Signed-off-by: Christian König
---
include/drm/ttm/ttm_bo_api.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 4fb523dfab32..df9fe596e7c5 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -603,9 +603,11 @@ static inline void ttm_bo_pin(struct
ttm_buffer_object *bo)
static inline void ttm_bo_unpin(struct ttm_buffer_object *bo)
{
dma_resv_assert_held(bo->base.resv);
- WARN_ON_ONCE(!bo->pin_count);
WARN_ON_ONCE(!kref_read(>kref));
- --bo->pin_count;
+ if (bo->pin_count)
+ --bo->pin_count;
+ else
+ WARN_ON_ONCE(true);
}
int ttm_mem_evict_first(struct ttm_device *bdev,
Since I now have been staring for half a year at the code of the
driver that made pinning an art, I have a couple of suggestions here,
Could we use an atomic for pin_count, allowing unlocked unpinning but
require the lock only for pin_count transition 0->1, (but unlocked for
pin_if_already_pinned). In particular I think vmwgfx would benefit
from unlocked unpins. Also if the atomic were a refcount_t, that would
probably give you the above behaviour?
Nope, I've considered this as well while moving the pin count into TTM.
The problem is that you not only need the BO reserved for 0->1
transitions, but also for 1->0 transitions to move the BO on the LRU
correctly.
Christian.
/Thomas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/bridge: sii9234: sil-sii8620.c: move to use request_irq by IRQF_NO_AUTOEN flag
After this patch cbe16f35bee68 genirq: Add IRQF_NO_AUTOEN for
request_irq/nmi() is merged. request_irq() after setting
IRQ_NOAUTOEN as below
irq_set_status_flags(irq, IRQ_NOAUTOEN);
request_irq(dev, irq...);
can be replaced by request_irq() with IRQF_NO_AUTOEN flag.
Signed-off-by: Tian Tao
---
drivers/gpu/drm/bridge/sii9234.c | 4 ++--
drivers/gpu/drm/bridge/sil-sii8620.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
index 15c98a7..2742d5d 100644
--- a/drivers/gpu/drm/bridge/sii9234.c
+++ b/drivers/gpu/drm/bridge/sii9234.c
@@ -911,10 +911,10 @@ static int sii9234_probe(struct i2c_client *client,
return -EINVAL;
}
- irq_set_status_flags(client->irq, IRQ_NOAUTOEN);
ret = devm_request_threaded_irq(dev, client->irq, NULL,
sii9234_irq_thread,
- IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT |
+ IRQ_NOAUTOEN,
"sii9234", ctx);
if (ret < 0) {
dev_err(dev, "failed to install IRQ handler\n");
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
b/drivers/gpu/drm/bridge/sil-sii8620.c
index 843265d..9476ef0 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -2308,10 +2308,10 @@ static int sii8620_probe(struct i2c_client *client,
dev_err(dev, "no irq provided\n");
return -EINVAL;
}
- irq_set_status_flags(client->irq, IRQ_NOAUTOEN);
ret = devm_request_threaded_irq(dev, client->irq, NULL,
sii8620_irq_thread,
- IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT |
+ IRQ_NOAUTOEN,
"sii8620", ctx);
if (ret < 0)
return dev_err_probe(dev, ret,
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 212255] WARNING: at arch/x86/kernel/fpu/core.c:129 kernel_fpu_begin_mask
https://bugzilla.kernel.org/show_bug.cgi?id=212255 --- Comment #4 from Felice Tufo (i...@felicetufo.com) --- I confirm that the fix solves the bug on kernel 5.12.0-rc3 (at least on my test system). -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/display: Remove unnecessary conversion to bool
Fix the following coccicheck warnings: ./drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c:358:69-74: WARNING: conversion to bool not needed here. Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong --- drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c index 3e6f760..e153109 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c @@ -355,7 +355,7 @@ void mpc3_set_output_gamma( next_mode = LUT_RAM_A; mpc3_power_on_ogam_lut(mpc, mpcc_id, true); - mpc3_configure_ogam_lut(mpc, mpcc_id, next_mode == LUT_RAM_A ? true:false); + mpc3_configure_ogam_lut(mpc, mpcc_id, next_mode == LUT_RAM_A); if (next_mode == LUT_RAM_A) mpc3_program_luta(mpc, mpcc_id, params); -- 1.8.3.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos/decon5433: Clean up GPIO includes
在 2021/3/15 14:35, Inki Dae 写道: Hi Tian Tao, 21. 3. 2. 오후 12:03에 Tian Tao 이(가) 쓴 글: remove the legacy GPIO headers but what it really uses is since only gpio_desc structs are ever referenced. This driver doesn't reference even linux/gpio/consumer.h so could you drop only of_gpio.h inclusion? Thanks for helping to review patch,I've posted a new patch to fix this problem If you can give me review-by, I can help push to drm-misc. Subject: [PATCH] drm/exynos/decon5433: Remove the unused include statements This driver doesn't reference of_gpio.h, so drop it. Signed-off-by: Tian Tao --- drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c index 1f79bc2..1510e4e 100644 --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c @@ -13,7 +13,6 @@ #include #include #include -#include #include #include #include Thanks, Inki Dae Signed-off-by: Tian Tao --- drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c index 1f79bc2..9fc51a6 100644 --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c @@ -11,9 +11,9 @@ #include #include #include +#include #include #include -#include #include #include #include . ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos/decon5433: Clean up GPIO includes
在 2021/3/15 14:35, Inki Dae 写道: Hi Tian Tao, 21. 3. 2. 오후 12:03에 Tian Tao 이(가) 쓴 글: remove the legacy GPIO headers but what it really uses is since only gpio_desc structs are ever referenced. This driver doesn't reference even linux/gpio/consumer.h so could you drop only of_gpio.h inclusion? Thanks for helping to review patch, If you can give me review-by, I can help push to drm-misc. Thanks, Inki Dae Signed-off-by: Tian Tao --- drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c index 1f79bc2..9fc51a6 100644 --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c @@ -11,9 +11,9 @@ #include #include #include +#include #include #include -#include #include #include #include . ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 15/17] iommu: remove DOMAIN_ATTR_NESTING
Hi Christoph, On 3/14/21 4:58 PM, Christoph Hellwig wrote: > On Sun, Mar 14, 2021 at 11:44:52AM +0100, Auger Eric wrote: >> As mentionned by Robin, there are series planning to use >> DOMAIN_ATTR_NESTING to get info about the nested caps of the iommu (ARM >> and Intel): >> >> [Patch v8 00/10] vfio: expose virtual Shared Virtual Addressing to VMs >> patches 1, 2, 3 >> >> Is the plan to introduce a new domain_get_nesting_info ops then? > > The plan as usual would be to add it the series adding that support. > Not sure what the merge plans are - if the series is ready to be > merged I could rebase on top of it, otherwise that series will need > to add the method. OK I think your series may be upstreamed first. Thanks Eric > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: make ttm_bo_unpin more defensive
Hi
Am 13.03.21 um 19:29 schrieb Thomas Hellström (Intel):
Hi, Christian
On 3/12/21 10:38 AM, Christian König wrote:
We seem to have some more driver bugs than thought.
Signed-off-by: Christian König
---
include/drm/ttm/ttm_bo_api.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 4fb523dfab32..df9fe596e7c5 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -603,9 +603,11 @@ static inline void ttm_bo_pin(struct
ttm_buffer_object *bo)
static inline void ttm_bo_unpin(struct ttm_buffer_object *bo)
{
dma_resv_assert_held(bo->base.resv);
- WARN_ON_ONCE(!bo->pin_count);
WARN_ON_ONCE(!kref_read(>kref));
- --bo->pin_count;
+ if (bo->pin_count)
+ --bo->pin_count;
+ else
+ WARN_ON_ONCE(true);
}
int ttm_mem_evict_first(struct ttm_device *bdev,
Since I now have been staring for half a year at the code of the driver
that made pinning an art, I have a couple of suggestions here, Could we
use an atomic for pin_count, allowing unlocked unpinning but require the
lock only for pin_count transition 0->1, (but unlocked for
pin_if_already_pinned). In particular I think vmwgfx would benefit from
unlocked unpins. Also if the atomic were a refcount_t, that would
probably give you the above behaviour?
What's the benefit?
I'm asking because, there's been talk about streamlining all the GEM
locking and actually allowing dma-buf resv locking in pin and vmap
operations. Atomic ops might not contradict this, but also might not be
useful in the long term.
Best regards
Thomas
/Thomas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/5] drm: Add and export function drm_gem_cma_sync_data
Hi
Am 11.03.21 um 13:33 schrieb Paul Cercueil:
Le jeu. 11 mars 2021 à 12:28, Christoph Hellwig a
écrit :
On Sun, Mar 07, 2021 at 08:28:34PM +, Paul Cercueil wrote:
+ drm_atomic_for_each_plane_damage(, ) {
+ for (i = 0; i < finfo->num_planes; i++) {
+ daddr = drm_fb_cma_get_gem_addr(state->fb, state, i);
+
+ /* Ignore x1/x2 values, invalidate complete lines */
+ offset = clip.y1 * state->fb->pitches[i];
+
+ dma_sync_single_for_device(dev, daddr + offset,
+ (clip.y2 - clip.y1) * state->fb->pitches[i],
+ DMA_TO_DEVICE);
Are these helpers only ever used to transfer data to the device and
never from it? If so please clearly document that.
Yes. In the DRM world, are there cases where we transfer data from the
device? I assume these cases are handled by v4l2 instead.
Software rendering (i.e., anything wrt dumb buffers) likely reads back
framebuffer content during blit operations. For devices where this is a
slow operation (e.g., PCI read) we set struct
drm_mode_config.prefer_shadow to hint renderers to use shadow buffering.
Best regards
Thomas
-Paul
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos/decon5433: Clean up GPIO includes
21. 3. 15. 오후 3:45에 tiantao (H) 이(가) 쓴 글: > > 在 2021/3/15 14:35, Inki Dae 写道: >> Hi Tian Tao, >> >> 21. 3. 2. 오후 12:03에 Tian Tao 이(가) 쓴 글: >>> remove the legacy GPIO headers but what it really >>> uses is since only gpio_desc structs are ever >>> referenced. >> This driver doesn't reference even linux/gpio/consumer.h so could you drop >> only of_gpio.h inclusion? > > Thanks for helping to review patch,I've posted a new patch to fix this > problem If you can give me review-by, I can help push to drm-misc. I can merge this patch and other one[1] to exynos drm tree after review, and will have a pull-request soon so no need to land to drm-misc tree. Let's not bother Daniel. :) [1] https://patchwork.kernel.org/project/dri-devel/patch/1615549385-33784-1-git-send-email-tiant...@hisilicon.com/ Thanks, Inki Dae > > Subject: [PATCH] drm/exynos/decon5433: Remove the unused include statements > > This driver doesn't reference of_gpio.h, so drop it. > > Signed-off-by: Tian Tao > --- > drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c > b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c > index 1f79bc2..1510e4e 100644 > --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c > +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c > @@ -13,7 +13,6 @@ > #include > #include > #include > -#include > #include > #include > #include > >> >> Thanks, >> Inki Dae >> >>> Signed-off-by: Tian Tao >>> --- >>> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c >>> b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c >>> index 1f79bc2..9fc51a6 100644 >>> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c >>> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c >>> @@ -11,9 +11,9 @@ >>> #include >>> #include >>> #include >>> +#include >>> #include >>> #include >>> -#include >>> #include >>> #include >>> #include >>> >> . >> > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/exynos/decon5433: Remove the unused include statements
This driver doesn't reference of_gpio.h, so drop it. Signed-off-by: Tian Tao --- drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c index 1f79bc2..1510e4e 100644 --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c @@ -13,7 +13,6 @@ #include #include #include -#include #include #include #include -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos/decon5433: Clean up GPIO includes
Hi Tian Tao, 21. 3. 2. 오후 12:03에 Tian Tao 이(가) 쓴 글: > remove the legacy GPIO headers but what it really > uses is since only gpio_desc structs are ever > referenced. This driver doesn't reference even linux/gpio/consumer.h so could you drop only of_gpio.h inclusion? Thanks, Inki Dae > > Signed-off-by: Tian Tao > --- > drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c > b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c > index 1f79bc2..9fc51a6 100644 > --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c > +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c > @@ -11,9 +11,9 @@ > #include > #include > #include > +#include > #include > #include > -#include > #include > #include > #include > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
