Re: [PATCH v2 24/25] drm/mediatek: respect page offset for PRIME mmap calls
Hi, Yongqiang: On Tue, 2019-04-16 at 16:33 +0800, CK Hu wrote: > Hi, Yongqiang: > > On Wed, 2019-03-27 at 14:19 +0800, yongqiang@mediatek.com wrote: > > From: Yongqiang Niu > > > > Respect page offset for PRIME mmap calls > > Reviewed-by: CK Hu This patch looks independent, so I've applied it to mediatek-drm-fixes-5.2 [1], thanks. [1] https://github.com/ckhu-mediatek/linux.git-tags/commits/mediatek-drm-fixes-5.2 Regards, CK > > > > > Signed-off-by: Yongqiang Niu > > --- > > drivers/gpu/drm/mediatek/mtk_drm_gem.c | 7 ++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c > > b/drivers/gpu/drm/mediatek/mtk_drm_gem.c > > index c230237..524e494 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c > > @@ -144,7 +144,6 @@ static int mtk_drm_gem_object_mmap(struct > > drm_gem_object *obj, > > * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap(). > > */ > > vma->vm_flags &= ~VM_PFNMAP; > > - vma->vm_pgoff = 0; > > > > ret = dma_mmap_attrs(priv->dma_dev, vma, mtk_gem->cookie, > > mtk_gem->dma_addr, obj->size, mtk_gem->dma_attrs); > > @@ -183,6 +182,12 @@ int mtk_drm_gem_mmap(struct file *filp, struct > > vm_area_struct *vma) > > > > obj = vma->vm_private_data; > > > > + /* > > +* Set vm_pgoff (used as a fake buffer offset by DRM) to 0 and map the > > +* whole buffer from the start. > > +*/ > > + vma->vm_pgoff = 0; > > + > > return mtk_drm_gem_object_mmap(obj, vma); > > } > > >
Re: [PATCH v2 22/25] drm/mediatek: adjust ddp clock control flow
Hi, Yongqiang: On Tue, 2019-04-16 at 16:24 +0800, CK Hu wrote: > Hi, Yongqiang: > > On Wed, 2019-03-27 at 14:19 +0800, yongqiang@mediatek.com wrote: > > From: Yongqiang Niu > > > > display hardware clock will not unprepare when > > crtc is disable, until crtc is destroyed. > > with this patch, hard clock will disable and unprepare > > at the same time. > > Reviewed-by: CK Hu This patch looks independent, so I've applied it to mediatek-drm-fixes-5.2 [1], thanks. [1] https://github.com/ckhu-mediatek/linux.git-tags/commits/mediatek-drm-fixes-5.2 Regards, CK > > > > > Signed-off-by: Yongqiang Niu > > --- > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 26 ++ > > 1 file changed, 6 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > index 0f97ee3..606c6e2 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > @@ -195,7 +195,7 @@ static int mtk_crtc_ddp_clk_enable(struct mtk_drm_crtc > > *mtk_crtc) > > > > DRM_DEBUG_DRIVER("%s\n", __func__); > > for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) { > > - ret = clk_enable(mtk_crtc->ddp_comp[i]->clk); > > + ret = clk_prepare_enable(mtk_crtc->ddp_comp[i]->clk); > > if (ret) { > > DRM_ERROR("Failed to enable clock %d: %d\n", i, ret); > > goto err; > > @@ -205,7 +205,7 @@ static int mtk_crtc_ddp_clk_enable(struct mtk_drm_crtc > > *mtk_crtc) > > return 0; > > err: > > while (--i >= 0) > > - clk_disable(mtk_crtc->ddp_comp[i]->clk); > > + clk_disable_unprepare(mtk_crtc->ddp_comp[i]->clk); > > return ret; > > } > > > > @@ -215,7 +215,7 @@ static void mtk_crtc_ddp_clk_disable(struct > > mtk_drm_crtc *mtk_crtc) > > > > DRM_DEBUG_DRIVER("%s\n", __func__); > > for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) > > - clk_disable(mtk_crtc->ddp_comp[i]->clk); > > + clk_disable_unprepare(mtk_crtc->ddp_comp[i]->clk); > > } > > > > static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc) > > @@ -615,15 +615,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, > > if (!comp) { > > dev_err(dev, "Component %pOF not initialized\n", node); > > ret = -ENODEV; > > - goto unprepare; > > - } > > - > > - ret = clk_prepare(comp->clk); > > - if (ret) { > > - dev_err(dev, > > - "Failed to prepare clock for component %pOF: > > %d\n", > > - node, ret); > > - goto unprepare; > > + return ret; > > } > > > > mtk_crtc->ddp_comp[i] = comp; > > @@ -649,23 +641,17 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, > > ret = mtk_plane_init(drm_dev, _crtc->planes[zpos], > > BIT(pipe), type); > > if (ret) > > - goto unprepare; > > + return ret; > > } > > > > ret = mtk_drm_crtc_init(drm_dev, mtk_crtc, _crtc->planes[0], > > mtk_crtc->layer_nr > 1 ? _crtc->planes[1] : > > NULL, pipe); > > if (ret < 0) > > - goto unprepare; > > + return ret; > > drm_mode_crtc_set_gamma_size(_crtc->base, MTK_LUT_SIZE); > > drm_crtc_enable_color_mgmt(_crtc->base, 0, false, MTK_LUT_SIZE); > > priv->num_pipes++; > > > > return 0; > > - > > -unprepare: > > - while (--i >= 0) > > - clk_unprepare(mtk_crtc->ddp_comp[i]->clk); > > - > > - return ret; > > } >
[PATCH v3 2/2] drm/komeda: Adds limitation check for AFBC wide block not support Rot90
From: "Lowry Li (Arm Technology China)" Komeda series hardware doesn't support Rot90 for AFBC wide block. So add limitation check to reject it if such configuration has been posted. Signed-off-by: Lowry Li (Arm Technology China) --- drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c | 15 +++ .../gpu/drm/arm/display/komeda/komeda_format_caps.c| 7 ++- .../gpu/drm/arm/display/komeda/komeda_format_caps.h| 8 +++- .../gpu/drm/arm/display/komeda/komeda_framebuffer.c| 18 +- .../gpu/drm/arm/display/komeda/komeda_framebuffer.h| 5 +++-- .../gpu/drm/arm/display/komeda/komeda_pipeline_state.c | 8 +++- drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 2 +- 7 files changed, 48 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c index 1c914f8..4563c2a 100644 --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c @@ -494,11 +494,26 @@ static int d71_enum_resources(struct komeda_dev *mdev) {__HW_ID(6, 7), 0/*DRM_FORMAT_YUV420_10BIT*/, 1,RICH, Rot_ALL_H_V,LYT_NM, AFB_TH}, }; +static bool d71_format_mod_supported(const struct komeda_format_caps *caps, +u32 layer_type, u64 modifier, u32 rot) +{ + uint64_t layout = modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK; + + if ((layout == AFBC_FORMAT_MOD_BLOCK_SIZE_32x8) && + drm_rotation_90_or_270(rot)) { + DRM_DEBUG_ATOMIC("D71 doesn't support ROT90 for WB-AFBC.\n"); + return false; + } + + return true; +} + static void d71_init_fmt_tbl(struct komeda_dev *mdev) { struct komeda_format_caps_table *table = >fmt_tbl; table->format_caps = d71_format_caps_table; + table->format_mod_supported = d71_format_mod_supported; table->n_formats = ARRAY_SIZE(d71_format_caps_table); } diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.c b/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.c index b219514..cd4d9f5 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.c @@ -74,7 +74,8 @@ }; bool komeda_format_mod_supported(struct komeda_format_caps_table *table, -u32 layer_type, u32 fourcc, u64 modifier) +u32 layer_type, u32 fourcc, u64 modifier, +u32 rot) { const struct komeda_format_caps *caps; @@ -85,6 +86,10 @@ bool komeda_format_mod_supported(struct komeda_format_caps_table *table, if (!(caps->supported_layer_types & layer_type)) return false; + if (table->format_mod_supported) + return table->format_mod_supported(caps, layer_type, modifier, + rot); + return true; } diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h b/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h index 96de22e..381e873 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h +++ b/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h @@ -71,10 +71,15 @@ struct komeda_format_caps { * * @n_formats: the size of format_caps list. * @format_caps: format_caps list. + * @format_mod_supported: Optional. Some HW may have special requirements or + * limitations which can not be described by format_caps, this func supply HW + * the ability to do the further HW specific check. */ struct komeda_format_caps_table { u32 n_formats; const struct komeda_format_caps *format_caps; + bool (*format_mod_supported)(const struct komeda_format_caps *caps, +u32 layer_type, u64 modifier, u32 rot); }; extern u64 komeda_supported_modifiers[]; @@ -100,6 +105,7 @@ u32 *komeda_get_layer_fourcc_list(struct komeda_format_caps_table *table, void komeda_put_fourcc_list(u32 *fourcc_list); bool komeda_format_mod_supported(struct komeda_format_caps_table *table, -u32 layer_type, u32 fourcc, u64 modifier); +u32 layer_type, u32 fourcc, u64 modifier, +u32 rot); #endif diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c index d0e713a..5f63dec 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c @@ -240,20 +240,20 @@ struct drm_framebuffer * } /* if the fb can be supported by a specific layer */ -bool komeda_fb_is_layer_supported(struct komeda_fb *kfb, u32 layer_type) +bool komeda_fb_is_layer_supported(struct komeda_fb *kfb, u32 layer_type, + u32 rot) { struct drm_framebuffer
[PATCH v3 1/2] drm/komeda: Add rotation support on Komeda driver
From: "Lowry Li (Arm Technology China)" - Adds rotation property to plane. - Komeda display rotation support diverges from the specific formats, so need to check the user required rotation type with the format caps and reject the commit if it can not be supported. - In the layer validate flow, sets the rotation value to the layer state. If r90 or r270, swap the width and height of the data flow for next stage. Signed-off-by: Lowry Li (Arm Technology China) --- drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h | 11 +++ .../gpu/drm/arm/display/komeda/komeda_pipeline_state.c | 7 +++ drivers/gpu/drm/arm/display/komeda/komeda_plane.c| 16 3 files changed, 34 insertions(+) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h b/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h index bc3b2df36..96de22e 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h +++ b/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h @@ -79,6 +79,17 @@ struct komeda_format_caps_table { extern u64 komeda_supported_modifiers[]; +static inline const char *komeda_get_format_name(u32 fourcc, u64 modifier) +{ + struct drm_format_name_buf buf; + static char name[64]; + + snprintf(name, sizeof(name), "%s with modifier: 0x%llx.", +drm_get_format_name(fourcc, ), modifier); + + return name; +} + const struct komeda_format_caps * komeda_get_format_caps(struct komeda_format_caps_table *table, u32 fourcc, u64 modifier); diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c index db34ea2..34737c0 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c @@ -339,6 +339,13 @@ struct komeda_pipeline_state * /* update the data flow for the next stage */ komeda_component_set_output(>input, >base, 0); + /* +* The rotation has been handled by layer, so adjusted the data flow for +* the next stage. +*/ + if (drm_rotation_90_or_270(st->rot)) + swap(dflow->in_h, dflow->in_w); + return 0; } diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c index 9b87c25..c9f37ff 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c @@ -10,6 +10,7 @@ #include #include "komeda_dev.h" #include "komeda_kms.h" +#include "komeda_framebuffer.h" static int komeda_plane_init_data_flow(struct drm_plane_state *st, @@ -17,6 +18,7 @@ { struct komeda_plane_state *kplane_st = to_kplane_st(st); struct drm_framebuffer *fb = st->fb; + const struct komeda_format_caps *caps = to_kfb(fb)->format_caps; memset(dflow, 0, sizeof(*dflow)); @@ -37,6 +39,15 @@ dflow->in_w = st->src_w >> 16; dflow->in_h = st->src_h >> 16; + dflow->rot = drm_rotation_simplify(st->rotation, caps->supported_rots); + if (!has_bits(dflow->rot, caps->supported_rots)) { + DRM_DEBUG_ATOMIC("rotation(0x%x) isn't supported by %s.\n", +dflow->rot, +komeda_get_format_name(caps->fourcc, + fb->modifier)); + return -EINVAL; + } + dflow->en_img_enhancement = kplane_st->img_enhancement; komeda_complete_data_flow_cfg(dflow); @@ -303,6 +314,11 @@ static int komeda_plane_add(struct komeda_kms_dev *kms, drm_plane_helper_add(plane, _plane_helper_funcs); + err = drm_plane_create_rotation_property(plane, DRM_MODE_ROTATE_0, +layer->supported_rots); + if (err) + goto cleanup; + err = drm_plane_create_alpha_property(plane); if (err) goto cleanup; -- 1.9.1
[PATCH v3 0/2] drm/komeda: Add rotation support on Komeda driver
Hi, This serie aims at adding the support for rotation on Komeda driver. For rotation, D71 doesn't support Rot90 for AFBC wide block. So this patch set also includes the limitation check. This patch series depends on: - https://patchwork.freedesktop.org/series/59915/ - https://patchwork.freedesktop.org/series/58665/ - https://patchwork.freedesktop.org/series/59000/ - https://patchwork.freedesktop.org/series/59002/ - https://patchwork.freedesktop.org/series/59471/ - https://patchwork.freedesktop.org/series/58710/ Changes since v1: - Modify patch denpendency in the comment Changes since v2: - Rebase the code Regards, Lowry Lowry Li (Arm Technology China) (2): drm/komeda: Add rotation support on Komeda driver drm/komeda: Adds limitation check for AFBC wide block not support Rot90 drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c | 15 +++ .../gpu/drm/arm/display/komeda/komeda_format_caps.c | 7 ++- .../gpu/drm/arm/display/komeda/komeda_format_caps.h | 19 ++- .../gpu/drm/arm/display/komeda/komeda_framebuffer.c | 18 +- .../gpu/drm/arm/display/komeda/komeda_framebuffer.h | 5 +++-- .../drm/arm/display/komeda/komeda_pipeline_state.c| 15 ++- drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 18 +- 7 files changed, 82 insertions(+), 15 deletions(-) -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109022] ring gfx timeout during particular shader generation on yuzu emulator
https://bugs.freedesktop.org/show_bug.cgi?id=109022 e88z4 changed: What|Removed |Added Version|18.3|git -- 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 110635] briefly flashing corruption when playing various OGL games
https://bugs.freedesktop.org/show_bug.cgi?id=110635 --- Comment #6 from Andrew Sheldon --- I get similar symptoms with Assassins Creed: Unity run under DXVK (with RADV). The issue doesn't occur with LLVM8, and seems to be a regression in LLVM9 since it worked fine with the last compile of LLVM9 I used (early May). The workarounds don't work in my case (nodma, zerovram) so it might be a separate issue. Here's a screenshot of the glitch: https://imgur.com/aUgSjW1 See top left for most obvious example of it, although you can see blockiness across the image. -- 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/2] drm: imx: Add NWL MIPI DSI host controller support
Hi Lucas, On Mon, May 27, 2019 at 03:36:53PM +0200, Lucas Stach wrote: > We have been looking at how to support DCSS in mainline for a while, > but most of the actual work got pushed behind in schedule due to other > priorities. I have some time to contribute. Would you suggest how I should help here? 1. You guys already have something close to completion and do not need more hands. 2. You guys already have some preliminary code and can use help from others. 3. You guys haven't got anything to start with, but just some design principles that anyone who wants to work on it should consider. Which is the one that you want me to read? > One thing I can can say for certain is that DCSS should not be > integrated into imx-drm. It's a totally different hardware and > downstream clearly shows that it's not a good idea to cram it into imx- > drm. I haven't gone deeper into the vendor code, but from a brief looking I didn't see so many problems with integrating DCSS into imx-drm. It's not so unreasonable to take imx-drm as an imx-display-subsystem which can have multiple CRTCs. So can you please elaborate a bit on why it's really a bad idea? > Also the artificial split between hardware control in > drivers/gpu/imx/dcss and the DRM driver is just cargo-cult from the > IPU/imx-drm split. For the IPU we did it as the IPU has legs in both > DRM for the output parts and V4L2 for the input parts. As the DCSS has > no video input capabilities the driver could be simplified a lot by > moving it all into a single DRM driver. Agreed on this. Shawn ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109022] ring gfx timeout during particular shader generation on yuzu emulator
https://bugs.freedesktop.org/show_bug.cgi?id=109022 --- Comment #21 from e88z4 --- Hi, I uploaded the apitrace in my google drive. https://drive.google.com/file/d/1hg1ep4rJ2Y_g7WmA_HbmNndrHd-x7Etc/view?usp=sharing When I replayed the trace, I was able to produce the error. My system is AMD Ryzen 2600 Radeon RX580 Mesa master 659aa3dd651 Kernel 5.1.3 Let me know if you need anything else. -- 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 109022] ring gfx timeout during particular shader generation on yuzu emulator
https://bugs.freedesktop.org/show_bug.cgi?id=109022 --- Comment #20 from e88z4 --- Hi Timothy, I have tried your suggestion on the other bug reporting https://bugs.freedesktop.org/show_bug.cgi?id=110472 You suggested to add an environment variable allow_glsl_extension_directive_midshader=true. This doesn't resolve the issue. I also noticed that the yuzu developers followed your suggestion from your bug reporting ticket https://github.com/yuzu-emu/yuzu/issues/2523 I decided to compile the latest source code with the merged code but the issue is not yet resolved. One thing that I noticed is the "ring fgx timeout" error message is no longer being produced". Instead error messages below are reproduced continuously until I kill the application. May 27 18:53:21 htpc org.gnome.Shell.desktop[1578]: amdgpu: amdgpu_cs_query_fence_status failed. May 27 18:53:21 htpc org.gnome.Shell.desktop[1578]: amdgpu: The CS has been cancelled because the context is lost. May 27 18:53:21 htpc org.gnome.Shell.desktop[1578]: amdgpu: amdgpu_cs_query_fence_status failed. May 27 18:53:21 htpc org.gnome.Shell.desktop[1578]: amdgpu: The CS has been cancelled because the context is lost. May 27 18:53:21 htpc org.gnome.Shell.desktop[1578]: amdgpu: The CS has been cancelled because the context is lost. May 27 18:53:21 htpc org.gnome.Shell.desktop[1578]: amdgpu: The CS has been cancelled because the context is lost. May 27 18:53:25 htpc org.gnome.Shell.desktop[1578]: amdgpu: The CS has been cancelled because the context is lost. May 27 18:53:25 htpc org.gnome.Shell.desktop[1578]: amdgpu: The CS has been cancelled because the context is lost. May 27 18:53:29 htpc org.gnome.Shell.desktop[1578]: amdgpu: The CS has been cancelled because the context is lost. May 27 18:53:29 htpc org.gnome.Shell.desktop[1578]: amdgpu: The CS has been cancelled because the context is lost. May 27 18:53:29 htpc org.gnome.Shell.desktop[1578]: amdgpu: The CS has been cancelled because the context is lost. May 27 18:53:29 htpc org.gnome.Shell.desktop[1578]: amdgpu: The CS has been cancelled because the context is lost. May 27 18:53:29 htpc org.gnome.Shell.desktop[1578]: amdgpu: The CS has been cancelled because the context is lost. May 27 18:53:29 htpc org.gnome.Shell.desktop[1578]: amdgpu: The CS has been cancelled because the context is lost. I will produce the apitrace shortly and will post it on a share location. -- 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 110711] American Truck Simulator shows strange colored reflections
https://bugs.freedesktop.org/show_bug.cgi?id=110711 --- Comment #3 from Timothy Arceri --- (In reply to Petr Sebor from comment #2) > Whoops, this one got unnoticed (unreported?) for quite some time it seems. > However, the game is still evolving and the rendering code improving over > the years. If there is a performance overhead tied together to zeroing VRAM, > I'd rather fix the game. I can do that. Please do not add workarounds if > that is going to hurt the game performance. Performance shouldn't be impacted too much be we would much rather not have these workarounds. I've pushed the workaround to master for now but if you fix it please report it here and I've revert the change. 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
[Bug 110711] American Truck Simulator shows strange colored reflections
https://bugs.freedesktop.org/show_bug.cgi?id=110711 --- Comment #2 from Petr Sebor --- Whoops, this one got unnoticed (unreported?) for quite some time it seems. However, the game is still evolving and the rendering code improving over the years. If there is a performance overhead tied together to zeroing VRAM, I'd rather fix the game. I can do that. Please do not add workarounds if that is going to hurt the game performance. -- 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 203731] amdgpu wrong refresh rate for hdmi output with deepcolor turned on
https://bugzilla.kernel.org/show_bug.cgi?id=203731 --- Comment #1 from Gluzskiy Alexandr (sss123n...@list.ru) --- Created attachment 282973 --> https://bugzilla.kernel.org/attachment.cgi?id=282973=edit photo monitor photo -- 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 203731] New: amdgpu wrong refresh rate for hdmi output with deepcolor turned on
https://bugzilla.kernel.org/show_bug.cgi?id=203731 Bug ID: 203731 Summary: amdgpu wrong refresh rate for hdmi output with deepcolor turned on Product: Drivers Version: 2.5 Kernel Version: 4.19.45 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: high Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: sss123n...@list.ru Regression: No Created attachment 282971 --> https://bugzilla.kernel.org/attachment.cgi?id=282971=edit xorg server log i have 3 display setup (dvi, dp, hdmi), displays on dp and hdmi are 10bit displays, if i turn on deepcolor option on amdgpu driver, hdmi display got wrong refresh rate (looks too low), display supported 1920x1200@60 mode, in xorg.log we can see what it detected properly, but instead of this display got like 40hz, and turn black with out of range error, i have played a bit with modes, and found what mode "xrandr --newmode "1920x1200_73.48" 240.00 1920 2064 2264 2608 1200 1203 1209 1254 -hsync +vsync" works, display also warns about out of range mode (58.7hz), but actually works, NOTE: this all happens only if deepcolor driver option is turned on. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/omap: Make sure device_id tables are NULL terminated
Make sure (of/i2c/platform)_device_id tables are NULL terminated. Signed-off-by: Thomas Meyer --- diff -u -p a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c --- a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c +++ b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c @@ -198,6 +198,7 @@ static const struct of_device_id omapdss { .compatible = "toppoly,td028ttec1" }, { .compatible = "tpo,td028ttec1" }, { .compatible = "tpo,td043mtea1" }, + {}, }; static int __init omapdss_boot_init(void)
Extend drm_bus_flags? [Was: [PATCH v3 2/3] drm: Add bus flag for Sharp-specific signals]
Hi all. Please see mail below - is it OK to extend drm_bus_flags to represent "SHARP signals"? Paul (and I) could not find any better way to let the panel tell the display driver that it requires the special SHARP signals. This has been pending almost a month now and it would only be fair to either accept the solution or to give Paul guidiance how to move forward. There is a display driver that awaits the resilutions of this issue. Sam On Fri, Apr 26, 2019 at 01:18:53AM +0200, Paul Cercueil wrote: > Add the DRM_BUS_FLAG_SHARP_SIGNALS to the drm_bus_flags enum. > > This flags can be used when the display must be driven with the > Sharp-specific signals SPL, CLS, REV, PS. > > Signed-off-by: Paul Cercueil > --- > > Notes: > v3: New patch > > include/drm/drm_connector.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 02a131202add..ac7d58fd1e03 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -323,6 +323,8 @@ enum drm_panel_orientation { > * edge of the pixel clock > * @DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE:Sync signals are sampled on the > falling > * edge of the pixel clock > + * @DRM_BUS_FLAG_SHARP_SIGNALS: Set if the Sharp-specific > signals > + * (SPL, CLS, PS, REV) must be used > */ > enum drm_bus_flags { > DRM_BUS_FLAG_DE_LOW = BIT(0), > @@ -341,6 +343,7 @@ enum drm_bus_flags { > DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE = DRM_BUS_FLAG_SYNC_NEGEDGE, > DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE = DRM_BUS_FLAG_SYNC_NEGEDGE, > DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE = DRM_BUS_FLAG_SYNC_POSEDGE, > + DRM_BUS_FLAG_SHARP_SIGNALS = BIT(8), > }; > > /** > -- > 2.21.0.593.g511ec345e18 > > ___ > 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 3/6] drm/bridge: extract some Analogix I2C DP common code
Hi Torsten. > index ..9cb30962032e > --- /dev/null > +++ b/drivers/gpu/drm/bridge/analogix/analogix-i2c-dptx.c > @@ -0,0 +1,169 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright(c) 2017 Icenowy Zheng > + * > + * Based on analogix-anx78xx.c, which is: > + * Copyright(c) 2016, Analogix Semiconductor. All rights reserved. > + */ > + > +#include > +#include > + > +#include > +#include Can we please avoid drmP.h in new files. The header file is deprecated and we try to get rid of it. Sam
[Bug 110712] [regression]Raven Ridge: System freeze but mouse cursor able to move when using Firefox Webrender.
https://bugs.freedesktop.org/show_bug.cgi?id=110712 Haxk20 changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #3 from Haxk20 --- Just updated MESA. Doesnt seem to be an issue anymore. -- 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 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #17 from tempel.jul...@gmail.com --- I applied your patch and patches 1 and 3 of that series on linux 5.2-rc2, but it unfortunately doesn't show any effect: -There is still the mouse input issue for the games described in this ticket. -Opening new windows still creates stutter. -And so do gamma adjustments via RedShift. -- 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 v1 1/2] drm/i2c: tda998x: access chip registers via a regmap
The tda988x i2c chip registers are accessed through a paged register scheme. The kernel regmap abstraction supports paged accesses. Replace this driver's dedicated i2c access functions with a standard i2c regmap. Pros: - dedicated i2c access code disappears: accesses now go through well-maintained regmap code - page atomicity requirements now handled by regmap - ro/wo/rw access modes are now explicitly defined: any inappropriate register accesses (e.g. write to a read-only register) will now be explicitly rejected by the regmap core - all tda988x registers are now visible via debugfs Cons: - this will probably require extensive testing Tested on a TDA19988 using a Freescale/NXP imx6q. Signed-off-by: Sven Van Asbroeck --- I originally hacked this together while debugging an incompatibility between the tda988x's audio input and the audio codec I was driving it with. That required me to have debug access to the chip's register values. A regmap did the trick, it has good debugfs support. drivers/gpu/drm/i2c/tda998x_drv.c | 350 -- 1 file changed, 234 insertions(+), 116 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 7f34601bb515..8153e2e19e18 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -43,10 +44,9 @@ struct tda998x_audio_port { struct tda998x_priv { struct i2c_client *cec; struct i2c_client *hdmi; - struct mutex mutex; + struct regmap *regmap; u16 rev; u8 cec_addr; - u8 current_page; bool is_on; bool supports_infoframes; bool sink_has_audio; @@ -88,12 +88,10 @@ struct tda998x_priv { /* The TDA9988 series of devices use a paged register scheme.. to simplify * things we encode the page # in upper bits of the register #. To read/ * write a given register, we need to make sure CURPAGE register is set - * appropriately. Which implies reads/writes are not atomic. Fun! + * appropriately. */ #define REG(page, addr) (((page) << 8) | (addr)) -#define REG2ADDR(reg) ((reg) & 0xff) -#define REG2PAGE(reg) (((reg) >> 8) & 0xff) #define REG_CURPAGE 0xff/* write */ @@ -285,8 +283,9 @@ struct tda998x_priv { /* Page 09h: EDID Control */ +/* EDID_DATA consists of 128 successive registers read */ #define REG_EDID_DATA_0 REG(0x09, 0x00) /* read */ -/* next 127 successive registers are the EDID block */ +#define REG_EDID_DATA_127 REG(0x09, 0x7f) /* read */ #define REG_EDID_CTRL REG(0x09, 0xfa) /* read/write */ #define REG_DDC_ADDR REG(0x09, 0xfb) /* read/write */ #define REG_DDC_OFFS REG(0x09, 0xfc) /* read/write */ @@ -295,11 +294,17 @@ struct tda998x_priv { /* Page 10h: information frames and packets */ +/* REG_IF1_HB consists of 32 successive registers read/write */ #define REG_IF1_HB0 REG(0x10, 0x20) /* read/write */ +/* REG_IF2_HB consists of 32 successive registers read/write */ #define REG_IF2_HB0 REG(0x10, 0x40) /* read/write */ +/* REG_IF3_HB consists of 32 successive registers read/write */ #define REG_IF3_HB0 REG(0x10, 0x60) /* read/write */ +/* REG_IF4_HB consists of 32 successive registers read/write */ #define REG_IF4_HB0 REG(0x10, 0x80) /* read/write */ +/* REG_IF5_HB consists of 32 successive registers read/write */ #define REG_IF5_HB0 REG(0x10, 0xa0) /* read/write */ +#define REG_IF5_HB31 REG(0x10, 0xbf) /* read/write */ /* Page 11h: audio settings and content info packets */ @@ -542,93 +547,29 @@ static void tda998x_cec_hook_release(void *data) cec_enamods(priv, CEC_ENAMODS_EN_CEC_CLK | CEC_ENAMODS_EN_CEC, false); } -static int -set_page(struct tda998x_priv *priv, u16 reg) -{ - if (REG2PAGE(reg) != priv->current_page) { - struct i2c_client *client = priv->hdmi; - u8 buf[] = { - REG_CURPAGE, REG2PAGE(reg) - }; - int ret = i2c_master_send(client, buf, sizeof(buf)); - if (ret < 0) { - dev_err(>dev, "%s %04x err %d\n", __func__, - reg, ret); - return ret; - } - - priv->current_page = REG2PAGE(reg); - } - return 0; -} - static int reg_read_range(struct tda998x_priv *priv, u16 reg, char *buf, int cnt) { - struct i2c_client *client = priv->hdmi; - u8 addr = REG2ADDR(reg); int ret; - mutex_lock(>mutex); - ret = set_page(priv, reg); - if (ret < 0) - goto out; - - ret = i2c_master_send(client, , sizeof(addr)); + ret =
[PATCH v1 2/2] drm/i2c: tda998x: remove indirect reg_read/_write() calls
Remove indirect reg_read/_write() calls, and replace them with direct calls to regmap functions. For the sake of readability, keep the following indirect register access calls: - reg_set() - reg_clear() - reg_write16() Signed-off-by: Sven Van Asbroeck --- drivers/gpu/drm/i2c/tda998x_drv.c | 333 ++ 1 file changed, 157 insertions(+), 176 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 8153e2e19e18..1bddd2cf92ea 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -548,89 +548,59 @@ static void tda998x_cec_hook_release(void *data) } static int -reg_read_range(struct tda998x_priv *priv, u16 reg, char *buf, int cnt) -{ - int ret; - - ret = regmap_bulk_read(priv->regmap, reg, buf, cnt); - if (ret < 0) - return ret; - return cnt; -} - -static void -reg_write_range(struct tda998x_priv *priv, u16 reg, u8 *p, int cnt) -{ - regmap_bulk_write(priv->regmap, reg, p, cnt); -} - -static int -reg_read(struct tda998x_priv *priv, u16 reg) -{ - int ret, val; - - ret = regmap_read(priv->regmap, reg, ); - if (ret < 0) - return ret; - return val; -} - -static void -reg_write(struct tda998x_priv *priv, u16 reg, u8 val) -{ - regmap_write(priv->regmap, reg, val); -} - -static void -reg_write16(struct tda998x_priv *priv, u16 reg, u16 val) +reg_write16(struct regmap *regmap, u16 reg, u16 val) { u8 buf[] = {val >> 8, val}; - regmap_bulk_write(priv->regmap, reg, buf, ARRAY_SIZE(buf)); + return regmap_bulk_write(regmap, reg, buf, ARRAY_SIZE(buf)); } -static void -reg_set(struct tda998x_priv *priv, u16 reg, u8 val) +static int +reg_set(struct regmap *regmap, u16 reg, u8 val) { - regmap_update_bits(priv->regmap, reg, val, val); + return regmap_update_bits(regmap, reg, val, val); } -static void -reg_clear(struct tda998x_priv *priv, u16 reg, u8 val) +static int +reg_clear(struct regmap *regmap, u16 reg, u8 val) { - regmap_update_bits(priv->regmap, reg, val, 0); + return regmap_update_bits(regmap, reg, val, 0); } static void tda998x_reset(struct tda998x_priv *priv) { + struct regmap *regmap = priv->regmap; + /* reset audio and i2c master: */ - reg_write(priv, REG_SOFTRESET, SOFTRESET_AUDIO | SOFTRESET_I2C_MASTER); + regmap_write(regmap, REG_SOFTRESET, + SOFTRESET_AUDIO | SOFTRESET_I2C_MASTER); msleep(50); - reg_write(priv, REG_SOFTRESET, 0); + regmap_write(regmap, REG_SOFTRESET, 0); msleep(50); /* reset transmitter: */ - reg_set(priv, REG_MAIN_CNTRL0, MAIN_CNTRL0_SR); - reg_clear(priv, REG_MAIN_CNTRL0, MAIN_CNTRL0_SR); + reg_set(regmap, REG_MAIN_CNTRL0, MAIN_CNTRL0_SR); + reg_clear(regmap, REG_MAIN_CNTRL0, MAIN_CNTRL0_SR); /* PLL registers common configuration */ - reg_write(priv, REG_PLL_SERIAL_1, 0x00); - reg_write(priv, REG_PLL_SERIAL_2, PLL_SERIAL_2_SRL_NOSC(1)); - reg_write(priv, REG_PLL_SERIAL_3, 0x00); - reg_write(priv, REG_SERIALIZER, 0x00); - reg_write(priv, REG_BUFFER_OUT, 0x00); - reg_write(priv, REG_PLL_SCG1, 0x00); - reg_write(priv, REG_AUDIO_DIV,AUDIO_DIV_SERCLK_8); - reg_write(priv, REG_SEL_CLK, SEL_CLK_SEL_CLK1 | SEL_CLK_ENA_SC_CLK); - reg_write(priv, REG_PLL_SCGN1,0xfa); - reg_write(priv, REG_PLL_SCGN2,0x00); - reg_write(priv, REG_PLL_SCGR1,0x5b); - reg_write(priv, REG_PLL_SCGR2,0x00); - reg_write(priv, REG_PLL_SCG2, 0x10); + regmap_write(regmap, REG_PLL_SERIAL_1, 0x00); + regmap_write(regmap, REG_PLL_SERIAL_2, PLL_SERIAL_2_SRL_NOSC(1)); + regmap_write(regmap, REG_PLL_SERIAL_3, 0x00); + regmap_write(regmap, REG_SERIALIZER, 0x00); + regmap_write(regmap, REG_BUFFER_OUT, 0x00); + regmap_write(regmap, REG_PLL_SCG1, 0x00); + regmap_write(regmap, REG_AUDIO_DIV,AUDIO_DIV_SERCLK_8); + regmap_write(regmap, REG_SEL_CLK, + SEL_CLK_SEL_CLK1 | SEL_CLK_ENA_SC_CLK); + regmap_write(regmap, REG_PLL_SCGN1,0xfa); + regmap_write(regmap, REG_PLL_SCGN2,0x00); + regmap_write(regmap, REG_PLL_SCGR1,0x5b); + regmap_write(regmap, REG_PLL_SCGR2,0x00); + regmap_write(regmap, REG_PLL_SCG2, 0x10); /* Write the default value MUX register */ - reg_write(priv, REG_MUX_VP_VIP_OUT, 0x24); + regmap_write(regmap, REG_MUX_VP_VIP_OUT, 0x24); } /* @@ -685,16 +655,18 @@ static void tda998x_detect_work(struct work_struct *work) static irqreturn_t tda998x_irq_thread(int irq, void *data) { struct tda998x_priv *priv = data; - u8 sta, cec, lvl, flag0, flag1, flag2; + struct regmap *regmap = priv->regmap; + u8 sta, cec, lvl; + unsigned int flag0, flag1, flag2;
Re: RFC: Run a dedicated hmm.git for 5.3
On Sat, May 25, 2019 at 03:52:10PM -0700, Andrew Morton wrote: > On Fri, 24 May 2019 09:44:55 -0300 Jason Gunthorpe wrote: > > > Now that -mm merged the basic hmm API skeleton I think running like > > this would get us quickly to the place we all want: comprehensive in tree > > users of hmm. > > > > Andrew, would this be acceptable to you? > > Sure. Please take care not to permit this to reduce the amount of > exposure and review which the core HMM pieces get. Certainly, thanks all Jerome: I started a HMM branch on v5.2-rc2 in the rdma.git here: git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=hmm Please send a series with the initial cross tree stuff: - kconfig fixing patches - The full removal of all the 'temporary for merging' APIs - Fixing the API of hmm_range_register to accept a mirror When these are ready I'll send a hmm PR to DRM so everyone is on the same API page. I'll also move the hugetlb patch that Andrew picked up into this git so we don't have a merge conflict risk In parallel let us also finish revising the mirror API and going through the ODP stuff. Regards, Jason
Re: [PATCH] drm/syncobj: Include the header
On Mon, May 27, 2019 at 03:39:31PM -0300, Fabio Estevam wrote: > The prototype for 'drm_timeout_abs_to_jiffies' is provided by > the header. > > Include this header to fix the following sparse warning: > > drivers/gpu/drm/drm_syncobj.c:937:13: warning: symbol > 'drm_timeout_abs_to_jiffies' was not declared. Should it be static? > > Signed-off-by: Fabio Estevam Uh I think this isn't quite what we want. There's both a drm_utils.h and a drm_util.h and no drm_util.c and generally we try to match them all up. I think drm_utils.h should disappear entirely, with drm_get_panel_orientation_quirk moved to drm_panel.h (that's at least how it's also grouped in the docs). And the drm_timeout_abs_to_jiffies helper moved to drm_util.h, with it's implementation moved to a new drm_util.c Plus I guess including all that into the a new kerneldoc section. Assuming your up for a notch more cleanup than just shutting up sparse? Thanks, Daniel > --- > drivers/gpu/drm/drm_syncobj.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c > index 3d400905100b..b06fa424bd44 100644 > --- a/drivers/gpu/drm/drm_syncobj.c > +++ b/drivers/gpu/drm/drm_syncobj.c > @@ -46,6 +46,7 @@ > * The file takes a reference on the kref. > */ > > +#include > #include > #include > #include > -- > 2.17.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/damage-helper: Use NULL instead of 0
On Mon, May 27, 2019 at 03:37:14PM -0300, Fabio Estevam wrote: > The 'clips' member is a pointer, so assign NULL instead of 0. > > This fixes the following sparse warning: > > drivers/gpu/drm/drm_damage_helper.c:289:31: warning: Using plain integer as > NULL pointer > > Signed-off-by: Fabio Estevam Applied, thanks for your patch. -Daniel > --- > drivers/gpu/drm/drm_damage_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_damage_helper.c > b/drivers/gpu/drm/drm_damage_helper.c > index ee67c96841fa..8230dac01a89 100644 > --- a/drivers/gpu/drm/drm_damage_helper.c > +++ b/drivers/gpu/drm/drm_damage_helper.c > @@ -286,7 +286,7 @@ drm_atomic_helper_damage_iter_init(struct > drm_atomic_helper_damage_iter *iter, > iter->plane_src.y2 = (state->src.y2 >> 16) + !!(state->src.y2 & 0x); > > if (!iter->clips || !drm_rect_equals(>src, _state->src)) { > - iter->clips = 0; > + iter->clips = NULL; > iter->num_clips = 0; > iter->full_update = true; > } > -- > 2.17.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
[Bug 106302] [radeonsi] Garbage content when accessing a texture in multiple shared EGL contexts
https://bugs.freedesktop.org/show_bug.cgi?id=106302 --- Comment #3 from Pierre-Eric Pelloux-Prayer --- I agree but "D.3.1 Determining Completion of Changes to an object" says: "The contents of an object T are considered to have been changed once a command such as described in section D.3 has completed. Completion of a command may be determined either by calling Finish, or by calling FenceSync and executing a WaitSync command on the associated sync object.". So in my understanding, the contents of your texture in your example haven't changed by the end of createTexture() (because the glTexImage command hasn't completed) so rule 4 isn't relevant. -- 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/syncobj: Include the header
The prototype for 'drm_timeout_abs_to_jiffies' is provided by the header. Include this header to fix the following sparse warning: drivers/gpu/drm/drm_syncobj.c:937:13: warning: symbol 'drm_timeout_abs_to_jiffies' was not declared. Should it be static? Signed-off-by: Fabio Estevam --- drivers/gpu/drm/drm_syncobj.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 3d400905100b..b06fa424bd44 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -46,6 +46,7 @@ * The file takes a reference on the kref. */ +#include #include #include #include -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 203627] [Regression] Boot fails with linux-firmware 20190514
https://bugzilla.kernel.org/show_bug.cgi?id=203627 Arne Fahrenwalde (macgene...@macgeneral.de) changed: What|Removed |Added CC||macgene...@macgeneral.de --- Comment #3 from Arne Fahrenwalde (macgene...@macgeneral.de) --- I'm just here to state that I'm affected as well. Kernel versions 5.x boot fine with linux-firmware 20190514.711d329-1, Kernel version 4.19.x crashes directly after trying to load the amdgpu driver. The screen turns off and the system only hangs/reacts to the reset button (CTRL-ALT-DELETE won't work either). Unfortunately nothing is logged. Downgrading to linux-firmware 20190424.4b6cf2b-1 fixed it for me for now. OS: Manjaro Linux Hardware affected: AMD Vega 64 (10) (ASUS ROG STRIX) -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/damage-helper: Use NULL instead of 0
The 'clips' member is a pointer, so assign NULL instead of 0. This fixes the following sparse warning: drivers/gpu/drm/drm_damage_helper.c:289:31: warning: Using plain integer as NULL pointer Signed-off-by: Fabio Estevam --- drivers/gpu/drm/drm_damage_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c index ee67c96841fa..8230dac01a89 100644 --- a/drivers/gpu/drm/drm_damage_helper.c +++ b/drivers/gpu/drm/drm_damage_helper.c @@ -286,7 +286,7 @@ drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter, iter->plane_src.y2 = (state->src.y2 >> 16) + !!(state->src.y2 & 0x); if (!iter->clips || !drm_rect_equals(>src, _state->src)) { - iter->clips = 0; + iter->clips = NULL; iter->num_clips = 0; iter->full_update = true; } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110635] briefly flashing corruption when playing various OGL games
https://bugs.freedesktop.org/show_bug.cgi?id=110635 Danylo changed: What|Removed |Added CC||danylo.pilia...@gmail.com --- Comment #5 from Danylo --- Yep, AMD_DEBUG=nodma works -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #16 from Nicholas Kazlauskas --- Created attachment 144354 --> https://bugs.freedesktop.org/attachment.cgi?id=144354=edit 0001-drm-amd-display-Allow-fast-updates-again-for-swappin.patch Sure, you can try the patch I've attached on applied after series fixing the problem in DRM: https://patchwork.kernel.org/cover/10837847/ Not sure if that applies cleanly, however. The important patches from should be: https://patchwork.kernel.org/patch/10837849/ https://patchwork.kernel.org/patch/10837853/ -- 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 v2 0/7] drm: make headers self-contained and drop drmP.h
On Mon, May 27, 2019 at 08:18:35AM +0200, Daniel Vetter wrote: > On Sun, May 26, 2019 at 07:35:28PM +0200, Sam Ravnborg wrote: > > While removing use of drmP.h from files in drm/* I > > noticed that I had to add the same include files due to > > dependencies in the header files. > > > > It is better to let the header files be self-contained and > > let the users pull in only the additional headers files required. > > So I went ahead and made the relevant header files self-contained. > > (I did not check if this made any includes redundant in some files, > > I do not have tooling in place to do so). > > > > Daniel suggested to add support for testing that they stay > > self contained. > > Jani Nikula has sent a patch to kbuild to make this part of the > > kbuild machinery. I have used it locally and as soon as it > > lands in kbuild I will start using it for drm. > > We could have duplicated the infrastructure now but that seemed > > too much code chrunch. > > > > This patchset include the actual removal of drmP.h as one big patch. > > This is build tested on alpha (always interesting), arm, arm64, x86 etc. > > > > For all files touched the following was done: > > - include files divided up in blocks in following order: > > linux/* > > video/* > > drm/* > > "" > > - within each block the include files are sorted alphabetically > > > > v2: > > - use same ordering af blocks > > - move includes down below license text > > - added patch with actual drmP.h removal > > - reworded some subjects to make them more descriptive > > - fixed a few spelling erros in changelogs (but a few may remain) > > > > Sam > > On the series: > > Acked-by: Daniel Vetter > > Did a bit of scrolling, looks all reasonable, but definitely didn't check > things in-depth. Thanks, applied and will be pushed out in a minute. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #15 from tempel.jul...@gmail.com --- Well, hope on the horizon. If applying debug patches would be helpful for trying to shed light into this issue, I would of course do it. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110777] Kernel 5.1-5.2 MCLK stuck at 167MHz Vega 10 (56)
https://bugs.freedesktop.org/show_bug.cgi?id=110777 Bug ID: 110777 Summary: Kernel 5.1-5.2 MCLK stuck at 167MHz Vega 10 (56) Product: DRI Version: DRI git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: blocker Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: ant...@gmx.de Hi, Since Kernel 5.1 up-to Kernel 5.2 my Vega 56 card's memory clock is stuck at 167MHz and does not boost up any more. The exact same setup boosts fine to 1000MHz memclk when running Kernel 5.0.13. Is there any info I can provide to get this fixed? -- 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/2] drm: imx: Add NWL MIPI DSI host controller support
Hi Lucas, On Mon, May 27, 2019 at 03:36:53PM +0200, Lucas Stach wrote: > Am Mittwoch, den 08.05.2019, 19:18 +0200 schrieb Guido Günther: > > Hi, > > On Thu, Mar 07, 2019 at 11:30:51AM +0100, Guido Günther wrote: > > > This adds initial support for the NWL MIPI DSI Host controller found on > > > i.MX8 > > > SoCs. > > > > > > It adds support for the i.MX8MQ but the same IP core can also be found on > > > e.g. > > > i.MX8QXP. I added the necessary hooks to support other imx8 variants but > > > since > > > I only have imx8mq boards to test I omitted the platform data for other > > > SoCs. > > > > > > The code is based on NXPs BSP so I added Robert Chiras as Co-authored-by > > > but > > > I'm happy to swap Author: and Co-authored-by: if that looks more > > > appropriate. > > > The most notable changes over the BSP driver are > > > - Calculate HS mode timing from phy_configure_opts_mipi_dphy > > > - Perform all clock setup via DT > > > - Merge nwl-imx and nwl drivers > > > - Add B0 silion revision quirk > > > > > > Posting this is likely a bit premature (hence v0) but I wanted for one > > > show how > > > this hooks into the mixel dphy posted earlier [1] and avoid duplicating > > > work. > > > So if there's other code out there doing the same I'm be happy to merge > > > efforts. > > > > Since this is likely not going anywhere until we have a dcss driver aimed > > for mainline I'm not going spam the list with further revisions. However > > the 5.x version is maintained here: > > > > > > https://source.puri.sm/guido.gunther/linux-imx8/tree/forward-upstream/next-20190506/imx-nwl/v1-wip > > > > Feedback is still welcome. It'll eventually be forwarded to newer > > linux-next versions. > > > > Changes over the posted version are: > > > > - Add quirk for IMQ8MQ silicon B0 revision to not mess with the > > system reset controller on power down since enable won't work > > afterwards otherwise. > > - Disable tx esc clock *after* the phy power down to unbreak > > disable/enable (unblank/blank) > > - Drop devm_free_irq() handled by the device driver core > > - Add ports to dt binding docs > > - Select GENERIC_PHY_MIPI_DPHY instead of GENERIC_PHY for > > phy_mipi_dphy_get_default_config > > - Include drm_print.h to fix build on next-20190408 > > - Drop some debugging messages > > - Newline terminate all DRM_ printouts > > > > If somebody is working on DCSS support it'd be cool to know since this > > driver is currently a component of imx-display-subsystem which will only > > work out if dcss is handled like this as well. > > We have been looking at how to support DCSS in mainline for a while, > but most of the actual work got pushed behind in schedule due to other > priorities. > > One thing I can can say for certain is that DCSS should not be > integrated into imx-drm. It's a totally different hardware and > downstream clearly shows that it's not a good idea to cram it into imx- > drm. > > Also the artificial split between hardware control in > drivers/gpu/imx/dcss and the DRM driver is just cargo-cult from the > IPU/imx-drm split. For the IPU we did it as the IPU has legs in both > DRM for the output parts and V4L2 for the input parts. As the DCSS has > no video input capabilities the driver could be simplified a lot by > moving it all into a single DRM driver. I agree. While moving if forward from NXPs tree this caused more trouble than good so let's keep it separate form imx-drm. Cheers, -- Guido ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #14 from Nicholas Kazlauskas --- (In reply to tempel.julian from comment #13) > Thanks for letting me know! > Could you please provide me with a loose estimate if those general atomic > modesetting performance limitations can be overcome in the next months? > Would really put my mind at ease. :) The core bits + the bits that affect amdgpu are reviewed. But I think it's still waiting on review from maintainers of the other drivers the patch impacts. I wouldn't expect it to land before 5.3 or even 5.4 at the earliest unfortunately. I would still need to debug to know for sure if that's the actual bug that's going on here but it seems likely given that it's atomic DC + cursor movement + xf86-video-amdgpu that's causing the issue. -- 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 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #13 from tempel.jul...@gmail.com --- Thanks for letting me know! Could you please provide me with a loose estimate if those general atomic modesetting performance limitations can be overcome in the next months? Would really put my mind at ease. :) -- 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 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #12 from Nicholas Kazlauskas --- I'm wondering if this is the async cursor update bug again. Maybe something with WINE or the game is trying to swap cursor buffers frequently and it's interacting with the cursor double buffering in xf86-video-amdgpu. We still can't do fast cursor updates for swapping cursor framebuffers because we'll hit page faults that can kill the driver due to the cursor framebuffer not being properly refcounted. The fix for this particular bug is still under review in DRM. I plan on removing the restriction I added in amdgpu DM after the fix has been merged. But for now, whenever the cursor swaps framebuffers we can't perform fast cursor updates so we're forced to wait for the previous flip to finish and the vblank event to be sent back to userspace. This can cause small jitters depending on how often the cursor is updating and when it updates during the vblank interval. -- 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 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #11 from tempel.jul...@gmail.com --- Happens also with plain wined3d inside official Steam Proton builds. In case of Skyrim, it is also affects the rendering performance and thus is visible in the frametime graph (unlike Hitman 2 with DXVK): https://abload.de/img/screenshot_20190527_1t1ktp.png Those spikes occur by just moving the mouse. Pressing keyboard buttons don't trigger them. -- 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 109206] Kernel 4.20 amdgpu fails to load firmware on Ryzen 2500U
https://bugs.freedesktop.org/show_bug.cgi?id=109206 --- Comment #45 from Ondrej Lang --- I just came across this article, which seems to suggest a fix for the issue mentioned in this thread is coming in a future linux-firmware update: https://www.phoronix.com/scan.php?page=news_item=AMD-Raven1-Skip-The-DMCU It seem s patch has already been proposed to the kernel tree so hopefully this will fix the problem with some laptop models with the Raven Ridge 1 CPUs. Patch url: https://lists.freedesktop.org/archives/amd-gfx/2019-May/034307.html -- 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: DRM/AST regression (likely 4.14 -> 4.19+), providing EDID manually fails
On Sun, 26 May 2019 12:50:51 -0700, Ilpo Järvinen wrote: > > Hi all, > > I've a workstation which has internal VGA that is detected as AST 2400 and > with it EDID has been always quite flaky (except for some time it worked > with 4.14 long enough that I thought the problems would be past until the > problems reappeared also with 4.14). Thus, I've provided manually the EDID > that I extracted from the monitor using other computer (the monitor itself > worked just fine on the earlier computer so it is likely fine). > > I setup the manual EDID using drm_kms_helper.edid_firmware, however, > after upgrading to 4.19.45 it stopped working (no "Got external EDID base > block" appears in dmesg, the text mode is kept in the lower res mode, and > Xorg logs no longer dumps the EDID info like it did with 4.14). So I guess > the EDID I provided manually on the command line is not correctly put into > use with 4.19+ kernels. > > The 4.19 dmesg indicated that drm_kms_helper.edid_firmware is deprecated > so I also tested with drm.edid_firmware it suggested as the replacement > but with no luck (but I believe also the drm_kms_helper one should have > worked as it was only "deprecated"). > > I also tried 5.1.2 but it did not work any better (and with it also tried > removing all the manual *.edid_firmware from the command line so I still > need to provide one manually to have it reliable working it seems). I believe there is a bug already tracking this, here: https://bugs.freedesktop.org/show_bug.cgi?id=107583 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm/vmwgfx: remove custom ioctl io encoding check
On 2019/05/27, Thomas Hellstrom wrote: > On 5/27/19 5:27 PM, Emil Velikov wrote: > > On 2019/05/27, Thomas Hellstrom wrote: > > > On 5/27/19 2:35 PM, Emil Velikov wrote: > > > > Hi Thomas, > > > > > > > > On 2019/05/27, Thomas Hellstrom wrote: > > > > > > > > > > I think we might be talking past each other, let's take a step back: > > > > > > > > > > > >- as of previous patch, all of vmwgfx ioctls size is consistently > > > > > > handled by the core > > > > > I don't think I follow you here, AFAICT patch 3/5 only affects and > > > > > relaxes the execbuf checking (and in fact a little more than I would > > > > > like)? > > > > > > > > > Precisely, it makes execbuf ioctl behave like all other ioctls - both > > > > vmwgfx and rest of drm. > > > But we're still enforcing a non-relaxed size check for the other vmwgfx > > > private ioctls, right? Which is relaxed, together with the directions, in > > > this commit? > > > > > Regardless of the patch, all !execbuf vmwgfx ioctls use the related size > > checking from core drm. > > Well it does, but since we (before this patch) enforce ioctl->cmd == cmd, we > also enforce > _IOC_SIZE(ioctl->cmd) == _IOC_SIZE(cmd), which makes the core check > pointless, or am I missing something? > You're spot on - I never looked at the _IOC_SIZE declaration. I was assuming some other magic. > > > > > (Not that it matters much to the discussion, though). > > > > > Agreed. > > > > > > > ... > > > > Can you provide a concrete example, please? > > > OK, let's say you were developing fence wait functionality. Like > > > vmw_fence_obj_wait ioctl. Then suddenly you started to wonder why the wait > > > never timed out as it should. The reason turn out to be that signals were > > > restarting the waits with the original timeout. So you change the ioctl > > > from > > > W to RW and add a kernel-computed time to the argument. Everything is > > > fine, > > > except that you forget to change this in a user-space application > > > somewhere. > > > > > > So now what happens, is that that user-space bug can live on undetected as > > > in 1), and that means you can never go back and implement a stricter check > > > because that would completely break old user-space. > > > > > If I understand you correctly, the W -> RW change in unnecessary. Yet > > the only negative effect that I can see is the copy_to_user() overhead. > > > > The copy should be negligible, yet it "feels" silly. > > > > Is there anything more serious that I've missed? > > Well the point is in this case, that the write was necessary, but the code > would work sort of OK anyway. It updated a kernel "cookie" to make sure the > timeout would be correct even with the next call repetition. Now if an old > header was floating around, there might be clients using it. And with the > current core checks that typically wouldn't get noticed. With the check we'd > immediately notice and abort. It feels a little like moving from ANSI C to > K :-) > Technically, the kernel (or any function really) should write output arguments only when needed. Agree though - it's sometimes annoying or inconvenient. For the ANSI C to K comment - sure, only if one cares about backward compat. If they don't - flip the config toggle and carry on ;-) > > > > > > Having a closer look - vmwgfx (et al) seems to stand out, such that it > > does not provide a UABI define including the encoding. Hence it sort of > > duplicates that in userspace, by using the explicit drmCommand* > > > > Guess I could follow the suggestion in vmwgfx_drv.c move the defines to > > UABI, sync header and update mesa/xf86-video-vmwgfx. > > > > What do you think - yes, or please don't? > > Please hold on for a while, and I'll discuss it internally. > Ack. > > > > > The current code will trap (and has historically trapped) code like this. > > > That's mainly why I'm reluctant to give it up, but I guess it can be > > > conditionally compiled in for debug purposes. > > > > > This piece here, is the holly grail. I'll go further and suggest: > > > > - add a strict encoding and size check, behind a config toggle > > - make it a core drm thing and drop the custom vmwgfx one > > > > Will keep it disabled by default - but will clearly document Kconfig and > > docs that devs should toggle it to catch bugs. > > Sounds good, but IIRC the reason why I kept it only to driver-private > ioctls, is that there were errors with the drm ioctls. But that was a long > time ago so I might remember incorrectly, or user-space has been fixed. > Good point - will have a quick look and send patches if needed. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 1/6] net: stmmac: sun8i: add support for Allwinner H6 EMAC
From: Icenowy Zheng The EMAC on Allwinner H6 is just like the one on A64. The "internal PHY" on H6 is on a co-packaged AC200 chip, and it's not really internal (it's connected via RMII at PA GPIO bank). Add support for the Allwinner H6 EMAC in the dwmac-sun8i driver. Signed-off-by: Icenowy Zheng Signed-off-by: Ondrej Jirman --- .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c| 16 1 file changed, 16 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index ba124a4da793..3258dec84d55 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -147,6 +147,20 @@ static const struct emac_variant emac_variant_a64 = { .tx_delay_max = 7, }; +static const struct emac_variant emac_variant_h6 = { + .default_syscon_value = 0x5, + .syscon_field = _syscon_reg_field, + /* The "Internal PHY" of H6 is not on the die. It's on the +* co-packaged AC200 chip instead. +*/ + .soc_has_internal_phy = false, + .support_mii = true, + .support_rmii = true, + .support_rgmii = true, + .rx_delay_max = 31, + .tx_delay_max = 7, +}; + #define EMAC_BASIC_CTL0 0x00 #define EMAC_BASIC_CTL1 0x04 #define EMAC_INT_STA0x08 @@ -1212,6 +1226,8 @@ static const struct of_device_id sun8i_dwmac_match[] = { .data = _variant_r40 }, { .compatible = "allwinner,sun50i-a64-emac", .data = _variant_a64 }, + { .compatible = "allwinner,sun50i-h6-emac", + .data = _variant_h6 }, { } }; MODULE_DEVICE_TABLE(of, sun8i_dwmac_match); -- 2.21.0
[PATCH v6 2/6] net: stmmac: sun8i: force select external PHY when no internal one
From: Icenowy Zheng The PHY selection bit also exists on SoCs without an internal PHY; if it's set to 1 (internal PHY, default value) then the MAC will not make use of any PHY such SoCs. This problem appears when adapting for H6, which has no real internal PHY (the "internal PHY" on H6 is not on-die, but on a co-packaged AC200 chip, connected via RMII interface at GPIO bank A). Force the PHY selection bit to 0 when the SOC doesn't have an internal PHY, to address the problem of a wrong default value. Signed-off-by: Icenowy Zheng Signed-off-by: Ondrej Jirman --- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index 3258dec84d55..0484c289f328 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -907,6 +907,11 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv) * address. No need to mask it again. */ reg |= 1 << H3_EPHY_ADDR_SHIFT; + } else { + /* For SoCs without internal PHY the PHY selection bit should be +* set to 0 (external PHY). +*/ + reg &= ~H3_EPHY_SELECT; } if (!of_property_read_u32(node, "allwinner,tx-delay-ps", )) { -- 2.21.0
[PATCH v6 4/6] dt-bindings: display: hdmi-connector: Support DDC bus enable
From: Ondrej Jirman Some Allwinner SoC using boards (Orange Pi 3 for example) need to enable on-board voltage shifting logic for the DDC bus using a gpio to be able to access DDC bus. Use ddc-en-gpios property on the hdmi-connector to model this. Add binding documentation for optional ddc-en-gpios property. Signed-off-by: Ondrej Jirman --- .../devicetree/bindings/display/connector/hdmi-connector.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/connector/hdmi-connector.txt b/Documentation/devicetree/bindings/display/connector/hdmi-connector.txt index 508aee461e0d..aeb07c4bd703 100644 --- a/Documentation/devicetree/bindings/display/connector/hdmi-connector.txt +++ b/Documentation/devicetree/bindings/display/connector/hdmi-connector.txt @@ -9,6 +9,7 @@ Optional properties: - label: a symbolic name for the connector - hpd-gpios: HPD GPIO number - ddc-i2c-bus: phandle link to the I2C controller used for DDC EDID probing +- ddc-en-gpios: signal to enable DDC bus Required nodes: - Video port for HDMI input -- 2.21.0
[PATCH v6 0/6] Add support for Orange Pi 3
From: Ondrej Jirman This series implements support for Xunlong Orange Pi 3 board. Unfortunately, this board needs some small driver patches, so I have split the boards DT patch into chunks that require patches for drivers in various subsystems. Suggested merging plan/dependencies: - stmmac patches are needed for ethernet support (patches 1-3) - these should be ready now - HDMI support (patches 4-6) - should be ready Changes in v2: - added dt-bindings documentation for the board's compatible string (suggested by Clement) - addressed checkpatch warnings and code formatting issues (on Maxime's suggestions) - stmmac: dropped useless parenthesis, reworded description of the patch (suggested by Sergei) - drop useles dev_info() about the selected io bias voltage - docummented io voltage bias selection variant macros - wifi: marked WiFi DTS patch and realted mmc1_pins as "DO NOT MERGE", because wifi depends on H6 RTC support that's not merged yet (suggested by Clement) - added missing signed-of-bys - changed dr_mode to otg, and added a note about VBUS - improved wording of HDMI driver's DDC power supply patch Changes in v3: - dropped already applied patches - changed pinctrl I/O bias selection constants to enum and renamed - added /omit-if-no-ref/ to mmc1_pins - made mmc1_pins default pinconf for mmc1 in H6 dtsi - move ddc-supply to HDMI connector node, updated patch descriptions, changed dt-bindings docs Changes in v4: - fix checkpatch warnings/style issues - use enum in struct sunxi_desc_function for io_bias_cfg_variant - collected acked-by's - fix compile error in drivers/pinctrl/sunxi/pinctrl-sun9i-a80-r.c:156 caused by missing conversion from has_io_bias_cfg struct member (I've kept the acked-by, because it's a trivial change, but feel free to object.) (reported by Martin A. on github) I did not have A80 pinctrl enabled for some reason, so I did not catch this sooner. - dropped brcm firmware patch (was already applied) - dropped the wifi dts patch (will re-send after H6 RTC gets merged, along with bluetooth support, in a separate series) Changes in v5: - dropped already applied patches (pinctrl patches, mmc1 pinconf patch) - rename GMAC-3V3 -> GMAC-3V to match the schematic (Jagan) - changed hdmi-connector's ddc-supply property to ddc-en-gpios (Rob Herring) Changes in v6: - added dt-bindings reviewed-by tag - fix wording in stmmac commit (as suggested by Sergei) Please take a look. thank you and regards, Ondrej Jirman Icenowy Zheng (2): net: stmmac: sun8i: add support for Allwinner H6 EMAC net: stmmac: sun8i: force select external PHY when no internal one Ondrej Jirman (4): arm64: dts: allwinner: orange-pi-3: Enable ethernet dt-bindings: display: hdmi-connector: Support DDC bus enable drm: sun4i: Add support for enabling DDC I2C bus to sun8i_dw_hdmi glue arm64: dts: allwinner: orange-pi-3: Enable HDMI output .../display/connector/hdmi-connector.txt | 1 + .../dts/allwinner/sun50i-h6-orangepi-3.dts| 70 +++ drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 55 ++- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 3 + .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 21 ++ 5 files changed, 147 insertions(+), 3 deletions(-) -- 2.21.0
[PATCH v6 5/6] drm: sun4i: Add support for enabling DDC I2C bus to sun8i_dw_hdmi glue
From: Ondrej Jirman Orange Pi 3 board requires enabling a voltage shifting circuit via GPIO for the DDC bus to be usable. Add support for hdmi-connector node's optional ddc-en-gpios property to support this use case. Signed-off-by: Ondrej Jirman --- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 55 +-- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 3 ++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index 39d8509d96a0..59b81ba02d96 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c @@ -98,6 +98,30 @@ static u32 sun8i_dw_hdmi_find_possible_crtcs(struct drm_device *drm, return crtcs; } +static int sun8i_dw_hdmi_find_connector_pdev(struct device *dev, +struct platform_device **pdev_out) +{ + struct platform_device *pdev; + struct device_node *remote; + + remote = of_graph_get_remote_node(dev->of_node, 1, -1); + if (!remote) + return -ENODEV; + + if (!of_device_is_compatible(remote, "hdmi-connector")) { + of_node_put(remote); + return -ENODEV; + } + + pdev = of_find_device_by_node(remote); + of_node_put(remote); + if (!pdev) + return -ENODEV; + + *pdev_out = pdev; + return 0; +} + static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, void *data) { @@ -151,16 +175,29 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, return PTR_ERR(hdmi->regulator); } + ret = sun8i_dw_hdmi_find_connector_pdev(dev, >connector_pdev); + if (!ret) { + hdmi->ddc_en = gpiod_get_optional(>connector_pdev->dev, + "ddc-en", GPIOD_OUT_HIGH); + if (IS_ERR(hdmi->ddc_en)) { + platform_device_put(hdmi->connector_pdev); + dev_err(dev, "Couldn't get ddc-en gpio\n"); + return PTR_ERR(hdmi->ddc_en); + } + } + ret = regulator_enable(hdmi->regulator); if (ret) { dev_err(dev, "Failed to enable regulator\n"); - return ret; + goto err_unref_ddc_en; } + gpiod_set_value(hdmi->ddc_en, 1); + ret = reset_control_deassert(hdmi->rst_ctrl); if (ret) { dev_err(dev, "Could not deassert ctrl reset control\n"); - goto err_disable_regulator; + goto err_disable_ddc_en; } ret = clk_prepare_enable(hdmi->clk_tmds); @@ -213,8 +250,14 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, clk_disable_unprepare(hdmi->clk_tmds); err_assert_ctrl_reset: reset_control_assert(hdmi->rst_ctrl); -err_disable_regulator: +err_disable_ddc_en: + gpiod_set_value(hdmi->ddc_en, 0); regulator_disable(hdmi->regulator); +err_unref_ddc_en: + if (hdmi->ddc_en) + gpiod_put(hdmi->ddc_en); + + platform_device_put(hdmi->connector_pdev); return ret; } @@ -228,7 +271,13 @@ static void sun8i_dw_hdmi_unbind(struct device *dev, struct device *master, sun8i_hdmi_phy_remove(hdmi); clk_disable_unprepare(hdmi->clk_tmds); reset_control_assert(hdmi->rst_ctrl); + gpiod_set_value(hdmi->ddc_en, 0); regulator_disable(hdmi->regulator); + + if (hdmi->ddc_en) + gpiod_put(hdmi->ddc_en); + + platform_device_put(hdmi->connector_pdev); } static const struct component_ops sun8i_dw_hdmi_ops = { diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 720c5aa8adc1..dad66b8301c2 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -190,6 +191,8 @@ struct sun8i_dw_hdmi { struct regulator*regulator; const struct sun8i_dw_hdmi_quirks *quirks; struct reset_control*rst_ctrl; + struct platform_device *connector_pdev; + struct gpio_desc*ddc_en; }; static inline struct sun8i_dw_hdmi * -- 2.21.0
[PATCH v6 3/6] arm64: dts: allwinner: orange-pi-3: Enable ethernet
From: Ondrej Jirman Orange Pi 3 has two regulators that power the Realtek RTL8211E. According to the phy datasheet, both regulators need to be enabled at the same time, but we can only specify a single phy-supply in the DT. This can be achieved by making one regulator depedning on the other via vin-supply. While it's not a technically correct description of the hardware, it achieves the purpose. All values of RX/TX delay were tested exhaustively and a middle one of the working values was chosen. Signed-off-by: Ondrej Jirman --- .../dts/allwinner/sun50i-h6-orangepi-3.dts| 44 +++ 1 file changed, 44 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts index 17d496990108..2c6807b74ff6 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts @@ -15,6 +15,7 @@ aliases { serial0 = + ethernet0 = }; chosen { @@ -44,6 +45,27 @@ regulator-max-microvolt = <500>; regulator-always-on; }; + + /* +* The board uses 2.5V RGMII signalling. Power sequence to enable +* the phy is to enable GMAC-2V5 and GMAC-3V (aldo2) power rails +* at the same time and to wait 100ms. +*/ + reg_gmac_2v5: gmac-2v5 { + compatible = "regulator-fixed"; + regulator-name = "gmac-2v5"; + regulator-min-microvolt = <250>; + regulator-max-microvolt = <250>; + startup-delay-us = <10>; + enable-active-high; + gpio = < 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */ + + /* The real parent of gmac-2v5 is reg_vcc5v, but we need to +* enable two regulators to power the phy. This is one way +* to achieve that. +*/ + vin-supply = <_aldo2>; /* GMAC-3V */ + }; }; { @@ -58,6 +80,28 @@ status = "okay"; }; + { + pinctrl-names = "default"; + pinctrl-0 = <_rgmii_pins>; + phy-mode = "rgmii"; + phy-handle = <_rgmii_phy>; + phy-supply = <_gmac_2v5>; + allwinner,rx-delay-ps = <1500>; + allwinner,tx-delay-ps = <700>; + status = "okay"; +}; + + { + ext_rgmii_phy: ethernet-phy@1 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <1>; + + reset-gpios = < 3 14 GPIO_ACTIVE_LOW>; /* PD14 */ + reset-assert-us = <15000>; + reset-deassert-us = <4>; + }; +}; + { vmmc-supply = <_cldo1>; cd-gpios = < 5 6 GPIO_ACTIVE_LOW>; /* PF6 */ -- 2.21.0
[PATCH v6 6/6] arm64: dts: allwinner: orange-pi-3: Enable HDMI output
From: Ondrej Jirman Orange Pi 3 has a DDC_CEC_EN signal connected to PH2, that enables the DDC I2C bus voltage shifter. Before EDID can be read, we need to pull PH2 high. This is realized by the ddc-en-gpios property. Signed-off-by: Ondrej Jirman --- .../dts/allwinner/sun50i-h6-orangepi-3.dts| 26 +++ 1 file changed, 26 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts index 2c6807b74ff6..01bb1bafe284 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts @@ -22,6 +22,18 @@ stdout-path = "serial0:115200n8"; }; + connector { + compatible = "hdmi-connector"; + ddc-en-gpios = < 7 2 GPIO_ACTIVE_HIGH>; /* PH2 */ + type = "a"; + + port { + hdmi_con_in: endpoint { + remote-endpoint = <_out_con>; + }; + }; + }; + leds { compatible = "gpio-leds"; @@ -72,6 +84,10 @@ cpu-supply = <_dcdca>; }; + { + status = "okay"; +}; + { status = "okay"; }; @@ -91,6 +107,16 @@ status = "okay"; }; + { + status = "okay"; +}; + +_out { + hdmi_out_con: endpoint { + remote-endpoint = <_con_in>; + }; +}; + { ext_rgmii_phy: ethernet-phy@1 { compatible = "ethernet-phy-ieee802.3-c22"; -- 2.21.0
Re: [PATCH 4/5] drm/vmwgfx: remove custom ioctl io encoding check
On 5/27/19 5:27 PM, Emil Velikov wrote: On 2019/05/27, Thomas Hellstrom wrote: On 5/27/19 2:35 PM, Emil Velikov wrote: Hi Thomas, On 2019/05/27, Thomas Hellstrom wrote: I think we might be talking past each other, let's take a step back: - as of previous patch, all of vmwgfx ioctls size is consistently handled by the core I don't think I follow you here, AFAICT patch 3/5 only affects and relaxes the execbuf checking (and in fact a little more than I would like)? Precisely, it makes execbuf ioctl behave like all other ioctls - both vmwgfx and rest of drm. But we're still enforcing a non-relaxed size check for the other vmwgfx private ioctls, right? Which is relaxed, together with the directions, in this commit? Regardless of the patch, all !execbuf vmwgfx ioctls use the related size checking from core drm. Well it does, but since we (before this patch) enforce ioctl->cmd == cmd, we also enforce _IOC_SIZE(ioctl->cmd) == _IOC_SIZE(cmd), which makes the core check pointless, or am I missing something? (Not that it matters much to the discussion, though). Agreed. ... Can you provide a concrete example, please? OK, let's say you were developing fence wait functionality. Like vmw_fence_obj_wait ioctl. Then suddenly you started to wonder why the wait never timed out as it should. The reason turn out to be that signals were restarting the waits with the original timeout. So you change the ioctl from W to RW and add a kernel-computed time to the argument. Everything is fine, except that you forget to change this in a user-space application somewhere. So now what happens, is that that user-space bug can live on undetected as in 1), and that means you can never go back and implement a stricter check because that would completely break old user-space. If I understand you correctly, the W -> RW change in unnecessary. Yet the only negative effect that I can see is the copy_to_user() overhead. The copy should be negligible, yet it "feels" silly. Is there anything more serious that I've missed? Well the point is in this case, that the write was necessary, but the code would work sort of OK anyway. It updated a kernel "cookie" to make sure the timeout would be correct even with the next call repetition. Now if an old header was floating around, there might be clients using it. And with the current core checks that typically wouldn't get noticed. With the check we'd immediately notice and abort. It feels a little like moving from ANSI C to K :-) Having a closer look - vmwgfx (et al) seems to stand out, such that it does not provide a UABI define including the encoding. Hence it sort of duplicates that in userspace, by using the explicit drmCommand* Guess I could follow the suggestion in vmwgfx_drv.c move the defines to UABI, sync header and update mesa/xf86-video-vmwgfx. What do you think - yes, or please don't? Please hold on for a while, and I'll discuss it internally. The current code will trap (and has historically trapped) code like this. That's mainly why I'm reluctant to give it up, but I guess it can be conditionally compiled in for debug purposes. This piece here, is the holly grail. I'll go further and suggest: - add a strict encoding and size check, behind a config toggle - make it a core drm thing and drop the custom vmwgfx one Will keep it disabled by default - but will clearly document Kconfig and docs that devs should toggle it to catch bugs. Sounds good, but IIRC the reason why I kept it only to driver-private ioctls, is that there were errors with the drm ioctls. But that was a long time ago so I might remember incorrectly, or user-space has been fixed. 2) Catch a lot of fuzzer combinations and error out early instead of forwarding them to the ioctl function where they may cause harm. Struggling to see why this is a problem? At some point the fuzzer will get past this first line of defence, so we want to make the rest of the ioctl is robust. I think the new user-space vs old kernel can be handled nicely in user- space with feature flags or API versions. That's the way we've handled them up to now? How is a feature flag doing to help if the encoding changes from _IOW to _IORW? Ah, you're referring to old user-space new kernel? Yes, I was probably reading a bit too fast. Sorry about that. So we're basically landing in a tradeoff between trapping problems like above, and hazzle-free ioctl argument definition change. OK, so I'm ok with that as long as there is a way we can compile in strict checking, which will likely has to be as a vmwgfx-specific wrapper. Ack, I'll proceed with the debug toggle suggestion. Great. Thank you for the insightful input. Emil Thanks, Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/05/27, Koenig, Christian wrote: > Am 27.05.19 um 15:26 schrieb Emil Velikov: > > On 2019/05/27, Daniel Vetter wrote: > >> On Mon, May 27, 2019 at 10:47:39AM +, Koenig, Christian wrote: > >>> Am 27.05.19 um 10:17 schrieb Emil Velikov: > From: Emil Velikov > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > render node. A seemingly deliberate design decision. > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > yet not all userspace checks if it's authenticated, but instead uses > uncommon assumptions. > > After days of digging through git log and testing, only a single (ab)use > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > assuming that failure implies lack of authentication. > > Affected versions are: > - the whole 18.2.x series, which is EOL > - the whole 18.3.x series, which is EOL > - the 19.0.x series, prior to 19.0.4 > >>> Well you could have saved your time, cause this is still a NAK. > >>> > >>> For the record: I strongly think that we don't want to expose any render > >>> functionality on the primary node. > >>> > >>> To even go a step further I would say that at least on AMD hardware we > >>> should completely disable DRI2 for one of the next generations. > >>> > >>> As a follow up I would then completely disallow the DRM authentication > >>> for amdgpu, so that the command submission interface on the primary node > >>> can only be used by the display server. > >> So amdgpu is running in one direction, while everyone else is running in > >> the other direction? Please note that your patch to change i915 landed > >> already, so that ship is sailing (but we could ofc revert that back > >> again). > >> > >> Imo really not a great idea if we do a amdgpu vs. everyone else split > >> here. If we want to deprecate dri2/flink/rendering on primary nodes across > >> the stack, then that should be a stack wide decision, not an amdgpu one. > >> > > Cannot agree more - I would love to see drivers stay consistent. > > Yeah, completely agree to that. That's why I think we should not do this > at all and just let Intel fix it's userspace bugs :P > Pretty sure I mentioned it before - might have been too subtle: The problem is _neither_ Intel nor libva specific. > Anyway my concern is really that we should stop extending functionality > on the primary node. > > E.g. the render node is for use by the clients and the primary node for > mode setting and use by the display server only. > Fully agreed. I'm not extending anything really. If anything - code is removed from drivers and core :-) Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm/bridge: dw-hdmi: Add support for dynamic output format setup
Hi! Dne ponedeljek, 20. maj 2019 ob 15:37:51 CEST je Neil Armstrong napisal(a): > In order to support the HDMI2.0 YUV420, YUV422 and the 10bit, 12bit and > 16bits outpu use cases, add support for the recently introduced bridge > callback format_set(). > > This callback will setup the new input format and encoding from encoder, > then these information will be used instead of the default ones > in the dw_hdmi_setup() function. > > To determine the output bus format, has been added : > - support for the connector display_info bus_formats, where a fixed > output bus format can be enforced by the encoder > - support for synami output bus format depending on the input format, > especially the YUV420 input bus format, enforcing YUV420 as output > with the correct bit depth > > Signed-off-by: Neil Armstrong > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 121 -- > 1 file changed, 112 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index > b50c49caf7ae..284ce59be8f8 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -103,6 +103,8 @@ struct hdmi_vmode { > }; > > struct hdmi_data_info { > + unsigned int bridge_in_bus_format; > + unsigned int bridge_in_encoding; > unsigned int enc_in_bus_format; > unsigned int enc_out_bus_format; > unsigned int enc_in_encoding; > @@ -1838,8 +1840,51 @@ static void hdmi_disable_overflow_interrupts(struct > dw_hdmi *hdmi) HDMI_IH_MUTE_FC_STAT2); > } > > +/* > + * The DW-HDMI CSC can only interpolate and decimate from 4:2:2 to > 4:4:4/RGB + * and from 4:4:4/RGB to 4:2:2. > + * Default to RGB output except if 4:2:0 as input, which CSC cannot > convert. + */ > +static unsigned long dw_hdmi_determine_output_bus_format(struct dw_hdmi > *hdmi) +{ > + unsigned int depth = hdmi_bus_fmt_color_depth( > + hdmi- >hdmi_data.enc_in_bus_format); > + bool is_420 = hdmi_bus_fmt_is_yuv420(hdmi- >hdmi_data.enc_in_bus_format); > + unsigned long fmt = MEDIA_BUS_FMT_RGB888_1X24; > + > + switch (depth) { > + case 8: > + if (is_420) > + fmt = MEDIA_BUS_FMT_UYYVYY8_0_5X24; > + else > + fmt = MEDIA_BUS_FMT_RGB888_1X24; > + break; > + case 10: > + if (is_420) > + fmt = MEDIA_BUS_FMT_UYYVYY10_0_5X30; > + else > + fmt = MEDIA_BUS_FMT_RGB101010_1X30; > + break; > + case 12: > + if (is_420) > + fmt = MEDIA_BUS_FMT_UYYVYY12_0_5X36; > + else > + fmt = MEDIA_BUS_FMT_RGB121212_1X36; > + break; > + case 16: > + if (is_420) > + fmt = MEDIA_BUS_FMT_UYYVYY16_0_5X48; > + else > + fmt = MEDIA_BUS_FMT_RGB161616_1X48; > + break; > + } > + > + return fmt; > +} > + > static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode > *mode) { > + struct drm_display_info *display = >connector.display_info; > int ret; > > hdmi_disable_overflow_interrupts(hdmi); > @@ -1853,9 +1898,9 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct > drm_display_mode *mode) } > > if ((hdmi->vic == 6) || (hdmi->vic == 7) || > - (hdmi->vic == 21) || (hdmi->vic == 22) || > - (hdmi->vic == 2) || (hdmi->vic == 3) || > - (hdmi->vic == 17) || (hdmi->vic == 18)) > + (hdmi->vic == 21) || (hdmi->vic == 22) || > + (hdmi->vic == 2) || (hdmi->vic == 3) || > + (hdmi->vic == 17) || (hdmi->vic == 18)) > hdmi->hdmi_data.enc_out_encoding = V4L2_YCBCR_ENC_601; > else > hdmi->hdmi_data.enc_out_encoding = V4L2_YCBCR_ENC_709; > @@ -1863,22 +1908,29 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, > struct drm_display_mode *mode) > hdmi->hdmi_data.video_mode.mpixelrepetitionoutput = 0; > hdmi->hdmi_data.video_mode.mpixelrepetitioninput = 0; > > - /* TOFIX: Get input format from plat data or fallback to RGB888 */ > - if (hdmi->plat_data->input_bus_format) > + if (hdmi->hdmi_data.bridge_in_bus_format) > + hdmi->hdmi_data.enc_in_bus_format = > + hdmi->hdmi_data.bridge_in_bus_format; > + else if (hdmi->plat_data->input_bus_format) > hdmi->hdmi_data.enc_in_bus_format = > hdmi->plat_data->input_bus_format; > else > hdmi->hdmi_data.enc_in_bus_format = MEDIA_BUS_FMT_RGB888_1X24; > > - /* TOFIX: Get input encoding from plat data or fallback to none */ > - if (hdmi->plat_data->input_bus_encoding) > + if (hdmi->hdmi_data.bridge_in_encoding) > + hdmi->hdmi_data.enc_in_encoding = > +
Re: [PATCH 2/5] drm/bridge: add encoder support to specify bridge input format
Hi! Thanks for working on this! Dne ponedeljek, 20. maj 2019 ob 15:37:50 CEST je Neil Armstrong napisal(a): > This patch adds a new format_set() callback to the bridge ops permitting > the encoder to specify the new input format and encoding. > > This allows supporting the very specific HDMI2.0 YUV420 output mode > when the bridge cannot convert from RGB or YUV444 to YUV420. > > In this case, the encode must downsample before the bridge and must > specify the bridge the new input bus format differs. > > This will also help supporting the YUV420 mode where the bridge cannot > downsample, and also support 10bit, 12bit and 16bit output modes > when the bridge cannot convert between different bit depths. > > Signed-off-by: Neil Armstrong > --- Reviewed-by: Jernej Skrabec Best regards, Jernej
Re: [PATCH 4/5] drm/vmwgfx: remove custom ioctl io encoding check
On 2019/05/27, Thomas Hellstrom wrote: > On 5/27/19 2:35 PM, Emil Velikov wrote: > > Hi Thomas, > > > > On 2019/05/27, Thomas Hellstrom wrote: > > > > > > I think we might be talking past each other, let's take a step back: > > > > > > > > - as of previous patch, all of vmwgfx ioctls size is consistently > > > > handled by the core > > > I don't think I follow you here, AFAICT patch 3/5 only affects and > > > relaxes the execbuf checking (and in fact a little more than I would > > > like)? > > > > > Precisely, it makes execbuf ioctl behave like all other ioctls - both > > vmwgfx and rest of drm. > > But we're still enforcing a non-relaxed size check for the other vmwgfx > private ioctls, right? Which is relaxed, together with the directions, in > this commit? > Regardless of the patch, all !execbuf vmwgfx ioctls use the related size checking from core drm. > (Not that it matters much to the discussion, though). > Agreed. > > > > > > - handling of featue flags, as always, is responsibility of the > > > > driver > > > > ifself > > > > - with this patch, ioctl direction is also handled by core. > > > > > > > > Here core ensures we only copy in/out as much data as the kernel > > > > implementation can handle. > > > > > > > > > > > > Let's consider the following real world example - msm and virtio_gpu. > > > > > > > > An in field of an _IOW ioctl becomes in/out aka _IORW ioctl. > > > > - we add a flag to annotate/request the out, as always invalid flags > > > > are return -EINVAL > > > > - we change the ioctl encoding > > > > > > > > As currently handled by core DRM, old kernel/new userspace and > > > > vice-versa works just fine. Sadly, vmwgfx will error out, while it > > > > could > > > > be avoided. > > > IMO basically we have a tradeoff between strict checking in this case, > > > and new user-space vs old kernel "hazzle-free" transition in the > > > relaxed case. > > > > > Precisely. If I read the code correctly, ATM new userspace will fail > > against old kernels. Unless userspace writes two versions of the ioctl - > > with with each encoding. > > > > > > As said above, I'll gladly adjust core and/or others, if this relaxed > > > > approach causes an issue somewhere. A specific use-case, real or > > > > hypothetical will be appreciated. > > > To me there are two important reasons to keep the strict approach. > > > > > > 1) Avoid user-space mistakes early in the development cycle. We can't > > > distinguish between buggy user-space and "new" user-space. This is > > > important because of [a]) below. > > > > > Can you provide a concrete example, please? > > OK, let's say you were developing fence wait functionality. Like > vmw_fence_obj_wait ioctl. Then suddenly you started to wonder why the wait > never timed out as it should. The reason turn out to be that signals were > restarting the waits with the original timeout. So you change the ioctl from > W to RW and add a kernel-computed time to the argument. Everything is fine, > except that you forget to change this in a user-space application somewhere. > > So now what happens, is that that user-space bug can live on undetected as > in 1), and that means you can never go back and implement a stricter check > because that would completely break old user-space. > If I understand you correctly, the W -> RW change in unnecessary. Yet the only negative effect that I can see is the copy_to_user() overhead. The copy should be negligible, yet it "feels" silly. Is there anything more serious that I've missed? Having a closer look - vmwgfx (et al) seems to stand out, such that it does not provide a UABI define including the encoding. Hence it sort of duplicates that in userspace, by using the explicit drmCommand* Guess I could follow the suggestion in vmwgfx_drv.c move the defines to UABI, sync header and update mesa/xf86-video-vmwgfx. What do you think - yes, or please don't? > The current code will trap (and has historically trapped) code like this. > That's mainly why I'm reluctant to give it up, but I guess it can be > conditionally compiled in for debug purposes. > This piece here, is the holly grail. I'll go further and suggest: - add a strict encoding and size check, behind a config toggle - make it a core drm thing and drop the custom vmwgfx one Will keep it disabled by default - but will clearly document Kconfig and docs that devs should toggle it to catch bugs. > > > > > 2) Catch a lot of fuzzer combinations and error out early instead of > > > forwarding them to the ioctl function where they may cause harm. > > > > > Struggling to see why this is a problem? At some point the fuzzer will > > get past this first line of defence, so we want to make the rest of the > > ioctl is robust. > > > > > > > I think the new user-space vs old kernel can be handled nicely in user- > > > space with feature flags or API versions. That's the way we've handled > > > them up to now? > > > > > How is a feature flag doing to help
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On Mon, May 27, 2019 at 3:42 PM Koenig, Christian wrote: > > Am 27.05.19 um 15:26 schrieb Emil Velikov: > > On 2019/05/27, Daniel Vetter wrote: > >> On Mon, May 27, 2019 at 10:47:39AM +, Koenig, Christian wrote: > >>> Am 27.05.19 um 10:17 schrieb Emil Velikov: > From: Emil Velikov > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > render node. A seemingly deliberate design decision. > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > yet not all userspace checks if it's authenticated, but instead uses > uncommon assumptions. > > After days of digging through git log and testing, only a single (ab)use > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > assuming that failure implies lack of authentication. > > Affected versions are: > - the whole 18.2.x series, which is EOL > - the whole 18.3.x series, which is EOL > - the 19.0.x series, prior to 19.0.4 > >>> Well you could have saved your time, cause this is still a NAK. > >>> > >>> For the record: I strongly think that we don't want to expose any render > >>> functionality on the primary node. > >>> > >>> To even go a step further I would say that at least on AMD hardware we > >>> should completely disable DRI2 for one of the next generations. > >>> > >>> As a follow up I would then completely disallow the DRM authentication > >>> for amdgpu, so that the command submission interface on the primary node > >>> can only be used by the display server. > >> So amdgpu is running in one direction, while everyone else is running in > >> the other direction? Please note that your patch to change i915 landed > >> already, so that ship is sailing (but we could ofc revert that back > >> again). > >> > >> Imo really not a great idea if we do a amdgpu vs. everyone else split > >> here. If we want to deprecate dri2/flink/rendering on primary nodes across > >> the stack, then that should be a stack wide decision, not an amdgpu one. > >> > > Cannot agree more - I would love to see drivers stay consistent. > > Yeah, completely agree to that. That's why I think we should not do this > at all and just let Intel fix it's userspace bugs :P So you're planning to submit that revert? You did jump the gun with sending out that patch after all too ... (aside from it got merged because of some other mixup with r-b tags and what not). -Daniel > Anyway my concern is really that we should stop extending functionality > on the primary node. > > E.g. the render node is for use by the clients and the primary node for > mode setting and use by the display server only. > > Regards, > Christian. > > > Fwiw, this series consistently paves the way toward nuking DRM_AUTH ;-) > > > > Thanks > > Emil > -- 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
[Bug 110635] briefly flashing corruption when playing various OGL games
https://bugs.freedesktop.org/show_bug.cgi?id=110635 --- Comment #4 from tempel.jul...@gmail.com --- It looks a lot like it that AMD_DEBUG=nodma prevents the artifacts from occurring. -- 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 13/13] drm: allow render capable master with DRM_AUTH ioctls
On Mon, May 27, 2019 at 4:01 PM Thomas Hellstrom wrote: > > On 5/27/19 3:16 PM, Daniel Vetter wrote: > > On Mon, May 27, 2019 at 02:39:18PM +0200, Thomas Hellstrom wrote: > >> On 5/27/19 10:17 AM, Emil Velikov wrote: > >>> From: Emil Velikov > >>> > >>> There are cases (in mesa and applications) where one would open the > >>> primary node without properly authenticating the client. > >>> > >>> Sometimes we don't check if the authentication succeeds, but there's > >>> also cases we simply forget to do it. > >>> > >>> The former was a case for Mesa where it did not not check the return > >>> value of drmGetMagic() [1]. That was fixed recently although, there's > >>> the question of older drivers or other apps that exbibit this behaviour. > >>> > >>> While omitting the call results in issues as seen in [2] and [3]. > >>> > >>> In the libva case, libva itself doesn't authenticate the DRM client and > >>> the vaGetDisplayDRM documentation doesn't mention if the app should > >>> either. > >>> > >>> As of today, the official vainfo utility doesn't authenticate. > >>> > >>> To workaround issues like these, some users resort to running their apps > >>> under sudo. Which admittedly isn't always a good idea. > >>> > >>> Since any DRIVER_RENDER driver has sufficient isolation between clients, > >>> we can use that, for unauthenticated [primary node] ioctls that require > >>> DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. > >>> > >>> v2: > >>> - Rework/simplify if check (Daniel V) > >>> - Add examples to commit messages, elaborate. (Daniel V) > >>> > >>> v3: > >>> - Use single unlikely (Daniel V) > >>> > >>> v4: > >>> - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU > >>> issue is fixed with earlier patch. > >>> > >>> [1] > >>> https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136 > >>> [2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html > >>> [3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1 > >>> Testcase: igt/core_unauth_vs_render > >>> Cc: intel-...@lists.freedesktop.org > >>> Signed-off-by: Emil Velikov > >>> Reviewed-by: Daniel Vetter > >>> Link: > >>> https://patchwork.freedesktop.org/patch/msgid/20190114085408.15933-2-emil.l.veli...@gmail.com > >>> --- > >>>drivers/gpu/drm/drm_ioctl.c | 20 > >>>1 file changed, 16 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > >>> index 9841c0076f02..b64b022a2b29 100644 > >>> --- a/drivers/gpu/drm/drm_ioctl.c > >>> +++ b/drivers/gpu/drm/drm_ioctl.c > >>> @@ -511,6 +511,13 @@ int drm_version(struct drm_device *dev, void *data, > >>> return err; > >>>} > >>> +static inline bool > >>> +drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags) > >>> +{ > >>> + return drm_core_check_feature(dev, DRIVER_RENDER) && > >>> + (flags & DRM_RENDER_ALLOW); > >>> +} > >>> + > >>>/** > >>> * drm_ioctl_permit - Check ioctl permissions against caller > >>> * > >>> @@ -525,14 +532,19 @@ int drm_version(struct drm_device *dev, void *data, > >>> */ > >>>int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) > >>>{ > >>> + const struct drm_device *dev = file_priv->minor->dev; > >>> + > >>> /* ROOT_ONLY is only for CAP_SYS_ADMIN */ > >>> if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN))) > >>> return -EACCES; > >>> - /* AUTH is only for authenticated or render client */ > >>> - if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) && > >>> -!file_priv->authenticated)) > >>> - return -EACCES; > >>> + /* AUTH is only for master ... */ > >>> + if (unlikely((flags & DRM_AUTH) && drm_is_primary_client(file_priv))) > >>> { > >>> + /* authenticated ones, or render capable on DRM_RENDER_ALLOW. > >>> */ > >>> + if (!file_priv->authenticated && > >>> + !drm_render_driver_and_ioctl(dev, flags)) > >>> + return -EACCES; > >>> + } > >> This breaks vmwgfx primary client authentication in the surface_reference > >> ioctl, which takes different paths in case of render clients and primary > >> clients, but adding an auth check in the primary path in the vmwgfx code > >> should fix this. > > Hm yeah we need to adjust that ... otoh kinda not sure why this is gated > > on authentication status, and not on "am I master or not" status. At least > > from a very cursory read ... > > -Daniel > > The code snippet in question is: > > > if (drm_is_primary_client(file_priv) && > user_srf->master != file_priv->master) { > DRM_ERROR("Trying to reference surface outside of" >" master domain.\n"); > ret = -EACCES; > goto out_bad_resource; > } > > > In gem term's this means a client can't open a
Re: [PATCHv3 22/23] drm/bridge: tc358767: add IRQ and HPD support
On 21/05/2019 17:18, Andrzej Hajda wrote: DisplayPort spec talks about doing the display-props reading and EDID reading when handling HPD. I think it would be best to change the code so that we read display props and EDID in HPD, but so that we also can read them later (when needed, probably bridge enable and get_modes) if we haven't done the reads already. I've had this in mind since I started the series, but as it didn't feel like a simple change, I left it for later. My approach and experience suggest that detect, should be rather lightweight and should not modify state, I am not even sure if it is called at all on forced connector. I just realized that this is not exactly perfect... Link training can adjust the link speed and/or number of lanes, although the driver doesn't support this at the moment. The speed and number of lanes affect the video modes that are possible, so they affect get_modes. So... I think the driver should set up the link fully before get_modes get called, instead of leaving the link setup to the point where we enable the bridge. Maybe... This is not exactly clear to me =). In any case, I think that's future work. I have changed the code to read the display props in get_modes(), and I have another small fix too. I'll send v4 this week, and hopefully we can get this merged. 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
[Bug 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #10 from tempel.jul...@gmail.com --- Nope, not related to it. Happens also with stable versions. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[GIT PULL] etnaviv-fixes for 5.2-rc3
Hi Daniel, hi Dave, please pull in this fix for a kernel crashing vmalloc buffer overrun in the etnaviv devcoredump code. Regards, Lucas The following changes since commit a188339ca5a396acc588e5851ed7e19f66b0ebd9: Linux 5.2-rc1 (2019-05-19 15:47:09 -0700) are available in the Git repository at: https://git.pengutronix.de/git/lst/linux etnaviv/fixes for you to fetch changes up to 1396500d673bd027683a0609ff84dca7eb6ea2e7: drm/etnaviv: lock MMU while dumping core (2019-05-27 16:08:38 +0200) Lucas Stach (1): drm/etnaviv: lock MMU while dumping core drivers/gpu/drm/etnaviv/etnaviv_dump.c | 5 + 1 file changed, 5 insertions(+) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #9 from tempel.jul...@gmail.com --- Until I get a new GPU or a FreeSync display, I use amdgpu.dc=1 only for testing purposes. So I can't judge if this is a regression or has always existed. But I gave Linux 4.19.46 LTS a try and it shows the same behavior. Hm, maybe no one noticed because pageflipping wasn't working before this commit? https://gitlab.freedesktop.org/xorg/driver/xf86-video-amdgpu/commit/bf61e6d7ac1a5754b1026d7f80acf25ef622c491 Will retest with latest stable versions of xorg / amdgpu DDX. It's btw. really not happening in every game, e.g. Elex seems to be 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: [PATCH 13/13] drm: allow render capable master with DRM_AUTH ioctls
On 5/27/19 3:16 PM, Daniel Vetter wrote: On Mon, May 27, 2019 at 02:39:18PM +0200, Thomas Hellstrom wrote: On 5/27/19 10:17 AM, Emil Velikov wrote: From: Emil Velikov There are cases (in mesa and applications) where one would open the primary node without properly authenticating the client. Sometimes we don't check if the authentication succeeds, but there's also cases we simply forget to do it. The former was a case for Mesa where it did not not check the return value of drmGetMagic() [1]. That was fixed recently although, there's the question of older drivers or other apps that exbibit this behaviour. While omitting the call results in issues as seen in [2] and [3]. In the libva case, libva itself doesn't authenticate the DRM client and the vaGetDisplayDRM documentation doesn't mention if the app should either. As of today, the official vainfo utility doesn't authenticate. To workaround issues like these, some users resort to running their apps under sudo. Which admittedly isn't always a good idea. Since any DRIVER_RENDER driver has sufficient isolation between clients, we can use that, for unauthenticated [primary node] ioctls that require DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. v2: - Rework/simplify if check (Daniel V) - Add examples to commit messages, elaborate. (Daniel V) v3: - Use single unlikely (Daniel V) v4: - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU issue is fixed with earlier patch. [1] https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136 [2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html [3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1 Testcase: igt/core_unauth_vs_render Cc: intel-...@lists.freedesktop.org Signed-off-by: Emil Velikov Reviewed-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190114085408.15933-2-emil.l.veli...@gmail.com --- drivers/gpu/drm/drm_ioctl.c | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 9841c0076f02..b64b022a2b29 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -511,6 +511,13 @@ int drm_version(struct drm_device *dev, void *data, return err; } +static inline bool +drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags) +{ + return drm_core_check_feature(dev, DRIVER_RENDER) && + (flags & DRM_RENDER_ALLOW); +} + /** * drm_ioctl_permit - Check ioctl permissions against caller * @@ -525,14 +532,19 @@ int drm_version(struct drm_device *dev, void *data, */ int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) { + const struct drm_device *dev = file_priv->minor->dev; + /* ROOT_ONLY is only for CAP_SYS_ADMIN */ if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN))) return -EACCES; - /* AUTH is only for authenticated or render client */ - if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) && -!file_priv->authenticated)) - return -EACCES; + /* AUTH is only for master ... */ + if (unlikely((flags & DRM_AUTH) && drm_is_primary_client(file_priv))) { + /* authenticated ones, or render capable on DRM_RENDER_ALLOW. */ + if (!file_priv->authenticated && + !drm_render_driver_and_ioctl(dev, flags)) + return -EACCES; + } This breaks vmwgfx primary client authentication in the surface_reference ioctl, which takes different paths in case of render clients and primary clients, but adding an auth check in the primary path in the vmwgfx code should fix this. Hm yeah we need to adjust that ... otoh kinda not sure why this is gated on authentication status, and not on "am I master or not" status. At least from a very cursory read ... -Daniel The code snippet in question is: if (drm_is_primary_client(file_priv) && user_srf->master != file_priv->master) { DRM_ERROR("Trying to reference surface outside of" " master domain.\n"); ret = -EACCES; goto out_bad_resource; } In gem term's this means a client can't open a surface that hasn't been flinked by a client in the same master realm: You can't read from resources belonging to another X server's clients /Thomas /Thomas /* MASTER is only for master or control clients */ if (unlikely((flags & DRM_MASTER) && ___ 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
Re: dpms mode change with wayland on iMX.6
On Mon, May 27, 2019 at 3:42 PM Pintu Agarwal wrote: > > On Mon, May 27, 2019 at 12:41 PM Pintu Agarwal wrote: > > > > Dear All, > > > > I have a iMX.6 (arm 32) board with Linux Kernel 3.10 and debian > > platform running. > > The board is connected to one LCD screen and one HDMI monitor. > > It have DRM + Wayland setup for display. > > Also, I noticed that it have two dri interface: > > /dev/dri/card0 > > /dev/dri/card1 > > > > I am not very familiar with Linux Graphics/Display subsystem, so I am > > looking for some help here. > > > > My requirement is that I have turn off HDMI display screen using a > > command line utility or test program. > > I learn that for X-server we can use xset : xset dpms force off (and > > it works on my ubuntu desktop with 16.04). > > > > However this command does not exists on my board. > > So, my question is: > > Is there any equivalent DPMS commands for Wayland/Wetson? > > > > - > > Further, in order to explore more, I cloned libdrm code from here: > > url = https://gitlab.freedesktop.org/mesa/drm > > > > Then I found some test utility under: drm/tests folder. > > After exploring more, and few modification, somehow I could able to > > cross-compile "proptest" for my board using below: > > arm-linux-gnueabi-gcc -o proptest.out proptest.c > > -I./target/libdrm_include/ -L./target/libdrm_lib/ -ldrm > > > > I found that "/dev/dri/card0" is not working with this test. > > So, I changed the test utility like this: > > fd = drmOpen("imx-drm", NULL); > > OR > > fd = open("/dev/dri/card1", O_RDWR); > > > > When I default run it on my board, I see that "Connector_id: 29" is > > showing for the HDMI display and it can support DPMS property. > > {{{ > > Connector 29 (11-1) > > 1 EDID: > > flags: immutable blob > > blobs: > > > > value: > > XXX > > 2 DPMS: > > flags: enum > > enums: On=0 Standby=1 Suspend=2 Off=3 > > value: 0 > > CRTC 24 > > CRTC 27 > > }}} > > > > Then, when I try to run it using below command: > > # ./proptest.out 29 connector 2 3 > > > > The program just returns successfully without any errors, but nothing > > happens. The display does not turns off. > > I saw that in my kernel 3.10 the ioctl(DRM_IOCTL_MODE_SETPROPERTY) is > > already supported under DRM. > > > > So, I am wondering what is the right way to verify DPMS mode property > > on wayland ? > > > > If anybody have any suggestions, please help me. > > > > > > Thanks, > > Pintu > > + etna...@lists.freedesktop.org One more point: Although it is having Kernel 3.10, but the DRM modules were upgraded to Kernel 4.9.xx from mainline. So, latest DRM changes are already applied. Thanks, Pintu ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/05/27, Daniel Vetter wrote: > On Mon, May 27, 2019 at 09:17:29AM +0100, Emil Velikov wrote: > > From: Emil Velikov > > > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > > render node. A seemingly deliberate design decision. > > > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > > yet not all userspace checks if it's authenticated, but instead uses > > uncommon assumptions. > > > > After days of digging through git log and testing, only a single (ab)use > > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > > assuming that failure implies lack of authentication. > > Maybe correction here: Id does not care about authentication at all. It > wants to figure out whether it has access to modeset resources, which is > something entirely different, but happened to match in amdgpu's case. > It feels like we have conflated the two (perhaps due to historical reasons) and I'm not 100% sure which one the AMDGPU code inspects. How about: Hence we can drop the DRM_AUTH all together (details in follow-up patch) yet that cause a regression in some userspace, when it conflates the authentication status with access to modeset resources. After days of digging through git log and testing, only a single (ab)use was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl. > > Affected versions are: > > - the whole 18.2.x series, which is EOL > > - the whole 18.3.x series, which is EOL > > - the 19.0.x series, prior to 19.0.4 > > Hm I think I'm blind, I'm not seeing the fix merged to mesa master. Still > there on gitlab: > > https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/amd/vulkan/radv_device.c#L291 > > What am I missing? > Opted for a simple, more generic solution see commit c962a78f18284e2971301bf68c9c60500ca398e4 This way, the same check is: - enforced where it's used - present for all Mesa Vulkan drivers Will include the sha + commit title for v2. > > Add a special quirk for that case, thus we can drop DRM_AUTH bits as > > mentioned earlier. > > > > Since all the affected userspace is EOL, we also add a kconfig option > > to disable this quirk. > > > > The whole approach is inspired by DRIVER_KMS_LEGACY_CONTEXT > > > > Cc: Alex Deucher > > Cc: Christian König > > Cc: amd-...@lists.freedesktop.org > > Cc: David Airlie > > Cc: Daniel Vetter > > Signed-off-by: Emil Velikov > > Aside from the nits I think reasonable approach. Great, glad to hear. Now all we need is to bribe^Wconvince Christian. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 3/6] drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz
The pixel clock unit in the first two registers (0x00 and 0x01) of sii9022 is 10kHz, not 1kHz as in struct drm_display_mode. Division by 10 fixes the issue. Signed-off-by: Jyri Sarha Reviewed-by: Andrzej Hajda Reviewed-by: Laurent Pinchart --- drivers/gpu/drm/bridge/sii902x.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 128d8fdb4ba6..19f982a00dba 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -249,10 +249,11 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge, struct regmap *regmap = sii902x->regmap; u8 buf[HDMI_INFOFRAME_SIZE(AVI)]; struct hdmi_avi_infoframe frame; + u16 pixel_clock_10kHz = adj->clock / 10; int ret; - buf[0] = adj->clock; - buf[1] = adj->clock >> 8; + buf[0] = pixel_clock_10kHz & 0xff; + buf[1] = pixel_clock_10kHz >> 8; buf[2] = adj->vrefresh; buf[3] = 0x00; buf[4] = adj->hdisplay; -- 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
[PATCH v8 6/6] drm/bridge: sii902x: Implement HDMI audio support
Implement HDMI audio support by using ASoC HDMI codec. The commit implements the necessary callbacks and configuration for the HDMI codec and registers a virtual platform device for the codec to attach. Signed-off-by: Jyri Sarha Reviewed-by: Andrzej Hajda --- drivers/gpu/drm/bridge/sii902x.c | 469 ++- 1 file changed, 463 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 19f982a00dba..bc3325c5e5c3 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -34,6 +35,8 @@ #include #include +#include + #define SII902X_TPI_VIDEO_DATA 0x0 #define SII902X_TPI_PIXEL_REPETITION 0x8 @@ -75,6 +78,77 @@ #define SII902X_AVI_POWER_STATE_MSKGENMASK(1, 0) #define SII902X_AVI_POWER_STATE_D(l) ((l) & SII902X_AVI_POWER_STATE_MSK) +/* Audio */ +#define SII902X_TPI_I2S_ENABLE_MAPPING_REG 0x1f +#define SII902X_TPI_I2S_CONFIG_FIFO0 (0 << 0) +#define SII902X_TPI_I2S_CONFIG_FIFO1 (1 << 0) +#define SII902X_TPI_I2S_CONFIG_FIFO2 (2 << 0) +#define SII902X_TPI_I2S_CONFIG_FIFO3 (3 << 0) +#define SII902X_TPI_I2S_LEFT_RIGHT_SWAP(1 << 2) +#define SII902X_TPI_I2S_AUTO_DOWNSAMPLE(1 << 3) +#define SII902X_TPI_I2S_SELECT_SD0 (0 << 4) +#define SII902X_TPI_I2S_SELECT_SD1 (1 << 4) +#define SII902X_TPI_I2S_SELECT_SD2 (2 << 4) +#define SII902X_TPI_I2S_SELECT_SD3 (3 << 4) +#define SII902X_TPI_I2S_FIFO_ENABLE(1 << 7) + +#define SII902X_TPI_I2S_INPUT_CONFIG_REG 0x20 +#define SII902X_TPI_I2S_FIRST_BIT_SHIFT_YES(0 << 0) +#define SII902X_TPI_I2S_FIRST_BIT_SHIFT_NO (1 << 0) +#define SII902X_TPI_I2S_SD_DIRECTION_MSB_FIRST (0 << 1) +#define SII902X_TPI_I2S_SD_DIRECTION_LSB_FIRST (1 << 1) +#define SII902X_TPI_I2S_SD_JUSTIFY_LEFT(0 << 2) +#define SII902X_TPI_I2S_SD_JUSTIFY_RIGHT (1 << 2) +#define SII902X_TPI_I2S_WS_POLARITY_LOW(0 << 3) +#define SII902X_TPI_I2S_WS_POLARITY_HIGH (1 << 3) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_128(0 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_256(1 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_384(2 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_512(3 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_768(4 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_1024 (5 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_1152 (6 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_192(7 << 4) +#define SII902X_TPI_I2S_SCK_EDGE_FALLING (0 << 7) +#define SII902X_TPI_I2S_SCK_EDGE_RISING(1 << 7) + +#define SII902X_TPI_I2S_STRM_HDR_BASE 0x21 +#define SII902X_TPI_I2S_STRM_HDR_SIZE 5 + +#define SII902X_TPI_AUDIO_CONFIG_BYTE2_REG 0x26 +#define SII902X_TPI_AUDIO_CODING_STREAM_HEADER (0 << 0) +#define SII902X_TPI_AUDIO_CODING_PCM (1 << 0) +#define SII902X_TPI_AUDIO_CODING_AC3 (2 << 0) +#define SII902X_TPI_AUDIO_CODING_MPEG1 (3 << 0) +#define SII902X_TPI_AUDIO_CODING_MP3 (4 << 0) +#define SII902X_TPI_AUDIO_CODING_MPEG2 (5 << 0) +#define SII902X_TPI_AUDIO_CODING_AAC (6 << 0) +#define SII902X_TPI_AUDIO_CODING_DTS (7 << 0) +#define SII902X_TPI_AUDIO_CODING_ATRAC (8 << 0) +#define SII902X_TPI_AUDIO_MUTE_DISABLE (0 << 4) +#define SII902X_TPI_AUDIO_MUTE_ENABLE (1 << 4) +#define SII902X_TPI_AUDIO_LAYOUT_2_CHANNELS(0 << 5) +#define SII902X_TPI_AUDIO_LAYOUT_8_CHANNELS(1 << 5) +#define SII902X_TPI_AUDIO_INTERFACE_DISABLE(0 << 6) +#define SII902X_TPI_AUDIO_INTERFACE_SPDIF (1 << 6) +#define SII902X_TPI_AUDIO_INTERFACE_I2S(2 << 6) + +#define SII902X_TPI_AUDIO_CONFIG_BYTE3_REG 0x27 +#define SII902X_TPI_AUDIO_FREQ_STREAM (0 << 3) +#define SII902X_TPI_AUDIO_FREQ_32KHZ (1 << 3) +#define SII902X_TPI_AUDIO_FREQ_44KHZ (2 << 3) +#define SII902X_TPI_AUDIO_FREQ_48KHZ (3 << 3) +#define SII902X_TPI_AUDIO_FREQ_88KHZ (4 << 3) +#define SII902X_TPI_AUDIO_FREQ_96KHZ (5 << 3) +#define SII902X_TPI_AUDIO_FREQ_176KHZ (6 << 3) +#define SII902X_TPI_AUDIO_FREQ_192KHZ (7 << 3) +#define SII902X_TPI_AUDIO_SAMPLE_SIZE_STREAM (0 << 6) +#define SII902X_TPI_AUDIO_SAMPLE_SIZE_16 (1 << 6) +#define SII902X_TPI_AUDIO_SAMPLE_SIZE_20
[PATCH v8 1/6] drm/bridge: sii902x: add input_bus_flags
From: Tomi Valkeinen The driver always sets InputBusFmt:EDGE to 0 (falling edge). Add drm_bridge_timings's input_bus_flags to reflect that the bridge samples on falling edges. Signed-off-by: Tomi Valkeinen Signed-off-by: Jyri Sarha Reviewed-by: Andrzej Hajda Reviewed-by: Laurent Pinchart --- drivers/gpu/drm/bridge/sii902x.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index cdb8dfdb2dff..0d3d730b97ff 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -461,6 +461,12 @@ static int sii902x_i2c_bypass_deselect(struct i2c_mux_core *mux, u32 chan_id) return 0; } +static const struct drm_bridge_timings default_sii902x_timings = { + .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE +| DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE +| DRM_BUS_FLAG_DE_HIGH, +}; + static int sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -531,6 +537,7 @@ static int sii902x_probe(struct i2c_client *client, sii902x->bridge.funcs = _bridge_funcs; sii902x->bridge.of_node = dev->of_node; + sii902x->bridge.timings = _sii902x_timings; drm_bridge_add(>bridge); i2c_set_clientdata(client, sii902x); -- 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
[PATCH v8 2/6] drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID
Set output mode to HDMI or DVI according to EDID HDMI signature. Signed-off-by: Jyri Sarha Reviewed-by: Andrzej Hajda Reviewed-by: Laurent Pinchart --- drivers/gpu/drm/bridge/sii902x.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 0d3d730b97ff..128d8fdb4ba6 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -181,12 +181,16 @@ static int sii902x_get_modes(struct drm_connector *connector) { struct sii902x *sii902x = connector_to_sii902x(connector); u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; + u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI; struct edid *edid; int num = 0, ret; edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]); drm_connector_update_edid_property(connector, edid); if (edid) { + if (drm_detect_hdmi_monitor(edid)) + output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI; + num = drm_add_edid_modes(connector, edid); kfree(edid); } @@ -196,6 +200,11 @@ static int sii902x_get_modes(struct drm_connector *connector) if (ret) return ret; + ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA, +SII902X_SYS_CTRL_OUTPUT_MODE, output_mode); + if (ret) + return ret; + return num; } -- 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
[PATCH v8 0/6] drm/bridge: sii902x: HDMI-audio support and some fixes
I think these should be ready for applying to drm-misc. Changes since v7: - Debased on top of the lasts drm-misc-next and tested - "dt-bindings: display: sii902x: Add HDMI audio bindings" - Dropped off "or higher to avoid conflict with video ports" and added "Reviewed-by: Rob Herring " Ther previous round: https://patchwork.kernel.org/cover/10919173/ Jyri Sarha (5): drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz dt-bindings: display: sii902x: Remove trailing white space dt-bindings: display: sii902x: Add HDMI audio bindings drm/bridge: sii902x: Implement HDMI audio support Tomi Valkeinen (1): drm/bridge: sii902x: add input_bus_flags .../bindings/display/bridge/sii902x.txt | 42 +- drivers/gpu/drm/bridge/sii902x.c | 488 +- 2 files changed, 522 insertions(+), 8 deletions(-) -- 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
[PATCH v8 5/6] dt-bindings: display: sii902x: Add HDMI audio bindings
The sii902x chip family supports also HDMI audio. Add binding for describing the necessary i2s and mclk wiring for it. Signed-off-by: Jyri Sarha Reviewed-by: Rob Herring --- .../bindings/display/bridge/sii902x.txt | 40 +++ 1 file changed, 40 insertions(+) diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt index c4c1855ca654..2df44b7d3821 100644 --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt @@ -9,6 +9,40 @@ Optional properties: about hotplug events. - reset-gpios: OF device-tree gpio specification for RST_N pin. + HDMI audio properties: + - #sound-dai-cells: <0> or <1>. <0> if only i2s or spdif pin + is wired, <1> if the both are wired. HDMI audio is + configured only if this property is found. + - sil,i2s-data-lanes: Array of up to 4 integers with values of 0-3 + Each integer indicates which i2s pin is connected to which + audio fifo. The first integer selects i2s audio pin for the + first audio fifo#0 (HDMI channels 1&2), second for fifo#1 + (HDMI channels 3&4), and so on. There is 4 fifos and 4 i2s + pins (SD0 - SD3). Any i2s pin can be connected to any fifo, + but there can be no gaps. E.g. an i2s pin must be mapped to + fifo#0 and fifo#1 before mapping a channel to fifo#2. Default + value is <0>, describing SD0 pin beiging routed to hdmi audio + fifo #0. + - clocks: phandle and clock specifier for each clock listed in + the clock-names property + - clock-names: "mclk" + Describes SII902x MCLK input. MCLK is used to produce + HDMI audio CTS values. This property is required if + "#sound-dai-cells"-property is present. This property follows + Documentation/devicetree/bindings/clock/clock-bindings.txt + consumer binding. + + If HDMI audio is configured the sii902x device becomes an I2S + and/or spdif audio codec component (e.g a digital audio sink), + that can be used in configuring a full audio devices with + simple-card or audio-graph-card binding. See their binding + documents on how to describe the way the sii902x device is + connected to the rest of the audio system: + Documentation/devicetree/bindings/sound/simple-card.txt + Documentation/devicetree/bindings/sound/audio-graph-card.txt + Note: In case of the audio-graph-card binding the used port + index should be 3. + Optional subnodes: - video input: this subnode can contain a video input port node to connect the bridge to a display controller output (See this @@ -21,6 +55,12 @@ Example: compatible = "sil,sii9022"; reg = <0x39>; reset-gpios = < 1 0>; + + #sound-dai-cells = <0>; + sil,i2s-data-lanes = < 0 1 2 >; + clocks = <>; + clock-names = "mclk"; + ports { #address-cells = <1>; #size-cells = <0>; -- 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
[PATCH v8 4/6] dt-bindings: display: sii902x: Remove trailing white space
Remove trailing white space from sii902x display bridge binding. Signed-off-by: Jyri Sarha Reviewed-by: Laurent Pinchart Reviewed-by: Rob Herring --- Documentation/devicetree/bindings/display/bridge/sii902x.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt index 72d2dc6c3e6b..c4c1855ca654 100644 --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt @@ -5,7 +5,7 @@ Required properties: - reg: i2c address of the bridge Optional properties: - - interrupts: describe the interrupt line used to inform the host + - interrupts: describe the interrupt line used to inform the host about hotplug events. - reset-gpios: OF device-tree gpio specification for RST_N pin. -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm/vmwgfx: remove custom ioctl io encoding check
On 5/27/19 2:35 PM, Emil Velikov wrote: Hi Thomas, On 2019/05/27, Thomas Hellstrom wrote: I think we might be talking past each other, let's take a step back: - as of previous patch, all of vmwgfx ioctls size is consistently handled by the core I don't think I follow you here, AFAICT patch 3/5 only affects and relaxes the execbuf checking (and in fact a little more than I would like)? Precisely, it makes execbuf ioctl behave like all other ioctls - both vmwgfx and rest of drm. But we're still enforcing a non-relaxed size check for the other vmwgfx private ioctls, right? Which is relaxed, together with the directions, in this commit? (Not that it matters much to the discussion, though). - handling of featue flags, as always, is responsibility of the driver ifself - with this patch, ioctl direction is also handled by core. Here core ensures we only copy in/out as much data as the kernel implementation can handle. Let's consider the following real world example - msm and virtio_gpu. An in field of an _IOW ioctl becomes in/out aka _IORW ioctl. - we add a flag to annotate/request the out, as always invalid flags are return -EINVAL - we change the ioctl encoding As currently handled by core DRM, old kernel/new userspace and vice-versa works just fine. Sadly, vmwgfx will error out, while it could be avoided. IMO basically we have a tradeoff between strict checking in this case, and new user-space vs old kernel "hazzle-free" transition in the relaxed case. Precisely. If I read the code correctly, ATM new userspace will fail against old kernels. Unless userspace writes two versions of the ioctl - with with each encoding. As said above, I'll gladly adjust core and/or others, if this relaxed approach causes an issue somewhere. A specific use-case, real or hypothetical will be appreciated. To me there are two important reasons to keep the strict approach. 1) Avoid user-space mistakes early in the development cycle. We can't distinguish between buggy user-space and "new" user-space. This is important because of [a]) below. Can you provide a concrete example, please? OK, let's say you were developing fence wait functionality. Like vmw_fence_obj_wait ioctl. Then suddenly you started to wonder why the wait never timed out as it should. The reason turn out to be that signals were restarting the waits with the original timeout. So you change the ioctl from W to RW and add a kernel-computed time to the argument. Everything is fine, except that you forget to change this in a user-space application somewhere. So now what happens, is that that user-space bug can live on undetected as in 1), and that means you can never go back and implement a stricter check because that would completely break old user-space. The current code will trap (and has historically trapped) code like this. That's mainly why I'm reluctant to give it up, but I guess it can be conditionally compiled in for debug purposes. 2) Catch a lot of fuzzer combinations and error out early instead of forwarding them to the ioctl function where they may cause harm. Struggling to see why this is a problem? At some point the fuzzer will get past this first line of defence, so we want to make the rest of the ioctl is robust. I think the new user-space vs old kernel can be handled nicely in user- space with feature flags or API versions. That's the way we've handled them up to now? How is a feature flag doing to help if the encoding changes from _IOW to _IORW? Ah, you're referring to old user-space new kernel? Yes, I was probably reading a bit too fast. Sorry about that. So we're basically landing in a tradeoff between trapping problems like above, and hazzle-free ioctl argument definition change. OK, so I'm ok with that as long as there is a way we can compile in strict checking, which will likely has to be as a vmwgfx-specific wrapper. /Thomas Thanks Emil ___ 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 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
Am 27.05.19 um 15:26 schrieb Emil Velikov: > On 2019/05/27, Daniel Vetter wrote: >> On Mon, May 27, 2019 at 10:47:39AM +, Koenig, Christian wrote: >>> Am 27.05.19 um 10:17 schrieb Emil Velikov: From: Emil Velikov Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the render node. A seemingly deliberate design decision. Hence we can drop the DRM_AUTH all together (details in follow-up patch) yet not all userspace checks if it's authenticated, but instead uses uncommon assumptions. After days of digging through git log and testing, only a single (ab)use was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and assuming that failure implies lack of authentication. Affected versions are: - the whole 18.2.x series, which is EOL - the whole 18.3.x series, which is EOL - the 19.0.x series, prior to 19.0.4 >>> Well you could have saved your time, cause this is still a NAK. >>> >>> For the record: I strongly think that we don't want to expose any render >>> functionality on the primary node. >>> >>> To even go a step further I would say that at least on AMD hardware we >>> should completely disable DRI2 for one of the next generations. >>> >>> As a follow up I would then completely disallow the DRM authentication >>> for amdgpu, so that the command submission interface on the primary node >>> can only be used by the display server. >> So amdgpu is running in one direction, while everyone else is running in >> the other direction? Please note that your patch to change i915 landed >> already, so that ship is sailing (but we could ofc revert that back >> again). >> >> Imo really not a great idea if we do a amdgpu vs. everyone else split >> here. If we want to deprecate dri2/flink/rendering on primary nodes across >> the stack, then that should be a stack wide decision, not an amdgpu one. >> > Cannot agree more - I would love to see drivers stay consistent. Yeah, completely agree to that. That's why I think we should not do this at all and just let Intel fix it's userspace bugs :P Anyway my concern is really that we should stop extending functionality on the primary node. E.g. the render node is for use by the clients and the primary node for mode setting and use by the display server only. Regards, Christian. > Fwiw, this series consistently paves the way toward nuking DRM_AUTH ;-) > > Thanks > Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] drm: imx: Add NWL MIPI DSI host controller support
Am Mittwoch, den 08.05.2019, 19:18 +0200 schrieb Guido Günther: > Hi, > On Thu, Mar 07, 2019 at 11:30:51AM +0100, Guido Günther wrote: > > This adds initial support for the NWL MIPI DSI Host controller found on > > i.MX8 > > SoCs. > > > > It adds support for the i.MX8MQ but the same IP core can also be found on > > e.g. > > i.MX8QXP. I added the necessary hooks to support other imx8 variants but > > since > > I only have imx8mq boards to test I omitted the platform data for other > > SoCs. > > > > The code is based on NXPs BSP so I added Robert Chiras as Co-authored-by but > > I'm happy to swap Author: and Co-authored-by: if that looks more > > appropriate. > > The most notable changes over the BSP driver are > > - Calculate HS mode timing from phy_configure_opts_mipi_dphy > > - Perform all clock setup via DT > > - Merge nwl-imx and nwl drivers > > - Add B0 silion revision quirk > > > > Posting this is likely a bit premature (hence v0) but I wanted for one show > > how > > this hooks into the mixel dphy posted earlier [1] and avoid duplicating > > work. > > So if there's other code out there doing the same I'm be happy to merge > > efforts. > > Since this is likely not going anywhere until we have a dcss driver aimed > for mainline I'm not going spam the list with further revisions. However > the 5.x version is maintained here: > > > https://source.puri.sm/guido.gunther/linux-imx8/tree/forward-upstream/next-20190506/imx-nwl/v1-wip > > Feedback is still welcome. It'll eventually be forwarded to newer > linux-next versions. > > Changes over the posted version are: > > - Add quirk for IMQ8MQ silicon B0 revision to not mess with the > system reset controller on power down since enable won't work > afterwards otherwise. > - Disable tx esc clock *after* the phy power down to unbreak > disable/enable (unblank/blank) > - Drop devm_free_irq() handled by the device driver core > - Add ports to dt binding docs > - Select GENERIC_PHY_MIPI_DPHY instead of GENERIC_PHY for > phy_mipi_dphy_get_default_config > - Include drm_print.h to fix build on next-20190408 > - Drop some debugging messages > - Newline terminate all DRM_ printouts > > If somebody is working on DCSS support it'd be cool to know since this > driver is currently a component of imx-display-subsystem which will only > work out if dcss is handled like this as well. We have been looking at how to support DCSS in mainline for a while, but most of the actual work got pushed behind in schedule due to other priorities. One thing I can can say for certain is that DCSS should not be integrated into imx-drm. It's a totally different hardware and downstream clearly shows that it's not a good idea to cram it into imx- drm. Also the artificial split between hardware control in drivers/gpu/imx/dcss and the DRM driver is just cargo-cult from the IPU/imx-drm split. For the IPU we did it as the IPU has legs in both DRM for the output parts and V4L2 for the input parts. As the DCSS has no video input capabilities the driver could be simplified a lot by moving it all into a single DRM driver. Regards, Lucas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On Mon, May 27, 2019 at 3:26 PM Daniel Vetter wrote: > > On Mon, May 27, 2019 at 01:52:05PM +0100, Emil Velikov wrote: > > On 2019/05/27, Koenig, Christian wrote: > > > Am 27.05.19 um 14:05 schrieb Emil Velikov: > > > > On 2019/05/27, Koenig, Christian wrote: > > > >> Am 27.05.19 um 10:17 schrieb Emil Velikov: > > > >>> From: Emil Velikov > > > >>> > > > >>> Currently one can circumvent DRM_AUTH, when the ioctl is exposed via > > > >>> the > > > >>> render node. A seemingly deliberate design decision. > > > >>> > > > >>> Hence we can drop the DRM_AUTH all together (details in follow-up > > > >>> patch) > > > >>> yet not all userspace checks if it's authenticated, but instead uses > > > >>> uncommon assumptions. > > > >>> > > > >>> After days of digging through git log and testing, only a eingle > > > >>> (ab)use > > > >>> was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > > > >>> assuming that failure implies lack of authentication. > > > >>> > > > >>> Affected versions are: > > > >>>- the whole 18.2.x series, which is EOL > > > >>>- the whole 18.3.x series, which is EOL > > > >>>- the 19.0.x series, prior to 19.0.4 > > > >> Well you could have saved your time, cause this is still a NAK. > > > >> > > > > Did I mention that I've spent quite a bit of time in AMDVLK? Even fixed > > > > a bug while I was there :-) > > > > > > Yeah, thanks for doing this. > > > > > > But we have done so much stuff with customers which can't be audited > > > this way, that I still have a really bad feeling about this :/ > > > > > Fair point, I've already thought about those. > > > > Example A: customer X, using closed source/private software Y. > > Ioctls A, B and C require the old behaviour - simply add FORCE_AUTH to > > the A B C and carry on happily. > > Hm, if the entire concern here is all the bazillion different versions of > blobs shipped in the past few years, why can't we just have a revert of > this in the amdgpu DKMS? Not like one more patch among the hundres will > matter. I'd suspect that the overlap of people wanting to run the blobs > and people who don't run the DKMS or similar is roughly 0. Always been the > case here at Intel at least. > > If there's other stuff that we need to audit (like rocm or whatever), then > we should look into those ofc. Also note that Emil's patch actually doesn't break anything, since default y. So you don't even need the revert (except maybe in 10 years or so when we throw that option out). -Daniel > > Example B: the team, as a whole thinks that this will be problematic for > > customer X - add FORCE_AUTH to all ioctls and carry on. > > > > I do see and understand why anyone can be hesitant about the series. > > > > IMHO the above examples, illustrate quite reasonable compromise between > > open-source and closed-source/private solutions. > > > > Don't you agree? > > > > > >> For the record: I strongly think that we don't want to expose any > > > >> render > > > >> functionality on the primary node. > > > >> > > > >> To even go a step further I would say that at least on AMD hardware we > > > >> should completely disable DRI2 for one of the next generations. > > > >> > > > > It's doable and overall pretty neat idea. > > > > > > > > There is a consern that this approach may cause far more regressions > > > > that this series. We are talking about years worth of Mesa drivers (et > > > > al) that depend on render functionality exposed via the primary node. > > > > > > Yeah, that's I'm perfectly aware of. It's the reason why I said we > > > should only do it for new hardware generations. > > > > > > But at some point I think we should do this and get rid of > > > authentication/flink/DRI2/ > > > > > Fwiw I share a similar sentiment. If you've looked through the series, > > this is effectively step 1, towards nuking DRM_AUTH :-) > > > > > > I'm OK with writing the patches, but it'll be up-to the AMDGPU team to > > > > follow-up with any regressions. Are you ok with that? > > > > > > As long as we have a check like adev->family_id > > > > WHATEVER_IS_THE_CURRENT_LATEST_UPSTREAM_HW, I think we are fine with that. > > > > > > If I understood Michel correctly xf86-video-amdgpu should work, but > > > modeset might break and needs a patch. > > > > > Unless I have concrete WHATEVER_IS_THE... I cannot do much here :-( > > > > > > Thanks > > Emil > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- 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 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/05/27, Daniel Vetter wrote: > On Mon, May 27, 2019 at 10:47:39AM +, Koenig, Christian wrote: > > Am 27.05.19 um 10:17 schrieb Emil Velikov: > > > From: Emil Velikov > > > > > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > > > render node. A seemingly deliberate design decision. > > > > > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > > > yet not all userspace checks if it's authenticated, but instead uses > > > uncommon assumptions. > > > > > > After days of digging through git log and testing, only a single (ab)use > > > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > > > assuming that failure implies lack of authentication. > > > > > > Affected versions are: > > > - the whole 18.2.x series, which is EOL > > > - the whole 18.3.x series, which is EOL > > > - the 19.0.x series, prior to 19.0.4 > > > > Well you could have saved your time, cause this is still a NAK. > > > > For the record: I strongly think that we don't want to expose any render > > functionality on the primary node. > > > > To even go a step further I would say that at least on AMD hardware we > > should completely disable DRI2 for one of the next generations. > > > > As a follow up I would then completely disallow the DRM authentication > > for amdgpu, so that the command submission interface on the primary node > > can only be used by the display server. > > So amdgpu is running in one direction, while everyone else is running in > the other direction? Please note that your patch to change i915 landed > already, so that ship is sailing (but we could ofc revert that back > again). > > Imo really not a great idea if we do a amdgpu vs. everyone else split > here. If we want to deprecate dri2/flink/rendering on primary nodes across > the stack, then that should be a stack wide decision, not an amdgpu one. > Cannot agree more - I would love to see drivers stay consistent. Fwiw, this series consistently paves the way toward nuking DRM_AUTH ;-) Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On Mon, May 27, 2019 at 01:52:05PM +0100, Emil Velikov wrote: > On 2019/05/27, Koenig, Christian wrote: > > Am 27.05.19 um 14:05 schrieb Emil Velikov: > > > On 2019/05/27, Koenig, Christian wrote: > > >> Am 27.05.19 um 10:17 schrieb Emil Velikov: > > >>> From: Emil Velikov > > >>> > > >>> Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > > >>> render node. A seemingly deliberate design decision. > > >>> > > >>> Hence we can drop the DRM_AUTH all together (details in follow-up patch) > > >>> yet not all userspace checks if it's authenticated, but instead uses > > >>> uncommon assumptions. > > >>> > > >>> After days of digging through git log and testing, only a eingle (ab)use > > >>> was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > > >>> assuming that failure implies lack of authentication. > > >>> > > >>> Affected versions are: > > >>>- the whole 18.2.x series, which is EOL > > >>>- the whole 18.3.x series, which is EOL > > >>>- the 19.0.x series, prior to 19.0.4 > > >> Well you could have saved your time, cause this is still a NAK. > > >> > > > Did I mention that I've spent quite a bit of time in AMDVLK? Even fixed > > > a bug while I was there :-) > > > > Yeah, thanks for doing this. > > > > But we have done so much stuff with customers which can't be audited > > this way, that I still have a really bad feeling about this :/ > > > Fair point, I've already thought about those. > > Example A: customer X, using closed source/private software Y. > Ioctls A, B and C require the old behaviour - simply add FORCE_AUTH to > the A B C and carry on happily. Hm, if the entire concern here is all the bazillion different versions of blobs shipped in the past few years, why can't we just have a revert of this in the amdgpu DKMS? Not like one more patch among the hundres will matter. I'd suspect that the overlap of people wanting to run the blobs and people who don't run the DKMS or similar is roughly 0. Always been the case here at Intel at least. If there's other stuff that we need to audit (like rocm or whatever), then we should look into those ofc. -Daniel > > Example B: the team, as a whole thinks that this will be problematic for > customer X - add FORCE_AUTH to all ioctls and carry on. > > I do see and understand why anyone can be hesitant about the series. > > IMHO the above examples, illustrate quite reasonable compromise between > open-source and closed-source/private solutions. > > Don't you agree? > > > >> For the record: I strongly think that we don't want to expose any render > > >> functionality on the primary node. > > >> > > >> To even go a step further I would say that at least on AMD hardware we > > >> should completely disable DRI2 for one of the next generations. > > >> > > > It's doable and overall pretty neat idea. > > > > > > There is a consern that this approach may cause far more regressions > > > that this series. We are talking about years worth of Mesa drivers (et > > > al) that depend on render functionality exposed via the primary node. > > > > Yeah, that's I'm perfectly aware of. It's the reason why I said we > > should only do it for new hardware generations. > > > > But at some point I think we should do this and get rid of > > authentication/flink/DRI2/ > > > Fwiw I share a similar sentiment. If you've looked through the series, > this is effectively step 1, towards nuking DRM_AUTH :-) > > > > I'm OK with writing the patches, but it'll be up-to the AMDGPU team to > > > follow-up with any regressions. Are you ok with that? > > > > As long as we have a check like adev->family_id > > > WHATEVER_IS_THE_CURRENT_LATEST_UPSTREAM_HW, I think we are fine with that. > > > > If I understood Michel correctly xf86-video-amdgpu should work, but > > modeset might break and needs a patch. > > > Unless I have concrete WHATEVER_IS_THE... I cannot do much here :-( > > > Thanks > Emil -- 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 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On Mon, May 27, 2019 at 10:47:39AM +, Koenig, Christian wrote: > Am 27.05.19 um 10:17 schrieb Emil Velikov: > > From: Emil Velikov > > > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > > render node. A seemingly deliberate design decision. > > > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > > yet not all userspace checks if it's authenticated, but instead uses > > uncommon assumptions. > > > > After days of digging through git log and testing, only a single (ab)use > > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > > assuming that failure implies lack of authentication. > > > > Affected versions are: > > - the whole 18.2.x series, which is EOL > > - the whole 18.3.x series, which is EOL > > - the 19.0.x series, prior to 19.0.4 > > Well you could have saved your time, cause this is still a NAK. > > For the record: I strongly think that we don't want to expose any render > functionality on the primary node. > > To even go a step further I would say that at least on AMD hardware we > should completely disable DRI2 for one of the next generations. > > As a follow up I would then completely disallow the DRM authentication > for amdgpu, so that the command submission interface on the primary node > can only be used by the display server. So amdgpu is running in one direction, while everyone else is running in the other direction? Please note that your patch to change i915 landed already, so that ship is sailing (but we could ofc revert that back again). Imo really not a great idea if we do a amdgpu vs. everyone else split here. If we want to deprecate dri2/flink/rendering on primary nodes across the stack, then that should be a stack wide decision, not an amdgpu one. Same for the other one, i.e. this stuff here. -Daniel > > Regards, > Christian. > > > > > Add a special quirk for that case, thus we can drop DRM_AUTH bits as > > mentioned earlier. > > > > Since all the affected userspace is EOL, we also add a kconfig option > > to disable this quirk. > > > > The whole approach is inspired by DRIVER_KMS_LEGACY_CONTEXT > > > > Cc: Alex Deucher > > Cc: Christian König > > Cc: amd-...@lists.freedesktop.org > > Cc: David Airlie > > Cc: Daniel Vetter > > Signed-off-by: Emil Velikov > > --- > > drivers/gpu/drm/amd/amdgpu/Kconfig | 16 > > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 12 +++- > > drivers/gpu/drm/drm_ioctl.c | 5 + > > include/drm/drm_ioctl.h | 17 + > > 4 files changed, 49 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig > > b/drivers/gpu/drm/amd/amdgpu/Kconfig > > index 9221e5489069..da415f445187 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/Kconfig > > +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig > > @@ -40,6 +40,22 @@ config DRM_AMDGPU_GART_DEBUGFS > > Selecting this option creates a debugfs file to inspect the mapped > > pages. Uses more memory for housekeeping, enable only for debugging. > > > > +config DRM_AMDGPU_FORCE_AUTH > > + bool "Force authentication check on AMDGPU_INFO ioctl" > > + default y > > + help > > + There were some version of the Mesa RADV drivers, which relied on > > + the ioctl failing, if the client is not authenticated. > > + > > + Namely, the following versions are affected: > > + - the whole 18.2.x series, which is EOL > > + - the whole 18.3.x series, which is EOL > > + - the 19.0.x series, prior to 19.0.4 > > + > > + Modern distributions, should disable this. That will allow various > > + other clients to work, that would otherwise require root privileges. > > + > > + > > source "drivers/gpu/drm/amd/acp/Kconfig" > > source "drivers/gpu/drm/amd/display/Kconfig" > > source "drivers/gpu/drm/amd/amdkfd/Kconfig" > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > index b17d0545728e..b8076929440b 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > @@ -1214,7 +1214,17 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { > > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, > > DRM_AUTH|DRM_RENDER_ALLOW), > > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_WAIT_IDLE, amdgpu_gem_wait_idle_ioctl, > > DRM_AUTH|DRM_RENDER_ALLOW), > > DRM_IOCTL_DEF_DRV(AMDGPU_CS, amdgpu_cs_ioctl, > > DRM_AUTH|DRM_RENDER_ALLOW), > > - DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, > > DRM_AUTH|DRM_RENDER_ALLOW), > > + /* The DRM_FORCE_AUTH is effectively a workaround for the RADV Mesa > > driver. > > +* This is required for Mesa: > > +* - the whole 18.2.x series, which is EOL > > +* - the whole 18.3.x series, which is EOL > > +* - the 19.0.x series, prior to 19.0.4 > > +*/ > > + DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, > > +#if defined(DRM_AMDGPU_FORCE_AUTH)
Re: [PATCH] drm/komeda: Added AFBC support for komeda driver
On Mon, May 27, 2019 at 06:51:18AM +, james qian wang (Arm Technology China) wrote: > On Fri, May 24, 2019 at 03:12:26PM +0300, Ville Syrjälä wrote: > > On Fri, May 24, 2019 at 11:10:09AM +, Brian Starkey wrote: > > > Hi, > > > > > > On Tue, May 21, 2019 at 09:45:58AM +0100, james qian wang (Arm Technology > > > China) wrote: > > > > On Thu, May 16, 2019 at 09:57:49PM +0800, Ayan Halder wrote: > > > > > On Thu, Apr 04, 2019 at 12:06:14PM +0100, james qian wang (Arm > > > > > Technology China) wrote: > > > > > > > > > > > > +static int > > > > > > +komeda_fb_afbc_size_check(struct komeda_fb *kfb, struct drm_file > > > > > > *file, > > > > > > + const struct drm_mode_fb_cmd2 *mode_cmd) > > > > > > +{ > > > > > > + struct drm_framebuffer *fb = >base; > > > > > > + const struct drm_format_info *info = fb->format; > > > > > > + struct drm_gem_object *obj; > > > > > > + u32 alignment_w = 0, alignment_h = 0, alignment_header; > > > > > > + u32 n_blocks = 0, min_size = 0; > > > > > > + > > > > > > + obj = drm_gem_object_lookup(file, mode_cmd->handles[0]); > > > > > > + if (!obj) { > > > > > > + DRM_DEBUG_KMS("Failed to lookup GEM object\n"); > > > > > > + return -ENOENT; > > > > > > + } > > > > > > + > > > > > > + switch (fb->modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) { > > > > > > + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8: > > > > > > + alignment_w = 32; > > > > > > + alignment_h = 8; > > > > > > + break; > > > > > > + case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16: > > > > > > + alignment_w = 16; > > > > > > + alignment_h = 16; > > > > > > + break; > > > > > > + default: > > > > > Can we have something like a warn here ? > > > > > > > > will add a WARN here. > > > > > > > > > > I think it's better not to. fb->modifier comes from > > > userspace, so a malicious app could spam us with WARNs, effectively > > > dos-ing the system. -EINVAL should be sufficient. > > > > Should probably check that the entire modifier+format is > > actually valid. Otherwise you risk passing on a bogus > > modifier deeper into the driver which may trigger > > interesting bugs. > > > > Also theoretically (however unlikely) some broken userspace > > might start to depend on the ability to create framebuffers > > with crap modifiers, which could later break if you change > > the way you handle the modifiers. Then you're stuck between > > the rock and hard place because you can't break existing > > userspace but you still want to change the way modifiers > > are handled in the kernel. > > > > Best not give userspace too much rope IMO. Two ways to go about > > that: > > 1) drm_any_plane_has_format() (assumes your .format_mod_supported() > >does its job properly) > > 2) roll your own > > > > -- > > Ville Syrjälä > > Intel > > Hi Brian & Ville: > > komed has a format+modifier check before the fb size check. > and for komeda_fb_create, the first step is do the format+modifier > check, the size check is the furthur check after the such format > valid check. and the detailed fb_create is like: > > struct drm_framebuffer * > komeda_fb_create(struct drm_device *dev, struct drm_file *file, >const struct drm_mode_fb_cmd2 *mode_cmd) > { > ... > /* Step 1: format+modifier valid check, if it can not be support, > * get_format_caps will return a NULL ptr. > */ > kfb->format_caps = komeda_get_format_caps(>fmt_tbl, > mode_cmd->pixel_format, > mode_cmd->modifier[0]); > if (!kfb->format_caps) { > DRM_DEBUG_KMS("FMT %x is not supported.\n", > mode_cmd->pixel_format); > kfree(kfb); > return ERR_PTR(-EINVAL); > } > > drm_helper_mode_fill_fb_struct(dev, >base, mode_cmd); > > /* step 2, do the size check */ > if (kfb->base.modifier) > ret = komeda_fb_afbc_size_check(kfb, file, mode_cmd); > else > ret = komeda_fb_none_afbc_size_check(mdev, kfb, file, mode_cmd); > if (ret < 0) > goto err_cleanup; > > ... > } > > So theoretically, the WARN in step2 is redundant if get_format_caps > function has no problem. :). the WARN here is only for reporting > the kernel BUG or code inconsitent with format caps check and the > fb size check. And I agree, basically it will not happene. > @Brian, I'm Ok to remove it. :) > > @Ville: > Basically komeda follow the way-1, but a little improvement for > matching komeda's requirement. for komeda which will do two level's > format+modifier check. > 1). In fb_create, A roughly check to see if the format+modifier can be > supported by current HW. Yeah, looks like it shouldn't allow any unspecfied modifiers to sneak through. So should be good. > 2). In plane_atomic_check: to see
Re: [PATCH 13/13] drm: allow render capable master with DRM_AUTH ioctls
On Mon, May 27, 2019 at 02:39:18PM +0200, Thomas Hellstrom wrote: > On 5/27/19 10:17 AM, Emil Velikov wrote: > > From: Emil Velikov > > > > There are cases (in mesa and applications) where one would open the > > primary node without properly authenticating the client. > > > > Sometimes we don't check if the authentication succeeds, but there's > > also cases we simply forget to do it. > > > > The former was a case for Mesa where it did not not check the return > > value of drmGetMagic() [1]. That was fixed recently although, there's > > the question of older drivers or other apps that exbibit this behaviour. > > > > While omitting the call results in issues as seen in [2] and [3]. > > > > In the libva case, libva itself doesn't authenticate the DRM client and > > the vaGetDisplayDRM documentation doesn't mention if the app should > > either. > > > > As of today, the official vainfo utility doesn't authenticate. > > > > To workaround issues like these, some users resort to running their apps > > under sudo. Which admittedly isn't always a good idea. > > > > Since any DRIVER_RENDER driver has sufficient isolation between clients, > > we can use that, for unauthenticated [primary node] ioctls that require > > DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. > > > > v2: > > - Rework/simplify if check (Daniel V) > > - Add examples to commit messages, elaborate. (Daniel V) > > > > v3: > > - Use single unlikely (Daniel V) > > > > v4: > > - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU > > issue is fixed with earlier patch. > > > > [1] > > https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136 > > [2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html > > [3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1 > > Testcase: igt/core_unauth_vs_render > > Cc: intel-...@lists.freedesktop.org > > Signed-off-by: Emil Velikov > > Reviewed-by: Daniel Vetter > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20190114085408.15933-2-emil.l.veli...@gmail.com > > --- > > drivers/gpu/drm/drm_ioctl.c | 20 > > 1 file changed, 16 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > > index 9841c0076f02..b64b022a2b29 100644 > > --- a/drivers/gpu/drm/drm_ioctl.c > > +++ b/drivers/gpu/drm/drm_ioctl.c > > @@ -511,6 +511,13 @@ int drm_version(struct drm_device *dev, void *data, > > return err; > > } > > +static inline bool > > +drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags) > > +{ > > + return drm_core_check_feature(dev, DRIVER_RENDER) && > > + (flags & DRM_RENDER_ALLOW); > > +} > > + > > /** > >* drm_ioctl_permit - Check ioctl permissions against caller > >* > > @@ -525,14 +532,19 @@ int drm_version(struct drm_device *dev, void *data, > >*/ > > int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) > > { > > + const struct drm_device *dev = file_priv->minor->dev; > > + > > /* ROOT_ONLY is only for CAP_SYS_ADMIN */ > > if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN))) > > return -EACCES; > > - /* AUTH is only for authenticated or render client */ > > - if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) && > > -!file_priv->authenticated)) > > - return -EACCES; > > + /* AUTH is only for master ... */ > > + if (unlikely((flags & DRM_AUTH) && drm_is_primary_client(file_priv))) { > > + /* authenticated ones, or render capable on DRM_RENDER_ALLOW. */ > > + if (!file_priv->authenticated && > > + !drm_render_driver_and_ioctl(dev, flags)) > > + return -EACCES; > > + } > > This breaks vmwgfx primary client authentication in the surface_reference > ioctl, which takes different paths in case of render clients and primary > clients, but adding an auth check in the primary path in the vmwgfx code > should fix this. Hm yeah we need to adjust that ... otoh kinda not sure why this is gated on authentication status, and not on "am I master or not" status. At least from a very cursory read ... -Daniel > > /Thomas > > > > /* MASTER is only for master or control clients */ > > if (unlikely((flags & DRM_MASTER) && > > > ___ > 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 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On Mon, May 27, 2019 at 09:17:29AM +0100, Emil Velikov wrote: > From: Emil Velikov > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > render node. A seemingly deliberate design decision. > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > yet not all userspace checks if it's authenticated, but instead uses > uncommon assumptions. > > After days of digging through git log and testing, only a single (ab)use > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > assuming that failure implies lack of authentication. Maybe correction here: Id does not care about authentication at all. It wants to figure out whether it has access to modeset resources, which is something entirely different, but happened to match in amdgpu's case. > Affected versions are: > - the whole 18.2.x series, which is EOL > - the whole 18.3.x series, which is EOL > - the 19.0.x series, prior to 19.0.4 Hm I think I'm blind, I'm not seeing the fix merged to mesa master. Still there on gitlab: https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/amd/vulkan/radv_device.c#L291 What am I missing? > Add a special quirk for that case, thus we can drop DRM_AUTH bits as > mentioned earlier. > > Since all the affected userspace is EOL, we also add a kconfig option > to disable this quirk. > > The whole approach is inspired by DRIVER_KMS_LEGACY_CONTEXT > > Cc: Alex Deucher > Cc: Christian König > Cc: amd-...@lists.freedesktop.org > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Emil Velikov Aside from the nits I think reasonable approach. -Daniel > --- > drivers/gpu/drm/amd/amdgpu/Kconfig | 16 > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 12 +++- > drivers/gpu/drm/drm_ioctl.c | 5 + > include/drm/drm_ioctl.h | 17 + > 4 files changed, 49 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig > b/drivers/gpu/drm/amd/amdgpu/Kconfig > index 9221e5489069..da415f445187 100644 > --- a/drivers/gpu/drm/amd/amdgpu/Kconfig > +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig > @@ -40,6 +40,22 @@ config DRM_AMDGPU_GART_DEBUGFS > Selecting this option creates a debugfs file to inspect the mapped > pages. Uses more memory for housekeeping, enable only for debugging. > > +config DRM_AMDGPU_FORCE_AUTH > + bool "Force authentication check on AMDGPU_INFO ioctl" > + default y > + help > + There were some version of the Mesa RADV drivers, which relied on > + the ioctl failing, if the client is not authenticated. > + > + Namely, the following versions are affected: > + - the whole 18.2.x series, which is EOL > + - the whole 18.3.x series, which is EOL > + - the 19.0.x series, prior to 19.0.4 > + > + Modern distributions, should disable this. That will allow various > + other clients to work, that would otherwise require root privileges. > + > + > source "drivers/gpu/drm/amd/acp/Kconfig" > source "drivers/gpu/drm/amd/display/Kconfig" > source "drivers/gpu/drm/amd/amdkfd/Kconfig" > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > index b17d0545728e..b8076929440b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > @@ -1214,7 +1214,17 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_WAIT_IDLE, amdgpu_gem_wait_idle_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_CS, amdgpu_cs_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > - DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > + /* The DRM_FORCE_AUTH is effectively a workaround for the RADV Mesa > driver. > + * This is required for Mesa: > + * - the whole 18.2.x series, which is EOL > + * - the whole 18.3.x series, which is EOL > + * - the 19.0.x series, prior to 19.0.4 > + */ > + DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, > +#if defined(DRM_AMDGPU_FORCE_AUTH) > + DRM_FORCE_AUTH| > +#endif > + DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_CS, amdgpu_cs_wait_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_FENCES, amdgpu_cs_wait_fences_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_METADATA, amdgpu_gem_metadata_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index 2263e3ddd822..9841c0076f02 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -544,6 +544,11 @@ int drm_ioctl_permit(u32 flags, struct drm_file > *file_priv) >drm_is_render_client(file_priv))) > return -EACCES; > > +
[Bug 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #8 from Nicholas Kazlauskas --- Do you happen to know if this was a regression? -- 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 13/13] drm: allow render capable master with DRM_AUTH ioctls
On 2019/05/27, Thomas Hellstrom wrote: > On 5/27/19 10:17 AM, Emil Velikov wrote: > > From: Emil Velikov > > > > There are cases (in mesa and applications) where one would open the > > primary node without properly authenticating the client. > > > > Sometimes we don't check if the authentication succeeds, but there's > > also cases we simply forget to do it. > > > > The former was a case for Mesa where it did not not check the return > > value of drmGetMagic() [1]. That was fixed recently although, there's > > the question of older drivers or other apps that exbibit this behaviour. > > > > While omitting the call results in issues as seen in [2] and [3]. > > > > In the libva case, libva itself doesn't authenticate the DRM client and > > the vaGetDisplayDRM documentation doesn't mention if the app should > > either. > > > > As of today, the official vainfo utility doesn't authenticate. > > > > To workaround issues like these, some users resort to running their apps > > under sudo. Which admittedly isn't always a good idea. > > > > Since any DRIVER_RENDER driver has sufficient isolation between clients, > > we can use that, for unauthenticated [primary node] ioctls that require > > DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. > > > > v2: > > - Rework/simplify if check (Daniel V) > > - Add examples to commit messages, elaborate. (Daniel V) > > > > v3: > > - Use single unlikely (Daniel V) > > > > v4: > > - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU > > issue is fixed with earlier patch. > > > > [1] > > https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136 > > [2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html > > [3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1 > > Testcase: igt/core_unauth_vs_render > > Cc: intel-...@lists.freedesktop.org > > Signed-off-by: Emil Velikov > > Reviewed-by: Daniel Vetter > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20190114085408.15933-2-emil.l.veli...@gmail.com > > --- > > drivers/gpu/drm/drm_ioctl.c | 20 > > 1 file changed, 16 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > > index 9841c0076f02..b64b022a2b29 100644 > > --- a/drivers/gpu/drm/drm_ioctl.c > > +++ b/drivers/gpu/drm/drm_ioctl.c > > @@ -511,6 +511,13 @@ int drm_version(struct drm_device *dev, void *data, > > return err; > > } > > +static inline bool > > +drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags) > > +{ > > + return drm_core_check_feature(dev, DRIVER_RENDER) && > > + (flags & DRM_RENDER_ALLOW); > > +} > > + > > /** > >* drm_ioctl_permit - Check ioctl permissions against caller > >* > > @@ -525,14 +532,19 @@ int drm_version(struct drm_device *dev, void *data, > >*/ > > int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) > > { > > + const struct drm_device *dev = file_priv->minor->dev; > > + > > /* ROOT_ONLY is only for CAP_SYS_ADMIN */ > > if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN))) > > return -EACCES; > > - /* AUTH is only for authenticated or render client */ > > - if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) && > > -!file_priv->authenticated)) > > - return -EACCES; > > + /* AUTH is only for master ... */ > > + if (unlikely((flags & DRM_AUTH) && drm_is_primary_client(file_priv))) { > > + /* authenticated ones, or render capable on DRM_RENDER_ALLOW. */ > > + if (!file_priv->authenticated && > > + !drm_render_driver_and_ioctl(dev, flags)) > > + return -EACCES; > > + } > > This breaks vmwgfx primary client authentication in the surface_reference > ioctl, which takes different paths in case of render clients and primary > clients, but adding an auth check in the primary path in the vmwgfx code > should fix this. > Ack. Thanks for having a look. Will include a permission check in v2 of the series. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/2] drm/stm: dsi: add power on/off phy ops
Hi Yannick, and thank you for your patch. Tested successfully on stm32f too. Acked-by: Philippe Cornu Tested-by: Philippe Cornu Philippe :-) On 5/27/19 12:21 PM, Yannick Fertré wrote: > These new physical operations are helpful to power_on/off the dsi > wrapper. If the dsi wrapper is powered in video mode, the display > controller (ltdc) register access will hang when DSI fifos are full. > > Signed-off-by: Yannick Fertré > --- > drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 21 - > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > index 01db020..0ab32fe 100644 > --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > @@ -210,10 +210,27 @@ static int dw_mipi_dsi_phy_init(void *priv_data) > if (ret) > DRM_DEBUG_DRIVER("!TIMEOUT! waiting PLL, let's continue\n"); > > + return 0; > +} > + > +static void dw_mipi_dsi_phy_power_on(void *priv_data) > +{ > + struct dw_mipi_dsi_stm *dsi = priv_data; > + > + DRM_DEBUG_DRIVER("\n"); > + > /* Enable the DSI wrapper */ > dsi_set(dsi, DSI_WCR, WCR_DSIEN); > +} > > - return 0; > +static void dw_mipi_dsi_phy_power_off(void *priv_data) > +{ > + struct dw_mipi_dsi_stm *dsi = priv_data; > + > + DRM_DEBUG_DRIVER("\n"); > + > + /* Disable the DSI wrapper */ > + dsi_clear(dsi, DSI_WCR, WCR_DSIEN); > } > > static int > @@ -287,6 +304,8 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct > drm_display_mode *mode, > > static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_stm_phy_ops = { > .init = dw_mipi_dsi_phy_init, > + .power_on = dw_mipi_dsi_phy_power_on, > + .power_off = dw_mipi_dsi_phy_power_off, > .get_lane_mbps = dw_mipi_dsi_get_lane_mbps, > }; > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/05/27, Koenig, Christian wrote: > Am 27.05.19 um 14:05 schrieb Emil Velikov: > > On 2019/05/27, Koenig, Christian wrote: > >> Am 27.05.19 um 10:17 schrieb Emil Velikov: > >>> From: Emil Velikov > >>> > >>> Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > >>> render node. A seemingly deliberate design decision. > >>> > >>> Hence we can drop the DRM_AUTH all together (details in follow-up patch) > >>> yet not all userspace checks if it's authenticated, but instead uses > >>> uncommon assumptions. > >>> > >>> After days of digging through git log and testing, only a eingle (ab)use > >>> was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > >>> assuming that failure implies lack of authentication. > >>> > >>> Affected versions are: > >>>- the whole 18.2.x series, which is EOL > >>>- the whole 18.3.x series, which is EOL > >>>- the 19.0.x series, prior to 19.0.4 > >> Well you could have saved your time, cause this is still a NAK. > >> > > Did I mention that I've spent quite a bit of time in AMDVLK? Even fixed > > a bug while I was there :-) > > Yeah, thanks for doing this. > > But we have done so much stuff with customers which can't be audited > this way, that I still have a really bad feeling about this :/ > Fair point, I've already thought about those. Example A: customer X, using closed source/private software Y. Ioctls A, B and C require the old behaviour - simply add FORCE_AUTH to the A B C and carry on happily. Example B: the team, as a whole thinks that this will be problematic for customer X - add FORCE_AUTH to all ioctls and carry on. I do see and understand why anyone can be hesitant about the series. IMHO the above examples, illustrate quite reasonable compromise between open-source and closed-source/private solutions. Don't you agree? > >> For the record: I strongly think that we don't want to expose any render > >> functionality on the primary node. > >> > >> To even go a step further I would say that at least on AMD hardware we > >> should completely disable DRI2 for one of the next generations. > >> > > It's doable and overall pretty neat idea. > > > > There is a consern that this approach may cause far more regressions > > that this series. We are talking about years worth of Mesa drivers (et > > al) that depend on render functionality exposed via the primary node. > > Yeah, that's I'm perfectly aware of. It's the reason why I said we > should only do it for new hardware generations. > > But at some point I think we should do this and get rid of > authentication/flink/DRI2/ > Fwiw I share a similar sentiment. If you've looked through the series, this is effectively step 1, towards nuking DRM_AUTH :-) > > I'm OK with writing the patches, but it'll be up-to the AMDGPU team to > > follow-up with any regressions. Are you ok with that? > > As long as we have a check like adev->family_id > > WHATEVER_IS_THE_CURRENT_LATEST_UPSTREAM_HW, I think we are fine with that. > > If I understood Michel correctly xf86-video-amdgpu should work, but > modeset might break and needs a patch. > Unless I have concrete WHATEVER_IS_THE... I cannot do much here :-( Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 1/2] drm/bridge/synopsys: dsi: add power on/off optional phy ops
Hi Yannick, and thank you for your patch. Tested successfully on stm32f too. Reviewed-by: Philippe Cornu Tested-by: Philippe Cornu Philippe :-) On 5/27/19 12:21 PM, Yannick Fertré wrote: > Add power on & off optional physical operation functions, helpful to > program specific registers of the DSI physical part. > > Signed-off-by: Yannick Fertré > --- > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8 > include/drm/bridge/dw_mipi_dsi.h | 2 ++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > index e915ae8..5bb676f 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > @@ -775,6 +775,10 @@ static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi > *dsi) > static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge) > { > struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge); > + const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops; > + > + if (phy_ops->power_off) > + phy_ops->power_off(dsi->plat_data->priv_data); > > /* >* Switch to command mode before panel-bridge post_disable & > @@ -874,11 +878,15 @@ static void dw_mipi_dsi_bridge_mode_set(struct > drm_bridge *bridge, > static void dw_mipi_dsi_bridge_enable(struct drm_bridge *bridge) > { > struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge); > + const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops; > > /* Switch to video mode for panel-bridge enable & panel enable */ > dw_mipi_dsi_set_mode(dsi, MIPI_DSI_MODE_VIDEO); > if (dsi->slave) > dw_mipi_dsi_set_mode(dsi->slave, MIPI_DSI_MODE_VIDEO); > + > + if (phy_ops->power_on) > + phy_ops->power_on(dsi->plat_data->priv_data); > } > > static enum drm_mode_status > diff --git a/include/drm/bridge/dw_mipi_dsi.h > b/include/drm/bridge/dw_mipi_dsi.h > index 7d3dd69..df6eda6 100644 > --- a/include/drm/bridge/dw_mipi_dsi.h > +++ b/include/drm/bridge/dw_mipi_dsi.h > @@ -14,6 +14,8 @@ struct dw_mipi_dsi; > > struct dw_mipi_dsi_phy_ops { > int (*init)(void *priv_data); > + void (*power_on)(void *priv_data); > + void (*power_off)(void *priv_data); > int (*get_lane_mbps)(void *priv_data, >const struct drm_display_mode *mode, >unsigned long mode_flags, u32 lanes, u32 format, > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/13] drm: allow render capable master with DRM_AUTH ioctls
On 5/27/19 10:17 AM, Emil Velikov wrote: From: Emil Velikov There are cases (in mesa and applications) where one would open the primary node without properly authenticating the client. Sometimes we don't check if the authentication succeeds, but there's also cases we simply forget to do it. The former was a case for Mesa where it did not not check the return value of drmGetMagic() [1]. That was fixed recently although, there's the question of older drivers or other apps that exbibit this behaviour. While omitting the call results in issues as seen in [2] and [3]. In the libva case, libva itself doesn't authenticate the DRM client and the vaGetDisplayDRM documentation doesn't mention if the app should either. As of today, the official vainfo utility doesn't authenticate. To workaround issues like these, some users resort to running their apps under sudo. Which admittedly isn't always a good idea. Since any DRIVER_RENDER driver has sufficient isolation between clients, we can use that, for unauthenticated [primary node] ioctls that require DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. v2: - Rework/simplify if check (Daniel V) - Add examples to commit messages, elaborate. (Daniel V) v3: - Use single unlikely (Daniel V) v4: - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU issue is fixed with earlier patch. [1] https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136 [2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html [3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1 Testcase: igt/core_unauth_vs_render Cc: intel-...@lists.freedesktop.org Signed-off-by: Emil Velikov Reviewed-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190114085408.15933-2-emil.l.veli...@gmail.com --- drivers/gpu/drm/drm_ioctl.c | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 9841c0076f02..b64b022a2b29 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -511,6 +511,13 @@ int drm_version(struct drm_device *dev, void *data, return err; } +static inline bool +drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags) +{ + return drm_core_check_feature(dev, DRIVER_RENDER) && + (flags & DRM_RENDER_ALLOW); +} + /** * drm_ioctl_permit - Check ioctl permissions against caller * @@ -525,14 +532,19 @@ int drm_version(struct drm_device *dev, void *data, */ int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) { + const struct drm_device *dev = file_priv->minor->dev; + /* ROOT_ONLY is only for CAP_SYS_ADMIN */ if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN))) return -EACCES; - /* AUTH is only for authenticated or render client */ - if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) && -!file_priv->authenticated)) - return -EACCES; + /* AUTH is only for master ... */ + if (unlikely((flags & DRM_AUTH) && drm_is_primary_client(file_priv))) { + /* authenticated ones, or render capable on DRM_RENDER_ALLOW. */ + if (!file_priv->authenticated && + !drm_render_driver_and_ioctl(dev, flags)) + return -EACCES; + } This breaks vmwgfx primary client authentication in the surface_reference ioctl, which takes different paths in case of render clients and primary clients, but adding an auth check in the primary path in the vmwgfx code should fix this. /Thomas /* MASTER is only for master or control clients */ if (unlikely((flags & DRM_MASTER) && ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm/vmwgfx: remove custom ioctl io encoding check
Hi Thomas, On 2019/05/27, Thomas Hellstrom wrote: > > I think we might be talking past each other, let's take a step back: > > > > - as of previous patch, all of vmwgfx ioctls size is consistently > > handled by the core > > I don't think I follow you here, AFAICT patch 3/5 only affects and > relaxes the execbuf checking (and in fact a little more than I would > like)? > Precisely, it makes execbuf ioctl behave like all other ioctls - both vmwgfx and rest of drm. > > - handling of featue flags, as always, is responsibility of the > > driver > > ifself > > - with this patch, ioctl direction is also handled by core. > > > > Here core ensures we only copy in/out as much data as the kernel > > implementation can handle. > > > > > > Let's consider the following real world example - msm and virtio_gpu. > > > > An in field of an _IOW ioctl becomes in/out aka _IORW ioctl. > > - we add a flag to annotate/request the out, as always invalid flags > > are return -EINVAL > > - we change the ioctl encoding > > > > As currently handled by core DRM, old kernel/new userspace and > > vice-versa works just fine. Sadly, vmwgfx will error out, while it > > could > > be avoided. > > IMO basically we have a tradeoff between strict checking in this case, > and new user-space vs old kernel "hazzle-free" transition in the > relaxed case. > Precisely. If I read the code correctly, ATM new userspace will fail against old kernels. Unless userspace writes two versions of the ioctl - with with each encoding. > > > > As said above, I'll gladly adjust core and/or others, if this relaxed > > approach causes an issue somewhere. A specific use-case, real or > > hypothetical will be appreciated. > > To me there are two important reasons to keep the strict approach. > > 1) Avoid user-space mistakes early in the development cycle. We can't > distinguish between buggy user-space and "new" user-space. This is > important because of [a]) below. > Can you provide a concrete example, please? > 2) Catch a lot of fuzzer combinations and error out early instead of > forwarding them to the ioctl function where they may cause harm. > Struggling to see why this is a problem? At some point the fuzzer will get past this first line of defence, so we want to make the rest of the ioctl is robust. > I think the new user-space vs old kernel can be handled nicely in user- > space with feature flags or API versions. That's the way we've handled > them up to now? > How is a feature flag doing to help if the encoding changes from _IOW to _IORW? Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/stm: ltdc: No message if probe
Hi Yannick, Thank you for your patch Acked-by: Philippe Cornu Philippe :-) On 5/27/19 12:14 PM, Yannick Fertré wrote: > Print display controller hardware version in debug mode only. > > Signed-off-by: Yannick Fertré > --- > drivers/gpu/drm/stm/ltdc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c > index d24ffc2..16b1103 100644 > --- a/drivers/gpu/drm/stm/ltdc.c > +++ b/drivers/gpu/drm/stm/ltdc.c > @@ -1211,7 +1211,7 @@ int ltdc_load(struct drm_device *ddev) > goto err; > } > > - DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version); > + DRM_DEBUG_DRIVER("ltdc hw version 0x%08x\n", ldev->caps.hw_version); > > /* Add endpoints panels or bridges if any */ > for (i = 0; i < MAX_ENDPOINTS; i++) { >
Re: [PATCH] drm/stm: ltdc: restore calls to clk_{enable/disable}
Hi Benjamin, Many thanks for this fix (and more generally for pushing STM patches on misc :-) Acked-by: Philippe Cornu Philippe :-) On 5/27/19 1:58 PM, Benjamin Gaignard wrote: > From: Benjamin Gaignard > > Restore calls to clk_{enable/disable} deleted after applying the wrong > version of the patch > > Fixes: fd6905fca4f0 ("drm/stm: ltdc: remove clk_round_rate comment") > > Signed-off-by: Benjamin Gaignard > --- > drivers/gpu/drm/stm/ltdc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c > index ae2aaf2a62ee..ac29890edeb6 100644 > --- a/drivers/gpu/drm/stm/ltdc.c > +++ b/drivers/gpu/drm/stm/ltdc.c > @@ -507,10 +507,12 @@ static bool ltdc_crtc_mode_fixup(struct drm_crtc *crtc, > struct ltdc_device *ldev = crtc_to_ltdc(crtc); > int rate = mode->clock * 1000; > > + clk_disable(ldev->pixel_clk); > if (clk_set_rate(ldev->pixel_clk, rate) < 0) { > DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate); > return false; > } > + clk_enable(ldev->pixel_clk); > > adjusted_mode->clock = clk_get_rate(ldev->pixel_clk) / 1000; > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/13] drm: allow render capable master with DRM_AUTH ioctls
Am 27.05.19 um 14:10 schrieb Emil Velikov: > On 2019/05/27, Christian König wrote: >> Am 27.05.19 um 10:17 schrieb Emil Velikov: >>> From: Emil Velikov >>> >>> There are cases (in mesa and applications) where one would open the >>> primary node without properly authenticating the client. >>> >>> Sometimes we don't check if the authentication succeeds, but there's >>> also cases we simply forget to do it. >>> >>> The former was a case for Mesa where it did not not check the return >>> value of drmGetMagic() [1]. That was fixed recently although, there's >>> the question of older drivers or other apps that exbibit this behaviour. >>> >>> While omitting the call results in issues as seen in [2] and [3]. >>> >>> In the libva case, libva itself doesn't authenticate the DRM client and >>> the vaGetDisplayDRM documentation doesn't mention if the app should >>> either. >>> >>> As of today, the official vainfo utility doesn't authenticate. >>> >>> To workaround issues like these, some users resort to running their apps >>> under sudo. Which admittedly isn't always a good idea. >>> >>> Since any DRIVER_RENDER driver has sufficient isolation between clients, >>> we can use that, for unauthenticated [primary node] ioctls that require >>> DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. >>> >>> v2: >>> - Rework/simplify if check (Daniel V) >>> - Add examples to commit messages, elaborate. (Daniel V) >>> >>> v3: >>> - Use single unlikely (Daniel V) >>> >>> v4: >>> - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU >>> issue is fixed with earlier patch. >> As far as I can see this only affects the following two IOCTLs after >> removing DRM_AUTH from the DRM_RENDER_ALLOW IOCTLs: >>> DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, >>> drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), >>> DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, >>> drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW) >> So I think it would be simpler to just remove DRM_AUTH from those two >> instead of allowing it for everybody. >> > If I understand you correctly this will remove DRM_AUTH also for drivers > which expose only a primary node. I'm not sure if that is a good idea. That's a good point, but I have doubts that those drivers implement the necessary callbacks and/or set the core feature flag for the IOCTLs. So the maximum what could happen is that you change the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS. Regards, Christian. > That said, if others are OK with the idea I will prepare a patch. > > Thanks > Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
Am 27.05.19 um 14:05 schrieb Emil Velikov: > On 2019/05/27, Koenig, Christian wrote: >> Am 27.05.19 um 10:17 schrieb Emil Velikov: >>> From: Emil Velikov >>> >>> Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the >>> render node. A seemingly deliberate design decision. >>> >>> Hence we can drop the DRM_AUTH all together (details in follow-up patch) >>> yet not all userspace checks if it's authenticated, but instead uses >>> uncommon assumptions. >>> >>> After days of digging through git log and testing, only a single (ab)use >>> was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and >>> assuming that failure implies lack of authentication. >>> >>> Affected versions are: >>>- the whole 18.2.x series, which is EOL >>>- the whole 18.3.x series, which is EOL >>>- the 19.0.x series, prior to 19.0.4 >> Well you could have saved your time, cause this is still a NAK. >> > Did I mention that I've spent quite a bit of time in AMDVLK? Even fixed > a bug while I was there :-) Yeah, thanks for doing this. But we have done so much stuff with customers which can't be audited this way, that I still have a really bad feeling about this :/ >> For the record: I strongly think that we don't want to expose any render >> functionality on the primary node. >> >> To even go a step further I would say that at least on AMD hardware we >> should completely disable DRI2 for one of the next generations. >> > It's doable and overall pretty neat idea. > > There is a consern that this approach may cause far more regressions > that this series. We are talking about years worth of Mesa drivers (et > al) that depend on render functionality exposed via the primary node. Yeah, that's I'm perfectly aware of. It's the reason why I said we should only do it for new hardware generations. But at some point I think we should do this and get rid of authentication/flink/DRI2/ > I'm OK with writing the patches, but it'll be up-to the AMDGPU team to > follow-up with any regressions. Are you ok with that? As long as we have a check like adev->family_id > WHATEVER_IS_THE_CURRENT_LATEST_UPSTREAM_HW, I think we are fine with that. If I understood Michel correctly xf86-video-amdgpu should work, but modeset might break and needs a patch. > Fwiw I could also move the FORCE_AUTH hack to core drm, if you prefer. Well, the hack is the least of my concerns. Thanks, Christian. > > Thanks > Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/13] drm: allow render capable master with DRM_AUTH ioctls
On 2019/05/27, Christian König wrote: > Am 27.05.19 um 10:17 schrieb Emil Velikov: > > From: Emil Velikov > > > > There are cases (in mesa and applications) where one would open the > > primary node without properly authenticating the client. > > > > Sometimes we don't check if the authentication succeeds, but there's > > also cases we simply forget to do it. > > > > The former was a case for Mesa where it did not not check the return > > value of drmGetMagic() [1]. That was fixed recently although, there's > > the question of older drivers or other apps that exbibit this behaviour. > > > > While omitting the call results in issues as seen in [2] and [3]. > > > > In the libva case, libva itself doesn't authenticate the DRM client and > > the vaGetDisplayDRM documentation doesn't mention if the app should > > either. > > > > As of today, the official vainfo utility doesn't authenticate. > > > > To workaround issues like these, some users resort to running their apps > > under sudo. Which admittedly isn't always a good idea. > > > > Since any DRIVER_RENDER driver has sufficient isolation between clients, > > we can use that, for unauthenticated [primary node] ioctls that require > > DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. > > > > v2: > > - Rework/simplify if check (Daniel V) > > - Add examples to commit messages, elaborate. (Daniel V) > > > > v3: > > - Use single unlikely (Daniel V) > > > > v4: > > - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU > > issue is fixed with earlier patch. > > As far as I can see this only affects the following two IOCTLs after > removing DRM_AUTH from the DRM_RENDER_ALLOW IOCTLs: > > DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, > > drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), > > DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, > > drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW) > > So I think it would be simpler to just remove DRM_AUTH from those two > instead of allowing it for everybody. > If I understand you correctly this will remove DRM_AUTH also for drivers which expose only a primary node. I'm not sure if that is a good idea. That said, if others are OK with the idea I will prepare a patch. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dt-bindings: display: Convert Allwinner DSI to a schema
The Allwinner SoCs have a MIPI-DSI and MIPI-D-PHY controllers supported in Linux, with a matching Device Tree binding. Now that we have the DT validation in place, let's convert the device tree bindings for that controller over to a YAML schemas. Signed-off-by: Maxime Ripard --- .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 100 ++ .../bindings/display/sunxi/sun6i-dsi.txt | 93 .../phy/allwinner,sun6i-a31-mipi-dphy.yaml| 57 ++ 3 files changed, 157 insertions(+), 93 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml delete mode 100644 Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml new file mode 100644 index ..47950fced28d --- /dev/null +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml @@ -0,0 +1,100 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/allwinner,sun6i-a31-mipi-dsi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Allwinner A31 MIPI-DSI Controller Device Tree Bindings + +maintainers: + - Chen-Yu Tsai + - Maxime Ripard + +properties: + "#address-cells": true + "#size-cells": true + + compatible: +const: allwinner,sun6i-a31-mipi-dsi + + reg: +maxItems: 1 + + interrupts: +maxItems: 1 + + clocks: +items: + - description: Bus Clock + - description: Module Clock + + clock-names: +items: + - const: bus + - const: mod + + resets: +maxItems: 1 + + phys: +maxItems: 1 + + phy-names: +const: dphy + + port: +type: object +description: + A port node with endpoint definitions as defined in + Documentation/devicetree/bindings/media/video-interfaces.txt. That + port should be the input endpoint, usually coming from the + associated TCON. + +patternProperties: + "^panel@[0-9]+$": true + +required: + - "#address-cells" + - "#size-cells" + - compatible + - reg + - interrupts + - clocks + - clock-names + - phys + - phy-names + - resets + - port + +additionalProperties: false + +examples: + - | +dsi0: dsi@1ca { +compatible = "allwinner,sun6i-a31-mipi-dsi"; +reg = <0x01ca 0x1000>; +interrupts = <0 89 4>; +clocks = < 23>, < 96>; +clock-names = "bus", "mod"; +resets = < 4>; +phys = <>; +phy-names = "dphy"; +#address-cells = <1>; +#size-cells = <0>; + +panel@0 { +compatible = "bananapi,lhr050h41", "ilitek,ili9881c"; +reg = <0>; +power-gpios = < 1 7 0>; /* PB07 */ +reset-gpios = <_pio 0 5 1>; /* PL05 */ +backlight = <_bl>; +}; + +port { +dsi0_in_tcon0: endpoint { +remote-endpoint = <_out_dsi0>; +}; +}; +}; + +... diff --git a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt deleted file mode 100644 index 6a6cf5de08b0.. --- a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt +++ /dev/null @@ -1,93 +0,0 @@ -Allwinner A31 DSI Encoder -= - -The DSI pipeline consists of two separate blocks: the DSI controller -itself, and its associated D-PHY. - -DSI Encoder - -The DSI Encoder generates the DSI signal from the TCON's. - -Required properties: - - compatible: value must be one of: -* allwinner,sun6i-a31-mipi-dsi - - reg: base address and size of memory-mapped region - - interrupts: interrupt associated to this IP - - clocks: phandles to the clocks feeding the DSI encoder -* bus: the DSI interface clock -* mod: the DSI module clock - - clock-names: the clock names mentioned above - - phys: phandle to the D-PHY - - phy-names: must be "dphy" - - resets: phandle to the reset controller driving the encoder - - - ports: A ports node with endpoint definitions as defined in -Documentation/devicetree/bindings/media/video-interfaces.txt. The -first port should be the input endpoint, usually coming from the -associated TCON. - -Any MIPI-DSI device attached to this should be described according to -the bindings defined in ../mipi-dsi-bus.txt - -D-PHY -- - -Required properties: - - compatible: value must be one of: -* allwinner,sun6i-a31-mipi-dphy - - reg: base address and size of memory-mapped region - - clocks: phandles to the clocks feeding the DSI encoder -* bus: the DSI interface clock -* mod: the DSI module clock - - clock-names: the clock names mentioned above - - resets: phandle to the
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/05/27, Koenig, Christian wrote: > Am 27.05.19 um 10:17 schrieb Emil Velikov: > > From: Emil Velikov > > > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > > render node. A seemingly deliberate design decision. > > > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > > yet not all userspace checks if it's authenticated, but instead uses > > uncommon assumptions. > > > > After days of digging through git log and testing, only a single (ab)use > > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > > assuming that failure implies lack of authentication. > > > > Affected versions are: > > - the whole 18.2.x series, which is EOL > > - the whole 18.3.x series, which is EOL > > - the 19.0.x series, prior to 19.0.4 > > Well you could have saved your time, cause this is still a NAK. > Did I mention that I've spent quite a bit of time in AMDVLK? Even fixed a bug while I was there :-) > For the record: I strongly think that we don't want to expose any render > functionality on the primary node. > > To even go a step further I would say that at least on AMD hardware we > should completely disable DRI2 for one of the next generations. > It's doable and overall pretty neat idea. There is a consern that this approach may cause far more regressions that this series. We are talking about years worth of Mesa drivers (et al) that depend on render functionality exposed via the primary node. I'm OK with writing the patches, but it'll be up-to the AMDGPU team to follow-up with any regressions. Are you ok with that? Fwiw I could also move the FORCE_AUTH hack to core drm, if you prefer. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/amdgpu: Remove duplicate including duplicate header
remove duplicate entry of soc15.h. Issue identified by includecheck Signed-off-by: Hariprasad Kelam --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index c763733..d723332 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -34,7 +34,6 @@ #include "vega10_enum.h" #include "hdp/hdp_4_0_offset.h" -#include "soc15.h" #include "soc15_common.h" #include "clearstate_gfx9.h" #include "v9_structs.h" -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 0/6] Allwinner H6 Mali GPU support
Hi Rob, On Wed, 22 May 2019 at 21:27, Rob Herring wrote: > > On Tue, May 21, 2019 at 11:11 AM Clément Péron wrote: > > > > Hi, > > > > The Allwinner H6 has a Mali-T720 MP2 which should be supported by > > the new panfrost driver. This series fix two issues and introduce the > > dt-bindings but a simple benchmark show that it's still NOT WORKING. > > > > I'm pushing it in case someone want to continue the work. > > > > This has been tested with Mesa3D 19.1.0-RC2 and a GPU bitness patch[1]. > > > > One patch is from Icenowy Zheng where I changed the order as required > > by Rob Herring[2]. > > > > Thanks, > > Clement > > > > [1] https://gitlab.freedesktop.org/kszaq/mesa/tree/panfrost_64_32 > > [2] https://patchwork.kernel.org/patch/10699829/ > > > > > > [ 345.204813] panfrost 180.gpu: mmu irq status=1 > > [ 345.209617] panfrost 180.gpu: Unhandled Page fault in AS0 at VA > > 0x02400400 > > [ 345.209617] Reason: TODO > > [ 345.209617] raw fault status: 0x82C1 > > [ 345.209617] decoded fault status: SLAVE FAULT > > [ 345.209617] exception type 0xC1: TRANSLATION_FAULT_LEVEL1 > > [ 345.209617] access type 0x2: READ > > [ 345.209617] source id 0x8000 > > [ 345.729957] panfrost 180.gpu: gpu sched timeout, js=0, > > status=0x8, head=0x2400400, tail=0x2400400, sched_job=9e204de9 > > [ 346.055876] panfrost 180.gpu: mmu irq status=1 > > [ 346.060680] panfrost 180.gpu: Unhandled Page fault in AS0 at VA > > 0x02C00A00 > > [ 346.060680] Reason: TODO > > [ 346.060680] raw fault status: 0x810002C1 > > [ 346.060680] decoded fault status: SLAVE FAULT > > [ 346.060680] exception type 0xC1: TRANSLATION_FAULT_LEVEL1 > > [ 346.060680] access type 0x2: READ > > [ 346.060680] source id 0x8100 > > [ 346.561955] panfrost 180.gpu: gpu sched timeout, js=1, > > status=0x8, head=0x2c00a00, tail=0x2c00a00, sched_job=b55a9a85 > > [ 346.573913] panfrost 180.gpu: mmu irq status=1 > > [ 346.578707] panfrost 180.gpu: Unhandled Page fault in AS0 at VA > > 0x02C00B80 > > > > Change in v5: > > - Remove fix indent > > > > Changes in v4: > > - Add bus_clock probe > > - Fix sanity check in io-pgtable > > - Add vramp-delay > > - Merge all boards into one patch > > - Remove upstreamed Neil A. patch > > > > Change in v3 (Thanks to Maxime Ripard): > > - Reauthor Icenowy for her path > > > > Changes in v2 (Thanks to Maxime Ripard): > > - Drop GPU OPP Table > > - Add clocks and clock-names in required > > > > Clément Péron (5): > > drm: panfrost: add optional bus_clock > > iommu: io-pgtable: fix sanity check for non 48-bit mali iommu > > dt-bindings: gpu: mali-midgard: Add H6 mali gpu compatible > > arm64: dts: allwinner: Add ARM Mali GPU node for H6 > > arm64: dts: allwinner: Add mali GPU supply for H6 boards > > > > Icenowy Zheng (1): > > dt-bindings: gpu: add bus clock for Mali Midgard GPUs > > I've applied patches 1 and 3 to drm-misc. I was going to do patch 4 > too, but it doesn't apply. Thanks, I have tried to applied on drm-misc/for-linux-next but it doesn't apply too. It looks like commit d5ff1adb3809e2f74a3b57cea2e57c8e37d693c4 is missing on drm-misc ? https://github.com/torvalds/linux/commit/d5ff1adb3809e2f74a3b57cea2e57c8e37d693c4#diff-c3172f5d421d492ff91d7bb44dd44917 Clément > > Patch 2 can go in via the iommu tree and the rest via the allwinner tree. > > Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [ADV7393] DRM Encoder Slave or DRM Bridge
Hi Nasser, No, problem was not solved and I left it as priorities of my work changed. Best Regards, Vikash On Mon, May 27, 2019 at 3:08 AM nasser afshin wrote: > Hi Vikash, > > As it's been quite a while, I want to know if the problem is solved > successfully > If so, could you please shed some light on the problem solving path? > > Working on a custom hardware based on TI AM5728, and having the same > problem at hand, I just was curious if some one has been able to write a > omapdrm based driver for ADV7393. > > Any help would greatly be appreciated. > > Kind Regards, > Nasser Afshin > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/msm/dpu: Use provided drm_minor to initialize debugfs
Quoting Sean Paul (2019-05-24 10:32:18) > From: Sean Paul > > Instead of reaching into dev->primary for debugfs_root, use the minor > passed into debugfs_init. > > This avoids creating the debug directory under /sys/kernel/debug/ and > instead creates the directory under the correct node in > /sys/kernel/debug/dri// So does this make it become /sys/kernel/debug/dri//debug? I wonder why it can't all be created under /sys/kernel/debug/dri/ and then avoid the extra "debug" directory that isn't adding any value? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/12] drm: remove prime sg_table caching
On Tue, 16 Apr 2019 20:38:35 +0200 Christian König wrote: > @ -331,14 +282,19 @@ EXPORT_SYMBOL(drm_gem_map_dma_buf); > * @sgt: scatterlist info of the buffer to unmap > * @dir: direction of DMA transfer > * > - * Not implemented. The unmap is done at drm_gem_map_detach(). This can be > - * used as the _buf_ops.unmap_dma_buf callback. > + * This can be used as the _buf_ops.unmap_dma_buf callback. > */ > void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach, > struct sg_table *sgt, > enum dma_data_direction dir) > { > - /* nothing to be done here */ > + if (!sgt) if (WARN_ON(!sgt)) ? > + return; > + > + dma_unmap_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir, > +DMA_ATTR_SKIP_CPU_SYNC); > + sg_free_table(sgt); > + kfree(sgt); > } > EXPORT_SYMBOL(drm_gem_unmap_dma_buf); > > -- > 2.17.1 > BR Hillf ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu/powerplay: remove duplicate entry of nbio_6_1_offset.h
asic_reg/nbio/nbio_6_1_offset.h is included twice. Issue identified by includecheck Signed-off-by: Hariprasad Kelam --- drivers/gpu/drm/amd/powerplay/hwmgr/vega12_inc.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_inc.h b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_inc.h index e6d9e84..0d08c57 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_inc.h +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_inc.h @@ -35,7 +35,6 @@ #include "asic_reg/gc/gc_9_2_1_sh_mask.h" #include "asic_reg/nbio/nbio_6_1_offset.h" -#include "asic_reg/nbio/nbio_6_1_offset.h" #include "asic_reg/nbio/nbio_6_1_sh_mask.h" #endif -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel