[Bug 108260] [Regression?] [powerplay] Failed to retrieve minimum clocks. 4.19-rc6+

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=108260

Martin Jørgensen  changed:

   What|Removed |Added

 Resolution|FIXED   |---
 Status|RESOLVED|REOPENED

--- Comment #11 from Martin Jørgensen  ---
Running on debian buster with 4.19 kernel I get the same warning, but only this
warning. No errors regarding the GPU. 

Will there be a patch for 4.19, or do we simply have to upgrade or manually
compile a kernel?

amdgpu: [powerplay] Failed to retrieve minimum clocks.


uname -a :


Linux mkjws 4.19.0-1-amd64 #1 SMP Debian 4.19.12-1 (2018-12-22) x86_64
GNU/Linux


lspci -v :

42:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI]
Ellesmere [Radeon RX 470/480] (rev e7) (prog-if 00 [VGA controller])
Subsystem: ASUSTeK Computer Inc. Ellesmere [Radeon RX
470/480/570/570X/580/580X/590]
Flags: bus master, fast devsel, latency 0, IRQ 70, NUMA node 1
Memory at 46 (64-bit, prefetchable) [size=8G]
Memory at 45 (64-bit, prefetchable) [size=2M]
I/O ports at 2000 [size=256]
Memory at 9f60 (32-bit, non-prefetchable) [size=256K]
Expansion ROM at 9f64 [disabled] [size=128K]
Capabilities: [48] Vendor Specific Information: Len=08 
Capabilities: [50] Power Management version 3
Capabilities: [58] Express Legacy Endpoint, MSI 00
Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1 Len=010

Capabilities: [150] Advanced Error Reporting
Capabilities: [200] #15
Capabilities: [270] #19
Capabilities: [2b0] Address Translation Service (ATS)
Capabilities: [2c0] Page Request Interface (PRI)
Capabilities: [2d0] Process Address Space ID (PASID)
Capabilities: [320] Latency Tolerance Reporting
Capabilities: [328] Alternative Routing-ID Interpretation (ARI)
Capabilities: [370] L1 PM Substates
Kernel driver in use: amdgpu
Kernel modules: amdgpu

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH] drm/modes: Prevent division by zero htotal

[Zhang, Tina]

> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Wednesday, January 23, 2019 6:56 PM
> To: Zhang, Tina 
> Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Adam
> Jackson ; Dave Airlie ; Daniel Vetter
> 
> Subject: Re: [PATCH] drm/modes: Prevent division by zero htotal
> 
> On Wed, Jan 23, 2019 at 03:28:59PM +0800, Tina Zhang wrote:
> > This patch prevents division by zero htotal.
> 
> How did you manage to get here with htotal == 0? This needs backtraces (or if
> this is just about static checkers, a mention of that).
> -Daniel

In GVT-g, we are trying to enable a virtual display w/o setting timings for a 
pipe
(a.k.a htotal=0), then we met the following kernel panic:

[   32.832048] divide error:  [#1] SMP PTI
[   32.833614] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.18.0-rc4-sriov+ #33
[   32.834438] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
rel-1.10.1-0-g8891697-dirty-20180511_165818-tinazhang-linux-1 04/01/2014
[   32.835901] RIP: 0010:drm_mode_hsync+0x1e/0x40
[   32.836004] Code: 31 c0 c3 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 8b 87 
d8 00 00 00 85 c0 75 22 8b 4f 68 85 c9 78 1b 69 47 58 e8 03 00 00 99  f9 b9 
d3 4d 62 10 05 f4 01 00 00 f7 e1 89 d0 c1 e8 06 f3 c3 66
[   32.836004] RSP: :c90ebb90 EFLAGS: 00010206
[   32.836004] RAX:  RBX: 88001c67c8a0 RCX: 
[   32.836004] RDX:  RSI: 88001c67c000 RDI: 88001c67c8a0
[   32.836004] RBP: 88001c7d03a0 R08: 88001c67c8a0 R09: 88001c7d0330
[   32.836004] R10: 822c3a98 R11: 0001 R12: 88001c67c000
[   32.836004] R13: 88001c7d0370 R14: 8207eb78 R15: 88001c67c800
[   32.836004] FS:  () GS:88001da0() 
knlGS:
[   32.836004] CS:  0010 DS:  ES:  CR0: 80050033
[   32.836004] CR2:  CR3: 0220a000 CR4: 06f0
[   32.836004] DR0:  DR1:  DR2: 
[   32.836004] DR3:  DR6: fffe0ff0 DR7: 0400
[   32.836004] Call Trace:
[   32.836004]  intel_mode_from_pipe_config+0x72/0x90
[   32.836004]  intel_modeset_setup_hw_state+0x569/0xf90
[   32.836004]  intel_modeset_init+0x905/0x1db0
[   32.836004]  i915_driver_load+0xb8c/0x1120
[   32.836004]  i915_pci_probe+0x4d/0xb0
[   32.836004]  local_pci_probe+0x44/0xa0
[   32.836004]  ? pci_assign_irq+0x27/0x130
[   32.836004]  pci_device_probe+0x102/0x1c0
[   32.836004]  driver_probe_device+0x2b8/0x480
[   32.836004]  __driver_attach+0x109/0x110
[   32.836004]  ? driver_probe_device+0x480/0x480
[   32.836004]  bus_for_each_dev+0x67/0xc0
[   32.836004]  ? klist_add_tail+0x3b/0x70
[   32.836004]  bus_add_driver+0x1e8/0x260
[   32.836004]  driver_register+0x5b/0xe0
[   32.836004]  ? mipi_dsi_bus_init+0x11/0x11
[   32.836004]  do_one_initcall+0x4d/0x1eb
[   32.836004]  kernel_init_freeable+0x197/0x237
[   32.836004]  ? rest_init+0xd0/0xd0
[   32.836004]  kernel_init+0xa/0x110
[   32.836004]  ret_from_fork+0x35/0x40
[   32.836004] Modules linked in:
[   32.859183] ---[ end trace 525608b0ed0e8665 ]---
[   32.859722] RIP: 0010:drm_mode_hsync+0x1e/0x40
[   32.860287] Code: 31 c0 c3 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 8b 87 
d8 00 00 00 85 c0 75 22 8b 4f 68 85 c9 78 1b 69 47 58 e8 03 00 00 99  f9 b9 
d3 4d 62 10 05 f4 01 00 00 f7 e1 89 d0 c1 e8 06 f3 c3 66
[   32.862680] RSP: :c90ebb90 EFLAGS: 00010206
[   32.863309] RAX:  RBX: 88001c67c8a0 RCX: 
[   32.864182] RDX:  RSI: 88001c67c000 RDI: 88001c67c8a0
[   32.865206] RBP: 88001c7d03a0 R08: 88001c67c8a0 R09: 88001c7d0330
[   32.866359] R10: 822c3a98 R11: 0001 R12: 88001c67c000
[   32.867213] R13: 88001c7d0370 R14: 8207eb78 R15: 88001c67c800
[   32.868075] FS:  () GS:88001da0() 
knlGS:
[   32.868983] CS:  0010 DS:  ES:  CR0: 80050033
[   32.869659] CR2:  CR3: 0220a000 CR4: 06f0
[   32.870599] DR0:  DR1:  DR2: 
[   32.871598] DR3:  DR6: fffe0ff0 DR7: 0400
[   32.872549] Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x000b

Since drm_mode_hsync() has the logic to check mode->htotal, I just extend it to 
cover the case htotal==0.

Thanks.

BR,
Tina
> 
> >
> > Signed-off-by: Tina Zhang 
> > Cc: Adam Jackson 
> > Cc: Dave Airlie 
> > Cc: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/drm_modes.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index adce9a2..59b92b1 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -751,7 +751,7 @@ int drm_mode_

[Bug 107152] GPU fault detected: 146 / VM_CONTEXT1_PROTECTION_FAULT / ring gfx timeout

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=107152

--- Comment #15 from Ida Wallace  ---
Thanks for letting us know about the duplicate bug of GPU fault and System
crashes, so solution seekers can refer both references to understand the bug
and try to solve it easily.

Ida,
http://www.assignmenthelpfolks.com/

-- 
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,4/8] drm/mediatek: fix the rate and divder of hdmi phy for MT2701

[CK Hu]

On Fri, 2019-01-18 at 20:59 +0800, Wangyan Wang wrote:
> From: chunhui dai 

Describe something here.

> 
> Fixes: 0fc721b2968e ("drm/mediatek: add hdmi driver for MT2701 and MT7623")
> Signed-off-by: chunhui dai 

Any one who pass a patch should sign off it.

Regards,
CK

> ---
>  drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c 
> b/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c
> index 43bc058d5528..88dd9e812ca0 100644
> --- a/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c
> +++ b/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c
> @@ -114,8 +114,8 @@ static int mtk_hdmi_pll_set_rate(struct clk_hw *hw, 
> unsigned long rate,
>  
>   if (rate <= 6400)
>   pos_div = 3;
> - else if (rate <= 1280)
> - pos_div = 1;
> + else if (rate <= 12800)
> + pos_div = 2;
>   else
>   pos_div = 1;
>  


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 109445] Graveyard Keeper: Lockup in under 5min of play.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=109445

--- Comment #2 from Mike Mestnik  
---
Created attachment 143217
  --> https://bugs.freedesktop.org/attachment.cgi?id=143217&action=edit
Renderdoc capture under DXVK

-- 
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

[v7 3/3] dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845

[Jayant Shekhar]

Add interconnect properties such as interconnect provider specifier
, the edge source and destination ports which are required by the
interconnect API to configure interconnect path for MDSS.

Changes in v2:
- None

Changes in v3:
- Remove common property definitions (Rob Herring)

Changes in v4:
- Use port macros and change port string names (Georgi Djakov)

Changes in v5-v7:
- None

Signed-off-by: Sravanthi Kollukuduru 
Signed-off-by: Jayant Shekhar 
---
 Documentation/devicetree/bindings/display/msm/dpu.txt | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt 
b/Documentation/devicetree/bindings/display/msm/dpu.txt
index ad2e883..a61dd40 100644
--- a/Documentation/devicetree/bindings/display/msm/dpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
@@ -28,6 +28,11 @@ Required properties:
 - #address-cells: number of address cells for the MDSS children. Should be 1.
 - #size-cells: Should be 1.
 - ranges: parent bus address space is the same as the child bus address space.
+- interconnects : interconnect path specifier for MDSS according to
+  Documentation/devicetree/bindings/interconnect/interconnect.txt. Should be
+  2 paths corresponding to 2 AXI ports.
+- interconnect-names : MDSS will have 2 port names to differentiate between the
+  2 interconnect paths defined with interconnect specifier.
 
 Optional properties:
 - assigned-clocks: list of clock specifiers for clocks needing rate assignment
@@ -86,6 +91,11 @@ Example:
interrupt-controller;
#interrupt-cells = <1>;
 
+   interconnects = <&rsc_hlos MASTER_MDP0 &rsc_hlos SLAVE_EBI1>,
+   <&rsc_hlos MASTER_MDP1 &rsc_hlos SLAVE_EBI1>;
+
+   interconnect-names = "mdp0-mem", "mdp1-mem";
+
iommus = <&apps_iommu 0>;
 
#address-cells = <2>;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[v7 2/3] drm/msm/dpu: Integrate interconnect API in MDSS

[Jayant Shekhar]

The interconnect framework is designed to provide a
standard kernel interface to control the settings of
the interconnects on a SoC.

The interconnect API uses a consumer/provider-based model,
where the providers are the interconnect buses and the
consumers could be various drivers.

MDSS is one of the interconnect consumers which uses the
interconnect APIs to get the path between endpoints and
set its bandwidth requirement for the given interconnected
path.

Changes in v2:
- Remove error log and unnecessary check (Jordan Crouse)

Changes in v3:
- Code clean involving variable name change, removal
  of extra paranthesis and variables (Matthias Kaehlcke)

Changes in v4:
- Add comments, spacings, tabs, proper port name
  and icc macro (Georgi Djakov)

Changes in v5:
- Commit text and parenthesis alignment (Georgi Djakov)

Changes in v6:
- Change to new icc_set API's (Doug Anderson)

Changes in v7:
- Fixed a typo

Signed-off-by: Sravanthi Kollukuduru 
Signed-off-by: Jayant Shekhar 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 49 +---
 1 file changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
index 38576f8..38daf8a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
@@ -4,11 +4,15 @@
  */
 
 #include "dpu_kms.h"
+#include 
 
 #define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base)
 
 #define HW_INTR_STATUS 0x0010
 
+/* Max BW defined in KBps */
+#define MAX_BW 680
+
 struct dpu_mdss {
struct msm_mdss base;
void __iomem *mmio;
@@ -16,8 +20,30 @@ struct dpu_mdss {
u32 hwversion;
struct dss_module_power mp;
struct dpu_irq_controller irq_controller;
+   struct icc_path *path[2];
+   u32 num_paths;
 };
 
+static int dpu_mdss_parse_data_bus_icc_path(struct drm_device *dev,
+   struct dpu_mdss *dpu_mdss)
+{
+   struct icc_path *path0 = of_icc_get(dev->dev, "mdp0-mem");
+   struct icc_path *path1 = of_icc_get(dev->dev, "mdp1-mem");
+
+   if (IS_ERR(path0))
+   return PTR_ERR(path0);
+
+   dpu_mdss->path[0] = path0;
+   dpu_mdss->num_paths = 1;
+
+   if (!IS_ERR(path1)) {
+   dpu_mdss->path[1] = path1;
+   dpu_mdss->num_paths++;
+   }
+
+   return 0;
+}
+
 static irqreturn_t dpu_mdss_irq(int irq, void *arg)
 {
struct dpu_mdss *dpu_mdss = arg;
@@ -127,7 +153,11 @@ static int dpu_mdss_enable(struct msm_mdss *mdss)
 {
struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
struct dss_module_power *mp = &dpu_mdss->mp;
-   int ret;
+   int ret, i;
+   u64 avg_bw = dpu_mdss->num_paths ? MAX_BW / dpu_mdss->num_paths : 0;
+
+   for (i = 0; i < dpu_mdss->num_paths; i++)
+   icc_set_bw(dpu_mdss->path[i], avg_bw, kBps_to_icc(MAX_BW));
 
ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, true);
if (ret)
@@ -140,12 +170,15 @@ static int dpu_mdss_disable(struct msm_mdss *mdss)
 {
struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
struct dss_module_power *mp = &dpu_mdss->mp;
-   int ret;
+   int ret, i;
 
ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false);
if (ret)
DPU_ERROR("clock disable failed, ret:%d\n", ret);
 
+   for (i = 0; i < dpu_mdss->num_paths; i++)
+   icc_set_bw(dpu_mdss->path[i], 0, 0);
+
return ret;
 }
 
@@ -155,6 +188,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)
struct msm_drm_private *priv = dev->dev_private;
struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss);
struct dss_module_power *mp = &dpu_mdss->mp;
+   int i;
 
pm_runtime_disable(dev->dev);
_dpu_mdss_irq_domain_fini(dpu_mdss);
@@ -162,6 +196,9 @@ static void dpu_mdss_destroy(struct drm_device *dev)
msm_dss_put_clk(mp->clk_config, mp->num_clk);
devm_kfree(&pdev->dev, mp->clk_config);
 
+   for (i = 0; i < dpu_mdss->num_paths; i++)
+   icc_put(dpu_mdss->path[i]);
+
if (dpu_mdss->mmio)
devm_iounmap(&pdev->dev, dpu_mdss->mmio);
dpu_mdss->mmio = NULL;
@@ -200,6 +237,10 @@ int dpu_mdss_init(struct drm_device *dev)
}
dpu_mdss->mmio_len = resource_size(res);
 
+   ret = dpu_mdss_parse_data_bus_icc_path(dev, dpu_mdss);
+   if (ret)
+   return ret;
+
mp = &dpu_mdss->mp;
ret = msm_dss_parse_clock(pdev, mp);
if (ret) {
@@ -221,14 +262,14 @@ int dpu_mdss_init(struct drm_device *dev)
goto irq_error;
}
 
+   priv->mdss = &dpu_mdss->base;
+
pm_runtime_enable(dev->dev);
 
pm_runtime_get_sync(dev->dev);
dpu_mdss->hwversion = readl_relaxed(dpu_mdss->mmio);
pm_r

[v7 1/3] drm/msm/dpu: clean up references of DPU custom bus scaling

[Jayant Shekhar]

Since the upstream interconnect bus framework has landed
upstream, the existing references of custom bus scaling
needs to be cleaned up.

Changes in v2:
- Fixed build error due to partial clean up

Changes in v3:
- Condense multiple lines into a single line (Sean Paul)

Changes in v4-v7:
- None

Signed-off-by: Sravanthi Kollukuduru 
Signed-off-by: Jayant Shekhar 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c| 174 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h|   4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c |  13 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c |  47 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h |  68 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h|  22 +--
 6 files changed, 89 insertions(+), 239 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 22e84b3..c75536e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -84,7 +84,6 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
struct dpu_core_perf_params *perf)
 {
struct dpu_crtc_state *dpu_cstate;
-   int i;
 
if (!kms || !kms->catalog || !crtc || !state || !perf) {
DPU_ERROR("invalid parameters\n");
@@ -95,35 +94,24 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
memset(perf, 0, sizeof(struct dpu_core_perf_params));
 
if (!dpu_cstate->bw_control) {
-   for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
-   perf->bw_ctl[i] = kms->catalog->perf.max_bw_high *
+   perf->bw_ctl = kms->catalog->perf.max_bw_high *
1000ULL;
-   perf->max_per_pipe_ib[i] = perf->bw_ctl[i];
-   }
+   perf->max_per_pipe_ib = perf->bw_ctl;
perf->core_clk_rate = kms->perf.max_core_clk_rate;
} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
-   for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
-   perf->bw_ctl[i] = 0;
-   perf->max_per_pipe_ib[i] = 0;
-   }
+   perf->bw_ctl = 0;
+   perf->max_per_pipe_ib = 0;
perf->core_clk_rate = 0;
} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED) {
-   for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
-   perf->bw_ctl[i] = kms->perf.fix_core_ab_vote;
-   perf->max_per_pipe_ib[i] = kms->perf.fix_core_ib_vote;
-   }
+   perf->bw_ctl = kms->perf.fix_core_ab_vote;
+   perf->max_per_pipe_ib = kms->perf.fix_core_ib_vote;
perf->core_clk_rate = kms->perf.fix_core_clk_rate;
}
 
DPU_DEBUG(
-   "crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu llcc_ib=%llu 
llcc_ab=%llu mem_ib=%llu mem_ab=%llu\n",
+   "crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu\n",
crtc->base.id, perf->core_clk_rate,
-   perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_MNOC],
-   perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_MNOC],
-   perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_LLCC],
-   perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_LLCC],
-   perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_EBI],
-   perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_EBI]);
+   perf->max_per_pipe_ib, perf->bw_ctl);
 }
 
 int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
@@ -136,7 +124,6 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
struct dpu_crtc_state *dpu_cstate;
struct drm_crtc *tmp_crtc;
struct dpu_kms *kms;
-   int i;
 
if (!crtc || !state) {
DPU_ERROR("invalid crtc\n");
@@ -158,31 +145,25 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
/* obtain new values */
_dpu_core_perf_calc_crtc(kms, crtc, state, &dpu_cstate->new_perf);
 
-   for (i = DPU_POWER_HANDLE_DBUS_ID_MNOC;
-   i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
-   bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl[i];
-   curr_client_type = dpu_crtc_get_client_type(crtc);
+   bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl;
+   curr_client_type = dpu_crtc_get_client_type(crtc);
 
-   drm_for_each_crtc(tmp_crtc, crtc->dev) {
-   if (_dpu_core_perf_crtc_is_power_on(tmp_crtc) &&
-   (dpu_crtc_get_client_type(tmp_crtc) ==
-   curr_client_type) &&
-   (tmp_crtc != crtc)) {
-   struct dpu_crtc_state *tmp_cstate =
-   to_dpu_crtc_state(

[v7 0/3] Use interconnect API in MDSS on SDM845

[Jayant Shekhar]

The interconnect API provides an interface for consumer drivers to express
their bandwidth needs in the SoC. This data is aggregated and the on-chip
interconnect hardware is configured to the appropriate power/performance
profile.

MDSS is one of the interconnect consumers which uses the interconnect APIs
to get the path between endpoints and set its bandwidth requirements
for the given interconnected path.

Subsequently, there is a clean up patch to remove all the references
of the DPU custom bus scaling.

There is corresponding DT patch with the source and destination ports
defined for display driver which will be sent separately.

Changes in v2:
- Remove error log and unnecessary check (Jordan Crouse)
- Fixed build error due to partial clean up

Changes in v3:
- Remove common property definitions (Rob Herring)
- Code clean up involving variable name change, removal
  of extra paranthesis and variables (Matthias Kaehlcke)
- Condense multiple lines into a single line (Sean Paul)

Changes in v4:
   - Add comments, spacings, tabs, proper port name and icc macro
   - Use port macros and change port string names (Georgi Djakov)

Changes in v5:
   - Updated commit text and parenthesis alignment (Georgi Djakov)

Changes in v6:
   - Change icc_set to icc_set_bw (Doug Anderson)

Changes in v7:
   - Fixed a typo

Jayant Shekhar (3):
  drm/msm/dpu: clean up references of DPU custom bus scaling
  drm/msm/dpu: Integrate interconnect API in MDSS
  dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on
SDM845

 .../devicetree/bindings/display/msm/dpu.txt|  10 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c  | 174 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h  |   4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c   |  13 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c   |  49 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c   |  47 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h   |  68 
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h  |  22 +--
 8 files changed, 144 insertions(+), 243 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: drm/komeda: Off by one in komeda_fb_get_pixel_addr()

[james qian wang (Arm Technology China)]

On Wed, Jan 23, 2019 at 12:37:55PM +0300, Dan Carpenter wrote:
> The > should be >= to avoid an off by one bug.
> 
> Fixes: c46c24bb6b11 ("drm/komeda: Add komeda_framebuffer")
> Signed-off-by: Dan Carpenter 
> ---
> 
> I'm 98% sure this is correct, but please review it carefully because I'm
> not 100% positive.

Hi Dan:

Thank you, this is a typo and your fix is correct.

Reviewed-by: James Qian Wang (Arm Technology China) 

Best Regards
James.

> 
>  drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c 
> b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> index 4ddd5314ca23..23ee74d42239 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> @@ -144,7 +144,7 @@ komeda_fb_get_pixel_addr(struct komeda_fb *kfb, int x, 
> int y, int plane)
>   const struct drm_gem_cma_object *obj;
>   u32 plane_x, plane_y, cpp, pitch, offset;
>  
> - if (plane > fb->format->num_planes) {
> + if (plane >= fb->format->num_planes) {
>   DRM_DEBUG_KMS("Out of max plane num.\n");
>   return -EINVAL;
>   }
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 1/2] drm: Add color management LUT validation helper (v4)

[Matt Roper]

On Sat, Jan 12, 2019 at 01:07:14PM +0100, Daniel Vetter wrote:
> On Fri, Jan 11, 2019 at 02:27:00PM -0800, Matt Roper wrote:
> > Dave, Daniel - any concerns if we merge this drm core patch through the
> > Intel tree?  The second patch in the series doesn't apply cleanly in
> > drm-misc-next.
> 
> Hm, need to discuss with Dave how good my ack powers are now. In the past
> we had the rule that I asked him for an explicit ack, but back then I was
> still maintainer of i915, so a bit a too obvious conflict there. I think
> with the separate roles me acking should be good enough.
> 
> Acked-by: Daniel Vetter 
> 
> Dave, sounds reasonable?
> -Daniel

Dave confirmed on IRC that Daniel's ack here is sufficient.  Merged both
patches through the Intel tree (dinq); thanks everyone for the reviews.


Matt

> 
> > 
> > 
> > Matt
> > 
> > 
> > On Mon, Dec 17, 2018 at 02:44:14PM -0800, Matt Roper wrote:
> > > Some hardware may place additional restrictions on the gamma/degamma
> > > curves described by our LUT properties.  E.g., that a gamma curve never
> > > decreases or that the red/green/blue channels of a LUT's entries must be
> > > equal.  Let's add a helper function that drivers can use to test that a
> > > userspace-provided LUT is valid and doesn't violate hardware
> > > requirements.
> > > 
> > > v2:
> > >  - Combine into a single helper that just takes a bitmask of the tests
> > >to apply.  (Brian Starkey)
> > >  - Add additional check (always performed) that LUT property blob size
> > >is always a multiple of the LUT entry size.  (stolen from ARM driver)
> > > 
> > > v3:
> > >  - Drop the LUT size check again since
> > >drm_atomic_replace_property_blob_from_id() already covers this for
> > >us.  (Alexandru Gheorghe)
> > > 
> > > v4:
> > >  - Use an enum to describe possible test values rather than #define's;
> > >this is cleaner to provide kerneldoc for.  (Daniel Vetter)
> > >  - s/DRM_COLOR_LUT_INCREASING/DRM_COLOR_LUT_NON_DECREASING/.  (Ville)
> > > 
> > > Cc: Uma Shankar 
> > > Cc: Swati Sharma 
> > > Cc: Brian Starkey 
> > > Cc: Daniel Vetter 
> > > Cc: Ville Syrjälä 
> > > Signed-off-by: Matt Roper 
> > > Reviewed-by(v1): Brian Starkey 
> > > Reviewed-by: Alexandru Gheorghe 
> > > Reviewed-by: Uma Shankar 
> > > ---
> > >  drivers/gpu/drm/drm_color_mgmt.c | 44 
> > > 
> > >  include/drm/drm_color_mgmt.h | 29 ++
> > >  2 files changed, 73 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c 
> > > b/drivers/gpu/drm/drm_color_mgmt.c
> > > index 07dcf47daafe..968ca7c91ad8 100644
> > > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > > @@ -462,3 +462,47 @@ int drm_plane_create_color_properties(struct 
> > > drm_plane *plane,
> > >   return 0;
> > >  }
> > >  EXPORT_SYMBOL(drm_plane_create_color_properties);
> > > +
> > > +/**
> > > + * drm_color_lut_check - check validity of lookup table
> > > + * @lut: property blob containing LUT to check
> > > + * @tests: bitmask of tests to run
> > > + *
> > > + * Helper to check whether a userspace-provided lookup table is valid and
> > > + * satisfies hardware requirements.  Drivers pass a bitmask indicating 
> > > which of
> > > + * the tests in &drm_color_lut_tests should be performed.
> > > + *
> > > + * Returns 0 on success, -EINVAL on failure.
> > > + */
> > > +int drm_color_lut_check(struct drm_property_blob *lut,
> > > + uint32_t tests)
> > > +{
> > > + struct drm_color_lut *entry;
> > > + int i;
> > > +
> > > + if (!lut || !tests)
> > > + return 0;
> > > +
> > > + entry = lut->data;
> > > + for (i = 0; i < drm_color_lut_size(lut); i++) {
> > > + if (tests & DRM_COLOR_LUT_EQUAL_CHANNELS) {
> > > + if (entry[i].red != entry[i].blue ||
> > > + entry[i].red != entry[i].green) {
> > > + DRM_DEBUG_KMS("All LUT entries must have equal 
> > > r/g/b\n");
> > > + return -EINVAL;
> > > + }
> > > + }
> > > +
> > > + if (i > 0 && tests & DRM_COLOR_LUT_NON_DECREASING) {
> > > + if (entry[i].red < entry[i - 1].red ||
> > > + entry[i].green < entry[i - 1].green ||
> > > + entry[i].blue < entry[i - 1].blue) {
> > > + DRM_DEBUG_KMS("LUT entries must never 
> > > decrease.\n");
> > > + return -EINVAL;
> > > + }
> > > + }
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL(drm_color_lut_check);
> > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > > index 90ef9996d9a4..6affbda6d9cb 100644
> > > --- a/include/drm/drm_color_mgmt.h
> > > +++ b/include/drm/drm_color_mgmt.h
> > > @@ -69,4 +69,33 @@ int drm_plane_create_color_properties(struct drm_plane 
> > > *plane,
> > > u32 supported_ranges,
> >

Re: [PATCH 2/2] drm/msm: Use DRM_DEV_INFO_RATELIMITED for shrinker messages

[Rob Clark]

On Mon, Jan 21, 2019 at 4:36 AM Jani Nikula  wrote:
>
> On Fri, 18 Jan 2019, "Kristian H. Kristensen"  wrote:
> > Otherwise we get hard to track down "Purging: 123123 bytes" messages in
> > the log.
> >
> > Signed-off-by: Kristian H. Kristensen 
> > ---
> >  drivers/gpu/drm/msm/msm_gem_shrinker.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c 
> > b/drivers/gpu/drm/msm/msm_gem_shrinker.c
> > index b72d8e6cd51d7..8161923892f55 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_shrinker.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c
> > @@ -98,7 +98,7 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct 
> > shrink_control *sc)
> >   mutex_unlock(&dev->struct_mutex);
> >
> >   if (freed > 0)
> > - pr_info_ratelimited("Purging %lu bytes\n", freed << 
> > PAGE_SHIFT);
> > + DRM_DEV_INFO_RATELIMITED(dev->dev, "Purging %lu bytes\n", 
> > freed << PAGE_SHIFT);
>
> I'm not opposed to the patches per se, but it does seem a bit odd to be
> printing info level messages in a way that might need ratelimiting. Is
> this a hint you should perhaps make it debug?
>

I'm probably to blame for it being info..  at least for "traditional"
linux userspace, hitting the srinker hard on a device with a moderate
amount of memory was kinda an abnormal situation and something I
wanted to at least be aware of in potential bug reports..

I guess since it seems to be more a "business as usual" situation on
android other such userspaces, maybe we should demote this to debug
(but ime it should still be ratelimited)

BR,
-R
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 0/9] mmu notifier provide context informations

[Dan Williams]

On Wed, Jan 23, 2019 at 3:05 PM Jerome Glisse  wrote:
>
> On Wed, Jan 23, 2019 at 02:54:40PM -0800, Dan Williams wrote:
> > On Wed, Jan 23, 2019 at 2:23 PM  wrote:
> > >
> > > From: Jérôme Glisse 
> > >
> > > Hi Andrew, i see that you still have my event patch in you queue [1].
> > > This patchset replace that single patch and is broken down in further
> > > step so that it is easier to review and ascertain that no mistake were
> > > made during mechanical changes. Here are the step:
> > >
> > > Patch 1 - add the enum values
> > > Patch 2 - coccinelle semantic patch to convert all call site of
> > >   mmu_notifier_range_init to default enum value and also
> > >   to passing down the vma when it is available
> > > Patch 3 - update many call site to more accurate enum values
> > > Patch 4 - add the information to the mmu_notifier_range struct
> > > Patch 5 - helper to test if a range is updated to read only
> > >
> > > All the remaining patches are update to various driver to demonstrate
> > > how this new information get use by device driver. I build tested
> > > with make all and make all minus everything that enable mmu notifier
> > > ie building with MMU_NOTIFIER=no. Also tested with some radeon,amd
> > > gpu and intel gpu.
> > >
> > > If they are no objections i believe best plan would be to merge the
> > > the first 5 patches (all mm changes) through your queue for 5.1 and
> > > then to delay driver update to each individual driver tree for 5.2.
> > > This will allow each individual device driver maintainer time to more
> > > thouroughly test this more then my own testing.
> > >
> > > Note that i also intend to use this feature further in nouveau and
> > > HMM down the road. I also expect that other user like KVM might be
> > > interested into leveraging this new information to optimize some of
> > > there secondary page table invalidation.
> >
> > "Down the road" users should introduce the functionality they want to
> > consume. The common concern with preemptively including
> > forward-looking infrastructure is realizing later that the
> > infrastructure is not needed, or needs changing. If it has no current
> > consumer, leave it out.
>
> This patchset already show that this is useful, what more can i do ?
> I know i will use this information, in nouveau for memory policy we
> allocate our own structure for every vma the GPU ever accessed or that
> userspace hinted we should set a policy for. Right now with existing
> mmu notifier i _must_ free those structure because i do not know if
> the invalidation is an munmap or something else. So i am loosing
> important informations and unecessarly free struct that i will have
> to re-allocate just couple jiffies latter. That's one way i am using
> this.

Understood, but that still seems to say stage the core support when
the nouveau enabling is ready.

> The other way is to optimize GPU page table update just like i
> am doing with all the patches to RDMA/ODP and various GPU drivers.

Yes.

>
>
> > > Here is an explaination on the rational for this patchset:
> > >
> > >
> > > CPU page table update can happens for many reasons, not only as a result
> > > of a syscall (munmap(), mprotect(), mremap(), madvise(), ...) but also
> > > as a result of kernel activities (memory compression, reclaim, migration,
> > > ...).
> > >
> > > This patch introduce a set of enums that can be associated with each of
> > > the events triggering a mmu notifier. Latter patches take advantages of
> > > those enum values.
> > >
> > > - UNMAP: munmap() or mremap()
> > > - CLEAR: page table is cleared (migration, compaction, reclaim, ...)
> > > - PROTECTION_VMA: change in access protections for the range
> > > - PROTECTION_PAGE: change in access protections for page in the range
> > > - SOFT_DIRTY: soft dirtyness tracking
> > >
> > > Being able to identify munmap() and mremap() from other reasons why the
> > > page table is cleared is important to allow user of mmu notifier to
> > > update their own internal tracking structure accordingly (on munmap or
> > > mremap it is not longer needed to track range of virtual address as it
> > > becomes invalid).
> >
> > The only context information consumed in this patch set is
> > MMU_NOTIFY_PROTECTION_VMA.
> >
> > What is the practical benefit of these "optimize out the case when a
> > range is updated to read only" optimizations? Any numbers to show this
> > is worth the code thrash?
>
> It depends on the workload for instance if you map to RDMA a file
> read only like a log file for export, all write back that would
> disrupt the RDMA mapping can be optimized out.
>
> See above for more reasons why it is beneficial (knowing when it is
> an munmap/mremap versus something else).
>
> I would have not thought that passing down information as something
> that controversial. Hopes this help you see the benefit of this.

I'm not asserting that it is controversial. I am asserting that
whenever a changelog

[Bug 201497] [amdgpu]: '*ERROR* No EDID read' is back in 4.19

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=201497

Elliot Thomas (e.singularity...@gmail.com) changed:

   What|Removed |Added

 CC||e.singularity...@gmail.com

--- Comment #7 from Elliot Thomas (e.singularity...@gmail.com) ---
Same here, RX580 and an ASUS PG278Q monitor, also with G-Sync support.
A different DisplayPort monitor (without G-Sync) worked perfectly.

I've noticed someone else with the same monitor having this issue over on the
freedesktop bug tracker: https://bugs.freedesktop.org/show_bug.cgi?id=108806

Interestingly, in more recent kernels, the affected monitor appears to flash
rapidly. I still see the EDID read fail in this case. I'll try and grab a dmesg
of this at some point.

-- 
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

[pull] amdgpu drm-fixes-5.0

[Alex Deucher]

Hi Dave, Daniel,

Just a couple of fixes for 5.0:
- Overclock fix for vega10
- Hybrid gfx laptop fix

The following changes since commit 35dad45d5cad3c9ca8d6a338cbf668cd7ea86469:

  drm/amd/display: Detach backlight from stream (2019-01-16 17:11:47 -0500)

are available in the Git repository at:

  git://people.freedesktop.org/~agd5f/linux drm-fixes-5.0

for you to fetch changes up to 6d87dc97eb3341de3f7b1efa3156cb0e014f4a96:

  drm/amd/powerplay: OD setting fix on Vega10 (2019-01-21 15:01:43 -0500)


Alex Deucher (1):
  drm/amdgpu: Add APTX quirk for Lenovo laptop

Kenneth Feng (1):
  drm/amd/powerplay: OD setting fix on Vega10

 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c   |  1 +
 .../amd/powerplay/hwmgr/vega10_processpptables.c   | 22 +-
 2 files changed, 22 insertions(+), 1 deletion(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 0/9] mmu notifier provide context informations

[Jerome Glisse]

On Wed, Jan 23, 2019 at 02:54:40PM -0800, Dan Williams wrote:
> On Wed, Jan 23, 2019 at 2:23 PM  wrote:
> >
> > From: Jérôme Glisse 
> >
> > Hi Andrew, i see that you still have my event patch in you queue [1].
> > This patchset replace that single patch and is broken down in further
> > step so that it is easier to review and ascertain that no mistake were
> > made during mechanical changes. Here are the step:
> >
> > Patch 1 - add the enum values
> > Patch 2 - coccinelle semantic patch to convert all call site of
> >   mmu_notifier_range_init to default enum value and also
> >   to passing down the vma when it is available
> > Patch 3 - update many call site to more accurate enum values
> > Patch 4 - add the information to the mmu_notifier_range struct
> > Patch 5 - helper to test if a range is updated to read only
> >
> > All the remaining patches are update to various driver to demonstrate
> > how this new information get use by device driver. I build tested
> > with make all and make all minus everything that enable mmu notifier
> > ie building with MMU_NOTIFIER=no. Also tested with some radeon,amd
> > gpu and intel gpu.
> >
> > If they are no objections i believe best plan would be to merge the
> > the first 5 patches (all mm changes) through your queue for 5.1 and
> > then to delay driver update to each individual driver tree for 5.2.
> > This will allow each individual device driver maintainer time to more
> > thouroughly test this more then my own testing.
> >
> > Note that i also intend to use this feature further in nouveau and
> > HMM down the road. I also expect that other user like KVM might be
> > interested into leveraging this new information to optimize some of
> > there secondary page table invalidation.
> 
> "Down the road" users should introduce the functionality they want to
> consume. The common concern with preemptively including
> forward-looking infrastructure is realizing later that the
> infrastructure is not needed, or needs changing. If it has no current
> consumer, leave it out.

This patchset already show that this is useful, what more can i do ?
I know i will use this information, in nouveau for memory policy we
allocate our own structure for every vma the GPU ever accessed or that
userspace hinted we should set a policy for. Right now with existing
mmu notifier i _must_ free those structure because i do not know if
the invalidation is an munmap or something else. So i am loosing
important informations and unecessarly free struct that i will have
to re-allocate just couple jiffies latter. That's one way i am using
this. The other way is to optimize GPU page table update just like i
am doing with all the patches to RDMA/ODP and various GPU drivers.


> > Here is an explaination on the rational for this patchset:
> >
> >
> > CPU page table update can happens for many reasons, not only as a result
> > of a syscall (munmap(), mprotect(), mremap(), madvise(), ...) but also
> > as a result of kernel activities (memory compression, reclaim, migration,
> > ...).
> >
> > This patch introduce a set of enums that can be associated with each of
> > the events triggering a mmu notifier. Latter patches take advantages of
> > those enum values.
> >
> > - UNMAP: munmap() or mremap()
> > - CLEAR: page table is cleared (migration, compaction, reclaim, ...)
> > - PROTECTION_VMA: change in access protections for the range
> > - PROTECTION_PAGE: change in access protections for page in the range
> > - SOFT_DIRTY: soft dirtyness tracking
> >
> > Being able to identify munmap() and mremap() from other reasons why the
> > page table is cleared is important to allow user of mmu notifier to
> > update their own internal tracking structure accordingly (on munmap or
> > mremap it is not longer needed to track range of virtual address as it
> > becomes invalid).
> 
> The only context information consumed in this patch set is
> MMU_NOTIFY_PROTECTION_VMA.
> 
> What is the practical benefit of these "optimize out the case when a
> range is updated to read only" optimizations? Any numbers to show this
> is worth the code thrash?

It depends on the workload for instance if you map to RDMA a file
read only like a log file for export, all write back that would
disrupt the RDMA mapping can be optimized out.

See above for more reasons why it is beneficial (knowing when it is
an munmap/mremap versus something else).

I would have not thought that passing down information as something
that controversial. Hopes this help you see the benefit of this.

Cheers,
Jérôme
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 0/9] mmu notifier provide context informations

[Dan Williams]

On Wed, Jan 23, 2019 at 2:23 PM  wrote:
>
> From: Jérôme Glisse 
>
> Hi Andrew, i see that you still have my event patch in you queue [1].
> This patchset replace that single patch and is broken down in further
> step so that it is easier to review and ascertain that no mistake were
> made during mechanical changes. Here are the step:
>
> Patch 1 - add the enum values
> Patch 2 - coccinelle semantic patch to convert all call site of
>   mmu_notifier_range_init to default enum value and also
>   to passing down the vma when it is available
> Patch 3 - update many call site to more accurate enum values
> Patch 4 - add the information to the mmu_notifier_range struct
> Patch 5 - helper to test if a range is updated to read only
>
> All the remaining patches are update to various driver to demonstrate
> how this new information get use by device driver. I build tested
> with make all and make all minus everything that enable mmu notifier
> ie building with MMU_NOTIFIER=no. Also tested with some radeon,amd
> gpu and intel gpu.
>
> If they are no objections i believe best plan would be to merge the
> the first 5 patches (all mm changes) through your queue for 5.1 and
> then to delay driver update to each individual driver tree for 5.2.
> This will allow each individual device driver maintainer time to more
> thouroughly test this more then my own testing.
>
> Note that i also intend to use this feature further in nouveau and
> HMM down the road. I also expect that other user like KVM might be
> interested into leveraging this new information to optimize some of
> there secondary page table invalidation.

"Down the road" users should introduce the functionality they want to
consume. The common concern with preemptively including
forward-looking infrastructure is realizing later that the
infrastructure is not needed, or needs changing. If it has no current
consumer, leave it out.

>
> Here is an explaination on the rational for this patchset:
>
>
> CPU page table update can happens for many reasons, not only as a result
> of a syscall (munmap(), mprotect(), mremap(), madvise(), ...) but also
> as a result of kernel activities (memory compression, reclaim, migration,
> ...).
>
> This patch introduce a set of enums that can be associated with each of
> the events triggering a mmu notifier. Latter patches take advantages of
> those enum values.
>
> - UNMAP: munmap() or mremap()
> - CLEAR: page table is cleared (migration, compaction, reclaim, ...)
> - PROTECTION_VMA: change in access protections for the range
> - PROTECTION_PAGE: change in access protections for page in the range
> - SOFT_DIRTY: soft dirtyness tracking
>
> Being able to identify munmap() and mremap() from other reasons why the
> page table is cleared is important to allow user of mmu notifier to
> update their own internal tracking structure accordingly (on munmap or
> mremap it is not longer needed to track range of virtual address as it
> becomes invalid).

The only context information consumed in this patch set is
MMU_NOTIFY_PROTECTION_VMA.

What is the practical benefit of these "optimize out the case when a
range is updated to read only" optimizations? Any numbers to show this
is worth the code thrash?

>
> [1] 
> https://www.ozlabs.org/~akpm/mmotm/broken-out/mm-mmu_notifier-contextual-information-for-event-triggering-invalidation-v2.patch
>
> Cc: Christian König 
> Cc: Jan Kara 
> Cc: Felix Kuehling 
> Cc: Jason Gunthorpe 
> Cc: Andrew Morton 
> Cc: Matthew Wilcox 
> Cc: Ross Zwisler 
> Cc: Dan Williams 
> Cc: Paolo Bonzini 
> Cc: Radim Krčmář 
> Cc: Michal Hocko 
> Cc: Ralph Campbell 
> Cc: John Hubbard 
> Cc: k...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-r...@vger.kernel.org
> Cc: linux-fsde...@vger.kernel.org
> Cc: Arnd Bergmann 
>
> Jérôme Glisse (9):
>   mm/mmu_notifier: contextual information for event enums
>   mm/mmu_notifier: contextual information for event triggering
> invalidation
>   mm/mmu_notifier: use correct mmu_notifier events for each invalidation
>   mm/mmu_notifier: pass down vma and reasons why mmu notifier is
> happening
>   mm/mmu_notifier: mmu_notifier_range_update_to_read_only() helper
>   gpu/drm/radeon: optimize out the case when a range is updated to read
> only
>   gpu/drm/amdgpu: optimize out the case when a range is updated to read
> only
>   gpu/drm/i915: optimize out the case when a range is updated to read
> only
>   RDMA/umem_odp: optimize out the case when a range is updated to read
> only
>
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c  | 13 
>  drivers/gpu/drm/i915/i915_gem_userptr.c | 16 ++
>  drivers/gpu/drm/radeon/radeon_mn.c  | 13 
>  drivers/infiniband/core/umem_odp.c  | 22 +++--
>  fs/proc/task_mmu.c  |  3 +-
>  include/linux/mmu_notifier.h| 42 -
>  include/rdma/ib_umem_odp.h  |  1 +
>  kernel/events/uprobe

Re: [PATCH v4 9/9] RDMA/umem_odp: optimize out the case when a range is updated to read only

[Jerome Glisse]

On Wed, Jan 23, 2019 at 10:32:00PM +, Jason Gunthorpe wrote:
> On Wed, Jan 23, 2019 at 05:23:15PM -0500, jgli...@redhat.com wrote:
> > From: Jérôme Glisse 
> > 
> > When range of virtual address is updated read only and corresponding
> > user ptr object are already read only it is pointless to do anything.
> > Optimize this case out.
> > 
> > Signed-off-by: Jérôme Glisse 
> > Cc: Christian König 
> > Cc: Jan Kara 
> > Cc: Felix Kuehling 
> > Cc: Jason Gunthorpe 
> > Cc: Andrew Morton 
> > Cc: Matthew Wilcox 
> > Cc: Ross Zwisler 
> > Cc: Dan Williams 
> > Cc: Paolo Bonzini 
> > Cc: Radim Krčmář 
> > Cc: Michal Hocko 
> > Cc: Ralph Campbell 
> > Cc: John Hubbard 
> > Cc: k...@vger.kernel.org
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-r...@vger.kernel.org
> > Cc: linux-fsde...@vger.kernel.org
> > Cc: Arnd Bergmann 
> >  drivers/infiniband/core/umem_odp.c | 22 +++---
> >  include/rdma/ib_umem_odp.h |  1 +
> >  2 files changed, 20 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/infiniband/core/umem_odp.c 
> > b/drivers/infiniband/core/umem_odp.c
> > index a4ec43093cb3..fa4e7fdcabfc 100644
> > +++ b/drivers/infiniband/core/umem_odp.c
> > @@ -140,8 +140,15 @@ static void ib_umem_notifier_release(struct 
> > mmu_notifier *mn,
> >  static int invalidate_range_start_trampoline(struct ib_umem_odp *item,
> >  u64 start, u64 end, void *cookie)
> >  {
> > +   bool update_to_read_only = *((bool *)cookie);
> > +
> > ib_umem_notifier_start_account(item);
> > -   item->umem.context->invalidate_range(item, start, end);
> > +   /*
> > +* If it is already read only and we are updating to read only then we
> > +* do not need to change anything. So save time and skip this one.
> > +*/
> > +   if (!update_to_read_only || !item->read_only)
> > +   item->umem.context->invalidate_range(item, start, end);
> > return 0;
> >  }
> >  
> > @@ -150,6 +157,7 @@ static int 
> > ib_umem_notifier_invalidate_range_start(struct mmu_notifier *mn,
> >  {
> > struct ib_ucontext_per_mm *per_mm =
> > container_of(mn, struct ib_ucontext_per_mm, mn);
> > +   bool update_to_read_only;
> >  
> > if (range->blockable)
> > down_read(&per_mm->umem_rwsem);
> > @@ -166,10 +174,13 @@ static int 
> > ib_umem_notifier_invalidate_range_start(struct mmu_notifier *mn,
> > return 0;
> > }
> >  
> > +   update_to_read_only = mmu_notifier_range_update_to_read_only(range);
> > +
> > return rbt_ib_umem_for_each_in_range(&per_mm->umem_tree, range->start,
> >  range->end,
> >  invalidate_range_start_trampoline,
> > -range->blockable, NULL);
> > +range->blockable,
> > +&update_to_read_only);
> >  }
> >  
> >  static int invalidate_range_end_trampoline(struct ib_umem_odp *item, u64 
> > start,
> > @@ -363,6 +374,9 @@ struct ib_umem_odp *ib_alloc_odp_umem(struct 
> > ib_ucontext_per_mm *per_mm,
> > goto out_odp_data;
> > }
> >  
> > +   /* Assume read only at first, each time GUP is call this is updated. */
> > +   odp_data->read_only = true;
> > +
> > odp_data->dma_list =
> > vzalloc(array_size(pages, sizeof(*odp_data->dma_list)));
> > if (!odp_data->dma_list) {
> > @@ -619,8 +633,10 @@ int ib_umem_odp_map_dma_pages(struct ib_umem_odp 
> > *umem_odp, u64 user_virt,
> > goto out_put_task;
> > }
> >  
> > -   if (access_mask & ODP_WRITE_ALLOWED_BIT)
> > +   if (access_mask & ODP_WRITE_ALLOWED_BIT) {
> > +   umem_odp->read_only = false;
> 
> No locking?

The mmu notitfier exclusion will ensure that it is not missed
ie it will be false before any mmu notifier might be call on
page GUPed with write flag which is what matter here. So lock
are useless here.

> 
> > flags |= FOLL_WRITE;
> > +   }
> >  
> > start_idx = (user_virt - ib_umem_start(umem)) >> page_shift;
> > k = start_idx;
> > diff --git a/include/rdma/ib_umem_odp.h b/include/rdma/ib_umem_odp.h
> > index 0b1446fe2fab..8256668c6170 100644
> > +++ b/include/rdma/ib_umem_odp.h
> > @@ -76,6 +76,7 @@ struct ib_umem_odp {
> > struct completion   notifier_completion;
> > int dying;
> > struct work_struct  work;
> > +   bool read_only;
> >  };
> 
> The ib_umem already has a writeable flag. This reflects if the user
> asked for write permission to be granted.. The tracking here is if any
> remote fault thus far has requested write, is this an important
> difference to justify the new flag?

I did that patch couple week ago and now i do not remember why
i did not use that, i remember thinking about it ... damm i need
to keep better notes. I will review the code again.

Cheers,
Jérôme
___
dri-devel maili

[PATCH v4 7/9] gpu/drm/amdgpu: optimize out the case when a range is updated to read only

[jglisse]

From: Jérôme Glisse 

When range of virtual address is updated read only and corresponding
user ptr object are already read only it is pointless to do anything.
Optimize this case out.

Signed-off-by: Jérôme Glisse 
Cc: Christian König 
Cc: Jan Kara 
Cc: Felix Kuehling 
Cc: Jason Gunthorpe 
Cc: Andrew Morton 
Cc: Matthew Wilcox 
Cc: Ross Zwisler 
Cc: Dan Williams 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Michal Hocko 
Cc: Ralph Campbell 
Cc: John Hubbard 
Cc: k...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-r...@vger.kernel.org
Cc: linux-fsde...@vger.kernel.org
Cc: Arnd Bergmann 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index 3e6823fdd939..7880eda064cd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -294,6 +294,7 @@ static int amdgpu_mn_invalidate_range_start_hsa(struct 
mmu_notifier *mn,
 {
struct amdgpu_mn *amn = container_of(mn, struct amdgpu_mn, mn);
struct interval_tree_node *it;
+   bool update_to_read_only;
unsigned long end;
 
/* notification is exclusive, but interval is inclusive */
@@ -302,6 +303,8 @@ static int amdgpu_mn_invalidate_range_start_hsa(struct 
mmu_notifier *mn,
if (amdgpu_mn_read_lock(amn, range->blockable))
return -EAGAIN;
 
+   update_to_read_only = mmu_notifier_range_update_to_read_only(range);
+
it = interval_tree_iter_first(&amn->objects, range->start, end);
while (it) {
struct amdgpu_mn_node *node;
@@ -317,6 +320,16 @@ static int amdgpu_mn_invalidate_range_start_hsa(struct 
mmu_notifier *mn,
 
list_for_each_entry(bo, &node->bos, mn_list) {
struct kgd_mem *mem = bo->kfd_bo;
+   bool read_only;
+
+   /*
+* If it is already read only and we are updating to
+* read only then we do not need to change anything.
+* So save time and skip this one.
+*/
+   read_only = amdgpu_ttm_tt_is_readonly(bo->tbo.ttm);
+   if (update_to_read_only && read_only)
+   continue;
 
if (amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm,
 range->start,
-- 
2.17.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 9/9] RDMA/umem_odp: optimize out the case when a range is updated to read only

[jglisse]

From: Jérôme Glisse 

When range of virtual address is updated read only and corresponding
user ptr object are already read only it is pointless to do anything.
Optimize this case out.

Signed-off-by: Jérôme Glisse 
Cc: Christian König 
Cc: Jan Kara 
Cc: Felix Kuehling 
Cc: Jason Gunthorpe 
Cc: Andrew Morton 
Cc: Matthew Wilcox 
Cc: Ross Zwisler 
Cc: Dan Williams 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Michal Hocko 
Cc: Ralph Campbell 
Cc: John Hubbard 
Cc: k...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-r...@vger.kernel.org
Cc: linux-fsde...@vger.kernel.org
Cc: Arnd Bergmann 
---
 drivers/infiniband/core/umem_odp.c | 22 +++---
 include/rdma/ib_umem_odp.h |  1 +
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/core/umem_odp.c 
b/drivers/infiniband/core/umem_odp.c
index a4ec43093cb3..fa4e7fdcabfc 100644
--- a/drivers/infiniband/core/umem_odp.c
+++ b/drivers/infiniband/core/umem_odp.c
@@ -140,8 +140,15 @@ static void ib_umem_notifier_release(struct mmu_notifier 
*mn,
 static int invalidate_range_start_trampoline(struct ib_umem_odp *item,
 u64 start, u64 end, void *cookie)
 {
+   bool update_to_read_only = *((bool *)cookie);
+
ib_umem_notifier_start_account(item);
-   item->umem.context->invalidate_range(item, start, end);
+   /*
+* If it is already read only and we are updating to read only then we
+* do not need to change anything. So save time and skip this one.
+*/
+   if (!update_to_read_only || !item->read_only)
+   item->umem.context->invalidate_range(item, start, end);
return 0;
 }
 
@@ -150,6 +157,7 @@ static int ib_umem_notifier_invalidate_range_start(struct 
mmu_notifier *mn,
 {
struct ib_ucontext_per_mm *per_mm =
container_of(mn, struct ib_ucontext_per_mm, mn);
+   bool update_to_read_only;
 
if (range->blockable)
down_read(&per_mm->umem_rwsem);
@@ -166,10 +174,13 @@ static int ib_umem_notifier_invalidate_range_start(struct 
mmu_notifier *mn,
return 0;
}
 
+   update_to_read_only = mmu_notifier_range_update_to_read_only(range);
+
return rbt_ib_umem_for_each_in_range(&per_mm->umem_tree, range->start,
 range->end,
 invalidate_range_start_trampoline,
-range->blockable, NULL);
+range->blockable,
+&update_to_read_only);
 }
 
 static int invalidate_range_end_trampoline(struct ib_umem_odp *item, u64 start,
@@ -363,6 +374,9 @@ struct ib_umem_odp *ib_alloc_odp_umem(struct 
ib_ucontext_per_mm *per_mm,
goto out_odp_data;
}
 
+   /* Assume read only at first, each time GUP is call this is updated. */
+   odp_data->read_only = true;
+
odp_data->dma_list =
vzalloc(array_size(pages, sizeof(*odp_data->dma_list)));
if (!odp_data->dma_list) {
@@ -619,8 +633,10 @@ int ib_umem_odp_map_dma_pages(struct ib_umem_odp 
*umem_odp, u64 user_virt,
goto out_put_task;
}
 
-   if (access_mask & ODP_WRITE_ALLOWED_BIT)
+   if (access_mask & ODP_WRITE_ALLOWED_BIT) {
+   umem_odp->read_only = false;
flags |= FOLL_WRITE;
+   }
 
start_idx = (user_virt - ib_umem_start(umem)) >> page_shift;
k = start_idx;
diff --git a/include/rdma/ib_umem_odp.h b/include/rdma/ib_umem_odp.h
index 0b1446fe2fab..8256668c6170 100644
--- a/include/rdma/ib_umem_odp.h
+++ b/include/rdma/ib_umem_odp.h
@@ -76,6 +76,7 @@ struct ib_umem_odp {
struct completion   notifier_completion;
int dying;
struct work_struct  work;
+   bool read_only;
 };
 
 static inline struct ib_umem_odp *to_ib_umem_odp(struct ib_umem *umem)
-- 
2.17.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 8/9] gpu/drm/i915: optimize out the case when a range is updated to read only

[jglisse]

From: Jérôme Glisse 

When range of virtual address is updated read only and corresponding
user ptr object are already read only it is pointless to do anything.
Optimize this case out.

Signed-off-by: Jérôme Glisse 
Cc: Christian König 
Cc: Jan Kara 
Cc: Felix Kuehling 
Cc: Jason Gunthorpe 
Cc: Andrew Morton 
Cc: Matthew Wilcox 
Cc: Ross Zwisler 
Cc: Dan Williams 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Michal Hocko 
Cc: Ralph Campbell 
Cc: John Hubbard 
Cc: k...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-r...@vger.kernel.org
Cc: linux-fsde...@vger.kernel.org
Cc: Arnd Bergmann 
---
 drivers/gpu/drm/i915/i915_gem_userptr.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 9558582c105e..23330ac3d7ea 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -59,6 +59,7 @@ struct i915_mmu_object {
struct interval_tree_node it;
struct list_head link;
struct work_struct work;
+   bool read_only;
bool attached;
 };
 
@@ -119,6 +120,7 @@ static int 
i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
container_of(_mn, struct i915_mmu_notifier, mn);
struct i915_mmu_object *mo;
struct interval_tree_node *it;
+   bool update_to_read_only;
LIST_HEAD(cancelled);
unsigned long end;
 
@@ -128,6 +130,8 @@ static int 
i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
/* interval ranges are inclusive, but invalidate range is exclusive */
end = range->end - 1;
 
+   update_to_read_only = mmu_notifier_range_update_to_read_only(range);
+
spin_lock(&mn->lock);
it = interval_tree_iter_first(&mn->objects, range->start, end);
while (it) {
@@ -145,6 +149,17 @@ static int 
i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
 * object if it is not in the process of being destroyed.
 */
mo = container_of(it, struct i915_mmu_object, it);
+
+   /*
+* If it is already read only and we are updating to
+* read only then we do not need to change anything.
+* So save time and skip this one.
+*/
+   if (update_to_read_only && mo->read_only) {
+   it = interval_tree_iter_next(it, range->start, end);
+   continue;
+   }
+
if (kref_get_unless_zero(&mo->obj->base.refcount))
queue_work(mn->wq, &mo->work);
 
@@ -270,6 +285,7 @@ i915_gem_userptr_init__mmu_notifier(struct 
drm_i915_gem_object *obj,
mo->mn = mn;
mo->obj = obj;
mo->it.start = obj->userptr.ptr;
+   mo->read_only = i915_gem_object_is_readonly(obj);
mo->it.last = obj->userptr.ptr + obj->base.size - 1;
INIT_WORK(&mo->work, cancel_userptr);
 
-- 
2.17.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 4/9] mm/mmu_notifier: pass down vma and reasons why mmu notifier is happening

[jglisse]

From: Jérôme Glisse 

CPU page table update can happens for many reasons, not only as a result
of a syscall (munmap(), mprotect(), mremap(), madvise(), ...) but also
as a result of kernel activities (memory compression, reclaim, migration,
...).

Users of mmu notifier API track changes to the CPU page table and take
specific action for them. While current API only provide range of virtual
address affected by the change, not why the changes is happening

This patch is just passing down the new informations by adding it to the
mmu_notifier_range structure.

Signed-off-by: Jérôme Glisse 
Cc: Christian König 
Cc: Jan Kara 
Cc: Felix Kuehling 
Cc: Jason Gunthorpe 
Cc: Andrew Morton 
Cc: Matthew Wilcox 
Cc: Ross Zwisler 
Cc: Dan Williams 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Michal Hocko 
Cc: Ralph Campbell 
Cc: John Hubbard 
Cc: k...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-r...@vger.kernel.org
Cc: linux-fsde...@vger.kernel.org
Cc: Arnd Bergmann 
---
 include/linux/mmu_notifier.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
index a9808add4070..7514775817de 100644
--- a/include/linux/mmu_notifier.h
+++ b/include/linux/mmu_notifier.h
@@ -56,9 +56,11 @@ struct mmu_notifier_mm {
 };
 
 struct mmu_notifier_range {
+   struct vm_area_struct *vma;
struct mm_struct *mm;
unsigned long start;
unsigned long end;
+   enum mmu_notifier_event event;
bool blockable;
 };
 
@@ -354,6 +356,8 @@ static inline void mmu_notifier_range_init(struct 
mmu_notifier_range *range,
   unsigned long start,
   unsigned long end)
 {
+   range->vma = vma;
+   range->event = event;
range->mm = mm;
range->start = start;
range->end = end;
-- 
2.17.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 5/9] mm/mmu_notifier: mmu_notifier_range_update_to_read_only() helper

[jglisse]

From: Jérôme Glisse 

Helper to test if a range is updated to read only (it is still valid
to read from the range). This is useful for device driver or anyone
who wish to optimize out update when they know that they already have
the range map read only.

Signed-off-by: Jérôme Glisse 
Cc: Christian König 
Cc: Jan Kara 
Cc: Felix Kuehling 
Cc: Jason Gunthorpe 
Cc: Andrew Morton 
Cc: Matthew Wilcox 
Cc: Ross Zwisler 
Cc: Dan Williams 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Michal Hocko 
Cc: Ralph Campbell 
Cc: John Hubbard 
Cc: k...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-r...@vger.kernel.org
Cc: linux-fsde...@vger.kernel.org
Cc: Arnd Bergmann 
---
 include/linux/mmu_notifier.h |  4 
 mm/mmu_notifier.c| 10 ++
 2 files changed, 14 insertions(+)

diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
index 7514775817de..be873c431886 100644
--- a/include/linux/mmu_notifier.h
+++ b/include/linux/mmu_notifier.h
@@ -257,6 +257,8 @@ extern void __mmu_notifier_invalidate_range_end(struct 
mmu_notifier_range *r,
  bool only_end);
 extern void __mmu_notifier_invalidate_range(struct mm_struct *mm,
  unsigned long start, unsigned long end);
+extern bool
+mmu_notifier_range_update_to_read_only(const struct mmu_notifier_range *range);
 
 static inline void mmu_notifier_release(struct mm_struct *mm)
 {
@@ -553,6 +555,8 @@ static inline void mmu_notifier_mm_destroy(struct mm_struct 
*mm)
 {
 }
 
+#define mmu_notifier_range_update_to_read_only(r) false
+
 #define ptep_clear_flush_young_notify ptep_clear_flush_young
 #define pmdp_clear_flush_young_notify pmdp_clear_flush_young
 #define ptep_clear_young_notify ptep_test_and_clear_young
diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c
index 9c884abc7850..0b2f77715a08 100644
--- a/mm/mmu_notifier.c
+++ b/mm/mmu_notifier.c
@@ -395,3 +395,13 @@ void mmu_notifier_unregister_no_release(struct 
mmu_notifier *mn,
mmdrop(mm);
 }
 EXPORT_SYMBOL_GPL(mmu_notifier_unregister_no_release);
+
+bool
+mmu_notifier_range_update_to_read_only(const struct mmu_notifier_range *range)
+{
+   if (!range->vma || range->event != MMU_NOTIFY_PROTECTION_VMA)
+   return false;
+   /* Return true if the vma still have the read flag set. */
+   return range->vma->vm_flags & VM_READ;
+}
+EXPORT_SYMBOL_GPL(mmu_notifier_range_update_to_read_only);
-- 
2.17.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 6/9] gpu/drm/radeon: optimize out the case when a range is updated to read only

[jglisse]

From: Jérôme Glisse 

When range of virtual address is updated read only and corresponding
user ptr object are already read only it is pointless to do anything.
Optimize this case out.

Signed-off-by: Jérôme Glisse 
Cc: Christian König 
Cc: Jan Kara 
Cc: Felix Kuehling 
Cc: Jason Gunthorpe 
Cc: Andrew Morton 
Cc: Matthew Wilcox 
Cc: Ross Zwisler 
Cc: Dan Williams 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Michal Hocko 
Cc: Ralph Campbell 
Cc: John Hubbard 
Cc: k...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-r...@vger.kernel.org
Cc: linux-fsde...@vger.kernel.org
Cc: Arnd Bergmann 
---
 drivers/gpu/drm/radeon/radeon_mn.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_mn.c 
b/drivers/gpu/drm/radeon/radeon_mn.c
index b3019505065a..f77294f58e63 100644
--- a/drivers/gpu/drm/radeon/radeon_mn.c
+++ b/drivers/gpu/drm/radeon/radeon_mn.c
@@ -124,6 +124,7 @@ static int radeon_mn_invalidate_range_start(struct 
mmu_notifier *mn,
struct radeon_mn *rmn = container_of(mn, struct radeon_mn, mn);
struct ttm_operation_ctx ctx = { false, false };
struct interval_tree_node *it;
+   bool update_to_read_only;
unsigned long end;
int ret = 0;
 
@@ -138,6 +139,8 @@ static int radeon_mn_invalidate_range_start(struct 
mmu_notifier *mn,
else if (!mutex_trylock(&rmn->lock))
return -EAGAIN;
 
+   update_to_read_only = mmu_notifier_range_update_to_read_only(range);
+
it = interval_tree_iter_first(&rmn->objects, range->start, end);
while (it) {
struct radeon_mn_node *node;
@@ -153,10 +156,20 @@ static int radeon_mn_invalidate_range_start(struct 
mmu_notifier *mn,
it = interval_tree_iter_next(it, range->start, end);
 
list_for_each_entry(bo, &node->bos, mn_list) {
+   bool read_only;
 
if (!bo->tbo.ttm || bo->tbo.ttm->state != tt_bound)
continue;
 
+   /*
+* If it is already read only and we are updating to
+* read only then we do not need to change anything.
+* So save time and skip this one.
+*/
+   read_only = radeon_ttm_tt_is_readonly(bo->tbo.ttm);
+   if (update_to_read_only && read_only)
+   continue;
+
r = radeon_bo_reserve(bo, true);
if (r) {
DRM_ERROR("(%ld) failed to reserve user bo\n", 
r);
-- 
2.17.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 3/9] mm/mmu_notifier: use correct mmu_notifier events for each invalidation

[jglisse]

From: Jérôme Glisse 

This update each existing invalidation to use the correct mmu notifier
event that represent what is happening to the CPU page table. See the
patch which introduced the events to see the rational behind this.

Signed-off-by: Jérôme Glisse 
Cc: Christian König 
Cc: Jan Kara 
Cc: Felix Kuehling 
Cc: Jason Gunthorpe 
Cc: Andrew Morton 
Cc: Matthew Wilcox 
Cc: Ross Zwisler 
Cc: Dan Williams 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Michal Hocko 
Cc: Ralph Campbell 
Cc: John Hubbard 
Cc: k...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-r...@vger.kernel.org
Cc: linux-fsde...@vger.kernel.org
Cc: Arnd Bergmann 
---
 fs/proc/task_mmu.c  |  2 +-
 kernel/events/uprobes.c |  2 +-
 mm/huge_memory.c| 14 ++
 mm/hugetlb.c|  7 ---
 mm/khugepaged.c |  2 +-
 mm/ksm.c|  4 ++--
 mm/madvise.c|  2 +-
 mm/memory.c | 16 
 mm/migrate.c|  4 ++--
 mm/mprotect.c   |  5 +++--
 mm/rmap.c   |  6 +++---
 11 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 57e7f98647d3..cce226f3305f 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1143,7 +1143,7 @@ static ssize_t clear_refs_write(struct file *file, const 
char __user *buf,
break;
}
 
-   mmu_notifier_range_init(&range, MMU_NOTIFY_UNMAP,
+   mmu_notifier_range_init(&range, MMU_NOTIFY_SOFT_DIRTY,
NULL, mm, 0, -1UL);
mmu_notifier_invalidate_range_start(&range);
}
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index b67fe7e59621..87e76a1dc758 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -174,7 +174,7 @@ static int __replace_page(struct vm_area_struct *vma, 
unsigned long addr,
struct mmu_notifier_range range;
struct mem_cgroup *memcg;
 
-   mmu_notifier_range_init(&range, MMU_NOTIFY_UNMAP, vma, mm, addr,
+   mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, vma, mm, addr,
addr + PAGE_SIZE);
 
VM_BUG_ON_PAGE(PageTransHuge(old_page), old_page);
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index b353e8b7876f..957d23754217 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1182,9 +1182,8 @@ static vm_fault_t do_huge_pmd_wp_page_fallback(struct 
vm_fault *vmf,
cond_resched();
}
 
-   mmu_notifier_range_init(&range, MMU_NOTIFY_UNMAP, vma, vma->vm_mm,
-   haddr,
-   haddr + HPAGE_PMD_SIZE);
+   mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, vma, vma->vm_mm,
+   haddr, haddr + HPAGE_PMD_SIZE);
mmu_notifier_invalidate_range_start(&range);
 
vmf->ptl = pmd_lock(vma->vm_mm, vmf->pmd);
@@ -1346,9 +1345,8 @@ vm_fault_t do_huge_pmd_wp_page(struct vm_fault *vmf, 
pmd_t orig_pmd)
vma, HPAGE_PMD_NR);
__SetPageUptodate(new_page);
 
-   mmu_notifier_range_init(&range, MMU_NOTIFY_UNMAP, vma, vma->vm_mm,
-   haddr,
-   haddr + HPAGE_PMD_SIZE);
+   mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, vma, vma->vm_mm,
+   haddr, haddr + HPAGE_PMD_SIZE);
mmu_notifier_invalidate_range_start(&range);
 
spin_lock(vmf->ptl);
@@ -2025,7 +2023,7 @@ void __split_huge_pud(struct vm_area_struct *vma, pud_t 
*pud,
spinlock_t *ptl;
struct mmu_notifier_range range;
 
-   mmu_notifier_range_init(&range, MMU_NOTIFY_UNMAP, vma, vma->vm_mm,
+   mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, vma, vma->vm_mm,
address & HPAGE_PUD_MASK,
(address & HPAGE_PUD_MASK) + HPAGE_PUD_SIZE);
mmu_notifier_invalidate_range_start(&range);
@@ -2244,7 +2242,7 @@ void __split_huge_pmd(struct vm_area_struct *vma, pmd_t 
*pmd,
spinlock_t *ptl;
struct mmu_notifier_range range;
 
-   mmu_notifier_range_init(&range, MMU_NOTIFY_UNMAP, vma, vma->vm_mm,
+   mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, vma, vma->vm_mm,
address & HPAGE_PMD_MASK,
(address & HPAGE_PMD_MASK) + HPAGE_PMD_SIZE);
mmu_notifier_invalidate_range_start(&range);
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index cbda46ad6a30..f691398ac6b6 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3246,7 +3246,7 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct 
mm_struct *src,
cow = (vma->vm_flags & (VM_SHARED | VM_MAYWRITE)) == VM_MAYWRITE;
 
if (cow) {
-   mmu_notifier_range_init(&range, MMU_NOTIFY_UNMAP, vma, src,
+   mmu_notifier_range_ini

[PATCH v4 0/9] mmu notifier provide context informations

[jglisse]

From: Jérôme Glisse 

Hi Andrew, i see that you still have my event patch in you queue [1].
This patchset replace that single patch and is broken down in further
step so that it is easier to review and ascertain that no mistake were
made during mechanical changes. Here are the step:

Patch 1 - add the enum values
Patch 2 - coccinelle semantic patch to convert all call site of
  mmu_notifier_range_init to default enum value and also
  to passing down the vma when it is available
Patch 3 - update many call site to more accurate enum values
Patch 4 - add the information to the mmu_notifier_range struct
Patch 5 - helper to test if a range is updated to read only

All the remaining patches are update to various driver to demonstrate
how this new information get use by device driver. I build tested
with make all and make all minus everything that enable mmu notifier
ie building with MMU_NOTIFIER=no. Also tested with some radeon,amd
gpu and intel gpu.

If they are no objections i believe best plan would be to merge the
the first 5 patches (all mm changes) through your queue for 5.1 and
then to delay driver update to each individual driver tree for 5.2.
This will allow each individual device driver maintainer time to more
thouroughly test this more then my own testing.

Note that i also intend to use this feature further in nouveau and
HMM down the road. I also expect that other user like KVM might be
interested into leveraging this new information to optimize some of
there secondary page table invalidation.

Here is an explaination on the rational for this patchset:


CPU page table update can happens for many reasons, not only as a result
of a syscall (munmap(), mprotect(), mremap(), madvise(), ...) but also
as a result of kernel activities (memory compression, reclaim, migration,
...).

This patch introduce a set of enums that can be associated with each of
the events triggering a mmu notifier. Latter patches take advantages of
those enum values.

- UNMAP: munmap() or mremap()
- CLEAR: page table is cleared (migration, compaction, reclaim, ...)
- PROTECTION_VMA: change in access protections for the range
- PROTECTION_PAGE: change in access protections for page in the range
- SOFT_DIRTY: soft dirtyness tracking

Being able to identify munmap() and mremap() from other reasons why the
page table is cleared is important to allow user of mmu notifier to
update their own internal tracking structure accordingly (on munmap or
mremap it is not longer needed to track range of virtual address as it
becomes invalid).

[1] 
https://www.ozlabs.org/~akpm/mmotm/broken-out/mm-mmu_notifier-contextual-information-for-event-triggering-invalidation-v2.patch

Cc: Christian König 
Cc: Jan Kara 
Cc: Felix Kuehling 
Cc: Jason Gunthorpe 
Cc: Andrew Morton 
Cc: Matthew Wilcox 
Cc: Ross Zwisler 
Cc: Dan Williams 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Michal Hocko 
Cc: Ralph Campbell 
Cc: John Hubbard 
Cc: k...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-r...@vger.kernel.org
Cc: linux-fsde...@vger.kernel.org
Cc: Arnd Bergmann 

Jérôme Glisse (9):
  mm/mmu_notifier: contextual information for event enums
  mm/mmu_notifier: contextual information for event triggering
invalidation
  mm/mmu_notifier: use correct mmu_notifier events for each invalidation
  mm/mmu_notifier: pass down vma and reasons why mmu notifier is
happening
  mm/mmu_notifier: mmu_notifier_range_update_to_read_only() helper
  gpu/drm/radeon: optimize out the case when a range is updated to read
only
  gpu/drm/amdgpu: optimize out the case when a range is updated to read
only
  gpu/drm/i915: optimize out the case when a range is updated to read
only
  RDMA/umem_odp: optimize out the case when a range is updated to read
only

 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c  | 13 
 drivers/gpu/drm/i915/i915_gem_userptr.c | 16 ++
 drivers/gpu/drm/radeon/radeon_mn.c  | 13 
 drivers/infiniband/core/umem_odp.c  | 22 +++--
 fs/proc/task_mmu.c  |  3 +-
 include/linux/mmu_notifier.h| 42 -
 include/rdma/ib_umem_odp.h  |  1 +
 kernel/events/uprobes.c |  3 +-
 mm/huge_memory.c| 14 +
 mm/hugetlb.c| 11 ---
 mm/khugepaged.c |  3 +-
 mm/ksm.c|  6 ++--
 mm/madvise.c|  3 +-
 mm/memory.c | 25 +--
 mm/migrate.c|  5 ++-
 mm/mmu_notifier.c   | 10 ++
 mm/mprotect.c   |  4 ++-
 mm/mremap.c |  3 +-
 mm/oom_kill.c   |  3 +-
 mm/rmap.c   |  6 ++--
 20 files changed, 171 insertions(+), 35 deletions(-)

-- 
2.17.2

___
dri

[PATCH v4 2/9] mm/mmu_notifier: contextual information for event triggering invalidation

[jglisse]

From: Jérôme Glisse 

CPU page table update can happens for many reasons, not only as a result
of a syscall (munmap(), mprotect(), mremap(), madvise(), ...) but also
as a result of kernel activities (memory compression, reclaim, migration,
...).

Users of mmu notifier API track changes to the CPU page table and take
specific action for them. While current API only provide range of virtual
address affected by the change, not why the changes is happening.

This patchset do the initial mechanical convertion of all the places that
calls mmu_notifier_range_init to also provide the default MMU_NOTIFY_UNMAP
event as well as the vma if it is know (most invalidation happens against
a given vma). Passing down the vma allows the users of mmu notifier to
inspect the new vma page protection.

The MMU_NOTIFY_UNMAP is always the safe default as users of mmu notifier
should assume that every for the range is going away when that event
happens. A latter patch do convert mm call path to use a more appropriate
events for each call.

This is done as 2 patches so that no call site is forgotten especialy
as it uses this following coccinelle patch:

%<--
@@
identifier I1, I2, I3, I4;
@@
static inline void mmu_notifier_range_init(struct mmu_notifier_range *I1,
+enum mmu_notifier_event event,
+struct vm_area_struct *vma,
struct mm_struct *I2, unsigned long I3, unsigned long I4) { ... }

@@
@@
-#define mmu_notifier_range_init(range, mm, start, end)
+#define mmu_notifier_range_init(range, event, vma, mm, start, end)

@@
expression E1, E3, E4;
identifier I1;
@@
<...
mmu_notifier_range_init(E1,
+MMU_NOTIFY_UNMAP, I1,
I1->vm_mm, E3, E4)
...>

@@
expression E1, E2, E3, E4;
identifier FN, VMA;
@@
FN(..., struct vm_area_struct *VMA, ...) {
<...
mmu_notifier_range_init(E1,
+MMU_NOTIFY_UNMAP, VMA,
E2, E3, E4)
...> }

@@
expression E1, E2, E3, E4;
identifier FN, VMA;
@@
FN(...) {
struct vm_area_struct *VMA;
<...
mmu_notifier_range_init(E1,
+MMU_NOTIFY_UNMAP, VMA,
E2, E3, E4)
...> }

@@
expression E1, E2, E3, E4;
identifier FN;
@@
FN(...) {
<...
mmu_notifier_range_init(E1,
+MMU_NOTIFY_UNMAP, NULL,
E2, E3, E4)
...> }
-->%

Applied with:
spatch --all-includes --sp-file mmu-notifier.spatch fs/proc/task_mmu.c 
--in-place
spatch --sp-file mmu-notifier.spatch --dir kernel/events/ --in-place
spatch --sp-file mmu-notifier.spatch --dir mm --in-place

Signed-off-by: Jérôme Glisse 
Cc: Christian König 
Cc: Jan Kara 
Cc: Felix Kuehling 
Cc: Jason Gunthorpe 
Cc: Andrew Morton 
Cc: Matthew Wilcox 
Cc: Ross Zwisler 
Cc: Dan Williams 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Michal Hocko 
Cc: Ralph Campbell 
Cc: John Hubbard 
Cc: k...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-r...@vger.kernel.org
Cc: linux-fsde...@vger.kernel.org
Cc: Arnd Bergmann 
---
 fs/proc/task_mmu.c   |  3 ++-
 include/linux/mmu_notifier.h |  4 +++-
 kernel/events/uprobes.c  |  3 ++-
 mm/huge_memory.c | 12 
 mm/hugetlb.c | 10 ++
 mm/khugepaged.c  |  3 ++-
 mm/ksm.c |  6 --
 mm/madvise.c |  3 ++-
 mm/memory.c  | 25 -
 mm/migrate.c |  5 -
 mm/mprotect.c|  3 ++-
 mm/mremap.c  |  3 ++-
 mm/oom_kill.c|  3 ++-
 mm/rmap.c|  6 --
 14 files changed, 59 insertions(+), 30 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index f0ec9edab2f3..57e7f98647d3 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1143,7 +1143,8 @@ static ssize_t clear_refs_write(struct file *file, const 
char __user *buf,
break;
}
 
-   mmu_notifier_range_init(&range, mm, 0, -1UL);
+   mmu_notifier_range_init(&range, MMU_NOTIFY_UNMAP,
+   NULL, mm, 0, -1UL);
mmu_notifier_invalidate_range_start(&range);
}
walk_page_range(0, mm->highest_vm_end, &clear_refs_walk);
diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
index abc9dbb7bcb6..a9808add4070 100644
--- a/include/linux/mmu_notifier.h
+++ b/include/linux/mmu_notifier.h
@@ -348,6 +348,8 @@ static inline void mmu_notifier_mm_destroy(struct mm_struct 
*mm)
 
 
 static inline void mmu_notifier_range_init(struct mmu_notifier_range *range,
+  enum mmu_notifier_event event,
+  struct vm_area_struct *vma,
   struct mm_struct *mm,
   unsigned long start,
   unsigned long end)
@@ -482,7 +484,7 @@ static inline void _mmu_notifier_range_init(struct 
mmu_no

[PATCH v4 1/9] mm/mmu_notifier: contextual information for event enums

[jglisse]

From: Jérôme Glisse 

CPU page table update can happens for many reasons, not only as a result
of a syscall (munmap(), mprotect(), mremap(), madvise(), ...) but also
as a result of kernel activities (memory compression, reclaim, migration,
...).

This patch introduce a set of enums that can be associated with each of
the events triggering a mmu notifier. Latter patches take advantages of
those enum values.

- UNMAP: munmap() or mremap()
- CLEAR: page table is cleared (migration, compaction, reclaim, ...)
- PROTECTION_VMA: change in access protections for the range
- PROTECTION_PAGE: change in access protections for page in the range
- SOFT_DIRTY: soft dirtyness tracking

Being able to identify munmap() and mremap() from other reasons why the
page table is cleared is important to allow user of mmu notifier to
update their own internal tracking structure accordingly (on munmap or
mremap it is not longer needed to track range of virtual address as it
becomes invalid).

Signed-off-by: Jérôme Glisse 
Cc: Christian König 
Cc: Jan Kara 
Cc: Felix Kuehling 
Cc: Jason Gunthorpe 
Cc: Andrew Morton 
Cc: Matthew Wilcox 
Cc: Ross Zwisler 
Cc: Dan Williams 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: Michal Hocko 
Cc: Ralph Campbell 
Cc: John Hubbard 
Cc: k...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-r...@vger.kernel.org
Cc: linux-fsde...@vger.kernel.org
Cc: Arnd Bergmann 
---
 include/linux/mmu_notifier.h | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
index 4050ec1c3b45..abc9dbb7bcb6 100644
--- a/include/linux/mmu_notifier.h
+++ b/include/linux/mmu_notifier.h
@@ -10,6 +10,36 @@
 struct mmu_notifier;
 struct mmu_notifier_ops;
 
+/**
+ * enum mmu_notifier_event - reason for the mmu notifier callback
+ * @MMU_NOTIFY_UNMAP: either munmap() that unmap the range or a mremap() that
+ * move the range
+ *
+ * @MMU_NOTIFY_CLEAR: clear page table entry (many reasons for this like
+ * madvise() or replacing a page by another one, ...).
+ *
+ * @MMU_NOTIFY_PROTECTION_VMA: update is due to protection change for the range
+ * ie using the vma access permission (vm_page_prot) to update the whole range
+ * is enough no need to inspect changes to the CPU page table (mprotect()
+ * syscall)
+ *
+ * @MMU_NOTIFY_PROTECTION_PAGE: update is due to change in read/write flag for
+ * pages in the range so to mirror those changes the user must inspect the CPU
+ * page table (from the end callback).
+ *
+ * @MMU_NOTIFY_SOFT_DIRTY: soft dirty accounting (still same page and same
+ * access flags). User should soft dirty the page in the end callback to make
+ * sure that anyone relying on soft dirtyness catch pages that might be written
+ * through non CPU mappings.
+ */
+enum mmu_notifier_event {
+   MMU_NOTIFY_UNMAP = 0,
+   MMU_NOTIFY_CLEAR,
+   MMU_NOTIFY_PROTECTION_VMA,
+   MMU_NOTIFY_PROTECTION_PAGE,
+   MMU_NOTIFY_SOFT_DIRTY,
+};
+
 #ifdef CONFIG_MMU_NOTIFIER
 
 /*
-- 
2.17.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/bridge: sil_sii8620: depend on INPUT instead of selecting it.

[Laurent Pinchart]

Hello Dmity,

On Wed, Jan 23, 2019 at 02:21:05PM -0800, Dmitry Torokhov wrote:
> On Thu, Jan 24, 2019 at 12:17:35AM +0200, Laurent Pinchart wrote:
> > On Wed, Jan 23, 2019 at 02:03:42PM -0800, Dmitry Torokhov wrote:
> >> On Wed, Jan 23, 2019 at 09:45:56AM +0100, Lukas Wunner wrote:
> >>> On Tue, Jan 22, 2019 at 06:13:11AM -0800, Ronald Tschalär wrote:
>  commit d6abe6df706c66d803e6dd4fe98c1b6b7f125a56 (drm/bridge:
>  sil_sii8620: do not have a dependency of RC_CORE) added a dependency on
>  INPUT. However, this causes problems with other drivers, in particular
>  an input driver that depends on MFD_INTEL_LPSS_PCI (to be added in a
>  future commit):
>  
>    drivers/clk/Kconfig:9:error: recursive dependency detected!
>    drivers/clk/Kconfig:9:symbol COMMON_CLK is selected by 
>  MFD_INTEL_LPSS
>    drivers/mfd/Kconfig:566:  symbol MFD_INTEL_LPSS is selected by 
>  MFD_INTEL_LPSS_PCI
>    drivers/mfd/Kconfig:580:  symbol MFD_INTEL_LPSS_PCI is implied by 
>  KEYBOARD_APPLESPI
>    drivers/input/keyboard/Kconfig:73:symbol KEYBOARD_APPLESPI depends 
>  on INPUT
>    drivers/input/Kconfig:8:  symbol INPUT is selected by 
>  DRM_SIL_SII8620
>    drivers/gpu/drm/bridge/Kconfig:83:symbol DRM_SIL_SII8620 depends 
>  on DRM_BRIDGE
>    drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by 
>  DRM_PL111
>    drivers/gpu/drm/pl111/Kconfig:1:  symbol DRM_PL111 depends on 
>  COMMON_CLK
>  
>  According to the docs, select should only be used for non-visible
>  symbols. Furthermore almost all other references to INPUT throughout the
>  kernel config are depends, not selects. Hence this change.
> >> 
> >> I think this is not as cut and dry. We should be able to select needed
> >> subsystems (such as INPUT, USB, etc) even if they are user visible.
> > 
> > Semantically, maybe, but given the current state of Kconfig this results
> > in a recursive dependencies nightmare. It's a no-go.
> > 
> >> User, when enabling a piece of hardware, does not need to know ultimate
> >> details of all subsystems the driver might need ti function.
> >> 
> >> It looks like one of the drivers implies MFD_INTEL_LPSS_PCI, maybe
> >> treating imply the same as select when detecting circular dependency is
> >> wrong as we are allowed to deselect implied dependencies?
> >> 
>  
>  CC: Inki Dae 
>  CC: Andrzej Hajda 
>  Signed-off-by: Ronald Tschalär 
> >>> 
> >>> Reviewed-by: Lukas Wunner 
> >>> 
> >>> I think this needs to be merged through the input tree as a prerequisite
> >>> for the applespi.c driver (keyboard + touchpad driver for 2015+ MacBook,
> >>> MacBook Air and MacBook Pro which uses SPI instead of USB) to avoid
> >>> breaking the build.  Adding Dmitry.
> >> 
> >> I have no idea what applespi.c is (it is definitely not in my tree), so
> >> I think it should be merged through the same tree that the original
> >> commit was introduced through.
> >> 
>  ---
>   drivers/gpu/drm/bridge/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>  
>  diff --git a/drivers/gpu/drm/bridge/Kconfig 
>  b/drivers/gpu/drm/bridge/Kconfig
>  index 2fee47b0d50b..eabedc83f25c 100644
>  --- a/drivers/gpu/drm/bridge/Kconfig
>  +++ b/drivers/gpu/drm/bridge/Kconfig
>  @@ -83,9 +83,9 @@ config DRM_PARADE_PS8622
>   config DRM_SIL_SII8620
>   tristate "Silicon Image SII8620 HDMI/MHL bridge"
>   depends on OF
>  +depends on INPUT
>   select DRM_KMS_HELPER
>   imply EXTCON
>  -select INPUT
>   select RC_CORE
> >> 
> >> Keeping "select RC_CORE" is wrong though, as the driver appears to be
> >> working find without RC. Maybe it should be stubbed out?
> > 
> > It should definitely not be select'ed as it's a user-visible symbol. My
> > preference would be to simply revert d6abe6df706c. If we want (and can)
> > work without RC core then it should be stubbed out.
> > 
> > Commit d6abe6df706c states
> > 
> > And some boards not using remote controller device don't really
> > need to know that RC_CORE config should be enabled to use sil_sii8620
> > driver only for HDMI.
> > 
> > The same reasoning applies to INPUT, if we agree that depending on
> > RC_CORE is confusing for users, then depending on INPUT is confusing as
> > well. There's not reason to apply different standards to INPUT and
> > RC_CORE, depending on one and selecting the other doesn't make much
> > sense.
> 
> OK, so revert + patch to stub out RC calls? That works for me (and I
> still say it should go through the same tree that introduced
> d6abe6df706c).

Yes, that sounds good to me.

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/bridge: sil_sii8620: depend on INPUT instead of selecting it.

[Laurent Pinchart]

Hello Dmity,

On Wed, Jan 23, 2019 at 02:03:42PM -0800, Dmitry Torokhov wrote:
> On Wed, Jan 23, 2019 at 09:45:56AM +0100, Lukas Wunner wrote:
> > On Tue, Jan 22, 2019 at 06:13:11AM -0800, Ronald Tschalär wrote:
> >> commit d6abe6df706c66d803e6dd4fe98c1b6b7f125a56 (drm/bridge:
> >> sil_sii8620: do not have a dependency of RC_CORE) added a dependency on
> >> INPUT. However, this causes problems with other drivers, in particular
> >> an input driver that depends on MFD_INTEL_LPSS_PCI (to be added in a
> >> future commit):
> >> 
> >>   drivers/clk/Kconfig:9:error: recursive dependency detected!
> >>   drivers/clk/Kconfig:9:symbol COMMON_CLK is selected by 
> >> MFD_INTEL_LPSS
> >>   drivers/mfd/Kconfig:566:  symbol MFD_INTEL_LPSS is selected by 
> >> MFD_INTEL_LPSS_PCI
> >>   drivers/mfd/Kconfig:580:  symbol MFD_INTEL_LPSS_PCI is implied by 
> >> KEYBOARD_APPLESPI
> >>   drivers/input/keyboard/Kconfig:73:symbol KEYBOARD_APPLESPI depends 
> >> on INPUT
> >>   drivers/input/Kconfig:8:  symbol INPUT is selected by DRM_SIL_SII8620
> >>   drivers/gpu/drm/bridge/Kconfig:83:symbol DRM_SIL_SII8620 depends on 
> >> DRM_BRIDGE
> >>   drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by 
> >> DRM_PL111
> >>   drivers/gpu/drm/pl111/Kconfig:1:  symbol DRM_PL111 depends on 
> >> COMMON_CLK
> >> 
> >> According to the docs, select should only be used for non-visible
> >> symbols. Furthermore almost all other references to INPUT throughout the
> >> kernel config are depends, not selects. Hence this change.
> 
> I think this is not as cut and dry. We should be able to select needed
> subsystems (such as INPUT, USB, etc) even if they are user visible.

Semantically, maybe, but given the current state of Kconfig this results
in a recursive dependencies nightmare. It's a no-go.

> User, when enabling a piece of hardware, does not need to know ultimate
> details of all subsystems the driver might need ti function.
> 
> It looks like one of the drivers implies MFD_INTEL_LPSS_PCI, maybe
> treating imply the same as select when detecting circular dependency is
> wrong as we are allowed to deselect implied dependencies?
> 
> >> 
> >> CC: Inki Dae 
> >> CC: Andrzej Hajda 
> >> Signed-off-by: Ronald Tschalär 
> > 
> > Reviewed-by: Lukas Wunner 
> > 
> > I think this needs to be merged through the input tree as a prerequisite
> > for the applespi.c driver (keyboard + touchpad driver for 2015+ MacBook,
> > MacBook Air and MacBook Pro which uses SPI instead of USB) to avoid
> > breaking the build.  Adding Dmitry.
> 
> I have no idea what applespi.c is (it is definitely not in my tree), so
> I think it should be merged through the same tree that the original
> commit was introduced through.
> 
> >> ---
> >>  drivers/gpu/drm/bridge/Kconfig | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/Kconfig 
> >> b/drivers/gpu/drm/bridge/Kconfig
> >> index 2fee47b0d50b..eabedc83f25c 100644
> >> --- a/drivers/gpu/drm/bridge/Kconfig
> >> +++ b/drivers/gpu/drm/bridge/Kconfig
> >> @@ -83,9 +83,9 @@ config DRM_PARADE_PS8622
> >>  config DRM_SIL_SII8620
> >>tristate "Silicon Image SII8620 HDMI/MHL bridge"
> >>depends on OF
> >> +  depends on INPUT
> >>select DRM_KMS_HELPER
> >>imply EXTCON
> >> -  select INPUT
> >>select RC_CORE
> 
> Keeping "select RC_CORE" is wrong though, as the driver appears to be
> working find without RC. Maybe it should be stubbed out?

It should definitely not be select'ed as it's a user-visible symbol. My
preference would be to simply revert d6abe6df706c. If we want (and can)
work without RC core then it should be stubbed out.

Commit d6abe6df706c states

And some boards not using remote controller device don't really
need to know that RC_CORE config should be enabled to use sil_sii8620
driver only for HDMI.

The same reasoning applies to INPUT, if we agree that depending on
RC_CORE is confusing for users, then depending on INPUT is confusing as
well. There's not reason to apply different standards to INPUT and
RC_CORE, depending on one and selecting the other doesn't make much
sense.

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 109445] Graveyard Keeper: Lockup in under 5min of play.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=109445

Mike Mestnik  changed:

   What|Removed |Added

URL||https://store.steampowered.
   ||com/app/599140/Graveyard_Ke
   ||eper/

-- 
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 109445] Graveyard Keeper: Lockup in under 5min of play.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=109445

--- Comment #1 from Mike Mestnik  
---
Created attachment 143210
  --> https://bugs.freedesktop.org/attachment.cgi?id=143210&action=edit
ar file containing glxinfo and Xorg.log

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 109445] Graveyard Keeper: Lockup in under 5min of play.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=109445

Bug ID: 109445
   Summary: Graveyard Keeper: Lockup in under 5min of play.
   Product: DRI
   Version: unspecified
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: cheako+bugs_freedesktop_...@mikemestnik.net

When I play graveyard keeper, native or via wine, my system locks-up(does not
respond to ping or ssh, but dose continue to spin fan) after under 5min of
play. 
 I can't even get to a point where i can save my progress. 

I'm able to load it under renderdoc, but am unable to create a capture. 
Playing under renderdoc results in lower framerate, but little else seems
different(still locks-up).

bnieuwen2uizen, on IRC, asked me to open a bug report.

ThinkPad E585
8GB RAM, 8GB Swap.
AMD Ryzen 7 2700U with Radeon Vega Mobile Gfx
Manufacturer: LENOVO
Product Name: 20KV000YUS

Debian sid multi-arch
Graveyard Keeper is a 32bit Windows app, can't confirm native architecture.

-- 
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 109444] Graveyard Keeper: Lockup in under 5min of play.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=109444

Bug ID: 109444
   Summary: Graveyard Keeper: Lockup in under 5min of play.
   Product: DRI
   Version: unspecified
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: cheako+bugs_freedesktop_...@mikemestnik.net

Created attachment 143209
  --> https://bugs.freedesktop.org/attachment.cgi?id=143209&action=edit
ar file with glxinfo and Xorg.log

When I play graveyard keeper, native or via wine, my system locks-up(does not
respond to ping or ssh, but dose continue to spin fan) after under 5min of
play. 
 I can't even get to a point where i can save my progress. 

I'm able to load it under renderdoc, but am unable to create a capture. 
Playing under renderdoc results in lower framerate, but little else seems
different(still locks-up).

bnieuwen2uizen, on IRC, asked me to open a bug report.

ThinkPad E585
8GB RAM, 8GB Swap.
AMD Ryzen 7 2700U with Radeon Vega Mobile Gfx
Manufacturer: LENOVO
Product Name: 20KV000YUS

Debian sid multi-arch
Graveyard Keeper is a 32bit Windows app, can't confirm native architecture.

-- 
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 201497] [amdgpu]: '*ERROR* No EDID read' is back in 4.19

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=201497

--- Comment #6 from Daniel Andersson (engyw...@gmail.com) ---
Created attachment 280701
  --> https://bugzilla.kernel.org/attachment.cgi?id=280701&action=edit
5.0-rc3 drm.debug=0x4

This is still an issue on 5.0-rc3.

-- 
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 201497] [amdgpu]: '*ERROR* No EDID read' is back in 4.19

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=201497

Daniel Andersson (engyw...@gmail.com) changed:

   What|Removed |Added

 Kernel Version|4.19|4.19 4.20 5.0-rc1 5.0-rc3
 Regression|No  |Yes

-- 
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 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105733

--- Comment #60 from l...@protonmail.ch ---
dwagner, my problem persists even if I completely power the system down after
shutting it down by holding down the power button and then turning the PSU
completely off. I have not tried shutting it off only using the PSU fearing
damage to my hardware, although I will try that the next time at least once.

Also, it is reassuring to see that I am not the only one experiencing such odd
behavior, but could it not be that we simply all use faulty hardware?

-- 
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] xf86drm: Add drmIsMaster()

[Christopher James Halse Rogers]

On 24 January 2019 6:18:32 am NZDT, Emil Velikov  
wrote:
>On Wed, 23 Jan 2019 at 04:39, Christopher James Halse Rogers
> wrote:
>>
>> We can't use drmSetMaster to query whether or not a drm fd is master
>> because it requires CAP_SYS_ADMIN, even if the fd *is* a master fd.
>>
>> Pick DRM_IOCTL_MODE_ATTACHMODE as a long-deprecated ioctl that is
>> DRM_MASTER but not DRM_ROOT_ONLY as the probe by which we can detect
>> whether or not the fd is master.
>>
>> This is useful for code that might get master by open()ing the drm
>device
>> while no other master exists, but can't call drmSetMaster itself
>because
>> it's not running as root or is in a container, where container-root
>isn't
>> real-root.
>>
>> v2: Use the AUTH_MAGIC request rather than MODE_ATTACHMODE, as it's
>more
>> clearly related to master status.
>>
>> Signed-off-by: Christopher James Halse Rogers
>
>> ---
>>  xf86drm.c | 15 +++
>>  xf86drm.h |  2 ++
>>  2 files changed, 17 insertions(+)
>>
>> diff --git a/xf86drm.c b/xf86drm.c
>> index 10df682b..adee5bd9 100644
>> --- a/xf86drm.c
>> +++ b/xf86drm.c
>> @@ -2741,6 +2741,21 @@ drm_public int drmDropMaster(int fd)
>>  return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL);
>>  }
>>
>> +drm_public bool drmIsMaster(int fd)
>> +{
>> +/* Detect master by attempting something that requires
>master.
>> + *
>> + * Authenticating magic tokens requires master and 0 is
>> + * guaranteed to be an invalid magic number. Attempting this
>on
>> + * a master fd will fail therefore fail with EINVAL because
>0 is
>> + * invalid.
>> + *
>> + * A non-master fd will fail with EACCESS, as the kernel
>checks for
>> + * master before attempting to do anything else.
>> + */
>> +return drmAuthMagic(fd, 0) == EINVAL;
>What magic value is valid, is a DRM implementation detail, which we
>don't need to depend upon.
>
>Instead we can check for EACCES, since we care if we have permissions
>- aka are we master.
>The function returns a negative errno, so I'd make this a:
>
>return drmAuthMagic(fd, 0) != -EACCES;
>
>If you and Daniel agree, I'll squash this locally and push.

That's a much better idea, thanks!

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 108916] polaris11 d3d9 rendering issue

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=108916

--- Comment #1 from Axel Davy  ---
Hi,

I just wanted to confirm the trace reproduces the issue, but it may not be a
nine issue but a radeonsi llvm issue. The trace has no glitches on llvmpipe.

-- 
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 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105733

--- Comment #59 from dwagner  ---
I don't think your observations indicate a hardware defect.

I have also a reproducible "hysteresis"-effect with regards to my RX460 GPU:
When I experience the bug scenario I reported in
https://bugs.freedesktop.org/show_bug.cgi?id=102322 and then reboot by pressing
the reset-button, the BIOS greeting and the GRUB loader are consistently not
shown (just a black screen, but with the connected TV indicating a valid HDMI
signal), only once Linux sets the console video mode during boot, then the
screen lights up again. If at that point, or at any time thereafter I reboot
either by typing "reboot" or by pressing the RESET button, then the BIOS
greeting and GRUB menu are shown as normal.

I think this is just due to some lack of thorough initialization upon reset,
because if I power down the machine by switching off the power supply, and then
reboot, the BIOS and GRUB menu always come up. Seems to me that pressing the
RESET button just isn't resetting as much as an actual power down does.

-- 
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 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105733

--- Comment #58 from krutoiles...@gmail.com ---
The corruption suggestion is interesting. My RX580 does this and now it
won't even boot on windows anymore, just crashes.

On Wed, Jan 23, 2019, 12:44 PM  l...@protonmail.ch changed bug 105733
> 
> What Removed Added
> CC   l...@protonmail.ch
>
> *Comment # 57  on
> bug 105733  from
> l...@protonmail.ch  *
>
> Created attachment 143206 
>  [details] 
> 
> I get these errors when attempting to boot after a normal GPU hang and KMS
> happens
>
> Recently I've been getting another type of hang somehow. After a normal hang
> happens, where my screen gets garbled output, I can't even get past KMS in the
> next couple of boots. I can fix this by flipping my VBIOS switch, which 
> heavily
> leads me to believe that amdgpu somehow corrupts the GPU's firmware. I have
> attached the error I get when KMS happens at boot, which happens after I get a
> hang while using the system normally. The monitor doesn't display anything 
> when
> this happens, but I can still control caps lock, etc., however I can not shut
> it off normally. I have not tested whether SSH and such still work.
>
> This honestly makes me doubt whether what I am experiencing is the same bug; 
> is
> it simply a faulty GPU? I am using a Sapphire RX 580 4GB, which I bought used
> from a windows user. It *did* work for him, so obviously it isn't entirely
> broken at least.
>
> --
> You are receiving this mail because:
>
>- You are on the CC list for the bug.
>
>

-- 
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 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105733

l...@protonmail.ch changed:

   What|Removed |Added

 CC||l...@protonmail.ch

--- Comment #57 from l...@protonmail.ch ---
Created attachment 143206
  --> https://bugs.freedesktop.org/attachment.cgi?id=143206&action=edit
I get these errors when attempting to boot after a normal GPU hang and KMS
happens

Recently I've been getting another type of hang somehow. After a normal hang
happens, where my screen gets garbled output, I can't even get past KMS in the
next couple of boots. I can fix this by flipping my VBIOS switch, which heavily
leads me to believe that amdgpu somehow corrupts the GPU's firmware. I have
attached the error I get when KMS happens at boot, which happens after I get a
hang while using the system normally. The monitor doesn't display anything when
this happens, but I can still control caps lock, etc., however I can not shut
it off normally. I have not tested whether SSH and such still work.

This honestly makes me doubt whether what I am experiencing is the same bug; is
it simply a faulty GPU? I am using a Sapphire RX 580 4GB, which I bought used
from a windows user. It *did* work for him, so obviously it isn't entirely
broken at least.

-- 
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] staging: android: ion: Allocate from heap ID directly without mask

[Andrew F. Davis]

Previously the heap to allocate from was selected by a mask of allowed
heap types. This may have been done as a primitive form of constraint
solving, the first heap type that matched any set bit of the heap mask
was allocated from, unless that heap was excluded by having its heap
ID bit not set in the separate passed in heap ID mask.

The heap type does not really represent constraints that should be
matched against to begin with. So the patch [0] removed the the heap type
mask matching but unfortunately left the heap ID mask check (possibly by
mistake or to preserve API). Therefor we now only have a mask of heap
IDs, but heap IDs are unique identifiers and have nothing to do with the
underling heap, so mask matching is not useful here. This also limits us
to 32 heaps total in a system.

With the heap query API users can find the right heap based on type or
name themselves then just supply the ID for that heap. Remove heap ID
mask and allow users to specify heap ID directly by its number.

I know this is an ABI break, but we are in staging so lets get this over
with now rather than limit ourselves later.

[0] 2bb9f5034ec7 ("gpu: ion: Remove heapmask from client")

Signed-off-by: Andrew F. Davis 
---

This also means we don't need to store the available heaps in a plist,
we only operation we care about is lookup, so a better data structure
should be chosen at some point, regular list or xarray maybe?

This is base on -next as to be on top of the other taken Ion patches.

 drivers/staging/android/ion/ion.c  | 21 ++---
 drivers/staging/android/uapi/ion.h |  6 ++
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 92c2914239e3..06dd5bb10ecb 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -387,7 +387,7 @@ static const struct dma_buf_ops dma_buf_ops = {
.unmap = ion_dma_buf_kunmap,
 };
 
-static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags)
+static int ion_alloc(size_t len, unsigned int heap_id, unsigned int flags)
 {
struct ion_device *dev = internal_dev;
struct ion_buffer *buffer = NULL;
@@ -396,23 +396,22 @@ static int ion_alloc(size_t len, unsigned int 
heap_id_mask, unsigned int flags)
int fd;
struct dma_buf *dmabuf;
 
-   pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
-len, heap_id_mask, flags);
-   /*
-* traverse the list of heaps available in this system in priority
-* order.  If the heap type is supported by the client, and matches the
-* request of the caller allocate from it.  Repeat until allocate has
-* succeeded or all heaps have been tried
-*/
+   pr_debug("%s: len %zu heap_id %u flags %x\n", __func__,
+len, heap_id, flags);
+
len = PAGE_ALIGN(len);
 
if (!len)
return -EINVAL;
 
+   /*
+* Traverse the list of heaps available in this system.  If the
+* heap id matches the request of the caller allocate from it.
+*/
down_read(&dev->lock);
plist_for_each_entry(heap, &dev->heaps, node) {
/* if the caller didn't specify this heap id */
-   if (!((1 << heap->id) & heap_id_mask))
+   if (heap->id != heap_id)
continue;
buffer = ion_buffer_create(heap, dev, len, flags);
if (!IS_ERR(buffer))
@@ -541,7 +540,7 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, 
unsigned long arg)
int fd;
 
fd = ion_alloc(data.allocation.len,
-  data.allocation.heap_id_mask,
+  data.allocation.heap_id,
   data.allocation.flags);
if (fd < 0)
return fd;
diff --git a/drivers/staging/android/uapi/ion.h 
b/drivers/staging/android/uapi/ion.h
index 5d7009884c13..6a78a1e23251 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -35,8 +35,6 @@ enum ion_heap_type {
   */
 };
 
-#define ION_NUM_HEAP_IDS   (sizeof(unsigned int) * 8)
-
 /**
  * allocation flags - the lower 16 bits are used by core ion, the upper 16
  * bits are reserved for use by the heaps themselves.
@@ -59,7 +57,7 @@ enum ion_heap_type {
 /**
  * struct ion_allocation_data - metadata passed from userspace for allocations
  * @len:   size of the allocation
- * @heap_id_mask:  mask of heap ids to allocate from
+ * @heap_id:   heap id to allocate from
  * @flags: flags passed to heap
  * @handle:pointer that will be populated with a cookie to use to
  * refer to this allocation
@@ -68,7 +66,7 @@ enum ion_heap_type {
  */
 struct ion_allocation_data {
__u64 len;
-   __u32 h

[Bug 108031] [Regression] amdgpu_device_ip_init failed on Bonaire XT

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=108031

--- Comment #4 from Alex Deucher  ---
(In reply to Alex Deucher from comment #3)
> Remove amdgpu.dpm=1 from your kernel command line.  The default dpm
> implementation changed in 4.19.

and setting dpm=1 overrides that resulting in different behavior.  power
management is always enabled by default.  the dpm option is only for debugging
and switching between implementations for asics that contain multiple
implementations.

-- 
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 108031] [Regression] amdgpu_device_ip_init failed on Bonaire XT

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=108031

--- Comment #3 from Alex Deucher  ---
Remove amdgpu.dpm=1 from your kernel command line.  The default dpm
implementation changed in 4.19.

-- 
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 v3 2/4] drm/vc4: Report underrun errors

[Eric Anholt]

Paul Kocialkowski  writes:

> From: Boris Brezillon 
>
> The DRM framework provides a generic way to report underrun errors.
> Let's implement the necessary hooks to support it in the VC4 driver.
>
> Signed-off-by: Boris Brezillon 
> ---
> Changes in v3:
> - Generic underrun report function has been dropped, adjust the
>   code accordingly

Update commit message for DRM framework not providing a generic way any
more?

>
> Changes in v2:
> - New patch
> ---
>  drivers/gpu/drm/vc4/vc4_debugfs.c |  1 +
>  drivers/gpu/drm/vc4/vc4_drv.h | 10 
>  drivers/gpu/drm/vc4/vc4_hvs.c | 87 +++
>  drivers/gpu/drm/vc4/vc4_kms.c |  4 ++
>  drivers/gpu/drm/vc4/vc4_regs.h| 46 
>  5 files changed, 113 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c 
> b/drivers/gpu/drm/vc4/vc4_debugfs.c
> index 7a0003de71ab..64021bcba041 100644
> --- a/drivers/gpu/drm/vc4/vc4_debugfs.c
> +++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
> @@ -23,6 +23,7 @@ static const struct drm_info_list vc4_debugfs_list[] = {
>   {"vec_regs", vc4_vec_debugfs_regs, 0},
>   {"txp_regs", vc4_txp_debugfs_regs, 0},
>   {"hvs_regs", vc4_hvs_debugfs_regs, 0},
> + {"hvs_underrun",  vc4_hvs_debugfs_underrun, 0},
>   {"crtc0_regs", vc4_crtc_debugfs_regs, 0, (void *)(uintptr_t)0},
>   {"crtc1_regs", vc4_crtc_debugfs_regs, 0, (void *)(uintptr_t)1},
>   {"crtc2_regs", vc4_crtc_debugfs_regs, 0, (void *)(uintptr_t)2},
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 955f157f5ad0..619fb8bec428 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -184,6 +184,13 @@ struct vc4_dev {
>   /* Bitmask of the current bin_alloc used for overflow memory. */
>   uint32_t bin_alloc_overflow;
>  
> + /* Incremented when an underrun error happened after an atomic commit.
> +  * This is particularly useful to detect when a specific modeset is too
> +  * demanding in term of memory or HVS bandwidth which is hard to guess
> +  * at atomic check time.
> +  */
> + atomic_t underrun;
> +
>   struct work_struct overflow_mem_work;
>  
>   int power_refcount;
> @@ -773,6 +780,9 @@ extern struct platform_driver vc4_hvs_driver;
>  void vc4_hvs_dump_state(struct drm_device *dev);
>  int vc4_hvs_debugfs_regs(struct seq_file *m, void *unused);
>  void vc4_hvs_sync_dlist(struct drm_device *dev);
> +int vc4_hvs_debugfs_underrun(struct seq_file *m, void *unused);
> +void vc4_hvs_unmask_underrun(struct drm_device *dev);
> +void vc4_hvs_mask_underrun(struct drm_device *dev);
>  
>  /* vc4_kms.c */
>  int vc4_kms_load(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index 1ba60b8e0c2d..3095fad2ff8b 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> @@ -22,6 +22,7 @@
>   * each CRTC.
>   */
>  
> +#include 
>  #include 
>  #include "vc4_drv.h"
>  #include "vc4_regs.h"
> @@ -102,6 +103,18 @@ int vc4_hvs_debugfs_regs(struct seq_file *m, void 
> *unused)
>  
>   return 0;
>  }
> +
> +int vc4_hvs_debugfs_underrun(struct seq_file *m, void *data)
> +{
> + struct drm_info_node *node = m->private;
> + struct drm_device *dev = node->minor->dev;
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> + struct drm_printer p = drm_seq_file_printer(m);
> +
> + drm_printf(&p, "%d\n", atomic_read(&vc4->underrun));
> +
> + return 0;
> +}
>  #endif
>  
>  /* The filter kernel is composed of dwords each containing 3 9-bit
> @@ -183,6 +196,62 @@ void vc4_hvs_sync_dlist(struct drm_device *dev)
>   }
>  }
>  
> +void vc4_hvs_mask_underrun(struct drm_device *dev)
> +{
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> + u32 dispctrl = HVS_READ(SCALER_DISPCTRL);
> +
> + dispctrl &= ~(SCALER_DISPCTRL_DSPEISLUR(0) |
> +   SCALER_DISPCTRL_DSPEISLUR(1) |
> +   SCALER_DISPCTRL_DSPEISLUR(2));
> +
> + HVS_WRITE(SCALER_DISPCTRL, dispctrl);
> +}
> +
> +void vc4_hvs_unmask_underrun(struct drm_device *dev)
> +{
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> + u32 dispctrl = HVS_READ(SCALER_DISPCTRL);
> +
> + dispctrl |= SCALER_DISPCTRL_DSPEISLUR(0) |
> + SCALER_DISPCTRL_DSPEISLUR(1) |
> + SCALER_DISPCTRL_DSPEISLUR(2);
> +
> + HVS_WRITE(SCALER_DISPSTAT,
> +   SCALER_DISPSTAT_EUFLOW(0) |
> +   SCALER_DISPSTAT_EUFLOW(1) |
> +   SCALER_DISPSTAT_EUFLOW(2));
> + HVS_WRITE(SCALER_DISPCTRL, dispctrl);
> +}
> +
> +static void vc4_hvs_report_underrun(struct drm_device *dev)
> +{
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> +
> + atomic_inc(&vc4->underrun);
> + DRM_DEV_ERROR(dev->dev, "HVS underrun\n");
> +}
> +
> +static irqreturn_t vc4_hvs_irq_handler(int irq, void *data)
> +{
> + struct drm_device *dev = data;
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> + u32 status;

Re: [PATCH v3 1/4] drm/vc4: Wait for display list synchronization when completing commit

[Eric Anholt]

Paul Kocialkowski  writes:

> During an atomic commit, the HVS is configured with a display list
> for the channel matching the associated CRTC. The Pixel Valve (CRTC)
> and encoder are also configured for the new setup at that time.
> While the Pixel Valve and encoder are reconfigured synchronously, the
> HVS is only reconfigured after the display list address (DISPLIST) has
> been updated to the current display list address (DISPLACTX), which is
> the responsibility of the hardware.
>
> The time frame during which the HVS is still running on its previous
> configuration but the CRTC and encoder have been reconfigured already
> can lead to a number of synchronization issues. They will eventually
> cause errors reported on the FIFOs, such as underruns.
>
> With underrun detection enabled (from Boris Brezillon's series), this
> leads to unreliable underrun detection with random false positives.
>
> To ensure a coherent state, wait for each enabled channel of the HVS
> to synchronize its current display list address. This fixes the issue
> of random underrun reporting on commits.
>
> Signed-off-by: Paul Kocialkowski 
> ---
>  drivers/gpu/drm/vc4/vc4_drv.h  |  1 +
>  drivers/gpu/drm/vc4/vc4_hvs.c  | 17 +
>  drivers/gpu/drm/vc4/vc4_kms.c  |  2 ++
>  drivers/gpu/drm/vc4/vc4_regs.h |  2 ++
>  4 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index c24b078f0593..955f157f5ad0 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -772,6 +772,7 @@ void vc4_irq_reset(struct drm_device *dev);
>  extern struct platform_driver vc4_hvs_driver;
>  void vc4_hvs_dump_state(struct drm_device *dev);
>  int vc4_hvs_debugfs_regs(struct seq_file *m, void *unused);
> +void vc4_hvs_sync_dlist(struct drm_device *dev);
>  
>  /* vc4_kms.c */
>  int vc4_kms_load(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index 5d8c749c9749..1ba60b8e0c2d 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> @@ -166,6 +166,23 @@ static int vc4_hvs_upload_linear_kernel(struct vc4_hvs 
> *hvs,
>   return 0;
>  }
>  
> +void vc4_hvs_sync_dlist(struct drm_device *dev)
> +{
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < SCALER_CHANNELS_COUNT; i++) {
> + if (!(HVS_READ(SCALER_DISPCTRLX(i)) & SCALER_DISPCTRLX_ENABLE))
> + continue;
> +
> + ret = wait_for(HVS_READ(SCALER_DISPLACTX(i)) ==
> +HVS_READ(SCALER_DISPLISTX(i)), 1000);
> + WARN(ret, "Timeout waiting for channel %d display list sync\n",
> +  i);
> + }
> +}
> +
>  static int vc4_hvs_bind(struct device *dev, struct device *master, void 
> *data)
>  {
>   struct platform_device *pdev = to_platform_device(dev);
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index 0490edb192a1..2d66a2b57a91 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -155,6 +155,8 @@ vc4_atomic_complete_commit(struct drm_atomic_state *state)
>  
>   drm_atomic_helper_commit_hw_done(state);
>  
> + vc4_hvs_sync_dlist(dev);
> +
>   drm_atomic_helper_wait_for_flip_done(dev, state);

wait_for_flip_done should already be waiting for DISPLACTX to match our
crtc's display list, though, right (see vc4_crtc_handle_page_flip())?
This seems like a no-op to me.


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] xf86drm: Add drmIsMaster()

[Emil Velikov]

On Wed, 23 Jan 2019 at 04:39, Christopher James Halse Rogers
 wrote:
>
> We can't use drmSetMaster to query whether or not a drm fd is master
> because it requires CAP_SYS_ADMIN, even if the fd *is* a master fd.
>
> Pick DRM_IOCTL_MODE_ATTACHMODE as a long-deprecated ioctl that is
> DRM_MASTER but not DRM_ROOT_ONLY as the probe by which we can detect
> whether or not the fd is master.
>
> This is useful for code that might get master by open()ing the drm device
> while no other master exists, but can't call drmSetMaster itself because
> it's not running as root or is in a container, where container-root isn't
> real-root.
>
> v2: Use the AUTH_MAGIC request rather than MODE_ATTACHMODE, as it's more
> clearly related to master status.
>
> Signed-off-by: Christopher James Halse Rogers 
> 
> ---
>  xf86drm.c | 15 +++
>  xf86drm.h |  2 ++
>  2 files changed, 17 insertions(+)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 10df682b..adee5bd9 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2741,6 +2741,21 @@ drm_public int drmDropMaster(int fd)
>  return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL);
>  }
>
> +drm_public bool drmIsMaster(int fd)
> +{
> +/* Detect master by attempting something that requires master.
> + *
> + * Authenticating magic tokens requires master and 0 is
> + * guaranteed to be an invalid magic number. Attempting this on
> + * a master fd will fail therefore fail with EINVAL because 0 is
> + * invalid.
> + *
> + * A non-master fd will fail with EACCESS, as the kernel checks for
> + * master before attempting to do anything else.
> + */
> +return drmAuthMagic(fd, 0) == EINVAL;
What magic value is valid, is a DRM implementation detail, which we
don't need to depend upon.

Instead we can check for EACCES, since we care if we have permissions
- aka are we master.
The function returns a negative errno, so I'd make this a:

return drmAuthMagic(fd, 0) != -EACCES;

If you and Daniel agree, I'll squash this locally and push.

-Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vkms: Fix flush_work() without INIT_WORK().

[Shayenne Moura]

> syzbot is hitting a lockdep warning [1] because flush_work() is called
> without INIT_WORK() after kzalloc() at vkms_atomic_crtc_reset().
>
> Commit 6c234fe37c57627a ("drm/vkms: Implement CRC debugfs API") added
> INIT_WORK() to only vkms_atomic_crtc_duplicate_state() side. Assuming
> that lifecycle of crc_work is appropriately managed, fix this problem
> by adding INIT_WORK() to vkms_atomic_crtc_reset() side.
>
> [1] 
> https://syzkaller.appspot.com/bug?id=a5954455fcfa51c29ca2ab55b203076337e1c770
>
> Reported-and-tested-by: syzbot 
> 
> Signed-off-by: Tetsuo Handa 
> ---
>  drivers/gpu/drm/vkms/vkms_crtc.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c 
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 177bbcb..3c37d8c 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -104,6 +104,7 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc)
> vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL);
> if (!vkms_state)
> return;
> +   INIT_WORK(&vkms_state->crc_work, vkms_crc_work_handle);
>
> crtc->state = &vkms_state->base;
> crtc->state->crtc = crtc;
> --
> 1.8.3.1
>

Thank you for your patch!

Reviewed-by: Shayenne Moura 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap

[Brian Starkey]

Hi Andrew,

On Wed, Jan 23, 2019 at 10:51:24AM -0600, Andrew F. Davis wrote:
> On 1/22/19 11:33 AM, Sumit Semwal wrote:
> > Hello everyone,
> > 
> > Sincere apologies for chiming in a bit late here, but was off due to
> > some health issues.
> > 
> 
> Hope you are feeling better friend :)
> 
> Looks like this email was a bit broken and you replied again, the
> responses are a little different in each email, so I'd like to respond
> to bits of both, I'll fix up the formatting.
> 
> > Also, adding Daniel Vetter to the mix, since he has been one of the
> > core guys who shaped up dma-buf as it is today.
> > 
> > On Tue, 22 Jan 2019 at 02:51, Andrew F. Davis  wrote:
> >>
> >> On 1/21/19 5:22 AM, Brian Starkey wrote:
> 
> [snip]
> 
> >>>
> >>> Actually I meant in the kernel, in exporters. I haven't seen anyone
> >>> using the API as it was intended (defer allocation until first map,
> >>> migrate between different attachments, etc.). Mostly, backing storage
> >>> seems to get allocated at the point of export, and device mappings are
> >>> often held persistently (e.g. the DRM prime code maps the buffer at
> >>> import time, and keeps it mapped: drm_gem_prime_import_dev).
> >>>
> >>
> > 
> > So I suppose some clarification on the 'intended use' part of dma-buf
> > about deferred allocation is due, so here it is: (Daniel, please feel
> > free to chime in with your opinion here)
> > 
> >  - dma-buf was of course designed as a framework to help intelligent
> > exporters to defer allocation until first map, and be able to migrate
> > backing storage if required etc. At the same time, it is not a
> > _requirement_ from any exporter, so exporters so far have just used it
> > as a convenient mechanism for zero-copy.
> > - ION is one of the few dma-buf exporters in kernel, which satisfies a
> > certain set of expectations from its users.
> > 
> 
> The issue here is that Ion is blocking the ability to late allocate, it
> expects its heaps to have the memory ready at allocation time. My point
> being if the DMA-BUFs intended design was to allow this then Ion should
> respect that and also allow the same from its heap exporters.
> 
> >> I haven't either, which is a shame as it allows for some really useful
> >> management strategies for shared memory resources. I'm working on one
> >> such case right now, maybe I'll get to be the first to upstream one :)
> >>
> > That will be a really good thing! Though perhaps we ought to think if
> > for what you're trying to do, is ION the right place, or should you
> > have a device-specific exporter, available to users via dma-buf apis?
> > 
> 
> I'm starting to question if Ion is the right place myself..
> 
> At a conceptual level I don't believe userspace should be picking the
> backing memory type. This is because the right type of backing memory
> for a task will change from system to system. The kernel should abstract
> away these hardware differences from userspace as much as it can to
> allow portable code.
> 
> For instance a device may need a contiguous buffer on one system but the
> same device on another may have some IOMMU. So which type of memory do
> we allocate? Same issue for cacheability and other properties.
> 
> What we need is a central allocator with full system knowledge to do the
> choosing for us. It seems many agree with the above and I take
> inspiration from your cenalloc patchset. The thing I'm not sure about is
> letting the device drivers set their constraints, because they also
> don't have the full system integration details. For cases where devices
> are behind an IOMMU it is easy enough for the device to know, but what
> about when we have external MMUs out on the bus for anyone to use (I'm
> guessing you remember TILER..).
> 
> I would propose the central allocator keep per-system knowledge (or
> fetch it from DT, or if this is considered policy then userspace) which
> it can use to directly check the attached devices and pick the right memory.
> 
> Anyway the central system allocator could handle 90% of cases I can
> think of, and this is where Ion comes back in, the other cases would
> still require the program to manually pick the right memory (maybe for
> performance reasons, etc.).
> 
> So my vision is to have Ion as the the main front-end for DMA-BUF
> allocations, and expose the central allocator through it (maybe as a
> default heap type that can be populated on a per-system basis), but also
> have other individual heap types exported for the edge cases where
> manual selection is needed like we do now.
> 
> Hence why Ion should allow direct control of the dma_buf_ops from the
> heaps, so we can build central allocators as Ion heaps.
> 
> If I'm off into the weeds here and you have some other ideas I'm all ears.
> 

This is a topic I've gone around a few times. The crux of it is, as
you know, a central allocator is Really Hard. I don't know what you've
seen/done so far in this area, so please forgive me if this is old hat
to you.

Android's p

Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap

[Andrew F. Davis]

On 1/22/19 9:23 PM, Sumit Semwal wrote:
> Hello everyone,
> 
> (Thanks to Dan for letting me know my last email got corrupted :/ -
> resending it here)
> 

Hmm, this one seems a bit messed up also (Thunderbird doesn't seem to
like it at least).

[snip]

> - from dma-buf PoV, ION is an exporter of dma-buf buffers, for its users
> that have specific requirements.
> 

This is what I'm hoping to change up a little bit, Ion shouldn't be the
exporter, its heaps should be the exporters (manage the dma_buf_ops),
Ion would only do advertising of available heaps and allow allocating
DMA-BUFs from them.

IMO that would clear up the other discussions going on right now about
how Ion should handle different dma-buf syncing tasks, it simply
wouldn't :). Plus Ion core gets slimmed down, maybe even enough for
destaging..

>> I haven't either, which is a shame as it allows for some really useful
>> management strategies for shared memory resources. I'm working on one
>> such case right now, maybe I'll get to be the first to upstream one :)
>>
> Yes, it would, and great that you're looking to be the first one to do it :)
> 
>> > I wasn't aware that CPU access before first device access was
>> > considered an abuse of the API - it seems like a valid thing to want
>> > to do.
>> >
>>
>> That's just it, I don't know if it is an abuse of API, I'm trying to get
>> some clarity on that. If we do want to allow early CPU access then that
>> seems to be in contrast to the idea of deferred allocation until first
>> device map, what is supposed to backing the buffer if no devices have
>> attached or mapped yet? Just some system memory followed by migration on
>> the first attach to the proper backing? Seems too time wasteful to be
>> have a valid use.
>>
>> Maybe it should be up to the exporter if early CPU access is allowed?
>>
>> I'm hoping someone with authority over the DMA-BUF framework can clarify
>> original intentions here.
>>
> 
> I suppose dma-buf as a framework can't know or decide what the exporter
> wants or can do - whether the exporter wants to use it for 'only
> zero-copy', or do some intelligent things behind the scene, I think
> should be best left to the exporter.
> 
> Hope this helps,

Yes, these inputs are very helpful, thanks,
Andrew

> Sumit.
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Split out drm_probe_helper.h

[Sam Ravnborg]

Hi Daniel.

On Thu, Jan 17, 2019 at 10:03:34PM +0100, Daniel Vetter wrote:
> Having the probe helper stuff (which pretty much everyone needs) in
> the drm_crtc_helper.h file (which atomic drivers should never need) is
> confusing. Split them out.
> 
> To make sure I actually achieved the goal here I went through all
> drivers. And indeed, all atomic drivers are now free of
> drm_crtc_helper.h includes.

How are the plans to get this patchset merged?
There were dependencies on onging drmP.h removal in i915 IIRC?
I guess my "Minimize drmP.h dependencies" patch-set also have a role in this.

This does not hold up any work of mine, mainly wanted to make
sure this was not lost between all the other patches.

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap

[Andrew F. Davis]

On 1/22/19 11:33 AM, Sumit Semwal wrote:
> Hello everyone,
> 
> Sincere apologies for chiming in a bit late here, but was off due to
> some health issues.
> 

Hope you are feeling better friend :)

Looks like this email was a bit broken and you replied again, the
responses are a little different in each email, so I'd like to respond
to bits of both, I'll fix up the formatting.

> Also, adding Daniel Vetter to the mix, since he has been one of the
> core guys who shaped up dma-buf as it is today.
> 
> On Tue, 22 Jan 2019 at 02:51, Andrew F. Davis  wrote:
>>
>> On 1/21/19 5:22 AM, Brian Starkey wrote:

[snip]

>>>
>>> Actually I meant in the kernel, in exporters. I haven't seen anyone
>>> using the API as it was intended (defer allocation until first map,
>>> migrate between different attachments, etc.). Mostly, backing storage
>>> seems to get allocated at the point of export, and device mappings are
>>> often held persistently (e.g. the DRM prime code maps the buffer at
>>> import time, and keeps it mapped: drm_gem_prime_import_dev).
>>>
>>
> 
> So I suppose some clarification on the 'intended use' part of dma-buf
> about deferred allocation is due, so here it is: (Daniel, please feel
> free to chime in with your opinion here)
> 
>  - dma-buf was of course designed as a framework to help intelligent
> exporters to defer allocation until first map, and be able to migrate
> backing storage if required etc. At the same time, it is not a
> _requirement_ from any exporter, so exporters so far have just used it
> as a convenient mechanism for zero-copy.
> - ION is one of the few dma-buf exporters in kernel, which satisfies a
> certain set of expectations from its users.
> 

The issue here is that Ion is blocking the ability to late allocate, it
expects its heaps to have the memory ready at allocation time. My point
being if the DMA-BUFs intended design was to allow this then Ion should
respect that and also allow the same from its heap exporters.

>> I haven't either, which is a shame as it allows for some really useful
>> management strategies for shared memory resources. I'm working on one
>> such case right now, maybe I'll get to be the first to upstream one :)
>>
> That will be a really good thing! Though perhaps we ought to think if
> for what you're trying to do, is ION the right place, or should you
> have a device-specific exporter, available to users via dma-buf apis?
> 

I'm starting to question if Ion is the right place myself..

At a conceptual level I don't believe userspace should be picking the
backing memory type. This is because the right type of backing memory
for a task will change from system to system. The kernel should abstract
away these hardware differences from userspace as much as it can to
allow portable code.

For instance a device may need a contiguous buffer on one system but the
same device on another may have some IOMMU. So which type of memory do
we allocate? Same issue for cacheability and other properties.

What we need is a central allocator with full system knowledge to do the
choosing for us. It seems many agree with the above and I take
inspiration from your cenalloc patchset. The thing I'm not sure about is
letting the device drivers set their constraints, because they also
don't have the full system integration details. For cases where devices
are behind an IOMMU it is easy enough for the device to know, but what
about when we have external MMUs out on the bus for anyone to use (I'm
guessing you remember TILER..).

I would propose the central allocator keep per-system knowledge (or
fetch it from DT, or if this is considered policy then userspace) which
it can use to directly check the attached devices and pick the right memory.

Anyway the central system allocator could handle 90% of cases I can
think of, and this is where Ion comes back in, the other cases would
still require the program to manually pick the right memory (maybe for
performance reasons, etc.).

So my vision is to have Ion as the the main front-end for DMA-BUF
allocations, and expose the central allocator through it (maybe as a
default heap type that can be populated on a per-system basis), but also
have other individual heap types exported for the edge cases where
manual selection is needed like we do now.

Hence why Ion should allow direct control of the dma_buf_ops from the
heaps, so we can build central allocators as Ion heaps.

If I'm off into the weeds here and you have some other ideas I'm all ears.

Andrew

>>> I wasn't aware that CPU access before first device access was
>>> considered an abuse of the API - it seems like a valid thing to want
>>> to do.
>>>
>>
>> That's just it, I don't know if it is an abuse of API, I'm trying to get
>> some clarity on that. If we do want to allow early CPU access then that
>> seems to be in contrast to the idea of deferred allocation until first
>> device map, what is supposed to backing the buffer if no devices have
>> attached or mapped yet? Just some system m

Re: [Intel-wired-lan] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jeff Kirsher]

On Wed, 2019-01-23 at 03:03 -0800, Kees Cook wrote:
> Variables declared in a switch statement before any case statements
> cannot be initialized, so move all instances out of the switches.
> After this, future always-initialized stack variables will work
> and not throw warnings like this:
> 
> fs/fcntl.c: In function ‘send_sigio_to_task’:
> fs/fcntl.c:738:13: warning: statement will never be executed [-
> Wswitch-unreachable]
>siginfo_t si;
>  ^~
> 
> Signed-off-by: Kees Cook 

Acked-by: Jeff Kirsher 

For the e1000 changes.

> ---
>  arch/x86/xen/enlighten_pv.c   |  7 ---
>  drivers/char/pcmcia/cm4000_cs.c   |  2 +-
>  drivers/char/ppdev.c  | 20 -
> --
>  drivers/gpu/drm/drm_edid.c|  4 ++--
>  drivers/gpu/drm/i915/intel_display.c  |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c   |  4 ++--
>  drivers/net/ethernet/intel/e1000/e1000_main.c |  3 ++-
>  drivers/tty/n_tty.c   |  3 +--
>  drivers/usb/gadget/udc/net2280.c  |  5 ++---
>  fs/fcntl.c|  3 ++-
>  mm/shmem.c|  5 +++--
>  net/core/skbuff.c |  4 ++--
>  net/ipv6/ip6_gre.c|  4 ++--
>  net/ipv6/ip6_tunnel.c |  4 ++--
>  net/openvswitch/flow_netlink.c|  7 +++
>  security/tomoyo/common.c  |  3 ++-
>  security/tomoyo/condition.c   |  7 ---
>  security/tomoyo/util.c|  4 ++--
>  18 files changed, 45 insertions(+), 46 deletions(-)



signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] iommu/intel: quirk to disable DMAR for QM57 igfx

[Joerg Roedel]

On Wed, Jan 23, 2019 at 05:02:38PM +0200, Joonas Lahtinen wrote:
> We have many reports where just having intel_iommu=on (and using the
> system normally, without any virtualization stuff going on) will cause
> unexplained GPU hangs. For those users, simply switching to
> intel_iommu=igfx_off solves the problems, and the debug often ends
> there.

If you can reproduce problems on your side, then you can try to enable
CONFIG_INTEL_IOMMU_BROKEN_GFX_WA to force the GFX devices into the
identity mapping. We can also add a boot-parameter and workarounds if it
turns out that this is sufficient to make the GFX devices work with
IOMMU enabled.


Regards,

Joerg
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 14/15] drm/i915/tv: Fix >1024 modes on gen3

[Ville Syrjälä]

On Wed, Jan 23, 2019 at 03:49:02PM +0200, Imre Deak wrote:
> On Mon, Nov 12, 2018 at 06:59:59PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > On gen3 we must disable the TV encoder vertical filter for >1024
> > pixel wide sources. Once that's done all we can is try to center
> > the image on the screen. Naturally the TV mode vertical resolution
> > must be equal or larger than the user mode vertical resolution
> > or else we'd have to cut off part of the user mode.
> > 
> > And while we may not be able to respect the user's choice of
> > top and bottom borders exactly (or we'd have to reject he mode
> > most likely), we can try to maintain the relative sizes of the
> > top and bottom border with respect to each orher.
> > 
> > Additionally we must configure the pipe as interlaced if the
> > TV mode is interlaced.
> > 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/intel_tv.c | 100 +---
> >  1 file changed, 92 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_tv.c 
> > b/drivers/gpu/drm/i915/intel_tv.c
> > index 75126fce655d..7099d837e31a 100644
> > --- a/drivers/gpu/drm/i915/intel_tv.c
> > +++ b/drivers/gpu/drm/i915/intel_tv.c
> > @@ -861,6 +861,44 @@ static const struct tv_mode tv_modes[] = {
> > },
> >  };
> >  
> > +struct intel_tv_connector_state {
> > +   struct drm_connector_state base;
> > +
> > +   /*
> > +* May need to override the user margins for
> > +* gen3 >1024 wide source vertical centering.
> > +*/
> > +   struct {
> > +   u16 top, bottom;
> > +   } margins;
> > +
> > +   bool bypass_vfilter;
> > +};
> > +
> > +#define to_intel_tv_connector_state(x) container_of(x, struct 
> > intel_tv_connector_state, base)
> > +
> > +/**
> > + * intel_digital_connector_duplicate_state - duplicate connector state
>   ^intel_tv_connector_duplicate_state
> > + * @connector: digital connector
>   ^tv connector?
> > + *
> > + * Allocates and returns a copy of the connector state (both common and
> > + * digital connector specific) for the specified connector.
> > + *
> > + * Returns: The newly allocated connector state, or NULL on failure.
> > + */
> > +struct drm_connector_state *
> > +intel_tv_connector_duplicate_state(struct drm_connector *connector)
> > +{
> > +   struct intel_tv_connector_state *state;
> > +
> > +   state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL);
> > +   if (!state)
> > +   return NULL;
> > +
> > +   __drm_atomic_helper_connector_duplicate_state(connector, &state->base);
> > +   return &state->base;
> > +}
> 
> You didn't add the corresponding checks for the new
> intel_tv_connector_state fields to intel_tv_atomic_check(). I suppose
> that's ok since something resulting in a change in those will force a
> modeset anyway:

The new fields are not visible to the user so nothing external
will change them. intel_tv_compute_config() (which is executed
after .atomic_check()) will just recompute them based on other
user visible state.

> 
> Reviewed-by: Imre Deak 
> 
> > +
> >  static struct intel_tv *enc_to_tv(struct intel_encoder *encoder)
> >  {
> > return container_of(encoder, struct intel_tv, base);
> > @@ -1129,6 +1167,9 @@ intel_tv_compute_config(struct intel_encoder *encoder,
> > struct intel_crtc_state *pipe_config,
> > struct drm_connector_state *conn_state)
> >  {
> > +   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +   struct intel_tv_connector_state *tv_conn_state =
> > +   to_intel_tv_connector_state(conn_state);
> > const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state);
> > struct drm_display_mode *adjusted_mode =
> > &pipe_config->base.adjusted_mode;
> > @@ -1149,6 +1190,43 @@ intel_tv_compute_config(struct intel_encoder 
> > *encoder,
> > pipe_config->port_clock = tv_mode->clock;
> >  
> > intel_tv_mode_to_mode(adjusted_mode, tv_mode);
> > +   drm_mode_set_crtcinfo(adjusted_mode, 0);
> > +
> > +   if (IS_GEN3(dev_priv) && hdisplay > 1024) {
> > +   int extra, top, bottom;
> > +
> > +   extra = adjusted_mode->crtc_vdisplay - vdisplay;
> > +
> > +   if (extra < 0) {
> > +   DRM_DEBUG_KMS("No vertical scaling for >1024 pixel wide 
> > modes\n");
> > +   return false;
> > +   }
> > +
> > +   /* Need to turn off the vertical filter and center the image */
> > +
> > +   /* Attempt to maintain the relative sizes of the margins */
> > +   top = conn_state->tv.margins.top;
> > +   bottom = conn_state->tv.margins.bottom;
> > +
> > +   if (top + bottom)
> > +   top = extra * top / (top + bottom);
> > +   else
> > +   top = extra / 2;
> > +   bottom = extra - top;
> > +
> > +   tv_conn_state->margins.top = top;
> > +   tv_conn_state->margins.bottom = bottom;

[Bug 108031] [Regression] amdgpu_device_ip_init failed on Bonaire XT

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=108031

--- Comment #2 from Alexander Schlarb  ---
Created attachment 143205
  --> https://bugs.freedesktop.org/attachment.cgi?id=143205&action=edit
4.19.0 Debian kernal boot log (external screen working)

I can reproduce this with

00:01.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI]
Carrizo (rev c4)
03:00.0 Display controller: Advanced Micro Devices, Inc. [AMD/ATI] Bonaire
XT [Radeon R9 M280X] (rev 80)

on a “Lenovo ideapad Y700-ACZ”.

When I boot into the released Linux 4.19.0 from Debian I get a blank screen,
however today I discovered that if you have an external displayed plugged
during boot-up that display will be successfully enabled and you can then log
in as usual. The internal laptop display will not ever be enabled however and
checking under *Systemsettings* → *Monitor Settings* (KDE) will not show it
either. Also the entire system will feel very sluggish (not CPU or disk
related, I checked) and is only usable with lots of patience. On shutdown the
system will then hang (nothing about this in system logs unfortunately).

-- 
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 3/4] drm/sun4i: dsi: Add burst support

[Maxime Ripard]

From: Konstantin Sudakov 

The current driver doesn't support the DSI burst operation mode.

Let's add the needed quirks to make it work.

Signed-off-by: Konstantin Sudakov 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 178 +++---
 1 file changed, 136 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c 
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index e3e4ba90c059..6840b3512a45 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -23,7 +23,9 @@
 #include 
 #include 
 
+#include "sun4i_crtc.h"
 #include "sun4i_drv.h"
+#include "sun4i_tcon.h"
 #include "sun6i_mipi_dsi.h"
 
 #include 
@@ -32,6 +34,8 @@
 #define SUN6I_DSI_CTL_EN   BIT(0)
 
 #define SUN6I_DSI_BASIC_CTL_REG0x00c
+#define SUN6I_DSI_BASIC_CTL_TRAIL_INV(n)   (((n) & 0xf) << 4)
+#define SUN6I_DSI_BASIC_CTL_TRAIL_FILL BIT(3)
 #define SUN6I_DSI_BASIC_CTL_HBP_DISBIT(2)
 #define SUN6I_DSI_BASIC_CTL_HSA_HSE_DISBIT(1)
 #define SUN6I_DSI_BASIC_CTL_VIDEO_BURSTBIT(0)
@@ -152,6 +156,8 @@
 
 #define SUN6I_DSI_CMD_TX_REG(n)(0x300 + (n) * 0x04)
 
+#define SUN6I_DSI_SYNC_POINT   40
+
 enum sun6i_dsi_start_inst {
DSI_START_LPRX,
DSI_START_LPTX,
@@ -365,31 +371,101 @@ static u16 sun6i_dsi_get_video_start_delay(struct 
sun6i_dsi *dsi,
return delay;
 }
 
+static u16 sun6i_dsi_get_line_num(struct sun6i_dsi *dsi,
+ struct drm_display_mode *mode)
+{
+   struct mipi_dsi_device *device = dsi->device;
+   unsigned Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
+
+   return mode->htotal * Bpp / device->lanes;
+}
+
+static u16 sun6i_dsi_get_drq_edge0(struct sun6i_dsi *dsi,
+  struct drm_display_mode *mode,
+  u16 line_num, u16 edge1)
+{
+   u16 edge0 = edge1;
+
+   edge0 += (mode->hdisplay + 40) * SUN6I_DSI_TCON_DIV / 8;
+
+   if (edge0 > line_num)
+   return edge0 - line_num;
+
+   return 1;
+}
+
+static u16 sun6i_dsi_get_drq_edge1(struct sun6i_dsi *dsi,
+  struct drm_display_mode *mode,
+  u16 line_num)
+{
+   struct mipi_dsi_device *device = dsi->device;
+   unsigned Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
+   unsigned hbp = mode->htotal - mode->hsync_end;
+   u16 edge1;
+
+
+   edge1 = SUN6I_DSI_SYNC_POINT;
+   edge1 += mode->hdisplay + hbp + 20;
+   edge1 = edge1 * Bpp / device->lanes;
+
+   if (edge1 > line_num)
+   return line_num;
+
+   return edge1;
+}
+
 static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
  struct drm_display_mode *mode)
 {
struct mipi_dsi_device *device = dsi->device;
-   u32 val = 0;
+   u32 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
+   u32 tcon0_drq = 0;
+
+   if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
+   u16 line_num = sun6i_dsi_get_line_num(dsi, mode);
+   u16 edge0, edge1;
+
+   edge1 = sun6i_dsi_get_drq_edge1(dsi, mode, line_num);
+   edge0 = sun6i_dsi_get_drq_edge0(dsi, mode, line_num, edge1);
 
-   if ((mode->hsync_end - mode->hdisplay) > 20) {
+   regmap_write(dsi->regs, SUN6I_DSI_BURST_DRQ_REG,
+SUN6I_DSI_BURST_DRQ_EDGE0(edge0) |
+SUN6I_DSI_BURST_DRQ_EDGE1(edge1));
+
+   regmap_write(dsi->regs, SUN6I_DSI_BURST_LINE_REG,
+SUN6I_DSI_BURST_LINE_NUM(line_num) |
+
SUN6I_DSI_BURST_LINE_SYNC_POINT(SUN6I_DSI_SYNC_POINT));
+
+   tcon0_drq = SUN6I_DSI_TCON_DRQ_ENABLE_MODE;
+   } else if ((mode->hsync_end - mode->hdisplay) > 20) {
/* Maagic */
u16 drq = (mode->hsync_end - mode->hdisplay) - 20;
-
-   drq *= mipi_dsi_pixel_format_to_bpp(device->format);
+   drq *= bpp;
drq /= 32;
 
-   val = (SUN6I_DSI_TCON_DRQ_ENABLE_MODE |
-  SUN6I_DSI_TCON_DRQ_SET(drq));
+   tcon0_drq = SUN6I_DSI_TCON_DRQ_ENABLE_MODE |
+   SUN6I_DSI_TCON_DRQ_SET(drq);
}
 
-   regmap_write(dsi->regs, SUN6I_DSI_TCON_DRQ_REG, val);
+   regmap_write(dsi->regs, SUN6I_DSI_TCON_DRQ_REG, tcon0_drq);
 }
 
 static void sun6i_dsi_setup_inst_loop(struct sun6i_dsi *dsi,
  struct drm_display_mode *mode)
 {
+   struct mipi_dsi_device *device = dsi->device;
u16 delay = 50 - 1;
 
+   if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
+   delay = (mode->htotal - mode->hdisplay) * 150;
+   delay /= (mode->clock / 1000) * 8;
+   delay -= 5

Re: [Freedreno] [PATCH v4 3/3] drm/msm/dpu: add display port support in DPU

[Sean Paul]

On Mon, Dec 17, 2018 at 02:35:05PM -0800, Jeykumar Sankaran wrote:
> Add display port support in DPU by creating hooks
> for DP encoder enumeration and encoder mode
> initialization.
> 
> This change is based on the SDM845 Display port
> driver changes[1].
> 
> changes in v2:
> - rebase on [2] (Sean Paul)
> - remove unwanted error checks and
>   switch cases (Jordan Crouse)
> changes in v3:
> - add dp support after fixing
>   the current code base for error logging (Sean Paul)
> changes in v4:
> -  avoid duplicate returns (Jordan Crouse)
> -  get rid of duplicate error logs (Jordan Crouse)
> 
> [1] https://lwn.net/Articles/768265/
> [2] https://lkml.org/lkml/2018/11/17/87
> 
> Signed-off-by: Jeykumar Sankaran 
> Reviewed-by: Jordan Crouse 
> Reviewed-by: Sean Paul 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  8 ++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 58 
> +
>  2 files changed, 54 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 0dda4a6..371d17d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -2031,7 +2031,7 @@ static int dpu_encoder_setup_display(struct 
> dpu_encoder_virt *dpu_enc,
>  {
>   int ret = 0;
>   int i = 0;
> - enum dpu_intf_type intf_type;
> + enum dpu_intf_type intf_type = INTF_NONE;
>   struct dpu_enc_phys_init_params phys_params;
>  
>   if (!dpu_enc || !dpu_kms) {
> @@ -2054,9 +2054,9 @@ static int dpu_encoder_setup_display(struct 
> dpu_encoder_virt *dpu_enc,
>   case DRM_MODE_ENCODER_DSI:
>   intf_type = INTF_DSI;
>   break;
> - default:
> - DPU_ERROR_ENC(dpu_enc, "unsupported display interface type\n");
> - return -EINVAL;
> + case DRM_MODE_ENCODER_TMDS:
> + intf_type = INTF_DP;
> + break;
>   }
>  
>   WARN_ON(disp_info->num_of_h_tiles < 1);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 885bf88..62b400c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -439,6 +439,31 @@ static int _dpu_kms_initialize_dsi(struct drm_device 
> *dev,
>   return rc;
>  }
>  
> +static int _dpu_kms_initialize_displayport(struct drm_device *dev,
> + struct msm_drm_private *priv,
> + struct dpu_kms *dpu_kms)
> +{
> + struct drm_encoder *encoder = NULL;
> + int rc;
> +
> + if (!priv->dp)
> + return 0;
> +
> + encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
> + if (IS_ERR(encoder)) {
> + DPU_ERROR("encoder init failed for dsi display\n");
> + return PTR_ERR(encoder);
> + }
> +
> + rc = msm_dp_modeset_init(priv->dp, dev, encoder);
> + if (rc) {
> + DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
> + drm_encoder_cleanup(encoder);
> + }
> +
> + return rc;
> +}
> +
>  /**
>   * _dpu_kms_setup_displays - create encoders, bridges and connectors
>   *   for underlying displays
> @@ -451,12 +476,22 @@ static int _dpu_kms_setup_displays(struct drm_device 
> *dev,
>   struct msm_drm_private *priv,
>   struct dpu_kms *dpu_kms)
>  {
> + int rc;
> +
> + rc = _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
> + if (rc)
> + goto fail;
> +
> + rc = _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
> + if (rc)
> + goto fail;
> +
>   /**
>* Extend this function to initialize other
>* types of displays
>*/
> -
> - return _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
> +fail:

One more thing, remove this label and just return directly above.

> + return rc;

Then this becomes return 0

>  }
>  
>  static void _dpu_kms_drm_obj_destroy(struct dpu_kms *dpu_kms)
> @@ -669,13 +704,20 @@ static void _dpu_kms_set_encoder_mode(struct msm_kms 
> *kms,
>   info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
>   MSM_DISPLAY_CAP_VID_MODE;
>  
> - /* TODO: No support for DSI swap */
> - for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> - if (priv->dsi[i]) {
> - info.h_tile_instance[info.num_of_h_tiles] = i;
> - info.num_of_h_tiles++;
> + switch (info.intf_type) {
> + case DRM_MODE_ENCODER_DSI:
> + /* TODO: No support for DSI swap */
> + for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> + if (priv->dsi[i]) {
> + info.h_tile_instance[info.num_of_h_tiles] = i;
> + info.num_of_h_tiles++;
> + }
>  

[PATCH 0/4] drm/sun4i: dsi: Add burst mode support

[Maxime Ripard]

Hi,

Here is a series implementing the burst mode support for DSI.

It's been tested on an A33 board with the panel supported on the 4th patch,
which should remove all quirks due to a different SoC from the equation.

Let me know what you think,
Maxime

Konstantin Sudakov (2):
  drm/sun4i: dsi: Add burst support
  drm/panel: Add Rondo RB070D30 panel

Maxime Ripard (2):
  drm/sun4i: dsi: Restrict DSI tcon clock divider
  drm/sun4i: dsi: Change the start delay calculation

 drivers/gpu/drm/panel/Kconfig|   9 +-
 drivers/gpu/drm/panel/Makefile   |   1 +-
 drivers/gpu/drm/panel/panel-rondo-rb070d30.c | 258 -
 drivers/gpu/drm/sun4i/sun4i_tcon.c   |   4 +-
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c   | 185 ++
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h   |   2 +-
 6 files changed, 414 insertions(+), 45 deletions(-)
 create mode 100644 drivers/gpu/drm/panel/panel-rondo-rb070d30.c

base-commit: f6028e7e48b0f3865b0837d400ca783d3d200b62
-- 
git-series 0.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] drm/panel: Add Rondo RB070D30 panel

[Maxime Ripard]

From: Konstantin Sudakov 

The Rondo RB070D30 panel is a MIPI-DSI panel based on a Fitipower EK79007
controller and a 1024x600 panel.

Signed-off-by: Konstantin Sudakov 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/panel/Kconfig|   9 +-
 drivers/gpu/drm/panel/Makefile   |   1 +-
 drivers/gpu/drm/panel/panel-rondo-rb070d30.c | 258 -
 3 files changed, 268 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-rondo-rb070d30.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 3f3537719beb..3164fa824a63 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -138,6 +138,15 @@ config DRM_PANEL_RAYDIUM_RM68200
  Say Y here if you want to enable support for Raydium RM68200
  720x1280 DSI video mode panel.
 
+config DRM_PANEL_RONDO_RB070D30
+   tristate "Rondo Electronics RB070D30 panel"
+   depends on OF
+   depends on DRM_MIPI_DSI
+   depends on BACKLIGHT_CLASS_DEVICE
+   help
+ Say Y here if you want to enable support for Rondo Electronics
+ RB070D30 1024x600 DSI panel.
+
 config DRM_PANEL_SAMSUNG_S6D16D0
tristate "Samsung S6D16D0 DSI video mode panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 4396658a7996..4fe4cf1bfdb5 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += 
panel-orisetech-otm8009a.o
 obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += 
panel-panasonic-vvx10f034n00.o
 obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += 
panel-raspberrypi-touchscreen.o
 obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
+obj-$(CONFIG_DRM_PANEL_RONDO_RB070D30) += panel-rondo-rb070d30.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
diff --git a/drivers/gpu/drm/panel/panel-rondo-rb070d30.c 
b/drivers/gpu/drm/panel/panel-rondo-rb070d30.c
new file mode 100644
index ..4f8e63f367b1
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-rondo-rb070d30.c
@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018, Bridge Systems BV
+ * Copyright (C) 2018, Bootlin
+ * Copyright (C) 2017, Free Electrons
+ *
+ * This file based on panel-ilitek-ili9881c.c
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+struct rb070d30_panel {
+   struct drm_panel panel;
+   struct mipi_dsi_device *dsi;
+   struct backlight_device *backlight;
+   struct regulator *supply;
+
+   struct {
+   struct gpio_desc *power;
+   struct gpio_desc *reset;
+   struct gpio_desc *updn;
+   struct gpio_desc *shlr;
+   } gpios;
+};
+
+static inline struct rb070d30_panel *panel_to_rb070d30_panel(struct drm_panel 
*panel)
+{
+   return container_of(panel, struct rb070d30_panel, panel);
+}
+
+static int rb070d30_panel_prepare(struct drm_panel *panel)
+{
+   struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
+   int ret;
+
+   ret = regulator_enable(ctx->supply);
+   if (ret < 0) {
+   dev_err(&ctx->dsi->dev, "Failed to enable supply: %d\n", ret);
+   return ret;
+   }
+
+   /* Reset */
+   msleep(20);
+   gpiod_set_value(ctx->gpios.power, 1);
+   msleep(20);
+   gpiod_set_value(ctx->gpios.reset, 1);
+   msleep(20);
+   return 0;
+}
+
+static int rb070d30_panel_unprepare(struct drm_panel *panel)
+{
+   struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
+
+   gpiod_set_value(ctx->gpios.power, 0);
+   gpiod_set_value(ctx->gpios.reset, 0);
+   regulator_disable(ctx->supply);
+
+   return 0;
+}
+
+static int rb070d30_panel_enable(struct drm_panel *panel)
+{
+   struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
+   int ret;
+
+   ret = mipi_dsi_dcs_exit_sleep_mode(ctx->dsi);
+   if (ret)
+   return ret;
+
+   ret = backlight_enable(ctx->backlight);
+   if (ret)
+   goto out;
+
+   return 0;
+
+out:
+   mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
+   return ret;
+}
+
+static int rb070d30_panel_disable(struct drm_panel *panel)
+{
+   struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
+
+   backlight_disable(ctx->backlight);
+   return mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
+}
+
+/* Default timings */
+static const struct drm_display_mode default_mode = {
+   .clock  = 51206,
+   .hdisplay   = 1024,
+   .hsync_start= 1024 + 160,
+   .hsync_end  = 1024 + 160 + 80,
+   .htotal = 1024 + 160 

[PATCH 2/4] drm/sun4i: dsi: Change the start delay calculation

[Maxime Ripard]

The current calculation for the video start delay in the current DSI driver
is that it is the total vertical size, minus the backporch and sync length,
plus 1.

However, the Allwinner code has it as the active vertical size, plus the
back porch and the sync length. This doesn't make any difference on the
only panel it has been tested with so far, since in that particular case
the front porch is equal to the sum of the back porch and sync length.

This is not the case for all panels, obviously, so we need to fix it. Since
the Allwinner code has a bunch of extra code to deal with out of bounds
values, so let's add them as well.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c 
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 380fc527a707..e3e4ba90c059 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -357,7 +357,12 @@ static void sun6i_dsi_inst_init(struct sun6i_dsi *dsi,
 static u16 sun6i_dsi_get_video_start_delay(struct sun6i_dsi *dsi,
   struct drm_display_mode *mode)
 {
-   return mode->vtotal - (mode->vsync_end - mode->vdisplay) + 1;
+   u16 delay = (mode->vsync_end + 1) % mode->vtotal;
+
+   if (!delay)
+   delay = 1;
+
+   return delay;
 }
 
 static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
-- 
git-series 0.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/sun4i: dsi: Restrict DSI tcon clock divider

[Maxime Ripard]

The current code allows the TCON clock divider to have a range between 4
and 127 when feeding the DSI controller.

The only display supported so far had a display clock rate that ended up
using a divider of 4, but testing with other displays show that only 4
seems to be functional.

This also aligns with what Allwinner is doing in their BSP, so let's just
hardcode that we want a divider of 4 when using the DSI output.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++--
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 0420f5c978b9..bee73ead732a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -341,8 +341,8 @@ static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon 
*tcon,
u32 block_space, start_delay;
u32 tcon_div;
 
-   tcon->dclk_min_div = 4;
-   tcon->dclk_max_div = 127;
+   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
+   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
 
sun4i_tcon0_mode_set_common(tcon, mode);
 
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h 
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index dbbc5b3ecbda..6d4a3c0fd9b5 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -13,6 +13,8 @@
 #include 
 #include 
 
+#define SUN6I_DSI_TCON_DIV 4
+
 struct sun6i_dphy {
struct clk  *bus_clk;
struct clk  *mod_clk;
-- 
git-series 0.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jani Nikula]

On Wed, 23 Jan 2019, Edwin Zimmerman  wrote:
> On Wed, 23 Jan 2019, Jani Nikula  wrote:
>> On Wed, 23 Jan 2019, Greg KH  wrote:
>> > On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
>> >> Variables declared in a switch statement before any case statements
>> >> cannot be initialized, so move all instances out of the switches.
>> >> After this, future always-initialized stack variables will work
>> >> and not throw warnings like this:
>> >>
>> >> fs/fcntl.c: In function ‘send_sigio_to_task’:
>> >> fs/fcntl.c:738:13: warning: statement will never be executed 
>> >> [-Wswitch-unreachable]
>> >>siginfo_t si;
>> >>  ^~
>> >
>> > That's a pain, so this means we can't have any new variables in { }
>> > scope except for at the top of a function?
>> >
>> > That's going to be a hard thing to keep from happening over time, as
>> > this is valid C :(
>> 
>> Not all valid C is meant to be used! ;)
>
> Very true.  The other thing to keep in mind is the burden of enforcing
> a prohibition on a valid C construct like this.  It seems to me that
> patch reviewers and maintainers have enough to do without forcing them
> to watch for variable declarations in switch statements.  Automating
> this prohibition, should it be accepted, seems like a good idea to me.

Considering that the treewide diffstat to fix this is:

 18 files changed, 45 insertions(+), 46 deletions(-)

and using the gcc plugin in question will trigger the switch-unreachable
warning, I think we're good. There'll probably be the occasional
declarations that pass through, and will get fixed afterwards.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] iommu/intel: quirk to disable DMAR for QM57 igfx

[Joonas Lahtinen]

Quoting Joerg Roedel (2019-01-22 18:51:35)
> On Tue, Jan 22, 2019 at 04:48:26PM +0200, Joonas Lahtinen wrote:
> > According to our IOMMU folks there exists some desire to be able to assign
> > the iGFX device aka have intel_iommu=on instead of intel_iommu=igfx_off
> > due to how the devices might be grouped in IOMMU groups. Even when you
> > would not be using the iGFX device.
> 
> You can force the igfx device into a SI domain, or does that also
> trigger the iommu issues on the chipset?

To be honest, we've had a mixture different issues on different SKUs
that have not been hit in the past when intel_iommu was just disabled by
default.

I know that in one group of the problems, the issue has been debugged
into the GPU having its own set of virtualization mapping translation
hardware with caching and it fails to track changes to the mapping. So
if a identity mapping was established and never changed, I'd assume that
to fix at least that class of problems.

Would just passing intel_iommu=on already cause a non-identity mapping to
possibly be used for the integrated GPU? If it did, then it would
explain quite few of the issues.

We have many reports where just having intel_iommu=on (and using the
system normally, without any virtualization stuff going on) will cause
unexplained GPU hangs. For those users, simply switching to
intel_iommu=igfx_off solves the problems, and the debug often ends
there.

Regards, Joonas

> In any case, if iommu=on breaks these systems I want to make them work
> again with opt-out, even at the cost of breaking assignability.
> 
> Regards,
> 
> Joerg
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv2 0/7] drm/bridge: tc358767: small fixes

[A H]

Hi Tomi,

śr., 23 sty 2019, 13:52: Tomi Valkeinen  napisał(a):

> Hi Andrzej,
>
> On 09/01/19 12:12, Andrzej Hajda wrote:
> > On 09.01.2019 10:51, Lucas Stach wrote:
> >> Am Mittwoch, den 09.01.2019, 11:12 +0200 schrieb Tomi Valkeinen:
> >>> Hi Andrzej,
> >>>
> >>> On 09/01/19 10:22, Andrzej Hajda wrote:
>  Hi Tomi,
> 
>  On 03.01.2019 12:59, Tomi Valkeinen wrote:
> > Hi,
> >
> > We have TC358867 on our board, which I believe is almost identical to
> > TC358767. We're using it with a DP connector instead of eDP with a
> fixed
> > panel.
> >
> > I have tested these patches only on TI's 4.14 based kernel, as
> > unfortunately we don't have all the necessary support in mainline
> yet.
> > These patches fix various bugs, but I'm still seeing at least two
> > issues:
> >
> > * Sync with some videomodes is not correct, resulting in a jumping
> and
> >   skewed display
> > * Link training fails sometimes
> >
> > I would appreciate if someone is able to verify these patches with
> > TC358767.
> 
>  Do you want to wait for testers or shall I queue this patchset?
> >>> I haven't heard from anyone, so I'm ok with pushing these.
> >> For the series:
> >>
> >> Tested-by: Lucas Stach 
> >>
> >> on a device with TC358767 and a 4.20 based kernel.
> >
> >
> > Already queued :)
>
> Did you push these somewhere? What's the route for these patches, drm-misc?
>

drm-misc-fixes:

https://github.com/freedesktop/drm-misc/commits/drm-misc-fixes/drivers/gpu/drm/bridge/tc358767.c

Andrzej


>  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
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jani Nikula]

On Wed, 23 Jan 2019, Jani Nikula  wrote:
> On Wed, 23 Jan 2019, Greg KH  wrote:
>> On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
>>> Variables declared in a switch statement before any case statements
>>> cannot be initialized, so move all instances out of the switches.
>>> After this, future always-initialized stack variables will work
>>> and not throw warnings like this:
>>> 
>>> fs/fcntl.c: In function ‘send_sigio_to_task’:
>>> fs/fcntl.c:738:13: warning: statement will never be executed 
>>> [-Wswitch-unreachable]
>>>siginfo_t si;
>>>  ^~
>>
>> That's a pain, so this means we can't have any new variables in { }
>> scope except for at the top of a function?
>>
>> That's going to be a hard thing to keep from happening over time, as
>> this is valid C :(
>
> Not all valid C is meant to be used! ;)
>
> Anyway, I think you're mistaking the limitation to arbitrary blocks
> while it's only about the switch block IIUC.
>
> Can't have:
>
>   switch (i) {
>   int j;
>   case 0:
>   /* ... */
>   }
>
> because it can't be turned into:
>
>   switch (i) {
>   int j = 0; /* not valid C */
>   case 0:
>   /* ... */
>   }
>
> but can have e.g.:
>
>   switch (i) {
>   case 0:
>   {
>   int j = 0;
>   /* ... */
>   }
>   }
>
> I think Kees' approach of moving such variable declarations to the
> enclosing block scope is better than adding another nesting block.

PS. The patch is

Reviewed-by: Jani Nikula 

and the drivers/gpu/drm/i915/* parts are

Acked-by: Jani Nikula 

for merging via whichever tree is appropriate. (There'll be minor
conflicts with in-flight work in our -next tree, but no biggie.)


-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jani Nikula]

On Wed, 23 Jan 2019, Greg KH  wrote:
> On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
>> Variables declared in a switch statement before any case statements
>> cannot be initialized, so move all instances out of the switches.
>> After this, future always-initialized stack variables will work
>> and not throw warnings like this:
>> 
>> fs/fcntl.c: In function ‘send_sigio_to_task’:
>> fs/fcntl.c:738:13: warning: statement will never be executed 
>> [-Wswitch-unreachable]
>>siginfo_t si;
>>  ^~
>
> That's a pain, so this means we can't have any new variables in { }
> scope except for at the top of a function?
>
> That's going to be a hard thing to keep from happening over time, as
> this is valid C :(

Not all valid C is meant to be used! ;)

Anyway, I think you're mistaking the limitation to arbitrary blocks
while it's only about the switch block IIUC.

Can't have:

switch (i) {
int j;
case 0:
/* ... */
}

because it can't be turned into:

switch (i) {
int j = 0; /* not valid C */
case 0:
/* ... */
}

but can have e.g.:

switch (i) {
case 0:
{
int j = 0;
/* ... */
}
}

I think Kees' approach of moving such variable declarations to the
enclosing block scope is better than adding another nesting block.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/5] drm/tegra: vic: Load firmware on demand

[Thierry Reding]

On Wed, Jan 23, 2019 at 03:47:45PM +0300, Dmitry Osipenko wrote:
> 23.01.2019 12:39, Thierry Reding пишет:
> > From: Thierry Reding 
> > 
> > Loading the firmware requires an allocation of IOVA space to make sure
> > that the VIC's Falcon microcontroller can read the firmware if address
> > translation via the SMMU is enabled.
> > 
> > However, the allocation currently happens at a time where the geometry
> > of an IOMMU domain may not have been initialized yet. This happens for
> > example on Tegra186 and later where an ARM SMMU is used. Domains which
> > are created by the ARM SMMU driver postpone the geometry setup until a
> > device is attached to the domain. This is because IOMMU domains aren't
> > attached to a specific IOMMU instance at allocation time and hence the
> > input address space, which defines the geometry, is not known yet.
> > 
> > Work around this by postponing the firmware load until it is needed at
> > the time where a channel is opened to the VIC. At this time the shared
> > IOMMU domain's geometry has been properly initialized.
> > 
> > As a byproduct this allows the Tegra DRM to be created in the absence
> > of VIC firmware, since the VIC initialization no longer fails if the
> > firmware can't be found.
> > 
> > Signed-off-by: Thierry Reding 
> > ---
> >  drivers/gpu/drm/tegra/vic.c | 17 ++---
> >  1 file changed, 10 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
> > index d47983deb1cf..afbdc33f49bc 100644
> > --- a/drivers/gpu/drm/tegra/vic.c
> > +++ b/drivers/gpu/drm/tegra/vic.c
> > @@ -181,13 +181,6 @@ static int vic_init(struct host1x_client *client)
> > vic->domain = tegra->domain;
> > }
> >  
> > -   if (!vic->falcon.data) {
> > -   vic->falcon.data = tegra;
> > -   err = falcon_load_firmware(&vic->falcon);
> > -   if (err < 0)
> > -   goto detach;
> > -   }
> > -
> > vic->channel = host1x_channel_request(client->dev);
> > if (!vic->channel) {
> > err = -ENOMEM;
> > @@ -256,6 +249,16 @@ static int vic_open_channel(struct tegra_drm_client 
> > *client,
> > if (err < 0)
> > return err;
> >  
> > +   if (!vic->falcon.data) {
> > +   vic->falcon.data = client->drm;
> > +
> > +   err = falcon_load_firmware(&vic->falcon);
> > +   if (err < 0) {
> > +   pm_runtime_put(vic->dev);
> > +   return err;
> > +   }
> > +   }
> > +
> > err = vic_boot(vic);
> > if (err < 0) {
> > pm_runtime_put(vic->dev);
> > 
> 
> This only moves the firmware data-copying to a later stage and doesn't
> touch reading out of the firmware file, hence the claim about the
> "byproduct" is invalid. Please take a look at the patch I posted
> sometime ago [0] and feel free to use it as a reference.

You're right, that hunk ended up in some other patch. And indeed this
patch looks pretty much like yours, so I've merged both together (mine
hadn't moved things out to a separate function, so I did that now, and
mine still reuses the client->drm pointer introduced in an earlier patch
to make it easier to pass that around).

Will send out v2 of this patch.

Thierry


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH AUTOSEL 4.20 034/117] drm/amdgpu: Correct get_crtc_scanoutpos behavior when vpos >= vtotal

[Sasha Levin]

On Wed, Jan 09, 2019 at 10:20:49AM +0100, Michel Dänzer wrote:

On 2019-01-08 8:25 p.m., Sasha Levin wrote:

From: Nicholas Kazlauskas 

[ Upstream commit 520f08df45fbe300ed650da786a74093d658b7e1 ]

When variable refresh rate is active [...]


Variable refresh rate (FreeSync) support is only landing in 5.0,
therefore this fix isn't needed in older kernels.


I'll drop it, thank you.

--
Thanks,
Sasha
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] drm/tegra: Restrict IOVA space to DMA mask

[Thierry Reding]

On Wed, Jan 23, 2019 at 04:41:44PM +0300, Dmitry Osipenko wrote:
> 23.01.2019 12:39, Thierry Reding пишет:
> > From: Thierry Reding 
> > 
> > On Tegra186 and later, the ARM SMMU provides an input address space that
> > is 48 bits wide. However, memory clients can only address up to 40 bits.
> > If the geometry is used as-is, allocations of IOVA space can end up in a
> > region that cannot be addressed by the memory clients.
> > 
> > To fix this, restrict the IOVA space to the DMA mask of the host1x
> > device. Note that, technically, the IOVA space needs to be restricted to
> > the intersection of the DMA masks for all clients that are attached to
> > the IOMMU domain. In practice using the DMA mask of the host1x device is
> > sufficient because all host1x clients share the same DMA mask.
> > 
> > Signed-off-by: Thierry Reding 
> > ---
> >  drivers/gpu/drm/tegra/drm.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> > index 271c7a5fc954..0c5f1e6a0446 100644
> > --- a/drivers/gpu/drm/tegra/drm.c
> > +++ b/drivers/gpu/drm/tegra/drm.c
> > @@ -136,11 +136,12 @@ static int tegra_drm_load(struct drm_device *drm, 
> > unsigned long flags)
> >  
> > if (tegra->domain) {
> > u64 carveout_start, carveout_end, gem_start, gem_end;
> > +   u64 dma_mask = dma_get_mask(&device->dev);
> > dma_addr_t start, end;
> > unsigned long order;
> >  
> > -   start = tegra->domain->geometry.aperture_start;
> > -   end = tegra->domain->geometry.aperture_end;
> > +   start = tegra->domain->geometry.aperture_start & dma_mask;
> > +   end = tegra->domain->geometry.aperture_end & dma_mask;
> >  
> > gem_start = start;
> > gem_end = end - CARVEOUT_SZ;
> > 
> 
> Wow, so IOVA could address >32bits on later Tegra's. AFAIK, currently
> there is no support for a proper programming of the 64bit addresses in
> the drivers code, hence.. won't it make sense to force IOVA mask to
> 32bit for now and hope that the second halve of address registers
> happen to be 0x in HW?

I think this restriction only applies to display at this point. In
practice you'd be hard put to trigger that case because IOVA memory is
allocated from the bottom, so you'd actually need to use up to 4 GiB of
IOVA space before hitting that.

That said, I vaguely remember typing up the patch to support writing the
WINBUF_START_ADDR_HI register and friends, but it looks as if that was
never merged.

I'll try to dig out that patch (or rewrite it, shouldn't be very
difficult) and make it part of this series. I'd rather fix that issue
than arbitrarily restrict the IOVA space, because that's likely to come
back and bite us at some point.

Thierry


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 15/15] drm/i915/tv: Filter out >1024 wide modes that would need vertical scaling on gen3

[Imre Deak]

On Mon, Nov 12, 2018 at 07:00:00PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Since gen3 can't handle >1024 wide sources with vertical scaling
> let's not advertize such modes in the mode list. Less tempetation
> to the user to try out things that won't work.
> 
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Imre Deak 

> ---
>  drivers/gpu/drm/i915/intel_tv.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 7099d837e31a..89c537839273 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1738,6 +1738,7 @@ intel_tv_set_mode_type(struct drm_display_mode *mode,
>  static int
>  intel_tv_get_modes(struct drm_connector *connector)
>  {
> + struct drm_i915_private *dev_priv = to_i915(connector->dev);
>   const struct tv_mode *tv_mode = intel_tv_mode_find(connector->state);
>   int i, count = 0;
>  
> @@ -1750,6 +1751,11 @@ intel_tv_get_modes(struct drm_connector *connector)
>   !tv_mode->component_only)
>   continue;
>  
> + /* no vertical scaling with wide sources on gen3 */
> + if (IS_GEN3(dev_priv) && input->w > 1024 &&
> + input->h > intel_tv_mode_vdisplay(tv_mode))
> + continue;
> +
>   mode = drm_mode_create(connector->dev);
>   if (!mode)
>   continue;
> -- 
> 2.18.1
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 14/15] drm/i915/tv: Fix >1024 modes on gen3

[Imre Deak]

On Mon, Nov 12, 2018 at 06:59:59PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> On gen3 we must disable the TV encoder vertical filter for >1024
> pixel wide sources. Once that's done all we can is try to center
> the image on the screen. Naturally the TV mode vertical resolution
> must be equal or larger than the user mode vertical resolution
> or else we'd have to cut off part of the user mode.
> 
> And while we may not be able to respect the user's choice of
> top and bottom borders exactly (or we'd have to reject he mode
> most likely), we can try to maintain the relative sizes of the
> top and bottom border with respect to each orher.
> 
> Additionally we must configure the pipe as interlaced if the
> TV mode is interlaced.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_tv.c | 100 +---
>  1 file changed, 92 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 75126fce655d..7099d837e31a 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -861,6 +861,44 @@ static const struct tv_mode tv_modes[] = {
>   },
>  };
>  
> +struct intel_tv_connector_state {
> + struct drm_connector_state base;
> +
> + /*
> +  * May need to override the user margins for
> +  * gen3 >1024 wide source vertical centering.
> +  */
> + struct {
> + u16 top, bottom;
> + } margins;
> +
> + bool bypass_vfilter;
> +};
> +
> +#define to_intel_tv_connector_state(x) container_of(x, struct 
> intel_tv_connector_state, base)
> +
> +/**
> + * intel_digital_connector_duplicate_state - duplicate connector state
  ^intel_tv_connector_duplicate_state
> + * @connector: digital connector
  ^tv connector?
> + *
> + * Allocates and returns a copy of the connector state (both common and
> + * digital connector specific) for the specified connector.
> + *
> + * Returns: The newly allocated connector state, or NULL on failure.
> + */
> +struct drm_connector_state *
> +intel_tv_connector_duplicate_state(struct drm_connector *connector)
> +{
> + struct intel_tv_connector_state *state;
> +
> + state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL);
> + if (!state)
> + return NULL;
> +
> + __drm_atomic_helper_connector_duplicate_state(connector, &state->base);
> + return &state->base;
> +}

You didn't add the corresponding checks for the new
intel_tv_connector_state fields to intel_tv_atomic_check(). I suppose
that's ok since something resulting in a change in those will force a
modeset anyway:

Reviewed-by: Imre Deak 

> +
>  static struct intel_tv *enc_to_tv(struct intel_encoder *encoder)
>  {
>   return container_of(encoder, struct intel_tv, base);
> @@ -1129,6 +1167,9 @@ intel_tv_compute_config(struct intel_encoder *encoder,
>   struct intel_crtc_state *pipe_config,
>   struct drm_connector_state *conn_state)
>  {
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_tv_connector_state *tv_conn_state =
> + to_intel_tv_connector_state(conn_state);
>   const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state);
>   struct drm_display_mode *adjusted_mode =
>   &pipe_config->base.adjusted_mode;
> @@ -1149,6 +1190,43 @@ intel_tv_compute_config(struct intel_encoder *encoder,
>   pipe_config->port_clock = tv_mode->clock;
>  
>   intel_tv_mode_to_mode(adjusted_mode, tv_mode);
> + drm_mode_set_crtcinfo(adjusted_mode, 0);
> +
> + if (IS_GEN3(dev_priv) && hdisplay > 1024) {
> + int extra, top, bottom;
> +
> + extra = adjusted_mode->crtc_vdisplay - vdisplay;
> +
> + if (extra < 0) {
> + DRM_DEBUG_KMS("No vertical scaling for >1024 pixel wide 
> modes\n");
> + return false;
> + }
> +
> + /* Need to turn off the vertical filter and center the image */
> +
> + /* Attempt to maintain the relative sizes of the margins */
> + top = conn_state->tv.margins.top;
> + bottom = conn_state->tv.margins.bottom;
> +
> + if (top + bottom)
> + top = extra * top / (top + bottom);
> + else
> + top = extra / 2;
> + bottom = extra - top;
> +
> + tv_conn_state->margins.top = top;
> + tv_conn_state->margins.bottom = bottom;
> +
> + tv_conn_state->bypass_vfilter = true;
> +
> + if (!tv_mode->progressive)
> + adjusted_mode->flags |= DRM_MODE_FLAG_INTERLACE;
> + } else {
> + tv_conn_state->margins.top = conn_state->tv.margins.top;
> + tv_conn_state->margins.bottom = conn_state->tv.margins.bottom;
> +
> + tv_conn_state->bypass_vfilter = false;
> + }
>  
>   DRM_DE

[PATCH] drm/tegra: vic: Do not clear driver data

[Thierry Reding]

From: Thierry Reding 

Upon driver failure, the driver core will take care of clearing the
driver data, so there's no need to do so explicitly in the driver.

Signed-off-by: Thierry Reding 
---
 drivers/gpu/drm/tegra/vic.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
index 9cb10d1e923a..d20fcbf6196d 100644
--- a/drivers/gpu/drm/tegra/vic.c
+++ b/drivers/gpu/drm/tegra/vic.c
@@ -412,7 +412,6 @@ static int vic_probe(struct platform_device *pdev)
err = host1x_client_register(&vic->client.base);
if (err < 0) {
dev_err(dev, "failed to register host1x client: %d\n", err);
-   platform_set_drvdata(pdev, NULL);
goto exit_falcon;
}
 
-- 
2.19.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/imx: only send commit done event when all state has been applied

[Daniel Vetter]

On Wed, Jan 23, 2019 at 12:35:02PM +0100, Philipp Zabel wrote:
> On Fri, 2018-10-05 at 17:11 +0200, Philipp Zabel wrote:
> > On Fri, 2018-09-14 at 18:59 +0200, Lucas Stach wrote:
> > > Currently there is a small race window where we could manage to arm the
> > > vblank event from atomic flush, but programming the hardware was too close
> > > to the frame end, so the hardware will only apply the current state on the
> > > next vblank. In this case we will send out the commit done event too early
> > > causing userspace to reuse framebuffes that are still in use.
> > > 
> > > Instead of using the event arming mechnism, just remember the pending 
> > > event
> > > and send it from the vblank IRQ handler, once we are sure that all state
> > > has been applied sucessfully.
> > > 
> > > Signed-off-by: Lucas Stach 
> > > ---
> > >  drivers/gpu/drm/imx/ipuv3-crtc.c | 32 
> > >  1 file changed, 28 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c 
> > > b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > > index 7d4b710b837a..b0c95565a28d 100644
> > > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> > > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > > @@ -42,6 +42,7 @@ struct ipu_crtc {
> > >   struct ipu_dc   *dc;
> > >   struct ipu_di   *di;
> > >   int irq;
> > > + struct drm_pending_vblank_event *event;
> > >  };
> > >  
> > >  static inline struct ipu_crtc *to_ipu_crtc(struct drm_crtc *crtc)
> > > @@ -181,8 +182,31 @@ static const struct drm_crtc_funcs ipu_crtc_funcs = {
> > >  static irqreturn_t ipu_irq_handler(int irq, void *dev_id)
> > >  {
> > >   struct ipu_crtc *ipu_crtc = dev_id;
> > > + struct drm_crtc *crtc = &ipu_crtc->base;
> > > + unsigned long flags;
> > > + int i;
> > > +
> > > + drm_crtc_handle_vblank(crtc);
> > > +
> > > + if (ipu_crtc->event) {
> > > + for (i = 0; i < ARRAY_SIZE(ipu_crtc->plane); i++) {
> > > + struct ipu_plane *plane = ipu_crtc->plane[i];
> > > +
> > > + if (!plane)
> > > + continue;
> > > +
> > > + if (!ipu_plane_atomic_update_done(&plane->base))
> > 
> > if (ipu_plane_atomic_update_pending(&plane->base))
> > 
> > > + break;
> > > + }
> > >  
> > > - drm_crtc_handle_vblank(&ipu_crtc->base);
> > > + if (i == ARRAY_SIZE(ipu_crtc->plane)) {
> > > + spin_lock_irqsave(&crtc->dev->event_lock, flags);
> > > + drm_crtc_send_vblank_event(crtc, ipu_crtc->event);
> > > + ipu_crtc->event = NULL;
> > 
> > These two happen under the event spinlock, but where event is set in
> > ipu_crtc_atomic_flush, the locking is removed.
> > 
> > > + drm_crtc_vblank_put(crtc);
> > > + spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> > > + }
> > > + }
> > >  
> > >   return IRQ_HANDLED;
> > >  }
> > > @@ -229,13 +253,13 @@ static void ipu_crtc_atomic_begin(struct drm_crtc 
> > > *crtc,
> > >  static void ipu_crtc_atomic_flush(struct drm_crtc *crtc,
> > > struct drm_crtc_state *old_crtc_state)
> > >  {
> > > - spin_lock_irq(&crtc->dev->event_lock);
> > > + struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
> > > +
> > >   if (crtc->state->event) {
> > >   WARN_ON(drm_crtc_vblank_get(crtc));
> > > - drm_crtc_arm_vblank_event(crtc, crtc->state->event);
> > > + ipu_crtc->event = crtc->state->event;
> > 
> > We assume here that ipu_crtc->event is NULL and the irq handler is never
> > running at the same time, otherwise we would drop an event. This is non-
> > obvious to me, and I think it warrants a comment.
> > 
> > My understanding is the following:
> > 
> > - It is virtually impossible for atomic_flush to race against a delayed
> >   previous ipu_irq_handler because the previous commit's commit_tail
> >   would still be waiting for the vblank event to release it from
> >   drm_atomic_helper_wait_for_flip_done.
> > 
> >   However, if the last commit's tail finishes after the irq_handler
> >   calls drm_crtc_send_vblank_event(), and the new commit is issued, and
> >   its tail work scheduled, all before the next line in the irq_handler,
> >   ipu_crtc->event = NULL, then the new commit's tail could call
> >   drm_atomic_helper_commit_planes and therefore ipu_crtc_atomic_flush
> >   and ipu_crtc->event would be overwritten.
> > 
> > - It is unproblematic for a delayed atomic_flush to race against the
> >   next ipu_irq_handler because ipu_crtc->event will just not be set
> >   when the irq handler checks it, and the vblank event will be deferred
> >   to the next interrupt.
> 
> How do we proceed with this? Keep the spin lock?

Yeah, standard practice is to protect these things with a spinlock,
usually the drm->event_lock. Then the flip_done wait should make sure
overall ordering is correct, too.

Might be good to improve the kerneldocs that this is

Re: [PATCHv2 0/7] drm/bridge: tc358767: small fixes

[Tomi Valkeinen]

Hi Andrzej,

On 09/01/19 12:12, Andrzej Hajda wrote:
> On 09.01.2019 10:51, Lucas Stach wrote:
>> Am Mittwoch, den 09.01.2019, 11:12 +0200 schrieb Tomi Valkeinen:
>>> Hi Andrzej,
>>>
>>> On 09/01/19 10:22, Andrzej Hajda wrote:
 Hi Tomi,

 On 03.01.2019 12:59, Tomi Valkeinen wrote:
> Hi,
>
> We have TC358867 on our board, which I believe is almost identical to
> TC358767. We're using it with a DP connector instead of eDP with a fixed
> panel.
>
> I have tested these patches only on TI's 4.14 based kernel, as
> unfortunately we don't have all the necessary support in mainline yet.
> These patches fix various bugs, but I'm still seeing at least two
> issues:
>
> * Sync with some videomodes is not correct, resulting in a jumping and
>   skewed display
> * Link training fails sometimes
>
> I would appreciate if someone is able to verify these patches with
> TC358767.

 Do you want to wait for testers or shall I queue this patchset?
>>> I haven't heard from anyone, so I'm ok with pushing these.
>> For the series:
>>
>> Tested-by: Lucas Stach 
>>
>> on a device with TC358767 and a 4.20 based kernel.
> 
> 
> Already queued :)

Did you push these somewhere? What's the route for these patches, drm-misc?

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] treewide: Lift switch variables out of switches

[Greg KH]

On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> Variables declared in a switch statement before any case statements
> cannot be initialized, so move all instances out of the switches.
> After this, future always-initialized stack variables will work
> and not throw warnings like this:
> 
> fs/fcntl.c: In function ‘send_sigio_to_task’:
> fs/fcntl.c:738:13: warning: statement will never be executed 
> [-Wswitch-unreachable]
>siginfo_t si;
>  ^~

That's a pain, so this means we can't have any new variables in { }
scope except for at the top of a function?

That's going to be a hard thing to keep from happening over time, as
this is valid C :(

greg k-h
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/imx: only send commit done event when all state has been applied

[Philipp Zabel]

On Fri, 2018-10-05 at 17:11 +0200, Philipp Zabel wrote:
> On Fri, 2018-09-14 at 18:59 +0200, Lucas Stach wrote:
> > Currently there is a small race window where we could manage to arm the
> > vblank event from atomic flush, but programming the hardware was too close
> > to the frame end, so the hardware will only apply the current state on the
> > next vblank. In this case we will send out the commit done event too early
> > causing userspace to reuse framebuffes that are still in use.
> > 
> > Instead of using the event arming mechnism, just remember the pending event
> > and send it from the vblank IRQ handler, once we are sure that all state
> > has been applied sucessfully.
> > 
> > Signed-off-by: Lucas Stach 
> > ---
> >  drivers/gpu/drm/imx/ipuv3-crtc.c | 32 
> >  1 file changed, 28 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c 
> > b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > index 7d4b710b837a..b0c95565a28d 100644
> > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > @@ -42,6 +42,7 @@ struct ipu_crtc {
> > struct ipu_dc   *dc;
> > struct ipu_di   *di;
> > int irq;
> > +   struct drm_pending_vblank_event *event;
> >  };
> >  
> >  static inline struct ipu_crtc *to_ipu_crtc(struct drm_crtc *crtc)
> > @@ -181,8 +182,31 @@ static const struct drm_crtc_funcs ipu_crtc_funcs = {
> >  static irqreturn_t ipu_irq_handler(int irq, void *dev_id)
> >  {
> > struct ipu_crtc *ipu_crtc = dev_id;
> > +   struct drm_crtc *crtc = &ipu_crtc->base;
> > +   unsigned long flags;
> > +   int i;
> > +
> > +   drm_crtc_handle_vblank(crtc);
> > +
> > +   if (ipu_crtc->event) {
> > +   for (i = 0; i < ARRAY_SIZE(ipu_crtc->plane); i++) {
> > +   struct ipu_plane *plane = ipu_crtc->plane[i];
> > +
> > +   if (!plane)
> > +   continue;
> > +
> > +   if (!ipu_plane_atomic_update_done(&plane->base))
> 
>   if (ipu_plane_atomic_update_pending(&plane->base))
> 
> > +   break;
> > +   }
> >  
> > -   drm_crtc_handle_vblank(&ipu_crtc->base);
> > +   if (i == ARRAY_SIZE(ipu_crtc->plane)) {
> > +   spin_lock_irqsave(&crtc->dev->event_lock, flags);
> > +   drm_crtc_send_vblank_event(crtc, ipu_crtc->event);
> > +   ipu_crtc->event = NULL;
> 
> These two happen under the event spinlock, but where event is set in
> ipu_crtc_atomic_flush, the locking is removed.
> 
> > +   drm_crtc_vblank_put(crtc);
> > +   spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> > +   }
> > +   }
> >  
> > return IRQ_HANDLED;
> >  }
> > @@ -229,13 +253,13 @@ static void ipu_crtc_atomic_begin(struct drm_crtc 
> > *crtc,
> >  static void ipu_crtc_atomic_flush(struct drm_crtc *crtc,
> >   struct drm_crtc_state *old_crtc_state)
> >  {
> > -   spin_lock_irq(&crtc->dev->event_lock);
> > +   struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
> > +
> > if (crtc->state->event) {
> > WARN_ON(drm_crtc_vblank_get(crtc));
> > -   drm_crtc_arm_vblank_event(crtc, crtc->state->event);
> > +   ipu_crtc->event = crtc->state->event;
> 
> We assume here that ipu_crtc->event is NULL and the irq handler is never
> running at the same time, otherwise we would drop an event. This is non-
> obvious to me, and I think it warrants a comment.
> 
> My understanding is the following:
> 
> - It is virtually impossible for atomic_flush to race against a delayed
>   previous ipu_irq_handler because the previous commit's commit_tail
>   would still be waiting for the vblank event to release it from
>   drm_atomic_helper_wait_for_flip_done.
> 
>   However, if the last commit's tail finishes after the irq_handler
>   calls drm_crtc_send_vblank_event(), and the new commit is issued, and
>   its tail work scheduled, all before the next line in the irq_handler,
>   ipu_crtc->event = NULL, then the new commit's tail could call
>   drm_atomic_helper_commit_planes and therefore ipu_crtc_atomic_flush
>   and ipu_crtc->event would be overwritten.
> 
> - It is unproblematic for a delayed atomic_flush to race against the
>   next ipu_irq_handler because ipu_crtc->event will just not be set
>   when the irq handler checks it, and the vblank event will be deferred
>   to the next interrupt.

How do we proceed with this? Keep the spin lock?

regards
Philipp
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] gpu: host1x: Represent host1x bus devices in debugfs

[Thierry Reding]

From: Thierry Reding 

This new debugfs file represents the state of host1x bus devices,
specifying the list of subdevices and marking which ones have
successfully registered.

Signed-off-by: Thierry Reding 
---
 drivers/gpu/host1x/bus.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
index b4c385d4a6af..103fffc1904b 100644
--- a/drivers/gpu/host1x/bus.c
+++ b/drivers/gpu/host1x/bus.c
@@ -15,8 +15,10 @@
  * along with this program.  If not, see .
  */
 
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -500,6 +502,36 @@ static void host1x_detach_driver(struct host1x *host1x,
mutex_unlock(&host1x->devices_lock);
 }
 
+static int host1x_devices_show(struct seq_file *s, void *data)
+{
+   struct host1x *host1x = s->private;
+   struct host1x_device *device;
+
+   mutex_lock(&host1x->devices_lock);
+
+   list_for_each_entry(device, &host1x->devices, list) {
+   struct host1x_subdev *subdev;
+
+   seq_printf(s, "%s\n", dev_name(&device->dev));
+
+   mutex_lock(&device->subdevs_lock);
+
+   list_for_each_entry(subdev, &device->active, list)
+   seq_printf(s, "  %pOFf: %s\n", subdev->np,
+  dev_name(subdev->client->dev));
+
+   list_for_each_entry(subdev, &device->subdevs, list)
+   seq_printf(s, "  %pOFf:\n", subdev->np);
+
+   mutex_unlock(&device->subdevs_lock);
+   }
+
+   mutex_unlock(&host1x->devices_lock);
+
+   return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(host1x_devices);
+
 /**
  * host1x_register() - register a host1x controller
  * @host1x: host1x controller
@@ -523,6 +555,9 @@ int host1x_register(struct host1x *host1x)
 
mutex_unlock(&drivers_lock);
 
+   debugfs_create_file("devices", S_IRUGO, host1x->debugfs, host1x,
+   &host1x_devices_fops);
+
return 0;
 }
 
-- 
2.19.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 1/2] xf86drm: fallback to MODALIAS for OF less platform devices

[Emil Velikov]

On Wed, 23 Jan 2019 at 11:04, Eric Engestrom  wrote:
>
> On Wednesday, 2019-01-23 10:45:17 +, Emil Velikov wrote:
> > From: Emil Velikov 
> >
> > Some devices can lack OF data or it may not be available in the uevent
> > file. Fallback to the MODALIAS data in those cases.
> >
> > We strip any leading "MODALIAS=.*:" thus the resulting information is
> > compatible with existing code in Mesa.
> >
> > Signed-off-by: Emil Velikov 
> > ---
> >  xf86drm.c | 55 ++-
> >  1 file changed, 42 insertions(+), 13 deletions(-)
> >
> > diff --git a/xf86drm.c b/xf86drm.c
> > index 10df682b..374734eb 100644
> > --- a/xf86drm.c
> > +++ b/xf86drm.c
> > @@ -3511,15 +3511,28 @@ free_device:
> >  static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr 
> > info)
> >  {
> >  #ifdef __linux__
> > -char path[PATH_MAX + 1], *name;
> > +char path[PATH_MAX + 1], *name, *foo;
>
> I assume you didn't mean to send this patch yet? :P
>
Thanks Eric, I intentionally sent it out. Mind was blank thinking for
a reasonable variable name :-\
Suggestions are more than welcome.

For reference with this patch drmdevice and other drmDevice API users list:
 - VGEM, needs "drm/vgem: Fix vgem_init to get drm device available."
- in v5.0 only :'-(
 - etnaviv, after "drm/etnaviv: remove the need for a gpu-subsystem DT
node" landed in v4.17/18

HTH
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 201795] [Regression] Wrong 4k resolution detected with DisplayPort to HDMI adapter on amdgpu

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=201795

tempel.jul...@gmail.com changed:

   What|Removed |Added

 CC||tempel.jul...@gmail.com

--- Comment #13 from tempel.jul...@gmail.com ---
As a workaround, I'd give forcing the correct edid for each display a try:
https://wiki.archlinux.org/index.php/kernel_mode_setting#Forcing_modes_and_EDID
And maybe also edit the edid to offer only one correct native mode you want to
use.
Editing edids on Linux might be painful, so perhaps do it on Windows.

-- 
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

[v1] drm/msm: Remove clock and bandwidth votes in mdss pm suspend

[Jayant Shekhar]

MDSS PM suspend is dependent on runtime suspend for disabling
clocks and removing bandwidth votes. In case of pm_suspend
triggered, dpm_prepare hold a refcount on power usage of device
and hence runtime suspend is never triggered during pm_suspend.
As runtime suspend is not triggered, clocks and bandwidth votes
remain. Hence explicitly trigger mdss disable in msm_pm_suspend
to disable clocks and remove the votes.

Signed-off-by: Jayant Shekhar 
---
 drivers/gpu/drm/msm/msm_drv.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 5c60bb3..ffe3a25 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -1068,12 +1068,16 @@ static int msm_pm_suspend(struct device *dev)
 {
struct drm_device *ddev = dev_get_drvdata(dev);
struct msm_drm_private *priv = ddev->dev_private;
+   struct msm_mdss *mdss = priv->mdss;
 
if (!IS_ERR_OR_NULL(priv->pm_state))
return 0;
 
priv->pm_state = drm_atomic_helper_suspend(ddev);
 
+   if (mdss && mdss->funcs)
+   mdss->funcs->disable(mdss);
+
return IS_ERR(priv->pm_state) ? PTR_ERR(priv->pm_state) : 0;
 }
 
@@ -1081,11 +1085,15 @@ static int msm_pm_resume(struct device *dev)
 {
struct drm_device *ddev = dev_get_drvdata(dev);
struct msm_drm_private *priv = ddev->dev_private;
+   struct msm_mdss *mdss = priv->mdss;
int ret;
 
if (IS_ERR_OR_NULL(priv->pm_state))
return 0;
 
+   if (mdss && mdss->funcs)
+   mdss->funcs->enable(mdss);
+
ret = drm_atomic_helper_resume(ddev, priv->pm_state);
if (ret == 0)
priv->pm_state = NULL;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] video/fbdev: refactor video= cmdline parsing

[Jani Nikula]

On Wed, 23 Jan 2019, Daniel Vetter  wrote:
> On Wed, Jan 23, 2019 at 11:38:17AM +0200, Jani Nikula wrote:
>> Make the video_setup() function slightly easier to read by removing the
>> repeated checks for !global. Remove the misleading return value comment
>> while at it.
>> 
>> I'm slightly hesitant to change any of this, but here goes anyway, with
>> hopes that the next person to have to look at this has it a wee bit
>> easier.
>> 
>> Signed-off-by: Jani Nikula 
>
> Reviewed-by: Daniel Vetter 

Thanks.

Just to be clear, I expect Bartlomiej to queue this via the fb tree
(provided he agrees with the change, of course).

BR,
Jani.


>
>> ---
>>  drivers/video/fbdev/core/fb_cmdline.c | 23 ++-
>>  1 file changed, 10 insertions(+), 13 deletions(-)
>> 
>> diff --git a/drivers/video/fbdev/core/fb_cmdline.c 
>> b/drivers/video/fbdev/core/fb_cmdline.c
>> index 39509ccd92f1..3b5bd666b952 100644
>> --- a/drivers/video/fbdev/core/fb_cmdline.c
>> +++ b/drivers/video/fbdev/core/fb_cmdline.c
>> @@ -75,36 +75,33 @@ EXPORT_SYMBOL(fb_get_options);
>>   *  NOTE: This function is a __setup and __init function.
>>   *It only stores the options.  Drivers have to call
>>   *fb_get_options() as necessary.
>> - *
>> - *  Returns zero.
>> - *
>>   */
>>  static int __init video_setup(char *options)
>>  {
>> -int i, global = 0;
>> -
>>  if (!options || !*options)
>> -global = 1;
>> +goto out;
>>  
>> -if (!global && !strncmp(options, "ofonly", 6)) {
>> +if (!strncmp(options, "ofonly", 6)) {
>>  ofonly = 1;
>> -global = 1;
>> +goto out;
>>  }
>>  
>> -if (!global && !strchr(options, ':')) {
>> -fb_mode_option = options;
>> -global = 1;
>> -}
>> +if (strchr(options, ':')) {
>> +/* named */
>> +int i;
>>  
>> -if (!global) {
>>  for (i = 0; i < FB_MAX; i++) {
>>  if (video_options[i] == NULL) {
>>  video_options[i] = options;
>>  break;
>>  }
>>  }
>> +} else {
>> +/* global */
>> +fb_mode_option = options;
>>  }
>>  
>> +out:
>>  return 1;
>>  }
>>  __setup("video=", video_setup);
>> -- 
>> 2.20.1
>> 
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 1/2] xf86drm: fallback to MODALIAS for OF less platform devices

[Eric Engestrom]

On Wednesday, 2019-01-23 10:45:17 +, Emil Velikov wrote:
> From: Emil Velikov 
> 
> Some devices can lack OF data or it may not be available in the uevent
> file. Fallback to the MODALIAS data in those cases.
> 
> We strip any leading "MODALIAS=.*:" thus the resulting information is
> compatible with existing code in Mesa.
> 
> Signed-off-by: Emil Velikov 
> ---
>  xf86drm.c | 55 ++-
>  1 file changed, 42 insertions(+), 13 deletions(-)
> 
> diff --git a/xf86drm.c b/xf86drm.c
> index 10df682b..374734eb 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -3511,15 +3511,28 @@ free_device:
>  static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr 
> info)
>  {
>  #ifdef __linux__
> -char path[PATH_MAX + 1], *name;
> +char path[PATH_MAX + 1], *name, *foo;

I assume you didn't mean to send this patch yet? :P

>  
>  snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
>  
>  name = sysfs_uevent_get(path, "OF_FULLNAME");
> -if (!name)
> -return -ENOENT;
> +foo = name;
> +if (!name) {
> +/* If the device lacks OF data, pick the MODALIAS info */
> +name = sysfs_uevent_get(path, "MODALIAS");
> +if (!name)
> +return -ENOENT;
> +
> +/* .. and strip the MODALIAS=[platform,usb...]: part. */
> +foo = strrchr(name, ':');
> +if (!foo) {
> +free(name);
> +return -ENOENT;
> +}
> +foo++;
> +}
>  
> -strncpy(info->fullname, name, DRM_PLATFORM_DEVICE_NAME_LEN);
> +strncpy(info->fullname, foo, DRM_PLATFORM_DEVICE_NAME_LEN);
>  info->fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0';
>  free(name);
>  
> @@ -3534,18 +3547,20 @@ static int drmParsePlatformDeviceInfo(int maj, int 
> min,
>drmPlatformDeviceInfoPtr info)
>  {
>  #ifdef __linux__
> -char path[PATH_MAX + 1], *value;
> +char path[PATH_MAX + 1], *value, *foo;
>  unsigned int count, i;
>  int err;
>  
>  snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
>  
>  value = sysfs_uevent_get(path, "OF_COMPATIBLE_N");
> -if (!value)
> -return -ENOENT;
> -
> -sscanf(value, "%u", &count);
> -free(value);
> +if (value) {
> +sscanf(value, "%u", &count);
> +free(value);
> +} else {
> +/* Assume one entry if the device lack OF data */
> +count = 1;
> +}
>  
>  info->compatible = calloc(count + 1, sizeof(*info->compatible));
>  if (!info->compatible)
> @@ -3553,12 +3568,26 @@ static int drmParsePlatformDeviceInfo(int maj, int 
> min,
>  
>  for (i = 0; i < count; i++) {
>  value = sysfs_uevent_get(path, "OF_COMPATIBLE_%u", i);
> +foo = value;
>  if (!value) {
> -err = -ENOENT;
> -goto free;
> +/* If the device lacks OF data, pick the MODALIAS info */
> +value = sysfs_uevent_get(path, "MODALIAS");
> +if (!value) {
> +err = -ENOENT;
> +goto free;
> +}
> +
> +/* .. and strip the MODALIAS=[platform,usb...]: part. */
> +foo = strrchr(value, ':');
> +if (!foo) {
> +free(value);
> +return -ENOENT;
> +}
> +foo = strdup(foo + 1);
> +free(value);
>  }
>  
> -info->compatible[i] = value;
> +info->compatible[i] = foo;
>  }
>  
>  return 0;
> -- 
> 2.20.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PULL] drm-misc-next

[Maxime Ripard]

Hi Dave, Daniel,

Here is the PR for drm-misc-next for this week.

Let me know if there's anything wrong.

Thanks!
Maxime

drm-misc-next-2019-01-23:
drm-misc-next for 5.1:

UAPI Changes:
 - Addition of the Allwinner tiled format modifier

Cross-subsystem Changes:

Core Changes:
 - dma-buf documentation improvements
 - Removal of now unused fbdev helpers
 - Addition of new drm fbdev helpers
 - Improvements to tinydrm
 - Addition of new drm_fourcc helpers
 - Impromevents to i2c-over-aux to handle I2C_M_STOP

Driver Changes:
 - Add support for the TI DS90C185 LVDS bridge
 - Improvements to the thc63lvdm83d bridge
 - Improvements to sun4i YUV and scaler support
 - Fix to the powerdown sequence of panel-innolux
The following changes since commit 94520db52fc0e931327bb77fe79a952a0e9dd2b0:

  drm: fix alpha build after drm_util.h change (2019-01-16 11:16:17 +0100)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2019-01-23

for you to fetch changes up to 46f3ceaffa81e846677bca8668e0ad40e643cffd:

  drm/panel: panel-innolux: set display off in innolux_panel_unprepare 
(2019-01-22 16:49:15 -0500)


drm-misc-next for 5.1:

UAPI Changes:
 - Addition of the Allwinner tiled format modifier

Cross-subsystem Changes:

Core Changes:
 - dma-buf documentation improvements
 - Removal of now unused fbdev helpers
 - Addition of new drm fbdev helpers
 - Improvements to tinydrm
 - Addition of new drm_fourcc helpers
 - Impromevents to i2c-over-aux to handle I2C_M_STOP

Driver Changes:
 - Add support for the TI DS90C185 LVDS bridge
 - Improvements to the thc63lvdm83d bridge
 - Improvements to sun4i YUV and scaler support
 - Fix to the powerdown sequence of panel-innolux


Hsin-Yi, Wang (1):
  drm/panel: panel-innolux: set display off in innolux_panel_unprepare

Jani Nikula (1):
  drm/dp: use DRM_DEBUG_DP() instead of drm_dbg for logging

Jonathan Corbet (1):
  dma-buf: Fix kerneldoc comment for struct dma_fence_array

Maxime Ripard (5):
  drm/sun4i: Move access control before setting the register as documented
  drm/sun4i: frontend: Add a quirk structure
  drm/sun4i: Set the coef_rdy bit right after the coef have been set
  drm/sun4i: Make COEF_RDY conditional
  drm/sun4i: frontend: Move the FIR filter phases to our quirks

Noralf Trønnes (6):
  drm/cma-helper: Remove unused fbdev code
  drm/gem-fb-helper: Add drm_gem_fb_create_with_dirty()
  drm/damage-helper: Add drm_atomic_helper_damage_merged()
  drm/tinydrm: Use struct drm_rect
  drm/tinydrm: Use damage helper for dirtyfb
  drm/todo: Tick off some tinydrm entries

Paul Kocialkowski (18):
  drm/fourcc: Add format info helpers for checking YUV planes disposition
  drm/fourcc: Add format info helpers for checking YUV sub-sampling
  drm/sun4i: backend: Use explicit fourcc helpers for packed YUV422 check
  drm/sun4i: frontend: Pass DRM format info to input format helpers
  drm/sun4i: frontend: Determine input format based on colorspace
  drm/sun4i: Move the BT.601 CSC coefficients to the frontend
  drm/sun4i: frontend: Configure and enable YUV to RGB CSC when needed
  drm/sun4i: frontend: Add support for packed YUV422 input formats
  drm/sun4i: frontend: Add support for semi-planar YUV input formats
  drm/sun4i: frontend: Add support for planar YUV input formats
  drm/fourcc: Add definitions for Allwinner vendor and VPU tiled format
  drm/sun4i: frontend: Add support for tiled YUV input mode configuration
  drm/sun4i: Add buffer stride and offset configuration for tiling mode
  drm/sun4i: frontend: Add and use helper for checking tiling support
  drm/sun4i: layer: Add tiled modifier support and helper
  drm/sun4i: drv: Allow framebuffer modifiers in mode config
  drm/sun4i: frontend: Hook-in support for the A10, with specific quirks
  drm/sun4i: frontend: Hook-in support for the A20

Peter Rosin (5):
  dt-bindings: display: bridge: fork out ti, ds90c185 from lvds-transmitter
  dt-bindings: display: bridge: lvds-transmitter: cleanup example
  dt-bindings: display: bridge: thc63lvdm83d: use standard powerdown-gpios
  drm/bridge: lvds-encoder: add dev helper variable in .probe()
  drm/bridge: lvds-encoder: add powerdown-gpios support

Sam Ravnborg (1):
  drm: fix drm_can_sleep() comment

Ville Syrjälä (1):
  drm/dp: Implement I2C_M_STOP for i2c-over-aux

YueHaibing (1):
  drm/stm: ltdc: remove set but not used variable 'src_h'

 .../bindings/display/bridge/lvds-transmitter.txt   |  12 +-
 .../bindings/display/bridge/thine,thc63lvdm83d.txt |   2 +-
 .../bindings/display/bridge/ti,ds90c185.txt|  55 
 Documentation/gpu/todo.rst |  35 --
 drivers/gpu/drm/Kconfig|   4 -
 drivers/gpu/drm/b

Re: [PATCH v2] xf86drm: Add drmIsMaster()

[Daniel Vetter]

On Wed, Jan 23, 2019 at 03:38:45PM +1100, Christopher James Halse Rogers wrote:
> We can't use drmSetMaster to query whether or not a drm fd is master
> because it requires CAP_SYS_ADMIN, even if the fd *is* a master fd.
> 
> Pick DRM_IOCTL_MODE_ATTACHMODE as a long-deprecated ioctl that is
> DRM_MASTER but not DRM_ROOT_ONLY as the probe by which we can detect
> whether or not the fd is master.
> 
> This is useful for code that might get master by open()ing the drm device
> while no other master exists, but can't call drmSetMaster itself because
> it's not running as root or is in a container, where container-root isn't
> real-root.
> 
> v2: Use the AUTH_MAGIC request rather than MODE_ATTACHMODE, as it's more
> clearly related to master status.
> 
> Signed-off-by: Christopher James Halse Rogers 
> 

Reviewed-by: Daniel Vetter 

Can I also motivate you for an igt, to make sure this uapi never breaks
again? I think adding a new subtest to core_auth.c would fit well, which
does:

1. open drm fd, check that his IsMaster function returns true (to avoid
dependencies with unreleased libdrm just make a localdrmIsMaster copy,
we'll collect it eventually).

2. keep the first fd open, open a 2nd fd, check that we're _not_ master
anymore.

3. close both fd.

Also, do you have libdrm commit rights to push this and make a release?

Cheers, Daniel

> ---
>  xf86drm.c | 15 +++
>  xf86drm.h |  2 ++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/xf86drm.c b/xf86drm.c
> index 10df682b..adee5bd9 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2741,6 +2741,21 @@ drm_public int drmDropMaster(int fd)
>  return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL);
>  }
>  
> +drm_public bool drmIsMaster(int fd)
> +{
> +/* Detect master by attempting something that requires master.
> + *
> + * Authenticating magic tokens requires master and 0 is
> + * guaranteed to be an invalid magic number. Attempting this on
> + * a master fd will fail therefore fail with EINVAL because 0 is
> + * invalid.
> + *
> + * A non-master fd will fail with EACCESS, as the kernel checks for
> + * master before attempting to do anything else.
> + */
> +return drmAuthMagic(fd, 0) == EINVAL;
> +}
> +
>  drm_public char *drmGetDeviceNameFromFd(int fd)
>  {
>  char name[128];
> diff --git a/xf86drm.h b/xf86drm.h
> index 7773d71a..9e920db9 100644
> --- a/xf86drm.h
> +++ b/xf86drm.h
> @@ -37,6 +37,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #if defined(__cplusplus)
> @@ -733,6 +734,7 @@ extern void drmMsg(const char *format, ...) 
> DRM_PRINTFLIKE(1, 2);
>  
>  extern int drmSetMaster(int fd);
>  extern int drmDropMaster(int fd);
> +extern bool drmIsMaster(int fd);
>  
>  #define DRM_EVENT_CONTEXT_VERSION 4
>  
> -- 
> 2.19.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] video/fbdev: refactor video= cmdline parsing

[Daniel Vetter]

On Wed, Jan 23, 2019 at 11:38:17AM +0200, Jani Nikula wrote:
> Make the video_setup() function slightly easier to read by removing the
> repeated checks for !global. Remove the misleading return value comment
> while at it.
> 
> I'm slightly hesitant to change any of this, but here goes anyway, with
> hopes that the next person to have to look at this has it a wee bit
> easier.
> 
> Signed-off-by: Jani Nikula 

Reviewed-by: Daniel Vetter 

> ---
>  drivers/video/fbdev/core/fb_cmdline.c | 23 ++-
>  1 file changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/video/fbdev/core/fb_cmdline.c 
> b/drivers/video/fbdev/core/fb_cmdline.c
> index 39509ccd92f1..3b5bd666b952 100644
> --- a/drivers/video/fbdev/core/fb_cmdline.c
> +++ b/drivers/video/fbdev/core/fb_cmdline.c
> @@ -75,36 +75,33 @@ EXPORT_SYMBOL(fb_get_options);
>   *   NOTE: This function is a __setup and __init function.
>   *It only stores the options.  Drivers have to call
>   *fb_get_options() as necessary.
> - *
> - *   Returns zero.
> - *
>   */
>  static int __init video_setup(char *options)
>  {
> - int i, global = 0;
> -
>   if (!options || !*options)
> - global = 1;
> + goto out;
>  
> - if (!global && !strncmp(options, "ofonly", 6)) {
> + if (!strncmp(options, "ofonly", 6)) {
>   ofonly = 1;
> - global = 1;
> + goto out;
>   }
>  
> - if (!global && !strchr(options, ':')) {
> - fb_mode_option = options;
> - global = 1;
> - }
> + if (strchr(options, ':')) {
> + /* named */
> + int i;
>  
> - if (!global) {
>   for (i = 0; i < FB_MAX; i++) {
>   if (video_options[i] == NULL) {
>   video_options[i] = options;
>   break;
>   }
>   }
> + } else {
> + /* global */
> + fb_mode_option = options;
>   }
>  
> +out:
>   return 1;
>  }
>  __setup("video=", video_setup);
> -- 
> 2.20.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/modes: Prevent division by zero htotal

[Daniel Vetter]

On Wed, Jan 23, 2019 at 03:28:59PM +0800, Tina Zhang wrote:
> This patch prevents division by zero htotal.

How did you manage to get here with htotal == 0? This needs backtraces
(or if this is just about static checkers, a mention of that).
-Daniel

> 
> Signed-off-by: Tina Zhang 
> Cc: Adam Jackson 
> Cc: Dave Airlie 
> Cc: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_modes.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index adce9a2..59b92b1 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -751,7 +751,7 @@ int drm_mode_hsync(const struct drm_display_mode *mode)
>   if (mode->hsync)
>   return mode->hsync;
>  
> - if (mode->htotal < 0)
> + if (mode->htotal <= 0)
>   return 0;
>  
>   calc_val = (mode->clock * 1000) / mode->htotal; /* hsync in Hz */
> -- 
> 2.7.4
> 

-- 
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/11] drm: Add devm_drm_dev_init/register

[Noralf Trønnes]

Den 22.01.2019 20.30, skrev Daniel Vetter:
> On Tue, Jan 22, 2019 at 8:07 PM Noralf Trønnes  wrote:
>>
>>
>>
>> Den 22.01.2019 10.32, skrev Daniel Vetter:
>>> On Mon, Jan 21, 2019 at 01:21:46PM +0100, Noralf Trønnes wrote:


 Den 21.01.2019 10.55, skrev Daniel Vetter:
> On Mon, Jan 21, 2019 at 10:10:14AM +0100, Daniel Vetter wrote:
>> On Sun, Jan 20, 2019 at 12:43:08PM +0100, Noralf Trønnes wrote:
>>> This adds resource managed (devres) versions of drm_dev_init() and
>>> drm_dev_register().
>>>
>>> Also added is devm_drm_dev_register_with_fbdev() which sets up generic
>>> fbdev emulation as well.
>>>
>>> devm_drm_dev_register() isn't exported since there are no users.
>>>
>>> Signed-off-by: Noralf Trønnes 
>>

 

>>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>>> index 381581b01d48..12129772be45 100644
>>> --- a/drivers/gpu/drm/drm_drv.c
>>> +++ b/drivers/gpu/drm/drm_drv.c
>>> @@ -36,6 +36,7 @@
>>>
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>
>>>  #include "drm_crtc_internal.h"
>>> @@ -871,6 +872,111 @@ void drm_dev_unregister(struct drm_device *dev)
>>>  }
>>>  EXPORT_SYMBOL(drm_dev_unregister);
>>>
>>> +static void devm_drm_dev_init_release(void *data)
>>> +{
>>> + drm_dev_put(data);
>
> We need drm_dev_unplug() here, or this isn't safe.

 This function is only used to cover the error path if probe fails before
 devm_drm_dev_register() is called. devm_drm_dev_register_release() is
 the one that calls unplug. There are comments about this in the functions.
>>>
>>> I think I get a prize for being ignorant and blind :-/
>>>

>
>>> +}
>>> +
>>> +/**
>>> + * devm_drm_dev_init - Resource managed drm_dev_init()
>>> + * @parent: Parent device object
>>> + * @dev: DRM device
>>> + * @driver: DRM driver
>>> + *
>>> + * Managed drm_dev_init(). The DRM device initialized with this 
>>> function is
>>> + * automatically released on driver detach. You must supply a
>
> I think a bit more clarity here would be good:
>
> "... automatically released on driver unbind by callind drm_dev_unplug()."
>
>>> + * &drm_driver.release callback to control the finalization explicitly.
>
> I think a loud warning for these is in order:
>
> "WARNING:
>
> "In generally it is unsafe to use devm functions for drm structures
> because the lifetimes of &drm_device and the underlying &device do not
> match. This here works because it doesn't immediately free anything, but
> only calls drm_dev_unplug(), which internally decrements the &drm_device
> refcount through drm_dev_put().
>
> "All other drm structures must still be explicitly released in the
> &drm_driver.release callback."
>
> While thinking about this I just realized that with this design we have no
> good place to call drm_atomic_helper_shutdown(). Which we need to, or all
> kinds of things will leak badly (connectors, fb, ...), but there's no
> place to call it:
> - unbind is too early, since we haven't yet called drm_dev_unplug, and the
>   drm_dev_unregister in there must be called _before_ we start to shut
>   down anything.
> - drm_driver.release is way too late.
>
> Ofc for a real hotunplug there's no point in shutting down the hw (it's
> already gone), but for a driver unload/unbind it would be nice if this
> happens automatically and in the right order.
>
> So not sure what to do here really.

 How about this change: (it breaks the rule of pulling helpers into the
 core, so maybe we should put the devm_ functions into the simple KMS
 helper instead?)
>>>
>>> Yeah smells a bit much like midlayer ... What would work is having a pile
>>> more devm_ helper functions, so that we onion-unwrap everything correctly,
>>> and in the right order. So:
>>>
>>> - devm_drm_dev_init (always does a drm_dev_put())
>>>
>>> - devm_drm_poll_enable (shuts down the poll helper with a devm action)
>>>
>>> - devm_drm_mode_config_reset (does an atomic_helper_shutdown() as it's 
>>> cleanup action)
>>>
>>> - devm_drm_dev_register (grabs an additional drm_dev_get() reference so it
>>>   can call drm_dev_unplug() unconditionally).
>>>
>>
>> Beautiful! I really like this, it's very flexible.
>>
>> Where should devm_drm_mode_config_reset() live? It will pull in the
>> atomic helper...
> 
> I think a new drm_devm.c helper would be nice for all this stuff.
> Especially since you can't freely mix devm-based setup/cleanup with
> normal cleanup I think it'd be good to have it all together in one
> place. And perhaps even a code example in the DOC: overview.
> 
>>> We'd need to make sure some of the cleanup actions dtrt when the device is
>>> gone, but I think we can achieve that by liberally spri

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Wed, Jan 23, 2019 at 12:03:40PM +0200, Jani Nikula wrote:
> On Tue, 22 Jan 2019, "Wentland, Harry"  wrote:
> > Would it make sense to append something like ", if such a test can be
> > reasonably made using IGT for the target HW." to make it clear to
> > contributors that in cases like the one discussed this is at the
> > reviewers discretion?
> 
> I think the simplest change would be to say API changes SHOULD have
> driver-agnostic testcases, with the RFC 2119 meaning of SHOULD:
> 
>SHOULD   This word, or the adjective "RECOMMENDED", mean that there
>may exist valid reasons in particular circumstances to ignore a
>particular item, but the full implications must be understood and
>carefully weighed before choosing a different course.
> 
> I.e. s/need/should/. I think it also catches the spirit of the
> discussion here; seems like everyone agrees having tests is a good goal.
> 
> You'll have to allow for reviewer/maintainer/community discretion no
> matter what. Judging by the discussion, CRC based tests don't currently
> meet the driver-agnostic requirement. Playing devil's advocate, you
> could argue any new APIs couldn't be tested with CRC either, even if it
> were the most reasonable approach for i915.

I think I'll combine both for v3, I wanted to do something like that
anyway to address Eric Anholt's similar concern.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Wed, Jan 23, 2019 at 12:11:36PM +0200, Jani Nikula wrote:
> On Wed, 23 Jan 2019, Daniel Stone  wrote:
> > If it helps, maybe we could draw up a list somewhere (README in igt?
> > wiki?) of which tests seem to pass generically across a few drivers,
> > which ones are expected to pass, which ones don't but really should,
> > etc. I'm currently running on imx-drm (complicated by hitting a
> > page-cache BUG!), and a couple of others are getting results from
> > rockchip, vc4, and msm. Those are pretty well supported and actively
> > maintained, so should give us a decent first cut at such a list.
> 
> See tests/intel-ci/README. igt supports test lists, we could maintain
> similar lists for different needs.

Anything with DRIVER_ANY in it should work. I just noticed that we have a
few generic tests with only DRIVER_INTEL|DRIVER_AMDGPU, those should be
DRIVER_ANY.

I think the best way to get there is to integrate vkms into our CI (I
think that's something we can justify, we'll benefit from the better
overall testsuite ecosystem too), then we'd get test results for every
commit&patch. And would have at least some way to make sure the DRIVER_ANY
tests are somewhat reasonable. Quick discussion with Martin and Arek on
irc says this is doable.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Wed, Jan 23, 2019 at 09:40:34AM +, Brian Starkey wrote:
> On Tue, Jan 22, 2019 at 08:19:10PM +0100, Daniel Vetter wrote:
> > On Tue, Jan 22, 2019 at 8:00 PM Wentland, Harry  
> > wrote:
> > > On 2019-01-16 11:39 a.m., Daniel Vetter wrote:
> > > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > > forward a lot:
> > > >
> > > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > > >   and a sysroot build (should address all the build/cross platform
> > > >   concerns raised in the RFC discussions).
> > > >
> > > > - tests reorganized into subdirectories so that the i915-gem tests
> > > >   don't clog the main/shared tests directory anymore
> > > >
> > > > - quite a few more non-intel people contributing/reviewing/committing
> > > >   igt tests patches.
> > > >
> > > > I think this addresses all the concerns raised in the RFC discussions,
> > > > and assuming there's enough Acks and no new issues that pop up, we can
> > > > go ahead with this.
> > > >
> > > > 1: https://patchwork.kernel.org/patch/10648851/
> > > > Cc: Petri Latvala 
> > > > Cc: Arkadiusz Hiler 
> > > > Cc: Liviu Dudau 
> > > > Cc: Sean Paul 
> > > > Cc: Eric Anholt 
> > > > Cc: Alex Deucher 
> > > > Cc: Dave Airlie 
> > > > Signed-off-by: Daniel Vetter 
> > >
> > > I'm all for anything resembling TDD and standardizing on one test 
> > > framework. IGT works quite well for us for testing display stuff. We 
> > > still have a way to go but have been trying to adopt this requirement 
> > > lately anyways for the DC driver. Can't really comment on anything beyond 
> > > display, though, for AMD.
> > >
> > > No matter how much I want this to be mandatory, seeing the discussions 
> > > with ARM and the comment about lack of CRC on Nouveau makes me think that 
> > > we might not be quite ready to go there. Implementing DWB is non-trivial. 
> > > VKMS knows how to compute a CRC from a framebuffer, but that's the 
> > > trivial part. Setting up the HW and SW to do DWB is the hard part.
> > 
> > We also discussed a bit writeback implementations on irc, and it looks
> > like you can't really use writeback to accurately test that your
> > compositing engine is programmed correctly, since on at least vc4,
> > malidp and msm (not yet merged upstream) the writeback engine can't be
> > shared with any other outputs, often it even needs a
> > dedicated/special-purpose CRTC (at least vc4 from what I can tell).
> > That means if you botch your programming and e.g. cause an underrun
> > scanning out continous-update outputs, then the writeback won't show
> > that to you, since it's composited separately. I guess we could teach
> > igt to run these tests on the special crtc->writeback pipeline only,
> > but essentially that's a new testcase, and not really testing the
> > actual display: It tests writeback, not hdmi/dp/panels/whatever real
> > outputs you have.
> 
> That doesn't sound right for mali-dp at least. Our writeback is
> synchronous with the normal output. If the normal output underruns
> then the writeback shows that, and if the writeback overruns, then the
> normal output dies too.
> 
> Our writeback encoder/connector is a "possible_clone" of the normal
> one. So, to integrate it in igt I was intending to set up a cloned
> output on the pipe being tested.

Hm, I grepped for that, didn't find the assignment anywhere. If you can do
possible_clones writeback, then I think doing the writeback-as-fake CRC in
the kernel is probably the best, since it's closest to testing the real
thing. Of course if the writeback connector is already used by igt (for a
writeback test) then the magic "auto" crc tap point would need to fail.
But I don't think that would be a problem for igt, since if you're using
both writeback _and_ crc I have no idea what you're trying to do :-)

I also wouldn't tie this crc writeback into atomic (except with a special
"crc writeback flag" to make sure atomic_check rejects any other use of
the writeback connector while it's used for crc generation), but just a
vblank driven worker that flips between 2 buffers and checksums the other
one.
-Daniel

> 
> Cheers,
> -Brian
> 
> > 
> > I'd say we'll shrug these cases off as "can't be reasonable tested,
> > won't have an igt". First driver team with hw that can be validated
> > gets to fill the gaps :-) In practice still going to be a lot better
> > than no tests at all, just exercising the feature will be useful, and
> > will make it a lot easier for the next team to add the crc based tests
> > on top.
> > 
> > -Daniel
> > --
> > 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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing

[PATCH libdrm 2/2] xf85drm: de-duplicate drmParse{Platform.Host1x}{Bus, Device}Info

[Emil Velikov]

From: Emil Velikov 

The functions are virtually identical, fold them up.

Signed-off-by: Emil Velikov 
---
 xf86drm.c | 98 +--
 1 file changed, 15 insertions(+), 83 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c
index 374734eb..18c9693a 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3508,7 +3508,7 @@ free_device:
 return ret;
 }
 
-static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr 
info)
+static int drmParseOFBusInfo(int maj, int min, char *fullname)
 {
 #ifdef __linux__
 char path[PATH_MAX + 1], *name, *foo;
@@ -3532,19 +3532,18 @@ static int drmParsePlatformBusInfo(int maj, int min, 
drmPlatformBusInfoPtr info)
 foo++;
 }
 
-strncpy(info->fullname, foo, DRM_PLATFORM_DEVICE_NAME_LEN);
-info->fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0';
+strncpy(fullname, foo, DRM_PLATFORM_DEVICE_NAME_LEN);
+fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0';
 free(name);
 
 return 0;
 #else
-#warning "Missing implementation of drmParsePlatformBusInfo"
+#warning "Missing implementation of drmParseOFBusInfo"
 return -EINVAL;
 #endif
 }
 
-static int drmParsePlatformDeviceInfo(int maj, int min,
-  drmPlatformDeviceInfoPtr info)
+static int drmParseOFDeviceInfo(int maj, int min, char ***compatible)
 {
 #ifdef __linux__
 char path[PATH_MAX + 1], *value, *foo;
@@ -3562,8 +3561,8 @@ static int drmParsePlatformDeviceInfo(int maj, int min,
 count = 1;
 }
 
-info->compatible = calloc(count + 1, sizeof(*info->compatible));
-if (!info->compatible)
+*compatible = calloc(count + 1, sizeof(char *));
+if (!*compatible)
 return -ENOMEM;
 
 for (i = 0; i < count; i++) {
@@ -3587,19 +3586,19 @@ static int drmParsePlatformDeviceInfo(int maj, int min,
 free(value);
 }
 
-info->compatible[i] = foo;
+*compatible[i] = foo;
 }
 
 return 0;
 
 free:
 while (i--)
-free(info->compatible[i]);
+free(*compatible[i]);
 
-free(info->compatible);
+free(*compatible);
 return err;
 #else
-#warning "Missing implementation of drmParsePlatformDeviceInfo"
+#warning "Missing implementation of drmParseOFDeviceInfo"
 return -EINVAL;
 #endif
 }
@@ -3622,7 +3621,7 @@ static int drmProcessPlatformDevice(drmDevicePtr *device,
 
 dev->businfo.platform = (drmPlatformBusInfoPtr)ptr;
 
-ret = drmParsePlatformBusInfo(maj, min, dev->businfo.platform);
+ret = drmParseOFBusInfo(maj, min, dev->businfo.platform->fullname);
 if (ret < 0)
 goto free_device;
 
@@ -3630,7 +3629,7 @@ static int drmProcessPlatformDevice(drmDevicePtr *device,
 ptr += sizeof(drmPlatformBusInfo);
 dev->deviceinfo.platform = (drmPlatformDeviceInfoPtr)ptr;
 
-ret = drmParsePlatformDeviceInfo(maj, min, dev->deviceinfo.platform);
+ret = drmParseOFDeviceInfo(maj, min, 
&dev->deviceinfo.platform->compatible);
 if (ret < 0)
 goto free_device;
 }
@@ -3644,73 +3643,6 @@ free_device:
 return ret;
 }
 
-static int drmParseHost1xBusInfo(int maj, int min, drmHost1xBusInfoPtr info)
-{
-#ifdef __linux__
-char path[PATH_MAX + 1], *name;
-
-snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
-
-name = sysfs_uevent_get(path, "OF_FULLNAME");
-if (!name)
-return -ENOENT;
-
-strncpy(info->fullname, name, DRM_HOST1X_DEVICE_NAME_LEN);
-info->fullname[DRM_HOST1X_DEVICE_NAME_LEN - 1] = '\0';
-free(name);
-
-return 0;
-#else
-#warning "Missing implementation of drmParseHost1xBusInfo"
-return -EINVAL;
-#endif
-}
-
-static int drmParseHost1xDeviceInfo(int maj, int min,
-drmHost1xDeviceInfoPtr info)
-{
-#ifdef __linux__
-char path[PATH_MAX + 1], *value;
-unsigned int count, i;
-int err;
-
-snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
-
-value = sysfs_uevent_get(path, "OF_COMPATIBLE_N");
-if (!value)
-return -ENOENT;
-
-sscanf(value, "%u", &count);
-free(value);
-
-info->compatible = calloc(count + 1, sizeof(*info->compatible));
-if (!info->compatible)
-return -ENOMEM;
-
-for (i = 0; i < count; i++) {
-value = sysfs_uevent_get(path, "OF_COMPATIBLE_%u", i);
-if (!value) {
-err = -ENOENT;
-goto free;
-}
-
-info->compatible[i] = value;
-}
-
-return 0;
-
-free:
-while (i--)
-free(info->compatible[i]);
-
-free(info->compatible);
-return err;
-#else
-#warning "Missing implementation of drmParseHost1xDeviceInfo"
-return -EINVAL;
-#endif
-}
-
 static int drmProcessHost1xDevice(drmDevicePtr *device,
   const char *node, int node_type,
   int maj, int min, bool fetch_deviceinfo,
@@ -3729,7 +3661,7 @@ static int drmProcessHost1xDevice(drmDevicePtr *

[PATCH libdrm 1/2] xf86drm: fallback to MODALIAS for OF less platform devices

[Emil Velikov]

From: Emil Velikov 

Some devices can lack OF data or it may not be available in the uevent
file. Fallback to the MODALIAS data in those cases.

We strip any leading "MODALIAS=.*:" thus the resulting information is
compatible with existing code in Mesa.

Signed-off-by: Emil Velikov 
---
 xf86drm.c | 55 ++-
 1 file changed, 42 insertions(+), 13 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c
index 10df682b..374734eb 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3511,15 +3511,28 @@ free_device:
 static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr 
info)
 {
 #ifdef __linux__
-char path[PATH_MAX + 1], *name;
+char path[PATH_MAX + 1], *name, *foo;
 
 snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
 
 name = sysfs_uevent_get(path, "OF_FULLNAME");
-if (!name)
-return -ENOENT;
+foo = name;
+if (!name) {
+/* If the device lacks OF data, pick the MODALIAS info */
+name = sysfs_uevent_get(path, "MODALIAS");
+if (!name)
+return -ENOENT;
+
+/* .. and strip the MODALIAS=[platform,usb...]: part. */
+foo = strrchr(name, ':');
+if (!foo) {
+free(name);
+return -ENOENT;
+}
+foo++;
+}
 
-strncpy(info->fullname, name, DRM_PLATFORM_DEVICE_NAME_LEN);
+strncpy(info->fullname, foo, DRM_PLATFORM_DEVICE_NAME_LEN);
 info->fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0';
 free(name);
 
@@ -3534,18 +3547,20 @@ static int drmParsePlatformDeviceInfo(int maj, int min,
   drmPlatformDeviceInfoPtr info)
 {
 #ifdef __linux__
-char path[PATH_MAX + 1], *value;
+char path[PATH_MAX + 1], *value, *foo;
 unsigned int count, i;
 int err;
 
 snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
 
 value = sysfs_uevent_get(path, "OF_COMPATIBLE_N");
-if (!value)
-return -ENOENT;
-
-sscanf(value, "%u", &count);
-free(value);
+if (value) {
+sscanf(value, "%u", &count);
+free(value);
+} else {
+/* Assume one entry if the device lack OF data */
+count = 1;
+}
 
 info->compatible = calloc(count + 1, sizeof(*info->compatible));
 if (!info->compatible)
@@ -3553,12 +3568,26 @@ static int drmParsePlatformDeviceInfo(int maj, int min,
 
 for (i = 0; i < count; i++) {
 value = sysfs_uevent_get(path, "OF_COMPATIBLE_%u", i);
+foo = value;
 if (!value) {
-err = -ENOENT;
-goto free;
+/* If the device lacks OF data, pick the MODALIAS info */
+value = sysfs_uevent_get(path, "MODALIAS");
+if (!value) {
+err = -ENOENT;
+goto free;
+}
+
+/* .. and strip the MODALIAS=[platform,usb...]: part. */
+foo = strrchr(value, ':');
+if (!foo) {
+free(value);
+return -ENOENT;
+}
+foo = strdup(foo + 1);
+free(value);
 }
 
-info->compatible[i] = value;
+info->compatible[i] = foo;
 }
 
 return 0;
-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] gpu: ipu-v3: pre: don't trigger update if buffer address doesn't change

[Philipp Zabel]

On Mon, 2019-01-07 at 12:58 +0100, Lucas Stach wrote:
> Am Freitag, den 21.12.2018, 12:11 +0100 schrieb Philipp Zabel:
> > Hi Lucas,
> > 
> > > On Wed, Dec 19, 2018 at 4:40 PM Lucas Stach  
> > > wrote:
> > > 
> > > On a NOP double buffer update where current buffer address is the same
> > > as the next buffer address, the SDW_UPDATE bit clears too late.
> > 
> > What does this mean, exactly? Does the hardware behave differently
> > if the writel to IPU_PRE_NEXT_BUF doesn't change the value?
> 
> To me it seems like that. When the address changes, the SDW_UPDATE bit
> is cleared by the time we handle the EOF IRQ. When the address doesn't
> change, it seems the SDW_UPDATE bit clears much later (I've tried the
> new frame IRQ without any success), maybe only at the next EOF,
> effectively doubling the update period if a plane is flipped with the
> same buffer.

Alright, in that case I think it is correct to carry this workaround in
the PRE driver.

> > > As we
> > > are now using this bit to determine when it is safe to signal flip
> > > completion to userspace this will delay completion of atomic commits
> > > where one plane doesn't change the buffer by a whole frame period.
> > 
> > This sounds like the problem is not the way PRE behaves, but that we are
> > setting the SDW_UPDATE flag again and then later use it to check whether
> > the previous buffer was released, resulting in a false negative.
> > 
> > > Fix this by remembering the last buffer address and just skip the
> > > double buffer update if it would not change the buffer address.
> > 
> > It looks to me like ipu_plane_atomic_update does not have to call
> > ipu_prg_channel_configure (which then just relays to ipu_pre_update)
> > at all in this case. Should we just skip this in ipuv3-plane.c already?
> 
> While we could handle it in ipuv3-plane, this would require more of the
> PRE/PRG state tracking to be moved there. Currently a lot of this is
> hidden behind the simple ipu_prg_channel_configure() call.
> 
> > > Signed-off-by: Lucas Stach 
> > > ---
> > >  drivers/gpu/ipu-v3/ipu-pre.c | 5 +
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/ipu-v3/ipu-pre.c b/drivers/gpu/ipu-v3/ipu-pre.c
> > > index f933c1911e65..d383e8dbd6cc 100644
> > > --- a/drivers/gpu/ipu-v3/ipu-pre.c
> > > +++ b/drivers/gpu/ipu-v3/ipu-pre.c
> > > @@ -106,6 +106,7 @@ struct ipu_pre {
> > > void*buffer_virt;
> > > boolin_use;
> > > unsigned intsafe_window_end;
> > > +   unsigned intlast_bufaddr;
> > 
> > This looks a lot like plane state.
> > 
> > >  };
> > > 
> > >  static DEFINE_MUTEX(ipu_pre_list_mutex);
> > > @@ -242,7 +243,11 @@ void ipu_pre_update(struct ipu_pre *pre, unsigned 
> > > int bufaddr)
> > > unsigned short current_yblock;
> > > u32 val;
> > > 
> > > +   if (bufaddr == pre->last_bufaddr)
> > > +   return;
> > 
> > Hypothetical question, could this wrongly return if the channel is
> > first disabled, leaving
> > last_buffaddr set to X, and then the channel is reenabled, which
> > doesn't touch last_bufaddr,
> > and then the first update contains a buffer at the same address X?
> 
> Nope, this code path is only used when doing no-modeset updates of an
> active plane. When the plane gets disabled the PRE goes through a
> complete reinitialization via ipu_pre_configure() when the channel is
> reenabled.

Let's initialize last_bufaddr in ipu_pre_configure then. I'll amend the
patch as follows:

--8<--
--- a/drivers/gpu/ipu-v3/ipu-pre.c
+++ b/drivers/gpu/ipu-v3/ipu-pre.c
@@ -186,6 +186,7 @@ void ipu_pre_configure(struct ipu_pre *pre, unsigned int 
width,
 
writel(bufaddr, pre->regs + IPU_PRE_CUR_BUF);
writel(bufaddr, pre->regs + IPU_PRE_NEXT_BUF);
+   pre->last_bufaddr = bufaddr;
 
val = IPU_PRE_PREF_ENG_CTRL_INPUT_PIXEL_FORMAT(0) |
  IPU_PRE_PREF_ENG_CTRL_INPUT_ACTIVE_BPP(active_bpp) |
-->8--

regards
Philipp
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 13/15] drm/i915/tv: Generate better pipe timings for TV encoder

[Imre Deak]

On Tue, Jan 22, 2019 at 07:34:55PM +0200, Ville Syrjälä wrote:
> On Tue, Jan 22, 2019 at 07:22:24PM +0200, Imre Deak wrote:
> > On Mon, Nov 12, 2018 at 06:59:58PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä 
> > > 
> > > To make vblank timestamps work better with the TV encoder let's
> > > scale the pipe timings such that the relationship between the
> > > TV active and TV blanking periods is mirrored in the
> > > corresponding pipe timings.
> > > 
> > > Note that in reality the pipe runs at a faster speed during the
> > > TV vblank, and correspondigly there are periods when the pipe
> > > is enitrely stopped. We pretend that this isn't the case and
> > > as such we incur some error in the vblank timestamps during
> > > the TV vblank. Further explanation of the issues in a big
> > > comment in the code.
> > > 
> > > This makes the vblank timestamps good enough to make
> > > i965gm (which doesn't have a working frame counter with
> > > the TV encoder) report correct frame numbers. Previously
> > > you could get all kinds of nonsense which resulted in
> > > eg. glxgears reporting that it's running at twice the
> > > actual framerate in most cases.
> > > 
> > > Signed-off-by: Ville Syrjälä 
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h |   1 +
> > >  drivers/gpu/drm/i915/intel_tv.c | 322 +++-
> > >  2 files changed, 278 insertions(+), 45 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index fe4b913e46ac..10813ae7bee7 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4848,6 +4848,7 @@ enum {
> > >  # define TV_OVERSAMPLE_NONE  (2 << 18)
> > >  /* Selects 8x oversampling */
> > >  # define TV_OVERSAMPLE_8X(3 << 18)
> > > +# define TV_OVERSAMPLE_MASK  (3 << 18)
> > >  /* Selects progressive mode rather than interlaced */
> > >  # define TV_PROGRESSIVE  (1 << 17)
> > >  /* Sets the colorburst to PAL mode.  Required for non-M PAL modes. */
> > > diff --git a/drivers/gpu/drm/i915/intel_tv.c 
> > > b/drivers/gpu/drm/i915/intel_tv.c
> > > index 216525dd144a..75126fce655d 100644
> > > --- a/drivers/gpu/drm/i915/intel_tv.c
> > > +++ b/drivers/gpu/drm/i915/intel_tv.c
> > > @@ -340,7 +340,6 @@ struct tv_mode {
> > >   const struct video_levels *composite_levels, *svideo_levels;
> > >   const struct color_conversion *composite_color, *svideo_color;
> > >   const u32 *filter_table;
> > > - u16 max_srcw;
> > >  };
> > >  
> > >  
> > > @@ -729,7 +728,6 @@ static const struct tv_mode tv_modes[] = {
> > >   .burst_ena  = false,
> > >  
> > >   .filter_table = filter_table,
> > > - .max_srcw = 800
> > >   },
> > >   {
> > >   .name   = "1080i@50Hz",
> > > @@ -947,13 +945,183 @@ intel_tv_mode_vdisplay(const struct tv_mode 
> > > *tv_mode)
> > >   return 2 * (tv_mode->nbr_end + 1);
> > >  }
> > >  
> > > +static void
> > > +intel_tv_mode_to_mode(struct drm_display_mode *mode,
> > > +   const struct tv_mode *tv_mode)
> > > +{
> > > + mode->clock = tv_mode->clock /
> > > + (tv_mode->oversample >> !tv_mode->progressive);
> > > +
> > > + /*
> > > +  * tv_mode horizontal timings:
> > > +  *
> > > +  * hsync_end
> > > +  *| hblank_end
> > > +  *|| hblank_start
> > > +  *||   | htotal
> > > +  *| ___|
> > > +  * /   \___
> > > +  * \__/\
> > > +  */
> > > + mode->hdisplay =
> > > + tv_mode->hblank_start - tv_mode->hblank_end;
> > > + mode->hsync_start = mode->hdisplay +
> > > + tv_mode->htotal - tv_mode->hblank_start;
> > > + mode->hsync_end = mode->hsync_start +
> > > + tv_mode->hsync_end;
> > > + mode->htotal = tv_mode->htotal + 1;
> > > +
> > > + /*
> > > +  * tv_mode vertical timings:
> > > +  *
> > > +  * vsync_start
> > > +  *| vsync_end
> > > +  *|  | vi_end nbr_end
> > > +  *|  ||   |
> > > +  *|  | ___
> > > +  * \__/   \
> > > +  *\__/
> > > +  */
> > > + mode->vdisplay = intel_tv_mode_vdisplay(tv_mode);
> > > + if (tv_mode->progressive) {
> > > + mode->vsync_start = mode->vdisplay +
> > > + tv_mode->vsync_start_f1 + 1;
> > > + mode->vsync_end = mode->vsync_start +
> > > + tv_mode->vsync_len;
> > > + mode->vtotal = mode->vdisplay +
> > > + tv_mode->vi_end_f1 + 1;
> > > + } else {
> > > + mode->vsync_start = mode->vdisplay +
> > > + tv_mode->vsync_start_f1 + 1 +
> > > + tv_mode->vsync_start_f2 + 1;
> > > + mode->vsync_end = mode->vsync_start +
> > > + 2 * tv_mode->vsync_len;
> > > + mode->vtotal = mode->vdisplay +
> > > + tv_mode->vi_end_f1 + 1 +
> > > + tv_mode->vi_end_f2 + 1;
> > > + }
> > > +
> > > + /* TV has it's own notion of 

[PATCH] drm/rockchip: rgb: update SPDX license identifier

[Sandy Huang]

update SPDX License Identifier from GPL-2.0+ to GPL-2.0
and drop some GPL text.

Signed-off-by: Sandy Huang 
---
 drivers/gpu/drm/rockchip/rockchip_rgb.c | 11 +--
 drivers/gpu/drm/rockchip/rockchip_rgb.h | 11 +--
 2 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c 
b/drivers/gpu/drm/rockchip/rockchip_rgb.c
index 96ac145..ad9358f 100644
--- a/drivers/gpu/drm/rockchip/rockchip_rgb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
@@ -1,17 +1,8 @@
-//SPDX-License-Identifier: GPL-2.0+
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
  * Author:
  *  Sandy Huang 
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
  */
 
 #include 
diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.h 
b/drivers/gpu/drm/rockchip/rockchip_rgb.h
index 38b52e6..27b9635 100644
--- a/drivers/gpu/drm/rockchip/rockchip_rgb.h
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.h
@@ -1,17 +1,8 @@
-//SPDX-License-Identifier: GPL-2.0+
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
  * Author:
  *  Sandy Huang 
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
  */
 
 #ifdef CONFIG_ROCKCHIP_RGB
-- 
2.7.4



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Jani Nikula]

On Wed, 23 Jan 2019, Daniel Stone  wrote:
> If it helps, maybe we could draw up a list somewhere (README in igt?
> wiki?) of which tests seem to pass generically across a few drivers,
> which ones are expected to pass, which ones don't but really should,
> etc. I'm currently running on imx-drm (complicated by hitting a
> page-cache BUG!), and a couple of others are getting results from
> rockchip, vc4, and msm. Those are pretty well supported and actively
> maintained, so should give us a decent first cut at such a list.

See tests/intel-ci/README. igt supports test lists, we could maintain
similar lists for different needs.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Stone]

Hi,

On Tue, 22 Jan 2019 at 19:42, Wentland, Harry  wrote:
> On 2019-01-22 2:19 p.m., Daniel Vetter wrote:
> > I'd say we'll shrug these cases off as "can't be reasonable tested,
> > won't have an igt". First driver team with hw that can be validated
> > gets to fill the gaps :-) In practice still going to be a lot better
> > than no tests at all, just exercising the feature will be useful, and
> > will make it a lot easier for the next team to add the crc based tests
> > on top.
>
> I think that's reasonable. After all, we want to start somewhere.
>
> Would it make sense to append something like ", if such a test can be 
> reasonably made using IGT for the target HW." to make it clear to 
> contributors that in cases like the one discussed this is at the reviewers 
> discretion?
>
> With that change (or anything else that clarifies your intentions as 
> described above) I'd be happy to give my AB.

I could definitely get behind that as well. For what it's worth, we're
(after a couple of stalls due to upstream rewrites) pushing forward
execution of igt within KernelCI. The results are not yet visible
within kernelci.org, but the tests are at least executing and we're
hoping they'll be tracked and visible soon.

If it helps, maybe we could draw up a list somewhere (README in igt?
wiki?) of which tests seem to pass generically across a few drivers,
which ones are expected to pass, which ones don't but really should,
etc. I'm currently running on imx-drm (complicated by hitting a
page-cache BUG!), and a couple of others are getting results from
rockchip, vc4, and msm. Those are pretty well supported and actively
maintained, so should give us a decent first cut at such a list.

Cheers,
Daniel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Jani Nikula]

On Tue, 22 Jan 2019, "Wentland, Harry"  wrote:
> Would it make sense to append something like ", if such a test can be
> reasonably made using IGT for the target HW." to make it clear to
> contributors that in cases like the one discussed this is at the
> reviewers discretion?

I think the simplest change would be to say API changes SHOULD have
driver-agnostic testcases, with the RFC 2119 meaning of SHOULD:

   SHOULD   This word, or the adjective "RECOMMENDED", mean that there
   may exist valid reasons in particular circumstances to ignore a
   particular item, but the full implications must be understood and
   carefully weighed before choosing a different course.

I.e. s/need/should/. I think it also catches the spirit of the
discussion here; seems like everyone agrees having tests is a good goal.

You'll have to allow for reviewer/maintainer/community discretion no
matter what. Judging by the discussion, CRC based tests don't currently
meet the driver-agnostic requirement. Playing devil's advocate, you
could argue any new APIs couldn't be tested with CRC either, even if it
were the most reasonable approach for i915.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel