[PATCH v3 0/3] Samsung Exynos 7870 DECON driver support

[PATCH v3 0/3] Samsung Exynos 7870 DECON driver support

[Kaustabh Chakraborty]

This patch series aims at adding support for Exynos7870's DECON in the
Exynos7 DECON driver. It introduces a driver data struct so that support
for DECON on other SoCs can be added to it in the future.

It also fixes a few bugs in the driver, such as functions receiving bad
pointers.

Tested on Samsung Galaxy J7 Prime (samsung-on7xelte), Samsung Galaxy A2
Core (samsung-a2corelte), and Samsung Galaxy J6 (samsung-j6lte).

Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
Changes in v3:
- Add a new commit documenting iommus and ports dt properties.
- Link to v2: https://lore.kernel.org/r/20250612-exynosdrm-decon-v2-0-d6c1d21c8057@disroot.org

Changes in v2:
- Add a new commit to prevent an occasional panic under circumstances.
- Rewrite and redo [v1 2/6] to be a more sensible commit.
- Link to v1: https://lore.kernel.org/r/20240919-exynosdrm-decon-v1-0-6c5861c1cb04@disroot.org

---
Kaustabh Chakraborty (3):
      dt-bindings: display: samsung,exynos7-decon: add properties for iommus and ports
      drm/exynos: exynos7_drm_decon: fix call of decon_commit()
      drm/exynos: exynos7_drm_decon: add vblank check in IRQ handling

 .../bindings/display/samsung/samsung,exynos7-decon.yaml           | 8 ++++++++
 drivers/gpu/drm/exynos/exynos7_drm_decon.c                        | 8 ++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)
---
base-commit: 1b152eeca84a02bdb648f16b82ef3394007a9dcf
change-id: 20240917-exynosdrm-decon-4c228dd1d2bf

Best regards,
-- 
Kaustabh Chakraborty <kauschluss@disroot.org>

[PATCH v3 1/3] dt-bindings: display: samsung,exynos7-decon: add properties for iommus and ports

[Kaustabh Chakraborty]

Similar to FIMD and Exynos5433's DECON, the Exynos7 DECON hardware:
- May optionally require an IOMMU to initialize a display region.
- May require a port connection to another block, say an MIC or a DSI
  master.

Document these bindings in the devicetree schema.

Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
 .../bindings/display/samsung/samsung,exynos7-decon.yaml           | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml b/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
index 53916e4c95d8c0369138941a556c23f5d42fbd39..1e9500c86590d555cfa6f04790e2b64da291b78b 100644
--- a/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
+++ b/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
@@ -80,6 +80,14 @@ properties:
       - const: vsync
       - const: lcd_sys
 
+  iommus:
+    maxItems: 1
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    description:
+      Contains a port which is connected to mic or dsim node.
+
   power-domains:
     maxItems: 1
 

-- 
2.49.0

Re: [PATCH v3 1/3] dt-bindings: display: samsung,exynos7-decon: add properties for iommus and ports

[Inki Dae]

2025년 6월 27일 (금) 오전 4:21, Kaustabh Chakraborty <kauschluss@disroot.org>님이 작성:
>
> Similar to FIMD and Exynos5433's DECON, the Exynos7 DECON hardware:
> - May optionally require an IOMMU to initialize a display region.
> - May require a port connection to another block, say an MIC or a DSI
>   master.
>
> Document these bindings in the devicetree schema.

Applied.

Thanks,
Inki Dae

>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
>  .../bindings/display/samsung/samsung,exynos7-decon.yaml           | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml b/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
> index 53916e4c95d8c0369138941a556c23f5d42fbd39..1e9500c86590d555cfa6f04790e2b64da291b78b 100644
> --- a/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
> +++ b/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
> @@ -80,6 +80,14 @@ properties:
>        - const: vsync
>        - const: lcd_sys
>
> +  iommus:
> +    maxItems: 1
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +    description:
> +      Contains a port which is connected to mic or dsim node.
> +
>    power-domains:
>      maxItems: 1
>
>
> --
> 2.49.0
>
>

Re: [PATCH v3 1/3] dt-bindings: display: samsung,exynos7-decon: add properties for iommus and ports

[Krzysztof Kozlowski]

On Fri, Jun 27, 2025 at 02:02:37PM +0900, Inki Dae wrote:
> 2025년 6월 27일 (금) 오전 4:21, Kaustabh Chakraborty <kauschluss@disroot.org>님이 작성:
> >
> > Similar to FIMD and Exynos5433's DECON, the Exynos7 DECON hardware:
> > - May optionally require an IOMMU to initialize a display region.
> > - May require a port connection to another block, say an MIC or a DSI
> >   master.
> >
> > Document these bindings in the devicetree schema.
> 
> Applied.

Please wait for DT review, giving us few hours is not enough. Please
drop the patchset, needs fixes.

Best regards,
Krzysztof

Re: [PATCH v3 1/3] dt-bindings: display: samsung,exynos7-decon: add properties for iommus and ports

[Inki Dae]

2025년 6월 27일 (금) 오후 4:51, Krzysztof Kozlowski <krzk@kernel.org>님이 작성:
>
> On Fri, Jun 27, 2025 at 02:02:37PM +0900, Inki Dae wrote:
> > 2025년 6월 27일 (금) 오전 4:21, Kaustabh Chakraborty <kauschluss@disroot.org>님이 작성:
> > >
> > > Similar to FIMD and Exynos5433's DECON, the Exynos7 DECON hardware:
> > > - May optionally require an IOMMU to initialize a display region.
> > > - May require a port connection to another block, say an MIC or a DSI
> > >   master.
> > >
> > > Document these bindings in the devicetree schema.
> >
> > Applied.
>
> Please wait for DT review, giving us few hours is not enough. Please
> drop the patchset, needs fixes.

Got it. Reverted.

Thanks,
Inki Dae

>
> Best regards,
> Krzysztof
>

Re: [PATCH v3 1/3] dt-bindings: display: samsung,exynos7-decon: add properties for iommus and ports

[Krzysztof Kozlowski]

On Fri, Jun 27, 2025 at 12:50:28AM +0530, Kaustabh Chakraborty wrote:
> Similar to FIMD and Exynos5433's DECON, the Exynos7 DECON hardware:
> - May optionally require an IOMMU to initialize a display region.
> - May require a port connection to another block, say an MIC or a DSI
>   master.
> 
> Document these bindings in the devicetree schema.
> 
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
>  .../bindings/display/samsung/samsung,exynos7-decon.yaml           | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml b/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
> index 53916e4c95d8c0369138941a556c23f5d42fbd39..1e9500c86590d555cfa6f04790e2b64da291b78b 100644
> --- a/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
> +++ b/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
> @@ -80,6 +80,14 @@ properties:
>        - const: vsync
>        - const: lcd_sys
>  
> +  iommus:
> +    maxItems: 1
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +    description:
> +      Contains a port which is connected to mic or dsim node.

You need to list and describe the ports.

Best regards,
Krzysztof

Re: [PATCH v3 1/3] dt-bindings: display: samsung,exynos7-decon: add properties for iommus and ports

[Kaustabh Chakraborty]

On 2025-06-27 07:50, Krzysztof Kozlowski wrote:
> On Fri, Jun 27, 2025 at 12:50:28AM +0530, Kaustabh Chakraborty wrote:
>> Similar to FIMD and Exynos5433's DECON, the Exynos7 DECON hardware:
>> - May optionally require an IOMMU to initialize a display region.
>> - May require a port connection to another block, say an MIC or a DSI
>>   master.
>> 
>> Document these bindings in the devicetree schema.
>> 
>> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
>> ---
>>  .../bindings/display/samsung/samsung,exynos7-decon.yaml           | 8 
>> ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml 
>> b/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
>> index 
>> 53916e4c95d8c0369138941a556c23f5d42fbd39..1e9500c86590d555cfa6f04790e2b64da291b78b 
>> 100644
>> --- 
>> a/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
>> +++ 
>> b/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
>> @@ -80,6 +80,14 @@ properties:
>>        - const: vsync
>>        - const: lcd_sys
>> 
>> +  iommus:
>> +    maxItems: 1
>> +
>> +  ports:
>> +    $ref: /schemas/graph.yaml#/properties/ports
>> +    description:
>> +      Contains a port which is connected to mic or dsim node.
> 
> You need to list and describe the ports.

-    description:
-      Contains a port which is connected to mic or dsim node.
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          Input port which is connected to either a Mobile Image
+          Compressor (MIC) or a DSI Master device.

I assume you want something like this?
Is the formatting correct? Should there be a space between
ports:$ref and ports:properties?

> 
> Best regards,
> Krzysztof

Re: [PATCH v3 1/3] dt-bindings: display: samsung,exynos7-decon: add properties for iommus and ports

[Krzysztof Kozlowski]

On 27/06/2025 15:44, Kaustabh Chakraborty wrote:
>>> a/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
>>> +++ 
>>> b/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
>>> @@ -80,6 +80,14 @@ properties:
>>>        - const: vsync
>>>        - const: lcd_sys
>>>
>>> +  iommus:
>>> +    maxItems: 1
>>> +
>>> +  ports:
>>> +    $ref: /schemas/graph.yaml#/properties/ports
>>> +    description:
>>> +      Contains a port which is connected to mic or dsim node.
>>
>> You need to list and describe the ports.
> 
> -    description:
> -      Contains a port which is connected to mic or dsim node.
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          Input port which is connected to either a Mobile Image
> +          Compressor (MIC) or a DSI Master device.


If this is only one port, then just 'port' property, but I have doubts
it should be one, because even you mentioned two - MIC could be the
input and MIPI DSIM would be the output.

Maybe if the MIC is integral part, it would not have been an input, but
then only 'port'.

> 
> I assume you want something like this?
> Is the formatting correct? Should there be a space between
> ports:$ref and ports:properties?

Look at toshiba,tc358768.yaml or the simple-bridge (except you should
name the input and output ports).



Best regards,
Krzysztof

Re: [PATCH v3 1/3] dt-bindings: display: samsung,exynos7-decon: add properties for iommus and ports

[Kaustabh Chakraborty]

On 2025-06-27 14:44, Krzysztof Kozlowski wrote:
> On 27/06/2025 15:44, Kaustabh Chakraborty wrote:
>>>> a/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
>>>> +++
>>>> b/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
>>>> @@ -80,6 +80,14 @@ properties:
>>>>        - const: vsync
>>>>        - const: lcd_sys
>>>> 
>>>> +  iommus:
>>>> +    maxItems: 1
>>>> +
>>>> +  ports:
>>>> +    $ref: /schemas/graph.yaml#/properties/ports
>>>> +    description:
>>>> +      Contains a port which is connected to mic or dsim node.
>>> 
>>> You need to list and describe the ports.
>> 
>> -    description:
>> -      Contains a port which is connected to mic or dsim node.
>> +    properties:
>> +      port@0:
>> +        $ref: /schemas/graph.yaml#/properties/port
>> +        description:
>> +          Input port which is connected to either a Mobile Image
>> +          Compressor (MIC) or a DSI Master device.
> 
> 
> If this is only one port, then just 'port' property, but I have doubts
> it should be one, because even you mentioned two - MIC could be the
> input and MIPI DSIM would be the output.

DECON is the first device in the pipeline. So it should only have
output.

It's either:
DECON -> DSIM -> panel
or
DECON -> MIC -> DSIM -> panel

Exynos7870 doesn't have MIC, but other SoCs may have one.

> 
> Maybe if the MIC is integral part, it would not have been an input, but
> then only 'port'.
> 
>> 
>> I assume you want something like this?
>> Is the formatting correct? Should there be a space between
>> ports:$ref and ports:properties?
> 
> Look at toshiba,tc358768.yaml or the simple-bridge (except you should
> name the input and output ports).

Okay, thanks!

Therefore this:

-    description:
-      Contains a port which is connected to mic or dsim node.
+
+    properties:
+      port:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          Output port which is connected to either a Mobile Image
+          Compressor (MIC) or a DSI Master device.

> 
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH v3 1/3] dt-bindings: display: samsung,exynos7-decon: add properties for iommus and ports

[Krzysztof Kozlowski]

On 27/06/2025 17:03, Kaustabh Chakraborty wrote:
> On 2025-06-27 14:44, Krzysztof Kozlowski wrote:
>> On 27/06/2025 15:44, Kaustabh Chakraborty wrote:
>>>>> a/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
>>>>> +++
>>>>> b/Documentation/devicetree/bindings/display/samsung/samsung,exynos7-decon.yaml
>>>>> @@ -80,6 +80,14 @@ properties:
>>>>>        - const: vsync
>>>>>        - const: lcd_sys
>>>>>
>>>>> +  iommus:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  ports:
>>>>> +    $ref: /schemas/graph.yaml#/properties/ports
>>>>> +    description:
>>>>> +      Contains a port which is connected to mic or dsim node.
>>>>
>>>> You need to list and describe the ports.
>>>
>>> -    description:
>>> -      Contains a port which is connected to mic or dsim node.
>>> +    properties:
>>> +      port@0:
>>> +        $ref: /schemas/graph.yaml#/properties/port
>>> +        description:
>>> +          Input port which is connected to either a Mobile Image
>>> +          Compressor (MIC) or a DSI Master device.
>>
>>
>> If this is only one port, then just 'port' property, but I have doubts
>> it should be one, because even you mentioned two - MIC could be the
>> input and MIPI DSIM would be the output.
> 
> DECON is the first device in the pipeline. So it should only have
> output.
> 
> It's either:
> DECON -> DSIM -> panel
> or
> DECON -> MIC -> DSIM -> panel
> 
> Exynos7870 doesn't have MIC, but other SoCs may have one.

Could be without ports entirely, but it is also fine to list port under.

> 
>>
>> Maybe if the MIC is integral part, it would not have been an input, but
>> then only 'port'.
>>
>>>
>>> I assume you want something like this?
>>> Is the formatting correct? Should there be a space between
>>> ports:$ref and ports:properties?
>>
>> Look at toshiba,tc358768.yaml or the simple-bridge (except you should
>> name the input and output ports).
> 
> Okay, thanks!
> 
> Therefore this:
> 
> -    description:
> -      Contains a port which is connected to mic or dsim node.
> +
> +    properties:
> +      port:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          Output port which is connected to either a Mobile Image
> +          Compressor (MIC) or a DSI Master device.

and additionalProperties: false in ports level.


Best regards,
Krzysztof

Re: [PATCH v3 1/3] dt-bindings: display: samsung,exynos7-decon: add properties for iommus and ports

[Krzysztof Kozlowski]

On 26/06/2025 21:20, Kaustabh Chakraborty wrote:
> @@ -80,6 +80,14 @@ properties:
>        - const: vsync
>        - const: lcd_sys
>  
> +  iommus:
> +    maxItems: 1
> +
> +  ports:

This was supposed to be port, no?

> +    $ref: /schemas/graph.yaml#/properties/ports
> +    description:
> +      Contains a port which is connected to mic or dsim node.
If you are using ports, then you need to list the ports.

Best regards,
Krzysztof

Re: [PATCH v3 1/3] dt-bindings: display: samsung,exynos7-decon: add properties for iommus and ports

[Kaustabh Chakraborty]

On 2025-07-06 07:33, Krzysztof Kozlowski wrote:
> On 26/06/2025 21:20, Kaustabh Chakraborty wrote:
>> @@ -80,6 +80,14 @@ properties:
>>        - const: vsync
>>        - const: lcd_sys
>> 
>> +  iommus:
>> +    maxItems: 1
>> +
>> +  ports:
> 
> This was supposed to be port, no?

Yes. You have reviewed this patch though, I didn't send a new rev
yet.

Either way, yes this should be port (since it has a single output
port).

> 
>> +    $ref: /schemas/graph.yaml#/properties/ports
>> +    description:
>> +      Contains a port which is connected to mic or dsim node.
> If you are using ports, then you need to list the ports.
> 
> Best regards,
> Krzysztof

Re: [PATCH v3 1/3] dt-bindings: display: samsung,exynos7-decon: add properties for iommus and ports

[Krzysztof Kozlowski]

On 06/07/2025 18:00, Kaustabh Chakraborty wrote:
> On 2025-07-06 07:33, Krzysztof Kozlowski wrote:
>> On 26/06/2025 21:20, Kaustabh Chakraborty wrote:
>>> @@ -80,6 +80,14 @@ properties:
>>>        - const: vsync
>>>        - const: lcd_sys
>>>
>>> +  iommus:
>>> +    maxItems: 1
>>> +
>>> +  ports:
>>
>> This was supposed to be port, no?
> 
> Yes. You have reviewed this patch though, I didn't send a new rev
> yet.
> 
> Either way, yes this should be port (since it has a single output
> port).

Ah, indeed, this popped up in my inbox and I totally missed that I
already responded.

Best regards,
Krzysztof

[PATCH v3 2/3] drm/exynos: exynos7_drm_decon: fix call of decon_commit()

[Kaustabh Chakraborty]

decon_commit() has a condition guard at the beginning:

	if (ctx->suspended)
		return;

But, when it is being called from decon_atomic_enable(), ctx->suspended
is still set to true, which prevents its execution. decon_commit() is
vital for setting up display timing values, without which the display
pipeline fails to function properly. Call the function after
ctx->suspended is set to false as a fix.

Cc: stable@vger.kernel.org
Fixes: 96976c3d9aff ("drm/exynos: Add DECON driver")
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
 drivers/gpu/drm/exynos/exynos7_drm_decon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index f91daefa9d2bc5e314c279822047e60ee0d7ca99..43bcbe2e2917df43d7c2d27a9771e892628dd682 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -583,9 +583,9 @@ static void decon_atomic_enable(struct exynos_drm_crtc *crtc)
 	if (test_and_clear_bit(0, &ctx->irq_flags))
 		decon_enable_vblank(ctx->crtc);
 
-	decon_commit(ctx->crtc);
-
 	ctx->suspended = false;
+
+	decon_commit(ctx->crtc);
 }
 
 static void decon_atomic_disable(struct exynos_drm_crtc *crtc)

-- 
2.49.0

Re: [PATCH v3 2/3] drm/exynos: exynos7_drm_decon: fix call of decon_commit()

[Inki Dae]

Hi,

2025년 6월 27일 (금) 오전 4:21, Kaustabh Chakraborty <kauschluss@disroot.org>님이 작성:
>
> decon_commit() has a condition guard at the beginning:
>
>         if (ctx->suspended)
>                 return;
>
> But, when it is being called from decon_atomic_enable(), ctx->suspended
> is still set to true, which prevents its execution. decon_commit() is
> vital for setting up display timing values, without which the display
> pipeline fails to function properly. Call the function after
> ctx->suspended is set to false as a fix.

Good observation. However, I think a more generic solution is needed.

>
> Cc: stable@vger.kernel.org
> Fixes: 96976c3d9aff ("drm/exynos: Add DECON driver")
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
>  drivers/gpu/drm/exynos/exynos7_drm_decon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> index f91daefa9d2bc5e314c279822047e60ee0d7ca99..43bcbe2e2917df43d7c2d27a9771e892628dd682 100644
> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> @@ -583,9 +583,9 @@ static void decon_atomic_enable(struct exynos_drm_crtc *crtc)
>         if (test_and_clear_bit(0, &ctx->irq_flags))
>                 decon_enable_vblank(ctx->crtc);
>
> -       decon_commit(ctx->crtc);
> -
>         ctx->suspended = false;
> +
> +       decon_commit(ctx->crtc);

There seem to be three possible solutions:

1. Remove all code related to ctx->suspended. If the pipeline flow is
properly managed as in the exynos5433_drm_decon.c module, checking the
ctx->suspended state may no longer be necessary.
2. Remove the ctx->suspended check from decon_commit(). Since the
runtime PM resume is already called before decon_commit() in
decon_atomic_enable(), the DECON controller should already be enabled
at the hardware level, and decon_commit() should work correctly.
3. Move the code that updates ctx->suspended from
decon_atomic_enable() and decon_atomic_disable() to
exynos7_decon_resume() and exynos7_decon_suspend(), respectively. The
decon_atomic_enable() function calls pm_runtime_resume_and_get(),
which ultimately triggers exynos7_decon_resume(). It would be more
appropriate to set ctx->suspended = false in the
exynos7_decon_resume() function, as this is the standard place to
handle hardware state changes and resume actions.
decon_atomic_enable() is responsible for requesting enablement of the
DECON controller, but actual hardware state transitions will be
handled within exynos7_decon_resume() and exynos7_decon_suspend().


Unfortunately, I do not have hardware to test this patch myself. Would
it be possible for you to try one of these approaches and verify the
behavior?
Option 1 would be the best solution if feasible.

Thanks,
Inki Dae

>  }
>
>  static void decon_atomic_disable(struct exynos_drm_crtc *crtc)
>
> --
> 2.49.0
>
>

Re: [PATCH v3 2/3] drm/exynos: exynos7_drm_decon: fix call of decon_commit()

[Kaustabh Chakraborty]

On 2025-06-27 04:06, Inki Dae wrote:
> Hi,
> 
> 2025년 6월 27일 (금) 오전 4:21, Kaustabh Chakraborty 
> <kauschluss@disroot.org>님이 작성:
>> 
>> decon_commit() has a condition guard at the beginning:
>> 
>>         if (ctx->suspended)
>>                 return;
>> 
>> But, when it is being called from decon_atomic_enable(), 
>> ctx->suspended
>> is still set to true, which prevents its execution. decon_commit() is
>> vital for setting up display timing values, without which the display
>> pipeline fails to function properly. Call the function after
>> ctx->suspended is set to false as a fix.
> 
> Good observation. However, I think a more generic solution is needed.
> 
>> 
>> Cc: stable@vger.kernel.org
>> Fixes: 96976c3d9aff ("drm/exynos: Add DECON driver")
>> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
>> ---
>>  drivers/gpu/drm/exynos/exynos7_drm_decon.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c 
>> b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
>> index 
>> f91daefa9d2bc5e314c279822047e60ee0d7ca99..43bcbe2e2917df43d7c2d27a9771e892628dd682 
>> 100644
>> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
>> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
>> @@ -583,9 +583,9 @@ static void decon_atomic_enable(struct 
>> exynos_drm_crtc *crtc)
>>         if (test_and_clear_bit(0, &ctx->irq_flags))
>>                 decon_enable_vblank(ctx->crtc);
>> 
>> -       decon_commit(ctx->crtc);
>> -
>>         ctx->suspended = false;
>> +
>> +       decon_commit(ctx->crtc);
> 
> There seem to be three possible solutions:
> 
> 1. Remove all code related to ctx->suspended. If the pipeline flow is
> properly managed as in the exynos5433_drm_decon.c module, checking the
> ctx->suspended state may no longer be necessary.
> 2. Remove the ctx->suspended check from decon_commit(). Since the
> runtime PM resume is already called before decon_commit() in
> decon_atomic_enable(), the DECON controller should already be enabled
> at the hardware level, and decon_commit() should work correctly.
> 3. Move the code that updates ctx->suspended from
> decon_atomic_enable() and decon_atomic_disable() to
> exynos7_decon_resume() and exynos7_decon_suspend(), respectively. The
> decon_atomic_enable() function calls pm_runtime_resume_and_get(),
> which ultimately triggers exynos7_decon_resume(). It would be more
> appropriate to set ctx->suspended = false in the
> exynos7_decon_resume() function, as this is the standard place to
> handle hardware state changes and resume actions.
> decon_atomic_enable() is responsible for requesting enablement of the
> DECON controller, but actual hardware state transitions will be
> handled within exynos7_decon_resume() and exynos7_decon_suspend().
> 
> 
> Unfortunately, I do not have hardware to test this patch myself. Would
> it be possible for you to try one of these approaches and verify the
> behavior?
> Option 1 would be the best solution if feasible.

Yes, it works fine indeed. Thanks!

> 
> Thanks,
> Inki Dae
> 
>>  }
>> 
>>  static void decon_atomic_disable(struct exynos_drm_crtc *crtc)
>> 
>> --
>> 2.49.0
>> 
>> 

[PATCH v3 3/3] drm/exynos: exynos7_drm_decon: add vblank check in IRQ handling

[Kaustabh Chakraborty]

If there's support for another console device (such as a TTY serial),
the kernel occasionally panics during boot. The panic message and a
relevant snippet of the call stack is as follows:

  Unable to handle kernel NULL pointer dereference at virtual address 000000000000000
  Call trace:
    drm_crtc_handle_vblank+0x10/0x30 (P)
    decon_irq_handler+0x88/0xb4
    [...]

Otherwise, the panics don't happen. This indicates that it's some sort
of race condition.

Add a check to validate if the drm device can handle vblanks before
calling drm_crtc_handle_vblank() to avoid this.

Cc: stable@vger.kernel.org
Fixes: 96976c3d9aff ("drm/exynos: Add DECON driver")
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
 drivers/gpu/drm/exynos/exynos7_drm_decon.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 43bcbe2e2917df43d7c2d27a9771e892628dd682..c0c0f23169c993ac315fc8d7bcbd09ea6ec9966a 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -636,6 +636,10 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)
 	if (!ctx->drm_dev)
 		goto out;
 
+	/* check if crtc and vblank have been initialized properly */
+	if (!drm_dev_has_vblank(ctx->drm_dev))
+		goto out;
+
 	if (!ctx->i80_if) {
 		drm_crtc_handle_vblank(&ctx->crtc->base);
 

-- 
2.49.0

Re: [PATCH v3 3/3] drm/exynos: exynos7_drm_decon: add vblank check in IRQ handling

[Inki Dae]

2025년 6월 27일 (금) 오전 4:21, Kaustabh Chakraborty <kauschluss@disroot.org>님이 작성:
>
> If there's support for another console device (such as a TTY serial),
> the kernel occasionally panics during boot. The panic message and a
> relevant snippet of the call stack is as follows:
>
>   Unable to handle kernel NULL pointer dereference at virtual address 000000000000000
>   Call trace:
>     drm_crtc_handle_vblank+0x10/0x30 (P)
>     decon_irq_handler+0x88/0xb4
>     [...]
>

It seems that if the display is already enabled by the bootloader
during the boot process, a vblank interrupt may be triggered before
the initialization of drm_dev is complete. This could be the root
cause of the issue.

Applied.

Thanks,
Inki Dae

> Otherwise, the panics don't happen. This indicates that it's some sort
> of race condition.
>
> Add a check to validate if the drm device can handle vblanks before
> calling drm_crtc_handle_vblank() to avoid this.
>
> Cc: stable@vger.kernel.org
> Fixes: 96976c3d9aff ("drm/exynos: Add DECON driver")
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
>  drivers/gpu/drm/exynos/exynos7_drm_decon.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> index 43bcbe2e2917df43d7c2d27a9771e892628dd682..c0c0f23169c993ac315fc8d7bcbd09ea6ec9966a 100644
> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> @@ -636,6 +636,10 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)
>         if (!ctx->drm_dev)
>                 goto out;
>
> +       /* check if crtc and vblank have been initialized properly */
> +       if (!drm_dev_has_vblank(ctx->drm_dev))
> +               goto out;
> +
>         if (!ctx->i80_if) {
>                 drm_crtc_handle_vblank(&ctx->crtc->base);
>
>
> --
> 2.49.0
>
>