Re: [PATCH] drm/exynos: Fix cleanup of IOMMU related objects

2020-03-04 Thread Inki Dae
Hi Marek,

20. 3. 2. 오후 11:27에 Marek Szyprowski 이(가) 쓴 글:
> Store the IOMMU mapping created by device core of each Exynos DRM
> sub-device and restore it when Exynos DRM driver is unbound. This fixes
> IOMMU initialization failure for the second time when deferred probe is
> triggered from the bind() callback of master's compound DRM driver.
> 
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  5 +++--
>  drivers/gpu/drm/exynos/exynos7_drm_decon.c|  5 +++--
>  drivers/gpu/drm/exynos/exynos_drm_dma.c   | 22 +++
>  drivers/gpu/drm/exynos/exynos_drm_drv.h   |  6 +++--
>  drivers/gpu/drm/exynos/exynos_drm_fimc.c  |  5 +++--
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c  |  5 +++--
>  drivers/gpu/drm/exynos/exynos_drm_g2d.c   |  5 +++--
>  drivers/gpu/drm/exynos/exynos_drm_gsc.c   |  5 +++--
>  drivers/gpu/drm/exynos/exynos_drm_rotator.c   |  5 +++--
>  drivers/gpu/drm/exynos/exynos_drm_scaler.c|  6 +++--
>  drivers/gpu/drm/exynos/exynos_mixer.c |  7 --
>  11 files changed, 47 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c 
> b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> index 8428ae12dfa5..1f79bc2a881e 100644
> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> @@ -55,6 +55,7 @@ static const char * const decon_clks_name[] = {
>  struct decon_context {
>   struct device   *dev;
>   struct drm_device   *drm_dev;
> + void*dma_priv;
>   struct exynos_drm_crtc  *crtc;
>   struct exynos_drm_plane planes[WINDOWS_NR];
>   struct exynos_drm_plane_config  configs[WINDOWS_NR];
> @@ -644,7 +645,7 @@ static int decon_bind(struct device *dev, struct device 
> *master, void *data)
>  
>   decon_clear_channels(ctx->crtc);
>  
> - return exynos_drm_register_dma(drm_dev, dev);
> + return exynos_drm_register_dma(drm_dev, dev, >dma_priv);
>  }
>  
>  static void decon_unbind(struct device *dev, struct device *master, void 
> *data)
> @@ -654,7 +655,7 @@ static void decon_unbind(struct device *dev, struct 
> device *master, void *data)
>   decon_atomic_disable(ctx->crtc);
>  
>   /* detach this sub driver from iommu mapping if supported. */
> - exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
> + exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, >dma_priv);
>  }
>  
>  static const struct component_ops decon_component_ops = {
> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c 
> b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> index ff59c641fa80..1eed3327999f 100644
> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> @@ -40,6 +40,7 @@
>  struct decon_context {
>   struct device   *dev;
>   struct drm_device   *drm_dev;
> + void*dma_priv;
>   struct exynos_drm_crtc  *crtc;
>   struct exynos_drm_plane planes[WINDOWS_NR];
>   struct exynos_drm_plane_config  configs[WINDOWS_NR];
> @@ -127,13 +128,13 @@ static int decon_ctx_initialize(struct decon_context 
> *ctx,
>  
>   decon_clear_channels(ctx->crtc);
>  
> - return exynos_drm_register_dma(drm_dev, ctx->dev);
> + return exynos_drm_register_dma(drm_dev, ctx->dev, >dma_priv);
>  }
>  
>  static void decon_ctx_remove(struct decon_context *ctx)
>  {
>   /* detach this sub driver from iommu mapping if supported. */
> - exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
> + exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, >dma_priv);
>  }
>  
>  static u32 decon_calc_clkdiv(struct decon_context *ctx,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c 
> b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> index 9ebc02768847..482bec7756fa 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dma.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> @@ -58,7 +58,7 @@ static inline void clear_dma_max_seg_size(struct device 
> *dev)
>   * mapping.
>   */
>  static int drm_iommu_attach_device(struct drm_device *drm_dev,
> - struct device *subdrv_dev)
> + struct device *subdrv_dev, void **dma_priv)
>  {
>   struct exynos_drm_private *priv = drm_dev->dev_private;
>   int ret;
> @@ -74,7 +74,8 @@ static int drm_iommu_attach_device(struct drm_device 
> *drm_dev,
>   return ret;
>  
>   if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
> - if (to_dma_iommu_mapping(subdrv_dev))
> + *dma_priv = to_dma_iommu_mapping(subdrv_dev);
> + if (*dma_priv)
>   arm_iommu_detach_device(subdrv_dev);
>  
>   ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
> @@ -98,19 +99,21 @@ static int drm_iommu_attach_device(struct drm_device 
> *drm_dev,
>   * mapping
>   */
>  static 

Re: [PATCH] drm/exynos: Fix memory leak and release IOMMU mapping structures

2020-03-04 Thread Marek Szyprowski
Hi Lukasz,

On 04.03.2020 23:00, Lukasz Luba wrote:
> There is a memory leak which left some objects not freed. The reference
> counter of mapping: 'mapping->kref' was 2 when calling
> arm_iommu_detach_device(), so the release_iommu_mapping() won't be called.
> Since the old mapping structure is not going to be used any more (because
> it is detached and new one attached), call arm_iommu_release_mapping()
> to trigger cleanup.

This will break IOMMU support in Exynos DRM if deferred probe happens. 
Here is a proper fix:

https://patchwork.kernel.org/patch/11415715/

The mapping initially created by DMA-mapping framework should be 
attached back when Exynos DRM releases the subdev device.

> Found using kmemleak detector, the output:
>
> unreferenced object 0xc2137640 (size 64):
>comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>hex dump (first 32 bytes):
>  50 a3 14 c2 80 a2 14 c2 01 00 00 00 20 00 00 00  P... ...
>  00 10 00 00 00 80 00 00 00 00 00 00 00 00 00 00  
>backtrace:
>  [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>  [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>  [] really_probe+0xb0/0x47c
>  [<4f510e4f>] driver_probe_device+0x78/0x1c4
>  [<7481a0cf>] device_driver_attach+0x58/0x60
>  [<0ff8f5c1>] __driver_attach+0xb8/0x158
>  [<86006144>] bus_for_each_dev+0x74/0xb4
>  [<10159dca>] bus_add_driver+0x1c0/0x200
>  [<8a265265>] driver_register+0x74/0x108
>  [] exynos_drm_init+0xb0/0x134
>  [] do_one_initcall+0x90/0x458
>  [<6da35917>] kernel_init_freeable+0x188/0x200
>  [] kernel_init+0x8/0x110
>  [<1f3cddf9>] ret_from_fork+0x14/0x20
>  [<8cd12507>] 0x0
> unreferenced object 0xc214a280 (size 128):
>comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>hex dump (first 32 bytes):
>  00 a0 ec ed 00 00 00 00 00 00 00 00 00 00 00 00  
>  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
>backtrace:
>  [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>  [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>  [] really_probe+0xb0/0x47c
>  [<4f510e4f>] driver_probe_device+0x78/0x1c4
>  [<7481a0cf>] device_driver_attach+0x58/0x60
>  [<0ff8f5c1>] __driver_attach+0xb8/0x158
>  [<86006144>] bus_for_each_dev+0x74/0xb4
>  [<10159dca>] bus_add_driver+0x1c0/0x200
>  [<8a265265>] driver_register+0x74/0x108
>  [] exynos_drm_init+0xb0/0x134
>  [] do_one_initcall+0x90/0x458
>  [<6da35917>] kernel_init_freeable+0x188/0x200
>  [] kernel_init+0x8/0x110
>  [<1f3cddf9>] ret_from_fork+0x14/0x20
>  [<8cd12507>] 0x0
> unreferenced object 0xedeca000 (size 4096):
>comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>hex dump (first 32 bytes):
>  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
>  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
>backtrace:
>  [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>  [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>  [] really_probe+0xb0/0x47c
>  [<4f510e4f>] driver_probe_device+0x78/0x1c4
>  [<7481a0cf>] device_driver_attach+0x58/0x60
>  [<0ff8f5c1>] __driver_attach+0xb8/0x158
>  [<86006144>] bus_for_each_dev+0x74/0xb4
>  [<10159dca>] bus_add_driver+0x1c0/0x200
>  [<8a265265>] driver_register+0x74/0x108
>  [] exynos_drm_init+0xb0/0x134
>  [] do_one_initcall+0x90/0x458
>  [<6da35917>] kernel_init_freeable+0x188/0x200
>  [] kernel_init+0x8/0x110
>  [<1f3cddf9>] ret_from_fork+0x14/0x20
>  [<8cd12507>] 0x0
> unreferenced object 0xc214a300 (size 128):
>comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>hex dump (first 32 bytes):
>  00 a3 14 c2 00 a3 14 c2 00 40 18 c2 00 80 18 c2  .@..
>  02 00 02 00 ad 4e ad de ff ff ff ff ff ff ff ff  .N..
>backtrace:
>  [<08cbd8bc>] iommu_domain_alloc+0x24/0x50
>  [] arm_iommu_create_mapping+0xe4/0x134
>  [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>  [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>  [] really_probe+0xb0/0x47c
>  [<4f510e4f>] driver_probe_device+0x78/0x1c4
>  [<7481a0cf>] device_driver_attach+0x58/0x60
>  [<0ff8f5c1>] __driver_attach+0xb8/0x158
>  [<86006144>] bus_for_each_dev+0x74/0xb4
>  [<10159dca>] bus_add_driver+0x1c0/0x200
>  [<8a265265>] driver_register+0x74/0x108
>  [] exynos_drm_init+0xb0/0x134
>  [] do_one_initcall+0x90/0x458
>  [<6da35917>] kernel_init_freeable+0x188/0x200
>  [] kernel_init+0x8/0x110
>  [<1f3cddf9>] ret_from_fork+0x14/0x20
>
> Signed-off-by: Lukasz Luba 
> ---
>
> Hi all,
>
> I have discovered this issue on OdroidXU4 while running some stress tests
> for upcoming Energy Model. To reproduce it, kernel must be compiled with
> DEBUG_KMEMLEAK. When the boot has finished, type:
> # echo scan > /sys/kernel/debug/kmemleak
> # cat /sys/kernel/debug/kmemleak
> You should expect similar 

Re: [PATCH] gpu/drm/v3d: Add ARCH_BCM2835 to DRM_V3D Kconfig

2020-03-04 Thread Eric Anholt
On Wed, Dec 18, 2019 at 6:39 AM Nicolas Saenz Julienne
 wrote:
>
> Hi Peter,
>
> On Wed, 2019-12-18 at 08:43 +, Peter Robinson wrote:
> > On arm64 the config ARCH_BCM doesn't exist so to be able to
> > build for platforms such as the Raspberry Pi 4 we need to add
> > ARCH_BCM2835 similar to what has been done on vc4.
> >
> > Signed-off-by: Peter Robinson 
> > ---
>
> v3d's upstream implementation doesn't support RPi4 for now. So I don't see how
> we could benefit from this.

All you need is a compatible string for making this driver work on
pi4's v3d, so this seems like a good change to be making, to me.

Peter, feel like defining the compatible string too?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] GPU: DRM: VC4/V3D Replace wait_for macros in to remove use of msleep

2020-03-04 Thread Eric Anholt
On Thu, Feb 20, 2020 at 1:44 AM James Hughes
 wrote:
>
> On Wed, 19 Feb 2020 at 22:51, Eric Anholt  wrote:
> >
> > On Mon, Feb 17, 2020 at 7:41 AM James Hughes
> >  wrote:
> > >
> > > The wait_for macro's for Broadcom VC4 and V3D drivers used msleep
> > > which is inappropriate due to its inaccuracy at low values (minimum
> > > wait time is about 30ms on the Raspberry Pi).
> > >
> > > This patch replaces the macro with the one from the Intel i915 version
> > > which uses usleep_range to provide more accurate waits.
> > >
> > > Signed-off-by: James Hughes 
> >
> > To apply this, we're going to want to split the patch up between v3d
> > (with a fixes tag to the introduction of the driver, since it's a
> > pretty critical fix) and vc4 (where it's used in KMS, and we're pretty
> > sure we want it but changing timing like this in KMS paths is risky so
> > we don't want to backport to stable).  And adjust the commit messages
> > to have consistent prefixes to the rest of the commits to those
> > drivers.
> >
> > I've been fighting with the drm maintainer tools today to try to apply
> > the patch, with no luck.   I'll keep trying, and if I succeed, I'll
> > push it.
>
> Hi Eric,
>
> unclear whether you want me to do the split or whether you are going
> to (your last paragraph). Also I'm a bit unclear on the exact
> requirements for the prefixes etc.

I debugged what was going on with my maintainer tools and got the
patches applied:

https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9daee6141cc9c75b09659b02b1cb9eeb2f5e16cc
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=7f2a09ecf2e8d86e22598dfb879db48e72c5a40e

Apologies for the wait!  I've fixed some mail filters I think so I
should notice pings like this in the future.  But also I hope Maxime
can feel enabled to merge patches to vc4/v3d in the future -- I
certainly don't want to be the limiting factor here, and it's under
drm-misc so any drm-misc maintainer can apply stuff.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[GIT PULL] mediatek drm fixes for 5.6

2020-03-04 Thread CK Hu
Hi, Dave & Daniel:

This include OVL, cursor, and gce fixup.

Regards,
CK

The following changes since commit
bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9:

  Linux 5.6-rc1 (2020-02-09 16:08:48 -0800)

are available in the Git repository at:

  https://github.com/ckhu-mediatek/linux.git-tags.git
tags/mediatek-drm-fixes-5.6

for you to fetch changes up to 3d2ed431b8f39483477bc3c3a2aefbc9778ffe12:

  drm/mediatek: Handle component type MTK_DISP_OVL_2L correctly
(2020-02-25 13:02:22 +0800)


Mediatek DRM Fixes for Linux 5.6


Bibby Hsieh (4):
  drm/mediatek: Add plane check in async_check function
  drm/mediatek: Add fb swap in async_update
  drm/mediatek: Move gce event property to mutex device node
  drm/mediatek: Make sure previous message done or be aborted before
send

Evan Benn (1):
  drm/mediatek: Find the cursor plane instead of hard coding it

Phong LE (1):
  drm/mediatek: Handle component type MTK_DISP_OVL_2L correctly

Sean Paul (1):
  drm/mediatek: Ensure the cursor plane is on top of other overlays

 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 30
+++--
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  1 +
 drivers/gpu/drm/mediatek/mtk_drm_plane.c|  7 +++
 3 files changed, 28 insertions(+), 10 deletions(-)


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 2/2] drm/virtio: add case for shmem objects in virtio_gpu_cleanup_object(..)

2020-03-04 Thread Gurchetan Singh
This function can be reused for hostmem objects.

v2: move virtio_gpu_is_shmem() check to virtio_gpu_cleanup_object()
v3: use-after free fix

Signed-off-by: Gurchetan Singh 
---
 drivers/gpu/drm/virtio/virtgpu_drv.h|  2 +-
 drivers/gpu/drm/virtio/virtgpu_object.c | 33 ++---
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 8e2027da6cce..c1824bdf2418 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -371,7 +371,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device 
*vgdev,
 struct virtio_gpu_object **bo_ptr,
 struct virtio_gpu_fence *fence);
 
-bool virtio_gpu_is_shmem(struct drm_gem_object *obj);
+bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo);
 
 /* virtgpu_prime.c */
 struct drm_gem_object *virtgpu_gem_prime_import_sg_table(
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c 
b/drivers/gpu/drm/virtio/virtgpu_object.c
index 1f8b062bb9eb..cc65e8b3ec8b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -65,21 +65,26 @@ static void virtio_gpu_resource_id_put(struct 
virtio_gpu_device *vgdev, uint32_t
 void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo)
 {
struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private;
-   struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
 
-   if (shmem->pages) {
-   if (shmem->mapped) {
-   dma_unmap_sg(vgdev->vdev->dev.parent,
-shmem->pages->sgl, shmem->mapped,
-DMA_TO_DEVICE);
-   shmem->mapped = 0;
+   virtio_gpu_resource_id_put(vgdev, bo->hw_res_handle);
+   if (virtio_gpu_is_shmem(bo)) {
+   struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
+
+   if (shmem->pages) {
+   if (shmem->mapped) {
+   dma_unmap_sg(vgdev->vdev->dev.parent,
+shmem->pages->sgl, shmem->mapped,
+DMA_TO_DEVICE);
+   shmem->mapped = 0;
+   }
+
+   sg_free_table(shmem->pages);
+   shmem->pages = NULL;
+   drm_gem_shmem_unpin(>base.base);
}
-   sg_free_table(shmem->pages);
-   shmem->pages = NULL;
-   drm_gem_shmem_unpin(>base.base);
+
+   drm_gem_shmem_free_object(>base.base);
}
-   virtio_gpu_resource_id_put(vgdev, bo->hw_res_handle);
-   drm_gem_shmem_free_object(>base.base);
 }
 
 static void virtio_gpu_free_object(struct drm_gem_object *obj)
@@ -110,9 +115,9 @@ static const struct drm_gem_object_funcs 
virtio_gpu_shmem_funcs = {
.mmap = drm_gem_shmem_mmap,
 };
 
-bool virtio_gpu_is_shmem(struct drm_gem_object *obj)
+bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo)
 {
-   return obj->funcs == _gpu_shmem_funcs;
+   return bo->base.base.funcs == _gpu_shmem_funcs;
 }
 
 struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev,
-- 
2.25.1.481.gfbce0eb801-goog

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object

2020-03-04 Thread Gurchetan Singh
A resource will be a shmem based resource or a (planned)
vram based resource, so it makes sense to factor out common fields
(resource handle, dumb).

v2: move mapped field to shmem object

Signed-off-by: Gurchetan Singh 
---
 drivers/gpu/drm/virtio/virtgpu_drv.h| 13 +++
 drivers/gpu/drm/virtio/virtgpu_object.c | 31 ++---
 drivers/gpu/drm/virtio/virtgpu_vq.c |  6 +++--
 3 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index ce73895cf74b..8e2027da6cce 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -69,16 +69,21 @@ struct virtio_gpu_object_params {
 struct virtio_gpu_object {
struct drm_gem_shmem_object base;
uint32_t hw_res_handle;
-
-   struct sg_table *pages;
-   uint32_t mapped;
-
bool dumb;
bool created;
 };
 #define gem_to_virtio_gpu_obj(gobj) \
container_of((gobj), struct virtio_gpu_object, base.base)
 
+struct virtio_gpu_object_shmem {
+   struct virtio_gpu_object base;
+   struct sg_table *pages;
+   uint32_t mapped;
+};
+
+#define to_virtio_gpu_shmem(virtio_gpu_object) \
+   container_of((virtio_gpu_object), struct virtio_gpu_object_shmem, base)
+
 struct virtio_gpu_object_array {
struct ww_acquire_ctx ticket;
struct list_head next;
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c 
b/drivers/gpu/drm/virtio/virtgpu_object.c
index c5cad949eb8d..1f8b062bb9eb 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -65,16 +65,17 @@ static void virtio_gpu_resource_id_put(struct 
virtio_gpu_device *vgdev, uint32_t
 void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo)
 {
struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private;
+   struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
 
-   if (bo->pages) {
-   if (bo->mapped) {
+   if (shmem->pages) {
+   if (shmem->mapped) {
dma_unmap_sg(vgdev->vdev->dev.parent,
-bo->pages->sgl, bo->mapped,
+shmem->pages->sgl, shmem->mapped,
 DMA_TO_DEVICE);
-   bo->mapped = 0;
+   shmem->mapped = 0;
}
-   sg_free_table(bo->pages);
-   bo->pages = NULL;
+   sg_free_table(shmem->pages);
+   shmem->pages = NULL;
drm_gem_shmem_unpin(>base.base);
}
virtio_gpu_resource_id_put(vgdev, bo->hw_res_handle);
@@ -133,6 +134,7 @@ static int virtio_gpu_object_shmem_init(struct 
virtio_gpu_device *vgdev,
unsigned int *nents)
 {
bool use_dma_api = !virtio_has_iommu_quirk(vgdev->vdev);
+   struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
struct scatterlist *sg;
int si, ret;
 
@@ -140,19 +142,20 @@ static int virtio_gpu_object_shmem_init(struct 
virtio_gpu_device *vgdev,
if (ret < 0)
return -EINVAL;
 
-   bo->pages = drm_gem_shmem_get_sg_table(>base.base);
-   if (!bo->pages) {
+   shmem->pages = drm_gem_shmem_get_sg_table(>base.base);
+   if (!shmem->pages) {
drm_gem_shmem_unpin(>base.base);
return -EINVAL;
}
 
if (use_dma_api) {
-   bo->mapped = dma_map_sg(vgdev->vdev->dev.parent,
-   bo->pages->sgl, bo->pages->nents,
-   DMA_TO_DEVICE);
-   *nents = bo->mapped;
+   shmem->mapped = dma_map_sg(vgdev->vdev->dev.parent,
+  shmem->pages->sgl,
+  shmem->pages->nents,
+  DMA_TO_DEVICE);
+   *nents = shmem->mapped;
} else {
-   *nents = bo->pages->nents;
+   *nents = shmem->pages->nents;
}
 
*ents = kmalloc_array(*nents, sizeof(struct virtio_gpu_mem_entry),
@@ -162,7 +165,7 @@ static int virtio_gpu_object_shmem_init(struct 
virtio_gpu_device *vgdev,
return -ENOMEM;
}
 
-   for_each_sg(bo->pages->sgl, sg, *nents, si) {
+   for_each_sg(shmem->pages->sgl, sg, *nents, si) {
(*ents)[si].addr = cpu_to_le64(use_dma_api
   ? sg_dma_address(sg)
   : sg_phys(sg));
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c 
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5e2375e0f7bb..73854915ec34 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -600,10 +600,11 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct 
virtio_gpu_device *vgdev,
struct 

Re: [PATCH v2 4/4] drm/bridge: dw-hdmi: rework csc related functions

2020-03-04 Thread Laurent Pinchart
Hi Jernej,

Thank you for the patch.

On Thu, Mar 05, 2020 at 12:25:12AM +0100, Jernej Skrabec wrote:
> is_color_space_conversion() is a misnomer. It checks not only if color
> space conversion is needed, but also if format conversion is needed.
> This is actually desired behaviour because result of this function
> determines if CSC block should be enabled or not (CSC block can also do
> format conversion).
> 
> In order to clear misunderstandings, let's rework
> is_color_space_conversion() to do exactly what is supposed to do and add
> another function which will determine if CSC block must be enabled or
> not.
> 
> Signed-off-by: Jernej Skrabec 
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 31 +++
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index c8a02e5b5e1b..7724191e0a8b 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -963,11 +963,14 @@ static void hdmi_video_sample(struct dw_hdmi *hdmi)
>  
>  static int is_color_space_conversion(struct dw_hdmi *hdmi)
>  {
> - return (hdmi->hdmi_data.enc_in_bus_format !=
> - hdmi->hdmi_data.enc_out_bus_format) ||
> -(hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_in_bus_format) &&
> - hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format) &&
> - hdmi->hdmi_data.rgb_limited_range);
> + struct hdmi_data_info *hdmi_data = >hdmi_data;
> + bool is_input_rgb, is_output_rgb;
> +
> + is_input_rgb = hdmi_bus_fmt_is_rgb(hdmi_data->enc_in_bus_format);
> + is_output_rgb = hdmi_bus_fmt_is_rgb(hdmi_data->enc_out_bus_format);
> +
> + return (is_input_rgb != is_output_rgb) ||
> +(is_input_rgb && is_output_rgb && hdmi_data->rgb_limited_range);
>  }
>  
>  static int is_color_space_decimation(struct dw_hdmi *hdmi)
> @@ -994,6 +997,13 @@ static int is_color_space_interpolation(struct dw_hdmi 
> *hdmi)
>   return 0;
>  }
>  
> +static bool is_conversion_needed(struct dw_hdmi *hdmi)

Maybe is_csc_needed ?

Reviewed-by: Laurent Pinchart 

> +{
> + return is_color_space_conversion(hdmi) ||
> +is_color_space_decimation(hdmi) ||
> +is_color_space_interpolation(hdmi);
> +}
> +
>  static void dw_hdmi_update_csc_coeffs(struct dw_hdmi *hdmi)
>  {
>   const u16 (*csc_coeff)[3][4] = _coeff_default;
> @@ -2014,18 +2024,19 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi 
> *hdmi)
>   hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
>  
>   /* Enable csc path */
> - if (is_color_space_conversion(hdmi)) {
> + if (is_conversion_needed(hdmi)) {
>   hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_CSCCLK_DISABLE;
>   hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
> - }
>  
> - /* Enable color space conversion if needed */
> - if (is_color_space_conversion(hdmi))
>   hdmi_writeb(hdmi, HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_IN_PATH,
>   HDMI_MC_FLOWCTRL);
> - else
> + } else {
> + hdmi->mc_clkdis |= HDMI_MC_CLKDIS_CSCCLK_DISABLE;
> + hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
> +
>   hdmi_writeb(hdmi, HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_BYPASS,
>   HDMI_MC_FLOWCTRL);
> + }
>  }
>  
>  /* Workaround to clear the overflow condition */

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 3/4] drm/bridge: dw-hdmi: Add support for RGB limited range

2020-03-04 Thread Laurent Pinchart
Hi Jernej,

Thank you for the patch.

On Thu, Mar 05, 2020 at 12:25:11AM +0100, Jernej Skrabec wrote:
> CEA 861 standard requestis that RGB quantization range is "limited" for
> CEA modes. Support that by adding CSC matrix which downscales values.
> 
> This allows proper color reproduction on TV and PC monitor at the same
> time. In future, override property can be added, like "Broadcast RGB"
> in i915 driver.
> 
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 63 +--
>  1 file changed, 46 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index de2c7ec887c8..c8a02e5b5e1b 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -92,6 +92,12 @@ static const u16 csc_coeff_rgb_in_eitu709[3][4] = {
>   { 0x6756, 0x78ab, 0x2000, 0x0200 }
>  };
>  
> +static const u16 csc_coeff_rgb_full_to_rgb_limited[3][4] = {
> + { 0x1b7c, 0x, 0x, 0x0020 },
> + { 0x, 0x1b7c, 0x, 0x0020 },
> + { 0x, 0x, 0x1b7c, 0x0020 }
> +};
> +
>  struct hdmi_vmode {
>   bool mdataenablepolarity;
>  
> @@ -109,6 +115,7 @@ struct hdmi_data_info {
>   unsigned int pix_repet_factor;
>   unsigned int hdcp_enable;
>   struct hdmi_vmode video_mode;
> + bool rgb_limited_range;
>  };
>  
>  struct dw_hdmi_i2c {
> @@ -956,7 +963,11 @@ static void hdmi_video_sample(struct dw_hdmi *hdmi)
>  
>  static int is_color_space_conversion(struct dw_hdmi *hdmi)
>  {
> - return hdmi->hdmi_data.enc_in_bus_format != 
> hdmi->hdmi_data.enc_out_bus_format;
> + return (hdmi->hdmi_data.enc_in_bus_format !=
> + hdmi->hdmi_data.enc_out_bus_format) ||
> +(hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_in_bus_format) &&
> + hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format) &&
> + hdmi->hdmi_data.rgb_limited_range);
>  }
>  
>  static int is_color_space_decimation(struct dw_hdmi *hdmi)
> @@ -986,25 +997,27 @@ static int is_color_space_interpolation(struct dw_hdmi 
> *hdmi)
>  static void dw_hdmi_update_csc_coeffs(struct dw_hdmi *hdmi)
>  {
>   const u16 (*csc_coeff)[3][4] = _coeff_default;
> + bool is_input_rgb, is_output_rgb;
>   unsigned i;
>   u32 csc_scale = 1;
>  
> - if (is_color_space_conversion(hdmi)) {
> - if (hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format)) {
> - if (hdmi->hdmi_data.enc_out_encoding ==
> - V4L2_YCBCR_ENC_601)
> - csc_coeff = _coeff_rgb_out_eitu601;
> - else
> - csc_coeff = _coeff_rgb_out_eitu709;
> - } else if (hdmi_bus_fmt_is_rgb(
> - hdmi->hdmi_data.enc_in_bus_format)) {
> - if (hdmi->hdmi_data.enc_out_encoding ==
> - V4L2_YCBCR_ENC_601)
> - csc_coeff = _coeff_rgb_in_eitu601;
> - else
> - csc_coeff = _coeff_rgb_in_eitu709;
> - csc_scale = 0;
> - }
> + is_input_rgb = hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_in_bus_format);
> + is_output_rgb = hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format);
> +
> + if (!is_input_rgb && is_output_rgb) {
> + if (hdmi->hdmi_data.enc_out_encoding == V4L2_YCBCR_ENC_601)
> + csc_coeff = _coeff_rgb_out_eitu601;
> + else
> + csc_coeff = _coeff_rgb_out_eitu709;
> + } else if (is_input_rgb && !is_output_rgb) {
> + if (hdmi->hdmi_data.enc_out_encoding == V4L2_YCBCR_ENC_601)
> + csc_coeff = _coeff_rgb_in_eitu601;
> + else
> + csc_coeff = _coeff_rgb_in_eitu709;
> + csc_scale = 0;
> + } else if (is_input_rgb && is_output_rgb &&
> +hdmi->hdmi_data.rgb_limited_range) {
> + csc_coeff = _coeff_rgb_full_to_rgb_limited;
>   }
>  
>   /* The CSC registers are sequential, alternating MSB then LSB */
> @@ -1614,6 +1627,18 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, 
> struct drm_display_mode *mode)
>   drm_hdmi_avi_infoframe_from_display_mode(,
>>connector, mode);
>  
> + if (hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format)) {
> + drm_hdmi_avi_infoframe_quant_range(, >connector,
> +mode,
> +
> hdmi->hdmi_data.rgb_limited_range ?
> +
> HDMI_QUANTIZATION_RANGE_LIMITED :
> +
> HDMI_QUANTIZATION_RANGE_FULL);
> + } else {
> 

[PATCH 0/3] drm/dp_mst: Fix bandwidth checking regressions from DSC patches

2020-03-04 Thread Lyude Paul
AMD's patch series for adding DSC support to the MST helpers
unfortunately introduced a few regressions into the kernel that I didn't
get around to fixing until just now. I would have reverted the changes
earlier, but seeing as that would have reverted all of amd's DSC support
+ everything that was done on top of that I really wanted to avoid
doing that.

Anyway, this should fix everything as far as I can tell. Note that I
don't have any DSC displays locally yet, so if someone from AMD could
sanity check this I would appreciate it ♥.

Cc: Mikita Lipski 
Cc: Alex Deucher 
Cc: Sean Paul 
Cc: Hans de Goede 

Lyude Paul (3):
  drm/dp_mst: Rename drm_dp_mst_is_dp_mst_end_device() to be less
redundant
  drm/dp_mst: Don't show connectors as connected before probing
available PBN
  drm/dp_mst: Rewrite and fix bandwidth limit checks

 drivers/gpu/drm/drm_dp_mst_topology.c | 124 --
 1 file changed, 96 insertions(+), 28 deletions(-)

-- 
2.24.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 2/3] drm/dp_mst: Don't show connectors as connected before probing available PBN

2020-03-04 Thread Lyude Paul
It's next to impossible for us to do connector probing on topologies
without occasionally racing with userspace, since creating a connector
itself causes a hotplug event which we have to send before probing the
available PBN of a connector. Even if we didn't have this hotplug event
sent, there's still always a chance that userspace started probing
connectors before we finished probing the topology.

This can be a problem when validating a new MST state since the
connector will be shown as connected briefly, but without any available
PBN - causing any atomic state which would enable said connector to fail
with -ENOSPC. So, let's simply workaround this by telling userspace new
MST connectors are disconnected until we've finished probing their PBN.
Since we always send a hotplug event at the end of the link address
probing process, userspace will still know to reprobe the connector when
we're ready.

Signed-off-by: Lyude Paul 
Fixes: cd82d82cbc04 ("drm/dp_mst: Add branch bandwidth validation to MST atomic 
check")
Cc: Mikita Lipski 
Cc: Alex Deucher 
Cc: Sean Paul 
Cc: Hans de Goede 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 207eef08d12c..7b0ff0cff954 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4033,6 +4033,19 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
ret = connector_status_connected;
break;
}
+
+   /* We don't want to tell userspace the port is actually plugged into
+* anything until we've finished probing it's available_pbn, otherwise
+* userspace will see racy atomic check failures
+*
+* Since we always send a hotplug at the end of probing topology
+* state, we can just let userspace reprobe this connector later.
+*/
+   if (ret == connector_status_connected && !port->available_pbn) {
+   DRM_DEBUG_KMS("[CONNECTOR:%d:%s] not ready yet (PBN not 
probed)\n",
+ connector->base.id, connector->name);
+   ret = connector_status_disconnected;
+   }
 out:
drm_dp_mst_topology_put_port(port);
return ret;
-- 
2.24.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 3/3] drm/dp_mst: Rewrite and fix bandwidth limit checks

2020-03-04 Thread Lyude Paul
Sigh, this is mostly my fault for not giving commit cd82d82cbc04
("drm/dp_mst: Add branch bandwidth validation to MST atomic check")
enough scrutiny during review. The way we're checking bandwidth
limitations here is mostly wrong.

First things first, we need to follow the locking conventions for MST.
Whenever traversing downwards (upwards is always safe) in the topology,
we need to hold >lock to prevent the topology from changing under
us. We don't currently do that when performing bandwidth limit checks.

Next we need to figure out the actual PBN limit for the primary MSTB.
Here we actually want to use the highest available_pbn value we can find
on each level of the topology, then make sure that the combined sum of
allocated PBN on each port of the branch device doesn't exceed that
amount. Currently, we just loop through each level of the topology and
use the last non-zero PBN we find.

Once we've done that, we then want to traverse down each branch device
we find in the topology with at least one downstream port that has PBN
allocated in our atomic state, and repeat the whole process on each
level of the topology as we travel down. While doing this, we need to
take care to avoid attempting to traverse down end devices. We don't
currently do this, although I'm not actually sure whether or not this
broke anything before.

Since there's a bit too many issues here to try to fix one by one, and
the drm_dp_mst_atomic_check_bw_limit() code is not entirely clear on all
of these pain points anyway, let's just take the easy way out and
rewrite the whole function. Additionally, we also add a kernel warning
if we find that any ports we performed bandwidth limit checks on didn't
actually have available_pbn populated - as this is always a bug in the
MST helpers.

This should fix regressions seen on nouveau, i915 and amdgpu where we
erroneously reject atomic states that should fit within bandwidth
limitations.

Signed-off-by: Lyude Paul 
Fixes: cd82d82cbc04 ("drm/dp_mst: Add branch bandwidth validation to MST atomic 
check")
Cc: Mikita Lipski 
Cc: Alex Deucher 
Cc: Sean Paul 
Cc: Hans de Goede 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 101 --
 1 file changed, 78 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 7b0ff0cff954..87dc7c92d339 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4853,41 +4853,90 @@ static bool drm_dp_mst_port_downstream_of_branch(struct 
drm_dp_mst_port *port,
return false;
 }
 
-static inline
-int drm_dp_mst_atomic_check_bw_limit(struct drm_dp_mst_branch *branch,
-struct drm_dp_mst_topology_state 
*mst_state)
+static int
+drm_dp_mst_atomic_check_bw_limit(struct drm_dp_mst_branch *branch,
+struct drm_dp_mst_topology_state *mst_state)
 {
struct drm_dp_mst_port *port;
struct drm_dp_vcpi_allocation *vcpi;
-   int pbn_limit = 0, pbn_used = 0;
+   int pbn_limit = 0, pbn_used = 0, ret;
 
-   list_for_each_entry(port, >ports, next) {
-   if (port->mstb)
-   if (drm_dp_mst_atomic_check_bw_limit(port->mstb, 
mst_state))
-   return -ENOSPC;
+   if (branch->port_parent)
+   DRM_DEBUG_ATOMIC("[MSTB:%p] [MST PORT:%p] checking [MSTB:%p]\n",
+branch->port_parent->parent,
+branch->port_parent, branch);
+   else
+   DRM_DEBUG_ATOMIC("Checking [MSTB:%p]\n", branch);
 
-   if (port->available_pbn > 0)
+   list_for_each_entry(port, >ports, next) {
+   /* Since each port shares a link, the highest PBN we find
+* should be assumed to be the limit for this branch device
+*/
+   if (pbn_limit < port->available_pbn)
pbn_limit = port->available_pbn;
-   }
-   DRM_DEBUG_ATOMIC("[MST BRANCH:%p] branch has %d PBN available\n",
-branch, pbn_limit);
 
-   list_for_each_entry(vcpi, _state->vcpis, next) {
-   if (!vcpi->pbn)
+   if (port->pdt == DP_PEER_DEVICE_NONE)
continue;
 
-   if (drm_dp_mst_port_downstream_of_branch(vcpi->port, branch))
-   pbn_used += vcpi->pbn;
+   if (drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
+   list_for_each_entry(vcpi, _state->vcpis, next) {
+   if (vcpi->port != port)
+   continue;
+   if (!vcpi->pbn)
+   break;
+
+   /* This should never happen, as it means we
+* tried to set a mode before querying the
+* available_pbn
+

[PATCH 1/3] drm/dp_mst: Rename drm_dp_mst_is_dp_mst_end_device() to be less redundant

2020-03-04 Thread Lyude Paul
It's already prefixed by dp_mst, so we don't really need to repeat
ourselves here. One of the changes I should have picked up originally
when reviewing MST DSC support.

There should be no functional changes here

Cc: Mikita Lipski 
Cc: Alex Deucher 
Cc: Sean Paul 
Cc: Hans de Goede 
Signed-off-by: Lyude Paul 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 61e7beada832..207eef08d12c 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1937,7 +1937,7 @@ static u8 drm_dp_calculate_rad(struct drm_dp_mst_port 
*port,
return parent_lct + 1;
 }
 
-static bool drm_dp_mst_is_dp_mst_end_device(u8 pdt, bool mcs)
+static bool drm_dp_mst_is_end_device(u8 pdt, bool mcs)
 {
switch (pdt) {
case DP_PEER_DEVICE_DP_LEGACY_CONV:
@@ -1967,13 +1967,13 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 
new_pdt,
 
/* Teardown the old pdt, if there is one */
if (port->pdt != DP_PEER_DEVICE_NONE) {
-   if (drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) {
+   if (drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
/*
 * If the new PDT would also have an i2c bus,
 * don't bother with reregistering it
 */
if (new_pdt != DP_PEER_DEVICE_NONE &&
-   drm_dp_mst_is_dp_mst_end_device(new_pdt, new_mcs)) {
+   drm_dp_mst_is_end_device(new_pdt, new_mcs)) {
port->pdt = new_pdt;
port->mcs = new_mcs;
return 0;
@@ -1993,7 +1993,7 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 
new_pdt,
port->mcs = new_mcs;
 
if (port->pdt != DP_PEER_DEVICE_NONE) {
-   if (drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) {
+   if (drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
/* add i2c over sideband */
ret = drm_dp_mst_register_i2c_bus(>aux);
} else {
@@ -2169,7 +2169,7 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch 
*mstb,
}
 
if (port->pdt != DP_PEER_DEVICE_NONE &&
-   drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) {
+   drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
port->cached_edid = drm_get_edid(port->connector,
 >aux.ddc);
drm_connector_set_tile_property(port->connector);
-- 
2.24.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] Make drm_dp_mst_dsc_aux_for_port() safe for old compilers

2020-03-04 Thread Paul E. McKenney
Older compilers either want two extra pairs of curly braces around the
initializer for local variable "desc", or they want a single pair of
curly braces with nothing inside.  Current Linux-kernel practice favors
the latter, so this commit makes it so.

This is a fix for a regression introduced into v5.6-rc1.

Fixes: 5b03f9d86880 ("drm/dp_mst: Add new quirk for Synaptics MST hubs")
Suggested-by: Chris Wilson 
Suggested-by: Joe Perches 
Suggested-by: Christoph Hellwig 
Signed-off-by: Paul E. McKenney 
Cc: Maxime Ripard 
Cc: David Airlie 
Cc: Daniel Vetter 

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 20cdaf3..b123f60 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -5396,7 +5396,7 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct 
drm_dp_mst_port *port)
 {
struct drm_dp_mst_port *immediate_upstream_port;
struct drm_dp_mst_port *fec_port;
-   struct drm_dp_desc desc = { 0 };
+   struct drm_dp_desc desc = { };
u8 endpoint_fec;
u8 endpoint_dsc;
 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/amd/display: Remove pointless NULL checks in dmub_psr_copy_settings

2020-03-04 Thread Alex Deucher
On Mon, Mar 2, 2020 at 5:43 PM Nathan Chancellor
 wrote:
>
> Clang warns:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dmub_psr.c:147:31: warning:
> address of 'pipe_ctx->plane_res' will always evaluate to 'true'
> [-Wpointer-bool-conversion]
> if (!pipe_ctx || !_ctx->plane_res || !_ctx->stream_res)
>  ~ ~~^
> drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dmub_psr.c:147:56: warning:
> address of 'pipe_ctx->stream_res' will always evaluate to 'true'
> [-Wpointer-bool-conversion]
> if (!pipe_ctx || !_ctx->plane_res || !_ctx->stream_res)
>   ~ ~~^~
> 2 warnings generated.
>
> As long as pipe_ctx is not NULL, the address of members in this struct
> cannot be NULL, which means these checks will always evaluate to false.
>
> Fixes: 4c1a1335dfe0 ("drm/amd/display: Driverside changes to support PSR in 
> DMCUB")
> Link: https://github.com/ClangBuiltLinux/linux/issues/915
> Signed-off-by: Nathan Chancellor 

Applied.  Thanks!

Alex


> ---
>  drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c 
> b/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c
> index 2c932c29f1f9..a9e1c01e9d9b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c
> @@ -144,7 +144,7 @@ static bool dmub_psr_copy_settings(struct dmub_psr *dmub,
> }
> }
>
> -   if (!pipe_ctx || !_ctx->plane_res || !_ctx->stream_res)
> +   if (!pipe_ctx)
> return false;
>
> // First, set the psr version
> --
> 2.25.1
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3] drm/dp: Add function to parse EDID descriptors for adaptive sync limits

2020-03-04 Thread Ville Syrjälä
On Wed, Mar 04, 2020 at 09:36:06AM -0800, Manasi Navare wrote:
> On Tue, Mar 03, 2020 at 03:42:12PM +0200, Ville Syrjälä wrote:
> > On Mon, Mar 02, 2020 at 04:08:59PM -0800, Manasi Navare wrote:
> > > Adaptive Sync is a VESA feature so add a DRM core helper to parse
> > > the EDID's detailed descritors to obtain the adaptive sync monitor range.
> > > Store this info as part fo drm_display_info so it can be used
> > > across all drivers.
> > > This part of the code is stripped out of amdgpu's function
> > > amdgpu_dm_update_freesync_caps() to make it generic and be used
> > > across all DRM drivers
> > > 
> > > v3:
> > > * Remove the edid parsing restriction for just DP (Nicholas)
> > > * Use drm_for_each_detailed_block (Ville)
> > > * Make the drm_get_adaptive_sync_range function static (Harry, Jani)
> > > v2:
> > > * Change vmin and vmax to use u8 (Ville)
> > > * Dont store pixel clock since that is just a max dotclock
> > > and not related to VRR mode (Manasi)
> > > 
> > > Cc: Ville Syrjälä 
> > > Cc: Harry Wentland 
> > > Cc: Clinton A Taylor 
> > > Cc: Kazlauskas, Nicholas 
> > > Signed-off-by: Manasi Navare 
> > > ---
> > >  drivers/gpu/drm/drm_edid.c  | 44 +
> > >  include/drm/drm_connector.h | 22 +++
> > >  2 files changed, 66 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > index ad41764a4ebe..e3f152180b6b 100644
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -4938,6 +4938,47 @@ static void drm_parse_cea_ext(struct drm_connector 
> > > *connector,
> > >   }
> > >  }
> > >  
> > > +static
> > > +void get_adaptive_sync_range(struct detailed_timing *timing,
> > > +  void *info_adaptive_sync)
> > > +{
> > > + struct drm_adaptive_sync_info *adaptive_sync = info_adaptive_sync;
> > > + const struct detailed_non_pixel *data = >data.other_data;
> > > + const struct detailed_data_monitor_range *range = >data.range;
> > > +
> > > + if (data->type != EDID_DETAIL_MONITOR_RANGE)a
> > 
> > is_display_descriptor()
> 
> Will change to use is_display_descriptor()
> 
> > 
> > > + return;
> > > +
> > > + /*
> > > +  * Check for flag range limits only. If flag == 1 then
> > > +  * no additional timing information provided.
> > > +  * Default GTF, GTF Secondary curve and CVT are not
> > > +  * supported
> > > +  */
> > > + if (range->flags != 1)
> > 
> > Pls name the flags.
> 
> I dont see that we have any enum with the flag names, do you want me to 
> define them looking
> at EDID spec?

What else?

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/fourcc: Add bayer formats and modifiers

2020-03-04 Thread Sakari Ailus
Hi Niklas,

Thank you for the patch.

On Fri, Feb 28, 2020 at 05:31:35PM +0100, Niklas Söderlund wrote:
> Bayer formats are used with cameras and contain green, red and blue
> components, with alternating lines of red and green, and blue and green
> pixels in different orders. For each block of 2x2 pixels there is one
> pixel with a red filter, two with a green filter, and one with a blue
> filter. The filters can be arranged in different patterns.
> 
> Add DRM fourcc formats to describe the most common Bayer formats. Also
> add a modifiers to describe the custom packing layouts used by the Intel
> IPU3 and in the MIPI (Mobile Industry Processor Interface) CSI-2
> specification.
> 
> Signed-off-by: Niklas Söderlund 
> ---
>  include/uapi/drm/drm_fourcc.h | 95 +++
>  1 file changed, 95 insertions(+)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 8bc0b31597d80737..561d5a08ffd16b69 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -286,6 +286,62 @@ extern "C" {
>  #define DRM_FORMAT_YVU444fourcc_code('Y', 'V', '2', '4') /* 
> non-subsampled Cr (1) and Cb (2) planes */
>  
>  
> +/*
> + * Bayer formats
> + *
> + * Bayer formats contain green, red and blue components, with alternating 
> lines
> + * of red and green, and blue and green pixels in different orders. For each
> + * block of 2x2 pixels there is one pixel with a red filter, two with a green
> + * filter, and one with a blue filter. The filters can be arranged in 
> different
> + * patterns.
> + *
> + * For example, RGGB:
> + *   row0: RGRGRGRG...
> + *   row1: GBGBGBGB...
> + *   row3: RGRGRGRG...
> + *   row4: GBGBGBGB...
> + *   ...
> + *
> + * Vendors have different methods to pack the sampling formats to increase 
> data
> + * density. For this reason the fourcc only describes pixel sample size and 
> the

This could be for other reasons than data density, such as for less
cumbersome memory access. I'd leave out "to increase data density".

> + * filter pattern for each block of 2x2 pixels. A modifier is needed to
> + * describe the memory layout.
> + *
> + * In addition to vendor modifiers for memory layout DRM_FORMAT_MOD_LINEAR 
> may
> + * be used to describe a layout where all samples are placed consecutively in
> + * memory. If the sample does not fit inside a single byte, the sample 
> storage
> + * is extended to the minimum number of (little endian) bytes that can hold 
> the
> + * sample and any unused most-significant bits are defined as padding.

Should it be added here that padding bits are all zero?

> + *
> + * For example, SRGGB10:
> + * Each 10-bit sample is contained in 2 consecutive little endian bytes, 
> where
> + * the 6 most-significant bits are unused.
> + */
> +
> +/* 8-bit Bayer formats */
> +#define DRM_FORMAT_SRGGB8fourcc_code('R', 'G', 'G', 'B')
> +#define DRM_FORMAT_SGRBG8fourcc_code('G', 'R', 'B', 'G')
> +#define DRM_FORMAT_SGBRG8fourcc_code('G', 'B', 'R', 'G')
> +#define DRM_FORMAT_SBGGR8fourcc_code('B', 'A', '8', '1')
> +
> +/* 10-bit Bayer formats */
> +#define DRM_FORMAT_SRGGB10   fourcc_code('R', 'G', '1', '0')
> +#define DRM_FORMAT_SGRBG10   fourcc_code('B', 'A', '1', '0')
> +#define DRM_FORMAT_SGBRG10   fourcc_code('G', 'B', '1', '0')
> +#define DRM_FORMAT_SBGGR10   fourcc_code('B', 'G', '1', '0')
> +
> +/* 12-bit Bayer formats */
> +#define DRM_FORMAT_SRGGB12   fourcc_code('R', 'G', '1', '2')
> +#define DRM_FORMAT_SGRBG12   fourcc_code('B', 'A', '1', '2')
> +#define DRM_FORMAT_SGBRG12   fourcc_code('G', 'B', '1', '2')
> +#define DRM_FORMAT_SBGGR12   fourcc_code('B', 'G', '1', '2')
> +
> +/* 14-bit Bayer formats */
> +#define DRM_FORMAT_SRGGB14   fourcc_code('R', 'G', '1', '4')
> +#define DRM_FORMAT_SGRBG14   fourcc_code('B', 'A', '1', '4')
> +#define DRM_FORMAT_SGBRG14   fourcc_code('G', 'B', '1', '4')
> +#define DRM_FORMAT_SBGGR14   fourcc_code('B', 'G', '1', '4')

The 4cc codes seemingly appear to those used in V4L2. Is that intentional?
Nothing wrong in that though, but is this a rule that's universally
followed?

> +
>  /*
>   * Format Modifiers:
>   *
> @@ -309,6 +365,7 @@ extern "C" {
>  #define DRM_FORMAT_MOD_VENDOR_BROADCOM 0x07
>  #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
>  #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
> +#define DRM_FORMAT_MOD_VENDOR_MIPI 0x0a
>  
>  /* add more to the end as needed */
>  
> @@ -434,6 +491,17 @@ extern "C" {
>   */
>  #define I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS fourcc_mod_code(INTEL, 7)
>  
> +
> +/*
> + * IPU3 Bayer packing layout
> + *
> + * The IPU3 raw Bayer formats use a custom packing layout where there are no
> + * gaps between each 10-bit sample. It packs 25 pixels into 32 bytes leaving
> + * the 6 most significant bits in the last byte unused. The format is little
> + * endian.

Where is endianness really specified? DRM_FORMAT_BIG_ENDIAN would seem
independent of that.

I might omit endianness definition here.

> + */
> 

Re: [PATCH v3] drm/dp: Add function to parse EDID descriptors for adaptive sync limits

2020-03-04 Thread Manasi Navare
On Tue, Mar 03, 2020 at 03:42:12PM +0200, Ville Syrjälä wrote:
> On Mon, Mar 02, 2020 at 04:08:59PM -0800, Manasi Navare wrote:
> > Adaptive Sync is a VESA feature so add a DRM core helper to parse
> > the EDID's detailed descritors to obtain the adaptive sync monitor range.
> > Store this info as part fo drm_display_info so it can be used
> > across all drivers.
> > This part of the code is stripped out of amdgpu's function
> > amdgpu_dm_update_freesync_caps() to make it generic and be used
> > across all DRM drivers
> > 
> > v3:
> > * Remove the edid parsing restriction for just DP (Nicholas)
> > * Use drm_for_each_detailed_block (Ville)
> > * Make the drm_get_adaptive_sync_range function static (Harry, Jani)
> > v2:
> > * Change vmin and vmax to use u8 (Ville)
> > * Dont store pixel clock since that is just a max dotclock
> > and not related to VRR mode (Manasi)
> > 
> > Cc: Ville Syrjälä 
> > Cc: Harry Wentland 
> > Cc: Clinton A Taylor 
> > Cc: Kazlauskas, Nicholas 
> > Signed-off-by: Manasi Navare 
> > ---
> >  drivers/gpu/drm/drm_edid.c  | 44 +
> >  include/drm/drm_connector.h | 22 +++
> >  2 files changed, 66 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index ad41764a4ebe..e3f152180b6b 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -4938,6 +4938,47 @@ static void drm_parse_cea_ext(struct drm_connector 
> > *connector,
> > }
> >  }
> >  
> > +static
> > +void get_adaptive_sync_range(struct detailed_timing *timing,
> > +void *info_adaptive_sync)
> > +{
> > +   struct drm_adaptive_sync_info *adaptive_sync = info_adaptive_sync;
> > +   const struct detailed_non_pixel *data = >data.other_data;
> > +   const struct detailed_data_monitor_range *range = >data.range;
> > +
> > +   if (data->type != EDID_DETAIL_MONITOR_RANGE)a
> 
> is_display_descriptor()

Will change to use is_display_descriptor()

> 
> > +   return;
> > +
> > +   /*
> > +* Check for flag range limits only. If flag == 1 then
> > +* no additional timing information provided.
> > +* Default GTF, GTF Secondary curve and CVT are not
> > +* supported
> > +*/
> > +   if (range->flags != 1)
> 
> Pls name the flags.

I dont see that we have any enum with the flag names, do you want me to define 
them looking
at EDID spec?

Manasi

> 
> > +   return;
> > +
> > +   adaptive_sync->min_vfreq = range->min_vfreq;
> > +   adaptive_sync->max_vfreq = range->max_vfreq;
> > +}
> > +
> > +static
> > +void drm_get_adaptive_sync_range(struct drm_connector *connector,
> > +const struct edid *edid)
> > +{
> > +   struct drm_display_info *info = >display_info;
> > +
> > +   if (!version_greater(edid, 1, 1))
> > +   return;
> > +
> > +   drm_for_each_detailed_block((u8 *)edid, get_adaptive_sync_range,
> > +   >adaptive_sync);
> > +
> > +   DRM_DEBUG_KMS("Adaptive Sync refresh rate range is %d Hz - %d Hz\n",
> > + info->adaptive_sync.min_vfreq,
> > + info->adaptive_sync.max_vfreq);
> > +}
> > +
> >  /* A connector has no EDID information, so we've got no EDID to compute 
> > quirks from. Reset
> >   * all of the values which would have been set from EDID
> >   */
> > @@ -4960,6 +5001,7 @@ drm_reset_display_info(struct drm_connector 
> > *connector)
> > memset(>hdmi, 0, sizeof(info->hdmi));
> >  
> > info->non_desktop = 0;
> > +   memset(>adaptive_sync, 0, sizeof(info->adaptive_sync));
> >  }
> >  
> >  u32 drm_add_display_info(struct drm_connector *connector, const struct 
> > edid *edid)
> > @@ -4975,6 +5017,8 @@ u32 drm_add_display_info(struct drm_connector 
> > *connector, const struct edid *edi
> >  
> > info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
> >  
> > +   drm_get_adaptive_sync_range(connector, edid);
> > +
> > DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
> >  
> > if (edid->revision < 3)
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 0df7a95ca5d9..2b22c0fa42c4 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -254,6 +254,23 @@ enum drm_panel_orientation {
> > DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
> >  };
> >  
> > +/**
> > + * struct drm_adaptive_sync_info - Panel's Adaptive Sync capabilities for
> > + * _display_info
> > + *
> > + * This struct is used to store a Panel's Adaptive Sync capabilities
> > + * as parsed from EDID's detailed monitor range descriptor block.
> > + *
> > + * @min_vfreq: This is the min supported refresh rate in Hz from
> > + * EDID's detailed monitor range.
> > + * @max_vfreq: This is the max supported refresh rate in Hz from
> > + * EDID's detailed monitor range
> > + */
> > +struct drm_adaptive_sync_info {
> > +   u8 min_vfreq;
> > +   u8 max_vfreq;
> > +};
> > +
> >  /*
> >   * 

Re: [PATCH 28/33] drm/panel-simple: Fix dotclock for Sharp LQ150X1LG11

2020-03-04 Thread Sam Ravnborg
Hi Peter.

> >>
> >> That said, I have no idea whatsoever if others have started using this
> >> panel entry. My guess is that it has zero users, but who can tell?
> > 
> > A quick grep shows that arch/arm/boot/dts/at91-nattis-2-natte-2.dts is
> > the only device tree that uses this panel in the upstream kernel.
> 
> This is our design, and what made us originally add the entry to simple
> panel, but as I said, we no longer need simple-panel support for it...

With the only upstream user using panel-lvds we should delete
it from panel-simple.
With all the features it does not belong in panel-simple anyway.
Peter - care to send a patch for that?

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] RFC: dma-buf: Add an API for importing and exporting sync files

2020-03-04 Thread Jason Ekstrand
On Wed, Mar 4, 2020 at 10:27 AM Jason Ekstrand  wrote:
>
> On Wed, Mar 4, 2020 at 2:34 AM Christian König  
> wrote:
> >
> > Am 03.03.20 um 20:10 schrieb Jason Ekstrand:
> > > On Thu, Feb 27, 2020 at 2:28 AM Christian König
> > >  wrote:
> > >> [SNIP]
> > >>> However, I'm not sure what the best way is to do garbage collection on
> > >>> that so that we don't get an impossibly list of fence arrays.
> > >> Exactly yes. That's also the reason why the dma_fence_chain container I
> > >> came up with for the sync timeline stuff has such a rather sophisticated
> > >> garbage collection.
> > >>
> > >> When some of the included fences signal you need to free up the
> > >> array/chain and make sure that the memory for the container can be 
> > >> reused.
> > > Currently (as of v2), I'm using dma_fence_array and being careful to
> > > not bother constructing one if there's only one fence in play.  Is
> > > this insufficient?  If so, maybe we should consider improving
> > > dma_fence_array.
> >
> > That still won't work correctly in all cases. See the problem is not
> > only optimization, but also avoiding situations where userspace can
> > abuse the interface to do nasty things.
> >
> > For example if userspace just calls that function in a loop you can
> > create a long chain of dma_fence_array objects.
> >
> > If that chain is then suddenly released the recursive dropping of
> > references can overwrite the kernel stack.
> >
> > For reference see what dance is necessary in the dma_fence_chain_release
> > function to avoid that:
> > > /* Manually unlink the chain as much as possible to avoid
> > > recursion
> > >  * and potential stack overflow.
> > >  */
> > > while ((prev = rcu_dereference_protected(chain->prev, true))) {
> > 
> >
> > It took me quite a while to figure out how to do this without causing
> > issues. But I don't see how this would be possible for dma_fence_array.
>
> Ah, I see the issue now!  It hadn't even occurred to me that userspace
> could use this to build up an infinite recursion chain.  That's nasty!
>  I'll give this some more thought and see if can come up with
> something clever.
>
> Here's one thought:  We could make dma_fence_array automatically
> collapse any arrays it references and instead directly reference their
> fences.  This way, no matter how much the client chains things, they
> will never get more than one dma_fence_array.  Of course, the
> difficulty here (answering my own question) comes if they ping-pong
> back-and-forth between something which constructs a dma_fence_array
> and something which constructs a dma_fence_chain to get
> array-of-chain-of-array-of-chain-of-...  More thought needed.

Answering my own questions again...  I think the
array-of-chain-of-array case is also solvable.

For array-of-chain, we can simply add all unsignaled dma_fences in the
chain to the array.  The array won't signal until all of them have
which is exactly the same behavior as if we'd added the chain itself.

For chain-of-array, we can add all unsignaled dma_fences in the array
to the same point in the chain.  There may be some fiddling with the
chain numbering required here but I think we can get it so the chain
won't signal until everything in the array has signaled and we get the
same behavior as if we'd added the dma_fence_array to the chain.

In both cases, we end up with either a single array or a single and
destruction doesn't require recursion.  Thoughts?

--Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 2/3] dt-bindings: display: panel: Add binding document for Elida KD35T133

2020-03-04 Thread Rob Herring
On Sat, 29 Feb 2020 16:15:05 +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner 
> 
> The KD35T133 is a 3.5" 320x480 DSI display used in the RK3326-based
> Odroid Go Advance handheld device.
> 
> Signed-off-by: Heiko Stuebner 
> Reviewed-by: Sam Ravnborg 
> ---
>  .../display/panel/elida,kd35t133.yaml | 49 +++
>  1 file changed, 49 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/elida,kd35t133.yaml
> 

Reviewed-by: Rob Herring 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 1/3] dt-bindings: Add vendor prefix for Elida

2020-03-04 Thread Rob Herring
On Sat, 29 Feb 2020 16:15:04 +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner 
> 
> Shenzen Elida Technology Co. Ltd. is a Chinese TFT manufacturer.
> 
> Signed-off-by: Heiko Stuebner 
> Acked-by: Sam Ravnborg 
> ---
> Hi Rob,
> 
> as can be seen on [0], Sam expects you to apply the vendor prefix
> to the main dt-tree.
> 
> Thanks
> Heiko
> 
> [0] http://lore.kernel.org/r/20200229125725.gc5...@ravnborg.org
> 
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Applied, thanks.

Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] RFC: dma-buf: Add an API for importing and exporting sync files

2020-03-04 Thread Jason Ekstrand
On Wed, Mar 4, 2020 at 2:34 AM Christian König  wrote:
>
> Am 03.03.20 um 20:10 schrieb Jason Ekstrand:
> > On Thu, Feb 27, 2020 at 2:28 AM Christian König
> >  wrote:
> >> [SNIP]
> >>> However, I'm not sure what the best way is to do garbage collection on
> >>> that so that we don't get an impossibly list of fence arrays.
> >> Exactly yes. That's also the reason why the dma_fence_chain container I
> >> came up with for the sync timeline stuff has such a rather sophisticated
> >> garbage collection.
> >>
> >> When some of the included fences signal you need to free up the
> >> array/chain and make sure that the memory for the container can be reused.
> > Currently (as of v2), I'm using dma_fence_array and being careful to
> > not bother constructing one if there's only one fence in play.  Is
> > this insufficient?  If so, maybe we should consider improving
> > dma_fence_array.
>
> That still won't work correctly in all cases. See the problem is not
> only optimization, but also avoiding situations where userspace can
> abuse the interface to do nasty things.
>
> For example if userspace just calls that function in a loop you can
> create a long chain of dma_fence_array objects.
>
> If that chain is then suddenly released the recursive dropping of
> references can overwrite the kernel stack.
>
> For reference see what dance is necessary in the dma_fence_chain_release
> function to avoid that:
> > /* Manually unlink the chain as much as possible to avoid
> > recursion
> >  * and potential stack overflow.
> >  */
> > while ((prev = rcu_dereference_protected(chain->prev, true))) {
> 
>
> It took me quite a while to figure out how to do this without causing
> issues. But I don't see how this would be possible for dma_fence_array.

Ah, I see the issue now!  It hadn't even occurred to me that userspace
could use this to build up an infinite recursion chain.  That's nasty!
 I'll give this some more thought and see if can come up with
something clever.

Here's one thought:  We could make dma_fence_array automatically
collapse any arrays it references and instead directly reference their
fences.  This way, no matter how much the client chains things, they
will never get more than one dma_fence_array.  Of course, the
difficulty here (answering my own question) comes if they ping-pong
back-and-forth between something which constructs a dma_fence_array
and something which constructs a dma_fence_chain to get
array-of-chain-of-array-of-chain-of-...  More thought needed.

> As far as I can see the only real option to implement this would be to
> change the dma_resv object container so that you can add fences without
> overriding existing ones.
>
> For shared fences that can be done relative easily, but I absolutely
> don't see how to do this for exclusive ones without a larger rework.

Fair enough.  Thanks for taking the time to explain the issue.  I'll
give this some more thought.

--Jason


> >>>(Note
> >>> the dma_resv has a lock that needs to be taken before adding an
> >>> exclusive fence, might be useful). Some code that does a thing like
> >>> this is __dma_resv_make_exclusive in
> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> >> Wanted to move that into dma_resv.c for quite a while since there are
> >> quite a few other cases where we need this.
> > I've roughly done that.  The primary difference is that my version
> > takes an optional additional fence to add to the array.  This makes it
> > a bit more complicated but I think I got it mostly right.
> >
> > I've also written userspace code which exercises this and it seems to
> > work.  Hopefully, that will give a better idea of what I'm trying to
> > accomplish.
>
> Yes, that is indeed a really nice to have feature.
>
> Regards,
> Christian.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v12 4/6] dt-bindings: display: mediatek: convert the document format from txt to yaml

2020-03-04 Thread Rob Herring
On Tue, Mar 03, 2020 at 01:27:20PM +0800, Jitao Shi wrote:
> Signed-off-by: Jitao Shi 
> ---
>  .../display/mediatek/mediatek,dpi.txt | 45 -
>  .../display/mediatek/mediatek,dpi.yaml| 92 +++
>  2 files changed, 92 insertions(+), 45 deletions(-)
>  delete mode 100644 
> Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt
>  create mode 100644 
> Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml

You need to run 'make dt_binding_check' on this and fix the errors.

[...]

> diff --git 
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml 
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
> new file mode 100644
> index ..eb2b0cb5eb5a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml
> @@ -0,0 +1,92 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/mediatek,dpi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: mediatek DPI Controller Device Tree Bindings
> +
> +maintainers:
> +  - CK Hu 
> +  - Jitao shi 
> +
> +description: |
> +  The Mediatek DPI function block is a sink of the display subsystem and
> +  provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a parallel
> +  output bus.
> +
> +properties:
> +  compatible:
> +enum:
> +  - mediatek,mt2701-dpi
> +  - mediatek,mt8173-dpi
> +  - mediatek,mt8183-dpi
> +
> +  reg:
> +maxItems: 1
> +
> +  interrupts:
> +maxItems: 1
> +
> +  clocks:
> +items:
> +  - description: Pixel Clock
> +  - description: Engine Clock
> +  - description: DPI PLL
> +
> +  clock-names:
> +items:
> +  - const: pixel
> +  - const: engine
> +  - const: pll
> +
> +  pinctrl-names:
> +items:
> +  - const: default
> +  - const: sleep

Doesn't match what you just added to the binding...

> +
> +  port:
> +type: object
> +description:
> +  Output port node with endpoint definitions as described in
> +  Documentation/devicetree/bindings/graph.txt. This port should be 
> connected
> +  to the input port of an attached HDMI or LVDS encoder chip.
> +
> +properties:
> +  pclk-sample:
> +  description: refer 
> Documentation/devicetree/bindings/media/video-interfaces.txt.

This is wrong in multiple ways. 'description' is missing indentation, so 
you are defining 'description' to be a property.

And 'pclk-sample' is not a property of 'port' node, but 'endpoint'.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +dpi0: dpi@1401d000 {
> +compatible = "mediatek,mt8173-dpi";
> +reg = <0 0x1401d000 0 0x1000>;
> +interrupts = ;
> +clocks = < CLK_MM_DPI_PIXEL>,
> + < CLK_MM_DPI_ENGINE>,
> + < CLK_APMIXED_TVDPLL>;

Examples are built now and you need to add includes for these defines.

> +clock-names = "pixel", "engine", "pll";
> +pinctrl-names = "default", "sleep";
> +pinctrl-0 = <_pin_func>;
> +pinctrl-1 = <_pin_idle>;
> +
> +port {
> +reg = <0>;

Drop 'reg'.

> +dpi0_out: endpoint {
> +pclk-sample = <0>;
> +remote-endpoint = <_in>;
> +};
> +};
> +};
> +
> +...
> -- 
> 2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] gpu: drm: context: Clean up documentation

2020-03-04 Thread Emil Velikov
On Mon, 3 Feb 2020 at 08:11, Benjamin Gaignard  wrote:
>
> Fix kernel doc comments to avoid warnings when compiling with W=1.
>
> Signed-off-by: Benjamin Gaignard 
> ---
>  drivers/gpu/drm/drm_context.c | 145 
> ++
>  1 file changed, 61 insertions(+), 84 deletions(-)
>
Since we're talking about legacy, aka user mode-setting code, I think
a wiser solution is to simply remove the documentation. It is _not_
something we should encourage people to read, let alone use.

Nit: prefix should be "drm:"

-Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/i915/dp: Add dpcd link_rate quirk for Apple 15" MBP 2017

2020-03-04 Thread Jani Nikula
On Sat, 29 Feb 2020, Mario Kleiner  wrote:
> This fixes a problem found on the MacBookPro 2017 Retina panel.
>
> The panel reports 10 bpc color depth in its EDID, and the
> firmware chooses link settings at boot which support enough
> bandwidth for 10 bpc (324000 kbit/sec = multiplier 0xc),
> but the DP_MAX_LINK_RATE dpcd register only reports
> 2.7 Gbps (multiplier value 0xa) as possible, in direct
> contradiction of what the firmware successfully set up.
>
> This restricts the panel to 8 bpc, not providing the full
> color depth of the panel.
>
> This patch adds a quirk specific to the MBP 2017 15" Retina
> panel to add the additiional 324000 kbps link rate during
> edp setup.
>
> Link to previous discussion of a different attempted fix
> with Ville and Jani:
>
> https://patchwork.kernel.org/patch/11325935/
>
> Signed-off-by: Mario Kleiner 
> Cc: Ville Syrjälä 
> Cc: Jani Nikula 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 2 ++
>  drivers/gpu/drm/i915/display/intel_dp.c | 7 +++
>  include/drm/drm_dp_helper.h | 7 +++
>  3 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 5a103e9b3c86..36a371c016cb 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1179,6 +1179,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = {
>   { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), 
> false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) },
>   /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */
>   { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, 
> BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) },
> + /* Apple MacBookPro 2017 15 inch eDP Retina panel reports too low 
> DP_MAX_LINK_RATE */
> + { OUI(0x00, 0x10, 0xfa), DEVICE_ID(101, 68, 21, 101, 98, 97), false, 
> BIT(DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS) },
>  };
>  
>  #undef OUI
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4074d83b1a5f..1f6bd659ad41 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -178,6 +178,13 @@ static void intel_dp_set_sink_rates(struct intel_dp 
> *intel_dp)
>   }
>  
>   intel_dp->num_sink_rates = i;
> +
> + if (drm_dp_has_quirk(_dp->desc,
> + DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS)) {
> + /* Needed for Apple MBP 2017, 15 inch eDP Retina panel */
> + intel_dp->sink_rates[i] = 324000;
> + intel_dp->num_sink_rates++;
> + }

If we can isolate the quirk to this one function, I'll be happy. \o/

However, even if this might work on said machine, I'd prefer it if we
didn't give the idea that you could just append a value in sink_rates
(it must be sorted). How about putting something like this in the
beginning of the function, to be a bit more explicit:

if (quirk) {
static const int quirk_rates[] = { 162000, 27, 324000 };

memcpy(intel_dp->sink_rates, quirk_rates, sizeof(quirk_rates));
intel_dp->num_sink_rates = ARRAY_SIZE(quirk_rates);

return;
}

BR,
Jani.

>  }
>  
>  /* Get length of rates array potentially limited by max_rate. */
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 262faf9e5e94..4b86a1f2a559 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1532,6 +1532,13 @@ enum drm_dp_quirk {
>* The DSC caps can be read from the physical aux instead.
>*/
>   DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD,
> + /**
> +  * @DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS:
> +  *
> +  * The device supports a link rate of 3.24 Gbps (multiplier 0xc) despite
> +  * the DP_MAX_LINK_RATE register reporting a lower max multiplier.
> +  */
> + DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS,
>  };
>  
>  /**

-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH resend 1/2] drm/i915: panel: Use intel_panel_compute_brightness() from pwm_setup_backlight()

2020-03-04 Thread Jani Nikula
On Tue, 03 Mar 2020, Hans de Goede  wrote:
> Hi All,
>
> On 2/21/20 6:29 PM, Hans de Goede wrote:
>> Use intel_panel_compute_brightness() from pwm_setup_backlight() so that
>> we correctly take i915_modparams.invert_brightness and/or
>> QUIRK_INVERT_BRIGHTNESS into account when setting + getting the initial
>> brightness value.
>> 
>> Signed-off-by: Hans de Goede 
>
> ping? Any chance I can get a review from someone on this series?
>
> Both patches are pretty trivial really...

For both,

Reviewed-by: Jani Nikula 

And sad trombone, I was hoping I could nuke the whole module parameter
one of these days. It used to be something associated with gen4 only.

BR,
Jani.


>
> Regards,
>
> Hans
>
>
>
>> ---
>>   drivers/gpu/drm/i915/display/intel_panel.c | 18 +++---
>>   1 file changed, 11 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c 
>> b/drivers/gpu/drm/i915/display/intel_panel.c
>> index 7b3ec6eb3382..9ebee7d93414 100644
>> --- a/drivers/gpu/drm/i915/display/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
>> @@ -1843,6 +1843,7 @@ static int pwm_setup_backlight(struct intel_connector 
>> *connector,
>>  struct drm_i915_private *dev_priv = to_i915(dev);
>>  struct intel_panel *panel = >panel;
>>  const char *desc;
>> +u32 level, ns;
>>  int retval;
>>   
>>  /* Get the right PWM chip for DSI backlight according to VBT */
>> @@ -1866,8 +1867,12 @@ static int pwm_setup_backlight(struct intel_connector 
>> *connector,
>>   */
>>  pwm_apply_args(panel->backlight.pwm);
>>   
>> -retval = pwm_config(panel->backlight.pwm, CRC_PMIC_PWM_PERIOD_NS,
>> -CRC_PMIC_PWM_PERIOD_NS);
>> +panel->backlight.min = 0; /* 0% */
>> +panel->backlight.max = 100; /* 100% */
>> +level = intel_panel_compute_brightness(connector, 100);
>> +ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100);
>> +
>> +retval = pwm_config(panel->backlight.pwm, ns, CRC_PMIC_PWM_PERIOD_NS);
>>  if (retval < 0) {
>>  DRM_ERROR("Failed to configure the pwm chip\n");
>>  pwm_put(panel->backlight.pwm);
>> @@ -1875,11 +1880,10 @@ static int pwm_setup_backlight(struct 
>> intel_connector *connector,
>>  return retval;
>>  }
>>   
>> -panel->backlight.min = 0; /* 0% */
>> -panel->backlight.max = 100; /* 100% */
>> -panel->backlight.level = DIV_ROUND_UP(
>> - pwm_get_duty_cycle(panel->backlight.pwm) * 100,
>> - CRC_PMIC_PWM_PERIOD_NS);
>> +level = DIV_ROUND_UP(pwm_get_duty_cycle(panel->backlight.pwm) * 100,
>> + CRC_PMIC_PWM_PERIOD_NS);
>> +panel->backlight.level =
>> +intel_panel_compute_brightness(connector, level);
>>  panel->backlight.enabled = panel->backlight.level != 0;
>>   
>>  DRM_INFO("Using %s PWM for LCD backlight control\n", desc);
>> 
>

-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v12 2/6] dt-bindings: display: mediatek: control dpi pins mode to avoid leakage

2020-03-04 Thread Rob Herring
On Tue, Mar 03, 2020 at 01:27:18PM +0800, Jitao Shi wrote:
> Add property "pinctrl-names" to swap pin mode between gpio and dpi mode. Set
> the dpi pins to gpio mode and output-low to avoid leakage current when dpi
> disabled.
> 
> Signed-off-by: Jitao Shi 
> ---
>  .../devicetree/bindings/display/mediatek/mediatek,dpi.txt  | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt 
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt
> index 58914cf681b8..77ca32a32399 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt
> @@ -17,6 +17,10 @@ Required properties:
>Documentation/devicetree/bindings/graph.txt. This port should be connected
>to the input port of an attached HDMI or LVDS encoder chip.
>  
> +Optional properties:
> +- pinctrl-names: Contain "gpiomode" and "dpimode".

Doesn't match the example.

> +  pinctrl-names see 
> Documentation/devicetree/bindings/pinctrlpinctrl-bindings.txt
> +
>  Example:
>  
>  dpi0: dpi@1401d000 {
> @@ -27,6 +31,9 @@ dpi0: dpi@1401d000 {
>< CLK_MM_DPI_ENGINE>,
>< CLK_APMIXED_TVDPLL>;
>   clock-names = "pixel", "engine", "pll";
> + pinctrl-names = "active", "idle";
> + pinctrl-0 = <_pin_func>;
> + pinctrl-1 = <_pin_idle>;
>  
>   port {
>   dpi0_out: endpoint {
> -- 
> 2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2] drm/simple-kms: Fix documentation for drm_simple_encoder_init()

2020-03-04 Thread Thomas Zimmermann
Brings the documentation of drm_simple_encoder_init() in sync with the
function's signature. Also add a paragraph clarifying the management of
the encoder's memory.

v2:
* document memory management

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Daniel Vetter 
Fixes: 63170ac6f2e8 ("drm/simple-kms: Add drm_simple_encoder_{init,create}()")
Cc: Sam Ravnborg 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_simple_kms_helper.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c 
b/drivers/gpu/drm/drm_simple_kms_helper.c
index 5a2abe2dea3e..74946690aba4 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -38,9 +38,10 @@ static const struct drm_encoder_funcs 
drm_simple_encoder_funcs_cleanup = {
 };

 /**
- * drm_simple_encoder_init - Initialize a preallocated encoder
+ * drm_simple_encoder_init - Initialize a preallocated encoder with
+ *   basic functionality.
  * @dev: drm device
- * @funcs: callbacks for this encoder
+ * @encoder: the encoder to initialize
  * @encoder_type: user visible type of the encoder
  *
  * Initialises a preallocated encoder that has no further functionality.
@@ -48,6 +49,15 @@ static const struct drm_encoder_funcs 
drm_simple_encoder_funcs_cleanup = {
  * The encoder will be cleaned up automatically as part of the mode-setting
  * cleanup.
  *
+ * The caller of drm_simple_encoder_init() is responsible for freeing
+ * the encoder's memory after the encoder has been cleaned up. At the
+ * moment this only works reliably if the encoder data structure is
+ * stored in the device structure. Free the encoder's memory as part of
+ * the device release function.
+ *
+ * FIXME: Later improvements to DRM's resource management may allow for
+ *an automated kfree() of the encoder's memory.
+ *
  * Returns:
  * Zero on success, error code on failure.
  */
--
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 33/33] drm/panel-simple: Fix dotclock for LG ACX467AKM-7

2020-03-04 Thread Linus Walleij
On Wed, Mar 4, 2020 at 2:17 PM Jonathan Marek  wrote:

[hs_rate / lp_rate]

> The msm DSI driver does predate the addition of those fields and doesn't
> use them at all.

I think it would be benficient to switch to these fields, because then
the .clock field (dot/pixelclock) is not abused to contain what I guess
is the desires hs_rate.

> Seems like it would be a bit of a hack too, since the frequency we want
> to use is not the "real limits of the hardware"..

That just means "clock it as high as the panel can take".

Normally that would come from the vendor datasheet of
the panel.

If you don't have the datasheet, whatever you use in the vendor
tree is fine, I suppose what is currently in .clock is fine.

Yours,
Linus Walleij
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] MAINTAINERS: adjust to reservation.h renaming

2020-03-04 Thread Christian König

Am 04.03.20 um 13:07 schrieb Lukas Bulwahn:

Commit 52791eeec1d9 ("dma-buf: rename reservation_object to dma_resv")
renamed include/linux/reservation.h to include/linux/dma-resv.h, but
missed the reference in the MAINTAINERS entry.

Since then, ./scripts/get_maintainer.pl --self-test complains:

   warning: no file matches F: include/linux/reservation.h

Adjust the DMA BUFFER SHARING FRAMEWORK entry in MAINTAINERS.

Co-developed-by: Sebastian Duda 
Signed-off-by: Sebastian Duda 
Signed-off-by: Lukas Bulwahn 


Reviewed-by: Christian König 


---
Christian, please pick this patch.
applies cleanly on current master and next-20200303

  MAINTAINERS | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6158a143a13e..3d6cb2789c9e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5022,7 +5022,7 @@ L:dri-devel@lists.freedesktop.org
  L:linaro-mm-...@lists.linaro.org (moderated for non-subscribers)
  F:drivers/dma-buf/
  F:include/linux/dma-buf*
-F: include/linux/reservation.h
+F: include/linux/dma-resv.h
  F:include/linux/*fence.h
  F:Documentation/driver-api/dma-buf.rst
  K:dma_(buf|fence|resv)


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 25/89] reset: simple: Add reset callback

2020-03-04 Thread Philipp Zabel
Hi Maxime,

On Mon, 2020-02-24 at 10:06 +0100, Maxime Ripard wrote:
> The reset-simple code lacks a reset callback that is still pretty easy to
> implement. The only real thing to consider is the delay needed for a device
> to be reset, so let's expose that as part of the reset-simple driver data.
>
> Cc: Philipp Zabel 
> Signed-off-by: Maxime Ripard 

This shoulod be done in such a way that simple reset drivers which do
not set the reset delay continue to return -ENOTSUPP from
reset_control_reset().

> ---
>  drivers/reset/reset-simple.c   | 21 +
>  include/linux/reset/reset-simple.h |  4 
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index c854aa351640..7a8c56512ae9 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -11,6 +11,7 @@
>   * Maxime Ripard 
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -63,6 +64,25 @@ static int reset_simple_deassert(struct 
> reset_controller_dev *rcdev,
>   return reset_simple_update(rcdev, id, false);
>  }
>  
> +static int reset_simple_reset(struct reset_controller_dev *rcdev,
> +unsigned long id)
> +{
> + struct reset_simple_data *data = to_reset_simple_data(rcdev);
> + int ret;

You could just return -ENOTSUPP here if data->reset_ms == 0.

> + ret = reset_simple_assert(rcdev, id);
> + if (ret)
> + return ret;
> +
> + mdelay(data->reset_ms);

Have you considered specifying the delay in microseconds instead?
That would allow to use usleep_range() for shorter delays.

> + ret = reset_simple_deassert(rcdev, id);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
>  static int reset_simple_status(struct reset_controller_dev *rcdev,
>  unsigned long id)
>  {
> @@ -80,6 +100,7 @@ static int reset_simple_status(struct reset_controller_dev 
> *rcdev,
>  const struct reset_control_ops reset_simple_ops = {
>   .assert = reset_simple_assert,
>   .deassert   = reset_simple_deassert,
> + .reset  = reset_simple_reset,
>   .status = reset_simple_status,
>  };
>  EXPORT_SYMBOL_GPL(reset_simple_ops);
> diff --git a/include/linux/reset/reset-simple.h 
> b/include/linux/reset/reset-simple.h
> index 08ccb25a55e6..a5887f6cbe50 100644
> --- a/include/linux/reset/reset-simple.h
> +++ b/include/linux/reset/reset-simple.h
> @@ -27,6 +27,9 @@
>   * @status_active_low: if true, bits read back as cleared while the reset is
>   * asserted. Otherwise, bits read back as set while the
>   * reset is asserted.
> + * @reset_ms: Minimum delay in milliseconds needed that needs to be
> + *waited for between an assert and a deassert to reset the
> + *device.

If multiple consumers with different delay requirements are connected to
this reset controllers, this must the largest minimum delay. Could you
add mention for this in the comment?

regards
Philipp
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 0/5] drm/lima: add error debug functionality

2020-03-04 Thread Andreas Baierl
I could successfully use the output with 
https://gitlab.freedesktop.org/lima/lima.dump


So you can add my
Tested-by: Andreas Baierl 


Am 22.02.2020 um 03:42 schrieb Qiang Yu:

Save task error state when it fail and export to user by
sysfs as a binary file which can be dumped and replayed
by lima_dump tool at:
https://gitlab.freedesktop.org/lima/lima_dump

Qiang Yu (5):
   drm/lima: save process info for debug usage
   drm/lima: add max_error_tasks module parameter
   drm/lima: save task info dump when task fail
   drm/lima: add error sysfs to export error task dump
   drm/lima: add LIMA_BO_FLAG_FORCE_VA

  drivers/gpu/drm/lima/lima_ctx.c|   3 +
  drivers/gpu/drm/lima/lima_ctx.h|   5 ++
  drivers/gpu/drm/lima/lima_device.c |  13 +++
  drivers/gpu/drm/lima/lima_device.h |   8 ++
  drivers/gpu/drm/lima/lima_drv.c| 123 +--
  drivers/gpu/drm/lima/lima_drv.h|   1 +
  drivers/gpu/drm/lima/lima_dump.h   |  77 +
  drivers/gpu/drm/lima/lima_gem.c|   7 +-
  drivers/gpu/drm/lima/lima_gem.h|   4 +-
  drivers/gpu/drm/lima/lima_sched.c  | 128 +
  drivers/gpu/drm/lima/lima_sched.h  |   7 ++
  drivers/gpu/drm/lima/lima_vm.c |  13 ++-
  include/uapi/drm/lima_drm.h|   9 +-
  13 files changed, 385 insertions(+), 13 deletions(-)
  create mode 100644 drivers/gpu/drm/lima/lima_dump.h



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] gpu: drm: context: Clean up documentation

2020-03-04 Thread Benjamin Gaignard
Le lun. 3 févr. 2020 à 09:11, Benjamin Gaignard
 a écrit :
>
> Fix kernel doc comments to avoid warnings when compiling with W=1.

gentle ping.

Benjamin

>
> Signed-off-by: Benjamin Gaignard 
> ---
>  drivers/gpu/drm/drm_context.c | 145 
> ++
>  1 file changed, 61 insertions(+), 84 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c
> index 1f802d8e5681..54e64d612a2b 100644
> --- a/drivers/gpu/drm/drm_context.c
> +++ b/drivers/gpu/drm/drm_context.c
> @@ -43,15 +43,11 @@ struct drm_ctx_list {
> struct drm_file *tag;
>  };
>
> -/**/
> -/** \name Context bitmap support */
> -/*@{*/
> -
>  /**
> - * Free a handle from the context bitmap.
> + * drm_legacy_ctxbitmap_free() - Free a handle from the context bitmap.
>   *
> - * \param dev DRM device.
> - * \param ctx_handle context handle.
> + * @dev: DRM device.
> + * @ctx_handle: context handle.
>   *
>   * Clears the bit specified by \p ctx_handle in drm_device::ctx_bitmap and 
> the entry
>   * in drm_device::ctx_idr, while holding the drm_device::struct_mutex
> @@ -69,10 +65,10 @@ void drm_legacy_ctxbitmap_free(struct drm_device * dev, 
> int ctx_handle)
>  }
>
>  /**
> - * Context bitmap allocation.
> + * drm_legacy_ctxbitmap_next() - Context bitmap allocation.
>   *
> - * \param dev DRM device.
> - * \return (non-negative) context handle on success or a negative number on 
> failure.
> + * @dev: DRM device.
> + * Return: (non-negative) context handle on success or a negative number on 
> failure.
>   *
>   * Allocate a new idr from drm_device::ctx_idr while holding the
>   * drm_device::struct_mutex lock.
> @@ -89,9 +85,9 @@ static int drm_legacy_ctxbitmap_next(struct drm_device * 
> dev)
>  }
>
>  /**
> - * Context bitmap initialization.
> + * drm_legacy_ctxbitmap_init() - Context bitmap initialization.
>   *
> - * \param dev DRM device.
> + * @dev: DRM device.
>   *
>   * Initialise the drm_device::ctx_idr
>   */
> @@ -105,9 +101,9 @@ void drm_legacy_ctxbitmap_init(struct drm_device * dev)
>  }
>
>  /**
> - * Context bitmap cleanup.
> + * drm_legacy_ctxbitmap_cleanup() - bitmap cleanup.
>   *
> - * \param dev DRM device.
> + * @dev: DRM device.
>   *
>   * Free all idr members using drm_ctx_sarea_free helper function
>   * while holding the drm_device::struct_mutex lock.
> @@ -157,20 +153,13 @@ void drm_legacy_ctxbitmap_flush(struct drm_device *dev, 
> struct drm_file *file)
> mutex_unlock(>ctxlist_mutex);
>  }
>
> -/*@}*/
> -
> -/**/
> -/** \name Per Context SAREA Support */
> -/*@{*/
> -
>  /**
> - * Get per-context SAREA.
> + * drm_legacy_getsareactx() - Get per-context SAREA.
>   *
> - * \param inode device inode.
> - * \param file_priv DRM file private.
> - * \param cmd command.
> - * \param arg user argument pointing to a drm_ctx_priv_map structure.
> - * \return zero on success or a negative number on failure.
> + * @dev: DRM device to operate on
> + * @data: request data
> + * @file_priv: DRM file private.
> + * Return:  zero on success or a negative number on failure.
>   *
>   * Gets the map from drm_device::ctx_idr with the handle specified and
>   * returns its handle.
> @@ -212,13 +201,12 @@ int drm_legacy_getsareactx(struct drm_device *dev, void 
> *data,
>  }
>
>  /**
> - * Set per-context SAREA.
> + * drm_legacy_setsareactx() - Set per-context SAREA.
>   *
> - * \param inode device inode.
> - * \param file_priv DRM file private.
> - * \param cmd command.
> - * \param arg user argument pointing to a drm_ctx_priv_map structure.
> - * \return zero on success or a negative number on failure.
> + * @dev: DRM device to operate on
> + * @data: request data
> + * @file_priv: DRM file private.
> + * Return: zero on success or a negative number on failure.
>   *
>   * Searches the mapping specified in \p arg and update the entry in
>   * drm_device::ctx_idr with it.
> @@ -257,19 +245,13 @@ int drm_legacy_setsareactx(struct drm_device *dev, void 
> *data,
> return 0;
>  }
>
> -/*@}*/
> -
> -/**/
> -/** \name The actual DRM context handling routines */
> -/*@{*/
> -
>  /**
> - * Switch context.
> + * drm_context_switch() - Switch context.
>   *
> - * \param dev DRM device.
> - * \param old old context handle.
> - * \param new new context handle.
> - * \return zero on success or a negative number on failure.
> + * @dev: DRM device.
> + * @old: old context handle.
> + * @new: new context handle.
> + * Return: zero on success or a negative number on failure.
>   *
>   * Attempt to set drm_device::context_flag.
>   */
> @@ -291,11 +273,12 @@ static int drm_context_switch(struct drm_device * dev, 
> int old, int new)
>  }
>
>  /**
> - * Complete context switch.
> + * drm_context_switch_complete() - Complete context switch.
>   *
> - * \param dev DRM device.
> - * 

[PATCH v5 09/11] drm/meson: venc: add support for YUV420 setup

2020-03-04 Thread Neil Armstrong
This patch adds encoding support for the YUV420 output from the
Amlogic Meson SoCs Video Processing Unit to the HDMI Controller.

The YUV420 is obtained by generating a YUV444 pixel stream like
the classic HDMI display modes, but then the Video Encoder output
can be configured to down-sample the YUV444 pixel stream to a YUV420
stream.

In addition if pixel stream down-sampling, the Y Cb Cr components must
also be mapped differently to align with the HDMI2.0 specifications.

Signed-off-by: Neil Armstrong 
Reviewed-by: Jernej Škrabec 
---
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 3 ++-
 drivers/gpu/drm/meson/meson_venc.c| 8 +---
 drivers/gpu/drm/meson/meson_venc.h| 2 ++
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c 
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 72c118142eaf..b2105c0fe205 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -752,7 +752,8 @@ static void meson_venc_hdmi_encoder_mode_set(struct 
drm_bridge *bridge,
DRM_DEBUG_DRIVER("\"%s\" vic %d\n", mode->name, vic);
 
/* VENC + VENC-DVI Mode setup */
-   meson_venc_hdmi_mode_set(priv, vic, mode);
+   meson_venc_hdmi_mode_set(priv, vic, ycrcb_map, false,
+VPU_HDMI_OUTPUT_CBYCR);
 
/* VCLK Set clock */
dw_hdmi_set_vclk(dw_hdmi, mode);
diff --git a/drivers/gpu/drm/meson/meson_venc.c 
b/drivers/gpu/drm/meson/meson_venc.c
index a9ab78970bfe..f93c725b6f02 100644
--- a/drivers/gpu/drm/meson/meson_venc.c
+++ b/drivers/gpu/drm/meson/meson_venc.c
@@ -946,6 +946,8 @@ bool meson_venc_hdmi_venc_repeat(int vic)
 EXPORT_SYMBOL_GPL(meson_venc_hdmi_venc_repeat);
 
 void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic,
+ unsigned int ycrcb_map,
+ bool yuv420_mode,
  const struct drm_display_mode *mode)
 {
union meson_hdmi_venc_mode *vmode = NULL;
@@ -1528,14 +1530,14 @@ void meson_venc_hdmi_mode_set(struct meson_drm *priv, 
int vic,
if (mode->flags & DRM_MODE_FLAG_PVSYNC)
reg |= VPU_HDMI_INV_VSYNC;
 
-   /* Output data format: CbYCr */
-   reg |= VPU_HDMI_OUTPUT_CBYCR;
+   /* Output data format */
+   reg |= ycrcb_map;
 
/*
 * Write rate to the async FIFO between VENC and HDMI.
 * One write every 2 wr_clk.
 */
-   if (venc_repeat)
+   if (venc_repeat || yuv420_mode)
reg |= VPU_HDMI_WR_RATE(2);
 
/*
diff --git a/drivers/gpu/drm/meson/meson_venc.h 
b/drivers/gpu/drm/meson/meson_venc.h
index 1abdcbdf51c0..9138255ffc9e 100644
--- a/drivers/gpu/drm/meson/meson_venc.h
+++ b/drivers/gpu/drm/meson/meson_venc.h
@@ -60,6 +60,8 @@ extern struct meson_cvbs_enci_mode meson_cvbs_enci_ntsc;
 void meson_venci_cvbs_mode_set(struct meson_drm *priv,
   struct meson_cvbs_enci_mode *mode);
 void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic,
+ unsigned int ycrcb_map,
+ bool yuv420_mode,
  const struct drm_display_mode *mode);
 unsigned int meson_venci_get_field(struct meson_drm *priv);
 
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 11/11] drm/meson: Add YUV420 output support

2020-03-04 Thread Neil Armstrong
This patch adds support for the YUV420 output from the Amlogic Meson SoCs
Video Processing Unit to the HDMI Controller.

The YUV420 is obtained by generating a YUV444 pixel stream like
the classic HDMI display modes, but then the Video Encoder output
can be configured to down-sample the YUV444 pixel stream to a YUV420
stream.
In addition if pixel stream down-sampling, the Y Cb Cr components must
also be mapped differently to align with the HDMI2.0 specifications.

This mode needs a different clock generation scheme since the TMDS PHY
clock must match the 10x ratio with the YUV420 pixel clock, but
the video encoder must run at 2x the pixel clock.

This patch enables the bridge bus format negociation, and handles
the YUV420 case if selected by the negociation.

Signed-off-by: Neil Armstrong 
Reviewed-by: Jernej Škrabec 
---
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 91 ---
 1 file changed, 70 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c 
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index b5b0d45eb314..e8c94915a4fc 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -150,6 +150,7 @@ struct meson_dw_hdmi {
struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
+   unsigned long output_bus_fmt;
 };
 #define encoder_to_meson_dw_hdmi(x) \
container_of(x, struct meson_dw_hdmi, encoder)
@@ -301,6 +302,10 @@ static void meson_hdmi_phy_setup_mode(struct meson_dw_hdmi 
*dw_hdmi,
struct meson_drm *priv = dw_hdmi->priv;
unsigned int pixel_clock = mode->clock;
 
+   /* For 420, pixel clock is half unlike venc clock */
+   if (dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
+   pixel_clock /= 2;
+
if (dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxm-dw-hdmi")) {
if (pixel_clock >= 371250) {
@@ -383,6 +388,10 @@ static void dw_hdmi_set_vclk(struct meson_dw_hdmi *dw_hdmi,
 
vclk_freq = mode->clock;
 
+   /* For 420, pixel clock is half unlike venc clock */
+   if (dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
+   vclk_freq /= 2;
+
/* TMDS clock is pixel_clock * 10 */
phy_freq = vclk_freq * 10;
 
@@ -392,13 +401,16 @@ static void dw_hdmi_set_vclk(struct meson_dw_hdmi 
*dw_hdmi,
return;
}
 
+   /* 480i/576i needs global pixel doubling */
if (mode->flags & DRM_MODE_FLAG_DBLCLK)
vclk_freq *= 2;
 
venc_freq = vclk_freq;
hdmi_freq = vclk_freq;
 
-   if (meson_venc_hdmi_venc_repeat(vic))
+   /* VENC double pixels for 1080i, 720p and YUV420 modes */
+   if (meson_venc_hdmi_venc_repeat(vic) ||
+   dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
venc_freq *= 2;
 
vclk_freq = max(venc_freq, hdmi_freq);
@@ -445,8 +457,9 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void 
*data,
/* Enable normal output to PHY */
dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_BIST_CNTL, BIT(12));
 
-   /* TMDS pattern setup (TOFIX Handle the YUV420 case) */
-   if (mode->clock > 34) {
+   /* TMDS pattern setup */
+   if (mode->clock > 34 &&
+   dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_YUV8_1X24) {
dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_01,
  0);
dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_23,
@@ -621,6 +634,7 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
   const struct drm_display_mode *mode)
 {
struct meson_drm *priv = connector->dev->dev_private;
+   bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported;
unsigned int phy_freq;
unsigned int vclk_freq;
unsigned int venc_freq;
@@ -630,9 +644,11 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
 
DRM_DEBUG_DRIVER("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
 
-   /* If sink max TMDS clock, we reject the mode */
+   /* If sink does not support 540MHz, reject the non-420 HDMI2 modes */
if (connector->display_info.max_tmds_clock &&
-   mode->clock > connector->display_info.max_tmds_clock)
+   mode->clock > connector->display_info.max_tmds_clock &&
+   !drm_mode_is_420_only(>display_info, mode) &&
+   !drm_mode_is_420_also(>display_info, mode))
return MODE_BAD;
 
/* Check against non-VIC supported modes */
@@ -648,6 +664,12 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
 
vclk_freq = mode->clock;
 
+   /* For 420, pixel clock is half unlike venc clock */
+   if (drm_mode_is_420_only(>display_info, mode) ||
+   (!is_hdmi2_sink &&
+drm_mode_is_420_also(>display_info, mode)))
+   

[PATCH v5 04/11] drm/bridge: synopsys: dw-hdmi: add bus format negociation

2020-03-04 Thread Neil Armstrong
Add the atomic_get_output_bus_fmts, atomic_get_input_bus_fmts to negociate
the possible output and input formats for the current mode and monitor,
and use the negotiated formats in a basic atomic_check callback.

Signed-off-by: Neil Armstrong 
Reviewed-by: Boris Brezillon 
Reviewed-by: Jernej Škrabec 
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 281 +-
 1 file changed, 277 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 9f2918898f60..de19e8993e1d 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2097,11 +2097,10 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct 
drm_display_mode *mode)
hdmi->hdmi_data.video_mode.mpixelrepetitionoutput = 0;
hdmi->hdmi_data.video_mode.mpixelrepetitioninput = 0;
 
-   /* TOFIX: Get input format from plat data or fallback to RGB888 */
if (hdmi->plat_data->input_bus_format)
hdmi->hdmi_data.enc_in_bus_format =
hdmi->plat_data->input_bus_format;
-   else
+   else if (hdmi->hdmi_data.enc_in_bus_format == MEDIA_BUS_FMT_FIXED)
hdmi->hdmi_data.enc_in_bus_format = MEDIA_BUS_FMT_RGB888_1X24;
 
/* TOFIX: Get input encoding from plat data or fallback to none */
@@ -2111,8 +2110,8 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct 
drm_display_mode *mode)
else
hdmi->hdmi_data.enc_in_encoding = V4L2_YCBCR_ENC_DEFAULT;
 
-   /* TOFIX: Default to RGB888 output format */
-   hdmi->hdmi_data.enc_out_bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+   if (hdmi->hdmi_data.enc_out_bus_format == MEDIA_BUS_FMT_FIXED)
+   hdmi->hdmi_data.enc_out_bus_format = MEDIA_BUS_FMT_RGB888_1X24;
 
hdmi->hdmi_data.pix_repet_factor = 0;
hdmi->hdmi_data.hdcp_enable = 0;
@@ -2390,6 +2389,277 @@ static const struct drm_connector_helper_funcs 
dw_hdmi_connector_helper_funcs =
.atomic_check = dw_hdmi_connector_atomic_check,
 };
 
+/*
+ * Possible output formats :
+ * - MEDIA_BUS_FMT_UYYVYY16_0_5X48,
+ * - MEDIA_BUS_FMT_UYYVYY12_0_5X36,
+ * - MEDIA_BUS_FMT_UYYVYY10_0_5X30,
+ * - MEDIA_BUS_FMT_UYYVYY8_0_5X24,
+ * - MEDIA_BUS_FMT_YUV16_1X48,
+ * - MEDIA_BUS_FMT_RGB161616_1X48,
+ * - MEDIA_BUS_FMT_UYVY12_1X24,
+ * - MEDIA_BUS_FMT_YUV12_1X36,
+ * - MEDIA_BUS_FMT_RGB121212_1X36,
+ * - MEDIA_BUS_FMT_UYVY10_1X20,
+ * - MEDIA_BUS_FMT_YUV10_1X30,
+ * - MEDIA_BUS_FMT_RGB101010_1X30,
+ * - MEDIA_BUS_FMT_UYVY8_1X16,
+ * - MEDIA_BUS_FMT_YUV8_1X24,
+ * - MEDIA_BUS_FMT_RGB888_1X24,
+ */
+
+/* Can return a maximum of 11 possible output formats for a mode/connector */
+#define MAX_OUTPUT_SEL_FORMATS 11
+
+static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge 
*bridge,
+   struct drm_bridge_state *bridge_state,
+   struct drm_crtc_state *crtc_state,
+   struct drm_connector_state *conn_state,
+   unsigned int *num_output_fmts)
+{
+   struct drm_connector *conn = conn_state->connector;
+   struct drm_display_info *info = >display_info;
+   struct drm_display_mode *mode = _state->mode;
+   u8 max_bpc = conn_state->max_requested_bpc;
+   bool is_hdmi2_sink = info->hdmi.scdc.supported ||
+(info->color_formats & DRM_COLOR_FORMAT_YCRCB420);
+   u32 *output_fmts;
+   unsigned int i = 0;
+
+   *num_output_fmts = 0;
+
+   output_fmts = kcalloc(MAX_OUTPUT_SEL_FORMATS, sizeof(*output_fmts),
+ GFP_KERNEL);
+   if (!output_fmts)
+   return NULL;
+
+   /* If dw-hdmi is the only bridge, avoid negociating with ourselves */
+   if (list_is_singular(>encoder->bridge_chain)) {
+   *num_output_fmts = 1;
+   output_fmts[0] = MEDIA_BUS_FMT_FIXED;
+
+   return output_fmts;
+   }
+
+   /*
+* If the current mode enforces 4:2:0, force the output but format
+* to 4:2:0 and do not add the YUV422/444/RGB formats
+*/
+   if (conn->ycbcr_420_allowed &&
+   (drm_mode_is_420_only(info, mode) ||
+(is_hdmi2_sink && drm_mode_is_420_also(info, mode {
+
+   /* Order bus formats from 16bit to 8bit if supported */
+   if (max_bpc >= 16 && info->bpc == 16 &&
+   (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_48))
+   output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY16_0_5X48;
+
+   if (max_bpc >= 12 && info->bpc >= 12 &&
+   (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_36))
+   output_fmts[i++] = MEDIA_BUS_FMT_UYYVYY12_0_5X36;
+
+   if (max_bpc >= 10 && info->bpc >= 10 &&
+   (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_30))
+ 

[PATCH v5 05/11] drm/bridge: synopsys: dw-hdmi: allow ycbcr420 modes for >= 0x200a

2020-03-04 Thread Neil Armstrong
Now the DW-HDMI Controller supports the HDMI2.0 modes, enable support
for these modes in the connector if the platform supports them.
We limit these modes to DW-HDMI IP version >= 0x200a which
are designed to support HDMI2.0 display modes.

Signed-off-by: Neil Armstrong 
Reviewed-by: Andrzej Hajda 
Reviewed-by: Laurent Pinchart 
Reviewed-by: Jernej Škrabec 
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 ++
 include/drm/bridge/dw_hdmi.h  | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index de19e8993e1d..f85c15ad8486 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -3252,6 +3252,12 @@ __dw_hdmi_probe(struct platform_device *pdev,
hdmi->bridge.of_node = pdev->dev.of_node;
 #endif
 
+   if (hdmi->version >= 0x200a)
+   hdmi->connector.ycbcr_420_allowed =
+   hdmi->plat_data->ycbcr_420_allowed;
+   else
+   hdmi->connector.ycbcr_420_allowed = false;
+
memset(, 0, sizeof(pdevinfo));
pdevinfo.parent = dev;
pdevinfo.id = PLATFORM_DEVID_AUTO;
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 9d4d5cc47969..0b34a12c4a1c 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -129,6 +129,7 @@ struct dw_hdmi_plat_data {
unsigned long input_bus_format;
unsigned long input_bus_encoding;
bool use_drm_infoframe;
+   bool ycbcr_420_allowed;
 
/* Vendor PHY support */
const struct dw_hdmi_phy_ops *phy_ops;
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 03/11] drm/bridge: dw-hdmi: Plug atomic state hooks to the default implementation

2020-03-04 Thread Neil Armstrong
Add atomic_duplicate_state/atomic_destroy_state/atomic_reset bridge
funcs to allow setup of atomic bridge state.

Signed-off-by: Neil Armstrong 
Reviewed-by: Boris Brezillon 
Reviewed-by: Laurent Pinchart 
Reviewed-by: Jernej Škrabec 
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index e097f60e6431..9f2918898f60 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2506,6 +2506,9 @@ static void dw_hdmi_bridge_enable(struct drm_bridge 
*bridge)
 }
 
 static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
+   .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+   .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+   .atomic_reset = drm_atomic_helper_bridge_reset,
.attach = dw_hdmi_bridge_attach,
.detach = dw_hdmi_bridge_detach,
.enable = dw_hdmi_bridge_enable,
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 08/11] drm/meson: dw-hdmi: stop enforcing input_bus_format

2020-03-04 Thread Neil Armstrong
To allow using formats from negotiation, stop enforcing input_bus_format
in the private dw-plat-data struct.

Signed-off-by: Neil Armstrong 
Reviewed-by: Boris Brezillon 
Reviewed-by: Jernej Škrabec 
---
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c 
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 8f51d032262c..72c118142eaf 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -1014,7 +1014,6 @@ static int meson_dw_hdmi_bind(struct device *dev, struct 
device *master,
dw_plat_data->phy_ops = _dw_hdmi_phy_ops;
dw_plat_data->phy_name = "meson_dw_hdmi_phy";
dw_plat_data->phy_data = meson_dw_hdmi;
-   dw_plat_data->input_bus_format = MEDIA_BUS_FMT_YUV8_1X24;
dw_plat_data->input_bus_encoding = V4L2_YCBCR_ENC_709;
 
if (dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 10/11] drm/meson: vclk: add support for YUV420 setup

2020-03-04 Thread Neil Armstrong
This patch adds clocking support for the YUV420 output from the
Amlogic Meson SoCs Video Processing Unit to the HDMI Controller.

The YUV420 is obtained by generating a YUV444 pixel stream like
the classic HDMI display modes, but then the Video Encoder output
can be configured to down-sample the YUV444 pixel stream to a YUV420
stream.

This mode needs a different clock generation scheme since the TMDS PHY
clock must match the 10x ratio with the YUV420 pixel clock, but
the video encoder must run at 2x the pixel clock.

This patch adds the TMDS PHY clock value in all the video clock setup
in order to better support these specific uses cases and switch
to the Common Clock framework for clocks handling in the future.

Signed-off-by: Neil Armstrong 
Reviewed-by: Jernej Škrabec 
---
 drivers/gpu/drm/meson/meson_dw_hdmi.c   | 24 ---
 drivers/gpu/drm/meson/meson_vclk.c  | 93 +++--
 drivers/gpu/drm/meson/meson_vclk.h  |  7 +-
 drivers/gpu/drm/meson/meson_venc_cvbs.c |  6 +-
 4 files changed, 95 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c 
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index b2105c0fe205..b5b0d45eb314 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -376,15 +376,19 @@ static void dw_hdmi_set_vclk(struct meson_dw_hdmi 
*dw_hdmi,
 {
struct meson_drm *priv = dw_hdmi->priv;
int vic = drm_match_cea_mode(mode);
+   unsigned int phy_freq;
unsigned int vclk_freq;
unsigned int venc_freq;
unsigned int hdmi_freq;
 
vclk_freq = mode->clock;
 
+   /* TMDS clock is pixel_clock * 10 */
+   phy_freq = vclk_freq * 10;
+
if (!vic) {
-   meson_vclk_setup(priv, MESON_VCLK_TARGET_DMT, vclk_freq,
-vclk_freq, vclk_freq, false);
+   meson_vclk_setup(priv, MESON_VCLK_TARGET_DMT, phy_freq,
+vclk_freq, vclk_freq, vclk_freq, false);
return;
}
 
@@ -402,11 +406,11 @@ static void dw_hdmi_set_vclk(struct meson_dw_hdmi 
*dw_hdmi,
if (mode->flags & DRM_MODE_FLAG_DBLCLK)
venc_freq /= 2;
 
-   DRM_DEBUG_DRIVER("vclk:%d venc=%d hdmi=%d enci=%d\n",
-   vclk_freq, venc_freq, hdmi_freq,
+   DRM_DEBUG_DRIVER("vclk:%d phy=%d venc=%d hdmi=%d enci=%d\n",
+   phy_freq, vclk_freq, venc_freq, hdmi_freq,
priv->venc.hdmi_use_enci);
 
-   meson_vclk_setup(priv, MESON_VCLK_TARGET_HDMI, vclk_freq,
+   meson_vclk_setup(priv, MESON_VCLK_TARGET_HDMI, phy_freq, vclk_freq,
 venc_freq, hdmi_freq, priv->venc.hdmi_use_enci);
 }
 
@@ -617,6 +621,7 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
   const struct drm_display_mode *mode)
 {
struct meson_drm *priv = connector->dev->dev_private;
+   unsigned int phy_freq;
unsigned int vclk_freq;
unsigned int venc_freq;
unsigned int hdmi_freq;
@@ -643,6 +648,9 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
 
vclk_freq = mode->clock;
 
+   /* TMDS clock is pixel_clock * 10 */
+   phy_freq = vclk_freq * 10;
+
/* 480i/576i needs global pixel doubling */
if (mode->flags & DRM_MODE_FLAG_DBLCLK)
vclk_freq *= 2;
@@ -659,10 +667,10 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
if (mode->flags & DRM_MODE_FLAG_DBLCLK)
venc_freq /= 2;
 
-   dev_dbg(connector->dev->dev, "%s: vclk:%d venc=%d hdmi=%d\n", __func__,
-   vclk_freq, venc_freq, hdmi_freq);
+   dev_dbg(connector->dev->dev, "%s: vclk:%d phy=%d venc=%d hdmi=%d\n",
+   __func__, phy_freq, vclk_freq, venc_freq, hdmi_freq);
 
-   return meson_vclk_vic_supported_freq(vclk_freq);
+   return meson_vclk_vic_supported_freq(phy_freq, vclk_freq);
 }
 
 /* Encoder */
diff --git a/drivers/gpu/drm/meson/meson_vclk.c 
b/drivers/gpu/drm/meson/meson_vclk.c
index f690793ae2d5..fdf26dac9fa8 100644
--- a/drivers/gpu/drm/meson/meson_vclk.c
+++ b/drivers/gpu/drm/meson/meson_vclk.c
@@ -354,12 +354,17 @@ enum {
 /* 2970 /1 /1 /1 /5 /2  => /1 /1 */
MESON_VCLK_HDMI_297000,
 /* 5940 /1 /1 /2 /5 /1  => /1 /1 */
-   MESON_VCLK_HDMI_594000
+   MESON_VCLK_HDMI_594000,
+/* 2970 /1 /1 /1 /5 /1  => /1 /2 */
+   MESON_VCLK_HDMI_594000_YUV420,
 };
 
 struct meson_vclk_params {
+   unsigned int pll_freq;
+   unsigned int phy_freq;
+   unsigned int vclk_freq;
+   unsigned int venc_freq;
unsigned int pixel_freq;
-   unsigned int pll_base_freq;
unsigned int pll_od1;
unsigned int pll_od2;
unsigned int pll_od3;
@@ -367,8 +372,11 @@ struct meson_vclk_params {
unsigned int vclk_div;
 } params[] = {
[MESON_VCLK_HDMI_ENCI_54000] = {
+   .pll_freq = 432,
+   .phy_freq = 27,
+   .vclk_freq = 54000,
+ 

[PATCH v5 01/11] drm/bridge: dw-hdmi: set mtmdsclock for deep color

2020-03-04 Thread Neil Armstrong
From: Jonas Karlman 

Configure the correct mtmdsclock for deep colors to prepare support
for 10, 12 & 16bit output.

Signed-off-by: Jonas Karlman 
Signed-off-by: Neil Armstrong 
Reviewed-by: Jernej Škrabec 
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 9bad194cfd0a..10f98c9ee77e 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1814,13 +1814,32 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
int hblank, vblank, h_de_hs, v_de_vs, hsync_len, vsync_len;
unsigned int vdisplay, hdisplay;
 
-   vmode->mtmdsclock = vmode->mpixelclock = mode->clock * 1000;
+   vmode->mpixelclock = mode->clock * 1000;
 
dev_dbg(hdmi->dev, "final pixclk = %d\n", vmode->mpixelclock);
 
+   vmode->mtmdsclock = vmode->mpixelclock;
+
+   if (!hdmi_bus_fmt_is_yuv422(hdmi->hdmi_data.enc_out_bus_format)) {
+   switch (hdmi_bus_fmt_color_depth(
+   hdmi->hdmi_data.enc_out_bus_format)) {
+   case 16:
+   vmode->mtmdsclock = vmode->mpixelclock * 2;
+   break;
+   case 12:
+   vmode->mtmdsclock = vmode->mpixelclock * 3 / 2;
+   break;
+   case 10:
+   vmode->mtmdsclock = vmode->mpixelclock * 5 / 4;
+   break;
+   }
+   }
+
if (hdmi_bus_fmt_is_yuv420(hdmi->hdmi_data.enc_out_bus_format))
vmode->mtmdsclock /= 2;
 
+   dev_dbg(hdmi->dev, "final tmdsclock = %d\n", vmode->mtmdsclock);
+
/* Set up HDMI_FC_INVIDCONF */
inv_val = (hdmi->hdmi_data.hdcp_enable ||
   (dw_hdmi_support_scdc(hdmi) &&
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 00/11] drm/bridge: dw-hdmi: implement bus-format negotiation and YUV420 support

2020-03-04 Thread Neil Armstrong
Hi Philippe, Heiko, Maxime, Laurent,

A bad negociation was detected on platforms not implementing a bridge on the
encoder side, which has been fixed in this version.

Could you check it doesn't break your platforms using dw-hdmi ? Especially
patches 1-5.

Thanks,
Neil

This patchset is based on Boris's merged "drm: Add support for bus-format 
negotiation"
patchset to implement full bus-format negotiation for DW-HDMI, including YUV420 
support and
10/12/16bit YUV444, YUV422 and RGB. The Color Space Converter support is 
already implemented.

And the counterpart implementation in the Amlogic Meson VPU dw-hdmi glue :
- basic bus-format negotiation to select YUV444 bus-format as DW-HDMI input
- YUV420 support when HDMI2.0 YUV420 modeset

This is a follow-up from the previous attempts :
- "drm/meson: Add support for HDMI2.0 YUV420 4k60" at [2]
- "drm/meson: Add support for HDMI2.0 4k60" at [3]

Changes since v4 at [7]:
- Cleaned up patch 1
- Added comment on patch 2
- Added commit message on patch 3
- Fixed invalid negociation when encoder is not yet a bridge (seen on allwinner 
& rockchip platforms) on patch 4
- Fixed invalid defines, handled MEDIA_BUS_FMT_FIXED and cleaned negociation 
debug on patch 4
- Added tags on patch 5, 6
- Removed meson_venc_hdmi_encoder_get_out_bus_fmts on patch 7
- Add off-list r-b from Jernej

Changes since v3 at [6]:
- Added "Plug atomic state hooks to the default implementation" on drm/bridge: 
dw-hdmi
- Also added these atomic state hooks in meson-dw-hdmi in patch 7
- Rebased on latest drm-misc-next including patches 1-7 of [1]

Changes since RFC v2 at [5]:
- Added fixes from Jonas, who tested and integrated it for Rockchip SoCs
- Added support for 10/12/16bit tmds clock calculation
- Added support for max_bcp connector property
- Adapted to Boris's v4 patchset
- Fixed typos reported by boris

Changes since RFC v1 at [4]:
- Rewrote negociation using the v2 patchset, including full DW-HDMI fmt 
negociation

[2] 
https://patchwork.freedesktop.org/patch/msgid/20190520133753.23871-1-narmstr...@baylibre.com
[3] 
https://patchwork.freedesktop.org/patch/msgid/1549022873-40549-1-git-send-email-narmstr...@baylibre.com
[4] 
https://patchwork.freedesktop.org/patch/msgid/20190820084109.24616-1-narmstr...@baylibre.com
[5] 
https://patchwork.freedesktop.org/patch/msgid/20190827081425.15011-1-narmstr...@baylibre.com
[6] 
https://patchwork.freedesktop.org/patch/msgid/20191218154637.17509-1-narmstr...@baylibre.com
[7] 
https://patchwork.freedesktop.org/patch/msgid/20200206191834.6125-1-narmstr...@baylibre.com

Jonas Karlman (2):
  drm/bridge: dw-hdmi: set mtmdsclock for deep color
  drm/bridge: dw-hdmi: add max bpc connector property

Neil Armstrong (9):
  drm/bridge: dw-hdmi: Plug atomic state hooks to the default
implementation
  drm/bridge: synopsys: dw-hdmi: add bus format negociation
  drm/bridge: synopsys: dw-hdmi: allow ycbcr420 modes for >= 0x200a
  drm/meson: venc: make drm_display_mode const
  drm/meson: meson_dw_hdmi: add bridge and switch to drm_bridge_funcs
  drm/meson: dw-hdmi: stop enforcing input_bus_format
  drm/meson: venc: add support for YUV420 setup
  drm/meson: vclk: add support for YUV420 setup
  drm/meson: Add YUV420 output support

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 319 +-
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 180 +---
 drivers/gpu/drm/meson/meson_vclk.c|  93 +--
 drivers/gpu/drm/meson/meson_vclk.h|   7 +-
 drivers/gpu/drm/meson/meson_venc.c|  10 +-
 drivers/gpu/drm/meson/meson_venc.h|   4 +-
 drivers/gpu/drm/meson/meson_venc_cvbs.c   |   6 +-
 include/drm/bridge/dw_hdmi.h  |   1 +
 8 files changed, 544 insertions(+), 76 deletions(-)

-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 02/11] drm/bridge: dw-hdmi: add max bpc connector property

2020-03-04 Thread Neil Armstrong
From: Jonas Karlman 

Add the max_bpc property to the dw-hdmi connector to prepare support
for 10, 12 & 16bit output support.

Signed-off-by: Jonas Karlman 
Signed-off-by: Neil Armstrong 
Reviewed-by: Laurent Pinchart 
Reviewed-by: Jernej Škrabec 
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 10f98c9ee77e..e097f60e6431 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2414,6 +2414,14 @@ static int dw_hdmi_bridge_attach(struct drm_bridge 
*bridge,
DRM_MODE_CONNECTOR_HDMIA,
hdmi->ddc);
 
+   /*
+* drm_connector_attach_max_bpc_property() requires the
+* connector to have a state.
+*/
+   drm_atomic_helper_connector_reset(connector);
+
+   drm_connector_attach_max_bpc_property(connector, 8, 16);
+
if (hdmi->version >= 0x200a && hdmi->plat_data->use_drm_infoframe)
drm_object_attach_property(>base,

connector->dev->mode_config.hdr_output_metadata_property, 0);
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 06/11] drm/meson: venc: make drm_display_mode const

2020-03-04 Thread Neil Armstrong
Before switching to bridge funcs, make sure drm_display_mode is passed
as const to the venc functions.

Signed-off-by: Neil Armstrong 
Reviewed-by: Boris Brezillon 
Acked-by: Laurent Pinchart 
Reviewed-by: Jernej Škrabec 
---
 drivers/gpu/drm/meson/meson_venc.c | 2 +-
 drivers/gpu/drm/meson/meson_venc.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_venc.c 
b/drivers/gpu/drm/meson/meson_venc.c
index 4efd7864d5bf..a9ab78970bfe 100644
--- a/drivers/gpu/drm/meson/meson_venc.c
+++ b/drivers/gpu/drm/meson/meson_venc.c
@@ -946,7 +946,7 @@ bool meson_venc_hdmi_venc_repeat(int vic)
 EXPORT_SYMBOL_GPL(meson_venc_hdmi_venc_repeat);
 
 void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic,
- struct drm_display_mode *mode)
+ const struct drm_display_mode *mode)
 {
union meson_hdmi_venc_mode *vmode = NULL;
union meson_hdmi_venc_mode vmode_dmt;
diff --git a/drivers/gpu/drm/meson/meson_venc.h 
b/drivers/gpu/drm/meson/meson_venc.h
index 576768bdd08d..1abdcbdf51c0 100644
--- a/drivers/gpu/drm/meson/meson_venc.h
+++ b/drivers/gpu/drm/meson/meson_venc.h
@@ -60,7 +60,7 @@ extern struct meson_cvbs_enci_mode meson_cvbs_enci_ntsc;
 void meson_venci_cvbs_mode_set(struct meson_drm *priv,
   struct meson_cvbs_enci_mode *mode);
 void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic,
- struct drm_display_mode *mode);
+ const struct drm_display_mode *mode);
 unsigned int meson_venci_get_field(struct meson_drm *priv);
 
 void meson_venc_enable_vsync(struct meson_drm *priv);
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 07/11] drm/meson: meson_dw_hdmi: add bridge and switch to drm_bridge_funcs

2020-03-04 Thread Neil Armstrong
Switch the dw-hdmi driver to drm_bridge_funcs by implementing a new local
bridge, connecting it to the dw-hdmi bridge, then implement the
atomic_get_input_bus_fmts/atomic_get_output_bus_fmts.

Signed-off-by: Neil Armstrong 
Reviewed-by: Jernej Škrabec 
---
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 85 ---
 1 file changed, 65 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c 
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 3bb7ffe5fc39..8f51d032262c 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -16,6 +16,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -135,6 +136,7 @@ struct meson_dw_hdmi_data {
 
 struct meson_dw_hdmi {
struct drm_encoder encoder;
+   struct drm_bridge bridge;
struct dw_hdmi_plat_data dw_plat_data;
struct meson_drm *priv;
struct device *dev;
@@ -151,6 +153,8 @@ struct meson_dw_hdmi {
 };
 #define encoder_to_meson_dw_hdmi(x) \
container_of(x, struct meson_dw_hdmi, encoder)
+#define bridge_to_meson_dw_hdmi(x) \
+   container_of(x, struct meson_dw_hdmi, bridge)
 
 static inline int dw_hdmi_is_compatible(struct meson_dw_hdmi *dw_hdmi,
const char *compat)
@@ -368,7 +372,7 @@ static inline void meson_dw_hdmi_phy_reset(struct 
meson_dw_hdmi *dw_hdmi)
 }
 
 static void dw_hdmi_set_vclk(struct meson_dw_hdmi *dw_hdmi,
-struct drm_display_mode *mode)
+const struct drm_display_mode *mode)
 {
struct meson_drm *priv = dw_hdmi->priv;
int vic = drm_match_cea_mode(mode);
@@ -663,6 +667,10 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
 
 /* Encoder */
 
+static const u32 meson_dw_hdmi_out_bus_fmts[] = {
+   MEDIA_BUS_FMT_YUV8_1X24,
+};
+
 static void meson_venc_hdmi_encoder_destroy(struct drm_encoder *encoder)
 {
drm_encoder_cleanup(encoder);
@@ -672,16 +680,43 @@ static const struct drm_encoder_funcs 
meson_venc_hdmi_encoder_funcs = {
.destroy= meson_venc_hdmi_encoder_destroy,
 };
 
-static int meson_venc_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
+static u32 *
+meson_venc_hdmi_encoder_get_inp_bus_fmts(struct drm_bridge *bridge,
+   struct drm_bridge_state *bridge_state,
+   struct drm_crtc_state *crtc_state,
+   struct drm_connector_state *conn_state,
+   u32 output_fmt,
+   unsigned int *num_input_fmts)
+{
+   u32 *input_fmts = NULL;
+
+   if (output_fmt == meson_dw_hdmi_out_bus_fmts[0]) {
+   *num_input_fmts = 1;
+   input_fmts = kcalloc(*num_input_fmts,
+sizeof(*input_fmts),
+GFP_KERNEL);
+   if (!input_fmts)
+   return NULL;
+
+   input_fmts[0] = output_fmt;
+   } else {
+   *num_input_fmts = 0;
+   }
+
+   return input_fmts;
+}
+
+static int meson_venc_hdmi_encoder_atomic_check(struct drm_bridge *bridge,
+   struct drm_bridge_state *bridge_state,
struct drm_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
 {
return 0;
 }
 
-static void meson_venc_hdmi_encoder_disable(struct drm_encoder *encoder)
+static void meson_venc_hdmi_encoder_disable(struct drm_bridge *bridge)
 {
-   struct meson_dw_hdmi *dw_hdmi = encoder_to_meson_dw_hdmi(encoder);
+   struct meson_dw_hdmi *dw_hdmi = bridge_to_meson_dw_hdmi(bridge);
struct meson_drm *priv = dw_hdmi->priv;
 
DRM_DEBUG_DRIVER("\n");
@@ -693,9 +728,9 @@ static void meson_venc_hdmi_encoder_disable(struct 
drm_encoder *encoder)
writel_relaxed(0, priv->io_base + _REG(ENCP_VIDEO_EN));
 }
 
-static void meson_venc_hdmi_encoder_enable(struct drm_encoder *encoder)
+static void meson_venc_hdmi_encoder_enable(struct drm_bridge *bridge)
 {
-   struct meson_dw_hdmi *dw_hdmi = encoder_to_meson_dw_hdmi(encoder);
+   struct meson_dw_hdmi *dw_hdmi = bridge_to_meson_dw_hdmi(bridge);
struct meson_drm *priv = dw_hdmi->priv;
 
DRM_DEBUG_DRIVER("%s\n", priv->venc.hdmi_use_enci ? "VENCI" : "VENCP");
@@ -706,11 +741,11 @@ static void meson_venc_hdmi_encoder_enable(struct 
drm_encoder *encoder)
writel_relaxed(1, priv->io_base + _REG(ENCP_VIDEO_EN));
 }
 
-static void meson_venc_hdmi_encoder_mode_set(struct drm_encoder *encoder,
-  struct drm_display_mode *mode,
-  struct drm_display_mode *adjusted_mode)
+static void meson_venc_hdmi_encoder_mode_set(struct drm_bridge *bridge,
+  const struct 

[PATCH v6 8/9] drm/vmwgfx: Introduce a huge page aligning TTM range manager

2020-03-04 Thread VMware
From: Thomas Hellstrom 

Using huge page-table entries requires that the physical address of the
start of a buffer object is huge page size aligned.
Make a special version of the TTM range manager that accomplishes this,
but falls back to a smaller page size alignment (PUD->PMD, PMD->NORMAL)
to avoid eviction.
If other drivers want to use it in the future, it can be made a
TTM generic helper. Note that drivers can force eviction for a certain
alignment by assigning the TTM GPU alignment correspondingly.

Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: "Matthew Wilcox (Oracle)" 
Cc: "Kirill A. Shutemov" 
Cc: Ralph Campbell 
Cc: "Jérôme Glisse" 
Cc: "Christian König" 
Cc: Dan Williams 
Signed-off-by: Thomas Hellstrom 
Reviewed-by: Roland Scheidegger 
Acked-by: Christian König 
---
 drivers/gpu/drm/vmwgfx/Makefile |   1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h |   7 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 166 
 3 files changed, 174 insertions(+)
 create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_thp.c

diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
index c877a21a0739..421dd2a497a5 100644
--- a/drivers/gpu/drm/vmwgfx/Makefile
+++ b/drivers/gpu/drm/vmwgfx/Makefile
@@ -11,4 +11,5 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o 
vmwgfx_drv.o \
vmwgfx_validation.o vmwgfx_page_dirty.o \
ttm_object.o ttm_lock.o
 
+vmwgfx-$(CONFIG_TRANSPARENT_HUGEPAGE) += vmwgfx_thp.o
 obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index bb2757c98f0a..fe5b7293b8d1 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1435,6 +1435,13 @@ vm_fault_t vmw_bo_vm_huge_fault(struct vm_fault *vmf,
enum page_entry_size pe_size);
 #endif
 
+/* Transparent hugepage support - vmwgfx_thp.c */
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+extern const struct ttm_mem_type_manager_func vmw_thp_func;
+#else
+#define vmw_thp_func ttm_bo_manager_func
+#endif
+
 /**
  * VMW_DEBUG_KMS - Debug output for kernel mode-setting
  *
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c
new file mode 100644
index ..b7c816ba7166
--- /dev/null
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Huge page-table-entry support for IO memory.
+ *
+ * Copyright (C) 2007-2019 Vmware, Inc. All rights reservedd.
+ */
+#include "vmwgfx_drv.h"
+#include 
+#include 
+#include 
+
+/**
+ * struct vmw_thp_manager - Range manager implementing huge page alignment
+ *
+ * @mm: The underlying range manager. Protected by @lock.
+ * @lock: Manager lock.
+ */
+struct vmw_thp_manager {
+   struct drm_mm mm;
+   spinlock_t lock;
+};
+
+static int vmw_thp_insert_aligned(struct drm_mm *mm, struct drm_mm_node *node,
+ unsigned long align_pages,
+ const struct ttm_place *place,
+ struct ttm_mem_reg *mem,
+ unsigned long lpfn,
+ enum drm_mm_insert_mode mode)
+{
+   if (align_pages >= mem->page_alignment &&
+   (!mem->page_alignment || align_pages % mem->page_alignment == 0)) {
+   return drm_mm_insert_node_in_range(mm, node,
+  mem->num_pages,
+  align_pages, 0,
+  place->fpfn, lpfn, mode);
+   }
+
+   return -ENOSPC;
+}
+
+static int vmw_thp_get_node(struct ttm_mem_type_manager *man,
+   struct ttm_buffer_object *bo,
+   const struct ttm_place *place,
+   struct ttm_mem_reg *mem)
+{
+   struct vmw_thp_manager *rman = (struct vmw_thp_manager *) man->priv;
+   struct drm_mm *mm = >mm;
+   struct drm_mm_node *node;
+   unsigned long align_pages;
+   unsigned long lpfn;
+   enum drm_mm_insert_mode mode = DRM_MM_INSERT_BEST;
+   int ret;
+
+   node = kzalloc(sizeof(*node), GFP_KERNEL);
+   if (!node)
+   return -ENOMEM;
+
+   lpfn = place->lpfn;
+   if (!lpfn)
+   lpfn = man->size;
+
+   mode = DRM_MM_INSERT_BEST;
+   if (place->flags & TTM_PL_FLAG_TOPDOWN)
+   mode = DRM_MM_INSERT_HIGH;
+
+   spin_lock(>lock);
+   if (IS_ENABLED(CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD)) {
+   align_pages = (HPAGE_PUD_SIZE >> PAGE_SHIFT);
+   if (mem->num_pages >= align_pages) {
+   ret = vmw_thp_insert_aligned(mm, node, align_pages,
+place, mem, lpfn, mode);
+   if (!ret)
+   goto found_unlock;
+   }
+   }
+
+   

[PATCH v6 7/9] drm: Add a drm_get_unmapped_area() helper

2020-03-04 Thread VMware
From: Thomas Hellstrom 

Unaligned virtual addresses makes it unlikely that huge page-table entries
can be used.
So align virtual buffer object address huge page boundaries to the
underlying physical address huge page boundaries taking buffer object
sizes into account to determine when it might be possible to use huge
page-table entries.

Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: "Matthew Wilcox (Oracle)" 
Cc: "Kirill A. Shutemov" 
Cc: Ralph Campbell 
Cc: "Jérôme Glisse" 
Cc: "Christian König" 
Cc: Dan Williams 
Signed-off-by: Thomas Hellstrom 
Reviewed-by: Roland Scheidegger 
Acked-by: Christian König 
---
 drivers/gpu/drm/drm_file.c | 140 +
 include/drm/drm_file.h |   9 +++
 2 files changed, 149 insertions(+)

diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 92d16724f949..77e691202b52 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -48,6 +48,11 @@
 #include "drm_internal.h"
 #include "drm_legacy.h"
 
+#ifdef CONFIG_MMU
+#include 
+#include 
+#endif
+
 /* from BKL pushdown */
 DEFINE_MUTEX(drm_global_mutex);
 
@@ -796,3 +801,138 @@ struct file *mock_drm_getfile(struct drm_minor *minor, 
unsigned int flags)
return file;
 }
 EXPORT_SYMBOL_FOR_TESTS_ONLY(mock_drm_getfile);
+
+#ifdef CONFIG_MMU
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+/*
+ * drm_addr_inflate() attempts to construct an aligned area by inflating
+ * the area size and skipping the unaligned start of the area.
+ * adapted from shmem_get_unmapped_area()
+ */
+static unsigned long drm_addr_inflate(unsigned long addr,
+ unsigned long len,
+ unsigned long pgoff,
+ unsigned long flags,
+ unsigned long huge_size)
+{
+   unsigned long offset, inflated_len;
+   unsigned long inflated_addr;
+   unsigned long inflated_offset;
+
+   offset = (pgoff << PAGE_SHIFT) & (huge_size - 1);
+   if (offset && offset + len < 2 * huge_size)
+   return addr;
+   if ((addr & (huge_size - 1)) == offset)
+   return addr;
+
+   inflated_len = len + huge_size - PAGE_SIZE;
+   if (inflated_len > TASK_SIZE)
+   return addr;
+   if (inflated_len < len)
+   return addr;
+
+   inflated_addr = current->mm->get_unmapped_area(NULL, 0, inflated_len,
+  0, flags);
+   if (IS_ERR_VALUE(inflated_addr))
+   return addr;
+   if (inflated_addr & ~PAGE_MASK)
+   return addr;
+
+   inflated_offset = inflated_addr & (huge_size - 1);
+   inflated_addr += offset - inflated_offset;
+   if (inflated_offset > offset)
+   inflated_addr += huge_size;
+
+   if (inflated_addr > TASK_SIZE - len)
+   return addr;
+
+   return inflated_addr;
+}
+
+/**
+ * drm_get_unmapped_area() - Get an unused user-space virtual memory area
+ * suitable for huge page table entries.
+ * @file: The struct file representing the address space being mmap()'d.
+ * @uaddr: Start address suggested by user-space.
+ * @len: Length of the area.
+ * @pgoff: The page offset into the address space.
+ * @flags: mmap flags
+ * @mgr: The address space manager used by the drm driver. This argument can
+ * probably be removed at some point when all drivers use the same
+ * address space manager.
+ *
+ * This function attempts to find an unused user-space virtual memory area
+ * that can accommodate the size we want to map, and that is properly
+ * aligned to facilitate huge page table entries matching actual
+ * huge pages or huge page aligned memory in buffer objects. Buffer objects
+ * are assumed to start at huge page boundary pfns (io memory) or be
+ * populated by huge pages aligned to the start of the buffer object
+ * (system- or coherent memory). Adapted from shmem_get_unmapped_area.
+ *
+ * Return: aligned user-space address.
+ */
+unsigned long drm_get_unmapped_area(struct file *file,
+   unsigned long uaddr, unsigned long len,
+   unsigned long pgoff, unsigned long flags,
+   struct drm_vma_offset_manager *mgr)
+{
+   unsigned long addr;
+   unsigned long inflated_addr;
+   struct drm_vma_offset_node *node;
+
+   if (len > TASK_SIZE)
+   return -ENOMEM;
+
+   /*
+* @pgoff is the file page-offset the huge page boundaries of
+* which typically aligns to physical address huge page boundaries.
+* That's not true for DRM, however, where physical address huge
+* page boundaries instead are aligned with the offset from
+* buffer object start. So adjust @pgoff to be the offset from
+* buffer object start.
+*/
+   drm_vma_offset_lock_lookup(mgr);
+   node = drm_vma_offset_lookup_locked(mgr, pgoff, 

[PATCH v6 1/9] fs: Constify vma argument to vma_is_dax

2020-03-04 Thread VMware
From: Thomas Hellstrom 

The function is used by upcoming vma_is_special_huge() with which we want
to use a const vma argument. Since for vma_is_dax() the vma argument is
only dereferenced for reading, constify it.

Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: "Matthew Wilcox (Oracle)" 
Cc: "Kirill A. Shutemov" 
Cc: Ralph Campbell 
Cc: "Jérôme Glisse" 
Cc: "Christian König" 
Cc: Dan Williams 
Signed-off-by: Thomas Hellstrom 
Reviewed-by: Roland Scheidegger 
Acked-by: Christian König 
---
 include/linux/fs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3cd4fe6b845e..2b38ce5b73ad 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3391,7 +3391,7 @@ static inline bool io_is_direct(struct file *filp)
return (filp->f_flags & O_DIRECT) || IS_DAX(filp->f_mapping->host);
 }
 
-static inline bool vma_is_dax(struct vm_area_struct *vma)
+static inline bool vma_is_dax(const struct vm_area_struct *vma)
 {
return vma->vm_file && IS_DAX(vma->vm_file->f_mapping->host);
 }
-- 
2.21.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v6 9/9] drm/vmwgfx: Hook up the helpers to align buffer objects

2020-03-04 Thread VMware
From: Thomas Hellstrom 

Start using the helpers that align buffer object user-space addresses and
buffer object vram addresses to huge page boundaries.
This is to improve the chances of allowing huge page-table entries.

Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: "Matthew Wilcox (Oracle)" 
Cc: "Kirill A. Shutemov" 
Cc: Ralph Campbell 
Cc: "Jérôme Glisse" 
Cc: "Christian König" 
Cc: Dan Williams 
Signed-off-by: Thomas Hellstrom 
Reviewed-by: Roland Scheidegger 
Acked-by: Christian König 
---
 drivers/gpu/drm/drm_file.c |  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 13 +
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h|  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  2 +-
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 77e691202b52..b29a72b68068 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -935,4 +935,5 @@ unsigned long drm_get_unmapped_area(struct file *file,
return current->mm->get_unmapped_area(file, uaddr, len, pgoff, flags);
 }
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
+EXPORT_SYMBOL_GPL(drm_get_unmapped_area);
 #endif /* CONFIG_MMU */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 827458f49112..77d85bc54ef0 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1223,6 +1223,18 @@ static void vmw_remove(struct pci_dev *pdev)
pci_disable_device(pdev);
 }
 
+static unsigned long
+vmw_get_unmapped_area(struct file *file, unsigned long uaddr,
+ unsigned long len, unsigned long pgoff,
+ unsigned long flags)
+{
+   struct drm_file *file_priv = file->private_data;
+   struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
+
+   return drm_get_unmapped_area(file, uaddr, len, pgoff, flags,
+_priv->vma_manager);
+}
+
 static int vmwgfx_pm_notifier(struct notifier_block *nb, unsigned long val,
  void *ptr)
 {
@@ -1394,6 +1406,7 @@ static const struct file_operations vmwgfx_driver_fops = {
.compat_ioctl = vmw_compat_ioctl,
 #endif
.llseek = noop_llseek,
+   .get_unmapped_area = vmw_get_unmapped_area,
 };
 
 static struct drm_driver driver = {
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index fe5b7293b8d1..4853bd95bf54 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -929,6 +929,7 @@ extern int vmw_mmap(struct file *filp, struct 
vm_area_struct *vma);
 
 extern void vmw_validation_mem_init_ttm(struct vmw_private *dev_priv,
size_t gran);
+
 /**
  * TTM buffer object driver - vmwgfx_ttm_buffer.c
  */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index d8ea3dd10af0..34c721ab3ff3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -754,7 +754,7 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
break;
case TTM_PL_VRAM:
/* "On-card" video ram */
-   man->func = _bo_manager_func;
+   man->func = _thp_func;
man->gpu_offset = 0;
man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_CACHED;
-- 
2.21.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v6 5/9] drm/ttm, drm/vmwgfx: Support huge TTM pagefaults

2020-03-04 Thread VMware
From: Thomas Hellstrom 

Support huge (PMD-size and PUD-size) page-table entries by providing a
huge_fault() callback.
We still support private mappings and write-notify by splitting the huge
page-table entries on write-access.

Note that for huge page-faults to occur, either the kernel needs to be
compiled with trans-huge-pages always enabled, or the kernel needs to be
compiled with trans-huge-pages enabled using madvise, and the user-space
app needs to call madvise() to enable trans-huge pages on a per-mapping
basis.

Furthermore huge page-faults will not succeed unless buffer objects and
user-space addresses are aligned on huge page size boundaries.

Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: "Matthew Wilcox (Oracle)" 
Cc: "Kirill A. Shutemov" 
Cc: Ralph Campbell 
Cc: "Jérôme Glisse" 
Cc: "Christian König" 
Cc: Dan Williams 
Signed-off-by: Thomas Hellstrom 
Reviewed-by: Roland Scheidegger 
Reviewed-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c| 161 -
 drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c |   2 +-
 include/drm/ttm/ttm_bo_api.h   |   3 +-
 3 files changed, 161 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 389128b8c4dd..0af14835504c 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -156,6 +156,89 @@ vm_fault_t ttm_bo_vm_reserve(struct ttm_buffer_object *bo,
 }
 EXPORT_SYMBOL(ttm_bo_vm_reserve);
 
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+/**
+ * ttm_bo_vm_insert_huge - Insert a pfn for PUD or PMD faults
+ * @vmf: Fault data
+ * @bo: The buffer object
+ * @page_offset: Page offset from bo start
+ * @fault_page_size: The size of the fault in pages.
+ * @pgprot: The page protections.
+ * Does additional checking whether it's possible to insert a PUD or PMD
+ * pfn and performs the insertion.
+ *
+ * Return: VM_FAULT_NOPAGE on successful insertion, VM_FAULT_FALLBACK if
+ * a huge fault was not possible, or on insertion error.
+ */
+static vm_fault_t ttm_bo_vm_insert_huge(struct vm_fault *vmf,
+   struct ttm_buffer_object *bo,
+   pgoff_t page_offset,
+   pgoff_t fault_page_size,
+   pgprot_t pgprot)
+{
+   pgoff_t i;
+   vm_fault_t ret;
+   unsigned long pfn;
+   pfn_t pfnt;
+   struct ttm_tt *ttm = bo->ttm;
+   bool write = vmf->flags & FAULT_FLAG_WRITE;
+
+   /* Fault should not cross bo boundary. */
+   page_offset &= ~(fault_page_size - 1);
+   if (page_offset + fault_page_size > bo->num_pages)
+   goto out_fallback;
+
+   if (bo->mem.bus.is_iomem)
+   pfn = ttm_bo_io_mem_pfn(bo, page_offset);
+   else
+   pfn = page_to_pfn(ttm->pages[page_offset]);
+
+   /* pfn must be fault_page_size aligned. */
+   if ((pfn & (fault_page_size - 1)) != 0)
+   goto out_fallback;
+
+   /* Check that memory is contiguous. */
+   if (!bo->mem.bus.is_iomem) {
+   for (i = 1; i < fault_page_size; ++i) {
+   if (page_to_pfn(ttm->pages[page_offset + i]) != pfn + i)
+   goto out_fallback;
+   }
+   } else if (bo->bdev->driver->io_mem_pfn) {
+   for (i = 1; i < fault_page_size; ++i) {
+   if (ttm_bo_io_mem_pfn(bo, page_offset + i) != pfn + i)
+   goto out_fallback;
+   }
+   }
+
+   pfnt = __pfn_to_pfn_t(pfn, PFN_DEV);
+   if (fault_page_size == (HPAGE_PMD_SIZE >> PAGE_SHIFT))
+   ret = vmf_insert_pfn_pmd_prot(vmf, pfnt, pgprot, write);
+#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
+   else if (fault_page_size == (HPAGE_PUD_SIZE >> PAGE_SHIFT))
+   ret = vmf_insert_pfn_pud_prot(vmf, pfnt, pgprot, write);
+#endif
+   else
+   WARN_ON_ONCE(ret = VM_FAULT_FALLBACK);
+
+   if (ret != VM_FAULT_NOPAGE)
+   goto out_fallback;
+
+   return VM_FAULT_NOPAGE;
+out_fallback:
+   count_vm_event(THP_FAULT_FALLBACK);
+   return VM_FAULT_FALLBACK;
+}
+#else
+static vm_fault_t ttm_bo_vm_insert_huge(struct vm_fault *vmf,
+   struct ttm_buffer_object *bo,
+   pgoff_t page_offset,
+   pgoff_t fault_page_size,
+   pgprot_t pgprot)
+{
+   return VM_FAULT_FALLBACK;
+}
+#endif
+
 /**
  * ttm_bo_vm_fault_reserved - TTM fault helper
  * @vmf: The struct vm_fault given as argument to the fault callback
@@ -163,6 +246,7 @@ EXPORT_SYMBOL(ttm_bo_vm_reserve);
  * @num_prefault: Maximum number of prefault pages. The caller may want to
  * specify this based on madvice settings and the size of the GPU object
  * backed by the memory.
+ * @fault_page_size: The size of the fault in 

[PATCH v6 3/9] mm: Split huge pages on write-notify or COW

2020-03-04 Thread VMware
From: Thomas Hellstrom 

The functions wp_huge_pmd() and wp_huge_pud() currently relies on the
huge_fault() callback to split huge page table entries if needed.
However for module users that requires export of the split_huge_xxx()
functionality which may be undesired. Instead split pre-existing huge
page-table entries on VM_FAULT_FALLBACK return.

We currently only do COW and write-notify on the PTE level, so if the
huge_fault() handler returns VM_FAULT_FALLBACK on wp faults,
split the huge pages and page-table entries. Also do this for huge PUDs
if there is no huge_fault() handler and the vma is not anonymous, similar
to how it's done for PMDs.

Note that fs/dax.c still does the splitting in the huge_fault() handler, but as
huge_fault() A follow-up patch can remove the dax.c split_huge_pmd() if needed.

Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: "Matthew Wilcox (Oracle)" 
Cc: "Kirill A. Shutemov" 
Cc: Ralph Campbell 
Cc: "Jérôme Glisse" 
Cc: "Christian König" 
Cc: Dan Williams 
Signed-off-by: Thomas Hellstrom 
Acked-by: Christian König 
---
 mm/memory.c | 27 +++
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 0bccc622e482..1e3fc1988790 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3932,11 +3932,14 @@ static inline vm_fault_t wp_huge_pmd(struct vm_fault 
*vmf, pmd_t orig_pmd)
 {
if (vma_is_anonymous(vmf->vma))
return do_huge_pmd_wp_page(vmf, orig_pmd);
-   if (vmf->vma->vm_ops->huge_fault)
-   return vmf->vma->vm_ops->huge_fault(vmf, PE_SIZE_PMD);
+   if (vmf->vma->vm_ops->huge_fault) {
+   vm_fault_t ret = vmf->vma->vm_ops->huge_fault(vmf, PE_SIZE_PMD);
 
-   /* COW handled on pte level: split pmd */
-   VM_BUG_ON_VMA(vmf->vma->vm_flags & VM_SHARED, vmf->vma);
+   if (!(ret & VM_FAULT_FALLBACK))
+   return ret;
+   }
+
+   /* COW or write-notify handled on pte level: split pmd. */
__split_huge_pmd(vmf->vma, vmf->pmd, vmf->address, false, NULL);
 
return VM_FAULT_FALLBACK;
@@ -3949,12 +3952,20 @@ static inline bool vma_is_accessible(struct 
vm_area_struct *vma)
 
 static vm_fault_t create_huge_pud(struct vm_fault *vmf)
 {
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+#if defined(CONFIG_TRANSPARENT_HUGEPAGE) &&\
+   defined(CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD)
/* No support for anonymous transparent PUD pages yet */
if (vma_is_anonymous(vmf->vma))
-   return VM_FAULT_FALLBACK;
-   if (vmf->vma->vm_ops->huge_fault)
-   return vmf->vma->vm_ops->huge_fault(vmf, PE_SIZE_PUD);
+   goto split;
+   if (vmf->vma->vm_ops->huge_fault) {
+   vm_fault_t ret = vmf->vma->vm_ops->huge_fault(vmf, PE_SIZE_PUD);
+
+   if (!(ret & VM_FAULT_FALLBACK))
+   return ret;
+   }
+split:
+   /* COW or write-notify not handled on PUD level: split pud.*/
+   __split_huge_pud(vmf->vma, vmf->pud, vmf->address);
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
return VM_FAULT_FALLBACK;
 }
-- 
2.21.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v6 6/9] drm/vmwgfx: Support huge page faults

2020-03-04 Thread VMware
From: Thomas Hellstrom 

With vmwgfx dirty-tracking we need a specialized huge_fault
callback. Implement and hook it up.

Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: "Matthew Wilcox (Oracle)" 
Cc: "Kirill A. Shutemov" 
Cc: Ralph Campbell 
Cc: "Jérôme Glisse" 
Cc: "Christian König" 
Cc: Dan Williams 
Signed-off-by: Thomas Hellstrom 
Reviewed-by: Roland Scheidegger 
Acked-by: Christian König 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h|  4 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c | 74 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c   |  5 +-
 3 files changed, 81 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 86b69397d166..bb2757c98f0a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1430,6 +1430,10 @@ void vmw_bo_dirty_unmap(struct vmw_buffer_object *vbo,
pgoff_t start, pgoff_t end);
 vm_fault_t vmw_bo_vm_fault(struct vm_fault *vmf);
 vm_fault_t vmw_bo_vm_mkwrite(struct vm_fault *vmf);
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+vm_fault_t vmw_bo_vm_huge_fault(struct vm_fault *vmf,
+   enum page_entry_size pe_size);
+#endif
 
 /**
  * VMW_DEBUG_KMS - Debug output for kernel mode-setting
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
index 17a5dca7b921..cde3e07ebaf7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
@@ -473,7 +473,7 @@ vm_fault_t vmw_bo_vm_fault(struct vm_fault *vmf)
 * a lot of unnecessary write faults.
 */
if (vbo->dirty && vbo->dirty->method == VMW_BO_DIRTY_MKWRITE)
-   prot = vma->vm_page_prot;
+   prot = vm_get_page_prot(vma->vm_flags & ~VM_SHARED);
else
prot = vm_get_page_prot(vma->vm_flags);
 
@@ -486,3 +486,75 @@ vm_fault_t vmw_bo_vm_fault(struct vm_fault *vmf)
 
return ret;
 }
+
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+vm_fault_t vmw_bo_vm_huge_fault(struct vm_fault *vmf,
+   enum page_entry_size pe_size)
+{
+   struct vm_area_struct *vma = vmf->vma;
+   struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
+   vma->vm_private_data;
+   struct vmw_buffer_object *vbo =
+   container_of(bo, struct vmw_buffer_object, base);
+   pgprot_t prot;
+   vm_fault_t ret;
+   pgoff_t fault_page_size;
+   bool write = vmf->flags & FAULT_FLAG_WRITE;
+   bool is_cow_mapping =
+   (vma->vm_flags & (VM_SHARED | VM_MAYWRITE)) == VM_MAYWRITE;
+
+   switch (pe_size) {
+   case PE_SIZE_PMD:
+   fault_page_size = HPAGE_PMD_SIZE >> PAGE_SHIFT;
+   break;
+#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
+   case PE_SIZE_PUD:
+   fault_page_size = HPAGE_PUD_SIZE >> PAGE_SHIFT;
+   break;
+#endif
+   default:
+   WARN_ON_ONCE(1);
+   return VM_FAULT_FALLBACK;
+   }
+
+   /* Always do write dirty-tracking and COW on PTE level. */
+   if (write && (READ_ONCE(vbo->dirty) || is_cow_mapping))
+   return VM_FAULT_FALLBACK;
+
+   ret = ttm_bo_vm_reserve(bo, vmf);
+   if (ret)
+   return ret;
+
+   if (vbo->dirty) {
+   pgoff_t allowed_prefault;
+   unsigned long page_offset;
+
+   page_offset = vmf->pgoff -
+   drm_vma_node_start(>base.vma_node);
+   if (page_offset >= bo->num_pages ||
+   vmw_resources_clean(vbo, page_offset,
+   page_offset + PAGE_SIZE,
+   _prefault)) {
+   ret = VM_FAULT_SIGBUS;
+   goto out_unlock;
+   }
+
+   /*
+* Write protect, so we get a new fault on write, and can
+* split.
+*/
+   prot = vm_get_page_prot(vma->vm_flags & ~VM_SHARED);
+   } else {
+   prot = vm_get_page_prot(vma->vm_flags);
+   }
+
+   ret = ttm_bo_vm_fault_reserved(vmf, prot, 1, fault_page_size);
+   if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
+   return ret;
+
+out_unlock:
+   dma_resv_unlock(bo->base.resv);
+
+   return ret;
+}
+#endif
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
index aa7e50f63b94..3c03b1746661 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
@@ -34,7 +34,10 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
.page_mkwrite = vmw_bo_vm_mkwrite,
.fault = vmw_bo_vm_fault,
.open = ttm_bo_vm_open,
-   .close = ttm_bo_vm_close
+   .close = ttm_bo_vm_close,
+#ifdef 

[PATCH v6 4/9] mm: Add vmf_insert_pfn_xxx_prot() for huge page-table entries

2020-03-04 Thread VMware
From: Thomas Hellstrom 

For graphics drivers needing to modify the page-protection, add
huge page-table entries counterparts to vmf_insert_pfn_prot().

Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: "Matthew Wilcox (Oracle)" 
Cc: "Kirill A. Shutemov" 
Cc: Ralph Campbell 
Cc: "Jérôme Glisse" 
Cc: "Christian König" 
Cc: Dan Williams 
Signed-off-by: Thomas Hellstrom 
Acked-by: Christian König 
---
 include/linux/huge_mm.h | 41 +++--
 mm/huge_memory.c| 38 --
 2 files changed, 71 insertions(+), 8 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 5aca3d1bdb32..f63b0882c1b3 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -47,8 +47,45 @@ extern bool move_huge_pmd(struct vm_area_struct *vma, 
unsigned long old_addr,
 extern int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
unsigned long addr, pgprot_t newprot,
int prot_numa);
-vm_fault_t vmf_insert_pfn_pmd(struct vm_fault *vmf, pfn_t pfn, bool write);
-vm_fault_t vmf_insert_pfn_pud(struct vm_fault *vmf, pfn_t pfn, bool write);
+vm_fault_t vmf_insert_pfn_pmd_prot(struct vm_fault *vmf, pfn_t pfn,
+  pgprot_t pgprot, bool write);
+
+/**
+ * vmf_insert_pfn_pmd - insert a pmd size pfn
+ * @vmf: Structure describing the fault
+ * @pfn: pfn to insert
+ * @pgprot: page protection to use
+ * @write: whether it's a write fault
+ *
+ * Insert a pmd size pfn. See vmf_insert_pfn() for additional info.
+ *
+ * Return: vm_fault_t value.
+ */
+static inline vm_fault_t vmf_insert_pfn_pmd(struct vm_fault *vmf, pfn_t pfn,
+   bool write)
+{
+   return vmf_insert_pfn_pmd_prot(vmf, pfn, vmf->vma->vm_page_prot, write);
+}
+vm_fault_t vmf_insert_pfn_pud_prot(struct vm_fault *vmf, pfn_t pfn,
+  pgprot_t pgprot, bool write);
+
+/**
+ * vmf_insert_pfn_pud - insert a pud size pfn
+ * @vmf: Structure describing the fault
+ * @pfn: pfn to insert
+ * @pgprot: page protection to use
+ * @write: whether it's a write fault
+ *
+ * Insert a pud size pfn. See vmf_insert_pfn() for additional info.
+ *
+ * Return: vm_fault_t value.
+ */
+static inline vm_fault_t vmf_insert_pfn_pud(struct vm_fault *vmf, pfn_t pfn,
+   bool write)
+{
+   return vmf_insert_pfn_pud_prot(vmf, pfn, vmf->vma->vm_page_prot, write);
+}
+
 enum transparent_hugepage_flag {
TRANSPARENT_HUGEPAGE_FLAG,
TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index ff7a8b85c3ba..e7c69882861d 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -824,11 +824,24 @@ static void insert_pfn_pmd(struct vm_area_struct *vma, 
unsigned long addr,
pte_free(mm, pgtable);
 }
 
-vm_fault_t vmf_insert_pfn_pmd(struct vm_fault *vmf, pfn_t pfn, bool write)
+/**
+ * vmf_insert_pfn_pmd_prot - insert a pmd size pfn
+ * @vmf: Structure describing the fault
+ * @pfn: pfn to insert
+ * @pgprot: page protection to use
+ * @write: whether it's a write fault
+ *
+ * Insert a pmd size pfn. See vmf_insert_pfn() for additional info and
+ * also consult the vmf_insert_mixed_prot() documentation when
+ * @pgprot != @vmf->vma->vm_page_prot.
+ *
+ * Return: vm_fault_t value.
+ */
+vm_fault_t vmf_insert_pfn_pmd_prot(struct vm_fault *vmf, pfn_t pfn,
+  pgprot_t pgprot, bool write)
 {
unsigned long addr = vmf->address & PMD_MASK;
struct vm_area_struct *vma = vmf->vma;
-   pgprot_t pgprot = vma->vm_page_prot;
pgtable_t pgtable = NULL;
 
/*
@@ -856,7 +869,7 @@ vm_fault_t vmf_insert_pfn_pmd(struct vm_fault *vmf, pfn_t 
pfn, bool write)
insert_pfn_pmd(vma, addr, vmf->pmd, pfn, pgprot, write, pgtable);
return VM_FAULT_NOPAGE;
 }
-EXPORT_SYMBOL_GPL(vmf_insert_pfn_pmd);
+EXPORT_SYMBOL_GPL(vmf_insert_pfn_pmd_prot);
 
 #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
 static pud_t maybe_pud_mkwrite(pud_t pud, struct vm_area_struct *vma)
@@ -902,11 +915,24 @@ static void insert_pfn_pud(struct vm_area_struct *vma, 
unsigned long addr,
spin_unlock(ptl);
 }
 
-vm_fault_t vmf_insert_pfn_pud(struct vm_fault *vmf, pfn_t pfn, bool write)
+/**
+ * vmf_insert_pfn_pud_prot - insert a pud size pfn
+ * @vmf: Structure describing the fault
+ * @pfn: pfn to insert
+ * @pgprot: page protection to use
+ * @write: whether it's a write fault
+ *
+ * Insert a pud size pfn. See vmf_insert_pfn() for additional info and
+ * also consult the vmf_insert_mixed_prot() documentation when
+ * @pgprot != @vmf->vma->vm_page_prot.
+ *
+ * Return: vm_fault_t value.
+ */
+vm_fault_t vmf_insert_pfn_pud_prot(struct vm_fault *vmf, pfn_t pfn,
+  pgprot_t pgprot, bool write)
 {
unsigned long addr = vmf->address & PUD_MASK;
struct vm_area_struct *vma = vmf->vma;
-   pgprot_t 

[PATCH v6 2/9] mm: Introduce vma_is_special_huge

2020-03-04 Thread VMware
From: Thomas Hellstrom 

For VM_PFNMAP and VM_MIXEDMAP vmas that want to support transhuge pages
and -page table entries, introduce vma_is_special_huge() that takes the
same codepaths as vma_is_dax().

The use of "special" follows the definition in memory.c, vm_normal_page():
"Special" mappings do not wish to be associated with a "struct page"
(either it doesn't exist, or it exists but they don't want to touch it)

For PAGE_SIZE pages, "special" is determined per page table entry to be
able to deal with COW pages. But since we don't have huge COW pages,
we can classify a vma as either "special huge" or "normal huge".

Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: "Matthew Wilcox (Oracle)" 
Cc: "Kirill A. Shutemov" 
Cc: Ralph Campbell 
Cc: "Jérôme Glisse" 
Cc: "Christian König" 
Cc: Dan Williams 
Signed-off-by: Thomas Hellstrom 
Acked-by: Christian König 
---
 include/linux/mm.h | 17 +
 mm/huge_memory.c   |  6 +++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 52269e56c514..e61cf3f609b4 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2863,6 +2863,23 @@ extern long copy_huge_page_from_user(struct page 
*dst_page,
const void __user *usr_src,
unsigned int pages_per_huge_page,
bool allow_pagefault);
+
+/**
+ * vma_is_special_huge - Are transhuge page-table entries considered special?
+ * @vma: Pointer to the struct vm_area_struct to consider
+ *
+ * Whether transhuge page-table entries are considered "special" following
+ * the definition in vm_normal_page().
+ *
+ * Return: true if transhuge page-table entries should be considered special,
+ * false otherwise.
+ */
+static inline bool vma_is_special_huge(const struct vm_area_struct *vma)
+{
+   return vma_is_dax(vma) || (vma->vm_file &&
+  (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP)));
+}
+
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_HUGETLBFS */
 
 #ifdef CONFIG_DEBUG_PAGEALLOC
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index b08b199f9a11..ff7a8b85c3ba 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1802,7 +1802,7 @@ int zap_huge_pmd(struct mmu_gather *tlb, struct 
vm_area_struct *vma,
orig_pmd = pmdp_huge_get_and_clear_full(tlb->mm, addr, pmd,
tlb->fullmm);
tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
-   if (vma_is_dax(vma)) {
+   if (vma_is_special_huge(vma)) {
if (arch_needs_pgtable_deposit())
zap_deposited_table(tlb->mm, pmd);
spin_unlock(ptl);
@@ -2066,7 +2066,7 @@ int zap_huge_pud(struct mmu_gather *tlb, struct 
vm_area_struct *vma,
 */
pudp_huge_get_and_clear_full(tlb->mm, addr, pud, tlb->fullmm);
tlb_remove_pud_tlb_entry(tlb, pud, addr);
-   if (vma_is_dax(vma)) {
+   if (vma_is_special_huge(vma)) {
spin_unlock(ptl);
/* No zero page support yet */
} else {
@@ -2175,7 +2175,7 @@ static void __split_huge_pmd_locked(struct vm_area_struct 
*vma, pmd_t *pmd,
 */
if (arch_needs_pgtable_deposit())
zap_deposited_table(mm, pmd);
-   if (vma_is_dax(vma))
+   if (vma_is_special_huge(vma))
return;
page = pmd_page(_pmd);
if (!PageDirty(page) && pmd_dirty(_pmd))
-- 
2.21.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v6 0/9] Huge page-table entries for TTM

2020-03-04 Thread VMware
In order to reduce CPU usage [1] and in theory TLB misses this patchset enables
huge- and giant page-table entries for TTM and TTM-enabled graphics drivers.

Patch 1 and 2 introduce a vma_is_special_huge() function to make the mm code
take the same path as DAX when splitting huge- and giant page table entries,
(which currently means zapping the page-table entry and rely on re-faulting).

Patch 3 makes the mm code split existing huge page-table entries
on huge_fault fallbacks. Typically on COW or on buffer-objects that want
write-notify. COW and write-notification is always done on the lowest
page-table level. See the patch log message for additional considerations.

Patch 4 introduces functions to allow the graphics drivers to manipulate
the caching- and encryption flags of huge page-table entries without ugly
hacks.

Patch 5 implements the huge_fault handler in TTM.
This enables huge page-table entries, provided that the kernel is configured
to support transhuge pages, either by default or using madvise().
However, they are unlikely to be inserted unless the kernel buffer object
pfns and user-space addresses align perfectly. There are various options
here, but since buffer objects that reside in system pages typically start
at huge page boundaries if they are backed by huge pages, we try to enforce
buffer object starting pfns and user-space addresses to be huge page-size
aligned if their size exceeds a huge page-size. If pud-size transhuge
("giant") pages are enabled by the arch, the same holds for those.

Patch 6 implements a specialized huge_fault handler for vmwgfx.
The vmwgfx driver may perform dirty-tracking and needs some special code
to handle that correctly.

Patch 7 implements a drm helper to align user-space addresses according
to the above scheme, if possible.

Patch 8 implements a TTM range manager for vmwgfx that does the same for
graphics IO memory. This may later be reused by other graphics drivers
if necessary.

Patch 9 finally hooks up the helpers of patch 7 and 8 to the vmwgfx driver.
A similar change is needed for graphics drivers that want a reasonable
likelyhood of actually using huge page-table entries.

If a buffer object size is not huge-page or giant-page aligned,
its size will NOT be inflated by this patchset. This means that the buffer
object tail will use smaller size page-table entries and thus no memory
overhead occurs. Drivers that want to pay the memory overhead price need to
implement their own scheme to inflate buffer-object sizes.

PMD size huge page-table-entries have been tested with vmwgfx and found to
work well both with system memory backed and IO memory backed buffer objects.

PUD size giant page-table-entries have seen limited (fault and COW) testing
using a modified kernel (to support 1GB page allocations) and a fake vmwgfx
TTM memory type. The vmwgfx driver does otherwise not support 1GB-size IO
memory resources.

Comments and suggestions welcome.
Thomas

Changes since RFC:
* Check for buffer objects present in contigous IO Memory (Christian König)
* Rebased on the vmwgfx emulated coherent memory functionality. That rebase
  adds patch 5.
Changes since v1:
* Make the new TTM range manager vmwgfx-specific. (Christian König)
* Minor fixes for configs that don't support or only partially support
  transhuge pages.
Changes since v2:
* Minor coding style and doc fixes in patch 5/9 (Christian König)
* Patch 5/9 doesn't touch mm. Remove from the patch title.
Changes since v3:
* Added reviews and acks
* Implemented ugly but generic ttm_pgprot_is_wrprotecting() instead of arch
  specific code.
Changes since v4:
* Added timings (Andrew Morton)
* Updated function documentation (Andrew Morton)
Changes since v6:
* Fix drm build error with !CONFIG_MMU

[1]
The below test program generates the following gnu time output when run on a
vmwgfx-enabled kernel without the patch series:

4.78user 6.02system 0:10.91elapsed 99%CPU (0avgtext+0avgdata 1624maxresident)k
0inputs+0outputs (0major+640077minor)pagefaults 0swaps

and with the patch series:

1.71user 3.60system 0:05.40elapsed 98%CPU (0avgtext+0avgdata 1656maxresident)k
0inputs+0outputs (0major+20079minor)pagefaults 0swaps

A consistent number of reduced graphics page-faults can be seen with normal
graphics applications, but due to the aggressive buffer object caching in
vmwgfx user-space drivers the CPU time reduction is within the error marginal.

#include 
#include 
#include 
#include 

static void checkerr(int ret, const char *name)
{
  if (ret < 0) {
perror(name);
exit(-1);
  }
}

int main(int agc, const char *argv[])
{
struct drm_mode_create_dumb c_arg = {0};
struct drm_mode_map_dumb m_arg = {0};
struct drm_mode_destroy_dumb d_arg = {0};
int ret, i, fd;
void *map;

fd = open("/dev/dri/card0", O_RDWR);
checkerr(fd, argv[0]);

for (i = 0; i < 1; ++i) {
  c_arg.bpp = 32;
  c_arg.width = 1024;
  c_arg.height = 1024;  
  ret = drmIoctl(fd, 

Re: [PATCHv6,2/6] drm/core: Add drm_afbc_framebuffer and a corresponding helper

2020-03-04 Thread james qian wang (Arm Technology China)
On Tue, Mar 03, 2020 at 01:01:32PM +0100, Andrzej Pietrasiewicz wrote:
> The new struct contains afbc-specific data.
> 
> The new function can be used by drivers which support afbc to complete
> the preparation of struct drm_afbc_framebuffer. It must be called after
> allocating the said struct and calling drm_gem_fb_init_with_funcs().
> 
> Signed-off-by: Andrzej Pietrasiewicz 
> Reported-by: kbuild test robot 
> Reported-by: kbuild test robot 
> ---
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 121 +++
>  include/drm/drm_framebuffer.h|  45 +++
>  include/drm/drm_gem_framebuffer_helper.h |  11 ++
>  3 files changed, 177 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c 
> b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 388a080cd2df..2a30f5b6829f 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -21,6 +21,13 @@
>  #include 
>  #include 
>  
> +#define AFBC_HEADER_SIZE 16
> +#define AFBC_TH_LAYOUT_ALIGNMENT 8
> +#define AFBC_HDR_ALIGN   64
> +#define AFBC_SUPERBLOCK_PIXELS   256
> +#define AFBC_SUPERBLOCK_ALIGNMENT128
> +#define AFBC_TH_BODY_START_ALIGNMENT 4096
> +
>  /**
>   * DOC: overview
>   *
> @@ -302,6 +309,120 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, 
> struct drm_file *file,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
>  
> +/**
> + * drm_afbc_get_superblock_wh - extract afbc block width/height from modifier
> + * @modifier: the modifier to be looked at
> + * @w: address of a place to store the block width
> + * @h: address of a place to store the block height
> + *
> + * Returns: true if the modifier describes a supported block size
> + */
> +bool drm_afbc_get_superblock_wh(u64 modifier, u32 *w, u32 *h)
> +{
> + switch (modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
> + *w = 16;
> + *h = 16;
> + break;
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
> + *w = 32;
> + *h = 8;
> + break;
> + /* no user exists yet - fall through */
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
> + default:
> + DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
> +   modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
> + return false;
> + }
> + return true;
> +}
> +EXPORT_SYMBOL_GPL(drm_afbc_get_superblock_wh);
> +

In this new series, seems we no need to build this get_block_wh to be
an individual func but can directly put them into the following func.

> +static int drm_gem_afbc_min_size(struct drm_device *dev,
> +  const struct drm_mode_fb_cmd2 *mode_cmd,
> +  struct drm_afbc_framebuffer *afbc_fb)
> +{
> + const struct drm_format_info *info;
> + u32 n_blocks, w_alignment, h_alignment, hdr_alignment;
> + u32 tmp_bpp; /* remove when all users properly encode cpp in 
> drm_format_info */
> +
> + if (!drm_afbc_get_superblock_wh(mode_cmd->modifier[0], 
> _fb->block_width, _fb->block_height))
> + return -EINVAL;
> +
> + /* tiled header afbc */
> + w_alignment = afbc_fb->block_width;
> + h_alignment = afbc_fb->block_height;
> + hdr_alignment = AFBC_HDR_ALIGN;
> + if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) {
> + w_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> + h_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> + hdr_alignment = AFBC_TH_BODY_START_ALIGNMENT;
> + }
> +
> + afbc_fb->aligned_width = ALIGN(mode_cmd->width, w_alignment);
> + afbc_fb->aligned_height = ALIGN(mode_cmd->height, h_alignment);
> + afbc_fb->offset = mode_cmd->offsets[0];
> +
> + /* Change to always using info->cpp[0] when all users properly encode 
> it */
> + info = drm_get_format_info(dev, mode_cmd);
> + tmp_bpp = info->cpp[0] ? info->cpp[0] * 8 : afbc_fb->bpp;
> +
> + n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height) / 
> AFBC_SUPERBLOCK_PIXELS;
> + afbc_fb->afbc_size = ALIGN(n_blocks * AFBC_HEADER_SIZE, hdr_alignment);
> + afbc_fb->afbc_size += n_blocks * ALIGN(tmp_bpp * AFBC_SUPERBLOCK_PIXELS 
> / 8, AFBC_SUPERBLOCK_ALIGNMENT);
> +
> + return 0;
> +}
> +
> +/**
> + * drm_gem_fb_afbc_init() - Helper function for drivers using afbc to
> + *   fill and validate all the afbc-specific
> + *   struct drm_afbc_framebuffer members
> + *
> + * @dev: DRM device
> + * @afbc_fb: afbc-specific framebuffer
> + * @mode_cmd: Metadata from the userspace framebuffer creation request
> + * @afbc_fb: afbc framebuffer
> + *
> + * This function can be used by drivers which support afbc to complete
> + * the preparation of struct drm_afbc_framebuffer. It must be called after
> + * 

Re: [PATCHv6,1/6] drm/core: Allow drivers allocate a subclass of struct drm_framebuffer

2020-03-04 Thread james qian wang (Arm Technology China)
On Tue, Mar 03, 2020 at 01:01:31PM +0100, Andrzej Pietrasiewicz wrote:
> Allow allocating a specialized version of struct drm_framebuffer
> by moving the actual fb allocation out of drm_gem_fb_create_with_funcs();
> the respective functions names are adjusted to reflect that fact.
> Please note, though, that standard size checks are performed on buffers,
> so the drm_gem_fb_init_with_funcs() is useful for cases where those
> standard size checks are appropriate or at least don't conflict the
> checks to be performed in the specialized case.
> 
> Thanks to this change the drivers can call drm_gem_fb_init_with_funcs()
> having allocated their special version of struct drm_framebuffer, exactly
> the way the new version of drm_gem_fb_create_with_funcs() does.
> 
> Signed-off-by: Andrzej Pietrasiewicz 
> ---
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 65 +++-
>  include/drm/drm_gem_framebuffer_helper.h |  5 ++
>  2 files changed, 56 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c 
> b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 3a7ace19a902..388a080cd2df 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -55,18 +55,14 @@ struct drm_gem_object *drm_gem_fb_get_obj(struct 
> drm_framebuffer *fb,
>  EXPORT_SYMBOL_GPL(drm_gem_fb_get_obj);
>  
>  static struct drm_framebuffer *
> -drm_gem_fb_alloc(struct drm_device *dev,
> +drm_gem_fb_init(struct drm_device *dev,
> +  struct drm_framebuffer *fb,
>const struct drm_mode_fb_cmd2 *mode_cmd,
>struct drm_gem_object **obj, unsigned int num_planes,
>const struct drm_framebuffer_funcs *funcs)
>  {
> - struct drm_framebuffer *fb;
>   int ret, i;
>  
> - fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> - if (!fb)
> - return ERR_PTR(-ENOMEM);
> -
>   drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
>  
>   for (i = 0; i < num_planes; i++)
> @@ -123,10 +119,13 @@ int drm_gem_fb_create_handle(struct drm_framebuffer 
> *fb, struct drm_file *file,
>  EXPORT_SYMBOL(drm_gem_fb_create_handle);
>  
>  /**
> - * drm_gem_fb_create_with_funcs() - Helper function for the
> - *  _mode_config_funcs.fb_create
> - *  callback
> + * drm_gem_fb_init_with_funcs() - Helper function for implementing
> + * _mode_config_funcs.fb_create
> + * callback in cases when the driver
> + * allocates a subclass of
> + * struct drm_framebuffer
>   * @dev: DRM device
> + * @fb: framebuffer object
>   * @file: DRM file that holds the GEM handle(s) backing the framebuffer
>   * @mode_cmd: Metadata from the userspace framebuffer creation request
>   * @funcs: vtable to be used for the new framebuffer object
> @@ -134,18 +133,21 @@ EXPORT_SYMBOL(drm_gem_fb_create_handle);
>   * This function can be used to set _framebuffer_funcs for drivers that 
> need
>   * custom framebuffer callbacks. Use drm_gem_fb_create() if you don't need to
>   * change _framebuffer_funcs. The function does buffer size validation.
> + * The buffer size validation is for a general case, though, so users should
> + * pay attention to the checks being appropriate for them or, at least,
> + * non-conflicting.
>   *
>   * Returns:
>   * Pointer to a _framebuffer on success or an error pointer on failure.
>   */
>  struct drm_framebuffer *
> -drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> -  const struct drm_mode_fb_cmd2 *mode_cmd,
> -  const struct drm_framebuffer_funcs *funcs)
> +drm_gem_fb_init_with_funcs(struct drm_device *dev, struct drm_framebuffer 
> *fb,
> +struct drm_file *file,
> +const struct drm_mode_fb_cmd2 *mode_cmd,
> +const struct drm_framebuffer_funcs *funcs)

For these two new added funcs: fb_init()/fb_init_with_funcs(), can we change
the return type "struct drm_framebuffer *" to "int".

>  {
>   const struct drm_format_info *info;
>   struct drm_gem_object *objs[4];
> - struct drm_framebuffer *fb;
>   int ret, i;
>  
>   info = drm_get_format_info(dev, mode_cmd);
> @@ -175,7 +177,7 @@ drm_gem_fb_create_with_funcs(struct drm_device *dev, 
> struct drm_file *file,
>   }
>   }
>  
> - fb = drm_gem_fb_alloc(dev, mode_cmd, objs, i, funcs);
> + fb = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs);
>   if (IS_ERR(fb)) {
>   ret = PTR_ERR(fb);
>   goto err_gem_object_put;
> @@ -189,6 +191,41 @@ drm_gem_fb_create_with_funcs(struct drm_device *dev, 
> struct drm_file *file,
>  
>   return ERR_PTR(ret);
>  }
> +EXPORT_SYMBOL_GPL(drm_gem_fb_init_with_funcs);
> +
> +/**
> + * drm_gem_fb_create_with_funcs() - 

Re: [PATCH v6] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges

2020-03-04 Thread Mika Westerberg
Hi,

On Tue, Mar 03, 2020 at 11:10:52AM +0100, Karol Herbst wrote:
> Fixes state transitions of Nvidia Pascal GPUs from D3cold into higher device
> states.

I think it is good to explain bit more here why this fix is needed.

> v2: convert to pci_dev quirk
> put a proper technical explanation of the issue as a in-code comment
> v3: disable it only for certain combinations of intel and nvidia hardware
> v4: simplify quirk by setting flag on the GPU itself
> v5: restructure quirk to make it easier to add new IDs
> fix whitespace issues
> fix potential NULL pointer access
> update the quirk documentation
> v6: move quirk into nouveau

This information typically goes under the '---' line.

> Signed-off-by: Karol Herbst 
> Cc: Bjorn Helgaas 
> Cc: Lyude Paul 
> Cc: Rafael J. Wysocki 
> Cc: Mika Westerberg 

I have few minor comments but regardless,

Reviewed-by: Mika Westerberg 

> Cc: linux-...@vger.kernel.org
> Cc: linux...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=205623
> ---
>  drivers/gpu/drm/nouveau/nouveau_drm.c | 56 +++
>  drivers/pci/pci.c |  8 
>  include/linux/pci.h   |  1 +
>  3 files changed, 65 insertions(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 2cd83849600f..51d3a7ba7731 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -618,6 +618,59 @@ nouveau_drm_device_fini(struct drm_device *dev)
>   kfree(drm);
>  }
>  
> +/*
> + * On some Intel PCIe bridge controllers doing a
> + * D0 -> D3hot -> D3cold -> D0 sequence causes Nvidia GPUs to not reappear.
> + * Skipping the intermediate D3hot step seems to make it work again. Thise is
^
Thise -> This

> + * probably caused by not meeting the expectation the involved AML code has
> + * when the GPU is put into D3hot state before invoking it.
> + *
> + * This leads to various manifestations of this issue:
> + *  - AML code execution to power on the GPU hits an infinite loop (as the
> + *code waits on device memory to change).
> + *  - kernel crashes, as all PCI reads return -1, which most code isn't able
> + *to handle well enough.
> + *
> + * In all cases dmesg will contain at least one line like this:
> + * 'nouveau :01:00.0: Refused to change power state, currently in D3'
> + * followed by a lot of nouveau timeouts.
> + *
> + * In the \_SB.PCI0.PEG0.PG00._OFF code deeper down writes bit 0x80 to the 
> not
> + * documented PCI config space register 0x248 of the Intel PCIe bridge
> + * controller (0x1901) in order to change the state of the PCIe link between
> + * the PCIe port and the GPU. There are alternative code paths using other
> + * registers, which seem to work fine (executed pre Windows 8):
> + *  - 0xbc bit 0x20 (publicly available documentation claims 'reserved')
> + *  - 0xb0 bit 0x10 (link disable)
> + * Changing the conditions inside the firmware by poking into the relevant
> + * addresses does resolve the issue, but it seemed to be ACPI private memory
> + * and not any device accessible memory at all, so there is no portable way 
> of
> + * changing the conditions.
> + * On a XPS 9560 that means bits [0,3] on \CPEX need to be cleared.
> + *
> + * The only systems where this behavior can be seen are hybrid graphics 
> laptops
> + * with a secondary Nvidia Maxwell, Pascal or Turing GPU. Its unclear 
> wheather
 ^^^ 

Its -> It's
wheather -> whether

> + * this issue only occurs in combination with listed Intel PCIe bridge
> + * controllers and the mentioned GPUs or other devices as well.
> + *
> + * documentation on the PCIe bridge controller can be found in the
> + * "7th Generation Intel® Processor Families for H Platforms Datasheet 
> Volume 2"
> + * Section "12 PCI Express* Controller (x16) Registers"
> + */
> +
> +static void quirk_broken_nv_runpm(struct pci_dev *dev)
> +{
> + struct pci_dev *bridge = pci_upstream_bridge(dev);
> +
> + if (!bridge || bridge->vendor != PCI_VENDOR_ID_INTEL)
> + return;
> +
> + switch (bridge->device) {
> + case 0x1901:
> + dev->parent_d3cold = 1;

I think it is better to add

break;

here.

> + }
> +}
> +
>  static int nouveau_drm_probe(struct pci_dev *pdev,
>const struct pci_device_id *pent)
>  {
> @@ -699,6 +752,7 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
>   if (ret)
>   goto fail_drm_dev_init;
>  
> + quirk_broken_nv_runpm(pdev);
>   return 0;
>  
>  fail_drm_dev_init:
> @@ -737,6 +791,8 @@ nouveau_drm_remove(struct pci_dev *pdev)
>  {
>   struct drm_device *dev = pci_get_drvdata(pdev);
>  
> + /* revert our 

Re: [PATCH][next] drm: i915_drm.h: Replace zero-length array with flexible-array member

2020-03-04 Thread Jani Nikula
On Tue, 03 Mar 2020, "Gustavo A. R. Silva"  wrote:
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:

My earlier question about making the change in uapi was really about
potentially bumping up userspace compiler requirements, though I did not
actually say so. :)

I guess effectively uapi already requires C99 to build? And we
(i915_drm.h) have both [0] and []. So go for it.

What's your baseline? I think you've missed one instance of struct
i915_engine_class_instance engines[0];

BR,
Jani.


>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
>
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
>
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
>
> This issue was found with the help of Coccinelle.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  include/uapi/drm/i915_drm.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 2813e579b480..413d923b332a 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1729,7 +1729,7 @@ struct i915_context_engines_load_balance {
>  
>   __u64 mbz64; /* reserved for future use; must be zero */
>  
> - struct i915_engine_class_instance engines[0];
> + struct i915_engine_class_instance engines[];
>  } __attribute__((packed));
>  
>  #define I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(name__, N__) struct { \
> @@ -1767,7 +1767,7 @@ struct i915_context_engines_bond {
>   __u64 flags; /* all undefined flags must be zero */
>   __u64 mbz64[4]; /* reserved for future use; must be zero */
>  
> - struct i915_engine_class_instance engines[0];
> + struct i915_engine_class_instance engines[];
>  } __attribute__((packed));
>  
>  #define I915_DEFINE_CONTEXT_ENGINES_BOND(name__, N__) struct { \

-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH][next] drm/i915: Replace zero-length array with flexible-array member

2020-03-04 Thread Jani Nikula
On Tue, 03 Mar 2020, "Gustavo A. R. Silva"  wrote:
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
>
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
>
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
>
> This issue was found with the help of Coccinelle.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva 

Reviewed-by: Jani Nikula 

> ---
>  drivers/gpu/drm/i915/display/intel_vbt_defs.h | 4 ++--
>  drivers/gpu/drm/i915/gt/intel_lrc.c   | 2 +-
>  drivers/gpu/drm/i915/i915_gpu_error.h | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h 
> b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index 05c7cbe32eb4..aef7fe932d1a 100644
> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> @@ -462,7 +462,7 @@ struct bdb_general_definitions {
>* number = (block_size - sizeof(bdb_general_definitions))/
>*   defs->child_dev_size;
>*/
> - u8 devices[0];
> + u8 devices[];
>  } __packed;
>  
>  /*
> @@ -839,7 +839,7 @@ struct bdb_mipi_config {
>  
>  struct bdb_mipi_sequence {
>   u8 version;
> - u8 data[0]; /* up to 6 variable length blocks */
> + u8 data[]; /* up to 6 variable length blocks */
>  } __packed;
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index b9b3f78f1324..a49ddda649b9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -216,7 +216,7 @@ struct virtual_engine {
>  
>   /* And finally, which physical engines this virtual engine maps onto. */
>   unsigned int num_siblings;
> - struct intel_engine_cs *siblings[0];
> + struct intel_engine_cs *siblings[];
>  };
>  
>  static struct virtual_engine *to_virtual_engine(struct intel_engine_cs 
> *engine)
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h 
> b/drivers/gpu/drm/i915/i915_gpu_error.h
> index 0d1f6c8ff355..5a6561f7a210 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.h
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.h
> @@ -42,7 +42,7 @@ struct i915_vma_coredump {
>   int num_pages;
>   int page_count;
>   int unused;
> - u32 *pages[0];
> + u32 *pages[];
>  };
>  
>  struct i915_request_coredump {

-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 33/33] drm/panel-simple: Fix dotclock for LG ACX467AKM-7

2020-03-04 Thread Linus Walleij
On Mon, Mar 2, 2020 at 9:49 PM Jonathan Marek  wrote:

> This is a command mode panel and the the msm/mdp5 driver uses the
> vrefresh field for the actual refresh rate, while the dotclock field is
> used for the DSI clocks. The dotclock needed to be a bit higher than
> necessary otherwise the panel would not work.

I don't know if this predates the support for defining DSI clocks
but for what we have in the kernel right now this is wrong.

struct mipi_dsi_device contains:

 * @hs_rate: maximum lane frequency for high speed mode in hertz, this should
 * be set to the real limits of the hardware, zero is only accepted for
 * legacy drivers
 * @lp_rate: maximum lane frequency for low power mode in hertz, this should
 * be set to the real limits of the hardware, zero is only accepted for
 * legacy drivers

The MDP driver should use these frequencies for a DSI command
mode panel, and the panel driver should define them.

These two clocks are/can be/should be completely orthogonal to
the dotclock/pixelclock inside the panel, which is likely driven from
its own crystal directly from the panel-internal framebuffer.

Yours,
Linus Walleij
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] RFC: dma-buf: Add an API for importing and exporting sync files

2020-03-04 Thread Christian König

Am 03.03.20 um 20:10 schrieb Jason Ekstrand:

On Thu, Feb 27, 2020 at 2:28 AM Christian König
 wrote:

[SNIP]

However, I'm not sure what the best way is to do garbage collection on
that so that we don't get an impossibly list of fence arrays.

Exactly yes. That's also the reason why the dma_fence_chain container I
came up with for the sync timeline stuff has such a rather sophisticated
garbage collection.

When some of the included fences signal you need to free up the
array/chain and make sure that the memory for the container can be reused.

Currently (as of v2), I'm using dma_fence_array and being careful to
not bother constructing one if there's only one fence in play.  Is
this insufficient?  If so, maybe we should consider improving
dma_fence_array.


That still won't work correctly in all cases. See the problem is not 
only optimization, but also avoiding situations where userspace can 
abuse the interface to do nasty things.


For example if userspace just calls that function in a loop you can 
create a long chain of dma_fence_array objects.


If that chain is then suddenly released the recursive dropping of 
references can overwrite the kernel stack.


For reference see what dance is necessary in the dma_fence_chain_release 
function to avoid that:
    /* Manually unlink the chain as much as possible to avoid 
recursion

 * and potential stack overflow.
 */
    while ((prev = rcu_dereference_protected(chain->prev, true))) {



It took me quite a while to figure out how to do this without causing 
issues. But I don't see how this would be possible for dma_fence_array.


As far as I can see the only real option to implement this would be to 
change the dma_resv object container so that you can add fences without 
overriding existing ones.


For shared fences that can be done relative easily, but I absolutely 
don't see how to do this for exclusive ones without a larger rework.



   (Note
the dma_resv has a lock that needs to be taken before adding an
exclusive fence, might be useful). Some code that does a thing like
this is __dma_resv_make_exclusive in
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c

Wanted to move that into dma_resv.c for quite a while since there are
quite a few other cases where we need this.

I've roughly done that.  The primary difference is that my version
takes an optional additional fence to add to the array.  This makes it
a bit more complicated but I think I got it mostly right.

I've also written userspace code which exercises this and it seems to
work.  Hopefully, that will give a better idea of what I'm trying to
accomplish.


Yes, that is indeed a really nice to have feature.

Regards,
Christian.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 2/2] drm/virtio: add case for shmem objects in virtio_gpu_cleanup_object(..)

2020-03-04 Thread Gerd Hoffmann
  Hi,

> + drm_gem_shmem_free_object(>base.base);
>   }
> +
>   virtio_gpu_resource_id_put(vgdev, bo->hw_res_handle);

use-after-free here.

cheers,
  Gerd

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [virtio-dev] [PATCH v2 4/4] drm/virtio: Support virtgpu exported resources

2020-03-04 Thread Gerd Hoffmann
On Tue, Mar 03, 2020 at 11:42:22AM +0900, David Stevens wrote:
> > cmd_p->hdr.ctx_id =
> >
> > Before this completion of this hypercall, this resource can be
> > considered context local, while afterward it can be considered
> > "exported".
> 
> Maybe I'm misunderstanding render contexts, but exporting a resource
> doesn't seem related to render contexts.

It isn't indeed.  Binding resources to contexts might need dma-buf
imports/exports on the host side, but that is another story and not
related to dma-buf exports inside the guest.

cheers,
  Gerd

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 4/4] drm/virtio: Support virtgpu exported resources

2020-03-04 Thread Gerd Hoffmann
  Hi,

> + if (vgdev->has_resource_assign_uuid) {
> + spin_lock(>resource_export_lock);
> + if (bo->uuid_state == UUID_NOT_INITIALIZED) {
> + bo->uuid_state = UUID_INITIALIZING;
> + needs_init = true;
> + }
> + spin_unlock(>resource_export_lock);
> +
> + if (needs_init) {
> + ret = virtio_gpu_cmd_resource_assign_uuid(vgdev, bo);

You can submit a fenced command, then wait on the fence here.  Removes
the need for UUID_INITIALIZING.

Also note that this function will be called only once, on the first
export.  When exporting the same object again drm will simply reuse
the existing dmabuf.  You can drop UUID_NOT_INITIALIZED and needs_init.

So you are left with only two uuid_state states.  You could turn uuid
into a pointer, so it gets only allocated when needed.  Also uuid ==
NULL can be used for "uuid not available" then.

cheers,
  Gerd

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel