Re: [PATCH 05/22] mm: export alloc_pages_vma

[Michal Hocko]

On Tue 25-06-19 12:52:18, Dan Williams wrote:
[...]
> > Documentation/process/stable-api-nonsense.rst
> 
> That document has failed to preclude symbol export fights in the past
> and there is a reasonable argument to try not to retract functionality
> that had been previously exported regardless of that document.

Can you point me to any specific example where this would be the case
for the core kernel symbols please?
-- 
Michal Hocko
SUSE Labs
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 18/22] mm: mark DEVICE_PUBLIC as broken

[Michal Hocko]

On Tue 25-06-19 20:15:28, John Hubbard wrote:
> On 6/19/19 12:27 PM, Jason Gunthorpe wrote:
> > On Thu, Jun 13, 2019 at 06:23:04PM -0700, John Hubbard wrote:
> >> On 6/13/19 5:43 PM, Ira Weiny wrote:
> >>> On Thu, Jun 13, 2019 at 07:58:29PM +, Jason Gunthorpe wrote:
>  On Thu, Jun 13, 2019 at 12:53:02PM -0700, Ralph Campbell wrote:
> >
> >> ...
> >>> So I think it is ok.  Frankly I was wondering if we should remove the 
> >>> public
> >>> type altogether but conceptually it seems ok.  But I don't see any users 
> >>> of it
> >>> so...  should we get rid of it in the code rather than turning the config 
> >>> off?
> >>>
> >>> Ira
> >>
> >> That seems reasonable. I recall that the hope was for those IBM Power 9
> >> systems to use _PUBLIC, as they have hardware-based coherent device (GPU)
> >> memory, and so the memory really is visible to the CPU. And the IBM team
> >> was thinking of taking advantage of it. But I haven't seen anything on
> >> that front for a while.
> > 
> > Does anyone know who those people are and can we encourage them to
> > send some patches? :)
> > 
> 
> I asked about this, and it seems that the idea was: DEVICE_PUBLIC was there
> in order to provide an alternative way to do things (such as migrate memory
> to and from a device), in case the combination of existing and near-future
> NUMA APIs was insufficient. This probably came as a follow-up to the early
> 2017-ish conversations about NUMA, in which the linux-mm recommendation was
> "try using HMM mechanisms, and if those are inadequate, then maybe we can
> look at enhancing NUMA so that it has better handling of advanced (GPU-like)
> devices".

Yes that was the original idea. It sounds so much better to use a common
framework rather than awkward special cased cpuless NUMA nodes with
a weird semantic. User of the neither of the two has shown up so I guess
that the envisioned HW just didn't materialized. Or has there been a
completely different approach chosen?

> In the end, however, _PUBLIC was never used, nor does anyone in the local
> (NVIDIA + IBM) kernel vicinity seem to have plans to use it.  So it really
> does seem safe to remove, although of course it's good to start with 
> BROKEN and see if anyone pops up and complains.

Well, I do not really see much of a difference. Preserving an unused
code which doesn't have any user in sight just adds a maintenance burden
whether the code depends on BROKEN or not. We can always revert patches
which remove the code once a real user shows up.
-- 
Michal Hocko
SUSE Labs
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH AUTOSEL 4.9 05/11] drm/mediatek: fix unbind functions

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit 8fd7a37b191f93737f6280a9b5de65f98acc12c9 ]

detatch panel in mtk_dsi_destroy_conn_enc(), since .bind will try to
attach it again.

Fixes: 2e54c14e310f ("drm/mediatek: Add DSI sub driver")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index eaa5a2240c0c..bdbf358697cd 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -720,6 +720,8 @@ static void mtk_dsi_destroy_conn_enc(struct mtk_dsi *dsi)
/* Skip connector cleanup if creation was delegated to the bridge */
if (dsi->conn.dev)
drm_connector_cleanup(>conn);
+   if (dsi->panel)
+   drm_panel_detach(dsi->panel);
 }
 
 static void mtk_dsi_ddp_start(struct mtk_ddp_comp *comp)
-- 
2.20.1

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

[PATCH AUTOSEL 4.14 08/21] drm/mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable()

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit 2458d9d6d94be982b917e93c61a89b4426f32e31 ]

mtk_dsi_stop() should be called after mtk_drm_crtc_atomic_disable(), which
needs ovl irq for drm_crtc_wait_one_vblank(), since after mtk_dsi_stop() is
called, ovl irq will be disabled. If drm_crtc_wait_one_vblank() is called
after last irq, it will timeout with this message: "vblank wait timed out
on crtc 0". This happens sometimes when turning off the screen.

In drm_atomic_helper.c#disable_outputs(),
the calling sequence when turning off the screen is:

1. mtk_dsi_encoder_disable()
 --> mtk_output_dsi_disable()
   --> mtk_dsi_stop();  /* sometimes make vblank timeout in
   atomic_disable */
   --> mtk_dsi_poweroff();
2. mtk_drm_crtc_atomic_disable()
 --> drm_crtc_wait_one_vblank();
 ...
   --> mtk_dsi_ddp_stop()
 --> mtk_dsi_poweroff();

mtk_dsi_poweroff() has reference count design, change to make
mtk_dsi_stop() called in mtk_dsi_poweroff() when refcount is 0.

Fixes: 0707632b5bac ("drm/mediatek: update DSI sub driver flow for sending 
commands to panel")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 413313f19c36..c1b8caad65e6 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -631,6 +631,15 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
if (--dsi->refcount != 0)
return;
 
+   /*
+* mtk_dsi_stop() and mtk_dsi_start() is asymmetric, since
+* mtk_dsi_stop() should be called after mtk_drm_crtc_atomic_disable(),
+* which needs irq for vblank, and mtk_dsi_stop() will disable irq.
+* mtk_dsi_start() needs to be called in mtk_output_dsi_enable(),
+* after dsi is fully set.
+*/
+   mtk_dsi_stop(dsi);
+
if (!mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500)) {
if (dsi->panel) {
if (drm_panel_unprepare(dsi->panel)) {
@@ -697,7 +706,6 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi)
}
}
 
-   mtk_dsi_stop(dsi);
mtk_dsi_poweroff(dsi);
 
dsi->enabled = false;
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 23/34] drm: panel-orientation-quirks: Add quirk for GPD MicroPC

[Sasha Levin]

From: Hans de Goede 

[ Upstream commit 652b8b086538c8a10de5aa5cbdaef79333b46358 ]

GPD has done it again, make a nice device (good), use way too generic
DMI strings (bad) and use a portrait screen rotated 90 degrees (ugly).

Because of the too generic DMI strings this entry is also doing bios-date
matching, so the gpd_micropc data struct may very well need to be updated
with some extra bios-dates in the future.

Acked-by: Maxime Ripard 
Signed-off-by: Hans de Goede 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20190524125759.14131-2-hdego...@redhat.com
(cherry picked from commit f2f2bb60d998abde10de7e483ef9e17639892450)
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/drm_panel_orientation_quirks.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c 
b/drivers/gpu/drm/drm_panel_orientation_quirks.c
index 088363675940..b44bed554211 100644
--- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
+++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
@@ -42,6 +42,14 @@ static const struct drm_dmi_panel_orientation_data 
asus_t100ha = {
.orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP,
 };
 
+static const struct drm_dmi_panel_orientation_data gpd_micropc = {
+   .width = 720,
+   .height = 1280,
+   .bios_dates = (const char * const []){ "04/26/2019",
+   NULL },
+   .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
+};
+
 static const struct drm_dmi_panel_orientation_data gpd_pocket = {
.width = 1200,
.height = 1920,
@@ -93,6 +101,14 @@ static const struct dmi_system_id orientation_data[] = {
  DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "T100HAN"),
},
.driver_data = (void *)_t100ha,
+   }, {/* GPD MicroPC (generic strings, also match on bios date) */
+   .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Default string"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Default string"),
+ DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Default string"),
+ DMI_EXACT_MATCH(DMI_BOARD_NAME, "Default string"),
+   },
+   .driver_data = (void *)_micropc,
}, {/*
 * GPD Pocket, note that the the DMI data is less generic then
 * it seems, devices with a board-vendor of "AMI Corporation"
-- 
2.20.1

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

[PATCH AUTOSEL 4.14 07/21] drm/mediatek: call drm_atomic_helper_shutdown() when unbinding driver

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit cf49b24ffa62766f8f04cd1c4cf17b75d29b240a ]

shutdown all CRTC when unbinding drm driver.

Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index a2ca90fc403c..cada1c75c41c 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -270,6 +270,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
 static void mtk_drm_kms_deinit(struct drm_device *drm)
 {
drm_kms_helper_poll_fini(drm);
+   drm_atomic_helper_shutdown(drm);
 
component_unbind_all(drm->dev, drm);
drm_mode_config_cleanup(drm);
-- 
2.20.1

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

[PATCH AUTOSEL 4.14 06/21] drm/mediatek: fix unbind functions

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit 8fd7a37b191f93737f6280a9b5de65f98acc12c9 ]

detatch panel in mtk_dsi_destroy_conn_enc(), since .bind will try to
attach it again.

Fixes: 2e54c14e310f ("drm/mediatek: Add DSI sub driver")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 7e5e24c2152a..413313f19c36 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -851,6 +851,8 @@ static void mtk_dsi_destroy_conn_enc(struct mtk_dsi *dsi)
/* Skip connector cleanup if creation was delegated to the bridge */
if (dsi->conn.dev)
drm_connector_cleanup(>conn);
+   if (dsi->panel)
+   drm_panel_detach(dsi->panel);
 }
 
 static void mtk_dsi_ddp_start(struct mtk_ddp_comp *comp)
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 11/34] drm/mediatek: clear num_pipes when unbind driver

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit a4cd1d2b016d5d043ab2c4b9c4ec50a5805f5396 ]

num_pipes is used for mutex created in mtk_drm_crtc_create(). If we
don't clear num_pipes count, when rebinding driver, the count will
be accumulated. From mtk_disp_mutex_get(), there can only be at most
10 mutex id. Clear this number so it starts from 0 in every rebind.

Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 3df8a9dbccfe..fd83046d8376 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -393,6 +393,7 @@ static void mtk_drm_unbind(struct device *dev)
drm_dev_unregister(private->drm);
mtk_drm_kms_deinit(private->drm);
drm_dev_put(private->drm);
+   private->num_pipes = 0;
private->drm = NULL;
 }
 
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 12/34] drm/mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable()

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit 2458d9d6d94be982b917e93c61a89b4426f32e31 ]

mtk_dsi_stop() should be called after mtk_drm_crtc_atomic_disable(), which
needs ovl irq for drm_crtc_wait_one_vblank(), since after mtk_dsi_stop() is
called, ovl irq will be disabled. If drm_crtc_wait_one_vblank() is called
after last irq, it will timeout with this message: "vblank wait timed out
on crtc 0". This happens sometimes when turning off the screen.

In drm_atomic_helper.c#disable_outputs(),
the calling sequence when turning off the screen is:

1. mtk_dsi_encoder_disable()
 --> mtk_output_dsi_disable()
   --> mtk_dsi_stop();  /* sometimes make vblank timeout in
   atomic_disable */
   --> mtk_dsi_poweroff();
2. mtk_drm_crtc_atomic_disable()
 --> drm_crtc_wait_one_vblank();
 ...
   --> mtk_dsi_ddp_stop()
 --> mtk_dsi_poweroff();

mtk_dsi_poweroff() has reference count design, change to make
mtk_dsi_stop() called in mtk_dsi_poweroff() when refcount is 0.

Fixes: 0707632b5bac ("drm/mediatek: update DSI sub driver flow for sending 
commands to panel")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 84bb66866631..0dd317ac5fe5 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -630,6 +630,15 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
if (--dsi->refcount != 0)
return;
 
+   /*
+* mtk_dsi_stop() and mtk_dsi_start() is asymmetric, since
+* mtk_dsi_stop() should be called after mtk_drm_crtc_atomic_disable(),
+* which needs irq for vblank, and mtk_dsi_stop() will disable irq.
+* mtk_dsi_start() needs to be called in mtk_output_dsi_enable(),
+* after dsi is fully set.
+*/
+   mtk_dsi_stop(dsi);
+
if (!mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500)) {
if (dsi->panel) {
if (drm_panel_unprepare(dsi->panel)) {
@@ -696,7 +705,6 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi)
}
}
 
-   mtk_dsi_stop(dsi);
mtk_dsi_poweroff(dsi);
 
dsi->enabled = false;
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 22/34] drm: panel-orientation-quirks: Add quirk for GPD pocket2

[Sasha Levin]

From: Hans de Goede 

[ Upstream commit 15abc7110a77555d3bf72aaef46d1557db0a4ac5 ]

GPD has done it again, make a nice device (good), use way too generic
DMI strings (bad) and use a portrait screen rotated 90 degrees (ugly).

Because of the too generic DMI strings this entry is also doing bios-date
matching, so the gpd_pocket2 data struct may very well need to be updated
with some extra bios-dates in the future.

Changes in v2:
-Add one more known BIOS date to the list of BIOS dates

Cc: Jurgen Kramer 
Reported-by: Jurgen Kramer 
Acked-by: Maxime Ripard 
Signed-off-by: Hans de Goede 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20190524125759.14131-1-hdego...@redhat.com
(cherry picked from commit 6dab9102dd7b144e5723915438e0d6c473018cd0)
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/drm_panel_orientation_quirks.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c 
b/drivers/gpu/drm/drm_panel_orientation_quirks.c
index ee4a5e1221f1..088363675940 100644
--- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
+++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
@@ -50,6 +50,14 @@ static const struct drm_dmi_panel_orientation_data 
gpd_pocket = {
.orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
 };
 
+static const struct drm_dmi_panel_orientation_data gpd_pocket2 = {
+   .width = 1200,
+   .height = 1920,
+   .bios_dates = (const char * const []){ "06/28/2018", "08/28/2018",
+   "12/07/2018", NULL },
+   .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
+};
+
 static const struct drm_dmi_panel_orientation_data gpd_win = {
.width = 720,
.height = 1280,
@@ -98,6 +106,14 @@ static const struct dmi_system_id orientation_data[] = {
  DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Default string"),
},
.driver_data = (void *)_pocket,
+   }, {/* GPD Pocket 2 (generic strings, also match on bios date) */
+   .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Default string"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Default string"),
+ DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Default string"),
+ DMI_EXACT_MATCH(DMI_BOARD_NAME, "Default string"),
+   },
+   .driver_data = (void *)_pocket2,
}, {/* GPD Win (same note on DMI match as GPD Pocket) */
.matches = {
  DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 09/34] drm/mediatek: unbind components in mtk_drm_unbind()

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit f0fd848342802bc0f74620d387eead53e8905804 ]

Unbinding components (i.e. mtk_dsi and mtk_disp_ovl/rdma/color) will
trigger master(mtk_drm)'s .unbind(), and currently mtk_drm's unbind
won't actually unbind components. During the next bind,
mtk_drm_kms_init() is called, and the components are added back.

.unbind() should call mtk_drm_kms_deinit() to unbind components.

And since component_master_del() in .remove() will trigger .unbind(),
which will also unregister device, it's fine to remove original functions
called here.

Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 47ec604289b7..bbe57ad9acf1 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -390,6 +390,7 @@ static void mtk_drm_unbind(struct device *dev)
struct mtk_drm_private *private = dev_get_drvdata(dev);
 
drm_dev_unregister(private->drm);
+   mtk_drm_kms_deinit(private->drm);
drm_dev_put(private->drm);
private->drm = NULL;
 }
@@ -559,13 +560,8 @@ static int mtk_drm_probe(struct platform_device *pdev)
 static int mtk_drm_remove(struct platform_device *pdev)
 {
struct mtk_drm_private *private = platform_get_drvdata(pdev);
-   struct drm_device *drm = private->drm;
int i;
 
-   drm_dev_unregister(drm);
-   mtk_drm_kms_deinit(drm);
-   drm_dev_put(drm);
-
component_master_del(>dev, _drm_ops);
pm_runtime_disable(>dev);
of_node_put(private->mutex_node);
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 08/34] drm/mediatek: fix unbind functions

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit 8fd7a37b191f93737f6280a9b5de65f98acc12c9 ]

detatch panel in mtk_dsi_destroy_conn_enc(), since .bind will try to
attach it again.

Fixes: 2e54c14e310f ("drm/mediatek: Add DSI sub driver")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 66df1b177959..84bb66866631 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -841,6 +841,8 @@ static void mtk_dsi_destroy_conn_enc(struct mtk_dsi *dsi)
/* Skip connector cleanup if creation was delegated to the bridge */
if (dsi->conn.dev)
drm_connector_cleanup(>conn);
+   if (dsi->panel)
+   drm_panel_detach(dsi->panel);
 }
 
 static void mtk_dsi_ddp_start(struct mtk_ddp_comp *comp)
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 10/34] drm/mediatek: call drm_atomic_helper_shutdown() when unbinding driver

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit cf49b24ffa62766f8f04cd1c4cf17b75d29b240a ]

shutdown all CRTC when unbinding drm driver.

Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index bbe57ad9acf1..3df8a9dbccfe 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -310,6 +310,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
 static void mtk_drm_kms_deinit(struct drm_device *drm)
 {
drm_kms_helper_poll_fini(drm);
+   drm_atomic_helper_shutdown(drm);
 
component_unbind_all(drm->dev, drm);
drm_mode_config_cleanup(drm);
-- 
2.20.1

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

[PATCH AUTOSEL 5.1 36/51] drm: panel-orientation-quirks: Add quirk for GPD pocket2

[Sasha Levin]

From: Hans de Goede 

[ Upstream commit 15abc7110a77555d3bf72aaef46d1557db0a4ac5 ]

GPD has done it again, make a nice device (good), use way too generic
DMI strings (bad) and use a portrait screen rotated 90 degrees (ugly).

Because of the too generic DMI strings this entry is also doing bios-date
matching, so the gpd_pocket2 data struct may very well need to be updated
with some extra bios-dates in the future.

Changes in v2:
-Add one more known BIOS date to the list of BIOS dates

Cc: Jurgen Kramer 
Reported-by: Jurgen Kramer 
Acked-by: Maxime Ripard 
Signed-off-by: Hans de Goede 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20190524125759.14131-1-hdego...@redhat.com
(cherry picked from commit 6dab9102dd7b144e5723915438e0d6c473018cd0)
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/drm_panel_orientation_quirks.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c 
b/drivers/gpu/drm/drm_panel_orientation_quirks.c
index 52e445bb1aa5..019f148d5a78 100644
--- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
+++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
@@ -50,6 +50,14 @@ static const struct drm_dmi_panel_orientation_data 
gpd_pocket = {
.orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
 };
 
+static const struct drm_dmi_panel_orientation_data gpd_pocket2 = {
+   .width = 1200,
+   .height = 1920,
+   .bios_dates = (const char * const []){ "06/28/2018", "08/28/2018",
+   "12/07/2018", NULL },
+   .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
+};
+
 static const struct drm_dmi_panel_orientation_data gpd_win = {
.width = 720,
.height = 1280,
@@ -106,6 +114,14 @@ static const struct dmi_system_id orientation_data[] = {
  DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Default string"),
},
.driver_data = (void *)_pocket,
+   }, {/* GPD Pocket 2 (generic strings, also match on bios date) */
+   .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Default string"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Default string"),
+ DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Default string"),
+ DMI_EXACT_MATCH(DMI_BOARD_NAME, "Default string"),
+   },
+   .driver_data = (void *)_pocket2,
}, {/* GPD Win (same note on DMI match as GPD Pocket) */
.matches = {
  DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
-- 
2.20.1

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

[PATCH AUTOSEL 5.1 37/51] drm: panel-orientation-quirks: Add quirk for GPD MicroPC

[Sasha Levin]

From: Hans de Goede 

[ Upstream commit 652b8b086538c8a10de5aa5cbdaef79333b46358 ]

GPD has done it again, make a nice device (good), use way too generic
DMI strings (bad) and use a portrait screen rotated 90 degrees (ugly).

Because of the too generic DMI strings this entry is also doing bios-date
matching, so the gpd_micropc data struct may very well need to be updated
with some extra bios-dates in the future.

Acked-by: Maxime Ripard 
Signed-off-by: Hans de Goede 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20190524125759.14131-2-hdego...@redhat.com
(cherry picked from commit f2f2bb60d998abde10de7e483ef9e17639892450)
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/drm_panel_orientation_quirks.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c 
b/drivers/gpu/drm/drm_panel_orientation_quirks.c
index 019f148d5a78..dd982563304d 100644
--- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
+++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
@@ -42,6 +42,14 @@ static const struct drm_dmi_panel_orientation_data 
asus_t100ha = {
.orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP,
 };
 
+static const struct drm_dmi_panel_orientation_data gpd_micropc = {
+   .width = 720,
+   .height = 1280,
+   .bios_dates = (const char * const []){ "04/26/2019",
+   NULL },
+   .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
+};
+
 static const struct drm_dmi_panel_orientation_data gpd_pocket = {
.width = 1200,
.height = 1920,
@@ -101,6 +109,14 @@ static const struct dmi_system_id orientation_data[] = {
  DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "T100HAN"),
},
.driver_data = (void *)_t100ha,
+   }, {/* GPD MicroPC (generic strings, also match on bios date) */
+   .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Default string"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Default string"),
+ DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Default string"),
+ DMI_EXACT_MATCH(DMI_BOARD_NAME, "Default string"),
+   },
+   .driver_data = (void *)_micropc,
}, {/*
 * GPD Pocket, note that the the DMI data is less generic then
 * it seems, devices with a board-vendor of "AMI Corporation"
-- 
2.20.1

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

[PATCH AUTOSEL 5.1 19/51] drm/mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable()

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit 2458d9d6d94be982b917e93c61a89b4426f32e31 ]

mtk_dsi_stop() should be called after mtk_drm_crtc_atomic_disable(), which
needs ovl irq for drm_crtc_wait_one_vblank(), since after mtk_dsi_stop() is
called, ovl irq will be disabled. If drm_crtc_wait_one_vblank() is called
after last irq, it will timeout with this message: "vblank wait timed out
on crtc 0". This happens sometimes when turning off the screen.

In drm_atomic_helper.c#disable_outputs(),
the calling sequence when turning off the screen is:

1. mtk_dsi_encoder_disable()
 --> mtk_output_dsi_disable()
   --> mtk_dsi_stop();  /* sometimes make vblank timeout in
   atomic_disable */
   --> mtk_dsi_poweroff();
2. mtk_drm_crtc_atomic_disable()
 --> drm_crtc_wait_one_vblank();
 ...
   --> mtk_dsi_ddp_stop()
 --> mtk_dsi_poweroff();

mtk_dsi_poweroff() has reference count design, change to make
mtk_dsi_stop() called in mtk_dsi_poweroff() when refcount is 0.

Fixes: 0707632b5bac ("drm/mediatek: update DSI sub driver flow for sending 
commands to panel")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 1ae3be99e0ff..179f2b080342 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -630,6 +630,15 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
if (--dsi->refcount != 0)
return;
 
+   /*
+* mtk_dsi_stop() and mtk_dsi_start() is asymmetric, since
+* mtk_dsi_stop() should be called after mtk_drm_crtc_atomic_disable(),
+* which needs irq for vblank, and mtk_dsi_stop() will disable irq.
+* mtk_dsi_start() needs to be called in mtk_output_dsi_enable(),
+* after dsi is fully set.
+*/
+   mtk_dsi_stop(dsi);
+
if (!mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500)) {
if (dsi->panel) {
if (drm_panel_unprepare(dsi->panel)) {
@@ -696,7 +705,6 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi)
}
}
 
-   mtk_dsi_stop(dsi);
mtk_dsi_poweroff(dsi);
 
dsi->enabled = false;
-- 
2.20.1

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

[PATCH AUTOSEL 5.1 15/51] drm/mediatek: fix unbind functions

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit 8fd7a37b191f93737f6280a9b5de65f98acc12c9 ]

detatch panel in mtk_dsi_destroy_conn_enc(), since .bind will try to
attach it again.

Fixes: 2e54c14e310f ("drm/mediatek: Add DSI sub driver")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index b00eb2d2e086..1ae3be99e0ff 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -844,6 +844,8 @@ static void mtk_dsi_destroy_conn_enc(struct mtk_dsi *dsi)
/* Skip connector cleanup if creation was delegated to the bridge */
if (dsi->conn.dev)
drm_connector_cleanup(>conn);
+   if (dsi->panel)
+   drm_panel_detach(dsi->panel);
 }
 
 static void mtk_dsi_ddp_start(struct mtk_ddp_comp *comp)
-- 
2.20.1

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

[PATCH AUTOSEL 5.1 17/51] drm/mediatek: call drm_atomic_helper_shutdown() when unbinding driver

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit cf49b24ffa62766f8f04cd1c4cf17b75d29b240a ]

shutdown all CRTC when unbinding drm driver.

Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index e7362bdafa82..8718d123ccaa 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -311,6 +311,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
 static void mtk_drm_kms_deinit(struct drm_device *drm)
 {
drm_kms_helper_poll_fini(drm);
+   drm_atomic_helper_shutdown(drm);
 
component_unbind_all(drm->dev, drm);
drm_mode_config_cleanup(drm);
-- 
2.20.1

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

[PATCH AUTOSEL 5.1 18/51] drm/mediatek: clear num_pipes when unbind driver

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit a4cd1d2b016d5d043ab2c4b9c4ec50a5805f5396 ]

num_pipes is used for mutex created in mtk_drm_crtc_create(). If we
don't clear num_pipes count, when rebinding driver, the count will
be accumulated. From mtk_disp_mutex_get(), there can only be at most
10 mutex id. Clear this number so it starts from 0 in every rebind.

Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 8718d123ccaa..bbfe3a464aea 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -400,6 +400,7 @@ static void mtk_drm_unbind(struct device *dev)
drm_dev_unregister(private->drm);
mtk_drm_kms_deinit(private->drm);
drm_dev_put(private->drm);
+   private->num_pipes = 0;
private->drm = NULL;
 }
 
-- 
2.20.1

[PATCH AUTOSEL 5.1 16/51] drm/mediatek: unbind components in mtk_drm_unbind()

[Sasha Levin]

From: Hsin-Yi Wang 

[ Upstream commit f0fd848342802bc0f74620d387eead53e8905804 ]

Unbinding components (i.e. mtk_dsi and mtk_disp_ovl/rdma/color) will
trigger master(mtk_drm)'s .unbind(), and currently mtk_drm's unbind
won't actually unbind components. During the next bind,
mtk_drm_kms_init() is called, and the components are added back.

.unbind() should call mtk_drm_kms_deinit() to unbind components.

And since component_master_del() in .remove() will trigger .unbind(),
which will also unregister device, it's fine to remove original functions
called here.

Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Hsin-Yi Wang 
Signed-off-by: CK Hu 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 57ce4708ef1b..e7362bdafa82 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -397,6 +397,7 @@ static void mtk_drm_unbind(struct device *dev)
struct mtk_drm_private *private = dev_get_drvdata(dev);
 
drm_dev_unregister(private->drm);
+   mtk_drm_kms_deinit(private->drm);
drm_dev_put(private->drm);
private->drm = NULL;
 }
@@ -568,13 +569,8 @@ static int mtk_drm_probe(struct platform_device *pdev)
 static int mtk_drm_remove(struct platform_device *pdev)
 {
struct mtk_drm_private *private = platform_get_drvdata(pdev);
-   struct drm_device *drm = private->drm;
int i;
 
-   drm_dev_unregister(drm);
-   mtk_drm_kms_deinit(drm);
-   drm_dev_put(drm);
-
component_master_del(>dev, _drm_ops);
pm_runtime_disable(>dev);
of_node_put(private->mutex_node);
-- 
2.20.1

Re: [PATCH v5 01/18] kunit: test: add KUnit test runner core

[Stephen Boyd]

Quoting Brendan Higgins (2019-06-25 13:28:25)
> On Wed, Jun 19, 2019 at 5:15 PM Stephen Boyd  wrote:
> >
> > Quoting Brendan Higgins (2019-06-17 01:25:56)
> > > diff --git a/kunit/test.c b/kunit/test.c
> > > new file mode 100644
> > > index 0..d05d254f1521f
> > > --- /dev/null
> > > +++ b/kunit/test.c
> > > @@ -0,0 +1,210 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Base unit test (KUnit) API.
> > > + *
> > > + * Copyright (C) 2019, Google LLC.
> > > + * Author: Brendan Higgins 
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +
> > > +static bool kunit_get_success(struct kunit *test)
> > > +{
> > > +   unsigned long flags;
> > > +   bool success;
> > > +
> > > +   spin_lock_irqsave(>lock, flags);
> > > +   success = test->success;
> > > +   spin_unlock_irqrestore(>lock, flags);
> >
> > I still don't understand the locking scheme in this code. Is the
> > intention to make getter and setter APIs that are "safe" by adding in a
> > spinlock that is held around getting and setting various members in the
> > kunit structure?
> 
> Yes, your understanding is correct. It is possible for a user to write
> a test such that certain elements may be updated in different threads;
> this would most likely happen in the case where someone wants to make
> an assertion or an expectation in a thread created by a piece of code
> under test. Although this should generally be avoided, it is possible,
> and there are occasionally good reasons to do so, so it is
> functionality that we should support.
> 
> Do you think I should add a comment to this effect?

No, I think the locking should be removed.

> 
> > In what situation is there more than one thread reading or writing the
> > kunit struct? Isn't it only a single process that is going to be
> 
> As I said above, it is possible that the code under test may spawn a
> new thread that may make an expectation or an assertion. It is not a
> super common use case, but it is possible.

Sure, sounds super possible and OK.

> 
> > operating on this structure? And why do we need to disable irqs? Are we
> > expecting to be modifying the unit tests from irq contexts?
> 
> There are instances where someone may want to test a driver which has
> an interrupt handler in it. I actually have (not the greatest) example
> here. Now in these cases, I expect someone to use a mock irqchip or
> some other fake mechanism to trigger the interrupt handler and not
> actual hardware; technically speaking in this case, it is not going to
> be accessed from a "real" irq context; however, the code under test
> should think that it is in an irq context; given that, I figured it is
> best to just treat it as a real irq context. Does that make sense?

Can you please describe the scenario in which grabbing the lock here,
updating a single variable, and then releasing the lock right after
does anything useful vs. not having the lock? I'm looking for a two CPU
scenario like below, but where it is a problem. There could be three
CPUs, or even one CPU and three threads if you want to describe the
extra thread scenario.

Here's my scenario where it isn't needed:

CPU0  CPU1
  
kunit_run_test()
  test_case_func()

  [mock hardirq]
kunit_set_success()
  [hardirq ends]
...
complete(_done)
  wait_for_completion(_done)
  kunit_get_success()

We don't need to care about having locking here because success or
failure only happens in one place and it's synchronized with the
completion.

> 
> > > +
> > > +   return success;
> > > +}
> > > +
> > > +static void kunit_set_success(struct kunit *test, bool success)
> > > +{
> > > +   unsigned long flags;
> > > +
> > > +   spin_lock_irqsave(>lock, flags);
> > > +   test->success = success;
> > > +   spin_unlock_irqrestore(>lock, flags);
> > > +}

Re: [PATCH v5 01/18] kunit: test: add KUnit test runner core

[Luis Chamberlain]

On Tue, Jun 25, 2019 at 05:07:32PM -0700, Brendan Higgins wrote:
> On Tue, Jun 25, 2019 at 3:33 PM Luis Chamberlain  wrote:
> >
> > On Mon, Jun 17, 2019 at 01:25:56AM -0700, Brendan Higgins wrote:
> > > +/**
> > > + * module_test() - used to register a  kunit_module with KUnit.
> > > + * @module: a statically allocated  kunit_module.
> > > + *
> > > + * Registers @module with the test framework. See  kunit_module 
> > > for more
> > > + * information.
> > > + */
> > > +#define module_test(module) \
> > > + static int module_kunit_init##module(void) \
> > > + { \
> > > + return kunit_run_tests(); \
> > > + } \
> > > + late_initcall(module_kunit_init##module)
> >
> > Becuase late_initcall() is used, if these modules are built-in, this
> > would preclude the ability to test things prior to this part of the
> > kernel under UML or whatever architecture runs the tests. So, this
> > limits the scope of testing. Small detail but the scope whould be
> > documented.
> 
> You aren't the first person to complain about this (and I am not sure
> it is the first time you have complained about it). Anyway, I have
> some follow on patches that will improve the late_initcall thing, and
> people seemed okay with discussing the follow on patches as part of a
> subsequent patchset after this gets merged.
> 
> I will nevertheless document the restriction until then.

To be clear, I am not complaining about it. I just find it simply
critical to document its limitations, so folks don't try to invest
time and energy on kunit right away for an early init test, if it
cannot support it.

If support for that requires some work, it may be worth mentioning
as well.

> > > +static void kunit_print_tap_version(void)
> > > +{
> > > + if (!kunit_has_printed_tap_version) {
> > > + kunit_printk_emit(LOGLEVEL_INFO, "TAP version 14\n");
> >
> > What is this TAP thing? Why should we care what version it is on?
> > Why are we printing this?
> 
> It's part of the TAP specification[1]. Greg and Frank asked me to make
> the intermediate format conform to TAP. Seems like something else I
> should probable document...

Yes thanks!

> > > + kunit_has_printed_tap_version = true;
> > > + }
> > > +}
> > > +
> > > +static size_t kunit_test_cases_len(struct kunit_case *test_cases)
> > > +{
> > > + struct kunit_case *test_case;
> > > + size_t len = 0;
> > > +
> > > + for (test_case = test_cases; test_case->run_case; test_case++)
> >
> > If we make the last test case NULL, we'd just check for test_case here,
> > and save ourselves an extra few bytes per test module. Any reason why
> > the last test case cannot be NULL?
> 
> Is there anyway to make that work with a statically defined array?

No you're right.

> Basically, I want to be able to do something like:
> 
> static struct kunit_case example_test_cases[] = {
> KUNIT_CASE(example_simple_test),
> KUNIT_CASE(example_mock_test),
> {}
> };
> 
> FYI,
> #define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name }

> 
> In order to do what you are proposing, I think I need an array of
> pointers to test cases, which is not ideal.

Yeah, you're right. The only other alternative is to have a:

struct kunit_module {
   const char name[256];
   int (*init)(struct kunit *test);
   void (*exit)(struct kunit *test);
   struct kunit_case *test_cases;
+   unsigned int num_cases;
};

And then something like:

#define KUNIT_MODULE(name, init, exit, cases) { \
.name = name, \
.init = init, \
.exit = exit, \
.test_cases = cases,
num_cases = ARRAY_SIZE(cases), \
}

Let's evaluate which is better: one extra test case per all test cases, or
an extra unsigned int for each kunit module.

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

Re: [v2 2/2] drm/panel: support for BOE tv101wum-nl6 wuxga dsi video mode panel

[Hsin-Yi Wang]

On Mon, Jun 24, 2019 at 6:03 AM Jitao Shi  wrote:

There are some error when unbinding this driver,
`` echo 14014000.dsi.0 >
/sys/bus/mipi-dsi/drivers/panel-boe-tv101wum-nl6/unbind ``

[   40.404393] WARNING: CPU: 1 PID: 2612 at
/mnt/host/source/src/third_party/kernel/v4.19/drivers/regulator/core.c:2054
_regulator_put+0xe4/0xec
[   40.417098] Modules linked in: rfcomm uinput thermal_generic_adc
hci_uart btqca bluetooth ecdh_generic mtk_scp mtk_rpmsg mtk_scp_ipi
bridge rpmsg_core mt6577_auxadc stp llc nf_nat_tftp nv
[   40.471107] CPU: 1 PID: 2612 Comm: bash Tainted: G S
4.19.53 #99
[   40.478410] Hardware name: MediaTek krane rev3 sku16 board (DT)
[   40.484327] pstate: 8005 (Nzcv daif -PAN -UAO)
[   40.489118] pc : _regulator_put+0xe4/0xec
[   40.493127] lr : regulator_put+0x2c/0x40
[   40.497044] sp : ff800cef3b00
[   40.500354] x29: ff800cef3b10 x28: fff0e7a22a00
[   40.505667] x27:  x26: fff13876fc00
[   40.510979] x25: ff800cef3b68 x24: ff91f0b92000
[   40.516291] x23: ff91f03f2917 x22: ff91f0453a1c
[   40.521604] x21: fff13876fe00 x20: ff91f0a80988
[   40.526916] x19: fff13876ff00 x18: 
[   40.532229] x17:  x16: 
[   40.537541] x15:  x14: 
[   40.542854] x13: 0010 x12: 
[   40.548166] x11:  x10: 
[   40.553478] x9 : fff0e7a22a00 x8 : 0001
[   40.558789] x7 : 0004be42 x6 : 42be0400
[   40.564102] x5 : ff91f043a5ef x4 : ff91f043a5ef
[   40.569414] x3 :  x2 : 
[   40.574726] x1 : 00fe x0 : fff13876ff00
[   40.580040] Call trace:
[   40.582488]  _regulator_put+0xe4/0xec
[   40.586148]  regulator_put+0x2c/0x40
[   40.589724]  devm_regulator_release+0x1c/0x28
[   40.594084]  release_nodes+0x1f0/0x244
[   40.597832]  devres_release_all+0x3c/0x54
[   40.601840]  device_release_driver_internal+0x148/0x1ec
[   40.607061]  device_release_driver+0x24/0x30
[   40.611328]  unbind_store+0x90/0xdc
[   40.614814]  drv_attr_store+0x3c/0x54
[   40.618478]  sysfs_kf_write+0x50/0x68
[   40.622137]  kernfs_fop_write+0x12c/0x1c8
[   40.626146]  __vfs_write+0x54/0x15c
[   40.629631]  vfs_write+0xcc/0x188
[   40.632943]  ksys_write+0x78/0xd8
[   40.636255]  __arm64_sys_write+0x20/0x2c
[   40.640181]  el0_svc_common+0xa4/0x154
[   40.643928]  el0_svc_compat_handler+0x2c/0x38
[   40.648283]  el0_svc_compat+0x8/0x18
[   40.651855] ---[ end trace 65d8c8e7436ab6e9 ]---
[   40.656765] panel-boe-tv101wum-nl6 14014000.dsi.0: Dropping the
link to regulator.8
[   40.664649] WARNING: CPU: 6 PID: 2612 at
/mnt/host/source/src/third_party/kernel/v4.19/drivers/regulator/core.c:2054
_regulator_put+0xe4/0xec
[   40.677335] Modules linked in: rfcomm uinput thermal_generic_adc
hci_uart btqca bluetooth ecdh_generic mtk_scp mtk_rpmsg mtk_scp_ipi
bridge rpmsg_core mt6577_auxadc stp llc nf_nat_tftp nv
[   40.731273] CPU: 6 PID: 2612 Comm: bash Tainted: G S  W
4.19.53 #99
[   40.738574] Hardware name: MediaTek krane rev3 sku16 board (DT)
[   40.744488] pstate: 8005 (Nzcv daif -PAN -UAO)
[   40.749276] pc : _regulator_put+0xe4/0xec
[   40.753282] lr : regulator_put+0x2c/0x40
[   40.757197] sp : ff800cef3b00
[   40.760505] x29: ff800cef3b10 x28: fff0e7a22a00
[   40.765814] x27:  x26: fff13876fa00
[   40.771122] x25: ff800cef3b68 x24: ff91f0b92000
[   40.776429] x23: ff91f03f2917 x22: ff91f0453a1c
[   40.781736] x21: fff13876fc00 x20: ff91f0a80988
[   40.787044] x19: fff13876fd00 x18: 0020
[   40.792351] x17: 0001 x16: 
[   40.797658] x15:  x14: 03f1
[   40.802966] x13: 0004 x12: 1f80c232
[   40.808272] x11:  x10: 0001
[   40.813580] x9 : 7ee042d282bebd00 x8 : 0001
[   40.818887] x7 : fefeff2f2f37306f x6 : 
[   40.824194] x5 :  x4 : ff91f0143fdc
[   40.829501] x3 :  x2 : 
[   40.834809] x1 : 038c x0 : fff13876fd00
[   40.840116] Call trace:
[   40.842560]  _regulator_put+0xe4/0xec
[   40.846218]  regulator_put+0x2c/0x40
[   40.849791]  devm_regulator_release+0x1c/0x28
[   40.854148]  release_nodes+0x1f0/0x244
[   40.857892]  devres_release_all+0x3c/0x54
[   40.861898]  device_release_driver_internal+0x148/0x1ec
[   40.867116]  device_release_driver+0x24/0x30
[   40.871380]  unbind_store+0x90/0xdc
[   40.874863]  drv_attr_store+0x3c/0x54
[   40.878524]  sysfs_kf_write+0x50/0x68
[   40.882180]  kernfs_fop_write+0x12c/0x1c8
[   40.886186]  __vfs_write+0x54/0x15c
[   40.889668]  vfs_write+0xcc/0x188
[   40.892978]  ksys_write+0x78/0xd8
[   40.896287]  __arm64_sys_write+0x20/0x2c
[   40.900208]  el0_svc_common+0xa4/0x154
[   40.903951]  el0_svc_compat_handler+0x2c/0x38
[   40.908302]  

[v3 4/4] drm/panel: support for auo,kd101n80-45na wuxga dsi video mode panel

[Jitao Shi]

Auo,kd101n80-45na's connector is same as boe,tv101wum-nl6.
The most codes can be reuse.
So auo,kd101n80-45na and boe,tv101wum-nl6 use one driver file.
Add the different parts in driver data.

Signed-off-by: Jitao Shi 
---
 .../gpu/drm/panel/panel-boe-tv101wum-nl6.c| 39 +++
 1 file changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c 
b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
index 30d1f53dcbaf..6ff49f900cd2 100644
--- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
+++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
@@ -372,6 +372,15 @@ static const struct panel_init_cmd boe_init_cmd[] = {
{},
 };
 
+static const struct panel_init_cmd auo_init_cmd[] = {
+   _INIT_DELAY_CMD(24),
+   _INIT_DCS_CMD(0x11),
+   _INIT_DELAY_CMD(120),
+   _INIT_DCS_CMD(0x29),
+   _INIT_DELAY_CMD(120),
+   {},
+};
+
 static inline struct boe_panel *to_boe_panel(struct drm_panel *panel)
 {
return container_of(panel, struct boe_panel, base);
@@ -571,6 +580,33 @@ static const struct panel_desc boe_tv101wum_nl6_desc = {
.init_cmds = boe_init_cmd,
 };
 
+static const struct drm_display_mode auo_default_mode = {
+   .clock = 157000,
+   .hdisplay = 1200,
+   .hsync_start = 1200 + 80,
+   .hsync_end = 1200 + 80 + 24,
+   .htotal = 1200 + 80 + 24 + 36,
+   .vdisplay = 1920,
+   .vsync_start = 1920 + 16,
+   .vsync_end = 1920 + 16 + 4,
+   .vtotal = 1920 + 16 + 4 + 16,
+   .vrefresh = 60,
+};
+
+static const struct panel_desc auo_kd101n80_45na_desc = {
+   .modes = _default_mode,
+   .bpc = 8,
+   .size = {
+   .width = 135,
+   .height = 216,
+   },
+   .lanes = 4,
+   .format = MIPI_DSI_FMT_RGB888,
+   .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
+ MIPI_DSI_MODE_LPM,
+   .init_cmds = auo_init_cmd,
+};
+
 static int boe_panel_get_modes(struct drm_panel *panel)
 {
struct boe_panel *boe = to_boe_panel(panel);
@@ -694,6 +730,9 @@ static const struct of_device_id boe_of_match[] = {
{ .compatible = "boe,tv101wum-nl6",
  .data = _tv101wum_nl6_desc
},
+   { .compatible = "auo,kd101n80-45na",
+ .data = _kd101n80_45na_desc
+   },
{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, boe_of_match);
-- 
2.21.0

[v3 2/4] drm/panel: support for BOE tv101wum-nl6 wuxga dsi video mode panel

[Jitao Shi]

Add driver for BOE tv101wum-nl6 panel is a 10.1" 1200x1920 panel.

Signed-off-by: Jitao Shi 
---
 drivers/gpu/drm/panel/Kconfig |   9 +
 drivers/gpu/drm/panel/Makefile|   1 +
 .../gpu/drm/panel/panel-boe-tv101wum-nl6.c| 714 ++
 3 files changed, 724 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index e36dbb4df867..706f6ff2b4fa 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -17,6 +17,15 @@ config DRM_PANEL_ARM_VERSATILE
  reference designs. The panel is detected using special registers
  in the Versatile family syscon registers.
 
+config DRM_PANEL_BOE_TV101WUM_NL6
+   tristate "BOE TV101WUM 1200x1920 panel"
+   depends on OF
+   depends on DRM_MIPI_DSI
+   depends on BACKLIGHT_CLASS_DEVICE
+   help
+ Say Y here if you want to support for BOE TV101WUM WUXGA PANEL
+ DSI Video Mode panel
+
 config DRM_PANEL_LVDS
tristate "Generic LVDS panel driver"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 78e3dc376bdd..8d009223c44e 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o
+obj-$(CONFIG_DRM_PANEL_BOE_TV101WUM_NL6) += panel-boe-tv101wum-nl6.o
 obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
 obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
 obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += 
panel-feiyang-fy07024di26a30d.o
diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c 
b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
new file mode 100644
index ..30d1f53dcbaf
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
@@ -0,0 +1,714 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 MediaTek Inc.
+ * Author: Jitao Shi 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include 
+
+struct panel_desc {
+   const struct drm_display_mode *modes;
+   unsigned int bpc;
+
+   /**
+* @width: width (in millimeters) of the panel's active display area
+* @height: height (in millimeters) of the panel's active display area
+*/
+   struct {
+   unsigned int width;
+   unsigned int height;
+   } size;
+
+   unsigned long mode_flags;
+   enum mipi_dsi_pixel_format format;
+   const struct panel_init_cmd *init_cmds;
+   unsigned int lanes;
+};
+
+struct boe_panel {
+   struct drm_panel base;
+   struct mipi_dsi_device *dsi;
+
+   const struct panel_desc *desc;
+
+   struct backlight_device *backlight;
+   struct regulator *pp1800;
+   struct regulator *avee;
+   struct regulator *avdd;
+   struct gpio_desc *enable_gpio;
+
+   bool prepared;
+   bool enabled;
+
+   const struct drm_display_mode *mode;
+};
+
+enum dsi_cmd_type {
+   INIT_DCS_CMD,
+   DELAY_CMD,
+};
+
+struct panel_init_cmd {
+   enum dsi_cmd_type type;
+   size_t len;
+   const char *data;
+};
+
+#define _INIT_DCS_CMD(...) { \
+   .type = INIT_DCS_CMD, \
+   .len = sizeof((char[]){__VA_ARGS__}), \
+   .data = (char[]){__VA_ARGS__} }
+
+#define _INIT_DELAY_CMD(...) { \
+   .type = DELAY_CMD,\
+   .len = sizeof((char[]){__VA_ARGS__}), \
+   .data = (char[]){__VA_ARGS__} }
+
+static const struct panel_init_cmd boe_init_cmd[] = {
+   _INIT_DELAY_CMD(24),
+   _INIT_DCS_CMD(0xB0, 0x05),
+   _INIT_DCS_CMD(0xB1, 0xE5),
+   _INIT_DCS_CMD(0xB3, 0x52),
+   _INIT_DCS_CMD(0xB0, 0x00),
+   _INIT_DCS_CMD(0xB3, 0x88),
+   _INIT_DCS_CMD(0xB0, 0x04),
+   _INIT_DCS_CMD(0xB8, 0x00),
+   _INIT_DCS_CMD(0xB0, 0x00),
+   _INIT_DCS_CMD(0xB6, 0x03),
+   _INIT_DCS_CMD(0xBA, 0x8B),
+   _INIT_DCS_CMD(0xBF, 0x1A),
+   _INIT_DCS_CMD(0xC0, 0x0F),
+   _INIT_DCS_CMD(0xC2, 0x0C),
+   _INIT_DCS_CMD(0xC3, 0x02),
+   _INIT_DCS_CMD(0xC4, 0x0C),
+   _INIT_DCS_CMD(0xC5, 0x02),
+   _INIT_DCS_CMD(0xB0, 0x01),
+   _INIT_DCS_CMD(0xE0, 0x26),
+   _INIT_DCS_CMD(0xE1, 0x26),
+   _INIT_DCS_CMD(0xDC, 0x00),
+   _INIT_DCS_CMD(0xDD, 0x00),
+   _INIT_DCS_CMD(0xCC, 0x26),
+   _INIT_DCS_CMD(0xCD, 0x26),
+   _INIT_DCS_CMD(0xC8, 0x00),
+   _INIT_DCS_CMD(0xC9, 0x00),
+   _INIT_DCS_CMD(0xD2, 0x03),
+   _INIT_DCS_CMD(0xD3, 0x03),
+   _INIT_DCS_CMD(0xE6, 0x04),
+   _INIT_DCS_CMD(0xE7, 0x04),
+   _INIT_DCS_CMD(0xC4, 0x09),
+   _INIT_DCS_CMD(0xC5, 0x09),
+   _INIT_DCS_CMD(0xD8, 0x0A),
+   _INIT_DCS_CMD(0xD9, 0x0A),
+   _INIT_DCS_CMD(0xC2, 0x0B),
+   _INIT_DCS_CMD(0xC3, 0x0B),
+   _INIT_DCS_CMD(0xD6, 0x0C),
+   _INIT_DCS_CMD(0xD7, 0x0C),
+   

[v3 1/4] dt-bindngs: display: panel: Add BOE tv101wum-n16 panel bindings

[Jitao Shi]

Add documentation for boe tv101wum-n16 panel.

Signed-off-by: Jitao Shi 
---
 .../display/panel/boe,tv101wum-nl6.txt| 34 +++
 1 file changed, 34 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.txt

diff --git 
a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.txt 
b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.txt
new file mode 100644
index ..bd44af636390
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.txt
@@ -0,0 +1,34 @@
+Boe Corporation 10.1" WUXGA TFT LCD panel
+
+Required properties:
+- compatible: should be "boe,tv101wum-nl6"
+- reg: the virtual channel number of a DSI peripheral
+- enable-gpios: a GPIO spec for the enable pin
+- pp1800-supply: core voltage supply
+- avdd-supply: phandle of the regulator that provides positive voltage
+- avee-supply: phandle of the regulator that provides negative voltage
+- backlight: phandle of the backlight device attached to the panel
+
+The device node can contain one 'port' child node with one child
+'endpoint' node, according to the bindings defined in
+media/video-interfaces.txt. This node should describe panel's video bus.
+
+Example:
+ {
+   ...
+   panel@0 {
+   compatible = "boe,tv101wum-nl6";
+   reg = <0>;
+   enable-gpios = < 45 0>;
+   avdd-supply = <_lcd>;
+   avee-supply = <_lcd>;
+   pp1800-supply = <_lcd>;
+   backlight = <_lcd0>;
+   status = "okay";
+   port {
+   panel_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   };
+};
-- 
2.21.0

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

[v3 0/4] Add drivers for auo, kd101n80-45na and boe, tv101wum-nl6 panels

[Jitao Shi]

Changes since v2:
 - correct the panel size
 - remove blank line in Kconfig
 - move auo,kd101n80-45na panel driver in this series.

Changes since v1:

 - update typo nl6 -> n16.
 - update new panel config and makefile are added in alphabetically order.
 - add the panel mode and panel info in driver data.
 - merge auo,kd101n80-45a and boe,tv101wum-nl6 in one driver

Jitao Shi (4):
  dt-bindngs: display: panel: Add BOE tv101wum-n16 panel bindings
  drm/panel: support for BOE tv101wum-nl6 wuxga dsi video mode panel
  dt-bindings: display: panel: add auo kd101n80-45na panel bindings
  drm/panel: support for auo,kd101n80-45na wuxga dsi video mode panel

 .../display/panel/auo,kd101n80-45na.txt   |  34 +
 .../display/panel/boe,tv101wum-nl6.txt|  34 +
 drivers/gpu/drm/panel/Kconfig |   9 +
 drivers/gpu/drm/panel/Makefile|   1 +
 .../gpu/drm/panel/panel-boe-tv101wum-nl6.c| 753 ++
 5 files changed, 831 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/auo,kd101n80-45na.txt
 create mode 100644 
Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.txt
 create mode 100644 drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c

-- 
2.21.0

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

[v3 3/4] dt-bindings: display: panel: add auo kd101n80-45na panel bindings

[Jitao Shi]

Add documentation for auo kd101n80-45na panel.

Signed-off-by: Jitao Shi 
Reviewed-by: Sam Ravnborg 
---
 .../display/panel/auo,kd101n80-45na.txt   | 34 +++
 1 file changed, 34 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/auo,kd101n80-45na.txt

diff --git 
a/Documentation/devicetree/bindings/display/panel/auo,kd101n80-45na.txt 
b/Documentation/devicetree/bindings/display/panel/auo,kd101n80-45na.txt
new file mode 100644
index ..994c2a13f942
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/auo,kd101n80-45na.txt
@@ -0,0 +1,34 @@
+AUO Corporation 10.1" WUXGA TFT LCD panel
+
+Required properties:
+- compatible: should be "auo,kd101n80-45na"
+- reg: the virtual channel number of a DSI peripheral
+- enable-gpios: a GPIO spec for the enable pin
+- pp1800-supply: core voltage supply
+- avdd-supply: phandle of the regulator that provides positive voltage
+- avee-supply: phandle of the regulator that provides negative voltage
+- backlight: phandle of the backlight device attached to the panel
+
+The device node can contain one 'port' child node with one child
+'endpoint' node, according to the bindings defined in
+media/video-interfaces.txt. This node should describe panel's video bus.
+
+Example:
+ {
+   ...
+   panel@0 {
+   compatible = "auo,kd101n80-45na";
+   reg = <0>;
+   enable-gpios = < 45 0>;
+   avdd-supply = <_lcd>;
+   avee-supply = <_lcd>;
+   pp1800-supply = <_lcd>;
+   backlight = <_lcd0>;
+   status = "okay";
+   port {
+   panel_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   };
+};
-- 
2.21.0

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

Re: [PATCH v5 00/18] kunit: introduce KUnit, the Linux kernel unit testing framework

[Luis Chamberlain]

On Wed, Jun 19, 2019 at 06:17:51PM -0700, Frank Rowand wrote:
> It does not matter whether KUnit provides additional things, relative
> to kselftest.  The point I was making is that there appears to be
> _some_ overlap between kselftest and KUnit, and if there is overlap
> then it is worth considering whether the overlap can be unified instead
> of duplicated.

From my experience as an author of several kselftests drivers, one
faily complex, and after reviewing the sysctl kunit test module, I
disagree with this.

Even if there were an overlap, I'd say let the best test harness win.
Just as we have different LSMs that do something similar.

But this is not about that though. Although both are testing code,
they do so in *very* different ways.

The design philosophy and architecture are fundamentally different. The
*only* thing I can think of where there is overlap is that both can test
similar code paths. Beyond that, the layout of how one itemizes tests
could be borrowed, but that would be up to each kselftest author to
decide, in fact some ksefltests do exist which follow similar pattern of
itemizing test cases and running them. Kunit just provides a proper
framework to do this easily but also with a focus on UML. This last
aspect makes kselftests fundamentally orthogonal from an architecture /
design perspective.

After careful review, I cannot personally identify what could be shared
at this point. Can you? If you did identify one part, how do you
recommend to share it?

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

[PATCH v3 2/6] dt-bindings: display: msm: gmu: add optional ocmem property

[Brian Masney]

Some A3xx and A4xx Adreno GPUs do not have GMEM inside the GPU core and
must use the On Chip MEMory (OCMEM) in order to be functional. Add the
optional ocmem property to the Adreno Graphics Management Unit bindings.

Signed-off-by: Brian Masney 
---
Changes since v2:
- Add a3xx example with OCMEM

Changes since v1:
- None

 .../devicetree/bindings/display/msm/gmu.txt   | 50 +++
 1 file changed, 50 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/gmu.txt 
b/Documentation/devicetree/bindings/display/msm/gmu.txt
index 90af5b0a56a9..e5596994df7b 100644
--- a/Documentation/devicetree/bindings/display/msm/gmu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gmu.txt
@@ -31,6 +31,10 @@ Required properties:
 - iommus: phandle to the adreno iommu
 - operating-points-v2: phandle to the OPP operating points
 
+Optional properties:
+- ocmem: phandle to the On Chip Memory (OCMEM) that's present on some 
Snapdragon
+ SoCs. See Documentation/devicetree/bindings/soc/qcom/qcom,ocmem.yaml.
+
 Example:
 
 / {
@@ -63,3 +67,49 @@ Example:
operating-points-v2 = <_opp_table>;
};
 };
+
+a3xx example with OCMEM support:
+
+/ {
+   ...
+
+   gpu: adreno@fdb0 {
+   compatible = "qcom,adreno-330.2",
+"qcom,adreno";
+   reg = <0xfdb0 0x1>;
+   reg-names = "kgsl_3d0_reg_memory";
+   interrupts = ;
+   interrupt-names = "kgsl_3d0_irq";
+   clock-names = "core",
+ "iface",
+ "mem_iface";
+   clocks = < OXILI_GFX3D_CLK>,
+< OXILICX_AHB_CLK>,
+< OXILICX_AXI_CLK>;
+   ocmem = <>;
+   power-domains = < OXILICX_GDSC>;
+   operating-points-v2 = <_opp_table>;
+   iommus = <_iommu 0>;
+   };
+
+   ocmem: ocmem@fdd0 {
+   compatible = "qcom,msm8974-ocmem";
+
+   reg = <0xfdd0 0x2000>,
+ <0xfec0 0x18>;
+   reg-names = "ctrl",
+"mem";
+
+   clocks = < RPM_SMD_OCMEMGX_CLK>,
+< OCMEMCX_OCMEMNOC_CLK>;
+   clock-names = "core",
+ "iface";
+
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   gmu-sram@0 {
+   reg = <0x0 0x10>;
+   };
+   };
+};
-- 
2.20.1

[PATCH v3 1/6] dt-bindings: soc: qcom: add On Chip MEMory (OCMEM) bindings

[Brian Masney]

Add device tree bindings for the On Chip Memory (OCMEM) that is present
on some Qualcomm Snapdragon SoCs.

Signed-off-by: Brian Masney 
---
Changes since v2:
- Add *-sram node and gmu-sram to example.

Changes since v1:
- Rename qcom,ocmem-msm8974 to qcom,msm8974-ocmem
- Renamed reg-names to ctrl and mem
- update hardware description
- moved from soc to sram namespace in the device tree bindings

 .../bindings/sram/qcom/qcom,ocmem.yaml| 84 +++
 1 file changed, 84 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sram/qcom/qcom,ocmem.yaml

diff --git a/Documentation/devicetree/bindings/sram/qcom/qcom,ocmem.yaml 
b/Documentation/devicetree/bindings/sram/qcom/qcom,ocmem.yaml
new file mode 100644
index ..a0bf0af4860a
--- /dev/null
+++ b/Documentation/devicetree/bindings/sram/qcom/qcom,ocmem.yaml
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sram/qcom/qcom,ocmem.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: On Chip Memory (OCMEM) that is present on some Qualcomm Snapdragon SoCs.
+
+maintainers:
+  - Brian Masney 
+
+description: |
+  The On Chip Memory (OCMEM) is typically used by the GPU, camera/video, and
+  audio components on some Snapdragon SoCs.
+
+properties:
+  compatible:
+const: qcom,msm8974-ocmem
+
+  reg:
+items:
+  - description: Control registers
+  - description: OCMEM address range
+
+  reg-names:
+items:
+  - const: ctrl
+  - const: mem
+
+  clocks:
+items:
+  - description: Core clock
+  - description: Interface clock
+
+  clock-names:
+items:
+  - const: core
+  - const: iface
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - clock-names
+
+patternProperties:
+  "^.+-sram$":
+type: object
+description: |
+  A region of reserved memory.
+
+properties:
+  reg:
+maxItems: 1
+
+required:
+  - reg
+
+examples:
+  - |
+  #include 
+  #include 
+
+  ocmem: ocmem@fdd0 {
+compatible = "qcom,msm8974-ocmem";
+
+reg = <0xfdd0 0x2000>,
+  <0xfec0 0x18>;
+reg-names = "ctrl",
+"mem";
+
+clocks = < RPM_SMD_OCMEMGX_CLK>,
+ < OCMEMCX_OCMEMNOC_CLK>;
+clock-names = "core",
+  "iface";
+
+#address-cells = <1>;
+#size-cells = <1>;
+
+gmu-sram@0 {
+reg = <0x0 0x10>;
+};
+  };
-- 
2.20.1

[PATCH v3 4/6] firmware: qcom: scm: add support to restore secure config to qcm_scm-32

[Brian Masney]

From: Rob Clark 

Add support to restore the secure configuration for qcm_scm-32.c. This
is needed by the On Chip MEMory (OCMEM) that is present on some
Snapdragon devices.

Signed-off-by: Rob Clark 
[masn...@onstation.org: ported to latest kernel; set ctx_bank_num to
 spare parameter.]
Signed-off-by: Brian Masney 
---
Changes since v2:
- None

Changes since v1:
- Use existing __qcom_scm_restore_sec_cfg() function stub in
  qcom_scm-32.c that was unimplemented
- Set the cfg.ctx_bank_num to the spare function parameter. It was
  previously set to the device_id.

 drivers/firmware/qcom_scm-32.c | 17 -
 drivers/firmware/qcom_scm.c| 13 +
 include/linux/qcom_scm.h   | 11 +++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 4c2514e5e249..5d90b7f5ab5a 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -617,7 +617,22 @@ int __qcom_scm_assign_mem(struct device *dev, phys_addr_t 
mem_region,
 int __qcom_scm_restore_sec_cfg(struct device *dev, u32 device_id,
   u32 spare)
 {
-   return -ENODEV;
+   struct msm_scm_sec_cfg {
+   __le32 id;
+   __le32 ctx_bank_num;
+   } cfg;
+   int ret, scm_ret = 0;
+
+   cfg.id = cpu_to_le32(device_id);
+   cfg.ctx_bank_num = cpu_to_le32(spare);
+
+   ret = qcom_scm_call(dev, QCOM_SCM_SVC_MP, QCOM_SCM_RESTORE_SEC_CFG,
+   , sizeof(cfg), _ret, sizeof(scm_ret));
+
+   if (ret || scm_ret)
+   return ret ? ret : -EINVAL;
+
+   return 0;
 }
 
 int __qcom_scm_iommu_secure_ptbl_size(struct device *dev, u32 spare,
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 2e12ea56c34c..54532331ddc1 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -366,6 +366,19 @@ static const struct reset_control_ops 
qcom_scm_pas_reset_ops = {
.deassert = qcom_scm_pas_reset_deassert,
 };
 
+/**
+ * qcom_scm_restore_sec_cfg_available() - Check if secure environment
+ * supports restore security config interface.
+ *
+ * Return true if restore-cfg interface is supported, false if not.
+ */
+bool qcom_scm_restore_sec_cfg_available(void)
+{
+   return __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_MP,
+   QCOM_SCM_RESTORE_SEC_CFG);
+}
+EXPORT_SYMBOL(qcom_scm_restore_sec_cfg_available);
+
 int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare)
 {
return __qcom_scm_restore_sec_cfg(__scm->dev, device_id, spare);
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index 521b089be1c9..8a24f7eb2588 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -34,6 +34,16 @@ enum qcom_scm_ocmem_client {
QCOM_SCM_OCMEM_DEBUG_ID,
 };
 
+enum qcom_scm_sec_dev_id {
+   QCOM_SCM_MDSS_DEV_ID= 1,
+   QCOM_SCM_OCMEM_DEV_ID   = 5,
+   QCOM_SCM_PCIE0_DEV_ID   = 11,
+   QCOM_SCM_PCIE1_DEV_ID   = 12,
+   QCOM_SCM_GFX_DEV_ID = 18,
+   QCOM_SCM_UFS_DEV_ID = 19,
+   QCOM_SCM_ICE_DEV_ID = 20,
+};
+
 #define QCOM_SCM_VMID_HLOS   0x3
 #define QCOM_SCM_VMID_MSS_MSA0xF
 #define QCOM_SCM_VMID_WLAN   0x18
@@ -69,6 +79,7 @@ extern int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t 
mem_sz,
 extern void qcom_scm_cpu_power_down(u32 flags);
 extern u32 qcom_scm_get_version(void);
 extern int qcom_scm_set_remote_state(u32 state, u32 id);
+extern bool qcom_scm_restore_sec_cfg_available(void);
 extern int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare);
 extern int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size);
 extern int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare);
-- 
2.20.1

[PATCH v3 5/6] soc: qcom: add OCMEM driver

[Brian Masney]

The OCMEM driver handles allocation and configuration of the On Chip
MEMory that is present on some Snapdragon SoCs. Devices which have
OCMEM do not have GMEM inside the GPU core, so the GPU must instead
use OCMEM to be functional. Since the GPU is currently the only OCMEM
user with an upstream driver, this is just a minimal implementation
sufficient for statically allocating to the GPU it's chunk of OCMEM.

This driver currently does not read the gmu-sram node that is described
in the device tree bindings. The starting memory address of the GPU's
reserved memory region is hardcoded to zero to match what the hardware
expects. The driver can be updated to read the reserved memory regions
from device tree once other users of OCMEM are added upstream.

Signed-off-by: Brian Masney 
Co-developed-by: Rob Clark 
Signed-off-by: Rob Clark 
---
Changes since v2
- Changed static inline stubs return -ENODEV when OCMEM is not
  configured into the kernel.

Changes since v1:
- ocmem_allocate(): check for alignment and minimum allocation size.
  The 64K values came from the downstream MSM kernel sources.
- add locking to memory allocations based on the client
- use clk_bulk_*() functions
- rename qcom,ocmem-msm8974 to qcom,msm8974-ocmem
- rename reg-names to ctrl and mem
- remove ocmem.xml.h file; use FIELD_PREP() instead for some nice cleanups
- add static inline noop versions of public-facing functions when ocmem
  is disabled to remove #ifdefs in adrenu_gpu.c
- use unsigned long for memory addresses
- move ocmem_dev_remove() below _probe() function
- remove error check from platform_get_resource_byname for ctrl resource
- add MODULE_DESCRIPTION() and MODULE_LICENSE()
- add description to top of ocmem.[ch]
- correct thin mode bit in update_ocmem()
- add 'WARN_ON(client != OCMEM_GRAPHICS)' to device_address()
- make of_get_ocmem return error codes via ERR_PTR instead of NULL
- ocmem_{allocate,free} - WARN_ON() if client != OCMEM_GRAPHICS.
  Simplify if statements.
- allow NULL to be passed into ocmem_free
- remove unnecessary initialization of i in update_ocmem()
- add dev_dbg to ocmem_allocate

Changes since Rob's last version of this patch from 2015:
https://patchwork.kernel.org/patch/7379801/
- reformatted driver to allow multiple instances
- updated logging of error paths during device probing
- remove unused psgsc_ctrl
- remove _clk from clock names
- propagate error code from devm_ioremap_resource()
- use device_get_match_data()
- SPDX license tags
- remove QCOM_SMD in Kconfig
- select ARCH_QCOM in Kconfig
- select ARCH_QCOM in Kconfig
- select QCOM_SCM in Kconfig
- longer description in Kconfig

 drivers/soc/qcom/Kconfig  |  10 +
 drivers/soc/qcom/Makefile |   1 +
 drivers/soc/qcom/ocmem.c  | 433 ++
 include/soc/qcom/ocmem.h  |  62 ++
 4 files changed, 506 insertions(+)
 create mode 100644 drivers/soc/qcom/ocmem.c
 create mode 100644 include/soc/qcom/ocmem.h

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index a6d1bfb17279..d18eb83b10da 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -74,6 +74,16 @@ config QCOM_MDT_LOADER
tristate
select QCOM_SCM
 
+config QCOM_OCMEM
+   tristate "Qualcomm On Chip Memory (OCMEM) driver"
+   depends on ARCH_QCOM
+   select QCOM_SCM
+   help
+  The On Chip Memory (OCMEM) allocator allows various clients to
+  allocate memory from OCMEM based on performance, latency and power
+  requirements. This is typically used by the GPU, camera/video, and
+  audio components on some Snapdragon SoCs.
+
 config QCOM_PM
bool "Qualcomm Power Management"
depends on ARCH_QCOM && !ARM64
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index eeb088beb15f..dfc378014a33 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
 obj-$(CONFIG_QCOM_GLINK_SSR) +=glink_ssr.o
 obj-$(CONFIG_QCOM_GSBI)+=  qcom_gsbi.o
 obj-$(CONFIG_QCOM_MDT_LOADER)  += mdt_loader.o
+obj-$(CONFIG_QCOM_OCMEM)   += ocmem.o
 obj-$(CONFIG_QCOM_PM)  +=  spm.o
 obj-$(CONFIG_QCOM_QMI_HELPERS) += qmi_helpers.o
 qmi_helpers-y  += qmi_encdec.o qmi_interface.o
diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
new file mode 100644
index ..7c28ad3108a6
--- /dev/null
+++ b/drivers/soc/qcom/ocmem.c
@@ -0,0 +1,433 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * The On Chip Memory (OCMEM) allocator allows various clients to allocate
+ * memory from OCMEM based on performance, latency and power requirements.
+ * This is typically used by the GPU, camera/video, and audio components on
+ * some Snapdragon SoCs.
+ *
+ * Copyright (C) 2019 Brian Masney 
+ * Copyright (C) 2015 Red Hat. Author: Rob Clark 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+

[PATCH v3 6/6] drm/msm/gpu: add ocmem init/cleanup functions

[Brian Masney]

The files a3xx_gpu.c and a4xx_gpu.c have ifdefs for the OCMEM support
that was missing upstream. Add two new functions (adreno_gpu_ocmem_init
and adreno_gpu_ocmem_cleanup) that removes some duplicated code.

Signed-off-by: Brian Masney 
---
Changes since v2:
- Check for -ENODEV error of_get_ocmem()
- remove fail_cleanup_ocmem label in a[34]xx_gpu_init

Changes since v1:
- remove CONFIG_QCOM_OCMEM #ifdefs
- use unsigned long for memory addresses instead of uint32_t
- add 'depends on QCOM_OCMEM || QCOM_OCMEM=n' to Kconfig

 drivers/gpu/drm/msm/Kconfig |  1 +
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c   | 28 +
 drivers/gpu/drm/msm/adreno/a3xx_gpu.h   |  3 +-
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c   | 25 
 drivers/gpu/drm/msm/adreno/a4xx_gpu.h   |  3 +-
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 40 +
 drivers/gpu/drm/msm/adreno/adreno_gpu.h | 10 +++
 7 files changed, 66 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 9c37e4de5896..b3d3b2172659 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -7,6 +7,7 @@ config DRM_MSM
depends on OF && COMMON_CLK
depends on MMU
depends on INTERCONNECT || !INTERCONNECT
+   depends on QCOM_OCMEM || QCOM_OCMEM=n
select QCOM_MDT_LOADER if ARCH_QCOM
select REGULATOR
select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index c3b4bc6e4155..b3ef06a39653 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -17,10 +17,6 @@
  * this program.  If not, see .
  */
 
-#ifdef CONFIG_MSM_OCMEM
-#  include 
-#endif
-
 #include "a3xx_gpu.h"
 
 #define A3XX_INT0_MASK \
@@ -206,9 +202,9 @@ static int a3xx_hw_init(struct msm_gpu *gpu)
gpu_write(gpu, REG_A3XX_RBBM_GPR0_CTL, 0x);
 
/* Set the OCMEM base address for A330, etc */
-   if (a3xx_gpu->ocmem_hdl) {
+   if (a3xx_gpu->ocmem.hdl) {
gpu_write(gpu, REG_A3XX_RB_GMEM_BASE_ADDR,
-   (unsigned int)(a3xx_gpu->ocmem_base >> 14));
+   (unsigned int)(a3xx_gpu->ocmem.base >> 14));
}
 
/* Turn on performance counters: */
@@ -329,10 +325,7 @@ static void a3xx_destroy(struct msm_gpu *gpu)
 
adreno_gpu_cleanup(adreno_gpu);
 
-#ifdef CONFIG_MSM_OCMEM
-   if (a3xx_gpu->ocmem_base)
-   ocmem_free(OCMEM_GRAPHICS, a3xx_gpu->ocmem_hdl);
-#endif
+   adreno_gpu_ocmem_cleanup(_gpu->ocmem);
 
kfree(a3xx_gpu);
 }
@@ -507,17 +500,10 @@ struct msm_gpu *a3xx_gpu_init(struct drm_device *dev)
 
/* if needed, allocate gmem: */
if (adreno_is_a330(adreno_gpu)) {
-#ifdef CONFIG_MSM_OCMEM
-   /* TODO this is different/missing upstream: */
-   struct ocmem_buf *ocmem_hdl =
-   ocmem_allocate(OCMEM_GRAPHICS, 
adreno_gpu->gmem);
-
-   a3xx_gpu->ocmem_hdl = ocmem_hdl;
-   a3xx_gpu->ocmem_base = ocmem_hdl->addr;
-   adreno_gpu->gmem = ocmem_hdl->len;
-   DBG("using %dK of OCMEM at 0x%08x", adreno_gpu->gmem / 1024,
-   a3xx_gpu->ocmem_base);
-#endif
+   ret = adreno_gpu_ocmem_init(_gpu->base.pdev->dev,
+   adreno_gpu, _gpu->ocmem);
+   if (ret)
+   goto fail;
}
 
if (!gpu->aspace) {
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.h 
b/drivers/gpu/drm/msm/adreno/a3xx_gpu.h
index ab60dc9e344e..727c34f38f9e 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.h
@@ -30,8 +30,7 @@ struct a3xx_gpu {
struct adreno_gpu base;
 
/* if OCMEM is used for GMEM: */
-   uint32_t ocmem_base;
-   void *ocmem_hdl;
+   struct adreno_ocmem ocmem;
 };
 #define to_a3xx_gpu(x) container_of(x, struct a3xx_gpu, base)
 
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
index ab2b752566d8..b01388a9e89e 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -2,9 +2,6 @@
 /* Copyright (c) 2014 The Linux Foundation. All rights reserved.
  */
 #include "a4xx_gpu.h"
-#ifdef CONFIG_MSM_OCMEM
-#  include 
-#endif
 
 #define A4XX_INT0_MASK \
(A4XX_INT0_RBBM_AHB_ERROR |\
@@ -188,7 +185,7 @@ static int a4xx_hw_init(struct msm_gpu *gpu)
(1 << 30) | 0x);
 
gpu_write(gpu, REG_A4XX_RB_GMEM_BASE_ADDR,
-   (unsigned int)(a4xx_gpu->ocmem_base >> 14));
+   (unsigned int)(a4xx_gpu->ocmem.base >> 14));
 
/* Turn on performance counters: */
gpu_write(gpu, REG_A4XX_RBBM_PERFCTR_CTL, 0x01);
@@ -318,10 +315,7 @@ static void a4xx_destroy(struct msm_gpu 

[PATCH v3 0/6] qcom: add OCMEM support

[Brian Masney]

This patch series adds support for Qualcomm's On Chip MEMory (OCMEM)
that is needed in order to support some a3xx and a4xx-based GPUs
upstream. This is based on Rob Clark's patch series that he submitted
in October 2015 and I am resubmitting updated patches with his
permission. See the individual patches for the changelog.

This was tested with the GPU on a LG Nexus 5 (hammerhead) phone and
this will work on other msm8974-based systems. For a summary of what
currently works upstream on the Nexus 5, see my status page at
https://masneyb.github.io/nexus-5-upstream/.

Brian Masney (4):
  dt-bindings: soc: qcom: add On Chip MEMory (OCMEM) bindings
  dt-bindings: display: msm: gmu: add optional ocmem property
  soc: qcom: add OCMEM driver
  drm/msm/gpu: add ocmem init/cleanup functions

Rob Clark (2):
  firmware: qcom: scm: add OCMEM lock/unlock interface
  firmware: qcom: scm: add support to restore secure config to
qcm_scm-32

 .../devicetree/bindings/display/msm/gmu.txt   |  50 ++
 .../bindings/sram/qcom/qcom,ocmem.yaml|  84 
 drivers/firmware/qcom_scm-32.c|  52 ++-
 drivers/firmware/qcom_scm-64.c|  12 +
 drivers/firmware/qcom_scm.c   |  53 +++
 drivers/firmware/qcom_scm.h   |   9 +
 drivers/gpu/drm/msm/Kconfig   |   1 +
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c |  28 +-
 drivers/gpu/drm/msm/adreno/a3xx_gpu.h |   3 +-
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c |  25 +-
 drivers/gpu/drm/msm/adreno/a4xx_gpu.h |   3 +-
 drivers/gpu/drm/msm/adreno/adreno_gpu.c   |  40 ++
 drivers/gpu/drm/msm/adreno/adreno_gpu.h   |  10 +
 drivers/soc/qcom/Kconfig  |  10 +
 drivers/soc/qcom/Makefile |   1 +
 drivers/soc/qcom/ocmem.c  | 433 ++
 include/linux/qcom_scm.h  |  26 ++
 include/soc/qcom/ocmem.h  |  62 +++
 18 files changed, 857 insertions(+), 45 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sram/qcom/qcom,ocmem.yaml
 create mode 100644 drivers/soc/qcom/ocmem.c
 create mode 100644 include/soc/qcom/ocmem.h

-- 
2.20.1

[PATCH v3 3/6] firmware: qcom: scm: add OCMEM lock/unlock interface

[Brian Masney]

From: Rob Clark 

Add support for the OCMEM lock/unlock interface that is needed by the
On Chip MEMory (OCMEM) that is present on some Snapdragon devices.

Signed-off-by: Rob Clark 
[masn...@onstation.org: ported to latest kernel; minor reformatting.]
Signed-off-by: Brian Masney 
Reviewed-by: Bjorn Andersson 
---
Rob's last version of this patch:
https://patchwork.kernel.org/patch/7340711/

Changes since v2:
- None

Changes since v1:
- None

 drivers/firmware/qcom_scm-32.c | 35 +
 drivers/firmware/qcom_scm-64.c | 12 ++
 drivers/firmware/qcom_scm.c| 40 ++
 drivers/firmware/qcom_scm.h|  9 
 include/linux/qcom_scm.h   | 15 +
 5 files changed, 111 insertions(+)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 215061c581e1..4c2514e5e249 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -442,6 +442,41 @@ int __qcom_scm_hdcp_req(struct device *dev, struct 
qcom_scm_hdcp_req *req,
req, req_cnt * sizeof(*req), resp, sizeof(*resp));
 }
 
+int __qcom_scm_ocmem_lock(struct device *dev, u32 id, u32 offset, u32 size,
+ u32 mode)
+{
+   struct ocmem_tz_lock {
+   __le32 id;
+   __le32 offset;
+   __le32 size;
+   __le32 mode;
+   } request;
+
+   request.id = cpu_to_le32(id);
+   request.offset = cpu_to_le32(offset);
+   request.size = cpu_to_le32(size);
+   request.mode = cpu_to_le32(mode);
+
+   return qcom_scm_call(dev, QCOM_SCM_OCMEM_SVC, QCOM_SCM_OCMEM_LOCK_CMD,
+, sizeof(request), NULL, 0);
+}
+
+int __qcom_scm_ocmem_unlock(struct device *dev, u32 id, u32 offset, u32 size)
+{
+   struct ocmem_tz_unlock {
+   __le32 id;
+   __le32 offset;
+   __le32 size;
+   } request;
+
+   request.id = cpu_to_le32(id);
+   request.offset = cpu_to_le32(offset);
+   request.size = cpu_to_le32(size);
+
+   return qcom_scm_call(dev, QCOM_SCM_OCMEM_SVC, QCOM_SCM_OCMEM_UNLOCK_CMD,
+, sizeof(request), NULL, 0);
+}
+
 void __qcom_scm_init(void)
 {
 }
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index 91d5ad7cf58b..c3a3d9874def 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -241,6 +241,18 @@ int __qcom_scm_hdcp_req(struct device *dev, struct 
qcom_scm_hdcp_req *req,
return ret;
 }
 
+int __qcom_scm_ocmem_lock(struct device *dev, uint32_t id, uint32_t offset,
+ uint32_t size, uint32_t mode)
+{
+   return -ENOTSUPP;
+}
+
+int __qcom_scm_ocmem_unlock(struct device *dev, uint32_t id, uint32_t offset,
+   uint32_t size)
+{
+   return -ENOTSUPP;
+}
+
 void __qcom_scm_init(void)
 {
u64 cmd;
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 2ddc118dba1b..2e12ea56c34c 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -190,6 +190,46 @@ bool qcom_scm_pas_supported(u32 peripheral)
 }
 EXPORT_SYMBOL(qcom_scm_pas_supported);
 
+/**
+ * qcom_scm_ocmem_lock_available() - is OCMEM lock/unlock interface available
+ */
+bool qcom_scm_ocmem_lock_available(void)
+{
+   return __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_OCMEM_SVC,
+   QCOM_SCM_OCMEM_LOCK_CMD);
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_lock_available);
+
+/**
+ * qcom_scm_ocmem_lock() - call OCMEM lock interface to assign an OCMEM
+ * region to the specified initiator
+ *
+ * @id: tz initiator id
+ * @offset: OCMEM offset
+ * @size:   OCMEM size
+ * @mode:   access mode (WIDE/NARROW)
+ */
+int qcom_scm_ocmem_lock(enum qcom_scm_ocmem_client id, u32 offset, u32 size,
+   u32 mode)
+{
+   return __qcom_scm_ocmem_lock(__scm->dev, id, offset, size, mode);
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_lock);
+
+/**
+ * qcom_scm_ocmem_unlock() - call OCMEM unlock interface to release an OCMEM
+ * region from the specified initiator
+ *
+ * @id: tz initiator id
+ * @offset: OCMEM offset
+ * @size:   OCMEM size
+ */
+int qcom_scm_ocmem_unlock(enum qcom_scm_ocmem_client id, u32 offset, u32 size)
+{
+   return __qcom_scm_ocmem_unlock(__scm->dev, id, offset, size);
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_unlock);
+
 /**
  * qcom_scm_pas_init_image() - Initialize peripheral authentication service
  *state machine for a given peripheral, using the
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 99506bd873c0..ef293ee67ec1 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -42,6 +42,15 @@ extern int __qcom_scm_hdcp_req(struct device *dev,
 
 extern void __qcom_scm_init(void);
 
+#define QCOM_SCM_OCMEM_SVC 0xf
+#define QCOM_SCM_OCMEM_LOCK_CMD0x1

Re: [PATCH v5 18/18] MAINTAINERS: add proc sysctl KUnit test to PROC SYSCTL section

[Luis Chamberlain]

On Mon, Jun 17, 2019 at 01:26:13AM -0700, Brendan Higgins wrote:
> Add entry for the new proc sysctl KUnit test to the PROC SYSCTL section.
> 
> Signed-off-by: Brendan Higgins 
> Reviewed-by: Greg Kroah-Hartman 
> Reviewed-by: Logan Gunthorpe 

Acked-by: Luis Chamberlain 

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

Re: [PATCH v5 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec()

[Luis Chamberlain]

On Mon, Jun 17, 2019 at 01:26:12AM -0700, Brendan Higgins wrote:
> From: Iurii Zaikin 
> 
> KUnit tests for initialized data behavior of proc_dointvec that is
> explicitly checked in the code. Includes basic parsing tests including
> int min/max overflow.

First, thanks for this work! My review below.
> 
> Signed-off-by: Iurii Zaikin 
> Signed-off-by: Brendan Higgins 
> Reviewed-by: Greg Kroah-Hartman 
> Reviewed-by: Logan Gunthorpe 
> ---
> Changes Since Last Revision:
>  - Iurii did some clean up (thanks Iurii!) as suggested by Stephen Boyd.
> ---
>  kernel/Makefile  |   2 +
>  kernel/sysctl-test.c | 242 +++
>  lib/Kconfig.debug|  10 ++
>  3 files changed, 254 insertions(+)
>  create mode 100644 kernel/sysctl-test.c
> 
> diff --git a/kernel/Makefile b/kernel/Makefile
> index a8d923b5481ba..50fd511cd0ee0 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -114,6 +114,8 @@ obj-$(CONFIG_HAS_IOMEM) += iomem.o
>  obj-$(CONFIG_ZONE_DEVICE) += memremap.o
>  obj-$(CONFIG_RSEQ) += rseq.o
>  
> +obj-$(CONFIG_SYSCTL_KUNIT_TEST) += sysctl-test.o

And we have lib/test_sysctl.c of selftests.

I'm fine with this going in as-is to its current place, but if we have
to learn from selftests I'd say we try to stick to a convention so
folks know what framework a test is for, and to ensure folks can
easily tell if its test code or not.

Perhaps simply a directory for kunit tests would suffice alone.

> +
>  obj-$(CONFIG_GCC_PLUGIN_STACKLEAK) += stackleak.o
>  KASAN_SANITIZE_stackleak.o := n
>  KCOV_INSTRUMENT_stackleak.o := n
> diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
> new file mode 100644
> index 0..cb61ad3c7db63
> --- /dev/null
> +++ b/kernel/sysctl-test.c
> @@ -0,0 +1,242 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test of proc sysctl.
> + */
> +
> +#include 
> +#include 
> +
> +static int i_zero;
> +static int i_one_hundred = 100;
> +
> +struct test_sysctl_data {
> + int int_0001;
> + int int_0002;
> + int int_0003[4];
> +
> + unsigned int uint_0001;
> +
> + char string_0001[65];
> +};
> +
> +static struct test_sysctl_data test_data = {
> + .int_0001 = 60,
> + .int_0002 = 1,
> +
> + .int_0003[0] = 0,
> + .int_0003[1] = 1,
> + .int_0003[2] = 2,
> + .int_0003[3] = 3,
> +
> + .uint_0001 = 314,
> +
> + .string_0001 = "(none)",
> +};
> +
> +static void sysctl_test_dointvec_null_tbl_data(struct kunit *test)
> +{
> + struct ctl_table table = {
> + .procname = "foo",
> + .data   = NULL,
> + .maxlen = sizeof(int),
> + .mode   = 0644,
> + .proc_handler   = proc_dointvec,
> + .extra1 = _zero,
> + .extra2 = _one_hundred,
> + };
> + void  *buffer = kunit_kzalloc(test, sizeof(int), GFP_USER);
> + size_t len;
> + loff_t pos;
> +
> + len = 1234;
> + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(, 0, buffer, , ));
> + KUNIT_EXPECT_EQ(test, (size_t)0, len);

It is a bit odd, but it does happen, for a developer to be calling
proc_dointvec() directly, instead typically folks just register a table
and let it do its thing.  That said, someone not too familiar with proc
code would see this and really have no clue exactly what is being
tested.

Even as a maintainer, I had to read the code for proc_dointvec() a bit
to understand that the above is a *read* attempt to the .data field
being allocated. Because its a write, the len set to a bogus does not
matter as we are expecting the proc_dointvec() to update len for us.

If a test fails, it would be good to for anyone to easily grasp what is
being tested. So... a few words documenting each test case would be nice.

> + len = 1234;
> + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(, 1, buffer, , ));
> + KUNIT_EXPECT_EQ(test, (size_t)0, len);

And this is a write...

A nice tests given the data on the table allocated is not assigned.

I don't see any other areas in the kernel where we open code a
proc_dointvec() call where the second argument is a digit, it
always is with a variable. As such there would be no need for
us to expose helpers to make it clear if one is a read or write.
But for *this* case, I think it would be useful to add two wrappers
inside this kunit test module which sprinkles the 0 or 1, this way
a reader can easily know what mode is being tested.

kunit_proc_dointvec_read()
kunit_proc_dointvec_write()

Or just use #define KUNIT_PROC_READ 0, #define KUNIT_PROC_WRITE 1.
Whatever makes this code more legible.

> +}
> +
> +static void sysctl_test_dointvec_table_maxlen_unset(struct kunit *test)
> +{
> + struct ctl_table table = {
> + .procname = "foo",
> + .data   = _data.int_0001,
> + .maxlen = 0,
> + .mode   = 0644,
> + .proc_handler   = proc_dointvec,
> + .extra1 = _zero,
> + 

Re: [PATCH V4] drm/drm_vblank: Change EINVAL by the correct errno

[Rodrigo Siqueira]

On 06/19, Daniel Vetter wrote:
> On Wed, Jun 19, 2019 at 09:48:56AM +0200, Daniel Vetter wrote:
> > On Tue, Jun 18, 2019 at 11:07:50PM -0300, Rodrigo Siqueira wrote:
> > > For historical reason, the function drm_wait_vblank_ioctl always return
> > > -EINVAL if something gets wrong. This scenario limits the flexibility
> > > for the userspace make detailed verification of the problem and take
> > > some action. In particular, the validation of “if (!dev->irq_enabled)”
> > > in the drm_wait_vblank_ioctl is responsible for checking if the driver
> > > support vblank or not. If the driver does not support VBlank, the
> > > function drm_wait_vblank_ioctl returns EINVAL which does not represent
> > > the real issue; this patch changes this behavior by return EOPNOTSUPP.
> > > Additionally, some operations are unsupported by this function, and
> > > returns EINVAL; this patch also changes the return value to EOPNOTSUPP
> > > in this case. Lastly, the function drm_wait_vblank_ioctl is invoked by
> > > libdrm, which is used by many compositors; because of this, it is
> > > important to check if this change breaks any compositor. In this sense,
> > > the following projects were examined:
> > > 
> > > * Drm-hwcomposer
> > > * Kwin
> > > * Sway
> > > * Wlroots
> > > * Wayland-core
> > > * Weston
> > > * Xorg (67 different drivers)
> > > 
> > > For each repository the verification happened in three steps:
> > > 
> > > * Update the main branch
> > > * Look for any occurrence "drmWaitVBlank" with the command:
> > >   git grep -n "drmWaitVBlank"
> > > * Look in the git history of the project with the command:
> > >   git log -SdrmWaitVBlank
> > > 
> > > Finally, none of the above projects validate the use of EINVAL which
> > > make safe, at least for these projects, to change the return values.
> > > 
> > > Change since V3:
> > >  - Return EINVAL for _DRM_VBLANK_SIGNAL (Daniel)
> > > 
> > > Change since V2:
> > >  Daniel Vetter and Chris Wilson
> > >  - Replace ENOTTY by EOPNOTSUPP
> > >  - Return EINVAL if the parameters are wrong
> > > 
> > 
> > Reviewed-by: Daniel Vetter 
> > 
> > Apologies for the confusion on the last time around. btw if someone tells
> > you "r-b (or a-b) with these changes", then just apply the r-b/a-b tag
> > next time around. Otherwise people will re-review the same thing over and
> > over again.
> 
> btw when resending patches it's good practice to add anyone who commented
> on it (or who commented on the igt test for the same patch and other way
> round) onto the explicit Cc: list of the patch. That way it's easier for
> them to follow the patch evolution and do a quick r-b once they're happy.

Thanks for these valuable tips.
Do you think that is a good idea to resend this patch CC's everybody? Or
is it ok if I just apply it?
 
> If you don't do that then much bigger chances your patch gets ignored.
> -Daniel
> > 
> > > Signed-off-by: Rodrigo Siqueira 
> > > ---
> > >  drivers/gpu/drm/drm_vblank.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > > index 603ab105125d..bed233361614 100644
> > > --- a/drivers/gpu/drm/drm_vblank.c
> > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > @@ -1582,7 +1582,7 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, 
> > > void *data,
> > >   unsigned int flags, pipe, high_pipe;
> > >  
> > >   if (!dev->irq_enabled)
> > > - return -EINVAL;
> > > + return -EOPNOTSUPP;
> > >  
> > >   if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
> > >   return -EINVAL;
> > > -- 
> > > 2.21.0
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Rodrigo Siqueira
https://siqueira.tech


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

Re: [PATCH 00/10] drm/vkms: rework crc worker

[Rodrigo Siqueira]

On 06/19, Daniel Vetter wrote:
> On Tue, Jun 18, 2019 at 11:54 PM Rodrigo Siqueira
>  wrote:
> >
> > On Tue, Jun 18, 2019 at 5:56 AM Daniel Vetter  wrote:
> > >
> > > On Mon, Jun 17, 2019 at 11:49:04PM -0300, Rodrigo Siqueira wrote:
> > > > On 06/12, Daniel Vetter wrote:
> > > > > On Wed, Jun 12, 2019 at 10:28:41AM -0300, Rodrigo Siqueira wrote:
> > > > > > Hi Daniel,
> > > > > >
> > > > > > First of all, thank you very much for your patchset.
> > > > > >
> > > > > > I tried to make a detailed review of your series, and you can see my
> > > > > > comments in each patch. You’ll notice that I asked many things 
> > > > > > related
> > > > > > to the DRM subsystem with the hope that I could learn a little bit
> > > > > > more about DRM from your comments.
> > > > > >
> > > > > > Before you go through the review, I would like to start a discussion
> > > > > > about the vkms race conditions. First, I have to admit that I did 
> > > > > > not
> > > > > > understand the race conditions that you described before because I
> > > > > > couldn’t reproduce them. Now, I'm suspecting that I could not
> > > > > > experience the problem because I'm using QEMU with KVM; with this 
> > > > > > idea
> > > > > > in mind, I suppose that we have two scenarios for using vkms in a
> > > > > > virtual machine:
> > > > > >
> > > > > > * Scenario 1: The user has hardware virtualization support; in this
> > > > > > case, it is a little bit harder to experience race conditions with
> > > > > > vkms.
> > > > > >
> > > > > > * Scenario 2: Without hardware virtualization support, it is much
> > > > > > easier to experience race conditions.
> > > > > >
> > > > > > With these two scenarios in mind, I conducted a bunch of experiments
> > > > > > for trying to shed light on this issue. I did:
> > > > > >
> > > > > > 1. Enabled lockdep
> > > > > >
> > > > > > 2. Defined two different environments for testing both using QEMU 
> > > > > > with
> > > > > > and without kvm. See below the QEMU hardware options:
> > > > > >
> > > > > > * env_kvm: -enable-kvm -daemonize -m 1G -smp cores=2,cpus=2
> > > > > > * env_no_kvm: -daemonize -m 2G -smp cores=4,cpus=4
> > > > > >
> > > > > > 3. My test protocol: I) turn on the vm, II) clean /proc/lock_stat,
> > > > > > III) execute kms_cursor_crc, III) save the lock_stat file, and IV)
> > > > > > turn off the vm.
> > > > > >
> > > > > > 4. From the lockdep_stat, I just highlighted the row related to vkms
> > > > > > and the columns holdtime-total and holdtime-avg
> > > > > >
> > > > > > I would like to highlight that the following data does not have any
> > > > > > statistical approach, and its intention is solely to assist our
> > > > > > discussion. See below the summary of the collected data:
> > > > > >
> > > > > > Summary of the experiment results:
> > > > > >
> > > > > > ++++
> > > > > > || env_kvm|   env_no_kvm   |
> > > > > > ++++
> > > > > > | Test   | Before | After | Before | After |
> > > > > > +++---++---+
> > > > > > | kms_cursor_crc |   S1   |   S2  |   M1   |   M2  |
> > > > > > +++---++---+
> > > > > >
> > > > > > * Before: before apply this patchset
> > > > > > * After: after apply this patchset
> > > > > >
> > > > > > -+--+---
> > > > > > S1: Without this patchset and with kvm   | holdtime-total | 
> > > > > > holdtime-avg
> > > > > > -++-
> > > > > > &(_out->lock)->rlock:   |  21983.52  |  6.21
> > > > > > (work_completion)(_state->crc_wo:   |  20.47 |  20.47
> > > > > > (wq_completion)vkms_crc_workq:   |  3999507.87|  3352.48
> > > > > > &(_out->state_lock)->rlock: |  378.47|  0.30
> > > > > > (work_completion)(_state->crc_#2:   |  3999066.30|  2848.34
> > > > > > -++-
> > > > > > S2: With this patchset and with kvm  ||
> > > > > > -++-
> > > > > > &(_out->lock)->rlock:   |  23262.83  |  6.34
> > > > > > (work_completion)(_state->crc_wo:   |  8.98  |  8.98
> > > > > > &(_out->crc_lock)->rlock:   |  307.28|  0.32
> > > > > > (wq_completion)vkms_crc_workq:   |  6567727.05|  
> > > > > > 12345.35
> > > > > > (work_completion)(_state->crc_#2:   |  6567135.15|  4488.81
> > > > > > -++-
> > > > > > M1: Without this patchset and without kvm||
> > > > > > -++-
> > > > > > &(_out->state_lock)->rlock: |  4994.72   |  1.61
> > > > > > 

[Bug 110795] Unable to install on latest Ubuntu (19.04)

[bugzilla-daemon]

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

--- Comment #27 from Felipe Marschall  ---
I just saw Andrew's script.. much easier!

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

[Bug 110795] Unable to install on latest Ubuntu (19.04)

[bugzilla-daemon]

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

Felipe Marschall  changed:

   What|Removed |Added

 Resolution|INVALID |WORKSFORME
 Status|RESOLVED|VERIFIED

--- Comment #26 from Felipe Marschall  ---
(In reply to Rolf from comment #3)
> If anyone else finds themselves in this fix by installing this driver, the
> only way to get apt-get working again is to forcefully remove all of the
> files using:
> 
> sudo dpkg --force-all -P [package name]
> 
> Here is the list of miss-installed packages:
> 
> 
> The following packages have unmet dependencies:
>  amdgpu-dkms : Depends: amdgpu-core but it is not going to be installed
>  amdgpu-lib : Depends: amdgpu-core (= 19.10-785425) but it is not going to
> be installed
>  ...

It happened to me now...
I managed to fix it by booting into recovery, enabling network and then
running:

dpkg --remove --force-remove-reinstreq amdgpu amdgpu-core amdgpu-dkms
amdgpu-lib glamor-amdgpu gst-omx-amdgpu libdrm-amdgpu-amdgpu1
libdrm-amdgpu-common libdrm2-amdgpu  libegl1-amdgpu-mesa
libegl1-amdgpu-mesa-drivers libgbm1-amdgpu libgl1-amdgpu-mesa-dri
libgl1-amdgpu-mesa-glx libglapi-amdgpu-mesa  libgles1-amdgpu-mesa
libgles2-amdgpu-mesa libllvm7.1-amdgpu libomxil-bellagio-bin libomxil-bellagio0
libosmesa6-amdgpu  libwayland-amdgpu-client0 libwayland-amdgpu-egl1
libwayland-amdgpu-server0 libxatracker2-amdgpu mesa-amdgpu-omx-drivers 
mesa-amdgpu-va-drivers mesa-amdgpu-vdpau-drivers
xserver-xorg-amdgpu-video-amdgpu

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

[PATCH V3 5/5] drm/vkms: Add support for writeback

[Rodrigo Siqueira]

This patch implements the necessary functions to add writeback support
for vkms. This feature is useful for testing compositors if you don't
have hardware with writeback support.

Change in V3 (Daniel):
- If writeback is enabled, compose everything into the writeback buffer
instead of CRC private buffer.
- Guarantees that the CRC will match exactly what we have in the
writeback buffer.

Change in V2:
- Rework signal completion (Brian)
- Integrates writeback with active_planes (Daniel)
- Compose cursor (Daniel)

Signed-off-by: Rodrigo Siqueira 
---
 drivers/gpu/drm/vkms/Makefile |   9 +-
 drivers/gpu/drm/vkms/vkms_composer.c  |  16 ++-
 drivers/gpu/drm/vkms/vkms_drv.c   |   4 +
 drivers/gpu/drm/vkms/vkms_drv.h   |   8 ++
 drivers/gpu/drm/vkms/vkms_output.c|  10 ++
 drivers/gpu/drm/vkms/vkms_writeback.c | 141 ++
 6 files changed, 185 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/vkms/vkms_writeback.c

diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
index 0b767d7efa24..333d3cead0e3 100644
--- a/drivers/gpu/drm/vkms/Makefile
+++ b/drivers/gpu/drm/vkms/Makefile
@@ -1,4 +1,11 @@
 # SPDX-License-Identifier: GPL-2.0-only
-vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o 
vkms_composer.o
+vkms-y := \
+   vkms_drv.o \
+   vkms_plane.o \
+   vkms_output.o \
+   vkms_crtc.o \
+   vkms_gem.o \
+   vkms_composer.o \
+   vkms_writeback.o
 
 obj-$(CONFIG_DRM_VKMS) += vkms.o
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c 
b/drivers/gpu/drm/vkms/vkms_composer.c
index 8126aa0f968f..2317803e7320 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -157,16 +157,17 @@ void vkms_composer_worker(struct work_struct *work)
struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
struct vkms_composer *primary_composer = NULL;
struct vkms_composer *cursor_composer = NULL;
+   bool crc_pending, wb_pending;
void *vaddr_out = NULL;
u32 crc32 = 0;
u64 frame_start, frame_end;
-   bool crc_pending;
int ret;
 
spin_lock_irq(>composer_lock);
frame_start = crtc_state->frame_start;
frame_end = crtc_state->frame_end;
crc_pending = crtc_state->crc_pending;
+   wb_pending = crtc_state->wb_pending;
crtc_state->frame_start = 0;
crtc_state->frame_end = 0;
crtc_state->crc_pending = false;
@@ -188,9 +189,12 @@ void vkms_composer_worker(struct work_struct *work)
if (!primary_composer)
return;
 
+   if (wb_pending)
+   vaddr_out = crtc_state->active_writeback;
+
ret = compose_planes(_out, primary_composer, cursor_composer);
if (ret) {
-   if (ret == -EINVAL)
+   if (ret == -EINVAL && !wb_pending)
kfree(vaddr_out);
return;
}
@@ -203,6 +207,14 @@ void vkms_composer_worker(struct work_struct *work)
while (frame_start <= frame_end)
drm_crtc_add_crc_entry(crtc, true, frame_start++, );
 
+   if (wb_pending) {
+   drm_writeback_signal_completion(>wb_connector, 0);
+   spin_lock_irq(>composer_lock);
+   crtc_state->wb_pending = false;
+   spin_unlock_irq(>composer_lock);
+   return;
+   }
+
kfree(vaddr_out);
 }
 
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index ac790b6527e4..152d7de24a76 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -30,6 +30,10 @@ bool enable_cursor;
 module_param_named(enable_cursor, enable_cursor, bool, 0444);
 MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
 
+bool enable_writeback;
+module_param_named(enable_writeback, enable_writeback, bool, 0444);
+MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector");
+
 static const struct file_operations vkms_driver_fops = {
.owner  = THIS_MODULE,
.open   = drm_open,
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index fc6cda164336..9ff2cd4ebf81 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define XRES_MIN20
@@ -19,6 +20,7 @@
 #define YRES_MAX  8192
 
 extern bool enable_cursor;
+extern bool enable_writeback;
 
 struct vkms_composer {
struct drm_framebuffer fb;
@@ -52,9 +54,11 @@ struct vkms_crtc_state {
int num_active_planes;
/* stack of active planes for crc computation, should be in z order */
struct vkms_plane_state **active_planes;
+   void *active_writeback;
 
/* below three are protected by vkms_output.composer_lock */
bool crc_pending;
+   bool wb_pending;
u64 frame_start;
u64 frame_end;
 };
@@ -63,6 +67,7 

[PATCH V3 4/5] drm/vkms: Compute CRC without change input data

[Rodrigo Siqueira]

The compute_crc() function is responsible for calculating the
framebuffer CRC value; due to the XRGB format, this function has to
ignore the alpha channel during the CRC computation. Therefore,
compute_crc() set zero to the alpha channel directly in the input
framebuffer, which is not a problem since this function receives a copy
of the original buffer. However, if we want to use this function in a
context without a buffer copy, it will change the initial value. This
patch makes compute_crc() calculate the CRC value without modifying the
input framebuffer.

Signed-off-by: Rodrigo Siqueira 
---
 drivers/gpu/drm/vkms/vkms_composer.c | 31 +---
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c 
b/drivers/gpu/drm/vkms/vkms_composer.c
index 51a270514219..8126aa0f968f 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -6,33 +6,40 @@
 #include 
 #include 
 
+static u32 get_pixel_from_buffer(int x, int y, const u8 *buffer,
+const struct vkms_composer *composer)
+{
+   int src_offset = composer->offset + (y * composer->pitch)
+ + (x * composer->cpp);
+
+   return *(u32 *)[src_offset];
+}
+
 /**
  * compute_crc - Compute CRC value on output frame
  *
- * @vaddr_out: address to final framebuffer
+ * @vaddr: address to final framebuffer
  * @composer: framebuffer's metadata
  *
  * returns CRC value computed using crc32 on the visible portion of
  * the final framebuffer at vaddr_out
  */
-static uint32_t compute_crc(void *vaddr_out, struct vkms_composer *composer)
+static uint32_t compute_crc(const u8 *vaddr,
+   const struct vkms_composer *composer)
 {
-   int i, j, src_offset;
+   int x, y;
int x_src = composer->src.x1 >> 16;
int y_src = composer->src.y1 >> 16;
int h_src = drm_rect_height(>src) >> 16;
int w_src = drm_rect_width(>src) >> 16;
-   u32 crc = 0;
+   u32 crc = 0, pixel = 0;
 
-   for (i = y_src; i < y_src + h_src; ++i) {
-   for (j = x_src; j < x_src + w_src; ++j) {
-   src_offset = composer->offset
-+ (i * composer->pitch)
-+ (j * composer->cpp);
+   for (y = y_src; y < y_src + h_src; ++y) {
+   for (x = x_src; x < x_src + w_src; ++x) {
/* XRGB format ignores Alpha channel */
-   memset(vaddr_out + src_offset + 24, 0,  8);
-   crc = crc32_le(crc, vaddr_out + src_offset,
-  sizeof(u32));
+   pixel = get_pixel_from_buffer(x, y, vaddr, composer);
+   bitmap_clear((void *), 0, 8);
+   crc = crc32_le(crc, (void *), sizeof(u32));
}
}
 
-- 
2.21.0

[PATCH V3 3/5] drm/vkms: Decouple crc operations from composer

[Rodrigo Siqueira]

In the vkms_composer.c, some of the functions related to CRC and compose
have interdependence between each other. This patch reworks some
functions inside vkms_composer to make crc and composer computation
decoupled.

This patch is preparation work for making vkms able to support new
features.

Signed-off-by: Rodrigo Siqueira 
---
 drivers/gpu/drm/vkms/vkms_composer.c | 49 
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c 
b/drivers/gpu/drm/vkms/vkms_composer.c
index eb7ea8be1f98..51a270514219 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -105,35 +105,31 @@ static void compose_cursor(struct vkms_composer 
*cursor_composer,
  primary_composer, cursor_composer);
 }
 
-static uint32_t _vkms_get_crc(struct vkms_composer *primary_composer,
- struct vkms_composer *cursor_composer)
+static int compose_planes(void **vaddr_out,
+ struct vkms_composer *primary_composer,
+ struct vkms_composer *cursor_composer)
 {
struct drm_framebuffer *fb = _composer->fb;
struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0);
struct vkms_gem_object *vkms_obj = drm_gem_to_vkms_gem(gem_obj);
-   void *vaddr_out = kzalloc(vkms_obj->gem.size, GFP_KERNEL);
-   u32 crc = 0;
 
-   if (!vaddr_out) {
-   DRM_ERROR("Failed to allocate memory for output frame.");
-   return 0;
+   if (!*vaddr_out) {
+   *vaddr_out = kzalloc(vkms_obj->gem.size, GFP_KERNEL);
+   if (!*vaddr_out) {
+   DRM_ERROR("Cannot allocate memory for output frame.");
+   return -ENOMEM;
+   }
}
 
-   if (WARN_ON(!vkms_obj->vaddr)) {
-   kfree(vaddr_out);
-   return crc;
-   }
+   if (WARN_ON(!vkms_obj->vaddr))
+   return -EINVAL;
 
-   memcpy(vaddr_out, vkms_obj->vaddr, vkms_obj->gem.size);
+   memcpy(*vaddr_out, vkms_obj->vaddr, vkms_obj->gem.size);
 
if (cursor_composer)
-   compose_cursor(cursor_composer, primary_composer, vaddr_out);
+   compose_cursor(cursor_composer, primary_composer, *vaddr_out);
 
-   crc = compute_crc(vaddr_out, primary_composer);
-
-   kfree(vaddr_out);
-
-   return crc;
+   return 0;
 }
 
 /**
@@ -154,9 +150,11 @@ void vkms_composer_worker(struct work_struct *work)
struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
struct vkms_composer *primary_composer = NULL;
struct vkms_composer *cursor_composer = NULL;
+   void *vaddr_out = NULL;
u32 crc32 = 0;
u64 frame_start, frame_end;
bool crc_pending;
+   int ret;
 
spin_lock_irq(>composer_lock);
frame_start = crtc_state->frame_start;
@@ -180,14 +178,25 @@ void vkms_composer_worker(struct work_struct *work)
if (crtc_state->num_active_planes == 2)
cursor_composer = crtc_state->active_planes[1]->composer;
 
-   if (primary_composer)
-   crc32 = _vkms_get_crc(primary_composer, cursor_composer);
+   if (!primary_composer)
+   return;
+
+   ret = compose_planes(_out, primary_composer, cursor_composer);
+   if (ret) {
+   if (ret == -EINVAL)
+   kfree(vaddr_out);
+   return;
+   }
+
+   crc32 = compute_crc(vaddr_out, primary_composer);
 
/*
 * The worker can fall behind the vblank hrtimer, make sure we catch up.
 */
while (frame_start <= frame_end)
drm_crtc_add_crc_entry(crtc, true, frame_start++, );
+
+   kfree(vaddr_out);
 }
 
 static const char * const pipe_crc_sources[] = {"auto"};
-- 
2.21.0

[PATCH V3 2/5] drm/vkms: Rename vkms_crc.c into vkms_composer.c

[Rodrigo Siqueira]

As a preparation work for introducing writeback to vkms, this patch
renames the file vkms_crc.c into vkms_composer.c. Accordingly, it also
adjusts the functions and data structures to match the changes.

No functional change.

Signed-off-by: Rodrigo Siqueira 
---
 drivers/gpu/drm/vkms/Makefile |  2 +-
 .../drm/vkms/{vkms_crc.c => vkms_composer.c}  | 98 ++-
 drivers/gpu/drm/vkms/vkms_crtc.c  | 30 +++---
 drivers/gpu/drm/vkms/vkms_drv.c   |  4 +-
 drivers/gpu/drm/vkms/vkms_drv.h   | 28 +++---
 drivers/gpu/drm/vkms/vkms_plane.c | 36 +++
 6 files changed, 101 insertions(+), 97 deletions(-)
 rename drivers/gpu/drm/vkms/{vkms_crc.c => vkms_composer.c} (65%)

diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
index 89f09bec7b23..0b767d7efa24 100644
--- a/drivers/gpu/drm/vkms/Makefile
+++ b/drivers/gpu/drm/vkms/Makefile
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
-vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o 
vkms_crc.o
+vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o 
vkms_composer.o
 
 obj-$(CONFIG_DRM_VKMS) += vkms.o
diff --git a/drivers/gpu/drm/vkms/vkms_crc.c 
b/drivers/gpu/drm/vkms/vkms_composer.c
similarity index 65%
rename from drivers/gpu/drm/vkms/vkms_crc.c
rename to drivers/gpu/drm/vkms/vkms_composer.c
index 30b048b67a32..eb7ea8be1f98 100644
--- a/drivers/gpu/drm/vkms/vkms_crc.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -10,25 +10,25 @@
  * compute_crc - Compute CRC value on output frame
  *
  * @vaddr_out: address to final framebuffer
- * @crc_out: framebuffer's metadata
+ * @composer: framebuffer's metadata
  *
  * returns CRC value computed using crc32 on the visible portion of
  * the final framebuffer at vaddr_out
  */
-static uint32_t compute_crc(void *vaddr_out, struct vkms_crc_data *crc_out)
+static uint32_t compute_crc(void *vaddr_out, struct vkms_composer *composer)
 {
int i, j, src_offset;
-   int x_src = crc_out->src.x1 >> 16;
-   int y_src = crc_out->src.y1 >> 16;
-   int h_src = drm_rect_height(_out->src) >> 16;
-   int w_src = drm_rect_width(_out->src) >> 16;
+   int x_src = composer->src.x1 >> 16;
+   int y_src = composer->src.y1 >> 16;
+   int h_src = drm_rect_height(>src) >> 16;
+   int w_src = drm_rect_width(>src) >> 16;
u32 crc = 0;
 
for (i = y_src; i < y_src + h_src; ++i) {
for (j = x_src; j < x_src + w_src; ++j) {
-   src_offset = crc_out->offset
-+ (i * crc_out->pitch)
-+ (j * crc_out->cpp);
+   src_offset = composer->offset
++ (i * composer->pitch)
++ (j * composer->cpp);
/* XRGB format ignores Alpha channel */
memset(vaddr_out + src_offset + 24, 0,  8);
crc = crc32_le(crc, vaddr_out + src_offset,
@@ -43,8 +43,8 @@ static uint32_t compute_crc(void *vaddr_out, struct 
vkms_crc_data *crc_out)
  * blend - belnd value at vaddr_src with value at vaddr_dst
  * @vaddr_dst: destination address
  * @vaddr_src: source address
- * @crc_dst: destination framebuffer's metadata
- * @crc_src: source framebuffer's metadata
+ * @dest_composer: destination framebuffer's metadata
+ * @src_composer: source framebuffer's metadata
  *
  * Blend value at vaddr_src with value at vaddr_dst.
  * Currently, this function write value at vaddr_src on value
@@ -55,31 +55,31 @@ static uint32_t compute_crc(void *vaddr_out, struct 
vkms_crc_data *crc_out)
  *  instead of overwriting it.
  */
 static void blend(void *vaddr_dst, void *vaddr_src,
- struct vkms_crc_data *crc_dst,
- struct vkms_crc_data *crc_src)
+ struct vkms_composer *dest_composer,
+ struct vkms_composer *src_composer)
 {
int i, j, j_dst, i_dst;
int offset_src, offset_dst;
 
-   int x_src = crc_src->src.x1 >> 16;
-   int y_src = crc_src->src.y1 >> 16;
+   int x_src = src_composer->src.x1 >> 16;
+   int y_src = src_composer->src.y1 >> 16;
 
-   int x_dst = crc_src->dst.x1;
-   int y_dst = crc_src->dst.y1;
-   int h_dst = drm_rect_height(_src->dst);
-   int w_dst = drm_rect_width(_src->dst);
+   int x_dst = src_composer->dst.x1;
+   int y_dst = src_composer->dst.y1;
+   int h_dst = drm_rect_height(_composer->dst);
+   int w_dst = drm_rect_width(_composer->dst);
 
int y_limit = y_src + h_dst;
int x_limit = x_src + w_dst;
 
for (i = y_src, i_dst = y_dst; i < y_limit; ++i) {
for (j = x_src, j_dst = x_dst; j < x_limit; ++j) {
-   offset_dst = crc_dst->offset
-+ (i_dst * crc_dst->pitch)
-+ (j_dst++ * 

[PATCH V3 1/5] drm/vkms: Avoid assigning 0 for possible_crtc

[Rodrigo Siqueira]

When vkms invoke drm_universal_plane_init(), it sets 0 for
possible_crtcs parameter which means that planes can't be attached to
any CRTC. It currently works due to some safeguard in the drm_crtc file;
however, it is possible to identify the problem by trying to append a
second connector. This patch fixes this issue by modifying
vkms_plane_init() to accept an index parameter which makes the code a
little bit more flexible and avoid set zero to possible_crtcs.

Signed-off-by: Rodrigo Siqueira 
---
 drivers/gpu/drm/vkms/vkms_drv.c| 2 +-
 drivers/gpu/drm/vkms/vkms_drv.h| 4 ++--
 drivers/gpu/drm/vkms/vkms_output.c | 6 +++---
 drivers/gpu/drm/vkms/vkms_plane.c  | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index cc53ef88a331..966b3d653189 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -127,7 +127,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev)
dev->mode_config.preferred_depth = 24;
dev->mode_config.helper_private = _mode_config_helpers;
 
-   return vkms_output_init(vkmsdev);
+   return vkms_output_init(vkmsdev, 0);
 }
 
 static int __init vkms_init(void)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 12b4db7ac641..e2d1aa089dec 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -115,10 +115,10 @@ bool vkms_get_vblank_timestamp(struct drm_device *dev, 
unsigned int pipe,
   int *max_error, ktime_t *vblank_time,
   bool in_vblank_irq);
 
-int vkms_output_init(struct vkms_device *vkmsdev);
+int vkms_output_init(struct vkms_device *vkmsdev, int index);
 
 struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
- enum drm_plane_type type);
+ enum drm_plane_type type, int index);
 
 /* Gem stuff */
 struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
diff --git a/drivers/gpu/drm/vkms/vkms_output.c 
b/drivers/gpu/drm/vkms/vkms_output.c
index 56fb5c2a2315..fb1941a6522c 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -35,7 +35,7 @@ static const struct drm_connector_helper_funcs 
vkms_conn_helper_funcs = {
.get_modes= vkms_conn_get_modes,
 };
 
-int vkms_output_init(struct vkms_device *vkmsdev)
+int vkms_output_init(struct vkms_device *vkmsdev, int index)
 {
struct vkms_output *output = >output;
struct drm_device *dev = >drm;
@@ -45,12 +45,12 @@ int vkms_output_init(struct vkms_device *vkmsdev)
struct drm_plane *primary, *cursor = NULL;
int ret;
 
-   primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY);
+   primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index);
if (IS_ERR(primary))
return PTR_ERR(primary);
 
if (enable_cursor) {
-   cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR);
+   cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index);
if (IS_ERR(cursor)) {
ret = PTR_ERR(cursor);
goto err_cursor;
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c 
b/drivers/gpu/drm/vkms/vkms_plane.c
index 0fceb6258422..18c630cfc485 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -176,7 +176,7 @@ static const struct drm_plane_helper_funcs 
vkms_primary_helper_funcs = {
 };
 
 struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
- enum drm_plane_type type)
+ enum drm_plane_type type, int index)
 {
struct drm_device *dev = >drm;
const struct drm_plane_helper_funcs *funcs;
@@ -198,7 +198,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device 
*vkmsdev,
funcs = _primary_helper_funcs;
}
 
-   ret = drm_universal_plane_init(dev, plane, 0,
+   ret = drm_universal_plane_init(dev, plane, 1 << index,
   _plane_funcs,
   formats, nformats,
   NULL, type, NULL);
-- 
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH V3 0/5] drm/vkms: Introduces writeback support

[Rodrigo Siqueira]

This is the V3 version of a series that introduces the writeback support
to vkms. As a result of the previous review, this patchset can be seen
in three parts: make vkms able to support multiple connector, pre-work
for vkms, and the vkms implementation. Follows the details:

* First part: The first patch of this series is a fix that enables vkms to
accept new connectors, such as writeback connector.

* Second part: The second part of this patchset starts on patch 02 and
finish on patch 04; basically it is a pre-work that aims to make vkms
composer operations a little bit more generic. These patches update the
CRC files and function to make it work as a composer; it also
centralizes the vkms framebuffer operations. Additionally, these changes
enable the composer to use the writeback framebuffer instead of creating
a copy.

* Third part: The final patch enables the support for writeback in vkms.

With this patchset, vkms can successfully pass all the kms_writeback
tests from IGT.

Note: Most of the changes in the V3 was suggested by Daniel Vetter as
can be seen at the link
https://patchwork.freedesktop.org/patch/311844/?series=61738=2

Note: This patchset depends on Daniel's rework of CRC, see it at
https://patchwork.freedesktop.org/series/61737/

Rodrigo Siqueira (5):
  drm/vkms: Avoid assigning 0 for possible_crtc
  drm/vkms: Rename vkms_crc.c into vkms_composer.c
  drm/vkms: Decouple crc operations from composer
  drm/vkms: Compute CRC without change input data
  drm/vkms: Add support for writeback

 drivers/gpu/drm/vkms/Makefile |   9 +-
 .../drm/vkms/{vkms_crc.c => vkms_composer.c}  | 174 ++
 drivers/gpu/drm/vkms/vkms_crtc.c  |  30 +--
 drivers/gpu/drm/vkms/vkms_drv.c   |  10 +-
 drivers/gpu/drm/vkms/vkms_drv.h   |  40 ++--
 drivers/gpu/drm/vkms/vkms_output.c|  16 +-
 drivers/gpu/drm/vkms/vkms_plane.c |  40 ++--
 drivers/gpu/drm/vkms/vkms_writeback.c | 141 ++
 8 files changed, 331 insertions(+), 129 deletions(-)
 rename drivers/gpu/drm/vkms/{vkms_crc.c => vkms_composer.c} (51%)
 create mode 100644 drivers/gpu/drm/vkms/vkms_writeback.c

-- 
2.21.0


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

Re: [RFC] Exposing plane type mask and handling 'all' planes

[Matt Roper]

On Wed, Jun 19, 2019 at 09:24:56PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 19, 2019 at 06:49:11PM +0100, Emil Velikov wrote:
> > On Wed, 19 Jun 2019 at 17:33, Ville Syrjälä
> >  wrote:
> > >
> > > On Wed, Jun 19, 2019 at 05:03:53PM +0100, Emil Velikov wrote:
> > > > Hi all,
> > > >
> > > > Recently I have been looking at i915 and its rather interesting planes.
> > > >
> > > > In particular newer hardware is capable of using 3 universal planes and
> > > > a separate cursor-only plane. At the same time only 1 top-most plane can
> > > > be enabled - lets calls those plane3 or cursor.
> > > >
> > > > Hence currently the hardware has an extra plane which we do not use.
> > >
> > > Only skl (and derivatives) have that. More modern platforms went back to
> > > the dedicated cursor plane. So IMO no point in exposing this mess at
> > > all.
> > >
> > I suspect you're talking about Ice Lake, correct?
> 
> And glk. The other relevant platform joined the fight club so we don't
> talk about it.
> 
> > 
> > > >
> > > > The current DRM design does not allow us to fully utilise the HW and I
> > > > would love to address that. Here are three approaches that come to mind:
> > > >
> > > > 1) Extend the DRM plane UAPI to:
> > > >  - expose a mask of supported types, and
> > > >  - allow userspace to select the type
> > > >
> > > > 2) Keep the exposed planes as-is and let the driver orchestrate which
> > > >one gets used.
> > > >  - flip between cursor/plane3 based on the use-case/API used, or
> > > >  - opt for plane3/cursor for atomic and legacy modeset code path, or
> > > >  - other
> > > >
> > > > 3) Expose plane3 and cursor, whereby using one of them "marks" the other
> > > >as used.
> > > >  - is this allowed by the modeset semantics/policy?
> > > >  - does existing user-space have fallback path?
> > > >
> > > >
> > > > Personally, I would love existing user-space to just work without
> > > > modification. At the same time opting for 2 this will cause a serious
> > > > amount of complication, and in case of 3 the whole thing will be very
> > > > fragile. So my current inclination is towards 1.
> > > >
> > > > Open questions:
> > > >  - Do we know of other hardware with this or related design which does
> > > > not fit the current DRM design?
> > > >  - Vendor KMS specific UAPI, is frowned upon. So if we are to extend the
> > > > UAPI, does the current approach sound useful for other HW?
> > > >  - Does this approach sound reasonable, can other share their view on a
> > > > better approach, that achieves the goal?
> > > >
> > Speaking hypothetically:
> > 
> > If the mutually exclusive design was very common, which of the
> > proposed solutions you think will be the best fit?
> > May I ask you for a mini-brain dump how you foresee that being implemented 
> > :-)
> 
> I would go for option 2. Option 1 is pretty much the same thing with a
> slight hint for userspace that maybe the plane can do a bit more than
> just "cursory" things. The problem is that we have no actual definition
> of what these "cursory" things are. It's totally hardware/driver
> specific. Eg. i915 already allows you to do various things with the
> cursor plane that may not work on other hardware. Also I'm sure there's
> plenty of hardware out there with no special purpose cursor planes, so
> those drivers will probably just designate one regular plane as
> "the cursor" anyway.
> 
> So I think we've been pretty much doing option 2 all along. So if some
> userspace wanted to it could just ignore the cursor type hint and try
> to use the plane in any way it sees fit. The usual rule of "try
> CHECK_ONLY and see it you get an error" applies here as well.
> 
> If we did want to go for option 1 then I think it should take the form
> of some better defined plane capabilities. The problem is that there
> are alwayas tons of hardware specific exceptions so you wouldn't be
> able to blindly trust the caps anyway.

PLANE_CURSOR is basically just an indication that that specific plane is
the one that's also hooked up to the legacy cursor ioctls; like Ville
says, it shouldn't directly indicate that the plane is less
feature-capable than other planes.  You can either detect the true
capabilities of the cursor plane by checking for the presence/absence of
other plane properties and/or experimenting with atomic TEST_ONLY
commits to see what's really possible.

The ideal solution for Intel gen9 hardware would have been to just never
have the driver advertise or program the dedicated hardware cursor at
all, but to instead expose the top-most universal plane to userspace,
describe it as PLANE_CURSOR, and route the legacy cursor ioctl's to that
plane instead.  That would allow legacy cursor behavior to work as
usual, but would also allow atomic userspace to use the plane in a more
full-featured manner.  I wrote patches to do exactly this a couple years
ago, but sadly we discovered that the universal planes on gen9 have a
slight alpha blending defect that the 

Re: [PATCH v5 01/18] kunit: test: add KUnit test runner core

[Brendan Higgins]

On Tue, Jun 25, 2019 at 3:33 PM Luis Chamberlain  wrote:
>
> On Mon, Jun 17, 2019 at 01:25:56AM -0700, Brendan Higgins wrote:
> > +/**
> > + * module_test() - used to register a  kunit_module with KUnit.
> > + * @module: a statically allocated  kunit_module.
> > + *
> > + * Registers @module with the test framework. See  kunit_module for 
> > more
> > + * information.
> > + */
> > +#define module_test(module) \
> > + static int module_kunit_init##module(void) \
> > + { \
> > + return kunit_run_tests(); \
> > + } \
> > + late_initcall(module_kunit_init##module)
>
> Becuase late_initcall() is used, if these modules are built-in, this
> would preclude the ability to test things prior to this part of the
> kernel under UML or whatever architecture runs the tests. So, this
> limits the scope of testing. Small detail but the scope whould be
> documented.

You aren't the first person to complain about this (and I am not sure
it is the first time you have complained about it). Anyway, I have
some follow on patches that will improve the late_initcall thing, and
people seemed okay with discussing the follow on patches as part of a
subsequent patchset after this gets merged.

I will nevertheless document the restriction until then.

> > +static void kunit_print_tap_version(void)
> > +{
> > + if (!kunit_has_printed_tap_version) {
> > + kunit_printk_emit(LOGLEVEL_INFO, "TAP version 14\n");
>
> What is this TAP thing? Why should we care what version it is on?
> Why are we printing this?

It's part of the TAP specification[1]. Greg and Frank asked me to make
the intermediate format conform to TAP. Seems like something else I
should probable document...

> > + kunit_has_printed_tap_version = true;
> > + }
> > +}
> > +
> > +static size_t kunit_test_cases_len(struct kunit_case *test_cases)
> > +{
> > + struct kunit_case *test_case;
> > + size_t len = 0;
> > +
> > + for (test_case = test_cases; test_case->run_case; test_case++)
>
> If we make the last test case NULL, we'd just check for test_case here,
> and save ourselves an extra few bytes per test module. Any reason why
> the last test case cannot be NULL?

Is there anyway to make that work with a statically defined array?

Basically, I want to be able to do something like:

static struct kunit_case example_test_cases[] = {
KUNIT_CASE(example_simple_test),
KUNIT_CASE(example_mock_test),
{}
};

FYI,
#define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name }

In order to do what you are proposing, I think I need an array of
pointers to test cases, which is not ideal.

> > +void kunit_init_test(struct kunit *test, const char *name)
> > +{
> > + spin_lock_init(>lock);
> > + test->name = name;
> > + test->success = true;
> > +}
> > +
> > +/*
> > + * Performs all logic to run a test case.
> > + */
> > +static void kunit_run_case(struct kunit_module *module,
> > +struct kunit_case *test_case)
> > +{
> > + struct kunit test;
> > + int ret = 0;
> > +
> > + kunit_init_test(, test_case->name);
> > +
> > + if (module->init) {
> > + ret = module->init();
>
> I believe if we used struct kunit_module *kmodule it would be much
> clearer who's init this is.

That's reasonable. I will fix in next revision.

Cheers!

[1] 
https://github.com/TestAnything/Specification/blob/tap-14-specification/specification.md
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/7] drm/fb-helper: use gem_bo.resv, not dma_buf.resv in prepare_fb

[Eric Anholt]

Daniel Vetter  writes:

> With
>
> commit 5f6ed9879a414636405a2bd77f122881695959e4
> Author: Daniel Vetter 
> Date:   Fri Jun 14 22:35:57 2019 +0200
>
> drm/prime: automatically set gem_obj->resv on import
>
> we consistently set drm_gem_bo.resv for imported buffers. Which means
> we don't need to check to check the dma-buf in the prepare_fb helper,
> but can generalize them so they're also useful for display+render
> drivers which use gem_bo.resv to track their own rendering for their
> own scanout buffers.

1-3 are:

Reviewed-by: Eric Anholt 


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

Re: [PATCH v5 13/18] kunit: tool: add Python wrappers for running KUnit tests

[Luis Chamberlain]

On Mon, Jun 17, 2019 at 01:26:08AM -0700, Brendan Higgins wrote:
>  create mode 100644 
> tools/testing/kunit/test_data/test_is_test_passed-all_passed.log
>  create mode 100644 
> tools/testing/kunit/test_data/test_is_test_passed-crash.log
>  create mode 100644 
> tools/testing/kunit/test_data/test_is_test_passed-failure.log
>  create mode 100644 
> tools/testing/kunit/test_data/test_is_test_passed-no_tests_run.log
>  create mode 100644 
> tools/testing/kunit/test_data/test_output_isolated_correctly.log
>  create mode 100644 tools/testing/kunit/test_data/test_read_from_file.kconfig

Why are these being added upstream? The commit log does not explain
this.

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

Re: Sleeping while atomic in virtio-gpu edid handling

[Gerd Hoffmann]

On Tue, Jun 25, 2019 at 05:15:41PM +0200, Cornelia Huck wrote:
> Hi Gerd,
> 
> flipping the virtio-gpu edid support in QEMU to default enabled exposed
> the following backtrace in my guest (from my bisect run down to the
> initial commit in Linux):
> 
> [drm] virgl 3d acceleration not supported by guest
> [drm] EDID support available.
> [drm] number of scanouts: 1
> [drm] number of cap sets: 0
> BUG: sleeping function called from invalid context at mm/slab.h:421
> in_atomic(): 1, irqs_disabled(): 0, pid: 7, name: kworker/0:1
> 3 locks held by kworker/0:1/7:
>  #0: (ptrval) ((wq_completion)"events"){+.+.}, at: 
> process_one_work+0x1c8/0x618
>  #1: (ptrval) ((work_completion)(>dequeue_work)){+.+.}, at: 
> process_one_work+0x1c8/0x618
>  #2: (ptrval) (&(>display_info_lock)->rlock){+.+.}, at: 
> virtio_gpu_cmd_get_edid_cb+0x6e/0xc0
> CPU: 0 PID: 7 Comm: kworker/0:1 Tainted: GW 4.20.0-rc1+ #142
> Hardware name: QEMU 2964 QEMU (KVM/Linux)
> Workqueue: events virtio_gpu_dequeue_ctrl_func
> Call Trace:
> ([<00112a2c>] show_stack+0x54/0xd0)
>  [<00ba7bd0>] dump_stack+0x90/0xc8 
>  [<001a8cf8>] ___might_sleep+0x240/0x258 
>  [<003560e6>] __kmalloc_node+0x2de/0x478 
>  [<007e0f64>] drm_property_create_blob.part.0+0x3c/0x138 
>  [<007e1bfe>] drm_property_replace_global_blob+0xb6/0x118 
>  [<007dedac>] drm_connector_update_edid_property+0x8c/0xb0 
>  [<007febe8>] virtio_gpu_cmd_get_edid_cb+0x88/0xc0 
>  [<007ff03a>] virtio_gpu_dequeue_ctrl_func+0x142/0x200 
>  [<0018fdbc>] process_one_work+0x284/0x618 
>  [<0019019a>] worker_thread+0x4a/0x3f0 
>  [<00197c92>] kthread+0x152/0x170 
>  [<00bcac76>] kernel_thread_starter+0x6/0xc 
>  [<00bcac70>] kernel_thread_starter+0x0/0xc 
> 3 locks held by kworker/0:1/7:
>  #0: (ptrval) ((wq_completion)"events"){+.+.}, at: 
> process_one_work+0x1c8/0x618
>  #1: (ptrval) ((work_completion)(>dequeue_work)){+.+.}, at: 
> process_one_work+0x1c8/0x618
>  #2: (ptrval) (&(>display_info_lock)->rlock){+.+.}, at: 
> virtio_gpu_cmd_get_edid_cb+0x6e/0xc0
> virtio_gpu virtio5: fb1: virtiodrmfb frame buffer device
> [drm] Initialized virtio_gpu 0.1.0 0 for virtio5 on minor 1
> 
> This is an s390x guest, run via tcg; the stack trace is triggered both
> for virtio-gpu-ccw and virtio-gpu-pci devices, so it's probably
> something generic. The device seems to initialize fine, but I have not
> tried to actually use it (I simply keep a virtio-gpu device in my QEMU
> command line for sanity checking.)
> 
> As said, I bisected this down to the initial commit
> 
> commit b4b01b4995fb15b55a2d067eb405917f5ab32709 (refs/bisect/bad)
> Author: Gerd Hoffmann 
> Date:   Tue Oct 30 07:32:06 2018 +0100
> 
> drm/virtio: add edid support
> 
> linux guest driver implementation of the VIRTIO_GPU_F_EDID feature.
> 
> Signed-off-by: Gerd Hoffmann 
> Acked-by: Daniel Vetter 
> Link: 
> http://patchwork.freedesktop.org/patch/msgid/20181030063206.19528-3-kra...@redhat.com
> 
> so it seems to have always been present, but I just noticed it now that
> the default for edid in QEMU has changed.
> 
> I have not tried it with a non-s390x guest, though.

https://patchwork.freedesktop.org/patch/296386/

(patch braucht noch ein review oder ack)

cheers,
  Gerd

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

[PATCH] drm/connector: Allow max possible encoders to attach to a connector

[Dhinakaran Pandiyan]

Currently we restrict the number of encoders that can be linked to
a connector to 3, increase it to match the maximum number of encoders
that can be initialized - 32. The current limitation looks artificial.
Increasing the limit to 32 does however increases the size of the static
u32 array keeping track of the encoder IDs.

Cc: José Roberto de Souza 
Cc: Ville Syrjälä 
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Dhinakaran Pandiyan 
---
 include/drm/drm_connector.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ca745d9feaf5..91455b4a9360 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1278,7 +1278,7 @@ struct drm_connector {
/** @override_edid: has the EDID been overwritten through debugfs for 
testing? */
bool override_edid;
 
-#define DRM_CONNECTOR_MAX_ENCODER 3
+#define DRM_CONNECTOR_MAX_ENCODER 32
/**
 * @encoder_ids: Valid encoders for this connector. Please only use
 * drm_connector_for_each_possible_encoder() to enumerate these.
-- 
2.17.1

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

Re: [PATCH v5 07/18] kunit: test: add initial tests

[Luis Chamberlain]

On Mon, Jun 17, 2019 at 01:26:02AM -0700, Brendan Higgins wrote:
> diff --git a/kunit/example-test.c b/kunit/example-test.c
> new file mode 100644
> index 0..f44b8ece488bb
> --- /dev/null
> +++ b/kunit/example-test.c

<-- snip -->

> +/*
> + * This defines a suite or grouping of tests.
> + *
> + * Test cases are defined as belonging to the suite by adding them to
> + * `kunit_cases`.
> + *
> + * Often it is desirable to run some function which will set up things which
> + * will be used by every test; this is accomplished with an `init` function
> + * which runs before each test case is invoked. Similarly, an `exit` function
> + * may be specified which runs after every test case and can be used to for
> + * cleanup. For clarity, running tests in a test module would behave as 
> follows:
> + *

To be clear this is not the kernel module init, but rather the kunit
module init. I think using kmodule would make this clearer to a reader.

> + * module.init(test);
> + * module.test_case[0](test);
> + * module.exit(test);
> + * module.init(test);
> + * module.test_case[1](test);
> + * module.exit(test);
> + * ...;
> + */

  Luis

Re: [PATCH v2] drm: return -EFAULT if copy_to_user() fails

[Al Viro]

On Tue, Jun 18, 2019 at 01:16:29PM -0400, Sean Paul wrote:
> On Tue, Jun 18, 2019 at 04:18:43PM +0300, Dan Carpenter wrote:
> > The copy_from_user() function returns the number of bytes remaining
> > to be copied but we want to return a negative error code.  Otherwise
> > the callers treat it as a successful copy.
> > 
> > Signed-off-by: Dan Carpenter 
> 
> Thanks Dan, I've applied this to drm-misc-fixes.

FWIW, Acked-by: Al Viro 

Re: [PATCH v5 06/18] kbuild: enable building KUnit

[Luis Chamberlain]

On Tue, Jun 25, 2019 at 03:41:29PM -0700, Brendan Higgins wrote:
> On Tue, Jun 25, 2019 at 3:13 PM Luis Chamberlain  wrote:
> >
> > On Mon, Jun 17, 2019 at 01:26:01AM -0700, Brendan Higgins wrote:
> > > diff --git a/Kconfig b/Kconfig
> > > index 48a80beab6853..10428501edb78 100644
> > > --- a/Kconfig
> > > +++ b/Kconfig
> > > @@ -30,3 +30,5 @@ source "crypto/Kconfig"
> > >  source "lib/Kconfig"
> > >
> > >  source "lib/Kconfig.debug"
> > > +
> > > +source "kunit/Kconfig"
> >
> > This patch would break compilation as kunit/Kconfig is not introduced. This
> > would would also break bisectability on this commit. This change should
> > either be folded in to the next patch, or just be a separate patch after
> > the next one.
> 
> Maybe my brain isn't working right now, but I am pretty darn sure that
> I introduce kunit/Kconfig in the very first patch of this series.
> Quoting from the change summary from the first commit:

Indeed, my mistake, thanks!

  Luis

Re: [PATCH v5 01/18] kunit: test: add KUnit test runner core

[Luis Chamberlain]

On Tue, Jun 25, 2019 at 03:14:45PM -0700, Brendan Higgins wrote:
> On Tue, Jun 25, 2019 at 2:44 PM Luis Chamberlain  wrote:
> > Since its a new architecture and since you seem to imply most tests
> > don't require locking or even IRQs disabled, I think its worth to
> > consider the impact of adding such extreme locking requirements for
> > an initial ramp up.
> 
> Fair enough, I can see the point of not wanting to use irq disabled
> until we get someone complaining about it, but I think making it
> thread safe is reasonable. It means there is one less thing to confuse
> a KUnit user and the only penalty paid is some very minor performance.

One reason I'm really excited about kunit is speed... so by all means I
think we're at a good point to analyze performance optimizationsm if
they do make sense.

While on the topic of parallization, what about support for running
different test cases in parallel? Or at the very least different kunit
modules in parallel.  Few questions come up based on this prospect:

  * Why not support parallelism from the start?
  * Are you opposed to eventually having this added? For instance, there is
enough code on lib/test_kmod.c for batching tons of kthreads each
one running its own thing for testing purposes which could be used
as template.
  * If we eventually *did* support it:
- Would logs be skewed?
- Could we have a way to query: give me log for only kunit module
  named "foo"?

  Luis

Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags

[kbuild test robot]

Hi Alastair,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.2-rc6 next-20190625]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Alastair-D-Silva/Hexdump-Enhancements/20190625-224046
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

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


sparse warnings: (new ones prefixed by >>)

   sound/soc/intel/skylake/skl-debug.c:191:34: sparse: sparse: incorrect type 
in argument 1 (different address spaces) @@expected void [noderef]  
*to @@got eref]  *to @@
   sound/soc/intel/skylake/skl-debug.c:191:34: sparse:expected void 
[noderef]  *to
   sound/soc/intel/skylake/skl-debug.c:191:34: sparse:got unsigned char *
   sound/soc/intel/skylake/skl-debug.c:191:51: sparse: sparse: incorrect type 
in argument 2 (different address spaces) @@expected void const *from @@
got void [noderef]  void const *from @@
   sound/soc/intel/skylake/skl-debug.c:191:51: sparse:expected void const 
*from
   sound/soc/intel/skylake/skl-debug.c:191:51: sparse:got void [noderef] 
 *[assigned] fw_reg_addr
>> sound/soc/intel/skylake/skl-debug.c:195:35: sparse: sparse: too many 
>> arguments for function hex_dump_to_buffer
--
>> drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c:93:27: sparse: sparse: too 
>> many arguments for function hex_dump_to_buffer
--
>> sound/soc/sof/xtensa/core.c:125:35: sparse: sparse: too many arguments for 
>> function hex_dump_to_buffer

vim +195 sound/soc/intel/skylake/skl-debug.c

d14700a0 Vinod Koul  2017-06-30  170  
bdd0384a Vunny Sodhi 2017-06-30  171  static ssize_t fw_softreg_read(struct 
file *file, char __user *user_buf,
bdd0384a Vunny Sodhi 2017-06-30  172   size_t count, 
loff_t *ppos)
bdd0384a Vunny Sodhi 2017-06-30  173  {
bdd0384a Vunny Sodhi 2017-06-30  174struct skl_debug *d = 
file->private_data;
bdd0384a Vunny Sodhi 2017-06-30  175struct sst_dsp *sst = 
d->skl->skl_sst->dsp;
bdd0384a Vunny Sodhi 2017-06-30  176size_t w0_stat_sz = 
sst->addr.w0_stat_sz;
bdd0384a Vunny Sodhi 2017-06-30  177void __iomem *in_base = 
sst->mailbox.in_base;
bdd0384a Vunny Sodhi 2017-06-30  178void __iomem *fw_reg_addr;
bdd0384a Vunny Sodhi 2017-06-30  179unsigned int offset;
bdd0384a Vunny Sodhi 2017-06-30  180char *tmp;
bdd0384a Vunny Sodhi 2017-06-30  181ssize_t ret = 0;
bdd0384a Vunny Sodhi 2017-06-30  182  
bdd0384a Vunny Sodhi 2017-06-30  183tmp = kzalloc(FW_REG_BUF, GFP_KERNEL);
bdd0384a Vunny Sodhi 2017-06-30  184if (!tmp)
bdd0384a Vunny Sodhi 2017-06-30  185return -ENOMEM;
bdd0384a Vunny Sodhi 2017-06-30  186  
bdd0384a Vunny Sodhi 2017-06-30  187fw_reg_addr = in_base - w0_stat_sz;
bdd0384a Vunny Sodhi 2017-06-30  188memset(d->fw_read_buff, 0, FW_REG_BUF);
bdd0384a Vunny Sodhi 2017-06-30  189  
bdd0384a Vunny Sodhi 2017-06-30  190if (w0_stat_sz > 0)
bdd0384a Vunny Sodhi 2017-06-30 @191
__iowrite32_copy(d->fw_read_buff, fw_reg_addr, w0_stat_sz >> 2);
bdd0384a Vunny Sodhi 2017-06-30  192  
bdd0384a Vunny Sodhi 2017-06-30  193for (offset = 0; offset < FW_REG_SIZE; 
offset += 16) {
bdd0384a Vunny Sodhi 2017-06-30  194ret += snprintf(tmp + ret, 
FW_REG_BUF - ret, "%#.4x: ", offset);
bdd0384a Vunny Sodhi 2017-06-30 @195
hex_dump_to_buffer(d->fw_read_buff + offset, 16, 16, 4,
bdd0384a Vunny Sodhi 2017-06-30  196   tmp + ret, 
FW_REG_BUF - ret, 0);
bdd0384a Vunny Sodhi 2017-06-30  197ret += strlen(tmp + ret);
bdd0384a Vunny Sodhi 2017-06-30  198  
bdd0384a Vunny Sodhi 2017-06-30  199/* print newline for each 
offset */
bdd0384a Vunny Sodhi 2017-06-30  200if (FW_REG_BUF - ret > 0)
bdd0384a Vunny Sodhi 2017-06-30  201tmp[ret++] = '\n';
bdd0384a Vunny Sodhi 2017-06-30  202}
bdd0384a Vunny Sodhi 2017-06-30  203  
bdd0384a Vunny Sodhi 2017-06-30  204ret = simple_read_from_buffer(user_buf, 
count, ppos, tmp, ret);
bdd0384a Vunny Sodhi 2017-06-30  205kfree(tmp);
bdd0384a Vunny Sodhi 2017-06-30  206  
bdd0384a Vunny Sodhi 2017-06-30  207return ret;
bdd0384a Vunny Sodhi 2017-06-30  208  }
bdd0384a Vunny Sodhi 2017-06-30  209  

:: The code at line 195 was first introduced by commit
:: bdd0384a5ada8bb5745e5f29c10a5ba88827efad ASoC: Intel: Skylake: Add 
support to read firmware registers

:: TO: Vunny Sodhi 
:: CC: Mark Brown 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all 

Re: [PATCH v5 06/18] kbuild: enable building KUnit

[Brendan Higgins]

On Tue, Jun 25, 2019 at 3:13 PM Luis Chamberlain  wrote:
>
> On Mon, Jun 17, 2019 at 01:26:01AM -0700, Brendan Higgins wrote:
> > diff --git a/Kconfig b/Kconfig
> > index 48a80beab6853..10428501edb78 100644
> > --- a/Kconfig
> > +++ b/Kconfig
> > @@ -30,3 +30,5 @@ source "crypto/Kconfig"
> >  source "lib/Kconfig"
> >
> >  source "lib/Kconfig.debug"
> > +
> > +source "kunit/Kconfig"
>
> This patch would break compilation as kunit/Kconfig is not introduced. This
> would would also break bisectability on this commit. This change should
> either be folded in to the next patch, or just be a separate patch after
> the next one.

Maybe my brain isn't working right now, but I am pretty darn sure that
I introduce kunit/Kconfig in the very first patch of this series.
Quoting from the change summary from the first commit:

>  include/kunit/test.h | 161 +
>  kunit/Kconfig|  17 
>  kunit/Makefile   |   1 +
>  kunit/test.c | 210 +++
>  4 files changed, 389 insertions(+)
>  create mode 100644 include/kunit/test.h
>  create mode 100644 kunit/Kconfig

I am not crazy, right?

>  create mode 100644 kunit/Makefile
>  create mode 100644 kunit/test.c
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[pull] drm/msm: msm-next for 5.3

[Rob Clark]

Hi Dave,

This time around:

 + usual progress on cleanups
 + dsi vs EPROBE_DEFER fixes
 + msm8998 (snapdragon 835 support)
   + a540 gpu support (mesa support already landed)
   + dsi, dsi-phy support
 + mdp5 and dpu interconnect (bus/memory scaling) support
 + initial prep work for per-context pagetables (at least the parts that
   don't have external dependencies like iommu/arm-smmu)

There is one more patch for fixing DSI cmd mode panels (part of a set of
patches to get things working on nexus5), but it would be conflicty with
1cff7440a86e04a613665803b42034 in drm-next without rebasing or back-merge,
and since it doesn't conflict with anything in msm-next, I think it best
if Sean merges that through drm-mix-fixes instead.

(In other news, I've been making some progress w/ getting efifb working
properly on sdm850 laptop without horrible hacks, and drm/msm + clk stuff
not totally falling over when bootloader enables display and things are
already running when driver probes.. but not quite ready yet, hopefully
we can post some of that for 5.4.. should help for both the sdm835 and
sdm850 laptops.)

The following changes since commit a188339ca5a396acc588e5851ed7e19f66b0ebd9:

  Linux 5.2-rc1 (2019-05-19 15:47:09 -0700)

are available in the Git repository at:

  https://gitlab.freedesktop.org/drm/msm.git drm-msm-next-2019-06-25

for you to fetch changes up to 648fdc3f6475d96de287a849a31d89e79ba7669c:

  drm/msm: add dirty framebuffer helper (2019-06-25 05:29:59 -0700)


Abhinav Kumar (2):
  drm/msm/dsi: add protection against NULL dsi device
  drm/msm/dpu: add icc voting in dpu_mdss_init

Brian Masney (2):
  drm/msm: correct attempted NULL pointer dereference in put_iova
  drm/msm: add dirty framebuffer helper

Georgi Djakov (1):
  drm/msm/mdp5: Use the interconnect API

Greg Kroah-Hartman (3):
  msm: adreno: no need to check return value of debugfs_create functions
  msm: dpu1: no need to check return value of debugfs_create functions
  msm: no need to check return value of debugfs_create functions

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

Jeffrey Hugo (6):
  drm/msm/mdp5: Fix mdp5_cfg_init error return
  dt-bindings: msm/dsi: Add 10nm phy for msm8998 compatible
  drm/msm/dsi: Add support for MSM8998 10nm dsi phy
  drm/msm/dsi: Add old timings quirk for 10nm phy
  drm/msm/dsi: Add support for MSM8998 DSI controller
  drm/msm/adreno: Add A540 support

Jordan Crouse (7):
  drm/msm/adreno: Enable 64 bit mode by default on a5xx and a6xx targets
  drm/msm: Print all 64 bits of the faulting IOMMU address
  drm/msm: Pass the MMU domain index in struct msm_file_private
  drm/msm/dpu: Fix error recovery after failing to enable clocks
  drm/msm/dpu: Avoid a null de-ref while recovering from kms init fail
  drm/msm/adreno: Call pm_runtime_force_suspend() during unbind
  drm/msm/adreno: Ensure that the zap shader region is big enough

Nathan Chancellor (1):
  drm/msm/dsi: Add parentheses to quirks check in
dsi_phy_hw_v3_0_lane_settings

Nathan Huckleberry (1):
  drm/msm/dpu: Fix Wunused-const-variable

Nicholas Mc Guire (1):
  drm/msm: check for equals 0 only

Rob Clark (1):
  drm/msm/a3xx: remove TPL1 regs from snapshot

Sean Paul (23):
  drm/msm/a6xx: Avoid freeing gmu resources multiple times
  drm/msm/a6xx: Remove duplicate irq disable from remove
  drm/msm/a6xx: Check for ERR or NULL before iounmap
  drm/msm/a6xx: Remove devm calls from gmu driver
  drm/msm/a6xx: Drop the device reference in gmu
  drm/msm/a6xx: Rename a6xx_gmu_probe to a6xx_gmu_init
  drm/msm/dpu: Use provided drm_minor to initialize debugfs
  drm/msm/dpu: Remove _dpu_debugfs_init
  drm/msm/dpu: Remove bogus comment
  drm/msm/dpu: Remove call to drm_mode_set_crtcinfo
  drm/msm/dpu: Avoid calling _dpu_kms_mmu_destroy() on init failure
  drm/msm/phy/dsi_phy: Set pll to NULL in case initialization fails
  drm/msm/dsi_pll_10nm: Release clk hw on destroy and failure
  drm/msm/dsi_pll_10nm: Remove impossible check
  drm/msm: Depopulate platform on probe failure
  drm/msm/dsi: Split mode_flags out of msm_dsi_host_get_panel()
  drm/msm/dsi: Don't store dsi host mode_flags in msm_dsi
  drm/msm/dsi: Pull out panel init code into function
  drm/msm/dsi: Simplify the logic in msm_dsi_manager_panel_init()
  drm/msm/dsi: Use the new setup_encoder function in attach_dsi_device
  drm/msm/dsi: Move dsi panel init into modeset init path
  drm/msm/dsi: Move setup_encoder to modeset_init
  drm/msm: Re-order uninit function to work during probe defer

 .../devicetree/bindings/display/msm/dpu.txt|  10 ++
 

Re: [PATCH v5 01/18] kunit: test: add KUnit test runner core

[Luis Chamberlain]

On Mon, Jun 17, 2019 at 01:25:56AM -0700, Brendan Higgins wrote:
> +/**
> + * module_test() - used to register a  kunit_module with KUnit.
> + * @module: a statically allocated  kunit_module.
> + *
> + * Registers @module with the test framework. See  kunit_module for 
> more
> + * information.
> + */
> +#define module_test(module) \
> + static int module_kunit_init##module(void) \
> + { \
> + return kunit_run_tests(); \
> + } \
> + late_initcall(module_kunit_init##module)

Becuase late_initcall() is used, if these modules are built-in, this
would preclude the ability to test things prior to this part of the
kernel under UML or whatever architecture runs the tests. So, this
limits the scope of testing. Small detail but the scope whould be
documented.

> +static void kunit_print_tap_version(void)
> +{
> + if (!kunit_has_printed_tap_version) {
> + kunit_printk_emit(LOGLEVEL_INFO, "TAP version 14\n");

What is this TAP thing? Why should we care what version it is on?
Why are we printing this?

> + kunit_has_printed_tap_version = true;
> + }
> +}
> +
> +static size_t kunit_test_cases_len(struct kunit_case *test_cases)
> +{
> + struct kunit_case *test_case;
> + size_t len = 0;
> +
> + for (test_case = test_cases; test_case->run_case; test_case++)

If we make the last test case NULL, we'd just check for test_case here,
and save ourselves an extra few bytes per test module. Any reason why
the last test case cannot be NULL?

> +void kunit_init_test(struct kunit *test, const char *name)
> +{
> + spin_lock_init(>lock);
> + test->name = name;
> + test->success = true;
> +}
> +
> +/*
> + * Performs all logic to run a test case.
> + */
> +static void kunit_run_case(struct kunit_module *module,
> +struct kunit_case *test_case)
> +{
> + struct kunit test;
> + int ret = 0;
> +
> + kunit_init_test(, test_case->name);
> +
> + if (module->init) {
> + ret = module->init();

I believe if we used struct kunit_module *kmodule it would be much
clearer who's init this is.

  Luis

Re: [PATCH v5 01/18] kunit: test: add KUnit test runner core

[Brendan Higgins]

On Tue, Jun 25, 2019 at 2:44 PM Luis Chamberlain  wrote:
>
> On Tue, Jun 25, 2019 at 01:28:25PM -0700, Brendan Higgins wrote:
> > On Wed, Jun 19, 2019 at 5:15 PM Stephen Boyd  wrote:
> > >
> > > Quoting Brendan Higgins (2019-06-17 01:25:56)
> > > > diff --git a/kunit/test.c b/kunit/test.c
> > > > new file mode 100644
> > > > index 0..d05d254f1521f
> > > > --- /dev/null
> > > > +++ b/kunit/test.c
> > > > @@ -0,0 +1,210 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Base unit test (KUnit) API.
> > > > + *
> > > > + * Copyright (C) 2019, Google LLC.
> > > > + * Author: Brendan Higgins 
> > > > + */
> > > > +
> > > > +#include 
> > > > +#include 
> > > > +
> > > > +static bool kunit_get_success(struct kunit *test)
> > > > +{
> > > > +   unsigned long flags;
> > > > +   bool success;
> > > > +
> > > > +   spin_lock_irqsave(>lock, flags);
> > > > +   success = test->success;
> > > > +   spin_unlock_irqrestore(>lock, flags);
> > >
> > > I still don't understand the locking scheme in this code. Is the
> > > intention to make getter and setter APIs that are "safe" by adding in a
> > > spinlock that is held around getting and setting various members in the
> > > kunit structure?
> >
> > Yes, your understanding is correct. It is possible for a user to write
> > a test such that certain elements may be updated in different threads;
> > this would most likely happen in the case where someone wants to make
> > an assertion or an expectation in a thread created by a piece of code
> > under test. Although this should generally be avoided, it is possible,
> > and there are occasionally good reasons to do so, so it is
> > functionality that we should support.
> >
> > Do you think I should add a comment to this effect?
> >
> > > In what situation is there more than one thread reading or writing the
> > > kunit struct? Isn't it only a single process that is going to be
> >
> > As I said above, it is possible that the code under test may spawn a
> > new thread that may make an expectation or an assertion. It is not a
> > super common use case, but it is possible.
>
> I wonder if it is worth to have then different types of tests based on
> locking requirements. One with no locking, since it seems you imply
> most tests would fall under this category, then locking, and another
> with IRQ context.
>
> If no locking is done at all for all tests which do not require locking,
> is there any gains at run time? I'm sure it might be minimum but
> curious.

Yeah, I don't think it is worth it.

I don't think we need to be squeezing every ounce of performance out
of unit tests, since they are inherently a cost and are not intended
to be run in a production deployed kernel as part of normal production
usage.

> > > operating on this structure? And why do we need to disable irqs? Are we
> > > expecting to be modifying the unit tests from irq contexts?
> >
> > There are instances where someone may want to test a driver which has
> > an interrupt handler in it. I actually have (not the greatest) example
> > here. Now in these cases, I expect someone to use a mock irqchip or
> > some other fake mechanism to trigger the interrupt handler and not
> > actual hardware; technically speaking in this case, it is not going to
> > be accessed from a "real" irq context; however, the code under test
> > should think that it is in an irq context; given that, I figured it is
> > best to just treat it as a real irq context. Does that make sense?
>
> Since its a new architecture and since you seem to imply most tests
> don't require locking or even IRQs disabled, I think its worth to
> consider the impact of adding such extreme locking requirements for
> an initial ramp up.

Fair enough, I can see the point of not wanting to use irq disabled
until we get someone complaining about it, but I think making it
thread safe is reasonable. It means there is one less thing to confuse
a KUnit user and the only penalty paid is some very minor performance.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 06/18] kbuild: enable building KUnit

[Luis Chamberlain]

On Mon, Jun 17, 2019 at 01:26:01AM -0700, Brendan Higgins wrote:
> diff --git a/Kconfig b/Kconfig
> index 48a80beab6853..10428501edb78 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -30,3 +30,5 @@ source "crypto/Kconfig"
>  source "lib/Kconfig"
>  
>  source "lib/Kconfig.debug"
> +
> +source "kunit/Kconfig"

This patch would break compilation as kunit/Kconfig is not introduced. This
would would also break bisectability on this commit. This change should
either be folded in to the next patch, or just be a separate patch after
the next one.

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

Re: [PATCH v2 10/15] dt-bindings: display: Convert tpo,tpg110 panel to DT schema

[Linus Walleij]

On Tue, Jun 25, 2019 at 4:26 PM Rob Herring  wrote:
> On Tue, Jun 25, 2019 at 2:26 AM Linus Walleij  
> wrote:

> > Can I simply just merge the panel-common patch as well and we
> > are all happy?
>
> I have drm-misc commit rights too, so I'll apply the whole lot when it's 
> ready.
>
> > I can also pick up more panel binding patches, IMO the yaml
> > conversions are especially uncontroversial and should have low
> > threshold for merging.
>
> Yes, but the threshold is at least 'make dt_binding_check' should not
> break. But don't worry, there are 2 other breakages in linux-next
> currently.

OK let's try to live with it for now, if it makes you too much trouble
we can just revert it, accidents happen.

Yours,
Linus Walleij

Re: [PATCH v5 01/18] kunit: test: add KUnit test runner core

[Luis Chamberlain]

On Tue, Jun 25, 2019 at 01:28:25PM -0700, Brendan Higgins wrote:
> On Wed, Jun 19, 2019 at 5:15 PM Stephen Boyd  wrote:
> >
> > Quoting Brendan Higgins (2019-06-17 01:25:56)
> > > diff --git a/kunit/test.c b/kunit/test.c
> > > new file mode 100644
> > > index 0..d05d254f1521f
> > > --- /dev/null
> > > +++ b/kunit/test.c
> > > @@ -0,0 +1,210 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Base unit test (KUnit) API.
> > > + *
> > > + * Copyright (C) 2019, Google LLC.
> > > + * Author: Brendan Higgins 
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +
> > > +static bool kunit_get_success(struct kunit *test)
> > > +{
> > > +   unsigned long flags;
> > > +   bool success;
> > > +
> > > +   spin_lock_irqsave(>lock, flags);
> > > +   success = test->success;
> > > +   spin_unlock_irqrestore(>lock, flags);
> >
> > I still don't understand the locking scheme in this code. Is the
> > intention to make getter and setter APIs that are "safe" by adding in a
> > spinlock that is held around getting and setting various members in the
> > kunit structure?
> 
> Yes, your understanding is correct. It is possible for a user to write
> a test such that certain elements may be updated in different threads;
> this would most likely happen in the case where someone wants to make
> an assertion or an expectation in a thread created by a piece of code
> under test. Although this should generally be avoided, it is possible,
> and there are occasionally good reasons to do so, so it is
> functionality that we should support.
> 
> Do you think I should add a comment to this effect?
> 
> > In what situation is there more than one thread reading or writing the
> > kunit struct? Isn't it only a single process that is going to be
> 
> As I said above, it is possible that the code under test may spawn a
> new thread that may make an expectation or an assertion. It is not a
> super common use case, but it is possible.

I wonder if it is worth to have then different types of tests based on
locking requirements. One with no locking, since it seems you imply
most tests would fall under this category, then locking, and another
with IRQ context.

If no locking is done at all for all tests which do not require locking,
is there any gains at run time? I'm sure it might be minimum but
curious.

> > operating on this structure? And why do we need to disable irqs? Are we
> > expecting to be modifying the unit tests from irq contexts?
> 
> There are instances where someone may want to test a driver which has
> an interrupt handler in it. I actually have (not the greatest) example
> here. Now in these cases, I expect someone to use a mock irqchip or
> some other fake mechanism to trigger the interrupt handler and not
> actual hardware; technically speaking in this case, it is not going to
> be accessed from a "real" irq context; however, the code under test
> should think that it is in an irq context; given that, I figured it is
> best to just treat it as a real irq context. Does that make sense?

Since its a new architecture and since you seem to imply most tests
don't require locking or even IRQs disabled, I think its worth to
consider the impact of adding such extreme locking requirements for
an initial ramp up.

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

Re: drm/msm/dpu: Correct dpu encoder spinlock initialization

[Jeykumar Sankaran]

On 2019-06-24 22:44, d...@codeaurora.org wrote:

On 2019-06-25 03:56, Jeykumar Sankaran wrote:

On 2019-06-23 23:27, Shubhashree Dhar wrote:

dpu encoder spinlock should be initialized during dpu encoder
init instead of dpu encoder setup which is part of commit.
There are chances that vblank control uses the uninitialized
spinlock if not initialized during encoder init.

Not much can be done if someone is performing a vblank operation
before encoder_setup is done.
Can you point to the path where this lock is acquired before
the encoder_setup?

Thanks
Jeykumar S.




When running some dp usecase, we are hitting this callstack.

Process kworker/u16:8 (pid: 215, stack limit = 0xdf9dd930)
Call trace:
 spin_dump+0x84/0x8c
 spin_dump+0x0/0x8c
 do_raw_spin_lock+0x80/0xb0
 _raw_spin_lock_irqsave+0x34/0x44
 dpu_encoder_toggle_vblank_for_crtc+0x8c/0xe8
 dpu_crtc_vblank+0x168/0x1a0
 dpu_kms_enable_vblank+0[   11.648998]  vblank_ctrl_worker+0x3c/0x60
 process_one_work+0x16c/0x2d8
 worker_thread+0x1d8/0x2b0
 kthread+0x124/0x134

Looks like vblank is getting enabled earlier causing this issue and we
are using the spinlock without initializing it.

Thanks,
Shubhashree


DP calls into set_encoder_mode during hotplug before even notifying the
u/s. Can you trace out the original caller of this stack?

Even though the patch is harmless, I am not entirely convinced to move 
this

initialization. Any call which acquires the lock before encoder_setup
will be a no-op since there will not be any physical encoder to work 
with.


Thanks and Regards,
Jeykumar S.


Change-Id: I5a18b95fa47397c834a266b22abf33a517b03a4e
Signed-off-by: Shubhashree Dhar 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 5f085b5..22938c7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2195,8 +2195,6 @@ int dpu_encoder_setup(struct drm_device *dev, 
struct

drm_encoder *enc,
if (ret)
goto fail;

-   spin_lock_init(_enc->enc_spinlock);
-
atomic_set(_enc->frame_done_timeout, 0);
timer_setup(_enc->frame_done_timer,
dpu_encoder_frame_done_timeout, 0);
@@ -2250,6 +2248,7 @@ struct drm_encoder *dpu_encoder_init(struct
drm_device *dev,

drm_encoder_helper_add(_enc->base, _encoder_helper_funcs);

+   spin_lock_init(_enc->enc_spinlock);
dpu_enc->enabled = false;

return _enc->base;


--
Jeykumar S

Re: [PATCH 2/4] drm/panel: jh057n0090: Don't use magic constant

[Sam Ravnborg]

On Tue, Jun 25, 2019 at 07:05:17PM +0200, Guido Günther wrote:
> 0xBF isn't in any ST7703 data sheet so mark it as unknown. This avoids
> confusion on whether there is a missing command in that
> dsi_generic_write_seq() call.
> 
> Signed-off-by: Guido Günther 
Reviewed-by: Sam Ravnborg 
> ---
>  drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c 
> b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
> index 6dcb692c4701..b8a069055fbc 100644
> --- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
> +++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
> @@ -33,6 +33,7 @@
>  #define ST7703_CMD_SETEXTC0xB9
>  #define ST7703_CMD_SETMIPI0xBA
>  #define ST7703_CMD_SETVDC 0xBC
> +#define ST7703_CMD_UNKNOWN0   0xBF
>  #define ST7703_CMD_SETSCR 0xC0
>  #define ST7703_CMD_SETPOWER   0xC1
>  #define ST7703_CMD_SETPANEL   0xCC
> @@ -94,7 +95,7 @@ static int jh057n_init_sequence(struct jh057n *ctx)
>   msleep(20);
>  
>   dsi_generic_write_seq(dsi, ST7703_CMD_SETVCOM, 0x3F, 0x3F);
> - dsi_generic_write_seq(dsi, 0xBF, 0x02, 0x11, 0x00);
> + dsi_generic_write_seq(dsi, ST7703_CMD_UNKNOWN0, 0x02, 0x11, 0x00);
>   dsi_generic_write_seq(dsi, ST7703_CMD_SETGIP1,
> 0x82, 0x10, 0x06, 0x05, 0x9E, 0x0A, 0xA5, 0x12,
> 0x31, 0x23, 0x37, 0x83, 0x04, 0xBC, 0x27, 0x38,
> -- 
> 2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/panel: jh057n0090: Add regulator support

[Sam Ravnborg]

On Tue, Jun 25, 2019 at 07:05:19PM +0200, Guido Günther wrote:
> Allow to specify regulators for vcc and iovcc. According to the data
> sheet the panel wants vcc (2.8V) and iovcc (1.8V) and there's no startup
> dependency between the two.
s/jh057n0090/jh057n00900

> 
> Signed-off-by: Guido Günther 
> ---
>  .../drm/panel/panel-rocktech-jh057n00900.c| 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c 
> b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
> index b8a069055fbc..f8f6f087b9bc 100644
> --- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
> +++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -47,6 +48,8 @@ struct jh057n {
>   struct drm_panel panel;
>   struct gpio_desc *reset_gpio;
>   struct backlight_device *backlight;
> + struct regulator *vcc;
> + struct regulator *iovcc;
>   bool prepared;
>  
>   struct dentry *debugfs;
> @@ -160,6 +163,8 @@ static int jh057n_unprepare(struct drm_panel *panel)
>   return 0;
>  
>   mipi_dsi_dcs_set_display_off(dsi);
> + regulator_disable(ctx->iovcc);
> + regulator_disable(ctx->vcc);
>   ctx->prepared = false;
>  
>   return 0;
> @@ -174,6 +179,13 @@ static int jh057n_prepare(struct drm_panel *panel)
>   return 0;
>  
>   DRM_DEV_DEBUG_DRIVER(ctx->dev, "Resetting the panel\n");
> + ret = regulator_enable(ctx->vcc);
> + if (ret < 0)
> + return ret;
> + ret = regulator_enable(ctx->iovcc);
> + if (ret < 0)
> + return ret;
> +
>   gpiod_set_value_cansleep(ctx->reset_gpio, 1);
>   usleep_range(20, 40);
>   gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> @@ -301,6 +313,13 @@ static int jh057n_probe(struct mipi_dsi_device *dsi)
>   if (IS_ERR(ctx->backlight))
>   return PTR_ERR(ctx->backlight);
>  
> + ctx->vcc = devm_regulator_get(dev, "vcc");
> + if (IS_ERR(ctx->vcc))
> + return PTR_ERR(ctx->vcc);
> + ctx->iovcc = devm_regulator_get(dev, "iovcc");
> + if (IS_ERR(ctx->iovcc))
> + return PTR_ERR(ctx->iovcc);
> +
Consider to write an error message.
The regulators are now mandatory, but they be missing in some device
trees. So it would be good to help them to understand why it fails.

With this considered:
Reviewed-by: Sam Ravnborg 

Sam

Re: [PATCH 3/4] dt-bindings: display/panel: jh057n0090: Document power supply properties

[Sam Ravnborg]

On Tue, Jun 25, 2019 at 07:05:18PM +0200, Guido Günther wrote:
> Document the vcc-supply and iovcc-supply propertes of the Rocktech
> jh057n0090 panel.
s/propertes/properties
s/jh057n0090/jh057n00900

With these fixed:
Reviewed-by: Sam Ravnborg 
> 
> Signed-off-by: Guido Günther 
> ---
>  .../bindings/display/panel/rocktech,jh057n00900.txt  | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/rocktech,jh057n00900.txt 
> b/Documentation/devicetree/bindings/display/panel/rocktech,jh057n00900.txt
> index 1b5763200cf6..a372c5d84695 100644
> --- a/Documentation/devicetree/bindings/display/panel/rocktech,jh057n00900.txt
> +++ b/Documentation/devicetree/bindings/display/panel/rocktech,jh057n00900.txt
> @@ -5,6 +5,9 @@ Required properties:
>  - reg: DSI virtual channel of the peripheral
>  - reset-gpios: panel reset gpio
>  - backlight: phandle of the backlight device attached to the panel
> +- vcc-supply: phandle of the regulator that provides the vcc supply voltage.
> +- iovcc-supply: phandle of the regulator that provides the iovcc supply
> +  voltage.
>  
>  Example:
>  
> @@ -14,5 +17,7 @@ Example:
>   reg = <0>;
>   backlight = <>;
>   reset-gpios = < 13 GPIO_ACTIVE_LOW>;
> + vcc-supply = <_2v8_p>;
> + iovcc-supply = <_1v8_p>;
>   };
>   };
> -- 
> 2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/7] drm/msm: Use drm_gem_fb_prepare_fb

[Rob Clark]

On Tue, Jun 25, 2019 at 1:42 PM Daniel Vetter  wrote:
>
> msm has switched over to drm_fb->obj[] a while ago already, so we can
> just use the helper.
>
> v2: Make it compile ... oops.
>
> Cc: Eric Anholt 
> Cc: Emil Velikov 
> Signed-off-by: Daniel Vetter 
> Cc: Rob Clark 
> Cc: Sean Paul 
> Cc: Jeykumar Sankaran 
> Cc: Jordan Crouse 
> Cc: Bruce Wang 
> Cc: Fritz Koenig 
> Cc: Daniel Vetter 
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org

with 1/7 this is:

Reviewed-by: Rob Clark 

(and 1/7 is r-b w/ commit msg fixup)

BR,
-R


> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 8 ++--
>  drivers/gpu/drm/msm/msm_atomic.c  | 8 ++--
>  2 files changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 44a72da71482..cc08f4366bdd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -11,6 +11,7 @@
>  #include 
>
>  #include 
> +#include 
>
>  #include "msm_drv.h"
>  #include "dpu_kms.h"
> @@ -763,8 +764,6 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
> struct dpu_plane *pdpu = to_dpu_plane(plane);
> struct dpu_plane_state *pstate = to_dpu_plane_state(new_state);
> struct dpu_hw_fmt_layout layout;
> -   struct drm_gem_object *obj;
> -   struct dma_fence *fence;
> struct dpu_kms *kms = _dpu_plane_get_kms(>base);
> int ret;
>
> @@ -781,10 +780,7 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
>  *   we can use msm_atomic_prepare_fb() instead of doing the
>  *   implicit fence and fb prepare by hand here.
>  */
> -   obj = msm_framebuffer_bo(new_state->fb, 0);
> -   fence = reservation_object_get_excl_rcu(obj->resv);
> -   if (fence)
> -   drm_atomic_set_fence_for_plane(new_state, fence);
> +   drm_gem_fb_prepare_fb(plane, new_state);
>
> if (pstate->aspace) {
> ret = msm_framebuffer_prepare(new_state->fb,
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c 
> b/drivers/gpu/drm/msm/msm_atomic.c
> index dd16babdd8c0..169d5f915e68 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -5,6 +5,7 @@
>   */
>
>  #include 
> +#include 
>
>  #include "msm_drv.h"
>  #include "msm_gem.h"
> @@ -37,16 +38,11 @@ int msm_atomic_prepare_fb(struct drm_plane *plane,
>  {
> struct msm_drm_private *priv = plane->dev->dev_private;
> struct msm_kms *kms = priv->kms;
> -   struct drm_gem_object *obj;
> -   struct dma_fence *fence;
>
> if (!new_state->fb)
> return 0;
>
> -   obj = msm_framebuffer_bo(new_state->fb, 0);
> -   fence = reservation_object_get_excl_rcu(obj->resv);
> -
> -   drm_atomic_set_fence_for_plane(new_state, fence);
> +   drm_gem_fb_prepare_fb(plane, new_state);
>
> return msm_framebuffer_prepare(new_state->fb, kms->aspace);
>  }
> --
> 2.20.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/7] drm/radeon: Fill out gem_object->resv

[Sam Ravnborg]

On Tue, Jun 25, 2019 at 10:42:05PM +0200, Daniel Vetter wrote:
> That way we can ditch our gem_prime_res_obj implementation. Since ttm
> absolutely needs the right reservation object all the boilerplate is
> already there and we just have to wire it up correctly.
> 
> Note that gem/prime doesn't care when we do this, as long as we do it
> before the bo is registered and someone can call the handle2fd ioctl
> on it.
> 
> Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
> of always passing a non-NULL resv to ttm_bo_init(). At least for gem
> drivers that would avoid having two of these, on in ttm_buffer_object
> and the other in drm_gem_object, one just there for confusion.
Something for todo.rst - so this does not get lost in a changelog
people will soon forget?

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

Re: [PATCH 1/7] drm/fb-helper: use gem_bo.resv, not dma_buf.resv in prepare_fb

[Sam Ravnborg]

On Tue, Jun 25, 2019 at 10:42:02PM +0200, Daniel Vetter wrote:
> With
> 
> commit 5f6ed9879a414636405a2bd77f122881695959e4
> Author: Daniel Vetter 
> Date:   Fri Jun 14 22:35:57 2019 +0200
> 
> drm/prime: automatically set gem_obj->resv on import
> 
> we consistently set drm_gem_bo.resv for imported buffers. Which means
> we don't need to check to check the dma-buf in the prepare_fb helper,
checked a bit too much?
> but can generalize them so they're also useful for display+render
> drivers which use gem_bo.resv to track their own rendering for their
> own scanout buffers.
> 
> Cc: Emil Velikov 
> Cc: Eric Anholt 
> Cc: Rob Clark 
> Signed-off-by: Daniel Vetter 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/doc: Document kapi doc expectations

[Sam Ravnborg]

On Tue, Jun 25, 2019 at 10:36:44PM +0200, Daniel Vetter wrote:
> We've had this already for anything new. With my drm_prime.c cleanup I
> also think documentations for everything already existing is complete,
s/documentations/documentation?

> and we can bake this in as a requirements subsystem wide.
> 
> Acked-by: Jani Nikula 
> Signed-off-by: Daniel Vetter 
> Cc: Laurent Pinchart 
> Cc: Jani Nikula 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: Sean Paul 
> ---
For what it is worth...
Acked-by: Sam Ravnborg 

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

Re: [PATCH v3 0/5] drm/panel-simple: Add panel parameters for ortustech-com37h3m05dtc/99dtc and sharp-lq070y3dg3b

[Sam Ravnborg]

Hi Nikolaus

> > V3:
> > * add bindings documentation (suggested by s...@ravnborg.org)
> > 
> > V2 2019-06-05 07:07:05:
> > * fix typo in 99dtc panel compatible string (reported by 
> > imir...@alum.mit.edu)
> > 
> > V1 2019-06-04 14:53:00:
> > 
> > Since v5.2-rc1 OMAP is no longer using a special display driver architecture
> > for DPI panels, but uses the general drm/panel/panel-simple.
> > 
> > So we finally can add SoC independent panel definitions for two panel models
> > which we already had worked on quite a while ago (before device tree was
> > introduced):
> > 
> > https://patchwork.kernel.org/patch/2851295/
> > 
> > 
> > 
> > H. Nikolaus Schaller (5):
> >  drm/panel: simple: Add Sharp LQ070Y3DG3B panel support
> >  drm/panel: simple: Add Ortustech COM37H3M panel support
> >  dt-bindings: drm/panel: simple: add ortustech,com37h3m05dtc panel
> >  dt-bindings: drm/panel: simple: add ortustech,com37h3m99dtc panel
> >  dt-bindings: drm/panel: simple: add sharp,lq070y3dg3b panel
> > 
> > .../display/panel/ortustech,com37h3m05dtc.txt | 12 
> > .../display/panel/ortustech,com37h3m99dtc.txt | 12 
> > .../display/panel/sharp,lq070y3dg3b.txt   | 12 
> > drivers/gpu/drm/panel/panel-simple.c  | 63 +++
> > 4 files changed, 99 insertions(+)
> > create mode 100644 
> > Documentation/devicetree/bindings/display/panel/ortustech,com37h3m05dtc.txt
> > create mode 100644 
> > Documentation/devicetree/bindings/display/panel/ortustech,com37h3m99dtc.txt
> > create mode 100644 
> > Documentation/devicetree/bindings/display/panel/sharp,lq070y3dg3b.txt
> > 
> > -- 
> > 2.19.1
> > 
> 
> any progress towards merging this somewhere? It did not yet arrive in 
> linux-next.
> 
> BTW: should also be applied to 5.2
The drm bits are reviewed. The DT bits needs OK from DT people.
When we have OK from DT people we can apply them all to drm-misc-next.

If we need them expedited towards the upstream kernel you will need help
from someone else. But let's get them in drm-misc-next first.

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

Re: [PATCH] drm/doc: Document kapi doc expectations

[Sean Paul]

On Tue, Jun 25, 2019 at 10:36:44PM +0200, Daniel Vetter wrote:
> We've had this already for anything new. With my drm_prime.c cleanup I
> also think documentations for everything already existing is complete,
> and we can bake this in as a requirements subsystem wide.
> 
> Acked-by: Jani Nikula 
> Signed-off-by: Daniel Vetter 
> Cc: Laurent Pinchart 
> Cc: Jani Nikula 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: Sean Paul 

Wholeheartedly-Acked-by: Sean Paul 

> ---
> resending stand-alone for more visibility and a-b gathering.
> -Daniel
> ---
>  Documentation/gpu/introduction.rst | 13 +
>  Documentation/gpu/todo.rst | 13 -
>  2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/gpu/introduction.rst 
> b/Documentation/gpu/introduction.rst
> index fccbe375244d..a94ad6ad1f54 100644
> --- a/Documentation/gpu/introduction.rst
> +++ b/Documentation/gpu/introduction.rst
> @@ -51,6 +51,19 @@ and "FIXME" where the interface could be cleaned up.
>  
>  Also read the :ref:`guidelines for the kernel documentation at large 
> `.
>  
> +Documentation Requirements for kAPI
> +---
> +
> +All kernel APIs exported to other modules must be documented, including their
> +datastructures and at least a short introductory section explaining the 
> overall
> +concepts. Documentation should be put into the code itself as kerneldoc 
> comments
> +as much as reasonable. Do not blindly document everything, but document only
> +what's relevant for driver authors: Internal functions of drm.ko and 
> definitely
> +static functions should not have formal kerneldoc comments. Use normal C
> +comments if you feel like a comment is warranted. Similar for data 
> structures,
> +annotate anything entirely private with ``/* private: */`` comments as per 
> the
> +documentation guide.
> +
>  Getting Started
>  ===
>  
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index e717f280f9ae..db88969a56ee 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -301,19 +301,6 @@ In the end no .c file should need to include ``drmP.h`` 
> anymore.
>  
>  Contact: Daniel Vetter
>  
> -Add missing kerneldoc for exported functions
> -
> -
> -The DRM reference documentation is still lacking kerneldoc in a few areas. 
> The
> -task would be to clean up interfaces like moving functions around between
> -files to better group them and improving the interfaces like dropping return
> -values for functions that never fail. Then write kerneldoc for all exported
> -functions and an overview section and integrate it all into the drm book.
> -
> -See https://dri.freedesktop.org/docs/drm/ for what's there already.
> -
> -Contact: Daniel Vetter
> -
>  Make panic handling work
>  
>  
> -- 
> 2.20.1
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 7/7] drm/prime: Ditch gem_prime_res_obj hook

[Daniel Vetter]

Everyone is just using gem_object->resv now.

Reviewed-by: Emil Velikov 
Signed-off-by: Daniel Vetter 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
---
 Documentation/gpu/todo.rst  |  9 -
 drivers/gpu/drm/drm_prime.c |  3 ---
 include/drm/drm_drv.h   | 12 
 3 files changed, 24 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index d49c1cc6dc28..e717f280f9ae 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -196,15 +196,6 @@ Might be good to also have some igt testcases for this.
 
 Contact: Daniel Vetter, Noralf Tronnes
 
-Remove the ->gem_prime_res_obj callback
-
-
-The ->gem_prime_res_obj callback can be removed from drivers by using the
-reservation_object in the drm_gem_object. It may also be possible to use the
-generic drm_gem_reservation_object_wait helper for waiting for a bo.
-
-Contact: Daniel Vetter
-
 idr_init_base()
 ---
 
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 189d980402ad..b3b044d4089a 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -835,9 +835,6 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object 
*obj,
.resv = obj->resv,
};
 
-   if (dev->driver->gem_prime_res_obj)
-   exp_info.resv = dev->driver->gem_prime_res_obj(obj);
-
return drm_gem_dmabuf_export(dev, _info);
 }
 EXPORT_SYMBOL(drm_gem_prime_export);
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 9927f4f894ef..380e134c5415 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -618,18 +618,6 @@ struct drm_driver {
 */
struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj);
 
-   /**
-* @gem_prime_res_obj:
-*
-* Optional hook to look up the _object for an buffer when
-* exporting it.
-*
-* FIXME: This hook is deprecated. Users of this hook should be replaced
-* by setting _gem_object.resv instead.
-*/
-   struct reservation_object * (*gem_prime_res_obj)(
-   struct drm_gem_object *obj);
-
/**
 * @gem_prime_import_sg_table:
 *
-- 
2.20.1

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

[PATCH 5/7] drm/nouveau: Fill out gem_object->resv

[Daniel Vetter]

That way we can ditch our gem_prime_res_obj implementation. Since ttm
absolutely needs the right reservation object all the boilerplate is
already there and we just have to wire it up correctly.

Note that gem/prime doesn't care when we do this, as long as we do it
before the bo is registered and someone can call the handle2fd ioctl
on it.

Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
of always passing a non-NULL resv to ttm_bo_init(). At least for gem
drivers that would avoid having two of these, on in ttm_buffer_object
and the other in drm_gem_object, one just there for confusion.

Reviewed-by: Emil Velikov 
Signed-off-by: Daniel Vetter 
Cc: Ben Skeggs 
Cc: nouv...@lists.freedesktop.org
---
 drivers/gpu/drm/nouveau/nouveau_bo.c| 2 ++
 drivers/gpu/drm/nouveau/nouveau_drm.c   | 1 -
 drivers/gpu/drm/nouveau/nouveau_gem.h   | 1 -
 drivers/gpu/drm/nouveau/nouveau_prime.c | 7 ---
 4 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 34a998012bf6..6f1217b3e6b9 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -299,6 +299,8 @@ nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align,
  type, >placement,
  align >> PAGE_SHIFT, false, acc_size, sg,
  robj, nouveau_bo_del_ttm);
+   nvbo->gem.resv = nvbo->bo.resv;
+
if (ret) {
/* ttm will call nouveau_bo_del_ttm if it fails.. */
return ret;
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 4377b836265f..2c36319c158f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1131,7 +1131,6 @@ driver_stub = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_pin = nouveau_gem_prime_pin,
-   .gem_prime_res_obj = nouveau_gem_prime_res_obj,
.gem_prime_unpin = nouveau_gem_prime_unpin,
.gem_prime_get_sg_table = nouveau_gem_prime_get_sg_table,
.gem_prime_import_sg_table = nouveau_gem_prime_import_sg_table,
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.h 
b/drivers/gpu/drm/nouveau/nouveau_gem.h
index fe39998f65cc..9dea015387fd 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.h
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.h
@@ -33,7 +33,6 @@ extern int nouveau_gem_ioctl_info(struct drm_device *, void *,
  struct drm_file *);
 
 extern int nouveau_gem_prime_pin(struct drm_gem_object *);
-struct reservation_object *nouveau_gem_prime_res_obj(struct drm_gem_object *);
 extern void nouveau_gem_prime_unpin(struct drm_gem_object *);
 extern struct sg_table *nouveau_gem_prime_get_sg_table(struct drm_gem_object 
*);
 extern struct drm_gem_object *nouveau_gem_prime_import_sg_table(
diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c 
b/drivers/gpu/drm/nouveau/nouveau_prime.c
index 1fefc93af1d7..ec50017692d4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_prime.c
+++ b/drivers/gpu/drm/nouveau/nouveau_prime.c
@@ -107,10 +107,3 @@ void nouveau_gem_prime_unpin(struct drm_gem_object *obj)
 
nouveau_bo_unpin(nvbo);
 }
-
-struct reservation_object *nouveau_gem_prime_res_obj(struct drm_gem_object 
*obj)
-{
-   struct nouveau_bo *nvbo = nouveau_gem_object(obj);
-
-   return nvbo->bo.resv;
-}
-- 
2.20.1

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

[PATCH 1/7] drm/fb-helper: use gem_bo.resv, not dma_buf.resv in prepare_fb

[Daniel Vetter]

With

commit 5f6ed9879a414636405a2bd77f122881695959e4
Author: Daniel Vetter 
Date:   Fri Jun 14 22:35:57 2019 +0200

drm/prime: automatically set gem_obj->resv on import

we consistently set drm_gem_bo.resv for imported buffers. Which means
we don't need to check to check the dma-buf in the prepare_fb helper,
but can generalize them so they're also useful for display+render
drivers which use gem_bo.resv to track their own rendering for their
own scanout buffers.

Cc: Emil Velikov 
Cc: Eric Anholt 
Cc: Rob Clark 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 29 ++--
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c 
b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 8fcbabf02dfd..a6426c95b383 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -271,11 +271,11 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
  * @plane: Plane
  * @state: Plane state the fence will be attached to
  *
- * This function prepares a GEM backed framebuffer for scanout by checking if
- * the plane framebuffer has a DMA-BUF attached. If it does, it extracts the
- * exclusive fence and attaches it to the plane state for the atomic helper to
- * wait on. This function can be used as the _plane_helper_funcs.prepare_fb
- * callback.
+ * This function extracts the exclusive fence from _gem_object.resv and
+ * attaches it to plane state for the atomic helper to wait on. This is
+ * necessary to correctly implement implicit synchronization for any buffers
+ * shared as a struct _buf. This function can be used as the
+ * _plane_helper_funcs.prepare_fb callback.
  *
  * There is no need for _plane_helper_funcs.cleanup_fb hook for simple
  * gem based framebuffer drivers which have their buffers always pinned in
@@ -287,17 +287,15 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
 int drm_gem_fb_prepare_fb(struct drm_plane *plane,
  struct drm_plane_state *state)
 {
-   struct dma_buf *dma_buf;
+   struct drm_gem_object *obj;
struct dma_fence *fence;
 
if (!state->fb)
return 0;
 
-   dma_buf = drm_gem_fb_get_obj(state->fb, 0)->dma_buf;
-   if (dma_buf) {
-   fence = reservation_object_get_excl_rcu(dma_buf->resv);
-   drm_atomic_set_fence_for_plane(state, fence);
-   }
+   obj = drm_gem_fb_get_obj(state->fb, 0);
+   fence = reservation_object_get_excl_rcu(obj->resv);
+   drm_atomic_set_fence_for_plane(state, fence);
 
return 0;
 }
@@ -309,10 +307,11 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb);
  * @pipe: Simple display pipe
  * @plane_state: Plane state
  *
- * This function uses drm_gem_fb_prepare_fb() to check if the plane FB has a
- * _buf attached, extracts the exclusive fence and attaches it to plane
- * state for the atomic helper to wait on. Drivers can use this as their
- * _simple_display_pipe_funcs.prepare_fb callback.
+ * This function uses drm_gem_fb_prepare_fb() to extract the exclusive fence
+ * from _gem_object.resv and attaches it to plane state for the atomic
+ * helper to wait on. This is necessary to correctly implement implicit
+ * synchronization for any buffers shared as a struct _buf. Drivers can use
+ * this as their _simple_display_pipe_funcs.prepare_fb callback.
  *
  * See drm_atomic_set_fence_for_plane() for a discussion of implicit and
  * explicit fencing in atomic modeset updates.
-- 
2.20.1

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

[PATCH 4/7] drm/radeon: Fill out gem_object->resv

[Daniel Vetter]

That way we can ditch our gem_prime_res_obj implementation. Since ttm
absolutely needs the right reservation object all the boilerplate is
already there and we just have to wire it up correctly.

Note that gem/prime doesn't care when we do this, as long as we do it
before the bo is registered and someone can call the handle2fd ioctl
on it.

Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
of always passing a non-NULL resv to ttm_bo_init(). At least for gem
drivers that would avoid having two of these, on in ttm_buffer_object
and the other in drm_gem_object, one just there for confusion.

Reviewed-by: Emil Velikov 
Signed-off-by: Daniel Vetter 
Cc: Alex Deucher 
Cc: "Christian König" 
Cc: "David (ChunMing) Zhou" 
Cc: amd-...@lists.freedesktop.org
---
 drivers/gpu/drm/radeon/radeon_drv.c| 2 --
 drivers/gpu/drm/radeon/radeon_object.c | 1 +
 drivers/gpu/drm/radeon/radeon_prime.c  | 7 ---
 3 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index 4403e76e1ae0..a4a78dfdef37 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -152,7 +152,6 @@ struct drm_gem_object 
*radeon_gem_prime_import_sg_table(struct drm_device *dev,
struct sg_table *sg);
 int radeon_gem_prime_pin(struct drm_gem_object *obj);
 void radeon_gem_prime_unpin(struct drm_gem_object *obj);
-struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object *);
 void *radeon_gem_prime_vmap(struct drm_gem_object *obj);
 void radeon_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
 
@@ -566,7 +565,6 @@ static struct drm_driver kms_driver = {
.gem_prime_export = radeon_gem_prime_export,
.gem_prime_pin = radeon_gem_prime_pin,
.gem_prime_unpin = radeon_gem_prime_unpin,
-   .gem_prime_res_obj = radeon_gem_prime_res_obj,
.gem_prime_get_sg_table = radeon_gem_prime_get_sg_table,
.gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
.gem_prime_vmap = radeon_gem_prime_vmap,
diff --git a/drivers/gpu/drm/radeon/radeon_object.c 
b/drivers/gpu/drm/radeon/radeon_object.c
index 21f73fc86f38..7a2bad843f8a 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -262,6 +262,7 @@ int radeon_bo_create(struct radeon_device *rdev,
r = ttm_bo_init(>mman.bdev, >tbo, size, type,
>placement, page_align, !kernel, acc_size,
sg, resv, _ttm_bo_destroy);
+   bo->gem_base.resv = bo->tbo.resv;
up_read(>pm.mclk_lock);
if (unlikely(r != 0)) {
return r;
diff --git a/drivers/gpu/drm/radeon/radeon_prime.c 
b/drivers/gpu/drm/radeon/radeon_prime.c
index deaffce50a2e..8ce3e8045d42 100644
--- a/drivers/gpu/drm/radeon/radeon_prime.c
+++ b/drivers/gpu/drm/radeon/radeon_prime.c
@@ -117,13 +117,6 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj)
 }
 
 
-struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object *obj)
-{
-   struct radeon_bo *bo = gem_to_radeon_bo(obj);
-
-   return bo->tbo.resv;
-}
-
 struct dma_buf *radeon_gem_prime_export(struct drm_gem_object *gobj,
int flags)
 {
-- 
2.20.1

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

[PATCH 6/7] drm/amdgpu: Fill out gem_object->resv

[Daniel Vetter]

That way we can ditch our gem_prime_res_obj implementation. Since ttm
absolutely needs the right reservation object all the boilerplate is
already there and we just have to wire it up correctly.

Note that gem/prime doesn't care when we do this, as long as we do it
before the bo is registered and someone can call the handle2fd ioctl
on it.

Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
of always passing a non-NULL resv to ttm_bo_init(). At least for gem
drivers that would avoid having two of these, on in ttm_buffer_object
and the other in drm_gem_object, one just there for confusion.

Reviewed-by: Emil Velikov 
Signed-off-by: Daniel Vetter 
Cc: Alex Deucher 
Cc: "Christian König" 
Cc: Daniel Vetter 
Cc: "Michel Dänzer" 
Cc: Chris Wilson 
Cc: Huang Rui 
Cc: Felix Kuehling 
Cc: Andrey Grodzovsky 
Cc: Evan Quan 
Cc: Sonny Jiang 
Cc: Amber Lin 
Cc: "Michał Mirosław" 
Cc: Junwei Zhang 
Cc: Thomas Zimmermann 
Cc: Samuel Li 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 17 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  |  2 ++
 4 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 4809d4a5d72a..02cd845e77b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -267,20 +267,6 @@ static void amdgpu_dma_buf_map_detach(struct dma_buf 
*dma_buf,
drm_gem_map_detach(dma_buf, attach);
 }
 
-/**
- * amdgpu_gem_prime_res_obj - _driver.gem_prime_res_obj implementation
- * @obj: GEM BO
- *
- * Returns:
- * The BO's reservation object.
- */
-struct reservation_object *amdgpu_gem_prime_res_obj(struct drm_gem_object *obj)
-{
-   struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
-
-   return bo->tbo.resv;
-}
-
 /**
  * amdgpu_dma_buf_begin_cpu_access - _buf_ops.begin_cpu_access 
implementation
  * @dma_buf: Shared DMA buffer
@@ -339,8 +325,7 @@ const struct dma_buf_ops amdgpu_dmabuf_ops = {
  * @gobj: GEM BO
  * @flags: Flags such as DRM_CLOEXEC and DRM_RDWR.
  *
- * The main work is done by the _gem_prime_export helper, which in turn
- * uses _gem_prime_res_obj.
+ * The main work is done by the _gem_prime_export helper.
  *
  * Returns:
  * Shared DMA buffer representing the GEM BO from the given device.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
index 7f73a4f94204..5012e6ab58f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
@@ -34,7 +34,6 @@ struct dma_buf *amdgpu_gem_prime_export(struct drm_gem_object 
*gobj,
int flags);
 struct drm_gem_object *amdgpu_gem_prime_import(struct drm_device *dev,
struct dma_buf *dma_buf);
-struct reservation_object *amdgpu_gem_prime_res_obj(struct drm_gem_object *);
 void *amdgpu_gem_prime_vmap(struct drm_gem_object *obj);
 void amdgpu_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
 int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 8e1b269351e8..3233c5abf5b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1333,7 +1333,6 @@ static struct drm_driver kms_driver = {
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_export = amdgpu_gem_prime_export,
.gem_prime_import = amdgpu_gem_prime_import,
-   .gem_prime_res_obj = amdgpu_gem_prime_res_obj,
.gem_prime_get_sg_table = amdgpu_gem_prime_get_sg_table,
.gem_prime_import_sg_table = amdgpu_gem_prime_import_sg_table,
.gem_prime_vmap = amdgpu_gem_prime_vmap,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 16f96f2e3671..7b251fd26bd5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -505,6 +505,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
if (unlikely(r != 0))
return r;
 
+   bo->gem_base.resv = bo->tbo.resv;
+
if (!amdgpu_gmc_vram_full_visible(>gmc) &&
bo->tbo.mem.mem_type == TTM_PL_VRAM &&
bo->tbo.mem.start < adev->gmc.visible_vram_size >> PAGE_SHIFT)
-- 
2.20.1

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

[PATCH 0/7] gem_bo.resv prime unification, leftovers

[Daniel Vetter]

Hi all,

Here's the unmerged leftovers from my big prime cleanup series:
- using the prepare_fb helper in vc4, now hopefully fixed up. The
  helper should be now even more useful.

- amd driver ->gem_prime_res_obj callback removal. I think this one
  might have functional conflicts with Gerd's patch series to embed
  drm_gem_object in ttm_bo, or at least needs to be re-reviewed before we
  merge the 2nd series.

Comments, testing, feedback as usual very much welcome.

Thanks, Daniel

Daniel Vetter (7):
  drm/fb-helper: use gem_bo.resv, not dma_buf.resv in prepare_fb
  drm/msm: Use drm_gem_fb_prepare_fb
  drm/vc4: Use drm_gem_fb_prepare_fb
  drm/radeon: Fill out gem_object->resv
  drm/nouveau: Fill out gem_object->resv
  drm/amdgpu: Fill out gem_object->resv
  drm/prime: Ditch gem_prime_res_obj hook

 Documentation/gpu/todo.rst   |  9 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c  | 17 +---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h  |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c   |  2 ++
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 29 ++--
 drivers/gpu/drm/drm_prime.c  |  3 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c|  8 ++
 drivers/gpu/drm/msm/msm_atomic.c |  8 ++
 drivers/gpu/drm/nouveau/nouveau_bo.c |  2 ++
 drivers/gpu/drm/nouveau/nouveau_drm.c|  1 -
 drivers/gpu/drm/nouveau/nouveau_gem.h|  1 -
 drivers/gpu/drm/nouveau/nouveau_prime.c  |  7 -
 drivers/gpu/drm/radeon/radeon_drv.c  |  2 --
 drivers/gpu/drm/radeon/radeon_object.c   |  1 +
 drivers/gpu/drm/radeon/radeon_prime.c|  7 -
 drivers/gpu/drm/vc4/vc4_plane.c  |  5 ++--
 include/drm/drm_drv.h| 12 
 18 files changed, 26 insertions(+), 90 deletions(-)

-- 
2.20.1

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

[PATCH 3/7] drm/vc4: Use drm_gem_fb_prepare_fb

[Daniel Vetter]

vc4 has switched to using drm_fb->obj[], so we can just use the helper
unchanged.

v2: Make it compile ... oops.

Cc: Eric Anholt 
Cc: Emil Velikov 
Signed-off-by: Daniel Vetter 
Cc: Eric Anholt 
---
 drivers/gpu/drm/vc4/vc4_plane.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 0a0207c350a5..a996ca8ff972 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "uapi/drm/vc4_drm.h"
 #include "vc4_drv.h"
@@ -1123,7 +1124,6 @@ static int vc4_prepare_fb(struct drm_plane *plane,
  struct drm_plane_state *state)
 {
struct vc4_bo *bo;
-   struct dma_fence *fence;
int ret;
 
if (!state->fb)
@@ -1131,8 +1131,7 @@ static int vc4_prepare_fb(struct drm_plane *plane,
 
bo = to_vc4_bo(_fb_cma_get_gem_obj(state->fb, 0)->base);
 
-   fence = reservation_object_get_excl_rcu(bo->base.base.resv);
-   drm_atomic_set_fence_for_plane(state, fence);
+   drm_gem_fb_prepare_fb(plane, state);
 
if (plane->state->fb == state->fb)
return 0;
-- 
2.20.1

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

[PATCH 2/7] drm/msm: Use drm_gem_fb_prepare_fb

[Daniel Vetter]

msm has switched over to drm_fb->obj[] a while ago already, so we can
just use the helper.

v2: Make it compile ... oops.

Cc: Eric Anholt 
Cc: Emil Velikov 
Signed-off-by: Daniel Vetter 
Cc: Rob Clark 
Cc: Sean Paul 
Cc: Jeykumar Sankaran 
Cc: Jordan Crouse 
Cc: Bruce Wang 
Cc: Fritz Koenig 
Cc: Daniel Vetter 
Cc: linux-arm-...@vger.kernel.org
Cc: freedr...@lists.freedesktop.org
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 8 ++--
 drivers/gpu/drm/msm/msm_atomic.c  | 8 ++--
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 44a72da71482..cc08f4366bdd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -11,6 +11,7 @@
 #include 
 
 #include 
+#include 
 
 #include "msm_drv.h"
 #include "dpu_kms.h"
@@ -763,8 +764,6 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_plane_state *pstate = to_dpu_plane_state(new_state);
struct dpu_hw_fmt_layout layout;
-   struct drm_gem_object *obj;
-   struct dma_fence *fence;
struct dpu_kms *kms = _dpu_plane_get_kms(>base);
int ret;
 
@@ -781,10 +780,7 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
 *   we can use msm_atomic_prepare_fb() instead of doing the
 *   implicit fence and fb prepare by hand here.
 */
-   obj = msm_framebuffer_bo(new_state->fb, 0);
-   fence = reservation_object_get_excl_rcu(obj->resv);
-   if (fence)
-   drm_atomic_set_fence_for_plane(new_state, fence);
+   drm_gem_fb_prepare_fb(plane, new_state);
 
if (pstate->aspace) {
ret = msm_framebuffer_prepare(new_state->fb,
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index dd16babdd8c0..169d5f915e68 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -5,6 +5,7 @@
  */
 
 #include 
+#include 
 
 #include "msm_drv.h"
 #include "msm_gem.h"
@@ -37,16 +38,11 @@ int msm_atomic_prepare_fb(struct drm_plane *plane,
 {
struct msm_drm_private *priv = plane->dev->dev_private;
struct msm_kms *kms = priv->kms;
-   struct drm_gem_object *obj;
-   struct dma_fence *fence;
 
if (!new_state->fb)
return 0;
 
-   obj = msm_framebuffer_bo(new_state->fb, 0);
-   fence = reservation_object_get_excl_rcu(obj->resv);
-
-   drm_atomic_set_fence_for_plane(new_state, fence);
+   drm_gem_fb_prepare_fb(plane, new_state);
 
return msm_framebuffer_prepare(new_state->fb, kms->aspace);
 }
-- 
2.20.1

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

Re: [PATCH v3 5/5] dt-bindings: drm/panel: simple: add sharp, lq070y3dg3b panel

[Sam Ravnborg]

On Fri, Jun 07, 2019 at 01:11:11PM +0200, H. Nikolaus Schaller wrote:
> Signed-off-by: H. Nikolaus Schaller 
Reviewed-by: Sam Ravnborg 
> ---
>  .../bindings/display/panel/sharp,lq070y3dg3b.txt | 12 
>  1 file changed, 12 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/sharp,lq070y3dg3b.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/sharp,lq070y3dg3b.txt 
> b/Documentation/devicetree/bindings/display/panel/sharp,lq070y3dg3b.txt
> new file mode 100644
> index ..95534b55ee5f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/sharp,lq070y3dg3b.txt
> @@ -0,0 +1,12 @@
> +Sharp LQ070Y3DG3B 7.0" WVGA landscape TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "sharp,lq070y3dg3b"
> +
> +Optional properties:
> +- enable-gpios: GPIO pin to enable or disable the panel
> +- backlight: phandle of the backlight device attached to the panel
> +- power-supply: phandle of the regulator that provides the supply voltage
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.
> -- 
> 2.19.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/5] drm/panel: simple: Add Ortustech COM37H3M panel support

[Sam Ravnborg]

On Fri, Jun 07, 2019 at 01:11:08PM +0200, H. Nikolaus Schaller wrote:
> The change adds support for the Ortustech COM37H3M05DTC/99DTC 3.7" TFT LCD 
> panel.
> 
> Tested on Letux3704.
> 
> Signed-off-by: H. Nikolaus Schaller 
Reviewed-by: Sam Ravnborg 
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 33 
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c 
> b/drivers/gpu/drm/panel/panel-simple.c
> index 5b27829c5a78..1fb74908a269 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -2007,6 +2007,33 @@ static const struct panel_desc ontat_yx700wv03 = {
>   .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
>  };
>  
> +static const struct drm_display_mode ortustech_com37h3m_mode  = {
> + .clock = 22153,
> + .hdisplay = 480,
> + .hsync_start = 480 + 8,
> + .hsync_end = 480 + 8 + 10,
> + .htotal = 480 + 8 + 10 + 10,
> + .vdisplay = 640,
> + .vsync_start = 640 + 4,
> + .vsync_end = 640 + 4 + 3,
> + .vtotal = 640 + 4 + 3 + 4,
> + .vrefresh = 60,
> + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> +};
> +
> +static const struct panel_desc ortustech_com37h3m = {
> + .modes = _com37h3m_mode,
> + .num_modes = 1,
> + .bpc = 8,
> + .size = {
> + .width = 56,/* 56.16mm */
> + .height = 75,   /* 74.88mm */
> + },
> + .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE |
> +  DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
> +};
> +
>  static const struct drm_display_mode ortustech_com43h4m85ulc_mode  = {
>   .clock = 25000,
>   .hdisplay = 480,
> @@ -2786,6 +2813,12 @@ static const struct of_device_id platform_of_match[] = 
> {
>   }, {
>   .compatible = "ontat,yx700wv03",
>   .data = _yx700wv03,
> + }, {
> + .compatible = "ortustech,com37h3m05dtc",
> + .data = _com37h3m,
> + }, {
> + .compatible = "ortustech,com37h3m99dtc",
> + .data = _com37h3m,
>   }, {
>   .compatible = "ortustech,com43h4m85ulc",
>   .data = _com43h4m85ulc,
> -- 
> 2.19.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 4/5] dt-bindings: drm/panel: simple: add ortustech, com37h3m99dtc panel

[Sam Ravnborg]

On Fri, Jun 07, 2019 at 01:11:10PM +0200, H. Nikolaus Schaller wrote:
> Signed-off-by: H. Nikolaus Schaller 
Reviewed-by: Sam Ravnborg 
> ---
>  .../display/panel/ortustech,com37h3m99dtc.txt| 12 
>  1 file changed, 12 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/ortustech,com37h3m99dtc.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/ortustech,com37h3m99dtc.txt 
> b/Documentation/devicetree/bindings/display/panel/ortustech,com37h3m99dtc.txt
> new file mode 100644
> index ..06a73c3f46b5
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/display/panel/ortustech,com37h3m99dtc.txt
> @@ -0,0 +1,12 @@
> +OrtusTech COM37H3M99DTC Blanview 3.7" VGA portrait TFT-LCD panel
> +
> +Required properties:
> +- compatible: should be "ortustech,com37h3m99dtc"
> +
> +Optional properties:
> +- enable-gpios: GPIO pin to enable or disable the panel
> +- backlight: phandle of the backlight device attached to the panel
> +- power-supply: phandle of the regulator that provides the supply voltage
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.
> -- 
> 2.19.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/5] drm/panel: simple: Add Sharp LQ070Y3DG3B panel support

[Sam Ravnborg]

On Fri, Jun 07, 2019 at 01:11:07PM +0200, H. Nikolaus Schaller wrote:
> The change adds support for the Sharp LQ070Y3DG3B 7.0" TFT LCD panel.
> 
> Tested on Letux7004.
> 
> Signed-off-by: H. Nikolaus Schaller 
Reviewed-by: Sam Ravnborg 
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 30 
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c 
> b/drivers/gpu/drm/panel/panel-simple.c
> index 569be4efd8d1..5b27829c5a78 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -2204,6 +2204,33 @@ static const struct panel_desc samsung_ltn140at29_301 
> = {
>   },
>  };
>  
> +static const struct drm_display_mode sharp_lq070y3dg3b_mode = {
> + .clock = 33260,
> + .hdisplay = 800,
> + .hsync_start = 800 + 64,
> + .hsync_end = 800 + 64 + 128,
> + .htotal = 800 + 64 + 128 + 64,
> + .vdisplay = 480,
> + .vsync_start = 480 + 8,
> + .vsync_end = 480 + 8 + 2,
> + .vtotal = 480 + 8 + 2 + 35,
> + .vrefresh = 60,
> + .flags = DISPLAY_FLAGS_PIXDATA_POSEDGE,
> +};
> +
> +static const struct panel_desc sharp_lq070y3dg3b = {
> + .modes = _lq070y3dg3b_mode,
> + .num_modes = 1,
> + .bpc = 8,
> + .size = {
> + .width = 152,   /* 152.4mm */
> + .height = 91,   /* 91.4mm */
> + },
> + .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE |
> +  DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
> +};
> +
>  static const struct drm_display_mode sharp_lq035q7db03_mode = {
>   .clock = 5500,
>   .hdisplay = 240,
> @@ -2786,6 +2813,9 @@ static const struct of_device_id platform_of_match[] = {
>   }, {
>   .compatible = "sharp,lq035q7db03",
>   .data = _lq035q7db03,
> + }, {
> + .compatible = "sharp,lq070y3dg3b",
> + .data = _lq070y3dg3b,
>   }, {
>   .compatible = "sharp,lq101k1ly04",
>   .data = _lq101k1ly04,
> -- 
> 2.19.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/doc: Document kapi doc expectations

[Daniel Vetter]

We've had this already for anything new. With my drm_prime.c cleanup I
also think documentations for everything already existing is complete,
and we can bake this in as a requirements subsystem wide.

Acked-by: Jani Nikula 
Signed-off-by: Daniel Vetter 
Cc: Laurent Pinchart 
Cc: Jani Nikula 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
---
resending stand-alone for more visibility and a-b gathering.
-Daniel
---
 Documentation/gpu/introduction.rst | 13 +
 Documentation/gpu/todo.rst | 13 -
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/Documentation/gpu/introduction.rst 
b/Documentation/gpu/introduction.rst
index fccbe375244d..a94ad6ad1f54 100644
--- a/Documentation/gpu/introduction.rst
+++ b/Documentation/gpu/introduction.rst
@@ -51,6 +51,19 @@ and "FIXME" where the interface could be cleaned up.
 
 Also read the :ref:`guidelines for the kernel documentation at large 
`.
 
+Documentation Requirements for kAPI
+---
+
+All kernel APIs exported to other modules must be documented, including their
+datastructures and at least a short introductory section explaining the overall
+concepts. Documentation should be put into the code itself as kerneldoc 
comments
+as much as reasonable. Do not blindly document everything, but document only
+what's relevant for driver authors: Internal functions of drm.ko and definitely
+static functions should not have formal kerneldoc comments. Use normal C
+comments if you feel like a comment is warranted. Similar for data structures,
+annotate anything entirely private with ``/* private: */`` comments as per the
+documentation guide.
+
 Getting Started
 ===
 
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index e717f280f9ae..db88969a56ee 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -301,19 +301,6 @@ In the end no .c file should need to include ``drmP.h`` 
anymore.
 
 Contact: Daniel Vetter
 
-Add missing kerneldoc for exported functions
-
-
-The DRM reference documentation is still lacking kerneldoc in a few areas. The
-task would be to clean up interfaces like moving functions around between
-files to better group them and improving the interfaces like dropping return
-values for functions that never fail. Then write kerneldoc for all exported
-functions and an overview section and integrate it all into the drm book.
-
-See https://dri.freedesktop.org/docs/drm/ for what's there already.
-
-Contact: Daniel Vetter
-
 Make panic handling work
 
 
-- 
2.20.1

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

Re: [PATCH v5 01/18] kunit: test: add KUnit test runner core

[Brendan Higgins]

On Wed, Jun 19, 2019 at 5:15 PM Stephen Boyd  wrote:
>
> Quoting Brendan Higgins (2019-06-17 01:25:56)
> > diff --git a/kunit/test.c b/kunit/test.c
> > new file mode 100644
> > index 0..d05d254f1521f
> > --- /dev/null
> > +++ b/kunit/test.c
> > @@ -0,0 +1,210 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Base unit test (KUnit) API.
> > + *
> > + * Copyright (C) 2019, Google LLC.
> > + * Author: Brendan Higgins 
> > + */
> > +
> > +#include 
> > +#include 
> > +
> > +static bool kunit_get_success(struct kunit *test)
> > +{
> > +   unsigned long flags;
> > +   bool success;
> > +
> > +   spin_lock_irqsave(>lock, flags);
> > +   success = test->success;
> > +   spin_unlock_irqrestore(>lock, flags);
>
> I still don't understand the locking scheme in this code. Is the
> intention to make getter and setter APIs that are "safe" by adding in a
> spinlock that is held around getting and setting various members in the
> kunit structure?

Yes, your understanding is correct. It is possible for a user to write
a test such that certain elements may be updated in different threads;
this would most likely happen in the case where someone wants to make
an assertion or an expectation in a thread created by a piece of code
under test. Although this should generally be avoided, it is possible,
and there are occasionally good reasons to do so, so it is
functionality that we should support.

Do you think I should add a comment to this effect?

> In what situation is there more than one thread reading or writing the
> kunit struct? Isn't it only a single process that is going to be

As I said above, it is possible that the code under test may spawn a
new thread that may make an expectation or an assertion. It is not a
super common use case, but it is possible.

> operating on this structure? And why do we need to disable irqs? Are we
> expecting to be modifying the unit tests from irq contexts?

There are instances where someone may want to test a driver which has
an interrupt handler in it. I actually have (not the greatest) example
here. Now in these cases, I expect someone to use a mock irqchip or
some other fake mechanism to trigger the interrupt handler and not
actual hardware; technically speaking in this case, it is not going to
be accessed from a "real" irq context; however, the code under test
should think that it is in an irq context; given that, I figured it is
best to just treat it as a real irq context. Does that make sense?

> > +
> > +   return success;
> > +}
> > +
> > +static void kunit_set_success(struct kunit *test, bool success)
> > +{
> > +   unsigned long flags;
> > +
> > +   spin_lock_irqsave(>lock, flags);
> > +   test->success = success;
> > +   spin_unlock_irqrestore(>lock, flags);
> > +}
> > +
> > +static int kunit_vprintk_emit(int level, const char *fmt, va_list args)
> > +{
> > +   return vprintk_emit(0, level, NULL, 0, fmt, args);
> > +}
> > +
> > +static int kunit_printk_emit(int level, const char *fmt, ...)
> > +{
> > +   va_list args;
> > +   int ret;
> > +
> > +   va_start(args, fmt);
> > +   ret = kunit_vprintk_emit(level, fmt, args);
> > +   va_end(args);
> > +
> > +   return ret;
> > +}
> > +
> > +static void kunit_vprintk(const struct kunit *test,
> > + const char *level,
> > + struct va_format *vaf)
> > +{
> > +   kunit_printk_emit(level[1] - '0', "\t# %s: %pV", test->name, vaf);
> > +}
> > +
> > +static bool kunit_has_printed_tap_version;
>
> Can you please move this into function local scope in the function
> below?

Sure, that makes sense.

> > +
> > +static void kunit_print_tap_version(void)
> > +{
> > +   if (!kunit_has_printed_tap_version) {
> > +   kunit_printk_emit(LOGLEVEL_INFO, "TAP version 14\n");
> > +   kunit_has_printed_tap_version = true;
> > +   }
> > +}
> > +
> [...]
> > +
> > +static bool kunit_module_has_succeeded(struct kunit_module *module)
> > +{
> > +   const struct kunit_case *test_case;
> > +   bool success = true;
> > +
> > +   for (test_case = module->test_cases; test_case->run_case; 
> > test_case++)
> > +   if (!test_case->success) {
> > +   success = false;
> > +   break;
>
> Why not 'return false'?

Also a good point. Will fix.

> > +   }
> > +
> > +   return success;
>
> And 'return true'?

Will fix.

> > +}
> > +
> > +static size_t kunit_module_counter = 1;
> > +
> > +static void kunit_print_subtest_end(struct kunit_module *module)
> > +{
> > +   kunit_print_ok_not_ok(false,
> > + kunit_module_has_succeeded(module),
> > + kunit_module_counter++,
> > + module->name);
> > +}
> > +
> > +static void kunit_print_test_case_ok_not_ok(struct kunit_case *test_case,
> > +   size_t 

Re: [PATCH v3 3/5] dt-bindings: drm/panel: simple: add ortustech, com37h3m05dtc panel

[Sam Ravnborg]

On Fri, Jun 07, 2019 at 01:11:09PM +0200, H. Nikolaus Schaller wrote:
> Signed-off-by: H. Nikolaus Schaller 
Reviewed-by: Sam Ravnborg 

We need OK from one of the DT people before we can apply this.

Sam

> ---
>  .../display/panel/ortustech,com37h3m05dtc.txt| 12 
>  1 file changed, 12 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/ortustech,com37h3m05dtc.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/ortustech,com37h3m05dtc.txt 
> b/Documentation/devicetree/bindings/display/panel/ortustech,com37h3m05dtc.txt
> new file mode 100644
> index ..c16907c02f80
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/display/panel/ortustech,com37h3m05dtc.txt
> @@ -0,0 +1,12 @@
> +OrtusTech COM37H3M05DTC Blanview 3.7" VGA portrait TFT-LCD panel
> +
> +Required properties:
> +- compatible: should be "ortustech,com37h3m05dtc"
> +
> +Optional properties:
> +- enable-gpios: GPIO pin to enable or disable the panel
> +- backlight: phandle of the backlight device attached to the panel
> +- power-supply: phandle of the regulator that provides the supply voltage
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.
> -- 
> 2.19.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 4/4] drm/imx: only send event on crtc disable if kept disabled

[Daniel Vetter]

On Tue, Jun 25, 2019 at 06:59:15PM +0100, Robert Beckett wrote:
> The event will be sent as part of the vblank enable during the modeset
> if the crtc is not being kept disabled.
> 
> Fixes: 5f2f911578fb ("drm/imx: atomic phase 3 step 1: Use atomic 
> configuration")
> 
> Signed-off-by: Robert Beckett 

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/imx/ipuv3-crtc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c 
> b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index e04d6efff1b5..c436a28d50e4 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -94,7 +94,7 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc,
>   drm_crtc_vblank_off(crtc);
>  
>   spin_lock_irq(>dev->event_lock);
> - if (crtc->state->event) {
> + if (crtc->state->event && !crtc->state->active) {
>   drm_crtc_send_vblank_event(crtc, crtc->state->event);
>   crtc->state->event = NULL;
>   }
> -- 
> 2.18.0
> 

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

Re: [PATCH v5 3/3] drm/panel: simple: Add GiantPlus GPM940B0 panel support

[Sam Ravnborg]

On Thu, Jun 06, 2019 at 12:22:47AM +0200, Paul Cercueil wrote:
> The GiantPlus GPM940B0 is a simple 3.0" 320x240 24-bit TFT panel.
> 
> Signed-off-by: Paul Cercueil 
> Tested-by: Artur Rojek 

All patches applied to drm-misc-next.
Added the ack from the mails to patch 2.

Sam

Re: [PATCH] video: fbdev: s3c-fb: Mark expected switch fall-throughs

[Kees Cook]

On Tue, Jun 25, 2019 at 10:49:01AM -0700, Joe Perches wrote:
> On Tue, 2019-06-25 at 10:31 -0700, Kees Cook wrote:
> > On Tue, Jun 25, 2019 at 09:52:23AM -0700, Joe Perches wrote:
> > > On Tue, 2019-06-25 at 11:01 -0500, Gustavo A. R. Silva wrote:
> > > > In preparation to enabling -Wimplicit-fallthrough, mark switch
> > > > cases where we are expecting to fall through.
> > > []
> > > > This patch is part of the ongoing efforts to enable
> > > > -Wimplicit-fallthrough.
> > > 
> > > Just enable the thing already.
> > 
> > Linus has been pretty clear about not wanting warning options enabled
> > without first fixing all the cases it warns about first.
> 
> Hey Kees.
> 
> I don't recall that particular tidbit.  Got a link?  

It was spread out over the discussion around removing __deprecated,
about enabling -Wvla, and in person at the kernel summit when asking
what approach to take for -Wimplicit-fallthrough.

-- 
Kees Cook

Re: [PATCH v3 3/4] drm/vblank: estimate vblank while disabling vblank if interrupt disabled

[Daniel Vetter]

On Tue, Jun 25, 2019 at 06:59:14PM +0100, Robert Beckett wrote:
> If interrupts are disabled (e.g. via vblank_disable_fn) and we come to
> disable vblank, update the vblank count to best guess as to what it
> would be had the interrupts remained enabled, and update the timesamp to
> now.
> 
> This avoids a stale vblank event being sent while disabling crtcs during
> atomic modeset.
> 
> Fixes: 68036b08b91bc ("drm/vblank: Do not update vblank count if interrupts
> are already disabled.")
> 
> Signed-off-by: Robert Beckett 
> ---
>  drivers/gpu/drm/drm_vblank.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 7dabb2bdb733..db68b8cbf797 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -375,9 +375,23 @@ void drm_vblank_disable_and_save(struct drm_device *dev, 
> unsigned int pipe)
>* interrupts were enabled. This avoids calling the ->disable_vblank()
>* operation in atomic context with the hardware potentially runtime
>* suspended.
> +  * If interrupts are disabled (e.g. via blank_disable_fn) then make
> +  * best guess as to what it would be now and make sure we have an up
> +  * to date timestamp.
>*/
> - if (!vblank->enabled)
> + if (!vblank->enabled) {
> + ktime_t now = ktime_get();

Would be nice to fake this a bit more accurately and round the timestamp
here to a multiple of the frame duration, i.e. ...
> + u32 diff = 0;
> + if (vblank->framedur_ns) {
> + u64 diff_ns = ktime_to_ns(ktime_sub(now, vblank->time));
> + diff = DIV_ROUND_CLOSEST_ULL(diff_ns,
> +  vblank->framedur_ns);
> + }

now = vblank->time + diff * vblank_framedur_ns;

Picking the right macro for doing 64bit multiplies left to you :-)

> +
> + store_vblank(dev, pipe, diff, now, vblank->count);
> +
>   goto out;
> + }
>  
>   /*
>* Update the count and timestamp to maintain the

Somewhat unhappy that we're duplicating this logic with
drm_update_vblank_count, but it's just 2 lines of math.
-Daniel

> -- 
> 2.18.0
> 

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

Re: [PATCH v3 2/4] drm/imx: notify drm core before sending event during crtc disable

[Daniel Vetter]

On Tue, Jun 25, 2019 at 06:59:13PM +0100, Robert Beckett wrote:
> Notify drm core before sending pending events during crtc disable.
> This fixes the first event after disable having an old stale timestamp
> by having drm_crtc_vblank_off update the timestamp to now.
> 
> This was seen while debugging weston log message:
> Warning: computed repaint delay is insane: -8212 msec
> 
> This occured due to:
> 1. driver starts up
> 2. fbcon comes along and restores fbdev, enabling vblank
> 3. vblank_disable_fn fires via timer disabling vblank, keeping vblank
> seq number and time set at current value
> (some time later)
> 4. weston starts and does a modeset
> 5. atomic commit disables crtc while it does the modeset
> 6. ipu_crtc_atomic_disable sends vblank with old seq number and time
> 
> Fixes: a474478642d5 ("drm/imx: fix crtc vblank state regression")
> 
> Signed-off-by: Robert Beckett 

Now that I understand what's going on here:

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/imx/ipuv3-crtc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c 
> b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 9cc1d678674f..e04d6efff1b5 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -91,14 +91,14 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc,
>   ipu_dc_disable(ipu);
>   ipu_prg_disable(ipu);
>  
> + drm_crtc_vblank_off(crtc);
> +
>   spin_lock_irq(>dev->event_lock);
>   if (crtc->state->event) {
>   drm_crtc_send_vblank_event(crtc, crtc->state->event);
>   crtc->state->event = NULL;
>   }
>   spin_unlock_irq(>dev->event_lock);
> -
> - drm_crtc_vblank_off(crtc);
>  }
>  
>  static void imx_drm_crtc_reset(struct drm_crtc *crtc)
> -- 
> 2.18.0
> 

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

Re: [PATCH v3 1/4] drm/vblank: warn on sending stale event

[Daniel Vetter]

On Tue, Jun 25, 2019 at 10:00:42PM +0200, Daniel Vetter wrote:
> On Tue, Jun 25, 2019 at 06:59:12PM +0100, Robert Beckett wrote:
> > Warn when about to send stale vblank info and add advice to
> > documentation on how to avoid.
> > 
> > Signed-off-by: Robert Beckett 
> > ---
> >  drivers/gpu/drm/drm_vblank.c | 17 +
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > index 603ab105125d..7dabb2bdb733 100644
> > --- a/drivers/gpu/drm/drm_vblank.c
> > +++ b/drivers/gpu/drm/drm_vblank.c
> > @@ -918,6 +918,19 @@ EXPORT_SYMBOL(drm_crtc_arm_vblank_event);
> >   *
> >   * See drm_crtc_arm_vblank_event() for a helper which can be used in 
> > certain
> >   * situation, especially to send out events for atomic commit operations.
> > + *
> > + * Care should be taken to avoid stale timestamps. If:
> > + *   - your driver has vblank support (i.e. dev->num_crtcs > 0)
> > + *   - the vblank irq is off (i.e. no one called drm_crtc_vblank_get)
> 
> drm_crtc_vblank_get() so it becomes a neat hyperlink.
> 
> > + *   - from the vblank code's pov the pipe is still running (i.e. not
> > + * in-between a drm_crtc_vblank_off()/on() pair)
> 
> Not sure the above will lead to great markup, maybe spell out the
> drm_crtc_vblank_on() and maybe make it a bit clearer like "i.e. not after
> the call to drm_crtc_vblank_off() but before the next call to
> drm_crtc_vblank_on()" so it's clear which said of this pair we're talking
> about.
> 
> > + * If all of these conditions hold then drm_crtc_send_vblank_event is
> 
> style nit: the enumeration is one sentence (and and oxford comman missing
> but whatever), but you don't continue it here. Also, does the enumeration
> look pretty in the htmldocs output?
> 
> > + * going to give you a garbage timestamp and and sequence number (the last
> > + * recorded before the irq was disabled). If you call 
> > drm_crtc_vblank_get/put
> > + * around it, or after vblank_off, then either of those will have rolled 
> > things
> > + * forward for you.
> 
> Again pls fix the markup so all the function reference stick out and work.

One more style nit: s/you/drivers/, so maybe:

"Drivers must either hold a vblank reference acquired through
drm_crtc_vblank_get() or the vblank must have been shut off by calling
drm_crtc_vblank_off()." Those functions then have in turn links and hints
what you also need to do, like not forget to call drm_crtc_vblank_put().
> 
> > + * So, drivers should call drm_crtc_vblank_off() before this function in 
> > their
> > + * crtc atomic_disable handlers.
> 
> Imo this sentence here is needed but a bit confusing, and we have it
> documented already in the atomic_disable hook.
> 
> Other option would be to list all the places where a driver might want to
> call this and how they could get it wrong, which imo doesn't make sense.
> 
> With the nits addressed:
> 
> Reviewed-by: Daniel Vetter 
> 
> >   */
> >  void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
> > struct drm_pending_vblank_event *e)
> > @@ -925,8 +938,12 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
> > struct drm_device *dev = crtc->dev;
> > u64 seq;
> > unsigned int pipe = drm_crtc_index(crtc);
> > +   struct drm_vblank_crtc *vblank = >vblank[pipe];
> > ktime_t now;
> >  
> > +   WARN_ONCE(dev->num_crtcs > 0 && !vblank->enabled && !vblank->inmodeset,
> > + "sending stale vblank info\n");
> > +
> > if (dev->num_crtcs > 0) {
> > seq = drm_vblank_count_and_time(dev, pipe, );
> > } else {
> > -- 
> > 2.18.0
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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

Re: [PATCH v3 1/4] drm/vblank: warn on sending stale event

[Daniel Vetter]

On Tue, Jun 25, 2019 at 06:59:12PM +0100, Robert Beckett wrote:
> Warn when about to send stale vblank info and add advice to
> documentation on how to avoid.
> 
> Signed-off-by: Robert Beckett 
> ---
>  drivers/gpu/drm/drm_vblank.c | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 603ab105125d..7dabb2bdb733 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -918,6 +918,19 @@ EXPORT_SYMBOL(drm_crtc_arm_vblank_event);
>   *
>   * See drm_crtc_arm_vblank_event() for a helper which can be used in certain
>   * situation, especially to send out events for atomic commit operations.
> + *
> + * Care should be taken to avoid stale timestamps. If:
> + *   - your driver has vblank support (i.e. dev->num_crtcs > 0)
> + *   - the vblank irq is off (i.e. no one called drm_crtc_vblank_get)

drm_crtc_vblank_get() so it becomes a neat hyperlink.

> + *   - from the vblank code's pov the pipe is still running (i.e. not
> + * in-between a drm_crtc_vblank_off()/on() pair)

Not sure the above will lead to great markup, maybe spell out the
drm_crtc_vblank_on() and maybe make it a bit clearer like "i.e. not after
the call to drm_crtc_vblank_off() but before the next call to
drm_crtc_vblank_on()" so it's clear which said of this pair we're talking
about.

> + * If all of these conditions hold then drm_crtc_send_vblank_event is

style nit: the enumeration is one sentence (and and oxford comman missing
but whatever), but you don't continue it here. Also, does the enumeration
look pretty in the htmldocs output?

> + * going to give you a garbage timestamp and and sequence number (the last
> + * recorded before the irq was disabled). If you call drm_crtc_vblank_get/put
> + * around it, or after vblank_off, then either of those will have rolled 
> things
> + * forward for you.

Again pls fix the markup so all the function reference stick out and work.

> + * So, drivers should call drm_crtc_vblank_off() before this function in 
> their
> + * crtc atomic_disable handlers.

Imo this sentence here is needed but a bit confusing, and we have it
documented already in the atomic_disable hook.

Other option would be to list all the places where a driver might want to
call this and how they could get it wrong, which imo doesn't make sense.

With the nits addressed:

Reviewed-by: Daniel Vetter 

>   */
>  void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
>   struct drm_pending_vblank_event *e)
> @@ -925,8 +938,12 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
>   struct drm_device *dev = crtc->dev;
>   u64 seq;
>   unsigned int pipe = drm_crtc_index(crtc);
> + struct drm_vblank_crtc *vblank = >vblank[pipe];
>   ktime_t now;
>  
> + WARN_ONCE(dev->num_crtcs > 0 && !vblank->enabled && !vblank->inmodeset,
> +   "sending stale vblank info\n");
> +
>   if (dev->num_crtcs > 0) {
>   seq = drm_vblank_count_and_time(dev, pipe, );
>   } else {
> -- 
> 2.18.0
> 

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

Re: [PATCH 05/22] mm: export alloc_pages_vma

[Dan Williams]

On Tue, Jun 25, 2019 at 12:01 PM Michal Hocko  wrote:
>
> On Tue 25-06-19 11:03:53, Dan Williams wrote:
> > On Tue, Jun 25, 2019 at 8:01 AM Michal Hocko  wrote:
> > >
> > > On Tue 25-06-19 09:23:17, Christoph Hellwig wrote:
> > > > On Mon, Jun 24, 2019 at 11:24:48AM -0700, Dan Williams wrote:
> > > > > I asked for this simply because it was not exported historically. In
> > > > > general I want to establish explicit export-type criteria so the
> > > > > community can spend less time debating when to use EXPORT_SYMBOL_GPL
> > > > > [1].
> > > > >
> > > > > The thought in this instance is that it is not historically exported
> > > > > to modules and it is safer from a maintenance perspective to start
> > > > > with GPL-only for new symbols in case we don't want to maintain that
> > > > > interface long-term for out-of-tree modules.
> > > > >
> > > > > Yes, we always reserve the right to remove / change interfaces
> > > > > regardless of the export type, but history has shown that external
> > > > > pressure to keep an interface stable (contrary to
> > > > > Documentation/process/stable-api-nonsense.rst) tends to be less for
> > > > > GPL-only exports.
> > > >
> > > > Fully agreed.  In the end the decision is with the MM maintainers,
> > > > though, although I'd prefer to keep it as in this series.
> > >
> > > I am sorry but I am not really convinced by the above reasoning wrt. to
> > > the allocator API and it has been a subject of many changes over time. I
> > > do not remember a single case where we would be bending the allocator
> > > API because of external modules and I am pretty sure we will push back
> > > heavily if that was the case in the future.
> >
> > This seems to say that you have no direct experience of dealing with
> > changing symbols that that a prominent out-of-tree module needs? GPU
> > drivers and the core-mm are on a path to increase their cooperation on
> > memory management mechanisms over time, and symbol export changes for
> > out-of-tree GPU drivers have been a significant source of friction in
> > the past.
>
> I have an experience e.g. to rework semantic of some gfp flags and that is
> something that users usualy get wrong and never heard that an out of
> tree code would insist on an old semantic and pushing us to the corner.
>
> > > So in this particular case I would go with consistency and export the
> > > same way we do with other functions. Also we do not want people to
> > > reinvent this API and screw that like we have seen in other cases when
> > > external modules try reimplement core functionality themselves.
> >
> > Consistency is a weak argument when the cost to the upstream community
> > is negligible. If the same functionality was available via another /
> > already exported interface *that* would be an argument to maintain the
> > existing export policy. "Consistency" in and of itself is not a
> > precedent we can use more widely in default export-type decisions.
> >
> > Effectively I'm arguing EXPORT_SYMBOL_GPL by default with a later
> > decision to drop the _GPL. Similar to how we are careful to mark sysfs
> > interfaces in Documentation/ABI/ that we are not fully committed to
> > maintaining over time, or are otherwise so new that there is not yet a
> > good read on whether they can be made permanent.
>
> Documentation/process/stable-api-nonsense.rst

That document has failed to preclude symbol export fights in the past
and there is a reasonable argument to try not to retract functionality
that had been previously exported regardless of that document.

> Really. If you want to play with GPL vs. EXPORT_SYMBOL else this is up
> to you but I do not see any technical argument to make this particular
> interface to the page allocator any different from all others that are
> exported to modules.

I'm failing to find any practical substance to your argument, but in
the end I agree with Chrishoph, it's up to MM maintainers.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: pwm_bl: Set pin to sleep state when powered down

[Uwe Kleine-König]

On Tue, Jun 25, 2019 at 11:58:21AM +0200, Thierry Reding wrote:
> On Tue, Jun 25, 2019 at 09:42:20AM +0200, Uwe Kleine-König wrote:
> > On Wed, May 22, 2019 at 06:34:28PM +0200, Paul Cercueil wrote:
> > > When the driver probes, the PWM pin is automatically configured to its
> > > default state, which should be the "pwm" function. However, at this
> > > point we don't know the actual level of the pin, which may be active or
> > > inactive. As a result, if the driver probes without enabling the
> > > backlight, the PWM pin might be active, and the backlight would be
> > > lit way before being officially enabled.
> > 
> > I'm not sure I understand the problem completely here. Let me try to
> > summarize the problem you solve here:
> > 
> > The backlight device's default pinctrl contains the PWM function of the
> > PWM pin. As the PWM is (or at least might be) in an undefined state the
> > default pinctrl should only be switched to when it's clear if the
> > backlight should be on or off.
> > 
> > So you use the "init"-pinctrl to keep the PWM pin in some (undriven?)
> > state and by switching to "sleep" you prevent "default" getting active.
> > 
> > Did I get this right? If not, please correct me.
> > 
> > What is the PWM pin configured to in "init" in your case? Is the pinctrl
> > just empty? Or is it a gpio-mode (together with a gpio-hog)?
> > 
> > My thoughts to this is are:
> > 
> >  a) This is a general problem that applies (I think) to most if not all
> > PWM consumers. If the PWM drives a motor, or makes your mobile
> > vibrate, or drives an LED, or a clk, the PWM shouldn't start
> > to do something before its consumer is ready.
> 
> Yes, it shouldn't start before it is explicitly told to do so by the
> consumer. One exception is if the PWM was already set up by firmware
> to run. So I think in general terms we always want the PWM to remain
> in the current state upon probe.

In the end this means that also pinmuxing should not be touched when the
PWM device probes.

> The atomic PWM API was designed with that in mind. The original use-
> case was to allow seamlessly taking over from a PWM regulator. In order
> to do so, the driver needs to be able to read back the hardware state
> and *not* initialize the PWM to some default state.
> 
> I think that same approach can be extended to backlights. The driver's
> probe needs to determine what the current state of the backlight is and
> preferable not touch it. And that basically propagates all the way to
> the display driver, which ultimately needs to determine whether or not
> the display configuration (including the backlight) is enabled.

Are you ambitious enough to handle cases like: PWM is running (maybe
because it cannot be disabled), but the pin is muxed to an "unconnected"
configuration such that the pin doesn't oscillate? In this case you'd
need an inspection function for pinmuxing.

> >  b) Thierry made it quite clear[1] that the PWM pin should be configured
> > in a pinctrl of the pwm device, not the backlight (or more general:
> > the consumer) device.
> > 
> > While I don't entirely agree with b) I think that even a) alone
> > justifies to think a bit more about the problem and preferably come up
> > with a solution that helps other consumers, too. Ideally if the
> > bootloader sets up the PWM to do something sensible, probing the
> > lowlevel PWM driver and the consumer driver should not interfere with
> > the bootloader's intention until the situation reaches a controlled
> > state. (I.e. if the backlight was left on by the bootloader to show a
> > nice logo, it should not flicker until a userspace program takes over
> > the display device.)
> 
> Yes, exactly that.
> 
> > A PWM is special in contrast to other devices as its intended behaviour
> > is only fixed once a consumer is present. Without a consumer it is
> > unknown if the PWM is inverted or not. And so the common approach that
> > pinctrl is setup by the device core only doesn't work without drawbacks
> > for PWMs.
> 
> Actually I don't think PWMs are special in this regard. A GPIO, for
> example, can also be active-low or active-high, and without a consumer
> there's not enough context to determine which one it should be.

Right, PWMs are more similar to GPIOs than to (say) backlight devices.
With your request to configure the pinmux for a PWM pin with the PWM
device instead of its consumer you're making some things more difficult.
For GPIOs it's quite common that they are muxed from their consumer
because there the same problems are present.

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |