Re: [PATCH] drm/gem_shmem: Use a writecombine mapping for ->vaddr
On Wed, May 29, 2019 at 08:51:21AM +0200, Boris Brezillon wrote:
> Right now, the BO is mapped as a cached region when ->vmap() is called
> and the underlying object is not a dmabuf.
> Doing that makes cache management a bit more complicated (you'd need
> to call dma_map/unmap_sg() on the ->sgt field everytime the BO is about
> to be passed to the GPU/CPU), so let's map the BO with writecombine
> attributes instead (as done in most drivers).
>
> Signed-off-by: Boris Brezillon
> ---
> Found this issue while working on panfrost perfcnt where the GPU dumps
> perf counter values in memory and the CPU reads them back in
> kernel-space. This patch seems to solve the unpredictable behavior I
> had.
>
> I can also go for the other option (call dma_map/unmap/_sg() when
> needed) if you think that's more appropriate.
Uh, I guess shmem helpers (or gem helpers in general) need some concept
about what kind of cpu mapping is desired. Since some cpus (like e.g.
i915) do actually want cached mode for everything.
Same is needed for the userspace mmap, those should all agree.
Default probably best if we go with uncached.
-Daniel
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 1ee208c2c85e..472ea5d81f82 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -255,7 +255,8 @@ static void *drm_gem_shmem_vmap_locked(struct
> drm_gem_shmem_object *shmem)
> if (obj->import_attach)
> shmem->vaddr = dma_buf_vmap(obj->import_attach->dmabuf);
> else
> - shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT,
> VM_MAP, PAGE_KERNEL);
> + shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT,
> + VM_MAP, pgprot_writecombine(PAGE_KERNEL));
>
> if (!shmem->vaddr) {
> DRM_DEBUG_KMS("Failed to vmap pages\n");
> --
> 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: [RESEND PATCH v2 2/2] drm/panfrost: Expose perf counters through debugfs
On Tue, 28 May 2019 16:01:15 +0200
Boris Brezillon wrote:
> On Tue, 28 May 2019 15:53:48 +0200
> Boris Brezillon wrote:
>
> > Hi Steve,
> >
> > On Thu, 16 May 2019 17:21:39 +0100
> > Steven Price wrote:
> >
> >
> > > >> +static void panfrost_perfcnt_setup(struct panfrost_device *pfdev)
> > > >> +{
> > > >> + u32 cfg;
> > > >> +
> > > >> + /*
> > > >> +* Always use address space 0 for now.
> > > >> +* FIXME: this needs to be updated when we start using
> > > >> different
> > > >> +* address space.
> > > >> +*/
> > > >> + cfg = GPU_PERFCNT_CFG_AS(0);
> > > >> + if (panfrost_model_cmp(pfdev, 0x1000) >= 0)
> > > >
> > > > You've got a couple of these. Perhaps we should add either a
> > > > model_is_bifrost() helper or an is_bifrost variable to use.
> > > >
> > > >> + cfg |= GPU_PERFCNT_CFG_SETSEL(1);
> > >
> > > mali_kbase only sets this if CONFIG_MALI_PRFCNT_SET_SECONDARY is defined
> > > - i.e. this selects a different selection of counters. At at the very
> > > least this should be an optional flag that can be set, but I suspect the
> > > primary set are the ones you are interested in.
> >
> > I'll add a param to pass the set of counters to monitor.
> >
> > >
> > > >> +
> > > >> + gpu_write(pfdev, GPU_PERFCNT_CFG,
> > > >> + cfg |
> > > >> GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_OFF));
> > > >> +
> > > >> + if (!pfdev->perfcnt->enabled)
> > > >> + return;
> > > >> +
> > > >> + gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0x);
> > > >> + gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0x);
> > > >> + gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0x);
> > > >> +
> > > >> + /*
> > > >> +* Due to PRLAM-8186 we need to disable the Tiler before we
> > > >> enable HW
> > > >> +* counters.
> > > >
> > > > Seems like you could just always apply the work-around? It doesn't
> > > > look like it would be much different.
> > >
> > > Technically the workaround causes the driver to perform the operation in
> > > the wrong order below (write TILER_EN when the mode is MANUAL) - this is
> > > a workaround for the hardware issue (and therefore works on that
> > > hardware). But I wouldn't like to say it works on other hardware which
> > > doesn't have the issue.
> >
> > Okay, I'll keep the workaround only for HW that are impacted by the bug.
> >
> > >
> > > >> +*/
> > > >> + if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> > > >> + gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
> > > >> + else
> > > >> + gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0x);
> > > >> +
> > > >> + gpu_write(pfdev, GPU_PERFCNT_CFG,
> > > >> + cfg |
> > > >> GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_MANUAL));
> > > >> +
> > > >> + if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> > > >> + gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0x);
> > > >> +}
> >
> > [...]
> >
> > > >> +int panfrost_perfcnt_init(struct panfrost_device *pfdev)
> > > >> +{
> > > >> + struct panfrost_perfcnt *perfcnt;
> > > >> + struct drm_gem_shmem_object *bo;
> > > >> + size_t size;
> > > >> + u32 status;
> > > >> + int ret;
> > > >> +
> > > >> + if (panfrost_has_hw_feature(pfdev, HW_FEATURE_V4)) {
> > > >> + unsigned int ncoregroups;
> > > >> +
> > > >> + ncoregroups = hweight64(pfdev->features.l2_present);
> > > >> + size = ncoregroups * BLOCKS_PER_COREGROUP *
> > > >> + COUNTERS_PER_BLOCK * BYTES_PER_COUNTER;
> > > >> + } else {
> > > >> + unsigned int nl2c, ncores;
> > > >> +
> > > >> + /*
> > > >> +* TODO: define a macro to extract the number of l2
> > > >> caches from
> > > >> +* mem_features.
> > > >> +*/
> > > >> + nl2c = ((pfdev->features.mem_features >> 8) &
> > > >> GENMASK(3, 0)) + 1;
> > > >> +
> > > >> + /*
> > > >> +* The ARM driver is grouping cores per core group and
> > > >> then
> > > >> +* only using the number of cores in group 0 to
> > > >> calculate the
> > > >> +* size. Not sure why this is done like that, but I
> > > >> guess
> > > >> +* shader_present will only show cores in the first
> > > >> group
> > > >> +* anyway.
> > > >> +*/
> > >
> > > I'm not sure I understand this comment. I'm not sure which version of
> > > the driver you are looking at, but shader_present should contain all the
> > > cores (in every core group).
> > >
> > > The kbase driver (in panfrost's git tree[1]), contains:
> > >
> > > base_gpu_props *props = &kbdev->gpu_props.props;
> > > u32 nr_l2 = props->l2_props.num_l2_slices;
> > > u64 core_
[PATCH] drm/gem_shmem: Use a writecombine mapping for ->vaddr
Right now, the BO is mapped as a cached region when ->vmap() is called
and the underlying object is not a dmabuf.
Doing that makes cache management a bit more complicated (you'd need
to call dma_map/unmap_sg() on the ->sgt field everytime the BO is about
to be passed to the GPU/CPU), so let's map the BO with writecombine
attributes instead (as done in most drivers).
Signed-off-by: Boris Brezillon
---
Found this issue while working on panfrost perfcnt where the GPU dumps
perf counter values in memory and the CPU reads them back in
kernel-space. This patch seems to solve the unpredictable behavior I
had.
I can also go for the other option (call dma_map/unmap/_sg() when
needed) if you think that's more appropriate.
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 1ee208c2c85e..472ea5d81f82 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -255,7 +255,8 @@ static void *drm_gem_shmem_vmap_locked(struct
drm_gem_shmem_object *shmem)
if (obj->import_attach)
shmem->vaddr = dma_buf_vmap(obj->import_attach->dmabuf);
else
- shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT,
VM_MAP, PAGE_KERNEL);
+ shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT,
+ VM_MAP, pgprot_writecombine(PAGE_KERNEL));
if (!shmem->vaddr) {
DRM_DEBUG_KMS("Failed to vmap pages\n");
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v2 0/6] ARM: qcom: initial Nexus 5 display support
On Wed, May 29, 2019 at 3:17 AM Brian Masney wrote: > It's in low speed mode but its usable. How low speed is that? > I assume that it's related to the > vblank events not working properly? They are only waiting for 50 ms before timing out, I raised it to 100ms in the -next kernel. I'm still suspicious about this even though I think you said this was not the problem. For a command mode panel in LP mode it will nevertheless be more than 100ms for sure, the update is visible to the naked eye. Raise it to 1 ms or something and see what happens. drivers/gpu/drm/drm_atomic_helper.c: msecs_to_jiffies(50) Yours, Linus Walleij
Re: [PATCH 2/3] drm: mediatek: remove clk_unprepare() in mtk_drm_crtc_destroy()
On Wed, May 29, 2019 at 1:58 PM CK Hu wrote: > > Hi, Hsin-Yi: > > On Mon, 2019-05-27 at 12:50 +0800, Hsin-Yi Wang wrote: > > There is no clk_prepare() called in mtk_drm_crtc_reset(), when unbinding > > drm device, mtk_drm_crtc_destroy() will be triggered, and the clocks will > > be disabled and unprepared in mtk_crtc_ddp_clk_disable. If clk_unprepare() > > is called here, we'll get warnings[1], so remove clk_unprepare() here. > > In original code, clk_prepare() is called in mtk_drm_crtc_create() and > clk_unprepare() is called in mtk_drm_crtc_destroy(). This looks correct. clk_prepare() is removed in https://patchwork.kernel.org/patch/10872777/. > I don't know why we should do any thing about clock in > mtk_drm_crtc_reset(). To debug this, the first step is to print message > when mediatek drm call clk_prepare() and clk_unprepare(). If these two > interface is called in pair, I think we should not modify mediatek drm > driver, the bug maybe in clock driver. >
Re: [PATCH 2/3] drm: mediatek: remove clk_unprepare() in mtk_drm_crtc_destroy()
Hi, Hsin-Yi:
On Mon, 2019-05-27 at 12:50 +0800, Hsin-Yi Wang wrote:
> There is no clk_prepare() called in mtk_drm_crtc_reset(), when unbinding
> drm device, mtk_drm_crtc_destroy() will be triggered, and the clocks will
> be disabled and unprepared in mtk_crtc_ddp_clk_disable. If clk_unprepare()
> is called here, we'll get warnings[1], so remove clk_unprepare() here.
In original code, clk_prepare() is called in mtk_drm_crtc_create() and
clk_unprepare() is called in mtk_drm_crtc_destroy(). This looks correct.
I don't know why we should do any thing about clock in
mtk_drm_crtc_reset(). To debug this, the first step is to print message
when mediatek drm call clk_prepare() and clk_unprepare(). If these two
interface is called in pair, I think we should not modify mediatek drm
driver, the bug maybe in clock driver.
Regards,
CK
>
> [1]
> [ 19.416020] mm_disp_ovl0 already unprepared
>
> [ 19.487536] pstate: 6005 (nZCv daif -PAN -UAO)
> [ 19.492325] pc : clk_core_unprepare+0x1d8/0x220
> [ 19.496851] lr : clk_core_unprepare+0x1d8/0x220
> [ 19.501373] sp : ff8017bbba30
> [ 19.504681] x29: ff8017bbba50 x28: fff3f7978000
> [ 19.509989] x27: x26:
> [ 19.515298] x25: 4400 x24: fff3f7978000
> [ 19.520605] x23: 0060 x22: ff9688a89f48
> [ 19.525912] x21: fff3f8755540 x20:
> [ 19.531219] x19: fff3f9d5ca00 x18: fffebd18
> [ 19.536526] x17: 003c x16: ff96881458e4
> [ 19.541833] x15: 0005 x14: 706572706e752079
> [ 19.547140] x13: ff80085cc950 x12:
> [ 19.552446] x11: x10:
> [ 19.557754] x9 : 1b0fa21f0ec0d800 x8 : 1b0fa21f0ec0d800
> [ 19.563060] x7 : x6 : ff9688b5dd07
> [ 19.568366] x5 : x4 :
> [ 19.573673] x3 : x2 : fff3fffa0248
> [ 19.578979] x1 : fff3fff97a00 x0 : 001f
> [ 19.584288] Call trace:
> [ 19.586734] clk_core_unprepare+0x1d8/0x220
> [ 19.590914] clk_unprepare+0x30/0x40
> [ 19.594491] mtk_drm_crtc_destroy+0x30/0x5c
> [ 19.598672] drm_mode_config_cleanup+0x124/0x290
> [ 19.603286] mtk_drm_unbind+0x44/0x5c
> [ 19.606946] take_down_master+0x40/0x54
> [ 19.610775] component_master_del+0x70/0x94
> [ 19.614952] mtk_drm_remove+0x28/0x44
> [ 19.618612] platform_drv_remove+0x28/0x50
> [ 19.622702] device_release_driver_internal+0x138/0x1ec
> [ 19.627921] device_release_driver+0x24/0x30
> [ 19.632185] unbind_store+0x90/0xdc
> [ 19.635667] drv_attr_store+0x3c/0x54
> [ 19.639327] sysfs_kf_write+0x50/0x68
> [ 19.642986] kernfs_fop_write+0x12c/0x1c8
> [ 19.646997] __vfs_write+0x54/0x15c
> [ 19.650482] vfs_write+0xcc/0x188
> [ 19.653792] ksys_write+0x78/0xd8
> [ 19.657104] __arm64_sys_write+0x20/0x2c
> [ 19.661027] el0_svc_common+0x9c/0xfc
> [ 19.664686] el0_svc_compat_handler+0x2c/0x38
> [ 19.669039] el0_svc_compat+0x8/0x18
> [ 19.672609] ---[ end trace 41ce954855cda6f0 ]---
>
> Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
> Signed-off-by: Hsin-Yi Wang
> ---
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index acad088173da..c2b38997ac8b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -98,10 +98,6 @@ static void mtk_drm_finish_page_flip(struct mtk_drm_crtc
> *mtk_crtc)
> static void mtk_drm_crtc_destroy(struct drm_crtc *crtc)
> {
> struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> - int i;
> -
> - for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
> - clk_unprepare(mtk_crtc->ddp_comp[i]->clk);
>
> mtk_disp_mutex_put(mtk_crtc->mutex);
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110658] MXGP3 (Steam, native Linux port, UE4): graphical glitches
https://bugs.freedesktop.org/show_bug.cgi?id=110658 --- Comment #4 from Timothy Arceri --- I've run it on llvm 8 and mesa 19.0.5 and was unable to reproduce the issues seen in the screen capture on my Vega 64. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v3 1/7] drm/mediatek: move mipi_dsi_host_register to probe
On Sun, May 19, 2019 at 9:25 AM Jitao Shi wrote:
> @@ -1045,12 +1045,6 @@ static int mtk_dsi_bind(struct device *dev, struct
> device *master, void *data)
> return ret;
> }
>
> - ret = mipi_dsi_host_register(&dsi->host);
> - if (ret < 0) {
> - dev_err(dev, "failed to register DSI host: %d\n", ret);
> - goto err_ddp_comp_unregister;
> - }
> -
> ret = mtk_dsi_create_conn_enc(drm, dsi);
> if (ret) {
> DRM_ERROR("Encoder create failed with %d\n", ret);
> @@ -1060,8 +1054,6 @@ static int mtk_dsi_bind(struct device *dev, struct
> device *master, void *data)
> return 0;
>
> err_unregister:
> - mipi_dsi_host_unregister(&dsi->host);
> -err_ddp_comp_unregister:
> mtk_ddp_comp_unregister(drm, &dsi->ddp_comp);
> return ret;
> }
> @@ -1097,31 +1089,37 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>
> dsi->host.ops = &mtk_dsi_ops;
> dsi->host.dev = dev;
> + dsi->dev = dev;
> + ret = mipi_dsi_host_register(&dsi->host);
> + if (ret < 0) {
> + dev_err(dev, "failed to register DSI host: %d\n", ret);
> + return ret;
> + }
>
Since mipi_dsi_host_register() is moved from .bind to .probe,
mipi_dsi_host_unregister() should also be moved from .unbind to
.remove?
Thanks
Re: [PATCH 1/3] drm: mediatek: fix unbind functions
On Wed, May 29, 2019 at 9:35 AM CK Hu wrote:
>
> Hi, Hsin-yi:
>
> On Mon, 2019-05-27 at 12:50 +0800, Hsin-Yi Wang wrote:
> > move mipi_dsi_host_unregister() to .remove since mipi_dsi_host_register()
> > is called in .probe.
>
> In the latest kernel [1], mipi_dsi_host_register() is called in
> mtk_dsi_bind(), I think we don't need this part.
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/mediatek/mtk_dsi.c?h=v5.2-rc2
This patch https://patchwork.kernel.org/patch/10949305/ moves
mipi_dsi_host_register() from .bind to .probe. I'll reply there to ask
the owner to do this.
>
> >
> > detatch panel in mtk_dsi_destroy_conn_enc(), since .bind will try to
> > attach it again.
> >
> > Fixes: 2e54c14e310f ("drm/mediatek: Add DSI sub driver")
> > Signed-off-by: Hsin-Yi Wang
> > ---
> > drivers/gpu/drm/mediatek/mtk_dsi.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index b00eb2d2e086..c9b6d3a68c8b 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -844,6 +844,8 @@ static void mtk_dsi_destroy_conn_enc(struct mtk_dsi
> > *dsi)
> > /* Skip connector cleanup if creation was delegated to the bridge */
> > if (dsi->conn.dev)
> > drm_connector_cleanup(&dsi->conn);
> > + if (dsi->panel)
> > + drm_panel_detach(dsi->panel);
>
> I think mtk_dsi_destroy_conn_enc() has much thing to do and I would like
> you to do more. You could refer to [2] for complete implementation.
>
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/exynos/exynos_drm_dsi.c?h=v5.2-rc2#n1575
Will update in next version.
Thanks
Re: [Freedreno] [PATCH RFC v2 0/6] ARM: qcom: initial Nexus 5 display support
On Tue, May 28, 2019 at 07:42:19PM -0600, Jeffrey Hugo wrote: > > > Do you know if the nexus 5 has a video or command mode panel? There > > > is some glitchyness with vblanks and command mode panels. > > > > Its in command mode. I know this because I see two 'pp done time out' > > messages, even on 4.17. Based on my understanding, the ping pong code is > > only applicable for command mode panels. > > Actually, the ping pong element exists in both modes, but 'pp done > time out' is a good indicator that it is command mode. > > Are you also seeing vblank timeouts? Yes, here's a snippet of the first one. [2.556014] WARNING: CPU: 0 PID: 5 at drivers/gpu/drm/drm_atomic_helper.c:1429 drm_atomic_helper_wait_for_vblanks.part.1+0x288/0x290 [2.556020] [CRTC:49:crtc-0] vblank wait timed out [2.556023] Modules linked in: [2.556034] CPU: 0 PID: 5 Comm: kworker/0:0 Not tainted 5.2.0-rc1-00178-g72c3c1fd5f86-dirty #426 [2.556038] Hardware name: Generic DT based system [2.556056] Workqueue: events deferred_probe_work_func ... > Do you have busybox? > > Can you run - > sudo busybox devmem 0xFD900614 > sudo busybox devmem 0xFD900714 > sudo busybox devmem 0xFD900814 > sudo busybox devmem 0xFD900914 > sudo busybox devmem 0xFD900A14 # busybox devmem 0xFD900614 0x00020020 # busybox devmem 0xFD900714 0x # busybox devmem 0xFD900814 0x # busybox devmem 0xFD900914 0x # busybox devmem 0xFD900A14 0x I get the same values with the mainline kernel and 4.17. Brian
Re: [Freedreno] [PATCH RFC v2 0/6] ARM: qcom: initial Nexus 5 display support
On Tue, May 28, 2019 at 8:15 PM Rob Clark wrote:
>
> On Tue, May 28, 2019 at 6:17 PM Brian Masney wrote:
> >
> > On Tue, May 28, 2019 at 03:46:14PM +0200, Linus Walleij wrote:
> > > On Thu, May 9, 2019 at 4:04 AM Brian Masney wrote:
> > >
> > > > Here is a patch series that adds initial display support for the LG
> > > > Nexus 5 (hammerhead) phone. It's not fully working so that's why some
> > > > of these patches are RFC until we can get it fully working.
> > > >
> > > > The phones boots into terminal mode, however there is a several second
> > > > (or more) delay when writing to tty1 compared to when the changes are
> > > > actually shown on the screen. The following errors are in dmesg:
> > >
> > > I tested to apply patches 2-6 and got the console up on the phone as well.
> > > I see the same timouts, and I also notice the update is slow in the
> > > display, as if the DSI panel was running in low power (LP) mode.
> > >
> > > Was booting this to do some other work, but happy to see the progress!
> >
> > Thanks!
> >
> > I've had three people email me off list regarding the display working on
> > 4.17 before the msm kms/drm driver was converted to the DRM atomic API so
> > this email is to get some more information out publicly.
> >
> > I pushed up a branch to my github with 15 patches applied against 4.17
> > that has a working display:
> >
> > https://github.com/masneyb/linux/commits/display-works-4.17
> >
> > It's in low speed mode but its usable. The first 10 patches are in
> > mainline now and the last 5 are in essence this patch series with the
> > exception of 'drm/atomic+msm: add helper to implement legacy dirtyfb'.
> > There's a slightly different version of that patch in mainline now.
> >
> > I'm planning to work on the msm8974 interconnect support once some of
> > the outstanding interconnect patches for the msm kms/drm driver arrive
> > in mainline. I'd really like to understand why the display works on
> > 4.17 with those patches though. I assume that it's related to the
> > vblank events not working properly? Let me preface this with I'm a
> > total DRM newbie, but it looked like the pre-DRM-atomic driver wasn't
> > looking for these events in the atomic commits before the migration?
> > See commit 70db18dca4e0 ("drm/msm: Remove msm_commit/worker, use atomic
> > helper commit"), specifically the drm_atomic_helper_wait_for_vblanks()
> > call that was added.
>
> interconnect probably good to get going anyways (and I need to find
> some time to respin those mdp5/dpu patches) but I guess not related to
> what you see (ie. I'd expect interconnect issue would trigger
> underflow irq's)..
>
> I'm not entirely sure why atomic would break things but cmd mode
> panels aren't especially well tested. I can't find it now but there
> was a thread (or IRC discussion?) that intf2vblank() should be
> returning MDP5_IRQ_PING_PONG__DONE instead of
> MDP5_IRQ_PING_PONG__RD_PTR, which seems likely and possibly related
> to vblank timing issues..
That was an irc discussion, and I've changed my mind on that.
My big issue ended up being that autorefresh was enabled (which
basically turns the command panel into a pseudo video mode panel),
which appears incompatible with using the start kick. If FW is
configuring things in autorefresh mode, the driver likely doesn't
know, and will make a mess of things by using the start kick.
Disabling autorefresh would make the driver work as is, but fbcon
wouldn't work well (you'd need to do a start kick per frame, which
doesn't seem to happen). Removing the start kick and using the
autorefresh feature seemed better in my testing, but I haven't cleaned
up my code tree to send something up.
However, lets see how the hardware is actually configured in Brian's case.
Re: [PATCH RFC v2 0/6] ARM: qcom: initial Nexus 5 display support
On Tue, May 28, 2019 at 6:17 PM Brian Masney wrote:
>
> On Tue, May 28, 2019 at 03:46:14PM +0200, Linus Walleij wrote:
> > On Thu, May 9, 2019 at 4:04 AM Brian Masney wrote:
> >
> > > Here is a patch series that adds initial display support for the LG
> > > Nexus 5 (hammerhead) phone. It's not fully working so that's why some
> > > of these patches are RFC until we can get it fully working.
> > >
> > > The phones boots into terminal mode, however there is a several second
> > > (or more) delay when writing to tty1 compared to when the changes are
> > > actually shown on the screen. The following errors are in dmesg:
> >
> > I tested to apply patches 2-6 and got the console up on the phone as well.
> > I see the same timouts, and I also notice the update is slow in the
> > display, as if the DSI panel was running in low power (LP) mode.
> >
> > Was booting this to do some other work, but happy to see the progress!
>
> Thanks!
>
> I've had three people email me off list regarding the display working on
> 4.17 before the msm kms/drm driver was converted to the DRM atomic API so
> this email is to get some more information out publicly.
>
> I pushed up a branch to my github with 15 patches applied against 4.17
> that has a working display:
>
> https://github.com/masneyb/linux/commits/display-works-4.17
>
> It's in low speed mode but its usable. The first 10 patches are in
> mainline now and the last 5 are in essence this patch series with the
> exception of 'drm/atomic+msm: add helper to implement legacy dirtyfb'.
> There's a slightly different version of that patch in mainline now.
>
> I'm planning to work on the msm8974 interconnect support once some of
> the outstanding interconnect patches for the msm kms/drm driver arrive
> in mainline. I'd really like to understand why the display works on
> 4.17 with those patches though. I assume that it's related to the
> vblank events not working properly? Let me preface this with I'm a
> total DRM newbie, but it looked like the pre-DRM-atomic driver wasn't
> looking for these events in the atomic commits before the migration?
> See commit 70db18dca4e0 ("drm/msm: Remove msm_commit/worker, use atomic
> helper commit"), specifically the drm_atomic_helper_wait_for_vblanks()
> call that was added.
interconnect probably good to get going anyways (and I need to find
some time to respin those mdp5/dpu patches) but I guess not related to
what you see (ie. I'd expect interconnect issue would trigger
underflow irq's)..
I'm not entirely sure why atomic would break things but cmd mode
panels aren't especially well tested. I can't find it now but there
was a thread (or IRC discussion?) that intf2vblank() should be
returning MDP5_IRQ_PING_PONG__DONE instead of
MDP5_IRQ_PING_PONG__RD_PTR, which seems likely and possibly related
to vblank timing issues..
BR,
-R
>
> Brian
[PATCH] drm/gma500: fix edid memory leak in SDVO
The variable edid returned by psb_intel_sdvo_get_edid() was never freed. Signed-off-by: Young Xiao <92siuy...@gmail.com> --- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c index dd3cec0..cc5fb85 100644 --- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c +++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c @@ -1656,6 +1656,7 @@ static bool psb_intel_sdvo_detect_hdmi_audio(struct drm_connector *connector) edid = psb_intel_sdvo_get_edid(connector); if (edid != NULL && edid->input & DRM_EDID_INPUT_DIGITAL) has_audio = drm_detect_monitor_audio(edid); + kfree(edid); return has_audio; } -- 2.7.4
Re: [Freedreno] [PATCH RFC v2 0/6] ARM: qcom: initial Nexus 5 display support
On Tue, May 28, 2019 at 7:37 PM Brian Masney wrote:
>
> On Tue, May 28, 2019 at 07:32:14PM -0600, Jeffrey Hugo wrote:
> > On Tue, May 28, 2019 at 7:17 PM Brian Masney wrote:
> > >
> > > On Tue, May 28, 2019 at 03:46:14PM +0200, Linus Walleij wrote:
> > > > On Thu, May 9, 2019 at 4:04 AM Brian Masney
> > > > wrote:
> > > >
> > > > > Here is a patch series that adds initial display support for the LG
> > > > > Nexus 5 (hammerhead) phone. It's not fully working so that's why some
> > > > > of these patches are RFC until we can get it fully working.
> > > > >
> > > > > The phones boots into terminal mode, however there is a several second
> > > > > (or more) delay when writing to tty1 compared to when the changes are
> > > > > actually shown on the screen. The following errors are in dmesg:
> > > >
> > > > I tested to apply patches 2-6 and got the console up on the phone as
> > > > well.
> > > > I see the same timouts, and I also notice the update is slow in the
> > > > display, as if the DSI panel was running in low power (LP) mode.
> > > >
> > > > Was booting this to do some other work, but happy to see the progress!
> > >
> > > Thanks!
> > >
> > > I've had three people email me off list regarding the display working on
> > > 4.17 before the msm kms/drm driver was converted to the DRM atomic API so
> > > this email is to get some more information out publicly.
> > >
> > > I pushed up a branch to my github with 15 patches applied against 4.17
> > > that has a working display:
> > >
> > > https://github.com/masneyb/linux/commits/display-works-4.17
> > >
> > > It's in low speed mode but its usable. The first 10 patches are in
> > > mainline now and the last 5 are in essence this patch series with the
> > > exception of 'drm/atomic+msm: add helper to implement legacy dirtyfb'.
> > > There's a slightly different version of that patch in mainline now.
> > >
> > > I'm planning to work on the msm8974 interconnect support once some of
> > > the outstanding interconnect patches for the msm kms/drm driver arrive
> > > in mainline. I'd really like to understand why the display works on
> > > 4.17 with those patches though. I assume that it's related to the
> > > vblank events not working properly? Let me preface this with I'm a
> > > total DRM newbie, but it looked like the pre-DRM-atomic driver wasn't
> > > looking for these events in the atomic commits before the migration?
> > > See commit 70db18dca4e0 ("drm/msm: Remove msm_commit/worker, use atomic
> > > helper commit"), specifically the drm_atomic_helper_wait_for_vblanks()
> > > call that was added.
> >
> > Do you know if the nexus 5 has a video or command mode panel? There
> > is some glitchyness with vblanks and command mode panels.
>
> Its in command mode. I know this because I see two 'pp done time out'
> messages, even on 4.17. Based on my understanding, the ping pong code is
> only applicable for command mode panels.
Actually, the ping pong element exists in both modes, but 'pp done
time out' is a good indicator that it is command mode.
Are you also seeing vblank timeouts?
Do you have busybox?
Can you run -
sudo busybox devmem 0xFD900614
sudo busybox devmem 0xFD900714
sudo busybox devmem 0xFD900814
sudo busybox devmem 0xFD900914
sudo busybox devmem 0xFD900A14
Re: [Freedreno] [PATCH RFC v2 0/6] ARM: qcom: initial Nexus 5 display support
On Tue, May 28, 2019 at 07:32:14PM -0600, Jeffrey Hugo wrote:
> On Tue, May 28, 2019 at 7:17 PM Brian Masney wrote:
> >
> > On Tue, May 28, 2019 at 03:46:14PM +0200, Linus Walleij wrote:
> > > On Thu, May 9, 2019 at 4:04 AM Brian Masney wrote:
> > >
> > > > Here is a patch series that adds initial display support for the LG
> > > > Nexus 5 (hammerhead) phone. It's not fully working so that's why some
> > > > of these patches are RFC until we can get it fully working.
> > > >
> > > > The phones boots into terminal mode, however there is a several second
> > > > (or more) delay when writing to tty1 compared to when the changes are
> > > > actually shown on the screen. The following errors are in dmesg:
> > >
> > > I tested to apply patches 2-6 and got the console up on the phone as well.
> > > I see the same timouts, and I also notice the update is slow in the
> > > display, as if the DSI panel was running in low power (LP) mode.
> > >
> > > Was booting this to do some other work, but happy to see the progress!
> >
> > Thanks!
> >
> > I've had three people email me off list regarding the display working on
> > 4.17 before the msm kms/drm driver was converted to the DRM atomic API so
> > this email is to get some more information out publicly.
> >
> > I pushed up a branch to my github with 15 patches applied against 4.17
> > that has a working display:
> >
> > https://github.com/masneyb/linux/commits/display-works-4.17
> >
> > It's in low speed mode but its usable. The first 10 patches are in
> > mainline now and the last 5 are in essence this patch series with the
> > exception of 'drm/atomic+msm: add helper to implement legacy dirtyfb'.
> > There's a slightly different version of that patch in mainline now.
> >
> > I'm planning to work on the msm8974 interconnect support once some of
> > the outstanding interconnect patches for the msm kms/drm driver arrive
> > in mainline. I'd really like to understand why the display works on
> > 4.17 with those patches though. I assume that it's related to the
> > vblank events not working properly? Let me preface this with I'm a
> > total DRM newbie, but it looked like the pre-DRM-atomic driver wasn't
> > looking for these events in the atomic commits before the migration?
> > See commit 70db18dca4e0 ("drm/msm: Remove msm_commit/worker, use atomic
> > helper commit"), specifically the drm_atomic_helper_wait_for_vblanks()
> > call that was added.
>
> Do you know if the nexus 5 has a video or command mode panel? There
> is some glitchyness with vblanks and command mode panels.
Its in command mode. I know this because I see two 'pp done time out'
messages, even on 4.17. Based on my understanding, the ping pong code is
only applicable for command mode panels.
Brian
Re: [PATCH 1/3] drm: mediatek: fix unbind functions
Hi, Hsin-yi:
On Mon, 2019-05-27 at 12:50 +0800, Hsin-Yi Wang wrote:
> move mipi_dsi_host_unregister() to .remove since mipi_dsi_host_register()
> is called in .probe.
In the latest kernel [1], mipi_dsi_host_register() is called in
mtk_dsi_bind(), I think we don't need this part.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/mediatek/mtk_dsi.c?h=v5.2-rc2
>
> detatch panel in mtk_dsi_destroy_conn_enc(), since .bind will try to
> attach it again.
>
> Fixes: 2e54c14e310f ("drm/mediatek: Add DSI sub driver")
> Signed-off-by: Hsin-Yi Wang
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index b00eb2d2e086..c9b6d3a68c8b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -844,6 +844,8 @@ static void mtk_dsi_destroy_conn_enc(struct mtk_dsi *dsi)
> /* Skip connector cleanup if creation was delegated to the bridge */
> if (dsi->conn.dev)
> drm_connector_cleanup(&dsi->conn);
> + if (dsi->panel)
> + drm_panel_detach(dsi->panel);
I think mtk_dsi_destroy_conn_enc() has much thing to do and I would like
you to do more. You could refer to [2] for complete implementation.
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/exynos/exynos_drm_dsi.c?h=v5.2-rc2#n1575
Regards,
CK
> }
>
> static void mtk_dsi_ddp_start(struct mtk_ddp_comp *comp)
> @@ -1073,7 +1075,6 @@ static void mtk_dsi_unbind(struct device *dev, struct
> device *master,
> struct mtk_dsi *dsi = dev_get_drvdata(dev);
>
> mtk_dsi_destroy_conn_enc(dsi);
> - mipi_dsi_host_unregister(&dsi->host);
> mtk_ddp_comp_unregister(drm, &dsi->ddp_comp);
> }
>
> @@ -1179,6 +1180,7 @@ static int mtk_dsi_remove(struct platform_device *pdev)
>
> mtk_output_dsi_disable(dsi);
> component_del(&pdev->dev, &mtk_dsi_component_ops);
> + mipi_dsi_host_unregister(&dsi->host);
>
> return 0;
> }
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH RFC v2 0/6] ARM: qcom: initial Nexus 5 display support
On Tue, May 28, 2019 at 7:17 PM Brian Masney wrote:
>
> On Tue, May 28, 2019 at 03:46:14PM +0200, Linus Walleij wrote:
> > On Thu, May 9, 2019 at 4:04 AM Brian Masney wrote:
> >
> > > Here is a patch series that adds initial display support for the LG
> > > Nexus 5 (hammerhead) phone. It's not fully working so that's why some
> > > of these patches are RFC until we can get it fully working.
> > >
> > > The phones boots into terminal mode, however there is a several second
> > > (or more) delay when writing to tty1 compared to when the changes are
> > > actually shown on the screen. The following errors are in dmesg:
> >
> > I tested to apply patches 2-6 and got the console up on the phone as well.
> > I see the same timouts, and I also notice the update is slow in the
> > display, as if the DSI panel was running in low power (LP) mode.
> >
> > Was booting this to do some other work, but happy to see the progress!
>
> Thanks!
>
> I've had three people email me off list regarding the display working on
> 4.17 before the msm kms/drm driver was converted to the DRM atomic API so
> this email is to get some more information out publicly.
>
> I pushed up a branch to my github with 15 patches applied against 4.17
> that has a working display:
>
> https://github.com/masneyb/linux/commits/display-works-4.17
>
> It's in low speed mode but its usable. The first 10 patches are in
> mainline now and the last 5 are in essence this patch series with the
> exception of 'drm/atomic+msm: add helper to implement legacy dirtyfb'.
> There's a slightly different version of that patch in mainline now.
>
> I'm planning to work on the msm8974 interconnect support once some of
> the outstanding interconnect patches for the msm kms/drm driver arrive
> in mainline. I'd really like to understand why the display works on
> 4.17 with those patches though. I assume that it's related to the
> vblank events not working properly? Let me preface this with I'm a
> total DRM newbie, but it looked like the pre-DRM-atomic driver wasn't
> looking for these events in the atomic commits before the migration?
> See commit 70db18dca4e0 ("drm/msm: Remove msm_commit/worker, use atomic
> helper commit"), specifically the drm_atomic_helper_wait_for_vblanks()
> call that was added.
Do you know if the nexus 5 has a video or command mode panel? There
is some glitchyness with vblanks and command mode panels.
Re: [PATCH RFC v2 0/6] ARM: qcom: initial Nexus 5 display support
On Tue, May 28, 2019 at 03:46:14PM +0200, Linus Walleij wrote:
> On Thu, May 9, 2019 at 4:04 AM Brian Masney wrote:
>
> > Here is a patch series that adds initial display support for the LG
> > Nexus 5 (hammerhead) phone. It's not fully working so that's why some
> > of these patches are RFC until we can get it fully working.
> >
> > The phones boots into terminal mode, however there is a several second
> > (or more) delay when writing to tty1 compared to when the changes are
> > actually shown on the screen. The following errors are in dmesg:
>
> I tested to apply patches 2-6 and got the console up on the phone as well.
> I see the same timouts, and I also notice the update is slow in the
> display, as if the DSI panel was running in low power (LP) mode.
>
> Was booting this to do some other work, but happy to see the progress!
Thanks!
I've had three people email me off list regarding the display working on
4.17 before the msm kms/drm driver was converted to the DRM atomic API so
this email is to get some more information out publicly.
I pushed up a branch to my github with 15 patches applied against 4.17
that has a working display:
https://github.com/masneyb/linux/commits/display-works-4.17
It's in low speed mode but its usable. The first 10 patches are in
mainline now and the last 5 are in essence this patch series with the
exception of 'drm/atomic+msm: add helper to implement legacy dirtyfb'.
There's a slightly different version of that patch in mainline now.
I'm planning to work on the msm8974 interconnect support once some of
the outstanding interconnect patches for the msm kms/drm driver arrive
in mainline. I'd really like to understand why the display works on
4.17 with those patches though. I assume that it's related to the
vblank events not working properly? Let me preface this with I'm a
total DRM newbie, but it looked like the pre-DRM-atomic driver wasn't
looking for these events in the atomic commits before the migration?
See commit 70db18dca4e0 ("drm/msm: Remove msm_commit/worker, use atomic
helper commit"), specifically the drm_atomic_helper_wait_for_vblanks()
call that was added.
Brian
[Bug 110787] Glitches in console of the Julia language plugin for Atom (Juno)
https://bugs.freedesktop.org/show_bug.cgi?id=110787 --- Comment #1 from Manuel Vögele --- Created attachment 144367 --> https://bugs.freedesktop.org/attachment.cgi?id=144367&action=edit Output of glxinfo -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110787] Glitches in console of the Julia language plugin for Atom (Juno)
https://bugs.freedesktop.org/show_bug.cgi?id=110787 Bug ID: 110787 Summary: Glitches in console of the Julia language plugin for Atom (Juno) Product: Mesa Version: 19.0 Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: deve...@manuel-voegele.de QA Contact: dri-devel@lists.freedesktop.org Created attachment 144366 --> https://bugs.freedesktop.org/attachment.cgi?id=144366&action=edit A screenshot showing the graphical artifacts in the console Since Mesa 19.0.5 the REPL-Console in the Julia language plugin is unusable and often shows artifacts. Mesa 19.0.4 seems not to be affectd by this. Steps to reproduce: 1. Install the Atom and the Julia language plugin (Juno) according to this gude: http://docs.junolab.org/latest/man/installation/ During the installation agree to open julia specific views by default. 2. Open up atom. A console windows should appear on the bottom. 3. Click around (switching between clicks in the console, in the editor and in windows that don't belong to atom). This usually causes graphical glitches to appear in the Julia console (REPL). If this doesn't happen try using the console - I am usually unable to use it since even if it doesn't get filled with artifacts it still won't render properly which makes it unusable. I've attached a screenshot of the broken console. I'm using Arch Linux with KDE Plasma as desktop environment. Unfortunately I'm unable to provide an apitrace since runnign atom with apitrace caused the bug to disappear. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm/adreno: Add A540 support
On Tue, May 28, 2019 at 2:45 PM Jordan Crouse wrote:
>
> On Tue, May 28, 2019 at 10:06:12AM -0700, Jeffrey Hugo wrote:
> > The A540 is a derivative of the A530, and is found in the MSM8998 SoC.
> >
> > Signed-off-by: Jeffrey Hugo
> > ---
> > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 22 +++
> > drivers/gpu/drm/msm/adreno/a5xx_power.c| 76 +-
> > drivers/gpu/drm/msm/adreno/adreno_device.c | 18 +
> > drivers/gpu/drm/msm/adreno/adreno_gpu.h| 6 ++
> > 4 files changed, 119 insertions(+), 3 deletions(-)
>
> This is awesome. I'm glad we can finally check this off the list.
Thanks for the quick review.
>
> > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > index e5fcefa49f19..7ca8fde22fd8 100644
> > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > @@ -318,12 +318,19 @@ static const struct {
> >
> > void a5xx_set_hwcg(struct msm_gpu *gpu, bool state)
> > {
> > + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> > unsigned int i;
> >
> > for (i = 0; i < ARRAY_SIZE(a5xx_hwcg); i++)
> > gpu_write(gpu, a5xx_hwcg[i].offset,
> > state ? a5xx_hwcg[i].value : 0);
> >
> > + if (adreno_is_a540(adreno_gpu)) {
> > + gpu_write(gpu, REG_A5XX_RBBM_CLOCK_HYST_GPMU, 0x0222);
>
> I don't think this one is needed - we can just go with the one two lines
> below.
I'll try without it, and see.
>
> > + gpu_write(gpu, REG_A5XX_RBBM_CLOCK_DELAY_GPMU, 0x0770);
> > + gpu_write(gpu, REG_A5XX_RBBM_CLOCK_HYST_GPMU, 0x0004);
>
> Both of these need the state ? : 0x0 hack to turn off HWCG if
> requested
I see. Will do.
>
> > + }
> > +
> > gpu_write(gpu, REG_A5XX_RBBM_CLOCK_CNTL, state ? 0xAAA8AA00 : 0);
> > gpu_write(gpu, REG_A5XX_RBBM_ISDB_CNT, state ? 0x182 : 0x180);
> > }
> > @@ -507,6 +514,9 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
> >
> > gpu_write(gpu, REG_A5XX_VBIF_ROUND_ROBIN_QOS_ARB, 0x0003);
> >
> > + if (adreno_is_a540(adreno_gpu))
> > + gpu_write(gpu, REG_A5XX_VBIF_GATE_OFF_WRREQ_EN, 0x0009);
> > +
> > /* Make all blocks contribute to the GPU BUSY perf counter */
> > gpu_write(gpu, REG_A5XX_RBBM_PERFCTR_GPU_BUSY_MASKED, 0x);
> >
> > @@ -592,6 +602,8 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
> > /* Set the highest bank bit */
> > gpu_write(gpu, REG_A5XX_TPL1_MODE_CNTL, 2 << 7);
> > gpu_write(gpu, REG_A5XX_RB_MODE_CNTL, 2 << 1);
> > + if (adreno_is_a540(adreno_gpu))
> > + gpu_write(gpu, REG_A5XX_UCHE_DBG_ECO_CNTL_2, 2);
> >
> > /* Protect registers from the CP */
> > gpu_write(gpu, REG_A5XX_CP_PROTECT_CNTL, 0x0007);
> > @@ -642,6 +654,16 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
> > REG_A5XX_RBBM_SECVID_TSB_TRUSTED_BASE_HI, 0x);
> > gpu_write(gpu, REG_A5XX_RBBM_SECVID_TSB_TRUSTED_SIZE, 0x);
> >
> > + /*
> > + * VPC corner case with local memory load kill leads to corrupt
> > + * internal state. Normal Disable does not work for all a5x chips.
> > + * So do the following setting to disable it.
> > + */
> > + if (adreno_gpu->info->quirks & ADRENO_QUIRK_LMLOADKILL_DISABLE) {
>
> This is a5xx only quirk but it applies to multiple 5xx targets so I think its
> reasonable to identify it as a quirk and use it as such to make it easier on
> future code porters.
>
> > + gpu_rmw(gpu, REG_A5XX_VPC_DBG_ECO_CNTL, 0, BIT(23));
> > + gpu_rmw(gpu, 0xE04, BIT(18), 0);
> > /*REG_A5XX_HLSQ_DBG_ECO_CNTL*/
>
> You should define and use REG_A5XX_HLSQ_DBG_ECO_CNTL symbolically. We can help
> you do the envytools dance if you need.
Thanks for the offline explanation. Will do
>
> > + }
> > +
> > ret = adreno_hw_init(gpu);
> > if (ret)
> > return ret;
> > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_power.c
> > b/drivers/gpu/drm/msm/adreno/a5xx_power.c
> > index 70e65c94e525..5cb325c6eb8f 100644
> > --- a/drivers/gpu/drm/msm/adreno/a5xx_power.c
> > +++ b/drivers/gpu/drm/msm/adreno/a5xx_power.c
> > @@ -32,6 +32,18 @@
> > #define AGC_POWER_CONFIG_PRODUCTION_ID 1
> > #define AGC_INIT_MSG_VALUE 0xBABEFACE
> >
> > +/* AGC_LM_CONFIG (A540+) */
> > +#define AGC_LM_CONFIG (136/4)
> > +#define AGC_LM_CONFIG_GPU_VERSION_SHIFT 17
> > +#define AGC_LM_CONFIG_ENABLE_GPMU_ADAPTIVE 1
> > +#define AGC_LM_CONFIG_THROTTLE_DISABLE (2 << 8)
> > +#define AGC_LM_CONFIG_ISENSE_ENABLE (1 << 4)
> > +#define AGC_LM_CONFIG_ENABLE_ERROR (3 << 4)
> > +#define AGC_LM_CONFIG_LLM_ENABLED (1 << 16)
> > +#define AGC_LM_CONFIG_BCL_DISABLED (1 << 24)
> > +
> > +#define AGC_LEVEL_CONFIG (140/4)
> > +
> > static struct {
> > uint32_t reg;
> > uint32_t value;
> > @@ -116,7 +128,7 @@ static inline uint32_t _get_mvolts(struct msm_gpu *gpu,
> > uint32_t freq)
> > }
> >
> > /*
Re: [PATCH] drm/msm/adreno: Add A540 support
On Tue, May 28, 2019 at 10:06:12AM -0700, Jeffrey Hugo wrote:
> The A540 is a derivative of the A530, and is found in the MSM8998 SoC.
>
> Signed-off-by: Jeffrey Hugo
> ---
> drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 22 +++
> drivers/gpu/drm/msm/adreno/a5xx_power.c| 76 +-
> drivers/gpu/drm/msm/adreno/adreno_device.c | 18 +
> drivers/gpu/drm/msm/adreno/adreno_gpu.h| 6 ++
> 4 files changed, 119 insertions(+), 3 deletions(-)
This is awesome. I'm glad we can finally check this off the list.
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index e5fcefa49f19..7ca8fde22fd8 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -318,12 +318,19 @@ static const struct {
>
> void a5xx_set_hwcg(struct msm_gpu *gpu, bool state)
> {
> + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> unsigned int i;
>
> for (i = 0; i < ARRAY_SIZE(a5xx_hwcg); i++)
> gpu_write(gpu, a5xx_hwcg[i].offset,
> state ? a5xx_hwcg[i].value : 0);
>
> + if (adreno_is_a540(adreno_gpu)) {
> + gpu_write(gpu, REG_A5XX_RBBM_CLOCK_HYST_GPMU, 0x0222);
I don't think this one is needed - we can just go with the one two lines below.
> + gpu_write(gpu, REG_A5XX_RBBM_CLOCK_DELAY_GPMU, 0x0770);
> + gpu_write(gpu, REG_A5XX_RBBM_CLOCK_HYST_GPMU, 0x0004);
Both of these need the state ? : 0x0 hack to turn off HWCG if requested
> + }
> +
> gpu_write(gpu, REG_A5XX_RBBM_CLOCK_CNTL, state ? 0xAAA8AA00 : 0);
> gpu_write(gpu, REG_A5XX_RBBM_ISDB_CNT, state ? 0x182 : 0x180);
> }
> @@ -507,6 +514,9 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
>
> gpu_write(gpu, REG_A5XX_VBIF_ROUND_ROBIN_QOS_ARB, 0x0003);
>
> + if (adreno_is_a540(adreno_gpu))
> + gpu_write(gpu, REG_A5XX_VBIF_GATE_OFF_WRREQ_EN, 0x0009);
> +
> /* Make all blocks contribute to the GPU BUSY perf counter */
> gpu_write(gpu, REG_A5XX_RBBM_PERFCTR_GPU_BUSY_MASKED, 0x);
>
> @@ -592,6 +602,8 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
> /* Set the highest bank bit */
> gpu_write(gpu, REG_A5XX_TPL1_MODE_CNTL, 2 << 7);
> gpu_write(gpu, REG_A5XX_RB_MODE_CNTL, 2 << 1);
> + if (adreno_is_a540(adreno_gpu))
> + gpu_write(gpu, REG_A5XX_UCHE_DBG_ECO_CNTL_2, 2);
>
> /* Protect registers from the CP */
> gpu_write(gpu, REG_A5XX_CP_PROTECT_CNTL, 0x0007);
> @@ -642,6 +654,16 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
> REG_A5XX_RBBM_SECVID_TSB_TRUSTED_BASE_HI, 0x);
> gpu_write(gpu, REG_A5XX_RBBM_SECVID_TSB_TRUSTED_SIZE, 0x);
>
> + /*
> + * VPC corner case with local memory load kill leads to corrupt
> + * internal state. Normal Disable does not work for all a5x chips.
> + * So do the following setting to disable it.
> + */
> + if (adreno_gpu->info->quirks & ADRENO_QUIRK_LMLOADKILL_DISABLE) {
This is a5xx only quirk but it applies to multiple 5xx targets so I think its
reasonable to identify it as a quirk and use it as such to make it easier on
future code porters.
> + gpu_rmw(gpu, REG_A5XX_VPC_DBG_ECO_CNTL, 0, BIT(23));
> + gpu_rmw(gpu, 0xE04, BIT(18), 0); /*REG_A5XX_HLSQ_DBG_ECO_CNTL*/
You should define and use REG_A5XX_HLSQ_DBG_ECO_CNTL symbolically. We can help
you do the envytools dance if you need.
> + }
> +
> ret = adreno_hw_init(gpu);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_power.c
> b/drivers/gpu/drm/msm/adreno/a5xx_power.c
> index 70e65c94e525..5cb325c6eb8f 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_power.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_power.c
> @@ -32,6 +32,18 @@
> #define AGC_POWER_CONFIG_PRODUCTION_ID 1
> #define AGC_INIT_MSG_VALUE 0xBABEFACE
>
> +/* AGC_LM_CONFIG (A540+) */
> +#define AGC_LM_CONFIG (136/4)
> +#define AGC_LM_CONFIG_GPU_VERSION_SHIFT 17
> +#define AGC_LM_CONFIG_ENABLE_GPMU_ADAPTIVE 1
> +#define AGC_LM_CONFIG_THROTTLE_DISABLE (2 << 8)
> +#define AGC_LM_CONFIG_ISENSE_ENABLE (1 << 4)
> +#define AGC_LM_CONFIG_ENABLE_ERROR (3 << 4)
> +#define AGC_LM_CONFIG_LLM_ENABLED (1 << 16)
> +#define AGC_LM_CONFIG_BCL_DISABLED (1 << 24)
> +
> +#define AGC_LEVEL_CONFIG (140/4)
> +
> static struct {
> uint32_t reg;
> uint32_t value;
> @@ -116,7 +128,7 @@ static inline uint32_t _get_mvolts(struct msm_gpu *gpu,
> uint32_t freq)
> }
>
> /* Setup thermal limit management */
> -static void a5xx_lm_setup(struct msm_gpu *gpu)
> +static void a530_lm_setup(struct msm_gpu *gpu)
> {
> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> struct a5xx_gpu *a5xx_gpu = to_a5xx_gpu(adreno_gpu);
> @@ -165,6 +177,45 @@ static void a5xx_lm_setup(struct msm_gpu *gpu)
> gpu_write(gpu, AGC_INIT_MSG_MAGIC, AGC_INIT_MSG_VALUE);
> }
>
Re: [Nouveau] [PATCH 1/2] drm/nouveau/disp/nv50-: force scaler for any non-default LVDS/eDP modes
Sigh ... looks like this doesn't actually do what we want. See the
last couple comments in:
https://bugs.freedesktop.org/show_bug.cgi?id=110660
It seems to work as expected with "mode" instead of "adjusted_mode".
Does that make sense? It kinda does based on the later code, which
copies mode into adjusted_mode...
Assuming it makes sense to use "mode", Ben, want to just do a fixup
locally, or want me to send a revert + new patch?
-ilia
On Mon, May 27, 2019 at 2:24 AM Ben Skeggs wrote:
>
> On Sun, 26 May 2019 at 08:41, Ilia Mirkin wrote:
> >
> > Higher layers tend to add a lot of modes not actually in the EDID, such
> > as the standard DMT modes. Changing this would be extremely intrusive to
> > everyone, so just force the scaler more often. There are no practical
> > cases we're aware of where a LVDS/eDP panel has multiple resolutions
> > exposed, and i915 already does it this way.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110660
> > Signed-off-by: Ilia Mirkin
> Thanks Ilia, grabbed both patches.
>
> > ---
> >
> > Untested for now, hoping that the bugzilla filer will test it out. Seems
> > obvious though.
> >
> > drivers/gpu/drm/nouveau/dispnv50/disp.c | 9 +++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > index 134701a837c8..ef8d7a71564a 100644
> > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > @@ -322,8 +322,13 @@ nv50_outp_atomic_check_view(struct drm_encoder
> > *encoder,
> > switch (connector->connector_type) {
> > case DRM_MODE_CONNECTOR_LVDS:
> > case DRM_MODE_CONNECTOR_eDP:
> > - /* Force use of scaler for non-EDID modes. */
> > - if (adjusted_mode->type & DRM_MODE_TYPE_DRIVER)
> > + /* Don't force scaler for EDID modes with
> > +* same size as the native one (e.g. different
> > +* refresh rate)
> > +*/
> > + if (adjusted_mode->hdisplay ==
> > native_mode->hdisplay &&
> > + adjusted_mode->vdisplay ==
> > native_mode->vdisplay &&
> > + adjusted_mode->type & DRM_MODE_TYPE_DRIVER)
> > break;
> > mode = native_mode;
> > asyc->scaler.full = true;
> > --
> > 2.21.0
> >
> > ___
> > Nouveau mailing list
> > nouv...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm/dpu: Remove bogus comment
On Tue, May 28, 2019 at 02:26:45PM -0400, Sean Paul wrote: > From: Sean Paul > > This comment doesn't make any sense, remove it. > > Suggested-by: Jordan Crouse > Signed-off-by: Sean Paul > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 - > 1 file changed, 1 deletion(-) Reviewed-by: Jordan Crouse > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index 885bf88afa3e..50d0e9ba5d2f 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -243,7 +243,6 @@ static int _dpu_debugfs_init(struct dpu_kms *dpu_kms) > if (IS_ERR_OR_NULL(entry)) > return -ENODEV; > > - /* allow root to be NULL */ > debugfs_create_x32(DPU_DEBUGFS_HWMASKNAME, 0600, entry, p); > > dpu_debugfs_danger_init(dpu_kms, entry); > -- > Sean Paul, Software Engineer, Google / Chromium OS > -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On Wed, 29 May 2019 at 02:47, Emil Velikov wrote: > > On 2019/05/28, Koenig, Christian wrote: > > Am 28.05.19 um 18:10 schrieb Emil Velikov: > > > On 2019/05/28, Daniel Vetter wrote: > > >> On Tue, May 28, 2019 at 10:03 AM Koenig, Christian > > >> wrote: > > >>> Am 28.05.19 um 09:38 schrieb Daniel Vetter: > > [SNIP] > > > Might be a good idea looking into reverting it partially, so that at > > > least command submission and buffer allocation is still blocked. > > I thought the issue is a lot more than vainfo, it's pretty much every > > hacked up compositor under the sun getting this wrong one way or > > another. Thinking about this some more, I also have no idea how you'd > > want to deprecate rendering on primary nodes in general. Apparently > > that breaks -modesetting already, and probably lots more compositors. > > And it looks like we're finally achieve the goal kms set out to 10 > > years ago, and new compositors are sprouting up all the time. I guess > > we could just break them all (on new hardware) and tell them to all > > suck it up. But I don't think that's a great option. And just > > deprecating this on amdgpu is going to be even harder, since then > > everywhere else it'll keep working, and it's just amdgpu.ko that looks > > broken. > > > > Aside: I'm not supporting Emil's idea here because it fixes any issues > > Intel has - Intel doesn't care. I support it because reality sucks, > > people get this render vs. primary vs. multi-gpu prime wrong all the > > time (that's also why we have hardcoded display+gpu pairs in mesa for > > the various soc combinations out there), and this looks like a > > pragmatic solution. It'd be nice if every compositor and everything > > else would perfectly support multi gpu and only use render nodes for > > rendering, and only primary nodes for display. But reality is that > > people hack on stuff until gears on screen and then move on to more > > interesting things (to them). So I don't think we'll ever win this :-/ > > >>> Yeah, but this is a classic case of working around user space issues by > > >>> making kernel changes instead of fixing user space. > > >>> > > >>> Having privileged (output control) and unprivileged (rendering control) > > >>> functionality behind the same node is a mistake we have made a long time > > >>> ago and render nodes finally seemed to be a way to fix that. > > >>> > > >>> I mean why are compositors using the primary node in the first place? > > >>> Because they want to have access to privileged resources I think and in > > >>> this case it is perfectly ok to do so. > > >>> > > >>> Now extending unprivileged access to the primary node actually sounds > > >>> like a step into the wrong direction to me. > > >>> > > >>> I rather think that we should go down the route of completely dropping > > >>> command submission and buffer allocation through the primary node for > > >>> non master clients. And then as next step at some point drop support for > > >>> authentication/flink. > > >>> > > >>> I mean we have done this with UMS as well and I don't see much other way > > >>> to move forward and get rid of those ancient interface in the long term. > > >> Well kms had some really good benefits that drove quick adoption, like > > >> "suspend/resume actually has a chance of working" or "comes with > > >> buffer management so you can run multiple gears". > > >> > > >> The render node thing is a lot more niche use case (prime, better priv > > >> separation), plus "it's cleaner design". And the "cleaner design" part > > >> is something that empirically doesn't seem to matter :-/ Just two > > >> examples: > > >> - KHR_display/leases just iterated display resources on the fd needed > > >> for rendering (and iirc there was even a patch to expose that for > > >> render nodes too so it works with DRI3), because implementing > > >> protocols is too hard. Barely managed to stop that one before it > > >> happened. > > >> - Various video players use the vblank ioctl on directly to schedule > > >> frames, without telling the compositor. I discovered that when I > > >> wanted to limite the vblank ioctl to master clients only. Again, > > >> apparently too hard to use the existing extensions, or fix the bugs in > > >> there, or whatever. One userspace got fixed last year, but it'll > > >> probably get copypasted around forever :-/ > > >> > > >> So I don't think we'll ever manage to roll a clean split out, and best > > >> we can do is give in and just hand userspace what it wants. As much as > > >> that's misguided and unclean and all that. Maybe it'll result in a > > >> least fewer stuff getting run as root to hack around this, because > > >> fixing properly seems not to be on the table. > > >> > > >> The beauty of kms is that we've achieved the mission, everyone's > > >> writing their own thing. Which is also terrible, and I don't
[Bug 110786] Adaptive sync (vrr, freesync): Cinnamon DE isn't blacklisted properly
https://bugs.freedesktop.org/show_bug.cgi?id=110786 Bug ID: 110786 Summary: Adaptive sync (vrr, freesync): Cinnamon DE isn't blacklisted properly Product: Mesa Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: dron1...@gmail.com QA Contact: dri-devel@lists.freedesktop.org Hello, Despite the muffin being blacklisted, Cinnamon still use adaptive sync causing flicker and mouse stuttering. Adding cinnamon manually to 00-mesa-defaults.conf solves issue: Not sure if issue existing on all Linux distros, but my Arch is affected. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110786] Adaptive sync (vrr, freesync): Cinnamon DE isn't blacklisted properly
https://bugs.freedesktop.org/show_bug.cgi?id=110786 dron1...@gmail.com changed: What|Removed |Added Severity|normal |enhancement -- 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 0/2] drm/amd/display: Add HDR output metadata support for amdgpu
This patch series enables HDR output metadata support in amdgpu using the DRM HDR interface merged in drm-misc-next. Enabled for DCE and DCN ASICs over DP and HDMI. It's limited to static HDR metadata support for now since that's all the DRM interface supports. It requires a modeset for entering and exiting HDR but the metadata can be changed without one. Cc: Harry Wentland Nicholas Kazlauskas (2): drm/amd/display: Expose HDR output metadata for supported connectors drm/amd/display: Only force modesets when toggling HDR .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 155 +- 1 file changed, 151 insertions(+), 4 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/amd/display: Only force modesets when toggling HDR
[Why]
We can issue HDR static metadata as part of stream updates for
non-modesets as long as we force a modeset when entering or exiting HDR.
This avoids unnecessary blanking for simple metadata updates.
[How]
When changing scaling and abm for the stream also check if HDR has
changed and send the stream update. This will only happen in non-modeset
cases.
Cc: Harry Wentland
Signed-off-by: Nicholas Kazlauskas
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 34 +++
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index eb31acca7ed6..443b13ec268d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3978,9 +3978,16 @@ amdgpu_dm_connector_atomic_check(struct drm_connector
*conn,
* DC considers the stream backends changed if the
* static metadata changes. Forcing the modeset also
* gives a simple way for userspace to switch from
-* 8bpc to 10bpc when setting the metadata.
+* 8bpc to 10bpc when setting the metadata to enter
+* or exit HDR.
+*
+* Changing the static metadata after it's been
+* set is permissible, however. So only force a
+* modeset if we're entering or exiting HDR.
*/
- new_crtc_state->mode_changed = true;
+ new_crtc_state->mode_changed =
+ !old_con_state->hdr_output_metadata ||
+ !new_con_state->hdr_output_metadata;
}
return 0;
@@ -5881,7 +5888,9 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
struct amdgpu_crtc *acrtc =
to_amdgpu_crtc(dm_new_con_state->base.crtc);
struct dc_surface_update dummy_updates[MAX_SURFACES];
struct dc_stream_update stream_update;
+ struct dc_info_packet hdr_packet;
struct dc_stream_status *status = NULL;
+ bool abm_changed, hdr_changed, scaling_changed;
memset(&dummy_updates, 0, sizeof(dummy_updates));
memset(&stream_update, 0, sizeof(stream_update));
@@ -5898,11 +5907,19 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
- if (!is_scaling_state_different(dm_new_con_state,
dm_old_con_state) &&
- (dm_new_crtc_state->abm_level ==
dm_old_crtc_state->abm_level))
+ scaling_changed = is_scaling_state_different(dm_new_con_state,
+dm_old_con_state);
+
+ abm_changed = dm_new_crtc_state->abm_level !=
+ dm_old_crtc_state->abm_level;
+
+ hdr_changed =
+ is_hdr_metadata_different(old_con_state, new_con_state);
+
+ if (!scaling_changed && !abm_changed && !hdr_changed)
continue;
- if (is_scaling_state_different(dm_new_con_state,
dm_old_con_state)) {
+ if (scaling_changed) {
update_stream_scaling_settings(&dm_new_con_state->base.crtc->mode,
dm_new_con_state, (struct
dc_stream_state *)dm_new_crtc_state->stream);
@@ -5910,12 +5927,17 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
stream_update.dst = dm_new_crtc_state->stream->dst;
}
- if (dm_new_crtc_state->abm_level !=
dm_old_crtc_state->abm_level) {
+ if (abm_changed) {
dm_new_crtc_state->stream->abm_level =
dm_new_crtc_state->abm_level;
stream_update.abm_level = &dm_new_crtc_state->abm_level;
}
+ if (hdr_changed) {
+ fill_hdr_info_packet(new_con_state, &hdr_packet);
+ stream_update.hdr_static_metadata = &hdr_packet;
+ }
+
status = dc_stream_get_status(dm_new_crtc_state->stream);
WARN_ON(!status);
WARN_ON(!status->plane_count);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/amd/display: Expose HDR output metadata for supported connectors
[Why]
For userspace to send static HDR metadata to the display we need to
attach the property on the connector and send it to DC.
[How]
The property is attached to HDMI and DP connectors. Since the metadata
isn't actually available when creating the connector this isn't a
property we can dynamically support based on the extension block
being available or not.
When the HDR metadata is changed a modeset will be forced for now.
We need to switch from 8bpc to 10bpc in most cases anyway, and we want
to fully exit HDR mode when userspace gives us a NULL metadata, so this
isn't completely unnecessary.
The requirement can later be reduced to just entering and exiting HDR
or switching max bpc.
Cc: Harry Wentland
Signed-off-by: Nicholas Kazlauskas
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 125 ++
1 file changed, 125 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 995f9df66142..eb31acca7ed6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3871,6 +3871,121 @@ enum drm_mode_status
amdgpu_dm_connector_mode_valid(struct drm_connector *connec
return result;
}
+static int fill_hdr_info_packet(const struct drm_connector_state *state,
+ struct dc_info_packet *out)
+{
+ struct hdmi_drm_infoframe frame;
+ unsigned char buf[30]; /* 26 + 4 */
+ ssize_t len;
+ int ret, i;
+
+ memset(out, 0, sizeof(*out));
+
+ if (!state->hdr_output_metadata)
+ return 0;
+
+ ret = drm_hdmi_infoframe_set_hdr_metadata(&frame, state);
+ if (ret)
+ return ret;
+
+ len = hdmi_drm_infoframe_pack_only(&frame, buf, sizeof(buf));
+ if (len < 0)
+ return (int)len;
+
+ /* Static metadata is a fixed 26 bytes + 4 byte header. */
+ if (len != 30)
+ return -EINVAL;
+
+ /* Prepare the infopacket for DC. */
+ switch (state->connector->connector_type) {
+ case DRM_MODE_CONNECTOR_HDMIA:
+ out->hb0 = 0x87; /* type */
+ out->hb1 = 0x01; /* version */
+ out->hb2 = 0x1A; /* length */
+ out->sb[0] = buf[3]; /* checksum */
+ i = 1;
+ break;
+
+ case DRM_MODE_CONNECTOR_DisplayPort:
+ case DRM_MODE_CONNECTOR_eDP:
+ out->hb0 = 0x00; /* sdp id, zero */
+ out->hb1 = 0x87; /* type */
+ out->hb2 = 0x1D; /* payload len - 1 */
+ out->hb3 = (0x13 << 2); /* sdp version */
+ out->sb[0] = 0x01; /* version */
+ out->sb[1] = 0x1A; /* length */
+ i = 2;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ memcpy(&out->sb[i], &buf[4], 26);
+ out->valid = true;
+
+ print_hex_dump(KERN_DEBUG, "HDR SB:", DUMP_PREFIX_NONE, 16, 1, out->sb,
+ sizeof(out->sb), false);
+
+ return 0;
+}
+
+static bool
+is_hdr_metadata_different(const struct drm_connector_state *old_state,
+ const struct drm_connector_state *new_state)
+{
+ struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
+ struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
+
+ if (old_blob != new_blob) {
+ if (old_blob && new_blob &&
+ old_blob->length == new_blob->length)
+ return memcmp(old_blob->data, new_blob->data,
+ old_blob->length);
+
+ return true;
+ }
+
+ return false;
+}
+
+static int
+amdgpu_dm_connector_atomic_check(struct drm_connector *conn,
+struct drm_connector_state *new_con_state)
+{
+ struct drm_atomic_state *state = new_con_state->state;
+ struct drm_connector_state *old_con_state =
+ drm_atomic_get_old_connector_state(state, conn);
+ struct drm_crtc *crtc = new_con_state->crtc;
+ struct drm_crtc_state *new_crtc_state;
+ int ret;
+
+ if (!crtc)
+ return 0;
+
+ if (is_hdr_metadata_different(old_con_state, new_con_state)) {
+ struct dc_info_packet hdr_infopacket;
+
+ ret = fill_hdr_info_packet(new_con_state, &hdr_infopacket);
+ if (ret)
+ return ret;
+
+ new_crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(new_crtc_state))
+ return PTR_ERR(new_crtc_state);
+
+ /*
+* DC considers the stream backends changed if the
+* static metadata changes. Forcing the modeset also
+* gives a simple way for userspace to switch from
+* 8bpc to 10bpc when setting the metadata.
+*/
+
[Bug 106302] [radeonsi] Garbage content when accessing a texture in multiple shared EGL contexts
https://bugs.freedesktop.org/show_bug.cgi?id=106302 --- Comment #4 from s...@vestigecounty.com --- Thank you for being exactly on point, it turns out I was using a frivolous interpretation of "change" rather than the one specified in OpenGL ES. The bug can safely be closed as invalid, as fence is necessary in this case. One thing that is curious to me still, is whether sampling the texture in the second thread should yield (0,0,0), since the texture change has not completed yet, and associated texture object is thus incomplete. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dpu: Remove bogus comment
From: Sean Paul This comment doesn't make any sense, remove it. Suggested-by: Jordan Crouse Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 885bf88afa3e..50d0e9ba5d2f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -243,7 +243,6 @@ static int _dpu_debugfs_init(struct dpu_kms *dpu_kms) if (IS_ERR_OR_NULL(entry)) return -ENODEV; - /* allow root to be NULL */ debugfs_create_x32(DPU_DEBUGFS_HWMASKNAME, 0600, entry, p); dpu_debugfs_danger_init(dpu_kms, entry); -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dpu: Fix mmu init/destroy functions
From: Sean Paul
There's a comment in _dpu_kms_hw_destroy() that reads "safe to call
these more than once during shutdown", referring to
_dpu_kms_mmu_destroy(). Unfortunately that's not the case, mmu_destroy
will fail hard if it's called twice. So fix that function to ensure it
can be called multiple times without burning.
While I'm at it, fix the error paths in _dpu_kms_mmu_init() to properly
clean up the iommu domain and not call _dpu_kms_mmu_destroy() when
things are only partially setup.
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 885bf88afa3e..d50afbb37a0a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -729,12 +729,16 @@ static int _dpu_kms_mmu_destroy(struct dpu_kms *dpu_kms)
{
struct msm_mmu *mmu;
+ if (!dpu_kms->base.aspace)
+ return 0;
+
mmu = dpu_kms->base.aspace->mmu;
mmu->funcs->detach(mmu, (const char **)iommu_ports,
ARRAY_SIZE(iommu_ports));
msm_gem_address_space_put(dpu_kms->base.aspace);
+ dpu_kms->base.aspace = NULL;
return 0;
}
@@ -754,25 +758,20 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms)
aspace = msm_gem_address_space_create(dpu_kms->dev->dev,
domain, "dpu1");
if (IS_ERR(aspace)) {
- ret = PTR_ERR(aspace);
- goto fail;
+ iommu_domain_free(domain);
+ return PTR_ERR(aspace);
}
- dpu_kms->base.aspace = aspace;
-
ret = aspace->mmu->funcs->attach(aspace->mmu, iommu_ports,
ARRAY_SIZE(iommu_ports));
if (ret) {
DPU_ERROR("failed to attach iommu %d\n", ret);
msm_gem_address_space_put(aspace);
- goto fail;
+ return ret;
}
+ dpu_kms->base.aspace = aspace;
return 0;
-fail:
- _dpu_kms_mmu_destroy(dpu_kms);
-
- return ret;
}
static struct dss_clk *_dpu_kms_get_clk(struct dpu_kms *dpu_kms,
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm/ttm, drm/vmwgfx: Have TTM support AMD SEV encryption
On 5/28/19 12:05 PM, Thomas Hellstrom wrote: > On 5/28/19 7:00 PM, Lendacky, Thomas wrote: >> On 5/28/19 11:32 AM, Koenig, Christian wrote: >>> Am 28.05.19 um 18:27 schrieb Thomas Hellstrom: On Tue, 2019-05-28 at 15:50 +, Lendacky, Thomas wrote: > On 5/28/19 10:17 AM, Koenig, Christian wrote: >> Hi Thomas, >> >> Am 28.05.19 um 17:11 schrieb Thomas Hellstrom: >>> Hi, Tom, >>> >>> Thanks for the reply. The question is not graphics specific, but >>> lies >>> in your answer further below: >>> >>> On 5/28/19 4:48 PM, Lendacky, Thomas wrote: On 5/28/19 2:31 AM, Thomas Hellstrom wrote: [SNIP] As for kernel vmaps and user-maps, those pages will be marked encrypted (unless explicitly made un-encrypted by calling set_memory_decrypted()). But, if you are copying to/from those areas into the un- encrypted DMA area then everything will be ok. >>> The question is regarding the above paragraph. >>> >>> AFAICT, set_memory_decrypted() only changes the fixed kernel map >>> PTEs. >>> But when setting up other aliased PTEs to the exact same >>> decrypted >>> pages, for example using dma_mmap_coherent(), >>> kmap_atomic_prot(), >>> vmap() etc. What code is responsible for clearing the encrypted >>> flag >>> on those PTEs? Is there something in the x86 platform code doing >>> that? >> Tom actually explained this: >>> The encryption bit is bit-47 of a physical address. >> In other words set_memory_decrypted() changes the physical address >> in >> the PTEs of the kernel mapping and all other use cases just copy >> that >> from there. > Except I don't think the PTE attributes are copied from the kernel > mapping +1! > in some cases. For example, dma_mmap_coherent() will create the same > vm_page_prot value regardless of whether or not the underlying memory > is > encrypted or not. But kmap_atomic_prot() will return the kernel > virtual > address of the page, so that would be fine. Yes, on 64-bit systems. On 32-bit systems (do they exist with SEV?), they don't. >>> I don't think so, but feel free to prove me wrong Tom. >> SEV is 64-bit only. > > And I just noticed that kmap_atomic_prot() indeed returns the kernel map > also for 32-bit lowmem. > >> And similarly TTM user-space mappings and vmap() doesn't copy from the kernel map either, so I think we actually do need to modify the page- prot like done in the patch. >>> Well the problem is that this won't have any effect. >>> >>> As Tom explained encryption is not implemented as a page protection bit, >>> but rather as part of the physical address of the part. >> This is where things get interesting. Even though the encryption bit is >> part of the physical address (e.g. under SME the device could/would use an >> address with the encryption bit set), it is implemented as part of the PTE >> attributes. So, for example, using _PAGE_ENC when building a pgprot value >> would produce an entry with the encryption bit set. >> >> And the thing to watch out for is using two virtual addresses that point >> to the same physical address (page) in DRAM but one has the encryption bit >> set and one doesn't. The hardware does not enforce coherency between an >> encrypted and un-encrypted mapping of the same physical address (page). >> See section 7.10.6 of the AMD64 Architecture Programmer's Manual Volume 2. > > Indeed. And I'm pretty sure the kernel map PTE and a TTM / vmap PTE > pointing to the same decrypted page differ in the encryption bit (47) > setting. > > But on the hypervisor that would sort of work, because from what I > understand with SEV we select between the guest key and the hypervisor > key with that bit. On the hypervisor both keys are the same? On a guest > it would probably break. For SEV, if the encryption bit is set then the guest key is used. If the encryption bit is not set, then the hypervisor key is used IFF the encryption bit is set in the hypervisor page tables. You can have SEV guests regardless of whether SME is active (note, there is a difference between SME being enabled vs. SME being active). For SME, there is only one key. The encryption bit determines whether the data is run through the encryption process or not. Thanks, Tom > > /Thomas > >> >> Thanks, >> Tom >> >>> I have no idea how that is actually handled thought, >>> Christian. >>> /Thomas > This is an area that needs looking into to be sure it is working > properly > with SME and SEV. > > Thanks, > Tom > >> That's rather nifty, because this way everybody will either use or >> not >> use encryption on the page. >> >> Christian. >> >>> Thanks, >>> Thomas >>> >>> Things get fuzzy for me when it comes to the GPU access of the memory
Re: [PATCH 07/10] drm/amd/display: use ttm_eu_reserve_buffers instead of amdgpu_bo_reserve v2
On Tue, May 28, 2019 at 1:05 PM Sam Ravnborg wrote:
>
> Hi Christian.
>
> On Tue, May 28, 2019 at 06:25:54PM +0200, Christian König wrote:
> > From: Chunming Zhou
> >
> > add ticket for display bo, so that it can preempt busy bo.
> >
> > v2: fix stupid rebase error
> >
> > Change-Id: I9f031cdcc8267de00e819ae303baa0a52df8ebb9
> What is this?
> I do not recall seeing this in a changelog before?
It's an artifact of gerrit which we use internally for git management.
Alex
>
> (Sorry for not commenting on the patch, most of it is beyond my
> understanding for now).
>
> Sam
>
> > Signed-off-by: Chunming Zhou
> > Reviewed-by: Christian König
> > ---
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 21 ++-
> > 1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 4a1755bce96c..56f320f3fd72 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -4182,6 +4182,9 @@ static int dm_plane_helper_prepare_fb(struct
> > drm_plane *plane,
> > struct amdgpu_device *adev;
> > struct amdgpu_bo *rbo;
> > struct dm_plane_state *dm_plane_state_new, *dm_plane_state_old;
> > + struct list_head list;
> > + struct ttm_validate_buffer tv;
> > + struct ww_acquire_ctx ticket;
> > uint64_t tiling_flags;
> > uint32_t domain;
> > int r;
> > @@ -4198,9 +4201,17 @@ static int dm_plane_helper_prepare_fb(struct
> > drm_plane *plane,
> > obj = new_state->fb->obj[0];
> > rbo = gem_to_amdgpu_bo(obj);
> > adev = amdgpu_ttm_adev(rbo->tbo.bdev);
> > - r = amdgpu_bo_reserve(rbo, false);
> > - if (unlikely(r != 0))
> > + INIT_LIST_HEAD(&list);
> > +
> > + tv.bo = &rbo->tbo;
> > + tv.num_shared = 1;
> > + list_add(&tv.head, &list);
> > +
> > + r = ttm_eu_reserve_buffers(&ticket, &list, false, NULL, true);
> > + if (r) {
> > + dev_err(adev->dev, "fail to reserve bo (%d)\n", r);
> > return r;
> > + }
> >
> > if (plane->type != DRM_PLANE_TYPE_CURSOR)
> > domain = amdgpu_display_supported_domains(adev);
> > @@ -4211,21 +4222,21 @@ static int dm_plane_helper_prepare_fb(struct
> > drm_plane *plane,
> > if (unlikely(r != 0)) {
> > if (r != -ERESTARTSYS)
> > DRM_ERROR("Failed to pin framebuffer with error
> > %d\n", r);
> > - amdgpu_bo_unreserve(rbo);
> > + ttm_eu_backoff_reservation(&ticket, &list);
> > return r;
> > }
> >
> > r = amdgpu_ttm_alloc_gart(&rbo->tbo);
> > if (unlikely(r != 0)) {
> > amdgpu_bo_unpin(rbo);
> > - amdgpu_bo_unreserve(rbo);
> > + ttm_eu_backoff_reservation(&ticket, &list);
> > DRM_ERROR("%p bind failed\n", rbo);
> > return r;
> > }
> >
> > amdgpu_bo_get_tiling_flags(rbo, &tiling_flags);
> >
> > - amdgpu_bo_unreserve(rbo);
> > + ttm_eu_backoff_reservation(&ticket, &list);
> >
> > afb->address = amdgpu_bo_gpu_offset(rbo);
> >
> > --
> > 2.17.1
> >
> > ___
> > 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/adreno: Add A540 support
The A540 is a derivative of the A530, and is found in the MSM8998 SoC.
Signed-off-by: Jeffrey Hugo
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 22 +++
drivers/gpu/drm/msm/adreno/a5xx_power.c| 76 +-
drivers/gpu/drm/msm/adreno/adreno_device.c | 18 +
drivers/gpu/drm/msm/adreno/adreno_gpu.h| 6 ++
4 files changed, 119 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index e5fcefa49f19..7ca8fde22fd8 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -318,12 +318,19 @@ static const struct {
void a5xx_set_hwcg(struct msm_gpu *gpu, bool state)
{
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
unsigned int i;
for (i = 0; i < ARRAY_SIZE(a5xx_hwcg); i++)
gpu_write(gpu, a5xx_hwcg[i].offset,
state ? a5xx_hwcg[i].value : 0);
+ if (adreno_is_a540(adreno_gpu)) {
+ gpu_write(gpu, REG_A5XX_RBBM_CLOCK_HYST_GPMU, 0x0222);
+ gpu_write(gpu, REG_A5XX_RBBM_CLOCK_DELAY_GPMU, 0x0770);
+ gpu_write(gpu, REG_A5XX_RBBM_CLOCK_HYST_GPMU, 0x0004);
+ }
+
gpu_write(gpu, REG_A5XX_RBBM_CLOCK_CNTL, state ? 0xAAA8AA00 : 0);
gpu_write(gpu, REG_A5XX_RBBM_ISDB_CNT, state ? 0x182 : 0x180);
}
@@ -507,6 +514,9 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
gpu_write(gpu, REG_A5XX_VBIF_ROUND_ROBIN_QOS_ARB, 0x0003);
+ if (adreno_is_a540(adreno_gpu))
+ gpu_write(gpu, REG_A5XX_VBIF_GATE_OFF_WRREQ_EN, 0x0009);
+
/* Make all blocks contribute to the GPU BUSY perf counter */
gpu_write(gpu, REG_A5XX_RBBM_PERFCTR_GPU_BUSY_MASKED, 0x);
@@ -592,6 +602,8 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
/* Set the highest bank bit */
gpu_write(gpu, REG_A5XX_TPL1_MODE_CNTL, 2 << 7);
gpu_write(gpu, REG_A5XX_RB_MODE_CNTL, 2 << 1);
+ if (adreno_is_a540(adreno_gpu))
+ gpu_write(gpu, REG_A5XX_UCHE_DBG_ECO_CNTL_2, 2);
/* Protect registers from the CP */
gpu_write(gpu, REG_A5XX_CP_PROTECT_CNTL, 0x0007);
@@ -642,6 +654,16 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
REG_A5XX_RBBM_SECVID_TSB_TRUSTED_BASE_HI, 0x);
gpu_write(gpu, REG_A5XX_RBBM_SECVID_TSB_TRUSTED_SIZE, 0x);
+ /*
+* VPC corner case with local memory load kill leads to corrupt
+* internal state. Normal Disable does not work for all a5x chips.
+* So do the following setting to disable it.
+*/
+ if (adreno_gpu->info->quirks & ADRENO_QUIRK_LMLOADKILL_DISABLE) {
+ gpu_rmw(gpu, REG_A5XX_VPC_DBG_ECO_CNTL, 0, BIT(23));
+ gpu_rmw(gpu, 0xE04, BIT(18), 0); /*REG_A5XX_HLSQ_DBG_ECO_CNTL*/
+ }
+
ret = adreno_hw_init(gpu);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_power.c
b/drivers/gpu/drm/msm/adreno/a5xx_power.c
index 70e65c94e525..5cb325c6eb8f 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_power.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_power.c
@@ -32,6 +32,18 @@
#define AGC_POWER_CONFIG_PRODUCTION_ID 1
#define AGC_INIT_MSG_VALUE 0xBABEFACE
+/* AGC_LM_CONFIG (A540+) */
+#define AGC_LM_CONFIG (136/4)
+#define AGC_LM_CONFIG_GPU_VERSION_SHIFT 17
+#define AGC_LM_CONFIG_ENABLE_GPMU_ADAPTIVE 1
+#define AGC_LM_CONFIG_THROTTLE_DISABLE (2 << 8)
+#define AGC_LM_CONFIG_ISENSE_ENABLE (1 << 4)
+#define AGC_LM_CONFIG_ENABLE_ERROR (3 << 4)
+#define AGC_LM_CONFIG_LLM_ENABLED (1 << 16)
+#define AGC_LM_CONFIG_BCL_DISABLED (1 << 24)
+
+#define AGC_LEVEL_CONFIG (140/4)
+
static struct {
uint32_t reg;
uint32_t value;
@@ -116,7 +128,7 @@ static inline uint32_t _get_mvolts(struct msm_gpu *gpu,
uint32_t freq)
}
/* Setup thermal limit management */
-static void a5xx_lm_setup(struct msm_gpu *gpu)
+static void a530_lm_setup(struct msm_gpu *gpu)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
struct a5xx_gpu *a5xx_gpu = to_a5xx_gpu(adreno_gpu);
@@ -165,6 +177,45 @@ static void a5xx_lm_setup(struct msm_gpu *gpu)
gpu_write(gpu, AGC_INIT_MSG_MAGIC, AGC_INIT_MSG_VALUE);
}
+#define PAYLOAD_SIZE(_size) ((_size) * sizeof(u32))
+#define LM_DCVS_LIMIT 1
+#define LEVEL_CONFIG ~(0x303)
+
+static void a540_lm_setup(struct msm_gpu *gpu)
+{
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ u32 config;
+
+ /* The battery current limiter isn't enabled for A540 */
+ config = AGC_LM_CONFIG_BCL_DISABLED;
+ config |= adreno_gpu->rev.patchid << AGC_LM_CONFIG_GPU_VERSION_SHIFT;
+
+ /* For now disable GPMU side throttling */
+ config |= AGC_LM_CONFIG_THROTTLE_DISABLE;
+
+ /* Until we get clock scaling 0 is always the active power level */
+ gpu_write(gpu, REG_A5XX_GPMU_GPMU_VOLTAGE, 0x8000 | 0);
+
+ /* Fixed at 6000 for no
Re: [PATCH 02/12] dma-buf: add dma_buf_(un)map_attachment_locked variants v3
On Mon, 27 May 2019 18:56:20 +0800 Christian Koenig wrote: > Thanks for the comments, but you are looking at a completely outdated > patchset. > > If you are interested in the newest one please ping me and I'm going to CC you > when I send out the next version. > Ping... Thanks Hillf ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/nouveau/svm: Convert to use hmm_range_fault()
On Tue, May 21, 2019 at 12:27 AM Souptick Joarder wrote:
>
> Convert to use hmm_range_fault().
Any comment on this patch ?
>
> Signed-off-by: Souptick Joarder
> ---
> drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
> b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index 93ed43c..8d56bd6 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -649,7 +649,7 @@ struct nouveau_svmm {
> range.values = nouveau_svm_pfn_values;
> range.pfn_shift = NVIF_VMM_PFNMAP_V0_ADDR_SHIFT;
> again:
> - ret = hmm_vma_fault(&range, true);
> + ret = hmm_range_fault(&range, true);
> if (ret == 0) {
> mutex_lock(&svmm->mutex);
> if (!hmm_vma_range_done(&range)) {
> --
> 1.9.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/10] drm/amd/display: use ttm_eu_reserve_buffers instead of amdgpu_bo_reserve v2
Hi Christian.
On Tue, May 28, 2019 at 06:25:54PM +0200, Christian König wrote:
> From: Chunming Zhou
>
> add ticket for display bo, so that it can preempt busy bo.
>
> v2: fix stupid rebase error
>
> Change-Id: I9f031cdcc8267de00e819ae303baa0a52df8ebb9
What is this?
I do not recall seeing this in a changelog before?
(Sorry for not commenting on the patch, most of it is beyond my
understanding for now).
Sam
> Signed-off-by: Chunming Zhou
> Reviewed-by: Christian König
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 21 ++-
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 4a1755bce96c..56f320f3fd72 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4182,6 +4182,9 @@ static int dm_plane_helper_prepare_fb(struct drm_plane
> *plane,
> struct amdgpu_device *adev;
> struct amdgpu_bo *rbo;
> struct dm_plane_state *dm_plane_state_new, *dm_plane_state_old;
> + struct list_head list;
> + struct ttm_validate_buffer tv;
> + struct ww_acquire_ctx ticket;
> uint64_t tiling_flags;
> uint32_t domain;
> int r;
> @@ -4198,9 +4201,17 @@ static int dm_plane_helper_prepare_fb(struct drm_plane
> *plane,
> obj = new_state->fb->obj[0];
> rbo = gem_to_amdgpu_bo(obj);
> adev = amdgpu_ttm_adev(rbo->tbo.bdev);
> - r = amdgpu_bo_reserve(rbo, false);
> - if (unlikely(r != 0))
> + INIT_LIST_HEAD(&list);
> +
> + tv.bo = &rbo->tbo;
> + tv.num_shared = 1;
> + list_add(&tv.head, &list);
> +
> + r = ttm_eu_reserve_buffers(&ticket, &list, false, NULL, true);
> + if (r) {
> + dev_err(adev->dev, "fail to reserve bo (%d)\n", r);
> return r;
> + }
>
> if (plane->type != DRM_PLANE_TYPE_CURSOR)
> domain = amdgpu_display_supported_domains(adev);
> @@ -4211,21 +4222,21 @@ static int dm_plane_helper_prepare_fb(struct
> drm_plane *plane,
> if (unlikely(r != 0)) {
> if (r != -ERESTARTSYS)
> DRM_ERROR("Failed to pin framebuffer with error %d\n",
> r);
> - amdgpu_bo_unreserve(rbo);
> + ttm_eu_backoff_reservation(&ticket, &list);
> return r;
> }
>
> r = amdgpu_ttm_alloc_gart(&rbo->tbo);
> if (unlikely(r != 0)) {
> amdgpu_bo_unpin(rbo);
> - amdgpu_bo_unreserve(rbo);
> + ttm_eu_backoff_reservation(&ticket, &list);
> DRM_ERROR("%p bind failed\n", rbo);
> return r;
> }
>
> amdgpu_bo_get_tiling_flags(rbo, &tiling_flags);
>
> - amdgpu_bo_unreserve(rbo);
> + ttm_eu_backoff_reservation(&ticket, &list);
>
> afb->address = amdgpu_bo_gpu_offset(rbo);
>
> --
> 2.17.1
>
> ___
> 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: [RFC PATCH] drm/ttm, drm/vmwgfx: Have TTM support AMD SEV encryption
On 5/28/19 7:00 PM, Lendacky, Thomas wrote: On 5/28/19 11:32 AM, Koenig, Christian wrote: Am 28.05.19 um 18:27 schrieb Thomas Hellstrom: On Tue, 2019-05-28 at 15:50 +, Lendacky, Thomas wrote: On 5/28/19 10:17 AM, Koenig, Christian wrote: Hi Thomas, Am 28.05.19 um 17:11 schrieb Thomas Hellstrom: Hi, Tom, Thanks for the reply. The question is not graphics specific, but lies in your answer further below: On 5/28/19 4:48 PM, Lendacky, Thomas wrote: On 5/28/19 2:31 AM, Thomas Hellstrom wrote: [SNIP] As for kernel vmaps and user-maps, those pages will be marked encrypted (unless explicitly made un-encrypted by calling set_memory_decrypted()). But, if you are copying to/from those areas into the un- encrypted DMA area then everything will be ok. The question is regarding the above paragraph. AFAICT, set_memory_decrypted() only changes the fixed kernel map PTEs. But when setting up other aliased PTEs to the exact same decrypted pages, for example using dma_mmap_coherent(), kmap_atomic_prot(), vmap() etc. What code is responsible for clearing the encrypted flag on those PTEs? Is there something in the x86 platform code doing that? Tom actually explained this: The encryption bit is bit-47 of a physical address. In other words set_memory_decrypted() changes the physical address in the PTEs of the kernel mapping and all other use cases just copy that from there. Except I don't think the PTE attributes are copied from the kernel mapping +1! in some cases. For example, dma_mmap_coherent() will create the same vm_page_prot value regardless of whether or not the underlying memory is encrypted or not. But kmap_atomic_prot() will return the kernel virtual address of the page, so that would be fine. Yes, on 64-bit systems. On 32-bit systems (do they exist with SEV?), they don't. I don't think so, but feel free to prove me wrong Tom. SEV is 64-bit only. And I just noticed that kmap_atomic_prot() indeed returns the kernel map also for 32-bit lowmem. And similarly TTM user-space mappings and vmap() doesn't copy from the kernel map either, so I think we actually do need to modify the page- prot like done in the patch. Well the problem is that this won't have any effect. As Tom explained encryption is not implemented as a page protection bit, but rather as part of the physical address of the part. This is where things get interesting. Even though the encryption bit is part of the physical address (e.g. under SME the device could/would use an address with the encryption bit set), it is implemented as part of the PTE attributes. So, for example, using _PAGE_ENC when building a pgprot value would produce an entry with the encryption bit set. And the thing to watch out for is using two virtual addresses that point to the same physical address (page) in DRAM but one has the encryption bit set and one doesn't. The hardware does not enforce coherency between an encrypted and un-encrypted mapping of the same physical address (page). See section 7.10.6 of the AMD64 Architecture Programmer's Manual Volume 2. Indeed. And I'm pretty sure the kernel map PTE and a TTM / vmap PTE pointing to the same decrypted page differ in the encryption bit (47) setting. But on the hypervisor that would sort of work, because from what I understand with SEV we select between the guest key and the hypervisor key with that bit. On the hypervisor both keys are the same? On a guest it would probably break. /Thomas Thanks, Tom I have no idea how that is actually handled thought, Christian. /Thomas This is an area that needs looking into to be sure it is working properly with SME and SEV. Thanks, Tom That's rather nifty, because this way everybody will either use or not use encryption on the page. Christian. Thanks, Thomas Things get fuzzy for me when it comes to the GPU access of the memory and what and how it is accessed. Thanks, Tom ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/4] drm/bridge: dw-hdmi: Add support for HDR metadata
Hi! Dne nedelja, 26. maj 2019 ob 23:18:46 CEST je Jonas Karlman napisal(a): > Add support for HDR metadata using the hdr_output_metadata connector > property, configure Dynamic Range and Mastering InfoFrame accordingly. > > A drm_infoframe flag is added to dw_hdmi_plat_data that platform drivers > can use to signal when Dynamic Range and Mastering infoframes is supported. > This flag is needed because Amlogic GXBB and GXL report same DW-HDMI > version, and only GXL support DRM InfoFrame. > > The first patch add functionality to configure DRM InfoFrame based on the > hdr_output_metadata connector property. > > The remaining patches sets the drm_infoframe flag on some SoCs supporting > Dynamic Range and Mastering InfoFrame. > > Note that this was based on top of drm-misc-next and Neil Armstrong's > "drm/meson: Add support for HDMI2.0 YUV420 4k60" series at [1] > > [1] https://patchwork.freedesktop.org/series/58725/#rev2 For Allwinner H6: Tested-by: Jernej Skrabec Thanks for working on this! Best regards, Jernej
Re: [PATCH v3 07/10] drm: rcar-du: Skip LVDS1 output on Gen3 when using dual-link LVDS mode
Hi Laurent.
On Tue, May 28, 2019 at 07:50:52PM +0300, Laurent Pinchart wrote:
> Hi Sam,
>
> On Tue, May 28, 2019 at 06:42:13PM +0200, Sam Ravnborg wrote:
> > On Tue, May 28, 2019 at 05:12:31PM +0300, Laurent Pinchart wrote:
> > > In dual-link LVDS mode, the LVDS1 encoder is used as a companion for
> > > LVDS0, and both encoders transmit data from DU0. The LVDS1 output of DU1
> > > can't be used in that case, don't create an encoder and connector for
> > > it.
> > >
> > > Signed-off-by: Laurent Pinchart
> > >
> > > Reviewed-by: Jacopo Mondi
> > > Tested-by: Jacopo Mondi
> > > ---
> > > Changess since v2:
> > >
> > > - Remove unneeded bridge NULL check
> > > ---
> > > drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 12
> > > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
> > > 2 files changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > > b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > > index 6c91753af7bc..0f00bdfe2366 100644
> > > --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > > @@ -16,6 +16,7 @@
> > > #include "rcar_du_drv.h"
> > > #include "rcar_du_encoder.h"
> > > #include "rcar_du_kms.h"
> > > +#include "rcar_lvds.h"
> > >
> > > /*
> > > -
> > > * Encoder
> > > @@ -97,6 +98,17 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
> > > }
> > > }
> > >
> > > + /*
> > > + * On Gen3 skip the LVDS1 output if the LVDS1 encoder is used as a
> > > + * companion for LVDS0 in dual-link mode.
> > > + */
> > > + if (rcdu->info->gen >= 3 && output == RCAR_DU_OUTPUT_LVDS1) {
> >
> > Both subject and comment above says "On Gen3", but the code looks like
> > it implements "On Gen3 or newer" - due to use of ">=".
> > Looks wrong to me.
>
> Gen3 is the newest generation :-) We thus use >= through the DU and LVDS
> drivers to prepare for support of Gen4, just in case.
OK, but I guess we agree that the comment needs a small update them.
Actually I implicitly reads that it is only from Gen3 onwards that the
LVDS1 encoder can be used as a companion.
My initial understanding reading the comment was that this implmented a
workaround for Gen3 - but it is a workarounf for missing features in
older than Gen3.
So, assuming this is correct, when trying to specify a companion on
older then Gen3 should result in some kind of error/warning?
(Maybe it does).
Sam
Re: [RFC PATCH] drm/ttm, drm/vmwgfx: Have TTM support AMD SEV encryption
On 5/28/19 11:32 AM, Koenig, Christian wrote: > Am 28.05.19 um 18:27 schrieb Thomas Hellstrom: >> On Tue, 2019-05-28 at 15:50 +, Lendacky, Thomas wrote: >>> On 5/28/19 10:17 AM, Koenig, Christian wrote: Hi Thomas, Am 28.05.19 um 17:11 schrieb Thomas Hellstrom: > Hi, Tom, > > Thanks for the reply. The question is not graphics specific, but > lies > in your answer further below: > > On 5/28/19 4:48 PM, Lendacky, Thomas wrote: >> On 5/28/19 2:31 AM, Thomas Hellstrom wrote: >> [SNIP] >> As for kernel vmaps and user-maps, those pages will be marked >> encrypted >> (unless explicitly made un-encrypted by calling >> set_memory_decrypted()). >> But, if you are copying to/from those areas into the un- >> encrypted DMA >> area then everything will be ok. > The question is regarding the above paragraph. > > AFAICT, set_memory_decrypted() only changes the fixed kernel map > PTEs. > But when setting up other aliased PTEs to the exact same > decrypted > pages, for example using dma_mmap_coherent(), > kmap_atomic_prot(), > vmap() etc. What code is responsible for clearing the encrypted > flag > on those PTEs? Is there something in the x86 platform code doing > that? Tom actually explained this: > The encryption bit is bit-47 of a physical address. In other words set_memory_decrypted() changes the physical address in the PTEs of the kernel mapping and all other use cases just copy that from there. >>> Except I don't think the PTE attributes are copied from the kernel >>> mapping >> +1! >> >>> in some cases. For example, dma_mmap_coherent() will create the same >>> vm_page_prot value regardless of whether or not the underlying memory >>> is >>> encrypted or not. But kmap_atomic_prot() will return the kernel >>> virtual >>> address of the page, so that would be fine. >> Yes, on 64-bit systems. On 32-bit systems (do they exist with SEV?), >> they don't. > > I don't think so, but feel free to prove me wrong Tom. SEV is 64-bit only. > >> And similarly TTM user-space mappings and vmap() doesn't copy from the >> kernel map either, so I think we actually do need to modify the page- >> prot like done in the patch. > > Well the problem is that this won't have any effect. > > As Tom explained encryption is not implemented as a page protection bit, > but rather as part of the physical address of the part. This is where things get interesting. Even though the encryption bit is part of the physical address (e.g. under SME the device could/would use an address with the encryption bit set), it is implemented as part of the PTE attributes. So, for example, using _PAGE_ENC when building a pgprot value would produce an entry with the encryption bit set. And the thing to watch out for is using two virtual addresses that point to the same physical address (page) in DRAM but one has the encryption bit set and one doesn't. The hardware does not enforce coherency between an encrypted and un-encrypted mapping of the same physical address (page). See section 7.10.6 of the AMD64 Architecture Programmer's Manual Volume 2. Thanks, Tom > > I have no idea how that is actually handled thought, > Christian. > >> >> /Thomas >> >>> This is an area that needs looking into to be sure it is working >>> properly >>> with SME and SEV. >>> >>> Thanks, >>> Tom >>> That's rather nifty, because this way everybody will either use or not use encryption on the page. Christian. > Thanks, > Thomas > > >> Things get fuzzy for me when it comes to the GPU access of the >> memory >> and what and how it is accessed. >> >> Thanks, >> Tom > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 04/10] dt-bindings: display: renesas: lvds: Add renesas, companion property
Hi Laurent. > > > > > > +Optional properties: > > > + > > > +- renesas,companion : phandle to the companion LVDS encoder. This > > > property is > > > + mandatory for the first LVDS encoder on D3 and E3 SoCs, and shall > > > point to > > > + the second encoder to be used as a companion in dual-link mode. It > > > shall not > > > + be set for any other LVDS encoder. > > > > If the D3 and E3 socs do not mandate the use of dual-link, then what to > > do in the DT? Because according to the above this property must be > > specified for D3 and E3 SOC's. > > This property doesn't enable dual-link mode, it only specifies the > companion LVDS encoder used for dual-link mode, when enabled (through > communication between the LVDS encoder and the LVDS receiver at > runtime). > > Jacopo had a similar comment so I suppose this isn't clear. How would > you word it to make it clear ? Let me try: - renesas,companion : phandle to the companion LVDS encoder. This property is mandatory for the first LVDS encoder on D3 and E3 SoCs when dual-link mode is supported. The property shall pont to the phandle of the second encoder to be used as a companion in dual-link mode. It shall not be set for any other LVDS encoder. The main difference is "when dual-link mode is supported". As per my understanding this property is only relevant when the actual HW supports / uses dual-link mode. So for a board that do not even wire up dual-link, then setting the property would be confusing. I hope this better describes my understanding. Sam
Re: [PATCH v3 07/10] drm: rcar-du: Skip LVDS1 output on Gen3 when using dual-link LVDS mode
Hi Sam,
On Tue, May 28, 2019 at 06:42:13PM +0200, Sam Ravnborg wrote:
> On Tue, May 28, 2019 at 05:12:31PM +0300, Laurent Pinchart wrote:
> > In dual-link LVDS mode, the LVDS1 encoder is used as a companion for
> > LVDS0, and both encoders transmit data from DU0. The LVDS1 output of DU1
> > can't be used in that case, don't create an encoder and connector for
> > it.
> >
> > Signed-off-by: Laurent Pinchart
> > Reviewed-by: Jacopo Mondi
> > Tested-by: Jacopo Mondi
> > ---
> > Changess since v2:
> >
> > - Remove unneeded bridge NULL check
> > ---
> > drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 12
> > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
> > 2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > index 6c91753af7bc..0f00bdfe2366 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> > @@ -16,6 +16,7 @@
> > #include "rcar_du_drv.h"
> > #include "rcar_du_encoder.h"
> > #include "rcar_du_kms.h"
> > +#include "rcar_lvds.h"
> >
> > /*
> > -
> > * Encoder
> > @@ -97,6 +98,17 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
> > }
> > }
> >
> > + /*
> > +* On Gen3 skip the LVDS1 output if the LVDS1 encoder is used as a
> > +* companion for LVDS0 in dual-link mode.
> > +*/
> > + if (rcdu->info->gen >= 3 && output == RCAR_DU_OUTPUT_LVDS1) {
>
> Both subject and comment above says "On Gen3", but the code looks like
> it implements "On Gen3 or newer" - due to use of ">=".
> Looks wrong to me.
Gen3 is the newest generation :-) We thus use >= through the DU and LVDS
drivers to prepare for support of Gen4, just in case.
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 04/10] dt-bindings: display: renesas: lvds: Add renesas, companion property
Hi Sam, On Tue, May 28, 2019 at 06:37:30PM +0200, Sam Ravnborg wrote: > On Tue, May 28, 2019 at 05:12:28PM +0300, Laurent Pinchart wrote: > > Add a new optional renesas,companion property to point to the companion > > LVDS encoder. This is used to support dual-link operation where the main > > LVDS encoder splits even-numbered and odd-numbered pixels between the > > two LVDS encoders. > > > > The new property doesn't control the mode of operation, it only > > describes the relationship between the master and companion LVDS > > encoders. > > > > Signed-off-by: Laurent Pinchart > > Reviewed-by: Jacopo Mondi > > Tested-by: Jacopo Mondi > > --- > > Changes since v2: > > > > - Clarify when the companion property is required or not allowed > > > > Changes since v1: > > > > - Fixed typo > > --- > > .../devicetree/bindings/display/bridge/renesas,lvds.txt| 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > > b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > > index 900a884ad9f5..2d24bd8cbec5 100644 > > --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > > @@ -45,6 +45,13 @@ OF graph bindings specified in > > Documentation/devicetree/bindings/graph.txt. > > > > Each port shall have a single endpoint. > > > > +Optional properties: > > + > > +- renesas,companion : phandle to the companion LVDS encoder. This property > > is > > + mandatory for the first LVDS encoder on D3 and E3 SoCs, and shall point > > to > > + the second encoder to be used as a companion in dual-link mode. It shall > > not > > + be set for any other LVDS encoder. > > If the D3 and E3 socs do not mandate the use of dual-link, then what to > do in the DT? Because according to the above this property must be > specified for D3 and E3 SOC's. This property doesn't enable dual-link mode, it only specifies the companion LVDS encoder used for dual-link mode, when enabled (through communication between the LVDS encoder and the LVDS receiver at runtime). Jacopo had a similar comment so I suppose this isn't clear. How would you word it to make it clear ? > > + > > > > Example: > > Always good with examples, maybe it comes later. Good point, I'll fix that. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/omap: remove open-coded drm_invalid_op()
On 28/05/2019 18:41, Emil Velikov wrote:
On 2019/05/28, Tomi Valkeinen wrote:
On 22/05/2019 18:02, Emil Velikov wrote:
From: Emil Velikov
Cc: Tomi Valkeinen
Signed-off-by: Emil Velikov
---
drivers/gpu/drm/omapdrm/omap_drv.c | 16 +---
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
b/drivers/gpu/drm/omapdrm/omap_drv.c
index 1b9b6f5e48e1..672e0f8ad11c 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -439,20 +439,6 @@ static int ioctl_get_param(struct drm_device *dev, void
*data,
return 0;
}
-static int ioctl_set_param(struct drm_device *dev, void *data,
- struct drm_file *file_priv)
-{
- struct drm_omap_param *args = data;
-
- switch (args->param) {
- default:
- DBG("unknown parameter %lld", args->param);
- return -EINVAL;
- }
-
- return 0;
-}
-
#define OMAP_BO_USER_MASK0x00ff /* flags settable by userspace
*/
static int ioctl_gem_new(struct drm_device *dev, void *data,
@@ -492,7 +478,7 @@ static int ioctl_gem_info(struct drm_device *dev, void
*data,
static const struct drm_ioctl_desc ioctls[DRM_COMMAND_END -
DRM_COMMAND_BASE] = {
DRM_IOCTL_DEF_DRV(OMAP_GET_PARAM, ioctl_get_param,
DRM_AUTH | DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(OMAP_SET_PARAM, ioctl_set_param,
+ DRM_IOCTL_DEF_DRV(OMAP_SET_PARAM, drm_invalid_op,
DRM_AUTH | DRM_MASTER | DRM_ROOT_ONLY),
DRM_IOCTL_DEF_DRV(OMAP_GEM_NEW, ioctl_gem_new,
DRM_AUTH | DRM_RENDER_ALLOW),
Thanks! Do you want to take this via drm-misc too, or can I pick it up?
Hoping to pick this via drm-misc, albeit I forgot to mention earlier.
Sounds fine to me.
Acked-by: Tomi Valkeinen
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/05/28, Koenig, Christian wrote: > Am 28.05.19 um 18:10 schrieb Emil Velikov: > > On 2019/05/28, Daniel Vetter wrote: > >> On Tue, May 28, 2019 at 10:03 AM Koenig, Christian > >> wrote: > >>> Am 28.05.19 um 09:38 schrieb Daniel Vetter: > [SNIP] > > Might be a good idea looking into reverting it partially, so that at > > least command submission and buffer allocation is still blocked. > I thought the issue is a lot more than vainfo, it's pretty much every > hacked up compositor under the sun getting this wrong one way or > another. Thinking about this some more, I also have no idea how you'd > want to deprecate rendering on primary nodes in general. Apparently > that breaks -modesetting already, and probably lots more compositors. > And it looks like we're finally achieve the goal kms set out to 10 > years ago, and new compositors are sprouting up all the time. I guess > we could just break them all (on new hardware) and tell them to all > suck it up. But I don't think that's a great option. And just > deprecating this on amdgpu is going to be even harder, since then > everywhere else it'll keep working, and it's just amdgpu.ko that looks > broken. > > Aside: I'm not supporting Emil's idea here because it fixes any issues > Intel has - Intel doesn't care. I support it because reality sucks, > people get this render vs. primary vs. multi-gpu prime wrong all the > time (that's also why we have hardcoded display+gpu pairs in mesa for > the various soc combinations out there), and this looks like a > pragmatic solution. It'd be nice if every compositor and everything > else would perfectly support multi gpu and only use render nodes for > rendering, and only primary nodes for display. But reality is that > people hack on stuff until gears on screen and then move on to more > interesting things (to them). So I don't think we'll ever win this :-/ > >>> Yeah, but this is a classic case of working around user space issues by > >>> making kernel changes instead of fixing user space. > >>> > >>> Having privileged (output control) and unprivileged (rendering control) > >>> functionality behind the same node is a mistake we have made a long time > >>> ago and render nodes finally seemed to be a way to fix that. > >>> > >>> I mean why are compositors using the primary node in the first place? > >>> Because they want to have access to privileged resources I think and in > >>> this case it is perfectly ok to do so. > >>> > >>> Now extending unprivileged access to the primary node actually sounds > >>> like a step into the wrong direction to me. > >>> > >>> I rather think that we should go down the route of completely dropping > >>> command submission and buffer allocation through the primary node for > >>> non master clients. And then as next step at some point drop support for > >>> authentication/flink. > >>> > >>> I mean we have done this with UMS as well and I don't see much other way > >>> to move forward and get rid of those ancient interface in the long term. > >> Well kms had some really good benefits that drove quick adoption, like > >> "suspend/resume actually has a chance of working" or "comes with > >> buffer management so you can run multiple gears". > >> > >> The render node thing is a lot more niche use case (prime, better priv > >> separation), plus "it's cleaner design". And the "cleaner design" part > >> is something that empirically doesn't seem to matter :-/ Just two > >> examples: > >> - KHR_display/leases just iterated display resources on the fd needed > >> for rendering (and iirc there was even a patch to expose that for > >> render nodes too so it works with DRI3), because implementing > >> protocols is too hard. Barely managed to stop that one before it > >> happened. > >> - Various video players use the vblank ioctl on directly to schedule > >> frames, without telling the compositor. I discovered that when I > >> wanted to limite the vblank ioctl to master clients only. Again, > >> apparently too hard to use the existing extensions, or fix the bugs in > >> there, or whatever. One userspace got fixed last year, but it'll > >> probably get copypasted around forever :-/ > >> > >> So I don't think we'll ever manage to roll a clean split out, and best > >> we can do is give in and just hand userspace what it wants. As much as > >> that's misguided and unclean and all that. Maybe it'll result in a > >> least fewer stuff getting run as root to hack around this, because > >> fixing properly seems not to be on the table. > >> > >> The beauty of kms is that we've achieved the mission, everyone's > >> writing their own thing. Which is also terrible, and I don't think > >> it'll get better. > > > > With the risk of coming rude I will repeat my earlier comment: > > > > The problem is _neither_ Intel nor libva specific. > > > > > > > > That said, let's step back for a moment
Re: [PATCH v3 00/10] R-Car DU: LVDS dual-link mode support
Hi Laurent. Nice series with small and well described patches. > On Tue, May 28, 2019 at 05:12:24PM +0300, Laurent Pinchart wrote: >> Hello everybody, >> >> This patch series implements support for LVDS dual-link mode in the >> R-Car DU and R-Car LVDS encoder drivers, and well as in the thc63lvd1024 >> LVDS decoder driver. Patches looks good. With my few comments addressed: Acked-by: Sam Ravnborg (Do not feel too confident in all the stuff, r-b seems to give me too much credit for spending less than half an hour reading the patches). Sam
Re: [PATCH v3 07/10] drm: rcar-du: Skip LVDS1 output on Gen3 when using dual-link LVDS mode
Hi Laurent.
On Tue, May 28, 2019 at 05:12:31PM +0300, Laurent Pinchart wrote:
> In dual-link LVDS mode, the LVDS1 encoder is used as a companion for
> LVDS0, and both encoders transmit data from DU0. The LVDS1 output of DU1
> can't be used in that case, don't create an encoder and connector for
> it.
>
> Signed-off-by: Laurent Pinchart
> Reviewed-by: Jacopo Mondi
> Tested-by: Jacopo Mondi
> ---
> Changess since v2:
>
> - Remove unneeded bridge NULL check
> ---
> drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 12
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> index 6c91753af7bc..0f00bdfe2366 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> @@ -16,6 +16,7 @@
> #include "rcar_du_drv.h"
> #include "rcar_du_encoder.h"
> #include "rcar_du_kms.h"
> +#include "rcar_lvds.h"
>
> /*
> -
> * Encoder
> @@ -97,6 +98,17 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
> }
> }
>
> + /*
> + * On Gen3 skip the LVDS1 output if the LVDS1 encoder is used as a
> + * companion for LVDS0 in dual-link mode.
> + */
> + if (rcdu->info->gen >= 3 && output == RCAR_DU_OUTPUT_LVDS1) {
Both subject and comment above says "On Gen3", but the code looks like
it implements "On Gen3 or newer" - due to use of ">=".
Looks wrong to me.
Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 04/10] dt-bindings: display: renesas: lvds: Add renesas, companion property
Hi Laurent. Reading through this nice series. On Tue, May 28, 2019 at 05:12:28PM +0300, Laurent Pinchart wrote: > Add a new optional renesas,companion property to point to the companion > LVDS encoder. This is used to support dual-link operation where the main > LVDS encoder splits even-numbered and odd-numbered pixels between the > two LVDS encoders. > > The new property doesn't control the mode of operation, it only > describes the relationship between the master and companion LVDS > encoders. > > Signed-off-by: Laurent Pinchart > Reviewed-by: Jacopo Mondi > Tested-by: Jacopo Mondi > --- > Changes since v2: > > - Clarify when the companion property is required or not allowed > > Changes since v1: > > - Fixed typo > --- > .../devicetree/bindings/display/bridge/renesas,lvds.txt| 7 +++ > 1 file changed, 7 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > index 900a884ad9f5..2d24bd8cbec5 100644 > --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > @@ -45,6 +45,13 @@ OF graph bindings specified in > Documentation/devicetree/bindings/graph.txt. > > Each port shall have a single endpoint. > > +Optional properties: > + > +- renesas,companion : phandle to the companion LVDS encoder. This property is > + mandatory for the first LVDS encoder on D3 and E3 SoCs, and shall point to > + the second encoder to be used as a companion in dual-link mode. It shall > not > + be set for any other LVDS encoder. If the D3 and E3 socs do not mandate the use of dual-link, then what to do in the DT? Because according to the above this property must be specified for D3 and E3 SOC's. > + > > Example: Always good with examples, maybe it comes later. Sam
Re: [RFC PATCH] drm/ttm, drm/vmwgfx: Have TTM support AMD SEV encryption
Am 28.05.19 um 18:27 schrieb Thomas Hellstrom: > On Tue, 2019-05-28 at 15:50 +, Lendacky, Thomas wrote: >> On 5/28/19 10:17 AM, Koenig, Christian wrote: >>> Hi Thomas, >>> >>> Am 28.05.19 um 17:11 schrieb Thomas Hellstrom: Hi, Tom, Thanks for the reply. The question is not graphics specific, but lies in your answer further below: On 5/28/19 4:48 PM, Lendacky, Thomas wrote: > On 5/28/19 2:31 AM, Thomas Hellstrom wrote: > [SNIP] > As for kernel vmaps and user-maps, those pages will be marked > encrypted > (unless explicitly made un-encrypted by calling > set_memory_decrypted()). > But, if you are copying to/from those areas into the un- > encrypted DMA > area then everything will be ok. The question is regarding the above paragraph. AFAICT, set_memory_decrypted() only changes the fixed kernel map PTEs. But when setting up other aliased PTEs to the exact same decrypted pages, for example using dma_mmap_coherent(), kmap_atomic_prot(), vmap() etc. What code is responsible for clearing the encrypted flag on those PTEs? Is there something in the x86 platform code doing that? >>> Tom actually explained this: The encryption bit is bit-47 of a physical address. >>> In other words set_memory_decrypted() changes the physical address >>> in >>> the PTEs of the kernel mapping and all other use cases just copy >>> that >>> from there. >> Except I don't think the PTE attributes are copied from the kernel >> mapping > +1! > >> in some cases. For example, dma_mmap_coherent() will create the same >> vm_page_prot value regardless of whether or not the underlying memory >> is >> encrypted or not. But kmap_atomic_prot() will return the kernel >> virtual >> address of the page, so that would be fine. > Yes, on 64-bit systems. On 32-bit systems (do they exist with SEV?), > they don't. I don't think so, but feel free to prove me wrong Tom. > And similarly TTM user-space mappings and vmap() doesn't copy from the > kernel map either, so I think we actually do need to modify the page- > prot like done in the patch. Well the problem is that this won't have any effect. As Tom explained encryption is not implemented as a page protection bit, but rather as part of the physical address of the part. I have no idea how that is actually handled thought, Christian. > > /Thomas > >> This is an area that needs looking into to be sure it is working >> properly >> with SME and SEV. >> >> Thanks, >> Tom >> >>> That's rather nifty, because this way everybody will either use or >>> not >>> use encryption on the page. >>> >>> Christian. >>> Thanks, Thomas > Things get fuzzy for me when it comes to the GPU access of the > memory > and what and how it is accessed. > > Thanks, > Tom ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm/ttm, drm/vmwgfx: Have TTM support AMD SEV encryption
On Tue, 2019-05-28 at 15:50 +, Lendacky, Thomas wrote: > On 5/28/19 10:17 AM, Koenig, Christian wrote: > > Hi Thomas, > > > > Am 28.05.19 um 17:11 schrieb Thomas Hellstrom: > > > Hi, Tom, > > > > > > Thanks for the reply. The question is not graphics specific, but > > > lies > > > in your answer further below: > > > > > > On 5/28/19 4:48 PM, Lendacky, Thomas wrote: > > > > On 5/28/19 2:31 AM, Thomas Hellstrom wrote: > > > > [SNIP] > > > > As for kernel vmaps and user-maps, those pages will be marked > > > > encrypted > > > > (unless explicitly made un-encrypted by calling > > > > set_memory_decrypted()). > > > > But, if you are copying to/from those areas into the un- > > > > encrypted DMA > > > > area then everything will be ok. > > > > > > The question is regarding the above paragraph. > > > > > > AFAICT, set_memory_decrypted() only changes the fixed kernel map > > > PTEs. > > > But when setting up other aliased PTEs to the exact same > > > decrypted > > > pages, for example using dma_mmap_coherent(), > > > kmap_atomic_prot(), > > > vmap() etc. What code is responsible for clearing the encrypted > > > flag > > > on those PTEs? Is there something in the x86 platform code doing > > > that? > > > > Tom actually explained this: > > > The encryption bit is bit-47 of a physical address. > > > > In other words set_memory_decrypted() changes the physical address > > in > > the PTEs of the kernel mapping and all other use cases just copy > > that > > from there. > > Except I don't think the PTE attributes are copied from the kernel > mapping +1! > in some cases. For example, dma_mmap_coherent() will create the same > vm_page_prot value regardless of whether or not the underlying memory > is > encrypted or not. But kmap_atomic_prot() will return the kernel > virtual > address of the page, so that would be fine. Yes, on 64-bit systems. On 32-bit systems (do they exist with SEV?), they don't. And similarly TTM user-space mappings and vmap() doesn't copy from the kernel map either, so I think we actually do need to modify the page- prot like done in the patch. /Thomas > > This is an area that needs looking into to be sure it is working > properly > with SME and SEV. > > Thanks, > Tom > > > That's rather nifty, because this way everybody will either use or > > not > > use encryption on the page. > > > > Christian. > > > > > Thanks, > > > Thomas > > > > > > > > > > Things get fuzzy for me when it comes to the GPU access of the > > > > memory > > > > and what and how it is accessed. > > > > > > > > Thanks, > > > > Tom ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 08/10] drm/amdgpu: drop some validation failure messages
The messages about amdgpu_cs_list_validate are duplicated because the
caller will complain into the logs as well and we can also get
interrupted by a signal here.
Also fix the the caller to not report -EAGAIN from validation.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index fff558cf385b..20f2955d2a55 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -671,16 +671,12 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser
*p,
}
r = amdgpu_cs_list_validate(p, &duplicates);
- if (r) {
- DRM_ERROR("amdgpu_cs_list_validate(duplicates) failed.\n");
+ if (r)
goto error_validate;
- }
r = amdgpu_cs_list_validate(p, &p->validated);
- if (r) {
- DRM_ERROR("amdgpu_cs_list_validate(validated) failed.\n");
+ if (r)
goto error_validate;
- }
amdgpu_cs_report_moved_bytes(p->adev, p->bytes_moved,
p->bytes_moved_vis);
@@ -1383,7 +1379,7 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
if (r) {
if (r == -ENOMEM)
DRM_ERROR("Not enough memory for command
submission!\n");
- else if (r != -ERESTARTSYS)
+ else if (r != -ERESTARTSYS && r != -EAGAIN)
DRM_ERROR("Failed to process the buffer list %d!\n", r);
goto out;
}
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/10] drm/amd/display: use ttm_eu_reserve_buffers instead of amdgpu_bo_reserve v2
From: Chunming Zhou
add ticket for display bo, so that it can preempt busy bo.
v2: fix stupid rebase error
Change-Id: I9f031cdcc8267de00e819ae303baa0a52df8ebb9
Signed-off-by: Chunming Zhou
Reviewed-by: Christian König
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 21 ++-
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 4a1755bce96c..56f320f3fd72 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4182,6 +4182,9 @@ static int dm_plane_helper_prepare_fb(struct drm_plane
*plane,
struct amdgpu_device *adev;
struct amdgpu_bo *rbo;
struct dm_plane_state *dm_plane_state_new, *dm_plane_state_old;
+ struct list_head list;
+ struct ttm_validate_buffer tv;
+ struct ww_acquire_ctx ticket;
uint64_t tiling_flags;
uint32_t domain;
int r;
@@ -4198,9 +4201,17 @@ static int dm_plane_helper_prepare_fb(struct drm_plane
*plane,
obj = new_state->fb->obj[0];
rbo = gem_to_amdgpu_bo(obj);
adev = amdgpu_ttm_adev(rbo->tbo.bdev);
- r = amdgpu_bo_reserve(rbo, false);
- if (unlikely(r != 0))
+ INIT_LIST_HEAD(&list);
+
+ tv.bo = &rbo->tbo;
+ tv.num_shared = 1;
+ list_add(&tv.head, &list);
+
+ r = ttm_eu_reserve_buffers(&ticket, &list, false, NULL, true);
+ if (r) {
+ dev_err(adev->dev, "fail to reserve bo (%d)\n", r);
return r;
+ }
if (plane->type != DRM_PLANE_TYPE_CURSOR)
domain = amdgpu_display_supported_domains(adev);
@@ -4211,21 +4222,21 @@ static int dm_plane_helper_prepare_fb(struct drm_plane
*plane,
if (unlikely(r != 0)) {
if (r != -ERESTARTSYS)
DRM_ERROR("Failed to pin framebuffer with error %d\n",
r);
- amdgpu_bo_unreserve(rbo);
+ ttm_eu_backoff_reservation(&ticket, &list);
return r;
}
r = amdgpu_ttm_alloc_gart(&rbo->tbo);
if (unlikely(r != 0)) {
amdgpu_bo_unpin(rbo);
- amdgpu_bo_unreserve(rbo);
+ ttm_eu_backoff_reservation(&ticket, &list);
DRM_ERROR("%p bind failed\n", rbo);
return r;
}
amdgpu_bo_get_tiling_flags(rbo, &tiling_flags);
- amdgpu_bo_unreserve(rbo);
+ ttm_eu_backoff_reservation(&ticket, &list);
afb->address = amdgpu_bo_gpu_offset(rbo);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 09/10] drm/amdgpu: create GDS, GWS and OA in system domain
And only move them in on validation. This allows for better control when multiple processes are fighting over those resources. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 93b2c5a48a71..30493429851e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -495,7 +495,11 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, #endif bo->tbo.bdev = &adev->mman.bdev; - amdgpu_bo_placement_from_domain(bo, bp->domain); + if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA | + AMDGPU_GEM_DOMAIN_GDS)) + amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU); + else + amdgpu_bo_placement_from_domain(bo, bp->domain); if (bp->type == ttm_bo_type_kernel) bo->tbo.priority = 1; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 06/10] drm/ttm: fix busy memory to fail other user v10
BOs on the LRU might be blocked during command submission
and cause OOM situations.
Avoid this by blocking for the first busy BO not locked by
the same ticket as the BO we are searching space for.
v10: completely start over with the patch since we didn't
handled a whole bunch of corner cases.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c | 77 ++--
1 file changed, 66 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 3bc31da1df67..98aa90056807 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -774,32 +774,72 @@ EXPORT_SYMBOL(ttm_bo_eviction_valuable);
* b. Otherwise, trylock it.
*/
static bool ttm_bo_evict_swapout_allowable(struct ttm_buffer_object *bo,
- struct ttm_operation_ctx *ctx, bool *locked)
+ struct ttm_operation_ctx *ctx, bool *locked, bool *busy)
{
bool ret = false;
- *locked = false;
if (bo->resv == ctx->resv) {
reservation_object_assert_held(bo->resv);
if (ctx->flags & TTM_OPT_FLAG_ALLOW_RES_EVICT
|| !list_empty(&bo->ddestroy))
ret = true;
+ *locked = false;
+ if (busy)
+ *busy = false;
} else {
- *locked = reservation_object_trylock(bo->resv);
- ret = *locked;
+ ret = reservation_object_trylock(bo->resv);
+ *locked = ret;
+ if (busy)
+ *busy = !ret;
}
return ret;
}
+/**
+ * ttm_mem_evict_wait_busy - wait for a busy BO to become available
+ *
+ * @busy_bo: BO which couldn't be locked with trylock
+ * @ctx: operation context
+ * @ticket: acquire ticket
+ *
+ * Try to lock a busy buffer object to avoid failing eviction.
+ */
+static int ttm_mem_evict_wait_busy(struct ttm_buffer_object *busy_bo,
+ struct ttm_operation_ctx *ctx,
+ struct ww_acquire_ctx *ticket)
+{
+ int r;
+
+ if (!busy_bo || !ticket)
+ return -EBUSY;
+
+ if (ctx->interruptible)
+ r = reservation_object_lock_interruptible(busy_bo->resv,
+ ticket);
+ else
+ r = reservation_object_lock(busy_bo->resv, ticket);
+
+ /*
+* TODO: It would be better to keep the BO locked until allocation is at
+* least tried one more time, but that would mean a much larger rework
+* of TTM.
+*/
+ if (!r)
+ reservation_object_unlock(busy_bo->resv);
+
+ return r == -EDEADLK ? -EAGAIN : r;
+}
+
static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
uint32_t mem_type,
const struct ttm_place *place,
- struct ttm_operation_ctx *ctx)
+ struct ttm_operation_ctx *ctx,
+ struct ww_acquire_ctx *ticket)
{
+ struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
struct ttm_bo_global *glob = bdev->glob;
struct ttm_mem_type_manager *man = &bdev->man[mem_type];
- struct ttm_buffer_object *bo = NULL;
bool locked = false;
unsigned i;
int ret;
@@ -807,8 +847,15 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
spin_lock(&glob->lru_lock);
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
list_for_each_entry(bo, &man->lru[i], lru) {
- if (!ttm_bo_evict_swapout_allowable(bo, ctx, &locked))
+ bool busy;
+
+ if (!ttm_bo_evict_swapout_allowable(bo, ctx, &locked,
+ &busy)) {
+ if (busy && !busy_bo &&
+ bo->resv->lock.ctx != ticket)
+ busy_bo = bo;
continue;
+ }
if (place && !bdev->driver->eviction_valuable(bo,
place)) {
@@ -827,8 +874,13 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
}
if (!bo) {
+ if (busy_bo)
+ ttm_bo_get(busy_bo);
spin_unlock(&glob->lru_lock);
- return -EBUSY;
+ ret = ttm_mem_evict_wait_busy(busy_bo, ctx, ticket);
+ if (busy_bo)
+ ttm_bo_put(busy_bo);
+ return ret;
}
kref_get(&bo->list_kref);
@@ -914,7 +966,8 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object
*bo,
return ret;
if (mem->mm_node)
[PATCH 05/10] drm/ttm: immediately move BOs to the new LRU v2
Move BOs which are currently in a lower domain to the new
LRU before allocating backing space while validating.
This makes sure that we always have enough entries on the
LRU to allow for other processes to wait for an operation
to complete.
v2: generalize the test
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c | 43 +++-
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 0dbb23b0dedd..3bc31da1df67 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -166,20 +166,22 @@ static void ttm_bo_release_list(struct kref *list_kref)
ttm_mem_global_free(bdev->glob->mem_glob, acc_size);
}
-void ttm_bo_add_to_lru(struct ttm_buffer_object *bo)
+static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object *bo,
+ struct ttm_mem_reg *mem)
{
struct ttm_bo_device *bdev = bo->bdev;
struct ttm_mem_type_manager *man;
reservation_object_assert_held(bo->resv);
+ BUG_ON(!list_empty(&bo->lru));
if (!list_empty(&bo->lru))
return;
- if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT)
+ if (mem->placement & TTM_PL_FLAG_NO_EVICT)
return;
- man = &bdev->man[bo->mem.mem_type];
+ man = &bdev->man[mem->mem_type];
list_add_tail(&bo->lru, &man->lru[bo->priority]);
kref_get(&bo->list_kref);
@@ -189,6 +191,11 @@ void ttm_bo_add_to_lru(struct ttm_buffer_object *bo)
kref_get(&bo->list_kref);
}
}
+
+void ttm_bo_add_to_lru(struct ttm_buffer_object *bo)
+{
+ ttm_bo_add_mem_to_lru(bo, &bo->mem);
+}
EXPORT_SYMBOL(ttm_bo_add_to_lru);
static void ttm_bo_ref_bug(struct kref *list_kref)
@@ -1001,6 +1008,14 @@ static int ttm_bo_mem_placement(struct ttm_buffer_object
*bo,
mem->mem_type = mem_type;
mem->placement = cur_flags;
+
+ if (bo->mem.mem_type < mem_type && !list_empty(&bo->lru)) {
+ spin_lock(&bo->bdev->glob->lru_lock);
+ ttm_bo_del_from_lru(bo);
+ ttm_bo_add_mem_to_lru(bo, mem);
+ spin_unlock(&bo->bdev->glob->lru_lock);
+ }
+
return 0;
}
@@ -1034,7 +1049,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
if (ret == -EBUSY)
continue;
if (ret)
- return ret;
+ goto error;
type_found = true;
mem->mm_node = NULL;
@@ -1044,13 +1059,13 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
man = &bdev->man[mem->mem_type];
ret = (*man->func->get_node)(man, bo, place, mem);
if (unlikely(ret))
- return ret;
+ goto error;
if (mem->mm_node) {
ret = ttm_bo_add_move_fence(bo, man, mem);
if (unlikely(ret)) {
(*man->func->put_node)(man, mem);
- return ret;
+ goto error;
}
return 0;
}
@@ -1063,7 +1078,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
if (ret == -EBUSY)
continue;
if (ret)
- return ret;
+ goto error;
type_found = true;
mem->mm_node = NULL;
@@ -1075,15 +1090,23 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
return 0;
if (ret && ret != -EBUSY)
- return ret;
+ goto error;
}
+ ret = -ENOMEM;
if (!type_found) {
pr_err(TTM_PFX "No compatible memory type found\n");
- return -EINVAL;
+ ret = -EINVAL;
}
- return -ENOMEM;
+error:
+ if (bo->mem.mem_type == TTM_PL_SYSTEM && !list_empty(&bo->lru)) {
+ spin_lock(&bo->bdev->glob->lru_lock);
+ ttm_bo_move_to_lru_tail(bo, NULL);
+ spin_unlock(&bo->bdev->glob->lru_lock);
+ }
+
+ return ret;
}
EXPORT_SYMBOL(ttm_bo_mem_space);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/10] drm/amdgpu: stop removing BOs from the LRU v3
This avoids OOM situations when we have lots of threads
submitting at the same time.
v3: apply this to the whole driver, not just CS
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c| 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c| 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 20f2955d2a55..3e2da24cd17a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -648,7 +648,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
}
r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true,
- &duplicates, true);
+ &duplicates, false);
if (unlikely(r != 0)) {
if (r != -ERESTARTSYS)
DRM_ERROR("ttm_eu_reserve_buffers failed.\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
index 06f83cac0d3a..f660628e6af9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
@@ -79,7 +79,7 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
list_add(&csa_tv.head, &list);
amdgpu_vm_get_pd_bo(vm, &list, &pd);
- r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, true);
+ r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, false);
if (r) {
DRM_ERROR("failed to reserve CSA,PD BOs: err=%d\n", r);
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index d513a5ad03dd..ed25a4e14404 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -171,7 +171,7 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
amdgpu_vm_get_pd_bo(vm, &list, &vm_pd);
- r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates, true);
+ r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates, false);
if (r) {
dev_err(adev->dev, "leaking bo va because "
"we fail to reserve bo (%d)\n", r);
@@ -608,7 +608,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
amdgpu_vm_get_pd_bo(&fpriv->vm, &list, &vm_pd);
- r = ttm_eu_reserve_buffers(&ticket, &list, true, &duplicates, true);
+ r = ttm_eu_reserve_buffers(&ticket, &list, true, &duplicates, false);
if (r)
goto error_unref;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index c430e8259038..d60593cc436e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -155,7 +155,7 @@ static inline int amdgpu_bo_reserve(struct amdgpu_bo *bo,
bool no_intr)
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
int r;
- r = ttm_bo_reserve(&bo->tbo, !no_intr, false, NULL);
+ r = __ttm_bo_reserve(&bo->tbo, !no_intr, false, NULL);
if (unlikely(r != 0)) {
if (r != -ERESTARTSYS)
dev_err(adev->dev, "%p reserve failed\n", bo);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 01/10] drm/ttm: Make LRU removal optional v2
We are already doing this for DMA-buf imports and also for
amdgpu VM BOs for quite a while now.
If this doesn't run into any problems we are probably going
to stop removing BOs from the LRU altogether.
v2: drop BUG_ON from ttm_bo_add_to_lru
Signed-off-by: Christian König
---
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 9
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 ++--
drivers/gpu/drm/qxl/qxl_release.c | 2 +-
drivers/gpu/drm/radeon/radeon_gem.c | 2 +-
drivers/gpu/drm/radeon/radeon_object.c| 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 23 ++-
drivers/gpu/drm/ttm/ttm_execbuf_util.c| 20 +---
drivers/gpu/drm/virtio/virtgpu_ioctl.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 3 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_validation.h| 2 +-
include/drm/ttm/ttm_bo_driver.h | 5 +++-
include/drm/ttm/ttm_execbuf_util.h| 3 ++-
14 files changed, 46 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index e1cae4a37113..647e18f9e136 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -574,7 +574,7 @@ static int reserve_bo_and_vm(struct kgd_mem *mem,
amdgpu_vm_get_pd_bo(vm, &ctx->list, &ctx->vm_pd[0]);
ret = ttm_eu_reserve_buffers(&ctx->ticket, &ctx->list,
-false, &ctx->duplicates);
+false, &ctx->duplicates, true);
if (!ret)
ctx->reserved = true;
else {
@@ -647,7 +647,7 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem,
}
ret = ttm_eu_reserve_buffers(&ctx->ticket, &ctx->list,
-false, &ctx->duplicates);
+false, &ctx->duplicates, true);
if (!ret)
ctx->reserved = true;
else
@@ -1800,7 +1800,8 @@ static int validate_invalid_user_pages(struct
amdkfd_process_info *process_info)
}
/* Reserve all BOs and page tables for validation */
- ret = ttm_eu_reserve_buffers(&ticket, &resv_list, false, &duplicates);
+ ret = ttm_eu_reserve_buffers(&ticket, &resv_list, false, &duplicates,
+true);
WARN(!list_empty(&duplicates), "Duplicates should be empty");
if (ret)
goto out_free;
@@ -2006,7 +2007,7 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info,
struct dma_fence **ef)
}
ret = ttm_eu_reserve_buffers(&ctx.ticket, &ctx.list,
-false, &duplicate_save);
+false, &duplicate_save, true);
if (ret) {
pr_debug("Memory eviction: TTM Reserve Failed. Try again\n");
goto ttm_reserve_fail;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index d72cc583ebd1..fff558cf385b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -648,7 +648,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
}
r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true,
- &duplicates);
+ &duplicates, true);
if (unlikely(r != 0)) {
if (r != -ERESTARTSYS)
DRM_ERROR("ttm_eu_reserve_buffers failed.\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
index 54dd02a898b9..06f83cac0d3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
@@ -79,7 +79,7 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
list_add(&csa_tv.head, &list);
amdgpu_vm_get_pd_bo(vm, &list, &pd);
- r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL);
+ r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, true);
if (r) {
DRM_ERROR("failed to reserve CSA,PD BOs: err=%d\n", r);
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 7b840367004c..d513a5ad03dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -171,7 +171,7 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
amdgpu_vm_get_pd_bo(vm, &list, &vm_pd);
- r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates);
+ r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates, true);
if (r) {
dev_err(adev->dev, "leaking bo va because "
"we
[PATCH 04/10] drm/ttm: cleanup ttm_bo_mem_space
We tried this once before, but that turned out to be more
complicated than thought. With all the right prerequisites
it looks like we can do this now.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c | 127 ++-
1 file changed, 66 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 702cd89adbf9..0dbb23b0dedd 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -893,13 +893,12 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object
*bo,
* space, or we've evicted everything and there isn't enough space.
*/
static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
- uint32_t mem_type,
- const struct ttm_place *place,
- struct ttm_mem_reg *mem,
- struct ttm_operation_ctx *ctx)
+ const struct ttm_place *place,
+ struct ttm_mem_reg *mem,
+ struct ttm_operation_ctx *ctx)
{
struct ttm_bo_device *bdev = bo->bdev;
- struct ttm_mem_type_manager *man = &bdev->man[mem_type];
+ struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
int ret;
do {
@@ -908,11 +907,11 @@ static int ttm_bo_mem_force_space(struct
ttm_buffer_object *bo,
return ret;
if (mem->mm_node)
break;
- ret = ttm_mem_evict_first(bdev, mem_type, place, ctx);
+ ret = ttm_mem_evict_first(bdev, mem->mem_type, place, ctx);
if (unlikely(ret != 0))
return ret;
} while (1);
- mem->mem_type = mem_type;
+
return ttm_bo_add_move_fence(bo, man, mem);
}
@@ -960,6 +959,51 @@ static bool ttm_bo_mt_compatible(struct
ttm_mem_type_manager *man,
return true;
}
+/**
+ * ttm_bo_mem_placement - check if placement is compatible
+ * @bo: BO to find memory for
+ * @place: where to search
+ * @mem: the memory object to fill in
+ * @ctx: operation context
+ *
+ * Check if placement is compatible and fill in mem structure.
+ * Returns -EBUSY if placement won't work or negative error code.
+ * 0 when placement can be used.
+ */
+static int ttm_bo_mem_placement(struct ttm_buffer_object *bo,
+ const struct ttm_place *place,
+ struct ttm_mem_reg *mem,
+ struct ttm_operation_ctx *ctx)
+{
+ struct ttm_bo_device *bdev = bo->bdev;
+ uint32_t mem_type = TTM_PL_SYSTEM;
+ struct ttm_mem_type_manager *man;
+ uint32_t cur_flags = 0;
+ int ret;
+
+ ret = ttm_mem_type_from_place(place, &mem_type);
+ if (ret)
+ return ret;
+
+ man = &bdev->man[mem_type];
+ if (!man->has_type || !man->use_type)
+ return -EBUSY;
+
+ if (!ttm_bo_mt_compatible(man, mem_type, place, &cur_flags))
+ return -EBUSY;
+
+ cur_flags = ttm_bo_select_caching(man, bo->mem.placement, cur_flags);
+ /*
+* Use the access and other non-mapping-related flag bits from
+* the memory placement flags to the current flags
+*/
+ ttm_flag_masked(&cur_flags, place->flags, ~TTM_PL_MASK_MEMTYPE);
+
+ mem->mem_type = mem_type;
+ mem->placement = cur_flags;
+ return 0;
+}
+
/**
* Creates space for memory region @mem according to its type.
*
@@ -974,11 +1018,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
struct ttm_operation_ctx *ctx)
{
struct ttm_bo_device *bdev = bo->bdev;
- struct ttm_mem_type_manager *man;
- uint32_t mem_type = TTM_PL_SYSTEM;
- uint32_t cur_flags = 0;
bool type_found = false;
- bool type_ok = false;
int i, ret;
ret = reservation_object_reserve_shared(bo->resv, 1);
@@ -988,37 +1028,20 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
mem->mm_node = NULL;
for (i = 0; i < placement->num_placement; ++i) {
const struct ttm_place *place = &placement->placement[i];
+ struct ttm_mem_type_manager *man;
- ret = ttm_mem_type_from_place(place, &mem_type);
+ ret = ttm_bo_mem_placement(bo, place, mem, ctx);
+ if (ret == -EBUSY)
+ continue;
if (ret)
return ret;
- man = &bdev->man[mem_type];
- if (!man->has_type || !man->use_type)
- continue;
-
- type_ok = ttm_bo_mt_compatible(man, mem_type, place,
- &cur_flags);
-
- if (!type_ok)
- continue;
type_found = true;
- cur_fl
[PATCH 03/10] drm/ttm: remove manual placement preference
If drivers don't prefer a system memory placement
they should not but it into the placement list first.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 7b59e5ecde7f..702cd89adbf9 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1012,8 +1012,12 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
ttm_flag_masked(&cur_flags, place->flags,
~TTM_PL_MASK_MEMTYPE);
- if (mem_type == TTM_PL_SYSTEM)
- break;
+ if (mem_type == TTM_PL_SYSTEM) {
+ mem->mem_type = mem_type;
+ mem->placement = cur_flags;
+ mem->mm_node = NULL;
+ return 0;
+ }
ret = (*man->func->get_node)(man, bo, place, mem);
if (unlikely(ret))
@@ -1025,16 +1029,12 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
(*man->func->put_node)(man, mem);
return ret;
}
- break;
+ mem->mem_type = mem_type;
+ mem->placement = cur_flags;
+ return 0;
}
}
- if ((type_ok && (mem_type == TTM_PL_SYSTEM)) || mem->mm_node) {
- mem->mem_type = mem_type;
- mem->placement = cur_flags;
- return 0;
- }
-
for (i = 0; i < placement->num_busy_placement; ++i) {
const struct ttm_place *place = &placement->busy_placement[i];
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 02/10] drm/ttm: return immediately in case of a signal
When a signal arrives we should return immediately for
handling it and not try other placements first.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 06bbcd2679b2..7b59e5ecde7f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -979,7 +979,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
uint32_t cur_flags = 0;
bool type_found = false;
bool type_ok = false;
- bool has_erestartsys = false;
int i, ret;
ret = reservation_object_reserve_shared(bo->resv, 1);
@@ -1070,8 +1069,8 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
mem->placement = cur_flags;
return 0;
}
- if (ret == -ERESTARTSYS)
- has_erestartsys = true;
+ if (ret && ret != -EBUSY)
+ return ret;
}
if (!type_found) {
@@ -1079,7 +1078,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
return -EINVAL;
}
- return (has_erestartsys) ? -ERESTARTSYS : -ENOMEM;
+ return -ENOMEM;
}
EXPORT_SYMBOL(ttm_bo_mem_space);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
Am 28.05.19 um 18:10 schrieb Emil Velikov: > On 2019/05/28, Daniel Vetter wrote: >> On Tue, May 28, 2019 at 10:03 AM Koenig, Christian >> wrote: >>> Am 28.05.19 um 09:38 schrieb Daniel Vetter: [SNIP] > Might be a good idea looking into reverting it partially, so that at > least command submission and buffer allocation is still blocked. I thought the issue is a lot more than vainfo, it's pretty much every hacked up compositor under the sun getting this wrong one way or another. Thinking about this some more, I also have no idea how you'd want to deprecate rendering on primary nodes in general. Apparently that breaks -modesetting already, and probably lots more compositors. And it looks like we're finally achieve the goal kms set out to 10 years ago, and new compositors are sprouting up all the time. I guess we could just break them all (on new hardware) and tell them to all suck it up. But I don't think that's a great option. And just deprecating this on amdgpu is going to be even harder, since then everywhere else it'll keep working, and it's just amdgpu.ko that looks broken. Aside: I'm not supporting Emil's idea here because it fixes any issues Intel has - Intel doesn't care. I support it because reality sucks, people get this render vs. primary vs. multi-gpu prime wrong all the time (that's also why we have hardcoded display+gpu pairs in mesa for the various soc combinations out there), and this looks like a pragmatic solution. It'd be nice if every compositor and everything else would perfectly support multi gpu and only use render nodes for rendering, and only primary nodes for display. But reality is that people hack on stuff until gears on screen and then move on to more interesting things (to them). So I don't think we'll ever win this :-/ >>> Yeah, but this is a classic case of working around user space issues by >>> making kernel changes instead of fixing user space. >>> >>> Having privileged (output control) and unprivileged (rendering control) >>> functionality behind the same node is a mistake we have made a long time >>> ago and render nodes finally seemed to be a way to fix that. >>> >>> I mean why are compositors using the primary node in the first place? >>> Because they want to have access to privileged resources I think and in >>> this case it is perfectly ok to do so. >>> >>> Now extending unprivileged access to the primary node actually sounds >>> like a step into the wrong direction to me. >>> >>> I rather think that we should go down the route of completely dropping >>> command submission and buffer allocation through the primary node for >>> non master clients. And then as next step at some point drop support for >>> authentication/flink. >>> >>> I mean we have done this with UMS as well and I don't see much other way >>> to move forward and get rid of those ancient interface in the long term. >> Well kms had some really good benefits that drove quick adoption, like >> "suspend/resume actually has a chance of working" or "comes with >> buffer management so you can run multiple gears". >> >> The render node thing is a lot more niche use case (prime, better priv >> separation), plus "it's cleaner design". And the "cleaner design" part >> is something that empirically doesn't seem to matter :-/ Just two >> examples: >> - KHR_display/leases just iterated display resources on the fd needed >> for rendering (and iirc there was even a patch to expose that for >> render nodes too so it works with DRI3), because implementing >> protocols is too hard. Barely managed to stop that one before it >> happened. >> - Various video players use the vblank ioctl on directly to schedule >> frames, without telling the compositor. I discovered that when I >> wanted to limite the vblank ioctl to master clients only. Again, >> apparently too hard to use the existing extensions, or fix the bugs in >> there, or whatever. One userspace got fixed last year, but it'll >> probably get copypasted around forever :-/ >> >> So I don't think we'll ever manage to roll a clean split out, and best >> we can do is give in and just hand userspace what it wants. As much as >> that's misguided and unclean and all that. Maybe it'll result in a >> least fewer stuff getting run as root to hack around this, because >> fixing properly seems not to be on the table. >> >> The beauty of kms is that we've achieved the mission, everyone's >> writing their own thing. Which is also terrible, and I don't think >> it'll get better. > > With the risk of coming rude I will repeat my earlier comment: > > The problem is _neither_ Intel nor libva specific. > > > > That said, let's step back for a moment and consider: > > - the "block everything but KMS via the primary node" idea is great but > orthogonal > > - the series does address issues that are vendor-agnostic > > - by default this series does _not_ cause a
[Bug 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781 --- Comment #2 from Richard Thier --- The "gallium/winsys/radeon/drm/radeon_drm_bo.c" is indeed used by r300 because I have put a log in it. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/05/28, Daniel Vetter wrote: > On Tue, May 28, 2019 at 10:03 AM Koenig, Christian > wrote: > > > > Am 28.05.19 um 09:38 schrieb Daniel Vetter: > > > [SNIP] > > >> Might be a good idea looking into reverting it partially, so that at > > >> least command submission and buffer allocation is still blocked. > > > I thought the issue is a lot more than vainfo, it's pretty much every > > > hacked up compositor under the sun getting this wrong one way or > > > another. Thinking about this some more, I also have no idea how you'd > > > want to deprecate rendering on primary nodes in general. Apparently > > > that breaks -modesetting already, and probably lots more compositors. > > > And it looks like we're finally achieve the goal kms set out to 10 > > > years ago, and new compositors are sprouting up all the time. I guess > > > we could just break them all (on new hardware) and tell them to all > > > suck it up. But I don't think that's a great option. And just > > > deprecating this on amdgpu is going to be even harder, since then > > > everywhere else it'll keep working, and it's just amdgpu.ko that looks > > > broken. > > > > > > Aside: I'm not supporting Emil's idea here because it fixes any issues > > > Intel has - Intel doesn't care. I support it because reality sucks, > > > people get this render vs. primary vs. multi-gpu prime wrong all the > > > time (that's also why we have hardcoded display+gpu pairs in mesa for > > > the various soc combinations out there), and this looks like a > > > pragmatic solution. It'd be nice if every compositor and everything > > > else would perfectly support multi gpu and only use render nodes for > > > rendering, and only primary nodes for display. But reality is that > > > people hack on stuff until gears on screen and then move on to more > > > interesting things (to them). So I don't think we'll ever win this :-/ > > > > Yeah, but this is a classic case of working around user space issues by > > making kernel changes instead of fixing user space. > > > > Having privileged (output control) and unprivileged (rendering control) > > functionality behind the same node is a mistake we have made a long time > > ago and render nodes finally seemed to be a way to fix that. > > > > I mean why are compositors using the primary node in the first place? > > Because they want to have access to privileged resources I think and in > > this case it is perfectly ok to do so. > > > > Now extending unprivileged access to the primary node actually sounds > > like a step into the wrong direction to me. > > > > I rather think that we should go down the route of completely dropping > > command submission and buffer allocation through the primary node for > > non master clients. And then as next step at some point drop support for > > authentication/flink. > > > > I mean we have done this with UMS as well and I don't see much other way > > to move forward and get rid of those ancient interface in the long term. > > Well kms had some really good benefits that drove quick adoption, like > "suspend/resume actually has a chance of working" or "comes with > buffer management so you can run multiple gears". > > The render node thing is a lot more niche use case (prime, better priv > separation), plus "it's cleaner design". And the "cleaner design" part > is something that empirically doesn't seem to matter :-/ Just two > examples: > - KHR_display/leases just iterated display resources on the fd needed > for rendering (and iirc there was even a patch to expose that for > render nodes too so it works with DRI3), because implementing > protocols is too hard. Barely managed to stop that one before it > happened. > - Various video players use the vblank ioctl on directly to schedule > frames, without telling the compositor. I discovered that when I > wanted to limite the vblank ioctl to master clients only. Again, > apparently too hard to use the existing extensions, or fix the bugs in > there, or whatever. One userspace got fixed last year, but it'll > probably get copypasted around forever :-/ > > So I don't think we'll ever manage to roll a clean split out, and best > we can do is give in and just hand userspace what it wants. As much as > that's misguided and unclean and all that. Maybe it'll result in a > least fewer stuff getting run as root to hack around this, because > fixing properly seems not to be on the table. > > The beauty of kms is that we've achieved the mission, everyone's > writing their own thing. Which is also terrible, and I don't think > it'll get better. With the risk of coming rude I will repeat my earlier comment: The problem is _neither_ Intel nor libva specific. That said, let's step back for a moment and consider: - the "block everything but KMS via the primary node" idea is great but orthogonal - the series does address issues that are vendor-agnostic - by default this series does _not_ cause any regression be that for new or old userspace - there are
Re: [PATCH] drm/omap: Migrate minimum FCK/PCK ratio from Kconfig to dts
Hi, On 28/05/2019 18:09, Adam Ford wrote: On Tue, May 28, 2019 at 4:11 AM Tomi Valkeinen wrote: Hi, On 10/05/2019 22:42, Adam Ford wrote: Currently the source code is compiled using hard-coded values from CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK. This patch allows this clock divider value to be moved to the device tree and be changed without having to recompile the kernel. Signed-off-by: Adam Ford I understand why you want to do this, but I'm not sure it's a good idea. It's really something the driver should figure out, and if we add it to the DT, it effectively becomes an ABI. That said... I'm not sure how good of a job the driver could ever do, as it can't know the future scaling needs of the userspace at the time it is configuring the clock. And so, I'm not nacking this patch, but I don't feel very good about this patch... The setting also affects all outputs (exluding venc), which may not be what the user wants. Then again, I think this setting is really only needed on OMAP2 & 3, which have only a single output. But that's the same with the current kconfig option, of course. So, the current CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK is an ugly hack, in my opinion, and moving it to DT makes it a worse hack =). But I don't have any good suggestions either. As it stands the Logic PD OMAP35 and AM37/DM37 boards (SOM-LV and Torpedo) require this to be hard coded to 4 or it hangs during start. This is the case for all versions 4.2+. I haven't tested it with older stuff. Tony has a DM3730 Torpedo kit and reported the hanging issue to me. I told him to set that value to 4 to make it not hang. He asked that I move it to the DT to avoid custom kernels. I agree it's a hack, but if it's create a customized defconfig file for 4 boards or modify the device tree, it seems like the device tree approach is less intrusive. Ok, well, I think that's a separate thing from its intended use. The point of the kconfig option is to ensure that the fclk/pclk ratio stays under a certain number to allow enough scaling range. It should never affect a basic non-scaling use case, unless you set it to a too high value, which prevents finding any pclk. Has anyone debugged why the hang is happening? If we can't fix the bug itself, rather than adding a DT option, we could change add a min_fck_per_pck field (as you do), keep the kconfig option, and set the min_fck_per_pck based on the kconfig option, _or_ in case of those affected SoCs, set the min_fck_per_pck even without the kconfig option. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm/ttm, drm/vmwgfx: Have TTM support AMD SEV encryption
On 5/28/19 10:17 AM, Koenig, Christian wrote: > Hi Thomas, > > Am 28.05.19 um 17:11 schrieb Thomas Hellstrom: >> Hi, Tom, >> >> Thanks for the reply. The question is not graphics specific, but lies >> in your answer further below: >> >> On 5/28/19 4:48 PM, Lendacky, Thomas wrote: >>> On 5/28/19 2:31 AM, Thomas Hellstrom wrote: >>> [SNIP] >>> As for kernel vmaps and user-maps, those pages will be marked encrypted >>> (unless explicitly made un-encrypted by calling set_memory_decrypted()). >>> But, if you are copying to/from those areas into the un-encrypted DMA >>> area then everything will be ok. >> >> The question is regarding the above paragraph. >> >> AFAICT, set_memory_decrypted() only changes the fixed kernel map PTEs. >> But when setting up other aliased PTEs to the exact same decrypted >> pages, for example using dma_mmap_coherent(), kmap_atomic_prot(), >> vmap() etc. What code is responsible for clearing the encrypted flag >> on those PTEs? Is there something in the x86 platform code doing that? > > Tom actually explained this: >> The encryption bit is bit-47 of a physical address. > > In other words set_memory_decrypted() changes the physical address in > the PTEs of the kernel mapping and all other use cases just copy that > from there. Except I don't think the PTE attributes are copied from the kernel mapping in some cases. For example, dma_mmap_coherent() will create the same vm_page_prot value regardless of whether or not the underlying memory is encrypted or not. But kmap_atomic_prot() will return the kernel virtual address of the page, so that would be fine. This is an area that needs looking into to be sure it is working properly with SME and SEV. Thanks, Tom > > That's rather nifty, because this way everybody will either use or not > use encryption on the page. > > Christian. > >> >> Thanks, >> Thomas >> >> >>> >>> Things get fuzzy for me when it comes to the GPU access of the memory >>> and what and how it is accessed. >>> >>> Thanks, >>> Tom > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/omap: remove open-coded drm_invalid_op()
On 2019/05/28, Tomi Valkeinen wrote:
> On 22/05/2019 18:02, Emil Velikov wrote:
> > From: Emil Velikov
> >
> > Cc: Tomi Valkeinen
> > Signed-off-by: Emil Velikov
> > ---
> > drivers/gpu/drm/omapdrm/omap_drv.c | 16 +---
> > 1 file changed, 1 insertion(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> > b/drivers/gpu/drm/omapdrm/omap_drv.c
> > index 1b9b6f5e48e1..672e0f8ad11c 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> > @@ -439,20 +439,6 @@ static int ioctl_get_param(struct drm_device *dev,
> > void *data,
> > return 0;
> > }
> > -static int ioctl_set_param(struct drm_device *dev, void *data,
> > - struct drm_file *file_priv)
> > -{
> > - struct drm_omap_param *args = data;
> > -
> > - switch (args->param) {
> > - default:
> > - DBG("unknown parameter %lld", args->param);
> > - return -EINVAL;
> > - }
> > -
> > - return 0;
> > -}
> > -
> > #define OMAP_BO_USER_MASK 0x00ff /* flags settable by userspace
> > */
> > static int ioctl_gem_new(struct drm_device *dev, void *data,
> > @@ -492,7 +478,7 @@ static int ioctl_gem_info(struct drm_device *dev, void
> > *data,
> > static const struct drm_ioctl_desc ioctls[DRM_COMMAND_END -
> > DRM_COMMAND_BASE] = {
> > DRM_IOCTL_DEF_DRV(OMAP_GET_PARAM, ioctl_get_param,
> > DRM_AUTH | DRM_RENDER_ALLOW),
> > - DRM_IOCTL_DEF_DRV(OMAP_SET_PARAM, ioctl_set_param,
> > + DRM_IOCTL_DEF_DRV(OMAP_SET_PARAM, drm_invalid_op,
> > DRM_AUTH | DRM_MASTER | DRM_ROOT_ONLY),
> > DRM_IOCTL_DEF_DRV(OMAP_GEM_NEW, ioctl_gem_new,
> > DRM_AUTH | DRM_RENDER_ALLOW),
> >
>
> Thanks! Do you want to take this via drm-misc too, or can I pick it up?
>
Hoping to pick this via drm-misc, albeit I forgot to mention earlier.
Thanks
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH v2] drm/msm/dpu: Use provided drm_minor to initialize debugfs
On Tue, May 28, 2019 at 11:13:39AM -0400, Sean Paul wrote:
> From: Sean Paul
>
> Instead of reaching into dev->primary for debugfs_root, use the minor
> passed into debugfs_init.
>
> This avoids creating a debug directory under /sys/kernel/debug/debug
> and instead uses /sys/kernel/debug/dri//
>
> Since we're now putting everything under */dri/, there's no
> need to create a duplicate "debug" directory. Just put everything in
> the root.
>
> Changes in v2:
> - Remove the unnecessary "debug" directory (Stephen)
>
> Link to v1:
> https://patchwork.freedesktop.org/patch/msgid/20190524173231.5040-1-s...@poorly.run
>
> Cc: Rob Clark
> Reported-by: Stephen Boyd
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 885bf88afa3ec..ad094704610f9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -231,18 +231,14 @@ void *dpu_debugfs_create_regset32(const char *name,
> umode_t mode,
> regset, &dpu_fops_regset32);
> }
>
> -static int _dpu_debugfs_init(struct dpu_kms *dpu_kms)
> +static int _dpu_debugfs_init(struct dpu_kms *dpu_kms, struct drm_minor
> *minor)
> {
> void *p = dpu_hw_util_get_log_mask_ptr();
> - struct dentry *entry;
> + struct dentry *entry = minor->debugfs_root;
>
> if (!p)
> return -EINVAL;
>
> - entry = debugfs_create_dir("debug",
> dpu_kms->dev->primary->debugfs_root);
> - if (IS_ERR_OR_NULL(entry))
> - return -ENODEV;
> -
> /* allow root to be NULL */
Minor nit, this comment seems to be misplaced even more now than it was before.
> debugfs_create_x32(DPU_DEBUGFS_HWMASKNAME, 0600, entry, p);
>
> @@ -581,7 +577,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
> #ifdef CONFIG_DEBUG_FS
> static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
> {
> - return _dpu_debugfs_init(to_dpu_kms(kms));
> + return _dpu_debugfs_init(to_dpu_kms(kms), minor);
> }
> #endif
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH v2] drm/msm/dpu: Use provided drm_minor to initialize debugfs
On Tue, May 28, 2019 at 8:13 AM Sean Paul wrote:
>
> From: Sean Paul
>
> Instead of reaching into dev->primary for debugfs_root, use the minor
> passed into debugfs_init.
>
> This avoids creating a debug directory under /sys/kernel/debug/debug
> and instead uses /sys/kernel/debug/dri//
>
> Since we're now putting everything under */dri/, there's no
> need to create a duplicate "debug" directory. Just put everything in
> the root.
>
> Changes in v2:
> - Remove the unnecessary "debug" directory (Stephen)
>
> Link to v1:
> https://patchwork.freedesktop.org/patch/msgid/20190524173231.5040-1-s...@poorly.run
>
> Cc: Rob Clark
> Reported-by: Stephen Boyd
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Sean Paul
nice!
Reviewed-by: Rob Clark
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 885bf88afa3ec..ad094704610f9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -231,18 +231,14 @@ void *dpu_debugfs_create_regset32(const char *name,
> umode_t mode,
> regset, &dpu_fops_regset32);
> }
>
> -static int _dpu_debugfs_init(struct dpu_kms *dpu_kms)
> +static int _dpu_debugfs_init(struct dpu_kms *dpu_kms, struct drm_minor
> *minor)
> {
> void *p = dpu_hw_util_get_log_mask_ptr();
> - struct dentry *entry;
> + struct dentry *entry = minor->debugfs_root;
>
> if (!p)
> return -EINVAL;
>
> - entry = debugfs_create_dir("debug",
> dpu_kms->dev->primary->debugfs_root);
> - if (IS_ERR_OR_NULL(entry))
> - return -ENODEV;
> -
> /* allow root to be NULL */
> debugfs_create_x32(DPU_DEBUGFS_HWMASKNAME, 0600, entry, p);
>
> @@ -581,7 +577,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
> #ifdef CONFIG_DEBUG_FS
> static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
> {
> - return _dpu_debugfs_init(to_dpu_kms(kms));
> + return _dpu_debugfs_init(to_dpu_kms(kms), minor);
> }
> #endif
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
>
> ___
> Freedreno mailing list
> freedr...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/komeda: Added AFBC support for komeda driver
Hi James, On Mon, May 27, 2019 at 07:51:18AM +0100, james qian wang (Arm Technology China) wrote: > Hi Brian & Ville: > > komed has a format+modifier check before the fb size check. > and for komeda_fb_create, the first step is do the format+modifier > check, the size check is the furthur check after the such format > valid check. and the detailed fb_create is like: Thanks for the detail, it sounds good. -Brian ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/omap: Migrate minimum FCK/PCK ratio from Kconfig to dts
> Am 28.05.2019 um 17:09 schrieb Adam Ford : > > On Tue, May 28, 2019 at 4:11 AM Tomi Valkeinen wrote: >> >> Hi, >> >> On 10/05/2019 22:42, Adam Ford wrote: >>> Currently the source code is compiled using hard-coded values >>> from CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK. This patch allows this >>> clock divider value to be moved to the device tree and be changed >>> without having to recompile the kernel. >>> >>> Signed-off-by: Adam Ford >> >> I understand why you want to do this, but I'm not sure it's a good idea. >> It's really something the driver should figure out, and if we add it to >> the DT, it effectively becomes an ABI. >> >> That said... I'm not sure how good of a job the driver could ever do, as >> it can't know the future scaling needs of the userspace at the time it >> is configuring the clock. And so, I'm not nacking this patch, but I >> don't feel very good about this patch... >> >> The setting also affects all outputs (exluding venc), which may not be >> what the user wants. Then again, I think this setting is really only >> needed on OMAP2 & 3, which have only a single output. But that's the >> same with the current kconfig option, of course. >> >> So, the current CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK is an ugly hack, in my >> opinion, and moving it to DT makes it a worse hack =). But I don't have >> any good suggestions either. > > As it stands the Logic PD OMAP35 and AM37/DM37 boards (SOM-LV and > Torpedo) require this to be hard coded to 4 or it hangs during start. > This is the case for all versions 4.2+. I haven't tested it with > older stuff. Tony has a DM3730 Torpedo kit and reported the hanging > issue to me. I told him to set that value to 4 to make it not hang. > He asked that I move it to the DT to avoid custom kernels. I agree > it's a hack, but if it's create a customized defconfig file for 4 > boards or modify the device tree, it seems like the device tree > approach is less intrusive. Well, if this boards needs a factor 4 to be defined, it is IMHO 100 % correct to describe this in the DTS and nowhere else. Like minimum and maximum voltage of a regulator which is also very board specific. Unless it can be figured out automatically. If it turns out later that it can, I would assume the drivers can simply ignore the hint in the DTS? Just my 2cts without knowing details and having tested anything on our DM37 boards. BR, Nikolaus
Re: [RFC PATCH] drm/ttm, drm/vmwgfx: Have TTM support AMD SEV encryption
Hi Thomas, Am 28.05.19 um 17:11 schrieb Thomas Hellstrom: > Hi, Tom, > > Thanks for the reply. The question is not graphics specific, but lies > in your answer further below: > > On 5/28/19 4:48 PM, Lendacky, Thomas wrote: >> On 5/28/19 2:31 AM, Thomas Hellstrom wrote: >> [SNIP] >> As for kernel vmaps and user-maps, those pages will be marked encrypted >> (unless explicitly made un-encrypted by calling set_memory_decrypted()). >> But, if you are copying to/from those areas into the un-encrypted DMA >> area then everything will be ok. > > The question is regarding the above paragraph. > > AFAICT, set_memory_decrypted() only changes the fixed kernel map PTEs. > But when setting up other aliased PTEs to the exact same decrypted > pages, for example using dma_mmap_coherent(), kmap_atomic_prot(), > vmap() etc. What code is responsible for clearing the encrypted flag > on those PTEs? Is there something in the x86 platform code doing that? Tom actually explained this: > The encryption bit is bit-47 of a physical address. In other words set_memory_decrypted() changes the physical address in the PTEs of the kernel mapping and all other use cases just copy that from there. That's rather nifty, because this way everybody will either use or not use encryption on the page. Christian. > > Thanks, > Thomas > > >> >> Things get fuzzy for me when it comes to the GPU access of the memory >> and what and how it is accessed. >> >> Thanks, >> Tom ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/msm/dpu: Use provided drm_minor to initialize debugfs
From: Sean Paul
Instead of reaching into dev->primary for debugfs_root, use the minor
passed into debugfs_init.
This avoids creating a debug directory under /sys/kernel/debug/debug
and instead uses /sys/kernel/debug/dri//
Since we're now putting everything under */dri/, there's no
need to create a duplicate "debug" directory. Just put everything in
the root.
Changes in v2:
- Remove the unnecessary "debug" directory (Stephen)
Link to v1:
https://patchwork.freedesktop.org/patch/msgid/20190524173231.5040-1-s...@poorly.run
Cc: Rob Clark
Reported-by: Stephen Boyd
Reviewed-by: Abhinav Kumar
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 885bf88afa3ec..ad094704610f9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -231,18 +231,14 @@ void *dpu_debugfs_create_regset32(const char *name,
umode_t mode,
regset, &dpu_fops_regset32);
}
-static int _dpu_debugfs_init(struct dpu_kms *dpu_kms)
+static int _dpu_debugfs_init(struct dpu_kms *dpu_kms, struct drm_minor *minor)
{
void *p = dpu_hw_util_get_log_mask_ptr();
- struct dentry *entry;
+ struct dentry *entry = minor->debugfs_root;
if (!p)
return -EINVAL;
- entry = debugfs_create_dir("debug",
dpu_kms->dev->primary->debugfs_root);
- if (IS_ERR_OR_NULL(entry))
- return -ENODEV;
-
/* allow root to be NULL */
debugfs_create_x32(DPU_DEBUGFS_HWMASKNAME, 0600, entry, p);
@@ -581,7 +577,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
#ifdef CONFIG_DEBUG_FS
static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
{
- return _dpu_debugfs_init(to_dpu_kms(kms));
+ return _dpu_debugfs_init(to_dpu_kms(kms), minor);
}
#endif
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm/ttm, drm/vmwgfx: Have TTM support AMD SEV encryption
Hi, Tom, Thanks for the reply. The question is not graphics specific, but lies in your answer further below: On 5/28/19 4:48 PM, Lendacky, Thomas wrote: On 5/28/19 2:31 AM, Thomas Hellstrom wrote: Hi, Tom, Could you shed some light on this? I don't have a lot of GPU knowledge, so let me start with an overview of how everything should work and see if that answers the questions being asked. First, SME: The encryption bit is bit-47 of a physical address. So, if a device does not support at least 48-bit DMA, it will have to use the SWIOTLB and bounce buffer the data. This is handled automatically if the driver is using the Linux DMA-api as all of SWIOTLB has been marked un-encrypted. Data is bounced between the un-encrypted SWIOTLB and the (presumably) encrypted area of the driver. For SEV: The encryption bit position is the same as SME. However, with SEV all DMA must use an un-encrypted area so all DMA goes through SWIOTLB. Just like SME, this is handled automatically if the driver is using the Linux DMA-api as all of SWIOTLB has been marked un-encrypted. And just like SME, data is bounced between the un-encrypted SWIOTLB and the (presumably) encrypted area of the driver. There is an optimization for dma_alloc_coherent() where the pages are allocated and marked un-encrypted, thus avoiding the bouncing (see file kernel/dma/direct.c, dma_direct_alloc_pages()). As for kernel vmaps and user-maps, those pages will be marked encrypted (unless explicitly made un-encrypted by calling set_memory_decrypted()). But, if you are copying to/from those areas into the un-encrypted DMA area then everything will be ok. The question is regarding the above paragraph. AFAICT, set_memory_decrypted() only changes the fixed kernel map PTEs. But when setting up other aliased PTEs to the exact same decrypted pages, for example using dma_mmap_coherent(), kmap_atomic_prot(), vmap() etc. What code is responsible for clearing the encrypted flag on those PTEs? Is there something in the x86 platform code doing that? Thanks, Thomas Things get fuzzy for me when it comes to the GPU access of the memory and what and how it is accessed. Thanks, Tom Thanks, Thomas On 5/24/19 5:08 PM, Alex Deucher wrote: + Tom He's been looking into SEV as well. On Fri, May 24, 2019 at 8:30 AM Thomas Hellstrom wrote: On 5/24/19 2:03 PM, Koenig, Christian wrote: Am 24.05.19 um 12:37 schrieb Thomas Hellstrom: [CAUTION: External Email] On 5/24/19 12:18 PM, Koenig, Christian wrote: Am 24.05.19 um 11:55 schrieb Thomas Hellstrom: [CAUTION: External Email] On 5/24/19 11:11 AM, Thomas Hellstrom wrote: Hi, Christian, On 5/24/19 10:37 AM, Koenig, Christian wrote: Am 24.05.19 um 10:11 schrieb Thomas Hellström (VMware): [CAUTION: External Email] From: Thomas Hellstrom With SEV encryption, all DMA memory must be marked decrypted (AKA "shared") for devices to be able to read it. In the future we might want to be able to switch normal (encrypted) memory to decrypted in exactly the same way as we handle caching states, and that would require additional memory pools. But for now, rely on memory allocated with dma_alloc_coherent() which is already decrypted with SEV enabled. Set up the page protection accordingly. Drivers must detect SEV enabled and switch to the dma page pool. This patch has not yet been tested. As a follow-up, we might want to cache decrypted pages in the dma page pool regardless of their caching state. This patch is unnecessary, SEV support already works fine with at least amdgpu and I would expect that it also works with other drivers as well. Also see this patch: commit 64e1f830ea5b3516a4256ed1c504a265d7f2a65c Author: Christian König Date: Wed Mar 13 10:11:19 2019 +0100 drm: fallback to dma_alloc_coherent when memory encryption is active We can't just map any randome page we get when memory encryption is active. Signed-off-by: Christian König Acked-by: Alex Deucher Link: https://patchwork.kernel.org/patch/10850833/ Regards, Christian. Yes, I noticed that. Although I fail to see where we automagically clear the PTE encrypted bit when mapping coherent memory? For the linear kernel map, that's done within dma_alloc_coherent() but for kernel vmaps and and user-space maps? Is that done automatically by the x86 platform layer? Yes, I think so. Haven't looked to closely at this either. This sounds a bit odd. If that were the case, the natural place would be the PAT tracking code, but it only handles caching flags AFAICT. Not encryption flags. But when you tested AMD with SEV, was that running as hypervisor rather than a guest, or did you run an SEV guest with PCI passthrough to the AMD device? Yeah, well the problem is we never tested this ourself :) /Thomas And, as a follow up question, why do we need dma_alloc_coherent() when using SME? I thought the hardware performs the decryption when DMA-ing to / from an encrypted page with SME, but
Re: [PATCH] drm/omap: Migrate minimum FCK/PCK ratio from Kconfig to dts
On Tue, May 28, 2019 at 4:11 AM Tomi Valkeinen wrote: > > Hi, > > On 10/05/2019 22:42, Adam Ford wrote: > > Currently the source code is compiled using hard-coded values > > from CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK. This patch allows this > > clock divider value to be moved to the device tree and be changed > > without having to recompile the kernel. > > > > Signed-off-by: Adam Ford > > I understand why you want to do this, but I'm not sure it's a good idea. > It's really something the driver should figure out, and if we add it to > the DT, it effectively becomes an ABI. > > That said... I'm not sure how good of a job the driver could ever do, as > it can't know the future scaling needs of the userspace at the time it > is configuring the clock. And so, I'm not nacking this patch, but I > don't feel very good about this patch... > > The setting also affects all outputs (exluding venc), which may not be > what the user wants. Then again, I think this setting is really only > needed on OMAP2 & 3, which have only a single output. But that's the > same with the current kconfig option, of course. > > So, the current CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK is an ugly hack, in my > opinion, and moving it to DT makes it a worse hack =). But I don't have > any good suggestions either. As it stands the Logic PD OMAP35 and AM37/DM37 boards (SOM-LV and Torpedo) require this to be hard coded to 4 or it hangs during start. This is the case for all versions 4.2+. I haven't tested it with older stuff. Tony has a DM3730 Torpedo kit and reported the hanging issue to me. I told him to set that value to 4 to make it not hang. He asked that I move it to the DT to avoid custom kernels. I agree it's a hack, but if it's create a customized defconfig file for 4 boards or modify the device tree, it seems like the device tree approach is less intrusive. adam > > Tomi > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [RFC PATCH] drm/ttm, drm/vmwgfx: Have TTM support AMD SEV encryption
Am 28.05.19 um 16:48 schrieb Lendacky, Thomas: On 5/28/19 2:31 AM, Thomas Hellstrom wrote: Hi, Tom, Could you shed some light on this? I don't have a lot of GPU knowledge, so let me start with an overview of how everything should work and see if that answers the questions being asked. First, SME: The encryption bit is bit-47 of a physical address. So, if a device does not support at least 48-bit DMA, it will have to use the SWIOTLB and bounce buffer the data. This is handled automatically if the driver is using the Linux DMA-api as all of SWIOTLB has been marked un-encrypted. Data is bounced between the un-encrypted SWIOTLB and the (presumably) encrypted area of the driver. Ok, that explains why we don't need to manually handle the encryption bit in TTM. For SEV: The encryption bit position is the same as SME. However, with SEV all DMA must use an un-encrypted area so all DMA goes through SWIOTLB. Just like SME, this is handled automatically if the driver is using the Linux DMA-api as all of SWIOTLB has been marked un-encrypted. And just like SME, data is bounced between the un-encrypted SWIOTLB and the (presumably) encrypted area of the driver. There is an optimization for dma_alloc_coherent() where the pages are allocated and marked un-encrypted, thus avoiding the bouncing (see file kernel/dma/direct.c, dma_direct_alloc_pages()). And that explains why we have to use dma_alloc_coherent() As for kernel vmaps and user-maps, those pages will be marked encrypted (unless explicitly made un-encrypted by calling set_memory_decrypted()). But, if you are copying to/from those areas into the un-encrypted DMA area then everything will be ok. Things get fuzzy for me when it comes to the GPU access of the memory and what and how it is accessed. I can fill in those lose ends, but it's probably going to be a long mail and a bit off topic. Thanks for filling in how that stuff actually works. And yeah, as far as I can see we actually don't need to do anything. The only way to get un-encrypted pages which don't bounce through SWIOTLB is using dma_alloc_coherent(). It's probably a bit unfortunate that TTM can't control it, but I think Thomas has to live with that. The use case for sharing encrypted pages is probably not so common anyway. Thanks, Christian. Thanks, Tom Thanks, Thomas On 5/24/19 5:08 PM, Alex Deucher wrote: + Tom He's been looking into SEV as well. On Fri, May 24, 2019 at 8:30 AM Thomas Hellstrom wrote: On 5/24/19 2:03 PM, Koenig, Christian wrote: Am 24.05.19 um 12:37 schrieb Thomas Hellstrom: [CAUTION: External Email] On 5/24/19 12:18 PM, Koenig, Christian wrote: Am 24.05.19 um 11:55 schrieb Thomas Hellstrom: [CAUTION: External Email] On 5/24/19 11:11 AM, Thomas Hellstrom wrote: Hi, Christian, On 5/24/19 10:37 AM, Koenig, Christian wrote: Am 24.05.19 um 10:11 schrieb Thomas Hellström (VMware): [CAUTION: External Email] From: Thomas Hellstrom With SEV encryption, all DMA memory must be marked decrypted (AKA "shared") for devices to be able to read it. In the future we might want to be able to switch normal (encrypted) memory to decrypted in exactly the same way as we handle caching states, and that would require additional memory pools. But for now, rely on memory allocated with dma_alloc_coherent() which is already decrypted with SEV enabled. Set up the page protection accordingly. Drivers must detect SEV enabled and switch to the dma page pool. This patch has not yet been tested. As a follow-up, we might want to cache decrypted pages in the dma page pool regardless of their caching state. This patch is unnecessary, SEV support already works fine with at least amdgpu and I would expect that it also works with other drivers as well. Also see this patch: commit 64e1f830ea5b3516a4256ed1c504a265d7f2a65c Author: Christian König Date: Wed Mar 13 10:11:19 2019 +0100 drm: fallback to dma_alloc_coherent when memory encryption is active We can't just map any randome page we get when memory encryption is active. Signed-off-by: Christian König Acked-by: Alex Deucher Link: https://patchwork.kernel.org/patch/10850833/ Regards, Christian. Yes, I noticed that. Although I fail to see where we automagically clear the PTE encrypted bit when mapping coherent memory? For the linear kernel map, that's done within dma_alloc_coherent() but for kernel vmaps and and user-space maps? Is that done automatically by the x86 platform layer? Yes, I think so. Haven't looked to closely at this either. This sounds a bit odd. If that were the case, the natural place would be the PAT tracking code, but it only handles caching flags AFAICT. Not encryption flags. But when you tested AMD with SEV, was that running as hypervisor rather than a guest, or did you run an SEV guest with PCI passthrough to the AMD device? Yeah, well the problem is we never tested this ourself :) /Thomas And, as a follow up questi
Re: [RFC PATCH] drm/ttm, drm/vmwgfx: Have TTM support AMD SEV encryption
On 5/28/19 2:31 AM, Thomas Hellstrom wrote: > Hi, Tom, > > Could you shed some light on this? I don't have a lot of GPU knowledge, so let me start with an overview of how everything should work and see if that answers the questions being asked. First, SME: The encryption bit is bit-47 of a physical address. So, if a device does not support at least 48-bit DMA, it will have to use the SWIOTLB and bounce buffer the data. This is handled automatically if the driver is using the Linux DMA-api as all of SWIOTLB has been marked un-encrypted. Data is bounced between the un-encrypted SWIOTLB and the (presumably) encrypted area of the driver. For SEV: The encryption bit position is the same as SME. However, with SEV all DMA must use an un-encrypted area so all DMA goes through SWIOTLB. Just like SME, this is handled automatically if the driver is using the Linux DMA-api as all of SWIOTLB has been marked un-encrypted. And just like SME, data is bounced between the un-encrypted SWIOTLB and the (presumably) encrypted area of the driver. There is an optimization for dma_alloc_coherent() where the pages are allocated and marked un-encrypted, thus avoiding the bouncing (see file kernel/dma/direct.c, dma_direct_alloc_pages()). As for kernel vmaps and user-maps, those pages will be marked encrypted (unless explicitly made un-encrypted by calling set_memory_decrypted()). But, if you are copying to/from those areas into the un-encrypted DMA area then everything will be ok. Things get fuzzy for me when it comes to the GPU access of the memory and what and how it is accessed. Thanks, Tom > > Thanks, > Thomas > > > On 5/24/19 5:08 PM, Alex Deucher wrote: >> + Tom >> >> He's been looking into SEV as well. >> >> On Fri, May 24, 2019 at 8:30 AM Thomas Hellstrom >> wrote: >>> On 5/24/19 2:03 PM, Koenig, Christian wrote: Am 24.05.19 um 12:37 schrieb Thomas Hellstrom: > [CAUTION: External Email] > > On 5/24/19 12:18 PM, Koenig, Christian wrote: >> Am 24.05.19 um 11:55 schrieb Thomas Hellstrom: >>> [CAUTION: External Email] >>> >>> On 5/24/19 11:11 AM, Thomas Hellstrom wrote: Hi, Christian, On 5/24/19 10:37 AM, Koenig, Christian wrote: > Am 24.05.19 um 10:11 schrieb Thomas Hellström (VMware): >> [CAUTION: External Email] >> >> From: Thomas Hellstrom >> >> With SEV encryption, all DMA memory must be marked decrypted >> (AKA "shared") for devices to be able to read it. In the future we >> might >> want to be able to switch normal (encrypted) memory to decrypted in >> exactly >> the same way as we handle caching states, and that would require >> additional >> memory pools. But for now, rely on memory allocated with >> dma_alloc_coherent() which is already decrypted with SEV enabled. >> Set up >> the page protection accordingly. Drivers must detect SEV enabled >> and >> switch >> to the dma page pool. >> >> This patch has not yet been tested. As a follow-up, we might >> want to >> cache decrypted pages in the dma page pool regardless of their >> caching >> state. > This patch is unnecessary, SEV support already works fine with at > least > amdgpu and I would expect that it also works with other drivers as > well. > > Also see this patch: > > commit 64e1f830ea5b3516a4256ed1c504a265d7f2a65c > Author: Christian König > Date: Wed Mar 13 10:11:19 2019 +0100 > > drm: fallback to dma_alloc_coherent when memory > encryption is > active > > We can't just map any randome page we get when memory > encryption is > active. > > Signed-off-by: Christian König > Acked-by: Alex Deucher > Link: https://patchwork.kernel.org/patch/10850833/ > > Regards, > Christian. Yes, I noticed that. Although I fail to see where we automagically clear the PTE encrypted bit when mapping coherent memory? For the linear kernel map, that's done within dma_alloc_coherent() but for kernel vmaps and and user-space maps? Is that done automatically by the x86 platform layer? >> Yes, I think so. Haven't looked to closely at this either. > This sounds a bit odd. If that were the case, the natural place would be > the PAT tracking code, but it only handles caching flags AFAICT. Not > encryption flags. > > But when you tested AMD with SEV, was that running as hypervisor rather > than a guest, or did you run an SEV guest with PCI passthrough to the > AMD device? Yeah, well the problem is we never tested this ourself :) /Thomas >>> And, as a follow up quest
[PATCH v3 10/10] [HACK] arm64: dts: renesas: ebisu: Enable LVDS dual-link operation
Enable and connect the second LVDS encoder to the second LVDS input of
the THC63LVD1024 for dual-link LVDS operation. This requires changing
the default settings of SW45 and SW47 to OFF and ON respectively.
Signed-off-by: Laurent Pinchart
Tested-by: Jacopo Mondi
---
.../arm64/boot/dts/renesas/r8a77990-ebisu.dts | 24 +--
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
index c72772589953..988d82609f41 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
@@ -93,11 +93,18 @@
port@0 {
reg = <0>;
- thc63lvd1024_in: endpoint {
+ thc63lvd1024_in0: endpoint {
remote-endpoint = <&lvds0_out>;
};
};
+ port@1 {
+ reg = <1>;
+ thc63lvd1024_in1: endpoint {
+ remote-endpoint = <&lvds1_out>;
+ };
+ };
+
port@2 {
reg = <2>;
thc63lvd1024_out: endpoint {
@@ -482,24 +489,27 @@
ports {
port@1 {
lvds0_out: endpoint {
- remote-endpoint = <&thc63lvd1024_in>;
+ remote-endpoint = <&thc63lvd1024_in0>;
};
};
};
};
&lvds1 {
- /*
-* Even though the LVDS1 output is not connected, the encoder must be
-* enabled to supply a pixel clock to the DU for the DPAD output when
-* LVDS0 is in use.
-*/
status = "okay";
clocks = <&cpg CPG_MOD 727>,
<&x13_clk>,
<&extal_clk>;
clock-names = "fck", "dclkin.0", "extal";
+
+ ports {
+ port@1 {
+ lvds1_out: endpoint {
+ remote-endpoint = <&thc63lvd1024_in1>;
+ };
+ };
+ };
};
&ohci0 {
--
Regards,
Laurent Pinchart
[PATCH v3 04/10] dt-bindings: display: renesas: lvds: Add renesas, companion property
Add a new optional renesas,companion property to point to the companion LVDS encoder. This is used to support dual-link operation where the main LVDS encoder splits even-numbered and odd-numbered pixels between the two LVDS encoders. The new property doesn't control the mode of operation, it only describes the relationship between the master and companion LVDS encoders. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Tested-by: Jacopo Mondi --- Changes since v2: - Clarify when the companion property is required or not allowed Changes since v1: - Fixed typo --- .../devicetree/bindings/display/bridge/renesas,lvds.txt| 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt index 900a884ad9f5..2d24bd8cbec5 100644 --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@ -45,6 +45,13 @@ OF graph bindings specified in Documentation/devicetree/bindings/graph.txt. Each port shall have a single endpoint. +Optional properties: + +- renesas,companion : phandle to the companion LVDS encoder. This property is + mandatory for the first LVDS encoder on D3 and E3 SoCs, and shall point to + the second encoder to be used as a companion in dual-link mode. It shall not + be set for any other LVDS encoder. + Example: -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 03/10] drm: bridge: thc63: Report input bus mode through bridge timings
Set a drm_bridge_timings in the drm_bridge, and use it to report the
input bus mode (single-link or dual-link). The other fields of the
timings structure are kept to 0 as they do not apply to LVDS buses.
Signed-off-by: Laurent Pinchart
Reviewed-by: Jacopo Mondi
Tested-by: Jacopo Mondi
---
Changes since v1:
- Ignore disabled remote device
---
drivers/gpu/drm/bridge/thc63lvd1024.c | 54 +--
1 file changed, 43 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c
b/drivers/gpu/drm/bridge/thc63lvd1024.c
index b083a740565c..709dd28b43d6 100644
--- a/drivers/gpu/drm/bridge/thc63lvd1024.c
+++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
@@ -31,6 +31,8 @@ struct thc63_dev {
struct drm_bridge bridge;
struct drm_bridge *next;
+
+ struct drm_bridge_timings timings;
};
static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
@@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge)
static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
const struct drm_display_mode *mode)
{
+ struct thc63_dev *thc63 = to_thc63(bridge);
+ unsigned int min_freq;
+ unsigned int max_freq;
+
/*
-* The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
-* mode. Note that the limits are different in dual-in, single-out mode,
-* and will need to be adjusted accordingly.
+* The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
+* dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
+* isn't supported by the driver yet, simply derive the limits from the
+* input mode.
*/
- if (mode->clock < 8000)
+ if (thc63->timings.dual_link) {
+ min_freq = 4;
+ max_freq = 15;
+ } else {
+ min_freq = 8000;
+ max_freq = 135000;
+ }
+
+ if (mode->clock < min_freq)
return MODE_CLOCK_LOW;
- if (mode->clock > 135000)
+ if (mode->clock > max_freq)
return MODE_CLOCK_HIGH;
return MODE_OK;
@@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func = {
static int thc63_parse_dt(struct thc63_dev *thc63)
{
- struct device_node *thc63_out;
+ struct device_node *endpoint;
struct device_node *remote;
- thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
- THC63_RGB_OUT0, -1);
- if (!thc63_out) {
+ endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
+THC63_RGB_OUT0, -1);
+ if (!endpoint) {
dev_err(thc63->dev, "Missing endpoint in port@%u\n",
THC63_RGB_OUT0);
return -ENODEV;
}
- remote = of_graph_get_remote_port_parent(thc63_out);
- of_node_put(thc63_out);
+ remote = of_graph_get_remote_port_parent(endpoint);
+ of_node_put(endpoint);
if (!remote) {
dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
THC63_RGB_OUT0);
@@ -132,6 +147,22 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
if (!thc63->next)
return -EPROBE_DEFER;
+ endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
+THC63_LVDS_IN1, -1);
+ if (endpoint) {
+ remote = of_graph_get_remote_port_parent(endpoint);
+ of_node_put(endpoint);
+
+ if (remote) {
+ if (of_device_is_available(remote))
+ thc63->timings.dual_link = true;
+ of_node_put(remote);
+ }
+ }
+
+ dev_dbg(thc63->dev, "operating in %s-link mode\n",
+ thc63->timings.dual_link ? "dual" : "single");
+
return 0;
}
@@ -188,6 +219,7 @@ static int thc63_probe(struct platform_device *pdev)
thc63->bridge.driver_private = thc63;
thc63->bridge.of_node = pdev->dev.of_node;
thc63->bridge.funcs = &thc63_bridge_func;
+ thc63->bridge.timings = &thc63->timings;
drm_bridge_add(&thc63->bridge);
--
Regards,
Laurent Pinchart
[PATCH v3 05/10] drm: rcar-du: lvds: Remove LVDS double-enable checks
The DRM core and DU driver guarantee that the LVDS bridge will not be
double-enabled or double-disabled. Remove the corresponding unnecessary
checks.
Signed-off-by: Laurent Pinchart
Reviewed-by: Jacopo Mondi
Tested-by: Jacopo Mondi
---
drivers/gpu/drm/rcar-du/rcar_lvds.c | 19 ---
1 file changed, 19 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 620b51aab291..a331f0c32187 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -63,7 +63,6 @@ struct rcar_lvds {
struct clk *extal; /* External clock */
struct clk *dotclkin[2];/* External DU clocks */
} clocks;
- bool enabled;
struct drm_display_mode display_mode;
enum rcar_lvds_mode mode;
@@ -368,15 +367,12 @@ int rcar_lvds_clk_enable(struct drm_bridge *bridge,
unsigned long freq)
dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
- WARN_ON(lvds->enabled);
-
ret = clk_prepare_enable(lvds->clocks.mod);
if (ret < 0)
return ret;
__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
- lvds->enabled = true;
return 0;
}
EXPORT_SYMBOL_GPL(rcar_lvds_clk_enable);
@@ -390,13 +386,9 @@ void rcar_lvds_clk_disable(struct drm_bridge *bridge)
dev_dbg(lvds->dev, "disabling LVDS PLL\n");
- WARN_ON(!lvds->enabled);
-
rcar_lvds_write(lvds, LVDPLLCR, 0);
clk_disable_unprepare(lvds->clocks.mod);
-
- lvds->enabled = false;
}
EXPORT_SYMBOL_GPL(rcar_lvds_clk_disable);
@@ -417,8 +409,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
u32 lvdcr0;
int ret;
- WARN_ON(lvds->enabled);
-
ret = clk_prepare_enable(lvds->clocks.mod);
if (ret < 0)
return;
@@ -507,16 +497,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
drm_panel_prepare(lvds->panel);
drm_panel_enable(lvds->panel);
}
-
- lvds->enabled = true;
}
static void rcar_lvds_disable(struct drm_bridge *bridge)
{
struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
- WARN_ON(!lvds->enabled);
-
if (lvds->panel) {
drm_panel_disable(lvds->panel);
drm_panel_unprepare(lvds->panel);
@@ -527,8 +513,6 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)
rcar_lvds_write(lvds, LVDPLLCR, 0);
clk_disable_unprepare(lvds->clocks.mod);
-
- lvds->enabled = false;
}
static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
@@ -592,8 +576,6 @@ static void rcar_lvds_mode_set(struct drm_bridge *bridge,
{
struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
- WARN_ON(lvds->enabled);
-
lvds->display_mode = *adjusted_mode;
rcar_lvds_get_lvds_mode(lvds);
@@ -793,7 +775,6 @@ static int rcar_lvds_probe(struct platform_device *pdev)
lvds->dev = &pdev->dev;
lvds->info = of_device_get_match_data(&pdev->dev);
- lvds->enabled = false;
ret = rcar_lvds_parse_dt(lvds);
if (ret < 0)
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 06/10] drm: rcar-du: lvds: Add support for dual-link mode
In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and
sends odd-numbered pixels to the LVDS1 encoder for transmission on a
separate link.
To implement support for this mode of operation, determine if the LVDS
connection operates in dual-link mode by querying the next device in the
pipeline, locate the companion encoder, and control it directly through
its bridge operations.
Signed-off-by: Laurent Pinchart
Reviewed-by: Jacopo Mondi
Tested-by: Jacopo Mondi
---
Changes since v2:
- Fail probe if the companion controller can't be found or is invalid
---
drivers/gpu/drm/rcar-du/rcar_lvds.c | 107
drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 ++
2 files changed, 99 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index a331f0c32187..d090191e858e 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -66,6 +66,9 @@ struct rcar_lvds {
struct drm_display_mode display_mode;
enum rcar_lvds_mode mode;
+
+ struct drm_bridge *companion;
+ bool dual_link;
};
#define bridge_to_rcar_lvds(bridge) \
@@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
{
struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
const struct drm_display_mode *mode = &lvds->display_mode;
- /*
-* FIXME: We should really retrieve the CRTC through the state, but how
-* do we get a state pointer?
-*/
- struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
u32 lvdhcr;
u32 lvdcr0;
int ret;
@@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
if (ret < 0)
return;
+ /* Enable the companion LVDS encoder in dual-link mode. */
+ if (lvds->dual_link && lvds->companion)
+ lvds->companion->funcs->enable(lvds->companion);
+
/*
* Hardcode the channels and control signals routing for now.
*
@@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
- /* Disable dual-link mode. */
- rcar_lvds_write(lvds, LVDSTRIPE, 0);
+ /*
+* Configure vertical stripe based on the mode of operation of
+* the connected device.
+*/
+ rcar_lvds_write(lvds, LVDSTRIPE,
+ lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
}
- /* PLL clock configuration. */
- lvds->info->pll_setup(lvds, mode->clock * 1000);
+ /*
+* PLL clock configuration on all instances but the companion in
+* dual-link mode.
+*/
+ if (!lvds->dual_link || lvds->companion)
+ lvds->info->pll_setup(lvds, mode->clock * 1000);
/* Set the LVDS mode and select the input. */
lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
- if (drm_crtc_index(crtc) == 2)
- lvdcr0 |= LVDCR0_DUSEL;
+
+ if (lvds->bridge.encoder) {
+ /*
+* FIXME: We should really retrieve the CRTC through the state,
+* but how do we get a state pointer?
+*/
+ if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)
+ lvdcr0 |= LVDCR0_DUSEL;
+ }
+
rcar_lvds_write(lvds, LVDCR0, lvdcr0);
/* Turn all the channels on. */
@@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)
rcar_lvds_write(lvds, LVDCR1, 0);
rcar_lvds_write(lvds, LVDPLLCR, 0);
+ /* Disable the companion LVDS encoder in dual-link mode. */
+ if (lvds->dual_link && lvds->companion)
+ lvds->companion->funcs->disable(lvds->companion);
+
clk_disable_unprepare(lvds->clocks.mod);
}
@@ -628,10 +650,57 @@ static const struct drm_bridge_funcs rcar_lvds_bridge_ops
= {
.mode_set = rcar_lvds_mode_set,
};
+bool rcar_lvds_dual_link(struct drm_bridge *bridge)
+{
+ struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
+
+ return lvds->dual_link;
+}
+EXPORT_SYMBOL_GPL(rcar_lvds_dual_link);
+
/*
-
* Probe & Remove
*/
+static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
+{
+ const struct of_device_id *match;
+ struct device_node *companion;
+ struct device *dev = lvds->dev;
+ int ret = 0;
+
+ /* Locate the companion LVDS encoder for dual-link operation, if any. */
+ companion = of_parse_phandle(dev->of_node, "renesas,companion", 0);
+ if (!companion) {
+ dev_err(dev, "Companion LVDS encoder not found\n");
+ return -ENXIO;
+ }
+
+ /*
+* Sanity check: the companion encoder must have
[PATCH v3 09/10] [HACK] arm64: dts: renesas: draak: Enable LVDS dual-link operation
Enable and connect the second LVDS encoder to the second LVDS input of
the THC63LVD1024 for dual-link LVDS operation. This requires changing
the default settings of SW45 and SW47 to OFF and ON respectively.
Signed-off-by: Laurent Pinchart
Tested-by: Jacopo Mondi
---
.../arm64/boot/dts/renesas/r8a77995-draak.dts | 24 +--
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index a7dc11e36fd9..0699f1c19b11 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -77,11 +77,18 @@
port@0 {
reg = <0>;
- thc63lvd1024_in: endpoint {
+ thc63lvd1024_in0: endpoint {
remote-endpoint = <&lvds0_out>;
};
};
+ port@1 {
+ reg = <1>;
+ thc63lvd1024_in1: endpoint {
+ remote-endpoint = <&lvds1_out>;
+ };
+ };
+
port@2 {
reg = <2>;
thc63lvd1024_out: endpoint {
@@ -360,24 +367,27 @@
ports {
port@1 {
lvds0_out: endpoint {
- remote-endpoint = <&thc63lvd1024_in>;
+ remote-endpoint = <&thc63lvd1024_in0>;
};
};
};
};
&lvds1 {
- /*
-* Even though the LVDS1 output is not connected, the encoder must be
-* enabled to supply a pixel clock to the DU for the DPAD output when
-* LVDS0 is in use.
-*/
status = "okay";
clocks = <&cpg CPG_MOD 727>,
<&x12_clk>,
<&extal_clk>;
clock-names = "fck", "dclkin.0", "extal";
+
+ ports {
+ port@1 {
+ lvds1_out: endpoint {
+ remote-endpoint = <&thc63lvd1024_in1>;
+ };
+ };
+ };
};
&ohci0 {
--
Regards,
Laurent Pinchart
[PATCH v3 07/10] drm: rcar-du: Skip LVDS1 output on Gen3 when using dual-link LVDS mode
In dual-link LVDS mode, the LVDS1 encoder is used as a companion for
LVDS0, and both encoders transmit data from DU0. The LVDS1 output of DU1
can't be used in that case, don't create an encoder and connector for
it.
Signed-off-by: Laurent Pinchart
Reviewed-by: Jacopo Mondi
Tested-by: Jacopo Mondi
---
Changess since v2:
- Remove unneeded bridge NULL check
---
drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 12
drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index 6c91753af7bc..0f00bdfe2366 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -16,6 +16,7 @@
#include "rcar_du_drv.h"
#include "rcar_du_encoder.h"
#include "rcar_du_kms.h"
+#include "rcar_lvds.h"
/*
-
* Encoder
@@ -97,6 +98,17 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
}
}
+ /*
+* On Gen3 skip the LVDS1 output if the LVDS1 encoder is used as a
+* companion for LVDS0 in dual-link mode.
+*/
+ if (rcdu->info->gen >= 3 && output == RCAR_DU_OUTPUT_LVDS1) {
+ if (rcar_lvds_dual_link(bridge)) {
+ ret = -ENOLINK;
+ goto done;
+ }
+ }
+
ret = drm_encoder_init(rcdu->ddev, encoder, &encoder_funcs,
DRM_MODE_ENCODER_NONE, NULL);
if (ret < 0)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index f8f7fff34dff..95c81e59e2f1 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -378,7 +378,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device
*rcdu,
}
ret = rcar_du_encoder_init(rcdu, output, entity);
- if (ret && ret != -EPROBE_DEFER)
+ if (ret && ret != -EPROBE_DEFER && ret != -ENOLINK)
dev_warn(rcdu->dev,
"failed to initialize encoder %pOF on output %u (%d),
skipping\n",
entity, output, ret);
--
Regards,
Laurent Pinchart
[PATCH v3 02/10] dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation
The THC63LVD1024 LVDS decoder can operate in two modes, single-link or dual-link. In dual-link mode both input ports are used to carry even- and odd-numbered pixels separately. Document this in the DT bindings, along with the related rules governing port and usage. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Rob Herring Tested-by: Jacopo Mondi --- .../bindings/display/bridge/thine,thc63lvd1024.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt index 37f0c04d5a28..d17d1e5820d7 100644 --- a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt @@ -28,6 +28,12 @@ Optional video port nodes: - port@1: Second LVDS input port - port@3: Second digital CMOS/TTL parallel output +The device can operate in single-link mode or dual-link mode. In single-link +mode, all pixels are received on port@0, and port@1 shall not contain any +endpoint. In dual-link mode, even-numbered pixels are received on port@0 and +odd-numbered pixels on port@1, and both port@0 and port@1 shall contain +endpoints. + Example: -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 08/10] arm64: dts: renesas: r8a7799[05]: Point LVDS0 to its companion LVDS1
Add the new renesas,companion property to the LVDS0 node to point to the
companion LVDS encoder LVDS1.
Signed-off-by: Laurent Pinchart
Reviewed-by: Jacopo Mondi
Tested-by: Jacopo Mondi
---
arch/arm64/boot/dts/renesas/r8a77990.dtsi | 2 ++
arch/arm64/boot/dts/renesas/r8a77995.dtsi | 2 ++
2 files changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index 56cb566ffa09..7cf5963eb3ba 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -1801,6 +1801,8 @@
resets = <&cpg 727>;
status = "disabled";
+ renesas,companion = <&lvds1>;
+
ports {
#address-cells = <1>;
#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
index 5bf3af246e14..94b5177eb152 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
@@ -1038,6 +1038,8 @@
resets = <&cpg 727>;
status = "disabled";
+ renesas,companion = <&lvds1>;
+
ports {
#address-cells = <1>;
#size-cells = <0>;
--
Regards,
Laurent Pinchart
[PATCH v3 00/10] R-Car DU: LVDS dual-link mode support
Hello everybody, This patch series implements support for LVDS dual-link mode in the R-Car DU and R-Car LVDS encoder drivers, and well as in the thc63lvd1024 LVDS decoder driver. LVDS dual-link is a mode of operation where two individual LVDS links are operated together to carry even- and odd-numbered pixels separately. This doubles the possible bandwidth of the video transmission. Both the transmitter and the receiver need to support this mode of operation. The R-Car D3 and E3 SoCs include two independent LVDS encoders that can be grouped together to operate in dual-link mode. When used separately, the LVDS encoders are connected to two different CRTCs and transmit independent video streams. When used in dual-link mode, the first LVDS encoder is connected to the first CRTC, and split even- and odd-numbered pixels. It transmits half of the pixels on its LVDS output, and sends the other half to the second LVDS encoder for transmittion over the second LVDS link. The second LVDS encoder thus operates under control of the first one, and isn't connected directly to a CRTC. On the receiving side, the THC63LVD1024 LVDS-to-parallel bridge has two LVDS inputs and two parallel outputs. It can operate in four different modes: - Single-in, single-out: The first LVDS input receives the video stream, and the bridge outputs it on the first parallel output. The second LVDS input and the second parallel output are not used. - Single-in, dual-out: The first LVDS input receives the video stream, and the bridge splits even- and odd-numbered pixels and outputs them on the first and second parallel outputs. The second LVDS input is not used. - Dual-in, single-out: The two LVDS inputs are used in dual-link mode, and the bridge combines the even- and odd-numbered pixels and outputs them on the first parallel output. The second parallel output is not used. - Dual-in, dual-out: The two LVDS inputs are used in dual-link mode, and the bridge outputs the even- and odd-numbered pixels on the first parallel output. The operating mode is selected by two input pins of the bridge, which are connected to DIP switches on the development boards I use. The mode is thus fixed from a Linux point of view. Patch 01/10 adds a new dual_link boolen field to the drm_bridge_timings structure to let bridges report their LVDS mode of operation. Patch 02/10 clarifies the THC63LVD1024 DT bindings to document dual-link operation, and patch 03/10 implements dual-link support in the thc64lvd1024 bridge driver by setting the drm_bridge_timings dual_link field according to the mode selected through DT. Patch 04/10 extends the R-Car LVDS DT bindings to specify the companion LVDS encoder for dual-link operation. Patches 05/10 then performs a small cleanup in the LVDS encoder driver. Patch 06/10 implements dual-link support in the LVDS encoder driver, which involves retrieving the operation mode from the LVDS receiver, locating the companion LVDS encoder, and configuring both encoders when dual-link operation is desired. The API towards the DU driver is also extended to report the mode of operation. Patch 07/10 implements dual-link mode support in the DU driver. There is no specific configuration to be performed there, as dual-link is fully implemented in the LVDS encoder driver, but the DU driver has to skip creation of the DRM encoder and connector related to the second LVDS encoder when dual-link is used, as the second LVDS encoder operates as a slave of the first one, transparently from a CRTC (and thus userspace) perspective. Patch 08/10 specifies the companion LVDS encoder in the D3 and E3 DT bindings. This by itself doesn't enable dual-link mode, the LVDS0 encoder is still connected to the HDMI output through LVDS receiver, and the LVDS1 encoder is not used. Patches 09/10 and 10/10, not intended to be merged, enable dual-link operation for the D3 and E3 boards for testing and require flipping DIP switches on the boards. The patches are based on top of my drm/du/next branch, and are available for convenience at git://linuxtv.org/pinchartl/media.git drm/du/lvds/dual-link They have been tested successfully on the D3 Draak board. I expect them to work on E3 as well, but I don't have access to an Ebisu board to test this. Laurent Pinchart (10): drm: bridge: Add dual_link field to the drm_bridge_timings structure dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation drm: bridge: thc63: Report input bus mode through bridge timings dt-bindings: display: renesas: lvds: Add renesas,companion property drm: rcar-du: lvds: Remove LVDS double-enable checks drm: rcar-du: lvds: Add support for dual-link mode drm: rcar-du: Skip LVDS1 output on Gen3 when using dual-link LVDS mode arm64: dts: renesas: r8a7799[05]: Point LVDS0 to its companion LVDS1 [HACK] arm64: dts: renesas: draak: Enable LVDS dual-link operation [HACK] arm64: dts: renesas: ebisu: Enable LVDS dual-link operation .../bin
[PATCH v3 01/10] drm: bridge: Add dual_link field to the drm_bridge_timings structure
Extend the drm_bridge_timings structure with a new dual_link field to
indicate that the bridge's input bus carries data on two separate
physical links. The first use case is LVDS dual-link mode where even-
and odd-numbered pixels are transferred on separate LVDS links.
Signed-off-by: Laurent Pinchart
Reviewed-by: Kieran Bingham
Tested-by: Jacopo Mondi
---
include/drm/drm_bridge.h | 8
1 file changed, 8 insertions(+)
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index d4428913a4e1..aea1fcfd92a7 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -265,6 +265,14 @@ struct drm_bridge_timings {
* input signal after the clock edge.
*/
u32 hold_time_ps;
+ /**
+* @dual_link:
+*
+* True if the bus operates in dual-link mode. The exact meaning is
+* dependent on the bus type. For LVDS buses, this indicates that even-
+* and odd-numbered pixels are received on separate links.
+*/
+ bool dual_link;
};
/**
--
Regards,
Laurent Pinchart
[PATCH 1/2] drm/dp: Add DP_DPCD_QUIRK_NO_SINK_COUNT
From: Ville Syrjälä
CH7511 eDP->LVDS bridge doesn't seem to set SINK_COUNT properly
causing i915 to detect it as disconnected. Add a quirk to ignore
SINK_COUNT on these devices.
Cc: David S.
Cc: Peteris Rudzusiks
Tested-by: Peteris Rudzusiks
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105406
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_dp_helper.c | 4 +++-
include/drm/drm_dp_helper.h | 7 +++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index e6af758a7d22..0b994d083a89 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1280,7 +1280,9 @@ static const struct dpcd_quirk dpcd_quirk_list[] = {
/* LG LP140WF6-SPM1 eDP panel */
{ OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'),
false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
/* Apple panels need some additional handling to support PSR */
- { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false,
BIT(DP_DPCD_QUIRK_NO_PSR) }
+ { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false,
BIT(DP_DPCD_QUIRK_NO_PSR) },
+ /* CH7511 seems to leave SINK_COUNT zeroed */
+ { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'),
false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) },
};
#undef OUI
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 3fc534ee8174..7e52eb81284a 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1414,6 +1414,13 @@ enum drm_dp_quirk {
* driver still need to implement proper handling for such device.
*/
DP_DPCD_QUIRK_NO_PSR,
+ /**
+* @DP_DPCD_QUIRK_NO_SINK_COUNT:
+*
+* The device does not set SINK_COUNT to a non-zero value.
+* The driver should ignore SINK_COUNT during detection.
+*/
+ DP_DPCD_QUIRK_NO_SINK_COUNT,
};
/**
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/i915: Skip SINK_COUNT read on CH7511
From: Ville Syrjälä
CH7511 doesn't update SINK_COUNT properly so in order to detect
the device as connected we have to ignore SINK_COUNT.
In order to have access to the quirk list early enough we
must move the drm_dp_read_desc() call to happen earlier.
We can also skip re-reading this on eDP since we know it
won't change.
Cc: David S.
Cc: Peteris Rudzusiks
Tested-by: Peteris Rudzusiks
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105406
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 24b56b2a76c8..8fe6571cda8f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4242,8 +4242,14 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
if (!intel_dp_read_dpcd(intel_dp))
return false;
- /* Don't clobber cached eDP rates. */
+ /*
+* Don't clobber cached eDP rates. Also skip re-reading
+* the OUI/ID since we know it won't change.
+*/
if (!intel_dp_is_edp(intel_dp)) {
+ drm_dp_read_desc(&intel_dp->aux, &intel_dp->desc,
+drm_dp_is_branch(intel_dp->dpcd));
+
intel_dp_set_sink_rates(intel_dp);
intel_dp_set_common_rates(intel_dp);
}
@@ -4252,7 +4258,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
* Some eDP panels do not set a valid value for sink count, that is why
* it don't care about read it here and in intel_edp_init_dpcd().
*/
- if (!intel_dp_is_edp(intel_dp)) {
+ if (!intel_dp_is_edp(intel_dp) &&
+ !drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_NO_SINK_COUNT)) {
u8 count;
ssize_t r;
@@ -5586,9 +5593,6 @@ intel_dp_detect(struct drm_connector *connector,
if (INTEL_GEN(dev_priv) >= 11)
intel_dp_get_dsc_sink_cap(intel_dp);
- drm_dp_read_desc(&intel_dp->aux, &intel_dp->desc,
-drm_dp_is_branch(intel_dp->dpcd));
-
intel_dp_configure_mst(intel_dp);
if (intel_dp->is_mst) {
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND PATCH v2 2/2] drm/panfrost: Expose perf counters through debugfs
On Tue, 28 May 2019 15:53:48 +0200
Boris Brezillon wrote:
> Hi Steve,
>
> On Thu, 16 May 2019 17:21:39 +0100
> Steven Price wrote:
>
>
> > >> +static void panfrost_perfcnt_setup(struct panfrost_device *pfdev)
> > >> +{
> > >> + u32 cfg;
> > >> +
> > >> + /*
> > >> +* Always use address space 0 for now.
> > >> +* FIXME: this needs to be updated when we start using different
> > >> +* address space.
> > >> +*/
> > >> + cfg = GPU_PERFCNT_CFG_AS(0);
> > >> + if (panfrost_model_cmp(pfdev, 0x1000) >= 0)
> > >
> > > You've got a couple of these. Perhaps we should add either a
> > > model_is_bifrost() helper or an is_bifrost variable to use.
> > >
> > >> + cfg |= GPU_PERFCNT_CFG_SETSEL(1);
> >
> > mali_kbase only sets this if CONFIG_MALI_PRFCNT_SET_SECONDARY is defined
> > - i.e. this selects a different selection of counters. At at the very
> > least this should be an optional flag that can be set, but I suspect the
> > primary set are the ones you are interested in.
>
> I'll add a param to pass the set of counters to monitor.
>
> >
> > >> +
> > >> + gpu_write(pfdev, GPU_PERFCNT_CFG,
> > >> + cfg | GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_OFF));
> > >> +
> > >> + if (!pfdev->perfcnt->enabled)
> > >> + return;
> > >> +
> > >> + gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0x);
> > >> + gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0x);
> > >> + gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0x);
> > >> +
> > >> + /*
> > >> +* Due to PRLAM-8186 we need to disable the Tiler before we
> > >> enable HW
> > >> +* counters.
> > >
> > > Seems like you could just always apply the work-around? It doesn't
> > > look like it would be much different.
> >
> > Technically the workaround causes the driver to perform the operation in
> > the wrong order below (write TILER_EN when the mode is MANUAL) - this is
> > a workaround for the hardware issue (and therefore works on that
> > hardware). But I wouldn't like to say it works on other hardware which
> > doesn't have the issue.
>
> Okay, I'll keep the workaround only for HW that are impacted by the bug.
>
> >
> > >> +*/
> > >> + if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> > >> + gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
> > >> + else
> > >> + gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0x);
> > >> +
> > >> + gpu_write(pfdev, GPU_PERFCNT_CFG,
> > >> + cfg |
> > >> GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_MANUAL));
> > >> +
> > >> + if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> > >> + gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0x);
> > >> +}
>
> [...]
>
> > >> +int panfrost_perfcnt_init(struct panfrost_device *pfdev)
> > >> +{
> > >> + struct panfrost_perfcnt *perfcnt;
> > >> + struct drm_gem_shmem_object *bo;
> > >> + size_t size;
> > >> + u32 status;
> > >> + int ret;
> > >> +
> > >> + if (panfrost_has_hw_feature(pfdev, HW_FEATURE_V4)) {
> > >> + unsigned int ncoregroups;
> > >> +
> > >> + ncoregroups = hweight64(pfdev->features.l2_present);
> > >> + size = ncoregroups * BLOCKS_PER_COREGROUP *
> > >> + COUNTERS_PER_BLOCK * BYTES_PER_COUNTER;
> > >> + } else {
> > >> + unsigned int nl2c, ncores;
> > >> +
> > >> + /*
> > >> +* TODO: define a macro to extract the number of l2
> > >> caches from
> > >> +* mem_features.
> > >> +*/
> > >> + nl2c = ((pfdev->features.mem_features >> 8) & GENMASK(3,
> > >> 0)) + 1;
> > >> +
> > >> + /*
> > >> +* The ARM driver is grouping cores per core group and
> > >> then
> > >> +* only using the number of cores in group 0 to
> > >> calculate the
> > >> +* size. Not sure why this is done like that, but I guess
> > >> +* shader_present will only show cores in the first group
> > >> +* anyway.
> > >> +*/
> >
> > I'm not sure I understand this comment. I'm not sure which version of
> > the driver you are looking at, but shader_present should contain all the
> > cores (in every core group).
> >
> > The kbase driver (in panfrost's git tree[1]), contains:
> >
> > base_gpu_props *props = &kbdev->gpu_props.props;
> > u32 nr_l2 = props->l2_props.num_l2_slices;
> > u64 core_mask = props->coherency_info.group[0].core_mask;
> >
> > u32 nr_blocks = fls64(core_mask);
> >
> > /* JM and tiler counter blocks are always present */
> > dump_size = (2 + nr_l2 + nr_blocks) *
> > NR_CNT_PER_BLOCK *
> > NR_BYTES_PER_CNT;
> >
> > [1]
> > https://gitlab.freedesktop.org/panfrost/mali_kbase/blob/ma
Re: [RESEND PATCH v2 2/2] drm/panfrost: Expose perf counters through debugfs
Hi Steve,
On Thu, 16 May 2019 17:21:39 +0100
Steven Price wrote:
> >> +static void panfrost_perfcnt_setup(struct panfrost_device *pfdev)
> >> +{
> >> + u32 cfg;
> >> +
> >> + /*
> >> +* Always use address space 0 for now.
> >> +* FIXME: this needs to be updated when we start using different
> >> +* address space.
> >> +*/
> >> + cfg = GPU_PERFCNT_CFG_AS(0);
> >> + if (panfrost_model_cmp(pfdev, 0x1000) >= 0)
> >
> > You've got a couple of these. Perhaps we should add either a
> > model_is_bifrost() helper or an is_bifrost variable to use.
> >
> >> + cfg |= GPU_PERFCNT_CFG_SETSEL(1);
>
> mali_kbase only sets this if CONFIG_MALI_PRFCNT_SET_SECONDARY is defined
> - i.e. this selects a different selection of counters. At at the very
> least this should be an optional flag that can be set, but I suspect the
> primary set are the ones you are interested in.
I'll add a param to pass the set of counters to monitor.
>
> >> +
> >> + gpu_write(pfdev, GPU_PERFCNT_CFG,
> >> + cfg | GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_OFF));
> >> +
> >> + if (!pfdev->perfcnt->enabled)
> >> + return;
> >> +
> >> + gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0x);
> >> + gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0x);
> >> + gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0x);
> >> +
> >> + /*
> >> +* Due to PRLAM-8186 we need to disable the Tiler before we enable
> >> HW
> >> +* counters.
> >
> > Seems like you could just always apply the work-around? It doesn't
> > look like it would be much different.
>
> Technically the workaround causes the driver to perform the operation in
> the wrong order below (write TILER_EN when the mode is MANUAL) - this is
> a workaround for the hardware issue (and therefore works on that
> hardware). But I wouldn't like to say it works on other hardware which
> doesn't have the issue.
Okay, I'll keep the workaround only for HW that are impacted by the bug.
>
> >> +*/
> >> + if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> >> + gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
> >> + else
> >> + gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0x);
> >> +
> >> + gpu_write(pfdev, GPU_PERFCNT_CFG,
> >> + cfg | GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_MANUAL));
> >> +
> >> + if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> >> + gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0x);
> >> +}
[...]
> >> +int panfrost_perfcnt_init(struct panfrost_device *pfdev)
> >> +{
> >> + struct panfrost_perfcnt *perfcnt;
> >> + struct drm_gem_shmem_object *bo;
> >> + size_t size;
> >> + u32 status;
> >> + int ret;
> >> +
> >> + if (panfrost_has_hw_feature(pfdev, HW_FEATURE_V4)) {
> >> + unsigned int ncoregroups;
> >> +
> >> + ncoregroups = hweight64(pfdev->features.l2_present);
> >> + size = ncoregroups * BLOCKS_PER_COREGROUP *
> >> + COUNTERS_PER_BLOCK * BYTES_PER_COUNTER;
> >> + } else {
> >> + unsigned int nl2c, ncores;
> >> +
> >> + /*
> >> +* TODO: define a macro to extract the number of l2 caches
> >> from
> >> +* mem_features.
> >> +*/
> >> + nl2c = ((pfdev->features.mem_features >> 8) & GENMASK(3,
> >> 0)) + 1;
> >> +
> >> + /*
> >> +* The ARM driver is grouping cores per core group and then
> >> +* only using the number of cores in group 0 to calculate
> >> the
> >> +* size. Not sure why this is done like that, but I guess
> >> +* shader_present will only show cores in the first group
> >> +* anyway.
> >> +*/
>
> I'm not sure I understand this comment. I'm not sure which version of
> the driver you are looking at, but shader_present should contain all the
> cores (in every core group).
>
> The kbase driver (in panfrost's git tree[1]), contains:
>
> base_gpu_props *props = &kbdev->gpu_props.props;
> u32 nr_l2 = props->l2_props.num_l2_slices;
> u64 core_mask = props->coherency_info.group[0].core_mask;
>
> u32 nr_blocks = fls64(core_mask);
>
> /* JM and tiler counter blocks are always present */
> dump_size = (2 + nr_l2 + nr_blocks) *
> NR_CNT_PER_BLOCK *
> NR_BYTES_PER_CNT;
>
> [1]
> https://gitlab.freedesktop.org/panfrost/mali_kbase/blob/master/driver/product/kernel/drivers/gpu/arm/midgard/mali_kbase_vinstr.c
>
> i.e. the number of cores + number of L2 caches (plus 2 for JM/tiler)
> multiplied by the block size.
Actually, I was referring to what's done in
kbase_gpuprops_construct_coherent_groups() [2].
>
> Also another difference with the below is that fls64 and hwe
Re: [PATCH RFC v2 0/6] ARM: qcom: initial Nexus 5 display support
On Thu, May 9, 2019 at 4:04 AM Brian Masney wrote: > Here is a patch series that adds initial display support for the LG > Nexus 5 (hammerhead) phone. It's not fully working so that's why some > of these patches are RFC until we can get it fully working. > > The phones boots into terminal mode, however there is a several second > (or more) delay when writing to tty1 compared to when the changes are > actually shown on the screen. The following errors are in dmesg: I tested to apply patches 2-6 and got the console up on the phone as well. I see the same timouts, and I also notice the update is slow in the display, as if the DSI panel was running in low power (LP) mode. Was booting this to do some other work, but happy to see the progress! Yours, Linus Walleij
[Bug 110783] Mesa 19.1 rc crashing MPV with VAAPI
https://bugs.freedesktop.org/show_bug.cgi?id=110783 --- Comment #1 from AngryPenguin --- Created attachment 144363 --> https://bugs.freedesktop.org/attachment.cgi?id=144363&action=edit inxi -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110783] Mesa 19.1 rc crashing MPV with VAAPI
https://bugs.freedesktop.org/show_bug.cgi?id=110783 Bug ID: 110783 Summary: Mesa 19.1 rc crashing MPV with VAAPI Product: Mesa Version: 19.1 Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/r600 Assignee: dri-devel@lists.freedesktop.org Reporter: angrypenguinpol...@gmail.com QA Contact: dri-devel@lists.freedesktop.org Created attachment 144362 --> https://bugs.freedesktop.org/attachment.cgi?id=144362&action=edit xorg Hi. I noticed that after the Mesa upgrade from stable 19.0X to 19.1rc when trying to play MPV video with VAAPI it crashing MPV with this error: Playing: /VID_20150204_135614.m4v (+) Video --vid=1 (*) (mpeg4 17.570fps) (+) Audio --aid=1 --alang=eng (*) (amr_nb 1ch 8000Hz) AO: [pulse] 8000Hz mono 1ch float VO: [vdpau] 640x480 yuv420p [vo/vdpau] Compositing window manager detected. Assuming timing info is inaccurate. EE ../src/gallium/drivers/r600/r600_shader.c:4290 tgsi_unsupported - DIV tgsi opcode unsupported EE ../src/gallium/drivers/r600/r600_shader.c:185 r600_pipe_shader_create - translation from TGSI failed ! EE ../src/gallium/drivers/r600/r600_state_common.c:879 r600_shader_select - Failed to build shader variant (type=5) -22 EE ../src/gallium/drivers/r600/r600_shader.c:4290 tgsi_unsupported - DIV tgsi opcode unsupported EE ../src/gallium/drivers/r600/r600_shader.c:185 r600_pipe_shader_create - translation from TGSI failed ! EE ../src/gallium/drivers/r600/r600_state_common.c:879 r600_shader_select - Failed to build shader variant (type=5) -22 Violation of memory protection (memory dump) <- (last line translated) It worked fine on Mesa 19.0X, it began crash after upgrade to Mesa 19.1rc2 and 19.1rc3. Trying libva 2.4.0 and also libva 2.4.1 - same results. MPV crashing at default launch options. Crashing also with --vo=vaapi or --vo=vdpau. Not crashing with --vo=gpu or --vo=xv Worth to add other video players like VLC not crashing wit VAAPI. Only MPV. Details: Linux OpenMandriva Lx 4 x86_64 Kernel 5.1.5 Mesa 19.1rc2/3 libva 2.4.0/2.4.1 Tested on two computers with R600, one PC with Radeon HD5850 and notebook with Radeon HD5650m. In attachment output of xorg log. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v2 1/7] drm: make drm/drm_auth.h self contained
Hi Jani. On Tue, May 28, 2019 at 03:54:48PM +0300, Jani Nikula wrote: > On Sun, 26 May 2019, Sam Ravnborg wrote: > > Do not require users of include/drm/drm_auth.h to include > > other files just to let it build. > > > > Signed-off-by: Sam Ravnborg > > Cc: Maarten Lankhorst > > Cc: Maxime Ripard > > Cc: Sean Paul > > Cc: David Airlie > > Cc: Daniel Vetter > > --- > > include/drm/drm_auth.h | 11 +-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h > > index 871008118bab..6bf8b2b78991 100644 > > --- a/include/drm/drm_auth.h > > +++ b/include/drm/drm_auth.h > > @@ -1,3 +1,6 @@ > > +#ifndef _DRM_AUTH_H_ > > +#define _DRM_AUTH_H_ > > + > > It's a bit of a bikeshed and this got applied already, but I think the > copyright/license comment should be the first thing in any file, and the > ifdefs should come after that. Using SPDX headers mandate this anyway. I was inspired by other files when I did this change. But you are right, this was a bad change. For future changes I will keep this in mind. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v2 1/7] drm: make drm/drm_auth.h self contained
On Sun, 26 May 2019, Sam Ravnborg wrote: > Do not require users of include/drm/drm_auth.h to include > other files just to let it build. > > Signed-off-by: Sam Ravnborg > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > --- > include/drm/drm_auth.h | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h > index 871008118bab..6bf8b2b78991 100644 > --- a/include/drm/drm_auth.h > +++ b/include/drm/drm_auth.h > @@ -1,3 +1,6 @@ > +#ifndef _DRM_AUTH_H_ > +#define _DRM_AUTH_H_ > + It's a bit of a bikeshed and this got applied already, but I think the copyright/license comment should be the first thing in any file, and the ifdefs should come after that. Using SPDX headers mandate this anyway. BR, Jani. > /* > * Internal Header for the Direct Rendering Manager > * > @@ -25,8 +28,12 @@ > * OTHER DEALINGS IN THE SOFTWARE. > */ > > -#ifndef _DRM_AUTH_H_ > -#define _DRM_AUTH_H_ > +#include > +#include > +#include > + > +struct drm_file; > +struct drm_hw_lock; > > /* > * Legacy DRI1 locking data structure. Only here instead of in drm_legacy.h > for -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 06/10] drm: rcar-du: lvds: Add support for dual-link mode
Hi Jacopo,
On Tue, May 28, 2019 at 11:35:50AM +0200, Jacopo Mondi wrote:
> Hi Laurent,
> a small note.
>
> On Sun, May 12, 2019 at 12:06:58AM +0300, Laurent Pinchart wrote:
> > In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and
> > sends odd-numbered pixels to the LVDS1 encoder for transmission on a
> > separate link.
> >
> > To implement support for this mode of operation, determine if the LVDS
> > connection operates in dual-link mode by querying the next device in the
> > pipeline, locate the companion encoder, and control it directly through
> > its bridge operations.
> >
> > Signed-off-by: Laurent Pinchart
> > Reviewed-by: Jacopo Mondi
> > ---
> > drivers/gpu/drm/rcar-du/rcar_lvds.c | 104
> > drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 ++
> > 2 files changed, 96 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index a331f0c32187..f7e4710fe33f 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -66,6 +66,9 @@ struct rcar_lvds {
> >
> > struct drm_display_mode display_mode;
> > enum rcar_lvds_mode mode;
> > +
> > + struct drm_bridge *companion;
> > + bool dual_link;
> > };
> >
> > #define bridge_to_rcar_lvds(bridge) \
> > @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> > {
> > struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> > const struct drm_display_mode *mode = &lvds->display_mode;
> > - /*
> > -* FIXME: We should really retrieve the CRTC through the state, but how
> > -* do we get a state pointer?
> > -*/
> > - struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
> > u32 lvdhcr;
> > u32 lvdcr0;
> > int ret;
> > @@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> > if (ret < 0)
> > return;
> >
> > + /* Enable the companion LVDS encoder in dual-link mode. */
> > + if (lvds->dual_link && lvds->companion)
> > + lvds->companion->funcs->enable(lvds->companion);
> > +
> > /*
> > * Hardcode the channels and control signals routing for now.
> > *
> > @@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge
> > *bridge)
> > rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
> >
> > if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> > - /* Disable dual-link mode. */
> > - rcar_lvds_write(lvds, LVDSTRIPE, 0);
> > + /*
> > +* Configure vertical stripe based on the mode of operation of
> > +* the connected device.
> > +*/
> > + rcar_lvds_write(lvds, LVDSTRIPE,
> > + lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
> > }
> >
> > - /* PLL clock configuration. */
> > - lvds->info->pll_setup(lvds, mode->clock * 1000);
> > + /*
> > +* PLL clock configuration on all instances but the companion in
> > +* dual-link mode.
> > +*/
> > + if (!lvds->dual_link || lvds->companion)
> > + lvds->info->pll_setup(lvds, mode->clock * 1000);
> >
> > /* Set the LVDS mode and select the input. */
> > lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
> > - if (drm_crtc_index(crtc) == 2)
> > - lvdcr0 |= LVDCR0_DUSEL;
> > +
> > + if (lvds->bridge.encoder) {
> > + /*
> > +* FIXME: We should really retrieve the CRTC through the state,
> > +* but how do we get a state pointer?
> > +*/
> > + if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)
> > + lvdcr0 |= LVDCR0_DUSEL;
> > + }
> > +
> > rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> >
> > /* Turn all the channels on. */
> > @@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge
> > *bridge)
> > rcar_lvds_write(lvds, LVDCR1, 0);
> > rcar_lvds_write(lvds, LVDPLLCR, 0);
> >
> > + /* Disable the companion LVDS encoder in dual-link mode. */
> > + if (lvds->dual_link && lvds->companion)
> > + lvds->companion->funcs->disable(lvds->companion);
> > +
> > clk_disable_unprepare(lvds->clocks.mod);
> > }
> >
> > @@ -628,10 +650,54 @@ static const struct drm_bridge_funcs
> > rcar_lvds_bridge_ops = {
> > .mode_set = rcar_lvds_mode_set,
> > };
> >
> > +bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> > +{
> > + struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> > +
> > + return lvds->dual_link;
> > +}
> > +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link);
> > +
> > /*
> > -
> > * Probe & Remove
> > */
> >
> > +static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
> > +{
> > + const struct of_device_id *match;
> > + struct device_node *companion;
> > + struct device *dev = lvds->dev;
> > + int ret = 0;
> > +
> > + /* Locate the companion LVDS encoder for dual-link operation, i
