mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/4] drm/bridge: lt8912b: Init & remove related fixes
@ 2023-08-04 10:48 Tomi Valkeinen
  2023-08-04 10:48 ` [PATCH 1/4] drm/bridge: lt8912b: Fix bridge_detach Tomi Valkeinen
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Tomi Valkeinen @ 2023-08-04 10:48 UTC (permalink / raw)
  To: Adrien Grassein, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, David Airlie,
	Daniel Vetter, Francesco Dolcini, Stefan Eichenberger
  Cc: dri-devel, linux-kernel, Tomi Valkeinen

Fix various issues in lt8912b driver.

 Tomi

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
Tomi Valkeinen (4):
      drm/bridge: lt8912b: Fix bridge_detach
      drm/bridge: lt8912b: Fix crash on bridge detach
      drm/bridge: lt8912b: Manually disable HPD only if it was enabled
      drm/bridge: lt8912b: Add missing drm_bridge_attach call

 drivers/gpu/drm/bridge/lontium-lt8912b.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)
---
base-commit: b0e9267d4ccce9be9217337f4bc364ca24cf7f73
change-id: 20230804-lt8912b-9cbb79018632

Best regards,
-- 
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] drm/bridge: lt8912b: Fix bridge_detach
  2023-08-04 10:48 [PATCH 0/4] drm/bridge: lt8912b: Init & remove related fixes Tomi Valkeinen
@ 2023-08-04 10:48 ` Tomi Valkeinen
  2023-08-04 15:10   ` Robert Foss
  2023-08-04 10:48 ` [PATCH 2/4] drm/bridge: lt8912b: Fix crash on bridge detach Tomi Valkeinen
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Tomi Valkeinen @ 2023-08-04 10:48 UTC (permalink / raw)
  To: Adrien Grassein, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, David Airlie,
	Daniel Vetter, Francesco Dolcini, Stefan Eichenberger
  Cc: dri-devel, linux-kernel, Tomi Valkeinen

The driver calls lt8912_bridge_detach() from its lt8912_remove()
function. As the DRM core detaches bridges automatically, this leads to
calling lt8912_bridge_detach() twice. The code probably has tried to
manage the double-call with the 'is_attached' variable, but the driver
never sets the variable to false, so its of no help.

Fix the issue by dropping the call to lt8912_bridge_detach() from
lt8912_remove(), as the DRM core will handle the detach call for us,
and also drop the useless is_attached field.

Fixes: 88abfc2b9e61 ("drm/bridge: Introduce LT8912B DSI to HDMI bridge")
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/bridge/lontium-lt8912b.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c
index 4eaea67fb71c..0e581f6e3c88 100644
--- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
+++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
@@ -45,7 +45,6 @@ struct lt8912 {
 
 	u8 data_lanes;
 	bool is_power_on;
-	bool is_attached;
 };
 
 static int lt8912_write_init_config(struct lt8912 *lt)
@@ -575,8 +574,6 @@ static int lt8912_bridge_attach(struct drm_bridge *bridge,
 	if (ret)
 		goto error;
 
-	lt->is_attached = true;
-
 	return 0;
 
 error:
@@ -588,15 +585,13 @@ static void lt8912_bridge_detach(struct drm_bridge *bridge)
 {
 	struct lt8912 *lt = bridge_to_lt8912(bridge);
 
-	if (lt->is_attached) {
-		lt8912_hard_power_off(lt);
+	lt8912_hard_power_off(lt);
 
-		if (lt->hdmi_port->ops & DRM_BRIDGE_OP_HPD)
-			drm_bridge_hpd_disable(lt->hdmi_port);
+	if (lt->hdmi_port->ops & DRM_BRIDGE_OP_HPD)
+		drm_bridge_hpd_disable(lt->hdmi_port);
 
-		drm_connector_unregister(&lt->connector);
-		drm_connector_cleanup(&lt->connector);
-	}
+	drm_connector_unregister(&lt->connector);
+	drm_connector_cleanup(&lt->connector);
 }
 
 static enum drm_connector_status
@@ -750,7 +745,6 @@ static void lt8912_remove(struct i2c_client *client)
 {
 	struct lt8912 *lt = i2c_get_clientdata(client);
 
-	lt8912_bridge_detach(&lt->bridge);
 	drm_bridge_remove(&lt->bridge);
 	lt8912_free_i2c(lt);
 	lt8912_put_dt(lt);

-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/4] drm/bridge: lt8912b: Fix crash on bridge detach
  2023-08-04 10:48 [PATCH 0/4] drm/bridge: lt8912b: Init & remove related fixes Tomi Valkeinen
  2023-08-04 10:48 ` [PATCH 1/4] drm/bridge: lt8912b: Fix bridge_detach Tomi Valkeinen
@ 2023-08-04 10:48 ` Tomi Valkeinen
  2023-08-04 15:07   ` Robert Foss
  2023-08-04 10:48 ` [PATCH 3/4] drm/bridge: lt8912b: Manually disable HPD only if it was enabled Tomi Valkeinen
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Tomi Valkeinen @ 2023-08-04 10:48 UTC (permalink / raw)
  To: Adrien Grassein, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, David Airlie,
	Daniel Vetter, Francesco Dolcini, Stefan Eichenberger
  Cc: dri-devel, linux-kernel, Tomi Valkeinen

The lt8912b driver, in its bridge detach function, calls
drm_connector_unregister() and drm_connector_cleanup().

drm_connector_unregister() should be called only for connectors
explicitly registered with drm_connector_register(), which is not the
case in lt8912b.

The driver's drm_connector_funcs.destroy hook is set to
drm_connector_cleanup().

Thus the driver should not call either drm_connector_unregister() nor
drm_connector_cleanup() in its lt8912_bridge_detach(), as they cause a
crash on bridge detach:

Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
Mem abort info:
  ESR = 0x0000000096000006
  EC = 0x25: DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
  FSC = 0x06: level 2 translation fault
Data abort info:
  ISV = 0, ISS = 0x00000006, ISS2 = 0x00000000
  CM = 0, WnR = 0, TnD = 0, TagAccess = 0
  GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=00000000858f3000
[0000000000000000] pgd=0800000085918003, p4d=0800000085918003, pud=0800000085431003, pmd=0000000000000000
Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP
Modules linked in: tidss(-) display_connector lontium_lt8912b tc358768 panel_lvds panel_simple drm_dma_helper drm_kms_helper drm drm_panel_orientation_quirks
CPU: 3 PID: 462 Comm: rmmod Tainted: G        W          6.5.0-rc2+ #2
Hardware name: Toradex Verdin AM62 on Verdin Development Board (DT)
pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : drm_connector_cleanup+0x78/0x2d4 [drm]
lr : lt8912_bridge_detach+0x54/0x6c [lontium_lt8912b]
sp : ffff800082ed3a90
x29: ffff800082ed3a90 x28: ffff0000040c1940 x27: 0000000000000000
x26: 0000000000000000 x25: dead000000000122 x24: dead000000000122
x23: dead000000000100 x22: ffff000003fb6388 x21: 0000000000000000
x20: 0000000000000000 x19: ffff000003fb6260 x18: fffffffffffe56e8
x17: 0000000000000000 x16: 0010000000000000 x15: 0000000000000038
x14: 0000000000000000 x13: ffff800081914b48 x12: 000000000000040e
x11: 000000000000015a x10: ffff80008196ebb8 x9 : ffff800081914b48
x8 : 00000000ffffefff x7 : ffff0000040c1940 x6 : ffff80007aa649d0
x5 : 0000000000000000 x4 : 0000000000000001 x3 : ffff80008159e008
x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
 drm_connector_cleanup+0x78/0x2d4 [drm]
 lt8912_bridge_detach+0x54/0x6c [lontium_lt8912b]
 drm_bridge_detach+0x44/0x84 [drm]
 drm_encoder_cleanup+0x40/0xb8 [drm]
 drmm_encoder_alloc_release+0x1c/0x30 [drm]
 drm_managed_release+0xac/0x148 [drm]
 drm_dev_put.part.0+0x88/0xb8 [drm]
 devm_drm_dev_init_release+0x14/0x24 [drm]
 devm_action_release+0x14/0x20
 release_nodes+0x5c/0x90
 devres_release_all+0x8c/0xe0
 device_unbind_cleanup+0x18/0x68
 device_release_driver_internal+0x208/0x23c
 driver_detach+0x4c/0x94
 bus_remove_driver+0x70/0xf4
 driver_unregister+0x30/0x60
 platform_driver_unregister+0x14/0x20
 tidss_platform_driver_exit+0x18/0xb2c [tidss]
 __arm64_sys_delete_module+0x1a0/0x2b4
 invoke_syscall+0x48/0x110
 el0_svc_common.constprop.0+0x60/0x10c
 do_el0_svc_compat+0x1c/0x40
 el0_svc_compat+0x40/0xac
 el0t_32_sync_handler+0xb0/0x138
 el0t_32_sync+0x194/0x198
Code: 9104a276 f2fbd5b7 aa0203e1 91008af8 (f85c0420)

Fixes: 30e2ae943c26 ("drm/bridge: Introduce LT8912B DSI to HDMI bridge")
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/bridge/lontium-lt8912b.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c
index 0e581f6e3c88..2d752e083433 100644
--- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
+++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
@@ -589,9 +589,6 @@ static void lt8912_bridge_detach(struct drm_bridge *bridge)
 
 	if (lt->hdmi_port->ops & DRM_BRIDGE_OP_HPD)
 		drm_bridge_hpd_disable(lt->hdmi_port);
-
-	drm_connector_unregister(&lt->connector);
-	drm_connector_cleanup(&lt->connector);
 }
 
 static enum drm_connector_status

-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/4] drm/bridge: lt8912b: Manually disable HPD only if it was enabled
  2023-08-04 10:48 [PATCH 0/4] drm/bridge: lt8912b: Init & remove related fixes Tomi Valkeinen
  2023-08-04 10:48 ` [PATCH 1/4] drm/bridge: lt8912b: Fix bridge_detach Tomi Valkeinen
  2023-08-04 10:48 ` [PATCH 2/4] drm/bridge: lt8912b: Fix crash on bridge detach Tomi Valkeinen
@ 2023-08-04 10:48 ` Tomi Valkeinen
  2023-09-04 18:08   ` Marcel Ziswiler
  2023-08-04 10:48 ` [PATCH 4/4] drm/bridge: lt8912b: Add missing drm_bridge_attach call Tomi Valkeinen
  2023-09-05 14:45 ` [PATCH 0/4] drm/bridge: lt8912b: Init & remove related fixes Robert Foss
  4 siblings, 1 reply; 10+ messages in thread
From: Tomi Valkeinen @ 2023-08-04 10:48 UTC (permalink / raw)
  To: Adrien Grassein, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, David Airlie,
	Daniel Vetter, Francesco Dolcini, Stefan Eichenberger
  Cc: dri-devel, linux-kernel, Tomi Valkeinen

lt8912b only calls drm_bridge_hpd_enable() if it creates a connector and
the next bridge has DRM_BRIDGE_OP_HPD set. However, when calling
drm_bridge_hpd_disable() it misses checking if a connector was created,
calling drm_bridge_hpd_disable() even if HPD was nenver enabled. I don't
see any issues causing by this wrong call, though.

Add the check to avoid wrongly calling drm_bridge_hpd_disable().

Fixes: 3b0a01a6a522 ("drm/bridge: lt8912b: Add hot plug detection")
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/bridge/lontium-lt8912b.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c
index 2d752e083433..9ee639e75a1c 100644
--- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
+++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
@@ -587,7 +587,7 @@ static void lt8912_bridge_detach(struct drm_bridge *bridge)
 
 	lt8912_hard_power_off(lt);
 
-	if (lt->hdmi_port->ops & DRM_BRIDGE_OP_HPD)
+	if (lt->connector.dev && lt->hdmi_port->ops & DRM_BRIDGE_OP_HPD)
 		drm_bridge_hpd_disable(lt->hdmi_port);
 }
 

-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 4/4] drm/bridge: lt8912b: Add missing drm_bridge_attach call
  2023-08-04 10:48 [PATCH 0/4] drm/bridge: lt8912b: Init & remove related fixes Tomi Valkeinen
                   ` (2 preceding siblings ...)
  2023-08-04 10:48 ` [PATCH 3/4] drm/bridge: lt8912b: Manually disable HPD only if it was enabled Tomi Valkeinen
@ 2023-08-04 10:48 ` Tomi Valkeinen
  2023-09-05 14:45 ` [PATCH 0/4] drm/bridge: lt8912b: Init & remove related fixes Robert Foss
  4 siblings, 0 replies; 10+ messages in thread
From: Tomi Valkeinen @ 2023-08-04 10:48 UTC (permalink / raw)
  To: Adrien Grassein, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, David Airlie,
	Daniel Vetter, Francesco Dolcini, Stefan Eichenberger
  Cc: dri-devel, linux-kernel, Tomi Valkeinen

The driver does not call drm_bridge_attach(), which causes the next
bridge to not be added to the bridge chain. This causes the pipeline
init to fail when DRM_BRIDGE_ATTACH_NO_CONNECTOR is used.

Add the call to drm_bridge_attach().

Fixes: 30e2ae943c26 ("drm/bridge: Introduce LT8912B DSI to HDMI bridge")
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/bridge/lontium-lt8912b.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c
index 9ee639e75a1c..03532efb893b 100644
--- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
+++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
@@ -558,6 +558,13 @@ static int lt8912_bridge_attach(struct drm_bridge *bridge,
 	struct lt8912 *lt = bridge_to_lt8912(bridge);
 	int ret;
 
+	ret = drm_bridge_attach(bridge->encoder, lt->hdmi_port, bridge,
+				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+	if (ret < 0) {
+		dev_err(lt->dev, "Failed to attach next bridge (%d)\n", ret);
+		return ret;
+	}
+
 	if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
 		ret = lt8912_bridge_connector_init(bridge);
 		if (ret) {

-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] drm/bridge: lt8912b: Fix crash on bridge detach
  2023-08-04 10:48 ` [PATCH 2/4] drm/bridge: lt8912b: Fix crash on bridge detach Tomi Valkeinen
@ 2023-08-04 15:07   ` Robert Foss
  0 siblings, 0 replies; 10+ messages in thread
From: Robert Foss @ 2023-08-04 15:07 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Adrien Grassein, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	Francesco Dolcini, Stefan Eichenberger, dri-devel, linux-kernel

On Fri, Aug 4, 2023 at 12:48 PM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
>
> The lt8912b driver, in its bridge detach function, calls
> drm_connector_unregister() and drm_connector_cleanup().
>
> drm_connector_unregister() should be called only for connectors
> explicitly registered with drm_connector_register(), which is not the
> case in lt8912b.
>
> The driver's drm_connector_funcs.destroy hook is set to
> drm_connector_cleanup().
>
> Thus the driver should not call either drm_connector_unregister() nor
> drm_connector_cleanup() in its lt8912_bridge_detach(), as they cause a
> crash on bridge detach:
>
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> Mem abort info:
>   ESR = 0x0000000096000006
>   EC = 0x25: DABT (current EL), IL = 32 bits
>   SET = 0, FnV = 0
>   EA = 0, S1PTW = 0
>   FSC = 0x06: level 2 translation fault
> Data abort info:
>   ISV = 0, ISS = 0x00000006, ISS2 = 0x00000000
>   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> user pgtable: 4k pages, 48-bit VAs, pgdp=00000000858f3000
> [0000000000000000] pgd=0800000085918003, p4d=0800000085918003, pud=0800000085431003, pmd=0000000000000000
> Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP
> Modules linked in: tidss(-) display_connector lontium_lt8912b tc358768 panel_lvds panel_simple drm_dma_helper drm_kms_helper drm drm_panel_orientation_quirks
> CPU: 3 PID: 462 Comm: rmmod Tainted: G        W          6.5.0-rc2+ #2
> Hardware name: Toradex Verdin AM62 on Verdin Development Board (DT)
> pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : drm_connector_cleanup+0x78/0x2d4 [drm]
> lr : lt8912_bridge_detach+0x54/0x6c [lontium_lt8912b]
> sp : ffff800082ed3a90
> x29: ffff800082ed3a90 x28: ffff0000040c1940 x27: 0000000000000000
> x26: 0000000000000000 x25: dead000000000122 x24: dead000000000122
> x23: dead000000000100 x22: ffff000003fb6388 x21: 0000000000000000
> x20: 0000000000000000 x19: ffff000003fb6260 x18: fffffffffffe56e8
> x17: 0000000000000000 x16: 0010000000000000 x15: 0000000000000038
> x14: 0000000000000000 x13: ffff800081914b48 x12: 000000000000040e
> x11: 000000000000015a x10: ffff80008196ebb8 x9 : ffff800081914b48
> x8 : 00000000ffffefff x7 : ffff0000040c1940 x6 : ffff80007aa649d0
> x5 : 0000000000000000 x4 : 0000000000000001 x3 : ffff80008159e008
> x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
> Call trace:
>  drm_connector_cleanup+0x78/0x2d4 [drm]
>  lt8912_bridge_detach+0x54/0x6c [lontium_lt8912b]
>  drm_bridge_detach+0x44/0x84 [drm]
>  drm_encoder_cleanup+0x40/0xb8 [drm]
>  drmm_encoder_alloc_release+0x1c/0x30 [drm]
>  drm_managed_release+0xac/0x148 [drm]
>  drm_dev_put.part.0+0x88/0xb8 [drm]
>  devm_drm_dev_init_release+0x14/0x24 [drm]
>  devm_action_release+0x14/0x20
>  release_nodes+0x5c/0x90
>  devres_release_all+0x8c/0xe0
>  device_unbind_cleanup+0x18/0x68
>  device_release_driver_internal+0x208/0x23c
>  driver_detach+0x4c/0x94
>  bus_remove_driver+0x70/0xf4
>  driver_unregister+0x30/0x60
>  platform_driver_unregister+0x14/0x20
>  tidss_platform_driver_exit+0x18/0xb2c [tidss]
>  __arm64_sys_delete_module+0x1a0/0x2b4
>  invoke_syscall+0x48/0x110
>  el0_svc_common.constprop.0+0x60/0x10c
>  do_el0_svc_compat+0x1c/0x40
>  el0_svc_compat+0x40/0xac
>  el0t_32_sync_handler+0xb0/0x138
>  el0t_32_sync+0x194/0x198
> Code: 9104a276 f2fbd5b7 aa0203e1 91008af8 (f85c0420)
>
> Fixes: 30e2ae943c26 ("drm/bridge: Introduce LT8912B DSI to HDMI bridge")
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/gpu/drm/bridge/lontium-lt8912b.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> index 0e581f6e3c88..2d752e083433 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> @@ -589,9 +589,6 @@ static void lt8912_bridge_detach(struct drm_bridge *bridge)
>
>         if (lt->hdmi_port->ops & DRM_BRIDGE_OP_HPD)
>                 drm_bridge_hpd_disable(lt->hdmi_port);
> -
> -       drm_connector_unregister(&lt->connector);
> -       drm_connector_cleanup(&lt->connector);
>  }
>
>  static enum drm_connector_status
>
> --
> 2.34.1
>


Reviewed-by: Robert Foss <rfoss@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] drm/bridge: lt8912b: Fix bridge_detach
  2023-08-04 10:48 ` [PATCH 1/4] drm/bridge: lt8912b: Fix bridge_detach Tomi Valkeinen
@ 2023-08-04 15:10   ` Robert Foss
  0 siblings, 0 replies; 10+ messages in thread
From: Robert Foss @ 2023-08-04 15:10 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Adrien Grassein, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	Francesco Dolcini, Stefan Eichenberger, dri-devel, linux-kernel

On Fri, Aug 4, 2023 at 12:48 PM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
>
> The driver calls lt8912_bridge_detach() from its lt8912_remove()
> function. As the DRM core detaches bridges automatically, this leads to
> calling lt8912_bridge_detach() twice. The code probably has tried to
> manage the double-call with the 'is_attached' variable, but the driver
> never sets the variable to false, so its of no help.
>
> Fix the issue by dropping the call to lt8912_bridge_detach() from
> lt8912_remove(), as the DRM core will handle the detach call for us,
> and also drop the useless is_attached field.
>
> Fixes: 88abfc2b9e61 ("drm/bridge: Introduce LT8912B DSI to HDMI bridge")
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/gpu/drm/bridge/lontium-lt8912b.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> index 4eaea67fb71c..0e581f6e3c88 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> @@ -45,7 +45,6 @@ struct lt8912 {
>
>         u8 data_lanes;
>         bool is_power_on;
> -       bool is_attached;
>  };
>
>  static int lt8912_write_init_config(struct lt8912 *lt)
> @@ -575,8 +574,6 @@ static int lt8912_bridge_attach(struct drm_bridge *bridge,
>         if (ret)
>                 goto error;
>
> -       lt->is_attached = true;
> -
>         return 0;
>
>  error:
> @@ -588,15 +585,13 @@ static void lt8912_bridge_detach(struct drm_bridge *bridge)
>  {
>         struct lt8912 *lt = bridge_to_lt8912(bridge);
>
> -       if (lt->is_attached) {
> -               lt8912_hard_power_off(lt);
> +       lt8912_hard_power_off(lt);
>
> -               if (lt->hdmi_port->ops & DRM_BRIDGE_OP_HPD)
> -                       drm_bridge_hpd_disable(lt->hdmi_port);
> +       if (lt->hdmi_port->ops & DRM_BRIDGE_OP_HPD)
> +               drm_bridge_hpd_disable(lt->hdmi_port);
>
> -               drm_connector_unregister(&lt->connector);
> -               drm_connector_cleanup(&lt->connector);
> -       }
> +       drm_connector_unregister(&lt->connector);
> +       drm_connector_cleanup(&lt->connector);
>  }
>
>  static enum drm_connector_status
> @@ -750,7 +745,6 @@ static void lt8912_remove(struct i2c_client *client)
>  {
>         struct lt8912 *lt = i2c_get_clientdata(client);
>
> -       lt8912_bridge_detach(&lt->bridge);
>         drm_bridge_remove(&lt->bridge);
>         lt8912_free_i2c(lt);
>         lt8912_put_dt(lt);
>
> --
> 2.34.1
>


Reviewed-by: Robert Foss <rfoss@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] drm/bridge: lt8912b: Manually disable HPD only if it was enabled
  2023-08-04 10:48 ` [PATCH 3/4] drm/bridge: lt8912b: Manually disable HPD only if it was enabled Tomi Valkeinen
@ 2023-09-04 18:08   ` Marcel Ziswiler
  2023-09-05  7:28     ` Tomi Valkeinen
  0 siblings, 1 reply; 10+ messages in thread
From: Marcel Ziswiler @ 2023-09-04 18:08 UTC (permalink / raw)
  To: Laurent.pinchart, andrzej.hajda, neil.armstrong,
	Stefan Eichenberger, tomi.valkeinen, adrien.grassein, rfoss,
	airlied, Francesco Dolcini, jonas, jernej.skrabec, daniel
  Cc: dri-devel, linux-kernel

Hi Tomi

Looks good. Thanks! Tested both on Verdin AM62 as well as on Verdin iMX8M Mini.

Just a minor nit-pick in your commit message.

On Fri, 2023-08-04 at 13:48 +0300, Tomi Valkeinen wrote:
> lt8912b only calls drm_bridge_hpd_enable() if it creates a connector and
> the next bridge has DRM_BRIDGE_OP_HPD set. However, when calling
> drm_bridge_hpd_disable() it misses checking if a connector was created,
> calling drm_bridge_hpd_disable() even if HPD was nenver enabled. I don't

was never enabled

> see any issues causing by this wrong call, though.

any issues caused by this wrong call

> Add the check to avoid wrongly calling drm_bridge_hpd_disable().
> 
> Fixes: 3b0a01a6a522 ("drm/bridge: lt8912b: Add hot plug detection")
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

For the whole series:

Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>

> ---
>  drivers/gpu/drm/bridge/lontium-lt8912b.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> index 2d752e083433..9ee639e75a1c 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> @@ -587,7 +587,7 @@ static void lt8912_bridge_detach(struct drm_bridge *bridge)
>  
>         lt8912_hard_power_off(lt);
>  
> -       if (lt->hdmi_port->ops & DRM_BRIDGE_OP_HPD)
> +       if (lt->connector.dev && lt->hdmi_port->ops & DRM_BRIDGE_OP_HPD)
>                 drm_bridge_hpd_disable(lt->hdmi_port);
>  }

Cheers

Marcel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] drm/bridge: lt8912b: Manually disable HPD only if it was enabled
  2023-09-04 18:08   ` Marcel Ziswiler
@ 2023-09-05  7:28     ` Tomi Valkeinen
  0 siblings, 0 replies; 10+ messages in thread
From: Tomi Valkeinen @ 2023-09-05  7:28 UTC (permalink / raw)
  To: Marcel Ziswiler
  Cc: dri-devel, linux-kernel, Laurent.pinchart, andrzej.hajda,
	neil.armstrong, Stefan Eichenberger, adrien.grassein, rfoss,
	airlied, Francesco Dolcini, jonas, jernej.skrabec, daniel

On 04/09/2023 21:08, Marcel Ziswiler wrote:
> Hi Tomi
> 
> Looks good. Thanks! Tested both on Verdin AM62 as well as on Verdin iMX8M Mini.
> 
> Just a minor nit-pick in your commit message.
> 
> On Fri, 2023-08-04 at 13:48 +0300, Tomi Valkeinen wrote:
>> lt8912b only calls drm_bridge_hpd_enable() if it creates a connector and
>> the next bridge has DRM_BRIDGE_OP_HPD set. However, when calling
>> drm_bridge_hpd_disable() it misses checking if a connector was created,
>> calling drm_bridge_hpd_disable() even if HPD was nenver enabled. I don't
> 
> was never enabled
> 
>> see any issues causing by this wrong call, though.
> 
> any issues caused by this wrong call
> 
>> Add the check to avoid wrongly calling drm_bridge_hpd_disable().
>>
>> Fixes: 3b0a01a6a522 ("drm/bridge: lt8912b: Add hot plug detection")
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> 
> For the whole series:
> 
> Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>

Thanks! I have fixed the typos in the desc.

  Tomi


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/4] drm/bridge: lt8912b: Init & remove related fixes
  2023-08-04 10:48 [PATCH 0/4] drm/bridge: lt8912b: Init & remove related fixes Tomi Valkeinen
                   ` (3 preceding siblings ...)
  2023-08-04 10:48 ` [PATCH 4/4] drm/bridge: lt8912b: Add missing drm_bridge_attach call Tomi Valkeinen
@ 2023-09-05 14:45 ` Robert Foss
  4 siblings, 0 replies; 10+ messages in thread
From: Robert Foss @ 2023-09-05 14:45 UTC (permalink / raw)
  To: Neil Armstrong, Tomi Valkeinen, Jonas Karlman, Andrzej Hajda,
	Adrien Grassein, Daniel Vetter, Laurent Pinchart, Jernej Skrabec,
	David Airlie, Francesco Dolcini, Stefan Eichenberger
  Cc: Robert Foss, linux-kernel, dri-devel

On Fri, 04 Aug 2023 13:48:09 +0300, Tomi Valkeinen wrote:
> Fix various issues in lt8912b driver.
> 
>  Tomi
> 
> 

Fixed the mentioned commit message typos, and an incorrect has
for a "Fixes" tag.

Applied, thanks!

[1/4] drm/bridge: lt8912b: Fix bridge_detach
      https://cgit.freedesktop.org/drm/drm-misc/commit/?id=941882a0e96d
[2/4] drm/bridge: lt8912b: Fix crash on bridge detach
      https://cgit.freedesktop.org/drm/drm-misc/commit/?id=44283993144a
[3/4] drm/bridge: lt8912b: Manually disable HPD only if it was enabled
      https://cgit.freedesktop.org/drm/drm-misc/commit/?id=6985c5efc405
[4/4] drm/bridge: lt8912b: Add missing drm_bridge_attach call
      https://cgit.freedesktop.org/drm/drm-misc/commit/?id=f45acf7acf75



Rob


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-09-05 16:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-04 10:48 [PATCH 0/4] drm/bridge: lt8912b: Init & remove related fixes Tomi Valkeinen
2023-08-04 10:48 ` [PATCH 1/4] drm/bridge: lt8912b: Fix bridge_detach Tomi Valkeinen
2023-08-04 15:10   ` Robert Foss
2023-08-04 10:48 ` [PATCH 2/4] drm/bridge: lt8912b: Fix crash on bridge detach Tomi Valkeinen
2023-08-04 15:07   ` Robert Foss
2023-08-04 10:48 ` [PATCH 3/4] drm/bridge: lt8912b: Manually disable HPD only if it was enabled Tomi Valkeinen
2023-09-04 18:08   ` Marcel Ziswiler
2023-09-05  7:28     ` Tomi Valkeinen
2023-08-04 10:48 ` [PATCH 4/4] drm/bridge: lt8912b: Add missing drm_bridge_attach call Tomi Valkeinen
2023-09-05 14:45 ` [PATCH 0/4] drm/bridge: lt8912b: Init & remove related fixes Robert Foss

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®