As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
This series attempts to do just that. While the original grep, AKA:
git grep 'if.*>prepared' -- drivers/gpu/drm/panel
git grep 'if.*>enabled' -- drivers/gpu/drm/panel
..still produces a few hits after my series, they are _mostly_ all
gone. The ones that are left are less trivial to fix.
One of the main reasons that many panels probably needed to store and
double-check their prepared/enabled appears to have been to handle
shutdown and/or remove. Panels drivers often wanted to force the power
off for panels in these cases and this was a good reason for the
double-check.
In response to my V1 series [1] we had much discussion of what to
do. The conclusion was that as long as DRM modeset drivers properly
called drm_atomic_helper_shutdown() that we should be able to remove
the explicit shutdown/remove handling in the panel drivers. Most of
the patches to improve DRM modeset drivers [2] [3] [4] have now
landed.
In contrast to my V1 series, I broke the V2 series up a lot
more. Since a few of the panel drivers in V1 already landed, we had
fewer total drivers and so we could devote a patch to each panel.
Also, since we were now relying on DRM modeset drivers I felt like we
should split the patches for each panel into two: one that's
definitely safe and one that could be reverted if we found a
problematic DRM modeset driver that we couldn't fix.
Sorry for the large number of patches. I've set things to mostly just
CC people on the cover letter and the patches that are relevant to
them. I've tried to CC people on the whole series that have shown
interest in this TODO item.
As patches in this series are reviewed and/or tested they could be
landed. There's really no ordering requirement for the series unless
patches touch the same driver.
NOTE: this touches _a lot_ of drivers, is repetitive, and is not
really possible to generate automatically. That means it's entirely
possible that my eyes glazed over and I did something wrong. Please
double-check me and don't assume that I got everything perfect, though
I did my best. I have at least confirmed that "allmodconfig" for arm64
doesn't fall on its face with this series. I haven't done a ton of
other testing.
[1] https://lore.kernel.org/r/20230804140605.RFC.4.I930069a32baab6faf46d6b234f89613b5cec0f14@changeid
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
[4] https://lore.kernel.org/r/[email protected]
Changes in v2:
- ("drm/panel: raydium-rm692e5: Stop tracking prepared") new for v2.
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
- panel-edp and panel-simple just get a comment now.
Douglas Anderson (48):
drm/panel: raydium-rm692e5: Stop tracking prepared
drm/panel: boe-himax8279d: Stop tracking prepared/enabled
drm/panel: boe-himax8279d: Don't call unprepare+disable at
shutdown/remove
drm/panel: boe-tv101wum-nl6: Stop tracking prepared
drm/panel: boe-tv101wum-nl6: Don't call unprepare+disable at
shutdown/remove
drm/panel: edp: Stop tracking prepared/enabled
drm/panel: edp: Add a comment about unprepare+disable at
shutdown/remove
drm/panel: innolux-p079zca: Stop tracking prepared/enabled
drm/panel: innolux-p079zca: Don't call unprepare+disable at
shutdown/remove
drm/panel: khadas-ts050: Stop tracking prepared/enabled
drm/panel: khadas-ts050: Don't call unprepare+disable at
shutdown/remove
drm/panel: kingdisplay-kd097d04: Stop tracking prepared/enabled
drm/panel: kingdisplay-kd097d04: Don't call unprepare+disable at
shutdown/remove
drm/panel: ltk050h3146w: Stop tracking prepared
drm/panel: ltk050h3146w: Don't call unprepare+disable at
shutdown/remove
drm/panel: ltk500hd1829: Stop tracking prepared
drm/panel: ltk500hd1829: Don't call unprepare+disable at
shutdown/remove
drm/panel: novatek-nt36672a: Stop tracking prepared
drm/panel: novatek-nt36672a: Don't call unprepare+disable at
shutdown/remove
drm/panel: olimex-lcd-olinuxino: Stop tracking prepared/enabled
drm/panel: olimex-lcd-olinuxino: Don't call unprepare+disable at
remove
drm/panel: osd-osd101t2587-53ts: Stop tracking prepared/enabled
drm/panel: osd-osd101t2587-53ts: Don't call unprepare+disable at
shutdown/remove
drm/panel: samsung-atna33xc20: Stop tracking prepared/enabled
drm/panel: samsung-atna33xc20: Don't call unprepare+disable at
shutdown/remove
drm/panel: simple: Stop tracking prepared/enabled
drm/panel: simple: Add a comment about unprepare+disable at
shutdown/remove
drm/panel: tdo-tl070wsh30: Stop tracking prepared
drm/panel: tdo-tl070wsh30: Don't call unprepare+disable at
shutdown/remove
drm/panel: xinpeng-xpp055c272: Stop tracking prepared
drm/panel: xinpeng-xpp055c272: Don't call unprepare+disable at
shutdown/remove
drm/panel: jdi-lt070me05000: Stop tracking prepared/enabled
drm/panel: jdi-lt070me05000: Don't call disable at shutdown/remove
drm/panel: panasonic-vvx10f034n00: Stop tracking prepared/enabled
drm/panel: panasonic-vvx10f034n00: Don't call disable at
shutdown/remove
drm/panel: seiko-43wvf1g: Stop tracking prepared/enabled
drm/panel: seiko-43wvf1g: Don't call disable at shutdown/remove
drm/panel: sharp-lq101r1sx01: Stop tracking prepared/enabled
drm/panel: sharp-lq101r1sx01: Don't call disable at shutdown/remove
drm/panel: sharp-ls043t1le01: Stop tracking prepared
drm/panel: sharp-ls043t1le01: Don't call disable at shutdown/remove
drm/panel: sitronix-st7703: Stop tracking prepared
drm/panel: sitronix-st7703: Don't call disable at shutdown/remove
drm/panel: raydium-rm67191: Stop tracking enabled
drm/panel: raydium-rm67191: Don't call unprepare+disable at shutdown
drm/panel: sony-acx565akm: Don't double-check enabled state in disable
drm/panel: sony-acx565akm: Don't call disable at remove
drm/panel: Update TODO list item for cleaning up prepared/enabled
tracking
Documentation/gpu/todo.rst | 47 +++++++-------
drivers/gpu/drm/panel/panel-boe-himax8279d.c | 40 ------------
.../gpu/drm/panel/panel-boe-tv101wum-nl6.c | 23 -------
drivers/gpu/drm/panel/panel-edp.c | 60 +++++++-----------
drivers/gpu/drm/panel/panel-innolux-p079zca.c | 55 ----------------
.../gpu/drm/panel/panel-jdi-lt070me05000.c | 35 -----------
drivers/gpu/drm/panel/panel-khadas-ts050.c | 39 ------------
.../drm/panel/panel-kingdisplay-kd097d04.c | 48 --------------
.../drm/panel/panel-leadtek-ltk050h3146w.c | 28 ---------
.../drm/panel/panel-leadtek-ltk500hd1829.c | 28 ---------
.../gpu/drm/panel/panel-novatek-nt36672a.c | 29 ---------
.../drm/panel/panel-olimex-lcd-olinuxino.c | 44 -------------
.../drm/panel/panel-osd-osd101t2587-53ts.c | 41 +-----------
.../drm/panel/panel-panasonic-vvx10f034n00.c | 47 +-------------
drivers/gpu/drm/panel/panel-raydium-rm67191.c | 26 --------
drivers/gpu/drm/panel/panel-raydium-rm692e5.c | 10 ---
.../gpu/drm/panel/panel-samsung-atna33xc20.c | 36 -----------
drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 49 ---------------
.../gpu/drm/panel/panel-sharp-lq101r1sx01.c | 63 +------------------
.../gpu/drm/panel/panel-sharp-ls043t1le01.c | 24 -------
drivers/gpu/drm/panel/panel-simple.c | 60 +++++++-----------
drivers/gpu/drm/panel/panel-sitronix-st7703.c | 35 +++--------
drivers/gpu/drm/panel/panel-sony-acx565akm.c | 6 --
drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c | 23 -------
.../gpu/drm/panel/panel-xinpeng-xpp055c272.c | 28 ---------
25 files changed, 83 insertions(+), 841 deletions(-)
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Luca Weiss <[email protected]>
Cc: Konrad Dybcio <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- New
drivers/gpu/drm/panel/panel-raydium-rm692e5.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c
index a613ba5b816c..21d97f6b8a2f 100644
--- a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c
+++ b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c
@@ -23,7 +23,6 @@ struct rm692e5_panel {
struct drm_dsc_config dsc;
struct regulator_bulk_data supplies[3];
struct gpio_desc *reset_gpio;
- bool prepared;
};
static inline struct rm692e5_panel *to_rm692e5_panel(struct drm_panel *panel)
@@ -171,9 +170,6 @@ static int rm692e5_prepare(struct drm_panel *panel)
struct device *dev = &ctx->dsi->dev;
int ret;
- if (ctx->prepared)
- return 0;
-
ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
if (ret < 0) {
dev_err(dev, "Failed to enable regulators: %d\n", ret);
@@ -213,8 +209,6 @@ static int rm692e5_prepare(struct drm_panel *panel)
mipi_dsi_generic_write_seq(ctx->dsi, 0xfe, 0x00);
- ctx->prepared = true;
-
return 0;
}
@@ -222,13 +216,9 @@ static int rm692e5_unprepare(struct drm_panel *panel)
{
struct rm692e5_panel *ctx = to_rm692e5_panel(panel);
- if (!ctx->prepared)
- return 0;
-
gpiod_set_value_cansleep(ctx->reset_gpio, 1);
regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
- ctx->prepared = false;
return 0;
}
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
NOTE: as part of this, transition the panel's direct calls to its
disable/unprepare functions in shutdown/remove to call through DRM
panel.
Cc: Jerry Han <[email protected]>
Cc: Jitao Shi <[email protected]>
Cc: Rock Wang <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-boe-himax8279d.c | 31 +++-----------------
1 file changed, 4 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-boe-himax8279d.c b/drivers/gpu/drm/panel/panel-boe-himax8279d.c
index e225840b0d67..12e14615298b 100644
--- a/drivers/gpu/drm/panel/panel-boe-himax8279d.c
+++ b/drivers/gpu/drm/panel/panel-boe-himax8279d.c
@@ -47,9 +47,6 @@ struct panel_info {
struct gpio_desc *enable_gpio;
struct gpio_desc *pp33_gpio;
struct gpio_desc *pp18_gpio;
-
- bool prepared;
- bool enabled;
};
static inline struct panel_info *to_panel_info(struct drm_panel *panel)
@@ -86,17 +83,12 @@ static int boe_panel_disable(struct drm_panel *panel)
struct panel_info *pinfo = to_panel_info(panel);
int err;
- if (!pinfo->enabled)
- return 0;
-
err = mipi_dsi_dcs_set_display_off(pinfo->link);
if (err < 0) {
dev_err(panel->dev, "failed to set display off: %d\n", err);
return err;
}
- pinfo->enabled = false;
-
return 0;
}
@@ -105,9 +97,6 @@ static int boe_panel_unprepare(struct drm_panel *panel)
struct panel_info *pinfo = to_panel_info(panel);
int err;
- if (!pinfo->prepared)
- return 0;
-
err = mipi_dsi_dcs_set_display_off(pinfo->link);
if (err < 0)
dev_err(panel->dev, "failed to set display off: %d\n", err);
@@ -121,8 +110,6 @@ static int boe_panel_unprepare(struct drm_panel *panel)
disable_gpios(pinfo);
- pinfo->prepared = false;
-
return 0;
}
@@ -131,9 +118,6 @@ static int boe_panel_prepare(struct drm_panel *panel)
struct panel_info *pinfo = to_panel_info(panel);
int err;
- if (pinfo->prepared)
- return 0;
-
gpiod_set_value(pinfo->pp18_gpio, 1);
/* T1: 5ms - 6ms */
usleep_range(5000, 6000);
@@ -180,8 +164,6 @@ static int boe_panel_prepare(struct drm_panel *panel)
/* T7: 20ms - 21ms */
usleep_range(20000, 21000);
- pinfo->prepared = true;
-
return 0;
poweroff:
@@ -194,9 +176,6 @@ static int boe_panel_enable(struct drm_panel *panel)
struct panel_info *pinfo = to_panel_info(panel);
int ret;
- if (pinfo->enabled)
- return 0;
-
usleep_range(120000, 121000);
ret = mipi_dsi_dcs_set_display_on(pinfo->link);
@@ -205,8 +184,6 @@ static int boe_panel_enable(struct drm_panel *panel)
return ret;
}
- pinfo->enabled = true;
-
return 0;
}
@@ -917,11 +894,11 @@ static void panel_remove(struct mipi_dsi_device *dsi)
struct panel_info *pinfo = mipi_dsi_get_drvdata(dsi);
int err;
- err = boe_panel_disable(&pinfo->base);
+ err = drm_panel_disable(&pinfo->base);
if (err < 0)
dev_err(&dsi->dev, "failed to disable panel: %d\n", err);
- err = boe_panel_unprepare(&pinfo->base);
+ err = drm_panel_unprepare(&pinfo->base);
if (err < 0)
dev_err(&dsi->dev, "failed to unprepare panel: %d\n", err);
@@ -936,8 +913,8 @@ static void panel_shutdown(struct mipi_dsi_device *dsi)
{
struct panel_info *pinfo = mipi_dsi_get_drvdata(dsi);
- boe_panel_disable(&pinfo->base);
- boe_panel_unprepare(&pinfo->base);
+ drm_panel_disable(&pinfo->base);
+ drm_panel_unprepare(&pinfo->base);
}
static struct mipi_dsi_driver panel_driver = {
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
Unfortunately, grepping mainline for this panel's compatible string
shows no hits, so we can't be 100% sure if the DRM modeset driver used
with this panel has been fixed. If it is found that the DRM modeset
driver hasn't been fixed then this patch could be temporarily reverted
until it is.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Jerry Han <[email protected]>
Cc: Jitao Shi <[email protected]>
Cc: Rock Wang <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-boe-himax8279d.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-boe-himax8279d.c b/drivers/gpu/drm/panel/panel-boe-himax8279d.c
index 12e14615298b..df746baae301 100644
--- a/drivers/gpu/drm/panel/panel-boe-himax8279d.c
+++ b/drivers/gpu/drm/panel/panel-boe-himax8279d.c
@@ -894,14 +894,6 @@ static void panel_remove(struct mipi_dsi_device *dsi)
struct panel_info *pinfo = mipi_dsi_get_drvdata(dsi);
int err;
- err = drm_panel_disable(&pinfo->base);
- if (err < 0)
- dev_err(&dsi->dev, "failed to disable panel: %d\n", err);
-
- err = drm_panel_unprepare(&pinfo->base);
- if (err < 0)
- dev_err(&dsi->dev, "failed to unprepare panel: %d\n", err);
-
err = mipi_dsi_detach(dsi);
if (err < 0)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", err);
@@ -909,14 +901,6 @@ static void panel_remove(struct mipi_dsi_device *dsi)
drm_panel_remove(&pinfo->base);
}
-static void panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct panel_info *pinfo = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_disable(&pinfo->base);
- drm_panel_unprepare(&pinfo->base);
-}
-
static struct mipi_dsi_driver panel_driver = {
.driver = {
.name = "panel-boe-himax8279d",
@@ -924,7 +908,6 @@ static struct mipi_dsi_driver panel_driver = {
},
.probe = panel_probe,
.remove = panel_remove,
- .shutdown = panel_shutdown,
};
module_mipi_dsi_driver(panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
Unfortunately, it's very difficult to know exactly which DRM modeset
drivers are using panel-edp due to the sheer number of panels it
handles. For now, we'll leave the calls and just add a comment to keep
people from copying this code.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
- panel-edp and panel-simple just get a comment now.
drivers/gpu/drm/panel/panel-edp.c | 33 +++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index 344f67871d41..9c4c5c0e39a3 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -944,13 +944,34 @@ static int panel_edp_probe(struct device *dev, const struct panel_desc *desc,
return err;
}
-static void panel_edp_remove(struct device *dev)
+static void panel_edp_shutdown(struct device *dev)
{
struct panel_edp *panel = dev_get_drvdata(dev);
- drm_panel_remove(&panel->base);
+ /*
+ * NOTE: the following two calls don't really belong here. It is the
+ * responsibility of a correctly written DRM modeset driver to call
+ * drm_atomic_helper_shutdown() at shutdown time and that should
+ * cause the panel to be disabled / unprepared if needed. For now,
+ * however, we'll keep these calls due to the sheer number of
+ * different DRM modeset drivers used with panel-edp. The fact that
+ * we're calling these and _also_ the drm_atomic_helper_shutdown()
+ * will try to disable/unprepare means that we can get a warning about
+ * trying to disable/unprepare an already disabled/unprepared panel,
+ * but that's something we'll have to live with until we've confirmed
+ * that all DRM modeset drivers are properly calling
+ * drm_atomic_helper_shutdown().
+ */
drm_panel_disable(&panel->base);
drm_panel_unprepare(&panel->base);
+}
+
+static void panel_edp_remove(struct device *dev)
+{
+ struct panel_edp *panel = dev_get_drvdata(dev);
+
+ drm_panel_remove(&panel->base);
+ panel_edp_shutdown(dev);
pm_runtime_dont_use_autosuspend(dev);
pm_runtime_disable(dev);
@@ -961,14 +982,6 @@ static void panel_edp_remove(struct device *dev)
panel->drm_edid = NULL;
}
-static void panel_edp_shutdown(struct device *dev)
-{
- struct panel_edp *panel = dev_get_drvdata(dev);
-
- drm_panel_disable(&panel->base);
- drm_panel_unprepare(&panel->base);
-}
-
static const struct display_timing auo_b101ean01_timing = {
.pixelclock = { 65300000, 72500000, 75000000 },
.hactive = { 1280, 1280, 1280 },
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-edp.c | 27 ---------------------------
1 file changed, 27 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index 6db277efcbb7..344f67871d41 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -222,11 +222,8 @@ struct edp_panel_entry {
struct panel_edp {
struct drm_panel base;
- bool enabled;
bool no_hpd;
- bool prepared;
-
ktime_t prepared_time;
ktime_t powered_on_time;
ktime_t unprepared_time;
@@ -395,14 +392,9 @@ static int panel_edp_disable(struct drm_panel *panel)
{
struct panel_edp *p = to_panel_edp(panel);
- if (!p->enabled)
- return 0;
-
if (p->desc->delay.disable)
msleep(p->desc->delay.disable);
- p->enabled = false;
-
return 0;
}
@@ -420,17 +412,11 @@ static int panel_edp_suspend(struct device *dev)
static int panel_edp_unprepare(struct drm_panel *panel)
{
- struct panel_edp *p = to_panel_edp(panel);
int ret;
- /* Unpreparing when already unprepared is a no-op */
- if (!p->prepared)
- return 0;
-
ret = pm_runtime_put_sync_suspend(panel->dev);
if (ret < 0)
return ret;
- p->prepared = false;
return 0;
}
@@ -542,21 +528,14 @@ static int panel_edp_resume(struct device *dev)
static int panel_edp_prepare(struct drm_panel *panel)
{
- struct panel_edp *p = to_panel_edp(panel);
int ret;
- /* Preparing when already prepared is a no-op */
- if (p->prepared)
- return 0;
-
ret = pm_runtime_get_sync(panel->dev);
if (ret < 0) {
pm_runtime_put_autosuspend(panel->dev);
return ret;
}
- p->prepared = true;
-
return 0;
}
@@ -565,9 +544,6 @@ static int panel_edp_enable(struct drm_panel *panel)
struct panel_edp *p = to_panel_edp(panel);
unsigned int delay;
- if (p->enabled)
- return 0;
-
delay = p->desc->delay.enable;
/*
@@ -598,8 +574,6 @@ static int panel_edp_enable(struct drm_panel *panel)
panel_edp_wait(p->powered_on_time, p->desc->delay.powered_on_to_enable);
- p->enabled = true;
-
return 0;
}
@@ -869,7 +843,6 @@ static int panel_edp_probe(struct device *dev, const struct panel_desc *desc,
if (!panel)
return -ENOMEM;
- panel->enabled = false;
panel->prepared_time = 0;
panel->desc = desc;
panel->aux = aux;
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Chris Zhong <[email protected]>
Cc: Lin Huang <[email protected]>
Cc: Brian Norris <[email protected]>
Cc: "Heiko Stübner" <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-innolux-p079zca.c | 39 -------------------
1 file changed, 39 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
index 485178a99910..5833d3a0fc79 100644
--- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
+++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
@@ -51,9 +51,6 @@ struct innolux_panel {
struct regulator_bulk_data *supplies;
struct gpio_desc *enable_gpio;
-
- bool prepared;
- bool enabled;
};
static inline struct innolux_panel *to_innolux_panel(struct drm_panel *panel)
@@ -61,26 +58,11 @@ static inline struct innolux_panel *to_innolux_panel(struct drm_panel *panel)
return container_of(panel, struct innolux_panel, base);
}
-static int innolux_panel_disable(struct drm_panel *panel)
-{
- struct innolux_panel *innolux = to_innolux_panel(panel);
-
- if (!innolux->enabled)
- return 0;
-
- innolux->enabled = false;
-
- return 0;
-}
-
static int innolux_panel_unprepare(struct drm_panel *panel)
{
struct innolux_panel *innolux = to_innolux_panel(panel);
int err;
- if (!innolux->prepared)
- return 0;
-
err = mipi_dsi_dcs_set_display_off(innolux->link);
if (err < 0)
dev_err(panel->dev, "failed to set display off: %d\n", err);
@@ -104,8 +86,6 @@ static int innolux_panel_unprepare(struct drm_panel *panel)
if (err < 0)
return err;
- innolux->prepared = false;
-
return 0;
}
@@ -114,9 +94,6 @@ static int innolux_panel_prepare(struct drm_panel *panel)
struct innolux_panel *innolux = to_innolux_panel(panel);
int err;
- if (innolux->prepared)
- return 0;
-
gpiod_set_value_cansleep(innolux->enable_gpio, 0);
err = regulator_bulk_enable(innolux->desc->num_supplies,
@@ -178,8 +155,6 @@ static int innolux_panel_prepare(struct drm_panel *panel)
/* T7: 5ms */
usleep_range(5000, 6000);
- innolux->prepared = true;
-
return 0;
poweroff:
@@ -189,18 +164,6 @@ static int innolux_panel_prepare(struct drm_panel *panel)
return err;
}
-static int innolux_panel_enable(struct drm_panel *panel)
-{
- struct innolux_panel *innolux = to_innolux_panel(panel);
-
- if (innolux->enabled)
- return 0;
-
- innolux->enabled = true;
-
- return 0;
-}
-
static const char * const innolux_p079zca_supply_names[] = {
"power",
};
@@ -407,10 +370,8 @@ static int innolux_panel_get_modes(struct drm_panel *panel,
}
static const struct drm_panel_funcs innolux_panel_funcs = {
- .disable = innolux_panel_disable,
.unprepare = innolux_panel_unprepare,
.prepare = innolux_panel_prepare,
- .enable = innolux_panel_enable,
.get_modes = innolux_panel_get_modes,
};
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by Rockchip boards. The Rockchip driver
appears to be correctly calling drm_atomic_helper_shutdown() so we can
remove the calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Chris Zhong <[email protected]>
Cc: Lin Huang <[email protected]>
Cc: Brian Norris <[email protected]>
Cc: "Heiko Stübner" <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-innolux-p079zca.c | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
index 5833d3a0fc79..280f0fc35b67 100644
--- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
+++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
@@ -471,13 +471,6 @@ static void innolux_panel_remove(struct mipi_dsi_device *dsi)
struct innolux_panel *innolux = mipi_dsi_get_drvdata(dsi);
int err;
- err = drm_panel_unprepare(&innolux->base);
- if (err < 0)
- dev_err(&dsi->dev, "failed to unprepare panel: %d\n", err);
-
- err = drm_panel_disable(&innolux->base);
- if (err < 0)
- dev_err(&dsi->dev, "failed to disable panel: %d\n", err);
err = mipi_dsi_detach(dsi);
if (err < 0)
@@ -486,14 +479,6 @@ static void innolux_panel_remove(struct mipi_dsi_device *dsi)
innolux_panel_del(innolux);
}
-static void innolux_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct innolux_panel *innolux = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_unprepare(&innolux->base);
- drm_panel_disable(&innolux->base);
-}
-
static struct mipi_dsi_driver innolux_panel_driver = {
.driver = {
.name = "panel-innolux-p079zca",
@@ -501,7 +486,6 @@ static struct mipi_dsi_driver innolux_panel_driver = {
},
.probe = innolux_panel_probe,
.remove = innolux_panel_remove,
- .shutdown = innolux_panel_shutdown,
};
module_mipi_dsi_driver(innolux_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Jacobe Zang <[email protected]>
Cc: Nicolas Belin <[email protected]>
Cc: Neil Armstrong <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-khadas-ts050.c | 28 ----------------------
1 file changed, 28 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-khadas-ts050.c b/drivers/gpu/drm/panel/panel-khadas-ts050.c
index c54be0cc3f08..e35762ebdbd1 100644
--- a/drivers/gpu/drm/panel/panel-khadas-ts050.c
+++ b/drivers/gpu/drm/panel/panel-khadas-ts050.c
@@ -26,9 +26,6 @@ struct khadas_ts050_panel {
struct gpio_desc *reset_gpio;
struct gpio_desc *enable_gpio;
struct khadas_ts050_panel_data *panel_data;
-
- bool prepared;
- bool enabled;
};
struct khadas_ts050_panel_cmd {
@@ -642,9 +639,6 @@ static int khadas_ts050_panel_prepare(struct drm_panel *panel)
unsigned int i;
int err;
- if (khadas_ts050->prepared)
- return 0;
-
gpiod_set_value_cansleep(khadas_ts050->enable_gpio, 0);
err = regulator_enable(khadas_ts050->supply);
@@ -708,8 +702,6 @@ static int khadas_ts050_panel_prepare(struct drm_panel *panel)
usleep_range(10000, 11000);
- khadas_ts050->prepared = true;
-
return 0;
poweroff:
@@ -726,11 +718,6 @@ static int khadas_ts050_panel_unprepare(struct drm_panel *panel)
struct khadas_ts050_panel *khadas_ts050 = to_khadas_ts050_panel(panel);
int err;
- if (!khadas_ts050->prepared)
- return 0;
-
- khadas_ts050->prepared = false;
-
err = mipi_dsi_dcs_enter_sleep_mode(khadas_ts050->link);
if (err < 0)
dev_err(panel->dev, "failed to enter sleep mode: %d\n", err);
@@ -747,31 +734,17 @@ static int khadas_ts050_panel_unprepare(struct drm_panel *panel)
return 0;
}
-static int khadas_ts050_panel_enable(struct drm_panel *panel)
-{
- struct khadas_ts050_panel *khadas_ts050 = to_khadas_ts050_panel(panel);
-
- khadas_ts050->enabled = true;
-
- return 0;
-}
-
static int khadas_ts050_panel_disable(struct drm_panel *panel)
{
struct khadas_ts050_panel *khadas_ts050 = to_khadas_ts050_panel(panel);
int err;
- if (!khadas_ts050->enabled)
- return 0;
-
err = mipi_dsi_dcs_set_display_off(khadas_ts050->link);
if (err < 0)
dev_err(panel->dev, "failed to set display off: %d\n", err);
usleep_range(10000, 11000);
- khadas_ts050->enabled = false;
-
return 0;
}
@@ -815,7 +788,6 @@ static int khadas_ts050_panel_get_modes(struct drm_panel *panel,
static const struct drm_panel_funcs khadas_ts050_panel_funcs = {
.prepare = khadas_ts050_panel_prepare,
.unprepare = khadas_ts050_panel_unprepare,
- .enable = khadas_ts050_panel_enable,
.disable = khadas_ts050_panel_disable,
.get_modes = khadas_ts050_panel_get_modes,
};
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Brian Norris <[email protected]>
Cc: Chris Zhong <[email protected]>
Cc: Nickey Yang <[email protected]>
Cc: "Heiko Stübner" <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
.../drm/panel/panel-kingdisplay-kd097d04.c | 31 -------------------
1 file changed, 31 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-kingdisplay-kd097d04.c b/drivers/gpu/drm/panel/panel-kingdisplay-kd097d04.c
index 17f8d80cf2b3..88d775e000d4 100644
--- a/drivers/gpu/drm/panel/panel-kingdisplay-kd097d04.c
+++ b/drivers/gpu/drm/panel/panel-kingdisplay-kd097d04.c
@@ -23,9 +23,6 @@ struct kingdisplay_panel {
struct regulator *supply;
struct gpio_desc *enable_gpio;
-
- bool prepared;
- bool enabled;
};
struct kingdisplay_panel_cmd {
@@ -185,15 +182,10 @@ static int kingdisplay_panel_disable(struct drm_panel *panel)
struct kingdisplay_panel *kingdisplay = to_kingdisplay_panel(panel);
int err;
- if (!kingdisplay->enabled)
- return 0;
-
err = mipi_dsi_dcs_set_display_off(kingdisplay->link);
if (err < 0)
dev_err(panel->dev, "failed to set display off: %d\n", err);
- kingdisplay->enabled = false;
-
return 0;
}
@@ -202,9 +194,6 @@ static int kingdisplay_panel_unprepare(struct drm_panel *panel)
struct kingdisplay_panel *kingdisplay = to_kingdisplay_panel(panel);
int err;
- if (!kingdisplay->prepared)
- return 0;
-
err = mipi_dsi_dcs_enter_sleep_mode(kingdisplay->link);
if (err < 0) {
dev_err(panel->dev, "failed to enter sleep mode: %d\n", err);
@@ -220,8 +209,6 @@ static int kingdisplay_panel_unprepare(struct drm_panel *panel)
if (err < 0)
return err;
- kingdisplay->prepared = false;
-
return 0;
}
@@ -231,9 +218,6 @@ static int kingdisplay_panel_prepare(struct drm_panel *panel)
int err, regulator_err;
unsigned int i;
- if (kingdisplay->prepared)
- return 0;
-
gpiod_set_value_cansleep(kingdisplay->enable_gpio, 0);
err = regulator_enable(kingdisplay->supply);
@@ -275,8 +259,6 @@ static int kingdisplay_panel_prepare(struct drm_panel *panel)
/* T7: 10ms */
usleep_range(10000, 11000);
- kingdisplay->prepared = true;
-
return 0;
poweroff:
@@ -289,18 +271,6 @@ static int kingdisplay_panel_prepare(struct drm_panel *panel)
return err;
}
-static int kingdisplay_panel_enable(struct drm_panel *panel)
-{
- struct kingdisplay_panel *kingdisplay = to_kingdisplay_panel(panel);
-
- if (kingdisplay->enabled)
- return 0;
-
- kingdisplay->enabled = true;
-
- return 0;
-}
-
static const struct drm_display_mode default_mode = {
.clock = 229000,
.hdisplay = 1536,
@@ -341,7 +311,6 @@ static const struct drm_panel_funcs kingdisplay_panel_funcs = {
.disable = kingdisplay_panel_disable,
.unprepare = kingdisplay_panel_unprepare,
.prepare = kingdisplay_panel_prepare,
- .enable = kingdisplay_panel_enable,
.get_modes = kingdisplay_panel_get_modes,
};
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by Rockchip boards. The Rockchip driver
appear to be correctly calling drm_atomic_helper_shutdown() so we can
remove the calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Brian Norris <[email protected]>
Cc: Chris Zhong <[email protected]>
Cc: Nickey Yang <[email protected]>
Cc: "Heiko Stübner" <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
.../gpu/drm/panel/panel-kingdisplay-kd097d04.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-kingdisplay-kd097d04.c b/drivers/gpu/drm/panel/panel-kingdisplay-kd097d04.c
index 88d775e000d4..d6b912277196 100644
--- a/drivers/gpu/drm/panel/panel-kingdisplay-kd097d04.c
+++ b/drivers/gpu/drm/panel/panel-kingdisplay-kd097d04.c
@@ -389,14 +389,6 @@ static void kingdisplay_panel_remove(struct mipi_dsi_device *dsi)
struct kingdisplay_panel *kingdisplay = mipi_dsi_get_drvdata(dsi);
int err;
- err = drm_panel_unprepare(&kingdisplay->base);
- if (err < 0)
- dev_err(&dsi->dev, "failed to unprepare panel: %d\n", err);
-
- err = drm_panel_disable(&kingdisplay->base);
- if (err < 0)
- dev_err(&dsi->dev, "failed to disable panel: %d\n", err);
-
err = mipi_dsi_detach(dsi);
if (err < 0)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", err);
@@ -404,14 +396,6 @@ static void kingdisplay_panel_remove(struct mipi_dsi_device *dsi)
kingdisplay_panel_del(kingdisplay);
}
-static void kingdisplay_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct kingdisplay_panel *kingdisplay = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_unprepare(&kingdisplay->base);
- drm_panel_disable(&kingdisplay->base);
-}
-
static struct mipi_dsi_driver kingdisplay_panel_driver = {
.driver = {
.name = "panel-kingdisplay-kd097d04",
@@ -419,7 +403,6 @@ static struct mipi_dsi_driver kingdisplay_panel_driver = {
},
.probe = kingdisplay_panel_probe,
.remove = kingdisplay_panel_remove,
- .shutdown = kingdisplay_panel_shutdown,
};
module_mipi_dsi_driver(kingdisplay_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
Unfortunately, grepping mainline for this panel's compatible string
shows no hits, so we can't be 100% sure if the DRM modeset driver used
with this panel has been fixed. If it is found that the DRM modeset
driver hasn't been fixed then this patch could be temporarily reverted
until it is.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Jacobe Zang <[email protected]>
Cc: Nicolas Belin <[email protected]>
Cc: Neil Armstrong <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-khadas-ts050.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-khadas-ts050.c b/drivers/gpu/drm/panel/panel-khadas-ts050.c
index e35762ebdbd1..14932cb3defc 100644
--- a/drivers/gpu/drm/panel/panel-khadas-ts050.c
+++ b/drivers/gpu/drm/panel/panel-khadas-ts050.c
@@ -880,16 +880,6 @@ static void khadas_ts050_panel_remove(struct mipi_dsi_device *dsi)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", err);
drm_panel_remove(&khadas_ts050->base);
- drm_panel_disable(&khadas_ts050->base);
- drm_panel_unprepare(&khadas_ts050->base);
-}
-
-static void khadas_ts050_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct khadas_ts050_panel *khadas_ts050 = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_disable(&khadas_ts050->base);
- drm_panel_unprepare(&khadas_ts050->base);
}
static struct mipi_dsi_driver khadas_ts050_panel_driver = {
@@ -899,7 +889,6 @@ static struct mipi_dsi_driver khadas_ts050_panel_driver = {
},
.probe = khadas_ts050_panel_probe,
.remove = khadas_ts050_panel_remove,
- .shutdown = khadas_ts050_panel_shutdown,
};
module_mipi_dsi_driver(khadas_ts050_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
Unfortunately, grepping mainline for this panel's compatible string
shows no hits, so we can't be 100% sure if the DRM modeset driver used
with this panel has been fixed. If it is found that the DRM modeset
driver hasn't been fixed then this patch could be temporarily reverted
until it is.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: "Heiko Stübner" <[email protected]>
Cc: Quentin Schulz <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
.../gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
index 5894bf30524a..292aa26a456d 100644
--- a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
+++ b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
@@ -673,27 +673,11 @@ static int ltk050h3146w_probe(struct mipi_dsi_device *dsi)
return 0;
}
-static void ltk050h3146w_shutdown(struct mipi_dsi_device *dsi)
-{
- struct ltk050h3146w *ctx = mipi_dsi_get_drvdata(dsi);
- int ret;
-
- ret = drm_panel_unprepare(&ctx->panel);
- if (ret < 0)
- dev_err(&dsi->dev, "Failed to unprepare panel: %d\n", ret);
-
- ret = drm_panel_disable(&ctx->panel);
- if (ret < 0)
- dev_err(&dsi->dev, "Failed to disable panel: %d\n", ret);
-}
-
static void ltk050h3146w_remove(struct mipi_dsi_device *dsi)
{
struct ltk050h3146w *ctx = mipi_dsi_get_drvdata(dsi);
int ret;
- ltk050h3146w_shutdown(dsi);
-
ret = mipi_dsi_detach(dsi);
if (ret < 0)
dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
@@ -725,7 +709,6 @@ static struct mipi_dsi_driver ltk050h3146w_driver = {
},
.probe = ltk050h3146w_probe,
.remove = ltk050h3146w_remove,
- .shutdown = ltk050h3146w_shutdown,
};
module_mipi_dsi_driver(ltk050h3146w_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: "Heiko Stübner" <[email protected]>
Cc: Quentin Schulz <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
index 1a26205701b5..5894bf30524a 100644
--- a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
+++ b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
@@ -36,7 +36,6 @@ struct ltk050h3146w {
struct regulator *vci;
struct regulator *iovcc;
const struct ltk050h3146w_desc *panel_desc;
- bool prepared;
};
static const struct ltk050h3146w_cmd page1_cmds[] = {
@@ -521,9 +520,6 @@ static int ltk050h3146w_unprepare(struct drm_panel *panel)
struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
int ret;
- if (!ctx->prepared)
- return 0;
-
ret = mipi_dsi_dcs_set_display_off(dsi);
if (ret < 0) {
dev_err(ctx->dev, "failed to set display off: %d\n", ret);
@@ -539,8 +535,6 @@ static int ltk050h3146w_unprepare(struct drm_panel *panel)
regulator_disable(ctx->iovcc);
regulator_disable(ctx->vci);
- ctx->prepared = false;
-
return 0;
}
@@ -550,9 +544,6 @@ static int ltk050h3146w_prepare(struct drm_panel *panel)
struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
int ret;
- if (ctx->prepared)
- return 0;
-
dev_dbg(ctx->dev, "Resetting the panel\n");
ret = regulator_enable(ctx->vci);
if (ret < 0) {
@@ -593,8 +584,6 @@ static int ltk050h3146w_prepare(struct drm_panel *panel)
msleep(50);
- ctx->prepared = true;
-
return 0;
disable_iovcc:
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: "Heiko Stübner" <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c b/drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c
index a4c9a5cb9811..ef27cab08f1d 100644
--- a/drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c
+++ b/drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c
@@ -40,7 +40,6 @@ struct ltk500hd1829 {
struct regulator *vcc;
struct regulator *iovcc;
const struct ltk500hd1829_desc *panel_desc;
- bool prepared;
};
static const struct ltk500hd1829_cmd ltk101b4029w_init[] = {
@@ -492,9 +491,6 @@ static int ltk500hd1829_unprepare(struct drm_panel *panel)
struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
int ret;
- if (!ctx->prepared)
- return 0;
-
ret = mipi_dsi_dcs_set_display_off(dsi);
if (ret < 0)
dev_err(panel->dev, "failed to set display off: %d\n", ret);
@@ -510,8 +506,6 @@ static int ltk500hd1829_unprepare(struct drm_panel *panel)
regulator_disable(ctx->iovcc);
regulator_disable(ctx->vcc);
- ctx->prepared = false;
-
return 0;
}
@@ -522,9 +516,6 @@ static int ltk500hd1829_prepare(struct drm_panel *panel)
unsigned int i;
int ret;
- if (ctx->prepared)
- return 0;
-
ret = regulator_enable(ctx->vcc);
if (ret < 0) {
dev_err(ctx->dev, "Failed to enable vci supply: %d\n", ret);
@@ -568,8 +559,6 @@ static int ltk500hd1829_prepare(struct drm_panel *panel)
goto disable_iovcc;
}
- ctx->prepared = true;
-
return 0;
disable_iovcc:
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
Unfortunately, grepping mainline for this panel's compatible string
shows no hits, so we can't be 100% sure if the DRM modeset driver used
with this panel has been fixed. If it is found that the DRM modeset
driver hasn't been fixed then this patch could be temporarily reverted
until it is.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: "Heiko Stübner" <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
.../gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c b/drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c
index ef27cab08f1d..6b18cf00fd4a 100644
--- a/drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c
+++ b/drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c
@@ -662,27 +662,11 @@ static int ltk500hd1829_probe(struct mipi_dsi_device *dsi)
return 0;
}
-static void ltk500hd1829_shutdown(struct mipi_dsi_device *dsi)
-{
- struct ltk500hd1829 *ctx = mipi_dsi_get_drvdata(dsi);
- int ret;
-
- ret = drm_panel_unprepare(&ctx->panel);
- if (ret < 0)
- dev_err(&dsi->dev, "Failed to unprepare panel: %d\n", ret);
-
- ret = drm_panel_disable(&ctx->panel);
- if (ret < 0)
- dev_err(&dsi->dev, "Failed to disable panel: %d\n", ret);
-}
-
static void ltk500hd1829_remove(struct mipi_dsi_device *dsi)
{
struct ltk500hd1829 *ctx = mipi_dsi_get_drvdata(dsi);
int ret;
- ltk500hd1829_shutdown(dsi);
-
ret = mipi_dsi_detach(dsi);
if (ret < 0)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", ret);
@@ -710,7 +694,6 @@ static struct mipi_dsi_driver ltk500hd1829_driver = {
},
.probe = ltk500hd1829_probe,
.remove = ltk500hd1829_remove,
- .shutdown = ltk500hd1829_shutdown,
};
module_mipi_dsi_driver(ltk500hd1829_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Peter Ujfalusi <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
.../drm/panel/panel-osd-osd101t2587-53ts.c | 27 +------------------
1 file changed, 1 insertion(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c b/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c
index 493e0504f6f7..c0da7d9512e8 100644
--- a/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c
+++ b/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c
@@ -21,9 +21,6 @@ struct osd101t2587_panel {
struct regulator *supply;
- bool prepared;
- bool enabled;
-
const struct drm_display_mode *default_mode;
};
@@ -37,13 +34,8 @@ static int osd101t2587_panel_disable(struct drm_panel *panel)
struct osd101t2587_panel *osd101t2587 = ti_osd_panel(panel);
int ret;
- if (!osd101t2587->enabled)
- return 0;
-
ret = mipi_dsi_shutdown_peripheral(osd101t2587->dsi);
- osd101t2587->enabled = false;
-
return ret;
}
@@ -51,11 +43,7 @@ static int osd101t2587_panel_unprepare(struct drm_panel *panel)
{
struct osd101t2587_panel *osd101t2587 = ti_osd_panel(panel);
- if (!osd101t2587->prepared)
- return 0;
-
regulator_disable(osd101t2587->supply);
- osd101t2587->prepared = false;
return 0;
}
@@ -63,16 +51,8 @@ static int osd101t2587_panel_unprepare(struct drm_panel *panel)
static int osd101t2587_panel_prepare(struct drm_panel *panel)
{
struct osd101t2587_panel *osd101t2587 = ti_osd_panel(panel);
- int ret;
- if (osd101t2587->prepared)
- return 0;
-
- ret = regulator_enable(osd101t2587->supply);
- if (!ret)
- osd101t2587->prepared = true;
-
- return ret;
+ return regulator_enable(osd101t2587->supply);
}
static int osd101t2587_panel_enable(struct drm_panel *panel)
@@ -80,15 +60,10 @@ static int osd101t2587_panel_enable(struct drm_panel *panel)
struct osd101t2587_panel *osd101t2587 = ti_osd_panel(panel);
int ret;
- if (osd101t2587->enabled)
- return 0;
-
ret = mipi_dsi_turn_on_peripheral(osd101t2587->dsi);
if (ret)
return ret;
- osd101t2587->enabled = true;
-
return ret;
}
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
Unfortunately, grepping mainline for this panel's compatible string
shows no hits, so we can't be 100% sure if the DRM modeset driver used
with this panel has been fixed. If it is found that the DRM modeset
driver hasn't been fixed then this patch could be temporarily reverted
until it is.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Stefan Mavrodiev <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c b/drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c
index 8a687d3ba236..94ae8c8270b8 100644
--- a/drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c
+++ b/drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c
@@ -251,9 +251,6 @@ static void lcd_olinuxino_remove(struct i2c_client *client)
struct lcd_olinuxino *panel = i2c_get_clientdata(client);
drm_panel_remove(&panel->panel);
-
- drm_panel_disable(&panel->panel);
- drm_panel_unprepare(&panel->panel);
}
static const struct of_device_id lcd_olinuxino_of_ids[] = {
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by TI OMAP boards. The OMAP driver appears
to be correctly calling drm_atomic_helper_shutdown() so we can remove
the calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Peter Ujfalusi <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c b/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c
index c0da7d9512e8..dbea84f51514 100644
--- a/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c
+++ b/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c
@@ -186,11 +186,6 @@ static void osd101t2587_panel_remove(struct mipi_dsi_device *dsi)
struct osd101t2587_panel *osd101t2587 = mipi_dsi_get_drvdata(dsi);
int ret;
- ret = drm_panel_disable(&osd101t2587->base);
- if (ret < 0)
- dev_warn(&dsi->dev, "failed to disable panel: %d\n", ret);
-
- drm_panel_unprepare(&osd101t2587->base);
drm_panel_remove(&osd101t2587->base);
ret = mipi_dsi_detach(dsi);
@@ -198,14 +193,6 @@ static void osd101t2587_panel_remove(struct mipi_dsi_device *dsi)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", ret);
}
-static void osd101t2587_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct osd101t2587_panel *osd101t2587 = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_disable(&osd101t2587->base);
- drm_panel_unprepare(&osd101t2587->base);
-}
-
static struct mipi_dsi_driver osd101t2587_panel_driver = {
.driver = {
.name = "panel-osd-osd101t2587-53ts",
@@ -213,7 +200,6 @@ static struct mipi_dsi_driver osd101t2587_panel_driver = {
},
.probe = osd101t2587_panel_probe,
.remove = osd101t2587_panel_remove,
- .shutdown = osd101t2587_panel_shutdown,
};
module_mipi_dsi_driver(osd101t2587_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Jitao Shi <[email protected]>
Cc: Cong Yang <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
index 0ffe8f8c01de..667e1bb4a58b 100644
--- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
+++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
@@ -50,8 +50,6 @@ struct boe_panel {
struct regulator *avee;
struct regulator *avdd;
struct gpio_desc *enable_gpio;
-
- bool prepared;
};
enum dsi_cmd_type {
@@ -1450,9 +1448,6 @@ static int boe_panel_unprepare(struct drm_panel *panel)
{
struct boe_panel *boe = to_boe_panel(panel);
- if (!boe->prepared)
- return 0;
-
if (boe->desc->discharge_on_disable) {
regulator_disable(boe->avee);
regulator_disable(boe->avdd);
@@ -1471,8 +1466,6 @@ static int boe_panel_unprepare(struct drm_panel *panel)
regulator_disable(boe->pp3300);
}
- boe->prepared = false;
-
return 0;
}
@@ -1481,9 +1474,6 @@ static int boe_panel_prepare(struct drm_panel *panel)
struct boe_panel *boe = to_boe_panel(panel);
int ret;
- if (boe->prepared)
- return 0;
-
gpiod_set_value(boe->enable_gpio, 0);
usleep_range(1000, 1500);
@@ -1523,8 +1513,6 @@ static int boe_panel_prepare(struct drm_panel *panel)
goto poweroff;
}
- boe->prepared = true;
-
return 0;
poweroff:
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by Qualcomm boards. The Qualcomm driver
appears to be correctly calling drm_atomic_helper_shutdown() so we can
remove the calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-samsung-atna33xc20.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
index a322dd0a532f..9a482a744b8c 100644
--- a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
+++ b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
@@ -327,21 +327,10 @@ static void atana33xc20_remove(struct dp_aux_ep_device *aux_ep)
struct atana33xc20_panel *panel = dev_get_drvdata(dev);
drm_panel_remove(&panel->base);
- drm_panel_disable(&panel->base);
- drm_panel_unprepare(&panel->base);
drm_edid_free(panel->drm_edid);
}
-static void atana33xc20_shutdown(struct dp_aux_ep_device *aux_ep)
-{
- struct device *dev = &aux_ep->dev;
- struct atana33xc20_panel *panel = dev_get_drvdata(dev);
-
- drm_panel_disable(&panel->base);
- drm_panel_unprepare(&panel->base);
-}
-
static const struct of_device_id atana33xc20_dt_match[] = {
{ .compatible = "samsung,atna33xc20", },
{ /* sentinal */ }
@@ -362,7 +351,6 @@ static struct dp_aux_ep_driver atana33xc20_driver = {
},
.probe = atana33xc20_probe,
.remove = atana33xc20_remove,
- .shutdown = atana33xc20_shutdown,
};
static int __init atana33xc20_init(void)
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-simple.c | 27 ---------------------------
1 file changed, 27 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index dcb6d0b6ced0..42d902d2bbbe 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -138,9 +138,6 @@ struct panel_desc {
struct panel_simple {
struct drm_panel base;
- bool enabled;
-
- bool prepared;
ktime_t unprepared_time;
@@ -290,14 +287,9 @@ static int panel_simple_disable(struct drm_panel *panel)
{
struct panel_simple *p = to_panel_simple(panel);
- if (!p->enabled)
- return 0;
-
if (p->desc->delay.disable)
msleep(p->desc->delay.disable);
- p->enabled = false;
-
return 0;
}
@@ -317,18 +309,12 @@ static int panel_simple_suspend(struct device *dev)
static int panel_simple_unprepare(struct drm_panel *panel)
{
- struct panel_simple *p = to_panel_simple(panel);
int ret;
- /* Unpreparing when already unprepared is a no-op */
- if (!p->prepared)
- return 0;
-
pm_runtime_mark_last_busy(panel->dev);
ret = pm_runtime_put_autosuspend(panel->dev);
if (ret < 0)
return ret;
- p->prepared = false;
return 0;
}
@@ -356,21 +342,14 @@ static int panel_simple_resume(struct device *dev)
static int panel_simple_prepare(struct drm_panel *panel)
{
- struct panel_simple *p = to_panel_simple(panel);
int ret;
- /* Preparing when already prepared is a no-op */
- if (p->prepared)
- return 0;
-
ret = pm_runtime_get_sync(panel->dev);
if (ret < 0) {
pm_runtime_put_autosuspend(panel->dev);
return ret;
}
- p->prepared = true;
-
return 0;
}
@@ -378,14 +357,9 @@ static int panel_simple_enable(struct drm_panel *panel)
{
struct panel_simple *p = to_panel_simple(panel);
- if (p->enabled)
- return 0;
-
if (p->desc->delay.enable)
msleep(p->desc->delay.enable);
- p->enabled = true;
-
return 0;
}
@@ -609,7 +583,6 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
if (!panel)
return -ENOMEM;
- panel->enabled = false;
panel->desc = desc;
panel->supply = devm_regulator_get(dev, "power");
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
.../gpu/drm/panel/panel-samsung-atna33xc20.c | 24 -------------------
1 file changed, 24 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
index a9f0d214a900..a322dd0a532f 100644
--- a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
+++ b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
@@ -25,8 +25,6 @@
struct atana33xc20_panel {
struct drm_panel base;
- bool prepared;
- bool enabled;
bool el3_was_on;
bool no_hpd;
@@ -143,13 +141,8 @@ static int atana33xc20_disable(struct drm_panel *panel)
{
struct atana33xc20_panel *p = to_atana33xc20(panel);
- /* Disabling when already disabled is a no-op */
- if (!p->enabled)
- return 0;
-
gpiod_set_value_cansleep(p->el_on3_gpio, 0);
p->el_on3_off_time = ktime_get_boottime();
- p->enabled = false;
/*
* Keep track of the fact that EL_ON3 was on but we haven't power
@@ -173,10 +166,6 @@ static int atana33xc20_enable(struct drm_panel *panel)
{
struct atana33xc20_panel *p = to_atana33xc20(panel);
- /* Enabling when already enabled is a no-op */
- if (p->enabled)
- return 0;
-
/*
* Once EL_ON3 drops we absolutely need a power cycle before the next
* enable or the backlight will never come on again. The code ensures
@@ -195,20 +184,14 @@ static int atana33xc20_enable(struct drm_panel *panel)
atana33xc20_wait(p->powered_on_time, 400);
gpiod_set_value_cansleep(p->el_on3_gpio, 1);
- p->enabled = true;
return 0;
}
static int atana33xc20_unprepare(struct drm_panel *panel)
{
- struct atana33xc20_panel *p = to_atana33xc20(panel);
int ret;
- /* Unpreparing when already unprepared is a no-op */
- if (!p->prepared)
- return 0;
-
/*
* Purposely do a put_sync, don't use autosuspend. The panel's tcon
* seems to sometimes crash when you stop giving it data and this is
@@ -220,26 +203,19 @@ static int atana33xc20_unprepare(struct drm_panel *panel)
ret = pm_runtime_put_sync_suspend(panel->dev);
if (ret < 0)
return ret;
- p->prepared = false;
return 0;
}
static int atana33xc20_prepare(struct drm_panel *panel)
{
- struct atana33xc20_panel *p = to_atana33xc20(panel);
int ret;
- /* Preparing when already prepared is a no-op */
- if (p->prepared)
- return 0;
-
ret = pm_runtime_get_sync(panel->dev);
if (ret < 0) {
pm_runtime_put_autosuspend(panel->dev);
return ret;
}
- p->prepared = true;
return 0;
}
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
Unfortunately, it's very difficult to know exactly which DRM modeset
drivers are using panel-simple due to the sheer number of panels it
handles. For now, we'll leave the calls and just add a comment to keep
people from copying this code.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
- panel-edp and panel-simple just get a comment now.
drivers/gpu/drm/panel/panel-simple.c | 33 +++++++++++++++++++---------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 42d902d2bbbe..f39122ffdead 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -716,26 +716,39 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
return err;
}
-static void panel_simple_remove(struct device *dev)
+static void panel_simple_shutdown(struct device *dev)
{
struct panel_simple *panel = dev_get_drvdata(dev);
- drm_panel_remove(&panel->base);
+ /*
+ * NOTE: the following two calls don't really belong here. It is the
+ * responsibility of a correctly written DRM modeset driver to call
+ * drm_atomic_helper_shutdown() at shutdown time and that should
+ * cause the panel to be disabled / unprepared if needed. For now,
+ * however, we'll keep these calls due to the sheer number of
+ * different DRM modeset drivers used with panel-simple. The fact that
+ * we're calling these and _also_ the drm_atomic_helper_shutdown()
+ * will try to disable/unprepare means that we can get a warning about
+ * trying to disable/unprepare an already disabled/unprepared panel,
+ * but that's something we'll have to live with until we've confirmed
+ * that all DRM modeset drivers are properly calling
+ * drm_atomic_helper_shutdown().
+ */
drm_panel_disable(&panel->base);
drm_panel_unprepare(&panel->base);
-
- pm_runtime_dont_use_autosuspend(dev);
- pm_runtime_disable(dev);
- if (panel->ddc)
- put_device(&panel->ddc->dev);
}
-static void panel_simple_shutdown(struct device *dev)
+static void panel_simple_remove(struct device *dev)
{
struct panel_simple *panel = dev_get_drvdata(dev);
- drm_panel_disable(&panel->base);
- drm_panel_unprepare(&panel->base);
+ drm_panel_remove(&panel->base);
+ panel_simple_shutdown(dev);
+
+ pm_runtime_dont_use_autosuspend(dev);
+ pm_runtime_disable(dev);
+ if (panel->ddc)
+ put_device(&panel->ddc->dev);
}
static const struct drm_display_mode ampire_am_1280800n3tzqw_t00h_mode = {
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by Mediatek and Qualcomm boards. Both of
those drivers appear to be correctly calling
drm_atomic_helper_shutdown() so we can remove the calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Jitao Shi <[email protected]>
Cc: Cong Yang <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
index 667e1bb4a58b..77b20e24cac7 100644
--- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
+++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
@@ -1910,21 +1910,11 @@ static int boe_panel_probe(struct mipi_dsi_device *dsi)
return ret;
}
-static void boe_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct boe_panel *boe = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_disable(&boe->base);
- drm_panel_unprepare(&boe->base);
-}
-
static void boe_panel_remove(struct mipi_dsi_device *dsi)
{
struct boe_panel *boe = mipi_dsi_get_drvdata(dsi);
int ret;
- boe_panel_shutdown(dsi);
-
ret = mipi_dsi_detach(dsi);
if (ret < 0)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", ret);
@@ -1972,7 +1962,6 @@ static struct mipi_dsi_driver boe_panel_driver = {
},
.probe = boe_panel_probe,
.remove = boe_panel_remove,
- .shutdown = boe_panel_shutdown,
};
module_mipi_dsi_driver(boe_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Neil Armstrong <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c b/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
index d8487bc6d611..36f27c664b69 100644
--- a/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
+++ b/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
@@ -24,8 +24,6 @@ struct tdo_tl070wsh30_panel {
struct regulator *supply;
struct gpio_desc *reset_gpio;
-
- bool prepared;
};
static inline
@@ -39,9 +37,6 @@ static int tdo_tl070wsh30_panel_prepare(struct drm_panel *panel)
struct tdo_tl070wsh30_panel *tdo_tl070wsh30 = to_tdo_tl070wsh30_panel(panel);
int err;
- if (tdo_tl070wsh30->prepared)
- return 0;
-
err = regulator_enable(tdo_tl070wsh30->supply);
if (err < 0)
return err;
@@ -74,8 +69,6 @@ static int tdo_tl070wsh30_panel_prepare(struct drm_panel *panel)
msleep(20);
- tdo_tl070wsh30->prepared = true;
-
return 0;
}
@@ -84,9 +77,6 @@ static int tdo_tl070wsh30_panel_unprepare(struct drm_panel *panel)
struct tdo_tl070wsh30_panel *tdo_tl070wsh30 = to_tdo_tl070wsh30_panel(panel);
int err;
- if (!tdo_tl070wsh30->prepared)
- return 0;
-
err = mipi_dsi_dcs_set_display_off(tdo_tl070wsh30->link);
if (err < 0)
dev_err(panel->dev, "failed to set display off: %d\n", err);
@@ -103,8 +93,6 @@ static int tdo_tl070wsh30_panel_unprepare(struct drm_panel *panel)
regulator_disable(tdo_tl070wsh30->supply);
- tdo_tl070wsh30->prepared = false;
-
return 0;
}
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: "Heiko Stübner" <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
index 8670386498a4..8262c00670cf 100644
--- a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
+++ b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
@@ -52,7 +52,6 @@ struct xpp055c272 {
struct gpio_desc *reset_gpio;
struct regulator *vci;
struct regulator *iovcc;
- bool prepared;
};
static inline struct xpp055c272 *panel_to_xpp055c272(struct drm_panel *panel)
@@ -136,9 +135,6 @@ static int xpp055c272_unprepare(struct drm_panel *panel)
struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
int ret;
- if (!ctx->prepared)
- return 0;
-
ret = mipi_dsi_dcs_set_display_off(dsi);
if (ret < 0)
dev_err(ctx->dev, "failed to set display off: %d\n", ret);
@@ -152,8 +148,6 @@ static int xpp055c272_unprepare(struct drm_panel *panel)
regulator_disable(ctx->iovcc);
regulator_disable(ctx->vci);
- ctx->prepared = false;
-
return 0;
}
@@ -163,9 +157,6 @@ static int xpp055c272_prepare(struct drm_panel *panel)
struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
int ret;
- if (ctx->prepared)
- return 0;
-
dev_dbg(ctx->dev, "Resetting the panel\n");
ret = regulator_enable(ctx->vci);
if (ret < 0) {
@@ -209,8 +200,6 @@ static int xpp055c272_prepare(struct drm_panel *panel)
msleep(50);
- ctx->prepared = true;
-
return 0;
disable_iovcc:
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
Unfortunately, grepping mainline for this panel's compatible string
shows no hits, so we can't be 100% sure if the DRM modeset driver used
with this panel has been fixed. If it is found that the DRM modeset
driver hasn't been fixed then this patch could be temporarily reverted
until it is.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Neil Armstrong <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c b/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
index 36f27c664b69..227f97f9b136 100644
--- a/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
+++ b/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
@@ -208,16 +208,6 @@ static void tdo_tl070wsh30_panel_remove(struct mipi_dsi_device *dsi)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", err);
drm_panel_remove(&tdo_tl070wsh30->base);
- drm_panel_disable(&tdo_tl070wsh30->base);
- drm_panel_unprepare(&tdo_tl070wsh30->base);
-}
-
-static void tdo_tl070wsh30_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct tdo_tl070wsh30_panel *tdo_tl070wsh30 = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_disable(&tdo_tl070wsh30->base);
- drm_panel_unprepare(&tdo_tl070wsh30->base);
}
static struct mipi_dsi_driver tdo_tl070wsh30_panel_driver = {
@@ -227,7 +217,6 @@ static struct mipi_dsi_driver tdo_tl070wsh30_panel_driver = {
},
.probe = tdo_tl070wsh30_panel_probe,
.remove = tdo_tl070wsh30_panel_remove,
- .shutdown = tdo_tl070wsh30_panel_shutdown,
};
module_mipi_dsi_driver(tdo_tl070wsh30_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
NOTE: as part of this, transition the panel's direct calls to its
disable function in shutdown/remove to call through DRM panel.
Cc: Vinay Simha BN <[email protected]>
Cc: Sumit Semwal <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Note: since we are able to identify that this panel only appears to be
used on Qualcomm boards and we have to touch the shutdown/remove path
in this patch anyway, we could possibly squash this with the next
patch that removes the disable call in shutdown/remove. For now I'm
keeping them separate just to keep the concepts separate.
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
.../gpu/drm/panel/panel-jdi-lt070me05000.c | 27 ++-----------------
1 file changed, 2 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
index f9a69f347068..4ddddee6fa1e 100644
--- a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
+++ b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
@@ -37,9 +37,6 @@ struct jdi_panel {
struct gpio_desc *dcdc_en_gpio;
struct backlight_device *backlight;
- bool prepared;
- bool enabled;
-
const struct drm_display_mode *mode;
};
@@ -176,13 +173,8 @@ static int jdi_panel_disable(struct drm_panel *panel)
{
struct jdi_panel *jdi = to_jdi_panel(panel);
- if (!jdi->enabled)
- return 0;
-
backlight_disable(jdi->backlight);
- jdi->enabled = false;
-
return 0;
}
@@ -192,9 +184,6 @@ static int jdi_panel_unprepare(struct drm_panel *panel)
struct device *dev = &jdi->dsi->dev;
int ret;
- if (!jdi->prepared)
- return 0;
-
jdi_panel_off(jdi);
ret = regulator_bulk_disable(ARRAY_SIZE(jdi->supplies), jdi->supplies);
@@ -207,8 +196,6 @@ static int jdi_panel_unprepare(struct drm_panel *panel)
gpiod_set_value(jdi->dcdc_en_gpio, 0);
- jdi->prepared = false;
-
return 0;
}
@@ -218,9 +205,6 @@ static int jdi_panel_prepare(struct drm_panel *panel)
struct device *dev = &jdi->dsi->dev;
int ret;
- if (jdi->prepared)
- return 0;
-
ret = regulator_bulk_enable(ARRAY_SIZE(jdi->supplies), jdi->supplies);
if (ret < 0) {
dev_err(dev, "regulator enable failed, %d\n", ret);
@@ -250,8 +234,6 @@ static int jdi_panel_prepare(struct drm_panel *panel)
goto poweroff;
}
- jdi->prepared = true;
-
return 0;
poweroff:
@@ -272,13 +254,8 @@ static int jdi_panel_enable(struct drm_panel *panel)
{
struct jdi_panel *jdi = to_jdi_panel(panel);
- if (jdi->enabled)
- return 0;
-
backlight_enable(jdi->backlight);
- jdi->enabled = true;
-
return 0;
}
@@ -475,7 +452,7 @@ static void jdi_panel_remove(struct mipi_dsi_device *dsi)
struct jdi_panel *jdi = mipi_dsi_get_drvdata(dsi);
int ret;
- ret = jdi_panel_disable(&jdi->base);
+ ret = drm_panel_disable(&jdi->base);
if (ret < 0)
dev_err(&dsi->dev, "failed to disable panel: %d\n", ret);
@@ -491,7 +468,7 @@ static void jdi_panel_shutdown(struct mipi_dsi_device *dsi)
{
struct jdi_panel *jdi = mipi_dsi_get_drvdata(dsi);
- jdi_panel_disable(&jdi->base);
+ drm_panel_disable(&jdi->base);
}
static struct mipi_dsi_driver jdi_panel_driver = {
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Marco Franchi <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Laurentiu Palcu <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: [email protected]
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 40 ---------------------
1 file changed, 40 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
index 658c7c040570..98480904126c 100644
--- a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
+++ b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
@@ -44,8 +44,6 @@ struct seiko_panel_desc {
struct seiko_panel {
struct drm_panel base;
- bool prepared;
- bool enabled;
const struct seiko_panel_desc *desc;
struct regulator *dvdd;
struct regulator *avdd;
@@ -122,25 +120,10 @@ static int seiko_panel_get_fixed_modes(struct seiko_panel *panel,
return num;
}
-static int seiko_panel_disable(struct drm_panel *panel)
-{
- struct seiko_panel *p = to_seiko_panel(panel);
-
- if (!p->enabled)
- return 0;
-
- p->enabled = false;
-
- return 0;
-}
-
static int seiko_panel_unprepare(struct drm_panel *panel)
{
struct seiko_panel *p = to_seiko_panel(panel);
- if (!p->prepared)
- return 0;
-
gpiod_set_value_cansleep(p->enable_gpio, 0);
regulator_disable(p->avdd);
@@ -150,8 +133,6 @@ static int seiko_panel_unprepare(struct drm_panel *panel)
regulator_disable(p->dvdd);
- p->prepared = false;
-
return 0;
}
@@ -160,9 +141,6 @@ static int seiko_panel_prepare(struct drm_panel *panel)
struct seiko_panel *p = to_seiko_panel(panel);
int err;
- if (p->prepared)
- return 0;
-
err = regulator_enable(p->dvdd);
if (err < 0) {
dev_err(panel->dev, "failed to enable dvdd: %d\n", err);
@@ -180,8 +158,6 @@ static int seiko_panel_prepare(struct drm_panel *panel)
gpiod_set_value_cansleep(p->enable_gpio, 1);
- p->prepared = true;
-
return 0;
disable_dvdd:
@@ -189,18 +165,6 @@ static int seiko_panel_prepare(struct drm_panel *panel)
return err;
}
-static int seiko_panel_enable(struct drm_panel *panel)
-{
- struct seiko_panel *p = to_seiko_panel(panel);
-
- if (p->enabled)
- return 0;
-
- p->enabled = true;
-
- return 0;
-}
-
static int seiko_panel_get_modes(struct drm_panel *panel,
struct drm_connector *connector)
{
@@ -228,10 +192,8 @@ static int seiko_panel_get_timings(struct drm_panel *panel,
}
static const struct drm_panel_funcs seiko_panel_funcs = {
- .disable = seiko_panel_disable,
.unprepare = seiko_panel_unprepare,
.prepare = seiko_panel_prepare,
- .enable = seiko_panel_enable,
.get_modes = seiko_panel_get_modes,
.get_timings = seiko_panel_get_timings,
};
@@ -246,8 +208,6 @@ static int seiko_panel_probe(struct device *dev,
if (!panel)
return -ENOMEM;
- panel->enabled = false;
- panel->prepared = false;
panel->desc = desc;
panel->dvdd = devm_regulator_get(dev, "dvdd");
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by IMX boards. As far as I can tell, all IMX
boards are now correctly calling drm_atomic_helper_shutdown() so we
can remove the calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Marco Franchi <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Laurentiu Palcu <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: [email protected]
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
index 98480904126c..8a3fe531c641 100644
--- a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
+++ b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
@@ -243,14 +243,6 @@ static void seiko_panel_remove(struct platform_device *pdev)
struct seiko_panel *panel = platform_get_drvdata(pdev);
drm_panel_remove(&panel->base);
- drm_panel_disable(&panel->base);
-}
-
-static void seiko_panel_shutdown(struct platform_device *pdev)
-{
- struct seiko_panel *panel = platform_get_drvdata(pdev);
-
- drm_panel_disable(&panel->base);
}
static const struct display_timing seiko_43wvf1g_timing = {
@@ -306,7 +298,6 @@ static struct platform_driver seiko_panel_platform_driver = {
},
.probe = seiko_panel_platform_probe,
.remove_new = seiko_panel_remove,
- .shutdown = seiko_panel_shutdown,
};
module_platform_driver(seiko_panel_platform_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by Rockchip boards. The Rockchip driver
appears to be correctly calling drm_atomic_helper_shutdown() so we can
remove the calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: "Heiko Stübner" <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
.../gpu/drm/panel/panel-xinpeng-xpp055c272.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
index 8262c00670cf..22a14006765e 100644
--- a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
+++ b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
@@ -306,27 +306,11 @@ static int xpp055c272_probe(struct mipi_dsi_device *dsi)
return 0;
}
-static void xpp055c272_shutdown(struct mipi_dsi_device *dsi)
-{
- struct xpp055c272 *ctx = mipi_dsi_get_drvdata(dsi);
- int ret;
-
- ret = drm_panel_unprepare(&ctx->panel);
- if (ret < 0)
- dev_err(&dsi->dev, "Failed to unprepare panel: %d\n", ret);
-
- ret = drm_panel_disable(&ctx->panel);
- if (ret < 0)
- dev_err(&dsi->dev, "Failed to disable panel: %d\n", ret);
-}
-
static void xpp055c272_remove(struct mipi_dsi_device *dsi)
{
struct xpp055c272 *ctx = mipi_dsi_get_drvdata(dsi);
int ret;
- xpp055c272_shutdown(dsi);
-
ret = mipi_dsi_detach(dsi);
if (ret < 0)
dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
@@ -347,7 +331,6 @@ static struct mipi_dsi_driver xpp055c272_driver = {
},
.probe = xpp055c272_probe,
.remove = xpp055c272_remove,
- .shutdown = xpp055c272_shutdown,
};
module_mipi_dsi_driver(xpp055c272_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Werner Johansson <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
index 855e64444daa..c86337954ad7 100644
--- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
@@ -26,8 +26,6 @@ struct sharp_nt_panel {
struct regulator *supply;
struct gpio_desc *reset_gpio;
-
- bool prepared;
};
static inline struct sharp_nt_panel *to_sharp_nt_panel(struct drm_panel *panel)
@@ -99,9 +97,6 @@ static int sharp_nt_panel_unprepare(struct drm_panel *panel)
struct sharp_nt_panel *sharp_nt = to_sharp_nt_panel(panel);
int ret;
- if (!sharp_nt->prepared)
- return 0;
-
ret = sharp_nt_panel_off(sharp_nt);
if (ret < 0) {
dev_err(panel->dev, "failed to set panel off: %d\n", ret);
@@ -112,8 +107,6 @@ static int sharp_nt_panel_unprepare(struct drm_panel *panel)
if (sharp_nt->reset_gpio)
gpiod_set_value(sharp_nt->reset_gpio, 0);
- sharp_nt->prepared = false;
-
return 0;
}
@@ -122,9 +115,6 @@ static int sharp_nt_panel_prepare(struct drm_panel *panel)
struct sharp_nt_panel *sharp_nt = to_sharp_nt_panel(panel);
int ret;
- if (sharp_nt->prepared)
- return 0;
-
ret = regulator_enable(sharp_nt->supply);
if (ret < 0)
return ret;
@@ -152,8 +142,6 @@ static int sharp_nt_panel_prepare(struct drm_panel *panel)
goto poweroff;
}
- sharp_nt->prepared = true;
-
return 0;
poweroff:
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
One thing to note for st7703 is that it has a special "allpixelson"
debugfs file. When this file is written the driver hacks a
disable/unprepare and then a prepare/enable to try to reset the
panel. Potentially that might have been relying on the old booleans we
removed. It'll still "work" because of the checks in the core but it
deserves a comment. This debugfs file didn't appear to be particularly
safe to use even before this patch since it would cause a
disabled/unprepared panel to become prepared/enabled.
Cc: "Guido Günther" <[email protected]>
Cc: "Ondřej Jirman" <[email protected]>
Cc: Chris Morgan <[email protected]>
Cc: Frank Oltmanns <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-sitronix-st7703.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7703.c b/drivers/gpu/drm/panel/panel-sitronix-st7703.c
index 7d8302cca091..6b2d940640ca 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7703.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7703.c
@@ -58,7 +58,6 @@ struct st7703 {
struct gpio_desc *reset_gpio;
struct regulator *vcc;
struct regulator *iovcc;
- bool prepared;
struct dentry *debugfs;
const struct st7703_panel_desc *desc;
@@ -752,13 +751,9 @@ static int st7703_unprepare(struct drm_panel *panel)
{
struct st7703 *ctx = panel_to_st7703(panel);
- if (!ctx->prepared)
- return 0;
-
gpiod_set_value_cansleep(ctx->reset_gpio, 1);
regulator_disable(ctx->iovcc);
regulator_disable(ctx->vcc);
- ctx->prepared = false;
return 0;
}
@@ -768,9 +763,6 @@ static int st7703_prepare(struct drm_panel *panel)
struct st7703 *ctx = panel_to_st7703(panel);
int ret;
- if (ctx->prepared)
- return 0;
-
dev_dbg(ctx->dev, "Resetting the panel\n");
gpiod_set_value_cansleep(ctx->reset_gpio, 1);
@@ -793,8 +785,6 @@ static int st7703_prepare(struct drm_panel *panel)
gpiod_set_value_cansleep(ctx->reset_gpio, 0);
usleep_range(15000, 20000);
- ctx->prepared = true;
-
return 0;
}
@@ -854,7 +844,13 @@ static int allpixelson_set(void *data, u64 val)
dev_dbg(ctx->dev, "Setting all pixels on\n");
mipi_dsi_generic_write_seq(dsi, ST7703_CMD_ALL_PIXEL_ON);
msleep(val * 1000);
- /* Reset the panel to get video back */
+
+ /*
+ * Reset the panel to get video back. NOTE: This isn't a
+ * particularly safe thing to do in general because it assumes
+ * that the screen was on to begin with, but this is just a
+ * debugfs file so it's not a huge deal.
+ */
drm_panel_disable(&ctx->panel);
drm_panel_unprepare(&ctx->panel);
drm_panel_prepare(&ctx->panel);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
The compatible strings used by this driver seem to show up across
boards using a variety of DRM drivers. It appears that the relevant
drivers have been converted, but at least one compatible string
doesn't seem to be found in any mainline dts files so we can't be 100%
sure. If it is found that the DRM modeset driver hasn't been fixed
then this patch could be temporarily reverted until it is.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: "Guido Günther" <[email protected]>
Cc: "Ondřej Jirman" <[email protected]>
Cc: Chris Morgan <[email protected]>
Cc: Frank Oltmanns <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-sitronix-st7703.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7703.c b/drivers/gpu/drm/panel/panel-sitronix-st7703.c
index 6b2d940640ca..77b30e045a57 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7703.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7703.c
@@ -937,27 +937,11 @@ static int st7703_probe(struct mipi_dsi_device *dsi)
return 0;
}
-static void st7703_shutdown(struct mipi_dsi_device *dsi)
-{
- struct st7703 *ctx = mipi_dsi_get_drvdata(dsi);
- int ret;
-
- ret = drm_panel_unprepare(&ctx->panel);
- if (ret < 0)
- dev_err(&dsi->dev, "Failed to unprepare panel: %d\n", ret);
-
- ret = drm_panel_disable(&ctx->panel);
- if (ret < 0)
- dev_err(&dsi->dev, "Failed to disable panel: %d\n", ret);
-}
-
static void st7703_remove(struct mipi_dsi_device *dsi)
{
struct st7703 *ctx = mipi_dsi_get_drvdata(dsi);
int ret;
- st7703_shutdown(dsi);
-
ret = mipi_dsi_detach(dsi);
if (ret < 0)
dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
@@ -981,7 +965,6 @@ MODULE_DEVICE_TABLE(of, st7703_of_match);
static struct mipi_dsi_driver st7703_driver = {
.probe = st7703_probe,
.remove = st7703_remove,
- .shutdown = st7703_shutdown,
.driver = {
.name = DRV_NAME,
.of_match_table = st7703_of_match,
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Thierry Reding <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
.../gpu/drm/panel/panel-sharp-lq101r1sx01.c | 39 -------------------
1 file changed, 39 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
index 14851408a5e1..8f6c21b99522 100644
--- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
@@ -24,9 +24,6 @@ struct sharp_panel {
struct regulator *supply;
- bool prepared;
- bool enabled;
-
const struct drm_display_mode *mode;
};
@@ -85,26 +82,11 @@ static __maybe_unused int sharp_panel_read(struct sharp_panel *sharp,
return err;
}
-static int sharp_panel_disable(struct drm_panel *panel)
-{
- struct sharp_panel *sharp = to_sharp_panel(panel);
-
- if (!sharp->enabled)
- return 0;
-
- sharp->enabled = false;
-
- return 0;
-}
-
static int sharp_panel_unprepare(struct drm_panel *panel)
{
struct sharp_panel *sharp = to_sharp_panel(panel);
int err;
- if (!sharp->prepared)
- return 0;
-
sharp_wait_frames(sharp, 4);
err = mipi_dsi_dcs_set_display_off(sharp->link1);
@@ -119,8 +101,6 @@ static int sharp_panel_unprepare(struct drm_panel *panel)
regulator_disable(sharp->supply);
- sharp->prepared = false;
-
return 0;
}
@@ -164,9 +144,6 @@ static int sharp_panel_prepare(struct drm_panel *panel)
u8 format = MIPI_DCS_PIXEL_FMT_24BIT;
int err;
- if (sharp->prepared)
- return 0;
-
err = regulator_enable(sharp->supply);
if (err < 0)
return err;
@@ -235,8 +212,6 @@ static int sharp_panel_prepare(struct drm_panel *panel)
goto poweroff;
}
- sharp->prepared = true;
-
/* wait for 6 frames before continuing */
sharp_wait_frames(sharp, 6);
@@ -247,18 +222,6 @@ static int sharp_panel_prepare(struct drm_panel *panel)
return err;
}
-static int sharp_panel_enable(struct drm_panel *panel)
-{
- struct sharp_panel *sharp = to_sharp_panel(panel);
-
- if (sharp->enabled)
- return 0;
-
- sharp->enabled = true;
-
- return 0;
-}
-
static const struct drm_display_mode default_mode = {
.clock = 278000,
.hdisplay = 2560,
@@ -295,10 +258,8 @@ static int sharp_panel_get_modes(struct drm_panel *panel,
}
static const struct drm_panel_funcs sharp_panel_funcs = {
- .disable = sharp_panel_disable,
.unprepare = sharp_panel_unprepare,
.prepare = sharp_panel_prepare,
- .enable = sharp_panel_enable,
.get_modes = sharp_panel_get_modes,
};
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Sumit Semwal <[email protected]>
Cc: Benni Steini <[email protected]>
Cc: Joel Selvaraj <[email protected]>
Cc: Marijn Suijten <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-novatek-nt36672a.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
index 3886372415c2..35aace79613a 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
@@ -72,8 +72,6 @@ struct nt36672a_panel {
struct regulator_bulk_data supplies[ARRAY_SIZE(nt36672a_regulator_names)];
struct gpio_desc *reset_gpio;
-
- bool prepared;
};
static inline struct nt36672a_panel *to_nt36672a_panel(struct drm_panel *panel)
@@ -119,9 +117,6 @@ static int nt36672a_panel_unprepare(struct drm_panel *panel)
struct nt36672a_panel *pinfo = to_nt36672a_panel(panel);
int ret;
- if (!pinfo->prepared)
- return 0;
-
/* send off cmds */
ret = nt36672a_send_cmds(panel, pinfo->desc->off_cmds,
pinfo->desc->num_off_cmds);
@@ -147,8 +142,6 @@ static int nt36672a_panel_unprepare(struct drm_panel *panel)
if (ret < 0)
dev_err(panel->dev, "power_off failed ret = %d\n", ret);
- pinfo->prepared = false;
-
return ret;
}
@@ -179,9 +172,6 @@ static int nt36672a_panel_prepare(struct drm_panel *panel)
struct nt36672a_panel *pinfo = to_nt36672a_panel(panel);
int err;
- if (pinfo->prepared)
- return 0;
-
err = nt36672a_panel_power_on(pinfo);
if (err < 0)
goto poweroff;
@@ -221,8 +211,6 @@ static int nt36672a_panel_prepare(struct drm_panel *panel)
msleep(120);
- pinfo->prepared = true;
-
return 0;
poweroff:
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by Tegra boards. The Tegra driver appears to
be correctly calling drm_atomic_helper_shutdown() so we can remove the
calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Thierry Reding <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
.../gpu/drm/panel/panel-sharp-lq101r1sx01.c | 24 ++-----------------
1 file changed, 2 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
index 8f6c21b99522..edc9425bb143 100644
--- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
@@ -357,32 +357,13 @@ static void sharp_panel_remove(struct mipi_dsi_device *dsi)
struct sharp_panel *sharp = mipi_dsi_get_drvdata(dsi);
int err;
- /* only detach from host for the DSI-LINK2 interface */
- if (!sharp) {
- mipi_dsi_detach(dsi);
- return;
- }
-
- err = drm_panel_disable(&sharp->base);
- if (err < 0)
- dev_err(&dsi->dev, "failed to disable panel: %d\n", err);
-
err = mipi_dsi_detach(dsi);
if (err < 0)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", err);
- sharp_panel_del(sharp);
-}
-
-static void sharp_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct sharp_panel *sharp = mipi_dsi_get_drvdata(dsi);
-
- /* nothing to do for DSI-LINK2 */
+ /* only detach from host for the DSI-LINK2 interface */
if (!sharp)
- return;
-
- drm_panel_disable(&sharp->base);
+ sharp_panel_del(sharp);
}
static struct mipi_dsi_driver sharp_panel_driver = {
@@ -392,7 +373,6 @@ static struct mipi_dsi_driver sharp_panel_driver = {
},
.probe = sharp_panel_probe,
.remove = sharp_panel_remove,
- .shutdown = sharp_panel_shutdown,
};
module_mipi_dsi_driver(sharp_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
The conversion of the rm67191 panel driver follows many of the other
panel drivers but has a few differences that need to be called out.
Like in commit 1e0465eb16a4 ("drm/panel: otm8009a: Don't double check
prepared/enabled"), this panel also uses the "prepared" flag to
prevent the backlight functions from running when the panel is powered
off. This is probably not the safest thing to do but the old behavior
was preserved. See the discussion in the otm8009a patch. Because of
this, I've left the driver tracking "prepared" but removed its
tracking of "enabled".
NOTE: as part of this, transition the panel's direct calls to its
disable/unprepare functions in shutdown to call through DRM panel.
Cc: Robert Chiras <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-raydium-rm67191.c | 21 ++-----------------
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-raydium-rm67191.c b/drivers/gpu/drm/panel/panel-raydium-rm67191.c
index dbb1ed4efbed..fa9bf89d3bb5 100644
--- a/drivers/gpu/drm/panel/panel-raydium-rm67191.c
+++ b/drivers/gpu/drm/panel/panel-raydium-rm67191.c
@@ -205,7 +205,6 @@ struct rad_panel {
unsigned int num_supplies;
bool prepared;
- bool enabled;
};
static const struct drm_display_mode default_mode = {
@@ -267,9 +266,6 @@ static int rad_panel_prepare(struct drm_panel *panel)
struct rad_panel *rad = to_rad_panel(panel);
int ret;
- if (rad->prepared)
- return 0;
-
ret = regulator_bulk_enable(rad->num_supplies, rad->supplies);
if (ret)
return ret;
@@ -291,9 +287,6 @@ static int rad_panel_unprepare(struct drm_panel *panel)
struct rad_panel *rad = to_rad_panel(panel);
int ret;
- if (!rad->prepared)
- return 0;
-
/*
* Right after asserting the reset, we need to release it, so that the
* touch driver can have an active connection with the touch controller
@@ -322,9 +315,6 @@ static int rad_panel_enable(struct drm_panel *panel)
int color_format = color_format_from_dsi_format(dsi->format);
int ret;
- if (rad->enabled)
- return 0;
-
dsi->mode_flags |= MIPI_DSI_MODE_LPM;
ret = rad_panel_push_cmd_list(dsi);
@@ -389,8 +379,6 @@ static int rad_panel_enable(struct drm_panel *panel)
backlight_enable(rad->backlight);
- rad->enabled = true;
-
return 0;
fail:
@@ -406,9 +394,6 @@ static int rad_panel_disable(struct drm_panel *panel)
struct device *dev = &dsi->dev;
int ret;
- if (!rad->enabled)
- return 0;
-
dsi->mode_flags |= MIPI_DSI_MODE_LPM;
backlight_disable(rad->backlight);
@@ -429,8 +414,6 @@ static int rad_panel_disable(struct drm_panel *panel)
return ret;
}
- rad->enabled = false;
-
return 0;
}
@@ -633,8 +616,8 @@ static void rad_panel_shutdown(struct mipi_dsi_device *dsi)
{
struct rad_panel *rad = mipi_dsi_get_drvdata(dsi);
- rad_panel_disable(&rad->panel);
- rad_panel_unprepare(&rad->panel);
+ drm_panel_disable(&rad->panel);
+ drm_panel_unprepare(&rad->panel);
}
static const struct of_device_id rad_of_match[] = {
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by Qualcomm boards. The Qualcomm driver
appears to be correctly calling drm_atomic_helper_shutdown() so we can
remove the calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Sumit Semwal <[email protected]>
Cc: Benni Steini <[email protected]>
Cc: Joel Selvaraj <[email protected]>
Cc: Marijn Suijten <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-novatek-nt36672a.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
index 35aace79613a..c2abd20e0734 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
@@ -656,14 +656,6 @@ static void nt36672a_panel_remove(struct mipi_dsi_device *dsi)
struct nt36672a_panel *pinfo = mipi_dsi_get_drvdata(dsi);
int err;
- err = drm_panel_unprepare(&pinfo->base);
- if (err < 0)
- dev_err(&dsi->dev, "failed to unprepare panel: %d\n", err);
-
- err = drm_panel_disable(&pinfo->base);
- if (err < 0)
- dev_err(&dsi->dev, "failed to disable panel: %d\n", err);
-
err = mipi_dsi_detach(dsi);
if (err < 0)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", err);
@@ -671,14 +663,6 @@ static void nt36672a_panel_remove(struct mipi_dsi_device *dsi)
drm_panel_remove(&pinfo->base);
}
-static void nt36672a_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct nt36672a_panel *pinfo = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_disable(&pinfo->base);
- drm_panel_unprepare(&pinfo->base);
-}
-
static const struct of_device_id tianma_fhd_video_of_match[] = {
{ .compatible = "tianma,fhd-video", .data = &tianma_fhd_video_panel_desc },
{ },
@@ -692,7 +676,6 @@ static struct mipi_dsi_driver nt36672a_panel_driver = {
},
.probe = nt36672a_panel_probe,
.remove = nt36672a_panel_remove,
- .shutdown = nt36672a_panel_shutdown,
};
module_mipi_dsi_driver(nt36672a_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
The acx565akm seems to do some unique stuff with the "enabled"
state. Specifically:
1. It seems to detect the enabled state based on how the bootloader
left the panel.
2. It uses the enabled state to prevent certain sysfs files from
accessing a disabled panel.
We'll leave the "enabled" state tracking for this. However, we can at
least get rid of the double-check when trying to disable.
Cc: Laurent Pinchart <[email protected]>
Cc: Sebastian Reichel <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-sony-acx565akm.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-sony-acx565akm.c b/drivers/gpu/drm/panel/panel-sony-acx565akm.c
index 3d6a286056a0..a9a545a56404 100644
--- a/drivers/gpu/drm/panel/panel-sony-acx565akm.c
+++ b/drivers/gpu/drm/panel/panel-sony-acx565akm.c
@@ -454,9 +454,6 @@ static int acx565akm_power_on(struct acx565akm_panel *lcd)
static void acx565akm_power_off(struct acx565akm_panel *lcd)
{
- if (!lcd->enabled)
- return;
-
acx565akm_set_display_state(lcd, 0);
acx565akm_set_sleep_mode(lcd, 1);
lcd->enabled = false;
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by TI OMAP boards. The TI OMAP driver
appears to be correctly calling drm_atomic_helper_shutdown() so we can
remove the calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Laurent Pinchart <[email protected]>
Cc: Sebastian Reichel <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-sony-acx565akm.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-sony-acx565akm.c b/drivers/gpu/drm/panel/panel-sony-acx565akm.c
index a9a545a56404..73ba93ff00fe 100644
--- a/drivers/gpu/drm/panel/panel-sony-acx565akm.c
+++ b/drivers/gpu/drm/panel/panel-sony-acx565akm.c
@@ -652,9 +652,6 @@ static void acx565akm_remove(struct spi_device *spi)
if (lcd->has_bc)
acx565akm_backlight_cleanup(lcd);
-
- drm_panel_disable(&lcd->panel);
- drm_panel_unprepare(&lcd->panel);
}
static const struct of_device_id acx565akm_of_match[] = {
--
2.45.0.rc1.225.g2a3ae87e7f-goog
Now that most panels have been updated not to track/double-check their
prepared/enabled state update the TODO with next steps.
Signed-off-by: Douglas Anderson <[email protected]>
---
(no changes since v1)
Documentation/gpu/todo.rst | 47 +++++++++++++++++++++-----------------
1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index e2a0585915b3..4063bc45bbd3 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -469,30 +469,35 @@ Contact: Thomas Zimmermann <[email protected]>
Level: Starter
-Clean up checks for already prepared/enabled in panels
-------------------------------------------------------
-
-In a whole pile of panel drivers, we have code to make the
-prepare/unprepare/enable/disable callbacks behave as no-ops if they've already
-been called. To get some idea of the duplicated code, try::
-
- git grep 'if.*>prepared' -- drivers/gpu/drm/panel
- git grep 'if.*>enabled' -- drivers/gpu/drm/panel
-
-In the patch ("drm/panel: Check for already prepared/enabled in drm_panel")
-we've moved this check to the core. Now we can most definitely remove the
-check from the individual panels and save a pile of code.
-
-In adition to removing the check from the individual panels, it is believed
-that even the core shouldn't need this check and that should be considered
-an error if other code ever relies on this check. The check in the core
-currently prints a warning whenever something is relying on this check with
-dev_warn(). After a little while, we likely want to promote this to a
-WARN(1) to help encourage folks not to rely on this behavior.
+Remove disable/unprepare in remove/shutdown in panel-simple and panel-edp
+-------------------------------------------------------------------------
+
+As of commit d2aacaf07395 ("drm/panel: Check for already prepared/enabled in
+drm_panel"), we have a check in the drm_panel core to make sure nobody
+double-calls prepare/enable/disable/unprepare. Eventually that should probably
+be turned into a WARN_ON() or somehow made louder, but right now we actually
+expect it to trigger and so we don't want it to be too loud.
+
+Specifically, that warning will trigger for panel-edp and panel-simple at
+shutdown time because those panels hardcode a call to drm_panel_disable()
+and drm_panel_unprepare() at shutdown and remove time that they call regardless
+of panel state. On systems with a properly coded DRM modeset driver that
+calls drm_atomic_helper_shutdown() this is pretty much guaranteed to cause
+the warning to fire.
+
+Unfortunately we can't safely remove the calls in panel-edp and panel-simple
+until we're sure that all DRM modeset drivers that are used with those panels
+properly call drm_atomic_helper_shutdown(). This TODO item is to validate
+that all DRM modeset drivers used with panel-edp and panel-simple properly
+call drm_atomic_helper_shutdown() and then remove the calls to
+disable/unprepare from those panels. Alternatively, this TODO item could be
+removed by convincing stakeholders that those calls are fine and downgrading
+the error message in drm_panel_disable() / drm_panel_unprepare() to a
+debug-level message.
Contact: Douglas Anderson <[email protected]>
-Level: Starter/Intermediate
+Level: Intermediate
Core refactorings
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Stefan Mavrodiev <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
.../drm/panel/panel-olimex-lcd-olinuxino.c | 41 -------------------
1 file changed, 41 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c b/drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c
index 4819ada69482..8a687d3ba236 100644
--- a/drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c
+++ b/drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c
@@ -64,9 +64,6 @@ struct lcd_olinuxino {
struct i2c_client *client;
struct mutex mutex;
- bool prepared;
- bool enabled;
-
struct regulator *supply;
struct gpio_desc *enable_gpio;
@@ -78,30 +75,13 @@ static inline struct lcd_olinuxino *to_lcd_olinuxino(struct drm_panel *panel)
return container_of(panel, struct lcd_olinuxino, panel);
}
-static int lcd_olinuxino_disable(struct drm_panel *panel)
-{
- struct lcd_olinuxino *lcd = to_lcd_olinuxino(panel);
-
- if (!lcd->enabled)
- return 0;
-
- lcd->enabled = false;
-
- return 0;
-}
-
static int lcd_olinuxino_unprepare(struct drm_panel *panel)
{
struct lcd_olinuxino *lcd = to_lcd_olinuxino(panel);
- if (!lcd->prepared)
- return 0;
-
gpiod_set_value_cansleep(lcd->enable_gpio, 0);
regulator_disable(lcd->supply);
- lcd->prepared = false;
-
return 0;
}
@@ -110,27 +90,11 @@ static int lcd_olinuxino_prepare(struct drm_panel *panel)
struct lcd_olinuxino *lcd = to_lcd_olinuxino(panel);
int ret;
- if (lcd->prepared)
- return 0;
-
ret = regulator_enable(lcd->supply);
if (ret < 0)
return ret;
gpiod_set_value_cansleep(lcd->enable_gpio, 1);
- lcd->prepared = true;
-
- return 0;
-}
-
-static int lcd_olinuxino_enable(struct drm_panel *panel)
-{
- struct lcd_olinuxino *lcd = to_lcd_olinuxino(panel);
-
- if (lcd->enabled)
- return 0;
-
- lcd->enabled = true;
return 0;
}
@@ -195,10 +159,8 @@ static int lcd_olinuxino_get_modes(struct drm_panel *panel,
}
static const struct drm_panel_funcs lcd_olinuxino_funcs = {
- .disable = lcd_olinuxino_disable,
.unprepare = lcd_olinuxino_unprepare,
.prepare = lcd_olinuxino_prepare,
- .enable = lcd_olinuxino_enable,
.get_modes = lcd_olinuxino_get_modes,
};
@@ -264,9 +226,6 @@ static int lcd_olinuxino_probe(struct i2c_client *client)
lcd->eeprom.num_modes = 4;
}
- lcd->enabled = false;
- lcd->prepared = false;
-
lcd->supply = devm_regulator_get(dev, "power");
if (IS_ERR(lcd->supply))
return PTR_ERR(lcd->supply);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by Qualcomm boards. The Qualcomm driver
appears to be correctly calling drm_atomic_helper_shutdown() so we can
remove the calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Vinay Simha BN <[email protected]>
Cc: Sumit Semwal <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
index 4ddddee6fa1e..b1ce186de261 100644
--- a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
+++ b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
@@ -452,10 +452,6 @@ static void jdi_panel_remove(struct mipi_dsi_device *dsi)
struct jdi_panel *jdi = mipi_dsi_get_drvdata(dsi);
int ret;
- ret = drm_panel_disable(&jdi->base);
- if (ret < 0)
- dev_err(&dsi->dev, "failed to disable panel: %d\n", ret);
-
ret = mipi_dsi_detach(dsi);
if (ret < 0)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n",
@@ -464,13 +460,6 @@ static void jdi_panel_remove(struct mipi_dsi_device *dsi)
jdi_panel_del(jdi);
}
-static void jdi_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct jdi_panel *jdi = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_disable(&jdi->base);
-}
-
static struct mipi_dsi_driver jdi_panel_driver = {
.driver = {
.name = "panel-jdi-lt070me05000",
@@ -478,7 +467,6 @@ static struct mipi_dsi_driver jdi_panel_driver = {
},
.probe = jdi_panel_probe,
.remove = jdi_panel_remove,
- .shutdown = jdi_panel_shutdown,
};
module_mipi_dsi_driver(jdi_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Werner Johansson <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
.../drm/panel/panel-panasonic-vvx10f034n00.c | 35 +------------------
1 file changed, 1 insertion(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c b/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c
index 8ba6d8287938..822ca2f971eb 100644
--- a/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c
+++ b/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c
@@ -32,9 +32,6 @@ struct wuxga_nt_panel {
struct regulator *supply;
- bool prepared;
- bool enabled;
-
ktime_t earliest_wake;
const struct drm_display_mode *mode;
@@ -53,28 +50,16 @@ static int wuxga_nt_panel_on(struct wuxga_nt_panel *wuxga_nt)
static int wuxga_nt_panel_disable(struct drm_panel *panel)
{
struct wuxga_nt_panel *wuxga_nt = to_wuxga_nt_panel(panel);
- int mipi_ret, bl_ret = 0;
-
- if (!wuxga_nt->enabled)
- return 0;
-
- mipi_ret = mipi_dsi_shutdown_peripheral(wuxga_nt->dsi);
-
- wuxga_nt->enabled = false;
- return mipi_ret ? mipi_ret : bl_ret;
+ return mipi_dsi_shutdown_peripheral(wuxga_nt->dsi);
}
static int wuxga_nt_panel_unprepare(struct drm_panel *panel)
{
struct wuxga_nt_panel *wuxga_nt = to_wuxga_nt_panel(panel);
- if (!wuxga_nt->prepared)
- return 0;
-
regulator_disable(wuxga_nt->supply);
wuxga_nt->earliest_wake = ktime_add_ms(ktime_get_real(), MIN_POFF_MS);
- wuxga_nt->prepared = false;
return 0;
}
@@ -85,9 +70,6 @@ static int wuxga_nt_panel_prepare(struct drm_panel *panel)
int ret;
s64 enablewait;
- if (wuxga_nt->prepared)
- return 0;
-
/*
* If the user re-enabled the panel before the required off-time then
* we need to wait the remaining period before re-enabling regulator
@@ -117,8 +99,6 @@ static int wuxga_nt_panel_prepare(struct drm_panel *panel)
goto poweroff;
}
- wuxga_nt->prepared = true;
-
return 0;
poweroff:
@@ -127,18 +107,6 @@ static int wuxga_nt_panel_prepare(struct drm_panel *panel)
return ret;
}
-static int wuxga_nt_panel_enable(struct drm_panel *panel)
-{
- struct wuxga_nt_panel *wuxga_nt = to_wuxga_nt_panel(panel);
-
- if (wuxga_nt->enabled)
- return 0;
-
- wuxga_nt->enabled = true;
-
- return 0;
-}
-
static const struct drm_display_mode default_mode = {
.clock = 164402,
.hdisplay = 1920,
@@ -178,7 +146,6 @@ static const struct drm_panel_funcs wuxga_nt_panel_funcs = {
.disable = wuxga_nt_panel_disable,
.unprepare = wuxga_nt_panel_unprepare,
.prepare = wuxga_nt_panel_prepare,
- .enable = wuxga_nt_panel_enable,
.get_modes = wuxga_nt_panel_get_modes,
};
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
Unfortunately, grepping mainline for this panel's compatible string
shows no hits, so we can't be 100% sure if the DRM modeset driver used
with this panel has been fixed. If it is found that the DRM modeset
driver hasn't been fixed then this patch could be temporarily reverted
until it is.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Werner Johansson <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c b/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c
index 822ca2f971eb..d1c5c9bc3c56 100644
--- a/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c
+++ b/drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c
@@ -222,10 +222,6 @@ static void wuxga_nt_panel_remove(struct mipi_dsi_device *dsi)
struct wuxga_nt_panel *wuxga_nt = mipi_dsi_get_drvdata(dsi);
int ret;
- ret = drm_panel_disable(&wuxga_nt->base);
- if (ret < 0)
- dev_err(&dsi->dev, "failed to disable panel: %d\n", ret);
-
ret = mipi_dsi_detach(dsi);
if (ret < 0)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", ret);
@@ -233,13 +229,6 @@ static void wuxga_nt_panel_remove(struct mipi_dsi_device *dsi)
wuxga_nt_panel_del(wuxga_nt);
}
-static void wuxga_nt_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct wuxga_nt_panel *wuxga_nt = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_disable(&wuxga_nt->base);
-}
-
static struct mipi_dsi_driver wuxga_nt_panel_driver = {
.driver = {
.name = "panel-panasonic-vvx10f034n00",
@@ -247,7 +236,6 @@ static struct mipi_dsi_driver wuxga_nt_panel_driver = {
},
.probe = wuxga_nt_panel_probe,
.remove = wuxga_nt_panel_remove,
- .shutdown = wuxga_nt_panel_shutdown,
};
module_mipi_dsi_driver(wuxga_nt_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by Qualcomm boards. The Qualcomm driver
appears to be correctly calling drm_atomic_helper_shutdown() so we can
remove the calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Werner Johansson <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
index c86337954ad7..729cbb0d8403 100644
--- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
@@ -267,10 +267,6 @@ static void sharp_nt_panel_remove(struct mipi_dsi_device *dsi)
struct sharp_nt_panel *sharp_nt = mipi_dsi_get_drvdata(dsi);
int ret;
- ret = drm_panel_disable(&sharp_nt->base);
- if (ret < 0)
- dev_err(&dsi->dev, "failed to disable panel: %d\n", ret);
-
ret = mipi_dsi_detach(dsi);
if (ret < 0)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", ret);
@@ -278,13 +274,6 @@ static void sharp_nt_panel_remove(struct mipi_dsi_device *dsi)
sharp_nt_panel_del(sharp_nt);
}
-static void sharp_nt_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct sharp_nt_panel *sharp_nt = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_disable(&sharp_nt->base);
-}
-
static const struct of_device_id sharp_nt_of_match[] = {
{ .compatible = "sharp,ls043t1le01-qhd", },
{ }
@@ -298,7 +287,6 @@ static struct mipi_dsi_driver sharp_nt_panel_driver = {
},
.probe = sharp_nt_panel_probe,
.remove = sharp_nt_panel_remove,
- .shutdown = sharp_nt_panel_shutdown,
};
module_mipi_dsi_driver(sharp_nt_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by IMX boards. The IMX driver appears to be
correctly calling drm_atomic_helper_shutdown() so we can remove the
calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Robert Chiras <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-raydium-rm67191.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-raydium-rm67191.c b/drivers/gpu/drm/panel/panel-raydium-rm67191.c
index fa9bf89d3bb5..b2029e035635 100644
--- a/drivers/gpu/drm/panel/panel-raydium-rm67191.c
+++ b/drivers/gpu/drm/panel/panel-raydium-rm67191.c
@@ -612,14 +612,6 @@ static void rad_panel_remove(struct mipi_dsi_device *dsi)
drm_panel_remove(&rad->panel);
}
-static void rad_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct rad_panel *rad = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_disable(&rad->panel);
- drm_panel_unprepare(&rad->panel);
-}
-
static const struct of_device_id rad_of_match[] = {
{ .compatible = "raydium,rm67191", },
{ /* sentinel */ }
@@ -633,7 +625,6 @@ static struct mipi_dsi_driver rad_panel_driver = {
},
.probe = rad_panel_probe,
.remove = rad_panel_remove,
- .shutdown = rad_panel_shutdown,
};
module_mipi_dsi_driver(rad_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
Hi Douglas Anderson,
Poco F1 is one of the main user for this panel. I tested the patch in my Poco F1 running postmarketOS. It works fine. Thank you. The panel itself requires other extra fixes to work properly which I intend to upstream in the upcoming weeks. But your patch doesn't break anything and works as expected. So.
Tested-by: Joel Selvaraj <[email protected]>
Regards
Joel Selvaraj
________________________________________
From: Douglas Anderson <[email protected]>
Sent: 04 May 2024 03:03
To: [email protected]; Maxime Ripard
Cc: Linus Walleij; Chris Morgan; Yuran Pereira; Neil Armstrong; Douglas Anderson; Sumit Semwal; Benni Steini; Joel Selvaraj; Marijn Suijten; Daniel Vetter; David Airlie; Jessica Zhang; Maarten Lankhorst; Sam Ravnborg; Thomas Zimmermann; [email protected]
Subject: [RFT PATCH v2 19/48] drm/panel: novatek-nt36672a: Don't call unprepare+disable at shutdown/remove
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by Qualcomm boards. The Qualcomm driver
appears to be correctly calling drm_atomic_helper_shutdown() so we can
remove the calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Sumit Semwal <[email protected]>
Cc: Benni Steini <[email protected]>
Cc: Joel Selvaraj <[email protected]>
Cc: Marijn Suijten <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-novatek-nt36672a.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
index 35aace79613a..c2abd20e0734 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
@@ -656,14 +656,6 @@ static void nt36672a_panel_remove(struct mipi_dsi_device *dsi)
struct nt36672a_panel *pinfo = mipi_dsi_get_drvdata(dsi);
int err;
- err = drm_panel_unprepare(&pinfo->base);
- if (err < 0)
- dev_err(&dsi->dev, "failed to unprepare panel: %d\n", err);
-
- err = drm_panel_disable(&pinfo->base);
- if (err < 0)
- dev_err(&dsi->dev, "failed to disable panel: %d\n", err);
-
err = mipi_dsi_detach(dsi);
if (err < 0)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", err);
@@ -671,14 +663,6 @@ static void nt36672a_panel_remove(struct mipi_dsi_device *dsi)
drm_panel_remove(&pinfo->base);
}
-static void nt36672a_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct nt36672a_panel *pinfo = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_disable(&pinfo->base);
- drm_panel_unprepare(&pinfo->base);
-}
-
static const struct of_device_id tianma_fhd_video_of_match[] = {
{ .compatible = "tianma,fhd-video", .data = &tianma_fhd_video_panel_desc },
{ },
@@ -692,7 +676,6 @@ static struct mipi_dsi_driver nt36672a_panel_driver = {
},
.probe = nt36672a_panel_probe,
.remove = nt36672a_panel_remove,
- .shutdown = nt36672a_panel_shutdown,
};
module_mipi_dsi_driver(nt36672a_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
Hi Douglas Anderson,
Poco F1 is one of the main user for this panel. I tested the patch in my Poco F1 running postmarketOS. It works fine. Thank you. The panel itself requires other extra fixes to work properly which I intend to upstream in the upcoming weeks. But your patch doesn't break anything and works as expected. So.
Tested-by: Joel Selvaraj <[email protected]>
Regards
Joel Selvaraj
________________________________________
From: Douglas Anderson <[email protected]>
Sent: 04 May 2024 03:02
To: [email protected]; Maxime Ripard
Cc: Linus Walleij; Chris Morgan; Yuran Pereira; Neil Armstrong; Douglas Anderson; Sumit Semwal; Benni Steini; Joel Selvaraj; Marijn Suijten; Daniel Vetter; David Airlie; Jessica Zhang; Maarten Lankhorst; Sam Ravnborg; Thomas Zimmermann; [email protected]
Subject: [RFT PATCH v2 18/48] drm/panel: novatek-nt36672a: Stop tracking prepared
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
Cc: Sumit Semwal <[email protected]>
Cc: Benni Steini <[email protected]>
Cc: Joel Selvaraj <[email protected]>
Cc: Marijn Suijten <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-novatek-nt36672a.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
index 3886372415c2..35aace79613a 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
@@ -72,8 +72,6 @@ struct nt36672a_panel {
struct regulator_bulk_data supplies[ARRAY_SIZE(nt36672a_regulator_names)];
struct gpio_desc *reset_gpio;
-
- bool prepared;
};
static inline struct nt36672a_panel *to_nt36672a_panel(struct drm_panel *panel)
@@ -119,9 +117,6 @@ static int nt36672a_panel_unprepare(struct drm_panel *panel)
struct nt36672a_panel *pinfo = to_nt36672a_panel(panel);
int ret;
- if (!pinfo->prepared)
- return 0;
-
/* send off cmds */
ret = nt36672a_send_cmds(panel, pinfo->desc->off_cmds,
pinfo->desc->num_off_cmds);
@@ -147,8 +142,6 @@ static int nt36672a_panel_unprepare(struct drm_panel *panel)
if (ret < 0)
dev_err(panel->dev, "power_off failed ret = %d\n", ret);
- pinfo->prepared = false;
-
return ret;
}
@@ -179,9 +172,6 @@ static int nt36672a_panel_prepare(struct drm_panel *panel)
struct nt36672a_panel *pinfo = to_nt36672a_panel(panel);
int err;
- if (pinfo->prepared)
- return 0;
-
err = nt36672a_panel_power_on(pinfo);
if (err < 0)
goto poweroff;
@@ -221,8 +211,6 @@ static int nt36672a_panel_prepare(struct drm_panel *panel)
msleep(120);
- pinfo->prepared = true;
-
return 0;
poweroff:
--
2.45.0.rc1.225.g2a3ae87e7f-goog
Sorry for the long lines. Had to use a different email client and ended messing it up.
Regards
Joel Selvaraj
________________________________________
From: Joel Selvaraj <[email protected]>
Sent: 05 May 2024 05:28
To: Douglas Anderson; [email protected]; Maxime Ripard
Cc: Linus Walleij; Chris Morgan; Yuran Pereira; Neil Armstrong; Sumit Semwal; Benni Steini; Marijn Suijten; Daniel Vetter; David Airlie; Jessica Zhang; Maarten Lankhorst; Sam Ravnborg; Thomas Zimmermann; [email protected]
Subject: Re: [RFT PATCH v2 19/48] drm/panel: novatek-nt36672a: Don't call unprepare+disable at shutdown/remove
Hi Douglas Anderson,
Poco F1 is one of the main user for this panel. I tested the patch in my Poco F1 running postmarketOS. It works fine. Thank you. The panel itself requires other extra fixes to work properly which I intend to upstream in the upcoming weeks. But your patch doesn't break anything and works as expected. So.
Tested-by: Joel Selvaraj <[email protected]>
Regards
Joel Selvaraj
________________________________________
From: Douglas Anderson <[email protected]>
Sent: 04 May 2024 03:03
To: [email protected]; Maxime Ripard
Cc: Linus Walleij; Chris Morgan; Yuran Pereira; Neil Armstrong; Douglas Anderson; Sumit Semwal; Benni Steini; Joel Selvaraj; Marijn Suijten; Daniel Vetter; David Airlie; Jessica Zhang; Maarten Lankhorst; Sam Ravnborg; Thomas Zimmermann; [email protected]
Subject: [RFT PATCH v2 19/48] drm/panel: novatek-nt36672a: Don't call unprepare+disable at shutdown/remove
It's the responsibility of a correctly written DRM modeset driver to
call drm_atomic_helper_shutdown() at shutdown time and that should be
disabling / unpreparing the panel if needed. Panel drivers shouldn't
be calling these functions themselves.
A recent effort was made to fix as many DRM modeset drivers as
possible [1] [2] [3] and most drivers are fixed now.
A grep through mainline for compatible strings used by this driver
indicates that it is used by Qualcomm boards. The Qualcomm driver
appears to be correctly calling drm_atomic_helper_shutdown() so we can
remove the calls.
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Cc: Sumit Semwal <[email protected]>
Cc: Benni Steini <[email protected]>
Cc: Joel Selvaraj <[email protected]>
Cc: Marijn Suijten <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Only handle 1 panel per patch.
- Split removal of prepared/enabled from handling of remove/shutdown.
drivers/gpu/drm/panel/panel-novatek-nt36672a.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
index 35aace79613a..c2abd20e0734 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
@@ -656,14 +656,6 @@ static void nt36672a_panel_remove(struct mipi_dsi_device *dsi)
struct nt36672a_panel *pinfo = mipi_dsi_get_drvdata(dsi);
int err;
- err = drm_panel_unprepare(&pinfo->base);
- if (err < 0)
- dev_err(&dsi->dev, "failed to unprepare panel: %d\n", err);
-
- err = drm_panel_disable(&pinfo->base);
- if (err < 0)
- dev_err(&dsi->dev, "failed to disable panel: %d\n", err);
-
err = mipi_dsi_detach(dsi);
if (err < 0)
dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", err);
@@ -671,14 +663,6 @@ static void nt36672a_panel_remove(struct mipi_dsi_device *dsi)
drm_panel_remove(&pinfo->base);
}
-static void nt36672a_panel_shutdown(struct mipi_dsi_device *dsi)
-{
- struct nt36672a_panel *pinfo = mipi_dsi_get_drvdata(dsi);
-
- drm_panel_disable(&pinfo->base);
- drm_panel_unprepare(&pinfo->base);
-}
-
static const struct of_device_id tianma_fhd_video_of_match[] = {
{ .compatible = "tianma,fhd-video", .data = &tianma_fhd_video_panel_desc },
{ },
@@ -692,7 +676,6 @@ static struct mipi_dsi_driver nt36672a_panel_driver = {
},
.probe = nt36672a_panel_probe,
.remove = nt36672a_panel_remove,
- .shutdown = nt36672a_panel_shutdown,
};
module_mipi_dsi_driver(nt36672a_panel_driver);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
On Fri, May 3, 2024 at 11:38 PM Douglas Anderson <dianders@chromiumorg> wrote:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
>
> The acx565akm seems to do some unique stuff with the "enabled"
> state. Specifically:
> 1. It seems to detect the enabled state based on how the bootloader
> left the panel.
> 2. It uses the enabled state to prevent certain sysfs files from
> accessing a disabled panel.
>
> We'll leave the "enabled" state tracking for this. However, we can at
> least get rid of the double-check when trying to disable.
>
> Cc: Laurent Pinchart <[email protected]>
> Cc: Sebastian Reichel <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Fri, May 3, 2024 at 11:38 PM Douglas Anderson <dianders@chromiumorg> wrote:
> It's the responsibility of a correctly written DRM modeset driver to
> call drm_atomic_helper_shutdown() at shutdown time and that should be
> disabling / unpreparing the panel if needed. Panel drivers shouldn't
> be calling these functions themselves.
>
> A recent effort was made to fix as many DRM modeset drivers as
> possible [1] [2] [3] and most drivers are fixed now.
>
> A grep through mainline for compatible strings used by this driver
> indicates that it is used by TI OMAP boards. The TI OMAP driver
> appears to be correctly calling drm_atomic_helper_shutdown() so we can
> remove the calls.
>
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
> [3] https://lore.kernel.org/r/[email protected]
>
> Cc: Laurent Pinchart <[email protected]>
> Cc: Sebastian Reichel <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
On Fri, May 3, 2024 at 11:38 PM Douglas Anderson <dianders@chromiumorg> wrote:
> It's the responsibility of a correctly written DRM modeset driver to
> call drm_atomic_helper_shutdown() at shutdown time and that should be
> disabling / unpreparing the panel if needed. Panel drivers shouldn't
> be calling these functions themselves.
>
> A recent effort was made to fix as many DRM modeset drivers as
> possible [1] [2] [3] and most drivers are fixed now.
>
> A grep through mainline for compatible strings used by this driver
> indicates that it is used by TI OMAP boards. The TI OMAP driver
> appears to be correctly calling drm_atomic_helper_shutdown() so we can
> remove the calls.
>
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
> [3] https://lore.kernel.org/r/[email protected]
>
> Cc: Laurent Pinchart <[email protected]>
> Cc: Sebastian Reichel <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Fri, May 3, 2024 at 11:36 PM Douglas Anderson <dianders@chromiumorg> wrote:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
>
> This series attempts to do just that. While the original grep, AKA:
> git grep 'if.*>prepared' -- drivers/gpu/drm/panel
> git grep 'if.*>enabled' -- drivers/gpu/drm/panel
> ...still produces a few hits after my series, they are _mostly_ all
> gone. The ones that are left are less trivial to fix.
>
> One of the main reasons that many panels probably needed to store and
> double-check their prepared/enabled appears to have been to handle
> shutdown and/or remove. Panels drivers often wanted to force the power
> off for panels in these cases and this was a good reason for the
> double-check.
>
> In response to my V1 series [1] we had much discussion of what to
> do. The conclusion was that as long as DRM modeset drivers properly
> called drm_atomic_helper_shutdown() that we should be able to remove
> the explicit shutdown/remove handling in the panel drivers. Most of
> the patches to improve DRM modeset drivers [2] [3] [4] have now
> landed.
>
> In contrast to my V1 series, I broke the V2 series up a lot
> more. Since a few of the panel drivers in V1 already landed, we had
> fewer total drivers and so we could devote a patch to each panel.
> Also, since we were now relying on DRM modeset drivers I felt like we
> should split the patches for each panel into two: one that's
> definitely safe and one that could be reverted if we found a
> problematic DRM modeset driver that we couldn't fix.
>
> Sorry for the large number of patches. I've set things to mostly just
> CC people on the cover letter and the patches that are relevant to
> them. I've tried to CC people on the whole series that have shown
> interest in this TODO item.
>
> As patches in this series are reviewed and/or tested they could be
> landed. There's really no ordering requirement for the series unless
> patches touch the same driver.
>
> NOTE: this touches _a lot_ of drivers, is repetitive, and is not
> really possible to generate automatically. That means it's entirely
> possible that my eyes glazed over and I did something wrong. Please
> double-check me and don't assume that I got everything perfect, though
> I did my best. I have at least confirmed that "allmodconfig" for arm64
> doesn't fall on its face with this series. I haven't done a ton of
> other testing.
>
> [1] https://lore.kernel.org/r/20230804140605.RFC.4.I930069a32baab6faf46d6b234f89613b5cec0f14@changeid
> [2] https://lore.kernel.org/r/[email protected]
> [3] https://lore.kernel.org/r/[email protected]
> [4] https://lore.kernel.org/r/[email protected]
This is the right thing to do, thanks for looking into this!
As for the behaviour of .remove() I doubt whether in many cases
the original driver authors have even tested this themselves.
I would say we should just apply the series as soon as it's non-RFC
after the next merge window and see what happens. I doubt it
will cause much trouble.
The series:
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Mon, May 06, 2024 at 08:52:39AM GMT, Linus Walleij wrote:
> On Fri, May 3, 2024 at 11:36 PM Douglas Anderson <[email protected]> wrote:
>
> > As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> > prepared/enabled in drm_panel"), we want to remove needless code from
> > panel drivers that was storing and double-checking the
> > prepared/enabled state. Even if someone was relying on the
> > double-check before, that double-check is now in the core and not
> > needed in individual drivers.
> >
> > This series attempts to do just that. While the original grep, AKA:
> > git grep 'if.*>prepared' -- drivers/gpu/drm/panel
> > git grep 'if.*>enabled' -- drivers/gpu/drm/panel
> > ...still produces a few hits after my series, they are _mostly_ all
> > gone. The ones that are left are less trivial to fix.
> >
> > One of the main reasons that many panels probably needed to store and
> > double-check their prepared/enabled appears to have been to handle
> > shutdown and/or remove. Panels drivers often wanted to force the power
> > off for panels in these cases and this was a good reason for the
> > double-check.
> >
> > In response to my V1 series [1] we had much discussion of what to
> > do. The conclusion was that as long as DRM modeset drivers properly
> > called drm_atomic_helper_shutdown() that we should be able to remove
> > the explicit shutdown/remove handling in the panel drivers. Most of
> > the patches to improve DRM modeset drivers [2] [3] [4] have now
> > landed.
> >
> > In contrast to my V1 series, I broke the V2 series up a lot
> > more. Since a few of the panel drivers in V1 already landed, we had
> > fewer total drivers and so we could devote a patch to each panel.
> > Also, since we were now relying on DRM modeset drivers I felt like we
> > should split the patches for each panel into two: one that's
> > definitely safe and one that could be reverted if we found a
> > problematic DRM modeset driver that we couldn't fix.
> >
> > Sorry for the large number of patches. I've set things to mostly just
> > CC people on the cover letter and the patches that are relevant to
> > them. I've tried to CC people on the whole series that have shown
> > interest in this TODO item.
> >
> > As patches in this series are reviewed and/or tested they could be
> > landed. There's really no ordering requirement for the series unless
> > patches touch the same driver.
> >
> > NOTE: this touches _a lot_ of drivers, is repetitive, and is not
> > really possible to generate automatically. That means it's entirely
> > possible that my eyes glazed over and I did something wrong. Please
> > double-check me and don't assume that I got everything perfect, though
> > I did my best. I have at least confirmed that "allmodconfig" for arm64
> > doesn't fall on its face with this series. I haven't done a ton of
> > other testing.
> >
> > [1] https://lore.kernel.org/r/20230804140605.RFC.4.I930069a32baab6faf46d6b234f89613b5cec0f14@changeid
> > [2] https://lore.kernel.org/r/[email protected]
> > [3] https://lore.kernel.org/r/[email protected]
> > [4] https://lore.kernel.org/r/[email protected]
>
> This is the right thing to do, thanks for looking into this!
>
> As for the behaviour of .remove() I doubt whether in many cases
> the original driver authors have even tested this themselves.
> I would say we should just apply the series as soon as it's non-RFC
> after the next merge window and see what happens. I doubt it
> will cause much trouble.
In the case of st7703 driver, yes tested, and proper shutdown of the panel is
necessary, because lack of it can lead to otherwise inexplainable blinking of
the entire screen, when the panel is quickly powered up and re-initialized again
(eg. as happens when bootloader has display support). Blinking then only ever
stops if the panel is left completely powered off for several minutes.
There's a note about this in the controller datasheet, that proper power off
is needed to enable dicharge of the panel.
Kind regards,
o.
> The series:
> Acked-by: Linus Walleij <[email protected]>
>
> Yours,
> Linus Walleij
On Fri, 3 May 2024 14:32:41 -0700, Douglas Anderson wrote:
>
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
>
> [ ... ]
Acked-by: Maxime Ripard <[email protected]>
Thanks!
Maxime
Hi Douglas,
On 5/3/24 11:32 PM, Douglas Anderson wrote:
> [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
>
> Cc: "Heiko Stübner" <[email protected]>
> Cc: Quentin Schulz <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Quentin Schulz <[email protected]>
Thanks!
Quentin
Hi Douglas,
On 5/3/24 11:32 PM, Douglas Anderson wrote:
> [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> It's the responsibility of a correctly written DRM modeset driver to
> call drm_atomic_helper_shutdown() at shutdown time and that should be
> disabling / unpreparing the panel if needed. Panel drivers shouldn't
> be calling these functions themselves.
>
> A recent effort was made to fix as many DRM modeset drivers as
> possible [1] [2] [3] and most drivers are fixed now.
>
> Unfortunately, grepping mainline for this panel's compatible string
> shows no hits, so we can't be 100% sure if the DRM modeset driver used
> with this panel has been fixed. If it is found that the DRM modeset
> driver hasn't been fixed then this patch could be temporarily reverted
> until it is.
>
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
> [3] https://lore.kernel.org/r/[email protected]
>
> Cc: "Heiko Stübner" <[email protected]>
> Cc: Quentin Schulz <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
I get:
"""
[ 27.239362] ------------[ cut here ]------------
[ 27.244549] refcount_t: addition on 0; use-after-free.
[ 27.250308] WARNING: CPU: 4 PID: 1 at lib/refcount.c:25
refcount_warn_saturate+0x120/0x144
[ 27.259556] Modules linked in: snd_soc_simple_card crct10dif_ce
snd_soc_simple_card_utils fuse [last unloaded: panel_leadtek_ltk050h3146w]
[ 27.273470] CPU: 4 PID: 1 Comm: systemd-shutdow Not tainted
6.9.0-rc7-00002-g4a8eaebfcfde-dirty #63
[ 27.283584] Hardware name: Theobroma Systems RK3399-Q7 SoM on Haikou
devkit with Video Demo adapter (DT)
[ 27.294180] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS
BTYPE=--)
[ 27.301962] pc : refcount_warn_saturate+0x120/0x144
[ 27.307411] lr : refcount_warn_saturate+0x120/0x144
[ 27.312860] sp : ffff800081babb10
[ 27.316559] x29: ffff800081babb10 x28: ffff000000640000 x27:
0000000000000000
[ 27.324539] x26: ffff8000814847f8 x25: 0000000000000001 x24:
ffff800081adb028
[ 27.332519] x23: ffff000000e13090 x22: ffff800081b3c280 x21:
ffff0000006c8680
[ 27.340489] x20: ffff0000006c8680 x19: ffff00000a6f8000 x18:
0000000000000006
[ 27.348468] x17: 000000040044ffff x16: 00500074b5503510 x15:
ffff800081bab580
[ 27.356447] x14: 0000000000000000 x13: ffff8000819944d0 x12:
0000000000000a2f
[ 27.364426] x11: 0000000000000365 x10: ffff8000819ec4d0 x9 :
ffff8000819944d0
[ 27.372396] x8 : 00000000ffffefff x7 : ffff8000819ec4d0 x6 :
80000000fffff000
[ 27.380375] x5 : 0000000000000366 x4 : 0000000000000000 x3 :
0000000000000000
[ 27.388353] x2 : 0000000000000000 x1 : 0000000000000000 x0 :
ffff000000640000
[ 27.396332] Call trace:
[ 27.399061] refcount_warn_saturate+0x120/0x144
[ 27.404122] drm_dev_get+0x78/0x7c
[ 27.407924] drm_atomic_state_init+0x78/0xd0
[ 27.412695] drm_atomic_state_alloc+0x6c/0x9c
[ 27.417563] drm_atomic_helper_disable_all+0x20/0x214
[ 27.423208] drm_atomic_helper_shutdown+0xa4/0x148
[ 27.428560] rockchip_drm_platform_shutdown+0x18/0x28
[ 27.434207] platform_shutdown+0x24/0x34
[ 27.438589] device_shutdown+0x150/0x258
[ 27.442973] kernel_power_off+0x38/0x7c
[ 27.447260] __do_sys_reboot+0x204/0x24c
[ 27.451643] __arm64_sys_reboot+0x24/0x30
[ 27.456122] invoke_syscall+0x48/0x114
[ 27.460311] el0_svc_common.constprop.0+0x40/0xe0
[ 27.465567] do_el0_svc+0x1c/0x28
[ 27.469269] el0_svc+0x34/0xd8
[ 27.472681] el0t_64_sync_handler+0x120/0x12c
[ 27.477548] el0t_64_sync+0x190/0x194
[ 27.481639] ---[ end trace 0000000000000000 ]---
[ 27.486831] ------------[ cut here ]------------
[ 27.491995] refcount_t: underflow; use-after-free.
[ 27.497414] WARNING: CPU: 4 PID: 1 at lib/refcount.c:28
refcount_warn_saturate+0xf4/0x144
[ 27.506558] Modules linked in: snd_soc_simple_card crct10dif_ce
snd_soc_simple_card_utils fuse [last unloaded: panel_leadtek_ltk050h3146w]
[ 27.520468] CPU: 4 PID: 1 Comm: systemd-shutdow Tainted: G W
6.9.0-rc7-00002-g4a8eaebfcfde-dirty #63
[ 27.532230] Hardware name: Theobroma Systems RK3399-Q7 SoM on Haikou
devkit with Video Demo adapter (DT)
[ 27.542826] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS
BTYPE=--)
[ 27.550607] pc : refcount_warn_saturate+0xf4/0x144
[ 27.555958] lr : refcount_warn_saturate+0xf4/0x144
[ 27.561309] sp : ffff800081babb10
[ 27.565008] x29: ffff800081babb10 x28: ffff000000640000 x27:
0000000000000000
[ 27.572988] x26: ffff8000814847f8 x25: 0000000000000001 x24:
ffff800081adb028
[ 27.580958] x23: ffff000000e13090 x22: ffff00000a6f82e0 x21:
ffff00000a6f8170
[ 27.588928] x20: ffff00000a6f8000 x19: ffff00000a6f8000 x18:
0000000000000006
[ 27.596907] x17: 000000040044ffff x16: 00500074b5503510 x15:
072007200720072e
[ 27.604877] x14: 0000000000000000 x13: 0000000000000046 x12:
0000000000000000
[ 27.612847] x11: 0000000000000000 x10: 0000000000000a20 x9 :
ffff800081bab980
[ 27.620826] x8 : ffff000000640a80 x7 : ffff0000054e1000 x6 :
00000000ffffffff
[ 27.628796] x5 : 00000000410fd080 x4 : 0000000000000002 x3 :
0000000000000000
[ 27.636775] x2 : 0000000000000000 x1 : 0000000000000000 x0 :
ffff000000640000
[ 27.644754] Call trace:
[ 27.647482] refcount_warn_saturate+0xf4/0x144
[ 27.652445] drm_dev_put.part.0+0xb0/0xc0
[ 27.656925] drm_dev_put+0x14/0x24
[ 27.660722] __drm_atomic_state_free+0xbc/0xd0
[ 27.665687] drm_atomic_helper_disable_all+0x108/0x214
[ 27.671429] drm_atomic_helper_shutdown+0xa4/0x148
[ 27.676781] rockchip_drm_platform_shutdown+0x18/0x28
[ 27.682425] platform_shutdown+0x24/0x34
[ 27.686807] device_shutdown+0x150/0x258
[ 27.691190] kernel_power_off+0x38/0x7c
[ 27.695475] __do_sys_reboot+0x204/0x24c
[ 27.699858] __arm64_sys_reboot+0x24/0x30
[ 27.704337] invoke_syscall+0x48/0x114
[ 27.708525] el0_svc_common.constprop.0+0x40/0xe0
[ 27.713782] do_el0_svc+0x1c/0x28
[ 27.717484] el0_svc+0x34/0xd8
[ 27.720894] el0t_64_sync_handler+0x120/0x12c
[ 27.725762] el0t_64_sync+0x190/0x194
[ 27.729851] ---[ end trace 0000000000000000 ]---
"""
when running "poweroff" after doing a modprobe -r of the driver on
RK3399 Puma and PX30 Ringneck (the trace comes from RK3399 Puma). It is
fine if I still have the device probed, no trace when powering off.
BUT, I have the same trace in the 6.9-rc7 already, so I guess:
Reviewed-by: Quentin Schulz <[email protected]>
Tested-by: Quentin Schulz <[email protected]> # RK3399 Puma with
Haikou Video Demo
Tested-by: Quentin Schulz <[email protected]> # PX30 Ringneck
with Haikou Video Demo
I'll let Heiko test this on RK3588 Tiger with Haikou Video Demo to check
the whole stack is ready there as well.
Thanks!
Quentin
Hi,
On Mon, May 6, 2024 at 7:04 AM Quentin Schulz <[email protected]> wrote:
>
> Hi Douglas,
>
> On 5/3/24 11:32 PM, Douglas Anderson wrote:
> > [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > It's the responsibility of a correctly written DRM modeset driver to
> > call drm_atomic_helper_shutdown() at shutdown time and that should be
> > disabling / unpreparing the panel if needed. Panel drivers shouldn't
> > be calling these functions themselves.
> >
> > A recent effort was made to fix as many DRM modeset drivers as
> > possible [1] [2] [3] and most drivers are fixed now.
> >
> > Unfortunately, grepping mainline for this panel's compatible string
> > shows no hits, so we can't be 100% sure if the DRM modeset driver used
> > with this panel has been fixed. If it is found that the DRM modeset
> > driver hasn't been fixed then this patch could be temporarily reverted
> > until it is.
> >
> > [1] https://lore.kernel.org/r/20230901234015.566018-1-dianders@chromiumorg
> > [2] https://lore.kernel.org/r/20230901234202.566951-1-dianders@chromiumorg
> > [3] https://lore.kernel.org/r/[email protected]
> >
> > Cc: "Heiko Stübner" <[email protected]>
> > Cc: Quentin Schulz <[email protected]>
> > Signed-off-by: Douglas Anderson <[email protected]>
>
> I get:
>
> """
> [ 27.239362] ------------[ cut here ]------------
> [ 27.244549] refcount_t: addition on 0; use-after-free.
> [ 27.250308] WARNING: CPU: 4 PID: 1 at lib/refcount.c:25
> refcount_warn_saturate+0x120/0x144
> [ 27.259556] Modules linked in: snd_soc_simple_card crct10dif_ce
> snd_soc_simple_card_utils fuse [last unloaded: panel_leadtek_ltk050h3146w]
> [ 27.273470] CPU: 4 PID: 1 Comm: systemd-shutdow Not tainted
> 6.9.0-rc7-00002-g4a8eaebfcfde-dirty #63
> [ 27.283584] Hardware name: Theobroma Systems RK3399-Q7 SoM on Haikou
> devkit with Video Demo adapter (DT)
> [ 27.294180] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS
> BTYPE=--)
> [ 27.301962] pc : refcount_warn_saturate+0x120/0x144
> [ 27.307411] lr : refcount_warn_saturate+0x120/0x144
> [ 27.312860] sp : ffff800081babb10
> [ 27.316559] x29: ffff800081babb10 x28: ffff000000640000 x27:
> 0000000000000000
> [ 27.324539] x26: ffff8000814847f8 x25: 0000000000000001 x24:
> ffff800081adb028
> [ 27.332519] x23: ffff000000e13090 x22: ffff800081b3c280 x21:
> ffff0000006c8680
> [ 27.340489] x20: ffff0000006c8680 x19: ffff00000a6f8000 x18:
> 0000000000000006
> [ 27.348468] x17: 000000040044ffff x16: 00500074b5503510 x15:
> ffff800081bab580
> [ 27.356447] x14: 0000000000000000 x13: ffff8000819944d0 x12:
> 0000000000000a2f
> [ 27.364426] x11: 0000000000000365 x10: ffff8000819ec4d0 x9 :
> ffff8000819944d0
> [ 27.372396] x8 : 00000000ffffefff x7 : ffff8000819ec4d0 x6 :
> 80000000fffff000
> [ 27.380375] x5 : 0000000000000366 x4 : 0000000000000000 x3 :
> 0000000000000000
> [ 27.388353] x2 : 0000000000000000 x1 : 0000000000000000 x0 :
> ffff000000640000
> [ 27.396332] Call trace:
> [ 27.399061] refcount_warn_saturate+0x120/0x144
> [ 27.404122] drm_dev_get+0x78/0x7c
> [ 27.407924] drm_atomic_state_init+0x78/0xd0
> [ 27.412695] drm_atomic_state_alloc+0x6c/0x9c
> [ 27.417563] drm_atomic_helper_disable_all+0x20/0x214
> [ 27.423208] drm_atomic_helper_shutdown+0xa4/0x148
> [ 27.428560] rockchip_drm_platform_shutdown+0x18/0x28
> [ 27.434207] platform_shutdown+0x24/0x34
> [ 27.438589] device_shutdown+0x150/0x258
> [ 27.442973] kernel_power_off+0x38/0x7c
> [ 27.447260] __do_sys_reboot+0x204/0x24c
> [ 27.451643] __arm64_sys_reboot+0x24/0x30
> [ 27.456122] invoke_syscall+0x48/0x114
> [ 27.460311] el0_svc_common.constprop.0+0x40/0xe0
> [ 27.465567] do_el0_svc+0x1c/0x28
> [ 27.469269] el0_svc+0x34/0xd8
> [ 27.472681] el0t_64_sync_handler+0x120/0x12c
> [ 27.477548] el0t_64_sync+0x190/0x194
> [ 27.481639] ---[ end trace 0000000000000000 ]---
> [ 27.486831] ------------[ cut here ]------------
> [ 27.491995] refcount_t: underflow; use-after-free.
> [ 27.497414] WARNING: CPU: 4 PID: 1 at lib/refcount.c:28
> refcount_warn_saturate+0xf4/0x144
> [ 27.506558] Modules linked in: snd_soc_simple_card crct10dif_ce
> snd_soc_simple_card_utils fuse [last unloaded: panel_leadtek_ltk050h3146w]
> [ 27.520468] CPU: 4 PID: 1 Comm: systemd-shutdow Tainted: G W
> 6.9.0-rc7-00002-g4a8eaebfcfde-dirty #63
> [ 27.532230] Hardware name: Theobroma Systems RK3399-Q7 SoM on Haikou
> devkit with Video Demo adapter (DT)
> [ 27.542826] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS
> BTYPE=--)
> [ 27.550607] pc : refcount_warn_saturate+0xf4/0x144
> [ 27.555958] lr : refcount_warn_saturate+0xf4/0x144
> [ 27.561309] sp : ffff800081babb10
> [ 27.565008] x29: ffff800081babb10 x28: ffff000000640000 x27:
> 0000000000000000
> [ 27.572988] x26: ffff8000814847f8 x25: 0000000000000001 x24:
> ffff800081adb028
> [ 27.580958] x23: ffff000000e13090 x22: ffff00000a6f82e0 x21:
> ffff00000a6f8170
> [ 27.588928] x20: ffff00000a6f8000 x19: ffff00000a6f8000 x18:
> 0000000000000006
> [ 27.596907] x17: 000000040044ffff x16: 00500074b5503510 x15:
> 072007200720072e
> [ 27.604877] x14: 0000000000000000 x13: 0000000000000046 x12:
> 0000000000000000
> [ 27.612847] x11: 0000000000000000 x10: 0000000000000a20 x9 :
> ffff800081bab980
> [ 27.620826] x8 : ffff000000640a80 x7 : ffff0000054e1000 x6 :
> 00000000ffffffff
> [ 27.628796] x5 : 00000000410fd080 x4 : 0000000000000002 x3 :
> 0000000000000000
> [ 27.636775] x2 : 0000000000000000 x1 : 0000000000000000 x0 :
> ffff000000640000
> [ 27.644754] Call trace:
> [ 27.647482] refcount_warn_saturate+0xf4/0x144
> [ 27.652445] drm_dev_put.part.0+0xb0/0xc0
> [ 27.656925] drm_dev_put+0x14/0x24
> [ 27.660722] __drm_atomic_state_free+0xbc/0xd0
> [ 27.665687] drm_atomic_helper_disable_all+0x108/0x214
> [ 27.671429] drm_atomic_helper_shutdown+0xa4/0x148
> [ 27.676781] rockchip_drm_platform_shutdown+0x18/0x28
> [ 27.682425] platform_shutdown+0x24/0x34
> [ 27.686807] device_shutdown+0x150/0x258
> [ 27.691190] kernel_power_off+0x38/0x7c
> [ 27.695475] __do_sys_reboot+0x204/0x24c
> [ 27.699858] __arm64_sys_reboot+0x24/0x30
> [ 27.704337] invoke_syscall+0x48/0x114
> [ 27.708525] el0_svc_common.constprop.0+0x40/0xe0
> [ 27.713782] do_el0_svc+0x1c/0x28
> [ 27.717484] el0_svc+0x34/0xd8
> [ 27.720894] el0t_64_sync_handler+0x120/0x12c
> [ 27.725762] el0t_64_sync+0x190/0x194
> [ 27.729851] ---[ end trace 0000000000000000 ]---
> """
>
> when running "poweroff" after doing a modprobe -r of the driver on
> RK3399 Puma and PX30 Ringneck (the trace comes from RK3399 Puma). It is
> fine if I still have the device probed, no trace when powering off.
I'm not too surprised there. It's _really_ hard to successfully
"remove" DRM devices that are used on systems that use the component
model to get everything up and running. My series doesn't attempt to
make this better but it also shouldn't make it worse. It does point to
the fact that probably many of the remove() functions in DRM panels
are never exercised...
> BUT, I have the same trace in the 6.9-rc7 already, so I guess:
>
> Reviewed-by: Quentin Schulz <[email protected]>
> Tested-by: Quentin Schulz <[email protected]> # RK3399 Puma with
> Haikou Video Demo
> Tested-by: Quentin Schulz <[email protected]> # PX30 Ringneck
> with Haikou Video Demo
>
> I'll let Heiko test this on RK3588 Tiger with Haikou Video Demo to check
> the whole stack is ready there as well.
Thank you for reviewing / testing! :-)
-Doug
Am Freitag, 3. Mai 2024, 23:32:56 CEST schrieb Douglas Anderson:
> It's the responsibility of a correctly written DRM modeset driver to
> call drm_atomic_helper_shutdown() at shutdown time and that should be
> disabling / unpreparing the panel if needed. Panel drivers shouldn't
> be calling these functions themselves.
>
> A recent effort was made to fix as many DRM modeset drivers as
> possible [1] [2] [3] and most drivers are fixed now.
>
> Unfortunately, grepping mainline for this panel's compatible string
> shows no hits, so we can't be 100% sure if the DRM modeset driver used
> with this panel has been fixed. If it is found that the DRM modeset
> driver hasn't been fixed then this patch could be temporarily reverted
> until it is.
>
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
> [3] https://lore.kernel.org/r/[email protected]
>
> Cc: "Heiko St?bner" <[email protected]>
> Cc: Quentin Schulz <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
On a rk3588-tiger with WIP DSI patches and this display
Tested-by: Heiko Stuebner <[email protected]>
Before this patch on reboot the system emits
[ 181.464230] panel-leadtek-ltk050h3146w fde20000.dsi.0: Skipping disable of already disabled panel
[ 181.465056] panel-leadtek-ltk050h3146w fde20000.dsi.0: Skipping unprepare of already unprepared panel
after applying this patch, those lines are gone.
Also the patch itself looks good
Reviewed-by: Heiko Stuebner <[email protected]>
Am Freitag, 3. Mai 2024, 23:32:55 CEST schrieb Douglas Anderson:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
>
> Cc: "Heiko St?bner" <[email protected]>
> Cc: Quentin Schulz <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
on a rk3588-tiger with WIP DSI patches
Tested-by: Heiko Stuebner <[email protected]>
Am Freitag, 3. Mai 2024, 23:32:50 CEST schrieb Douglas Anderson:
> It's the responsibility of a correctly written DRM modeset driver to
> call drm_atomic_helper_shutdown() at shutdown time and that should be
> disabling / unpreparing the panel if needed. Panel drivers shouldn't
> be calling these functions themselves.
>
> A recent effort was made to fix as many DRM modeset drivers as
> possible [1] [2] [3] and most drivers are fixed now.
>
> A grep through mainline for compatible strings used by this driver
> indicates that it is used by Rockchip boards. The Rockchip driver
> appears to be correctly calling drm_atomic_helper_shutdown() so we can
> remove the calls.
>
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
> [3] https://lore.kernel.org/r/[email protected]
>
> Cc: Chris Zhong <[email protected]>
> Cc: Lin Huang <[email protected]>
> Cc: Brian Norris <[email protected]>
> Cc: "Heiko St?bner" <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)
Reviewed-by: Heiko Stuebner <[email protected]>
Am Freitag, 3. Mai 2024, 23:32:53 CEST schrieb Douglas Anderson:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
>
> Cc: Brian Norris <[email protected]>
> Cc: Chris Zhong <[email protected]>
> Cc: Nickey Yang <[email protected]>
> Cc: "Heiko St?bner" <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)
Reviewed-by: Heiko Stuebner <[email protected]>
Am Freitag, 3. Mai 2024, 23:32:54 CEST schrieb Douglas Anderson:
> It's the responsibility of a correctly written DRM modeset driver to
> call drm_atomic_helper_shutdown() at shutdown time and that should be
> disabling / unpreparing the panel if needed. Panel drivers shouldn't
> be calling these functions themselves.
>
> A recent effort was made to fix as many DRM modeset drivers as
> possible [1] [2] [3] and most drivers are fixed now.
>
> A grep through mainline for compatible strings used by this driver
> indicates that it is used by Rockchip boards. The Rockchip driver
> appear to be correctly calling drm_atomic_helper_shutdown() so we can
> remove the calls.
>
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
> [3] https://lore.kernel.org/r/[email protected]
>
> Cc: Brian Norris <[email protected]>
> Cc: Chris Zhong <[email protected]>
> Cc: Nickey Yang <[email protected]>
> Cc: "Heiko St?bner" <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)
Reviewed-by: Heiko Stuebner <[email protected]>
Am Freitag, 3. Mai 2024, 23:32:57 CEST schrieb Douglas Anderson:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
>
> Cc: "Heiko St?bner" <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)
Reviewed-by: Heiko Stuebner <[email protected]>
Am Freitag, 3. Mai 2024, 23:32:49 CEST schrieb Douglas Anderson:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
>
> Cc: Chris Zhong <[email protected]>
> Cc: Lin Huang <[email protected]>
> Cc: Brian Norris <[email protected]>
> Cc: "Heiko St?bner" <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)
Reviewed-by: Heiko Stuebner <[email protected]>
Am Freitag, 3. Mai 2024, 23:32:58 CEST schrieb Douglas Anderson:
> It's the responsibility of a correctly written DRM modeset driver to
> call drm_atomic_helper_shutdown() at shutdown time and that should be
> disabling / unpreparing the panel if needed. Panel drivers shouldn't
> be calling these functions themselves.
>
> A recent effort was made to fix as many DRM modeset drivers as
> possible [1] [2] [3] and most drivers are fixed now.
>
> Unfortunately, grepping mainline for this panel's compatible string
> shows no hits, so we can't be 100% sure if the DRM modeset driver used
> with this panel has been fixed. If it is found that the DRM modeset
> driver hasn't been fixed then this patch could be temporarily reverted
> until it is.
>
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
> [3] https://lore.kernel.org/r/[email protected]
>
> Cc: "Heiko St?bner" <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)
Reviewed-by: Heiko Stuebner <[email protected]>
Am Freitag, 3. Mai 2024, 23:33:12 CEST schrieb Douglas Anderson:
> It's the responsibility of a correctly written DRM modeset driver to
> call drm_atomic_helper_shutdown() at shutdown time and that should be
> disabling / unpreparing the panel if needed. Panel drivers shouldn't
> be calling these functions themselves.
>
> A recent effort was made to fix as many DRM modeset drivers as
> possible [1] [2] [3] and most drivers are fixed now.
>
> A grep through mainline for compatible strings used by this driver
> indicates that it is used by Rockchip boards. The Rockchip driver
> appears to be correctly calling drm_atomic_helper_shutdown() so we can
> remove the calls.
>
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
> [3] https://lore.kernel.org/r/[email protected]
>
> Cc: "Heiko St?bner" <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)
Reviewed-by: Heiko Stuebner <[email protected]>
Am Freitag, 3. Mai 2024, 23:33:11 CEST schrieb Douglas Anderson:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
>
> Cc: "Heiko St?bner" <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)
Reviewed-by: Heiko Stuebner <[email protected]>
Hi,
On Sun, May 5, 2024 at 11:52 PM Linus Walleij <linus.walleij@linaroorg> wrote:
>
> On Fri, May 3, 2024 at 11:36 PM Douglas Anderson <[email protected]> wrote:
>
> > As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> > prepared/enabled in drm_panel"), we want to remove needless code from
> > panel drivers that was storing and double-checking the
> > prepared/enabled state. Even if someone was relying on the
> > double-check before, that double-check is now in the core and not
> > needed in individual drivers.
> >
> > This series attempts to do just that. While the original grep, AKA:
> > git grep 'if.*>prepared' -- drivers/gpu/drm/panel
> > git grep 'if.*>enabled' -- drivers/gpu/drm/panel
> > ...still produces a few hits after my series, they are _mostly_ all
> > gone. The ones that are left are less trivial to fix.
> >
> > One of the main reasons that many panels probably needed to store and
> > double-check their prepared/enabled appears to have been to handle
> > shutdown and/or remove. Panels drivers often wanted to force the power
> > off for panels in these cases and this was a good reason for the
> > double-check.
> >
> > In response to my V1 series [1] we had much discussion of what to
> > do. The conclusion was that as long as DRM modeset drivers properly
> > called drm_atomic_helper_shutdown() that we should be able to remove
> > the explicit shutdown/remove handling in the panel drivers. Most of
> > the patches to improve DRM modeset drivers [2] [3] [4] have now
> > landed.
> >
> > In contrast to my V1 series, I broke the V2 series up a lot
> > more. Since a few of the panel drivers in V1 already landed, we had
> > fewer total drivers and so we could devote a patch to each panel.
> > Also, since we were now relying on DRM modeset drivers I felt like we
> > should split the patches for each panel into two: one that's
> > definitely safe and one that could be reverted if we found a
> > problematic DRM modeset driver that we couldn't fix.
> >
> > Sorry for the large number of patches. I've set things to mostly just
> > CC people on the cover letter and the patches that are relevant to
> > them. I've tried to CC people on the whole series that have shown
> > interest in this TODO item.
> >
> > As patches in this series are reviewed and/or tested they could be
> > landed. There's really no ordering requirement for the series unless
> > patches touch the same driver.
> >
> > NOTE: this touches _a lot_ of drivers, is repetitive, and is not
> > really possible to generate automatically. That means it's entirely
> > possible that my eyes glazed over and I did something wrong. Please
> > double-check me and don't assume that I got everything perfect, though
> > I did my best. I have at least confirmed that "allmodconfig" for arm64
> > doesn't fall on its face with this series. I haven't done a ton of
> > other testing.
> >
> > [1] https://lore.kernel.org/r/20230804140605.RFC.4.I930069a32baab6faf46d6b234f89613b5cec0f14@changeid
> > [2] https://lore.kernel.org/r/20230901234015.566018-1-dianders@chromiumorg
> > [3] https://lore.kernel.org/r/20230901234202.566951-1-dianders@chromiumorg
> > [4] https://lore.kernel.org/r/[email protected]
>
> This is the right thing to do, thanks for looking into this!
>
> As for the behaviour of .remove() I doubt whether in many cases
> the original driver authors have even tested this themselves.
Yeah, I'd tend to agree.
> I would say we should just apply the series as soon as it's non-RFC
It's not actually RFC now, but "RFT" (request for testing). I don't
_think_ there's any need to send a version without the RFT tag before
landing unless someone really feels strongly about it.
> after the next merge window
With drm-misc there's not really any specific reason to wait for the
merge window to open/close as we can land in drm-misc-next at any time
regardless of the merge window. drm-misc-next will simply stop feeding
linuxnext for a while.
That all being said, I'm happy to delay landing this until after the
next -rc1 comes out if people would prefer that. If I don't hear
anything, I guess I'll just wait until -rc1 before landing any of
these.
> and see what happens. I doubt it
> will cause much trouble.
I can land the whole series if that's what everyone agrees on. As I
mentioned above, I'm at least slightly worried that I did something
stupid _somewhere_ in this series since no automation was possible and
with repetitive tasks like this it's super easy to flub something up.
It's _probably_ fine, but I guess I still have the worry in the back
of my mind.
If folks think I should just apply the whole series then I'm happy to
do that. If folks think I should just land parts of the series as they
are reviewed/tested I can do that as well. Let me know. If I don't
hear anything I'd tend to just land patches that are reviewed/tested.
Then after a month or so (hopefully) I'd send out a v2 with anything
left.
> The series:
> Acked-by: Linus Walleij <[email protected]>
Thanks!
-Doug
On Fri May 3, 2024 at 11:32 PM CEST, Douglas Anderson wrote:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
>
> Cc: Luca Weiss <[email protected]>
> Cc: Konrad Dybcio <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
Seems to match with the changes I did for another (generated) panel
driver I upstreamed - see also:
https://github.com/msm8916-mainline/linux-mdss-dsi-panel-driver-generator/commit/74c104440dfd828aa94194fd279c0c505ab55854
Functionally also seems to be fine, I don't see any problems. Thanks!
Tested-by: Luca Weiss <[email protected]>
Regards
Luca
> ---
>
> Changes in v2:
> - New
>
> drivers/gpu/drm/panel/panel-raydium-rm692e5.c | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c
> index a613ba5b816c..21d97f6b8a2f 100644
> --- a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c
> +++ b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c
> @@ -23,7 +23,6 @@ struct rm692e5_panel {
> struct drm_dsc_config dsc;
> struct regulator_bulk_data supplies[3];
> struct gpio_desc *reset_gpio;
> - bool prepared;
> };
>
> static inline struct rm692e5_panel *to_rm692e5_panel(struct drm_panel *panel)
> @@ -171,9 +170,6 @@ static int rm692e5_prepare(struct drm_panel *panel)
> struct device *dev = &ctx->dsi->dev;
> int ret;
>
> - if (ctx->prepared)
> - return 0;
> -
> ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> if (ret < 0) {
> dev_err(dev, "Failed to enable regulators: %d\n", ret);
> @@ -213,8 +209,6 @@ static int rm692e5_prepare(struct drm_panel *panel)
>
> mipi_dsi_generic_write_seq(ctx->dsi, 0xfe, 0x00);
>
> - ctx->prepared = true;
> -
> return 0;
> }
>
> @@ -222,13 +216,9 @@ static int rm692e5_unprepare(struct drm_panel *panel)
> {
> struct rm692e5_panel *ctx = to_rm692e5_panel(panel);
>
> - if (!ctx->prepared)
> - return 0;
> -
> gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
>
> - ctx->prepared = false;
> return 0;
> }
>
Hi,
On 2024/5/4 05:33, Douglas Anderson wrote:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
I think you are right, as I see drm_panel already has embed
the 'prepared' and 'enabled' as members, remove them from
the derived structure could probably save memory footprint.
Reducing boilerplate is also a side benefit.
> Signed-off-by: Douglas Anderson <[email protected]>
Acked-by: Sui Jingfeng <[email protected]>
--
Best regards,
Sui
Hi,
On Wed, May 8, 2024 at 2:14 PM Doug Anderson <[email protected]> wrote:
>
> > This is the right thing to do, thanks for looking into this!
> >
> > As for the behaviour of .remove() I doubt whether in many cases
> > the original driver authors have even tested this themselves.
>
> Yeah, I'd tend to agree.
>
>
> > I would say we should just apply the series as soon as it's non-RFC
>
> It's not actually RFC now, but "RFT" (request for testing). I don't
> _think_ there's any need to send a version without the RFT tag before
> landing unless someone really feels strongly about it.
>
>
> > after the next merge window
>
> With drm-misc there's not really any specific reason to wait for the
> merge window to open/close as we can land in drm-misc-next at any time
> regardless of the merge window. drm-misc-next will simply stop feeding
> linuxnext for a while.
>
> That all being said, I'm happy to delay landing this until after the
> next -rc1 comes out if people would prefer that. If I don't hear
> anything, I guess I'll just wait until -rc1 before landing any of
> these.
>
>
> > and see what happens. I doubt it
> > will cause much trouble.
>
> I can land the whole series if that's what everyone agrees on. As I
> mentioned above, I'm at least slightly worried that I did something
> stupid _somewhere_ in this series since no automation was possible and
> with repetitive tasks like this it's super easy to flub something up.
> It's _probably_ fine, but I guess I still have the worry in the back
> of my mind.
>
> If folks think I should just apply the whole series then I'm happy to
> do that. If folks think I should just land parts of the series as they
> are reviewed/tested I can do that as well. Let me know. If I don't
> hear anything I'd tend to just land patches that are reviewed/tested.
> Then after a month or so (hopefully) I'd send out a v2 with anything
> left.
Nobody said anything, so I did what I indicated above:
1. I've applied all patches that someone responded to with Linus +
Maxime's Acks + any given tags. This includes the st7703 panels which
Ondřej replied to the cover letter about but didn't officially get any
tags.
2. I also applied patches for panels that I was personally involved
with. This includes panel-edp, panel-simple, samsung-atna33xc20,
boe-tv101wum-nl6.
Anything totally unresponded to I've left unapplied. I'll wait a
little while (at least a week) and then plan to send a v2 with
anything still outstanding. If someone sends Tested-by/Reviewed-by for
some panels in the meantime I'll apply them.
Here are the 25 patches applied to drm-misc-next:
[01/48] drm/panel: raydium-rm692e5: Stop tracking prepared
commit: 598dc42f25cc3060fd350db0f52af1075af3f500
[04/48] drm/panel: boe-tv101wum-nl6: Stop tracking prepared
commit: 3c24e31c908eb12e99420ff33b74c01f045253fe
[05/48] drm/panel: boe-tv101wum-nl6: Don't call unprepare+disable at
shutdown/remove
commit: 1985e3512b5a3777f6a18c36e40f3926037120bb
[06/48] drm/panel: edp: Stop tracking prepared/enabled
commit: 3904f317fd977533f6d7d3c4bfd75e0ac6169bb7
[07/48] drm/panel: edp: Add a comment about unprepare+disable at shutdown/remove
commit: ec7629859331fb67dbfb6bcd47f887a402e390ff
[08/48] drm/panel: innolux-p079zca: Stop tracking prepared/enabled
commit: f9055051292442d52092f17e191cf0a58d23d4ed
[09/48] drm/panel: innolux-p079zca: Don't call unprepare+disable at
shutdown/remove
commit: eeb133ff78476eb1e6e88154dfb75a741e8a034a
[12/48] drm/panel: kingdisplay-kd097d04: Stop tracking prepared/enabled
commit: 157c1381780a453e06430f8b35bb8c5d439eb8c6
[13/48] drm/panel: kingdisplay-kd097d04: Don't call unprepare+disable
at shutdown/remove
commit: 68c205ef3c39edce4a3346b8a53fd2b700394a0c
[14/48] drm/panel: ltk050h3146w: Stop tracking prepared
commit: f124478dd18c519544489caddce78e7c5796a758
[15/48] drm/panel: ltk050h3146w: Don't call unprepare+disable at shutdown/remove
commit: b7ca446ecb53205944968617b158f073bcacaedc
[16/48] drm/panel: ltk500hd1829: Stop tracking prepared
commit: 2b8c19b9d7bc9d03e8c44bd391d21e95c07a2c83
[17/48] drm/panel: ltk500hd1829: Don't call unprepare+disable at shutdown/remove
commit: 3357f6f465e62c0bc5e906365063734740c9f6d4
[18/48] drm/panel: novatek-nt36672a: Stop tracking prepared
commit: b605f257f386b7f4b6fc9c0f82b86b75d0579287
[19/48] drm/panel: novatek-nt36672a: Don't call unprepare+disable at
shutdown/remove
commit: 2a9487b5aa55753993fde80e4841128c8da4df71
[24/48] drm/panel: samsung-atna33xc20: Stop tracking prepared/enabled
commit: 5a847750aac8454a1604070ab99d689c0a6e4290
[25/48] drm/panel: samsung-atna33xc20: Don't call unprepare+disable at
shutdown/remove
commit: 49869668ff0e3f380858b4c20b8d0cb02b933f48
[26/48] drm/panel: simple: Stop tracking prepared/enabled
commit: 2a1c99d7159b798288bfb20a76c1e665e2344126
[27/48] drm/panel: simple: Add a comment about unprepare+disable at
shutdown/remove
commit: bc62654df3c888dec735343f5db9907ac93aea60
[30/48] drm/panel: xinpeng-xpp055c272: Stop tracking prepared
commit: 4e5e6fa77a9d40cdf85ade7f86d07dc8929941c9
[31/48] drm/panel: xinpeng-xpp055c272: Don't call unprepare+disable at
shutdown/remove
commit: ac9e1786271f771ff1f774742602330be2d57a12
[42/48] drm/panel: sitronix-st7703: Stop tracking prepared
commit: 3004d2e9cca5d59d25dff670a03a005d40601ded
[43/48] drm/panel: sitronix-st7703: Don't call disable at shutdown/remove
commit: 718bd8a1a5ee873778a72523c06da054a89108b4
[46/48] drm/panel: sony-acx565akm: Don't double-check enabled state in disable
commit: e28df86aeeff0b84c13e676f641ea879abbdb809
[47/48] drm/panel: sony-acx565akm: Don't call disable at remove
commit: 6afebd850d1ab5518c273b32532f0b2086cc633a
-Doug