This series is pretty much just addressing a few minor error handling
bugs that I noticed recently while reviewing some panel patches. For
the most part these are all old issues.
This also adjusts the new himax-hx83102 in a similar way that Dmitry
did in his recent series that included commit f79d6d28d8fe
("drm/mipi-dsi: wrap more functions for streamline handling"). His
series handled the panel driver that himax-hx83102 forked from but not
himax-hx83102.
Douglas Anderson (8):
drm/panel: himax-hx8394: Handle errors from
mipi_dsi_dcs_set_display_on() better
drm/panel: boe-tv101wum-nl6: If prepare fails, disable GPIO before
regulators
drm/panel: boe-tv101wum-nl6: Check for errors on the NOP in prepare()
drm/panel: ilitek-ili9882t: If prepare fails, disable GPIO before
regulators
drm/panel: ilitek-ili9882t: Check for errors on the NOP in prepare()
drm/panel: himax-hx83102: If prepare fails, disable GPIO before
regulators
drm/panel: himax-hx83102: Check for errors on the NOP in prepare()
drm/panel: himax-hx83102: use wrapped MIPI DCS functions
.../gpu/drm/panel/panel-boe-tv101wum-nl6.c | 8 +-
drivers/gpu/drm/panel/panel-himax-hx83102.c | 92 ++++++-------------
drivers/gpu/drm/panel/panel-himax-hx8394.c | 3 +-
drivers/gpu/drm/panel/panel-ilitek-ili9882t.c | 8 +-
4 files changed, 43 insertions(+), 68 deletions(-)
--
2.45.0.rc1.225.g2a3ae87e7f-goog
If mipi_dsi_dcs_set_display_on() returned an error then we'd store
that in the "ret" variable and jump to error handling. We'd then
attempt an orderly poweroff. Unfortunately we then blew away the value
stored in "ret". That means that if the orderly poweroff actually
worked then we're return 0 (no error) from hx8394_enable() even though
the panel wasn't enabled.
Fix this by not blowing away "ret".
Found by code inspection.
Fixes: 65dc9360f741 ("drm: panel: Add Himax HX8394 panel controller driver")
Signed-off-by: Douglas Anderson <[email protected]>
---
drivers/gpu/drm/panel/panel-himax-hx8394.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-himax-hx8394.c b/drivers/gpu/drm/panel/panel-himax-hx8394.c
index ff0dc08b9829..cb9f46e853de 100644
--- a/drivers/gpu/drm/panel/panel-himax-hx8394.c
+++ b/drivers/gpu/drm/panel/panel-himax-hx8394.c
@@ -370,8 +370,7 @@ static int hx8394_enable(struct drm_panel *panel)
sleep_in:
/* This will probably fail, but let's try orderly power off anyway. */
- ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
- if (!ret)
+ if (!mipi_dsi_dcs_enter_sleep_mode(dsi))
msleep(50);
return ret;
--
2.45.0.rc1.225.g2a3ae87e7f-goog
The enable GPIO should clearly be set low before turning off
regulators. That matches both the inverse order that things were
enabled and also the order in unprepare().
Fixes: a869b9db7adf ("drm/panel: support for boe tv101wum-nl6 wuxga dsi video mode panel")
Signed-off-by: Douglas Anderson <[email protected]>
---
drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
index 8e839a1749e4..028625d2d37d 100644
--- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
+++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
@@ -1475,13 +1475,13 @@ static int boe_panel_prepare(struct drm_panel *panel)
return 0;
poweroff:
+ gpiod_set_value(boe->enable_gpio, 0);
regulator_disable(boe->avee);
poweroffavdd:
regulator_disable(boe->avdd);
poweroff1v8:
usleep_range(5000, 7000);
regulator_disable(boe->pp1800);
- gpiod_set_value(boe->enable_gpio, 0);
return ret;
}
--
2.45.0.rc1.225.g2a3ae87e7f-goog
The mipi_dsi_dcs_nop() function returns an error but we weren't
checking it in boe_panel_prepare(). Add a check. This is highly
unlikely to matter in practice. If the NOP failed then likely later
MIPI commands would fail too.
Found by code inspection.
Fixes: 812562b8d881 ("drm/panel: boe-tv101wum-nl6: Fine tune the panel power sequence")
Signed-off-by: Douglas Anderson <[email protected]>
---
drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
index 028625d2d37d..f7beace455c3 100644
--- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
+++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
@@ -1456,7 +1456,11 @@ static int boe_panel_prepare(struct drm_panel *panel)
usleep_range(10000, 11000);
if (boe->desc->lp11_before_reset) {
- mipi_dsi_dcs_nop(boe->dsi);
+ ret = mipi_dsi_dcs_nop(boe->dsi);
+ if (ret < 0) {
+ dev_err(&boe->dsi->dev, "Failed to send NOP: %d\n", ret);
+ goto poweroff;
+ }
usleep_range(1000, 2000);
}
gpiod_set_value(boe->enable_gpio, 1);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
The enable GPIO should clearly be set low before turning off
regulators. That matches both the inverse order that things were
enabled and also the order in unprepare().
Fixes: e2450d32e5fb ("drm/panel: ili9882t: Break out as separate driver")
Signed-off-by: Douglas Anderson <[email protected]>
---
drivers/gpu/drm/panel/panel-ilitek-ili9882t.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c
index 830d7cfbe857..a2ea25bb6624 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c
@@ -495,13 +495,13 @@ static int ili9882t_prepare(struct drm_panel *panel)
return 0;
poweroff:
+ gpiod_set_value(ili->enable_gpio, 0);
regulator_disable(ili->avee);
poweroffavdd:
regulator_disable(ili->avdd);
poweroff1v8:
usleep_range(5000, 7000);
regulator_disable(ili->pp1800);
- gpiod_set_value(ili->enable_gpio, 0);
return ret;
}
--
2.45.0.rc1.225.g2a3ae87e7f-goog
The mipi_dsi_dcs_nop() function returns an error but we weren't
checking it in ili9882t_prepare(). Add a check. This is highly
unlikely to matter in practice. If the NOP failed then likely later
MIPI commands would fail too.
Found by code inspection.
Fixes: e2450d32e5fb ("drm/panel: ili9882t: Break out as separate driver")
Signed-off-by: Douglas Anderson <[email protected]>
---
drivers/gpu/drm/panel/panel-ilitek-ili9882t.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c
index a2ea25bb6624..266a087fe14c 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c
@@ -478,7 +478,11 @@ static int ili9882t_prepare(struct drm_panel *panel)
usleep_range(10000, 11000);
// MIPI needs to keep the LP11 state before the lcm_reset pin is pulled high
- mipi_dsi_dcs_nop(ili->dsi);
+ ret = mipi_dsi_dcs_nop(ili->dsi);
+ if (ret < 0) {
+ dev_err(&ili->dsi->dev, "Failed to send NOP: %d\n", ret);
+ goto poweroff;
+ }
usleep_range(1000, 2000);
gpiod_set_value(ili->enable_gpio, 1);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
The enable GPIO should clearly be set low before turning off
regulators. That matches both the inverse order that things were
enabled and also the order in unprepare().
Fixes: 0ef94554dc40 ("drm/panel: himax-hx83102: Break out as separate driver")
Signed-off-by: Douglas Anderson <[email protected]>
---
drivers/gpu/drm/panel/panel-himax-hx83102.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-himax-hx83102.c b/drivers/gpu/drm/panel/panel-himax-hx83102.c
index 1a6975937f30..4ac7f9d8b232 100644
--- a/drivers/gpu/drm/panel/panel-himax-hx83102.c
+++ b/drivers/gpu/drm/panel/panel-himax-hx83102.c
@@ -578,13 +578,13 @@ static int hx83102_prepare(struct drm_panel *panel)
return 0;
poweroff:
+ gpiod_set_value(ctx->enable_gpio, 0);
regulator_disable(ctx->avee);
poweroffavdd:
regulator_disable(ctx->avdd);
poweroff1v8:
usleep_range(5000, 7000);
regulator_disable(ctx->pp1800);
- gpiod_set_value(ctx->enable_gpio, 0);
return ret;
}
--
2.45.0.rc1.225.g2a3ae87e7f-goog
The mipi_dsi_dcs_nop() function returns an error but we weren't
checking it in hx83102_prepare(). Add a check. This is highly unlikely
to matter in practice. If the NOP failed then likely later MIPI
commands would fail too.
Found by code inspection.
Fixes: 0ef94554dc40 ("drm/panel: himax-hx83102: Break out as separate driver")
Signed-off-by: Douglas Anderson <[email protected]>
---
drivers/gpu/drm/panel/panel-himax-hx83102.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-himax-hx83102.c b/drivers/gpu/drm/panel/panel-himax-hx83102.c
index 4ac7f9d8b232..1ba623e41924 100644
--- a/drivers/gpu/drm/panel/panel-himax-hx83102.c
+++ b/drivers/gpu/drm/panel/panel-himax-hx83102.c
@@ -547,7 +547,11 @@ static int hx83102_prepare(struct drm_panel *panel)
usleep_range(10000, 11000);
- mipi_dsi_dcs_nop(ctx->dsi);
+ ret = mipi_dsi_dcs_nop(ctx->dsi);
+ if (ret < 0) {
+ dev_err(dev, "Failed to send NOP: %d\n", ret);
+ goto poweroff;
+ }
usleep_range(1000, 2000);
gpiod_set_value(ctx->enable_gpio, 1);
--
2.45.0.rc1.225.g2a3ae87e7f-goog
Take advantage of some of the new wrapped routines introduced by
commit f79d6d28d8fe ("drm/mipi-dsi: wrap more functions for streamline
handling") to simplify the himax-hx83102 driver a bit more. This gets
rid of some extra error prints (since the _multi functions all print
errors for you) and simplifies the code a bit.
One thing here that isn't just refactoring is that in a few places we
now check with errors with "if (err)" instead of "if (err < 0)". All
errors are expected to be negative so this is not expected to have any
impact. The _multi code internally considers anything non-zero to be
an error so this just makes things consistent.
It can also be noted that hx83102_prepare() has a mix of things that
can take advantage of _multi calls and things that can't. The cleanest
seemed to be to use the multi_ctx still but consistently use the
"accum_err" variable for error returns, though that's definitely a
style decision with pros and cons.
Signed-off-by: Douglas Anderson <[email protected]>
---
drivers/gpu/drm/panel/panel-himax-hx83102.c | 92 +++++++--------------
1 file changed, 28 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-himax-hx83102.c b/drivers/gpu/drm/panel/panel-himax-hx83102.c
index 1ba623e41924..6009a3fe1b8f 100644
--- a/drivers/gpu/drm/panel/panel-himax-hx83102.c
+++ b/drivers/gpu/drm/panel/panel-himax-hx83102.c
@@ -285,12 +285,10 @@ static int boe_nv110wum_init(struct hx83102 *ctx)
mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83102_SETSPCCMD, 0x3f);
mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83102_SETBANK, 0x00);
hx83102_enable_extended_cmds(&dsi_ctx, false);
- if (dsi_ctx.accum_err)
- return dsi_ctx.accum_err;
- msleep(50);
+ mipi_dsi_msleep(dsi_ctx, 50);
- return 0;
+ return dsi_ctx.accum_err;
};
static int ivo_t109nw41_init(struct hx83102 *ctx)
@@ -392,12 +390,10 @@ static int ivo_t109nw41_init(struct hx83102 *ctx)
mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83102_SETSPCCMD, 0x3f);
mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83102_SETBANK, 0x00);
hx83102_enable_extended_cmds(&dsi_ctx, false);
- if (dsi_ctx.accum_err)
- return dsi_ctx.accum_err;
- msleep(60);
+ mipi_dsi_msleep(dsi_ctx, 60);
- return 0;
+ return dsi_ctx.accum_err;
};
static const struct drm_display_mode starry_mode = {
@@ -472,40 +468,20 @@ static int hx83102_enable(struct drm_panel *panel)
return 0;
}
-static int hx83102_panel_enter_sleep_mode(struct hx83102 *ctx)
-{
- struct mipi_dsi_device *dsi = ctx->dsi;
- int ret;
-
- dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
-
- ret = mipi_dsi_dcs_set_display_off(dsi);
- if (ret < 0)
- return ret;
-
- ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
- if (ret < 0)
- return ret;
-
- return 0;
-}
-
static int hx83102_disable(struct drm_panel *panel)
{
struct hx83102 *ctx = panel_to_hx83102(panel);
struct mipi_dsi_device *dsi = ctx->dsi;
- struct device *dev = &dsi->dev;
- int ret;
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
- ret = hx83102_panel_enter_sleep_mode(ctx);
- if (ret < 0) {
- dev_err(dev, "failed to set panel off: %d\n", ret);
- return ret;
- }
+ dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
- msleep(150);
+ mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+ mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
- return 0;
+ mipi_dsi_msleep(&dsi_ctx, 150);
+
+ return dsi_ctx.accum_err;
}
static int hx83102_unprepare(struct drm_panel *panel)
@@ -526,32 +502,30 @@ static int hx83102_prepare(struct drm_panel *panel)
{
struct hx83102 *ctx = panel_to_hx83102(panel);
struct mipi_dsi_device *dsi = ctx->dsi;
- struct device *dev = &dsi->dev;
- int ret;
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
gpiod_set_value(ctx->enable_gpio, 0);
usleep_range(1000, 1500);
- ret = regulator_enable(ctx->pp1800);
- if (ret < 0)
- return ret;
+ dsi_ctx.accum_err = regulator_enable(ctx->pp1800);
+ if (dsi_ctx.accum_err)
+ return dsi_ctx.accum_err;
usleep_range(3000, 5000);
- ret = regulator_enable(ctx->avdd);
- if (ret < 0)
+ dsi_ctx.accum_err = regulator_enable(ctx->avdd);
+ if (dsi_ctx.accum_err)
goto poweroff1v8;
- ret = regulator_enable(ctx->avee);
- if (ret < 0)
+ dsi_ctx.accum_err = regulator_enable(ctx->avee);
+ if (dsi_ctx.accum_err)
goto poweroffavdd;
usleep_range(10000, 11000);
- ret = mipi_dsi_dcs_nop(ctx->dsi);
- if (ret < 0) {
- dev_err(dev, "Failed to send NOP: %d\n", ret);
+ mipi_dsi_dcs_nop_multi(&dsi_ctx);
+ if (dsi_ctx.accum_err)
goto poweroff;
- }
+
usleep_range(1000, 2000);
gpiod_set_value(ctx->enable_gpio, 1);
@@ -561,23 +535,13 @@ static int hx83102_prepare(struct drm_panel *panel)
gpiod_set_value(ctx->enable_gpio, 1);
usleep_range(6000, 10000);
- ret = ctx->desc->init(ctx);
- if (ret < 0)
- goto poweroff;
-
- ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
- if (ret) {
- dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
- goto poweroff;
- }
-
- msleep(120);
+ dsi_ctx.accum_err = ctx->desc->init(ctx);
- ret = mipi_dsi_dcs_set_display_on(dsi);
- if (ret) {
- dev_err(dev, "Failed to turn on the display: %d\n", ret);
+ mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+ mipi_dsi_msleep(dsi_ctx, 120);
+ mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+ if (dsi_ctx.accum_err)
goto poweroff;
- }
return 0;
@@ -590,7 +554,7 @@ static int hx83102_prepare(struct drm_panel *panel)
usleep_range(5000, 7000);
regulator_disable(ctx->pp1800);
- return ret;
+ return dsi_ctx.accum_err;
}
static int hx83102_get_modes(struct drm_panel *panel,
--
2.45.0.rc1.225.g2a3ae87e7f-goog
On Fri, May 17, 2024 at 11:37 PM Douglas Anderson <[email protected]> wrote:
> This series is pretty much just addressing a few minor error handling
> bugs that I noticed recently while reviewing some panel patches. For
> the most part these are all old issues.
All patches look good to me:
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Fri, May 17, 2024 at 02:36:36PM -0700, Douglas Anderson wrote:
> If mipi_dsi_dcs_set_display_on() returned an error then we'd store
> that in the "ret" variable and jump to error handling. We'd then
> attempt an orderly poweroff. Unfortunately we then blew away the value
> stored in "ret". That means that if the orderly poweroff actually
> worked then we're return 0 (no error) from hx8394_enable() even though
> the panel wasn't enabled.
>
> Fix this by not blowing away "ret".
>
> Found by code inspection.
>
> Fixes: 65dc9360f741 ("drm: panel: Add Himax HX8394 panel controller driver")
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
>
> drivers/gpu/drm/panel/panel-himax-hx8394.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Fri, May 17, 2024 at 02:36:37PM -0700, Douglas Anderson wrote:
> The enable GPIO should clearly be set low before turning off
> regulators. That matches both the inverse order that things were
> enabled and also the order in unprepare().
>
> Fixes: a869b9db7adf ("drm/panel: support for boe tv101wum-nl6 wuxga dsi video mode panel")
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
>
> drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Fri, May 17, 2024 at 02:36:39PM -0700, Douglas Anderson wrote:
> The enable GPIO should clearly be set low before turning off
> regulators. That matches both the inverse order that things were
> enabled and also the order in unprepare().
>
> Fixes: e2450d32e5fb ("drm/panel: ili9882t: Break out as separate driver")
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
>
> drivers/gpu/drm/panel/panel-ilitek-ili9882t.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Fri, May 17, 2024 at 02:36:38PM -0700, Douglas Anderson wrote:
> The mipi_dsi_dcs_nop() function returns an error but we weren't
> checking it in boe_panel_prepare(). Add a check. This is highly
> unlikely to matter in practice. If the NOP failed then likely later
> MIPI commands would fail too.
>
> Found by code inspection.
>
> Fixes: 812562b8d881 ("drm/panel: boe-tv101wum-nl6: Fine tune the panel power sequence")
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
>
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Fri, May 17, 2024 at 02:36:40PM -0700, Douglas Anderson wrote:
> The mipi_dsi_dcs_nop() function returns an error but we weren't
> checking it in ili9882t_prepare(). Add a check. This is highly
> unlikely to matter in practice. If the NOP failed then likely later
> MIPI commands would fail too.
>
> Found by code inspection.
>
> Fixes: e2450d32e5fb ("drm/panel: ili9882t: Break out as separate driver")
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
>
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Fri, May 17, 2024 at 02:36:41PM -0700, Douglas Anderson wrote:
> The enable GPIO should clearly be set low before turning off
> regulators. That matches both the inverse order that things were
> enabled and also the order in unprepare().
>
> Fixes: 0ef94554dc40 ("drm/panel: himax-hx83102: Break out as separate driver")
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
>
> drivers/gpu/drm/panel/panel-himax-hx83102.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Fri, May 17, 2024 at 02:36:42PM -0700, Douglas Anderson wrote:
> The mipi_dsi_dcs_nop() function returns an error but we weren't
> checking it in hx83102_prepare(). Add a check. This is highly unlikely
> to matter in practice. If the NOP failed then likely later MIPI
> commands would fail too.
>
> Found by code inspection.
>
> Fixes: 0ef94554dc40 ("drm/panel: himax-hx83102: Break out as separate driver")
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
>
> drivers/gpu/drm/panel/panel-himax-hx83102.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Fri, May 17, 2024 at 02:36:43PM -0700, Douglas Anderson wrote:
> Take advantage of some of the new wrapped routines introduced by
> commit f79d6d28d8fe ("drm/mipi-dsi: wrap more functions for streamline
> handling") to simplify the himax-hx83102 driver a bit more. This gets
> rid of some extra error prints (since the _multi functions all print
> errors for you) and simplifies the code a bit.
>
> One thing here that isn't just refactoring is that in a few places we
> now check with errors with "if (err)" instead of "if (err < 0)". All
> errors are expected to be negative so this is not expected to have any
> impact. The _multi code internally considers anything non-zero to be
> an error so this just makes things consistent.
>
> It can also be noted that hx83102_prepare() has a mix of things that
> can take advantage of _multi calls and things that can't. The cleanest
> seemed to be to use the multi_ctx still but consistently use the
> "accum_err" variable for error returns, though that's definitely a
> style decision with pros and cons.
>
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
>
> drivers/gpu/drm/panel/panel-himax-hx83102.c | 92 +++++++--------------
> 1 file changed, 28 insertions(+), 64 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
Il 17/05/24 23:36, Douglas Anderson ha scritto:
> The mipi_dsi_dcs_nop() function returns an error but we weren't
> checking it in boe_panel_prepare(). Add a check. This is highly
> unlikely to matter in practice. If the NOP failed then likely later
> MIPI commands would fail too.
>
> Found by code inspection.
>
> Fixes: 812562b8d881 ("drm/panel: boe-tv101wum-nl6: Fine tune the panel power sequence")
> Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Hi,
On Fri, 17 May 2024 14:36:35 -0700, Douglas Anderson wrote:
> This series is pretty much just addressing a few minor error handling
> bugs that I noticed recently while reviewing some panel patches. For
> the most part these are all old issues.
>
> This also adjusts the new himax-hx83102 in a similar way that Dmitry
> did in his recent series that included commit f79d6d28d8fe
> ("drm/mipi-dsi: wrap more functions for streamline handling"). His
> series handled the panel driver that himax-hx83102 forked from but not
> himax-hx83102.
>
> [...]
Thanks, Applied to https://gitlab.freedesktop.org/drm/misc/kernel.git (drm-misc-next)
[1/8] drm/panel: himax-hx8394: Handle errors from mipi_dsi_dcs_set_display_on() better
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/cc2db2ef8d9eebc0df03808ac0dadbdb96733499
[2/8] drm/panel: boe-tv101wum-nl6: If prepare fails, disable GPIO before regulators
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/587c48f622374e5d47b1d515c6006a4df4dee882
[3/8] drm/panel: boe-tv101wum-nl6: Check for errors on the NOP in prepare()
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/6320b9199dd99622668649c234d4e8a99e44a9c8
[4/8] drm/panel: ilitek-ili9882t: If prepare fails, disable GPIO before regulators
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/554c00181968d43426bfe68c86541b89265075de
[5/8] drm/panel: ilitek-ili9882t: Check for errors on the NOP in prepare()
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/6a7bd6cde73f0fb7e5faa964dbdeb45b55c64698
[6/8] drm/panel: himax-hx83102: If prepare fails, disable GPIO before regulators
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/509eaa8aeee64bd7a41ca53d8728e497a9991074
[7/8] drm/panel: himax-hx83102: Check for errors on the NOP in prepare()
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/676a079fb3be66aed12cf40f236c77b8e7c189c3
[8/8] drm/panel: himax-hx83102: use wrapped MIPI DCS functions
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/a2ab7cb169da38757323929f7b3b4cf396ec53b5
--
Neil