[Bug 110257] Major artifacts in mpeg2 vaapi hw decoding
https://bugs.freedesktop.org/show_bug.cgi?id=110257 67b02...@casix.org changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #3 from 67b02...@casix.org --- Yes indeed, this bug is even listed in the 19.0.2 release notes. It was probably just forgotten to comment here. Anyhow, thanks Boyuan Zhang for the fix, works perfectly fine now with mesa 19.0.3 installed from normal repos! -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110257] Major artifacts in mpeg2 vaapi hw decoding
https://bugs.freedesktop.org/show_bug.cgi?id=110257 --- Comment #2 from Andrew Randrianasulu --- wasn't this fixed by https://cgit.freedesktop.org/mesa/mesa/commit/?id=d507bcdcf26b417dea201090165af651253b6b11 -- st/va: reverse qt matrix back to its original order The quantiser matrix that VAAPI provides has been applied with inverse z-scan. However, what we expect in MPEG2 picture description is the original order. Therefore, we need to reverse it back to its original order. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110257 Cc: mesa-sta...@lists.freedesktop.org -- I can only test with Cinelerra-GG, and my nv92 crad decodes 1080p25 mpeg2 very slowly (video decoder clock problem) - but for me images were ok! -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 203627] [Regression] Boot fails with linux-firmware 20190514
https://bugzilla.kernel.org/show_bug.cgi?id=203627 Aleksandr Mezin (mezin.alexan...@gmail.com) changed: What|Removed |Added Regression|No |Yes -- 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 203627] New: [Regression] Boot fails with linux-firmware 20190514
https://bugzilla.kernel.org/show_bug.cgi?id=203627 Bug ID: 203627 Summary: [Regression] Boot fails with linux-firmware 20190514 Product: Drivers Version: 2.5 Kernel Version: 4.19.44 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: mezin.alexan...@gmail.com Regression: No After commit https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/commit/?id=2579167548be33afb1fe2a9a5c141561ee5a8bbe system doesn't boot anymore with kernels 4.19.x (at least 4.19.40 and 4.19.44). 5.1 boots fine. Displays turn off early when amdgpu driver loads, never turn on again. Can't reach the system over ssh. Also can't get any logs from journald (that boot attempt just doesn't get recorded) GPU: RX Vega 64 (Sapphire Nitro+) Dual 4K displays connected over DisplayPort -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/3] dt-bindings: add vendor prefix for Evervision Electronics
On Tue, Apr 23, 2019 at 7:26 AM Thierry Reding wrote: > > On Tue, Apr 16, 2019 at 12:06:43PM +0200, Marco Felsch wrote: > > Evervision Electronics is a panel manufacturer from Taipei. > > http://www.evervisionlcd.com/index.php?lang=en > > > > Signed-off-by: Marco Felsch > > Reviewed-by: Rob Herring > > --- > > Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + > > 1 file changed, 1 insertion(+) > > Applied, thanks. I've converted this file to json-schema as of v5.2-rc1. See commit 8122de54602e. Applied, but doesn't seem to be in linux-next? Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 02/18] kunit: test: add test resource management API
Quoting Brendan Higgins (2019-05-14 15:16:55) > diff --git a/kunit/test.c b/kunit/test.c > index 86f65ba2bcf92..a15e6f8c41582 100644 > --- a/kunit/test.c > +++ b/kunit/test.c [..] > + > +void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp) > +{ > + struct kunit_kmalloc_params params; > + struct kunit_resource *res; > + > + params.size = size; > + params.gfp = gfp; > + > + res = kunit_alloc_resource(test, > + kunit_kmalloc_init, > + kunit_kmalloc_free, > + ¶ms); > + > + if (res) > + return res->allocation; > + else > + return NULL; Can be written as if (res) return return and some static analysis tools prefer this. > +} > + > +void kunit_cleanup(struct kunit *test) > +{ > + struct kunit_resource *resource, *resource_safe; > + unsigned long flags; > + > + spin_lock_irqsave(&test->lock, flags); Ah ok, test->lock is protecting everything now? Does it need to be a spinlock, or can it be a mutex? > + list_for_each_entry_safe(resource, > +resource_safe, > +&test->resources, > +node) { > + kunit_free_resource(test, resource); > + } > + spin_unlock_irqrestore(&test->lock, flags); > +} > + ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 01/18] kunit: test: add KUnit test runner core
Quoting Brendan Higgins (2019-05-14 15:16:54) > diff --git a/include/kunit/test.h b/include/kunit/test.h > new file mode 100644 > index 0..e682ea0e1f9a5 > --- /dev/null > +++ b/include/kunit/test.h > @@ -0,0 +1,162 @@ [..] > +/** > + * struct kunit - represents a running instance of a test. > + * @priv: for user to store arbitrary data. Commonly used to pass data > created > + * in the init function (see &struct kunit_module). > + * > + * Used to store information about the current context under which the test > is > + * running. Most of this data is private and should only be accessed > indirectly > + * via public functions; the one exception is @priv which can be used by the > + * test writer to store arbitrary data. > + */ > +struct kunit { > + void *priv; > + > + /* private: internal use only. */ > + const char *name; /* Read only after initialization! */ > + spinlock_t lock; /* Gaurds all mutable test state. */ > + bool success; /* Protected by lock. */ Is this all the spinlock protects? Doesn't seem useful if it's just protecting access to the variable being set or not because code that reads it will have a stale view of the value. > diff --git a/kunit/test.c b/kunit/test.c > new file mode 100644 > index 0..86f65ba2bcf92 > --- /dev/null > +++ b/kunit/test.c > @@ -0,0 +1,229 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Base unit test (KUnit) API. > + * > + * Copyright (C) 2019, Google LLC. > + * Author: Brendan Higgins > + */ > + > +#include > +#include > +#include > + [...] > + > +size_t kunit_module_counter = 1; static? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] drm/rockchip: dw_hdmi: Handle suspend/resume
Hi, On Thu, May 16, 2019 at 2:40 PM Douglas Anderson wrote: > > On Rockchip rk3288-based Chromebooks when you do a suspend/resume > cycle: > > 1. You lose the ability to detect an HDMI device being plugged in. > > 2. If you're using the i2c bus built in to dw_hdmi then it stops > working. > > Let's call the core dw-hdmi's suspend/resume functions to restore > things. > > NOTE: in downstream Chrome OS (based on kernel 3.14) we used the > "late/early" versions of suspend/resume because we found that the VOP > was sometimes resuming before dw_hdmi and then calling into us before > we were fully resumed. For now I have gone back to the normal > suspend/resume because I can't reproduce the problems. > > Signed-off-by: Douglas Anderson > --- > > Changes in v2: > - Add forgotten static (Laurent) > - No empty stub for suspend (Laurent) > > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 12 > 1 file changed, 12 insertions(+) Whoops, forgot that I should have carried forward: Reviewed-by: Laurent Pinchart -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 4/5 v4] dma-buf: heaps: Add CMA heap to dmabuf heaps
On Tue, May 14, 2019 at 3:40 AM Xiaqing (A) wrote: > > > > On 2019/5/14 2:37, John Stultz wrote: > > This adds a CMA heap, which allows userspace to allocate > > a dma-buf of contiguous memory out of a CMA region. > > > > This code is an evolution of the Android ION implementation, so > > thanks to its original author and maintainters: > >Benjamin Gaignard, Laura Abbott, and others! > > > > Cc: Laura Abbott > > Cc: Benjamin Gaignard > > Cc: Sumit Semwal > > Cc: Liam Mark > > Cc: Pratik Patel > > Cc: Brian Starkey > > Cc: Vincent Donnefort > > Cc: Sudipto Paul > > Cc: Andrew F. Davis > > Cc: Xu YiPing > > Cc: "Chenfeng (puck)" > > Cc: butao > > Cc: "Xiaqing (A)" > > Cc: Yudongbin > > Cc: Christoph Hellwig > > Cc: Chenbo Feng > > Cc: Alistair Strachan > > Cc: dri-devel@lists.freedesktop.org > > Signed-off-by: John Stultz > > --- > > v2: > > * Switch allocate to return dmabuf fd > > * Simplify init code > > * Checkpatch fixups > > v3: > > * Switch to inline function for to_cma_heap() > > * Minor cleanups suggested by Brian > > * Fold in new registration style from Andrew > > * Folded in changes from Andrew to use simplified page list > >from the heap helpers > > * Use the fd_flags when creating dmabuf fd (Suggested by > >Benjamin) > > * Use precalculated pagecount (Suggested by Andrew) > > --- > > drivers/dma-buf/heaps/Kconfig| 8 ++ > > drivers/dma-buf/heaps/Makefile | 1 + > > drivers/dma-buf/heaps/cma_heap.c | 169 +++ > > 3 files changed, 178 insertions(+) > > create mode 100644 drivers/dma-buf/heaps/cma_heap.c > > > > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig > > index 205052744169..a5eef06c4226 100644 > > --- a/drivers/dma-buf/heaps/Kconfig > > +++ b/drivers/dma-buf/heaps/Kconfig > > @@ -4,3 +4,11 @@ config DMABUF_HEAPS_SYSTEM > > help > > Choose this option to enable the system dmabuf heap. The system heap > > is backed by pages from the buddy allocator. If in doubt, say Y. > > + > > +config DMABUF_HEAPS_CMA > > + bool "DMA-BUF CMA Heap" > > + depends on DMABUF_HEAPS && DMA_CMA > > + help > > + Choose this option to enable dma-buf CMA heap. This heap is backed > > + by the Contiguous Memory Allocator (CMA). If your system has these > > + regions, you should say Y here. > > diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile > > index d1808eca2581..6e54cdec3da0 100644 > > --- a/drivers/dma-buf/heaps/Makefile > > +++ b/drivers/dma-buf/heaps/Makefile > > @@ -1,3 +1,4 @@ > > # SPDX-License-Identifier: GPL-2.0 > > obj-y += heap-helpers.o > > obj-$(CONFIG_DMABUF_HEAPS_SYSTEM) += system_heap.o > > +obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o > > diff --git a/drivers/dma-buf/heaps/cma_heap.c > > b/drivers/dma-buf/heaps/cma_heap.c > > new file mode 100644 > > index ..3d0ffbbd0a34 > > --- /dev/null > > +++ b/drivers/dma-buf/heaps/cma_heap.c > > @@ -0,0 +1,169 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * DMABUF CMA heap exporter > > + * > > + * Copyright (C) 2012, 2019 Linaro Ltd. > > + * Author: for ST-Ericsson. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "heap-helpers.h" > > + > > +struct cma_heap { > > + struct dma_heap *heap; > > + struct cma *cma; > > +}; > > + > > +static void cma_heap_free(struct heap_helper_buffer *buffer) > > +{ > > + struct cma_heap *cma_heap = > > dma_heap_get_data(buffer->heap_buffer.heap); > > + unsigned long nr_pages = buffer->pagecount; > > + struct page *pages = buffer->priv_virt; > > + > > + /* free page list */ > > + kfree(buffer->pages); > > + /* release memory */ > > + cma_release(cma_heap->cma, pages, nr_pages); > > + kfree(buffer); > > +} > > + > > +/* dmabuf heap CMA operations functions */ > > +static int cma_heap_allocate(struct dma_heap *heap, > > + unsigned long len, > > + unsigned long fd_flags, > > + unsigned long heap_flags) > > +{ > > + struct cma_heap *cma_heap = dma_heap_get_data(heap); > > + struct heap_helper_buffer *helper_buffer; > > + struct page *pages; > > + size_t size = PAGE_ALIGN(len); > > + unsigned long nr_pages = size >> PAGE_SHIFT; > > + unsigned long align = get_order(size); > > + DEFINE_DMA_BUF_EXPORT_INFO(exp_info); > > + struct dma_buf *dmabuf; > > + int ret = -ENOMEM; > > + pgoff_t pg; > > + > > + if (align > CONFIG_CMA_ALIGNMENT) > > + align = CONFIG_CMA_ALIGNMENT; > > + > > + helper_buffer = kzalloc(sizeof(*helper_buffer), GFP_KERNEL); > > + if (!helper_buffer) > > + return -ENOMEM; > > + > > + INIT_HEAP_HELPER_BUFFER(helper_buffer, cma_heap_free);
[PATCH v2 2/2] drm/rockchip: dw_hdmi: Handle suspend/resume
On Rockchip rk3288-based Chromebooks when you do a suspend/resume cycle: 1. You lose the ability to detect an HDMI device being plugged in. 2. If you're using the i2c bus built in to dw_hdmi then it stops working. Let's call the core dw-hdmi's suspend/resume functions to restore things. NOTE: in downstream Chrome OS (based on kernel 3.14) we used the "late/early" versions of suspend/resume because we found that the VOP was sometimes resuming before dw_hdmi and then calling into us before we were fully resumed. For now I have gone back to the normal suspend/resume because I can't reproduce the problems. Signed-off-by: Douglas Anderson --- Changes in v2: - Add forgotten static (Laurent) - No empty stub for suspend (Laurent) drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 4cdc9f86c2e5..beffe44c248a 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -542,11 +542,23 @@ static int dw_hdmi_rockchip_remove(struct platform_device *pdev) return 0; } +static int __maybe_unused dw_hdmi_rockchip_resume(struct device *dev) +{ + struct rockchip_hdmi *hdmi = dev_get_drvdata(dev); + + return dw_hdmi_resume(hdmi->hdmi); +} + +static const struct dev_pm_ops dw_hdmi_rockchip_pm = { + SET_SYSTEM_SLEEP_PM_OPS(NULL, dw_hdmi_rockchip_resume) +}; + struct platform_driver dw_hdmi_rockchip_pltfm_driver = { .probe = dw_hdmi_rockchip_probe, .remove = dw_hdmi_rockchip_remove, .driver = { .name = "dwhdmi-rockchip", + .pm = &dw_hdmi_rockchip_pm, .of_match_table = dw_hdmi_rockchip_dt_ids, }, }; -- 2.21.0.1020.gf2820cf01a-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/7] drm/dp_mst: Register AUX devices for MST ports
So a couple of things: On Thu, 2019-05-16 at 11:17 -0400, sunpeng...@amd.com wrote: > From: Ville Syrjälä > > All available downstream ports - physical and logical - are exposed for > each MST device. They are listed in /dev/, following the same naming > scheme as SST devices by appending an incremental ID. > > Although all downstream ports are exposed, only some will work as > expected. Consider the following topology: > >+-+ >| ASIC | >+-+ > Conn-0| > | >+v+ > +| MST HUB |+ > |+-+| > | | > |Port-1 Port-2| > +-v-+ +-v-+ > | MST | | SST | > | Display | | Display | > +---+ +---+ > |Port-1 > x > > MST Path | MST Device > --+-- > sst:0 | MST Hub > mst:0-1 | MST Display > mst:0-1-1 | MST Display's disconnected DP out > mst:0-1-8 | MST Display's internal sink > mst:0-2 | SST Display > > On certain MST displays, the upstream physical port will ACK DPCD reads. > However, reads on the local logical port to the internal sink will > *NAK*. i.e. reading mst:0-1 ACKs, but mst:0-1-8 NAKs. > > There may also be duplicates. Some displays will return the same GUID > when reading DPCD from both mst:0-1 and mst:0-1-8. > > There are some device-dependent behavior as well. The MST hub used > during testing will actually *ACK* read requests on a disconnected > physical port, whereas the MST displays will NAK. > > In light of these discrepancies, it's simpler to expose all downstream > ports - both physical and logical - and let the user decide what to use. > > Cc: Lyude Paul > Signed-off-by: Ville Syrjälä > Signed-off-by: Leo Li > --- > drivers/gpu/drm/drm_dp_aux_dev.c | 14 - > drivers/gpu/drm/drm_dp_mst_topology.c | 103 + > - > include/drm/drm_dp_helper.h | 4 ++ > include/drm/drm_dp_mst_helper.h | 6 ++ > 4 files changed, 112 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c > b/drivers/gpu/drm/drm_dp_aux_dev.c > index 6d84611..01c02b9 100644 > --- a/drivers/gpu/drm/drm_dp_aux_dev.c > +++ b/drivers/gpu/drm/drm_dp_aux_dev.c > @@ -34,6 +34,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -114,6 +115,7 @@ static ssize_t name_show(struct device *dev, > > return res; > } > + > static DEVICE_ATTR_RO(name); > > static struct attribute *drm_dp_aux_attrs[] = { > @@ -160,7 +162,11 @@ static ssize_t auxdev_read_iter(struct kiocb *iocb, > struct iov_iter *to) > break; > } > > - res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo); > + if (aux_dev->aux->is_remote) > + res = drm_dp_mst_dpcd_read(aux_dev->aux, pos, buf, > todo); > + else > + res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo); > + It's still not at all clear to me why we're trying to avoid specifying actual callbacks for the aux device. We should remove this part entirely, remove the is_remote entry from struct drm_dp_aux, and then just specify our own hook in struct drm_dp_aux->transfer(). > if (res <= 0) > break; > > @@ -207,7 +213,11 @@ static ssize_t auxdev_write_iter(struct kiocb *iocb, > struct iov_iter *from) > break; > } > > - res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo); > + if (aux_dev->aux->is_remote) > + res = drm_dp_mst_dpcd_write(aux_dev->aux, pos, buf, > todo); > + else > + res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo); > + > if (res <= 0) > break; > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 2ab16c9..54da68e 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -35,6 +35,8 @@ > #include > #include > > +#include "drm_crtc_helper_internal.h" > + Unless I'm mistaken, this looks like some sort of ditritus. > /** > * DOC: dp mst helper > * > @@ -52,6 +54,9 @@ static int drm_dp_dpcd_write_payload(struct > drm_dp_mst_topology_mgr *mgr, >int id, >struct drm_dp_payload *payload); > > +static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr, > + struct drm_dp_mst_port *port, > + int offset, int size, u8 *bytes); > static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr, > struct drm_dp_mst_port *port, >
[PATCH v2 1/2] drm: bridge: dw-hdmi: Add hook for resume
On Rockchip rk3288-based Chromebooks when you do a suspend/resume cycle: 1. You lose the ability to detect an HDMI device being plugged in. 2. If you're using the i2c bus built in to dw_hdmi then it stops working. Let's add a hook to the core dw-hdmi driver so that we can call it in dw_hdmi-rockchip in the next commit. NOTE: the exact set of steps I've done here in resume come from looking at the normal dw_hdmi init sequence in upstream Linux plus the sequence that we did in downstream Chrome OS 3.14. Testing show that it seems to work, but if an extra step is needed or something here is not needed we could improve it. As part of this change we'll refactor the hardware init bits of dw-hdmi to happen all in one function and all at the same time. Since we need to init the interrupt mutes before we request the IRQ, this means moving the hardware init earlier in the function, but there should be no problems with that. Also as part of this we now unconditionally init the "i2c" parts of dw-hdmi, but again that ought to be fine. Signed-off-by: Douglas Anderson --- Changes in v2: - No empty stub for suspend (Laurent) - Refactor to use the same code in probe and resume (Laurent) - Unconditionally init i2c (seems OK + needed before hdmi->i2c init) - Combine "init" of i2c and "setup" of i2c (no reason to split) drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 50 ++- include/drm/bridge/dw_hdmi.h | 2 + 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ab7968c8f6a2..636d55d1398c 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -227,6 +227,13 @@ static void hdmi_mask_writeb(struct dw_hdmi *hdmi, u8 data, unsigned int reg, static void dw_hdmi_i2c_init(struct dw_hdmi *hdmi) { + hdmi_writeb(hdmi, HDMI_PHY_I2CM_INT_ADDR_DONE_POL, + HDMI_PHY_I2CM_INT_ADDR); + + hdmi_writeb(hdmi, HDMI_PHY_I2CM_CTLINT_ADDR_NAC_POL | + HDMI_PHY_I2CM_CTLINT_ADDR_ARBITRATION_POL, + HDMI_PHY_I2CM_CTLINT_ADDR); + /* Software reset */ hdmi_writeb(hdmi, 0x00, HDMI_I2CM_SOFTRSTZ); @@ -1925,16 +1932,6 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode) return 0; } -static void dw_hdmi_setup_i2c(struct dw_hdmi *hdmi) -{ - hdmi_writeb(hdmi, HDMI_PHY_I2CM_INT_ADDR_DONE_POL, - HDMI_PHY_I2CM_INT_ADDR); - - hdmi_writeb(hdmi, HDMI_PHY_I2CM_CTLINT_ADDR_NAC_POL | - HDMI_PHY_I2CM_CTLINT_ADDR_ARBITRATION_POL, - HDMI_PHY_I2CM_CTLINT_ADDR); -} - static void initialize_hdmi_ih_mutes(struct dw_hdmi *hdmi) { u8 ih_mute; @@ -2435,6 +2432,21 @@ static const struct regmap_config hdmi_regmap_32bit_config = { .max_register = HDMI_I2CM_FS_SCL_LCNT_0_ADDR << 2, }; +static void dw_hdmi_init_hw(struct dw_hdmi *hdmi) +{ + initialize_hdmi_ih_mutes(hdmi); + + /* +* Reset HDMI DDC I2C master controller and mute I2CM interrupts. +* Even if we are using a separate i2c adapter doing this doesn't +* hurt. +*/ + dw_hdmi_i2c_init(hdmi); + + if (hdmi->phy.ops->setup_hpd) + hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data); +} + static struct dw_hdmi * __dw_hdmi_probe(struct platform_device *pdev, const struct dw_hdmi_plat_data *plat_data) @@ -2586,7 +2598,7 @@ __dw_hdmi_probe(struct platform_device *pdev, prod_id1 & HDMI_PRODUCT_ID1_HDCP ? "with" : "without", hdmi->phy.name); - initialize_hdmi_ih_mutes(hdmi); + dw_hdmi_init_hw(hdmi); irq = platform_get_irq(pdev, 0); if (irq < 0) { @@ -2625,10 +2637,6 @@ __dw_hdmi_probe(struct platform_device *pdev, hdmi->bridge.of_node = pdev->dev.of_node; #endif - dw_hdmi_setup_i2c(hdmi); - if (hdmi->phy.ops->setup_hpd) - hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data); - memset(&pdevinfo, 0, sizeof(pdevinfo)); pdevinfo.parent = dev; pdevinfo.id = PLATFORM_DEVID_AUTO; @@ -2681,10 +2689,6 @@ __dw_hdmi_probe(struct platform_device *pdev, hdmi->cec = platform_device_register_full(&pdevinfo); } - /* Reset HDMI DDC I2C master controller and mute I2CM interrupts */ - if (hdmi->i2c) - dw_hdmi_i2c_init(hdmi); - return hdmi; err_iahb: @@ -2788,6 +2792,14 @@ void dw_hdmi_unbind(struct dw_hdmi *hdmi) } EXPORT_SYMBOL_GPL(dw_hdmi_unbind); +int dw_hdmi_resume(struct dw_hdmi *hdmi) +{ + dw_hdmi_init_hw(hdmi); + + return 0; +} +EXPORT_SYMBOL_GPL(dw_hdmi_resume); + MODULE_AUTHOR("Sascha Hauer "); MODULE_AUTHOR("Andy Yan "); MODULE_AUTHOR("Yakir Yang "); diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h index 66e70770cce5..1626731e1681 10064
Re: [PATCH 7/7] drm/nouveau: Use connector kdev as aux device parent
Reviewed-by: Lyude Paul On Thu, 2019-05-16 at 11:18 -0400, sunpeng...@amd.com wrote: > From: Leo Li > > Set the connector's kernel device as the parent for the aux kernel > device. This allows udev rules to access connector attributes when > creating symlinks to aux devices. > > Cc: Ben Skeggs > Cc: Lyude Paul > Signed-off-by: Leo Li > --- > drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c > b/drivers/gpu/drm/nouveau/nouveau_connector.c > index 3f463c9..738782a 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_connector.c > +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c > @@ -1345,7 +1345,7 @@ nouveau_connector_create(struct drm_device *dev, > break; > case DRM_MODE_CONNECTOR_DisplayPort: > case DRM_MODE_CONNECTOR_eDP: > - nv_connector->aux.dev = dev->dev; > + nv_connector->aux.dev = connector->kdev; > nv_connector->aux.transfer = nouveau_connector_aux_xfer; > snprintf(aux_name, sizeof(aux_name), "sor-%04x-%04x", >dcbe->hasht, dcbe->hashm); -- Cheers, Lyude Paul ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/7] drm/dp-mst: Use connector kdev as aux device parent
aghsorry, but I need to take back my Reviewed-by. Noticed an issue when reloading drm and i915. I'll explain it when I respond to patch 2/7 in a moment On Thu, 2019-05-16 at 11:17 -0400, sunpeng...@amd.com wrote: > From: Leo Li > > Placing the MST aux device as a child of the connector gives udev the > ability to access the connector device's attributes. This will come in > handy when writing udev rules to create more descriptive symlinks to the > MST aux devices. > > Cc: Ville Syrjälä > Cc: Lyude Paul > Signed-off-by: Leo Li > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 54da68e..cd2f3c4 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -1269,6 +1269,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch > *mstb, > } > (*mstb->mgr->cbs->register_connector)(port->connector); > > + if (port->connector->registration_state == > DRM_CONNECTOR_REGISTERED) > + port->aux.dev = port->connector->kdev; > + > drm_dp_aux_register_devnode(&port->aux); > } > -- Cheers, Lyude Paul ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/7] drm/dp-mst: Use connector kdev as aux device parent
On Thu, 2019-05-16 at 11:17 -0400, sunpeng...@amd.com wrote: > From: Leo Li > > Placing the MST aux device as a child of the connector gives udev the > ability to access the connector device's attributes. This will come in > handy when writing udev rules to create more descriptive symlinks to the > MST aux devices. > > Cc: Ville Syrjälä > Cc: Lyude Paul > Signed-off-by: Leo Li > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 54da68e..cd2f3c4 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -1269,6 +1269,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch > *mstb, > } > (*mstb->mgr->cbs->register_connector)(port->connector); > > + if (port->connector->registration_state == > DRM_CONNECTOR_REGISTERED) > + port->aux.dev = port->connector->kdev; > + Line wrapping please! Probably worth running checkpatch on all of these patches as well. With that nitpick addressed: Reviewed-by: Lyude Paul > drm_dp_aux_register_devnode(&port->aux); > } > -- Cheers, Lyude Paul ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/3] drm: shmem: Add drm_gem_shmem_map_offset() wrapper
On Thu, May 16, 2019 at 03:14:46PM +0100, Steven Price wrote: > Provide a wrapper for drm_gem_map_offset() for clients of shmem. This > wrapper provides the correct semantics for the drm_gem_shmem_mmap() > callback. > > Signed-off-by: Steven Price > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 20 > include/drm/drm_gem_shmem_helper.h | 2 ++ > 2 files changed, 22 insertions(+) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > b/drivers/gpu/drm/drm_gem_shmem_helper.c > index 1ee208c2c85e..9dbebc4897d1 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -400,6 +400,26 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, > struct drm_device *dev, > } > EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create); > > +/** > + * drm_gem_map_offset - return the fake mmap offset for a gem object > + * @file: drm file-private structure containing the gem object > + * @dev: corresponding drm_device > + * @handle: gem object handle > + * @offset: return location for the fake mmap offset > + * > + * This provides an offset suitable for user space to return to the > + * drm_gem_shmem_mmap() callback via an mmap() call. > + * > + * Returns: > + * 0 on success or a negative error code on failure. > + */ > +int drm_gem_shmem_map_offset(struct drm_file *file, struct drm_device *dev, > + u32 handle, u64 *offset) > +{ > + return drm_gem_map_offset(file, dev, handle, offset); > +} > +EXPORT_SYMBOL_GPL(drm_gem_shmem_map_offset); Not seeing the point of this mapper, since drm_gem_shmem_map_offset isn't speficic at all. It works for dumb, shmem, cma and private objects all equally well. I'd drop this and just directly call the underlying thing, no need to layer helpers. -Daniel > + > static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf) > { > struct vm_area_struct *vma = vmf->vma; > diff --git a/include/drm/drm_gem_shmem_helper.h > b/include/drm/drm_gem_shmem_helper.h > index 038b6d313447..4239dd4f 100644 > --- a/include/drm/drm_gem_shmem_helper.h > +++ b/include/drm/drm_gem_shmem_helper.h > @@ -128,6 +128,8 @@ drm_gem_shmem_create_with_handle(struct drm_file > *file_priv, > int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev, > struct drm_mode_create_dumb *args); > > +int drm_gem_shmem_map_offset(struct drm_file *file, struct drm_device *dev, > + u32 handle, u64 *offset); > int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma); > > extern const struct vm_operations_struct drm_gem_shmem_vm_ops; > -- > 2.20.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v2 1/3] drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset()
On Thu, May 16, 2019 at 03:14:45PM +0100, Steven Price wrote: > drm_gem_dumb_map_offset() is a useful helper for non-dumb clients, so > rename it to remove the _dumb. > > Signed-off-by: Steven Price Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- > drivers/gpu/drm/drm_gem.c | 6 +++--- > drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 +-- > include/drm/drm_gem.h | 4 ++-- > 4 files changed, 8 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dumb_buffers.c > b/drivers/gpu/drm/drm_dumb_buffers.c > index 81dfdd33753a..956665464296 100644 > --- a/drivers/gpu/drm/drm_dumb_buffers.c > +++ b/drivers/gpu/drm/drm_dumb_buffers.c > @@ -46,7 +46,7 @@ > * To support dumb objects drivers must implement the &drm_driver.dumb_create > * operation. &drm_driver.dumb_destroy defaults to drm_gem_dumb_destroy() if > * not set and &drm_driver.dumb_map_offset defaults to > - * drm_gem_dumb_map_offset(). See the callbacks for further details. > + * drm_gem_map_offset(). See the callbacks for further details. > * > * Note that dumb objects may not be used for gpu acceleration, as has been > * attempted on some ARM embedded platforms. Such drivers really must have > @@ -125,7 +125,7 @@ int drm_mode_mmap_dumb_ioctl(struct drm_device *dev, > args->handle, > &args->offset); > else > - return drm_gem_dumb_map_offset(file_priv, dev, args->handle, > + return drm_gem_map_offset(file_priv, dev, args->handle, > &args->offset); > } > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 50de138c89e0..99bb7f79a70b 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -294,7 +294,7 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) > EXPORT_SYMBOL(drm_gem_handle_delete); > > /** > - * drm_gem_dumb_map_offset - return the fake mmap offset for a gem object > + * drm_gem_map_offset - return the fake mmap offset for a gem object > * @file: drm file-private structure containing the gem object > * @dev: corresponding drm_device > * @handle: gem object handle > @@ -306,7 +306,7 @@ EXPORT_SYMBOL(drm_gem_handle_delete); > * Returns: > * 0 on success or a negative error code on failure. > */ > -int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, > +int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev, > u32 handle, u64 *offset) > { > struct drm_gem_object *obj; > @@ -332,7 +332,7 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct > drm_device *dev, > > return ret; > } > -EXPORT_SYMBOL_GPL(drm_gem_dumb_map_offset); > +EXPORT_SYMBOL_GPL(drm_gem_map_offset); > > /** > * drm_gem_dumb_destroy - dumb fb callback helper for gem based drivers > diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c > b/drivers/gpu/drm/exynos/exynos_drm_gem.c > index a55f5ac41bf3..5e3aa9e4a096 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c > @@ -276,8 +276,7 @@ int exynos_drm_gem_map_ioctl(struct drm_device *dev, void > *data, > { > struct drm_exynos_gem_map *args = data; > > - return drm_gem_dumb_map_offset(file_priv, dev, args->handle, > -&args->offset); > + return drm_gem_map_offset(file_priv, dev, args->handle, &args->offset); > } > > struct exynos_drm_gem *exynos_drm_gem_get(struct drm_file *filp, > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > index 5047c7ee25f5..91b07c2325e9 100644 > --- a/include/drm/drm_gem.h > +++ b/include/drm/drm_gem.h > @@ -395,8 +395,8 @@ int drm_gem_fence_array_add(struct xarray *fence_array, > int drm_gem_fence_array_add_implicit(struct xarray *fence_array, >struct drm_gem_object *obj, >bool write); > -int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, > - u32 handle, u64 *offset); > +int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev, > +u32 handle, u64 *offset); > int drm_gem_dumb_destroy(struct drm_file *file, >struct drm_device *dev, >uint32_t handle); > -- > 2.20.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 0/7] Add DP MST AUX devices
On 2019-05-16 3:54 p.m., Lyude Paul wrote: > [CAUTION: External Email] > > Hi, could we (and for future versions of this series and others) get a respin > of this that's actually rebased against drm-tip? That is the defacto standard > branch to do development on, and otherwise anyone trying to test these patches > has to resolve merge conflicts (along with maintainers). The branch this > appears to be based off of doesn't even have the new kref scheme for branch > devices and ports. > Sorry, this was laziness on my part :) Rebasing this now. Leo > On Thu, 2019-05-16 at 11:17 -0400, sunpeng...@amd.com wrote: >> From: Leo Li >> >> This series adds support for MST AUX devices. >> >> A more descriptive 'mstpath' attribute is also added to MST connector >> devices. In addition, the DP aux device is made to be a child of the >> corresponding connector's device where possible (*). This allows udev >> rules to provide descriptive symlinks to the AUX devices. >> >> (*) This can only be done on drivers that register their connector and >> aux devices via drm_connector_register() and drm_dp_aux_register() >> respectively. The connector also needs to be registered before the aux >> device. >> >> Leo Li (6): >>drm/dp: Use non-cyclic idr >>drm/dp-mst: Use connector kdev as aux device parent >>drm/sysfs: Add mstpath attribute to connector devices >>drm/amd/display: Use connector kdev as aux device parent >>drm/bridge/analogix-anx78xx: Use connector kdev as aux device parent >>drm/nouveau: Use connector kdev as aux device parent >> >> Ville Syrjälä (1): >>drm/dp_mst: Register AUX devices for MST ports >> >> .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 2 +- >> drivers/gpu/drm/bridge/analogix-anx78xx.c | 22 ++--- >> drivers/gpu/drm/drm_dp_aux_dev.c | 17 +++- >> drivers/gpu/drm/drm_dp_mst_topology.c | 106 >> ++--- >> drivers/gpu/drm/drm_sysfs.c| 23 + >> drivers/gpu/drm/nouveau/nouveau_connector.c| 2 +- >> include/drm/drm_dp_helper.h| 4 + >> include/drm/drm_dp_mst_helper.h| 6 ++ >> 8 files changed, 152 insertions(+), 30 deletions(-) >> > -- > Cheers, > Lyude Paul > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/7] Add DP MST AUX devices
Whoops-one more thing I forgot to mention. This is just personal preference for me, but if you're ccing me on any of the patches in the series feel free to just do it for all of them. Makes my inbox a little less confusing to look at On Thu, 2019-05-16 at 15:54 -0400, Lyude Paul wrote: > Hi, could we (and for future versions of this series and others) get a > respin > of this that's actually rebased against drm-tip? That is the defacto > standard > branch to do development on, and otherwise anyone trying to test these > patches > has to resolve merge conflicts (along with maintainers). The branch this > appears to be based off of doesn't even have the new kref scheme for branch > devices and ports. > > On Thu, 2019-05-16 at 11:17 -0400, sunpeng...@amd.com wrote: > > From: Leo Li > > > > This series adds support for MST AUX devices. > > > > A more descriptive 'mstpath' attribute is also added to MST connector > > devices. In addition, the DP aux device is made to be a child of the > > corresponding connector's device where possible (*). This allows udev > > rules to provide descriptive symlinks to the AUX devices. > > > > (*) This can only be done on drivers that register their connector and > > aux devices via drm_connector_register() and drm_dp_aux_register() > > respectively. The connector also needs to be registered before the aux > > device. > > > > Leo Li (6): > > drm/dp: Use non-cyclic idr > > drm/dp-mst: Use connector kdev as aux device parent > > drm/sysfs: Add mstpath attribute to connector devices > > drm/amd/display: Use connector kdev as aux device parent > > drm/bridge/analogix-anx78xx: Use connector kdev as aux device parent > > drm/nouveau: Use connector kdev as aux device parent > > > > Ville Syrjälä (1): > > drm/dp_mst: Register AUX devices for MST ports > > > > .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 2 +- > > drivers/gpu/drm/bridge/analogix-anx78xx.c | 22 ++--- > > drivers/gpu/drm/drm_dp_aux_dev.c | 17 +++- > > drivers/gpu/drm/drm_dp_mst_topology.c | 106 > > ++--- > > drivers/gpu/drm/drm_sysfs.c| 23 + > > drivers/gpu/drm/nouveau/nouveau_connector.c| 2 +- > > include/drm/drm_dp_helper.h| 4 + > > include/drm/drm_dp_mst_helper.h| 6 ++ > > 8 files changed, 152 insertions(+), 30 deletions(-) > > -- Cheers, Lyude Paul ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/7] Add DP MST AUX devices
Hi, could we (and for future versions of this series and others) get a respin of this that's actually rebased against drm-tip? That is the defacto standard branch to do development on, and otherwise anyone trying to test these patches has to resolve merge conflicts (along with maintainers). The branch this appears to be based off of doesn't even have the new kref scheme for branch devices and ports. On Thu, 2019-05-16 at 11:17 -0400, sunpeng...@amd.com wrote: > From: Leo Li > > This series adds support for MST AUX devices. > > A more descriptive 'mstpath' attribute is also added to MST connector > devices. In addition, the DP aux device is made to be a child of the > corresponding connector's device where possible (*). This allows udev > rules to provide descriptive symlinks to the AUX devices. > > (*) This can only be done on drivers that register their connector and > aux devices via drm_connector_register() and drm_dp_aux_register() > respectively. The connector also needs to be registered before the aux > device. > > Leo Li (6): > drm/dp: Use non-cyclic idr > drm/dp-mst: Use connector kdev as aux device parent > drm/sysfs: Add mstpath attribute to connector devices > drm/amd/display: Use connector kdev as aux device parent > drm/bridge/analogix-anx78xx: Use connector kdev as aux device parent > drm/nouveau: Use connector kdev as aux device parent > > Ville Syrjälä (1): > drm/dp_mst: Register AUX devices for MST ports > > .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 2 +- > drivers/gpu/drm/bridge/analogix-anx78xx.c | 22 ++--- > drivers/gpu/drm/drm_dp_aux_dev.c | 17 +++- > drivers/gpu/drm/drm_dp_mst_topology.c | 106 > ++--- > drivers/gpu/drm/drm_sysfs.c| 23 + > drivers/gpu/drm/nouveau/nouveau_connector.c| 2 +- > include/drm/drm_dp_helper.h| 4 + > include/drm/drm_dp_mst_helper.h| 6 ++ > 8 files changed, 152 insertions(+), 30 deletions(-) > -- Cheers, Lyude Paul ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 00/12] drm: Add self refresh helpers
Am Mittwoch, 8. Mai 2019, 18:09:05 CEST schrieb Sean Paul: > From: Sean Paul > > Another version of the SR helpers for your consumption. > > Pretty minor differences between v4 and v3: > - lots of documentation changes > - Use connector to get at crtc state in encoders > - Use the damage helpers in rockchip to fix fbdev > > Note that the rockchip patches require > e9abc611a941d4051cde1d94b2ab7473fdb50102 which is making its way through > the -fixes branches. > > PTAL > > Laurent Pinchart (1): > drm: Add drm_atomic_get_(old|new-_connector_for_encoder() helpers > > Sean Paul (10): > drm: Add atomic variants of enable/disable to encoder helper funcs > drm: Add atomic variants for bridge enable/disable > drm: Convert connector_helper_funcs->atomic_check to accept > drm_atomic_state > drm: Add helpers to kick off self refresh mode in drivers > drm/rockchip: Use dirtyfb helper > drm/rockchip: Check for fast link training before enabling psr > drm/rockchip: Use the helpers for PSR > drm/rockchip: Use vop_win in vop_win_disable instead of vop_win_data > drm/rockchip: Don't fully disable vop on self refresh > drm/rockchip: Use drm_atomic_helper_commit_tail_rpm whole series on rk3399-gru-kevin (analogix-dp with psr), rk3288-veyron-pinky (analogix-dp without psr + dw_hdmi) Tested-by: Heiko Stuebner Patch2 had a tiny bit of issue when trying to apply (surrounding code seems to have changed), but all display stuff seems to keep working, including the blinking cursor on the psr display :-) . Thanks Heiko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] arm64: DTS: allwinner: a64: enable ANX6345 bridge on Teres-I
On Thu, May 16, 2019 at 9:49 AM Torsten Duwe wrote: > > On Thu, May 16, 2019 at 09:06:41AM -0700, Vasily Khoruzhick wrote: > > > > Driver can talk to the panel over AUX channel only after t1+t3, t1 is > > up to 10ms, t3 is up to 200ms. > > This is after power-on. The boot loader needs to deal with this. Actually panel driver has to deal with it and not bootloader. > > It works with older version of driver > > that keeps panel always on because it takes a while between driver > > probe and pipeline start. > > No lid switch, no USB, no WiFi, no MMC. If you disable DCDC1 you'll > run out of wakeup-sources ;-) IOW: I see no practical way any OS > driver can switch this panel voltage off and survive... Ouch, looks like someone made a huge mistake in HW design? > > All in all - you don't need panel timings since there's EDID but you > > still need panel delays. Anyway, it's up to you and maintainers. > > Let's give it a try. > > Torsten >
Re: [PATCH v2 1/3] dt-bindings: gpu: add #cooling-cells property to the ARM Mali Midgard GPU binding
Hi, On Thu, May 16, 2019 at 10:25 AM Matthias Kaehlcke wrote: > The GPU can be used as a thermal cooling device, add an optional > '#cooling-cells' property. > > Signed-off-by: Matthias Kaehlcke > --- > Changes in v2: > - patch added to the series > --- > Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt > b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt > index 18a2cde2e5f3..61fd41a20f99 100644 > --- a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt > @@ -37,6 +37,8 @@ Optional properties: > - operating-points-v2 : Refer to > Documentation/devicetree/bindings/opp/opp.txt >for details. > > +- #cooling-cells: Refer to > Documentation/devicetree/bindings/thermal/thermal.txt > + for details. > > Example for a Mali-T760: > > @@ -51,6 +53,7 @@ gpu@ffa3 { > mali-supply = <&vdd_gpu>; > operating-points-v2 = <&gpu_opp_table>; > power-domains = <&power RK3288_PD_GPU>; > + #cooling-cells = <2>; > }; You will conflict with d5ff1adb3809 ("dt-bindings: gpu: mali-midgard: Add resets property"), but it's easy to rebase. I'll leave it to whoever is going to land this to decide if they would like you to re-post or if they can handle resolving the conflict themselves. +Kevin who appears to be the one who landed the conflicting commit. With that: Reviewed-by: Douglas Anderson
Re: [PATCH v2 3/3] ARM: dts: rockchip: Use GPU as cooling device for the GPU thermal zone of the rk3288
Hi, On Thu, May 16, 2019 at 10:25 AM Matthias Kaehlcke wrote: > Currently the CPUs are used as cooling devices of the rk3288 GPU > thermal zone. The CPUs are also configured as cooling devices in the > CPU thermal zone, which indirectly helps with cooling the GPU thermal > zone, since the CPU and GPU temperatures are correlated on the rk3288. > > Configure the ARM Mali Midgard GPU as cooling device for the GPU > thermal zone instead of the CPUs. > > Signed-off-by: Matthias Kaehlcke > --- > Changes in v2: > - patch added to the series > --- > arch/arm/boot/dts/rk3288.dtsi | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) This makes sense to me unless there is some better way to model the intertwined nature of the CPU and GPU temperature. It's my understanding that the original device tree snippet was there because it was added before the gpu node existed in the device tree so the best we could do is to suggest that the cpu could cool things down. Reviewed-by: Douglas Anderson ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/3] ARM: dts: rockchip: Use GPU as cooling device for the GPU thermal zone of the rk3288
Currently the CPUs are used as cooling devices of the rk3288 GPU thermal zone. The CPUs are also configured as cooling devices in the CPU thermal zone, which indirectly helps with cooling the GPU thermal zone, since the CPU and GPU temperatures are correlated on the rk3288. Configure the ARM Mali Midgard GPU as cooling device for the GPU thermal zone instead of the CPUs. Signed-off-by: Matthias Kaehlcke --- Changes in v2: - patch added to the series --- arch/arm/boot/dts/rk3288.dtsi | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi index 14d9609f0b15..988555c5118d 100644 --- a/arch/arm/boot/dts/rk3288.dtsi +++ b/arch/arm/boot/dts/rk3288.dtsi @@ -547,10 +547,7 @@ map0 { trip = <&gpu_alert0>; cooling-device = - <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, - <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, - <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, - <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; + <&gpu THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; }; }; }; -- 2.21.0.1020.gf2820cf01a-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/7] drm/amd/display: Use connector kdev as aux device parent
On 5/16/19 11:18 AM, sunpeng...@amd.com wrote: > > From: Leo Li > > Set the connector's kernel device as the parent for the aux kernel > device. This allows udev rules to access connector attributes when > creating symlinks to aux devices. > > For example, the following udev rule: > > SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{edid}=="*", > SYMLINK+="drm_dp_aux/by-name/$id" > > Will create the following symlinks using the connector's name: > > $ ls /dev/drm_dp_aux/by-name/ > card0-DP-1 card0-DP-2 card0-DP-3 > > Cc: Ville Syrjälä > Cc: Lyude Paul > Cc: Nicholas Kazlauskas > Cc: Harry Wentland > Cc: Jerry (Fangzhi) Zuo > Signed-off-by: Leo Li Reviewed-by: Nicholas Kazlauskas No idea why it wasn't like this in the first place. Nicholas Kazlauskas > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > index a6f44a4..083fb97 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > @@ -385,7 +385,7 @@ void amdgpu_dm_initialize_dp_connector(struct > amdgpu_display_manager *dm, > struct amdgpu_dm_connector > *aconnector) > { > aconnector->dm_dp_aux.aux.name = "dmdc"; > - aconnector->dm_dp_aux.aux.dev = dm->adev->dev; > + aconnector->dm_dp_aux.aux.dev = aconnector->base.kdev; > aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer; > aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc; > > -- > 2.7.4 > > ___ > 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
[PATCH v2 1/3] dt-bindings: gpu: add #cooling-cells property to the ARM Mali Midgard GPU binding
The GPU can be used as a thermal cooling device, add an optional '#cooling-cells' property. Signed-off-by: Matthias Kaehlcke --- Changes in v2: - patch added to the series --- Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt index 18a2cde2e5f3..61fd41a20f99 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt @@ -37,6 +37,8 @@ Optional properties: - operating-points-v2 : Refer to Documentation/devicetree/bindings/opp/opp.txt for details. +- #cooling-cells: Refer to Documentation/devicetree/bindings/thermal/thermal.txt + for details. Example for a Mali-T760: @@ -51,6 +53,7 @@ gpu@ffa3 { mali-supply = <&vdd_gpu>; operating-points-v2 = <&gpu_opp_table>; power-domains = <&power RK3288_PD_GPU>; + #cooling-cells = <2>; }; gpu_opp_table: opp_table0 { -- 2.21.0.1020.gf2820cf01a-goog
[PATCH v2 2/3] ARM: dts: rockchip: Add #cooling-cells entry for rk3288 GPU
The Mali GPU of the rk3288 can be used as cooling device, add a #cooling-cells entry for it. Signed-off-by: Matthias Kaehlcke Reviewed-by: Douglas Anderson --- Changes in v2: - added Doug's 'Reviewed-by' tag --- arch/arm/boot/dts/rk3288.dtsi | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi index a024d1e7e74c..14d9609f0b15 100644 --- a/arch/arm/boot/dts/rk3288.dtsi +++ b/arch/arm/boot/dts/rk3288.dtsi @@ -1273,6 +1273,7 @@ interrupt-names = "job", "mmu", "gpu"; clocks = <&cru ACLK_GPU>; operating-points-v2 = <&gpu_opp_table>; + #cooling-cells = <2>; /* min followed by max */ power-domains = <&power RK3288_PD_GPU>; status = "disabled"; }; -- 2.21.0.1020.gf2820cf01a-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Fix drm.h uapi header for GNU/kFreeBSD
On Thursday, 2019-05-16 09:37:40 -0700, Eric Anholt wrote: > Eric Anholt writes: > > > [ Unknown signature status ] > > James Clarke writes: > > > >> Like GNU/Linux, GNU/kFreeBSD's sys/types.h does not define the uintX_t > >> types, which differs from the BSDs' headers. Thus we should include > >> stdint.h to ensure we have all the required integer types. > >> > >> Signed-off-by: James Clarke > > > > Reviewed-by: Eric Anholt > > And pushed to drm-misc-next now. Thanks Eric! James, that means you can now sync libdrm to get the fix there as well :) See include/drm/README for details ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 7/7] add syncobj timeline tests v3
On Thu, May 16, 2019 at 6:33 PM Michel Dänzer wrote: > On 2019-05-16 2:47 p.m., Daniel Vetter wrote: > > On Thu, May 16, 2019 at 2:46 PM Michel Dänzer wrote: > >> On 2019-05-16 12:09 p.m., Christian König wrote: > >>> Am 16.05.19 um 10:16 schrieb zhoucm1: > I was able to push changes to libdrm, but now seems after libdrm is > migrated to gitlab, I cannot yet. What step do I need to get back my > permission? I already can login into gitlab with old freedesktop account. > > @Christian, Can you help submit this patch set to libdrm first? > >>> > >>> Done. > >> > >> This broke amdgpu-symbol-check: > >> https://gitlab.freedesktop.org/mesa/drm/pipelines/37177 > >> > >> > >> I pushed the trivial fix. Please consider using GitLab MRs, so that the > >> CI pipeline can catch issues like this before they can break the master > >> branch. > > > > Should we switch docs to recommend MR? Make it the default? I guess > > mesa hasn't made them mandatory yet, so doing that for libdrm is a bit > > jumping ahread ... > > Why can't libdrm go first? > > With Mesa, it took some effort to get the CI pipeline to finish in an > acceptable amount of time, but that doesn't seem to be an issue with > libdrm (though it could probably still be sped up somewhat, e.g. by > using pre-generated docker images as in other projects, or just by > passing -j4 to make). tbh I'm all for doing that, just didn't want to mix things up too much :-) And yeah libdrm is small enough that a quick MR-only experiment wont upset anyone if it somehow goes wrong. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2 v2] drm/doc: Allow new UAPI to be used once it's in drm-next/drm-misc-next.
Daniel Vetter writes: > On Wed, Apr 24, 2019 at 03:06:38PM -0700, Eric Anholt wrote: >> I was trying to figure out if it was permissible to merge the Mesa >> side of V3D's CSD support yet while it's in drm-misc-next but not >> drm-next, and developers on #dri-devel IRC had differing opinions of >> what the requirement was. >> >> v2: Restrict to just drm-next or drm-misc-next on airlied's request. > > Personally I think that's a bit too strict (if people want to screw up, > they will be able to anyway). But since I'm all for clearer rules where > possible, this has my support too. > > Reviewed-by: Daniel Vetter Pushed to drm-misc-next now. signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Fix drm.h uapi header for GNU/kFreeBSD
Eric Anholt writes: > [ Unknown signature status ] > James Clarke writes: > >> Like GNU/Linux, GNU/kFreeBSD's sys/types.h does not define the uintX_t >> types, which differs from the BSDs' headers. Thus we should include >> stdint.h to ensure we have all the required integer types. >> >> Signed-off-by: James Clarke > > Reviewed-by: Eric Anholt And pushed to drm-misc-next now. signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108824] Invalid handling when GL buffer is bound on one context and invalidated on another
https://bugs.freedesktop.org/show_bug.cgi?id=108824 --- Comment #7 from Baldur Karlsson --- I applied the patchset on top of latest mesa (aa040d3b3c7d068e1ece61c71770c16a54745f89) and I seem to get some rendered corruption that I don't get with the parent commit before applying the patches. It seems to only appear in RenderDoc, or at least it doesn't happen when running tiny demo programs. I can't isolate a simpler test case just now but it seems reliably reproducible and only shows up when I build with the patches applied. To repro with RenderDoc: * Download or build RenderDoc 1.4 * Build gears3d from https://github.com/gears3d/gears3d * Launch gears3d through RenderDoc, capture, open the frame * Step back and forth through the drawcalls and the texture viewer will show up with some corruption. Screenshot here: https://i.imgur.com/1Dk7diS.png -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 7/7] add syncobj timeline tests v3
On 2019-05-16 2:47 p.m., Daniel Vetter wrote: > On Thu, May 16, 2019 at 2:46 PM Michel Dänzer wrote: >> On 2019-05-16 12:09 p.m., Christian König wrote: >>> Am 16.05.19 um 10:16 schrieb zhoucm1: I was able to push changes to libdrm, but now seems after libdrm is migrated to gitlab, I cannot yet. What step do I need to get back my permission? I already can login into gitlab with old freedesktop account. @Christian, Can you help submit this patch set to libdrm first? >>> >>> Done. >> >> This broke amdgpu-symbol-check: >> https://gitlab.freedesktop.org/mesa/drm/pipelines/37177 >> >> >> I pushed the trivial fix. Please consider using GitLab MRs, so that the >> CI pipeline can catch issues like this before they can break the master >> branch. > > Should we switch docs to recommend MR? Make it the default? I guess > mesa hasn't made them mandatory yet, so doing that for libdrm is a bit > jumping ahread ... Why can't libdrm go first? With Mesa, it took some effort to get the CI pipeline to finish in an acceptable amount of time, but that doesn't seem to be an issue with libdrm (though it could probably still be sped up somewhat, e.g. by using pre-generated docker images as in other projects, or just by passing -j4 to make). -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: Add drm_gem_vram_{pin/unpin}_reserved() and convert mgag200
The new interfaces drm_gem_vram_{pin/unpin}_reserved() are variants of the GEM VRAM pin/unpin functions that do not reserve the BO during validation. The mgag200 driver requires this behavior for its cursor handling. The patch also converts the driver to use the new interfaces. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/drm_gem_vram_helper.c| 75 drivers/gpu/drm/mgag200/mgag200_cursor.c | 18 +++--- include/drm/drm_gem_vram_helper.h| 3 + 3 files changed, 88 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 8f142b810eb4..a002c03eaf4c 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -254,6 +254,47 @@ int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag) } EXPORT_SYMBOL(drm_gem_vram_pin); +/** + * drm_gem_vram_pin_reserved() - Pins a GEM VRAM object in a region. + * @gbo: the GEM VRAM object + * @pl_flag: a bitmask of possible memory regions + * + * Pinning a buffer object ensures that it is not evicted from + * a memory region. A pinned buffer object has to be unpinned before + * it can be pinned to another region. + * + * This function pins a GEM VRAM object that has already been + * reserved. Use drm_gem_vram_pin() if possible. + * + * Returns: + * 0 on success, or + * a negative error code otherwise. + */ +int drm_gem_vram_pin_reserved(struct drm_gem_vram_object *gbo, + unsigned long pl_flag) +{ + int i, ret; + struct ttm_operation_ctx ctx = { false, false }; + + if (gbo->pin_count) { + ++gbo->pin_count; + return 0; + } + + drm_gem_vram_placement(gbo, pl_flag); + for (i = 0; i < gbo->placement.num_placement; ++i) + gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT; + + ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx); + if (ret < 0) + return ret; + + gbo->pin_count = 1; + + return 0; +} +EXPORT_SYMBOL(drm_gem_vram_pin_reserved); + /** * drm_gem_vram_unpin() - Unpins a GEM VRAM object * @gbo: the GEM VRAM object @@ -285,6 +326,40 @@ int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo) } EXPORT_SYMBOL(drm_gem_vram_unpin); +/** + * drm_gem_vram_unpin_reserved() - Unpins a GEM VRAM object + * @gbo: the GEM VRAM object + * + * This function unpins a GEM VRAM object that has already been + * reserved. Use drm_gem_vram_unpin() if possible. + * + * Returns: + * 0 on success, or + * a negative error code otherwise. + */ +int drm_gem_vram_unpin_reserved(struct drm_gem_vram_object *gbo) +{ + int i, ret; + struct ttm_operation_ctx ctx = { false, false }; + + if (WARN_ON_ONCE(!gbo->pin_count)) + return 0; + + --gbo->pin_count; + if (gbo->pin_count) + return 0; + + for (i = 0; i < gbo->placement.num_placement ; ++i) + gbo->placements[i].flags &= ~TTM_PL_FLAG_NO_EVICT; + + ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx); + if (ret < 0) + return ret; + + return 0; +} +EXPORT_SYMBOL(drm_gem_vram_unpin_reserved); + /** * drm_gem_vram_push_to_system() - \ Unpins a GEM VRAM object and moves it to system memory diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c index 6c1a9d724d85..1c4fc85315a0 100644 --- a/drivers/gpu/drm/mgag200/mgag200_cursor.c +++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c @@ -23,9 +23,9 @@ static void mga_hide_cursor(struct mga_device *mdev) WREG8(MGA_CURPOSXL, 0); WREG8(MGA_CURPOSXH, 0); if (mdev->cursor.pixels_1->pin_count) - drm_gem_vram_unpin(mdev->cursor.pixels_1); + drm_gem_vram_unpin_reserved(mdev->cursor.pixels_1); if (mdev->cursor.pixels_2->pin_count) - drm_gem_vram_unpin(mdev->cursor.pixels_2); + drm_gem_vram_unpin_reserved(mdev->cursor.pixels_2); } int mga_crtc_cursor_set(struct drm_crtc *crtc, @@ -96,26 +96,28 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc, /* Move cursor buffers into VRAM if they aren't already */ if (!pixels_1->pin_count) { - ret = drm_gem_vram_pin(pixels_1, DRM_GEM_VRAM_PL_FLAG_VRAM); + ret = drm_gem_vram_pin_reserved(pixels_1, + DRM_GEM_VRAM_PL_FLAG_VRAM); if (ret) goto out1; gpu_addr = drm_gem_vram_offset(pixels_1); if (gpu_addr < 0) { - drm_gem_vram_unpin(pixels_1); + drm_gem_vram_unpin_reserved(pixels_1); goto out1; } mdev->cursor.pixels_1_gpu_addr = gpu_addr; } if (!pixels_2->pin_count) { - ret = drm_gem_vram_pin(pixels_2, DRM_GEM_VRAM
[PATCH 0/2] Add BO reservation to GEM VRAM pin/unpin/push_to_system
A kernel test bot reported a problem with the locktorture testcase that was triggered by the GEM VRAM helpers. ... [ 10.004734] RIP: 0010:ttm_bo_validate+0x41/0x141 [ttm] ... [ 10.015669] ? kvm_sched_clock_read+0x5/0xd [ 10.016157] ? get_lock_stats+0x11/0x3f [ 10.016607] drm_gem_vram_pin+0x77/0xa2 [drm_vram_helper] [ 10.017229] drm_gem_vram_driver_gem_prime_vmap+0xe/0x39 [drm_vram_helper] [ 10.018015] drm_gem_vmap+0x36/0x43 [drm] [ 10.018491] drm_client_framebuffer_create+0xc6/0x1ca [drm] [ 10.019143] drm_fb_helper_generic_probe+0x4c/0x157 [drm_kms_helper] [ 10.019864] __drm_fb_helper_initial_config_and_unlock+0x307/0x442 [drm_kms_helper] [ 10.020739] drm_fbdev_client_hotplug+0xc8/0x115 [drm_kms_helper] [ 10.021442] drm_fbdev_generic_setup+0xc4/0xf1 [drm_kms_helper] [ 10.022120] bochs_pci_probe+0x123/0x143 [bochs_drm] [ 10.022688] local_pci_probe+0x34/0x75 [ 10.023127] pci_device_probe+0xf8/0x150A ... It turns out that the bochs and vbox drivers automatically reserved and unreserved the BO from within their pin and unpin functions. The other drivers; ast, hibmc and mgag200; performed reservation explicitly. With the GEM VRAM conversion, automatic BO reservation within pin and unpin functions accidentally got lost. So for bochs and vbox, ttm_bo_validate() worked on unlocked BOs. This patch set fixes the problem by adding automatic reservation to the implementation of drm_gem_vram_{pin,unpin,push_to_system}() to fix bochs and vbox. It removes explicit BO reservation around the pin, unpin and push-to-system calls in the ast, hibmc and mgag200 drivers. The only exception is the cursor handling of mgag200. In this case, the mgag200 driver now calls drm_gem_vram_{pin,unpin}_reserved(), which works with reserved BOs. The respective code should be refactored in a future patch to work with the regular pin and unpin functions. Thomas Zimmermann (2): drm: Add drm_gem_vram_{pin/unpin}_reserved() and convert mgag200 drm: Reserve/unreserve GEM VRAM BOs from within pin/unpin functions drivers/gpu/drm/ast/ast_mode.c| 24 +--- drivers/gpu/drm/drm_gem_vram_helper.c | 115 +- .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 6 - .../gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 17 +-- drivers/gpu/drm/mgag200/mgag200_cursor.c | 18 +-- drivers/gpu/drm/mgag200/mgag200_mode.c| 19 +-- include/drm/drm_gem_vram_helper.h | 3 + 7 files changed, 126 insertions(+), 76 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm: Reserve/unreserve GEM VRAM BOs from within pin/unpin functions
The original bochs and vbox implementations of pin and unpin functions automatically reserved BOs during validation. This functionality got lost while converting the code to a generic implementation. This may result in validating unlocked TTM BOs. Adding the reserve and unreserve operations to GEM VRAM's pin and unpin functions fixes the bochs and vbox drivers. Additionally the patch changes the mgag200, ast and hibmc drivers to not reserve BOs by themselves. Signed-off-by: Thomas Zimmermann Fixes: a3232987fdbf0bede92a9d7c7e2db99a5084d31b ("drm/bochs: Convert [...]") Reported-by: kernel test robot --- drivers/gpu/drm/ast/ast_mode.c| 24 + drivers/gpu/drm/drm_gem_vram_helper.c | 54 ++- .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 6 --- .../gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 17 +- drivers/gpu/drm/mgag200/mgag200_mode.c| 19 +-- 5 files changed, 45 insertions(+), 75 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 3475591a22c3..9aca9135a5cc 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -539,24 +539,16 @@ static int ast_crtc_do_set_base(struct drm_crtc *crtc, ast_fb = to_ast_framebuffer(fb); obj = ast_fb->obj; gbo = drm_gem_vram_of_gem(obj); - ret = drm_gem_vram_reserve(gbo, false); - if (ret) - return ret; drm_gem_vram_push_to_system(gbo); - drm_gem_vram_unreserve(gbo); } ast_fb = to_ast_framebuffer(crtc->primary->fb); obj = ast_fb->obj; gbo = drm_gem_vram_of_gem(obj); - ret = drm_gem_vram_reserve(gbo, false); - if (ret) - return ret; - ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM); if (ret) - goto err_drm_gem_vram_unreserve; + return ret; gpu_addr = drm_gem_vram_offset(gbo); if (gpu_addr < 0) { ret = (int)gpu_addr; @@ -573,7 +565,6 @@ static int ast_crtc_do_set_base(struct drm_crtc *crtc, ast_fbdev_set_base(ast, gpu_addr); } } - drm_gem_vram_unreserve(gbo); ast_set_offset_reg(crtc); ast_set_start_address_crt1(crtc, (u32)gpu_addr); @@ -582,8 +573,6 @@ static int ast_crtc_do_set_base(struct drm_crtc *crtc, err_drm_gem_vram_unpin: drm_gem_vram_unpin(gbo); -err_drm_gem_vram_unreserve: - drm_gem_vram_unreserve(gbo); return ret; } @@ -630,8 +619,6 @@ static int ast_crtc_mode_set(struct drm_crtc *crtc, static void ast_crtc_disable(struct drm_crtc *crtc) { - int ret; - DRM_DEBUG_KMS("\n"); ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); if (crtc->primary->fb) { @@ -639,11 +626,7 @@ static void ast_crtc_disable(struct drm_crtc *crtc) struct drm_gem_object *obj = ast_fb->obj; struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(obj); - ret = drm_gem_vram_reserve(gbo, false); - if (ret) - return; drm_gem_vram_push_to_system(gbo); - drm_gem_vram_unreserve(gbo); } crtc->primary->fb = NULL; } @@ -939,12 +922,7 @@ static int ast_cursor_init(struct drm_device *dev) if (ret) return ret; gbo = drm_gem_vram_of_gem(obj); - ret = drm_gem_vram_reserve(gbo, false); - if (unlikely(ret != 0)) - goto fail; - ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM); - drm_gem_vram_unreserve(gbo); if (ret) goto fail; gpu_addr = drm_gem_vram_offset(gbo); diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index a002c03eaf4c..bde8237e8021 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -235,10 +235,12 @@ int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag) int i, ret; struct ttm_operation_ctx ctx = { false, false }; - if (gbo->pin_count) { - ++gbo->pin_count; - return 0; - } + ret = ttm_bo_reserve(&gbo->bo, true, false, NULL); + if (ret < 0) + return ret; + + if (gbo->pin_count) + goto out; drm_gem_vram_placement(gbo, pl_flag); for (i = 0; i < gbo->placement.num_placement; ++i) @@ -246,11 +248,17 @@ int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag) ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx); if (ret < 0) - return ret; + goto err_ttm_bo_unreserve; - gbo->pin_count = 1; +out: + ++gbo->pin_count; + ttm_bo_unreserve(&gbo->bo); return 0; + +err_ttm_bo_unreserve: + ttm_bo_unreserve(&gbo->bo); +
Re: [PATCH v2] drm/bridge: Remove duplicate header
Hello Sabyasachi, Thank you for the patch. On Thu, May 16, 2019 at 08:55:56PM +0530, Sabyasachi Gupta wrote: > Remove duplicate header which is included twice > > Signed-off-by: Sabyasachi Gupta Reviewed-by: Laurent Pinchart > --- > v2: rebased the code against drm -next and arranged the headers alphabetically > > drivers/gpu/drm/bridge/panel.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > index 38eeaf8..000ba7c 100644 > --- a/drivers/gpu/drm/bridge/panel.c > +++ b/drivers/gpu/drm/bridge/panel.c > @@ -9,13 +9,12 @@ > */ > > #include > -#include > #include > #include > #include > #include > -#include > #include > +#include > > struct panel_bridge { > struct drm_bridge bridge; -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 0/4] drm/vc4: Binner BO management improvements
Paul Kocialkowski writes: > Changes sinve v8: > * Added collected Reviewed-by; > * Fixed up another problematic case as discussed on v8. I think this is ready to go. Thanks! signature.asc Description: PGP signature
Re: [PATCH 4/4] arm64: DTS: allwinner: a64: enable ANX6345 bridge on Teres-I
On Thu, May 16, 2019 at 8:48 AM Torsten Duwe wrote: > > On Wed, May 15, 2019 at 08:08:57AM -0700, Vasily Khoruzhick wrote: > > On Wed, May 15, 2019 at 12:32 AM Torsten Duwe wrote: > > > > > > It does comply with the bindings. The ports are all optional. > > > As far as DT is concerned, the signal path ends here. This is also the > > > final component _required_ to get the Linux kernel DRI up and running. > > > > Ugh, then bindings should be fixed. It's not optional. It may work if > > bootloader enables power for you, but it won't if you disable display > > driver in u-boot. > > I double-checked. On the Teres-I, mentioning the panel _is_ optional. It's not. See power on sequence in https://www.olimex.com/Products/DIY-Laptop/SPARE-PARTS/TERES-015-LCD11-6/resources/N116BGE-EA2.pdf Driver can talk to the panel over AUX channel only after t1+t3, t1 is up to 10ms, t3 is up to 200ms. It works with older version of driver that keeps panel always on because it takes a while between driver probe and pipeline start. It'll likely break with newer version of driver that turns on panel only when bridge is active. You'll see AUX timeouts - it won't be able to probe EDID in some cases. Problem can be intermittent and device dependent. All in all - you don't need panel timings since there's EDID but you still need panel delays. Anyway, it's up to you and maintainers. > PD23 powers down panel and backlight as much as possible, see > 24bd5d2cb93bc arm64: dts: allwinner: a64: teres-i: enable backlight > (currently only in Maxime's repo) and the Teres-I schematics... > > And the driver in your repo neatly guards all accesses with > "if (anx6345->panel)" -- good! > But I found the Vdds are required, so I added them as such. > > > I guess you're testing it with older version of anx6345. Newer version > > that supports power management [1] needs startup delay for panel. > > Another issue that you're seeing is that backlight is not disabled on > > DPMS events. All in all, you need to describe panel in dts. > > > > [1] > > https://github.com/anarsoul/linux-2.6/commit/2fbf9c242419c8bda698e8331a02d4312143ae2c > > > > Should I also have added a Tested-by: ? ;-) > > > > I don't have Teres, so I haven't tested these. > > *I* have one, and this works. I'll retest with your newer driver, > just in case. Nonetheless, the changes in this series should be fine. > Sending out v2 in a moment... > > Torsten > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #5 from tempel.jul...@gmail.com --- Yes, it also happens with Linux 5.1. It btw. runs fine on xwayland inside a Plasma Wayland session. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/4] arm64: DTS: allwinner: a64: Enable audio on Teres-I
From: Harald Geyer The TERES-I has internal speakers (left, right), internal microphone and a headset combo jack (headphones + mic), "CTIA" (android) pinout. The headphone and mic detect lines of the A64 are connected properly, but AFAIK currently unsupported by the driver. Signed-off-by: Harald Geyer Signed-off-by: Torsten Duwe --- .../boot/dts/allwinner/sun50i-a64-teres-i.dts | 53 +++ 1 file changed, 53 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts index f9eede0a8bd3..d57049fbdaca 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts @@ -70,6 +70,25 @@ compatible = "mmc-pwrseq-simple"; reset-gpios = <&r_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2 */ }; + + speaker_amp: audio-amplifier { + compatible = "simple-audio-amplifier"; + enable-gpios = <&r_pio 0 12 GPIO_ACTIVE_HIGH>; /* PL12 */ + sound-name-prefix = "Speaker Amp"; + }; +}; + +&codec { + status = "okay"; +}; + +&codec_analog { + cpvdd-supply = <®_eldo1>; + status = "okay"; +}; + +&dai { + status = "okay"; }; &ehci1 { @@ -258,6 +286,29 @@ vcc-hdmi-supply = <®_dldo1>; }; +&sound { + simple-audio-card,aux-devs = <&codec_analog>, <&speaker_amp>; + simple-audio-card,widgets = "Headphone", "Headphone Jack", + "Microphone", "Headset Microphone", + "Microphone", "Internal Microphone", + "Speaker", "Internal Speaker"; + simple-audio-card,routing = + "Left DAC", "AIF1 Slot 0 Left", + "Right DAC", "AIF1 Slot 0 Right", + "AIF1 Slot 0 Left ADC", "Left ADC", + "AIF1 Slot 0 Right ADC", "Right ADC", + "Headphone Jack", "HP", + "Speaker Amp INL", "LINEOUT", + "Speaker Amp INR", "LINEOUT", + "Internal Speaker", "Speaker Amp OUTL", + "Internal Speaker", "Speaker Amp OUTR", + "Internal Microphone", "MBIAS", + "MIC1", "Internal Microphone", + "Headset Microphone", "HBIAS", + "MIC2", "Headset Microphone"; + status = "okay"; +}; + &uart0 { pinctrl-names = "default"; pinctrl-0 = <&uart0_pb_pins>;
[PATCH v2 1/4] arm64: DTS: allwinner: a64: Add pinmux for RGB666 LCD
From: Icenowy Zheng Allwinner A64's TCON0 can output RGB666 LCD signal. Add its pinmux. Signed-off-by: Icenowy Zheng Signed-off-by: Vasily Khoruzhick Signed-off-by: Torsten Duwe --- arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 + 1 file changed, 9 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi index 2abb335145a6..a8bbee84e7da 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi @@ -559,6 +559,16 @@ function = "i2c1"; }; + /omit-if-no-ref/ + lcd_rgb666_pins: lcd-rgb666-pins { + pins = "PD0", "PD1", "PD2", "PD3", "PD4", + "PD5", "PD6", "PD7", "PD8", "PD9", + "PD10", "PD11", "PD12", "PD13", + "PD14", "PD15", "PD16", "PD17", + "PD18", "PD19", "PD20", "PD21"; + function = "lcd0"; + }; + mmc0_pins: mmc0-pins { pins = "PF0", "PF1", "PF2", "PF3", "PF4", "PF5";
[PATCH v2 4/4] arm64: DTS: allwinner: a64: enable ANX6345 bridge on Teres-I
From: Icenowy Zheng TERES-I has an ANX6345 bridge connected to the RGB666 LCD output, and the I2C controlling signals are connected to I2C0 bus. Enable it in the device tree. Signed-off-by: Icenowy Zheng Signed-off-by: Torsten Duwe --- originally: patchwork.kernel.org/patch/10646867 Changed the reset polarity, which is active low, according to the (terse) datasheet, Teres-I and pinebook schematics, and the confusing parts of the linux driver code (not yet included here). Active low -> no more confusion. --- .../boot/dts/allwinner/sun50i-a64-teres-i.dts | 40 +-- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts index c455b24dd079..bc1d0d6c0672 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts @@ -72,20 +72,38 @@ }; }; +&de { + status = "okay"; +}; + &ehci1 { status = "okay"; }; -/* The ANX6345 eDP-bridge is on i2c0. There is no linux (mainline) - * driver for this chip at the moment, the bootloader initializes it. - * However it can be accessed with the i2c-dev driver from user space. - */ &i2c0 { clock-frequency = <10>; pinctrl-names = "default"; pinctrl-0 = <&i2c0_pins>; status = "okay"; + + anx6345: anx6345@38 { + compatible = "analogix,anx6345"; + reg = <0x38>; + reset-gpios = <&pio 3 24 GPIO_ACTIVE_LOW>; /* PD24 */ + dvdd25-supply = <®_dldo2>; + dvdd12-supply = <®_dldo3>; + + port { + anx6345_in: endpoint { + remote-endpoint = <&tcon0_out_anx6345>; + }; + }; + }; +}; + +&mixer0 { + status = "okay"; }; &mmc0 { @@ -258,6 +276,20 @@ vcc-hdmi-supply = <®_dldo1>; }; +&tcon0 { + pinctrl-names = "default"; + pinctrl-0 = <&lcd_rgb666_pins>; + + status = "okay"; +}; + +&tcon0_out { + tcon0_out_anx6345: endpoint@0 { + reg = <0>; + remote-endpoint = <&anx6345_in>; + }; +}; + &uart0 { pinctrl-names = "default"; pinctrl-0 = <&uart0_pb_pins>;
[PATCH v2 2/4] dt-bindings: Add ANX6345 DP/eDP transmitter binding
From: Icenowy Zheng The ANX6345 is an ultra-low power DisplayPort/eDP transmitter designed for portable devices. Add a binding document for it. Signed-off-by: Icenowy Zheng Signed-off-by: Vasily Khoruzhick Reviewed-by: Rob Herring Signed-off-by: Torsten Duwe --- .../bindings/display/bridge/anx6345.txt | 56 +++ 1 file changed, 56 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/anx6345.txt diff --git a/Documentation/devicetree/bindings/display/bridge/anx6345.txt b/Documentation/devicetree/bindings/display/bridge/anx6345.txt new file mode 100644 index ..e79a11348d11 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/anx6345.txt @@ -0,0 +1,56 @@ +Analogix ANX6345 eDP Transmitter + + +The ANX6345 is an ultra-low power Full-HD eDP transmitter designed for +portable devices. + +Required properties: + + - compatible : "analogix,anx6345" + - reg : I2C address of the device + - reset-gpios : Which GPIO to use for reset + - dvdd12-supply : Regulator for 1.2V digital core power. + - dvdd25-supply : Regulator for 2.5V digital core power. + +Optional properties: + + - Video ports for RGB input and eDP output using the DT bindings + defined in [1] + +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt + +Example: + +anx6345: anx6345@38 { + compatible = "analogix,anx6345"; + reg = <0x38>; + reset-gpios = <&pio 3 24 GPIO_ACTIVE_LOW>; /* PD24 */ + dvdd25-supply = <®_dldo2>; + dvdd12-supply = <®_fldo1>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + anx6345_in: port@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + anx6345_in_tcon0: endpoint@0 { + reg = <0>; + remote-endpoint = <&tcon0_out_anx6345>; + }; + }; + + anx6345_out: port@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + + anx6345_out_panel: endpoint@0 { + reg = <0>; + remote-endpoint = <&panel_in_edp>; + }; + }; + }; +};
[PATCH v2 0/4] Add missing device nodes for Olimex Teres-I
Hi all, based on Maxime's sunxi-dt64-for-5.2, here is what I found so far still missing in the device tree. Those bits and pieces have already been submitted but were not yet applied. Changes since v1: * lcd-rgb666-pins - * dvdd12-supply, dvdd25-supply now are required by the anx6345 bindings * updated Harald's commit message, removing the ref to the now-deleted debug pin and added a "CTIA" (android) pinout mention. * removed the refs to the old patchwork Torsten
Re: [PATCH 4/4] arm64: DTS: allwinner: a64: enable ANX6345 bridge on Teres-I
On Wed, May 15, 2019 at 08:08:57AM -0700, Vasily Khoruzhick wrote: > On Wed, May 15, 2019 at 12:32 AM Torsten Duwe wrote: > > > > It does comply with the bindings. The ports are all optional. > > As far as DT is concerned, the signal path ends here. This is also the > > final component _required_ to get the Linux kernel DRI up and running. > > Ugh, then bindings should be fixed. It's not optional. It may work if > bootloader enables power for you, but it won't if you disable display > driver in u-boot. I double-checked. On the Teres-I, mentioning the panel _is_ optional. PD23 powers down panel and backlight as much as possible, see 24bd5d2cb93bc arm64: dts: allwinner: a64: teres-i: enable backlight (currently only in Maxime's repo) and the Teres-I schematics... And the driver in your repo neatly guards all accesses with "if (anx6345->panel)" -- good! But I found the Vdds are required, so I added them as such. > I guess you're testing it with older version of anx6345. Newer version > that supports power management [1] needs startup delay for panel. > Another issue that you're seeing is that backlight is not disabled on > DPMS events. All in all, you need to describe panel in dts. > > [1] > https://github.com/anarsoul/linux-2.6/commit/2fbf9c242419c8bda698e8331a02d4312143ae2c > > Should I also have added a Tested-by: ? ;-) > > I don't have Teres, so I haven't tested these. *I* have one, and this works. I'll retest with your newer driver, just in case. Nonetheless, the changes in this series should be fine. Sending out v2 in a moment... Torsten
Re: [PATCH v2 0/3] drm/panfrost: drm_gem_map_offset() helper
Providing maintainers more aware of the substance review it and ok it, patches 1-2 are: Acked-by: Alyssa Rosenzweig Patch 3 should be: Reviewed-by: Alyssa Rosenzweig
Re: [PATCH v5 00/11] drm/fb-helper: Move modesetting code to drm_client
Hi Noralf. > > drm/fb-helper: Remove drm_fb_helper_crtc > > drm/fb-helper: Prepare to move out commit code > > drm/fb-helper: Move out commit code > > drm/fb-helper: Remove drm_fb_helper_connector > > Patches 5-8 are still in need of review... With the improved changelogs the remaining patches are all good to go as far as I am concerned. (Not the bootsplash hack - but thats kind of obvious) Nice series! Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 10/11] drm/fb-helper: Move out modeset config code
Hi Noralf. After clarifying patch 8 this looks good (moved code touched n patch 8). So I consider this: Reviewed-by: Sam Ravnborg ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 08/11] drm/fb-helper: Remove drm_fb_helper_connector
Hi Noralf. On Thu, May 16, 2019 at 03:53:07PM +0200, Noralf Trønnes wrote: > > > Den 16.05.2019 15.07, skrev Sam Ravnborg: > > Hi Noralf. > > > > See few comments in the following. > > > > Sam > > > > On Mon, May 06, 2019 at 08:01:36PM +0200, Noralf Trønnes wrote: > >> All drivers add all their connectors so there's no need to keep around an > >> array of available connectors. > > I could expand on this text a little: > > All drivers add all their connectors so there's no need to keep around an > array of available connectors. Instead we just put the useable (not > writeback) connectors in a temporary array using > drm_client_for_each_connector_iter() everytime we probe the outputs. > Other places where it's necessary to look at the connectors, we just > iterate over them using the same iterator function. Better, thanks. After you clarified things looks good to me and you can add my: Reviewed-by: Sam Ravnborg ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/7] drm/dp_mst: Register AUX devices for MST ports
From: Ville Syrjälä All available downstream ports - physical and logical - are exposed for each MST device. They are listed in /dev/, following the same naming scheme as SST devices by appending an incremental ID. Although all downstream ports are exposed, only some will work as expected. Consider the following topology: +-+ | ASIC | +-+ Conn-0| | +v+ +| MST HUB |+ |+-+| | | |Port-1 Port-2| +-v-+ +-v-+ | MST | | SST | | Display | | Display | +---+ +---+ |Port-1 x MST Path | MST Device --+-- sst:0 | MST Hub mst:0-1 | MST Display mst:0-1-1 | MST Display's disconnected DP out mst:0-1-8 | MST Display's internal sink mst:0-2 | SST Display On certain MST displays, the upstream physical port will ACK DPCD reads. However, reads on the local logical port to the internal sink will *NAK*. i.e. reading mst:0-1 ACKs, but mst:0-1-8 NAKs. There may also be duplicates. Some displays will return the same GUID when reading DPCD from both mst:0-1 and mst:0-1-8. There are some device-dependent behavior as well. The MST hub used during testing will actually *ACK* read requests on a disconnected physical port, whereas the MST displays will NAK. In light of these discrepancies, it's simpler to expose all downstream ports - both physical and logical - and let the user decide what to use. Cc: Lyude Paul Signed-off-by: Ville Syrjälä Signed-off-by: Leo Li --- drivers/gpu/drm/drm_dp_aux_dev.c | 14 - drivers/gpu/drm/drm_dp_mst_topology.c | 103 +- include/drm/drm_dp_helper.h | 4 ++ include/drm/drm_dp_mst_helper.h | 6 ++ 4 files changed, 112 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c index 6d84611..01c02b9 100644 --- a/drivers/gpu/drm/drm_dp_aux_dev.c +++ b/drivers/gpu/drm/drm_dp_aux_dev.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -114,6 +115,7 @@ static ssize_t name_show(struct device *dev, return res; } + static DEVICE_ATTR_RO(name); static struct attribute *drm_dp_aux_attrs[] = { @@ -160,7 +162,11 @@ static ssize_t auxdev_read_iter(struct kiocb *iocb, struct iov_iter *to) break; } - res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo); + if (aux_dev->aux->is_remote) + res = drm_dp_mst_dpcd_read(aux_dev->aux, pos, buf, todo); + else + res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo); + if (res <= 0) break; @@ -207,7 +213,11 @@ static ssize_t auxdev_write_iter(struct kiocb *iocb, struct iov_iter *from) break; } - res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo); + if (aux_dev->aux->is_remote) + res = drm_dp_mst_dpcd_write(aux_dev->aux, pos, buf, todo); + else + res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo); + if (res <= 0) break; diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 2ab16c9..54da68e 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -35,6 +35,8 @@ #include #include +#include "drm_crtc_helper_internal.h" + /** * DOC: dp mst helper * @@ -52,6 +54,9 @@ static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr, int id, struct drm_dp_payload *payload); +static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr, +struct drm_dp_mst_port *port, +int offset, int size, u8 *bytes); static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, int offset, int size, u8 *bytes); @@ -941,6 +946,8 @@ static void drm_dp_destroy_port(struct kref *kref) struct drm_dp_mst_topology_mgr *mgr = port->mgr; if (!port->input) { + drm_dp_aux_unregister_devnode(&port->aux); + port->vcpi.num_slots = 0; kfree(port->cached_edid); @@ -1095,6 +1102,46 @@ static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port) return send_link; } +/** + * drm_dp_mst_dpcd_read() - read a series of bytes from the DPCD via sideband + * @aux: Fake sideband AUX CH + * @offset: address of the
[PATCH 1/7] drm/dp: Use non-cyclic idr
From: Leo Li In preparation for adding aux devices for DP MST, make the IDR non-cyclic. That way, hotplug cycling MST devices won't needlessly increment the minor version index. Signed-off-by: Leo Li Reviewed-by: Lyude Paul Reviewed-by: Ville Syrjälä --- drivers/gpu/drm/drm_dp_aux_dev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c index 0e4f25d..6d84611 100644 --- a/drivers/gpu/drm/drm_dp_aux_dev.c +++ b/drivers/gpu/drm/drm_dp_aux_dev.c @@ -80,8 +80,7 @@ static struct drm_dp_aux_dev *alloc_drm_dp_aux_dev(struct drm_dp_aux *aux) kref_init(&aux_dev->refcount); mutex_lock(&aux_idr_mutex); - index = idr_alloc_cyclic(&aux_idr, aux_dev, 0, DRM_AUX_MINORS, -GFP_KERNEL); + index = idr_alloc(&aux_idr, aux_dev, 0, DRM_AUX_MINORS, GFP_KERNEL); mutex_unlock(&aux_idr_mutex); if (index < 0) { kfree(aux_dev); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/7] drm/bridge/analogix-anx78xx: Use connector kdev as aux device parent
From: Leo Li Set the connector's kernel device as the parent for the aux kernel device. This allows udev rules to access connector attributes when creating symlinks to aux devices. To do so, the connector needs to be registered beforehand. Therefore, shift aux registration to be after connector registration. Cc: Enric Balletbo i Serra Cc: Nicolas Boichat Signed-off-by: Leo Li --- drivers/gpu/drm/bridge/analogix-anx78xx.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c index f8433c9..9fc8b4c 100644 --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c @@ -1018,17 +1018,6 @@ static int anx78xx_bridge_attach(struct drm_bridge *bridge) return -ENODEV; } - /* Register aux channel */ - anx78xx->aux.name = "DP-AUX"; - anx78xx->aux.dev = &anx78xx->client->dev; - anx78xx->aux.transfer = anx78xx_aux_transfer; - - err = drm_dp_aux_register(&anx78xx->aux); - if (err < 0) { - DRM_ERROR("Failed to register aux channel: %d\n", err); - return err; - } - err = drm_connector_init(bridge->dev, &anx78xx->connector, &anx78xx_connector_funcs, DRM_MODE_CONNECTOR_DisplayPort); @@ -1048,6 +1037,17 @@ static int anx78xx_bridge_attach(struct drm_bridge *bridge) anx78xx->connector.polled = DRM_CONNECTOR_POLL_HPD; + /* Register aux channel */ + anx78xx->aux.name = "DP-AUX"; + anx78xx->aux.dev = &anx78xx->connector.kdev; + anx78xx->aux.transfer = anx78xx_aux_transfer; + + err = drm_dp_aux_register(&anx78xx->aux); + if (err < 0) { + DRM_ERROR("Failed to register aux channel: %d\n", err); + return err; + } + err = drm_connector_attach_encoder(&anx78xx->connector, bridge->encoder); if (err) { -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/7] drm/dp-mst: Use connector kdev as aux device parent
From: Leo Li Placing the MST aux device as a child of the connector gives udev the ability to access the connector device's attributes. This will come in handy when writing udev rules to create more descriptive symlinks to the MST aux devices. Cc: Ville Syrjälä Cc: Lyude Paul Signed-off-by: Leo Li --- drivers/gpu/drm/drm_dp_mst_topology.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 54da68e..cd2f3c4 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1269,6 +1269,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, } (*mstb->mgr->cbs->register_connector)(port->connector); + if (port->connector->registration_state == DRM_CONNECTOR_REGISTERED) + port->aux.dev = port->connector->kdev; + drm_dp_aux_register_devnode(&port->aux); } -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/7] drm/sysfs: Add mstpath attribute to connector devices
From: Leo Li This can be used to create more descriptive symlinks for MST aux devices. Consider the following udev rule: SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{mstpath}=="?*", SYMLINK+="drm_dp_aux/by-path/$attr{mstpath}" The following symlinks will be created (depending on your MST topology): $ ls /dev/drm_dp_aux/by-path/ card0-mst:0-1 card0-mst:0-1-1 card0-mst:0-1-8 card0-mst:0-8 Cc: Ville Syrjälä Cc: Lyude Paul Signed-off-by: Leo Li --- drivers/gpu/drm/drm_sysfs.c | 23 +++ 1 file changed, 23 insertions(+) diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index ecb7b33..e000e0c 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -229,16 +229,39 @@ static ssize_t modes_show(struct device *device, return written; } +static ssize_t mstpath_show(struct device *device, + struct device_attribute *attr, + char *buf) +{ + struct drm_connector *connector = to_drm_connector(device); + ssize_t ret = 0; + char *path; + + mutex_lock(&connector->dev->mode_config.mutex); + if (!connector->path_blob_ptr) + goto unlock; + + path = connector->path_blob_ptr->data; + ret = snprintf(buf, PAGE_SIZE, "card%d-%s\n", + connector->dev->primary->index, path); + +unlock: + mutex_unlock(&connector->dev->mode_config.mutex); + return ret; +} + static DEVICE_ATTR_RW(status); static DEVICE_ATTR_RO(enabled); static DEVICE_ATTR_RO(dpms); static DEVICE_ATTR_RO(modes); +static DEVICE_ATTR_RO(mstpath); static struct attribute *connector_dev_attrs[] = { &dev_attr_status.attr, &dev_attr_enabled.attr, &dev_attr_dpms.attr, &dev_attr_modes.attr, + &dev_attr_mstpath.attr, NULL }; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/7] drm/amd/display: Use connector kdev as aux device parent
From: Leo Li Set the connector's kernel device as the parent for the aux kernel device. This allows udev rules to access connector attributes when creating symlinks to aux devices. For example, the following udev rule: SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{edid}=="*", SYMLINK+="drm_dp_aux/by-name/$id" Will create the following symlinks using the connector's name: $ ls /dev/drm_dp_aux/by-name/ card0-DP-1 card0-DP-2 card0-DP-3 Cc: Ville Syrjälä Cc: Lyude Paul Cc: Nicholas Kazlauskas Cc: Harry Wentland Cc: Jerry (Fangzhi) Zuo Signed-off-by: Leo Li --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index a6f44a4..083fb97 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -385,7 +385,7 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, struct amdgpu_dm_connector *aconnector) { aconnector->dm_dp_aux.aux.name = "dmdc"; - aconnector->dm_dp_aux.aux.dev = dm->adev->dev; + aconnector->dm_dp_aux.aux.dev = aconnector->base.kdev; aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer; aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 7/7] drm/nouveau: Use connector kdev as aux device parent
From: Leo Li Set the connector's kernel device as the parent for the aux kernel device. This allows udev rules to access connector attributes when creating symlinks to aux devices. Cc: Ben Skeggs Cc: Lyude Paul Signed-off-by: Leo Li --- drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c index 3f463c9..738782a 100644 --- a/drivers/gpu/drm/nouveau/nouveau_connector.c +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c @@ -1345,7 +1345,7 @@ nouveau_connector_create(struct drm_device *dev, break; case DRM_MODE_CONNECTOR_DisplayPort: case DRM_MODE_CONNECTOR_eDP: - nv_connector->aux.dev = dev->dev; + nv_connector->aux.dev = connector->kdev; nv_connector->aux.transfer = nouveau_connector_aux_xfer; snprintf(aux_name, sizeof(aux_name), "sor-%04x-%04x", dcbe->hasht, dcbe->hashm); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/7] Add DP MST AUX devices
From: Leo Li This series adds support for MST AUX devices. A more descriptive 'mstpath' attribute is also added to MST connector devices. In addition, the DP aux device is made to be a child of the corresponding connector's device where possible (*). This allows udev rules to provide descriptive symlinks to the AUX devices. (*) This can only be done on drivers that register their connector and aux devices via drm_connector_register() and drm_dp_aux_register() respectively. The connector also needs to be registered before the aux device. Leo Li (6): drm/dp: Use non-cyclic idr drm/dp-mst: Use connector kdev as aux device parent drm/sysfs: Add mstpath attribute to connector devices drm/amd/display: Use connector kdev as aux device parent drm/bridge/analogix-anx78xx: Use connector kdev as aux device parent drm/nouveau: Use connector kdev as aux device parent Ville Syrjälä (1): drm/dp_mst: Register AUX devices for MST ports .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 2 +- drivers/gpu/drm/bridge/analogix-anx78xx.c | 22 ++--- drivers/gpu/drm/drm_dp_aux_dev.c | 17 +++- drivers/gpu/drm/drm_dp_mst_topology.c | 106 ++--- drivers/gpu/drm/drm_sysfs.c| 23 + drivers/gpu/drm/nouveau/nouveau_connector.c| 2 +- include/drm/drm_dp_helper.h| 4 + include/drm/drm_dp_mst_helper.h| 6 ++ 8 files changed, 152 insertions(+), 30 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/bridge: Remove duplicate header
Remove duplicate header which is included twice Signed-off-by: Sabyasachi Gupta --- v2: rebased the code against drm -next and arranged the headers alphabetically drivers/gpu/drm/bridge/panel.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index 38eeaf8..000ba7c 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -9,13 +9,12 @@ */ #include -#include #include #include #include #include -#include #include +#include struct panel_bridge { struct drm_bridge bridge; -- 2.7.4
Re: [git pull] drm fixes for 5.2-rc1
The pull request you sent on Thu, 16 May 2019 12:27:54 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-next-2019-05-16 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/cc7ce90153e74f8266eefee9fba466faa1a2d5df Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit
On Thu, May 16, 2019 at 10:10 AM Tejun Heo wrote: > I haven't gone through the patchset yet but some quick comments. > > On Wed, May 15, 2019 at 10:29:21PM -0400, Kenny Ho wrote: > > Given this controller is specific to the drm kernel subsystem which > > uses minor to identify drm device, I don't see a need to complicate > > the interfaces more by having major and a key. As you can see in the > > examples below, the drm device minor corresponds to the line number. > > I am not sure how strict cgroup upstream is about the convention but I > > We're pretty strict. > > > am hoping there are flexibility here to allow for what I have > > implemented. There are a couple of other things I have done that is > > So, please follow the interface conventions. We can definitely add > new ones but that would need functional reasons. > > > not described in the convention: 1) inclusion of read-only *.help file > > at the root cgroup, 2) use read-only (which I can potentially make rw) > > *.default file instead of having a default entries (since the default > > can be different for different devices) inside the control files (this > > way, the resetting of cgroup values for all the drm devices, can be > > done by a simple 'cp'.) > > Again, please follow the existing conventions. There's a lot more > harm than good in every controller being creative in their own way. > It's trivial to build convenience features in userspace. Please do it > there. I can certainly remove the ro *.help file and leave the documentation to Documentation/, but for the *.default I do have a functional reason to it. As far as I can tell from the convention, the default is per cgroup and there is no way to describe per device default. Although, perhaps we are talking about two different kinds of defaults. Anyway, I can leave the discussion to a more detailed review. Regards, Kenny ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 0/4] drm/vc4: Binner BO management improvements
Changes sinve v8: * Added collected Reviewed-by; * Fixed up another problematic case as discussed on v8. Changes since v7: * Moved the used bool to vc4_v3d_bin_bo_get in order to check it locked and avoid a possible race condition; Changes since v6: * Removed vc4_v3d_bin_bo_put from error paths; * Added WARN_ON_ONCE when no bin BO at refcount release. Changes since v5: * Fix more locking mistakes; * Introduce get/put helpers; * Grabbed a reference when submitting an exec job with a binner slot. * Addressed misc comments. Changes since v4: * Used a kref on the binner bo instead of firstopen/lastclose; * Added a mutex to prevent race conditions; * Took care of enabling the OOM interrupt when we have a binner BO allocated. Changes since v3: * Split changes into more commits when possible; * Reworked binner bo alloc condition as discussed. Changes since v2: * Removed deprecated sentence about fristopen; * Added collected Reviewed-By tags. Changes since v1: * Squashed the two final patches into one. Paul Kocialkowski (4): drm/vc4: Reformat and the binner bo allocation helper drm/vc4: Check for V3D before binner bo alloc drm/vc4: Check for the binner bo before handling OOM interrupt drm/vc4: Allocate binner bo when starting to use the V3D drivers/gpu/drm/vc4/vc4_bo.c | 31 ++- drivers/gpu/drm/vc4/vc4_drv.c | 6 +++ drivers/gpu/drm/vc4/vc4_drv.h | 14 +++ drivers/gpu/drm/vc4/vc4_gem.c | 11 ++ drivers/gpu/drm/vc4/vc4_irq.c | 20 -- drivers/gpu/drm/vc4/vc4_v3d.c | 72 ++- 6 files changed, 132 insertions(+), 22 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 2/4] drm/vc4: Check for V3D before binner bo alloc
Check that we have a V3D device registered before attempting to allocate a binner buffer object. Signed-off-by: Paul Kocialkowski Reviewed-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_v3d.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c index 7c490106e185..c16db4665af6 100644 --- a/drivers/gpu/drm/vc4/vc4_v3d.c +++ b/drivers/gpu/drm/vc4/vc4_v3d.c @@ -241,6 +241,9 @@ static int bin_bo_alloc(struct vc4_dev *vc4) int ret = 0; struct list_head list; + if (!v3d) + return -ENODEV; + /* We may need to try allocating more than once to get a BO * that doesn't cross 256MB. Track the ones we've allocated * that failed so far, so that we can free them when we've got -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 1/4] drm/vc4: Reformat and the binner bo allocation helper
In preparation for wrapping the binner bo allocation helper with put/get helpers, pass the vc4 dev directly and drop the vc4 prefix. Signed-off-by: Paul Kocialkowski Reviewed-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_v3d.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c index a4b6859e3af6..7c490106e185 100644 --- a/drivers/gpu/drm/vc4/vc4_v3d.c +++ b/drivers/gpu/drm/vc4/vc4_v3d.c @@ -213,7 +213,7 @@ int vc4_v3d_get_bin_slot(struct vc4_dev *vc4) } /** - * vc4_allocate_bin_bo() - allocates the memory that will be used for + * bin_bo_alloc() - allocates the memory that will be used for * tile binning. * * The binner has a limitation that the addresses in the tile state @@ -234,9 +234,8 @@ int vc4_v3d_get_bin_slot(struct vc4_dev *vc4) * overall CMA pool before they make scenes complicated enough to run * out of bin space. */ -static int vc4_allocate_bin_bo(struct drm_device *drm) +static int bin_bo_alloc(struct vc4_dev *vc4) { - struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_v3d *v3d = vc4->v3d; uint32_t size = 16 * 1024 * 1024; int ret = 0; @@ -251,7 +250,7 @@ static int vc4_allocate_bin_bo(struct drm_device *drm) INIT_LIST_HEAD(&list); while (true) { - struct vc4_bo *bo = vc4_bo_create(drm, size, true, + struct vc4_bo *bo = vc4_bo_create(vc4->dev, size, true, VC4_BO_TYPE_BIN); if (IS_ERR(bo)) { @@ -333,7 +332,7 @@ static int vc4_v3d_runtime_resume(struct device *dev) struct vc4_dev *vc4 = v3d->vc4; int ret; - ret = vc4_allocate_bin_bo(vc4->dev); + ret = bin_bo_alloc(vc4); if (ret) return ret; @@ -403,7 +402,7 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data) if (ret != 0) return ret; - ret = vc4_allocate_bin_bo(drm); + ret = bin_bo_alloc(vc4); if (ret) { clk_disable_unprepare(v3d->clk); return ret; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 3/4] drm/vc4: Check for the binner bo before handling OOM interrupt
Since the OOM interrupt directly deals with the binner bo, it doesn't make sense to try and handle it without a binner buffer registered. Signed-off-by: Paul Kocialkowski Reviewed-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_irq.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c index ffd0a4388752..723dc86b4511 100644 --- a/drivers/gpu/drm/vc4/vc4_irq.c +++ b/drivers/gpu/drm/vc4/vc4_irq.c @@ -64,6 +64,9 @@ vc4_overflow_mem_work(struct work_struct *work) struct vc4_exec_info *exec; unsigned long irqflags; + if (!bo) + return; + bin_bo_slot = vc4_v3d_get_bin_slot(vc4); if (bin_bo_slot < 0) { DRM_ERROR("Couldn't allocate binner overflow mem\n"); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 4/4] drm/vc4: Allocate binner bo when starting to use the V3D
The binner BO is not required until the V3D is in use, so avoid allocating it at probe and do it on the first non-dumb BO allocation. Keep track of which clients are using the V3D and liberate the buffer when there is none left, using a kref. Protect the logic with a mutex to avoid race conditions. The binner BO is created at the time of the first render ioctl and is destroyed when there is no client and no exec job using it left. The Out-Of-Memory (OOM) interrupt also gets some tweaking, to avoid enabling it before having allocated a binner bo. We also want to keep the BO alive during runtime suspend/resume to avoid failing to allocate it at resume. This happens when the CMA pool is full at that point and results in a hard crash. Signed-off-by: Paul Kocialkowski Reviewed-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_bo.c | 31 +- drivers/gpu/drm/vc4/vc4_drv.c | 6 drivers/gpu/drm/vc4/vc4_drv.h | 14 drivers/gpu/drm/vc4/vc4_gem.c | 11 +++ drivers/gpu/drm/vc4/vc4_irq.c | 21 drivers/gpu/drm/vc4/vc4_v3d.c | 62 +++ 6 files changed, 125 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c index 88ebd681d7eb..1434bb829267 100644 --- a/drivers/gpu/drm/vc4/vc4_bo.c +++ b/drivers/gpu/drm/vc4/vc4_bo.c @@ -799,13 +799,36 @@ vc4_prime_import_sg_table(struct drm_device *dev, return obj; } +static int vc4_grab_bin_bo(struct vc4_dev *vc4, struct vc4_file *vc4file) +{ + int ret; + + if (!vc4->v3d) + return -ENODEV; + + if (vc4file->bin_bo_used) + return 0; + + ret = vc4_v3d_bin_bo_get(vc4, &vc4file->bin_bo_used); + if (ret) + return ret; + + return 0; +} + int vc4_create_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_vc4_create_bo *args = data; + struct vc4_file *vc4file = file_priv->driver_priv; + struct vc4_dev *vc4 = to_vc4_dev(dev); struct vc4_bo *bo = NULL; int ret; + ret = vc4_grab_bin_bo(vc4, vc4file); + if (ret) + return ret; + /* * We can't allocate from the BO cache, because the BOs don't * get zeroed, and that might leak data between users. @@ -846,6 +869,8 @@ vc4_create_shader_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_vc4_create_shader_bo *args = data; + struct vc4_file *vc4file = file_priv->driver_priv; + struct vc4_dev *vc4 = to_vc4_dev(dev); struct vc4_bo *bo = NULL; int ret; @@ -865,6 +890,10 @@ vc4_create_shader_bo_ioctl(struct drm_device *dev, void *data, return -EINVAL; } + ret = vc4_grab_bin_bo(vc4, vc4file); + if (ret) + return ret; + bo = vc4_bo_create(dev, args->size, true, VC4_BO_TYPE_V3D_SHADER); if (IS_ERR(bo)) return PTR_ERR(bo); @@ -894,7 +923,7 @@ vc4_create_shader_bo_ioctl(struct drm_device *dev, void *data, */ ret = drm_gem_handle_create(file_priv, &bo->base.base, &args->handle); - fail: +fail: drm_gem_object_put_unlocked(&bo->base.base); return ret; diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c index 6d9be20a32be..0f99ad03614e 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.c +++ b/drivers/gpu/drm/vc4/vc4_drv.c @@ -128,8 +128,12 @@ static int vc4_open(struct drm_device *dev, struct drm_file *file) static void vc4_close(struct drm_device *dev, struct drm_file *file) { + struct vc4_dev *vc4 = to_vc4_dev(dev); struct vc4_file *vc4file = file->driver_priv; + if (vc4file->bin_bo_used) + vc4_v3d_bin_bo_put(vc4); + vc4_perfmon_close_file(vc4file); kfree(vc4file); } @@ -274,6 +278,8 @@ static int vc4_drm_bind(struct device *dev) drm->dev_private = vc4; INIT_LIST_HEAD(&vc4->debugfs_list); + mutex_init(&vc4->bin_bo_lock); + ret = vc4_bo_cache_init(drm); if (ret) goto dev_put; diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 4f13f6262491..9170a24ec5f5 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -216,6 +216,11 @@ struct vc4_dev { * the minor is available (after drm_dev_register()). */ struct list_head debugfs_list; + + /* Mutex for binner bo allocation. */ + struct mutex bin_bo_lock; + /* Reference count for our binner bo. */ + struct kref bin_bo_kref; }; static inline struct vc4_dev * @@ -584,6 +589,11 @@ struct vc4_exec_info { * NULL otherwise. */ struct vc4_perfmon *perfmon; + + /* Whether the exec has taken a reference to the binner BO, which should +* happen with a VC4_PACKET_TILE_BINNING_MODE_C
drm-sync timeline signaling
Hi all, While picking up the IGT tests for timeline syncobj, I noticed that although we deal with multi wait across both timeline (with point value > 0) and binary (point value = 0) syncobjs, we don't seem to have a similar behavior with signaling. Do you have any thought on this? I'm considering writing some docs but I'm not quite sure whether this difference between signaling/waiting was intentional or just overlooked. Thanks, -Lionel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit
On Thu, May 16, 2019 at 10:12 AM Christian König wrote: > Am 16.05.19 um 16:03 schrieb Kenny Ho: > > On Thu, May 16, 2019 at 3:25 AM Christian König > > wrote: > >> Am 16.05.19 um 09:16 schrieb Koenig, Christian: > >> We need something like the Linux sysfs location or similar to have a > >> stable implementation. > > I get that, which is why I don't use minor to identify cards in user > > space apps I wrote: > > https://github.com/RadeonOpenCompute/k8s-device-plugin/blob/c2659c9d1d0713cad36fb5256681125121e6e32f/internal/pkg/amdgpu/amdgpu.go#L85 > > Yeah, that is certainly a possibility. > > > But within the kernel, I think my use of minor is consistent with the > > rest of the drm subsystem. I hope I don't need to reform the way the > > drm subsystem use minor in order to introduce a cgroup controller. > > Well I would try to avoid using the minor and at least look for > alternatives. E.g. what does udev uses to identify the devices for > example? And IIRC we have something like a "device-name" in the kernel > as well (what's printed in the logs). > > The minimum we need to do is get away from the minor=linenum approach, > cause as Daniel pointed out the minor allocation is quite a mess and not > necessary contiguous. I noticed :) but looks like there isn't much of a choice from what Tejun/cgroup replied about convention. Regards, Kenny ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 04/10] drm: Convert connector_helper_funcs->atomic_check to accept drm_atomic_state
On Thu, May 16, 2019 at 03:00:01PM +0300, Laurent Pinchart wrote: > Hi Sean, > > On Mon, May 13, 2019 at 10:38:58AM -0400, Sean Paul wrote: > > On Sat, May 11, 2019 at 3:12 PM Laurent Pinchart wrote: > > > On Thu, May 02, 2019 at 03:49:46PM -0400, Sean Paul wrote: > > >> From: Sean Paul > > >> > > >> Everyone who implements connector_helper_funcs->atomic_check reaches > > >> into the connector state to get the atomic state. Instead of continuing > > >> this pattern, change the callback signature to just give atomic state > > >> and let the driver determine what it does and does not need from it. > > >> > > >> Eventually all atomic functions should do this, but that's just too much > > >> busy work for me. > > > > > > Given that drivers also access the connector state, isn't this slightly > > > more inefficient ? > > > > Inefficient in terms of what? > > In terms of the operation having to lookup the connector state, when the > caller has it and can easily pass it. As Daniel commented, this may be > the price to pay for a cleaner API, but I wonder how much overhead all > the state tracking is costing. > It'd be interesting to quantify, but iirc the last time we benchmarked atomic check commits they were virtually free (~thousands/s). > > Agree that in isolation this patch might seem unnecessary, but it ties > > in with the encoder and bridge CLs which accept drm_atomic_state in > > CLs ? Googleism for patches, I usually catch these before sending... guess I missed one. Sean > > > their hooks. In general the idea is to convert all atomic functions to > > take overall atomic state instead of just their object state. Reality > > has proven to be more complicated and we need more access than what > > the current implementation provides. > > > > Sean > > > > >> Changes in v3: > > >> - Added to the set > > >> > > >> Cc: Daniel Vetter > > >> Cc: Ville Syrjälä > > >> Cc: Jani Nikula > > >> Cc: Joonas Lahtinen > > >> Cc: Rodrigo Vivi > > >> Cc: Ben Skeggs > > >> Cc: Laurent Pinchart > > >> Cc: Kieran Bingham > > >> Cc: Eric Anholt > > >> Signed-off-by: Sean Paul > > >> --- > > >> drivers/gpu/drm/drm_atomic_helper.c | 4 ++-- > > >> drivers/gpu/drm/i915/intel_atomic.c | 8 +--- > > >> drivers/gpu/drm/i915/intel_dp_mst.c | 7 --- > > >> drivers/gpu/drm/i915/intel_drv.h | 2 +- > > >> drivers/gpu/drm/i915/intel_sdvo.c| 9 + > > >> drivers/gpu/drm/i915/intel_tv.c | 8 +--- > > >> drivers/gpu/drm/nouveau/dispnv50/disp.c | 5 +++-- > > >> drivers/gpu/drm/rcar-du/rcar_lvds.c | 12 +++- > > >> drivers/gpu/drm/vc4/vc4_txp.c| 7 --- > > >> include/drm/drm_modeset_helper_vtables.h | 2 +- > > >> 10 files changed, 37 insertions(+), 27 deletions(-) > > >> > > >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > >> b/drivers/gpu/drm/drm_atomic_helper.c > > >> index 9d9e47276839..fa5a367507c1 100644 > > >> --- a/drivers/gpu/drm/drm_atomic_helper.c > > >> +++ b/drivers/gpu/drm/drm_atomic_helper.c > > >> @@ -683,7 +683,7 @@ drm_atomic_helper_check_modeset(struct drm_device > > >> *dev, > > >> } > > >> > > >> if (funcs->atomic_check) > > >> - ret = funcs->atomic_check(connector, > > >> new_connector_state); > > >> + ret = funcs->atomic_check(connector, state); > > >> if (ret) > > >> return ret; > > >> > > >> @@ -725,7 +725,7 @@ drm_atomic_helper_check_modeset(struct drm_device > > >> *dev, > > >> continue; > > >> > > >> if (funcs->atomic_check) > > >> - ret = funcs->atomic_check(connector, > > >> new_connector_state); > > >> + ret = funcs->atomic_check(connector, state); > > >> if (ret) > > >> return ret; > > >> } > > >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c > > >> b/drivers/gpu/drm/i915/intel_atomic.c > > >> index b844e8840c6f..e8a5b82e9242 100644 > > >> --- a/drivers/gpu/drm/i915/intel_atomic.c > > >> +++ b/drivers/gpu/drm/i915/intel_atomic.c > > >> @@ -103,12 +103,14 @@ int > > >> intel_digital_connector_atomic_set_property(struct drm_connector > > >> *connector, > > >> } > > >> > > >> int intel_digital_connector_atomic_check(struct drm_connector *conn, > > >> - struct drm_connector_state > > >> *new_state) > > >> + struct drm_atomic_state *state) > > >> { > > >> + struct drm_connector_state *new_state = > > >> + drm_atomic_get_new_connector_state(state, conn); > > >> struct intel_digital_connector_state *new_conn_state = > > >> to_intel_digital_connector_state(new_state); > > >> struct drm_connector_state *old_state = > > >> - drm_atomic_get_old_connector_state(new_state->state, conn); > > >> + drm_atomic_get_old_connector_stat
[PATCH v2 1/3] drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset()
drm_gem_dumb_map_offset() is a useful helper for non-dumb clients, so rename it to remove the _dumb. Signed-off-by: Steven Price --- drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- drivers/gpu/drm/drm_gem.c | 6 +++--- drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 +-- include/drm/drm_gem.h | 4 ++-- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c index 81dfdd33753a..956665464296 100644 --- a/drivers/gpu/drm/drm_dumb_buffers.c +++ b/drivers/gpu/drm/drm_dumb_buffers.c @@ -46,7 +46,7 @@ * To support dumb objects drivers must implement the &drm_driver.dumb_create * operation. &drm_driver.dumb_destroy defaults to drm_gem_dumb_destroy() if * not set and &drm_driver.dumb_map_offset defaults to - * drm_gem_dumb_map_offset(). See the callbacks for further details. + * drm_gem_map_offset(). See the callbacks for further details. * * Note that dumb objects may not be used for gpu acceleration, as has been * attempted on some ARM embedded platforms. Such drivers really must have @@ -125,7 +125,7 @@ int drm_mode_mmap_dumb_ioctl(struct drm_device *dev, args->handle, &args->offset); else - return drm_gem_dumb_map_offset(file_priv, dev, args->handle, + return drm_gem_map_offset(file_priv, dev, args->handle, &args->offset); } diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 50de138c89e0..99bb7f79a70b 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -294,7 +294,7 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) EXPORT_SYMBOL(drm_gem_handle_delete); /** - * drm_gem_dumb_map_offset - return the fake mmap offset for a gem object + * drm_gem_map_offset - return the fake mmap offset for a gem object * @file: drm file-private structure containing the gem object * @dev: corresponding drm_device * @handle: gem object handle @@ -306,7 +306,7 @@ EXPORT_SYMBOL(drm_gem_handle_delete); * Returns: * 0 on success or a negative error code on failure. */ -int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, +int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev, u32 handle, u64 *offset) { struct drm_gem_object *obj; @@ -332,7 +332,7 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, return ret; } -EXPORT_SYMBOL_GPL(drm_gem_dumb_map_offset); +EXPORT_SYMBOL_GPL(drm_gem_map_offset); /** * drm_gem_dumb_destroy - dumb fb callback helper for gem based drivers diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index a55f5ac41bf3..5e3aa9e4a096 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c @@ -276,8 +276,7 @@ int exynos_drm_gem_map_ioctl(struct drm_device *dev, void *data, { struct drm_exynos_gem_map *args = data; - return drm_gem_dumb_map_offset(file_priv, dev, args->handle, - &args->offset); + return drm_gem_map_offset(file_priv, dev, args->handle, &args->offset); } struct exynos_drm_gem *exynos_drm_gem_get(struct drm_file *filp, diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 5047c7ee25f5..91b07c2325e9 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -395,8 +395,8 @@ int drm_gem_fence_array_add(struct xarray *fence_array, int drm_gem_fence_array_add_implicit(struct xarray *fence_array, struct drm_gem_object *obj, bool write); -int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, - u32 handle, u64 *offset); +int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev, + u32 handle, u64 *offset); int drm_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev, uint32_t handle); -- 2.20.1
[PATCH v2 0/3] drm/panfrost: drm_gem_map_offset() helper
Panfrost has a re-implementation of drm_gem_dumb_map_offset() with an extra bug regarding the handling of imported buffers. However we don't really want Panfrost calling _dumb functions because it's not a KMS driver. This series renames drm_gem_dumb_map_offset() to drop the '_dumb' and introduces a shmem helper to wrap it. This means that the shmem implementation can be kept in sync with the semantics the drm_gem_shmem_mmap() callback provides. v1: https://lore.kernel.org/lkml/20190513143244.16478-1-steven.pr...@arm.com/ Changes since v1: * Rename drm_gem_dumb_map_offset to drop _dumb * Add a shmem helper Steven Price (3): drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset() drm: shmem: Add drm_gem_shmem_map_offset() wrapper drm/panfrost: Use drm_gem_shmem_map_offset() drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- drivers/gpu/drm/drm_gem.c | 6 +++--- drivers/gpu/drm/drm_gem_shmem_helper.c | 20 drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 +-- drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++-- include/drm/drm_gem.h | 4 ++-- include/drm/drm_gem_shmem_helper.h | 2 ++ 7 files changed, 32 insertions(+), 23 deletions(-) -- 2.20.1
[Bug 108487] Wayland compositors are unable to use hardware acceleration on i915
https://bugs.freedesktop.org/show_bug.cgi?id=108487 Daniel Stone changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #19 from Daniel Stone --- Fixed with b2200514 in Mesa. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit
Am 16.05.19 um 16:03 schrieb Kenny Ho: On Thu, May 16, 2019 at 3:25 AM Christian König wrote: Am 16.05.19 um 09:16 schrieb Koenig, Christian: Am 16.05.19 um 04:29 schrieb Kenny Ho: On Wed, May 15, 2019 at 5:26 PM Welty, Brian wrote: On 5/9/2019 2:04 PM, Kenny Ho wrote: Each file is multi-lined with one entry/line per drm device. Multi-line is correct for multiple devices, but I believe you need to use a KEY to denote device for both your set and get routines. I didn't see your set functions reading a key, or the get functions printing the key in output. cgroups-v2 conventions mention using KEY of major:minor, but I think you can use drm_minor as key? Given this controller is specific to the drm kernel subsystem which uses minor to identify drm device, Wait a second, using the DRM minor is a good idea in the first place. Well that should have read "is not a good idea".. I have a test system with a Vega10 and a Vega20. Which device gets which minor is not stable, but rather defined by the scan order of the PCIe bus. Normally the scan order is always the same, but adding or removing devices or delaying things just a little bit during init is enough to change this. We need something like the Linux sysfs location or similar to have a stable implementation. I get that, which is why I don't use minor to identify cards in user space apps I wrote: https://github.com/RadeonOpenCompute/k8s-device-plugin/blob/c2659c9d1d0713cad36fb5256681125121e6e32f/internal/pkg/amdgpu/amdgpu.go#L85 Yeah, that is certainly a possibility. But within the kernel, I think my use of minor is consistent with the rest of the drm subsystem. I hope I don't need to reform the way the drm subsystem use minor in order to introduce a cgroup controller. Well I would try to avoid using the minor and at least look for alternatives. E.g. what does udev uses to identify the devices for example? And IIRC we have something like a "device-name" in the kernel as well (what's printed in the logs). The minimum we need to do is get away from the minor=linenum approach, cause as Daniel pointed out the minor allocation is quite a mess and not necessary contiguous. Regards, Christian. Regards, Kenny ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit
Hello, I haven't gone through the patchset yet but some quick comments. On Wed, May 15, 2019 at 10:29:21PM -0400, Kenny Ho wrote: > Given this controller is specific to the drm kernel subsystem which > uses minor to identify drm device, I don't see a need to complicate > the interfaces more by having major and a key. As you can see in the > examples below, the drm device minor corresponds to the line number. > I am not sure how strict cgroup upstream is about the convention but I We're pretty strict. > am hoping there are flexibility here to allow for what I have > implemented. There are a couple of other things I have done that is So, please follow the interface conventions. We can definitely add new ones but that would need functional reasons. > not described in the convention: 1) inclusion of read-only *.help file > at the root cgroup, 2) use read-only (which I can potentially make rw) > *.default file instead of having a default entries (since the default > can be different for different devices) inside the control files (this > way, the resetting of cgroup values for all the drm devices, can be > done by a simple 'cp'.) Again, please follow the existing conventions. There's a lot more harm than good in every controller being creative in their own way. It's trivial to build convenience features in userspace. Please do it there. > > Is this really useful for an administrator to control? > > Isn't the resource we want to control actually the physical backing store? > That's correct. This is just the first level of control since the > backing store can be backed by different type of memory. I am in the > process of adding at least two more resources. Stay tuned. I am > doing the charge here to enforce the idea of "creator is deemed owner" > at a place where the code is shared by all (the init function.) Ideally, controller should only control hard resources which impact behaviors and performance which are immediately visible to users. Thanks. -- tejun ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit
On Thu, May 16, 2019 at 3:25 AM Christian König wrote: > Am 16.05.19 um 09:16 schrieb Koenig, Christian: > > Am 16.05.19 um 04:29 schrieb Kenny Ho: > >> On Wed, May 15, 2019 at 5:26 PM Welty, Brian wrote: > >>> On 5/9/2019 2:04 PM, Kenny Ho wrote: > Each file is multi-lined with one entry/line per drm device. > >>> Multi-line is correct for multiple devices, but I believe you need > >>> to use a KEY to denote device for both your set and get routines. > >>> I didn't see your set functions reading a key, or the get functions > >>> printing the key in output. > >>> cgroups-v2 conventions mention using KEY of major:minor, but I think > >>> you can use drm_minor as key? > >> Given this controller is specific to the drm kernel subsystem which > >> uses minor to identify drm device, > > Wait a second, using the DRM minor is a good idea in the first place. > Well that should have read "is not a good idea".. > > I have a test system with a Vega10 and a Vega20. Which device gets which > minor is not stable, but rather defined by the scan order of the PCIe bus. > > Normally the scan order is always the same, but adding or removing > devices or delaying things just a little bit during init is enough to > change this. > > We need something like the Linux sysfs location or similar to have a > stable implementation. I get that, which is why I don't use minor to identify cards in user space apps I wrote: https://github.com/RadeonOpenCompute/k8s-device-plugin/blob/c2659c9d1d0713cad36fb5256681125121e6e32f/internal/pkg/amdgpu/amdgpu.go#L85 But within the kernel, I think my use of minor is consistent with the rest of the drm subsystem. I hope I don't need to reform the way the drm subsystem use minor in order to introduce a cgroup controller. Regards, Kenny ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit
Am 16.05.19 um 14:28 schrieb Daniel Vetter: > [CAUTION: External Email] > > On Thu, May 16, 2019 at 09:25:31AM +0200, Christian König wrote: >> Am 16.05.19 um 09:16 schrieb Koenig, Christian: >>> Am 16.05.19 um 04:29 schrieb Kenny Ho: [CAUTION: External Email] On Wed, May 15, 2019 at 5:26 PM Welty, Brian wrote: > On 5/9/2019 2:04 PM, Kenny Ho wrote: >> There are four control file types, >> stats (ro) - display current measured values for a resource >> max (rw) - limits for a resource >> default (ro, root cgroup only) - default values for a resource >> help (ro, root cgroup only) - help string for a resource >> >> Each file is multi-lined with one entry/line per drm device. > Multi-line is correct for multiple devices, but I believe you need > to use a KEY to denote device for both your set and get routines. > I didn't see your set functions reading a key, or the get functions > printing the key in output. > cgroups-v2 conventions mention using KEY of major:minor, but I think > you can use drm_minor as key? Given this controller is specific to the drm kernel subsystem which uses minor to identify drm device, >>> Wait a second, using the DRM minor is a good idea in the first place. >> Well that should have read "is not a good idea".. > What else should we use? Well what does for example udev uses to identify a device? >> Christian. >> >>> I have a test system with a Vega10 and a Vega20. Which device gets which >>> minor is not stable, but rather defined by the scan order of the PCIe bus. >>> >>> Normally the scan order is always the same, but adding or removing >>> devices or delaying things just a little bit during init is enough to >>> change this. >>> >>> We need something like the Linux sysfs location or similar to have a >>> stable implementation. > You can go from sysfs location to drm class directory (in sysfs) and back. > That means if you care you need to walk sysfs yourself a bit, but using > the drm minor isn't a blocker itself. Yeah, agreed that userspace could do this. But I think if there is an of hand alternative we should use this instead. > One downside with the drm minor is that it's pretty good nonsense once you > have more than 64 gpus though, due to how we space render and legacy nodes > in the minor ids :-) Ok, another good reason to at least not use the minor=linenum approach. Christian. > -Daniel >>> Regards, >>> Christian. >>> I don't see a need to complicate the interfaces more by having major and a key. As you can see in the examples below, the drm device minor corresponds to the line number. I am not sure how strict cgroup upstream is about the convention but I am hoping there are flexibility here to allow for what I have implemented. There are a couple of other things I have done that is not described in the convention: 1) inclusion of read-only *.help file at the root cgroup, 2) use read-only (which I can potentially make rw) *.default file instead of having a default entries (since the default can be different for different devices) inside the control files (this way, the resetting of cgroup values for all the drm devices, can be done by a simple 'cp'.) >> Usage examples: >> // set limit for card1 to 1GB >> sed -i '2s/.*/1073741824/' /sys/fs/cgroup//drm.buffer.total.max >> >> // set limit for card0 to 512MB >> sed -i '1s/.*/536870912/' /sys/fs/cgroup//drm.buffer.total.max >> /** @file drm_gem.c >> @@ -154,6 +156,9 @@ void drm_gem_private_object_init(struct drm_device >> *dev, >> obj->handle_count = 0; >> obj->size = size; >> drm_vma_node_reset(&obj->vma_node); >> + >> + obj->drmcgrp = get_drmcgrp(current); >> + drmcgrp_chg_bo_alloc(obj->drmcgrp, dev, size); > Why do the charging here? > There is no backing store yet for the buffer, so this is really tracking > something akin to allowed virtual memory for GEM objects? > Is this really useful for an administrator to control? > Isn't the resource we want to control actually the physical backing store? That's correct. This is just the first level of control since the backing store can be backed by different type of memory. I am in the process of adding at least two more resources. Stay tuned. I am doing the charge here to enforce the idea of "creator is deemed owner" at a place where the code is shared by all (the init function.) >> + while (i <= max_minor && limits != NULL) { >> + sval = strsep(&limits, "\n"); >> + rc = kstrtoll(sval, 0, &val); > Input should be "KEY VALUE", so KEY will determine device to apply this > to. > Also, per cgroups-v2 documentation of limits, I believe need to parse and > handle the special "max" input value. > > parse_resource
Re: [PATCH libdrm 7/7] add syncobj timeline tests v3
Oh, please not that problem again :( Please just try "ssh gitlab.freedesktop.org" if that also times out like this you need to contact AMD network IT and ask why ssh once more doesn't work. Christian. Am 16.05.19 um 13:43 schrieb Zhou, David(ChunMing): It mentioned me I cannot push to gitlab directly. After that, I added my ssh pub to gitlab web, and also added gitlab url to git remote. then push again, it mentions "connection timeout". -David Original Message Subject: Re: [PATCH libdrm 7/7] add syncobj timeline tests v3 From: Christian König To: "Zhou, David(ChunMing)" ,"Koenig, Christian" ,"Zhou, David(ChunMing)" ,dri-devel@lists.freedesktop.org CC: [CAUTION: External Email] Am 16.05.19 um 12:19 schrieb zhoucm1: > > > On 2019年05月16日 18:09, Christian König wrote: >> [CAUTION: External Email] >> >> Am 16.05.19 um 10:16 schrieb zhoucm1: >>> I was able to push changes to libdrm, but now seems after libdrm is >>> migrated to gitlab, I cannot yet. What step do I need to get back my >>> permission? I already can login into gitlab with old freedesktop >>> account. >>> >>> @Christian, Can you help submit this patch set to libdrm first? >> >> Done. And I think you can now request write permission to a repository >> through the web-interface and all the "owners" of the project can grant >> that to you. > Any guide for that? I failed to find where to request permission. Not of hand. What does the system say when you try to push? Christian. > > -David >> >> Christian. >> >>> >>> >>> Thanks, >>> >>> -David >>> >>> >>> On 2019年05月16日 16:07, Chunming Zhou wrote: v2: drop DRM_SYNCOBJ_CREATE_TYPE_TIMELINE, fix timeout calculation, fix some warnings v3: add export/import and cpu signal testing cases Signed-off-by: Chunming Zhou Acked-by: Christian König Acked-by: Lionel Landwerlin --- tests/amdgpu/Makefile.am | 3 +- tests/amdgpu/amdgpu_test.c | 11 ++ tests/amdgpu/amdgpu_test.h | 21 +++ tests/amdgpu/meson.build | 2 +- tests/amdgpu/syncobj_tests.c | 290 +++ 5 files changed, 325 insertions(+), 2 deletions(-) create mode 100644 tests/amdgpu/syncobj_tests.c diff --git a/tests/amdgpu/Makefile.am b/tests/amdgpu/Makefile.am index 48278848..920882d0 100644 --- a/tests/amdgpu/Makefile.am +++ b/tests/amdgpu/Makefile.am @@ -34,4 +34,5 @@ amdgpu_test_SOURCES = \ uve_ib.h \ deadlock_tests.c \ vm_tests.c \ - ras_tests.c + ras_tests.c \ + syncobj_tests.c diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c index 35c8bf6c..73403fb4 100644 --- a/tests/amdgpu/amdgpu_test.c +++ b/tests/amdgpu/amdgpu_test.c @@ -57,6 +57,7 @@ #define DEADLOCK_TESTS_STR "Deadlock Tests" #define VM_TESTS_STR "VM Tests" #define RAS_TESTS_STR "RAS Tests" +#define SYNCOBJ_TIMELINE_TESTS_STR "SYNCOBJ TIMELINE Tests" /** * Open handles for amdgpu devices @@ -123,6 +124,12 @@ static CU_SuiteInfo suites[] = { .pCleanupFunc = suite_ras_tests_clean, .pTests = ras_tests, }, + { + .pName = SYNCOBJ_TIMELINE_TESTS_STR, + .pInitFunc = suite_syncobj_timeline_tests_init, + .pCleanupFunc = suite_syncobj_timeline_tests_clean, + .pTests = syncobj_timeline_tests, + }, CU_SUITE_INFO_NULL, }; @@ -176,6 +183,10 @@ static Suites_Active_Status suites_active_stat[] = { .pName = RAS_TESTS_STR, .pActive = suite_ras_tests_enable, }, + { + .pName = SYNCOBJ_TIMELINE_TESTS_STR, + .pActive = suite_syncobj_timeline_tests_enable, + }, }; diff --git a/tests/amdgpu/amdgpu_test.h b/tests/amdgpu/amdgpu_test.h index bcd0bc7e..36675ea3 100644 --- a/tests/amdgpu/amdgpu_test.h +++ b/tests/amdgpu/amdgpu_test.h @@ -216,6 +216,27 @@ CU_BOOL suite_ras_tests_enable(void); extern CU_TestInfo ras_tests[]; +/** + * Initialize syncobj timeline test suite + */ +int suite_syncobj_timeline_tests_init(); + +/** + * Deinitialize syncobj timeline test suite + */ +int suite_syncobj_timeline_tests_clean(); + +/** + * Decide if the suite is enabled by default or not. + */ +CU_BOOL suite_syncobj_timeline_tests_enable(void); + +/** + * Tests in syncobj timeline test suite + */ +extern CU_TestInfo syncobj_timeline_tests[]; + + /** * Helper functions */ diff --git a/tests/amdgpu/meson.build b/tests/amdgpu/meson.build index 95ed9305..1726cb43 100644 --- a/tests/amdgpu/meson.build +++ b/tests/amdgpu/meson.build @@ -24,7 +24,7 @
Re: [PATCH] drm/ttm, drm/vmwgfx: Use a configuration option for the TTM dma page pool
Am 16.05.19 um 13:23 schrieb Thomas Hellstrom: On Thu, 2019-05-16 at 12:05 +0200, Christian König wrote: Am 16.05.19 um 11:23 schrieb Thomas Hellstrom: Drivers like vmwgfx may want to test whether the dma page pool is present or not. Since it's activated by default by TTM if compiled-in, define a hidden configuration option that the driver can test for. Cc: Christian König Signed-off-by: Thomas Hellstrom There are at least also occasions of this in radeon and amdgpu, but those can be cleaned up later on. Reviewed-by: Christian König for now. Which tree should we use for merging? Thanks, Christian. We can take it through an AMD tree if it's OK with you. Then it would be easier to add similar changes to the AMD drivers. Perfectly fine with me, going to pick that up and doing the radeon/amdgpu cleanups on top. Christian. I'll send out v2 with some whitespace cleanup, a config help text and R-b next. Thanks, Thomas --- drivers/gpu/drm/Kconfig | 5 + drivers/gpu/drm/ttm/Makefile | 4 ++-- drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 3 --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 3 +-- include/drm/ttm/ttm_page_alloc.h | 2 +- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 2267e84d5cb4..f733a9273b3f 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -160,6 +160,11 @@ config DRM_TTM GPU memory types. Will be enabled automatically if a device driver uses it. +config DRM_TTM_DMA_PAGE_POOL +bool + depends on DRM_TTM && (SWIOTLB || INTEL_IOMMU) + default y + config DRM_GEM_CMA_HELPER bool depends on DRM diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile index 01fc670ce7a2..caea2a099496 100644 --- a/drivers/gpu/drm/ttm/Makefile +++ b/drivers/gpu/drm/ttm/Makefile @@ -4,8 +4,8 @@ ttm-y := ttm_memory.o ttm_tt.o ttm_bo.o \ ttm_bo_util.o ttm_bo_vm.o ttm_module.o \ - ttm_execbuf_util.o ttm_page_alloc.o ttm_bo_manager.o \ - ttm_page_alloc_dma.o + ttm_execbuf_util.o ttm_page_alloc.o ttm_bo_manager.o ttm-$(CONFIG_AGP) += ttm_agp_backend.o +ttm-$(CONFIG_DRM_TTM_DMA_PAGE_POOL) += ttm_page_alloc_dma.o obj-$(CONFIG_DRM_TTM) += ttm.o diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c index d594f7520b7b..98d100fd1599 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c @@ -33,7 +33,6 @@ * when freed). */ -#if defined(CONFIG_SWIOTLB) || defined(CONFIG_INTEL_IOMMU) #define pr_fmt(fmt) "[TTM] " fmt #include @@ -1234,5 +1233,3 @@ int ttm_dma_page_alloc_debugfs(struct seq_file *m, void *data) return 0; } EXPORT_SYMBOL_GPL(ttm_dma_page_alloc_debugfs); - -#endif diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index d59c474be38e..bc259d4df1cb 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -572,8 +572,7 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv) else dev_priv->map_mode = vmw_dma_map_populate; - /* No TTM coherent page pool? FIXME: Ask TTM instead! */ -if (!(IS_ENABLED(CONFIG_SWIOTLB) || IS_ENABLED(CONFIG_INTEL_IOMMU)) && +if (!IS_ENABLED(CONFIG_DRM_TTM_DMA_PAGE_POOL) && (dev_priv->map_mode == vmw_dma_alloc_coherent)) return -EINVAL; diff --git a/include/drm/ttm/ttm_page_alloc.h b/include/drm/ttm/ttm_page_alloc.h index 4d9b019d253c..a6b6ef5f9bf4 100644 --- a/include/drm/ttm/ttm_page_alloc.h +++ b/include/drm/ttm/ttm_page_alloc.h @@ -74,7 +74,7 @@ void ttm_unmap_and_unpopulate_pages(struct device *dev, struct ttm_dma_tt *tt); */ int ttm_page_alloc_debugfs(struct seq_file *m, void *data); -#if defined(CONFIG_SWIOTLB) || defined(CONFIG_INTEL_IOMMU) +#if defined(CONFIG_DRM_TTM_DMA_PAGE_POOL) /** * Initialize pool allocator. */ ___ 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] drm/komeda: Added AFBC support for komeda driver
On Thu, Apr 04, 2019 at 12:06:14PM +0100, james qian wang (Arm Technology China) wrote: > For supporting AFBC: > 1. Check if the user requested modifier can be supported by display HW. > 2. Check the obj->size with AFBC's requirement. > 3. Configure HW according to the modifier (afbc features) Can we have a bit more detailed commit message listing the various constraints (about which combination of modifiers, format and block sizes are valid) ? > > This patch depends on: > - https://patchwork.freedesktop.org/series/54448/ > - https://patchwork.freedesktop.org/series/54449/ > - https://patchwork.freedesktop.org/series/54450/ > - https://patchwork.freedesktop.org/series/58976/ > - https://patchwork.freedesktop.org/series/59000/ > > Signed-off-by: James Qian Wang (Arm Technology China) > > --- > .../arm/display/komeda/d71/d71_component.c| 39 ++ > .../arm/display/komeda/komeda_format_caps.c | 53 + > .../arm/display/komeda/komeda_format_caps.h | 5 ++ > .../arm/display/komeda/komeda_framebuffer.c | 74 ++- > .../arm/display/komeda/komeda_framebuffer.h | 4 + > .../gpu/drm/arm/display/komeda/komeda_kms.c | 2 +- > .../drm/arm/display/komeda/komeda_pipeline.h | 4 + > .../display/komeda/komeda_pipeline_state.c| 18 - > .../gpu/drm/arm/display/komeda/komeda_plane.c | 15 +++- > 9 files changed, 209 insertions(+), 5 deletions(-) > > 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 b582afcf9210..33ca1718b5cd 100644 > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c > @@ -134,6 +134,27 @@ static u32 to_rot_ctrl(u32 rot) > return lr_ctrl; > } > > +static u32 to_ad_ctrl(u64 modifier) > +{ > + u32 afbc_ctrl = AD_AEN; > + > + if (!modifier) > + return 0; > + > + if ((modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) == > + AFBC_FORMAT_MOD_BLOCK_SIZE_32x8) > + afbc_ctrl |= AD_WB; > + > + if (modifier & AFBC_FORMAT_MOD_YTR) > + afbc_ctrl |= AD_YT; > + if (modifier & AFBC_FORMAT_MOD_SPLIT) > + afbc_ctrl |= AD_BS; > + if (modifier & AFBC_FORMAT_MOD_TILED) > + afbc_ctrl |= AD_TH; > + > + return afbc_ctrl; > +} > + > static inline u32 to_d71_input_id(struct komeda_component_output *output) > { > struct komeda_component *comp = output->component; > @@ -173,6 +194,24 @@ static void d71_layer_update(struct komeda_component *c, > fb->pitches[i] & 0x); > } > > + malidp_write32(reg, AD_CONTROL, to_ad_ctrl(fb->modifier)); > + if (fb->modifier) { > + u64 addr; > + > + malidp_write32(reg, LAYER_AD_H_CROP, HV_CROP(st->afbc_crop_l, > + st->afbc_crop_r)); > + malidp_write32(reg, LAYER_AD_V_CROP, HV_CROP(st->afbc_crop_t, > + st->afbc_crop_b)); > + /* afbc 1.2 wants payload, afbc 1.0/1.1 wants end_addr */ > + if (fb->modifier & AFBC_FORMAT_MOD_TILED) > + addr = st->addr[0] + kfb->offset_payload; > + else > + addr = st->addr[0] + kfb->afbc_size - 1; > + > + malidp_write32(reg, BLK_P1_PTR_LOW, lower_32_bits(addr)); > + malidp_write32(reg, BLK_P1_PTR_HIGH, upper_32_bits(addr)); > + } > + > malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id); > malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize)); > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.c > b/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.c > index 1e17bd6107a4..b2195142e3f3 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.c > @@ -35,6 +35,59 @@ komeda_get_format_caps(struct komeda_format_caps_table > *table, > return NULL; > } > > +/* Two assumptions > + * 1. RGB always has YTR > + * 2. Tiled RGB always has SC > + */ > +u64 komeda_supported_modifiers[] = { > + /* AFBC_16x16 + features: YUV+RGB both */ > + AFBC_16x16(0), > + /* SPARSE */ > + AFBC_16x16(_SPARSE), > + /* YTR + (SPARSE) */ > + AFBC_16x16(_YTR | _SPARSE), > + AFBC_16x16(_YTR), > + /* SPLIT + SPARSE + YTR RGB only */ > + /* split mode is only allowed for sparse mode */ > + AFBC_16x16(_SPLIT | _SPARSE | _YTR), > + /* TILED + (SPARSE) */ > + /* TILED YUV format only */ > + AFBC_16x16(_TILED | _SPARSE), > + AFBC_16x16(_TILED), > + /* TILED + SC + (SPLIT+SPARSE | SPARSE) + (YTR) */ > + AFBC_16x16(_TILED | _SC | _SPLIT | _SPARSE | _YTR), > + AFBC_16x16(_TILED | _SC | _SPARSE | _YTR), > + AFBC_16x16(_TILED | _SC | _YTR), > + /* AFBC_32x8 + features: which are RGB formats on
Re: [PATCH v5 08/11] drm/fb-helper: Remove drm_fb_helper_connector
Den 16.05.2019 15.07, skrev Sam Ravnborg: > Hi Noralf. > > See few comments in the following. > > Sam > > On Mon, May 06, 2019 at 08:01:36PM +0200, Noralf Trønnes wrote: >> All drivers add all their connectors so there's no need to keep around an >> array of available connectors. I could expand on this text a little: All drivers add all their connectors so there's no need to keep around an array of available connectors. Instead we just put the useable (not writeback) connectors in a temporary array using drm_client_for_each_connector_iter() everytime we probe the outputs. Other places where it's necessary to look at the connectors, we just iterate over them using the same iterator function. >> >> Rename functions which signature is changed since they will be moved to >> drm_client in a later patch. >> >> Signed-off-by: Noralf Trønnes >> --- >> @@ -1645,8 +1461,9 @@ static int drm_fb_helper_single_fb_probe(struct >> drm_fb_helper *fb_helper, >> struct drm_client_dev *client = &fb_helper->client; >> int ret = 0; >> int crtc_count = 0; >> -int i; >> +struct drm_connector_list_iter conn_iter; >> struct drm_fb_helper_surface_size sizes; >> +struct drm_connector *connector; >> struct drm_mode_set *mode_set; >> int best_depth = 0; >> >> @@ -1663,11 +1480,11 @@ static int drm_fb_helper_single_fb_probe(struct >> drm_fb_helper *fb_helper, >> if (preferred_bpp != sizes.surface_bpp) >> sizes.surface_depth = sizes.surface_bpp = preferred_bpp; >> >> -drm_fb_helper_for_each_connector(fb_helper, i) { >> -struct drm_fb_helper_connector *fb_helper_conn = >> fb_helper->connector_info[i]; >> +drm_connector_list_iter_begin(fb_helper->dev, &conn_iter); >> +drm_client_for_each_connector_iter(connector, &conn_iter) { >> struct drm_cmdline_mode *cmdline_mode; > > I fail to see when to use drm_client_for_each_connector_iter() and when > to use a simple for loop. > In this patch drm_fb_helper_for_each_connector() is here replaced by the > iterator variant and in many other placed a simple for loop. > When I use a for loop it's in the 'probe for outputs' path where we are operating on a temporary connector array that is created in drm_setup_crtcs(). > The old code, in this place, would go through all connectors, but this > code will skipDRM_MODE_CONNECTOR_WRITEBACK conectors. > So it does not like identical code. > If you look at the removed drm_fb_helper_single_add_all_connectors() you'll see that it skips writeback connectors. >> >> -cmdline_mode = &fb_helper_conn->connector->cmdline_mode; >> +cmdline_mode = &connector->cmdline_mode; >> >> if (cmdline_mode->bpp_specified) { >> switch (cmdline_mode->bpp) { >> @@ -1692,6 +1509,7 @@ static int drm_fb_helper_single_fb_probe(struct >> drm_fb_helper *fb_helper, >> break; >> } >> } >> +drm_connector_list_iter_end(&conn_iter); >> >> /* >> * If we run into a situation where, for example, the primary plane >> @@ -1876,26 +1694,12 @@ void drm_fb_helper_fill_info(struct fb_info *info, >> } >> EXPORT_SYMBOL(drm_fb_helper_fill_info); >> >> +static void drm_client_connectors_enabled(struct drm_connector **connectors, >> + unsigned int connector_count, >> + bool *enabled) >> { >> bool any_enabled = false; >> struct drm_connector *connector; >> int i = 0; >> >> -drm_fb_helper_for_each_connector(fb_helper, i) { >> -connector = fb_helper->connector_info[i]->connector; >> +for (i = 0; i < connector_count; i++) { >> +connector = connectors[i]; > This is for example a place where the drm_fb_helper_for_each_connector() > is replaced by a simple for loop. Yeah, this is downstream from drm_setup_crtcs() and we're operating on the temporary 'connectors' array. > The simple for loop is nice and readable - just a comment replated to > the above comment. > >> struct drm_display_mode *mode = modes[i]; >> struct drm_crtc *crtc = crtcs[i]; >> struct drm_fb_offset *offset = &offsets[i]; >> >> if (mode && crtc) { >> struct drm_mode_set *modeset = >> drm_client_find_modeset(client, crtc); >> -struct drm_connector *connector = >> -fb_helper->connector_info[i]->connector; >> +struct drm_connector *connector = connectors[i]; >> >> DRM_DEBUG_KMS("desired mode %s set on crtc %d >> (%d,%d)\n", >>mode->name, crtc->base.id, offset->x, >> offset->y); >> @@ -2539,6 +2354,10 @@ static void drm_setup_crtcs(struct drm_fb_helper >> *fb_helper, >> kfree(modes); >> kfree(offsets); >> kfree(enabled); >> +free_connectors: >> +for (i = 0
[v11 09/12] drm/i915:Enabled Modeset when HDR Infoframe changes
This patch enables modeset whenever HDR metadata needs to be updated to sink. v2: Addressed Shashank's review comments. v3: Added Shashank's RB. v4: Addressed Ville's review comments. v5: Addressed Ville's review comments. Signed-off-by: Ville Syrjälä Signed-off-by: Uma Shankar Reviewed-by: Shashank Sharma --- drivers/gpu/drm/i915/intel_atomic.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 58b8049..6b985e8 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -105,6 +105,16 @@ int intel_digital_connector_atomic_set_property(struct drm_connector *connector, return -EINVAL; } +static bool blob_equal(const struct drm_property_blob *a, + const struct drm_property_blob *b) +{ + if (a && b) + return a->length == b->length && + !memcmp(a->data, b->data, a->length); + + return !a == !b; +} + int intel_digital_connector_atomic_check(struct drm_connector *conn, struct drm_connector_state *new_state) { @@ -132,7 +142,9 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn, new_conn_state->base.colorspace != old_conn_state->base.colorspace || new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio || new_conn_state->base.content_type != old_conn_state->base.content_type || - new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode) + new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode || + !blob_equal(new_conn_state->base.hdr_output_metadata, + old_conn_state->base.hdr_output_metadata)) crtc_state->mode_changed = true; return 0; -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v11 06/12] drm/i915: Write HDR infoframe and send to panel
Enable writing of HDR metadata infoframe to panel. The data will be provid by usersapace compositors, based on blending policies and passsed to driver through a blob property. v2: Rebase v3: Fixed a warning message v4: Addressed Shashank's review comments v5: Rebase. Added infoframe calculation in compute config. v6: Addressed Shashank's review comment. Added HDR metadata support from GEN10 onwards as per Shashank's recommendation. v7: Addressed Shashank's review comments v8: Added Shashank's RB. v9: Addressed Ville's review comments. Signed-off-by: Uma Shankar Reviewed-by: Shashank Sharma --- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_hdmi.c | 44 +++ 2 files changed, 45 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 5258abb..40e2c52 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -910,6 +910,7 @@ struct intel_crtc_state { union hdmi_infoframe avi; union hdmi_infoframe spd; union hdmi_infoframe hdmi; + union hdmi_infoframe drm; } infoframes; /* HDMI scrambling status */ diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 92597d8..d3b8e09 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -573,6 +573,7 @@ static u32 hsw_infoframes_enabled(struct intel_encoder *encoder, HDMI_INFOFRAME_TYPE_AVI, HDMI_INFOFRAME_TYPE_SPD, HDMI_INFOFRAME_TYPE_VENDOR, + HDMI_INFOFRAME_TYPE_DRM, }; u32 intel_hdmi_infoframe_enable(unsigned int type) @@ -795,6 +796,41 @@ void intel_read_infoframe(struct intel_encoder *encoder, return true; } +static bool +intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder, +struct intel_crtc_state *crtc_state, +struct drm_connector_state *conn_state) +{ + struct hdmi_drm_infoframe *frame = &crtc_state->infoframes.drm.drm; + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); + int ret; + + if (!(INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))) + return true; + + if (!crtc_state->has_infoframe) + return true; + + if (!conn_state->hdr_output_metadata || + conn_state->hdr_output_metadata->length == 0) + return true; + + crtc_state->infoframes.enable |= + intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_DRM); + + ret = drm_hdmi_infoframe_set_hdr_metadata(frame, conn_state); + if (ret < 0) { + DRM_ERROR("couldn't set HDR metadata in infoframe\n"); + return false; + } + + ret = hdmi_drm_infoframe_check(frame); + if (WARN_ON(ret)) + return false; + + return true; +} + static void g4x_set_infoframes(struct intel_encoder *encoder, bool enable, const struct intel_crtc_state *crtc_state, @@ -1180,6 +1216,9 @@ static void hsw_set_infoframes(struct intel_encoder *encoder, intel_write_infoframe(encoder, crtc_state, HDMI_INFOFRAME_TYPE_VENDOR, &crtc_state->infoframes.hdmi); + intel_write_infoframe(encoder, crtc_state, + HDMI_INFOFRAME_TYPE_DRM, + &crtc_state->infoframes.drm); } void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable) @@ -2386,6 +2425,11 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder, return -EINVAL; } + if (!intel_hdmi_compute_drm_infoframe(encoder, pipe_config, conn_state)) { + DRM_DEBUG_KMS("bad DRM infoframe\n"); + return -EINVAL; + } + return 0; } -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v11 08/12] drm/i915: Enable infoframes on GLK+ for HDR
From: Ville Syrjälä This patch enables infoframes on GLK+ to be used to send HDR metadata to HDMI sink. v2: Addressed Shashank's review comment. v3: Addressed Shashank's review comment. v4: Added Shashank's RB. v5: Dropped hdr_metadata_change check while modeset, as per Ville's suggestion. Signed-off-by: Ville Syrjälä Signed-off-by: Uma Shankar Reviewed-by: Shashank Sharma --- drivers/gpu/drm/i915/i915_reg.h | 4 drivers/gpu/drm/i915/intel_hdmi.c | 19 +++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index e97c47f..d3f5510 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4694,6 +4694,7 @@ enum { #define VIDEO_DIP_FREQ_MASK (3 << 16) /* HSW and later: */ #define DRM_DIP_ENABLE (1 << 28) +#define VIDEO_DIP_ENABLE_DRM_GLK (1 << 28) #define PSR_VSC_BIT_7_SET(1 << 27) #define VSC_SELECT_MASK (0x3 << 25) #define VSC_SELECT_SHIFT 25 @@ -8146,6 +8147,7 @@ enum { #define _HSW_VIDEO_DIP_SPD_DATA_A 0x602A0 #define _HSW_VIDEO_DIP_GMP_DATA_A 0x602E0 #define _HSW_VIDEO_DIP_VSC_DATA_A 0x60320 +#define _GLK_VIDEO_DIP_DRM_DATA_A 0x60440 #define _HSW_VIDEO_DIP_AVI_ECC_A 0x60240 #define _HSW_VIDEO_DIP_VS_ECC_A0x60280 #define _HSW_VIDEO_DIP_SPD_ECC_A 0x602C0 @@ -8159,6 +8161,7 @@ enum { #define _HSW_VIDEO_DIP_SPD_DATA_B 0x612A0 #define _HSW_VIDEO_DIP_GMP_DATA_B 0x612E0 #define _HSW_VIDEO_DIP_VSC_DATA_B 0x61320 +#define _GLK_VIDEO_DIP_DRM_DATA_B 0x61440 #define _HSW_VIDEO_DIP_BVI_ECC_B 0x61240 #define _HSW_VIDEO_DIP_VS_ECC_B0x61280 #define _HSW_VIDEO_DIP_SPD_ECC_B 0x612C0 @@ -8184,6 +8187,7 @@ enum { #define HSW_TVIDEO_DIP_SPD_DATA(trans, i) _MMIO_TRANS2(trans, _HSW_VIDEO_DIP_SPD_DATA_A + (i) * 4) #define HSW_TVIDEO_DIP_GMP_DATA(trans, i) _MMIO_TRANS2(trans, _HSW_VIDEO_DIP_GMP_DATA_A + (i) * 4) #define HSW_TVIDEO_DIP_VSC_DATA(trans, i) _MMIO_TRANS2(trans, _HSW_VIDEO_DIP_VSC_DATA_A + (i) * 4) +#define GLK_TVIDEO_DIP_DRM_DATA(trans, i) _MMIO_TRANS2(trans, _GLK_VIDEO_DIP_DRM_DATA_A + (i) * 4) #define ICL_VIDEO_DIP_PPS_DATA(trans, i) _MMIO_TRANS2(trans, _ICL_VIDEO_DIP_PPS_DATA_A + (i) * 4) #define ICL_VIDEO_DIP_PPS_ECC(trans, i)_MMIO_TRANS2(trans, _ICL_VIDEO_DIP_PPS_ECC_A + (i) * 4) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index d3b8e09..8bd7c07 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -152,6 +152,8 @@ static u32 hsw_infoframe_enable(unsigned int type) return VIDEO_DIP_ENABLE_SPD_HSW; case HDMI_INFOFRAME_TYPE_VENDOR: return VIDEO_DIP_ENABLE_VS_HSW; + case HDMI_INFOFRAME_TYPE_DRM: + return VIDEO_DIP_ENABLE_DRM_GLK; default: MISSING_CASE(type); return 0; @@ -177,6 +179,8 @@ static u32 hsw_infoframe_enable(unsigned int type) return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder, i); case HDMI_INFOFRAME_TYPE_VENDOR: return HSW_TVIDEO_DIP_VS_DATA(cpu_transcoder, i); + case HDMI_INFOFRAME_TYPE_DRM: + return GLK_TVIDEO_DIP_DRM_DATA(cpu_transcoder, i); default: MISSING_CASE(type); return INVALID_MMIO_REG; @@ -560,10 +564,16 @@ static u32 hsw_infoframes_enabled(struct intel_encoder *encoder, { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); u32 val = I915_READ(HSW_TVIDEO_DIP_CTL(pipe_config->cpu_transcoder)); + u32 mask; - return val & (VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW | - VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW | - VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW); + mask = (VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW | + VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW | + VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW); + + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) + mask |= VIDEO_DIP_ENABLE_DRM_GLK; + + return val & mask; } static const u8 infoframe_type_to_idx[] = { @@ -1193,7 +1203,8 @@ static void hsw_set_infoframes(struct intel_encoder *encoder, val &= ~(VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW | -VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW); +VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW | +VIDEO_DIP_ENABLE_DRM_GLK); if (!enable) { I915_WRITE(reg, val); -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org htt
Re: [PATCH] drm/bridge: Remove duplicate header
Hi Sabyasachi, On Thu, May 16, 2019 at 06:45:04PM +0530, Sabyasachi Gupta wrote: > On Tue, May 14, 2019 at 1:05 PM Laurent Pinchart wrote: > > On Tue, May 14, 2019 at 01:01:41PM +0530, Sabyasachi Gupta wrote: > > > Remove drm/drm_panel.h which is included more than once > > > > > > Signed-off-by: Sabyasachi Gupta > > > --- > > > drivers/gpu/drm/bridge/panel.c | 1 - > > > 1 file changed, 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/bridge/panel.c > > > b/drivers/gpu/drm/bridge/panel.c > > > index 7cbaba2..402b318 100644 > > > --- a/drivers/gpu/drm/bridge/panel.c > > > +++ b/drivers/gpu/drm/bridge/panel.c > > > @@ -15,7 +15,6 @@ > > > #include > > > #include > > > #include > > > -#include > > > > Which tree is this against ? The patch applies on neither drm-next nor > > drm-misc-next. > > > > It is against linux-next tree You will likely have to rebase it on top of the drm-next or drm-next-misc branch. > > While at it, could you you reorder the other header alphabetically to > > make this kind of issue easier to notice ? > > It is already arranged in alphabetical order There's an #include at the top of the list of includes. That's the one that is out of place. > > > > > > struct panel_bridge { > > > struct drm_bridge bridge; -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v11 02/12] drm: Add reference counting on HDR metadata blob
From: Jonas Karlman This adds reference count for HDR metadata blob, handled as part of duplicate and destroy connector state functions. v2: Removed the hdr_metadata_changed initialization as the variable is dropped and not required. Signed-off-by: Jonas Karlman Signed-off-by: Uma Shankar --- drivers/gpu/drm/drm_atomic_state_helper.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index ac929f6..ec13823 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -391,6 +391,9 @@ void drm_atomic_helper_connector_reset(struct drm_connector *connector) drm_connector_get(connector); state->commit = NULL; + if (state->hdr_output_metadata) + drm_property_blob_get(state->hdr_output_metadata); + /* Don't copy over a writeback job, they are used only once */ state->writeback_job = NULL; } @@ -438,6 +441,8 @@ struct drm_connector_state * if (state->writeback_job) drm_writeback_cleanup_job(state->writeback_job); + + drm_property_blob_put(state->hdr_output_metadata); } EXPORT_SYMBOL(__drm_atomic_helper_connector_destroy_state); -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v11 12/12] drm/i915: Add state readout for DRM infoframe
Added state readout for DRM infoframe and enabled state validation for DRM infoframe. v2: Addressed Ville's review comments and dropped the unused drm infoframe read at intel_hdmi_init. v3: Removed a redundant platform check as per Ville's comment. Signed-off-by: Uma Shankar --- drivers/gpu/drm/i915/intel_ddi.c | 3 +++ drivers/gpu/drm/i915/intel_display.c | 1 + 2 files changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 0af47f3..c789de9 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -3834,6 +3834,9 @@ void intel_ddi_get_config(struct intel_encoder *encoder, intel_read_infoframe(encoder, pipe_config, HDMI_INFOFRAME_TYPE_VENDOR, &pipe_config->infoframes.hdmi); + intel_read_infoframe(encoder, pipe_config, +HDMI_INFOFRAME_TYPE_DRM, +&pipe_config->infoframes.drm); } static enum intel_output_type diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e35ba8d..c89b214 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12274,6 +12274,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv) PIPE_CONF_CHECK_INFOFRAME(avi); PIPE_CONF_CHECK_INFOFRAME(spd); PIPE_CONF_CHECK_INFOFRAME(hdmi); + PIPE_CONF_CHECK_INFOFRAME(drm); #undef PIPE_CONF_CHECK_X #undef PIPE_CONF_CHECK_I -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v11 05/12] drm/i915: Attach HDR metadata property to connector
Attach HDR metadata property to connector object. v2: Rebase v3: Updated the property name as per updated name while creating hdr metadata property Signed-off-by: Uma Shankar Reviewed-by: Shashank Sharma --- drivers/gpu/drm/i915/intel_hdmi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 2a4086c..92597d8 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -2724,6 +2724,8 @@ static void intel_hdmi_destroy(struct drm_connector *connector) drm_connector_attach_content_type_property(connector); connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; + drm_object_attach_property(&connector->base, + connector->dev->mode_config.hdr_output_metadata_property, 0); if (!HAS_GMCH(dev_priv)) drm_connector_attach_max_bpc_property(connector, 8, 12); -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v11 10/12] drm/i915: Added DRM Infoframe handling for BYT/CHT
BYT/CHT doesn't support DRM Infoframe. This caused a WARN_ON due to a missing CASE while executing intel_hdmi_infoframes_enabled function. This patch fixes the same. Signed-off-by: Uma Shankar --- drivers/gpu/drm/i915/intel_hdmi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 8bd7c07..90e0587 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -129,6 +129,8 @@ static u32 g4x_infoframe_enable(unsigned int type) return VIDEO_DIP_ENABLE_SPD; case HDMI_INFOFRAME_TYPE_VENDOR: return VIDEO_DIP_ENABLE_VENDOR; + case HDMI_INFOFRAME_TYPE_DRM: + return 0; default: MISSING_CASE(type); return 0; -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v11 11/12] video/hdmi: Add Unpack function for DRM infoframe
Added unpack function for DRM infoframe for dynamic range and mastering infoframe readout. v2: Addressed Ville's review comments. Suggested-by: Ville Syrjälä Signed-off-by: Uma Shankar --- drivers/video/hdmi.c | 67 1 file changed, 67 insertions(+) diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index 481f036..b99ba01 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -1805,6 +1805,70 @@ static int hdmi_audio_infoframe_unpack(struct hdmi_audio_infoframe *frame, } /** + * hdmi_drm_infoframe_unpack() - unpack binary buffer to a HDMI DRM infoframe + * @frame: HDMI DRM infoframe + * @buffer: source buffer + * @size: size of buffer + * + * Unpacks the information contained in binary @buffer into a structured + * @frame of the HDMI Dynamic Range and Mastering (DRM) information frame. + * Also verifies the checksum as required by section 5.3.5 of the HDMI 1.4 + * specification. + * + * Returns 0 on success or a negative error code on failure. + */ +static int hdmi_drm_infoframe_unpack(struct hdmi_drm_infoframe *frame, +const void *buffer, size_t size) +{ + const u8 *ptr = buffer; + const u8 *temp; + u8 x_lsb, x_msb; + u8 y_lsb, y_msb; + int ret; + int i; + + if (size < HDMI_INFOFRAME_SIZE(DRM)) + return -EINVAL; + + if (ptr[0] != HDMI_INFOFRAME_TYPE_DRM || + ptr[1] != 1 || + ptr[2] != HDMI_DRM_INFOFRAME_SIZE) + return -EINVAL; + + if (hdmi_infoframe_checksum(buffer, HDMI_INFOFRAME_SIZE(DRM)) != 0) + return -EINVAL; + + ret = hdmi_drm_infoframe_init(frame); + if (ret) + return ret; + + ptr += HDMI_INFOFRAME_HEADER_SIZE; + + frame->eotf = ptr[0] & 0x7; + frame->metadata_type = ptr[1] & 0x7; + + temp = ptr + 2; + for (i = 0; i < 3; i++) { + x_lsb = *temp++; + x_msb = *temp++; + frame->display_primaries[i].x = (x_msb << 8) | x_lsb; + y_lsb = *temp++; + y_msb = *temp++; + frame->display_primaries[i].y = (y_msb << 8) | y_lsb; + } + + frame->white_point.x = (ptr[15] << 8) | ptr[14]; + frame->white_point.y = (ptr[17] << 8) | ptr[16]; + + frame->max_display_mastering_luminance = (ptr[19] << 8) | ptr[18]; + frame->min_display_mastering_luminance = (ptr[21] << 8) | ptr[20]; + frame->max_cll = (ptr[23] << 8) | ptr[22]; + frame->max_fall = (ptr[25] << 8) | ptr[24]; + + return 0; +} + +/** * hdmi_infoframe_unpack() - unpack binary buffer to a HDMI infoframe * @frame: HDMI infoframe * @buffer: source buffer @@ -1830,6 +1894,9 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame, case HDMI_INFOFRAME_TYPE_AVI: ret = hdmi_avi_infoframe_unpack(&frame->avi, buffer, size); break; + case HDMI_INFOFRAME_TYPE_DRM: + ret = hdmi_drm_infoframe_unpack(&frame->drm, buffer, size); + break; case HDMI_INFOFRAME_TYPE_SPD: ret = hdmi_spd_infoframe_unpack(&frame->spd, buffer, size); break; -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v11 07/12] drm: Add HLG EOTF
From: Ville Syrjälä ADD HLG EOTF to the list of EOTF transfer functions supported. Hybrid Log-Gamma (HLG) is a high dynamic range (HDR) standard. HLG defines a nonlinear transfer function in which the lower half of the signal values use a gamma curve and the upper half of the signal values use a logarithmic curve. v2: Rebase v3: Fixed a warning message v4: Addressed Shashank's review comments v5: Addressed Jonas Karlman's review comment and dropped the i915 tag from header. Signed-off-by: Ville Syrjälä Signed-off-by: Uma Shankar Reviewed-by: Shashank Sharma --- drivers/gpu/drm/drm_edid.c | 3 ++- include/linux/hdmi.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 73560c9..262510c 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3854,7 +3854,8 @@ static uint8_t eotf_supported(const u8 *edid_ext) return edid_ext[2] & (BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) | BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) | -BIT(HDMI_EOTF_SMPTE_ST2084)); +BIT(HDMI_EOTF_SMPTE_ST2084) | +BIT(HDMI_EOTF_BT_2100_HLG)); } static uint8_t hdr_metadata_type(const u8 *edid_ext) diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index bcf3c6c..ee55ba5 100644 --- a/include/linux/hdmi.h +++ b/include/linux/hdmi.h @@ -162,6 +162,7 @@ enum hdmi_eotf { HDMI_EOTF_TRADITIONAL_GAMMA_SDR, HDMI_EOTF_TRADITIONAL_GAMMA_HDR, HDMI_EOTF_SMPTE_ST2084, + HDMI_EOTF_BT_2100_HLG, }; struct hdmi_avi_infoframe { -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v11 04/12] drm: Enable HDR infoframe support
Enable Dynamic Range and Mastering Infoframe for HDR content, which is defined in CEA 861.3 spec. The metadata will be computed based on blending policy in userspace compositors and passed as a connector property blob to driver. The same will be sent as infoframe to panel which support HDR. Added the const version of infoframe for DRM metadata for HDR. v2: Rebase and added Ville's POC changes. v3: No Change v4: Addressed Shashank's review comments and merged the patch making drm infoframe function arguments as constant. v5: Rebase v6: Fixed checkpatch warnings with --strict option. Addressed Shashank's review comments and added his RB. v7: Addressed Brian Starkey's review comments. Merged 2 patches into one. v8: Addressed Jonas Karlman review comments. v9: Addressed Jonas Karlman review comments. v10: Addressed Ville's review comments. v11: Added BUILD_BUG_ON and sizeof instead of magic numbers as per Ville's comments. Signed-off-by: Uma Shankar Signed-off-by: Ville Syrjälä Reviewed-by: Shashank Sharma --- drivers/gpu/drm/drm_edid.c | 72 + drivers/video/hdmi.c | 190 + include/drm/drm_edid.h | 5 ++ include/linux/hdmi.h | 28 +++ 4 files changed, 295 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index a5ef9f4..73560c9 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4904,6 +4904,78 @@ static bool is_hdmi2_sink(struct drm_connector *connector) connector->display_info.color_formats & DRM_COLOR_FORMAT_YCRCB420; } +static inline bool is_eotf_supported(u8 output_eotf, u8 sink_eotf) +{ + return sink_eotf & BIT(output_eotf); +} + +/** + * drm_hdmi_infoframe_set_hdr_metadata() - fill an HDMI DRM infoframe with + * HDR metadata from userspace + * @frame: HDMI DRM infoframe + * @hdr_metadata: hdr_source_metadata info from userspace + * + * Return: 0 on success or a negative error code on failure. + */ +int +drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame, + const struct drm_connector_state *conn_state) +{ + struct drm_connector *connector; + struct hdr_output_metadata *hdr_metadata; + int err; + + if (!frame || !conn_state) + return -EINVAL; + + connector = conn_state->connector; + + if (!conn_state->hdr_output_metadata) + return -EINVAL; + + hdr_metadata = conn_state->hdr_output_metadata->data; + + if (!hdr_metadata || !connector) + return -EINVAL; + + /* Sink EOTF is Bit map while infoframe is absolute values */ + if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf, + connector->hdr_sink_metadata.hdmi_type1.eotf)) { + DRM_DEBUG_KMS("EOTF Not Supported\n"); + return -EINVAL; + } + + err = hdmi_drm_infoframe_init(frame); + if (err < 0) + return err; + + frame->eotf = hdr_metadata->hdmi_metadata_type1.eotf; + frame->metadata_type = hdr_metadata->hdmi_metadata_type1.metadata_type; + + BUILD_BUG_ON(sizeof(frame->display_primaries) != + sizeof(hdr_metadata->hdmi_metadata_type1.display_primaries)); + BUILD_BUG_ON(sizeof(frame->white_point) != +sizeof(hdr_metadata->hdmi_metadata_type1.white_point)); + + memcpy(&frame->display_primaries, + &hdr_metadata->hdmi_metadata_type1.display_primaries, + sizeof(frame->display_primaries)); + + memcpy(&frame->white_point, + &hdr_metadata->hdmi_metadata_type1.white_point, + sizeof(frame->white_point)); + + frame->max_display_mastering_luminance = + hdr_metadata->hdmi_metadata_type1.max_display_mastering_luminance; + frame->min_display_mastering_luminance = + hdr_metadata->hdmi_metadata_type1.min_display_mastering_luminance; + frame->max_fall = hdr_metadata->hdmi_metadata_type1.max_fall; + frame->max_cll = hdr_metadata->hdmi_metadata_type1.max_cll; + + return 0; +} +EXPORT_SYMBOL(drm_hdmi_infoframe_set_hdr_metadata); + /** * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with * data from a DRM display mode diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index 799ae49..481f036 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -650,6 +650,150 @@ ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame, return 0; } +/** + * hdmi_drm_infoframe_init() - initialize an HDMI Dynaminc Range and + * mastering infoframe + * @frame: HDMI DRM infoframe + * + * Returns 0 on success or a negative error code on failure. + */ +int hdmi_drm_infoframe_init(struct hdmi_drm_infoframe *frame) +{ + memset(frame, 0, sizeof(*frame)); +
[v11 00/12] Add HDR Metadata Parsing and handling in DRM layer
This patch series enables HDR support in drm. It basically defines HDR metadata structures, property to pass content (after blending) metadata from user space compositors to driver. Dynamic Range and Mastering infoframe creation and sending. ToDo: 1. We need to get the color framework in place for all planes which support HDR content in hardware. This is already in progres and patches are out for review in mailing list. 2. UserSpace/Compositors: Blending policies and metadata blob creation and passing to driver. Work is already in progress by Intel's middleware teams on wayland and the patches for the same are in review. A POC has already been developed by Ville based on wayland. Please refer below link to see the component interactions and usage: https://lists.freedesktop.org/archives/wayland-devel/2017-December/036403.html v2: Updated Ville's POC changes to the patch series.Incorporated cleanups and fixes from Ville. Rebase on latest drm-tip. v3: Fixed a warning causing builds to break on CI. No major change. v4: Addressed Shashank's review comments. v5: Rebase on top of Ville's infoframe refactoring changes. Fixed non modeset case for HDR metadata update. Dropped a redundant patch. v6: Addressed Shashank's review comments and added RB's received. v7: Squashed 2 patches, dropped 1 change and addressed Brian Starkey's and Shashank's review comments. v8: Addressed Jonas Karlman review comments. Added Shashank's RB to the series, fixed a WARN_ON on BYT/CHT. v9: Addressed Ville and Jonas Karlman's review comments. Added the infoframe state readout and metadata reference count. v10: Addressed review comments from Jonas and Ville. Dropped one patch related to i915 fastset handling as per Ville's feedback. v11: Addressed Ville's review comments. Note: v9 version is already tested with Kodi and a confirmation from team kodi has been received. Branch details for the same as below: https://github.com/xbmc/xbmc/tree/feature_drmprime-vaapi v9 of this series is: Tested-by: Jonas Karlman Jonas Karlman (1): drm: Add reference counting on HDR metadata blob Uma Shankar (9): drm: Add HDR source metadata property drm: Parse HDR metadata info from EDID drm: Enable HDR infoframe support drm/i915: Attach HDR metadata property to connector drm/i915: Write HDR infoframe and send to panel drm/i915:Enabled Modeset when HDR Infoframe changes drm/i915: Added DRM Infoframe handling for BYT/CHT video/hdmi: Add Unpack function for DRM infoframe drm/i915: Add state readout for DRM infoframe Ville Syrjälä (2): drm: Add HLG EOTF drm/i915: Enable infoframes on GLK+ for HDR drivers/gpu/drm/drm_atomic_state_helper.c | 5 + drivers/gpu/drm/drm_atomic_uapi.c | 12 ++ drivers/gpu/drm/drm_connector.c | 6 + drivers/gpu/drm/drm_edid.c| 124 ++ drivers/gpu/drm/i915/i915_reg.h | 4 + drivers/gpu/drm/i915/intel_atomic.c | 14 +- drivers/gpu/drm/i915/intel_ddi.c | 3 + drivers/gpu/drm/i915/intel_display.c | 1 + drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_hdmi.c | 67 +++- drivers/video/hdmi.c | 257 ++ include/drm/drm_connector.h | 10 ++ include/drm/drm_edid.h| 5 + include/drm/drm_mode_config.h | 7 + include/linux/hdmi.h | 55 +++ include/uapi/drm/drm_mode.h | 23 +++ 16 files changed, 589 insertions(+), 5 deletions(-) -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v11 03/12] drm: Parse HDR metadata info from EDID
HDR metadata block is introduced in CEA-861.3 spec. Parsing the same to get the panel's HDR metadata. v2: Rebase and added Ville's POC changes to the patch. v3: No Change v4: Addressed Shashank's review comments v5: Addressed Shashank's comment and added his RB. v6: Addressed Jonas Karlman review comments. v7: Adressed Ville's review comments and fixed the issue with length handling. v8: Put the length check as per the convention followed in existing code, as suggested by Ville. Signed-off-by: Uma Shankar Reviewed-by: Shashank Sharma --- drivers/gpu/drm/drm_edid.c | 51 ++ 1 file changed, 51 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 852bdd8..a5ef9f4 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2852,6 +2852,7 @@ static int drm_cvt_modes(struct drm_connector *connector, #define VIDEO_BLOCK 0x02 #define VENDOR_BLOCK0x03 #define SPEAKER_BLOCK 0x04 +#define HDR_STATIC_METADATA_BLOCK 0x6 #define USE_EXTENDED_TAG 0x07 #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 #define EXT_VIDEO_DATA_BLOCK_420 0x0E @@ -3834,6 +3835,54 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) mode->clock = clock; } +static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) +{ + if (cea_db_tag(db) != USE_EXTENDED_TAG) + return false; + + if (db[1] != HDR_STATIC_METADATA_BLOCK) + return false; + + if (cea_db_payload_len(db) < 3) + return false; + + return true; +} + +static uint8_t eotf_supported(const u8 *edid_ext) +{ + return edid_ext[2] & + (BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) | +BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) | +BIT(HDMI_EOTF_SMPTE_ST2084)); +} + +static uint8_t hdr_metadata_type(const u8 *edid_ext) +{ + return edid_ext[3] & + BIT(HDMI_STATIC_METADATA_TYPE1); +} + +static void +drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db) +{ + u16 len; + + len = cea_db_payload_len(db); + + connector->hdr_sink_metadata.hdmi_type1.eotf = + eotf_supported(db); + connector->hdr_sink_metadata.hdmi_type1.metadata_type = + hdr_metadata_type(db); + + if (len >= 4) + connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4]; + if (len >= 5) + connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5]; + if (len >= 6) + connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6]; +} + static void drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) { @@ -4461,6 +4510,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, drm_parse_y420cmdb_bitmap(connector, db); if (cea_db_is_vcdb(db)) drm_parse_vcdb(connector, db); + if (cea_db_is_hdmi_hdr_metadata_block(db)) + drm_parse_hdr_metadata_block(connector, db); } } -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v11 01/12] drm: Add HDR source metadata property
This patch adds a blob property to get HDR metadata information from userspace. This will be send as part of AVI Infoframe to panel. It also implements get() and set() functions for HDR output metadata property.The blob data is received from userspace and saved in connector state, the same is returned as blob in get property call to userspace. v2: Rebase and modified the metadata structure elements as per Ville's POC changes. v3: No Change v4: Addressed Shashank's review comments v5: Rebase. v6: Addressed Brian Starkey's review comments, defined new structure with header for dynamic metadata scalability. Merge get/set property functions for metadata in this patch. v7: Addressed Jonas Karlman review comments and defined separate structure for infoframe to better align with CTA 861.G spec. Added Shashank's RB. v8: Addressed Ville's review comments. Moved sink metadata structure out of uapi headers as suggested by Jonas Karlman. v9: Rebase and addressed Jonas Karlman review comments. v10: Addressed Ville's review comments, dropped the metdata_changed state variable as its not needed anymore. Signed-off-by: Uma Shankar Reviewed-by: Shashank Sharma --- drivers/gpu/drm/drm_atomic_uapi.c | 12 drivers/gpu/drm/drm_connector.c | 6 ++ include/drm/drm_connector.h | 10 ++ include/drm/drm_mode_config.h | 7 +++ include/linux/hdmi.h | 26 ++ include/uapi/drm/drm_mode.h | 23 +++ 6 files changed, 84 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 4131e66..eb22e8b 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -676,6 +676,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, { struct drm_device *dev = connector->dev; struct drm_mode_config *config = &dev->mode_config; + bool replaced = false; + int ret; if (property == config->prop_crtc_id) { struct drm_crtc *crtc = drm_crtc_find(dev, file_priv, val); @@ -726,6 +728,13 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, */ if (state->link_status != DRM_LINK_STATUS_GOOD) state->link_status = val; + } else if (property == config->hdr_output_metadata_property) { + ret = drm_atomic_replace_property_blob_from_id(dev, + &state->hdr_output_metadata, + val, + sizeof(struct hdr_output_metadata), -1, + &replaced); + return ret; } else if (property == config->aspect_ratio_property) { state->picture_aspect_ratio = val; } else if (property == config->content_type_property) { @@ -814,6 +823,9 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, *val = state->colorspace; } else if (property == connector->scaling_mode_property) { *val = state->scaling_mode; + } else if (property == config->hdr_output_metadata_property) { + *val = state->hdr_output_metadata ? + state->hdr_output_metadata->base.id : 0; } else if (property == config->content_protection_property) { *val = state->content_protection; } else if (property == config->writeback_fb_id_property) { diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 11fcd25..c9ac8b9 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1051,6 +1051,12 @@ int drm_connector_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.non_desktop_property = prop; + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB, + "HDR_OUTPUT_METADATA", 0); + if (!prop) + return -ENOMEM; + dev->mode_config.hdr_output_metadata_property = prop; + return 0; } diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index e257b87..f8f4003 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -603,6 +603,12 @@ struct drm_connector_state { * and the connector bpc limitations obtained from edid. */ u8 max_bpc; + + /** +* @hdr_output_metadata: +* DRM blob property for HDR output metadata +*/ + struct drm_property_blob *hdr_output_metadata; }; /** @@ -1237,6 +1243,10 @@ struct drm_connector { * &drm_mode_config.connector_free_work. */ struct llist_node free_node; + + /* HDR metdata */ + struct hdr_output_metadata hdr_output_metadata; + struct hdr_sink_metadata hdr_sink_metadata; }; #define obj_to_connector(x) container_of(x, str
Re: [PATCH v3 04/10] drm: Convert connector_helper_funcs->atomic_check to accept drm_atomic_state
On Thu, May 16, 2019 at 02:07:34PM +0200, Daniel Vetter wrote: > On Thu, May 16, 2019 at 2:02 PM Laurent Pinchart > wrote: > > > > Hi Daniel, > > > > On Mon, May 13, 2019 at 04:47:47PM +0200, Daniel Vetter wrote: > > > On Sat, May 11, 2019 at 10:12:02PM +0300, Laurent Pinchart wrote: > > > > On Thu, May 02, 2019 at 03:49:46PM -0400, Sean Paul wrote: > > > >> From: Sean Paul > > > >> > > > >> Everyone who implements connector_helper_funcs->atomic_check reaches > > > >> into the connector state to get the atomic state. Instead of continuing > > > >> this pattern, change the callback signature to just give atomic state > > > >> and let the driver determine what it does and does not need from it. > > > >> > > > >> Eventually all atomic functions should do this, but that's just too > > > >> much > > > >> busy work for me. > > > > > > > > Given that drivers also access the connector state, isn't this slightly > > > > more inefficient ? > > > > > > It's atomic code, we're trying to optimize for clean code at the expense > > > of a bit of runtime overhead due to more pointer chasing. And I agree with > > > the general push, the pile of old/new_state pointers of various objects > > > we're passing around is confusing. Passing the overall drm_atomic_state > > > seems much more reasonable, and with that we can get everything else. Plus > > > it's much more obvious whether you have the old/new state (since that's > > > explicit when you look it up from the drm_atomic_state). > > > > Yes, I agree it's cleaner. I just hope the atomic state tracking cost > > can be kept under control :-) > > > > By the way, this is likely not going to happen as it would be way too > > intrusive, but it would be nice to rename drm_atomic_state to > > drm_atomic_transaction (or something similar). It doesn't model a state, > > but a change between an old state to a new state. This confused me in > > the past, and I'm sure it can still be confusing to newcomers. > > Why are you the first to suggest this, this is awesome! Can't quite tell if that's irony or not. Anyways, this has been suggested before but no volunteers stepped forward. > drm_atomic_state is indeed not a state, but a transaction representing > how we go from the old to the new state. On a semi-related topic, I've occasionally pondered about moving mode_changed & co. from the obj states to the top level state/transaction (maybe stored as a bitmask). But that would definitely not be a trivial sed job. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 0/8] Allwinner H6 Mali GPU support
On 16/05/2019 12:19, Robin Murphy wrote: [...] > I was expecting to see a similar behaviour to my T620 (which I now > assume was down to 64-bit job descriptors sort-of-but-not-quite working) > but this does look a bit more fundamental - the fact that it's a level 1 > fault with VA == head == tail suggests to me that the MMU can't see the > page tables at all to translate anything. I really hope that the H6 GPU > integration doesn't suffer from the same DMA offset as the Allwinner > display pipeline stuff, because that would be a real pain to support in > io-pgtable. Assuming you mean the case where the physical address (as seen by the CPU) is different from the dma address (as seen by the GPU), then I highly doubt it because mali_kbase doesn't support it: [from kbase_mem_pool_alloc_page() in mali_kbase_mem_pool.c]: dma_addr = dma_map_page(dev, p, 0, PAGE_SIZE, DMA_BIDIRECTIONAL); if (dma_mapping_error(dev, dma_addr)) { __free_page(p); return NULL; } WARN_ON(dma_addr != page_to_phys(p)); That being said it's quite possible there could be something in the bus which needs configuring to make this work - in which case your best bet is to look at the vendor kernel and see if anything extra is poked when the Mali driver is loaded. Steve
Re: [PATCH] drm/bridge: Remove duplicate header
On Tue, May 14, 2019 at 1:05 PM Laurent Pinchart wrote: > > Hi Sabyasachi, > > Thank you for the patch. > > On Tue, May 14, 2019 at 01:01:41PM +0530, Sabyasachi Gupta wrote: > > Remove drm/drm_panel.h which is included more than once > > > > Signed-off-by: Sabyasachi Gupta > > --- > > drivers/gpu/drm/bridge/panel.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > > index 7cbaba2..402b318 100644 > > --- a/drivers/gpu/drm/bridge/panel.c > > +++ b/drivers/gpu/drm/bridge/panel.c > > @@ -15,7 +15,6 @@ > > #include > > #include > > #include > > -#include > > Which tree is this against ? The patch applies on neither drm-next nor > drm-misc-next. > It is against linux-next tree > While at it, could you you reorder the other header alphabetically to > make this kind of issue easier to notice ? > It is already arranged in alphabetical order > > > > struct panel_bridge { > > struct drm_bridge bridge; > > -- > Regards, > > Laurent Pinchart
Re: [v10 09/12] drm/i915:Enabled Modeset when HDR Infoframe changes
On Thu, May 16, 2019 at 10:54:14AM +, Shankar, Uma wrote: > > > >-Original Message- > >From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf > >Of Ville > >Syrjälä > >Sent: Thursday, May 16, 2019 12:57 AM > >To: Shankar, Uma > >Cc: dcasta...@chromium.org; jo...@kwiboo.se; intel-...@lists.freedesktop.org; > >emil.l.veli...@gmail.com; dri-devel@lists.freedesktop.org; > >seanp...@chromium.org > >Subject: Re: [v10 09/12] drm/i915:Enabled Modeset when HDR Infoframe changes > > > >On Tue, May 14, 2019 at 11:06:31PM +0530, Uma Shankar wrote: > >> This patch enables modeset whenever HDR metadata needs to be updated > >> to sink. > >> > >> v2: Addressed Shashank's review comments. > >> > >> v3: Added Shashank's RB. > >> > >> v4: Addressed Ville's review comments. > >> > >> Signed-off-by: Ville Syrjälä > >> Signed-off-by: Uma Shankar > >> Reviewed-by: Shashank Sharma > >> --- > >> drivers/gpu/drm/i915/intel_atomic.c | 14 +- > >> drivers/gpu/drm/i915/intel_hdmi.c | 13 + > >> 2 files changed, 26 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c > >> b/drivers/gpu/drm/i915/intel_atomic.c > >> index 58b8049..6b985e8 100644 > >> --- a/drivers/gpu/drm/i915/intel_atomic.c > >> +++ b/drivers/gpu/drm/i915/intel_atomic.c > >> @@ -105,6 +105,16 @@ int intel_digital_connector_atomic_set_property(struct > >drm_connector *connector, > >>return -EINVAL; > >> } > >> > >> +static bool blob_equal(const struct drm_property_blob *a, > >> + const struct drm_property_blob *b) { > >> + if (a && b) > >> + return a->length == b->length && > >> + !memcmp(a->data, b->data, a->length); > >> + > >> + return !a == !b; > >> +} > >> + > >> int intel_digital_connector_atomic_check(struct drm_connector *conn, > >> struct drm_connector_state *new_state) > >> { > >@@ -132,7 +142,9 @@ > >> int intel_digital_connector_atomic_check(struct drm_connector *conn, > >>new_conn_state->base.colorspace != old_conn_state->base.colorspace > >> || > >>new_conn_state->base.picture_aspect_ratio != old_conn_state- > >>base.picture_aspect_ratio || > >>new_conn_state->base.content_type != old_conn_state- > >>base.content_type || > >> - new_conn_state->base.scaling_mode != old_conn_state- > >>base.scaling_mode) > >> + new_conn_state->base.scaling_mode != old_conn_state- > >>base.scaling_mode || > >> + !blob_equal(new_conn_state->base.hdr_output_metadata, > >> + old_conn_state->base.hdr_output_metadata)) > >>crtc_state->mode_changed = true; > >> > >>return 0; > >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > >> b/drivers/gpu/drm/i915/intel_hdmi.c > >> index b80406b..e97bf6e 100644 > >> --- a/drivers/gpu/drm/i915/intel_hdmi.c > >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c > >> @@ -806,6 +806,11 @@ void intel_read_infoframe(struct intel_encoder > >> *encoder, > >>return true; > >> } > >> > >> +static inline bool is_eotf_supported(u8 output_eotf, u8 sink_eotf) { > >> + return sink_eotf & BIT(output_eotf); } > >> + > >> static bool > >> intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder, > >> struct intel_crtc_state *crtc_state, @@ -814,6 > >+819,7 @@ void > >> intel_read_infoframe(struct intel_encoder *encoder, > >>struct hdmi_drm_infoframe *frame = &crtc_state->infoframes.drm.drm; > >>struct hdr_output_metadata *hdr_metadata; > >>struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > >> + struct drm_connector *connector = conn_state->connector; > >>int ret; > >> > >>if (!(INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))) @@ > >> -828,6 +834,13 @@ void intel_read_infoframe(struct intel_encoder > >> *encoder, > >> > >>hdr_metadata = conn_state->hdr_output_metadata->data; > >> > >> + /* Sink EOTF is Bit map while infoframe is absolute values */ > >> + if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf, > >> + connector->hdr_sink_metadata.hdmi_type1.eotf)) { > >> + DRM_ERROR("EOTF Not Supported\n"); > >> + return true; > >> + } > > > >I was going to say that this should probably be in the > >drm_set_hdr_metdata() or whatever it was called. > > > >But now I'm now wondering if we can even have this check here. What happens > >if > >someone does a display switcheroo while the machine is suspended? Depends on > >when we're going to reprobe the displays I suppose. Hmm. Maybe it's fine. We > >already have a similar issue after all wih the has_hdmi2_sink stuff. > > > >Either way the user triggerable DRM_ERROR()s are at least a nono. > > Ok, Will keep the check and move it inside the drm_set_hdr_metdata(). Also > downgrade > the print as INFO instead of ERROR. Hope this is fine. DEBUG_KMS like everything else. > > >> + > >>crtc_state->infoframes.enable |= > >>intel_hdmi_inf
Re: [PATCH v5 08/11] drm/fb-helper: Remove drm_fb_helper_connector
Hi Noralf. See few comments in the following. Sam On Mon, May 06, 2019 at 08:01:36PM +0200, Noralf Trønnes wrote: > All drivers add all their connectors so there's no need to keep around an > array of available connectors. > > Rename functions which signature is changed since they will be moved to > drm_client in a later patch. > > Signed-off-by: Noralf Trønnes > --- > @@ -1645,8 +1461,9 @@ static int drm_fb_helper_single_fb_probe(struct > drm_fb_helper *fb_helper, > struct drm_client_dev *client = &fb_helper->client; > int ret = 0; > int crtc_count = 0; > - int i; > + struct drm_connector_list_iter conn_iter; > struct drm_fb_helper_surface_size sizes; > + struct drm_connector *connector; > struct drm_mode_set *mode_set; > int best_depth = 0; > > @@ -1663,11 +1480,11 @@ static int drm_fb_helper_single_fb_probe(struct > drm_fb_helper *fb_helper, > if (preferred_bpp != sizes.surface_bpp) > sizes.surface_depth = sizes.surface_bpp = preferred_bpp; > > - drm_fb_helper_for_each_connector(fb_helper, i) { > - struct drm_fb_helper_connector *fb_helper_conn = > fb_helper->connector_info[i]; > + drm_connector_list_iter_begin(fb_helper->dev, &conn_iter); > + drm_client_for_each_connector_iter(connector, &conn_iter) { > struct drm_cmdline_mode *cmdline_mode; I fail to see when to use drm_client_for_each_connector_iter() and when to use a simple for loop. In this patch drm_fb_helper_for_each_connector() is here replaced by the iterator variant and in many other placed a simple for loop. The old code, in this place, would go through all connectors, but this code will skip DRM_MODE_CONNECTOR_WRITEBACK conectors. So it does not like identical code. > > - cmdline_mode = &fb_helper_conn->connector->cmdline_mode; > + cmdline_mode = &connector->cmdline_mode; > > if (cmdline_mode->bpp_specified) { > switch (cmdline_mode->bpp) { > @@ -1692,6 +1509,7 @@ static int drm_fb_helper_single_fb_probe(struct > drm_fb_helper *fb_helper, > break; > } > } > + drm_connector_list_iter_end(&conn_iter); > > /* >* If we run into a situation where, for example, the primary plane > @@ -1876,26 +1694,12 @@ void drm_fb_helper_fill_info(struct fb_info *info, > } > EXPORT_SYMBOL(drm_fb_helper_fill_info); > > +static void drm_client_connectors_enabled(struct drm_connector **connectors, > + unsigned int connector_count, > + bool *enabled) > { > bool any_enabled = false; > struct drm_connector *connector; > int i = 0; > > - drm_fb_helper_for_each_connector(fb_helper, i) { > - connector = fb_helper->connector_info[i]->connector; > + for (i = 0; i < connector_count; i++) { > + connector = connectors[i]; This is for example a place where the drm_fb_helper_for_each_connector() is replaced by a simple for loop. The simple for loop is nice and readable - just a comment replated to the above comment. > struct drm_display_mode *mode = modes[i]; > struct drm_crtc *crtc = crtcs[i]; > struct drm_fb_offset *offset = &offsets[i]; > > if (mode && crtc) { > struct drm_mode_set *modeset = > drm_client_find_modeset(client, crtc); > - struct drm_connector *connector = > - fb_helper->connector_info[i]->connector; > + struct drm_connector *connector = connectors[i]; > > DRM_DEBUG_KMS("desired mode %s set on crtc %d > (%d,%d)\n", > mode->name, crtc->base.id, offset->x, > offset->y); > @@ -2539,6 +2354,10 @@ static void drm_setup_crtcs(struct drm_fb_helper > *fb_helper, > kfree(modes); > kfree(offsets); > kfree(enabled); > +free_connectors: > + for (i = 0; i < connector_count; i++) > + drm_connector_put(connectors[i]); This may call drm_connector_put(NULL) as not all connectors are init. > + kfree(connectors); > } > > /* > @@ -2551,10 +2370,11 @@ static void drm_setup_crtcs(struct drm_fb_helper > *fb_helper, > static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper) > { > struct drm_client_dev *client = &fb_helper->client; > + struct drm_connector_list_iter conn_iter; > struct fb_info *info = fb_helper->fbdev; > unsigned int rotation, sw_rotations = 0; > + struct drm_connector *connector; > struct drm_mode_set *modeset; > - int i; > > mutex_lock(&client->modeset_mutex); > drm_client_for_each_modeset(modeset, client) { > @@ -2571,10 +2391,8 @@ static void drm_setup_crtcs_fb(struct drm_fb_helper > *fb_helper) > } > mutex_unlock(&client->modeset_mutex); > > - mutex_l