Re: [PATCH] drm/mediatek: Check return value of mtk_drm_ddp_comp_for_plane.
Hi, Pi-Hsun:
On Mon, 2019-11-18 at 14:18 +0800, Pi-Hsun Shih wrote:
> The mtk_drm_ddp_comp_for_plane can return NULL, but the usage doesn't
> check for it. Add check for it.
Reviewed-by: CK Hu
>
> Fixes: d6b53f68356f ("drm/mediatek: Add helper to get component for a plane")
> Signed-off-by: Pi-Hsun Shih
> ---
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 13 +
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index f80a8ba75977..4c4f976c994e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -310,7 +310,9 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc
> *mtk_crtc)
>
> plane_state = to_mtk_plane_state(plane->state);
> comp = mtk_drm_ddp_comp_for_plane(crtc, plane, _layer);
> - mtk_ddp_comp_layer_config(comp, local_layer, plane_state);
> + if (comp)
> + mtk_ddp_comp_layer_config(comp, local_layer,
> + plane_state);
> }
>
> return 0;
> @@ -386,8 +388,9 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
> comp = mtk_drm_ddp_comp_for_plane(crtc, plane,
> _layer);
>
> - mtk_ddp_comp_layer_config(comp, local_layer,
> - plane_state);
> + if (comp)
> + mtk_ddp_comp_layer_config(comp, local_layer,
> + plane_state);
> plane_state->pending.config = false;
> }
> mtk_crtc->pending_planes = false;
> @@ -401,7 +404,9 @@ int mtk_drm_crtc_plane_check(struct drm_crtc *crtc,
> struct drm_plane *plane,
> struct mtk_ddp_comp *comp;
>
> comp = mtk_drm_ddp_comp_for_plane(crtc, plane, _layer);
> - return mtk_ddp_comp_layer_check(comp, local_layer, state);
> + if (comp)
> + return mtk_ddp_comp_layer_check(comp, local_layer, state);
> + return 0;
> }
>
> static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc,
>
> base-commit: 5a6fcbeabe3e20459ed8504690b2515dacc5246f
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/8] drm/mediatek: don't open-code drm_gem_fb_create
Hi, Daniel:
On Fri, 2019-11-15 at 10:21 +0100, Daniel Vetter wrote:
> Aside: There's a few other fb_create implementations which
> simply check for valid buffer format (or an approximation thereof),
> and then call drm_gem_fb_create. For atomic drivers at least we could
> walk all planes and make sure the format/modifier combo is valid,
> and remove even more code.
>
> For non-atomic drivers that's not possible, since the format list for
> the primary buffer might be garbage (and most likely it is).
>
> Also delete mtk_drm_fb.[hc] since it would now only contain one
> function.
Acked-by: CK Hu
>
> Signed-off-by: Daniel Vetter
> Cc: CK Hu
> Cc: Philipp Zabel
> Cc: Matthias Brugger
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-media...@lists.infradead.org
> ---
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 16 -
> drivers/gpu/drm/mediatek/mtk_drm_fb.c| 92
> drivers/gpu/drm/mediatek/mtk_drm_fb.h| 13
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 1 -
> 4 files changed, 15 insertions(+), 107 deletions(-)
> delete mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fb.c
> delete mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fb.h
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 84d14213d992..2b1c122066ea 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -16,8 +16,10 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -27,7 +29,6 @@
> #include "mtk_drm_ddp.h"
> #include "mtk_drm_ddp_comp.h"
> #include "mtk_drm_drv.h"
> -#include "mtk_drm_fb.h"
> #include "mtk_drm_gem.h"
>
> #define DRIVER_NAME "mediatek"
> @@ -115,6 +116,19 @@ static int mtk_atomic_commit(struct drm_device *drm,
> return 0;
> }
>
> +static struct drm_framebuffer *
> +mtk_drm_mode_fb_create(struct drm_device *dev,
> +struct drm_file *file,
> +const struct drm_mode_fb_cmd2 *cmd)
> +{
> + const struct drm_format_info *info = drm_get_format_info(dev, cmd);
> +
> + if (info->num_planes != 1)
> + return ERR_PTR(-EINVAL);
> +
> + return drm_gem_fb_create(dev, file, cmd);
> +}
> +
> static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = {
> .fb_create = mtk_drm_mode_fb_create,
> .atomic_check = drm_atomic_helper_check,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> deleted file mode 100644
> index 3f230a28a2dc..
> --- a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> +++ /dev/null
> @@ -1,92 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Copyright (c) 2015 MediaTek Inc.
> - */
> -
> -#include
> -#include
> -
> -#include
> -#include
> -#include
> -#include
> -#include
> -
> -#include "mtk_drm_drv.h"
> -#include "mtk_drm_fb.h"
> -#include "mtk_drm_gem.h"
> -
> -static const struct drm_framebuffer_funcs mtk_drm_fb_funcs = {
> - .create_handle = drm_gem_fb_create_handle,
> - .destroy = drm_gem_fb_destroy,
> -};
> -
> -static struct drm_framebuffer *mtk_drm_framebuffer_init(struct drm_device
> *dev,
> - const struct drm_mode_fb_cmd2 *mode,
> - struct drm_gem_object *obj)
> -{
> - const struct drm_format_info *info = drm_get_format_info(dev, mode);
> - struct drm_framebuffer *fb;
> - int ret;
> -
> - if (info->num_planes != 1)
> - return ERR_PTR(-EINVAL);
> -
> - fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> - if (!fb)
> - return ERR_PTR(-ENOMEM);
> -
> - drm_helper_mode_fill_fb_struct(dev, fb, mode);
> -
> - fb->obj[0] = obj;
> -
> - ret = drm_framebuffer_init(dev, fb, _drm_fb_funcs);
> - if (ret) {
> - DRM_ERROR("failed to initialize framebuffer\n");
> - kfree(fb);
> - return ERR_PTR(ret);
> - }
> -
> - return fb;
> -}
> -
> -struct drm_framebuffer *mtk_drm_mode_fb_create(struct drm_device *dev,
> -struct drm_file *file,
> -const struct drm_mode_fb_cmd2
> *cmd)
> -{
> - const struct drm_format_info *info = drm_get_format_info(dev, cmd);
> - struct drm_framebuffer *fb;
> - struct drm_gem_object *gem;
> - unsigned int width = cmd->width;
> - unsigned int height = cmd->height;
> - unsigned int size, bpp;
> - int ret;
> -
> - if (info->num_planes != 1)
> - return ERR_PTR(-EINVAL);
> -
> - gem = drm_gem_object_lookup(file, cmd->handles[0]);
> - if (!gem)
> - return ERR_PTR(-ENOENT);
> -
> - bpp = info->cpp[0];
> - size = (height - 1) * cmd->pitches[0] + width * bpp;
> - size += cmd->offsets[0];
> -
> - if (gem->size < size) {
> - ret = -EINVAL;
> - goto
[PATCH 2/2] drm/mcde: Do not needlessly logically and with 3
The i index i always 0..3 in these statements so there
is no need to tag "& 3" to clamp it to 3 here. Make
the operator precedence explicit even if it's correct
as it is, the paranthesis creates less cognitive stress
for humans.
Cc: Stephan Gerhold
Signed-off-by: Linus Walleij
---
drivers/gpu/drm/mcde/mcde_dsi.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c
index dc07b534f01f..21cee4d9d2fd 100644
--- a/drivers/gpu/drm/mcde/mcde_dsi.c
+++ b/drivers/gpu/drm/mcde/mcde_dsi.c
@@ -237,25 +237,25 @@ static ssize_t mcde_dsi_host_transfer(struct
mipi_dsi_host *host,
if (txlen > 0) {
val = 0;
for (i = 0; i < 4 && i < txlen; i++)
- val |= tx[i] << (i & 3) * 8;
+ val |= tx[i] << (i * 8);
}
writel(val, d->regs + DSI_DIRECT_CMD_WRDAT0);
if (txlen > 4) {
val = 0;
for (i = 0; i < 4 && (i + 4) < txlen; i++)
- val |= tx[i + 4] << (i & 3) * 8;
+ val |= tx[i + 4] << (i * 8);
writel(val, d->regs + DSI_DIRECT_CMD_WRDAT1);
}
if (txlen > 8) {
val = 0;
for (i = 0; i < 4 && (i + 8) < txlen; i++)
- val |= tx[i + 8] << (i & 3) * 8;
+ val |= tx[i + 8] << (i * 8);
writel(val, d->regs + DSI_DIRECT_CMD_WRDAT2);
}
if (txlen > 12) {
val = 0;
for (i = 0; i < 4 && (i + 12) < txlen; i++)
- val |= tx[i + 12] << (i & 3) * 8;
+ val |= tx[i + 12] << (i * 8);
writel(val, d->regs + DSI_DIRECT_CMD_WRDAT3);
}
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/mcde: Reuse global DSI command defs
The MCDE DSI include file redefines some commands that already exist in the common header. Cc: Stephan Gerhold Signed-off-by: Linus Walleij --- drivers/gpu/drm/mcde/mcde_dsi.c | 2 +- drivers/gpu/drm/mcde/mcde_dsi_regs.h | 11 --- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c index d4a12fe7ff01..dc07b534f01f 100644 --- a/drivers/gpu/drm/mcde/mcde_dsi.c +++ b/drivers/gpu/drm/mcde/mcde_dsi.c @@ -350,7 +350,7 @@ void mcde_dsi_te_request(struct mipi_dsi_device *mdsi) val |= 0 << DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_ID_SHIFT; val |= 2 << DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_SIZE_SHIFT; val |= DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_LP_EN; - val |= DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_DCS_SHORT_WRITE_1 << + val |= MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM << DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_SHIFT; writel(val, d->regs + DSI_DIRECT_CMD_MAIN_SETTINGS); diff --git a/drivers/gpu/drm/mcde/mcde_dsi_regs.h b/drivers/gpu/drm/mcde/mcde_dsi_regs.h index b03a336c235f..8089db805c57 100644 --- a/drivers/gpu/drm/mcde/mcde_dsi_regs.h +++ b/drivers/gpu/drm/mcde/mcde_dsi_regs.h @@ -123,17 +123,6 @@ #define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_LONGNOTSHORT BIT(3) #define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_SHIFT 8 #define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_MASK 0x3F00 -#define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_TURN_ON_PERIPHERAL 50 -#define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_SHUT_DOWN_PERIPHERAL 34 -#define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_GENERIC_SHORT_WRITE_0 3 -#define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_GENERIC_SHORT_WRITE_1 19 -#define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_GENERIC_SHORT_WRITE_2 35 -#define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_GENERIC_LONG_WRITE 41 -#define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_DCS_SHORT_WRITE_0 5 -#define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_DCS_SHORT_WRITE_1 21 -#define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_DCS_LONG_WRITE 57 -#define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_DCS_READ 6 -#define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_SET_MAX_PKT_SIZE 55 #define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_ID_SHIFT 14 #define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_SIZE_SHIFT 16 #define DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_LP_EN BIT(21) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/imx: fix memory leak in imx_pd_bind
Hi Navid,
On 19-11-21 12:31, Navid Emamdoost wrote:
> On Fri, Oct 4, 2019 at 2:09 PM Navid Emamdoost
> wrote:
> >
> > In imx_pd_bind, the duplicated memory for imxpd->edid via kmemdup should
> > be released in drm_of_find_panel_or_bridge or imx_pd_register fail.
> >
> > Fixes: ebc944613567 ("drm: convert drivers to use
> > drm_of_find_panel_or_bridge")
> > Fixes: 19022aaae677 ("staging: drm/imx: Add parallel display support")
> > Signed-off-by: Navid Emamdoost
> > ---
>
> Would you please review this patch?
>
> Thanks,
I currently work on the drm/imx driver(s) to avoid errors like [1].
Hopefully I have a working version till next week. There I fixed this
issue by using the devm_kmemdup() and dropped the explicit kfree()
within unbind().
[1] https://www.spinics.net/lists/dri-devel/msg189388.html
Regards,
Marco
>
> > drivers/gpu/drm/imx/parallel-display.c | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/imx/parallel-display.c
> > b/drivers/gpu/drm/imx/parallel-display.c
> > index e7ce17503ae1..9522d2cb0ad5 100644
> > --- a/drivers/gpu/drm/imx/parallel-display.c
> > +++ b/drivers/gpu/drm/imx/parallel-display.c
> > @@ -227,14 +227,18 @@ static int imx_pd_bind(struct device *dev, struct
> > device *master, void *data)
> >
> > /* port@1 is the output port */
> > ret = drm_of_find_panel_or_bridge(np, 1, 0, >panel,
> > >bridge);
> > - if (ret && ret != -ENODEV)
> > + if (ret && ret != -ENODEV) {
> > + kfree(imxpd->edid);
> > return ret;
> > + }
> >
> > imxpd->dev = dev;
> >
> > ret = imx_pd_register(drm, imxpd);
> > - if (ret)
> > + if (ret) {
> > + kfree(imxpd->edid);
> > return ret;
> > + }
> >
> > dev_set_drvdata(dev, imxpd);
> >
> > --
> > 2.17.1
> >
>
>
> --
> Navid.
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110605] "*ERROR* Waiting for fences timed out." happens every time when I select "Story" in the main game menu RE2.
https://bugs.freedesktop.org/show_bug.cgi?id=110605 NAGENDRA changed: What|Removed |Added Alias||NAGENDRA -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm: call drm_gem_object_funcs.mmap with fake offset
The fake offset is going to stay, so change the calling convention for
drm_gem_object_funcs.mmap to include the fake offset. Update all users
accordingly.
Note that this reverts 83b8a6f242ea ("drm/gem: Fix mmap fake offset
handling for drm_gem_object_funcs.mmap") and on top then adds the fake
offset to drm_gem_prime_mmap to make sure all paths leading to
obj->funcs->mmap are consistent.
Fixes: 83b8a6f242ea ("drm/gem: Fix mmap fake offset handling for
drm_gem_object_funcs.mmap")
Cc: Daniel Vetter
Cc: Rob Herring
Signed-off-by: Gerd Hoffmann
Reviewed-by: Daniel Vetter
---
include/drm/drm_gem.h | 4 +---
drivers/gpu/drm/drm_gem.c | 3 ---
drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++
drivers/gpu/drm/drm_prime.c| 3 +++
drivers/gpu/drm/ttm/ttm_bo_vm.c| 7 ---
5 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 97a48165642c..0b375069cd48 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -159,9 +159,7 @@ struct drm_gem_object_funcs {
*
* The callback is used by by both drm_gem_mmap_obj() and
* drm_gem_prime_mmap(). When @mmap is present @vm_ops is not
-* used, the @mmap callback must set vma->vm_ops instead. The @mmap
-* callback is always called with a 0 offset. The caller will remove
-* the fake offset as necessary.
+* used, the @mmap callback must set vma->vm_ops instead.
*/
int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 2f2b889096b0..56f42e0f2584 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1106,9 +1106,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned
long obj_size,
return -EINVAL;
if (obj->funcs && obj->funcs->mmap) {
- /* Remove the fake offset */
- vma->vm_pgoff -= drm_vma_node_start(>vma_node);
-
ret = obj->funcs->mmap(obj, vma);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 0810d3ef6961..a421a2eed48a 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -528,6 +528,9 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct
vm_area_struct *vma)
struct drm_gem_shmem_object *shmem;
int ret;
+ /* Remove the fake offset */
+ vma->vm_pgoff -= drm_vma_node_start(>vma_node);
+
shmem = to_drm_gem_shmem_obj(obj);
ret = drm_gem_shmem_get_pages(shmem);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 0814211b0f3f..a9633bd241bb 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -714,6 +714,9 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct
vm_area_struct *vma)
int ret;
if (obj->funcs && obj->funcs->mmap) {
+ /* Add the fake offset */
+ vma->vm_pgoff += drm_vma_node_start(>vma_node);
+
ret = obj->funcs->mmap(obj, vma);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index e6495ca2630b..3e8c3de91ae4 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -514,13 +514,6 @@ EXPORT_SYMBOL(ttm_bo_mmap);
int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
{
ttm_bo_get(bo);
-
- /*
-* FIXME: _gem_object_funcs.mmap is called with the fake offset
-* removed. Add it back here until the rest of TTM works without it.
-*/
- vma->vm_pgoff += drm_vma_node_start(>base.vma_node);
-
ttm_bo_mmap_vma_setup(bo, vma);
return 0;
}
--
2.18.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] drm: share address space for dma bufs
Use the shared address space of the drm device (see drm_open() in
drm_file.c) for dma-bufs too. That removes a difference betweem drm
device mmap vmas and dma-buf mmap vmas and fixes corner cases like
dropping ptes (using madvise(DONTNEED) for example) not working
properly.
Also remove amdgpu driver's private dmabuf update. It is not needed
any more now that we are doing this for everybody.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 +---
drivers/gpu/drm/drm_prime.c | 4 +++-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index d5bcdfefbad6..586db4fb46bd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -361,10 +361,8 @@ struct dma_buf *amdgpu_gem_prime_export(struct
drm_gem_object *gobj,
return ERR_PTR(-EPERM);
buf = drm_gem_prime_export(gobj, flags);
- if (!IS_ERR(buf)) {
- buf->file->f_mapping = gobj->dev->anon_inode->i_mapping;
+ if (!IS_ERR(buf))
buf->ops = _dmabuf_ops;
- }
return buf;
}
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index a9633bd241bb..c3fc341453c0 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -240,6 +240,7 @@ void drm_prime_destroy_file_private(struct
drm_prime_file_private *prime_fpriv)
struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
struct dma_buf_export_info *exp_info)
{
+ struct drm_gem_object *obj = exp_info->priv;
struct dma_buf *dma_buf;
dma_buf = dma_buf_export(exp_info);
@@ -247,7 +248,8 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device
*dev,
return dma_buf;
drm_dev_get(dev);
- drm_gem_object_get(exp_info->priv);
+ drm_gem_object_get(obj);
+ dma_buf->file->f_mapping = obj->dev->anon_inode->i_mapping;
return dma_buf;
}
--
2.18.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/2] drm: mmap fixups
Tested on qemu, with bochs (vram helpers) and cirrus (shmem helpers). Cc'ing intel-gfx for CI coverage. Gerd Hoffmann (2): drm: call drm_gem_object_funcs.mmap with fake offset drm: share address space for dma bufs include/drm/drm_gem.h | 4 +--- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 +--- drivers/gpu/drm/drm_gem.c | 3 --- drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++ drivers/gpu/drm/drm_prime.c | 7 ++- drivers/gpu/drm/ttm/ttm_bo_vm.c | 7 --- 6 files changed, 11 insertions(+), 17 deletions(-) -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/2] drm: share address space for dma bufs
On Thu, Nov 21, 2019 at 04:42:10PM +, Ruhl, Michael J wrote: > >-Original Message- > >From: Intel-gfx On Behalf Of Gerd > >Hoffmann > >Sent: Thursday, November 21, 2019 5:38 AM > >To: dri-devel@lists.freedesktop.org > >Cc: David Airlie ; intel-...@lists.freedesktop.org; open > >list > >; Maxime Ripard ; Gerd > >Hoffmann > >Subject: [Intel-gfx] [PATCH 2/2] drm: share address space for dma bufs > > > >Use the shared address space of the drm device (see drm_open() in > >drm_file.c) for dma-bufs too. That removes a difference betweem drm > >device mmap vmas and dma-buf mmap vmas and fixes corner cases like > >unmaps not working properly. > > Hi Gerd, > > Just want to make sure I understand this... > > So unmaps will not work correctly for mappings when a driver does a > drm_vma_node_unamp()? Completely removing the mapping (aka munmap syscall) works fine. Zapping the pte's (using madvise(dontneed) for example) doesn't. > This is a day one bug? I guess so, but I'll leave that to others being active longer than me in drm hacking to answer ... cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH AUTOSEL 4.14 104/127] gpu: ipu-v3: pre: don't trigger update if buffer address doesn't change
From: Lucas Stach
[ Upstream commit eb0200a4357da100064971689d3a0e9e3cf57f33 ]
On a NOP double buffer update where current buffer address is the same
as the next buffer address, the SDW_UPDATE bit clears too late. As we
are now using this bit to determine when it is safe to signal flip
completion to userspace this will delay completion of atomic commits
where one plane doesn't change the buffer by a whole frame period.
Fix this by remembering the last buffer address and just skip the
double buffer update if it would not change the buffer address.
Signed-off-by: Lucas Stach
[p.za...@pengutronix.de: initialize last_bufaddr in ipu_pre_configure]
Signed-off-by: Philipp Zabel
Signed-off-by: Sasha Levin
---
drivers/gpu/ipu-v3/ipu-pre.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/ipu-v3/ipu-pre.c b/drivers/gpu/ipu-v3/ipu-pre.c
index 1d1612e28854b..6fd4af647f599 100644
--- a/drivers/gpu/ipu-v3/ipu-pre.c
+++ b/drivers/gpu/ipu-v3/ipu-pre.c
@@ -102,6 +102,7 @@ struct ipu_pre {
void*buffer_virt;
boolin_use;
unsigned intsafe_window_end;
+ unsigned intlast_bufaddr;
};
static DEFINE_MUTEX(ipu_pre_list_mutex);
@@ -177,6 +178,7 @@ void ipu_pre_configure(struct ipu_pre *pre, unsigned int
width,
writel(bufaddr, pre->regs + IPU_PRE_CUR_BUF);
writel(bufaddr, pre->regs + IPU_PRE_NEXT_BUF);
+ pre->last_bufaddr = bufaddr;
val = IPU_PRE_PREF_ENG_CTRL_INPUT_PIXEL_FORMAT(0) |
IPU_PRE_PREF_ENG_CTRL_INPUT_ACTIVE_BPP(active_bpp) |
@@ -218,7 +220,11 @@ void ipu_pre_update(struct ipu_pre *pre, unsigned int
bufaddr)
unsigned short current_yblock;
u32 val;
+ if (bufaddr == pre->last_bufaddr)
+ return;
+
writel(bufaddr, pre->regs + IPU_PRE_NEXT_BUF);
+ pre->last_bufaddr = bufaddr;
do {
if (time_after(jiffies, timeout)) {
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH AUTOSEL 4.19 176/219] gpu: ipu-v3: pre: don't trigger update if buffer address doesn't change
From: Lucas Stach
[ Upstream commit eb0200a4357da100064971689d3a0e9e3cf57f33 ]
On a NOP double buffer update where current buffer address is the same
as the next buffer address, the SDW_UPDATE bit clears too late. As we
are now using this bit to determine when it is safe to signal flip
completion to userspace this will delay completion of atomic commits
where one plane doesn't change the buffer by a whole frame period.
Fix this by remembering the last buffer address and just skip the
double buffer update if it would not change the buffer address.
Signed-off-by: Lucas Stach
[p.za...@pengutronix.de: initialize last_bufaddr in ipu_pre_configure]
Signed-off-by: Philipp Zabel
Signed-off-by: Sasha Levin
---
drivers/gpu/ipu-v3/ipu-pre.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/ipu-v3/ipu-pre.c b/drivers/gpu/ipu-v3/ipu-pre.c
index 2f8db9d625514..4a28f3fbb0a28 100644
--- a/drivers/gpu/ipu-v3/ipu-pre.c
+++ b/drivers/gpu/ipu-v3/ipu-pre.c
@@ -106,6 +106,7 @@ struct ipu_pre {
void*buffer_virt;
boolin_use;
unsigned intsafe_window_end;
+ unsigned intlast_bufaddr;
};
static DEFINE_MUTEX(ipu_pre_list_mutex);
@@ -185,6 +186,7 @@ void ipu_pre_configure(struct ipu_pre *pre, unsigned int
width,
writel(bufaddr, pre->regs + IPU_PRE_CUR_BUF);
writel(bufaddr, pre->regs + IPU_PRE_NEXT_BUF);
+ pre->last_bufaddr = bufaddr;
val = IPU_PRE_PREF_ENG_CTRL_INPUT_PIXEL_FORMAT(0) |
IPU_PRE_PREF_ENG_CTRL_INPUT_ACTIVE_BPP(active_bpp) |
@@ -242,7 +244,11 @@ void ipu_pre_update(struct ipu_pre *pre, unsigned int
bufaddr)
unsigned short current_yblock;
u32 val;
+ if (bufaddr == pre->last_bufaddr)
+ return;
+
writel(bufaddr, pre->regs + IPU_PRE_NEXT_BUF);
+ pre->last_bufaddr = bufaddr;
do {
if (time_after(jiffies, timeout)) {
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 02/24] mm/gup: factor out duplicate code from four routines
On 11/21/19 1:54 AM, Jan Kara wrote: On Thu 21-11-19 00:29:59, John Hubbard wrote: Otherwise this looks fine and might be a worthwhile cleanup to feed Andrew for 5.5 independent of the gut of the changes. Reviewed-by: Christoph Hellwig Thanks for the reviews! Say, it sounds like your view here is that this series should be targeted at 5.6 (not 5.5), is that what you have in mind? And get the preparatory patches (1-9, and maybe even 10-16) into 5.5? One more note :) If you are going to push pin_user_pages() interfaces (which I'm fine with), it would probably make sense to push also the put_user_pages() -> unpin_user_pages() renaming so that that inconsistency in naming does not exist in the released upstream kernel. Honza Yes, that's what this patch series does. But I'm not sure if "push" here means, "push out: defer to 5.6", "push (now) into 5.5", or "advocate for"? I will note that it's not going to be easy to rename in one step, now that this is being split up. Because various put_user_pages()-based items are going into 5.5 via different maintainer trees now. Probably I'd need to introduce unpin_user_page() alongside put_user_page()...thoughts? thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: 2989f64510 ("dma-buf: Add selftests for dma-fence"): WARNING: CPU: 0 PID: 1 at lib/debugobjects.c:524 __debug_object_init
On Thu, Nov 21, 2019 at 07:26:05AM +, Chris Wilson wrote: > Quoting kernel test robot (2019-11-21 07:19:43) > > Greetings, > > > > 0day kernel testing robot got the below dmesg and the first bad commit is > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > > > > commit 2989f6451084aed3f8cc9992477f7a9bf57a3716 > > Author: Chris Wilson > > AuthorDate: Mon Aug 19 10:59:27 2019 +0100 > > Commit: Chris Wilson > > CommitDate: Mon Aug 19 18:09:46 2019 +0100 > > That's a belated report, fixed by Hi Chris, thanks for the feedback, we will double check this report and provide update later. > > commit 6ac3a0ebfcc2f0c75ca0ca6974389ce421aa5cbd > Author: Chris Wilson > Date: Tue Aug 20 13:21:18 2019 +0100 > > dmabuf: Mark up onstack timer for selftests > > No? > -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
On Thu, 2019-11-21 at 10:28 +0100, Daniel Vetter wrote: > On Thu, Nov 21, 2019 at 9:26 AM Timo Aaltonen > wrote: > > On 9.10.2019 18.50, Daniel Vetter wrote: > > > On Thu, Oct 03, 2019 at 11:31:25AM -0700, Juston Li wrote: > > > > From: Daniel Stone > > > > > > > > getfb2 allows us to pass multiple planes and modifiers, just > > > > like addfb2 > > > > over addfb. > > > > > > > > Changes since v1: > > > > - unused modifiers set to 0 instead of DRM_FORMAT_MOD_INVALID > > > > - update ioctl number > > > > > > > > Signed-off-by: Daniel Stone > > > > Signed-off-by: Juston Li > > > > > > Looks all good from a very quick glance (kernel, libdrm, igt), > > > but where's > > > the userspace? Link to weston/drm_hwc/whatever MR good enough. > > > -Daniel > > > > xserver too > > > > https://lists.x.org/archives/xorg-devel/2018-March/056363.html > > > > to fix > > > > https://gitlab.freedesktop.org/xorg/xserver/issues/33 > > > > which forces Ubuntu to disable CCS compression, and I'd like to get > > rid > > of that patch. > > > > Thanks Juston for pushing this! > > Acked-by: Daniel Vetter > > ... but someone needs to review all the patches and make sure kernel > patch + igt work and pass intel CI and then push it all, and given > the > pile of committers we have that's not me I think. Just in case people > expect me to push this forward, I only jumped in here to make sure > new > uapi is done by the book and checks all the boxes. > -Daniel Thanks for clarifying Daniel, I'll try to find someone to review. Juston ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[git pull] drm fixes for 5.4
Hey Linus, Two sets of fixes in here, one for amdgpu, and one for i915. The amdgpu ones are pretty small, i915's CI system seems to have a few problems in the last week or so, there is one major regression fix for fb_mmap, but there are a bunch of other issues fixed in there as well, oops, screen flashes and rcu related. Dave. drm-fixes-2019-11-22: drm fixes for 5.4 amdgpu: - Remove experimental flag for navi14 - Fix confusing power message failures on older VI parts - Hang fix for gfxoff when using the read register interface - Two stability regression fixes for Raven i915: - Fix kernel oops on dumb_create ioctl on no crtc situation - Fix bad ugly colored flash on VLV/CHV related to gamma LUT update - Fix unity of the frequencies reported on PMU - Fix kernel oops on set_page_dirty using better locks around it - Protect the request pointer with RCU to prevent it being freed while we might need still - Make pool objects read-only - Restore physical addresses for fb_map to avoid corrupted page table The following changes since commit af42d3466bdc8f39806b26f593604fdc54140bcb: Linux 5.4-rc8 (2019-11-17 14:47:30 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2019-11-22 for you to fetch changes up to 51658c04c338d7ef98d6c2c19009e4814632db50: Merge tag 'drm-intel-fixes-2019-11-21' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2019-11-22 10:29:52 +1000) drm fixes for 5.4 amdgpu: - Remove experimental flag for navi14 - Fix confusing power message failures on older VI parts - Hang fix for gfxoff when using the read register interface - Two stability regression fixes for Raven i915: - Fix kernel oops on dumb_create ioctl on no crtc situation - Fix bad ugly colored flash on VLV/CHV related to gamma LUT update - Fix unity of the frequencies reported on PMU - Fix kernel oops on set_page_dirty using better locks around it - Protect the request pointer with RCU to prevent it being freed while we might need still - Make pool objects read-only - Restore physical addresses for fb_map to avoid corrupted page table Alex Deucher (4): drm/amdgpu: remove experimental flag for Navi14 drm/amdgpu: disable gfxoff when using register read interface drm/amdgpu: disable gfxoff on original raven Revert "drm/amd/display: enable S/G for RAVEN chip" Chris Wilson (4): drm/i915/pmu: "Frequency" is reported as accumulated cycles drm/i915/userptr: Try to acquire the page lock around set_page_dirty() drm/i915: Protect request peeking with RCU drm/i915/fbdev: Restore physical addresses for fb_mmap() Dave Airlie (2): Merge tag 'drm-fixes-5.4-2019-11-20' of git://people.freedesktop.org/~agd5f/linux into drm-fixes Merge tag 'drm-intel-fixes-2019-11-21' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Evan Quan (2): drm/amd/powerplay: issue no PPSMC_MSG_GetCurrPkgPwr on unsupported ASICs drm/amd/powerplay: correct fine grained dpm force level setting Matthew Auld (1): drm/i915: make pool objects read-only Ville Syrjälä (2): drm/i915: Don't oops in dumb_create ioctl if we have no crtcs drm/i915: Preload LUTs if the hw isn't currently using them drivers/gpu/drm/amd/amdgpu/amdgpu_display.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 8 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c| 6 ++- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 9 +++- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 23 ++-- drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 6 +++ drivers/gpu/drm/i915/display/intel_atomic.c| 1 + drivers/gpu/drm/i915/display/intel_color.c | 61 ++ drivers/gpu/drm/i915/display/intel_display.c | 9 drivers/gpu/drm/i915/display/intel_display_types.h | 1 + drivers/gpu/drm/i915/display/intel_fbdev.c | 9 ++-- drivers/gpu/drm/i915/gem/i915_gem_userptr.c| 22 +++- drivers/gpu/drm/i915/gt/intel_engine_pool.c| 2 + drivers/gpu/drm/i915/i915_pmu.c| 4 +- drivers/gpu/drm/i915/i915_scheduler.c | 50 ++ 16 files changed, 183 insertions(+), 32 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
Fixes state transitions of Nvidia Pascal GPUs from D3cold into higher device
states.
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
Signed-off-by: Karol Herbst
Cc: Bjorn Helgaas
Cc: Lyude Paul
Cc: Rafael J. Wysocki
Cc: 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/pci/pci.c| 7 ++
drivers/pci/quirks.c | 51
include/linux/pci.h | 1 +
3 files changed, 59 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 57f15a7e6f0b..e08db2daa924 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -850,6 +850,13 @@ static int pci_raw_set_power_state(struct pci_dev *dev,
pci_power_t state)
|| (state == PCI_D2 && !dev->d2_support))
return -EIO;
+ /*
+* Check if we have a bad combination of bridge controller and nvidia
+* GPU, see quirk_broken_nv_runpm for more info
+*/
+ if (state != PCI_D0 && dev->broken_nv_runpm)
+ return 0;
+
pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, );
/*
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 44c4ae1abd00..24e3f247d291 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5268,3 +5268,54 @@ static void quirk_reset_lenovo_thinkpad_p50_nvgpu(struct
pci_dev *pdev)
DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1,
PCI_CLASS_DISPLAY_VGA, 8,
quirk_reset_lenovo_thinkpad_p50_nvgpu);
+
+/*
+ * Some Intel PCIe bridge controllers cause devices to not reappear doing a
+ * D0 -> D3hot -> D3cold -> D0 sequence. Skipping the intermediate D3hot step
+ * seems to make it work again.
+ *
+ * This leads to various manifestations of this issue:
+ * - AIML code execution hits an infinite loop (as the coe waits on device
+ *memory to change).
+ * - kernel crashes, as all PCI reads return -1, which most code isn't able
+ *to handle well enough.
+ * - sudden shutdowns, as the kernel identified an unrecoverable error after
+ *userspace tries to access the GPU.
+ *
+ * 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.
+ *
+ * ACPI code writes bit 0x80 to the not documented PCI register 0x248 of the
+ * Intel PCIe bridge controller (0x1901) in order to power down the GPU.
+ * Nonetheless, there are other code paths inside the ACPI firmware which use
+ * other registers, which seem to work fine:
+ * - 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.
+ *
+ * The only systems where this behavior can be seen are hybrid graphics laptops
+ * with a secondary Nvidia Maxwell, Pascal or Turing GPU. It cannot be ruled
+ * out that this issue only occurs in combination with listed Intel PCIe
+ * bridge controllers and the mentioned GPUs or if it's only a hw bug in the
+ * bridge controller.
+ */
+
+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->broken_nv_runpm = 1;
+ }
+}
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID,
+ PCI_BASE_CLASS_DISPLAY, 16,
+ quirk_broken_nv_runpm);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index ac8a6c4e1792..903a0b3a39ec 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -416,6 +416,7 @@ struct pci_dev {
unsigned int__aer_firmware_first_valid:1;
unsigned int__aer_firmware_first:1;
unsigned intbroken_intx_masking:1; /* INTx masking can't be used */
+ unsigned intbroken_nv_runpm:1; /* some combinations of intel
bridge controller and nvidia GPUs break rtd3 */
unsigned intio_window_1k:1; /* Intel bridge 1K I/O windows
*/
unsigned intirq_managed:1;
unsigned inthas_secondary_link:1;
--
2.23.0
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
so while trying to test with d3cold disabled, I noticed that I run into the exact same error. And I verified that the \_SB.PCI0.PEG0.PG00._STA returns 1, which indicates it should still be turned on. On Thu, Nov 21, 2019 at 11:50 PM Karol Herbst wrote: > > On Thu, Nov 21, 2019 at 11:39 PM Rafael J. Wysocki wrote: > > > > On Thu, Nov 21, 2019 at 8:49 PM Mika Westerberg > > wrote: > > > > > > On Thu, Nov 21, 2019 at 04:43:24PM +0100, Rafael J. Wysocki wrote: > > > > On Thu, Nov 21, 2019 at 1:52 PM Mika Westerberg > > > > wrote: > > > > > > > > > > On Thu, Nov 21, 2019 at 01:46:14PM +0200, Mika Westerberg wrote: > > > > > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > > > > > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > > > > > > wrote: > > > > > > > > > > > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki > > > > > > > > wrote: > > > > > > > > > > last week or so I found systems where the GPU was under the > > > > > > > > > > "PCI > > > > > > > > > > Express Root Port" (name from lspci) and on those systems > > > > > > > > > > all of that > > > > > > > > > > seems to work. So I am wondering if it's indeed just the > > > > > > > > > > 0x1901 one, > > > > > > > > > > which also explains Mikas case that Thunderbolt stuff works > > > > > > > > > > as devices > > > > > > > > > > never get populated under this particular bridge > > > > > > > > > > controller, but under > > > > > > > > > > those "Root Port"s > > > > > > > > > > > > > > > > > > It always is a PCIe port, but its location within the SoC may > > > > > > > > > matter. > > > > > > > > > > > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are > > > > > > > > called > > > > > > > > PEG, PCI Express Graphics, ports), and the PCH side. I think > > > > > > > > the IP is > > > > > > > > still the same. > > > > > > > > > > > > > > > > > Also some custom AML-based power management is involved and > > > > > > > > > that may > > > > > > > > > be making specific assumptions on the configuration of the > > > > > > > > > SoC and the > > > > > > > > > GPU at the time of its invocation which unfortunately are not > > > > > > > > > known to > > > > > > > > > us. > > > > > > > > > > > > > > > > > > However, it looks like the AML invoked to power down the GPU > > > > > > > > > from > > > > > > > > > acpi_pci_set_power_state() gets confused if it is not in PCI > > > > > > > > > D0 at > > > > > > > > > that point, so it looks like that AML tries to access device > > > > > > > > > memory on > > > > > > > > > the GPU (beyond the PCI config space) or similar which is not > > > > > > > > > accessible in PCI power states below D0. > > > > > > > > > > > > > > > > Or the PCI config space of the GPU when the parent root port is > > > > > > > > in D3hot > > > > > > > > (as it is the case here). Also then the GPU config space is not > > > > > > > > accessible. > > > > > > > > > > > > > > Why would the parent port be in D3hot at that point? Wouldn't > > > > > > > that be > > > > > > > a suspend ordering violation? > > > > > > > > > > > > No. We put the GPU into D3hot first, > > > > > > > > OK > > > > > > > > Does this involve any AML, like a _PS3 under the GPU object? > > > > > > I don't see _PS3 (nor _PS0) for that object. If I read it right the GPU > > > itself is not described in ACPI tables at all. > > > > OK > > > > > > > > then the root port and then turn > > > > > > off the power resource (which is attached to the root port) > > > > > > resulting > > > > > > the topology entering D3cold. > > > > > > > > > > I don't see that happening in the AML though. > > > > > > > > Which AML do you mean, specifically? The _OFF method for the root > > > > port's _PR3 power resource or something else? > > > > > > The root port's _OFF method for the power resource returned by its _PR3. > > > > OK, so without the $subject patch we (1) program the downstream > > component (GPU) into D3hot, then we (2) program the port holding it > > into D3hot and then we (3) let the AML (_OFF for the power resource > > listed by _PR3 under the port object) run. > > > > Something strange happens at this point (and I guess that _OFF doesn't > > even reach the point where it removes power from the port which is why > > we see a lock-up). > > > > it does though. I will post the data shortly (with the change in power > consumption), as I also want to do the ACPI traces now. > > > We know that skipping (1) makes things work and we kind of suspect > > that skipping (3) would make things work either, but what about doing > > (1) and (3) without (2)? > > > > > > > Basically the difference is that when Windows 7 or Linux (the _REV==5 > > > > > check) then we directly do link disable whereas in Windows 8+ we > > > > > invoke > > > > > LKDS() method that puts the link into L2/L3. None of the fields they > > > > > access seem to touch the GPU itself. > > > > > > > > So that may be where the problem is. > > > > > > > >
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 11:39 PM Rafael J. Wysocki wrote: > > On Thu, Nov 21, 2019 at 8:49 PM Mika Westerberg > wrote: > > > > On Thu, Nov 21, 2019 at 04:43:24PM +0100, Rafael J. Wysocki wrote: > > > On Thu, Nov 21, 2019 at 1:52 PM Mika Westerberg > > > wrote: > > > > > > > > On Thu, Nov 21, 2019 at 01:46:14PM +0200, Mika Westerberg wrote: > > > > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > > > > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > > > > > wrote: > > > > > > > > > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > > > > > > last week or so I found systems where the GPU was under the > > > > > > > > > "PCI > > > > > > > > > Express Root Port" (name from lspci) and on those systems all > > > > > > > > > of that > > > > > > > > > seems to work. So I am wondering if it's indeed just the > > > > > > > > > 0x1901 one, > > > > > > > > > which also explains Mikas case that Thunderbolt stuff works > > > > > > > > > as devices > > > > > > > > > never get populated under this particular bridge controller, > > > > > > > > > but under > > > > > > > > > those "Root Port"s > > > > > > > > > > > > > > > > It always is a PCIe port, but its location within the SoC may > > > > > > > > matter. > > > > > > > > > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are > > > > > > > called > > > > > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the > > > > > > > IP is > > > > > > > still the same. > > > > > > > > > > > > > > > Also some custom AML-based power management is involved and > > > > > > > > that may > > > > > > > > be making specific assumptions on the configuration of the SoC > > > > > > > > and the > > > > > > > > GPU at the time of its invocation which unfortunately are not > > > > > > > > known to > > > > > > > > us. > > > > > > > > > > > > > > > > However, it looks like the AML invoked to power down the GPU > > > > > > > > from > > > > > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 > > > > > > > > at > > > > > > > > that point, so it looks like that AML tries to access device > > > > > > > > memory on > > > > > > > > the GPU (beyond the PCI config space) or similar which is not > > > > > > > > accessible in PCI power states below D0. > > > > > > > > > > > > > > Or the PCI config space of the GPU when the parent root port is > > > > > > > in D3hot > > > > > > > (as it is the case here). Also then the GPU config space is not > > > > > > > accessible. > > > > > > > > > > > > Why would the parent port be in D3hot at that point? Wouldn't that > > > > > > be > > > > > > a suspend ordering violation? > > > > > > > > > > No. We put the GPU into D3hot first, > > > > > > OK > > > > > > Does this involve any AML, like a _PS3 under the GPU object? > > > > I don't see _PS3 (nor _PS0) for that object. If I read it right the GPU > > itself is not described in ACPI tables at all. > > OK > > > > > > then the root port and then turn > > > > > off the power resource (which is attached to the root port) resulting > > > > > the topology entering D3cold. > > > > > > > > I don't see that happening in the AML though. > > > > > > Which AML do you mean, specifically? The _OFF method for the root > > > port's _PR3 power resource or something else? > > > > The root port's _OFF method for the power resource returned by its _PR3. > > OK, so without the $subject patch we (1) program the downstream > component (GPU) into D3hot, then we (2) program the port holding it > into D3hot and then we (3) let the AML (_OFF for the power resource > listed by _PR3 under the port object) run. > > Something strange happens at this point (and I guess that _OFF doesn't > even reach the point where it removes power from the port which is why > we see a lock-up). > it does though. I will post the data shortly (with the change in power consumption), as I also want to do the ACPI traces now. > We know that skipping (1) makes things work and we kind of suspect > that skipping (3) would make things work either, but what about doing > (1) and (3) without (2)? > > > > > Basically the difference is that when Windows 7 or Linux (the _REV==5 > > > > check) then we directly do link disable whereas in Windows 8+ we invoke > > > > LKDS() method that puts the link into L2/L3. None of the fields they > > > > access seem to touch the GPU itself. > > > > > > So that may be where the problem is. > > > > > > Putting the downstream component into PCI D[1-3] is expected to put > > > the link into L1, so I'm not sure how that plays with the later > > > attempt to put it into L2/L3 Ready. > > > > That should be fine. What I've seen the link goes into L1 when > > downstream component is put to D-state (not D0) and then it is put back > > to L0 when L2/3 ready is propagated. Eventually it goes into L2 or L3. > > Well, that's the expected behavior, but the observed behavior isn't as > expected. :-) > > > > Also, L2/L3
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 8:49 PM Mika Westerberg
wrote:
>
> On Thu, Nov 21, 2019 at 04:43:24PM +0100, Rafael J. Wysocki wrote:
> > On Thu, Nov 21, 2019 at 1:52 PM Mika Westerberg
> > wrote:
> > >
> > > On Thu, Nov 21, 2019 at 01:46:14PM +0200, Mika Westerberg wrote:
> > > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote:
> > > > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg
> > > > > wrote:
> > > > > >
> > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote:
> > > > > > > > last week or so I found systems where the GPU was under the "PCI
> > > > > > > > Express Root Port" (name from lspci) and on those systems all
> > > > > > > > of that
> > > > > > > > seems to work. So I am wondering if it's indeed just the 0x1901
> > > > > > > > one,
> > > > > > > > which also explains Mikas case that Thunderbolt stuff works as
> > > > > > > > devices
> > > > > > > > never get populated under this particular bridge controller,
> > > > > > > > but under
> > > > > > > > those "Root Port"s
> > > > > > >
> > > > > > > It always is a PCIe port, but its location within the SoC may
> > > > > > > matter.
> > > > > >
> > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called
> > > > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP
> > > > > > is
> > > > > > still the same.
> > > > > >
> > > > > > > Also some custom AML-based power management is involved and that
> > > > > > > may
> > > > > > > be making specific assumptions on the configuration of the SoC
> > > > > > > and the
> > > > > > > GPU at the time of its invocation which unfortunately are not
> > > > > > > known to
> > > > > > > us.
> > > > > > >
> > > > > > > However, it looks like the AML invoked to power down the GPU from
> > > > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at
> > > > > > > that point, so it looks like that AML tries to access device
> > > > > > > memory on
> > > > > > > the GPU (beyond the PCI config space) or similar which is not
> > > > > > > accessible in PCI power states below D0.
> > > > > >
> > > > > > Or the PCI config space of the GPU when the parent root port is in
> > > > > > D3hot
> > > > > > (as it is the case here). Also then the GPU config space is not
> > > > > > accessible.
> > > > >
> > > > > Why would the parent port be in D3hot at that point? Wouldn't that be
> > > > > a suspend ordering violation?
> > > >
> > > > No. We put the GPU into D3hot first,
> >
> > OK
> >
> > Does this involve any AML, like a _PS3 under the GPU object?
>
> I don't see _PS3 (nor _PS0) for that object. If I read it right the GPU
> itself is not described in ACPI tables at all.
OK
> > > > then the root port and then turn
> > > > off the power resource (which is attached to the root port) resulting
> > > > the topology entering D3cold.
> > >
> > > I don't see that happening in the AML though.
> >
> > Which AML do you mean, specifically? The _OFF method for the root
> > port's _PR3 power resource or something else?
>
> The root port's _OFF method for the power resource returned by its _PR3.
OK, so without the $subject patch we (1) program the downstream
component (GPU) into D3hot, then we (2) program the port holding it
into D3hot and then we (3) let the AML (_OFF for the power resource
listed by _PR3 under the port object) run.
Something strange happens at this point (and I guess that _OFF doesn't
even reach the point where it removes power from the port which is why
we see a lock-up).
We know that skipping (1) makes things work and we kind of suspect
that skipping (3) would make things work either, but what about doing
(1) and (3) without (2)?
> > > Basically the difference is that when Windows 7 or Linux (the _REV==5
> > > check) then we directly do link disable whereas in Windows 8+ we invoke
> > > LKDS() method that puts the link into L2/L3. None of the fields they
> > > access seem to touch the GPU itself.
> >
> > So that may be where the problem is.
> >
> > Putting the downstream component into PCI D[1-3] is expected to put
> > the link into L1, so I'm not sure how that plays with the later
> > attempt to put it into L2/L3 Ready.
>
> That should be fine. What I've seen the link goes into L1 when
> downstream component is put to D-state (not D0) and then it is put back
> to L0 when L2/3 ready is propagated. Eventually it goes into L2 or L3.
Well, that's the expected behavior, but the observed behavior isn't as
expected. :-)
> > Also, L2/L3 Ready is expected to be transient, so finally power should
> > be removed somehow.
>
> There is GPIO for both power and PERST, I think the line here:
>
> \_SB.SGOV (0x01010004, Zero)
>
> is the one that removes power.
OK
> > > LKDS() for the first PEG port looks like this:
> > >
> > >P0L2 = One
> > >Sleep (0x10)
> > >Local0 = Zero
> > >While (P0L2)
> > >{
> > > If ((Local0 > 0x04))
> > > {
> > > Break
> > > }
> > >
> > >
Re: [PATCH v7 05/24] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages
On 11/21/19 8:59 AM, Dan Williams wrote: On Thu, Nov 21, 2019 at 12:57 AM John Hubbard wrote: On 11/21/19 12:05 AM, Christoph Hellwig wrote: So while this looks correct and I still really don't see the major benefit of the new code organization, especially as it bloats all put_page callers. I'd love to see code size change stats for an allyesconfig on this commit. Right, I'm running that now, will post the results. (btw, if there is a script and/or standard format I should use, I'm all ears. I'll dig through lwn...) Just run: size vmlinux Beautiful. I thought it would involve a lot more. Here's results: linux.git (Linux 5.4-rc8+): == text data bss dec hex filename 227578032 213267935 76877984517723951 1edbd72f vmlinux With patches 4 and 5 applied to linux.git: == text data bss dec hex filename 229698560 213288379 76853408519840347 1efc225b vmlinux Analysis: = This increased the size of text by 0.93%. Which is a measurable bloat, so the inlining really is undesirable here, yes. I'll do it differently. thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 09/24] vfio, mm: fix get_user_pages_remote() and FOLL_LONGTERM
On 11/21/19 1:35 PM, Alex Williamson wrote:
On Wed, 20 Nov 2019 23:13:39 -0800
John Hubbard wrote:
As it says in the updated comment in gup.c: current FOLL_LONGTERM
behavior is incompatible with FAULT_FLAG_ALLOW_RETRY because of the
FS DAX check requirement on vmas.
However, the corresponding restriction in get_user_pages_remote() was
slightly stricter than is actually required: it forbade all
FOLL_LONGTERM callers, but we can actually allow FOLL_LONGTERM callers
that do not set the "locked" arg.
Update the code and comments accordingly, and update the VFIO caller
to take advantage of this, fixing a bug as a result: the VFIO caller
is logically a FOLL_LONGTERM user.
Also, remove an unnessary pair of calls that were releasing and
reacquiring the mmap_sem. There is no need to avoid holding mmap_sem
just in order to call page_to_pfn().
Also, move the DAX check ("if a VMA is DAX, don't allow long term
pinning") from the VFIO call site, all the way into the internals
of get_user_pages_remote() and __gup_longterm_locked(). That is:
get_user_pages_remote() calls __gup_longterm_locked(), which in turn
calls check_dax_vmas(). It's lightly explained in the comments as well.
Thanks to Jason Gunthorpe for pointing out a clean way to fix this,
and to Dan Williams for helping clarify the DAX refactoring.
Reviewed-by: Jason Gunthorpe
Reviewed-by: Ira Weiny
Suggested-by: Jason Gunthorpe
Cc: Dan Williams
Cc: Jerome Glisse
Signed-off-by: John Hubbard
---
drivers/vfio/vfio_iommu_type1.c | 30 +-
mm/gup.c| 27 ++-
2 files changed, 27 insertions(+), 30 deletions(-)
Tested with device assignment and Intel mdev vGPU assignment with QEMU
userspace:
Tested-by: Alex Williamson
Acked-by: Alex Williamson
Feel free to include for 19/24 as well. Thanks,
Alex
Great! Thanks for the testing and ack on those. I'm about to repackage
(and split up as CH requested) for 5.5, and will keep you on CC, of course.
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 02/24] mm/gup: factor out duplicate code from four routines
On 11/21/19 1:49 AM, Jan Kara wrote: On Thu 21-11-19 00:29:59, John Hubbard wrote: On 11/21/19 12:03 AM, Christoph Hellwig wrote: Otherwise this looks fine and might be a worthwhile cleanup to feed Andrew for 5.5 independent of the gut of the changes. Reviewed-by: Christoph Hellwig Thanks for the reviews! Say, it sounds like your view here is that this series should be targeted at 5.6 (not 5.5), is that what you have in mind? And get the preparatory patches (1-9, and maybe even 10-16) into 5.5? Yeah, actually I feel the same. The merge window is going to open on Sunday and the series isn't still fully baked and happily sitting in linux-next (and larger changes should really sit in linux-next for at least a week, preferably two, before the merge window opens to get some reasonable test coverage). So I'd take out the independent easy patches that are already reviewed, get them merged into Andrew's (or whatever other appropriate tree) now so that they get at least a week of testing in linux-next before going upstream. And the more involved bits will have to wait for 5.6 - which means let's just continue working on them as we do now because ideally in 4 weeks we should have them ready with all the reviews so that they can be picked up and integrated into linux-next. Honza OK, thanks for spelling it out. I'll shift over to getting the easy patches prepared for 5.5, for now. thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 09/24] vfio, mm: fix get_user_pages_remote() and FOLL_LONGTERM
On Wed, 20 Nov 2019 23:13:39 -0800
John Hubbard wrote:
> As it says in the updated comment in gup.c: current FOLL_LONGTERM
> behavior is incompatible with FAULT_FLAG_ALLOW_RETRY because of the
> FS DAX check requirement on vmas.
>
> However, the corresponding restriction in get_user_pages_remote() was
> slightly stricter than is actually required: it forbade all
> FOLL_LONGTERM callers, but we can actually allow FOLL_LONGTERM callers
> that do not set the "locked" arg.
>
> Update the code and comments accordingly, and update the VFIO caller
> to take advantage of this, fixing a bug as a result: the VFIO caller
> is logically a FOLL_LONGTERM user.
>
> Also, remove an unnessary pair of calls that were releasing and
> reacquiring the mmap_sem. There is no need to avoid holding mmap_sem
> just in order to call page_to_pfn().
>
> Also, move the DAX check ("if a VMA is DAX, don't allow long term
> pinning") from the VFIO call site, all the way into the internals
> of get_user_pages_remote() and __gup_longterm_locked(). That is:
> get_user_pages_remote() calls __gup_longterm_locked(), which in turn
> calls check_dax_vmas(). It's lightly explained in the comments as well.
>
> Thanks to Jason Gunthorpe for pointing out a clean way to fix this,
> and to Dan Williams for helping clarify the DAX refactoring.
>
> Reviewed-by: Jason Gunthorpe
> Reviewed-by: Ira Weiny
> Suggested-by: Jason Gunthorpe
> Cc: Dan Williams
> Cc: Jerome Glisse
> Signed-off-by: John Hubbard
> ---
> drivers/vfio/vfio_iommu_type1.c | 30 +-
> mm/gup.c| 27 ++-
> 2 files changed, 27 insertions(+), 30 deletions(-)
Tested with device assignment and Intel mdev vGPU assignment with QEMU
userspace:
Tested-by: Alex Williamson
Acked-by: Alex Williamson
Feel free to include for 19/24 as well. Thanks,
Alex
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index d864277ea16f..c7a111ad9975 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -340,7 +340,6 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned
> long vaddr,
> {
> struct page *page[1];
> struct vm_area_struct *vma;
> - struct vm_area_struct *vmas[1];
> unsigned int flags = 0;
> int ret;
>
> @@ -348,33 +347,14 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned
> long vaddr,
> flags |= FOLL_WRITE;
>
> down_read(>mmap_sem);
> - if (mm == current->mm) {
> - ret = get_user_pages(vaddr, 1, flags | FOLL_LONGTERM, page,
> - vmas);
> - } else {
> - ret = get_user_pages_remote(NULL, mm, vaddr, 1, flags, page,
> - vmas, NULL);
> - /*
> - * The lifetime of a vaddr_get_pfn() page pin is
> - * userspace-controlled. In the fs-dax case this could
> - * lead to indefinite stalls in filesystem operations.
> - * Disallow attempts to pin fs-dax pages via this
> - * interface.
> - */
> - if (ret > 0 && vma_is_fsdax(vmas[0])) {
> - ret = -EOPNOTSUPP;
> - put_page(page[0]);
> - }
> - }
> - up_read(>mmap_sem);
> -
> + ret = get_user_pages_remote(NULL, mm, vaddr, 1, flags | FOLL_LONGTERM,
> + page, NULL, NULL);
> if (ret == 1) {
> *pfn = page_to_pfn(page[0]);
> - return 0;
> + ret = 0;
> + goto done;
> }
>
> - down_read(>mmap_sem);
> -
> vaddr = untagged_addr(vaddr);
>
> vma = find_vma_intersection(mm, vaddr, vaddr + 1);
> @@ -384,7 +364,7 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned
> long vaddr,
> if (is_invalid_reserved_pfn(*pfn))
> ret = 0;
> }
> -
> +done:
> up_read(>mmap_sem);
> return ret;
> }
> diff --git a/mm/gup.c b/mm/gup.c
> index 14fcdc502166..cce2c9676853 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -29,6 +29,13 @@ struct follow_page_context {
> unsigned int page_mask;
> };
>
> +static __always_inline long __gup_longterm_locked(struct task_struct *tsk,
> + struct mm_struct *mm,
> + unsigned long start,
> + unsigned long nr_pages,
> + struct page **pages,
> + struct vm_area_struct **vmas,
> + unsigned int flags);
> /*
> * Return the compound head page with ref appropriately incremented,
> * or NULL if that failed.
> @@ -1167,13 +1174,23 @@ long get_user_pages_remote(struct task_struct *tsk,
> struct mm_struct *mm,
> struct
Re: [PATCH] drm/shmem: Add docbook comments for drm_gem_shmem_object madvise fields
On Fri, Nov 1, 2019 at 4:37 PM Rob Herring wrote:
>
> Add missing docbook comments to madvise fields in struct
> drm_gem_shmem_object which fixes these warnings:
>
> include/drm/drm_gem_shmem_helper.h:87: warning: Function parameter or member
> 'madv' not described in 'drm_gem_shmem_object'
> include/drm/drm_gem_shmem_helper.h:87: warning: Function parameter or member
> 'madv_list' not described in 'drm_gem_shmem_object'
>
> Fixes: 17acb9f35ed7 ("drm/shmem: Add madvise state and purge helpers")
> Reported-by: Sean Paul
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: David Airlie
> Cc: Daniel Vetter
> Signed-off-by: Rob Herring
Kerneldoc for the functions you've added in 17acb9f35ed7 ("drm/shmem:
Add madvise state and purge helpers") is also missing. Can you pls fix
that too?
Thanks, Daniel
> ---
> include/drm/drm_gem_shmem_helper.h | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/drm/drm_gem_shmem_helper.h
> b/include/drm/drm_gem_shmem_helper.h
> index e2e9947b4770..901eafb09209 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -44,7 +44,20 @@ struct drm_gem_shmem_object {
> */
> unsigned int pages_use_count;
>
> + /**
> +* @madv: State for madvise
> +*
> +* 0 is active/inuse.
> +* A negative value is the object is purged.
> +* Positive values are driver specific and not used by the helpers.
> +*/
> int madv;
> +
> + /**
> +* @madv_list: List entry for madvise tracking
> +*
> +* Typically used by drivers to track purgeable objects
> +*/
> struct list_head madv_list;
>
> /**
> --
> 2.20.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 04:43:24PM +0100, Rafael J. Wysocki wrote:
> On Thu, Nov 21, 2019 at 1:52 PM Mika Westerberg
> wrote:
> >
> > On Thu, Nov 21, 2019 at 01:46:14PM +0200, Mika Westerberg wrote:
> > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote:
> > > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg
> > > > wrote:
> > > > >
> > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote:
> > > > > > > last week or so I found systems where the GPU was under the "PCI
> > > > > > > Express Root Port" (name from lspci) and on those systems all of
> > > > > > > that
> > > > > > > seems to work. So I am wondering if it's indeed just the 0x1901
> > > > > > > one,
> > > > > > > which also explains Mikas case that Thunderbolt stuff works as
> > > > > > > devices
> > > > > > > never get populated under this particular bridge controller, but
> > > > > > > under
> > > > > > > those "Root Port"s
> > > > > >
> > > > > > It always is a PCIe port, but its location within the SoC may
> > > > > > matter.
> > > > >
> > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called
> > > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is
> > > > > still the same.
> > > > >
> > > > > > Also some custom AML-based power management is involved and that may
> > > > > > be making specific assumptions on the configuration of the SoC and
> > > > > > the
> > > > > > GPU at the time of its invocation which unfortunately are not known
> > > > > > to
> > > > > > us.
> > > > > >
> > > > > > However, it looks like the AML invoked to power down the GPU from
> > > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at
> > > > > > that point, so it looks like that AML tries to access device memory
> > > > > > on
> > > > > > the GPU (beyond the PCI config space) or similar which is not
> > > > > > accessible in PCI power states below D0.
> > > > >
> > > > > Or the PCI config space of the GPU when the parent root port is in
> > > > > D3hot
> > > > > (as it is the case here). Also then the GPU config space is not
> > > > > accessible.
> > > >
> > > > Why would the parent port be in D3hot at that point? Wouldn't that be
> > > > a suspend ordering violation?
> > >
> > > No. We put the GPU into D3hot first,
>
> OK
>
> Does this involve any AML, like a _PS3 under the GPU object?
I don't see _PS3 (nor _PS0) for that object. If I read it right the GPU
itself is not described in ACPI tables at all.
> > > then the root port and then turn
> > > off the power resource (which is attached to the root port) resulting
> > > the topology entering D3cold.
> >
> > I don't see that happening in the AML though.
>
> Which AML do you mean, specifically? The _OFF method for the root
> port's _PR3 power resource or something else?
The root port's _OFF method for the power resource returned by its _PR3.
> > Basically the difference is that when Windows 7 or Linux (the _REV==5
> > check) then we directly do link disable whereas in Windows 8+ we invoke
> > LKDS() method that puts the link into L2/L3. None of the fields they
> > access seem to touch the GPU itself.
>
> So that may be where the problem is.
>
> Putting the downstream component into PCI D[1-3] is expected to put
> the link into L1, so I'm not sure how that plays with the later
> attempt to put it into L2/L3 Ready.
That should be fine. What I've seen the link goes into L1 when
downstream component is put to D-state (not D0) and then it is put back
to L0 when L2/3 ready is propagated. Eventually it goes into L2 or L3.
> Also, L2/L3 Ready is expected to be transient, so finally power should
> be removed somehow.
There is GPIO for both power and PERST, I think the line here:
\_SB.SGOV (0x01010004, Zero)
is the one that removes power.
> > LKDS() for the first PEG port looks like this:
> >
> >P0L2 = One
> >Sleep (0x10)
> >Local0 = Zero
> >While (P0L2)
> >{
> > If ((Local0 > 0x04))
> > {
> > Break
> > }
> >
> > Sleep (0x10)
> > Local0++
> >}
> >
> > One thing that comes to mind is that the loop can end even if P0L2 is
> > not cleared as it does only 5 iterations with 16 ms sleep between. Maybe
> > Sleep() is implemented differently in Windows? I mean Linux may be
> > "faster" here and return prematurely and if we leave the port into D0
> > this does not happen, or something. I'm just throwing out ideas :)
>
> But this actually works for the downstream component in D0, doesn't it?
It does and that leaves the link in L0 so it could be that then the
above AML works better or something.
That reminds me, ASPM may have something to do with this as well.
> Also, if the downstream component is in D0, the port actually should
> stay in D0 too, so what would happen with the $subject patch applied?
Parent port cannot be lower D-state than the child so I agree it should
stay in D0 as well. However, it seems that what happens
Re: [PATCH v4 6/6] drm/komeda: Expose side_by_side by sysfs/config_id
On Thu, 21 Nov 2019 at 20:21, james qian wang (Arm Technology China)
wrote:
>
> On Thu, Nov 21, 2019 at 10:49:26AM +0100, Daniel Vetter wrote:
> > On Thu, Nov 21, 2019 at 07:12:55AM +, james qian wang (Arm Technology
> > China) wrote:
> > > There are some restrictions if HW works on side_by_side, expose it via
> > > config_id to user.
> > >
> > > Signed-off-by: James Qian Wang (Arm Technology China)
> > >
> > > ---
> > > drivers/gpu/drm/arm/display/include/malidp_product.h | 3 ++-
> > > drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 1 +
> > > 2 files changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h
> > > b/drivers/gpu/drm/arm/display/include/malidp_product.h
> > > index 1053b11352eb..96e2e4016250 100644
> > > --- a/drivers/gpu/drm/arm/display/include/malidp_product.h
> > > +++ b/drivers/gpu/drm/arm/display/include/malidp_product.h
> > > @@ -27,7 +27,8 @@ union komeda_config_id {
> > > n_scalers:2, /* number of scalers per pipeline */
> > > n_layers:3, /* number of layers per pipeline */
> > > n_richs:3, /* number of rich layers per pipeline */
> > > - reserved_bits:6;
> > > + side_by_side:1, /* if HW works on side_by_side mode */
> > > + reserved_bits:5;
> > > };
> > > __u32 value;
> > > };
> > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > > b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > > index c3fa4835cb8d..4dd4699d4e3d 100644
> > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > > @@ -83,6 +83,7 @@ config_id_show(struct device *dev, struct
> > > device_attribute *attr, char *buf)
> >
> > Uh, this sysfs file here looks a lot like uapi for some compositor to
> > decide what to do. Do you have the userspace for this?
>
> Yes, our HWC driver uses this config_id and product_id for identifying the
> HW caps.
>
This seems like it should be done more in the kernel, why does
userspace needs all that info, to make more informed decisions?
How would drm_hwcomposer get the same result?
I'd prefer we just remove the sysfs nodes from upstream unless we have
an upstream user for them.
Dave.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: build failure after merge of the tip tree
On Wed, Nov 20, 2019 at 10:54 PM Stephen Rothwell wrote:
>
> Hi all,
>
> After merging the tip tree, today's linux-next build (x86_64 allmodconfig)
> failed like this:
>
> In file included from include/trace/define_trace.h:102,
> from drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:502,
> from drivers/gpu/drm/amd/amdgpu/amdgpu_trace_points.c:29:
> include/trace/../../drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:476:52: error:
> expected expression before ';' token
> 476 | __string(ring, sched_job->base.sched->name);
> |^
> include/trace/trace_events.h:435:2: note: in definition of macro
> 'DECLARE_EVENT_CLASS'
> 435 | tstruct\
> | ^~~
> include/trace/trace_events.h:77:9: note: in expansion of macro 'PARAMS'
>77 | PARAMS(tstruct), \
> | ^~
> include/trace/../../drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:472:1: note: in
> expansion of macro 'TRACE_EVENT'
> 472 | TRACE_EVENT(amdgpu_ib_pipe_sync,
> | ^~~
> include/trace/../../drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:475:6: note: in
> expansion of macro 'TP_STRUCT__entry'
> 475 | TP_STRUCT__entry(
> | ^~~~
>
> Caused by commit
>
> 2c2fdb8bca29 ("drm/amdgpu: fix amdgpu trace event print string format
> error")
>
> from the drm tree interacting with commit
>
> 60fdad00827c ("ftrace: Rework event_create_dir()")
>
> from the tip tree.
>
> I have added the following merge fix patch:
Applied. Thanks!
Alex
>
> From: Stephen Rothwell
> Date: Thu, 21 Nov 2019 14:46:00 +1100
> Subject: [PATCH] merge fix for "ftrace: Rework event_create_dir()"
>
> Signed-off-by: Stephen Rothwell
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> index f940526c5889..63e734a125fb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> @@ -473,7 +473,7 @@ TRACE_EVENT(amdgpu_ib_pipe_sync,
> TP_PROTO(struct amdgpu_job *sched_job, struct dma_fence *fence),
> TP_ARGS(sched_job, fence),
> TP_STRUCT__entry(
> -__string(ring, sched_job->base.sched->name);
> +__string(ring, sched_job->base.sched->name)
> __field(uint64_t, id)
> __field(struct dma_fence *, fence)
> __field(uint64_t, ctx)
> --
> 2.23.0
>
> --
> Cheers,
> Stephen Rothwell
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/amdgpu: remove redundant assignment to pointer write_frame
Applied. thanks!
Alex
On Thu, Nov 21, 2019 at 11:54 AM Colin King wrote:
>
> From: Colin Ian King
>
> The pointer write_frame is being initialized with a value that is
> never read and it is being updated later with a new value. The
> initialization is redundant and can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 2a8a08aa6eaf..c02f9ffe5c6b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -1728,7 +1728,7 @@ int psp_ring_cmd_submit(struct psp_context *psp,
> int index)
> {
> unsigned int psp_write_ptr_reg = 0;
> - struct psp_gfx_rb_frame *write_frame = psp->km_ring.ring_mem;
> + struct psp_gfx_rb_frame *write_frame;
> struct psp_ring *ring = >km_ring;
> struct psp_gfx_rb_frame *ring_buffer_start = ring->ring_mem;
> struct psp_gfx_rb_frame *ring_buffer_end = ring_buffer_start +
> --
> 2.24.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv3/RFC 1/4] drm/arm: Factor out generic afbc helpers
These are useful for other users of afbc, e.g. rockchip.
Signed-off-by: Andrzej Pietrasiewicz
---
drivers/gpu/drm/Makefile | 2 +-
drivers/gpu/drm/drm_afbc.c| 84 +++
drivers/gpu/drm/drm_fourcc.c | 11 +++-
drivers/gpu/drm/drm_framebuffer.c | 71 +-
include/drm/drm_afbc.h| 35 +
5 files changed, 199 insertions(+), 4 deletions(-)
create mode 100644 drivers/gpu/drm/drm_afbc.c
create mode 100644 include/drm/drm_afbc.h
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index d9bcc9f2a0a4..3a58f30b83a6 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -44,7 +44,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o
drm_dsc.o drm_probe_helper
drm_simple_kms_helper.o drm_modeset_helper.o \
drm_scdc_helper.o drm_gem_framebuffer_helper.o \
drm_atomic_state_helper.o drm_damage_helper.o \
- drm_format_helper.o drm_self_refresh_helper.o
+ drm_format_helper.o drm_self_refresh_helper.o drm_afbc.o
drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
diff --git a/drivers/gpu/drm/drm_afbc.c b/drivers/gpu/drm/drm_afbc.c
new file mode 100644
index ..f308c4719546
--- /dev/null
+++ b/drivers/gpu/drm/drm_afbc.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) 2019 Collabora Ltd.
+ *
+ * author: Andrzej Pietrasiewicz
+ *
+ */
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * drm_afbc_get_superblk_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_superblk_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;
+ case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
+ /* fall through */
+ case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
+ /* fall through */
+ 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_superblk_wh);
+
+/**
+ * drm_afbc_get_parameters - extract afbc parameters from mode command
+ * @mode_cmd: mode command to be looked at
+ * @afbc: address of a struct to be filled in
+ */
+void drm_afbc_get_parameters(const struct drm_mode_fb_cmd2 *mode_cmd,
+struct drm_afbc *afbc)
+{
+ drm_afbc_get_superblk_wh(mode_cmd->modifier[0],
+>tile_w, >tile_h);
+ afbc->width = mode_cmd->pitches[0];
+ afbc->height =
+ DIV_ROUND_UP(mode_cmd->height, afbc->tile_h) * afbc->tile_h;
+ afbc->offset = mode_cmd->offsets[0];
+}
+EXPORT_SYMBOL(drm_afbc_get_parameters);
+
+/**
+ * drm_is_afbc - test if the modifier describes an afbc buffer
+ * @modifier - modifier to be tested
+ *
+ * Returns: true if the modifier describes an afbc buffer
+ */
+bool drm_is_afbc(u64 modifier)
+{
+ /* is it ARM AFBC? */
+ if ((modifier & DRM_FORMAT_MOD_ARM_AFBC(0)) == 0)
+ return false;
+
+ /* Block size must be known */
+ if ((modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) == 0)
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL_GPL(drm_is_afbc);
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index c630064ccf41..8d9f197cc0ab 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -27,6 +27,7 @@
#include
#include
+#include
#include
#include
@@ -322,8 +323,14 @@ drm_get_format_info(struct drm_device *dev,
{
const struct drm_format_info *info = NULL;
- if (dev->mode_config.funcs->get_format_info)
- info = dev->mode_config.funcs->get_format_info(mode_cmd);
+ /* bypass driver callback if afbc */
+ if (!drm_is_afbc(mode_cmd->modifier[0]))
+ if (dev->mode_config.funcs->get_format_info) {
+ const struct drm_mode_config_funcs *funcs;
+
+ funcs = dev->mode_config.funcs;
+ info = funcs->get_format_info(mode_cmd);
+ }
if (!info)
info = drm_format_info(mode_cmd->pixel_format);
diff --git a/drivers/gpu/drm/drm_framebuffer.c
b/drivers/gpu/drm/drm_framebuffer.c
index 57564318ceea..303eea624a02 100644
---
[PATCHv3/RFC 3/4] drm/komeda: Use afbc helper
AFBC helpers are available. Use those which increase code readability.
Signed-off-by: Andrzej Pietrasiewicz
---
.../drm/arm/display/komeda/komeda_framebuffer.c | 17 +++--
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
index 1b01a625f40e..f7edde3ac319 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
@@ -4,6 +4,7 @@
* Author: James.Qian.Wang
*
*/
+#include
#include
#include
#include
@@ -52,20 +53,8 @@ komeda_fb_afbc_size_check(struct komeda_fb *kfb, struct
drm_file *file,
return -ENOENT;
}
- switch (fb->modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
- case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
- alignment_w = 32;
- alignment_h = 8;
- break;
- case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
- alignment_w = 16;
- alignment_h = 16;
- break;
- default:
- WARN(1, "Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
-fb->modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
- break;
- }
+ if (!drm_afbc_get_superblk_wh(fb->modifier, _w, _h))
+ return -EINVAL;
/* tiled header afbc */
if (fb->modifier & AFBC_FORMAT_MOD_TILED) {
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv3/RFC 4/4] drm/rockchip: Add support for afbc
This patch adds support for afbc handling. afbc is a compressed format
which reduces the necessary memory bandwidth.
Co-developed-by: Mark Yao
Signed-off-by: Mark Yao
Signed-off-by: Andrzej Pietrasiewicz
---
drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 29
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 142 +++-
drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 12 ++
drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 84 +++-
4 files changed, 263 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index ca01234c037c..7eaa3fdc03b2 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -8,6 +8,7 @@
#include
#include
+#include
#include
#include
#include
@@ -18,6 +19,8 @@
#include "rockchip_drm_fb.h"
#include "rockchip_drm_gem.h"
+#define ROCKCHIP_MAX_AFBC_WIDTH2560
+
static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
.destroy = drm_gem_fb_destroy,
.create_handle = drm_gem_fb_create_handle,
@@ -32,6 +35,32 @@ rockchip_fb_alloc(struct drm_device *dev, const struct
drm_mode_fb_cmd2 *mode_cm
int ret;
int i;
+ if (drm_is_afbc(mode_cmd->modifier[0])) {
+ struct drm_afbc afbc;
+
+ drm_afbc_get_parameters(mode_cmd, );
+
+ if (afbc.offset) {
+ DRM_WARN("AFBC plane offset must be zero!\n");
+
+ return ERR_PTR(-EINVAL);
+ }
+
+ if (afbc.tile_w != 16 || afbc.tile_h != 16) {
+ DRM_WARN("Unsupported afbc tile w/h [%d/%d]\n",
+afbc.tile_w, afbc.tile_h);
+
+ return ERR_PTR(-EINVAL);
+ }
+
+ if (afbc.width > ROCKCHIP_MAX_AFBC_WIDTH) {
+ DRM_WARN("Unsupported width %d>%d\n",
+afbc.width, ROCKCHIP_MAX_AFBC_WIDTH);
+
+ return ERR_PTR(-EINVAL);
+ }
+ }
+
fb = kzalloc(sizeof(*fb), GFP_KERNEL);
if (!fb)
return ERR_PTR(-ENOMEM);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index d04b3492bdac..31f72ba61361 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -91,9 +91,22 @@
#define VOP_WIN_TO_INDEX(vop_win) \
((vop_win) - (vop_win)->vop->win)
+#define VOP_AFBC_SET(vop, name, v) \
+ do { \
+ if ((vop)->data->afbc) \
+ vop_reg_set((vop), &(vop)->data->afbc->name, \
+ 0, ~0, v, #name); \
+ } while (0)
+
#define to_vop(x) container_of(x, struct vop, crtc)
#define to_vop_win(x) container_of(x, struct vop_win, base)
+#define AFBC_FMT_RGB5650x0
+#define AFBC_FMT_U8U8U8U8 0x5
+#define AFBC_FMT_U8U8U80x4
+
+#define AFBC_TILE_16x16BIT(4)
+
/*
* The coefficients of the following matrix are all fixed points.
* The format is S2.10 for the 3x3 part of the matrix, and S9.12 for the
offsets.
@@ -166,6 +179,7 @@ struct vop {
/* optional internal rgb encoder */
struct rockchip_rgb *rgb;
+ struct vop_win *afbc_win;
struct vop_win win[];
};
@@ -274,6 +288,29 @@ static enum vop_data_format vop_convert_format(uint32_t
format)
}
}
+static int vop_convert_afbc_format(uint32_t format)
+{
+ switch (format) {
+ case DRM_FORMAT_XRGB:
+ case DRM_FORMAT_ARGB:
+ case DRM_FORMAT_XBGR:
+ case DRM_FORMAT_ABGR:
+ return AFBC_FMT_U8U8U8U8;
+ case DRM_FORMAT_RGB888:
+ case DRM_FORMAT_BGR888:
+ return AFBC_FMT_U8U8U8;
+ case DRM_FORMAT_RGB565:
+ case DRM_FORMAT_BGR565:
+ return AFBC_FMT_RGB565;
+ /* either of the below should not be reachable */
+ default:
+ DRM_WARN_ONCE("unsupported AFBC format[%08x]\n", format);
+ return -EINVAL;
+ }
+
+ return -EINVAL;
+}
+
static uint16_t scl_vop_cal_scale(enum scale_mode mode, uint32_t src,
uint32_t dst, bool is_horizontal,
int vsu_mode, int *vskiplines)
@@ -598,6 +635,15 @@ static int vop_enable(struct drm_crtc *crtc, struct
drm_crtc_state *old_state)
vop_win_disable(vop, vop_win);
}
}
+
+ if (vop->data->afbc) {
+ /*
+* Disable AFBC and forget there was a vop window with AFBC
+*/
+ VOP_AFBC_SET(vop, enable, 0);
+ vop->afbc_win = NULL;
+ }
+
spin_unlock(>reg_lock);
vop_cfg_done(vop);
@@ -710,6 +756,34 @@ static void vop_plane_destroy(struct drm_plane
[PATCHv3/RFC 2/4] drm/malidp: use afbc helpers
There are afbc helpers available.
Signed-off-by: Andrzej Pietrasiewicz
---
drivers/gpu/drm/arm/malidp_drv.c | 121 +--
1 file changed, 20 insertions(+), 101 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 37d92a06318e..ff8364680676 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -15,6 +15,7 @@
#include
#include
+#include
#include
#include
#include
@@ -35,8 +36,6 @@
#include "malidp_hw.h"
#define MALIDP_CONF_VALID_TIMEOUT 250
-#define AFBC_HEADER_SIZE 16
-#define AFBC_SUPERBLK_ALIGNMENT128
static void malidp_write_gamma_table(struct malidp_hw_device *hwdev,
u32 data[MALIDP_COEFFTAB_NUM_COEFFS])
@@ -269,112 +268,32 @@ static const struct drm_mode_config_helper_funcs
malidp_mode_config_helpers = {
.atomic_commit_tail = malidp_atomic_commit_tail,
};
-static bool
-malidp_verify_afbc_framebuffer_caps(struct drm_device *dev,
- const struct drm_mode_fb_cmd2 *mode_cmd)
-{
- if (malidp_format_mod_supported(dev, mode_cmd->pixel_format,
- mode_cmd->modifier[0]) == false)
- return false;
-
- if (mode_cmd->offsets[0] != 0) {
- DRM_DEBUG_KMS("AFBC buffers' plane offset should be 0\n");
- return false;
- }
-
- switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
- case AFBC_SIZE_16X16:
- if ((mode_cmd->width % 16) || (mode_cmd->height % 16)) {
- DRM_DEBUG_KMS("AFBC buffers must be aligned to 16
pixels\n");
- return false;
- }
- break;
- default:
- DRM_DEBUG_KMS("Unsupported AFBC block size\n");
- return false;
- }
-
- return true;
-}
-
-static bool
-malidp_verify_afbc_framebuffer_size(struct drm_device *dev,
- struct drm_file *file,
- const struct drm_mode_fb_cmd2 *mode_cmd)
+static struct drm_framebuffer *
+malidp_fb_create(struct drm_device *dev, struct drm_file *file,
+const struct drm_mode_fb_cmd2 *mode_cmd)
{
- int n_superblocks = 0;
- const struct drm_format_info *info;
- struct drm_gem_object *objs = NULL;
- u32 afbc_superblock_size = 0, afbc_superblock_height = 0;
- u32 afbc_superblock_width = 0, afbc_size = 0;
- int bpp = 0;
-
- switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
- case AFBC_SIZE_16X16:
- afbc_superblock_height = 16;
- afbc_superblock_width = 16;
- break;
- default:
- DRM_DEBUG_KMS("AFBC superblock size is not supported\n");
- return false;
- }
-
- info = drm_get_format_info(dev, mode_cmd);
-
- n_superblocks = (mode_cmd->width / afbc_superblock_width) *
- (mode_cmd->height / afbc_superblock_height);
-
- bpp = malidp_format_get_bpp(info->format);
-
- afbc_superblock_size = (bpp * afbc_superblock_width *
afbc_superblock_height)
- / BITS_PER_BYTE;
-
- afbc_size = ALIGN(n_superblocks * AFBC_HEADER_SIZE,
AFBC_SUPERBLK_ALIGNMENT);
- afbc_size += n_superblocks * ALIGN(afbc_superblock_size,
AFBC_SUPERBLK_ALIGNMENT);
-
- if ((mode_cmd->width * bpp) != (mode_cmd->pitches[0] * BITS_PER_BYTE)) {
- DRM_DEBUG_KMS("Invalid value of (pitch * BITS_PER_BYTE) (=%u) "
- "should be same as width (=%u) * bpp (=%u)\n",
- (mode_cmd->pitches[0] * BITS_PER_BYTE),
- mode_cmd->width, bpp);
- return false;
- }
-
- objs = drm_gem_object_lookup(file, mode_cmd->handles[0]);
- if (!objs) {
- DRM_DEBUG_KMS("Failed to lookup GEM object\n");
- return false;
- }
+ if (mode_cmd->modifier[0]) {
+ struct drm_afbc afbc;
- if (objs->size < afbc_size) {
- DRM_DEBUG_KMS("buffer size (%zu) too small for AFBC buffer size
= %u\n",
- objs->size, afbc_size);
- drm_gem_object_put_unlocked(objs);
- return false;
- }
+ if (malidp_format_mod_supported(dev, mode_cmd->pixel_format,
+ mode_cmd->modifier[0]) == false)
+ return ERR_PTR(-EINVAL);
- drm_gem_object_put_unlocked(objs);
+ drm_afbc_get_parameters(mode_cmd, );
- return true;
-}
+ if (afbc.tile_w != 16 || afbc.tile_h != 16) {
+ DRM_DEV_DEBUG(dev->dev,
+ "Unsupported AFBC block size %dx%d\n",
+ afbc.tile_w, afbc.tile_h);
[PATCHv3/RFC 0/4] AFBC rework and support for Rockchip
v2..v3: - addressed (some) comments from Daniel Stone, Liviu Dudau, Daniel Vetter and Brian Starkey - thank you all In this iteration some rework has been done. The checking logic is now moved to framebuffer_check() so it is common to all drivers. But the common part is not good for komeda, so this series is not good for merging yet. I kindly ask for feedback whether the changes are in the right direction. I also kindly ask for input on how to accommodate komeda. The CONFIG_DRM_AFBC option has been eliminated in favour of adding drm_afbc.c to drm_kms_helper. v1..v2: This series adds AFBC support for Rockchip. It is inspired by: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/factory-gru-9017.B-chromeos-4.4/drivers/gpu/drm/rockchip/rockchip_drm_vop.c - addressed comments from Daniel Stone, Ayan Halder, Mihail Atanassov - coding style fixes Andrzej Pietrasiewicz (4): drm/arm: Factor out generic afbc helpers drm/malidp: use afbc helpers drm/komeda: Use afbc helper drm/rockchip: Add support for afbc drivers/gpu/drm/Makefile | 2 +- .../arm/display/komeda/komeda_framebuffer.c | 17 +-- drivers/gpu/drm/arm/malidp_drv.c | 121 +++ drivers/gpu/drm/drm_afbc.c| 84 +++ drivers/gpu/drm/drm_fourcc.c | 11 +- drivers/gpu/drm/drm_framebuffer.c | 71 - drivers/gpu/drm/rockchip/rockchip_drm_fb.c| 29 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 142 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 12 ++ drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 84 ++- include/drm/drm_afbc.h| 35 + 11 files changed, 485 insertions(+), 123 deletions(-) create mode 100644 drivers/gpu/drm/drm_afbc.c create mode 100644 include/drm/drm_afbc.h -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 05/24] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages
On Thu, Nov 21, 2019 at 12:57 AM John Hubbard wrote: > > On 11/21/19 12:05 AM, Christoph Hellwig wrote: > > So while this looks correct and I still really don't see the major > > benefit of the new code organization, especially as it bloats all > > put_page callers. > > > > I'd love to see code size change stats for an allyesconfig on this > > commit. > > > > Right, I'm running that now, will post the results. (btw, if there is > a script and/or standard format I should use, I'm all ears. I'll dig > through lwn...) > Just run: size vmlinux ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH][next] drm/amdgpu: remove redundant assignment to pointer write_frame
From: Colin Ian King
The pointer write_frame is being initialized with a value that is
never read and it is being updated later with a new value. The
initialization is redundant and can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 2a8a08aa6eaf..c02f9ffe5c6b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -1728,7 +1728,7 @@ int psp_ring_cmd_submit(struct psp_context *psp,
int index)
{
unsigned int psp_write_ptr_reg = 0;
- struct psp_gfx_rb_frame *write_frame = psp->km_ring.ring_mem;
+ struct psp_gfx_rb_frame *write_frame;
struct psp_ring *ring = >km_ring;
struct psp_gfx_rb_frame *ring_buffer_start = ring->ring_mem;
struct psp_gfx_rb_frame *ring_buffer_end = ring_buffer_start +
--
2.24.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v20 2/2] drm/bridge: Add I2C based driver for ps8640 bridge
Hi, cc'ing the drm/bridge maintainers which I think are missing (Andrzej Hajda and Neil Armstrong) Missatge de Matthias Brugger del dia dl., 7 d’oct. 2019 a les 13:04: > > > > On 07/10/2019 10:22, Ulrich Hecht wrote: > > From: Jitao Shi > > > > This patch adds drm_bridge driver for parade DSI to eDP bridge chip. > > > > Signed-off-by: Jitao Shi > > Reviewed-by: Daniel Kurtz > > Reviewed-by: Enric Balletbo i Serra > > [uli: followed API changes, removed FW update feature] > > Signed-off-by: Ulrich Hecht > > Now it works, thanks for pushing this forward :) > > Tested-by: Matthias Brugger > This bridge is really needed in case you have an Acer Chromebook R13, otherwise, you don't have the display working. Would be nice have it upstream :-), If it helps, I tested this patch on top of current 5.4-rc8 and I get the display working, so Tested-by: Enric Balletbo i Serra Thanks, Enric > > --- > > > > Changes since v19: > > - fixed return value of ps8640_probe() when no panel is found > > > > Changes since v18: > > - followed DRM API changes > > - use DEVICE_ATTR_RO() > > - remove firmware update code > > - add SPDX identifier > > > > Changes since v17: > > - remove some unused head files. > > - add macros for ps8640 pages. > > - remove ddc_i2c client > > - add mipi_dsi_device_register_full > > - remove the manufacturer from the name and i2c_device_id > > > > Changes since v16: > > - Disable ps8640 DSI MCS Function. > > - Rename gpios name more clearly. > > - Tune the ps8640 power on sequence. > > > > Changes since v15: > > - Drop drm_connector_(un)register calls from parade ps8640. > >The main DRM driver mtk_drm_drv now calls > >drm_connector_register_all() after drm_dev_register() in the > >mtk_drm_bind() function. That function should iterate over all > >connectors and call drm_connector_register() for each of them. > >So, remove drm_connector_(un)register calls from parade ps8640. > > > > Changes since v14: > > - update copyright info. > > - change bridge_to_ps8640 and connector_to_ps8640 to inline function. > > - fix some coding style. > > - use sizeof as array counter. > > - use drm_get_edid when read edid. > > - add mutex when firmware updating. > > > > Changes since v13: > > - add const on data, ps8640_write_bytes(struct i2c_client *client, const > > u8 *data, u16 data_len) > > - fix PAGE2_SW_REST tyro. > > - move the buf[3] init to entrance of the function. > > > > Changes since v12: > > - fix hw_chip_id build warning > > > > Changes since v11: > > - Remove depends on I2C, add DRM depends > > - Reuse ps8640_write_bytes() in ps8640_write_byte() > > - Use timer check for polling like the routines in > > - Fix no drm_connector_unregister/drm_connector_cleanup when > > ps8640_bridge_attach fail > > - Check the ps8640 hardware id in ps8640_validate_firmware > > - Remove fw_version check > > - Move ps8640_validate_firmware before ps8640_enter_bl > > - Add ddc_i2c unregister when probe fail and ps8640_remove > > > > > > drivers/gpu/drm/bridge/Kconfig | 12 + > > drivers/gpu/drm/bridge/Makefile| 1 + > > drivers/gpu/drm/bridge/parade-ps8640.c | 672 > > + > > 3 files changed, 685 insertions(+) > > create mode 100644 drivers/gpu/drm/bridge/parade-ps8640.c > > > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > > index 1cc9f50..61c6415 100644 > > --- a/drivers/gpu/drm/bridge/Kconfig > > +++ b/drivers/gpu/drm/bridge/Kconfig > > @@ -82,6 +82,18 @@ config DRM_PARADE_PS8622 > > ---help--- > > Parade eDP-LVDS bridge chip driver. > > > > +config DRM_PARADE_PS8640 > > + tristate "Parade PS8640 MIPI DSI to eDP Converter" > > + depends on DRM > > + depends on OF > > + select DRM_KMS_HELPER > > + select DRM_MIPI_DSI > > + select DRM_PANEL > > + help > > + Choose this option if you have PS8640 for display > > + The PS8640 is a high-performance and low-power > > + MIPI DSI to eDP converter > > + > > config DRM_SIL_SII8620 > > tristate "Silicon Image SII8620 HDMI/MHL bridge" > > depends on OF > > diff --git a/drivers/gpu/drm/bridge/Makefile > > b/drivers/gpu/drm/bridge/Makefile > > index 4934fcf..14660ab 100644 > > --- a/drivers/gpu/drm/bridge/Makefile > > +++ b/drivers/gpu/drm/bridge/Makefile > > @@ -6,6 +6,7 @@ obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o > > obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) += > > megachips-stdp-ge-b850v3-fw.o > > obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o > > obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o > > +obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o > > obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o > > obj-$(CONFIG_DRM_SII902X) += sii902x.o > > obj-$(CONFIG_DRM_SII9234) += sii9234.o > > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c > > b/drivers/gpu/drm/bridge/parade-ps8640.c > > new file mode 100644 > > index
[PULL] drm-intel-fixes
Hi Dave and Daniel, A special thanks to our CI and to Chris here. https://intel-gfx-ci.01.org/tree/drm-intel-fixes/index.html For finding and providing the quick fix for 5.4 on time to avoid the bad corruption with fbdev mmap. Plus other kernel oops and corruption fixes. There was a conflict here with drm-next but it was easy to solve and it is recorded on drm-rerere so that might be transparent now. More details below. Here goes drm-intel-fixes-2019-11-21: - Fix kernel oops on dumb_create ioctl on no crtc situation - Fix bad ugly colored flash on VLV/CHV related to gamma LUT update - Fix unity of the frequencies reported on PMU - Fix kernel oops on set_page_dirty using better locks around it - Protect the request pointer with RCU to prevent it being freed while we might need still - Make pool objects read-only - Restore physical addresses for fb_mmap to avoid corrupted page table Thanks, Rodrigo. The following changes since commit af42d3466bdc8f39806b26f593604fdc54140bcb: Linux 5.4-rc8 (2019-11-17 14:47:30 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2019-11-21 for you to fetch changes up to 71d122629c04707eb7f65447fb2f5bd092d98ce3: drm/i915/fbdev: Restore physical addresses for fb_mmap() (2019-11-21 00:09:22 -0800) - Fix kernel oops on dumb_create ioctl on no crtc situation - Fix bad ugly colored flash on VLV/CHV related to gamma LUT update - Fix unity of the frequencies reported on PMU - Fix kernel oops on set_page_dirty using better locks around it - Protect the request pointer with RCU to prevent it being freed while we might need still - Make pool objects read-only - Restore physical addresses for fb_map to avoid corrupted page table Chris Wilson (4): drm/i915/pmu: "Frequency" is reported as accumulated cycles drm/i915/userptr: Try to acquire the page lock around set_page_dirty() drm/i915: Protect request peeking with RCU drm/i915/fbdev: Restore physical addresses for fb_mmap() Matthew Auld (1): drm/i915: make pool objects read-only Ville Syrjälä (2): drm/i915: Don't oops in dumb_create ioctl if we have no crtcs drm/i915: Preload LUTs if the hw isn't currently using them drivers/gpu/drm/i915/display/intel_atomic.c| 1 + drivers/gpu/drm/i915/display/intel_color.c | 61 ++ drivers/gpu/drm/i915/display/intel_display.c | 9 drivers/gpu/drm/i915/display/intel_display_types.h | 1 + drivers/gpu/drm/i915/display/intel_fbdev.c | 9 ++-- drivers/gpu/drm/i915/gem/i915_gem_userptr.c| 22 +++- drivers/gpu/drm/i915/gt/intel_engine_pool.c| 2 + drivers/gpu/drm/i915/i915_pmu.c| 4 +- drivers/gpu/drm/i915/i915_scheduler.c | 50 ++ 9 files changed, 141 insertions(+), 18 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Intel-gfx] [PATCH 2/2] drm: share address space for dma bufs
>-Original Message-
>From: Intel-gfx On Behalf Of Gerd
>Hoffmann
>Sent: Thursday, November 21, 2019 5:38 AM
>To: dri-devel@lists.freedesktop.org
>Cc: David Airlie ; intel-...@lists.freedesktop.org; open list
>; Maxime Ripard ; Gerd
>Hoffmann
>Subject: [Intel-gfx] [PATCH 2/2] drm: share address space for dma bufs
>
>Use the shared address space of the drm device (see drm_open() in
>drm_file.c) for dma-bufs too. That removes a difference betweem drm
>device mmap vmas and dma-buf mmap vmas and fixes corner cases like
>unmaps not working properly.
Hi Gerd,
Just want to make sure I understand this...
So unmaps will not work correctly for mappings when a driver does a
drm_vma_node_unamp()?
I.e. the dmabuf mappings will not get cleaned up correctly?
This is a day one bug?
Thanks,
Mike
>Signed-off-by: Gerd Hoffmann
>---
> drivers/gpu/drm/drm_prime.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
>index a9633bd241bb..c3fc341453c0 100644
>--- a/drivers/gpu/drm/drm_prime.c
>+++ b/drivers/gpu/drm/drm_prime.c
>@@ -240,6 +240,7 @@ void drm_prime_destroy_file_private(struct
>drm_prime_file_private *prime_fpriv)
> struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
> struct dma_buf_export_info *exp_info)
> {
>+ struct drm_gem_object *obj = exp_info->priv;
> struct dma_buf *dma_buf;
>
> dma_buf = dma_buf_export(exp_info);
>@@ -247,7 +248,8 @@ struct dma_buf *drm_gem_dmabuf_export(struct
>drm_device *dev,
> return dma_buf;
>
> drm_dev_get(dev);
>- drm_gem_object_get(exp_info->priv);
>+ drm_gem_object_get(obj);
>+ dma_buf->file->f_mapping = obj->dev->anon_inode->i_mapping;
>
> return dma_buf;
> }
>--
>2.18.1
>
>___
>Intel-gfx mailing list
>intel-...@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 5:06 PM Karol Herbst wrote: > > On Thu, Nov 21, 2019 at 4:47 PM Rafael J. Wysocki wrote: > > > > On Thu, Nov 21, 2019 at 1:53 PM Karol Herbst wrote: > > > > > > On Thu, Nov 21, 2019 at 12:46 PM Mika Westerberg > > > wrote: > > > > > > > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > > > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > > > > wrote: > > > > > > > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > > > > > last week or so I found systems where the GPU was under the "PCI > > > > > > > > Express Root Port" (name from lspci) and on those systems all > > > > > > > > of that > > > > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 > > > > > > > > one, > > > > > > > > which also explains Mikas case that Thunderbolt stuff works as > > > > > > > > devices > > > > > > > > never get populated under this particular bridge controller, > > > > > > > > but under > > > > > > > > those "Root Port"s > > > > > > > > > > > > > > It always is a PCIe port, but its location within the SoC may > > > > > > > matter. > > > > > > > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > > > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP > > > > > > is > > > > > > still the same. > > > > > > > > > > > > yeah, I meant the bridge controller with the ID 0x1901 is on the CPU > > > side. And if the Nvidia GPU is on a port on the PCH side it all seems > > > to work just fine. > > > > But that may involve different AML too, may it not? > > > > > > > > > Also some custom AML-based power management is involved and that > > > > > > > may > > > > > > > be making specific assumptions on the configuration of the SoC > > > > > > > and the > > > > > > > GPU at the time of its invocation which unfortunately are not > > > > > > > known to > > > > > > > us. > > > > > > > > > > > > > > However, it looks like the AML invoked to power down the GPU from > > > > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > > > > > that point, so it looks like that AML tries to access device > > > > > > > memory on > > > > > > > the GPU (beyond the PCI config space) or similar which is not > > > > > > > accessible in PCI power states below D0. > > > > > > > > > > > > Or the PCI config space of the GPU when the parent root port is in > > > > > > D3hot > > > > > > (as it is the case here). Also then the GPU config space is not > > > > > > accessible. > > > > > > > > > > Why would the parent port be in D3hot at that point? Wouldn't that be > > > > > a suspend ordering violation? > > > > > > > > No. We put the GPU into D3hot first, then the root port and then turn > > > > off the power resource (which is attached to the root port) resulting > > > > the topology entering D3cold. > > > > > > > > > > If the kernel does a D0 -> D3hot -> D0 cycle this works as well, but > > > the power savings are way lower, so I kind of prefer skipping D3hot > > > instead of D3cold. Skipping D3hot doesn't seem to make any difference > > > in power savings in my testing. > > > > OK > > > > What exactly did you do to skip D3cold in your testing? > > > > For that I poked into the PCI registers directly and skipped doing the > ACPI calls and simply checked for the idle power consumption on my > laptop. That doesn't involve the PCIe port PM, however. > But I guess I should retest with calling pci_d3cold_disable > from nouveau instead? Or is there a different preferable way of > testing this? There is a sysfs attribute called "d3cold_allowed" which can be used for "blocking" D3cold, so can you please retest using that? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v2 0/7] PCI: Prefer pcie_capability_read_word()
> -Original Message- > From: amd-gfx On Behalf Of > Bjorn Helgaas > Sent: Thursday, November 21, 2019 9:02 AM > To: linux-...@vger.kernel.org > Cc: Zhou, David(ChunMing) ; Frederick Lawler > ; Dave Airlie ; linux- > ker...@vger.kernel.org; dri-devel@lists.freedesktop.org; Bjorn Helgaas > ; amd-...@lists.freedesktop.org; Daniel Vetter > ; Alex Deucher ; Koenig, > Christian > Subject: [PATCH v2 0/7] PCI: Prefer pcie_capability_read_word() > > From: Bjorn Helgaas > > Use pcie_capability_read_word() and similar instead of using > pci_read_config_word() directly. Add #defines to replace some magic > numbers. Fix typos in use of Transmit Margin field. > > These are currently on my pci/misc branch for v5.5. Let me know if you see > any issues. > Series is: Reviewed-by: Alex Deucher > > Bjorn Helgaas (5): > PCI: Add #defines for Enter Compliance, Transmit Margin > drm/amdgpu: Correct Transmit Margin masks > drm/amdgpu: Replace numbers with PCI_EXP_LNKCTL2 definitions > drm/radeon: Correct Transmit Margin masks > drm/radeon: Replace numbers with PCI_EXP_LNKCTL2 definitions > > Frederick Lawler (2): > drm/amdgpu: Prefer pcie_capability_read_word() > drm/radeon: Prefer pcie_capability_read_word() > > drivers/gpu/drm/amd/amdgpu/cik.c | 95 +++ > drivers/gpu/drm/amd/amdgpu/si.c | 97 > drivers/gpu/drm/radeon/cik.c | 94 +++ > drivers/gpu/drm/radeon/si.c | 97 > include/uapi/linux/pci_regs.h| 2 + > 5 files changed, 243 insertions(+), 142 deletions(-) > > -- > 2.24.0.432.g9d3f5f5b63-goog > > ___ > 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] drm/amd: Fix Kconfig indentation
Reviewed-by: Alex Deucher From: Krzysztof Kozlowski Sent: Thursday, November 21, 2019 8:29 AM To: linux-ker...@vger.kernel.org Cc: Krzysztof Kozlowski ; Deucher, Alexander ; Koenig, Christian ; Zhou, David(ChunMing) ; David Airlie ; Daniel Vetter ; amd-...@lists.freedesktop.org ; dri-devel@lists.freedesktop.org Subject: [PATCH] drm/amd: Fix Kconfig indentation Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- drivers/gpu/drm/amd/acp/Kconfig | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/acp/Kconfig b/drivers/gpu/drm/amd/acp/Kconfig index d968c2471412..19bae9100da4 100644 --- a/drivers/gpu/drm/amd/acp/Kconfig +++ b/drivers/gpu/drm/amd/acp/Kconfig @@ -2,11 +2,11 @@ menu "ACP (Audio CoProcessor) Configuration" config DRM_AMD_ACP - bool "Enable AMD Audio CoProcessor IP support" - depends on DRM_AMDGPU - select MFD_CORE - select PM_GENERIC_DOMAINS if PM - help + bool "Enable AMD Audio CoProcessor IP support" + depends on DRM_AMDGPU + select MFD_CORE + select PM_GENERIC_DOMAINS if PM + help Choose this option to enable ACP IP support for AMD SOCs. This adds the ACP (Audio CoProcessor) IP driver and wires it up into the amdgpu driver. The ACP block provides the DMA -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 4:47 PM Rafael J. Wysocki wrote: > > On Thu, Nov 21, 2019 at 1:53 PM Karol Herbst wrote: > > > > On Thu, Nov 21, 2019 at 12:46 PM Mika Westerberg > > wrote: > > > > > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > > > wrote: > > > > > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > > > > last week or so I found systems where the GPU was under the "PCI > > > > > > > Express Root Port" (name from lspci) and on those systems all of > > > > > > > that > > > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 > > > > > > > one, > > > > > > > which also explains Mikas case that Thunderbolt stuff works as > > > > > > > devices > > > > > > > never get populated under this particular bridge controller, but > > > > > > > under > > > > > > > those "Root Port"s > > > > > > > > > > > > It always is a PCIe port, but its location within the SoC may > > > > > > matter. > > > > > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > > > > > still the same. > > > > > > > > > yeah, I meant the bridge controller with the ID 0x1901 is on the CPU > > side. And if the Nvidia GPU is on a port on the PCH side it all seems > > to work just fine. > > But that may involve different AML too, may it not? > > > > > > > Also some custom AML-based power management is involved and that may > > > > > > be making specific assumptions on the configuration of the SoC and > > > > > > the > > > > > > GPU at the time of its invocation which unfortunately are not known > > > > > > to > > > > > > us. > > > > > > > > > > > > However, it looks like the AML invoked to power down the GPU from > > > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > > > > that point, so it looks like that AML tries to access device memory > > > > > > on > > > > > > the GPU (beyond the PCI config space) or similar which is not > > > > > > accessible in PCI power states below D0. > > > > > > > > > > Or the PCI config space of the GPU when the parent root port is in > > > > > D3hot > > > > > (as it is the case here). Also then the GPU config space is not > > > > > accessible. > > > > > > > > Why would the parent port be in D3hot at that point? Wouldn't that be > > > > a suspend ordering violation? > > > > > > No. We put the GPU into D3hot first, then the root port and then turn > > > off the power resource (which is attached to the root port) resulting > > > the topology entering D3cold. > > > > > > > If the kernel does a D0 -> D3hot -> D0 cycle this works as well, but > > the power savings are way lower, so I kind of prefer skipping D3hot > > instead of D3cold. Skipping D3hot doesn't seem to make any difference > > in power savings in my testing. > > OK > > What exactly did you do to skip D3cold in your testing? > For that I poked into the PCI registers directly and skipped doing the ACPI calls and simply checked for the idle power consumption on my laptop. But I guess I should retest with calling pci_d3cold_disable from nouveau instead? Or is there a different preferable way of testing this? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 1:53 PM Karol Herbst wrote: > > On Thu, Nov 21, 2019 at 12:46 PM Mika Westerberg > wrote: > > > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > > wrote: > > > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > > > last week or so I found systems where the GPU was under the "PCI > > > > > > Express Root Port" (name from lspci) and on those systems all of > > > > > > that > > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 one, > > > > > > which also explains Mikas case that Thunderbolt stuff works as > > > > > > devices > > > > > > never get populated under this particular bridge controller, but > > > > > > under > > > > > > those "Root Port"s > > > > > > > > > > It always is a PCIe port, but its location within the SoC may matter. > > > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > > > > still the same. > > > > > > yeah, I meant the bridge controller with the ID 0x1901 is on the CPU > side. And if the Nvidia GPU is on a port on the PCH side it all seems > to work just fine. But that may involve different AML too, may it not? > > > > > Also some custom AML-based power management is involved and that may > > > > > be making specific assumptions on the configuration of the SoC and the > > > > > GPU at the time of its invocation which unfortunately are not known to > > > > > us. > > > > > > > > > > However, it looks like the AML invoked to power down the GPU from > > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > > > that point, so it looks like that AML tries to access device memory on > > > > > the GPU (beyond the PCI config space) or similar which is not > > > > > accessible in PCI power states below D0. > > > > > > > > Or the PCI config space of the GPU when the parent root port is in D3hot > > > > (as it is the case here). Also then the GPU config space is not > > > > accessible. > > > > > > Why would the parent port be in D3hot at that point? Wouldn't that be > > > a suspend ordering violation? > > > > No. We put the GPU into D3hot first, then the root port and then turn > > off the power resource (which is attached to the root port) resulting > > the topology entering D3cold. > > > > If the kernel does a D0 -> D3hot -> D0 cycle this works as well, but > the power savings are way lower, so I kind of prefer skipping D3hot > instead of D3cold. Skipping D3hot doesn't seem to make any difference > in power savings in my testing. OK What exactly did you do to skip D3cold in your testing? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 1:52 PM Mika Westerberg
wrote:
>
> On Thu, Nov 21, 2019 at 01:46:14PM +0200, Mika Westerberg wrote:
> > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote:
> > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg
> > > wrote:
> > > >
> > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote:
> > > > > > last week or so I found systems where the GPU was under the "PCI
> > > > > > Express Root Port" (name from lspci) and on those systems all of
> > > > > > that
> > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 one,
> > > > > > which also explains Mikas case that Thunderbolt stuff works as
> > > > > > devices
> > > > > > never get populated under this particular bridge controller, but
> > > > > > under
> > > > > > those "Root Port"s
> > > > >
> > > > > It always is a PCIe port, but its location within the SoC may matter.
> > > >
> > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called
> > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is
> > > > still the same.
> > > >
> > > > > Also some custom AML-based power management is involved and that may
> > > > > be making specific assumptions on the configuration of the SoC and the
> > > > > GPU at the time of its invocation which unfortunately are not known to
> > > > > us.
> > > > >
> > > > > However, it looks like the AML invoked to power down the GPU from
> > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at
> > > > > that point, so it looks like that AML tries to access device memory on
> > > > > the GPU (beyond the PCI config space) or similar which is not
> > > > > accessible in PCI power states below D0.
> > > >
> > > > Or the PCI config space of the GPU when the parent root port is in D3hot
> > > > (as it is the case here). Also then the GPU config space is not
> > > > accessible.
> > >
> > > Why would the parent port be in D3hot at that point? Wouldn't that be
> > > a suspend ordering violation?
> >
> > No. We put the GPU into D3hot first,
OK
Does this involve any AML, like a _PS3 under the GPU object?
> > then the root port and then turn
> > off the power resource (which is attached to the root port) resulting
> > the topology entering D3cold.
>
> I don't see that happening in the AML though.
Which AML do you mean, specifically? The _OFF method for the root
port's _PR3 power resource or something else?
> Basically the difference is that when Windows 7 or Linux (the _REV==5
> check) then we directly do link disable whereas in Windows 8+ we invoke
> LKDS() method that puts the link into L2/L3. None of the fields they
> access seem to touch the GPU itself.
So that may be where the problem is.
Putting the downstream component into PCI D[1-3] is expected to put
the link into L1, so I'm not sure how that plays with the later
attempt to put it into L2/L3 Ready.
Also, L2/L3 Ready is expected to be transient, so finally power should
be removed somehow.
> LKDS() for the first PEG port looks like this:
>
>P0L2 = One
>Sleep (0x10)
>Local0 = Zero
>While (P0L2)
>{
> If ((Local0 > 0x04))
> {
> Break
> }
>
> Sleep (0x10)
> Local0++
>}
>
> One thing that comes to mind is that the loop can end even if P0L2 is
> not cleared as it does only 5 iterations with 16 ms sleep between. Maybe
> Sleep() is implemented differently in Windows? I mean Linux may be
> "faster" here and return prematurely and if we leave the port into D0
> this does not happen, or something. I'm just throwing out ideas :)
But this actually works for the downstream component in D0, doesn't it?
Also, if the downstream component is in D0, the port actually should
stay in D0 too, so what would happen with the $subject patch applied?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/7] drm/amdgpu: Correct Transmit Margin masks
From: Bjorn Helgaas Previously we masked PCIe Link Control 2 register values with "7 << 9", which was apparently intended to be the Transmit Margin field, but instead was the high order bit of Transmit Margin, the Enter Modified Compliance bit, and the Compliance SOS bit. Correct the mask to "7 << 7", which is the Transmit Margin field. Link: https://lore.kernel.org/r/20191112173503.176611-3-helg...@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/cik.c | 8 drivers/gpu/drm/amd/amdgpu/si.c | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c index b81bb414fcb3..13a5696d2a6a 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik.c +++ b/drivers/gpu/drm/amd/amdgpu/cik.c @@ -1498,13 +1498,13 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) /* linkctl2 */ pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE(ixPCIE_LC_CNTL4); diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c index 493af42152f2..1e350172dc7b 100644 --- a/drivers/gpu/drm/amd/amdgpu/si.c +++ b/drivers/gpu/drm/amd/amdgpu/si.c @@ -1737,13 +1737,13 @@ static void si_pcie_gen3_enable(struct amdgpu_device *adev) pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL, tmp16); pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); -- 2.24.0.432.g9d3f5f5b63-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/7] drm/amdgpu: Replace numbers with PCI_EXP_LNKCTL2 definitions
From: Bjorn Helgaas Replace hard-coded magic numbers with the descriptive PCI_EXP_LNKCTL2 definitions. No functional change intended. Link: https://lore.kernel.org/r/20191112173503.176611-4-helg...@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/cik.c | 22 ++ drivers/gpu/drm/amd/amdgpu/si.c | 22 ++ 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c index 13a5696d2a6a..3067bb874032 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik.c +++ b/drivers/gpu/drm/amd/amdgpu/cik.c @@ -1498,13 +1498,19 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) /* linkctl2 */ pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (bridge_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (gpu_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE(ixPCIE_LC_CNTL4); @@ -1521,13 +1527,13 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) WREG32_PCIE(ixPCIE_LC_SPEED_CNTL, speed_cntl); pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~0xf; + tmp16 &= ~PCI_EXP_LNKCTL2_TLS; if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3) - tmp16 |= 3; /* gen3 */ + tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */ else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2) - tmp16 |= 2; /* gen2 */ + tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */ else - tmp16 |= 1; /* gen1 */ + tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */ pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); speed_cntl = RREG32_PCIE(ixPCIE_LC_SPEED_CNTL); diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c index 1e350172dc7b..a7dcb0d0f039 100644 --- a/drivers/gpu/drm/amd/amdgpu/si.c +++ b/drivers/gpu/drm/amd/amdgpu/si.c @@ -1737,13 +1737,19 @@ static void si_pcie_gen3_enable(struct amdgpu_device *adev) pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL, tmp16); pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (bridge_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (gpu_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); @@ -1758,13 +1764,13 @@ static void si_pcie_gen3_enable(struct amdgpu_device *adev)
[PATCH 6/7] drm/radeon: Replace numbers with PCI_EXP_LNKCTL2 definitions
From: Bjorn Helgaas Replace hard-coded magic numbers with the descriptive PCI_EXP_LNKCTL2 definitions. No functional change intended. Link: https://lore.kernel.org/r/20191112173503.176611-4-helg...@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Alex Deucher --- drivers/gpu/drm/radeon/cik.c | 22 ++ drivers/gpu/drm/radeon/si.c | 22 ++ 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index 14cdfdf78bde..a280442c81aa 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -9619,13 +9619,19 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) /* linkctl2 */ pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (bridge_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (gpu_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); @@ -9641,13 +9647,13 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl); pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~0xf; + tmp16 &= ~PCI_EXP_LNKCTL2_TLS; if (speed_cap == PCIE_SPEED_8_0GT) - tmp16 |= 3; /* gen3 */ + tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */ else if (speed_cap == PCIE_SPEED_5_0GT) - tmp16 |= 2; /* gen2 */ + tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */ else - tmp16 |= 1; /* gen1 */ + tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */ pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL); diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index 9b7042d3ef1b..529e70a42019 100644 --- a/drivers/gpu/drm/radeon/si.c +++ b/drivers/gpu/drm/radeon/si.c @@ -7202,13 +7202,19 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev) /* linkctl2 */ pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (bridge_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 7)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); + tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN); + tmp16 |= (gpu_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); @@ -7224,13 +7230,13 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev) WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl); pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, ); -
[PATCH 5/7] drm/radeon: Correct Transmit Margin masks
From: Bjorn Helgaas Previously we masked PCIe Link Control 2 register values with "7 << 9", which was apparently intended to be the Transmit Margin field, but instead was the high order bit of Transmit Margin, the Enter Modified Compliance bit, and the Compliance SOS bit. Correct the mask to "7 << 7", which is the Transmit Margin field. Link: https://lore.kernel.org/r/20191112173503.176611-3-helg...@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Alex Deucher --- drivers/gpu/drm/radeon/cik.c | 8 drivers/gpu/drm/radeon/si.c | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index 62eab82a64f9..14cdfdf78bde 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -9619,13 +9619,13 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) /* linkctl2 */ pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index 05894d198a79..9b7042d3ef1b 100644 --- a/drivers/gpu/drm/radeon/si.c +++ b/drivers/gpu/drm/radeon/si.c @@ -7202,13 +7202,13 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev) /* linkctl2 */ pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (bridge_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(root, bridge_pos + PCI_EXP_LNKCTL2, tmp16); pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, ); - tmp16 &= ~((1 << 4) | (7 << 9)); - tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 9))); + tmp16 &= ~((1 << 4) | (7 << 7)); + tmp16 |= (gpu_cfg2 & ((1 << 4) | (7 << 7))); pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); -- 2.24.0.432.g9d3f5f5b63-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/7] PCI: Prefer pcie_capability_read_word()
From: Bjorn Helgaas Use pcie_capability_read_word() and similar instead of using pci_read_config_word() directly. Add #defines to replace some magic numbers. Fix typos in use of Transmit Margin field. These are currently on my pci/misc branch for v5.5. Let me know if you see any issues. Bjorn Helgaas (5): PCI: Add #defines for Enter Compliance, Transmit Margin drm/amdgpu: Correct Transmit Margin masks drm/amdgpu: Replace numbers with PCI_EXP_LNKCTL2 definitions drm/radeon: Correct Transmit Margin masks drm/radeon: Replace numbers with PCI_EXP_LNKCTL2 definitions Frederick Lawler (2): drm/amdgpu: Prefer pcie_capability_read_word() drm/radeon: Prefer pcie_capability_read_word() drivers/gpu/drm/amd/amdgpu/cik.c | 95 +++ drivers/gpu/drm/amd/amdgpu/si.c | 97 drivers/gpu/drm/radeon/cik.c | 94 +++ drivers/gpu/drm/radeon/si.c | 97 include/uapi/linux/pci_regs.h| 2 + 5 files changed, 243 insertions(+), 142 deletions(-) -- 2.24.0.432.g9d3f5f5b63-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 7/7] drm/radeon: Prefer pcie_capability_read_word()
From: Frederick Lawler
Commit 8c0d3a02c130 ("PCI: Add accessors for PCI Express Capability")
added accessors for the PCI Express Capability so that drivers didn't
need to be aware of differences between v1 and v2 of the PCI
Express Capability.
Replace pci_read_config_word() and pci_write_config_word() calls with
pcie_capability_read_word() and pcie_capability_write_word().
Link: https://lore.kernel.org/r/20191118003513.10852-1-f...@fredlawl.com
Signed-off-by: Frederick Lawler
Signed-off-by: Bjorn Helgaas
---
drivers/gpu/drm/radeon/cik.c | 70 +-
drivers/gpu/drm/radeon/si.c | 73 +++-
2 files changed, 90 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
index a280442c81aa..09a4709e67f0 100644
--- a/drivers/gpu/drm/radeon/cik.c
+++ b/drivers/gpu/drm/radeon/cik.c
@@ -9504,7 +9504,6 @@ static void cik_pcie_gen3_enable(struct radeon_device
*rdev)
{
struct pci_dev *root = rdev->pdev->bus->self;
enum pci_bus_speed speed_cap;
- int bridge_pos, gpu_pos;
u32 speed_cntl, current_data_rate;
int i;
u16 tmp16;
@@ -9546,12 +9545,7 @@ static void cik_pcie_gen3_enable(struct radeon_device
*rdev)
DRM_INFO("enabling PCIE gen 2 link speeds, disable with
radeon.pcie_gen2=0\n");
}
- bridge_pos = pci_pcie_cap(root);
- if (!bridge_pos)
- return;
-
- gpu_pos = pci_pcie_cap(rdev->pdev);
- if (!gpu_pos)
+ if (!pci_is_pcie(root) || !pci_is_pcie(rdev->pdev))
return;
if (speed_cap == PCIE_SPEED_8_0GT) {
@@ -9561,14 +9555,17 @@ static void cik_pcie_gen3_enable(struct radeon_device
*rdev)
u16 bridge_cfg2, gpu_cfg2;
u32 max_lw, current_lw, tmp;
- pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL,
_cfg);
- pci_read_config_word(rdev->pdev, gpu_pos +
PCI_EXP_LNKCTL, _cfg);
+ pcie_capability_read_word(root, PCI_EXP_LNKCTL,
+ _cfg);
+ pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL,
+ _cfg);
tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD;
- pci_write_config_word(root, bridge_pos +
PCI_EXP_LNKCTL, tmp16);
+ pcie_capability_write_word(root, PCI_EXP_LNKCTL, tmp16);
tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD;
- pci_write_config_word(rdev->pdev, gpu_pos +
PCI_EXP_LNKCTL, tmp16);
+ pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL,
+ tmp16);
tmp = RREG32_PCIE_PORT(PCIE_LC_STATUS1);
max_lw = (tmp & LC_DETECTED_LINK_WIDTH_MASK) >>
LC_DETECTED_LINK_WIDTH_SHIFT;
@@ -9586,15 +9583,23 @@ static void cik_pcie_gen3_enable(struct radeon_device
*rdev)
for (i = 0; i < 10; i++) {
/* check status */
- pci_read_config_word(rdev->pdev, gpu_pos +
PCI_EXP_DEVSTA, );
+ pcie_capability_read_word(rdev->pdev,
+ PCI_EXP_DEVSTA,
+ );
if (tmp16 & PCI_EXP_DEVSTA_TRPND)
break;
- pci_read_config_word(root, bridge_pos +
PCI_EXP_LNKCTL, _cfg);
- pci_read_config_word(rdev->pdev, gpu_pos +
PCI_EXP_LNKCTL, _cfg);
+ pcie_capability_read_word(root, PCI_EXP_LNKCTL,
+ _cfg);
+ pcie_capability_read_word(rdev->pdev,
+ PCI_EXP_LNKCTL,
+ _cfg);
- pci_read_config_word(root, bridge_pos +
PCI_EXP_LNKCTL2, _cfg2);
- pci_read_config_word(rdev->pdev, gpu_pos +
PCI_EXP_LNKCTL2, _cfg2);
+ pcie_capability_read_word(root, PCI_EXP_LNKCTL2,
+ _cfg2);
+ pcie_capability_read_word(rdev->pdev,
+ PCI_EXP_LNKCTL2,
+ _cfg2);
tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4);
tmp |= LC_SET_QUIESCE;
@@ -9607,32 +9612,45 @@ static void cik_pcie_gen3_enable(struct radeon_device
*rdev)
msleep(100);
[PATCH 4/7] drm/amdgpu: Prefer pcie_capability_read_word()
From: Frederick Lawler
Commit 8c0d3a02c130 ("PCI: Add accessors for PCI Express Capability")
added accessors for the PCI Express Capability so that drivers didn't
need to be aware of differences between v1 and v2 of the PCI
Express Capability.
Replace pci_read_config_word() and pci_write_config_word() calls with
pcie_capability_read_word() and pcie_capability_write_word().
[bhelgaas: fix a couple remaining instances in cik.c]
Link: https://lore.kernel.org/r/20191118003513.10852-1-f...@fredlawl.com
Signed-off-by: Frederick Lawler
Signed-off-by: Bjorn Helgaas
---
drivers/gpu/drm/amd/amdgpu/cik.c | 71
drivers/gpu/drm/amd/amdgpu/si.c | 71
2 files changed, 90 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c
index 3067bb874032..38b06ae6357a 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik.c
@@ -1384,7 +1384,6 @@ static int cik_set_vce_clocks(struct amdgpu_device *adev,
u32 evclk, u32 ecclk)
static void cik_pcie_gen3_enable(struct amdgpu_device *adev)
{
struct pci_dev *root = adev->pdev->bus->self;
- int bridge_pos, gpu_pos;
u32 speed_cntl, current_data_rate;
int i;
u16 tmp16;
@@ -1419,12 +1418,7 @@ static void cik_pcie_gen3_enable(struct amdgpu_device
*adev)
DRM_INFO("enabling PCIE gen 2 link speeds, disable with
amdgpu.pcie_gen2=0\n");
}
- bridge_pos = pci_pcie_cap(root);
- if (!bridge_pos)
- return;
-
- gpu_pos = pci_pcie_cap(adev->pdev);
- if (!gpu_pos)
+ if (!pci_is_pcie(root) || !pci_is_pcie(adev->pdev))
return;
if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3) {
@@ -1434,14 +1428,17 @@ static void cik_pcie_gen3_enable(struct amdgpu_device
*adev)
u16 bridge_cfg2, gpu_cfg2;
u32 max_lw, current_lw, tmp;
- pci_read_config_word(root, bridge_pos + PCI_EXP_LNKCTL,
_cfg);
- pci_read_config_word(adev->pdev, gpu_pos +
PCI_EXP_LNKCTL, _cfg);
+ pcie_capability_read_word(root, PCI_EXP_LNKCTL,
+ _cfg);
+ pcie_capability_read_word(adev->pdev, PCI_EXP_LNKCTL,
+ _cfg);
tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD;
- pci_write_config_word(root, bridge_pos +
PCI_EXP_LNKCTL, tmp16);
+ pcie_capability_write_word(root, PCI_EXP_LNKCTL, tmp16);
tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD;
- pci_write_config_word(adev->pdev, gpu_pos +
PCI_EXP_LNKCTL, tmp16);
+ pcie_capability_write_word(adev->pdev, PCI_EXP_LNKCTL,
+ tmp16);
tmp = RREG32_PCIE(ixPCIE_LC_STATUS1);
max_lw = (tmp &
PCIE_LC_STATUS1__LC_DETECTED_LINK_WIDTH_MASK) >>
@@ -1465,15 +1462,23 @@ static void cik_pcie_gen3_enable(struct amdgpu_device
*adev)
for (i = 0; i < 10; i++) {
/* check status */
- pci_read_config_word(adev->pdev, gpu_pos +
PCI_EXP_DEVSTA, );
+ pcie_capability_read_word(adev->pdev,
+ PCI_EXP_DEVSTA,
+ );
if (tmp16 & PCI_EXP_DEVSTA_TRPND)
break;
- pci_read_config_word(root, bridge_pos +
PCI_EXP_LNKCTL, _cfg);
- pci_read_config_word(adev->pdev, gpu_pos +
PCI_EXP_LNKCTL, _cfg);
+ pcie_capability_read_word(root, PCI_EXP_LNKCTL,
+ _cfg);
+ pcie_capability_read_word(adev->pdev,
+ PCI_EXP_LNKCTL,
+ _cfg);
- pci_read_config_word(root, bridge_pos +
PCI_EXP_LNKCTL2, _cfg2);
- pci_read_config_word(adev->pdev, gpu_pos +
PCI_EXP_LNKCTL2, _cfg2);
+ pcie_capability_read_word(root, PCI_EXP_LNKCTL2,
+ _cfg2);
+ pcie_capability_read_word(adev->pdev,
+ PCI_EXP_LNKCTL2,
+ _cfg2);
tmp = RREG32_PCIE(ixPCIE_LC_CNTL4);
tmp |= PCIE_LC_CNTL4__LC_SET_QUIESCE_MASK;
@@
[PATCH 1/7] PCI: Add #defines for Enter Compliance, Transmit Margin
From: Bjorn Helgaas Add definitions for the Enter Compliance and Transmit Margin fields of the PCIe Link Control 2 register. Link: https://lore.kernel.org/r/20191112173503.176611-2-helg...@kernel.org Signed-off-by: Bjorn Helgaas Reviewed-by: Alex Deucher --- include/uapi/linux/pci_regs.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index 29d6e93fd15e..5869e5778a05 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -673,6 +673,8 @@ #define PCI_EXP_LNKCTL2_TLS_8_0GT 0x0003 /* Supported Speed 8GT/s */ #define PCI_EXP_LNKCTL2_TLS_16_0GT0x0004 /* Supported Speed 16GT/s */ #define PCI_EXP_LNKCTL2_TLS_32_0GT0x0005 /* Supported Speed 32GT/s */ +#define PCI_EXP_LNKCTL2_ENTER_COMP0x0010 /* Enter Compliance */ +#define PCI_EXP_LNKCTL2_TX_MARGIN 0x0380 /* Transmit Margin */ #define PCI_EXP_LNKSTA250 /* Link Status 2 */ #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2 52 /* v2 endpoints with link end here */ #define PCI_EXP_SLTCAP252 /* Slot Capabilities 2 */ -- 2.24.0.432.g9d3f5f5b63-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: call drm_gem_object_funcs.mmap with fake offset
On Thu, Nov 21, 2019 at 02:52:59PM +0100, Daniel Vetter wrote:
> On Thu, Nov 21, 2019 at 11:38:06AM +0100, Gerd Hoffmann wrote:
> > The fake offset is going to stay, so change the calling convention for
> > drm_gem_object_funcs.mmap to include the fake offset. Update all users
> > accordingly.
>
> Please add to the commit message:
>
> Note that this reverts 83b8a6f242ea ("drm/gem: Fix mmap fake offset
> handling for drm_gem_object_funcs.mmap") and on top then adds the fake
> offset to drm_gem_prime_mmap to make sure all paths leading to
> obj->funcs->mmap are consistent.
>
> Fixes: 83b8a6f242ea ("drm/gem: Fix mmap fake offset handling for
> drm_gem_object_funcs.mmap")
> Cc: Gerd Hoffmann
> Cc: Daniel Vetter
> Cc: Rob Herring
>
> With that also Reviewed-by: Daniel Vetter
Also added Rob to cc here.
Rob, can you pls take a look an ack? The sage took another turn :-)
-Daniel
> -Daniel
> >
> > Signed-off-by: Gerd Hoffmann
> > ---
> > include/drm/drm_gem.h | 4 +---
> > drivers/gpu/drm/drm_gem.c | 3 ---
> > drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++
> > drivers/gpu/drm/drm_prime.c| 3 +++
> > drivers/gpu/drm/ttm/ttm_bo_vm.c| 7 ---
> > 5 files changed, 7 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> > index 97a48165642c..0b375069cd48 100644
> > --- a/include/drm/drm_gem.h
> > +++ b/include/drm/drm_gem.h
> > @@ -159,9 +159,7 @@ struct drm_gem_object_funcs {
> > *
> > * The callback is used by by both drm_gem_mmap_obj() and
> > * drm_gem_prime_mmap(). When @mmap is present @vm_ops is not
> > -* used, the @mmap callback must set vma->vm_ops instead. The @mmap
> > -* callback is always called with a 0 offset. The caller will remove
> > -* the fake offset as necessary.
> > +* used, the @mmap callback must set vma->vm_ops instead.
> > */
> > int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
> >
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index 2f2b889096b0..56f42e0f2584 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -1106,9 +1106,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj,
> > unsigned long obj_size,
> > return -EINVAL;
> >
> > if (obj->funcs && obj->funcs->mmap) {
> > - /* Remove the fake offset */
> > - vma->vm_pgoff -= drm_vma_node_start(>vma_node);
> > -
> > ret = obj->funcs->mmap(obj, vma);
> > if (ret)
> > return ret;
> > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > index 0810d3ef6961..a421a2eed48a 100644
> > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > @@ -528,6 +528,9 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj,
> > struct vm_area_struct *vma)
> > struct drm_gem_shmem_object *shmem;
> > int ret;
> >
> > + /* Remove the fake offset */
> > + vma->vm_pgoff -= drm_vma_node_start(>vma_node);
> > +
> > shmem = to_drm_gem_shmem_obj(obj);
> >
> > ret = drm_gem_shmem_get_pages(shmem);
> > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > index 0814211b0f3f..a9633bd241bb 100644
> > --- a/drivers/gpu/drm/drm_prime.c
> > +++ b/drivers/gpu/drm/drm_prime.c
> > @@ -714,6 +714,9 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj,
> > struct vm_area_struct *vma)
> > int ret;
> >
> > if (obj->funcs && obj->funcs->mmap) {
> > + /* Add the fake offset */
> > + vma->vm_pgoff += drm_vma_node_start(>vma_node);
> > +
> > ret = obj->funcs->mmap(obj, vma);
> > if (ret)
> > return ret;
> > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> > b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> > index e6495ca2630b..3e8c3de91ae4 100644
> > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> > @@ -514,13 +514,6 @@ EXPORT_SYMBOL(ttm_bo_mmap);
> > int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object
> > *bo)
> > {
> > ttm_bo_get(bo);
> > -
> > - /*
> > -* FIXME: _gem_object_funcs.mmap is called with the fake offset
> > -* removed. Add it back here until the rest of TTM works without it.
> > -*/
> > - vma->vm_pgoff += drm_vma_node_start(>base.vma_node);
> > -
> > ttm_bo_mmap_vma_setup(bo, vma);
> > return 0;
> > }
> > --
> > 2.18.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/1] drm: Prefer pcie_capability_read_word()
On Mon, Nov 18, 2019 at 12:42:25PM -0500, Alex Deucher wrote:
> On Mon, Nov 18, 2019 at 3:37 AM Frederick Lawler wrote:
> >
> > Commit 8c0d3a02c130 ("PCI: Add accessors for PCI Express Capability")
> > added accessors for the PCI Express Capability so that drivers didn't
> > need to be aware of differences between v1 and v2 of the PCI
> > Express Capability.
> >
> > Replace pci_read_config_word() and pci_write_config_word() calls with
> > pcie_capability_read_word() and pcie_capability_write_word().
> >
> > Signed-off-by: Frederick Lawler
> >
> > ---
> > V2
> > - Squash both drm commits into one
> > - Rebase ontop of d46eac1e658b
> > ---
> > drivers/gpu/drm/amd/amdgpu/cik.c | 63 ---
> > drivers/gpu/drm/amd/amdgpu/si.c | 71 +++
> > drivers/gpu/drm/radeon/cik.c | 70 ++
> > drivers/gpu/drm/radeon/si.c | 73
>
> Can you split this into two patches? One for amdgpu and one for radeon?
I split this, and I also went back and split the related patches that
preceded this one. I'll post the resulting series for reference.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm: share address space for dma bufs
On Thu, Nov 21, 2019 at 11:38:07AM +0100, Gerd Hoffmann wrote:
> Use the shared address space of the drm device (see drm_open() in
> drm_file.c) for dma-bufs too. That removes a difference betweem drm
> device mmap vmas and dma-buf mmap vmas and fixes corner cases like
> unmaps not working properly.
>
> Signed-off-by: Gerd Hoffmann
> ---
> drivers/gpu/drm/drm_prime.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index a9633bd241bb..c3fc341453c0 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -240,6 +240,7 @@ void drm_prime_destroy_file_private(struct
> drm_prime_file_private *prime_fpriv)
> struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
> struct dma_buf_export_info *exp_info)
> {
> + struct drm_gem_object *obj = exp_info->priv;
> struct dma_buf *dma_buf;
>
> dma_buf = dma_buf_export(exp_info);
> @@ -247,7 +248,8 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device
> *dev,
> return dma_buf;
>
> drm_dev_get(dev);
> - drm_gem_object_get(exp_info->priv);
> + drm_gem_object_get(obj);
> + dma_buf->file->f_mapping = obj->dev->anon_inode->i_mapping;
Can you pls also throw the change to amdgpu_gem_prime_export on top here?
Imo makes a lot more sense that way. With that added I'm happy to r-b.
-Daniel
>
> return dma_buf;
> }
> --
> 2.18.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: call drm_gem_object_funcs.mmap with fake offset
On Thu, Nov 21, 2019 at 11:38:06AM +0100, Gerd Hoffmann wrote:
> The fake offset is going to stay, so change the calling convention for
> drm_gem_object_funcs.mmap to include the fake offset. Update all users
> accordingly.
Please add to the commit message:
Note that this reverts 83b8a6f242ea ("drm/gem: Fix mmap fake offset
handling for drm_gem_object_funcs.mmap") and on top then adds the fake
offset to drm_gem_prime_mmap to make sure all paths leading to
obj->funcs->mmap are consistent.
Fixes: 83b8a6f242ea ("drm/gem: Fix mmap fake offset handling for
drm_gem_object_funcs.mmap")
Cc: Gerd Hoffmann
Cc: Daniel Vetter
Cc: Rob Herring
With that also Reviewed-by: Daniel Vetter
-Daniel
>
> Signed-off-by: Gerd Hoffmann
> ---
> include/drm/drm_gem.h | 4 +---
> drivers/gpu/drm/drm_gem.c | 3 ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++
> drivers/gpu/drm/drm_prime.c| 3 +++
> drivers/gpu/drm/ttm/ttm_bo_vm.c| 7 ---
> 5 files changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 97a48165642c..0b375069cd48 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -159,9 +159,7 @@ struct drm_gem_object_funcs {
>*
>* The callback is used by by both drm_gem_mmap_obj() and
>* drm_gem_prime_mmap(). When @mmap is present @vm_ops is not
> - * used, the @mmap callback must set vma->vm_ops instead. The @mmap
> - * callback is always called with a 0 offset. The caller will remove
> - * the fake offset as necessary.
> + * used, the @mmap callback must set vma->vm_ops instead.
>*/
> int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 2f2b889096b0..56f42e0f2584 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1106,9 +1106,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj,
> unsigned long obj_size,
> return -EINVAL;
>
> if (obj->funcs && obj->funcs->mmap) {
> - /* Remove the fake offset */
> - vma->vm_pgoff -= drm_vma_node_start(>vma_node);
> -
> ret = obj->funcs->mmap(obj, vma);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 0810d3ef6961..a421a2eed48a 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -528,6 +528,9 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct
> vm_area_struct *vma)
> struct drm_gem_shmem_object *shmem;
> int ret;
>
> + /* Remove the fake offset */
> + vma->vm_pgoff -= drm_vma_node_start(>vma_node);
> +
> shmem = to_drm_gem_shmem_obj(obj);
>
> ret = drm_gem_shmem_get_pages(shmem);
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 0814211b0f3f..a9633bd241bb 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -714,6 +714,9 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct
> vm_area_struct *vma)
> int ret;
>
> if (obj->funcs && obj->funcs->mmap) {
> + /* Add the fake offset */
> + vma->vm_pgoff += drm_vma_node_start(>vma_node);
> +
> ret = obj->funcs->mmap(obj, vma);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index e6495ca2630b..3e8c3de91ae4 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -514,13 +514,6 @@ EXPORT_SYMBOL(ttm_bo_mmap);
> int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
> {
> ttm_bo_get(bo);
> -
> - /*
> - * FIXME: _gem_object_funcs.mmap is called with the fake offset
> - * removed. Add it back here until the rest of TTM works without it.
> - */
> - vma->vm_pgoff += drm_vma_node_start(>base.vma_node);
> -
> ttm_bo_mmap_vma_setup(bo, vma);
> return 0;
> }
> --
> 2.18.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205049] garbled graphics
https://bugzilla.kernel.org/show_bug.cgi?id=205049 --- Comment #15 from Pierre-Eric Pelloux-Prayer (pierre-eric.pelloux-pra...@amd.com) --- I opened a Merge Request for Mesa to address this issue: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/2836 Testing it and reporting the results (here or in the MR comments) would be appreciated. Thanks! -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205589] Green screen crash with 3400G
https://bugzilla.kernel.org/show_bug.cgi?id=205589 --- Comment #4 from p...@spth.de --- Today, I've also installed a 5.1.21 kernel from Ubuntu on my Debian GNU/Linux testing system, and logged into XFCE thrice. I did not see the green screen crash with that kernel, though one I got a crash (system frozen, only mouse pointer still moveable) a few minutes after logging in. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd: Fix Kconfig indentation
Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- drivers/gpu/drm/amd/acp/Kconfig | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/acp/Kconfig b/drivers/gpu/drm/amd/acp/Kconfig index d968c2471412..19bae9100da4 100644 --- a/drivers/gpu/drm/amd/acp/Kconfig +++ b/drivers/gpu/drm/amd/acp/Kconfig @@ -2,11 +2,11 @@ menu "ACP (Audio CoProcessor) Configuration" config DRM_AMD_ACP - bool "Enable AMD Audio CoProcessor IP support" - depends on DRM_AMDGPU - select MFD_CORE - select PM_GENERIC_DOMAINS if PM - help + bool "Enable AMD Audio CoProcessor IP support" + depends on DRM_AMDGPU + select MFD_CORE + select PM_GENERIC_DOMAINS if PM + help Choose this option to enable ACP IP support for AMD SOCs. This adds the ACP (Audio CoProcessor) IP driver and wires it up into the amdgpu driver. The ACP block provides the DMA -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/sun4i: Fix Kconfig indentation
Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- drivers/gpu/drm/sun4i/Kconfig | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig index 37e90e42943f..5755f0432e77 100644 --- a/drivers/gpu/drm/sun4i/Kconfig +++ b/drivers/gpu/drm/sun4i/Kconfig @@ -17,18 +17,18 @@ config DRM_SUN4I if DRM_SUN4I config DRM_SUN4I_HDMI - tristate "Allwinner A10 HDMI Controller Support" - default DRM_SUN4I - help + tristate "Allwinner A10 HDMI Controller Support" + default DRM_SUN4I + help Choose this option if you have an Allwinner SoC with an HDMI controller. config DRM_SUN4I_HDMI_CEC - bool "Allwinner A10 HDMI CEC Support" - depends on DRM_SUN4I_HDMI - select CEC_CORE - select CEC_PIN - help + bool "Allwinner A10 HDMI CEC Support" + depends on DRM_SUN4I_HDMI + select CEC_CORE + select CEC_PIN + help Choose this option if you have an Allwinner SoC with an HDMI controller and want to use CEC. -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vc4: Fix Kconfig indentation
Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- drivers/gpu/drm/vc4/Kconfig | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig index 7c2317efd5b7..118e8a426b1a 100644 --- a/drivers/gpu/drm/vc4/Kconfig +++ b/drivers/gpu/drm/vc4/Kconfig @@ -22,9 +22,9 @@ config DRM_VC4 our display setup. config DRM_VC4_HDMI_CEC - bool "Broadcom VC4 HDMI CEC Support" - depends on DRM_VC4 - select CEC_CORE - help + bool "Broadcom VC4 HDMI CEC Support" + depends on DRM_VC4 + select CEC_CORE + help Choose this option if you have a Broadcom VC4 GPU and want to use CEC. -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 1:52 PM Mika Westerberg
wrote:
>
> On Thu, Nov 21, 2019 at 01:46:14PM +0200, Mika Westerberg wrote:
> > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote:
> > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg
> > > wrote:
> > > >
> > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote:
> > > > > > last week or so I found systems where the GPU was under the "PCI
> > > > > > Express Root Port" (name from lspci) and on those systems all of
> > > > > > that
> > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 one,
> > > > > > which also explains Mikas case that Thunderbolt stuff works as
> > > > > > devices
> > > > > > never get populated under this particular bridge controller, but
> > > > > > under
> > > > > > those "Root Port"s
> > > > >
> > > > > It always is a PCIe port, but its location within the SoC may matter.
> > > >
> > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called
> > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is
> > > > still the same.
> > > >
> > > > > Also some custom AML-based power management is involved and that may
> > > > > be making specific assumptions on the configuration of the SoC and the
> > > > > GPU at the time of its invocation which unfortunately are not known to
> > > > > us.
> > > > >
> > > > > However, it looks like the AML invoked to power down the GPU from
> > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at
> > > > > that point, so it looks like that AML tries to access device memory on
> > > > > the GPU (beyond the PCI config space) or similar which is not
> > > > > accessible in PCI power states below D0.
> > > >
> > > > Or the PCI config space of the GPU when the parent root port is in D3hot
> > > > (as it is the case here). Also then the GPU config space is not
> > > > accessible.
> > >
> > > Why would the parent port be in D3hot at that point? Wouldn't that be
> > > a suspend ordering violation?
> >
> > No. We put the GPU into D3hot first, then the root port and then turn
> > off the power resource (which is attached to the root port) resulting
> > the topology entering D3cold.
>
> I don't see that happening in the AML though.
>
> Basically the difference is that when Windows 7 or Linux (the _REV==5
> check) then we directly do link disable whereas in Windows 8+ we invoke
> LKDS() method that puts the link into L2/L3. None of the fields they
> access seem to touch the GPU itself.
>
> LKDS() for the first PEG port looks like this:
>
>P0L2 = One
>Sleep (0x10)
>Local0 = Zero
>While (P0L2)
>{
> If ((Local0 > 0x04))
> {
> Break
> }
>
> Sleep (0x10)
> Local0++
>}
>
> One thing that comes to mind is that the loop can end even if P0L2 is
> not cleared as it does only 5 iterations with 16 ms sleep between. Maybe
> Sleep() is implemented differently in Windows? I mean Linux may be
> "faster" here and return prematurely and if we leave the port into D0
> this does not happen, or something. I'm just throwing out ideas :)
>
keep in mind, that I am able to hit this bug with my python script:
https://raw.githubusercontent.com/karolherbst/pci-stub-runpm/master/nv_runpm_bug_test.py
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 01:46:14PM +0200, Mika Westerberg wrote:
> On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote:
> > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg
> > wrote:
> > >
> > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote:
> > > > > last week or so I found systems where the GPU was under the "PCI
> > > > > Express Root Port" (name from lspci) and on those systems all of that
> > > > > seems to work. So I am wondering if it's indeed just the 0x1901 one,
> > > > > which also explains Mikas case that Thunderbolt stuff works as devices
> > > > > never get populated under this particular bridge controller, but under
> > > > > those "Root Port"s
> > > >
> > > > It always is a PCIe port, but its location within the SoC may matter.
> > >
> > > Exactly. Intel hardware has PCIe ports on CPU side (these are called
> > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is
> > > still the same.
> > >
> > > > Also some custom AML-based power management is involved and that may
> > > > be making specific assumptions on the configuration of the SoC and the
> > > > GPU at the time of its invocation which unfortunately are not known to
> > > > us.
> > > >
> > > > However, it looks like the AML invoked to power down the GPU from
> > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at
> > > > that point, so it looks like that AML tries to access device memory on
> > > > the GPU (beyond the PCI config space) or similar which is not
> > > > accessible in PCI power states below D0.
> > >
> > > Or the PCI config space of the GPU when the parent root port is in D3hot
> > > (as it is the case here). Also then the GPU config space is not
> > > accessible.
> >
> > Why would the parent port be in D3hot at that point? Wouldn't that be
> > a suspend ordering violation?
>
> No. We put the GPU into D3hot first, then the root port and then turn
> off the power resource (which is attached to the root port) resulting
> the topology entering D3cold.
I don't see that happening in the AML though.
Basically the difference is that when Windows 7 or Linux (the _REV==5
check) then we directly do link disable whereas in Windows 8+ we invoke
LKDS() method that puts the link into L2/L3. None of the fields they
access seem to touch the GPU itself.
LKDS() for the first PEG port looks like this:
P0L2 = One
Sleep (0x10)
Local0 = Zero
While (P0L2)
{
If ((Local0 > 0x04))
{
Break
}
Sleep (0x10)
Local0++
}
One thing that comes to mind is that the loop can end even if P0L2 is
not cleared as it does only 5 iterations with 16 ms sleep between. Maybe
Sleep() is implemented differently in Windows? I mean Linux may be
"faster" here and return prematurely and if we leave the port into D0
this does not happen, or something. I'm just throwing out ideas :)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:46 PM Mika Westerberg wrote: > > On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > > wrote: > > > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > > last week or so I found systems where the GPU was under the "PCI > > > > > Express Root Port" (name from lspci) and on those systems all of that > > > > > seems to work. So I am wondering if it's indeed just the 0x1901 one, > > > > > which also explains Mikas case that Thunderbolt stuff works as devices > > > > > never get populated under this particular bridge controller, but under > > > > > those "Root Port"s > > > > > > > > It always is a PCIe port, but its location within the SoC may matter. > > > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > > > still the same. > > > yeah, I meant the bridge controller with the ID 0x1901 is on the CPU side. And if the Nvidia GPU is on a port on the PCH side it all seems to work just fine. > > > > Also some custom AML-based power management is involved and that may > > > > be making specific assumptions on the configuration of the SoC and the > > > > GPU at the time of its invocation which unfortunately are not known to > > > > us. > > > > > > > > However, it looks like the AML invoked to power down the GPU from > > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > > that point, so it looks like that AML tries to access device memory on > > > > the GPU (beyond the PCI config space) or similar which is not > > > > accessible in PCI power states below D0. > > > > > > Or the PCI config space of the GPU when the parent root port is in D3hot > > > (as it is the case here). Also then the GPU config space is not > > > accessible. > > > > Why would the parent port be in D3hot at that point? Wouldn't that be > > a suspend ordering violation? > > No. We put the GPU into D3hot first, then the root port and then turn > off the power resource (which is attached to the root port) resulting > the topology entering D3cold. > If the kernel does a D0 -> D3hot -> D0 cycle this works as well, but the power savings are way lower, so I kind of prefer skipping D3hot instead of D3cold. Skipping D3hot doesn't seem to make any difference in power savings in my testing. > > > I took a look at the HP Omen ACPI tables which has similar problem and > > > there is also check for Windows 7 (but not Linux) so I think one > > > alternative workaround would be to add these devices into > > > acpi_osi_dmi_table[] where .callback is set to dmi_disable_osi_win8 (or > > > pass 'acpi_osi="!Windows 2012"' in the kernel command line). > > > > I'd like to understand the facts that have been established so far > > before deciding what to do about them. :-) > > Yes, I agree :) > Yeah.. and I think those would be too many as we know of several models with this laptops from Lenovo, Dell and HP and random other models from random other OEMs. I think we won't ever be able to blacklist all models if we go that way as those might be just way too many. Also I know from some reports on bumblebee bugs (hitting the same issue essentially) that the acpi_osi overwrite didn't help every user. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 4/7] drm/sun4i: dsi: Handle bus clock explicitly
Hi Maxime,
On Sun, Nov 3, 2019 at 11:02 PM Maxime Ripard wrote:
>
> On Fri, Nov 01, 2019 at 07:42:55PM +0530, Jagan Teki wrote:
> > Hi Maxime,
> >
> > On Tue, Oct 29, 2019 at 2:24 PM Maxime Ripard wrote:
> > >
> > > On Tue, Oct 29, 2019 at 04:03:56AM +0530, Jagan Teki wrote:
> > > > > > explicit handling of common clock would require since the A64
> > > > > > doesn't need to mention the clock-names explicitly in dts since it
> > > > > > support only one bus clock.
> > > > > >
> > > > > > Also pass clk_id NULL instead "bus" to regmap clock init function
> > > > > > since the single clock variants no need to mention clock-names
> > > > > > explicitly.
> > > > >
> > > > > You don't need explicit clock handling. Passing NULL as the argument
> > > > > in regmap_init_mmio_clk will make it use the first clock, which is the
> > > > > bus clock.
> > > >
> > > > Indeed I tried that, since NULL clk_id wouldn't enable the bus clock
> > > > during regmap_mmio_gen_context code, passing NULL triggering vblank
> > > > timeout.
> > >
> > > There's a bunch of users of NULL in tree, so finding out why NULL
> > > doesn't work is the way forward.
> >
> > I'd have looked the some of the users before checking the code as
> > well. As I said passing NULL clk_id to devm_regmap_init_mmio_clk =>
> > __devm_regmap_init_mmio_clk would return before processing the clock.
> >
> > Here is the code snippet on the tree just to make sure I'm on the same
> > page or not.
> >
> > static struct regmap_mmio_context *regmap_mmio_gen_context(struct device
> > *dev,
> > const char *clk_id,
> > void __iomem *regs,
> > const struct regmap_config *config)
> > {
> > ---
> > --
> > if (clk_id == NULL)
> > return ctx;
> >
> > ctx->clk = clk_get(dev, clk_id);
> > if (IS_ERR(ctx->clk)) {
> > ret = PTR_ERR(ctx->clk);
> > goto err_free;
> > }
> >
> > ret = clk_prepare(ctx->clk);
> > if (ret < 0) {
> > clk_put(ctx->clk);
> > goto err_free;
> > }
> > -
> > ---
> > }
> >
> > Yes, I did check on the driver in the tree before committing explicit
> > clock handle, which make similar requirements like us in [1]. this
> > imx2 wdt driver is handling the explicit clock as well. I'm sure this
> > driver is updated as I have seen few changes related to this driver in
> > ML.
>
> I guess we have two ways to go at this then.
>
> Either we remove the return, but it might have a few side-effects, or
> we call clk_get with NULL or bus depending on the case, and then call
> regmap_mmio_attach_clk.
Thanks for the inputs.
Please have a look at this snippet, I have used your second
suggestions. let me know if you have any comments?
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 8fa90cfc2ac8..91c95e56d870 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -1109,24 +1109,36 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
return PTR_ERR(dsi->regulator);
}
-dsi->regs = devm_regmap_init_mmio_clk(dev, "bus", base,
- _dsi_regmap_config);
-if (IS_ERR(dsi->regs)) {
-dev_err(dev, "Couldn't create the DSI encoder regmap\n");
-return PTR_ERR(dsi->regs);
-}
-
dsi->reset = devm_reset_control_get_shared(dev, NULL);
if (IS_ERR(dsi->reset)) {
dev_err(dev, "Couldn't get our reset line\n");
return PTR_ERR(dsi->reset);
}
+dsi->regs = regmap_init_mmio(dev, base, _dsi_regmap_config);
+if (IS_ERR(dsi->regs)) {
+dev_err(dev, "Couldn't init regmap\n");
+return PTR_ERR(dsi->regs);
+}
+
+dsi->bus_clk = devm_clk_get(dev, NULL);
+if (IS_ERR(dsi->bus_clk)) {
+dev_err(dev, "Couldn't get the DSI bus clock\n");
+ret = PTR_ERR(dsi->bus_clk);
+goto err_regmap;
+} else {
+printk("Jagan.. Got the BUS clock\n");
+ret = regmap_mmio_attach_clk(dsi->regs, dsi->bus_clk);
+if (ret)
+goto err_bus_clk;
+}
+
if (dsi->variant->has_mod_clk) {
dsi->mod_clk = devm_clk_get(dev, "mod");
if (IS_ERR(dsi->mod_clk)) {
dev_err(dev, "Couldn't get the DSI mod clock\n");
-return PTR_ERR(dsi->mod_clk);
+ret = PTR_ERR(dsi->mod_clk);
+goto err_attach_clk;
}
}
@@ -1167,6 +1179,14 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
err_unprotect_clk:
if (dsi->variant->has_mod_clk)
clk_rate_exclusive_put(dsi->mod_clk);
+err_attach_clk:
+if (!IS_ERR(dsi->bus_clk))
+regmap_mmio_detach_clk(dsi->regs);
+err_bus_clk:
+if (!IS_ERR(dsi->bus_clk))
+
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:34:22PM +0100, Rafael J. Wysocki wrote: > On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg > wrote: > > > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > > last week or so I found systems where the GPU was under the "PCI > > > > Express Root Port" (name from lspci) and on those systems all of that > > > > seems to work. So I am wondering if it's indeed just the 0x1901 one, > > > > which also explains Mikas case that Thunderbolt stuff works as devices > > > > never get populated under this particular bridge controller, but under > > > > those "Root Port"s > > > > > > It always is a PCIe port, but its location within the SoC may matter. > > > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > > still the same. > > > > > Also some custom AML-based power management is involved and that may > > > be making specific assumptions on the configuration of the SoC and the > > > GPU at the time of its invocation which unfortunately are not known to > > > us. > > > > > > However, it looks like the AML invoked to power down the GPU from > > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > > that point, so it looks like that AML tries to access device memory on > > > the GPU (beyond the PCI config space) or similar which is not > > > accessible in PCI power states below D0. > > > > Or the PCI config space of the GPU when the parent root port is in D3hot > > (as it is the case here). Also then the GPU config space is not > > accessible. > > Why would the parent port be in D3hot at that point? Wouldn't that be > a suspend ordering violation? No. We put the GPU into D3hot first, then the root port and then turn off the power resource (which is attached to the root port) resulting the topology entering D3cold. > > I took a look at the HP Omen ACPI tables which has similar problem and > > there is also check for Windows 7 (but not Linux) so I think one > > alternative workaround would be to add these devices into > > acpi_osi_dmi_table[] where .callback is set to dmi_disable_osi_win8 (or > > pass 'acpi_osi="!Windows 2012"' in the kernel command line). > > I'd like to understand the facts that have been established so far > before deciding what to do about them. :-) Yes, I agree :) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:28 PM Mika Westerberg wrote: > > On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > > last week or so I found systems where the GPU was under the "PCI > > > Express Root Port" (name from lspci) and on those systems all of that > > > seems to work. So I am wondering if it's indeed just the 0x1901 one, > > > which also explains Mikas case that Thunderbolt stuff works as devices > > > never get populated under this particular bridge controller, but under > > > those "Root Port"s > > > > It always is a PCIe port, but its location within the SoC may matter. > > Exactly. Intel hardware has PCIe ports on CPU side (these are called > PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is > still the same. > > > Also some custom AML-based power management is involved and that may > > be making specific assumptions on the configuration of the SoC and the > > GPU at the time of its invocation which unfortunately are not known to > > us. > > > > However, it looks like the AML invoked to power down the GPU from > > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > > that point, so it looks like that AML tries to access device memory on > > the GPU (beyond the PCI config space) or similar which is not > > accessible in PCI power states below D0. > > Or the PCI config space of the GPU when the parent root port is in D3hot > (as it is the case here). Also then the GPU config space is not > accessible. Why would the parent port be in D3hot at that point? Wouldn't that be a suspend ordering violation? > I took a look at the HP Omen ACPI tables which has similar problem and > there is also check for Windows 7 (but not Linux) so I think one > alternative workaround would be to add these devices into > acpi_osi_dmi_table[] where .callback is set to dmi_disable_osi_win8 (or > pass 'acpi_osi="!Windows 2012"' in the kernel command line). I'd like to understand the facts that have been established so far before deciding what to do about them. :-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:17 PM Mika Westerberg wrote: > > On Thu, Nov 21, 2019 at 12:03:52PM +0100, Rafael J. Wysocki wrote: > > On Thu, Nov 21, 2019 at 11:14 AM Mika Westerberg > > wrote: > > > > > > On Wed, Nov 20, 2019 at 10:36:31PM +0100, Karol Herbst wrote: > > > > with the branch and patch applied: > > > > https://gist.githubusercontent.com/karolherbst/03c4c8141b0fa292d781badfa186479e/raw/5c62640afbc57d6e69ea924c338bd2836e770d02/gistfile1.txt > > > > > > Thanks for testing. Too bad it did not help :( I suppose there is no > > > change if you increase the delay to say 1s? > > > > Well, look at the original patch in this thread. > > > > What it does is to prevent the device (GPU in this particular case) > > from going into a PCI low-power state before invoking AML to power it > > down (the AML is still invoked after this patch AFAICS), so why would > > that have anything to do with the delays? > > Yes, I know what it does :) I was just thinking that maybe it's still > the link that does not come up when we go back to D0 I guess that's not > the case here. I'm not sure why that would be related to putting the device into, say, PCI D3 before invoking AML to remove power from it. If it is not in PCI D3 at this point, the AML still runs and still removes power from it IIUC, so on the way back the situation is the same regardless: the device has no power which (again) needs to be restored by AML. That (in principle) should not depend on what happened to the device before it lost power. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Wed, Nov 20, 2019 at 11:29:33PM +0100, Rafael J. Wysocki wrote: > > last week or so I found systems where the GPU was under the "PCI > > Express Root Port" (name from lspci) and on those systems all of that > > seems to work. So I am wondering if it's indeed just the 0x1901 one, > > which also explains Mikas case that Thunderbolt stuff works as devices > > never get populated under this particular bridge controller, but under > > those "Root Port"s > > It always is a PCIe port, but its location within the SoC may matter. Exactly. Intel hardware has PCIe ports on CPU side (these are called PEG, PCI Express Graphics, ports), and the PCH side. I think the IP is still the same. > Also some custom AML-based power management is involved and that may > be making specific assumptions on the configuration of the SoC and the > GPU at the time of its invocation which unfortunately are not known to > us. > > However, it looks like the AML invoked to power down the GPU from > acpi_pci_set_power_state() gets confused if it is not in PCI D0 at > that point, so it looks like that AML tries to access device memory on > the GPU (beyond the PCI config space) or similar which is not > accessible in PCI power states below D0. Or the PCI config space of the GPU when the parent root port is in D3hot (as it is the case here). Also then the GPU config space is not accessible. I took a look at the HP Omen ACPI tables which has similar problem and there is also check for Windows 7 (but not Linux) so I think one alternative workaround would be to add these devices into acpi_osi_dmi_table[] where .callback is set to dmi_disable_osi_win8 (or pass 'acpi_osi="!Windows 2012"' in the kernel command line). ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:03:52PM +0100, Rafael J. Wysocki wrote: > On Thu, Nov 21, 2019 at 11:14 AM Mika Westerberg > wrote: > > > > On Wed, Nov 20, 2019 at 10:36:31PM +0100, Karol Herbst wrote: > > > with the branch and patch applied: > > > https://gist.githubusercontent.com/karolherbst/03c4c8141b0fa292d781badfa186479e/raw/5c62640afbc57d6e69ea924c338bd2836e770d02/gistfile1.txt > > > > Thanks for testing. Too bad it did not help :( I suppose there is no > > change if you increase the delay to say 1s? > > Well, look at the original patch in this thread. > > What it does is to prevent the device (GPU in this particular case) > from going into a PCI low-power state before invoking AML to power it > down (the AML is still invoked after this patch AFAICS), so why would > that have anything to do with the delays? Yes, I know what it does :) I was just thinking that maybe it's still the link that does not come up when we go back to D0 I guess that's not the case here. > The only reason would be the AML running too early, but that doesn't > seem likely. IMO more likely is that the AML does something which > cannot be done to a device in a PCI low-power state. It may very well be the case. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:08 PM Rafael J. Wysocki wrote: > > On Thu, Nov 21, 2019 at 12:03 PM Rafael J. Wysocki wrote: > > > > On Thu, Nov 21, 2019 at 11:14 AM Mika Westerberg > > wrote: > > > > > > On Wed, Nov 20, 2019 at 10:36:31PM +0100, Karol Herbst wrote: > > > > with the branch and patch applied: > > > > https://gist.githubusercontent.com/karolherbst/03c4c8141b0fa292d781badfa186479e/raw/5c62640afbc57d6e69ea924c338bd2836e770d02/gistfile1.txt > > > > > > Thanks for testing. Too bad it did not help :( I suppose there is no > > > change if you increase the delay to say 1s? > > > > Well, look at the original patch in this thread. > > > > What it does is to prevent the device (GPU in this particular case) > > from going into a PCI low-power state before invoking AML to power it > > down (the AML is still invoked after this patch AFAICS), so why would > > that have anything to do with the delays? > > > > The only reason would be the AML running too early, but that doesn't > > seem likely. IMO more likely is that the AML does something which > > cannot be done to a device in a PCI low-power state. > > BTW, I'm wondering if anyone has tried to skip the AML instead of > skipping the PCI PM in this case (as of 5.4-rc that would be a similar > patch to skip the invocations of > __pci_start/complete_power_transition() in pci_set_power_state() for > the affected device). Moving the dev->broken_nv_runpm test into pci_platform_power_transition() (also for transitions into D0) would be sufficient for that test if I'm not mistaken. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 12:03 PM Rafael J. Wysocki wrote: > > On Thu, Nov 21, 2019 at 11:14 AM Mika Westerberg > wrote: > > > > On Wed, Nov 20, 2019 at 10:36:31PM +0100, Karol Herbst wrote: > > > with the branch and patch applied: > > > https://gist.githubusercontent.com/karolherbst/03c4c8141b0fa292d781badfa186479e/raw/5c62640afbc57d6e69ea924c338bd2836e770d02/gistfile1.txt > > > > Thanks for testing. Too bad it did not help :( I suppose there is no > > change if you increase the delay to say 1s? > > Well, look at the original patch in this thread. > > What it does is to prevent the device (GPU in this particular case) > from going into a PCI low-power state before invoking AML to power it > down (the AML is still invoked after this patch AFAICS), so why would > that have anything to do with the delays? > > The only reason would be the AML running too early, but that doesn't > seem likely. IMO more likely is that the AML does something which > cannot be done to a device in a PCI low-power state. BTW, I'm wondering if anyone has tried to skip the AML instead of skipping the PCI PM in this case (as of 5.4-rc that would be a similar patch to skip the invocations of __pci_start/complete_power_transition() in pci_set_power_state() for the affected device). ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Thu, Nov 21, 2019 at 11:14 AM Mika Westerberg wrote: > > On Wed, Nov 20, 2019 at 10:36:31PM +0100, Karol Herbst wrote: > > with the branch and patch applied: > > https://gist.githubusercontent.com/karolherbst/03c4c8141b0fa292d781badfa186479e/raw/5c62640afbc57d6e69ea924c338bd2836e770d02/gistfile1.txt > > Thanks for testing. Too bad it did not help :( I suppose there is no > change if you increase the delay to say 1s? Well, look at the original patch in this thread. What it does is to prevent the device (GPU in this particular case) from going into a PCI low-power state before invoking AML to power it down (the AML is still invoked after this patch AFAICS), so why would that have anything to do with the delays? The only reason would be the AML running too early, but that doesn't seem likely. IMO more likely is that the AML does something which cannot be done to a device in a PCI low-power state. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] drm: mmap fixups
Tested on qemu, with bochs (vram helpers) and cirrus (shmem helpers). Cc'ing intel-gfx for CI coverage. Gerd Hoffmann (2): drm: call drm_gem_object_funcs.mmap with fake offset drm: share address space for dma bufs include/drm/drm_gem.h | 4 +--- drivers/gpu/drm/drm_gem.c | 3 --- drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++ drivers/gpu/drm/drm_prime.c| 7 ++- drivers/gpu/drm/ttm/ttm_bo_vm.c| 7 --- 5 files changed, 10 insertions(+), 14 deletions(-) -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: call drm_gem_object_funcs.mmap with fake offset
The fake offset is going to stay, so change the calling convention for
drm_gem_object_funcs.mmap to include the fake offset. Update all users
accordingly.
Signed-off-by: Gerd Hoffmann
---
include/drm/drm_gem.h | 4 +---
drivers/gpu/drm/drm_gem.c | 3 ---
drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++
drivers/gpu/drm/drm_prime.c| 3 +++
drivers/gpu/drm/ttm/ttm_bo_vm.c| 7 ---
5 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 97a48165642c..0b375069cd48 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -159,9 +159,7 @@ struct drm_gem_object_funcs {
*
* The callback is used by by both drm_gem_mmap_obj() and
* drm_gem_prime_mmap(). When @mmap is present @vm_ops is not
-* used, the @mmap callback must set vma->vm_ops instead. The @mmap
-* callback is always called with a 0 offset. The caller will remove
-* the fake offset as necessary.
+* used, the @mmap callback must set vma->vm_ops instead.
*/
int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 2f2b889096b0..56f42e0f2584 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1106,9 +1106,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned
long obj_size,
return -EINVAL;
if (obj->funcs && obj->funcs->mmap) {
- /* Remove the fake offset */
- vma->vm_pgoff -= drm_vma_node_start(>vma_node);
-
ret = obj->funcs->mmap(obj, vma);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 0810d3ef6961..a421a2eed48a 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -528,6 +528,9 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct
vm_area_struct *vma)
struct drm_gem_shmem_object *shmem;
int ret;
+ /* Remove the fake offset */
+ vma->vm_pgoff -= drm_vma_node_start(>vma_node);
+
shmem = to_drm_gem_shmem_obj(obj);
ret = drm_gem_shmem_get_pages(shmem);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 0814211b0f3f..a9633bd241bb 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -714,6 +714,9 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct
vm_area_struct *vma)
int ret;
if (obj->funcs && obj->funcs->mmap) {
+ /* Add the fake offset */
+ vma->vm_pgoff += drm_vma_node_start(>vma_node);
+
ret = obj->funcs->mmap(obj, vma);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index e6495ca2630b..3e8c3de91ae4 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -514,13 +514,6 @@ EXPORT_SYMBOL(ttm_bo_mmap);
int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
{
ttm_bo_get(bo);
-
- /*
-* FIXME: _gem_object_funcs.mmap is called with the fake offset
-* removed. Add it back here until the rest of TTM works without it.
-*/
- vma->vm_pgoff += drm_vma_node_start(>base.vma_node);
-
ttm_bo_mmap_vma_setup(bo, vma);
return 0;
}
--
2.18.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm: share address space for dma bufs
Use the shared address space of the drm device (see drm_open() in
drm_file.c) for dma-bufs too. That removes a difference betweem drm
device mmap vmas and dma-buf mmap vmas and fixes corner cases like
unmaps not working properly.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/drm_prime.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index a9633bd241bb..c3fc341453c0 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -240,6 +240,7 @@ void drm_prime_destroy_file_private(struct
drm_prime_file_private *prime_fpriv)
struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
struct dma_buf_export_info *exp_info)
{
+ struct drm_gem_object *obj = exp_info->priv;
struct dma_buf *dma_buf;
dma_buf = dma_buf_export(exp_info);
@@ -247,7 +248,8 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device
*dev,
return dma_buf;
drm_dev_get(dev);
- drm_gem_object_get(exp_info->priv);
+ drm_gem_object_get(obj);
+ dma_buf->file->f_mapping = obj->dev->anon_inode->i_mapping;
return dma_buf;
}
--
2.18.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap
On Thu, Nov 21, 2019 at 11:18 AM Gerd Hoffmann wrote: > > On Thu, Nov 21, 2019 at 09:49:53AM +0100, Daniel Vetter wrote: > > On Thu, Nov 21, 2019 at 09:02:59AM +0100, Gerd Hoffmann wrote: > > > Hi, > > > > > > > > update-object-after-move works fine. > > > > > > > > > > try zap mappings with madvise(dontneed) has no bad effects (after vram > > > > > move, trying to force re-creating the ptes). > > > > > > > > Well if it's broken the zapping wouldn't work :-) > > > > > > > > > didn't try the memory pressure thing yet. > > > > > > > > I'm surprised ... and I have no idea how/why it keeps working. > > > > > > > > For my paranoia, can you instrument the ttm page fault handler, and > > > > double > > > > check that we get new faults after the move, and set up new ptes for the > > > > same old mapping, but now pointing at the new place in vram? > > > > > > Hmm, only the drm device mapping is faulted in after moving it, > > > the dma-buf mapping is not. Fixed by: > > > > Ah yes, that's more what I'd expect to happen, and the below is what I'd > > expect to fix things up. I think we should move it up ahead of the device > > callback (so that drivers can overwrite) and then push as a fix. Separate > > from a possible patch to undo the fake offset removal. > > -Daniel > > Is there a more elegant way than copying the intel list on non-intel > patches to kick intel ci? Nope, not really. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 6/6] drm/komeda: Expose side_by_side by sysfs/config_id
On Thu, Nov 21, 2019 at 10:49:26AM +0100, Daniel Vetter wrote:
> On Thu, Nov 21, 2019 at 07:12:55AM +, james qian wang (Arm Technology
> China) wrote:
> > There are some restrictions if HW works on side_by_side, expose it via
> > config_id to user.
> >
> > Signed-off-by: James Qian Wang (Arm Technology China)
> >
> > ---
> > drivers/gpu/drm/arm/display/include/malidp_product.h | 3 ++-
> > drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 1 +
> > 2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h
> > b/drivers/gpu/drm/arm/display/include/malidp_product.h
> > index 1053b11352eb..96e2e4016250 100644
> > --- a/drivers/gpu/drm/arm/display/include/malidp_product.h
> > +++ b/drivers/gpu/drm/arm/display/include/malidp_product.h
> > @@ -27,7 +27,8 @@ union komeda_config_id {
> > n_scalers:2, /* number of scalers per pipeline */
> > n_layers:3, /* number of layers per pipeline */
> > n_richs:3, /* number of rich layers per pipeline */
> > - reserved_bits:6;
> > + side_by_side:1, /* if HW works on side_by_side mode */
> > + reserved_bits:5;
> > };
> > __u32 value;
> > };
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > index c3fa4835cb8d..4dd4699d4e3d 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > @@ -83,6 +83,7 @@ config_id_show(struct device *dev, struct
> > device_attribute *attr, char *buf)
>
> Uh, this sysfs file here looks a lot like uapi for some compositor to
> decide what to do. Do you have the userspace for this?
Yes, our HWC driver uses this config_id and product_id for identifying the
HW caps.
> Also a few more thoughts on this:
> - You can't just add more fields to uapi structs.
> - This doesn't really feel like it was ever reviewed to fit into atomic.
> - sysfs should be one value per file, not a smorgasbrod of things stuffed
> into a binary structure.
I will sent a series and split this struct to multiple files.
| This doesn't really feel like it was ever reviewed to fit into atomic
These values don't have atomic problem, since config_id is for
representing the HW caps info, which are not configurable.
Thanks
James
> -Daniel
>
> > memset(_id, 0, sizeof(config_id));
> >
> > config_id.max_line_sz = pipe->layers[0]->hsize_in.end;
> > + config_id.side_by_side = mdev->side_by_side;
> > config_id.n_pipelines = mdev->n_pipelines;
> > config_id.n_scalers = pipe->n_scalers;
> > config_id.n_layers = pipe->n_layers;
> > --
> > 2.20.1
> >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 1/5] fbtft: Make sure string is NULL terminated
On 20/11/2019 09:57, Andy Shevchenko wrote: > New GCC warns about inappropriate use of strncpy(): > > drivers/staging/fbtft/fbtft-core.c: In function ‘fbtft_framebuffer_alloc’: > drivers/staging/fbtft/fbtft-core.c:665:2: warning: ‘strncpy’ specified bound > 16 equals destination size [-Wstringop-truncation] > 665 | strncpy(info->fix.id, dev->driver->name, 16); > | ^~~~ > > Later on the copy is being used with the assumption to be NULL terminated. > Make sure string is NULL terminated by switching to snprintf(). Even better would be strscpy(): strscpy(info->fix.id, dev->driver->name, sizeof(info->fix.id)); Steve > > Signed-off-by: Andy Shevchenko > --- > drivers/staging/fbtft/fbtft-core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/fbtft/fbtft-core.c > b/drivers/staging/fbtft/fbtft-core.c > index a0a67aa517f0..61f0286fb157 100644 > --- a/drivers/staging/fbtft/fbtft-core.c > +++ b/drivers/staging/fbtft/fbtft-core.c > @@ -666,7 +666,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct > fbtft_display *display, > fbdefio->deferred_io = fbtft_deferred_io; > fb_deferred_io_init(info); > > - strncpy(info->fix.id, dev->driver->name, 16); > + snprintf(info->fix.id, sizeof(info->fix.id), "%s", dev->driver->name); > info->fix.type = FB_TYPE_PACKED_PIXELS; > info->fix.visual = FB_VISUAL_TRUECOLOR; > info->fix.xpanstep = 0; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap
On Thu, Nov 21, 2019 at 09:49:53AM +0100, Daniel Vetter wrote: > On Thu, Nov 21, 2019 at 09:02:59AM +0100, Gerd Hoffmann wrote: > > Hi, > > > > > > update-object-after-move works fine. > > > > > > > > try zap mappings with madvise(dontneed) has no bad effects (after vram > > > > move, trying to force re-creating the ptes). > > > > > > Well if it's broken the zapping wouldn't work :-) > > > > > > > didn't try the memory pressure thing yet. > > > > > > I'm surprised ... and I have no idea how/why it keeps working. > > > > > > For my paranoia, can you instrument the ttm page fault handler, and double > > > check that we get new faults after the move, and set up new ptes for the > > > same old mapping, but now pointing at the new place in vram? > > > > Hmm, only the drm device mapping is faulted in after moving it, > > the dma-buf mapping is not. Fixed by: > > Ah yes, that's more what I'd expect to happen, and the below is what I'd > expect to fix things up. I think we should move it up ahead of the device > callback (so that drivers can overwrite) and then push as a fix. Separate > from a possible patch to undo the fake offset removal. > -Daniel Is there a more elegant way than copying the intel list on non-intel patches to kick intel ci? cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
On Wed, Nov 20, 2019 at 10:36:31PM +0100, Karol Herbst wrote: > with the branch and patch applied: > https://gist.githubusercontent.com/karolherbst/03c4c8141b0fa292d781badfa186479e/raw/5c62640afbc57d6e69ea924c338bd2836e770d02/gistfile1.txt Thanks for testing. Too bad it did not help :( I suppose there is no change if you increase the delay to say 1s? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm: Don't init ww_mutec acquire ctx before needed
On Wed, Nov 20, 2019 at 04:24:48PM -0800, Rob Clark wrote: > On Wed, Nov 20, 2019 at 2:56 AM Daniel Vetter wrote: > > > > For locking semantics it really doesn't matter when we grab the > > ticket. But for lockdep validation it does: the acquire ctx is a fake > > lockdep. Since other drivers might want to do a full multi-lock dance > > in their fault-handler, not just lock a single dma_resv. Therefore we > > must init the acquire_ctx only after we've done all the copy_*_user or > > anything else that might trigger a pagefault. For msm this means we > > need to move it past submit_lookup_objects. > > > > Aside: Why is msm still using struct_mutex, it seems to be using > > dma_resv_lock for general buffer state protection? > > > > v2: > > - Add comment to explain why the ww ticket setup is separate (Rob) > > - Fix up error handling, we need to make sure we don't call > > ww_acquire_fini without _init. > > > > Signed-off-by: Daniel Vetter > > Cc: Rob Clark > > Cc: Sean Paul > > Cc: linux-arm-...@vger.kernel.org > > Cc: freedr...@lists.freedesktop.org > > found a few minutes to take this for a spin and seems fine.. t-b && r-b Thanks for taking this for a spin, entire series applied. -Daniel > > BR, > -R > > > --- > > drivers/gpu/drm/msm/msm_gem_submit.c | 9 ++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c > > b/drivers/gpu/drm/msm/msm_gem_submit.c > > index fb1fdd7fa3ef..385d4965a8d0 100644 > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > > @@ -54,7 +54,6 @@ static struct msm_gem_submit *submit_create(struct > > drm_device *dev, > > > > INIT_LIST_HEAD(>node); > > INIT_LIST_HEAD(>bo_list); > > - ww_acquire_init(>ticket, _ww_class); > > > > return submit; > > } > > @@ -390,8 +389,6 @@ static void submit_cleanup(struct msm_gem_submit > > *submit) > > list_del_init(_obj->submit_entry); > > drm_gem_object_put(_obj->base); > > } > > - > > - ww_acquire_fini(>ticket); > > } > > > > int msm_ioctl_gem_submit(struct drm_device *dev, void *data, > > @@ -408,6 +405,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void > > *data, > > struct msm_ringbuffer *ring; > > int out_fence_fd = -1; > > struct pid *pid = get_pid(task_pid(current)); > > + bool has_ww_ticket = false; > > unsigned i; > > int ret, submitid; > > if (!gpu) > > @@ -489,6 +487,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void > > *data, > > if (ret) > > goto out; > > > > + /* copy_*_user while holding a ww ticket upsets lockdep */ > > + ww_acquire_init(>ticket, _ww_class); > > + has_ww_ticket = true; > > ret = submit_lock_objects(submit); > > if (ret) > > goto out; > > @@ -588,6 +589,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void > > *data, > > > > out: > > submit_cleanup(submit); > > + if (has_ww_ticket) > > + ww_acquire_fini(>ticket); > > if (ret) > > msm_gem_submit_free(submit); > > out_unlock: > > -- > > 2.24.0 > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 02/24] mm/gup: factor out duplicate code from four routines
On Thu 21-11-19 00:29:59, John Hubbard wrote: > > > > Otherwise this looks fine and might be a worthwhile cleanup to feed > > Andrew for 5.5 independent of the gut of the changes. > > > > Reviewed-by: Christoph Hellwig > > > > Thanks for the reviews! Say, it sounds like your view here is that this > series should be targeted at 5.6 (not 5.5), is that what you have in mind? > And get the preparatory patches (1-9, and maybe even 10-16) into 5.5? One more note :) If you are going to push pin_user_pages() interfaces (which I'm fine with), it would probably make sense to push also the put_user_pages() -> unpin_user_pages() renaming so that that inconsistency in naming does not exist in the released upstream kernel. Honza -- Jan Kara SUSE Labs, CR ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 6/6] drm/komeda: Expose side_by_side by sysfs/config_id
On Thu, Nov 21, 2019 at 07:12:55AM +, james qian wang (Arm Technology
China) wrote:
> There are some restrictions if HW works on side_by_side, expose it via
> config_id to user.
>
> Signed-off-by: James Qian Wang (Arm Technology China)
>
> ---
> drivers/gpu/drm/arm/display/include/malidp_product.h | 3 ++-
> drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h
> b/drivers/gpu/drm/arm/display/include/malidp_product.h
> index 1053b11352eb..96e2e4016250 100644
> --- a/drivers/gpu/drm/arm/display/include/malidp_product.h
> +++ b/drivers/gpu/drm/arm/display/include/malidp_product.h
> @@ -27,7 +27,8 @@ union komeda_config_id {
> n_scalers:2, /* number of scalers per pipeline */
> n_layers:3, /* number of layers per pipeline */
> n_richs:3, /* number of rich layers per pipeline */
> - reserved_bits:6;
> + side_by_side:1, /* if HW works on side_by_side mode */
> + reserved_bits:5;
> };
> __u32 value;
> };
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index c3fa4835cb8d..4dd4699d4e3d 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -83,6 +83,7 @@ config_id_show(struct device *dev, struct device_attribute
> *attr, char *buf)
Uh, this sysfs file here looks a lot like uapi for some compositor to
decide what to do. Do you have the userspace for this?
Also a few more thoughts on this:
- You can't just add more fields to uapi structs.
- This doesn't really feel like it was ever reviewed to fit into atomic.
- sysfs should be one value per file, not a smorgasbrod of things stuffed
into a binary structure.
-Daniel
> memset(_id, 0, sizeof(config_id));
>
> config_id.max_line_sz = pipe->layers[0]->hsize_in.end;
> + config_id.side_by_side = mdev->side_by_side;
> config_id.n_pipelines = mdev->n_pipelines;
> config_id.n_scalers = pipe->n_scalers;
> config_id.n_layers = pipe->n_layers;
> --
> 2.20.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 02/24] mm/gup: factor out duplicate code from four routines
On Thu 21-11-19 00:29:59, John Hubbard wrote: > On 11/21/19 12:03 AM, Christoph Hellwig wrote: > > Otherwise this looks fine and might be a worthwhile cleanup to feed > > Andrew for 5.5 independent of the gut of the changes. > > > > Reviewed-by: Christoph Hellwig > > > > Thanks for the reviews! Say, it sounds like your view here is that this > series should be targeted at 5.6 (not 5.5), is that what you have in mind? > And get the preparatory patches (1-9, and maybe even 10-16) into 5.5? Yeah, actually I feel the same. The merge window is going to open on Sunday and the series isn't still fully baked and happily sitting in linux-next (and larger changes should really sit in linux-next for at least a week, preferably two, before the merge window opens to get some reasonable test coverage). So I'd take out the independent easy patches that are already reviewed, get them merged into Andrew's (or whatever other appropriate tree) now so that they get at least a week of testing in linux-next before going upstream. And the more involved bits will have to wait for 5.6 - which means let's just continue working on them as we do now because ideally in 4 weeks we should have them ready with all the reviews so that they can be picked up and integrated into linux-next. Honza -- Jan Kara SUSE Labs, CR ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 17/24] mm/gup: track FOLL_PIN pages
On Wed 20-11-19 23:13:47, John Hubbard wrote:
> Add tracking of pages that were pinned via FOLL_PIN.
>
> As mentioned in the FOLL_PIN documentation, callers who effectively set
> FOLL_PIN are required to ultimately free such pages via put_user_page().
> The effect is similar to FOLL_GET, and may be thought of as "FOLL_GET
> for DIO and/or RDMA use".
>
> Pages that have been pinned via FOLL_PIN are identifiable via a
> new function call:
>
>bool page_dma_pinned(struct page *page);
>
> What to do in response to encountering such a page, is left to later
> patchsets. There is discussion about this in [1], [2], and [3].
>
> This also changes a BUG_ON(), to a WARN_ON(), in follow_page_mask().
>
> [1] Some slow progress on get_user_pages() (Apr 2, 2019):
> https://lwn.net/Articles/784574/
> [2] DMA and get_user_pages() (LPC: Dec 12, 2018):
> https://lwn.net/Articles/774411/
> [3] The trouble with get_user_pages() (Apr 30, 2018):
> https://lwn.net/Articles/753027/
>
> Suggested-by: Jan Kara
> Suggested-by: Jérôme Glisse
> Signed-off-by: John Hubbard
Thanks for the patch! We are mostly getting there. Some smaller comments
below.
> +/**
> + * try_pin_compound_head() - mark a compound page as being used by
> + * pin_user_pages*().
> + *
> + * This is the FOLL_PIN counterpart to try_get_compound_head().
> + *
> + * @page:pointer to page to be marked
> + * @Return: true for success, false for failure
> + */
> +__must_check bool try_pin_compound_head(struct page *page, int refs)
> +{
> + page = try_get_compound_head(page, GUP_PIN_COUNTING_BIAS * refs);
> + if (!page)
> + return false;
> +
> + __update_proc_vmstat(page, NR_FOLL_PIN_REQUESTED, refs);
> + return true;
> +}
> +
> +#ifdef CONFIG_DEV_PAGEMAP_OPS
> +static bool __put_devmap_managed_user_page(struct page *page)
Probably call this __unpin_devmap_managed_user_page()? To match the later
conversion of put_user_page() to unpin_user_page()?
> +{
> + bool is_devmap = page_is_devmap_managed(page);
> +
> + if (is_devmap) {
> + int count = page_ref_sub_return(page, GUP_PIN_COUNTING_BIAS);
> +
> + __update_proc_vmstat(page, NR_FOLL_PIN_RETURNED, 1);
> + /*
> + * devmap page refcounts are 1-based, rather than 0-based: if
> + * refcount is 1, then the page is free and the refcount is
> + * stable because nobody holds a reference on the page.
> + */
> + if (count == 1)
> + free_devmap_managed_page(page);
> + else if (!count)
> + __put_page(page);
> + }
> +
> + return is_devmap;
> +}
> +#else
> +static bool __put_devmap_managed_user_page(struct page *page)
> +{
> + return false;
> +}
> +#endif /* CONFIG_DEV_PAGEMAP_OPS */
> +
> +/**
> + * put_user_page() - release a dma-pinned page
> + * @page:pointer to page to be released
> + *
> + * Pages that were pinned via pin_user_pages*() must be released via either
> + * put_user_page(), or one of the put_user_pages*() routines. This is so that
> + * such pages can be separately tracked and uniquely handled. In particular,
> + * interactions with RDMA and filesystems need special handling.
> + */
> +void put_user_page(struct page *page)
> +{
> + page = compound_head(page);
> +
> + /*
> + * For devmap managed pages we need to catch refcount transition from
> + * GUP_PIN_COUNTING_BIAS to 1, when refcount reach one it means the
> + * page is free and we need to inform the device driver through
> + * callback. See include/linux/memremap.h and HMM for details.
> + */
> + if (__put_devmap_managed_user_page(page))
> + return;
> +
> + if (page_ref_sub_and_test(page, GUP_PIN_COUNTING_BIAS))
> + __put_page(page);
> +
> + __update_proc_vmstat(page, NR_FOLL_PIN_RETURNED, 1);
> +}
> +EXPORT_SYMBOL(put_user_page);
> +
> /**
> * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned
> pages
> * @pages: array of pages to be maybe marked dirty, and definitely released.
> @@ -237,10 +327,11 @@ static struct page *follow_page_pte(struct
> vm_area_struct *vma,
> }
>
> page = vm_normal_page(vma, address, pte);
> - if (!page && pte_devmap(pte) && (flags & FOLL_GET)) {
> + if (!page && pte_devmap(pte) && (flags & (FOLL_GET | FOLL_PIN))) {
> /*
> - * Only return device mapping pages in the FOLL_GET case since
> - * they are only valid while holding the pgmap reference.
> + * Only return device mapping pages in the FOLL_GET or FOLL_PIN
> + * case since they are only valid while holding the pgmap
> + * reference.
>*/
> *pgmap = get_dev_pagemap(pte_pfn(pte), *pgmap);
> if (*pgmap)
> @@ -283,6 +374,11 @@ static struct page *follow_page_pte(struct
> vm_area_struct *vma,
>
Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
On Thu, Nov 21, 2019 at 9:26 AM Timo Aaltonen wrote: > > On 9.10.2019 18.50, Daniel Vetter wrote: > > On Thu, Oct 03, 2019 at 11:31:25AM -0700, Juston Li wrote: > >> From: Daniel Stone > >> > >> getfb2 allows us to pass multiple planes and modifiers, just like addfb2 > >> over addfb. > >> > >> Changes since v1: > >> - unused modifiers set to 0 instead of DRM_FORMAT_MOD_INVALID > >> - update ioctl number > >> > >> Signed-off-by: Daniel Stone > >> Signed-off-by: Juston Li > > > > Looks all good from a very quick glance (kernel, libdrm, igt), but where's > > the userspace? Link to weston/drm_hwc/whatever MR good enough. > > -Daniel > > xserver too > > https://lists.x.org/archives/xorg-devel/2018-March/056363.html > > to fix > > https://gitlab.freedesktop.org/xorg/xserver/issues/33 > > which forces Ubuntu to disable CCS compression, and I'd like to get rid > of that patch. > > Thanks Juston for pushing this! Acked-by: Daniel Vetter ... but someone needs to review all the patches and make sure kernel patch + igt work and pass intel CI and then push it all, and given the pile of committers we have that's not me I think. Just in case people expect me to push this forward, I only jumped in here to make sure new uapi is done by the book and checks all the boxes. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: build failure after merge of the tip tree
sure, this is a issue.
but it wiil build succesed on local drm-next branch.
and you can submit this patch to fix this issue.
thanks.
Reviewed-by: Kevin Wang
From: Stephen Rothwell
Sent: Thursday, November 21, 2019 11:54 AM
To: Thomas Gleixner; Ingo Molnar; H. Peter Anvin; Peter Zijlstra; Dave Airlie;
DRI
Cc: Linux Next Mailing List; Linux Kernel Mailing List; Wang, Kevin(Yang);
Deucher, Alexander
Subject: linux-next: build failure after merge of the tip tree
Hi all,
After merging the tip tree, today's linux-next build (x86_64 allmodconfig)
failed like this:
In file included from include/trace/define_trace.h:102,
from drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:502,
from drivers/gpu/drm/amd/amdgpu/amdgpu_trace_points.c:29:
include/trace/../../drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:476:52: error:
expected expression before ';' token
476 | __string(ring, sched_job->base.sched->name);
|^
include/trace/trace_events.h:435:2: note: in definition of macro
'DECLARE_EVENT_CLASS'
435 | tstruct\
| ^~~
include/trace/trace_events.h:77:9: note: in expansion of macro 'PARAMS'
77 | PARAMS(tstruct), \
| ^~
include/trace/../../drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:472:1: note: in
expansion of macro 'TRACE_EVENT'
472 | TRACE_EVENT(amdgpu_ib_pipe_sync,
| ^~~
include/trace/../../drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h:475:6: note: in
expansion of macro 'TP_STRUCT__entry'
475 | TP_STRUCT__entry(
| ^~~~
Caused by commit
2c2fdb8bca29 ("drm/amdgpu: fix amdgpu trace event print string format error")
from the drm tree interacting with commit
60fdad00827c ("ftrace: Rework event_create_dir()")
from the tip tree.
I have added the following merge fix patch:
From: Stephen Rothwell
Date: Thu, 21 Nov 2019 14:46:00 +1100
Subject: [PATCH] merge fix for "ftrace: Rework event_create_dir()"
Signed-off-by: Stephen Rothwell
---
drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
index f940526c5889..63e734a125fb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -473,7 +473,7 @@ TRACE_EVENT(amdgpu_ib_pipe_sync,
TP_PROTO(struct amdgpu_job *sched_job, struct dma_fence *fence),
TP_ARGS(sched_job, fence),
TP_STRUCT__entry(
-__string(ring, sched_job->base.sched->name);
+__string(ring, sched_job->base.sched->name)
__field(uint64_t, id)
__field(struct dma_fence *, fence)
__field(uint64_t, ctx)
--
2.23.0
--
Cheers,
Stephen Rothwell
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 05/24] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages
On 11/21/19 12:05 AM, Christoph Hellwig wrote: So while this looks correct and I still really don't see the major benefit of the new code organization, especially as it bloats all put_page callers. I'd love to see code size change stats for an allyesconfig on this commit. Right, I'm running that now, will post the results. (btw, if there is a script and/or standard format I should use, I'm all ears. I'll dig through lwn...) thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 09/24] vfio, mm: fix get_user_pages_remote() and FOLL_LONGTERM
On 11/21/19 12:10 AM, Christoph Hellwig wrote: Should this be two patches, one for th core infrastructure and one for the user? These changes also look like another candidate to pre-load. OK, I'll split them up. thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap
On Thu, Nov 21, 2019 at 09:02:59AM +0100, Gerd Hoffmann wrote:
> Hi,
>
> > > update-object-after-move works fine.
> > >
> > > try zap mappings with madvise(dontneed) has no bad effects (after vram
> > > move, trying to force re-creating the ptes).
> >
> > Well if it's broken the zapping wouldn't work :-)
> >
> > > didn't try the memory pressure thing yet.
> >
> > I'm surprised ... and I have no idea how/why it keeps working.
> >
> > For my paranoia, can you instrument the ttm page fault handler, and double
> > check that we get new faults after the move, and set up new ptes for the
> > same old mapping, but now pointing at the new place in vram?
>
> Hmm, only the drm device mapping is faulted in after moving it,
> the dma-buf mapping is not. Fixed by:
Ah yes, that's more what I'd expect to happen, and the below is what I'd
expect to fix things up. I think we should move it up ahead of the device
callback (so that drivers can overwrite) and then push as a fix. Separate
from a possible patch to undo the fake offset removal.
-Daniel
>
> -- cut here
> From 3a7f6c6dbf3b1e4b412c2b283b2ea4edff9f33b5 Mon Sep 17 00:00:00 2001
> From: Gerd Hoffmann
> Date: Thu, 21 Nov 2019 08:39:17 +0100
> Subject: [PATCH] drm: share address space for dma bufs
>
> ---
> drivers/gpu/drm/drm_prime.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 0814211b0f3f..07c88d2aedee 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -240,6 +240,7 @@ void drm_prime_destroy_file_private(struct
> drm_prime_file_private *prime_fpriv)
> struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
> struct dma_buf_export_info *exp_info)
> {
> + struct drm_gem_object *obj = exp_info->priv;
> struct dma_buf *dma_buf;
>
> dma_buf = dma_buf_export(exp_info);
> @@ -247,7 +248,8 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device
> *dev,
> return dma_buf;
>
> drm_dev_get(dev);
> - drm_gem_object_get(exp_info->priv);
> + drm_gem_object_get(obj);
> + dma_buf->file->f_mapping = obj->dev->anon_inode->i_mapping;
>
> return dma_buf;
> }
> --
> 2.18.1
>
> -- cut here
>
> git branch: https://git.kraxel.org/cgit/linux/log/?h=drm-mmap-debug
>
> cheers,
> Gerd
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap
On Thu, Nov 21, 2019 at 09:10:21AM +0100, Gerd Hoffmann wrote: > Hi, > > > Aside: the amdgpu isn't great because it's racy, userspace could have > > guessed the fd and already started an mmap before we managed to update > > stuff. > > Can't see that race. When I read the code correctly the fd is created > and installed (using dma_buf_fd) only after dmabuf setup is finished. Right, I mixed things up. Looks all good. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: locking refcounting for ttm_bo_kmap/dma_buf_vmap
On Thu, Nov 21, 2019 at 8:59 AM Thomas Zimmermann wrote: > > Hi > > Am 20.11.19 um 12:47 schrieb Daniel Vetter: > > Hi all, > > > > I've been looking at dma_buf_v(un)map, with a goal to standardize > > locking for at least dynamic dma-buf exporters/importers, most likely > > by requiring dma_resv_lock. And I got questions around how this is > > supposed to work, since a big chunk of drivers seem to entirely lack > > locking around ttm_bo_kmap. Two big ones: > > > > - ttm_bo_kmap looks at bo->mem to figure out what/where to kmap to get > > at that buffer. bo->mem is supposed to be protected with > > dma_resv_lock, but at least amgpu/nouveau/radeon/qxl don't grab that > > in their prime vmap function. > > > > - between the vmap and vunmap something needs to make sure the backing > > storage doesn't move around. I didn't find that either anywhere, > > ttm_bo_kmap simply seems to set up the mapping, leaving locking and > > refcounting to callers. > > You have to pin and unpin storage before and after mapping. Yeah, but you = the exporter, not the importer. I.e. the vmap callback should pin and vunmap should unpin. It's not a huge deal since currently all users of dma_buf_vmap I've found do a dma_buf_attach (even if they never use the attachment directly), and with drm prime helpers the attach results in a pin. Or that's at least the story I told myself to believe this is all not a totally fireworks show right now :-) > > - vram helpers have at least locking, but I'm still missing the > > refcounting. vmwgfx doesn't bother with vmap. > > There's a ref counter for pinning [1] and there's a counter for mapping. > [2] Are you looking for something else? vram helpers actually looked pretty good in this regard. If you look at other callers of ttm_bo_kmap, they're a lot more fast here. -Daniel > > Best regards > Thomas > > [1] > https://cgit.freedesktop.org/drm/drm-tip/tree/include/drm/drm_gem_vram_helper.h?id=8d3996ceedcd5c64f5a354e9dcc64db4a1f72dd6#n69 > [2] > https://cgit.freedesktop.org/drm/drm-tip/tree/include/drm/drm_gem_vram_helper.h?id=8d3996ceedcd5c64f5a354e9dcc64db4a1f72dd6#n63 > > > > > What am I missing? > > > > Thanks, Daniel > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 06/24] goldish_pipe: rename local pin_user_pages() routine
On 11/21/19 12:08 AM, Christoph Hellwig wrote: On Wed, Nov 20, 2019 at 11:13:36PM -0800, John Hubbard wrote: +static int pin_goldfish_pages(unsigned long first_page, + unsigned long last_page, + unsigned int last_page_size, + int is_write, + struct page *pages[MAX_BUFFERS_PER_COMMAND], + unsigned int *iter_last_page_size) Why not goldfish_pin_pages? Normally we put the module / subsystem in front. Heh, is that how it's supposed to go? Sure, I'll change it. :) Also can we get this queued up for 5.5 to get some trivial changes out of the way? Is that a question to Andrew, or a request for me to send this as a separate patch email (or both)? thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: build failure after merge of the tip tree
On Thu, Nov 21, 2019 at 02:54:03PM +1100, Stephen Rothwell wrote: > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > index f940526c5889..63e734a125fb 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > @@ -473,7 +473,7 @@ TRACE_EVENT(amdgpu_ib_pipe_sync, > TP_PROTO(struct amdgpu_job *sched_job, struct dma_fence *fence), > TP_ARGS(sched_job, fence), > TP_STRUCT__entry( > - __string(ring, sched_job->base.sched->name); > + __string(ring, sched_job->base.sched->name) >__field(uint64_t, id) >__field(struct dma_fence *, fence) >__field(uint64_t, ctx) Correct, ';' there is invalid and now results in very verbose compile errors :-) signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 02/24] mm/gup: factor out duplicate code from four routines
On 11/21/19 12:03 AM, Christoph Hellwig wrote:
On Wed, Nov 20, 2019 at 11:13:32PM -0800, John Hubbard wrote:
There are four locations in gup.c that have a fair amount of code
duplication. This means that changing one requires making the same
changes in four places, not to mention reading the same code four
times, and wondering if there are subtle differences.
Factor out the common code into static functions, thus reducing the
overall line count and the code's complexity.
Also, take the opportunity to slightly improve the efficiency of the
error cases, by doing a mass subtraction of the refcount, surrounded
by get_page()/put_page().
Also, further simplify (slightly), by waiting until the the successful
end of each routine, to increment *nr.
Any reason for the spurious underscore in the function name?
argghh, I just fixed that, but applied the fix to the wrong patch! So now
patch 17 ("mm/gup: track FOLL_PIN pages") is improperly renaming it, instead
of this patch naming it correctly in the first place. Will fix.
Otherwise this looks fine and might be a worthwhile cleanup to feed
Andrew for 5.5 independent of the gut of the changes.
Reviewed-by: Christoph Hellwig
Thanks for the reviews! Say, it sounds like your view here is that this
series should be targeted at 5.6 (not 5.5), is that what you have in mind?
And get the preparatory patches (1-9, and maybe even 10-16) into 5.5?
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 01/24] mm/gup: pass flags arg to __gup_device_* functions
On 11/21/19 12:06 AM, Christoph Hellwig wrote: On Wed, Nov 20, 2019 at 11:13:31PM -0800, John Hubbard wrote: A subsequent patch requires access to gup flags, so pass the flags argument through to the __gup_device_* functions. Looks fine, but why not fold this into the patch using the flags. Yes, I'll do that. Also you can use up your full 73 chars per line in the commit log. OK. thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
On 9.10.2019 18.50, Daniel Vetter wrote: > On Thu, Oct 03, 2019 at 11:31:25AM -0700, Juston Li wrote: >> From: Daniel Stone >> >> getfb2 allows us to pass multiple planes and modifiers, just like addfb2 >> over addfb. >> >> Changes since v1: >> - unused modifiers set to 0 instead of DRM_FORMAT_MOD_INVALID >> - update ioctl number >> >> Signed-off-by: Daniel Stone >> Signed-off-by: Juston Li > > Looks all good from a very quick glance (kernel, libdrm, igt), but where's > the userspace? Link to weston/drm_hwc/whatever MR good enough. > -Daniel xserver too https://lists.x.org/archives/xorg-devel/2018-March/056363.html to fix https://gitlab.freedesktop.org/xorg/xserver/issues/33 which forces Ubuntu to disable CCS compression, and I'd like to get rid of that patch. Thanks Juston for pushing this! -- t ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] drm/komeda: Enable new product D32 support
D32 is simple version of D71, the difference is:
- Only has one pipeline
- Drop the periph block and merge it to GCU
v2: Rebase.
Signed-off-by: James Qian Wang (Arm Technology China)
---
.../drm/arm/display/include/malidp_product.h | 3 +-
.../arm/display/komeda/d71/d71_component.c| 2 +-
.../gpu/drm/arm/display/komeda/d71/d71_dev.c | 43 ---
.../gpu/drm/arm/display/komeda/d71/d71_regs.h | 13 ++
.../gpu/drm/arm/display/komeda/komeda_drv.c | 1 +
5 files changed, 44 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h
b/drivers/gpu/drm/arm/display/include/malidp_product.h
index 96e2e4016250..dbd3d4765065 100644
--- a/drivers/gpu/drm/arm/display/include/malidp_product.h
+++ b/drivers/gpu/drm/arm/display/include/malidp_product.h
@@ -18,7 +18,8 @@
#define MALIDP_CORE_ID_STATUS(__core_id) (((__u32)(__core_id)) & 0xFF)
/* Mali-display product IDs */
-#define MALIDP_D71_PRODUCT_ID 0x0071
+#define MALIDP_D71_PRODUCT_ID 0x0071
+#define MALIDP_D32_PRODUCT_ID 0x0032
union komeda_config_id {
struct {
diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
index 6dadf4413ef3..c7f7e9c545c7 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
@@ -1274,7 +1274,7 @@ static int d71_timing_ctrlr_init(struct d71_dev *d71,
ctrlr = to_ctrlr(c);
- ctrlr->supports_dual_link = true;
+ ctrlr->supports_dual_link = d71->supports_dual_link;
return 0;
}
diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
index 9b3bf353b6cc..2d429e310e5b 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
@@ -371,23 +371,33 @@ static int d71_enum_resources(struct komeda_dev *mdev)
goto err_cleanup;
}
- /* probe PERIPH */
+ /* Only the legacy HW has the periph block, the newer merges the periph
+* into GCU
+*/
value = malidp_read32(d71->periph_addr, BLK_BLOCK_INFO);
- if (BLOCK_INFO_BLK_TYPE(value) != D71_BLK_TYPE_PERIPH) {
- DRM_ERROR("access blk periph but got blk: %d.\n",
- BLOCK_INFO_BLK_TYPE(value));
- err = -EINVAL;
- goto err_cleanup;
+ if (BLOCK_INFO_BLK_TYPE(value) != D71_BLK_TYPE_PERIPH)
+ d71->periph_addr = NULL;
+
+ if (d71->periph_addr) {
+ /* probe PERIPHERAL in legacy HW */
+ value = malidp_read32(d71->periph_addr,
PERIPH_CONFIGURATION_ID);
+
+ d71->max_line_size = value & PERIPH_MAX_LINE_SIZE ? 4096 :
2048;
+ d71->max_vsize = 4096;
+ d71->num_rich_layers= value & PERIPH_NUM_RICH_LAYERS ? 2 :
1;
+ d71->supports_dual_link = !!(value & PERIPH_SPLIT_EN);
+ d71->integrates_tbu = !!(value & PERIPH_TBU_EN);
+ } else {
+ value = malidp_read32(d71->gcu_addr, GCU_CONFIGURATION_ID0);
+ d71->max_line_size = GCU_MAX_LINE_SIZE(value);
+ d71->max_vsize = GCU_MAX_NUM_LINES(value);
+
+ value = malidp_read32(d71->gcu_addr, GCU_CONFIGURATION_ID1);
+ d71->num_rich_layers= GCU_NUM_RICH_LAYERS(value);
+ d71->supports_dual_link = GCU_DISPLAY_SPLIT_EN(value);
+ d71->integrates_tbu = GCU_DISPLAY_TBU_EN(value);
}
- value = malidp_read32(d71->periph_addr, PERIPH_CONFIGURATION_ID);
-
- d71->max_line_size = value & PERIPH_MAX_LINE_SIZE ? 4096 : 2048;
- d71->max_vsize = 4096;
- d71->num_rich_layers= value & PERIPH_NUM_RICH_LAYERS ? 2 : 1;
- d71->supports_dual_link = value & PERIPH_SPLIT_EN ? true : false;
- d71->integrates_tbu = value & PERIPH_TBU_EN ? true : false;
-
for (i = 0; i < d71->num_pipelines; i++) {
pipe = komeda_pipeline_add(mdev, sizeof(struct d71_pipeline),
_pipeline_funcs);
@@ -415,7 +425,7 @@ static int d71_enum_resources(struct komeda_dev *mdev)
}
/* loop the register blks and probe */
- i = 2; /* exclude GCU and PERIPH */
+ i = 1; /* exclude GCU */
offset = D71_BLOCK_SIZE; /* skip GCU */
while (i < d71->num_blocks) {
blk_base = mdev->reg_base + (offset >> 2);
@@ -425,9 +435,9 @@ static int d71_enum_resources(struct komeda_dev *mdev)
err = d71_probe_block(d71, , blk_base);
if (err)
goto err_cleanup;
- i++;
}
+ i++;
offset += D71_BLOCK_SIZE;
}
@@ -603,6 +613,7 @@ d71_identify(u32 __iomem
