[PATCH v2 0/4] drm/panel: atna33xc20: Fix the Samsung ATNA45AF01 panel

[PATCH v2 0/4] drm/panel: atna33xc20: Fix the Samsung ATNA45AF01 panel

[Stephan Gerhold]

The backlight of the Samsung ATNA45AF01 panel used in the Qualcomm X1E80100
CRD does not work correctly with the current display panel configuration
and drivers: It works after boot, but once the display gets disabled it is
not possible to get it back on. It turns out that the ATNA45AF01 panel
needs exactly the same non-standard power sequence as implemented for
ATNA33XC20 in the panel-samsung-atna33xc20 driver.

Move the ATNA45AF01 panel from the generic panel-edp driver to the
panel-samsung-atna33xc20 driver and fix the panel configuration in the
x1e80100-crd device tree to make the panel work correctly.

The panel and DT changes can be picked up independently. Since v2 uses the
existing "samsung,atna33xc20" compatible as fallback, the DT changes work
even without the driver changes.

Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
Changes in v2:
- Use fallback compatible to avoid changes in driver (Doug)
- ... this allows applying the DT change as fix, so add Fixes: tag there
- Add review tags (except Krzysztof's, since the binding changed quite a bit)
- Link to v1: https://lore.kernel.org/r/20240710-x1e80100-crd-backlight-v1-0-eb242311a23e@linaro.org

---
Stephan Gerhold (4):
      dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01
      Revert "drm/panel-edp: Add SDC ATNA45AF01"
      arm64: dts: qcom: x1e80100-crd: Fix backlight
      arm64: defconfig: Add CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20

 .../bindings/display/panel/samsung,atna33xc20.yaml      |  8 +++++++-
 arch/arm64/boot/dts/qcom/x1e80100-crd.dts               | 17 +++++++++++++++--
 arch/arm64/configs/defconfig                            |  1 +
 drivers/gpu/drm/panel/panel-edp.c                       |  2 --
 4 files changed, 23 insertions(+), 5 deletions(-)
---
base-commit: 91e3b24eb7d297d9d99030800ed96944b8652eaf
change-id: 20240710-x1e80100-crd-backlight-632f9bf936ef

Best regards,
-- 
Stephan Gerhold <stephan.gerhold@linaro.org>

[PATCH v2 1/4] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01

[Stephan Gerhold]

The Samsung ATNA45AF01 panel is an AMOLED eDP panel that has backlight
control over the DP AUX channel. While it works almost correctly with the
generic "edp-panel" compatible, the backlight needs special handling to
work correctly. It is similar to the existing ATNA33XC20 panel, just with
a larger resolution and size.

Add a new "samsung,atna45af01" compatible to describe this panel in the DT.
Use the existing "samsung,atna33xc20" as fallback compatible since existing
drivers should work as-is, given that resolution and size are discoverable
through the eDP link.

Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
 .../devicetree/bindings/display/panel/samsung,atna33xc20.yaml     | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
index 765ca155c83a..5192c93fbd67 100644
--- a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
+++ b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
@@ -14,7 +14,13 @@ allOf:
 
 properties:
   compatible:
-    const: samsung,atna33xc20
+    oneOf:
+      # Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
+      - const: samsung,atna33xc20
+      # Samsung 14.5" WQXGA+ (2880x1800 pixels) eDP AMOLED panel
+      - items:
+          - const: samsung,atna45af01
+          - const: samsung,atna33xc20
 
   enable-gpios: true
   port: true

-- 
2.44.1

Re: [PATCH v2 1/4] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01

[Neil Armstrong]

On 15/07/2024 14:15, Stephan Gerhold wrote:
> The Samsung ATNA45AF01 panel is an AMOLED eDP panel that has backlight
> control over the DP AUX channel. While it works almost correctly with the
> generic "edp-panel" compatible, the backlight needs special handling to
> work correctly. It is similar to the existing ATNA33XC20 panel, just with
> a larger resolution and size.
> 
> Add a new "samsung,atna45af01" compatible to describe this panel in the DT.
> Use the existing "samsung,atna33xc20" as fallback compatible since existing
> drivers should work as-is, given that resolution and size are discoverable
> through the eDP link.
> 
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
>   .../devicetree/bindings/display/panel/samsung,atna33xc20.yaml     | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
> index 765ca155c83a..5192c93fbd67 100644
> --- a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
> @@ -14,7 +14,13 @@ allOf:
>   
>   properties:
>     compatible:
> -    const: samsung,atna33xc20
> +    oneOf:
> +      # Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
> +      - const: samsung,atna33xc20
> +      # Samsung 14.5" WQXGA+ (2880x1800 pixels) eDP AMOLED panel
> +      - items:
> +          - const: samsung,atna45af01
> +          - const: samsung,atna33xc20
>   
>     enable-gpios: true
>     port: true
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH v2 1/4] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01

[Doug Anderson]

Hi,

Reviewed-by: Douglas Anderson <dianders@chromium.org>


On Mon, Jul 15, 2024 at 5:16 AM Stephan Gerhold
<stephan.gerhold@linaro.org> wrote:
>
> The Samsung ATNA45AF01 panel is an AMOLED eDP panel that has backlight
> control over the DP AUX channel. While it works almost correctly with the
> generic "edp-panel" compatible, the backlight needs special handling to
> work correctly. It is similar to the existing ATNA33XC20 panel, just with
> a larger resolution and size.
>
> Add a new "samsung,atna45af01" compatible to describe this panel in the DT.
> Use the existing "samsung,atna33xc20" as fallback compatible since existing
> drivers should work as-is, given that resolution and size are discoverable
> through the eDP link.
>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
>  .../devicetree/bindings/display/panel/samsung,atna33xc20.yaml     | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
> index 765ca155c83a..5192c93fbd67 100644
> --- a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
> @@ -14,7 +14,13 @@ allOf:
>
>  properties:
>    compatible:
> -    const: samsung,atna33xc20
> +    oneOf:
> +      # Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
> +      - const: samsung,atna33xc20
> +      # Samsung 14.5" WQXGA+ (2880x1800 pixels) eDP AMOLED panel
> +      - items:
> +          - const: samsung,atna45af01
> +          - const: samsung,atna33xc20
>
>    enable-gpios: true
>    port: true
>
> --
> 2.44.1
>

Re: [PATCH v2 1/4] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 789 bytes --]

On Mon, Jul 15, 2024 at 02:15:37PM +0200, Stephan Gerhold wrote:
> The Samsung ATNA45AF01 panel is an AMOLED eDP panel that has backlight
> control over the DP AUX channel. While it works almost correctly with the
> generic "edp-panel" compatible, the backlight needs special handling to
> work correctly. It is similar to the existing ATNA33XC20 panel, just with
> a larger resolution and size.
> 
> Add a new "samsung,atna45af01" compatible to describe this panel in the DT.
> Use the existing "samsung,atna33xc20" as fallback compatible since existing
> drivers should work as-is, given that resolution and size are discoverable
> through the eDP link.
> 
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/4] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01

[Doug Anderson]

Conor (and/or) Krzysztof and Rob,

On Mon, Jul 15, 2024 at 8:31 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Mon, Jul 15, 2024 at 02:15:37PM +0200, Stephan Gerhold wrote:
> > The Samsung ATNA45AF01 panel is an AMOLED eDP panel that has backlight
> > control over the DP AUX channel. While it works almost correctly with the
> > generic "edp-panel" compatible, the backlight needs special handling to
> > work correctly. It is similar to the existing ATNA33XC20 panel, just with
> > a larger resolution and size.
> >
> > Add a new "samsung,atna45af01" compatible to describe this panel in the DT.
> > Use the existing "samsung,atna33xc20" as fallback compatible since existing
> > drivers should work as-is, given that resolution and size are discoverable
> > through the eDP link.
> >
> > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>

Can you comment on whether you would consider this bindings a "Fix"
since it's a dependency for later patches in this series (which are
"Fix"es) to pass dtbs_check? See:

https://lore.kernel.org/r/4bca316a-2334-425b-87a6-e1bb241d26b5@linaro.org

-Doug

Re: [PATCH v2 1/4] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01

[Krzysztof Kozlowski]

On 18/07/2024 02:21, Doug Anderson wrote:
> Conor (and/or) Krzysztof and Rob,
> 
> On Mon, Jul 15, 2024 at 8:31 AM Conor Dooley <conor@kernel.org> wrote:
>>
>> On Mon, Jul 15, 2024 at 02:15:37PM +0200, Stephan Gerhold wrote:
>>> The Samsung ATNA45AF01 panel is an AMOLED eDP panel that has backlight
>>> control over the DP AUX channel. While it works almost correctly with the
>>> generic "edp-panel" compatible, the backlight needs special handling to
>>> work correctly. It is similar to the existing ATNA33XC20 panel, just with
>>> a larger resolution and size.
>>>
>>> Add a new "samsung,atna45af01" compatible to describe this panel in the DT.
>>> Use the existing "samsung,atna33xc20" as fallback compatible since existing
>>> drivers should work as-is, given that resolution and size are discoverable
>>> through the eDP link.
>>>
>>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
>>
>> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Can you comment on whether you would consider this bindings a "Fix"
> since it's a dependency for later patches in this series (which are
> "Fix"es) to pass dtbs_check? See:
> 
> https://lore.kernel.org/r/4bca316a-2334-425b-87a6-e1bb241d26b5@linaro.org

The patch itself is not a fix, for sure, but it might be a dependency of
a fix (which you wrote above), thus could be pulled to stable as a
dependency.

I do not care about dtbs_check warnings in stable kernels, mostly
because dtbs_check warnings depend heavily on dtschema and dtschema
follows mainline kernel. Basically if you had warnings-free v6.8 but try
to run dtbs_check now with latest dtschema, your results will differ.

At some point in the future, I could imagine "no new dtbs_check warnings
in stable kernels" requirement or at least preference, but so far I
don't think there is any benefit.

Best regards,
Krzysztof

Re: [PATCH v2 1/4] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01

[Doug Anderson]

Hi,

On Wed, Jul 17, 2024 at 11:19 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 18/07/2024 02:21, Doug Anderson wrote:
> > Conor (and/or) Krzysztof and Rob,
> >
> > On Mon, Jul 15, 2024 at 8:31 AM Conor Dooley <conor@kernel.org> wrote:
> >>
> >> On Mon, Jul 15, 2024 at 02:15:37PM +0200, Stephan Gerhold wrote:
> >>> The Samsung ATNA45AF01 panel is an AMOLED eDP panel that has backlight
> >>> control over the DP AUX channel. While it works almost correctly with the
> >>> generic "edp-panel" compatible, the backlight needs special handling to
> >>> work correctly. It is similar to the existing ATNA33XC20 panel, just with
> >>> a larger resolution and size.
> >>>
> >>> Add a new "samsung,atna45af01" compatible to describe this panel in the DT.
> >>> Use the existing "samsung,atna33xc20" as fallback compatible since existing
> >>> drivers should work as-is, given that resolution and size are discoverable
> >>> through the eDP link.
> >>>
> >>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> >>
> >> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> >
> > Can you comment on whether you would consider this bindings a "Fix"
> > since it's a dependency for later patches in this series (which are
> > "Fix"es) to pass dtbs_check? See:
> >
> > https://lore.kernel.org/r/4bca316a-2334-425b-87a6-e1bb241d26b5@linaro.org
>
> The patch itself is not a fix, for sure, but it might be a dependency of
> a fix (which you wrote above), thus could be pulled to stable as a
> dependency.
>
> I do not care about dtbs_check warnings in stable kernels, mostly
> because dtbs_check warnings depend heavily on dtschema and dtschema
> follows mainline kernel. Basically if you had warnings-free v6.8 but try
> to run dtbs_check now with latest dtschema, your results will differ.
>
> At some point in the future, I could imagine "no new dtbs_check warnings
> in stable kernels" requirement or at least preference, but so far I
> don't think there is any benefit.

In this case it's not about whether it makes it to the stable kernel
but about which main kernel it goes through.

If we land the bindings in drm-misc-next right now then it'll be a
long time before it makes it into Linus's tree because of the way that
drm-misc-next merges. It will make it to Linus's tree at 6.12. You can
see the drm-misc merging strategy at:

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

If we land the dts change (a fix) through the Qualcomm tree as a fix
then it should target 6.11.

This means that the 6.11 tree will have a dtbs_check error because it
has the dts change (a fix) but not the bindings change (not a fix).

One way to resolve this would be to treat this bindings as a "fix" and
land it through "drm-misc-fixes". That would make the bindings and
device tree change meet up in Linux 6.11.

Did I get that all correct?

-Doug

Re: [PATCH v2 1/4] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3253 bytes --]

On Thu, Jul 18, 2024 at 07:45:57AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Wed, Jul 17, 2024 at 11:19 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On 18/07/2024 02:21, Doug Anderson wrote:
> > > Conor (and/or) Krzysztof and Rob,
> > >
> > > On Mon, Jul 15, 2024 at 8:31 AM Conor Dooley <conor@kernel.org> wrote:
> > >>
> > >> On Mon, Jul 15, 2024 at 02:15:37PM +0200, Stephan Gerhold wrote:
> > >>> The Samsung ATNA45AF01 panel is an AMOLED eDP panel that has backlight
> > >>> control over the DP AUX channel. While it works almost correctly with the
> > >>> generic "edp-panel" compatible, the backlight needs special handling to
> > >>> work correctly. It is similar to the existing ATNA33XC20 panel, just with
> > >>> a larger resolution and size.
> > >>>
> > >>> Add a new "samsung,atna45af01" compatible to describe this panel in the DT.
> > >>> Use the existing "samsung,atna33xc20" as fallback compatible since existing
> > >>> drivers should work as-is, given that resolution and size are discoverable
> > >>> through the eDP link.
> > >>>
> > >>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > >>
> > >> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > >
> > > Can you comment on whether you would consider this bindings a "Fix"
> > > since it's a dependency for later patches in this series (which are
> > > "Fix"es) to pass dtbs_check? See:
> > >
> > > https://lore.kernel.org/r/4bca316a-2334-425b-87a6-e1bb241d26b5@linaro.org
> >
> > The patch itself is not a fix, for sure, but it might be a dependency of
> > a fix (which you wrote above), thus could be pulled to stable as a
> > dependency.
> >
> > I do not care about dtbs_check warnings in stable kernels, mostly
> > because dtbs_check warnings depend heavily on dtschema and dtschema
> > follows mainline kernel. Basically if you had warnings-free v6.8 but try
> > to run dtbs_check now with latest dtschema, your results will differ.
> >
> > At some point in the future, I could imagine "no new dtbs_check warnings
> > in stable kernels" requirement or at least preference, but so far I
> > don't think there is any benefit.
> 
> In this case it's not about whether it makes it to the stable kernel
> but about which main kernel it goes through.
> 
> If we land the bindings in drm-misc-next right now then it'll be a
> long time before it makes it into Linus's tree because of the way that
> drm-misc-next merges. It will make it to Linus's tree at 6.12. You can
> see the drm-misc merging strategy at:
> 
> https://drm.pages.freedesktop.org/maintainer-tools/drm-misc.html
> 
> If we land the dts change (a fix) through the Qualcomm tree as a fix
> then it should target 6.11.
> 
> This means that the 6.11 tree will have a dtbs_check error because it
> has the dts change (a fix) but not the bindings change (not a fix).
> 
> One way to resolve this would be to treat this bindings as a "fix" and
> land it through "drm-misc-fixes". That would make the bindings and
> device tree change meet up in Linux 6.11.
> 
> Did I get that all correct?

Is not not fairly established that a dependency for a fix can go onto a
fixes branch even if it is not a fix in and of itself?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/4] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01

[Doug Anderson]

Hi,

On Thu, Jul 18, 2024 at 7:56 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Thu, Jul 18, 2024 at 07:45:57AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Wed, Jul 17, 2024 at 11:19 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >
> > > On 18/07/2024 02:21, Doug Anderson wrote:
> > > > Conor (and/or) Krzysztof and Rob,
> > > >
> > > > On Mon, Jul 15, 2024 at 8:31 AM Conor Dooley <conor@kernel.org> wrote:
> > > >>
> > > >> On Mon, Jul 15, 2024 at 02:15:37PM +0200, Stephan Gerhold wrote:
> > > >>> The Samsung ATNA45AF01 panel is an AMOLED eDP panel that has backlight
> > > >>> control over the DP AUX channel. While it works almost correctly with the
> > > >>> generic "edp-panel" compatible, the backlight needs special handling to
> > > >>> work correctly. It is similar to the existing ATNA33XC20 panel, just with
> > > >>> a larger resolution and size.
> > > >>>
> > > >>> Add a new "samsung,atna45af01" compatible to describe this panel in the DT.
> > > >>> Use the existing "samsung,atna33xc20" as fallback compatible since existing
> > > >>> drivers should work as-is, given that resolution and size are discoverable
> > > >>> through the eDP link.
> > > >>>
> > > >>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > > >>
> > > >> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > >
> > > > Can you comment on whether you would consider this bindings a "Fix"
> > > > since it's a dependency for later patches in this series (which are
> > > > "Fix"es) to pass dtbs_check? See:
> > > >
> > > > https://lore.kernel.org/r/4bca316a-2334-425b-87a6-e1bb241d26b5@linaro.org
> > >
> > > The patch itself is not a fix, for sure, but it might be a dependency of
> > > a fix (which you wrote above), thus could be pulled to stable as a
> > > dependency.
> > >
> > > I do not care about dtbs_check warnings in stable kernels, mostly
> > > because dtbs_check warnings depend heavily on dtschema and dtschema
> > > follows mainline kernel. Basically if you had warnings-free v6.8 but try
> > > to run dtbs_check now with latest dtschema, your results will differ.
> > >
> > > At some point in the future, I could imagine "no new dtbs_check warnings
> > > in stable kernels" requirement or at least preference, but so far I
> > > don't think there is any benefit.
> >
> > In this case it's not about whether it makes it to the stable kernel
> > but about which main kernel it goes through.
> >
> > If we land the bindings in drm-misc-next right now then it'll be a
> > long time before it makes it into Linus's tree because of the way that
> > drm-misc-next merges. It will make it to Linus's tree at 6.12. You can
> > see the drm-misc merging strategy at:
> >
> > https://drm.pages.freedesktop.org/maintainer-tools/drm-misc.html
> >
> > If we land the dts change (a fix) through the Qualcomm tree as a fix
> > then it should target 6.11.
> >
> > This means that the 6.11 tree will have a dtbs_check error because it
> > has the dts change (a fix) but not the bindings change (not a fix).
> >
> > One way to resolve this would be to treat this bindings as a "fix" and
> > land it through "drm-misc-fixes". That would make the bindings and
> > device tree change meet up in Linux 6.11.
> >
> > Did I get that all correct?
>
> Is not not fairly established that a dependency for a fix can go onto a
> fixes branch even if it is not a fix in and of itself?

That would certainly be my take on it, but DT folks confirmation was
requested by Neil in:

https://lore.kernel.org/all/4bca316a-2334-425b-87a6-e1bb241d26b5@linaro.org/

-Doug

Re: [PATCH v2 1/4] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01

[Doug Anderson]

Hi,

On Thu, Jul 18, 2024 at 7:59 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Thu, Jul 18, 2024 at 7:56 AM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Thu, Jul 18, 2024 at 07:45:57AM -0700, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Wed, Jul 17, 2024 at 11:19 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > >
> > > > On 18/07/2024 02:21, Doug Anderson wrote:
> > > > > Conor (and/or) Krzysztof and Rob,
> > > > >
> > > > > On Mon, Jul 15, 2024 at 8:31 AM Conor Dooley <conor@kernel.org> wrote:
> > > > >>
> > > > >> On Mon, Jul 15, 2024 at 02:15:37PM +0200, Stephan Gerhold wrote:
> > > > >>> The Samsung ATNA45AF01 panel is an AMOLED eDP panel that has backlight
> > > > >>> control over the DP AUX channel. While it works almost correctly with the
> > > > >>> generic "edp-panel" compatible, the backlight needs special handling to
> > > > >>> work correctly. It is similar to the existing ATNA33XC20 panel, just with
> > > > >>> a larger resolution and size.
> > > > >>>
> > > > >>> Add a new "samsung,atna45af01" compatible to describe this panel in the DT.
> > > > >>> Use the existing "samsung,atna33xc20" as fallback compatible since existing
> > > > >>> drivers should work as-is, given that resolution and size are discoverable
> > > > >>> through the eDP link.
> > > > >>>
> > > > >>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > > > >>
> > > > >> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > > >
> > > > > Can you comment on whether you would consider this bindings a "Fix"
> > > > > since it's a dependency for later patches in this series (which are
> > > > > "Fix"es) to pass dtbs_check? See:
> > > > >
> > > > > https://lore.kernel.org/r/4bca316a-2334-425b-87a6-e1bb241d26b5@linaro.org
> > > >
> > > > The patch itself is not a fix, for sure, but it might be a dependency of
> > > > a fix (which you wrote above), thus could be pulled to stable as a
> > > > dependency.
> > > >
> > > > I do not care about dtbs_check warnings in stable kernels, mostly
> > > > because dtbs_check warnings depend heavily on dtschema and dtschema
> > > > follows mainline kernel. Basically if you had warnings-free v6.8 but try
> > > > to run dtbs_check now with latest dtschema, your results will differ.
> > > >
> > > > At some point in the future, I could imagine "no new dtbs_check warnings
> > > > in stable kernels" requirement or at least preference, but so far I
> > > > don't think there is any benefit.
> > >
> > > In this case it's not about whether it makes it to the stable kernel
> > > but about which main kernel it goes through.
> > >
> > > If we land the bindings in drm-misc-next right now then it'll be a
> > > long time before it makes it into Linus's tree because of the way that
> > > drm-misc-next merges. It will make it to Linus's tree at 6.12. You can
> > > see the drm-misc merging strategy at:
> > >
> > > https://drm.pages.freedesktop.org/maintainer-tools/drm-misc.html
> > >
> > > If we land the dts change (a fix) through the Qualcomm tree as a fix
> > > then it should target 6.11.
> > >
> > > This means that the 6.11 tree will have a dtbs_check error because it
> > > has the dts change (a fix) but not the bindings change (not a fix).
> > >
> > > One way to resolve this would be to treat this bindings as a "fix" and
> > > land it through "drm-misc-fixes". That would make the bindings and
> > > device tree change meet up in Linux 6.11.
> > >
> > > Did I get that all correct?
> >
> > Is not not fairly established that a dependency for a fix can go onto a
> > fixes branch even if it is not a fix in and of itself?
>
> That would certainly be my take on it, but DT folks confirmation was
> requested by Neil in:
>
> https://lore.kernel.org/all/4bca316a-2334-425b-87a6-e1bb241d26b5@linaro.org/

FWIW, I'd rather not let this stagnate too long. I'm fairly confident
in my assertion that this should go into drm-misc-fixes. I'll give it
until Monday and then I'm just going to land this bindings change in
drm-misc-fixes. Shout soon if you feel strongly that I shouldn't do
this. If someone wants to flame me after the fact then so be it.

-Doug

Re: [PATCH v2 1/4] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4661 bytes --]

On Fri, Jul 19, 2024 at 10:07:29AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jul 18, 2024 at 7:59 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Thu, Jul 18, 2024 at 7:56 AM Conor Dooley <conor@kernel.org> wrote:
> > >
> > > On Thu, Jul 18, 2024 at 07:45:57AM -0700, Doug Anderson wrote:
> > > > Hi,
> > > >
> > > > On Wed, Jul 17, 2024 at 11:19 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > > >
> > > > > On 18/07/2024 02:21, Doug Anderson wrote:
> > > > > > Conor (and/or) Krzysztof and Rob,
> > > > > >
> > > > > > On Mon, Jul 15, 2024 at 8:31 AM Conor Dooley <conor@kernel.org> wrote:
> > > > > >>
> > > > > >> On Mon, Jul 15, 2024 at 02:15:37PM +0200, Stephan Gerhold wrote:
> > > > > >>> The Samsung ATNA45AF01 panel is an AMOLED eDP panel that has backlight
> > > > > >>> control over the DP AUX channel. While it works almost correctly with the
> > > > > >>> generic "edp-panel" compatible, the backlight needs special handling to
> > > > > >>> work correctly. It is similar to the existing ATNA33XC20 panel, just with
> > > > > >>> a larger resolution and size.
> > > > > >>>
> > > > > >>> Add a new "samsung,atna45af01" compatible to describe this panel in the DT.
> > > > > >>> Use the existing "samsung,atna33xc20" as fallback compatible since existing
> > > > > >>> drivers should work as-is, given that resolution and size are discoverable
> > > > > >>> through the eDP link.
> > > > > >>>
> > > > > >>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > > > > >>
> > > > > >> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > > > >
> > > > > > Can you comment on whether you would consider this bindings a "Fix"
> > > > > > since it's a dependency for later patches in this series (which are
> > > > > > "Fix"es) to pass dtbs_check? See:
> > > > > >
> > > > > > https://lore.kernel.org/r/4bca316a-2334-425b-87a6-e1bb241d26b5@linaro.org
> > > > >
> > > > > The patch itself is not a fix, for sure, but it might be a dependency of
> > > > > a fix (which you wrote above), thus could be pulled to stable as a
> > > > > dependency.
> > > > >
> > > > > I do not care about dtbs_check warnings in stable kernels, mostly
> > > > > because dtbs_check warnings depend heavily on dtschema and dtschema
> > > > > follows mainline kernel. Basically if you had warnings-free v6.8 but try
> > > > > to run dtbs_check now with latest dtschema, your results will differ.
> > > > >
> > > > > At some point in the future, I could imagine "no new dtbs_check warnings
> > > > > in stable kernels" requirement or at least preference, but so far I
> > > > > don't think there is any benefit.
> > > >
> > > > In this case it's not about whether it makes it to the stable kernel
> > > > but about which main kernel it goes through.
> > > >
> > > > If we land the bindings in drm-misc-next right now then it'll be a
> > > > long time before it makes it into Linus's tree because of the way that
> > > > drm-misc-next merges. It will make it to Linus's tree at 6.12. You can
> > > > see the drm-misc merging strategy at:
> > > >
> > > > https://drm.pages.freedesktop.org/maintainer-tools/drm-misc.html
> > > >
> > > > If we land the dts change (a fix) through the Qualcomm tree as a fix
> > > > then it should target 6.11.
> > > >
> > > > This means that the 6.11 tree will have a dtbs_check error because it
> > > > has the dts change (a fix) but not the bindings change (not a fix).
> > > >
> > > > One way to resolve this would be to treat this bindings as a "fix" and
> > > > land it through "drm-misc-fixes". That would make the bindings and
> > > > device tree change meet up in Linux 6.11.
> > > >
> > > > Did I get that all correct?
> > >
> > > Is not not fairly established that a dependency for a fix can go onto a
> > > fixes branch even if it is not a fix in and of itself?
> >
> > That would certainly be my take on it, but DT folks confirmation was
> > requested by Neil in:
> >
> > https://lore.kernel.org/all/4bca316a-2334-425b-87a6-e1bb241d26b5@linaro.org/
> 
> FWIW, I'd rather not let this stagnate too long.

I dunno if you were waiting for me (or Krzk/Rob) to reply, but I didn't
cos I figured I'd already pretty much said there was nothing wrong with
the prereq being on -fixes too.

> I'm fairly confident
> in my assertion that this should go into drm-misc-fixes. I'll give it
> until Monday and then I'm just going to land this bindings change in
> drm-misc-fixes. Shout soon if you feel strongly that I shouldn't do
> this. If someone wants to flame me after the fact then so be it.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/4] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01

[Doug Anderson]

Hi,

On Fri, Jul 19, 2024 at 10:07 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Thu, Jul 18, 2024 at 7:59 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Thu, Jul 18, 2024 at 7:56 AM Conor Dooley <conor@kernel.org> wrote:
> > >
> > > On Thu, Jul 18, 2024 at 07:45:57AM -0700, Doug Anderson wrote:
> > > > Hi,
> > > >
> > > > On Wed, Jul 17, 2024 at 11:19 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > > >
> > > > > On 18/07/2024 02:21, Doug Anderson wrote:
> > > > > > Conor (and/or) Krzysztof and Rob,
> > > > > >
> > > > > > On Mon, Jul 15, 2024 at 8:31 AM Conor Dooley <conor@kernel.org> wrote:
> > > > > >>
> > > > > >> On Mon, Jul 15, 2024 at 02:15:37PM +0200, Stephan Gerhold wrote:
> > > > > >>> The Samsung ATNA45AF01 panel is an AMOLED eDP panel that has backlight
> > > > > >>> control over the DP AUX channel. While it works almost correctly with the
> > > > > >>> generic "edp-panel" compatible, the backlight needs special handling to
> > > > > >>> work correctly. It is similar to the existing ATNA33XC20 panel, just with
> > > > > >>> a larger resolution and size.
> > > > > >>>
> > > > > >>> Add a new "samsung,atna45af01" compatible to describe this panel in the DT.
> > > > > >>> Use the existing "samsung,atna33xc20" as fallback compatible since existing
> > > > > >>> drivers should work as-is, given that resolution and size are discoverable
> > > > > >>> through the eDP link.
> > > > > >>>
> > > > > >>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > > > > >>
> > > > > >> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > > > >
> > > > > > Can you comment on whether you would consider this bindings a "Fix"
> > > > > > since it's a dependency for later patches in this series (which are
> > > > > > "Fix"es) to pass dtbs_check? See:
> > > > > >
> > > > > > https://lore.kernel.org/r/4bca316a-2334-425b-87a6-e1bb241d26b5@linaro.org
> > > > >
> > > > > The patch itself is not a fix, for sure, but it might be a dependency of
> > > > > a fix (which you wrote above), thus could be pulled to stable as a
> > > > > dependency.
> > > > >
> > > > > I do not care about dtbs_check warnings in stable kernels, mostly
> > > > > because dtbs_check warnings depend heavily on dtschema and dtschema
> > > > > follows mainline kernel. Basically if you had warnings-free v6.8 but try
> > > > > to run dtbs_check now with latest dtschema, your results will differ.
> > > > >
> > > > > At some point in the future, I could imagine "no new dtbs_check warnings
> > > > > in stable kernels" requirement or at least preference, but so far I
> > > > > don't think there is any benefit.
> > > >
> > > > In this case it's not about whether it makes it to the stable kernel
> > > > but about which main kernel it goes through.
> > > >
> > > > If we land the bindings in drm-misc-next right now then it'll be a
> > > > long time before it makes it into Linus's tree because of the way that
> > > > drm-misc-next merges. It will make it to Linus's tree at 6.12. You can
> > > > see the drm-misc merging strategy at:
> > > >
> > > > https://drm.pages.freedesktop.org/maintainer-tools/drm-misc.html
> > > >
> > > > If we land the dts change (a fix) through the Qualcomm tree as a fix
> > > > then it should target 6.11.
> > > >
> > > > This means that the 6.11 tree will have a dtbs_check error because it
> > > > has the dts change (a fix) but not the bindings change (not a fix).
> > > >
> > > > One way to resolve this would be to treat this bindings as a "fix" and
> > > > land it through "drm-misc-fixes". That would make the bindings and
> > > > device tree change meet up in Linux 6.11.
> > > >
> > > > Did I get that all correct?
> > >
> > > Is not not fairly established that a dependency for a fix can go onto a
> > > fixes branch even if it is not a fix in and of itself?
> >
> > That would certainly be my take on it, but DT folks confirmation was
> > requested by Neil in:
> >
> > https://lore.kernel.org/all/4bca316a-2334-425b-87a6-e1bb241d26b5@linaro.org/
>
> FWIW, I'd rather not let this stagnate too long. I'm fairly confident
> in my assertion that this should go into drm-misc-fixes. I'll give it
> until Monday and then I'm just going to land this bindings change in
> drm-misc-fixes. Shout soon if you feel strongly that I shouldn't do
> this. If someone wants to flame me after the fact then so be it.

Landed in drm-misc-next fixes as per the flow chart [1] since the
"samsung,atna33xc20.yaml" split from "panel-simple.yaml" is in
mainline linux but not in any "rc" candidates yet.

[1/4] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01
      commit: b6f7d984ebf826069d3dc6fa187b4d1cfb90f965

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

-Doug

[PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[Stephan Gerhold]

This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.

The panel should be handled through the samsung-atna33xc20 driver for
correct power up timings. Otherwise the backlight does not work correctly.

We have existing users of this panel through the generic "edp-panel"
compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
partially in that configuration: It works after boot but once the screen
gets disabled it does not turn on again until after reboot. It behaves the
same way with the default "conservative" timings, so we might as well drop
the configuration from the panel-edp driver. That way, users with old DTBs
will get a warning and can move to the new driver.

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
 drivers/gpu/drm/panel/panel-edp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index 3a574a9b46e7..d2d682385e89 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1960,8 +1960,6 @@ static const struct edp_panel_entry edp_panels[] = {
 	EDP_PANEL_ENTRY('L', 'G', 'D', 0x05af, &delay_200_500_e200_d200, "Unknown"),
 	EDP_PANEL_ENTRY('L', 'G', 'D', 0x05f1, &delay_200_500_e200_d200, "Unknown"),
 
-	EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200, "ATNA45AF01"),
-
 	EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50, "LQ140M1JW48"),
 	EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &delay_80_500_e50, "LQ140M1JW46"),
 	EDP_PANEL_ENTRY('S', 'H', 'P', 0x153a, &delay_200_500_e50, "LQ140T1JH01"),

-- 
2.44.1

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[Johan Hovold]

On Mon, Jul 15, 2024 at 02:15:38PM +0200, Stephan Gerhold wrote:
> This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.
> 
> The panel should be handled through the samsung-atna33xc20 driver for
> correct power up timings. Otherwise the backlight does not work correctly.
> 
> We have existing users of this panel through the generic "edp-panel"
> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> partially in that configuration: It works after boot but once the screen
> gets disabled it does not turn on again until after reboot. It behaves the
> same way with the default "conservative" timings, so we might as well drop
> the configuration from the panel-edp driver. That way, users with old DTBs
> will get a warning and can move to the new driver.
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[Neil Armstrong]

Hi,

On 15/07/2024 14:15, Stephan Gerhold wrote:
> This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.
> 
> The panel should be handled through the samsung-atna33xc20 driver for
> correct power up timings. Otherwise the backlight does not work correctly.
> 
> We have existing users of this panel through the generic "edp-panel"
> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> partially in that configuration: It works after boot but once the screen
> gets disabled it does not turn on again until after reboot. It behaves the
> same way with the default "conservative" timings, so we might as well drop
> the configuration from the panel-edp driver. That way, users with old DTBs
> will get a warning and can move to the new driver.
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
>   drivers/gpu/drm/panel/panel-edp.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> index 3a574a9b46e7..d2d682385e89 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -1960,8 +1960,6 @@ static const struct edp_panel_entry edp_panels[] = {
>   	EDP_PANEL_ENTRY('L', 'G', 'D', 0x05af, &delay_200_500_e200_d200, "Unknown"),
>   	EDP_PANEL_ENTRY('L', 'G', 'D', 0x05f1, &delay_200_500_e200_d200, "Unknown"),
>   
> -	EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200, "ATNA45AF01"),
> -
>   	EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50, "LQ140M1JW48"),
>   	EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &delay_80_500_e50, "LQ140M1JW46"),
>   	EDP_PANEL_ENTRY('S', 'H', 'P', 0x153a, &delay_200_500_e50, "LQ140T1JH01"),
> 

How will we handle current/old crd DT with new kernels ?

Same question for patch 3, thie serie introduces a bindings that won't be valid
if we backport patch 3. I don't think patch should be backported, and this patch
should be dropped.

Neil

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[Stephan Gerhold]

On Mon, Jul 15, 2024 at 02:42:12PM +0200, Neil Armstrong wrote:
> On 15/07/2024 14:15, Stephan Gerhold wrote:
> > This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.
> > 
> > The panel should be handled through the samsung-atna33xc20 driver for
> > correct power up timings. Otherwise the backlight does not work correctly.
> > 
> > We have existing users of this panel through the generic "edp-panel"
> > compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> > partially in that configuration: It works after boot but once the screen
> > gets disabled it does not turn on again until after reboot. It behaves the
> > same way with the default "conservative" timings, so we might as well drop
> > the configuration from the panel-edp driver. That way, users with old DTBs
> > will get a warning and can move to the new driver.
> > 
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > ---
> >   drivers/gpu/drm/panel/panel-edp.c | 2 --
> >   1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > index 3a574a9b46e7..d2d682385e89 100644
> > --- a/drivers/gpu/drm/panel/panel-edp.c
> > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > @@ -1960,8 +1960,6 @@ static const struct edp_panel_entry edp_panels[] = {
> >   	EDP_PANEL_ENTRY('L', 'G', 'D', 0x05af, &delay_200_500_e200_d200, "Unknown"),
> >   	EDP_PANEL_ENTRY('L', 'G', 'D', 0x05f1, &delay_200_500_e200_d200, "Unknown"),
> > -	EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200, "ATNA45AF01"),
> > -
> >   	EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50, "LQ140M1JW48"),
> >   	EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &delay_80_500_e50, "LQ140M1JW46"),
> >   	EDP_PANEL_ENTRY('S', 'H', 'P', 0x153a, &delay_200_500_e50, "LQ140T1JH01"),
> > 
> 
> How will we handle current/old crd DT with new kernels ?
> 

I think this is answered in the commit message:

> > We have existing users of this panel through the generic "edp-panel"
> > compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> > partially in that configuration: It works after boot but once the screen
> > gets disabled it does not turn on again until after reboot. It behaves the
> > same way with the default "conservative" timings, so we might as well drop
> > the configuration from the panel-edp driver. That way, users with old DTBs
> > will get a warning and can move to the new driver.

Basically with the entry removed, the panel-edp driver will fallback to
default "conservative" timings when using old DTBs. There will be a
warning in dmesg, but otherwise the panel will somewhat work just as
before. I think this is a good way to remind users to upgrade.

> Same question for patch 3, thie serie introduces a bindings that won't be valid
> if we backport patch 3. I don't think patch should be backported, and this patch
> should be dropped.

There would be a dtbs_check warning, yeah. Functionally, it would work
just fine. Is that reason enough to keep display partially broken for
6.11? We could also apply the minor binding change for 6.11 if needed.

I'm also fine if this just goes into 6.12 though.

Thanks,
Stephan

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[Neil Armstrong]

On 15/07/2024 14:54, Stephan Gerhold wrote:
> On Mon, Jul 15, 2024 at 02:42:12PM +0200, Neil Armstrong wrote:
>> On 15/07/2024 14:15, Stephan Gerhold wrote:
>>> This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.
>>>
>>> The panel should be handled through the samsung-atna33xc20 driver for
>>> correct power up timings. Otherwise the backlight does not work correctly.
>>>
>>> We have existing users of this panel through the generic "edp-panel"
>>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
>>> partially in that configuration: It works after boot but once the screen
>>> gets disabled it does not turn on again until after reboot. It behaves the
>>> same way with the default "conservative" timings, so we might as well drop
>>> the configuration from the panel-edp driver. That way, users with old DTBs
>>> will get a warning and can move to the new driver.
>>>
>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
>>> ---
>>>    drivers/gpu/drm/panel/panel-edp.c | 2 --
>>>    1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
>>> index 3a574a9b46e7..d2d682385e89 100644
>>> --- a/drivers/gpu/drm/panel/panel-edp.c
>>> +++ b/drivers/gpu/drm/panel/panel-edp.c
>>> @@ -1960,8 +1960,6 @@ static const struct edp_panel_entry edp_panels[] = {
>>>    	EDP_PANEL_ENTRY('L', 'G', 'D', 0x05af, &delay_200_500_e200_d200, "Unknown"),
>>>    	EDP_PANEL_ENTRY('L', 'G', 'D', 0x05f1, &delay_200_500_e200_d200, "Unknown"),
>>> -	EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200, "ATNA45AF01"),
>>> -
>>>    	EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50, "LQ140M1JW48"),
>>>    	EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &delay_80_500_e50, "LQ140M1JW46"),
>>>    	EDP_PANEL_ENTRY('S', 'H', 'P', 0x153a, &delay_200_500_e50, "LQ140T1JH01"),
>>>
>>
>> How will we handle current/old crd DT with new kernels ?
>>
> 
> I think this is answered in the commit message:
> 
>>> We have existing users of this panel through the generic "edp-panel"
>>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
>>> partially in that configuration: It works after boot but once the screen
>>> gets disabled it does not turn on again until after reboot. It behaves the
>>> same way with the default "conservative" timings, so we might as well drop
>>> the configuration from the panel-edp driver. That way, users with old DTBs
>>> will get a warning and can move to the new driver.
> 
> Basically with the entry removed, the panel-edp driver will fallback to
> default "conservative" timings when using old DTBs. There will be a
> warning in dmesg, but otherwise the panel will somewhat work just as
> before. I think this is a good way to remind users to upgrade.

I consider this as a regression

> 
>> Same question for patch 3, thie serie introduces a bindings that won't be valid
>> if we backport patch 3. I don't think patch should be backported, and this patch
>> should be dropped.
> 
> There would be a dtbs_check warning, yeah. Functionally, it would work
> just fine. Is that reason enough to keep display partially broken for
> 6.11? We could also apply the minor binding change for 6.11 if needed.

I don't know how to answer this, I'll let the DT maintainer comment this.

The problem is I do not think we can pass the whole patchset as fixes
for v6.11, patches 2 & 3 could, patches 1 & 4 definitely can't.

Neil

> 
> I'm also fine if this just goes into 6.12 though.
> 
> Thanks,
> Stephan

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[Stephan Gerhold]

On Mon, Jul 15, 2024 at 03:01:57PM +0200, Neil Armstrong wrote:
> On 15/07/2024 14:54, Stephan Gerhold wrote:
> > On Mon, Jul 15, 2024 at 02:42:12PM +0200, Neil Armstrong wrote:
> > > On 15/07/2024 14:15, Stephan Gerhold wrote:
> > > > This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.
> > > > 
> > > > The panel should be handled through the samsung-atna33xc20 driver for
> > > > correct power up timings. Otherwise the backlight does not work correctly.
> > > > 
> > > > We have existing users of this panel through the generic "edp-panel"
> > > > compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> > > > partially in that configuration: It works after boot but once the screen
> > > > gets disabled it does not turn on again until after reboot. It behaves the
> > > > same way with the default "conservative" timings, so we might as well drop
> > > > the configuration from the panel-edp driver. That way, users with old DTBs
> > > > will get a warning and can move to the new driver.
> > > > 
> > > > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > > > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > > > ---
> > > >    drivers/gpu/drm/panel/panel-edp.c | 2 --
> > > >    1 file changed, 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > > > index 3a574a9b46e7..d2d682385e89 100644
> > > > --- a/drivers/gpu/drm/panel/panel-edp.c
> > > > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > > > @@ -1960,8 +1960,6 @@ static const struct edp_panel_entry edp_panels[] = {
> > > >    	EDP_PANEL_ENTRY('L', 'G', 'D', 0x05af, &delay_200_500_e200_d200, "Unknown"),
> > > >    	EDP_PANEL_ENTRY('L', 'G', 'D', 0x05f1, &delay_200_500_e200_d200, "Unknown"),
> > > > -	EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200, "ATNA45AF01"),
> > > > -
> > > >    	EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50, "LQ140M1JW48"),
> > > >    	EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &delay_80_500_e50, "LQ140M1JW46"),
> > > >    	EDP_PANEL_ENTRY('S', 'H', 'P', 0x153a, &delay_200_500_e50, "LQ140T1JH01"),
> > > > 
> > > 
> > > How will we handle current/old crd DT with new kernels ?
> > > 
> > 
> > I think this is answered in the commit message:
> > 
> > > > We have existing users of this panel through the generic "edp-panel"
> > > > compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> > > > partially in that configuration: It works after boot but once the screen
> > > > gets disabled it does not turn on again until after reboot. It behaves the
> > > > same way with the default "conservative" timings, so we might as well drop
> > > > the configuration from the panel-edp driver. That way, users with old DTBs
> > > > will get a warning and can move to the new driver.
> > 
> > Basically with the entry removed, the panel-edp driver will fallback to
> > default "conservative" timings when using old DTBs. There will be a
> > warning in dmesg, but otherwise the panel will somewhat work just as
> > before. I think this is a good way to remind users to upgrade.
> 
> I consider this as a regression
> 

Personally, I don't think we can regress something that was already
broken. There is no point in continuing to use the broken state - it is
rather frustrating if your display goes off for power saving or suspend
and you cannot get it back on until you reboot.

> > 
> > > Same question for patch 3, thie serie introduces a bindings that won't be valid
> > > if we backport patch 3. I don't think patch should be backported, and this patch
> > > should be dropped.
> > 
> > There would be a dtbs_check warning, yeah. Functionally, it would work
> > just fine. Is that reason enough to keep display partially broken for
> > 6.11? We could also apply the minor binding change for 6.11 if needed.
> 
> I don't know how to answer this, I'll let the DT maintainer comment this.
> 
> The problem is I do not think we can pass the whole patchset as fixes
> for v6.11, patches 2 & 3 could, patches 1 & 4 definitely can't.
> 

Fair enough, I'm also fine if these patches go just into 6.12. I think
there are no changes in the patches needed for that, the Fixes tag is
still appropriate and I intentionally omitted the Cc stable tag.

Thanks,
Stephan

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[Doug Anderson]

Hi,

On Mon, Jul 15, 2024 at 6:02 AM Neil Armstrong
<neil.armstrong@linaro.org> wrote:
>
> On 15/07/2024 14:54, Stephan Gerhold wrote:
> > On Mon, Jul 15, 2024 at 02:42:12PM +0200, Neil Armstrong wrote:
> >> On 15/07/2024 14:15, Stephan Gerhold wrote:
> >>> This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.
> >>>
> >>> The panel should be handled through the samsung-atna33xc20 driver for
> >>> correct power up timings. Otherwise the backlight does not work correctly.
> >>>
> >>> We have existing users of this panel through the generic "edp-panel"
> >>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> >>> partially in that configuration: It works after boot but once the screen
> >>> gets disabled it does not turn on again until after reboot. It behaves the
> >>> same way with the default "conservative" timings, so we might as well drop
> >>> the configuration from the panel-edp driver. That way, users with old DTBs
> >>> will get a warning and can move to the new driver.
> >>>
> >>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> >>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> >>> ---
> >>>    drivers/gpu/drm/panel/panel-edp.c | 2 --
> >>>    1 file changed, 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> >>> index 3a574a9b46e7..d2d682385e89 100644
> >>> --- a/drivers/gpu/drm/panel/panel-edp.c
> >>> +++ b/drivers/gpu/drm/panel/panel-edp.c
> >>> @@ -1960,8 +1960,6 @@ static const struct edp_panel_entry edp_panels[] = {
> >>>     EDP_PANEL_ENTRY('L', 'G', 'D', 0x05af, &delay_200_500_e200_d200, "Unknown"),
> >>>     EDP_PANEL_ENTRY('L', 'G', 'D', 0x05f1, &delay_200_500_e200_d200, "Unknown"),
> >>> -   EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200, "ATNA45AF01"),
> >>> -
> >>>     EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50, "LQ140M1JW48"),
> >>>     EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &delay_80_500_e50, "LQ140M1JW46"),
> >>>     EDP_PANEL_ENTRY('S', 'H', 'P', 0x153a, &delay_200_500_e50, "LQ140T1JH01"),
> >>>
> >>
> >> How will we handle current/old crd DT with new kernels ?
> >>
> >
> > I think this is answered in the commit message:
> >
> >>> We have existing users of this panel through the generic "edp-panel"
> >>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> >>> partially in that configuration: It works after boot but once the screen
> >>> gets disabled it does not turn on again until after reboot. It behaves the
> >>> same way with the default "conservative" timings, so we might as well drop
> >>> the configuration from the panel-edp driver. That way, users with old DTBs
> >>> will get a warning and can move to the new driver.
> >
> > Basically with the entry removed, the panel-edp driver will fallback to
> > default "conservative" timings when using old DTBs. There will be a
> > warning in dmesg, but otherwise the panel will somewhat work just as
> > before. I think this is a good way to remind users to upgrade.
>
> I consider this as a regression
>
> >
> >> Same question for patch 3, thie serie introduces a bindings that won't be valid
> >> if we backport patch 3. I don't think patch should be backported, and this patch
> >> should be dropped.
> >
> > There would be a dtbs_check warning, yeah. Functionally, it would work
> > just fine. Is that reason enough to keep display partially broken for
> > 6.11? We could also apply the minor binding change for 6.11 if needed.
>
> I don't know how to answer this, I'll let the DT maintainer comment this.
>
> The problem is I do not think we can pass the whole patchset as fixes
> for v6.11, patches 2 & 3 could, patches 1 & 4 definitely can't.
>
> Neil

IMO: patch #3 (dts) and #4 (CONFIG) go through the Qualcomm tree
whenever those folks agree to it. If we're worried about the
dtbs_check breakage I personally wouldn't mind "Ack"ing patch #1 to go
through the Qualcomm tree as long as it made it into 6.11-rc1. I have
a hunch that there are going to be more Samsung OLED panels in the
future that will need to touch the same file, but if the change is in
-rc1 it should make it back into drm-misc quickly, right?

Personally I think patch #2 could go in anytime since, as people have
said, things are pretty broken today and the worst that happens is
that someone gets an extra warning. That would be my preference. That
being said, we could also snooze that patch for a month or two and
land it later. There's no real hurry.

-Doug

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[neil.armstrong]

On 15/07/2024 15:51, Doug Anderson wrote:
> Hi,
> 
> On Mon, Jul 15, 2024 at 6:02 AM Neil Armstrong
> <neil.armstrong@linaro.org> wrote:
>>
>> On 15/07/2024 14:54, Stephan Gerhold wrote:
>>> On Mon, Jul 15, 2024 at 02:42:12PM +0200, Neil Armstrong wrote:
>>>> On 15/07/2024 14:15, Stephan Gerhold wrote:
>>>>> This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.
>>>>>
>>>>> The panel should be handled through the samsung-atna33xc20 driver for
>>>>> correct power up timings. Otherwise the backlight does not work correctly.
>>>>>
>>>>> We have existing users of this panel through the generic "edp-panel"
>>>>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
>>>>> partially in that configuration: It works after boot but once the screen
>>>>> gets disabled it does not turn on again until after reboot. It behaves the
>>>>> same way with the default "conservative" timings, so we might as well drop
>>>>> the configuration from the panel-edp driver. That way, users with old DTBs
>>>>> will get a warning and can move to the new driver.
>>>>>
>>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>>>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
>>>>> ---
>>>>>     drivers/gpu/drm/panel/panel-edp.c | 2 --
>>>>>     1 file changed, 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
>>>>> index 3a574a9b46e7..d2d682385e89 100644
>>>>> --- a/drivers/gpu/drm/panel/panel-edp.c
>>>>> +++ b/drivers/gpu/drm/panel/panel-edp.c
>>>>> @@ -1960,8 +1960,6 @@ static const struct edp_panel_entry edp_panels[] = {
>>>>>      EDP_PANEL_ENTRY('L', 'G', 'D', 0x05af, &delay_200_500_e200_d200, "Unknown"),
>>>>>      EDP_PANEL_ENTRY('L', 'G', 'D', 0x05f1, &delay_200_500_e200_d200, "Unknown"),
>>>>> -   EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200, "ATNA45AF01"),
>>>>> -
>>>>>      EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50, "LQ140M1JW48"),
>>>>>      EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &delay_80_500_e50, "LQ140M1JW46"),
>>>>>      EDP_PANEL_ENTRY('S', 'H', 'P', 0x153a, &delay_200_500_e50, "LQ140T1JH01"),
>>>>>
>>>>
>>>> How will we handle current/old crd DT with new kernels ?
>>>>
>>>
>>> I think this is answered in the commit message:
>>>
>>>>> We have existing users of this panel through the generic "edp-panel"
>>>>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
>>>>> partially in that configuration: It works after boot but once the screen
>>>>> gets disabled it does not turn on again until after reboot. It behaves the
>>>>> same way with the default "conservative" timings, so we might as well drop
>>>>> the configuration from the panel-edp driver. That way, users with old DTBs
>>>>> will get a warning and can move to the new driver.
>>>
>>> Basically with the entry removed, the panel-edp driver will fallback to
>>> default "conservative" timings when using old DTBs. There will be a
>>> warning in dmesg, but otherwise the panel will somewhat work just as
>>> before. I think this is a good way to remind users to upgrade.
>>
>> I consider this as a regression
>>
>>>
>>>> Same question for patch 3, thie serie introduces a bindings that won't be valid
>>>> if we backport patch 3. I don't think patch should be backported, and this patch
>>>> should be dropped.
>>>
>>> There would be a dtbs_check warning, yeah. Functionally, it would work
>>> just fine. Is that reason enough to keep display partially broken for
>>> 6.11? We could also apply the minor binding change for 6.11 if needed.
>>
>> I don't know how to answer this, I'll let the DT maintainer comment this.
>>
>> The problem is I do not think we can pass the whole patchset as fixes
>> for v6.11, patches 2 & 3 could, patches 1 & 4 definitely can't.
>>
>> Neil
> 
> IMO: patch #3 (dts) and #4 (CONFIG) go through the Qualcomm tree
> whenever those folks agree to it. If we're worried about the
> dtbs_check breakage I personally wouldn't mind "Ack"ing patch #1 to go
> through the Qualcomm tree as long as it made it into 6.11-rc1. I have
> a hunch that there are going to be more Samsung OLED panels in the
> future that will need to touch the same file, but if the change is in
> -rc1 it should make it back into drm-misc quickly, right?

Not sure the Soc maintainer would accept any patches for -rc1 at this
point, but Bjorn can try to push both #3 and #4 as fixes for -rc2.

Not sure #1 would be accepted as fix via any tree, but having a bindings
error for a kernel release is not a big deal if in-fine the bindings change
has been reviewed and queued for next version.

Basically when the -rc is tagged it gets backmerged into drm-misc-next,
so we can backmerge any -rc we want.

> 
> Personally I think patch #2 could go in anytime since, as people have
> said, things are pretty broken today and the worst that happens is
> that someone gets an extra warning. That would be my preference. That
> being said, we could also snooze that patch for a month or two and
> land it later. There's no real hurry.

We can push it now to drm-misc-next so it gets naturally delayed until v6.12

Neil

> 
> -Doug
> 

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[Doug Anderson]

Hi,

On Mon, Jul 15, 2024 at 6:57 AM <neil.armstrong@linaro.org> wrote:
>
> On 15/07/2024 15:51, Doug Anderson wrote:
> > Hi,
> >
> > On Mon, Jul 15, 2024 at 6:02 AM Neil Armstrong
> > <neil.armstrong@linaro.org> wrote:
> >>
> >> On 15/07/2024 14:54, Stephan Gerhold wrote:
> >>> On Mon, Jul 15, 2024 at 02:42:12PM +0200, Neil Armstrong wrote:
> >>>> On 15/07/2024 14:15, Stephan Gerhold wrote:
> >>>>> This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.
> >>>>>
> >>>>> The panel should be handled through the samsung-atna33xc20 driver for
> >>>>> correct power up timings. Otherwise the backlight does not work correctly.
> >>>>>
> >>>>> We have existing users of this panel through the generic "edp-panel"
> >>>>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> >>>>> partially in that configuration: It works after boot but once the screen
> >>>>> gets disabled it does not turn on again until after reboot. It behaves the
> >>>>> same way with the default "conservative" timings, so we might as well drop
> >>>>> the configuration from the panel-edp driver. That way, users with old DTBs
> >>>>> will get a warning and can move to the new driver.
> >>>>>
> >>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> >>>>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> >>>>> ---
> >>>>>     drivers/gpu/drm/panel/panel-edp.c | 2 --
> >>>>>     1 file changed, 2 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> >>>>> index 3a574a9b46e7..d2d682385e89 100644
> >>>>> --- a/drivers/gpu/drm/panel/panel-edp.c
> >>>>> +++ b/drivers/gpu/drm/panel/panel-edp.c
> >>>>> @@ -1960,8 +1960,6 @@ static const struct edp_panel_entry edp_panels[] = {
> >>>>>      EDP_PANEL_ENTRY('L', 'G', 'D', 0x05af, &delay_200_500_e200_d200, "Unknown"),
> >>>>>      EDP_PANEL_ENTRY('L', 'G', 'D', 0x05f1, &delay_200_500_e200_d200, "Unknown"),
> >>>>> -   EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200, "ATNA45AF01"),
> >>>>> -
> >>>>>      EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50, "LQ140M1JW48"),
> >>>>>      EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &delay_80_500_e50, "LQ140M1JW46"),
> >>>>>      EDP_PANEL_ENTRY('S', 'H', 'P', 0x153a, &delay_200_500_e50, "LQ140T1JH01"),
> >>>>>
> >>>>
> >>>> How will we handle current/old crd DT with new kernels ?
> >>>>
> >>>
> >>> I think this is answered in the commit message:
> >>>
> >>>>> We have existing users of this panel through the generic "edp-panel"
> >>>>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> >>>>> partially in that configuration: It works after boot but once the screen
> >>>>> gets disabled it does not turn on again until after reboot. It behaves the
> >>>>> same way with the default "conservative" timings, so we might as well drop
> >>>>> the configuration from the panel-edp driver. That way, users with old DTBs
> >>>>> will get a warning and can move to the new driver.
> >>>
> >>> Basically with the entry removed, the panel-edp driver will fallback to
> >>> default "conservative" timings when using old DTBs. There will be a
> >>> warning in dmesg, but otherwise the panel will somewhat work just as
> >>> before. I think this is a good way to remind users to upgrade.
> >>
> >> I consider this as a regression
> >>
> >>>
> >>>> Same question for patch 3, thie serie introduces a bindings that won't be valid
> >>>> if we backport patch 3. I don't think patch should be backported, and this patch
> >>>> should be dropped.
> >>>
> >>> There would be a dtbs_check warning, yeah. Functionally, it would work
> >>> just fine. Is that reason enough to keep display partially broken for
> >>> 6.11? We could also apply the minor binding change for 6.11 if needed.
> >>
> >> I don't know how to answer this, I'll let the DT maintainer comment this.
> >>
> >> The problem is I do not think we can pass the whole patchset as fixes
> >> for v6.11, patches 2 & 3 could, patches 1 & 4 definitely can't.
> >>
> >> Neil
> >
> > IMO: patch #3 (dts) and #4 (CONFIG) go through the Qualcomm tree
> > whenever those folks agree to it. If we're worried about the
> > dtbs_check breakage I personally wouldn't mind "Ack"ing patch #1 to go
> > through the Qualcomm tree as long as it made it into 6.11-rc1. I have
> > a hunch that there are going to be more Samsung OLED panels in the
> > future that will need to touch the same file, but if the change is in
> > -rc1 it should make it back into drm-misc quickly, right?
>
> Not sure the Soc maintainer would accept any patches for -rc1 at this
> point, but Bjorn can try to push both #3 and #4 as fixes for -rc2.

Yeah, I guess it's pretty late for -rc1.


> Not sure #1 would be accepted as fix via any tree, but having a bindings
> error for a kernel release is not a big deal if in-fine the bindings change
> has been reviewed and queued for next version.

In general my understanding is that we don't worry too much about
temporary bindings errors as long as things meet up in linuxnext and
get fixed. ...but in this case we're talking about the dts going into
6.11 and the bindings in 6.12 which means that v6.11 will be released
and still have the bindings error. That's non-ideal...

Can we really not consider the bindings as "Fix" since it's required
to pass dts validation for the dts patch which is a "Fix"? If we can
consider this bindings change a Fix then we could land it in
drm-misc-fixes and then things could meet up nicely I think, right?

-Doug

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[neil.armstrong]

On 15/07/2024 16:40, Doug Anderson wrote:
> Hi,
> 
> On Mon, Jul 15, 2024 at 6:57 AM <neil.armstrong@linaro.org> wrote:
>>
>> On 15/07/2024 15:51, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Mon, Jul 15, 2024 at 6:02 AM Neil Armstrong
>>> <neil.armstrong@linaro.org> wrote:
>>>>
>>>> On 15/07/2024 14:54, Stephan Gerhold wrote:
>>>>> On Mon, Jul 15, 2024 at 02:42:12PM +0200, Neil Armstrong wrote:
>>>>>> On 15/07/2024 14:15, Stephan Gerhold wrote:
>>>>>>> This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.
>>>>>>>
>>>>>>> The panel should be handled through the samsung-atna33xc20 driver for
>>>>>>> correct power up timings. Otherwise the backlight does not work correctly.
>>>>>>>
>>>>>>> We have existing users of this panel through the generic "edp-panel"
>>>>>>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
>>>>>>> partially in that configuration: It works after boot but once the screen
>>>>>>> gets disabled it does not turn on again until after reboot. It behaves the
>>>>>>> same way with the default "conservative" timings, so we might as well drop
>>>>>>> the configuration from the panel-edp driver. That way, users with old DTBs
>>>>>>> will get a warning and can move to the new driver.
>>>>>>>
>>>>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>>>>>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
>>>>>>> ---
>>>>>>>      drivers/gpu/drm/panel/panel-edp.c | 2 --
>>>>>>>      1 file changed, 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
>>>>>>> index 3a574a9b46e7..d2d682385e89 100644
>>>>>>> --- a/drivers/gpu/drm/panel/panel-edp.c
>>>>>>> +++ b/drivers/gpu/drm/panel/panel-edp.c
>>>>>>> @@ -1960,8 +1960,6 @@ static const struct edp_panel_entry edp_panels[] = {
>>>>>>>       EDP_PANEL_ENTRY('L', 'G', 'D', 0x05af, &delay_200_500_e200_d200, "Unknown"),
>>>>>>>       EDP_PANEL_ENTRY('L', 'G', 'D', 0x05f1, &delay_200_500_e200_d200, "Unknown"),
>>>>>>> -   EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200, "ATNA45AF01"),
>>>>>>> -
>>>>>>>       EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50, "LQ140M1JW48"),
>>>>>>>       EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &delay_80_500_e50, "LQ140M1JW46"),
>>>>>>>       EDP_PANEL_ENTRY('S', 'H', 'P', 0x153a, &delay_200_500_e50, "LQ140T1JH01"),
>>>>>>>
>>>>>>
>>>>>> How will we handle current/old crd DT with new kernels ?
>>>>>>
>>>>>
>>>>> I think this is answered in the commit message:
>>>>>
>>>>>>> We have existing users of this panel through the generic "edp-panel"
>>>>>>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
>>>>>>> partially in that configuration: It works after boot but once the screen
>>>>>>> gets disabled it does not turn on again until after reboot. It behaves the
>>>>>>> same way with the default "conservative" timings, so we might as well drop
>>>>>>> the configuration from the panel-edp driver. That way, users with old DTBs
>>>>>>> will get a warning and can move to the new driver.
>>>>>
>>>>> Basically with the entry removed, the panel-edp driver will fallback to
>>>>> default "conservative" timings when using old DTBs. There will be a
>>>>> warning in dmesg, but otherwise the panel will somewhat work just as
>>>>> before. I think this is a good way to remind users to upgrade.
>>>>
>>>> I consider this as a regression
>>>>
>>>>>
>>>>>> Same question for patch 3, thie serie introduces a bindings that won't be valid
>>>>>> if we backport patch 3. I don't think patch should be backported, and this patch
>>>>>> should be dropped.
>>>>>
>>>>> There would be a dtbs_check warning, yeah. Functionally, it would work
>>>>> just fine. Is that reason enough to keep display partially broken for
>>>>> 6.11? We could also apply the minor binding change for 6.11 if needed.
>>>>
>>>> I don't know how to answer this, I'll let the DT maintainer comment this.
>>>>
>>>> The problem is I do not think we can pass the whole patchset as fixes
>>>> for v6.11, patches 2 & 3 could, patches 1 & 4 definitely can't.
>>>>
>>>> Neil
>>>
>>> IMO: patch #3 (dts) and #4 (CONFIG) go through the Qualcomm tree
>>> whenever those folks agree to it. If we're worried about the
>>> dtbs_check breakage I personally wouldn't mind "Ack"ing patch #1 to go
>>> through the Qualcomm tree as long as it made it into 6.11-rc1. I have
>>> a hunch that there are going to be more Samsung OLED panels in the
>>> future that will need to touch the same file, but if the change is in
>>> -rc1 it should make it back into drm-misc quickly, right?
>>
>> Not sure the Soc maintainer would accept any patches for -rc1 at this
>> point, but Bjorn can try to push both #3 and #4 as fixes for -rc2.
> 
> Yeah, I guess it's pretty late for -rc1.
> 
> 
>> Not sure #1 would be accepted as fix via any tree, but having a bindings
>> error for a kernel release is not a big deal if in-fine the bindings change
>> has been reviewed and queued for next version.
> 
> In general my understanding is that we don't worry too much about
> temporary bindings errors as long as things meet up in linuxnext and
> get fixed. ...but in this case we're talking about the dts going into
> 6.11 and the bindings in 6.12 which means that v6.11 will be released
> and still have the bindings error. That's non-ideal...
> 
> Can we really not consider the bindings as "Fix" since it's required
> to pass dts validation for the dts patch which is a "Fix"? If we can
> consider this bindings change a Fix then we could land it in
> drm-misc-fixes and then things could meet up nicely I think, right?

Probably, we'll need insight from Krzysztof/Conor/Rob on this point,
and perhaps Maxime aswell.

Neil

> 
> -Doug
> 

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[Doug Anderson]

Hi,

On Mon, Jul 15, 2024 at 6:51 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Jul 15, 2024 at 6:02 AM Neil Armstrong
> <neil.armstrong@linaro.org> wrote:
> >
> > On 15/07/2024 14:54, Stephan Gerhold wrote:
> > > On Mon, Jul 15, 2024 at 02:42:12PM +0200, Neil Armstrong wrote:
> > >> On 15/07/2024 14:15, Stephan Gerhold wrote:
> > >>> This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.
> > >>>
> > >>> The panel should be handled through the samsung-atna33xc20 driver for
> > >>> correct power up timings. Otherwise the backlight does not work correctly.
> > >>>
> > >>> We have existing users of this panel through the generic "edp-panel"
> > >>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> > >>> partially in that configuration: It works after boot but once the screen
> > >>> gets disabled it does not turn on again until after reboot. It behaves the
> > >>> same way with the default "conservative" timings, so we might as well drop
> > >>> the configuration from the panel-edp driver. That way, users with old DTBs
> > >>> will get a warning and can move to the new driver.
> > >>>
> > >>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > >>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > >>> ---
> > >>>    drivers/gpu/drm/panel/panel-edp.c | 2 --
> > >>>    1 file changed, 2 deletions(-)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > >>> index 3a574a9b46e7..d2d682385e89 100644
> > >>> --- a/drivers/gpu/drm/panel/panel-edp.c
> > >>> +++ b/drivers/gpu/drm/panel/panel-edp.c
> > >>> @@ -1960,8 +1960,6 @@ static const struct edp_panel_entry edp_panels[] = {
> > >>>     EDP_PANEL_ENTRY('L', 'G', 'D', 0x05af, &delay_200_500_e200_d200, "Unknown"),
> > >>>     EDP_PANEL_ENTRY('L', 'G', 'D', 0x05f1, &delay_200_500_e200_d200, "Unknown"),
> > >>> -   EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200, "ATNA45AF01"),
> > >>> -
> > >>>     EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50, "LQ140M1JW48"),
> > >>>     EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &delay_80_500_e50, "LQ140M1JW46"),
> > >>>     EDP_PANEL_ENTRY('S', 'H', 'P', 0x153a, &delay_200_500_e50, "LQ140T1JH01"),
> > >>>
> > >>
> > >> How will we handle current/old crd DT with new kernels ?
> > >>
> > >
> > > I think this is answered in the commit message:
> > >
> > >>> We have existing users of this panel through the generic "edp-panel"
> > >>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> > >>> partially in that configuration: It works after boot but once the screen
> > >>> gets disabled it does not turn on again until after reboot. It behaves the
> > >>> same way with the default "conservative" timings, so we might as well drop
> > >>> the configuration from the panel-edp driver. That way, users with old DTBs
> > >>> will get a warning and can move to the new driver.
> > >
> > > Basically with the entry removed, the panel-edp driver will fallback to
> > > default "conservative" timings when using old DTBs. There will be a
> > > warning in dmesg, but otherwise the panel will somewhat work just as
> > > before. I think this is a good way to remind users to upgrade.
> >
> > I consider this as a regression
> >
> > >
> > >> Same question for patch 3, thie serie introduces a bindings that won't be valid
> > >> if we backport patch 3. I don't think patch should be backported, and this patch
> > >> should be dropped.
> > >
> > > There would be a dtbs_check warning, yeah. Functionally, it would work
> > > just fine. Is that reason enough to keep display partially broken for
> > > 6.11? We could also apply the minor binding change for 6.11 if needed.
> >
> > I don't know how to answer this, I'll let the DT maintainer comment this.
> >
> > The problem is I do not think we can pass the whole patchset as fixes
> > for v6.11, patches 2 & 3 could, patches 1 & 4 definitely can't.
> >
> > Neil
>
> IMO: patch #3 (dts) and #4 (CONFIG) go through the Qualcomm tree
> whenever those folks agree to it. If we're worried about the
> dtbs_check breakage I personally wouldn't mind "Ack"ing patch #1 to go
> through the Qualcomm tree as long as it made it into 6.11-rc1. I have
> a hunch that there are going to be more Samsung OLED panels in the
> future that will need to touch the same file, but if the change is in
> -rc1 it should make it back into drm-misc quickly, right?
>
> Personally I think patch #2 could go in anytime since, as people have
> said, things are pretty broken today and the worst that happens is
> that someone gets an extra warning. That would be my preference. That
> being said, we could also snooze that patch for a month or two and
> land it later. There's no real hurry.

For now I'm going to snooze this patch for a month just to avoid any
controversy. I'll plan to apply it (to drm-misc-next) when I see the
device tree patch land. Since the device tree patch should land as a
fix that should keep things landing in the correct order. ...and, as
per above, the worst case is that if someone has an old DTS and a new
kernel then a panel that was already not working well will print a fat
warning and startup a bit slower.

If somehow I mess up and forget about this patch, feel free to send me
a poke when the device tree patch is landed.

-Doug

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[Doug Anderson]

Hi,

On Mon, Jul 22, 2024 at 8:49 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Jul 15, 2024 at 6:51 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Mon, Jul 15, 2024 at 6:02 AM Neil Armstrong
> > <neil.armstrong@linaro.org> wrote:
> > >
> > > On 15/07/2024 14:54, Stephan Gerhold wrote:
> > > > On Mon, Jul 15, 2024 at 02:42:12PM +0200, Neil Armstrong wrote:
> > > >> On 15/07/2024 14:15, Stephan Gerhold wrote:
> > > >>> This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.
> > > >>>
> > > >>> The panel should be handled through the samsung-atna33xc20 driver for
> > > >>> correct power up timings. Otherwise the backlight does not work correctly.
> > > >>>
> > > >>> We have existing users of this panel through the generic "edp-panel"
> > > >>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> > > >>> partially in that configuration: It works after boot but once the screen
> > > >>> gets disabled it does not turn on again until after reboot. It behaves the
> > > >>> same way with the default "conservative" timings, so we might as well drop
> > > >>> the configuration from the panel-edp driver. That way, users with old DTBs
> > > >>> will get a warning and can move to the new driver.
> > > >>>
> > > >>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > > >>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > > >>> ---
> > > >>>    drivers/gpu/drm/panel/panel-edp.c | 2 --
> > > >>>    1 file changed, 2 deletions(-)
> > > >>>
> > > >>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > > >>> index 3a574a9b46e7..d2d682385e89 100644
> > > >>> --- a/drivers/gpu/drm/panel/panel-edp.c
> > > >>> +++ b/drivers/gpu/drm/panel/panel-edp.c
> > > >>> @@ -1960,8 +1960,6 @@ static const struct edp_panel_entry edp_panels[] = {
> > > >>>     EDP_PANEL_ENTRY('L', 'G', 'D', 0x05af, &delay_200_500_e200_d200, "Unknown"),
> > > >>>     EDP_PANEL_ENTRY('L', 'G', 'D', 0x05f1, &delay_200_500_e200_d200, "Unknown"),
> > > >>> -   EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200, "ATNA45AF01"),
> > > >>> -
> > > >>>     EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50, "LQ140M1JW48"),
> > > >>>     EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &delay_80_500_e50, "LQ140M1JW46"),
> > > >>>     EDP_PANEL_ENTRY('S', 'H', 'P', 0x153a, &delay_200_500_e50, "LQ140T1JH01"),
> > > >>>
> > > >>
> > > >> How will we handle current/old crd DT with new kernels ?
> > > >>
> > > >
> > > > I think this is answered in the commit message:
> > > >
> > > >>> We have existing users of this panel through the generic "edp-panel"
> > > >>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> > > >>> partially in that configuration: It works after boot but once the screen
> > > >>> gets disabled it does not turn on again until after reboot. It behaves the
> > > >>> same way with the default "conservative" timings, so we might as well drop
> > > >>> the configuration from the panel-edp driver. That way, users with old DTBs
> > > >>> will get a warning and can move to the new driver.
> > > >
> > > > Basically with the entry removed, the panel-edp driver will fallback to
> > > > default "conservative" timings when using old DTBs. There will be a
> > > > warning in dmesg, but otherwise the panel will somewhat work just as
> > > > before. I think this is a good way to remind users to upgrade.
> > >
> > > I consider this as a regression
> > >
> > > >
> > > >> Same question for patch 3, thie serie introduces a bindings that won't be valid
> > > >> if we backport patch 3. I don't think patch should be backported, and this patch
> > > >> should be dropped.
> > > >
> > > > There would be a dtbs_check warning, yeah. Functionally, it would work
> > > > just fine. Is that reason enough to keep display partially broken for
> > > > 6.11? We could also apply the minor binding change for 6.11 if needed.
> > >
> > > I don't know how to answer this, I'll let the DT maintainer comment this.
> > >
> > > The problem is I do not think we can pass the whole patchset as fixes
> > > for v6.11, patches 2 & 3 could, patches 1 & 4 definitely can't.
> > >
> > > Neil
> >
> > IMO: patch #3 (dts) and #4 (CONFIG) go through the Qualcomm tree
> > whenever those folks agree to it. If we're worried about the
> > dtbs_check breakage I personally wouldn't mind "Ack"ing patch #1 to go
> > through the Qualcomm tree as long as it made it into 6.11-rc1. I have
> > a hunch that there are going to be more Samsung OLED panels in the
> > future that will need to touch the same file, but if the change is in
> > -rc1 it should make it back into drm-misc quickly, right?
> >
> > Personally I think patch #2 could go in anytime since, as people have
> > said, things are pretty broken today and the worst that happens is
> > that someone gets an extra warning. That would be my preference. That
> > being said, we could also snooze that patch for a month or two and
> > land it later. There's no real hurry.
>
> For now I'm going to snooze this patch for a month just to avoid any
> controversy. I'll plan to apply it (to drm-misc-next) when I see the
> device tree patch land. Since the device tree patch should land as a
> fix that should keep things landing in the correct order. ...and, as
> per above, the worst case is that if someone has an old DTS and a new
> kernel then a panel that was already not working well will print a fat
> warning and startup a bit slower.
>
> If somehow I mess up and forget about this patch, feel free to send me
> a poke when the device tree patch is landed.

More than a month has passed now. One last warning before I apply this
revert in a few more days.

-Doug

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[neil.armstrong]

On 27/08/2024 17:36, Doug Anderson wrote:
> Hi,
> 
> On Mon, Jul 22, 2024 at 8:49 AM Doug Anderson <dianders@chromium.org> wrote:
>>
>> Hi,
>>
>> On Mon, Jul 15, 2024 at 6:51 AM Doug Anderson <dianders@chromium.org> wrote:
>>>
>>> Hi,
>>>
>>> On Mon, Jul 15, 2024 at 6:02 AM Neil Armstrong
>>> <neil.armstrong@linaro.org> wrote:
>>>>
>>>> On 15/07/2024 14:54, Stephan Gerhold wrote:
>>>>> On Mon, Jul 15, 2024 at 02:42:12PM +0200, Neil Armstrong wrote:
>>>>>> On 15/07/2024 14:15, Stephan Gerhold wrote:
>>>>>>> This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.
>>>>>>>
>>>>>>> The panel should be handled through the samsung-atna33xc20 driver for
>>>>>>> correct power up timings. Otherwise the backlight does not work correctly.
>>>>>>>
>>>>>>> We have existing users of this panel through the generic "edp-panel"
>>>>>>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
>>>>>>> partially in that configuration: It works after boot but once the screen
>>>>>>> gets disabled it does not turn on again until after reboot. It behaves the
>>>>>>> same way with the default "conservative" timings, so we might as well drop
>>>>>>> the configuration from the panel-edp driver. That way, users with old DTBs
>>>>>>> will get a warning and can move to the new driver.
>>>>>>>
>>>>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>>>>>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
>>>>>>> ---
>>>>>>>     drivers/gpu/drm/panel/panel-edp.c | 2 --
>>>>>>>     1 file changed, 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
>>>>>>> index 3a574a9b46e7..d2d682385e89 100644
>>>>>>> --- a/drivers/gpu/drm/panel/panel-edp.c
>>>>>>> +++ b/drivers/gpu/drm/panel/panel-edp.c
>>>>>>> @@ -1960,8 +1960,6 @@ static const struct edp_panel_entry edp_panels[] = {
>>>>>>>      EDP_PANEL_ENTRY('L', 'G', 'D', 0x05af, &delay_200_500_e200_d200, "Unknown"),
>>>>>>>      EDP_PANEL_ENTRY('L', 'G', 'D', 0x05f1, &delay_200_500_e200_d200, "Unknown"),
>>>>>>> -   EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200, "ATNA45AF01"),
>>>>>>> -
>>>>>>>      EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50, "LQ140M1JW48"),
>>>>>>>      EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &delay_80_500_e50, "LQ140M1JW46"),
>>>>>>>      EDP_PANEL_ENTRY('S', 'H', 'P', 0x153a, &delay_200_500_e50, "LQ140T1JH01"),
>>>>>>>
>>>>>>
>>>>>> How will we handle current/old crd DT with new kernels ?
>>>>>>
>>>>>
>>>>> I think this is answered in the commit message:
>>>>>
>>>>>>> We have existing users of this panel through the generic "edp-panel"
>>>>>>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
>>>>>>> partially in that configuration: It works after boot but once the screen
>>>>>>> gets disabled it does not turn on again until after reboot. It behaves the
>>>>>>> same way with the default "conservative" timings, so we might as well drop
>>>>>>> the configuration from the panel-edp driver. That way, users with old DTBs
>>>>>>> will get a warning and can move to the new driver.
>>>>>
>>>>> Basically with the entry removed, the panel-edp driver will fallback to
>>>>> default "conservative" timings when using old DTBs. There will be a
>>>>> warning in dmesg, but otherwise the panel will somewhat work just as
>>>>> before. I think this is a good way to remind users to upgrade.
>>>>
>>>> I consider this as a regression
>>>>
>>>>>
>>>>>> Same question for patch 3, thie serie introduces a bindings that won't be valid
>>>>>> if we backport patch 3. I don't think patch should be backported, and this patch
>>>>>> should be dropped.
>>>>>
>>>>> There would be a dtbs_check warning, yeah. Functionally, it would work
>>>>> just fine. Is that reason enough to keep display partially broken for
>>>>> 6.11? We could also apply the minor binding change for 6.11 if needed.
>>>>
>>>> I don't know how to answer this, I'll let the DT maintainer comment this.
>>>>
>>>> The problem is I do not think we can pass the whole patchset as fixes
>>>> for v6.11, patches 2 & 3 could, patches 1 & 4 definitely can't.
>>>>
>>>> Neil
>>>
>>> IMO: patch #3 (dts) and #4 (CONFIG) go through the Qualcomm tree
>>> whenever those folks agree to it. If we're worried about the
>>> dtbs_check breakage I personally wouldn't mind "Ack"ing patch #1 to go
>>> through the Qualcomm tree as long as it made it into 6.11-rc1. I have
>>> a hunch that there are going to be more Samsung OLED panels in the
>>> future that will need to touch the same file, but if the change is in
>>> -rc1 it should make it back into drm-misc quickly, right?
>>>
>>> Personally I think patch #2 could go in anytime since, as people have
>>> said, things are pretty broken today and the worst that happens is
>>> that someone gets an extra warning. That would be my preference. That
>>> being said, we could also snooze that patch for a month or two and
>>> land it later. There's no real hurry.
>>
>> For now I'm going to snooze this patch for a month just to avoid any
>> controversy. I'll plan to apply it (to drm-misc-next) when I see the
>> device tree patch land. Since the device tree patch should land as a
>> fix that should keep things landing in the correct order. ...and, as
>> per above, the worst case is that if someone has an old DTS and a new
>> kernel then a panel that was already not working well will print a fat
>> warning and startup a bit slower.
>>
>> If somehow I mess up and forget about this patch, feel free to send me
>> a poke when the device tree patch is landed.
> 
> More than a month has passed now. One last warning before I apply this
> revert in a few more days.

It's fine if you apply it now

Neil

> 
> -Doug
> 

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[Doug Anderson]

Hi,

On Tue, Aug 27, 2024 at 9:26 AM <neil.armstrong@linaro.org> wrote:
>
> On 27/08/2024 17:36, Doug Anderson wrote:
> > Hi,
> >
> > On Mon, Jul 22, 2024 at 8:49 AM Doug Anderson <dianders@chromium.org> wrote:
> >>
> >> Hi,
> >>
> >> On Mon, Jul 15, 2024 at 6:51 AM Doug Anderson <dianders@chromium.org> wrote:
> >>>
> >>> Hi,
> >>>
> >>> On Mon, Jul 15, 2024 at 6:02 AM Neil Armstrong
> >>> <neil.armstrong@linaro.org> wrote:
> >>>>
> >>>> On 15/07/2024 14:54, Stephan Gerhold wrote:
> >>>>> On Mon, Jul 15, 2024 at 02:42:12PM +0200, Neil Armstrong wrote:
> >>>>>> On 15/07/2024 14:15, Stephan Gerhold wrote:
> >>>>>>> This reverts commit 8ebb1fc2e69ab8b89a425e402c7bd85e053b7b01.
> >>>>>>>
> >>>>>>> The panel should be handled through the samsung-atna33xc20 driver for
> >>>>>>> correct power up timings. Otherwise the backlight does not work correctly.
> >>>>>>>
> >>>>>>> We have existing users of this panel through the generic "edp-panel"
> >>>>>>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> >>>>>>> partially in that configuration: It works after boot but once the screen
> >>>>>>> gets disabled it does not turn on again until after reboot. It behaves the
> >>>>>>> same way with the default "conservative" timings, so we might as well drop
> >>>>>>> the configuration from the panel-edp driver. That way, users with old DTBs
> >>>>>>> will get a warning and can move to the new driver.
> >>>>>>>
> >>>>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> >>>>>>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> >>>>>>> ---
> >>>>>>>     drivers/gpu/drm/panel/panel-edp.c | 2 --
> >>>>>>>     1 file changed, 2 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> >>>>>>> index 3a574a9b46e7..d2d682385e89 100644
> >>>>>>> --- a/drivers/gpu/drm/panel/panel-edp.c
> >>>>>>> +++ b/drivers/gpu/drm/panel/panel-edp.c
> >>>>>>> @@ -1960,8 +1960,6 @@ static const struct edp_panel_entry edp_panels[] = {
> >>>>>>>      EDP_PANEL_ENTRY('L', 'G', 'D', 0x05af, &delay_200_500_e200_d200, "Unknown"),
> >>>>>>>      EDP_PANEL_ENTRY('L', 'G', 'D', 0x05f1, &delay_200_500_e200_d200, "Unknown"),
> >>>>>>> -   EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200, "ATNA45AF01"),
> >>>>>>> -
> >>>>>>>      EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50, "LQ140M1JW48"),
> >>>>>>>      EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &delay_80_500_e50, "LQ140M1JW46"),
> >>>>>>>      EDP_PANEL_ENTRY('S', 'H', 'P', 0x153a, &delay_200_500_e50, "LQ140T1JH01"),
> >>>>>>>
> >>>>>>
> >>>>>> How will we handle current/old crd DT with new kernels ?
> >>>>>>
> >>>>>
> >>>>> I think this is answered in the commit message:
> >>>>>
> >>>>>>> We have existing users of this panel through the generic "edp-panel"
> >>>>>>> compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> >>>>>>> partially in that configuration: It works after boot but once the screen
> >>>>>>> gets disabled it does not turn on again until after reboot. It behaves the
> >>>>>>> same way with the default "conservative" timings, so we might as well drop
> >>>>>>> the configuration from the panel-edp driver. That way, users with old DTBs
> >>>>>>> will get a warning and can move to the new driver.
> >>>>>
> >>>>> Basically with the entry removed, the panel-edp driver will fallback to
> >>>>> default "conservative" timings when using old DTBs. There will be a
> >>>>> warning in dmesg, but otherwise the panel will somewhat work just as
> >>>>> before. I think this is a good way to remind users to upgrade.
> >>>>
> >>>> I consider this as a regression
> >>>>
> >>>>>
> >>>>>> Same question for patch 3, thie serie introduces a bindings that won't be valid
> >>>>>> if we backport patch 3. I don't think patch should be backported, and this patch
> >>>>>> should be dropped.
> >>>>>
> >>>>> There would be a dtbs_check warning, yeah. Functionally, it would work
> >>>>> just fine. Is that reason enough to keep display partially broken for
> >>>>> 6.11? We could also apply the minor binding change for 6.11 if needed.
> >>>>
> >>>> I don't know how to answer this, I'll let the DT maintainer comment this.
> >>>>
> >>>> The problem is I do not think we can pass the whole patchset as fixes
> >>>> for v6.11, patches 2 & 3 could, patches 1 & 4 definitely can't.
> >>>>
> >>>> Neil
> >>>
> >>> IMO: patch #3 (dts) and #4 (CONFIG) go through the Qualcomm tree
> >>> whenever those folks agree to it. If we're worried about the
> >>> dtbs_check breakage I personally wouldn't mind "Ack"ing patch #1 to go
> >>> through the Qualcomm tree as long as it made it into 6.11-rc1. I have
> >>> a hunch that there are going to be more Samsung OLED panels in the
> >>> future that will need to touch the same file, but if the change is in
> >>> -rc1 it should make it back into drm-misc quickly, right?
> >>>
> >>> Personally I think patch #2 could go in anytime since, as people have
> >>> said, things are pretty broken today and the worst that happens is
> >>> that someone gets an extra warning. That would be my preference. That
> >>> being said, we could also snooze that patch for a month or two and
> >>> land it later. There's no real hurry.
> >>
> >> For now I'm going to snooze this patch for a month just to avoid any
> >> controversy. I'll plan to apply it (to drm-misc-next) when I see the
> >> device tree patch land. Since the device tree patch should land as a
> >> fix that should keep things landing in the correct order. ...and, as
> >> per above, the worst case is that if someone has an old DTS and a new
> >> kernel then a panel that was already not working well will print a fat
> >> warning and startup a bit slower.
> >>
> >> If somehow I mess up and forget about this patch, feel free to send me
> >> a poke when the device tree patch is landed.
> >
> > More than a month has passed now. One last warning before I apply this
> > revert in a few more days.
>
> It's fine if you apply it now

Thanks! Pushed to drm-misc-next:

[2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"
      commit: 01cc7b2e8a59fcae0c4493720561e5b33a195fe7

-Doug

Re: [PATCH v2 2/4] Revert "drm/panel-edp: Add SDC ATNA45AF01"

[Johan Hovold]

On Mon, Jul 15, 2024 at 02:54:59PM +0200, Stephan Gerhold wrote:
> On Mon, Jul 15, 2024 at 02:42:12PM +0200, Neil Armstrong wrote:

> > How will we handle current/old crd DT with new kernels ?
> 
> I think this is answered in the commit message:
> 
> > > We have existing users of this panel through the generic "edp-panel"
> > > compatible (e.g. the Qualcomm X1E80100 CRD), but the screen works only
> > > partially in that configuration: It works after boot but once the screen
> > > gets disabled it does not turn on again until after reboot. It behaves the
> > > same way with the default "conservative" timings, so we might as well drop
> > > the configuration from the panel-edp driver. That way, users with old DTBs
> > > will get a warning and can move to the new driver.
> 
> Basically with the entry removed, the panel-edp driver will fallback to
> default "conservative" timings when using old DTBs. There will be a
> warning in dmesg, but otherwise the panel will somewhat work just as
> before. I think this is a good way to remind users to upgrade.
> 
> > Same question for patch 3, thie serie introduces a bindings that won't be valid
> > if we backport patch 3. I don't think patch should be backported, and this patch
> > should be dropped.
> 
> There would be a dtbs_check warning, yeah. Functionally, it would work
> just fine. Is that reason enough to keep display partially broken for
> 6.11? We could also apply the minor binding change for 6.11 if needed.
> 
> I'm also fine if this just goes into 6.12 though.

No, we should definitely fix this for 6.11. This machine is not very
useable without it. Whether to backport is a separate question, but note
that patch 3 is not even marked for backport currently.

Fixing the backlight at the cost of a dtb checker warning should not be
an issue, but backporting would break existing setups unless people have
the new panel driver enabled and this may be a valid concern. On the
other hand, support for this platform is in a bit of flux already and it
looks like most fixes aren't even tagged for stable (presumably for that
reason).

Johan

[PATCH v2 3/4] arm64: dts: qcom: x1e80100-crd: Fix backlight

[Stephan Gerhold]

The backlight does not work correctly with the current display panel
configuration: It works after boot, but once the display gets disabled it
is not possible to get it back on. It turns out that the ATNA45AF01 panel
needs exactly the same non-standard power sequence as implemented by the
panel-samsung-atna33xc20 driver for sc7180-trogdor-homestar.

Switch the panel in the DT to the new compatible and make two more changes
to make it work correctly:

 1. Add the missing GPIO for the panel EL_ON3 line (EDP_BL_EN on CRD and
    enable-gpios in the DT).
 2. Drop the regulator-always-on for the panel regulator. The panel does
    not seem to power off properly if the regulator stays on.

Fixes: d7e03cce0400 ("arm64: dts: qcom: x1e80100-crd: Enable more support")
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
This can be applied as fix for 6.11 since the driver works as-is with the
fallback compatible. If so, the defconfig patch should ideally also go
there to ensure people actually have the driver enabled.
---
 arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100-crd.dts b/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
index 6152bcd0bc1f..4a4eb7211e69 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
@@ -268,7 +268,6 @@ vreg_edp_3p3: regulator-edp-3p3 {
 		pinctrl-0 = <&edp_reg_en>;
 		pinctrl-names = "default";
 
-		regulator-always-on;
 		regulator-boot-on;
 	};
 
@@ -724,9 +723,13 @@ &mdss_dp3 {
 
 	aux-bus {
 		panel {
-			compatible = "edp-panel";
+			compatible = "samsung,atna45af01", "samsung,atna33xc20";
+			enable-gpios = <&pmc8380_3_gpios 4 GPIO_ACTIVE_HIGH>;
 			power-supply = <&vreg_edp_3p3>;
 
+			pinctrl-0 = <&edp_bl_en>;
+			pinctrl-names = "default";
+
 			port {
 				edp_panel_in: endpoint {
 					remote-endpoint = <&mdss_dp3_out>;
@@ -785,6 +788,16 @@ &pcie6a_phy {
 	status = "okay";
 };
 
+&pmc8380_3_gpios {
+	edp_bl_en: edp-bl-en-state {
+		pins = "gpio4";
+		function = "normal";
+		power-source = <1>; /* 1.8V */
+		input-disable;
+		output-enable;
+	};
+};
+
 &qupv3_0 {
 	status = "okay";
 };

-- 
2.44.1

Re: [PATCH v2 3/4] arm64: dts: qcom: x1e80100-crd: Fix backlight

[Johan Hovold]

On Mon, Jul 15, 2024 at 02:15:39PM +0200, Stephan Gerhold wrote:
> The backlight does not work correctly with the current display panel
> configuration: It works after boot, but once the display gets disabled it
> is not possible to get it back on. It turns out that the ATNA45AF01 panel
> needs exactly the same non-standard power sequence as implemented by the
> panel-samsung-atna33xc20 driver for sc7180-trogdor-homestar.
> 
> Switch the panel in the DT to the new compatible and make two more changes
> to make it work correctly:
> 
>  1. Add the missing GPIO for the panel EL_ON3 line (EDP_BL_EN on CRD and
>     enable-gpios in the DT).
>  2. Drop the regulator-always-on for the panel regulator. The panel does
>     not seem to power off properly if the regulator stays on.
> 
> Fixes: d7e03cce0400 ("arm64: dts: qcom: x1e80100-crd: Enable more support")
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
> This can be applied as fix for 6.11 since the driver works as-is with the
> fallback compatible. If so, the defconfig patch should ideally also go
> there to ensure people actually have the driver enabled.

Verified that I can change the brightness setting and that the backlight
comes on again after being turned off (e.g. at suspend):

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>

[PATCH v2 4/4] arm64: defconfig: Add CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20

[Stephan Gerhold]

This is needed for the display panel to work on the Qualcomm
sc7180-trogdor-homestar and x1e80100-crd.

Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 7d32fca64996..362df9390263 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -887,6 +887,7 @@ CONFIG_DRM_PANEL_KHADAS_TS050=m
 CONFIG_DRM_PANEL_MANTIX_MLAF057WE51=m
 CONFIG_DRM_PANEL_NOVATEK_NT36672E=m
 CONFIG_DRM_PANEL_RAYDIUM_RM67191=m
+CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20=m
 CONFIG_DRM_PANEL_SITRONIX_ST7703=m
 CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA=m
 CONFIG_DRM_PANEL_VISIONOX_VTDR6130=m

-- 
2.44.1

Re: [PATCH v2 4/4] arm64: defconfig: Add CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20

[Johan Hovold]

On Mon, Jul 15, 2024 at 02:15:40PM +0200, Stephan Gerhold wrote:
> This is needed for the display panel to work on the Qualcomm
> sc7180-trogdor-homestar and x1e80100-crd.
> 
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>

Re: [PATCH v2 4/4] arm64: defconfig: Add CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20

[Doug Anderson]

Hi,

On Mon, Jul 15, 2024 at 5:16 AM Stephan Gerhold
<stephan.gerhold@linaro.org> wrote:
>
> This is needed for the display panel to work on the Qualcomm
> sc7180-trogdor-homestar and x1e80100-crd.
>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
>  arch/arm64/configs/defconfig | 1 +
>  1 file changed, 1 insertion(+)

I'd assume that this will go through the Qualcomm tree.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: (subset) [PATCH v2 0/4] drm/panel: atna33xc20: Fix the Samsung ATNA45AF01 panel

[Bjorn Andersson]

On Mon, 15 Jul 2024 14:15:36 +0200, Stephan Gerhold wrote:
> The backlight of the Samsung ATNA45AF01 panel used in the Qualcomm X1E80100
> CRD does not work correctly with the current display panel configuration
> and drivers: It works after boot, but once the display gets disabled it is
> not possible to get it back on. It turns out that the ATNA45AF01 panel
> needs exactly the same non-standard power sequence as implemented for
> ATNA33XC20 in the panel-samsung-atna33xc20 driver.
> 
> [...]

Applied, thanks!

[4/4] arm64: defconfig: Add CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20
      commit: 10f98bb9d98137b544b00abb4f9df45e9be7878d

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>