[Bug 111077] link_shader and deserialize_glsl_program suddenly consume huge amount of RAM
https://bugs.freedesktop.org/show_bug.cgi?id=111077 --- Comment #8 from Matt Turner --- (In reply to rol...@rptd.ch from comment #4) > I tried compiling from source but it does not work. Seems to have troubles > with libdrm. > > configure: error: Package requirements (libdrm >= 2.4.75 libdrm_intel >= > 2.4.75) were not met: > > Can't seem to get past this one. You must be building with more things enabled than your system Mesa is built with. Run > ebuild /path/to/mesa-19.0.8.ebuild configure clean and copy the line that it uses to configure with meson. Use that in your build that you're using to bisect. Alternatively, add "intel" to your VIDEO_CARDS setting and rebuild libdrm. -- 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 1/2] drm/ttm: use the same attributes when freeing d_page->vaddr
Koenig, Christian 於 2019年7月16日週二 下午9:38寫道: > > Am 11.07.19 um 05:10 schrieb Fuqian Huang: > > In function __ttm_dma_alloc_page(), d_page->addr is allocated > > by dma_alloc_attrs() but freed with use dma_free_coherent() in > > __ttm_dma_free_page(). > > Use the correct dma_free_attrs() to free d_page->vaddr. > > > > Signed-off-by: Fuqian Huang > > Reviewed-by: Christian König > > How do you want to upstream that? Should I pull it into our tree? I just came across this misuse case accidentally. I am not very clear about 'How to upstream that'. Are there more than one way to upstream the code and fix the problem? >From my side, it is ok that you pull it into your tree and fix it or fix it in other way. :) It will be fine if the problem is fixed. Thanks. > > Thanks, > Christian. > > > --- > > drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 6 +- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > > b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > > index d594f7520b7b..7d78e6deac89 100644 > > --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > > +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > > @@ -285,9 +285,13 @@ static int ttm_set_pages_caching(struct dma_pool *pool, > > > > static void __ttm_dma_free_page(struct dma_pool *pool, struct dma_page > > *d_page) > > { > > + unsigned long attrs = 0; > > dma_addr_t dma = d_page->dma; > > d_page->vaddr &= ~VADDR_FLAG_HUGE_POOL; > > - dma_free_coherent(pool->dev, pool->size, (void *)d_page->vaddr, dma); > > + if (pool->type & IS_HUGE) > > + attrs = DMA_ATTR_NO_WARN; > > + > > + dma_free_attrs(pool->dev, pool->size, (void *)d_page->vaddr, dma, > > attrs); > > > > kfree(d_page); > > d_page = NULL; >
[PATCH] drm/radeon: Prefer pcie_capability_read_word()
Commit 8c0d3a02c130 ("PCI: Add accessors for PCI Express Capability") added accessors for the PCI Express Capability so that drivers didn't need to be aware of differences between v1 and v2 of the PCI Express Capability. Replace pci_read_config_word() and pci_write_config_word() calls with pcie_capability_read_word() and pcie_capability_write_word(). Signed-off-by: Frederick Lawler --- drivers/gpu/drm/radeon/cik.c | 70 +- drivers/gpu/drm/radeon/si.c | 73 +++- 2 files changed, 90 insertions(+), 53 deletions(-) diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index ab7b4e2ffcd2..f6c91ac5427a 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -9500,7 +9500,6 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) { struct pci_dev *root = rdev->pdev->bus->self; enum pci_bus_speed speed_cap; - int bridge_pos, gpu_pos; u32 speed_cntl, current_data_rate; int i; u16 tmp16; @@ -9542,12 +9541,7 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) DRM_INFO("enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0\n"); } - bridge_pos = pci_pcie_cap(root); - if (!bridge_pos) - return; - - gpu_pos = pci_pcie_cap(rdev->pdev); - if (!gpu_pos) + if (!pci_is_pcie(root) || !pci_is_pcie(rdev->pdev)) return; if (speed_cap == PCIE_SPEED_8_0GT) { @@ -9557,14 +9551,17 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) u16 bridge_cfg2, gpu_cfg2; u32 max_lw, current_lw, tmp; - pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL, &bridge_cfg); - pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL, &gpu_cfg); + pcie_capability_read_word(root, PCI_EXP_LNKCTL, + &bridge_cfg); + pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL, + &gpu_cfg); tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD; - pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL, tmp16); + pcie_capability_write_word(root, PCI_EXP_LNKCTL, tmp16); tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD; - pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL, tmp16); + pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL, + tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_STATUS1); max_lw = (tmp & LC_DETECTED_LINK_WIDTH_MASK) >> LC_DETECTED_LINK_WIDTH_SHIFT; @@ -9582,15 +9579,23 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) for (i = 0; i < 10; i++) { /* check status */ - pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_DEVSTA, &tmp16); + pcie_capability_read_word(rdev->pdev, + PCI_EXP_DEVSTA, + &tmp16); if (tmp16 & PCI_EXP_DEVSTA_TRPND) break; - pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL, &bridge_cfg); - pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL, &gpu_cfg); + pcie_capability_read_word(root, PCI_EXP_LNKCTL, + &bridge_cfg); + pcie_capability_read_word(rdev->pdev, + PCI_EXP_LNKCTL, + &gpu_cfg); - pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, &bridge_cfg2); - pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &gpu_cfg2); + pcie_capability_read_word(root, PCI_EXP_LNKCTL2, + &bridge_cfg2); + pcie_capability_read_word(rdev->pdev, + PCI_EXP_LNKCTL2, + &gpu_cfg2); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); tmp |= LC_SET_QUIESCE; @@ -9603,26 +9608,39 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) msleep(100); /* linkctl */ -
[PATCH] drm/amdgpu: Prefer pcie_capability_read_word()
Commit 8c0d3a02c130 ("PCI: Add accessors for PCI Express Capability") added accessors for the PCI Express Capability so that drivers didn't need to be aware of differences between v1 and v2 of the PCI Express Capability. Replace pci_read_config_word() and pci_write_config_word() calls with pcie_capability_read_word() and pcie_capability_write_word(). Signed-off-by: Frederick Lawler --- drivers/gpu/drm/amd/amdgpu/cik.c | 70 drivers/gpu/drm/amd/amdgpu/si.c | 70 2 files changed, 88 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c index 07c1f239e9c3..2f33dd0f7a11 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik.c +++ b/drivers/gpu/drm/amd/amdgpu/cik.c @@ -1377,7 +1377,6 @@ static int cik_set_vce_clocks(struct amdgpu_device *adev, u32 evclk, u32 ecclk) static void cik_pcie_gen3_enable(struct amdgpu_device *adev) { struct pci_dev *root = adev->pdev->bus->self; - int bridge_pos, gpu_pos; u32 speed_cntl, current_data_rate; int i; u16 tmp16; @@ -1412,12 +1411,7 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) DRM_INFO("enabling PCIE gen 2 link speeds, disable with amdgpu.pcie_gen2=0\n"); } - bridge_pos = pci_pcie_cap(root); - if (!bridge_pos) - return; - - gpu_pos = pci_pcie_cap(adev->pdev); - if (!gpu_pos) + if (!pci_is_pcie(root) || !pci_is_pcie(adev->pdev)) return; if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3) { @@ -1427,14 +1421,17 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) u16 bridge_cfg2, gpu_cfg2; u32 max_lw, current_lw, tmp; - pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL, &bridge_cfg); - pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL, &gpu_cfg); + pcie_capability_read_word(root, PCI_EXP_LNKCTL, + &bridge_cfg); + pcie_capability_read_word(adev->pdev, PCI_EXP_LNKCTL, + &gpu_cfg); tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD; - pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL, tmp16); + pcie_capability_write_word(root, PCI_EXP_LNKCTL, tmp16); tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD; - pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL, tmp16); + pcie_capability_write_word(adev->pdev, PCI_EXP_LNKCTL, + tmp16); tmp = RREG32_PCIE(ixPCIE_LC_STATUS1); max_lw = (tmp & PCIE_LC_STATUS1__LC_DETECTED_LINK_WIDTH_MASK) >> @@ -1458,15 +1455,23 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) for (i = 0; i < 10; i++) { /* check status */ - pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_DEVSTA, &tmp16); + pcie_capability_read_word(adev->pdev, + PCI_EXP_DEVSTA, + &tmp16); if (tmp16 & PCI_EXP_DEVSTA_TRPND) break; - pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL, &bridge_cfg); - pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL, &gpu_cfg); + pcie_capability_read_word(root, PCI_EXP_LNKCTL, + &bridge_cfg); + pcie_capability_read_word(adev->pdev, + PCI_EXP_LNKCTL, + &gpu_cfg); - pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, &bridge_cfg2); - pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &gpu_cfg2); + pcie_capability_read_word(root, PCI_EXP_LNKCTL2, + &bridge_cfg2); + pcie_capability_read_word(adev->pdev, + PCI_EXP_LNKCTL2, + &gpu_cfg2); tmp = RREG32_PCIE(ixPCIE_LC_CNTL4); tmp |= PCIE_LC_CNTL4__LC_SET_QUIESCE_MASK; @@ -1479,26 +1484,39 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev)
[GIT PULL] drm/arc: Minor improvements
Hi Dave, Daniel, The following changes since commit 7aaddd96d5febcf5b24357a326b3038d49a20532: drm/modes: Don't apply cmdline's rotation if it wasn't specified (2019-07-16 10:34:38 +0200) are available in the Git repository at: g...@github.com:abrodkin/linux.git tags/arcpgu-updates-2019.07.18 for you to fetch changes up to cee17a71656e9e1b5ffc01767844026550d5f4a9: drm/arcpgu: rework encoder search (2019-07-17 23:36:56 +0300) This is a pretty simple improvement that allows to find encoder as the one and only (ARC PGU doesn't support more than one) endpoint instead of using non-standard "encoder-slave" property. Eugeniy Paltsev (1): drm/arcpgu: rework encoder search drivers/gpu/drm/arc/arcpgu_drv.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) Regards, Alexey ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
nouveau-next 5.3
Various cleanup patches, improvements to display colour management, fixes to ACR ("secure boot") issues on various newer systems, TU116 support. Ben. The following changes since commit 3729fe2bc2a01f4cc1aa88be8f64af06084c87d6: Revert "Merge branch 'vmwgfx-next' of git://people.freedesktop.org/~thomash/linux into drm-next" (2019-07-16 04:07:13 +1000) are available in the Git repository at: git://github.com/skeggsb/linux linux-5.3 for you to fetch changes up to aaef0d7ec692985f42b18ca6bac4ddb1180c9dc5: drm/nouveau/secboot: Make acr_r352_ls_gpccs_func static (2019-07-18 14:48:49 +1000) Ben Skeggs (21): drm/nouveau/kms: disallow dual-link harder if hdmi connection detected drm/nouveau/kms/gv100: allow windows to use PACKED8BPP formats drm/nouveau/kms/tu102-: disable input lut when input is already FP16 drm/nouveau/kms/nv50-: disable input lut harder drm/nouveau/core: recognise TU116 chipset drm/nouveau/fifo/gf1xx: convert to using nvkm_fault_data drm/nouveau/fifo/gk104-: fix parsing of mmu fault data drm/nouveau/disp/tu102-: wire up scdc parameter setter drm/nouveau/kms/gv100-: use premultiplied alpha blending between planes drm/nouveau/kms/gv100-: implement csc + enable modern colour managment properties drm/nouveau/kms/nv50-: use __drm_atomic_helper_plane_reset() drm/nouveau/kms/nv50-: create primary plane before overlay planes drm/nouveau/kms/nv50-: attach immutable zpos property to planes drm/nouveau/kms/gv100-: add support for plane zpos property drm/nouveau/kms/gv100-: attach alpha property to planes drm/nouveau/kms/gv100-: attach pixel blend mode property to planes drm/nouveau: fix bogus GPL-2 license header drm/nouveau/therm: skip probing for devices not specified in thermal tables drm/nouveau/therm: don't attempt fan control where PMU is already managing it drm/nouveau/flcn/gp102-: improve implementation of bind_context() on SEC2/GSP drm/nouveau/secboot/gp102-: remove WAR for SEC2 RTOS start bug Colin Ian King (1): drm/nouveau/bios/init: fix spelling mistake "CONDITON" -> "CONDITION" Emil Velikov (1): drm/nouveau: remove open-coded drm_invalid_op() Gustavo A. R. Silva (1): drm/nouveau/mmu: use struct_size() helper Hariprasad Kelam (2): drm/nouveau/dispnv04: subdev/bios.h is included more than once drm/nouveau: fix nvif/device.h is included more than once Ilia Mirkin (7): drm/nouveau/disp/nv50-: force scaler for any non-default LVDS/eDP modes drm/nouveau/disp/nv50-: fix center/aspect-corrected scaling drm/nouveau/kms/nv50-: add fp16 scanout support drm/nouveau/kms/nv50-: remove overlay alpha formats drm/nouveau/kms/gf119-: add ctm property support drm/nouveau/kms/nv50-: enable modern color management properties drm/nouveau: fix bogus GPL-2 license header Karol Herbst (1): drm/nouveau/hwmon: return EINVAL if the GPU is powered down for sensors reads Lyude Paul (1): drm/nouveau/i2c: Enable i2c pads & busses during preinit Ralph Campbell (1): drm/nouveau/dmem: missing mutex_lock in error path Rhys Kidd (3): drm/nouveau/bios: downgrade absence of tmds table to info from an error drm/nouveau/bios/init: handle INIT_RESET_BEGUN devinit opcode drm/nouveau/bios/init: handle INIT_RESET_END devinit opcode Sam Ravnborg (4): drm/nouveau: drop use of DRM_UDELAY drm/nouveau: drop drmP.h from nouveau_drv.h drm/nouveau: drop drmP.h from all header files drm/nouveau: drop use of drmp.h Timo Wiren (1): drm/nouveau/mcp89/mmu: Use mcp77_mmu_new instead of g84_mmu_new on MCP89. Ville Syrjälä (1): drm/nouveau: Disable atomic support on a per-device basis Yongxin Liu (1): drm/nouveau: fix memory leak in nouveau_conn_reset() YueHaibing (1): drm/nouveau/secboot: Make acr_r352_ls_gpccs_func static drivers/gpu/drm/nouveau/Kbuild | 2 +- drivers/gpu/drm/nouveau/dispnv04/Kbuild| 2 +- drivers/gpu/drm/nouveau/dispnv04/arb.c | 2 - drivers/gpu/drm/nouveau/dispnv04/crtc.c| 3 +- drivers/gpu/drm/nouveau/dispnv04/cursor.c | 3 +- drivers/gpu/drm/nouveau/dispnv04/dac.c | 1 - drivers/gpu/drm/nouveau/dispnv04/dfp.c | 2 +- drivers/gpu/drm/nouveau/dispnv04/disp.c| 3 +- drivers/gpu/drm/nouveau/dispnv04/disp.h| 3 +- drivers/gpu/drm/nouveau/dispnv04/hw.c | 1 - drivers/gpu/drm/nouveau/dispnv04/hw.h | 1 - drivers/gpu/drm/nouveau/dispnv04/overlay.c | 1 - drivers/gpu/drm/nouveau/dispnv04/tvmodesnv17.c | 1 - drivers/gpu/drm/nouveau/dispnv04/tvnv04.c | 1 - drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 1 - drivers/gpu/drm/nouveau/dispnv50/Kbuild| 2 +- drivers/gpu/
Re: [PATCH 1/2] drm/ttm: use the same attributes when freeing d_page->vaddr
Am 18.07.19 um 09:21 schrieb Fuqian Huang: > Koenig, Christian 於 2019年7月16日週二 下午9:38寫道: >> Am 11.07.19 um 05:10 schrieb Fuqian Huang: >>> In function __ttm_dma_alloc_page(), d_page->addr is allocated >>> by dma_alloc_attrs() but freed with use dma_free_coherent() in >>> __ttm_dma_free_page(). >>> Use the correct dma_free_attrs() to free d_page->vaddr. >>> >>> Signed-off-by: Fuqian Huang >> Reviewed-by: Christian König >> >> How do you want to upstream that? Should I pull it into our tree? > I just came across this misuse case accidentally. > I am not very clear about 'How to upstream that'. > Are there more than one way to upstream the code and fix the problem? Well I can add it to the TTM tree which send to David or it could be pulled through some other way towards Linus. > From my side, it is ok that you pull it into your tree and fix it or > fix it in other way. > :) It will be fine if the problem is fixed. Ok, fine with me :) Christian. > > Thanks. > >> Thanks, >> Christian. >> >>> --- >>>drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 6 +- >>>1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c >>> b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c >>> index d594f7520b7b..7d78e6deac89 100644 >>> --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c >>> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c >>> @@ -285,9 +285,13 @@ static int ttm_set_pages_caching(struct dma_pool *pool, >>> >>>static void __ttm_dma_free_page(struct dma_pool *pool, struct dma_page >>> *d_page) >>>{ >>> + unsigned long attrs = 0; >>>dma_addr_t dma = d_page->dma; >>>d_page->vaddr &= ~VADDR_FLAG_HUGE_POOL; >>> - dma_free_coherent(pool->dev, pool->size, (void *)d_page->vaddr, dma); >>> + if (pool->type & IS_HUGE) >>> + attrs = DMA_ATTR_NO_WARN; >>> + >>> + dma_free_attrs(pool->dev, pool->size, (void *)d_page->vaddr, dma, >>> attrs); >>> >>>kfree(d_page); >>>d_page = NULL; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 10/33] drm/zte: drop use of drmP.h
On Sun, Jun 30, 2019 at 08:18:59AM +0200, Sam Ravnborg wrote: > Drop use of the deprecated drmP.h header file. > Fix fallout. > > Signed-off-by: Sam Ravnborg > Cc: Shawn Guo > Cc: David Airlie > Cc: Daniel Vetter Acked-by: Shawn Guo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] vt: Grab console_lock around con_is_bound in show_bind
Not really harmful not to, but also not harm in grabbing the lock. And this shuts up a new WARNING I introduced in commit ddde3c18b700 ("vt: More locking checks"). Reported-by: Jens Remus Cc: linux-ker...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-fb...@vger.kernel.org Cc: linux-s...@vger.kernel.org Cc: Greg Kroah-Hartman Cc: Nicolas Pitre Cc: Martin Hostettler Cc: Adam Borowski Cc: Mikulas Patocka Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Sam Ravnborg --- drivers/tty/vt/vt.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index ec92f36ab5c4..34aa39d1aed9 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -3771,7 +3771,11 @@ static ssize_t show_bind(struct device *dev, struct device_attribute *attr, char *buf) { struct con_driver *con = dev_get_drvdata(dev); - int bind = con_is_bound(con->con); + int bind; + + console_lock(); + bind = con_is_bound(con->con); + console_unlock(); return snprintf(buf, PAGE_SIZE, "%i\n", bind); } -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98369] gnome shell slows down after boot
https://bugs.freedesktop.org/show_bug.cgi?id=98369 Michel Dänzer changed: What|Removed |Added Status|NEW |RESOLVED Product|xorg|DRI Component|Driver/AMDgpu |DRM/AMDgpu Assignee|xorg-driver-...@lists.x.org |dri-devel@lists.freedesktop ||.org Resolution|--- |FIXED QA Contact|xorg-t...@lists.x.org | --- Comment #8 from Michel Dänzer --- Assuming this was more likely a kernel issue, and is no longer an issue with current gnome-shell & drivers. Feel free to reopen otherwise. -- 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: [EXT] Re: [PATCH] drm/arm/mali-dp: Add display QoS interface configuration
On Thu, Jul 18, 2019 at 03:29:48AM +, Wen He wrote: > > > > -Original Message- > > From: Liviu Dudau > > Sent: 2019年7月17日 19:22 > > To: Wen He > > Cc: dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org; > > brian.star...@arm.com; airl...@linux.ie; dan...@ffwll.ch; Leo Li > > > > Subject: [EXT] Re: [PATCH] drm/arm/mali-dp: Add display QoS interface > > configuration > > > > > > Hi Wen, > > > > Hi Liviu, Hi Wen, > > > On Wed, Jul 17, 2019 at 05:23:53PM +0800, Wen He wrote: > > > Configure the display Quality of service (QoS) levels to high priority > > > if the level is defined as high as in DTS. The ARQOS for DP500 is > > > driven from the "RQOS" register, needed to program the RQOS register > > > value < 7 for the 4k resolution flicker to disappear on the LS1028A > > > platform. > > > > Thanks for taking time to come up with a more generic patch for your issue! > > > > Thanks for the review and comments, > > > I have a question: what happens if you program the > > MALIDP500_RQOS_QUALITY register to 0xd000d000 for all pixelclocks? > > > > We can't do that, because: > 1. The flicker issue only reproduced in 4k@60. Can you clarify? Does this mean that with the RQOS setting of 0xd000d000 you don't see flicker at lower resolutions, or that you haven't tested at other resolution than 4k@60? > 2. This configuration will be reduced DDR benchmark performance data, because > the > LCDC and DDR are both to connect CCI-400. we have to make sure that DDR > performance > at first when work together with other resolutions. Hmm, I'm not sure I'm sharing the same view of the problem. You are writing into DP500's QoS registers, which don't control how CCI-400 or DDR are going to behave. Now I agree that a more aggressive QoS from DP500 is going to lead to more contention to the DDR, but maybe you can look into CCI-400's QoS settings and tweak the bandwidth allocation there as well. > > > Also, some suggestions further down: > > > > > > > > Signed-off-by: Wen He > > > --- > > > change in v2: > > > - add new implementation for 4k flicker issue on the LS1028A > > > > > > drivers/gpu/drm/arm/malidp_drv.c | 5 + > > > drivers/gpu/drm/arm/malidp_hw.c | 13 + > > > drivers/gpu/drm/arm/malidp_hw.h | 3 +++ > > > drivers/gpu/drm/arm/malidp_regs.h | 12 > > > 4 files changed, 33 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > > > b/drivers/gpu/drm/arm/malidp_drv.c > > > index f25ec4382277..d2b2cf52ac87 100644 > > > --- a/drivers/gpu/drm/arm/malidp_drv.c > > > +++ b/drivers/gpu/drm/arm/malidp_drv.c > > > @@ -818,6 +818,11 @@ static int malidp_bind(struct device *dev) > > > > > > malidp->core_id = version; > > > > > > + hwdev->arqos_high_level = false; > > > > This is not needed. > > > > > + > > > + hwdev->arqos_high_level = of_property_read_bool(dev->of_node, > > > + "arm,malidp-arqos-high-level"); > > > > I think it would be better to have this property as a u32 value, rather > > than a > > boolean, and put the value that we want to program MALIDP_RQOS_QUALITY > > with in there. > > I really thought that, but I've tested DDR performance for 4k resolution with > different RQOS setting, the best test performance data is when set the RQOS > register value's 0xd000d000. So the value is fixed, I think the Boolean type > is > better used here, that's why I did it. Yes, it is fixed for your platform, but not fixed for other ODMs that might decide to use the LS1028A chip to create a board. They might use different DDRs, or their PCB traces might have different lengths. I think it is still valuable to put the 0xd000d000 value into the device tree and read it from there. For LS1028A NXP boards it will do then the right thing. > > > > > Also, you need to add the documentation for this optional property in > > Documentation/devicetree/bindings/display/arm,malidp.txt. > > Understand, I will generate another patch to add this change for the DT > bindings. > > > > > > > + > > > /* set the number of lines used for output of RGB data */ > > > ret = of_property_read_u8_array(dev->of_node, > > > "arm,malidp-output-port-lines", > > > diff --git a/drivers/gpu/drm/arm/malidp_hw.c > > > b/drivers/gpu/drm/arm/malidp_hw.c index 50af399d7f6f..eaa1658cd86b > > > 100644 > > > --- a/drivers/gpu/drm/arm/malidp_hw.c > > > +++ b/drivers/gpu/drm/arm/malidp_hw.c > > > @@ -374,6 +374,19 @@ static void malidp500_modeset(struct > > malidp_hw_device *hwdev, struct videomode * > > > malidp_hw_setbits(hwdev, MALIDP_DISP_FUNC_ILACED, > > MALIDP_DE_DISPLAY_FUNC); > > > else > > > malidp_hw_clearbits(hwdev, MALIDP_DISP_FUNC_ILACED, > > > MALIDP_DE_DISPLAY_FUNC); > > > + > > > + /* > > > + * Program the RQoS register to increasing QoS levels for > > > + * the 4k resolution flicker to disappear o
Re: [PATCH v6 2/5] dma-buf: heaps: Add heap helpers
> +void INIT_HEAP_HELPER_BUFFER(struct heap_helper_buffer *buffer, > + void (*free)(struct heap_helper_buffer *)) Please use a lower case naming following the naming scheme for the rest of the file. > +static void *dma_heap_map_kernel(struct heap_helper_buffer *buffer) > +{ > + void *vaddr; > + > + vaddr = vmap(buffer->pages, buffer->pagecount, VM_MAP, PAGE_KERNEL); > + if (!vaddr) > + return ERR_PTR(-ENOMEM); > + > + return vaddr; > +} Unless I'm misreading the patches this is used for the same pages that also might be dma mapped. In this case you need to use flush_kernel_vmap_range and invalidate_kernel_vmap_range in the right places to ensure coherency between the vmap and device view. Please also document the buffer ownership, as this really can get complicated. > +static vm_fault_t dma_heap_vm_fault(struct vm_fault *vmf) > +{ > + struct vm_area_struct *vma = vmf->vma; > + struct heap_helper_buffer *buffer = vma->vm_private_data; > + > + vmf->page = buffer->pages[vmf->pgoff]; > + get_page(vmf->page); > + > + return 0; > +} Is there any exlusion between mmap / vmap and the device accessing the data? Without that you are going to run into a lot of coherency problems.
Re: [PATCH v6 4/5] dma-buf: heaps: Add CMA heap to dmabuf heaps
This and the previous one seem very much duplicated boilerplate code. Why can't just normal branches for allocating and freeing normal pages vs cma? We even have an existing helper for that with dma_alloc_contiguous().
Re: Why is Thunderbolt 3 limited to 2.5 GT/s on Linux?
On 2019-07-18 11:06 a.m., Timur Kristóf wrote: >>> Thanks Marek, I didn't know about that option. >>> Tried it, here is the output: https://pastebin.com/raw/9SAAbbAA >>> >>> I'm not quite sure how to interpret the numbers, they are >>> inconsistent >>> with the results from both pcie_bw and amdgpu.benchmark, for >>> example >>> GTT->VRAM at a 128 KB is around 1400 MB/s (I assume that is >>> megabytes / >>> sec, right?). >> >> Based on the SDMA results, you have 2.4 GB/s. For 128KB, it's 2.2 >> GB/s for GTT->VRAM copies. > > In the meantime I had a chat with Michel on IRC and he suggested that > maybe amdgpu.benchmark=3 gives lower results because it uses a less > than optimal way to do the benchmark. > > Looking at the results from the mesa benchmark a bit more closely, I > see that the SDMA can do: > VRAM->GTT: 3087 MB/s = 24 Gbit/sec > GTT->VRAM: 2433 MB/s = 19 Gbit/sec > > So on Polaris at least, the SDMA is the fastest, and the other transfer > methods can't match it. I also did the same test on Navi, where it's > different: all other transfer methods are much closer to the SDMA, but > the max speed is still around 20-24 Gbit / sec. > > I still have a few questions: > > 1. Why is the GTT->VRAM copy so much slower than the VRAM->GTT copy? > > 2. Why is the bus limited to 24 Gbit/sec? I would expect the > Thunderbolt port to give me at least 32 Gbit/sec for PCIe traffic. That's unrealistic I'm afraid. As I said on IRC, from the GPU POV there's an 8 GT/s x4 PCIe link, so ~29.8 Gbit/s (= 32 billion bit/s; I missed this nuance on IRC) is the theoretical raw bandwidth. However, in practice that's not achievable due to various overhead[0], and I'm only seeing up to ~90% utilization of the theoretical bandwidth with a "normal" x16 link as well. I wouldn't expect higher utilization without seeing some evidence to suggest it's possible. [0] According to https://www.tested.com/tech/457440-theoretical-vs-actual-bandwidth-pci-express-and-thunderbolt/ , PCIe 3.0 uses 1.54% of the raw bandwidth for its internal encoding. Also keep in mind all CPU<->GPU communication has to go through the PCIe link, e.g. for programming the transfers, in-band signalling from the GPU to the PCIe port where the data is being transferred to/from, ... -- 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
Re: [PATCH] vt: Grab console_lock around con_is_bound in show_bind
Hi Daniel. Patch looks good. You can add my: Acked-by: Sam Ravnborg For good measure I checked all other users of con_is_bound() and they looked good from a locking perspective. Then I looked a bit more for missing locking and lost the overview. On Thu, Jul 18, 2019 at 10:09:03AM +0200, Daniel Vetter wrote: > Not really harmful not to, but also not harm in grabbing the lock. And > this shuts up a new WARNING I introduced in commit ddde3c18b700 ("vt: > More locking checks"). Maybe add the warning that Jens reported to the changelog, in case someone hits something that looks like this warning. Mainly for google fodder, but also in case changelogs are searched. Sam > > Reported-by: Jens Remus > Cc: linux-ker...@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: linux-fb...@vger.kernel.org > Cc: linux-s...@vger.kernel.org > Cc: Greg Kroah-Hartman > Cc: Nicolas Pitre > Cc: Martin Hostettler > Cc: Adam Borowski > Cc: Mikulas Patocka > Signed-off-by: Daniel Vetter > Cc: Daniel Vetter > Cc: Sam Ravnborg > --- > drivers/tty/vt/vt.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c > index ec92f36ab5c4..34aa39d1aed9 100644 > --- a/drivers/tty/vt/vt.c > +++ b/drivers/tty/vt/vt.c > @@ -3771,7 +3771,11 @@ static ssize_t show_bind(struct device *dev, struct > device_attribute *attr, >char *buf) > { > struct con_driver *con = dev_get_drvdata(dev); > - int bind = con_is_bound(con->con); > + int bind; > + > + console_lock(); > + bind = con_is_bound(con->con); > + console_unlock(); > > return snprintf(buf, PAGE_SIZE, "%i\n", bind); > } > -- > 2.20.1
RE: [EXT] Re: [PATCH] drm/arm/mali-dp: Add display QoS interface configuration
> -Original Message- > From: Liviu Dudau > Sent: 2019年7月18日 17:37 > To: Wen He > Cc: dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org; > brian.star...@arm.com; airl...@linux.ie; dan...@ffwll.ch; Leo Li > > Subject: Re: [EXT] Re: [PATCH] drm/arm/mali-dp: Add display QoS interface > configuration > > Caution: EXT Email > > On Thu, Jul 18, 2019 at 03:29:48AM +, Wen He wrote: > > > > > > > -Original Message- > > > From: Liviu Dudau > > > Sent: 2019年7月17日 19:22 > > > To: Wen He > > > Cc: dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org; > > > brian.star...@arm.com; airl...@linux.ie; dan...@ffwll.ch; Leo Li > > > > > > Subject: [EXT] Re: [PATCH] drm/arm/mali-dp: Add display QoS > > > interface configuration > > > > > > > > > Hi Wen, > > > > > > > Hi Liviu, > > Hi Wen, > > > > > > On Wed, Jul 17, 2019 at 05:23:53PM +0800, Wen He wrote: > > > > Configure the display Quality of service (QoS) levels to high > > > > priority if the level is defined as high as in DTS. The ARQOS for > > > > DP500 is driven from the "RQOS" register, needed to program the > > > > RQOS register value < 7 for the 4k resolution flicker to disappear on > > > > the > LS1028A platform. > > > > > > Thanks for taking time to come up with a more generic patch for your > issue! > > > > > > > Thanks for the review and comments, > > > > > I have a question: what happens if you program the > > > MALIDP500_RQOS_QUALITY register to 0xd000d000 for all pixelclocks? > > > > > > > We can't do that, because: > > 1. The flicker issue only reproduced in 4k@60. > > Can you clarify? Does this mean that with the RQOS setting of 0xd000d000 > you don't see flicker at lower resolutions, or that you haven't tested at > other > resolution than 4k@60? I mean that I didn't see flicker issue at lower resolutions without the RQOS setting of 0xd000d000. The ROQS register default value's 0x00010001, in this case, only found the flicker issue can be reproduced in 4k@60, other lower resolutions not found this issue. So If setting the RQOS register value's to 0xd001d001, 4k flicker issue will be resolve. > > > 2. This configuration will be reduced DDR benchmark performance data, > > because the LCDC and DDR are both to connect CCI-400. we have to make > > sure that DDR performance at first when work together with other > resolutions. > > Hmm, I'm not sure I'm sharing the same view of the problem. You are writing > into DP500's QoS registers, which don't control how CCI-400 or DDR are going > to behave. Now I agree that a more aggressive QoS from DP500 is going to > lead to more contention to the DDR, but maybe you can look into CCI-400's > QoS settings and tweak the bandwidth allocation there as well. Yes, I agree with you, but after discussed with our design team, I chose them advice that only change the DP500's QoS. Here're comments from our design team: --- There are multiple registers that impact QOS values in the system, particularly as it relates to transactions from the LCD Controller. "The “best” approach (which in theory should result in the LCD Controller having the bandwidth it needs to avoid buffer underrun, while also having the minimal impact on the system) is to use the dynamic QoS information (ARQOS) coming from the LCD Controller. To do this, you need to program the LCD Controller’s “RQOS” register. See the attached DP-500 TRM, Section 2.4.6, “Display Quality of Service Interface” and Section 4.2.9, “Display engine quality of service registers”. --- > > > > > > Also, some suggestions further down: > > > > > > > > > > > Signed-off-by: Wen He > > > > --- > > > > change in v2: > > > > - add new implementation for 4k flicker issue on the > > > > LS1028A > > > > > > > > drivers/gpu/drm/arm/malidp_drv.c | 5 + > > > > drivers/gpu/drm/arm/malidp_hw.c | 13 + > > > > drivers/gpu/drm/arm/malidp_hw.h | 3 +++ > > > > drivers/gpu/drm/arm/malidp_regs.h | 12 > > > > 4 files changed, 33 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > > > > b/drivers/gpu/drm/arm/malidp_drv.c > > > > index f25ec4382277..d2b2cf52ac87 100644 > > > > --- a/drivers/gpu/drm/arm/malidp_drv.c > > > > +++ b/drivers/gpu/drm/arm/malidp_drv.c > > > > @@ -818,6 +818,11 @@ static int malidp_bind(struct device *dev) > > > > > > > > malidp->core_id = version; > > > > > > > > + hwdev->arqos_high_level = false; > > > > > > This is not needed. > > > > > > > + > > > > + hwdev->arqos_high_level = of_property_read_bool(dev->of_node, > > > > + > > > > + "arm,malidp-arqos-high-level"); > > > > > > I think it would be better to have this property as a u32 value, > > > rather than a boolean, and put the value that we want to program > > > MALIDP_RQOS_QUALITY with in there. > > > > I really thought that, but I've tested DDR performance for 4k > > resolution with different RQOS setting, the best test performance data > >
[Bug 110258] Lenovo V110-15AST AMD A9-9410 AMD R5 Stoney hangs after waking after suspend. 5.0-5.1rc2
https://bugs.freedesktop.org/show_bug.cgi?id=110258 --- Comment #8 from Paul Gover --- Git bisect: 106c7d6148e5aadd394e6701f7e498df49b869d1 is the first bad commit commit 106c7d6148e5aadd394e6701f7e498df49b869d1 Author: Likun Gao Date: Thu Nov 8 20:19:54 2018 +0800 drm/amdgpu: abstract the function of enter/exit safe mode for RLC Abstract the function of amdgpu_gfx_rlc_enter/exit_safe_mode and some part of rlc_init to improve the reusability of RLC. Signed-off-by: Likun Gao Acked-by: Christian König Reviewed-by: Alex Deucher Signed-off-by: Alex Deucher :04 04 8f3b365496f3bbd380a62032f20642ace51c8fef e14ec968011019e3f601df3f15682bb9ae0bafc6 M drivers This run on my HP 15-bw0xx cpu:AMD A9-9420 RADEON R5, 5 COMPUTE CORES 2C+3G with integrated graphics: Stoney [Radeon R2/R3/R4/R5 Graphics] [1002:98E4] I get the same symptoms as above; a more involved scenario that may shed light is to switch to a tty and stop xdm (and hence sddm) so I have no graphics sessions running. pm-suspend followed by resume works and brings me back to the tty, but when I then start xdm, I get a broken screen, usually garbage or grey, and syslog shows something like the following: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled seq=49, emitted seq=51 [drm] IP block:gfx_v8_0 is hung! [drm] GPU recovery disabled. If I enable amdgpu.gpu_recovery=1 kernel: [ 279.726475] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled seq=57, emitted seq=59 kernel: [ 279.726536] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process X pid 2860 thread X:cs0 pid 2861 kernel: [ 279.726542] [drm] IP block:gfx_v8_0 is hung! kernel: [ 279.726609] amdgpu :00:01.0: GPU reset begin! kernel: [ 279.726992] amdgpu :00:01.0: GRBM_SOFT_RESET=0x000F0001 kernel: [ 279.727047] amdgpu :00:01.0: SRBM_SOFT_RESET=0x0100 kernel: [ 279.863162] [drm] recover vram bo from shadow start kernel: [ 279.863164] [drm] recover vram bo from shadow done kernel: [ 279.863166] [drm] Skip scheduling IBs! kernel: [ 279.863191] amdgpu :00:01.0: GPU reset(2) succeeded! kernel: [ 280.015794] [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! I can probably run diagnostics or collect a trace if someone tells me what and how. The problem persists - I still get it running kernel 5.2.1 -- 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] drm/syncobj: return meaningful value to user space
if WAIT_FOR_SUBMIT isn't set and in the meanwhile no underlying fence on syncobj, then return non-block error code to user sapce. Signed-off-by: Chunming Zhou --- drivers/gpu/drm/drm_syncobj.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 361a01a08c18..929f7c64f9a2 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -252,7 +252,7 @@ int drm_syncobj_find_fence(struct drm_file *file_private, return 0; dma_fence_put(*fence); } else { - ret = -EINVAL; + ret = -ENOTBLK; } if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)) @@ -832,7 +832,7 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { continue; } else { - timeout = -EINVAL; + timeout = -ENOTBLK; goto cleanup_entries; } } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: return meaningful value to user space
Quoting Chunming Zhou (2019-07-18 12:13:39) > if WAIT_FOR_SUBMIT isn't set and in the meanwhile no underlying fence on > syncobj, > then return non-block error code to user sapce. Block device required? I presume you tried the EWOULDBLOCK which would be implied by your sentence and found that would be an interesting experience. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: return meaningful value to user space
On 18/07/2019 14:13, Chunming Zhou wrote: if WAIT_FOR_SUBMIT isn't set and in the meanwhile no underlying fence on syncobj, then return non-block error code to user sapce. Signed-off-by: Chunming Zhou --- drivers/gpu/drm/drm_syncobj.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 361a01a08c18..929f7c64f9a2 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -252,7 +252,7 @@ int drm_syncobj_find_fence(struct drm_file *file_private, return 0; dma_fence_put(*fence); } else { - ret = -EINVAL; + ret = -ENOTBLK; } if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)) @@ -832,7 +832,7 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { continue; } else { - timeout = -EINVAL; + timeout = -ENOTBLK; goto cleanup_entries; } } This would break existing tests for binary syncobjs. Is this really what we want? -Lionel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] vt: Grab console_lock around con_is_bound in show_bind
Am 18.07.2019 um 10:09 schrieb Daniel Vetter: Not really harmful not to, but also not harm in grabbing the lock. And this shuts up a new WARNING I introduced in commit ddde3c18b700 ("vt: More locking checks"). Reported-by: Jens Remus Cc: linux-ker...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-fb...@vger.kernel.org Cc: linux-s...@vger.kernel.org Cc: Greg Kroah-Hartman Cc: Nicolas Pitre Cc: Martin Hostettler Cc: Adam Borowski Cc: Mikulas Patocka Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Sam Ravnborg --- drivers/tty/vt/vt.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) Thank you for the quick fix! Looks fine to me. Did test with cat as well as our dump2tar utility. The warning is gone. Tested-by: Jens Remus Regards, Jens Remus -- Linux on Z and z/VSE Development & Service (D3229) IBM Systems & Technology Group, Pure Systems & Modular Software Development IBM Data Privacy Statement: https://www.ibm.com/privacy/us/en/ IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Matthias Hartmann Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [PATCH] drm: Add Grain Media GM12U320 driver
Den 17.07.2019 22.37, skrev Hans de Goede: > Hi Noralf, > > Thank you for the review. > > On 17-07-19 17:24, Noralf Trønnes wrote: >> >> >> Den 12.07.2019 20.53, skrev Hans de Goede: >>> Add a modesetting driver for Grain Media GM12U320 based devices >>> (primarily Acer C120 projector, but there may be compatible devices). >>> >>> This is based on the fb driver from Viacheslav Nurmekhamitov: >>> https://github.com/slavrn/gm12u320 >>> >>> This driver uses drm_simple_display_pipe to deal with all the atomic >>> stuff, gem_shmem_helper functions for buffer management and >>> drm_fbdev_generic_setup for fbdev emulation, so that leaves the driver >>> itself with only the actual code for talking to the gm13u320 chip, >>> leading to a nice simple and clean driver. >> >> Yeah, it's a lot smaller now than when it was first submitted a couple >> of years ago ;-) > > Ack, this is much better now. > >>> Signed-off-by: Hans de Goede >>> --- >>> MAINTAINERS | 5 + >>> drivers/gpu/drm/Kconfig | 2 + >>> drivers/gpu/drm/Makefile | 1 + >>> drivers/gpu/drm/gm12u320/Kconfig | 9 + >>> drivers/gpu/drm/gm12u320/Makefile | 2 + >>> drivers/gpu/drm/gm12u320/gm12u320.c | 817 >>> 6 files changed, 836 insertions(+) >>> +static void gm12u320_pipe_update(struct drm_simple_display_pipe *pipe, >>> + struct drm_plane_state *old_state) >>> +{ >>> + struct drm_plane_state *state = pipe->plane.state; >>> + struct drm_crtc *crtc = &pipe->crtc; >>> + struct drm_rect rect; >>> + >>> + if (drm_atomic_helper_damage_merged(old_state, state, &rect)) >>> + gm12u320_fb_mark_dirty(pipe->plane.state->fb, &rect); >> >> I'm about to write a usb display driver myself, so I'm curious about why >> you punt off the update to a worker instead of doing the update inline? > > There are 2 reasons: > > 1) Doing the update inline is going to take a while, where as userspace > desktop code expects the flip to be nearly instant, so if we block long > here we are introducing significant latency to various userspace code > paths which is undesirable. > > 2) The hardware in question falls back to showing a builtin screen > with driver installation instruction if you do not send it a new > frame every 2 seconds. So if a desktop environment is smart (energy > consumption aware) enough to not re-render needlessly and the user > is just sitting there watching at the screen (so the ui is idle), > then without the worker we will get this driver install screen > after 2 seconds instead of the desktop. This is also why the loop > in the worker uses wait_event_timeout() instead of plain wait_event() > > Interesting that you are working on an usb display driver, can > you share for which hardware this is? > It's more of a generic usb display solution. This is what I replied to Sam yesterday when discussing tiny SPI displays: When I'm done with the tinydrm cleanup, I'm going to work on an idea I have: turn the Raspberry Pi Zero into a $5 USB to HDMI/SDTV/DSI/DPI/SPI-display adapter. I'm planning to write a generic USB host display driver with a matching Linux OTG device driver. I plan to make it easy to do the display OTG side on a microcontroller as well. This way it will be possible for manufacturers to do USB connected displays of (nearly) all sizes without having to write a Linux driver. I'm going to try and do a generic regmap over USB thing that I can put a generic usb display on to of. The idea is that this regmap can be used for generic gpio over usb, adc over usb, etc. I don't know if it will work out in the end but I'll give it a go. >>> + >>> + if (crtc->state->event) { >>> + spin_lock_irq(&crtc->dev->event_lock); >>> + drm_crtc_send_vblank_event(crtc, crtc->state->event); >>> + crtc->state->event = NULL; >>> + spin_unlock_irq(&crtc->dev->event_lock); >>> + } I'm wondering about this signaling here, you're signaling page flip done before the display has been updated. Shouldn't you do that in the worker after the update is sent? > I must admit I spend a lot of time testing the driver as > PRIME secondary GPU video output, filing a bunch of xserver > bugs I hit: > > "Xorg's software-cursor support + mutter results in a mess" > https://gitlab.freedesktop.org/xorg/xserver/issues/828 > "Software cursor results in pointer trails" > https://gitlab.freedesktop.org/xorg/xserver/issues/829 > "xserver does not see secondary GPU devices when they are present when > Xorg starts" > https://gitlab.freedesktop.org/xorg/xserver/issues/830 > "Xorg crashes when unplugging a USB attached secondary (output only) GPU" > https://gitlab.freedesktop.org/xorg/xserver/issues/831 > > I've got a set of fixes for 828 for the 1.20 branch, the > other 3 happen only on master. I've not submitted the 828 > fixes upstream yet, since the fixes have issues with master... > >
Re: [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer()
Den 17.07.2019 21.48, skrev David Lechner: > On 7/17/19 6:58 AM, Noralf Trønnes wrote: >> This is only used by mipi-dbi drivers so move it there. >> >> The reason this isn't moved to the SPI subsystem is that it will in a >> later patch pass a dummy rx buffer for SPI controllers that need this. >> Low memory boards (64MB) can run into a problem allocating such a "large" >> contiguous buffer on every transfer after a long up time. >> This leaves a very specific use case, so we'll keep the function here. >> mipi-dbi will first go through a refactoring though, before this will >> be done. >> >> Remove SPI todo entry now that we're done with the tinydrm.ko SPI code. >> >> Additionally move the mipi_dbi_spi_init() declaration to the other SPI >> functions. >> >> Cc: David Lechner >> Signed-off-by: Noralf Trønnes >> --- > > Acked-by: : David Lechner > > I assume that the comments here might have something to do with the > issue[1] I raised a while back? Should I dust that patch off and resend > it after this series lands? > > [1]: > https://lore.kernel.org/lkml/1519082461-32405-1-git-send-email-da...@lechnology.com/ > Yep, that's the one. I want to refactor mipi-dbi first splitting struct mipi_dbi into an interface and display pipeline part. The helper is going to be moved to drivers/gpu/drm with the other helpers. Please wait until that is done, I want to see what kind of coupling I end up between the two structs and don't want another dependency to deal with if I can avoid it. Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/10] drm/tinydrm: Move tinydrm_machine_little_endian()
Den 17.07.2019 22.09, skrev David Lechner: > On 7/17/19 6:58 AM, Noralf Trønnes wrote: >> The tinydrm helper is going away so move it into the only user mipi-dbi. >> >> Signed-off-by: Noralf Trønnes >> --- >> drivers/gpu/drm/tinydrm/mipi-dbi.c | 15 --- >> include/drm/tinydrm/tinydrm-helpers.h | 15 --- >> 2 files changed, 12 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c >> b/drivers/gpu/drm/tinydrm/mipi-dbi.c >> index 6a8f2d66377f..73db287e5c52 100644 >> --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c >> +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c >> @@ -628,6 +628,15 @@ u32 mipi_dbi_spi_cmd_max_speed(struct spi_device >> *spi, size_t len) >> } >> EXPORT_SYMBOL(mipi_dbi_spi_cmd_max_speed); >> +static bool mipi_dbi_machine_little_endian(void) >> +{ >> +#if defined(__LITTLE_ENDIAN) >> + return true; >> +#else >> + return false; >> +#endif >> +} >> + > > I'm kind of surprised that there isn't something like this elsewhere > in the kernel already. The way this function is being used it kind of > seems like it should be static __always_inline (or a macro) so that > the compiler can do a better job optimizing the code (although it is > a very minor improvement). Ideally this should be in the core somewhere, but I don't want to spend more time on refactoring. I have a usb driver that I want to write and I've waited nearly 2 years now. I got sucked into a giant refactoring hole :-) Doing a quick scan I found virtio_legacy_is_little_endian() which does the same, but it's clearly virtio related and I don't want to drag in that. Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/10] drm/tinydrm: Move tinydrm_display_pipe_init() to mipi-dbi
Den 17.07.2019 22.46, skrev David Lechner: > On 7/17/19 6:58 AM, Noralf Trønnes wrote: >> tinydrm_display_pipe_init() has only one user now, so move it to >> mipi-dbi. >> >> Changes: >> - Remove drm_connector_helper_funcs.detect, it's always connected. >> - Store the connector and mode in mipi_dbi instead of it's own struct. >> >> Otherwise remove some leftover tinydrm-helpers.h inclusions. > > Were the uses of tinydrm-helpers.h removed in this series? If so, then th > #include should probably be removed at the same time. If not, removing the > #include lines could just be an independent patch from this series. tinydrm-helpers.h has only one function declaration that is remove in this patch, so the file is deleted. That's why I need to remove the #include's. Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/4] dt-bindings: backlight: Add led-backlight binding
Cc devicet...@vger.kernel.org list - Rob once informed us this gets higher priority in his queue this way. On 7/17/19 4:15 PM, Jean-Jacques Hiblot wrote: > Add DT binding for led-backlight. > > Signed-off-by: Jean-Jacques Hiblot > --- > .../bindings/leds/backlight/led-backlight.txt | 28 +++ > 1 file changed, 28 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/leds/backlight/led-backlight.txt > > diff --git > a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt > b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt > new file mode 100644 > index ..4c7dfbe7f67a > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt > @@ -0,0 +1,28 @@ > +led-backlight bindings > + > +This binding is used to describe a basic backlight device made of LEDs. > +It can also be used to describe a backlight device controlled by the output > of > +a LED driver. > + > +Required properties: > + - compatible: "led-backlight" > + - leds: a list of LEDs > + > +Optional properties: > + - brightness-levels: Array of distinct brightness levels. The levels must > be > + in the range accepted by the underlying LED devices. > + This is used to translate a backlight brightness level > + into a LED brightness level. If it is not provided, > the > + identity mapping is used. > + > + - default-brightness-level: The default brightness level. > + > +Example: > + > + backlight { > + compatible = "led-backlight"; > + > + leds = <&led1>, <&led2>; > + brightness-levels = <0 4 8 16 32 64 128 255>; > + default-brightness-level = <6>; > + }; > -- Best regards, Jacek Anaszewski
Re: [PATCH] drm/radeon: Prefer pcie_capability_read_word()
On Wed, Jul 17, 2019 at 9:08 PM Frederick Lawler wrote: > > Commit 8c0d3a02c130 ("PCI: Add accessors for PCI Express Capability") > added accessors for the PCI Express Capability so that drivers didn't > need to be aware of differences between v1 and v2 of the PCI > Express Capability. > > Replace pci_read_config_word() and pci_write_config_word() calls with > pcie_capability_read_word() and pcie_capability_write_word(). > > Signed-off-by: Frederick Lawler > --- > drivers/gpu/drm/radeon/cik.c | 70 +- > drivers/gpu/drm/radeon/si.c | 73 +++- > 2 files changed, 90 insertions(+), 53 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c > index ab7b4e2ffcd2..f6c91ac5427a 100644 > --- a/drivers/gpu/drm/radeon/cik.c > +++ b/drivers/gpu/drm/radeon/cik.c > @@ -9500,7 +9500,6 @@ static void cik_pcie_gen3_enable(struct radeon_device > *rdev) > { > struct pci_dev *root = rdev->pdev->bus->self; > enum pci_bus_speed speed_cap; > - int bridge_pos, gpu_pos; > u32 speed_cntl, current_data_rate; > int i; > u16 tmp16; > @@ -9542,12 +9541,7 @@ static void cik_pcie_gen3_enable(struct radeon_device > *rdev) > DRM_INFO("enabling PCIE gen 2 link speeds, disable with > radeon.pcie_gen2=0\n"); > } > > - bridge_pos = pci_pcie_cap(root); > - if (!bridge_pos) > - return; > - > - gpu_pos = pci_pcie_cap(rdev->pdev); > - if (!gpu_pos) > + if (!pci_is_pcie(root) || !pci_is_pcie(rdev->pdev)) > return; > > if (speed_cap == PCIE_SPEED_8_0GT) { > @@ -9557,14 +9551,17 @@ static void cik_pcie_gen3_enable(struct radeon_device > *rdev) > u16 bridge_cfg2, gpu_cfg2; > u32 max_lw, current_lw, tmp; > > - pci_read_config_word(root, bridge_pos + > PCI_EXP_LNKCTL, &bridge_cfg); > - pci_read_config_word(rdev->pdev, gpu_pos + > PCI_EXP_LNKCTL, &gpu_cfg); > + pcie_capability_read_word(root, PCI_EXP_LNKCTL, > + &bridge_cfg); > + pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL, > + &gpu_cfg); > > tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD; > - pci_write_config_word(root, bridge_pos + > PCI_EXP_LNKCTL, tmp16); > + pcie_capability_write_word(root, PCI_EXP_LNKCTL, > tmp16); > > tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD; > - pci_write_config_word(rdev->pdev, gpu_pos + > PCI_EXP_LNKCTL, tmp16); > + pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL, > + tmp16); > > tmp = RREG32_PCIE_PORT(PCIE_LC_STATUS1); > max_lw = (tmp & LC_DETECTED_LINK_WIDTH_MASK) >> > LC_DETECTED_LINK_WIDTH_SHIFT; > @@ -9582,15 +9579,23 @@ static void cik_pcie_gen3_enable(struct radeon_device > *rdev) > > for (i = 0; i < 10; i++) { > /* check status */ > - pci_read_config_word(rdev->pdev, gpu_pos + > PCI_EXP_DEVSTA, &tmp16); > + pcie_capability_read_word(rdev->pdev, > + PCI_EXP_DEVSTA, > + &tmp16); > if (tmp16 & PCI_EXP_DEVSTA_TRPND) > break; > > - pci_read_config_word(root, bridge_pos + > PCI_EXP_LNKCTL, &bridge_cfg); > - pci_read_config_word(rdev->pdev, gpu_pos + > PCI_EXP_LNKCTL, &gpu_cfg); > + pcie_capability_read_word(root, > PCI_EXP_LNKCTL, > + &bridge_cfg); > + pcie_capability_read_word(rdev->pdev, > + PCI_EXP_LNKCTL, > + &gpu_cfg); > > - pci_read_config_word(root, bridge_pos + > PCI_EXP_LNKCTL2, &bridge_cfg2); > - pci_read_config_word(rdev->pdev, gpu_pos + > PCI_EXP_LNKCTL2, &gpu_cfg2); > + pcie_capability_read_word(root, > PCI_EXP_LNKCTL2, > + &bridge_cfg2); > + pcie_capability_read_word(rdev->pdev, > + PCI_EXP_LNKCTL2, > + &gpu_cfg2); > > tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4)
Re: [PATCH] drm/amdgpu: Prefer pcie_capability_read_word()
On Wed, Jul 17, 2019 at 9:08 PM Frederick Lawler wrote: > > Commit 8c0d3a02c130 ("PCI: Add accessors for PCI Express Capability") > added accessors for the PCI Express Capability so that drivers didn't > need to be aware of differences between v1 and v2 of the PCI > Express Capability. > > Replace pci_read_config_word() and pci_write_config_word() calls with > pcie_capability_read_word() and pcie_capability_write_word(). > > Signed-off-by: Frederick Lawler > - pci_read_config_word(adev->pdev, gpu_pos + > PCI_EXP_LNKCTL2, &tmp16); > + pcie_capability_read_word(adev->pdev, > + PCI_EXP_LNKCTL2, > + &tmp16); > tmp16 &= ~((1 << 4) | (7 << 9)); > tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 9))); Same comments as for radeon. Looks like a lot of similar code between radeon and amdgpu. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: return meaningful value to user space
在 2019/7/18 19:31, Lionel Landwerlin 写道: > On 18/07/2019 14:13, Chunming Zhou wrote: >> if WAIT_FOR_SUBMIT isn't set and in the meanwhile no underlying fence >> on syncobj, >> then return non-block error code to user sapce. >> >> Signed-off-by: Chunming Zhou >> --- >> drivers/gpu/drm/drm_syncobj.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_syncobj.c >> b/drivers/gpu/drm/drm_syncobj.c >> index 361a01a08c18..929f7c64f9a2 100644 >> --- a/drivers/gpu/drm/drm_syncobj.c >> +++ b/drivers/gpu/drm/drm_syncobj.c >> @@ -252,7 +252,7 @@ int drm_syncobj_find_fence(struct drm_file >> *file_private, >> return 0; >> dma_fence_put(*fence); >> } else { >> - ret = -EINVAL; >> + ret = -ENOTBLK; >> } >> if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)) >> @@ -832,7 +832,7 @@ static signed long >> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, >> if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { >> continue; >> } else { >> - timeout = -EINVAL; >> + timeout = -ENOTBLK; >> goto cleanup_entries; >> } >> } > > > This would break existing tests for binary syncobjs. How does this breaks binary syncobj? > > Is this really what we want? I want to use this meaningful return value to judge if WaitBeforeSignal happens. I think this is the cheapest change for that. -David > > > -Lionel > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: return meaningful value to user space
在 2019/7/18 19:18, Chris Wilson 写道: > Quoting Chunming Zhou (2019-07-18 12:13:39) >> if WAIT_FOR_SUBMIT isn't set and in the meanwhile no underlying fence on >> syncobj, >> then return non-block error code to user sapce. > Block device required? Yes, if WAIT_FOR_SUBMIT is set, then that will block device. -David > > I presume you tried the EWOULDBLOCK which would be implied by your > sentence and found that would be an interesting experience. > -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: return meaningful value to user space
Quoting Chunming Zhou (2019-07-18 14:04:09) > > 在 2019/7/18 19:18, Chris Wilson 写道: > > Quoting Chunming Zhou (2019-07-18 12:13:39) > >> if WAIT_FOR_SUBMIT isn't set and in the meanwhile no underlying fence on > >> syncobj, > >> then return non-block error code to user sapce. > > Block device required? > > Yes, if WAIT_FOR_SUBMIT is set, then that will block device. No, the error message is that it requires a "block device", not that it will block the device, which is EWOULDBLOCK. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: return meaningful value to user space
在 2019/7/18 21:10, Chris Wilson 写道: > Quoting Chunming Zhou (2019-07-18 14:04:09) >> 在 2019/7/18 19:18, Chris Wilson 写道: >>> Quoting Chunming Zhou (2019-07-18 12:13:39) if WAIT_FOR_SUBMIT isn't set and in the meanwhile no underlying fence on syncobj, then return non-block error code to user sapce. >>> Block device required? >> Yes, if WAIT_FOR_SUBMIT is set, then that will block device. > No, the error message is that it requires a "block device", not that it > will block the device, which is EWOULDBLOCK. OK, I got your mean. Any other possile value suggestted? -David > -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/komeda: Adds error event print functionality
On Thu, Jun 27, 2019 at 04:10:36AM +0100, Lowry Li (Arm Technology China) wrote: > Adds to print the event message when error happens and the same event > will not be printed until next vsync. > > Signed-off-by: Lowry Li (Arm Technology China) > --- > drivers/gpu/drm/arm/display/komeda/Makefile | 1 + > drivers/gpu/drm/arm/display/komeda/komeda_dev.h | 13 ++ > drivers/gpu/drm/arm/display/komeda/komeda_event.c | 144 > ++ > drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 2 + > 4 files changed, 160 insertions(+) > create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_event.c > > diff --git a/drivers/gpu/drm/arm/display/komeda/Makefile > b/drivers/gpu/drm/arm/display/komeda/Makefile > index 38aa584..3f53d2d 100644 > --- a/drivers/gpu/drm/arm/display/komeda/Makefile > +++ b/drivers/gpu/drm/arm/display/komeda/Makefile > @@ -7,6 +7,7 @@ ccflags-y := \ > komeda-y := \ > komeda_drv.o \ > komeda_dev.o \ > + komeda_event.o \ > komeda_format_caps.o \ > komeda_coeffs.o \ > komeda_color_mgmt.o \ > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h > b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h > index 096f9f7..e863ec3 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h > @@ -40,6 +40,17 @@ > #define KOMEDA_ERR_TTNG BIT_ULL(30) > #define KOMEDA_ERR_TTF BIT_ULL(31) > > +#define KOMEDA_ERR_EVENTS\ > + (KOMEDA_EVENT_URUN | KOMEDA_EVENT_IBSY | KOMEDA_EVENT_OVR |\ > + KOMEDA_ERR_TETO | KOMEDA_ERR_TEMR | KOMEDA_ERR_TITR |\ > + KOMEDA_ERR_CPE | KOMEDA_ERR_CFGE | KOMEDA_ERR_AXIE |\ > + KOMEDA_ERR_ACE0 | KOMEDA_ERR_ACE1 | KOMEDA_ERR_ACE2 |\ > + KOMEDA_ERR_ACE3 | KOMEDA_ERR_DRIFTTO| KOMEDA_ERR_FRAMETO |\ > + KOMEDA_ERR_ZME | KOMEDA_ERR_MERR | KOMEDA_ERR_TCF |\ > + KOMEDA_ERR_TTNG | KOMEDA_ERR_TTF) > + > +#define KOMEDA_WARN_EVENTS KOMEDA_ERR_CSCE > + > /* malidp device id */ > enum { > MALI_D71 = 0, > @@ -207,6 +218,8 @@ struct komeda_dev { > > struct komeda_dev *dev_to_mdev(struct device *dev); > > +void komeda_print_events(struct komeda_events *evts); > + > int komeda_dev_resume(struct komeda_dev *mdev); > int komeda_dev_suspend(struct komeda_dev *mdev); > #endif /*_KOMEDA_DEV_H_*/ > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_event.c > b/drivers/gpu/drm/arm/display/komeda/komeda_event.c > new file mode 100644 > index 000..309dbe2 > --- /dev/null > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_event.c > @@ -0,0 +1,144 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * (C) COPYRIGHT 2019 ARM Limited. All rights reserved. > + * Author: James.Qian.Wang > + * > + */ > +#include > + > +#include "komeda_dev.h" > + > +struct komeda_str { > + char *str; > + u32 sz; > + u32 len; > +}; > + > +/* return 0 on success, < 0 on no space. > + */ > +static int komeda_sprintf(struct komeda_str *str, const char *fmt, ...) > +{ > + va_list args; > + int num, free_sz; > + int err; > + > + free_sz = str->sz - str->len; > + if (free_sz <= 0) > + return -ENOSPC; > + > + va_start(args, fmt); > + > + num = vsnprintf(str->str + str->len, free_sz, fmt, args); > + > + va_end(args); > + > + if (num <= free_sz) { > + str->len += num; > + err = 0; > + } else { > + str->len = str->sz; > + err = -ENOSPC; > + } > + > + return err; > +} > + > +static void evt_sprintf(struct komeda_str *str, u64 evt, const char *msg) > +{ > + if (evt) > + komeda_sprintf(str, msg); > +} Why do we need this wrapper? > + > +static void evt_str(struct komeda_str *str, u64 events) > +{ > + if (events == 0ULL) { > + evt_sprintf(str, 1, "None"); > + return; > + } > + > + evt_sprintf(str, events & KOMEDA_EVENT_VSYNC, "VSYNC|"); > + evt_sprintf(str, events & KOMEDA_EVENT_FLIP, "FLIP|"); > + evt_sprintf(str, events & KOMEDA_EVENT_EOW, "EOW|"); > + evt_sprintf(str, events & KOMEDA_EVENT_MODE, "OP-MODE|"); > + > + evt_sprintf(str, events & KOMEDA_EVENT_URUN, "UNDERRUN|"); > + evt_sprintf(str, events & KOMEDA_EVENT_OVR, "OVERRUN|"); > + > + /* GLB error */ > + evt_sprintf(str, events & KOMEDA_ERR_MERR, "MERR|"); > + evt_sprintf(str, events & KOMEDA_ERR_FRAMETO, "FRAMETO|"); > + > + /* DOU error */ > + evt_sprintf(str, events & KOMEDA_ERR_DRIFTTO, "DRIFTTO|"); > + evt_sprintf(str, events & KOMEDA_ERR_FRAMETO, "FRAMETO|"); > + evt_sprintf(str, events & KOMEDA_ERR_TETO, "TETO|"); > + evt_sprintf(str, events & KOMEDA_ERR_CSCE, "CSCE|"); > + > + /* LPU errors or events */ > + evt_sprintf(str, events & KOMEDA_EVENT_IBSY, "IBSY|"); > + evt_sprintf(str, events & KOMEDA_ERR_AXIE, "AXIE|"); > +
Re: [PATCH v4 0/4] Add a generic driver for LED-based backlight
On 7/17/19 4:15 PM, Jean-Jacques Hiblot wrote: > This series aims to add a led-backlight driver, similar to pwm-backlight, > but using a LED class device underneath. > > A few years ago (2015), Tomi Valkeinen posted a series implementing a > backlight driver on top of a LED device: > https://patchwork.kernel.org/patch/7293991/ > https://patchwork.kernel.org/patch/7294001/ > https://patchwork.kernel.org/patch/7293981/ > > The discussion stopped because Tomi lacked the time to work on it. > > changes in v4: > - fix dev_err() messages and commit logs following the advices of Pavel > - cosmetic changes (indents, getting rid of "? 1 : 0" in > led_match_led_node()) > > changes in v3: > - dt binding: don't limit the brightness range to 0-255. Use the range of > the underlying LEDs. as a side-effect, all LEDs must now have the same > range > - driver: Adapt to dt binding update. > - driver: rework probe() for clarity and remove the remaining goto. > > changes in v2: > - handle more than one LED. > - don't make the backlight device a child of the LED controller. > - make brightness-levels and default-brightness-level optional > - removed the option to use a GPIO enable. > - removed the option to use a regulator. It should be handled by the LED > core > - don't make any change to the LED core (not needed anymore) > > Jean-Jacques Hiblot (2): > leds: Add managed API to get a LED from a device driver > dt-bindings: backlight: Add led-backlight binding > > Tomi Valkeinen (2): > leds: Add of_led_get() and led_put() > backlight: add led-backlight driver > > .../bindings/leds/backlight/led-backlight.txt | 28 ++ > drivers/leds/led-class.c | 92 ++ > drivers/video/backlight/Kconfig | 7 + > drivers/video/backlight/Makefile | 1 + > drivers/video/backlight/led_bl.c | 268 ++ > include/linux/leds.h | 6 + > 6 files changed, 402 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/leds/backlight/led-backlight.txt > create mode 100644 drivers/video/backlight/led_bl.c > For the whole set: Acked-by: Jacek Anaszewski Lee - we need to create immutable branch for this set since there will be some interfering changes in the LED core in this cycle. I can create the branch and send the pull request once we will obtain the ack from Rob for DT bindings, unless you have other preference. -- Best regards, Jacek Anaszewski
Re: [PATCH] drm/syncobj: return meaningful value to user space
Quoting Chunming Zhou (2019-07-18 14:15:49) > > 在 2019/7/18 21:10, Chris Wilson 写道: > > Quoting Chunming Zhou (2019-07-18 14:04:09) > >> 在 2019/7/18 19:18, Chris Wilson 写道: > >>> Quoting Chunming Zhou (2019-07-18 12:13:39) > if WAIT_FOR_SUBMIT isn't set and in the meanwhile no underlying fence on > syncobj, > then return non-block error code to user sapce. > >>> Block device required? > >> Yes, if WAIT_FOR_SUBMIT is set, then that will block device. > > No, the error message is that it requires a "block device", not that it > > will block the device, which is EWOULDBLOCK. > > OK, I got your mean. > > Any other possile value suggestted? ENXIO -- for the non-existent "address" along the timeline. EOPNOTSUPP -- also makes sense, but we've started to use that for interface not supported, so would be a bit inconsistent across drm. Or ENOTBLK, being very clear in the commit message how it doesn't reflect the original meaning (as would be given by strerror()) and why the seemingly more natural EWOULDBLOCK doesn't work for drm in this case, and what use case that needs to distinguish this particular case (i.e. why EINVAL isn't good enough). -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: return meaningful value to user space
在 2019/7/18 21:15, Zhou, David(ChunMing) 写道: > 在 2019/7/18 21:10, Chris Wilson 写道: >> Quoting Chunming Zhou (2019-07-18 14:04:09) >>> 在 2019/7/18 19:18, Chris Wilson 写道: Quoting Chunming Zhou (2019-07-18 12:13:39) > if WAIT_FOR_SUBMIT isn't set and in the meanwhile no underlying fence on > syncobj, > then return non-block error code to user sapce. Block device required? >>> Yes, if WAIT_FOR_SUBMIT is set, then that will block device. >> No, the error message is that it requires a "block device", not that it >> will block the device, which is EWOULDBLOCK. Ah, I think I don't want EWOULDBLOCK, which will try again and again ioctl, that is not my movitation. I just need a meaningful value to identify the underlying fence isn't ready on syncobj. Because it is in 'else' case, ENOTBLK is correct to say here need block but WAIT_FOR_SUBMIT isn't set. -David > OK, I got your mean. > > Any other possile value suggestted? > > -David > >> -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/bridge: fix RC_CORE dependency
Using 'imply' causes a new problem, as it allows the case of CONFIG_INPUT=m with RC_CORE=y, which fails to link: drivers/media/rc/rc-main.o: In function `ir_do_keyup': rc-main.c:(.text+0x2b4): undefined reference to `input_event' drivers/media/rc/rc-main.o: In function `rc_repeat': rc-main.c:(.text+0x350): undefined reference to `input_event' drivers/media/rc/rc-main.o: In function `rc_allocate_device': rc-main.c:(.text+0x90c): undefined reference to `input_allocate_device' Add a 'depends on' that allows building both with and without CONFIG_RC_CORE, but disallows combinations that don't link. Fixes: 5023cf32210d ("drm/bridge: make remote control optional") Signed-off-by: Arnd Bergmann --- drivers/gpu/drm/bridge/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index f64c91defdc3..70a8ed2505aa 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -85,8 +85,8 @@ config DRM_SIL_SII8620 tristate "Silicon Image SII8620 HDMI/MHL bridge" depends on OF select DRM_KMS_HELPER + depends on RC_CORE || !RC_CORE imply EXTCON - imply RC_CORE help Silicon Image SII8620 HDMI/MHL bridge chip driver. -- 2.20.0
Re: [Bug 109955] amdgpu [RX Vega 64] system freeze while gaming
Playing dota2 vulkan or GL? I guess it's vulkan: and there I don't know how vulkan deal with multiple WSIs, and how dota2 selects the one it will use. The idea is to clearly identify the code paths which would be "buggy". (my custom distro is x11 native) That said, I don't know the status of wayland: did they reach the same "cluster f*ck" level that x11 is at? (irony, since wayland reason to exist is to be orders of magnitude less kludgy than x11) -- Sylvain ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109955] amdgpu [RX Vega 64] system freeze while gaming
https://bugs.freedesktop.org/show_bug.cgi?id=109955 --- Comment #56 from Sylvain BERTRAND --- Playing dota2 vulkan or GL? I guess it's vulkan: and there I don't know how vulkan deal with multiple WSIs, and how dota2 selects the one it will use. The idea is to clearly identify the code paths which would be "buggy". (my custom distro is x11 native) That said, I don't know the status of wayland: did they reach the same "cluster f*ck" level that x11 is at? (irony, since wayland reason to exist is to be orders of magnitude less kludgy than x11) -- 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: Why is Thunderbolt 3 limited to 2.5 GT/s on Linux?
On Thu, Jul 18, 2019 at 5:11 AM Timur Kristóf wrote: > > On Fri, 2019-07-05 at 09:36 -0400, Alex Deucher wrote: > > On Thu, Jul 4, 2019 at 6:55 AM Michel Dänzer > > wrote: > > > On 2019-07-03 1:04 p.m., Timur Kristóf wrote: > > > > > > There may be other factors, yes. I can't offer a good > > > > > > explanation > > > > > > on > > > > > > what exactly is happening, but it's pretty clear that amdgpu > > > > > > can't > > > > > > take > > > > > > full advantage of the TB3 link, so it seemed like a good idea > > > > > > to > > > > > > start > > > > > > investigating this first. > > > > > > > > > > Yeah, actually it would be consistent with ~16-32 KB > > > > > granularity > > > > > transfers based on your measurements above, which is plausible. > > > > > So > > > > > making sure that the driver doesn't artificially limit the PCIe > > > > > bandwidth might indeed help. > > > > > > > > Can you point me to the place where amdgpu decides the PCIe link > > > > speed? > > > > I'd like to try to tweak it a little bit to see if that helps at > > > > all. > > > > > > I'm not sure offhand, Alex or anyone? > > > > amdgpu_device_get_pcie_info() in amdgpu_device.c. > > > Hi Alex, > > I took a look at amdgpu_device_get_pcie_info() and found that it uses > pcie_bandwidth_available to determine the capabilities of the PCIe > port. However, pcie_bandwidth_available gives you only the current > bandwidth as set by the PCIe link status register, not the maximum > capability. > > I think something along these lines would fix it: > https://pastebin.com/LscEMKMc > > It seems to me that the PCIe capabilities are only used in a few places > in the code, so this patch fixes pp_dpm_pcie. However it doesn't affect > the actual performance. > > What do you think? I think we want the current bandwidth. The GPU can only control the speed of its local link. If there are upstream links that are slower than its local link, it doesn't make sense to run the local link at faster speeds because it will burn extra power it will just run into a bottleneck at the next link. In general, most systems negotiate the fastest link speed supported by both ends at power up. Alex > > Best regards, > Tim > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: return meaningful value to user space
On 18/07/2019 16:02, Chunming Zhou wrote: 在 2019/7/18 19:31, Lionel Landwerlin 写道: On 18/07/2019 14:13, Chunming Zhou wrote: if WAIT_FOR_SUBMIT isn't set and in the meanwhile no underlying fence on syncobj, then return non-block error code to user sapce. Signed-off-by: Chunming Zhou --- drivers/gpu/drm/drm_syncobj.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 361a01a08c18..929f7c64f9a2 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -252,7 +252,7 @@ int drm_syncobj_find_fence(struct drm_file *file_private, return 0; dma_fence_put(*fence); } else { - ret = -EINVAL; + ret = -ENOTBLK; } if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)) @@ -832,7 +832,7 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { continue; } else { - timeout = -EINVAL; + timeout = -ENOTBLK; goto cleanup_entries; } } This would break existing tests for binary syncobjs. How does this breaks binary syncobj? This is used in the submission path of several drivers. Changing the error code will change what the drivers are reporting to userspace and could break tests. i915 doesn't use that function so it's not affected but lima/panfrost/vc4 seem to be. Is this really what we want? I want to use this meaningful return value to judge if WaitBeforeSignal happens. I think this is the cheapest change for that. I thought the plan was to add a new ioctl to query the last submitted value. Did I misunderstand? Thanks, -Lionel -David -Lionel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109206] Kernel 4.20 amdgpu fails to load firmware on Ryzen 2500U
https://bugs.freedesktop.org/show_bug.cgi?id=109206 --- Comment #61 from Talha Khan --- (In reply to Michael Eagle from comment #60) > Created attachment 144787 [details] > attachment-8612-0.html > > I am seeing reports with old BIOS, such as F.19. > I have a 15-cp0001na > https://support.hp.com/ie-en/drivers/selfservice/hp-envy-15-cp-x360- > convertible-pc/20270303/model/23086446 > Latest available is F.42 Rev.A > I am wondering if by any chance would be a match to other models also. The latest BIOS for my HP Envy x360 15-bq100 is F.21: https://support.hp.com/us-en/drivers/selfservice/swdetails/hp-envy-15-bq100-x360-convertible-pc/16851053/model/18706859/swItemId/ob-232955-1?sku=1ZA02AV My current kernel is 5.1.17-300 and so far there hasn't been any boot issues yet. -- 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] drm/bridge: fix RC_CORE dependency
Hi Arnd, On 18.07.2019 15:42, Arnd Bergmann wrote: > Using 'imply' causes a new problem, as it allows the case of > CONFIG_INPUT=m with RC_CORE=y, which fails to link: > > drivers/media/rc/rc-main.o: In function `ir_do_keyup': > rc-main.c:(.text+0x2b4): undefined reference to `input_event' > drivers/media/rc/rc-main.o: In function `rc_repeat': > rc-main.c:(.text+0x350): undefined reference to `input_event' > drivers/media/rc/rc-main.o: In function `rc_allocate_device': > rc-main.c:(.text+0x90c): undefined reference to `input_allocate_device' > > Add a 'depends on' that allows building both with and without > CONFIG_RC_CORE, but disallows combinations that don't link. > > Fixes: 5023cf32210d ("drm/bridge: make remote control optional") > Signed-off-by: Arnd Bergmann Proper solution has been already merged via input tree[1]. [1]: https://lore.kernel.org/lkml/cakdakrtgxnbusukasnglfwuwc7asod9k5baylpwpu7ex-42...@mail.gmail.com/ Regards Andrzej > --- > drivers/gpu/drm/bridge/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index f64c91defdc3..70a8ed2505aa 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -85,8 +85,8 @@ config DRM_SIL_SII8620 > tristate "Silicon Image SII8620 HDMI/MHL bridge" > depends on OF > select DRM_KMS_HELPER > + depends on RC_CORE || !RC_CORE > imply EXTCON > - imply RC_CORE > help > Silicon Image SII8620 HDMI/MHL bridge chip driver. >
Re: [PATCH] drm/bridge: fix RC_CORE dependency
On Thu, Jul 18, 2019 at 4:16 PM Andrzej Hajda wrote: > > Hi Arnd, > > On 18.07.2019 15:42, Arnd Bergmann wrote: > > Using 'imply' causes a new problem, as it allows the case of > > CONFIG_INPUT=m with RC_CORE=y, which fails to link: > > > > drivers/media/rc/rc-main.o: In function `ir_do_keyup': > > rc-main.c:(.text+0x2b4): undefined reference to `input_event' > > drivers/media/rc/rc-main.o: In function `rc_repeat': > > rc-main.c:(.text+0x350): undefined reference to `input_event' > > drivers/media/rc/rc-main.o: In function `rc_allocate_device': > > rc-main.c:(.text+0x90c): undefined reference to `input_allocate_device' > > > > Add a 'depends on' that allows building both with and without > > CONFIG_RC_CORE, but disallows combinations that don't link. > > > > Fixes: 5023cf32210d ("drm/bridge: make remote control optional") > > Signed-off-by: Arnd Bergmann > > > Proper solution has been already merged via input tree[1]. > > > [1]: > https://lore.kernel.org/lkml/cakdakrtgxnbusukasnglfwuwc7asod9k5baylpwpu7ex-42...@mail.gmail.com/ At that link, I only see the patch that caused the regression, not the solution. Are you sure it's fixed? Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: return meaningful value to user space
在 2019/7/18 22:08, Lionel Landwerlin 写道: > On 18/07/2019 16:02, Chunming Zhou wrote: >> 在 2019/7/18 19:31, Lionel Landwerlin 写道: >>> On 18/07/2019 14:13, Chunming Zhou wrote: if WAIT_FOR_SUBMIT isn't set and in the meanwhile no underlying fence on syncobj, then return non-block error code to user sapce. Signed-off-by: Chunming Zhou --- drivers/gpu/drm/drm_syncobj.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 361a01a08c18..929f7c64f9a2 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -252,7 +252,7 @@ int drm_syncobj_find_fence(struct drm_file *file_private, return 0; dma_fence_put(*fence); } else { - ret = -EINVAL; + ret = -ENOTBLK; } if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT)) @@ -832,7 +832,7 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { continue; } else { - timeout = -EINVAL; + timeout = -ENOTBLK; goto cleanup_entries; } } >>> >>> This would break existing tests for binary syncobjs. >> How does this breaks binary syncobj? > > > This is used in the submission path of several drivers. > > Changing the error code will change what the drivers are reporting to > userspace and could break tests. > > > i915 doesn't use that function so it's not affected but > lima/panfrost/vc4 seem to be. anyone from vc4 can confirm this? There are many place in wait_ioctl being able to return previous EINVAL, not sure what they use to. > > >> >> >>> Is this really what we want? >> I want to use this meaningful return value to judge if WaitBeforeSignal >> happens. >> >> I think this is the cheapest change for that. > > > I thought the plan was to add a new ioctl to query the last submitted > value. Yes, that is optional way too. I just thought changing return value is very cheap, isn't it? -David > > Did I misunderstand? > > > Thanks, > > > -Lionel > > >> >> -David >> >> >>> >>> -Lionel >>> >>> > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 06/12] drm/i915: Switch to using DP_MSA_MISC_* defines
From: Ville Syrjälä Now that we have standard defines for the MSA MISC bits lets use them on HSW+ where we program these directly into the TRANS_MSA_MISC register. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_ddi.c | 18 +- drivers/gpu/drm/i915/i915_reg.h | 13 + 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 7dd54f573f35..0c0148c8c996 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -1704,20 +1704,20 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state) WARN_ON(transcoder_is_dsi(cpu_transcoder)); - temp = TRANS_MSA_SYNC_CLK; + temp = DP_MSA_MISC_SYNC_CLOCK; switch (crtc_state->pipe_bpp) { case 18: - temp |= TRANS_MSA_6_BPC; + temp |= DP_MSA_MISC_6_BPC; break; case 24: - temp |= TRANS_MSA_8_BPC; + temp |= DP_MSA_MISC_8_BPC; break; case 30: - temp |= TRANS_MSA_10_BPC; + temp |= DP_MSA_MISC_10_BPC; break; case 36: - temp |= TRANS_MSA_12_BPC; + temp |= DP_MSA_MISC_12_BPC; break; default: MISSING_CASE(crtc_state->pipe_bpp); @@ -1729,7 +1729,7 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state) crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB); if (crtc_state->limited_color_range) - temp |= TRANS_MSA_CEA_RANGE; + temp |= DP_MSA_MISC_COLOR_CEA_RGB; /* * As per DP 1.2 spec section 2.3.4.3 while sending @@ -1737,8 +1737,7 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state) * colorspace information. */ if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) - temp |= TRANS_MSA_SAMPLING_444 | TRANS_MSA_CLRSP_YCBCR | - TRANS_MSA_YCBCR_BT709; + temp |= DP_MSA_MISC_COLOR_YCBCR_444_BT709; /* * As per DP 1.4a spec section 2.2.4.3 [MSA Field for Indication @@ -1747,7 +1746,8 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state) * indicate VSC SDP for the Pixel Encoding/Colorimetry Format. */ if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) - temp |= TRANS_MSA_USE_VSC_SDP; + temp |= DP_MSA_MISC_COLOR_VSC_SDP; + I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp); } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 35133b2ef6c9..91bf714897e5 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9602,18 +9602,7 @@ enum skl_power_gate { #define _TRANSC_MSA_MISC 0x62410 #define _TRANS_EDP_MSA_MISC0x6f410 #define TRANS_MSA_MISC(tran) _MMIO_TRANS2(tran, _TRANSA_MSA_MISC) - -#define TRANS_MSA_SYNC_CLK(1 << 0) -#define TRANS_MSA_SAMPLING_444(2 << 1) -#define TRANS_MSA_CLRSP_YCBCR (1 << 3) -#define TRANS_MSA_YCBCR_BT709 (1 << 4) -#define TRANS_MSA_6_BPC (0 << 5) -#define TRANS_MSA_8_BPC (1 << 5) -#define TRANS_MSA_10_BPC (2 << 5) -#define TRANS_MSA_12_BPC (3 << 5) -#define TRANS_MSA_16_BPC (4 << 5) -#define TRANS_MSA_CEA_RANGE (1 << 3) -#define TRANS_MSA_USE_VSC_SDP (1 << 14) +/* See DP_MSA_MISC_* for the bit definitions */ /* LCPLL Control */ #define LCPLL_CTL _MMIO(0x130040) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/12] drm/i915: Extract intel_hdmi_limited_color_range()
From: Ville Syrjälä Pull the code for computing the limited color range setting into a small helper. We'll add a bit more to it later. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_hdmi.c | 30 +++ 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index b8100cf21dd0..ca377ba3a15e 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2297,6 +2297,24 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector, return true; } +static bool intel_hdmi_limited_color_range(const struct intel_crtc_state *crtc_state, + const struct drm_connector_state *conn_state) +{ + const struct intel_digital_connector_state *intel_conn_state = + to_intel_digital_connector_state(conn_state); + const struct drm_display_mode *adjusted_mode = + &crtc_state->base.adjusted_mode; + + if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) { + /* See CEA-861-E - 5.1 Default Encoding Parameters */ + return crtc_state->has_hdmi_sink && + drm_default_rgb_quant_range(adjusted_mode) == + HDMI_QUANTIZATION_RANGE_LIMITED; + } else { + return intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED; + } +} + int intel_hdmi_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state) @@ -2323,16 +2341,8 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder, if (pipe_config->has_hdmi_sink) pipe_config->has_infoframe = true; - if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) { - /* See CEA-861-E - 5.1 Default Encoding Parameters */ - pipe_config->limited_color_range = - pipe_config->has_hdmi_sink && - drm_default_rgb_quant_range(adjusted_mode) == - HDMI_QUANTIZATION_RANGE_LIMITED; - } else { - pipe_config->limited_color_range = - intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED; - } + pipe_config->limited_color_range = + intel_hdmi_limited_color_range(pipe_config, conn_state); if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) { pipe_config->pixel_multiplier = 2; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 03/12] drm/i915: Fix AVI infoframe quantization range for YCbCr output
From: Ville Syrjälä We're configuring the AVI infoframe quantization range bits as if we're always transmitting RGB pixels. Let's fix this so that we correctly indicate limited range YCC quantization range when transmitting YCbCr instead. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_hdmi.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 9bf28de10401..b8100cf21dd0 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -724,11 +724,16 @@ intel_hdmi_compute_avi_infoframe(struct intel_encoder *encoder, drm_hdmi_avi_infoframe_colorspace(frame, conn_state); - drm_hdmi_avi_infoframe_quant_range(frame, connector, - adjusted_mode, - crtc_state->limited_color_range ? - HDMI_QUANTIZATION_RANGE_LIMITED : - HDMI_QUANTIZATION_RANGE_FULL); + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_RGB) { + drm_hdmi_avi_infoframe_quant_range(frame, connector, + adjusted_mode, + crtc_state->limited_color_range ? + HDMI_QUANTIZATION_RANGE_LIMITED : + HDMI_QUANTIZATION_RANGE_FULL); + } else { + frame->quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT; + frame->ycc_quantization_range = HDMI_YCC_QUANTIZATION_RANGE_LIMITED; + } drm_hdmi_avi_infoframe_content_type(frame, conn_state); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 00/12] drm/i915: YCbCr output fixes and prep work for YCbCr 4:4:4 output
From: Ville Syrjälä I was playing around with YCbCr 4:4:4 output and noticed several things wrong in our code. So I fixed it all and tossed in the prep work for YCbCr 4:4:4 output on ilk+. Ville Syrjälä (12): drm/dp: Add definitons for MSA MISC bits drm/i915: Fix HSW+ DP MSA YCbCr colorspace indication drm/i915: Fix AVI infoframe quantization range for YCbCr output drm/i915: Extract intel_hdmi_limited_color_range() drm/i915: Never set limited_color_range=true for YCbCr output drm/i915: Switch to using DP_MSA_MISC_* defines drm/i915: Don't look at unrelated PIPECONF bits for interlaced readout drm/i915: Simplify intel_get_crtc_ycbcr_config() drm/i915: Add PIPECONF YCbCr 4:4:4 programming for HSW drm/i915: Document ILK+ pipe csc matrix better drm/i915: Set up ILK/SNB csc unit properly for YCbCr output drm/i915: Add PIPECONF YCbCr 4:4:4 programming for ILK-IVB drivers/gpu/drm/i915/display/intel_color.c | 51 ++-- drivers/gpu/drm/i915/display/intel_ddi.c | 28 +++-- drivers/gpu/drm/i915/display/intel_display.c | 120 --- drivers/gpu/drm/i915/display/intel_dp.c | 10 ++ drivers/gpu/drm/i915/display/intel_hdmi.c| 61 +++--- drivers/gpu/drm/i915/i915_reg.h | 31 ++--- include/drm/drm_dp_helper.h | 42 +++ 7 files changed, 247 insertions(+), 96 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 11/12] drm/i915: Set up ILK/SNB csc unit properly for YCbCr output
From: Ville Syrjälä Prepare the pipe csc for YCbCr output on ilk/snb. The main difference to IVB+ is the lack of explicit post offsets, and instead we must configure the CSC info RGB->YUV mode (which takes care of offsetting Cb/Cr properly) and enable the "black screen offset" bit to add the required offset to Y. And while at it throw some comments around the bit defines to document which platforms have which bits. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_color.c | 25 +- drivers/gpu/drm/i915/i915_reg.h| 10 - 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 736c42720daf..a902f7809840 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -1213,6 +1213,21 @@ static u32 ilk_gamma_mode(const struct intel_crtc_state *crtc_state) return GAMMA_MODE_MODE_10BIT; } +static u32 ilk_csc_mode(const struct intel_crtc_state *crtc_state) +{ + /* +* CSC comes after the LUT in RGB->YCbCr mode. +* RGB->YCbCr needs the limited range offsets added to +* the output. RGB limited range output is handled by +* the hw automagically elsewhere. +*/ + if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) + return CSC_BLACK_SCREEN_OFFSET; + + return CSC_MODE_YUV_TO_RGB | + CSC_POSITION_BEFORE_GAMMA; +} + static int ilk_color_check(struct intel_crtc_state *crtc_state) { int ret; @@ -1226,15 +1241,15 @@ static int ilk_color_check(struct intel_crtc_state *crtc_state) !crtc_state->c8_planes; /* -* We don't expose the ctm on ilk/snb currently, -* nor do we enable YCbCr output. Also RGB limited -* range output is handled by the hw automagically. +* We don't expose the ctm on ilk/snb currently, also RGB +* limited range output is handled by the hw automagically. */ - crtc_state->csc_enable = false; + crtc_state->csc_enable = + crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB; crtc_state->gamma_mode = ilk_gamma_mode(crtc_state); - crtc_state->csc_mode = 0; + crtc_state->csc_mode = ilk_csc_mode(crtc_state); ret = intel_color_add_affected_planes(crtc_state); if (ret) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 58471312b8b2..33d535ae0944 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -10106,11 +10106,11 @@ enum skl_power_gate { #define _PIPE_A_CSC_COEFF_BV 0x49024 #define _PIPE_A_CSC_MODE 0x49028 -#define ICL_CSC_ENABLE(1 << 31) -#define ICL_OUTPUT_CSC_ENABLE (1 << 30) -#define CSC_BLACK_SCREEN_OFFSET (1 << 2) -#define CSC_POSITION_BEFORE_GAMMA (1 << 1) -#define CSC_MODE_YUV_TO_RGB (1 << 0) +#define ICL_CSC_ENABLE(1 << 31) /* icl+ */ +#define ICL_OUTPUT_CSC_ENABLE (1 << 30) /* icl+ */ +#define CSC_BLACK_SCREEN_OFFSET (1 << 2) /* ilk/snb */ +#define CSC_POSITION_BEFORE_GAMMA (1 << 1) /* pre-glk */ +#define CSC_MODE_YUV_TO_RGB (1 << 0) /* ilk/snb */ #define _PIPE_A_CSC_PREOFF_HI 0x49030 #define _PIPE_A_CSC_PREOFF_ME 0x49034 -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 01/12] drm/dp: Add definitons for MSA MISC bits
From: Ville Syrjälä Add definitions for the MSA (Main Stream Attribute) MISC bits. On some hardware you can program these directly into a register. Signed-off-by: Ville Syrjälä --- include/drm/drm_dp_helper.h | 42 + 1 file changed, 42 insertions(+) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 397896b5b21a..d3f429795fea 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -42,6 +42,48 @@ * 1.2 formally includes both eDP and DPI definitions. */ +/* MSA (Main Stream Attribute) MISC bits (as MISC1<<8|MISC0) */ +#define DP_MSA_MISC_SYNC_CLOCK (1 << 0) +#define DP_MSA_MISC_INTERLACE_VTOTAL_EVEN (1 << 8) +#define DP_MSA_MISC_STEREO_NO_3D (0 << 9) +#define DP_MSA_MISC_STEREO_PROG_RIGHT_EYE (1 << 9) +#define DP_MSA_MISC_STEREO_PROG_LEFT_EYE (3 << 9) +/* bits per component for non-RAW */ +#define DP_MSA_MISC_6_BPC (0 << 5) +#define DP_MSA_MISC_8_BPC (1 << 5) +#define DP_MSA_MISC_10_BPC (2 << 5) +#define DP_MSA_MISC_12_BPC (3 << 5) +#define DP_MSA_MISC_16_BPC (4 << 5) +/* bits per component for RAW */ +#define DP_MSA_MISC_RAW_6_BPC (1 << 5) +#define DP_MSA_MISC_RAW_7_BPC (2 << 5) +#define DP_MSA_MISC_RAW_8_BPC (3 << 5) +#define DP_MSA_MISC_RAW_10_BPC (4 << 5) +#define DP_MSA_MISC_RAW_12_BPC (5 << 5) +#define DP_MSA_MISC_RAW_14_BPC (6 << 5) +#define DP_MSA_MISC_RAW_16_BPC (7 << 5) +/* pixel encoding/colorimetry format */ +#define _DP_MSA_MISC_COLOR(misc1_7, misc0_21, misc0_3, misc0_4) \ + ((misc1_7) << 15 | (misc0_4) << 4 | (misc0_3) << 3 | ((misc0_21) << 1)) +#define DP_MSA_MISC_COLOR_RGB _DP_MSA_MISC_COLOR(0, 0, 0, 0) +#define DP_MSA_MISC_COLOR_CEA_RGB _DP_MSA_MISC_COLOR(0, 0, 1, 0) +#define DP_MSA_MISC_COLOR_RGB_WIDE_FIXED _DP_MSA_MISC_COLOR(0, 3, 0, 0) +#define DP_MSA_MISC_COLOR_RGB_WIDE_FLOAT _DP_MSA_MISC_COLOR(0, 3, 0, 1) +#define DP_MSA_MISC_COLOR_Y_ONLY _DP_MSA_MISC_COLOR(1, 0, 0, 0) +#define DP_MSA_MISC_COLOR_RAW _DP_MSA_MISC_COLOR(1, 1, 0, 0) +#define DP_MSA_MISC_COLOR_YCBCR_422_BT601 _DP_MSA_MISC_COLOR(0, 1, 1, 0) +#define DP_MSA_MISC_COLOR_YCBCR_422_BT709 _DP_MSA_MISC_COLOR(0, 1, 1, 1) +#define DP_MSA_MISC_COLOR_YCBCR_444_BT601 _DP_MSA_MISC_COLOR(0, 2, 1, 0) +#define DP_MSA_MISC_COLOR_YCBCR_444_BT709 _DP_MSA_MISC_COLOR(0, 2, 1, 1) +#define DP_MSA_MISC_COLOR_XVYCC_422_BT601 _DP_MSA_MISC_COLOR(0, 1, 0, 0) +#define DP_MSA_MISC_COLOR_XVYCC_422_BT709 _DP_MSA_MISC_COLOR(0, 1, 0, 1) +#define DP_MSA_MISC_COLOR_XVYCC_444_BT601 _DP_MSA_MISC_COLOR(0, 2, 0, 0) +#define DP_MSA_MISC_COLOR_XVYCC_444_BT709 _DP_MSA_MISC_COLOR(0, 2, 0, 1) +#define DP_MSA_MISC_COLOR_OPRGB_DP_MSA_MISC_COLOR(0, 0, 1, 1) +#define DP_MSA_MISC_COLOR_DCI_P3 _DP_MSA_MISC_COLOR(0, 3, 1, 0) +#define DP_MSA_MISC_COLOR_COLOR_PROFILE_DP_MSA_MISC_COLOR(0, 3, 1, 1) +#define DP_MSA_MISC_COLOR_VSC_SDP (1 << 14) + #define DP_AUX_MAX_PAYLOAD_BYTES 16 #define DP_AUX_I2C_WRITE 0x0 -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/12] drm/i915: Document ILK+ pipe csc matrix better
From: Ville Syrjälä Add comments to explain the ilk pipe csc operation a bit better. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_color.c | 26 +- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 23a84dd7989f..736c42720daf 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -42,6 +42,21 @@ #define LEGACY_LUT_LENGTH 256 +/* + * ILK+ csc matrix: + * + * |R/Cr| | c0 c1 c2 | ( |R/Cr| |preoff0| ) |postoff0| + * |G/Y | = | c3 c4 c5 | x ( |G/Y | + |preoff1| ) + |postoff1| + * |B/Cb| | c6 c7 c8 | ( |B/Cb| |preoff2| ) |postoff2| + * + * ILK/SNB don't have explicit post offsets, and instead + * CSC_MODE_YUV_TO_RGB and CSC_BLACK_SCREEN_OFFSET are used: + * CSC_MODE_YUV_TO_RGB=0 + CSC_BLACK_SCREEN_OFFSET=0 -> 1/2, 0, 1/2 + * CSC_MODE_YUV_TO_RGB=0 + CSC_BLACK_SCREEN_OFFSET=1 -> 1/2, 1/16, 1/2 + * CSC_MODE_YUV_TO_RGB=1 + CSC_BLACK_SCREEN_OFFSET=0 -> 0, 0, 0 + * CSC_MODE_YUV_TO_RGB=1 + CSC_BLACK_SCREEN_OFFSET=1 -> 1/16, 1/16, 1/16 + */ + /* * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point * format). This macro takes the coefficient we want transformed and the @@ -59,37 +74,38 @@ #define ILK_CSC_POSTOFF_LIMITED_RANGE (16 * (1 << 12) / 255) +/* Nop pre/post offsets */ static const u16 ilk_csc_off_zero[3] = {}; +/* Identity matrix */ static const u16 ilk_csc_coeff_identity[9] = { ILK_CSC_COEFF_1_0, 0, 0, 0, ILK_CSC_COEFF_1_0, 0, 0, 0, ILK_CSC_COEFF_1_0, }; +/* Limited range RGB post offsets */ static const u16 ilk_csc_postoff_limited_range[3] = { ILK_CSC_POSTOFF_LIMITED_RANGE, ILK_CSC_POSTOFF_LIMITED_RANGE, ILK_CSC_POSTOFF_LIMITED_RANGE, }; +/* Full range RGB -> limited range RGB matrix */ static const u16 ilk_csc_coeff_limited_range[9] = { ILK_CSC_COEFF_LIMITED_RANGE, 0, 0, 0, ILK_CSC_COEFF_LIMITED_RANGE, 0, 0, 0, ILK_CSC_COEFF_LIMITED_RANGE, }; -/* - * These values are direct register values specified in the Bspec, - * for RGB->YUV conversion matrix (colorspace BT709) - */ +/* BT.709 full range RGB -> limited range YCbCr matrix */ static const u16 ilk_csc_coeff_rgb_to_ycbcr[9] = { 0x1e08, 0x9cc0, 0xb528, 0x2ba8, 0x09d8, 0x37e8, 0xbce8, 0x9ad8, 0x1e08, }; -/* Post offset values for RGB->YCBCR conversion */ +/* Limited range YCbCr post offsets */ static const u16 ilk_csc_postoff_rgb_to_ycbcr[3] = { 0x0800, 0x0100, 0x0800, }; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 05/12] drm/i915: Never set limited_color_range=true for YCbCr output
From: Ville Syrjälä crtc_state->limited_color_range only applies to RGB output but we're currently setting it even for YCbCr output. That will lead to conflicting MSA and PIPECONF settings which can mess up the image. Let's make sure limited_color_range stays unset with YCbCr output. Also WARN if we end up with such a bogus combination when programming the MSA MISC bits as it's impossible to even indicate quantization rangle for YCbCr via MSA MISC. YCbCr output is simply assumed to be limited range always. Note that VSC SDP does provide a mechanism for full range YCbCr, so in the future we may want to rethink how we compute/store this state. And for good measure we add the same WARN to the HDMI path. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_ddi.c | 10 +++--- drivers/gpu/drm/i915/display/intel_dp.c | 10 ++ drivers/gpu/drm/i915/display/intel_hdmi.c | 20 +--- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 157c5851a688..7dd54f573f35 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -1706,9 +1706,6 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state) temp = TRANS_MSA_SYNC_CLK; - if (crtc_state->limited_color_range) - temp |= TRANS_MSA_CEA_RANGE; - switch (crtc_state->pipe_bpp) { case 18: temp |= TRANS_MSA_6_BPC; @@ -1727,6 +1724,13 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state) break; } + /* nonsense combination */ + WARN_ON(crtc_state->limited_color_range && + crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB); + + if (crtc_state->limited_color_range) + temp |= TRANS_MSA_CEA_RANGE; + /* * As per DP 1.2 spec section 2.3.4.3 while sending * YCBCR 444 signals we should program MSA MISC1/0 fields with diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 0eb5d66f87a7..84d2724f0854 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2126,6 +2126,16 @@ bool intel_dp_limited_color_range(const struct intel_crtc_state *crtc_state, const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode; + /* +* Our YCbCr output is always limited range. +* crtc_state->limited_color_range only applies to RGB, +* and it must never be set for YCbCr or we risk setting +* some conflicting bits in PIPECONF which will mess up +* the colors on the monitor. +*/ + if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) + return false; + if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) { /* * See: diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index ca377ba3a15e..3603eacb82ba 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -724,6 +724,10 @@ intel_hdmi_compute_avi_infoframe(struct intel_encoder *encoder, drm_hdmi_avi_infoframe_colorspace(frame, conn_state); + /* nonsense combination */ + WARN_ON(crtc_state->limited_color_range && + crtc_state->output_format == INTEL_OUTPUT_FORMAT_RGB); + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_RGB) { drm_hdmi_avi_infoframe_quant_range(frame, connector, adjusted_mode, @@ -2305,6 +2309,16 @@ static bool intel_hdmi_limited_color_range(const struct intel_crtc_state *crtc_s const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode; + /* +* Our YCbCr output is always limited range. +* crtc_state->limited_color_range only applies to RGB, +* and it must never be set for YCbCr or we risk setting +* some conflicting bits in PIPECONF which will mess up +* the colors on the monitor. +*/ + if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) + return false; + if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) { /* See CEA-861-E - 5.1 Default Encoding Parameters */ return crtc_state->has_hdmi_sink && @@ -2341,9 +2355,6 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder, if (pipe_config->has_hdmi_sink) pipe_config->has_infoframe = true; - pipe_config->limited_color_range = - intel_hdmi_limited_color_range(pipe_config, conn_state); - if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) { pipe_config->pixel_multiplier =
[PATCH 07/12] drm/i915: Don't look at unrelated PIPECONF bits for interlaced readout
From: Ville Syrjälä Since HSW the PIPECONF progressive vs. interlaced selection is done with just two bits instead of the earlier three. Let's not look at the extra bit on HSW+. Also gen2 doesn't support interlaced displays at all. This is actually fine as is currently because the extra bit is mbz (as are all three bits on gen2). But just to avoid mishaps in the future if the bits get reused let's only look at what's properly defined. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index e25b82d07d4f..ffdc350dc24a 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -8189,6 +8189,21 @@ static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state) (crtc_state->pipe_src_h - 1)); } +static bool intel_pipe_is_interlaced(struct intel_crtc_state *crtc_state) +{ + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; + + if (IS_GEN(dev_priv, 2)) + return false; + + if (INTEL_GEN(dev_priv) >= 9 || + IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv)) + return I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK_HSW; + else + return I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK; +} + static void intel_get_pipe_timings(struct intel_crtc *crtc, struct intel_crtc_state *pipe_config) { @@ -8227,7 +8242,7 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc, pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp & 0x) + 1; pipe_config->base.adjusted_mode.crtc_vsync_end = ((tmp >> 16) & 0x) + 1; - if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK) { + if (intel_pipe_is_interlaced(pipe_config)) { pipe_config->base.adjusted_mode.flags |= DRM_MODE_FLAG_INTERLACE; pipe_config->base.adjusted_mode.crtc_vtotal += 1; pipe_config->base.adjusted_mode.crtc_vblank_end += 1; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 08/12] drm/i915: Simplify intel_get_crtc_ycbcr_config()
From: Ville Syrjälä Make intel_get_crtc_ycbcr_config() simpler and rename it to bdw_get_pipemisc_output_format() to better reflect what it does. Also toss in some comments to document that the 4:2:0 PIPECONF bits are glk+ only. They are mbz on earlier platforms so reading them unconditionally is safe however. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 71 +--- drivers/gpu/drm/i915/i915_reg.h | 4 +- 2 files changed, 34 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index ffdc350dc24a..1dd1aa29a649 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -8713,47 +8713,24 @@ static void chv_crtc_clock_get(struct intel_crtc *crtc, pipe_config->port_clock = chv_calc_dpll_params(refclk, &clock); } -static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc, - struct intel_crtc_state *pipe_config) +static enum intel_output_format +bdw_get_pipemisc_output_format(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - enum intel_output_format output = INTEL_OUTPUT_FORMAT_RGB; - - pipe_config->lspcon_downsampling = false; - - if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) { - u32 tmp = I915_READ(PIPEMISC(crtc->pipe)); - - if (tmp & PIPEMISC_OUTPUT_COLORSPACE_YUV) { - bool ycbcr420_enabled = tmp & PIPEMISC_YUV420_ENABLE; - bool blend = tmp & PIPEMISC_YUV420_MODE_FULL_BLEND; - - if (ycbcr420_enabled) { - /* We support 4:2:0 in full blend mode only */ - if (!blend) - output = INTEL_OUTPUT_FORMAT_INVALID; - else if (!(IS_GEMINILAKE(dev_priv) || - INTEL_GEN(dev_priv) >= 10)) - output = INTEL_OUTPUT_FORMAT_INVALID; - else - output = INTEL_OUTPUT_FORMAT_YCBCR420; - } else { - /* -* Currently there is no interface defined to -* check user preference between RGB/YCBCR444 -* or YCBCR420. So the only possible case for -* YCBCR444 usage is driving YCBCR420 output -* with LSPCON, when pipe is configured for -* YCBCR444 output and LSPCON takes care of -* downsampling it. -*/ - pipe_config->lspcon_downsampling = true; - output = INTEL_OUTPUT_FORMAT_YCBCR444; - } - } - } + u32 tmp; + + tmp = I915_READ(PIPEMISC(crtc->pipe)); - pipe_config->output_format = output; + if (tmp & PIPEMISC_YUV420_ENABLE) { + /* We support 4:2:0 in full blend mode only */ + WARN_ON((tmp & PIPEMISC_YUV420_MODE_FULL_BLEND) == 0); + + return INTEL_OUTPUT_FORMAT_YCBCR420; + } else if (tmp & PIPEMISC_OUTPUT_COLORSPACE_YUV) { + return INTEL_OUTPUT_FORMAT_YCBCR444; + } else { + return INTEL_OUTPUT_FORMAT_RGB; + } } static void i9xx_get_pipe_color_config(struct intel_crtc_state *crtc_state) @@ -10445,7 +10422,23 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, } intel_get_pipe_src_size(crtc, pipe_config); - intel_get_crtc_ycbcr_config(crtc, pipe_config); + + if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) { + pipe_config->output_format = + bdw_get_pipemisc_output_format(crtc); + + /* +* Currently there is no interface defined to +* check user preference between RGB/YCBCR444 +* or YCBCR420. So the only possible case for +* YCBCR444 usage is driving YCBCR420 output +* with LSPCON, when pipe is configured for +* YCBCR444 output and LSPCON takes care of +* downsampling it. +*/ + pipe_config->lspcon_downsampling = + pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR444; + } pipe_config->gamma_mode = I915_READ(GAMMA_MODE(crtc->pipe)); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 91bf714897e5..66f7f417231f 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5803,8 +5803,8 @@ enum { #define
[PATCH 02/12] drm/i915: Fix HSW+ DP MSA YCbCr colorspace indication
From: Ville Syrjälä Looks like we're currently setting the MSA to xvYCC BT.709 instead of the YCbCr BT.601 claimed by the comment. But even that comment is wrong since we configure the CSC matrix to BT.709. Let's remove the bogus statement from the comment and fix the MSA to indicate YCbCr BT.709 so that it matches the actual pixel data we're transmitting. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_ddi.c | 6 -- drivers/gpu/drm/i915/i915_reg.h | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 18bc0f2690c9..157c5851a688 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -1730,10 +1730,12 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state) /* * As per DP 1.2 spec section 2.3.4.3 while sending * YCBCR 444 signals we should program MSA MISC1/0 fields with -* colorspace information. The output colorspace encoding is BT601. +* colorspace information. */ if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) - temp |= TRANS_MSA_SAMPLING_444 | TRANS_MSA_CLRSP_YCBCR; + temp |= TRANS_MSA_SAMPLING_444 | TRANS_MSA_CLRSP_YCBCR | + TRANS_MSA_YCBCR_BT709; + /* * As per DP 1.4a spec section 2.2.4.3 [MSA Field for Indication * of Color Encoding Format and Content Color Gamut] while sending diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index fdd9bc01e694..35133b2ef6c9 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9605,7 +9605,8 @@ enum skl_power_gate { #define TRANS_MSA_SYNC_CLK(1 << 0) #define TRANS_MSA_SAMPLING_444(2 << 1) -#define TRANS_MSA_CLRSP_YCBCR (2 << 3) +#define TRANS_MSA_CLRSP_YCBCR (1 << 3) +#define TRANS_MSA_YCBCR_BT709 (1 << 4) #define TRANS_MSA_6_BPC (0 << 5) #define TRANS_MSA_8_BPC (1 << 5) #define TRANS_MSA_10_BPC (2 << 5) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 09/12] drm/i915: Add PIPECONF YCbCr 4:4:4 programming for HSW
From: Ville Syrjälä On HSW the pipe colorspace is configured via PIPECONF (as opposed to PIPEMISC in BDW+). Let's configure+readout that stuff correctly. Enablling YCbCr 4:4:4 output will now be a simple matter of setting crtc_state->output_format appropriately in the encoder .compute_config(). Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 13 - drivers/gpu/drm/i915/i915_reg.h | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 1dd1aa29a649..bd3ff96c1618 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -9430,6 +9430,10 @@ static void haswell_set_pipeconf(const struct intel_crtc_state *crtc_state) else val |= PIPECONF_PROGRESSIVE; + if (IS_HASWELL(dev_priv) && + crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) + val |= PIPECONF_OUTPUT_COLORSPACE_YUV_HSW; + I915_WRITE(PIPECONF(cpu_transcoder), val); POSTING_READ(PIPECONF(cpu_transcoder)); } @@ -10423,7 +10427,14 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, intel_get_pipe_src_size(crtc, pipe_config); - if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) { + if (IS_HASWELL(dev_priv)) { + u32 tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder)); + + if (tmp & PIPECONF_OUTPUT_COLORSPACE_YUV_HSW) + pipe_config->output_format = INTEL_OUTPUT_FORMAT_YCBCR444; + else + pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB; + } else { pipe_config->output_format = bdw_get_pipemisc_output_format(crtc); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 66f7f417231f..58471312b8b2 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5712,6 +5712,7 @@ enum { #define PIPECONF_CXSR_DOWNCLOCK (1 << 16) #define PIPECONF_EDP_RR_MODE_SWITCH_VLV (1 << 14) #define PIPECONF_COLOR_RANGE_SELECT (1 << 13) +#define PIPECONF_OUTPUT_COLORSPACE_YUV_HSW (1 << 11) /* hsw only */ #define PIPECONF_BPC_MASK(0x7 << 5) #define PIPECONF_8BPC(0 << 5) #define PIPECONF_10BPC (1 << 5) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 12/12] drm/i915: Add PIPECONF YCbCr 4:4:4 programming for ILK-IVB
From: Ville Syrjälä On ILK-IVB the pipe colorspace is configured via PIPECONF (as opposed to PIPEMISC in BDW+). Let's configure+readout that stuff correctly. Enablling YCbCr 4:4:4 output will now be a simple matter of setting crtc_state->output_format appropriately in the encoder .compute_config(). However, when we do that we must be aware of the fact that YCbCr DP output doesn't seem to work on ILK (resulting image is totally garbled), but on SNB+ it works fine. However HDMI YCbCr output does work correctly even on ILK. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 21 +++- drivers/gpu/drm/i915/i915_reg.h | 4 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index bd3ff96c1618..8e98715cd63b 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -9406,9 +9406,19 @@ static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state) else val |= PIPECONF_PROGRESSIVE; + /* +* This would end up with an odd purple hue over +* the entire display. Make sure we don't do it. +*/ + WARN_ON(crtc_state->limited_color_range && + crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB); + if (crtc_state->limited_color_range) val |= PIPECONF_COLOR_RANGE_SELECT; + if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) + val |= PIPECONF_OUTPUT_COLORSPACE_YUV709; + val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode); I915_WRITE(PIPECONF(pipe), val); @@ -9945,7 +9955,6 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc, if (!wakeref) return false; - pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB; pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe; pipe_config->shared_dpll = NULL; @@ -9974,6 +9983,16 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc, if (tmp & PIPECONF_COLOR_RANGE_SELECT) pipe_config->limited_color_range = true; + switch (tmp & PIPECONF_OUTPUT_COLORSPACE_MASK) { + case PIPECONF_OUTPUT_COLORSPACE_YUV601: + case PIPECONF_OUTPUT_COLORSPACE_YUV709: + pipe_config->output_format = INTEL_OUTPUT_FORMAT_YCBCR444; + break; + default: + pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB; + break; + } + pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_ILK) >> PIPECONF_GAMMA_MODE_SHIFT; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 33d535ae0944..3d33a1e03a45 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5712,6 +5712,10 @@ enum { #define PIPECONF_CXSR_DOWNCLOCK (1 << 16) #define PIPECONF_EDP_RR_MODE_SWITCH_VLV (1 << 14) #define PIPECONF_COLOR_RANGE_SELECT (1 << 13) +#define PIPECONF_OUTPUT_COLORSPACE_MASK (3 << 11) /* ilk-ivb */ +#define PIPECONF_OUTPUT_COLORSPACE_RGB (0 << 11) /* ilk-ivb */ +#define PIPECONF_OUTPUT_COLORSPACE_YUV601(1 << 11) /* ilk-ivb */ +#define PIPECONF_OUTPUT_COLORSPACE_YUV709(2 << 11) /* ilk-ivb */ #define PIPECONF_OUTPUT_COLORSPACE_YUV_HSW (1 << 11) /* hsw only */ #define PIPECONF_BPC_MASK(0x7 << 5) #define PIPECONF_8BPC(0 << 5) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: fix RC_CORE dependency
On 18.07.2019 16:21, Arnd Bergmann wrote: > On Thu, Jul 18, 2019 at 4:16 PM Andrzej Hajda wrote: >> Hi Arnd, >> >> On 18.07.2019 15:42, Arnd Bergmann wrote: >>> Using 'imply' causes a new problem, as it allows the case of >>> CONFIG_INPUT=m with RC_CORE=y, which fails to link: >>> >>> drivers/media/rc/rc-main.o: In function `ir_do_keyup': >>> rc-main.c:(.text+0x2b4): undefined reference to `input_event' >>> drivers/media/rc/rc-main.o: In function `rc_repeat': >>> rc-main.c:(.text+0x350): undefined reference to `input_event' >>> drivers/media/rc/rc-main.o: In function `rc_allocate_device': >>> rc-main.c:(.text+0x90c): undefined reference to `input_allocate_device' >>> >>> Add a 'depends on' that allows building both with and without >>> CONFIG_RC_CORE, but disallows combinations that don't link. >>> >>> Fixes: 5023cf32210d ("drm/bridge: make remote control optional") >>> Signed-off-by: Arnd Bergmann >> >> Proper solution has been already merged via input tree[1]. >> >> >> [1]: >> https://lore.kernel.org/lkml/cakdakrtgxnbusukasnglfwuwc7asod9k5baylpwpu7ex-42...@mail.gmail.com/ > At that link, I only see the patch that caused the regression, not > the solution. Are you sure it's fixed? Ups, you are right, I though you are fixing what this patch attempted to fix :) Anyway, we want to avoid dependency on RC_CORE - this driver does not require it, but with RC_CORE it has additional features. Maybe "imply INPUT" would help? Regards Andrzej > > Arnd >
Re: [PATCH] drm/bridge: fix RC_CORE dependency
On Thu, Jul 18, 2019 at 4:56 PM Andrzej Hajda wrote: > On 18.07.2019 16:21, Arnd Bergmann wrote: > > On Thu, Jul 18, 2019 at 4:16 PM Andrzej Hajda wrote: > >> Hi Arnd, > >> > >> On 18.07.2019 15:42, Arnd Bergmann wrote: > >>> Using 'imply' causes a new problem, as it allows the case of > >>> CONFIG_INPUT=m with RC_CORE=y, which fails to link: > >>> > >>> drivers/media/rc/rc-main.o: In function `ir_do_keyup': > >>> rc-main.c:(.text+0x2b4): undefined reference to `input_event' > >>> drivers/media/rc/rc-main.o: In function `rc_repeat': > >>> rc-main.c:(.text+0x350): undefined reference to `input_event' > >>> drivers/media/rc/rc-main.o: In function `rc_allocate_device': > >>> rc-main.c:(.text+0x90c): undefined reference to `input_allocate_device' > >>> > >>> Add a 'depends on' that allows building both with and without > >>> CONFIG_RC_CORE, but disallows combinations that don't link. > >>> > >>> Fixes: 5023cf32210d ("drm/bridge: make remote control optional") > >>> Signed-off-by: Arnd Bergmann > >> > >> Proper solution has been already merged via input tree[1]. > >> > >> > >> [1]: > >> https://lore.kernel.org/lkml/cakdakrtgxnbusukasnglfwuwc7asod9k5baylpwpu7ex-42...@mail.gmail.com/ > > At that link, I only see the patch that caused the regression, not > > the solution. Are you sure it's fixed? > > > Ups, you are right, I though you are fixing what this patch attempted to > fix :) > > Anyway, we want to avoid dependency on RC_CORE - this driver does not > require it, but with RC_CORE it has additional features. Right, that's what my patch does: if RC_CORE is disabled, you can still set DRM_SIL_SII8620=y, but if RC_CORE=m, DRM_SIL_SII8620 can only be =m or =n. > Maybe "imply INPUT" would help? No, that would make it worse. Device drivers really have no business turning on other subsystems. Arnd
[PULL] drm-misc-next-fixes
Hi team, I am Guest-Maarten this week and next! Not exactly a quiet last PR for the merge window, but I think this is right in line with how things have gone over the rest of 5.3. Although there's more volume than we'd like, I don't think there's anything here that is contraversial. So, welcome komeda to -misc, and happy pulling :-) drm-misc-next-fixes-2019-07-18: - Rip out komeda internal properties and move the driver to -misc (Daniel) - Handle a couple edge cases with incomplete/incorrect cmdline modes (Dmitry) - Fix some silly warnings (Arnd) - Add orientation quirk for newer GPD MicroPCs (Hans) Cc: Daniel Vetter Cc: Liviu Dudau Cc: Dmitry Osipenko Cc: Arnd Bergmann Cc: Hans de Goede Cheers, Sean The following changes since commit daed277e4d5ace0883d30b9be245d35c46289f49: Merge tag 'topic/remove-fbcon-notifiers-2019-06-26' into drm-misc-next-fixes (2019-06-26 12:26:34 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2019-07-18 for you to fetch changes up to 7aaddd96d5febcf5b24357a326b3038d49a20532: drm/modes: Don't apply cmdline's rotation if it wasn't specified (2019-07-16 10:34:38 +0200) - Rip out komeda internal properties and move the driver to -misc (Daniel) - Handle a couple edge cases with incomplete/incorrect cmdline modes (Dmitry) - Fix some silly warnings (Arnd) - Add orientation quirk for newer GPD MicroPCs (Hans) Cc: Daniel Vetter Cc: Liviu Dudau Cc: Dmitry Osipenko Cc: Arnd Bergmann Cc: Hans de Goede Arnd Bergmann (2): drm/selftests: reduce stack usage drm: connector: remove bogus NULL check Daniel Vetter (5): drm/komeda: Remove clock ratio property drm/komeda: remove slave_planes property drm/komeda: remove img_enhancement property drm/komeda: Remove layer_split property MAINTAINERS: maintain drm/arm drivers in drm-misc for now Dmitry Osipenko (2): drm/modes: Skip invalid cmdline mode drm/modes: Don't apply cmdline's rotation if it wasn't specified Gerd Hoffmann (1): drm/bochs: fix framebuffer setup. Hans de Goede (1): drm: panel-orientation-quirks: Add extra quirk table entry for GPD MicroPC james qian wang (Arm Technology China) (2): drm/komeda: Computing layer_split internally drm/komeda: Computing image enhancer internally MAINTAINERS| 4 +- drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 63 -- drivers/gpu/drm/arm/display/komeda/komeda_kms.h| 18 +-- .../gpu/drm/arm/display/komeda/komeda_pipeline.h | 3 +- .../drm/arm/display/komeda/komeda_pipeline_state.c | 15 ++- drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 84 + .../drm/arm/display/komeda/komeda_wb_connector.c | 10 +- drivers/gpu/drm/bochs/bochs.h | 2 +- drivers/gpu/drm/bochs/bochs_hw.c | 14 ++- drivers/gpu/drm/bochs/bochs_kms.c | 3 +- drivers/gpu/drm/drm_client_modeset.c | 5 +- drivers/gpu/drm/drm_connector.c| 2 +- drivers/gpu/drm/drm_modes.c| 14 ++- drivers/gpu/drm/drm_panel_orientation_quirks.c | 12 ++ .../gpu/drm/selftests/test-drm_cmdline_parser.c| 136 - include/drm/drm_modes.h| 2 +- 16 files changed, 110 insertions(+), 277 deletions(-) -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: fix RC_CORE dependency
On Thu, Jul 18, 2019 at 6:13 PM Arnd Bergmann wrote: > > On Thu, Jul 18, 2019 at 4:56 PM Andrzej Hajda wrote: > > On 18.07.2019 16:21, Arnd Bergmann wrote: > > > On Thu, Jul 18, 2019 at 4:16 PM Andrzej Hajda wrote: > > >> Hi Arnd, > > >> > > >> On 18.07.2019 15:42, Arnd Bergmann wrote: > > >>> Using 'imply' causes a new problem, as it allows the case of > > >>> CONFIG_INPUT=m with RC_CORE=y, which fails to link: > > >>> > > >>> drivers/media/rc/rc-main.o: In function `ir_do_keyup': > > >>> rc-main.c:(.text+0x2b4): undefined reference to `input_event' > > >>> drivers/media/rc/rc-main.o: In function `rc_repeat': > > >>> rc-main.c:(.text+0x350): undefined reference to `input_event' > > >>> drivers/media/rc/rc-main.o: In function `rc_allocate_device': > > >>> rc-main.c:(.text+0x90c): undefined reference to `input_allocate_device' > > >>> > > >>> Add a 'depends on' that allows building both with and without > > >>> CONFIG_RC_CORE, but disallows combinations that don't link. > > >>> > > >>> Fixes: 5023cf32210d ("drm/bridge: make remote control optional") > > >>> Signed-off-by: Arnd Bergmann > > >> > > >> Proper solution has been already merged via input tree[1]. > > >> > > >> > > >> [1]: > > >> https://lore.kernel.org/lkml/cakdakrtgxnbusukasnglfwuwc7asod9k5baylpwpu7ex-42...@mail.gmail.com/ > > > At that link, I only see the patch that caused the regression, not > > > the solution. Are you sure it's fixed? > > > > > > Ups, you are right, I though you are fixing what this patch attempted to > > fix :) > > > > Anyway, we want to avoid dependency on RC_CORE - this driver does not > > require it, but with RC_CORE it has additional features. > > Right, that's what my patch does: if RC_CORE is disabled, you can > still set DRM_SIL_SII8620=y, but if RC_CORE=m, DRM_SIL_SII8620 > can only be =m or =n. > > > Maybe "imply INPUT" would help? > > No, that would make it worse. Device drivers really have no business > turning on other subsystems. > OK, in the meantime I will redo the branch by dropping the sil-sii8620.c Kconfig changes and also drop all "imply" business from applespi driver as they give us more trouble than they are worth. We do not have "imply" for i801_smbus for Symaptics SMBUS mode and it works fine. It it distro's task to configure the kernel properly. Thanks. -- Dmitry
Re: [PATCH] drm/komeda: Adds error event print functionality
On Thu, Jul 18, 2019 at 02:17:37PM +0100, Liviu Dudau wrote: > On Thu, Jun 27, 2019 at 04:10:36AM +0100, Lowry Li (Arm Technology China) > wrote: /snip > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > > index 647bce5..1462bac 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > > @@ -47,6 +47,8 @@ static irqreturn_t komeda_kms_irq_handler(int irq, void > > *data) > > memset(&evts, 0, sizeof(evts)); > > status = mdev->funcs->irq_handler(mdev, &evts); > > > > + komeda_print_events(&evts); > > Calling this function from the IRQ handler is a bad idea. We should use > debugfs > if you really want to have a trace of the events, but I personally don't see > value in having this functionality in the kernel at all. You can expose the > value of the evts->global and evts->pipes[] as integers and decode that in > userspace or as a debugfs entry. Alternatively, consider using kernel trace events. They allow you to selectively turn on/off certain events and also allow you to customize which data is recorded and how it's formatted. Seems like a good fit from the quick scan I've done. Sean > > Best regards, > Liviu > > > + > > /* Notify the crtc to handle the events */ > > for (i = 0; i < kms->n_crtcs; i++) > > komeda_crtc_handle_event(&kms->crtcs[i], &evts); > > -- > > 1.9.1 > > > > -- > > | I would like to | > | fix the world, | > | but they're not | > | giving me the | > \ source code! / > --- > ¯\_(ツ)_/¯ -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: fix RC_CORE dependency
On Thu, Jul 18, 2019 at 5:17 PM Dmitry Torokhov wrote: > On Thu, Jul 18, 2019 at 6:13 PM Arnd Bergmann wrote: > > On Thu, Jul 18, 2019 at 4:56 PM Andrzej Hajda wrote: > > > On 18.07.2019 16:21, Arnd Bergmann wrote: > > > > On Thu, Jul 18, 2019 at 4:16 PM Andrzej Hajda > > > > wrote: > > > >> Proper solution has been already merged via input tree[1]. > > > >> > > > >> > > > >> [1]: > > > >> https://lore.kernel.org/lkml/cakdakrtgxnbusukasnglfwuwc7asod9k5baylpwpu7ex-42...@mail.gmail.com/ > > > > At that link, I only see the patch that caused the regression, not > > > > the solution. Are you sure it's fixed? > > > > > > > > > Ups, you are right, I though you are fixing what this patch attempted to > > > fix :) > > > > > > Anyway, we want to avoid dependency on RC_CORE - this driver does not > > > require it, but with RC_CORE it has additional features. > > > > Right, that's what my patch does: if RC_CORE is disabled, you can > > still set DRM_SIL_SII8620=y, but if RC_CORE=m, DRM_SIL_SII8620 > > can only be =m or =n. > > > > > Maybe "imply INPUT" would help? > > > > No, that would make it worse. Device drivers really have no business > > turning on other subsystems. > > > > OK, in the meantime I will redo the branch by dropping the > sil-sii8620.c Kconfig changes and also drop all "imply" business from > applespi driver as they give us more trouble than they are worth. We > do not have "imply" for i801_smbus for Symaptics SMBUS mode and it > works fine. It it distro's task to configure the kernel properly. Thanks! I think the "drm/bridge: make remote control optional" patch is fine with my fixup, the IS_ENABLED() checks take care of the case where RC_CORE is unavailable, and the 'depends on RC_CORE || !RC_CORE' line takes care of the RC_CORE=m case. I suppose Ronald could send a replacement patch with my fixup after the merge window. Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 6/6] drm/via: drop DRM_WAIT_ON() in via_video
Replace DRM_WAIT_ON() with wait_event_interruptible(). Be careful to keep same return value semantics Signed-off-by: Sam Ravnborg Cc: Kevin Brace Cc: Thomas Hellstrom Cc: "Gustavo A. R. Silva" Cc: Mike Marshall Cc: Ira Weiny Cc: Daniel Vetter Cc: Emil Velikov --- drivers/gpu/drm/via/via_video.c | 21 - 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/via/via_video.c b/drivers/gpu/drm/via/via_video.c index 4e165b1b0b18..1f88180affef 100644 --- a/drivers/gpu/drm/via/via_video.c +++ b/drivers/gpu/drm/via/via_video.c @@ -26,7 +26,6 @@ */ #include -#include #include #include "via_drv.h" @@ -73,7 +72,7 @@ int via_decoder_futex(struct drm_device *dev, void *data, struct drm_file *file_ volatile int *lock; drm_via_private_t *dev_priv = (drm_via_private_t *) dev->dev_private; drm_via_sarea_t *sAPriv = dev_priv->sarea_priv; - int ret = 0; + int ret; DRM_DEBUG("\n"); @@ -84,9 +83,21 @@ int via_decoder_futex(struct drm_device *dev, void *data, struct drm_file *file_ switch (fx->func) { case VIA_FUTEX_WAIT: - DRM_WAIT_ON(ret, dev_priv->decoder_queue[fx->lock], - (fx->ms / 10) * (HZ / 100), *lock != fx->val); - return ret; + ret = wait_event_interruptible_timeout( + dev_priv->decoder_queue[fx->lock], + *lock != fx->val, + msecs_to_jiffies(fx->ms)); + switch (ret) { + case 0: + /* timeout */ + return -EBUSY; + case -ERESTARTSYS: + /* interrupted by signal */ + return -EINTR; + default: + return 0; + } + case VIA_FUTEX_WAKE: wake_up(&(dev_priv->decoder_queue[fx->lock])); return 0; -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH/RFT v1 0/6] drm/via: drop use of deprecated headers drmP.h and drm_os_linux.h
This is some janitorial updates to the via driver that is required to get rid of deprecated headers in the drm subsystem. The first three patches are trivial, where the dependencies on drmP.h and drm_os_linux are dropped. The remaining three patches drop use of DRM_WAIT_ON(). They are replaced by wait_event_interruptible_timeout(). These patches could use a more critical review. When replacing DRM_WAIT_ON() I took care to not change the uapi, by continue to use the original return codes. The following table was used for the transition: DRM_WAIT_ON wait_event_interruptible_timeout --- - condition OK:ret >= 1 timeout:-EBUSY 0 interrupted:-EINTR -ERESTARTSYS The changes has been build tested only. Testing on a real device would be highly appreciated. I had preferred that the via driver was replaced by the openchrome driver, but until we have it then we need to deal with the legacy via driver to remove old cruft in the drm subsystem. Note: A simpler approach had been to copy DRM_WAIT_ON to via_drv.h, but then the actual solution is presumeably better. Sam Sam Ravnborg (6): drm/via: drop use of DRM(READ|WRITE) macros drm/via: make via_drv.h self-contained drm/via: drop use of drmP.h drm/via: drop DRM_WAIT_ON() in via_dmablit.c drm/via: drop DRM_WAIT_ON() in via_irq drm/via: drop DRM_WAIT_ON() in via_video drivers/gpu/drm/via/via_dma.c | 9 +- drivers/gpu/drm/via/via_dmablit.c | 66 +++--- drivers/gpu/drm/via/via_drv.c | 7 ++-- drivers/gpu/drm/via/via_drv.h | 21 +--- drivers/gpu/drm/via/via_irq.c | 37 +++-- drivers/gpu/drm/via/via_map.c | 6 +++- drivers/gpu/drm/via/via_mm.c | 7 +++- drivers/gpu/drm/via/via_verifier.c | 10 +++--- drivers/gpu/drm/via/via_video.c| 23 ++--- 9 files changed, 137 insertions(+), 49 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 2/6] drm/via: make via_drv.h self-contained
Added the necessary header files to make this header file self-contained. Signed-off-by: Sam Ravnborg Cc: Kevin Brace Cc: Thomas Hellstrom Cc: "Gustavo A. R. Silva" Cc: Mike Marshall Cc: Ira Weiny Cc: Daniel Vetter Cc: Emil Velikov --- drivers/gpu/drm/via/via_drv.h | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/via/via_drv.h b/drivers/gpu/drm/via/via_drv.h index d5a2b1ffd8c1..f36ac1134424 100644 --- a/drivers/gpu/drm/via/via_drv.h +++ b/drivers/gpu/drm/via/via_drv.h @@ -24,8 +24,12 @@ #ifndef _VIA_DRV_H_ #define _VIA_DRV_H_ -#include +#include + +#include #include +#include +#include #define DRIVER_AUTHOR "Various" -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 3/6] drm/via: drop use of drmP.h
Drop use of the deprecated drmP.h header. While touching the files divide include files in blocks and sort the files alphabetically. Signed-off-by: Sam Ravnborg Cc: Kevin Brace Cc: Thomas Hellstrom Cc: "Gustavo A. R. Silva" Cc: Mike Marshall Cc: Ira Weiny Cc: Daniel Vetter Cc: Emil Velikov --- drivers/gpu/drm/via/via_dma.c | 9 - drivers/gpu/drm/via/via_dmablit.c | 14 +- drivers/gpu/drm/via/via_drv.c | 7 +-- drivers/gpu/drm/via/via_irq.c | 5 - drivers/gpu/drm/via/via_map.c | 6 +- drivers/gpu/drm/via/via_mm.c | 7 ++- drivers/gpu/drm/via/via_verifier.c | 10 +- drivers/gpu/drm/via/via_video.c| 4 +++- 8 files changed, 45 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/via/via_dma.c b/drivers/gpu/drm/via/via_dma.c index d17d8f245c1a..4e50834dd222 100644 --- a/drivers/gpu/drm/via/via_dma.c +++ b/drivers/gpu/drm/via/via_dma.c @@ -34,8 +34,15 @@ *Thomas Hellstrom. */ -#include +#include +#include + +#include +#include +#include +#include #include + #include "via_drv.h" #include "via_3d_reg.h" diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c index 062067438f1d..87d88bdd20c6 100644 --- a/drivers/gpu/drm/via/via_dmablit.c +++ b/drivers/gpu/drm/via/via_dmablit.c @@ -34,13 +34,17 @@ * the same DMA mappings? */ -#include -#include -#include "via_drv.h" -#include "via_dmablit.h" - #include #include +#include + +#include +#include +#include +#include + +#include "via_dmablit.h" +#include "via_drv.h" #define VIA_PGDN(x) (((unsigned long)(x)) & PAGE_MASK) #define VIA_PGOFF(x) (((unsigned long)(x)) & ~PAGE_MASK) diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c index af6a12d3c058..666a16de84f9 100644 --- a/drivers/gpu/drm/via/via_drv.c +++ b/drivers/gpu/drm/via/via_drv.c @@ -24,11 +24,14 @@ #include -#include +#include +#include +#include +#include #include + #include "via_drv.h" -#include static int via_driver_open(struct drm_device *dev, struct drm_file *file) { diff --git a/drivers/gpu/drm/via/via_irq.c b/drivers/gpu/drm/via/via_irq.c index c96830ccc0ec..9d47feb367d8 100644 --- a/drivers/gpu/drm/via/via_irq.c +++ b/drivers/gpu/drm/via/via_irq.c @@ -35,8 +35,11 @@ * The refresh rate is also calculated for video playback sync purposes. */ -#include +#include +#include +#include #include + #include "via_drv.h" #define VIA_REG_INTERRUPT 0x200 diff --git a/drivers/gpu/drm/via/via_map.c b/drivers/gpu/drm/via/via_map.c index 2ad865870372..431c150df014 100644 --- a/drivers/gpu/drm/via/via_map.c +++ b/drivers/gpu/drm/via/via_map.c @@ -21,8 +21,12 @@ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER * DEALINGS IN THE SOFTWARE. */ -#include + +#include +#include +#include #include + #include "via_drv.h" static int via_do_init_map(struct drm_device *dev, drm_via_init_t *init) diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c index 4217d66a5cc6..45cc9e900260 100644 --- a/drivers/gpu/drm/via/via_mm.c +++ b/drivers/gpu/drm/via/via_mm.c @@ -25,8 +25,13 @@ * Authors: Thomas Hellström */ -#include +#include + +#include +#include +#include #include + #include "via_drv.h" #define VIA_MM_ALIGN_SHIFT 4 diff --git a/drivers/gpu/drm/via/via_verifier.c b/drivers/gpu/drm/via/via_verifier.c index fb2609434df7..361a450058f2 100644 --- a/drivers/gpu/drm/via/via_verifier.c +++ b/drivers/gpu/drm/via/via_verifier.c @@ -28,13 +28,13 @@ * be very slow. */ -#include "via_3d_reg.h" -#include -#include +#include #include -#include "via_verifier.h" +#include + +#include "via_3d_reg.h" #include "via_drv.h" -#include +#include "via_verifier.h" typedef enum { state_command, diff --git a/drivers/gpu/drm/via/via_video.c b/drivers/gpu/drm/via/via_video.c index a9ffbad1cfdd..4e165b1b0b18 100644 --- a/drivers/gpu/drm/via/via_video.c +++ b/drivers/gpu/drm/via/via_video.c @@ -25,8 +25,10 @@ * Video and XvMC related functions. */ -#include +#include +#include #include + #include "via_drv.h" void via_init_futex(drm_via_private_t *dev_priv) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 1/6] drm/via: drop use of DRM(READ|WRITE) macros
The DRM_READ, DRM_WRITE macros comes from the deprecated drm_os_linux.h header file. Remove their use to remove this dependency. Replace the use of the macros with open coded variants. Signed-off-by: Sam Ravnborg Cc: Kevin Brace Cc: Thomas Hellstrom Cc: "Gustavo A. R. Silva" Cc: Mike Marshall Cc: Ira Weiny Cc: Daniel Vetter Cc: Emil Velikov --- drivers/gpu/drm/via/via_drv.h | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/via/via_drv.h b/drivers/gpu/drm/via/via_drv.h index 6d1ae834484c..d5a2b1ffd8c1 100644 --- a/drivers/gpu/drm/via/via_drv.h +++ b/drivers/gpu/drm/via/via_drv.h @@ -115,10 +115,17 @@ enum via_family { /* VIA MMIO register access */ #define VIA_BASE ((dev_priv->mmio)) -#define VIA_READ(reg) DRM_READ32(VIA_BASE, reg) -#define VIA_WRITE(reg, val)DRM_WRITE32(VIA_BASE, reg, val) -#define VIA_READ8(reg) DRM_READ8(VIA_BASE, reg) -#define VIA_WRITE8(reg, val) DRM_WRITE8(VIA_BASE, reg, val) +#define VIA_READ(reg) \ + readl(((void __iomem *)VIA_BASE->handle) + (reg)) + +#define VIA_WRITE(reg, val) \ + writel(val, ((void __iomem *)VIA_BASE->handle) + (reg)) + +#define VIA_READ8(reg) \ + readb(((void __iomem *)VIA_BASE->handle) + (reg)) + +#define VIA_WRITE8(reg, val) \ + writeb(val, ((void __iomem *)VIA_BASE->handle) + (reg)) extern const struct drm_ioctl_desc via_ioctls[]; extern int via_max_ioctl; -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 4/6] drm/via: drop DRM_WAIT_ON() in via_dmablit.c
DRM_WAIT_ON() is a reliec from the past and is discouraged. Use the standard wait_event_*() as replacement. Be carefull to keep the same return values. via_dma_blit_sync() changed -EINTR to -EAGAIN. Moved this to via_dmablit_sync() so return value is adjusted only once. Signed-off-by: Sam Ravnborg Cc: Kevin Brace Cc: Thomas Hellstrom Cc: "Gustavo A. R. Silva" Cc: Mike Marshall Cc: Ira Weiny Cc: Daniel Vetter Cc: Emil Velikov --- drivers/gpu/drm/via/via_dmablit.c | 54 ++- 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c index 87d88bdd20c6..27e2a6411502 100644 --- a/drivers/gpu/drm/via/via_dmablit.c +++ b/drivers/gpu/drm/via/via_dmablit.c @@ -39,7 +39,6 @@ #include #include -#include #include #include @@ -428,8 +427,12 @@ via_dmablit_active(drm_via_blitq_t *blitq, int engine, uint32_t handle, wait_que /* * Sync. Wait for at least three seconds for the blit to be performed. + * + * Returns: + * 0 if blit was performed within the three seconds period + * -EBUSY if timeout occured + * -EAGAIN if a signal interrupted the wait */ - static int via_dmablit_sync(struct drm_device *dev, uint32_t handle, int engine) { @@ -437,16 +440,26 @@ via_dmablit_sync(struct drm_device *dev, uint32_t handle, int engine) drm_via_private_t *dev_priv = (drm_via_private_t *)dev->dev_private; drm_via_blitq_t *blitq = dev_priv->blit_queues + engine; wait_queue_head_t *queue; - int ret = 0; + int ret = 1; if (via_dmablit_active(blitq, engine, handle, &queue)) { - DRM_WAIT_ON(ret, *queue, 3 * HZ, - !via_dmablit_active(blitq, engine, handle, NULL)); + ret = wait_event_interruptible_timeout(*queue, + !via_dmablit_active(blitq, engine, handle, NULL), + msecs_to_jiffies(3 * 1000)); } DRM_DEBUG("DMA blit sync handle 0x%x engine %d returned %d\n", handle, engine, ret); - return ret; + switch (ret) { + case 0: + /* timeout */ + return -EBUSY; + case -ERESTARTSYS: + /* interrupted by signal */ + return -EAGAIN; + default: + return 0; + } } @@ -677,13 +690,17 @@ via_build_sg_info(struct drm_device *dev, drm_via_sg_info_t *vsg, drm_via_dmabli /* * Reserve one free slot in the blit queue. Will wait for one second for one - * to become available. Otherwise -EBUSY is returned. + * to become available. + * + * Returns: + * 0 if slot was reserved + * -EBUSY if timeout while waiting for free slot + * -EAGAIN if interrupted by a signal */ - static int via_dmablit_grab_slot(drm_via_blitq_t *blitq, int engine) { - int ret = 0; + int ret; unsigned long irqsave; DRM_DEBUG("Num free is %d\n", blitq->num_free); @@ -691,9 +708,19 @@ via_dmablit_grab_slot(drm_via_blitq_t *blitq, int engine) while (blitq->num_free == 0) { spin_unlock_irqrestore(&blitq->blit_lock, irqsave); - DRM_WAIT_ON(ret, blitq->busy_queue, HZ, blitq->num_free > 0); - if (ret) - return (-EINTR == ret) ? -EAGAIN : ret; + ret = wait_event_interruptible_timeout(blitq->busy_queue, + blitq->num_free > 0, + msecs_to_jiffies(1000)); + switch (ret) { + case 0: + /* timeout */ + return -EBUSY; + case -ERESTARTSYS: + /* interrupted by signal */ + return -EAGAIN; + default: + return 0; + } spin_lock_irqsave(&blitq->blit_lock, irqsave); } @@ -786,9 +813,6 @@ via_dma_blit_sync(struct drm_device *dev, void *data, struct drm_file *file_priv err = via_dmablit_sync(dev, sync->sync_handle, sync->engine); - if (-EINTR == err) - err = -EAGAIN; - return err; } -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 5/6] drm/via: drop DRM_WAIT_ON() in via_irq
Replace DRM_WAIT_ON() with wait_event_interruptible(). While replacing be careful to keep same return value semantics. Signed-off-by: Sam Ravnborg Cc: Kevin Brace Cc: Thomas Hellstrom Cc: "Gustavo A. R. Silva" Cc: Mike Marshall Cc: Ira Weiny Cc: Daniel Vetter Cc: Emil Velikov --- drivers/gpu/drm/via/via_irq.c | 34 -- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/via/via_irq.c b/drivers/gpu/drm/via/via_irq.c index 9d47feb367d8..6de15065a3c0 100644 --- a/drivers/gpu/drm/via/via_irq.c +++ b/drivers/gpu/drm/via/via_irq.c @@ -36,7 +36,6 @@ */ #include -#include #include #include @@ -201,6 +200,12 @@ void via_disable_vblank(struct drm_device *dev, unsigned int pipe) DRM_ERROR("%s: bad crtc %u\n", __func__, pipe); } +/* + * Returns: + * 0 if interrupt occured within 3 secs + * -EBUSY if timeout happended + * -EINTR if interrupted by a signal + */ static int via_driver_irq_wait(struct drm_device *dev, unsigned int irq, int force_sequence, unsigned int *sequence) @@ -208,7 +213,7 @@ via_driver_irq_wait(struct drm_device *dev, unsigned int irq, int force_sequence drm_via_private_t *dev_priv = (drm_via_private_t *) dev->dev_private; unsigned int cur_irq_sequence; drm_via_irq_t *cur_irq; - int ret = 0; + int ret; maskarray_t *masks; int real_irq; @@ -236,18 +241,27 @@ via_driver_irq_wait(struct drm_device *dev, unsigned int irq, int force_sequence cur_irq = dev_priv->via_irqs + real_irq; if (masks[real_irq][2] && !force_sequence) { - DRM_WAIT_ON(ret, cur_irq->irq_queue, 3 * HZ, - ((VIA_READ(masks[irq][2]) & masks[irq][3]) == -masks[irq][4])); + ret = wait_event_interruptible_timeout(cur_irq->irq_queue, + ((VIA_READ(masks[irq][2]) & masks[irq][3]) == masks[irq][4]), + msecs_to_jiffies(3000)); cur_irq_sequence = atomic_read(&cur_irq->irq_received); } else { - DRM_WAIT_ON(ret, cur_irq->irq_queue, 3 * HZ, - (((cur_irq_sequence = - atomic_read(&cur_irq->irq_received)) - - *sequence) <= (1 << 23))); + ret = wait_event_interruptible_timeout(cur_irq->irq_queue, + (((cur_irq_sequence = atomic_read(&cur_irq->irq_received)) - *sequence) <= (1 << 23)), + msecs_to_jiffies(3000)); } *sequence = cur_irq_sequence; - return ret; + + switch (ret) { + case 0: + /* timeout */ + return -EBUSY; + case -ERESTARTSYS: + /* interrupted by signal */ + return -EINTR; + default: + return 0; + } } -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH] drm/msm: stop abusing dma_map/unmap for cache
On Sun, Jun 30, 2019 at 05:47:22AM -0700, Rob Clark wrote: > From: Rob Clark > > Recently splats like this started showing up: > >WARNING: CPU: 4 PID: 251 at drivers/iommu/dma-iommu.c:451 > __iommu_dma_unmap+0xb8/0xc0 >Modules linked in: ath10k_snoc ath10k_core fuse msm ath mac80211 uvcvideo > cfg80211 videobuf2_vmalloc videobuf2_memops vide >CPU: 4 PID: 251 Comm: kworker/u16:4 Tainted: GW > 5.2.0-rc5-next-20190619+ #2317 >Hardware name: LENOVO 81JL/LNVNB161216, BIOS 9UCN23WW(V1.06) 10/25/2018 >Workqueue: msm msm_gem_free_work [msm] >pstate: 80c5 (Nzcv daif +PAN +UAO) >pc : __iommu_dma_unmap+0xb8/0xc0 >lr : __iommu_dma_unmap+0x54/0xc0 >sp : 119abce0 >x29: 119abce0 x28: >x27: 8001f9946648 x26: 8001ec271068 >x25: x24: 8001ea3580a8 >x23: 8001f95ba010 x22: 80018e83ba88 >x21: 8001e548f000 x20: f000 >x19: 1000 x18: c1fe >x17: x16: >x15: 15b70068 x14: 0005 >x13: 0003142cc1be1768 x12: 0001 >x11: 8001f6de9100 x10: 0009 >x9 : 15b78000 x8 : >x7 : 0001 x6 : f000 >x5 : 0fff x4 : 1065dbc8 >x3 : 000d x2 : 1000 >x1 : f000 x0 : >Call trace: > __iommu_dma_unmap+0xb8/0xc0 > iommu_dma_unmap_sg+0x98/0xb8 > put_pages+0x5c/0xf0 [msm] > msm_gem_free_work+0x10c/0x150 [msm] > process_one_work+0x1e0/0x330 > worker_thread+0x40/0x438 > kthread+0x12c/0x130 > ret_from_fork+0x10/0x18 >---[ end trace afc0dc5ab81a06bf ]--- > > Not quite sure what triggered that, but we really shouldn't be abusing > dma_{map,unmap}_sg() for cache maint. I'm sure we'll see this rear its head again someday. My kingdom for leaf driver cache control that makes sense. Reviewed-by: Jordan Crouse > Signed-off-by: Rob Clark > Cc: Stephen Boyd > --- > drivers/gpu/drm/msm/msm_gem.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c > index d31d9f927887..3b84cbdcafa3 100644 > --- a/drivers/gpu/drm/msm/msm_gem.c > +++ b/drivers/gpu/drm/msm/msm_gem.c > @@ -108,7 +108,7 @@ static struct page **get_pages(struct drm_gem_object *obj) >* because display controller, GPU, etc. are not coherent: >*/ > if (msm_obj->flags & (MSM_BO_WC|MSM_BO_UNCACHED)) > - dma_map_sg(dev->dev, msm_obj->sgt->sgl, > + dma_sync_sg_for_device(dev->dev, msm_obj->sgt->sgl, > msm_obj->sgt->nents, DMA_BIDIRECTIONAL); > } > > @@ -138,7 +138,7 @@ static void put_pages(struct drm_gem_object *obj) >* GPU, etc. are not coherent: >*/ > if (msm_obj->flags & (MSM_BO_WC|MSM_BO_UNCACHED)) > - dma_unmap_sg(obj->dev->dev, msm_obj->sgt->sgl, > + dma_sync_sg_for_cpu(obj->dev->dev, > msm_obj->sgt->sgl, >msm_obj->sgt->nents, >DMA_BIDIRECTIONAL); > > -- > 2.20.1 > > ___ > Freedreno mailing list > freedr...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 0/11] drm: header maintenance
First patch from Jani fixes so drm_print.h is self-contained. Next two patches are trivial removal of uapi dependencies. ati_pcigart is fixed to drop use of drm_os_linux.h drm_vblank is likewise fixed to drop use of drm_os_linux.h This was a non-trivial conversion, *review requested!* The remaining patches are preparation for and removal of uapi/drm/drmh from drm_file.h. There were a few files where we had to push include of drm/drm.h out to to have a clean build. CK Hu - please let me apply the mediatek patch to drm-misc-next, as it is required for the final patch. Or push it to drm-misc-next yourself. Sam Jani Nikula (1): drm/panel: make drm_panel.h self-contained Sam Ravnborg (10): drm: drop uapi dependency from drm_print.h drm: drop uapi dependency from drm_vblank.h drm/ati_pcigart: drop dependency on drm_os_linux.h drm/vblank: drop use of DRM_WAIT_ON() drm: direct include of drm.h in drm_gem.c drm: direct include of drm.h in drm_gem_shmem_helper.c drm: direct include of drm.h in drm_prime.c drm: direct include of drm.h in drm_syncobj.c drm/mediatek: direct include of drm.h in mtk_drm_gem.c drm: drop uapi dependency from drm_file.h drivers/gpu/drm/ati_pcigart.c | 10 ++ drivers/gpu/drm/drm_gem.c | 1 + drivers/gpu/drm/drm_gem_shmem_helper.c | 1 + drivers/gpu/drm/drm_prime.c| 1 + drivers/gpu/drm/drm_syncobj.c | 1 + drivers/gpu/drm/drm_vblank.c | 29 - drivers/gpu/drm/mediatek/mtk_drm_gem.c | 1 + include/drm/drm_file.h | 4 +--- include/drm/drm_panel.h| 1 + include/drm/drm_print.h| 4 +--- include/drm/drm_vblank.h | 1 - 11 files changed, 34 insertions(+), 20 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 03/11] drm: drop uapi dependency from drm_vblank.h
drm_vblank.h included uapi/drm/drm.h. It turns out this include was not required - delete it. Note: uapi/drm/drm.h is included indirect via drm_file.h, but there are no dependencies in drm_vblank.h so the removal is legit. Signed-off-by: Sam Ravnborg Reviewed-by: Daniel Vetter Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Cc: Stefan Agner Cc: Thierry Reding --- include/drm/drm_vblank.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h index e528bb2f659d..9fe4ba8bc622 100644 --- a/include/drm/drm_vblank.h +++ b/include/drm/drm_vblank.h @@ -30,7 +30,6 @@ #include #include -#include struct drm_device; struct drm_crtc; -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 01/11] drm/panel: make drm_panel.h self-contained
From: Jani Nikula Fix build warning if drm_panel.h is built with CONFIG_OF=n or CONFIG_DRM_PANEL=n and included without the prerequisite err.h: ./include/drm/drm_panel.h: In function ‘of_drm_find_panel’: ./include/drm/drm_panel.h:203:9: error: implicit declaration of function ‘ERR_PTR’ [-Werror=implicit-function-declaration] return ERR_PTR(-ENODEV); ^~~ ./include/drm/drm_panel.h:203:9: error: returning ‘int’ from a function with return type ‘struct drm_panel *’ makes pointer from integer without a cast [-Werror=int-conversion] return ERR_PTR(-ENODEV); ^~~~ Fixes: 5fa8e4a22182 ("drm/panel: Make of_drm_find_panel() return an ERR_PTR() instead of NULL") Cc: Boris Brezillon Signed-off-by: Jani Nikula Acked-by: Thierry Reding Reviewed-by: Sam Ravnborg --- include/drm/drm_panel.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 8c738c0e6e9f..26377836141c 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -24,6 +24,7 @@ #ifndef __DRM_PANEL_H__ #define __DRM_PANEL_H__ +#include #include #include -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 02/11] drm: drop uapi dependency from drm_print.h
drm_print.h used DRM_NAME - thus adding a dependency from include/drm/drm_print.h => uapi/drm/drm.h Hardcode the name "drm" to break this dependency. The idea is that there shall be a minimal dependency between include/drm/* and uapi/* Signed-off-by: Sam Ravnborg Suggested-by: Daniel Vetter Reviewed-by: Daniel Vetter Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Rob Clark Cc: Sean Paul Cc: Chris Wilson Cc: Daniel Vetter --- include/drm/drm_print.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index a5d6f2f3e430..760d1bd0eaf1 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -32,8 +32,6 @@ #include #include -#include - /** * DOC: print * @@ -287,7 +285,7 @@ void drm_err(const char *format, ...); /* Macros to make printk easier */ #define _DRM_PRINTK(once, level, fmt, ...) \ - printk##once(KERN_##level "[" DRM_NAME "] " fmt, ##__VA_ARGS__) + printk##once(KERN_##level "[drm] " fmt, ##__VA_ARGS__) #define DRM_INFO(fmt, ...) \ _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 06/11] drm: direct include of drm.h in drm_gem.c
Do not rely on including drm.h from drm_file.h, as the include in drm_file.h will be dropped. Signed-off-by: Sam Ravnborg Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Cc: Eric Anholt Cc: Thomas Zimmermann Cc: Rob Herring --- drivers/gpu/drm/drm_gem.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index e6c12c6ec728..243f43d70f42 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -39,6 +39,7 @@ #include #include +#include #include #include #include -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 05/11] drm/vblank: drop use of DRM_WAIT_ON()
DRM_WAIT_ON() is from the deprecated drm_os_linux header and the modern replacement is the wait_event_*. The return values differ, so a conversion is needed to keep the original interface towards userspace. Introduced a switch/case to make code obvious and to allow different debug prints depending on the result. Signed-off-by: Sam Ravnborg Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter --- drivers/gpu/drm/drm_vblank.c | 29 - 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 603ab105125d..8e9ac187500e 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -31,7 +31,6 @@ #include #include #include -#include #include #include "drm_internal.h" @@ -1672,19 +1671,31 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data, if (req_seq != seq) { DRM_DEBUG("waiting on vblank count %llu, crtc %u\n", req_seq, pipe); - DRM_WAIT_ON(ret, vblank->queue, 3 * HZ, - vblank_passed(drm_vblank_count(dev, pipe), - req_seq) || - !READ_ONCE(vblank->enabled)); + ret = wait_event_interruptible_timeout(vblank->queue, + vblank_passed(drm_vblank_count(dev, pipe), req_seq) || + !READ_ONCE(vblank->enabled), + msecs_to_jiffies(3000)); } - if (ret != -EINTR) { + switch (ret) { + case 0: + /* timeout */ + ret = -EBUSY; drm_wait_vblank_reply(dev, pipe, &vblwait->reply); - - DRM_DEBUG("crtc %d returning %u to client\n", + DRM_DEBUG("timeout waiting for vblank. crtc %d returning %u to client\n", pipe, vblwait->reply.sequence); - } else { + break; + case -ERESTARTSYS: + /* interrupted by signal */ + ret = -EINTR; DRM_DEBUG("crtc %d vblank wait interrupted by signal\n", pipe); + break; + default: + ret = 0; + drm_wait_vblank_reply(dev, pipe, &vblwait->reply); + DRM_DEBUG("crtc %d returning %u to client\n", + pipe, vblwait->reply.sequence); + break; } done: -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 04/11] drm/ati_pcigart: drop dependency on drm_os_linux.h
The drm_os_linux.h header is deprecated. Just opencode the sole DRM_WRITE32(). Signed-off-by: Sam Ravnborg Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter --- drivers/gpu/drm/ati_pcigart.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ati_pcigart.c b/drivers/gpu/drm/ati_pcigart.c index 2a413e291a60..580aa2676358 100644 --- a/drivers/gpu/drm/ati_pcigart.c +++ b/drivers/gpu/drm/ati_pcigart.c @@ -35,7 +35,6 @@ #include #include -#include #include #include @@ -169,6 +168,7 @@ int drm_ati_pcigart_init(struct drm_device *dev, struct drm_ati_pcigart_info *ga page_base = (u32) entry->busaddr[i]; for (j = 0; j < (PAGE_SIZE / ATI_PCIGART_PAGE_SIZE); j++) { + u32 offset; u32 val; switch(gart_info->gart_reg_if) { @@ -184,10 +184,12 @@ int drm_ati_pcigart_init(struct drm_device *dev, struct drm_ati_pcigart_info *ga break; } if (gart_info->gart_table_location == - DRM_ATI_GART_MAIN) + DRM_ATI_GART_MAIN) { pci_gart[gart_idx] = cpu_to_le32(val); - else - DRM_WRITE32(map, gart_idx * sizeof(u32), val); + } else { + offset = gart_idx * sizeof(u32); + writel(val, (void __iomem *)map->handle + offset); + } gart_idx++; page_base += ATI_PCIGART_PAGE_SIZE; } -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 07/11] drm: direct include of drm.h in drm_gem_shmem_helper.c
Do not rely on including drm.h from drm_file.h, as the include in drm_file.h will be dropped. Signed-off-by: Sam Ravnborg Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Cc: Eric Anholt Cc: Thomas Zimmermann Cc: Rob Herring --- drivers/gpu/drm/drm_gem_shmem_helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 472ea5d81f82..2f64667ac805 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -10,6 +10,7 @@ #include #include +#include #include #include #include -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 08/11] drm: direct include of drm.h in drm_prime.c
Do not rely on including drm.h from drm_file.h, as the include in drm_file.h will be dropped. Signed-off-by: Sam Ravnborg Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Cc: Christian König Cc: Noralf Trønnes Cc: Chris Wilson Cc: Eric Anholt --- drivers/gpu/drm/drm_prime.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 189d980402ad..eca484106cc2 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -30,6 +30,7 @@ #include #include +#include #include #include #include -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 10/11] drm/mediatek: direct include of drm.h in mtk_drm_gem.c
Do not rely on including drm.h from drm_file.h, as the include in drm_file.h will be dropped. Signed-off-by: Sam Ravnborg Cc: CK Hu Cc: Philipp Zabel Cc: Matthias Brugger Cc: linux-arm-ker...@lists.infradead.org Cc: linux-media...@lists.infradead.org --- drivers/gpu/drm/mediatek/mtk_drm_gem.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c b/drivers/gpu/drm/mediatek/mtk_drm_gem.c index 9434f88c6341..ca672f1d140d 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c @@ -5,6 +5,7 @@ #include +#include #include #include #include -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 11/11] drm: drop uapi dependency from drm_file.h
drm_file used drm_magic_t from uapi/drm/drm.h. This is a simple unsigned int. Just opencode it as such to break the dependency from this header file to uapi. Signed-off-by: Sam Ravnborg Suggested-by: Daniel Vetter Cc: Sean Paul Cc: Liviu Dudau Cc: Chris Wilson Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: David Airlie Cc: Daniel Vetter Cc: Jani Nikula Cc: Eric Anholt --- include/drm/drm_file.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 67af60bb527a..046cd1bf91eb 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -34,8 +34,6 @@ #include #include -#include - #include struct dma_fence; @@ -227,7 +225,7 @@ struct drm_file { struct pid *pid; /** @magic: Authentication magic, see @authenticated. */ - drm_magic_t magic; + unsigned int magic; /** * @lhead: -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 09/11] drm: direct include of drm.h in drm_syncobj.c
Do not rely on including drm.h from drm_file.h, as the include in drm_file.h will be dropped. Signed-off-by: Sam Ravnborg Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Cc: Lionel Landwerlin Cc: Chunming Zhou Cc: Christian König --- drivers/gpu/drm/drm_syncobj.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index a199c8d56b95..75cb4bb7619e 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -53,6 +53,7 @@ #include #include +#include #include #include #include -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111077] link_shader and deserialize_glsl_program suddenly consume huge amount of RAM
https://bugs.freedesktop.org/show_bug.cgi?id=111077 --- Comment #9 from rol...@rptd.ch --- I get two config attempts. This is the second one. Do you see anything out of place here? meson --buildtype plain --libdir lib64 --localstatedir /var/lib --prefix /usr --sysconfdir /etc --wrap-mode nodownload -Dplatforms=x11,surfaceless,wayland,drm -Dllvm=true -Dlmsensors=true -Dlibunwind=false -Dgallium-nine=false -Dgallium-va=false -Dgallium-vdpau=false -Dgallium-xa=false -Dgallium-xvmc=false -Dgallium-opencl=icd -Dosmesa=none -Dbuild-tests=false -Dglx=dri -Dshared-glapi=true -Ddri3=true -Degl=true -Dgbm=true -Dgles1=false -Dgles2=true -Dglvnd=false -Dselinux=false -Dvalgrind=false -Ddri-drivers=r100,r200 -Dgallium-drivers=r300,r600,radeonsi,swrast -Dvulkan-drivers=amd --buildtype plain -Db_ndebug=true /var/tmp/portage/media-libs/mesa-19.0.8/work/mesa-19.0.8 /var/tmp/portage/media-libs/mesa-19.0.8/work/mesa-19.0.8-abi_x86_64.amd64 -- 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 v2 5/7] drm/bridge: Add Analogix anx6345 support
On Wed, Jun 12, 2019 at 11:13:10AM +0200, Andrzej Hajda wrote: > On 04.06.2019 14:23, Torsten Duwe wrote: > > + > > +static void anx6345_poweron(struct anx6345 *anx6345) > > +{ > > + int err; > > + > > + /* Ensure reset is asserted before starting power on sequence */ > > + gpiod_set_value_cansleep(anx6345->gpiod_reset, 1); > > With fixed devm_gpiod_get below this line can be removed. In any case, reset must be asserted for this procedure to succeed... > > +static enum drm_mode_status > > +anx6345_bridge_mode_valid(struct drm_bridge *bridge, > > + const struct drm_display_mode *mode) > > +{ > > + if (mode->flags & DRM_MODE_FLAG_INTERLACE) > > + return MODE_NO_INTERLACE; > > + > > + /* Max 1200p at 5.4 Ghz, one lane */ > > + if (mode->clock > 154000) > > + return MODE_CLOCK_HIGH; > > I wonder if you shouldn't take into account training results here, ie. > training perfrormed before validation. Sure, but this is verbatim from the anx78xx.c sibling, code provided by analogix. Lacking in-depth datasheets, this is probably the best effort. > > + > > + /* 2.5V digital core power regulator */ > > + anx6345->dvdd25 = devm_regulator_get(dev, "dvdd25-supply"); > > + if (IS_ERR(anx6345->dvdd25)) { > > + DRM_ERROR("dvdd25-supply not found\n"); > > + return PTR_ERR(anx6345->dvdd25); > > + } > > + > > + /* GPIO for chip reset */ > > + anx6345->gpiod_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW); > > Shouldn't be set to GPIOD_OUT_HIGH? It used to be in the original submission, and confused even more people ;-) Fact is, the reset for this chip _is_ low active; I'm following Documentation/devicetree/bindings/gpio/gpio.txt, "1.1) GPIO specifier best practices", which I find rather comprehensive. Any suggestions on how to phrase this even better are appreciated. > > +}; > > +module_i2c_driver(anx6345_driver); > > + > > +MODULE_DESCRIPTION("ANX6345 eDP Transmitter driver"); > > +MODULE_AUTHOR("Enric Balletbo i Serra "); > > Submitter, patch author, and module author are different, this can be > correct, but maybe somebody forgot to update some of these fields. As mentioned in the v2 cover letter, I had a closer look on which code got actually introduced and which lines were simply copied around, and made the copyright and authorship stick to where they belong. *I* believe this is correct now; specific improvements welcome. Torsten
[PATCH v2 05/12] drm/i915: Never set limited_color_range=true for YCbCr output
From: Ville Syrjälä crtc_state->limited_color_range only applies to RGB output but we're currently setting it even for YCbCr output. That will lead to conflicting MSA and PIPECONF settings which can mess up the image. Let's make sure limited_color_range stays unset with YCbCr output. Also WARN if we end up with such a bogus combination when programming the MSA MISC bits as it's impossible to even indicate quantization rangle for YCbCr via MSA MISC. YCbCr output is simply assumed to be limited range always. Note that VSC SDP does provide a mechanism for full range YCbCr, so in the future we may want to rethink how we compute/store this state. And for good measure we add the same WARN to the HDMI path. v2: s/==/!=/ in the HDMI WARN Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_ddi.c | 10 +++--- drivers/gpu/drm/i915/display/intel_dp.c | 10 ++ drivers/gpu/drm/i915/display/intel_hdmi.c | 20 +--- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 157c5851a688..7dd54f573f35 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -1706,9 +1706,6 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state) temp = TRANS_MSA_SYNC_CLK; - if (crtc_state->limited_color_range) - temp |= TRANS_MSA_CEA_RANGE; - switch (crtc_state->pipe_bpp) { case 18: temp |= TRANS_MSA_6_BPC; @@ -1727,6 +1724,13 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state) break; } + /* nonsense combination */ + WARN_ON(crtc_state->limited_color_range && + crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB); + + if (crtc_state->limited_color_range) + temp |= TRANS_MSA_CEA_RANGE; + /* * As per DP 1.2 spec section 2.3.4.3 while sending * YCBCR 444 signals we should program MSA MISC1/0 fields with diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 0eb5d66f87a7..84d2724f0854 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2126,6 +2126,16 @@ bool intel_dp_limited_color_range(const struct intel_crtc_state *crtc_state, const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode; + /* +* Our YCbCr output is always limited range. +* crtc_state->limited_color_range only applies to RGB, +* and it must never be set for YCbCr or we risk setting +* some conflicting bits in PIPECONF which will mess up +* the colors on the monitor. +*/ + if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) + return false; + if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) { /* * See: diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index ca377ba3a15e..325abd462a46 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -724,6 +724,10 @@ intel_hdmi_compute_avi_infoframe(struct intel_encoder *encoder, drm_hdmi_avi_infoframe_colorspace(frame, conn_state); + /* nonsense combination */ + WARN_ON(crtc_state->limited_color_range && + crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB); + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_RGB) { drm_hdmi_avi_infoframe_quant_range(frame, connector, adjusted_mode, @@ -2305,6 +2309,16 @@ static bool intel_hdmi_limited_color_range(const struct intel_crtc_state *crtc_s const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode; + /* +* Our YCbCr output is always limited range. +* crtc_state->limited_color_range only applies to RGB, +* and it must never be set for YCbCr or we risk setting +* some conflicting bits in PIPECONF which will mess up +* the colors on the monitor. +*/ + if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) + return false; + if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) { /* See CEA-861-E - 5.1 Default Encoding Parameters */ return crtc_state->has_hdmi_sink && @@ -2341,9 +2355,6 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder, if (pipe_config->has_hdmi_sink) pipe_config->has_infoframe = true; - pipe_config->limited_color_range = - intel_hdmi_limited_color_range(pipe_config, conn_state); - if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) { p
Re: [PATCH v1 02/11] drm: drop uapi dependency from drm_print.h
Quoting Sam Ravnborg (2019-07-18 17:14:58) > drm_print.h used DRM_NAME - thus adding a dependency from > include/drm/drm_print.h => uapi/drm/drm.h > > Hardcode the name "drm" to break this dependency. > The idea is that there shall be a minimal dependency > between include/drm/* and uapi/* > > Signed-off-by: Sam Ravnborg > Suggested-by: Daniel Vetter > Reviewed-by: Daniel Vetter > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Rob Clark > Cc: Sean Paul > Cc: Chris Wilson > Cc: Daniel Vetter > --- > include/drm/drm_print.h | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > index a5d6f2f3e430..760d1bd0eaf1 100644 > --- a/include/drm/drm_print.h > +++ b/include/drm/drm_print.h > @@ -32,8 +32,6 @@ > #include > #include > > -#include > - > /** > * DOC: print > * > @@ -287,7 +285,7 @@ void drm_err(const char *format, ...); > /* Macros to make printk easier */ > > #define _DRM_PRINTK(once, level, fmt, ...) \ > - printk##once(KERN_##level "[" DRM_NAME "] " fmt, ##__VA_ARGS__) > + printk##once(KERN_##level "[drm] " fmt, ##__VA_ARGS__) I guess I'm th only one who #undef DRM_NAME #define DRM_NAME i915 just so that I didn't have inane logs? One might suggest that instead of hardcoding it, follow the pr_fmt() pattern and only add "[drm]" for the drm core. Even then it so useless (which drm driver is this message for???) that I want to remove them all :( -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 25/60] drm: Add helper to create a connector for a chain of bridges
On Sun, Jul 07, 2019 at 09:19:02PM +0300, Laurent Pinchart wrote: > Most bridge drivers create a DRM connector to model the connector at the > output of the bridge. This model is historical and has worked pretty > well so far, but causes several issues: > > - It prevents supporting more complex display pipelines where DRM > connector operations are split over multiple components. For instance a > pipeline with a bridge connected to the DDC signals to read EDID data, > and another one connected to the HPD signal to detect connection and > disconnection, will not be possible to support through this model. > > - It requires every bridge driver to implement similar connector > handling code, resulting in code duplication. > > - It assumes that a bridge will either be wired to a connector or to > another bridge, but doesn't support bridges that can be used in both > positions very well (although there is some ad-hoc support for this in > the analogix_dp bridge driver). > > In order to solve these issues, ownership of the connector needs to be > moved to the display controller driver. > > To avoid code duplication in display controller drivers, add a new > helper to create and manage a DRM connector backed by a chain of > bridges. All connector operations are delegating to the appropriate > bridge in the chain. > > Signed-off-by: Laurent Pinchart > --- > drivers/gpu/drm/Makefile | 3 +- > drivers/gpu/drm/drm_bridge_connector.c | 385 + > include/drm/drm_bridge_connector.h | 18 ++ > 3 files changed, 405 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/drm_bridge_connector.c > create mode 100644 include/drm/drm_bridge_connector.h > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 9f0d2ee35794..1b74653c9db9 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -37,7 +37,8 @@ drm_vram_helper-y := drm_gem_vram_helper.o \ >drm_vram_mm_helper.o > obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o > > -drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_dsc.o > drm_probe_helper.o \ > +drm_kms_helper-y := drm_bridge_connector.o drm_crtc_helper.o drm_dp_helper.o > \ > + drm_dsc.o drm_probe_helper.o \ > drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \ > drm_kms_helper_common.o drm_dp_dual_mode_helper.o \ > drm_simple_kms_helper.o drm_modeset_helper.o \ > diff --git a/drivers/gpu/drm/drm_bridge_connector.c > b/drivers/gpu/drm/drm_bridge_connector.c > new file mode 100644 > index ..09f2d6bfb561 > --- /dev/null > +++ b/drivers/gpu/drm/drm_bridge_connector.c > @@ -0,0 +1,385 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2019 Laurent Pinchart > + */ > + > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/** > + * DOC: overview > + * > + * The DRM bridge connector helper object provides a DRM connector > + * implementation that wraps a chain of &struct drm_bridge. The connector > + * operations are fully implemented based on the operations of the bridges in > + * the chain, and don't require any intervention from the display controller > + * driver at runtime. > + * > + * To use the helper, display controller drivers create a bridge connector > with > + * a call to drm_bridge_connector_init(). This associates the newly created > + * connector with the chain of bridges passed to the function and registers > it > + * with the DRM device. At that point the connector becomes fully usable, no > + * further operation is needed. > + * > + * The DRM bridge connector operations are implemented based on the > operations > + * provided by the bridges in the chain. Each connector operation is > delegated > + * to the bridge closest to the connector (at the end of the chain) that > + * provides the relevant functionality. > + * > + * To make use of this helper, all bridges in the chain shall report bridge > + * operation flags (&drm_bridge->ops) and bridge output type > + * (&drm_bridge->type), and none of them may create a DRM connector directly. > + */ > + > +/** > + * struct drm_bridge_connector - A connector backed by a chain of bridges > + */ > +struct drm_bridge_connector { > + /** > + * @base: The base DRM connector > + */ > + struct drm_connector base; > + /** > + * @bridge: > + * > + * The first bridge in the chain (connected to the output of the CRTC). > + */ > + struct drm_bridge *bridge; > + /** > + * @bridge_edid: > + * > + * The last bridge in the chain (closest to the connector) that provides > + * EDID read support, if any (see &DRM_BRIDGE_OP_EDID). > + */ > + struct drm_bridge *bridge_edid; > + /** > + * @bridge_hpd: > + * > + * The last bridge in the chain (closest to
Re: [PATCH 3/5] drm/panfrost: Add a no execute flag for BO allocations
On Thu, Jul 18, 2019 at 9:03 AM Steven Price wrote: > > On 17/07/2019 19:33, Rob Herring wrote: > > Executable buffers have an alignment restriction that they can't cross > > 16MB boundary as the GPU program counter is 24-bits. This restriction is > > currently not handled and we just get lucky. As current userspace > > Actually it depends which GPU you are using - some do have a bigger > program counter - so it might be the choice of GPU to test on rather > than just luck. But kbase always assumes the worst case (24 bit) as in > practise that's enough. > > > assumes all BOs are executable, that has to remain the default. So add a > > new PANFROST_BO_NOEXEC flag to allow userspace to indicate which BOs are > > not executable. > > > > There is also a restriction that executable buffers cannot start or end > > on a 4GB boundary. This is mostly avoided as there is only 4GB of space > > currently and the beginning is already blocked out for NULL ptr > > detection. Add support to handle this restriction fully regardless of > > the current constraints. > > > > For existing userspace, all created BOs remain executable, but the GPU > > VA alignment will be increased to the size of the BO. This shouldn't > > matter as there is plenty of GPU VA space. > > > > Cc: Tomeu Vizoso > > Cc: Boris Brezillon > > Cc: Robin Murphy > > Cc: Steven Price > > Cc: Alyssa Rosenzweig > > Signed-off-by: Rob Herring > > --- > > drivers/gpu/drm/panfrost/panfrost_drv.c | 20 +++- > > drivers/gpu/drm/panfrost/panfrost_gem.c | 18 -- > > drivers/gpu/drm/panfrost/panfrost_gem.h | 3 ++- > > drivers/gpu/drm/panfrost/panfrost_mmu.c | 3 +++ > > include/uapi/drm/panfrost_drm.h | 2 ++ > > 5 files changed, 42 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c > > b/drivers/gpu/drm/panfrost/panfrost_drv.c > > index d354b92964d5..b91e991bc6a3 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > > @@ -49,7 +49,8 @@ static int panfrost_ioctl_create_bo(struct drm_device > > *dev, void *data, > > struct panfrost_gem_object *bo; > > struct drm_panfrost_create_bo *args = data; > > > > - if (!args->size || args->flags || args->pad) > > + if (!args->size || args->pad || > > + (args->flags & ~PANFROST_BO_NOEXEC)) > > return -EINVAL; > > > > bo = panfrost_gem_create_with_handle(file, dev, args->size, > > args->flags, > > @@ -367,6 +368,22 @@ static struct drm_driver panfrost_drm_driver = { > > .gem_prime_mmap = drm_gem_prime_mmap, > > }; > > > > +#define PFN_4G_MASK((SZ_4G - 1) >> PAGE_SHIFT) > > + > > +static void panfrost_drm_mm_color_adjust(const struct drm_mm_node *node, > > + unsigned long color, > > + u64 *start, u64 *end) > > +{ > > + /* Executable buffers can't start or end on a 4GB boundary */ > > + if (!color) { > > + if ((*start & PFN_4G_MASK) == 0) > > + (*start)++; > > + > > + if ((*end & PFN_4G_MASK) == 0) > > + (*end)--; > > + } > > +} > > Unless I'm mistaken this won't actually provide the guarantee if the > memory region is >4GB (which admittedly it isn't at the moment). For > example a 8GB region would have the beginning/end trimmed off, but > there's another 4GB in the middle which could be allocated next to. Humm, other than not allowing sizes greater than 4G-(2*PAGE_SIZE), I'm not sure how we solve that. I guess avoiding sizes of (n*4G - PAGE_SIZE) would avoid the problem. Seems like almost 4GB executable buffer should be enough for anyone(TM). > Also a minor issue, but we might want to consider having some constants > for 'color' - it's not obvious from this code that color==no_exec. Yeah, I was just going worry about that when we had a second bit to pass. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 01/11] drm/panel: make drm_panel.h self-contained
On Thu, Jul 18, 2019 at 06:14:57PM +0200, Sam Ravnborg wrote: > From: Jani Nikula > > Fix build warning if drm_panel.h is built with CONFIG_OF=n or > CONFIG_DRM_PANEL=n and included without the prerequisite err.h: > > ./include/drm/drm_panel.h: In function ‘of_drm_find_panel’: > ./include/drm/drm_panel.h:203:9: error: implicit declaration of function > ‘ERR_PTR’ [-Werror=implicit-function-declaration] > return ERR_PTR(-ENODEV); > ^~~ > ./include/drm/drm_panel.h:203:9: error: returning ‘int’ from a function with > return type ‘struct drm_panel *’ makes pointer from integer without a cast > [-Werror=int-conversion] > return ERR_PTR(-ENODEV); > ^~~~ > > Fixes: 5fa8e4a22182 ("drm/panel: Make of_drm_find_panel() return an ERR_PTR() > instead of NULL") > Cc: Boris Brezillon > Signed-off-by: Jani Nikula > Acked-by: Thierry Reding > Reviewed-by: Sam Ravnborg Reviewed-by: Sean Paul > --- > include/drm/drm_panel.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h > index 8c738c0e6e9f..26377836141c 100644 > --- a/include/drm/drm_panel.h > +++ b/include/drm/drm_panel.h > @@ -24,6 +24,7 @@ > #ifndef __DRM_PANEL_H__ > #define __DRM_PANEL_H__ > > +#include > #include > #include > > -- > 2.20.1 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 02/11] drm: drop uapi dependency from drm_print.h
On Thu, Jul 18, 2019 at 06:14:58PM +0200, Sam Ravnborg wrote: > drm_print.h used DRM_NAME - thus adding a dependency from > include/drm/drm_print.h => uapi/drm/drm.h > > Hardcode the name "drm" to break this dependency. > The idea is that there shall be a minimal dependency > between include/drm/* and uapi/* You might also want to clean up the other uses of DRM_NAME in armada and i915 while you're at it. The easiest way to satisfy Chris' usecase and remove the dependency would be to add #define DRM_PRINT_NAME "drm" in drm_print.h and use that. Sean > > Signed-off-by: Sam Ravnborg > Suggested-by: Daniel Vetter > Reviewed-by: Daniel Vetter > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Rob Clark > Cc: Sean Paul > Cc: Chris Wilson > Cc: Daniel Vetter > --- > include/drm/drm_print.h | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > index a5d6f2f3e430..760d1bd0eaf1 100644 > --- a/include/drm/drm_print.h > +++ b/include/drm/drm_print.h > @@ -32,8 +32,6 @@ > #include > #include > > -#include > - > /** > * DOC: print > * > @@ -287,7 +285,7 @@ void drm_err(const char *format, ...); > /* Macros to make printk easier */ > > #define _DRM_PRINTK(once, level, fmt, ...) \ > - printk##once(KERN_##level "[" DRM_NAME "] " fmt, ##__VA_ARGS__) > + printk##once(KERN_##level "[drm] " fmt, ##__VA_ARGS__) > > #define DRM_INFO(fmt, ...) \ > _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__) > -- > 2.20.1 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 03/11] drm: drop uapi dependency from drm_vblank.h
On Thu, Jul 18, 2019 at 06:14:59PM +0200, Sam Ravnborg wrote: > drm_vblank.h included uapi/drm/drm.h. > It turns out this include was not required - delete it. > > Note: uapi/drm/drm.h is included indirect via drm_file.h, > but there are no dependencies in drm_vblank.h so the removal > is legit. > > Signed-off-by: Sam Ravnborg > Reviewed-by: Daniel Vetter > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > Cc: Stefan Agner > Cc: Thierry Reding Reviewed-by: Sean Paul > --- > include/drm/drm_vblank.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h > index e528bb2f659d..9fe4ba8bc622 100644 > --- a/include/drm/drm_vblank.h > +++ b/include/drm/drm_vblank.h > @@ -30,7 +30,6 @@ > > #include > #include > -#include > > struct drm_device; > struct drm_crtc; > -- > 2.20.1 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 04/11] drm/ati_pcigart: drop dependency on drm_os_linux.h
On Thu, Jul 18, 2019 at 06:15:00PM +0200, Sam Ravnborg wrote: > The drm_os_linux.h header is deprecated. > Just opencode the sole DRM_WRITE32(). Any plans for the other users of DRM_WRITE()? It seems like it'd be trivial to fix it up for via and mga. I don't really have any background on drm_os_linux.h, but it doesn't seem like it'd be that much more effort to just remove the whole thing. Sean > > Signed-off-by: Sam Ravnborg > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > --- > drivers/gpu/drm/ati_pcigart.c | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/ati_pcigart.c b/drivers/gpu/drm/ati_pcigart.c > index 2a413e291a60..580aa2676358 100644 > --- a/drivers/gpu/drm/ati_pcigart.c > +++ b/drivers/gpu/drm/ati_pcigart.c > @@ -35,7 +35,6 @@ > > #include > #include > -#include > #include > #include > > @@ -169,6 +168,7 @@ int drm_ati_pcigart_init(struct drm_device *dev, struct > drm_ati_pcigart_info *ga > page_base = (u32) entry->busaddr[i]; > > for (j = 0; j < (PAGE_SIZE / ATI_PCIGART_PAGE_SIZE); j++) { > + u32 offset; > u32 val; > > switch(gart_info->gart_reg_if) { > @@ -184,10 +184,12 @@ int drm_ati_pcigart_init(struct drm_device *dev, struct > drm_ati_pcigart_info *ga > break; > } > if (gart_info->gart_table_location == > - DRM_ATI_GART_MAIN) > + DRM_ATI_GART_MAIN) { > pci_gart[gart_idx] = cpu_to_le32(val); > - else > - DRM_WRITE32(map, gart_idx * sizeof(u32), val); > + } else { > + offset = gart_idx * sizeof(u32); > + writel(val, (void __iomem *)map->handle + > offset); > + } > gart_idx++; > page_base += ATI_PCIGART_PAGE_SIZE; > } > -- > 2.20.1 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger
Quoting Brendan Higgins (2019-07-16 11:52:01) > On Tue, Jul 16, 2019 at 10:50 AM Stephen Boyd wrote: > > > > > The only hypothetical case where this can't be done is a complicated > > assertion or expectation that does more than one check and can't be > > written as a function that dumps out what went wrong. Is this a real > > problem? Maybe such an assertion should just open code that logic so we > > don't have to build up a string for all the other simple cases. > > I have some expectations in follow up patchsets for which I created a > set of composable matchers for matching structures and function calls > that by their nature cannot be written as a single function. The > matcher thing is a bit speculative, I know, but for any kind of > function call matching, you need to store a record of functions you > are expecting to have called and then each one needs to have a set of > expectations defined by the user; I don't think there is a way to do > that that doesn't involve having multiple separate functions each > having some information useful to constructing the message. > > I know the code in question isn't in this patchset; the function > matching code was in one of the earlier versions of the RFC, but I > dropped it to make this patchset smaller and more manageable. So I get > it if you would like me to drop it and add it back in when I try to > get the function and structure matching stuff in, but I would really > prefer to keep it as is if you don't care too much. Do you have a link to those earlier patches? > > > It seems far simpler to get rid of the string stream API and just have a > > struct for this. > > > > struct kunit_fail_msg { > > const char *line; > > const char *file; > > const char *func; > > const char *msg; > > }; > > > > Then you can have the assertion macros create this on the stack (with > > another macro?). > > > > #define DEFINE_KUNIT_FAIL_MSG(name, _msg) \ > > struct kunit_fail_msg name = { \ > > .line = __LINE__, \ > > .file = __FILE__, \ > > .func = __func__, \ > > .msg = _msg, \ > > } > > > > I don't want to derail this whole series on this topic, but it seems > > like a bunch of code is there to construct this same set of information > > over and over again into a buffer a little bit at a time and then throw > > it away when nothing fails just because we may want to support the case > > where we have some unstructured data to inform the user about. > > Yeah, that's fair. I think there are a number of improvements to be > made with how the expectations are defined other than that, but I was > hoping I could do that after this patchset is merged. I just figured > with the kinds of things I would like to do, it would lead to a whole > new round of discussion. > > In either case, I think I would still like to use the `struct > kunit_stream` as part of the interface to share the failure message > with the test case runner code in test.c, at least eventually, so that > I only have to have one way to receive data from expectations, but I > think I can do that and still do what you suggest by just constructing > the kunit_stream at the end of expectations where it is feasible. > > All in all I agree with what you are saying, but I would rather do it > as a follow up possibly once we have some more code on the table. I > could just see this opening up a whole new can of worms where we > debate about exactly how expectations and assertions work for another > several months, only to rip it all out shortly there after. I know > that's how these things go, but that's my preference. > > I can do what you suggest if you feel strongly about it, but I would > prefer to hold off until later. It's your call. > The crux of my complaint is that the string stream API is too loosely defined to be usable. It allows tests to build up a string of unstructured information, but with certain calling constraints so we have to tread carefully. If there was more structure to the data that's being recorded then the test case runner could operate on the data without having to do string/stream operations, allocations, etc. This would make the assertion logic much more concrete and specific to kunit, instead of this small kunit wrapper that's been placed on top of string stream. TL;DR: If we can get rid of the string stream API I'd view that as an improvement because building arbitrary strings in the kernel is complex, error prone and has calling context concerns. Is the intention that other code besides unit tests will use this string stream API to build up strings? Any targets in mind? This would be a good way to get the API merged upstream given that its 2019 and we haven't had such an API in the kernel so far. An "object oriented" (strong quotes!) approach where kunit_fail_ms
Re: [PATCH v1 06/11] drm: direct include of drm.h in drm_gem.c
On Thu, Jul 18, 2019 at 06:15:02PM +0200, Sam Ravnborg wrote: > Do not rely on including drm.h from drm_file.h, > as the include in drm_file.h will be dropped. > > Signed-off-by: Sam Ravnborg Reviewed-by: Sean Paul > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > Cc: Eric Anholt > Cc: Thomas Zimmermann > Cc: Rob Herring > --- > drivers/gpu/drm/drm_gem.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index e6c12c6ec728..243f43d70f42 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -39,6 +39,7 @@ > #include > #include > > +#include > #include > #include > #include > -- > 2.20.1 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 05/11] drm/vblank: drop use of DRM_WAIT_ON()
On Thu, Jul 18, 2019 at 06:15:01PM +0200, Sam Ravnborg wrote: > DRM_WAIT_ON() is from the deprecated drm_os_linux header and > the modern replacement is the wait_event_*. > > The return values differ, so a conversion is needed to > keep the original interface towards userspace. > Introduced a switch/case to make code obvious and to allow > different debug prints depending on the result. > > Signed-off-by: Sam Ravnborg Reviewed-by: Sean Paul > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > --- > drivers/gpu/drm/drm_vblank.c | 29 - > 1 file changed, 20 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index 603ab105125d..8e9ac187500e 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -31,7 +31,6 @@ > #include > #include > #include > -#include > #include > > #include "drm_internal.h" > @@ -1672,19 +1671,31 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, > void *data, > if (req_seq != seq) { > DRM_DEBUG("waiting on vblank count %llu, crtc %u\n", > req_seq, pipe); > - DRM_WAIT_ON(ret, vblank->queue, 3 * HZ, > - vblank_passed(drm_vblank_count(dev, pipe), > - req_seq) || > - !READ_ONCE(vblank->enabled)); > + ret = wait_event_interruptible_timeout(vblank->queue, > + vblank_passed(drm_vblank_count(dev, pipe), req_seq) || > + !READ_ONCE(vblank->enabled), > + msecs_to_jiffies(3000)); > } > > - if (ret != -EINTR) { > + switch (ret) { > + case 0: > + /* timeout */ > + ret = -EBUSY; > drm_wait_vblank_reply(dev, pipe, &vblwait->reply); > - > - DRM_DEBUG("crtc %d returning %u to client\n", > + DRM_DEBUG("timeout waiting for vblank. crtc %d returning %u to > client\n", > pipe, vblwait->reply.sequence); > - } else { > + break; > + case -ERESTARTSYS: > + /* interrupted by signal */ > + ret = -EINTR; > DRM_DEBUG("crtc %d vblank wait interrupted by signal\n", pipe); > + break; > + default: > + ret = 0; > + drm_wait_vblank_reply(dev, pipe, &vblwait->reply); > + DRM_DEBUG("crtc %d returning %u to client\n", > + pipe, vblwait->reply.sequence); > + break; > } > > done: > -- > 2.20.1 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 07/11] drm: direct include of drm.h in drm_gem_shmem_helper.c
On Thu, Jul 18, 2019 at 06:15:03PM +0200, Sam Ravnborg wrote: > Do not rely on including drm.h from drm_file.h, > as the include in drm_file.h will be dropped. > > Signed-off-by: Sam Ravnborg Reviewed-by: Sean Paul > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > Cc: Eric Anholt > Cc: Thomas Zimmermann > Cc: Rob Herring > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > b/drivers/gpu/drm/drm_gem_shmem_helper.c > index 472ea5d81f82..2f64667ac805 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -10,6 +10,7 @@ > #include > #include > > +#include > #include > #include > #include > -- > 2.20.1 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 10/11] drm/mediatek: direct include of drm.h in mtk_drm_gem.c
On Thu, Jul 18, 2019 at 06:15:06PM +0200, Sam Ravnborg wrote: > Do not rely on including drm.h from drm_file.h, > as the include in drm_file.h will be dropped. > > Signed-off-by: Sam Ravnborg Reviewed-by: Sean Paul > Cc: CK Hu > Cc: Philipp Zabel > Cc: Matthias Brugger > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-media...@lists.infradead.org > --- > drivers/gpu/drm/mediatek/mtk_drm_gem.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c > b/drivers/gpu/drm/mediatek/mtk_drm_gem.c > index 9434f88c6341..ca672f1d140d 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c > @@ -5,6 +5,7 @@ > > #include > > +#include > #include > #include > #include > -- > 2.20.1 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 08/11] drm: direct include of drm.h in drm_prime.c
On Thu, Jul 18, 2019 at 06:15:04PM +0200, Sam Ravnborg wrote: > Do not rely on including drm.h from drm_file.h, > as the include in drm_file.h will be dropped. > > Signed-off-by: Sam Ravnborg Reviewed-by: Sean Paul > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > Cc: Christian König > Cc: Noralf Trønnes > Cc: Chris Wilson > Cc: Eric Anholt > --- > drivers/gpu/drm/drm_prime.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 189d980402ad..eca484106cc2 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -30,6 +30,7 @@ > #include > #include > > +#include > #include > #include > #include > -- > 2.20.1 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel