On 30-04-2021 02:33, Doug Anderson wrote:
Hi,
On Thu, Apr 29, 2021 at 11:04 AM Rob Herring <robh@xxxxxxxxxx> wrote:
On Mon, Apr 26, 2021 at 11:29:15AM +0530, Rajeev Nandan wrote:
> Add bindings for DisplayPort aux backlight driver.
>
> Changes in v2:
> - New
>
> Signed-off-by: Rajeev Nandan <rajeevny@xxxxxxxxxxxxxx>
> ---
> .../bindings/leds/backlight/dp-aux-backlight.yaml | 49 ++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/backlight/dp-aux-backlight.yaml
>
> diff --git a/Documentation/devicetree/bindings/leds/backlight/dp-aux-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/dp-aux-backlight.yaml
> new file mode 100644
> index 00000000..0fa8bf0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/dp-aux-backlight.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/dp-aux-backlight.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: DisplayPort aux backlight driver bindings
> +
> +maintainers:
> + - Rajeev Nandan <rajeevny@xxxxxxxxxxxxxx>
> +
> +description:
> + Backlight driver to control the brightness over DisplayPort aux channel.
> +
> +allOf:
> + - $ref: common.yaml#
> +
> +properties:
> + compatible:
> + const: dp-aux-backlight
> +
> + ddc-i2c-bus:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + A phandle to the system I2C controller connected to the DDC bus used
> + for the DisplayPort AUX channel.
> +
> + enable-gpios:
> + maxItems: 1
> + description: GPIO specifier for backlight enable pin.
> +
> + max-brightness: true
> +
> +required:
> + - compatible
> + - ddc-i2c-bus
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + backlight {
> + compatible = "dp-aux-backlight";
> + ddc-i2c-bus = <&sn65dsi86_bridge>;
> + enable-gpios = <&tlmm 12 GPIO_ACTIVE_HIGH>;
So the DDC bus is connected to a backlight and also a panel? This
binding is not reflecting the h/w, but rather what you want for some
driver.
There's only one thing here and that's an eDP panel which supports
backlight control via DP aux channel. You can figure all that out from
the panel's compatible and/or reading the EDID.
You might also be interested in this thread:
https://lore.kernel.org/lkml/YIKsDtjcIHGNvW0u@xxxxxxxxxxxxxxx/
I think Rajeev needs to rework everything anyway as per:
https://lore.kernel.org/r/87zgxl5qar.fsf@xxxxxxxxx
...but you're right that it makes sense not to model the backlight as
a separate node in the device tree. The panel driver can handle
setting up the backlight.
-Doug
It was not a good idea to create a separate backlight driver and use
ddc-i2c-bus to get access to DP aux. I am working to move the code
to the panel driver and to utilize the new DRM helper functions
(drm_edp_backlight_*) Lyude has added [1].
To use these helper functions, the panel driver should have access to
the
"struct drm_dp_aux *". The simple-panel has a "ddc-i2c-bus" property
to give the panel access to the DDC bus and is currently being used to
get the EDID from the panel. Can I use the same ddc bus i2c_adapter to
get
the "struct drm_dp_aux *"?
As per the suggestion [2], I get the "struct drm_dp_aux *" from the
i2c_adapter of ddc bus (maybe I didn't understand the suggestion
correctly),
and, it turned out, the way I have implemented is not the right way [3].
So, I am afraid to use the same method in the panel driver.
[1] https://lore.kernel.org/dri-devel/871rb5bcf9.fsf@xxxxxxxxx/
[2] https://www.spinics.net/lists/dri-devel/msg295429.html
[3]
https://lore.kernel.org/dri-devel/20210426111116.4lc3ekxjugjr3oho@xxxxxxxxx/
Thanks,
Rajeev
