* [PATCH 1/5] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01
2024-07-10 17:04 [PATCH 0/5] drm/panel: atna33xc20: Fix the Samsung ATNA45AF01 panel Stephan Gerhold
@ 2024-07-10 17:04 ` Stephan Gerhold
2024-07-10 17:35 ` Doug Anderson
2024-07-11 6:25 ` Krzysztof Kozlowski
2024-07-10 17:04 ` [PATCH 2/5] drm/panel: samsung-atna33xc20: Add compatible for ATNA45AF01 Stephan Gerhold
` (3 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: Stephan Gerhold @ 2024-07-10 17:04 UTC (permalink / raw)
To: Neil Armstrong, Bjorn Andersson, Konrad Dybcio
Cc: Jessica Zhang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Douglas Anderson, dri-devel, devicetree, linux-kernel,
linux-arm-msm, Abel Vesa, Johan Hovold
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.
Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
.../devicetree/bindings/display/panel/samsung,atna33xc20.yaml | 6 +++++-
1 file changed, 5 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..d668e8d0d296 100644
--- a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
+++ b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
@@ -14,7 +14,11 @@ allOf:
properties:
compatible:
- const: samsung,atna33xc20
+ enum:
+ # Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
+ - samsung,atna33xc20
+ # Samsung 14.5" WQXGA+ (2880x1800 pixels) eDP AMOLED panel
+ - samsung,atna45af01
enable-gpios: true
port: true
--
2.44.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 1/5] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01
2024-07-10 17:04 ` [PATCH 1/5] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01 Stephan Gerhold
@ 2024-07-10 17:35 ` Doug Anderson
2024-07-10 19:03 ` Stephan Gerhold
2024-07-11 6:27 ` Krzysztof Kozlowski
2024-07-11 6:25 ` Krzysztof Kozlowski
1 sibling, 2 replies; 15+ messages in thread
From: Doug Anderson @ 2024-07-10 17:35 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Neil Armstrong, Bjorn Andersson, Konrad Dybcio, Jessica Zhang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, dri-devel,
devicetree, linux-kernel, linux-arm-msm, Abel Vesa, Johan Hovold
Hi,
On Wed, Jul 10, 2024 at 10:05 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.
>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
> .../devicetree/bindings/display/panel/samsung,atna33xc20.yaml | 6 +++++-
> 1 file changed, 5 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..d668e8d0d296 100644
> --- a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
> @@ -14,7 +14,11 @@ allOf:
>
> properties:
> compatible:
> - const: samsung,atna33xc20
> + enum:
> + # Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
> + - samsung,atna33xc20
> + # Samsung 14.5" WQXGA+ (2880x1800 pixels) eDP AMOLED panel
> + - samsung,atna45af01
Seems OK, but a few thoughts:
1. Is it worth renaming this file? Something like
"samsung,atna-oled-panel.yaml"? I'd be interested in DT maintainer
folks' opinions here.
2. In theory you could make your compatible look like this:
compatible = "samsung,atna45af01", "samsung,atna33xc20"
...which would say "I have a 45af01 but if the OS doesn't have
anything special to do that it would be fine to use the 33xc20
driver". That would allow device trees to work without the kernel
changes and would allow you to land the DT changes in parallel with
the driver changes and things would keep working.
...and, in fact, that would mean you _didn't_ need to add the new
compatible string to the driver, which is nice.
-Doug
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01
2024-07-10 17:35 ` Doug Anderson
@ 2024-07-10 19:03 ` Stephan Gerhold
2024-07-10 19:16 ` Doug Anderson
2024-07-11 6:27 ` Krzysztof Kozlowski
1 sibling, 1 reply; 15+ messages in thread
From: Stephan Gerhold @ 2024-07-10 19:03 UTC (permalink / raw)
To: Doug Anderson
Cc: Neil Armstrong, Bjorn Andersson, Konrad Dybcio, Jessica Zhang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, dri-devel,
devicetree, linux-kernel, linux-arm-msm, Abel Vesa, Johan Hovold
On Wed, Jul 10, 2024 at 10:35:28AM -0700, Doug Anderson wrote:
> On Wed, Jul 10, 2024 at 10:05 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.
> >
> > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > ---
> > .../devicetree/bindings/display/panel/samsung,atna33xc20.yaml | 6 +++++-
> > 1 file changed, 5 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..d668e8d0d296 100644
> > --- a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
> > @@ -14,7 +14,11 @@ allOf:
> >
> > properties:
> > compatible:
> > - const: samsung,atna33xc20
> > + enum:
> > + # Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
> > + - samsung,atna33xc20
> > + # Samsung 14.5" WQXGA+ (2880x1800 pixels) eDP AMOLED panel
> > + - samsung,atna45af01
>
> Seems OK, but a few thoughts:
>
> 1. Is it worth renaming this file? Something like
> "samsung,atna-oled-panel.yaml"? I'd be interested in DT maintainer
> folks' opinions here.
>
I think examples for both approaches exist in the kernel tree, so I am
also interested in the opinion of the DT maintainers here. :-)
> 2. In theory you could make your compatible look like this:
>
> compatible = "samsung,atna45af01", "samsung,atna33xc20"
>
> ...which would say "I have a 45af01 but if the OS doesn't have
> anything special to do that it would be fine to use the 33xc20
> driver". That would allow device trees to work without the kernel
> changes and would allow you to land the DT changes in parallel with
> the driver changes and things would keep working.
>
> ...and, in fact, that would mean you _didn't_ need to add the new
> compatible string to the driver, which is nice.
>
Yeah, I considered this. I mentioned the reason why I decided against
this in patch 2:
> While ATNA45AF01 would also work with "samsung,atna33xc20" as a fallback
> compatible, the original submission of the compatible in commit
> 4bfe6c8f7c23 ("drm/panel-simple: Add Samsung ATNA33XC20") had the timings
> and resolution hardcoded. These would not work for ATNA45AF01.
Basically, it works with the current driver. But if you would run the
kernel at the state of the original submission then it would behave
incorrectly. This is why I considered the resolution and timings to be
part of the "samsung,atna33xc20" "ABI". The new panel would not be
compatible with that.
I don't mind changing it, if there is consensus that we should ignore
this and use the fallback compatible instead.
Thanks,
Stephan
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/5] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01
2024-07-10 19:03 ` Stephan Gerhold
@ 2024-07-10 19:16 ` Doug Anderson
2024-07-12 10:11 ` Stephan Gerhold
0 siblings, 1 reply; 15+ messages in thread
From: Doug Anderson @ 2024-07-10 19:16 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Neil Armstrong, Bjorn Andersson, Konrad Dybcio, Jessica Zhang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, dri-devel,
devicetree, linux-kernel, linux-arm-msm, Abel Vesa, Johan Hovold
Hi,
On Wed, Jul 10, 2024 at 12:03 PM Stephan Gerhold
<stephan.gerhold@linaro.org> wrote:
>
> > 2. In theory you could make your compatible look like this:
> >
> > compatible = "samsung,atna45af01", "samsung,atna33xc20"
> >
> > ...which would say "I have a 45af01 but if the OS doesn't have
> > anything special to do that it would be fine to use the 33xc20
> > driver". That would allow device trees to work without the kernel
> > changes and would allow you to land the DT changes in parallel with
> > the driver changes and things would keep working.
> >
> > ...and, in fact, that would mean you _didn't_ need to add the new
> > compatible string to the driver, which is nice.
> >
>
> Yeah, I considered this. I mentioned the reason why I decided against
> this in patch 2:
>
> > While ATNA45AF01 would also work with "samsung,atna33xc20" as a fallback
> > compatible, the original submission of the compatible in commit
> > 4bfe6c8f7c23 ("drm/panel-simple: Add Samsung ATNA33XC20") had the timings
> > and resolution hardcoded. These would not work for ATNA45AF01.
>
> Basically, it works with the current driver. But if you would run the
> kernel at the state of the original submission then it would behave
> incorrectly. This is why I considered the resolution and timings to be
> part of the "samsung,atna33xc20" "ABI". The new panel would not be
> compatible with that.
Ah, oops. My eyes totally glazed over the description since the patch
was so simple. :-P Sorry about that.
IMO I'd still prefer using the fallback compatible, but happy to hear
other opinions. In the original commit things were pretty broken still
(sorta like how it's broken for you using "edp-panel") and the
resolution hasn't been hardcoded for a long while...
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/5] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01
2024-07-10 19:16 ` Doug Anderson
@ 2024-07-12 10:11 ` Stephan Gerhold
0 siblings, 0 replies; 15+ messages in thread
From: Stephan Gerhold @ 2024-07-12 10:11 UTC (permalink / raw)
To: Doug Anderson
Cc: Neil Armstrong, Bjorn Andersson, Konrad Dybcio, Jessica Zhang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, dri-devel,
devicetree, linux-kernel, linux-arm-msm, Abel Vesa, Johan Hovold
On Wed, Jul 10, 2024 at 12:16:58PM -0700, Doug Anderson wrote:
> On Wed, Jul 10, 2024 at 12:03 PM Stephan Gerhold
> <stephan.gerhold@linaro.org> wrote:
> >
> > > 2. In theory you could make your compatible look like this:
> > >
> > > compatible = "samsung,atna45af01", "samsung,atna33xc20"
> > >
> > > ...which would say "I have a 45af01 but if the OS doesn't have
> > > anything special to do that it would be fine to use the 33xc20
> > > driver". That would allow device trees to work without the kernel
> > > changes and would allow you to land the DT changes in parallel with
> > > the driver changes and things would keep working.
> > >
> > > ...and, in fact, that would mean you _didn't_ need to add the new
> > > compatible string to the driver, which is nice.
> > >
> >
> > Yeah, I considered this. I mentioned the reason why I decided against
> > this in patch 2:
> >
> > > While ATNA45AF01 would also work with "samsung,atna33xc20" as a fallback
> > > compatible, the original submission of the compatible in commit
> > > 4bfe6c8f7c23 ("drm/panel-simple: Add Samsung ATNA33XC20") had the timings
> > > and resolution hardcoded. These would not work for ATNA45AF01.
> >
> > Basically, it works with the current driver. But if you would run the
> > kernel at the state of the original submission then it would behave
> > incorrectly. This is why I considered the resolution and timings to be
> > part of the "samsung,atna33xc20" "ABI". The new panel would not be
> > compatible with that.
>
> Ah, oops. My eyes totally glazed over the description since the patch
> was so simple. :-P Sorry about that.
>
> IMO I'd still prefer using the fallback compatible, but happy to hear
> other opinions. In the original commit things were pretty broken still
> (sorta like how it's broken for you using "edp-panel") and the
> resolution hasn't been hardcoded for a long while...
I briefly discussed this with Krzysztof on IRC yesterday and we
concluded that a fallback compatible is better. If one considers just
the non-discoverable part of the interface for the binding (i.e. the
non-standard power sequence), then the two panels are indeed compatible.
I will send a v2 with the fallback compatible on Monday. I think this
can also simplify backporting the backlight fix as you mentioned, since
then no driver change is required to make it work.
Thanks,
Stephan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01
2024-07-10 17:35 ` Doug Anderson
2024-07-10 19:03 ` Stephan Gerhold
@ 2024-07-11 6:27 ` Krzysztof Kozlowski
1 sibling, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-11 6:27 UTC (permalink / raw)
To: Doug Anderson, Stephan Gerhold
Cc: Neil Armstrong, Bjorn Andersson, Konrad Dybcio, Jessica Zhang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, dri-devel,
devicetree, linux-kernel, linux-arm-msm, Abel Vesa, Johan Hovold
On 10/07/2024 19:35, Doug Anderson wrote:
> Hi,
>
> On Wed, Jul 10, 2024 at 10:05 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.
>>
>> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
>> ---
>> .../devicetree/bindings/display/panel/samsung,atna33xc20.yaml | 6 +++++-
>> 1 file changed, 5 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..d668e8d0d296 100644
>> --- a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
>> +++ b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
>> @@ -14,7 +14,11 @@ allOf:
>>
>> properties:
>> compatible:
>> - const: samsung,atna33xc20
>> + enum:
>> + # Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
>> + - samsung,atna33xc20
>> + # Samsung 14.5" WQXGA+ (2880x1800 pixels) eDP AMOLED panel
>> + - samsung,atna45af01
>
> Seems OK, but a few thoughts:
>
> 1. Is it worth renaming this file? Something like
> "samsung,atna-oled-panel.yaml"? I'd be interested in DT maintainer
> folks' opinions here.
No, rather keep existing name, because it should be based on compatible.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01
2024-07-10 17:04 ` [PATCH 1/5] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01 Stephan Gerhold
2024-07-10 17:35 ` Doug Anderson
@ 2024-07-11 6:25 ` Krzysztof Kozlowski
1 sibling, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-11 6:25 UTC (permalink / raw)
To: Stephan Gerhold, Neil Armstrong, Bjorn Andersson, Konrad Dybcio
Cc: Jessica Zhang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Douglas Anderson, dri-devel, devicetree, linux-kernel,
linux-arm-msm, Abel Vesa, Johan Hovold
On 10/07/2024 19:04, 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.
>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/5] drm/panel: samsung-atna33xc20: Add compatible for ATNA45AF01
2024-07-10 17:04 [PATCH 0/5] drm/panel: atna33xc20: Fix the Samsung ATNA45AF01 panel Stephan Gerhold
2024-07-10 17:04 ` [PATCH 1/5] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01 Stephan Gerhold
@ 2024-07-10 17:04 ` Stephan Gerhold
2024-07-10 17:35 ` Doug Anderson
2024-07-10 17:04 ` [PATCH 3/5] Revert "drm/panel-edp: Add SDC ATNA45AF01" Stephan Gerhold
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Stephan Gerhold @ 2024-07-10 17:04 UTC (permalink / raw)
To: Neil Armstrong, Bjorn Andersson, Konrad Dybcio
Cc: Jessica Zhang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Douglas Anderson, dri-devel, devicetree, linux-kernel,
linux-arm-msm, Abel Vesa, Johan Hovold
The Samsung ATNA45AF01 panel needs exactly the same non-standard power
sequence as the Samsung ATNA33XC20 panel for backlight to work properly.
Add the new "samsung,atna45af01" compatible to the driver to make it handle
these panels as well.
While ATNA45AF01 would also work with "samsung,atna33xc20" as a fallback
compatible, the original submission of the compatible in commit
4bfe6c8f7c23 ("drm/panel-simple: Add Samsung ATNA33XC20") had the timings
and resolution hardcoded. These would not work for ATNA45AF01.
Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
drivers/gpu/drm/panel/panel-samsung-atna33xc20.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
index 9a482a744b8c..fd56fd02df87 100644
--- a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
+++ b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
@@ -333,6 +333,7 @@ static void atana33xc20_remove(struct dp_aux_ep_device *aux_ep)
static const struct of_device_id atana33xc20_dt_match[] = {
{ .compatible = "samsung,atna33xc20", },
+ { .compatible = "samsung,atna45af01", },
{ /* sentinal */ }
};
MODULE_DEVICE_TABLE(of, atana33xc20_dt_match);
--
2.44.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/5] drm/panel: samsung-atna33xc20: Add compatible for ATNA45AF01
2024-07-10 17:04 ` [PATCH 2/5] drm/panel: samsung-atna33xc20: Add compatible for ATNA45AF01 Stephan Gerhold
@ 2024-07-10 17:35 ` Doug Anderson
0 siblings, 0 replies; 15+ messages in thread
From: Doug Anderson @ 2024-07-10 17:35 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Neil Armstrong, Bjorn Andersson, Konrad Dybcio, Jessica Zhang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, dri-devel,
devicetree, linux-kernel, linux-arm-msm, Abel Vesa, Johan Hovold
Hi,
On Wed, Jul 10, 2024 at 10:05 AM Stephan Gerhold
<stephan.gerhold@linaro.org> wrote:
>
> The Samsung ATNA45AF01 panel needs exactly the same non-standard power
> sequence as the Samsung ATNA33XC20 panel for backlight to work properly.
> Add the new "samsung,atna45af01" compatible to the driver to make it handle
> these panels as well.
>
> While ATNA45AF01 would also work with "samsung,atna33xc20" as a fallback
> compatible, the original submission of the compatible in commit
> 4bfe6c8f7c23 ("drm/panel-simple: Add Samsung ATNA33XC20") had the timings
> and resolution hardcoded. These would not work for ATNA45AF01.
>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
> drivers/gpu/drm/panel/panel-samsung-atna33xc20.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
> index 9a482a744b8c..fd56fd02df87 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
> @@ -333,6 +333,7 @@ static void atana33xc20_remove(struct dp_aux_ep_device *aux_ep)
>
> static const struct of_device_id atana33xc20_dt_match[] = {
> { .compatible = "samsung,atna33xc20", },
> + { .compatible = "samsung,atna45af01", },
As per my response to patch #1, you don't need this change at all if
you just add a fallback compatible. Later if there is anything special
we need to do for this panel we can match against it, but right now
there is no need.
-Doug
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/5] Revert "drm/panel-edp: Add SDC ATNA45AF01"
2024-07-10 17:04 [PATCH 0/5] drm/panel: atna33xc20: Fix the Samsung ATNA45AF01 panel Stephan Gerhold
2024-07-10 17:04 ` [PATCH 1/5] dt-bindings: display: panel: samsung,atna33xc20: Document ATNA45AF01 Stephan Gerhold
2024-07-10 17:04 ` [PATCH 2/5] drm/panel: samsung-atna33xc20: Add compatible for ATNA45AF01 Stephan Gerhold
@ 2024-07-10 17:04 ` Stephan Gerhold
2024-07-10 17:35 ` Doug Anderson
2024-07-10 17:05 ` [PATCH 4/5] arm64: dts: qcom: x1e80100-crd: Fix backlight Stephan Gerhold
2024-07-10 17:05 ` [PATCH 5/5] arm64: defconfig: Add CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20 Stephan Gerhold
4 siblings, 1 reply; 15+ messages in thread
From: Stephan Gerhold @ 2024-07-10 17:04 UTC (permalink / raw)
To: Neil Armstrong, Bjorn Andersson, Konrad Dybcio
Cc: Jessica Zhang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Douglas Anderson, dri-devel, devicetree, linux-kernel,
linux-arm-msm, Abel Vesa, Johan Hovold
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.
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 3/5] Revert "drm/panel-edp: Add SDC ATNA45AF01"
2024-07-10 17:04 ` [PATCH 3/5] Revert "drm/panel-edp: Add SDC ATNA45AF01" Stephan Gerhold
@ 2024-07-10 17:35 ` Doug Anderson
0 siblings, 0 replies; 15+ messages in thread
From: Doug Anderson @ 2024-07-10 17:35 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Neil Armstrong, Bjorn Andersson, Konrad Dybcio, Jessica Zhang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, dri-devel,
devicetree, linux-kernel, linux-arm-msm, Abel Vesa, Johan Hovold
Hi,
On Wed, Jul 10, 2024 at 10:05 AM Stephan Gerhold
<stephan.gerhold@linaro.org> 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.
>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
> drivers/gpu/drm/panel/panel-edp.c | 2 --
> 1 file changed, 2 deletions(-)
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/5] arm64: dts: qcom: x1e80100-crd: Fix backlight
2024-07-10 17:04 [PATCH 0/5] drm/panel: atna33xc20: Fix the Samsung ATNA45AF01 panel Stephan Gerhold
` (2 preceding siblings ...)
2024-07-10 17:04 ` [PATCH 3/5] Revert "drm/panel-edp: Add SDC ATNA45AF01" Stephan Gerhold
@ 2024-07-10 17:05 ` Stephan Gerhold
2024-07-10 22:07 ` Konrad Dybcio
2024-07-10 17:05 ` [PATCH 5/5] arm64: defconfig: Add CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20 Stephan Gerhold
4 siblings, 1 reply; 15+ messages in thread
From: Stephan Gerhold @ 2024-07-10 17:05 UTC (permalink / raw)
To: Neil Armstrong, Bjorn Andersson, Konrad Dybcio
Cc: Jessica Zhang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Douglas Anderson, dri-devel, devicetree, linux-kernel,
linux-arm-msm, Abel Vesa, Johan Hovold
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.
Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
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..7d6800dd9b8a 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";
+ 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 4/5] arm64: dts: qcom: x1e80100-crd: Fix backlight
2024-07-10 17:05 ` [PATCH 4/5] arm64: dts: qcom: x1e80100-crd: Fix backlight Stephan Gerhold
@ 2024-07-10 22:07 ` Konrad Dybcio
0 siblings, 0 replies; 15+ messages in thread
From: Konrad Dybcio @ 2024-07-10 22:07 UTC (permalink / raw)
To: Stephan Gerhold, Neil Armstrong, Bjorn Andersson
Cc: Jessica Zhang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Douglas Anderson, dri-devel, devicetree, linux-kernel,
linux-arm-msm, Abel Vesa, Johan Hovold
On 10.07.2024 7:05 PM, 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.
>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
[...]
> + power-source = <1>; /* 1.8V */
Would be nice to get the #defines for this PMIC instead..
> + input-disable;
> + output-enable;
LGTM otherwise
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Konrad
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/5] arm64: defconfig: Add CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20
2024-07-10 17:04 [PATCH 0/5] drm/panel: atna33xc20: Fix the Samsung ATNA45AF01 panel Stephan Gerhold
` (3 preceding siblings ...)
2024-07-10 17:05 ` [PATCH 4/5] arm64: dts: qcom: x1e80100-crd: Fix backlight Stephan Gerhold
@ 2024-07-10 17:05 ` Stephan Gerhold
4 siblings, 0 replies; 15+ messages in thread
From: Stephan Gerhold @ 2024-07-10 17:05 UTC (permalink / raw)
To: Neil Armstrong, Bjorn Andersson, Konrad Dybcio
Cc: Jessica Zhang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Douglas Anderson, dri-devel, devicetree, linux-kernel,
linux-arm-msm, Abel Vesa, Johan Hovold
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 5c9fcf9ad395..9572c337ec29 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -889,6 +889,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
^ permalink raw reply related [flat|nested] 15+ messages in thread