Re: [PATCH 13/14] drm/kmb: Enable alpha blended second plane

2021-08-02 Thread Sam Ravnborg
Hi Anitha,

On Mon, Aug 02, 2021 at 08:44:26PM +, Chrisanthus, Anitha wrote:
> Hi Sam,
> Thanks. Where should this go, drm-misc-fixes or drm-misc-next?

Looks like a drm-misc-next candidate to me.
I may improve something for existing users, but it does not look like it
fixes an existing bug.

Sam


Re: [RESEND] [PATCH v2 1/2] dt-bindings: display: bridge: Add binding for R-Car MIPI DSI/CSI-2 TX

2021-08-02 Thread Laurent Pinchart
Hi Rob,

On Mon, Aug 02, 2021 at 04:37:38PM -0600, Rob Herring wrote:
> On Wed, Jul 28, 2021 at 07:26:39PM +0300, Laurent Pinchart wrote:
> > The R-Car MIPI DSI/CSI-2 TX is embedded in the Renesas R-Car V3U SoC. It
> > can operate in either DSI or CSI-2 mode, with up to four data lanes.
> > 
> > Signed-off-by: Laurent Pinchart 
> > Reviewed-by: Kieran Bingham 
> > ---
> > Looks like I forgot to CC the devicetree mailing list and Rob Herring on
> > the first try. Resending, sorry about that.
> > ---
> >  .../display/bridge/renesas,dsi-csi2-tx.yaml   | 118 ++
> >  MAINTAINERS   |   1 +
> >  2 files changed, 119 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> 
> Reviewed-by: Rob Herring 
> 
> BTW, b4 doesn't like your message-id.

I've told base64, but it shrugged.

Is there a recommendation to change mutt's message_id_format from the
default ?

-- 
Regards,

Laurent Pinchart


[PATCH 3/4] drm/i915/guc: Add DG1 GuC / HuC firmware defs

2021-08-02 Thread Matthew Brost
Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c 
b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index f8cb00ffb506..a685d563df72 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -51,6 +51,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
 #define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
fw_def(ALDERLAKE_P, 0, guc_def(adlp, 62, 0, 3), huc_def(tgl, 7, 9, 3)) \
fw_def(ALDERLAKE_S, 0, guc_def(tgl, 62, 0, 0), huc_def(tgl,  7, 9, 3)) \
+   fw_def(DG1, 0, guc_def(dg1, 62, 0, 0), huc_def(dg1,  7, 9, 3)) \
fw_def(ROCKETLAKE,  0, guc_def(tgl, 62, 0, 0), huc_def(tgl,  7, 9, 3)) \
fw_def(TIGERLAKE,   0, guc_def(tgl, 62, 0, 0), huc_def(tgl,  7, 9, 3)) \
fw_def(JASPERLAKE,  0, guc_def(ehl, 62, 0, 0), huc_def(ehl,  9, 0, 0)) \
-- 
2.28.0



[PATCH 4/4] drm/i915/guc: Enable GuC submission by default on DG1

2021-08-02 Thread Matthew Brost
Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index da57d18d9f6b..fc2fc8d111d8 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -35,7 +35,7 @@ static void uc_expand_default_options(struct intel_uc *uc)
}
 
/* Intermediate platforms are HuC authentication only */
-   if (IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
+   if (IS_ALDERLAKE_S(i915)) {
i915->params.enable_guc = ENABLE_GUC_LOAD_HUC;
return;
}
-- 
2.28.0



[PATCH 1/4] drm/i915: Do not define vma on stack

2021-08-02 Thread Matthew Brost
From: Venkata Sandeep Dhanalakota 

Defining vma on stack can cause stack overflow, if
vma gets populated with new fields.

Cc: Daniele Ceraolo Spurio 
Cc: Tvrtko Ursulin 
Signed-off-by: Venkata Sandeep Dhanalakota 
Signef-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 18 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h |  2 ++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c 
b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 3a16d08608a5..f632dbd32b42 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -413,20 +413,20 @@ static void uc_fw_bind_ggtt(struct intel_uc_fw *uc_fw)
 {
struct drm_i915_gem_object *obj = uc_fw->obj;
struct i915_ggtt *ggtt = __uc_fw_to_gt(uc_fw)->ggtt;
-   struct i915_vma dummy = {
-   .node.start = uc_fw_ggtt_offset(uc_fw),
-   .node.size = obj->base.size,
-   .pages = obj->mm.pages,
-   .vm = >vm,
-   };
+   struct i915_vma *dummy = _fw->dummy;
+
+   dummy->node.start = uc_fw_ggtt_offset(uc_fw);
+   dummy->node.size = obj->base.size;
+   dummy->pages = obj->mm.pages;
+   dummy->vm = >vm;
 
GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
-   GEM_BUG_ON(dummy.node.size > ggtt->uc_fw.size);
+   GEM_BUG_ON(dummy->node.size > ggtt->uc_fw.size);
 
/* uc_fw->obj cache domains were not controlled across suspend */
-   drm_clflush_sg(dummy.pages);
+   drm_clflush_sg(dummy->pages);
 
-   ggtt->vm.insert_entries(>vm, , I915_CACHE_NONE, 0);
+   ggtt->vm.insert_entries(>vm, dummy, I915_CACHE_NONE, 0);
 }
 
 static void uc_fw_unbind_ggtt(struct intel_uc_fw *uc_fw)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h 
b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index 99bb1fe1af66..693cc0ebcd63 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -10,6 +10,7 @@
 #include "intel_uc_fw_abi.h"
 #include "intel_device_info.h"
 #include "i915_gem.h"
+#include "i915_vma.h"
 
 struct drm_printer;
 struct drm_i915_private;
@@ -75,6 +76,7 @@ struct intel_uc_fw {
bool user_overridden;
size_t size;
struct drm_i915_gem_object *obj;
+   struct i915_vma dummy;
 
/*
 * The firmware build process will generate a version header file with 
major and
-- 
2.28.0



[PATCH 0/4] Enable GuC submission by default on DG1

2021-08-02 Thread Matthew Brost
Minimum set of patches to enable GuC submission on DG1 and enable it by
default.

A little difficult to test as IGTs do not work with DG1 due to a bunch
of uAPI features being disabled (e.g. relocations, caching memory
options, etc...).

Tested with the loading the driver and 'live' selftests. Submissions
seem to work. 

Signed-off-by: Matthew Brost 

Daniele Ceraolo Spurio (1):
  drm/i915/guc: put all guc objects in lmem when available

Matthew Brost (2):
  drm/i915/guc: Add DG1 GuC / HuC firmware defs
  drm/i915/guc: Enable GuC submission by default on DG1

Venkata Sandeep Dhanalakota (1):
  drm/i915: Do not define vma on stack

 drivers/gpu/drm/i915/gem/i915_gem_lmem.c  | 26 +++
 drivers/gpu/drm/i915/gem/i915_gem_lmem.h  |  4 +
 drivers/gpu/drm/i915/gt/uc/intel_guc.c|  9 ++-
 drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 11 ++-
 drivers/gpu/drm/i915/gt/uc/intel_huc.c| 14 +++-
 drivers/gpu/drm/i915/gt/uc/intel_uc.c |  2 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  | 90 ---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h  |  2 +
 8 files changed, 138 insertions(+), 20 deletions(-)

-- 
2.28.0



[PATCH 2/4] drm/i915/guc: put all guc objects in lmem when available

2021-08-02 Thread Matthew Brost
From: Daniele Ceraolo Spurio 

The firmware binary has to be loaded from lmem and the recommendation is
to put all other objects in there as well. Note that we don't fall back
to system memory if the allocation in lmem fails because all objects are
allocated during driver load and if we have issues with lmem at that point
something is seriously wrong with the system, so no point in trying to
handle it.

Cc: Matthew Auld 
Cc: Abdiel Janulgue 
Cc: Michal Wajdeczko 
Cc: Vinay Belgaumkar 
Cc: Radoslaw Szwichtenberg 
Signed-off-by: Daniele Ceraolo Spurio 
Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gem/i915_gem_lmem.c  | 26 
 drivers/gpu/drm/i915/gem/i915_gem_lmem.h  |  4 ++
 drivers/gpu/drm/i915/gt/uc/intel_guc.c|  9 ++-
 drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 11 +++-
 drivers/gpu/drm/i915/gt/uc/intel_huc.c| 14 -
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  | 75 +--
 6 files changed, 127 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c 
b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
index eb345305dc52..034226c5d4d0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
@@ -103,6 +103,32 @@ __i915_gem_object_create_lmem_with_ps(struct 
drm_i915_private *i915,
 size, page_size, flags);
 }
 
+struct drm_i915_gem_object *
+i915_gem_object_create_lmem_from_data(struct drm_i915_private *i915,
+ const void *data, size_t size)
+{
+   struct drm_i915_gem_object *obj;
+   void *map;
+
+   obj = i915_gem_object_create_lmem(i915,
+ round_up(size, PAGE_SIZE),
+ I915_BO_ALLOC_CONTIGUOUS);
+   if (IS_ERR(obj))
+   return obj;
+
+   map = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WC);
+   if (IS_ERR(map)) {
+   i915_gem_object_put(obj);
+   return map;
+   }
+
+   memcpy(map, data, size);
+
+   i915_gem_object_unpin_map(obj);
+
+   return obj;
+}
+
 struct drm_i915_gem_object *
 i915_gem_object_create_lmem(struct drm_i915_private *i915,
resource_size_t size,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h 
b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
index 4ee81fc66302..1b88ea13435c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
@@ -23,6 +23,10 @@ bool i915_gem_object_is_lmem(struct drm_i915_gem_object 
*obj);
 
 bool __i915_gem_object_is_lmem(struct drm_i915_gem_object *obj);
 
+struct drm_i915_gem_object *
+i915_gem_object_create_lmem_from_data(struct drm_i915_private *i915,
+ const void *data, size_t size);
+
 struct drm_i915_gem_object *
 __i915_gem_object_create_lmem_with_ps(struct drm_i915_private *i915,
  resource_size_t size,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 979128e28372..55160d3e401a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -3,6 +3,7 @@
  * Copyright © 2014-2019 Intel Corporation
  */
 
+#include "gem/i915_gem_lmem.h"
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_irq.h"
 #include "gt/intel_gt_pm_irq.h"
@@ -630,7 +631,13 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc 
*guc, u32 size)
u64 flags;
int ret;
 
-   obj = i915_gem_object_create_shmem(gt->i915, size);
+   if (HAS_LMEM(gt->i915))
+   obj = i915_gem_object_create_lmem(gt->i915, size,
+ I915_BO_ALLOC_CPU_CLEAR |
+ I915_BO_ALLOC_CONTIGUOUS);
+   else
+   obj = i915_gem_object_create_shmem(gt->i915, size);
+
if (IS_ERR(obj))
return ERR_CAST(obj);
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 76fe766ad1bc..962be0c12208 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -41,7 +41,7 @@ static void guc_prepare_xfer(struct intel_uncore *uncore)
 }
 
 /* Copy RSA signature from the fw image to HW for verification */
-static void guc_xfer_rsa(struct intel_uc_fw *guc_fw,
+static int guc_xfer_rsa(struct intel_uc_fw *guc_fw,
 struct intel_uncore *uncore)
 {
u32 rsa[UOS_RSA_SCRATCH_COUNT];
@@ -49,10 +49,13 @@ static void guc_xfer_rsa(struct intel_uc_fw *guc_fw,
int i;
 
copied = intel_uc_fw_copy_rsa(guc_fw, rsa, sizeof(rsa));
-   GEM_BUG_ON(copied < sizeof(rsa));
+   if (copied < sizeof(rsa))
+   return -ENOMEM;
 
for (i = 0; i < UOS_RSA_SCRATCH_COUNT; i++)
intel_uncore_write(uncore, UOS_RSA_SCRATCH(i), rsa[i]);
+
+   return 0;
 }
 
 

Re: [PATCH v5 2/6] soc: mediatek: add mtk-mmsys support for mt8195 vdosys0

2021-08-02 Thread Jason-JH Lin
Hi Enric,

On Fri, 2021-07-30 at 13:41 +0200, Enric Balletbo i Serra wrote:
> Hi Jason,
> 
> Thank you for your patch.
> 
> On 29/7/21 19:07, jason-jh.lin wrote:
> > Add mt8195 vdosys0 clock driver name and routing table to
> > the driver data of mtk-mmsys.
> > 
> 
> This patch is the one that is really introducing mt8195 mmsys
> support. It is a
> bit confusing sent the binding on another patchset. Please include
> [1] in this
> patchset so it's clear.
> 
> [1]
> 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210722092624.14401-2-jason-jh@mediatek.com/__;!!CTRNKA9wMg0ARbw!3MaHl_gSJTB1nJ3RMsmAf7oLpKF8I3nDGIgkcVBlZ_LjV8Z0g88U5giQCAXRidxVujDS$
>  
Ok, I'll add this.
> 
> > Signed-off-by: jason-jh.lin 
> > ---
> > This patch is base on [1]
> > 
> > [1]add mt8195 SoC DRM binding
> > - 
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/list/?series=519597__;!!CTRNKA9wMg0ARbw!3MaHl_gSJTB1nJ3RMsmAf7oLpKF8I3nDGIgkcVBlZ_LjV8Z0g88U5giQCAXRiX1vDsmL$
> >  
> > ---
> >  drivers/soc/mediatek/mt8195-mmsys.h| 96
> > ++
> >  drivers/soc/mediatek/mtk-mmsys.c   | 11 +++
> >  include/linux/soc/mediatek/mtk-mmsys.h |  9 +++
> >  3 files changed, 116 insertions(+)
> >  create mode 100644 drivers/soc/mediatek/mt8195-mmsys.h
> > 
> > diff --git a/drivers/soc/mediatek/mt8195-mmsys.h
> > b/drivers/soc/mediatek/mt8195-mmsys.h
> > new file mode 100644
> > index ..9339a786ec5d
> > --- /dev/null
> > +++ b/drivers/soc/mediatek/mt8195-mmsys.h
> > @@ -0,0 +1,96 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef __SOC_MEDIATEK_MT8195_MMSYS_H
> > +#define __SOC_MEDIATEK_MT8195_MMSYS_H
> > +
> > +#define MT8195_VDO0_OVL_MOUT_EN
> > 0xf14
> > +#define MT8195_MOUT_DISP_OVL0_TO_DISP_RDMA0
> > BIT(0)
> > +#define MT8195_MOUT_DISP_OVL0_TO_DISP_WDMA0
> > BIT(1)
> > +#define MT8195_MOUT_DISP_OVL0_TO_DISP_OVL1 BIT(2)
> > +#define MT8195_MOUT_DISP_OVL1_TO_DISP_RDMA1
> > BIT(4)
> > +#define MT8195_MOUT_DISP_OVL1_TO_DISP_WDMA1
> > BIT(5)
> > +#define MT8195_MOUT_DISP_OVL1_TO_DISP_OVL0 BIT(6)
> > +
> > +#define MT8195_VDO0_SEL_IN 0xf34
> > +#define MT8195_SEL_IN_VPP_MERGE_FROM_DSC_WRAP0_OUT (0 <<
> > 0)
> > +#define MT8195_SEL_IN_VPP_MERGE_FROM_DISP_DITHER1  (1 <<
> > 0)
> > +#define MT8195_SEL_IN_VPP_MERGE_FROM_VDO1_VIRTUAL0 (2 <<
> > 0)
> > +#define MT8195_SEL_IN_DSC_WRAP0_IN_FROM_DISP_DITHER0   
> > (0 << 4)
> > +#define MT8195_SEL_IN_DSC_WRAP0_IN_FROM_VPP_MERGE  (1 <<
> > 4)
> > +#define MT8195_SEL_IN_DSC_WRAP1_IN_FROM_DISP_DITHER1   
> > (0 << 5)
> > +#define MT8195_SEL_IN_DSC_WRAP1_IN_FROM_VPP_MERGE  (1 <<
> > 5)
> > +#define MT8195_SEL_IN_SINA_VIRTUAL0_FROM_VPP_MERGE (0 <<
> > 8)
> > +#define MT8195_SEL_IN_SINA_VIRTUAL0_FROM_DSC_WRAP1_OUT 
> > (1 << 8)
> > +#define MT8195_SEL_IN_SINB_VIRTUAL0_FROM_DSC_WRAP0_OUT 
> > (0 << 9)
> > +#define MT8195_SEL_IN_DP_INTF0_FROM_DSC_WRAP1_OUT  (0 <<
> > 12)
> > +#define MT8195_SEL_IN_DP_INTF0_FROM_VPP_MERGE  
> > (1 << 12)
> > +#define MT8195_SEL_IN_DP_INTF0_FROM_VDO1_VIRTUAL0  (2 <<
> > 12)
> > +#define MT8195_SEL_IN_DSI0_FROM_DSC_WRAP0_OUT  
> > (0 << 16)
> > +#define MT8195_SEL_IN_DSI0_FROM_DISP_DITHER0   
> > (1 << 16)
> > +#define MT8195_SEL_IN_DSI1_FROM_DSC_WRAP1_OUT  
> > (0 << 17)
> > +#define MT8195_SEL_IN_DSI1_FROM_VPP_MERGE  (1 <<
> > 17)
> > +#define MT8195_SEL_IN_DISP_WDMA1_FROM_DISP_OVL1
> > (0 << 20)
> > +#define MT8195_SEL_IN_DISP_WDMA1_FROM_VPP_MERGE
> > (1 << 20)
> > +#define MT8195_SEL_IN_DSC_WRAP1_OUT_FROM_DSC_WRAP1_IN  
> > (0 << 21)
> > +#define MT8195_SEL_IN_DSC_WRAP1_OUT_FROM_DISP_DITHER1  
> > (1 << 21)
> > +#define MT8195_SEL_IN_DISP_WDMA0_FROM_DISP_OVL0
> > (0 << 22)
> > +#define MT8195_SEL_IN_DISP_WDMA0_FROM_VPP_MERGE
> > (1 << 22)
> > +
> > +#define MT8195_VDO0_SEL_OUT
> > 0xf38
> > +#define MT8195_SOUT_DISP_DITHER0_TO_DSC_WRAP0_IN   (0 <<
> > 0)
> > +#define MT8195_SOUT_DISP_DITHER0_TO_DSI0   (1 <<
> > 0)
> > +#define MT8195_SOUT_DISP_DITHER1_TO_DSC_WRAP1_IN   (0 <<
> > 1)
> > +#define MT8195_SOUT_DISP_DITHER1_TO_VPP_MERGE  
> > (1 << 1)
> > +#define MT8195_SOUT_DISP_DITHER1_TO_DSC_WRAP1_OUT  (2 <<
> > 1)
> > +#define MT8195_SOUT_VDO1_VIRTUAL0_TO_VPP_MERGE 
> > (0 << 4)
> > +#define MT8195_SOUT_VDO1_VIRTUAL0_TO_DP_INTF0  
> > (1 << 4)
> > +#define MT8195_SOUT_VPP_MERGE_TO_DSI1

Re: [PATCH v5 0/6] Add Mediatek Soc DRM (vdosys0) support for mt8195

2021-08-02 Thread Jason-JH Lin
Hi Enric,

On Fri, 2021-07-30 at 13:45 +0200, Enric Balletbo i Serra wrote:
> Hi Jason,
> 
> Thank you for your patch.
> 
> On 29/7/21 19:07, jason-jh.lin wrote:
> > The hardware path of vdosys0 with eDP panel output need to go
> > through
> > by several modules, such as, OVL, RDMA, COLOR, CCORR, AAL, GAMMA,
> > DITHER, DSC and MERGE.
> > 
> 
> 
> You said in other discussions that vdosys0 has eDP panel output and
> vdosys1 has
> DP output. Is it possible to switch the outputs? What I am wondering
> is if this
> configuration is hardware specific or board specific, i.e it'll be
> possible to
> have another board that has DP output on vdosys0 and eDP output for
> vdosys1?
> 
> Thanks,
>   Enric
> 

Because the configuration is hardware specific in mt8195 SoC, it is
possible to switch the output to DP or HDMI for vdosys1 and to switch
the output to DSI or eDP for vdosys0.
Due to the hot plug reqirement for HDMI and DP, vdosys1 will support
runtime sitching output. vdosys0 can do that too, but there is no hot
plug requirement for DSI panel or eDP panel.
So we configure the output to eDP for vdosys0 in build time currently.

In another board, such as mt8173 SoC, it doesn't have vdosys1 but it is
able to set the output to DSI and HDMI for vdosys0 at the same time.

Regards,
Jason-JH.Lin

> 
> > Change in v5:
> > - add power-domain property into vdosys0 and vdosys1 dts node.
> > - add MT8195 prifix and remove unused VDO1 define in mt8195-mmsys.h
> > 
> > Change in v4:
> > - extract dt-binding patches to another patch series
> >   
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/list/?series=519597__;!!CTRNKA9wMg0ARbw!3juKPT3EahvXs9aSRTDPxQfapIiXFGGx996Kss9b8DITyA19pd5F4K0AWsEp1U3C3FSl$
> >  
> > - squash DSC module into mtk_drm_ddp_comp.c
> > - add coment and simplify MERGE config function
> > 
> > Change in v3:
> > - change mmsys and display dt-bindings document from txt to yaml
> > - add MERGE additional description in display dt-bindings document
> > - fix mboxes-cells number of vdosys0 node in dts
> > - drop mutex eof convert define
> > - remove pm_runtime apis in DSC and MERGE
> > - change DSC and MERGE enum to alphabetic order
> > 
> > Change in v2:
> > - add DSC yaml file
> > - add mt8195 drm driver porting parts in to one patch
> > - remove useless define, variable, structure member and function
> > - simplify DSC and MERGE file and switch threre order
> > 
> > jason-jh.lin (6):
> >   arm64: dts: mt8195: add display node for vdosys0
> >   soc: mediatek: add mtk-mmsys support for mt8195 vdosys0
> >   soc: mediatek: add mtk-mutex support for mt8195 vdosys0
> >   drm/mediatek: add mediatek-drm of vdosys0 support for mt8195
> >   drm/mediatek: add DSC support for mt8195
> >   drm/mediatek: add MERGE support for mt8195
> > 
> >  arch/arm64/boot/dts/mediatek/mt8195.dtsi| 112 
> >  drivers/gpu/drm/mediatek/Makefile   |   1 +
> >  drivers/gpu/drm/mediatek/mtk_disp_drv.h |   8 +
> >  drivers/gpu/drm/mediatek/mtk_disp_merge.c   | 277
> > 
> >  drivers/gpu/drm/mediatek/mtk_disp_rdma.c|   6 +
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  62 +
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   2 +
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.c  |  32 ++-
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.h  |   1 +
> >  drivers/soc/mediatek/mt8195-mmsys.h |  96 +++
> >  drivers/soc/mediatek/mtk-mmsys.c|  11 +
> >  drivers/soc/mediatek/mtk-mutex.c|  93 ++-
> >  include/linux/soc/mediatek/mtk-mmsys.h  |   9 +
> >  13 files changed, 706 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_merge.c
> >  create mode 100644 drivers/soc/mediatek/mt8195-mmsys.h
> > 
-- 
Jason-JH Lin 


Re: [PATCH] dma-buf: heaps: Set allocation limit for system heap

2021-08-02 Thread John Stultz
On Thu, Jul 22, 2021 at 12:07 PM Hridya Valsaraju  wrote:
> This patch limits the size of total memory that can be requested in a
> single allocation from the system heap. This would prevent a
> buggy/malicious client from depleting system memory by requesting for an
> extremely large allocation which might destabilize the system.
>
> The limit is set to half the size of the device's total RAM which is the
> same as what was set by the deprecated ION system heap.
>
> Signed-off-by: Hridya Valsaraju 

Seems sane to me, unless folks have better suggestions for allocation limits.

Reviewed-by: John Stultz 

thanks
-john


Re: [Freedreno] [PATCH 0/3] iommu/drm/msm: Allow non-coherent masters to use system cache

2021-08-02 Thread Rob Clark
On Mon, Aug 2, 2021 at 8:14 AM Will Deacon  wrote:
>
> On Mon, Aug 02, 2021 at 08:08:07AM -0700, Rob Clark wrote:
> > On Mon, Aug 2, 2021 at 3:55 AM Will Deacon  wrote:
> > >
> > > On Thu, Jul 29, 2021 at 10:08:22AM +0530, Sai Prakash Ranjan wrote:
> > > > On 2021-07-28 19:30, Georgi Djakov wrote:
> > > > > On Mon, Jan 11, 2021 at 07:45:02PM +0530, Sai Prakash Ranjan wrote:
> > > > > > commit ecd7274fb4cd ("iommu: Remove unused IOMMU_SYS_CACHE_ONLY 
> > > > > > flag")
> > > > > > removed unused IOMMU_SYS_CACHE_ONLY prot flag and along with it went
> > > > > > the memory type setting required for the non-coherent masters to use
> > > > > > system cache. Now that system cache support for GPU is added, we 
> > > > > > will
> > > > > > need to set the right PTE attribute for GPU buffers to be sys 
> > > > > > cached.
> > > > > > Without this, the system cache lines are not allocated for GPU.
> > > > > >
> > > > > > So the patches in this series introduces a new prot flag IOMMU_LLC,
> > > > > > renames IO_PGTABLE_QUIRK_ARM_OUTER_WBWA to IO_PGTABLE_QUIRK_PTW_LLC
> > > > > > and makes GPU the user of this protection flag.
> > > > >
> > > > > Thank you for the patchset! Are you planning to refresh it, as it does
> > > > > not apply anymore?
> > > > >
> > > >
> > > > I was waiting on Will's reply [1]. If there are no changes needed, then
> > > > I can repost the patch.
> > >
> > > I still think you need to handle the mismatched alias, no? You're adding
> > > a new memory type to the SMMU which doesn't exist on the CPU side. That
> > > can't be right.
> > >
> >
> > Just curious, and maybe this is a dumb question, but what is your
> > concern about mismatched aliases?  I mean the cache hierarchy on the
> > GPU device side (anything beyond the LLC) is pretty different and
> > doesn't really care about the smmu pgtable attributes..
>
> If the CPU accesses a shared buffer with different attributes to those which
> the device is using then you fall into the "mismatched memory attributes"
> part of the Arm architecture. It's reasonably unforgiving (you should go and
> read it) and in some cases can apply to speculative accesses as well, but
> the end result is typically loss of coherency.

Ok, I might have a few other sections to read first to decipher the
terminology..

But my understanding of LLC is that it looks just like system memory
to the CPU and GPU (I think that would make it "the point of
coherence" between the GPU and CPU?)  If that is true, shouldn't it be
invisible from the point of view of different CPU mapping options?

BR,
-R


Re: [PATCH 3/4] drm/tiny: add simple-dbi driver

2021-08-02 Thread Icenowy Zheng
在 2021-08-02星期一的 17:08 +0200,Thomas Zimmermann写道:
> Hi
> 
> Am 02.08.21 um 08:35 schrieb Icenowy Zheng:
> > Add a driver for generic MIPI DBI panels initialized with MIPI DCS
> > commands.
> > 
> > Currently a ST7789V-based panel is added to it. This panel has its
> > configuration pre-programmed into the controller, so no vendor-
> > specific
> > configuration is needed.
> > 
> > Signed-off-by: Icenowy Zheng 
> > ---
> >   drivers/gpu/drm/tiny/Kconfig  |  12 ++
> >   drivers/gpu/drm/tiny/Makefile |   1 +
> >   drivers/gpu/drm/tiny/simple-dbi.c | 244
> > ++
> >   3 files changed, 257 insertions(+)
> >   create mode 100644 drivers/gpu/drm/tiny/simple-dbi.c
> > 
> > diff --git a/drivers/gpu/drm/tiny/Kconfig
> > b/drivers/gpu/drm/tiny/Kconfig
> > index d31be274a2bd..6cfc57b68a46 100644
> > --- a/drivers/gpu/drm/tiny/Kconfig
> > +++ b/drivers/gpu/drm/tiny/Kconfig
> > @@ -144,6 +144,18 @@ config TINYDRM_REPAPER
> >   
> >   If M is selected the module will be called repaper.
> >   
> > +config TINYDRM_SIMPLE_DBI
> > +   tristate "DRM support for generic MIPI DBI panels"
> > +   depends on DRM && SPI
> > +   select DRM_KMS_HELPER
> > +   select DRM_KMS_CMA_HELPER
> > +   select DRM_MIPI_DBI
> > +   help
> > + DRM driver for generic DBI panels that are MIPI DCS
> > compatible
> > + and needs no vendor-specific setup commands.
> > +
> > + If M is selected the module will be called simple-dbi.
> > +
> >   config TINYDRM_ST7586
> > tristate "DRM support for Sitronix ST7586 display panels"
> > depends on DRM && SPI
> > diff --git a/drivers/gpu/drm/tiny/Makefile
> > b/drivers/gpu/drm/tiny/Makefile
> > index e09942895c77..2553de651aa3 100644
> > --- a/drivers/gpu/drm/tiny/Makefile
> > +++ b/drivers/gpu/drm/tiny/Makefile
> > @@ -13,3 +13,4 @@ obj-$(CONFIG_TINYDRM_MI0283QT)+=
> > mi0283qt.o
> >   obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o
> >   obj-$(CONFIG_TINYDRM_ST7586)  += st7586.o
> >   obj-$(CONFIG_TINYDRM_ST7735R) += st7735r.o
> > +obj-$(CONFIG_TINYDRM_SIMPLE_DBI)   += simple-dbi.o
> > diff --git a/drivers/gpu/drm/tiny/simple-dbi.c
> > b/drivers/gpu/drm/tiny/simple-dbi.c
> > new file mode 100644
> > index ..2b207e43d500
> > --- /dev/null
> > +++ b/drivers/gpu/drm/tiny/simple-dbi.c
> > @@ -0,0 +1,244 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * DRM driver for display panels with configuration preset and
> > needs only
> > + * standard MIPI DCS commands to bring up.
> > + *
> > + * Copyright (C) 2021 Sipeed
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define MIPI_DCS_ADDRESS_MODE_BGR  BIT(3)
> > +#define MIPI_DCS_ADDRESS_MODE_REVERSE  BIT(5)
> > +#define MIPI_DCS_ADDRESS_MODE_RTL  BIT(6)
> > +#define MIPI_DCS_ADDRESS_MODE_BTT  BIT(7)
> > +
> > +struct simple_dbi_cfg {
> > +   const struct drm_display_mode mode;
> > +   unsigned int left_offset;
> > +   unsigned int top_offset;
> > +   bool inverted;
> > +   bool write_only;
> > +   bool bgr;
> > +   bool right_to_left;
> > +   bool bottom_to_top;
> > +};
> > +
> > +struct simple_dbi_priv {
> > +   struct mipi_dbi_dev dbidev; /* Must be first for
> > .release() */
> 
> Which release?

Ah, some copy-n-paste waste.

Will drop this in v2.

> 
> > +   const struct simple_dbi_cfg *cfg;
> > +};
> > +
> > +static void simple_dbi_pipe_enable(struct drm_simple_display_pipe
> > *pipe,
> > +   struct drm_crtc_state *crtc_state,
> > +   struct drm_plane_state
> > *plane_state)
> > +{
> > +   struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe-
> > >crtc.dev);
> > +   struct simple_dbi_priv *priv = container_of(dbidev,
> > +   struct
> > simple_dbi_priv,
> > +   dbidev);
> > +   struct mipi_dbi *dbi = >dbi;
> > +   int ret, idx;
> > +   u8 addr_mode = 0x00;
> > +
> > +   if (!drm_dev_enter(pipe->crtc.dev, ))
> > +   return;
> > +
> > +   DRM_DEBUG_KMS("\n");
> 
> I'm not a friend of such debugging statements. If you absolutely want
> to 
> keep it, rather use drm_dbg_kms().

Should be copy-n-paste waste too.

> 
> > +
> > +   ret = mipi_dbi_poweron_reset(dbidev);
> > +   if (ret)
> > +   goto out_exit;
> > +
> > +   mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
> > +   msleep(5);
> > +
> > +   /* Currently tinydrm supports 16bit only now */
> > +   mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT,
> > +    MIPI_DCS_PIXEL_FMT_16BIT);
> > +
> > +   if 

Re: [Freedreno] [PATCH 11/11] drm/msm/dsi: Pass DSC params to drm_panel

2021-08-02 Thread abhinavk

On 2021-07-14 23:52, Vinod Koul wrote:

When DSC is enabled, we need to pass the DSC parameters to panel driver
as well, so add a dsc parameter in panel and set it when DSC is enabled

Signed-off-by: Vinod Koul 


based on the comments on prev patches in the series, this will need to 
be reworked

too as there wont be any priv->dsc anymore.

Also, can you also post the panel changes? Would like to see how you 
will use the

dsc param there.


---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 16 +++-
 include/drm/drm_panel.h|  7 +++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 4e8ab1b1df8b..ee21cda243a7 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -2193,6 +2193,7 @@ int msm_dsi_host_modeset_init(struct 
mipi_dsi_host *host,

const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
struct platform_device *pdev = msm_host->pdev;
struct msm_drm_private *priv;
+   struct drm_panel *panel;
int ret;

msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
@@ -2212,8 +2213,21 @@ int msm_dsi_host_modeset_init(struct 
mipi_dsi_host *host,

}

msm_host->dev = dev;
+   panel = msm_dsi_host_get_panel(_host->base);
priv = dev->dev_private;
-   priv->dsc = msm_host->dsc;
+
+   if (panel && panel->dsc) {
+   struct msm_display_dsc_config *dsc = priv->dsc;
+
+   if (!dsc) {
+   dsc = kzalloc(sizeof(*dsc), GFP_KERNEL);
+   if (!dsc)
+   return -ENOMEM;
+   dsc->drm = panel->dsc;
+   priv->dsc = dsc;
+   msm_host->dsc = dsc;
+   }
+   }

ret = cfg_hnd->ops->tx_buf_alloc(msm_host, SZ_4K);
if (ret) {
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 33605c3f0eba..27a7808a29f2 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -171,6 +171,13 @@ struct drm_panel {
 * Panel entry in registry.
 */
struct list_head list;
+
+   /**
+* @dsc:
+*
+* Panel DSC pps payload to be sent
+*/
+   struct drm_dsc_config *dsc;
 };

 void drm_panel_init(struct drm_panel *panel, struct device *dev,


Re: [Freedreno] [PATCH 10/11] drm/msm/dsi: Add support for DSC configuration

2021-08-02 Thread abhinavk

On 2021-07-14 23:52, Vinod Koul wrote:

When DSC is enabled, we need to configure DSI registers accordingly and
configure the respective stream compression registers.

Add support to calculate the register setting based on DSC params and
timing information and configure these registers.

Signed-off-by: Vinod Koul 


same comments as dmitry on this one: 
https://patchwork.freedesktop.org/patch/444082/?series=90413=2

nothing more to add.


---
 drivers/gpu/drm/msm/dsi/dsi.xml.h  |  10 ++
 drivers/gpu/drm/msm/dsi/dsi_host.c | 142 +++--
 2 files changed, 142 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.xml.h
b/drivers/gpu/drm/msm/dsi/dsi.xml.h
index 50eb4d1b8fdd..b8e9e608abfc 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.xml.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.xml.h
@@ -2310,4 +2310,14 @@ static inline uint32_t
REG_DSI_7nm_PHY_LN_TX_DCTRL(uint32_t i0) { return 0x0

 #define REG_DSI_7nm_PHY_PLL_PERF_OPTIMIZE  0x0260

+#define REG_DSI_VIDEO_COMPRESSION_MODE_CTRL0x029c
+
+#define REG_DSI_VIDEO_COMPRESSION_MODE_CTRL2   0x02a0
+
+#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL  0x02a4
+
+#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2 0x02a8
+
+#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL3 0x02ac
+
 #endif /* DSI_XML */
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index e1e5d91809b5..4e8ab1b1df8b 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -942,6 +942,26 @@ static void dsi_ctrl_config(struct msm_dsi_host
*msm_host, bool enable,
dsi_write(msm_host, REG_DSI_CTRL, data);
 }

+static int dsi_dsc_update_pic_dim(struct msm_display_dsc_config *dsc,
+ int pic_width, int pic_height)
+{
+   if (!dsc || !pic_width || !pic_height) {
+   pr_err("DSI: invalid input: pic_width: %d pic_height: %d\n",
pic_width, pic_height);
+   return -EINVAL;
+   }
+
+   if ((pic_width % dsc->drm->slice_width) || (pic_height %
dsc->drm->slice_height)) {
+   pr_err("DSI: pic_dim %dx%d has to be multiple of slice %dx%d\n",
+		   pic_width, pic_height, dsc->drm->slice_width, 
dsc->drm->slice_height);

+   return -EINVAL;
+   }
+
+   dsc->drm->pic_width = pic_width;
+   dsc->drm->pic_height = pic_height;
+
+   return 0;
+}
+
 static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool 
is_dual_dsi)

 {
struct drm_display_mode *mode = msm_host->mode;
@@ -956,6 +976,7 @@ static void dsi_timing_setup(struct msm_dsi_host
*msm_host, bool is_dual_dsi)
u32 va_end = va_start + mode->vdisplay;
u32 hdisplay = mode->hdisplay;
u32 wc;
+   u32 data;

DBG("");

@@ -974,7 +995,73 @@ static void dsi_timing_setup(struct msm_dsi_host
*msm_host, bool is_dual_dsi)
hdisplay /= 2;
}

+   if (msm_host->dsc) {
+   struct msm_display_dsc_config *dsc = msm_host->dsc;
+
+   /* update dsc params with timing params */
+   dsi_dsc_update_pic_dim(dsc, mode->hdisplay, mode->vdisplay);
+   DBG("Mode Width- %d x Height %d\n", dsc->drm->pic_width,
dsc->drm->pic_height);
+
+   /* we do the calculations for dsc parameters here so that
+* panel can use these parameters
+*/
+   dsi_populate_dsc_params(dsc);
+
+   /* Divide the display by 3 but keep back/font porch and
+* pulse width same
+*/
+   h_total -= hdisplay;
+   hdisplay /= 3;
+   h_total += hdisplay;
+   ha_end = ha_start + hdisplay;
+   }
+
if (msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) {
+   if (msm_host->dsc) {
+   struct msm_display_dsc_config *dsc = msm_host->dsc;
+   u32 reg, intf_width, slice_per_intf, width;
+   u32 total_bytes_per_intf;
+
+   /* first calculate dsc parameters and then program
+* compress mode registers
+*/
+   intf_width = hdisplay;
+   slice_per_intf = DIV_ROUND_UP(intf_width, 
dsc->drm->slice_width);
+
+   /* If slice_count > slice_per_intf, then use 1
+* This can happen during partial update
+*/
+   dsc->drm->slice_count = 1;
+
+   dsc->bytes_in_slice = 
DIV_ROUND_UP(dsc->drm->slice_width * 8, 8);
+   total_bytes_per_intf = dsc->bytes_in_slice * 
slice_per_intf;
+
+   dsc->eol_byte_num = total_bytes_per_intf % 3;
+   dsc->pclk_per_line =  
DIV_ROUND_UP(total_bytes_per_intf, 3);
+ 

Re: [Freedreno] [PATCH 09/11] drm/msm/disp/dpu1: Add support for DSC in topology

2021-08-02 Thread abhinavk

On 2021-07-14 23:52, Vinod Koul wrote:

For DSC to work we typically need a 2,2,1 configuration. This should
suffice for resolutions upto 4k. For more resolutions like 8k this 
won't

work.

The topology information is provided by DTS so we try to deduce the
topology required for DSC.
Furthermore, we can use 1 DSC encoder in lesser resolutions, but that 
is

not power efficient according to Abhinav, it is better to use 2 mixers
as that will split width/2 and is proven to be power efficient.


I think now that we have added the technical reason of why it is better 
to use
2-2-1 ( using 2 LMs is better than one as it will half layer width), we 
can drop my name from the commit text

as it holds less value than the actual reason itself :)
You can still keep my signed-off and co-developed by tag



Also, the panel has been tested only with 2,2,1 configuration, so for
now we blindly create 2,2,1 topology when DSC is enabled

Co-developed-by: Abhinav Kumar 
Signed-off-by: Abhinav Kumar 
Signed-off-by: Vinod Koul 
---
Changes since RFC:
 - Add more details in changelog

 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 41140b781e66..8f0a8bd9c8ff 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -573,6 +573,8 @@ static struct msm_display_topology 
dpu_encoder_get_topology(

struct drm_display_mode *mode)
 {
struct msm_display_topology topology = {0};
+   struct drm_encoder *drm_enc;
+   struct msm_drm_private *priv;
int i, intf_count = 0;

for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
@@ -607,8 +609,22 @@ static struct msm_display_topology
dpu_encoder_get_topology(
topology.num_enc = 0;
topology.num_intf = intf_count;

+   drm_enc = _enc->base;
+   priv = drm_enc->dev->dev_private;
+   if (priv && priv->dsc) {

if dsc is moved to the encoder, this will need to be changed too

+   /* In case of Display Stream Compression DSC, we would use
+* 2 encoders, 2 line mixers and 1 interface
+* this is power optimal and can drive upto (including) 4k
+* screens
+*/
+   topology.num_enc = 2;
+   topology.num_intf = 1;
+   topology.num_lm = 2;
+   }
+
return topology;
 }
+
 static int dpu_encoder_virt_atomic_check(
struct drm_encoder *drm_enc,
struct drm_crtc_state *crtc_state,


Re: [Freedreno] [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder

2021-08-02 Thread abhinavk

On 2021-07-14 23:52, Vinod Koul wrote:

When DSC is enabled in DT, we need to configure the encoder for DSC
configuration, calculate DSC parameters for the given timing.

This patch adds that support by adding dpu_encoder_prep_dsc() which is
invoked when DSC is enabled in DT
correct me if wrong but this commit text is not valid anymore in my 
opinion.
are there any params you are getting from DT now? I thought its all 
coming from the panel

driver directly.


Signed-off-by: Vinod Koul 
agree with dmitry's comment's 
https://patchwork.freedesktop.org/patch/444078/?series=90413=2


instead of dsc being part of priv->dsc it should be per encoder.

On top of his comment, I also think that like on the newer chipsets, 
moving the dsc related
encoder configuration to a dpu_encoder_dce.c will help for future 
expansion of other topologies

and also for other compression algorithms.


---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 142 +++-
 1 file changed, 141 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 8d942052db8a..41140b781e66 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -21,12 +21,17 @@
 #include "dpu_hw_intf.h"
 #include "dpu_hw_ctl.h"
 #include "dpu_hw_dspp.h"
+#include "dpu_hw_dsc.h"
 #include "dpu_formats.h"
 #include "dpu_encoder_phys.h"
 #include "dpu_crtc.h"
 #include "dpu_trace.h"
 #include "dpu_core_irq.h"

+#define DSC_MODE_SPLIT_PANEL   BIT(0)
+#define DSC_MODE_MULTIPLEX BIT(1)
+#define DSC_MODE_VIDEO BIT(2)
+
 #define DPU_DEBUG_ENC(e, fmt, ...) DPU_DEBUG("enc%d " fmt,\
(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)

@@ -135,6 +140,7 @@ enum dpu_enc_rc_states {
  * @cur_slave: As above but for the slave encoder.
  * @hw_pp: Handle to the pingpong blocks used for the display. No.
  * pingpong blocks can be different than num_phys_encs.
+ * @hw_dsc Handle to the DSC blocks used for the display.
  * @intfs_swapped:	Whether or not the phys_enc interfaces have been 
swapped

  * for partial update right-only cases, such as pingpong
  * split where virtual pingpong does not generate IRQs
@@ -180,6 +186,7 @@ struct dpu_encoder_virt {
struct dpu_encoder_phys *cur_master;
struct dpu_encoder_phys *cur_slave;
struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
+   struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];

bool intfs_swapped;

@@ -1008,7 +1015,8 @@ static void dpu_encoder_virt_mode_set(struct
drm_encoder *drm_enc,
struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL };
-   int num_lm, num_ctl, num_pp;
+   struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
+   int num_lm, num_ctl, num_pp, num_dsc;
int i, j;

if (!drm_enc) {
@@ -1061,11 +1069,16 @@ static void dpu_encoder_virt_mode_set(struct
drm_encoder *drm_enc,
dpu_rm_get_assigned_resources(_kms->rm, global_state,
drm_enc->base.id, DPU_HW_BLK_DSPP, hw_dspp,
ARRAY_SIZE(hw_dspp));
+   num_dsc = dpu_rm_get_assigned_resources(_kms->rm, global_state,
+   drm_enc->base.id, DPU_HW_BLK_DSC, hw_dsc, ARRAY_SIZE(hw_dsc));

for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])
: NULL;

+   for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
+   dpu_enc->hw_dsc[i] = i < num_dsc ? to_dpu_hw_dsc(hw_dsc[i]) : 
NULL;
+
cstate = to_dpu_crtc_state(drm_crtc->state);

for (i = 0; i < num_lm; i++) {
@@ -1810,10 +1823,133 @@ static void
dpu_encoder_vsync_event_work_handler(struct kthread_work *work)
nsecs_to_jiffies(ktime_to_ns(wakeup_time)));
 }

+static void
+dpu_encoder_dsc_pclk_param_calc(struct msm_display_dsc_config *dsc, 
u32 width)

+{
+   int slice_count, slice_per_intf;
+   int bytes_in_slice, total_bytes_per_intf;
+
+   if (!dsc || !dsc->drm->slice_width || !dsc->drm->slice_count) {
+   DPU_ERROR("Invalid DSC/slices\n");
+   return;
+   }
+
+   slice_count = dsc->drm->slice_count;
+   slice_per_intf = DIV_ROUND_UP(width, dsc->drm->slice_width);
+
+   /*
+* If slice_count is greater than slice_per_intf then default to 1.
+* This can happen during partial update.
+*/
+   if (slice_count > slice_per_intf)
+   slice_count = 1;
+
+   bytes_in_slice = DIV_ROUND_UP(dsc->drm->slice_width *
+ dsc->drm->bits_per_pixel, 8);
+   total_bytes_per_intf = bytes_in_slice * slice_per_intf;
+
+   dsc->eol_byte_num = 

Re: [Intel-gfx] [PATCH 6/9] drm/i915: Drop __rcu from gem_context->vm

2021-08-02 Thread kernel test robot
Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next 
tegra-drm/drm/tegra/for-next drm/drm-next v5.14-rc3 next-20210730]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Daniel-Vetter/remove-rcu-support-from-i915_address_space/20210802-234929
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: i386-randconfig-s002-20210802 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-341-g8af24329-dirty
# 
https://github.com/0day-ci/linux/commit/4a70c02a8b49ee9845e8222c55b4bf932e843224
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Daniel-Vetter/remove-rcu-support-from-i915_address_space/20210802-234929
git checkout 4a70c02a8b49ee9845e8222c55b4bf932e843224
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir 
ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 


sparse warnings: (new ones prefixed by >>)
   drivers/gpu/drm/i915/gem/i915_gem_context.c: note: in included file (through 
drivers/gpu/drm/i915/gt/intel_gt_requests.h, 
drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c):
   /usr/lib/gcc/x86_64-linux-gnu/10/include/stddef.h:406:9: sparse: sparse: 
preprocessor token offsetof redefined
   drivers/gpu/drm/i915/gem/i915_gem_context.c: note: in included file (through 
include/uapi/linux/posix_types.h, include/uapi/linux/types.h, 
include/linux/types.h, ...):
   include/linux/stddef.h:17:9: sparse: this was the original definition
   drivers/gpu/drm/i915/gem/i915_gem_context.c: note: in included file:
>> drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c:698:33: sparse: 
>> sparse: incompatible types in comparison expression (different address 
>> spaces):
>> drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c:698:33: sparse:
>> struct i915_address_space [noderef] __rcu *
>> drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c:698:33: sparse:
>> struct i915_address_space *

vim +698 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c

f2085c8e950d53 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c Chris 
Wilson   2019-08-27  631  
791ff39ae32a34 drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2017-02-13  632  static int igt_ctx_exec(void *arg)
791ff39ae32a34 drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2017-02-13  633  {
791ff39ae32a34 drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2017-02-13  634 struct drm_i915_private *i915 = arg;
e0695db7298ec2 drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2019-03-22  635 struct intel_engine_cs *engine;
6e1281412ab9e6 drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2017-11-14  636 int err = -ENODEV;
791ff39ae32a34 drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2017-02-13  637  
0fdbe58c4a0f8c drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2018-07-06  638 /*
0fdbe58c4a0f8c drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2018-07-06  639  * Create a few different contexts (with different 
mm) and write
791ff39ae32a34 drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2017-02-13  640  * through each ctx/mm using the GPU making sure 
those writes end
791ff39ae32a34 drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2017-02-13  641  * up in the expected pages of our obj.
791ff39ae32a34 drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2017-02-13  642  */
791ff39ae32a34 drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2017-02-13  643  
0fdbe58c4a0f8c drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2018-07-06  644 if (!DRIVER_CAPS(i915)->has_logical_contexts)
0fdbe58c4a0f8c drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2018-07-06  645 return 0;
0fdbe58c4a0f8c drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2018-07-06  646  
51757cf4d7e6e1 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c Tvrtko 
Ursulin 2019-10-22  647 for_each_uabi_engine(engine, i915) {
e0695db7298ec2 drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 
Wilson   2019-03-22  648 struct drm_i915_gem_object *obj = NULL;
e0695db7298ec2 drivers/gpu/drm/i915/selftests/i915_gem_context.c Chris 

Re: [Freedreno] [PATCH 07/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d

2021-08-02 Thread abhinavk

On 2021-07-14 23:51, Vinod Koul wrote:

We cannot enable mode_3d when we are using the DSC. So pass
configuration to detect DSC is enabled and not enable mode_3d
when we are using DSC

We add a helper dpu_encoder_helper_get_dsc_mode() to detect dsc
enabled and pass this to .setup_intf_cfg()

This is not entirely correct. This is true only for the 2-2-1 topology 
you are using

on this panel.

When you are using 2-2-1, you are using 2 LMs, 2 DSCs and 1 DSI.
So 3D mux shouldnt be used.

If you are using something like 4-2-1 or 4-2-2, then you have 4LMs,
2 DSCs and 2/1 DSI.

Here you need the 3D mux to convert the data from 4LMs to 2 DSCs.

So please correct the commit text here and also add a check for the 
topology.



Signed-off-by: Vinod Koul 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 11 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c |  2 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c   |  5 +++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h   |  2 ++
 4 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index ecbc4be98980..d43b804528eb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -336,6 +336,17 @@ static inline enum dpu_3d_blend_mode
dpu_encoder_helper_get_3d_blend_mode(
return BLEND_3D_NONE;
 }

+static inline bool dpu_encoder_helper_get_dsc_mode(struct
dpu_encoder_phys *phys_enc)
+{
+   struct drm_encoder *drm_enc = phys_enc->parent;
+   struct msm_drm_private *priv = drm_enc->dev->dev_private;
+
+   if (priv->dsc)
+   return true;
+
+   return false;
+}

Check whether DSC is enabled and only if its 2-2-1 topology.
This needs to be reworked when other topologies are supported.


+
 /**
  * dpu_encoder_helper_split_config - split display configuration
helper function
  * This helper function may be used by physical encoders to configure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index b2be39b9144e..5fe87881c30c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -69,6 +69,8 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_CMD;
intf_cfg.stream_sel = cmd_enc->stream_sel;
intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
+   intf_cfg.dsc = dpu_encoder_helper_get_dsc_mode(phys_enc);
+
ctl->ops.setup_intf_cfg(ctl, _cfg);
 }

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index aeea6add61ee..f059416311ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -121,7 +121,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct
dpu_hw_ctl *ctx)
return ctx->pending_flush_mask;
 }

-static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
+static void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
 {
 	DPU_REG_WRITE(>hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | 
BIT(3));


@@ -522,7 +522,8 @@ static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl 
*ctx,


intf_cfg |= (cfg->intf & 0xF) << 4;

-   if (cfg->mode_3d) {
+   /* In DSC we can't set merge, so check for dsc too */
+   if (cfg->mode_3d && !cfg->dsc) {
intf_cfg |= BIT(19);
intf_cfg |= (cfg->mode_3d - 0x1) << 20;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
index 806c171e5df2..347a653c1e01 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
@@ -39,6 +39,7 @@ struct dpu_hw_stage_cfg {
  * @mode_3d:   3d mux configuration
  * @merge_3d:  3d merge block used
  * @intf_mode_sel: Interface mode, cmd / vid
+ * @dsc:   DSC is enabled
  * @stream_sel:Stream selection for multi-stream 
interfaces

  */
 struct dpu_hw_intf_cfg {
@@ -46,6 +47,7 @@ struct dpu_hw_intf_cfg {
enum dpu_3d_blend_mode mode_3d;
enum dpu_merge_3d merge_3d;
enum dpu_ctl_mode_sel intf_mode_sel;
+   bool dsc;
int stream_sel;
 };


Re: [Freedreno] [PATCH 06/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl

2021-08-02 Thread abhinavk

On 2021-07-14 23:51, Vinod Koul wrote:
Later gens of hardware have DSC bits moved to hw_ctl, so configure 
these

bits so that DSC would work there as well

Signed-off-by: Vinod Koul 
Please correct me if wrong but here you seem to be flushing all the DSC 
bits
even the unused ones. This will end-up enabling DSC even when DSC is 
unused on

the newer targets.
If so, thats wrong.
We need to implement bit-mask based approach to avoid this change and 
only enable

those DSCs which are used.


---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 2d4645e01ebf..aeea6add61ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -25,6 +25,8 @@
 #define   CTL_MERGE_3D_ACTIVE   0x0E4
 #define   CTL_INTF_ACTIVE   0x0F4
 #define   CTL_MERGE_3D_FLUSH0x100
+#define   CTL_DSC_ACTIVE0x0E8
+#define   CTL_DSC_FLUSH0x104
 #define   CTL_INTF_FLUSH0x110
 #define   CTL_INTF_MASTER   0x134
 #define   CTL_FETCH_PIPE_ACTIVE 0x0FC
@@ -34,6 +36,7 @@

 #define DPU_REG_RESET_TIMEOUT_US2000
 #define  MERGE_3D_IDX   23
+#define  DSC_IDX22
 #define  INTF_IDX   31
 #define CTL_INVALID_BIT 0x

@@ -120,6 +123,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct
dpu_hw_ctl *ctx)

 static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
 {
+	DPU_REG_WRITE(>hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | 
BIT(3));


if (ctx->pending_flush_mask & BIT(MERGE_3D_IDX))
DPU_REG_WRITE(>hw, CTL_MERGE_3D_FLUSH,
@@ -128,7 +132,7 @@ static inline void
dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
DPU_REG_WRITE(>hw, CTL_INTF_FLUSH,
ctx->pending_intf_flush_mask);

-   DPU_REG_WRITE(>hw, CTL_FLUSH, ctx->pending_flush_mask);
+	DPU_REG_WRITE(>hw, CTL_FLUSH, ctx->pending_flush_mask |  
BIT(DSC_IDX));

 }

 static inline void dpu_hw_ctl_trigger_flush(struct dpu_hw_ctl *ctx)
@@ -507,6 +511,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct 
dpu_hw_ctl *ctx,

if (cfg->merge_3d)
DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
  BIT(cfg->merge_3d - MERGE_3D_0));
+   DPU_REG_WRITE(c, CTL_DSC_ACTIVE, BIT(0) | BIT(1) | BIT(2) | BIT(3));
 }

 static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,


Re: [Freedreno] [PATCH 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog

2021-08-02 Thread abhinavk

On 2021-07-14 23:51, Vinod Koul wrote:

This add SDM845 DSC blocks into hw_catalog

/add --> adds


Signed-off-by: Vinod Koul 
---
Changes since RFC:
 - use BIT values from MASK

 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 22 +++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index b569030a0847..b45a08303c99 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -40,6 +40,8 @@

 #define PINGPONG_SDM845_MASK BIT(DPU_PINGPONG_DITHER)

+#define DSC_SDM845_MASK BIT(1)
agree with dmitry again : 
https://patchwork.freedesktop.org/patch/444072/?series=90413=2

this is unused. you can use .features = 0

+
 #define PINGPONG_SDM845_SPLIT_MASK \
(PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2))

@@ -751,6 +753,24 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = 
{

PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk),
PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk),
 };
+
+/*
+ * DSC sub blocks config
+ */
+#define DSC_BLK(_name, _id, _base) \
+   {\
+   .name = _name, .id = _id, \
+   .base = _base, .len = 0x140, \
+   .features = DSC_SDM845_MASK, \
+   }
+
+static struct dpu_dsc_cfg sdm845_dsc[] = {
+   DSC_BLK("dsc_0", DSC_0, 0x8),
+   DSC_BLK("dsc_1", DSC_1, 0x80400),
+   DSC_BLK("dsc_2", DSC_2, 0x80800),
+   DSC_BLK("dsc_3", DSC_3, 0x80c00),
+};
+
 /*
  * INTF sub blocks config
  */
@@ -1053,6 +1073,8 @@ static void sdm845_cfg_init(struct dpu_mdss_cfg 
*dpu_cfg)

.mixer = sdm845_lm,
.pingpong_count = ARRAY_SIZE(sdm845_pp),
.pingpong = sdm845_pp,
+   .dsc_count = ARRAY_SIZE(sdm845_dsc),
+   .dsc = sdm845_dsc,
.intf_count = ARRAY_SIZE(sdm845_intf),
.intf = sdm845_intf,
.vbif_count = ARRAY_SIZE(sdm845_vbif),


Re: [Freedreno] [PATCH 04/11] drm/msm/disp/dpu1: Add DSC support in RM

2021-08-02 Thread abhinavk

On 2021-07-14 23:51, Vinod Koul wrote:

This add the bits in RM to enable the DSC blocks

Signed-off-by: Vinod Koul 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  1 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c  | 32 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h  |  1 +
 3 files changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index d6717d6672f7..d56c05146dfe 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -165,6 +165,7 @@ struct dpu_global_state {
uint32_t ctl_to_enc_id[CTL_MAX - CTL_0];
uint32_t intf_to_enc_id[INTF_MAX - INTF_0];
uint32_t dspp_to_enc_id[DSPP_MAX - DSPP_0];
+   uint32_t dsc_to_enc_id[DSC_MAX - DSC_0];
 };

 struct dpu_global_state
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index fd2d104f0a91..4da6d72b7996 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -11,6 +11,7 @@
 #include "dpu_hw_intf.h"
 #include "dpu_hw_dspp.h"
 #include "dpu_hw_merge3d.h"
+#include "dpu_hw_dsc.h"
 #include "dpu_encoder.h"
 #include "dpu_trace.h"

@@ -75,6 +76,14 @@ int dpu_rm_destroy(struct dpu_rm *rm)
dpu_hw_intf_destroy(hw);
}
}
+   for (i = 0; i < ARRAY_SIZE(rm->dsc_blks); i++) {
+   struct dpu_hw_dsc *hw;
+
+   if (rm->intf_blks[i]) {
same comment as dmitry on this 
https://patchwork.freedesktop.org/patch/444070/?series=90413=2

+   hw = to_dpu_hw_dsc(rm->dsc_blks[i]);
+   dpu_hw_dsc_destroy(hw);
+   }
+   }

return 0;
 }
@@ -221,6 +230,19 @@ int dpu_rm_init(struct dpu_rm *rm,
rm->dspp_blks[dspp->id - DSPP_0] = >base;
}

+   for (i = 0; i < cat->dsc_count; i++) {
+   struct dpu_hw_dsc *hw;
+   const struct dpu_dsc_cfg *dsc = >dsc[i];
+
+   hw = dpu_hw_dsc_init(dsc->id, mmio, cat);
+   if (IS_ERR_OR_NULL(hw)) {
+   rc = PTR_ERR(hw);
+   DPU_ERROR("failed dsc object creation: err %d\n", rc);
+   goto fail;
+   }
+   rm->dsc_blks[dsc->id - DSC_0] = >base;
+   }
+
return 0;

 fail:
@@ -476,6 +498,9 @@ static int _dpu_rm_reserve_intf(
}

global_state->intf_to_enc_id[idx] = enc_id;
+
+   global_state->dsc_to_enc_id[0] = enc_id;
+   global_state->dsc_to_enc_id[1] = enc_id;
return 0;
 }
agree with dmitry again here, why are DSCs being reserved in the 
_dpu_rm_reserve_intf function?

First, for clarity, they should be in a function of their own.
Allocating the DSCs has to also account for the PP availability of that 
DSC and other factors need to

be considered as well.
I suggest checking _sde_rm_reserve_dsc() from downstream to improve the 
DSC reservation logic.


@@ -567,6 +592,8 @@ void dpu_rm_release(struct dpu_global_state 
*global_state,

ARRAY_SIZE(global_state->ctl_to_enc_id), enc->base.id);
_dpu_rm_clear_mapping(global_state->intf_to_enc_id,
ARRAY_SIZE(global_state->intf_to_enc_id), enc->base.id);
+   _dpu_rm_clear_mapping(global_state->dsc_to_enc_id,
+   ARRAY_SIZE(global_state->dsc_to_enc_id), enc->base.id);
 }

 int dpu_rm_reserve(
@@ -640,6 +667,11 @@ int dpu_rm_get_assigned_resources(struct dpu_rm 
*rm,

hw_to_enc_id = global_state->dspp_to_enc_id;
max_blks = ARRAY_SIZE(rm->dspp_blks);
break;
+   case DPU_HW_BLK_DSC:
+   hw_blks = rm->dsc_blks;
+   hw_to_enc_id = global_state->dsc_to_enc_id;
+   max_blks = ARRAY_SIZE(rm->dsc_blks);
+   break;
default:
DPU_ERROR("blk type %d not managed by rm\n", type);
return 0;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 1f12c8d5b8aa..278d2a510b80 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -30,6 +30,7 @@ struct dpu_rm {
struct dpu_hw_blk *intf_blks[INTF_MAX - INTF_0];
struct dpu_hw_blk *dspp_blks[DSPP_MAX - DSPP_0];
struct dpu_hw_blk *merge_3d_blks[MERGE_3D_MAX - MERGE_3D_0];
+   struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0];

uint32_t lm_max_width;
 };


Re: [PATCH v2 7/7] drm/mediatek: mtk_dsi: Reset the dsi0 hardware

2021-08-02 Thread Chun-Kuang Hu
Hi, Enric:

Enric Balletbo i Serra  於 2021年7月14日 週三 下午6:12寫道:
>
> Reset dsi0 HW to default when power on. This prevents to have different
> settingbetween the bootloader and the kernel.
>
> As not all Mediatek boards have the reset consumer configured in their
> board description, also is not needed on all of them, the reset is optional,
> so the change is compatible with all boards.

Acked-by: Chun-Kuang Hu 

>
> Cc: Jitao Shi 
> Suggested-by: Chun-Kuang Hu 
> Signed-off-by: Enric Balletbo i Serra 
> ---
>
> (no changes since v1)
>
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index ae403c67cbd9..d8b81e2ab841 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -11,6 +11,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -980,8 +981,10 @@ static int mtk_dsi_bind(struct device *dev, struct 
> device *master, void *data)
> struct mtk_dsi *dsi = dev_get_drvdata(dev);
>
> ret = mtk_dsi_encoder_init(drm, dsi);
> +   if (ret)
> +   return ret;
>
> -   return ret;
> +   return device_reset_optional(dev);
>  }
>
>  static void mtk_dsi_unbind(struct device *dev, struct device *master,
> --
> 2.30.2
>


Re: [PATCH v3] drm/mediatek: clear pending flag when cmdq packet is done.

2021-08-02 Thread Chun-Kuang Hu
Hi, Yongqiang:

Yongqiang Niu  於 2021年8月2日 週一 下午3:11寫道:
>
> In cmdq mode, packet may be flushed before it is executed, so
> the pending flag should be cleared after cmdq packet is done.
>
> Signed-off-by: Yongqiang Niu 
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 51 
> +
>  1 file changed, 46 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c 
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 4c25e33..2a2d43e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -267,6 +267,40 @@ static void ddp_cmdq_cb(struct mbox_client *cl, void 
> *mssg)
>  {
> struct mtk_drm_crtc *mtk_crtc = container_of(cl, struct mtk_drm_crtc, 
> cmdq_cl);
> struct cmdq_cb_data *data = mssg;
> +   struct mtk_crtc_state *state;
> +   unsigned int i;
> +
> +   state = to_mtk_crtc_state(mtk_crtc->base.state);
> +
> +   if (state->pending_config) {

No matter pending_config is true or not, it would be false after this.
So we could simply drop this checking.

> +   state->pending_config = false;
> +   }
> +
> +   if (mtk_crtc->pending_planes) {
> +   for (i = 0; i < mtk_crtc->layer_nr; i++) {
> +   struct drm_plane *plane = _crtc->planes[i];
> +   struct mtk_plane_state *plane_state;
> +
> +   plane_state = to_mtk_plane_state(plane->state);
> +
> +   if (plane_state->pending.config)

Ditto.

> +   plane_state->pending.config = false;
> +   }
> +   mtk_crtc->pending_planes = false;
> +   }
> +
> +   if (mtk_crtc->pending_async_planes) {
> +   for (i = 0; i < mtk_crtc->layer_nr; i++) {
> +   struct drm_plane *plane = _crtc->planes[i];
> +   struct mtk_plane_state *plane_state;
> +
> +   plane_state = to_mtk_plane_state(plane->state);
> +
> +   if (plane_state->pending.async_config)

Ditto.

Regards,
Chun-Kuang.

> +   plane_state->pending.async_config = false;
> +   }
> +   mtk_crtc->pending_async_planes = false;
> +   }
>
> mtk_crtc->cmdq_vblank_cnt = 0;
> mtk_drm_cmdq_pkt_destroy(mtk_crtc->cmdq_chan, data->pkt);
> @@ -423,7 +457,8 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc,
> state->pending_vrefresh, 0,
> cmdq_handle);
>
> -   state->pending_config = false;
> +   if (!cmdq_handle)
> +   state->pending_config = false;
> }
>
> if (mtk_crtc->pending_planes) {
> @@ -443,9 +478,12 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc,
> mtk_ddp_comp_layer_config(comp, local_layer,
>   plane_state,
>   cmdq_handle);
> -   plane_state->pending.config = false;
> +   if (!cmdq_handle)
> +   plane_state->pending.config = false;
> }
> -   mtk_crtc->pending_planes = false;
> +
> +   if (!cmdq_handle)
> +   mtk_crtc->pending_planes = false;
> }
>
> if (mtk_crtc->pending_async_planes) {
> @@ -465,9 +503,12 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc,
> mtk_ddp_comp_layer_config(comp, local_layer,
>   plane_state,
>   cmdq_handle);
> -   plane_state->pending.async_config = false;
> +   if (!cmdq_handle)
> +   plane_state->pending.async_config = false;
> }
> -   mtk_crtc->pending_async_planes = false;
> +
> +   if (!cmdq_handle)
> +   mtk_crtc->pending_async_planes = false;
> }
>  }
>
> --
> 1.8.1.1.dirty
>


Re: [Freedreno] [PATCH 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block

2021-08-02 Thread abhinavk

On 2021-07-14 23:51, Vinod Koul wrote:
In SDM845, DSC can be enabled by writing to pingpong block registers, 
so

add support for DSC in hw_pp

Signed-off-by: Vinod Koul 

Reviewed-by: Abhinav Kumar 

---
 .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c   | 32 +++
 .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h   | 14 
 2 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
index 245a7a62b5c6..07fc131ca9aa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
@@ -28,6 +28,9 @@
 #define PP_FBC_MODE 0x034
 #define PP_FBC_BUDGET_CTL   0x038
 #define PP_FBC_LOSSY_MODE   0x03C
+#define PP_DSC_MODE 0x0a0
+#define PP_DCE_DATA_IN_SWAP 0x0ac
+#define PP_DCE_DATA_OUT_SWAP0x0c8

 #define PP_DITHER_EN   0x000
 #define PP_DITHER_BITDEPTH 0x004
@@ -245,6 +248,32 @@ static u32 dpu_hw_pp_get_line_count(struct
dpu_hw_pingpong *pp)
return line;
 }

+static int dpu_hw_pp_dsc_enable(struct dpu_hw_pingpong *pp)
+{
+   struct dpu_hw_blk_reg_map *c = >hw;
+
+   DPU_REG_WRITE(c, PP_DSC_MODE, 1);
+   return 0;
+}
+
+static void dpu_hw_pp_dsc_disable(struct dpu_hw_pingpong *pp)
+{
+   struct dpu_hw_blk_reg_map *c = >hw;
+
+   DPU_REG_WRITE(c, PP_DSC_MODE, 0);
+}
+
+static int dpu_hw_pp_setup_dsc(struct dpu_hw_pingpong *pp)
+{
+   struct dpu_hw_blk_reg_map *pp_c = >hw;
+   int data;
+
+   data = DPU_REG_READ(pp_c, PP_DCE_DATA_OUT_SWAP);
+   data |= BIT(18); /* endian flip */
+   DPU_REG_WRITE(pp_c, PP_DCE_DATA_OUT_SWAP, data);
+   return 0;
+}
+
 static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
unsigned long features)
 {
@@ -256,6 +285,9 @@ static void _setup_pingpong_ops(struct 
dpu_hw_pingpong *c,

c->ops.get_autorefresh = dpu_hw_pp_get_autorefresh_config;
c->ops.poll_timeout_wr_ptr = dpu_hw_pp_poll_timeout_wr_ptr;
c->ops.get_line_count = dpu_hw_pp_get_line_count;
+   c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
+   c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
+   c->ops.disable_dsc = dpu_hw_pp_dsc_disable;

if (test_bit(DPU_PINGPONG_DITHER, ))
c->ops.setup_dither = dpu_hw_pp_setup_dither;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
index 845b9ce80e31..5058e41ffbc0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
@@ -124,6 +124,20 @@ struct dpu_hw_pingpong_ops {
 */
void (*setup_dither)(struct dpu_hw_pingpong *pp,
struct dpu_hw_dither_cfg *cfg);
+   /**
+* Enable DSC
+*/
+   int (*enable_dsc)(struct dpu_hw_pingpong *pp);
+
+   /**
+* Disable DSC
+*/
+   void (*disable_dsc)(struct dpu_hw_pingpong *pp);
+
+   /**
+* Setup DSC
+*/
+   int (*setup_dsc)(struct dpu_hw_pingpong *pp);
 };

 struct dpu_hw_pingpong {


Re: [Freedreno] [PATCH 02/11] drm/msm/disp/dpu1: Add support for DSC

2021-08-02 Thread abhinavk

On 2021-07-14 23:51, Vinod Koul wrote:

Display Stream Compression (DSC) is one of the hw blocks in dpu, so add
support by adding hw blocks for DSC

Signed-off-by: Vinod Koul 
---
Changes since RFC:
 - Drop unused enums

 drivers/gpu/drm/msm/Makefile  |   1 +
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h|  13 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c| 221 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h|  77 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h   |  13 ++
 5 files changed, 325 insertions(+)
 create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
 create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h

diff --git a/drivers/gpu/drm/msm/Makefile 
b/drivers/gpu/drm/msm/Makefile

index 610d630326bb..fd8fc57f1f58 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -61,6 +61,7 @@ msm-y := \
disp/dpu1/dpu_hw_blk.o \
disp/dpu1/dpu_hw_catalog.o \
disp/dpu1/dpu_hw_ctl.o \
+   disp/dpu1/dpu_hw_dsc.o \
disp/dpu1/dpu_hw_interrupts.o \
disp/dpu1/dpu_hw_intf.o \
disp/dpu1/dpu_hw_lm.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 4dfd8a20ad5c..b8b4dc36880c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -547,6 +547,16 @@ struct dpu_merge_3d_cfg  {
const struct dpu_merge_3d_sub_blks *sblk;
 };

+/**
+ * struct dpu_dsc_cfg - information of DSC blocks
+ * @id enum identifying this block
+ * @base   register offset of this block
+ * @features   bit mask identifying sub-blocks/features
+ */
+struct dpu_dsc_cfg {
+   DPU_HW_BLK_INFO;
+};
+
 /**
  * struct dpu_intf_cfg - information of timing engine blocks
  * @id enum identifying this block
@@ -748,6 +758,9 @@ struct dpu_mdss_cfg {
u32 merge_3d_count;
const struct dpu_merge_3d_cfg *merge_3d;

+   u32 dsc_count;
+   struct dpu_dsc_cfg *dsc;
+
u32 intf_count;
const struct dpu_intf_cfg *intf;

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
new file mode 100644
index ..e27e67bd42e8
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
@@ -0,0 +1,221 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, Linaro Limited

Copyright year needs an update : 2020-2021?

+ */
+
+#include "dpu_kms.h"
+#include "dpu_hw_catalog.h"
+#include "dpu_hwio.h"
+#include "dpu_hw_mdss.h"
+#include "dpu_hw_dsc.h"
+
+#define DSC_COMMON_MODE0x000
+#define DSC_ENC 0X004
+#define DSC_PICTURE 0x008
+#define DSC_SLICE   0x00C
+#define DSC_CHUNK_SIZE  0x010
+#define DSC_DELAY   0x014
+#define DSC_SCALE_INITIAL   0x018
+#define DSC_SCALE_DEC_INTERVAL  0x01C
+#define DSC_SCALE_INC_INTERVAL  0x020
+#define DSC_FIRST_LINE_BPG_OFFSET   0x024
+#define DSC_BPG_OFFSET  0x028
+#define DSC_DSC_OFFSET  0x02C
+#define DSC_FLATNESS0x030
+#define DSC_RC_MODEL_SIZE   0x034
+#define DSC_RC  0x038
+#define DSC_RC_BUF_THRESH   0x03C
+#define DSC_RANGE_MIN_QP0x074
+#define DSC_RANGE_MAX_QP0x0B0
+#define DSC_RANGE_BPG_OFFSET0x0EC
+
+static void dpu_hw_dsc_disable(struct dpu_hw_dsc *dsc)
+{
+   struct dpu_hw_blk_reg_map *c = >hw;
+
+   DPU_REG_WRITE(c, DSC_COMMON_MODE, 0);
+}
+
+static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
+ struct msm_display_dsc_config *dsc, u32 mode)
+{
+   struct dpu_hw_blk_reg_map *c = _dsc->hw;
+   u32 data, lsb, bpp;
+   u32 initial_lines = dsc->initial_lines;
+   bool is_cmd_mode = !(mode & BIT(2));
+
+   DPU_REG_WRITE(c, DSC_COMMON_MODE, mode);
+
+   if (is_cmd_mode)
+   initial_lines += 1;
+
+   data = (initial_lines << 20);
+   data |= ((dsc->slice_last_group_size - 1) << 18);
+   /* bpp is 6.4 format, 4 LSBs bits are for fractional part */
+   data |= dsc->drm->bits_per_pixel << 12;
+   lsb = dsc->drm->bits_per_pixel % 4;
+   bpp = dsc->drm->bits_per_pixel / 4;
+   bpp *= 4;
+   bpp <<= 4;
+   bpp |= lsb;
+
+   data |= bpp << 8;
+   data |= (dsc->drm->block_pred_enable << 7);
+   data |= (dsc->drm->line_buf_depth << 3);
+   data |= (dsc->drm->simple_422 << 2);
+   data |= (dsc->drm->convert_rgb << 1);
+   data |= dsc->drm->bits_per_component;
+
+   DPU_REG_WRITE(c, DSC_ENC, data);
+
+   data = dsc->drm->pic_width << 16;
+   data |= dsc->drm->pic_height;
+   DPU_REG_WRITE(c, DSC_PICTURE, data);
+
+   data = dsc->drm->slice_width << 16;
+   data |= 

Re: [PATCH v5] drm/mediatek: add dither 6 setting

2021-08-02 Thread Chun-Kuang Hu
Hi, Yongqiang:

Yongqiang Niu  於 2021年7月20日 週二 下午2:30寫道:
>
> dither 6 setting is missed in a6b7c98afdca
> bit 1 is lfsr_en( "Enables LFSR-type dithering"), need enable
> bit 2 is rdither_en(Enables running order dithering), need disable
>
> Fixes: a6b7c98afdca(drm/mediatek: add mtk_dither_set_common())
> Signed-off-by: Yongqiang Niu 
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c 
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index 99cbf44..7dd8e05 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -26,6 +26,7 @@
>  #define DISP_OD_CFG0x0020
>  #define DISP_OD_SIZE   0x0030
>  #define DISP_DITHER_5  0x0114
> +#define DISP_DITHER_6  0x0118
>  #define DISP_DITHER_7  0x011c
>  #define DISP_DITHER_15 0x013c
>  #define DISP_DITHER_16 0x0140
> @@ -135,6 +136,7 @@ void mtk_dither_set_common(void __iomem *regs, struct 
> cmdq_client_reg *cmdq_reg,
>
> if (bpc >= MTK_MIN_BPC) {
> mtk_ddp_write(cmdq_pkt, 0, cmdq_reg, regs, DISP_DITHER_5);
> +   mtk_ddp_write(cmdq_pkt, 0x3002, cmdq_reg, regs, 
> DISP_DITHER_6);

Symbolized 0x3002. BIT(1) is lfsr_en.

Regards,
Chun-Kuang.

> mtk_ddp_write(cmdq_pkt, 0, cmdq_reg, regs, DISP_DITHER_7);
> mtk_ddp_write(cmdq_pkt,
>   DITHER_LSB_ERR_SHIFT_R(MTK_MAX_BPC - bpc) |
> --
> 1.8.1.1.dirty
>


Re: [Freedreno] [PATCH 01/11] drm/msm/dsi: add support for dsc data

2021-08-02 Thread abhinavk

On 2021-07-14 23:51, Vinod Koul wrote:

Display Stream Compression (DSC) parameters need to be calculated. Add
helpers and struct msm_display_dsc_config in msm_drv for this
msm_display_dsc_config uses drm_dsc_config for DSC parameters.

Signed-off-by: Vinod Koul 
---
Changes since RFC:
 - Drop the DT parsing code
 - Port dsc param calculation from downstream

 drivers/gpu/drm/msm/dsi/dsi_host.c | 133 +
 drivers/gpu/drm/msm/msm_drv.h  |  21 +
 2 files changed, 154 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 8a10e4343281..e1e5d91809b5 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -30,6 +30,8 @@

 #define DSI_RESET_TOGGLE_DELAY_MS 20

+static int dsi_populate_dsc_params(struct msm_display_dsc_config 
*dsc);

+
 static int dsi_get_version(const void __iomem *base, u32 *major, u32 
*minor)

 {
u32 ver;
@@ -156,6 +158,7 @@ struct msm_dsi_host {
struct regmap *sfpb;

struct drm_display_mode *mode;
+   struct msm_display_dsc_config *dsc;

/* connected device info */
struct device_node *device_node;
@@ -1744,6 +1747,136 @@ static int dsi_host_parse_lane_data(struct
msm_dsi_host *msm_host,
return -EINVAL;
 }

+static u32 dsi_dsc_rc_buf_thresh[DSC_NUM_BUF_RANGES - 1] = {
+   0x0e, 0x1c, 0x2a, 0x38, 0x46, 0x54, 0x62,
+   0x69, 0x70, 0x77, 0x79, 0x7b, 0x7d, 0x7e
+};
+
+/* only 8bpc, 8bpp added */
+static char min_qp[DSC_NUM_BUF_RANGES] = {
+   0, 0, 1, 1, 3, 3, 3, 3, 3, 3, 5, 5, 5, 7, 13
+};
+
+static char max_qp[DSC_NUM_BUF_RANGES] = {
+   4, 4, 5, 6, 7, 7, 7, 8, 9, 10, 11, 12, 13, 13, 15
+};
+
+static char bpg_offset[DSC_NUM_BUF_RANGES] = {
+   2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12
+};
+
+static int dsi_populate_dsc_params(struct msm_display_dsc_config *dsc)
+{
+   int mux_words_size;
+   int groups_per_line, groups_total;
+   int min_rate_buffer_size;
+   int hrd_delay;
+   int pre_num_extra_mux_bits, num_extra_mux_bits;
+   int slice_bits;
+   int target_bpp_x16;
+   int data;
+   int final_value, final_scale;
+   int i;
+
+   dsc->drm->rc_model_size = 8192;
+   dsc->drm->first_line_bpg_offset = 12;
+   dsc->drm->rc_edge_factor = 6;
+   dsc->drm->rc_tgt_offset_high = 3;
+   dsc->drm->rc_tgt_offset_low = 3;
+   dsc->drm->simple_422 = 0;
+   dsc->drm->convert_rgb = 1;
+   dsc->drm->vbr_enable = 0;
+
+   /* handle only bpp = bpc = 8 */
+   for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++)
+   dsc->drm->rc_buf_thresh[i] = dsi_dsc_rc_buf_thresh[i];
+
+   for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
+   dsc->drm->rc_range_params[i].range_min_qp = min_qp[i];
+   dsc->drm->rc_range_params[i].range_max_qp = max_qp[i];
+   dsc->drm->rc_range_params[i].range_bpg_offset = bpg_offset[i];
+   }
+
+   dsc->drm->initial_offset = 6144; /* Not bpp 12 */
+   if (dsc->drm->bits_per_pixel != 8)
+   dsc->drm->initial_offset = 2048;  /* bpp = 12 */
+
+   mux_words_size = 48;/* bpc == 8/10 */
+   if (dsc->drm->bits_per_component == 12)
+   mux_words_size = 64;
+
+   dsc->drm->initial_xmit_delay = 512;
+   dsc->drm->initial_scale_value = 32;
+   dsc->drm->first_line_bpg_offset = 12;
+   dsc->drm->line_buf_depth = dsc->drm->bits_per_component + 1;
+
+   /* bpc 8 */
+   dsc->drm->flatness_min_qp = 3;
+   dsc->drm->flatness_max_qp = 12;
+	dsc->det_thresh_flatness = 7 + 2 * (dsc->drm->bits_per_component - 
8);

+   dsc->drm->rc_quant_incr_limit0 = 11;
+   dsc->drm->rc_quant_incr_limit1 = 11;
+   dsc->drm->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC;
+
+	/* FIXME: need to call drm_dsc_compute_rc_parameters() so that rest 
of

+* params are calculated
+*/
+   dsc->slice_last_group_size = 3 - (dsc->drm->slice_width % 3);
+   groups_per_line = DIV_ROUND_UP(dsc->drm->slice_width, 3);
+   dsc->drm->slice_chunk_size = dsc->drm->slice_width *
dsc->drm->bits_per_pixel / 8;
+   if ((dsc->drm->slice_width * dsc->drm->bits_per_pixel) % 8)
+   dsc->drm->slice_chunk_size++;
+
+   /* rbs-min */
+	min_rate_buffer_size =  dsc->drm->rc_model_size - 
dsc->drm->initial_offset +

+   dsc->drm->initial_xmit_delay * 
dsc->drm->bits_per_pixel +
+   groups_per_line * 
dsc->drm->first_line_bpg_offset;
+
+	hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, 
dsc->drm->bits_per_pixel);

+
+	dsc->drm->initial_dec_delay = hrd_delay - 
dsc->drm->initial_xmit_delay;

+
+   dsc->drm->initial_scale_value = 8 * dsc->drm->rc_model_size /
+  (dsc->drm->rc_model_size - 
dsc->drm->initial_offset);
+
+   slice_bits = 8 * dsc->drm->slice_chunk_size * dsc->drm->slice_height;
+
+   

Re: [RESEND] [PATCH v2 1/2] dt-bindings: display: bridge: Add binding for R-Car MIPI DSI/CSI-2 TX

2021-08-02 Thread Rob Herring
On Wed, Jul 28, 2021 at 07:26:39PM +0300, Laurent Pinchart wrote:
> The R-Car MIPI DSI/CSI-2 TX is embedded in the Renesas R-Car V3U SoC. It
> can operate in either DSI or CSI-2 mode, with up to four data lanes.
> 
> Signed-off-by: Laurent Pinchart 
> Reviewed-by: Kieran Bingham 
> ---
> Looks like I forgot to CC the devicetree mailing list and Rob Herring on
> the first try. Resending, sorry about that.
> ---
>  .../display/bridge/renesas,dsi-csi2-tx.yaml   | 118 ++
>  MAINTAINERS   |   1 +
>  2 files changed, 119 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml

Reviewed-by: Rob Herring 

BTW, b4 doesn't like your message-id.


Re: [PATCH 1/9] dt-bindings: phy: mediatek: tphy: support type switch by pericfg

2021-08-02 Thread Rob Herring
On Wed, 28 Jul 2021 15:58:23 +0800, Chunfeng Yun wrote:
> Add support type switch by pericfg register between USB3, PCIe,
> SATA, SGMII, this is used to replace the way through efuse or
> jumper.
> 
> Signed-off-by: Chunfeng Yun 
> ---
>  .../devicetree/bindings/phy/mediatek,tphy.yaml   | 16 
>  1 file changed, 16 insertions(+)
> 

Reviewed-by: Rob Herring 


Re: [PATCH V4 1/2] dt-bindings: display: bridge: lvds-codec: Document LVDS data mapping select

2021-08-02 Thread Rob Herring
On Tue, 27 Jul 2021 18:13:56 +0200, Marek Vasut wrote:
> Decoder input LVDS format is a property of the decoder chip or even
> its strapping. Add DT property data-mapping the same way lvds-panel
> does, to define the LVDS data mapping.
> 
> Signed-off-by: Marek Vasut 
> Cc: Laurent Pinchart 
> Cc: Rob Herring 
> Cc: Sam Ravnborg 
> Cc: devicet...@vger.kernel.org
> To: dri-devel@lists.freedesktop.org
> ---
> V2: - Use allOf
> - Move the data-mapping to endpoint
> V3: - Rebase on V2 submitted a while ago, reinstate changelog
> - Drop the allOf and un-rebase on previous pclk patch
> V4: - port@1, remove $ref: /schemas/graph.yaml#/properties/port and
>   add $ref: /schemas/graph.yaml#/$defs/port-base
> ---
>  .../bindings/display/bridge/lvds-codec.yaml   | 33 ++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring 


RE: [PATCH 13/14] drm/kmb: Enable alpha blended second plane

2021-08-02 Thread Chrisanthus, Anitha
Hi Sam,
Thanks. Where should this go, drm-misc-fixes or drm-misc-next?

Anitha

> -Original Message-
> From: Sam Ravnborg 
> Sent: Wednesday, July 28, 2021 12:29 AM
> To: Chrisanthus, Anitha 
> Cc: dri-devel@lists.freedesktop.org; Dea, Edmund J
> 
> Subject: Re: [PATCH 13/14] drm/kmb: Enable alpha blended second plane
> 
> Hi Anitha,
> On Tue, Jul 27, 2021 at 05:31:25PM -0700, Anitha Chrisanthus wrote:
> > From: Edmund Dea 
> >
> > Enable one additional plane that is alpha blended on top
> > of the primary plane.
> >
> > Signed-off-by: Edmund Dea 
> Your s-o-b is missing.
> 
> With this fixed:
> Acked-by: Sam Ravnborg 
> > ---
> >  drivers/gpu/drm/kmb/kmb_drv.c   |  8 ++--
> >  drivers/gpu/drm/kmb/kmb_plane.c | 81 +-
> ---
> >  drivers/gpu/drm/kmb/kmb_plane.h |  5 +-
> >  drivers/gpu/drm/kmb/kmb_regs.h  |  3 ++
> >  4 files changed, 82 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/kmb/kmb_drv.c
> b/drivers/gpu/drm/kmb/kmb_drv.c
> > index 12f35c43d838..d0de1db03493 100644
> > --- a/drivers/gpu/drm/kmb/kmb_drv.c
> > +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> > @@ -173,10 +173,10 @@ static int kmb_setup_mode_config(struct
> drm_device *drm)
> > ret = drmm_mode_config_init(drm);
> > if (ret)
> > return ret;
> > -   drm->mode_config.min_width = KMB_MIN_WIDTH;
> > -   drm->mode_config.min_height = KMB_MIN_HEIGHT;
> > -   drm->mode_config.max_width = KMB_MAX_WIDTH;
> > -   drm->mode_config.max_height = KMB_MAX_HEIGHT;
> > +   drm->mode_config.min_width = KMB_FB_MIN_WIDTH;
> > +   drm->mode_config.min_height = KMB_FB_MIN_HEIGHT;
> > +   drm->mode_config.max_width = KMB_FB_MAX_WIDTH;
> > +   drm->mode_config.max_height = KMB_FB_MAX_HEIGHT;
> > drm->mode_config.funcs = _mode_config_funcs;
> >
> > ret = kmb_setup_crtc(drm);
> > diff --git a/drivers/gpu/drm/kmb/kmb_plane.c
> b/drivers/gpu/drm/kmb/kmb_plane.c
> > index 4523af949ea1..cbe4e981d73e 100644
> > --- a/drivers/gpu/drm/kmb/kmb_plane.c
> > +++ b/drivers/gpu/drm/kmb/kmb_plane.c
> > @@ -118,9 +118,10 @@ static int kmb_plane_atomic_check(struct
> drm_plane *plane,
> > if (ret)
> > return ret;
> >
> > -   if (new_plane_state->crtc_w > KMB_MAX_WIDTH || new_plane_state-
> >crtc_h > KMB_MAX_HEIGHT)
> > -   return -EINVAL;
> > -   if (new_plane_state->crtc_w < KMB_MIN_WIDTH || new_plane_state-
> >crtc_h < KMB_MIN_HEIGHT)
> > +   if (new_plane_state->crtc_w > KMB_FB_MAX_WIDTH ||
> > +   new_plane_state->crtc_h > KMB_FB_MAX_HEIGHT ||
> > +   new_plane_state->crtc_w < KMB_FB_MIN_WIDTH ||
> > +   new_plane_state->crtc_h < KMB_FB_MIN_HEIGHT)
> > return -EINVAL;
> >
> > /* Due to HW limitations, changing plane height or width after
> > @@ -311,6 +312,44 @@ static void config_csc(struct kmb_drm_private
> *kmb, int plane_id)
> > kmb_write_lcd(kmb, LCD_LAYERn_CSC_OFF3(plane_id),
> csc_coef_lcd[11]);
> >  }
> >
> > +static void kmb_plane_set_alpha(struct kmb_drm_private *kmb,
> > +   const struct drm_plane_state *state,
> > +   unsigned char plane_id,
> > +   unsigned int *val)
> > +{
> > +   u16 plane_alpha = state->alpha;
> > +   u16 pixel_blend_mode = state->pixel_blend_mode;
> > +   int has_alpha = state->fb->format->has_alpha;
> > +
> > +   if (plane_alpha != DRM_BLEND_ALPHA_OPAQUE)
> > +   *val |= LCD_LAYER_ALPHA_STATIC;
> > +
> > +   if (has_alpha) {
> > +   switch (pixel_blend_mode) {
> > +   case DRM_MODE_BLEND_PIXEL_NONE:
> > +   break;
> > +   case DRM_MODE_BLEND_PREMULTI:
> > +   *val |= LCD_LAYER_ALPHA_EMBED |
> LCD_LAYER_ALPHA_PREMULT;
> > +   break;
> > +   case DRM_MODE_BLEND_COVERAGE:
> > +   *val |= LCD_LAYER_ALPHA_EMBED;
> > +   break;
> > +   default:
> > +   DRM_DEBUG("Missing pixel blend mode case (%s ==
> %ld)\n",
> > + __stringify(pixel_blend_mode),
> > + (long)pixel_blend_mode);
> > +   break;
> > +   }
> > +   }
> > +
> > +   if (plane_alpha == DRM_BLEND_ALPHA_OPAQUE && !has_alpha) {
> > +   *val &= LCD_LAYER_ALPHA_DISABLED;
> > +   return;
> > +   }
> > +
> > +   kmb_write_lcd(kmb, LCD_LAYERn_ALPHA(plane_id), plane_alpha);
> > +}
> > +
> >  static void kmb_plane_atomic_update(struct drm_plane *plane,
> > struct drm_atomic_state *state)
> >  {
> > @@ -341,11 +380,12 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
> > fb = new_plane_state->fb;
> > if (!fb)
> > return;
> > +
> > num_planes = fb->format->num_planes;
> > kmb_plane = to_kmb_plane(plane);
> > -   plane_id = kmb_plane->id;
> >
> > kmb = to_kmb(plane->dev);
> > +   plane_id = kmb_plane->id;
> >
> > spin_lock_irq(>irq_lock);
> > if (kmb->kmb_under_flow || 

[PATCH v2] drm/msm/dp: update is_connected status base on sink count at dp_pm_resume()

2021-08-02 Thread Kuogee Hsieh
Currently at dp_pm_resume() is_connected state is decided base on hpd connection
status only. This will put is_connected in wrongly "true" state at the scenario
that dongle attached to DUT but without hmdi cable connecting to it. Fix this
problem by adding read sink count from dongle and decided is_connected state 
base
on both sink count and hpd connection status.

Changes in v2:
-- remove dp_get_sink_count() cand call drm_dp_read_sink_count()

Fixes: d9aa6571b28ba ("drm/msm/dp: check sink_count before update is_connected 
status")
Signed-off-by: Kuogee Hsieh 
---
 drivers/gpu/drm/msm/dp/dp_display.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index 8b69114..6dcb78e 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1403,6 +1403,7 @@ static int dp_pm_resume(struct device *dev)
struct msm_dp *dp_display = platform_get_drvdata(pdev);
struct dp_display_private *dp;
u32 status;
+   int sink_count = 0;
 
dp = container_of(dp_display, struct dp_display_private, dp_display);
 xlog(__func__, 1,0,0, dp->core_initialized, dp_display->power_on);
@@ -1417,15 +1418,26 @@ xlog(__func__, 1,0,0, dp->core_initialized, 
dp_display->power_on);
 
dp_catalog_ctrl_hpd_config(dp->catalog);
 
-   status = dp_catalog_link_is_connected(dp->catalog);
+   /*
+* set sink to normal operation mode -- D0
+* before dpcd read
+*/
+   dp_link_psm_config(dp->link, >panel->link_info, false);
+
+   /* if sink conencted, do dpcd read sink count */
+   if ((status = dp_catalog_link_is_connected(dp->catalog))) {
+   sink_count = drm_dp_read_sink_count(dp->aux);
+   if (sink_count < 0)
+   sink_count = 0;
+   }
 
+   dp->link->sink_count = sink_count;
/*
 * can not declared display is connected unless
 * HDMI cable is plugged in and sink_count of
 * dongle become 1
 */
-xlog(__func__, 0x12,0,0, 0, dp->link->sink_count);
-   if (status && dp->link->sink_count)
+   if (dp->link->sink_count)
dp->dp_display.is_connected = true;
else
dp->dp_display.is_connected = false;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



Re: [PATCH 0/3] drm/amdgpu: modernize virtual display feature

2021-08-02 Thread Alex Deucher
On Tue, Jul 27, 2021 at 8:11 PM Ryan Taylor  wrote:
>
> The amdgpu vkms interface provides a virtual KMS interface for several use
> cases: devices without display hardware, platforms where the actual display
> hardware is not useful (e.g., servers), SR-IOV virtual functions, device
> emulation/simulation, and device bring up prior to display hardware being
> usable. We previously emulated a legacy KMS interface, but there was a desire
> to move to the atomic KMS interface. The vkms driver did everything we
> needed, but we wanted KMS support natively in the driver without buffer
> sharing and the ability to support an instance of VKMS per device. We first
> looked at splitting vkms into a stub driver and a helper module that other
> drivers could use to implement a virtual display, but this strategy ended up
> being messy due to driver specific callbacks needed for buffer management.
> Ultimately, it proved easier to import the vkms code as it mostly used core
> drm helpers anyway.

Series is:
Reviewed-by: Alex Deucher 

>
> Ryan Taylor (3):
>   drm/amdgpu: create amdgpu_vkms (v4)
>   drm/amdgpu: cleanup dce_virtual
>   drm/amdgpu: replace dce_virtual with amdgpu_vkms (v3)
>
>  drivers/gpu/drm/amd/amdgpu/Makefile  |   2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h  |   1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  |   2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c   |   2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 641 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.h |  26 +
>  drivers/gpu/drm/amd/amdgpu/cik.c |  10 +-
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 780 ---
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.h |  30 -
>  drivers/gpu/drm/amd/amdgpu/nv.c  |  22 +-
>  drivers/gpu/drm/amd/amdgpu/si.c  |   8 +-
>  drivers/gpu/drm/amd/amdgpu/soc15.c   |  10 +-
>  drivers/gpu/drm/amd/amdgpu/vi.c  |  14 +-
>  13 files changed, 703 insertions(+), 845 deletions(-)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.h
>  delete mode 100644 drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>  delete mode 100644 drivers/gpu/drm/amd/amdgpu/dce_virtual.h
>
>
> base-commit: e0186426a7efeb506164da7d4a56cfdaea38b380
> --
> 2.32.0
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [BISECTED] 5.14.0-rc4 broke drm/ttm when !CONFIG_DEBUG_FS

2021-08-02 Thread Duncan
[Not subscribed so please CC me.  Manual quoting after using lore's
in-reply-to functionality.  First time doing that so hope I got it
right.]

Mikael Pettersson  wrote...
> Booting 5.14.0-rc4 on my box with Radeon graphics breaks with
> 
> [drm:radeon_ttm_init [radeon]] *ERROR* failed initializing buffer
> object driver(-19).
> radeon :01:00.0: Fatal error during GPU init

Seeing this here too.  amdgpu on polaris-11, on an old amd-fx6100
system.

> after which the screen goes black for the rest of kernel boot
> and early user-space init.

*NOT* seeing that.  However, I have boot messages turned on by default
and I see them as usual, only it stays in vga-console mode instead of
switching to framebuffer after early-boot. I'm guessing MP has a
high-res boot-splash which doesn't work in vga mode, thus the
black-screen until the login shows up.

> Once the console login shows up the screen is in some legacy low-res
> mode and Xorg can't be started.
>
> A git bisect between v5.14-rc3 (good) and v5.14-rc4 (bad) identified
> 
> # first bad commit: [69de4421bb4c103ef42a32bafc596e23918c106f]
> drm/ttm: Initialize debugfs from ttm_global_init()
> 
> Reverting that from 5.14.0-rc4 gives me a working kernel again.
> 
> Note that I have
> # CONFIG_DEBUG_FS is not set

That all matches here, including the unset CONFIG_DEBUG_FS and
confirming the revert on 5.14.0-rc4 works.

-- 
Duncan - HTML messages treated as spam
"They that can give up essential liberty to obtain a little
temporary safety, deserve neither liberty nor safety."
Benjamin Franklin


Re: [PATCH] gpu/drm/amd: Remove duplicated include of drm_drv.h

2021-08-02 Thread Alex Deucher
On Mon, Aug 2, 2021 at 3:32 AM zhouchuangao  wrote:
>
> Duplicate include header file 
> line 28: #include 
> line 44: #include 
>
> Signed-off-by: zhouchuangao 

Applied.  Thanks!

Alex


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 3ec5099..05f864f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -41,8 +41,6 @@
>  #include "amdgpu_securedisplay.h"
>  #include "amdgpu_atomfirmware.h"
>
> -#include 
> -
>  static int psp_sysfs_init(struct amdgpu_device *adev);
>  static void psp_sysfs_fini(struct amdgpu_device *adev);
>
> --
> 2.7.4
>


Re: [Intel-gfx] [PATCH v4 3/7] dyndbg: add dyndbg-bitmap definer and callbacks

2021-08-02 Thread jim . cromie
On Mon, Aug 2, 2021 at 10:24 AM Emil Velikov  wrote:
>
> Hi Jim,
>
> On Sat, 31 Jul 2021 at 22:42, Jim Cromie  wrote:
>
> > +struct dyndbg_bitdesc {
> > +   /* bitpos is inferred from index in containing array */
> > +   char *prefix;
> > +   char *help;
> AFAICT these two should also be constant, right?
>
>
> > +int param_set_dyndbg(const char *instr, const struct kernel_param *kp)
> > +{
> > +   unsigned int val;
> > +   unsigned long changes, result;
> > +   int rc, chgct = 0, totct = 0, bitpos, bitsmax;
> > +   char query[OUR_QUERY_SIZE];
> > +   struct dyndbg_bitdesc *bitmap = (struct dyndbg_bitdesc *) kp->data;
> > +
> > +   // pr_info("set_dyndbg: instr: %s curr: %d\n", instr, *kp->arg);
> Left-over debug code, here and below?

yup, all fixed up locally, with a version that fully works.
thanks.

>
> -Emil


Re: [PATCH v4 2/7] moduleparam: add data member to struct kernel_param

2021-08-02 Thread jim . cromie
On Mon, Aug 2, 2021 at 10:18 AM Emil Velikov  wrote:
>
> Hi Jim,
>
> On Sat, 31 Jul 2021 at 22:42, Jim Cromie  wrote:
>
> > Use of this new data member will be rare, it might be worth redoing
> > this as a separate/sub-type to keep the base case.
> >
> > Signed-off-by: Jim Cromie 
> > ---
> >  include/linux/moduleparam.h | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> > index eed280fae433..e9495b1e794d 100644
> > --- a/include/linux/moduleparam.h
> > +++ b/include/linux/moduleparam.h
> > @@ -78,6 +78,7 @@ struct kernel_param {
> > const struct kparam_string *str;
> > const struct kparam_array *arr;
> > };
> > +   void *data;
>
> Might as well make this "const void *" since it is a compile-time constant?
>

yes indeed. revising.  thanks

> -Emil


[BISECTED] 5.14.0-rc4 broke drm/ttm when !CONFIG_DEBUG_FS

2021-08-02 Thread Mikael Pettersson
Booting 5.14.0-rc4 on my box with Radeon graphics breaks with

[drm:radeon_ttm_init [radeon]] *ERROR* failed initializing buffer
object driver(-19).
radeon :01:00.0: Fatal error during GPU init

after which the screen goes black for the rest of kernel boot and
early user-space init.
Once the console login shows up the screen is in some legacy low-res
mode and Xorg can't be started.

A git bisect between v5.14-rc3 (good) and v5.14-rc4 (bad) identified

# first bad commit: [69de4421bb4c103ef42a32bafc596e23918c106f]
drm/ttm: Initialize debugfs from ttm_global_init()

Reverting that from 5.14.0-rc4 gives me a working kernel again.

Note that I have
# CONFIG_DEBUG_FS is not set

/Mikael


Re: [RFC PATCH v3 1/6] drm/doc: Color Management and HDR10 RFC

2021-08-02 Thread Brian Starkey
Hi,

Thanks for having a stab at this, it's a massive complex topic to
solve.

Do you have the the HTML rendered somewhere for convenience?

On Fri, Jul 30, 2021 at 04:41:29PM -0400, Harry Wentland wrote:
> Use the new DRM RFC doc section to capture the RFC previously only
> described in the cover letter at
> https://patchwork.freedesktop.org/series/89506/
> 
> v3:
>  * Add sections on single-plane and multi-plane HDR
>  * Describe approach to define HW details vs approach to define SW intentions
>  * Link Jeremy Cline's excellent HDR summaries
>  * Outline intention behind overly verbose doc
>  * Describe FP16 use-case
>  * Clean up links
> 
> v2: create this doc
> 
> v1: n/a
> 
> Signed-off-by: Harry Wentland 
> ---
>  Documentation/gpu/rfc/color_intentions.drawio |   1 +
>  Documentation/gpu/rfc/color_intentions.svg|   3 +
>  Documentation/gpu/rfc/colorpipe   |   1 +
>  Documentation/gpu/rfc/colorpipe.svg   |   3 +
>  Documentation/gpu/rfc/hdr-wide-gamut.rst  | 580 ++
>  Documentation/gpu/rfc/index.rst   |   1 +
>  6 files changed, 589 insertions(+)
>  create mode 100644 Documentation/gpu/rfc/color_intentions.drawio
>  create mode 100644 Documentation/gpu/rfc/color_intentions.svg
>  create mode 100644 Documentation/gpu/rfc/colorpipe
>  create mode 100644 Documentation/gpu/rfc/colorpipe.svg
>  create mode 100644 Documentation/gpu/rfc/hdr-wide-gamut.rst
> 

-- snip --

> +
> +Mastering Luminances
> +
> +
> +Even though we are able to describe the absolute luminance of a pixel
> +using the PQ 2084 EOTF we are presented with physical limitations of the
> +display technologies on the market today. Here are a few examples of
> +luminance ranges of displays.
> +
> +.. flat-table::
> +   :header-rows: 1
> +
> +   * - Display
> + - Luminance range in nits
> +
> +   *  - Typical PC display
> +  - 0.3 - 200
> +
> +   *  - Excellent LCD HDTV
> +  - 0.3 - 400
> +
> +   *  - HDR LCD w/ local dimming
> +  - 0.05 - 1,500
> +
> +Since no display can currently show the full 0.0005 to 10,000 nits
> +luminance range of PQ the display will need to tone-map the HDR content,
> +i.e to fit the content within a display's capabilities. To assist
> +with tone-mapping HDR content is usually accompanied by a metadata
> +that describes (among other things) the minimum and maximum mastering
> +luminance, i.e. the maximum and minimum luminance of the display that
> +was used to master the HDR content.
> +
> +The HDR metadata is currently defined on the drm_connector via the
> +hdr_output_metadata blob property.
> +
> +It might be useful to define per-plane hdr metadata, as different planes
> +might have been mastered differently.

I think this only applies to the approach where all the processing is
decided in the kernel right?

If we directly expose each pipeline stage, and userspace controls
everything, there's no need for the kernel to know the mastering
luminance of any of the input content. The kernel would only need to
know the eventual *output* luminance range, which might not even match
any of the input content!


...

> +
> +How are we solving the problem?
> +===
> +
> +Single-plane
> +
> +
> +If a single drm_plane is used no further work is required. The compositor
> +will provide one HDR plane alongside a drm_connector's hdr_output_metadata
> +and the display HW will output this plane without further processing if
> +no CRTC LUTs are provided.
> +
> +If desired a compositor can use the CRTC LUTs for HDR content but without
> +support for PWL or multi-segmented LUTs the quality of the operation is
> +expected to be subpar for HDR content.
> +
> +
> +Multi-plane
> +---
> +
> +In multi-plane configurations we need to solve the problem of blending
> +HDR and SDR content. This blending should be done in linear space and
> +therefore requires framebuffer data that is presented in linear space
> +or a way to convert non-linear data to linear space. Additionally
> +we need a way to define the luminance of any SDR content in relation
> +to the HDR content.
> +

Android doesn't blend in linear space, so any API shouldn't be built
around an assumption of linear blending.

> +In order to present framebuffer data in linear space without losing a
> +lot of precision it needs to be presented using 16 bpc precision.
> +
> +
> +Defining HW Details
> +---
> +
> +One way to take full advantage of modern HW's color pipelines is by
> +defining a "generic" pipeline that matches all capable HW. Something
> +like this, which I took `from Uma Shankar`_ and expanded on:
> +
> +.. _from Uma Shankar: https://patchwork.freedesktop.org/series/90826/
> +
> +.. kernel-figure::  colorpipe.svg

I don't think this pipeline is expressive enough, in part because of
Android's non-linear blending as I mentioned above, but also because
the "tonemapping" block is a bit of a monolithic black-box.

I'd be in 

Re: [PATCH v4 5/7] i915/gvt: control pr_debug("gvt:")s with bits in parameters/debug_gvt

2021-08-02 Thread Emil Velikov
Hi Jim,

On Sat, 31 Jul 2021 at 22:42, Jim Cromie  wrote:

> DYNDBG_BITMAP_DESC(__gvt_debug, "dyndbg bitmap desc",
> { "gvt: cmd: ",  "command processing" },
> { "gvt: core: ", "core help" },
> { "gvt: dpy: ",  "display help" },
> { "gvt: el: ",   "help" },
> { "gvt: irq: ",  "help" },
> { "gvt: mm: ",   "help" },
> { "gvt: mmio: ", "help" },
> { "gvt: render: ", "help" },
> { "gvt: sched: " "help" });
>
Previous commit removed the space after the colon. The above example
needs updating.

This concludes a casual read-through on my end. Hope it helps o/
-Emil


Re: [Intel-gfx] [PATCH v4 3/7] dyndbg: add dyndbg-bitmap definer and callbacks

2021-08-02 Thread Emil Velikov
Hi Jim,

On Sat, 31 Jul 2021 at 22:42, Jim Cromie  wrote:

> +struct dyndbg_bitdesc {
> +   /* bitpos is inferred from index in containing array */
> +   char *prefix;
> +   char *help;
AFAICT these two should also be constant, right?


> +int param_set_dyndbg(const char *instr, const struct kernel_param *kp)
> +{
> +   unsigned int val;
> +   unsigned long changes, result;
> +   int rc, chgct = 0, totct = 0, bitpos, bitsmax;
> +   char query[OUR_QUERY_SIZE];
> +   struct dyndbg_bitdesc *bitmap = (struct dyndbg_bitdesc *) kp->data;
> +
> +   // pr_info("set_dyndbg: instr: %s curr: %d\n", instr, *kp->arg);
Left-over debug code, here and below?

-Emil


Re: [PATCH 42/64] net: qede: Use memset_after() for counters

2021-08-02 Thread Kees Cook
On Mon, Aug 02, 2021 at 02:29:28PM +, Shai Malin wrote:
> 
> On Tue, Jul 31, 2021 at 07:07:00PM -0300, Kees Cook wrote:
> > On Tue, Jul 27, 2021 at 01:58:33PM -0700, Kees Cook wrote:
> > > In preparation for FORTIFY_SOURCE performing compile-time and run-time
> > > field bounds checking for memset(), avoid intentionally writing across
> > > neighboring fields.
> > >
> > > Use memset_after() so memset() doesn't get confused about writing
> > > beyond the destination member that is intended to be the starting point
> > > of zeroing through the end of the struct.
> > >
> > > Signed-off-by: Kees Cook 
> > > ---
> > > The old code seems to be doing the wrong thing: starting from not the
> > > first member, but sized for the whole struct. Which is correct?
> > 
> > Quick ping on this question.
> > 
> > The old code seems to be doing the wrong thing: it starts from the second
> > member and writes beyond int_info, clobbering qede_lock:
> 
> Thanks for highlighting the problem, but actually, the memset is redundant.
> We will remove it so the change will not be needed.
> 
> > 
> > struct qede_dev {
> > ...
> > struct qed_int_info int_info;
> > 
> > /* Smaller private variant of the RTNL lock */
> > struct mutexqede_lock;
> > ...
> > 
> > 
> > struct qed_int_info {
> > struct msix_entry   *msix;
> > u8  msix_cnt;
> > 
> > /* This should be updated by the protocol driver */
> > u8  used_cnt;
> > };
> > 
> > Should this also clear the "msix" member, or should this not write
> > beyond int_info? This patch does the latter.
> 
> It should clear only the msix_cnt, no need to clear the entire 
> qed_int_info structure.

Should used_cnt be cleared too? It is currently. Better yet, what patch
do you suggest I replace this proposed one with? :)

Thanks for looking at this!

-Kees

-- 
Kees Cook


Re: [PATCH v4 2/7] moduleparam: add data member to struct kernel_param

2021-08-02 Thread Emil Velikov
Hi Jim,

On Sat, 31 Jul 2021 at 22:42, Jim Cromie  wrote:

> Use of this new data member will be rare, it might be worth redoing
> this as a separate/sub-type to keep the base case.
>
> Signed-off-by: Jim Cromie 
> ---
>  include/linux/moduleparam.h | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index eed280fae433..e9495b1e794d 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -78,6 +78,7 @@ struct kernel_param {
> const struct kparam_string *str;
> const struct kparam_array *arr;
> };
> +   void *data;

Might as well make this "const void *" since it is a compile-time constant?

-Emil


Re: [PATCH] drm/msm/dp: update is_connected status base on sink count at dp_pm_resume()

2021-08-02 Thread khsieh

On 2021-07-30 11:57, Stephen Boyd wrote:

Quoting Kuogee Hsieh (2021-07-28 14:30:54)
Currently at dp_pm_resume() is_connected state is decided base on hpd 
connection
status only. This will put is_connected in wrongly "true" state at the 
scenario
that dongle attached to DUT but without hmdi cable connecting to it. 
Fix this
problem by adding read sink count from dongle and decided is_connected 
state base

on both sink count and hpd connection status.



Please add a Fixes tag.


Signed-off-by: Kuogee Hsieh 
---
 drivers/gpu/drm/msm/dp/dp_display.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c

index 2b660e9..9bcb261 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1308,6 +1308,17 @@ static int dp_display_remove(struct 
platform_device *pdev)

return 0;
 }

+static int dp_get_sink_count(struct dp_display_private *dp)
+{
+   u8 sink_count;
+
+   sink_count = drm_dp_read_sink_count(dp->aux);


drm_dp_read_sink_count() returns an int, not a u8. Comparing a u8 to
less than zero doesn't make any sense as it isn't signed.


+   if (sink_count < 0)
+   return 0;
+
+   return sink_count;
+}


We can drop this function and just have an int count in dp_pm_resume()
that is compared to < 0 and then ignored.


+
 static int dp_pm_resume(struct device *dev)
 {
struct platform_device *pdev = to_platform_device(dev);
@@ -1327,14 +1338,22 @@ static int dp_pm_resume(struct device *dev)

dp_catalog_ctrl_hpd_config(dp->catalog);

-   status = dp_catalog_link_is_connected(dp->catalog);
+   /*
+* set sink to normal operation mode -- D0
+* before dpcd read
+*/
+   dp_link_psm_config(dp->link, >panel->link_info, false);

+   if ((status = dp_catalog_link_is_connected(dp->catalog)))
+   dp->link->sink_count = dp_get_sink_count(dp);


Do we need to call drm_dp_read_sink_count_cap() as well?

no, we only need sink_count



+   else
+   dp->link->sink_count = 0;
/*
 * can not declared display is connected unless
 * HDMI cable is plugged in and sink_count of
 * dongle become 1
 */
-   if (status && dp->link->sink_count)


Is 'status' used anymore? If not, please remove it.
Yes, it still used which used to decided to perform dpcd read sink count 
or not



+   if (dp->link->sink_count)
dp->dp_display.is_connected = true;
else
dp->dp_display.is_connected = false;


[PATCH 9/9] drm/i915: Split out intel_context_create_user

2021-08-02 Thread Daniel Vetter
There's quite a fundamental difference between userspace contexts, and
kernel contexts. Latter all share intel_gt->vm, former get their vm
from gem_ctx->vm (on full ppgtt at least).

By splitting context creation for userspace from kernel-internal ones
we can make this all a bit more strict and WARN_ON if there's a vm
already set in intel_context_set_gem().

All this is only possible because gem_ctx cannot chance their VM
anymore since

commit ccbc1b97948ab671335e950271e39766729736c3
Author: Jason Ekstrand 
Date:   Thu Jul 8 10:48:30 2021 -0500

drm/i915/gem: Don't allow changing the VM on running contexts (v4)

Signed-off-by: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Chris Wilson 
Cc: Maarten Lankhorst 
Cc: Joonas Lahtinen 
Cc: Daniel Vetter 
Cc: "Thomas Hellström" 
Cc: Matthew Auld 
Cc: Lionel Landwerlin 
Cc: Dave Airlie 
Cc: Jason Ekstrand 
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c   |  6 ++---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c|  4 +++-
 .../gpu/drm/i915/gem/selftests/mock_context.c |  2 +-
 drivers/gpu/drm/i915/gt/intel_context.c   | 22 +--
 drivers/gpu/drm/i915/gt/intel_context.h   |  2 ++
 5 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 2f3cc73d4710..13358e6749d9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -746,7 +746,7 @@ static int intel_context_set_gem(struct intel_context *ce,
 
ce->ring_size = SZ_16K;
 
-   i915_vm_put(ce->vm);
+   WARN_ON(ce->vm);
ce->vm = i915_gem_context_get_eb_vm(ctx);
 
if (ctx->sched.priority >= I915_PRIORITY_NORMAL &&
@@ -856,7 +856,7 @@ static struct i915_gem_engines *default_engines(struct 
i915_gem_context *ctx,
GEM_BUG_ON(engine->legacy_idx >= I915_NUM_ENGINES);
GEM_BUG_ON(e->engines[engine->legacy_idx]);
 
-   ce = intel_context_create(engine);
+   ce = intel_context_create_user(engine);
if (IS_ERR(ce)) {
err = ERR_CAST(ce);
goto free_engines;
@@ -897,7 +897,7 @@ static struct i915_gem_engines *user_engines(struct 
i915_gem_context *ctx,
 
switch (pe[n].type) {
case I915_GEM_ENGINE_TYPE_PHYSICAL:
-   ce = intel_context_create(pe[n].engine);
+   ce = intel_context_create_user(pe[n].engine);
break;
 
case I915_GEM_ENGINE_TYPE_BALANCED:
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index bdf2b5785a81..54de94433365 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -30,15 +30,17 @@
 
 struct eb_vma {
struct i915_vma *vma;
+   struct drm_i915_gem_object *obj;
unsigned int flags;
 
+   u32 handle;
+
/** This vma's place in the execbuf reservation list */
struct drm_i915_gem_exec_object2 *exec;
struct list_head bind_link;
struct list_head reloc_link;
 
struct hlist_node node;
-   u32 handle;
 };
 
 enum {
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c 
b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
index fee070df1c97..e5efda1058a3 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
@@ -124,7 +124,7 @@ live_context_for_engine(struct intel_engine_cs *engine, 
struct file *file)
return ctx;
}
 
-   ce = intel_context_create(engine);
+   ce = intel_context_create_user(engine);
if (IS_ERR(ce)) {
__free_engines(engines, 0);
return ERR_CAST(ce);
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c 
b/drivers/gpu/drm/i915/gt/intel_context.c
index 745e84c72c90..9e33efb594dd 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -34,6 +34,23 @@ void intel_context_free(struct intel_context *ce)
call_rcu(>rcu, rcu_context_free);
 }
 
+/* for user contexts, callers must set ce->vm correctly */
+struct intel_context *
+intel_context_create_user(struct intel_engine_cs *engine)
+{
+   struct intel_context *ce;
+
+   ce = intel_context_alloc();
+   if (!ce)
+   return ERR_PTR(-ENOMEM);
+
+   intel_context_init(ce, engine);
+
+   trace_intel_context_create(ce);
+   return ce;
+}
+
+/* for kernel-internal users only, sets ce->vm to intel_gt.vm */
 struct intel_context *
 intel_context_create(struct intel_engine_cs *engine)
 {
@@ -44,6 +61,8 @@ intel_context_create(struct intel_engine_cs *engine)
return ERR_PTR(-ENOMEM);
 
intel_context_init(ce, engine);
+   ce->vm = i915_vm_get(engine->gt->vm);
+

[PATCH 8/9] drm/i915: Stop rcu support for i915_address_space

2021-08-02 Thread Daniel Vetter
The full audit is quite a bit of work:

- i915_dpt has very simple lifetime (somehow we create a display pagetable vm
  per object, so its _very_ simple, there's only ever a single vma in there),
  and uses i915_vm_close(), which internally does a i915_vm_put(). No rcu.

  Aside: wtf is i915_dpt doing in the intel_display.c garbage collector as a new
  feature, instead of added as a separate file with some clean-ish interface.

  Also, i915_dpt unfortunately re-introduces some coding patterns from
  pre-dma_resv_lock conversion times.

- i915_gem_proto_ctx is fully refcounted and no rcu, all protected by
  fpriv->proto_context_lock.

- i915_gem_context is itself rcu protected, and that might leak to anything it
  points at. Before

commit cf977e18610e66e48c31619e7e0cfa871be9eada
Author: Chris Wilson 
Date:   Wed Dec 2 11:21:40 2020 +

drm/i915/gem: Spring clean debugfs

  and

commit db80a1294c231b6ac725085f046bb2931e00c9db
Author: Chris Wilson 
Date:   Mon Jan 18 11:08:54 2021 +

drm/i915/gem: Remove per-client stats from debugfs/i915_gem_objects

  we had a bunch of debugfs files that relied on rcu protecting everything, but
  those are gone now. The main one was removed even earlier with

  There doesn't seem to be anything left that's actually protecting
  stuff now that the ctx->vm itself is invariant. See

commit ccbc1b97948ab671335e950271e39766729736c3
Author: Jason Ekstrand 
Date:   Thu Jul 8 10:48:30 2021 -0500

drm/i915/gem: Don't allow changing the VM on running contexts (v4)

  Note that we drop the vm refcount before the final release of the gem context
  refcount, so this is all very dangerous even without rcu. Note that aside from
  later on creating new engines (a defunct feature) and debug output we're never
  looked at gem_ctx->vm for anything functional, hence why this is ok.
  Fingers crossed.

  Preceeding patches removed all vestiges of rcu use from gem_ctx->vm
  derferencing to make it clear it's really not used.

  The gem_ctx->rcu protection was introduced in

commit a4e7ccdac38ec8335d9e4e2656c1a041c77feae1
Author: Chris Wilson 
Date:   Fri Oct 4 14:40:09 2019 +0100

drm/i915: Move context management under GEM

  The commit message is somewhat entertaining because it fails to
  mention this fact completely, and compensates that by an in-commit
  changelog entry that claims that ctx->vm is protected by ctx->mutex.
  Which was the case _before_ this commit, but no longer after it.

- intel_context holds a full reference. Unfortunately intel_context is also rcu
  protected and the reference to the ->vm is dropped before the
  rcu barrier - only the kfree is delayed. So again we need to check
  whether that leaks anywhere on the intel_context->vm. RCU is only
  used to protect intel_context sitting on the breadcrumb lists, which
  don't look at the vm anywhere, so we are fine.

  Nothing else relies on rcu protection of intel_context and hence is
  fully protected by the kref refcount alone, which protects
  intel_context->vm in turn.

  The breadcrumbs rcu usage was added in

commit c744d50363b714783bbc88d986cc16def13710f7
Author: Chris Wilson 
Date:   Thu Nov 26 14:04:06 2020 +

drm/i915/gt: Split the breadcrumb spinlock between global and 
contexts

  its parent commit added the intel_context rcu protection:

commit 14d1eaf08845c534963c83f754afe0cb14cb2512
Author: Chris Wilson 
Date:   Thu Nov 26 14:04:05 2020 +

drm/i915/gt: Protect context lifetime with RCU

  given some credence to my claim that I've actually caught them all.

- drm_i915_gem_object's shares_resv_from pointer has a full refcount to the
  dma_resv, which is a sub-refcount that's released after the final
  i915_vm_put() has been called. Safe.

  Aside: Maybe we should have a struct dma_resv_shared which is just dma_resv +
  kref as a stand-alone thing. It's a pretty useful pattern which other drivers
  might want to copy.

  For a bit more context see

commit 4d8151ae5329cf50781a02fd2298a909589a5bab
Author: Thomas Hellström 
Date:   Tue Jun 1 09:46:41 2021 +0200

drm/i915: Don't free shared locks while shared

- the fpriv->vm_xa was relying on rcu_read_lock for lookup, but that
  was updated in a prep patch too to just be a spinlock-protected
  lookup.

- intel_gt->vm is set at driver load in intel_gt_init() and released
  in intel_gt_driver_release(). There seems to be some issue that
  in some error paths this is called twice, but otherwise no rcu to be
  found anywhere. This was added in the below commit, which
  unfortunately doesn't explain why this complication exists.

commit e6ba76480299a0d77c51d846f7467b1673aad25b
Author: Chris Wilson 
Date:   Sat Dec 21 16:03:24 2019 +

drm/i915: Remove 

[PATCH 6/9] drm/i915: Drop __rcu from gem_context->vm

2021-08-02 Thread Daniel Vetter
It's been invariant since

commit ccbc1b97948ab671335e950271e39766729736c3
Author: Jason Ekstrand 
Date:   Thu Jul 8 10:48:30 2021 -0500

drm/i915/gem: Don't allow changing the VM on running contexts (v4)

this just completes the deed. I've tried to split out prep work for
more careful review as much as possible, this is what's left:

- get_ppgtt gets simplified since we don't need to grab a temporary
  reference - we can rely on the temporary reference for the gem_ctx
  while we inspect the vm. The new vm_id still needs a full
  i915_vm_open ofc. This also removes the final caller of context_get_vm_rcu

- A pile of selftests can now just look at ctx->vm instead of
  rcu_dereference_protected( , true) or similar things.

- All callers of i915_gem_context_vm also disappear.

- I've changed the hugepage selftest to set scrub_64K without any
  locking, because when we inspect that setting we're also not taking
  any locks either. It works because it's a selftests that's careful
  (single threaded gives you nice ordering) and not a live driver
  where races can happen from anywhere.

These can only be split up further if we have some intermediate state
with a bunch more rcu_dereference_protected(ctx->vm, true), just to
shut up lockdep and sparse.

The conversion to __rcu happened in

commit a4e7ccdac38ec8335d9e4e2656c1a041c77feae1
Author: Chris Wilson 
Date:   Fri Oct 4 14:40:09 2019 +0100

drm/i915: Move context management under GEM

Note that we're not breaking the actual bugfix in there: The real
bugfix is pushing the i915_vm_relase onto a separate worker, to avoid
locking inversion issues. The rcu conversion was just thrown in for
entertainment value on top (no vm lookup isn't even close to anything
that's a hotpath where removing the single spinlock can be measured).

Signed-off-by: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Chris Wilson 
Cc: Maarten Lankhorst 
Cc: Joonas Lahtinen 
Cc: Daniel Vetter 
Cc: "Thomas Hellström" 
Cc: Matthew Auld 
Cc: Lionel Landwerlin 
Cc: Dave Airlie 
Cc: Jason Ekstrand 
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c   | 53 ++-
 drivers/gpu/drm/i915/gem/i915_gem_context.h   | 14 ++---
 .../gpu/drm/i915/gem/i915_gem_context_types.h |  2 +-
 .../gpu/drm/i915/gem/selftests/huge_pages.c   |  4 +-
 .../drm/i915/gem/selftests/i915_gem_context.c | 24 -
 drivers/gpu/drm/i915/i915_trace.h |  2 +-
 drivers/gpu/drm/i915/selftests/i915_vma.c |  2 +-
 7 files changed, 21 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index fd24a1236682..2f3cc73d4710 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -735,44 +735,6 @@ static int set_proto_ctx_param(struct 
drm_i915_file_private *fpriv,
return ret;
 }
 
-static struct i915_address_space *
-context_get_vm_rcu(struct i915_gem_context *ctx)
-{
-   GEM_BUG_ON(!rcu_access_pointer(ctx->vm));
-
-   do {
-   struct i915_address_space *vm;
-
-   /*
-* We do not allow downgrading from full-ppgtt [to a shared
-* global gtt], so ctx->vm cannot become NULL.
-*/
-   vm = rcu_dereference(ctx->vm);
-   if (!kref_get_unless_zero(>ref))
-   continue;
-
-   /*
-* This ppgtt may have be reallocated between
-* the read and the kref, and reassigned to a third
-* context. In order to avoid inadvertent sharing
-* of this ppgtt with that third context (and not
-* src), we have to confirm that we have the same
-* ppgtt after passing through the strong memory
-* barrier implied by a successful
-* kref_get_unless_zero().
-*
-* Once we have acquired the current ppgtt of ctx,
-* we no longer care if it is released from ctx, as
-* it cannot be reallocated elsewhere.
-*/
-
-   if (vm == rcu_access_pointer(ctx->vm))
-   return rcu_pointer_handoff(vm);
-
-   i915_vm_put(vm);
-   } while (1);
-}
-
 static int intel_context_set_gem(struct intel_context *ce,
 struct i915_gem_context *ctx,
 struct intel_sseu sseu)
@@ -1193,7 +1155,7 @@ static void context_close(struct i915_gem_context *ctx)
 
set_closed_name(ctx);
 
-   vm = i915_gem_context_vm(ctx);
+   vm = ctx->vm;
if (vm)
i915_vm_close(vm);
 
@@ -1350,7 +1312,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
vm = >vm;
}
if (vm) {
-   RCU_INIT_POINTER(ctx->vm, i915_vm_open(vm));
+   ctx->vm = i915_vm_open(vm);
 
/* i915_vm_open() takes a 

[PATCH 7/9] drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups

2021-08-02 Thread Daniel Vetter
We don't need the absolute speed of rcu for this. And
i915_address_space in general dont need rcu protection anywhere else,
after we've made gem contexts and engines a lot more immutable.

Note that this semantically reverts

commit aabbe344dc3ca5f7d8263a02608ba6179e8a4499
Author: Chris Wilson 
Date:   Fri Aug 30 19:03:25 2019 +0100

drm/i915: Use RCU for unlocked vm_idr lookup

except we have the conversion from idr to xarray in between.

Signed-off-by: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Chris Wilson 
Cc: Maarten Lankhorst 
Cc: Joonas Lahtinen 
Cc: Daniel Vetter 
Cc: "Thomas Hellström" 
Cc: Matthew Auld 
Cc: Lionel Landwerlin 
Cc: Dave Airlie 
Cc: Jason Ekstrand 
---
 drivers/gpu/drm/i915/i915_drv.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1488d166d91c..df2d723c894a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1880,11 +1880,11 @@ i915_gem_vm_lookup(struct drm_i915_file_private 
*file_priv, u32 id)
 {
struct i915_address_space *vm;
 
-   rcu_read_lock();
+   xa_lock(_priv->vm_xa);
vm = xa_load(_priv->vm_xa, id);
if (vm && !kref_get_unless_zero(>ref))
vm = NULL;
-   rcu_read_unlock();
+   xa_unlock(_priv->vm_xa);
 
return vm;
 }
-- 
2.32.0



[PATCH 5/9] drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem

2021-08-02 Thread Daniel Vetter
Since

commit ccbc1b97948ab671335e950271e39766729736c3
Author: Jason Ekstrand 
Date:   Thu Jul 8 10:48:30 2021 -0500

drm/i915/gem: Don't allow changing the VM on running contexts (v4)

the gem_ctx->vm can't change anymore. Plus we always set the
intel_context->vm, so might as well use the helper we have for that.

This makes it very clear that we always overwrite intel_context->vm
for userspace contexts, since the default is gt->vm, which is
explicitly reserved for kernel context use. It would be good to split
things up a bit further and avoid any possibility for an accident
where we run kernel stuff in userspace vm or the other way round.

Signed-off-by: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Chris Wilson 
Cc: Maarten Lankhorst 
Cc: Joonas Lahtinen 
Cc: Daniel Vetter 
Cc: "Thomas Hellström" 
Cc: Matthew Auld 
Cc: Lionel Landwerlin 
Cc: Dave Airlie 
Cc: Jason Ekstrand 
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index a80b06c98dba..fd24a1236682 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -784,16 +784,8 @@ static int intel_context_set_gem(struct intel_context *ce,
 
ce->ring_size = SZ_16K;
 
-   if (rcu_access_pointer(ctx->vm)) {
-   struct i915_address_space *vm;
-
-   rcu_read_lock();
-   vm = context_get_vm_rcu(ctx); /* hmm */
-   rcu_read_unlock();
-
-   i915_vm_put(ce->vm);
-   ce->vm = vm;
-   }
+   i915_vm_put(ce->vm);
+   ce->vm = i915_gem_context_get_eb_vm(ctx);
 
if (ctx->sched.priority >= I915_PRIORITY_NORMAL &&
intel_engine_has_timeslices(ce->engine) &&
-- 
2.32.0



[PATCH 4/9] drm/i915: Add i915_gem_context_is_full_ppgtt

2021-08-02 Thread Daniel Vetter
And use it anywhere we have open-coded checks for ctx->vm that really
only check for full ppgtt.

Plus for paranoia add a GEM_BUG_ON that checks it's really only set
when we have full ppgtt, just in case. gem_context->vm is different
since it's NULL in ggtt mode, unlike intel_context->vm or gt->vm,
which is always set.

Signed-off-by: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Chris Wilson 
Cc: Maarten Lankhorst 
Cc: Joonas Lahtinen 
Cc: Daniel Vetter 
Cc: "Thomas Hellström" 
Cc: Matthew Auld 
Cc: Lionel Landwerlin 
Cc: Dave Airlie 
Cc: Jason Ekstrand 
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c   | 2 +-
 drivers/gpu/drm/i915/gem/i915_gem_context.h   | 7 +++
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c| 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 4 ++--
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 6263563e15d6..a80b06c98dba 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1581,7 +1581,7 @@ static int get_ppgtt(struct drm_i915_file_private 
*file_priv,
int err;
u32 id;
 
-   if (!rcu_access_pointer(ctx->vm))
+   if (!i915_gem_context_is_full_ppgtt(ctx))
return -ENODEV;
 
rcu_read_lock();
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h 
b/drivers/gpu/drm/i915/gem/i915_gem_context.h
index da6e8b506d96..37536a260e6e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h
@@ -154,6 +154,13 @@ i915_gem_context_vm(struct i915_gem_context *ctx)
return rcu_dereference_protected(ctx->vm, lockdep_is_held(>mutex));
 }
 
+static inline bool i915_gem_context_is_full_ppgtt(struct i915_gem_context *ctx)
+{
+   GEM_BUG_ON(!!rcu_access_pointer(ctx->vm) != HAS_FULL_PPGTT(ctx->i915));
+
+   return !!rcu_access_pointer(ctx->vm);
+}
+
 static inline struct i915_address_space *
 i915_gem_context_get_eb_vm(struct i915_gem_context *ctx)
 {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 69e47b97d786..bdf2b5785a81 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -749,7 +749,7 @@ static int eb_select_context(struct i915_execbuffer *eb)
return PTR_ERR(ctx);
 
eb->gem_context = ctx;
-   if (rcu_access_pointer(ctx->vm))
+   if (i915_gem_context_is_full_ppgtt(ctx))
eb->invalid_flags |= EXEC_OBJECT_NEEDS_GTT;
 
return 0;
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index d436ce7fa25c..5442b8e59629 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -838,7 +838,7 @@ static int igt_shared_ctx_exec(void *arg)
pr_err("Failed to fill dword %lu [%lu/%lu] with 
gpu (%s) [full-ppgtt? %s], err=%d\n",
   ndwords, dw, max_dwords(obj),
   engine->name,
-  yesno(!!rcu_access_pointer(ctx->vm)),
+  
yesno(i915_gem_context_is_full_ppgtt(ctx)),
   err);
intel_context_put(ce);
kernel_context_close(ctx);
@@ -1417,7 +1417,7 @@ static int igt_ctx_readonly(void *arg)
pr_err("Failed to fill dword %lu [%lu/%lu] with 
gpu (%s) [full-ppgtt? %s], err=%d\n",
   ndwords, dw, max_dwords(obj),
   ce->engine->name,
-  yesno(!!ctx_vm(ctx)),
+  
yesno(i915_gem_context_is_full_ppgtt(ctx)),
   err);
i915_gem_context_unlock_engines(ctx);
goto out_file;
-- 
2.32.0



[PATCH 3/9] drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam

2021-08-02 Thread Daniel Vetter
Consolidates the "which is the vm my execbuf runs in" code a bit. We
do some get/put which isn't really required, but all the other users
want the refcounting, and I figured doing a function just for this
getparam to avoid 2 atomis is a bit much.

Signed-off-by: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Chris Wilson 
Cc: Maarten Lankhorst 
Cc: Joonas Lahtinen 
Cc: Daniel Vetter 
Cc: "Thomas Hellström" 
Cc: Matthew Auld 
Cc: Lionel Landwerlin 
Cc: Dave Airlie 
Cc: Jason Ekstrand 
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index cff72679ad7c..6263563e15d6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -2124,6 +2124,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device 
*dev, void *data,
struct drm_i915_file_private *file_priv = file->driver_priv;
struct drm_i915_gem_context_param *args = data;
struct i915_gem_context *ctx;
+   struct i915_address_space *vm;
int ret = 0;
 
ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
@@ -2133,12 +2134,10 @@ int i915_gem_context_getparam_ioctl(struct drm_device 
*dev, void *data,
switch (args->param) {
case I915_CONTEXT_PARAM_GTT_SIZE:
args->size = 0;
-   rcu_read_lock();
-   if (rcu_access_pointer(ctx->vm))
-   args->value = rcu_dereference(ctx->vm)->total;
-   else
-   args->value = to_i915(dev)->ggtt.vm.total;
-   rcu_read_unlock();
+   vm = i915_gem_context_get_eb_vm(ctx);
+   args->value = vm->total;
+   i915_vm_put(vm);
+
break;
 
case I915_CONTEXT_PARAM_NO_ERROR_CAPTURE:
-- 
2.32.0



[PATCH 2/9] drm/i915: Rename i915_gem_context_get_vm_rcu to i915_gem_context_get_eb_vm

2021-08-02 Thread Daniel Vetter
The important part isn't so much that this does an rcu lookup - that's
more an implementation detail, which will also be removed.

The thing that makes this different from other functions is that it's
gettting you the vm that batchbuffers will run in for that gem
context, which is either a full ppgtt stored in gem->ctx, or the ggtt.

We'll make more use of this function later on.

Signed-off-by: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Chris Wilson 
Cc: Maarten Lankhorst 
Cc: Joonas Lahtinen 
Cc: Daniel Vetter 
Cc: "Thomas Hellström" 
Cc: Matthew Auld 
Cc: Lionel Landwerlin 
Cc: Dave Airlie 
Cc: Jason Ekstrand 
---
 drivers/gpu/drm/i915/gem/i915_gem_context.h   | 2 +-
 drivers/gpu/drm/i915/gem/selftests/huge_pages.c   | 4 ++--
 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 4 ++--
 drivers/gpu/drm/i915/gt/selftest_execlists.c  | 2 +-
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c  | 2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 4 ++--
 drivers/gpu/drm/i915/selftests/i915_vma.c | 2 +-
 7 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h 
b/drivers/gpu/drm/i915/gem/i915_gem_context.h
index 18060536b0c2..da6e8b506d96 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h
@@ -155,7 +155,7 @@ i915_gem_context_vm(struct i915_gem_context *ctx)
 }
 
 static inline struct i915_address_space *
-i915_gem_context_get_vm_rcu(struct i915_gem_context *ctx)
+i915_gem_context_get_eb_vm(struct i915_gem_context *ctx)
 {
struct i915_address_space *vm;
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c 
b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index a094f3ce1a90..6c68fe26bb32 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1456,7 +1456,7 @@ static int igt_tmpfs_fallback(void *arg)
struct i915_gem_context *ctx = arg;
struct drm_i915_private *i915 = ctx->i915;
struct vfsmount *gemfs = i915->mm.gemfs;
-   struct i915_address_space *vm = i915_gem_context_get_vm_rcu(ctx);
+   struct i915_address_space *vm = i915_gem_context_get_eb_vm(ctx);
struct drm_i915_gem_object *obj;
struct i915_vma *vma;
u32 *vaddr;
@@ -1512,7 +1512,7 @@ static int igt_shrink_thp(void *arg)
 {
struct i915_gem_context *ctx = arg;
struct drm_i915_private *i915 = ctx->i915;
-   struct i915_address_space *vm = i915_gem_context_get_vm_rcu(ctx);
+   struct i915_address_space *vm = i915_gem_context_get_eb_vm(ctx);
struct drm_i915_gem_object *obj;
struct i915_gem_engines_iter it;
struct intel_context *ce;
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index 8eb5050f8cb3..d436ce7fa25c 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -1528,7 +1528,7 @@ static int write_to_scratch(struct i915_gem_context *ctx,
 
intel_gt_chipset_flush(engine->gt);
 
-   vm = i915_gem_context_get_vm_rcu(ctx);
+   vm = i915_gem_context_get_eb_vm(ctx);
vma = i915_vma_instance(obj, vm, NULL);
if (IS_ERR(vma)) {
err = PTR_ERR(vma);
@@ -1607,7 +1607,7 @@ static int read_from_scratch(struct i915_gem_context *ctx,
if (GRAPHICS_VER(i915) >= 8) {
const u32 GPR0 = engine->mmio_base + 0x600;
 
-   vm = i915_gem_context_get_vm_rcu(ctx);
+   vm = i915_gem_context_get_eb_vm(ctx);
vma = i915_vma_instance(obj, vm, NULL);
if (IS_ERR(vma)) {
err = PTR_ERR(vma);
diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c 
b/drivers/gpu/drm/i915/gt/selftest_execlists.c
index f12ffe797639..b3863abc51f5 100644
--- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
+++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
@@ -3493,7 +3493,7 @@ static int smoke_submit(struct preempt_smoke *smoke,
if (batch) {
struct i915_address_space *vm;
 
-   vm = i915_gem_context_get_vm_rcu(ctx);
+   vm = i915_gem_context_get_eb_vm(ctx);
vma = i915_vma_instance(batch, vm, NULL);
i915_vm_put(vm);
if (IS_ERR(vma))
diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c 
b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 08f011f893b2..6023c418ee8a 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -117,7 +117,7 @@ static struct i915_request *
 hang_create_request(struct hang *h, struct intel_engine_cs *engine)
 {
struct intel_gt *gt = h->gt;
-   struct i915_address_space *vm = i915_gem_context_get_vm_rcu(h->ctx);
+   struct i915_address_space *vm = i915_gem_context_get_eb_vm(h->ctx);

[PATCH 1/9] drm/i915: Drop code to handle set-vm races from execbuf

2021-08-02 Thread Daniel Vetter
Changing the vm from a finalized gem ctx is no longer possible, which
means we don't have to check for that anymore.

I was pondering whether to keep the check as a WARN_ON, but things go
boom real bad real fast if the vm of a vma is wrong. Plus we'd need to
also get the ggtt vm for !full-ppgtt platforms. Ditching it all seemed
like a better idea.

References: ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running 
contexts (v4)")
Signed-off-by: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Chris Wilson 
Cc: Maarten Lankhorst 
Cc: Joonas Lahtinen 
Cc: Daniel Vetter 
Cc: "Thomas Hellström" 
Cc: Matthew Auld 
Cc: Lionel Landwerlin 
Cc: Dave Airlie 
Cc: Jason Ekstrand 
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 538d9d2e52b7..69e47b97d786 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -775,11 +775,7 @@ static int __eb_add_lut(struct i915_execbuffer *eb,
/* Check that the context hasn't been closed in the meantime */
err = -EINTR;
if (!mutex_lock_interruptible(>lut_mutex)) {
-   struct i915_address_space *vm = rcu_access_pointer(ctx->vm);
-
-   if (unlikely(vm && vma->vm != vm))
-   err = -EAGAIN; /* user racing with ctx set-vm */
-   else if (likely(!i915_gem_context_is_closed(ctx)))
+   if (likely(!i915_gem_context_is_closed(ctx)))
err = radix_tree_insert(>handles_vma, handle, vma);
else
err = -ENOENT;
-- 
2.32.0



[PATCH 0/9] remove rcu support from i915_address_space

2021-08-02 Thread Daniel Vetter
Hi all,

Jason wanted to do that as part of the scheduler series, but I object
since rcu is very, very hard to review when adding, and much, much harder
even to review when removing.

This is because simply looking for __rcu pointer annotations and rcu
functions isn't enough, rcu is also relied upon in many datastructures
which have internally and rcu_read_lock protection (or at least the
required amount of barriers), like xarray.

The other problem is that it inherits when chasing pointers, e.g.
i915_gem_engines has an rcu pointer to intel_context, which has a non-rcu
pointer to i915_address_space. But since we could look-up the entire chain
under rcu i.e. engines->context[i]->vm this means more code to audit.

The audit explodes pretty quickly.

Anyway I'm reasonable confident I got them all in the current code, and
slightly less confident that I managed to stitch together the full
history.

References to relevant commits throughout the series.

Cheers, Daniel

Daniel Vetter (9):
  drm/i915: Drop code to handle set-vm races from execbuf
  drm/i915: Rename i915_gem_context_get_vm_rcu to
i915_gem_context_get_eb_vm
  drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam
  drm/i915: Add i915_gem_context_is_full_ppgtt
  drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem
  drm/i915: Drop __rcu from gem_context->vm
  drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups
  drm/i915: Stop rcu support for i915_address_space
  drm/i915: Split out intel_context_create_user

 drivers/gpu/drm/i915/gem/i915_gem_context.c   | 82 ---
 drivers/gpu/drm/i915/gem/i915_gem_context.h   | 13 ++-
 .../gpu/drm/i915/gem/i915_gem_context_types.h |  2 +-
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 12 ++-
 .../gpu/drm/i915/gem/selftests/huge_pages.c   |  8 +-
 .../drm/i915/gem/selftests/i915_gem_context.c | 32 +++-
 .../gpu/drm/i915/gem/selftests/mock_context.c |  2 +-
 drivers/gpu/drm/i915/gt/intel_context.c   | 22 -
 drivers/gpu/drm/i915/gt/intel_context.h   |  2 +
 drivers/gpu/drm/i915/gt/intel_ggtt.c  |  1 -
 drivers/gpu/drm/i915/gt/intel_gtt.c   |  6 +-
 drivers/gpu/drm/i915/gt/intel_gtt.h   |  2 +-
 drivers/gpu/drm/i915/gt/selftest_execlists.c  |  2 +-
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c  |  2 +-
 drivers/gpu/drm/i915/i915_drv.h   |  4 +-
 drivers/gpu/drm/i915/i915_trace.h |  2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
 drivers/gpu/drm/i915/selftests/i915_vma.c |  4 +-
 18 files changed, 79 insertions(+), 123 deletions(-)

-- 
2.32.0



linux-next: manual merge of the drm-intel tree with Linus' tree

2021-08-02 Thread Mark Brown
Hi all,

Today's linux-next merge of the drm-intel tree got a conflict in:

  drivers/gpu/drm/i915/display/intel_display.c

between commit:

  b4bde5554f70 ("drm/i915/display: split DISPLAY_VER 9 and 10 in 
intel_setup_outputs()")

from Linus' tree and commits:

  cad83b405fe4 ("drm/i915/display: remove PORT_F workaround for CNL")
  ec387b8ff8d7 ("drm/i915/display: split DISPLAY_VER 9 and 10 in 
intel_setup_outputs()")

from the drm-intel tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

diff --cc drivers/gpu/drm/i915/display/intel_display.c
index 557871ee07db,3faedcb7ef42..
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c


Re: [PATCH 09/14] drm/radeon: Convert to Linux IRQ interfaces

2021-08-02 Thread Alex Deucher
On Tue, Jul 27, 2021 at 2:27 PM Thomas Zimmermann  wrote:
>
> Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
> IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
> don't benefit from using it.
>
> DRM IRQ callbacks are now being called directly or inlined.
>
> Signed-off-by: Thomas Zimmermann 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/radeon/radeon_drv.c |  4 ---
>  drivers/gpu/drm/radeon/radeon_irq_kms.c | 44 +
>  drivers/gpu/drm/radeon/radeon_kms.h |  4 ---
>  3 files changed, 37 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index c8dd68152d65..b74cebca1f89 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -607,10 +607,6 @@ static const struct drm_driver kms_driver = {
> .postclose = radeon_driver_postclose_kms,
> .lastclose = radeon_driver_lastclose_kms,
> .unload = radeon_driver_unload_kms,
> -   .irq_preinstall = radeon_driver_irq_preinstall_kms,
> -   .irq_postinstall = radeon_driver_irq_postinstall_kms,
> -   .irq_uninstall = radeon_driver_irq_uninstall_kms,
> -   .irq_handler = radeon_driver_irq_handler_kms,
> .ioctls = radeon_ioctls_kms,
> .num_ioctls = ARRAY_SIZE(radeon_ioctls_kms),
> .dumb_create = radeon_mode_dumb_create,
> diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c 
> b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> index a36ce826d0c0..3907785d0798 100644
> --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> @@ -31,7 +31,7 @@
>
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -51,7 +51,7 @@
>   * radeon_irq_process is a macro that points to the per-asic
>   * irq handler callback.
>   */
> -irqreturn_t radeon_driver_irq_handler_kms(int irq, void *arg)
> +static irqreturn_t radeon_driver_irq_handler_kms(int irq, void *arg)
>  {
> struct drm_device *dev = (struct drm_device *) arg;
> struct radeon_device *rdev = dev->dev_private;
> @@ -118,7 +118,7 @@ static void radeon_dp_work_func(struct work_struct *work)
>   * Gets the hw ready to enable irqs (all asics).
>   * This function disables all interrupt sources on the GPU.
>   */
> -void radeon_driver_irq_preinstall_kms(struct drm_device *dev)
> +static void radeon_driver_irq_preinstall_kms(struct drm_device *dev)
>  {
> struct radeon_device *rdev = dev->dev_private;
> unsigned long irqflags;
> @@ -150,7 +150,7 @@ void radeon_driver_irq_preinstall_kms(struct drm_device 
> *dev)
>   * Handles stuff to be done after enabling irqs (all asics).
>   * Returns 0 on success.
>   */
> -int radeon_driver_irq_postinstall_kms(struct drm_device *dev)
> +static int radeon_driver_irq_postinstall_kms(struct drm_device *dev)
>  {
> struct radeon_device *rdev = dev->dev_private;
>
> @@ -169,7 +169,7 @@ int radeon_driver_irq_postinstall_kms(struct drm_device 
> *dev)
>   *
>   * This function disables all interrupt sources on the GPU (all asics).
>   */
> -void radeon_driver_irq_uninstall_kms(struct drm_device *dev)
> +static void radeon_driver_irq_uninstall_kms(struct drm_device *dev)
>  {
> struct radeon_device *rdev = dev->dev_private;
> unsigned long irqflags;
> @@ -194,6 +194,36 @@ void radeon_driver_irq_uninstall_kms(struct drm_device 
> *dev)
> spin_unlock_irqrestore(>irq.lock, irqflags);
>  }
>
> +static int radeon_irq_install(struct radeon_device *rdev, int irq)
> +{
> +   struct drm_device *dev = rdev->ddev;
> +   int ret;
> +
> +   if (irq == IRQ_NOTCONNECTED)
> +   return -ENOTCONN;
> +
> +   radeon_driver_irq_preinstall_kms(dev);
> +
> +   /* PCI devices require shared interrupts. */
> +   ret = request_irq(irq, radeon_driver_irq_handler_kms,
> + IRQF_SHARED, dev->driver->name, dev);
> +   if (ret)
> +   return ret;
> +
> +   radeon_driver_irq_postinstall_kms(dev);
> +
> +   return 0;
> +}
> +
> +static void radeon_irq_uninstall(struct radeon_device *rdev)
> +{
> +   struct drm_device *dev = rdev->ddev;
> +   struct pci_dev *pdev = to_pci_dev(dev->dev);
> +
> +   radeon_driver_irq_uninstall_kms(dev);
> +   free_irq(pdev->irq, dev);
> +}
> +
>  /**
>   * radeon_msi_ok - asic specific msi checks
>   *
> @@ -314,7 +344,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
> INIT_WORK(>audio_work, r600_audio_update_hdmi);
>
> rdev->irq.installed = true;
> -   r = drm_irq_install(rdev->ddev, rdev->pdev->irq);
> +   r = radeon_irq_install(rdev, rdev->pdev->irq);
> if (r) {
> rdev->irq.installed = false;
> flush_delayed_work(>hotplug_work);
> @@ -335,7 +365,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
>  void radeon_irq_kms_fini(struct radeon_device *rdev)
> 

linux-next: manual merge of the drm-intel tree with the drm-intel-fixes tree

2021-08-02 Thread Mark Brown
Hi all,

Today's linux-next merge of the drm-intel tree got a conflict in:

  drivers/gpu/drm/i915/intel_device_info.c

between commit:

  0f9ed3b2c9ec ("drm/i915/display/cnl+: Handle fused off DSC")

from the drm-intel-fixes tree and commit:

  a4d082fc194a ("drm/i915: rename/remove CNL registers")

from the drm-intel tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

diff --cc drivers/gpu/drm/i915/intel_device_info.c
index e0a10f36acc1,305facedd284..
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c


Re: [Freedreno] [PATCH 0/3] iommu/drm/msm: Allow non-coherent masters to use system cache

2021-08-02 Thread Will Deacon
On Mon, Aug 02, 2021 at 08:08:07AM -0700, Rob Clark wrote:
> On Mon, Aug 2, 2021 at 3:55 AM Will Deacon  wrote:
> >
> > On Thu, Jul 29, 2021 at 10:08:22AM +0530, Sai Prakash Ranjan wrote:
> > > On 2021-07-28 19:30, Georgi Djakov wrote:
> > > > On Mon, Jan 11, 2021 at 07:45:02PM +0530, Sai Prakash Ranjan wrote:
> > > > > commit ecd7274fb4cd ("iommu: Remove unused IOMMU_SYS_CACHE_ONLY flag")
> > > > > removed unused IOMMU_SYS_CACHE_ONLY prot flag and along with it went
> > > > > the memory type setting required for the non-coherent masters to use
> > > > > system cache. Now that system cache support for GPU is added, we will
> > > > > need to set the right PTE attribute for GPU buffers to be sys cached.
> > > > > Without this, the system cache lines are not allocated for GPU.
> > > > >
> > > > > So the patches in this series introduces a new prot flag IOMMU_LLC,
> > > > > renames IO_PGTABLE_QUIRK_ARM_OUTER_WBWA to IO_PGTABLE_QUIRK_PTW_LLC
> > > > > and makes GPU the user of this protection flag.
> > > >
> > > > Thank you for the patchset! Are you planning to refresh it, as it does
> > > > not apply anymore?
> > > >
> > >
> > > I was waiting on Will's reply [1]. If there are no changes needed, then
> > > I can repost the patch.
> >
> > I still think you need to handle the mismatched alias, no? You're adding
> > a new memory type to the SMMU which doesn't exist on the CPU side. That
> > can't be right.
> >
> 
> Just curious, and maybe this is a dumb question, but what is your
> concern about mismatched aliases?  I mean the cache hierarchy on the
> GPU device side (anything beyond the LLC) is pretty different and
> doesn't really care about the smmu pgtable attributes..

If the CPU accesses a shared buffer with different attributes to those which
the device is using then you fall into the "mismatched memory attributes"
part of the Arm architecture. It's reasonably unforgiving (you should go and
read it) and in some cases can apply to speculative accesses as well, but
the end result is typically loss of coherency.

Will


Re: [PATCH 3/4] drm/tiny: add simple-dbi driver

2021-08-02 Thread Thomas Zimmermann

Hi

Am 02.08.21 um 08:35 schrieb Icenowy Zheng:

Add a driver for generic MIPI DBI panels initialized with MIPI DCS
commands.

Currently a ST7789V-based panel is added to it. This panel has its
configuration pre-programmed into the controller, so no vendor-specific
configuration is needed.

Signed-off-by: Icenowy Zheng 
---
  drivers/gpu/drm/tiny/Kconfig  |  12 ++
  drivers/gpu/drm/tiny/Makefile |   1 +
  drivers/gpu/drm/tiny/simple-dbi.c | 244 ++
  3 files changed, 257 insertions(+)
  create mode 100644 drivers/gpu/drm/tiny/simple-dbi.c

diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index d31be274a2bd..6cfc57b68a46 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -144,6 +144,18 @@ config TINYDRM_REPAPER
  
  	  If M is selected the module will be called repaper.
  
+config TINYDRM_SIMPLE_DBI

+   tristate "DRM support for generic MIPI DBI panels"
+   depends on DRM && SPI
+   select DRM_KMS_HELPER
+   select DRM_KMS_CMA_HELPER
+   select DRM_MIPI_DBI
+   help
+ DRM driver for generic DBI panels that are MIPI DCS compatible
+ and needs no vendor-specific setup commands.
+
+ If M is selected the module will be called simple-dbi.
+
  config TINYDRM_ST7586
tristate "DRM support for Sitronix ST7586 display panels"
depends on DRM && SPI
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index e09942895c77..2553de651aa3 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_TINYDRM_MI0283QT)+= mi0283qt.o
  obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o
  obj-$(CONFIG_TINYDRM_ST7586)  += st7586.o
  obj-$(CONFIG_TINYDRM_ST7735R) += st7735r.o
+obj-$(CONFIG_TINYDRM_SIMPLE_DBI)   += simple-dbi.o
diff --git a/drivers/gpu/drm/tiny/simple-dbi.c 
b/drivers/gpu/drm/tiny/simple-dbi.c
new file mode 100644
index ..2b207e43d500
--- /dev/null
+++ b/drivers/gpu/drm/tiny/simple-dbi.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DRM driver for display panels with configuration preset and needs only
+ * standard MIPI DCS commands to bring up.
+ *
+ * Copyright (C) 2021 Sipeed
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MIPI_DCS_ADDRESS_MODE_BGR  BIT(3)
+#define MIPI_DCS_ADDRESS_MODE_REVERSE  BIT(5)
+#define MIPI_DCS_ADDRESS_MODE_RTL  BIT(6)
+#define MIPI_DCS_ADDRESS_MODE_BTT  BIT(7)
+
+struct simple_dbi_cfg {
+   const struct drm_display_mode mode;
+   unsigned int left_offset;
+   unsigned int top_offset;
+   bool inverted;
+   bool write_only;
+   bool bgr;
+   bool right_to_left;
+   bool bottom_to_top;
+};
+
+struct simple_dbi_priv {
+   struct mipi_dbi_dev dbidev; /* Must be first for .release() */


Which release?


+   const struct simple_dbi_cfg *cfg;
+};
+
+static void simple_dbi_pipe_enable(struct drm_simple_display_pipe *pipe,
+   struct drm_crtc_state *crtc_state,
+   struct drm_plane_state *plane_state)
+{
+   struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+   struct simple_dbi_priv *priv = container_of(dbidev,
+   struct simple_dbi_priv,
+   dbidev);
+   struct mipi_dbi *dbi = >dbi;
+   int ret, idx;
+   u8 addr_mode = 0x00;
+
+   if (!drm_dev_enter(pipe->crtc.dev, ))
+   return;
+
+   DRM_DEBUG_KMS("\n");


I'm not a friend of such debugging statements. If you absolutely want to 
keep it, rather use drm_dbg_kms().



+
+   ret = mipi_dbi_poweron_reset(dbidev);
+   if (ret)
+   goto out_exit;
+
+   mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
+   msleep(5);
+
+   /* Currently tinydrm supports 16bit only now */
+   mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT,
+MIPI_DCS_PIXEL_FMT_16BIT);
+
+   if (priv->cfg->inverted)
+   mipi_dbi_command(dbi, MIPI_DCS_ENTER_INVERT_MODE);
+   else
+   mipi_dbi_command(dbi, MIPI_DCS_EXIT_INVERT_MODE);
+
+   if (priv->cfg->bgr)
+   addr_mode |= MIPI_DCS_ADDRESS_MODE_BGR;
+
+   if (priv->cfg->right_to_left)
+   addr_mode |= MIPI_DCS_ADDRESS_MODE_RTL;
+
+   if (priv->cfg->bottom_to_top)
+   addr_mode |= MIPI_DCS_ADDRESS_MODE_BTT;
+
+   switch (dbidev->rotation) {
+   default:
+   break;
+   case 90:
+   addr_mode ^= MIPI_DCS_ADDRESS_MODE_REVERSE;
+   addr_mode ^= MIPI_DCS_ADDRESS_MODE_RTL;
+   break;
+   case 180:
+   addr_mode ^= 

Re: [Freedreno] [PATCH 0/3] iommu/drm/msm: Allow non-coherent masters to use system cache

2021-08-02 Thread Rob Clark
On Mon, Aug 2, 2021 at 3:55 AM Will Deacon  wrote:
>
> On Thu, Jul 29, 2021 at 10:08:22AM +0530, Sai Prakash Ranjan wrote:
> > On 2021-07-28 19:30, Georgi Djakov wrote:
> > > On Mon, Jan 11, 2021 at 07:45:02PM +0530, Sai Prakash Ranjan wrote:
> > > > commit ecd7274fb4cd ("iommu: Remove unused IOMMU_SYS_CACHE_ONLY flag")
> > > > removed unused IOMMU_SYS_CACHE_ONLY prot flag and along with it went
> > > > the memory type setting required for the non-coherent masters to use
> > > > system cache. Now that system cache support for GPU is added, we will
> > > > need to set the right PTE attribute for GPU buffers to be sys cached.
> > > > Without this, the system cache lines are not allocated for GPU.
> > > >
> > > > So the patches in this series introduces a new prot flag IOMMU_LLC,
> > > > renames IO_PGTABLE_QUIRK_ARM_OUTER_WBWA to IO_PGTABLE_QUIRK_PTW_LLC
> > > > and makes GPU the user of this protection flag.
> > >
> > > Thank you for the patchset! Are you planning to refresh it, as it does
> > > not apply anymore?
> > >
> >
> > I was waiting on Will's reply [1]. If there are no changes needed, then
> > I can repost the patch.
>
> I still think you need to handle the mismatched alias, no? You're adding
> a new memory type to the SMMU which doesn't exist on the CPU side. That
> can't be right.
>

Just curious, and maybe this is a dumb question, but what is your
concern about mismatched aliases?  I mean the cache hierarchy on the
GPU device side (anything beyond the LLC) is pretty different and
doesn't really care about the smmu pgtable attributes..

BR,
-R


[PATCH] drm/msm/gpu: fix link failure with QCOM_SCM=m

2021-08-02 Thread Arnd Bergmann
From: Arnd Bergmann 

Another missed dependency when SCM is a loadable module
and adreno is built-in:

drivers/gpu/drm/msm/adreno/adreno_gpu.o: In function `adreno_zap_shader_load':
adreno_gpu.c:(.text+0x1e8): undefined reference to `qcom_scm_is_available'
drivers/gpu/drm/msm/adreno/a5xx_gpu.o: In function `a5xx_hw_init':
a5xx_gpu.c:(.text+0x28a6): undefined reference to `qcom_scm_set_remote_state'

Change it so the dependency on QCOM_SCM and QCOM_MDT_LOADER can be
ignored if we are not building for ARCH_QCOM, but prevent the
link error during compile testing when SCM is a loadable module
and ARCH_QCOM is disabled.

Fixes: a9e2559c931d ("drm/msm/gpu: Move zap shader loading to adreno")
Fixes: 5ea4dba68305 ("drm/msm/a6xx: add CONFIG_QCOM_LLCC dependency")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/msm/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 52536e7adb95..69fbfe4568b2 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -9,14 +9,14 @@ config DRM_MSM
depends on QCOM_OCMEM || QCOM_OCMEM=n
depends on QCOM_LLCC || QCOM_LLCC=n
depends on QCOM_COMMAND_DB || QCOM_COMMAND_DB=n
+   depends on QCOM_SCM || (QCOM_SCM=n && ARCH_QCOM=n)
+   depends on QCOM_MDT_LOADER || ARCH_QCOM=n
select IOMMU_IO_PGTABLE
-   select QCOM_MDT_LOADER if ARCH_QCOM
select REGULATOR
select DRM_KMS_HELPER
select DRM_PANEL
select SHMEM
select TMPFS
-   select QCOM_SCM if ARCH_QCOM
select WANT_DEV_COREDUMP
select SND_SOC_HDMI_CODEC if SND_SOC
select SYNC_FILE
-- 
2.29.2



Re: [PATCH] drm/radeon: Update pitch for page flip

2021-08-02 Thread Alex Deucher
On Mon, Aug 2, 2021 at 4:31 AM Daniel Vetter  wrote:
>
> On Mon, Aug 02, 2021 at 10:12:47AM +0200, Christian König wrote:
> > Am 02.08.21 um 09:43 schrieb Zhenneng Li:
> > > When primary bo is updated, crtc's pitch may
> > > have not been updated, this will lead to show
> > > disorder content when user changes display mode,
> > > we update crtc's pitch in page flip to avoid
> > > this bug.
> > > This refers to amdgpu's pageflip.
> >
> > Alex is the expert to ask about that code, but I'm not sure if that is
> > really correct for the old hardware.
> >
> > As far as I know the crtc's pitch should not change during a page flip, but
> > only during a full mode set.
> >
> > So could you elaborate a bit more how you trigger this?
>
> legacy page_flip ioctl only verifies that the fb->format stays the same.
> It doesn't check anything else (afair never has), this is all up to
> drivers to verify.
>
> Personally I'd say add a check to reject this, since testing this and
> making sure it really works everywhere is probably a bit much on this old
> hw.

If just the pitch changed, that probably wouldn't be much of a
problem, but if the pitch is changing, that probably implies other
stuff has changed as well and we'll just be chasing changes.  I agree
it would be best to just reject anything other than updating the
scanout address.

Alex

> -Daniel
>
> >
> > Thanks,
> > Christian.
> >
> > >
> > > Cc: Alex Deucher 
> > > Cc: "Christian König" 
> > > Cc: "Pan, Xinhui" 
> > > Cc: David Airlie 
> > > Cc: Daniel Vetter 
> > > Cc: amd-...@lists.freedesktop.org
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: linux-ker...@vger.kernel.org
> > > Signed-off-by: Zhenneng Li 
> > > ---
> > >   drivers/gpu/drm/radeon/evergreen.c | 8 +++-
> > >   1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/radeon/evergreen.c 
> > > b/drivers/gpu/drm/radeon/evergreen.c
> > > index 36a888e1b179..eeb590d2dec2 100644
> > > --- a/drivers/gpu/drm/radeon/evergreen.c
> > > +++ b/drivers/gpu/drm/radeon/evergreen.c
> > > @@ -28,6 +28,7 @@
> > >   #include 
> > >   #include 
> > > +#include 
> > >   #include "atom.h"
> > >   #include "avivod.h"
> > > @@ -1414,10 +1415,15 @@ void evergreen_page_flip(struct radeon_device 
> > > *rdev, int crtc_id, u64 crtc_base,
> > >  bool async)
> > >   {
> > > struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id];
> > > +   struct drm_framebuffer *fb = radeon_crtc->base.primary->fb;
> > > -   /* update the scanout addresses */
> > > +   /* flip at hsync for async, default is vsync */
> > > WREG32(EVERGREEN_GRPH_FLIP_CONTROL + radeon_crtc->crtc_offset,
> > >async ? EVERGREEN_GRPH_SURFACE_UPDATE_H_RETRACE_EN : 0);
> > > +   /* update pitch */
> > > +   WREG32(EVERGREEN_GRPH_PITCH + radeon_crtc->crtc_offset,
> > > +  fb->pitches[0] / fb->format->cpp[0]);
> > > +   /* update the scanout addresses */
> > > WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS_HIGH + 
> > > radeon_crtc->crtc_offset,
> > >upper_32_bits(crtc_base));
> > > WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS + 
> > > radeon_crtc->crtc_offset,
> > >
> > > No virus found
> > > Checked by Hillstone Network AntiVirus
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch


Re: [PATCH] drm/msm/dp: Remove unused variable

2021-08-02 Thread Souptick Joarder
On Thu, Jul 15, 2021 at 3:09 AM Stephen Boyd  wrote:
>
> Quoting Souptick Joarder (2021-07-08 19:48:34)
> > Kernel test roobot throws below warning ->
> >
> > drivers/gpu/drm/msm/dp/dp_display.c:1017:21:
> > warning: variable 'drm' set but not used [-Wunused-but-set-variable]
> >
> > Removed unused variable drm.
> >

Can we get this in queue for 5.15, if no further comment ?


> > Reported-by: kernel test robot 
> > Signed-off-by: Souptick Joarder 
> > ---
>
> Reviewed-by: Stephen Boyd 


Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability

2021-08-02 Thread Daniel Vetter
On Mon, Aug 2, 2021 at 2:51 PM Gerd Hoffmann  wrote:
>
>   Hi,
>
> > > That sounds sensible to me.  Fence the virtio commands, make sure (on
> > > the host side) the command completes only when the work is actually done
> > > not only submitted.  Has recently been added to qemu for RESOURCE_FLUSH
> > > (aka frontbuffer rendering) and doing the same for SET_SCANOUT (aka
> > > pageflipping), then send vblank events to userspace on command
> > > completion certainly makes sense.
> >
> > Hm how does this all work? At least drm/virtio uses
> > drm_atomic_helper_dirtyfb, so both DIRTYFB ioctl and atomic flips all end
> > up in the same driver path for everything. Or do you just combine the
> > resource_flush with the flip as needed and let the host side figure it all
> > out? From a quick read of virtgpu_plane.c that seems to be the case ...
>
> virtio_gpu_primary_plane_update() will send RESOURCE_FLUSH only for
> DIRTYFB and both SET_SCANOUT + RESOURCE_FLUSH for page-flip, and I
> think for the page-flip case the host (aka qemu) doesn't get the
> "wait until old framebuffer is not in use any more" right yet.

Hm reading the code I think you simply elide the set_scanout if it's
still the same buffer. There's no difference betweeen dirtyfb and an
atomic commit that just hands the damage rects to the driver. At least
if you use the helpers.

> So we'll need a host-side fix for that and a guest-side fix to switch
> from a blocking wait on the fence to vblank events.
>
> > Also to make this work we don't just need the fence, we need the timestamp
> > (in a clock domain the guest can correct for ofc) of the host side kms
> > driver flip completion. If you just have the fence then the jitter from
> > going through all the layers will most likely make it unusable.
>
> Well, there are no timestamps in the virtio-gpu protocol ...
>
> Also I'm not sure they would be that helpful, any timing is *much* less
> predictable in a virtual machine, especially in case the host machine is
> loaded.

Hm yeah if the output is currently not displaying, then the timestamp
is very fake. But if you display you should be able to pass it all
around in both direction. So target vblank (or whatever it's called)
would go from guest to host to host-compositor (over wayland protocol)
to host-side kms, and the timestamp could travel all the way back.

But yeah making this all work correctly is going to be a pile of work.
Also I have no idea how well compositors take it when a kms driver
switches between high-precision timestamps and frame scheduling to the
entirely virtual/vblank-less approach on the fly.
-Daniel

> take care,
>   Gerd
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[Bug 205089] amdgpu : drm:amdgpu_cs_ioctl : Failed to initialize parser -125

2021-08-02 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=205089

--- Comment #18 from jes...@jnsn.dev ---
On 02/08/21 at 02:13pm, bugzilla-dae...@bugzilla.kernel.org wrote:
>Does up/downgrading the mesa driver help?

Upgrading to the latest git revision of mesa has fixed Dota 2 for me at least.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 213935] AMDGPU Renoir crash/freeze while using vaapi with some video types in some apps - drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out!

2021-08-02 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=213935

Alex Deucher (alexdeuc...@gmail.com) changed:

   What|Removed |Added

 CC||alexdeuc...@gmail.com

--- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) ---
Can you try a newer or older version of mesa?  Most likely this is a bug in the
user mode driver.  The kernel is just the messenger.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 205089] amdgpu : drm:amdgpu_cs_ioctl : Failed to initialize parser -125

2021-08-02 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=205089

--- Comment #17 from Alex Deucher (alexdeuc...@gmail.com) ---
Does up/downgrading the mesa driver help?

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

Re: [PATCH v2 1/6] dt-bindings: drm/panel-simple: Introduce generic eDP panels

2021-08-02 Thread Rob Herring
On Fri, 30 Jul 2021 14:26:20 -0700, Douglas Anderson wrote:
> eDP panels generally contain almost everything needed to control them
> in their EDID. This comes from their DP heritage were a computer needs
> to be able to properly control pretty much any DP display that's
> plugged into it.
> 
> The one big issue with eDP panels and the reason that we need a panel
> driver for them is that the power sequencing can be different per
> panel.
> 
> While it is true that eDP panel sequencing can be arbitrarily complex,
> in practice it turns out that many eDP panels are compatible with just
> some slightly different delays. See the contents of the bindings file
> introduced in this patch for some details.
> 
> The fact that eDP panels are 99% probable and that the power
> sequencing (especially power up) can be compatible between many panels
> means that there's a constant desire to plug multiple different panels
> into the same board. This could be for second sourcing purposes or to
> support multiple SKUs (maybe a 11" and a 13", for instance).
> 
> As discussed [1], it should be OK to support this by adding two
> properties to the device tree to specify the delays needed for
> powering up the panel the first time. We'll create a new "edp-panel"
> bindings file and define the two delays that might need to be
> specified. NOTE: in the vast majority of the cases (HPD is hooked up
> and isn't glitchy or is debounced) even these delays aren't needed.
> 
> [1] 
> https://lore.kernel.org/r/CAD=FV=vzyompwqzzwdhjgh5cjjww_ecm-wqveivz-bdgxjp...@mail.gmail.com
> 
> Signed-off-by: Douglas Anderson 
> ---
> 
> Changes in v2:
> - No longer allow fallback to panel-simple.
> - Add "-ms" suffix to delays.
> 
>  .../bindings/display/panel/panel-edp.yaml | 188 ++
>  1 file changed, 188 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/panel-edp.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/panel-edp.example.dt.yaml:
 bridge@2d: 'aux-bus' does not match any of the regexes: 'pinctrl-[0-9]+'
From schema: 
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
\ndoc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1511822

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.



[PATCH] fbdev/efifb: Release PCI device's runtime PM ref during FB destroy

2021-08-02 Thread Imre Deak
Atm the EFI FB driver gets a runtime PM reference for the associated GFX
PCI device during driver probing and releases it only when removing the
driver.

When fbcon switches to the FB provided by the PCI device's driver (for
instance i915/drmfb), the EFI FB will get only unregistered without the
EFI FB driver getting unloaded, keeping the runtime PM reference
acquired during driver probing. This reference will prevent the PCI
driver from runtime suspending the device.

Fix this by releasing the RPM reference from the EFI FB's destroy hook,
called when the FB gets unregistered.

Fixes: a6c0fd3d5a8b ("efifb: Ensure graphics device for efifb stays at PCI D0")
Cc: Kai-Heng Feng 
Signed-off-by: Imre Deak 
---
 drivers/video/fbdev/efifb.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 8ea8f079cde26..25cdea32b9633 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -47,6 +47,8 @@ static bool use_bgrt = true;
 static bool request_mem_succeeded = false;
 static u64 mem_flags = EFI_MEMORY_WC | EFI_MEMORY_UC;
 
+static struct pci_dev *efifb_pci_dev;  /* dev with BAR covering the efifb */
+
 static struct fb_var_screeninfo efifb_defined = {
.activate   = FB_ACTIVATE_NOW,
.height = -1,
@@ -243,6 +245,9 @@ static inline void efifb_show_boot_graphics(struct fb_info 
*info) {}
 
 static void efifb_destroy(struct fb_info *info)
 {
+   if (efifb_pci_dev)
+   pm_runtime_put(_pci_dev->dev);
+
if (info->screen_base) {
if (mem_flags & (EFI_MEMORY_UC | EFI_MEMORY_WC))
iounmap(info->screen_base);
@@ -333,7 +338,6 @@ ATTRIBUTE_GROUPS(efifb);
 
 static bool pci_dev_disabled;  /* FB base matches BAR of a disabled device */
 
-static struct pci_dev *efifb_pci_dev;  /* dev with BAR covering the efifb */
 static struct resource *bar_resource;
 static u64 bar_offset;
 
@@ -603,8 +607,6 @@ static int efifb_remove(struct platform_device *pdev)
unregister_framebuffer(info);
sysfs_remove_groups(>dev.kobj, efifb_groups);
framebuffer_release(info);
-   if (efifb_pci_dev)
-   pm_runtime_put(_pci_dev->dev);
 
return 0;
 }
-- 
2.27.0



Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability

2021-08-02 Thread Gerd Hoffmann
  Hi,

> > That sounds sensible to me.  Fence the virtio commands, make sure (on
> > the host side) the command completes only when the work is actually done
> > not only submitted.  Has recently been added to qemu for RESOURCE_FLUSH
> > (aka frontbuffer rendering) and doing the same for SET_SCANOUT (aka
> > pageflipping), then send vblank events to userspace on command
> > completion certainly makes sense.
> 
> Hm how does this all work? At least drm/virtio uses
> drm_atomic_helper_dirtyfb, so both DIRTYFB ioctl and atomic flips all end
> up in the same driver path for everything. Or do you just combine the
> resource_flush with the flip as needed and let the host side figure it all
> out? From a quick read of virtgpu_plane.c that seems to be the case ...

virtio_gpu_primary_plane_update() will send RESOURCE_FLUSH only for
DIRTYFB and both SET_SCANOUT + RESOURCE_FLUSH for page-flip, and I
think for the page-flip case the host (aka qemu) doesn't get the
"wait until old framebuffer is not in use any more" right yet.

So we'll need a host-side fix for that and a guest-side fix to switch
from a blocking wait on the fence to vblank events.

> Also to make this work we don't just need the fence, we need the timestamp
> (in a clock domain the guest can correct for ofc) of the host side kms
> driver flip completion. If you just have the fence then the jitter from
> going through all the layers will most likely make it unusable.

Well, there are no timestamps in the virtio-gpu protocol ...

Also I'm not sure they would be that helpful, any timing is *much* less
predictable in a virtual machine, especially in case the host machine is
loaded.

take care,
  Gerd



Re: [PATCH 07/11] treewide: Replace the use of mem_encrypt_active() with prot_guest_has()

2021-08-02 Thread Christophe Leroy




Le 28/07/2021 à 00:26, Tom Lendacky a écrit :

Replace occurrences of mem_encrypt_active() with calls to prot_guest_has()
with the PATTR_MEM_ENCRYPT attribute.



What about 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210730114231.23445-1-w...@kernel.org/ ?


Christophe




Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: Andy Lutomirski 
Cc: Peter Zijlstra 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: VMware Graphics 
Cc: Joerg Roedel 
Cc: Will Deacon 
Cc: Dave Young 
Cc: Baoquan He 
Signed-off-by: Tom Lendacky 
---
  arch/x86/kernel/head64.c| 4 ++--
  arch/x86/mm/ioremap.c   | 4 ++--
  arch/x86/mm/mem_encrypt.c   | 5 ++---
  arch/x86/mm/pat/set_memory.c| 3 ++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +++-
  drivers/gpu/drm/drm_cache.c | 4 ++--
  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++--
  drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 6 +++---
  drivers/iommu/amd/iommu.c   | 3 ++-
  drivers/iommu/amd/iommu_v2.c| 3 ++-
  drivers/iommu/iommu.c   | 3 ++-
  fs/proc/vmcore.c| 6 +++---
  kernel/dma/swiotlb.c| 4 ++--
  13 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index de01903c3735..cafed6456d45 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -19,7 +19,7 @@
  #include 
  #include 
  #include 
-#include 
+#include 
  #include 
  
  #include 

@@ -285,7 +285,7 @@ unsigned long __head __startup_64(unsigned long physaddr,
 * there is no need to zero it after changing the memory encryption
 * attribute.
 */
-   if (mem_encrypt_active()) {
+   if (prot_guest_has(PATTR_MEM_ENCRYPT)) {
vaddr = (unsigned long)__start_bss_decrypted;
vaddr_end = (unsigned long)__end_bss_decrypted;
for (; vaddr < vaddr_end; vaddr += PMD_SIZE) {
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 0f2d5ace5986..5e1c1f5cbbe8 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -693,7 +693,7 @@ static bool __init 
early_memremap_is_setup_data(resource_size_t phys_addr,
  bool arch_memremap_can_ram_remap(resource_size_t phys_addr, unsigned long 
size,
 unsigned long flags)
  {
-   if (!mem_encrypt_active())
+   if (!prot_guest_has(PATTR_MEM_ENCRYPT))
return true;
  
  	if (flags & MEMREMAP_ENC)

@@ -723,7 +723,7 @@ pgprot_t __init 
early_memremap_pgprot_adjust(resource_size_t phys_addr,
  {
bool encrypted_prot;
  
-	if (!mem_encrypt_active())

+   if (!prot_guest_has(PATTR_MEM_ENCRYPT))
return prot;
  
  	encrypted_prot = true;

diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 451de8e84fce..0f1533dbe81c 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -364,8 +364,7 @@ int __init early_set_memory_encrypted(unsigned long vaddr, 
unsigned long size)
  /*
   * SME and SEV are very similar but they are not the same, so there are
   * times that the kernel will need to distinguish between SME and SEV. The
- * sme_active() and sev_active() functions are used for this.  When a
- * distinction isn't needed, the mem_encrypt_active() function can be used.
+ * sme_active() and sev_active() functions are used for this.
   *
   * The trampoline code is a good example for this requirement.  Before
   * paging is activated, SME will access all memory as decrypted, but SEV
@@ -451,7 +450,7 @@ void __init mem_encrypt_free_decrypted_mem(void)
 * The unused memory range was mapped decrypted, change the encryption
 * attribute from decrypted to encrypted before freeing it.
 */
-   if (mem_encrypt_active()) {
+   if (sme_me_mask) {
r = set_memory_encrypted(vaddr, npages);
if (r) {
pr_warn("failed to free unused decrypted pages\n");
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index ad8a5c586a35..6925f2bb4be1 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -18,6 +18,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #include 

  #include 
@@ -1986,7 +1987,7 @@ static int __set_memory_enc_dec(unsigned long addr, int 
numpages, bool enc)
int ret;
  
  	/* Nothing to do if memory encryption is not active */

-   if (!mem_encrypt_active())
+   if (!prot_guest_has(PATTR_MEM_ENCRYPT))
return 0;
  
  	/* Should not be working on unaligned addresses */

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index abb928894eac..8407224717df 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -38,6 +38,7 @@
  

[PATCH v3] drm: Fix typo in comments

2021-08-02 Thread Cai Huoqing
fix typo for drm:
*achived ==> achieved
*userpace ==> userspace
*commited ==> committed
*depencies ==> dependencies
*preceeding ==> preceding
*similiar ==> similar
*accidently ==> accidentally
*transfered ==> transferred
*openeing ==> opening
*Propage ==> Propagate
*interpretes ==> interprets
*evry ==> every
*interruptable ==> interruptible
*Unfortunatley ==> Unfortunately
*Deterine ==> Determine
*non-existant ==> non-existent
*satifying ==> satisfying
*initalizing ==> initializing
*possibily ==> possibly
*accidently ==> accidentally
*firmare ==> firmware
*shuld ==> should
*apperture ==> aperture
*falure ==> failure
*inforamtion ==> information
*implemnt ==> implement
*paramter ==> parameter
*candiates ==> candidates
*identifer ==> identifier
*wil ==> will
*overlayed ==> overlaid
*accomodate ==> accommodate
*uniqe ==> unique
*intializes ==> initializes
*useable ==> usable
*arguement ==> argument
*destry ==> destroy
*savely ==> safely
*Unsinged ==> Unsigned
*unititialized ==> uninitialized
*draing ==> drawing
*invers ==> inverse
*Enpoint ==> Endpointc
*mimick ==> mimic
*fo ==> for
*functons ==> functions

v1->v2:
*respin with the change "iff ==> implies that"
v2->v3:
*revert the change of "iff" which means "if and only if"
*remove "Reviewed-by:"
*add other typo
*update changelog

Signed-off-by: Cai Huoqing 
---
 drivers/gpu/drm/drm_aperture.c|  2 +-
 drivers/gpu/drm/drm_atomic.c  |  2 +-
 drivers/gpu/drm/drm_atomic_helper.c   | 14 +++---
 drivers/gpu/drm/drm_atomic_uapi.c |  6 +++---
 drivers/gpu/drm/drm_auth.c|  2 +-
 drivers/gpu/drm/drm_bridge.c  |  2 +-
 drivers/gpu/drm/drm_bufs.c|  2 +-
 drivers/gpu/drm/drm_cache.c   |  2 +-
 drivers/gpu/drm/drm_color_mgmt.c  |  2 +-
 drivers/gpu/drm/drm_damage_helper.c   |  2 +-
 drivers/gpu/drm/drm_dp_dual_mode_helper.c |  2 +-
 drivers/gpu/drm/drm_dp_helper.c   |  8 
 drivers/gpu/drm/drm_dp_mst_topology.c |  2 +-
 drivers/gpu/drm/drm_drv.c |  4 ++--
 drivers/gpu/drm/drm_dsc.c |  2 +-
 drivers/gpu/drm/drm_edid.c|  4 ++--
 drivers/gpu/drm/drm_fb_helper.c   |  4 ++--
 drivers/gpu/drm/drm_file.c|  6 +++---
 drivers/gpu/drm/drm_format_helper.c   |  2 +-
 drivers/gpu/drm/drm_framebuffer.c |  2 +-
 drivers/gpu/drm/drm_gem.c |  4 ++--
 drivers/gpu/drm/drm_gem_atomic_helper.c   |  4 ++--
 drivers/gpu/drm/drm_gem_shmem_helper.c|  2 +-
 drivers/gpu/drm/drm_gem_vram_helper.c |  2 +-
 drivers/gpu/drm/drm_ioctl.c   |  4 ++--
 drivers/gpu/drm/drm_irq.c |  2 +-
 drivers/gpu/drm/drm_mm.c  |  2 +-
 drivers/gpu/drm/drm_mode_object.c |  2 +-
 drivers/gpu/drm/drm_modes.c   |  4 ++--
 drivers/gpu/drm/drm_plane.c   |  2 +-
 drivers/gpu/drm/drm_plane_helper.c|  2 +-
 drivers/gpu/drm/drm_prime.c   |  2 +-
 drivers/gpu/drm/drm_probe_helper.c|  2 +-
 drivers/gpu/drm/drm_property.c|  2 +-
 drivers/gpu/drm/drm_syncobj.c |  2 +-
 drivers/gpu/drm/drm_vblank.c  |  6 +++---
 36 files changed, 58 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
index 9ac39cf11694..74bd4a76b253 100644
--- a/drivers/gpu/drm/drm_aperture.c
+++ b/drivers/gpu/drm/drm_aperture.c
@@ -78,7 +78,7 @@
  *
  * Drivers that are susceptible to being removed by other drivers, such as
  * generic EFI or VESA drivers, have to register themselves as owners of their
- * given framebuffer memory. Ownership of the framebuffer memory is achived
+ * given framebuffer memory. Ownership of the framebuffer memory is achieved
  * by calling devm_aperture_acquire_from_firmware(). On success, the driver
  * is the owner of the framebuffer range. The function fails if the
  * framebuffer is already by another driver. See below for an example.
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index d820423fac32..b127e30082ba 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -723,7 +723,7 @@ static void drm_atomic_plane_print_state(struct drm_printer 
*p,
  * clocks, scaler units, bandwidth and fifo limits shared among a group of
  * planes or CRTCs, and so on) it makes sense to model these as independent
  * objects. Drivers then need to do similar state tracking and commit ordering 
for
- * such private (since not exposed to userpace) objects as the atomic core and
+ * such private (since not exposed to userspace) objects as the atomic core and
  * helpers already provide for connectors, planes and CRTCs.
  *
  * To make this easier on drivers the atomic core provides some support to 
track
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index f7bf1ea62d58..4d92036422ce 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ 

Re: [PATCH] drm/vbox: Convert to Linux IRQ interfaces

2021-08-02 Thread Hans de Goede
Hi,

On 7/6/21 9:50 AM, Thomas Zimmermann wrote:
> Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
> IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
> don't benefit from using it.
> 
> Signed-off-by: Thomas Zimmermann 

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede 

And to make sure things don't regress I've also given this a test spin:

Tested-by: Hans de Goede 

Note I assume that you will push this out do drmi-misc yourself
(if you've not done so already given that this patch is somewhat old).

Regards,

Hans



> ---
>  drivers/gpu/drm/vboxvideo/vbox_drv.c |  1 -
>  drivers/gpu/drm/vboxvideo/vbox_drv.h |  1 -
>  drivers/gpu/drm/vboxvideo/vbox_irq.c | 16 +++-
>  3 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c 
> b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> index 879a2445cc44..2b81cb259d23 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> @@ -184,7 +184,6 @@ static const struct drm_driver driver = {
>   .lastclose = drm_fb_helper_lastclose,
>  
>   .fops = _fops,
> - .irq_handler = vbox_irq_handler,
>   .name = DRIVER_NAME,
>   .desc = DRIVER_DESC,
>   .date = DRIVER_DATE,
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.h 
> b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> index ac7c2effc46f..4903b91d7fe4 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.h
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> @@ -145,7 +145,6 @@ void vbox_mm_fini(struct vbox_private *vbox);
>  int vbox_irq_init(struct vbox_private *vbox);
>  void vbox_irq_fini(struct vbox_private *vbox);
>  void vbox_report_hotplug(struct vbox_private *vbox);
> -irqreturn_t vbox_irq_handler(int irq, void *arg);
>  
>  /* vbox_hgsmi.c */
>  void *hgsmi_buffer_alloc(struct gen_pool *guest_pool, size_t size,
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_irq.c 
> b/drivers/gpu/drm/vboxvideo/vbox_irq.c
> index b3ded68603ba..903a6c48ee8b 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_irq.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_irq.c
> @@ -10,7 +10,8 @@
>   */
>  
>  #include 
> -#include 
> +
> +#include 
>  #include 
>  
>  #include "vbox_drv.h"
> @@ -31,7 +32,7 @@ void vbox_report_hotplug(struct vbox_private *vbox)
>   schedule_work(>hotplug_work);
>  }
>  
> -irqreturn_t vbox_irq_handler(int irq, void *arg)
> +static irqreturn_t vbox_irq_handler(int irq, void *arg)
>  {
>   struct drm_device *dev = (struct drm_device *)arg;
>   struct vbox_private *vbox = to_vbox_dev(dev);
> @@ -170,16 +171,21 @@ static void vbox_hotplug_worker(struct work_struct 
> *work)
>  
>  int vbox_irq_init(struct vbox_private *vbox)
>  {
> - struct pci_dev *pdev = to_pci_dev(vbox->ddev.dev);
> + struct drm_device *dev = >ddev;
> + struct pci_dev *pdev = to_pci_dev(dev->dev);
>  
>   INIT_WORK(>hotplug_work, vbox_hotplug_worker);
>   vbox_update_mode_hints(vbox);
>  
> - return drm_irq_install(>ddev, pdev->irq);
> + /* PCI devices require shared interrupts. */
> + return request_irq(pdev->irq, vbox_irq_handler, IRQF_SHARED, 
> dev->driver->name, dev);
>  }
>  
>  void vbox_irq_fini(struct vbox_private *vbox)
>  {
> - drm_irq_uninstall(>ddev);
> + struct drm_device *dev = >ddev;
> + struct pci_dev *pdev = to_pci_dev(dev->dev);
> +
> + free_irq(pdev->irq, dev);
>   flush_work(>hotplug_work);
>  }
> 



[RESEND PATCH v2 2/2] drm: add lockdep assert to drm_is_current_master_locked

2021-08-02 Thread Desmond Cheong Zhi Xi
In drm_is_current_master_locked, accessing drm_file.master should be
protected by either drm_file.master_lookup_lock or
drm_device.master_mutex. This was previously awkward to assert with
lockdep.

Following patch ("locking/lockdep: Provide lockdep_assert{,_once}()
helpers"), this assertion is now convenient. So we add in the
assertion and explain this lock design in the kerneldoc.

Signed-off-by: Desmond Cheong Zhi Xi 
Acked-by: Boqun Feng 
Acked-by: Waiman Long 
Acked-by: Peter Zijlstra (Intel) 
---
 drivers/gpu/drm/drm_auth.c | 6 +++---
 include/drm/drm_file.h | 4 
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 9c24b8cc8e36..6f4d7ff23c80 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -63,9 +63,9 @@
 
 static bool drm_is_current_master_locked(struct drm_file *fpriv)
 {
-   /* Either drm_device.master_mutex or drm_file.master_lookup_lock
-* should be held here.
-*/
+   lockdep_assert_once(lockdep_is_held(>master_lookup_lock) ||
+   lockdep_is_held(>minor->dev->master_mutex));
+
return fpriv->is_master && drm_lease_owner(fpriv->master) == 
fpriv->minor->dev->master;
 }
 
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 726cfe0ff5f5..a3acb7ac3550 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -233,6 +233,10 @@ struct drm_file {
 * this only matches _device.master if the master is the currently
 * active one.
 *
+* To update @master, both _device.master_mutex and
+* @master_lookup_lock need to be held, therefore holding either of
+* them is safe and enough for the read side.
+*
 * When dereferencing this pointer, either hold struct
 * _device.master_mutex for the duration of the pointer's use, or
 * use drm_file_get_master() if struct _device.master_mutex is not
-- 
2.25.1



[RESEND PATCH v2 1/2] locking/lockdep: Provide lockdep_assert{, _once}() helpers

2021-08-02 Thread Desmond Cheong Zhi Xi
From: Peter Zijlstra 

Extract lockdep_assert{,_once}() helpers to more easily write composite
assertions like, for example:

lockdep_assert(lockdep_is_held(_device.master_mutex) ||
   lockdep_is_held(_file.master_lookup_lock));

Signed-off-by: Peter Zijlstra (Intel) 
Signed-off-by: Desmond Cheong Zhi Xi 
Acked-by: Boqun Feng 
Acked-by: Waiman Long 
Acked-by: Peter Zijlstra (Intel) 
---
 include/linux/lockdep.h | 41 +
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 5cf387813754..9fe165beb0f9 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -306,31 +306,29 @@ extern void lock_unpin_lock(struct lockdep_map *lock, 
struct pin_cookie);
 
 #define lockdep_depth(tsk) (debug_locks ? (tsk)->lockdep_depth : 0)
 
-#define lockdep_assert_held(l) do {\
-   WARN_ON(debug_locks &&  \
-   lockdep_is_held(l) == LOCK_STATE_NOT_HELD); \
-   } while (0)
+#define lockdep_assert(cond)   \
+   do { WARN_ON(debug_locks && !(cond)); } while (0)
 
-#define lockdep_assert_not_held(l) do {\
-   WARN_ON(debug_locks &&  \
-   lockdep_is_held(l) == LOCK_STATE_HELD); \
-   } while (0)
+#define lockdep_assert_once(cond)  \
+   do { WARN_ON_ONCE(debug_locks && !(cond)); } while (0)
 
-#define lockdep_assert_held_write(l)   do {\
-   WARN_ON(debug_locks && !lockdep_is_held_type(l, 0));\
-   } while (0)
+#define lockdep_assert_held(l) \
+   lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD)
 
-#define lockdep_assert_held_read(l)do {\
-   WARN_ON(debug_locks && !lockdep_is_held_type(l, 1));\
-   } while (0)
+#define lockdep_assert_not_held(l) \
+   lockdep_assert(lockdep_is_held(l) != LOCK_STATE_HELD)
 
-#define lockdep_assert_held_once(l)do {\
-   WARN_ON_ONCE(debug_locks && !lockdep_is_held(l));   \
-   } while (0)
+#define lockdep_assert_held_write(l)   \
+   lockdep_assert(lockdep_is_held_type(l, 0))
 
-#define lockdep_assert_none_held_once()do {
\
-   WARN_ON_ONCE(debug_locks && current->lockdep_depth);\
-   } while (0)
+#define lockdep_assert_held_read(l)\
+   lockdep_assert(lockdep_is_held_type(l, 1))
+
+#define lockdep_assert_held_once(l)\
+   lockdep_assert_once(lockdep_is_held(l) != LOCK_STATE_NOT_HELD)
+
+#define lockdep_assert_none_held_once()\
+   lockdep_assert_once(!current->lockdep_depth)
 
 #define lockdep_recursing(tsk) ((tsk)->lockdep_recursion)
 
@@ -407,6 +405,9 @@ extern int lock_is_held(const void *);
 extern int lockdep_is_held(const void *);
 #define lockdep_is_held_type(l, r) (1)
 
+#define lockdep_assert(c)  do { } while (0)
+#define lockdep_assert_once(c) do { } while (0)
+
 #define lockdep_assert_held(l) do { (void)(l); } while (0)
 #define lockdep_assert_not_held(l) do { (void)(l); } while (0)
 #define lockdep_assert_held_write(l)   do { (void)(l); } while (0)
-- 
2.25.1



[RESEND PATCH v2 0/2] locking/lockdep, drm: apply new lockdep assert in drm_auth.c

2021-08-02 Thread Desmond Cheong Zhi Xi
Hi all,

My bad for the resend. Adding cc: intel-gfx, and the maintainers and
mailing lists for include/drm/drm_file.h.

Following a discussion on the patch ("drm: use the lookup lock in
drm_is_current_master") [1], Peter Zijlstra proposed new lockdep_assert
helpers to make it convenient to compose lockdep checks together.

This series includes the patch that introduces the new lockdep helpers,
then utilizes these helpers in drm_is_current_master_locked in the
following patch.

v1 -> v2:
Patch 2:
- Updated the kerneldoc on the lock design of drm_file.master to explain
the use of lockdep_assert(). As suggested by Boqun Feng.

Link: 
https://lore.kernel.org/lkml/20210722092929.244629-2-desmondcheon...@gmail.com/ 
[1]

Best wishes,
Desmond

Desmond Cheong Zhi Xi (1):
  drm: add lockdep assert to drm_is_current_master_locked

Peter Zijlstra (1):
  locking/lockdep: Provide lockdep_assert{,_once}() helpers

 drivers/gpu/drm/drm_auth.c |  6 +++---
 include/drm/drm_file.h |  4 
 include/linux/lockdep.h| 41 +++---
 3 files changed, 28 insertions(+), 23 deletions(-)

-- 
2.25.1



Re: [PATCH 0/3] iommu/drm/msm: Allow non-coherent masters to use system cache

2021-08-02 Thread Will Deacon
On Thu, Jul 29, 2021 at 10:08:22AM +0530, Sai Prakash Ranjan wrote:
> On 2021-07-28 19:30, Georgi Djakov wrote:
> > On Mon, Jan 11, 2021 at 07:45:02PM +0530, Sai Prakash Ranjan wrote:
> > > commit ecd7274fb4cd ("iommu: Remove unused IOMMU_SYS_CACHE_ONLY flag")
> > > removed unused IOMMU_SYS_CACHE_ONLY prot flag and along with it went
> > > the memory type setting required for the non-coherent masters to use
> > > system cache. Now that system cache support for GPU is added, we will
> > > need to set the right PTE attribute for GPU buffers to be sys cached.
> > > Without this, the system cache lines are not allocated for GPU.
> > > 
> > > So the patches in this series introduces a new prot flag IOMMU_LLC,
> > > renames IO_PGTABLE_QUIRK_ARM_OUTER_WBWA to IO_PGTABLE_QUIRK_PTW_LLC
> > > and makes GPU the user of this protection flag.
> > 
> > Thank you for the patchset! Are you planning to refresh it, as it does
> > not apply anymore?
> > 
> 
> I was waiting on Will's reply [1]. If there are no changes needed, then
> I can repost the patch.

I still think you need to handle the mismatched alias, no? You're adding
a new memory type to the SMMU which doesn't exist on the CPU side. That
can't be right.

Will


Re: [PATCH 08/11] mm: Remove the now unused mem_encrypt_active() function

2021-08-02 Thread Joerg Roedel
On Tue, Jul 27, 2021 at 05:26:11PM -0500, Tom Lendacky wrote:
> The mem_encrypt_active() function has been replaced by prot_guest_has(),
> so remove the implementation.
> 
> Signed-off-by: Tom Lendacky 

Reviewed-by: Joerg Roedel 


Re: [PATCH 09/11] x86/sev: Remove the now unused mem_encrypt_active() function

2021-08-02 Thread Joerg Roedel
On Tue, Jul 27, 2021 at 05:26:12PM -0500, Tom Lendacky wrote:
> The mem_encrypt_active() function has been replaced by prot_guest_has(),
> so remove the implementation.
> 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Signed-off-by: Tom Lendacky 

Reviewed-by: Joerg Roedel 


Re: [PATCH 06/11] x86/sev: Replace occurrences of sev_es_active() with prot_guest_has()

2021-08-02 Thread Joerg Roedel
On Tue, Jul 27, 2021 at 05:26:09PM -0500, Tom Lendacky wrote:
> @@ -48,7 +47,7 @@ static void sme_sev_setup_real_mode(struct 
> trampoline_header *th)
>   if (prot_guest_has(PATTR_HOST_MEM_ENCRYPT))
>   th->flags |= TH_FLAGS_SME_ACTIVE;
>  
> - if (sev_es_active()) {
> + if (prot_guest_has(PATTR_GUEST_PROT_STATE)) {
>   /*
>* Skip the call to verify_cpu() in secondary_startup_64 as it
>* will cause #VC exceptions when the AP can't handle them yet.

Not sure how TDX will handle AP booting, are you sure it needs this
special setup as well? Otherwise a check for SEV-ES would be better
instead of the generic PATTR_GUEST_PROT_STATE.

Regards,

Joerg


Re: [PATCH v2 0/2] locking/lockdep, drm: apply new lockdep assert in drm_auth.c

2021-08-02 Thread Desmond Cheong Zhi Xi

On 2/8/21 4:26 pm, Daniel Vetter wrote:

On Sat, Jul 31, 2021 at 04:24:56PM +0800, Desmond Cheong Zhi Xi wrote:

Hi,

Following a discussion on the patch ("drm: use the lookup lock in
drm_is_current_master") [1], Peter Zijlstra proposed new lockdep_assert
helpers to make it convenient to compose lockdep checks together.

This series includes the patch that introduces the new lockdep helpers,
then utilizes these helpers in drm_is_current_master_locked in the
following patch.

v1 -> v2:
Patch 2:
- Updated the kerneldoc on the lock design of drm_file.master to explain
the use of lockdep_assert(). As suggested by Boqun Feng.

Link: 
https://lore.kernel.org/lkml/20210722092929.244629-2-desmondcheon...@gmail.com/ 
[1]


Can you pls also cc: this to intel-gfx so the local CI there can pick it
up and verify? Just to check we got it all.
-Daniel



Oops my bad, I missed out the CI for this series. Will resend with the 
proper cc.


Best wishes,
Desmond





Best wishes,
Desmond

Desmond Cheong Zhi Xi (1):
   drm: add lockdep assert to drm_is_current_master_locked

Peter Zijlstra (1):
   locking/lockdep: Provide lockdep_assert{,_once}() helpers

  drivers/gpu/drm/drm_auth.c |  6 +++---
  include/drm/drm_file.h |  4 
  include/linux/lockdep.h| 41 +++---
  3 files changed, 28 insertions(+), 23 deletions(-)

--
2.25.1







Re: [PATCH 05/11] x86/sev: Replace occurrences of sev_active() with prot_guest_has()

2021-08-02 Thread Joerg Roedel
On Tue, Jul 27, 2021 at 05:26:08PM -0500, Tom Lendacky wrote:
> Replace occurrences of sev_active() with the more generic prot_guest_has()
> using PATTR_GUEST_MEM_ENCRYPT, except for in arch/x86/mm/mem_encrypt*.c
> where PATTR_SEV will be used. If future support is added for other memory
> encryption technologies, the use of PATTR_GUEST_MEM_ENCRYPT can be
> updated, as required, to use PATTR_SEV.
> 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: Dave Hansen 
> Cc: Andy Lutomirski 
> Cc: Peter Zijlstra 
> Cc: Ard Biesheuvel 
> Signed-off-by: Tom Lendacky 

Reviewed-by: Joerg Roedel 


Re: [PATCH 04/11] x86/sme: Replace occurrences of sme_active() with prot_guest_has()

2021-08-02 Thread Joerg Roedel
On Tue, Jul 27, 2021 at 05:26:07PM -0500, Tom Lendacky wrote:
> Replace occurrences of sme_active() with the more generic prot_guest_has()
> using PATTR_HOST_MEM_ENCRYPT, except for in arch/x86/mm/mem_encrypt*.c
> where PATTR_SME will be used. If future support is added for other memory
> encryption technologies, the use of PATTR_HOST_MEM_ENCRYPT can be
> updated, as required, to use PATTR_SME.
> 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: Dave Hansen 
> Cc: Andy Lutomirski 
> Cc: Peter Zijlstra 
> Cc: Joerg Roedel 
> Cc: Will Deacon 
> Signed-off-by: Tom Lendacky 

Reviewed-by: Joerg Roedel 


[PATCH v3 0/3] Mainline imx6 based SKOV boards

2021-08-02 Thread Oleksij Rempel
changes v3:
- drop panel bindings patches, it is already in drm-misc-next
- remove some new lines
- reorder compatibles at the start of the nodes
- use lowercase for hex value
- add enable-active-high to the regulator-vcc-mmc-io and fix MMC voltage
  configuration.

changes v2:
- remove unnecessary newlines.
- change linux,wakeup to wakeup-source
- change switch@3 unit-address to @0
- sort aliases alphabetically

Mainline imx6 based DTs for SKOV A/S boards

Oleksij Rempel (2):
  dt-bindings: vendor-prefixes: Add an entry for SKOV A/S
  dt-bindings: arm: fsl: add SKOV imx6q and imx6dl based boards

Sam Ravnborg (1):
  ARM: dts: add SKOV imx6q and imx6dl based boards

 .../devicetree/bindings/arm/fsl.yaml  |   5 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 arch/arm/boot/dts/Makefile|   5 +
 arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts|  13 +
 arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts| 106 
 arch/arm/boot/dts/imx6q-skov-revc-lt2.dts |  36 ++
 arch/arm/boot/dts/imx6q-skov-revc-lt6.dts | 128 +
 .../dts/imx6q-skov-reve-mi1010ait-1cp1.dts| 127 +
 arch/arm/boot/dts/imx6qdl-skov-cpu-revc.dtsi  |  54 ++
 arch/arm/boot/dts/imx6qdl-skov-cpu.dtsi   | 476 ++
 10 files changed, 952 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts
 create mode 100644 arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts
 create mode 100644 arch/arm/boot/dts/imx6q-skov-revc-lt2.dts
 create mode 100644 arch/arm/boot/dts/imx6q-skov-revc-lt6.dts
 create mode 100644 arch/arm/boot/dts/imx6q-skov-reve-mi1010ait-1cp1.dts
 create mode 100644 arch/arm/boot/dts/imx6qdl-skov-cpu-revc.dtsi
 create mode 100644 arch/arm/boot/dts/imx6qdl-skov-cpu.dtsi

-- 
2.30.2



[PATCH v3 1/3] dt-bindings: vendor-prefixes: Add an entry for SKOV A/S

2021-08-02 Thread Oleksij Rempel
Add "skov" entry for the SKOV A/S: https://www.skov.com/en/

Signed-off-by: Oleksij Rempel 
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml 
b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 62cb1d9341f5..738d4f44f0de 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1068,6 +1068,8 @@ patternProperties:
 description: Silicon Integrated Systems Corp.
   "^sitronix,.*":
 description: Sitronix Technology Corporation
+  "^skov,.*":
+description: SKOV A/S
   "^skyworks,.*":
 description: Skyworks Solutions, Inc.
   "^smartlabs,.*":
-- 
2.30.2



[PATCH v3 3/3] ARM: dts: add SKOV imx6q and imx6dl based boards

2021-08-02 Thread Oleksij Rempel
From: Sam Ravnborg 

Add SKOV imx6q/dl LT2, LT6 and mi1010ait-1cp1 boards.

Signed-off-by: Sam Ravnborg 
Signed-off-by: Søren Andersen 
Signed-off-by: Juergen Borleis 
Signed-off-by: Ulrich Ölmann 
Signed-off-by: Michael Grzeschik 
Signed-off-by: Marco Felsch 
Signed-off-by: Lucas Stach 
Signed-off-by: Oleksij Rempel 
---
 arch/arm/boot/dts/Makefile|   5 +
 arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts|  13 +
 arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts| 106 
 arch/arm/boot/dts/imx6q-skov-revc-lt2.dts |  36 ++
 arch/arm/boot/dts/imx6q-skov-revc-lt6.dts | 128 +
 .../dts/imx6q-skov-reve-mi1010ait-1cp1.dts| 127 +
 arch/arm/boot/dts/imx6qdl-skov-cpu-revc.dtsi  |  54 ++
 arch/arm/boot/dts/imx6qdl-skov-cpu.dtsi   | 476 ++
 8 files changed, 945 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts
 create mode 100644 arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts
 create mode 100644 arch/arm/boot/dts/imx6q-skov-revc-lt2.dts
 create mode 100644 arch/arm/boot/dts/imx6q-skov-revc-lt6.dts
 create mode 100644 arch/arm/boot/dts/imx6q-skov-reve-mi1010ait-1cp1.dts
 create mode 100644 arch/arm/boot/dts/imx6qdl-skov-cpu-revc.dtsi
 create mode 100644 arch/arm/boot/dts/imx6qdl-skov-cpu.dtsi

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 828fefc9c436..ecff0743d35f 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -475,6 +475,8 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
imx6dl-sabrelite.dtb \
imx6dl-sabresd.dtb \
imx6dl-savageboard.dtb \
+   imx6dl-skov-revc-lt2.dtb \
+   imx6dl-skov-revc-lt6.dtb \
imx6dl-solidsense.dtb \
imx6dl-ts4900.dtb \
imx6dl-ts7970.dtb \
@@ -576,6 +578,9 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
imx6q-sabresd.dtb \
imx6q-savageboard.dtb \
imx6q-sbc6x.dtb \
+   imx6q-skov-revc-lt2.dtb \
+   imx6q-skov-revc-lt6.dtb \
+   imx6q-skov-reve-mi1010ait-1cp1.dtb \
imx6q-solidsense.dtb \
imx6q-tbs2910.dtb \
imx6q-ts4900.dtb \
diff --git a/arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts 
b/arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts
new file mode 100644
index ..667b8faa1807
--- /dev/null
+++ b/arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+//
+// Copyright (C) 2020 Pengutronix, Ulrich Oelmann 
+
+/dts-v1/;
+#include "imx6dl.dtsi"
+#include "imx6qdl-skov-cpu.dtsi"
+#include "imx6qdl-skov-cpu-revc.dtsi"
+
+/ {
+   model = "SKOV IMX6 CPU SoloCore";
+   compatible = "skov,imx6dl-skov-revc-lt2", "fsl,imx6dl";
+};
diff --git a/arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts 
b/arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts
new file mode 100644
index ..5dcc433fe2af
--- /dev/null
+++ b/arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+//
+// Copyright (C) 2020 Pengutronix, Ulrich Oelmann 
+
+/dts-v1/;
+#include "imx6dl.dtsi"
+#include "imx6qdl-skov-cpu.dtsi"
+#include "imx6qdl-skov-cpu-revc.dtsi"
+
+/ {
+   model = "SKOV IMX6 CPU SoloCore";
+   compatible = "skov,imx6dl-skov-revc-lt6", "fsl,imx6dl";
+
+   backlight: backlight {
+   compatible = "pwm-backlight";
+   pinctrl-names = "default";
+   pinctrl-0 = <_backlight>;
+   enable-gpios = < 23 GPIO_ACTIVE_LOW>;
+   pwms = < 0 2 0>;
+   brightness-levels = <0 255>;
+   num-interpolated-steps = <17>;
+   default-brightness-level = <8>;
+   power-supply = <_24v0>;
+   };
+
+   display {
+   compatible = "fsl,imx-parallel-display";
+   pinctrl-names = "default";
+   pinctrl-0 = <_ipu1>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+
+   display0_in: endpoint {
+   remote-endpoint = <_di0_disp0>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+
+   display0_out: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+   };
+
+   panel {
+   compatible = "logictechno,lttd800480070-l6wh-rt";
+   backlight = <>;
+   power-supply = <_3v3>;
+
+   port {
+   panel_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   };
+};
+
+_di0_disp0 {
+   remote-endpoint = <_in>;
+};
+
+ {
+   pinctrl_backlight: backlightgrp {
+   fsl,pins = <
+   MX6QDL_PAD_RGMII_TD3__GPIO6_IO230x58
+   >;
+   };
+
+   pinctrl_ipu1: ipu1grp {
+   fsl,pins = <
+  

[PATCH v3 2/3] dt-bindings: arm: fsl: add SKOV imx6q and imx6dl based boards

2021-08-02 Thread Oleksij Rempel
Add SKOV imx6q/dl LT2, LT6 and mi1010ait-1cp1 boards.

Signed-off-by: Oleksij Rempel 
---
 Documentation/devicetree/bindings/arm/fsl.yaml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml 
b/Documentation/devicetree/bindings/arm/fsl.yaml
index e2097011c4b0..600dfe984ab3 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -221,6 +221,9 @@ properties:
   - prt,prti6q# Protonic PRTI6Q board
   - prt,prtwd2# Protonic WD2 board
   - rex,imx6q-rex-pro # Rex Pro i.MX6 Quad Board
+  - skov,imx6q-skov-revc-lt2  # SKOV IMX6 CPU QuadCore lt2
+  - skov,imx6q-skov-revc-lt6  # SKOV IMX6 CPU QuadCore lt6
+  - skov,imx6q-skov-reve-mi1010ait-1cp1 # SKOV IMX6 CPU QuadCore 
mi1010ait-1cp1
   - solidrun,cubox-i/q# SolidRun Cubox-i Dual/Quad
   - solidrun,hummingboard/q
   - solidrun,hummingboard2/q
@@ -378,6 +381,8 @@ properties:
   - prt,prtvt7# Protonic VT7 board
   - rex,imx6dl-rex-basic  # Rex Basic i.MX6 Dual Lite Board
   - riot,imx6s-riotboard  # RIoTboard i.MX6S
+  - skov,imx6dl-skov-revc-lt2 # SKOV IMX6 CPU SoloCore lt2
+  - skov,imx6dl-skov-revc-lt6 # SKOV IMX6 CPU SoloCore lt6
   - solidrun,cubox-i/dl# SolidRun Cubox-i Solo/DualLite
   - solidrun,hummingboard/dl
   - solidrun,hummingboard2/dl  # SolidRun HummingBoard2 
Solo/DualLite
-- 
2.30.2



Re: [PATCH 02/11] x86/sev: Add an x86 version of prot_guest_has()

2021-08-02 Thread Joerg Roedel
On Tue, Jul 27, 2021 at 05:26:05PM -0500, Tom Lendacky wrote:
> Introduce an x86 version of the prot_guest_has() function. This will be
> used in the more generic x86 code to replace vendor specific calls like
> sev_active(), etc.
> 
> While the name suggests this is intended mainly for guests, it will
> also be used for host memory encryption checks in place of sme_active().
> 
> The amd_prot_guest_has() function does not use EXPORT_SYMBOL_GPL for the
> same reasons previously stated when changing sme_active(), sev_active and
> sme_me_mask to EXPORT_SYBMOL:
>   commit 87df26175e67 ("x86/mm: Unbreak modules that rely on external 
> PAGE_KERNEL availability")
>   commit 9d5f38ba6c82 ("x86/mm: Unbreak modules that use the DMA API")
> 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: Dave Hansen 
> Cc: Andy Lutomirski 
> Cc: Peter Zijlstra 
> Co-developed-by: Andi Kleen 
> Signed-off-by: Andi Kleen 
> Co-developed-by: Kuppuswamy Sathyanarayanan 
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan 
> 
> Signed-off-by: Tom Lendacky 

Reviewed-by: Joerg Roedel 


Re: [PATCH 01/11] mm: Introduce a function to check for virtualization protection features

2021-08-02 Thread Joerg Roedel
On Tue, Jul 27, 2021 at 05:26:04PM -0500, Tom Lendacky wrote:
> In prep for other protected virtualization technologies, introduce a
> generic helper function, prot_guest_has(), that can be used to check
> for specific protection attributes, like memory encryption. This is
> intended to eliminate having to add multiple technology-specific checks
> to the code (e.g. if (sev_active() || tdx_active())).
> 
> Co-developed-by: Andi Kleen 
> Signed-off-by: Andi Kleen 
> Co-developed-by: Kuppuswamy Sathyanarayanan 
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan 
> 
> Signed-off-by: Tom Lendacky 

Reviewed-by: Joerg Roedel 


Re: [PATCH v7 00/12] Clean up "mediatek,larb"

2021-08-02 Thread Joerg Roedel
On Fri, Jul 30, 2021 at 10:52:26AM +0800, Yong Wu wrote:
>  .../display/mediatek/mediatek,disp.txt|  9 
>  .../bindings/media/mediatek-jpeg-decoder.yaml |  9 
>  .../bindings/media/mediatek-jpeg-encoder.yaml |  9 
>  .../bindings/media/mediatek-mdp.txt   |  8 
>  .../bindings/media/mediatek-vcodec.txt|  4 --
>  arch/arm/boot/dts/mt2701.dtsi |  2 -
>  arch/arm/boot/dts/mt7623n.dtsi|  5 --
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi  | 16 ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi  |  6 ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c   |  9 +++-
>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c  |  9 +++-
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c   | 15 +++---
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c   | 36 +--
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  1 -
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c|  5 +-
>  drivers/iommu/mtk_iommu.c | 24 +-
>  drivers/iommu/mtk_iommu_v1.c  | 31 -
>  .../media/platform/mtk-jpeg/mtk_jpeg_core.c   | 45 +-
>  .../media/platform/mtk-jpeg/mtk_jpeg_core.h   |  2 -
>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 46 +--
>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.h |  2 -
>  drivers/media/platform/mtk-mdp/mtk_mdp_core.c |  1 -
>  .../platform/mtk-vcodec/mtk_vcodec_dec_pm.c   | 37 ++-
>  .../platform/mtk-vcodec/mtk_vcodec_drv.h  |  3 --
>  .../platform/mtk-vcodec/mtk_vcodec_enc.c  |  1 -
>  .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c   | 44 ++
>  drivers/memory/mtk-smi.c  | 14 --
>  include/soc/mediatek/smi.h| 20 
>  28 files changed, 92 insertions(+), 321 deletions(-)

So this is likely not going through the IOMMU tree, given Matthias has
reviewed the IOMMU changes you can add my Acked-by: Joerg Roedel 



Re: [PATCH v2 1/2] locking/lockdep: Provide lockdep_assert{, _once}() helpers

2021-08-02 Thread Maarten Lankhorst
Op 31-07-2021 om 10:24 schreef Desmond Cheong Zhi Xi:
> From: Peter Zijlstra 
>
> Extract lockdep_assert{,_once}() helpers to more easily write composite
> assertions like, for example:
>
>   lockdep_assert(lockdep_is_held(_device.master_mutex) ||
>  lockdep_is_held(_file.master_lookup_lock));
>
> Signed-off-by: Peter Zijlstra (Intel) 
> Signed-off-by: Desmond Cheong Zhi Xi 
> Acked-by: Boqun Feng 
> Acked-by: Waiman Long 
> ---
>  include/linux/lockdep.h | 41 +
>  1 file changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> index 5cf387813754..9fe165beb0f9 100644
> --- a/include/linux/lockdep.h
> +++ b/include/linux/lockdep.h
> @@ -306,31 +306,29 @@ extern void lock_unpin_lock(struct lockdep_map *lock, 
> struct pin_cookie);
>  
>  #define lockdep_depth(tsk)   (debug_locks ? (tsk)->lockdep_depth : 0)
>  
> -#define lockdep_assert_held(l)   do {
> \
> - WARN_ON(debug_locks &&  \
> - lockdep_is_held(l) == LOCK_STATE_NOT_HELD); \
> - } while (0)
> +#define lockdep_assert(cond) \
> + do { WARN_ON(debug_locks && !(cond)); } while (0)
>  
> -#define lockdep_assert_not_held(l)   do {\
> - WARN_ON(debug_locks &&  \
> - lockdep_is_held(l) == LOCK_STATE_HELD); \
> - } while (0)
> +#define lockdep_assert_once(cond)\
> + do { WARN_ON_ONCE(debug_locks && !(cond)); } while (0)
>  
> -#define lockdep_assert_held_write(l) do {\
> - WARN_ON(debug_locks && !lockdep_is_held_type(l, 0));\
> - } while (0)
> +#define lockdep_assert_held(l)   \
> + lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD)
>  
> -#define lockdep_assert_held_read(l)  do {\
> - WARN_ON(debug_locks && !lockdep_is_held_type(l, 1));\
> - } while (0)
> +#define lockdep_assert_not_held(l)   \
> + lockdep_assert(lockdep_is_held(l) != LOCK_STATE_HELD)
>  
> -#define lockdep_assert_held_once(l)  do {\
> - WARN_ON_ONCE(debug_locks && !lockdep_is_held(l));   \
> - } while (0)
> +#define lockdep_assert_held_write(l) \
> + lockdep_assert(lockdep_is_held_type(l, 0))
>  
> -#define lockdep_assert_none_held_once()  do {
> \
> - WARN_ON_ONCE(debug_locks && current->lockdep_depth);\
> - } while (0)
> +#define lockdep_assert_held_read(l)  \
> + lockdep_assert(lockdep_is_held_type(l, 1))
> +
> +#define lockdep_assert_held_once(l)  \
> + lockdep_assert_once(lockdep_is_held(l) != LOCK_STATE_NOT_HELD)
> +
> +#define lockdep_assert_none_held_once()  \
> + lockdep_assert_once(!current->lockdep_depth)
>  
>  #define lockdep_recursing(tsk)   ((tsk)->lockdep_recursion)
>  
> @@ -407,6 +405,9 @@ extern int lock_is_held(const void *);
>  extern int lockdep_is_held(const void *);
>  #define lockdep_is_held_type(l, r)   (1)
>  
> +#define lockdep_assert(c)do { } while (0)
> +#define lockdep_assert_once(c)   do { } while (0)
> +
>  #define lockdep_assert_held(l)   do { (void)(l); } while 
> (0)
>  #define lockdep_assert_not_held(l)   do { (void)(l); } while (0)
>  #define lockdep_assert_held_write(l) do { (void)(l); } while (0)

All other macros seem to do (void)(c); in this case?


Otherwise looks good.



Re: [PATCH v2 0/2] locking/lockdep, drm: apply new lockdep assert in drm_auth.c

2021-08-02 Thread Peter Zijlstra
On Mon, Aug 02, 2021 at 10:26:16AM +0200, Daniel Vetter wrote:
> On Sat, Jul 31, 2021 at 04:24:56PM +0800, Desmond Cheong Zhi Xi wrote:
> > Hi,
> > 
> > Following a discussion on the patch ("drm: use the lookup lock in
> > drm_is_current_master") [1], Peter Zijlstra proposed new lockdep_assert
> > helpers to make it convenient to compose lockdep checks together.
> > 
> > This series includes the patch that introduces the new lockdep helpers,
> > then utilizes these helpers in drm_is_current_master_locked in the
> > following patch.
> > 
> > v1 -> v2:
> > Patch 2:
> > - Updated the kerneldoc on the lock design of drm_file.master to explain
> > the use of lockdep_assert(). As suggested by Boqun Feng.
> > 
> > Link: 
> > https://lore.kernel.org/lkml/20210722092929.244629-2-desmondcheon...@gmail.com/
> >  [1]
> 
> Can you pls also cc: this to intel-gfx so the local CI there can pick it
> up and verify? Just to check we got it all.

Acked-by: Peter Zijlstra (Intel) 

Feel free to take it through the drm tree.


Re: [Intel-gfx] [PATCH 1/1] drm/i915: Check if engine has heartbeat when closing a context

2021-08-02 Thread Tvrtko Ursulin



On 30/07/2021 19:13, John Harrison wrote:

On 7/30/2021 02:49, Tvrtko Ursulin wrote:

On 30/07/2021 01:13, John Harrison wrote:

On 7/28/2021 17:34, Matthew Brost wrote:
If an engine associated with a context does not have a heartbeat, 
ban it

immediately. This is needed for GuC submission as a idle pulse doesn't
kick the context off the hardware where it then can check for a
heartbeat and ban the context.


Pulse, that is a request with I915_PRIORITY_BARRIER, does not preempt 
a running normal priority context?


Why does it matter then whether or not heartbeats are enabled - when 
heartbeat just ends up sending the same engine pulse (eventually, with 
raising priority)?
The point is that the pulse is pointless. See the rest of my comments 
below, specifically "the context will get resubmitted to the hardware 
after the pulse completes". To re-iterate...


Yes, it preempts the context. Yes, it does so whether heartbeats are 
enabled or not. But so what? Who cares? You have preempted a context. It 
is no longer running on the hardware. BUT IT IS STILL A VALID CONTEXT. 


It is valid yes, and it even may be the current ABI so another question 
is whether it is okay to change that.


The backend scheduler will just resubmit it to the hardware as soon as 
the pulse completes. The only reason this works at all is because of the 
horrid hack in the execlist scheduler's back end implementation (in 
__execlists_schedule_in):

     if (unlikely(intel_context_is_closed(ce) &&
  !intel_engine_has_heartbeat(engine)))
     intel_context_set_banned(ce);


Right, is the above code then needed with this patch - when ban is 
immediately applied on the higher level?


The actual back end scheduler is saying "Is this a zombie context? Is 
the heartbeat disabled? Then ban it". No other scheduler backend is 
going to have knowledge of zombie context status or of the heartbeat 
status. Nor are they going to call back into the higher levels of the 
i915 driver to trigger a ban operation. Certainly a hardware implemented 
scheduler is not going to be looking at private i915 driver information 
to decide whether to submit a context or whether to tell the OS to kill 
it off instead.


For persistence to work with a hardware scheduler (or a non-Intel 
specific scheduler such as the DRM one), the handling of zombie 
contexts, banning, etc. *must* be done entirely in the front end. It 
cannot rely on any backend hacks. That means you can't rely on any fancy 
behaviour of pulses.


If you want to ban a context then you must explicitly ban that context. 
If you want to ban it at some later point then you need to track it at 
the top level as a zombie and then explicitly ban that zombie at 
whatever later point.


I am still trying to understand it all. If I go by the commit message:

"""
This is needed for GuC submission as a idle pulse doesn't
kick the context off the hardware where it then can check for a
heartbeat and ban the context.
"""

That did not explain things for me. Sentence does not appear to make 
sense. Now, it seems "kick off the hardware" is meant as revoke and not 
just preempt. Which is fine, perhaps just needs to be written more 
explicitly. But the part of checking for heartbeat after idle pulse does 
not compute for me. It is the heartbeat which emits idle pulses, not 
idle pulse emitting heartbeats.


But anyway, I can buy the handling at the front end story completely. It 
makes sense. We just need to agree that a) it is okay to change the ABI 
and b) remove the backend check from execlists if it is not needed any 
longer.


And if ABI change is okay then commit message needs to talk about it 
loudly and clearly.


Or perhaps there is no ABI change? I am not really clear how does 
setting banned status propagate to the GuC backend. I mean at which 
point does i915 ends up passing that info to the firmware?


Regards,

Tvrtko






It's worse than this. If the engine in question is an individual 
physical engine then sending a pulse (with sufficiently high 
priority) will pre-empt the engine and kick the context off. However, 
the GuC 


Why it is different for physical vs virtual, aren't both just 
schedulable contexts with different engine masks for what GuC is 
concerned? Oh, is it a matter of needing to send pulses to all engines 
which comprise a virtual one?
It isn't different. It is totally broken for both. It is potentially 
more broken for virtual engines because of the question of which engine 
to pulse. But as stated above, the pulse is pointless anyway so the 
which engine question doesn't even matter.


John.




scheduler does not have hacks in it to check the state of the 
heartbeat or whether a context is actually a zombie or not. Thus, the 
context will get resubmitted to the hardware after the pulse 
completes and effectively nothing will have happened.


I would assume that the DRM scheduler which we are meant to be 
switching to for execlist as well as 

Re: [PATCH v2 1/2] drm/i915: document caching related bits

2021-08-02 Thread Mika Kuoppala
Matthew Auld  writes:

> Try to document the object caching related bits, like cache_coherent and
> cache_dirty.
>
> v2(Ville):
>  - As pointed out by Ville, fix the completely incorrect assumptions
>about the "partial" coherency on shared LLC platforms.
>
> Suggested-by: Daniel Vetter 
> Signed-off-by: Matthew Auld 
> Cc: Ville Syrjälä 
> Cc: Mika Kuoppala 
> ---
>  .../gpu/drm/i915/gem/i915_gem_object_types.h  | 173 +-
>  drivers/gpu/drm/i915/i915_drv.h   |   9 -
>  2 files changed, 169 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h 
> b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> index ef3de2ae9723..a809424bc8c1 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> @@ -92,6 +92,76 @@ struct drm_i915_gem_object_ops {
>   const char *name; /* friendly name for debug, e.g. lockdep classes */
>  };
>  
> +/**
> + * enum i915_cache_level - The supported GTT caching values for system memory
> + * pages.
> + *
> + * These translate to some special GTT PTE bits when binding pages into some
> + * address space. It also determines whether an object, or rather its pages 
> are
> + * coherent with the GPU, when also reading or writing through the CPU cache
> + * with those pages.
> + *
> + * Userspace can also control this through struct drm_i915_gem_caching.
> + */
> +enum i915_cache_level {
> + /**
> +  * @I915_CACHE_NONE:
> +  *
> +  * Not coherent with the CPU cache. If the cache is dirty and we need
> +  * the underlying pages to be coherent with some later GPU access then
> +  * we need to manually flush the pages.
> +  *
> +  * Note that on shared LLC platforms reads and writes through the CPU
> +  * cache are still coherent even with this setting. See also
> +  * _i915_gem_object.cache_coherent for more details.
> +  *
> +  * Note that on platforms with a shared LLC this should ideally only be
> +  * used for scanout surfaces, otherwise we end up over-flushing in some
> +  * places.
> +  */
> + I915_CACHE_NONE = 0,
> + /**
> +  * @I915_CACHE_LLC:
> +  *
> +  * Coherent with the CPU cache. If the cache is dirty, then the GPU will
> +  * ensure that access remains coherent, when both reading and writing
> +  * through the CPU cache.
> +  *
> +  * Not used for scanout surfaces.
> +  *
> +  * Applies to both platforms with shared LLC(HAS_LLC), and snooping
> +  * based platforms(HAS_SNOOP).
> +  *
> +  * This should be the default for platforms which share the LLC with the
> +  * CPU. The only exception is scanout objects, where the display engine
> +  * is not coherent with the LLC. For such objects I915_CACHE_NONE or
> +  * I915_CACHE_WT should be used.
> +  */
> + I915_CACHE_LLC,
> + /**
> +  * @I915_CACHE_L3_LLC:
> +  *
> +  * Explicitly enable the Gfx L3 cache, with snooped LLC.
> +  *
> +  * The Gfx L3 sits between the domain specific caches, e.g
> +  * sampler/render caches, and the larger LLC. LLC is coherent with the
> +  * GPU, but L3 is only visible to the GPU, so likely needs to be flushed
> +  * when the workload completes.
> +  *
> +  * Not used for scanout surfaces.
> +  *
> +  * Only exposed on some gen7 + GGTT. More recent hardware has dropped
> +  * this.
> +  */

This is stellar. Thanks!
-Mika

> + I915_CACHE_L3_LLC,
> + /**
> +  * @I915_CACHE_WT:
> +  *
> +  * hsw:gt3e Write-through for scanout buffers.
> +  */
> + I915_CACHE_WT,
> +};
> +
>  enum i915_map_type {
>   I915_MAP_WB = 0,
>   I915_MAP_WC,
> @@ -228,14 +298,109 @@ struct drm_i915_gem_object {
>   unsigned int mem_flags;
>  #define I915_BO_FLAG_STRUCT_PAGE BIT(0) /* Object backed by struct pages */
>  #define I915_BO_FLAG_IOMEM   BIT(1) /* Object backed by IO memory */
> - /*
> -  * Is the object to be mapped as read-only to the GPU
> -  * Only honoured if hardware has relevant pte bit
> + /**
> +  * @cache_level: The desired GTT caching level.
> +  *
> +  * See enum i915_cache_level for possible values, along with what
> +  * each does.
>*/
>   unsigned int cache_level:3;
> - unsigned int cache_coherent:2;
> + /**
> +  * @cache_coherent:
> +  *
> +  * Track whether the pages are coherent with the GPU if reading or
> +  * writing through the CPU caches. The largely depends on the
> +  * @cache_level setting.
> +  *
> +  * On platforms which don't have the shared LLC(HAS_SNOOP), like on Atom
> +  * platforms, coherency must be explicitly requested with some special
> +  * GTT caching bits(see enum i915_cache_level). When enabling coherency
> +  * it does come at a performance and power cost on such platforms. On
> +  * the flip side the kernel 

[PATCH] drm/radeon: Update pitch for page flip

2021-08-02 Thread Zhenneng Li

When primary bo is updated, crtc's pitch may
have not been updated, this will lead to show
disorder content when user changes display mode,
we update crtc's pitch in page flip to avoid
this bug.
This refers to amdgpu's pageflip.

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: "Pan, Xinhui" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Zhenneng Li 
---
 drivers/gpu/drm/radeon/evergreen.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c 
b/drivers/gpu/drm/radeon/evergreen.c
index 36a888e1b179..eeb590d2dec2 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -28,6 +28,7 @@
 
 #include 
 #include 
+#include 
 
 #include "atom.h"
 #include "avivod.h"
@@ -1414,10 +1415,15 @@ void evergreen_page_flip(struct radeon_device *rdev, 
int crtc_id, u64 crtc_base,
 bool async)
 {
struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id];
+   struct drm_framebuffer *fb = radeon_crtc->base.primary->fb;
 
-   /* update the scanout addresses */
+   /* flip at hsync for async, default is vsync */
WREG32(EVERGREEN_GRPH_FLIP_CONTROL + radeon_crtc->crtc_offset,
   async ? EVERGREEN_GRPH_SURFACE_UPDATE_H_RETRACE_EN : 0);
+   /* update pitch */
+   WREG32(EVERGREEN_GRPH_PITCH + radeon_crtc->crtc_offset,
+  fb->pitches[0] / fb->format->cpp[0]);
+   /* update the scanout addresses */
WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS_HIGH + 
radeon_crtc->crtc_offset,
   upper_32_bits(crtc_base));
WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS + 
radeon_crtc->crtc_offset,
-- 
2.25.1

Content-type: Text/plain

No virus found
Checked by Hillstone Network AntiVirus


Re: [PATCH v2 4/4] ARM: dts: add SKOV imx6q and imx6dl based boards

2021-08-02 Thread Oleksij Rempel
[...]
> > +   reg_vcc_mmc_io: regulator-vcc-mmc-io {
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_vcc_mmc_io>;
> > +   compatible = "regulator-gpio";
> > +   vin-supply = <_5v0>;
> > +   regulator-name = "mmc_io_supply";
> > +   regulator-type = "voltage";
> > +   regulator-min-microvolt = <180>;
> > +   regulator-max-microvolt = <330>;
> > +   gpios = < 13 GPIO_ACTIVE_HIGH>;
> 
> enable-active-high?

right, thx!

> > +   states = <180 0x1>, <330 0x0>;
> 
> Hmm, I do not see this 'states' in fixed-regulator.yaml.

It is in gpio-regulator.yaml

Regards,
Oleksij
-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |


Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability

2021-08-02 Thread Michel Dänzer
On 2021-08-02 11:06 a.m., Daniel Vetter wrote:
> On Mon, Aug 02, 2021 at 10:49:37AM +0200, Michel Dänzer wrote:
>> On 2021-08-02 9:59 a.m., Daniel Vetter wrote:
>>> On Fri, Jul 30, 2021 at 02:50:10PM +0200, Michel Dänzer wrote:
 On 2021-07-30 12:25 p.m., Daniel Vetter wrote:
> On Thu, Jul 29, 2021 at 01:16:55AM -0700, Vivek Kasireddy wrote:
>> By separating the OUT_FENCE signalling from pageflip completion allows
>> a Guest compositor to start a new repaint cycle with a new buffer
>> instead of waiting for the old buffer to be free. 
>>
>> This work is based on the idea/suggestion from Simon and Pekka.
>>
>> This capability can be a solution for this issue:
>> https://gitlab.freedesktop.org/wayland/weston/-/issues/514
>>
>> Corresponding Weston MR:
>> https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/668
>
> Uh I kinda wanted to discuss this a bit more before we jump into typing
> code, but well I guess not that much work yet.
>
> So maybe I'm not understanding the problem, but I think the fundamental
> underlying issue is that with KMS you can have at most 2 buffers
> in-flight, due to our queue depth limit of 1 pending flip.
>
> Unfortunately that means for virtual hw where it takes a few more
> steps/vblanks until the framebuffer actually shows up on screen and is
> scanned out, we suffer deeply. The usual fix for that is to drop the
> latency and increase throughput, and have more buffers in-flight. Which
> this patch tries to do.

 Per
 https://gitlab.freedesktop.org/wayland/weston/-/issues/514#note_986797 ,
 IMO the underlying issue is actually that the guest compositor repaint
 cycle is not aligned with the host compositor one. If they were aligned,
 the problem would not occur even without allowing multiple page flips in
 flight, and latency would be lower.
>>>
>>> Yeah my proposal here is under the premise that we do actually need to fix
>>> this with a deeper queue depth.
>>>
> Now I think where we go wrong here is that we're trying to hack this up by
> defining different semantics for the out-fence and for the drm-event. Imo
> that's wrong, they're both meant to show eactly the same thing:
> - when is the new frame actually visible to the user (as in, eyeballs in a
>   human head, preferrably, not the time when we've handed the buffer off
>   to the virtual hw)
> - when is the previous buffer no longer being used by the scanout hw
>
> We do cheat a bit right now in so far that we assume they're both the
> same, as in, panel-side latency is currently the compositor's problem to
> figure out.
>
> So for virtual hw I think the timestamp and even completion really need to
> happen only when the buffer has been pushed through the entire
> virtualization chain, i.e. ideally we get the timestamp from the kms
> driver from the host side. Currently that's not done, so this is most
> likely quite broken already (virtio relies on the no-vblank auto event
> sending, which definitely doesn't wait for anything, or I'm completely
> missing something).
>
> I think instead of hacking up some ill-defined 1.5 queue depth support,
> what we should do is support queue depth > 1 properly. So:
>
> - Change atomic to support queue depth > 1, this needs to be a per-driver
>   thing due to a bunch of issues in driver code. Essentially drivers must
>   never look at obj->state pointers, and only ever look up state through
>   the passed-in drm_atomic_state * update container.
>
> - Aside: virtio should loose all it's empty hooks, there's no point in
>   that.
>
> - We fix virtio to send out the completion event at the end of this entire
>   pipeline, i.e. virtio code needs to take care of sending out the
>   crtc_state->event correctly.
>
> - We probably also want some kind of (maybe per-crtc) recommended queue
>   depth property so compositors know how many buffers to keep in flight.
>   Not sure about that.

 I'd say there would definitely need to be some kind of signal for the
 display server that it should queue multiple flips, since this is
 normally not desirable for latency. In other words, this wouldn't really
 be useful on bare metal (in contrast to the ability to replace a pending
 flip with a newer one).
>>>
>>> Hm I was thinking that the compositor can tune this. If the round-trip
>>> latency (as measured by events) is too long to get full refresh rate, it
>>> can add more buffers to the queue. That's kinda why I think the returned
>>> event really must be accurate wrt actual display time (and old buffer
>>> release time), so that this computation in the compositor because a pretty
>>> simple
>>>
>>> num_buffers = (flip_time - submit_time) / frame_time
>>>
>>> With maybe some rounding up and 

Re: [PATCH] drm: Fix oops in damage self-tests by mocking damage property

2021-08-02 Thread Maarten Lankhorst
Op 30-07-2021 om 11:52 schreef Daniel Vetter:
> I've added a new check to make sure that drivers which insepct the
> damage property have it set up correctly, but somehow missed that this
> borke the damage selftest in the CI result noise.
>
> Fix it up by mocking enough of drm_device and drm_plane so we can call
> drm_plane_enable_fb_damage_clips() to make the new check happy.
>
> Since there's a lot of duplicated mock code already copy-pasted into
> each test I've also refactored this a bit to trim it down.
>
> Signed-off-by: Daniel Vetter 
> Fixes: c7fcbf251397 ("drm/plane: check that fb_damage is set up when used")
> Cc: José Roberto de Souza  (v1)
> Cc: Ville Syrjälä 
> Cc: Gwan-gyeong Mun 
> Cc: José Roberto de Souza 
> Cc: Hans de Goede 
> Cc: Daniel Vetter 
> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: Thomas Zimmermann 
> ---
>  .../drm/selftests/test-drm_damage_helper.c| 287 +-
>  1 file changed, 71 insertions(+), 216 deletions(-)
>
> diff --git a/drivers/gpu/drm/selftests/test-drm_damage_helper.c 
> b/drivers/gpu/drm/selftests/test-drm_damage_helper.c
> index 9d2bcdf8bc29..1b585c13e042 100644
> --- a/drivers/gpu/drm/selftests/test-drm_damage_helper.c
> +++ b/drivers/gpu/drm/selftests/test-drm_damage_helper.c
> @@ -6,9 +6,37 @@
>  #define pr_fmt(fmt) "drm_damage_helper: " fmt
>  
>  #include 
> +#include 
> +#include 
>  
>  #include "test-drm_modeset_common.h"
>  
> +struct drm_driver mock_driver;
> +struct drm_device mock_device;
> +struct drm_object_properties mock_obj_props;
> +struct drm_plane mock_plane;
> +struct drm_property mock_prop;
> +
> +static void mock_setup(struct drm_plane_state *state)
> +{
> + static bool setup_done = false;
> +
> + state->plane = _plane;
> +
> + if (setup_done)
> + return;
> +
> + /* just enough so that drm_plane_enable_fb_damage_clips() works */
> + mock_device.driver = _driver;
> + mock_device.mode_config.prop_fb_damage_clips = _prop;
> + mock_plane.dev = _device;
> + mock_plane.base.properties = _obj_props;
> + mock_prop.base.id = 1; /* 0 is an invalid id */
> + mock_prop.dev = _device;
> +
> + drm_plane_enable_fb_damage_clips(_plane);
> +}
> +
>  static void set_plane_src(struct drm_plane_state *state, int x1, int y1, int 
> x2,
> int y2)
>  {
> @@ -70,23 +98,29 @@ static bool check_damage_clip(struct drm_plane_state 
> *state, struct drm_rect *r,
>   return true;
>  }
>  
> +const struct drm_framebuffer fb = {
> + .width = 2048,
> + .height = 2048
> +};
> +
> +/* common mocked structs many tests need */
> +#define MOCK_VARIABLES() \
> + struct drm_plane_state old_state; \
> + struct drm_plane_state state = { \
> + .crtc = ZERO_SIZE_PTR, \
> + .fb = (struct drm_framebuffer *) , \
> + .visible = true, \
> + }; \
> + mock_setup(_state); \
> + mock_setup();
> +
>  int igt_damage_iter_no_damage(void *ignored)
>  {
>   struct drm_atomic_helper_damage_iter iter;
> - struct drm_plane_state old_state;
>   struct drm_rect clip;
>   uint32_t num_hits = 0;
>  
> - struct drm_framebuffer fb = {
> - .width = 2048,
> - .height = 2048
> - };
> -
> - struct drm_plane_state state = {
> - .crtc = ZERO_SIZE_PTR,
> - .fb = ,
> - .visible = true,
> - };
> + MOCK_VARIABLES();
>  
>   /* Plane src same as fb size. */
>   set_plane_src(_state, 0, 0, fb.width << 16, fb.height << 16);
> @@ -104,20 +138,10 @@ int igt_damage_iter_no_damage(void *ignored)
>  int igt_damage_iter_no_damage_fractional_src(void *ignored)
>  {
>   struct drm_atomic_helper_damage_iter iter;
> - struct drm_plane_state old_state;
>   struct drm_rect clip;
>   uint32_t num_hits = 0;
>  
> - struct drm_framebuffer fb = {
> - .width = 2048,
> - .height = 2048
> - };
> -
> - struct drm_plane_state state = {
> - .crtc = ZERO_SIZE_PTR,
> - .fb = ,
> - .visible = true,
> - };
> + MOCK_VARIABLES();
>  
>   /* Plane src has fractional part. */
>   set_plane_src(_state, 0x3fffe, 0x3fffe,
> @@ -137,20 +161,10 @@ int igt_damage_iter_no_damage_fractional_src(void 
> *ignored)
>  int igt_damage_iter_no_damage_src_moved(void *ignored)
>  {
>   struct drm_atomic_helper_damage_iter iter;
> - struct drm_plane_state old_state;
>   struct drm_rect clip;
>   uint32_t num_hits = 0;
>  
> - struct drm_framebuffer fb = {
> - .width = 2048,
> - .height = 2048
> - };
> -
> - struct drm_plane_state state = {
> - .crtc = ZERO_SIZE_PTR,
> - .fb = ,
> - .visible = true,
> - };
> + MOCK_VARIABLES();
>  
>   /* Plane src moved since old plane state. */
>   set_plane_src(_state, 0, 0, 1024 << 16, 768 << 16);
> @@ -169,20 +183,10 @@ int 

Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability

2021-08-02 Thread Daniel Vetter
On Fri, Jul 30, 2021 at 03:38:50PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > - We fix virtio to send out the completion event at the end of this entire
> >   pipeline, i.e. virtio code needs to take care of sending out the
> >   crtc_state->event correctly.
> 
> That sounds sensible to me.  Fence the virtio commands, make sure (on
> the host side) the command completes only when the work is actually done
> not only submitted.  Has recently been added to qemu for RESOURCE_FLUSH
> (aka frontbuffer rendering) and doing the same for SET_SCANOUT (aka
> pageflipping), then send vblank events to userspace on command
> completion certainly makes sense.

Hm how does this all work? At least drm/virtio uses
drm_atomic_helper_dirtyfb, so both DIRTYFB ioctl and atomic flips all end
up in the same driver path for everything. Or do you just combine the
resource_flush with the flip as needed and let the host side figure it all
out? From a quick read of virtgpu_plane.c that seems to be the case ...

Also to make this work we don't just need the fence, we need the timestamp
(in a clock domain the guest can correct for ofc) of the host side kms
driver flip completion. If you just have the fence then the jitter from
going through all the layers will most likely make it unusable.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH 3/4] drm/tiny: add simple-dbi driver

2021-08-02 Thread Icenowy Zheng
在 2021-08-02星期一的 14:35 +0800,Icenowy Zheng写道:
> Add a driver for generic MIPI DBI panels initialized with MIPI DCS
> commands.
> 
> Currently a ST7789V-based panel is added to it. This panel has its
> configuration pre-programmed into the controller, so no vendor-
> specific
> configuration is needed.
> 
> Signed-off-by: Icenowy Zheng 
> ---
>  drivers/gpu/drm/tiny/Kconfig  |  12 ++
>  drivers/gpu/drm/tiny/Makefile |   1 +
>  drivers/gpu/drm/tiny/simple-dbi.c | 244
> ++
>  3 files changed, 257 insertions(+)
>  create mode 100644 drivers/gpu/drm/tiny/simple-dbi.c
> 
> diff --git a/drivers/gpu/drm/tiny/Kconfig
> b/drivers/gpu/drm/tiny/Kconfig
> index d31be274a2bd..6cfc57b68a46 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -144,6 +144,18 @@ config TINYDRM_REPAPER
>  
>   If M is selected the module will be called repaper.
>  
> +config TINYDRM_SIMPLE_DBI
> +   tristate "DRM support for generic MIPI DBI panels"
> +   depends on DRM && SPI
> +   select DRM_KMS_HELPER
> +   select DRM_KMS_CMA_HELPER
> +   select DRM_MIPI_DBI
> +   help
> + DRM driver for generic DBI panels that are MIPI DCS
> compatible
> + and needs no vendor-specific setup commands.
> +
> + If M is selected the module will be called simple-dbi.
> +
>  config TINYDRM_ST7586
> tristate "DRM support for Sitronix ST7586 display panels"
> depends on DRM && SPI
> diff --git a/drivers/gpu/drm/tiny/Makefile
> b/drivers/gpu/drm/tiny/Makefile
> index e09942895c77..2553de651aa3 100644
> --- a/drivers/gpu/drm/tiny/Makefile
> +++ b/drivers/gpu/drm/tiny/Makefile
> @@ -13,3 +13,4 @@ obj-$(CONFIG_TINYDRM_MI0283QT)+=
> mi0283qt.o
>  obj-$(CONFIG_TINYDRM_REPAPER)  += repaper.o
>  obj-$(CONFIG_TINYDRM_ST7586)   += st7586.o
>  obj-$(CONFIG_TINYDRM_ST7735R)  += st7735r.o
> +obj-$(CONFIG_TINYDRM_SIMPLE_DBI)   += simple-dbi.o
> diff --git a/drivers/gpu/drm/tiny/simple-dbi.c
> b/drivers/gpu/drm/tiny/simple-dbi.c
> new file mode 100644
> index ..2b207e43d500
> --- /dev/null
> +++ b/drivers/gpu/drm/tiny/simple-dbi.c
> @@ -0,0 +1,244 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * DRM driver for display panels with configuration preset and needs
> only
> + * standard MIPI DCS commands to bring up.
> + *
> + * Copyright (C) 2021 Sipeed
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define MIPI_DCS_ADDRESS_MODE_BGR  BIT(3)
> +#define MIPI_DCS_ADDRESS_MODE_REVERSE  BIT(5)
> +#define MIPI_DCS_ADDRESS_MODE_RTL  BIT(6)
> +#define MIPI_DCS_ADDRESS_MODE_BTT  BIT(7)
> +
> +struct simple_dbi_cfg {
> +   const struct drm_display_mode mode;
> +   unsigned int left_offset;
> +   unsigned int top_offset;
> +   bool inverted;
> +   bool write_only;
> +   bool bgr;
> +   bool right_to_left;
> +   bool bottom_to_top;
> +};
> +
> +struct simple_dbi_priv {
> +   struct mipi_dbi_dev dbidev; /* Must be first for
> .release() */
> +   const struct simple_dbi_cfg *cfg;
> +};
> +
> +static void simple_dbi_pipe_enable(struct drm_simple_display_pipe
> *pipe,
> +   struct drm_crtc_state *crtc_state,
> +   struct drm_plane_state *plane_state)
> +{
> +   struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe-
> >crtc.dev);
> +   struct simple_dbi_priv *priv = container_of(dbidev,
> +   struct
> simple_dbi_priv,
> +   dbidev);
> +   struct mipi_dbi *dbi = >dbi;
> +   int ret, idx;
> +   u8 addr_mode = 0x00;
> +
> +   if (!drm_dev_enter(pipe->crtc.dev, ))
> +   return;
> +
> +   DRM_DEBUG_KMS("\n");
> +
> +   ret = mipi_dbi_poweron_reset(dbidev);
> +   if (ret)
> +   goto out_exit;
> +
> +   mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
> +   msleep(5);
> +
> +   /* Currently tinydrm supports 16bit only now */
> +   mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT,
> +    MIPI_DCS_PIXEL_FMT_16BIT);
> +
> +   if (priv->cfg->inverted)
> +   mipi_dbi_command(dbi, MIPI_DCS_ENTER_INVERT_MODE);
> +   else
> +   mipi_dbi_command(dbi, MIPI_DCS_EXIT_INVERT_MODE);
> +
> +   if (priv->cfg->bgr)
> +   addr_mode |= MIPI_DCS_ADDRESS_MODE_BGR;
> +
> +   if (priv->cfg->right_to_left)
> +   addr_mode |= MIPI_DCS_ADDRESS_MODE_RTL;
> +
> +   if (priv->cfg->bottom_to_top)
> +   addr_mode |= MIPI_DCS_ADDRESS_MODE_BTT;
> +
> +   switch (dbidev->rotation) {
> +   default:
> +   break;
> +   case 90:
> +

  1   2   >