date:20220518


On 5/9/22 11:44, Alexander Stein wrote:

Hi Lucas,

Am Freitag, 6. Mai 2022, 20:10:25 CEST schrieb Lucas Stach:

second round of the i.MX8MP HDMI work. Still not split up into proper
parts for merging through the various trees this needs to go into, but
should make it easy for people to test.

I've worked in the feedback I got from the last round, including fixing
the system hang that could happen when the drivers were built as modules.

Series is based on linux-next/master, as there are some prerequisite
patches in both the drm and imx tree already. The last patch from [1]
and the patches from [2] need to be applied. Please note that this series
expects the sync polarity from the LCDIF to be set according to the
comments I made in [2]. Please test and provide feedback.


Thanks for the 2nd round of HDMI support patches. Sorry I wasn't able to reply
to your questions, but the PLL locking seems to be gone on my system.

I still get the error

imx-lcdif 32fc6000.display-controller: Unknown media bus format 0x200f


To answer the other question on the last patchset

Do have a 4k HDMI display connected that wants to do YUV input? That's
something I have to admit I didn't test yet and would be likely to
cause this bus format selection.


This is a FullHD HDMI monitor, ASUS PB238Q. Apparently the color format is
YCBCR422. From what I can see is that
dw_hdmi_bridge_atomic_get_output_bus_fmts() adds MEDIA_BUS_FMT_UYVY8_1X16
(0x200f) to the output formats. This is then passed to


Try LCDIFv3 v3 patchset I just posted, that should work then.

[PATCH v3 2/2] drm: lcdif: Add support for i.MX8MP LCDIF variant

Add support for i.MX8MP LCDIF variant. This is called LCDIFv3 and is
completely different from the LCDIFv3 found in i.MX23 in that it has
a completely scrambled register layout compared to all previous LCDIF
variants. The new LCDIFv3 also supports 36bit address space.

Add a separate driver which is really a fork of MXSFB driver with the
i.MX8MP LCDIF variant handling filled in.

Signed-off-by: Marek Vasut 
Cc: Alexander Stein 
Cc: Laurent Pinchart 
Cc: Lucas Stach 
Cc: Peng Fan 
Cc: Robby Cai 
Cc: Sam Ravnborg 
Cc: Stefan Agner 
---
V2: - Drop the pitch check from lcdif_fb_create()
- Drop connector caching
- Wait for shadow load bit to be cleared in IRQ handler
- Make all clock mandatory and grab them all by name
- Wait for EN to be cleared in lcdif_disable_controller
- Rename to imx-lcdif
- Move shadow load to atomic_flush
V3: - Invert DE polarity to match MX8MPRM datasheet
- Enable CSC in RGB to YUV mode for MEDIA_BUS_FMT_UYVY8_1X16
---
 drivers/gpu/drm/mxsfb/Kconfig  |  16 +
 drivers/gpu/drm/mxsfb/Makefile |   2 +
 drivers/gpu/drm/mxsfb/lcdif_drv.c  | 361 
 drivers/gpu/drm/mxsfb/lcdif_drv.h  |  47 +++
 drivers/gpu/drm/mxsfb/lcdif_kms.c  | 507 +
 drivers/gpu/drm/mxsfb/lcdif_regs.h | 257 +++
 6 files changed, 1190 insertions(+)
 create mode 100644 drivers/gpu/drm/mxsfb/lcdif_drv.c
 create mode 100644 drivers/gpu/drm/mxsfb/lcdif_drv.h
 create mode 100644 drivers/gpu/drm/mxsfb/lcdif_kms.c
 create mode 100644 drivers/gpu/drm/mxsfb/lcdif_regs.h

diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
index 987170e16ebd6..873551b4552f5 100644
--- a/drivers/gpu/drm/mxsfb/Kconfig
+++ b/drivers/gpu/drm/mxsfb/Kconfig
@@ -19,3 +19,19 @@ config DRM_MXSFB
  i.MX28, i.MX6SX, i.MX7 and i.MX8M).
 
  If M is selected the module will be called mxsfb.
+
+config DRM_IMX_LCDIF
+   tristate "i.MX LCDIFv3 LCD controller"
+   depends on DRM && OF
+   depends on COMMON_CLK
+   select DRM_MXS
+   select DRM_KMS_HELPER
+   select DRM_GEM_CMA_HELPER
+   select DRM_PANEL
+   select DRM_PANEL_BRIDGE
+   help
+ Choose this option if you have an LCDIFv3 LCD controller.
+ Those devices are found in various i.MX SoC (i.MX8MP,
+ i.MXRT).
+
+ If M is selected the module will be called imx-lcdif.
diff --git a/drivers/gpu/drm/mxsfb/Makefile b/drivers/gpu/drm/mxsfb/Makefile
index 26d153896d720..3fa44059b9d85 100644
--- a/drivers/gpu/drm/mxsfb/Makefile
+++ b/drivers/gpu/drm/mxsfb/Makefile
@@ -1,3 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
 mxsfb-y := mxsfb_drv.o mxsfb_kms.o
 obj-$(CONFIG_DRM_MXSFB)+= mxsfb.o
+imx-lcdif-y := lcdif_drv.o lcdif_kms.o
+obj-$(CONFIG_DRM_IMX_LCDIF) += imx-lcdif.o
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c 
b/drivers/gpu/drm/mxsfb/lcdif_drv.c
new file mode 100644
index 0..3e29c8a768487
--- /dev/null
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -0,0 +1,361 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2022 Marek Vasut 
+ *
+ * This code is based on drivers/gpu/drm/mxsfb/mxsfb*
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lcdif_drv.h"
+#include "lcdif_regs.h"
+
+static struct drm_framebuffer *
+lcdif_fb_create(struct drm_device *dev, struct drm_file *file_priv,
+   const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+   const struct drm_format_info *info;
+
+   info = drm_get_format_info(dev, mode_cmd);
+   if (!info)
+   return ERR_PTR(-EINVAL);
+
+   return drm_gem_fb_create(dev, file_priv, mode_cmd);
+}
+
+static const struct drm_mode_config_funcs lcdif_mode_config_funcs = {
+   .fb_create  = lcdif_fb_create,
+   .atomic_check   = drm_atomic_helper_check,
+   .atomic_commit  = drm_atomic_helper_commit,
+};
+
+static const struct drm_mode_config_helper_funcs lcdif_mode_config_helpers = {
+   .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
+};
+
+static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
+{
+   struct drm_device *drm = lcdif->drm;
+   struct drm_bridge *bridge;
+   struct drm_panel *panel;
+   int ret;
+
+   ret = drm_of_find_panel_or_bridge(drm->dev->of_node, 0, 0, ,
+ );
+   if (ret)
+   return ret;
+
+   if (panel) {
+   bridge = devm_drm_panel_bridge_add_typed(drm->dev, panel,
+
DRM_MODE_CONNECTOR_DPI);
+   if (IS_ERR(bridge))
+   return PTR_ERR(bridge);
+   }
+
+   if (!bridge)
+   return -ENODEV;
+
+   ret = drm_bridge_attach(>encoder, bridge, NULL, 0);

[PATCH v3 1/2] dt-bindings: lcdif: Add compatible for i.MX8MP

Add compatible string for i.MX8MP LCDIF variant. This is called LCDIFv3
and is completely different from the LCDIFv3 found in i.MX23 in that it
has a completely scrambled register layout compared to all previous LCDIF
variants. The new LCDIFv3 also supports 36bit address space. However,
except for the complete bit reshuffling, this is still LCDIF and it still
works like one.

Signed-off-by: Marek Vasut 
Cc: Alexander Stein 
Cc: Laurent Pinchart 
Cc: Lucas Stach 
Cc: Peng Fan 
Cc: Rob Herring 
Cc: Robby Cai 
Cc: Sam Ravnborg 
Cc: Stefan Agner 
Cc: devicet...@vger.kernel.org
---
V2: No change
V3: No change
---
 Documentation/devicetree/bindings/display/fsl,lcdif.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml 
b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
index 900a56cae80e6..876015a44a1e6 100644
--- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
+++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
@@ -20,6 +20,7 @@ properties:
   - fsl,imx23-lcdif
   - fsl,imx28-lcdif
   - fsl,imx6sx-lcdif
+  - fsl,imx8mp-lcdif
   - items:
   - enum:
   - fsl,imx6sl-lcdif
-- 
2.35.1

Re: [PATCH v2] drm/msm/dpu: handle pm_runtime_get_sync() errors in bind path

Quoting Abhinav Kumar (2022-05-18 15:34:07)
> If there are errors while trying to enable the pm in the
> bind path, it will lead to unclocked access of hw revision
> register thereby crashing the device.
>
> This will not address why the pm_runtime_get_sync() fails
> but at the very least we should be able to prevent the
> crash by handling the error and bailing out earlier.
>
> changes in v2:
> - use pm_runtime_resume_and_get() instead of
>   pm_runtime_get_sync()
>
> Signed-off-by: Abhinav Kumar 
> ---

Any Fixes tag? When did pm errors start happening in the bind path?

Reviewed-by: Stephen Boyd

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

2022-05-18 Thread bugzilla-daemon

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

Rev (r...@pop.ms) changed:

   What|Removed |Added

 CC||r...@pop.ms

--- Comment #24 from Rev (r...@pop.ms) ---
Issue is back in 5.17.9 (and 8), AMD 5600G, Mesa
22.2~git2205170600.fffafa~oibaf~f, so its 4 years now?

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

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

Re: [PATCH v2] drm/msm/dpu: handle pm_runtime_get_sync() errors in bind path

2022-05-18 Thread Rob Clark

On Wed, May 18, 2022 at 3:34 PM Abhinav Kumar  wrote:
>
> If there are errors while trying to enable the pm in the
> bind path, it will lead to unclocked access of hw revision
> register thereby crashing the device.
>
> This will not address why the pm_runtime_get_sync() fails
> but at the very least we should be able to prevent the
> crash by handling the error and bailing out earlier.
>
> changes in v2:
> - use pm_runtime_resume_and_get() instead of
>   pm_runtime_get_sync()
>
> Signed-off-by: Abhinav Kumar 

Reviewed-by: Rob Clark 

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 2b9d931474e0..bce47647d891 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1089,7 +1089,9 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>
> dpu_kms_parse_data_bus_icc_path(dpu_kms);
>
> -   pm_runtime_get_sync(_kms->pdev->dev);
> +   rc = pm_runtime_resume_and_get(_kms->pdev->dev);
> +   if (rc < 0)
> +   goto error;
>
> dpu_kms->core_rev = readl_relaxed(dpu_kms->mmio + 0x0);
>
> --
> 2.35.1
>

[PATCH] drm/bridge: ti-sn65dsi83: Handle dsi_lanes == 0 as invalid

Handle empty data-lanes = < >; property, which translates to
dsi_lanes = 0 as invalid.

Fixes: ceb515ba29ba6 ("drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 
driver")
Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Lucas Stach 
Cc: Marek Vasut 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Robert Foss 
Cc: Sam Ravnborg 
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index d64d4385188dd..dc65f424e7f3c 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -585,7 +585,7 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum 
sn65dsi83_model model)
ctx->host_node = of_graph_get_remote_port_parent(endpoint);
of_node_put(endpoint);
 
-   if (ctx->dsi_lanes < 0 || ctx->dsi_lanes > 4) {
+   if (ctx->dsi_lanes <= 0 || ctx->dsi_lanes > 4) {
ret = -EINVAL;
goto err_put_node;
}
-- 
2.35.1

[PATCH 2/3] drm/bridge: tc358767: Report DSI-to-(e)DP as supported

The DSI-to-e(DP) mode is now supported, update the driver comment
to reflect this. No functional change.

Fixes: 3080c21a043ab ("drm/bridge: tc358767: Add DSI-to-(e)DP mode support")
Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Lucas Stach 
Cc: Marek Vasut 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Robert Foss 
Cc: Sam Ravnborg 
---
 drivers/gpu/drm/bridge/tc358767.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index 84e6b5aa8dd1f..a8bb6de9e600a 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -6,7 +6,7 @@
  * The following modes are supported:
  *   DPI->(e)DP -- supported
  *   DSI->DPI  supported
- *   DSI->(e)DP .. NOT supported
+ *   DSI->(e)DP .. supported
  *
  * Copyright (C) 2016 CogentEmbedded Inc
  * Author: Andrey Gusakov 
-- 
2.35.1

[PATCH 3/3] drm/bridge: tc358767: Make sure Refclk clock are enabled

The Refclk may be supplied by SoC clock output instead of crystal
oscillator, make sure the clock are enabled before any other action
is performed with the bridge chip, otherwise it may either fail to
operate at all, or miss reset GPIO toggle.

Fixes: 7caff0fc4296e ("drm/bridge: tc358767: Add DPI to eDP bridge driver")
Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Lucas Stach 
Cc: Marek Vasut 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Robert Foss 
Cc: Sam Ravnborg 
---
 drivers/gpu/drm/bridge/tc358767.c | 51 ---
 1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index a8bb6de9e600a..4b15acbc73c4e 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -2055,10 +2055,24 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
if (ret)
return ret;
 
+   tc->refclk = devm_clk_get(dev, "ref");
+   if (IS_ERR(tc->refclk)) {
+   ret = PTR_ERR(tc->refclk);
+   dev_err(dev, "Failed to get refclk: %d\n", ret);
+   return ret;
+   }
+
+   clk_prepare_enable(tc->refclk);
+
+   /* tRSTW = 100 cycles , at 13 MHz that is ~7.69 us */
+   usleep_range(10, 15);
+
/* Shut down GPIO is optional */
tc->sd_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH);
-   if (IS_ERR(tc->sd_gpio))
-   return PTR_ERR(tc->sd_gpio);
+   if (IS_ERR(tc->sd_gpio)) {
+   ret = PTR_ERR(tc->sd_gpio);
+   goto err_clk;
+   }
 
if (tc->sd_gpio) {
gpiod_set_value_cansleep(tc->sd_gpio, 0);
@@ -2067,26 +2081,21 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
 
/* Reset GPIO is optional */
tc->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
-   if (IS_ERR(tc->reset_gpio))
-   return PTR_ERR(tc->reset_gpio);
+   if (IS_ERR(tc->reset_gpio)) {
+   ret = PTR_ERR(tc->reset_gpio);
+   goto err_clk;
+   }
 
if (tc->reset_gpio) {
gpiod_set_value_cansleep(tc->reset_gpio, 1);
usleep_range(5000, 1);
}
 
-   tc->refclk = devm_clk_get(dev, "ref");
-   if (IS_ERR(tc->refclk)) {
-   ret = PTR_ERR(tc->refclk);
-   dev_err(dev, "Failed to get refclk: %d\n", ret);
-   return ret;
-   }
-
tc->regmap = devm_regmap_init_i2c(client, _regmap_config);
if (IS_ERR(tc->regmap)) {
ret = PTR_ERR(tc->regmap);
dev_err(dev, "Failed to initialize regmap: %d\n", ret);
-   return ret;
+   goto err_clk;
}
 
ret = of_property_read_u32(dev->of_node, "toshiba,hpd-pin",
@@ -2096,7 +2105,7 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
} else {
if (tc->hpd_pin < 0 || tc->hpd_pin > 1) {
dev_err(dev, "failed to parse HPD number\n");
-   return ret;
+   goto err_clk;
}
}
 
@@ -2110,7 +2119,7 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
"tc358767-irq", tc);
if (ret) {
dev_err(dev, "failed to register dp interrupt\n");
-   return ret;
+   goto err_clk;
}
 
tc->have_irq = true;
@@ -2119,12 +2128,13 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
ret = regmap_read(tc->regmap, TC_IDREG, >rev);
if (ret) {
dev_err(tc->dev, "can not read device ID: %d\n", ret);
-   return ret;
+   goto err_clk;
}
 
if ((tc->rev != 0x6601) && (tc->rev != 0x6603)) {
dev_err(tc->dev, "invalid device ID: 0x%08x\n", tc->rev);
-   return -EINVAL;
+   ret = -EINVAL;
+   goto err_clk;
}
 
tc->assr = (tc->rev == 0x6601); /* Enable ASSR for eDP panels */
@@ -2164,7 +2174,7 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
if (tc->bridge.type != DRM_MODE_CONNECTOR_DPI) { /* (e)DP output */
ret = tc_aux_link_setup(tc);
if (ret)
-   return ret;
+   goto err_clk;
}
 
tc->bridge.of_node = dev->of_node;
@@ -2176,11 +2186,15 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
ret = tc_mipi_dsi_host_attach(tc);
if (ret) {
drm_bridge_remove(>bridge);
-   return ret;
+   goto err_clk;

[PATCH 1/3] drm/bridge: tc358767: Handle dsi_lanes == 0 as invalid

Handle empty data-lanes = < >; property, which translates to
dsi_lanes = 0 as invalid.

Fixes: bbfd3190b6562 ("drm/bridge: tc358767: Add DSI-to-DPI mode support")
Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Lucas Stach 
Cc: Marek Vasut 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Robert Foss 
Cc: Sam Ravnborg 
---
 drivers/gpu/drm/bridge/tc358767.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index 7366968dd7b21..84e6b5aa8dd1f 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1902,7 +1902,7 @@ static int tc_mipi_dsi_host_attach(struct tc_data *tc)
of_node_put(host_node);
of_node_put(endpoint);
 
-   if (dsi_lanes < 0 || dsi_lanes > 4)
+   if (dsi_lanes <= 0 || dsi_lanes > 4)
return -EINVAL;
 
if (!host)
-- 
2.35.1

Re: [PATCH V2 0/3] DSI host and peripheral initialisation ordering

2022-05-18 Thread Andrzej Hajda

On 18.05.2022 16:05, Marek Szyprowski wrote:

Hi Dave,

On 11.05.2022 17:47, Dave Stevenson wrote:

On Wed, 11 May 2022 at 15:58, Marek Szyprowski wrote:

On 05.04.2022 13:43, Dave Stevenson wrote:

On Fri, 18 Mar 2022 at 12:25, Dave Stevenson
wrote:

On Fri, 4 Mar 2022 at 15:18, Dave Stevenson
wrote:

Hi All

A gentle ping on this series. Any comments on the approach?
Thanks.

I realise the merge window has just closed and therefore folks have
been busy, but no responses on this after a month?

Do I give up and submit a patch to document that DSI is broken and no one cares?

Thanks for pointing this patchset in the 'drm: bridge: Add Samsung MIPI
DSIM bridge' thread, otherwise I would miss it since I'm not involved
much in the DRM development.

This resolves most of the issues in the Exynos DSI and its recent
conversion to the drm bridge framework. I've added the needed
prepare_upstream_first flags to the panels and everything works fine
without the bridge chain order hacks.

Feel free to add:

Tested-by: Marek Szyprowski

Thanks for testing it. I was almost at the stage of abandoning the patch set.

The only remaining thing to resolve is the moment of enabling DSI host.
The proper sequence is:

1. host power on, 2. device power on, 3. host init, 4. device init, 5.
video enable.

#1 is done in dsi's pre_enable, #2 is done in panel's prepare. #3 was so
far done in the first host transfer call, which usually happens in
panel's prepare, then the #4 happens. Then video enable is done in the
enable callbacks.

What's your definition of host power on and host init here? What state
are you defining the DSI interface to be in after each operation?

Well, lets start from the point that I'm not a DSI specialist nor I'm
not the exynos-dsi author. I just played a bit with the code trying to
restore proper driver operation on the various Exynos based boards I have.

By the host/device power on I mean enabling their power regulators. By
host init I mean executing the samsung_dsim_init() function, which
basically sets the lp-11 state if I understand it right.

Jagan wants to move it to the dsi host pre_enable() to let it work with
DSI bridges controlled over different interfaces
(https://lore.kernel.org/all/20220504114021.33265-6-ja...@amarulasolutions.com/
).

I think I'm in agreement with Jagan.
As documented in patch 4/4:
+ * A DSI host should keep the PHY powered down until the pre_enable
operation is
+ * called. All lanes are in an undefined idle state up to this point, and it
+ * must not be assumed that it is LP-11.
+ * pre_enable should initialise the PHY, set the data lanes to LP-11, and the
+ * clock lane to either LP-11 or HS depending on the mode_flag
+ * %MIPI_DSI_CLOCK_NON_CONTINUOUS.

Right, this theory makes sense.

However Exynos DSI for some reasons did the host initialization in the
first call of the samsung_dsim_host_transfer().

It was a way to fit exynos DSI order of initialization to the frameworks
present at the time it was written:
exynos_dsi was an drm_encoder, and it was connected to drm_panel (DSI
controlled panel).
1. In exynos_dsi_enable host was powered on then drm_panel_prepare was
called.
2. drm_panel_prepare called .prepare callback of the panel, which
powered on the panel and then requested dsi transfers to initialize panel.
3. 1st dsi transfer performed dsi host init (LP-11), after that all
transfers started by panel performed panel initialization.

4. after that control goes back to exynos_dsi_enable.
5. exynos_dsi_enable starts video transfer and notify panel about it via
drm_panel_enable.
6. .enable callback of the panel starts displaying frames (after some
delay).

This construction allowed to keep proper order of hw initialization:
host power on, panel power on, host init, panel init, start video
transmission, start display frames.
Almost all elements of this sequence are enforced by DSI specification
(at least for devices controlled via dsi).
The only thing which I am not sure is placement of "panel power on". It
does not seem to be a part of DSI spec, but I am not 100% sure.
It could be specific to platform (mis)configuration (regulators, level
shifters, ...), or just dsi host init sequence tries to do too much.
I wonder if dropping checking stop state in exynos_dsi wouldn't solve
the mystery [1], or moving it to 1st transfer, maybe with(or w/o)
subsequent code.

Does IMX have some 'vendor code' of DSI host to compare with?

[1]:
https://elixir.bootlin.com/linux/v5.15.1/source/drivers/gpu/drm/exynos/exynos_drm_dsi.c#L860

Regards
Andrzej

If I moved the host
initialization to pre_enable (before powering the panel on), executing
DSI commands failed (timeout). This issue happens on all boards I have
access (Trats, Trats2, Arndale, TM2e), so this must be an issue with
Exynos DSI host itself not related to particular panel/bridge.

If I call samsung_dsim_init() once again, before issuing the first DSI
command, then everything works fine. I've tried

[PATCH v2] drm/msm/dpu: handle pm_runtime_get_sync() errors in bind path

2022-05-18 Thread Abhinav Kumar

If there are errors while trying to enable the pm in the
bind path, it will lead to unclocked access of hw revision
register thereby crashing the device.

This will not address why the pm_runtime_get_sync() fails
but at the very least we should be able to prevent the
crash by handling the error and bailing out earlier.

changes in v2:
- use pm_runtime_resume_and_get() instead of
  pm_runtime_get_sync()

Signed-off-by: Abhinav Kumar 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 2b9d931474e0..bce47647d891 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1089,7 +1089,9 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
 
dpu_kms_parse_data_bus_icc_path(dpu_kms);
 
-   pm_runtime_get_sync(_kms->pdev->dev);
+   rc = pm_runtime_resume_and_get(_kms->pdev->dev);
+   if (rc < 0)
+   goto error;
 
dpu_kms->core_rev = readl_relaxed(dpu_kms->mmio + 0x0);
 
-- 
2.35.1

Re: [PATCH v4 1/3] phy/qualcomm: add regulator_set_load to edp phy

On Thu, 19 May 2022 at 00:36, Kuogee Hsieh  wrote:
>
> This patch add regulator_set_load() before enable regulator at
> eDP phy driver.
>
> Signed-off-by: Kuogee Hsieh 
> ---
>  drivers/phy/qualcomm/phy-qcom-edp.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c 
> b/drivers/phy/qualcomm/phy-qcom-edp.c
> index cacd32f..fbe0be0 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -87,14 +87,20 @@ struct qcom_edp {
>
> struct clk_bulk_data clks[2];
> struct regulator_bulk_data supplies[2];
> +   int enable_load[2];
>  };
>
>  static int qcom_edp_phy_init(struct phy *phy)
>  {
> struct qcom_edp *edp = phy_get_drvdata(phy);
> int ret;
> +   int num_consumers = ARRAY_SIZE(edp->supplies);

No need to. ARRAY_SIZE is compile-time calculated.

> +   int i;
>
> -   ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
> +   for (i = num_consumers - 1; i >= 0; --i)
> +   regulator_set_load(edp->supplies[i].consumer, 
> edp->enable_load[i]);

Please. Change this to the ascending order or provide a good
description why it's done the other way. Your pointer to
regulator_bulk_enable() is not valid, as it processes arrays in the
ascending order.

> +
> +   ret = regulator_bulk_enable(num_consumers, edp->supplies);
> if (ret)
> return ret;
>
> @@ -635,6 +641,8 @@ static int qcom_edp_phy_probe(struct platform_device 
> *pdev)
>
> edp->supplies[0].supply = "vdda-phy";
> edp->supplies[1].supply = "vdda-pll";
> +   edp->enable_load[0] = 21800;/* load for 1.2 V vdda-phy supply */
> +   edp->enable_load[1] = 36000;/* load for 1.2 V vdda-pll supply */

Isn't vdda-pll 0.9V? On sm8250 I see VDD_A_USB0_SS_DP_1P2 and
VDD_A_USB0_SS_DP_CORE (which is 0.9V).

> ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), 
> edp->supplies);
> if (ret)
> return ret;
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>


-- 
With best wishes
Dmitry

Re: [PATCH v4 3/3] drm/msm/dp: delete vdda regulator related functions from eDP/DP controller

On Thu, 19 May 2022 at 00:36, Kuogee Hsieh  wrote:
>
> Vdda regulators are related to both eDP and DP phy so that it should be
> managed at eDP and DP phy driver instead of controller. This patch removes
> vdda regulators related functions out of eDP/DP controller.
>
> Signed-off-by: Kuogee Hsieh 
> Reviewed-by: Stephen Boyd 

Reviewed-by: Dmitry Baryshkov 

> ---
>  drivers/gpu/drm/msm/dp/dp_parser.c | 14 --
>  drivers/gpu/drm/msm/dp/dp_parser.h |  6 ---
>  drivers/gpu/drm/msm/dp/dp_power.c  | 95 
> +-
>  3 files changed, 2 insertions(+), 113 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
> b/drivers/gpu/drm/msm/dp/dp_parser.c
> index 8f9fed9..4ef2130 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -22,14 +22,6 @@
>  #define DP_DEFAULT_P0_OFFSET   0x1000
>  #define DP_DEFAULT_P0_SIZE 0x0400
>
> -static const struct dp_regulator_cfg sdm845_dp_reg_cfg = {
> -   .num = 2,
> -   .regs = {
> -   {"vdda-1p2", 21800, 4 },/* 1.2 V */
> -   {"vdda-0p9", 36000, 32 },   /* 0.9 V */
> -   },
> -};
> -
>  static void __iomem *dp_ioremap(struct platform_device *pdev, int idx, 
> size_t *len)
>  {
> struct resource *res;
> @@ -298,12 +290,6 @@ static int dp_parser_parse(struct dp_parser *parser)
> if (rc)
> return rc;
>
> -   /* Map the corresponding regulator information according to
> -* version. Currently, since we only have one supported platform,
> -* mapping the regulator directly.
> -*/
> -   parser->regulator_cfg = _dp_reg_cfg;
> -
> return 0;
>  }
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h 
> b/drivers/gpu/drm/msm/dp/dp_parser.h
> index 3a4d797..b56b4d7 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -101,11 +101,6 @@ struct dp_reg_entry {
> int disable_load;
>  };
>
> -struct dp_regulator_cfg {
> -   int num;
> -   struct dp_reg_entry regs[DP_DEV_REGULATOR_MAX];
> -};
> -
>  /**
>   * struct dp_parser - DP parser's data exposed to clients
>   *
> @@ -121,7 +116,6 @@ struct dp_parser {
> struct dp_pinctrl pinctrl;
> struct dp_io io;
> struct dp_display_data disp_data;
> -   const struct dp_regulator_cfg *regulator_cfg;
> u32 max_dp_lanes;
> struct drm_bridge *next_bridge;
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_power.c 
> b/drivers/gpu/drm/msm/dp/dp_power.c
> index d9e0117..b52ac1d 100644
> --- a/drivers/gpu/drm/msm/dp/dp_power.c
> +++ b/drivers/gpu/drm/msm/dp/dp_power.c
> @@ -20,82 +20,10 @@ struct dp_power_private {
> struct clk *link_clk_src;
> struct clk *pixel_provider;
> struct clk *link_provider;
> -   struct regulator_bulk_data supplies[DP_DEV_REGULATOR_MAX];
>
> struct dp_power dp_power;
>  };
>
> -static void dp_power_regulator_disable(struct dp_power_private *power)
> -{
> -   struct regulator_bulk_data *s = power->supplies;
> -   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
> -   int num = power->parser->regulator_cfg->num;
> -   int i;
> -
> -   DBG("");
> -   for (i = num - 1; i >= 0; i--)
> -   if (regs[i].disable_load >= 0)
> -   regulator_set_load(s[i].consumer,
> -  regs[i].disable_load);
> -
> -   regulator_bulk_disable(num, s);
> -}
> -
> -static int dp_power_regulator_enable(struct dp_power_private *power)
> -{
> -   struct regulator_bulk_data *s = power->supplies;
> -   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
> -   int num = power->parser->regulator_cfg->num;
> -   int ret, i;
> -
> -   DBG("");
> -   for (i = 0; i < num; i++) {
> -   if (regs[i].enable_load >= 0) {
> -   ret = regulator_set_load(s[i].consumer,
> -regs[i].enable_load);
> -   if (ret < 0) {
> -   pr_err("regulator %d set op mode failed, 
> %d\n",
> -   i, ret);
> -   goto fail;
> -   }
> -   }
> -   }
> -
> -   ret = regulator_bulk_enable(num, s);
> -   if (ret < 0) {
> -   pr_err("regulator enable failed, %d\n", ret);
> -   goto fail;
> -   }
> -
> -   return 0;
> -
> -fail:
> -   for (i--; i >= 0; i--)
> -   regulator_set_load(s[i].consumer, regs[i].disable_load);
> -   return ret;
> -}
> -
> -static int dp_power_regulator_init(struct dp_power_private *power)
> -{
> -   struct regulator_bulk_data *s = power->supplies;
> -   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
> -   struct platform_device *pdev = power->pdev;
> -   int num =

Re: [PATCH v4 2/3] phy/qualcomm: add regulator_set_load to dp phy

On Thu, 19 May 2022 at 00:36, Kuogee Hsieh  wrote:
>
> This patch add regulator_set_load() before enable regulator at
> DP phy driver.
>
> Signed-off-by: Kuogee Hsieh 
> Reviewed-by: Stephen Boyd 
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp.c | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
> b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index b144ae1..20ac446 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -3130,6 +3130,7 @@ struct qmp_phy_cfg {
> int num_resets;
> /* regulators to be requested */
> const char * const *vreg_list;
> +   const unsigned int *vreg_enable_load;
> int num_vregs;
>
> /* array of registers with different offsets */
> @@ -3346,6 +3347,10 @@ static const char * const qmp_phy_vreg_l[] = {
> "vdda-phy", "vdda-pll",
>  };
>
> +static const unsigned int qmp_phy_vreg_enable_load[] = {
> +   21800, 36000
> +};
> +
>  static const struct qmp_phy_cfg ipq8074_usb3phy_cfg = {
> .type   = PHY_TYPE_USB3,
> .nlanes = 1,
> @@ -4072,6 +4077,7 @@ static const struct qmp_phy_cfg sm8250_usb3phy_cfg = {
> .reset_list = msm8996_usb3phy_reset_l,
> .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
> .vreg_list  = qmp_phy_vreg_l,
> +   .vreg_enable_load   = qmp_phy_vreg_enable_load,
> .num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
> .regs   = qmp_v4_usb3phy_regs_layout,
>
> @@ -4139,6 +4145,7 @@ static const struct qmp_phy_cfg sm8250_dpphy_cfg = {
> .reset_list = msm8996_usb3phy_reset_l,
> .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
> .vreg_list  = qmp_phy_vreg_l,
> +   .vreg_enable_load   = qmp_phy_vreg_enable_load,
> .num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
> .regs   = qmp_v4_usb3phy_regs_layout,

I'd ask again: what about the sdm845? SC8180x? SC7180? Do we need to
change them too?
Currently they will all be handled by the DP driver.

>
> @@ -5008,6 +5015,11 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)
> return 0;
> }
>
> +   if (cfg->vreg_enable_load) {
> +   for (i = cfg->num_vregs - 1; i >= 0; --i)
> +   regulator_set_load(qmp->vregs[i].consumer, 
> cfg->vreg_enable_load[i]);
> +   }
> +
> /* turn on regulator supplies */
> ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
> if (ret) {
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>


-- 
With best wishes
Dmitry

[PATCH v4 3/3] drm/msm/dp: delete vdda regulator related functions from eDP/DP controller

Vdda regulators are related to both eDP and DP phy so that it should be
managed at eDP and DP phy driver instead of controller. This patch removes
vdda regulators related functions out of eDP/DP controller.

Signed-off-by: Kuogee Hsieh 
Reviewed-by: Stephen Boyd 
---
 drivers/gpu/drm/msm/dp/dp_parser.c | 14 --
 drivers/gpu/drm/msm/dp/dp_parser.h |  6 ---
 drivers/gpu/drm/msm/dp/dp_power.c  | 95 +-
 3 files changed, 2 insertions(+), 113 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
b/drivers/gpu/drm/msm/dp/dp_parser.c
index 8f9fed9..4ef2130 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -22,14 +22,6 @@
 #define DP_DEFAULT_P0_OFFSET   0x1000
 #define DP_DEFAULT_P0_SIZE 0x0400
 
-static const struct dp_regulator_cfg sdm845_dp_reg_cfg = {
-   .num = 2,
-   .regs = {
-   {"vdda-1p2", 21800, 4 },/* 1.2 V */
-   {"vdda-0p9", 36000, 32 },   /* 0.9 V */
-   },
-};
-
 static void __iomem *dp_ioremap(struct platform_device *pdev, int idx, size_t 
*len)
 {
struct resource *res;
@@ -298,12 +290,6 @@ static int dp_parser_parse(struct dp_parser *parser)
if (rc)
return rc;
 
-   /* Map the corresponding regulator information according to
-* version. Currently, since we only have one supported platform,
-* mapping the regulator directly.
-*/
-   parser->regulator_cfg = _dp_reg_cfg;
-
return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h 
b/drivers/gpu/drm/msm/dp/dp_parser.h
index 3a4d797..b56b4d7 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -101,11 +101,6 @@ struct dp_reg_entry {
int disable_load;
 };
 
-struct dp_regulator_cfg {
-   int num;
-   struct dp_reg_entry regs[DP_DEV_REGULATOR_MAX];
-};
-
 /**
  * struct dp_parser - DP parser's data exposed to clients
  *
@@ -121,7 +116,6 @@ struct dp_parser {
struct dp_pinctrl pinctrl;
struct dp_io io;
struct dp_display_data disp_data;
-   const struct dp_regulator_cfg *regulator_cfg;
u32 max_dp_lanes;
struct drm_bridge *next_bridge;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_power.c 
b/drivers/gpu/drm/msm/dp/dp_power.c
index d9e0117..b52ac1d 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.c
+++ b/drivers/gpu/drm/msm/dp/dp_power.c
@@ -20,82 +20,10 @@ struct dp_power_private {
struct clk *link_clk_src;
struct clk *pixel_provider;
struct clk *link_provider;
-   struct regulator_bulk_data supplies[DP_DEV_REGULATOR_MAX];
 
struct dp_power dp_power;
 };
 
-static void dp_power_regulator_disable(struct dp_power_private *power)
-{
-   struct regulator_bulk_data *s = power->supplies;
-   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
-   int num = power->parser->regulator_cfg->num;
-   int i;
-
-   DBG("");
-   for (i = num - 1; i >= 0; i--)
-   if (regs[i].disable_load >= 0)
-   regulator_set_load(s[i].consumer,
-  regs[i].disable_load);
-
-   regulator_bulk_disable(num, s);
-}
-
-static int dp_power_regulator_enable(struct dp_power_private *power)
-{
-   struct regulator_bulk_data *s = power->supplies;
-   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
-   int num = power->parser->regulator_cfg->num;
-   int ret, i;
-
-   DBG("");
-   for (i = 0; i < num; i++) {
-   if (regs[i].enable_load >= 0) {
-   ret = regulator_set_load(s[i].consumer,
-regs[i].enable_load);
-   if (ret < 0) {
-   pr_err("regulator %d set op mode failed, %d\n",
-   i, ret);
-   goto fail;
-   }
-   }
-   }
-
-   ret = regulator_bulk_enable(num, s);
-   if (ret < 0) {
-   pr_err("regulator enable failed, %d\n", ret);
-   goto fail;
-   }
-
-   return 0;
-
-fail:
-   for (i--; i >= 0; i--)
-   regulator_set_load(s[i].consumer, regs[i].disable_load);
-   return ret;
-}
-
-static int dp_power_regulator_init(struct dp_power_private *power)
-{
-   struct regulator_bulk_data *s = power->supplies;
-   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
-   struct platform_device *pdev = power->pdev;
-   int num = power->parser->regulator_cfg->num;
-   int i, ret;
-
-   for (i = 0; i < num; i++)
-   s[i].supply = regs[i].name;
-
-   ret = devm_regulator_bulk_get(>dev, num, s);
-   if (ret < 0) {
-   pr_err("%s: failed to init regulator, ret=%d\n",
-   __func__, ret);
-   return

[PATCH v4 2/3] phy/qualcomm: add regulator_set_load to dp phy

This patch add regulator_set_load() before enable regulator at
DP phy driver.

Signed-off-by: Kuogee Hsieh 
Reviewed-by: Stephen Boyd 
---
 drivers/phy/qualcomm/phy-qcom-qmp.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
b/drivers/phy/qualcomm/phy-qcom-qmp.c
index b144ae1..20ac446 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -3130,6 +3130,7 @@ struct qmp_phy_cfg {
int num_resets;
/* regulators to be requested */
const char * const *vreg_list;
+   const unsigned int *vreg_enable_load;
int num_vregs;
 
/* array of registers with different offsets */
@@ -3346,6 +3347,10 @@ static const char * const qmp_phy_vreg_l[] = {
"vdda-phy", "vdda-pll",
 };
 
+static const unsigned int qmp_phy_vreg_enable_load[] = {
+   21800, 36000
+};
+
 static const struct qmp_phy_cfg ipq8074_usb3phy_cfg = {
.type   = PHY_TYPE_USB3,
.nlanes = 1,
@@ -4072,6 +4077,7 @@ static const struct qmp_phy_cfg sm8250_usb3phy_cfg = {
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
.vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,
.num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
.regs   = qmp_v4_usb3phy_regs_layout,
 
@@ -4139,6 +4145,7 @@ static const struct qmp_phy_cfg sm8250_dpphy_cfg = {
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
.vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,
.num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
.regs   = qmp_v4_usb3phy_regs_layout,
 
@@ -5008,6 +5015,11 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)
return 0;
}
 
+   if (cfg->vreg_enable_load) {
+   for (i = cfg->num_vregs - 1; i >= 0; --i)
+   regulator_set_load(qmp->vregs[i].consumer, 
cfg->vreg_enable_load[i]);
+   }
+
/* turn on regulator supplies */
ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
if (ret) {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v4 1/3] phy/qualcomm: add regulator_set_load to edp phy

This patch add regulator_set_load() before enable regulator at
eDP phy driver.

Signed-off-by: Kuogee Hsieh 
---
 drivers/phy/qualcomm/phy-qcom-edp.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c 
b/drivers/phy/qualcomm/phy-qcom-edp.c
index cacd32f..fbe0be0 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -87,14 +87,20 @@ struct qcom_edp {
 
struct clk_bulk_data clks[2];
struct regulator_bulk_data supplies[2];
+   int enable_load[2];
 };
 
 static int qcom_edp_phy_init(struct phy *phy)
 {
struct qcom_edp *edp = phy_get_drvdata(phy);
int ret;
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;
 
-   ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
+   for (i = num_consumers - 1; i >= 0; --i)
+   regulator_set_load(edp->supplies[i].consumer, 
edp->enable_load[i]);
+
+   ret = regulator_bulk_enable(num_consumers, edp->supplies);
if (ret)
return ret;
 
@@ -635,6 +641,8 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
 
edp->supplies[0].supply = "vdda-phy";
edp->supplies[1].supply = "vdda-pll";
+   edp->enable_load[0] = 21800;/* load for 1.2 V vdda-phy supply */
+   edp->enable_load[1] = 36000;/* load for 1.2 V vdda-pll supply */
ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), 
edp->supplies);
if (ret)
return ret;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v4 0/3] eDP/DP Phy vdda realted function

1) add regulator_set_load() to eDP phy
2) add regulator_set_load() to DP phy
3) remove vdda related function out of eDP/DP controller

Kuogee Hsieh (3):
  phy/qualcomm: add regulator_set_load to edp phy
  phy/qualcomm: add regulator_set_load to dp phy
  drm/msm/dp: delete vdda regulator related functions from eDP/DP
controller

 drivers/gpu/drm/msm/dp/dp_parser.c  | 14 --
 drivers/gpu/drm/msm/dp/dp_parser.h  |  6 ---
 drivers/gpu/drm/msm/dp/dp_power.c   | 95 +
 drivers/phy/qualcomm/phy-qcom-edp.c | 10 +++-
 drivers/phy/qualcomm/phy-qcom-qmp.c | 12 +
 5 files changed, 23 insertions(+), 114 deletions(-)

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

Re: [PATCH v3 3/3] drm/msm/dp: delete vdda regulator related functions from eDP/DP controller

Quoting Kuogee Hsieh (2022-05-18 13:24:04)
> Vdda regulators are related to both eDP and DP phy so that it should be
> managed at eDP and DP phy driver instead of controller. This patch removes
> vdda regulators related functions out of eDP/DP controller.
>
> Signed-off-by: Kuogee Hsieh 
> ---

Reviewed-by: Stephen Boyd

[pull] amdgpu, amdkfd drm-next-5.19

2022-05-18 Thread Alex Deucher

Hi Dave, Daniel,

Stuff for 5.19.  A bit late for new stuff, but it's just additional enablement
for new IPs so they shouldn't affect existing parts.  The rest is just the usual
fixes.

The following changes since commit 81c5495910e81c2cadcb9118ca0c8803ab3bde61:

  drm/amdgpu: Remove duplicated argument in vcn_v4_0 (2022-05-10 17:53:13 -0400)

are available in the Git repository at:

  https://gitlab.freedesktop.org/agd5f/linux.git 
tags/amd-drm-next-5.19-2022-05-18

for you to fetch changes up to 0223e516470aa0589da6c03e6d177c10594cabbd:

  drm/amd: Don't reset dGPUs if the system is going to s2idle (2022-05-18 
15:20:18 -0400)


amd-drm-next-5.19-2022-05-18:

amdgpu:
- Misc code cleanups
- Additional SMU 13.x enablement
- Smartshift fixes
- GFX11 fixes
- Support for SMU 13.0.4
- SMU mutex fix
- Suspend/resume fix

amdkfd:
- static checker fix
- Doorbell/MMIO resource handling fix


Alex Deucher (1):
  drm/amdgpu/ctx: only reset stable pstate if the user changed it (v2)

Graham Sider (1):
  drm/amdkfd: Fix static checker warning on MES queue type

Hans de Goede (1):
  drm/amdgpu: Move mutex_init(>message_lock) to smu_early_init()

Haohui Mai (1):
  drm/amdgpu: Ensure the DMA engine is deactivated during set ups

Huang Rui (1):
  drm/amdgpu/pm: add smu v13.0.4 driver SMU if headers

Jack Xiao (2):
  drm/amdgpu/gfx11: fix me field handling in map_queue packet
  drm/amdgpu/gfx11: fix mes mqd settings

Jiapeng Chong (2):
  drm/amdgpu: clean up some inconsistent indenting
  drm/amd/display: clean up some inconsistent indenting

Lang Yu (1):
  drm/amdkfd: allocate MMIO/DOORBELL BOs with AMDGPU_GEM_CREATE_PREEMPTIBLE

Luben Tuikov (1):
  drm/amdgpu: Unmap legacy queue when MES is enabled

Mario Limonciello (1):
  drm/amd: Don't reset dGPUs if the system is going to s2idle

Sathishkumar S (4):
  drm/amd/pm: support ss metrics read for smu11
  drm/amd/pm: update smartshift powerboost calc for smu12
  drm/amd/pm: update smartshift powerboost calc for smu13
  drm/amd/pm: consistent approach for smartshift

Tim Huang (5):
  drm/amdgpu/pm: add EnableGfxImu message dummy map for SMU IP v13.0.4
  drm/amdgpu/pm: add some common ppt functions for SMU IP v13.0.x
  drm/amdgpu/pm: add swsmu ppt implementation for SMU IP v13.0.4
  drm/amdgpu/pm: enable swsmu for SMU IP v13.0.4
  drm/amdgpu/pm: add GFXOFF control IP version check for SMU IP v13.0.4

Xiaojian Du (1):
  drm/amdgpu/discovery: add SMU v13.0.4 into the IP discovery list

Yang Wang (4):
  drm/amd/pm: add smu pp_feature_mask callback for smu_v13_0_7
  drm/amd/pm: add smu feature map support for smu_v13_0_7
  drm/amd/pm: add smu feature map support for smu_v13_0_0
  drm/amd/pm: add smu power_limit callback for smu_v13_0_7

 drivers/gpu/drm/amd/amdgpu/amdgpu.h|2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c   |   14 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c   |   32 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c|5 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c  |1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|2 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c |2 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c |9 +-
 drivers/gpu/drm/amd/amdgpu/mes_v11_0.c |   20 +-
 drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c |   17 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c |  109 +-
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  |7 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |7 +-
 drivers/gpu/drm/amd/pm/amdgpu_pm.c |  130 ++-
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c  |8 +-
 .../inc/pmfw_if/smu11_driver_if_sienna_cichlid.h   |   63 ++
 .../pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_4.h |  267 +
 .../amd/pm/swsmu/inc/pmfw_if/smu_v13_0_4_pmfw.h|  137 +++
 .../amd/pm/swsmu/inc/pmfw_if/smu_v13_0_4_ppsmc.h   |  138 +++
 drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h   |   26 +-
 drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h   |9 +-
 .../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c|  161 ++-
 drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c|   60 +-
 drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile|3 +-
 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c |  107 +-
 .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c   |   69 +-
 .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_4_ppt.c   | 1044 
 .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_4_ppt.h   |   28 +
 .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c   |   99 +-
 .../gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c   |   62 +-
 30 files changed, 2364 insertions(+), 274 deletions(-)
 create mode 100644 
drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_4.h
 create mode 100644

Re: [PATCH 1/1] drm/panfrost: Add support for devcoredump

2022-05-18 Thread Rob Clark

On Tue, May 17, 2022 at 10:42 AM Adrián Larumbe
 wrote:
>
> In the event of a job timeout, debug dump information will be written into
> /sys/class/devcoredump.
>
> Inspired by etnaviv's similar feature.
>
> Signed-off-by: Adrián Larumbe 
> ---
>  drivers/gpu/drm/panfrost/Kconfig |   1 +
>  drivers/gpu/drm/panfrost/Makefile|   3 +-
>  drivers/gpu/drm/panfrost/panfrost_dump.c | 198 +++
>  drivers/gpu/drm/panfrost/panfrost_dump.h |  12 ++
>  drivers/gpu/drm/panfrost/panfrost_job.c  |   3 +
>  include/uapi/drm/panfrost_drm.h  |  32 
>  6 files changed, 248 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/panfrost/panfrost_dump.c
>  create mode 100644 drivers/gpu/drm/panfrost/panfrost_dump.h
>
> diff --git a/drivers/gpu/drm/panfrost/Kconfig 
> b/drivers/gpu/drm/panfrost/Kconfig
> index 86cdc0ce79e6..079600328be1 100644
> --- a/drivers/gpu/drm/panfrost/Kconfig
> +++ b/drivers/gpu/drm/panfrost/Kconfig
> @@ -11,6 +11,7 @@ config DRM_PANFROST
> select DRM_GEM_SHMEM_HELPER
> select PM_DEVFREQ
> select DEVFREQ_GOV_SIMPLE_ONDEMAND
> +   select WANT_DEV_COREDUMP
> help
>   DRM driver for ARM Mali Midgard (T6xx, T7xx, T8xx) and
>   Bifrost (G3x, G5x, G7x) GPUs.
> diff --git a/drivers/gpu/drm/panfrost/Makefile 
> b/drivers/gpu/drm/panfrost/Makefile
> index b71935862417..7da2b3f02ed9 100644
> --- a/drivers/gpu/drm/panfrost/Makefile
> +++ b/drivers/gpu/drm/panfrost/Makefile
> @@ -9,6 +9,7 @@ panfrost-y := \
> panfrost_gpu.o \
> panfrost_job.o \
> panfrost_mmu.o \
> -   panfrost_perfcnt.o
> +   panfrost_perfcnt.o \
> +   panfrost_dump.o
>
>  obj-$(CONFIG_DRM_PANFROST) += panfrost.o
> diff --git a/drivers/gpu/drm/panfrost/panfrost_dump.c 
> b/drivers/gpu/drm/panfrost/panfrost_dump.c
> new file mode 100644
> index ..a76dcf4acf6f
> --- /dev/null
> +++ b/drivers/gpu/drm/panfrost/panfrost_dump.c
> @@ -0,0 +1,198 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright 2021 Collabora ltd. */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "panfrost_job.h"
> +#include "panfrost_gem.h"
> +#include "panfrost_regs.h"
> +#include "panfrost_dump.h"
> +#include "panfrost_device.h"
> +
> +static bool panfrost_dump_core = true;
> +module_param_named(dump_core, panfrost_dump_core, bool, 0600);
> +
> +struct panfrost_dump_iterator {
> +   void *start;
> +   struct panfrost_dump_object_header *hdr;
> +   void *data;
> +};
> +
> +static const unsigned short panfrost_dump_registers[] = {
> +   GPU_ID,
> +   GPU_L2_FEATURES,
> +   GPU_CORE_FEATURES,
> +   GPU_TILER_FEATURES,
> +   GPU_MEM_FEATURES,
> +   GPU_MMU_FEATURES,
> +   GPU_AS_PRESENT,
> +   GPU_JS_PRESENT,
> +   GPU_INT_RAWSTAT,
> +   GPU_INT_CLEAR,
> +   GPU_INT_MASK,
> +   GPU_INT_STAT,
> +};
> +
> +static void panfrost_core_dump_header(struct panfrost_dump_iterator *iter,
> +   u32 type, void *data_end)
> +{
> +   struct panfrost_dump_object_header *hdr = iter->hdr;
> +
> +   hdr->magic = cpu_to_le32(PANFROSTDUMP_MAGIC);
> +   hdr->type = cpu_to_le32(type);
> +   hdr->file_offset = cpu_to_le32(iter->data - iter->start);
> +   hdr->file_size = cpu_to_le32(data_end - iter->data);
> +
> +   iter->hdr++;
> +   iter->data += le32_to_cpu(hdr->file_size);
> +}
> +
> +static void
> +panfrost_core_dump_registers(struct panfrost_dump_iterator *iter,
> +struct panfrost_device *pfdev)
> +{
> +   struct panfrost_dump_registers *reg = iter->data;
> +   unsigned int i;
> +
> +   for (i = 0; i < ARRAY_SIZE(panfrost_dump_registers); i++, reg++) {
> +   reg->reg = cpu_to_le32(panfrost_dump_registers[i]);
> +   reg->value = cpu_to_le32(gpu_read(pfdev, 
> panfrost_dump_registers[i]));
> +   }
> +
> +   panfrost_core_dump_header(iter, PANFROSTDUMP_BUF_REG, reg);
> +}
> +
> +void panfrost_core_dump(struct panfrost_job *job)
> +{
> +   struct panfrost_device *pfdev = job->pfdev;
> +   struct panfrost_dump_iterator iter;
> +   struct drm_gem_object *dbo;
> +   unsigned int n_obj, n_bomap_pages;
> +   __le64 *bomap, *bomap_start;
> +   size_t file_size;
> +   int ret, i;
> +
> +   /* Only catch the first event, or when manually re-armed */
> +   if (!panfrost_dump_core)
> +   return;
> +   panfrost_dump_core = false;
> +
> +   /* At least, we dump registers and end marker */
> +   n_obj = 2;
> +   n_bomap_pages = 0;
> +   file_size = ARRAY_SIZE(panfrost_dump_registers) *
> +   sizeof(struct panfrost_dump_registers);
> +
> +   /* Add in the active buffer objects */
> +   for (i = 0; i < job->bo_count; i++) {
> +   dbo = job->bos[i];
> +   file_size += dbo->size;
> +   n_bomap_pages +=

Re: [PATCH v3 2/3] phy/qualcomm: add regulator_set_load to dp phy

Quoting Kuogee Hsieh (2022-05-18 13:24:03)
> This patch add regulator_set_load() before enable regulator at
> DP phy driver.
>
> Changes in v2:
> -- no regulator_set_load() before disable regulator
>
> Changes in v3:
> --  split into two patches

Same changelog comment

>
> Signed-off-by: Kuogee Hsieh 
> ---

Reviewed-by: Stephen Boyd

Re: [PATCH v3 1/3] phy/qualcomm: add regulator_set_load to edp phy

Should subject be "phy: qcom:" prefix?

Quoting Kuogee Hsieh (2022-05-18 13:24:02)
> This patch add regulator_set_load() before enable regulator at
> eDP phy driver.
>
> Changes in v3:
> -- no regulator_set_load before disable regulator
> -- no supply name string change at probe
> -- split into two patches

These don't go here because this isn't a drm patch.

>
> Signed-off-by: Kuogee Hsieh 
> ---
>  drivers/phy/qualcomm/phy-qcom-edp.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c 
> b/drivers/phy/qualcomm/phy-qcom-edp.c
> index cacd32f..00b6726 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -635,6 +641,8 @@ static int qcom_edp_phy_probe(struct platform_device 
> *pdev)
>
> edp->supplies[0].supply = "vdda-phy";
> edp->supplies[1].supply = "vdda-pll";
> +   edp->enable_load[0] = 21800;/* 1.2 V */
> +   edp->enable_load[1] = 36000;/* 1.2 V */

What does the comment mean? This is the load for 1.2V supply? Can we
have that sort of comment instead of "1.2 V"?

> ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), 
> edp->supplies);
> if (ret)

[PATCH v3 2/3] phy/qualcomm: add regulator_set_load to dp phy

This patch add regulator_set_load() before enable regulator at
DP phy driver.

Changes in v2:
-- no regulator_set_load() before disable regulator

Changes in v3:
--  split into two patches

Signed-off-by: Kuogee Hsieh 
---
 drivers/phy/qualcomm/phy-qcom-qmp.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
b/drivers/phy/qualcomm/phy-qcom-qmp.c
index b144ae1..20ac446 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -3130,6 +3130,7 @@ struct qmp_phy_cfg {
int num_resets;
/* regulators to be requested */
const char * const *vreg_list;
+   const unsigned int *vreg_enable_load;
int num_vregs;
 
/* array of registers with different offsets */
@@ -3346,6 +3347,10 @@ static const char * const qmp_phy_vreg_l[] = {
"vdda-phy", "vdda-pll",
 };
 
+static const unsigned int qmp_phy_vreg_enable_load[] = {
+   21800, 36000
+};
+
 static const struct qmp_phy_cfg ipq8074_usb3phy_cfg = {
.type   = PHY_TYPE_USB3,
.nlanes = 1,
@@ -4072,6 +4077,7 @@ static const struct qmp_phy_cfg sm8250_usb3phy_cfg = {
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
.vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,
.num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
.regs   = qmp_v4_usb3phy_regs_layout,
 
@@ -4139,6 +4145,7 @@ static const struct qmp_phy_cfg sm8250_dpphy_cfg = {
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
.vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,
.num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
.regs   = qmp_v4_usb3phy_regs_layout,
 
@@ -5008,6 +5015,11 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)
return 0;
}
 
+   if (cfg->vreg_enable_load) {
+   for (i = cfg->num_vregs - 1; i >= 0; --i)
+   regulator_set_load(qmp->vregs[i].consumer, 
cfg->vreg_enable_load[i]);
+   }
+
/* turn on regulator supplies */
ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
if (ret) {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 3/3] drm/msm/dp: delete vdda regulator related functions from eDP/DP controller

Vdda regulators are related to both eDP and DP phy so that it should be
managed at eDP and DP phy driver instead of controller. This patch removes
vdda regulators related functions out of eDP/DP controller.

Signed-off-by: Kuogee Hsieh 
---
 drivers/gpu/drm/msm/dp/dp_parser.c | 14 --
 drivers/gpu/drm/msm/dp/dp_parser.h |  6 ---
 drivers/gpu/drm/msm/dp/dp_power.c  | 95 +-
 3 files changed, 2 insertions(+), 113 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
b/drivers/gpu/drm/msm/dp/dp_parser.c
index 8f9fed9..4ef2130 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -22,14 +22,6 @@
 #define DP_DEFAULT_P0_OFFSET   0x1000
 #define DP_DEFAULT_P0_SIZE 0x0400
 
-static const struct dp_regulator_cfg sdm845_dp_reg_cfg = {
-   .num = 2,
-   .regs = {
-   {"vdda-1p2", 21800, 4 },/* 1.2 V */
-   {"vdda-0p9", 36000, 32 },   /* 0.9 V */
-   },
-};
-
 static void __iomem *dp_ioremap(struct platform_device *pdev, int idx, size_t 
*len)
 {
struct resource *res;
@@ -298,12 +290,6 @@ static int dp_parser_parse(struct dp_parser *parser)
if (rc)
return rc;
 
-   /* Map the corresponding regulator information according to
-* version. Currently, since we only have one supported platform,
-* mapping the regulator directly.
-*/
-   parser->regulator_cfg = _dp_reg_cfg;
-
return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h 
b/drivers/gpu/drm/msm/dp/dp_parser.h
index 3a4d797..b56b4d7 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -101,11 +101,6 @@ struct dp_reg_entry {
int disable_load;
 };
 
-struct dp_regulator_cfg {
-   int num;
-   struct dp_reg_entry regs[DP_DEV_REGULATOR_MAX];
-};
-
 /**
  * struct dp_parser - DP parser's data exposed to clients
  *
@@ -121,7 +116,6 @@ struct dp_parser {
struct dp_pinctrl pinctrl;
struct dp_io io;
struct dp_display_data disp_data;
-   const struct dp_regulator_cfg *regulator_cfg;
u32 max_dp_lanes;
struct drm_bridge *next_bridge;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_power.c 
b/drivers/gpu/drm/msm/dp/dp_power.c
index d9e0117..b52ac1d 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.c
+++ b/drivers/gpu/drm/msm/dp/dp_power.c
@@ -20,82 +20,10 @@ struct dp_power_private {
struct clk *link_clk_src;
struct clk *pixel_provider;
struct clk *link_provider;
-   struct regulator_bulk_data supplies[DP_DEV_REGULATOR_MAX];
 
struct dp_power dp_power;
 };
 
-static void dp_power_regulator_disable(struct dp_power_private *power)
-{
-   struct regulator_bulk_data *s = power->supplies;
-   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
-   int num = power->parser->regulator_cfg->num;
-   int i;
-
-   DBG("");
-   for (i = num - 1; i >= 0; i--)
-   if (regs[i].disable_load >= 0)
-   regulator_set_load(s[i].consumer,
-  regs[i].disable_load);
-
-   regulator_bulk_disable(num, s);
-}
-
-static int dp_power_regulator_enable(struct dp_power_private *power)
-{
-   struct regulator_bulk_data *s = power->supplies;
-   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
-   int num = power->parser->regulator_cfg->num;
-   int ret, i;
-
-   DBG("");
-   for (i = 0; i < num; i++) {
-   if (regs[i].enable_load >= 0) {
-   ret = regulator_set_load(s[i].consumer,
-regs[i].enable_load);
-   if (ret < 0) {
-   pr_err("regulator %d set op mode failed, %d\n",
-   i, ret);
-   goto fail;
-   }
-   }
-   }
-
-   ret = regulator_bulk_enable(num, s);
-   if (ret < 0) {
-   pr_err("regulator enable failed, %d\n", ret);
-   goto fail;
-   }
-
-   return 0;
-
-fail:
-   for (i--; i >= 0; i--)
-   regulator_set_load(s[i].consumer, regs[i].disable_load);
-   return ret;
-}
-
-static int dp_power_regulator_init(struct dp_power_private *power)
-{
-   struct regulator_bulk_data *s = power->supplies;
-   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
-   struct platform_device *pdev = power->pdev;
-   int num = power->parser->regulator_cfg->num;
-   int i, ret;
-
-   for (i = 0; i < num; i++)
-   s[i].supply = regs[i].name;
-
-   ret = devm_regulator_bulk_get(>dev, num, s);
-   if (ret < 0) {
-   pr_err("%s: failed to init regulator, ret=%d\n",
-   __func__, ret);
-   return ret;
-   }
-
-

[PATCH v3 1/3] phy/qualcomm: add regulator_set_load to edp phy

This patch add regulator_set_load() before enable regulator at
eDP phy driver.

Changes in v3:
-- no regulator_set_load before disable regulator
-- no supply name string change at probe
-- split into two patches

Signed-off-by: Kuogee Hsieh 
---
 drivers/phy/qualcomm/phy-qcom-edp.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c 
b/drivers/phy/qualcomm/phy-qcom-edp.c
index cacd32f..00b6726 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -87,14 +87,20 @@ struct qcom_edp {
 
struct clk_bulk_data clks[2];
struct regulator_bulk_data supplies[2];
+   int enable_load[2];
 };
 
 static int qcom_edp_phy_init(struct phy *phy)
 {
struct qcom_edp *edp = phy_get_drvdata(phy);
int ret;
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;
 
-   ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
+   for (i = num_consumers - 1; i >= 0; --i)
+   regulator_set_load(edp->supplies[i].consumer, 
edp->enable_load[i]);
+
+   ret = regulator_bulk_enable(num_consumers, edp->supplies);
if (ret)
return ret;
 
@@ -635,6 +641,8 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
 
edp->supplies[0].supply = "vdda-phy";
edp->supplies[1].supply = "vdda-pll";
+   edp->enable_load[0] = 21800;/* 1.2 V */
+   edp->enable_load[1] = 36000;/* 1.2 V */
ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), 
edp->supplies);
if (ret)
return ret;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 0/3] eDP/DP Phy vdda realted function

1) add regulator_set_load() to eDP phy
2) add regulator_set_load() to DP phy
3) remove vdda related function out of eDP/DP controller

Kuogee Hsieh (3):
  phy/qualcomm: add regulator_set_load to edp phy
  phy/qualcomm: add regulator_set_load to dp phy
  drm/msm/dp: delete vdda regulator related functions from eDP/DP
controller

 drivers/gpu/drm/msm/dp/dp_parser.c  | 14 --
 drivers/gpu/drm/msm/dp/dp_parser.h  |  6 ---
 drivers/gpu/drm/msm/dp/dp_power.c   | 95 +
 drivers/phy/qualcomm/phy-qcom-edp.c | 10 +++-
 drivers/phy/qualcomm/phy-qcom-qmp.c | 12 +
 5 files changed, 23 insertions(+), 114 deletions(-)

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

[pull] amdgpu drm-fixes-5.18

2022-05-18 Thread Alex Deucher

Hi Dave, Daniel,

Just one suspend/resume regression fix.

The following changes since commit 42226c989789d8da4af1de0c31070c96726d990c:

  Linux 5.18-rc7 (2022-05-15 18:08:58 -0700)

are available in the Git repository at:

  https://gitlab.freedesktop.org/agd5f/linux.git 
tags/amd-drm-fixes-5.18-2022-05-18

for you to fetch changes up to 7123d39dc24dcd21ff23d75f46f926b15269b9da:

  drm/amd: Don't reset dGPUs if the system is going to s2idle (2022-05-18 
15:24:35 -0400)


amd-drm-fixes-5.18-2022-05-18:

amdgpu:
- Suspend/resume regression fix


Mario Limonciello (1):
  drm/amd: Don't reset dGPUs if the system is going to s2idle

 drivers/gpu/drm/amd/amdgpu/amdgpu.h  |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 14 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  |  2 +-
 3 files changed, 17 insertions(+), 1 deletion(-)

Re: [PATCH v2 0/2] eDP/DP Phy vdda realted function




On 5/18/2022 10:31 AM, Dmitry Baryshkov wrote:

On 18/05/2022 20:29, Kuogee Hsieh wrote:


On 5/18/2022 10:16 AM, Dmitry Baryshkov wrote:
On Wed, 18 May 2022 at 19:43, Kuogee Hsieh  
wrote:

1) add regulator_set_load() to eDP/DP phy
2) remove vdda related function out of eDP/DP controller

These patches touch two subsystems and have a dependency between them.
How do we merge them?


currently, both phy and controller are vote for regulator. The last 
vote will just increase count.


Therefore the dependency should be very loose.


So, do you propose to merge dp change a cycle after the phy changes go 
in?



yes,




Kuogee Hsieh (2):
   phy/qcom: add regulator_set_load to edp/dp phy
   drm/msm/dp: delete vdda regulator related functions from eDP/DP
 controller

  drivers/gpu/drm/msm/dp/dp_parser.c  | 14 --
  drivers/gpu/drm/msm/dp/dp_parser.h  |  6 ---
  drivers/gpu/drm/msm/dp/dp_power.c   | 95 
+

  drivers/phy/qualcomm/phy-qcom-edp.c | 25 --
  drivers/phy/qualcomm/phy-qcom-qmp.c | 13 +
  5 files changed, 36 insertions(+), 117 deletions(-)

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

a Linux Foundation Collaborative Project

Re: [PATCH] drm/msm/dpu: handle pm_runtime_get_sync() errors in bind path

Quoting Abhinav Kumar (2022-05-18 12:55:40)
> If there are errors while trying to enable the pm in the
> bind path, it will lead to unclocked access of hw revision
> register thereby crashing the device.
>
> This will not address why the pm_runtime_get_sync() fails
> but at the very least we should be able to prevent the
> crash by handling the error and bailing out earlier.
>
> Signed-off-by: Abhinav Kumar 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 2b9d931474e0..2fd1f5b70a06 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1089,7 +1089,11 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>
> dpu_kms_parse_data_bus_icc_path(dpu_kms);
>
> -   pm_runtime_get_sync(_kms->pdev->dev);
> +   rc = pm_runtime_get_sync(_kms->pdev->dev);

Any reason to not use pm_runtime_resume_and_get()?

> +   if (rc < 0) {
> +   pm_runtime_put_autosuspend(_kms->pdev->dev);
> +   goto error;

Then this is a single goto error

[PATCH] drm/msm/dpu: handle pm_runtime_get_sync() errors in bind path

2022-05-18 Thread Abhinav Kumar

If there are errors while trying to enable the pm in the
bind path, it will lead to unclocked access of hw revision
register thereby crashing the device.

This will not address why the pm_runtime_get_sync() fails
but at the very least we should be able to prevent the
crash by handling the error and bailing out earlier.

Signed-off-by: Abhinav Kumar 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 2b9d931474e0..2fd1f5b70a06 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1089,7 +1089,11 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
 
dpu_kms_parse_data_bus_icc_path(dpu_kms);
 
-   pm_runtime_get_sync(_kms->pdev->dev);
+   rc = pm_runtime_get_sync(_kms->pdev->dev);
+   if (rc < 0) {
+   pm_runtime_put_autosuspend(_kms->pdev->dev);
+   goto error;
+   }
 
dpu_kms->core_rev = readl_relaxed(dpu_kms->mmio + 0x0);
 
-- 
2.35.1

Re: [PATCH] drm/msm/dpu: limit writeback modes according to max_linewidth


On 14/05/2022 01:59, Abhinav Kumar wrote:

Writeback modes were being added according to mode_config.max_width
but this is assigned to double of max_mixer_width.

For compositors/clients using a single SSPP, this will fail
the dpu_plane's atomic check as it checks for max_linewidth.

Limit writeback modes according to max_linewidth to allow
even compositors/clients which use only a single SSPP to
use writeback.

Fixes: 77b001acdcfeb ("drm/msm/dpu: add the writeback connector layer")
Reported-by: Jessica Zhang 
Signed-off-by: Abhinav Kumar 


Reviewed-by: Dmitry Baryshkov 


---
  drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
index 7620ffe55779..399115e4e217 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
@@ -8,8 +8,10 @@
  static int dpu_wb_conn_get_modes(struct drm_connector *connector)
  {
struct drm_device *dev = connector->dev;
+   struct msm_drm_private *priv = dev->dev_private;
+   struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
  
-	return drm_add_modes_noedid(connector, dev->mode_config.max_width,

+   return drm_add_modes_noedid(connector, 
dpu_kms->catalog->caps->max_linewidth,
dev->mode_config.max_height);
  }
  



--
With best wishes
Dmitry

RE: [PATCH] video: hyperv_fb: Allow resolutions with size > 64 MB for Gen1

2022-05-18 Thread Dexuan Cui

> From: Dexuan Cui 
> Sent: Wednesday, May 4, 2022 10:05 AM
> To: Haiyang Zhang ; Wei Liu ;
> > ...
> > When I initially implemented this driver 10 years ago, I believe there
> > was smaller limit for the fb... But I think this patch is good for the
> > newer MMIO alloc scheme. I hope to see reviews also from
> >  @Dexuan Cui @Michael Kelley (LINUX) who are more familiar with
> > the PCI/BAR/MMIO area.
> >
> > Thanks,
> > - Haiyang
> 
> The patch looks good to me but I suggest we check with the Hyper-V
> team to figure out how a Gen1 Windows VM supports a higher
> resolution that needs a VRAM size of more than 64MB. Just in case we
> miss something..
> 
> Thanks,
> -- Dexuan

Reviewed-by: Dexuan Cui 

Saurabh checked this with Hyper-V team, who said there is no
Generation 1-specific block for larger VRAM sizes in Windows VM.

When the driver was originally developed, we didn't have the API
vmbus_allocate_mmio(), and I guess we just used the PCI device's BAR
address for simplicity, and didn't realize the restriction with very high
resolutions that require >64 MB VRAM. It looks like the synthetic
VMBus framebuffer device doesn't have to use the same MMIO range
used by the Hyper-V legacy PCI framebuffer device, so the patch
looks good to me.

BTW, please check the hyperv-drm driver as well:
drivers/gpu/drm/hyperv/hyperv_drm_drv.c
I think we should make the same change there to support 7680x4320
for Gen1 VMs.

Thanks,
Dexuan

Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers

2022-05-18 Thread Michal Suchánek

Hello,

On Wed, May 18, 2022 at 08:30:06PM +0200, Thomas Zimmermann wrote:
> Open Firmware provides basic display output via the 'display' node.
> DT platform code already provides a device that represents the node's
> framebuffer. Add a DRM driver for the device. The display mode and
> color format is pre-initialized by the system's firmware. Runtime
> modesetting via DRM is not possible. The display is useful during
> early boot stages or as error fallback.
> 
> Similar functionality is already provided by fbdev's offb driver,
> which is insufficient for modern userspace. The old driver includes
> support for BootX device tree, which can be found on old 32-bit
> PowerPC Macintosh systems. If these are still in use, the
> functionality can be added to ofdrm or implemented in a new
> driver. As with simepldrm, the fbdev driver cannot be selected is
> ofdrm is already enabled.
> 
> Two noteable points about the driver:
> 
>  * Reading the framebuffer aperture from the device tree is not
> reliable on all systems. Ofdrm takes the heuristics and a comment
> from offb to pick the correct range.
> 
>  * No resource management may be tied to the underlying PCI device.
> Otherwise the handover to the native driver will fail with a resource
> conflict. PCI management is therefore done as part of the platform
> device's cleanup.
> 
> The driver has been tested on qemu's ppc64le emulation. The device
> hand-over has been tested with bochs.
> 
> Signed-off-by: Thomas Zimmermann 
> ---
>  MAINTAINERS   |   1 +
>  drivers/gpu/drm/tiny/Kconfig  |  12 +
>  drivers/gpu/drm/tiny/Makefile |   1 +
>  drivers/gpu/drm/tiny/ofdrm.c  | 748 ++
>  drivers/video/fbdev/Kconfig   |   1 +
>  5 files changed, 763 insertions(+)
>  create mode 100644 drivers/gpu/drm/tiny/ofdrm.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 43d833273ae9..090cbe1aa5e3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6395,6 +6395,7 @@ L:  dri-devel@lists.freedesktop.org
>  S:   Maintained
>  T:   git git://anongit.freedesktop.org/drm/drm-misc
>  F:   drivers/gpu/drm/drm_aperture.c
> +F:   drivers/gpu/drm/tiny/ofdrm.c
>  F:   drivers/gpu/drm/tiny/simpledrm.c
>  F:   include/drm/drm_aperture.h
>  
> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> index 627d637a1e7e..0bc54af42e7f 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -51,6 +51,18 @@ config DRM_GM12U320
>This is a KMS driver for projectors which use the GM12U320 chipset
>for video transfer over USB2/3, such as the Acer C120 mini projector.
>  
> +config DRM_OFDRM
> + tristate "Open Firmware display driver"
> + depends on DRM && MMU && PPC

Does this build with !PCI?

The driver uses some PCI functions, so it might possibly break with
randconfig. I don't think there are practical !PCI PPC configurations.

Thanks

Michal

Re: [PATCH v3] uapi/drm/i915: Document memory residency and Flat-CCS capability of obj

2022-05-18 Thread Ye, Tony


Media driver never creates a BO with more than one backing regions.

Acked-by: Tony Ye 

Thanks,

Tony

On 5/2/2022 7:15 AM, Ramalingam C wrote:

Capture the impact of memory region preference list of the objects, on
their memory residency and Flat-CCS capability.

v2:
   Fix the Flat-CCS capability of an obj with {lmem, smem} preference
   list [Thomas]
v3:
   Reworded the doc [Matt]

Signed-off-by: Ramalingam C 
cc: Matthew Auld 
cc: Thomas Hellstrom 
cc: Daniel Vetter 
cc: Jon Bloomfield 
cc: Lionel Landwerlin 
cc: Kenneth Graunke 
cc: mesa-...@lists.freedesktop.org
cc: Jordan Justen 
cc: Tony Ye 
Reviewed-by: Matthew Auld 
---
  include/uapi/drm/i915_drm.h | 16 
  1 file changed, 16 insertions(+)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index a2def7b27009..b7e1c2fe08dc 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -3443,6 +3443,22 @@ struct drm_i915_gem_create_ext {
   * At which point we get the object handle in _i915_gem_create_ext.handle,
   * along with the final object size in _i915_gem_create_ext.size, which
   * should account for any rounding up, if required.
+ *
+ * Note that userspace has no means of knowing the current backing region
+ * for objects where @num_regions is larger than one. The kernel will only
+ * ensure that the priority order of the @regions array is honoured, either
+ * when initially placing the object, or when moving memory around due to
+ * memory pressure
+ *
+ * On Flat-CCS capable HW, compression is supported for the objects residing
+ * in I915_MEMORY_CLASS_DEVICE. When such objects (compressed) has other
+ * memory class in @regions and migrated (by I915, due to memory
+ * constrain) to the non I915_MEMORY_CLASS_DEVICE region, then I915 needs to
+ * decompress the content. But I915 dosen't have the required information to
+ * decompress the userspace compressed objects.
+ *
+ * So I915 supports Flat-CCS, only on the objects which can reside only on
+ * I915_MEMORY_CLASS_DEVICE regions.
   */
  struct drm_i915_gem_create_ext_memory_regions {
/** @base: Extension link. See struct i915_user_extension. */

[PATCH 0/2] drm: Add driverof PowerPC OF displays

2022-05-18 Thread Thomas Zimmermann

PowerPC's Open Firmware offers a simple display buffer for graphics
output. Add ofdrm, a DRM driver for the device. As with the existing
simpledrm driver, the graphics hardware is pre-initialized by the
firmware. The driver only provides blitting, no actual DRM modesetting
is possible.

Thomas Zimmermann (2):
  MAINTAINERS: Broaden scope of simpledrm entry
  drm/tiny: Add ofdrm for Open Firmware framebuffers

 MAINTAINERS   |   5 +-
 drivers/gpu/drm/tiny/Kconfig  |  12 +
 drivers/gpu/drm/tiny/Makefile |   1 +
 drivers/gpu/drm/tiny/ofdrm.c  | 748 ++
 drivers/video/fbdev/Kconfig   |   1 +
 5 files changed, 766 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/tiny/ofdrm.c

-- 
2.36.1

[PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers

2022-05-18 Thread Thomas Zimmermann

Open Firmware provides basic display output via the 'display' node.
DT platform code already provides a device that represents the node's
framebuffer. Add a DRM driver for the device. The display mode and
color format is pre-initialized by the system's firmware. Runtime
modesetting via DRM is not possible. The display is useful during
early boot stages or as error fallback.

Similar functionality is already provided by fbdev's offb driver,
which is insufficient for modern userspace. The old driver includes
support for BootX device tree, which can be found on old 32-bit
PowerPC Macintosh systems. If these are still in use, the
functionality can be added to ofdrm or implemented in a new
driver. As with simepldrm, the fbdev driver cannot be selected is
ofdrm is already enabled.

Two noteable points about the driver:

 * Reading the framebuffer aperture from the device tree is not
reliable on all systems. Ofdrm takes the heuristics and a comment
from offb to pick the correct range.

 * No resource management may be tied to the underlying PCI device.
Otherwise the handover to the native driver will fail with a resource
conflict. PCI management is therefore done as part of the platform
device's cleanup.

The driver has been tested on qemu's ppc64le emulation. The device
hand-over has been tested with bochs.

Signed-off-by: Thomas Zimmermann 
---
 MAINTAINERS   |   1 +
 drivers/gpu/drm/tiny/Kconfig  |  12 +
 drivers/gpu/drm/tiny/Makefile |   1 +
 drivers/gpu/drm/tiny/ofdrm.c  | 748 ++
 drivers/video/fbdev/Kconfig   |   1 +
 5 files changed, 763 insertions(+)
 create mode 100644 drivers/gpu/drm/tiny/ofdrm.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 43d833273ae9..090cbe1aa5e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6395,6 +6395,7 @@ L:dri-devel@lists.freedesktop.org
 S: Maintained
 T: git git://anongit.freedesktop.org/drm/drm-misc
 F: drivers/gpu/drm/drm_aperture.c
+F: drivers/gpu/drm/tiny/ofdrm.c
 F: drivers/gpu/drm/tiny/simpledrm.c
 F: include/drm/drm_aperture.h
 
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index 627d637a1e7e..0bc54af42e7f 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -51,6 +51,18 @@ config DRM_GM12U320
 This is a KMS driver for projectors which use the GM12U320 chipset
 for video transfer over USB2/3, such as the Acer C120 mini projector.
 
+config DRM_OFDRM
+   tristate "Open Firmware display driver"
+   depends on DRM && MMU && PPC
+   select DRM_GEM_SHMEM_HELPER
+   select DRM_KMS_HELPER
+   help
+ DRM driver for Open Firmware framebuffers.
+
+ This driver assumes that the display hardware has been initialized
+ by the Open Firmware before the kernel boots. Scanout buffer, size,
+ and display format must be provided via device tree.
+
 config DRM_PANEL_MIPI_DBI
tristate "DRM support for MIPI DBI compatible panels"
depends on DRM && SPI
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index 1d9d6227e7ab..76dde89a044b 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_ARCPGU)+= arcpgu.o
 obj-$(CONFIG_DRM_BOCHS)+= bochs.o
 obj-$(CONFIG_DRM_CIRRUS_QEMU)  += cirrus.o
 obj-$(CONFIG_DRM_GM12U320) += gm12u320.o
+obj-$(CONFIG_DRM_OFDRM)+= ofdrm.o
 obj-$(CONFIG_DRM_PANEL_MIPI_DBI)   += panel-mipi-dbi.o
 obj-$(CONFIG_DRM_SIMPLEDRM)+= simpledrm.o
 obj-$(CONFIG_TINYDRM_HX8357D)  += hx8357d.o
diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c
new file mode 100644
index ..aca715b36179
--- /dev/null
+++ b/drivers/gpu/drm/tiny/ofdrm.c
@@ -0,0 +1,748 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+#define DRIVER_NAME"ofdrm"
+#define DRIVER_DESC"DRM driver for OF platform devices"
+#define DRIVER_DATE"20220501"
+#define DRIVER_MAJOR   1
+#define DRIVER_MINOR   0
+
+/*
+ * Assume a monitor resolution of 96 dpi to
+ * get a somewhat reasonable screen size.
+ */
+#define RES_MM(d)  \
+   (((d) * 254ul) / (96ul * 10ul))
+
+#define OFDRM_MODE(hd, vd) \
+   DRM_SIMPLE_MODE(hd, vd, RES_MM(hd), RES_MM(vd))
+
+/*
+ * Helpers for display nodes
+ */
+
+static int display_get_validated_int(struct drm_device *dev, const char *name, 
uint32_t value)
+{
+   if (value > INT_MAX) {
+   drm_err(dev, "invalid framebuffer %s of %u\n", name, value);
+   return -EINVAL;
+   }
+   return (int)value;
+}
+
+static int display_get_validated_int0(struct drm_device *dev, const char 
*name, uint32_t value)
+{
+   if (!value)

[PATCH 1/2] MAINTAINERS: Broaden scope of simpledrm entry

2022-05-18 Thread Thomas Zimmermann

There will be more DRM drivers for firmware-provided framebuffers. Use
the existing entry for simpledrm instead of adding a new one for each
driver. Also add DRM's aperture helpers, which are part of the driver's
infrastructure.

Signed-off-by: Thomas Zimmermann 
---
 MAINTAINERS | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5c1fd93d9050..43d833273ae9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6388,13 +6388,15 @@ S:  Orphan / Obsolete
 F: drivers/gpu/drm/savage/
 F: include/uapi/drm/savage_drm.h
 
-DRM DRIVER FOR SIMPLE FRAMEBUFFERS
+DRM DRIVER FOR FIRMWARE FRAMEBUFFERS
 M: Thomas Zimmermann 
 M: Javier Martinez Canillas 
 L: dri-devel@lists.freedesktop.org
 S: Maintained
 T: git git://anongit.freedesktop.org/drm/drm-misc
+F: drivers/gpu/drm/drm_aperture.c
 F: drivers/gpu/drm/tiny/simpledrm.c
+F: include/drm/drm_aperture.h
 
 DRM DRIVER FOR SIS VIDEO CARDS
 S: Orphan / Obsolete
-- 
2.36.1

Re: [PATCH v2 1/2] phy/qcom: add regulator_set_load to edp/dp phy




On 5/18/2022 10:52 AM, Dmitry Baryshkov wrote:

On 18/05/2022 20:36, Kuogee Hsieh wrote:


On 5/18/2022 10:12 AM, Dmitry Baryshkov wrote:
On Wed, 18 May 2022 at 19:43, Kuogee Hsieh  
wrote:

This patch add regulator_set_load() to both eDP and DP phy driver
to have totally control regulators.

Changes in v2:
-- no regulator_set_laod() before disable regulator

Signed-off-by: Kuogee Hsieh 
---
  drivers/phy/qualcomm/phy-qcom-edp.c | 25 +
  drivers/phy/qualcomm/phy-qcom-qmp.c | 13 +

Split into -edp and -qmp part.


  2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c 
b/drivers/phy/qualcomm/phy-qcom-edp.c

index cacd32f..9b55095 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -87,17 +87,24 @@ struct qcom_edp {

 struct clk_bulk_data clks[2];
 struct regulator_bulk_data supplies[2];
+   int enable_load[2];
+   int disable_load[2];
As noticed in the review of the previous patch, disable_load is 
unnecessary.



  };

  static int qcom_edp_phy_init(struct phy *phy)
  {
 struct qcom_edp *edp = phy_get_drvdata(phy);
 int ret;
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;

-   ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), 
edp->supplies);

+   ret = regulator_bulk_enable(num_consumers, edp->supplies);
 if (ret)
 return ret;

+   for (i = num_consumers - 1; i >= 0; --i)
+ regulator_set_load(edp->supplies[i].consumer, edp->enable_load[i]);
+
 ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), 
edp->clks);

 if (ret)
 goto out_disable_supplies;
@@ -425,9 +432,15 @@ static int qcom_edp_phy_power_off(struct phy 
*phy)

  static int qcom_edp_phy_exit(struct phy *phy)
  {
 struct qcom_edp *edp = phy_get_drvdata(phy);
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;

clk_bulk_disable_unprepare(ARRAY_SIZE(edp->clks), edp->clks);
-   regulator_bulk_disable(ARRAY_SIZE(edp->supplies), 
edp->supplies);

+
+   for (i = num_consumers - 1; i >= 0; --i)
+ regulator_set_load(edp->supplies[i].consumer, edp->disable_load[i]);
+
+   regulator_bulk_disable(num_consumers, edp->supplies);

 return 0;
  }
@@ -633,8 +646,12 @@ static int qcom_edp_phy_probe(struct 
platform_device *pdev)

 if (ret)
 return ret;

-   edp->supplies[0].supply = "vdda-phy";
-   edp->supplies[1].supply = "vdda-pll";
+   edp->supplies[0].supply = "vdda-1p2";
+   edp->supplies[1].supply = "vdda-0p9";

NAK, You can not randomly change supply names.


if you do no change here, then we have to change dtsi.

They are not match.


Where is no match? I don't see any in-kernel dtsi using them.


my mistake, we did not pull in Doug's patch at our internal release 
where i run my test.







+   edp->enable_load[0] = 21800;    /* 1.2 V */
+   edp->enable_load[1] = 36000;    /* 1.2 V */
+   edp->disable_load[0] = 4;   /* 0.9 V */
+   edp->disable_load[1] = 4;   /* 10.9V */

Again, 10.9V here. Kuogee. Have you read the review points?

I have read it. but forget to make  change at edp file.


 ret = devm_regulator_bulk_get(dev, 
ARRAY_SIZE(edp->supplies), edp->supplies);

 if (ret)
 return ret;
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
b/drivers/phy/qualcomm/phy-qcom-qmp.c

index b144ae1..0a4c8a8 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -3130,6 +3130,7 @@ struct qmp_phy_cfg {
 int num_resets;
 /* regulators to be requested */
 const char * const *vreg_list;
+   const unsigned int *vreg_enable_load;
 int num_vregs;

 /* array of registers with different offsets */
@@ -3346,6 +3347,10 @@ static const char * const qmp_phy_vreg_l[] = {
 "vdda-phy", "vdda-pll",
  };

+static const unsigned int qmp_phy_vreg_enable_load[] = {
+   21800, 36000
+};
+
  static const struct qmp_phy_cfg ipq8074_usb3phy_cfg = {
 .type   = PHY_TYPE_USB3,
 .nlanes = 1,
@@ -4072,6 +4077,7 @@ static const struct qmp_phy_cfg 
sm8250_usb3phy_cfg = {

 .reset_list = msm8996_usb3phy_reset_l,
 .num_resets = 
ARRAY_SIZE(msm8996_usb3phy_reset_l),

 .vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,
 .num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
 .regs   = qmp_v4_usb3phy_regs_layout,

@@ -4139,6 +4145,7 @@ static const struct qmp_phy_cfg 
sm8250_dpphy_cfg = {

 .reset_list = msm8996_usb3phy_reset_l,
 .num_resets = 
ARRAY_SIZE(msm8996_usb3phy_reset_l),

 .vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,

Re: [PATCH v2 1/2] phy/qcom: add regulator_set_load to edp/dp phy


On 18/05/2022 20:36, Kuogee Hsieh wrote:


On 5/18/2022 10:12 AM, Dmitry Baryshkov wrote:
On Wed, 18 May 2022 at 19:43, Kuogee Hsieh  
wrote:

This patch add regulator_set_load() to both eDP and DP phy driver
to have totally control regulators.

Changes in v2:
-- no regulator_set_laod() before disable regulator

Signed-off-by: Kuogee Hsieh 
---
  drivers/phy/qualcomm/phy-qcom-edp.c | 25 +
  drivers/phy/qualcomm/phy-qcom-qmp.c | 13 +

Split into -edp and -qmp part.


  2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c 
b/drivers/phy/qualcomm/phy-qcom-edp.c

index cacd32f..9b55095 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -87,17 +87,24 @@ struct qcom_edp {

 struct clk_bulk_data clks[2];
 struct regulator_bulk_data supplies[2];
+   int enable_load[2];
+   int disable_load[2];
As noticed in the review of the previous patch, disable_load is 
unnecessary.



  };

  static int qcom_edp_phy_init(struct phy *phy)
  {
 struct qcom_edp *edp = phy_get_drvdata(phy);
 int ret;
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;

-   ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), 
edp->supplies);

+   ret = regulator_bulk_enable(num_consumers, edp->supplies);
 if (ret)
 return ret;

+   for (i = num_consumers - 1; i >= 0; --i)
+   regulator_set_load(edp->supplies[i].consumer, 
edp->enable_load[i]);

+
 ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), 
edp->clks);

 if (ret)
 goto out_disable_supplies;
@@ -425,9 +432,15 @@ static int qcom_edp_phy_power_off(struct phy *phy)
  static int qcom_edp_phy_exit(struct phy *phy)
  {
 struct qcom_edp *edp = phy_get_drvdata(phy);
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;

 clk_bulk_disable_unprepare(ARRAY_SIZE(edp->clks), edp->clks);
-   regulator_bulk_disable(ARRAY_SIZE(edp->supplies), 
edp->supplies);

+
+   for (i = num_consumers - 1; i >= 0; --i)
+   regulator_set_load(edp->supplies[i].consumer, 
edp->disable_load[i]);

+
+   regulator_bulk_disable(num_consumers, edp->supplies);

 return 0;
  }
@@ -633,8 +646,12 @@ static int qcom_edp_phy_probe(struct 
platform_device *pdev)

 if (ret)
 return ret;

-   edp->supplies[0].supply = "vdda-phy";
-   edp->supplies[1].supply = "vdda-pll";
+   edp->supplies[0].supply = "vdda-1p2";
+   edp->supplies[1].supply = "vdda-0p9";

NAK, You can not randomly change supply names.


if you do no change here, then we have to change dtsi.

They are not match.


Where is no match? I don't see any in-kernel dtsi using them.



+   edp->enable_load[0] = 21800;    /* 1.2 V */
+   edp->enable_load[1] = 36000;    /* 1.2 V */
+   edp->disable_load[0] = 4;   /* 0.9 V */
+   edp->disable_load[1] = 4;   /* 10.9V */

Again, 10.9V here. Kuogee. Have you read the review points?

I have read it. but forget to make  change at edp file.


 ret = devm_regulator_bulk_get(dev, 
ARRAY_SIZE(edp->supplies), edp->supplies);

 if (ret)
 return ret;
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
b/drivers/phy/qualcomm/phy-qcom-qmp.c

index b144ae1..0a4c8a8 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -3130,6 +3130,7 @@ struct qmp_phy_cfg {
 int num_resets;
 /* regulators to be requested */
 const char * const *vreg_list;
+   const unsigned int *vreg_enable_load;
 int num_vregs;

 /* array of registers with different offsets */
@@ -3346,6 +3347,10 @@ static const char * const qmp_phy_vreg_l[] = {
 "vdda-phy", "vdda-pll",
  };

+static const unsigned int qmp_phy_vreg_enable_load[] = {
+   21800, 36000
+};
+
  static const struct qmp_phy_cfg ipq8074_usb3phy_cfg = {
 .type   = PHY_TYPE_USB3,
 .nlanes = 1,
@@ -4072,6 +4077,7 @@ static const struct qmp_phy_cfg 
sm8250_usb3phy_cfg = {

 .reset_list = msm8996_usb3phy_reset_l,
 .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
 .vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,
 .num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
 .regs   = qmp_v4_usb3phy_regs_layout,

@@ -4139,6 +4145,7 @@ static const struct qmp_phy_cfg 
sm8250_dpphy_cfg = {

 .reset_list = msm8996_usb3phy_reset_l,
 .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
 .vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,


So, you apply this change only to the sm8250 (sc7280) config. Are you 
sure that both of them have the same

Re: nouveau lockdep deadlock report with 5.18-rc6

2022-05-18 Thread Lyude Paul

Yeah I noticed this as well, I will try to bisect this the next change that I
get


On Tue, 2022-05-17 at 13:10 +0200, Hans de Goede wrote:
> Hi All,
> 
> I just noticed the below lockdep possible deadlock report with a 5.18-rc6
> kernel on a Dell Latitude E6430 laptop with the following nvidia GPU:
> 
> 01:00.0 VGA compatible controller [0300]: NVIDIA Corporation GF108GLM [NVS
> 5200M] [10de:0dfc] (rev a1)
> 01:00.1 Audio device [0403]: NVIDIA Corporation GF108 High Definition Audio
> Controller [10de:0bea] (rev a1)
> 
> This is with the laptop in Optimus mode, so with the Intel integrated
> gfx from the i5-3320M CPU driving the LCD panel and with nothing connected
> to the HDMI connector, which is always routed to the NVIDIA GPU on this
> laptop.
> 
> The lockdep possible deadlock warning seems to happen when the NVIDIA GPU
> is runtime suspended shortly after gdm has loaded:
> 
> [   24.859171] ==
> [   24.859173] WARNING: possible circular locking dependency detected
> [   24.859175] 5.18.0-rc6+ #34 Tainted: G    E    
> [   24.859178] --
> [   24.859179] kworker/1:1/46 is trying to acquire lock:
> [   24.859181] 92b0c0ee0518 (>mutex){+.+.}-{3:3}, at:
> nouveau_vga_lastclose+0x910/0x1030 [nouveau]
> [   24.859231] 
>    but task is already holding lock:
> [   24.859233] 92b0c4bf35a0 (reservation_ww_class_mutex){+.+.}-{3:3},
> at: ttm_bo_wait+0x7d/0x140 [ttm]
> [   24.859243] 
>    which lock already depends on the new lock.
> 
> [   24.859244] 
>    the existing dependency chain (in reverse order) is:
> [   24.859246] 
>    -> #1 (reservation_ww_class_mutex){+.+.}-{3:3}:
> [   24.859249]    __ww_mutex_lock.constprop.0+0xb3/0xfb0
> [   24.859256]    ww_mutex_lock+0x38/0xa0
> [   24.859259]    nouveau_bo_pin+0x30/0x380 [nouveau]
> [   24.859297]    nouveau_channel_del+0x1d7/0x3e0 [nouveau]
> [   24.859328]    nouveau_channel_new+0x48/0x730 [nouveau]
> [   24.859358]    nouveau_abi16_ioctl_channel_alloc+0x113/0x360
> [nouveau]
> [   24.859389]    drm_ioctl_kernel+0xa1/0x150
> [   24.859392]    drm_ioctl+0x21c/0x410
> [   24.859395]    nouveau_drm_ioctl+0x56/0x1820 [nouveau]
> [   24.859431]    __x64_sys_ioctl+0x8d/0xc0
> [   24.859436]    do_syscall_64+0x5b/0x80
> [   24.859440]    entry_SYSCALL_64_after_hwframe+0x44/0xae
> [   24.859443] 
>    -> #0 (>mutex){+.+.}-{3:3}:
> [   24.859446]    __lock_acquire+0x12e2/0x1f90
> [   24.859450]    lock_acquire+0xad/0x290
> [   24.859453]    __mutex_lock+0x90/0x830
> [   24.859456]    nouveau_vga_lastclose+0x910/0x1030 [nouveau]
> [   24.859493]    ttm_bo_move_to_lru_tail+0x32c/0x980 [ttm]
> [   24.859498]    ttm_mem_evict_first+0x25c/0x4b0 [ttm]
> [   24.859503]    ttm_resource_manager_evict_all+0x93/0x1b0 [ttm]
> [   24.859509]    nouveau_debugfs_fini+0x161/0x260 [nouveau]
> [   24.859545]    nouveau_drm_ioctl+0xa4a/0x1820 [nouveau]
> [   24.859582]    pci_pm_runtime_suspend+0x5c/0x180
> [   24.859585]    __rpm_callback+0x48/0x1b0
> [   24.859589]    rpm_callback+0x5a/0x70
> [   24.859591]    rpm_suspend+0x10a/0x6f0
> [   24.859594]    pm_runtime_work+0xa0/0xb0
> [   24.859596]    process_one_work+0x254/0x560
> [   24.859601]    worker_thread+0x4f/0x390
> [   24.859604]    kthread+0xe6/0x110
> [   24.859607]    ret_from_fork+0x22/0x30
> [   24.859611] 
>    other info that might help us debug this:
> 
> [   24.859612]  Possible unsafe locking scenario:
> 
> [   24.859613]    CPU0    CPU1
> [   24.859615]        
> [   24.859616]   lock(reservation_ww_class_mutex);
> [   24.859618]    lock(>mutex);
> [   24.859620]   
> lock(reservation_ww_class_mutex);
> [   24.859622]   lock(>mutex);
> [   24.859624] 
>     *** DEADLOCK ***
> 
> [   24.859625] 3 locks held by kworker/1:1/46:
> [   24.859627]  #0: 92b0c0bb4338 ((wq_completion)pm){+.+.}-{0:0}, at:
> process_one_work+0x1d0/0x560
> [   24.859634]  #1: a8ffc02dfe80 ((work_completion)(
> >power.work)){+.+.}-{0:0}, at: process_one_work+0x1d0/0x560
> [   24.859641]  #2: 92b0c4bf35a0 (reservation_ww_class_mutex){+.+.}-
> {3:3}, at: ttm_bo_wait+0x7d/0x140 [ttm]
> [   24.859649] 
>    stack backtrace:
> [   24.859651] CPU: 1 PID: 46 Comm: kworker/1:1 Tainted: G    E
> 5.18.0-rc6+ #34
> [   24.859654] Hardware name: Dell Inc. Latitude E6430/0H3MT5, BIOS A21
> 05/08/2017
> [   24.859656] Workqueue: pm pm_runtime_work
> [   24.859660] Call Trace:
> [   24.859662]  
> [   24.859665]  dump_stack_lvl+0x5b/0x74
> [   24.859669]  check_noncircular+0xdf/0x100
> [   24.859672]  ? register_lock_class+0x38/0x470
> [   24.859678]  __lock_acquire+0x12e2/0x1f90
> [   24.859683]

Re: [PATCH 12/14] drm/nouveau: Register ACPI video backlight when nv_backlight registration fails

2022-05-18 Thread Lyude Paul

On Tue, 2022-05-17 at 17:23 +0200, Hans de Goede wrote:
> Typically the acpi_video driver will initialize before nouveau, which
> used to cause /sys/class/backlight/acpi_video0 to get registered and then
> nouveau would register its own nv_backlight device later. After which
> the drivers/acpi/video_detect.c code unregistered the acpi_video0 device
> to avoid there being 2 backlight devices.
> 
> This means that userspace used to briefly see 2 devices and the
> disappearing of acpi_video0 after a brief time confuses the systemd
> backlight level save/restore code, see e.g.:
> https://bbs.archlinux.org/viewtopic.php?id=269920
> 
> To fix this the ACPI video code has been modified to make backlight class
> device registration a separate step, relying on the drm/kms driver to
> ask for the acpi_video backlight registration after it is done setting up
> its native backlight device.
> 
> Add a call to the new acpi_video_register_backlight() when native backlight
> device registration has failed / was skipped to ensure that there is a
> backlight device available before the drm_device gets registered with
> userspace.
> 
> Signed-off-by: Hans de Goede 
> ---
>  drivers/gpu/drm/nouveau/nouveau_backlight.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index f56ff797c78c..0ae8793357a4 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -436,6 +436,13 @@ nouveau_backlight_init(struct drm_connector *connector)
>  
>  fail_alloc:
> kfree(bl);
> +   /*
> +    * If we get here we have an internal panel, but no nv_backlight,
> +    * try registering an ACPI video backlight device instead.
> +    */
> +   if (ret == 0)
> +   acpi_video_register_backlight();

Assuming we don't need to return the value of acpi_video_register_backlight()
here:

Reviewed-by: Lyude Paul 

> +
> return ret;
>  }
>  

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

Re: [PATCH v2 1/2] phy/qcom: add regulator_set_load to edp/dp phy




On 5/18/2022 10:12 AM, Dmitry Baryshkov wrote:

On Wed, 18 May 2022 at 19:43, Kuogee Hsieh  wrote:

This patch add regulator_set_load() to both eDP and DP phy driver
to have totally control regulators.

Changes in v2:
-- no regulator_set_laod() before disable regulator

Signed-off-by: Kuogee Hsieh 
---
  drivers/phy/qualcomm/phy-qcom-edp.c | 25 +
  drivers/phy/qualcomm/phy-qcom-qmp.c | 13 +

Split into -edp and -qmp part.


  2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c 
b/drivers/phy/qualcomm/phy-qcom-edp.c
index cacd32f..9b55095 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -87,17 +87,24 @@ struct qcom_edp {

 struct clk_bulk_data clks[2];
 struct regulator_bulk_data supplies[2];
+   int enable_load[2];
+   int disable_load[2];

As noticed in the review of the previous patch, disable_load is unnecessary.


  };

  static int qcom_edp_phy_init(struct phy *phy)
  {
 struct qcom_edp *edp = phy_get_drvdata(phy);
 int ret;
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;

-   ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
+   ret = regulator_bulk_enable(num_consumers, edp->supplies);
 if (ret)
 return ret;

+   for (i = num_consumers - 1; i >= 0; --i)
+   regulator_set_load(edp->supplies[i].consumer, 
edp->enable_load[i]);
+
 ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), edp->clks);
 if (ret)
 goto out_disable_supplies;
@@ -425,9 +432,15 @@ static int qcom_edp_phy_power_off(struct phy *phy)
  static int qcom_edp_phy_exit(struct phy *phy)
  {
 struct qcom_edp *edp = phy_get_drvdata(phy);
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;

 clk_bulk_disable_unprepare(ARRAY_SIZE(edp->clks), edp->clks);
-   regulator_bulk_disable(ARRAY_SIZE(edp->supplies), edp->supplies);
+
+   for (i = num_consumers - 1; i >= 0; --i)
+   regulator_set_load(edp->supplies[i].consumer, 
edp->disable_load[i]);
+
+   regulator_bulk_disable(num_consumers, edp->supplies);

 return 0;
  }
@@ -633,8 +646,12 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
 if (ret)
 return ret;

-   edp->supplies[0].supply = "vdda-phy";
-   edp->supplies[1].supply = "vdda-pll";
+   edp->supplies[0].supply = "vdda-1p2";
+   edp->supplies[1].supply = "vdda-0p9";

NAK, You can not randomly change supply names.


if you do no change here, then we have to change dtsi.

They are not match.






+   edp->enable_load[0] = 21800;/* 1.2 V */
+   edp->enable_load[1] = 36000;/* 1.2 V */
+   edp->disable_load[0] = 4;   /* 0.9 V */
+   edp->disable_load[1] = 4;   /* 10.9V */

Again, 10.9V here. Kuogee. Have you read the review points?

I have read it. but forget to make  change at edp file.



 ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), 
edp->supplies);
 if (ret)
 return ret;
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
b/drivers/phy/qualcomm/phy-qcom-qmp.c
index b144ae1..0a4c8a8 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -3130,6 +3130,7 @@ struct qmp_phy_cfg {
 int num_resets;
 /* regulators to be requested */
 const char * const *vreg_list;
+   const unsigned int *vreg_enable_load;
 int num_vregs;

 /* array of registers with different offsets */
@@ -3346,6 +3347,10 @@ static const char * const qmp_phy_vreg_l[] = {
 "vdda-phy", "vdda-pll",
  };

+static const unsigned int qmp_phy_vreg_enable_load[] = {
+   21800, 36000
+};
+
  static const struct qmp_phy_cfg ipq8074_usb3phy_cfg = {
 .type   = PHY_TYPE_USB3,
 .nlanes = 1,
@@ -4072,6 +4077,7 @@ static const struct qmp_phy_cfg sm8250_usb3phy_cfg = {
 .reset_list = msm8996_usb3phy_reset_l,
 .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
 .vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,
 .num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
 .regs   = qmp_v4_usb3phy_regs_layout,

@@ -4139,6 +4145,7 @@ static const struct qmp_phy_cfg sm8250_dpphy_cfg = {
 .reset_list = msm8996_usb3phy_reset_l,
 .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
 .vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,
 .num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
 .regs   = qmp_v4_usb3phy_regs_layout,

@@ -5008,6 +5015,11 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)
 return 0;

Re: [PATCH v2 0/2] eDP/DP Phy vdda realted function


On 18/05/2022 20:29, Kuogee Hsieh wrote:


On 5/18/2022 10:16 AM, Dmitry Baryshkov wrote:
On Wed, 18 May 2022 at 19:43, Kuogee Hsieh  
wrote:

1) add regulator_set_load() to eDP/DP phy
2) remove vdda related function out of eDP/DP controller

These patches touch two subsystems and have a dependency between them.
How do we merge them?


currently, both phy and controller are vote for regulator. The last vote 
will just increase count.


Therefore the dependency should be very loose.


So, do you propose to merge dp change a cycle after the phy changes go in?





Kuogee Hsieh (2):
   phy/qcom: add regulator_set_load to edp/dp phy
   drm/msm/dp: delete vdda regulator related functions from eDP/DP
 controller

  drivers/gpu/drm/msm/dp/dp_parser.c  | 14 --
  drivers/gpu/drm/msm/dp/dp_parser.h  |  6 ---
  drivers/gpu/drm/msm/dp/dp_power.c   | 95 
+

  drivers/phy/qualcomm/phy-qcom-edp.c | 25 --
  drivers/phy/qualcomm/phy-qcom-qmp.c | 13 +
  5 files changed, 36 insertions(+), 117 deletions(-)

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

a Linux Foundation Collaborative Project






--
With best wishes
Dmitry

Re: [PATCH v2 0/2] eDP/DP Phy vdda realted function




On 5/18/2022 10:16 AM, Dmitry Baryshkov wrote:

On Wed, 18 May 2022 at 19:43, Kuogee Hsieh  wrote:

1) add regulator_set_load() to eDP/DP phy
2) remove vdda related function out of eDP/DP controller

These patches touch two subsystems and have a dependency between them.
How do we merge them?


currently, both phy and controller are vote for regulator. The last vote 
will just increase count.


Therefore the dependency should be very loose.



Kuogee Hsieh (2):
   phy/qcom: add regulator_set_load to edp/dp phy
   drm/msm/dp: delete vdda regulator related functions from eDP/DP
 controller

  drivers/gpu/drm/msm/dp/dp_parser.c  | 14 --
  drivers/gpu/drm/msm/dp/dp_parser.h  |  6 ---
  drivers/gpu/drm/msm/dp/dp_power.c   | 95 +
  drivers/phy/qualcomm/phy-qcom-edp.c | 25 --
  drivers/phy/qualcomm/phy-qcom-qmp.c | 13 +
  5 files changed, 36 insertions(+), 117 deletions(-)

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

Re: [PATCH v2 0/2] eDP/DP Phy vdda realted function

On Wed, 18 May 2022 at 19:43, Kuogee Hsieh  wrote:
>
> 1) add regulator_set_load() to eDP/DP phy
> 2) remove vdda related function out of eDP/DP controller

These patches touch two subsystems and have a dependency between them.
How do we merge them?

>
> Kuogee Hsieh (2):
>   phy/qcom: add regulator_set_load to edp/dp phy
>   drm/msm/dp: delete vdda regulator related functions from eDP/DP
> controller
>
>  drivers/gpu/drm/msm/dp/dp_parser.c  | 14 --
>  drivers/gpu/drm/msm/dp/dp_parser.h  |  6 ---
>  drivers/gpu/drm/msm/dp/dp_power.c   | 95 
> +
>  drivers/phy/qualcomm/phy-qcom-edp.c | 25 --
>  drivers/phy/qualcomm/phy-qcom-qmp.c | 13 +
>  5 files changed, 36 insertions(+), 117 deletions(-)
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>


-- 
With best wishes
Dmitry

Re: [RFC PATCH v3 02/19] KVM: x86: inhibit APICv/AVIC when the guest and/or host changes apic id/base from the defaults.

On Wed, 2022-05-18 at 15:39 +, Sean Christopherson wrote:
> On Wed, May 18, 2022, Maxim Levitsky wrote:
> > On Wed, 2022-05-18 at 16:28 +0800, Chao Gao wrote:
> > > > struct kvm_arch {
> > > > @@ -1258,6 +1260,7 @@ struct kvm_arch {
> > > > hpa_t   hv_root_tdp;
> > > > spinlock_t hv_root_tdp_lock;
> > > > #endif
> > > > +   bool apic_id_changed;
> > > 
> > > What's the value of this boolean? No one reads it.
> > 
> > I use it in later patches to kill the guest during nested VM entry 
> > if it attempts to use nested AVIC after any vCPU changed APIC ID.
> 
> Then the flag should be introduced in the later patch, because (a) it's dead 
> code
> if that patch is never merged and (b) it's impossible to review this patch for
> correctness without seeing the usage, e.g. setting apic_id_changed isn't 
> guarded
> with a lock and so the usage may or may not be susceptible to races.

I can't disagree with you on this, this was just somewhat a hack I wasn't sure
(and not yet 100% sure I will move forward with) so I cut this corner.

Thanks for the review!

Best regards,
Maxim Levitsky

> 
> > > > +   apic->vcpu->kvm->arch.apic_id_changed = true;
> > > > +}
> > > > +

Re: [RFC PATCH v3 01/19] KVM: x86: document AVIC/APICv inhibit reasons

On Wed, 2022-05-18 at 15:56 +, Sean Christopherson wrote:
> On Wed, Apr 27, 2022, Maxim Levitsky wrote:
> > These days there are too many AVIC/APICv inhibit
> > reasons, and it doesn't hurt to have some documentation
> > for them.
> 
> Please wrap at ~75 chars.
> 
> > Signed-off-by: Maxim Levitsky 
> > ---
> >  arch/x86/include/asm/kvm_host.h | 15 +++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/arch/x86/include/asm/kvm_host.h 
> > b/arch/x86/include/asm/kvm_host.h
> > index f164c6c1514a4..63eae00625bda 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1046,14 +1046,29 @@ struct kvm_x86_msr_filter {
> >  };
> >  
> >  enum kvm_apicv_inhibit {
> > +   /* APICv/AVIC is disabled by module param and/or not supported in 
> > hardware */
> 
> Rather than tag every one as APICv vs. AVIC, what about reorganizing the 
> enums so
> that the common vs. AVIC flags are bundled together?  And then the redundant 
> info
> in the comments about "XYZ is inhibited" can go away too, i.e. the individual
> comments can focus on explaining what triggers the inhibit (and for some, why 
> that
> action is incompatible with APIC virtualization).

Very good idea, will do!

Best regards,
Maxim Levitsky

> 
> E.g.
>   /***/
>   /* INHIBITs are relevant to both Intel's APICv and AMD's AVIC. */
>   /***/
> 
>   /* APIC/AVIC is unsupported and/or disabled via module param. */
>   APICV_INHIBIT_REASON_DISABLE,
> 
>   /* The local APIC is not in-kernel.  See KVM_CREATE_IRQCHIP. */
>   APICV_INHIBIT_REASON_ABSENT,
> 
>   /*
>* At least one IRQ vector is configured for HyperV's AutoEOI, which
>* requires manually injecting the IRQ to do EOI on behalf of the guest.
>*/
>   APICV_INHIBIT_REASON_HYPERV,
>   
> 
>   /**/
>   /* INHIBITs relevant only to AMD's AVIC. */
>   /**/
> 
> > APICV_INHIBIT_REASON_DISABLE,
> > +   /* APICv/AVIC is inhibited because AutoEOI feature is being used by a 
> > HyperV guest*/
> > APICV_INHIBIT_REASON_HYPERV,
> > +   /* AVIC is inhibited on a CPU because it runs a nested guest */
> > APICV_INHIBIT_REASON_NESTED,
> > +   /* AVIC is inhibited due to wait for an irq window (AVIC doesn't 
> > support this) */
> > APICV_INHIBIT_REASON_IRQWIN,
> > +   /*
> > +* AVIC is inhibited because i8254 're-inject' mode is used
> > +* which needs EOI intercept which AVIC doesn't support
> > +*/
> > APICV_INHIBIT_REASON_PIT_REINJ,
> > +   /* AVIC is inhibited because the guest has x2apic in its CPUID*/
> > APICV_INHIBIT_REASON_X2APIC,
> > +   /* AVIC/APICv is inhibited because KVM_GUESTDBG_BLOCKIRQ was enabled */
> > APICV_INHIBIT_REASON_BLOCKIRQ,
> > +   /*
> > +* AVIC/APICv is inhibited because the guest didn't yet
> 
> s/guest/userspace
> 
> > +* enable kernel/split irqchip
> > +*/
> > APICV_INHIBIT_REASON_ABSENT,
> > +   /* AVIC is disabled because SEV doesn't support it */
> > APICV_INHIBIT_REASON_SEV,
> >  };
> >  
> > -- 
> > 2.26.3
> >

Re: [PATCH v2 2/2] drm/msm/dp: delete vdda regulator related functions from eDP/DP controller

On Wed, 18 May 2022 at 19:43, Kuogee Hsieh  wrote:
>
> Vdda regulators are related to both eDP and DP phy so that it should be
> managed at eDP and DP phy driver instead of controller. This patch remove

removes

> vdda regulators related functions out of eDP/DP controller.
> Signed-off-by: Kuogee Hsieh 
> ---
>  drivers/gpu/drm/msm/dp/dp_parser.c | 14 --
>  drivers/gpu/drm/msm/dp/dp_parser.h |  6 ---
>  drivers/gpu/drm/msm/dp/dp_power.c  | 95 
> +-
>  3 files changed, 2 insertions(+), 113 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
> b/drivers/gpu/drm/msm/dp/dp_parser.c
> index 8f9fed9..4ef2130 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -22,14 +22,6 @@
>  #define DP_DEFAULT_P0_OFFSET   0x1000
>  #define DP_DEFAULT_P0_SIZE 0x0400
>
> -static const struct dp_regulator_cfg sdm845_dp_reg_cfg = {
> -   .num = 2,
> -   .regs = {
> -   {"vdda-1p2", 21800, 4 },/* 1.2 V */
> -   {"vdda-0p9", 36000, 32 },   /* 0.9 V */
> -   },
> -};
> -
>  static void __iomem *dp_ioremap(struct platform_device *pdev, int idx, 
> size_t *len)
>  {
> struct resource *res;
> @@ -298,12 +290,6 @@ static int dp_parser_parse(struct dp_parser *parser)
> if (rc)
> return rc;
>
> -   /* Map the corresponding regulator information according to
> -* version. Currently, since we only have one supported platform,
> -* mapping the regulator directly.
> -*/
> -   parser->regulator_cfg = _dp_reg_cfg;
> -
> return 0;
>  }
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h 
> b/drivers/gpu/drm/msm/dp/dp_parser.h
> index 3a4d797..b56b4d7 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -101,11 +101,6 @@ struct dp_reg_entry {
> int disable_load;
>  };
>
> -struct dp_regulator_cfg {
> -   int num;
> -   struct dp_reg_entry regs[DP_DEV_REGULATOR_MAX];
> -};
> -
>  /**
>   * struct dp_parser - DP parser's data exposed to clients
>   *
> @@ -121,7 +116,6 @@ struct dp_parser {
> struct dp_pinctrl pinctrl;
> struct dp_io io;
> struct dp_display_data disp_data;
> -   const struct dp_regulator_cfg *regulator_cfg;
> u32 max_dp_lanes;
> struct drm_bridge *next_bridge;
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_power.c 
> b/drivers/gpu/drm/msm/dp/dp_power.c
> index d9e0117..b52ac1d 100644
> --- a/drivers/gpu/drm/msm/dp/dp_power.c
> +++ b/drivers/gpu/drm/msm/dp/dp_power.c
> @@ -20,82 +20,10 @@ struct dp_power_private {
> struct clk *link_clk_src;
> struct clk *pixel_provider;
> struct clk *link_provider;
> -   struct regulator_bulk_data supplies[DP_DEV_REGULATOR_MAX];
>
> struct dp_power dp_power;
>  };
>
> -static void dp_power_regulator_disable(struct dp_power_private *power)
> -{
> -   struct regulator_bulk_data *s = power->supplies;
> -   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
> -   int num = power->parser->regulator_cfg->num;
> -   int i;
> -
> -   DBG("");
> -   for (i = num - 1; i >= 0; i--)
> -   if (regs[i].disable_load >= 0)
> -   regulator_set_load(s[i].consumer,
> -  regs[i].disable_load);
> -
> -   regulator_bulk_disable(num, s);
> -}
> -
> -static int dp_power_regulator_enable(struct dp_power_private *power)
> -{
> -   struct regulator_bulk_data *s = power->supplies;
> -   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
> -   int num = power->parser->regulator_cfg->num;
> -   int ret, i;
> -
> -   DBG("");
> -   for (i = 0; i < num; i++) {
> -   if (regs[i].enable_load >= 0) {
> -   ret = regulator_set_load(s[i].consumer,
> -regs[i].enable_load);
> -   if (ret < 0) {
> -   pr_err("regulator %d set op mode failed, 
> %d\n",
> -   i, ret);
> -   goto fail;
> -   }
> -   }
> -   }
> -
> -   ret = regulator_bulk_enable(num, s);
> -   if (ret < 0) {
> -   pr_err("regulator enable failed, %d\n", ret);
> -   goto fail;
> -   }
> -
> -   return 0;
> -
> -fail:
> -   for (i--; i >= 0; i--)
> -   regulator_set_load(s[i].consumer, regs[i].disable_load);
> -   return ret;
> -}
> -
> -static int dp_power_regulator_init(struct dp_power_private *power)
> -{
> -   struct regulator_bulk_data *s = power->supplies;
> -   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
> -   struct platform_device *pdev = power->pdev;
> -   int num = power->parser->regulator_cfg->num;
> -   int i, ret;
> -

Re: [PATCH v2 1/2] phy/qcom: add regulator_set_load to edp/dp phy

On Wed, 18 May 2022 at 19:43, Kuogee Hsieh  wrote:
>
> This patch add regulator_set_load() to both eDP and DP phy driver
> to have totally control regulators.
>
> Changes in v2:
> -- no regulator_set_laod() before disable regulator
>
> Signed-off-by: Kuogee Hsieh 
> ---
>  drivers/phy/qualcomm/phy-qcom-edp.c | 25 +
>  drivers/phy/qualcomm/phy-qcom-qmp.c | 13 +

Split into -edp and -qmp part.

>  2 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c 
> b/drivers/phy/qualcomm/phy-qcom-edp.c
> index cacd32f..9b55095 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -87,17 +87,24 @@ struct qcom_edp {
>
> struct clk_bulk_data clks[2];
> struct regulator_bulk_data supplies[2];
> +   int enable_load[2];
> +   int disable_load[2];

As noticed in the review of the previous patch, disable_load is unnecessary.

>  };
>
>  static int qcom_edp_phy_init(struct phy *phy)
>  {
> struct qcom_edp *edp = phy_get_drvdata(phy);
> int ret;
> +   int num_consumers = ARRAY_SIZE(edp->supplies);
> +   int i;
>
> -   ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
> +   ret = regulator_bulk_enable(num_consumers, edp->supplies);
> if (ret)
> return ret;
>
> +   for (i = num_consumers - 1; i >= 0; --i)
> +   regulator_set_load(edp->supplies[i].consumer, 
> edp->enable_load[i]);
> +
> ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), edp->clks);
> if (ret)
> goto out_disable_supplies;
> @@ -425,9 +432,15 @@ static int qcom_edp_phy_power_off(struct phy *phy)
>  static int qcom_edp_phy_exit(struct phy *phy)
>  {
> struct qcom_edp *edp = phy_get_drvdata(phy);
> +   int num_consumers = ARRAY_SIZE(edp->supplies);
> +   int i;
>
> clk_bulk_disable_unprepare(ARRAY_SIZE(edp->clks), edp->clks);
> -   regulator_bulk_disable(ARRAY_SIZE(edp->supplies), edp->supplies);
> +
> +   for (i = num_consumers - 1; i >= 0; --i)
> +   regulator_set_load(edp->supplies[i].consumer, 
> edp->disable_load[i]);
> +
> +   regulator_bulk_disable(num_consumers, edp->supplies);
>
> return 0;
>  }
> @@ -633,8 +646,12 @@ static int qcom_edp_phy_probe(struct platform_device 
> *pdev)
> if (ret)
> return ret;
>
> -   edp->supplies[0].supply = "vdda-phy";
> -   edp->supplies[1].supply = "vdda-pll";
> +   edp->supplies[0].supply = "vdda-1p2";
> +   edp->supplies[1].supply = "vdda-0p9";

NAK, You can not randomly change supply names.

> +   edp->enable_load[0] = 21800;/* 1.2 V */
> +   edp->enable_load[1] = 36000;/* 1.2 V */
> +   edp->disable_load[0] = 4;   /* 0.9 V */
> +   edp->disable_load[1] = 4;   /* 10.9V */

Again, 10.9V here. Kuogee. Have you read the review points?

> ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), 
> edp->supplies);
> if (ret)
> return ret;
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
> b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index b144ae1..0a4c8a8 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -3130,6 +3130,7 @@ struct qmp_phy_cfg {
> int num_resets;
> /* regulators to be requested */
> const char * const *vreg_list;
> +   const unsigned int *vreg_enable_load;
> int num_vregs;
>
> /* array of registers with different offsets */
> @@ -3346,6 +3347,10 @@ static const char * const qmp_phy_vreg_l[] = {
> "vdda-phy", "vdda-pll",
>  };
>
> +static const unsigned int qmp_phy_vreg_enable_load[] = {
> +   21800, 36000
> +};
> +
>  static const struct qmp_phy_cfg ipq8074_usb3phy_cfg = {
> .type   = PHY_TYPE_USB3,
> .nlanes = 1,
> @@ -4072,6 +4077,7 @@ static const struct qmp_phy_cfg sm8250_usb3phy_cfg = {
> .reset_list = msm8996_usb3phy_reset_l,
> .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
> .vreg_list  = qmp_phy_vreg_l,
> +   .vreg_enable_load   = qmp_phy_vreg_enable_load,
> .num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
> .regs   = qmp_v4_usb3phy_regs_layout,
>
> @@ -4139,6 +4145,7 @@ static const struct qmp_phy_cfg sm8250_dpphy_cfg = {
> .reset_list = msm8996_usb3phy_reset_l,
> .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
> .vreg_list  = qmp_phy_vreg_l,
> +   .vreg_enable_load   = qmp_phy_vreg_enable_load,
> .num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
> .regs   = qmp_v4_usb3phy_regs_layout,
>
> @@ -5008,6 +5015,11 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)
> return 0;
> }

Re: [PATCH 05/14] drm/nouveau: Don't register backlight when another backlight should be used

2022-05-18 Thread Lyude Paul

Reviewed-by: Lyude Paul 

Also, ack on this being pushed to drm-misc, along with any other patches I r-b

On Tue, 2022-05-17 at 17:23 +0200, Hans de Goede wrote:
> Before this commit when we want userspace to use the acpi_video backlight
> device we register both the GPU's native backlight device and acpi_video's
> firmware acpi_video# backlight device. This relies on userspace preferring
> firmware type backlight devices over native ones.
> 
> Registering 2 backlight devices for a single display really is
> undesirable, don't register the GPU's native backlight device when
> another backlight device should be used.
> 
> Signed-off-by: Hans de Goede 
> ---
>  drivers/gpu/drm/nouveau/nouveau_backlight.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index daf9f87477ba..f56ff797c78c 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -34,6 +34,8 @@
>  #include 
>  #include 
>  
> +#include 
> +
>  #include "nouveau_drv.h"
>  #include "nouveau_reg.h"
>  #include "nouveau_encoder.h"
> @@ -404,6 +406,11 @@ nouveau_backlight_init(struct drm_connector *connector)
> goto fail_alloc;
> }
>  
> +   if (acpi_video_get_backlight_type(true) != acpi_backlight_native) {
> +   NV_INFO(drm, "Skipping nv_backlight registration\n");
> +   goto fail_alloc;
> +   }
> +
> if (!nouveau_get_backlight_name(backlight_name, bl)) {
> NV_ERROR(drm, "Failed to retrieve a unique name for the
> backlight interface\n");
> goto fail_alloc;

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

Re: [PATCH] drm/bridge: fix anx6345 power up sequence

2022-05-18 Thread Vasily Khoruzhick

On Thu, Apr 28, 2022 at 8:58 AM Torsten Duwe  wrote:
>
> On Mon, 18 Apr 2022 17:25:57 -0700
> Vasily Khoruzhick  wrote:
>
> > On Sun, Apr 17, 2022 at 11:52 AM Vasily Khoruzhick
> >  wrote:
>
> > > The change looks good to me, but I'll need some time to actually
> > > test it. If you don't hear from me for longer than a week please
> > > ping me.
> >
> > Your change doesn't fix the issue for me. Running "xrandr --output
> > eDP-1 --off; xrandr --output eDP-1 --auto" in a loop triggers the
> > issue pretty quickly even with the patch.
>
> Nope, even that works fine here. Side question: how do you initially
> power on the eDP bridge? Could there be any leftovers from that
> mechanism? I use a hacked-up U-Boot with a procedure similar to the
> kernel driver as fixed by this change.
>
> But the main question is: does this patch in any way worsen the
> situation on the pinebook?

I don't think it worsens anything, but according to the datasheet the
change makes no sense. Could you try increasing T2 instead of changing
the power sequence?

> Torsten

[PATCH v2 2/2] drm/msm/dp: delete vdda regulator related functions from eDP/DP controller

Vdda regulators are related to both eDP and DP phy so that it should be
managed at eDP and DP phy driver instead of controller. This patch remove
vdda regulators related functions out of eDP/DP controller.

Signed-off-by: Kuogee Hsieh 
---
 drivers/gpu/drm/msm/dp/dp_parser.c | 14 --
 drivers/gpu/drm/msm/dp/dp_parser.h |  6 ---
 drivers/gpu/drm/msm/dp/dp_power.c  | 95 +-
 3 files changed, 2 insertions(+), 113 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
b/drivers/gpu/drm/msm/dp/dp_parser.c
index 8f9fed9..4ef2130 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -22,14 +22,6 @@
 #define DP_DEFAULT_P0_OFFSET   0x1000
 #define DP_DEFAULT_P0_SIZE 0x0400
 
-static const struct dp_regulator_cfg sdm845_dp_reg_cfg = {
-   .num = 2,
-   .regs = {
-   {"vdda-1p2", 21800, 4 },/* 1.2 V */
-   {"vdda-0p9", 36000, 32 },   /* 0.9 V */
-   },
-};
-
 static void __iomem *dp_ioremap(struct platform_device *pdev, int idx, size_t 
*len)
 {
struct resource *res;
@@ -298,12 +290,6 @@ static int dp_parser_parse(struct dp_parser *parser)
if (rc)
return rc;
 
-   /* Map the corresponding regulator information according to
-* version. Currently, since we only have one supported platform,
-* mapping the regulator directly.
-*/
-   parser->regulator_cfg = _dp_reg_cfg;
-
return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h 
b/drivers/gpu/drm/msm/dp/dp_parser.h
index 3a4d797..b56b4d7 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -101,11 +101,6 @@ struct dp_reg_entry {
int disable_load;
 };
 
-struct dp_regulator_cfg {
-   int num;
-   struct dp_reg_entry regs[DP_DEV_REGULATOR_MAX];
-};
-
 /**
  * struct dp_parser - DP parser's data exposed to clients
  *
@@ -121,7 +116,6 @@ struct dp_parser {
struct dp_pinctrl pinctrl;
struct dp_io io;
struct dp_display_data disp_data;
-   const struct dp_regulator_cfg *regulator_cfg;
u32 max_dp_lanes;
struct drm_bridge *next_bridge;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_power.c 
b/drivers/gpu/drm/msm/dp/dp_power.c
index d9e0117..b52ac1d 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.c
+++ b/drivers/gpu/drm/msm/dp/dp_power.c
@@ -20,82 +20,10 @@ struct dp_power_private {
struct clk *link_clk_src;
struct clk *pixel_provider;
struct clk *link_provider;
-   struct regulator_bulk_data supplies[DP_DEV_REGULATOR_MAX];
 
struct dp_power dp_power;
 };
 
-static void dp_power_regulator_disable(struct dp_power_private *power)
-{
-   struct regulator_bulk_data *s = power->supplies;
-   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
-   int num = power->parser->regulator_cfg->num;
-   int i;
-
-   DBG("");
-   for (i = num - 1; i >= 0; i--)
-   if (regs[i].disable_load >= 0)
-   regulator_set_load(s[i].consumer,
-  regs[i].disable_load);
-
-   regulator_bulk_disable(num, s);
-}
-
-static int dp_power_regulator_enable(struct dp_power_private *power)
-{
-   struct regulator_bulk_data *s = power->supplies;
-   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
-   int num = power->parser->regulator_cfg->num;
-   int ret, i;
-
-   DBG("");
-   for (i = 0; i < num; i++) {
-   if (regs[i].enable_load >= 0) {
-   ret = regulator_set_load(s[i].consumer,
-regs[i].enable_load);
-   if (ret < 0) {
-   pr_err("regulator %d set op mode failed, %d\n",
-   i, ret);
-   goto fail;
-   }
-   }
-   }
-
-   ret = regulator_bulk_enable(num, s);
-   if (ret < 0) {
-   pr_err("regulator enable failed, %d\n", ret);
-   goto fail;
-   }
-
-   return 0;
-
-fail:
-   for (i--; i >= 0; i--)
-   regulator_set_load(s[i].consumer, regs[i].disable_load);
-   return ret;
-}
-
-static int dp_power_regulator_init(struct dp_power_private *power)
-{
-   struct regulator_bulk_data *s = power->supplies;
-   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
-   struct platform_device *pdev = power->pdev;
-   int num = power->parser->regulator_cfg->num;
-   int i, ret;
-
-   for (i = 0; i < num; i++)
-   s[i].supply = regs[i].name;
-
-   ret = devm_regulator_bulk_get(>dev, num, s);
-   if (ret < 0) {
-   pr_err("%s: failed to init regulator, ret=%d\n",
-   __func__, ret);
-   return ret;
-   }
-
-

[PATCH v2 1/2] phy/qcom: add regulator_set_load to edp/dp phy

This patch add regulator_set_load() to both eDP and DP phy driver
to have totally control regulators.

Changes in v2:
-- no regulator_set_laod() before disable regulator

Signed-off-by: Kuogee Hsieh 
---
 drivers/phy/qualcomm/phy-qcom-edp.c | 25 +
 drivers/phy/qualcomm/phy-qcom-qmp.c | 13 +
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c 
b/drivers/phy/qualcomm/phy-qcom-edp.c
index cacd32f..9b55095 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -87,17 +87,24 @@ struct qcom_edp {
 
struct clk_bulk_data clks[2];
struct regulator_bulk_data supplies[2];
+   int enable_load[2];
+   int disable_load[2];
 };
 
 static int qcom_edp_phy_init(struct phy *phy)
 {
struct qcom_edp *edp = phy_get_drvdata(phy);
int ret;
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;
 
-   ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
+   ret = regulator_bulk_enable(num_consumers, edp->supplies);
if (ret)
return ret;
 
+   for (i = num_consumers - 1; i >= 0; --i)
+   regulator_set_load(edp->supplies[i].consumer, 
edp->enable_load[i]);
+
ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), edp->clks);
if (ret)
goto out_disable_supplies;
@@ -425,9 +432,15 @@ static int qcom_edp_phy_power_off(struct phy *phy)
 static int qcom_edp_phy_exit(struct phy *phy)
 {
struct qcom_edp *edp = phy_get_drvdata(phy);
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;
 
clk_bulk_disable_unprepare(ARRAY_SIZE(edp->clks), edp->clks);
-   regulator_bulk_disable(ARRAY_SIZE(edp->supplies), edp->supplies);
+
+   for (i = num_consumers - 1; i >= 0; --i)
+   regulator_set_load(edp->supplies[i].consumer, 
edp->disable_load[i]);
+
+   regulator_bulk_disable(num_consumers, edp->supplies);
 
return 0;
 }
@@ -633,8 +646,12 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
if (ret)
return ret;
 
-   edp->supplies[0].supply = "vdda-phy";
-   edp->supplies[1].supply = "vdda-pll";
+   edp->supplies[0].supply = "vdda-1p2";
+   edp->supplies[1].supply = "vdda-0p9";
+   edp->enable_load[0] = 21800;/* 1.2 V */
+   edp->enable_load[1] = 36000;/* 1.2 V */
+   edp->disable_load[0] = 4;   /* 0.9 V */
+   edp->disable_load[1] = 4;   /* 10.9V */
ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), 
edp->supplies);
if (ret)
return ret;
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
b/drivers/phy/qualcomm/phy-qcom-qmp.c
index b144ae1..0a4c8a8 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -3130,6 +3130,7 @@ struct qmp_phy_cfg {
int num_resets;
/* regulators to be requested */
const char * const *vreg_list;
+   const unsigned int *vreg_enable_load;
int num_vregs;
 
/* array of registers with different offsets */
@@ -3346,6 +3347,10 @@ static const char * const qmp_phy_vreg_l[] = {
"vdda-phy", "vdda-pll",
 };
 
+static const unsigned int qmp_phy_vreg_enable_load[] = {
+   21800, 36000
+};
+
 static const struct qmp_phy_cfg ipq8074_usb3phy_cfg = {
.type   = PHY_TYPE_USB3,
.nlanes = 1,
@@ -4072,6 +4077,7 @@ static const struct qmp_phy_cfg sm8250_usb3phy_cfg = {
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
.vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,
.num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
.regs   = qmp_v4_usb3phy_regs_layout,
 
@@ -4139,6 +4145,7 @@ static const struct qmp_phy_cfg sm8250_dpphy_cfg = {
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
.vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,
.num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
.regs   = qmp_v4_usb3phy_regs_layout,
 
@@ -5008,6 +5015,11 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)
return 0;
}
 
+   if (cfg->vreg_enable_load) {
+   for (i = cfg->num_vregs - 1; i >= 0; --i)
+   regulator_set_load(qmp->vregs[i].consumer, 
cfg->vreg_enable_load[i]);
+   }
+
/* turn on regulator supplies */
ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
if (ret) {
@@ -5116,6 +5128,7 @@ static int qcom_qmp_phy_com_exit(struct qmp_phy *qphy)
 
clk_bulk_disable_unprepare(cfg->num_clks, qmp->clks);
 
+   /* no minimum

[PATCH v2 0/2] eDP/DP Phy vdda realted function

1) add regulator_set_load() to eDP/DP phy
2) remove vdda related function out of eDP/DP controller

Kuogee Hsieh (2):
  phy/qcom: add regulator_set_load to edp/dp phy
  drm/msm/dp: delete vdda regulator related functions from eDP/DP
controller

 drivers/gpu/drm/msm/dp/dp_parser.c  | 14 --
 drivers/gpu/drm/msm/dp/dp_parser.h  |  6 ---
 drivers/gpu/drm/msm/dp/dp_power.c   | 95 +
 drivers/phy/qualcomm/phy-qcom-edp.c | 25 --
 drivers/phy/qualcomm/phy-qcom-qmp.c | 13 +
 5 files changed, 36 insertions(+), 117 deletions(-)

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

Re: [PATCH v1 1/2] phy/qcom: add regulator_set_load to edp/dp phy




On 5/18/2022 2:29 AM, Vinod Koul wrote:

On 17-05-22, 10:25, Kuogee Hsieh wrote:

pls use the correct subsystem tag, "phy: xxx" in this case


This patch add regulator_set_load() to both eDP and DP phy driver
to have totally control regulators.

Can you explain what is meant by "totally control regulators"


Original regulator_set_load() is done by DP controller.

This patch has moved regulator_set_load() from DP controller to DP phy.

Therefore DP phy has total control of both vdda-phy and vda-pll regulators.





Signed-off-by: Kuogee Hsieh 
---
  drivers/phy/qualcomm/phy-qcom-edp.c | 25 +
  drivers/phy/qualcomm/phy-qcom-qmp.c | 24 
  2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c 
b/drivers/phy/qualcomm/phy-qcom-edp.c
index cacd32f..9b55095 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -87,17 +87,24 @@ struct qcom_edp {
  
  	struct clk_bulk_data clks[2];

struct regulator_bulk_data supplies[2];
+   int enable_load[2];
+   int disable_load[2];
  };
  
  static int qcom_edp_phy_init(struct phy *phy)

  {
struct qcom_edp *edp = phy_get_drvdata(phy);
int ret;
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;
  
-	ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);

+   ret = regulator_bulk_enable(num_consumers, edp->supplies);
if (ret)
return ret;
  
+	for (i = num_consumers - 1; i >= 0; --i)

+   regulator_set_load(edp->supplies[i].consumer, 
edp->enable_load[i]);
+
ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), edp->clks);
if (ret)
goto out_disable_supplies;
@@ -425,9 +432,15 @@ static int qcom_edp_phy_power_off(struct phy *phy)
  static int qcom_edp_phy_exit(struct phy *phy)
  {
struct qcom_edp *edp = phy_get_drvdata(phy);
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;
  
  	clk_bulk_disable_unprepare(ARRAY_SIZE(edp->clks), edp->clks);

-   regulator_bulk_disable(ARRAY_SIZE(edp->supplies), edp->supplies);
+
+   for (i = num_consumers - 1; i >= 0; --i)
+   regulator_set_load(edp->supplies[i].consumer, 
edp->disable_load[i]);
+
+   regulator_bulk_disable(num_consumers, edp->supplies);
  
  	return 0;

  }
@@ -633,8 +646,12 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
if (ret)
return ret;
  
-	edp->supplies[0].supply = "vdda-phy";

-   edp->supplies[1].supply = "vdda-pll";
+   edp->supplies[0].supply = "vdda-1p2";
+   edp->supplies[1].supply = "vdda-0p9";

These are documented in bindings, so cannot be removed, Reminder binding
is an ABI
  
You have not documented the new names either...



+   edp->enable_load[0] = 21800; /* 1.2 V */
+   edp->enable_load[1] = 36000; /* 1.2 V */
+   edp->disable_load[0] = 4;/* 0.9 V */
+   edp->disable_load[1] = 4;/* 10.9V */

is that correct, 10.9V?


ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), 
edp->supplies);
if (ret)
return ret;
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
b/drivers/phy/qualcomm/phy-qcom-qmp.c
index b144ae1..c589231 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c

This is a different driver, so should be a different patch!


@@ -3130,6 +3130,8 @@ struct qmp_phy_cfg {
int num_resets;
/* regulators to be requested */
const char * const *vreg_list;
+   const unsigned int *vreg_enable_load;
+   const unsigned int *vreg_disable_load;
int num_vregs;
  
  	/* array of registers with different offsets */

@@ -3346,6 +3348,14 @@ static const char * const qmp_phy_vreg_l[] = {
"vdda-phy", "vdda-pll",
  };
  
+static const unsigned int qmp_phy_vreg_enable_load[] = {

+   21800, 36000
+};
+
+static const unsigned int qmp_phy_vreg_disable_load[] = {
+   4, 32
+};
+
  static const struct qmp_phy_cfg ipq8074_usb3phy_cfg = {
.type   = PHY_TYPE_USB3,
.nlanes = 1,
@@ -4072,6 +4082,8 @@ static const struct qmp_phy_cfg sm8250_usb3phy_cfg = {
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
.vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,
+   .vreg_disable_load  = qmp_phy_vreg_disable_load,
.num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
.regs   = qmp_v4_usb3phy_regs_layout,
  
@@ -4139,6 +4151,8 @@ static const struct qmp_phy_cfg sm8250_dpphy_cfg = {

.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
.vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   =

Re: [RFC] drm/kms: control display brightness through drm_connector properties

2022-05-18 Thread Ville Syrjälä

On Wed, May 18, 2022 at 02:59:58PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 4/14/22 15:10, Jani Nikula wrote:
> > There are some cases where we can actually get a rough PWM/luminance
> > curve from i915 opregion. I think maybe 16 data points. We've never
> > exposed that. My idea was that you'd have a property where you could add
> > data points for the curve, it could get pre-populated by the kernel if
> > the kernel knows how to do it, defaulting to linear, but it could also
> > be set or adjusted by userspace. The point would be that the userspace
> > adjusts brightness linearly, and the kernel would use the curve data
> > points to adjust it non-linearly. The userspace could have completely
> > separated brightness adjustment and curve adjustment, and the brightness
> > adjustment would be dead simple.
> 
> Interesting, I guess this could be a future feature addition on top
> of my work.

Here's an outdated branch:
https://github.com/vsyrjala/linux/commits/blcm_backlight

Wrote that some years ago after getting fed up with the useless
non-linear respose of the brightness up/down buttons on my laptop.
Been running it ever since.

-- 
Ville Syrjälä
Intel

Re: [RFC] drm/kms: control display brightness through drm_connector properties

On Wed, 18 May 2022, Hans de Goede  wrote:
> So how about: display_brightness or panel_brightness ?

This is a prime opportunity to look at all the existing properties, and
come up with a new combination of capitalization, spacing, hyphens,
underscores, etc. to accompany the total lack of unification we
currently have. DisPLay_brIgh7ne55. :p

I think "display_brightness" is probably fine.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH v2 05/12] drm: bridge: samsung-dsim: Add DSI init in bridge pre_enable()

2022-05-18 Thread Marek Szyprowski

On 11.05.2022 17:02, Marek Szyprowski wrote:
> On 04.05.2022 13:40, Jagan Teki wrote:
>> Host transfer() in DSI master will invoke only when the DSI commands
>> are sent from DSI devices like DSI Panel or DSI bridges and this
>> host transfer wouldn't invoke for I2C-based-DSI bridge drivers.
>>
>> Handling DSI host initialization in transfer calls misses the
>> controller setup for I2C configured DSI bridges.
>>
>> This patch adds the DSI initialization from transfer to bridge
>> pre_enable as the bridge pre_enable API is invoked by core as
>> it is common across all classes of DSI device drivers.
>>
>> v2:
>> * check initialized state in samsung_dsim_init
>>
>> v1:
>> * keep DSI init in host transfer
>>
>> Signed-off-by: Jagan Teki 
>
> This doesn't work with Exynos DSI and DSI panels. Here is a bit more 
> detailed explanation and my solution for this:
>
> https://lore.kernel.org/all/e96197f9-948a-997e-5453-9f9d179b5...@samsung.com/ 
>


I wonder if your modification works on i.MX8MM with a pure DSI-based 
panel/bridge. In your tree I only see that you have tested it with the 
i2c-controlled DSI-to-LVDS converter.


After the comments from the above mentioned thread I've reworked the 
initialization again. It looks that the ultimate solution for both 
worlds is to call samsung_dsim_init() twice, once in pre_enable, then 
before the first host_transfer, see 
https://github.com/mszyprow/linux/tree/v5.18-next-20220511-dsi-rework-v2

This way at least it works fine on all my Exynos based test boards.

>> ---
>>   drivers/gpu/drm/bridge/samsung-dsim.c | 18 --
>>   1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>> index 60dc863113a0..b9361af5ef2d 100644
>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>> @@ -1259,6 +1259,9 @@ static int samsung_dsim_init(struct 
>> samsung_dsim *dsi)
>>   {
>>   const struct samsung_dsim_driver_data *driver_data = 
>> dsi->driver_data;
>>   +    if (dsi->state & DSIM_STATE_INITIALIZED)
>> +    return 0;
>> +
>>   samsung_dsim_reset(dsi);
>>   samsung_dsim_enable_irq(dsi);
>>   @@ -1271,6 +1274,8 @@ static int samsung_dsim_init(struct 
>> samsung_dsim *dsi)
>>   samsung_dsim_set_phy_ctrl(dsi);
>>   samsung_dsim_init_link(dsi);
>>   +    dsi->state |= DSIM_STATE_INITIALIZED;
>> +
>>   return 0;
>>   }
>>   @@ -1290,6 +1295,10 @@ static void 
>> samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
>>   }
>>     dsi->state |= DSIM_STATE_ENABLED;
>> +
>> +    ret = samsung_dsim_init(dsi);
>> +    if (ret)
>> +    return;
>>   }
>>     static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
>> @@ -1464,12 +1473,9 @@ static ssize_t 
>> samsung_dsim_host_transfer(struct mipi_dsi_host *host,
>>   if (!(dsi->state & DSIM_STATE_ENABLED))
>>   return -EINVAL;
>>   -    if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
>> -    ret = samsung_dsim_init(dsi);
>> -    if (ret)
>> -    return ret;
>> -    dsi->state |= DSIM_STATE_INITIALIZED;
>> -    }
>> +    ret = samsung_dsim_init(dsi);
>> +    if (ret)
>> +    return ret;
>>     ret = mipi_dsi_create_packet(, msg);
>>   if (ret < 0)
>
> Best regards

Best regards
-- 
Marek Szyprowski, PhD
Samsung R Institute Poland

Re: [Intel-gfx] [V2 3/3] drm/amd/display: Move connector debugfs to drm

2022-05-18 Thread Harry Wentland




On 5/18/22 01:38, Modem, Bhanuprakash wrote:
> On Mon-16-05-2022 02:09 pm, Jani Nikula wrote:
>> On Mon, 02 May 2022, Harry Wentland  wrote:
>>> Both the kernel and IGT series look good to me.
>>>
>>> I recommend you merge the entire kernel set as one into drm-next. We
>>> can pull it into amd-staging-drm-next so as not to break our CI once
>>> the IGT patches land.
> 
> @Harry
> 
> Can we have your Ack-by to merge this series via drm-misc-next?
> IGT patches are already landed. :-)

Sure. :)

Acked-by: Harry Wentland 

Harry

> 
> - Bhanu
> 
>>
>> Can we read that as an ack to merge via drm-misc-next? :)
>>
>> BR,
>> Jani.
>>
>>
>

Re: [greybus-dev] Re: [PATCH] [v2] Kbuild: move to -std=gnu11

2022-05-18 Thread Guenter Roeck


On 5/18/22 00:46, Arnd Bergmann wrote:

On Mon, May 16, 2022 at 3:19 PM Guenter Roeck  wrote:

On 5/16/22 06:31, Greg KH wrote:

On Mon, May 16, 2022 at 06:10:23AM -0700, Guenter Roeck wrote:

On Mon, Feb 28, 2022 at 11:27:43AM +0100, Arnd Bergmann wrote:

From: Arnd Bergmann 

During a patch discussion, Linus brought up the option of changing
the C standard version from gnu89 to gnu99, which allows using variable
declaration inside of a for() loop. While the C99, C11 and later standards
introduce many other features, most of these are already available in
gnu89 as GNU extensions as well.


The downside is that backporting affected patches to older kernel branches
now fails with error messages such as

mm/kfence/core.c: In function ‘kfence_init_pool’:
mm/kfence/core.c:595:2: error: ‘for’ loop initial declarations are only allowed 
in C99 or C11 mode

Just something to keep in mind when writing patches.


I just ran across this very issue on this commit.  It's an easy fixup
for 5.17.y to make this work, so I did that in my tree.  If this gets to
be too much, we might need to reconsider adding c11 to older stable
kernels.



I think I'll do just that for ChromeOS; I don't want to have to deal
with the backports, and we are using recent compilers anyway.


I think it would be better not to have the --std=gnu11 change in the older
stable kernels by default, as this has introduced build warnings and other
smaller issues, as well as raising the minimum compiler version.

The users that are stuck on older kernels for some reason tend to
overlap with those on older compilers. One example here is Android,
which used to ship with a gcc-4.9 build as the only non-clang toolchain,
and was using this for building their kernels. If someone wants to
pull in stable updates into an older Android, this would fail with
-std=gnu11. Others may be in the same situation.

Changing some of the 5.x stable branches to -std=gnu11 is probably
less of a problem, but I would not know where to draw the line exactly.
Maybe check with the Android team to see what the newest kernel is
that they expect to be built with the old gcc-4.9.



I don't think they still build anything with gcc. We (ChromeOS) only
need it for test builds of chromeos-4.4 (sigh), and that will hopefully
be gone in a couple of months.

We already enabled -std=gnu11 in chromeos-5.10 and chromeos-5.15.
We'll see if that is possible with chromeos-5.4 as well.
We won't bother with older kernel branches, but those should not
get many patches from upstream outside stable release merges,
so it is less of a problem.

Guenter

Re: [PATCH V2 0/3] DSI host and peripheral initialisation ordering

2022-05-18 Thread Marek Szyprowski

Hi Dave,

On 11.05.2022 17:47, Dave Stevenson wrote:
> On Wed, 11 May 2022 at 15:58, Marek Szyprowski  
> wrote:
>> On 05.04.2022 13:43, Dave Stevenson wrote:
>>> On Fri, 18 Mar 2022 at 12:25, Dave Stevenson
>>>   wrote:
 On Fri, 4 Mar 2022 at 15:18, Dave Stevenson
   wrote:
> Hi All
 A gentle ping on this series. Any comments on the approach?
 Thanks.
>>> I realise the merge window has just closed and therefore folks have
>>> been busy, but no responses on this after a month?
>>>
>>> Do I give up and submit a patch to document that DSI is broken and no one 
>>> cares?
>> Thanks for pointing this patchset in the 'drm: bridge: Add Samsung MIPI
>> DSIM bridge' thread, otherwise I would miss it since I'm not involved
>> much in the DRM development.
>>
>> This resolves most of the issues in the Exynos DSI and its recent
>> conversion to the drm bridge framework. I've added the needed
>> prepare_upstream_first flags to the panels and everything works fine
>> without the bridge chain order hacks.
>>
>> Feel free to add:
>>
>> Tested-by: Marek Szyprowski 
> Thanks for testing it. I was almost at the stage of abandoning the patch set.
>
>> The only remaining thing to resolve is the moment of enabling DSI host.
>> The proper sequence is:
>>
>> 1. host power on, 2. device power on, 3. host init, 4. device init, 5.
>> video enable.
>>
>> #1 is done in dsi's pre_enable, #2 is done in panel's prepare. #3 was so
>> far done in the first host transfer call, which usually happens in
>> panel's prepare, then the #4 happens. Then video enable is done in the
>> enable callbacks.
> What's your definition of host power on and host init here? What state
> are you defining the DSI interface to be in after each operation?

Well, lets start from the point that I'm not a DSI specialist nor I'm 
not the exynos-dsi author. I just played a bit with the code trying to 
restore proper driver operation on the various Exynos based boards I have.

By the host/device power on I mean enabling their power regulators. By 
host init I mean executing the samsung_dsim_init() function, which 
basically sets the lp-11 state if I understand it right.

>> Jagan wants to move it to the dsi host pre_enable() to let it work with
>> DSI bridges controlled over different interfaces
>> (https://lore.kernel.org/all/20220504114021.33265-6-ja...@amarulasolutions.com/
>> ).
> I think I'm in agreement with Jagan.
> As documented in patch 4/4:
> + * A DSI host should keep the PHY powered down until the pre_enable
> operation is
> + * called. All lanes are in an undefined idle state up to this point, and it
> + * must not be assumed that it is LP-11.
> + * pre_enable should initialise the PHY, set the data lanes to LP-11, and the
> + * clock lane to either LP-11 or HS depending on the mode_flag
> + * %MIPI_DSI_CLOCK_NON_CONTINUOUS.

Right, this theory makes sense.

However Exynos DSI for some reasons did the host initialization in the 
first call of the samsung_dsim_host_transfer(). If I moved the host 
initialization to pre_enable (before powering the panel on), executing 
DSI commands failed (timeout). This issue happens on all boards I have 
access (Trats, Trats2, Arndale, TM2e), so this must be an issue with 
Exynos DSI host itself not related to particular panel/bridge.

If I call samsung_dsim_init() once again, before issuing the first DSI 
command, then everything works fine. I've tried to check which part of 
that function is needed to be executed before transferring the commands, 
but it turned out that the complete host reset and (re)configuration is 
necessary. It looks that the initialization will need to be done twice, 
first time in the pre_enable to satisfy Jagan case, then on the first 
dsi transfer to make it work with real DSI panels.

Here is a git repo with such change: 
https://github.com/mszyprow/linux/tree/v5.18-next-20220511-dsi-rework-v2

>> This however fails on Exynos with DSI panels, because when dsi's
>> pre_enable is called, the dsi device is not yet powered.
> Sorry, I'm not following what the issue is here? DSI lanes being at
> LP-11 when the sink isn't powered, so potential for leakage through
> the device?

I also have no idea why sending DSI commands fails if host is 
initialized without device being powered up first. Maybe powering it 
later causes some glitches on the lines? However it looks doing the 
initialization again on the first transfer is enough to fix it.

> In which case the device should NOT set pre_enable_upstream first, and
> the host gets powered up and down with each host_transfer call the
> device makes in pre_enable.

Doing the initialization on each host_transfer also is not an option, 
because in such case the panel is not initialized properly. I get no 
errors, but also there is no valid display on the panel in such case.

> (Whilst I can't claim that I know of every single DSI device, most
> datasheets I've encountered have required LP-11 on the lanes before
> powering up the

[PATCH 2/5] dma-buf: cleanup dma_fence_unwrap implementation

Move the code from the inline functions into exported functions.

Signed-off-by: Christian König 
Acked-by: Daniel Vetter 
---
 drivers/dma-buf/Makefile   |  2 +-
 drivers/dma-buf/dma-fence-unwrap.c | 59 ++
 include/linux/dma-fence-unwrap.h   | 52 ++
 3 files changed, 64 insertions(+), 49 deletions(-)
 create mode 100644 drivers/dma-buf/dma-fence-unwrap.c

diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
index 4c9eb53ba3f8..70ec901edf2c 100644
--- a/drivers/dma-buf/Makefile
+++ b/drivers/dma-buf/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \
-dma-resv.o
+dma-fence-unwrap.o dma-resv.o
 obj-$(CONFIG_DMABUF_HEAPS) += dma-heap.o
 obj-$(CONFIG_DMABUF_HEAPS) += heaps/
 obj-$(CONFIG_SYNC_FILE)+= sync_file.o
diff --git a/drivers/dma-buf/dma-fence-unwrap.c 
b/drivers/dma-buf/dma-fence-unwrap.c
new file mode 100644
index ..711be125428c
--- /dev/null
+++ b/drivers/dma-buf/dma-fence-unwrap.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * dma-fence-util: misc functions for dma_fence objects
+ *
+ * Copyright (C) 2022 Advanced Micro Devices, Inc.
+ * Authors:
+ * Christian König 
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+/* Internal helper to start new array iteration, don't use directly */
+static struct dma_fence *
+__dma_fence_unwrap_array(struct dma_fence_unwrap *cursor)
+{
+   cursor->array = dma_fence_chain_contained(cursor->chain);
+   cursor->index = 0;
+   return dma_fence_array_first(cursor->array);
+}
+
+/**
+ * dma_fence_unwrap_first - return the first fence from fence containers
+ * @head: the entrypoint into the containers
+ * @cursor: current position inside the containers
+ *
+ * Unwraps potential dma_fence_chain/dma_fence_array containers and return the
+ * first fence.
+ */
+struct dma_fence *dma_fence_unwrap_first(struct dma_fence *head,
+struct dma_fence_unwrap *cursor)
+{
+   cursor->chain = dma_fence_get(head);
+   return __dma_fence_unwrap_array(cursor);
+}
+EXPORT_SYMBOL_GPL(dma_fence_unwrap_first);
+
+/**
+ * dma_fence_unwrap_next - return the next fence from a fence containers
+ * @cursor: current position inside the containers
+ *
+ * Continue unwrapping the dma_fence_chain/dma_fence_array containers and 
return
+ * the next fence from them.
+ */
+struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor)
+{
+   struct dma_fence *tmp;
+
+   ++cursor->index;
+   tmp = dma_fence_array_next(cursor->array, cursor->index);
+   if (tmp)
+   return tmp;
+
+   cursor->chain = dma_fence_chain_walk(cursor->chain);
+   return __dma_fence_unwrap_array(cursor);
+}
+EXPORT_SYMBOL_GPL(dma_fence_unwrap_next);
diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h
index 77e335a1bcac..e7c219da4ed7 100644
--- a/include/linux/dma-fence-unwrap.h
+++ b/include/linux/dma-fence-unwrap.h
@@ -1,7 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * fence-chain: chain fences together in a timeline
- *
  * Copyright (C) 2022 Advanced Micro Devices, Inc.
  * Authors:
  * Christian König 
@@ -10,8 +8,7 @@
 #ifndef __LINUX_DMA_FENCE_UNWRAP_H
 #define __LINUX_DMA_FENCE_UNWRAP_H
 
-#include 
-#include 
+struct dma_fence;
 
 /**
  * struct dma_fence_unwrap - cursor into the container structure
@@ -33,50 +30,9 @@ struct dma_fence_unwrap {
unsigned int index;
 };
 
-/* Internal helper to start new array iteration, don't use directly */
-static inline struct dma_fence *
-__dma_fence_unwrap_array(struct dma_fence_unwrap * cursor)
-{
-   cursor->array = dma_fence_chain_contained(cursor->chain);
-   cursor->index = 0;
-   return dma_fence_array_first(cursor->array);
-}
-
-/**
- * dma_fence_unwrap_first - return the first fence from fence containers
- * @head: the entrypoint into the containers
- * @cursor: current position inside the containers
- *
- * Unwraps potential dma_fence_chain/dma_fence_array containers and return the
- * first fence.
- */
-static inline struct dma_fence *
-dma_fence_unwrap_first(struct dma_fence *head, struct dma_fence_unwrap *cursor)
-{
-   cursor->chain = dma_fence_get(head);
-   return __dma_fence_unwrap_array(cursor);
-}
-
-/**
- * dma_fence_unwrap_next - return the next fence from a fence containers
- * @cursor: current position inside the containers
- *
- * Continue unwrapping the dma_fence_chain/dma_fence_array containers and 
return
- * the next fence from them.
- */
-static inline struct dma_fence *
-dma_fence_unwrap_next(struct dma_fence_unwrap *cursor)
-{
-   struct dma_fence *tmp;
-
-   ++cursor->index;
-   tmp = dma_fence_array_next(cursor->array, cursor->index);
-   if (tmp)
-   return tmp;
-
-   cursor->chain =

[PATCH 5/5] drm: use dma_fence_unwrap_merge() in drm_syncobj

The unwrap merge function is now intended for this use case.

Signed-off-by: Christian König 
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_syncobj.c | 57 +--
 1 file changed, 7 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 7e48dcd1bee4..bbad9e4696e7 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -184,6 +184,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -853,57 +854,12 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, 
void *data,
>handle);
 }
 
-
-/*
- * Try to flatten a dma_fence_chain into a dma_fence_array so that it can be
- * added as timeline fence to a chain again.
- */
-static int drm_syncobj_flatten_chain(struct dma_fence **f)
-{
-   struct dma_fence_chain *chain = to_dma_fence_chain(*f);
-   struct dma_fence *tmp, **fences;
-   struct dma_fence_array *array;
-   unsigned int count;
-
-   if (!chain)
-   return 0;
-
-   count = 0;
-   dma_fence_chain_for_each(tmp, >base)
-   ++count;
-
-   fences = kmalloc_array(count, sizeof(*fences), GFP_KERNEL);
-   if (!fences)
-   return -ENOMEM;
-
-   count = 0;
-   dma_fence_chain_for_each(tmp, >base)
-   fences[count++] = dma_fence_get(tmp);
-
-   array = dma_fence_array_create(count, fences,
-  dma_fence_context_alloc(1),
-  1, false);
-   if (!array)
-   goto free_fences;
-
-   dma_fence_put(*f);
-   *f = >base;
-   return 0;
-
-free_fences:
-   while (count--)
-   dma_fence_put(fences[count]);
-
-   kfree(fences);
-   return -ENOMEM;
-}
-
 static int drm_syncobj_transfer_to_timeline(struct drm_file *file_private,
struct drm_syncobj_transfer *args)
 {
struct drm_syncobj *timeline_syncobj = NULL;
+   struct dma_fence *fence, *tmp;
struct dma_fence_chain *chain;
-   struct dma_fence *fence;
int ret;
 
timeline_syncobj = drm_syncobj_find(file_private, args->dst_handle);
@@ -912,13 +868,14 @@ static int drm_syncobj_transfer_to_timeline(struct 
drm_file *file_private,
}
ret = drm_syncobj_find_fence(file_private, args->src_handle,
 args->src_point, args->flags,
-);
+);
if (ret)
goto err_put_timeline;
 
-   ret = drm_syncobj_flatten_chain();
-   if (ret)
-   goto err_free_fence;
+   fence = dma_fence_unwrap_merge(tmp);
+   dma_fence_put(tmp);
+   if (!fence)
+   goto err_put_timeline;
 
chain = dma_fence_chain_alloc();
if (!chain) {
-- 
2.25.1

[PATCH 4/5] dma-buf: generalize dma_fence unwrap & merging v3

Introduce a dma_fence_unwrap_merge() macro which allows to unwrap fences
which potentially can be containers as well and then merge them back
together into a flat dma_fence_array.

v2: rename the function, add some more comments about how the wrapper is
used, move filtering of signaled fences into the unwrap iterator,
add complex selftest which covers more cases.
v3: fix signaled fence filtering once more

Signed-off-by: Christian König 
Reviewed-by: Daniel Vetter 
---
 drivers/dma-buf/dma-fence-unwrap.c| 103 ++
 drivers/dma-buf/st-dma-fence-unwrap.c | 109 +++
 drivers/dma-buf/sync_file.c   | 119 ++
 include/linux/dma-fence-unwrap.h  |  24 ++
 4 files changed, 242 insertions(+), 113 deletions(-)

diff --git a/drivers/dma-buf/dma-fence-unwrap.c 
b/drivers/dma-buf/dma-fence-unwrap.c
index 711be125428c..502a65ea6d44 100644
--- a/drivers/dma-buf/dma-fence-unwrap.c
+++ b/drivers/dma-buf/dma-fence-unwrap.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Internal helper to start new array iteration, don't use directly */
 static struct dma_fence *
@@ -57,3 +58,105 @@ struct dma_fence *dma_fence_unwrap_next(struct 
dma_fence_unwrap *cursor)
return __dma_fence_unwrap_array(cursor);
 }
 EXPORT_SYMBOL_GPL(dma_fence_unwrap_next);
+
+/* Implementation for the dma_fence_merge() marco, don't use directly */
+struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences,
+  struct dma_fence **fences,
+  struct dma_fence_unwrap *iter)
+{
+   struct dma_fence_array *result;
+   struct dma_fence *tmp, **array;
+   unsigned int i;
+   size_t count;
+
+   count = 0;
+   for (i = 0; i < num_fences; ++i) {
+   dma_fence_unwrap_for_each(tmp, [i], fences[i])
+   ++count;
+   }
+
+   if (count == 0)
+   return dma_fence_get_stub();
+
+   array = kmalloc_array(count, sizeof(*array), GFP_KERNEL);
+   if (!array)
+   return NULL;
+
+   /*
+* This trashes the input fence array and uses it as position for the
+* following merge loop. This works because the dma_fence_merge()
+* wrapper macro is creating this temporary array on the stack together
+* with the iterators.
+*/
+   for (i = 0; i < num_fences; ++i)
+   fences[i] = dma_fence_unwrap_first(fences[i], [i]);
+
+   count = 0;
+   do {
+   unsigned int sel;
+
+restart:
+   tmp = NULL;
+   for (i = 0; i < num_fences; ++i) {
+   struct dma_fence *next;
+
+   while (fences[i] && dma_fence_is_signaled(fences[i]))
+   fences[i] = dma_fence_unwrap_next([i]);
+
+   next = fences[i];
+   if (!next)
+   continue;
+
+   /*
+* We can't guarantee that inpute fences are ordered by
+* context, but it is still quite likely when this
+* function is used multiple times. So attempt to order
+* the fences by context as we pass over them and merge
+* fences with the same context.
+*/
+   if (!tmp || tmp->context > next->context) {
+   tmp = next;
+   sel = i;
+
+   } else if (tmp->context < next->context) {
+   continue;
+
+   } else if (dma_fence_is_later(tmp, next)) {
+   fences[i] = dma_fence_unwrap_next([i]);
+   goto restart;
+   } else {
+   fences[sel] = dma_fence_unwrap_next([sel]);
+   goto restart;
+   }
+   }
+
+   if (tmp) {
+   array[count++] = dma_fence_get(tmp);
+   fences[sel] = dma_fence_unwrap_next([sel]);
+   }
+   } while (tmp);
+
+   if (count == 0) {
+   tmp = dma_fence_get_stub();
+   goto return_tmp;
+   }
+
+   if (count == 1) {
+   tmp = array[0];
+   goto return_tmp;
+   }
+
+   result = dma_fence_array_create(count, array,
+   dma_fence_context_alloc(1),
+   1, false);
+   if (!result) {
+   tmp = NULL;
+   goto return_tmp;
+   }
+   return >base;
+
+return_tmp:
+   kfree(array);
+   return tmp;
+}
+EXPORT_SYMBOL_GPL(__dma_fence_unwrap_merge);
diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c 
b/drivers/dma-buf/st-dma-fence-unwrap.c

[PATCH 3/5] dma-buf: return only unsignaled fences in dma_fence_unwrap_for_each v3

dma_fence_chain containers cleanup signaled fences automatically, so
filter those out from arrays as well.

v2: fix missing walk over the array
v3: massively simplify the patch and actually update the description.

Signed-off-by: Christian König 
---
 include/linux/dma-fence-unwrap.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h
index e7c219da4ed7..a4d342fef8e0 100644
--- a/include/linux/dma-fence-unwrap.h
+++ b/include/linux/dma-fence-unwrap.h
@@ -43,9 +43,13 @@ struct dma_fence *dma_fence_unwrap_next(struct 
dma_fence_unwrap *cursor);
  * Unwrap dma_fence_chain and dma_fence_array containers and deep dive into all
  * potential fences in them. If @head is just a normal fence only that one is
  * returned.
+ *
+ * Note that signalled fences are opportunistically filtered out, which
+ * means the iteration is potentially over no fence at all.
  */
 #define dma_fence_unwrap_for_each(fence, cursor, head) \
for (fence = dma_fence_unwrap_first(head, cursor); fence;   \
-fence = dma_fence_unwrap_next(cursor))
+fence = dma_fence_unwrap_next(cursor)) \
+   if (!dma_fence_is_signaled(fence))
 
 #endif
-- 
2.25.1

[PATCH 1/5] dma-buf: cleanup dma_fence_unwrap selftest v2

The selftests, fix the error handling, remove unused functions and stop
leaking memory in failed tests.

v2: fix the memory leak correctly.

Signed-off-by: Christian König 
---
 drivers/dma-buf/st-dma-fence-unwrap.c | 48 +++
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c 
b/drivers/dma-buf/st-dma-fence-unwrap.c
index 039f016b57be..e20c5a7dcfe4 100644
--- a/drivers/dma-buf/st-dma-fence-unwrap.c
+++ b/drivers/dma-buf/st-dma-fence-unwrap.c
@@ -4,27 +4,19 @@
  * Copyright (C) 2022 Advanced Micro Devices, Inc.
  */
 
+#include 
+#include 
+#include 
 #include 
-#if 0
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#endif
 
 #include "selftest.h"
 
 #define CHAIN_SZ (4 << 10)
 
-static inline struct mock_fence {
+struct mock_fence {
struct dma_fence base;
spinlock_t lock;
-} *to_mock_fence(struct dma_fence *f) {
-   return container_of(f, struct mock_fence, base);
-}
+};
 
 static const char *mock_name(struct dma_fence *f)
 {
@@ -45,7 +37,8 @@ static struct dma_fence *mock_fence(void)
return NULL;
 
spin_lock_init(>lock);
-   dma_fence_init(>base, _ops, >lock, 0, 0);
+   dma_fence_init(>base, _ops, >lock,
+  dma_fence_context_alloc(1), 1);
 
return >base;
 }
@@ -59,7 +52,7 @@ static struct dma_fence *mock_array(unsigned int num_fences, 
...)
 
fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);
if (!fences)
-   return NULL;
+   goto error_put;
 
va_start(valist, num_fences);
for (i = 0; i < num_fences; ++i)
@@ -70,13 +63,17 @@ static struct dma_fence *mock_array(unsigned int 
num_fences, ...)
   dma_fence_context_alloc(1),
   1, false);
if (!array)
-   goto cleanup;
+   goto error_free;
return >base;
 
-cleanup:
-   for (i = 0; i < num_fences; ++i)
-   dma_fence_put(fences[i]);
+error_free:
kfree(fences);
+
+error_put:
+   va_start(valist, num_fences);
+   for (i = 0; i < num_fences; ++i)
+   dma_fence_put(va_arg(valist, typeof(*fences)));
+   va_end(valist);
return NULL;
 }
 
@@ -113,7 +110,6 @@ static int sanitycheck(void *arg)
if (!chain)
return -ENOMEM;
 
-   dma_fence_signal(f);
dma_fence_put(chain);
return err;
 }
@@ -154,10 +150,8 @@ static int unwrap_array(void *arg)
err = -EINVAL;
}
 
-   dma_fence_signal(f1);
-   dma_fence_signal(f2);
dma_fence_put(array);
-   return 0;
+   return err;
 }
 
 static int unwrap_chain(void *arg)
@@ -196,10 +190,8 @@ static int unwrap_chain(void *arg)
err = -EINVAL;
}
 
-   dma_fence_signal(f1);
-   dma_fence_signal(f2);
dma_fence_put(chain);
-   return 0;
+   return err;
 }
 
 static int unwrap_chain_array(void *arg)
@@ -242,10 +234,8 @@ static int unwrap_chain_array(void *arg)
err = -EINVAL;
}
 
-   dma_fence_signal(f1);
-   dma_fence_signal(f2);
dma_fence_put(chain);
-   return 0;
+   return err;
 }
 
 int dma_fence_unwrap(void)
-- 
2.25.1

[no subject]

Just sending that to intel-gfx to let the CI systems take a look at it.

Regards,
Christian.

Re: [PATCH AUTOSEL 5.17 13/23] drm/amd/display: undo clearing of z10 related function pointers

2022-05-18 Thread Deucher, Alexander

[Public]

DCN 3.1.6 needs it, but I don't know if yellow carp needs it.  I think this is 
only applicable to kernel 5.18.  @Kazlauskas, 
Nicholas can you verify?

Alex


From: VURDIGERENATARAJ, CHANDAN 
Sent: Wednesday, May 18, 2022 8:36 AM
To: Sasha Levin ; linux-ker...@vger.kernel.org 
; sta...@vger.kernel.org 
Cc: Yang, Eric ; haonan.wa...@amd.com 
; Li, Sun peng (Leo) ; Pan, Xinhui 
; Siqueira, Rodrigo ; 
amd-...@lists.freedesktop.org ; Koenig, 
Christian ; airl...@linux.ie ; 
dri-devel@lists.freedesktop.org ; 
dan...@ffwll.ch ; wyatt.w...@amd.com ; 
Deucher, Alexander ; mikita.lip...@amd.com 
; Wentland, Harry ; Kazlauskas, 
Nicholas ; Kotarac, Pavle 
Subject: RE: [PATCH AUTOSEL 5.17 13/23] drm/amd/display: undo clearing of z10 
related function pointers

Hi,

Is S0i3 verified for DCN 3.1.6 with this?

BR,
Chandan V N

>From: Eric Yang 
>
>[ Upstream commit 9b9bd3f640640f94272a461b2dfe558f91b322c5 ]
>
> [Why]
>Z10 and S0i3 have some shared path. Previous code clean up , incorrectly 
>removed these pointers, which breaks s0i3 restore
>
> [How]
>Do not clear the function pointers based on Z10 disable.
>
>Reviewed-by: Nicholas Kazlauskas 
>Acked-by: Pavle Kotarac 
>Signed-off-by: Eric Yang 
>Signed-off-by: Alex Deucher 
>Signed-off-by: Sasha Levin 
>---
> drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c | 5 -
> 1 file changed, 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c 
>b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
>index d7559e5a99ce..e708f07fe75a 100644
>--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
>+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
>@@ -153,9 +153,4 @@ void dcn31_hw_sequencer_construct(struct dc *dc)
>dc->hwss.init_hw = dcn20_fpga_init_hw;
>dc->hwseq->funcs.init_pipes = NULL;
>}
>-  if (dc->debug.disable_z10) {
>-  /*hw not support z10 or sw disable it*/
>-  dc->hwss.z10_restore = NULL;
>-  dc->hwss.z10_save_init = NULL;
>-  }
> }
>--
>2.35.1
>

Re: [RFC] drm/kms: control display brightness through drm_connector properties

2022-05-18 Thread Hans de Goede

Hi,

On 4/14/22 15:10, Jani Nikula wrote:
> On Thu, 07 Apr 2022, Hans de Goede  wrote:
>> As discussed already several times in the past:
>>  https://www.x.org/wiki/Events/XDC2014/XDC2014GoedeBacklight/
>>  
>> https://lore.kernel.org/all/4b17ba08-39f3-57dd-5aad-d37d844b0...@linux.intel.com/
>>
>> The current userspace API for brightness control offered by
>> /sys/class/backlight devices has various issues, the biggest 2 being:
>>
>> 1. There is no way to map the backlight device to a specific
>>display-output / panel (1)
>> 2. Controlling the brightness requires root-rights requiring
>>desktop-environments to use suid-root helpers for this.
>>
>> As already discussed on various conference's hallway tracks
>> and as has been proposed on the dri-devel list once before (2),
>> it seems that there is consensus that the best way to to solve these
>> 2 issues is to add support for controlling a video-output's brightness
>> through properties on the drm_connector.
>>
>> This RFC outlines my plan to try and actually implement this,
>> which has 3 phases:
>>
>>
>> Phase 1: Stop registering multiple /sys/class/backlight devs for a single 
>> display
>> =
>>
>> On x86 there can be multiple firmware + direct-hw-access methods
>> for controlling the backlight and in some cases the kernel registers
>> multiple backlight-devices for a single internal laptop LCD panel:
>>
>> a) i915 and nouveau unconditionally register their "native" backlight dev
>>even on devices where /sys/class/backlight/acpi_video0 must be used
>>to control the backlight, relying on userspace to prefer the "firmware"
>>acpi_video0 device over "native" devices.
>> b) amdgpu and nouveau rely on the acpi_video driver initializing before
>>them, which currently causes /sys/class/backlight/acpi_video0 to usually
>>show up and then they register their own native backlight driver after
>>which the drivers/acpi/video_detect.c code unregisters the acpi_video0
>>device. This means that userspace briefly sees 2 devices and the
>>disappearing of acpi_video0 after a brief time confuses the systemd
>>backlight level save/restore code, see e.g.:
>>https://bbs.archlinux.org/viewtopic.php?id=269920
>>
>> I already have a pretty detailed plan to tackle this, which I will
>> post in a separate RFC email. I plan to start working on this right
>> away, as it will be good to have this fixed regardless.
>>
>>
>> Phase 2: Add drm_connector properties mirroring the matching backlight device
>> =
>>
>> The plan is to add a drm_connector helper function, which optionally takes
>> a pointer to the backlight device for the GPU's native backlight device,
>> which will then mirror the backlight settings from the backlight device
>> in a set of read/write brightness* properties on the connector.
>>
>> This function can then be called by GPU drivers for the drm_connector for
>> the internal panel and it will then take care of everything. When there
>> is no native GPU backlight device, or when it should not be used then
>> (on x86) the helper will use the acpi_video_get_backlight_type() to
>> determine which backlight-device should be used instead and it will find
>> + mirror that one.
>>
>>
>> Phase 3: Deprecate /sys/class/backlight uAPI
>> 
>>
>> Once most userspace has moved over to using the new drm_connector
>> brightness props, a Kconfig option can be added to stop exporting
>> the backlight-devices under /sys/class/backlight. The plan is to
>> just disable the sysfs interface and keep the existing backlight-device
>> internal kernel abstraction as is, since some abstraction for (non GPU
>> native) backlight devices will be necessary regardless.
>>
>> An alternative to disabling the sysfs class entirely, would be
>> to allow setting it to read-only through Kconfig.
>>
>>
>> What scale to use for the drm_connector bl_brightness property?
>> ===
>>
>> The tricky part of this plan is phase 2 and then esp. defining what the
>> new brightness properties will look like and how they will work.
>>
>> The biggest challenge here is to decide on a fixed scale for the main
>> brightness property, say 0-65535, using scaling where the actual hw scale
>> is different, or if this should simply be a 1:1 mirror of the current
>> backlight interface, with the actual hw scale / brightness_max value
>> exposed as a drm_connector property.
>>
>> 1:1 advantages / 0-65535 disadvantages
>> - Userspace will likely move over to the connector-props quite slowly and
>>   we can expect various userspace bits, esp. also custom user scripts, to
>>   keep using the old uAPI for a long time. Using the 2 APIs are intermixed
>>   is fine when using a 1:1 brightness scale mapping. But if we end up doing
>>

Re: [RFC PATCH v3 02/19] KVM: x86: inhibit APICv/AVIC when the guest and/or host changes apic id/base from the defaults.

On Wed, 2022-05-18 at 19:51 +0800, Chao Gao wrote:
> On Wed, May 18, 2022 at 12:50:27PM +0300, Maxim Levitsky wrote:
> > > > struct kvm_arch {
> > > > @@ -1258,6 +1260,7 @@ struct kvm_arch {
> > > > hpa_t   hv_root_tdp;
> > > > spinlock_t hv_root_tdp_lock;
> > > > #endif
> > > > +   bool apic_id_changed;
> > > 
> > > What's the value of this boolean? No one reads it.
> > 
> > I use it in later patches to kill the guest during nested VM entry 
> > if it attempts to use nested AVIC after any vCPU changed APIC ID.
> > 
> > I mentioned this boolean in the commit description.
> > 
> > This boolean avoids the need to go over all vCPUs and checking
> > if they still have the initial apic id.
> 
> Do you want to kill the guest if APIC base got changed? If yes,
> you can check if APICV_INHIBIT_REASON_RO_SETTINGS is set and save
> the boolean.

Yep, I thrown in the apic base just because I can. It doesn't matter to 
my nested AVIC logic at all, but since it is also something that guests
don't change, I also don't care if this will lead to inhibit and
killing the guest if it attempts to use nested AVIC.

That boolean should have the same value as the APICV_INHIBIT_REASON_RO_SETTINGS
inhibit, so yes I can instead check if the inhibit is active.

I don't know if that is cleaner that this boolean though, individual
inhibit value is currently not something that anybody uses in logic.

Best regards,
Maxim Levitsky


> 
> > In the future maybe we can introduce a more generic 'taint'
> > bitmap with various flags like that, indicating that the guest
> > did something unexpected.
> > 
> > BTW, the other option in regard to the nested AVIC is just to ignore this 
> > issue completely.
> > The code itself always uses vcpu_id's, thus regardless of when/how often 
> > the guest changes
> > its apic ids, my code would just use the initial APIC ID values 
> > consistently.
> > 
> > In this case I won't need this boolean.
> > 
> > > > };
> > > > 
> > > > struct kvm_vm_stat {
> > > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > > > index 66b0eb0bda94e..8996675b3ef4c 100644
> > > > --- a/arch/x86/kvm/lapic.c
> > > > +++ b/arch/x86/kvm/lapic.c
> > > > @@ -2038,6 +2038,19 @@ static void apic_manage_nmi_watchdog(struct 
> > > > kvm_lapic *apic, u32 lvt0_val)
> > > > }
> > > > }
> > > > 
> > > > +static void kvm_lapic_check_initial_apic_id(struct kvm_lapic *apic)
> > > > +{
> > > > +   if (kvm_apic_has_initial_apic_id(apic))
> > > > +   return;
> > > > +
> > > > +   pr_warn_once("APIC ID change is unsupported by KVM");
> > > 
> > > It is misleading because changing xAPIC ID is supported by KVM; it just
> > > isn't compatible with APICv. Probably this pr_warn_once() should be
> > > removed.
> > 
> > Honestly since nobody uses this feature, I am not sure if to call this 
> > supported,
> > I am sure that KVM has more bugs in regard of using non standard APIC ID.
> > This warning might hopefuly make someone complain about it if this
> > feature is actually used somewhere.
> 
> Now I got you. It is fine to me.
>

RE: [PATCH AUTOSEL 5.17 13/23] drm/amd/display: undo clearing of z10 related function pointers

2022-05-18 Thread VURDIGERENATARAJ, CHANDAN

Hi,

Is S0i3 verified for DCN 3.1.6 with this?

BR,
Chandan V N

>From: Eric Yang 
>
>[ Upstream commit 9b9bd3f640640f94272a461b2dfe558f91b322c5 ]
>
> [Why]
>Z10 and S0i3 have some shared path. Previous code clean up , incorrectly 
>removed these pointers, which breaks s0i3 restore
>
> [How]
>Do not clear the function pointers based on Z10 disable.
>
>Reviewed-by: Nicholas Kazlauskas 
>Acked-by: Pavle Kotarac 
>Signed-off-by: Eric Yang 
>Signed-off-by: Alex Deucher 
>Signed-off-by: Sasha Levin 
>---
> drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c | 5 -
> 1 file changed, 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c 
>b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
>index d7559e5a99ce..e708f07fe75a 100644
>--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
>+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
>@@ -153,9 +153,4 @@ void dcn31_hw_sequencer_construct(struct dc *dc)
>   dc->hwss.init_hw = dcn20_fpga_init_hw;
>   dc->hwseq->funcs.init_pipes = NULL;
>   }
>-  if (dc->debug.disable_z10) {
>-  /*hw not support z10 or sw disable it*/
>-  dc->hwss.z10_restore = NULL;
>-  dc->hwss.z10_save_init = NULL;
>-  }
> }
>--
>2.35.1
>

[PATCH AUTOSEL 5.15 07/17] fbdev: Prevent possible use-after-free in fb_release()

From: Daniel Vetter 

[ Upstream commit 89bfd4017e58faaf70411555e7f508495114e90b ]

Most fbdev drivers have issues with the fb_info lifetime, because call to
framebuffer_release() from their driver's .remove callback, rather than
doing from fbops.fb_destroy callback.

Doing that will destroy the fb_info too early, while references to it may
still exist, leading to a use-after-free error.

To prevent this, check the fb_info reference counter when attempting to
kfree the data structure in framebuffer_release(). That will leak it but
at least will prevent the mentioned error.

Signed-off-by: Daniel Vetter 
Signed-off-by: Javier Martinez Canillas 
Reviewed-by: Thomas Zimmermann 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20220505220413.365977-1-javi...@redhat.com
Signed-off-by: Sasha Levin 
---
 drivers/video/fbdev/core/fbsysfs.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/video/fbdev/core/fbsysfs.c 
b/drivers/video/fbdev/core/fbsysfs.c
index 65dae05fff8e..ce699396d6ba 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -80,6 +80,10 @@ void framebuffer_release(struct fb_info *info)
 {
if (!info)
return;
+
+   if (WARN_ON(refcount_read(>count)))
+   return;
+
kfree(info->apertures);
kfree(info);
 }
-- 
2.35.1

[PATCH AUTOSEL 5.15 06/17] Revert "fbdev: Make fb_release() return -ENODEV if fbdev was unregistered"

From: Javier Martinez Canillas 

[ Upstream commit 135332f34ba2662bc1e32b5c612e06a8cc41a053 ]

This reverts commit aafa025c76dcc7d1a8c8f0bdefcbe4eb480b2f6a. That commit
attempted to fix a NULL pointer dereference, caused by the struct fb_info
associated with a framebuffer device to not longer be valid when the file
descriptor was closed.

The issue was exposed by commit 27599aacbaef ("fbdev: Hot-unplug firmware
fb devices on forced removal"), which added a new path that goes through
the struct device removal instead of directly unregistering the fb.

Most fbdev drivers have issues with the fb_info lifetime, because call to
framebuffer_release() from their driver's .remove callback, rather than
doing from fbops.fb_destroy callback. This meant that due to this switch,
the fb_info was now destroyed too early, while references still existed,
while before it was simply leaked.

The patch we're reverting here reinstated that leak, hence "fixed" the
regression. But the proper solution is to fix the drivers to not release
the fb_info too soon.

Suggested-by: Daniel Vetter 
Signed-off-by: Javier Martinez Canillas 
Reviewed-by: Daniel Vetter 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20220504115917.758787-1-javi...@redhat.com
Signed-off-by: Sasha Levin 
---
 drivers/video/fbdev/core/fbmem.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 8e38a7a5cf2f..0371ad233fdf 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1436,10 +1436,7 @@ fb_release(struct inode *inode, struct file *file)
 __acquires(>lock)
 __releases(>lock)
 {
-   struct fb_info * const info = file_fb_info(file);
-
-   if (!info)
-   return -ENODEV;
+   struct fb_info * const info = file->private_data;
 
lock_fb_info(info);
if (info->fbops->fb_release)
-- 
2.35.1

[PATCH AUTOSEL 5.17 13/23] drm/amd/display: undo clearing of z10 related function pointers

From: Eric Yang 

[ Upstream commit 9b9bd3f640640f94272a461b2dfe558f91b322c5 ]

[Why]
Z10 and S0i3 have some shared path. Previous code clean up ,
incorrectly removed these pointers, which breaks s0i3 restore

[How]
Do not clear the function pointers based on Z10 disable.

Reviewed-by: Nicholas Kazlauskas 
Acked-by: Pavle Kotarac 
Signed-off-by: Eric Yang 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c 
b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
index d7559e5a99ce..e708f07fe75a 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
@@ -153,9 +153,4 @@ void dcn31_hw_sequencer_construct(struct dc *dc)
dc->hwss.init_hw = dcn20_fpga_init_hw;
dc->hwseq->funcs.init_pipes = NULL;
}
-   if (dc->debug.disable_z10) {
-   /*hw not support z10 or sw disable it*/
-   dc->hwss.z10_restore = NULL;
-   dc->hwss.z10_save_init = NULL;
-   }
 }
-- 
2.35.1

[PATCH AUTOSEL 5.17 08/23] fbdev: Prevent possible use-after-free in fb_release()

From: Daniel Vetter 

[ Upstream commit 89bfd4017e58faaf70411555e7f508495114e90b ]

Most fbdev drivers have issues with the fb_info lifetime, because call to
framebuffer_release() from their driver's .remove callback, rather than
doing from fbops.fb_destroy callback.

Doing that will destroy the fb_info too early, while references to it may
still exist, leading to a use-after-free error.

To prevent this, check the fb_info reference counter when attempting to
kfree the data structure in framebuffer_release(). That will leak it but
at least will prevent the mentioned error.

Signed-off-by: Daniel Vetter 
Signed-off-by: Javier Martinez Canillas 
Reviewed-by: Thomas Zimmermann 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20220505220413.365977-1-javi...@redhat.com
Signed-off-by: Sasha Levin 
---
 drivers/video/fbdev/core/fbsysfs.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/video/fbdev/core/fbsysfs.c 
b/drivers/video/fbdev/core/fbsysfs.c
index 26892940c213..82e31a2d845e 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -80,6 +80,10 @@ void framebuffer_release(struct fb_info *info)
 {
if (!info)
return;
+
+   if (WARN_ON(refcount_read(>count)))
+   return;
+
kfree(info->apertures);
kfree(info);
 }
-- 
2.35.1

[PATCH AUTOSEL 5.17 07/23] Revert "fbdev: Make fb_release() return -ENODEV if fbdev was unregistered"

From: Javier Martinez Canillas 

[ Upstream commit 135332f34ba2662bc1e32b5c612e06a8cc41a053 ]

This reverts commit aafa025c76dcc7d1a8c8f0bdefcbe4eb480b2f6a. That commit
attempted to fix a NULL pointer dereference, caused by the struct fb_info
associated with a framebuffer device to not longer be valid when the file
descriptor was closed.

The issue was exposed by commit 27599aacbaef ("fbdev: Hot-unplug firmware
fb devices on forced removal"), which added a new path that goes through
the struct device removal instead of directly unregistering the fb.

Most fbdev drivers have issues with the fb_info lifetime, because call to
framebuffer_release() from their driver's .remove callback, rather than
doing from fbops.fb_destroy callback. This meant that due to this switch,
the fb_info was now destroyed too early, while references still existed,
while before it was simply leaked.

The patch we're reverting here reinstated that leak, hence "fixed" the
regression. But the proper solution is to fix the drivers to not release
the fb_info too soon.

Suggested-by: Daniel Vetter 
Signed-off-by: Javier Martinez Canillas 
Reviewed-by: Daniel Vetter 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20220504115917.758787-1-javi...@redhat.com
Signed-off-by: Sasha Levin 
---
 drivers/video/fbdev/core/fbmem.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 10a9369c9dea..00f0f282e7a1 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1438,10 +1438,7 @@ fb_release(struct inode *inode, struct file *file)
 __acquires(>lock)
 __releases(>lock)
 {
-   struct fb_info * const info = file_fb_info(file);
-
-   if (!info)
-   return -ENODEV;
+   struct fb_info * const info = file->private_data;
 
lock_fb_info(info);
if (info->fbops->fb_release)
-- 
2.35.1

Re: [PATCH v2] dma-buf: Move sysfs work out of DMA-BUF export path

2022-05-18 Thread Greg Kroah-Hartman

On Tue, May 17, 2022 at 04:09:36PM -0700, T.J. Mercier wrote:
> On Mon, May 16, 2022 at 11:13 PM Greg Kroah-Hartman
>  wrote:
> >
> > On Mon, May 16, 2022 at 05:08:05PM -0700, T.J. Mercier wrote:
> > > On Mon, May 16, 2022 at 12:21 PM Christian König
> > >  wrote:
> > > >
> > > > Am 16.05.22 um 20:08 schrieb T.J. Mercier:
> > > > > On Mon, May 16, 2022 at 10:20 AM Christian König
> > > > >  wrote:
> > > > >> Am 16.05.22 um 19:13 schrieb T.J. Mercier:
> > > > >>> Recently, we noticed an issue where a process went into direct 
> > > > >>> reclaim
> > > > >>> while holding the kernfs rw semaphore for sysfs in write (exclusive)
> > > > >>> mode. This caused processes who were doing DMA-BUF exports and 
> > > > >>> releases
> > > > >>> to go into uninterruptible sleep since they needed to acquire the 
> > > > >>> same
> > > > >>> semaphore for the DMA-BUF sysfs entry creation/deletion. In order 
> > > > >>> to avoid
> > > > >>> blocking DMA-BUF export for an indeterminate amount of time while
> > > > >>> another process is holding the sysfs rw semaphore in exclusive mode,
> > > > >>> this patch moves the per-buffer sysfs file creation to the default 
> > > > >>> work
> > > > >>> queue. Note that this can lead to a short-term inaccuracy in the 
> > > > >>> dmabuf
> > > > >>> sysfs statistics, but this is a tradeoff to prevent the hot path 
> > > > >>> from
> > > > >>> being blocked. A work_struct is added to dma_buf to achieve this, 
> > > > >>> but as
> > > > >>> it is unioned with the kobject in the sysfs_entry, dma_buf does not
> > > > >>> increase in size.
> > > > >> I'm still not very keen of this approach as it strongly feels like we
> > > > >> are working around shortcoming somewhere else.
> > > > >>
> > > > > My read of the thread for the last version is that we're running into
> > > > > a situation where sysfs is getting used for something it wasn't
> > > > > originally intended for, but we're also stuck with this sysfs
> > > > > functionality for dmabufs.
> > > > >
> > > > >>> Fixes: bdb8d06dfefd ("dmabuf: Add the capability to expose DMA-BUF 
> > > > >>> stats in sysfs")
> > > > >>> Originally-by: Hridya Valsaraju 
> > > > >>> Signed-off-by: T.J. Mercier 
> > > > >>>
> > > > >>> ---
> > > > >>> See the originally submitted patch by Hridya Valsaraju here:
> > > > >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2022%2F1%2F4%2F1066data=05%7C01%7Cchristian.koenig%40amd.com%7C794614324d114880a25508da37672e4b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637883213566903705%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=bGlA2FeubfSeL5XDHYyWMZqUXfScoCphZjjK4jrqQJs%3Dreserved=0
> > > > >>>
> > > > >>> v2 changes:
> > > > >>> - Defer only sysfs creation instead of creation and teardown per
> > > > >>> Christian König
> > > > >>>
> > > > >>> - Use a work queue instead of a kthread for deferred work per
> > > > >>> Christian König
> > > > >>> ---
> > > > >>>drivers/dma-buf/dma-buf-sysfs-stats.c | 56 
> > > > >>> ---
> > > > >>>include/linux/dma-buf.h   | 14 ++-
> > > > >>>2 files changed, 54 insertions(+), 16 deletions(-)
> > > > >>>
> > > > >>> diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c 
> > > > >>> b/drivers/dma-buf/dma-buf-sysfs-stats.c
> > > > >>> index 2bba0babcb62..67b0a298291c 100644
> > > > >>> --- a/drivers/dma-buf/dma-buf-sysfs-stats.c
> > > > >>> +++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
> > > > >>> @@ -11,6 +11,7 @@
> > > > >>>#include 
> > > > >>>#include 
> > > > >>>#include 
> > > > >>> +#include 
> > > > >>>
> > > > >>>#include "dma-buf-sysfs-stats.h"
> > > > >>>
> > > > >>> @@ -168,10 +169,46 @@ void dma_buf_uninit_sysfs_statistics(void)
> > > > >>>kset_unregister(dma_buf_stats_kset);
> > > > >>>}
> > > > >>>
> > > > >>> +static void sysfs_add_workfn(struct work_struct *work)
> > > > >>> +{
> > > > >>> + struct dma_buf_sysfs_entry *sysfs_entry =
> > > > >>> + container_of(work, struct dma_buf_sysfs_entry, 
> > > > >>> sysfs_add_work);
> > > > >>> + struct dma_buf *dmabuf = sysfs_entry->dmabuf;
> > > > >>> +
> > > > >>> + /*
> > > > >>> +  * A dmabuf is ref-counted via its file member. If this 
> > > > >>> handler holds the only
> > > > >>> +  * reference to the dmabuf, there is no need for sysfs 
> > > > >>> kobject creation. This is an
> > > > >>> +  * optimization and a race; when the reference count drops to 
> > > > >>> 1 immediately after
> > > > >>> +  * this check it is not harmful as the sysfs entry will still 
> > > > >>> get cleaned up in
> > > > >>> +  * dma_buf_stats_teardown, which won't get called until the 
> > > > >>> final dmabuf reference
> > > > >>> +  * is released, and that can't happen until the end of this 
> > > > >>> function.
> > > > >>> +  */
> > > > >>> + if (file_count(dmabuf->file) > 1) {
> > > > >> Please completely

Re: [PATCH v2] dma-buf: Move sysfs work out of DMA-BUF export path

Am 18.05.22 um 01:09 schrieb T.J. Mercier:

[SNIP]

Perhaps we should go just one step further and make a misc device node
for dmabug debugging information to be in and just have userspace
poll/read on the device node and we spit the info that used to be in
debugfs out through that? That way this only affects systems when they
want to read the information and not normal code paths? Yeah that's a
hack, but this whole thing feels overly complex now.

Yeah, totally agree on the complexity note. I'm just absolutely not keen
to add hack over hack over hack to make something work which from my
point of view has some serious issues with it's design.

Why is this patch a hack? We found a problem with the initial design
which nobody saw when it was originally created, and now we're trying
to address it within the constraints that exist.

Well the issue is that you don't try to tackle the underlying problem,
but rather just mitigate the unforeseen consequences. That is pretty
clearly a hack to me.

Is there some other
solution to the problem of exports getting blocked that you would
suggest here?

Well pretty much the same as Greg outlined as well. Go back to your
drawing board and come back with a solution which does not need such
workarounds.

Alternatively you can give me a full overview of the design and explain
why exactly that interface here is necessary in exactly that form.

For example trying to do accounting based on DMA-bufs is extremely
questionable to begin with. See a modern game for example can have
between 10k and 100k of different buffers, reserving one file descriptor
for each of those objects is absolutely not going to work.

So my request is to please describe your full use case and not just why
you think this patch is justified.

The use case was described in the commit message when the feature was
initially added (after discussion about it on the list) including
links to code that uses the feature:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20210603214758.2955251-1-hridya%40google.com%2Fdata=05%7C01%7Cchristian.koenig%40amd.com%7C3f6e3e98fc6f45ead80d08da385a59e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637884257979664387%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=i%2BSfpB%2B6iBolBHu6KrKH3njq0zo1SBbrKDHZOjpsIks%3Dreserved=0

Yeah and to be honest I have the strong feeling now that this was
absolutely not well thought through. This description as well as the in
kernel documentation are not even remotely sufficient to explain what
you guys are doing with this.

So please come up with a complete design description for both userspace
and kernel why this interface here is necessary inside the upstream kernel.

If you can't convince me that this is a good idea I will just bluntly
mark this DMA-buf sysfs interface as deprecated.

Regards,
Christian.

thanks,

greg k-h

[PATCH v2 3/3] drm/panel: simple: add bus-format support for panel-dpi

From: Max Krummenacher 

Evaluate the device tree bus-format property to set bus_format for
a 'panel-dpi' panel. Additionally infer the bpc value from the
given bus-format.

Valid values for bus-format are found in:


This completes the addition of panel-dpi to completely specify
a panel-simple panel from the device tree.

Signed-off-by: Max Krummenacher 

---

Changes in v2:
None

 drivers/gpu/drm/panel/panel-simple.c | 43 
 1 file changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index a34f4198a534..090c60abb014 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -21,6 +21,7 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -453,6 +454,7 @@ static int panel_dpi_probe(struct device *dev,
struct panel_desc *desc;
unsigned int bus_flags;
struct videomode vm;
+   u32 bus_format;
int ret;
 
np = dev->of_node;
@@ -477,6 +479,47 @@ static int panel_dpi_probe(struct device *dev,
of_property_read_u32(np, "width-mm", >size.width);
of_property_read_u32(np, "height-mm", >size.height);
 
+   if (!of_property_read_u32(np, "bus-format", _format)) {
+   /* infer bpc from bus-format */
+   switch (bus_format) {
+   case DT_MEDIA_BUS_FMT_RGB565_1X16:
+   desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16;
+   desc->bpc = 6;
+   break;
+   case DT_MEDIA_BUS_FMT_RGB666_1X18:
+   desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18;
+   desc->bpc = 6;
+   break;
+   case DT_MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
+   desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI;
+   desc->bpc = 6;
+   break;
+   case DT_MEDIA_BUS_FMT_BGR888_1X24:
+   desc->bus_format = MEDIA_BUS_FMT_BGR888_1X24;
+   desc->bpc = 8;
+   break;
+   case DT_MEDIA_BUS_FMT_GBR888_1X24:
+   desc->bus_format = MEDIA_BUS_FMT_GBR888_1X24;
+   desc->bpc = 8;
+   break;
+   case DT_MEDIA_BUS_FMT_RBG888_1X24:
+   desc->bus_format = MEDIA_BUS_FMT_RBG888_1X24;
+   desc->bpc = 8;
+   break;
+   case DT_MEDIA_BUS_FMT_RGB888_1X24:
+   desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+   desc->bpc = 8;
+   break;
+   case DT_MEDIA_BUS_FMT_RGB888_1X32_PADHI:
+   desc->bus_format = MEDIA_BUS_FMT_RGB888_1X32_PADHI;
+   desc->bpc = 8;
+   break;
+   default:
+   dev_err(dev, "%pOF: unknown bus-format property\n", np);
+   return -EINVAL;
+   }
+   }
+
/* Extract bus_flags from display_timing */
bus_flags = 0;
vm.flags = timing->flags;
-- 
2.20.1

[PATCH v2 2/3] dt-bindings: display: startek, startek-kd050c: allow bus-format property

From: Max Krummenacher 

Allow to specify the optional bus-format property newly added to
panel-dpi.

Signed-off-by: Max Krummenacher 

---

Changes in v2:
- New commit

 .../bindings/display/panel/startek,startek-kd050c.yaml   | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/Documentation/devicetree/bindings/display/panel/startek,startek-kd050c.yaml 
b/Documentation/devicetree/bindings/display/panel/startek,startek-kd050c.yaml
index fd668640afd1..05306713044e 100644
--- 
a/Documentation/devicetree/bindings/display/panel/startek,startek-kd050c.yaml
+++ 
b/Documentation/devicetree/bindings/display/panel/startek,startek-kd050c.yaml
@@ -19,6 +19,7 @@ properties:
   - {} # panel-dpi, but not listed here to avoid false select
 
   backlight: true
+  bus-format: true
   enable-gpios: true
   height-mm: true
   label: true
-- 
2.20.1

[PATCH v2 1/3] dt-bindings: display: add new bus-format property for panel-dpi

From: Max Krummenacher 

The property is used to set the enum bus_format and infer the bpc
for a panel defined by 'panel-dpi'.
This specifies how the panel is connected to the display interface.

Signed-off-by: Max Krummenacher 

---

Changes in v2:
- Fix errors found by dt_binding_check

 .../bindings/display/panel/panel-dpi.yaml | 11 +
 .../dt-bindings/display/dt-media-bus-format.h | 23 +++
 2 files changed, 34 insertions(+)
 create mode 100644 include/dt-bindings/display/dt-media-bus-format.h

diff --git a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml 
b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml
index dae0676b5c6e..a20b5898941e 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml
@@ -21,6 +21,14 @@ properties:
   - {}
   - const: panel-dpi
 
+  bus-format:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: |
+  Describes how the display panel is connected to the display interface.
+  Valid values are defined in .
+  The mapping between the color/significance of the panel lines to the
+  parallel data lines are defined in [1].
+  [1] 
https://www.kernel.org/doc/html/v5.17/userspace-api/media/v4l/subdev-formats.html#packed-rgb-formats
   backlight: true
   enable-gpios: true
   height-mm: true
@@ -39,11 +47,14 @@ additionalProperties: false
 
 examples:
   - |
+#include 
+
 panel {
 compatible = "startek,startek-kd050c", "panel-dpi";
 label = "osddisplay";
 power-supply = <_supply>;
 backlight = <>;
+bus-format = ;
 
 port {
 lcd_in: endpoint {
diff --git a/include/dt-bindings/display/dt-media-bus-format.h 
b/include/dt-bindings/display/dt-media-bus-format.h
new file mode 100644
index ..c0f2a7b59aa1
--- /dev/null
+++ b/include/dt-bindings/display/dt-media-bus-format.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
+/*
+ * Copyright 2022 Max Krummenacher 
+ */
+
+#ifndef __DT_BINDINGS_DT_MEDIA_BUS_FORMAT_H
+#define __DT_BINDINGS_DT_MEDIA_BUS_FORMAT_H
+
+/*
+ * Attention: Keep these macro names in sync with
+ * include/uapi/linux/media-bus-format.h
+ */
+
+#define DT_MEDIA_BUS_FMT_RGB565_1X16   1
+#define DT_MEDIA_BUS_FMT_RGB666_1X18   2
+#define DT_MEDIA_BUS_FMT_RBG888_1X24   3
+#define DT_MEDIA_BUS_FMT_RGB666_1X24_CPADHI4
+#define DT_MEDIA_BUS_FMT_BGR888_1X24   5
+#define DT_MEDIA_BUS_FMT_GBR888_1X24   6
+#define DT_MEDIA_BUS_FMT_RGB888_1X24   7
+#define DT_MEDIA_BUS_FMT_RGB888_1X32_PADHI 8
+
+#endif /* __DT_BINDINGS_DT_MEDIA_BUS_FORMAT_H */
-- 
2.20.1

[PATCH v2 0/3] drm/panel: simple: add bus-format support for panel-dpi

From: Max Krummenacher 


Commit 4a1d0dbc8332 ("drm/panel: simple: add panel-dpi support") added
support for defining a panel from device tree provided data.

However support for setting the bus format is missing, so that with
the current implementation a 'panel-dpi' panel can only be used
if the driver of the display interface connected can cope with a
missing bus_format.

This patch series defines the new property bus-format and adds it to
the panel-dpi implementation.

Check initial discussions [1] and [2].
[1] 
https://lore.kernel.org/all/20220201110717.3585-1-cniederma...@dh-electronics.com/
[2] 
https://lore.kernel.org/all/20220222084723.14310-1-max.krummenac...@toradex.com/


Changes in v2:
- Fix errors found by dt_binding_check

Max Krummenacher (3):
  dt-bindings: display: add new bus-format property for panel-dpi
  dt-bindings: display: startek,startek-kd050c: allow bus-format
property
  drm/panel: simple: add bus-format support for panel-dpi

 .../bindings/display/panel/panel-dpi.yaml | 11 +
 .../display/panel/startek,startek-kd050c.yaml |  1 +
 drivers/gpu/drm/panel/panel-simple.c  | 43 +++
 .../dt-bindings/display/dt-media-bus-format.h | 23 ++
 4 files changed, 78 insertions(+)
 create mode 100644 include/dt-bindings/display/dt-media-bus-format.h

-- 
2.20.1

Re: [RFC PATCH v3 02/19] KVM: x86: inhibit APICv/AVIC when the guest and/or host changes apic id/base from the defaults.

2022-05-18 Thread Chao Gao

On Wed, May 18, 2022 at 12:50:27PM +0300, Maxim Levitsky wrote:
>> > struct kvm_arch {
>> > @@ -1258,6 +1260,7 @@ struct kvm_arch {
>> >hpa_t   hv_root_tdp;
>> >spinlock_t hv_root_tdp_lock;
>> > #endif
>> > +  bool apic_id_changed;
>> 
>> What's the value of this boolean? No one reads it.
>
>I use it in later patches to kill the guest during nested VM entry 
>if it attempts to use nested AVIC after any vCPU changed APIC ID.
>
>I mentioned this boolean in the commit description.
>
>This boolean avoids the need to go over all vCPUs and checking
>if they still have the initial apic id.

Do you want to kill the guest if APIC base got changed? If yes,
you can check if APICV_INHIBIT_REASON_RO_SETTINGS is set and save
the boolean.

>
>In the future maybe we can introduce a more generic 'taint'
>bitmap with various flags like that, indicating that the guest
>did something unexpected.
>
>BTW, the other option in regard to the nested AVIC is just to ignore this 
>issue completely.
>The code itself always uses vcpu_id's, thus regardless of when/how often the 
>guest changes
>its apic ids, my code would just use the initial APIC ID values consistently.
>
>In this case I won't need this boolean.
>
>> 
>> > };
>> > 
>> > struct kvm_vm_stat {
>> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> > index 66b0eb0bda94e..8996675b3ef4c 100644
>> > --- a/arch/x86/kvm/lapic.c
>> > +++ b/arch/x86/kvm/lapic.c
>> > @@ -2038,6 +2038,19 @@ static void apic_manage_nmi_watchdog(struct 
>> > kvm_lapic *apic, u32 lvt0_val)
>> >}
>> > }
>> > 
>> > +static void kvm_lapic_check_initial_apic_id(struct kvm_lapic *apic)
>> > +{
>> > +  if (kvm_apic_has_initial_apic_id(apic))
>> > +  return;
>> > +
>> > +  pr_warn_once("APIC ID change is unsupported by KVM");
>> 
>> It is misleading because changing xAPIC ID is supported by KVM; it just
>> isn't compatible with APICv. Probably this pr_warn_once() should be
>> removed.
>
>Honestly since nobody uses this feature, I am not sure if to call this 
>supported,
>I am sure that KVM has more bugs in regard of using non standard APIC ID.
>This warning might hopefuly make someone complain about it if this
>feature is actually used somewhere.

Now I got you. It is fine to me.

Re: [PATCH -next] drm/i915: fix compilation errors caused by `-fsanitize=shift`

On Tue, 17 May 2022, "GONG, Ruiqi"  wrote:
> Fix the compilation errors produced by building recent mainline on x86
> with allmodconfig:
>
> (1st type of errors)
>   drivers/gpu/drm/i915/display/intel_ddi.c:1916:2: error: case label does not 
> reduce to an integer constant
>   case PORT_CLK_SEL_WRPLL1:
>   ^~~~
>
> (2nd type of errors)
>   ././include/linux/compiler_types.h:352:38: error: call to 
> ‘__compiletime_assert_1360’ declared with attribute error: FIELD_PREP: mask 
> is not constant
> _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> ^
>   ...
>   drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:2316:3: note: in 
> expansion of macro ‘FIELD_PREP’
>  FIELD_PREP(GUC_KLV_0_KEY, GUC_CONTEXT_POLICIES_KLV_ID_##id) | \
>  ^~
>   drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:2323:1: note: in 
> expansion of macro ‘MAKE_CONTEXT_POLICY_ADD’
>MAKE_CONTEXT_POLICY_ADD(preemption_timeout, PREEMPTION_TIMEOUT)
>^~~
>
> which are all induced by `-fsanitize=shift`.
>
> Signed-off-by: GONG, Ruiqi 

Please see [1] and [2].

BR,
Jani.


[1] https://lore.kernel.org/r/20220405151517.29753-12...@alien8.de
[2] https://patchwork.freedesktop.org/series/104122/


> ---
>  drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h |  2 +-
>  .../i915/gt/uc/abi/guc_communication_ctb_abi.h   |  2 +-
>  drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h|  4 ++--
>  .../gpu/drm/i915/gt/uc/abi/guc_messages_abi.h|  2 +-
>  drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h   |  2 +-
>  drivers/gpu/drm/i915/i915_reg.h  | 16 
>  6 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h 
> b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> index be9ac47fa9d0..3ada7358a698 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> @@ -50,7 +50,7 @@
>  
>  #define HOST2GUC_SELF_CFG_REQUEST_MSG_LEN
> (GUC_HXG_REQUEST_MSG_MIN_LEN + 3u)
>  #define HOST2GUC_SELF_CFG_REQUEST_MSG_0_MBZ  
> GUC_HXG_REQUEST_MSG_0_DATA0
> -#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY  (0x << 16)
> +#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY  (0xu << 16)
>  #define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_LEN  (0x << 0)
>  #define HOST2GUC_SELF_CFG_REQUEST_MSG_2_VALUE32  
> GUC_HXG_REQUEST_MSG_n_DATAn
>  #define HOST2GUC_SELF_CFG_REQUEST_MSG_3_VALUE64  
> GUC_HXG_REQUEST_MSG_n_DATAn
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h 
> b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
> index c9086a600bce..c97ff7c38576 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
> @@ -82,7 +82,7 @@ static_assert(sizeof(struct guc_ct_buffer_desc) == 64);
>  #define GUC_CTB_HDR_LEN  1u
>  #define GUC_CTB_MSG_MIN_LEN  GUC_CTB_HDR_LEN
>  #define GUC_CTB_MSG_MAX_LEN  256u
> -#define GUC_CTB_MSG_0_FENCE  (0x << 16)
> +#define GUC_CTB_MSG_0_FENCE  (0xu << 16)
>  #define GUC_CTB_MSG_0_FORMAT (0xf << 12)
>  #define   GUC_CTB_FORMAT_HXG 0u
>  #define GUC_CTB_MSG_0_RESERVED   (0xf << 8)
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h 
> b/drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h
> index 4a59478c3b5c..e811896a80a0 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h
> @@ -29,8 +29,8 @@
>   */
>  
>  #define GUC_KLV_LEN_MIN  1u
> -#define GUC_KLV_0_KEY(0x << 16)
> -#define GUC_KLV_0_LEN(0x << 0)
> +#define GUC_KLV_0_KEY(0xu << 16)
> +#define GUC_KLV_0_LEN(0xu << 0)
>  #define GUC_KLV_n_VALUE  (0x << 0)
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h 
> b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
> index 29ac823acd4c..901595300f82 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
> @@ -40,7 +40,7 @@
>   */
>  
>  #define GUC_HXG_MSG_MIN_LEN  1u
> -#define GUC_HXG_MSG_0_ORIGIN (0x1 << 31)
> +#define GUC_HXG_MSG_0_ORIGIN (0x1u << 31)
>  #define   GUC_HXG_ORIGIN_HOST0u
>  #define   GUC_HXG_ORIGIN_GUC 1u
>  #define GUC_HXG_MSG_0_TYPE   (0x7 << 28)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h 
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> index

Re: [PATCH 11/11] drm/i915: Fix undefined behavior due to shift overflowing the constant

On Wed, 18 May 2022, Borislav Petkov  wrote:
> On Tue, May 17, 2022 at 04:05:46PM -0700, Randy Dunlap wrote:
>> 
>> 
>> On 4/5/22 08:15, Borislav Petkov wrote:
>> > From: Borislav Petkov 
>> > 
>> > Fix:
>> > 
>> >   In file included from :0:0:
>> >   drivers/gpu/drm/i915/gt/uc/intel_guc.c: In function 
>> > ‘intel_guc_send_mmio’:
>> >   ././include/linux/compiler_types.h:352:38: error: call to 
>> > ‘__compiletime_assert_1047’ \
>> >   declared with attribute error: FIELD_PREP: mask is not constant
>> > _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>> > 
>> > and other build errors due to shift overflowing values.
>> > 
>> > See https://lore.kernel.org/r/ykwq6%2btih8gqp...@zn.tnic for the gory
>> > details as to why it triggers with older gccs only.
>> > 
>> 
>> Acked-by: Randy Dunlap 
>> Tested-by: Randy Dunlap 
>> 
>> Is this merged anywhere?
>
> It's state is "new" in their patchwork:
>
> https://patchwork.freedesktop.org/patch/480756/

Basically we run all patches through CI before merging, and because only
one patch was sent to intel-gfx, the CI just sat waiting for the rest of
the series to arrive...

Anyway, didn't really like the changes in i915_reg.h, sent my version of
that and the rest separately [1].

> so I guess not yet.
>
>> It could/should at least be in linux-next so that other people
>> don't waste time on it.
>
> -ETOOMANYPATCHES I guess. :-\

Yeah, sorry about that.


BR,
Jani.

[1] https://patchwork.freedesktop.org/series/104122/


-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH v6, 6/7] media: mediatek: vcodec: prevent kernel crash when scp ipi timeout

2022-05-18 Thread Hans Verkuil




On 5/18/22 13:29, yunfei.d...@mediatek.com wrote:
> Dear Hans,
> 
> Thanks for your review.
> On Wed, 2022-05-18 at 11:37 +0200, Hans Verkuil wrote:
>> Hi Yunfei,
>>
>> On 5/13/22 11:25, Yunfei Dong wrote:
>>> When SCP timeout during playing video, kernel crashes with
>>> following
>>> message. It's caused by accessing NULL pointer in
>>> vpu_dec_ipi_handler.
>>> This patch doesn't solve the root cause of NULL pointer, but merely
>>> prevent kernel crashed when encounter the NULL pointer.
>>
>> Is the root cause being addressed as well? Where is the root cause?
>> Is it
>> in this driver or in the scp (i.e. the remoteproc) driver?
>>
>> I need a bit more information to decide whether this series is ready
>> to
>> be merged for 5.20 or not.
>>
>> Regards,
>>
>>  Hans
>>
> Vpu will be NUll when scp(micro processor) is hang or crash. Need to
> keep kernel works well , so add this patch.

OK, I think this should be stated in the commit log, and also in the code
(see below).

> 
> Best Regards,
> Yunfei Dong



>>> diff --git a/drivers/media/platform/mediatek/vcodec/vdec_vpu_if.c
>>> b/drivers/media/platform/mediatek/vcodec/vdec_vpu_if.c
>>> index 35f4d5583084..1041dd663e76 100644
>>> --- a/drivers/media/platform/mediatek/vcodec/vdec_vpu_if.c
>>> +++ b/drivers/media/platform/mediatek/vcodec/vdec_vpu_if.c
>>> @@ -91,6 +91,11 @@ static void vpu_dec_ipi_handler(void *data,
>>> unsigned int len, void *priv)
>>> struct vdec_vpu_inst *vpu = (struct vdec_vpu_inst *)
>>> (unsigned long)msg-
 ap_inst_addr;
>>>  
>>> +   if (!vpu) {
>>> +   mtk_v4l2_err("ap_inst_addr is NULL");

E.g., either add a comment here or perhaps change the error message to:

"ap_inst_addr is NULL, did the SCP hang?"

Or something along those lines.

Shouldn't there be a \n at the end of this message as well? Or does
mtk_v4l2_err add that?

Regards,

Hans

>>> +   return;
>>> +   }
>>> +
>>> mtk_vcodec_debug(vpu, "+ id=%X", msg->msg_id);
>>>  
>>> vpu->failure = msg->status;
>

Re: [PATCH 1/1] drm/panfrost: Add support for devcoredump

2022-05-18 Thread Steven Price

On 17/05/2022 18:42, Adrián Larumbe wrote:
> In the event of a job timeout, debug dump information will be written into
> /sys/class/devcoredump.
> 
> Inspired by etnaviv's similar feature.
> 
> Signed-off-by: Adrián Larumbe 

Nice! Some comments below.

> ---
>  drivers/gpu/drm/panfrost/Kconfig |   1 +
>  drivers/gpu/drm/panfrost/Makefile|   3 +-
>  drivers/gpu/drm/panfrost/panfrost_dump.c | 198 +++
>  drivers/gpu/drm/panfrost/panfrost_dump.h |  12 ++
>  drivers/gpu/drm/panfrost/panfrost_job.c  |   3 +
>  include/uapi/drm/panfrost_drm.h  |  32 
>  6 files changed, 248 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/panfrost/panfrost_dump.c
>  create mode 100644 drivers/gpu/drm/panfrost/panfrost_dump.h
> 
> diff --git a/drivers/gpu/drm/panfrost/Kconfig 
> b/drivers/gpu/drm/panfrost/Kconfig
> index 86cdc0ce79e6..079600328be1 100644
> --- a/drivers/gpu/drm/panfrost/Kconfig
> +++ b/drivers/gpu/drm/panfrost/Kconfig
> @@ -11,6 +11,7 @@ config DRM_PANFROST
>   select DRM_GEM_SHMEM_HELPER
>   select PM_DEVFREQ
>   select DEVFREQ_GOV_SIMPLE_ONDEMAND
> + select WANT_DEV_COREDUMP
>   help
> DRM driver for ARM Mali Midgard (T6xx, T7xx, T8xx) and
> Bifrost (G3x, G5x, G7x) GPUs.
> diff --git a/drivers/gpu/drm/panfrost/Makefile 
> b/drivers/gpu/drm/panfrost/Makefile
> index b71935862417..7da2b3f02ed9 100644
> --- a/drivers/gpu/drm/panfrost/Makefile
> +++ b/drivers/gpu/drm/panfrost/Makefile
> @@ -9,6 +9,7 @@ panfrost-y := \
>   panfrost_gpu.o \
>   panfrost_job.o \
>   panfrost_mmu.o \
> - panfrost_perfcnt.o
> + panfrost_perfcnt.o \
> + panfrost_dump.o
>  
>  obj-$(CONFIG_DRM_PANFROST) += panfrost.o
> diff --git a/drivers/gpu/drm/panfrost/panfrost_dump.c 
> b/drivers/gpu/drm/panfrost/panfrost_dump.c
> new file mode 100644
> index ..a76dcf4acf6f
> --- /dev/null
> +++ b/drivers/gpu/drm/panfrost/panfrost_dump.c
> @@ -0,0 +1,198 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright 2021 Collabora ltd. */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "panfrost_job.h"
> +#include "panfrost_gem.h"
> +#include "panfrost_regs.h"
> +#include "panfrost_dump.h"
> +#include "panfrost_device.h"
> +
> +static bool panfrost_dump_core = true;
> +module_param_named(dump_core, panfrost_dump_core, bool, 0600);
> +
> +struct panfrost_dump_iterator {
> + void *start;
> + struct panfrost_dump_object_header *hdr;
> + void *data;
> +};
> +
> +static const unsigned short panfrost_dump_registers[] = {
> + GPU_ID,
> + GPU_L2_FEATURES,
> + GPU_CORE_FEATURES,
> + GPU_TILER_FEATURES,
> + GPU_MEM_FEATURES,
> + GPU_MMU_FEATURES,
> + GPU_AS_PRESENT,
> + GPU_JS_PRESENT,
> + GPU_INT_RAWSTAT,
> + GPU_INT_CLEAR,
> + GPU_INT_MASK,
> + GPU_INT_STAT,
> +};

It would be nice to also dump the contents of JS_HEAD/JS_TAIL and the
exception status (i.e. what panfrost_job_handle_err() prints). Although
I'm not sure how easy it is to plumb that in at the moment. The handling
of job faults in the Panfrost driver isn't great. So maybe that can wait
for a v2 and we can rely on dmesg for now.

To be honest this list of registers is a little random, some (like
JS_PRESENT) are almost entirely useless, but then we're missing some
which userspace uses such as most of the GPU_THREAD_xxx registers,
although usually when debugging such things you are well aware of the
platform the dump comes from.

But I'm not an expert on debugging descriptors - my approach in the past
with the similar kbase feature has been to simply replay the specific
job on a software model of the GPU (hence the value of JS_HEAD/JS_TAIL).

You may find the list of registers that kbase dumps[1] to be an
interesting reference, the set is designed to spot kernel bugs (e.g.
core power states being different from expected) and allow the job which
failed to be replayed. In the blob this is combined with a debugfs file
which allows dumping the complete GPU memory[2] and userspace code to
trigger a dump after a job fault. These can then be combined to make a
'dump file' which our tools allow replaying on the model, hardware or RTL.

[1]
https://github.com/LibreELEC/mali-midgard/blob/TX011-SW-99002-r28p0-01rel0/driver/product/kernel/drivers/gpu/arm/midgard/backend/gpu/mali_kbase_debug_job_fault_backend.c

[2]
https://github.com/LibreELEC/mali-midgard/blob/TX011-SW-99002-r28p0-01rel0/driver/product/kernel/drivers/gpu/arm/midgard/mali_kbase_debug_mem_view.c

> +
> +static void panfrost_core_dump_header(struct panfrost_dump_iterator *iter,
> + u32 type, void *data_end)
> +{
> + struct panfrost_dump_object_header *hdr = iter->hdr;
> +
> + hdr->magic = cpu_to_le32(PANFROSTDUMP_MAGIC);
> + hdr->type = cpu_to_le32(type);
> + hdr->file_offset = cpu_to_le32(iter->data - iter->start);
> + hdr->file_size = cpu_to_le32(data_end -

Re: [PATCH 00/14] drm/kms: Stop registering multiple /sys/class/backlight devs for a single display

On Wed, 18 May 2022, Hans de Goede  wrote:
> Hi,
>
> On 5/18/22 10:44, Jani Nikula wrote:
>> On Tue, 17 May 2022, Hans de Goede  wrote:
>>> Hi All,
>>>
>>> As mentioned in my RFC titled "drm/kms: control display brightness through
>>> drm_connector properties":
>>> https://lore.kernel.org/dri-devel/0d188965-d809-81b5-74ce-7d30c49fe...@redhat.com/
>>>
>>> The first step towards this is to deal with some existing technical debt
>>> in backlight handling on x86/ACPI boards, specifically we need to stop
>>> registering multiple /sys/class/backlight devs for a single display.
>> 
>> I guess my question here is, how do you know it's for a *single*
>> display?
>> 
>> There are already designs out there with two flat panels, with
>> independent brightness controls. They're still rare and I don't think we
>> handle them very well. But we've got code to register multiple native
>> backlight interfaces, see e.g. commit 20f85ef89d94 ("drm/i915/backlight:
>> use unique backlight device names").
>> 
>> So imagine a design where one of the panels needs backlight control via
>> ACPI and the other via native backlight control. Granted, I don't know
>> if one exists, but I think it's very much in the realm of possible
>> things the OEMs might do. For example, have an EC PWM for primary panel
>> backlight, and use GPU PWM for secondary. How do you know you actually
>> do need to register two interfaces?
>
> On x86/ACPI devices this is all driven by acpi_video_get_backlight_type() /
> by the drivers/acpi/video_detect.c code. That code already will break on
> systems where there are 2 backlight controls, with one being ACPI based
> and the other a native GPU PWM backlight device.
>
> In this scenario as soon as the native GPU PWM backlight device shows up
> then, if acpi_video_get_backlight_type()==native, video_detect.c will
> currently unregister the acpi_video# backlight device(s) since userspace
> prefers the firmware type over the native type, so for userspace to
> actually honor the acpi_video_get_backlight_type()==native we need to get
> the acpi_video# backlight device "out of the way" which is currently
> handled by unregistering it.
>
> Note in a way we already have a case where userspace sees 2 panels,
> in hybrid laptop setups with a mux and on those systems we may see
> either 2 native backlight devices; or 2 native backlight devices +
> 2 acpi_video backlight devices with userspace preferring the ACPI
> ones.
>
> Also note that userspace already has code to detect if the related
> panel is active (iow which way the mux between the GPU and the panels
> points) and then uses that backlight device. Userspace here very
> much assumes a single panel though.
>
>> I'm fine with dealing with such cases as they arise to avoid
>> over-engineering up front, but I also don't want us to completely paint
>> ourselves in a corner either.
>
> Right. Note that the current code (both with and without this patchset)
> already will work fine from a kernel pov as long as both panels
> are either using native GPU PWM or are both using ACPI. But if we
> ever get a mix then this will need special handling.
>
> Note that all userspace code I know is currently hardcoded
> to assume a single panel. Userspace already picks one preferred
> device under /sys/class/backlight and ignores the rest. Actually
> atm userspace must behave this way, because on x86/ACPI boards we
> do often register multiple backlight devices for a single panel.
>
> So in a way moving to registering only a single backlight device
> prepares for actually having multiple panels work.
>
> Also keep in mind that this is preparation work for making the
> panel brightness a drm_connector property. When the panel uses
> a backlight device other then the native GPU PWM to control the
> brightness then the helper code for this needs to have a way to
> select which backlight_device to use then and the non native
> types are not "linked" to a specific connector so in this case
> we really need there to be only 1 backlight device registered
> so that the code looking up the (non native) backlight device
> for the connector gets the right one and not merely the one
> which happened to get registered first.
>
> And I believe that having the panel brightness be a drm_connector
> property is the way to make it possible for userspace to deal
> with the multiple panels which each have a separate brightness
> control case.

Agreed.

Thanks for the explanations and recording them here.

BR,
Jani.

>
> Regards,
>
> Hans
>
>
>
>
>
>> 
>> BR,
>> Jani.
>> 
>> 
>>>
>>> This series implements my RFC describing my plan for these cleanups:
>>> https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/
>>>
>>> Specifically patches 1-6 implement the "Fixing kms driver unconditionally
>>> register their "native" backlight dev" part.
>>>
>>> And patches 7-14 implement the "Fixing acpi_video0 getting registered for
>>> a brief time" time.
>>>
>>> Note this series does not

Re: [PATCH 01/14] ACPI: video: Add a native function parameter to acpi_video_get_backlight_type()

2022-05-18 Thread Hans de Goede

Hi,

On 5/18/22 10:55, Jani Nikula wrote:
> On Tue, 17 May 2022, Hans de Goede  wrote:
>> ATM on x86 laptops where we want userspace to use the acpi_video backlight
>> device we often register both the GPU's native backlight device and
>> acpi_video's firmware acpi_video# backlight device. This relies on
>> userspace preferring firmware type backlight devices over native ones, but
>> registering 2 backlight devices for a single display really is undesirable.
>>
>> On x86 laptops where the native GPU backlight device should be used,
>> the registering of other backlight devices is avoided by their drivers
>> using acpi_video_get_backlight_type() and only registering their backlight
>> if the return value matches their type.
>>
>> acpi_video_get_backlight_type() uses
>> backlight_device_get_by_type(BACKLIGHT_RAW) to determine if a native
>> driver is available and will never return native if this returns
>> false. This means that the GPU's native backlight registering code
>> cannot just call acpi_video_get_backlight_type() to determine if it
>> should register its backlight, since acpi_video_get_backlight_type() will
>> never return native until the native backlight has already registered.
>>
>> To fix this add a native function parameter to
>> acpi_video_get_backlight_type(), which when set to true will make
>> acpi_video_get_backlight_type() behave as if a native backlight has
>> already been registered.
>>
>> Note that all current callers are updated to pass false for the new
>> parameter, so this change in itself causes no functional changes.
> 
> 
>> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
>> index becc198e4c22..0a06f0edd298 100644
>> --- a/drivers/acpi/video_detect.c
>> +++ b/drivers/acpi/video_detect.c
>> @@ -17,12 +17,14 @@
>>   * Otherwise vendor specific drivers like thinkpad_acpi, asus-laptop,
>>   * sony_acpi,... can take care about backlight brightness.
>>   *
>> - * Backlight drivers can use acpi_video_get_backlight_type() to determine
>> - * which driver should handle the backlight.
>> + * Backlight drivers can use acpi_video_get_backlight_type() to determine 
>> which
>> + * driver should handle the backlight. RAW/GPU-driver backlight drivers must
>> + * pass true for the native function argument, other drivers must pass 
>> false.
>>   *
>>   * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module 
>> (m)
>>   * this file will not be compiled and acpi_video_get_backlight_type() will
>> - * always return acpi_backlight_vendor.
>> + * return acpi_backlight_native when its native argument is true and
>> + * acpi_backlight_vendor when it is false.
>>   */
> 
> Frankly, I think the boolean native parameter here, and at the call
> sites, is confusing, and the slightly different explanations in the
> commit message and comment here aren't helping.

Can you elaborate the "slightly different explanations in the
commit message and comment" part a bit (so that I can fix this) ?

> I suggest adding a separate function that the native backlight drivers
> should use. I think it's more obvious all around, and easier to document
> too.

Code wise I think this would mean renaming the original and
then adding 2 wrappers, but that is fine with me. I've no real
preference either way and I'm happy with adding a new variant of
acpi_video_get_backlight_type() for the native backlight drivers
any suggestion for a name ?

Regards,

Hans

Re: [PATCH 00/14] drm/kms: Stop registering multiple /sys/class/backlight devs for a single display

2022-05-18 Thread Hans de Goede

Hi,

On 5/18/22 10:44, Jani Nikula wrote:
> On Tue, 17 May 2022, Hans de Goede  wrote:
>> Hi All,
>>
>> As mentioned in my RFC titled "drm/kms: control display brightness through
>> drm_connector properties":
>> https://lore.kernel.org/dri-devel/0d188965-d809-81b5-74ce-7d30c49fe...@redhat.com/
>>
>> The first step towards this is to deal with some existing technical debt
>> in backlight handling on x86/ACPI boards, specifically we need to stop
>> registering multiple /sys/class/backlight devs for a single display.
> 
> I guess my question here is, how do you know it's for a *single*
> display?
> 
> There are already designs out there with two flat panels, with
> independent brightness controls. They're still rare and I don't think we
> handle them very well. But we've got code to register multiple native
> backlight interfaces, see e.g. commit 20f85ef89d94 ("drm/i915/backlight:
> use unique backlight device names").
> 
> So imagine a design where one of the panels needs backlight control via
> ACPI and the other via native backlight control. Granted, I don't know
> if one exists, but I think it's very much in the realm of possible
> things the OEMs might do. For example, have an EC PWM for primary panel
> backlight, and use GPU PWM for secondary. How do you know you actually
> do need to register two interfaces?

On x86/ACPI devices this is all driven by acpi_video_get_backlight_type() /
by the drivers/acpi/video_detect.c code. That code already will break on
systems where there are 2 backlight controls, with one being ACPI based
and the other a native GPU PWM backlight device.

In this scenario as soon as the native GPU PWM backlight device shows up
then, if acpi_video_get_backlight_type()==native, video_detect.c will
currently unregister the acpi_video# backlight device(s) since userspace
prefers the firmware type over the native type, so for userspace to
actually honor the acpi_video_get_backlight_type()==native we need to get
the acpi_video# backlight device "out of the way" which is currently
handled by unregistering it.

Note in a way we already have a case where userspace sees 2 panels,
in hybrid laptop setups with a mux and on those systems we may see
either 2 native backlight devices; or 2 native backlight devices +
2 acpi_video backlight devices with userspace preferring the ACPI
ones.

Also note that userspace already has code to detect if the related
panel is active (iow which way the mux between the GPU and the panels
points) and then uses that backlight device. Userspace here very
much assumes a single panel though.

> I'm fine with dealing with such cases as they arise to avoid
> over-engineering up front, but I also don't want us to completely paint
> ourselves in a corner either.

Right. Note that the current code (both with and without this patchset)
already will work fine from a kernel pov as long as both panels
are either using native GPU PWM or are both using ACPI. But if we
ever get a mix then this will need special handling.

Note that all userspace code I know is currently hardcoded
to assume a single panel. Userspace already picks one preferred
device under /sys/class/backlight and ignores the rest. Actually
atm userspace must behave this way, because on x86/ACPI boards we
do often register multiple backlight devices for a single panel.

So in a way moving to registering only a single backlight device
prepares for actually having multiple panels work.

Also keep in mind that this is preparation work for making the
panel brightness a drm_connector property. When the panel uses
a backlight device other then the native GPU PWM to control the
brightness then the helper code for this needs to have a way to
select which backlight_device to use then and the non native
types are not "linked" to a specific connector so in this case
we really need there to be only 1 backlight device registered
so that the code looking up the (non native) backlight device
for the connector gets the right one and not merely the one
which happened to get registered first.

And I believe that having the panel brightness be a drm_connector
property is the way to make it possible for userspace to deal
with the multiple panels which each have a separate brightness
control case.

Regards,

Hans

> 
> BR,
> Jani.
> 
> 
>>
>> This series implements my RFC describing my plan for these cleanups:
>> https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/
>>
>> Specifically patches 1-6 implement the "Fixing kms driver unconditionally
>> register their "native" backlight dev" part.
>>
>> And patches 7-14 implement the "Fixing acpi_video0 getting registered for
>> a brief time" time.
>>
>> Note this series does not deal yet with the "Other issues" part, I plan
>> to do a follow up series for that.
>>
>> The changes in this series are good to have regardless of the further
>> "drm/kms: control display brightness through drm_connector properties"
>> plans. So I plan to push these upstream once

Re: [RFC PATCH v3 02/19] KVM: x86: inhibit APICv/AVIC when the guest and/or host changes apic id/base from the defaults.