2021-06-19 20:27:47

by Rajeev Nandan

[permalink] [raw]
Subject: [v7 0/5] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20

This series adds the support for the eDP panel that needs the backlight
controlling over the DP AUX channel using DPCD registers of the panel
as per the VESA's standard.

This series also adds support for the Samsung eDP AMOLED panel that
needs DP AUX to control the backlight, and introduces new delays in the
@panel_desc.delay to support this panel.

This patch series depends on the following two series:
- Doug's series [1], exposed the DP AUX channel to the panel-simple.
- Lyude's series [2], introduced new drm helper functions for DPCD
backlight.

This series is the logical successor to the series [3].

Changes in v1:
- Created dpcd backlight helper with very basic functionality, added
backlight registration in the ti-sn65dsi86 bridge driver.

Changes in v2:
- Created a new DisplayPort aux backlight driver and moved the code from
drm_dp_aux_backlight.c (v1) to the new driver.

Changes in v3:
- Fixed module compilation (kernel test bot).

Changes in v4:
- Added basic DPCD backlight support in panel-simple.
- Added support for a new Samsung panel ATNA33XC20 that needs DPCD
backlight controlling and has a requirement of delays between enable
GPIO and regulator.

Changes in v5:
Addressed review suggestions from Douglas:
- Created a new API drm_panel_dp_aux_backlight() in drm_panel.c
- Moved DP AUX backlight functions from panel-simple.c to drm_panel.c
- panel-simple probe() calls drm_panel_dp_aux_backlight() to create
backlight when the backlight phandle is not specified in panel DT
and DP AUX channel is present.
- Added check for drm_edp_backlight_supported() before registering.
- Removed the @uses_dpcd_backlight flag from panel_desc as this
should be auto-detected.
- Updated comments/descriptions.

Changes in v6:
- Rebased
- Updated wanrning messages, fixed word wrapping in comments.
- Fixed ordering of memory allocation

Changes in v7:
- Updated the disable_to_power_off and power_to_enable panel delays
as discovered at <https://crrev.com/c/2966167> (Douglas)

[1] https://lore.kernel.org/dri-devel/[email protected]/
[2] https://lore.kernel.org/dri-devel/[email protected]/
[3] https://lore.kernel.org/dri-devel/[email protected]/

Rajeev Nandan (5):
drm/panel: add basic DP AUX backlight support
drm/panel-simple: Support DP AUX backlight
drm/panel-simple: Support for delays between GPIO & regulator
dt-bindings: display: simple: Add Samsung ATNA33XC20
drm/panel-simple: Add Samsung ATNA33XC20

.../bindings/display/panel/panel-simple.yaml | 2 +
drivers/gpu/drm/drm_panel.c | 108 +++++++++++++++++++++
drivers/gpu/drm/panel/panel-simple.c | 67 +++++++++++++
include/drm/drm_panel.h | 15 ++-
4 files changed, 188 insertions(+), 4 deletions(-)

--
2.7.4


2021-06-19 20:27:56

by Rajeev Nandan

[permalink] [raw]
Subject: [v7 5/5] drm/panel-simple: Add Samsung ATNA33XC20

Add Samsung 13.3" FHD eDP AMOLED panel.

Signed-off-by: Rajeev Nandan <[email protected]>
Reviewed-by: Douglas Anderson <[email protected]>
---

Changes in v4:
- New

Changes in v5:
- Remove "uses_dpcd_backlight" property, not required now. (Douglas)

Changes in v7:
- Update disable_to_power_off and power_to_enable delays. (Douglas)

drivers/gpu/drm/panel/panel-simple.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 86e5a45..4adc44a 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -3562,6 +3562,36 @@ static const struct panel_desc rocktech_rk101ii01d_ct = {
.connector_type = DRM_MODE_CONNECTOR_LVDS,
};

+static const struct drm_display_mode samsung_atna33xc20_mode = {
+ .clock = 138770,
+ .hdisplay = 1920,
+ .hsync_start = 1920 + 48,
+ .hsync_end = 1920 + 48 + 32,
+ .htotal = 1920 + 48 + 32 + 80,
+ .vdisplay = 1080,
+ .vsync_start = 1080 + 8,
+ .vsync_end = 1080 + 8 + 8,
+ .vtotal = 1080 + 8 + 8 + 16,
+ .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+static const struct panel_desc samsung_atna33xc20 = {
+ .modes = &samsung_atna33xc20_mode,
+ .num_modes = 1,
+ .bpc = 10,
+ .size = {
+ .width = 294,
+ .height = 165,
+ },
+ .delay = {
+ .disable_to_power_off = 200,
+ .power_to_enable = 400,
+ .hpd_absent_delay = 200,
+ .unprepare = 500,
+ },
+ .connector_type = DRM_MODE_CONNECTOR_eDP,
+};
+
static const struct drm_display_mode samsung_lsn122dl01_c01_mode = {
.clock = 271560,
.hdisplay = 2560,
@@ -4563,6 +4593,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "rocktech,rk101ii01d-ct",
.data = &rocktech_rk101ii01d_ct,
}, {
+ .compatible = "samsung,atna33xc20",
+ .data = &samsung_atna33xc20,
+ }, {
.compatible = "samsung,lsn122dl01-c01",
.data = &samsung_lsn122dl01_c01,
}, {
--
2.7.4

2021-06-19 20:28:09

by Rajeev Nandan

[permalink] [raw]
Subject: [v7 4/5] dt-bindings: display: simple: Add Samsung ATNA33XC20

Add Samsung 13.3" FHD eDP AMOLED panel.

Signed-off-by: Rajeev Nandan <[email protected]>
Reviewed-by: Douglas Anderson <[email protected]>
Acked-by: Rob Herring <[email protected]>
---

(no changes since v4)

Changes in v4:
- New

Documentation/devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 4a0a5e1..f5acfd6 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -242,6 +242,8 @@ properties:
- rocktech,rk101ii01d-ct
# Rocktech Display Ltd. RK070ER9427 800(RGB)x480 TFT LCD panel
- rocktech,rk070er9427
+ # Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
+ - samsung,atna33xc20
# Samsung 12.2" (2560x1600 pixels) TFT LCD panel
- samsung,lsn122dl01-c01
# Samsung Electronics 10.1" WSVGA TFT LCD panel
--
2.7.4

2021-06-19 20:30:10

by Rajeev Nandan

[permalink] [raw]
Subject: [v7 2/5] drm/panel-simple: Support DP AUX backlight

If there is no backlight specified in the device tree and the panel
has access to the DP AUX channel then create a DP AUX backlight if
supported by the panel.

Signed-off-by: Rajeev Nandan <[email protected]>
Reviewed-by: Douglas Anderson <[email protected]>
---

(no changes since v5)

This patch depends on the previous patch (2/5) of this series.

Changes in v4:
- New

Changes in v5:
- Address review comments and move backlight functions to drm_panel.c (Douglas)
- Create and register DP AUX backlight if there is no backlight specified in the
device tree and panel has the DP AUX channel. (Douglas)
- The new drm_panel_dp_aux_backlight() will do the drm_edp_backlight_supported() check.

drivers/gpu/drm/panel/panel-simple.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index df6fbd1..26555ec 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -800,6 +800,12 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc,
if (err)
goto disable_pm_runtime;

+ if (!panel->base.backlight && panel->aux) {
+ err = drm_panel_dp_aux_backlight(&panel->base, panel->aux);
+ if (err)
+ goto disable_pm_runtime;
+ }
+
drm_panel_add(&panel->base);

return 0;
--
2.7.4

2021-06-21 15:43:19

by Doug Anderson

[permalink] [raw]
Subject: Re: [v7 5/5] drm/panel-simple: Add Samsung ATNA33XC20

Hi,

On Sun, Jun 20, 2021 at 3:01 AM Sam Ravnborg <[email protected]> wrote:
>
> Hi Rajeev
> On Sat, Jun 19, 2021 at 04:10:30PM +0530, Rajeev Nandan wrote:
> > Add Samsung 13.3" FHD eDP AMOLED panel.
> >
> > Signed-off-by: Rajeev Nandan <[email protected]>
> > Reviewed-by: Douglas Anderson <[email protected]>
> > ---
> >
> > Changes in v4:
> > - New
> >
> > Changes in v5:
> > - Remove "uses_dpcd_backlight" property, not required now. (Douglas)
> >
> > Changes in v7:
> > - Update disable_to_power_off and power_to_enable delays. (Douglas)
> >
> > drivers/gpu/drm/panel/panel-simple.c | 33 +++++++++++++++++++++++++++++++++
> > 1 file changed, 33 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > index 86e5a45..4adc44a 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -3562,6 +3562,36 @@ static const struct panel_desc rocktech_rk101ii01d_ct = {
> > .connector_type = DRM_MODE_CONNECTOR_LVDS,
> > };
> >
> > +static const struct drm_display_mode samsung_atna33xc20_mode = {
> > + .clock = 138770,
> > + .hdisplay = 1920,
> > + .hsync_start = 1920 + 48,
> > + .hsync_end = 1920 + 48 + 32,
> > + .htotal = 1920 + 48 + 32 + 80,
> > + .vdisplay = 1080,
> > + .vsync_start = 1080 + 8,
> > + .vsync_end = 1080 + 8 + 8,
> > + .vtotal = 1080 + 8 + 8 + 16,
> > + .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC,
> > +};
> > +
> > +static const struct panel_desc samsung_atna33xc20 = {
> > + .modes = &samsung_atna33xc20_mode,
> > + .num_modes = 1,
> > + .bpc = 10,
> > + .size = {
> > + .width = 294,
> > + .height = 165,
> > + },
> > + .delay = {
> > + .disable_to_power_off = 200,
> > + .power_to_enable = 400,
> > + .hpd_absent_delay = 200,
> > + .unprepare = 500,
> > + },
> > + .connector_type = DRM_MODE_CONNECTOR_eDP,
> > +};
>
> bus_format is missing. There should be a warning about this when you
> probe the display.

Sam: I'm curious about the requirement of hardcoding bus_format like
this for eDP panels. Most eDP panels support a variety of bits per
pixel and do so dynamically. Ones I've poked at freely support 6bpp
and 8bpp. Presumably this one supports both of those modes and also
10bpp. I haven't done detailed research on it, but it would also
surprise me if the "bus format" for a given bpp needed to be specified
for eDP. Presumably since eDP has most of the "autodetect" type
features of DP then if the format needed to be accounted for that you
could query the hardware?

Looking at the datasheet for the ti-sn65dsi86 MIPI-to-eDP bridge chip
I see that it explicitly calls out the bus formats that it supports
for the MIPI side but doesn't call out anything for eDP. That would
tend to support my belief that there isn't variance on the eDP side...

Maybe the right fix is to actually change the check not to give a
warning for eDP panels? ...or am I misunderstanding?


> The bpc of 10 in unusual, the current code warns if bpc is neither 6 nor
> 8. If 10 is correct then update the code to accept bpc=10.

I'm pretty sure it's 10 based on this panel's datasheet, though this
panel also accepts 8 bpc. Fixing the warning seems like a good idea to
me--I wasn't aware of it.

-Doug

2021-06-22 18:38:01

by Doug Anderson

[permalink] [raw]
Subject: Re: [v7 5/5] drm/panel-simple: Add Samsung ATNA33XC20

Hi,

On Mon, Jun 21, 2021 at 11:42 AM Sam Ravnborg <[email protected]> wrote:
>
> Hi Doug,
>
> On Mon, Jun 21, 2021 at 08:34:51AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Sun, Jun 20, 2021 at 3:01 AM Sam Ravnborg <[email protected]> wrote:
> > >
> > > Hi Rajeev
> > > On Sat, Jun 19, 2021 at 04:10:30PM +0530, Rajeev Nandan wrote:
> > > > Add Samsung 13.3" FHD eDP AMOLED panel.
> > > >
> > > > Signed-off-by: Rajeev Nandan <[email protected]>
> > > > Reviewed-by: Douglas Anderson <[email protected]>
> > > > ---
> > > >
> > > > Changes in v4:
> > > > - New
> > > >
> > > > Changes in v5:
> > > > - Remove "uses_dpcd_backlight" property, not required now. (Douglas)
> > > >
> > > > Changes in v7:
> > > > - Update disable_to_power_off and power_to_enable delays. (Douglas)
> > > >
> > > > drivers/gpu/drm/panel/panel-simple.c | 33 +++++++++++++++++++++++++++++++++
> > > > 1 file changed, 33 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > > > index 86e5a45..4adc44a 100644
> > > > --- a/drivers/gpu/drm/panel/panel-simple.c
> > > > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > > > @@ -3562,6 +3562,36 @@ static const struct panel_desc rocktech_rk101ii01d_ct = {
> > > > .connector_type = DRM_MODE_CONNECTOR_LVDS,
> > > > };
> > > >
> > > > +static const struct drm_display_mode samsung_atna33xc20_mode = {
> > > > + .clock = 138770,
> > > > + .hdisplay = 1920,
> > > > + .hsync_start = 1920 + 48,
> > > > + .hsync_end = 1920 + 48 + 32,
> > > > + .htotal = 1920 + 48 + 32 + 80,
> > > > + .vdisplay = 1080,
> > > > + .vsync_start = 1080 + 8,
> > > > + .vsync_end = 1080 + 8 + 8,
> > > > + .vtotal = 1080 + 8 + 8 + 16,
> > > > + .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC,
> > > > +};
> > > > +
> > > > +static const struct panel_desc samsung_atna33xc20 = {
> > > > + .modes = &samsung_atna33xc20_mode,
> > > > + .num_modes = 1,
> > > > + .bpc = 10,
> > > > + .size = {
> > > > + .width = 294,
> > > > + .height = 165,
> > > > + },
> > > > + .delay = {
> > > > + .disable_to_power_off = 200,
> > > > + .power_to_enable = 400,
> > > > + .hpd_absent_delay = 200,
> > > > + .unprepare = 500,
> > > > + },
> > > > + .connector_type = DRM_MODE_CONNECTOR_eDP,
> > > > +};
> > >
> > > bus_format is missing. There should be a warning about this when you
> > > probe the display.
> >
> > Sam: I'm curious about the requirement of hardcoding bus_format like
> > this for eDP panels. Most eDP panels support a variety of bits per
> > pixel and do so dynamically. Ones I've poked at freely support 6bpp
> > and 8bpp. Presumably this one supports both of those modes and also
> > 10bpp. I haven't done detailed research on it, but it would also
> > surprise me if the "bus format" for a given bpp needed to be specified
> > for eDP. Presumably since eDP has most of the "autodetect" type
> > features of DP then if the format needed to be accounted for that you
> > could query the hardware?
> >
> > Looking at the datasheet for the ti-sn65dsi86 MIPI-to-eDP bridge chip
> > I see that it explicitly calls out the bus formats that it supports
> > for the MIPI side but doesn't call out anything for eDP. That would
> > tend to support my belief that there isn't variance on the eDP side...
> >
> > Maybe the right fix is to actually change the check not to give a
> > warning for eDP panels? ...or am I misunderstanding?
>
> I have never dived into the datasheets of eDP panels so I do not know.
> The checks were added based on what we had in-tree and it is no suprise
> if they need an update or are just plain wrong.
> I expect you to be in a better position to make the call here - but we
> should not add panels that triggers warnings so either fix the warnings
> or fix the panel description.

Agreed. I'd support a patch that removes this warning for eDP panels
unless someone knows that it makes sense. I haven't been able to find
anything indicating that it does.

-Doug