[Bug 201273] Fatal error during GPU init amdgpu RX560
https://bugzilla.kernel.org/show_bug.cgi?id=201273 --- Comment #23 from quirin.blae...@freenet.de --- Created attachment 280009 --> https://bugzilla.kernel.org/attachment.cgi?id=280009&action=edit dmesg amd_gpu_firmware_info (In reply to Alex Deucher from comment #22) > You'll also need this updated firmware: > https://people.freedesktop.org/~agd5f/radeon_ucode/polaris11_k_mc.bin I've downloaded this Firmware and copied it to /lib/firmware/amdgpu. At next boot system crashed with a slightly modified behavior: plymouth low res -> clear -> backlights off -> backlights on -> plymouth high res -> crash Last message: Starting X display manager Archive contains dmesg (normal) and amd_gpu_firmware_info from /sys/kernel/debug/dri/1/ -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108992] Regression: Lenovo e585 (ryzen 2500u) freezes during boot with 4.20-rc5/rc6, amdgpu error
https://bugs.freedesktop.org/show_bug.cgi?id=108992 --- Comment #7 from chris --- Yes, same here. The system boots until GDM wants to start, then it freezes with the mentioned amdgpu error. Disabling amdgpu let the system start up completely including gdm. -- 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/etnaviv: fix broken build
Fix a broken build because of a typo in
"drm/scheduler: Add drm_sched_suspend/resume_timeout()".
Signed-off-by: Christian König
---
drivers/gpu/drm/etnaviv/etnaviv_dump.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_dump.c
b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
index fd6bad2100cf..a07c828c6a9a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_dump.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
@@ -135,13 +135,13 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu)
mmu_size + gpu->buffer.size;
/* Add in the active command buffers */
- spin_lock_irqsave(&sched->job_list_lock, flags);
+ spin_lock_irqsave(&gpu->sched->job_list_lock, flags);
list_for_each_entry(s_job, &gpu->sched.ring_mirror_list, node) {
submit = to_etnaviv_submit(s_job);
file_size += submit->cmdbuf.size;
n_obj++;
}
- spin_unlock_irqrestore(&sched->job_list_lock, flags);
+ spin_unlock_irqrestore(&gpu->sched->job_list_lock, flags);
/* Add in the active buffer objects */
list_for_each_entry(vram, &gpu->mmu->mappings, mmu_node) {
@@ -183,14 +183,14 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu)
gpu->buffer.size,
etnaviv_cmdbuf_get_va(&gpu->buffer));
- spin_lock_irqsave(&sched->job_list_lock, flags);
+ spin_lock_irqsave(&gpu->sched->job_list_lock, flags);
list_for_each_entry(s_job, &gpu->sched.ring_mirror_list, node) {
submit = to_etnaviv_submit(s_job);
etnaviv_core_dump_mem(&iter, ETDUMP_BUF_CMD,
submit->cmdbuf.vaddr, submit->cmdbuf.size,
etnaviv_cmdbuf_get_va(&submit->cmdbuf));
}
- spin_unlock_irqrestore(&sched->job_list_lock, flags);
+ spin_unlock_irqrestore(&gpu->sched->job_list_lock, flags);
/* Reserve space for the bomap */
if (n_bomap_pages) {
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: rcar-du: Convert live DT patches to sugar syntax
Hi Laurent,
On 29/11/2018 12:43, Laurent Pinchart wrote:
> Hi Geert,
>
> Thank you for the patch.
>
> On Thursday, 29 November 2018 12:58:50 EET Geert Uytterhoeven wrote:
>> Using overlay sugar syntax makes the DTS files easier to read (and
>> write).
>>
>> Overlay syntactic sugar for generating target-path fragments is
>> supported by the version of dtc supplied with the kernel since commit
>> 50aafd60898a8b3e ("scripts/dtc: Update to upstream version
>> v1.4.6-21-g84e414b0b5bc").
>>
>> Signed-off-by: Geert Uytterhoeven
>> ---
>> The generated dtbs before/after are identical.
>
> I was going to ask whether you had tested the patch, but this should be good
> enough :-)
>
> The result is nicer to read, thanks.
>
I agree!
FWIW:
Reviewed-by: Kieran Bingham
> Reviewed-by: Laurent Pinchart
>
> and taken in my tree.
I was hoping to reference this patch - but I can't see it in a tree
anywhere.
Is there a branch/repo that this is available in now?
--
Regards
Kieran
>
>> ---
>> .../drm/rcar-du/rcar_du_of_lvds_r8a7790.dts | 93 +--
>> .../drm/rcar-du/rcar_du_of_lvds_r8a7791.dts | 53 +--
>> .../drm/rcar-du/rcar_du_of_lvds_r8a7793.dts | 53 +--
>> .../drm/rcar-du/rcar_du_of_lvds_r8a7795.dts | 53 +--
>> .../drm/rcar-du/rcar_du_of_lvds_r8a7796.dts | 53 +--
>> 5 files changed, 135 insertions(+), 170 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
>> b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts index
>> 579753e04f3b3712..8bee4e787a0ab4eb 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
>> @@ -7,70 +7,63 @@
>>
>> /dts-v1/;
>> /plugin/;
>> -/ {
>> -fragment@0 {
>> -target-path = "/";
>> -__overlay__ {
>> -#address-cells = <2>;
>> -#size-cells = <2>;
>>
>> -lvds@feb9 {
>> -compatible = "renesas,r8a7790-lvds";
>> -reg = <0 0xfeb9 0 0x1c>;
>> +&{/} {
>> +#address-cells = <2>;
>> +#size-cells = <2>;
>>
>> -ports {
>> -#address-cells = <1>;
>> -#size-cells = <0>;
>> +lvds@feb9 {
>> +compatible = "renesas,r8a7790-lvds";
>> +reg = <0 0xfeb9 0 0x1c>;
>>
>> -port@0 {
>> -reg = <0>;
>> -lvds0_input: endpoint {
>> -};
>> -};
>> -port@1 {
>> -reg = <1>;
>> -lvds0_out: endpoint {
>> -};
>> -};
>> +ports {
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +
>> +port@0 {
>> +reg = <0>;
>> +lvds0_input: endpoint {
>> };
>> };
>> -
>> -lvds@feb94000 {
>> -compatible = "renesas,r8a7790-lvds";
>> -reg = <0 0xfeb94000 0 0x1c>;
>> -
>> -ports {
>> -#address-cells = <1>;
>> -#size-cells = <0>;
>> -
>> -port@0 {
>> -reg = <0>;
>> -lvds1_input: endpoint {
>> -};
>> -};
>> -port@1 {
>> -reg = <1>;
>> -lvds1_out: endpoint {
>> -};
>> -};
>> +port@1 {
>> +reg = <1>;
>> +lvds0_out: endpoint {
>> };
>> };
>> };
>> };
>>
>> -fragment@1 {
>> -target-path = "/display@feb0/ports";
>> -__overlay__ {
>> -port@1 {
>> -endpoint {
>> -remote-endpoint = <&lvds0_input>;
>> +lvds@feb94000 {
>> +compatible = "renesas,r8a7790-lvds";
>> +reg = <0 0xfeb94000 0 0x1c>;
>> +
>> +ports {
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +
>> +port@0 {
>> +
[Bug 108992] Regression: Lenovo e585 (ryzen 2500u) freezes during boot with 4.20-rc5/rc6, amdgpu error
https://bugs.freedesktop.org/show_bug.cgi?id=108992 --- Comment #6 from Brian Schott --- To clarify, the system can boot with the amdgpu module, but it will lock up when LightDM/X starts. Booting with the amdgpu module blacklisted works. -- 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
[GIT PULL v2] exynos-drm-next
Hi Dave, Just adding configurable plane alpha and pixel blend mode support for FIMD device I missed. Please kindly let me know if there is any problem. Thanks, Inki Dae The following changes since commit 2a3c83f5fe0770d13bbb71b23674886ff4111f44: Merge tag 'vmwgfx-next-2018-12-13' of git://people.freedesktop.org/~thomash/linux into drm-next (2018-12-14 04:57:45 +1000) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos tags/exynos-drm-next-for-v4.21-v2 for you to fetch changes up to 3b5129b3a7c62fdec9cc69b1b3f20917c36ab5d4: drm/exynos: fimd: Make pixel blend mode configurable (2018-12-14 15:46:15 +0900) Add configurable plane alpha and pixel blend mode support - This patch series adds configurable plane alpha and pixel blend mode support for FIMD device driver. Christoph Manszewski (2): drm/exynos: fimd: Make plane alpha configurable drm/exynos: fimd: Make pixel blend mode configurable drivers/gpu/drm/exynos/exynos_drm_fimd.c | 123 +-- include/video/samsung_fimd.h | 10 +++ 2 files changed, 110 insertions(+), 23 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/xen-front: Make shmem backed display buffer coherent
On 12/13/18 5:48 PM, Daniel Vetter wrote:
On Thu, Dec 13, 2018 at 12:17:54PM +0200, Oleksandr Andrushchenko wrote:
Daniel, could you please comment?
Cross-revieweing someone else's stuff would scale better,
fair enough
I don't think
I'll get around to anything before next year.
I put you on CC explicitly because you had comments on other patch [1]
and this one tries to solve the issue raised (I tried to figure out
at [2] if this is the way to go, but it seems I have no alternative here).
While at it [3] (I hope) addresses your comments and the series just
needs your single ack/nack to get in: all the rest ack/r-b are already
there. Do you mind looking at it?
-Daniel
Thank you very much for your time,
Oleksandr
Thank you
On 11/27/18 12:32 PM, Oleksandr Andrushchenko wrote:
From: Oleksandr Andrushchenko
When GEM backing storage is allocated with drm_gem_get_pages
the backing pages may be cached, thus making it possible that
the backend sees only partial content of the buffer which may
lead to screen artifacts. Make sure that the frontend's
memory is coherent and the backend always sees correct display
buffer content.
Fixes: c575b7eeb89f ("drm/xen-front: Add support for Xen PV display frontend")
Signed-off-by: Oleksandr Andrushchenko
---
drivers/gpu/drm/xen/xen_drm_front_gem.c | 62 +++--
1 file changed, 48 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c
b/drivers/gpu/drm/xen/xen_drm_front_gem.c
index 47ff019d3aef..c592735e49d2 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
@@ -33,8 +33,11 @@ struct xen_gem_object {
/* set for buffers allocated by the backend */
bool be_alloc;
- /* this is for imported PRIME buffer */
- struct sg_table *sgt_imported;
+ /*
+* this is for imported PRIME buffer or the one allocated via
+* drm_gem_get_pages.
+*/
+ struct sg_table *sgt;
};
static inline struct xen_gem_object *
@@ -77,10 +80,21 @@ static struct xen_gem_object *gem_create_obj(struct
drm_device *dev,
return xen_obj;
}
+struct sg_table *xen_drm_front_gem_get_sg_table(struct drm_gem_object *gem_obj)
+{
+ struct xen_gem_object *xen_obj = to_xen_gem_obj(gem_obj);
+
+ if (!xen_obj->pages)
+ return ERR_PTR(-ENOMEM);
+
+ return drm_prime_pages_to_sg(xen_obj->pages, xen_obj->num_pages);
+}
+
static struct xen_gem_object *gem_create(struct drm_device *dev, size_t size)
{
struct xen_drm_front_drm_info *drm_info = dev->dev_private;
struct xen_gem_object *xen_obj;
+ struct address_space *mapping;
int ret;
size = round_up(size, PAGE_SIZE);
@@ -113,10 +127,14 @@ static struct xen_gem_object *gem_create(struct
drm_device *dev, size_t size)
xen_obj->be_alloc = true;
return xen_obj;
}
+
/*
* need to allocate backing pages now, so we can share those
* with the backend
*/
+ mapping = xen_obj->base.filp->f_mapping;
+ mapping_set_gfp_mask(mapping, GFP_USER | __GFP_DMA32);
+
xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
xen_obj->pages = drm_gem_get_pages(&xen_obj->base);
if (IS_ERR_OR_NULL(xen_obj->pages)) {
@@ -125,8 +143,27 @@ static struct xen_gem_object *gem_create(struct drm_device
*dev, size_t size)
goto fail;
}
+ xen_obj->sgt = xen_drm_front_gem_get_sg_table(&xen_obj->base);
+ if (IS_ERR_OR_NULL(xen_obj->sgt)){
+ ret = PTR_ERR(xen_obj->sgt);
+ xen_obj->sgt = NULL;
+ goto fail_put_pages;
+ }
+
+ if (!dma_map_sg(dev->dev, xen_obj->sgt->sgl, xen_obj->sgt->nents,
+ DMA_BIDIRECTIONAL)) {
+ ret = -EFAULT;
+ goto fail_free_sgt;
+ }
+
return xen_obj;
+fail_free_sgt:
+ sg_free_table(xen_obj->sgt);
+ xen_obj->sgt = NULL;
+fail_put_pages:
+ drm_gem_put_pages(&xen_obj->base, xen_obj->pages, true, false);
+ xen_obj->pages = NULL;
fail:
DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
return ERR_PTR(ret);
@@ -149,7 +186,7 @@ void xen_drm_front_gem_free_object_unlocked(struct
drm_gem_object *gem_obj)
struct xen_gem_object *xen_obj = to_xen_gem_obj(gem_obj);
if (xen_obj->base.import_attach) {
- drm_prime_gem_destroy(&xen_obj->base, xen_obj->sgt_imported);
+ drm_prime_gem_destroy(&xen_obj->base, xen_obj->sgt);
gem_free_pages_array(xen_obj);
} else {
if (xen_obj->pages) {
@@ -158,6 +195,13 @@ void xen_drm_front_gem_free_object_unlocked(struct
drm_gem_object *gem_obj)
xen_obj->pages);
gem_free_pages_array(xen_obj);
} else {
+
[PATCH] drm/nouveau/volt/gf117: fix speedo readout register
GF117 appears to use the same register as GK104 (but still with the
general Fermi readout mechanism).
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108980
Signed-off-by: Ilia Mirkin
---
drivers/gpu/drm/nouveau/nvkm/subdev/volt/gf100.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/volt/gf100.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/volt/gf100.c
index d9ed6925ca64..d87c2efe67e7 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/volt/gf100.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/volt/gf100.c
@@ -30,11 +30,12 @@ gf100_volt_speedo_read(struct nvkm_volt *volt)
{
struct nvkm_device *device = volt->subdev.device;
struct nvkm_fuse *fuse = device->fuse;
+ const u32 speedo_fuse = device->chipset == 0xd7 ? 0x3a8 : 0x1cc;
if (!fuse)
return -EINVAL;
- return nvkm_fuse_read(fuse, 0x1cc);
+ return nvkm_fuse_read(fuse, speedo_fuse);
}
int
--
2.18.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/nouveau/falcon: avoid touching registers if engine is off
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108980
Signed-off-by: Ilia Mirkin
---
drivers/gpu/drm/nouveau/nvkm/engine/falcon.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/falcon.c
b/drivers/gpu/drm/nouveau/nvkm/engine/falcon.c
index 816ccaedfc73..8675613e142b 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/falcon.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/falcon.c
@@ -22,6 +22,7 @@
#include
#include
+#include
#include
#include
@@ -107,8 +108,10 @@ nvkm_falcon_fini(struct nvkm_engine *engine, bool suspend)
}
}
- nvkm_mask(device, base + 0x048, 0x0003, 0x);
- nvkm_wr32(device, base + 0x014, 0x);
+ if (nvkm_mc_enabled(device, engine->subdev.index)) {
+ nvkm_mask(device, base + 0x048, 0x0003, 0x);
+ nvkm_wr32(device, base + 0x014, 0x);
+ }
return 0;
}
--
2.18.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [BUG] drm_rockchip: rk3066_hdmi: No driver support for vblank timestamp query.
Hi Johan, On Fri, Dec 14, 2018 at 2:32 AM Johan Jonker wrote: > > Bug fix? (PART 7) > > A little bit of success here. > Penguins and other colors are on the screen. > DRM and FB old style seems to work with DVI-D. > Pure HDMI and sound not tested. > I think 'someone' forgot to add HCLK_HDMI to the pmu node. > > Added a qos_hdmi idea. Let me know if that's OK? > Please advise what for qos_hdmi address I can use here. Thanks for keeping investigating this and good to hear that you've made some progress. Would you be able to send a patch following the patch submission guidelines? Please see the link below. https://www.kernel.org/doc/html/latest/process/submitting-patches.html# Best regards, Tomasz ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[git pull] drm fixes for 4.20-rc7
Hi Linus, While I hoped things would calm down, the world hasn't joined with me, but it's a few things scattered over a wide area. The i915 workarounds regression fix is probably the largest, the rest are more usual sized. We also get some new AMD PCI IDs. There is also a patch in here to MAINTAINERS to added Daniel as an official DRM toplevel co-maintainer, he's decided he wants to step up and share the glory, and he'll likely process next weeks fixes while I'm away on holidays. I'm also going to send an early drm-next to you today, it might get superseded but I'll get it out there in advance of holiday period. Dave. summary: amdgpu: - some new PCI IDs, - fixed firmware image updates - power management fixes - locking warning fix nouveau: - framebuffer flushing fix - memory leak fix - tegra device init regression fix vmwgfx: - OOM kernel memory fix - excess return in function fix i915: - the biggest fix is a regression fix where workarounds weren't getting reapplied after a gpu hang causing further crashing, this fixes the workaround application to make it happen again, - GPU hang fixes for Braswell and some GEN3 GPUs. - GVT fix for broadwell tiling rockchip: - revert to fix a regression causing a WARN on shutdown mediatek: - avoid crash attaching to non-existant bridges drm-fixes-2018-12-14: drm vmwgfx, mediatek, nouveau, amdgpu, rockchip, intel fixes The following changes since commit 40e020c129cfc991e8ab4736d2665351ffd1468d: Linux 4.20-rc6 (2018-12-09 15:31:00 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2018-12-14 for you to fetch changes up to 1df07a7f2244947c888dc35bba2b237f714afc56: Merge branch 'vmwgfx-fixes-4.20' of git://people.freedesktop.org/~thomash/linux into drm-fixes (2018-12-14 05:37:44 +1000) drm vmwgfx, mediatek, nouveau, amdgpu, rockchip, intel fixes Alex Deucher (5): drm/amdgpu: update smu firmware images for VI variants (v2) drm/amdgpu: add some additional vega10 pci ids drm/amdgpu: add some additional vega20 pci ids drm/amdkfd: add new vega10 pci ids drm/amdkfd: add new vega20 pci id Andrey Grodzovsky (1): drm/amdgpu: Fix DEBUG_LOCKS_WARN_ON(depth <= 0) in amdgpu_ctx.lock Ben Skeggs (1): drm/nouveau/kms/nv50-: also flush fb writes when rewinding push buffer Brian Norris (1): Revert "drm/rockchip: Allow driver to be shutdown on reboot/kexec" Chris Wilson (3): drm/i915/execlists: Apply a full mb before execution for Braswell drm/i915: Allocate a common scratch page drm/i915: Flush GPU relocs harder for gen3 Colin Ian King (1): drm/vmwgfx: remove redundant return ret statement Daniel Vetter (1): MAINTAINERS: Daniel for drm co-maintainer Dave Airlie (7): Merge tag 'drm-intel-fixes-2018-12-07' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Merge tag 'drm-intel-fixes-2018-12-12-1' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Merge tag 'drm-misc-fixes-2018-12-12' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge branch 'drm-fixes-4.20' of git://people.freedesktop.org/~agd5f/linux into drm-fixes Merge branch 'linux-4.20' of git://github.com/skeggsb/linux into drm-fixes Merge branch 'mediatek-drm-fixes-4.20' of https://github.com/ckhu-mediatek/linux.git-tags into drm-fixes Merge branch 'vmwgfx-fixes-4.20' of git://people.freedesktop.org/~thomash/linux into drm-fixes Evan Quan (1): drm/amdgpu: drop fclk/gfxclk ratio setting Joonas Lahtinen (1): Merge tag 'gvt-fixes-2018-12-04' of https://github.com/intel/gvt-linux into drm-intel-fixes Junwei Zhang (1): drm/amdgpu: update SMC firmware image for polaris10 variants Kenneth Feng (1): drm/amdgpu/powerplay: Apply avfs cks-off voltages on VI Lyude Paul (1): drm/nouveau/kms: Fix memory leak in nv50_mstm_del() Nicolas Boichat (1): drm/mediatek: Only try to attach bridge if there is one Thierry Reding (1): drm/nouveau/drm/nouveau: tegra: Call nouveau_drm_device_init() Thomas Hellstrom (1): drm/vmwgfx: Protect from excessive execbuf kernel memory allocations v3 Tina Zhang (1): drm/i915/gvt: Fix tiled memory decoding bug on BDW Tvrtko Ursulin (2): drm/i915: Record GT workarounds in a list drm/i915: Introduce per-engine workarounds MAINTAINERS| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c| 36 +- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 7 + drivers/gpu/drm/amd/amdkfd/kfd_device.c| 7 + drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 2 +- drivers/gpu/drm/amd/powerplay/inc/smu7_ppsmc.h | 2 + .../drm/amd/powerplay/smumgr/polaris10_smumgr.c|
linux-next: manual merge of the drm tree with the drm-fixes tree
Hi all,
Today's linux-next merge of the drm tree got a conflict in:
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
between commit:
fd567467753f ("drm/vmwgfx: Protect from excessive execbuf kernel memory
allocations v3")
from the drm-fixes tree and commit:
a64f784bb14a ("drm/ttm: initialize globals during device init (v2)")
from the drm tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
index f88247046721,154eb09aa91e..
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
@@@ -42,93 -42,3 +42,39 @@@ int vmw_mmap(struct file *filp, struct
dev_priv = vmw_priv(file_priv->minor->dev);
return ttm_bo_mmap(filp, vma, &dev_priv->bdev);
}
+
- static int vmw_ttm_mem_global_init(struct drm_global_reference *ref)
- {
- DRM_INFO("global init.\n");
- return ttm_mem_global_init(ref->object);
- }
-
- static void vmw_ttm_mem_global_release(struct drm_global_reference *ref)
- {
- ttm_mem_global_release(ref->object);
- }
-
- int vmw_ttm_global_init(struct vmw_private *dev_priv)
- {
- struct drm_global_reference *global_ref;
- int ret;
-
- global_ref = &dev_priv->mem_global_ref;
- global_ref->global_type = DRM_GLOBAL_TTM_MEM;
- global_ref->size = sizeof(struct ttm_mem_global);
- global_ref->init = &vmw_ttm_mem_global_init;
- global_ref->release = &vmw_ttm_mem_global_release;
-
- ret = drm_global_item_ref(global_ref);
- if (unlikely(ret != 0)) {
- DRM_ERROR("Failed setting up TTM memory accounting.\n");
- return ret;
- }
-
- dev_priv->bo_global_ref.mem_glob =
- dev_priv->mem_global_ref.object;
- global_ref = &dev_priv->bo_global_ref.ref;
- global_ref->global_type = DRM_GLOBAL_TTM_BO;
- global_ref->size = sizeof(struct ttm_bo_global);
- global_ref->init = &ttm_bo_global_init;
- global_ref->release = &ttm_bo_global_release;
- ret = drm_global_item_ref(global_ref);
-
- if (unlikely(ret != 0)) {
- DRM_ERROR("Failed setting up TTM buffer objects.\n");
- goto out_no_bo;
- }
-
- return 0;
- out_no_bo:
- drm_global_item_unref(&dev_priv->mem_global_ref);
- return ret;
- }
-
- void vmw_ttm_global_release(struct vmw_private *dev_priv)
- {
- drm_global_item_unref(&dev_priv->bo_global_ref.ref);
- drm_global_item_unref(&dev_priv->mem_global_ref);
- }
-
+/* struct vmw_validation_mem callback */
+static int vmw_vmt_reserve(struct vmw_validation_mem *m, size_t size)
+{
+ static struct ttm_operation_ctx ctx = {.interruptible = false,
+ .no_wait_gpu = false};
+ struct vmw_private *dev_priv = container_of(m, struct vmw_private, vvm);
+
+ return ttm_mem_global_alloc(vmw_mem_glob(dev_priv), size, &ctx);
+}
+
+/* struct vmw_validation_mem callback */
+static void vmw_vmt_unreserve(struct vmw_validation_mem *m, size_t size)
+{
+ struct vmw_private *dev_priv = container_of(m, struct vmw_private, vvm);
+
+ return ttm_mem_global_free(vmw_mem_glob(dev_priv), size);
+}
+
+/**
+ * vmw_validation_mem_init_ttm - Interface the validation memory tracker
+ * to ttm.
+ * @dev_priv: Pointer to struct vmw_private. The reason we choose a vmw
private
+ * rather than a struct vmw_validation_mem is to make sure assumption in the
+ * callbacks that struct vmw_private derives from struct vmw_validation_mem
+ * holds true.
+ * @gran: The recommended allocation granularity
+ */
+void vmw_validation_mem_init_ttm(struct vmw_private *dev_priv, size_t gran)
+{
+ struct vmw_validation_mem *vvm = &dev_priv->vvm;
+
+ vvm->reserve_mem = vmw_vmt_reserve;
+ vvm->unreserve_mem = vmw_vmt_unreserve;
+ vvm->gran = gran;
+}
pgp1O7EUgqZOg.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[WIP PATCH 13/15] drm/dp_mst: Start tracking per-port VCPI allocations
There has been a TODO waiting for quite a long time in drm_dp_mst_topology.c: /* We cannot rely on port->vcpi.num_slots to update * topology_state->avail_slots as the port may not exist if the parent * branch device was unplugged. This should be fixed by tracking * per-port slot allocation in drm_dp_mst_topology_state instead of * depending on the caller to tell us how many slots to release. */ That's not the only reason we should fix this: forcing the driver to track the VCPI allocations throughout a state's atomic check is error prone, because it means that extra care has to be taken with the order that drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() are called in in order to ensure idempotency. Currently the only driver actually using these helpers, i915, doesn't even do this correctly: multiple ->best_encoder() checks with i915's current implementation would not be idempotent and would over-allocate VCPI slots, something I learned trying to implement fallback retraining in MST. So: simplify this whole mess, and teach drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() to track the VCPI allocations for each port. This allows us to ensure idempotency without having to rely on the driver as much. Additionally: the driver doesn't need to do any kind of VCPI slot tracking anymore if it doesn't need it for it's own internal state. Additionally; this adds a new drm_dp_mst_atomic_check() helper which must be used by atomic drivers to perform validity checks for the new VCPI allocations incurred by a state. Also: update the documentation and make it more obvious that these /must/ be called by /all/ atomic drivers supporting MST. Changes since v6: - Keep a kref to all of the ports we have allocations on. This required a good bit of changing to when we call drm_dp_find_vcpi_slots(), mainly that we need to ensure that we only redo VCPI allocations on actual mode or CRTC changes, not crtc_state->active changes. Additionally, we no longer take the registration of the DRM connector for each port into account because so long as we have a kref to the port in the new or previous atomic state, the connector will stay registered. - Use the small changes to drm_dp_put_port() to add even more error checking to make misusage of the helpers more obvious. I added this after having to chase down various use-after-free conditions that started popping up from the new helpers so no one else has to troubleshoot that. - Move some accidental DRM_DEBUG_KMS() calls to DRM_DEBUG_ATOMIC() - Update documentation again, note that find/release() should both not be called on the same port in a single atomic check phase (but multiple calls to one or the other is OK) Changes since v4: - Don't skip the atomic checks for VCPI allocations if no new VCPI allocations happen in a state. This makes the next change I'm about to list here a lot easier to implement. - Don't ignore VCPI allocations on destroyed ports, instead ensure that when ports are destroyed and still have VCPI allocations in the topology state, the only state changes allowed are releasing said ports' VCPI. This prevents a state with a mix of VCPI allocations from destroyed ports, and allocations from valid ports. Changes since v3: - Don't release VCPI allocations in the topology state immediately in drm_dp_atomic_release_vcpi_slots(), instead mark them as 0 and skip over them in drm_dp_mst_duplicate_state(). This makes it so drm_dp_atomic_release_vcpi_slots() is still idempotent while also throwing warnings if the driver messes up it's book keeping and tries to release VCPI slots on a port that doesn't have any pre-existing VCPI allocation - danvet - Change mst_state/state in some debugging messages to "mst state" Changes since v2: - Use kmemdup() for duplicating MST state - danvet - Move port validation out of duplicate state callback - danvet - Handle looping through MST topology states in drm_dp_mst_atomic_check() so the driver doesn't have to do it - Fix documentation in drm_dp_atomic_find_vcpi_slots() - Move the atomic check for each individual topology state into it's own function, reduces indenting - Don't consider "stale" MST ports when calculating the bandwidth requirements. This is needed because originally we relied on the state duplication functions to prune any stale ports from the new state, which would prevent us from incorrectly considering their bandwidth requirements alongside legitimate new payloads. - Add function references in drm_dp_atomic_release_vcpi_slots() - danvet - Annotate atomic VCPI and atomic check functions with __must_check - danvet Changes since v1: - Don't use the now-removed ->atomic_check() for private objects hook, just give drivers a function to call themselves Signed-off-by: Lyude Paul Cc: Daniel Vetter --- drivers/gpu/dr
[WIP PATCH 14/15] drm/dp_mst: Check payload count in drm_dp_mst_atomic_check()
It occurred to me that we never actually check this! So let's start
doing that.
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
---
drivers/gpu/drm/drm_dp_mst_topology.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index b9374c981a5b..ebffb834f5d6 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3538,7 +3538,7 @@ drm_dp_mst_atomic_check_topology_state(struct
drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_topology_state
*mst_state)
{
struct drm_dp_vcpi_allocation *vcpi;
- int avail_slots = 63, ret;
+ int avail_slots = 63, payload_count = 0, ret;
/* There's no possible scenario where releasing VCPI or keeping it the
* same would make the state invalid
@@ -3575,6 +3575,13 @@ drm_dp_mst_atomic_check_topology_state(struct
drm_dp_mst_topology_mgr *mgr,
goto port_fail;
}
+ if (++payload_count > mgr->max_payloads) {
+ DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p has too many
payloads (max=%d)\n",
+mgr, mst_state, mgr->max_payloads);
+ ret = -EINVAL;
+ goto port_fail;
+ }
+
drm_dp_mst_topology_put_port(vcpi->port);
}
DRM_DEBUG_ATOMIC("[MST MGR:%p] mst state %p VCPI avail=%d used=%d\n",
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[WIP PATCH 09/15] drm/nouveau: Fix potential use-after-frees for MSTCs
Now that we finally have a sane way to keep port allocations around, use
it to fix the potential unchecked ->port accesses that nouveau makes by
making sure we keep the mst port allocated for as long as it's
drm_connector is accessible.
Additionally, now that we've guaranteed that mstc->port is allocated for
as long as we keep mstc around we can remove the connector registration
checks for codepaths which release payloads, allowing us to release
payloads on active topologies properly. These registration checks were
only required before in order to avoid situations where mstc->port could
technically be pointing at freed memory.
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 0f7d72518604..982054bbcc8b 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -964,7 +964,11 @@ static void
nv50_mstc_destroy(struct drm_connector *connector)
{
struct nv50_mstc *mstc = nv50_mstc(connector);
+
drm_connector_cleanup(&mstc->connector);
+ if (mstc->port)
+ drm_dp_mst_put_port_malloc(mstc->port);
+
kfree(mstc);
}
@@ -1012,6 +1016,7 @@ nv50_mstc_new(struct nv50_mstm *mstm, struct
drm_dp_mst_port *port,
drm_object_attach_property(&mstc->connector.base,
dev->mode_config.path_property, 0);
drm_object_attach_property(&mstc->connector.base,
dev->mode_config.tile_property, 0);
drm_connector_set_path_property(&mstc->connector, path);
+ drm_dp_mst_get_port_malloc(port);
return 0;
}
@@ -1077,6 +1082,7 @@ nv50_mstm_destroy_connector(struct
drm_dp_mst_topology_mgr *mgr,
drm_fb_helper_remove_one_connector(&drm->fbcon->helper,
&mstc->connector);
drm_modeset_lock(&drm->dev->mode_config.connection_mutex, NULL);
+ drm_dp_mst_put_port_malloc(mstc->port);
mstc->port = NULL;
drm_modeset_unlock(&drm->dev->mode_config.connection_mutex);
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[WIP PATCH 12/15] drm/dp_mst: Add some atomic state iterator macros
Changes since v6:
- Move EXPORT_SYMBOL() for drm_dp_mst_topology_state_funcs to this
commit
- Document __drm_dp_mst_state_iter_get() and note that it shouldn't be
called directly
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
---
drivers/gpu/drm/drm_dp_mst_topology.c | 5 +-
include/drm/drm_dp_mst_helper.h | 96 +++
2 files changed, 99 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 93f08bfd2ab3..8d94c8943ac7 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3415,10 +3415,11 @@ static void drm_dp_mst_destroy_state(struct
drm_private_obj *obj,
kfree(mst_state);
}
-static const struct drm_private_state_funcs mst_state_funcs = {
+const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs = {
.atomic_duplicate_state = drm_dp_mst_duplicate_state,
.atomic_destroy_state = drm_dp_mst_destroy_state,
};
+EXPORT_SYMBOL(drm_dp_mst_topology_state_funcs);
/**
* drm_atomic_get_mst_topology_state: get MST topology state
@@ -3502,7 +3503,7 @@ int drm_dp_mst_topology_mgr_init(struct
drm_dp_mst_topology_mgr *mgr,
drm_atomic_private_obj_init(dev, &mgr->base,
&mst_state->base,
- &mst_state_funcs);
+ &drm_dp_mst_topology_state_funcs);
return 0;
}
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 50643a39765d..b8a8d75410d0 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -637,4 +637,100 @@ int drm_dp_send_power_updown_phy(struct
drm_dp_mst_topology_mgr *mgr,
void drm_dp_mst_get_port_malloc(struct drm_dp_mst_port *port);
void drm_dp_mst_put_port_malloc(struct drm_dp_mst_port *port);
+extern const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs;
+
+/**
+ * __drm_dp_mst_state_iter_get - private atomic state iterator function for
+ * macro-internal use
+ * @state: &struct drm_atomic_state pointer
+ * @mgr: pointer to the &struct drm_dp_mst_topology_mgr iteration cursor
+ * @old_state: optional pointer to the old &struct drm_dp_mst_topology_state
+ * iteration cursor
+ * @new_state: optional pointer to the new &struct drm_dp_mst_topology_state
+ * iteration cursor
+ * @i: int iteration cursor, for macro-internal use
+ *
+ * Used by for_each_oldnew_mst_mgr_in_state(),
+ * for_each_old_mst_mgr_in_state(), and for_each_new_mst_mgr_in_state(). Don't
+ * call this directly.
+ *
+ * Returns:
+ * True if the current &struct drm_private_obj is a &struct
+ * drm_dp_mst_topology_mgr, false otherwise.
+ */
+static inline bool
+__drm_dp_mst_state_iter_get(struct drm_atomic_state *state,
+ struct drm_dp_mst_topology_mgr **mgr,
+ struct drm_dp_mst_topology_state **old_state,
+ struct drm_dp_mst_topology_state **new_state,
+ int i)
+{
+ struct __drm_private_objs_state *objs_state = &state->private_objs[i];
+
+ if (objs_state->ptr->funcs != &drm_dp_mst_topology_state_funcs)
+ return false;
+
+ *mgr = to_dp_mst_topology_mgr(objs_state->ptr);
+ if (old_state)
+ *old_state = to_dp_mst_topology_state(objs_state->old_state);
+ if (new_state)
+ *new_state = to_dp_mst_topology_state(objs_state->new_state);
+
+ return true;
+}
+
+/**
+ * for_each_oldnew_mst_mgr_in_state - iterate over all DP MST topology
+ * managers in an atomic update
+ * @__state: &struct drm_atomic_state pointer
+ * @mgr: &struct drm_dp_mst_topology_mgr iteration cursor
+ * @old_state: &struct drm_dp_mst_topology_state iteration cursor for the old
+ * state
+ * @new_state: &struct drm_dp_mst_topology_state iteration cursor for the new
+ * state
+ * @__i: int iteration cursor, for macro-internal use
+ *
+ * This iterates over all DRM DP MST topology managers in an atomic update,
+ * tracking both old and new state. This is useful in places where the state
+ * delta needs to be considered, for example in atomic check functions.
+ */
+#define for_each_oldnew_mst_mgr_in_state(__state, mgr, old_state, new_state,
__i) \
+ for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \
+ for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr),
&(old_state), &(new_state), (__i)))
+
+/**
+ * for_each_old_mst_mgr_in_state - iterate over all DP MST topology managers
+ * in an atomic update
+ * @__state: &struct drm_atomic_state pointer
+ * @mgr: &struct drm_dp_mst_topology_mgr iteration cursor
+ * @old_state: &struct drm_dp_mst_topology_state iteration cursor for the old
+ * state
+ * @__i: int iteration cursor, for macro-internal use
+ *
+ * This iterates over all DRM DP MST topology managers in an atomic update,
+ * tracking only the old state. This is useful i
[WIP PATCH 05/15] drm/dp_mst: Fix payload deallocation on hotplugs using malloc refs
Up until now, freeing payloads on remote MST hubs that just had ports
removed has almost never worked because we've been relying on port
validation in order to stop us from accessing ports that have already
been freed from memory, but ports which need their payloads released due
to being removed will never be a valid part of the topology after
they've been removed.
Since we've introduced malloc refs, we can replace all of the validation
logic in payload helpers which are used for deallocation with some
well-placed malloc krefs. This ensures that regardless of whether or not
the ports are still valid and in the topology, any port which has an
allocated payload will remain allocated in memory until it's payloads
have been removed - finally allowing us to actually release said
payloads correctly.
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 54 +++
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index ae9d019af9f2..93f08bfd2ab3 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1989,10 +1989,6 @@ static int drm_dp_payload_send_msg(struct
drm_dp_mst_topology_mgr *mgr,
u8 sinks[DRM_DP_MAX_SDP_STREAMS];
int i;
- port = drm_dp_mst_topology_get_port_validated(mgr, port);
- if (!port)
- return -EINVAL;
-
port_num = port->port_num;
mstb = drm_dp_mst_topology_get_mstb_validated(mgr, port->parent);
if (!mstb) {
@@ -2000,10 +1996,8 @@ static int drm_dp_payload_send_msg(struct
drm_dp_mst_topology_mgr *mgr,
port->parent,
&port_num);
- if (!mstb) {
- drm_dp_mst_topology_put_port(port);
+ if (!mstb)
return -EINVAL;
- }
}
txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
@@ -2032,7 +2026,6 @@ static int drm_dp_payload_send_msg(struct
drm_dp_mst_topology_mgr *mgr,
kfree(txmsg);
fail_put:
drm_dp_mst_topology_put_mstb(mstb);
- drm_dp_mst_topology_put_port(port);
return ret;
}
@@ -2137,15 +2130,16 @@ static int drm_dp_destroy_payload_step2(struct
drm_dp_mst_topology_mgr *mgr,
*/
int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
{
- int i, j;
- int cur_slots = 1;
struct drm_dp_payload req_payload;
struct drm_dp_mst_port *port;
+ int i, j;
+ int cur_slots = 1;
mutex_lock(&mgr->payload_lock);
for (i = 0; i < mgr->max_payloads; i++) {
struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
struct drm_dp_payload *payload = &mgr->payloads[i];
+ bool put_port = false;
/* solve the current payloads - compare to the hw ones
- update the hw view */
@@ -2153,12 +2147,20 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
if (vcpi) {
port = container_of(vcpi, struct drm_dp_mst_port,
vcpi);
- port = drm_dp_mst_topology_get_port_validated(mgr,
- port);
- if (!port) {
- mutex_unlock(&mgr->payload_lock);
- return -EINVAL;
+
+ /* Validated ports don't matter if we're releasing
+* VCPI
+*/
+ if (vcpi->num_slots) {
+ port = drm_dp_mst_topology_get_port_validated(
+ mgr, port);
+ if (!port) {
+ mutex_unlock(&mgr->payload_lock);
+ return -EINVAL;
+ }
+ put_port = true;
}
+
req_payload.num_slots = vcpi->num_slots;
req_payload.vcpi = vcpi->vcpi;
} else {
@@ -2190,7 +2192,7 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
}
cur_slots += req_payload.num_slots;
- if (port)
+ if (put_port)
drm_dp_mst_topology_put_port(port);
}
@@ -3005,6 +3007,8 @@ bool drm_dp_mst_allocate_vcpi(struct
drm_dp_mst_topology_mgr *mgr,
DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",
pbn, port->vcpi.num_slots);
+ /* Keep port allocated until it's payload has been removed */
+ drm_dp_mst_get_port_malloc(port);
drm_dp_mst_topology_put_port(port);
[WIP PATCH 15/15] drm/nouveau: Use atomic VCPI helpers for MST
Currently, nouveau uses the yolo method of setting up MST displays: it
uses the old VCPI helpers (drm_dp_find_vcpi_slots()) for computing the
display configuration. These helpers don't take care to make sure they
take a reference to the mstb port that they're checking, and
additionally don't actually check whether or not the topology still has
enough bandwidth to provide the VCPI tokens required.
So, drop usage of the old helpers and move entirely over to the atomic
helpers.
Changes since v5:
- Update nv50_msto_atomic_check() and nv50_mstc_atomic_check() to the
new requirements for drm_dp_atomic_find_vcpi_slots() and
drm_dp_atomic_release_vcpi_slots()
Signed-off-by: Lyude Paul
Cc: Daniel Vetter
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 52 ++---
1 file changed, 46 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 67f7bf97e5d9..df696008d205 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -762,16 +762,22 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
struct drm_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
{
- struct nv50_mstc *mstc = nv50_mstc(conn_state->connector);
+ struct drm_atomic_state *state = crtc_state->state;
+ struct drm_connector *connector = conn_state->connector;
+ struct nv50_mstc *mstc = nv50_mstc(connector);
struct nv50_mstm *mstm = mstc->mstm;
- int bpp = conn_state->connector->display_info.bpc * 3;
+ int bpp = connector->display_info.bpc * 3;
int slots;
- mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, bpp);
+ mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock,
+bpp);
- slots = drm_dp_find_vcpi_slots(&mstm->mgr, mstc->pbn);
- if (slots < 0)
- return slots;
+ if (crtc_state->connectors_changed || crtc_state->mode_changed) {
+ slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr,
+ mstc->port, mstc->pbn);
+ if (slots < 0)
+ return slots;
+ }
return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state,
mstc->native);
@@ -934,12 +940,42 @@ nv50_mstc_get_modes(struct drm_connector *connector)
return ret;
}
+static int
+nv50_mstc_atomic_check(struct drm_connector *connector,
+ struct drm_connector_state *new_conn_state)
+{
+ struct drm_atomic_state *state = new_conn_state->state;
+ struct nv50_mstc *mstc = nv50_mstc(connector);
+ struct drm_dp_mst_topology_mgr *mgr = &mstc->mstm->mgr;
+ struct drm_connector_state *old_conn_state =
+ drm_atomic_get_old_connector_state(state, connector);
+ struct drm_crtc_state *old_crtc_state;
+ struct drm_crtc *new_crtc = new_conn_state->crtc,
+ *old_crtc = old_conn_state->crtc;
+
+ if (!old_crtc)
+ return 0;
+
+ old_crtc_state = drm_atomic_get_old_crtc_state(state, old_crtc);
+ if (!old_crtc_state || !old_crtc_state->enable)
+ return 0;
+
+ if (new_crtc)
+ return 0;
+
+ /* This connector will be left without an enabled CRTC, so its VCPI
+* must be released here
+*/
+ return drm_dp_atomic_release_vcpi_slots(state, mgr, mstc->port);
+}
+
static const struct drm_connector_helper_funcs
nv50_mstc_help = {
.get_modes = nv50_mstc_get_modes,
.mode_valid = nv50_mstc_mode_valid,
.best_encoder = nv50_mstc_best_encoder,
.atomic_best_encoder = nv50_mstc_atomic_best_encoder,
+ .atomic_check = nv50_mstc_atomic_check,
};
static enum drm_connector_status
@@ -2121,6 +2157,10 @@ nv50_disp_atomic_check(struct drm_device *dev, struct
drm_atomic_state *state)
return ret;
}
+ ret = drm_dp_mst_atomic_check(state);
+ if (ret)
+ return ret;
+
return 0;
}
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[WIP PATCH 11/15] drm/nouveau: Grab payload lock in nv50_msto_payload()
Going through the currently programmed payloads isn't safe without
holding mgr->payload_lock, so actually do that and warn if anyone tries
calling nv50_msto_payload() in the future without grabbing the right
locks.
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 157d208d37b5..67f7bf97e5d9 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -680,6 +680,8 @@ nv50_msto_payload(struct nv50_msto *msto)
struct nv50_mstm *mstm = mstc->mstm;
int vcpi = mstc->port->vcpi.vcpi, i;
+ WARN_ON(!mutex_is_locked(&mstm->mgr.payload_lock));
+
NV_ATOMIC(drm, "%s: vcpi %d\n", msto->encoder.name, vcpi);
for (i = 0; i < mstm->mgr.max_payloads; i++) {
struct drm_dp_payload *payload = &mstm->mgr.payloads[i];
@@ -733,6 +735,8 @@ nv50_msto_prepare(struct nv50_msto *msto)
(0x0100 << msto->head->base.index),
};
+ mutex_lock(&mstm->mgr.payload_lock);
+
NV_ATOMIC(drm, "%s: msto prepare\n", msto->encoder.name);
if (mstc->port->vcpi.vcpi > 0) {
struct drm_dp_payload *payload = nv50_msto_payload(msto);
@@ -748,7 +752,9 @@ nv50_msto_prepare(struct nv50_msto *msto)
msto->encoder.name, msto->head->base.base.name,
args.vcpi.start_slot, args.vcpi.num_slots,
args.vcpi.pbn, args.vcpi.aligned_pbn);
+
nvif_mthd(&drm->display->disp.object, 0, &args, sizeof(args));
+ mutex_unlock(&mstm->mgr.payload_lock);
}
static int
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[WIP PATCH 04/15] drm/dp_mst: Stop releasing VCPI when removing ports from topology
This has never actually worked, and isn't needed anyway: the driver's
always going to try to deallocate VCPI when it tears down the display
that the VCPI belongs to.
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index c196fb580beb..ae9d019af9f2 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1084,8 +1084,6 @@ static void drm_dp_destroy_port(struct kref *kref)
struct drm_dp_mst_topology_mgr *mgr = port->mgr;
if (!port->input) {
- port->vcpi.num_slots = 0;
-
kfree(port->cached_edid);
/*
@@ -3381,12 +3379,6 @@ static void drm_dp_destroy_connector_work(struct
work_struct *work)
drm_dp_port_teardown_pdt(port, port->pdt);
port->pdt = DP_PEER_DEVICE_NONE;
- if (!port->input && port->vcpi.vcpi > 0) {
- drm_dp_mst_reset_vcpi_slots(mgr, port);
- drm_dp_update_payload_part1(mgr);
- drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
- }
-
drm_dp_mst_put_port_malloc(port);
send_hotplug = true;
}
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[WIP PATCH 10/15] drm/nouveau: Stop unsetting mstc->port, use malloc refs
Same as we did for i915, but for nouveau this time. Additionally, we
grab a malloc reference to the port that lasts for the entire lifetime
of nv50_mstc, which gives us the guarantee that mstc->port will always
point to valid memory for as long as the mstc stays around.
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 18 +-
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 982054bbcc8b..157d208d37b5 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -709,8 +709,7 @@ nv50_msto_cleanup(struct nv50_msto *msto)
NV_ATOMIC(drm, "%s: msto cleanup\n", msto->encoder.name);
- if (mstc->port)
- drm_dp_mst_deallocate_vcpi(&mstm->mgr, mstc->port);
+ drm_dp_mst_deallocate_vcpi(&mstm->mgr, mstc->port);
msto->mstc = NULL;
msto->head = NULL;
@@ -735,7 +734,7 @@ nv50_msto_prepare(struct nv50_msto *msto)
};
NV_ATOMIC(drm, "%s: msto prepare\n", msto->encoder.name);
- if (mstc->port && mstc->port->vcpi.vcpi > 0) {
+ if (mstc->port->vcpi.vcpi > 0) {
struct drm_dp_payload *payload = nv50_msto_payload(msto);
if (payload) {
args.vcpi.start_slot = payload->start_slot;
@@ -832,8 +831,7 @@ nv50_msto_disable(struct drm_encoder *encoder)
struct nv50_mstc *mstc = msto->mstc;
struct nv50_mstm *mstm = mstc->mstm;
- if (mstc->port)
- drm_dp_mst_reset_vcpi_slots(&mstm->mgr, mstc->port);
+ drm_dp_mst_reset_vcpi_slots(&mstm->mgr, mstc->port);
mstm->outp->update(mstm->outp, msto->head->base.index, NULL, 0, 0);
mstm->modified = true;
@@ -945,7 +943,7 @@ nv50_mstc_detect(struct drm_connector *connector, bool
force)
enum drm_connector_status conn_status;
int ret;
- if (!mstc->port)
+ if (drm_connector_is_unregistered(connector))
return connector_status_disconnected;
ret = pm_runtime_get_sync(connector->dev->dev);
@@ -966,8 +964,7 @@ nv50_mstc_destroy(struct drm_connector *connector)
struct nv50_mstc *mstc = nv50_mstc(connector);
drm_connector_cleanup(&mstc->connector);
- if (mstc->port)
- drm_dp_mst_put_port_malloc(mstc->port);
+ drm_dp_mst_put_port_malloc(mstc->port);
kfree(mstc);
}
@@ -1081,11 +1078,6 @@ nv50_mstm_destroy_connector(struct
drm_dp_mst_topology_mgr *mgr,
drm_fb_helper_remove_one_connector(&drm->fbcon->helper,
&mstc->connector);
- drm_modeset_lock(&drm->dev->mode_config.connection_mutex, NULL);
- drm_dp_mst_put_port_malloc(mstc->port);
- mstc->port = NULL;
- drm_modeset_unlock(&drm->dev->mode_config.connection_mutex);
-
drm_connector_put(&mstc->connector);
}
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[WIP PATCH 07/15] drm/nouveau: Remove bogus cleanup in nv50_mstm_add_connector()
Trying to destroy the connector using mstc->connector.funcs->destroy()
if connector initialization fails is wrong: there is no possible
codepath in nv50_mstc_new where nv50_mstm_add_connector() would return
<0 and mstc would be non-NULL.
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 26af45785939..641252208e67 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1099,11 +1099,8 @@ nv50_mstm_add_connector(struct drm_dp_mst_topology_mgr
*mgr,
int ret;
ret = nv50_mstc_new(mstm, port, path, &mstc);
- if (ret) {
- if (mstc)
- mstc->connector.funcs->destroy(&mstc->connector);
+ if (ret)
return NULL;
- }
return &mstc->connector;
}
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[WIP PATCH 00/15] MST refcounting/atomic helpers cleanup
This is a WIP version of the series I've been working on for a while now to get all of the atomic DRM drivers in the tree to use the atomic MST helpers, and to make the atomic MST helpers actually idempotent. Turns out it's a lot more difficult to do that without also fixing how port and branch device refcounting works so that it actually makes sense, since the current upstream implementation requires a ton of magic in the atomic helpers to work around properly and in many situations just plain doesn't work as intended. This patch series is starting to get bigger, and since there's still a few bits here and there regarding the new refcount implementation that I haven't quite decided on yet I figured I should get an opinion from everyone else. Currently I've got a couple of thoughts on how I could improve this further: * Get rid of drm_dp_mst_get_*_validated() entirely - I'm 90% sure that with the new refcounting scheme we might not actually need port validation at all anymore, assuming we make the use of malloc references in all of the DRM drivers. Either way, I don't think validation was ever actually a concept that worked: without malloc references, the port or branch device that's being passed to drm_dp_mst_get_*_validated() could be freed which also in turn means that that the stale pointer could in theory have gotten reused for a new port and thus-cause us to consider a freed port validated. * Get rid of drm_dp_mst_get_vcpi_slots() - with malloc references, I don't think there's any use for this either * Get rid of drm_dp_mst_reset_vcpi_slots() - I think the only time this function ever made sense was with port validation? Honestly, I wonder if we ever needed this at all... Note: I haven't applied some of the comments from the reviews for the series this is based off of: drm/dp_mst: Improve VCPI helpers, use in nouveau https://patchwork.freedesktop.org/series/51414/ This is just getting put on the ML so I can get some feedback on this. Lyude Paul (15): drm/dp_mst: Remove bogus conditional in drm_dp_update_payload_part1() drm/dp_mst: Refactor drm_dp_update_payload_part1() drm/dp_mst: Introduce new refcounting scheme for mstbs and ports drm/dp_mst: Stop releasing VCPI when removing ports from topology drm/dp_mst: Fix payload deallocation on hotplugs using malloc refs drm/i915: Keep malloc references to MST ports drm/nouveau: Remove bogus cleanup in nv50_mstm_add_connector() drm/nouveau: Remove unnecessary VCPI checks in nv50_msto_cleanup() drm/nouveau: Fix potential use-after-frees for MSTCs drm/nouveau: Stop unsetting mstc->port, use malloc refs drm/nouveau: Grab payload lock in nv50_msto_payload() drm/dp_mst: Add some atomic state iterator macros drm/dp_mst: Start tracking per-port VCPI allocations drm/dp_mst: Check payload count in drm_dp_mst_atomic_check() drm/nouveau: Use atomic VCPI helpers for MST .../gpu/dp-mst/topology-figure-1.dot | 31 + .../gpu/dp-mst/topology-figure-2.dot | 37 + .../gpu/dp-mst/topology-figure-3.dot | 40 + Documentation/gpu/drm-kms-helpers.rst | 125 ++- drivers/gpu/drm/drm_dp_mst_topology.c | 910 ++ drivers/gpu/drm/i915/intel_connector.c| 4 + drivers/gpu/drm/i915/intel_display.c | 4 + drivers/gpu/drm/i915/intel_dp_mst.c | 66 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 94 +- include/drm/drm_dp_mst_helper.h | 139 ++- 10 files changed, 1178 insertions(+), 272 deletions(-) create mode 100644 Documentation/gpu/dp-mst/topology-figure-1.dot create mode 100644 Documentation/gpu/dp-mst/topology-figure-2.dot create mode 100644 Documentation/gpu/dp-mst/topology-figure-3.dot -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[WIP PATCH 02/15] drm/dp_mst: Refactor drm_dp_update_payload_part1()
There should be no functional changes here
Signed-off-by: Lyude Paul
Cc: Juston Li
---
drivers/gpu/drm/drm_dp_mst_topology.c | 71 ---
1 file changed, 42 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 9b1b5c9b1fa0..2ab16c9e6243 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1879,39 +1879,48 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
mutex_lock(&mgr->payload_lock);
for (i = 0; i < mgr->max_payloads; i++) {
+ struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
+ struct drm_dp_payload *payload = &mgr->payloads[i];
+
/* solve the current payloads - compare to the hw ones
- update the hw view */
req_payload.start_slot = cur_slots;
- if (mgr->proposed_vcpis[i]) {
- port = container_of(mgr->proposed_vcpis[i], struct
drm_dp_mst_port, vcpi);
+ if (vcpi) {
+ port = container_of(vcpi, struct drm_dp_mst_port,
+ vcpi);
port = drm_dp_get_validated_port_ref(mgr, port);
if (!port) {
mutex_unlock(&mgr->payload_lock);
return -EINVAL;
}
- req_payload.num_slots =
mgr->proposed_vcpis[i]->num_slots;
- req_payload.vcpi = mgr->proposed_vcpis[i]->vcpi;
+ req_payload.num_slots = vcpi->num_slots;
+ req_payload.vcpi = vcpi->vcpi;
} else {
port = NULL;
req_payload.num_slots = 0;
}
- mgr->payloads[i].start_slot = req_payload.start_slot;
+ payload->start_slot = req_payload.start_slot;
/* work out what is required to happen with this payload */
- if (mgr->payloads[i].num_slots != req_payload.num_slots) {
+ if (payload->num_slots != req_payload.num_slots) {
/* need to push an update for this payload */
if (req_payload.num_slots) {
- drm_dp_create_payload_step1(mgr,
mgr->proposed_vcpis[i]->vcpi, &req_payload);
- mgr->payloads[i].num_slots =
req_payload.num_slots;
- mgr->payloads[i].vcpi = req_payload.vcpi;
- } else if (mgr->payloads[i].num_slots) {
- mgr->payloads[i].num_slots = 0;
- drm_dp_destroy_payload_step1(mgr, port,
mgr->payloads[i].vcpi, &mgr->payloads[i]);
- req_payload.payload_state =
mgr->payloads[i].payload_state;
- mgr->payloads[i].start_slot = 0;
+ drm_dp_create_payload_step1(mgr, vcpi->vcpi,
+ &req_payload);
+ payload->num_slots = req_payload.num_slots;
+ payload->vcpi = req_payload.vcpi;
+
+ } else if (payload->num_slots) {
+ payload->num_slots = 0;
+ drm_dp_destroy_payload_step1(mgr, port,
+payload->vcpi,
+payload);
+ req_payload.payload_state =
+ payload->payload_state;
+ payload->start_slot = 0;
}
- mgr->payloads[i].payload_state =
req_payload.payload_state;
+ payload->payload_state = req_payload.payload_state;
}
cur_slots += req_payload.num_slots;
@@ -1920,22 +1929,26 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
}
for (i = 0; i < mgr->max_payloads; i++) {
- if (mgr->payloads[i].payload_state == DP_PAYLOAD_DELETE_LOCAL) {
- DRM_DEBUG_KMS("removing payload %d\n", i);
- for (j = i; j < mgr->max_payloads - 1; j++) {
- memcpy(&mgr->payloads[j], &mgr->payloads[j +
1], sizeof(struct drm_dp_payload));
- mgr->proposed_vcpis[j] = mgr->proposed_vcpis[j
+ 1];
- if (mgr->proposed_vcpis[j] &&
mgr->proposed_vcpis[j]->num_slots) {
- set_bit(j + 1, &mgr->payload_mask);
- } else {
- clear_bit(j + 1, &mgr->payload_mask);
-
[WIP PATCH 08/15] drm/nouveau: Remove unnecessary VCPI checks in nv50_msto_cleanup()
There is no need to look at the port's VCPI allocation before calling
drm_dp_mst_deallocate_vcpi(), as we already have msto->disabled to let
us avoid cleaning up an msto more then once. The DP MST core will never
call drm_dp_mst_deallocate_vcpi() on it's own, which is presumably what
these checks are meant to protect against.
More importantly though, we're about to stop clearing mstc->port in the
next commit, which means if we could potentially hit a use-after-free
error if we tried to check mstc->port->vcpi here. So to make life easier
for anyone who bisects this code in the future, use msto->disabled
instead to check whether or not we need to deallocate VCPI instead.
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 641252208e67..0f7d72518604 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -704,14 +704,17 @@ nv50_msto_cleanup(struct nv50_msto *msto)
struct nv50_mstc *mstc = msto->mstc;
struct nv50_mstm *mstm = mstc->mstm;
+ if (!msto->disabled)
+ return;
+
NV_ATOMIC(drm, "%s: msto cleanup\n", msto->encoder.name);
- if (mstc->port && mstc->port->vcpi.vcpi > 0 && !nv50_msto_payload(msto))
+
+ if (mstc->port)
drm_dp_mst_deallocate_vcpi(&mstm->mgr, mstc->port);
- if (msto->disabled) {
- msto->mstc = NULL;
- msto->head = NULL;
- msto->disabled = false;
- }
+
+ msto->mstc = NULL;
+ msto->head = NULL;
+ msto->disabled = false;
}
static void
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[WIP PATCH 06/15] drm/i915: Keep malloc references to MST ports
So that the ports stay around until we've destroyed the connectors, in order to ensure that we don't pass an invalid pointer to any MST helpers once we introduce the new MST VCPI helpers. Signed-off-by: Lyude Paul --- drivers/gpu/drm/i915/intel_connector.c | 4 drivers/gpu/drm/i915/intel_dp_mst.c| 2 ++ 2 files changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_connector.c b/drivers/gpu/drm/i915/intel_connector.c index 18e370f607bc..37d2c644f4b8 100644 --- a/drivers/gpu/drm/i915/intel_connector.c +++ b/drivers/gpu/drm/i915/intel_connector.c @@ -95,6 +95,10 @@ void intel_connector_destroy(struct drm_connector *connector) intel_panel_fini(&intel_connector->panel); drm_connector_cleanup(connector); + + if (intel_connector->port) + drm_dp_mst_put_port_malloc(intel_connector->port); + kfree(connector); } diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index f05427b74e34..4d6ced34d465 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -484,6 +484,8 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo if (ret) goto err; + drm_dp_mst_get_port_malloc(port); + return connector; err: -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[WIP PATCH 01/15] drm/dp_mst: Remove bogus conditional in drm_dp_update_payload_part1()
There's no reason we need this, it's just confusing looking.
Signed-off-by: Lyude Paul
Cc: Juston Li
---
drivers/gpu/drm/drm_dp_mst_topology.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index ad0fb6d003be..9b1b5c9b1fa0 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1896,9 +1896,7 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
req_payload.num_slots = 0;
}
- if (mgr->payloads[i].start_slot != req_payload.start_slot) {
- mgr->payloads[i].start_slot = req_payload.start_slot;
- }
+ mgr->payloads[i].start_slot = req_payload.start_slot;
/* work out what is required to happen with this payload */
if (mgr->payloads[i].num_slots != req_payload.num_slots) {
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[WIP PATCH 03/15] drm/dp_mst: Introduce new refcounting scheme for mstbs and ports
The current way of handling refcounting in the DP MST helpers is really
confusing and probably just plain wrong because it's been hacked up many
times over the years without anyone actually going over the code and
seeing if things could be simplified.
To the best of my understanding, the current scheme works like this:
drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When
this refcount hits 0 for either of the two, they're removed from the
topology state, but not immediately freed. Both ports and branch devices
will reinitialize their kref once it's hit 0 before actually destroying
themselves. The intended purpose behind this is so that we can avoid
problems like not being able to free a remote payload that might still
be active, due to us having removed all of the port/branch device
structures in memory, as per:
91a25e463130 ("drm/dp/mst: deallocate payload on port destruction")
Which may have worked, but then it caused use-after-free errors. Being
new to MST at the time, I tried fixing it;
263efde31f97 ("drm/dp/mst: Get validated port ref in
drm_dp_update_payload_part1()")
But, that was broken: both drm_dp_mst_port and drm_dp_mst_branch structs
are validated in almost every DP MST helper function. Simply put, this
means we go through the topology and try to see if the given
drm_dp_mst_branch or drm_dp_mst_port is still attached to something
before trying to use it in order to avoid dereferencing freed memory
(something that has happened a LOT in the past with this library).
Because of this it doesn't actually matter whether or not we keep keep
the ports and branches around in memory as that's not enough, because
any function that validates the branches and ports passed to it will
still reject them anyway since they're no longer in the topology
structure. So, use-after-free errors were fixed but payload deallocation
was completely broken.
Two years later, AMD informed me about this issue and I attempted to
come up with a temporary fix, pending a long-overdue cleanup of this
library:
c54c7374ff44 ("drm/dp_mst: Skip validating ports during destruction, just ref")
But then that introduced use-after-free errors, so I quickly reverted
it:
9765635b3075 ("Revert "drm/dp_mst: Skip validating ports during destruction,
just ref"")
And in the process, learned that there is just no simple fix for this:
the design is just broken. Unfortuntely, the usage of these helpers are
quite broken as well. Some drivers like i915 have been smart enough to
avoid accessing any kind of information from MST port structures, but
others like nouveau have assumed, understandably so, that
drm_dp_mst_port structures are normal and can just be accessed at any
time without worrying about use-after-free errors.
After a lot of discussion, me and Daniel Vetter came up with a better
idea to replace all of this.
To summarize, since this is documented far more indepth in the
documentation this patch introduces, we make it so that drm_dp_mst_port
and drm_dp_mst_branch structures have two different classes of
refcounts: topology_kref, and malloc_kref. topology_kref corresponds to
the lifetime of the given drm_dp_mst_port or drm_dp_mst_branch in it's
given topology. Once it hits zero, any associated connectors are removed
and the branch or port can no longer be validated. malloc_kref
corresponds to the lifetime of the memory allocation for the actual
structure, and will always be non-zero so long as the topology_kref is
non-zero. This gives us a way to allow callers to hold onto port and
branch device structures past their topology lifetime, and dramatically
simplifies the lifetimes of both structures. This also finally fixes the
port deallocation problem, properly.
Additionally: since this now means that we can keep ports and branch
devices allocated in memory for however long we need, we no longer need
a significant amount of the port validation that we currently do.
Additionally, there is one last scenario that this fixes, which couldn't
have been fixed properly beforehand:
- CPU1 unrefs port from topology (refcount 1->0)
- CPU2 refs port in topology(refcount 0->1)
Since we now can guarantee memory safety for ports and branches
as-needed, we also can make our main reference counting functions fix
this problem by using kref_get_unless_zero() internally so that topology
refcounts can only ever reach 0 once.
Signed-off-by: Lyude Paul
Cc: Daniel Vetter
Cc: David Airlie
Cc: Jerry Zuo
Cc: Harry Wentland
Cc: Juston Li
---
.../gpu/dp-mst/topology-figure-1.dot | 31 ++
.../gpu/dp-mst/topology-figure-2.dot | 37 ++
.../gpu/dp-mst/topology-figure-3.dot | 40 ++
Documentation/gpu/drm-kms-helpers.rst | 125 -
drivers/gpu/drm/drm_dp_mst_topology.c | 512 +-
include/drm/drm_dp_mst_helper.h | 19 +-
6 files changed, 637 insertions(+), 127 deletions(-)
create mode 100644 Documentation/gpu/dp-mst/topology-figure-1.dot
create mode 100644 Do
[Bug 109060] android-x86 64bit builds crashes due to android/gralloc_handle.h
https://bugs.freedesktop.org/show_bug.cgi?id=109060 Bug ID: 109060 Summary: android-x86 64bit builds crashes due to android/gralloc_handle.h Product: DRI Version: DRI git Hardware: x86-64 (AMD64) OS: other Status: NEW Severity: blocker Priority: medium Component: libdrm Assignee: dri-devel@lists.freedesktop.org Reporter: issor.or...@gmail.com Created attachment 142809 --> https://bugs.freedesktop.org/attachment.cgi?id=142809&action=edit draft patch that solves the crashes Hi, there is a series of Android apps using 32bit libs on 64bit Android that are crashing (Jackpal terminal, Sky Force, Olympus Rising, Vulkan V1, 3dmark benchmarks and many others) The cause is the difference in size of gralloc handle structs created by 32bit libraries and 64bit architetures libraries. The problem was observed and patched in gbm_gralloc in the past by Chih-Wei Huang, but it was not taken into account in later evolution, now with android/gralloc_handle.h in libdrm the problem is again present, because the handle sizes in 32bit and 64bit are again different. The attached draft patch solves the issue, tested on all apps that were crashing Please review and provide feedback Mauro -- 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 2/4] drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well
On Thu, 2018-12-13 at 07:18 +0200, Ville Syrjälä wrote:
> On Wed, Dec 12, 2018 at 04:32:02PM -0800, Dhinakaran Pandiyan wrote:
> > On Tue, 2018-11-20 at 18:13 +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä
> > >
> > > Fill out the AVI infoframe quantization range bits using
> > > drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI encoder as
> > > well.
> > >
> > > Signed-off-by: Ville Syrjälä
> > > ---
> > > drivers/gpu/drm/i915/intel_sdvo.c | 19 ++-
> > > 1 file changed, 10 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> > > b/drivers/gpu/drm/i915/intel_sdvo.c
> > > index 1277d31adb54..9c16e273fb8d 100644
> > > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > > @@ -984,6 +984,8 @@ static bool
> > > intel_sdvo_set_avi_infoframe(struct
> > > intel_sdvo *intel_sdvo,
> > >const struct intel_crtc_state
> > > *pipe_config,
> > >const struct
> > > drm_connector_state *conn_state)
> > > {
> > > + const struct drm_display_mode *adjusted_mode =
> > > + &pipe_config->base.adjusted_mode;
> > > uint8_t sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
> > > union hdmi_infoframe frame;
> > > int ret;
> > > @@ -991,20 +993,19 @@ static bool
> > > intel_sdvo_set_avi_infoframe(struct
> > > intel_sdvo *intel_sdvo,
> > >
> > > ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> > > conn_state-
> > > > connector,
> > >
> > > -&pipe_config-
> > > > base.adjusted_mode);
> > >
> > > +adjusted_mode);
> > > if (ret < 0) {
> > > DRM_ERROR("couldn't fill AVI infoframe\n");
> > > return false;
> > > }
> > >
> > > - if (intel_sdvo->rgb_quant_range_selectable) {
> > > - if (pipe_config->limited_color_range)
> > > - frame.avi.quantization_range =
> > > - HDMI_QUANTIZATION_RANGE_LIMITED;
> > > - else
> > > - frame.avi.quantization_range =
> > > - HDMI_QUANTIZATION_RANGE_FULL;
> > > - }
> > > + drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> > > +conn_state->connector,
> > > +adjusted_mode,
> > > +pipe_config-
> > > > limited_color_range ?
> > >
> > > +rgb_quant_range_selectableTE
> > > D :
> > > +HDMI_QUANTIZATION_RANGE_FULL
> > > ,
> > > +intel_sdvo-
> > > > rgb_quant_range_selectable);
> >
> > Seems like avi.quantization_range can now get set to _LIMITED or
> > _FULL
> > even when ->rgb_quant_range_selectable == false, i.e., it is not
> > _DEFAULT anymore. Is that change in behavior intended?
>
> ->quant_range_selectable will be passed to
> drm_hdmi_avi_infoframe_quant_range() which will do the right thing
> with
> it.
>
> That said, there is a slight behavioural change in that it will set
Okay, I was indeed referring to this case.
> the Q bit even with QS==1 iff the quantization range matches the
> default quantization range for the mode. I noted this in the radeon
> patch but forgot to mention it here.
I'll let someone else with knowledge of HDMI to review this behavioral
change. I'm trying to get hold of the HDMI spec now and will review if
this hasn't been looked at by then.
>
> >
> >
> > >
> > > len = hdmi_infoframe_pack(&frame, sdvo_data,
> > > sizeof(sdvo_data));
> > > if (len < 0)
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Remove complete task from TODO documentation
This patch remove the follow complete task from TODO documentation: drm_mode_config.crtc_idr is misnamed, since it contains all KMS object. Should be renamed to drm_mode_config.object_idr. Signed-off-by: Shayenne da Luz Moura --- Documentation/gpu/todo.rst | 3 --- 1 file changed, 3 deletions(-) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 14191b64446d..41da7b06195c 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -354,9 +354,6 @@ KMS cleanups Some of these date from the very introduction of KMS in 2008 ... -- drm_mode_config.crtc_idr is misnamed, since it contains all KMS object. Should - be renamed to drm_mode_config.object_idr. - - drm_display_mode doesn't need to be derived from drm_mode_object. That's leftovers from older (never merged into upstream) KMS designs where modes where set using their ID, including support to add/remove modes. -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/2] Add gamma/degamma LUT validation helper
Previous version of this series was here: https://lists.freedesktop.org/archives/dri-devel/2018-December/200178.html Gamma and degamma LUT's uploaded by userspace need to be checked to ensure they're valid tables and that they meet any additional constraints of a given platform's hardware. Let's add a DRM helper that drivers can call to perform some common LUT sanity tests that are likely to be useful on multiple platforms: - LUT entries are always increasing or flat, never decreasing - LUT entries have equal red, green, and blue values for each entry - LUT size is valid (i.e., it's a multiple of sizeof(struct drm_color_lut)) The size test will always be performed (since it's verifying that the proper ABI was followed), but the other two tests are optional and will only be applied as requested by the driver. This revision incorporates Brian Starkey's suggestion to combine the separate helpers into a single function that takes a bitmask of tests to apply. It also adds an additional LUT size test inspired by the ARM malidp driver. Matt Roper (2): drm: Add color management LUT validation helper (v2) drm/i915: Validate userspace-provided color management LUT's (v2) drivers/gpu/drm/drm_color_mgmt.c | 64 ++ drivers/gpu/drm/i915/intel_color.c | 19 +++ include/drm/drm_color_mgmt.h | 5 +++ 3 files changed, 88 insertions(+) -- 2.14.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm: Add color management LUT validation helper (v2)
Some hardware may place additional restrictions on the gamma/degamma
curves described by our LUT properties. E.g., that a gamma curve never
decreases or that the red/green/blue channels of a LUT's entries must be
equal. Let's add a helper function that drivers can use to test that a
userspace-provided LUT is valid and doesn't violate hardware
requirements.
v2:
- Combine into a single helper that just takes a bitmask of the tests
to apply. (Brian Starkey)
- Add additional check (always performed) that LUT property blob size
is always a multiple of the LUT entry size. (stolen from ARM driver)
Cc: Uma Shankar
Cc: Swati Sharma
Cc: Brian Starkey
Signed-off-by: Matt Roper
Reviewed-by(v1): Brian Starkey
---
drivers/gpu/drm/drm_color_mgmt.c | 64
include/drm/drm_color_mgmt.h | 5
2 files changed, 69 insertions(+)
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 07dcf47daafe..5c2a2d228412 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -462,3 +462,67 @@ int drm_plane_create_color_properties(struct drm_plane
*plane,
return 0;
}
EXPORT_SYMBOL(drm_plane_create_color_properties);
+
+/**
+ * drm_color_lut_check - check validity of lookup table
+ * @lut: property blob containing LUT to check
+ * @tests: bitmask of tests to run
+ *
+ * Helper to check whether a userspace-provided lookup table is valid and
+ * satisfies additional hardware requirements. All table sizes should be a
+ * multiple of sizeof(struct drm_color_lut). Drivers pass a bitmask indicating
+ * which of the following additional tests should also be performed:
+ *
+ * "DRM_COLOR_LUT_EQUAL_CHANNELS":
+ * Checks whether the entries of a LUT all have equal values for the red,
+ * green, and blue channels. Intended for hardware that only accepts a
+ * single value per LUT entry and assumes that value applies to all three
+ * color components.
+ *
+ * "DRM_COLOR_LUT_INCREASING":
+ * Checks whether the entries of a LUT are always flat or increasing
+ * (never decreasing).
+ *
+ * Returns 0 on success, -EINVAL on failure.
+ */
+int drm_color_lut_check(struct drm_property_blob *lut,
+uint32_t tests)
+{
+ struct drm_color_lut *entry;
+ int i;
+
+ if (!lut)
+ return 0;
+
+ if (lut->length % sizeof(struct drm_color_lut)) {
+ DRM_DEBUG_KMS("LUT size (%lu) is not a multiple of LUT entry
size (%lu)\n",
+ lut->length, sizeof(struct drm_color_lut));
+ return -EINVAL;
+ }
+
+ if (!tests)
+ return 0;
+
+ entry = lut->data;
+ for (i = 0; i < drm_color_lut_size(lut); i++) {
+ if (tests & DRM_COLOR_LUT_EQUAL_CHANNELS) {
+ if (entry[i].red != entry[i].blue ||
+ entry[i].red != entry[i].green) {
+ DRM_DEBUG_KMS("All LUT entries must have equal
r/g/b\n");
+ return -EINVAL;
+ }
+ }
+
+ if (i > 0 && tests & DRM_COLOR_LUT_INCREASING) {
+ if (entry[i].red < entry[i - 1].red ||
+ entry[i].green < entry[i - 1].green ||
+ entry[i].blue < entry[i - 1].blue) {
+ DRM_DEBUG_KMS("LUT entries must never
decrease.\n");
+ return -EINVAL;
+ }
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_color_lut_check);
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index 90ef9996d9a4..7de16f70bcc3 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -69,4 +69,9 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
u32 supported_ranges,
enum drm_color_encoding default_encoding,
enum drm_color_range default_range);
+
+#define DRM_COLOR_LUT_EQUAL_CHANNELS BIT(0)
+#define DRM_COLOR_LUT_INCREASING BIT(1)
+int drm_color_lut_check(struct drm_property_blob *lut,
+ uint32_t tests);
#endif
--
2.14.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] drm/i915: Validate userspace-provided color management LUT's (v2)
We currently program userspace-provided gamma and degamma LUT's into our
hardware without really checking to see whether they satisfy our
hardware's rules. We should try to catch tables that are invalid for
our hardware early and reject the atomic transaction.
All of our platforms that accept a degamma LUT expect that the entries
in the LUT are always flat or increasing, never decreasing. Also, our
GLK and ICL platforms only accept degamma tables with r=g=b entries; so
we should also add the relevant checks for that in anticipation of
degamma support landing for those platforms.
v2:
- Use new API (single check function with bitmask of tests to apply)
- Call helper for our gamma table as well (with no additional tests
specified) so that the table size will be validated.
Cc: Uma Shankar
Cc: Swati Sharma
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/intel_color.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_color.c
b/drivers/gpu/drm/i915/intel_color.c
index 37fd9ddf762e..5ad4459a5f3c 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -609,10 +609,29 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
{
struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
size_t gamma_length, degamma_length;
+ uint32_t tests = DRM_COLOR_LUT_INCREASING;
degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+ /*
+* All of our platforms mandate that the degamma curve be
+* non-decreasing. Additionally, GLK and gen11 only accept a single
+* value for red, green, and blue in the degamma table. Make sure
+* userspace didn't try to pass us something we can't handle.
+*
+* We don't have any extra hardware constraints on the gamma table,
+* so we just test that it's a proper size multiple
+* (tablesize % entrysize == 0).
+*/
+ if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 11)
+ tests |= DRM_COLOR_LUT_EQUAL_CHANNELS;
+
+ if (drm_color_lut_check(crtc_state->base.degamma_lut, tests) != 0)
+ return -EINVAL;
+ if (drm_color_lut_check(crtc_state->base.gamma_lut, 0) != 0)
+ return -EINVAL;
+
/*
* We allow both degamma & gamma luts at the right size or
* NULL.
--
2.14.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Outreachy kernel] [RESEND PATCH v2] drm: Rename crtc_idr as object_idr to KMS cleanups
On Thu, Dec 13, 2018 at 07:29:57PM -0200, Shayenne da Luz Moura wrote:
> This patch solves this TODO task:
> drm_mode_config.crtc_idr is misnamed, since it contains all KMS object.
> Should be renamed to drm_mode_config.object_idr.
>
> Signed-off-by: Shayenne da Luz Moura
>
> ---
> Changes in v2:
> - Make commit message more clear and change header file
Applied now, thanks for your patch. For next time around, pls ping after a
week already, likely no answer after a week means the patch simply fell
through the cracks :-/
Can you pls follow up with a patch to remove the todo item, now that it's
completed?
Thanks, Daniel
>
> drivers/gpu/drm/drm_lease.c | 6 +++---
> drivers/gpu/drm/drm_mode_config.c | 4 ++--
> drivers/gpu/drm/drm_mode_object.c | 8
> include/drm/drm_mode_config.h | 6 +++---
> 4 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
> index 977f069f6d90..b2cbb58834bc 100644
> --- a/drivers/gpu/drm/drm_lease.c
> +++ b/drivers/gpu/drm/drm_lease.c
> @@ -218,7 +218,7 @@ static struct drm_master *drm_lease_create(struct
> drm_master *lessor, struct idr
>
> idr_for_each_entry(leases, entry, object) {
> error = 0;
> - if (!idr_find(&dev->mode_config.crtc_idr, object))
> + if (!idr_find(&dev->mode_config.object_idr, object))
> error = -ENOENT;
> else if (!_drm_lease_held_master(lessor, object))
> error = -EACCES;
> @@ -434,7 +434,7 @@ static int fill_object_idr(struct drm_device *dev,
> /*
>* We're using an IDR to hold the set of leased
>* objects, but we don't need to point at the object's
> - * data structure from the lease as the main crtc_idr
> + * data structure from the lease as the main object_idr
>* will be used to actually find that. Instead, all we
>* really want is a 'leased/not-leased' result, for
>* which any non-NULL pointer will work fine.
> @@ -675,7 +675,7 @@ int drm_mode_get_lease_ioctl(struct drm_device *dev,
>
> if (lessee->lessor == NULL)
> /* owner can use all objects */
> - object_idr = &lessee->dev->mode_config.crtc_idr;
> + object_idr = &lessee->dev->mode_config.object_idr;
> else
> /* lessee can only use allowed object */
> object_idr = &lessee->leases;
> diff --git a/drivers/gpu/drm/drm_mode_config.c
> b/drivers/gpu/drm/drm_mode_config.c
> index ee80788f2c40..ab553b6465e2 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -381,7 +381,7 @@ void drm_mode_config_init(struct drm_device *dev)
> INIT_LIST_HEAD(&dev->mode_config.property_list);
> INIT_LIST_HEAD(&dev->mode_config.property_blob_list);
> INIT_LIST_HEAD(&dev->mode_config.plane_list);
> - idr_init(&dev->mode_config.crtc_idr);
> + idr_init(&dev->mode_config.object_idr);
> idr_init(&dev->mode_config.tile_idr);
> ida_init(&dev->mode_config.connector_ida);
> spin_lock_init(&dev->mode_config.connector_list_lock);
> @@ -484,7 +484,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>
> ida_destroy(&dev->mode_config.connector_ida);
> idr_destroy(&dev->mode_config.tile_idr);
> - idr_destroy(&dev->mode_config.crtc_idr);
> + idr_destroy(&dev->mode_config.object_idr);
> drm_modeset_lock_fini(&dev->mode_config.connection_mutex);
> }
> EXPORT_SYMBOL(drm_mode_config_cleanup);
> diff --git a/drivers/gpu/drm/drm_mode_object.c
> b/drivers/gpu/drm/drm_mode_object.c
> index cd9bc0ce9be0..bb1dd46496cd 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -38,7 +38,7 @@ int __drm_mode_object_add(struct drm_device *dev, struct
> drm_mode_object *obj,
> int ret;
>
> mutex_lock(&dev->mode_config.idr_mutex);
> - ret = idr_alloc(&dev->mode_config.crtc_idr, register_obj ? obj : NULL,
> + ret = idr_alloc(&dev->mode_config.object_idr, register_obj ? obj : NULL,
> 1, 0, GFP_KERNEL);
> if (ret >= 0) {
> /*
> @@ -79,7 +79,7 @@ void drm_mode_object_register(struct drm_device *dev,
> struct drm_mode_object *obj)
> {
> mutex_lock(&dev->mode_config.idr_mutex);
> - idr_replace(&dev->mode_config.crtc_idr, obj, obj->id);
> + idr_replace(&dev->mode_config.object_idr, obj, obj->id);
> mutex_unlock(&dev->mode_config.idr_mutex);
> }
>
> @@ -99,7 +99,7 @@ void drm_mode_object_unregister(struct drm_device *dev,
> {
> mutex_lock(&dev->mode_config.idr_mutex);
> if (object->id) {
> - idr_remove(&dev->mode_config.crtc_idr, object->id);
> + idr_remove(&dev->mode_config.object_idr, object->id);
> object->id = 0;
> }
> mutex_unl
Re: [PATCH] drm: rcar-du: Add r8a774c0 device support
Hi Fabrizio,
Thank you for the patch.
On Thursday, 13 December 2018 22:23:27 EET Fabrizio Castro wrote:
> Add support for the RZ/G2E (R8A774C0) SoC to the R-Car DU driver.
>
> Signed-off-by: Fabrizio Castro
Reviewed-by: Laurent Pinchart
and applied to my tree.
> ---
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 28
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 9caff39..ac32510 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -104,6 +104,33 @@ static const struct rcar_du_device_info
> rzg1_du_r8a77470_info = { },
> };
>
> +static const struct rcar_du_device_info rcar_du_r8a774c0_info = {
> + .gen = 3,
> + .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + | RCAR_DU_FEATURE_EXT_CTRL_REGS
> + | RCAR_DU_FEATURE_VSP1_SOURCE,
> + .channels_mask = BIT(1) | BIT(0),
> + .routes = {
> + /*
> + * R8A774C0 has one RGB output and two LVDS outputs
> + */
> + [RCAR_DU_OUTPUT_DPAD0] = {
> + .possible_crtcs = BIT(0) | BIT(1),
> + .port = 0,
> + },
> + [RCAR_DU_OUTPUT_LVDS0] = {
> + .possible_crtcs = BIT(0),
> + .port = 1,
> + },
> + [RCAR_DU_OUTPUT_LVDS1] = {
> + .possible_crtcs = BIT(1),
> + .port = 2,
> + },
> + },
> + .num_lvds = 2,
> + .lvds_clk_mask = BIT(1) | BIT(0),
> +};
> +
> static const struct rcar_du_device_info rcar_du_r8a7779_info = {
> .gen = 2,
> .features = RCAR_DU_FEATURE_INTERLACED
> @@ -371,6 +398,7 @@ static const struct of_device_id rcar_du_of_table[] = {
> { .compatible = "renesas,du-r8a7744", .data = &rzg1_du_r8a7743_info },
> { .compatible = "renesas,du-r8a7745", .data = &rzg1_du_r8a7745_info },
> { .compatible = "renesas,du-r8a77470", .data = &rzg1_du_r8a77470_info },
> + { .compatible = "renesas,du-r8a774c0", .data = &rcar_du_r8a774c0_info },
> { .compatible = "renesas,du-r8a7779", .data = &rcar_du_r8a7779_info },
> { .compatible = "renesas,du-r8a7790", .data = &rcar_du_r8a7790_info },
> { .compatible = "renesas,du-r8a7791", .data = &rcar_du_r8a7791_info },
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dt-bindings: display: renesas: lvds: Document r8a774c0 bindings
Hi Fabrizio, Thank you for the patch. On Thursday, 13 December 2018 22:20:54 EET Fabrizio Castro wrote: > The RZ/G2E (r8a774c0) supports two LVDS channels. Extend the binding to > support them. > > Signed-off-by: Fabrizio Castro Reviewed-by: Laurent Pinchart and applied to my tree. > --- > Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt index > 3aeb0ec..cb7c328 100644 > --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > @@ -8,6 +8,7 @@ Required properties: > > - compatible : Shall contain one of >- "renesas,r8a7743-lvds" for R8A7743 (RZ/G1M) compatible LVDS encoders > + - "renesas,r8a774c0-lvds" for R8A774C0 (RZ/G2E) compatible LVDS encoders >- "renesas,r8a7790-lvds" for R8A7790 (R-Car H2) compatible LVDS encoders >- "renesas,r8a7791-lvds" for R8A7791 (R-Car M2-W) compatible LVDS > encoders - "renesas,r8a7793-lvds" for R8A7793 (R-Car M2-N) compatible LVDS > encoders @@ -24,7 +25,7 @@ Required properties: > - clock-names: Name of the clocks. This property is model-dependent. >- The functional clock, which mandatory for all models, shall be listed > first, and shall be named "fck". > - - On R8A77990 and R8A77995, the LVDS encoder can use the EXTAL or > + - On R8A77990, R8A77995 and R8A774C0, the LVDS encoder can use the EXTAL > or DU_DOTCLKINx clocks. Those clocks are optional. When supplied they must > be named "extal" and "dclkin.x" respectively, with "x" being the > DU_DOTCLKIN numerical index. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: rcar-du: lvds: add R8A774C0 support
Hi Fabrizio,
Thank you for the patch.
On Thursday, 13 December 2018 22:23:36 EET Fabrizio Castro wrote:
> The LVDS implementation on the RZ/G2E (a.k.a. R8A774C0) is very similar
> to the one found on R-Car E3 (a.k.a. R8A77990), therefore add RZ/G2E
> LVDS support to the LVDS encoder driver in a similar fashion to what
s/what/what is/
> done for R-Car E3.
>
> Signed-off-by: Fabrizio Castro
Reviewed-by: Laurent Pinchart
and applied to my tree with the above fix.
> ---
> drivers/gpu/drm/rcar-du/rcar_lvds.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 173d7ad..329dfca 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -785,6 +785,7 @@ static const struct rcar_lvds_device_info
> rcar_lvds_r8a77995_info = {
>
> static const struct of_device_id rcar_lvds_of_table[] = {
> { .compatible = "renesas,r8a7743-lvds", .data = &rcar_lvds_gen2_info },
> + { .compatible = "renesas,r8a774c0-lvds", .data =
> &rcar_lvds_r8a77990_info
> }, { .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_r8a7790_info
> }, { .compatible = "renesas,r8a7791-lvds", .data = &rcar_lvds_gen2_info },
> { .compatible = "renesas,r8a7793-lvds", .data = &rcar_lvds_gen2_info },
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dt-bindings: display: renesas: du: Document r8a774c0 bindings
Hi Fabrizio, Thank you for the patch. On Thursday, 13 December 2018 22:20:45 EET Fabrizio Castro wrote: > Document the RZ/G2E (a.k.a. r8a774c0) SoC in the R-Car DU bindings. > > Signed-off-by: Fabrizio Castro Reviewed-by: Laurent Pinchart and applied to my tree. > --- > Documentation/devicetree/bindings/display/renesas,du.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt > b/Documentation/devicetree/bindings/display/renesas,du.txt index > 3c855d9..aedb22b 100644 > --- a/Documentation/devicetree/bindings/display/renesas,du.txt > +++ b/Documentation/devicetree/bindings/display/renesas,du.txt > @@ -7,6 +7,7 @@ Required Properties: > - "renesas,du-r8a7744" for R8A7744 (RZ/G1N) compatible DU > - "renesas,du-r8a7745" for R8A7745 (RZ/G1E) compatible DU > - "renesas,du-r8a77470" for R8A77470 (RZ/G1C) compatible DU > +- "renesas,du-r8a774c0" for R8A774C0 (RZ/G2E) compatible DU > - "renesas,du-r8a7779" for R8A7779 (R-Car H1) compatible DU > - "renesas,du-r8a7790" for R8A7790 (R-Car H2) compatible DU > - "renesas,du-r8a7791" for R8A7791 (R-Car M2-W) compatible DU > @@ -57,6 +58,7 @@ corresponding to each DU output. > R8A7744 (RZ/G1N) DPAD 0 LVDS 0 - - > R8A7745 (RZ/G1E) DPAD 0 DPAD 1 - - > R8A77470 (RZ/G1C) DPAD 0 DPAD 1 LVDS 0 - > + R8A774C0 (RZ/G2E) DPAD 0 LVDS 0 LVDS 1 - > R8A7779 (R-Car H1) DPAD 0 DPAD 1 - - > R8A7790 (R-Car H2) DPAD 0 LVDS 0 LVDS 1 - > R8A7791 (R-Car M2-W) DPAD 0 LVDS 0 - - -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RESEND PATCH v2] drm: Rename crtc_idr as object_idr to KMS cleanups
This patch solves this TODO task:
drm_mode_config.crtc_idr is misnamed, since it contains all KMS object.
Should be renamed to drm_mode_config.object_idr.
Signed-off-by: Shayenne da Luz Moura
---
Changes in v2:
- Make commit message more clear and change header file
drivers/gpu/drm/drm_lease.c | 6 +++---
drivers/gpu/drm/drm_mode_config.c | 4 ++--
drivers/gpu/drm/drm_mode_object.c | 8
include/drm/drm_mode_config.h | 6 +++---
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index 977f069f6d90..b2cbb58834bc 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -218,7 +218,7 @@ static struct drm_master *drm_lease_create(struct
drm_master *lessor, struct idr
idr_for_each_entry(leases, entry, object) {
error = 0;
- if (!idr_find(&dev->mode_config.crtc_idr, object))
+ if (!idr_find(&dev->mode_config.object_idr, object))
error = -ENOENT;
else if (!_drm_lease_held_master(lessor, object))
error = -EACCES;
@@ -434,7 +434,7 @@ static int fill_object_idr(struct drm_device *dev,
/*
* We're using an IDR to hold the set of leased
* objects, but we don't need to point at the object's
-* data structure from the lease as the main crtc_idr
+* data structure from the lease as the main object_idr
* will be used to actually find that. Instead, all we
* really want is a 'leased/not-leased' result, for
* which any non-NULL pointer will work fine.
@@ -675,7 +675,7 @@ int drm_mode_get_lease_ioctl(struct drm_device *dev,
if (lessee->lessor == NULL)
/* owner can use all objects */
- object_idr = &lessee->dev->mode_config.crtc_idr;
+ object_idr = &lessee->dev->mode_config.object_idr;
else
/* lessee can only use allowed object */
object_idr = &lessee->leases;
diff --git a/drivers/gpu/drm/drm_mode_config.c
b/drivers/gpu/drm/drm_mode_config.c
index ee80788f2c40..ab553b6465e2 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -381,7 +381,7 @@ void drm_mode_config_init(struct drm_device *dev)
INIT_LIST_HEAD(&dev->mode_config.property_list);
INIT_LIST_HEAD(&dev->mode_config.property_blob_list);
INIT_LIST_HEAD(&dev->mode_config.plane_list);
- idr_init(&dev->mode_config.crtc_idr);
+ idr_init(&dev->mode_config.object_idr);
idr_init(&dev->mode_config.tile_idr);
ida_init(&dev->mode_config.connector_ida);
spin_lock_init(&dev->mode_config.connector_list_lock);
@@ -484,7 +484,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
ida_destroy(&dev->mode_config.connector_ida);
idr_destroy(&dev->mode_config.tile_idr);
- idr_destroy(&dev->mode_config.crtc_idr);
+ idr_destroy(&dev->mode_config.object_idr);
drm_modeset_lock_fini(&dev->mode_config.connection_mutex);
}
EXPORT_SYMBOL(drm_mode_config_cleanup);
diff --git a/drivers/gpu/drm/drm_mode_object.c
b/drivers/gpu/drm/drm_mode_object.c
index cd9bc0ce9be0..bb1dd46496cd 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -38,7 +38,7 @@ int __drm_mode_object_add(struct drm_device *dev, struct
drm_mode_object *obj,
int ret;
mutex_lock(&dev->mode_config.idr_mutex);
- ret = idr_alloc(&dev->mode_config.crtc_idr, register_obj ? obj : NULL,
+ ret = idr_alloc(&dev->mode_config.object_idr, register_obj ? obj : NULL,
1, 0, GFP_KERNEL);
if (ret >= 0) {
/*
@@ -79,7 +79,7 @@ void drm_mode_object_register(struct drm_device *dev,
struct drm_mode_object *obj)
{
mutex_lock(&dev->mode_config.idr_mutex);
- idr_replace(&dev->mode_config.crtc_idr, obj, obj->id);
+ idr_replace(&dev->mode_config.object_idr, obj, obj->id);
mutex_unlock(&dev->mode_config.idr_mutex);
}
@@ -99,7 +99,7 @@ void drm_mode_object_unregister(struct drm_device *dev,
{
mutex_lock(&dev->mode_config.idr_mutex);
if (object->id) {
- idr_remove(&dev->mode_config.crtc_idr, object->id);
+ idr_remove(&dev->mode_config.object_idr, object->id);
object->id = 0;
}
mutex_unlock(&dev->mode_config.idr_mutex);
@@ -131,7 +131,7 @@ struct drm_mode_object *__drm_mode_object_find(struct
drm_device *dev,
struct drm_mode_object *obj = NULL;
mutex_lock(&dev->mode_config.idr_mutex);
- obj = idr_find(&dev->mode_config.crtc_idr, id);
+ obj = idr_find(&dev->mode_config.object_idr, id);
if (obj && type != DRM_MODE_OBJECT_ANY && obj->type != type)
obj = NULL;
if (o
Re: [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options
Hi Maxime, One more small note. On Fri, Dec 07, 2018 at 02:55:30PM +0100, Maxime Ripard wrote: ... > + /** > + * @wakeup: > + * > + * Time, in picoseconds, that a transmitter drives a Mark-1 > + * state prior to a Stop state in order to initiate an exit > + * from ULPS. > + * > + * Minimum value: 10 ps > + */ > + unsigned intwakeup; This is very close to the higher limit of the type's value range. How about using ns or µs for this one? -- Sakari Ailus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options
On Thu, Dec 13, 2018 at 10:54:28PM +0200, sakari.ai...@iki.fi wrote: > Hi Maxime, > > One more small note. > > On Fri, Dec 07, 2018 at 02:55:30PM +0100, Maxime Ripard wrote: > > ... > > > + /** > > +* @wakeup: > > +* > > +* Time, in picoseconds, that a transmitter drives a Mark-1 > > +* state prior to a Stop state in order to initiate an exit > > +* from ULPS. > > +* > > +* Minimum value: 10 ps > > +*/ > > + unsigned intwakeup; > > This is very close to the higher limit of the type's value range. How about > using ns or µs for this one? Same for init actually. -- Sakari Ailus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options
Hi Maxime,
On Fri, Dec 07, 2018 at 02:55:30PM +0100, Maxime Ripard wrote:
> Now that we have some infrastructure for it, allow the MIPI D-PHY phy's to
> be configured through the generic functions through a custom structure
> added to the generic union.
>
> The parameters added here are the ones defined in the MIPI D-PHY spec, plus
> the number of lanes in use. The current set of parameters should cover all
> the potential users.
>
> Signed-off-by: Maxime Ripard
> ---
> include/linux/phy/phy-mipi-dphy.h | 279 +++-
> include/linux/phy/phy.h | 6 +-
> 2 files changed, 285 insertions(+)
> create mode 100644 include/linux/phy/phy-mipi-dphy.h
>
> diff --git a/include/linux/phy/phy-mipi-dphy.h
> b/include/linux/phy/phy-mipi-dphy.h
> new file mode 100644
> index ..29bf94db88ad
> --- /dev/null
> +++ b/include/linux/phy/phy-mipi-dphy.h
> @@ -0,0 +1,279 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2018 Cadence Design Systems Inc.
> + */
> +
> +#ifndef __PHY_MIPI_DPHY_H_
> +#define __PHY_MIPI_DPHY_H_
> +
> +#include
Where do you need this? Please remove.
> +
> +/**
> + * struct phy_configure_opts_mipi_dphy - MIPI D-PHY configuration set
> + *
> + * This structure is used to represent the configuration state of a
> + * MIPI D-PHY phy.
> + */
> +struct phy_configure_opts_mipi_dphy {
> + /**
> + * @clk_miss:
> + *
> + * Timeout, in picoseconds, for receiver to detect absence of
> + * Clock transitions and disable the Clock Lane HS-RX.
> + *
> + * Maximum value: 6 ps
> + */
> + unsigned intclk_miss;
> +
> + /**
> + * @clk_post:
> + *
> + * Time, in picoseconds, that the transmitter continues to
> + * send HS clock after the last associated Data Lane has
> + * transitioned to LP Mode. Interval is defined as the period
> + * from the end of @hs_trail to the beginning of @clk_trail.
> + *
> + * Minimum value: 6 ps + 52 * @hs_clk_rate period in ps
> + */
> + unsigned intclk_post;
> +
> + /**
> + * @clk_pre:
> + *
> + * Time, in UI, that the HS clock shall be driven by
> + * the transmitter prior to any associated Data Lane beginning
> + * the transition from LP to HS mode.
> + *
> + * Minimum value: 8 UI
> + */
> + unsigned intclk_pre;
> +
> + /**
> + * @clk_prepare:
> + *
> + * Time, in picoseconds, that the transmitter drives the Clock
> + * Lane LP-00 Line state immediately before the HS-0 Line
> + * state starting the HS transmission.
> + *
> + * Minimum value: 38000 ps
> + * Maximum value: 95000 ps
> + */
> + unsigned intclk_prepare;
> +
> + /**
> + * @clk_settle:
> + *
> + * Time interval, in picoseconds, during which the HS receiver
> + * should ignore any Clock Lane HS transitions, starting from
> + * the beginning of @clk_prepare.
> + *
> + * Minimum value: 95000 ps
> + * Maximum value: 30 ps
> + */
> + unsigned intclk_settle;
> +
> + /**
> + * @clk_term_en:
> + *
> + * Time, in picoseconds, for the Clock Lane receiver to enable
> + * the HS line termination.
> + *
> + * Maximum value: 38000 ps
> + */
> + unsigned intclk_term_en;
> +
> + /**
> + * @clk_trail:
> + *
> + * Time, in picoseconds, that the transmitter drives the HS-0
> + * state after the last payload clock bit of a HS transmission
> + * burst.
> + *
> + * Minimum value: 6 ps
> + */
> + unsigned intclk_trail;
> +
> + /**
> + * @clk_zero:
> + *
> + * Time, in picoseconds, that the transmitter drives the HS-0
> + * state prior to starting the Clock.
> + */
> + unsigned intclk_zero;
> +
> + /**
> + * @d_term_en:
> + *
> + * Time, in picoseconds, for the Data Lane receiver to enable
> + * the HS line termination.
> + *
> + * Maximum value: 35000 ps + 4 * @hs_clk_rate period in ps
> + */
> + unsigned intd_term_en;
> +
> + /**
> + * @eot:
> + *
> + * Transmitted time interval, in picoseconds, from the start
> + * of @hs_trail or @clk_trail, to the start of the LP- 11
> + * state following a HS burst.
> + *
> + * Maximum value: 105000 ps + 12 * @hs_clk_rate period in ps
> + */
> + unsigned inteot;
> +
> + /**
> + * @hs_exit:
> + *
> + * Time, in picoseconds, that the transmitter drives LP-11
> + * following a HS burst.
> + *
> + * Minimum value: 10 ps
> + */
> + unsigned inths_exit;
> +
> + /**
> + * @hs_prepare:
> + *
> + * Time, in picoseconds, that the transmitter drives the Data
> + * Lane LP-00 Li
Re: [PATCH v3 02/10] phy: Add configuration interface
Hi Maxime,
On Fri, Dec 07, 2018 at 02:55:29PM +0100, Maxime Ripard wrote:
> The phy framework is only allowing to configure the power state of the PHY
> using the init and power_on hooks, and their power_off and exit
> counterparts.
>
> While it works for most, simple, PHYs supported so far, some more advanced
> PHYs need some configuration depending on runtime parameters. These PHYs
> have been supported by a number of means already, often by using ad-hoc
> drivers in their consumer drivers.
>
> That doesn't work too well however, when a consumer device needs to deal
> with multiple PHYs, or when multiple consumers need to deal with the same
> PHY (a DSI driver and a CSI driver for example).
>
> So we'll add a new interface, through two funtions, phy_validate and
> phy_configure. The first one will allow to check that a current
> configuration, for a given mode, is applicable. It will also allow the PHY
> driver to tune the settings given as parameters as it sees fit.
>
> phy_configure will actually apply that configuration in the phy itself.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/phy/phy-core.c | 64 ++-
> include/linux/phy/phy.h | 58 ++-
> 2 files changed, 122 insertions(+)
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index df3d4ba516ab..19b05e824ee4 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -408,6 +408,70 @@ int phy_calibrate(struct phy *phy)
> EXPORT_SYMBOL_GPL(phy_calibrate);
>
> /**
> + * phy_configure() - Changes the phy parameters
> + * @phy: the phy returned by phy_get()
> + * @opts: New configuration to apply
> + *
> + * Used to change the PHY parameters. phy_init() must have been called
> + * on the phy. The configuration will be applied on the current phy
> + * mode, that can be changed using phy_set_mode().
> + *
> + * Returns: 0 if successful, an negative error code otherwise
> + */
> +int phy_configure(struct phy *phy, union phy_configure_opts *opts)
> +{
> + int ret;
> +
> + if (!phy)
> + return -EINVAL;
> +
> + if (!phy->ops->configure)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&phy->mutex);
> + ret = phy->ops->configure(phy, opts);
> + mutex_unlock(&phy->mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_configure);
> +
> +/**
> + * phy_validate() - Checks the phy parameters
> + * @phy: the phy returned by phy_get()
> + * @mode: phy_mode the configuration is applicable to.
> + * @submode: PHY submode the configuration is applicable to.
> + * @opts: Configuration to check
> + *
> + * Used to check that the current set of parameters can be handled by
> + * the phy. Implementations are free to tune the parameters passed as
> + * arguments if needed by some implementation detail or
> + * constraints. It will not change any actual configuration of the
> + * PHY, so calling it as many times as deemed fit will have no side
> + * effect.
> + *
> + * Returns: 0 if successful, an negative error code otherwise
> + */
> +int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
> + union phy_configure_opts *opts)
> +{
> + int ret;
> +
> + if (!phy)
> + return -EINVAL;
> +
> + if (!phy->ops->validate)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&phy->mutex);
> + ret = phy->ops->validate(phy, mode, submode, opts);
> + mutex_unlock(&phy->mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_validate);
> +
> +/**
> * _of_phy_get() - lookup and obtain a reference to a phy by phandle
> * @np: device_node for which to get the phy
> * @index: the index of the phy
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 453f21834685..04476c026b5a 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -43,6 +43,12 @@ enum phy_mode {
> };
>
> /**
> + * union phy_configure_opts - Opaque generic phy configuration
> + */
> +union phy_configure_opts {
> +};
> +
> +/**
> * struct phy_ops - set of function pointers for performing phy operations
> * @init: operation to be performed for initializing phy
> * @exit: operation to be performed while exiting
> @@ -59,6 +65,37 @@ struct phy_ops {
> int (*power_on)(struct phy *phy);
> int (*power_off)(struct phy *phy);
> int (*set_mode)(struct phy *phy, enum phy_mode mode, int submode);
> +
> + /**
> + * @configure:
> + *
> + * Optional.
> + *
> + * Used to change the PHY parameters. phy_init() must have
> + * been called on the phy.
> + *
> + * Returns: 0 if successful, an negative error code otherwise
> + */
> + int (*configure)(struct phy *phy, union phy_configure_opts *opts);
> +
> + /**
> + * @validate:
> + *
> + * Optional.
> + *
> + * Used to check that the current set of parameters can be
> + * handled by the
Re: [PATCH v2 11/12] drm/panel: Add Feiyang FY07024DI26A30-D MIPI-DSI LCD panel
On Fri, Dec 14, 2018 at 12:56:03AM +0530, Jagan Teki wrote:
> On Thu, Dec 13, 2018 at 8:37 PM Sean Paul wrote:
> >
> > On Fri, Nov 16, 2018 at 10:09:15PM +0530, Jagan Teki wrote:
> > > Feiyang FY07024DI26A30-D is 1024x600, 4-lane MIPI-DSI LCD panel.
> > >
> > > Add panel driver for it.
> > >
> > > Signed-off-by: Jagan Teki
> > > ---
> > > MAINTAINERS | 6 +
> > > drivers/gpu/drm/panel/Kconfig | 9 +
> > > drivers/gpu/drm/panel/Makefile| 1 +
> > > .../drm/panel/panel-feiyang-fy07024di26a30d.c | 286 ++
> > > 4 files changed, 302 insertions(+)
> > > create mode 100644 drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> > >
/snip
> > > diff --git a/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> > > b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> > > new file mode 100644
> > > index ..a4b46bd8fdbe
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
/snip
> > > +static int feiyang_prepare(struct drm_panel *panel)
> > > +{
> > > + struct feiyang *ctx = panel_to_feiyang(panel);
> > > + struct mipi_dsi_device *dsi = ctx->dsi;
> > > + unsigned int i;
> > > + int ret;
> > > +
> > > + ret = regulator_enable(ctx->dvdd);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + msleep(100);
> >
> > nit: You should do your best to correlate the sleeps with the timing
> > parameters
> > from the datasheet with a comment.
> >
> > ie:
> > /* T1: > 100ms */
> > msleep(100);
>
> Sorry, what does this mean?
On page 9 of the datasheet you sent me [1], it has the delays required to safely
power up the panel. This delay is the time between dvdd going high and avdd
going high. On the figure in the datasheet, this would be T2 (T1 is dvdd rise
time and should be handled in the regulator subsystem (iirc)). Also according to
the datasheet, T2 just needs to be > 0, so you don't even need this delay. You
could replace this with a comment like:
/* T1 (dvdd rise time) + T2 (dvdd->avdd) > 0 */
So for all of the msleeps below you should get the delays from the datasheet and
add a comment referencing them.
Sean
[1] -
http://files.pine64.org/doc/datasheet/pine64/FY07024DI26A30-D_feiyang_LCD_panel.pdf
>
> >
> > > +
> > > + ret = regulator_enable(ctx->avdd);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + msleep(20);
> > > +
> > > + gpiod_set_value(ctx->reset, 1);
> > > + msleep(50);
> > > +
> > > + gpiod_set_value(ctx->reset, 0);
> > > + msleep(20);
> > > +
> > > + gpiod_set_value(ctx->reset, 1);
> > > + msleep(200);
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(feiyang_init_cmds); i++) {
> > > + const struct feiyang_init_cmd *cmd =
> > > + &feiyang_init_cmds[i];
> > > +
> > > + ret = mipi_dsi_dcs_write_buffer(dsi, cmd->data, cmd->len);
> >
> > ret = mipi_dsi_dcs_write_buffer(dsi, cmd->data,
> > FEIYANG_INIT_CMD_LEN);
> >
> > > + if (ret < 0)
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int feiyang_enable(struct drm_panel *panel)
> > > +{
> > > + struct feiyang *ctx = panel_to_feiyang(panel);
> > > +
> > > + msleep(120);
> > > +
> > > + mipi_dsi_dcs_set_display_on(ctx->dsi);
> > > + backlight_enable(ctx->backlight);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int feiyang_disable(struct drm_panel *panel)
> > > +{
> > > + struct feiyang *ctx = panel_to_feiyang(panel);
> > > +
> > > + backlight_disable(ctx->backlight);
> > > + return mipi_dsi_dcs_set_display_on(ctx->dsi);
> >
> > set_display_on? You probably want set_display_off here :)
> >
> > > +}
> > > +
> > > +static int feiyang_unprepare(struct drm_panel *panel)
> > > +{
> > > + struct feiyang *ctx = panel_to_feiyang(panel);
> > > + int ret;
> > > +
> > > + ret = mipi_dsi_dcs_set_display_off(ctx->dsi);
> > > + if (ret < 0)
> > > + DRM_DEV_ERROR(panel->dev, "failed to set display off: %d\n",
> > > + ret);
> > > +
> > > + ret = mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
> > > + if (ret < 0)
> > > + DRM_DEV_ERROR(panel->dev, "failed to enter sleep mode:
> > > %d\n",
> > > + ret);
> > > +
> > > + msleep(100);
> > > +
> > > + regulator_disable(ctx->avdd);
> > > +
> > > + regulator_disable(ctx->dvdd);
> > > +
> > > + gpiod_set_value(ctx->reset, 0);
> > > +
> > > + gpiod_set_value(ctx->reset, 1);
> > > +
> > > + gpiod_set_value(ctx->reset, 0);
> >
> > Presumably this reset line toggle isn't needed since the rails are already
> > disabled?
>
> Yes, rails need to reset and turn off. will swap.
>
> >
> > > +
> > > + return 0;
> > > +}
Re: [PATCH] drm/virtio: switch to generic fbdev emulation
On Thu, 13 Dec 2018 at 23:49, Gerd Hoffmann wrote: > > Signed-off-by: Gerd Hoffmann Seems correct to me, Reviewed-by: Dave Airlie ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107826] amdgpu-pro 18.30/18.40: Missing xserver modesetting package (--px install)
https://bugs.freedesktop.org/show_bug.cgi?id=107826 --- Comment #2 from Jeremy Newton --- Sorry, I thought I responded to this already. PX is not supported on the PRO driver anymore. It seems we forgot to put in a deprecated message in the docs. I'll try make note of it for future releases. We recommend using the all open driver for mobile uses and using PRIME for switchable gfx. You can install the all open driver by running the "./amdgpu-install" script with no parameters. If you need vulkan, install the package "vulkan-amdgpu" after the driver is installed. If you need to know to use PRIME, there's a good resource on the ARCH Linux wiki: https://wiki.archlinux.org/index.php/PRIME -- 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 06/10] drm/syncobj: use the timeline point in drm_syncobj_find_fence v3
Am 13.12.18 um 18:26 schrieb Daniel Vetter: >>> Code sharing just because the code looks similar is imo a really >>> bad idea, when the semantics are entirely different (that was also the >>> reason behind not reusing all the cpu event stuff for dma_fence, they're >>> not normal cpu events). >> Ok, the last sentence is what I don't understand. >> >> What exactly is the semantic difference between the dma_fence_wait and >> the wait_event interface? >> >> I mean the wait_event interface was introduced to prevent drivers from >> openly coding an event interface and getting it wrong all the time. >> >> So a good part of the bugs we have seen around waiting for dma-fences >> are exactly why wait_event was invented in the first place. >> >> The only big thing I can see missing in the wait_event interface is >> waiting for many events at the same time, but that should be a rather >> easy addition. > So this bikeshed was years ago, maybe I should type a patch to > document it, but as far as I remember the big difference is: > > - wait_event and friends generally Just Work. It can go wrong of > course, but the usual pattern is that the waker-side does and > uncoditional wake_up_all, and hence all the waiter needs to do is add > themselves to the waiter list. > > - dma_buf otoh is entirely different: We wanted to support all kinds > fo signalling modes, including having interrupts disabled by default > (not sure whether we actually achieve this still with all the cpu-side > scheduling the big drivers do). Which means the waker does not > unconditionally call wake_up_all, at least not timeline, and waiters > need to call dma_fence_enable_signalling before they can add > themselves to the waiter list and call schedule(). Well that is not something I'm questioning because we really need this behavior as well. But all of this can be perfectly implemented on top of wake_up_all. > The other bit difference is how you check for the classic wakeup races > where the event happens between when you checked for it and when you > go to sleep. Because hw is involved, the rules are again a bit > different, and their different between drivers because hw is > incoherent/broken in all kinds of ways. So there's also really tricky > things going on between adding the waiter to the waiter list and > dma_fence_enable_signalling. For pure cpu events you can ignore this > and bake the few necessary barriers into the various macros, dma_fence > needs more. Ah, yes I think I know what you mean with that and I also consider this a bad idea as well. Only very few drivers actually need this behavior and the ones who do should be perfectly able to implement this inside the driver code. The crux is that leaking this behavior into the dma-fence made it unnecessary complicated and result in quite a bunch of unnecessary irq_work and delayed_work usage. I will take a look at this over the holidays. Shouldn't be to hard to fix and actually has some value additional to being just a nice cleanup. Regards, Christian. > > Adding Maarten, maybe there was more. I definitely remember huge&very > long discussions about all this. > -Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/3] drm/msm/dpu: add display port support in DPU
Add display port support in DPU by creating hooks
for DP encoder enumeration and encoder mode
initialization.
This change is based on the SDM845 Display port
driver changes[1].
changes in v2:
- rebase on [2] (Sean Paul)
- remove unwanted error checks and
switch cases (Jordan Crouse)
changes in v3:
- add dp support after fixing
the current code base for error logging (Sean Paul)
[1] https://lwn.net/Articles/768265/
[2] https://lkml.org/lkml/2018/11/17/87
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 ++---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 51 +
2 files changed, 49 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 36158b7..b79fd9d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2029,7 +2029,7 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt *dpu_enc,
{
int ret = 0;
int i = 0;
- enum dpu_intf_type intf_type;
+ enum dpu_intf_type intf_type = INTF_NONE;
struct dpu_enc_phys_init_params phys_params;
if (!dpu_enc || !dpu_kms) {
@@ -2052,9 +2052,9 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt *dpu_enc,
case DRM_MODE_ENCODER_DSI:
intf_type = INTF_DSI;
break;
- default:
- DPU_ERROR_ENC(dpu_enc, "unsupported display interface type\n");
- return -EINVAL;
+ case DRM_MODE_ENCODER_TMDS:
+ intf_type = INTF_DP;
+ break;
}
WARN_ON(disp_info->num_of_h_tiles < 1);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 39c8549..ba3e75c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -437,6 +437,32 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
return rc;
}
+static int _dpu_kms_initialize_displayport(struct drm_device *dev,
+ struct msm_drm_private *priv,
+ struct dpu_kms *dpu_kms)
+{
+ struct drm_encoder *encoder = NULL;
+ int rc = 0;
+
+ if (!priv->dp)
+ return rc;
+
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
+ if (IS_ERR(encoder)) {
+ DPU_ERROR("encoder init failed for dsi display\n");
+ return PTR_ERR(encoder);
+ }
+
+ rc = msm_dp_modeset_init(priv->dp, dev, encoder);
+ if (rc) {
+ DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
+ drm_encoder_cleanup(encoder);
+ return rc;
+ }
+
+ return rc;
+}
+
/**
* _dpu_kms_setup_displays - create encoders, bridges and connectors
* for underlying displays
@@ -457,6 +483,12 @@ static int _dpu_kms_setup_displays(struct drm_device *dev,
return rc;
}
+ rc = _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
+ if (rc) {
+ DPU_ERROR("failed to initialize display port, rc = %d\n", rc);
+ return rc;
+ }
+
/**
* Extend this function to initialize other
* types of displays
@@ -675,13 +707,20 @@ static void _dpu_kms_set_encoder_mode(struct msm_kms *kms,
info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
MSM_DISPLAY_CAP_VID_MODE;
- /* TODO: No support for DSI swap */
- for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
- if (priv->dsi[i]) {
- info.h_tile_instance[info.num_of_h_tiles] = i;
- info.num_of_h_tiles++;
+ switch (info.intf_type) {
+ case DRM_MODE_ENCODER_DSI:
+ /* TODO: No support for DSI swap */
+ for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
+ if (priv->dsi[i]) {
+ info.h_tile_instance[info.num_of_h_tiles] = i;
+ info.num_of_h_tiles++;
+ }
}
- }
+ break;
+ case DRM_MODE_ENCODER_TMDS:
+ info.num_of_h_tiles = 1;
+ break;
+ };
rc = dpu_encoder_setup(encoder->dev, encoder, &info);
if (rc)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/3] drm/msm/dpu: handle failures while initializing displays
Bail out KMS hw init on display initialization failures with
proper error logging.
changes in v3:
- introduced in the series
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 39 +++--
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 685686e..39c8549 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -405,33 +405,36 @@ static void dpu_kms_wait_for_commit_done(struct msm_kms
*kms,
}
}
-static void _dpu_kms_initialize_dsi(struct drm_device *dev,
+static int _dpu_kms_initialize_dsi(struct drm_device *dev,
struct msm_drm_private *priv,
struct dpu_kms *dpu_kms)
{
struct drm_encoder *encoder = NULL;
- int i, rc;
+ int i, rc = 0;
+
+ if (!(priv->dsi[0] || priv->dsi[1]))
+ return rc;
/*TODO: Support two independent DSI connectors */
encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
- if (IS_ERR_OR_NULL(encoder)) {
+ if (IS_ERR(encoder)) {
DPU_ERROR("encoder init failed for dsi display\n");
- return;
+ return PTR_ERR(encoder);
}
for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
- if (!priv->dsi[i]) {
- DPU_DEBUG("invalid msm_dsi for ctrl %d\n", i);
- return;
- }
+ if (!priv->dsi[i])
+ continue;
rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
if (rc) {
DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
i, rc);
- continue;
+ return rc;
}
}
+
+ return rc;
}
/**
@@ -442,16 +445,24 @@ static void _dpu_kms_initialize_dsi(struct drm_device
*dev,
* @dpu_kms:Pointer to dpu kms structure
* Returns: Zero on success
*/
-static void _dpu_kms_setup_displays(struct drm_device *dev,
+static int _dpu_kms_setup_displays(struct drm_device *dev,
struct msm_drm_private *priv,
struct dpu_kms *dpu_kms)
{
- _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
+ int rc = 0;
+
+ rc = _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
+ if (rc) {
+ DPU_ERROR("failed to initialize dsi, rc = %d\n", rc);
+ return rc;
+ }
/**
* Extend this function to initialize other
* types of displays
*/
+
+ return rc;
}
static void _dpu_kms_drm_obj_destroy(struct dpu_kms *dpu_kms)
@@ -517,7 +528,11 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
* Create encoder and query display drivers to create
* bridges and connectors
*/
- _dpu_kms_setup_displays(dev, priv, dpu_kms);
+ ret = _dpu_kms_setup_displays(dev, priv, dpu_kms);
+ if (ret) {
+ DPU_ERROR("failed to setup display, rc = %d\n", ret);
+ goto fail;
+ }
max_crtc_count = min(catalog->mixer_count,
(u32)dev->mode_config.num_encoder);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/3] drm/msm/dpu: fix documentation for intf_type
Fix intf_type description in msm_disp_info to show that
it represents drm encoder mode of the display.
changes in v3:
- introduced in the series
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/msm_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 60a7da0..4b0ae70 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -119,7 +119,7 @@ struct msm_display_topology {
/**
* struct msm_display_info - defines display properties
- * @intf_type: DRM_MODE_CONNECTOR_ display type
+ * @intf_type: DRM_MODE_ENCODER_ type
* @capabilities: Bitmask of display flags
* @num_of_h_tiles: Number of horizontal tiles in case of split interface
* @h_tile_instance:Controller instance used per tile. Number of elements
is
--
The Qualcomm Innovation Center, Inc. is a member of the 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 v6 2/2] arm64: dts: sdm845: Add gpu and gmu device nodes
Hi,
On Wed, Dec 12, 2018 at 1:18 PM Jordan Crouse wrote:
>
> Add the nodes to describe the Adreno GPU and GMU devices.
>
> Signed-off-by: Jordan Crouse
> ---
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 123 +++
> 1 file changed, 123 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 233a5898ebc2..a608afed502e 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -11,6 +11,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -1349,6 +1350,128 @@
> };
> };
>
> +
> + gpu@500 {
nit that you're adding an extra blank line here that you don't need.
Given the quantity of outstanding dts patches though, it's almost
certain that Andy will need to manually resolve conflicts when
applying this patch so presumably he can fix up when he lands.
In any case, feel free to add:
Reviewed-by: Douglas Anderson
Tested-by: Douglas Anderson
-Doug
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 1/2] dt-bindings: drm/msm/a6xx: Document GMU and update GPU bindings
Hi, On Wed, Dec 12, 2018 at 1:18 PM Jordan Crouse wrote: > diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt > b/Documentation/devicetree/bindings/display/msm/gpu.txt > index 43fac0fe09bb..8d9415180c22 100644 > --- a/Documentation/devicetree/bindings/display/msm/gpu.txt > +++ b/Documentation/devicetree/bindings/display/msm/gpu.txt > @@ -8,14 +8,21 @@ Required properties: >with the chip-id. > - reg: Physical base address and length of the controller's registers. > - interrupts: The interrupt signal from the gpu. > -- clocks: device clocks > +- interrupt-names: List of names for the interrupt signals. The following > can be > + provided: > + * "kgsl_3d0_irq" > +- clocks: device clocks (if applicable) >See ../clocks/clock-bindings.txt for details. > -- clock-names: the following clocks are required: > +- clock-names: the following clocks can be provided: >* "core" >* "iface" >* "mem_iface" > +- iommus: optional phandle to an adreno iommu instance > +- operating-points-v2: optional phandle to the OPP operating points > +- qcom,gmu: For a6xx and newer targets a phandle to the GMU device that will > + control the power for the GPU Seems fine to me. If you happen to spin it again for some other reason it might be nice to be more explicit about exactly when clocks are required and when they aren't. IIUC they are always required on systems without a GMU and they are never present on systems with a GMU. ...but I wouldn't spin it just for that. Reviewed-by: Douglas Anderson ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] arm64: dts: sdm845: Add interconnect for GPU
Hi, On Thu, Dec 13, 2018 at 9:32 AM Jordan Crouse wrote: > > Define an interconnect port for the GPU to set bus > capabilities. > > Signed-off-by: Jordan Crouse > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi > b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 0a5ddfc4c59b..9323f211ca40 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -1370,6 +1370,9 @@ > > operating-points-v2 = <&gpu_opp_table>; > > + interconnects = <&qnoc MASTER_GFX3D &qnoc SLAVE_EBI1>; > + interconnect-names = "gfx-mem"; Thanks for sending this patch. Unfortunately the interconnect node is not called "qnoc" anymore. It's called "&rsc_hlos" in v12 of Georgi's patchset: https://lkml.kernel.org/r/20181208170216.32555-7-georgi.dja...@linaro.org Can you repost with that fix (the bindings patch should be updated too)? Other than that feel feel free to add my Reviewed-by to this patch. Thanks! -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201273] Fatal error during GPU init amdgpu RX560
https://bugzilla.kernel.org/show_bug.cgi?id=201273 --- Comment #22 from Alex Deucher (alexdeuc...@gmail.com) --- You'll also need this updated firmware: https://people.freedesktop.org/~agd5f/radeon_ucode/polaris11_k_mc.bin -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drivers/base: use a worker for sysfs unbind
On Thu, Dec 13, 2018 at 5:25 PM Daniel Vetter wrote: > > On Thu, Dec 13, 2018 at 5:18 PM Rafael J. Wysocki wrote: > > > > On Thu, Dec 13, 2018 at 1:36 PM Daniel Vetter > > wrote: [cut] > > > I can do the old code exactly, but afaict the non-NULL parent just > > > takes care of the parent bus locking for us, instead of hand-rolling > > > it in the caller. But if I missed something, I can easily undo that > > > part. > > > > It is different if device links are present, but I'm not worried about > > that case honestly. :-) > > What would change with device links? We have some cleanup plans to > remove our usage for early/late s/r hooks with a device link, to make > sure i915 resumes before snd_hda_intel. Digging more into the code I > only see the temporary dropping of the parent's device_lock, but I > have no idea what that even implies ... That's just it (which is why I said I was not worried). Running device_links_unbind_consumers() with the parent lock held may deadlock if another child of the same parent also is a consumer of the current device (which really is a corner case), but the current code has this problem - it goes away with your change. But dev->bus->need_parent_lock checks are missing in there AFAICS, let me cut a patch to fix that. Cheers, Rafael ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 35/39] misc/mei/hdcp: Component framework for I915 Interface
On Thu, Dec 13, 2018 at 5:27 PM Winkler, Tomas wrote: > > > On Thu, Dec 13, 2018 at 1:36 PM C, Ramalingam > > wrote: > > > > > > Tomas and Daniel, > > > > > > We got an issue here. > > > > > > The relationship that we try to build between I915 and mei_hdcp is as > > > follows: > > > > > > We are using the components to establish the relationship. > > > I915 is component master where as mei_hdcp is component. > > > I915 adds the component master during the module load. mei_hdcp adds the > > component when the driver->probe is called (on device driver binding). > > > I915 forces itself such that until mei_hdcp component is added I915_load > > wont be complete. > > > Similarly on complete system, if mei_hdcp component is removed, > > immediately I915 unregister itself and HW will be shutdown. > > > > > > This is completely fine when the modules are loaded and unloaded. > > > > > > But during suspend, mei device disappears and mei bus handles it by > > unbinding device and driver by calling driver->remove. > > > This in-turn removes the component and triggers the master unbind of I915 > > where, I915 unregister itself. > > > This cause the HW state mismatch during the suspend and resume. > > > > > > Please check the powerwell mismatch errors at CI report for v9 > > > https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_3412/fi-glk-j4005/igt@ > > > gem_exec_susp...@basic-s3.html > > > > > > More over unregistering I915 during the suspend is not expected. So how do > > we handle this? > > > > Bit more context from our irc discussion with Ram: > > > > I found this very surprising, since I don't know of any other subsystems > > where > > the devices get outright removed when going through a suspend/resume cycle. > > The device model was built to handle this stuff > > correctly: First clients/devices/interfaces get suspend, then the > > parent/bridge/bus. Same dance in reverse when resuming. This even holds for > > lots of hotpluggable buses, where child devices could indeed disappear on > > resume, but as long as they don't, everything stays the same. It's really > > surprising for something that's soldered onto the board like ME. > > HDCP is an application in the ME it's not ME itself.. On the linux side > HDCP2 is a virtual device on mei client virtual bus, > the bus is teared down on ME reset, which mostly happen on power > transitions. > Theoretically, we could keep it up during power transitions, but so fare it > was not necessary > and second it's not guarantie that the all ME applications will reappear > after reset. When does this happen that an ME application doesn't come back after e.g. suspend/resume? Also, what's all the place where this reset can happen? Just suspend/resume/hibernate and all these, or also at other times? How does userspace deal with the reset over s/r? I'm assuming that at least the device node file will become invalid (or whatever you're using as userspace api), so if userspace is accessing stuff on the me at the same time as we do a suspend/resume, what happens? > > Aside: We'll probably need a device_link to make sure mei_hdcp is fully > > resumed before i915 gets resumed, but that's kinda a detail for later on. > > Frankly I don’t believe there is currently exact abstraction that supports > this model, > neither components nor device_link . > So fare we used class interface for other purposes, it worked well. I'm not clear on what class interface has to do with component or device link. They all solve different problems, at least as far as I understand all this stuff ... -Daniel > > Tomas, can you pls explain why mei is designed like this? Or is there > > something > > else we're missing (I didn't dig through the mei bus in detail at all, so > > not clear > > on what exactly is going on there). > Above. > > > > Also pulling in device model and suspend/resume experts. > > > > Thanks, Daniel > > > > > > > > -Ram > > > > > > On 12/13/2018 9:31 AM, Ramalingam C wrote: > > > > > > Mei hdcp driver is designed as component slave for the I915 component > > > master. > > > > > > v2: Rebased. > > > v3: > > > Notifier chain is adopted for cldev state update [Tomas] > > > v4: > > > Made static dummy functions as inline in mei_hdcp.h > > > API for polling client device status > > > IS_ENABLED used in header, for config status for mei_hdcp. > > > v5: > > > Replacing the notifier with component framework. [Daniel] > > > v6: > > > Rebased on the I915 comp master redesign. > > > v7: > > > mei_hdcp_component_registered is made static [Uma] > > > Need for global static variable mei_cldev is removed. > > > > > > Signed-off-by: Ramalingam C > > > Reviewed-by: Uma Shankar > > > --- > > > drivers/misc/mei/hdcp/mei_hdcp.c | 67 > > > +--- > > > 1 file changed, 63 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c > > > b/drivers/misc/mei/hdcp/mei_hdcp.c > > > index b22a71e8c5d7..3de1700dcc9f 1006
[PATCH 2/2] arm64: dts: sdm845: Add interconnect for GPU
Define an interconnect port for the GPU to set bus
capabilities.
Signed-off-by: Jordan Crouse
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0a5ddfc4c59b..9323f211ca40 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1370,6 +1370,9 @@
operating-points-v2 = <&gpu_opp_table>;
+ interconnects = <&qnoc MASTER_GFX3D &qnoc SLAVE_EBI1>;
+ interconnect-names = "gfx-mem";
+
qcom,gmu = <&gmu>;
gpu_opp_table: opp-table {
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] arm64: dts: sdm845: Add sdm845 GPU interconnect
Two quick patches to document and add an interconnect port definition for the sdm845 GPU. This is based on the base GPU DT changes: https://patchwork.freedesktop.org/series/39308/ As well as the DT nodes from Georgi: https://patchwork.kernel.org/patch/10719483/ Jordan Crouse (2): dt-bindings: drm/msm/a6xx: Document interconnect properties for GPU arm64: dts: sdm845: Add interconnect for GPU Documentation/devicetree/bindings/display/msm/gpu.txt | 6 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 3 +++ 2 files changed, 9 insertions(+) -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] dt-bindings: drm/msm/a6xx: Document interconnect properties for GPU
Add documentation for the interconnect and interconnect-names bindings for the GPU node as detailed by bindings/interconnect/interconnect.txt. Signed-off-by: Jordan Crouse --- Documentation/devicetree/bindings/display/msm/gpu.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt index 8d9415180c22..be6b5628e052 100644 --- a/Documentation/devicetree/bindings/display/msm/gpu.txt +++ b/Documentation/devicetree/bindings/display/msm/gpu.txt @@ -19,6 +19,9 @@ Required properties: * "mem_iface" - iommus: optional phandle to an adreno iommu instance - operating-points-v2: optional phandle to the OPP operating points +- interconnect: optional phandle to a interconnect provider. See + ../interconnect/interconnect.txt for details. +- interconnect-names: Name string for the interconnects. - qcom,gmu: For a6xx and newer targets a phandle to the GMU device that will control the power for the GPU @@ -68,6 +71,9 @@ Example a6xx (with GMU): operating-points-v2 = <&gpu_opp_table>; + interconnects = <&qnoc MASTER_GFX3D &qnoc SLAVE_EBI1>; + interconnect-names = "gfx-mem"; + qcom,gmu = <&gmu>; }; }; -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108992] Regression: Lenovo e585 (ryzen 2500u) freezes during boot with 4.20-rc5/rc6, amdgpu error
https://bugs.freedesktop.org/show_bug.cgi?id=108992 --- Comment #5 from chris --- Can you boot the system without amdgpu loaded (e.g., append modprobe.blacklist=amdgpu) -> Doing this, i am able to boot my system. -- 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 201985] mc: Failed to load firmware "amdgpu/polaris11_k_mc.bin"
https://bugzilla.kernel.org/show_bug.cgi?id=201985 --- Comment #3 from nancy (nancy...@gmx.com) --- Thank you for the quick hint: Now the system starts without problems. $ uname -a Linux tux 4.20.0-rc6 # 1 SMP Thu Dec 13 18:15:31 CET 2018 x86_64 AMD Ryzen 5 1600 Six-Core Processor AuthenticAMD GNU / Linux Solved -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 06/10] drm/syncobj: use the timeline point in drm_syncobj_find_fence v3
On Thu, Dec 13, 2018 at 5:47 PM Koenig, Christian
wrote:
>
> Am 13.12.18 um 17:01 schrieb Daniel Vetter:
> > On Thu, Dec 13, 2018 at 12:24:57PM +, Koenig, Christian wrote:
> >> Am 13.12.18 um 13:21 schrieb Chris Wilson:
> >>> Quoting Koenig, Christian (2018-12-13 12:11:10)
> Am 13.12.18 um 12:37 schrieb Chris Wilson:
> > Quoting Chunming Zhou (2018-12-11 10:34:45)
> >> From: Christian König
> >>
> >> Implement finding the right timeline point in drm_syncobj_find_fence.
> >>
> >> v2: return -EINVAL when the point is not submitted yet.
> >> v3: fix reference counting bug, add flags handling as well
> >>
> >> Signed-off-by: Christian König
> >> ---
> >> drivers/gpu/drm/drm_syncobj.c | 43
> >> ---
> >> 1 file changed, 40 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_syncobj.c
> >> b/drivers/gpu/drm/drm_syncobj.c
> >> index 76ce13dafc4d..d964b348ecba 100644
> >> --- a/drivers/gpu/drm/drm_syncobj.c
> >> +++ b/drivers/gpu/drm/drm_syncobj.c
> >> @@ -231,16 +231,53 @@ int drm_syncobj_find_fence(struct drm_file
> >> *file_private,
> >> struct dma_fence **fence)
> >> {
> >>struct drm_syncobj *syncobj =
> >> drm_syncobj_find(file_private, handle);
> >> - int ret = 0;
> >> + struct syncobj_wait_entry wait;
> >> + int ret;
> >>
> >>if (!syncobj)
> >>return -ENOENT;
> >>
> >>*fence = drm_syncobj_fence_get(syncobj);
> >> - if (!*fence) {
> >> + drm_syncobj_put(syncobj);
> >> +
> >> + if (*fence) {
> >> + ret = dma_fence_chain_find_seqno(fence, point);
> >> + if (!ret)
> >> + return 0;
> >> + dma_fence_put(*fence);
> >> + } else {
> >>ret = -EINVAL;
> >>}
> >> - drm_syncobj_put(syncobj);
> >> +
> >> + if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT))
> >> + return ret;
> >> +
> >> + memset(&wait, 0, sizeof(wait));
> >> + wait.task = current;
> >> + wait.point = point;
> >> + drm_syncobj_fence_add_wait(syncobj, &wait);
> >> +
> >> + do {
> >> + set_current_state(TASK_INTERRUPTIBLE);
> >> + if (wait.fence) {
> >> + ret = 0;
> >> + break;
> >> + }
> >> +
> >> + if (signal_pending(current)) {
> >> + ret = -ERESTARTSYS;
> >> + break;
> >> + }
> >> +
> >> + schedule();
> >> + } while (1);
> > I've previously used a dma_fence_proxy so that we could do nonblocking
> > waits on future submits. That would be preferrable (a requirement for
> > our stupid BKL-driven code).
> That is exactly what I would definitely NAK.
>
> I would rather say we should come up with a wait_multiple_events() macro
> and completely nuke the custom implementation of this in:
> 1. dma_fence_default_wait and dma_fence_wait_any_timeout
> 2. the radeon fence implementation
> 3. the nouveau fence implementation
> 4. the syncobj code
>
> Cause all of them do exactly the same. The dma_fence implementation
> unfortunately came up with a custom event handling mechanism instead of
> extending the core Linux wait_event() system.
> >>> I don't want a blocking wait at all.
> >> Ok I wasn't clear enough :) That is exactly what I would NAK!
> >>
> >> The wait must be blocking or otherwise you would allow wait-before-signal.
> > Well the current implementation is pulling a rather big trick on readers
> > in this regard: It looks like a dma_fence, it's implemented as one even,
> > heck you even open-code a dma_fence_wait here.
> >
> > Except the semantics are completely different.
> >
> > So aside from the discussion whether we really want to fully chain them I
> > think it just doesn't make sense to implement the "wait for fence submit"
> > as a dma_fence wait. And I'd outright remove that part from the uapi, and
> > force the wait. The current radv/anv plans I discussed with Jason was that
> > we'd have a separate submit thread, and hence unconditionally blocking
> > until the fence has materialized is the right thing to do. Even allowing
> > that option, either through a flag, or making these things look like
> > dma_fences (they are _not_) just tricks folks into misunderstanding what's
> > going on.
>
> Good, that sounds strongly like something I can agree on as well.
>
> > Code sharing just because the code looks similar is imo a really
> > bad idea, when the semantics are entirely different
[Bug 201273] Fatal error during GPU init amdgpu RX560
https://bugzilla.kernel.org/show_bug.cgi?id=201273 --- Comment #21 from quirin.blae...@freenet.de --- (In reply to Alex Deucher from comment #20) > Can you try one of these branches? > https://cgit.freedesktop.org/~agd5f/linux/log/?h=amd-staging-drm-next > https://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-next-4.21-wip > Do they help? As i understand, the first one matches v4.19. So this may be a candidate, but your server is slow. I'll try it again, when US is sleeping. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/3] mm/mmu_notifier: contextual information for event triggering invalidation v2
On Thu, Dec 13, 2018 at 9:14 AM wrote: > > From: Jérôme Glisse > > CPU page table update can happens for many reasons, not only as a result > of a syscall (munmap(), mprotect(), mremap(), madvise(), ...) but also > as a result of kernel activities (memory compression, reclaim, migration, > ...). > > Users of mmu notifier API track changes to the CPU page table and take > specific action for them. While current API only provide range of virtual > address affected by the change, not why the changes is happening. > > This patchset adds event information so that users of mmu notifier can > differentiate among broad category: > - UNMAP: munmap() or mremap() > - CLEAR: page table is cleared (migration, compaction, reclaim, ...) > - PROTECTION_VMA: change in access protections for the range > - PROTECTION_PAGE: change in access protections for page in the range > - SOFT_DIRTY: soft dirtyness tracking > > Being able to identify munmap() and mremap() from other reasons why the > page table is cleared is important to allow user of mmu notifier to > update their own internal tracking structure accordingly (on munmap or > mremap it is not longer needed to track range of virtual address as it > becomes invalid). Who consumes these new enum values? The consumer of the new infrastructure should be included in the patchset that adds the new functionality. So a NAK from me until the consumer is clarified / included. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/3] mm/mmu_notifier: contextual information for event triggering invalidation v2
From: Jérôme Glisse
CPU page table update can happens for many reasons, not only as a result
of a syscall (munmap(), mprotect(), mremap(), madvise(), ...) but also
as a result of kernel activities (memory compression, reclaim, migration,
...).
Users of mmu notifier API track changes to the CPU page table and take
specific action for them. While current API only provide range of virtual
address affected by the change, not why the changes is happening.
This patchset adds event information so that users of mmu notifier can
differentiate among broad category:
- UNMAP: munmap() or mremap()
- CLEAR: page table is cleared (migration, compaction, reclaim, ...)
- PROTECTION_VMA: change in access protections for the range
- PROTECTION_PAGE: change in access protections for page in the range
- SOFT_DIRTY: soft dirtyness tracking
Being able to identify munmap() and mremap() from other reasons why the
page table is cleared is important to allow user of mmu notifier to
update their own internal tracking structure accordingly (on munmap or
mremap it is not longer needed to track range of virtual address as it
becomes invalid).
Changes since v1:
- use mmu_notifier_range_init() helper to to optimize out the case
when mmu notifier is not enabled
- use kernel doc format for describing the enum values
Signed-off-by: Jérôme Glisse
Acked-by: Christian König
Acked-by: Jan Kara
Acked-by: Felix Kuehling
Acked-by: Jason Gunthorpe
Cc: Andrew Morton
Cc: Matthew Wilcox
Cc: Ross Zwisler
Cc: Dan Williams
Cc: Paolo Bonzini
Cc: Radim Krčmář
Cc: Michal Hocko
Cc: Christian Koenig
Cc: Ralph Campbell
Cc: John Hubbard
Cc: k...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-r...@vger.kernel.org
Cc: linux-fsde...@vger.kernel.org
Cc: Arnd Bergmann
---
fs/dax.c | 7 +++
fs/proc/task_mmu.c | 3 ++-
include/linux/mmu_notifier.h | 35 +--
kernel/events/uprobes.c | 3 ++-
mm/huge_memory.c | 12
mm/hugetlb.c | 10 ++
mm/khugepaged.c | 3 ++-
mm/ksm.c | 6 --
mm/madvise.c | 3 ++-
mm/memory.c | 18 --
mm/migrate.c | 5 +++--
mm/mprotect.c| 3 ++-
mm/mremap.c | 3 ++-
mm/oom_kill.c| 2 +-
mm/rmap.c| 6 --
15 files changed, 90 insertions(+), 29 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index 874085bacaf5..6056b03a1626 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -768,6 +768,13 @@ static void dax_entry_mkclean(struct address_space
*mapping, pgoff_t index,
address = pgoff_address(index, vma);
+ /*
+* All the field are populated by follow_pte_pmd() except
+* the event field.
+*/
+ mmu_notifier_range_init(&range, NULL, 0, -1UL,
+ MMU_NOTIFY_PROTECTION_PAGE);
+
/*
* Note because we provide start/end to follow_pte_pmd it will
* call mmu_notifier_invalidate_range_start() on our behalf
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index b3ddceb003bc..f68a9ebb0218 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1141,7 +1141,8 @@ static ssize_t clear_refs_write(struct file *file, const
char __user *buf,
break;
}
- mmu_notifier_range_init(&range, mm, 0, -1UL);
+ mmu_notifier_range_init(&range, mm, 0, -1UL,
+ MMU_NOTIFY_SOFT_DIRTY);
mmu_notifier_invalidate_range_start(&range);
}
walk_page_range(0, mm->highest_vm_end, &clear_refs_walk);
diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
index 39b06772427f..d249e24acea5 100644
--- a/include/linux/mmu_notifier.h
+++ b/include/linux/mmu_notifier.h
@@ -25,10 +25,39 @@ struct mmu_notifier_mm {
spinlock_t lock;
};
+/**
+ * enum mmu_notifier_event - reason for the mmu notifier callback
+ * @MMU_NOTIFY_UNMAP: either munmap() that unmap the range or a mremap() that
+ * move the range
+ *
+ * @MMU_NOTIFY_CLEAR: clear page table entry (many reasons for this like
+ * madvise() or replacing a page by another one, ...).
+ *
+ * @MMU_NOTIFY_PROTECTION_VMA: update is due to protection change for the range
+ * ie using the vma access permission (vm_page_prot) to update the whole range
+ * is enough no need to inspect changes to the CPU page table (mprotect()
+ * syscall)
+ *
+ * @MMU_NOTIFY_PROTECTION_PAGE: update is due to change in read/write flag for
+ * pages in the range so to mirror those changes the user must inspect the CPU
+ * page table (from the end callback).
+ *
+ * @MMU_NOTIFY_SOFT_DIRTY: soft dirty accounting (sti
[PATCH v3 2/3] mm/mmu_notifier: use structure for invalidate_range_start/end calls v3
From: Jérôme Glisse
To avoid having to change many call sites everytime we want to add a
parameter use a structure to group all parameters for the mmu_notifier
invalidate_range_start/end cakks. No functional changes with this
patch.
Changes since v2:
- fix build warning in migrate.c when CONFIG_MMU_NOTIFIER=n
Changes since v1:
- introduce mmu_notifier_range_init() as an helper to initialize
the range structure allowing to optimize out the case when mmu
notifier is not enabled
- fix mm/migrate.c migrate_vma_collect()
Signed-off-by: Jérôme Glisse
Acked-by: Christian König
Acked-by: Jan Kara
Acked-by: Felix Kuehling
Acked-by: Jason Gunthorpe
Cc: Andrew Morton
Cc: Matthew Wilcox
Cc: Ross Zwisler
Cc: Dan Williams
Cc: Paolo Bonzini
Cc: Radim Krčmář
Cc: Michal Hocko
Cc: Christian Koenig
Cc: Ralph Campbell
Cc: John Hubbard
Cc: k...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-r...@vger.kernel.org
Cc: linux-fsde...@vger.kernel.org
Cc: Arnd Bergmann
---
fs/dax.c | 8 +--
fs/proc/task_mmu.c | 7 ++-
include/linux/mm.h | 4 +-
include/linux/mmu_notifier.h | 87 +---
kernel/events/uprobes.c | 10 ++--
mm/huge_memory.c | 54 ++--
mm/hugetlb.c | 52 ++-
mm/khugepaged.c | 10 ++--
mm/ksm.c | 21
mm/madvise.c | 21
mm/memory.c | 97 ++--
mm/migrate.c | 28 +--
mm/mmu_notifier.c| 35 +++--
mm/mprotect.c| 15 +++---
mm/mremap.c | 10 ++--
mm/oom_kill.c| 17 ---
mm/rmap.c| 30 ++-
17 files changed, 259 insertions(+), 247 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index 9bcce89ea18e..874085bacaf5 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -758,7 +758,8 @@ static void dax_entry_mkclean(struct address_space
*mapping, pgoff_t index,
i_mmap_lock_read(mapping);
vma_interval_tree_foreach(vma, &mapping->i_mmap, index, index) {
- unsigned long address, start, end;
+ struct mmu_notifier_range range;
+ unsigned long address;
cond_resched();
@@ -772,7 +773,8 @@ static void dax_entry_mkclean(struct address_space
*mapping, pgoff_t index,
* call mmu_notifier_invalidate_range_start() on our behalf
* before taking any lock.
*/
- if (follow_pte_pmd(vma->vm_mm, address, &start, &end, &ptep,
&pmdp, &ptl))
+ if (follow_pte_pmd(vma->vm_mm, address, &range,
+ &ptep, &pmdp, &ptl))
continue;
/*
@@ -814,7 +816,7 @@ static void dax_entry_mkclean(struct address_space
*mapping, pgoff_t index,
pte_unmap_unlock(ptep, ptl);
}
- mmu_notifier_invalidate_range_end(vma->vm_mm, start, end);
+ mmu_notifier_invalidate_range_end(&range);
}
i_mmap_unlock_read(mapping);
}
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 47c3764c469b..b3ddceb003bc 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1096,6 +1096,7 @@ static ssize_t clear_refs_write(struct file *file, const
char __user *buf,
return -ESRCH;
mm = get_task_mm(task);
if (mm) {
+ struct mmu_notifier_range range;
struct clear_refs_private cp = {
.type = type,
};
@@ -1139,11 +1140,13 @@ static ssize_t clear_refs_write(struct file *file,
const char __user *buf,
downgrade_write(&mm->mmap_sem);
break;
}
- mmu_notifier_invalidate_range_start(mm, 0, -1);
+
+ mmu_notifier_range_init(&range, mm, 0, -1UL);
+ mmu_notifier_invalidate_range_start(&range);
}
walk_page_range(0, mm->highest_vm_end, &clear_refs_walk);
if (type == CLEAR_REFS_SOFT_DIRTY)
- mmu_notifier_invalidate_range_end(mm, 0, -1);
+ mmu_notifier_invalidate_range_end(&range);
tlb_finish_mmu(&tlb, 0, -1);
up_read(&mm->mmap_sem);
out_mm:
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 5411de93a363..e7b6f2b30713 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1397,6 +1397,8 @@ struct mm_walk {
void *private;
};
+struct mmu_notifier_range;
+
int walk_page_range(unsigned long addr, unsigned long end,
struct mm_walk *walk);
int walk_page_vma(struct vm_area_struct *vma, struct mm_walk *walk);
@@ -1405,7 +1407,7 @@ void free_pgd_range(struct mmu_gather *tlb, unsigned l
[PATCH v3 1/3] mm/mmu_notifier: use structure for invalidate_range_start/end callback v2
From: Jérôme Glisse
To avoid having to change many callback definition everytime we want
to add a parameter use a structure to group all parameters for the
mmu_notifier invalidate_range_start/end callback. No functional changes
with this patch.
Changed since v1:
- fix make htmldocs warning in amdgpu_mn.c
Signed-off-by: Jérôme Glisse
Acked-by: Jan Kara
Acked-by: Felix Kuehling
Acked-by: Jason Gunthorpe
Cc: Andrew Morton
Cc: Matthew Wilcox
Cc: Ross Zwisler
Cc: Dan Williams
Cc: Paolo Bonzini
Cc: Radim Krčmář
Cc: Michal Hocko
Cc: Christian Koenig
Cc: Ralph Campbell
Cc: John Hubbard
Cc: k...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-r...@vger.kernel.org
Cc: linux-fsde...@vger.kernel.org
Cc: Arnd Bergmann
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 47 +++--
drivers/gpu/drm/i915/i915_gem_userptr.c | 14
drivers/gpu/drm/radeon/radeon_mn.c | 16 -
drivers/infiniband/core/umem_odp.c | 20 +--
drivers/infiniband/hw/hfi1/mmu_rb.c | 13 +++
drivers/misc/mic/scif/scif_dma.c| 11 ++
drivers/misc/sgi-gru/grutlbpurge.c | 14
drivers/xen/gntdev.c| 12 +++
include/linux/mmu_notifier.h| 14 +---
mm/hmm.c| 23 +---
mm/mmu_notifier.c | 21 +--
virt/kvm/kvm_main.c | 14 +++-
12 files changed, 103 insertions(+), 116 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index e55508b39496..3e6823fdd939 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -238,44 +238,40 @@ static void amdgpu_mn_invalidate_node(struct
amdgpu_mn_node *node,
* amdgpu_mn_invalidate_range_start_gfx - callback to notify about mm change
*
* @mn: our notifier
- * @mm: the mm this callback is about
- * @start: start of updated range
- * @end: end of updated range
+ * @range: mmu notifier context
*
* Block for operations on BOs to finish and mark pages as accessed and
* potentially dirty.
*/
static int amdgpu_mn_invalidate_range_start_gfx(struct mmu_notifier *mn,
-struct mm_struct *mm,
-unsigned long start,
-unsigned long end,
-bool blockable)
+ const struct mmu_notifier_range *range)
{
struct amdgpu_mn *amn = container_of(mn, struct amdgpu_mn, mn);
struct interval_tree_node *it;
+ unsigned long end;
/* notification is exclusive, but interval is inclusive */
- end -= 1;
+ end = range->end - 1;
/* TODO we should be able to split locking for interval tree and
* amdgpu_mn_invalidate_node
*/
- if (amdgpu_mn_read_lock(amn, blockable))
+ if (amdgpu_mn_read_lock(amn, range->blockable))
return -EAGAIN;
- it = interval_tree_iter_first(&amn->objects, start, end);
+ it = interval_tree_iter_first(&amn->objects, range->start, end);
while (it) {
struct amdgpu_mn_node *node;
- if (!blockable) {
+ if (!range->blockable) {
amdgpu_mn_read_unlock(amn);
return -EAGAIN;
}
node = container_of(it, struct amdgpu_mn_node, it);
- it = interval_tree_iter_next(it, start, end);
+ it = interval_tree_iter_next(it, range->start, end);
- amdgpu_mn_invalidate_node(node, start, end);
+ amdgpu_mn_invalidate_node(node, range->start, end);
}
return 0;
@@ -294,39 +290,38 @@ static int amdgpu_mn_invalidate_range_start_gfx(struct
mmu_notifier *mn,
* are restorted in amdgpu_mn_invalidate_range_end_hsa.
*/
static int amdgpu_mn_invalidate_range_start_hsa(struct mmu_notifier *mn,
-struct mm_struct *mm,
-unsigned long start,
-unsigned long end,
-bool blockable)
+ const struct mmu_notifier_range *range)
{
struct amdgpu_mn *amn = container_of(mn, struct amdgpu_mn, mn);
struct interval_tree_node *it;
+ unsigned long end;
/* notification is exclusive, but interval is inclusive */
- end -= 1;
+ end = range->end - 1;
- if (amdgpu_mn_read_lock(amn, blockable))
+ if (amdgpu_mn_read_lock(amn, range->blockable))
return -EAGAIN;
- it = interval_tree_iter_first(&amn->objects, start, end);
+ it = interval_tree_iter_first(&amn->objects, range->start, end);
while (it) {
struct amd
[PATCH v3 0/3] mmu notifier contextual informations
From: Jérôme Glisse Changes since v2: - fix build warning with CONFIG_MMU_NOTIFIER=n - fix make htmldocs warning Changes since v1: - fix build with CONFIG_MMU_NOTIFIER=n - kernel docs Original cover letter: This patchset add contextual information, why an invalidation is happening, to mmu notifier callback. This is necessary for user of mmu notifier that wish to maintains their own data structure without having to add new fields to struct vm_area_struct (vma). For instance device can have they own page table that mirror the process address space. When a vma is unmap (munmap() syscall) the device driver can free the device page table for the range. Today we do not have any information on why a mmu notifier call back is happening and thus device driver have to assume that it is always an munmap(). This is inefficient at it means that it needs to re-allocate device page table on next page fault and rebuild the whole device driver data structure for the range. Other use case beside munmap() also exist, for instance it is pointless for device driver to invalidate the device page table when the invalidation is for the soft dirtyness tracking. Or device driver can optimize away mprotect() that change the page table permission access for the range. This patchset enable all this optimizations for device driver. I do not include any of those in this serie but other patchset i am posting will leverage this. From code point of view the patchset is pretty simple, the first two patches consolidate all mmu notifier arguments into a struct so that it is easier to add/change arguments. The last patch adds the contextual information (munmap, protection, soft dirty, clear, ...). Cheers, Jérôme Cc: Christian König Cc: Jan Kara Cc: Felix Kuehling Cc: Jason Gunthorpe Cc: Andrew Morton Cc: Matthew Wilcox Cc: Ross Zwisler Cc: Dan Williams Cc: Paolo Bonzini Cc: Radim Krčmář Cc: Michal Hocko Cc: Christian Koenig Cc: Ralph Campbell Cc: John Hubbard Cc: k...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-r...@vger.kernel.org Cc: linux-fsde...@vger.kernel.org Cc: Arnd Bergmann Jérôme Glisse (3): mm/mmu_notifier: use structure for invalidate_range_start/end callback v2 mm/mmu_notifier: use structure for invalidate_range_start/end calls v3 mm/mmu_notifier: contextual information for event triggering invalidation v2 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 47 - drivers/gpu/drm/i915/i915_gem_userptr.c | 14 ++- drivers/gpu/drm/radeon/radeon_mn.c | 16 ++- drivers/infiniband/core/umem_odp.c | 20 ++-- drivers/infiniband/hw/hfi1/mmu_rb.c | 13 +-- drivers/misc/mic/scif/scif_dma.c| 11 +- drivers/misc/sgi-gru/grutlbpurge.c | 14 ++- drivers/xen/gntdev.c| 12 +-- fs/dax.c| 15 ++- fs/proc/task_mmu.c | 8 +- include/linux/mm.h | 4 +- include/linux/mmu_notifier.h| 132 ++-- kernel/events/uprobes.c | 11 +- mm/hmm.c| 23 ++--- mm/huge_memory.c| 58 +-- mm/hugetlb.c| 54 +- mm/khugepaged.c | 11 +- mm/ksm.c| 23 ++--- mm/madvise.c| 22 ++-- mm/memory.c | 103 +- mm/migrate.c| 29 +++--- mm/mmu_notifier.c | 22 ++-- mm/mprotect.c | 16 +-- mm/mremap.c | 11 +- mm/oom_kill.c | 17 +-- mm/rmap.c | 32 +++--- virt/kvm/kvm_main.c | 14 +-- 27 files changed, 406 insertions(+), 346 deletions(-) -- 2.17.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 06/10] drm/syncobj: use the timeline point in drm_syncobj_find_fence v3
Am 13.12.18 um 17:01 schrieb Daniel Vetter:
> On Thu, Dec 13, 2018 at 12:24:57PM +, Koenig, Christian wrote:
>> Am 13.12.18 um 13:21 schrieb Chris Wilson:
>>> Quoting Koenig, Christian (2018-12-13 12:11:10)
Am 13.12.18 um 12:37 schrieb Chris Wilson:
> Quoting Chunming Zhou (2018-12-11 10:34:45)
>> From: Christian König
>>
>> Implement finding the right timeline point in drm_syncobj_find_fence.
>>
>> v2: return -EINVAL when the point is not submitted yet.
>> v3: fix reference counting bug, add flags handling as well
>>
>> Signed-off-by: Christian König
>> ---
>> drivers/gpu/drm/drm_syncobj.c | 43
>> ---
>> 1 file changed, 40 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_syncobj.c
>> b/drivers/gpu/drm/drm_syncobj.c
>> index 76ce13dafc4d..d964b348ecba 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -231,16 +231,53 @@ int drm_syncobj_find_fence(struct drm_file
>> *file_private,
>> struct dma_fence **fence)
>> {
>>struct drm_syncobj *syncobj = drm_syncobj_find(file_private,
>> handle);
>> - int ret = 0;
>> + struct syncobj_wait_entry wait;
>> + int ret;
>>
>>if (!syncobj)
>>return -ENOENT;
>>
>>*fence = drm_syncobj_fence_get(syncobj);
>> - if (!*fence) {
>> + drm_syncobj_put(syncobj);
>> +
>> + if (*fence) {
>> + ret = dma_fence_chain_find_seqno(fence, point);
>> + if (!ret)
>> + return 0;
>> + dma_fence_put(*fence);
>> + } else {
>>ret = -EINVAL;
>>}
>> - drm_syncobj_put(syncobj);
>> +
>> + if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT))
>> + return ret;
>> +
>> + memset(&wait, 0, sizeof(wait));
>> + wait.task = current;
>> + wait.point = point;
>> + drm_syncobj_fence_add_wait(syncobj, &wait);
>> +
>> + do {
>> + set_current_state(TASK_INTERRUPTIBLE);
>> + if (wait.fence) {
>> + ret = 0;
>> + break;
>> + }
>> +
>> + if (signal_pending(current)) {
>> + ret = -ERESTARTSYS;
>> + break;
>> + }
>> +
>> + schedule();
>> + } while (1);
> I've previously used a dma_fence_proxy so that we could do nonblocking
> waits on future submits. That would be preferrable (a requirement for
> our stupid BKL-driven code).
That is exactly what I would definitely NAK.
I would rather say we should come up with a wait_multiple_events() macro
and completely nuke the custom implementation of this in:
1. dma_fence_default_wait and dma_fence_wait_any_timeout
2. the radeon fence implementation
3. the nouveau fence implementation
4. the syncobj code
Cause all of them do exactly the same. The dma_fence implementation
unfortunately came up with a custom event handling mechanism instead of
extending the core Linux wait_event() system.
>>> I don't want a blocking wait at all.
>> Ok I wasn't clear enough :) That is exactly what I would NAK!
>>
>> The wait must be blocking or otherwise you would allow wait-before-signal.
> Well the current implementation is pulling a rather big trick on readers
> in this regard: It looks like a dma_fence, it's implemented as one even,
> heck you even open-code a dma_fence_wait here.
>
> Except the semantics are completely different.
>
> So aside from the discussion whether we really want to fully chain them I
> think it just doesn't make sense to implement the "wait for fence submit"
> as a dma_fence wait. And I'd outright remove that part from the uapi, and
> force the wait. The current radv/anv plans I discussed with Jason was that
> we'd have a separate submit thread, and hence unconditionally blocking
> until the fence has materialized is the right thing to do. Even allowing
> that option, either through a flag, or making these things look like
> dma_fences (they are _not_) just tricks folks into misunderstanding what's
> going on.
Good, that sounds strongly like something I can agree on as well.
> Code sharing just because the code looks similar is imo a really
> bad idea, when the semantics are entirely different (that was also the
> reason behind not reusing all the cpu event stuff for dma_fence, they're
> not normal cpu events).
Ok, the last sentence is what I don't understand.
What exactly is the semantic difference between the dma_fence_wait and
the wait_event interface?
[Bug 201985] mc: Failed to load firmware "amdgpu/polaris11_k_mc.bin"
https://bugzilla.kernel.org/show_bug.cgi?id=201985 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #2 from Alex Deucher (alexdeuc...@gmail.com) --- The patch was sent to linux-firmware a couple of weeks ago. It should land any day now. In the mean time you can grab it here: https://people.freedesktop.org/~agd5f/radeon_ucode/ -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201985] mc: Failed to load firmware "amdgpu/polaris11_k_mc.bin"
https://bugzilla.kernel.org/show_bug.cgi?id=201985 --- Comment #1 from nancy (nancy...@gmx.com) --- Created attachment 27 --> https://bugzilla.kernel.org/attachment.cgi?id=27&action=edit dmesg-4.20.0-rc6 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v9 35/39] misc/mei/hdcp: Component framework for I915 Interface
> On Thu, Dec 13, 2018 at 1:36 PM C, Ramalingam
> wrote:
> >
> > Tomas and Daniel,
> >
> > We got an issue here.
> >
> > The relationship that we try to build between I915 and mei_hdcp is as
> > follows:
> >
> > We are using the components to establish the relationship.
> > I915 is component master where as mei_hdcp is component.
> > I915 adds the component master during the module load. mei_hdcp adds the
> component when the driver->probe is called (on device driver binding).
> > I915 forces itself such that until mei_hdcp component is added I915_load
> wont be complete.
> > Similarly on complete system, if mei_hdcp component is removed,
> immediately I915 unregister itself and HW will be shutdown.
> >
> > This is completely fine when the modules are loaded and unloaded.
> >
> > But during suspend, mei device disappears and mei bus handles it by
> unbinding device and driver by calling driver->remove.
> > This in-turn removes the component and triggers the master unbind of I915
> where, I915 unregister itself.
> > This cause the HW state mismatch during the suspend and resume.
> >
> > Please check the powerwell mismatch errors at CI report for v9
> > https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_3412/fi-glk-j4005/igt@
> > gem_exec_susp...@basic-s3.html
> >
> > More over unregistering I915 during the suspend is not expected. So how do
> we handle this?
>
> Bit more context from our irc discussion with Ram:
>
> I found this very surprising, since I don't know of any other subsystems where
> the devices get outright removed when going through a suspend/resume cycle.
> The device model was built to handle this stuff
> correctly: First clients/devices/interfaces get suspend, then the
> parent/bridge/bus. Same dance in reverse when resuming. This even holds for
> lots of hotpluggable buses, where child devices could indeed disappear on
> resume, but as long as they don't, everything stays the same. It's really
> surprising for something that's soldered onto the board like ME.
HDCP is an application in the ME it's not ME itself.. On the linux side HDCP2
is a virtual device on mei client virtual bus,
the bus is teared down on ME reset, which mostly happen on power transitions.
Theoretically, we could keep it up during power transitions, but so fare it
was not necessary
and second it's not guarantie that the all ME applications will reappear after
reset.
>
> Aside: We'll probably need a device_link to make sure mei_hdcp is fully
> resumed before i915 gets resumed, but that's kinda a detail for later on.
Frankly I don’t believe there is currently exact abstraction that supports this
model,
neither components nor device_link .
So fare we used class interface for other purposes, it worked well.
>
> Tomas, can you pls explain why mei is designed like this? Or is there
> something
> else we're missing (I didn't dig through the mei bus in detail at all, so not
> clear
> on what exactly is going on there).
Above.
>
> Also pulling in device model and suspend/resume experts.
>
> Thanks, Daniel
>
> >
> > -Ram
> >
> > On 12/13/2018 9:31 AM, Ramalingam C wrote:
> >
> > Mei hdcp driver is designed as component slave for the I915 component
> > master.
> >
> > v2: Rebased.
> > v3:
> > Notifier chain is adopted for cldev state update [Tomas]
> > v4:
> > Made static dummy functions as inline in mei_hdcp.h
> > API for polling client device status
> > IS_ENABLED used in header, for config status for mei_hdcp.
> > v5:
> > Replacing the notifier with component framework. [Daniel]
> > v6:
> > Rebased on the I915 comp master redesign.
> > v7:
> > mei_hdcp_component_registered is made static [Uma]
> > Need for global static variable mei_cldev is removed.
> >
> > Signed-off-by: Ramalingam C
> > Reviewed-by: Uma Shankar
> > ---
> > drivers/misc/mei/hdcp/mei_hdcp.c | 67
> > +---
> > 1 file changed, 63 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c
> > b/drivers/misc/mei/hdcp/mei_hdcp.c
> > index b22a71e8c5d7..3de1700dcc9f 100644
> > --- a/drivers/misc/mei/hdcp/mei_hdcp.c
> > +++ b/drivers/misc/mei/hdcp/mei_hdcp.c
> > @@ -23,11 +23,14 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > #include "mei_hdcp.h"
> >
> > +static bool mei_hdcp_component_registered;
> > +
> > /**
> > * mei_initiate_hdcp2_session() - Initiate a Wired HDCP2.2 Tx Session in ME
> FW
> > * @dev: device corresponding to the mei_cl_device @@ -691,8 +694,7
> > @@ mei_close_hdcp_session(struct device *dev, struct hdcp_port_data
> *data)
> > return 0;
> > }
> >
> > -static __attribute__((unused))
> > -struct i915_hdcp_component_ops mei_hdcp_ops = {
> > +static struct i915_hdcp_component_ops mei_hdcp_ops = {
> > .owner = THIS_MODULE,
> > .initiate_hdcp2_session = mei_initiate_hdcp2_session,
> > .verify_receiver_cert_prepare_km =
> > mei_verify_receiver_cert_prepare_km,
> > @@ -707,20 +709,77 @@
Re: [PATCH] drivers/base: use a worker for sysfs unbind
On Thu, Dec 13, 2018 at 5:18 PM Rafael J. Wysocki wrote: > > On Thu, Dec 13, 2018 at 1:36 PM Daniel Vetter wrote: > > > > On Thu, Dec 13, 2018 at 11:23 AM Rafael J. Wysocki > > wrote: > > > > > > On Thu, Dec 13, 2018 at 10:58 AM Daniel Vetter wrote: > > > > > > > > On Thu, Dec 13, 2018 at 10:38:14AM +0100, Rafael J. Wysocki wrote: > > > > > On Mon, Dec 10, 2018 at 9:47 AM Daniel Vetter > > > > > wrote: > > > > > > > > > > > > Drivers might want to remove some sysfs files, which needs the same > > > > > > locks and ends up angering lockdep. Relevant snippet of the stack > > > > > > trace: > > > > > > > > > > > > kernfs_remove_by_name_ns+0x3b/0x80 > > > > > > bus_remove_driver+0x92/0xa0 > > > > > > acpi_video_unregister+0x24/0x40 > > > > > > i915_driver_unload+0x42/0x130 [i915] > > > > > > i915_pci_remove+0x19/0x30 [i915] > > > > > > pci_device_remove+0x36/0xb0 > > > > > > device_release_driver_internal+0x185/0x250 > > > > > > unbind_store+0xaf/0x180 > > > > > > kernfs_fop_write+0x104/0x190 > > > > > > > > > > Is the acpi_bus_unregister_driver() in acpi_video_unregister() the > > > > > source of the lockdep unhappiness? > > > > > > > > Yeah I guess I cut out too much of the lockdep splat. It complains about > > > > kernfs_fop_write and kernfs_remove_by_name_ns acquiring the same lock > > > > class. It's ofc not the same lock, so no real deadlock. Getting the > > > > device_release_driver outside of the callchain under kernfs_fop_write, > > > > which this patch does, "fixes" it. For "fixes" = shut up lockdep. > > > > > > OK, so the problem really is that the operation is started via sysfs > > > which means that this code is running under a lock already. > > > > > > Which lock does lockdep complain about, exactly? > > > > mutex_lock(&of->mutex); > > OK (I thought so) > > > > > Other options: > > > > - Anotate the recursion with the usual lockdep annotations. Potentially > > > > results in lockdep not catching real deadlocks (you can still have > > > > other > > > > loops closing the deadlock, maybe through some subsystem/bus lock). > > > > > > > > - Rewrite kernfs_fop_write to drop the lock (optionally, for callbacks > > > > that know what they're doing), which should be fine if we refcount > > > > everything properly (bus, driver & device). > > > > > > > > - Also note that probably the same bug exists on the bind sysfs > > > > interface, > > > > but we don't use that, so I don't care :-) > > > > > > > > - Most of these issues are never visible in normal usage, since normally > > > > driver bind/unbind is done from a kthread or model_load/unload, > > > > neither > > > > of which is running in the context of that kernfs mutex > > > > kernfs_fop_write > > > > holds. That's why I think the task work is the best solution, since it > > > > changes the locking context of the unbind sysfs to match the locking > > > > context of module unload and hotunplug. > > > > > > I think that using a task work here makes sense. There is a drawback, > > > which is that the original sysfs write will not wait for the driver to > > > actually be released before returning to user space AFAICS, but that > > > probably isn't a big deal. > > > > This would happen with a normal work_struct, which runs on some other > > thread eventually. That added asynonchrouns execution uncovered lots > > of bugs in our CI (fbcon isn't solid, let's put it that way). Hence > > the task work, which will be run before the syscall returns to > > userspace, but outside of anything else. Was originally created to > > avoid locking inversion on the final fput, where the same "must > > complete before returning to userspace, but outside of any other > > locking context" issue was causing trouble. > > I didn't realize that it would run completely before returning to user > space, thanks for pointing this out. > > This isn't an issue then. > > > > Also please note that the patch changes the code flow slightly, > > > because passing a non-NULL parent pointer to > > > device_release_driver_internal() potentially has side effects, but > > > that should not be a big deal either. > > > > I can do the old code exactly, but afaict the non-NULL parent just > > takes care of the parent bus locking for us, instead of hand-rolling > > it in the caller. But if I missed something, I can easily undo that > > part. > > It is different if device links are present, but I'm not worried about > that case honestly. :-) What would change with device links? We have some cleanup plans to remove our usage for early/late s/r hooks with a device link, to make sure i915 resumes before snd_hda_intel. Digging more into the code I only see the temporary dropping of the parent's device_lock, but I have no idea what that even implies ... -Daniel > > > > > Unfortunately that trick doesn't work for the bind sysfs file, since > > > > that way we can't thread the errno value back to userspace. > > > > > > Right. That is unless we wait for the o
[Bug 201985] New: mc: Failed to load firmware "amdgpu/polaris11_k_mc.bin"
https://bugzilla.kernel.org/show_bug.cgi?id=201985 Bug ID: 201985 Summary: mc: Failed to load firmware "amdgpu/polaris11_k_mc.bin" Product: Drivers Version: 2.5 Kernel Version: 4.20.0-rc6 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: nancy...@gmx.com Regression: No Created attachment 279995 --> https://bugzilla.kernel.org/attachment.cgi?id=279995&action=edit Config-4.20-rc6 Kernel 4.20.0-rc5 starts fine and is stable on my system. With the same .config I get kernel 4.20.0-rc6 in dmesg the error given above: "mc: Failed to load firmware" amdgpu / polaris11_k_mc.bin "". The GPU will not initialize and Xorg will not start. I found this e-mail: https://lists.freedesktop.org/archives/amd-gfx/2018-December/029175.html The problem is, in Linux firmware there is no polaris11_k_mc.bin. There is only one polaris11_k_smc.bin. My question: is this a typo or will polaris11_k_mc.bin be delivered soon? Thank you for your attention -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drivers/base: use a worker for sysfs unbind
On Thu, Dec 13, 2018 at 1:36 PM Daniel Vetter wrote: > > On Thu, Dec 13, 2018 at 11:23 AM Rafael J. Wysocki wrote: > > > > On Thu, Dec 13, 2018 at 10:58 AM Daniel Vetter wrote: > > > > > > On Thu, Dec 13, 2018 at 10:38:14AM +0100, Rafael J. Wysocki wrote: > > > > On Mon, Dec 10, 2018 at 9:47 AM Daniel Vetter > > > > wrote: > > > > > > > > > > Drivers might want to remove some sysfs files, which needs the same > > > > > locks and ends up angering lockdep. Relevant snippet of the stack > > > > > trace: > > > > > > > > > > kernfs_remove_by_name_ns+0x3b/0x80 > > > > > bus_remove_driver+0x92/0xa0 > > > > > acpi_video_unregister+0x24/0x40 > > > > > i915_driver_unload+0x42/0x130 [i915] > > > > > i915_pci_remove+0x19/0x30 [i915] > > > > > pci_device_remove+0x36/0xb0 > > > > > device_release_driver_internal+0x185/0x250 > > > > > unbind_store+0xaf/0x180 > > > > > kernfs_fop_write+0x104/0x190 > > > > > > > > Is the acpi_bus_unregister_driver() in acpi_video_unregister() the > > > > source of the lockdep unhappiness? > > > > > > Yeah I guess I cut out too much of the lockdep splat. It complains about > > > kernfs_fop_write and kernfs_remove_by_name_ns acquiring the same lock > > > class. It's ofc not the same lock, so no real deadlock. Getting the > > > device_release_driver outside of the callchain under kernfs_fop_write, > > > which this patch does, "fixes" it. For "fixes" = shut up lockdep. > > > > OK, so the problem really is that the operation is started via sysfs > > which means that this code is running under a lock already. > > > > Which lock does lockdep complain about, exactly? > > mutex_lock(&of->mutex); OK (I thought so) > > > Other options: > > > - Anotate the recursion with the usual lockdep annotations. Potentially > > > results in lockdep not catching real deadlocks (you can still have other > > > loops closing the deadlock, maybe through some subsystem/bus lock). > > > > > > - Rewrite kernfs_fop_write to drop the lock (optionally, for callbacks > > > that know what they're doing), which should be fine if we refcount > > > everything properly (bus, driver & device). > > > > > > - Also note that probably the same bug exists on the bind sysfs interface, > > > but we don't use that, so I don't care :-) > > > > > > - Most of these issues are never visible in normal usage, since normally > > > driver bind/unbind is done from a kthread or model_load/unload, neither > > > of which is running in the context of that kernfs mutex kernfs_fop_write > > > holds. That's why I think the task work is the best solution, since it > > > changes the locking context of the unbind sysfs to match the locking > > > context of module unload and hotunplug. > > > > I think that using a task work here makes sense. There is a drawback, > > which is that the original sysfs write will not wait for the driver to > > actually be released before returning to user space AFAICS, but that > > probably isn't a big deal. > > This would happen with a normal work_struct, which runs on some other > thread eventually. That added asynonchrouns execution uncovered lots > of bugs in our CI (fbcon isn't solid, let's put it that way). Hence > the task work, which will be run before the syscall returns to > userspace, but outside of anything else. Was originally created to > avoid locking inversion on the final fput, where the same "must > complete before returning to userspace, but outside of any other > locking context" issue was causing trouble. I didn't realize that it would run completely before returning to user space, thanks for pointing this out. This isn't an issue then. > > Also please note that the patch changes the code flow slightly, > > because passing a non-NULL parent pointer to > > device_release_driver_internal() potentially has side effects, but > > that should not be a big deal either. > > I can do the old code exactly, but afaict the non-NULL parent just > takes care of the parent bus locking for us, instead of hand-rolling > it in the caller. But if I missed something, I can easily undo that > part. It is different if device links are present, but I'm not worried about that case honestly. :-) > > > Unfortunately that trick doesn't work for the bind sysfs file, since that > > > way we can't thread the errno value back to userspace. > > > > Right. That is unless we wait for the operation to complete and check > > the error left behind by it. That should be doable, but somewhat > > complicated. > > For real deadlocks this doesn't fix anything, it just hides it from > lockdep. cross-release lockdep would still complain. If we want to fix > the bind side _and_ keep reporting the errno from the driver's bind > function, then we need to rework kernfs to and add a callback which > doesn't hold the mutex. Should be doable, just a pile more work. It should be possible to store the error in a variable and export that via a separate attribute for user space to inspect. That would be a
Re: [PATCH v9 35/39] misc/mei/hdcp: Component framework for I915 Interface
On Thu, Dec 13, 2018 at 1:36 PM C, Ramalingam wrote:
>
> Tomas and Daniel,
>
> We got an issue here.
>
> The relationship that we try to build between I915 and mei_hdcp is as follows:
>
> We are using the components to establish the relationship.
> I915 is component master where as mei_hdcp is component.
> I915 adds the component master during the module load. mei_hdcp adds the
> component when the driver->probe is called (on device driver binding).
> I915 forces itself such that until mei_hdcp component is added I915_load wont
> be complete.
> Similarly on complete system, if mei_hdcp component is removed, immediately
> I915 unregister itself and HW will be shutdown.
>
> This is completely fine when the modules are loaded and unloaded.
>
> But during suspend, mei device disappears and mei bus handles it by unbinding
> device and driver by calling driver->remove.
> This in-turn removes the component and triggers the master unbind of I915
> where, I915 unregister itself.
> This cause the HW state mismatch during the suspend and resume.
>
> Please check the powerwell mismatch errors at CI report for v9
> https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_3412/fi-glk-j4005/igt@gem_exec_susp...@basic-s3.html
>
> More over unregistering I915 during the suspend is not expected. So how do we
> handle this?
Bit more context from our irc discussion with Ram:
I found this very surprising, since I don't know of any other
subsystems where the devices get outright removed when going through a
suspend/resume cycle. The device model was built to handle this stuff
correctly: First clients/devices/interfaces get suspend, then the
parent/bridge/bus. Same dance in reverse when resuming. This even
holds for lots of hotpluggable buses, where child devices could indeed
disappear on resume, but as long as they don't, everything stays the
same. It's really surprising for something that's soldered onto the
board like ME.
Aside: We'll probably need a device_link to make sure mei_hdcp is
fully resumed before i915 gets resumed, but that's kinda a detail for
later on.
Tomas, can you pls explain why mei is designed like this? Or is there
something else we're missing (I didn't dig through the mei bus in
detail at all, so not clear on what exactly is going on there).
Also pulling in device model and suspend/resume experts.
Thanks, Daniel
>
> -Ram
>
> On 12/13/2018 9:31 AM, Ramalingam C wrote:
>
> Mei hdcp driver is designed as component slave for the I915 component
> master.
>
> v2: Rebased.
> v3:
> Notifier chain is adopted for cldev state update [Tomas]
> v4:
> Made static dummy functions as inline in mei_hdcp.h
> API for polling client device status
> IS_ENABLED used in header, for config status for mei_hdcp.
> v5:
> Replacing the notifier with component framework. [Daniel]
> v6:
> Rebased on the I915 comp master redesign.
> v7:
> mei_hdcp_component_registered is made static [Uma]
> Need for global static variable mei_cldev is removed.
>
> Signed-off-by: Ramalingam C
> Reviewed-by: Uma Shankar
> ---
> drivers/misc/mei/hdcp/mei_hdcp.c | 67
> +---
> 1 file changed, 63 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c
> b/drivers/misc/mei/hdcp/mei_hdcp.c
> index b22a71e8c5d7..3de1700dcc9f 100644
> --- a/drivers/misc/mei/hdcp/mei_hdcp.c
> +++ b/drivers/misc/mei/hdcp/mei_hdcp.c
> @@ -23,11 +23,14 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> #include "mei_hdcp.h"
>
> +static bool mei_hdcp_component_registered;
> +
> /**
> * mei_initiate_hdcp2_session() - Initiate a Wired HDCP2.2 Tx Session in ME
> FW
> * @dev: device corresponding to the mei_cl_device
> @@ -691,8 +694,7 @@ mei_close_hdcp_session(struct device *dev, struct
> hdcp_port_data *data)
> return 0;
> }
>
> -static __attribute__((unused))
> -struct i915_hdcp_component_ops mei_hdcp_ops = {
> +static struct i915_hdcp_component_ops mei_hdcp_ops = {
> .owner = THIS_MODULE,
> .initiate_hdcp2_session = mei_initiate_hdcp2_session,
> .verify_receiver_cert_prepare_km = mei_verify_receiver_cert_prepare_km,
> @@ -707,20 +709,77 @@ struct i915_hdcp_component_ops mei_hdcp_ops = {
> .close_hdcp_session = mei_close_hdcp_session,
> };
>
> +static int mei_hdcp_component_bind(struct device *mei_kdev,
> + struct device *i915_kdev, void *data)
> +{
> + struct i915_component_master *master_comp = data;
> +
> + dev_info(mei_kdev, "MEI HDCP comp bind\n");
> + WARN_ON(master_comp->hdcp_ops);
> + master_comp->hdcp_ops = &mei_hdcp_ops;
> + master_comp->mei_dev = mei_kdev;
> +
> + return 0;
> +}
> +
> +static void mei_hdcp_component_unbind(struct device *mei_kdev,
> + struct device *i915_kdev, void *data)
> +{
> + struct i915_component_master *master_comp = data;
> +
> + dev_info(mei_kdev, "MEI HDCP comp unbind\n");
> + master_comp->hdcp_ops = NULL;
> + master_comp->mei_dev = NULL;
> +}
> +
> +static const struct component_ops mei_hdcp_component_
Re: [Intel-gfx] [PATCH 06/10] drm/syncobj: use the timeline point in drm_syncobj_find_fence v3
On Thu, Dec 13, 2018 at 12:24:57PM +, Koenig, Christian wrote:
> Am 13.12.18 um 13:21 schrieb Chris Wilson:
> > Quoting Koenig, Christian (2018-12-13 12:11:10)
> >> Am 13.12.18 um 12:37 schrieb Chris Wilson:
> >>> Quoting Chunming Zhou (2018-12-11 10:34:45)
> From: Christian König
>
> Implement finding the right timeline point in drm_syncobj_find_fence.
>
> v2: return -EINVAL when the point is not submitted yet.
> v3: fix reference counting bug, add flags handling as well
>
> Signed-off-by: Christian König
> ---
> drivers/gpu/drm/drm_syncobj.c | 43 ---
> 1 file changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c
> b/drivers/gpu/drm/drm_syncobj.c
> index 76ce13dafc4d..d964b348ecba 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -231,16 +231,53 @@ int drm_syncobj_find_fence(struct drm_file
> *file_private,
> struct dma_fence **fence)
> {
> struct drm_syncobj *syncobj = drm_syncobj_find(file_private,
> handle);
> - int ret = 0;
> + struct syncobj_wait_entry wait;
> + int ret;
>
> if (!syncobj)
> return -ENOENT;
>
> *fence = drm_syncobj_fence_get(syncobj);
> - if (!*fence) {
> + drm_syncobj_put(syncobj);
> +
> + if (*fence) {
> + ret = dma_fence_chain_find_seqno(fence, point);
> + if (!ret)
> + return 0;
> + dma_fence_put(*fence);
> + } else {
> ret = -EINVAL;
> }
> - drm_syncobj_put(syncobj);
> +
> + if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT))
> + return ret;
> +
> + memset(&wait, 0, sizeof(wait));
> + wait.task = current;
> + wait.point = point;
> + drm_syncobj_fence_add_wait(syncobj, &wait);
> +
> + do {
> + set_current_state(TASK_INTERRUPTIBLE);
> + if (wait.fence) {
> + ret = 0;
> + break;
> + }
> +
> + if (signal_pending(current)) {
> + ret = -ERESTARTSYS;
> + break;
> + }
> +
> + schedule();
> + } while (1);
> >>> I've previously used a dma_fence_proxy so that we could do nonblocking
> >>> waits on future submits. That would be preferrable (a requirement for
> >>> our stupid BKL-driven code).
> >> That is exactly what I would definitely NAK.
> >>
> >> I would rather say we should come up with a wait_multiple_events() macro
> >> and completely nuke the custom implementation of this in:
> >> 1. dma_fence_default_wait and dma_fence_wait_any_timeout
> >> 2. the radeon fence implementation
> >> 3. the nouveau fence implementation
> >> 4. the syncobj code
> >>
> >> Cause all of them do exactly the same. The dma_fence implementation
> >> unfortunately came up with a custom event handling mechanism instead of
> >> extending the core Linux wait_event() system.
> > I don't want a blocking wait at all.
>
> Ok I wasn't clear enough :) That is exactly what I would NAK!
>
> The wait must be blocking or otherwise you would allow wait-before-signal.
Well the current implementation is pulling a rather big trick on readers
in this regard: It looks like a dma_fence, it's implemented as one even,
heck you even open-code a dma_fence_wait here.
Except the semantics are completely different.
So aside from the discussion whether we really want to fully chain them I
think it just doesn't make sense to implement the "wait for fence submit"
as a dma_fence wait. And I'd outright remove that part from the uapi, and
force the wait. The current radv/anv plans I discussed with Jason was that
we'd have a separate submit thread, and hence unconditionally blocking
until the fence has materialized is the right thing to do. Even allowing
that option, either through a flag, or making these things look like
dma_fences (they are _not_) just tricks folks into misunderstanding what's
going on. Code sharing just because the code looks similar is imo a really
bad idea, when the semantics are entirely different (that was also the
reason behind not reusing all the cpu event stuff for dma_fence, they're
not normal cpu events).
-Daniel
>
> Christian.
>
> > -Chris
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_
Re: [PATCH] drm/xen-front: Make shmem backed display buffer coherent
On Thu, Dec 13, 2018 at 12:17:54PM +0200, Oleksandr Andrushchenko wrote:
> Daniel, could you please comment?
Cross-revieweing someone else's stuff would scale better, I don't think
I'll get around to anything before next year.
-Daniel
>
> Thank you
>
> On 11/27/18 12:32 PM, Oleksandr Andrushchenko wrote:
> > From: Oleksandr Andrushchenko
> >
> > When GEM backing storage is allocated with drm_gem_get_pages
> > the backing pages may be cached, thus making it possible that
> > the backend sees only partial content of the buffer which may
> > lead to screen artifacts. Make sure that the frontend's
> > memory is coherent and the backend always sees correct display
> > buffer content.
> >
> > Fixes: c575b7eeb89f ("drm/xen-front: Add support for Xen PV display
> > frontend")
> >
> > Signed-off-by: Oleksandr Andrushchenko
> > ---
> > drivers/gpu/drm/xen/xen_drm_front_gem.c | 62 +++--
> > 1 file changed, 48 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c
> > b/drivers/gpu/drm/xen/xen_drm_front_gem.c
> > index 47ff019d3aef..c592735e49d2 100644
> > --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
> > +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
> > @@ -33,8 +33,11 @@ struct xen_gem_object {
> > /* set for buffers allocated by the backend */
> > bool be_alloc;
> > - /* this is for imported PRIME buffer */
> > - struct sg_table *sgt_imported;
> > + /*
> > +* this is for imported PRIME buffer or the one allocated via
> > +* drm_gem_get_pages.
> > +*/
> > + struct sg_table *sgt;
> > };
> > static inline struct xen_gem_object *
> > @@ -77,10 +80,21 @@ static struct xen_gem_object *gem_create_obj(struct
> > drm_device *dev,
> > return xen_obj;
> > }
> > +struct sg_table *xen_drm_front_gem_get_sg_table(struct drm_gem_object
> > *gem_obj)
> > +{
> > + struct xen_gem_object *xen_obj = to_xen_gem_obj(gem_obj);
> > +
> > + if (!xen_obj->pages)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + return drm_prime_pages_to_sg(xen_obj->pages, xen_obj->num_pages);
> > +}
> > +
> > static struct xen_gem_object *gem_create(struct drm_device *dev, size_t
> > size)
> > {
> > struct xen_drm_front_drm_info *drm_info = dev->dev_private;
> > struct xen_gem_object *xen_obj;
> > + struct address_space *mapping;
> > int ret;
> > size = round_up(size, PAGE_SIZE);
> > @@ -113,10 +127,14 @@ static struct xen_gem_object *gem_create(struct
> > drm_device *dev, size_t size)
> > xen_obj->be_alloc = true;
> > return xen_obj;
> > }
> > +
> > /*
> > * need to allocate backing pages now, so we can share those
> > * with the backend
> > */
> > + mapping = xen_obj->base.filp->f_mapping;
> > + mapping_set_gfp_mask(mapping, GFP_USER | __GFP_DMA32);
> > +
> > xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
> > xen_obj->pages = drm_gem_get_pages(&xen_obj->base);
> > if (IS_ERR_OR_NULL(xen_obj->pages)) {
> > @@ -125,8 +143,27 @@ static struct xen_gem_object *gem_create(struct
> > drm_device *dev, size_t size)
> > goto fail;
> > }
> > + xen_obj->sgt = xen_drm_front_gem_get_sg_table(&xen_obj->base);
> > + if (IS_ERR_OR_NULL(xen_obj->sgt)){
> > + ret = PTR_ERR(xen_obj->sgt);
> > + xen_obj->sgt = NULL;
> > + goto fail_put_pages;
> > + }
> > +
> > + if (!dma_map_sg(dev->dev, xen_obj->sgt->sgl, xen_obj->sgt->nents,
> > + DMA_BIDIRECTIONAL)) {
> > + ret = -EFAULT;
> > + goto fail_free_sgt;
> > + }
> > +
> > return xen_obj;
> > +fail_free_sgt:
> > + sg_free_table(xen_obj->sgt);
> > + xen_obj->sgt = NULL;
> > +fail_put_pages:
> > + drm_gem_put_pages(&xen_obj->base, xen_obj->pages, true, false);
> > + xen_obj->pages = NULL;
> > fail:
> > DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
> > return ERR_PTR(ret);
> > @@ -149,7 +186,7 @@ void xen_drm_front_gem_free_object_unlocked(struct
> > drm_gem_object *gem_obj)
> > struct xen_gem_object *xen_obj = to_xen_gem_obj(gem_obj);
> > if (xen_obj->base.import_attach) {
> > - drm_prime_gem_destroy(&xen_obj->base, xen_obj->sgt_imported);
> > + drm_prime_gem_destroy(&xen_obj->base, xen_obj->sgt);
> > gem_free_pages_array(xen_obj);
> > } else {
> > if (xen_obj->pages) {
> > @@ -158,6 +195,13 @@ void xen_drm_front_gem_free_object_unlocked(struct
> > drm_gem_object *gem_obj)
> > xen_obj->pages);
> > gem_free_pages_array(xen_obj);
> > } else {
> > + if (xen_obj->sgt) {
> > + dma_unmap_sg(xen_obj->base.dev->dev,
> > +xen_obj->sgt->sgl,
> > +xen_obj->sgt->nents,
> > +
[PATCH v4 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
Implement the exec_mipi_pmic_seq_element callback for the CHT Whiskey Cove
PMIC.
On some CHT devices this fixes the LCD panel not lighting up when it was
not initialized by the GOP, because an external monitor was plugged in and
the GOP initialized only the external monitor.
Signed-off-by: Hans de Goede
---
Changes in v4:
-The decoding of the raw data of the PMIC MIPI sequence element is now done
in our caller, so drop this and adjust the callback prototype to accept
the decoded addresses, value and mask
Changes in v3:
-Use hex values for out of range checks
-Make intel_cht_wc_exec_mipi_pmic_seq_element return errors
Changes in v2:
-Interpret data passed to the PMIC MIPI elements according to the docs
instead of my own reverse engineered interpretation
---
drivers/acpi/pmic/intel_pmic_chtwc.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c
b/drivers/acpi/pmic/intel_pmic_chtwc.c
index 078b0448f30a..c5037c5c5219 100644
--- a/drivers/acpi/pmic/intel_pmic_chtwc.c
+++ b/drivers/acpi/pmic/intel_pmic_chtwc.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include "intel_pmic.h"
#define CHT_WC_V1P05A_CTRL 0x6e3b
@@ -231,6 +232,24 @@ static int intel_cht_wc_pmic_update_power(struct regmap
*regmap, int reg,
return regmap_update_bits(regmap, reg, bitmask, on ? 1 : 0);
}
+static int intel_cht_wc_exec_mipi_pmic_seq_element(struct regmap *regmap,
+ u16 i2c_client_address,
+ u32 reg_address,
+ u32 value, u32 mask)
+{
+ u32 address;
+
+ if (i2c_client_address > 0xff || reg_address > 0xff) {
+ pr_warn("%s warning addresses too big client 0x%x reg 0x%x\n",
+ __func__, i2c_client_address, reg_address);
+ return -ERANGE;
+ }
+
+ address = (i2c_client_address << 8) | reg_address;
+
+ return regmap_update_bits(regmap, address, mask, value);
+}
+
/*
* The thermal table and ops are empty, we do not support the Thermal opregion
* (DPTF) due to lacking documentation.
@@ -238,6 +257,7 @@ static int intel_cht_wc_pmic_update_power(struct regmap
*regmap, int reg,
static struct intel_pmic_opregion_data intel_cht_wc_pmic_opregion_data = {
.get_power = intel_cht_wc_pmic_get_power,
.update_power = intel_cht_wc_pmic_update_power,
+ .exec_mipi_pmic_seq_element = intel_cht_wc_exec_mipi_pmic_seq_element,
.power_table= power_table,
.power_table_count = ARRAY_SIZE(power_table),
};
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 3/3] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences
Add support for PMIC MIPI sequences using the new
intel_soc_pmic_exec_mipi_pmic_seq_element function.
This fixes the DSI LCD panel not lighting up when not initialized by the
GOP (because an external monitor was connected) on GPD win and GPD pocket
devices.
Specifically the LCD panel seems to need GPIO pin 9 on the PMIC to be
driven high, which is done through a PMIC MIPI sequence. Before this commit
if the sequence was not executed by the GOP the pin would stay low causing
the LCD panel to not work. Having the MIPI sequences properly control this
GPIO should also help save some power when the panel is off.
Changes in v2, v3:
-Only changes to other patches in this patch-set
Changes in v4:
-Move decoding of the raw 15 bytes PMIC MIPI sequence element into
i2c-address, register-address, value and mask into the mipi_exec_pmic()
function instead of passing the raw data to
intel_soc_pmic_exec_mipi_pmic_seq_element()
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/i915/intel_dsi_vbt.c | 22 +-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c
b/drivers/gpu/drm/i915/intel_dsi_vbt.c
index f27af47c6e49..ebe7e25614ce 100644
--- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
@@ -29,9 +29,11 @@
#include
#include
#include
+#include
#include
#include
#include
+#include
#include
#include "i915_drv.h"
#include "intel_drv.h"
@@ -371,7 +373,25 @@ static const u8 *mipi_exec_spi(struct intel_dsi
*intel_dsi, const u8 *data)
static const u8 *mipi_exec_pmic(struct intel_dsi *intel_dsi, const u8 *data)
{
- DRM_DEBUG_KMS("Skipping PMIC element execution\n");
+#ifdef CONFIG_PMIC_OPREGION
+ u32 value, mask, reg_address;
+ u16 i2c_address;
+ int ret;
+
+ /* byte 0 aka PMIC Flag is reserved */
+ i2c_address = get_unaligned_le16(data + 1);
+ reg_address = get_unaligned_le32(data + 3);
+ value = get_unaligned_le32(data + 7);
+ mask= get_unaligned_le32(data + 11);
+
+ ret = intel_soc_pmic_exec_mipi_pmic_seq_element(i2c_address,
+ reg_address,
+ value, mask);
+ if (ret)
+ DRM_ERROR("%s failed, error: %d\n", __func__, ret);
+#else
+ DRM_ERROR("Your hardware requires CONFIG_PMIC_OPREGION and it is not
set\n");
+#endif
return data + 15;
}
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
DSI LCD panels describe an initialization sequence in the Video BIOS
Tables using so called MIPI sequences. One possible element in these
sequences is a PMIC specific element of 15 bytes.
Although this is not really an ACPI opregion, the ACPI opregion code is the
closest thing we have. We need to have support for these PMIC specific MIPI
sequence elements somwhere. Since we already instantiate a special platform
device for Intel PMICs for the ACPI PMIC OpRegion handler to bind to,
with PMIC specific implementations of the OpRegion, the handling of MIPI
sequence PMIC elements fits very well in the ACPI PMIC OpRegion code.
This commit adds a new intel_soc_pmic_exec_mipi_pmic_seq_element()
function, which is to be backed by a PMIC specific
exec_mipi_pmic_seq_element callback. This function will be called by the
i915 code to execture MIPI sequence PMIC elements.
Signed-off-by: Hans de Goede
---
Changes in v4:
-Pass i2c-address + register-address + value + mask as separate arguments to
the intel_soc_pmic_exec_mipi_pmic_seq_element function. Instead of passing
the 15 bytes of raw MIPI sequence data. The decoding will be done in the
i915 VBT code instead.
Changes in v3:
-Add kerneldoc for intel_soc_pmic_exec_mipi_pmic_seq_element
-Make intel_soc_pmic_exec_mipi_pmic_seq_element return errors
---
drivers/acpi/pmic/intel_pmic.c | 49 ++
drivers/acpi/pmic/intel_pmic.h | 2 ++
include/linux/mfd/intel_soc_pmic.h | 3 ++
3 files changed, 54 insertions(+)
diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
index ca18e0d23df9..6bd25e96f41f 100644
--- a/drivers/acpi/pmic/intel_pmic.c
+++ b/drivers/acpi/pmic/intel_pmic.c
@@ -15,6 +15,7 @@
#include
#include
+#include
#include
#include
#include "intel_pmic.h"
@@ -36,6 +37,8 @@ struct intel_pmic_opregion {
struct intel_pmic_regs_handler_ctx ctx;
};
+static struct intel_pmic_opregion *intel_pmic_opregion;
+
static int pmic_get_reg_bit(int address, struct pmic_table *table,
int count, int *reg, int *bit)
{
@@ -304,6 +307,7 @@ int intel_pmic_install_opregion_handler(struct device *dev,
acpi_handle handle,
}
opregion->data = d;
+ intel_pmic_opregion = opregion;
return 0;
out_remove_thermal_handler:
@@ -319,3 +323,48 @@ int intel_pmic_install_opregion_handler(struct device
*dev, acpi_handle handle,
return ret;
}
EXPORT_SYMBOL_GPL(intel_pmic_install_opregion_handler);
+
+/**
+ * intel_soc_pmic_exec_mipi_pmic_seq_element - Execute PMIC MIPI sequence
+ * @i2c_address: I2C client address for the PMIC
+ * @reg_address: PMIC register address
+ * @value:New value for the register bits to change
+ * @mask: Mask indicating which register bits to change
+ *
+ * DSI LCD panels describe an initialization sequence in the i915 VBT (Video
+ * BIOS Tables) using so called MIPI sequences. One possible element in these
+ * sequences is a PMIC specific element of 15 bytes.
+ *
+ * This function executes these PMIC specific elements sending the embedded
+ * commands to the PMIC.
+ *
+ * Return 0 on success, < 0 on failure.
+ */
+int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
+ u32 value, u32 mask)
+{
+ struct intel_pmic_opregion_data *d;
+ int ret;
+
+ if (!intel_pmic_opregion) {
+ pr_warn("%s: No PMIC registered\n", __func__);
+ return -ENXIO;
+ }
+
+ d = intel_pmic_opregion->data;
+ if (!d->exec_mipi_pmic_seq_element) {
+ pr_warn("%s: Not implemented\n", __func__);
+ pr_warn("%s: i2c-addr: 0x%x reg-addr 0x%x value 0x%x mask
0x%x\n",
+ __func__, i2c_address, reg_address, value, mask);
+ return -EOPNOTSUPP;
+ }
+
+ mutex_lock(&intel_pmic_opregion->lock);
+ ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap,
+ i2c_address, reg_address,
+ value, mask);
+ mutex_unlock(&intel_pmic_opregion->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(intel_soc_pmic_exec_mipi_pmic_seq_element);
diff --git a/drivers/acpi/pmic/intel_pmic.h b/drivers/acpi/pmic/intel_pmic.h
index 095afc96952e..5cd195fabca8 100644
--- a/drivers/acpi/pmic/intel_pmic.h
+++ b/drivers/acpi/pmic/intel_pmic.h
@@ -15,6 +15,8 @@ struct intel_pmic_opregion_data {
int (*update_aux)(struct regmap *r, int reg, int raw_temp);
int (*get_policy)(struct regmap *r, int reg, int bit, u64 *value);
int (*update_policy)(struct regmap *r, int reg, int bit, int enable);
+ int (*exec_mipi_pmic_seq_element)(struct regmap *r, u16 i2c_address,
+ u32 reg_address, u32 value, u32 mask);
struct pmic_table *power_table;
int power_table_count;
struct pmic_table *thermal
[Bug 108992] Regression: Lenovo e585 (ryzen 2500u) freezes during boot with 4.20-rc5/rc6, amdgpu error
https://bugs.freedesktop.org/show_bug.cgi?id=108992 --- Comment #4 from Alex Deucher --- Can you boot the system without amdgpu loaded (e.g., append modprobe.blacklist=amdgpu)? Or is this a general platform problem? -- 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: [pull] amdgpu drm-next-4.21
On Wed, Dec 12, 2018 at 7:08 PM Dave Airlie wrote: > > On Thu, 13 Dec 2018 at 07:13, Alex Deucher wrote: > > > > Hi Dave, > > > > Updates for 4.21: > > - Powerplay updates for newer polaris variants > > - Add cursor plane update fast path > > - Enable gpu reset by default on CI parts > > - Fix config with KFD/HSA not enabled > > - Misc bug fixes > > > > Either this or the previous one broke the etnaviv build, I see two > reports on the list of this breakage from the kbuild robot but no > mention of it, can you guys either stick some arm cross compiles in > your build paths or keep an eye on the kbuild robot emails for your > branches. > > I created my own fix and pushed it in the merge commit. Sorry about that. I did see it, but I mixed it up with another issue on drm-misc which was already fixed and thought it was a result of the last drm-next merge into my tree. I'll pay closer attention next time. Alex > > Dave. > > > The following changes since commit 22666cc1481ae3814d9c7718418cc4a3aa7d90c3: > > > > drm/amdgpu: move IV prescreening into the GMC code (2018-12-07 18:14:26 > > -0500) > > > > are available in the git repository at: > > > > git://people.freedesktop.org/~agd5f/linux drm-next-4.21 > > > > for you to fetch changes up to 674e78acae0dfb4beb56132e41cbae5b60f7d662: > > > > drm/amd/display: Add fast path for cursor plane updates (2018-12-12 > > 15:32:10 -0500) > > > > > > Alex Deucher (1): > > drm/amdgpu/powerplay: Add special avfs cases for some polaris asics > > (v3) > > > > Andrey Grodzovsky (1): > > drm/amdgpu: Enable GPU recovery by default for CI > > > > Kuehling, Felix (1): > > drm/amdgpu: Fix stub function name > > > > Nicholas Kazlauskas (4): > > Revert "drm/amd/display: Set RMX_ASPECT as default" > > drm/amd/display: Fix unintialized max_bpc state values > > drm/amd/display: Fix duplicating scaling/underscan connector state > > drm/amd/display: Add fast path for cursor plane updates > > > > Rex Zhu (1): > > drm/amdgpu: Limit vm max ctx number to 4096 > > > > Tiecheng Zhou (1): > > drm/amdgpu: bypass RLC init under sriov for Tonga (v2) > > > > YueHaibing (1): > > drm/amdgpu: remove set but not used variable 'grbm_soft_reset' > > > > drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 + > > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +- > > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c| 2 +- > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 + > > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 11 +-- > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 79 > > -- > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 8 +++ > > .../drm/amd/powerplay/smumgr/polaris10_smumgr.c| 54 +++ > > 8 files changed, 147 insertions(+), 12 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201273] Fatal error during GPU init amdgpu RX560
https://bugzilla.kernel.org/show_bug.cgi?id=201273 --- Comment #20 from Alex Deucher (alexdeuc...@gmail.com) --- Can you try one of these branches? https://cgit.freedesktop.org/~agd5f/linux/log/?h=amd-staging-drm-next https://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-next-4.21-wip Do they help? -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 11/12] drm/panel: Add Feiyang FY07024DI26A30-D MIPI-DSI LCD panel
On Fri, Nov 16, 2018 at 10:09:15PM +0530, Jagan Teki wrote:
> Feiyang FY07024DI26A30-D is 1024x600, 4-lane MIPI-DSI LCD panel.
>
> Add panel driver for it.
>
> Signed-off-by: Jagan Teki
> ---
> MAINTAINERS | 6 +
> drivers/gpu/drm/panel/Kconfig | 9 +
> drivers/gpu/drm/panel/Makefile| 1 +
> .../drm/panel/panel-feiyang-fy07024di26a30d.c | 286 ++
> 4 files changed, 302 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3dac08d0b3cb..40c8bfc974f4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4620,6 +4620,12 @@ T: git git://anongit.freedesktop.org/drm/drm-misc
> S: Maintained
> F: drivers/gpu/drm/tve200/
>
> +DRM DRIVER FOR FEIYANG FY07024DI26A30-D MIPI-DSI LCD PANELS
> +M: Jagan Teki
> +S: Maintained
> +F: drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> +F:
> Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.txt
> +
> DRM DRIVER FOR ILITEK ILI9225 PANELS
> M: David Lechner
> S: Maintained
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index d0d4e60f5153..bc70896fe43c 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -47,6 +47,15 @@ config DRM_PANEL_SIMPLE
> that it can be automatically turned off when the panel goes into a
> low power state.
>
> +config DRM_PANEL_FEIYANG_FY07024DI26A30D
> + tristate "Feiyang FY07024DI26A30-D MIPI-DSI LCD panel"
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> + Say Y if you want to enable support for panels based on the
> + Feiyang FY07024DI26A30-D MIPI-DSI interface.
> +
> config DRM_PANEL_ILITEK_IL9322
> tristate "Ilitek ILI9322 320x240 QVGA panels"
> depends on OF && SPI
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 88011f06edb8..e23c017639c7 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o
> obj-$(CONFIG_DRM_PANEL_BANANAPI_S070WV20_ICN6211) +=
> panel-bananapi-s070wv20-icn6211.o
> obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
> obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
> +obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) +=
> panel-feiyang-fy07024di26a30d.o
> obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
> obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
> obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
> diff --git a/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> new file mode 100644
> index ..a4b46bd8fdbe
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> @@ -0,0 +1,286 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2018 Amarula Solutions
> + * Author: Jagan Teki
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +
> +struct feiyang {
> + struct drm_panelpanel;
> + struct mipi_dsi_device *dsi;
> +
> + struct backlight_device *backlight;
> + struct regulator*dvdd;
> + struct regulator*avdd;
> + struct gpio_desc*reset;
> +};
> +
> +static inline struct feiyang *panel_to_feiyang(struct drm_panel *panel)
> +{
> + return container_of(panel, struct feiyang, panel);
> +}
> +
> +struct feiyang_init_cmd {
> + size_t len;
> + const char *data;
> +};
> +
> +#define FY07024DI26A30D(...) { \
> + .len = sizeof((char[]){__VA_ARGS__}), \
> + .data = (char[]){__VA_ARGS__} }
> +
> +static const struct feiyang_init_cmd feiyang_init_cmds[] = {
> + FY07024DI26A30D(0x80, 0x58),
> + FY07024DI26A30D(0x81, 0x47),
> + FY07024DI26A30D(0x82, 0xD4),
> + FY07024DI26A30D(0x83, 0x88),
> + FY07024DI26A30D(0x84, 0xA9),
> + FY07024DI26A30D(0x85, 0xC3),
> + FY07024DI26A30D(0x86, 0x82),
> +};
These init cmds don't have to be so complicated. You've only got len == 2, so
just hardcode it in and avoid the macro:
#define FEIYANG_INIT_CMD_LEN2
struct feiyang_init_cmd {
u8 data[FEIYANG_INIT_CMD];
};
static const struct feiyang_init_cmd feiyang_init_cmds[] = {
{ .data = { 0x80, 0x58 } },
...
};
> +
> +static int feiyang_prepare(struct drm_panel *panel)
> +{
> + struct feiyang *ctx = panel_to_feiyang(panel);
> + struct mipi_dsi_device *dsi = ctx->dsi;
> + unsigned int i;
> + int ret;
> +
> + ret = regulator_enable(ctx->dvdd);
> + if (ret)
> + return ret;
> +
> + msleep(100);
nit: You should do your best to correlate the sleeps with the timing parameters
from the datasheet with a
Re: [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
Hi,
On 13-12-18 14:08, Ville Syrjälä wrote:
On Thu, Dec 13, 2018 at 01:40:27PM +0100, Hans de Goede wrote:
Hi,
On 13-12-18 13:14, Ville Syrjälä wrote:
+static int intel_cht_wc_exec_mipi_pmic_seq_element(struct regmap *regmap,
+ const u8 *data)
+{
+ u32 value, mask, reg_address, address;
+ u16 i2c_client_address;
+
+ /* byte 0 aka PMIC Flag is reserved */
+ i2c_client_address = get_unaligned_le16(data + 1);
+ reg_address = get_unaligned_le32(data + 3);
+ value = get_unaligned_le32(data + 7);
+ mask= get_unaligned_le32(data + 11);
Upon further reflection maybe it would better to do this decoding in
the i915 code and just pass each parameter to this hook separately?
That way we wouldn't be spreading the vbt details all over the place.
Interesting point, if the VBT spec says that this encoding is PMIC
independent, then yes we should probably fo the decoding in the VBT
code and change the intel_soc_pmic_exec_mipi_pmic_seq_element
prototype to:
int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
u32 value, u32 mask);
If you agree please let me know and I will do a v4 of the patchset.
Yeah, I think that's probably better. The spec has just the one
interpretation for the sequence.
Ok, v4. coming up.
Oh, and we should probably change the DRM_DEBUG_KMS() for the
PMIC_OPREGION=n case to a DRM_ERROR() which tells people to
enable PMIC_OPREGION=y.
Will do.
Regards,
Hans
p.s.
Have you also seen these 2 DSI related series (I've not see any feedback
on these 2 series yet) ? :
https://patchwork.kernel.org/patch/10707629/ (patch 1/4)
https://patchwork.kernel.org/patch/10707647/ (patch 1/4)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
HI,
On 13-12-18 14:45, Jani Nikula wrote:
On Thu, 13 Dec 2018, Hans de Goede wrote:
DSI LCD panels describe an initialization sequence in the Video BIOS
Tables using so called MIPI sequences. One possible element in these
sequences is a PMIC specific element of 15 bytes.
Although this is not really an ACPI opregion, the ACPI opregion code is the
closest thing we have. We need to have support for these PMIC specific MIPI
sequence elements somwhere. Since we already instantiate a special platform
device for Intel PMICs for the ACPI PMIC OpRegion handler to bind to,
with PMIC specific implementations of the OpRegion, the handling of MIPI
sequence PMIC elements fits very well in the ACPI PMIC OpRegion code.
This commit adds a new intel_soc_pmic_exec_mipi_pmic_seq_element()
function, which is to be backed by a PMIC specific
exec_mipi_pmic_seq_element callback. This function will be called by the
i915 code to execture MIPI sequence PMIC elements.
Signed-off-by: Hans de Goede
---
Changes in v3:
-Add kerneldoc for intel_soc_pmic_exec_mipi_pmic_seq_element
-Make intel_soc_pmic_exec_mipi_pmic_seq_element return errors
---
drivers/acpi/pmic/intel_pmic.c | 42 ++
drivers/acpi/pmic/intel_pmic.h | 1 +
include/linux/mfd/intel_soc_pmic.h | 2 ++
3 files changed, 45 insertions(+)
diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
index ca18e0d23df9..61f99735fd41 100644
--- a/drivers/acpi/pmic/intel_pmic.c
+++ b/drivers/acpi/pmic/intel_pmic.c
@@ -15,6 +15,7 @@
#include
#include
+#include
#include
#include
#include "intel_pmic.h"
@@ -36,6 +37,8 @@ struct intel_pmic_opregion {
struct intel_pmic_regs_handler_ctx ctx;
};
+static struct intel_pmic_opregion *intel_pmic_opregion;
+
static int pmic_get_reg_bit(int address, struct pmic_table *table,
int count, int *reg, int *bit)
{
@@ -304,6 +307,7 @@ int intel_pmic_install_opregion_handler(struct device *dev,
acpi_handle handle,
}
opregion->data = d;
+ intel_pmic_opregion = opregion;
return 0;
out_remove_thermal_handler:
@@ -319,3 +323,41 @@ int intel_pmic_install_opregion_handler(struct device
*dev, acpi_handle handle,
return ret;
}
EXPORT_SYMBOL_GPL(intel_pmic_install_opregion_handler);
+
+/**
+ * intel_soc_pmic_exec_mipi_pmic_seq_element - Execute PMIC MIPI sequence
+ * @data: Pointer to *15* byte PMIC MIPI sequence element
+ *
+ * DSI LCD panels describe an initialization sequence in the i915 VBT (Video
+ * BIOS Tables) using so called MIPI sequences. One possible element in these
+ * sequences is a PMIC specific element of 15 bytes.
+ *
+ * This function executes these PMIC specific elements sending the embedded
+ * commands to the PMIC.
+ *
+ * Return 0 on success, < 0 on failure.
+ */
+int intel_soc_pmic_exec_mipi_pmic_seq_element(const u8 *data)
+{
+ struct intel_pmic_opregion_data *d;
+ int ret;
+
+ if (!intel_pmic_opregion) {
+ pr_warn("%s: No PMIC registered\n", __func__);
+ return -ENXIO;
+ }
+
+ d = intel_pmic_opregion->data;
+ if (!d->exec_mipi_pmic_seq_element) {
+ pr_warn("%s: Not implemented\n", __func__);
+ pr_warn("%s: Data: %15ph\n", __func__, data);
+ return -EOPNOTSUPP;
+ }
+
+ mutex_lock(&intel_pmic_opregion->lock);
+ ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap, data);
+ mutex_unlock(&intel_pmic_opregion->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(intel_soc_pmic_exec_mipi_pmic_seq_element);
diff --git a/drivers/acpi/pmic/intel_pmic.h b/drivers/acpi/pmic/intel_pmic.h
index 095afc96952e..1cc5e69af0c3 100644
--- a/drivers/acpi/pmic/intel_pmic.h
+++ b/drivers/acpi/pmic/intel_pmic.h
@@ -15,6 +15,7 @@ struct intel_pmic_opregion_data {
int (*update_aux)(struct regmap *r, int reg, int raw_temp);
int (*get_policy)(struct regmap *r, int reg, int bit, u64 *value);
int (*update_policy)(struct regmap *r, int reg, int bit, int enable);
+ int (*exec_mipi_pmic_seq_element)(struct regmap *r, const u8 *data);
struct pmic_table *power_table;
int power_table_count;
struct pmic_table *thermal_table;
diff --git a/include/linux/mfd/intel_soc_pmic.h
b/include/linux/mfd/intel_soc_pmic.h
index ed1dfba5e5f9..71a8607a0a8c 100644
--- a/include/linux/mfd/intel_soc_pmic.h
+++ b/include/linux/mfd/intel_soc_pmic.h
@@ -26,4 +26,6 @@ struct intel_soc_pmic {
struct device *dev;
};
+int intel_soc_pmic_exec_mipi_pmic_seq_element(const u8 *data);
Add a static inline dummy implementation with error return for
CONFIG_PMIC_OPREGION=n? Maybe even with pr_err or pr_warn or something?
There is only one caller and Ville has requested for the caller to
do a DRM_ERROR("Your hardware requires CONFIG_PMIC_OPREGION and it is not set")
if we hit that code-path and CONFIG_PMIC_OPREGION is not se
[Bug 201585] 144Hz 2560x1440 no longer works (caps at 120Hz)
https://bugzilla.kernel.org/show_bug.cgi?id=201585 --- Comment #14 from Alexander Walker (a...@alexwalker.info) --- Also experiencing this on AMD Vega 56 on 4.20. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/virtio: switch to generic fbdev emulation
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 14 ---
drivers/gpu/drm/virtio/virtgpu_display.c | 1 -
drivers/gpu/drm/virtio/virtgpu_drv.c | 9 +-
drivers/gpu/drm/virtio/virtgpu_fb.c | 191 ---
drivers/gpu/drm/virtio/virtgpu_kms.c | 8 --
5 files changed, 8 insertions(+), 215 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 1deb41d42e..63704915f8 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -137,19 +137,10 @@ struct virtio_gpu_framebuffer {
#define to_virtio_gpu_framebuffer(x) \
container_of(x, struct virtio_gpu_framebuffer, base)
-struct virtio_gpu_fbdev {
- struct drm_fb_helper helper;
- struct virtio_gpu_framebuffer vgfb;
- struct virtio_gpu_device *vgdev;
- struct delayed_workwork;
-};
-
struct virtio_gpu_mman {
struct ttm_bo_devicebdev;
};
-struct virtio_gpu_fbdev;
-
struct virtio_gpu_queue {
struct virtqueue *vq;
spinlock_t qlock;
@@ -180,8 +171,6 @@ struct virtio_gpu_device {
struct virtio_gpu_mman mman;
- /* pointer to fbdev info structure */
- struct virtio_gpu_fbdev *vgfbdev;
struct virtio_gpu_output outputs[VIRTIO_GPU_MAX_SCANOUTS];
uint32_t num_scanouts;
@@ -249,9 +238,6 @@ int virtio_gpu_mode_dumb_mmap(struct drm_file *file_priv,
uint32_t handle, uint64_t *offset_p);
/* virtio_fb */
-#define VIRTIO_GPUFB_CONN_LIMIT 1
-int virtio_gpu_fbdev_init(struct virtio_gpu_device *vgdev);
-void virtio_gpu_fbdev_fini(struct virtio_gpu_device *vgdev);
int virtio_gpu_surface_dirty(struct virtio_gpu_framebuffer *qfb,
struct drm_clip_rect *clips,
unsigned int num_clips);
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c
b/drivers/gpu/drm/virtio/virtgpu_display.c
index b5580b11a0..e1c223e18d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -390,6 +390,5 @@ void virtio_gpu_modeset_fini(struct virtio_gpu_device
*vgdev)
for (i = 0 ; i < vgdev->num_scanouts; ++i)
kfree(vgdev->outputs[i].edid);
- virtio_gpu_fbdev_fini(vgdev);
drm_mode_config_cleanup(vgdev->ddev);
}
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c
b/drivers/gpu/drm/virtio/virtgpu_drv.c
index f7f32a885a..7df50920c1 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -42,13 +42,20 @@ module_param_named(modeset, virtio_gpu_modeset, int, 0400);
static int virtio_gpu_probe(struct virtio_device *vdev)
{
+ int ret;
+
if (vgacon_text_force() && virtio_gpu_modeset == -1)
return -EINVAL;
if (virtio_gpu_modeset == 0)
return -EINVAL;
- return drm_virtio_init(&driver, vdev);
+ ret = drm_virtio_init(&driver, vdev);
+ if (ret)
+ return ret;
+
+ drm_fbdev_generic_setup(vdev->priv, 32);
+ return 0;
}
static void virtio_gpu_remove(struct virtio_device *vdev)
diff --git a/drivers/gpu/drm/virtio/virtgpu_fb.c
b/drivers/gpu/drm/virtio/virtgpu_fb.c
index fb1cc8b2f1..b07584b1c2 100644
--- a/drivers/gpu/drm/virtio/virtgpu_fb.c
+++ b/drivers/gpu/drm/virtio/virtgpu_fb.c
@@ -27,8 +27,6 @@
#include
#include "virtgpu_drv.h"
-#define VIRTIO_GPU_FBCON_POLL_PERIOD (HZ / 60)
-
static int virtio_gpu_dirty_update(struct virtio_gpu_framebuffer *fb,
bool store, int x, int y,
int width, int height)
@@ -150,192 +148,3 @@ int virtio_gpu_surface_dirty(struct
virtio_gpu_framebuffer *vgfb,
left, top, right - left, bottom - top);
return 0;
}
-
-static void virtio_gpu_fb_dirty_work(struct work_struct *work)
-{
- struct delayed_work *delayed_work = to_delayed_work(work);
- struct virtio_gpu_fbdev *vfbdev =
- container_of(delayed_work, struct virtio_gpu_fbdev, work);
- struct virtio_gpu_framebuffer *vgfb = &vfbdev->vgfb;
-
- virtio_gpu_dirty_update(&vfbdev->vgfb, false, vgfb->x1, vgfb->y1,
- vgfb->x2 - vgfb->x1, vgfb->y2 - vgfb->y1);
-}
-
-static void virtio_gpu_3d_fillrect(struct fb_info *info,
- const struct fb_fillrect *rect)
-{
- struct virtio_gpu_fbdev *vfbdev = info->par;
-
- drm_fb_helper_sys_fillrect(info, rect);
- virtio_gpu_dirty_update(&vfbdev->vgfb, true, rect->dx, rect->dy,
-rect->width, rect->height);
- schedule_delayed_work(&vfbdev->work, VIRTIO_GPU_FBCON_POLL_PERIOD);
-}
-
-static void virtio_gpu_3d_copyarea(struct fb_info *info,
- const struct fb_copyarea *area)
-{
- struct virtio_gpu_fbdev *vf
Re: [PATCH v3 3/3] drm/i915/intel_dsi_vbt: Add support for PMIC MIPI sequences
On Thu, 13 Dec 2018, Hans de Goede wrote:
> Add support for PMIC MIPI sequences using the new
> intel_soc_pmic_exec_mipi_pmic_seq_element function.
>
> This fixes the DSI LCD panel not lighting up when not initialized by the
> GOP (because an external monitor was connected) on GPD win and GPD pocket
> devices.
>
> Specifically the LCD panel seems to need GPIO pin 9 on the PMIC to be
> driven high, which is done through a PMIC MIPI sequence. Before this commit
> if the sequence was not executed by the GOP the pin would stay low causing
> the LCD panel to not work. Having the MIPI sequences properly control this
> GPIO should also help save some power when the panel is off.
>
> Signed-off-by: Hans de Goede
> ---
> drivers/gpu/drm/i915/intel_dsi_vbt.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> index f27af47c6e49..6a2ed1ca72e0 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -371,7 +372,11 @@ static const u8 *mipi_exec_spi(struct intel_dsi
> *intel_dsi, const u8 *data)
>
> static const u8 *mipi_exec_pmic(struct intel_dsi *intel_dsi, const u8 *data)
> {
> +#ifdef CONFIG_PMIC_OPREGION
> + intel_soc_pmic_exec_mipi_pmic_seq_element(data);
> +#else
> DRM_DEBUG_KMS("Skipping PMIC element execution\n");
> +#endif
Please check the return value, and log with DRM_DEBUG_KMS or
DRM_ERROR. With the dummy implementation of
intel_soc_pmic_exec_mipi_pmic_seq_element() returning e.g. -ENODEV this
would also cover the CONFIG_PMIC_OPREGION=n case.
BR,
Jani.
>
> return data + 15;
> }
--
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 v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
On Thu, 13 Dec 2018, Ville Syrjälä wrote:
> On Thu, Dec 13, 2018 at 01:40:27PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 13-12-18 13:14, Ville Syrjälä wrote:
>> > On Thu, Dec 13, 2018 at 12:21:35PM +0100, Hans de Goede wrote:
>> >> Implement the exec_mipi_pmic_seq_element callback for the CHT Whiskey Cove
>> >> PMIC.
>> >>
>> >> On some CHT devices this fixes the LCD panel not lighting up when it was
>> >> not initialized by the GOP, because an external monitor was plugged in and
>> >> the GOP initialized only the external monitor.
>> >>
>> >> Signed-off-by: Hans de Goede
>> >> ---
>> >> Changes in v2:
>> >> -Interpret data passed to the PMIC MIPI elements according to the docs
>> >> instead of my own reverse engineered interpretation
>> >> Changes in v3:
>> >> -Use hex values for out of range checks
>> >> -Make intel_cht_wc_exec_mipi_pmic_seq_element return errors
>> >> ---
>> >> drivers/acpi/pmic/intel_pmic_chtwc.c | 25 +
>> >> 1 file changed, 25 insertions(+)
>> >>
>> >> diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c
>> >> b/drivers/acpi/pmic/intel_pmic_chtwc.c
>> >> index 078b0448f30a..8ede74e9b89f 100644
>> >> --- a/drivers/acpi/pmic/intel_pmic_chtwc.c
>> >> +++ b/drivers/acpi/pmic/intel_pmic_chtwc.c
>> >> @@ -12,6 +12,7 @@
>> >> #include
>> >> #include
>> >> #include
>> >> +#include
>> >> #include "intel_pmic.h"
>> >>
>> >> #define CHT_WC_V1P05A_CTRL 0x6e3b
>> >> @@ -231,6 +232,29 @@ static int intel_cht_wc_pmic_update_power(struct
>> >> regmap *regmap, int reg,
>> >> return regmap_update_bits(regmap, reg, bitmask, on ? 1 : 0);
>> >> }
>> >>
>> >> +static int intel_cht_wc_exec_mipi_pmic_seq_element(struct regmap *regmap,
>> >> +const u8 *data)
>> >> +{
>> >> + u32 value, mask, reg_address, address;
>> >> + u16 i2c_client_address;
>> >> +
>> >> + /* byte 0 aka PMIC Flag is reserved */
>> >> + i2c_client_address = get_unaligned_le16(data + 1);
>> >> + reg_address = get_unaligned_le32(data + 3);
>> >> + value = get_unaligned_le32(data + 7);
>> >> + mask= get_unaligned_le32(data + 11);
>> >
>> > Upon further reflection maybe it would better to do this decoding in
>> > the i915 code and just pass each parameter to this hook separately?
>> > That way we wouldn't be spreading the vbt details all over the place.
>>
>> Interesting point, if the VBT spec says that this encoding is PMIC
>> independent, then yes we should probably fo the decoding in the VBT
>> code and change the intel_soc_pmic_exec_mipi_pmic_seq_element
>> prototype to:
>>
>> int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32
>> reg_address,
>>u32 value, u32 mask);
>>
>> If you agree please let me know and I will do a v4 of the patchset.
>
> Yeah, I think that's probably better. The spec has just the one
> interpretation for the sequence.
Agreed.
BR,
Jani.
>
>>
>> I've also been thinking about trying to make the implementation
>> under drivers/acpi/pmic pmic independent, but not all pmic
>> drivers use the regmap the same way. The CHT Whiskey Cove PMIC
>> mfd driver uses a regmap with 16 bit addresses where the upper
>> byte is the i2c client address and the lower byte is the register
>> address (this PMIC listens on multiple addresses, with different
>> registers behind each i2c address).
>>
>> Where as most PMIC mfd drivers simply use the standard
>> devm_regmap_init_i2c() method of creating a regmap. For these
>> others we could do a standard implementation where we check the
>> passed in i2c_address is what we expect (for that type PMIC) and
>> then pass the other 3 parameters to regmap_update_bits.
>>
>> But I think it would be best to wait with such a generic implementation
>> until we encounter a device using the PMIC MIPI sequence element
>> with another type of PMIC. Since we still need the special
>> implementation for the CHT WC case, we still need an operation
>> pointer for this in intel_pmic_opregion_data anyways, so we can
>> easily plug in the generic implementation for others later.
>
> Yeah, probably not worth worrying about this until we
> encounter a machine that needs it.
>
> Oh, and we should probably change the DRM_DEBUG_KMS() for the
> PMIC_OPREGION=n case to a DRM_ERROR() which tells people to
> enable PMIC_OPREGION=y. Not sure why all these random knobs are
> even user configurable. No one can really be expected to know
> how to configure them properly. There was a recent problem with
> someone having set I2C_DESIGNWARE_BAYTRAIL=n as well because
> they had a CHT/BSW instead of a BYT :(
--
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 v3 1/3] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements
On Thu, 13 Dec 2018, Hans de Goede wrote:
> DSI LCD panels describe an initialization sequence in the Video BIOS
> Tables using so called MIPI sequences. One possible element in these
> sequences is a PMIC specific element of 15 bytes.
>
> Although this is not really an ACPI opregion, the ACPI opregion code is the
> closest thing we have. We need to have support for these PMIC specific MIPI
> sequence elements somwhere. Since we already instantiate a special platform
> device for Intel PMICs for the ACPI PMIC OpRegion handler to bind to,
> with PMIC specific implementations of the OpRegion, the handling of MIPI
> sequence PMIC elements fits very well in the ACPI PMIC OpRegion code.
>
> This commit adds a new intel_soc_pmic_exec_mipi_pmic_seq_element()
> function, which is to be backed by a PMIC specific
> exec_mipi_pmic_seq_element callback. This function will be called by the
> i915 code to execture MIPI sequence PMIC elements.
>
> Signed-off-by: Hans de Goede
> ---
> Changes in v3:
> -Add kerneldoc for intel_soc_pmic_exec_mipi_pmic_seq_element
> -Make intel_soc_pmic_exec_mipi_pmic_seq_element return errors
> ---
> drivers/acpi/pmic/intel_pmic.c | 42 ++
> drivers/acpi/pmic/intel_pmic.h | 1 +
> include/linux/mfd/intel_soc_pmic.h | 2 ++
> 3 files changed, 45 insertions(+)
>
> diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
> index ca18e0d23df9..61f99735fd41 100644
> --- a/drivers/acpi/pmic/intel_pmic.c
> +++ b/drivers/acpi/pmic/intel_pmic.c
> @@ -15,6 +15,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include "intel_pmic.h"
> @@ -36,6 +37,8 @@ struct intel_pmic_opregion {
> struct intel_pmic_regs_handler_ctx ctx;
> };
>
> +static struct intel_pmic_opregion *intel_pmic_opregion;
> +
> static int pmic_get_reg_bit(int address, struct pmic_table *table,
> int count, int *reg, int *bit)
> {
> @@ -304,6 +307,7 @@ int intel_pmic_install_opregion_handler(struct device
> *dev, acpi_handle handle,
> }
>
> opregion->data = d;
> + intel_pmic_opregion = opregion;
> return 0;
>
> out_remove_thermal_handler:
> @@ -319,3 +323,41 @@ int intel_pmic_install_opregion_handler(struct device
> *dev, acpi_handle handle,
> return ret;
> }
> EXPORT_SYMBOL_GPL(intel_pmic_install_opregion_handler);
> +
> +/**
> + * intel_soc_pmic_exec_mipi_pmic_seq_element - Execute PMIC MIPI sequence
> + * @data: Pointer to *15* byte PMIC MIPI sequence element
> + *
> + * DSI LCD panels describe an initialization sequence in the i915 VBT (Video
> + * BIOS Tables) using so called MIPI sequences. One possible element in these
> + * sequences is a PMIC specific element of 15 bytes.
> + *
> + * This function executes these PMIC specific elements sending the embedded
> + * commands to the PMIC.
> + *
> + * Return 0 on success, < 0 on failure.
> + */
> +int intel_soc_pmic_exec_mipi_pmic_seq_element(const u8 *data)
> +{
> + struct intel_pmic_opregion_data *d;
> + int ret;
> +
> + if (!intel_pmic_opregion) {
> + pr_warn("%s: No PMIC registered\n", __func__);
> + return -ENXIO;
> + }
> +
> + d = intel_pmic_opregion->data;
> + if (!d->exec_mipi_pmic_seq_element) {
> + pr_warn("%s: Not implemented\n", __func__);
> + pr_warn("%s: Data: %15ph\n", __func__, data);
> + return -EOPNOTSUPP;
> + }
> +
> + mutex_lock(&intel_pmic_opregion->lock);
> + ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap, data);
> + mutex_unlock(&intel_pmic_opregion->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(intel_soc_pmic_exec_mipi_pmic_seq_element);
> diff --git a/drivers/acpi/pmic/intel_pmic.h b/drivers/acpi/pmic/intel_pmic.h
> index 095afc96952e..1cc5e69af0c3 100644
> --- a/drivers/acpi/pmic/intel_pmic.h
> +++ b/drivers/acpi/pmic/intel_pmic.h
> @@ -15,6 +15,7 @@ struct intel_pmic_opregion_data {
> int (*update_aux)(struct regmap *r, int reg, int raw_temp);
> int (*get_policy)(struct regmap *r, int reg, int bit, u64 *value);
> int (*update_policy)(struct regmap *r, int reg, int bit, int enable);
> + int (*exec_mipi_pmic_seq_element)(struct regmap *r, const u8 *data);
> struct pmic_table *power_table;
> int power_table_count;
> struct pmic_table *thermal_table;
> diff --git a/include/linux/mfd/intel_soc_pmic.h
> b/include/linux/mfd/intel_soc_pmic.h
> index ed1dfba5e5f9..71a8607a0a8c 100644
> --- a/include/linux/mfd/intel_soc_pmic.h
> +++ b/include/linux/mfd/intel_soc_pmic.h
> @@ -26,4 +26,6 @@ struct intel_soc_pmic {
> struct device *dev;
> };
>
> +int intel_soc_pmic_exec_mipi_pmic_seq_element(const u8 *data);
Add a static inline dummy implementation with error return for
CONFIG_PMIC_OPREGION=n? Maybe even with pr_err or pr_warn or something?
BR,
Jani.
> +
> #endif /* __INTEL_SOC_PMIC_H__ */
--
Jani Nikula, Intel Open Source Gr
[Bug 109049] [CI][BAT] igt@amdgpu/amd_prime@amd-to-i915 - fail - Failed assertion: r == 0, Last errno: 28, No space left on device
https://bugs.freedesktop.org/show_bug.cgi?id=109049 --- Comment #5 from Chris Wilson --- Regression from commit 08d1bdd4cc57e89d037205687a61d2b6ff94 Author: Rex Zhu Date: Wed Oct 24 16:10:33 2018 +0800 drm/amdgpu: Limit vm max ctx number to 4096 driver need to reserve resource for each ctx for some hw features. so add this limitation. Reviewed-by: Christian König Signed-off-by: Rex Zhu Signed-off-by: Alex Deucher -- 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 109049] [CI][BAT] igt@amdgpu/amd_prime@amd-to-i915 - fail - Failed assertion: r == 0, Last errno: 28, No space left on device
https://bugs.freedesktop.org/show_bug.cgi?id=109049 Chris Wilson changed: What|Removed |Added Summary|[CI][BAT] |[CI][BAT] |igt@kms_atomic_transition@p |igt@amdgpu/amd_prime@amd-to |lane-all-modeset-transition |-i915 - fail - Failed |/ |assertion: r == 0, Last |igt@amdgpu/amd_prime@amd-to |errno: 28, No space left on |-i915 - fail - Failed |device |assertion: r == 0, Last | |errno: 28, No space left on | |device | -- 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 109049] [CI][BAT] igt@kms_atomic_transition@plane-all-modeset-transition / igt@amdgpu/amd_prime@amd-to-i915 - fail - Failed assertion: r == 0, Last errno: 28, No space left on device
https://bugs.freedesktop.org/show_bug.cgi?id=109049 --- Comment #4 from Chris Wilson --- Now limited to 4095 contexts. Isn't that a coincidence! -- 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 109049] [CI][BAT] igt@kms_atomic_transition@plane-all-modeset-transition / igt@amdgpu/amd_prime@amd-to-i915 - fail - Failed assertion: r == 0, Last errno: 28, No space left on device
https://bugs.freedesktop.org/show_bug.cgi?id=109049 Chris Wilson changed: What|Removed |Added QA Contact|intel-gfx-bugs@lists.freede | |sktop.org | Assignee|intel-gfx-bugs@lists.freede |dri-devel@lists.freedesktop |sktop.org |.org Component|DRM/Intel |DRM/AMDgpu --- Comment #3 from Chris Wilson --- (In reply to Martin Peres from comment #2) > Moving the bug back to i915 because of the following issue: > > https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_168/fi-skl-6700k2/ > igt@kms_atomic_transit...@plane-all-modeset-transition.html > > Starting subtest: plane-all-modeset-transition > (kms_atomic_transition:1188) igt_kms-CRITICAL: Test assertion failure > function do_display_commit, file ../lib/igt_kms.c:3320: > (kms_atomic_transition:1188) igt_kms-CRITICAL: Failed assertion: ret == 0 > (kms_atomic_transition:1188) igt_kms-CRITICAL: Last errno: 28, No space left > on device > (kms_atomic_transition:1188) igt_kms-CRITICAL: error: -28 != 0 > Subtest plane-all-modeset-transition failed. Nothing to do with the amdgpu ENOSPC. -- 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
[git pull] vmwgfx-next-2018-12-13
Hi, Dave Two minor fixes for the previous vmwgfx-next pull: The following changes since commit 9a01135b98b9d5a7033c544245da7aad0d886758: drm/vmwgfx: Use the standard atomic helpers for page-flip (2018-12-05 10:09:55 +0100) are available in the Git repository at: git://people.freedesktop.org/~thomash/linux tags/vmwgfx-next-2018-12-13 for you to fetch changes up to 0660d8cd83938ef5cc91582fe6f951e8935f2d0f: drm: Fix docs warning in drm_damage_helper.c (2018-12-11 15:20:13 +0100) Pull request of 2018-12-13 Colin Ian King (1): drm/selftest: fix spelling mistake "dimention" -> "dimension" Deepak Rawat (1): drm: Fix docs warning in drm_damage_helper.c drivers/gpu/drm/drm_damage_helper.c| 2 +- drivers/gpu/drm/selftests/test-drm_damage_helper.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC
On Thu, Dec 13, 2018 at 01:40:27PM +0100, Hans de Goede wrote:
> Hi,
>
> On 13-12-18 13:14, Ville Syrjälä wrote:
> > On Thu, Dec 13, 2018 at 12:21:35PM +0100, Hans de Goede wrote:
> >> Implement the exec_mipi_pmic_seq_element callback for the CHT Whiskey Cove
> >> PMIC.
> >>
> >> On some CHT devices this fixes the LCD panel not lighting up when it was
> >> not initialized by the GOP, because an external monitor was plugged in and
> >> the GOP initialized only the external monitor.
> >>
> >> Signed-off-by: Hans de Goede
> >> ---
> >> Changes in v2:
> >> -Interpret data passed to the PMIC MIPI elements according to the docs
> >> instead of my own reverse engineered interpretation
> >> Changes in v3:
> >> -Use hex values for out of range checks
> >> -Make intel_cht_wc_exec_mipi_pmic_seq_element return errors
> >> ---
> >> drivers/acpi/pmic/intel_pmic_chtwc.c | 25 +
> >> 1 file changed, 25 insertions(+)
> >>
> >> diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c
> >> b/drivers/acpi/pmic/intel_pmic_chtwc.c
> >> index 078b0448f30a..8ede74e9b89f 100644
> >> --- a/drivers/acpi/pmic/intel_pmic_chtwc.c
> >> +++ b/drivers/acpi/pmic/intel_pmic_chtwc.c
> >> @@ -12,6 +12,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >> #include "intel_pmic.h"
> >>
> >> #define CHT_WC_V1P05A_CTRL 0x6e3b
> >> @@ -231,6 +232,29 @@ static int intel_cht_wc_pmic_update_power(struct
> >> regmap *regmap, int reg,
> >>return regmap_update_bits(regmap, reg, bitmask, on ? 1 : 0);
> >> }
> >>
> >> +static int intel_cht_wc_exec_mipi_pmic_seq_element(struct regmap *regmap,
> >> + const u8 *data)
> >> +{
> >> + u32 value, mask, reg_address, address;
> >> + u16 i2c_client_address;
> >> +
> >> + /* byte 0 aka PMIC Flag is reserved */
> >> + i2c_client_address = get_unaligned_le16(data + 1);
> >> + reg_address = get_unaligned_le32(data + 3);
> >> + value = get_unaligned_le32(data + 7);
> >> + mask= get_unaligned_le32(data + 11);
> >
> > Upon further reflection maybe it would better to do this decoding in
> > the i915 code and just pass each parameter to this hook separately?
> > That way we wouldn't be spreading the vbt details all over the place.
>
> Interesting point, if the VBT spec says that this encoding is PMIC
> independent, then yes we should probably fo the decoding in the VBT
> code and change the intel_soc_pmic_exec_mipi_pmic_seq_element
> prototype to:
>
> int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32
> reg_address,
> u32 value, u32 mask);
>
> If you agree please let me know and I will do a v4 of the patchset.
Yeah, I think that's probably better. The spec has just the one
interpretation for the sequence.
>
> I've also been thinking about trying to make the implementation
> under drivers/acpi/pmic pmic independent, but not all pmic
> drivers use the regmap the same way. The CHT Whiskey Cove PMIC
> mfd driver uses a regmap with 16 bit addresses where the upper
> byte is the i2c client address and the lower byte is the register
> address (this PMIC listens on multiple addresses, with different
> registers behind each i2c address).
>
> Where as most PMIC mfd drivers simply use the standard
> devm_regmap_init_i2c() method of creating a regmap. For these
> others we could do a standard implementation where we check the
> passed in i2c_address is what we expect (for that type PMIC) and
> then pass the other 3 parameters to regmap_update_bits.
>
> But I think it would be best to wait with such a generic implementation
> until we encounter a device using the PMIC MIPI sequence element
> with another type of PMIC. Since we still need the special
> implementation for the CHT WC case, we still need an operation
> pointer for this in intel_pmic_opregion_data anyways, so we can
> easily plug in the generic implementation for others later.
Yeah, probably not worth worrying about this until we
encounter a machine that needs it.
Oh, and we should probably change the DRM_DEBUG_KMS() for the
PMIC_OPREGION=n case to a DRM_ERROR() which tells people to
enable PMIC_OPREGION=y. Not sure why all these random knobs are
even user configurable. No one can really be expected to know
how to configure them properly. There was a recent problem with
someone having set I2C_DESIGNWARE_BAYTRAIL=n as well because
they had a CHT/BSW instead of a BYT :(
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
Hi Steve,
On Tue, 2018-10-16 at 17:00 -0700, Steve Longerbeam wrote:
> The function ipu_csi_init_interface() was inverting the F-bit for
> NTSC case, in the CCIR_CODE_1/2 registers. The result being that
> for NTSC bottom-top field order, the CSI would swap fields and
> capture in top-bottom order.
>
> Instead, base field swap on the field order of the input to the CSI,
> and the field order of the requested output. If the input/output
> fields are sequential but different, swap fields, otherwise do
> not swap. This requires passing both the input and output mbus
> frame formats to ipu_csi_init_interface().
>
> Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
> that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
> possibly interlaced BT.1120 in the future).
>
> When detecting input video standard from the input frame width/height,
> make sure to double height if input field type is alternate, since
> in that case input height only includes lines for one field.
>
> Signed-off-by: Steve Longerbeam
> ---
> Changes since v4:
> - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
> by Philipp Zabel.
> - Fixed a regression in csi_setup(), caught by Philipp.
> ---
> drivers/gpu/ipu-v3/ipu-csi.c | 119 +++---
> drivers/staging/media/imx/imx-media-csi.c | 17 +---
> include/video/imx-ipu-v3.h| 3 +-
> 3 files changed, 88 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> index aa0e30a2ba18..4a15e513fa05 100644
> --- a/drivers/gpu/ipu-v3/ipu-csi.c
> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> @@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct
> ipu_csi_bus_config *cfg, u32 mbus_code,
> return 0;
> }
>
> +/* translate alternate field mode based on given standard */
> +static inline enum v4l2_field
> +ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
> +{
> + return (field != V4L2_FIELD_ALTERNATE) ? field :
> + ((std & V4L2_STD_525_60) ?
> + V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
> +}
> +
> /*
> * Fill a CSI bus config struct from mbus_config and mbus_framefmt.
> */
> @@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config
> *csicfg,
> return 0;
> }
>
> +static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
> +struct v4l2_mbus_framefmt *infmt,
> +struct v4l2_mbus_framefmt *outfmt,
infmt and outfmt parameters could be const.
> +v4l2_std_id std)
> +{
> + enum v4l2_field infield, outfield;
> + bool swap_fields;
> +
> + /* get translated field type of input and output */
> + infield = ipu_csi_translate_field(infmt->field, std);
> + outfield = ipu_csi_translate_field(outfmt->field, std);
> +
> + /*
> + * Write the H-V-F codes the CSI will match against the
> + * incoming data for start/end of active and blanking
> + * field intervals. If input and output field types are
> + * sequential but not the same (one is SEQ_BT and the other
> + * is SEQ_TB), swap the F-bit so that the CSI will capture
> + * field 1 lines before field 0 lines.
> + */
> + swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
> +V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
> +infield != outfield);
> +
> + if (!swap_fields) {
> + /*
> + * Field0BlankEnd = 110, Field0BlankStart = 010
> + * Field0ActiveEnd = 100, Field0ActiveStart = 000
> + * Field1BlankEnd = 111, Field1BlankStart = 011
> + * Field1ActiveEnd = 101, Field1ActiveStart = 001
> + */
> + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> + } else {
> + dev_dbg(csi->ipu->dev, "capture field swap\n");
> +
> + /* same as above but with F-bit inverted */
> + ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> + }
> +
> + ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3);
> +
> + return 0;
> +}
> +
> +
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> -struct v4l2_mbus_framefmt *mbus_fmt)
> +struct v4l2_mbus_framefmt *infmt,
> +struct v4l2_mbus_framefmt *outfmt)
> {
> struct ipu_csi_bus_config cfg;
> unsigned long flags;
> u32 width, height, data = 0;
> + v4l2_std_id std;
> int ret;
>
> - ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
> + ret = fill_csi_bus_cfg(&cfg, mbus_cfg
