drm/panel: mantix panel reset fixes
Subsystems (5)
Guido Günther
maintainercover drm/panel: mantix panel reset fixes
Posting as RFC since I'm not sure how to handle the bindings, please see below.
The first patch in this series fixes dereferencing a NULL mode in the error path. The second one extends the resets to not only reset RESX but also TP_RSTN since otherwise the display will stay completely blank. I didn't spot that before initial submission since the reset line was bound to the touch controller and although that failed to probe it came far enough to deassert the reset line (at an arbitrary point in time during boot) and hence we got a picture. Since touch and panel are on the same IC they're not completely independent and i might have to turn the whole thing into an MFD at some point but this series gets the panel to a reliably working state on boot and on fb blank/unblank.
Since the reset-gpios are active low we can deassert in prepare and assert in unprepare simplifying the code making sure lines are kept low when the panel is off.
As for the bindings it seems I can't override the maxItems of reset-gpios since that is already set in bindings/display/panel/panel-common.yaml to maxItems:
1. Is there a way to do that instead of side stepping the issue and using a different name or using enable-gpios for one of them or forcing everyone else to set maxItems: 1`?
The binding were not part of a stable kernel so I hope it's okay to not worry about backward compatibility.
Guido Günther (3):
drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset
dt-binding: display: Require two rests on mantix panel
.../display/panel/mantix,mlaf057we51-x.yaml | 7 +++- .../gpu/drm/panel/panel-mantix-mlaf057we51.c | 39 ++++++++++++------- 2 files changed, 30 insertions(+), 16 deletions(-)
Signature · 6 lines
2.26.2 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Guido Günther
maintainerRFC PATCH 1/3 drm/panel: mantix: Don't dereference NULL mode
Don't dereference mode which was just NULL checked.
--- drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c +1 -1
@@ -204,7 +204,7 @@ static int mantix_get_modes(struct drm_panel *panel,if (!mode) {dev_err (ctx->dev,"Failed to add mode %ux%u@%u\n" , default_mode.hdisplay, default_mode.vdisplay, -drm_mode_vrefresh (mode)); +drm_mode_vrefresh (&default_mode));return -ENOMEM ; }
Signature · 6 lines
2.26.2 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Guido Günther
maintainerRFC PATCH 2/3 drm/panel: mantix: Fix panel reset
In reply to Guido GüntherThe mantix panel needs two reset lines (RESX and TP_RSTN) deasserted to output an image. Only deasserting RESX is not enough and the display will stay blank. Deassert in prepare() and assert in unprepare() to keep device held in reset when off.
--- .../gpu/drm/panel/panel-mantix-mlaf057we51.c | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-)
drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c +24 -13
@@ -23,10 +23,13 @@#define MANTIX_CMD_OTP_STOP_RELOAD_MIPI 0x41#define MANTIX_CMD_INT_CANCEL 0x4C +#define MANTIX_NUM_RESETS 2 +struct mantix {struct device *dev;struct drm_panel panel; -struct gpio_desc *reset_gpio; +/* RESX and TP_RSTN */ +struct gpio_descs *reset_gpios;struct regulator *avdd;struct regulator *avee;
@@ -122,8 +125,13 @@ static int mantix_disable(struct drm_panel *panel)static int mantix_unprepare (struct drm_panel *panel) { +DECLARE_BITMAP (asserted_values,BITS_PER_TYPE (1 ));struct mantix *ctx =panel_to_mantix (panel); +gpiod_set_array_value_cansleep (ctx->reset_gpios->ndescs, + ctx->reset_gpios->desc, + ctx->reset_gpios->info, asserted_values); +regulator_disable (ctx->avee);regulator_disable (ctx->avdd);/* T11 */
@@ -137,6 +145,7 @@ static int mantix_unprepare(struct drm_panel *panel)static int mantix_prepare (struct drm_panel *panel) { +DECLARE_BITMAP (deasserted_values,BITS_PER_TYPE (0 ));struct mantix *ctx =panel_to_mantix (panel);int ret;
@@ -165,14 +174,11 @@ static int mantix_prepare(struct drm_panel *panel)return ret; } -/* T3+T5 */ -usleep_range (10000 ,12000 ); - -gpiod_set_value_cansleep (ctx->reset_gpio,1 ); -usleep_range (5150 ,7000 ); - -gpiod_set_value_cansleep (ctx->reset_gpio,0 ); - +/* T3 + T4 + time for voltage to become stable: */ +usleep_range (6000 ,7000 ); +gpiod_set_array_value_cansleep (ctx->reset_gpios->ndescs, + ctx->reset_gpios->desc, + ctx->reset_gpios->info, deasserted_values);/* T6 */ msleep (50 );
@@ -236,10 +242,15 @@ static int mantix_probe(struct mipi_dsi_device *dsi)if (!ctx)return -ENOMEM ; - ctx->reset_gpio =devm_gpiod_get (dev,"reset" ,GPIOD_OUT_LOW ); -if (IS_ERR (ctx->reset_gpio)) { -dev_err (dev,"cannot get reset gpio\n" ); -return PTR_ERR (ctx->reset_gpio); + ctx->reset_gpios =devm_gpiod_get_array (dev,"reset" ,GPIOD_OUT_HIGH ); +if (IS_ERR (ctx->reset_gpios)) { +dev_err (dev,"cannot get reset gpios\n" ); +return PTR_ERR (ctx->reset_gpios); + } + +if (ctx->reset_gpios->ndescs !=MANTIX_NUM_RESETS ) { +dev_err (dev,"Need exactly %d reset-gpios\n" ,MANTIX_NUM_RESETS ); +return -EINVAL ; }mipi_dsi_set_drvdata (dsi, ctx);
Signature · 1 line
2.26.2
Guido Günther
maintainerRFC PATCH 3/3 dt-binding: display: Require two rests on mantix panel
In reply to Guido GüntherWe need to reset both for the panel to show an image.
--- .../bindings/display/panel/mantix,mlaf057we51-x.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml +5 -2
@@ -35,7 +35,9 @@ properties: vddi-supply: description: 1.8V I/O voltage supply - reset-gpios: true + reset-gpios: + minItems: 2 + maxItems: 2 backlight: true
@@ -62,7 +64,8 @@ examples: avdd-supply = <®_avdd>; avee-supply = <®_avee>; vddi-supply = <®_1v8_p>; - reset-gpios = <&gpio1 29 GPIO_ACTIVE_LOW>; + reset-gpios = <&gpio1 29 GPIO_ACTIVE_LOW>, + <&gpio1 24 GPIO_ACTIVE_LOW>; backlight = <&backlight>; }; };
Signature · 1 line
2.26.2
Re: [RFC PATCH v1 3/3] dt-binding: display: Require two rests on mantix panel
Hi Guido,
Quoted 2 lines · Guido Günther
We need to reset both for the panel to show an image.
There is a typo in "resets" in the Subject line.
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Guido Günther
maintainerRe: [RFC PATCH v1 3/3] dt-binding: display: Require two rests on mantix panel
Hi,
Quoted 7 lines · Fabio Estevam
Hi Guido,
Quoted 2 lines · Guido Günther
We need to reset both for the panel to show an image.
There is a typo in "resets" in the Subject line.
Fix queued for v2. Thanks!
-- Guido
Re: [RFC PATCH v1 3/3] dt-binding: display: Require two rests on mantix panel
In reply to Guido GüntherHi Guido.
Quoted 12 lines · Guido Günther
We need to reset both for the panel to show an image.
Signed-off-by Guido Günther <[email protected]>--- .../bindings/display/panel/mantix,mlaf057we51-x.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml +0
-0
Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml properties: · line 40
Quoted 7 lines
Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml +3
-1@@ -35,7 +35,9 @@ properties: vddi-supply: description: 1.8V I/O voltage supply - reset-gpios: true + reset-gpios: + minItems: 2 + maxItems: 2
reset-gpios is, as you already wrote, defined in panel-common.yaml. Do not try to change it here.
It would be much better, I think, to introduce a mantix,reset-gpios property.
This would avoid that we had two different reset-gpios definitions.
Sam
Quoted 15 lines
Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml +2
-1backlight: true@@ -62,7 +64,8 @@ examples: avdd-supply = <®_avdd>; avee-supply = <®_avee>; vddi-supply = <®_1v8_p>; - reset-gpios = <&gpio1 29 GPIO_ACTIVE_LOW>; + reset-gpios = <&gpio1 29 GPIO_ACTIVE_LOW>, + <&gpio1 24 GPIO_ACTIVE_LOW>; backlight = <&backlight>; }; };Signature · 1 line
2.26.2
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v1 3/3] dt-binding: display: Require two rests on mantix panel
Quoted 28 lines · Sam Ravnborg
Hi Guido.
Quoted 12 lines · Guido Günther
We need to reset both for the panel to show an image.
Signed-off-by Guido Günther <[email protected]>--- .../bindings/display/panel/mantix,mlaf057we51-x.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml +0
-0Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml properties: · line 40
Quoted 7 lines
Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml +3
-1@@ -35,7 +35,9 @@ properties: vddi-supply: description: 1.8V I/O voltage supply - reset-gpios: true + reset-gpios: + minItems: 2 + maxItems: 2reset-gpios is, as you already wrote, defined in
panel-common.yaml. Do not try to change it here.
It would be much better, I think, to introduce a mantix,reset-gpios property.
Quoted 1 line
This would avoid that we had two different reset-gpios definitions.
My understanding is that this will only override the defaults defined in panel-common.yaml. So everything else about the panel-common.yaml#/properties/reset-gpios definition remains valid and all this does is say that when this matched on "mantix,mlaf057we51-x", then the "reset-gpios" property is expected to contain two entries as opposed to the single entry that is expected by default.
Thierry
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Guido Günther
maintainerRe: [RFC PATCH v1 3/3] dt-binding: display: Require two rests on mantix panel
Hi Thierry,
Quoted 29 lines · Thierry Reding
Quoted 28 lines · Sam Ravnborg
Hi Guido.
Quoted 12 lines · Guido Günther
We need to reset both for the panel to show an image.
Signed-off-by Guido Günther <[email protected]>--- .../bindings/display/panel/mantix,mlaf057we51-x.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml +0
-0Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml properties: · line 40
Quoted 7 lines
Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml +3
-1@@ -35,7 +35,9 @@ properties: vddi-supply: description: 1.8V I/O voltage supply - reset-gpios: true + reset-gpios: + minItems: 2 + maxItems: 2reset-gpios is, as you already wrote, defined in
panel-common.yaml. Do not try to change it here.
It would be much better, I think, to introduce a mantix,reset-gpios property.
Quoted 8 lines
Quoted 1 line
This would avoid that we had two different reset-gpios definitions.
My understanding is that this will only override the defaults defined in
panel-common.yaml. So everything else about the panel-common.yaml#/properties/reset-gpios definition remains valid and all this does is say that when this matched on "mantix,mlaf057we51-x", then the "reset-gpios" property is expected to contain two entries as opposed to the single entry that is expected by default.
I thought so too but I get:
linux/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.example.dt.yaml: panel@0: reset-gpios: [[4294967295, 29, 1], [4294967295, 24, 1]] is too long
From schema: /.../linux/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
It seems to bepossible to *extend* reset-gpios though because when i use an example like:
reset-gpios = <&gpio1 29 GPIO_ACTIVE_LOW>,
<&gpio1 24 GPIO_ACTIVE_LOW>;i correctly get
linux/linux/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.example.dt.yaml: panel@0: reset-gpios: [[4294967295, 29, 1]] is too short
From schema: /.../linux/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
so the minItems: 2 is being picked up. This mens i can add properties but i can't overwrite the existing maxItems: 2 - that's why i was wondering how that is supposed to work (if properties should be overrideable from common files or not).
Cheers,
-- Guido
Quoted 2 lines
Thierry
Rob Herring
maintainerRe: [RFC PATCH v1 3/3] dt-binding: display: Require two rests on mantix panel
In reply to Sam RavnborgQuoted 18 lines · Sam Ravnborg
Hi Guido.
Quoted 15 lines · Guido Günther
We need to reset both for the panel to show an image.
Signed-off-by Guido Günther <[email protected]>--- .../bindings/display/panel/mantix,mlaf057we51-x.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml +0
-0@@ -35,7 +35,9 @@ properties: vddi-supply: description: 1.8V I/O voltage supply
Quoted 9 lines
Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml properties: · line 40
Quoted 4 lines
Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml +3
-1- reset-gpios: true + reset-gpios: + minItems: 2 + maxItems: 2reset-gpios is, as you already wrote, defined in
panel-common.yaml. Do not try to change it here.
It would be much better, I think, to introduce a mantix,reset-gpios property.
Yes.
You also need to define what each reset entry corresponds to as the assertion/deassertion order could be important. You might just do 2 properties with <pinname>-gpios.
Rob