[Bug 111077] link_shader and deserialize_glsl_program suddenly consume huge amount of RAM

2019-07-18 Thread bugzilla-daemon
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

2019-07-18 Thread 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?

>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()

2019-07-18 Thread Frederick Lawler
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()

2019-07-18 Thread Frederick Lawler
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

2019-07-18 Thread Alexey Brodkin
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

2019-07-18 Thread Ben Skeggs
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

2019-07-18 Thread Koenig, Christian
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

2019-07-18 Thread Shawn Guo
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

2019-07-18 Thread 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(-)

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

2019-07-18 Thread bugzilla-daemon
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

2019-07-18 Thread Liviu Dudau
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

2019-07-18 Thread Christoph Hellwig
> +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

2019-07-18 Thread Christoph Hellwig
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?

2019-07-18 Thread Michel Dänzer
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Wen He


> -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

2019-07-18 Thread bugzilla-daemon
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

2019-07-18 Thread Chunming Zhou
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

2019-07-18 Thread 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?

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

2019-07-18 Thread 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.

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

2019-07-18 Thread Jens Remus

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

2019-07-18 Thread Noralf Trønnes


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()

2019-07-18 Thread Noralf Trønnes


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()

2019-07-18 Thread Noralf Trønnes


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

2019-07-18 Thread Noralf Trønnes


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

2019-07-18 Thread Jacek Anaszewski
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()

2019-07-18 Thread Bjorn Helgaas
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()

2019-07-18 Thread Bjorn Helgaas
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-07-18 Thread Chunming Zhou

在 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-07-18 Thread Chunming Zhou

在 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

2019-07-18 Thread 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.
-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-07-18 Thread Chunming Zhou

在 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

2019-07-18 Thread Liviu Dudau
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

2019-07-18 Thread Jacek Anaszewski
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

2019-07-18 Thread Chris Wilson
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-07-18 Thread Chunming Zhou

在 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

2019-07-18 Thread Arnd Bergmann
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

2019-07-18 Thread 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)

-- 
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

2019-07-18 Thread bugzilla-daemon
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?

2019-07-18 Thread Alex Deucher
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

2019-07-18 Thread 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.







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

2019-07-18 Thread bugzilla-daemon
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

2019-07-18 Thread Andrzej Hajda
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

2019-07-18 Thread Arnd Bergmann
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-07-18 Thread Chunming Zhou

在 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

2019-07-18 Thread Ville Syrjala
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()

2019-07-18 Thread Ville Syrjala
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

2019-07-18 Thread Ville Syrjala
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

2019-07-18 Thread Ville Syrjala
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

2019-07-18 Thread Ville Syrjala
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

2019-07-18 Thread Ville Syrjala
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

2019-07-18 Thread Ville Syrjala
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

2019-07-18 Thread Ville Syrjala
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

2019-07-18 Thread Ville Syrjala
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()

2019-07-18 Thread Ville Syrjala
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

2019-07-18 Thread Ville Syrjala
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

2019-07-18 Thread Ville Syrjala
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

2019-07-18 Thread Ville Syrjala
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

2019-07-18 Thread Andrzej Hajda
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

2019-07-18 Thread Arnd Bergmann
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

2019-07-18 Thread Sean Paul

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

2019-07-18 Thread Dmitry Torokhov
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

2019-07-18 Thread Sean Paul
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

2019-07-18 Thread Arnd Bergmann
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Jordan Crouse
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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()

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread Sam Ravnborg
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

2019-07-18 Thread bugzilla-daemon
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

2019-07-18 Thread Torsten Duwe
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

2019-07-18 Thread Ville Syrjala
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

2019-07-18 Thread Chris Wilson
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

2019-07-18 Thread Daniel Vetter
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

2019-07-18 Thread Rob Herring
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

2019-07-18 Thread Sean Paul
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

2019-07-18 Thread Sean Paul
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

2019-07-18 Thread Sean Paul
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

2019-07-18 Thread Sean Paul
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

2019-07-18 Thread Stephen Boyd
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

2019-07-18 Thread Sean Paul
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()

2019-07-18 Thread Sean Paul
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

2019-07-18 Thread Sean Paul
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

2019-07-18 Thread Sean Paul
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

2019-07-18 Thread Sean Paul
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

  1   2   3   >