Re: [PATCH v2 24/25] drm/mediatek: respect page offset for PRIME mmap calls

2019-05-27 Thread CK Hu
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

2019-05-27 Thread CK Hu
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

2019-05-27 Thread Lowry Li (Arm Technology China)
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

2019-05-27 Thread Lowry Li (Arm Technology China)
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

2019-05-27 Thread Lowry Li (Arm Technology China)
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Shawn Guo
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Thomas Meyer
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]

2019-05-27 Thread Sam Ravnborg
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

2019-05-27 Thread Sam Ravnborg
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.

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Sven Van Asbroeck
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

2019-05-27 Thread Sven Van Asbroeck
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

2019-05-27 Thread Jason Gunthorpe
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Fabio Estevam
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Fabio Estevam
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Sam Ravnborg
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

2019-05-27 Thread bugzilla-daemon
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)

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Guido Günther
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Ashutosh Dixit
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

2019-05-27 Thread Emil Velikov
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

2019-05-27 Thread megous
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

2019-05-27 Thread megous
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

2019-05-27 Thread megous
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

2019-05-27 Thread megous
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

2019-05-27 Thread megous
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

2019-05-27 Thread megous
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

2019-05-27 Thread megous
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

2019-05-27 Thread Thomas Hellstrom

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

2019-05-27 Thread Emil Velikov
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

2019-05-27 Thread Jernej Škrabec
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

2019-05-27 Thread Jernej Škrabec
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

2019-05-27 Thread Emil Velikov
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Tomi Valkeinen

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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Lucas Stach
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Thomas Hellstrom

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

2019-05-27 Thread Pintu Agarwal
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

2019-05-27 Thread Emil Velikov
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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Thomas Hellstrom

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

2019-05-27 Thread Koenig, Christian
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

2019-05-27 Thread Lucas Stach
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread 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.

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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Ville Syrjälä
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Emil Velikov
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

2019-05-27 Thread Philippe CORNU
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

2019-05-27 Thread Emil Velikov
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

2019-05-27 Thread Philippe CORNU
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

2019-05-27 Thread Thomas Hellstrom

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

2019-05-27 Thread Emil Velikov
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

2019-05-27 Thread Philippe CORNU
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}

2019-05-27 Thread Philippe CORNU
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

2019-05-27 Thread Koenig, Christian
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

2019-05-27 Thread Koenig, Christian
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

2019-05-27 Thread 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 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

2019-05-27 Thread Maxime Ripard
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

2019-05-27 Thread 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 :-)

> 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

2019-05-27 Thread Hariprasad Kelam
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

2019-05-27 Thread Clément Péron
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

2019-05-27 Thread Vikas Patil
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

2019-05-27 Thread Stephen Boyd
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

2019-05-27 Thread Hillf Danton

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

2019-05-27 Thread Hariprasad Kelam
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

  1   2   >