[Bug 199357] amdgpu: hang a few seconds after logging in, most likely due to regression
https://bugzilla.kernel.org/show_bug.cgi?id=199357 --- Comment #10 from Mathias Tillman (master.ho...@gmail.com) --- Since that commit was pushed to v4.16, shouldn't it also be reverted on linux-stable to make it to a future 4.16.y release? -- 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 102638] Incorrect rendering in OpenGL 4 (Unigine Superposition + others)
https://bugs.freedesktop.org/show_bug.cgi?id=102638 --- Comment #5 from mj.wilson...@gmail.com --- Please consider this resolved. At some point as mesa packages were upgraded, this issue went away. I'm now on 17.3.6, and cannot reproduce the issue. Thanks. -- 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 v10 10/11] drm: Add aspect ratio parsing in DRM layer
On Tue, Apr 17, 2018 at 10:45:07AM +0530, Nautiyal, Ankit K wrote:
>
> On 4/6/2018 11:14 PM, Ville Syrjälä wrote:
> > On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:
> > > This patch is causing failure of IGT test kms_3d. The kms_3d test
> > > expects the no. of 3d modes to be 13.
> > >
> > > (The test has hard-coded value for expected no. of 3d modes as 13)
> > >
> > > But due to the addition of "matching aspect_ratio" in drm_mode_equal in
> > > this patch, the total no. of
> > >
> > > modes in the connector modelist is increased by 2, resulting in failure
> > > of assertion 'mode_count==13'.
> > If kms_3d isn't setting the aspect ratio cap how is it affected by these
> > changes?
> In drm_mode.c, the drm_mode_connector_list_update() uses drm_mode_equal,
> to remove duplicate modes in connector_modes from the
> connector->probe_modes.
> Earlier, it wasn't matching for aspect-ratio and was discarding two of the
> modes with aspect ratio,
> as duplicates of other modes in the list.
>
> Later, when we are pruning the modes in drm_mode_get_connector, the logic
> there assumes,
> that the modes are in a sorted order so that we just match with the last
> valid mode for uniqueness.
> This isn't the case with the spoofed edid in kms_3d.
> Earlier, I was thinking if we should change the no. of expected modes in
> kms_3d,
> but that's not correct approach.
>
> So finally, The pruning logic needs to be changed, to do away with any
> assumption and check
> all the modes in the list for duplicates. This however will take more time
> to remove duplicates.
Agreed that the best option looks like just fixing the pruning logic.
Since we're talking about very few modes, and in the probe path (where
reading edid takes 20ms easily) the additional cpu overhead really doesn't
matter I think.
-Daniel
> Any other suggestions on this?
>
> Regards
> -Ankit
>
> >
> > > Perhaps this need to be handled in the test.
> > >
> > > -Regards,
> > >
> > > Ankit
> > >
> > >
> > > On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:
> > > > From: "Sharma, Shashank"
> > > >
> > > > Current DRM layer functions don't parse aspect ratio information
> > > > while converting a user mode->kernel mode or vice versa. This
> > > > causes modeset to pick mode with wrong aspect ratio, eventually
> > > > causing failures in HDMI compliance test cases, due to wrong VIC.
> > > >
> > > > This patch adds aspect ratio information in DRM's mode conversion
> > > > and mode comparision functions, to make sure kernel picks mode
> > > > with right aspect ratio (as per the VIC).
> > > >
> > > > Background:
> > > > This patch was once reviewed and merged, and later reverted due to
> > > > lack of DRM cap protection. This is a re-spin of this patch, this
> > > > time with DRM cap protection, to avoid aspect ratio information, when
> > > > the client doesn't request for it.
> > > >
> > > > Review link: https://pw-emeril.freedesktop.org/patch/104068/
> > > > Background discussion: https://patchwork.kernel.org/patch/9379057/
> > > >
> > > > Signed-off-by: Shashank Sharma
> > > > Signed-off-by: Lin, Jia
> > > > Signed-off-by: Akashdeep Sharma
> > > > Reviewed-by: Jim Bride (V2)
> > > > Reviewed-by: Jose Abreu (V4)
> > > >
> > > > Cc: Ville Syrjala
> > > > Cc: Jim Bride
> > > > Cc: Jose Abreu
> > > > Cc: Ankit Nautiyal
> > > >
> > > > V3: modified the aspect-ratio check in drm_mode_equal as per new flags
> > > > provided by Ville. https://patchwork.freedesktop.org/patch/188043/
> > > > V4: rebase
> > > > V5: rebase
> > > > V6: As recommended by Ville, avoided matching of aspect-ratio in
> > > > drm_fb_helper, while trying to find a common mode among connectors
> > > > for the target clone mode.
> > > > V7: rebase
> > > > V8: rebase
> > > > V9: rebase
> > > > V10: rebase
> > > > ---
> > > >drivers/gpu/drm/drm_fb_helper.c | 12 ++--
> > > >drivers/gpu/drm/drm_modes.c | 35
> > > > ++-
> > > >2 files changed, 44 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > > b/drivers/gpu/drm/drm_fb_helper.c
> > > > index 0646b10..2ee1eaa 100644
> > > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > > @@ -2183,7 +2183,11 @@ static bool drm_target_cloned(struct
> > > > drm_fb_helper *fb_helper,
> > > > for (j = 0; j < i; j++) {
> > > > if (!enabled[j])
> > > > continue;
> > > > - if (!drm_mode_equal(modes[j], modes[i]))
> > > > + if (!drm_mode_match(modes[j], modes[i],
> > > > + DRM_MODE_MATCH_TIMINGS |
> > > > + DRM_MODE_MATCH_CLOCK |
> > > > + DRM_MODE_MATCH_FLAGS |
> > > > + DRM_MODE_MATCH_3D_FLAGS))
[PATCH] drm/xen-front: Remove CMA support
From: Oleksandr Andrushchenko
Even if xen-front allocates its buffers from contiguous memory
those are still not contiguous in PA space, e.g. the buffer is only
contiguous in IPA space.
The only use-case for this mode was if xen-front is used to allocate
dumb buffers which later be used by some other driver requiring
contiguous memory, but there is no currently such a use-case or
it can be worked around with xen-front.
Signed-off-by: Oleksandr Andrushchenko
Suggested-by: Daniel Vetter
---
Documentation/gpu/xen-front.rst | 12
drivers/gpu/drm/xen/Kconfig | 13
drivers/gpu/drm/xen/Makefile| 9 +--
drivers/gpu/drm/xen/xen_drm_front.c | 62 +++-
drivers/gpu/drm/xen/xen_drm_front.h | 42 ++-
drivers/gpu/drm/xen/xen_drm_front_gem.c | 12 +---
drivers/gpu/drm/xen/xen_drm_front_gem.h | 3 -
drivers/gpu/drm/xen/xen_drm_front_gem_cma.c | 79 -
drivers/gpu/drm/xen/xen_drm_front_shbuf.c | 22 --
drivers/gpu/drm/xen/xen_drm_front_shbuf.h | 8 ---
10 files changed, 21 insertions(+), 241 deletions(-)
delete mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem_cma.c
diff --git a/Documentation/gpu/xen-front.rst b/Documentation/gpu/xen-front.rst
index 009d942386c5..d988da7d1983 100644
--- a/Documentation/gpu/xen-front.rst
+++ b/Documentation/gpu/xen-front.rst
@@ -18,18 +18,6 @@ Buffers allocated by the frontend driver
.. kernel-doc:: drivers/gpu/drm/xen/xen_drm_front.h
:doc: Buffers allocated by the frontend driver
-With GEM CMA helpers
-
-
-.. kernel-doc:: drivers/gpu/drm/xen/xen_drm_front.h
- :doc: With GEM CMA helpers
-
-Without GEM CMA helpers
-~~~
-
-.. kernel-doc:: drivers/gpu/drm/xen/xen_drm_front.h
- :doc: Without GEM CMA helpers
-
Buffers allocated by the backend
diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig
index 4f4abc91f3b6..4cca160782ab 100644
--- a/drivers/gpu/drm/xen/Kconfig
+++ b/drivers/gpu/drm/xen/Kconfig
@@ -15,16 +15,3 @@ config DRM_XEN_FRONTEND
help
Choose this option if you want to enable a para-virtualized
frontend DRM/KMS driver for Xen guest OSes.
-
-config DRM_XEN_FRONTEND_CMA
- bool "Use DRM CMA to allocate dumb buffers"
- depends on DRM_XEN_FRONTEND
- select DRM_KMS_CMA_HELPER
- select DRM_GEM_CMA_HELPER
- help
- Use DRM CMA helpers to allocate display buffers.
- This is useful for the use-cases when guest driver needs to
- share or export buffers to other drivers which only expect
- contiguous buffers.
- Note: in this mode driver cannot use buffers allocated
- by the backend.
diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
index 352730dc6c13..712afff5ffc3 100644
--- a/drivers/gpu/drm/xen/Makefile
+++ b/drivers/gpu/drm/xen/Makefile
@@ -5,12 +5,7 @@ drm_xen_front-objs := xen_drm_front.o \
xen_drm_front_conn.o \
xen_drm_front_evtchnl.o \
xen_drm_front_shbuf.o \
- xen_drm_front_cfg.o
-
-ifeq ($(CONFIG_DRM_XEN_FRONTEND_CMA),y)
- drm_xen_front-objs += xen_drm_front_gem_cma.o
-else
- drm_xen_front-objs += xen_drm_front_gem.o
-endif
+ xen_drm_front_cfg.o \
+ xen_drm_front_gem.o
obj-$(CONFIG_DRM_XEN_FRONTEND) += drm_xen_front.o
diff --git a/drivers/gpu/drm/xen/xen_drm_front.c
b/drivers/gpu/drm/xen/xen_drm_front.c
index 4a08b77f1c9e..1b0ea9ac330e 100644
--- a/drivers/gpu/drm/xen/xen_drm_front.c
+++ b/drivers/gpu/drm/xen/xen_drm_front.c
@@ -12,7 +12,6 @@
#include
#include
#include
-#include
#include
@@ -167,10 +166,9 @@ int xen_drm_front_mode_set(struct
xen_drm_front_drm_pipeline *pipeline,
return ret;
}
-static int be_dbuf_create_int(struct xen_drm_front_info *front_info,
+int xen_drm_front_dbuf_create(struct xen_drm_front_info *front_info,
u64 dbuf_cookie, u32 width, u32 height,
- u32 bpp, u64 size, struct page **pages,
- struct sg_table *sgt)
+ u32 bpp, u64 size, struct page **pages)
{
struct xen_drm_front_evtchnl *evtchnl;
struct xen_drm_front_shbuf *shbuf;
@@ -187,7 +185,6 @@ static int be_dbuf_create_int(struct xen_drm_front_info
*front_info,
buf_cfg.xb_dev = front_info->xb_dev;
buf_cfg.pages = pages;
buf_cfg.size = size;
- buf_cfg.sgt = sgt;
buf_cfg.be_alloc = front_info->cfg.be_alloc;
shbuf = xen_drm_front_shbuf_alloc(&buf_cfg);
@@ -237,22 +234,6 @@ static int be_dbuf_create_int(struct xen_drm_front_info
*front_info,
return ret;
}
-int xen_drm_front_dbuf_create_from_sgt(struct xen_drm_front_info *front_info,
- u64 dbuf_cookie, u32 width, u
[PATCH] gpu: drm: vgem: Change return type to vm_fault_t
Use new return type vm_fault_t for fault handler.
Signed-off-by: Souptick Joarder
Reviewed-by: Matthew Wilcox
---
drivers/gpu/drm/vgem/vgem_drv.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 2524ff1..c64a859 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -61,13 +61,13 @@ static void vgem_gem_free_object(struct drm_gem_object *obj)
kfree(vgem_obj);
}
-static int vgem_gem_fault(struct vm_fault *vmf)
+static vm_fault_t vgem_gem_fault(struct vm_fault *vmf)
{
struct vm_area_struct *vma = vmf->vma;
struct drm_vgem_gem_object *obj = vma->vm_private_data;
/* We don't use vmf->pgoff since that has the fake offset */
unsigned long vaddr = vmf->address;
- int ret;
+ vm_fault_t ret = VM_FAULT_SIGBUS;
loff_t num_pages;
pgoff_t page_offset;
page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT;
@@ -77,7 +77,6 @@ static int vgem_gem_fault(struct vm_fault *vmf)
if (page_offset > num_pages)
return VM_FAULT_SIGBUS;
- ret = -ENOENT;
mutex_lock(&obj->pages_lock);
if (obj->pages) {
get_page(obj->pages[page_offset]);
--
1.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: GPU/DRM issue with DSI commands on msm 8916
Hi Archit, On Monday, April 09, 2018 03:08 PM, Archit Taneja wrote: >>> You could comment out the pm_runtime_put_sync() calls in >>> drivers/gpu/drm/msm/dsi/dsi_host.c, in case some registers got >>> reset during put_sync and weren't restored correctly after get_sync(). >> >> That was my first suspicion too, but unfortunately, that's not the culprit. >> I think it would be good to comment out the put_sync() calls in > drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c and > drivers/gpu/drm/msm/msm_drv.c too, since there is a parent-child > hierarchy between DSI > and the top level MDSS block. Commenting out the put_syncs() just > in put_sync() might still result in the MDSS GDSC to switch off. I spent some more time debugging this today and it turns out that calling into dsi_link_clk_enable() from msm_dsi_host_xfer_prepare() is already causing the drop-outs, even when no command buffers, DMA transfers etc. are involved. I then drilled further down and it showed that at least clk_set_rate(msm_host->byte_clk, msm_host->byte_clk_rate); in dsi_link_clk_enable_6g() one of the culprits. If I don't touch the clocks anymore after the initialization is done, everything is fine. That rules out all other components such as GPU and IOMMU, but I still don't grok what's going on, because I can't see a big difference in the relevant clock functions in the dsi driver and the clock drivers between 4.9 and 4.14. Any idea? I'll do some more debugging tomorrow. Thanks, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 2/4] drm/tegra: dc: Rename supports_blending to legacy_blending
Older Tegra's support blending. Rename SoC info entry supports_blending
to legacy_blending to eliminate confusion.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/tegra/dc.c | 20 ++--
drivers/gpu/drm/tegra/dc.h | 2 +-
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index a2bf216d8854..a54eefea2513 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -440,10 +440,10 @@ static void tegra_dc_setup_window(struct tegra_plane
*plane,
tegra_plane_writel(plane, value, DC_WIN_WIN_OPTIONS);
- if (dc->soc->supports_blending)
- tegra_plane_setup_blending(plane, window);
- else
+ if (dc->soc->legacy_blending)
tegra_plane_setup_blending_legacy(plane);
+ else
+ tegra_plane_setup_blending(plane, window);
}
static const u32 tegra20_primary_formats[] = {
@@ -549,7 +549,7 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
* the corresponding opaque formats. However, the opaque formats can
* be emulated by disabling alpha blending for the plane.
*/
- if (!dc->soc->supports_blending) {
+ if (dc->soc->legacy_blending) {
err = tegra_plane_setup_legacy_state(tegra, plane_state);
if (err < 0)
return err;
@@ -2092,7 +2092,7 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info
= {
.supports_interlacing = false,
.supports_cursor = false,
.supports_block_linear = false,
- .supports_blending = false,
+ .legacy_blending = true,
.pitch_align = 8,
.has_iommu = false,
.has_powergate = false,
@@ -2110,7 +2110,7 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info
= {
.supports_interlacing = false,
.supports_cursor = false,
.supports_block_linear = false,
- .supports_blending = false,
+ .legacy_blending = true,
.pitch_align = 8,
.has_iommu = true,
.has_powergate = false,
@@ -2128,7 +2128,7 @@ static const struct tegra_dc_soc_info
tegra114_dc_soc_info = {
.supports_interlacing = false,
.supports_cursor = false,
.supports_block_linear = false,
- .supports_blending = false,
+ .legacy_blending = true,
.pitch_align = 64,
.has_iommu = true,
.has_powergate = true,
@@ -2146,7 +2146,7 @@ static const struct tegra_dc_soc_info
tegra124_dc_soc_info = {
.supports_interlacing = true,
.supports_cursor = true,
.supports_block_linear = true,
- .supports_blending = true,
+ .legacy_blending = false,
.pitch_align = 64,
.has_iommu = true,
.has_powergate = true,
@@ -2164,7 +2164,7 @@ static const struct tegra_dc_soc_info
tegra210_dc_soc_info = {
.supports_interlacing = true,
.supports_cursor = true,
.supports_block_linear = true,
- .supports_blending = true,
+ .legacy_blending = false,
.pitch_align = 64,
.has_iommu = true,
.has_powergate = true,
@@ -2216,7 +2216,7 @@ static const struct tegra_dc_soc_info
tegra186_dc_soc_info = {
.supports_interlacing = true,
.supports_cursor = true,
.supports_block_linear = true,
- .supports_blending = true,
+ .legacy_blending = false,
.pitch_align = 64,
.has_powergate = false,
.coupled_pm = false,
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index d0bbcbcde4c1..3156006e75c6 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -55,7 +55,7 @@ struct tegra_dc_soc_info {
bool supports_interlacing;
bool supports_cursor;
bool supports_block_linear;
- bool supports_blending;
+ bool legacy_blending;
unsigned int pitch_align;
bool has_iommu;
bool has_powergate;
--
2.17.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] gpu: drm: armada: Adding new typedef vm_fault_t
Use new return type vm_fault_t for fault handler
in struct vm_operations_struct. For now, this is
just documenting that the function returns a VM_
FAULT value rather than an errno. Once all inst
ances are converted, vm_fault_t will become a di
stinct type.
Signed-off-by: Souptick Joarder
Reviewed-by: Matthew Wilcox
---
drivers/gpu/drm/armada/armada_gem.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_gem.c
b/drivers/gpu/drm/armada/armada_gem.c
index a97f509..81a55b7 100644
--- a/drivers/gpu/drm/armada/armada_gem.c
+++ b/drivers/gpu/drm/armada/armada_gem.c
@@ -13,25 +13,14 @@
#include
#include "armada_ioctlP.h"
-static int armada_gem_vm_fault(struct vm_fault *vmf)
+static vm_fault_t armada_gem_vm_fault(struct vm_fault *vmf)
{
struct drm_gem_object *gobj = vmf->vma->vm_private_data;
struct armada_gem_object *obj = drm_to_armada_gem(gobj);
unsigned long pfn = obj->phys_addr >> PAGE_SHIFT;
- int ret;
pfn += (vmf->address - vmf->vma->vm_start) >> PAGE_SHIFT;
- ret = vm_insert_pfn(vmf->vma, vmf->address, pfn);
-
- switch (ret) {
- case 0:
- case -EBUSY:
- return VM_FAULT_NOPAGE;
- case -ENOMEM:
- return VM_FAULT_OOM;
- default:
- return VM_FAULT_SIGBUS;
- }
+ return vmf_insert_pfn(vmf->vma, vmf->address, pfn);
}
const struct vm_operations_struct armada_gem_vm_ops = {
--
1.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 0/4] More DRM object properties on Tegra
This patchset adds new custom plane / CRTC properties that are very useful for displaying video overlay, they will be used by Opentegra Xorg driver. Also generic zPos property is implemented for older Tegra's. Dmitry Osipenko (4): drm/tegra: plane: Implement zPos plane property for older Tegra's drm/tegra: dc: Rename supports_blending to legacy_blending drm/tegra: plane: Add custom colorkey properties for older Tegra's drm/tegra: plane: Add custom CSC BLOB property drivers/gpu/drm/tegra/dc.c| 402 +- drivers/gpu/drm/tegra/dc.h| 31 ++- drivers/gpu/drm/tegra/plane.c | 268 ++- drivers/gpu/drm/tegra/plane.h | 27 ++- include/uapi/drm/tegra_drm.h | 11 + 5 files changed, 623 insertions(+), 116 deletions(-) -- 2.17.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 24/30] drm/rockchip: Disable PSR on input events
On Thu, 2018-04-05 at 11:49 +0200, Enric Balletbo i Serra wrote: > From: "Kristian H. Kristensen" > > To improve PSR exit latency, we speculatively start exiting when we > receive input events. Occasionally, this may lead to false positives, > but most of the time we get a head start on coming out of PSR. > Depending > on how userspace takes to produce a new frame in response to the > event, > this can completely hide the exit latency. In case of Chrome OS, we > typically get the input notifier 50ms or more before the dirty_fb > triggered exit. > Why is this rockchip-specific? It sounds more like something we'd want to integrate generically for drivers to leverage. Regards, Eze ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] gpu: drm: tegra: Adding new typedef vm_fault_t
Use new return type vm_fault_t for fault handler
in vm_operations_struct.
Signed-off-by: Souptick Joarder
---
drivers/gpu/drm/tegra/gem.c | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 49b9bf2..6121493 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -422,14 +422,13 @@ int tegra_bo_dumb_create(struct drm_file *file, struct
drm_device *drm,
return 0;
}
-static int tegra_bo_fault(struct vm_fault *vmf)
+static vm_fault_t tegra_bo_fault(struct vm_fault *vmf)
{
struct vm_area_struct *vma = vmf->vma;
struct drm_gem_object *gem = vma->vm_private_data;
struct tegra_bo *bo = to_tegra_bo(gem);
struct page *page;
pgoff_t offset;
- int err;
if (!bo->pages)
return VM_FAULT_SIGBUS;
@@ -437,20 +436,7 @@ static int tegra_bo_fault(struct vm_fault *vmf)
offset = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
page = bo->pages[offset];
- err = vm_insert_page(vma, vmf->address, page);
- switch (err) {
- case -EAGAIN:
- case 0:
- case -ERESTARTSYS:
- case -EINTR:
- case -EBUSY:
- return VM_FAULT_NOPAGE;
-
- case -ENOMEM:
- return VM_FAULT_OOM;
- }
-
- return VM_FAULT_SIGBUS;
+ return vmf_insert_page(vma, vmf->address, page);
}
const struct vm_operations_struct tegra_bo_vm_ops = {
--
1.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 24/30] drm/rockchip: Disable PSR on input events
On Mon, Apr 16, 2018 at 09:19:21AM +0200, Andrzej Hajda wrote:
>
> +CC: linux-input list and maintainer
>
>
> On 05.04.2018 11:49, Enric Balletbo i Serra wrote:
> > From: "Kristian H. Kristensen"
> >
> > To improve PSR exit latency, we speculatively start exiting when we
> > receive input events. Occasionally, this may lead to false positives,
> > but most of the time we get a head start on coming out of PSR. Depending
> > on how userspace takes to produce a new frame in response to the event,
> > this can completely hide the exit latency. In case of Chrome OS, we
> > typically get the input notifier 50ms or more before the dirty_fb
> > triggered exit.
>
> As I see from the code below, you just need notification from input
> subsystem on user activity.
> Maybe there is some simpler notification mechanism for such things?
> If not, maybe such helper should be created in input subsystem, I
> suppose it could be reused in other places as well.
Creating an input_handler is how you get user activity. It allows you to
decide what devices you are interested in and you get events so you
decide what to do with them.
I am pretty sure using something like atomic notifier is not that much
simpler, but much less flexible.
I wonder though we we really need to open devices when we connect to
them, or if we can leave it as is and only receive events if someone
else opened the device and is consuming events.
Thanks.
>
> Regards
> Andrzej
>
> >
> > Signed-off-by: Kristian H. Kristensen
> > Signed-off-by: Thierry Escande
> > Signed-off-by: Enric Balletbo i Serra
> > Tested-by: Marek Szyprowski
> > ---
> >
> > drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 134
> >
> > 1 file changed, 134 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> > b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> > index 9376f4396b6b..a107845ba97c 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> > @@ -12,6 +12,8 @@
> > * GNU General Public License for more details.
> > */
> >
> > +#include
> > +
> > #include
> > #include
> >
> > @@ -35,6 +37,9 @@ struct psr_drv {
> > enum psr_state state;
> >
> > struct delayed_work flush_work;
> > + struct work_struct disable_work;
> > +
> > + struct input_handlerinput_handler;
> >
> > int (*set)(struct drm_encoder *encoder, bool enable);
> > };
> > @@ -133,6 +138,18 @@ static void psr_flush_handler(struct work_struct *work)
> > mutex_unlock(&psr->lock);
> > }
> >
> > +static void psr_disable_handler(struct work_struct *work)
> > +{
> > + struct psr_drv *psr = container_of(work, struct psr_drv, disable_work);
> > +
> > + /* If the state has changed since we initiated the flush, do nothing */
> > + mutex_lock(&psr->lock);
> > + if (psr->state == PSR_ENABLE)
> > + psr_set_state_locked(psr, PSR_FLUSH);
> > + mutex_unlock(&psr->lock);
> > + mod_delayed_work(system_wq, &psr->flush_work, PSR_FLUSH_TIMEOUT_MS);
> > +}
> > +
> > /**
> > * rockchip_drm_psr_activate - activate PSR on the given pipe
> > * @encoder: encoder to obtain the PSR encoder
> > @@ -173,6 +190,7 @@ int rockchip_drm_psr_deactivate(struct drm_encoder
> > *encoder)
> > psr->active = false;
> > mutex_unlock(&psr->lock);
> > cancel_delayed_work_sync(&psr->flush_work);
> > + cancel_work_sync(&psr->disable_work);
> >
> > return 0;
> > }
> > @@ -226,6 +244,95 @@ void rockchip_drm_psr_flush_all(struct drm_device *dev)
> > }
> > EXPORT_SYMBOL(rockchip_drm_psr_flush_all);
> >
> > +static void psr_input_event(struct input_handle *handle,
> > + unsigned int type, unsigned int code,
> > + int value)
> > +{
> > + struct psr_drv *psr = handle->handler->private;
> > +
> > + schedule_work(&psr->disable_work);
> > +}
> > +
> > +static int psr_input_connect(struct input_handler *handler,
> > +struct input_dev *dev,
> > +const struct input_device_id *id)
> > +{
> > + struct input_handle *handle;
> > + int error;
> > +
> > + handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
> > + if (!handle)
> > + return -ENOMEM;
> > +
> > + handle->dev = dev;
> > + handle->handler = handler;
> > + handle->name = "rockchip-psr";
> > +
> > + error = input_register_handle(handle);
> > + if (error)
> > + goto err2;
> > +
> > + error = input_open_device(handle);
> > + if (error)
> > + goto err1;
> > +
> > + return 0;
> > +
> > +err1:
> > + input_unregister_handle(handle);
> > +err2:
> > + kfree(handle);
> > + return error;
> > +}
> > +
> > +static void psr_input_disconnect(struct input_handle *handle)
> > +{
> > + input_close_device(handle);
> > + input_unregister_handle(handle);
> > + kfree(handle);
> > +}
> > +
> > +/* Same device ids as cpu-boost */
> > +static const s
[PATCH v1 3/4] drm/tegra: plane: Add custom colorkey properties for older Tegra's
Colorkey'ing allows to draw on top of overlapping planes, like for example
on top of a video plane. Older Tegra's have a limited colorkey'ing
capability such that blending features are reduced when colorkey'ing is
enabled. In particular dependent weighting isn't possible, meaning that
cursors plane can't be displayed properly. In most cases it is more useful
to display content on top of video overlay, sacrificing mouse cursor
in the area of three planes intersection with colorkey mismatch. This
patch adds a custom colorkey properties to primary plane and CRTC's of
older Tegra's, allowing userspace like Opentegra Xorg driver to implement
colorkey support for XVideo extension.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/tegra/dc.c| 166 ++
drivers/gpu/drm/tegra/dc.h| 18 +++-
drivers/gpu/drm/tegra/plane.c | 40
drivers/gpu/drm/tegra/plane.h | 9 +-
4 files changed, 231 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index a54eefea2513..b19e954a223f 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -172,6 +172,24 @@ static void tegra_plane_setup_blending_legacy(struct
tegra_plane *plane)
state = to_tegra_plane_state(plane->base.state);
+ /*
+* Assuming default zPos window order, enable color keying for cases
+* of overlapping with topping windows, excluding overlap with
+* window B. Due to limited HW capabilities, this allows to draw
+* primary plane on top of video overlay in areas where key isn't
+* matching. Though window C will be completely transparent in a
+* region of three windows intersection + key mismatch.
+*/
+ if (state->ckey0_enabled) {
+ background[0] |= BLEND_COLOR_KEY_0;
+ background[2] |= BLEND_COLOR_KEY_0;
+ }
+
+ if (state->ckey1_enabled) {
+ background[0] |= BLEND_COLOR_KEY_1;
+ background[2] |= BLEND_COLOR_KEY_1;
+ }
+
if (state->opaque) {
/*
* Since custom fix-weight blending isn't utilized and weight
@@ -729,6 +747,35 @@ static unsigned long tegra_plane_get_possible_crtcs(struct
drm_device *drm)
return 1 << drm->mode_config.num_crtc;
}
+static void tegra_plane_create_legacy_properties(struct tegra_plane *plane,
+struct drm_device *drm)
+{
+ plane->props.color_key0 = drm_property_create_bool(
+ drm, 0, "color key 0");
+ plane->props.color_key1 = drm_property_create_bool(
+ drm, 0, "color key 1");
+
+ if (!plane->props.color_key0 ||
+ !plane->props.color_key1)
+ goto err_cleanup;
+
+ drm_object_attach_property(&plane->base.base, plane->props.color_key0,
+ false);
+ drm_object_attach_property(&plane->base.base, plane->props.color_key1,
+ false);
+
+ return;
+
+err_cleanup:
+ if (plane->props.color_key0)
+ drm_property_destroy(drm, plane->props.color_key0);
+
+ if (plane->props.color_key1)
+ drm_property_destroy(drm, plane->props.color_key1);
+
+ dev_err(plane->dc->dev, "failed to create legacy plane properties\n");
+}
+
static struct drm_plane *tegra_primary_plane_create(struct drm_device *drm,
struct tegra_dc *dc)
{
@@ -764,6 +811,9 @@ static struct drm_plane *tegra_primary_plane_create(struct
drm_device *drm,
drm_plane_helper_add(&plane->base, &tegra_plane_helper_funcs);
drm_plane_create_zpos_property(&plane->base, plane->index, 0, 255);
+ if (dc->soc->legacy_blending)
+ tegra_plane_create_legacy_properties(plane, drm);
+
return &plane->base;
}
@@ -1153,6 +1203,8 @@ tegra_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
copy->pclk = state->pclk;
copy->div = state->div;
copy->planes = state->planes;
+ copy->ckey0 = state->ckey0;
+ copy->ckey1 = state->ckey1;
return ©->base;
}
@@ -1537,6 +1589,50 @@ static void tegra_dc_disable_vblank(struct drm_crtc
*crtc)
tegra_dc_writel(dc, value, DC_CMD_INT_MASK);
}
+static int tegra_crtc_atomic_set_property(struct drm_crtc *crtc,
+ struct drm_crtc_state *state,
+ struct drm_property *property,
+ uint64_t value)
+{
+ struct tegra_dc_state *tegra_state = to_dc_state(state);
+ struct tegra_dc *dc = to_tegra_dc(crtc);
+
+ if (property == dc->props.ckey0_lower)
+ tegra_state->ckey0.lower = value;
+ else if (property == dc->props.ckey0_upper)
+ tegra_state->ckey0.upper = value;
Re: [Freedreno] [DPU PATCH v2 2/2] drm/panel: add backlight control support for truly panel
On Mon 16 Apr 15:45 PDT 2018, abhin...@codeaurora.org wrote:
> Hi Bjorn
>
> Thanks for the review.
>
> Reply inline.
>
> On 2018-04-16 09:41, Bjorn Andersson wrote:
> > On Sat 14 Apr 00:25 PDT 2018, Abhinav Kumar wrote:
> >
> > > Register truly panel as a backlight led device and
> > > provide methods to control its backlight operation.
> > >
> > > Changes in v2:
> > > - Removed redundant NULL checks
> > > - Arranged headers alphabetically
> > > - Formatting fixes
> >
> > The change log goes below the "---" line.
> >
> [Abhinav] As sean mentioned, its quite common to list it to view it in the
> log.
> This practice has been followed by quite a few in DRM
> Another reference here https://patchwork.freedesktop.org/patch/211361/
>
If that's the practice in DRM land, then that's what you should do.
> > >
> > > Signed-off-by: Abhinav Kumar
> > > ---
> > [..]
> > > +static int truly_backlight_setup(struct truly_wqxga *ctx)
> > > +{
> > > + struct backlight_properties props;
> > > + char bl_node_name[BL_NODE_NAME_SIZE];
> > > +
> > > + if (!ctx->backlight) {
> > > + memset(&props, 0, sizeof(props));
> > > + props.type = BACKLIGHT_RAW;
> > > + props.power = FB_BLANK_UNBLANK;
> > > + props.max_brightness = 4096;
> > > +
> > > + snprintf(bl_node_name, BL_NODE_NAME_SIZE, "panel%u-backlight",
> > > + PRIM_DISPLAY_NODE);
> > > +
> > > + ctx->backlight = backlight_device_register(bl_node_name,
> > > + ctx->dev, ctx,
> > > + &truly_backlight_device_ops, &props);
> > > +
> > > + if (IS_ERR_OR_NULL(ctx->backlight)) {
> > > + pr_err("Failed to register backlight\n");
> > > + ctx->backlight = NULL;
> > > + return -ENODEV;
> > > + }
> > > +
> > > + /* Register with the LED driver interface */
> > > + led_trigger_register_simple("bkl-trigger", &ctx->wled);
> > > +
> > > + if (!ctx->wled) {
> > > + pr_err("backlight led registration failed\n");
> > > + return -ENODEV;
> > > + }
> >
> > It seems like you're registering a backlight driver for the sake of
> > invoking the LED backlight trigger to control the WLED.
> >
> > The WLED is a backlight driver, so all you should have to do is add the
> > following line to your probe:
> >
> > ctx->backlight = devm_of_find_backlight(dev);
> >
> > and then add "backlight = <&wled>" to your dt node.
> >
> > Regards,
> > Bjorn
> [Abhinav] Thats not the only purpose of backlight_device_register().
> We want to hook up our panel with the parent backlight driver in
> drivers/video/backlight/backlight.c and also route all the
> update_backlight_status()
> calls through the sysfs of the newly registered node.
>
> The of_find_backlight() method doesnt seem to allow us to register our own
> sysfs method.
>
Are you saying that you want to be able to create an alias for the wled
entry already in /sys/class/backlight named panel0-backlight?
> BTW, this isnt something which we are doing uniquely.
> There are other panels which seem to be doing this :
>
> drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
>
What seems to be going on in the s6e3ha2 driver is that the hardware has
some sort of builtin backlight control, so the driver is creating a
backlight driver for the purpose of exposing those controls.
> Can you please comment on whether we can have our own sysfs without
> the device_register()?
>
If the panel isn't actually a piece of backlight hardware then it should
not register a backlight device. Why do you need your own sysfs?
Regards,
Bjorn
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [DPU PATCH v2 2/2] drm/panel: add backlight control support for truly panel
On Sat 14 Apr 00:25 PDT 2018, Abhinav Kumar wrote:
> Register truly panel as a backlight led device and
> provide methods to control its backlight operation.
>
> Changes in v2:
> - Removed redundant NULL checks
> - Arranged headers alphabetically
> - Formatting fixes
The change log goes below the "---" line.
>
> Signed-off-by: Abhinav Kumar
> ---
[..]
> +static int truly_backlight_setup(struct truly_wqxga *ctx)
> +{
> + struct backlight_properties props;
> + char bl_node_name[BL_NODE_NAME_SIZE];
> +
> + if (!ctx->backlight) {
> + memset(&props, 0, sizeof(props));
> + props.type = BACKLIGHT_RAW;
> + props.power = FB_BLANK_UNBLANK;
> + props.max_brightness = 4096;
> +
> + snprintf(bl_node_name, BL_NODE_NAME_SIZE, "panel%u-backlight",
> + PRIM_DISPLAY_NODE);
> +
> + ctx->backlight = backlight_device_register(bl_node_name,
> + ctx->dev, ctx,
> + &truly_backlight_device_ops, &props);
> +
> + if (IS_ERR_OR_NULL(ctx->backlight)) {
> + pr_err("Failed to register backlight\n");
> + ctx->backlight = NULL;
> + return -ENODEV;
> + }
> +
> + /* Register with the LED driver interface */
> + led_trigger_register_simple("bkl-trigger", &ctx->wled);
> +
> + if (!ctx->wled) {
> + pr_err("backlight led registration failed\n");
> + return -ENODEV;
> + }
It seems like you're registering a backlight driver for the sake of
invoking the LED backlight trigger to control the WLED.
The WLED is a backlight driver, so all you should have to do is add the
following line to your probe:
ctx->backlight = devm_of_find_backlight(dev);
and then add "backlight = <&wled>" to your dt node.
Regards,
Bjorn
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 4/4] drm/tegra: plane: Add custom CSC BLOB property
This new property allows userspace to apply custom color conversion
coefficients per plane, making possible to utilize display controller
for color adjustments of a video overlay.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/tegra/dc.c| 86 +++
drivers/gpu/drm/tegra/dc.h| 11 +
drivers/gpu/drm/tegra/plane.c | 35 ++
drivers/gpu/drm/tegra/plane.h | 5 ++
include/uapi/drm/tegra_drm.h | 11 +
5 files changed, 139 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index b19e954a223f..24a1317871d4 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -435,15 +435,15 @@ static void tegra_dc_setup_window(struct tegra_plane
*plane,
value = WIN_ENABLE;
if (yuv) {
- /* setup default colorspace conversion coefficients */
- tegra_plane_writel(plane, 0x00f0, DC_WIN_CSC_YOF);
- tegra_plane_writel(plane, 0x012a, DC_WIN_CSC_KYRGB);
- tegra_plane_writel(plane, 0x, DC_WIN_CSC_KUR);
- tegra_plane_writel(plane, 0x0198, DC_WIN_CSC_KVR);
- tegra_plane_writel(plane, 0x039b, DC_WIN_CSC_KUG);
- tegra_plane_writel(plane, 0x032f, DC_WIN_CSC_KVG);
- tegra_plane_writel(plane, 0x0204, DC_WIN_CSC_KUB);
- tegra_plane_writel(plane, 0x, DC_WIN_CSC_KVB);
+ /* setup colorspace conversion coefficients */
+ tegra_plane_writel(plane, window->csc.yof, DC_WIN_CSC_YOF);
+ tegra_plane_writel(plane, window->csc.kyrgb, DC_WIN_CSC_KYRGB);
+ tegra_plane_writel(plane, window->csc.kur, DC_WIN_CSC_KUR);
+ tegra_plane_writel(plane, window->csc.kvr, DC_WIN_CSC_KVR);
+ tegra_plane_writel(plane, window->csc.kug, DC_WIN_CSC_KUG);
+ tegra_plane_writel(plane, window->csc.kvg, DC_WIN_CSC_KVG);
+ tegra_plane_writel(plane, window->csc.kub, DC_WIN_CSC_KUB);
+ tegra_plane_writel(plane, window->csc.kvb, DC_WIN_CSC_KVB);
value |= CSC_ENABLE;
} else if (window->bits_per_pixel < 24) {
@@ -624,6 +624,7 @@ static void tegra_plane_atomic_update(struct drm_plane
*plane,
struct drm_framebuffer *fb = plane->state->fb;
struct tegra_plane *p = to_tegra_plane(plane);
struct tegra_dc_window window;
+ const struct drm_tegra_plane_csc_blob *csc;
unsigned int i;
/* rien ne va plus */
@@ -665,6 +666,28 @@ static void tegra_plane_atomic_update(struct drm_plane
*plane,
window.stride[i] = fb->pitches[i];
}
+ if (state->csc_blob) {
+ csc = state->csc_blob->data;
+
+ window.csc.yof = csc->yof;
+ window.csc.kyrgb = csc->kyrgb;
+ window.csc.kur = csc->kur;
+ window.csc.kvr = csc->kvr;
+ window.csc.kug = csc->kug;
+ window.csc.kvg = csc->kvg;
+ window.csc.kub = csc->kub;
+ window.csc.kvb = csc->kvb;
+ } else {
+ window.csc.yof = 0x00f0;
+ window.csc.kyrgb = 0x012a;
+ window.csc.kur = 0x;
+ window.csc.kvr = 0x0198;
+ window.csc.kug = 0x039b;
+ window.csc.kvg = 0x032f;
+ window.csc.kub = 0x0204;
+ window.csc.kvb = 0x;
+ }
+
tegra_dc_setup_window(p, &window);
}
@@ -776,6 +799,42 @@ static void tegra_plane_create_legacy_properties(struct
tegra_plane *plane,
dev_err(plane->dc->dev, "failed to create legacy plane properties\n");
}
+static void tegra_plane_create_csc_property(struct tegra_plane *plane)
+{
+ /* set default colorspace conversion coefficients to ITU-R BT.601 */
+ struct drm_tegra_plane_csc_blob csc_bt601 = {
+ .yof = 0x00f0,
+ .kyrgb = 0x012a,
+ .kur = 0x,
+ .kvr = 0x0198,
+ .kug = 0x039b,
+ .kvg = 0x032f,
+ .kub = 0x0204,
+ .kvb = 0x,
+ };
+ struct drm_property_blob *blob;
+
+ blob = drm_property_create_blob(plane->base.dev, sizeof(csc_bt601),
+ &csc_bt601);
+ if (!blob) {
+ dev_err(plane->dc->dev, "failed to create CSC BLOB\n");
+ return;
+ }
+
+ plane->props.csc_blob = drm_property_create(
+ plane->base.dev, DRM_MODE_PROP_BLOB, "YUV to RGB CSC", 0);
+
+ if (!plane->props.csc_blob) {
+ dev_err(plane->dc->dev, "failed to create CSC property\n");
+ drm_property_blob_put(blob);
+ return;
+ }
+
+ drm_object_attach_property(&plane->base.base, plane->props.csc_blob, 0);
+
+ plane->csc_default = blob;
+}
+
static struct drm_plane *tegra_primary_plane_create(struct d
[PATCH v1 1/4] drm/tegra: plane: Implement zPos plane property for older Tegra's
Older Tegra's do not support planes z position handling in hardware,
but HW provides knobs for zPos implementation in software.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/tegra/dc.c| 134 ---
drivers/gpu/drm/tegra/plane.c | 193 --
drivers/gpu/drm/tegra/plane.h | 13 ++-
3 files changed, 244 insertions(+), 96 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 9d885d63724d..a2bf216d8854 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -162,29 +162,90 @@ static void tegra_plane_setup_blending_legacy(struct
tegra_plane *plane)
u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |
BLEND_COLOR_KEY_NONE;
u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
+ u32 blending[2];
struct tegra_plane_state *state;
unsigned int i;
+ /* disable blending for non-overlapping case */
+ tegra_plane_writel(plane, blendnokey, DC_WIN_BLEND_NOKEY);
+ tegra_plane_writel(plane, foreground, DC_WIN_BLEND_1WIN);
+
state = to_tegra_plane_state(plane->base.state);
- /* alpha contribution is 1 minus sum of overlapping windows */
- for (i = 0; i < 3; i++) {
- if (state->dependent[i])
- background[i] |= BLEND_CONTROL_DEPENDENT;
- }
+ if (state->opaque) {
+ /*
+* Since custom fix-weight blending isn't utilized and weight
+* of top window is set to max, we can enforce dependent
+* blending which in this case results in transparent bottom
+* window if top window is opaque and if top window enables
+* alpha blending, then bottom window is getting alpha value
+* of 1 minus the sum of alpha components of the overlapping
+* plane.
+*/
+ background[0] |= BLEND_CONTROL_DEPENDENT;
+ background[1] |= BLEND_CONTROL_DEPENDENT;
- /* enable alpha blending if pixel format has an alpha component */
- if (!state->opaque)
+ /*
+* The region where three windows overlap is the intersection
+* of the two regions where two windows overlap. It contributes
+* to the area if all of the windows on top of it have an alpha
+* component.
+*/
+ switch (state->base.normalized_zpos) {
+ case 0:
+ if (state->blending[0].alpha &&
+ state->blending[1].alpha)
+ background[2] |= BLEND_CONTROL_DEPENDENT;
+ break;
+
+ case 1:
+ background[2] |= BLEND_CONTROL_DEPENDENT;
+ break;
+ }
+ } else {
+ /*
+* Enable alpha blending if pixel format has an alpha
+* component.
+*/
foreground |= BLEND_CONTROL_ALPHA;
- /*
-* Disable blending and assume Window A is the bottom-most window,
-* Window C is the top-most window and Window B is in the middle.
-*/
- tegra_plane_writel(plane, blendnokey, DC_WIN_BLEND_NOKEY);
- tegra_plane_writel(plane, foreground, DC_WIN_BLEND_1WIN);
+ /*
+* If any of the windows on top of this window is opaque, it
+* will completely conceal this window within that area. If
+* top window has an alpha component, it is blended over the
+* bottom window.
+*/
+ for (i = 0; i < 2; i++) {
+ if (state->blending[i].alpha &&
+ state->blending[i].top)
+ background[i] |= BLEND_CONTROL_DEPENDENT;
+ }
+
+ switch (state->base.normalized_zpos) {
+ case 0:
+ if (state->blending[0].alpha &&
+ state->blending[1].alpha)
+ background[2] |= BLEND_CONTROL_DEPENDENT;
+ break;
+
+ case 1:
+ /*
+* When both middle and topmost windows have an alpha,
+* these windows a mixed together and then the result
+* is blended over the bottom window.
+*/
+ if ((state->blending[0].alpha &&
+state->blending[0].top))
+ background[2] |= BLEND_CONTROL_ALPHA;
- switch (plane->index) {
+ if (state->blending[1].alpha &&
+ state->blending[1].top)
+ background[2] |= BLEND_CONTROL_ALPHA;
+ brea
Re: [PATCH] gpu: drm: gma500: Change return type to vm_fault_t
>>> drivers/gpu/drm/gma500/framebuffer.c:136:9: error: implicit declaration of >>> function 'vmf_insert_mixed'; did you mean 'vm_insert_mixed'? >>> [-Werror=implicit-function-declaration] > ret = vmf_insert_mixed(vma, address, > ^~~~ >cc1: some warnings being treated as errors > -- >>> drivers/gpu/drm/gma500/gem.c:165:10: error: implicit declaration of >>> function 'vmf_error' [-Werror=implicit-function-declaration] >ret = vmf_error(err); Sorry about it. vmf_error() is not yet merged in linus tree and it break the build. >>> drivers/gpu/drm/gma500/gem.c:180:8: error: implicit declaration of function >>> 'vmf_insert_pfn'; did you mean 'vm_insert_pfn'? >>> [-Werror=implicit-function-declaration] > ret = vmf_insert_pfn(vma, vmf->address, pfn); vm_insert_mixed() , vmf_insert_pfn() is defined in and vm_fault_t is defined in vm_fault_t in and merged in both 4.17-rc1 and next-20180413. So these shouldn't break the build. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 199047] [amdgpu CARRIZO] disabled backlight after S3 resume
https://bugzilla.kernel.org/show_bug.cgi?id=199047 Johannes Hirte (johannes.hi...@datenkhaos.de) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |CODE_FIX --- Comment #8 from Johannes Hirte (johannes.hi...@datenkhaos.de) --- fixed -- 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 105684] Loading amdgpu hits general protection fault: 0000 [#1] SMP NOPTI
https://bugs.freedesktop.org/show_bug.cgi?id=105684 --- Comment #16 from jian-h...@endlessm.com --- Created attachment 138874 --> https://bugs.freedesktop.org/attachment.cgi?id=138874&action=edit dmesg of loading amdgpu module - tested in 4.17-rc1 I have tried Linux kernel 4.17-rc1 and load amdgpu module manually. The error looks same as the test with linux kernel v4.16-rc7: "general protection fault: [#1] SMP NOPTI" when load amdgpu module. [ 34.068370] [drm] use_doorbell being set to: [true] [ 34.092115] [drm] Found VCN firmware Version: 1.45 Family ID: 18 [ 34.261809] amdgpu: [powerplay] dpm has been enabled [ 34.274305] [drm] Display Core initialized with v3.1.38! [ 34.274343] general protection fault: [#1] SMP NOPTI [ 34.274345] Modules linked in: amdkfd amd_iommu_v2 amdgpu(+) chash gpu_sched ttm drm_kms_helper drm i2c_algo_bit fb_sys_fops syscopyarea sysfillrect sysimgblt efi_pstore cmac bnep arc4 btusb btrtl btbcm zram btintel bluetooth snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep ecdh_generic input_leds snd_pcm edac_mce_amd kvm_amd ccp kvm snd_timer snd r8169 soundcore irqbypass crct10dif_pclmul crc32_pclmul ath10k_pci ath10k_core ath ghash_clmulni_intel mac80211 pcbc aesni_intel cfg80211 mii aes_x86_64 ahci libahci crypto_simd wmi_bmof shpchp sparse_keymap i2c_piix4 cryptd glue_helper psmouse mac_hid wmi ip_tables x_tables hid_generic uas serio_raw usbhid usb_storage hid video [ 34.274391] CPU: 0 PID: 766 Comm: modprobe Not tainted 4.17.0-rc1 #12 [ 34.274393] Hardware name: Acer Aspire TC-380/Aspire TC-380, BIOS D05 02/01/2018 [ 34.274400] RIP: 0010:prefetch_freepointer+0x14/0x30 [ 34.274402] RSP: 0018:b860482d7838 EFLAGS: 00010206 [ 34.274404] RAX: RBX: 9ed4392e2400 RCX: 064d [ 34.274406] RDX: 064c RSI: 69a633535ad058a5 RDI: 9ed43f006e80 [ 34.274408] RBP: b860482d7838 R08: 9ed43f627160 R09: 9ed41a3d6900 [ 34.274410] R10: 0024 R11: 9ed4203278cf R12: 014080c0 [ 34.274412] R13: 9ed43f006e80 R14: 9ed4392e2400 R15: 9ed43f006e80 [ 34.274415] FS: 7f3c39e38700() GS:9ed43f60() knlGS: [ 34.274417] CS: 0010 DS: ES: CR0: 80050033 [ 34.274419] CR2: 7f8b1427e000 CR3: 0007fb03 CR4: 003406f0 [ 34.274421] Call Trace: [ 34.274426] kmem_cache_alloc_trace+0xbb/0x1d0 [ 34.274479] ? dm_hw_init+0x476/0xe60 [amdgpu] [ 34.274522] dm_hw_init+0x476/0xe60 [amdgpu] [ 34.274526] ? vprintk_func+0x27/0x60 [ 34.274528] ? printk+0x52/0x6e [ 34.274564] amdgpu_device_init+0x13c5/0x1490 [amdgpu] [ 34.274599] amdgpu_driver_load_kms+0x8b/0x2c0 [amdgpu] [ 34.274611] drm_dev_register+0x149/0x1e0 [drm] [ 34.274646] amdgpu_pci_probe+0x13f/0x1f0 [amdgpu] [ 34.274650] local_pci_probe+0x4a/0xa0 [ 34.274652] pci_device_probe+0x109/0x1b0 [ 34.274655] driver_probe_device+0x2bb/0x4a0 [ 34.274658] __driver_attach+0xe2/0xf0 [ 34.274660] ? driver_probe_device+0x4a0/0x4a0 [ 34.274663] bus_for_each_dev+0x6a/0xc0 [ 34.274665] ? kmem_cache_alloc_trace+0x1c4/0x1d0 [ 34.274667] driver_attach+0x1e/0x20 [ 34.274670] bus_add_driver+0x170/0x260 [ 34.274672] driver_register+0x60/0xe0 [ 34.274674] ? 0xc0a76000 [ 34.274677] __pci_register_driver+0x5a/0x60 [ 34.274711] amdgpu_init+0x7a/0x89 [amdgpu] [ 34.274714] do_one_initcall+0x52/0x1cd [ 34.274717] ? __vunmap+0x81/0xb0 [ 34.274720] ? _cond_resched+0x1a/0x50 [ 34.274722] ? kmem_cache_alloc_trace+0xbb/0x1d0 [ 34.274725] ? do_init_module+0x27/0x219 [ 34.274727] do_init_module+0x5f/0x219 [ 34.274729] load_module+0x260e/0x2e10 [ 34.274733] __do_sys_finit_module+0xe5/0x120 [ 34.274735] ? __do_sys_finit_module+0xe5/0x120 [ 34.274738] __x64_sys_finit_module+0x1a/0x20 [ 34.274741] do_syscall_64+0x5a/0x110 [ 34.274743] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 34.274745] RIP: 0033:0x7f3c3997d229 [ 34.274747] RSP: 002b:7ffdc43c4e58 EFLAGS: 0246 ORIG_RAX: 0139 [ 34.274750] RAX: ffda RBX: 55be49331280 RCX: 7f3c3997d229 [ 34.274752] RDX: RSI: 55be487e9638 RDI: 000d [ 34.274754] RBP: 55be487e9638 R08: R09: [ 34.274756] R10: 000d R11: 0246 R12: [ 34.274758] R13: 55be493313b0 R14: 0004 R15: [ 34.274760] Code: 5b 90 84 e8 12 88 e9 ff eb 91 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 85 f6 48 89 e5 74 13 8b 47 20 48 01 c6 <48> 33 36 48 33 b7 38 01 00 00 0f 18 0e 5d c3 0f 1f 00 66 2e 0f [ 34.274785] RIP: prefetch_freepointer+0x14/0x30 RSP: b860482d7838 [ 34.274788] ---[ end trace 2cfc1725d9f54c54 ]--- -- You are receiving this mail because: You are the assignee for the bug.__
[Bug 105760] [4.17-rc1] RIP: smu7_populate_single_firmware_entry.isra.6+0x57/0xc0 [amdgpu] RSP: ffffa17901efb930
https://bugs.freedesktop.org/show_bug.cgi?id=105760 --- Comment #5 from taij...@posteo.de --- Assuming that you mean amdgpu. -- 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 105760] [4.17-rc1] RIP: smu7_populate_single_firmware_entry.isra.6+0x57/0xc0 [amdgpu] RSP: ffffa17901efb930
https://bugs.freedesktop.org/show_bug.cgi?id=105760 --- Comment #4 from taij...@posteo.de --- It is build as a module and then embedded in the initramfs. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On Mon, Apr 16, 2018 at 12:29:05PM -0700, Dongwon Kim wrote: > Yeah, I definitely agree on the idea of expanding the use case to the > general domain where dmabuf sharing is used. However, what you are > targetting with proposed changes is identical to the core design of > hyper_dmabuf. > > On top of this basic functionalities, hyper_dmabuf has driver level > inter-domain communication, that is needed for dma-buf remote tracking > (no fence forwarding though), event triggering and event handling, extra > meta data exchange and hyper_dmabuf_id that represents grefs > (grefs are shared implicitly on driver level) This really isn't a positive design aspect of hyperdmabuf imo. The core code in xen-zcopy (ignoring the ioctl side, which will be cleaned up) is very simple & clean. If there's a clear need later on we can extend that. But for now xen-zcopy seems to cover the basic use-case needs, so gets the job done. > Also it is designed with frontend (common core framework) + backend > (hyper visor specific comm and memory sharing) structure for portability. > We just can't limit this feature to Xen because we want to use the same > uapis not only for Xen but also other applicable hypervisor, like ACORN. See the discussion around udmabuf and the needs for kvm. I think trying to make an ioctl/uapi that works for multiple hypervisors is misguided - it likely won't work. On top of that the 2nd hypervisor you're aiming to support is ACRN. That's not even upstream yet, nor have I seen any patches proposing to land linux support for ACRN. Since it's not upstream, it doesn't really matter for upstream consideration. I'm doubting that ACRN will use the same grant references as xen, so the same uapi won't work on ACRN as on Xen anyway. > So I am wondering we can start with this hyper_dmabuf then modify it for > your use-case if needed and polish and fix any glitches if we want to > to use this for all general dma-buf usecases. Imo xen-zcopy is a much more reasonable starting point for upstream, which can then be extended (if really proven to be necessary). > Also, I still have one unresolved question regarding the export/import flow > in both of hyper_dmabuf and xen-zcopy. > > @danvet: Would this flow (guest1->import existing dmabuf->share underlying > pages->guest2->map shared pages->create/export dmabuf) be acceptable now? I think if you just look at the pages, and make sure you handle the sg_page == NULL case it's ok-ish. It's not great, but mostly it should work. The real trouble with hyperdmabuf was the forwarding of all these calls, instead of just passing around a list of grant references. -Daniel > > Regards, > DW > > On Mon, Apr 16, 2018 at 05:33:46PM +0300, Oleksandr Andrushchenko wrote: > > Hello, all! > > > > After discussing xen-zcopy and hyper-dmabuf [1] approaches > > > > it seems that xen-zcopy can be made not depend on DRM core any more > > > > and be dma-buf centric (which it in fact is). > > > > The DRM code was mostly there for dma-buf's FD import/export > > > > with DRM PRIME UAPI and with DRM use-cases in mind, but it comes out that if > > > > the proposed 2 IOCTLs (DRM_XEN_ZCOPY_DUMB_FROM_REFS and > > DRM_XEN_ZCOPY_DUMB_TO_REFS) > > > > are extended to also provide a file descriptor of the corresponding dma-buf, > > then > > > > PRIME stuff in the driver is not needed anymore. > > > > That being said, xen-zcopy can safely be detached from DRM and moved from > > > > drivers/gpu/drm/xen into drivers/xen/dma-buf-backend(?). > > > > This driver then becomes a universal way to turn any shared buffer between > > Dom0/DomD > > > > and DomU(s) into a dma-buf, e.g. one can create a dma-buf from any grant > > references > > > > or represent a dma-buf as grant-references for export. > > > > This way the driver can be used not only for DRM use-cases, but also for > > other > > > > use-cases which may require zero copying between domains. > > > > For example, the use-cases we are about to work in the nearest future will > > use > > > > V4L, e.g. we plan to support cameras, codecs etc. and all these will benefit > > > > from zero copying much. Potentially, even block/net devices may benefit, > > > > but this needs some evaluation. > > > > > > I would love to hear comments for authors of the hyper-dmabuf > > > > and Xen community, as well as DRI-Devel and other interested parties. > > > > > > Thank you, > > > > Oleksandr > > > > > > On 03/29/2018 04:19 PM, Oleksandr Andrushchenko wrote: > > >From: Oleksandr Andrushchenko > > > > > >Hello! > > > > > >When using Xen PV DRM frontend driver then on backend side one will need > > >to do copying of display buffers' contents (filled by the > > >frontend's user-space) into buffers allocated at the backend side. > > >Taking into account the size of display buffers and frames per seconds > > >it may result in unneeded huge data bus occupation and performance loss. > > > > > >This helper driver allows implementing zero-copying us
[Bug 199425] New: BUG: KASAN: use-after-free in drm_atomic_helper_wait_for_flip_done+0x247/0x260
https://bugzilla.kernel.org/show_bug.cgi?id=199425 Bug ID: 199425 Summary: BUG: KASAN: use-after-free in drm_atomic_helper_wait_for_flip_done+0x247/0x260 Product: Drivers Version: 2.5 Kernel Version: 4.17-rc1 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: johannes.hi...@datenkhaos.de Regression: No With dc enabled, I get the following use-after-free on my Carrizo: [53213.875800] == [53213.875826] BUG: KASAN: use-after-free in drm_atomic_helper_wait_for_flip_done+0x247/0x260 [53213.875835] Read of size 8 at addr 8801063aaa88 by task kworker/u8:3/9911 [53213.875848] CPU: 3 PID: 9911 Comm: kworker/u8:3 Not tainted 4.17.0-rc1-1-g9e7729e9a66c #566 [53213.875855] Hardware name: HP HP ProBook 645 G2/80FE, BIOS N77 Ver. 01.12 12/19/2017 [53213.875864] Workqueue: events_unbound commit_work [53213.875870] Call Trace: [53213.875881] dump_stack+0x5b/0x8b [53213.875890] ? drm_atomic_helper_wait_for_flip_done+0x247/0x260 [53213.875899] print_address_description+0x65/0x270 [53213.875907] ? drm_atomic_helper_wait_for_flip_done+0x247/0x260 [53213.875913] kasan_report+0x232/0x350 [53213.875920] drm_atomic_helper_wait_for_flip_done+0x247/0x260 [53213.875930] amdgpu_dm_atomic_commit_tail+0x1b19/0x4010 [53213.875940] ? _raw_spin_unlock_irq+0x35/0x50 [53213.875946] ? wait_for_completion_timeout+0x215/0x2b0 [53213.875953] ? btrfs_rmap_block+0x9c0/0x9c0 [53213.875959] ? dm_update_crtcs_state+0xcb0/0xcb0 [53213.875966] ? _raw_spin_unlock_irqrestore+0x3a/0x70 [53213.875973] ? try_to_wake_up+0xa1/0xf90 [53213.875980] ? drm_atomic_helper_wait_for_dependencies+0x3de/0x7d0 [53213.875986] ? normal_work_helper+0x273/0xa70 [53213.875993] commit_tail+0x95/0xf0 [53213.876000] process_one_work+0x7c8/0x1330 [53213.876006] ? _raw_spin_lock_irq+0x1c/0x40 [53213.876013] worker_thread+0xc9/0xef0 [53213.876021] ? process_one_work+0x1330/0x1330 [53213.876026] kthread+0x2d6/0x390 [53213.876032] ? kthread_create_worker+0xd0/0xd0 [53213.876038] ret_from_fork+0x22/0x40 [53213.876049] Allocated by task 508: [53213.876056] kasan_kmalloc+0xa0/0xd0 [53213.876063] kmem_cache_alloc_trace+0xf3/0x1f0 [53213.876068] dm_crtc_duplicate_state+0x73/0x130 [53213.876075] drm_atomic_get_crtc_state+0x142/0x400 [53213.876080] page_flip_common+0x52/0x220 [53213.876086] drm_atomic_helper_page_flip+0xa1/0x100 [53213.876093] drm_mode_page_flip_ioctl+0xbe3/0xff0 [53213.876100] drm_ioctl_kernel+0x13d/0x1d0 [53213.876106] drm_ioctl+0x63d/0x920 [53213.876112] amdgpu_drm_ioctl+0xc7/0x1a0 [53213.876120] do_vfs_ioctl+0x173/0xde0 [53213.876125] ksys_ioctl+0x6b/0x80 [53213.876130] __x64_sys_ioctl+0x6a/0xb0 [53213.876137] do_syscall_64+0x95/0x2f0 [53213.876142] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [53213.876149] Freed by task 637: [53213.876154] __kasan_slab_free+0x130/0x180 [53213.876159] kfree+0x8b/0x1c0 [53213.876164] drm_atomic_state_default_clear+0x2c5/0xa00 [53213.876169] __drm_atomic_state_free+0x30/0xc0 [53213.876174] drm_atomic_helper_update_plane+0xb6/0x350 [53213.876179] __setplane_internal+0x48c/0x7f0 [53213.876184] drm_mode_cursor_universal+0x2e7/0x970 [53213.876189] drm_mode_cursor_common+0x493/0x860 [53213.876194] drm_mode_cursor_ioctl+0x7a/0xa0 [53213.876199] drm_ioctl_kernel+0x13d/0x1d0 [53213.876203] drm_ioctl+0x63d/0x920 [53213.876207] amdgpu_drm_ioctl+0xc7/0x1a0 [53213.876212] do_vfs_ioctl+0x173/0xde0 [53213.876216] ksys_ioctl+0x6b/0x80 [53213.876221] __x64_sys_ioctl+0x6a/0xb0 [53213.876225] do_syscall_64+0x95/0x2f0 [53213.876230] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [53213.876239] The buggy address belongs to the object at 8801063aa880 which belongs to the cache kmalloc-1024 of size 1024 [53213.876247] The buggy address is located 520 bytes inside of 1024-byte region [8801063aa880, 8801063aac80) [53213.876252] The buggy address belongs to the page: [53213.876258] page:ea000418ea00 count:1 mapcount:0 mapping: index:0x0 compound_mapcount: 0 [53213.876268] flags: 0x20008100(slab|head) [53213.876278] raw: 20008100 0001801c001c [53213.876284] raw: dead0100 dead0200 8803f3402c40 [53213.876288] page dumped because: kasan: bad access detected [53213.876294] Memory state around the buggy address: [53213.876300] 8801063aa980: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [53213.876305] 8801063aaa00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [53213.876310] >8801063aaa80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [53213.876313] ^ [532
Re: [Intel-gfx] [Freedreno] [PATCH 01/10] include: Move ascii85 functions from i915 to linux/ascii85.h
On Mon, Apr 16, 2018 at 10:52:59AM -0700, Eric Anholt wrote: > Chris Wilson writes: > > > Quoting Jordan Crouse (2018-04-05 23:06:53) > >> On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote: > >> > The i915 DRM driver very cleverly used ascii85 encoding for their > >> > GPU state file. Move the encode functions to a general header file to > >> > support other drivers that might be interested in the same > >> > functionality. > >> > >> In a previous version of this patch, Chris asked what tree I wanted this > >> applied > >> to, and the answer is: I'm not sure? I'm hoping that Rob will be cool with > >> picking the rest up for msm-next for 4.18 but I'm okay with putting this > >> particular patch wherever it is easiest for the maintainers. > > > > We are a bit late to sneak it into the 4.17 drm base via i915. I don't > > anticipate any problems (for i915) with this patch going in through > > msm-next, so happy to have it land there and percolate back to i915 3 > > months later. > > I'd love to have it in drm-misc-next, so I can build a similar hang dump > interface for vc5. But most importantly, I'd like to have it somewhere > soon. Maarten will likely send the first pull for drm-misc-next at the end of this week. So if you just push it there today, there's no hold-up for msm (or anyone else) really. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [RFC v4 19/25] drm/client: Finish the in-kernel client API
On Mon, Apr 16, 2018 at 05:58:23PM +0200, Noralf Trønnes wrote:
>
> Den 16.04.2018 10.27, skrev Daniel Vetter:
> > On Sat, Apr 14, 2018 at 01:53:12PM +0200, Noralf Trønnes wrote:
> > > The modesetting code is already present, this adds the rest of the API.
> > Mentioning the TODO in the commit message would be good. Helps readers
> > like me who have an attention span measured in seconds :-)
> >
> > Just commenting on the create_buffer leak here
> >
> > > +static struct drm_client_buffer *
> > > +drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32
> > > height, u32 format)
> > > +{
> > > + struct drm_mode_create_dumb dumb_args = { };
> > > + struct drm_prime_handle prime_args = { };
> > > + struct drm_client_buffer *buffer;
> > > + struct dma_buf *dma_buf;
> > > + void *vaddr;
> > > + int ret;
> > > +
> > > + buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
> > > + if (!buffer)
> > > + return ERR_PTR(-ENOMEM);
> > > +
> > > + buffer->client = client;
> > > + buffer->width = width;
> > > + buffer->height = height;
> > > + buffer->format = format;
> > > +
> > > + dumb_args.width = buffer->width;
> > > + dumb_args.height = buffer->height;
> > > + dumb_args.bpp = drm_format_plane_cpp(format, 0) * 8;
> > > + ret = drm_mode_create_dumb(client->dev, &dumb_args, client->file);
> > > + if (ret)
> > > + goto err_free;
> > > +
> > > + buffer->handle = dumb_args.handle;
> > > + buffer->pitch = dumb_args.pitch;
> > > + buffer->size = dumb_args.size;
> > > +
> > > + prime_args.handle = dumb_args.handle;
> > > + ret = drm_prime_handle_to_fd(client->dev, &prime_args, client->file);
> > > + if (ret)
> > > + goto err_delete;
> > > +
> > > + dma_buf = dma_buf_get(prime_args.fd);
> > > + if (IS_ERR(dma_buf)) {
> > > + ret = PTR_ERR(dma_buf);
> > > + goto err_delete;
> > > + }
> > > +
> > > + /*
> > > + * If called from a worker the dmabuf fd isn't closed and the ref
> > > + * doesn't drop to zero on free.
> > > + * If I use __close_fd() it's all fine, but that function is not
> > > exported.
> > > + *
> > > + * How do I get rid of this fd when in a worker/kernel thread?
> > > + * The fd isn't used beyond this function.
> > > + */
> > > +// WARN_ON(__close_fd(current->files, prime_args.fd));
> > Hm, this isn't 100% what I had in mind as the sequence for generic buffer
> > creation. Pseudo-code:
> >
> > ret = drm_mode_create_dumb(client->dev, &dumb_args, client->file);
> > if (ret)
> > goto err_free;
> >
> > gem_bo = drm_gem_object_lookup(client->file, dumb_args.handle);
> >
> > gives you _really_ directly the underlying gem_bo. Of course this doesn't
> > work for non-gem based driver, but reality is that (almost) all of them
> > are. And we will not accept any new drivers which aren't gem based. So
> > ignoring vmwgfx for this drm_client work is imo perfectly fine. We should
> > ofc keep the option in the fb helpers to use non-gem buffers (so that
> > vmwgfx could switch over from their own in-driver fbdev helpers). All we
> > need for that is to keep the fb_probe callback.
> >
> > Was there any other reason than vmwgfx for using prime buffers instead of
> > just directly using gem?
>
> The reason for using a prime buffer is that it gives me easy access to a
> dma_buf which I use to get the virtual address (dma_buf_vmap) and for
> mmap (dma_buf_mmap).
Ah yes, I missed that.
Wrt mmap, not sure we should use the dma-buf mmap or the dumb mmap. I
guess in the end it wont matter much really.
>
> Would this stripped down version of drm_gem_prime_handle_to_fd() work?
>
> struct dma_buf *drm_gem_to_dmabuf(struct drm_gem_object *obj)
> {
> struct dma_buf *dmabuf;
>
> mutex_lock(&obj->dev->object_name_lock);
> /* re-export the original imported object */
> if (obj->import_attach) {
> dmabuf = obj->import_attach->dmabuf;
> get_dma_buf(dmabuf);
> goto out;
> }
>
> if (obj->dma_buf) {
> dmabuf = obj->dma_buf;
> get_dma_buf(dmabuf);
> goto out;
> }
>
> dmabuf = export_and_register_object(obj->dev, obj, 0);
> out:
> mutex_unlock(&obj->dev->object_name_lock);
>
> return dmabuf;
> }
>
> Now I could do this:
>
> ret = drm_mode_create_dumb(dev, &dumb_args, file);
>
> obj = drm_gem_object_lookup(file, dumb_args.handle);
>
> dmabuf = drm_gem_to_dmabuf(obj);
>
> vaddr = dma_buf_vmap(dmabuf);
Nah, if we need the dma-buf anyway, I'd try to go directly from the handle
to the dma-buf. So roughly:
ret = drm_mode_create_dumb(dev, &dumb_args, file);
dma_buf = drm_gem_prime_handle_to_dmabuf(file, dumb_args.handle);
See my reply to the ioctl wrapper patch for details.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [RFC v4 00/25] drm: Add generic fbdev emulation
On Mon, Apr 16, 2018 at 08:49:23PM +0200, Noralf Trønnes wrote: > > Den 16.04.2018 10.21, skrev Daniel Vetter: > > On Sat, Apr 14, 2018 at 01:52:53PM +0200, Noralf Trønnes wrote: > > > This patchset explores the possibility of having generic fbdev emulation > > > in DRM for drivers that supports dumb buffers which they can export. An > > > API is added to support in-kernel clients in general. > > > > > > In this version I was able to reuse the modesetting code from > > > drm_fb_helper in the client API. This avoids code duplication, carries > > > over lessons learned and the modesetting code is bisectable. The > > > downside is that it takes +10 patches to rip drm_fb_helper in two, so > > > maybe it's not worth it wrt possible breakage and a challenging review. > > So my idea wasn't to rip the fbdev helper in half first (that's indeed a > > lot of work). But start out right away with using every piece of the > > drm_client infrastructure you're adding in the existing fbdev code. > > > > That way there's not a huge patch series which just adds code, with no > > users, but every step of the way and every addition is tested almost right > > away. That makes more gradual merging also easier. The things I have in > > mind here is the generic fb_probe, or the drm_client block/unblock masters > > and all that stuff. > > > > Then, once we've demonstrated all these auxiliary pieces necessary for > > drm_client.c work, we can cut the fb-helper in half and move the modeset > > code into the drm_client library. > > I agree. I wished for a way to cut this patchset in half, but I just > couldn't see how. I was tired of working on this, so I just put it out > hoping that you would provide some clarity. Which you did, thanks :-) Yeah, that's why I'm also ok with things as-is. I think we've reached agreement on the design and big picture now, which really is the important part. How exactly we get there (as long as it's gradual steps in bisectable patches) doesn't matter that much really. > So, I think I'll strip this down to just the buffer part of the client API > and use that in the generic fbdev emulation, and start converting some > drivers. Yes the buffer stuff is definitely the core parts. I think rolling out the master_block/unblock stuff after that would be really neat too, since it would fix a long-standing race in our fbdev emulation. > I'll pick up the rest of the client API when I'm done with moving tinydrm > over to vmalloc buffers and have added support for device unplug. Sounds like a good plan. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [RFC v4 21/25] drm/fb-helper: Add drm_fb_helper_fb_open/release()
On Mon, Apr 16, 2018 at 06:10:07PM +0200, Noralf Trønnes wrote:
>
> Den 16.04.2018 10.46, skrev Daniel Vetter:
> > On Sat, Apr 14, 2018 at 01:53:14PM +0200, Noralf Trønnes wrote:
> > > These helpers keep track of fbdev users and drm_driver.last_close will
> > > only restore fbdev when actually in use. Additionally the display is
> > > turned off when the last user is closing. fbcon is a user in this context.
> > >
> > > If struct fb_ops is defined in a library, fb_open() takes a ref on the
> > > library (fb_ops.owner) instead of the driver module. Fix that by ensuring
> > > that the driver module is pinned.
> > >
> > > The functions are not added to the DRM_FB_HELPER_DEFAULT_OPS() macro,
> > > because some of its users do set fb_open/release themselves.
> > >
> > > Signed-off-by: Noralf Trønnes
> > > ---
> > > drivers/gpu/drm/drm_fb_helper.c | 54
> > > -
> > > include/drm/drm_fb_helper.h | 29 ++
> > > 2 files changed, 82 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > b/drivers/gpu/drm/drm_fb_helper.c
> > > index 98e5bc92c9f2..b1124c08b1ed 100644
> > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > @@ -177,7 +177,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct
> > > drm_fb_helper *fb_helper)
> > > if (!drm_fbdev_emulation || !fb_helper)
> > > return -ENODEV;
> > > - if (READ_ONCE(fb_helper->deferred_setup))
> > > + if (READ_ONCE(fb_helper->deferred_setup) ||
> > > !READ_ONCE(fb_helper->open_count))
> > > return 0;
> > > mutex_lock(&fb_helper->lock);
> > > @@ -368,6 +368,7 @@ void drm_fb_helper_prepare(struct drm_device *dev,
> > > struct drm_fb_helper *helper,
> > > INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work);
> > > helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
> > > mutex_init(&helper->lock);
> > > + helper->open_count = 1;
> > > helper->funcs = funcs;
> > > helper->dev = dev;
> > > }
> > > @@ -620,6 +621,53 @@ int drm_fb_helper_defio_init(struct drm_fb_helper
> > > *fb_helper)
> > > }
> > > EXPORT_SYMBOL(drm_fb_helper_defio_init);
> > > +/**
> > > + * drm_fb_helper_fb_open - implementation for &fb_ops.fb_open
> > > + * @info: fbdev registered by the helper
> > > + * @user: 1=userspace, 0=fbcon
> > > + *
> > > + * Increase fbdev use count.
> > > + * If &fb_ops is wrapped in a library, pin the driver module.
> > > + */
> > > +int drm_fb_helper_fb_open(struct fb_info *info, int user)
> > > +{
> > > + struct drm_fb_helper *fb_helper = info->par;
> > > + struct drm_device *dev = fb_helper->dev;
> > > +
> > > + if (info->fbops->owner != dev->driver->fops->owner) {
> > > + if (!try_module_get(dev->driver->fops->owner))
> > > + return -ENODEV;
> > > + }
> > > +
> > > + fb_helper->open_count++;
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL(drm_fb_helper_fb_open);
> > > +
> > > +/**
> > > + * drm_fb_helper_fb_release - implementation for &fb_ops.fb_release
> > > + * @info: fbdev registered by the helper
> > > + * @user: 1=userspace, 0=fbcon
> > > + *
> > > + * Decrease fbdev use count and turn off if there are no users left.
> > > + * If &fb_ops is wrapped in a library, unpin the driver module.
> > > + */
> > > +int drm_fb_helper_fb_release(struct fb_info *info, int user)
> > > +{
> > > + struct drm_fb_helper *fb_helper = info->par;
> > > + struct drm_device *dev = fb_helper->dev;
> > > +
> > > + if (!(--fb_helper->open_count))
> > > + drm_fb_helper_dpms(info, DRM_MODE_DPMS_OFF);
> > > +
> > > + if (info->fbops->owner != dev->driver->fops->owner)
> > > + module_put(dev->driver->fops->owner);
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL(drm_fb_helper_fb_release);
> > > +
> > > /**
> > >* drm_fb_helper_sys_read - wrapper around fb_sys_read
> > >* @info: fb_info struct pointer
> > > @@ -1436,6 +1484,10 @@ static int drm_fb_helper_single_fb_probe(struct
> > > drm_fb_helper *fb_helper,
> > > if (ret < 0)
> > > return ret;
> > > + /* Block restore without users if we do track it */
> > > + if (fb_helper->fbdev->fbops->fb_open == drm_fb_helper_fb_open)
> > > + fb_helper->open_count = 0;
> > Since we kzcalloc, isn't this entirely redundant? Looks confusing to me at
> > least.
>
> I have to keep the existing restore behaviour for those that don't use
> drm_fb_helper_fb_open(). I do this by setting fb_helper->open_count = 1
> in drm_fb_helper_prepare(). I have tried to give a describtion in the
> @open_count docs.
Ah I missed that. I think having the fbops->fb_op == drm_fb_helper_fb_open
check in the restore function itself, plus a big comment explaining what
you're doing, would be clearer. So instead of always checking the
open_count, only check it when this helper is in use.
> Ofc I could have changed all drive
Re: [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On 04/17/2018 10:59 AM, Daniel Vetter wrote: On Mon, Apr 16, 2018 at 12:29:05PM -0700, Dongwon Kim wrote: Yeah, I definitely agree on the idea of expanding the use case to the general domain where dmabuf sharing is used. However, what you are targetting with proposed changes is identical to the core design of hyper_dmabuf. On top of this basic functionalities, hyper_dmabuf has driver level inter-domain communication, that is needed for dma-buf remote tracking (no fence forwarding though), event triggering and event handling, extra meta data exchange and hyper_dmabuf_id that represents grefs (grefs are shared implicitly on driver level) This really isn't a positive design aspect of hyperdmabuf imo. The core code in xen-zcopy (ignoring the ioctl side, which will be cleaned up) is very simple & clean. If there's a clear need later on we can extend that. But for now xen-zcopy seems to cover the basic use-case needs, so gets the job done. After we decided to remove DRM PRIME code from the zcopy driver I think we can extend the existing Xen drivers instead of introducing a new one: gntdev [1], [2] - to handle export/import of the dma-bufs to/from grefs balloon [3] - to allow allocating CMA buffers Also it is designed with frontend (common core framework) + backend (hyper visor specific comm and memory sharing) structure for portability. We just can't limit this feature to Xen because we want to use the same uapis not only for Xen but also other applicable hypervisor, like ACORN. See the discussion around udmabuf and the needs for kvm. I think trying to make an ioctl/uapi that works for multiple hypervisors is misguided - it likely won't work. On top of that the 2nd hypervisor you're aiming to support is ACRN. That's not even upstream yet, nor have I seen any patches proposing to land linux support for ACRN. Since it's not upstream, it doesn't really matter for upstream consideration. I'm doubting that ACRN will use the same grant references as xen, so the same uapi won't work on ACRN as on Xen anyway. So I am wondering we can start with this hyper_dmabuf then modify it for your use-case if needed and polish and fix any glitches if we want to to use this for all general dma-buf usecases. Imo xen-zcopy is a much more reasonable starting point for upstream, which can then be extended (if really proven to be necessary). Also, I still have one unresolved question regarding the export/import flow in both of hyper_dmabuf and xen-zcopy. @danvet: Would this flow (guest1->import existing dmabuf->share underlying pages->guest2->map shared pages->create/export dmabuf) be acceptable now? I think if you just look at the pages, and make sure you handle the sg_page == NULL case it's ok-ish. It's not great, but mostly it should work. The real trouble with hyperdmabuf was the forwarding of all these calls, instead of just passing around a list of grant references. -Daniel Regards, DW On Mon, Apr 16, 2018 at 05:33:46PM +0300, Oleksandr Andrushchenko wrote: Hello, all! After discussing xen-zcopy and hyper-dmabuf [1] approaches Even more context for the discussion [4], so Xen community can catch up it seems that xen-zcopy can be made not depend on DRM core any more and be dma-buf centric (which it in fact is). The DRM code was mostly there for dma-buf's FD import/export with DRM PRIME UAPI and with DRM use-cases in mind, but it comes out that if the proposed 2 IOCTLs (DRM_XEN_ZCOPY_DUMB_FROM_REFS and DRM_XEN_ZCOPY_DUMB_TO_REFS) are extended to also provide a file descriptor of the corresponding dma-buf, then PRIME stuff in the driver is not needed anymore. That being said, xen-zcopy can safely be detached from DRM and moved from drivers/gpu/drm/xen into drivers/xen/dma-buf-backend(?). This driver then becomes a universal way to turn any shared buffer between Dom0/DomD and DomU(s) into a dma-buf, e.g. one can create a dma-buf from any grant references or represent a dma-buf as grant-references for export. This way the driver can be used not only for DRM use-cases, but also for other use-cases which may require zero copying between domains. For example, the use-cases we are about to work in the nearest future will use V4L, e.g. we plan to support cameras, codecs etc. and all these will benefit from zero copying much. Potentially, even block/net devices may benefit, but this needs some evaluation. I would love to hear comments for authors of the hyper-dmabuf and Xen community, as well as DRI-Devel and other interested parties. Thank you, Oleksandr On 03/29/2018 04:19 PM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Hello! When using Xen PV DRM frontend driver then on backend side one will need to do copying of display buffers' contents (filled by the frontend's user-space) into buffers allocated at the backend side. Taking into account the size of display buffers and frames per seconds it may result in unneeded huge data bus occupation and performance loss. This he
Re: [PATCH libdrm] amdgpu: only add DRM_RDWR when kernel accept it
Hi guys,
CCing: dri-devel.
question to the usual suspects here: Does somebody of hand knows when
DRM_RDWR was added and how we can check reliable if it's available or not?
Using the version number of amdgpu for this sounds a bit questionable,
cause that reflects amdgpu functionality level and not DRM prime
functionality level.
Thanks,
Christian.
Am 17.04.2018 um 10:48 schrieb Qiang Yu:
For kernel < 4.6 drm_prime_handle_to_fd_ioctl won't
accept DRM_RDWR.
Sigend-off-by: Qiang Yu
---
amdgpu/amdgpu_bo.c | 8 ++--
amdgpu/amdgpu_internal.h | 2 ++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index 9e37b14..17f848f 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -229,6 +229,7 @@ int amdgpu_bo_export(amdgpu_bo_handle bo,
uint32_t *shared_handle)
{
int r;
+ uint32_t flags;
switch (type) {
case amdgpu_bo_handle_type_gem_flink_name:
@@ -246,8 +247,11 @@ int amdgpu_bo_export(amdgpu_bo_handle bo,
case amdgpu_bo_handle_type_dma_buf_fd:
amdgpu_add_handle_to_table(bo);
- return drmPrimeHandleToFD(bo->dev->fd, bo->handle,
- DRM_CLOEXEC | DRM_RDWR,
+ flags = DRM_CLOEXEC;
+ if (AMDGPU_VERSION(bo->dev->major_version, bo->dev->minor_version)
>=
+ AMDGPU_VERSION(3, 1))
+ flags |= DRM_RDWR;
+ return drmPrimeHandleToFD(bo->dev->fd, bo->handle, flags,
(int*)shared_handle);
}
return -EINVAL;
diff --git a/amdgpu/amdgpu_internal.h b/amdgpu/amdgpu_internal.h
index 99b8ce0..09f5036 100644
--- a/amdgpu/amdgpu_internal.h
+++ b/amdgpu/amdgpu_internal.h
@@ -39,6 +39,8 @@
#define ROUND_UP(x, y) x)-1) | __round_mask(x, y))+1)
#define ROUND_DOWN(x, y) ((x) & ~__round_mask(x, y))
+#define AMDGPU_VERSION(major, minor) ((major << 16) | minor)
+
#define AMDGPU_INVALID_VA_ADDRESS 0x
#define AMDGPU_NULL_SUBMIT_SEQ0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [RESEND PATCH 1/1] drm/i915/glk: Add MODULE_FIRMWARE for Geminilake
On Mon, 16 Apr 2018, "Srivatsa, Anusha" wrote: >>-Original Message- >>From: Jani Nikula [mailto:jani.nik...@linux.intel.com] >>Sent: Wednesday, April 11, 2018 5:27 AM >>To: Ian W MORRISON >>Cc: Vivi, Rodrigo ; Srivatsa, Anusha >>; Wajdeczko, Michal >>; Greg KH ; >>airl...@linux.ie; joonas.lahti...@linux.intel.com; >>linux-ker...@vger.kernel.org; >>sta...@vger.kernel.org; intel-...@lists.freedesktop.org; dri- >>de...@lists.freedesktop.org >>Subject: Re: [RESEND PATCH 1/1] drm/i915/glk: Add MODULE_FIRMWARE for >>Geminilake >> >>On Wed, 11 Apr 2018, Ian W MORRISON wrote: >>> >>> NAK on indiscriminate Cc: stable. There are zero guarantees that older kernels will work with whatever firmware you throw at them. >>> >>> I included 'Cc: stable' so the patch would get added to the v4.16 and >>> v4.15 kernels which I have tested with the patch. I found that earlier >>> kernels didn't support the 'linux-firmware' package required to get >>> wifi working on Intel's new Gemini Lake NUC. >> >>You realize that this patch should have nothing to do with wifi? >> >>Rodrigo, Anusha, if you think Cc: stable is appropriate, please indicate the >>specific >>versions of stable it is appropriate for. > > Hi Jani, > > The stable kernel version is 4.12 and beyond. > It is appropriate to add the CC: stable in my opinion Who tested the firmware with v4.12 and later? We only have the CI results against *current* drm-tip. We don't even know about v4.16. I'm not going to ack and take responsibility for the stable backports unless someone actually comes forward with credible Tested-bys. BR, Jani. > > Anusha >>BR, >>Jani. >> >>> PS. How is this a "RESEND"? I haven't seen this before. >>> >>> It is a 'RESEND' for that very reason. I initially sent the patch to >>> the same people as a similar patch >>> (https://patchwork.kernel.org/patch/10143637/) however after realising >>> this omitted required addresses I added them and resent the patch. >>> >>> Best regards, >>> Ian >> >>-- >>Jani Nikula, Intel Open Source Technology Center -- Jani Nikula, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 3/4] drm/tegra: plane: Add custom colorkey properties for older Tegra's
On Mon, Apr 16, 2018 at 03:16:27PM +0300, Dmitry Osipenko wrote:
> Colorkey'ing allows to draw on top of overlapping planes, like for example
> on top of a video plane. Older Tegra's have a limited colorkey'ing
> capability such that blending features are reduced when colorkey'ing is
> enabled. In particular dependent weighting isn't possible, meaning that
> cursors plane can't be displayed properly. In most cases it is more useful
> to display content on top of video overlay, sacrificing mouse cursor
> in the area of three planes intersection with colorkey mismatch. This
> patch adds a custom colorkey properties to primary plane and CRTC's of
> older Tegra's, allowing userspace like Opentegra Xorg driver to implement
> colorkey support for XVideo extension.
>
> Signed-off-by: Dmitry Osipenko
Since this is your own uapi, where's the userspace per
https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
And why wo we need a tegra-private colorkey property here? I thought
other's have been discussing this in the context of other drivers.
-Daniel
> ---
> drivers/gpu/drm/tegra/dc.c| 166 ++
> drivers/gpu/drm/tegra/dc.h| 18 +++-
> drivers/gpu/drm/tegra/plane.c | 40
> drivers/gpu/drm/tegra/plane.h | 9 +-
> 4 files changed, 231 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index a54eefea2513..b19e954a223f 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -172,6 +172,24 @@ static void tegra_plane_setup_blending_legacy(struct
> tegra_plane *plane)
>
> state = to_tegra_plane_state(plane->base.state);
>
> + /*
> + * Assuming default zPos window order, enable color keying for cases
> + * of overlapping with topping windows, excluding overlap with
> + * window B. Due to limited HW capabilities, this allows to draw
> + * primary plane on top of video overlay in areas where key isn't
> + * matching. Though window C will be completely transparent in a
> + * region of three windows intersection + key mismatch.
> + */
> + if (state->ckey0_enabled) {
> + background[0] |= BLEND_COLOR_KEY_0;
> + background[2] |= BLEND_COLOR_KEY_0;
> + }
> +
> + if (state->ckey1_enabled) {
> + background[0] |= BLEND_COLOR_KEY_1;
> + background[2] |= BLEND_COLOR_KEY_1;
> + }
> +
> if (state->opaque) {
> /*
>* Since custom fix-weight blending isn't utilized and weight
> @@ -729,6 +747,35 @@ static unsigned long
> tegra_plane_get_possible_crtcs(struct drm_device *drm)
> return 1 << drm->mode_config.num_crtc;
> }
>
> +static void tegra_plane_create_legacy_properties(struct tegra_plane *plane,
> + struct drm_device *drm)
> +{
> + plane->props.color_key0 = drm_property_create_bool(
> + drm, 0, "color key 0");
> + plane->props.color_key1 = drm_property_create_bool(
> + drm, 0, "color key 1");
> +
> + if (!plane->props.color_key0 ||
> + !plane->props.color_key1)
> + goto err_cleanup;
> +
> + drm_object_attach_property(&plane->base.base, plane->props.color_key0,
> +false);
> + drm_object_attach_property(&plane->base.base, plane->props.color_key1,
> +false);
> +
> + return;
> +
> +err_cleanup:
> + if (plane->props.color_key0)
> + drm_property_destroy(drm, plane->props.color_key0);
> +
> + if (plane->props.color_key1)
> + drm_property_destroy(drm, plane->props.color_key1);
> +
> + dev_err(plane->dc->dev, "failed to create legacy plane properties\n");
> +}
> +
> static struct drm_plane *tegra_primary_plane_create(struct drm_device *drm,
> struct tegra_dc *dc)
> {
> @@ -764,6 +811,9 @@ static struct drm_plane
> *tegra_primary_plane_create(struct drm_device *drm,
> drm_plane_helper_add(&plane->base, &tegra_plane_helper_funcs);
> drm_plane_create_zpos_property(&plane->base, plane->index, 0, 255);
>
> + if (dc->soc->legacy_blending)
> + tegra_plane_create_legacy_properties(plane, drm);
> +
> return &plane->base;
> }
>
> @@ -1153,6 +1203,8 @@ tegra_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
> copy->pclk = state->pclk;
> copy->div = state->div;
> copy->planes = state->planes;
> + copy->ckey0 = state->ckey0;
> + copy->ckey1 = state->ckey1;
>
> return ©->base;
> }
> @@ -1537,6 +1589,50 @@ static void tegra_dc_disable_vblank(struct drm_crtc
> *crtc)
> tegra_dc_writel(dc, value, DC_CMD_INT_MASK);
> }
>
> +static int tegra_crtc_atomic_set_property(struct drm_crtc *crtc,
> +
Re: [PATCH v1 4/4] drm/tegra: plane: Add custom CSC BLOB property
On Mon, Apr 16, 2018 at 03:16:28PM +0300, Dmitry Osipenko wrote:
> This new property allows userspace to apply custom color conversion
> coefficients per plane, making possible to utilize display controller
> for color adjustments of a video overlay.
>
> Signed-off-by: Dmitry Osipenko
Same here, this needs corresponding userspace:
https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
And again there's even more people who discussed extending the existing
color management support for crtcs to planes. I think we definitely want a
standard interface for this, not each driver doing their own thing.
-Daniel
> ---
> drivers/gpu/drm/tegra/dc.c| 86 +++
> drivers/gpu/drm/tegra/dc.h| 11 +
> drivers/gpu/drm/tegra/plane.c | 35 ++
> drivers/gpu/drm/tegra/plane.h | 5 ++
> include/uapi/drm/tegra_drm.h | 11 +
> 5 files changed, 139 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index b19e954a223f..24a1317871d4 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -435,15 +435,15 @@ static void tegra_dc_setup_window(struct tegra_plane
> *plane,
> value = WIN_ENABLE;
>
> if (yuv) {
> - /* setup default colorspace conversion coefficients */
> - tegra_plane_writel(plane, 0x00f0, DC_WIN_CSC_YOF);
> - tegra_plane_writel(plane, 0x012a, DC_WIN_CSC_KYRGB);
> - tegra_plane_writel(plane, 0x, DC_WIN_CSC_KUR);
> - tegra_plane_writel(plane, 0x0198, DC_WIN_CSC_KVR);
> - tegra_plane_writel(plane, 0x039b, DC_WIN_CSC_KUG);
> - tegra_plane_writel(plane, 0x032f, DC_WIN_CSC_KVG);
> - tegra_plane_writel(plane, 0x0204, DC_WIN_CSC_KUB);
> - tegra_plane_writel(plane, 0x, DC_WIN_CSC_KVB);
> + /* setup colorspace conversion coefficients */
> + tegra_plane_writel(plane, window->csc.yof, DC_WIN_CSC_YOF);
> + tegra_plane_writel(plane, window->csc.kyrgb, DC_WIN_CSC_KYRGB);
> + tegra_plane_writel(plane, window->csc.kur, DC_WIN_CSC_KUR);
> + tegra_plane_writel(plane, window->csc.kvr, DC_WIN_CSC_KVR);
> + tegra_plane_writel(plane, window->csc.kug, DC_WIN_CSC_KUG);
> + tegra_plane_writel(plane, window->csc.kvg, DC_WIN_CSC_KVG);
> + tegra_plane_writel(plane, window->csc.kub, DC_WIN_CSC_KUB);
> + tegra_plane_writel(plane, window->csc.kvb, DC_WIN_CSC_KVB);
>
> value |= CSC_ENABLE;
> } else if (window->bits_per_pixel < 24) {
> @@ -624,6 +624,7 @@ static void tegra_plane_atomic_update(struct drm_plane
> *plane,
> struct drm_framebuffer *fb = plane->state->fb;
> struct tegra_plane *p = to_tegra_plane(plane);
> struct tegra_dc_window window;
> + const struct drm_tegra_plane_csc_blob *csc;
> unsigned int i;
>
> /* rien ne va plus */
> @@ -665,6 +666,28 @@ static void tegra_plane_atomic_update(struct drm_plane
> *plane,
> window.stride[i] = fb->pitches[i];
> }
>
> + if (state->csc_blob) {
> + csc = state->csc_blob->data;
> +
> + window.csc.yof = csc->yof;
> + window.csc.kyrgb = csc->kyrgb;
> + window.csc.kur = csc->kur;
> + window.csc.kvr = csc->kvr;
> + window.csc.kug = csc->kug;
> + window.csc.kvg = csc->kvg;
> + window.csc.kub = csc->kub;
> + window.csc.kvb = csc->kvb;
> + } else {
> + window.csc.yof = 0x00f0;
> + window.csc.kyrgb = 0x012a;
> + window.csc.kur = 0x;
> + window.csc.kvr = 0x0198;
> + window.csc.kug = 0x039b;
> + window.csc.kvg = 0x032f;
> + window.csc.kub = 0x0204;
> + window.csc.kvb = 0x;
> + }
> +
> tegra_dc_setup_window(p, &window);
> }
>
> @@ -776,6 +799,42 @@ static void tegra_plane_create_legacy_properties(struct
> tegra_plane *plane,
> dev_err(plane->dc->dev, "failed to create legacy plane properties\n");
> }
>
> +static void tegra_plane_create_csc_property(struct tegra_plane *plane)
> +{
> + /* set default colorspace conversion coefficients to ITU-R BT.601 */
> + struct drm_tegra_plane_csc_blob csc_bt601 = {
> + .yof = 0x00f0,
> + .kyrgb = 0x012a,
> + .kur = 0x,
> + .kvr = 0x0198,
> + .kug = 0x039b,
> + .kvg = 0x032f,
> + .kub = 0x0204,
> + .kvb = 0x,
> + };
> + struct drm_property_blob *blob;
> +
> + blob = drm_property_create_blob(plane->base.dev, sizeof(csc_bt601),
> + &csc_bt601);
> + if (!blob) {
> + dev_err(plane->dc->dev, "failed to create CSC BLOB\n");
> + return;
> + }
> +
>
Re: [PATCH] drm/xen-front: Remove CMA support
On Tue, Apr 17, 2018 at 10:40:12AM +0300, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko > > Even if xen-front allocates its buffers from contiguous memory > those are still not contiguous in PA space, e.g. the buffer is only > contiguous in IPA space. > The only use-case for this mode was if xen-front is used to allocate > dumb buffers which later be used by some other driver requiring > contiguous memory, but there is no currently such a use-case or > it can be worked around with xen-front. Please also mention the nents confusion here, and the patch that fixes it. Or just outright take the commit message from my patch with all the details: drm/xen: Dissable CMA support It turns out this was only needed to paper over a bug in the CMA helpers, which was addressed in commit 998fb1a0f478b83492220ff79583bf9ad538bdd8 Author: Liviu Dudau Date: Fri Nov 10 13:33:10 2017 + drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1 Without this the following pipeline didn't work: domU: 1. xen-front allocates a non-contig buffer 2. creates grants out of it dom0: 3. converts the grants into a dma-buf. Since they're non-contig, the scatter-list is huge. 4. imports it into rcar-du, which requires dma-contig memory for scanout. -> On this given platform there's an IOMMU, so in theory this should work. But in practice this failed, because of the huge number of sg entries, even though the IOMMU driver mapped it all into a dma-contig range. With a guest-contig buffer allocated in step 1, this problem doesn't exist. But there's technically no reason to require guest-contig memory for xen buffer sharing using grants. With the commit message improved: Acked-by: Daniel Vetter > > Signed-off-by: Oleksandr Andrushchenko > Suggested-by: Daniel Vetter > --- > Documentation/gpu/xen-front.rst | 12 > drivers/gpu/drm/xen/Kconfig | 13 > drivers/gpu/drm/xen/Makefile| 9 +-- > drivers/gpu/drm/xen/xen_drm_front.c | 62 +++- > drivers/gpu/drm/xen/xen_drm_front.h | 42 ++- > drivers/gpu/drm/xen/xen_drm_front_gem.c | 12 +--- > drivers/gpu/drm/xen/xen_drm_front_gem.h | 3 - > drivers/gpu/drm/xen/xen_drm_front_gem_cma.c | 79 - > drivers/gpu/drm/xen/xen_drm_front_shbuf.c | 22 -- > drivers/gpu/drm/xen/xen_drm_front_shbuf.h | 8 --- > 10 files changed, 21 insertions(+), 241 deletions(-) > delete mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem_cma.c > > diff --git a/Documentation/gpu/xen-front.rst b/Documentation/gpu/xen-front.rst > index 009d942386c5..d988da7d1983 100644 > --- a/Documentation/gpu/xen-front.rst > +++ b/Documentation/gpu/xen-front.rst > @@ -18,18 +18,6 @@ Buffers allocated by the frontend driver > .. kernel-doc:: drivers/gpu/drm/xen/xen_drm_front.h > :doc: Buffers allocated by the frontend driver > > -With GEM CMA helpers > - > - > -.. kernel-doc:: drivers/gpu/drm/xen/xen_drm_front.h > - :doc: With GEM CMA helpers > - > -Without GEM CMA helpers > -~~~ > - > -.. kernel-doc:: drivers/gpu/drm/xen/xen_drm_front.h > - :doc: Without GEM CMA helpers > - > Buffers allocated by the backend > > > diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig > index 4f4abc91f3b6..4cca160782ab 100644 > --- a/drivers/gpu/drm/xen/Kconfig > +++ b/drivers/gpu/drm/xen/Kconfig > @@ -15,16 +15,3 @@ config DRM_XEN_FRONTEND > help > Choose this option if you want to enable a para-virtualized > frontend DRM/KMS driver for Xen guest OSes. > - > -config DRM_XEN_FRONTEND_CMA > - bool "Use DRM CMA to allocate dumb buffers" > - depends on DRM_XEN_FRONTEND > - select DRM_KMS_CMA_HELPER > - select DRM_GEM_CMA_HELPER > - help > - Use DRM CMA helpers to allocate display buffers. > - This is useful for the use-cases when guest driver needs to > - share or export buffers to other drivers which only expect > - contiguous buffers. > - Note: in this mode driver cannot use buffers allocated > - by the backend. > diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile > index 352730dc6c13..712afff5ffc3 100644 > --- a/drivers/gpu/drm/xen/Makefile > +++ b/drivers/gpu/drm/xen/Makefile > @@ -5,12 +5,7 @@ drm_xen_front-objs := xen_drm_front.o \ > xen_drm_front_conn.o \ > xen_drm_front_evtchnl.o \ > xen_drm_front_shbuf.o \ > - xen_drm_front_cfg.o > - > -ifeq ($(CONFIG_DRM_XEN_FRONTEND_CMA),y) > - drm_xen_front-objs += xen_drm_front_gem_cma.o > -else > - drm_xen_front-objs += xen_drm_front_gem.o > -endif > + xen_drm_front_cfg.o \ > + xen_drm
Re: [PATCH] drm/xen-front: Remove CMA support
On 04/17/2018 12:04 PM, Daniel Vetter wrote: On Tue, Apr 17, 2018 at 10:40:12AM +0300, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Even if xen-front allocates its buffers from contiguous memory those are still not contiguous in PA space, e.g. the buffer is only contiguous in IPA space. The only use-case for this mode was if xen-front is used to allocate dumb buffers which later be used by some other driver requiring contiguous memory, but there is no currently such a use-case or it can be worked around with xen-front. Please also mention the nents confusion here, and the patch that fixes it. Or just outright take the commit message from my patch with all the details: ok, if you don't mind then I'll use your commit message entirely drm/xen: Dissable CMA support It turns out this was only needed to paper over a bug in the CMA helpers, which was addressed in commit 998fb1a0f478b83492220ff79583bf9ad538bdd8 Author: Liviu Dudau Date: Fri Nov 10 13:33:10 2017 + drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1 Without this the following pipeline didn't work: domU: 1. xen-front allocates a non-contig buffer 2. creates grants out of it dom0: 3. converts the grants into a dma-buf. Since they're non-contig, the scatter-list is huge. 4. imports it into rcar-du, which requires dma-contig memory for scanout. -> On this given platform there's an IOMMU, so in theory this should work. But in practice this failed, because of the huge number of sg entries, even though the IOMMU driver mapped it all into a dma-contig range. With a guest-contig buffer allocated in step 1, this problem doesn't exist. But there's technically no reason to require guest-contig memory for xen buffer sharing using grants. With the commit message improved: Acked-by: Daniel Vetter Thank you, I'll wait for a day and apply to drm-misc-next if this is ok Signed-off-by: Oleksandr Andrushchenko Suggested-by: Daniel Vetter --- Documentation/gpu/xen-front.rst | 12 drivers/gpu/drm/xen/Kconfig | 13 drivers/gpu/drm/xen/Makefile| 9 +-- drivers/gpu/drm/xen/xen_drm_front.c | 62 +++- drivers/gpu/drm/xen/xen_drm_front.h | 42 ++- drivers/gpu/drm/xen/xen_drm_front_gem.c | 12 +--- drivers/gpu/drm/xen/xen_drm_front_gem.h | 3 - drivers/gpu/drm/xen/xen_drm_front_gem_cma.c | 79 - drivers/gpu/drm/xen/xen_drm_front_shbuf.c | 22 -- drivers/gpu/drm/xen/xen_drm_front_shbuf.h | 8 --- 10 files changed, 21 insertions(+), 241 deletions(-) delete mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem_cma.c diff --git a/Documentation/gpu/xen-front.rst b/Documentation/gpu/xen-front.rst index 009d942386c5..d988da7d1983 100644 --- a/Documentation/gpu/xen-front.rst +++ b/Documentation/gpu/xen-front.rst @@ -18,18 +18,6 @@ Buffers allocated by the frontend driver .. kernel-doc:: drivers/gpu/drm/xen/xen_drm_front.h :doc: Buffers allocated by the frontend driver -With GEM CMA helpers - - -.. kernel-doc:: drivers/gpu/drm/xen/xen_drm_front.h - :doc: With GEM CMA helpers - -Without GEM CMA helpers -~~~ - -.. kernel-doc:: drivers/gpu/drm/xen/xen_drm_front.h - :doc: Without GEM CMA helpers - Buffers allocated by the backend diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig index 4f4abc91f3b6..4cca160782ab 100644 --- a/drivers/gpu/drm/xen/Kconfig +++ b/drivers/gpu/drm/xen/Kconfig @@ -15,16 +15,3 @@ config DRM_XEN_FRONTEND help Choose this option if you want to enable a para-virtualized frontend DRM/KMS driver for Xen guest OSes. - -config DRM_XEN_FRONTEND_CMA - bool "Use DRM CMA to allocate dumb buffers" - depends on DRM_XEN_FRONTEND - select DRM_KMS_CMA_HELPER - select DRM_GEM_CMA_HELPER - help - Use DRM CMA helpers to allocate display buffers. - This is useful for the use-cases when guest driver needs to - share or export buffers to other drivers which only expect - contiguous buffers. - Note: in this mode driver cannot use buffers allocated - by the backend. diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile index 352730dc6c13..712afff5ffc3 100644 --- a/drivers/gpu/drm/xen/Makefile +++ b/drivers/gpu/drm/xen/Makefile @@ -5,12 +5,7 @@ drm_xen_front-objs := xen_drm_front.o \ xen_drm_front_conn.o \ xen_drm_front_evtchnl.o \ xen_drm_front_shbuf.o \ - xen_drm_front_cfg.o - -ifeq ($(CONFIG_DRM_XEN_FRONTEND_CMA),y) - drm_xen_front-objs += xen_drm_front_gem_cma.o -else - drm_xen_f
RE: [Intel-gfx] [PATCH v1 2/2] i915: content-type property for HDMI connector
Hi Sean,
Thank you for comments!
Could you please clarify a bit more here, as I've just
started recently working on drm side, so I took an
aspect ratio property as an example.
> @@ -491,6 +491,8 @@ static void intel_hdmi_set_avi_infoframe(struct
> drm_encoder *encoder,
>
> intel_hdmi->rgb_quant_range_selectable,
> is_hdmi2_sink);
>
> + frame.avi.content_type = connector->state->content_type;
> +
> /* TODO: handle pixel repetition for YCBCR420 outputs */
> intel_write_infoframe(encoder, crtc_state, &frame); } @@ -2065,7
> +2067,9 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct
> drm_connector *c
> intel_attach_force_audio_property(connector);
> intel_attach_broadcast_rgb_property(connector);
> intel_attach_aspect_ratio_property(connector);
> + intel_attach_content_type_property(connector);
> connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> + connector->state->content_type = HDMI_CONTENT_TYPE_GRAPHICS;
>This is redudant, the attach function already sets this.
As you can see aspect ratio is set exactly same way,
which is also an HDMI avi info frame property.
Also there are actually two different enums: HDMI_CONTENT_TYPE_*
and DRM_MODE_CONTENT_TYPE_* i.e:
there are one in drm_connector.c:
static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_NONE, "Automatic" },
{ DRM_MODE_PICTURE_ASPECT_4_3, "4:3" },
{ DRM_MODE_PICTURE_ASPECT_16_9, "16:9" },
};
so I added
static const struct drm_prop_enum_list drm_content_type_enum_list[] = {
{ DRM_MODE_CONTENT_TYPE_GRAPHICS, "GRAPHICS" },
{ DRM_MODE_CONTENT_TYPE_PHOTO, "PHOTO" },
{ DRM_MODE_CONTENT_TYPE_CINEMA, "CINEMA" },
{ DRM_MODE_CONTENT_TYPE_GAME, "GAME" },
};
and the one in linux/hdmi.h:
enum hdmi_picture_aspect {
HDMI_PICTURE_ASPECT_NONE,
HDMI_PICTURE_ASPECT_4_3,
HDMI_PICTURE_ASPECT_16_9,
HDMI_PICTURE_ASPECT_64_27,
HDMI_PICTURE_ASPECT_256_135,
HDMI_PICTURE_ASPECT_RESERVED,
};
enum hdmi_content_type {
HDMI_CONTENT_TYPE_GRAPHICS,
HDMI_CONTENT_TYPE_PHOTO,
HDMI_CONTENT_TYPE_CINEMA,
HDMI_CONTENT_TYPE_GAME,
};
For some reason the latter enums are used in drm_connector_state, but not
the drm_content_type_enum_list(those are actually defined values which simply
match):
From drm_connector.c:
/**
* @picture_aspect_ratio: Connector property to control the
* HDMI infoframe aspect ratio setting.
*
* The %DRM_MODE_PICTURE_ASPECT_\* values much match the
* values for &enum hdmi_picture_aspect
*/
enum hdmi_picture_aspect picture_aspect_ratio;
/**
* @content_type: Connector property to control the
* HDMI infoframe content type setting.
*
* The %DRM_MODE_CONTENT_TYPE_\* values much match the
* values for &enum hdmi_content_type
*/
enum hdmi_content_type content_type;
That's why I did it exactly as it is done with aspect ratio. Just want to
clarify,
as I was assuming this was done for reason.
> }
>
> /*
> diff --git a/drivers/gpu/drm/i915/intel_modes.c
> b/drivers/gpu/drm/i915/intel_modes.c
> index b39846613e3c..232811ab71a3 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -133,3 +133,13 @@ intel_attach_aspect_ratio_property(struct drm_connector
> *connector)
> connector->dev->mode_config.aspect_ratio_property,
> DRM_MODE_PICTURE_ASPECT_NONE);
> }
> +
> +void
> +intel_attach_content_type_property(struct drm_connector *connector) {
> + if (!drm_mode_create_content_type_property(connector->dev))
> + drm_object_attach_property(&connector->base,
> + connector->dev->mode_config.content_type_property,
> + DRM_MODE_CONTENT_TYPE_GRAPHICS);
> +}
>I think the "in" thing to do is to add this helper to the core, since this is
>a core property.
Could you please explain a bit more, what do you mean by core here?
I just thought it is one of HDMI infoframe properties, as stated in spec:
https://www.hdmi.org/manufacturer/hdmi_1_4/content_type.aspx
Best Regards,
Lisovskiy Stanislav
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105425] 3D & games produce periodic GPU crashes (Radeon R7 370)
https://bugs.freedesktop.org/show_bug.cgi?id=105425 --- Comment #31 from i...@yahoo.com --- (In reply to MirceaKitsune from comment #29) > For the first time ever, I might finally have some very good news on this > issue! It will take several more days to confirm, then possibly another > month to pinpoint the exact option responsible. However it's possible I may > have found something that finally gets rid of the crash. > > The issue appears to go away when playing Xonotic with those parameters: > > export LIBGL_DEBUG=true LIBGL_NO_DRAWARRAYS=true LIBGL_DRI3_DISABLE=true > MESA_DEBUG=true MESA_NO_ASM=true MESA_NO_MMX=true MESA_NO_3DNOW=true > MESA_NO_SSE=true MESA_NO_ERROR=true MESA_GLSL_CACHE_DISABLE=true > MESA_NO_MINMAX_CACHE=true RADEON_NO_TCL=true DRAW_NO_FSE=true DRAW_USE_LLVM=0 > > I additionally disabled the cvar "r_shadows 2" which I forgot I had on for a > while now, as it enabled a shadowing system that might have itself been the > culprit. > > With these two changes, I was able to clock up to 120 minutes of continuous > gameplay last night, followed by an outstanding 200 minutes today! That's > over 2 respectively 3 hours with no system freeze whatsoever. I need to > repeat this test several times to be 100% sure there's not still some > obscure chance of it happening, but in any case there is definitely a major > difference visible. "MESA_NO_ASM=true" supersedes the other "MESA_NO_MMX=true MESA_NO_3DNOW=true MESA_NO_SSE=true", so you don't need to make combinations with all of them. Also I don't see you testing `export mesa_glthread=false`. Race conditions are one of the hardest bugs to catch and reproduce. If you think that 'r_shadow' could quickly and "reliably" trigger a hang, then I would ask you to focus on it first. 1. Read about sysrq and make sure you have it enabled in the kernel and that it works. Make sure you have text console, as it might need it. 2. Enable back "r_shadows 2" 3. Use apitrace to capture a hang, while playing the game. 4. Try to reboot gracefully, using sysrq to sync and reboot, or get in text console and restart. 5. Test if the recorded trace could reproduce the crash reliably. If the trace seems complete and it cannot reproduce the bug, then maybe it does capture everything, but the bug is not simple infinite loop in the shader. (These seem to be common cause of hangs). If the bug can be reliably reproduced, it will be fixed. Good luck. -- 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 101928] GRID via Wine crashes mid-race
https://bugs.freedesktop.org/show_bug.cgi?id=101928 --- Comment #2 from i...@yahoo.com --- The Ixit Nine issue has been closed. The issue seems to be caused by running out of memory, due to a memory leak in wine builtin OpenAL wrapper. This could be workarounded by using native OpenAL32.dll or disabling sound altogether. Please retest. -- 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 105760] [4.17-rc1] RIP: smu7_populate_single_firmware_entry.isra.6+0x57/0xc0 [amdgpu] RSP: ffffa17901efb930
https://bugs.freedesktop.org/show_bug.cgi?id=105760 --- Comment #6 from taij...@posteo.de --- If I wanted to try to embed amdgpu in the kernel for testing, how would I even go about doing that? Simply editing my config file from =m to =y does not seem to do anything. -- 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/rockchip: vop: fixup linebuffer mode calc error
When video width is bigger than 3840 the linebuffer mode should be LB_YUV_3840X5. Signed-off-by: Sandy Huang --- drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h index 56bbd2e..3e7501b 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h @@ -332,7 +332,7 @@ static inline int scl_vop_cal_lb_mode(int width, bool is_yuv) if (width > 2560) lb_mode = LB_RGB_3840X2; - else if (width > 1920) + else if (!is_yuv && width > 1920) lb_mode = LB_RGB_2560X4; else if (!is_yuv) lb_mode = LB_RGB_1920X5; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v4 23/25] drm: Add DRM device registered notifier
On Sat, Apr 14, 2018 at 01:53:16PM +0200, Noralf Trønnes wrote:
> Add a notifier that fires when a new DRM device is registered.
> This can be used by the bootsplash client to connect to all devices.
>
> Signed-off-by: Noralf Trønnes
So I freaked out temporarily about your usage of notifiers here. But I
think this one of the few cases where using notifiers is actually
perfectly fine. Still not sure we really want that, instead of just
hard-coding calls to the various register functions directly (and using a
dummy no-op replacement if that part isn't enabled in Kconfig).
-Daniel
> ---
> drivers/gpu/drm/drm_drv.c | 32
> include/drm/drm_drv.h | 4
> 2 files changed, 36 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 6f21bafb29be..e42ce320ad07 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -31,6 +31,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -79,6 +80,8 @@ static struct dentry *drm_debugfs_root;
>
> DEFINE_STATIC_SRCU(drm_unplug_srcu);
>
> +static BLOCKING_NOTIFIER_HEAD(drm_dev_notifier);
> +
> /*
> * DRM Minors
> * A DRM device can provide several char-dev interfaces on the DRM-Major.
> Each
> @@ -837,6 +840,8 @@ int drm_dev_register(struct drm_device *dev, unsigned
> long flags)
>dev->dev ? dev_name(dev->dev) : "virtual device",
>dev->primary->index);
>
> + blocking_notifier_call_chain(&drm_dev_notifier, 0, dev);
> +
> goto out_unlock;
>
> err_minors:
> @@ -894,6 +899,33 @@ void drm_dev_unregister(struct drm_device *dev)
> }
> EXPORT_SYMBOL(drm_dev_unregister);
>
> +/**
> + * drm_dev_register_notifier - Register a notifier for new DRM devices
> + * @nb: Notifier block
> + *
> + * Register a notifier that fires when a new &drm_device is registered.
> + *
> + * Note:
> + * Users of this function has to be linked into drm.ko. This is done to make
> + * life simple avoiding tricky race situations.
> + */
> +void drm_dev_register_notifier(struct notifier_block *nb)
> +{
> + /* Currently this can't fail, but catch it in case this changes */
> + WARN_ON(blocking_notifier_chain_register(&drm_dev_notifier, nb));
> +}
> +
> +/**
> + * drm_dev_unregister_notifier - Unregister DRM device notifier
> + * @nb: Notifier block
> + *
> + * This is a no-op if the notifier isn't registered.
> + */
> +void drm_dev_unregister_notifier(struct notifier_block *nb)
> +{
> + blocking_notifier_chain_unregister(&drm_dev_notifier, nb);
> +}
> +
> /**
> * drm_dev_set_unique - Set the unique name of a DRM device
> * @dev: device of which to set the unique name
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 13356e6fd40c..5e6c6ed0d59d 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -40,6 +40,7 @@ struct drm_minor;
> struct dma_buf_attachment;
> struct drm_display_mode;
> struct drm_mode_create_dumb;
> +struct notifier_block;
> struct drm_printer;
>
> /* driver capabilities and requirements mask */
> @@ -641,6 +642,9 @@ struct drm_device *drm_dev_alloc(struct drm_driver
> *driver,
> int drm_dev_register(struct drm_device *dev, unsigned long flags);
> void drm_dev_unregister(struct drm_device *dev);
>
> +void drm_dev_register_notifier(struct notifier_block *nb);
> +void drm_dev_unregister_notifier(struct notifier_block *nb);
> +
> void drm_dev_get(struct drm_device *dev);
> void drm_dev_put(struct drm_device *dev);
> void drm_dev_unref(struct drm_device *dev);
> --
> 2.15.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/rockchip: vop: fixup linebuffer mode calc error
Hi Sandy, Am Dienstag, 17. April 2018, 12:15:07 CEST schrieb Sandy Huang: > When video width is bigger than 3840 the linebuffer mode > should be LB_YUV_3840X5. Can you explain that a bit better? I.e. when looking at the code, the very first check is width > 2560. So it seems your change targets some YUV mode with width > 3840 which should be mentioned in the commit message, so people like me don't scratch their head in confusion ;-) Similarly that check is actually width > 1280 to set LB_YUV_3840X5, so I guess you're actually wanting any YUV mode bigger than 1280px should use LB_YUV_3840X5? Heiko > Signed-off-by: Sandy Huang > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h > index 56bbd2e..3e7501b 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h > @@ -332,7 +332,7 @@ static inline int scl_vop_cal_lb_mode(int width, bool > is_yuv) > > if (width > 2560) > lb_mode = LB_RGB_3840X2; > - else if (width > 1920) > + else if (!is_yuv && width > 1920) > lb_mode = LB_RGB_2560X4; > else if (!is_yuv) > lb_mode = LB_RGB_1920X5; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/stm: ltdc: fix deferred endpoint management
When a driver related to one of the endpoints is deferred due to probe dependencies (i2c, spi...) but the other one is ready, ltdc probe continues and the deferred driver will never be probed again. The fix consists in waiting for all deferred endpoints before continuing the ltdc probe. Signed-off-by: Philippe Cornu --- drivers/gpu/drm/stm/ltdc.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index e3121d9e4230..014cef8cef37 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -987,14 +987,13 @@ int ltdc_load(struct drm_device *ddev) &bridge[i]); /* -* If at least one endpoint is ready, continue probing, -* else if at least one endpoint is -EPROBE_DEFER and -* there is no previous ready endpoints, defer probing. +* If at least one endpoint is -EPROBE_DEFER, defer probing, +* else if at least one endpoint is ready, continue probing. */ - if (!ret) + if (ret == -EPROBE_DEFER) + return ret; + else if (!ret) endpoint_not_ready = 0; - else if (ret == -EPROBE_DEFER && endpoint_not_ready) - endpoint_not_ready = -EPROBE_DEFER; } if (endpoint_not_ready) -- 2.15.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/stm: ltdc: add mode_valid()
Add mode_valid() function to filter modes according to available
pll clock values and "preferred" modes. It is particularly
useful for hdmi modes that require precise pixel clocks.
Note that "preferred" modes are always accepted:
- this is important for panels because panel clock tolerances are
bigger than hdmi ones and there is no reason to not accept them
(the fps may vary a little but it is not a problem).
- the hdmi preferred mode will be accepted too, but userland will
be able to use others hdmi "valid" modes if necessary.
Signed-off-by: Philippe Cornu
---
drivers/gpu/drm/stm/ltdc.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 014cef8cef37..616191fe98ae 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -445,6 +445,43 @@ static void ltdc_crtc_atomic_disable(struct drm_crtc *crtc,
reg_set(ldev->regs, LTDC_SRCR, SRCR_IMR);
}
+#define CLK_TOLERANCE_HZ 50
+
+static enum drm_mode_status
+ltdc_crtc_mode_valid(struct drm_crtc *crtc,
+const struct drm_display_mode *mode)
+{
+ struct ltdc_device *ldev = crtc_to_ltdc(crtc);
+ int target = mode->clock * 1000;
+ int target_min = target - CLK_TOLERANCE_HZ;
+ int target_max = target + CLK_TOLERANCE_HZ;
+ int result;
+
+ /*
+* Accept all "preferred" modes:
+* - this is important for panels because panel clock tolerances are
+* bigger than hdmi ones and there is no reason to not accept them
+* (the fps may vary a little but it is not a problem).
+* - the hdmi preferred mode will be accepted too, but userland will
+* be able to use others hdmi "valid" modes if necessary.
+*/
+ if (mode->type & DRM_MODE_TYPE_PREFERRED)
+ return MODE_OK;
+
+ result = clk_round_rate(ldev->pixel_clk, target);
+
+ DRM_DEBUG_DRIVER("clk rate target %d, available %d\n", target, result);
+
+ /*
+* Filter modes according to the clock value, particularly useful for
+* hdmi modes that require precise pixel clocks.
+*/
+ if (result < target_min || result > target_max)
+ return MODE_CLOCK_RANGE;
+
+ return MODE_OK;
+}
+
static bool ltdc_crtc_mode_fixup(struct drm_crtc *crtc,
const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
@@ -559,6 +596,7 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc,
}
static const struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = {
+ .mode_valid = ltdc_crtc_mode_valid,
.mode_fixup = ltdc_crtc_mode_fixup,
.mode_set_nofb = ltdc_crtc_mode_set_nofb,
.atomic_flush = ltdc_crtc_atomic_flush,
--
2.15.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2 v2] drm/pl111: Enable device-specific assigned memory
On Sun, Apr 8, 2018 at 3:08 AM, Eric Anholt wrote:
>> if (of_property_read_u32(dev->of_node, "max-memory-bandwidth",
>>&priv->memory_bw)) {
>> dev_info(dev, "no max memory bandwidth specified, assume
>> unlimited\n");
>> @@ -275,7 +280,8 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>> priv->regs = devm_ioremap_resource(dev, &amba_dev->res);
>> if (IS_ERR(priv->regs)) {
>> dev_err(dev, "%s failed mmio\n", __func__);
>> - return PTR_ERR(priv->regs);
>> + ret = PTR_ERR(priv->regs);
>> + goto mem_rel;
>> }
>
> Shouldn't this error path be jumping to dev_unref, as well? We're
> trying to match drm_dev_alloc(), right?
OK I fixed.
> I'm still a little bothered that we're allowing imports to pl111 of CMA
> buffers that we can't scan out. Could we just refuse all
> .gem_prime_import*() on this platform?
I am sorry but I do not understand how CMA, dmabuf and GEM play
together to decode this and figure out what to do.
Do you mean that if we find device assigned memory, i.e. that there
is this special memory which is all the controller can scan out,
I should just implement .gem_prime_impport() instead of the
currently assigned drm_gem_prime_import() to something just
returning return ERR_PTR(-EINVAL); so it always fails?
What about .gem_prime_import_sg_table()? Exporting should
work fine since the memory is always readable by CPU.
Yours,
Linus Walleij
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2 v2] drm/pl111: Support the Versatile Express
On Tue, Apr 10, 2018 at 11:57 AM, Liviu Dudau wrote: > I need to do a bit of archeology to get my Versatile Express back > to working order in order to test your patches, which might not > happen this week or so, but I have some comments on your approach > which I would like clarification on. OK no hurries :) > On Fri, Apr 06, 2018 at 04:19:34PM +0200, Linus Walleij wrote: >> >> The Versatile Express uses a special configuration controller >> deeply embedded in the system motherboard FPGA to multiplex the >> two to three (!) CLCD instances out to the single SiI9022 >> bridge. > > That configuration controller is already supported by the > drivers/misc/vexpress-syscfg.c driver and the muxfpga node is present > in arch/arm/boot/dts/vexpress-v2m-rs1.dtsi (and vexpress-v2m.dtsi for > older CoreTiles that don't use the RS1 memory map). Yes the code is using that driver through the public interface in , so it should be fine. >> Set up an extra file with the logic to probe to the FPGA mux >> register on the system controller bus, then parse the memory >> range argument to figure out what path the CLCD signal is >> actually taking, and set up the mux accordingly. > > I'm not sure you need all this extra code, I think the commit message may be confusing I will try to reword it. > I would have thought that > all you want is to use the regmap that you already have in order to > switch the MUX to the correct input. The MUXFPGA in the DT is already > setup to use function 7 (which is the function you need for FPGA), you > just need a property in the PL111 node to tell you which "site" you are > on (Motherboard, Daughterboard 1 or 2) and use the relevant value when > setting the regmap. > > Info on what the correct values are are here: > http://infocenter.arm.com/help/topic/com.arm.doc.dui0447j/CACDEFGH.html#CACCGJFF Yes this is what I'm using, I am manipulating SYS_CFG_MUXFPGA through the interface exposed in . >> If there is a CLCD instance on the core tile (the daughterboard >> with the CPUs fitted) then that CLCD instance will take >> precedence since it can address all memory. > > What does "take precedence" mean here? You mean prefer to switch the MUX > to that source instead of using the MB? That should always be the > default choice, as the MB CLCD is mostly for boot up info being > displayed and it is known to have horrendous performance. Yes, so that is what I am trying to do. If there is a CLCD on the core tile (daughterboard as it its called in the manual), that one should take precedence. Unfortunately there is just one single vexpress core tile in the upstream kernel that define a CLCD controller, the CA9 (4xA9) that I am using. All the others just use the MB CLCD. I am thinking there is some never finished DTS upstreaming here that ought to happen so we use the core tile CLCD on some other boards as well. Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/3] drm/vc4: Remove the need for the GPU-subsystem DT node.
On Mon, Apr 16, 2018 at 5:52 PM, Eric Anholt wrote:
> The GPU subsystem node was a workaround to have a central device to
> bind V3D and display to. Following the lead of 246774d17fc0
> ("drm/etnaviv: remove the need for a gpu-subsystem DT node"), remove
> the subsystem node usage and just create a platform device for the DRM
> device to attach to if any of the subsystem devices are present.
>
> v2: Simplify the DT walking code.
> v3: Always put the node.
>
> Signed-off-by: Eric Anholt
> ---
> .../bindings/display/brcm,bcm-vc4.txt | 7 -
> drivers/gpu/drm/vc4/vc4_drv.c | 28 +--
> drivers/gpu/drm/vc4/vc4_hvs.c | 1 +
> drivers/gpu/drm/vc4/vc4_v3d.c | 1 +
> 4 files changed, 22 insertions(+), 15 deletions(-)
Reviewed-by: Rob Herring
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2 v2] drm/pl111: Support the Versatile Express
On 17/04/18 13:32, Linus Walleij wrote: [...] Unfortunately there is just one single vexpress core tile in the upstream kernel that define a CLCD controller, the CA9 (4xA9) that I am using. All the others just use the MB CLCD. I am thinking there is some never finished DTS upstreaming here that ought to happen so we use the core tile CLCD on some other boards as well. Barring custom FPGA images, I think V2P-CA9 *is* the only VExpress tile implementing PL11x; all the more recent test chips had HDLCD instead. Robin. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 02/18] drm_hwcomposer: vsyncworker: Fix deadlock on exit path
Hi Sean,
On Mon, Apr 16, 2018 at 03:25:42PM -0400, Sean Paul wrote:
> On Wed, Apr 11, 2018 at 04:22:13PM +0100, Alexandru Gheorghe wrote:
> > vsyncworker::Routine assumes that when -EINTR is returned by
> > WaitForSignalOrExitLocked the lock as been released, which is not
> > true, so it hangs if a vsyncworker is never enabled and Exit is
> > called.
> >
> > There are two code paths in WaitForSignalOrExitLocked that return
> > -EINTR, one releases the lock the other doesn't.
> > Looking at the clients of WaitForSignalOrExitLocked all assume the lock
> > is still held, except vsyncworker::Routine.
> > So, the proper fix needs two changes:
> > - Make WaitForSignalOrExitLocked consistent and always hold the lock
> > when exiting.
> > - Release lock in vsynworker::Routine on all code paths.
> >
> > Signed-off-by: Alexandru Gheorghe
> > ---
> > vsyncworker.cpp | 1 +
> > worker.cpp | 6 +++---
> > 2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/vsyncworker.cpp b/vsyncworker.cpp
> > index 3bfe4be..7c0c741 100644
> > --- a/vsyncworker.cpp
> > +++ b/vsyncworker.cpp
> > @@ -120,6 +120,7 @@ void VSyncWorker::Routine() {
> >if (!enabled_) {
> > ret = WaitForSignalOrExitLocked();
> > if (ret == -EINTR) {
> > + Unlock();
> >return;
> > }
> >}
> > diff --git a/worker.cpp b/worker.cpp
> > index da6c580..5b351e0 100644
> > --- a/worker.cpp
> > +++ b/worker.cpp
> > @@ -66,13 +66,13 @@ int Worker::WaitForSignalOrExitLocked(int64_t
> > max_nanoseconds) {
> > ret = -ETIMEDOUT;
> >}
> >
> > + // release leaves mutex locked when going out of scope
> > + lk.release();
> > +
> >// exit takes precedence on timeout
> >if (should_exit())
> > ret = -EINTR;
> >
> > - // release leaves mutex locked when going out of scope
> > - lk.release();
> > -
>
> I'm not sure why this chunk makes a difference? If the above was
> "return -EINTR;" it would, but it's just assigning ret.
>
You are certainly right, just dyslexia on my side.
I will update the patch.
> Sean
>
> >return ret;
> > }
> >
> > --
> > 2.7.4
> >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
--
Cheers,
Alex G
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] CHROMIUM: drm/rockchip: Disable blending for win0
On Mon, Apr 16, 2018 at 03:22:15PM -0700, Kristian H. Kristensen wrote:
> Blending win0 with the background color doesn't seem to work
> correctly. We only get the background color, no matter the contents of
> the win0 framebuffer. However, blending pre-multiplied color with the
> default opaque black default background color is a no-op, so we can
> just disable blending to get the correct result.
>
> Signed-off-by: Kristian H. Kristensen
> Cc: Sandy Huang
> Cc: Sean Paul
Sandy, when you push this, could you please remove the CHROMIUM: prefix from the
subject?
Reviewed-by: Sean Paul
> ---
> As per Eric's suggestion, we can just disable blending. This replaces
> the previous "Filter out alpha formats for primary plane" patch.
>
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fae37b1cd691..1c1dd11d489a 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -961,7 +961,14 @@ static void vop_plane_atomic_update(struct drm_plane
> *plane,
> rb_swap = has_rb_swapped(fb->format->format);
> VOP_WIN_SET(vop, win, rb_swap, rb_swap);
>
> - if (is_alpha_support(fb->format->format)) {
> + /*
> + * Blending win0 with the background color doesn't seem to work
> + * correctly. We only get the background color, no matter the contents
> + * of the win0 framebuffer. However, blending pre-multiplied color
> + * with the default opaque black default background color is a no-op,
> + * so we can just disable blending to get the correct result.
> + */
> + if (is_alpha_support(fb->format->format) && win_index > 0) {
> VOP_WIN_SET(vop, win, dst_alpha_ctl,
> DST_FACTOR_M0(ALPHA_SRC_INVERSE));
> val = SRC_ALPHA_EN(1) | SRC_COLOR_M0(ALPHA_SRC_PRE_MUL) |
> --
> 2.17.0.484.g0c8726318c-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] CHROMIUM: drm/rockchip: Disable blending for win0
On Tue, Apr 17, 2018 at 09:37:44AM -0400, Sean Paul wrote:
> On Mon, Apr 16, 2018 at 03:22:15PM -0700, Kristian H. Kristensen wrote:
> > Blending win0 with the background color doesn't seem to work
> > correctly. We only get the background color, no matter the contents of
> > the win0 framebuffer. However, blending pre-multiplied color with the
> > default opaque black default background color is a no-op, so we can
> > just disable blending to get the correct result.
> >
> > Signed-off-by: Kristian H. Kristensen
> > Cc: Sandy Huang
> > Cc: Sean Paul
>
> Sandy, when you push this, could you please remove the CHROMIUM: prefix from
> the
> subject?
>
> Reviewed-by: Sean Paul
Sigh. I should read replies before opening my mouth. Looks like you'll need to
rebase on drm-misc-next and fix the compile issue.
Sean
>
>
> > ---
> > As per Eric's suggestion, we can just disable blending. This replaces
> > the previous "Filter out alpha formats for primary plane" patch.
> >
> > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 9 -
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > index fae37b1cd691..1c1dd11d489a 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > @@ -961,7 +961,14 @@ static void vop_plane_atomic_update(struct drm_plane
> > *plane,
> > rb_swap = has_rb_swapped(fb->format->format);
> > VOP_WIN_SET(vop, win, rb_swap, rb_swap);
> >
> > - if (is_alpha_support(fb->format->format)) {
> > + /*
> > +* Blending win0 with the background color doesn't seem to work
> > +* correctly. We only get the background color, no matter the contents
> > +* of the win0 framebuffer. However, blending pre-multiplied color
> > +* with the default opaque black default background color is a no-op,
> > +* so we can just disable blending to get the correct result.
> > +*/
> > + if (is_alpha_support(fb->format->format) && win_index > 0) {
> > VOP_WIN_SET(vop, win, dst_alpha_ctl,
> > DST_FACTOR_M0(ALPHA_SRC_INVERSE));
> > val = SRC_ALPHA_EN(1) | SRC_COLOR_M0(ALPHA_SRC_PRE_MUL) |
> > --
> > 2.17.0.484.g0c8726318c-goog
> >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 05/18] drm_hwcomposer: Enable resource manager support
Hi Sean,
On Mon, Apr 16, 2018 at 03:54:02PM -0400, Sean Paul wrote:
> On Wed, Apr 11, 2018 at 04:22:16PM +0100, Alexandru Gheorghe wrote:
> > Use the newly added ResourceManager for creating and detecting all the
> > drm devices instead of assuming that there is only one device.
> >
> > Signed-off-by: Alexandru Gheorghe
> > ---
> > drmhwctwo.cpp| 34 +-
> > drmhwctwo.h | 4 +---
> > drmresources.cpp | 25 ++---
> > drmresources.h | 14 +++---
> > 4 files changed, 43 insertions(+), 34 deletions(-)
> >
> > diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp
> > index dfca1a6..cddd5da 100644
> > --- a/drmhwctwo.cpp
> > +++ b/drmhwctwo.cpp
> > @@ -58,40 +58,32 @@ DrmHwcTwo::DrmHwcTwo() {
> > }
> >
> > HWC2::Error DrmHwcTwo::Init() {
> > - int ret = drm_.Init();
> > + int ret = resource_manager_.Init();
> >if (ret) {
> > -ALOGE("Can't initialize drm object %d", ret);
> > +ALOGE("Can't initialize the resource manager %d", ret);
> > return HWC2::Error::NoResources;
> >}
> >
> > - importer_.reset(Importer::CreateInstance(&drm_));
> > - if (!importer_) {
> > -ALOGE("Failed to create importer instance");
> > + DrmResources *drm =
> > resource_manager_.GetDrmResources(HWC_DISPLAY_PRIMARY);
> > + std::shared_ptr importer =
> > + resource_manager_.GetImporter(HWC_DISPLAY_PRIMARY);
> > + if (!drm || !importer) {
> > +ALOGE("Failed to get a valid drmresource and importer");
> > return HWC2::Error::NoResources;
> >}
> > + displays_.emplace(
> > + std::piecewise_construct, std::forward_as_tuple(HWC_DISPLAY_PRIMARY),
> > + std::forward_as_tuple(drm, importer, resource_manager_.GetGralloc(),
> > +HWC_DISPLAY_PRIMARY,
> > HWC2::DisplayType::Physical));
> >
> > - ret = hw_get_module(GRALLOC_HARDWARE_MODULE_ID,
> > - (const hw_module_t **)&gralloc_);
> > - if (ret) {
> > -ALOGE("Failed to open gralloc module %d", ret);
> > -return HWC2::Error::NoResources;
> > - }
> > -
> > - displays_.emplace(std::piecewise_construct,
> > -std::forward_as_tuple(HWC_DISPLAY_PRIMARY),
> > -std::forward_as_tuple(&drm_, importer_, gralloc_,
> > - HWC_DISPLAY_PRIMARY,
> > - HWC2::DisplayType::Physical));
> > -
> > - DrmCrtc *crtc =
> > drm_.GetCrtcForDisplay(static_cast(HWC_DISPLAY_PRIMARY));
> > + DrmCrtc *crtc =
> > drm->GetCrtcForDisplay(static_cast(HWC_DISPLAY_PRIMARY));
> >if (!crtc) {
> > ALOGE("Failed to get crtc for display %d",
> >static_cast(HWC_DISPLAY_PRIMARY));
> > return HWC2::Error::BadDisplay;
> >}
> > -
> >std::vector display_planes;
> > - for (auto &plane : drm_.planes()) {
> > + for (auto &plane : drm->planes()) {
> > if (plane->GetCrtcSupported(*crtc))
> >display_planes.push_back(plane.get());
> >}
> > diff --git a/drmhwctwo.h b/drmhwctwo.h
> > index 0490e2a..beb5d2d 100644
> > --- a/drmhwctwo.h
> > +++ b/drmhwctwo.h
> > @@ -262,9 +262,7 @@ class DrmHwcTwo : public hwc2_device_t {
> >HWC2::Error RegisterCallback(int32_t descriptor, hwc2_callback_data_t
> > data,
> > hwc2_function_pointer_t function);
> >
> > - DrmResources drm_;
> > - std::shared_ptr importer_; // Shared with HwcDisplay
> > - const gralloc_module_t *gralloc_;
> > + ResourceManager resource_manager_;
> >std::map displays_;
> >std::map callbacks_;
> > };
> > diff --git a/drmresources.cpp b/drmresources.cpp
> > index 32dd376..a5ddda0 100644
> > --- a/drmresources.cpp
> > +++ b/drmresources.cpp
> > @@ -42,10 +42,9 @@ DrmResources::~DrmResources() {
> >event_listener_.Exit();
> > }
> >
> > -int DrmResources::Init() {
> > - char path[PROPERTY_VALUE_MAX];
> > - property_get("hwc.drm.device", path, "/dev/dri/card0");
> > -
> > +int DrmResources::Init(ResourceManager *resource_manager, char *path,
> > + int start_display_index) {
> > + resource_manager_ = resource_manager;
>
> You can avoid the backpointer if you just pass the RM to the right places
> (looks
> like compositor + composition). Bonus points if you can remove drm_ from those
> objects once you've done that.
That's the thing Compositor/Composition already had drm_, hence the
need of the backpointer. Didn't want to touch that as well. I suppose
there is no strong reason why both Compositor & Composition shouldn't
have just a ResourceManager object.
>
> >/* TODO: Use drmOpenControl here instead */
> >fd_.Set(open(path, O_RDWR));
> >if (fd() < 0) {
> > @@ -76,8 +75,8 @@ int DrmResources::Init() {
> >max_resolution_ =
> >std::pair(res->max_width, res->max_height);
> >
> > - bool found_primary = false;
> > - int display_num = 1;
> > + bool found_primary = start_display_index != 0;
> > + int display_num = found_primary ? star
Re: [PATCH hwc v2 01/18] drm_hwcomposer: vsyncworker: Fix uninitialized enabled_ field
On Mon, Apr 16, 2018 at 01:18:53PM +0100, Alexandru-Cosmin Gheorghe wrote:
> On Mon, Apr 16, 2018 at 12:30:13PM +0200, Robert Foss wrote:
>
> Hi Rob,
>
> Thanks for the review.
>
> > Hey Alexandru,
> >
> > Feel free to add:
> > Signed-off-by: Robert Foss
> >
>
> Should I re-send this 3 patches or could you just pushed them to
> master and adding your SoB in the process?
I've pushed patches 1 & 3 with Robert's Ack. I'll review the rest this morning
at which time you can send a v2 with the unapplied patches. Alternatively, if
you want to try out the new gitlab flow, you can send v2 via a merge request
there.
Sean
>
> >
> > Rob.
> >
> > On 04/11/2018 05:22 PM, Alexandru Gheorghe wrote:
> > >Signed-off-by: Alexandru Gheorghe
> > >---
> > > vsyncworker.cpp | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > >diff --git a/vsyncworker.cpp b/vsyncworker.cpp
> > >index 3ad16fe..3bfe4be 100644
> > >--- a/vsyncworker.cpp
> > >+++ b/vsyncworker.cpp
> > >@@ -35,6 +35,7 @@ VSyncWorker::VSyncWorker()
> > > : Worker("vsync", HAL_PRIORITY_URGENT_DISPLAY),
> > >drm_(NULL),
> > >display_(-1),
> > >+ enabled_(false),
> > >last_timestamp_(-1) {
> > > }
> > >
>
> --
> Cheers,
> Alex G
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 06/18] drm_hwcomposer: Add writeback connector support
Hi Sean,
On Mon, Apr 16, 2018 at 03:59:07PM -0400, Sean Paul wrote:
> On Wed, Apr 11, 2018 at 04:22:17PM +0100, Alexandru Gheorghe wrote:
> > Writeback connector is a special case of connector, which can be
> > linked to a CRTC in order to get the result of the composition back to
> > a memory buffer. This had not been merged to the mainline kernel yet,
> > latest version of the kernel patches could be found here [1].
> >
> > [1]
> > https://lists.freedesktop.org/archives/dri-devel/2018-February/167703.html
> >
> > Signed-off-by: Alexandru Gheorghe
> > ---
> > drmconnector.cpp | 42 +-
> > drmconnector.h | 7 +++
> > 2 files changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/drmconnector.cpp b/drmconnector.cpp
> > index 145518f..e482832 100644
> > --- a/drmconnector.cpp
> > +++ b/drmconnector.cpp
> > @@ -52,6 +52,26 @@ int DrmConnector::Init() {
> > ALOGE("Could not get CRTC_ID property\n");
> > return ret;
> >}
> > + if (writeback()) {
> > +ret = drm_->GetConnectorProperty(*this, "WRITEBACK_PIXEL_FORMATS",
> > + &writeback_pixel_formats_);
> > +if (ret) {
> > + ALOGE("Could not get WRITEBACK_PIXEL_FORMATS connector_id = %d\n",
> > id_);
> > + return ret;
> > +}
> > +ret =
> > +drm_->GetConnectorProperty(*this, "WRITEBACK_FB_ID",
> > &writeback_fb_id_);
>
> Please break on the function arguments instead of the =
It's clang-format-diff-3.8 fault again.
Will do.
>
>
> > +if (ret) {
> > + ALOGE("Could not get WRITEBACK_FB_ID connector_id = %d\n", id_);
> > + return ret;
> > +}
> > +ret = drm_->GetConnectorProperty(*this, "WRITEBACK_OUT_FENCE_PTR",
> > + &writeback_out_fence_);
>
> Like this :)
That's me :).
>
> With that,
>
> Reviewed-by: Sean Paul
>
>
>
>
> > +if (ret) {
> > + ALOGE("Could not get WRITEBACK_OUT_FENCE_PTR connector_id = %d\n",
> > id_);
> > + return ret;
> > +}
> > + }
> >return 0;
> > }
> >
> > @@ -78,8 +98,16 @@ bool DrmConnector::external() const {
> > type_ == DRM_MODE_CONNECTOR_VGA;
> > }
> >
> > +bool DrmConnector::writeback() const {
> > +#ifdef DRM_MODE_CONNECTOR_WRITEBACK
> > + return type_ == DRM_MODE_CONNECTOR_WRITEBACK;
> > +#else
> > + return false;
> > +#endif
> > +}
> > +
> > bool DrmConnector::valid_type() const {
> > - return internal() || external();
> > + return internal() || external() || writeback();
> > }
> >
> > int DrmConnector::UpdateModes() {
> > @@ -130,6 +158,18 @@ const DrmProperty &DrmConnector::crtc_id_property()
> > const {
> >return crtc_id_property_;
> > }
> >
> > +const DrmProperty &DrmConnector::writeback_pixel_formats() const {
> > + return writeback_pixel_formats_;
> > +}
> > +
> > +const DrmProperty &DrmConnector::writeback_fb_id() const {
> > + return writeback_fb_id_;
> > +}
> > +
> > +const DrmProperty &DrmConnector::writeback_out_fence() const {
> > + return writeback_out_fence_;
> > +}
> > +
> > DrmEncoder *DrmConnector::encoder() const {
> >return encoder_;
> > }
> > diff --git a/drmconnector.h b/drmconnector.h
> > index 5601e06..e139730 100644
> > --- a/drmconnector.h
> > +++ b/drmconnector.h
> > @@ -46,6 +46,7 @@ class DrmConnector {
> >
> >bool internal() const;
> >bool external() const;
> > + bool writeback() const;
> >bool valid_type() const;
> >
> >int UpdateModes();
> > @@ -58,6 +59,9 @@ class DrmConnector {
> >
> >const DrmProperty &dpms_property() const;
> >const DrmProperty &crtc_id_property() const;
> > + const DrmProperty &writeback_pixel_formats() const;
> > + const DrmProperty &writeback_fb_id() const;
> > + const DrmProperty &writeback_out_fence() const;
> >
> >const std::vector &possible_encoders() const {
> > return possible_encoders_;
> > @@ -88,6 +92,9 @@ class DrmConnector {
> >
> >DrmProperty dpms_property_;
> >DrmProperty crtc_id_property_;
> > + DrmProperty writeback_pixel_formats_;
> > + DrmProperty writeback_fb_id_;
> > + DrmProperty writeback_out_fence_;
> >
> >std::vector possible_encoders_;
> > };
> > --
> > 2.7.4
> >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
--
Cheers,
Alex G
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 07/18] drm_hwcomposer: Add display field to Drmencoder
Hi Sean,
On Mon, Apr 16, 2018 at 04:02:07PM -0400, Sean Paul wrote:
> On Wed, Apr 11, 2018 at 04:22:18PM +0100, Alexandru Gheorghe wrote:
> > In the current implementation TryEncoderForDisplay just looks
> > at the crtc linked to the display, if that's not assigned to
> > a display it means the encoder could be used, otherwise iterate
> > to the list of possible_crtcs and find one which is not used.
> >
> > This logic works fine when you have just one encoder connected to a
> > crtc but with two or more, like is the case when we attach a writeback
> > connector, we need to know if we already assigned the encoder to a
> > display.
> >
> > Signed-off-by: Alexandru Gheorghe
> > ---
> > drmencoder.cpp | 14 ++
> > drmencoder.h | 4
> > 2 files changed, 18 insertions(+)
> >
> > diff --git a/drmencoder.cpp b/drmencoder.cpp
> > index 3d762f3..1da7ec3 100644
> > --- a/drmencoder.cpp
> > +++ b/drmencoder.cpp
> > @@ -27,6 +27,7 @@ DrmEncoder::DrmEncoder(drmModeEncoderPtr e, DrmCrtc
> > *current_crtc,
> > const std::vector &possible_crtcs)
> > : id_(e->encoder_id),
> >crtc_(current_crtc),
> > + display_(-1),
> >possible_crtcs_(possible_crtcs) {
> > }
> >
> > @@ -40,5 +41,18 @@ DrmCrtc *DrmEncoder::crtc() const {
> >
> > void DrmEncoder::set_crtc(DrmCrtc *crtc) {
> >crtc_ = crtc;
> > + set_display(crtc->display());
> > +}
> > +
> > +int DrmEncoder::display() const {
> > + return display_;
> > +}
> > +
> > +void DrmEncoder::set_display(int display) {
> > + display_ = display;
> > +}
>
> Instead of adding this, just call set_crtc() in TryEncoderForDisplay() for the
> already-bound case. That way we only have one entry point for this.
Fair enough, I will remove set_display.
>
> > +
> > +bool DrmEncoder::can_bind(int display) const {
> > + return display_ == -1 || display_ == display;
> > }
> > }
> > diff --git a/drmencoder.h b/drmencoder.h
> > index 58ccbfb..7e06691 100644
> > --- a/drmencoder.h
> > +++ b/drmencoder.h
> > @@ -36,6 +36,9 @@ class DrmEncoder {
> >
> >DrmCrtc *crtc() const;
> >void set_crtc(DrmCrtc *crtc);
> > + bool can_bind(int display) const;
> > + void set_display(int display);
> > + int display() const;
> >
> >const std::vector &possible_crtcs() const {
> > return possible_crtcs_;
> > @@ -44,6 +47,7 @@ class DrmEncoder {
> > private:
> >uint32_t id_;
> >DrmCrtc *crtc_;
> > + int display_;
> >
> >std::vector possible_crtcs_;
> > };
> > --
> > 2.7.4
> >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
--
Cheers,
Alex G
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105425] 3D & games produce periodic GPU crashes (Radeon R7 370)
https://bugs.freedesktop.org/show_bug.cgi?id=105425 --- Comment #32 from MirceaKitsune --- (In reply to iive from comment #31) Sounds a lot more complicated, but I'm gladly willing to try it as long as there's no risk of anything permanently breaking my system. The main problem is that I wasn't able to get the SysRq keys working in openSUSE Tumbleweed, which I tried as to enable the "REISUB" keys. I could really use clear instructions on how to enable and test them in openSUSE... ideally during runtime without having to make any permanent system changes. I need to remember how apitrace works, been a while since I used that. Also I remember it generated really a huge file, and the longer you run the program for the bigger it gets... if it doesn't happen within a few seconds I may gave a +1 GB trace, and I'm not sure where I can share that with the devs online. One thing to note: I have two computers at home, with mine being the crashy one and my mother's being an old and slow but stable machine. I can use SSH to connect in between them from bash. The problem is that the moment my machine freezes, its SSH connection instantly dies on the other PC as well... therefore I'm not sure how helpful this option is. The "r_shadows 2" option in Xonotic clearly makes a difference: Without it the crash only occurs after 3 hours... with it it's anywhere between a few seconds and at most 45 minutes. Definitely my best test case so far. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 08/18] drm_hwcomposer: Parse and store possible_clones information
Hi Sean,
On Mon, Apr 16, 2018 at 04:19:14PM -0400, Sean Paul wrote:
> On Wed, Apr 11, 2018 at 04:22:19PM +0100, Alexandru Gheorghe wrote:
> > drmModeEncoder has a field called possible_clones. It's a bit mask
> > which tells if the encoder could be simultaneously connected, to the
> > same CRTC, with the encoders specified in the possible_clones mask.
> >
> > Signed-off-by: Alexandru Gheorghe
> > ---
> > drmencoder.cpp | 8
> > drmencoder.h | 4
> > drmresources.cpp | 9 -
> > 3 files changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/drmencoder.cpp b/drmencoder.cpp
> > index 1da7ec3..ff675f5 100644
> > --- a/drmencoder.cpp
> > +++ b/drmencoder.cpp
> > @@ -39,6 +39,14 @@ DrmCrtc *DrmEncoder::crtc() const {
> >return crtc_;
> > }
> >
> > +bool DrmEncoder::can_clone(DrmEncoder *encoder) {
> > + return possible_clones_.find(encoder) != possible_clones_.end();
> > +}
>
> The find() call is probably enough to justify CamelCase for this function.
> FTR,
> I _hate_ this part of the style guide and wish I had just picked one or the
> other.
>
> To improve readability, can you also change the name of "encoder" to
> "possible_clone" like below so it's super obvious what this does?
>
Sure, will do.
> > +
> > +void DrmEncoder::add_possible_clone(DrmEncoder *possible_clone) {
> > + possible_clones_[possible_clone] = true;
> > +}
> > +
> > void DrmEncoder::set_crtc(DrmCrtc *crtc) {
> >crtc_ = crtc;
> >set_display(crtc->display());
> > diff --git a/drmencoder.h b/drmencoder.h
> > index 7e06691..5e7c010 100644
> > --- a/drmencoder.h
> > +++ b/drmencoder.h
> > @@ -21,6 +21,7 @@
> >
> > #include
> > #include
> > +#include
>
> Alphabetical
Sure, wil do.
>
> > #include
> >
> > namespace android {
> > @@ -43,6 +44,8 @@ class DrmEncoder {
> >const std::vector &possible_crtcs() const {
> > return possible_crtcs_;
> >}
> > + bool can_clone(DrmEncoder *encoder);
> > + void add_possible_clone(DrmEncoder *possible_clone);
> >
> > private:
> >uint32_t id_;
> > @@ -50,6 +53,7 @@ class DrmEncoder {
> >int display_;
> >
> >std::vector possible_crtcs_;
> > + std::map possible_clones_;
> > };
> > }
> >
> > diff --git a/drmresources.cpp b/drmresources.cpp
> > index a5ddda0..39f50be 100644
> > --- a/drmresources.cpp
> > +++ b/drmresources.cpp
> > @@ -97,6 +97,7 @@ int DrmResources::Init(ResourceManager *resource_manager,
> > char *path,
> > crtcs_.emplace_back(std::move(crtc));
> >}
> >
> > + std::vector possible_clones;
> >for (int i = 0; !ret && i < res->count_encoders; ++i) {
> > drmModeEncoderPtr e = drmModeGetEncoder(fd(), res->encoders[i]);
> > if (!e) {
> > @@ -117,12 +118,18 @@ int DrmResources::Init(ResourceManager
> > *resource_manager, char *path,
> >
> > std::unique_ptr enc(
> > new DrmEncoder(e, current_crtc, possible_crtcs));
> > -
> > +possible_clones.push_back(e->possible_clones);
> > drmModeFreeEncoder(e);
> >
> > encoders_.emplace_back(std::move(enc));
> >}
> >
> > + for (uint32_t i = 0; i < encoders_.size(); i++) {
> > +for (uint32_t j = 0; j < encoders_.size(); j++)
>
> for (auto &enc: encoders_) {
> for (auto &clone: encoders_) {
>
> Or something similarly C++'y, looping through indices is sooo last decade :-)
Oldie but goldie.
I do need the index in order to check the possible clones mask.
I will try to find something more millennial/Generation z :).
>
>
> > + if (possible_clones[i] & (1 << j))
> > +encoders_[i]->add_possible_clone(encoders_[j].get());
> > + }
> > +
> >for (int i = 0; !ret && i < res->count_connectors; ++i) {
> > drmModeConnectorPtr c = drmModeGetConnector(fd(), res->connectors[i]);
> > if (!c) {
> > --
> > 2.7.4
> >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
--
Cheers,
Alex G
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 01/18] drm_hwcomposer: vsyncworker: Fix uninitialized enabled_ field
On Tue, Apr 17, 2018 at 09:45:06AM -0400, Sean Paul wrote:
> On Mon, Apr 16, 2018 at 01:18:53PM +0100, Alexandru-Cosmin Gheorghe wrote:
> > On Mon, Apr 16, 2018 at 12:30:13PM +0200, Robert Foss wrote:
> >
> > Hi Rob,
> >
> > Thanks for the review.
> >
> > > Hey Alexandru,
> > >
> > > Feel free to add:
> > > Signed-off-by: Robert Foss
> > >
> >
> > Should I re-send this 3 patches or could you just pushed them to
> > master and adding your SoB in the process?
>
> I've pushed patches 1 & 3 with Robert's Ack. I'll review the rest this morning
> at which time you can send a v2 with the unapplied patches. Alternatively, if
> you want to try out the new gitlab flow, you can send v2 via a merge request
> there.
Thanks.
I'm all for trying new things. So, I will give gitlab flow a chance.
>
> Sean
>
> >
> > >
> > > Rob.
> > >
> > > On 04/11/2018 05:22 PM, Alexandru Gheorghe wrote:
> > > >Signed-off-by: Alexandru Gheorghe
> > > >---
> > > > vsyncworker.cpp | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > >diff --git a/vsyncworker.cpp b/vsyncworker.cpp
> > > >index 3ad16fe..3bfe4be 100644
> > > >--- a/vsyncworker.cpp
> > > >+++ b/vsyncworker.cpp
> > > >@@ -35,6 +35,7 @@ VSyncWorker::VSyncWorker()
> > > > : Worker("vsync", HAL_PRIORITY_URGENT_DISPLAY),
> > > >drm_(NULL),
> > > >display_(-1),
> > > >+ enabled_(false),
> > > >last_timestamp_(-1) {
> > > > }
> > > >
> >
> > --
> > Cheers,
> > Alex G
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
--
Cheers,
Alex G
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [RESEND PATCH 1/1] drm/i915/glk: Add MODULE_FIRMWARE for Geminilake
Quoting Jani Nikula (2018-04-17 12:02:52) > On Mon, 16 Apr 2018, "Srivatsa, Anusha" wrote: > >>-Original Message- > >>From: Jani Nikula [mailto:jani.nik...@linux.intel.com] > >>Sent: Wednesday, April 11, 2018 5:27 AM > >>To: Ian W MORRISON > >>Cc: Vivi, Rodrigo ; Srivatsa, Anusha > >>; Wajdeczko, Michal > >>; Greg KH ; > >>airl...@linux.ie; joonas.lahti...@linux.intel.com; > >>linux-ker...@vger.kernel.org; > >>sta...@vger.kernel.org; intel-...@lists.freedesktop.org; dri- > >>de...@lists.freedesktop.org > >>Subject: Re: [RESEND PATCH 1/1] drm/i915/glk: Add MODULE_FIRMWARE for > >>Geminilake > >> > >>On Wed, 11 Apr 2018, Ian W MORRISON wrote: > >>> > >>> > > NAK on indiscriminate Cc: stable. There are zero guarantees that > older kernels will work with whatever firmware you throw at them. > > >>> > >>> I included 'Cc: stable' so the patch would get added to the v4.16 and > >>> v4.15 kernels which I have tested with the patch. I found that earlier > >>> kernels didn't support the 'linux-firmware' package required to get > >>> wifi working on Intel's new Gemini Lake NUC. > >> > >>You realize that this patch should have nothing to do with wifi? > >> > >>Rodrigo, Anusha, if you think Cc: stable is appropriate, please indicate > >>the specific > >>versions of stable it is appropriate for. > > > > Hi Jani, > > > > The stable kernel version is 4.12 and beyond. > > It is appropriate to add the CC: stable in my opinion > > Who tested the firmware with v4.12 and later? We only have the CI > results against *current* drm-tip. We don't even know about v4.16. > > I'm not going to ack and take responsibility for the stable backports > unless someone actually comes forward with credible Tested-bys. And even then, some distros will be surprised of the new MODULE_FIRMWARE and will need to update the linux-firmware package, too. Regards, Joonas > > BR, > Jani. > > > > > > Anusha > >>BR, > >>Jani. > >> > >>> > > PS. How is this a "RESEND"? I haven't seen this before. > > >>> > >>> It is a 'RESEND' for that very reason. I initially sent the patch to > >>> the same people as a similar patch > >>> (https://patchwork.kernel.org/patch/10143637/) however after realising > >>> this omitted required addresses I added them and resent the patch. > >>> > >>> Best regards, > >>> Ian > >> > >>-- > >>Jani Nikula, Intel Open Source Technology Center > > -- > Jani Nikula, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 05/18] drm_hwcomposer: Enable resource manager support
On Tue, Apr 17, 2018 at 02:43:17PM +0100, Alexandru-Cosmin Gheorghe wrote:
> Hi Sean,
>
> On Mon, Apr 16, 2018 at 03:54:02PM -0400, Sean Paul wrote:
> > On Wed, Apr 11, 2018 at 04:22:16PM +0100, Alexandru Gheorghe wrote:
> > > Use the newly added ResourceManager for creating and detecting all the
> > > drm devices instead of assuming that there is only one device.
> > >
> > > Signed-off-by: Alexandru Gheorghe
> > > ---
> > > diff --git a/drmresources.cpp b/drmresources.cpp
> > > index 32dd376..a5ddda0 100644
> > > --- a/drmresources.cpp
> > > +++ b/drmresources.cpp
> > > @@ -42,10 +42,9 @@ DrmResources::~DrmResources() {
> > >event_listener_.Exit();
> > > }
> > >
> > > -int DrmResources::Init() {
> > > - char path[PROPERTY_VALUE_MAX];
> > > - property_get("hwc.drm.device", path, "/dev/dri/card0");
> > > -
> > > +int DrmResources::Init(ResourceManager *resource_manager, char *path,
> > > + int start_display_index) {
> > > + resource_manager_ = resource_manager;
> >
> > You can avoid the backpointer if you just pass the RM to the right places
> > (looks
> > like compositor + composition). Bonus points if you can remove drm_ from
> > those
> > objects once you've done that.
>
> That's the thing Compositor/Composition already had drm_, hence the
> need of the backpointer. Didn't want to touch that as well. I suppose
> there is no strong reason why both Compositor & Composition shouldn't
> have just a ResourceManager object.
Yeah, exactly what I was thinking. It'll be a bit more refactoring, but worth it
IMO.
> > > diff --git a/drmresources.h b/drmresources.h
> > > index 4cca48c..4cdcd87 100644
> > > --- a/drmresources.h
> > > +++ b/drmresources.h
> > > @@ -17,22 +17,26 @@
> > > #ifndef ANDROID_DRM_H_
> > > #define ANDROID_DRM_H_
> > >
> > > +#include
> > > #include "drmconnector.h"
> > > #include "drmcrtc.h"
> > > #include "drmencoder.h"
> > > #include "drmeventlistener.h"
> > > #include "drmplane.h"
> > > -
> > > -#include
> >
> > Why this change?
>
> I blame clang-format-diff-3.8 -i. I suppose it should be in a different
> commit.
I'd rather fix it at the source. If stdint.h include needs to move, that
probably means one of our headers needs to include it. So let's figure out which
needs it and add it there instead of shuffling things here.
Sean
> >
> > > +#include "platform.h"
> > > +#include "resourcemanager.h"
> > >
> > > namespace android {
> > >
> > > +class ResourceManager;
> > > +
> > > class DrmResources {
> > > public:
> > >DrmResources();
> > >~DrmResources();
> > >
> > > - int Init();
> > > + int Init(ResourceManager *resource_manager, char *path,
> > > + int start_display_index);
> > >
> > >int fd() const {
> > > return fd_.get();
> > > @@ -58,6 +62,7 @@ class DrmResources {
> > >DrmCrtc *GetCrtcForDisplay(int display) const;
> > >DrmPlane *GetPlane(uint32_t id) const;
> > >DrmEventListener *event_listener();
> > > + ResourceManager *resource_manager();
> > >
> > >int GetPlaneProperty(const DrmPlane &plane, const char *prop_name,
> > > DrmProperty *property);
> > > @@ -71,6 +76,7 @@ class DrmResources {
> > >
> > >int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id);
> > >int DestroyPropertyBlob(uint32_t blob_id);
> > > + bool HandlesDisplay(int display) const;
> > >
> > > private:
> > >int TryEncoderForDisplay(int display, DrmEncoder *enc);
> > > @@ -90,6 +96,8 @@ class DrmResources {
> > >
> > >std::pair min_resolution_;
> > >std::pair max_resolution_;
> > > + std::map displays_;
> > > + ResourceManager *resource_manager_;
> > > };
> > > }
> > >
> > > --
> > > 2.7.4
> > >
> >
> > --
> > Sean Paul, Software Engineer, Google / Chromium OS
>
> --
> Cheers,
> Alex G
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 05/18] drm_hwcomposer: Enable resource manager support
On Wed, Apr 11, 2018 at 04:22:16PM +0100, Alexandru Gheorghe wrote:
> Use the newly added ResourceManager for creating and detecting all the
> drm devices instead of assuming that there is only one device.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> index 4cca48c..4cdcd87 100644
> --- a/drmresources.h
> +++ b/drmresources.h
> @@ -17,22 +17,26 @@
> #ifndef ANDROID_DRM_H_
> #define ANDROID_DRM_H_
>
> +#include
> #include "drmconnector.h"
> #include "drmcrtc.h"
> #include "drmencoder.h"
> #include "drmeventlistener.h"
> #include "drmplane.h"
> -
> -#include
> +#include "platform.h"
> +#include "resourcemanager.h"
>
> namespace android {
>
> +class ResourceManager;
> +
> class DrmResources {
One more thing I've been thinking about. Let's rename this to DrmDevice now that
we can have more than one. It's immediately obvious what a collection of
DrmDevices is, it's less obvious if they're DrmResources. I think
ResourceManager is Ok to keep, but if you think there's a better name I'm open
to that.
Sean
> public:
>DrmResources();
>~DrmResources();
>
> - int Init();
> + int Init(ResourceManager *resource_manager, char *path,
> + int start_display_index);
>
>int fd() const {
> return fd_.get();
> @@ -58,6 +62,7 @@ class DrmResources {
>DrmCrtc *GetCrtcForDisplay(int display) const;
>DrmPlane *GetPlane(uint32_t id) const;
>DrmEventListener *event_listener();
> + ResourceManager *resource_manager();
>
>int GetPlaneProperty(const DrmPlane &plane, const char *prop_name,
> DrmProperty *property);
> @@ -71,6 +76,7 @@ class DrmResources {
>
>int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id);
>int DestroyPropertyBlob(uint32_t blob_id);
> + bool HandlesDisplay(int display) const;
>
> private:
>int TryEncoderForDisplay(int display, DrmEncoder *enc);
> @@ -90,6 +96,8 @@ class DrmResources {
>
>std::pair min_resolution_;
>std::pair max_resolution_;
> + std::map displays_;
> + ResourceManager *resource_manager_;
> };
> }
>
> --
> 2.7.4
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Patch 2/4] dt-bindings: display/ti: Add plane binding to dispc node
Hi Rob, On 09/04/18 21:17, Rob Herring wrote: >>> For HDMI, you can't know in advance what resolution will be. So I >>> think you always need to reserve 2 planes. Now, if you want to reduce >> >> We can decide not to support 2k+ resolutions for HDMI, which, with this >> series, happens by not reserving dual-plane for the HDMI. > > Right. So turn this around. Define in DT what is the maximum > resolution supported for HDMI and configure the planes based on that. But the kernel cannot know what the user wants to do, so it cannot configure the planes. If we have an HDMI output which supports 2k+ and a -2k LCD, and 4 hw planes, we can set up the planes at least in the following ways: - One virtual plane on HDMI, two normal planes on LCD. Here the normal planes can also be used on the HDMI, as long as the input width is -2k. - One virtual plane on HDMI, one virtual plane on LCD, but sometimes both planes on the same display (either HDMI or LCD). - No virtual planes (and fbdev support disabled). This needs the userspace to take care not to configure 2k+ planes. But considering that the modes supported are still quit close to 2k in width, I believe upscaling a 2k plane cover the whole display would provide quite ok image. Each of those requires a different virtual plane setup. >> But reserve how many of the planes? We have N planes and M displays. For >> some of the displays we know they're 2k+, some are known to be -2k and >> some are unknown. The driver can't independently make any sensible >> static reservation of the planes for the displays, because it doesn't >> know what the user wants to do. > > After you've handled HDMI as above and any permanently attached panels > with fixed resolutions, what is left for a user to configure? Perhaps > only one display can support an overlay at that point because you are > out of planes? I think I covered this in the example use cases above. >> So either we reserve the extra planes at runtime on demand, making it >> difficult to manage for the userspace, or we rely on the user to give >> the driver a static partitioning of the planes according to the user's >> use case. > > And by user, who do you mean exactly? The use case is tied to the > board design and product or tied to the whims of an end user (e.g. I > want to do video playback with overlay to disp 2)? You should equate > users making DT changes with telling users to update/change their > BIOS. By user I mean the owner of the device, but it in some cases it could be the vendor too. If we have a board with HDMI that can support 2k+, then the board vendor could provide DT files that do not specify virtual planes (so no 2k+), but give instructions how to define them for the users who want 2k+. So here the end user needs to deal with the static partitioning. If we have a board with a fixed resolution 2k+ LCD, then the vendor has to have a virtual plane defined in the DT, and the vendor has to pick a configuration that it thinks is most useful. And yes, it sucks to have to make changes in the DT or BIOS, but I still don't see a (good) alternative. I think one option is to have the detailed DT configuration optional: We'd have a flag in the DT to mark that a display supports 2k+ (or the max-resolution property as you suggested). Based on that, the driver guesses what kind of setup the user wants, which probably is just to set up planes in a way that each display has a fully functional "root" plane, and then split the remaining ones in some way. But the user could also have detailed DT descriptions for the planes when he needs a more special setup. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [Intel-gfx] [PATCH 01/10] include: Move ascii85 functions from i915 to linux/ascii85.h
On Mon, Apr 16, 2018 at 10:52:59AM -0700, Eric Anholt wrote: > Chris Wilson writes: > > > Quoting Jordan Crouse (2018-04-05 23:06:53) > >> On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote: > >> > The i915 DRM driver very cleverly used ascii85 encoding for their > >> > GPU state file. Move the encode functions to a general header file to > >> > support other drivers that might be interested in the same > >> > functionality. > >> > >> In a previous version of this patch, Chris asked what tree I wanted this > >> applied > >> to, and the answer is: I'm not sure? I'm hoping that Rob will be cool with > >> picking the rest up for msm-next for 4.18 but I'm okay with putting this > >> particular patch wherever it is easiest for the maintainers. > > > > We are a bit late to sneak it into the 4.17 drm base via i915. I don't > > anticipate any problems (for i915) with this patch going in through > > msm-next, so happy to have it land there and percolate back to i915 3 > > months later. > > I'd love to have it in drm-misc-next, so I can build a similar hang dump > interface for vc5. But most importantly, I'd like to have it somewhere > soon. I'll fix the bot error and push it up again today. Jordan -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 199357] amdgpu: hang a few seconds after logging in, most likely due to regression
https://bugzilla.kernel.org/show_bug.cgi?id=199357 --- Comment #11 from Alex Deucher (alexdeuc...@gmail.com) --- Yes, the revert cc'ed stable so it will show up in 4.16 as well. -- 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 176311] Fiji DisplayPort amdgpu_crtc_page_flip *ERROR* failed to get vblank before flip
https://bugzilla.kernel.org/show_bug.cgi?id=176311 --- Comment #7 from Michel Dänzer (mic...@daenzer.net) --- Does https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=daf8809626c0ee7a152f9c34058fc3b43385dd51 help for this, by any chance? BTW, I made a mistake in comment 4, need to test xf86-video-amdgpu here, not xf86-video-ati. -- 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] gpu: drm: i915: Change return type to vm_fault_t
On Tue, 17 Apr 2018, Souptick Joarder wrote:
> Use new return type vm_fault_t for fault handler. For
> now, this is just documenting that the function returns
> a VM_FAULT value rather than an errno. Once all instances
> are converted, vm_fault_t will become a distinct type.
>
> Reference id -> 1c8f422059ae ("mm: change return type to
> vm_fault_t")
>
> Signed-off-by: Souptick Joarder
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> drivers/gpu/drm/i915/i915_gem.c | 15 ---
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a42deeb..95b0d50 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -51,6 +51,7 @@
> #include
> #include
> #include
> +#include
>
> #include "i915_params.h"
> #include "i915_reg.h"
> @@ -3363,7 +3364,7 @@ int i915_gem_wait_for_idle(struct drm_i915_private
> *dev_priv,
> unsigned int flags);
> int __must_check i915_gem_suspend(struct drm_i915_private *dev_priv);
> void i915_gem_resume(struct drm_i915_private *dev_priv);
> -int i915_gem_fault(struct vm_fault *vmf);
> +vm_fault_t i915_gem_fault(struct vm_fault *vmf);
> int i915_gem_object_wait(struct drm_i915_gem_object *obj,
>unsigned int flags,
>long timeout,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index dd89abd..bdac690 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1882,7 +1882,7 @@ int i915_gem_mmap_gtt_version(void)
> * The current feature set supported by i915_gem_fault() and thus GTT mmaps
> * is exposed via I915_PARAM_MMAP_GTT_VERSION (see
> i915_gem_mmap_gtt_version).
> */
> -int i915_gem_fault(struct vm_fault *vmf)
> +vm_fault_t i915_gem_fault(struct vm_fault *vmf)
> {
> #define MIN_CHUNK_PAGES ((1 << 20) >> PAGE_SHIFT) /* 1 MiB */
> struct vm_area_struct *area = vmf->vma;
> @@ -1895,6 +1895,7 @@ int i915_gem_fault(struct vm_fault *vmf)
> pgoff_t page_offset;
> unsigned int flags;
> int ret;
> + vm_fault_t retval;
What's the point of changing the name? An unnecessary change.
BR,
Jani.
>
> /* We don't use vmf->pgoff since that has the fake offset */
> page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT;
> @@ -2000,7 +2001,7 @@ int i915_gem_fault(struct vm_fault *vmf)
>* and so needs to be reported.
>*/
> if (!i915_terminally_wedged(&dev_priv->gpu_error)) {
> - ret = VM_FAULT_SIGBUS;
> + retval = VM_FAULT_SIGBUS;
> break;
> }
> case -EAGAIN:
> @@ -2017,21 +2018,21 @@ int i915_gem_fault(struct vm_fault *vmf)
>* EBUSY is ok: this just means that another thread
>* already did the job.
>*/
> - ret = VM_FAULT_NOPAGE;
> + retval = VM_FAULT_NOPAGE;
> break;
> case -ENOMEM:
> - ret = VM_FAULT_OOM;
> + retval = VM_FAULT_OOM;
> break;
> case -ENOSPC:
> case -EFAULT:
> - ret = VM_FAULT_SIGBUS;
> + retval = VM_FAULT_SIGBUS;
> break;
> default:
> WARN_ONCE(ret, "unhandled error in i915_gem_fault: %i\n", ret);
> - ret = VM_FAULT_SIGBUS;
> + retval = VM_FAULT_SIGBUS;
> break;
> }
> - return ret;
> + return retval;
> }
>
> static void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
> --
> 1.9.1
>
--
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 97025] flip queue failed: Device or resource busy
https://bugs.freedesktop.org/show_bug.cgi?id=97025 --- Comment #26 from Michel Dänzer --- Does https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=daf8809626c0ee7a152f9c34058fc3b43385dd51 help for this? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 04/18] drm_hwcomposer: Add resource manager class
On Wed, Apr 11, 2018 at 04:22:15PM +0100, Alexandru Gheorghe wrote:
> Add a resource manager object that is responsible for detecting all
> kms devices and allocates unique display numbers for every detected
> display.
>
> This is controlled by the value of hwc.drm.device property, if it ends
> with a %, it will try to open minor devices until and error is detected.
> E.g: /dev/dri/card%
I'm a bit on the fence as to whether to use the %, or add a new
hwc.drm.scan_devices property. I guess since we need to convey the path anyways
this is fine, but it really needs to be better documented.
>
> Additionally, this will be used for finding an available writeback
> connector that will be used for the flattening of the currently
> displayed scene.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> Android.mk | 1 +
> resourcemanager.cpp | 71
> +
> resourcemanager.h | 29 ++
> 3 files changed, 101 insertions(+)
> create mode 100644 resourcemanager.cpp
> create mode 100644 resourcemanager.h
>
> diff --git a/Android.mk b/Android.mk
> index 1add286..736fe24 100644
> --- a/Android.mk
> +++ b/Android.mk
> @@ -52,6 +52,7 @@ LOCAL_C_INCLUDES := \
>
> LOCAL_SRC_FILES := \
> autolock.cpp \
> + resourcemanager.cpp \
> drmresources.cpp \
> drmconnector.cpp \
> drmcrtc.cpp \
> diff --git a/resourcemanager.cpp b/resourcemanager.cpp
> new file mode 100644
> index 000..e7b654e
> --- /dev/null
> +++ b/resourcemanager.cpp
> @@ -0,0 +1,71 @@
> +#include "resourcemanager.h"
> +#include
> +#include
> +
> +namespace android {
> +
> +ResourceManager::ResourceManager() : gralloc_(NULL) {
> +}
> +
> +int ResourceManager::Init() {
> + char path_pattern[PROPERTY_VALUE_MAX];
> + property_get("hwc.drm.device", path_pattern, "/dev/dri/card%");
We probably also don't want to default to scanning, since that might change
behavior in existing boards.
> +
> + uint8_t minor = 0;
Please use unsigned/int instead of fixed-size types. Unless the number of bits
is relevant to use of the variable, let the compiler handle it.
> + int last_display_index = 0;
Could we just call this num_displays? It was kind of hard for me to reason
through this, especially when it's call "start_display_index" when you jump into
drm::Init(). I also think drm->Init's return pair should return
instead of , so each time you call
Init(), you're adding to num_displays.
> + int last_char = strlen(path_pattern) - 1;
> + do {
> +char path[PROPERTY_VALUE_MAX];
Please use string
> +if (path_pattern[last_char] == '%') {
> + path_pattern[last_char] = '\0';
> + snprintf(path, PROPERTY_VALUE_MAX, "%s%d", path_pattern, minor);
> + path_pattern[last_char] = '%';
This doesn't work for minor > 10.
> +} else {
> + snprintf(path, PROPERTY_VALUE_MAX, "%s", path_pattern);
> +}
> +std::unique_ptr drm = std::make_unique();
> +last_display_index = drm->Init(this, path, last_display_index);
> +if (last_display_index < 0) {
> + break;
> +}
> +std::shared_ptr importer;
> +importer.reset(Importer::CreateInstance(drm.get()));
> +if (!importer) {
> + ALOGE("Failed to create importer instance");
> + break;
> +}
> +importers_.push_back(importer);
> +drms_.push_back(std::move(drm));
> +minor++;
> +last_display_index++;
> + } while (path_pattern[last_char] == '%');
> +
> + if (!drms_.size()) {
> +ALOGE("Failed to find any working drm device");
> +return -EINVAL;
> + }
> +
> + return hw_get_module(GRALLOC_HARDWARE_MODULE_ID,
> + (const hw_module_t **)&gralloc_);
> +}
Consider the following:
int ResourceManager::AddDrmDevice(std::string path) {
std::unique_ptr drm = std::make_unique();
int displays_added, ret;
std::tie(displays_added, ret) = drm->Init(path.c_str(), num_displays_);
if (ret)
return ret;
std::shared_ptr importer;
importer.reset(Importer::CreateInstance(drm.get()));
if (!importer) {
ALOGE("Failed to create importer instance");
return -ENODEV;
}
importers_.push_back(importer);
drms_.push_back(std::move(drm));
num_displays_ += displays_added;
return 0;
}
int ResourceManager::Init() {
char path_pattern[PROPERTY_VALUE_MAX];
int path_len = property_get("hwc.drm.device", path_pattern, "/dev/dri/card%");
if (path_pattern[path_len - 1] != '%')
return AddDrmDevice(std::string(path_pattern);
path_pattern[path_len - 1] = '\0';
for (int ret = 0, idx = 0; !ret; ++idx) {
ostringstream path;
path << path_pattern << idx;
ret = AddDrmDevice(path.str());
}
if (!num_displays_) {
ALOGE("Failed to initialize any displays");
return -EINVAL;
}
return hw_get_module(GRALLOC_HARDWARE_MODULE_ID,
(const hw_module_t **)&gralloc_);
}
I think resolves the issues from the original patches and incorporates the
suggestions of drm->Init() returning t
Re: [PATCH hwc v2 09/18] drm_hwcomposer: Handle writeback connectors
On Wed, Apr 11, 2018 at 04:22:20PM +0100, Alexandru Gheorghe wrote:
> When writeback connectors are available assign them to displays, in
> order to be able to use them for flattening of the current displayed
> scene. The pipeline for each display will look like this:
>
> CRTC encoder display connector.
> |--- writeback enc -- writeback connector.
>
> However, the writeback connector will be later used/enabled only if
> one of the following conditions are met:
> - Could be a clone of the display connector, as pointed by the
>possible_clones property.
> - The display_connector is disconnected, so we are safe to use it for
>flattening the scene that's already presented on another display.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> drmresources.cpp | 62
> ++--
> drmresources.h | 3 +++
> 2 files changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/drmresources.cpp b/drmresources.cpp
> index 39f50be..fef6835 100644
> --- a/drmresources.cpp
> +++ b/drmresources.cpp
> @@ -64,6 +64,14 @@ int DrmResources::Init(ResourceManager *resource_manager,
> char *path,
> return ret;
>}
>
> +#ifdef DRM_CLIENT_CAP_WRITEBACK_CONNECTORS
> + ret = drmSetClientCap(fd(), DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> + if (ret) {
> +ALOGI("Failed to set writeback cap %d", ret);
> +ret = 0;
> + }
> +#endif
> +
>drmModeResPtr res = drmModeGetResources(fd());
>if (!res) {
> ALOGE("Failed to get DrmResources resources");
> @@ -169,7 +177,7 @@ int DrmResources::Init(ResourceManager *resource_manager,
> char *path,
>conn->set_display(0);
>displays_[0] = 0;
>found_primary = true;
> -} else {
> +} else if (conn->external()) {
>conn->set_display(display_num);
>displays_[display_num] = display_num;
>++display_num;
> @@ -230,6 +238,8 @@ int DrmResources::Init(ResourceManager *resource_manager,
> char *path,
>}
>
>for (auto &conn : connectors_) {
> +if (conn->writeback())
> + continue;
> ret = CreateDisplayPipe(conn.get());
> if (ret) {
>ALOGE("Failed CreateDisplayPipe %d with %d", conn->id(), ret);
> @@ -245,7 +255,15 @@ bool DrmResources::HandlesDisplay(int display) const {
>
> DrmConnector *DrmResources::GetConnectorForDisplay(int display) const {
>for (auto &conn : connectors_) {
> -if (conn->display() == display)
> +if (conn->display() == display && !conn->writeback())
> + return conn.get();
> + }
> + return NULL;
> +}
> +
> +DrmConnector *DrmResources::GetWritebackConnectorForDisplay(int display)
> const {
> + for (auto &conn : connectors_) {
> +if (conn->display() == display && conn->writeback())
>return conn.get();
>}
>return NULL;
> @@ -280,6 +298,7 @@ int DrmResources::TryEncoderForDisplay(int display,
> DrmEncoder *enc) {
>DrmCrtc *crtc = enc->crtc();
>if (crtc && crtc->can_bind(display)) {
> crtc->set_display(display);
> +enc->set_display(display);
> return 0;
>}
>
> @@ -306,6 +325,7 @@ int DrmResources::CreateDisplayPipe(DrmConnector
> *connector) {
>if (connector->encoder()) {
> int ret = TryEncoderForDisplay(display, connector->encoder());
> if (!ret) {
> + AttachWriteback(connector);
AttachWriteback returns int, but you throw it away here. Additionally,
AttachWriteback always follows a successful TryEncoderForDisplay, so it makes
sense to just call it from there.
>return 0;
> } else if (ret != -EAGAIN) {
>ALOGE("Could not set mode %d/%d", display, ret);
> @@ -317,6 +337,7 @@ int DrmResources::CreateDisplayPipe(DrmConnector
> *connector) {
> int ret = TryEncoderForDisplay(display, enc);
> if (!ret) {
>connector->set_encoder(enc);
> + AttachWriteback(connector);
>return 0;
> } else if (ret != -EAGAIN) {
>ALOGE("Could not set mode %d/%d", display, ret);
> @@ -328,6 +349,43 @@ int DrmResources::CreateDisplayPipe(DrmConnector
> *connector) {
>return -ENODEV;
> }
>
> +/*
> + * Attach writeback connector to the CRTC linked to the display_conn
> + *
> + */
> +int DrmResources::AttachWriteback(DrmConnector *display_conn) {
> + int ret = -EINVAL;
This isn't really used, just return the error code directly at the bottom.
> + if (display_conn->writeback())
> +return -EINVAL;
This condition would benefit from a log
> + DrmEncoder *display_enc = display_conn->encoder();
> + if (!display_enc)
> +return -EINVAL;
> + DrmCrtc *display_crtc = display_enc->crtc();
> + if (!display_crtc)
> +return -EINVAL;
Are these possible given this is only called after a successful
TryEncoderForDisplay()?
> + if (GetWritebackConnectorForDisplay(display_crtc->display()) != NULL)
> +return -EINVAL;
Again, logging would be useful.
> + for (auto &writeback_conn : connectors_) {
> +if (writeback_conn->display() >= 0 || !writeback_co
[PATCH v2 1/3] drm/prime: Iterate SG DMA addresses separately
For dma_map_sg(), DMA API implementations are free to merge consecutive
segments into a single DMA mapping if conditions are suitable, thus the
resulting DMA addresses which drm_prime_sg_to_page_addr_arrays()
iterates may be packed into fewer entries than ttm->sg->nents implies.
The current implementation does not account for this, meaning that its
callers either have to reject the 0 < count < nents case or risk getting
bogus DMA addresses beyond the first segment. Fortunately this is quite
easy to handle without having to rejig structures to also store the
mapped count, since the total DMA length should still be equal to the
total buffer length. All we need is a second scatterlist cursor to
iterate through the DMA addresses independently of the page addresses.
Signed-off-by: Robin Murphy
---
v2: Remember to iterate dma_len correctly as well.
drivers/gpu/drm/drm_prime.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 7856a9b3f8a8..b8ca06ea7d80 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -933,16 +933,18 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table
*sgt, struct page **pages,
dma_addr_t *addrs, int max_entries)
{
unsigned count;
- struct scatterlist *sg;
+ struct scatterlist *sg, *dma_sg;
struct page *page;
- u32 len, index;
+ u32 len, dma_len, index;
dma_addr_t addr;
index = 0;
+ dma_sg = sgt->sgl;
+ dma_len = sg_dma_len(dma_sg);
+ addr = sg_dma_address(dma_sg);
for_each_sg(sgt->sgl, sg, sgt->nents, count) {
len = sg->length;
page = sg_page(sg);
- addr = sg_dma_address(sg);
while (len > 0) {
if (WARN_ON(index >= max_entries))
@@ -955,8 +957,15 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt,
struct page **pages,
page++;
addr += PAGE_SIZE;
len -= PAGE_SIZE;
+ dma_len -= PAGE_SIZE;
index++;
}
+
+ if (dma_len == 0) {
+ dma_sg = sg_next(dma_sg);
+ dma_len = sg_dma_len(dma_sg);
+ addr = sg_dma_address(dma_sg);
+ }
}
return 0;
}
--
2.16.1.dirty
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/3] drm/amdgpu: Allow dma_map_sg() coalescing
The amdgpu driver doesn't appear to directly use the scatterlist mapped by amdgpu_ttm_tt_pin_userptr(), it merely hands it off to drm_prime_sg_to_page_addr_arrays() to generate the dma_address array which it actually cares about. Now that the latter can cope with dma_map_sg() coalescing dma-contiguous segments such that it returns 0 < count < nents, we can relax the current count == nents check to only consider genuine failure as other drivers do. This avoids a false negative on arm64 systems with an IOMMU. Reported-by: Sinan Kaya Signed-off-by: Robin Murphy --- v2: Expand commit message to clarify drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 205da3ff9cd0..f81e96a4242f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -813,7 +813,7 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm) r = -ENOMEM; nents = dma_map_sg(adev->dev, ttm->sg->sgl, ttm->sg->nents, direction); - if (nents != ttm->sg->nents) + if (nents == 0) goto release_sg; drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages, -- 2.16.1.dirty ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/3] drm/radeon: Allow dma_map_sg() coalescing
Much like amdgpu, the radeon driver doesn't appear to directly use the scatterlist mapped by radeon_ttm_tt_pin_userptr(), it merely hands it off to drm_prime_sg_to_page_addr_arrays() to generate the dma_address array which it actually cares about. Now that the latter can cope with dma_map_sg() coalescing dma-contiguous segments such that it returns 0 < count < nents, we can relax the current count == nents check to only consider genuine failure as other drivers do. Suggested-by: Christian König Signed-off-by: Robin Murphy --- v2: New drivers/gpu/drm/radeon/radeon_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 8689fcca051c..7c099192c7fa 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -585,7 +585,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm) r = -ENOMEM; nents = dma_map_sg(rdev->dev, ttm->sg->sgl, ttm->sg->nents, direction); - if (nents != ttm->sg->nents) + if (nents == 0) goto release_sg; drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages, -- 2.16.1.dirty ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 04/18] drm_hwcomposer: Add resource manager class
Hey,
On 04/17/2018 05:33 PM, Sean Paul wrote:
On Wed, Apr 11, 2018 at 04:22:15PM +0100, Alexandru Gheorghe wrote:
Add a resource manager object that is responsible for detecting all
kms devices and allocates unique display numbers for every detected
display.
This is controlled by the value of hwc.drm.device property, if it ends
with a %, it will try to open minor devices until and error is detected.
E.g: /dev/dri/card%
I'm a bit on the fence as to whether to use the %, or add a new
hwc.drm.scan_devices property. I guess since we need to convey the path anyways
this is fine, but it really needs to be better documented.
I'm looking at this stuff in another series about DRM Node probing[1],
and I'll look into using properties to define what we are looking for.
But those properties won't be paths, but rather PCI IDs and driver vendor names.
As for what to do in the series, I don't have much of an opinion. But I'm likely
to try to change it in the not too distant future.
[1] https://www.spinics.net/lists/dri-devel/msg172496.html
Additionally, this will be used for finding an available writeback
connector that will be used for the flattening of the currently
displayed scene.
Signed-off-by: Alexandru Gheorghe
---
Android.mk | 1 +
resourcemanager.cpp | 71 +
resourcemanager.h | 29 ++
3 files changed, 101 insertions(+)
create mode 100644 resourcemanager.cpp
create mode 100644 resourcemanager.h
diff --git a/Android.mk b/Android.mk
index 1add286..736fe24 100644
--- a/Android.mk
+++ b/Android.mk
@@ -52,6 +52,7 @@ LOCAL_C_INCLUDES := \
LOCAL_SRC_FILES := \
autolock.cpp \
+ resourcemanager.cpp \
drmresources.cpp \
drmconnector.cpp \
drmcrtc.cpp \
diff --git a/resourcemanager.cpp b/resourcemanager.cpp
new file mode 100644
index 000..e7b654e
--- /dev/null
+++ b/resourcemanager.cpp
@@ -0,0 +1,71 @@
+#include "resourcemanager.h"
+#include
+#include
+
+namespace android {
+
+ResourceManager::ResourceManager() : gralloc_(NULL) {
+}
+
+int ResourceManager::Init() {
+ char path_pattern[PROPERTY_VALUE_MAX];
+ property_get("hwc.drm.device", path_pattern, "/dev/dri/card%");
We probably also don't want to default to scanning, since that might change
behavior in existing boards.
+
+ uint8_t minor = 0;
Please use unsigned/int instead of fixed-size types. Unless the number of bits
is relevant to use of the variable, let the compiler handle it.
+ int last_display_index = 0;
Could we just call this num_displays? It was kind of hard for me to reason
through this, especially when it's call "start_display_index" when you jump into
drm::Init(). I also think drm->Init's return pair should return
instead of , so each time you call
Init(), you're adding to num_displays.
+ int last_char = strlen(path_pattern) - 1;
+ do {
+char path[PROPERTY_VALUE_MAX];
Please use string
+if (path_pattern[last_char] == '%') {
+ path_pattern[last_char] = '\0';
+ snprintf(path, PROPERTY_VALUE_MAX, "%s%d", path_pattern, minor);
+ path_pattern[last_char] = '%';
This doesn't work for minor > 10.
+} else {
+ snprintf(path, PROPERTY_VALUE_MAX, "%s", path_pattern);
+}
+std::unique_ptr drm = std::make_unique();
+last_display_index = drm->Init(this, path, last_display_index);
+if (last_display_index < 0) {
+ break;
+}
+std::shared_ptr importer;
+importer.reset(Importer::CreateInstance(drm.get()));
+if (!importer) {
+ ALOGE("Failed to create importer instance");
+ break;
+}
+importers_.push_back(importer);
+drms_.push_back(std::move(drm));
+minor++;
+last_display_index++;
+ } while (path_pattern[last_char] == '%');
+
+ if (!drms_.size()) {
+ALOGE("Failed to find any working drm device");
+return -EINVAL;
+ }
+
+ return hw_get_module(GRALLOC_HARDWARE_MODULE_ID,
+ (const hw_module_t **)&gralloc_);
+}
Consider the following:
int ResourceManager::AddDrmDevice(std::string path) {
std::unique_ptr drm = std::make_unique();
int displays_added, ret;
std::tie(displays_added, ret) = drm->Init(path.c_str(), num_displays_);
if (ret)
return ret;
std::shared_ptr importer;
importer.reset(Importer::CreateInstance(drm.get()));
if (!importer) {
ALOGE("Failed to create importer instance");
return -ENODEV;
}
importers_.push_back(importer);
drms_.push_back(std::move(drm));
num_displays_ += displays_added;
return 0;
}
int ResourceManager::Init() {
char path_pattern[PROPERTY_VALUE_MAX];
int path_len = property_get("hwc.drm.device", path_pattern,
"/dev/dri/card%");
if (path_pattern[path_len - 1] != '%')
return AddDrmDevice(std::string(path_pattern);
path_pattern[path_len - 1] = '\0';
for (int ret = 0, idx = 0; !ret; ++idx) {
ostringstream path;
path << path_pattern << idx;
ret = Add
Re: [PATCH hwc v2 10/18] drm_hwcomposer: hwcutils: Add function for cloning a DrmHwcLayer
On Wed, Apr 11, 2018 at 04:22:21PM +0100, Alexandru Gheorghe wrote:
> When doing flattening of a composition on a different CRTC we need to be
> able to clone a layer in order to import it and then pass it to another CRTC.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> drmhwcomposer.h | 1 +
> hwcutils.cpp| 11 +++
> 2 files changed, 12 insertions(+)
>
> diff --git a/drmhwcomposer.h b/drmhwcomposer.h
> index f8440fb..b256caf 100644
> --- a/drmhwcomposer.h
> +++ b/drmhwcomposer.h
> @@ -150,6 +150,7 @@ struct DrmHwcLayer {
>
>int InitFromHwcLayer(hwc_layer_1_t *sf_layer, Importer *importer,
> const gralloc_module_t *gralloc);
> + int PopulateFromDrmHwcLayer(DrmHwcLayer *layer);
>int ImportBuffer(Importer *importer, const gralloc_module_t *gralloc);
>
>void SetTransform(int32_t sf_transform);
> diff --git a/hwcutils.cpp b/hwcutils.cpp
> index 53a7d82..ff37c3b 100644
> --- a/hwcutils.cpp
> +++ b/hwcutils.cpp
> @@ -149,6 +149,17 @@ int DrmHwcLayer::InitFromHwcLayer(hwc_layer_1_t
> *sf_layer, Importer *importer,
>return ImportBuffer(importer, gralloc);
> }
>
> +int DrmHwcLayer::PopulateFromDrmHwcLayer(DrmHwcLayer *src_layer) {
> + blending = src_layer->blending;
> + sf_handle = src_layer->sf_handle;
> + acquire_fence = dup(src_layer->acquire_fence.get());
Hmm, I think this is the only place where we duplicate a UniqueFd. I _think_
this will be Ok, but do you need to use this? Could you instead defer trying to
flatten if the original acquire_fence hasn't fired? Also, you don't check the
return value.
> + display_frame = src_layer->display_frame;
> + alpha = src_layer->alpha;
> + source_crop = src_layer->source_crop;
> + transform = src_layer->transform;
It also doesn't seem like you're populating all of the fields so the function
name is misleading.
> + return 0;
> +}
> +
> int DrmHwcLayer::ImportBuffer(Importer *importer,
>const gralloc_module_t *gralloc) {
>int ret = buffer.ImportBuffer(sf_handle, importer);
> --
> 2.7.4
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] gpu: drm: i915: Change return type to vm_fault_t
On Tue, Apr 17, 2018 at 5:29 PM, Jani Nikula
wrote:
> On Tue, 17 Apr 2018, Souptick Joarder wrote:
>> Use new return type vm_fault_t for fault handler. For
>> now, this is just documenting that the function returns
>> a VM_FAULT value rather than an errno. Once all instances
>> are converted, vm_fault_t will become a distinct type.
>>
>> Reference id -> 1c8f422059ae ("mm: change return type to
>> vm_fault_t")
>>
>> Signed-off-by: Souptick Joarder
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
>> drivers/gpu/drm/i915/i915_gem.c | 15 ---
>> 2 files changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index a42deeb..95b0d50 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -51,6 +51,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include "i915_params.h"
>> #include "i915_reg.h"
>> @@ -3363,7 +3364,7 @@ int i915_gem_wait_for_idle(struct drm_i915_private
>> *dev_priv,
>> unsigned int flags);
>> int __must_check i915_gem_suspend(struct drm_i915_private *dev_priv);
>> void i915_gem_resume(struct drm_i915_private *dev_priv);
>> -int i915_gem_fault(struct vm_fault *vmf);
>> +vm_fault_t i915_gem_fault(struct vm_fault *vmf);
>> int i915_gem_object_wait(struct drm_i915_gem_object *obj,
>>unsigned int flags,
>>long timeout,
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c
>> b/drivers/gpu/drm/i915/i915_gem.c
>> index dd89abd..bdac690 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -1882,7 +1882,7 @@ int i915_gem_mmap_gtt_version(void)
>> * The current feature set supported by i915_gem_fault() and thus GTT mmaps
>> * is exposed via I915_PARAM_MMAP_GTT_VERSION (see
>> i915_gem_mmap_gtt_version).
>> */
>> -int i915_gem_fault(struct vm_fault *vmf)
>> +vm_fault_t i915_gem_fault(struct vm_fault *vmf)
>> {
>> #define MIN_CHUNK_PAGES ((1 << 20) >> PAGE_SHIFT) /* 1 MiB */
>> struct vm_area_struct *area = vmf->vma;
>> @@ -1895,6 +1895,7 @@ int i915_gem_fault(struct vm_fault *vmf)
>> pgoff_t page_offset;
>> unsigned int flags;
>> int ret;
>> + vm_fault_t retval;
>
> What's the point of changing the name? An unnecessary change.
int ret;
already exists and is used. You can't also have a vm_fault_t ret; on
top of that :-)
-Daniel
>
> BR,
> Jani.
>
>>
>> /* We don't use vmf->pgoff since that has the fake offset */
>> page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT;
>> @@ -2000,7 +2001,7 @@ int i915_gem_fault(struct vm_fault *vmf)
>>* and so needs to be reported.
>>*/
>> if (!i915_terminally_wedged(&dev_priv->gpu_error)) {
>> - ret = VM_FAULT_SIGBUS;
>> + retval = VM_FAULT_SIGBUS;
>> break;
>> }
>> case -EAGAIN:
>> @@ -2017,21 +2018,21 @@ int i915_gem_fault(struct vm_fault *vmf)
>>* EBUSY is ok: this just means that another thread
>>* already did the job.
>>*/
>> - ret = VM_FAULT_NOPAGE;
>> + retval = VM_FAULT_NOPAGE;
>> break;
>> case -ENOMEM:
>> - ret = VM_FAULT_OOM;
>> + retval = VM_FAULT_OOM;
>> break;
>> case -ENOSPC:
>> case -EFAULT:
>> - ret = VM_FAULT_SIGBUS;
>> + retval = VM_FAULT_SIGBUS;
>> break;
>> default:
>> WARN_ONCE(ret, "unhandled error in i915_gem_fault: %i\n", ret);
>> - ret = VM_FAULT_SIGBUS;
>> + retval = VM_FAULT_SIGBUS;
>> break;
>> }
>> - return ret;
>> + return retval;
>> }
>>
>> static void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
>> --
>> 1.9.1
>>
>
> --
> Jani Nikula, Intel Open Source Technology Center
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/3] drm/prime: Iterate SG DMA addresses separately
Am 17.04.2018 um 17:58 schrieb Robin Murphy:
For dma_map_sg(), DMA API implementations are free to merge consecutive
segments into a single DMA mapping if conditions are suitable, thus the
resulting DMA addresses which drm_prime_sg_to_page_addr_arrays()
iterates may be packed into fewer entries than ttm->sg->nents implies.
The current implementation does not account for this, meaning that its
callers either have to reject the 0 < count < nents case or risk getting
bogus DMA addresses beyond the first segment. Fortunately this is quite
easy to handle without having to rejig structures to also store the
mapped count, since the total DMA length should still be equal to the
total buffer length. All we need is a second scatterlist cursor to
iterate through the DMA addresses independently of the page addresses.
Signed-off-by: Robin Murphy
Reviewed-by: Christian König for the whole
series.
---
v2: Remember to iterate dma_len correctly as well.
drivers/gpu/drm/drm_prime.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 7856a9b3f8a8..b8ca06ea7d80 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -933,16 +933,18 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table
*sgt, struct page **pages,
dma_addr_t *addrs, int max_entries)
{
unsigned count;
- struct scatterlist *sg;
+ struct scatterlist *sg, *dma_sg;
struct page *page;
- u32 len, index;
+ u32 len, dma_len, index;
dma_addr_t addr;
index = 0;
+ dma_sg = sgt->sgl;
+ dma_len = sg_dma_len(dma_sg);
+ addr = sg_dma_address(dma_sg);
for_each_sg(sgt->sgl, sg, sgt->nents, count) {
len = sg->length;
page = sg_page(sg);
- addr = sg_dma_address(sg);
while (len > 0) {
if (WARN_ON(index >= max_entries))
@@ -955,8 +957,15 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt,
struct page **pages,
page++;
addr += PAGE_SIZE;
len -= PAGE_SIZE;
+ dma_len -= PAGE_SIZE;
index++;
}
+
+ if (dma_len == 0) {
+ dma_sg = sg_next(dma_sg);
+ dma_len = sg_dma_len(dma_sg);
+ addr = sg_dma_address(dma_sg);
+ }
}
return 0;
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 11/18] drm_hwcomposer: Add utility functions to copy displaycomposition internals
On Wed, Apr 11, 2018 at 04:22:22PM +0100, Alexandru Gheorghe wrote:
> Add utility functions to copy the DrmHwcLayer and DrmCompositionPlanes
> from another DrmDisplayComposition.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> drmdisplaycomposition.cpp | 29 +
> drmdisplaycomposition.h | 3 +++
> 2 files changed, 32 insertions(+)
>
> diff --git a/drmdisplaycomposition.cpp b/drmdisplaycomposition.cpp
> index 66e67a4..dd64f46 100644
> --- a/drmdisplaycomposition.cpp
> +++ b/drmdisplaycomposition.cpp
> @@ -99,6 +99,35 @@ int DrmDisplayComposition::SetLayers(DrmHwcLayer *layers,
> size_t num_layers,
>return 0;
> }
>
> +int DrmDisplayComposition::CopyLayers(DrmDisplayComposition *src) {
> + geometry_changed_ = true;
> + type_ = DRM_COMPOSITION_TYPE_FRAME;
> + std::shared_ptr importer =
> + drm_->resource_manager()->GetImporter(crtc()->display());
> + if (!importer) {
> +ALOGE("Failed to find a valid importer");
> +return -EINVAL;
> + }
> + for (DrmHwcLayer &src_layer : src->layers()) {
> +DrmHwcLayer copy;
> +copy.PopulateFromDrmHwcLayer(&src_layer);
> +int ret = copy.ImportBuffer(importer.get(),
> +drm_->resource_manager()->GetGralloc());
> +if (ret) {
> + ALOGE("Failed to import buffer ret = %d", ret);
> + return -EINVAL;
> +}
> +layers_.emplace_back(std::move(copy));
> + }
> + return 0;
> +}
This seems to do more than just CopyLayers, and it seems quite specialized to
your purpose. Can you do something similar to SquashFrame(), where the new
composition is crafted in context? It might be a little less awkward than
sprinkling in these seemingly generic copy functions.
Sean
> +
> +void DrmDisplayComposition::CopyCompPlanes(DrmDisplayComposition *src) {
> + for (auto comp_plane : src->composition_planes()) {
> +composition_planes_.push_back(comp_plane);
> + }
> +}
> +
> int DrmDisplayComposition::SetDpmsMode(uint32_t dpms_mode) {
>if (!validate_composition_type(DRM_COMPOSITION_TYPE_DPMS))
> return -EINVAL;
> diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h
> index 9183925..c646420 100644
> --- a/drmdisplaycomposition.h
> +++ b/drmdisplaycomposition.h
> @@ -68,6 +68,7 @@ class DrmCompositionPlane {
>
>DrmCompositionPlane() = default;
>DrmCompositionPlane(DrmCompositionPlane &&rhs) = default;
> + DrmCompositionPlane(const DrmCompositionPlane &rhs) = default;
>DrmCompositionPlane &operator=(DrmCompositionPlane &&other) = default;
>DrmCompositionPlane(Type type, DrmPlane *plane, DrmCrtc *crtc)
>: type_(type), plane_(plane), crtc_(crtc) {
> @@ -120,6 +121,8 @@ class DrmDisplayComposition {
> Planner *planner, uint64_t frame_no);
>
>int SetLayers(DrmHwcLayer *layers, size_t num_layers, bool
> geometry_changed);
> + int CopyLayers(DrmDisplayComposition *src);
> + void CopyCompPlanes(DrmDisplayComposition *src);
>int AddPlaneComposition(DrmCompositionPlane plane);
>int AddPlaneDisable(DrmPlane *plane);
>int SetDpmsMode(uint32_t dpms_mode);
> --
> 2.7.4
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 12/18] drm_hwcomposer: Add utility function to create an initialized composition
On Wed, Apr 11, 2018 at 04:22:23PM +0100, Alexandru Gheorghe wrote:
> There is a lot of boilerplate for creating an initialized
> drmdisplaycomposition. This patch gathers that in a separate method.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> drmdisplaycompositor.cpp | 23 +++
> drmdisplaycompositor.h | 2 ++
> 2 files changed, 25 insertions(+)
>
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index e556e86..6e5be24 100644
> --- a/drmdisplaycompositor.cpp
> +++ b/drmdisplaycompositor.cpp
> @@ -221,6 +221,7 @@ int DrmDisplayCompositor::Init(DrmResources *drm, int
> display) {
> ALOGE("Failed to initialize drm compositor lock %d\n", ret);
> return ret;
>}
> + planner_ = Planner::CreateInstance(drm);
What's this?
>
>initialized_ = true;
>return 0;
> @@ -231,6 +232,28 @@ std::unique_ptr
> DrmDisplayCompositor::CreateComposition()
>return std::unique_ptr(new DrmDisplayComposition());
> }
>
> +std::unique_ptr
> +DrmDisplayCompositor::CreateInitializedComposition() const {
> + DrmCrtc *crtc = drm_->GetCrtcForDisplay(display_);
> + if (!crtc) {
> +ALOGE("Failed to find crtc for display = %d", display_);
> +return std::unique_ptr();
> + }
> + std::unique_ptr comp = CreateComposition();
> + std::shared_ptr importer =
> + drm_->resource_manager()->GetImporter(display_);
> + if (!importer) {
> +ALOGE("Failed to find resources for display = %d", display_);
> +return std::unique_ptr();
> + }
> + int ret = comp->Init(drm_, crtc, importer.get(), planner_.get(), 0);
> + if (ret) {
> +ALOGE("Failed to init composition for display = %d", display_);
> +return std::unique_ptr();
> + }
> + return comp;
> +}
> +
This seems sufficiently small that you can squash it into the patch that uses
it. The same can be said for some of the other "Add function to do X" which
don't use the function.
> std::tuple
> DrmDisplayCompositor::GetActiveModeResolution() {
>DrmConnector *connector = drm_->GetConnectorForDisplay(display_);
> diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
> index f1965fb..ccaffb4 100644
> --- a/drmdisplaycompositor.h
> +++ b/drmdisplaycompositor.h
> @@ -87,6 +87,7 @@ class DrmDisplayCompositor {
>int Init(DrmResources *drm, int display);
>
>std::unique_ptr CreateComposition() const;
> + std::unique_ptr CreateInitializedComposition()
> const;
>int ApplyComposition(std::unique_ptr composition);
>int Composite();
>int SquashAll();
> @@ -155,6 +156,7 @@ class DrmDisplayCompositor {
>// we need to reset them on every Dump() call.
>mutable uint64_t dump_frames_composited_;
>mutable uint64_t dump_last_timestamp_ns_;
> + std::unique_ptr planner_;
> };
> }
>
> --
> 2.7.4
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 13/18] drm_hwcomposer: Pass buffer sizes to Prepareframebuffer
On Wed, Apr 11, 2018 at 04:22:24PM +0100, Alexandru Gheorghe wrote:
> Currently Prepareframebuffer uses the mode of the connected connector
> to decide how big the buffer should be, however when using the
> drmdisplaycompositor just for flattening, the mode had not been set
> yet, so we need a way to pass the desired buffer sizes.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> drmdisplaycompositor.cpp | 7 ---
> drmdisplaycompositor.h | 3 ++-
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index 6e5be24..afd3b05 100644
> --- a/drmdisplaycompositor.cpp
> +++ b/drmdisplaycompositor.cpp
> @@ -268,14 +268,15 @@ DrmDisplayCompositor::GetActiveModeResolution() {
> }
>
> int DrmDisplayCompositor::PrepareFramebuffer(
> -DrmFramebuffer &fb, DrmDisplayComposition *display_comp) {
> +DrmFramebuffer &fb, DrmDisplayComposition *display_comp, uint32_t width,
> +uint32_t height) {
>int ret = fb.WaitReleased(-1);
>if (ret) {
> ALOGE("Failed to wait for framebuffer release %d", ret);
> return ret;
>}
> - uint32_t width, height;
> - std::tie(width, height, ret) = GetActiveModeResolution();
> + if (width == 0 || height == 0)
> +std::tie(width, height, ret) = GetActiveModeResolution();
Just plumb it through at the other callsites.
>if (ret) {
> ALOGE(
> "Failed to allocate framebuffer because the display resolution could
> "
> diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
> index ccaffb4..0f8daad 100644
> --- a/drmdisplaycompositor.h
> +++ b/drmdisplaycompositor.h
> @@ -115,7 +115,8 @@ class DrmDisplayCompositor {
>static const int kAcquireWaitTimeoutMs = 100;
>
>int PrepareFramebuffer(DrmFramebuffer &fb,
> - DrmDisplayComposition *display_comp);
> + DrmDisplayComposition *display_comp,
> + uint32_t width = 0, uint32_t height = 0);
>int ApplySquash(DrmDisplayComposition *display_comp);
>int ApplyPreComposite(DrmDisplayComposition *display_comp);
>int PrepareFrame(DrmDisplayComposition *display_comp);
> --
> 2.7.4
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 14/18] drm_hwcomposer: Fix race in ApplyFrame
On Wed, Apr 11, 2018 at 04:22:25PM +0100, Alexandru Gheorghe wrote:
> ApplyFrame holds the lock just when it swaps the value of
> active_composition_, in a multithread context we could end up in a
> situation where something is shown on the screen, but something else
> is set in active_composition_. Fix it by holding the lock during
> CommitFrame.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> drmdisplaycompositor.cpp | 40 +---
> drmdisplaycompositor.h | 2 +-
> 2 files changed, 18 insertions(+), 24 deletions(-)
>
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index afd3b05..576539b 100644
> --- a/drmdisplaycompositor.cpp
> +++ b/drmdisplaycompositor.cpp
> @@ -791,11 +791,6 @@ std::tuple
> DrmDisplayCompositor::CreateModeBlob(
> }
>
> void DrmDisplayCompositor::ClearDisplay() {
> - AutoLock lock(&lock_, "compositor");
> - int ret = lock.Lock();
> - if (ret)
> -return;
> -
>if (!active_composition_)
> return;
>
> @@ -808,11 +803,25 @@ void DrmDisplayCompositor::ClearDisplay() {
> }
>
> void DrmDisplayCompositor::ApplyFrame(
> -std::unique_ptr composition, int status) {
> +std::unique_ptr composition, int status,
> +bool writeback) {
The writeback argument addition seems unrelated to this change.
> + AutoLock lock(&lock_, __FUNCTION__);
> + if (lock.Lock())
> +return;
>int ret = status;
> -
> - if (!ret)
> + if (!ret) {
> +if (writeback && !CountdownExpired()) {
> + ALOGE("Abort playing back scene");
> + return;
> +}
> ret = CommitFrame(composition.get(), false);
> +if (!ret) {
> + ++dump_frames_composited_;
> + if (active_composition_)
> +active_composition_->SignalCompositionDone();
> + active_composition_.swap(composition);
Why move this stuff?
> +}
> + }
>
>if (ret) {
> ALOGE("Composite failed for display %d", display_);
> @@ -821,21 +830,6 @@ void DrmDisplayCompositor::ApplyFrame(
> ClearDisplay();
> return;
>}
> - ++dump_frames_composited_;
> -
> - if (active_composition_)
> -active_composition_->SignalCompositionDone();
> -
> - ret = pthread_mutex_lock(&lock_);
> - if (ret)
> -ALOGE("Failed to acquire lock for active_composition swap");
> -
> - active_composition_.swap(composition);
> -
> - if (!ret)
> -ret = pthread_mutex_unlock(&lock_);
> - if (ret)
> -ALOGE("Failed to release lock for active_composition swap");
> }
>
> int DrmDisplayCompositor::ApplyComposition(
> diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
> index 0f8daad..b35ef70 100644
> --- a/drmdisplaycompositor.h
> +++ b/drmdisplaycompositor.h
> @@ -127,7 +127,7 @@ class DrmDisplayCompositor {
>
>void ClearDisplay();
>void ApplyFrame(std::unique_ptr composition,
> - int status);
> + int status, bool writeback = false);
>
>std::tuple CreateModeBlob(const DrmMode &mode);
>
> --
> 2.7.4
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 15/18] drm_hwcomposer: Add worker to trigger scene flattenning
On Wed, Apr 11, 2018 at 04:22:26PM +0100, Alexandru Gheorghe wrote:
> Add a vsync worker that calls back into the DrmDisplayCompositor,
> for now at every 60 vsyncs if the scene does not change we trigger
> the flattening of the scene using the writeback connector.
> Other, more complex and proper heuristics could be implemented later
> on.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> drmdisplaycompositor.cpp | 45 ++---
> drmdisplaycompositor.h | 12 +++-
> 2 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index 576539b..e535e8a 100644
> --- a/drmdisplaycompositor.cpp
> +++ b/drmdisplaycompositor.cpp
> @@ -39,6 +39,20 @@
>
> namespace android {
>
> +class CompositorVsyncCallback : public VsyncCallback {
> + public:
> + CompositorVsyncCallback(DrmDisplayCompositor *compositor)
> + : compositor_(compositor) {
> + }
> +
> + void Callback(int display, int64_t timestamp) {
> +compositor_->Vsync(display, timestamp);
> + }
> +
> + private:
> + DrmDisplayCompositor *compositor_;
> +};
> +
> void SquashState::Init(DrmHwcLayer *layers, size_t num_layers) {
>generation_number_++;
>valid_history_ = 0;
> @@ -183,7 +197,8 @@ DrmDisplayCompositor::DrmDisplayCompositor()
>framebuffer_index_(0),
>squash_framebuffer_index_(0),
>dump_frames_composited_(0),
> - dump_last_timestamp_ns_(0) {
> + dump_last_timestamp_ns_(0),
> + flatten_countdown_(FLATTEN_COUNTDOWN_INIT) {
>struct timespec ts;
>if (clock_gettime(CLOCK_MONOTONIC, &ts))
> return;
> @@ -193,7 +208,7 @@ DrmDisplayCompositor::DrmDisplayCompositor()
> DrmDisplayCompositor::~DrmDisplayCompositor() {
>if (!initialized_)
> return;
> -
> + vsync_worker_.Exit();
>int ret = pthread_mutex_lock(&lock_);
>if (ret)
> ALOGE("Failed to acquire compositor lock %d", ret);
> @@ -222,7 +237,9 @@ int DrmDisplayCompositor::Init(DrmResources *drm, int
> display) {
> return ret;
>}
>planner_ = Planner::CreateInstance(drm);
> -
> + vsync_worker_.Init(drm_, display_);
> + auto callback = std::make_shared(this);
> + vsync_worker_.RegisterCallback(callback);
>initialized_ = true;
>return 0;
> }
> @@ -896,6 +913,10 @@ int DrmDisplayCompositor::ApplyComposition(
>return ret;
> }
>
> +int DrmDisplayCompositor::FlattenScene() {
> + return -EINVAL;
> +}
Hmm... not sure this is a useful inclusion.
> +
> int DrmDisplayCompositor::SquashAll() {
>AutoLock lock(&lock_, "compositor");
>int ret = lock.Lock();
> @@ -1044,6 +1065,24 @@ move_layers_back:
>return ret;
> }
>
> +bool DrmDisplayCompositor::CountdownExpired() const {
> + return flatten_countdown_ <= 0;
> +}
> +
> +void DrmDisplayCompositor::Vsync(int display, int64_t timestamp) {
> + AutoLock lock(&lock_, __FUNCTION__);
We use __func__ elsewhere, we should probably be consistent (same goes for other
instances).
> + if (lock.Lock())
> +return;
> + flatten_countdown_--;
> + if (CountdownExpired()) {
Doing:
if (--flatten_countdown_ > 0)
return;
Allows you to remove the CountdownExpired() function and save a level of
indentation for the rest of the function.
> +lock.Unlock();
> +int ret = FlattenScene();
> +ALOGI("scene flattening triggered for display %d at timestamp %" PRIu64
> + " result = %d \n",
> + display, timestamp, ret);
> + }
> +}
> +
> void DrmDisplayCompositor::Dump(std::ostringstream *out) const {
>int ret = pthread_mutex_lock(&lock_);
>if (ret)
> diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
> index b35ef70..26201b9 100644
> --- a/drmdisplaycompositor.h
> +++ b/drmdisplaycompositor.h
> @@ -29,11 +29,16 @@
>
> #include
> #include
> +#include
"vsyncworker.h"
>
> // One for the front, one for the back, and one for cases where we need to
> // squash a frame that the hw can't display with hw overlays.
> #define DRM_DISPLAY_BUFFERS 3
>
> +// If a scene is still for this number of vblanks flatten it to reduce power
> +// consumption.
> +#define FLATTEN_COUNTDOWN_INIT 60
> +
> namespace android {
>
> class GLWorkerCompositor;
> @@ -92,7 +97,7 @@ class DrmDisplayCompositor {
>int Composite();
>int SquashAll();
>void Dump(std::ostringstream *out) const;
> -
> + void Vsync(int display, int64_t timestamp);
>std::tuple GetActiveModeResolution();
>
>SquashState *squash_state() {
> @@ -128,6 +133,9 @@ class DrmDisplayCompositor {
>void ClearDisplay();
>void ApplyFrame(std::unique_ptr composition,
>int status, bool writeback = false);
> + int FlattenScene();
> +
> + bool CountdownExpired() const;
>
>std::tuple CreateModeBlob(const DrmMode &mode);
>
> @@ -157,6 +165,8 @@ class DrmDisplayCompositor {
>// we need to reset them on every Dump() call.
>mutable uint64_t dump_frames_composited_;
>mutable uint64_t dump_
Re: [PATCH hwc v2 16/18] drm_hwcomposer: Find writeback connector for scene flattening
On Wed, Apr 11, 2018 at 04:22:27PM +0100, Alexandru Gheorghe wrote:
> Add logic for finding a suitable writeback connector, there are two
> possibilities for finding an usable writeback connector:
>
> 1) Attached to the same CRTC as the display and can function
>concurrently with the display connector.
>
> 2) On a different CRTC and the display connector is not used (state !=
> DRM_MODE_CONNECTED). What's not handled here and should be handle is
> what happens if connector changes state while flattening, but since
> hotplug is not wired yet, it's not something we should worry about.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> drmresources.cpp| 25 +
> drmresources.h | 2 +-
> resourcemanager.cpp | 24
> resourcemanager.h | 1 +
> 4 files changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drmresources.cpp b/drmresources.cpp
> index fef6835..70126a4 100644
> --- a/drmresources.cpp
> +++ b/drmresources.cpp
> @@ -269,6 +269,31 @@ DrmConnector
> *DrmResources::GetWritebackConnectorForDisplay(int display) const {
>return NULL;
> }
>
> +// TODO what happens when hotplugging
> +DrmConnector *DrmResources::AvailableWritebackConnector(int display) const {
> + DrmConnector *writeback_conn = GetWritebackConnectorForDisplay(display);
> + DrmConnector *display_conn = GetConnectorForDisplay(display);
> + // If we have a writeback already attached to the same CRTC, just use
> that, if
> + // possible
> + if (display_conn && writeback_conn &&
> + writeback_conn->encoder()->can_clone(display_conn->encoder()))
> +return writeback_conn;
> +
> + // Use another CRTC if available and doesn't have any connector
> + for (auto &crtc : crtcs_) {
> +if (crtc->display() == display)
> + continue;
> +display_conn = GetConnectorForDisplay(crtc->display());
> +// If we have a display connected don't use it for writeback
> +if (display_conn && display_conn->state() == DRM_MODE_CONNECTED)
> + continue;
> +writeback_conn = GetWritebackConnectorForDisplay(crtc->display());
> +if (writeback_conn)
> + return writeback_conn;
> + }
> + return NULL;
> +}
> +
> DrmCrtc *DrmResources::GetCrtcForDisplay(int display) const {
>for (auto &crtc : crtcs_) {
> if (crtc->display() == display)
> diff --git a/drmresources.h b/drmresources.h
> index 4fb17fc..9176b8e 100644
> --- a/drmresources.h
> +++ b/drmresources.h
> @@ -60,7 +60,7 @@ class DrmResources {
>
>DrmConnector *GetConnectorForDisplay(int display) const;
>DrmConnector *GetWritebackConnectorForDisplay(int display) const;
> - DrmConnector *FindWritebackConnector(int display) const;
What's up with this?
> + DrmConnector *AvailableWritebackConnector(int display) const;
>DrmCrtc *GetCrtcForDisplay(int display) const;
>DrmPlane *GetPlane(uint32_t id) const;
>DrmEventListener *event_listener();
> diff --git a/resourcemanager.cpp b/resourcemanager.cpp
> index e7b654e..b2a4458 100644
> --- a/resourcemanager.cpp
> +++ b/resourcemanager.cpp
> @@ -49,6 +49,30 @@ int ResourceManager::Init() {
> (const hw_module_t **)&gralloc_);
> }
>
> +DrmConnector *ResourceManager::AvailableWritebackConnector(int display) {
> + DrmResources *drm_resource = GetDrmResources(display);
> + DrmConnector *writeback_conn = NULL;
> + if (drm_resource) {
> +writeback_conn = drm_resource->AvailableWritebackConnector(display);
> +if (writeback_conn) {
> + ALOGI("Use writeback connected to display %d\n",
> +writeback_conn->display());
This seems a little chatty, it's written every flatten, yeah?
> + return writeback_conn;
> +}
> + }
> + for (auto &drm : drms_) {
> +if (drm.get() == drm_resource)
> + continue;
> +writeback_conn = drm->AvailableWritebackConnector(display);
> +if (writeback_conn) {
> + ALOGI("Use writeback connected to display %d\n",
> +writeback_conn->display());
> + return writeback_conn;
> +}
> + }
> + return writeback_conn;
> +}
> +
> DrmResources *ResourceManager::GetDrmResources(int display) {
>for (uint32_t i = 0; i < drms_.size(); i++) {
> if (drms_[i]->HandlesDisplay(display))
> diff --git a/resourcemanager.h b/resourcemanager.h
> index b8caa9a..57f7a2a 100644
> --- a/resourcemanager.h
> +++ b/resourcemanager.h
> @@ -18,6 +18,7 @@ class ResourceManager {
>DrmResources *GetDrmResources(int display);
>std::shared_ptr GetImporter(int display);
>const gralloc_module_t *GetGralloc();
> + DrmConnector *AvailableWritebackConnector(int display);
>
> private:
>std::vector> drms_;
> --
> 2.7.4
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer
On Tue, Apr 17, 2018 at 10:45:07AM +0530, Nautiyal, Ankit K wrote:
>
> On 4/6/2018 11:14 PM, Ville Syrjälä wrote:
> > On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:
> >> This patch is causing failure of IGT test kms_3d. The kms_3d test
> >> expects the no. of 3d modes to be 13.
> >>
> >> (The test has hard-coded value for expected no. of 3d modes as 13)
> >>
> >> But due to the addition of "matching aspect_ratio" in drm_mode_equal in
> >> this patch, the total no. of
> >>
> >> modes in the connector modelist is increased by 2, resulting in failure
> >> of assertion 'mode_count==13'.
> > If kms_3d isn't setting the aspect ratio cap how is it affected by these
> > changes?
> In drm_mode.c, the drm_mode_connector_list_update() uses drm_mode_equal,
> to remove duplicate modes in connector_modes from the
> connector->probe_modes.
> Earlier, it wasn't matching for aspect-ratio and was discarding two of
> the modes with aspect ratio,
> as duplicates of other modes in the list.
>
> Later, when we are pruning the modes in drm_mode_get_connector, the
> logic there assumes,
> that the modes are in a sorted order so that we just match with the last
> valid mode for uniqueness.
> This isn't the case with the spoofed edid in kms_3d.
> Earlier, I was thinking if we should change the no. of expected modes in
> kms_3d,
> but that's not correct approach.
>
> So finally, The pruning logic needs to be changed, to do away with any
> assumption and check
> all the modes in the list for duplicates. This however will take more
> time to remove duplicates.
>
> Any other suggestions on this?
What are all the modes this EDID gives us? The order in which the
modes are listed in the EDID should not be relevant as we sort
the mode list ourselves, and thus similar modes should appear back to
back on the list. So I don't really understand how we fail to
discard these two modes.
>
> Regards
> -Ankit
>
> >
> >> Perhaps this need to be handled in the test.
> >>
> >> -Regards,
> >>
> >> Ankit
> >>
> >>
> >> On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:
> >>> From: "Sharma, Shashank"
> >>>
> >>> Current DRM layer functions don't parse aspect ratio information
> >>> while converting a user mode->kernel mode or vice versa. This
> >>> causes modeset to pick mode with wrong aspect ratio, eventually
> >>> causing failures in HDMI compliance test cases, due to wrong VIC.
> >>>
> >>> This patch adds aspect ratio information in DRM's mode conversion
> >>> and mode comparision functions, to make sure kernel picks mode
> >>> with right aspect ratio (as per the VIC).
> >>>
> >>> Background:
> >>> This patch was once reviewed and merged, and later reverted due to
> >>> lack of DRM cap protection. This is a re-spin of this patch, this
> >>> time with DRM cap protection, to avoid aspect ratio information, when
> >>> the client doesn't request for it.
> >>>
> >>> Review link: https://pw-emeril.freedesktop.org/patch/104068/
> >>> Background discussion: https://patchwork.kernel.org/patch/9379057/
> >>>
> >>> Signed-off-by: Shashank Sharma
> >>> Signed-off-by: Lin, Jia
> >>> Signed-off-by: Akashdeep Sharma
> >>> Reviewed-by: Jim Bride (V2)
> >>> Reviewed-by: Jose Abreu (V4)
> >>>
> >>> Cc: Ville Syrjala
> >>> Cc: Jim Bride
> >>> Cc: Jose Abreu
> >>> Cc: Ankit Nautiyal
> >>>
> >>> V3: modified the aspect-ratio check in drm_mode_equal as per new flags
> >>> provided by Ville. https://patchwork.freedesktop.org/patch/188043/
> >>> V4: rebase
> >>> V5: rebase
> >>> V6: As recommended by Ville, avoided matching of aspect-ratio in
> >>> drm_fb_helper, while trying to find a common mode among connectors
> >>> for the target clone mode.
> >>> V7: rebase
> >>> V8: rebase
> >>> V9: rebase
> >>> V10: rebase
> >>> ---
> >>>drivers/gpu/drm/drm_fb_helper.c | 12 ++--
> >>>drivers/gpu/drm/drm_modes.c | 35
> >>> ++-
> >>>2 files changed, 44 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> >>> b/drivers/gpu/drm/drm_fb_helper.c
> >>> index 0646b10..2ee1eaa 100644
> >>> --- a/drivers/gpu/drm/drm_fb_helper.c
> >>> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >>> @@ -2183,7 +2183,11 @@ static bool drm_target_cloned(struct drm_fb_helper
> >>> *fb_helper,
> >>> for (j = 0; j < i; j++) {
> >>> if (!enabled[j])
> >>> continue;
> >>> - if (!drm_mode_equal(modes[j], modes[i]))
> >>> + if (!drm_mode_match(modes[j], modes[i],
> >>> + DRM_MODE_MATCH_TIMINGS |
> >>> + DRM_MODE_MATCH_CLOCK |
> >>> + DRM_MODE_MATCH_FLAGS |
> >>> + DRM_MODE_MATCH_3D_FLAGS))
> >>> can_clone = false;
> >>> }
> >>> }
> >>> @@ -2203,7 +2207,11 @@ stat
Re: [PATCH hwc v2 17/18] drm_hwcomposer: Flatten scene synchronously
On Wed, Apr 11, 2018 at 04:22:28PM +0100, Alexandru Gheorghe wrote:
> Flatten scene on the same CRTC as the one driving the display.
> The active composition is played back to the display with a buffer
> attached to the writeback connector.
> Then we build a composition that has only one plane enabled and that
> uses the result of the writeback as the input.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> drmdisplaycompositor.cpp | 203
> +--
> drmdisplaycompositor.h | 7 +-
> 2 files changed, 204 insertions(+), 6 deletions(-)
>
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index e535e8a..cb670e6 100644
> --- a/drmdisplaycompositor.cpp
> +++ b/drmdisplaycompositor.cpp
> @@ -36,6 +36,7 @@
> #include "drmplane.h"
> #include "drmresources.h"
> #include "glworker.h"
> +static const uint32_t kWaitWritebackFence = 100; // ms
>
> namespace android {
>
> @@ -523,7 +524,9 @@ int
> DrmDisplayCompositor::PrepareFrame(DrmDisplayComposition *display_comp) {
> }
>
> int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> - bool test_only) {
> + bool test_only,
> + DrmDisplayComposition *writeback_comp,
> + DrmConnector *writeback_conn) {
>ATRACE_CALL();
>
>int ret = 0;
> @@ -532,6 +535,7 @@ int
> DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
>std::vector &comp_planes =
>display_comp->composition_planes();
>uint64_t out_fences[drm_->crtcs().size()];
> + int writeback_fence = -1;
>
>DrmConnector *connector = drm_->GetConnectorForDisplay(display_);
>if (!connector) {
> @@ -550,9 +554,37 @@ int
> DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> return -ENOMEM;
>}
>
> + if (writeback_comp != NULL) {
> +if (writeback_conn == NULL)
> + return -EINVAL;
> +if (writeback_conn->writeback_fb_id().id() == 0 ||
> +writeback_conn->writeback_out_fence().id() == 0) {
> + ALOGE("Writeback properties don't exit");
> + return -EINVAL;
> +}
> +if (writeback_comp->layers().size() != 1) {
> + ALOGE("Invalid number of layers for writeback composition");
> + return -EINVAL;
> +}
> +ret = drmModeAtomicAddProperty(
> +pset, writeback_conn->id(), writeback_conn->writeback_fb_id().id(),
> +writeback_comp->layers().back().buffer->fb_id);
> +if (ret < 0) {
> + ALOGE("Failed to add writeback_fb_id");
> + return ret;
> +}
> +ret = drmModeAtomicAddProperty(pset, writeback_conn->id(),
> +
> writeback_conn->writeback_out_fence().id(),
> + (uint64_t)&writeback_fence);
Upcasting int to u64 isn't a great idea, please go the other way (as we do with
out_fences).
> +if (ret < 0) {
> + ALOGE("Failed to add writeback_out_fence");
> + return ret;
> +}
> + }
This would be more readable if it was split off into a function.
>if (crtc->out_fence_ptr_property().id() != 0) {
> -ret = drmModeAtomicAddProperty(pset, crtc->id(),
> crtc->out_fence_ptr_property().id(),
> - (uint64_t) &out_fences[crtc->pipe()]);
> +ret = drmModeAtomicAddProperty(pset, crtc->id(),
> + crtc->out_fence_ptr_property().id(),
> + (uint64_t)&out_fences[crtc->pipe()]);
> if (ret < 0) {
>ALOGE("Failed to add OUT_FENCE_PTR property to pset: %d", ret);
>drmModeAtomicFree(pset);
> @@ -580,6 +612,15 @@ int
> DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> }
>}
>
> + if (writeback_conn != NULL) {
> +ret = drmModeAtomicAddProperty(pset, writeback_conn->id(),
> + writeback_conn->crtc_id_property().id(),
> + crtc->id());
> +if (ret < 0) {
> + ALOGE("Failed to attach writeback");
> +}
> + }
Can you do this above with the rest of the writeback properties?
> +
>for (DrmCompositionPlane &comp_plane : comp_planes) {
> DrmPlane *plane = comp_plane.plane();
> DrmCrtc *crtc = comp_plane.crtc();
> @@ -729,8 +770,18 @@ int
> DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
>
>if (!ret) {
> uint32_t flags = DRM_MODE_ATOMIC_ALLOW_MODESET;
> -if (test_only)
> +if (test_only) {
>flags |= DRM_MODE_ATOMIC_TEST_ONLY;
> +} else {
> + if (writeback_comp != NULL) {
> +if (!CountdownExpired() && active_composition_) {
Given that we're holding the lock throughout this function, can't you just
abort at the start?
> + ALOGE("Writeback composition not needed, abort commit");
> + drmModeAtomicFree(pset);
> + return -EINVAL;
> +
Re: [PATCH] drm/i915: Fix LSPCON TMDS output buffer enabling from low-power state
On Mon, Apr 16, 2018 at 06:53:09PM +0300, Imre Deak wrote:
> LSPCON adapters in low-power state may ignore the first I2C write during
> TMDS output buffer enabling, resulting in a blank screen even with an
> otherwise enabled pipe. Fix this by reading back and validating the
> written value a few times.
>
> The problem was noticed on GLK machines with an onboard LSPCON adapter
> after entering/exiting DC5 power state. Doing an I2C read of the adapter
> ID as the first transaction - instead of the I2C write to enable the
> TMDS buffers - returns the correct value. Based on this we assume that
> the transaction itself is sent properly, it's only the adapter that is
> not ready for some reason to accept this first write after waking from
> low-power state. In my case the second I2C write attempt always
> succeeded.
Yeah, I guess this is an OK approach. Probably not much point in
optimizing the non-LSPCON case since this is modeset stuff,
although we do have the adaptor type passed in so we could if we
wanted to.
Reviewed-by: Ville Syrjälä
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105854
> Cc: Clinton Taylor
> Cc: Ville Syrjälä
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/drm_dp_dual_mode_helper.c | 39
> +--
> 1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> index 02a50929af67..e7f4fe2848a5 100644
> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> @@ -350,19 +350,44 @@ int drm_dp_dual_mode_set_tmds_output(enum
> drm_dp_dual_mode_type type,
> {
> uint8_t tmds_oen = enable ? 0 : DP_DUAL_MODE_TMDS_DISABLE;
> ssize_t ret;
> + int retry;
>
> if (type < DRM_DP_DUAL_MODE_TYPE2_DVI)
> return 0;
>
> - ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_TMDS_OEN,
> - &tmds_oen, sizeof(tmds_oen));
> - if (ret) {
> - DRM_DEBUG_KMS("Failed to %s TMDS output buffers\n",
> - enable ? "enable" : "disable");
> - return ret;
> + /*
> + * LSPCON adapters in low-power state may ignore the first write, so
> + * read back and verify the written value a few times.
> + */
> + for (retry = 0; retry < 3; retry++) {
> + uint8_t tmp;
> +
> + ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_TMDS_OEN,
> + &tmds_oen, sizeof(tmds_oen));
> + if (ret) {
> + DRM_DEBUG_KMS("Failed to %s TMDS output buffers (%d
> attempts)\n",
> + enable ? "enable" : "disable",
> + retry + 1);
> + return ret;
> + }
> +
> + ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_TMDS_OEN,
> + &tmp, sizeof(tmp));
> + if (ret) {
> + DRM_DEBUG_KMS("I2C read failed during TMDS output
> buffer %s (%d attempts)\n",
> + enable ? "enabling" : "disabling",
> + retry + 1);
> + return ret;
> + }
> +
> + if (tmp == tmds_oen)
> + return 0;
> }
>
> - return 0;
> + DRM_DEBUG_KMS("I2C write value mismatch during TMDS output buffer %s\n",
> + enable ? "enabling" : "disabling");
> +
> + return -EIO;
> }
> EXPORT_SYMBOL(drm_dp_dual_mode_set_tmds_output);
>
> --
> 2.13.2
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105083] Random blinking display
https://bugs.freedesktop.org/show_bug.cgi?id=105083 Harry Wentland changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #18 from Harry Wentland --- A fix should be in drm-next-4.17 of Alex's git repo at https://cgit.freedesktop.org/~agd5f/linux/?h=drm-next-4.17 That fix should also make it into 4.16 stable. White color fix with redshift/nightshift should also be in that branch. -- 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 199319] Flickering screen on AMDGPU and DC with Linux 4.16-2
https://bugzilla.kernel.org/show_bug.cgi?id=199319 --- Comment #16 from Harry Wentland (harry.wentl...@amd.com) --- A fix should be in drm-next-4.17 of Alex's git repo at https://cgit.freedesktop.org/~agd5f/linux/?h=drm-next-4.17 and should make it into Linus's tree from there. That fix should also make it into 4.16 stable. -- 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 105177] amdgpu wrong colors with rx460 connected via hdmi
https://bugs.freedesktop.org/show_bug.cgi?id=105177 --- Comment #22 from Harry Wentland --- Please keep one issue per ticket. Open a new ticket if needed. That said, a fix for the flickering issue should be in drm-next-4.17 of Alex's git repo at https://cgit.freedesktop.org/~agd5f/linux/?h=drm-next-4.17 and should make it into Linus's tree from there. That fix should also make it into 4.16 stable. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC] drm: Add ASPEED GFX driver
Joel Stanley writes:
> This driver is for the ASPEED BMC SoC's GFX display hardware. This
> driver runs on the ARM based BMC systems, unlike the ast driver which
> runs on a host CPU and is is for a PCIe graphics device that happens to
> live in the BMC's silicon, but is otherwise available for use by the BMC.
>
> The AST2500 supports a total of 3 output paths:
>
> 1. VGA output (aka host PCIe graphics device), the output target can
> choose either or both to the DAC or DVO interface.
>
> 2. Graphics CRT output, the output target can choose either or both to
> the DAC or DVO interface.
>
> 3. Video input from DVO, the video input can be used for video engine
> capture or DAC display output.
>
> Output options for are selected in SCU2C. This must be toggled by the
> BMC in order to select between the host and BMC's display output.
>
> The "VGA mode" device is the PCI attached controller. The "Graphics CRT"
> is the BMC's internal display controller.
>
> This driver only supports a simple configuration consisting of a 40MHz
> pixel clock (fixed by hardware limitations) and the VGA output path.
The confusing part of this driver to me is understanding where this
display output is going -- if you're going out a VGA connector to a
monitor (is that what "DAC" meant?), we ought to have
DRM_MODE_CONNECTOR_VGA and a .get_modes that does I2C to get the EDID.
If it's fed into the input of some other display controller, then I'm
not sure what's the right thing to do.
I'd recommend moving some of this commit message into kerneldoc in the
_drv.c explaining what the HW does and what parts of the HW the driver
exposes.
> Signed-off-by: Joel Stanley
> ---
>
> Hello!
>
> This driver is working on hardware, with a few oddities. The things that
> I know need cleaning up are:
>
> - clocks: The device can source a clock from three different sources,
>but they depend on the configuration of other devices in the system
>(the USB PHY and the 'host' display controller). For this reason we
>limit the driver to the 40MHz pixel clock that comes from the USB
>PHY.
You may want to look into setting up a little mux clock provider that
can source from whatever parents are available to get you the best,
faster-than-requested clock. Then, if the rate it gives you is too far
off from what you wanted, maybe stretch the hblank intervals of your
modeline to get as close to the target vrefresh as possible (check out
vc4_dsi.c:vc4_dsi_encoder_mode_fixup() for an example of doing this)
> - Setting an initial mode. I can only get the system to output when
>running X. fbset and fb-test can't seem to do this on their own. The
>system this is being developed for intends to run a simple fbterm
>based application, so this needs fixing.
Not sure what would be going on here. You do have all of the DRM FB
emulation bits enabled, right?
> I wanted to get feedback on the way the driver is structured, and if
> possible a some advice on why fb tools can't set a mode themselves.
drm_simple_display_pipe seems like a good way to get started.
Eventually to merge you'll need a DT binding document
(Documentation/devicetree/bindings/display)
> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h
> b/drivers/gpu/drm/aspeed/aspeed_gfx.h
> new file mode 100644
> index ..fb56e425bd48
> --- /dev/null
> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2018 IBM Corporation
> +
> +#include
> +#include
> +
> +struct aspeed_gfx {
> + void __iomem*base;
> + struct clk *clk;
> + struct reset_control*rst;
> + struct regmap *scu;
> +
> + struct drm_simple_display_pipe pipe;
> + struct drm_connectorconnector;
> + struct drm_fbdev_cma*fbdev;
> +};
> +
> +int aspeed_gfx_create_pipe(struct drm_device *drm);
> +int aspeed_gfx_create_output(struct drm_device *drm);
> +
> +#define CRT_CTRL10x60 /* CRT Control I */
> +#define CRT_CTRL20x64 /* CRT Control II */
> +#define CRT_STATUS 0x68 /* CRT Status */
> +#define CRT_MISC 0x6c /* CRT Misc Setting */
> +#define CRT_HORIZ0 0x70 /* CRT Horizontal Total & Display Enable
> End */
> +#define CRT_HORIZ1 0x74 /* CRT Horizontal Retrace Start & End */
> +#define CRT_VERT00x78 /* CRT Vertical Total & Display Enable End
> */
> +#define CRT_VERT10x7C /* CRT Vertical Retrace Start & End */
> +#define CRT_ADDR 0x80 /* CRT Display Starting Address */
> +#define CRT_OFFSET 0x84 /* CRT Display Offset & Terminal Count */
> +#define CRT_THROD0x88 /* CRT Threshold */
> +#define CRT_XSCALE 0x8C /* CRT Scaling-Up Factor */
> +#define CRT_CURSOR0 0x90 /* CRT Hardware Cursor X & Y Offset */
> +#define CRT_CURSOR1 0x94 /* CRT Hardware Cursor X & Y Position *
[Bug 104412] RX 460 HDMI 4k 60fps not working, DisplayPort is.
https://bugs.freedesktop.org/show_bug.cgi?id=104412 --- Comment #18 from Harry Wentland --- Please keep one issue per ticket. Open a new ticket if needed. That said, a fix for the flickering issue should be in drm-next-4.17 of Alex's git repo at https://cgit.freedesktop.org/~agd5f/linux/?h=drm-next-4.17 and should make it into Linus's tree from there. That fix should also make it into 4.16 stable. -- 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 106110] vaapi encoding with gstreamer 1.14 doesn't work
https://bugs.freedesktop.org/show_bug.cgi?id=106110 Bug ID: 106110 Summary: vaapi encoding with gstreamer 1.14 doesn't work Product: Mesa Version: git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: haa...@frickel.club QA Contact: dri-devel@lists.freedesktop.org It could also be a gstreamer-vaapi bug since it happens only since gstreamer 1.14, but I have seen nobody else complain anywhere, so it must work for some people at least. RX 480, recent mesa git revisions, Linux 4.15 - drm-next-4.18-wip To test: gst-launch-1.0 videotestsrc ! vaapih264enc ! "video/x-h264,profile=baseline" ! h264parse ! matroskamux ! filesink location=output.mkv With gstreamer 1.14 this never starts encoding and produces a 0 byte file when cancelling with ctrl+c. With gstreamer 1.12.4 it worked fine. -- 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: [Freedreno] [DPU PATCH v2 2/2] drm/panel: add backlight control support for truly panel
Hi Bjorn
Thanks for the comments.
Reply inline.
On 2018-04-16 23:13, Bjorn Andersson wrote:
On Mon 16 Apr 15:45 PDT 2018, abhin...@codeaurora.org wrote:
Hi Bjorn
Thanks for the review.
Reply inline.
On 2018-04-16 09:41, Bjorn Andersson wrote:
> On Sat 14 Apr 00:25 PDT 2018, Abhinav Kumar wrote:
>
> > Register truly panel as a backlight led device and
> > provide methods to control its backlight operation.
> >
> > Changes in v2:
> > - Removed redundant NULL checks
> > - Arranged headers alphabetically
> > - Formatting fixes
>
> The change log goes below the "---" line.
>
[Abhinav] As sean mentioned, its quite common to list it to view it in
the
log.
This practice has been followed by quite a few in DRM
Another reference here https://patchwork.freedesktop.org/patch/211361/
If that's the practice in DRM land, then that's what you should do.
> >
> > Signed-off-by: Abhinav Kumar
> > ---
> [..]
> > +static int truly_backlight_setup(struct truly_wqxga *ctx)
> > +{
> > + struct backlight_properties props;
> > + char bl_node_name[BL_NODE_NAME_SIZE];
> > +
> > + if (!ctx->backlight) {
> > + memset(&props, 0, sizeof(props));
> > + props.type = BACKLIGHT_RAW;
> > + props.power = FB_BLANK_UNBLANK;
> > + props.max_brightness = 4096;
> > +
> > + snprintf(bl_node_name, BL_NODE_NAME_SIZE, "panel%u-backlight",
> > + PRIM_DISPLAY_NODE);
> > +
> > + ctx->backlight = backlight_device_register(bl_node_name,
> > + ctx->dev, ctx,
> > + &truly_backlight_device_ops, &props);
> > +
> > + if (IS_ERR_OR_NULL(ctx->backlight)) {
> > + pr_err("Failed to register backlight\n");
> > + ctx->backlight = NULL;
> > + return -ENODEV;
> > + }
> > +
> > + /* Register with the LED driver interface */
> > + led_trigger_register_simple("bkl-trigger", &ctx->wled);
> > +
> > + if (!ctx->wled) {
> > + pr_err("backlight led registration failed\n");
> > + return -ENODEV;
> > + }
>
> It seems like you're registering a backlight driver for the sake of
> invoking the LED backlight trigger to control the WLED.
>
> The WLED is a backlight driver, so all you should have to do is add the
> following line to your probe:
>
>ctx->backlight = devm_of_find_backlight(dev);
>
> and then add "backlight = <&wled>" to your dt node.
>
> Regards,
> Bjorn
[Abhinav] Thats not the only purpose of backlight_device_register().
We want to hook up our panel with the parent backlight driver in
drivers/video/backlight/backlight.c and also route all the
update_backlight_status()
calls through the sysfs of the newly registered node.
The of_find_backlight() method doesnt seem to allow us to register our
own
sysfs method.
Are you saying that you want to be able to create an alias for the wled
entry already in /sys/class/backlight named panel0-backlight?
BTW, this isnt something which we are doing uniquely.
There are other panels which seem to be doing this :
drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
What seems to be going on in the s6e3ha2 driver is that the hardware
has
some sort of builtin backlight control, so the driver is creating a
backlight driver for the purpose of exposing those controls.
Can you please comment on whether we can have our own sysfs without
the device_register()?
If the panel isn't actually a piece of backlight hardware then it
should
not register a backlight device. Why do you need your own sysfs?
Regards,
Bjorn
[Abhinav] This particular panel isnt a piece of backlight hardware.
But, we want to have our own sysfs to give flexibility to our userspace
to write and read stuff for its proprietary uses.
Thats how our downstream has been working so far and hence to maintain
the compatibility would like to have it.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/3] drm/prime: Iterate SG DMA addresses separately
On 17/04/18 17:29, Christian König wrote:
Am 17.04.2018 um 17:58 schrieb Robin Murphy:
For dma_map_sg(), DMA API implementations are free to merge consecutive
segments into a single DMA mapping if conditions are suitable, thus the
resulting DMA addresses which drm_prime_sg_to_page_addr_arrays()
iterates may be packed into fewer entries than ttm->sg->nents implies.
The current implementation does not account for this, meaning that its
callers either have to reject the 0 < count < nents case or risk getting
bogus DMA addresses beyond the first segment. Fortunately this is quite
easy to handle without having to rejig structures to also store the
mapped count, since the total DMA length should still be equal to the
total buffer length. All we need is a second scatterlist cursor to
iterate through the DMA addresses independently of the page addresses.
Signed-off-by: Robin Murphy
Reviewed-by: Christian König for the whole
series.
Thanks Christian.
FWIW, the following *completely untested* hack should in theory give the
AMD IOMMU similar segment-merging behaviour to the arm64 IOMMU DMA ops,
if that helps widen the scope for testing/investigation (I have neither
an AMD/ATI graphics card nor a PCIe-capable arm64 box to hand just at
the moment).
Robin.
->8-
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 2a99f0f14795..60b0e495b567 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2489,11 +2489,11 @@ static int map_sg(struct device *dev, struct
scatterlist *sglist,
int nelems, enum dma_data_direction direction,
unsigned long attrs)
{
- int mapped_pages = 0, npages = 0, prot = 0, i;
+ int mapped_pages = 0, npages = 0, prot = 0, i, count;
struct protection_domain *domain;
struct dma_ops_domain *dma_dom;
- struct scatterlist *s;
- unsigned long address;
+ struct scatterlist *s, *d;
+ unsigned long address, max_seg;
u64 dma_mask;
domain = get_domain(dev);
@@ -2535,7 +2535,28 @@ static int map_sg(struct device *dev, struct
scatterlist *sglist,
s->dma_length = s->length;
}
- return nelems;
+ d = sglist;
+ max_seg = dma_get_max_seg_size(dev);
+ count = 1;
+ nelems -= 1;
+ for_each_sg(sg_next(sglist), s, nelems, i) {
+ dma_addr_t s_dma_addr = s->dma_address;
+ unsigned int s_dma_len = s->dma_length;
+
+ s->dma_address = 0;
+ s->dma_length = 0;
+ if (s_dma_addr == d->dma_address + d->dma_length &&
+ d->dma_length + s_dma_len <= max_seg) {
+ d->dma_length += s_dma_len;
+ } else {
+ d = sg_next(d);
+ d->dma_address = s_dma_addr;
+ d->dma_length = s_dma_len;
+ count++;
+ }
+ }
+
+ return count;
out_unmap:
pr_err("%s: IOMMU mapping error in map_sg (io-pages: %d)\n",
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/3] drm/prime: Iterate SG DMA addresses separately
+ Suravee
From: Robin Murphy
Sent: Tuesday, April 17, 2018 2:22:46 PM
To: Koenig, Christian; amd-...@lists.freedesktop.org;
dri-devel@lists.freedesktop.org
Cc: ok...@codeaurora.org; Deucher, Alexander
Subject: Re: [PATCH v2 1/3] drm/prime: Iterate SG DMA addresses separately
On 17/04/18 17:29, Christian König wrote:
> Am 17.04.2018 um 17:58 schrieb Robin Murphy:
>> For dma_map_sg(), DMA API implementations are free to merge consecutive
>> segments into a single DMA mapping if conditions are suitable, thus the
>> resulting DMA addresses which drm_prime_sg_to_page_addr_arrays()
>> iterates may be packed into fewer entries than ttm->sg->nents implies.
>>
>> The current implementation does not account for this, meaning that its
>> callers either have to reject the 0 < count < nents case or risk getting
>> bogus DMA addresses beyond the first segment. Fortunately this is quite
>> easy to handle without having to rejig structures to also store the
>> mapped count, since the total DMA length should still be equal to the
>> total buffer length. All we need is a second scatterlist cursor to
>> iterate through the DMA addresses independently of the page addresses.
>>
>> Signed-off-by: Robin Murphy
>
> Reviewed-by: Christian König for the whole
> series.
Thanks Christian.
FWIW, the following *completely untested* hack should in theory give the
AMD IOMMU similar segment-merging behaviour to the arm64 IOMMU DMA ops,
if that helps widen the scope for testing/investigation (I have neither
an AMD/ATI graphics card nor a PCIe-capable arm64 box to hand just at
the moment).
Robin.
->8-
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 2a99f0f14795..60b0e495b567 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2489,11 +2489,11 @@ static int map_sg(struct device *dev, struct
scatterlist *sglist,
int nelems, enum dma_data_direction direction,
unsigned long attrs)
{
- int mapped_pages = 0, npages = 0, prot = 0, i;
+ int mapped_pages = 0, npages = 0, prot = 0, i, count;
struct protection_domain *domain;
struct dma_ops_domain *dma_dom;
- struct scatterlist *s;
- unsigned long address;
+ struct scatterlist *s, *d;
+ unsigned long address, max_seg;
u64 dma_mask;
domain = get_domain(dev);
@@ -2535,7 +2535,28 @@ static int map_sg(struct device *dev, struct
scatterlist *sglist,
s->dma_length = s->length;
}
- return nelems;
+ d = sglist;
+ max_seg = dma_get_max_seg_size(dev);
+ count = 1;
+ nelems -= 1;
+ for_each_sg(sg_next(sglist), s, nelems, i) {
+ dma_addr_t s_dma_addr = s->dma_address;
+ unsigned int s_dma_len = s->dma_length;
+
+ s->dma_address = 0;
+ s->dma_length = 0;
+ if (s_dma_addr == d->dma_address + d->dma_length &&
+ d->dma_length + s_dma_len <= max_seg) {
+ d->dma_length += s_dma_len;
+ } else {
+ d = sg_next(d);
+ d->dma_address = s_dma_addr;
+ d->dma_length = s_dma_len;
+ count++;
+ }
+ }
+
+ return count;
out_unmap:
pr_err("%s: IOMMU mapping error in map_sg (io-pages: %d)\n",
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2 v2] drm/pl111: Enable device-specific assigned memory
Linus Walleij writes:
> On Sun, Apr 8, 2018 at 3:08 AM, Eric Anholt wrote:
>
>>> if (of_property_read_u32(dev->of_node, "max-memory-bandwidth",
>>>&priv->memory_bw)) {
>>> dev_info(dev, "no max memory bandwidth specified, assume
>>> unlimited\n");
>>> @@ -275,7 +280,8 @@ static int pl111_amba_probe(struct amba_device
>>> *amba_dev,
>>> priv->regs = devm_ioremap_resource(dev, &amba_dev->res);
>>> if (IS_ERR(priv->regs)) {
>>> dev_err(dev, "%s failed mmio\n", __func__);
>>> - return PTR_ERR(priv->regs);
>>> + ret = PTR_ERR(priv->regs);
>>> + goto mem_rel;
>>> }
>>
>> Shouldn't this error path be jumping to dev_unref, as well? We're
>> trying to match drm_dev_alloc(), right?
>
> OK I fixed.
>
>> I'm still a little bothered that we're allowing imports to pl111 of CMA
>> buffers that we can't scan out. Could we just refuse all
>> .gem_prime_import*() on this platform?
>
> I am sorry but I do not understand how CMA, dmabuf and GEM play
> together to decode this and figure out what to do.
>
> Do you mean that if we find device assigned memory, i.e. that there
> is this special memory which is all the controller can scan out,
> I should just implement .gem_prime_impport() instead of the
> currently assigned drm_gem_prime_import() to something just
> returning return ERR_PTR(-EINVAL); so it always fails?
>
> What about .gem_prime_import_sg_table()? Exporting should
> work fine since the memory is always readable by CPU.
Oh, I think you still want drm_gem_prime_import()'s path for "I'm
importing an fd from this same driver to into another process". So,
yeah, have our .import_sg_table hook throw -EINVAL if called on the
device memory platform, and otherwise call down to
drm_gem_cma_prime_import_sg_table().
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105425] 3D & games produce periodic GPU crashes (Radeon R7 370)
https://bugs.freedesktop.org/show_bug.cgi?id=105425 --- Comment #33 from i...@yahoo.com --- This doesn't sound good. The sshd dying indicates that the kernel or the CPU has hang. If there is GPU shader hang this doesn't happen right away, it usually waits 10 seconds before attempting to reset the GPU and then panics. 1. When the system hangs, do you see LEDs on the keyboard flashing? When kernel panics this is how it signals it. You might need to wait for 10 seconds or minute... 2. It seems that OpenSuse disables "sysrq", google told me that "You can enable it in YaST->Security and Users->Security Center and Hardening..." Alternatively you should be able to enable it with executing this as root: echo 1 > /proc/sys/kernel/sysrq Check if it works with "Alt+PrtScr+h", it should display help message in `dmesg` . 3. After you have sysrq working, try to reproduce the crash, (without apitrace). This is to check if sysrq is working at all during hang and if it does then hopefully getting a kernel panic message in the log. 4. If you cannot get crash messages in the logs/journal, then you might to use `serial console` or `netconsole`. The Serial console is best option, if both computers have their own serial ports and you happen to have a serial cable to connect them. linux-source/Documentation/admin-guide/serial-console.rst Otherwise you might try network console logger, that sends UDP packets to the second computer. linux-source/Documentation/networking/netconsole.txt Setting up these might be tricky, as they might not even be compiled in the stock kernel. So if you need detailed instructions, at least check if they are present as modules or built-in the kernel. zgrep CONFIG_NETCONSOLE /proc/config.gz zgrep SERIAL_8250_CONSOLE /proc/config.gz 5. Disable vsync and run `glxgears` for hours. Leave it to work through the night or something. I just want to know if your computer hangs with that simple 3D. vblank_mode=1 glxgears --- Let me be clear. I want to see the crash messages for only 2 reasons: - To see that there is a kernel crash. - To see if the crash is in the graphics stack. Since the `sshd` stops working, it might be network-card crash. (Multiplayer games, using network...) If the machine just hangs, without actual kernel crash... then it might be hardware problem, but not a graphic card, it might also be MB, CPU, PSU, RAM, etc... -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2 v2] drm/pl111: Support the Versatile Express
On Tue, Apr 17, 2018 at 3:12 PM, Robin Murphy wrote: > On 17/04/18 13:32, Linus Walleij wrote: > [...] >> >> Unfortunately there is just one single vexpress core tile in the >> upstream kernel that define a CLCD controller, the CA9 (4xA9) >> that I am using. All the others just use the MB CLCD. >> >> I am thinking there is some never finished DTS upstreaming >> here that ought to happen so we use the core tile CLCD on some >> other boards as well. > > > Barring custom FPGA images, I think V2P-CA9 *is* the only VExpress tile > implementing PL11x; all the more recent test chips had HDLCD instead. Yup it looks like that. I am restructuring the code to look for any graphics on the core tile and only turn on the motherboard CLCD if there is no CLCD or HDLCD on the core tile, e.g. if that device node has been set to "disabled" or the HDLCD driver is not compiled in but the CLCD driver is. I found some vague mentions of a LCD display that can be connected to the motherboard, so that would then be sitting on the CLCD, is this something you ARM guys have seen around? In that case I guess the VExpress (etc) would actually be able to have two screens, one LCD panel on the motherboard CLCD and a DVI from the core tile. Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 106110] vaapi encoding with gstreamer 1.14 doesn't work
https://bugs.freedesktop.org/show_bug.cgi?id=106110 Christoph Haag changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #1 from Christoph Haag --- remco on #radeon figured out that it works when adding stream-format=byte-stream to the video format, like this: gst-launch-1.0 videotestsrc ! vaapih264enc ! "video/x-h264,profile=baseline,stream-format=byte-stream" ! h264parse ! matroskamux ! filesink location=output.mkv -- 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 105425] 3D & games produce periodic GPU crashes (Radeon R7 370)
https://bugs.freedesktop.org/show_bug.cgi?id=105425 --- Comment #34 from MirceaKitsune --- (In reply to iive from comment #33) Ahhh... you've reminded me of a detail that I have in fact noticed but forgot to mention: After the machine freezes and becomes completely unresponsive, some keyboard leds will indeed turn off after roughly 10 seconds. I only noticed this because I currently have a backlit keyboard that has the lighting controlled by the Scroll Lock LED... I saw that a few seconds after the crash, the keyboard lighting always turns itself off. I cannot connect the computers with a serial cable: I think the motherboards are too modern to have a serial port, and they're at a far distance in opposite rooms. Both computers are connected to the same home router via LAN cables though, and can communicate through local IP... so net console sounds like a good idea, but I've never heard of it before so I'll have to look this up. Your sysrq suggestion seems to have worked! I first did this: echo 1 > /proc/sys/kernel/sysrq Then I pressed 'Alt + PrintScreen + H'. Now dmesg shows me: [265102.938475] sysrq: SysRq : HELP : loglevel(0-9) reboot(b) crash(c) terminate-all-tasks(e) memory-full-oom-kill(f) kill-all-tasks(i) thaw-filesystems(j) sak(k) show-backtrace-all-active-cpus(l) show-memory-usage(m) nice-all-RT-tasks(n) poweroff(o) show-registers(p) show-all-timers(q) unraw(r) sync(s) show-task-states(t) unmount(u) force-fb(V) show-blocked-tasks(w) dump-ftrace-buffer(z) I assume that after the crash, I should first use it to test REISUB? And here is the output of the kernel features you said to check. If they're not there, I'm out of luck on this one, as I don't know how to compile my own kernel and can't risk breaking my machine with dangerous tests. mircea@linux-qz0r:~> zgrep CONFIG_NETCONSOLE /proc/config.gz CONFIG_NETCONSOLE=m CONFIG_NETCONSOLE_DYNAMIC=y mircea@linux-qz0r:~> zgrep SERIAL_8250_CONSOLE /proc/config.gz CONFIG_SERIAL_8250_CONSOLE=y Lastly I'll try glxgears without vsync for a few hours in the next days: I have to leave my computer locked while I'm away from home or sleeping, but can leave it on for roughly 3 hours of the day while I'm around but AFK. I should note that I tried running Xonotic without vsync, and that seems to have made absolutely no difference. Also this likely isn't network related, I always test in a local match with bots and not online multiplayer. -- 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: [DPU PATCH v3 1/2] drm/msm/dsi: check video mode engine status before waiting
On Mon, Apr 16, 2018 at 06:56:45PM -0700, Abhinav Kumar wrote:
> Make sure the video mode engine is on before waiting
> for the video done interrupt.
>
> Changes in v2:
> - Replace pr_err with dev_err
> - Changed error message
>
> Changes in v3:
> - Move the return value check to another
> patch
>
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 7a03a94..8df0d44 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -173,6 +173,7 @@ struct msm_dsi_host {
>
> bool registered;
> bool power_on;
> + bool enabled;
> int irq;
> };
>
> @@ -1001,7 +1002,7 @@ static void dsi_wait4video_eng_busy(struct msm_dsi_host
> *msm_host)
> if (!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO))
> return;
>
> - if (msm_host->power_on) {
> + if (msm_host->power_on && msm_host->enabled) {
> dsi_wait4video_done(msm_host);
> /* delay 4 ms to skip BLLP */
> usleep_range(2000, 4000);
> @@ -2203,7 +2204,7 @@ int msm_dsi_host_enable(struct mipi_dsi_host *host)
>* pm_runtime_put_autosuspend(&msm_host->pdev->dev);
>* }
>*/
> -
> + msm_host->enabled = true;
> return 0;
> }
>
> @@ -2219,7 +2220,7 @@ int msm_dsi_host_disable(struct mipi_dsi_host *host)
>* Reset to disable video engine so that we can send off cmd.
>*/
> dsi_sw_reset(msm_host);
> -
> + msm_host->enabled = false;
I thought this was moving to the start of the function?
> return 0;
> }
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
