Skip to contentSkip to conversation

drm/panel: mantix panel reset fixes

PATCH In review Guido Günther 3 participants 15 messages Oct 13, 2020 – Oct 23, 2020

Subsystems (5)
lore
Share & tools
lore

Guido Günther

author maintainer
#0 lore raw

v2 cover drm/panel: mantix panel reset fixes

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.

The binding were not part of a stable kernel so I hope it's okay to not worry about backward compatibility.

Changes from v1:
- As per review comments by Fabio Estevam
https://lore.kernel.org/dri-devel/CAOMZO5B5ECcConvKej=RcaF8wvOxgq7nUzKJ-ad0aSAOzUqtbQ@mail.gmail.com/
- Fix typo in commit messages
- As per review comments by Rob Herring
https://lore.kernel.org/dri-devel/20200929174624.GA832332@bogus/
- Don't use an array of reset lines

Guido Günther (3):
drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset
dt-binding: display: Require two resets on mantix panel

 .../display/panel/mantix,mlaf057we51-x.yaml   |  4 +++
 .../gpu/drm/panel/panel-mantix-mlaf057we51.c  | 25 +++++++++++++------
 2 files changed, 21 insertions(+), 8 deletions(-)
Signature · 1 line
2.28.0
author maintainer
#1 lore raw

PATCH v2 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
index 3482e28e30fc..4a7fbf64bb7a 100644
@@ -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.28.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author maintainer
#2 lore raw

PATCH v2 2/3 drm/panel: mantix: Fix panel reset

In reply to Guido Günther

The 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  | 23 +++++++++++++------
 1 file changed, 16 insertions(+), 7 deletions(-)
drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c +16 -7
index 4a7fbf64bb7a..0c5f22e95c2d 100644
@@ -26,7 +26,9 @@
 struct mantix {
 	struct device *dev;
 	struct drm_panel panel;
+
 	struct gpio_desc *reset_gpio;
+	struct gpio_desc *tp_rstn_gpio;
 
 	struct regulator *avdd;
 	struct regulator *avee;
@@ -124,6 +126,10 @@ static int mantix_unprepare(struct drm_panel *panel)
 {
 	struct mantix *ctx = panel_to_mantix(panel);
 
+	gpiod_set_value_cansleep(ctx->tp_rstn_gpio, 1);
+	usleep_range(5000, 6000);
+	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+
 	regulator_disable(ctx->avee);
 	regulator_disable(ctx->avdd);
 	/* T11 */
@@ -165,13 +171,10 @@ 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);
-
+	/* T3 + T4 + time for voltage to become stable: */
+	usleep_range(6000, 7000);
 	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+	gpiod_set_value_cansleep(ctx->tp_rstn_gpio, 0);
 
 	/* T6 */
 	msleep(50);
@@ -236,12 +239,18 @@ static int mantix_probe(struct mipi_dsi_device *dsi)
 	if (!ctx)
 		return -ENOMEM;
 
-	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(ctx->reset_gpio)) {
 		dev_err(dev, "cannot get reset gpio\n");
 		return PTR_ERR(ctx->reset_gpio);
 	}
 
+	ctx->tp_rstn_gpio = devm_gpiod_get(dev, "mantix,tp-rstn", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->tp_rstn_gpio)) {
+		dev_err(dev, "cannot get tp-rstn gpio\n");
+		return PTR_ERR(ctx->tp_rstn_gpio);
+	}
+
 	mipi_dsi_set_drvdata(dsi, ctx);
 	ctx->dev = dev;
 
Signature · 1 line
2.28.0
author maintainer
#3 lore raw

PATCH v2 3/3 dt-binding: display: Require two resets on mantix panel

In reply to Guido Günther

We need to reset both for the panel to show an image.

---
 .../bindings/display/panel/mantix,mlaf057we51-x.yaml          | 4 ++++
 1 file changed, 4 insertions(+)
Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml +4 -0
index 937323cc9aaa..51f423297ec8 100644
@@ -37,6 +37,9 @@ properties:
 
   reset-gpios: true
 
+  'mantix,tp-rstn-gpios':
+    description: second reset line that triggers DSI config load
+
   backlight: true
 
 required:
@@ -63,6 +66,7 @@ examples:
             avee-supply = <&reg_avee>;
             vddi-supply = <&reg_1v8_p>;
             reset-gpios = <&gpio1 29 GPIO_ACTIVE_LOW>;
+            mantix,tp-rstn-gpios = <&gpio1 24 GPIO_ACTIVE_LOW>;
             backlight = <&backlight>;
         };
     };
Signature · 1 line
2.28.0
#4 lore raw
In reply to Guido Günther

Hi Guido.

Quoted 27 lines · Guido Günther

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.

The binding were not part of a stable kernel so I hope it's okay to not worry about backward compatibility.

Changes from v1:
- As per review comments by Fabio Estevam
https://lore.kernel.org/dri-devel/CAOMZO5B5ECcConvKej=RcaF8wvOxgq7nUzKJ-ad0aSAOzUqtbQ@mail.gmail.com/
- Fix typo in commit messages
- As per review comments by Rob Herring
https://lore.kernel.org/dri-devel/20200929174624.GA832332@bogus/
- Don't use an array of reset lines

Quoted 4 lines

Guido Günther (3):
drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset
dt-binding: display: Require two resets on mantix panel

All applied to drm-misc-next and pushed out. And then I remembered you had commit right - sigh.

Sam

Quoted 12 lines
 .../display/panel/mantix,mlaf057we51-x.yaml   |  4 +++
 .../gpu/drm/panel/panel-mantix-mlaf057we51.c  | 25 +++++++++++++------
 2 files changed, 21 insertions(+), 8 deletions(-)
Signature · 6 lines
2.28.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author maintainer
#5 lore raw

Hi Sam,

Quoted 2 lines · Sam Ravnborg

Hi Guido.
On Tue, Oct 13, 2020 at 12:32:45PM +0200, Guido Günther wrote:

[..snip..]

Quoted 9 lines
Quoted 9 lines

Changes from v1:
- As per review comments by Fabio Estevam
https://lore.kernel.org/dri-devel/CAOMZO5B5ECcConvKej=RcaF8wvOxgq7nUzKJ-ad0aSAOzUqtbQ@mail.gmail.com/
- Fix typo in commit messages
- As per review comments by Rob Herring
https://lore.kernel.org/dri-devel/20200929174624.GA832332@bogus/
- Don't use an array of reset lines

Quoted 7 lines
Quoted 4 lines

Guido Günther (3):
drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset
dt-binding: display: Require two resets on mantix panel

All applied to drm-misc-next and pushed out. And then I remembered you had commit right - sigh.

Thanks! Is there any special care needed to get that into 5.10? The driver landed there in 72967d5616d3f0c714f8eb6c4e258179a9031c45. Cheers,
-- Guido

Quoted 16 lines

Sam

Quoted 12 lines
 .../display/panel/mantix,mlaf057we51-x.yaml   |  4 +++
 .../gpu/drm/panel/panel-mantix-mlaf057we51.c  | 25 +++++++++++++------
 2 files changed, 21 insertions(+), 8 deletions(-)
Signature · 6 lines
2.28.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#6 lore raw

Hi Guido.

Quoted 19 lines · Guido Günther

Hi Sam,

Quoted 2 lines · Sam Ravnborg

Hi Guido.
On Tue, Oct 13, 2020 at 12:32:45PM +0200, Guido Günther wrote:

[..snip..]

Quoted 14 lines
Quoted 13 lines

Changes from v1:
- As per review comments by Fabio Estevam
https://lore.kernel.org/dri-devel/CAOMZO5B5ECcConvKej=RcaF8wvOxgq7nUzKJ-ad0aSAOzUqtbQ@mail.gmail.com/
- Fix typo in commit messages
- As per review comments by Rob Herring
https://lore.kernel.org/dri-devel/20200929174624.GA832332@bogus/
- Don't use an array of reset lines

Guido Günther (3):
drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset
dt-binding: display: Require two resets on mantix panel

Quoted 5 lines
Quoted 2 lines

All applied to drm-misc-next and pushed out. And then I remembered you had commit right - sigh.

Thanks! Is there any special care needed to get that into 5.10? The driver landed there in 72967d5616d3f0c714f8eb6c4e258179a9031c45.

As the patches was applied to drm-misc-next the easiet path would be to cherry-pick them and apply to drm-misc-fixes.
dim has cherry-pick support - try to use it rahter than doing it by hand.

When you apply to drm-misc-fixes include a Fixes: tag so the tooling will pick the patches automagically.

In hindsight the patches should have carried a Fixes: tag from a start and should have been applied to drm-misc-fixes from a start too.

I have done something like above once or twice but anyway reach out if you have questions. Or ask at #dri-devel.

Sam
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author maintainer
#7 lore raw

Hi Sam,

Quoted 25 lines · Sam Ravnborg

Hi Guido.

Quoted 22 lines · Guido Günther

Hi Sam,

Quoted 2 lines · Sam Ravnborg

Hi Guido.
On Tue, Oct 13, 2020 at 12:32:45PM +0200, Guido Günther wrote:

[..snip..]

Quoted 16 lines
Quoted 9 lines

Changes from v1:
- As per review comments by Fabio Estevam
https://lore.kernel.org/dri-devel/CAOMZO5B5ECcConvKej=RcaF8wvOxgq7nUzKJ-ad0aSAOzUqtbQ@mail.gmail.com/
- Fix typo in commit messages
- As per review comments by Rob Herring
https://lore.kernel.org/dri-devel/20200929174624.GA832332@bogus/
- Don't use an array of reset lines

Quoted 4 lines

Guido Günther (3):
drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset
dt-binding: display: Require two resets on mantix panel

All applied to drm-misc-next and pushed out. And then I remembered you had commit right - sigh.

Quoted 7 lines
Quoted 2 lines

Thanks! Is there any special care needed to get that into 5.10? The driver landed there in 72967d5616d3f0c714f8eb6c4e258179a9031c45.

As the patches was applied to drm-misc-next the easiet path would be to cherry-pick them and apply to drm-misc-fixes.
dim has cherry-pick support - try to use it rahter than doing it by hand.

Does that require any further ACKs or just cherry-pick and push out?

Quoted 2 lines

When you apply to drm-misc-fixes include a Fixes: tag so the tooling will pick the patches automagically.

Will do. It was unfortunate that the initial ones didn't get them but i think the drm merge happened while v2 was out on the list. I'll try add that relevant information to the cover letter in the future.

Cheers,
-- Guido

Quoted 9 lines

In hindsight the patches should have carried a Fixes: tag from a start and should have been applied to drm-misc-fixes from a start too.

I have done something like above once or twice but anyway reach out if you have questions. Or ask at #dri-devel.

Sam

#8 lore raw

Hi Guido

Quoted 31 lines · Guido Günther

Hi Sam,

Quoted 28 lines · Sam Ravnborg

Hi Guido.

Quoted 24 lines · Guido Günther

Hi Sam,

Quoted 2 lines · Sam Ravnborg

Hi Guido.
On Tue, Oct 13, 2020 at 12:32:45PM +0200, Guido Günther wrote:

[..snip..]

Quoted 9 lines
Quoted 9 lines

Changes from v1:
- As per review comments by Fabio Estevam
https://lore.kernel.org/dri-devel/CAOMZO5B5ECcConvKej=RcaF8wvOxgq7nUzKJ-ad0aSAOzUqtbQ@mail.gmail.com/
- Fix typo in commit messages
- As per review comments by Rob Herring
https://lore.kernel.org/dri-devel/20200929174624.GA832332@bogus/
- Don't use an array of reset lines

Quoted 7 lines
Quoted 4 lines

Guido Günther (3):
drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset
dt-binding: display: Require two resets on mantix panel

All applied to drm-misc-next and pushed out. And then I remembered you had commit right - sigh.

Thanks! Is there any special care needed to get that into 5.10? The driver landed there in 72967d5616d3f0c714f8eb6c4e258179a9031c45.

Quoted 6 lines
Quoted 4 lines

As the patches was applied to drm-misc-next the easiet path would be to cherry-pick them and apply to drm-misc-fixes.
dim has cherry-pick support - try to use it rahter than doing it by hand.

Does that require any further ACKs or just cherry-pick and push out?

To the best of my knowledge just cherry-pick, test and push out.

Sam

Quoted 24 lines
Quoted 3 lines

When you apply to drm-misc-fixes include a Fixes: tag so the tooling will pick the patches automagically.

Will do. It was unfortunate that the initial ones didn't get them but i think the drm merge happened while v2 was out on the list. I'll try add that relevant information to the cover letter in the future.

Cheers,
-- Guido

Quoted 9 lines

In hindsight the patches should have carried a Fixes: tag from a start and should have been applied to drm-misc-fixes from a start too.

I have done something like above once or twice but anyway reach out if you have questions. Or ask at #dri-devel.

Sam

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author maintainer
#9 lore raw

Hi,

Quoted 32 lines · Sam Ravnborg

Hi Guido

Quoted 31 lines · Guido Günther

Hi Sam,

Quoted 28 lines · Sam Ravnborg

Hi Guido.

Quoted 24 lines · Guido Günther

Hi Sam,

Quoted 2 lines · Sam Ravnborg

Hi Guido.
On Tue, Oct 13, 2020 at 12:32:45PM +0200, Guido Günther wrote:

[..snip..]

Quoted 9 lines
Quoted 9 lines

Changes from v1:
- As per review comments by Fabio Estevam
https://lore.kernel.org/dri-devel/CAOMZO5B5ECcConvKej=RcaF8wvOxgq7nUzKJ-ad0aSAOzUqtbQ@mail.gmail.com/
- Fix typo in commit messages
- As per review comments by Rob Herring
https://lore.kernel.org/dri-devel/20200929174624.GA832332@bogus/
- Don't use an array of reset lines

Quoted 7 lines
Quoted 4 lines

Guido Günther (3):
drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset
dt-binding: display: Require two resets on mantix panel

All applied to drm-misc-next and pushed out. And then I remembered you had commit right - sigh.

Thanks! Is there any special care needed to get that into 5.10? The driver landed there in 72967d5616d3f0c714f8eb6c4e258179a9031c45.

Quoted 7 lines
Quoted 6 lines
Quoted 4 lines

As the patches was applied to drm-misc-next the easiet path would be to cherry-pick them and apply to drm-misc-fixes.
dim has cherry-pick support - try to use it rahter than doing it by hand.

Does that require any further ACKs or just cherry-pick and push out?

To the best of my knowledge just cherry-pick, test and push out.

drm-misc-fixes does not yet have the corresponding code so i assume i have to wait for back merge of 5.10-rc1 ? At least

https://drm.pages.freedesktop.org/maintainer-tools/drm-misc.html

seems to indicate that.
Cheers,
-- Guido

Quoted 28 lines

Sam

Quoted 24 lines
Quoted 3 lines

When you apply to drm-misc-fixes include a Fixes: tag so the tooling will pick the patches automagically.

Will do. It was unfortunate that the initial ones didn't get them but i think the drm merge happened while v2 was out on the list. I'll try add that relevant information to the cover letter in the future.

Cheers,
-- Guido

Quoted 9 lines

In hindsight the patches should have carried a Fixes: tag from a start and should have been applied to drm-misc-fixes from a start too.

I have done something like above once or twice but anyway reach out if you have questions. Or ask at #dri-devel.

Sam

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

#10 lore raw
In reply to Sam Ravnborg
Quoted 25 lines · Sam Ravnborg

Hi Guido.

Quoted 22 lines · Guido Günther

Hi Sam,

Quoted 2 lines · Sam Ravnborg

Hi Guido.
On Tue, Oct 13, 2020 at 12:32:45PM +0200, Guido Günther wrote:

[..snip..]

Quoted 16 lines
Quoted 9 lines

Changes from v1:
- As per review comments by Fabio Estevam
https://lore.kernel.org/dri-devel/CAOMZO5B5ECcConvKej=RcaF8wvOxgq7nUzKJ-ad0aSAOzUqtbQ@mail.gmail.com/
- Fix typo in commit messages
- As per review comments by Rob Herring
https://lore.kernel.org/dri-devel/20200929174624.GA832332@bogus/
- Don't use an array of reset lines

Quoted 4 lines

Guido Günther (3):
drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset
dt-binding: display: Require two resets on mantix panel

All applied to drm-misc-next and pushed out. And then I remembered you had commit right - sigh.

Quoted 7 lines
Quoted 2 lines

Thanks! Is there any special care needed to get that into 5.10? The driver landed there in 72967d5616d3f0c714f8eb6c4e258179a9031c45.

As the patches was applied to drm-misc-next the easiet path would be to cherry-pick them and apply to drm-misc-fixes.
dim has cherry-pick support - try to use it rahter than doing it by hand.

drm-misc-next-fixes while we're between freeze and merge window end:

https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#where-do-i-apply-my-patch

Cheers, Daniel

Quoted 12 lines

When you apply to drm-misc-fixes include a Fixes: tag so the tooling will pick the patches automagically.

In hindsight the patches should have carried a Fixes: tag from a start and should have been applied to drm-misc-fixes from a start too.

I have done something like above once or twice but anyway reach out if you have questions. Or ask at #dri-devel.

Sam

Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author maintainer
#11 lore raw

Hi Daniel, Sam,

Quoted 29 lines · Daniel Vetter
Quoted 28 lines · Sam Ravnborg

Hi Guido.

Quoted 24 lines · Guido Günther

Hi Sam,

Quoted 2 lines · Sam Ravnborg

Hi Guido.
On Tue, Oct 13, 2020 at 12:32:45PM +0200, Guido Günther wrote:

[..snip..]

Quoted 9 lines
Quoted 9 lines

Changes from v1:
- As per review comments by Fabio Estevam
https://lore.kernel.org/dri-devel/CAOMZO5B5ECcConvKej=RcaF8wvOxgq7nUzKJ-ad0aSAOzUqtbQ@mail.gmail.com/
- Fix typo in commit messages
- As per review comments by Rob Herring
https://lore.kernel.org/dri-devel/20200929174624.GA832332@bogus/
- Don't use an array of reset lines

Quoted 7 lines
Quoted 4 lines

Guido Günther (3):
drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset
dt-binding: display: Require two resets on mantix panel

All applied to drm-misc-next and pushed out. And then I remembered you had commit right - sigh.

Thanks! Is there any special care needed to get that into 5.10? The driver landed there in 72967d5616d3f0c714f8eb6c4e258179a9031c45.

Quoted 8 lines
Quoted 4 lines

As the patches was applied to drm-misc-next the easiet path would be to cherry-pick them and apply to drm-misc-fixes.
dim has cherry-pick support - try to use it rahter than doing it by hand.

drm-misc-next-fixes while we're between freeze and merge window end:

https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#where-do-i-apply-my-patch

Great. Thanks for the pointer, that works. Now i get:

  $ ../maintainer-tools/dim push --dry-run
  dim: 3532f0117258 ("dt-binding: display: Require two resets on mantix panel"): mandatory review missing.
  dim: c90f95ad6d05 ("drm/panel: mantix: Fix panel reset"): mandatory review missing.
  dim: 8b557f793e69 ("drm/panel: mantix: Don't dereference NULL mode"): mandatory review missing.
  dim: ERROR: issues in commits detected, aborting

and in fact there's only Signed-off-by's on it:

Sam, I assume your Signed-off-by's should have been Reviewed-by ? O.k. to fix that up in the commit message before pushing to drm-misc-next?

Cheers,
-- Guido

Quoted 21 lines

Cheers, Daniel

Quoted 12 lines

When you apply to drm-misc-fixes include a Fixes: tag so the tooling will pick the patches automagically.

In hindsight the patches should have carried a Fixes: tag from a start and should have been applied to drm-misc-fixes from a start too.

I have done something like above once or twice but anyway reach out if you have questions. Or ask at #dri-devel.

Sam

Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
#12 lore raw

Hi Guido.

Quoted 50 lines · Guido Günther

Hi Daniel, Sam,

Quoted 29 lines · Daniel Vetter
Quoted 28 lines · Sam Ravnborg

Hi Guido.

Quoted 24 lines · Guido Günther

Hi Sam,

Quoted 2 lines · Sam Ravnborg

Hi Guido.
On Tue, Oct 13, 2020 at 12:32:45PM +0200, Guido Günther wrote:

[..snip..]

Quoted 9 lines
Quoted 9 lines

Changes from v1:
- As per review comments by Fabio Estevam
https://lore.kernel.org/dri-devel/CAOMZO5B5ECcConvKej=RcaF8wvOxgq7nUzKJ-ad0aSAOzUqtbQ@mail.gmail.com/
- Fix typo in commit messages
- As per review comments by Rob Herring
https://lore.kernel.org/dri-devel/20200929174624.GA832332@bogus/
- Don't use an array of reset lines

Quoted 7 lines
Quoted 4 lines

Guido Günther (3):
drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset
dt-binding: display: Require two resets on mantix panel

All applied to drm-misc-next and pushed out. And then I remembered you had commit right - sigh.

Thanks! Is there any special care needed to get that into 5.10? The driver landed there in 72967d5616d3f0c714f8eb6c4e258179a9031c45.

Quoted 8 lines
Quoted 4 lines

As the patches was applied to drm-misc-next the easiet path would be to cherry-pick them and apply to drm-misc-fixes.
dim has cherry-pick support - try to use it rahter than doing it by hand.

drm-misc-next-fixes while we're between freeze and merge window end:

https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#where-do-i-apply-my-patch

Great. Thanks for the pointer, that works. Now i get:

  $ ../maintainer-tools/dim push --dry-run
  dim: 3532f0117258 ("dt-binding: display: Require two resets on mantix panel"): mandatory review missing.
  dim: c90f95ad6d05 ("drm/panel: mantix: Fix panel reset"): mandatory review missing.
  dim: 8b557f793e69 ("drm/panel: mantix: Don't dereference NULL mode"): mandatory review missing.
  dim: ERROR: issues in commits detected, aborting

and in fact there's only Signed-off-by's on it:

Quoted 8 lines

Sam, I assume your Signed-off-by's should have been Reviewed-by ? O.k. to fix that up in the commit message before pushing to drm-misc-next?

Fine to add my r-b.

For patches I push I always read the patch - but seldom claim that I review them. It seems a bit implicit I think - but maybe this is just bad practice on my side.

Then I also avoid the endless discussion with myself it what I did warrant an a-b or r-b.

Sam

author maintainer
#13 lore raw

Hi Sam,

Quoted 58 lines · Sam Ravnborg

Hi Guido.

Quoted 55 lines · Guido Günther

Hi Daniel, Sam,

Quoted 29 lines · Daniel Vetter
Quoted 28 lines · Sam Ravnborg

Hi Guido.

Quoted 24 lines · Guido Günther

Hi Sam,

Quoted 2 lines · Sam Ravnborg

Hi Guido.
On Tue, Oct 13, 2020 at 12:32:45PM +0200, Guido Günther wrote:

[..snip..]

Quoted 9 lines
Quoted 9 lines

Changes from v1:
- As per review comments by Fabio Estevam
https://lore.kernel.org/dri-devel/CAOMZO5B5ECcConvKej=RcaF8wvOxgq7nUzKJ-ad0aSAOzUqtbQ@mail.gmail.com/
- Fix typo in commit messages
- As per review comments by Rob Herring
https://lore.kernel.org/dri-devel/20200929174624.GA832332@bogus/
- Don't use an array of reset lines

Quoted 7 lines
Quoted 4 lines

Guido Günther (3):
drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset
dt-binding: display: Require two resets on mantix panel

All applied to drm-misc-next and pushed out. And then I remembered you had commit right - sigh.

Thanks! Is there any special care needed to get that into 5.10? The driver landed there in 72967d5616d3f0c714f8eb6c4e258179a9031c45.

Quoted 8 lines
Quoted 4 lines

As the patches was applied to drm-misc-next the easiet path would be to cherry-pick them and apply to drm-misc-fixes.
dim has cherry-pick support - try to use it rahter than doing it by hand.

drm-misc-next-fixes while we're between freeze and merge window end:

https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#where-do-i-apply-my-patch

Great. Thanks for the pointer, that works. Now i get:

  $ ../maintainer-tools/dim push --dry-run
  dim: 3532f0117258 ("dt-binding: display: Require two resets on mantix panel"): mandatory review missing.
  dim: c90f95ad6d05 ("drm/panel: mantix: Fix panel reset"): mandatory review missing.
  dim: 8b557f793e69 ("drm/panel: mantix: Don't dereference NULL mode"): mandatory review missing.
  dim: ERROR: issues in commits detected, aborting

and in fact there's only Signed-off-by's on it:

Quoted 8 lines
Quoted 3 lines

Sam, I assume your Signed-off-by's should have been Reviewed-by ? O.k. to fix that up in the commit message before pushing to drm-misc-next?

Fine to add my r-b.

For patches I push I always read the patch - but seldom claim that I review them. It seems a bit implicit I think - but maybe this is just bad practice on my side.

I don't know but for fixes i seems fine for dim to complain if they show up without a r-b.

Quoted 3 lines

Then I also avoid the endless discussion with myself it what I did warrant an a-b or r-b.

Pushed out now, thanks a lot for the help!
Cheers,
-- Guido
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

#14 lore raw
In reply to Guido Günther
Quoted 40 lines · Guido Günther

Hi Daniel, Sam,

Quoted 29 lines · Daniel Vetter
Quoted 28 lines · Sam Ravnborg

Hi Guido.

Quoted 24 lines · Guido Günther

Hi Sam,

Quoted 2 lines · Sam Ravnborg

Hi Guido.
On Tue, Oct 13, 2020 at 12:32:45PM +0200, Guido Günther wrote:

[..snip..]

Quoted 9 lines
Quoted 9 lines

Changes from v1:
- As per review comments by Fabio Estevam
https://lore.kernel.org/dri-devel/CAOMZO5B5ECcConvKej=RcaF8wvOxgq7nUzKJ-ad0aSAOzUqtbQ@mail.gmail.com/
- Fix typo in commit messages
- As per review comments by Rob Herring
https://lore.kernel.org/dri-devel/20200929174624.GA832332@bogus/
- Don't use an array of reset lines

Quoted 7 lines
Quoted 4 lines

Guido Günther (3):
drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset
dt-binding: display: Require two resets on mantix panel

All applied to drm-misc-next and pushed out. And then I remembered you had commit right - sigh.

Thanks! Is there any special care needed to get that into 5.10? The driver landed there in 72967d5616d3f0c714f8eb6c4e258179a9031c45.

Quoted 8 lines
Quoted 4 lines

As the patches was applied to drm-misc-next the easiet path would be to cherry-pick them and apply to drm-misc-fixes.
dim has cherry-pick support - try to use it rahter than doing it by hand.

drm-misc-next-fixes while we're between freeze and merge window end:

https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#where-do-i-apply-my-patch

Quoted 7 lines

Great. Thanks for the pointer, that works. Now i get:

  $ ../maintainer-tools/dim push --dry-run
  dim: 3532f0117258 ("dt-binding: display: Require two resets on mantix panel"): mandatory review missing.
  dim: c90f95ad6d05 ("drm/panel: mantix: Fix panel reset"): mandatory review missing.
  dim: 8b557f793e69 ("drm/panel: mantix: Don't dereference NULL mode"): mandatory review missing.
  dim: ERROR: issues in commits detected, aborting

Ah yes, if author != committer then dim assumes there has been some oversight and review.

But when you cherry-pick over then it's again author == committer and in that case dim is looking for an a-b or r-b line, hence why the cherry-pick fails.

For a bit of context.
-Daniel

Quoted 39 lines

and in fact there's only Signed-off-by's on it:

Sam, I assume your Signed-off-by's should have been Reviewed-by ? O.k. to fix that up in the commit message before pushing to drm-misc-next?

Cheers,
-- Guido

Quoted 8 lines

Cheers, Daniel

Quoted 5 lines

When you apply to drm-misc-fixes include a Fixes: tag so the tooling will pick the patches automagically.

Quoted 13 lines
Quoted 7 lines

In hindsight the patches should have carried a Fixes: tag from a start and should have been applied to drm-misc-fixes from a start too.

I have done something like above once or twice but anyway reach out if you have questions. Or ask at #dri-devel.

Sam

Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel