Re: [PATCH v2 1/5] dt-bindings: connector: add GE SUNH hotplug addon connector
finding out the model ID would become very annoying. However the EEPROM is on the add-on, so describing it in the main DT would be wrong. Not only conceptually, as hardware not present should not be in the live DT, but also practically, as when the add-on is removed and then a possibly different add-on is connected we need the EEPROM driver to probe again, in order to do any initialization that might be needed in the EEPROM driver probe function. So I see no option but adding the EEPROM in an overlay. But it cannot be the "full" overlay because before accessing the EEPROM we don't know which model is loaded. Do you have in mind a better approach that I just didn't think about? Best regards, Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2 0/5] Add support for GE SUNH hot-pluggable connector (was: "drm: add support for hot-pluggable bridges")
Hello Sima, thanks again for your comments. Not yet enough for me to clearly focus what you are suggesting, but getting closer. See my questions below. On Tue, 21 May 2024 14:01:19 +0200 Daniel Vetter wrote: > On Mon, May 20, 2024 at 02:01:48PM +0200, Luca Ceresoli wrote: > > Hello Daniel, > > > > On Thu, 16 May 2024 15:22:01 +0200 > > Daniel Vetter wrote: > > > > > Apologies for missing v1 ... > > > > > > On Fri, May 10, 2024 at 09:10:36AM +0200, Luca Ceresoli wrote: > > > > DRM hotplug bridge driver > > > > = > > > > > > > > DRM natively supports pipelines whose display can be removed, but all > > > > the > > > > components preceding it (all the display controller and any bridges) are > > > > assumed to be fixed and cannot be plugged, removed or modified at > > > > runtime. > > > > > > > > This series adds support for DRM pipelines having a removable part after > > > > the encoder, thus also allowing bridges to be removed and reconnected at > > > > runtime, possibly with different components. > > > > > > > > This picture summarizes the DRM structure implemented by this series: > > > > > > > > .. > > > > | DISPLAY CONTROLLER | > > > > | .-. .--. | > > > > | | ENCODER |<--| CRTC | | > > > > | '-' '--' | > > > > '--|-' > > > > | > > > > |DSIHOTPLUG > > > > V CONNECTOR > > > >.-..--..-..-. .---. > > > >| 0 to N || _| _| || 1 to N | | | > > > >| BRIDGES |--DSI-->||_ |_ |--DSI-->| BRIDGES |--LVDS-->| PANEL | > > > >| || || || | | | > > > >'-''--''-''-' '---' > > > > > > > > [--- fixed components --] [--- removable add-on ---] > > > > > > > > Fixed components include: > > > > > > > > * all components up to the DRM encoder, usually part of the SoC > > > > * optionally some bridges, in the SoC and/or as external chips > > > > > > > > Components on the removable add-on include: > > > > > > > > * one or more bridges > > > > * a fixed connector (not one natively supporting hotplug such as HDMI) > > > > * the panel > > > > > > So I think at a high level this design approach makes sense, > > > > Good starting point :) > > > > > but the > > > implementation needs some serious thought. One big thing upfront though, > > > we need to have a clear plan for the overlay hotunload issues, otherwise > > > trying to make drm bridges hotpluggable makes no sense to me. Hotunload is > > > very, very tricky, full of lifetime issues, and those need to be sorted > > > out first or we're just trying to build a castle on quicksand. > > > > > > For bridges itself I don't think the current locking works. You're trying > > > to really cleverly hide it all behind a normal-looking bridge driver, but > > > there's many things beyond that which will blow up if bridges just > > > disappear. Most importantly the bridge states part of an atomic update. > > > > Surely possible as atomic updates are definitely not stimulated in my > > use case. Can you recommend any testing tools to be able to trigger any > > issues? > > Uh really hard ... You'd need to create an atomic commit that's blocked on > a sync_file in-fence (so that you can extend the race window). And then > hotunplug the bridge chain _before_ you signal that fence. > > That's not going to cover all possible races, but at least a large chunk > of the really big ones. > > > The main setups I used for my testing so far are 'modetest -s' for my > > daily work and a simple weston setup to periodically test a complete > > user space stack. > > > > > Now in drm we have drm_connector as the only hotunpluggable thing, and it > > > took years to sort out all the issues. I think we should either model the > > > bridge hotunplug locking after that, or just outright reuse the connector > > > locking and lifetime rules. I much prefer the latter personally. > > >
Re: [PATCH v2 0/5] Add support for GE SUNH hot-pluggable connector (was: "drm: add support for hot-pluggable bridges")
Hello Daniel, On Thu, 16 May 2024 15:22:01 +0200 Daniel Vetter wrote: > Apologies for missing v1 ... > > On Fri, May 10, 2024 at 09:10:36AM +0200, Luca Ceresoli wrote: > > DRM hotplug bridge driver > > = > > > > DRM natively supports pipelines whose display can be removed, but all the > > components preceding it (all the display controller and any bridges) are > > assumed to be fixed and cannot be plugged, removed or modified at runtime. > > > > This series adds support for DRM pipelines having a removable part after > > the encoder, thus also allowing bridges to be removed and reconnected at > > runtime, possibly with different components. > > > > This picture summarizes the DRM structure implemented by this series: > > > > .. > > | DISPLAY CONTROLLER | > > | .-. .--. | > > | | ENCODER |<--| CRTC | | > > | '-' '--' | > > '--|-' > > | > > |DSIHOTPLUG > > V CONNECTOR > >.-..--..-..-. .---. > >| 0 to N || _| _| || 1 to N | | | > >| BRIDGES |--DSI-->||_ |_ |--DSI-->| BRIDGES |--LVDS-->| PANEL | > >| || || || | | | > >'-''--''-''-' '---' > > > > [--- fixed components --] [--- removable add-on ---] > > > > Fixed components include: > > > > * all components up to the DRM encoder, usually part of the SoC > > * optionally some bridges, in the SoC and/or as external chips > > > > Components on the removable add-on include: > > > > * one or more bridges > > * a fixed connector (not one natively supporting hotplug such as HDMI) > > * the panel > > So I think at a high level this design approach makes sense, Good starting point :) > but the > implementation needs some serious thought. One big thing upfront though, > we need to have a clear plan for the overlay hotunload issues, otherwise > trying to make drm bridges hotpluggable makes no sense to me. Hotunload is > very, very tricky, full of lifetime issues, and those need to be sorted > out first or we're just trying to build a castle on quicksand. > > For bridges itself I don't think the current locking works. You're trying > to really cleverly hide it all behind a normal-looking bridge driver, but > there's many things beyond that which will blow up if bridges just > disappear. Most importantly the bridge states part of an atomic update. Surely possible as atomic updates are definitely not stimulated in my use case. Can you recommend any testing tools to be able to trigger any issues? The main setups I used for my testing so far are 'modetest -s' for my daily work and a simple weston setup to periodically test a complete user space stack. > Now in drm we have drm_connector as the only hotunpluggable thing, and it > took years to sort out all the issues. I think we should either model the > bridge hotunplug locking after that, or just outright reuse the connector > locking and lifetime rules. I much prefer the latter personally. > > Anyway the big issues: > > - We need to refcount the hotpluggable bridges, because software (like > atomic state updates) might hang onto pointers for longer than the > bridge physically exists. Assuming that you can all tear it down > synchronously will not work. > > If we reuse connector locking/lifetime then we could put the > hotpluggable part of the bridge chain into the drm_connector, since that > already has refcounting as needed. It would mean that finding the next > bridge in the chain becomes a lot more tricky though. With that model > we'd create a new connector every time the bridge is hotplugged, which I > think is also the cleaner model (because you might plug in a hdmi > connector after a panel, so things like the connector type change). I have been investigating the option of adding/removing a connector based on hot-plug/unplug events initially, see my reply to Maxime after v1 [0]: > The first approach is based on removing the drm_connector. My laptop > uses the i915 driver, and I have observed that attaching/removing a > USB-C dock with an HDMI connector connected to a monitor, a new > drm_connector appears/disappears for the card. User space gets notified > and the external monitor is enabled/disabled, just the way a desktop > user would expect, so this is possible. I had a look at the driver but > how this magic happens w
Re: [PATCH v2 0/5] Add support for GE SUNH hot-pluggable connector (was: "drm: add support for hot-pluggable bridges")
Hello Rob, On Fri, 10 May 2024 11:44:49 -0500 Rob Herring wrote: > On Fri, May 10, 2024 at 09:10:36AM +0200, Luca Ceresoli wrote: [...] > > Overall approach > > > > > > Device tree overlays appear as the most natural solution to support the > > addition and removal of devices from a running system. > > > > Several features are missing from the mainline Linux kernel in order to > > support this use case: > > > > 1. runtime (un)loading of device tree overlays is not supported > > Not true. Device specific applying of overlays has been supported > since we merged DT overlay support. What's not supported is a general > purpose interface to userspace to change any part of the DT at any point > in time. I think I should replace "supported" with "exposed [to user space]". In other words, there is no user of of_overlay_fdt_apply() / of_overlay_remove*() in mainline Linux, except in unittest. Would it be a correct rewording? > > 2. if enabled, overlay (un)loading exposes several bugs > > Hence why there is no general purpose interface. Absolutely. Best regards, Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2 1/5] dt-bindings: connector: add GE SUNH hotplug addon connector
Hello Rob,
+cc Srinivas and Miquèl for the NVMEM cell discussion below
On Fri, 10 May 2024 11:36:25 -0500
Rob Herring wrote:
> On Fri, May 10, 2024 at 09:10:37AM +0200, Luca Ceresoli wrote:
> > Add bindings for the GE SUNH add-on connector. This is a physical,
> > hot-pluggable connector that allows to attach and detach at runtime an
> > add-on adding peripherals on non-discoverable busses.
> >
> > Signed-off-by: Luca Ceresoli
[...]
> > +++
> > b/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml
> > @@ -0,0 +1,197 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/connector/ge,sunh-addon-connector.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: GE SUNH hotplug add-on connector
> > +
> > +maintainers:
> > + - Luca Ceresoli
> > +
> > +description:
> > + Represent the physical connector present on GE SUNH devices that allows
> > + to attach and detach at runtime an add-on adding peripherals on
> > + non-discoverable busses.
> > +
> > + This connector has status GPIOs to notify the connection status to the
> > + CPU and a reset GPIO to allow the CPU to reset all the peripherals on the
> > + add-on. It also has a 4-lane MIPI DSI bus.
> > +
> > + Add-on removal can happen at any moment under user control and without
> > + prior notice to the CPU, making all of its components not usable
> > + anymore. Later on, the same or a different add-on model can be
> > connected.
>
> Is there any documentation for this connector?
>
> Is the connector supposed to be generic in that any board with any SoC
> could have it? If so, the connector needs to be able to remap things so
> overlays aren't tied to the base dts, but only the connector. If not,
> then doing that isn't required, but still a good idea IMO.
It is not generic. The connector pinout is very specific to this
product, and there is no public documentation.
> > +examples:
> > + # Main DTS describing the "main" board up to the connector
> > + - |
> > +/ {
> > +#include
> > +
> > +addon_connector: addon-connector {
>
> Just 'connector' for the node name.
OK
> > +compatible = "ge,sunh-addon-connector";
> > +reset-gpios = < 1 GPIO_ACTIVE_LOW>;
> > +plugged-gpios = < 2 GPIO_ACTIVE_LOW>;
> > +powergood-gpios = < 3 GPIO_ACTIVE_HIGH>;
> > +
> > +ports {
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +
> > +port@0 {
> > +reg = <0>;
> > +
> > +hotplug_conn_dsi_in: endpoint {
> > +remote-endpoint = <_bridge_out>;
> > +};
> > +};
> > +
> > +port@1 {
> > +reg = <1>;
> > +
> > +hotplug_conn_dsi_out: endpoint {
> > +// remote-endpoint to be added by overlay
> > +};
> > +};
> > +};
> > +};
> > +};
> > +
> > + # "base" overlay describing the common components on every add-on that
> > + # are required to read the model ID
>
> This is located on the add-on board, right?
Exactly. Each add-on has an EEPROM with the add-on model ID stored
along with other data.
> Is it really any better to have this as a separate overlay rather than
> just making it an include? Better to have just 1 overlay per board
> applied atomically than splitting it up.
(see below)
> > + - |
> > + {
>
> Generally, I think everything on an add-on board should be underneath
> the connector node. For starters, that makes controlling probing and
> removal of devices easier. For example, you'll want to handle
> reset-gpios and powergood-gpios before any devices 'appear'. Otherwise,
> you add devices on i2c1, start probing them, and then reset them at some
> async time?
This is not a problem because the code is asserting reset before
loading the first overlay. From patch 5/5:
static int sunh_conn_attach(struct sunh_conn *conn)
{
int err;
/* Reset the plugged board in order to start from a stable state */
sunh_conn_reset(conn, false);
err = sunh_conn_load_base_overlay(conn);
...
}
> For i2c, it could look something like this:
>
Re: [PATCH v2 5/5] misc: add ge-addon-connector driver
Hi Greg, Arnd, On Fri, 10 May 2024 12:57:24 +0200 "Arnd Bergmann" wrote: > On Fri, May 10, 2024, at 12:54, Luca Ceresoli wrote: > > On Fri, 10 May 2024 12:24:06 +0200 "Arnd Bergmann" wrote: > >> On Fri, May 10, 2024, at 09:55, Greg Kroah-Hartman wrote: > >> > On Fri, May 10, 2024 at 09:10:41AM +0200, Luca Ceresoli wrote: > >> >> > >> >> +config GE_SUNH_CONNECTOR > >> >> + tristate "GE SUNH hotplug add-on connector" > >> >> + depends on OF > >> >> + select OF_OVERLAY > >> >> + select FW_LOADER > >> >> + select NVMEM > >> >> + select DRM_HOTPLUG_BRIDGE > >> > > >> > Can these be depends instead of select? 'select' causes dependencies > >> > that are hard, if not almost impossible, to detect at times why > >> > something is being enabled. > >> > >> I think FW_LOADER needs to be 'select' since it is normally > >> a hidden symbol and gets selected by its users, all the other > >> ones should be 'depends on'. > > > > I see, makes sense. > > > > And as you pointed that out, I realize perhaps DRM_HOTPLUG_BRIDGE could > > become a hidden symbol as it's not expected to be used alone. > > It's slightly easier to keep it as a visible symbol > with 'depends on' though, since otherwise you have to > add 'depends on' statments for anything that DRM_HOTPLUG_BRIDGE > in turn depends on, most notably DRM itself. I see, sure. Thanks both, changes applied locally. Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2 1/5] dt-bindings: connector: add GE SUNH hotplug addon connector
Hello Rob, On Fri, 10 May 2024 08:22:53 -0500 Rob Herring wrote: > On Fri, May 10, 2024 at 5:37 AM Luca Ceresoli > wrote: > > > > Hello Rob, > > > > On Fri, 10 May 2024 03:41:35 -0500 > > "Rob Herring (Arm)" wrote: > > > > > On Fri, 10 May 2024 09:10:37 +0200, Luca Ceresoli wrote: > > > > Add bindings for the GE SUNH add-on connector. This is a physical, > > > > hot-pluggable connector that allows to attach and detach at runtime an > > > > add-on adding peripherals on non-discoverable busses. > > > > > > > > Signed-off-by: Luca Ceresoli > > > > > > > > --- > > > > > > > > NOTE: the second and third examples fail 'make dt_binding_check' because > > > > they are example of DT overlay code -- I'm not aware of a way to > > > > validate overlay examples as of now > > > > As mentioned here... > > > > > > > > > > This patch is new in v2. > > > > --- > > > > .../connector/ge,sunh-addon-connector.yaml | 197 > > > > + > > > > MAINTAINERS| 5 + > > > > 2 files changed, 202 insertions(+) > > > > > > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > > > > > yamllint warnings/errors: > > > > > > dtschema/dtc warnings/errors: > > > Error: > > > Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.example.dts:49.9-14 > > > syntax error > > > FATAL ERROR: Unable to parse input tree > > > > ...this is expected. > > > > Any hints on how this can be managed in bindings examples would be very > > useful. > > Overlays in examples are not supported. Add actual .dtso files if you > want examples of overlays (maybe you did, shrug). > > Overlays are somewhat orthogonal to bindings. Bindings define the ABI. > It only makes sense to validate applied overlays. Now maybe overlays > contain complete nodes and we could validate those, but that's a > problem for actual overlay files and not something we need to > complicate examples with. Many thanks for the insights. The reason I added overlays in the bindings examples is that this specific device calls for overlays by its very nature. And in fact the implementation is based on overlays. However I understand the reasons for not having overlays in examples. I think I can just remove the two examples and mention the nvmem-cells and nvmem-cell-names nodes as regular properties, and explain in their descriptions that these are supposed to be loaded via overlays. Quick draft: properties: nvmem-cell-names: items: - const: speed-bin nvmem-cells: maxItems: 1 description: NVMEM cell containing the add-on model ID for the add-on that is inserted. Multiple add-on models can be connected, and in order to find out the exact model connected all of them have an EEPROM at the same I2C bus and address with an ID at the same offset. By its nature, this and the nvmem-cell-names nodes are supposed to be added by an overlay once ad add-on is detected. So they must not be present in the initial device tree, and they must be added by an overlay before the add-on can be used. Looks reasonable? Best regards, Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2 5/5] misc: add ge-addon-connector driver
Hello Greg, Arnd, On Fri, 10 May 2024 12:24:06 +0200 "Arnd Bergmann" wrote: > On Fri, May 10, 2024, at 09:55, Greg Kroah-Hartman wrote: > > On Fri, May 10, 2024 at 09:10:41AM +0200, Luca Ceresoli wrote: > >> > >> +config GE_SUNH_CONNECTOR > >> + tristate "GE SUNH hotplug add-on connector" > >> + depends on OF > >> + select OF_OVERLAY > >> + select FW_LOADER > >> + select NVMEM > >> + select DRM_HOTPLUG_BRIDGE > > > > Can these be depends instead of select? 'select' causes dependencies > > that are hard, if not almost impossible, to detect at times why > > something is being enabled. > > I think FW_LOADER needs to be 'select' since it is normally > a hidden symbol and gets selected by its users, all the other > ones should be 'depends on'. I see, makes sense. And as you pointed that out, I realize perhaps DRM_HOTPLUG_BRIDGE could become a hidden symbol as it's not expected to be used alone. Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2 5/5] misc: add ge-addon-connector driver
Hello Greg,
thanks for reviewing.
On Fri, 10 May 2024 08:55:29 +0100
Greg Kroah-Hartman wrote:
> On Fri, May 10, 2024 at 09:10:41AM +0200, Luca Ceresoli wrote:
> > Add a driver to support the runtime hot-pluggable add-on connector on the
> > GE SUNH device. This connector allows connecting and disconnecting an
> > add-on to/from the main device to augment its features. Connection and
> > disconnection can happen at runtime at any moment without notice.
> >
> > Different add-on models can be connected, and each has an EEPROM with a
> > model identifier at a fixed address.
> >
> > The add-on hardware is added and removed using device tree overlay loading
> > and unloading.
> >
> > Co-developed-by: Herve Codina
> > Signed-off-by: Herve Codina
> > Signed-off-by: Luca Ceresoli
> >
> > ---
> >
> > This commit is new in v2.
> > ---
> > MAINTAINERS | 1 +
> > drivers/misc/Kconfig | 15 ++
> > drivers/misc/Makefile| 1 +
> > drivers/misc/ge-sunh-connector.c | 464
> > +++
> > 4 files changed, 481 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 672c26372c92..0bdb4fc496b8 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9905,6 +9905,7 @@ F:drivers/iio/pressure/mprls0025pa*
> > HOTPLUG CONNECTOR FOR GE SUNH ADDONS
> > M: Luca Ceresoli
> > S: Maintained
> > +F: drivers/misc/ge-sunh-connector.c
> > F: Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml
> >
> > HP BIOSCFG DRIVER
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > index 4fb291f0bf7c..99ef2eccbbaa 100644
> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -574,6 +574,21 @@ config NSM
> > To compile this driver as a module, choose M here.
> > The module will be called nsm.
> >
> > +config GE_SUNH_CONNECTOR
> > + tristate "GE SUNH hotplug add-on connector"
> > + depends on OF
> > + select OF_OVERLAY
> > + select FW_LOADER
> > + select NVMEM
> > + select DRM_HOTPLUG_BRIDGE
>
> Can these be depends instead of select? 'select' causes dependencies
> that are hard, if not almost impossible, to detect at times why
> something is being enabled.
(see reply to Arnd's follow-up e-mail for this)
> > + help
> > + Driver for the runtime hot-pluggable add-on connector on the GE SUNH
> > + device. This connector allows connecting and disconnecting an add-on
> > + to/from the main device to augment its features. Connection and
> > + disconnection can be done at runtime at any moment without
> > + notice. Different add-on models can be connected, and each has an
> > EEPROM
> > + with a model identifier at a fixed address.
>
> Module name?
OK, will add.
> > +static void sunh_conn_reset(struct sunh_conn *conn, bool keep_reset)
> > +{
> > + dev_dbg(conn->dev, "reset\n");
>
> ftrace is your friend.
ACK.
> > +static int sunh_conn_handle_event(struct sunh_conn *conn, bool plugged)
> > +{
> > + int err;
> > +
> > + if (plugged == conn->plugged)
> > + return 0;
> > +
> > + dev_info(conn->dev, "%s\n", plugged ? "connected" : "disconnected");
>
> Please remove debugging code from stuff you want to see merged.
>
> Same for all dev_info() calls here, when drivers work properly, they are
> quiet.
While agree for other dev_info() calls, this one seems quite similar in
principle to the link up/down messages that get logged by the MII code
at [0]:
[347229.872315] asix 1-1.3.2:1.0 enx000cf616fecb: link up, 100Mbps,
full-duplex, lpa 0xC5E1 [347229.920449] asix 1-1.3.2:1.0 enx000cf616fecb:
link down
In my case it is logging that a removable part of the hardware has been
added or removed, which appears useful. Do you think it make sense in
this scenario?
Luca
[0] https://elixir.bootlin.com/linux/v6.8.9/source/drivers/net/mii.c#L557
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v2 1/5] dt-bindings: connector: add GE SUNH hotplug addon connector
Hello Rob, On Fri, 10 May 2024 03:41:35 -0500 "Rob Herring (Arm)" wrote: > On Fri, 10 May 2024 09:10:37 +0200, Luca Ceresoli wrote: > > Add bindings for the GE SUNH add-on connector. This is a physical, > > hot-pluggable connector that allows to attach and detach at runtime an > > add-on adding peripherals on non-discoverable busses. > > > > Signed-off-by: Luca Ceresoli > > > > --- > > > > NOTE: the second and third examples fail 'make dt_binding_check' because > > they are example of DT overlay code -- I'm not aware of a way to > > validate overlay examples as of now As mentioned here... > > > > This patch is new in v2. > > --- > > .../connector/ge,sunh-addon-connector.yaml | 197 > > + > > MAINTAINERS| 5 + > > 2 files changed, 202 insertions(+) > > > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > Error: > Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.example.dts:49.9-14 > syntax error > FATAL ERROR: Unable to parse input tree ...this is expected. Any hints on how this can be managed in bindings examples would be very useful. Best regards, Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH v2 5/5] misc: add ge-addon-connector driver
Add a driver to support the runtime hot-pluggable add-on connector on the
GE SUNH device. This connector allows connecting and disconnecting an
add-on to/from the main device to augment its features. Connection and
disconnection can happen at runtime at any moment without notice.
Different add-on models can be connected, and each has an EEPROM with a
model identifier at a fixed address.
The add-on hardware is added and removed using device tree overlay loading
and unloading.
Co-developed-by: Herve Codina
Signed-off-by: Herve Codina
Signed-off-by: Luca Ceresoli
---
This commit is new in v2.
---
MAINTAINERS | 1 +
drivers/misc/Kconfig | 15 ++
drivers/misc/Makefile| 1 +
drivers/misc/ge-sunh-connector.c | 464 +++
4 files changed, 481 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 672c26372c92..0bdb4fc496b8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9905,6 +9905,7 @@ F:drivers/iio/pressure/mprls0025pa*
HOTPLUG CONNECTOR FOR GE SUNH ADDONS
M: Luca Ceresoli
S: Maintained
+F: drivers/misc/ge-sunh-connector.c
F: Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml
HP BIOSCFG DRIVER
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 4fb291f0bf7c..99ef2eccbbaa 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -574,6 +574,21 @@ config NSM
To compile this driver as a module, choose M here.
The module will be called nsm.
+config GE_SUNH_CONNECTOR
+ tristate "GE SUNH hotplug add-on connector"
+ depends on OF
+ select OF_OVERLAY
+ select FW_LOADER
+ select NVMEM
+ select DRM_HOTPLUG_BRIDGE
+ help
+ Driver for the runtime hot-pluggable add-on connector on the GE SUNH
+ device. This connector allows connecting and disconnecting an add-on
+ to/from the main device to augment its features. Connection and
+ disconnection can be done at runtime at any moment without
+ notice. Different add-on models can be connected, and each has an
EEPROM
+ with a model identifier at a fixed address.
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index ea6ea5bbbc9c..d973de89bd19 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -68,3 +68,4 @@ obj-$(CONFIG_TMR_INJECT) += xilinx_tmr_inject.o
obj-$(CONFIG_TPS6594_ESM) += tps6594-esm.o
obj-$(CONFIG_TPS6594_PFSM) += tps6594-pfsm.o
obj-$(CONFIG_NSM) += nsm.o
+obj-$(CONFIG_GE_SUNH_CONNECTOR)+= ge-sunh-connector.o
diff --git a/drivers/misc/ge-sunh-connector.c b/drivers/misc/ge-sunh-connector.c
new file mode 100644
index ..a40bf4bb56bf
--- /dev/null
+++ b/drivers/misc/ge-sunh-connector.c
@@ -0,0 +1,464 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * GE SUNH hotplug add-on connector
+ *
+ * Driver for the runtime hot-pluggable add-on connector on the GE SUNH
+ * device. Add-on connection is detected via GPIOs (+ a debugfs
+ * trigger). On connection, a "base" DT overlay is added that describes
+ * enough to reach the NVMEM cell with the model ID. Based on the ID, an
+ * add-on-specific overlay is loaded on top to describe everything else.
+ *
+ * Copyright (C) 2024, GE HealthCare
+ *
+ * Authors:
+ * Luca Ceresoli
+ * Herve Codina
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+enum sunh_conn_overlay_level {
+ SUNH_CONN_OVERLAY_BASE,
+ SUNH_CONN_OVERLAY_ADDON,
+ SUNH_CONN_OVERLAY_N_LEVELS
+};
+
+#define SUNH_CONN_N_STATUS_GPIOS 2
+static const char * const sunh_conn_status_gpio_name[SUNH_CONN_N_STATUS_GPIOS]
= {
+ "plugged", "powergood"
+};
+
+struct sunh_conn {
+ struct device *dev;
+ struct gpio_desc *reset_gpio;
+ struct gpio_desc *status_gpio[SUNH_CONN_N_STATUS_GPIOS];
+
+ bool plugged;
+ int ovcs_id[SUNH_CONN_OVERLAY_N_LEVELS];
+ struct mutex ovl_mutex; // serialize overlay code
+ struct notifier_block nvmem_nb;
+ struct work_struct nvmem_notifier_work;
+
+ struct platform_device *hpb_pdev;
+ struct dentry *debugfs_root;
+};
+
+static int sunh_conn_insert_overlay(struct sunh_conn *conn,
+ enum sunh_conn_overlay_level level,
+ const char *filename)
+{
+ const struct firmware *fw;
+ int err;
+
+ err = request_firmware(, filename, conn->dev);
+ if (err)
+ return dev_err_probe(conn->dev, err, "Error requesting overlay
%s", filename);
+
+ dev_dbg(conn->dev, "insert overlay %d: %s", level, filename);
+ err = of_overlay
[PATCH v2 3/5] drm/encoder: add drm_encoder_cleanup_from()
Supporting hardware whose final part of the DRM pipeline can be physically
removed requires the ability to detach all bridges from a given point to
the end of the pipeline.
Introduce a variant of drm_encoder_cleanup() for this.
Signed-off-by: Luca Ceresoli
---
Changed in v2:
- fix a typo in a comment
---
drivers/gpu/drm/drm_encoder.c | 21 +
include/drm/drm_encoder.h | 1 +
2 files changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 8f2bc6a28482..472dfbefe296 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -207,6 +207,27 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
}
EXPORT_SYMBOL(drm_encoder_cleanup);
+/**
+ * drm_encoder_cleanup_from - remove a given bridge and all the following
+ * @encoder: encoder whole list of bridges shall be pruned
+ * @bridge: first bridge to remove
+ *
+ * Removes from an encoder all the bridges starting with a given bridge
+ * and until the end of the chain.
+ *
+ * This should not be used in "normal" DRM pipelines. It is only useful for
+ * devices whose final part of the DRM chain can be physically removed and
+ * later reconnected (possibly with different hardware).
+ */
+void drm_encoder_cleanup_from(struct drm_encoder *encoder, struct drm_bridge
*bridge)
+{
+ struct drm_bridge *next;
+
+ list_for_each_entry_safe_from(bridge, next, >bridge_chain,
chain_node)
+ drm_bridge_detach(bridge);
+}
+EXPORT_SYMBOL(drm_encoder_cleanup_from);
+
static void drmm_encoder_alloc_release(struct drm_device *dev, void *ptr)
{
struct drm_encoder *encoder = ptr;
diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index 977a9381c8ba..bafcabb24267 100644
--- a/include/drm/drm_encoder.h
+++ b/include/drm/drm_encoder.h
@@ -320,6 +320,7 @@ static inline struct drm_encoder *drm_encoder_find(struct
drm_device *dev,
}
void drm_encoder_cleanup(struct drm_encoder *encoder);
+void drm_encoder_cleanup_from(struct drm_encoder *encoder, struct drm_bridge
*bridge);
/**
* drm_for_each_encoder_mask - iterate over encoders specified by bitmask
--
2.34.1
[PATCH v2 4/5] drm/bridge: hotplug-bridge: add driver to support hot-pluggable DSI bridges
This driver implements the point of a DRM pipeline where a connector allows removal of all the following bridges up to the panel. The DRM subsystem currently allows hotplug of the monitor but not preceding components. However there are embedded devices where the "tail" of the DRM pipeline, including one or more bridges, can be physically removed: .. | DISPLAY CONTROLLER | | .-. .--. | | | ENCODER |<--| CRTC | | | '-' '--' | '--|-' | | HOTPLUG V CONNECTOR .-..--..-..-. .---. | 0 to N || _| _| || 1 to N | | | | BRIDGES |--DSI-->||_ |_ |--DSI-->| BRIDGES |--LVDS-->| PANEL | | || || || | | | '-''--''-''-' '---' [--- fixed components --] [--- removable add-on ---] This driver supports such devices, where the final segment of a MIPI DSI bus, including one or more bridges, can be physically disconnected and reconnected at runtime, possibly with a different model. This implementation supports a MIPI DSI bus only, but it is designed to be as far as possible generic and extendable to other busses that have no native hotplug and model ID discovery. This driver does not provide facilities to add and remove the hot-pluggable components from the kernel: this needs to be done by other means (e.g. device tree overlay runtime insertion and removal). The hotplug-bridge gets notified of hot-plugging by the DRM bridge notifier callbacks after they get added or before they get removed. The hotplug-bridge role is to implement the "hot-pluggable connector" in the bridge chain. In this position, what the hotplug-bridge should ideally do is: * communicate with the previous component (bridge or encoder) so that it believes it always has a connected bridge following it and the DRM card is always present * be notified of the addition and removal of the following bridge and attach/detach to/from it * communicate with the following bridge so that it will attach and detach using the normal procedure (as if the entire pipeline were being created or destroyed, not only the tail) * expose the "add-on connected/disconnected" status via the DRM connector connected/disconnected status, so that users of the DRM pipeline know when they can render output on the display However some aspects make it a bit more complex than that. Most notably: * the next bridge can be probed and removed at any moment and all probing sequences need to be handled * the DSI host/device registration process, which adds to the DRM bridge attach process, makes the initial card registration tricky * the need to register and deregister the following bridges at runtime without tearing down the whole DRM card prevents using the functions that are normally recommended * the automatic mechanism to call the appropriate .get_modes operation (typically provided by the panel bridge) cannot work as the panel can disappear and reappear as a different model, so an ad-hoc lookup is needed The code handling these and other tricky aspects is accurately documented by comments in the code. Co-developed-by: Paul Kocialkowski Signed-off-by: Paul Kocialkowski Signed-off-by: Luca Ceresoli --- Changed in v2: - change to be a platform device instantiated from the connector driver instead of a self-standing OF driver - add missing error handling for devm_drm_bridge_add() - various cleanups and style improvements --- MAINTAINERS | 5 + drivers/gpu/drm/bridge/Kconfig | 15 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/hotplug-bridge.c | 577 4 files changed, 598 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 4955501217eb..672c26372c92 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6712,6 +6712,11 @@ T: git git://anongit.freedesktop.org/drm/drm-misc F: Documentation/devicetree/bindings/display/panel/himax,hx8394.yaml F: drivers/gpu/drm/panel/panel-himax-hx8394.c +DRM DRIVER FOR HOTPLUG VIDEO CONNECTOR BRIDGE +M: Luca Ceresoli +S: Maintained +F: drivers/gpu/drm/bridge/hotplug-bridge.c + DRM DRIVER FOR HX8357D PANELS S: Orphan T: git git://anongit.freedesktop.org/drm/drm-misc diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index efd996f6c138..409d090ee94d 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -90,6 +90,21 @@ config DRM_FSL_LDB help Support for i.MX8MP DPI-to-LVDS on-SoC encoder. +config DRM_HOTPLUG_BRIDGE + tristate "Hotplug DRM bridge support" + depends on OF +
[PATCH v2 2/5] drm/bridge: add bridge notifier to be notified of bridge addition and removal
From: Paul Kocialkowski
In preparation for allowing bridges to be added to and removed from a DRM
card without destroying the whole card, add a DRM bridge notifier. Notified
events are addition and removal to/from the global bridge list.
Co-developed-by: Luca Ceresoli
Signed-off-by: Luca Ceresoli
Signed-off-by: Paul Kocialkowski
---
drivers/gpu/drm/drm_bridge.c | 35 +++
include/drm/drm_bridge.h | 19 +++
2 files changed, 54 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 521a71c61b16..245f7fa4ea22 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
@@ -197,6 +198,36 @@
static DEFINE_MUTEX(bridge_lock);
static LIST_HEAD(bridge_list);
+static BLOCKING_NOTIFIER_HEAD(bridge_notifier);
+
+/**
+ * drm_bridge_notifier_register - add a DRM bridge notifier
+ * @nb: the notifier block to be registered
+ *
+ * The notifier block will be notified of events defined in
+ * _bridge_notifier_event
+ */
+int drm_bridge_notifier_register(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(_notifier, nb);
+}
+EXPORT_SYMBOL(drm_bridge_notifier_register);
+
+/**
+ * drm_bridge_notifier_unregister - remove a DRM bridge notifier
+ * @nb: the notifier block to be unregistered
+ */
+int drm_bridge_notifier_unregister(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_unregister(_notifier, nb);
+}
+EXPORT_SYMBOL(drm_bridge_notifier_unregister);
+
+static void drm_bridge_notifier_notify(unsigned long event,
+ struct drm_bridge *bridge)
+{
+ blocking_notifier_call_chain(_notifier, event, bridge);
+}
/**
* drm_bridge_add - add the given bridge to the global bridge list
@@ -210,6 +241,8 @@ void drm_bridge_add(struct drm_bridge *bridge)
mutex_lock(_lock);
list_add_tail(>list, _list);
mutex_unlock(_lock);
+
+ drm_bridge_notifier_notify(DRM_BRIDGE_NOTIFY_ADD, bridge);
}
EXPORT_SYMBOL(drm_bridge_add);
@@ -243,6 +276,8 @@ EXPORT_SYMBOL(devm_drm_bridge_add);
*/
void drm_bridge_remove(struct drm_bridge *bridge)
{
+ drm_bridge_notifier_notify(DRM_BRIDGE_NOTIFY_REMOVE, bridge);
+
mutex_lock(_lock);
list_del_init(>list);
mutex_unlock(_lock);
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4baca0d9107b..ee48c1eb76ae 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -43,6 +43,22 @@ struct drm_panel;
struct edid;
struct i2c_adapter;
+/**
+ * enum drm_bridge_notifier_event - DRM bridge events
+ */
+enum drm_bridge_notifier_event {
+ /**
+* @DRM_BRIDGE_NOTIFY_ADD: A bridge has just been added to the
+* global bridge list. See drm_bridge_add().
+*/
+ DRM_BRIDGE_NOTIFY_ADD,
+ /**
+* @DRM_BRIDGE_NOTIFY_REMOVE: A bridge is about to be removed from
+* the global bridge list. See drm_bridge_remove().
+*/
+ DRM_BRIDGE_NOTIFY_REMOVE,
+};
+
/**
* enum drm_bridge_attach_flags - Flags for _bridge_funcs.attach
*/
@@ -781,6 +797,9 @@ drm_priv_to_bridge(struct drm_private_obj *priv)
return container_of(priv, struct drm_bridge, base);
}
+int drm_bridge_notifier_register(struct notifier_block *nb);
+int drm_bridge_notifier_unregister(struct notifier_block *nb);
+
void drm_bridge_add(struct drm_bridge *bridge);
int devm_drm_bridge_add(struct device *dev, struct drm_bridge *bridge);
void drm_bridge_remove(struct drm_bridge *bridge);
--
2.34.1
[PATCH v2 1/5] dt-bindings: connector: add GE SUNH hotplug addon connector
Add bindings for the GE SUNH add-on connector. This is a physical,
hot-pluggable connector that allows to attach and detach at runtime an
add-on adding peripherals on non-discoverable busses.
Signed-off-by: Luca Ceresoli
---
NOTE: the second and third examples fail 'make dt_binding_check' because
they are example of DT overlay code -- I'm not aware of a way to
validate overlay examples as of now
This patch is new in v2.
---
.../connector/ge,sunh-addon-connector.yaml | 197 +
MAINTAINERS| 5 +
2 files changed, 202 insertions(+)
diff --git
a/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml
b/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml
new file mode 100644
index ..c7ac62e5f2c9
--- /dev/null
+++ b/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml
@@ -0,0 +1,197 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/connector/ge,sunh-addon-connector.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GE SUNH hotplug add-on connector
+
+maintainers:
+ - Luca Ceresoli
+
+description:
+ Represent the physical connector present on GE SUNH devices that allows
+ to attach and detach at runtime an add-on adding peripherals on
+ non-discoverable busses.
+
+ This connector has status GPIOs to notify the connection status to the
+ CPU and a reset GPIO to allow the CPU to reset all the peripherals on the
+ add-on. It also has a 4-lane MIPI DSI bus.
+
+ Add-on removal can happen at any moment under user control and without
+ prior notice to the CPU, making all of its components not usable
+ anymore. Later on, the same or a different add-on model can be connected.
+
+properties:
+ compatible:
+const: ge,sunh-addon-connector
+
+ reset-gpios:
+description: An output GPIO to reset the peripherals on the add-on.
+maxItems: 1
+
+ plugged-gpios:
+description: An input GPIO that is asserted if and only if an add-on is
+ physically connected.
+maxItems: 1
+
+ powergood-gpios:
+description: An input GPIO that is asserted if and only if power rails
+ on the add-on are stable.
+maxItems: 1
+
+ ports:
+$ref: /schemas/graph.yaml#/properties/ports
+
+description: OF graph bindings modeling the MIPI DSI bus across the
+ connector. The connector splits the video pipeline in a fixed part
+ and a removable part.
+
+ The fixed part of the video pipeline includes all components up to
+ the display controller and 0 or more bridges. The removable part
+ includes any bridges and any other components up to the panel.
+
+properties:
+ port@0:
+$ref: /schemas/graph.yaml#/properties/port
+description: The MIPI DSI bus line from the CPU to the connector.
+ The remote-endpoint sub-node must point to the last non-removable
+ component of the video pipeline.
+
+ port@1:
+$ref: /schemas/graph.yaml#/properties/port
+
+description: The MIPI DSI bus line from the connector to the
+ add-on. The remote-endpoint sub-node must point to the first
+ add-on component of the video pipeline. As it describes the
+ hot-pluggable hardware, the endpoint node cannot be filled until
+ an add-on is detected, so this needs to be done by a device tree
+ overlay at runtime.
+
+required:
+ - port@0
+ - port@1
+
+required:
+ - compatible
+
+unevaluatedProperties: false
+
+examples:
+ # Main DTS describing the "main" board up to the connector
+ - |
+/ {
+#include
+
+addon_connector: addon-connector {
+compatible = "ge,sunh-addon-connector";
+reset-gpios = < 1 GPIO_ACTIVE_LOW>;
+plugged-gpios = < 2 GPIO_ACTIVE_LOW>;
+powergood-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 {
+reg = <0>;
+
+hotplug_conn_dsi_in: endpoint {
+remote-endpoint = <_bridge_out>;
+};
+};
+
+port@1 {
+reg = <1>;
+
+hotplug_conn_dsi_out: endpoint {
+// remote-endpoint to be added by overlay
+};
+};
+};
+};
+};
+
+ # "base" overlay describing the common components on every add-on that
+ # are required to read the model ID
+ - |
+ {
+#address-cells = <1>;
+#size-cells = <0>;
+
+eeprom@50 {
+compatible = "atmel,24c64";
+reg = <0x50>;
+
+nvmem-layout {
+
[PATCH v2 0/5] Add support for GE SUNH hot-pluggable connector (was: "drm: add support for hot-pluggable bridges")
having to even be aware of hot-plugging.
The hotplug-bridge it is based on a few self-contained additions to
drm_bridge and drm_encoder, which are provided as individual patches in
this series, and does not require any modifications to other bridges.
However the implementation has some tricky aspects that make it more
complex than in an ideal design. See the patch adding the driver for the
details.
Outstanding bugs and issues
===
Unsurprisingly, enabling device tree overlay loading/unloading at runtime
is exposing a number of issues. While testing this work and another,
unrelated work also using overlay insertion/removal [1] we ancountered
several and we are working on solving them one by one. Here is a list of
the issues for which we have sent some patches:
1. ERROR: remove_proc_entry: removing non-empty directory 'irq/233', leaking
at least 'gpiomon'
-
https://lore.kernel.org/all/20240227113426.253232-1-herve.cod...@bootlin.com/
2. leds: gpio: Add devlink between the leds-gpio device and the gpio used
-
https://lore.kernel.org/all/20240220133950.138452-1-herve.cod...@bootlin.com/
3. kobject: 'gpiochip8' ((ptrval)): is not initialized, yet
kobject_get() is being called.
-
https://lore.kernel.org/all/CAGETcx_YjRzA0joyESsgk=xjkbqqfd7yzeswku1a1deo-ey...@mail.gmail.com/
Overlay removal is also known for memory leaks of some properties (the
"deadprops" list). This is being examined for a proper solution.
An issue related to devlink appeared since commit 782bfd03c3ae ("of:
property: Improve finding the supplier of a remote-endpoint property"),
merged in v6.8-rc5, as reported in:
https://lore.kernel.org/all/20240223171849.10f9901d@booty
This is on my todo list as well. The current workaround is reverting
782bfd03c3ae.
Finally, a known issue is related to NVMEM. The connector driver uses NVMEM
notifiers to be notified of cell addition. This works reliably assuming the
cell to be available when the notification fucntion is called, but this
does not happen. Two alternative patches have been sent that would fix
this:
- https://lore.kernel.org/all/20240130160021.70ddef92@xps-13/
-
https://lore.kernel.org/all/20231229-nvmem-cell-add-notification-v1-1-8d8b426be...@bootlin.com/
There was no activity recently about this. Continuing this work in on my
todo list.
[1] https://lore.kernel.org/all/20240430083730.134918-1-herve.cod...@bootlin.com
That's all
==
Thanks for you patience in reading this!
Luca
Changes in v2:
- Added bindings and driver for ge,sunh-addon-connector
- Removed bindings for the hotplug-video-connector, this is now represented
in DT as part of the ge,sunh-addon-connector
- Various monior improvements to the DRM hotplug-bridge driver
- Link to v1:
https://lore.kernel.org/r/20240326-hotplug-drm-bridge-v1-0-4b51b5eb7...@bootlin.com
Co-developed-by: Paul Kocialkowski
Signed-off-by: Luca Ceresoli
---
Luca Ceresoli (4):
dt-bindings: connector: add GE SUNH hotplug addon connector
drm/encoder: add drm_encoder_cleanup_from()
drm/bridge: hotplug-bridge: add driver to support hot-pluggable DSI
bridges
misc: add ge-addon-connector driver
Paul Kocialkowski (1):
drm/bridge: add bridge notifier to be notified of bridge addition and
removal
.../connector/ge,sunh-addon-connector.yaml | 197 +++
MAINTAINERS| 11 +
drivers/gpu/drm/bridge/Kconfig | 15 +
drivers/gpu/drm/bridge/Makefile| 1 +
drivers/gpu/drm/bridge/hotplug-bridge.c| 577 +
drivers/gpu/drm/drm_bridge.c | 35 ++
drivers/gpu/drm/drm_encoder.c | 21 +
drivers/misc/Kconfig | 15 +
drivers/misc/Makefile | 1 +
drivers/misc/ge-sunh-connector.c | 464 +
include/drm/drm_bridge.h | 19 +
include/drm/drm_encoder.h | 1 +
12 files changed, 1357 insertions(+)
---
base-commit: 5e3810587f43a24d2c568b7e08fcff7ce05d71a9
change-id: 20240319-hotplug-drm-bridge-16b86e67fe92
Best regards,
--
Luca Ceresoli
[PATCH RESEND] Revert "drm/bridge: ti-sn65dsi83: Fix enable error path"
This reverts commit 8a91b29f1f50ce7742cdbe5cf11d17f128511f3f.
The regulator_disable() added by the original commit solves one kind of
regulator imbalance but adds another one as it allows the regulator to be
disabled one more time than it is enabled in the following scenario:
1. Start video pipeline -> sn65dsi83_atomic_pre_enable -> regulator_enable
2. PLL lock fails -> regulator_disable
3. Stop video pipeline -> sn65dsi83_atomic_disable -> regulator_disable
The reason is clear from the code flow, which looks like this (after
removing unrelated code):
static void sn65dsi83_atomic_pre_enable()
{
regulator_enable(ctx->vcc);
if (PLL failed locking) {
regulator_disable(ctx->vcc); < added by patch being reverted
return;
}
}
static void sn65dsi83_atomic_disable()
{
regulator_disable(ctx->vcc);
}
The use case for introducing the additional regulator_disable() was
removing the module for debugging (see link below for the discussion). If
the module is removed after a .atomic_pre_enable, i.e. with an active
pipeline from the DRM point of view, .atomic_disable is not called and thus
the regulator would not be disabled.
According to the discussion however there is no actual use case for
removing the module with an active pipeline, except for
debugging/development.
On the other hand, the occurrence of a PLL lock failure is possible due to
any physical reason (e.g. a temporary hardware failure for electrical
reasons) so handling it gracefully should be supported. As there is no way
for .atomic[_pre]_enable to report an error to the core, the only clean way
to support it is calling regulator_disabled() only in .atomic_disable,
unconditionally, as it was before.
Link: https://lore.kernel.org/all/15244220.uLZWGnKmhe@steina-w/
Fixes: 8a91b29f1f50 ("drm/bridge: ti-sn65dsi83: Fix enable error path")
Reviewed-by: Alexander Stein
Signed-off-by: Luca Ceresoli
---
Many thanks to Alexander for the discussion.
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 4814b7b6d1fd..57a7ed13f996 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -478,7 +478,6 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge
*bridge,
dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
/* On failure, disable PLL again and exit. */
regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
- regulator_disable(ctx->vcc);
return;
}
--
2.34.1
Re: [PATCH 4/4] drm/bridge: hotplug-bridge: add driver to support hot-pluggable DSI bridges
Hi Maxime, On Wed, 27 Mar 2024 17:08:49 +0100 Luca Ceresoli wrote: [...] > > There's several additional hurdles there: > > > > - You mentioned the connector in your ideal scenario. But as soon as > >you remove the last bridge, the connector will probably go away too. > >There's two scenarii here then: > > > >- The driver is ok, and it will stay there until the last user its to > > the main DRM device. Which means that if you create a new one, > > you'll have the old one and the new one together, but you can't > > tell which one you're supposed to use. > > > >- If the driver isn't ok, the connector will be freed immediately. > > There's plenty of lingering pointers in the framework, and > > especially the states though, leading to use-after-free errors. > > > > - So far, we told everyone that the graphics pipeline wasn't going to > >change. How do you expect applications to deal with a connector going > >away without any regression? I guess the natural thing here would be > >to emit a uevent just like we do when the connection status change, > >but the thing is: we're doing that for the connector, and the > >connector is gone. > > Thanks for your feedback. I probably should have discussed this aspect > in my cover letter, sorry about that, let me amend now. > > I think there are two possible approaches. > > The first approach is based on removing the drm_connector. My laptop > uses the i915 driver, and I have observed that attaching/removing a > USB-C dock with an HDMI connector connected to a monitor, a new > drm_connector appears/disappears for the card. User space gets notified > and the external monitor is enabled/disabled, just the way a desktop > user would expect, so this is possible. I had a look at the driver but > how this magic happens was not clear to me honestly. > > The second approach is simpler and based on keeping the drm_connector > always instantiated, and it is what this driver does. The drm_connector > is added by the hotplug-bridge driver in the drm_bridge_funcs.attach op, > which happens initially, and only removed by drm_bridge_funcs.detach, > so it is never removed when detaching the _following_ part of the > pipeline (which the card is unaware of). So the encoder always has a > drm_connector. > > Note when attaching to the downstream bridge we pass the > DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, which _should_ prevent creation of a > second connector. I'd expect some drivers to not honour that flag, but > they can be fixed if needed. > > When the tail of the pipeline is connected/removed, the > hpb->next_bridge pointer becomes valid/NULL. And > hotplug_bridge_detect() looks at exactly that pointer to return a > connected or disconnected status. > > The result is that when the add-on is connected, 'modetest -c' shows: > > Connectors: > id encoder status namesize (mm) modes > encoders > 37 0 connected DSI-1 293x165 1 36 > modes: > index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot > #0 1920x1080 60.00 1920 1978 2020 2108 1080 1088 1102 1116 141140 flags: > ; type: preferred, driver > props: > ... > > and when it is disconnected, it shows: > > Connectors: > id encoder status namesize (mm) modes > encoders > 37 0 disconnectedDSI-1 0x0 0 36 > props: > ... > > weston detects the HPD events from the connector and starts/stops using > the removable display correctly. > > Does this clarify the approach? > > I could be missing some aspects of course, especially in case of more > complex hardware setups than the one I have. However the code in this > series has been tested for a long time and no memory-safety issue has > appeared. > > > Between the userspace expectations and the memory-safety issue plaguing > > way too many drivers, I'm not sure this approach can work. > > > > I guess one way to somewhat achieve what you're trying to do would be to > > introduce the connection status at the bridge level, reflect the > > aggregate connection status of all bridges on the connector, and make > > each bridge driver probe its device in the connect hook through DCS or > > I2C. > > I think you mean: keeping all the bridge drivers instantiated, even > when the physical chip is removed. > > This is of course another possible approach. However it would be more > invasive, forcing bridge drivers to change their current behaviou
Re: [PATCH 1/4] dt-bindings: display: bridge: add the Hot-plug MIPI DSI connector
Hello Rob,
[+Cc Wolfram for the I2C discussion below]
thanks for your feedback.
On Wed, 27 Mar 2024 11:09:08 -0500
Rob Herring wrote:
> On Tue, Mar 26, 2024 at 05:28:11PM +0100, Luca Ceresoli wrote:
> > Add bindings for a physical, hot-pluggable connector allowing the far end
> > of a MIPI DSI bus to be connected and disconnected at runtime.
> >
> > Signed-off-by: Luca Ceresoli
> > ---
> > .../bridge/hotplug-video-connector-dsi.yaml| 87
> > ++
> > MAINTAINERS| 5 ++
> > 2 files changed, 92 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/hotplug-video-connector-dsi.yaml
> >
> > b/Documentation/devicetree/bindings/display/bridge/hotplug-video-connector-dsi.yaml
> > new file mode 100644
> > index ..05beb8aa9ab4
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/display/bridge/hotplug-video-connector-dsi.yaml
> > @@ -0,0 +1,87 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> > http://devicetree.org/schemas/display/bridge/hotplug-video-connector-dsi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Hot-pluggable connector on a MIPI DSI bus
> > +
> > +maintainers:
> > + - Luca Ceresoli
> > +
> > +description:
> > + A bridge representing a physical, hot-pluggable connector on a MIPI DSI
> > + video bus. The connector splits the video pipeline in a fixed part and a
> > + removable part.
> > +
> > + The fixed part of the video pipeline includes all components up to the
> > + display controller and 0 or more bridges. The removable part includes one
> > + or more bridges and any other components up to the panel.
> > +
> > + The removable part of the pipeline can be physically disconnected at any
> > + moment, making all of its components not usable anymore. The same or a
> > + different removable part of the pipeline can be reconnected later on.
> > +
> > + Note that the hotplug-video-connector does not describe video busses
> > + having native hotplug capabilities in the hardware, such as HDMI.
> > +
> > +properties:
> > + compatible:
> > +const: hotplug-video-connector-dsi
>
> Got a spec for this connector? How do I know if I have one or not?
>
> The problem here is what else is on this connector? GPIO controls,
> power rails, etc.?
>
> If this is some kind of standard connector, then we need to be able to
> remap everything on the connector not just DSI signals. And for that,
> it's not just DSI signals, so I'd say we would need some sort of generic
> graph remapping that the core graph code handles transparently.
>
> If it is not standard, then you don't need any remapping and can just
> use an overlay that connects the ports directly.
This is not a standardized connector. And it couldn't be: to the best of
my knowledge no standard removable MIPI DSI connector exists at all.
This whole work is unavoidably breaking some long-standing assumptions
and opening some new challenges: giving a proper DT description to
runtime-pluggable hardware and breaking the dogma that a DRM pipeline
is not removable, to name some. So I think it's better to take a step
back and describe the big picture.
As mentioned in the cover letter ("Development roadmap" section),
together with Hervé we are working on a set of patches to allow this
sort of removable hardware to be handled properly by the Linux kernel.
From the cover letter:
> The use case we are working on is to support professional products that
> have a portable "main" part running on battery, with the main SoC and able
> to work autonomously with limited features, and that can be connected to an
> "add-on" part that is not portable and adds more features.
The add-on gets connected via a connector that is not standardized.
There is a well-defined part number for the mechanical connector, but
the pin usage is custom for the product. Connector pins include:
* I2C lines to access various chips on the add-on
- one of these chips is an EEPROM with the add-on product ID
* Some pins to report to the CPU whether the add-on is connected and
add-on power rails are stable (these are wired to SoC GPIOs)
* An interrupt line collecting IRQs from the add-on chips
* MIPI DSI to drive a panel on the add-on
* Gigabit Ethernet (4 pairs)
* USB lines
* A few more accessory pins
Some of these are not problematic: USB is hot-pluggable and
auto-discoverable by nature, Ethernet pins are just connected directly
to the RJ-45 connector so
Re: [PATCH 4/4] drm/bridge: hotplug-bridge: add driver to support hot-pluggable DSI bridges
Hi Maxime, On Wed, 27 Mar 2024 13:42:40 +0100 Maxime Ripard wrote: > On Tue, Mar 26, 2024 at 05:28:14PM +0100, Luca Ceresoli wrote: > > This driver implements the point of a DRM pipeline where a connector allows > > removal of all the following bridges up to the panel. > > > > The DRM subsystem currently allows hotplug of the monitor but not preceding > > components. However there are embedded devices where the "tail" of the DRM > > pipeline, including one or more bridges, can be physically removed: > > > > .. > > | DISPLAY CONTROLLER | > > | .-. .--. | > > | | ENCODER |<--| CRTC | | > > | '-' '--' | > > '--|-' > > | > > | HOTPLUG > > V CONNECTOR > >.-..--..-..-. .---. > >| 0 to N || _| _| || 1 to N | | | > >| BRIDGES |--DSI-->||_ |_ |--DSI-->| BRIDGES |--LVDS-->| PANEL | > >| || || || | | | > >'-''--''-''-' '---' > > > > [--- fixed components --] [--- removable add-on ---] > > > > This driver supports such devices, where the final segment of a MIPI DSI > > bus, including one or more bridges, can be physically disconnected and > > reconnected at runtime, possibly with a different model. > > > > This implementation supports a MIPI DSI bus only, but it is designed to be > > as far as possible generic and extendable to other busses that have no > > native hotplug and model ID discovery. > > > > This driver does not provide facilities to add and remove the hot-pluggable > > components from the kernel: this needs to be done by other means > > (e.g. device tree overlay runtime insertion and removal). The > > hotplug-bridge gets notified of hot-plugging by the DRM bridge notifier > > callbacks after they get added or before they get removed. > > > > The hotplug-bridge role is to implement the "hot-pluggable connector" in > > the bridge chain. In this position, what the hotplug-bridge should ideally > > do is: > > > > * communicate with the previous component (bridge or encoder) so that it > >believes it always has a connected bridge following it and the DRM card > >is always present > > * be notified of the addition and removal of the following bridge and > >attach/detach to/from it > > * communicate with the following bridge so that it will attach and detach > >using the normal procedure (as if the entire pipeline were being created > >or destroyed, not only the tail) > > * expose the "add-on connected/disconnected" status via the DRM connector > >connected/disconnected status, so that users of the DRM pipeline know > >when they can render output on the display > > > > However some aspects make it a bit more complex than that. Most notably: > > > > * the next bridge can be probed and removed at any moment and all probing > >sequences need to be handled > > * the DSI host/device registration process, which adds to the DRM bridge > >attach process, makes the initial card registration tricky > > * the need to register and deregister the following bridges at runtime > >without tearing down the whole DRM card prevents using the functions > >that are normally recommended > > * the automatic mechanism to call the appropriate .get_modes operation > >(typically provided by the panel bridge) cannot work as the panel can > >disappear and reappear as a different model, so an ad-hoc lookup is > >needed > > There's several additional hurdles there: > > - You mentioned the connector in your ideal scenario. But as soon as >you remove the last bridge, the connector will probably go away too. >There's two scenarii here then: > >- The driver is ok, and it will stay there until the last user its to > the main DRM device. Which means that if you create a new one, > you'll have the old one and the new one together, but you can't > tell which one you're supposed to use. > >- If the driver isn't ok, the connector will be freed immediately. > There's plenty of lingering pointers in the framework, and > especially the states though, leading to use-after-free errors. > > - So far, we told everyone that the graphics pipeline wasn't going to >change. How do you
[PATCH 4/4] drm/bridge: hotplug-bridge: add driver to support hot-pluggable DSI bridges
This driver implements the point of a DRM pipeline where a connector allows removal of all the following bridges up to the panel. The DRM subsystem currently allows hotplug of the monitor but not preceding components. However there are embedded devices where the "tail" of the DRM pipeline, including one or more bridges, can be physically removed: .. | DISPLAY CONTROLLER | | .-. .--. | | | ENCODER |<--| CRTC | | | '-' '--' | '--|-' | | HOTPLUG V CONNECTOR .-..--..-..-. .---. | 0 to N || _| _| || 1 to N | | | | BRIDGES |--DSI-->||_ |_ |--DSI-->| BRIDGES |--LVDS-->| PANEL | | || || || | | | '-''--''-''-' '---' [--- fixed components --] [--- removable add-on ---] This driver supports such devices, where the final segment of a MIPI DSI bus, including one or more bridges, can be physically disconnected and reconnected at runtime, possibly with a different model. This implementation supports a MIPI DSI bus only, but it is designed to be as far as possible generic and extendable to other busses that have no native hotplug and model ID discovery. This driver does not provide facilities to add and remove the hot-pluggable components from the kernel: this needs to be done by other means (e.g. device tree overlay runtime insertion and removal). The hotplug-bridge gets notified of hot-plugging by the DRM bridge notifier callbacks after they get added or before they get removed. The hotplug-bridge role is to implement the "hot-pluggable connector" in the bridge chain. In this position, what the hotplug-bridge should ideally do is: * communicate with the previous component (bridge or encoder) so that it believes it always has a connected bridge following it and the DRM card is always present * be notified of the addition and removal of the following bridge and attach/detach to/from it * communicate with the following bridge so that it will attach and detach using the normal procedure (as if the entire pipeline were being created or destroyed, not only the tail) * expose the "add-on connected/disconnected" status via the DRM connector connected/disconnected status, so that users of the DRM pipeline know when they can render output on the display However some aspects make it a bit more complex than that. Most notably: * the next bridge can be probed and removed at any moment and all probing sequences need to be handled * the DSI host/device registration process, which adds to the DRM bridge attach process, makes the initial card registration tricky * the need to register and deregister the following bridges at runtime without tearing down the whole DRM card prevents using the functions that are normally recommended * the automatic mechanism to call the appropriate .get_modes operation (typically provided by the panel bridge) cannot work as the panel can disappear and reappear as a different model, so an ad-hoc lookup is needed The code handling these and other tricky aspects is accurately documented by comments in the code. Co-developed-by: Paul Kocialkowski Signed-off-by: Paul Kocialkowski Signed-off-by: Luca Ceresoli --- MAINTAINERS | 1 + drivers/gpu/drm/bridge/Kconfig | 15 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/hotplug-bridge.c | 561 4 files changed, 578 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index e1affd13e30b..b3fe36ed35a0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6720,6 +6720,7 @@ DRM DRIVER FOR HOTPLUG VIDEO CONNECTOR BRIDGE M: Luca Ceresoli S: Maintained F: Documentation/devicetree/bindings/display/bridge/hotplug-video-connector-dsi.yaml +F: drivers/gpu/drm/bridge/hotplug-bridge.c DRM DRIVER FOR HX8357D PANELS S: Orphan diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index efd996f6c138..409d090ee94d 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -90,6 +90,21 @@ config DRM_FSL_LDB help Support for i.MX8MP DPI-to-LVDS on-SoC encoder. +config DRM_HOTPLUG_BRIDGE + tristate "Hotplug DRM bridge support" + depends on OF + select DRM_PANEL_BRIDGE + select DRM_MIPI_DSI + select DRM_KMS_HELPER + help + Driver for a DRM bridge representing a physical connector that + splits a DRM pipeline into a fixed part and a physically + removable part. The fixed part includes up to the encoder and + zero or more bridges. The removable part includes any following +
[PATCH 3/4] drm/encoder: add drm_encoder_cleanup_from()
Supporting hardware whose final part of the DRM pipeline can be physically
removed requires the ability to detach all bridges from a given point to
the end of the pipeline.
Introduce a variant of drm_encoder_cleanup() for this.
Signed-off-by: Luca Ceresoli
---
drivers/gpu/drm/drm_encoder.c | 21 +
include/drm/drm_encoder.h | 1 +
2 files changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 8f2bc6a28482..13149447bec8 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -207,6 +207,27 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
}
EXPORT_SYMBOL(drm_encoder_cleanup);
+/**
+ * drm_encoder_cleanup_from - remove a given bridge and all the following
+ * @encoder: encoder whole list of bridges shall be pruned
+ * @bridge: first bridge to remove
+ *
+ * Removes from an encoder all the bridges starting with a given bridges
+ * and until the end of the chain.
+ *
+ * This should not be used in "normal" DRM pipelines. It is only useful for
+ * devices whose final part of the DRM chain can be physically removed and
+ * later reconnected (possibly with different hardware).
+ */
+void drm_encoder_cleanup_from(struct drm_encoder *encoder, struct drm_bridge
*bridge)
+{
+ struct drm_bridge *next;
+
+ list_for_each_entry_safe_from(bridge, next, >bridge_chain,
chain_node)
+ drm_bridge_detach(bridge);
+}
+EXPORT_SYMBOL(drm_encoder_cleanup_from);
+
static void drmm_encoder_alloc_release(struct drm_device *dev, void *ptr)
{
struct drm_encoder *encoder = ptr;
diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index 977a9381c8ba..bafcabb24267 100644
--- a/include/drm/drm_encoder.h
+++ b/include/drm/drm_encoder.h
@@ -320,6 +320,7 @@ static inline struct drm_encoder *drm_encoder_find(struct
drm_device *dev,
}
void drm_encoder_cleanup(struct drm_encoder *encoder);
+void drm_encoder_cleanup_from(struct drm_encoder *encoder, struct drm_bridge
*bridge);
/**
* drm_for_each_encoder_mask - iterate over encoders specified by bitmask
--
2.34.1
[PATCH 1/4] dt-bindings: display: bridge: add the Hot-plug MIPI DSI connector
Add bindings for a physical, hot-pluggable connector allowing the far end
of a MIPI DSI bus to be connected and disconnected at runtime.
Signed-off-by: Luca Ceresoli
---
.../bridge/hotplug-video-connector-dsi.yaml| 87 ++
MAINTAINERS| 5 ++
2 files changed, 92 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/bridge/hotplug-video-connector-dsi.yaml
b/Documentation/devicetree/bindings/display/bridge/hotplug-video-connector-dsi.yaml
new file mode 100644
index ..05beb8aa9ab4
--- /dev/null
+++
b/Documentation/devicetree/bindings/display/bridge/hotplug-video-connector-dsi.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id:
http://devicetree.org/schemas/display/bridge/hotplug-video-connector-dsi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Hot-pluggable connector on a MIPI DSI bus
+
+maintainers:
+ - Luca Ceresoli
+
+description:
+ A bridge representing a physical, hot-pluggable connector on a MIPI DSI
+ video bus. The connector splits the video pipeline in a fixed part and a
+ removable part.
+
+ The fixed part of the video pipeline includes all components up to the
+ display controller and 0 or more bridges. The removable part includes one
+ or more bridges and any other components up to the panel.
+
+ The removable part of the pipeline can be physically disconnected at any
+ moment, making all of its components not usable anymore. The same or a
+ different removable part of the pipeline can be reconnected later on.
+
+ Note that the hotplug-video-connector does not describe video busses
+ having native hotplug capabilities in the hardware, such as HDMI.
+
+properties:
+ compatible:
+const: hotplug-video-connector-dsi
+
+ ports:
+$ref: /schemas/graph.yaml#/properties/ports
+
+properties:
+ port@0:
+$ref: /schemas/graph.yaml#/properties/port
+description:
+ The end of the fixed part of the MIPI DSI bus (terminating at the
+ hotplug connector). The remote-endpoint sub-node must point to
+ the previous component of the video pipeline.
+
+ port@1:
+$ref: /schemas/graph.yaml#/properties/port
+description:
+ The start of the removable part of the MIPI DSI bus (starting
+ from the hotplug connector). The remote-endpoint sub-node must
+ point to the next component of the video pipeline.
+
+required:
+ - port@0
+ - port@1
+
+required:
+ - compatible
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+hotplug-video-connector {
+compatible = "hotplug-video-connector-dsi";
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 {
+reg = <0>;
+
+hotplug_connector_in: endpoint {
+remote-endpoint = <_bridge_out>;
+};
+};
+
+port@1 {
+reg = <1>;
+
+hotplug_connector_out: endpoint {
+remote-endpoint = <_bridge_in>;
+};
+};
+};
+};
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index aa3b947fb080..e1affd13e30b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6716,6 +6716,11 @@ T: git git://anongit.freedesktop.org/drm/drm-misc
F: Documentation/devicetree/bindings/display/panel/himax,hx8394.yaml
F: drivers/gpu/drm/panel/panel-himax-hx8394.c
+DRM DRIVER FOR HOTPLUG VIDEO CONNECTOR BRIDGE
+M: Luca Ceresoli
+S: Maintained
+F:
Documentation/devicetree/bindings/display/bridge/hotplug-video-connector-dsi.yaml
+
DRM DRIVER FOR HX8357D PANELS
S: Orphan
T: git git://anongit.freedesktop.org/drm/drm-misc
--
2.34.1
[PATCH 2/4] drm/bridge: add bridge notifier to be notified of bridge addition and removal
From: Paul Kocialkowski
In preparation for allowing bridges to be added to and removed from a DRM
card without destroying the whole card, add a DRM bridge notifier. Notified
events are addition and removal to/from the global bridge list.
Co-developed-by: Luca Ceresoli
Signed-off-by: Luca Ceresoli
Signed-off-by: Paul Kocialkowski
---
drivers/gpu/drm/drm_bridge.c | 35 +++
include/drm/drm_bridge.h | 19 +++
2 files changed, 54 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 521a71c61b16..245f7fa4ea22 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
@@ -197,6 +198,36 @@
static DEFINE_MUTEX(bridge_lock);
static LIST_HEAD(bridge_list);
+static BLOCKING_NOTIFIER_HEAD(bridge_notifier);
+
+/**
+ * drm_bridge_notifier_register - add a DRM bridge notifier
+ * @nb: the notifier block to be registered
+ *
+ * The notifier block will be notified of events defined in
+ * _bridge_notifier_event
+ */
+int drm_bridge_notifier_register(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(_notifier, nb);
+}
+EXPORT_SYMBOL(drm_bridge_notifier_register);
+
+/**
+ * drm_bridge_notifier_unregister - remove a DRM bridge notifier
+ * @nb: the notifier block to be unregistered
+ */
+int drm_bridge_notifier_unregister(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_unregister(_notifier, nb);
+}
+EXPORT_SYMBOL(drm_bridge_notifier_unregister);
+
+static void drm_bridge_notifier_notify(unsigned long event,
+ struct drm_bridge *bridge)
+{
+ blocking_notifier_call_chain(_notifier, event, bridge);
+}
/**
* drm_bridge_add - add the given bridge to the global bridge list
@@ -210,6 +241,8 @@ void drm_bridge_add(struct drm_bridge *bridge)
mutex_lock(_lock);
list_add_tail(>list, _list);
mutex_unlock(_lock);
+
+ drm_bridge_notifier_notify(DRM_BRIDGE_NOTIFY_ADD, bridge);
}
EXPORT_SYMBOL(drm_bridge_add);
@@ -243,6 +276,8 @@ EXPORT_SYMBOL(devm_drm_bridge_add);
*/
void drm_bridge_remove(struct drm_bridge *bridge)
{
+ drm_bridge_notifier_notify(DRM_BRIDGE_NOTIFY_REMOVE, bridge);
+
mutex_lock(_lock);
list_del_init(>list);
mutex_unlock(_lock);
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4baca0d9107b..ee48c1eb76ae 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -43,6 +43,22 @@ struct drm_panel;
struct edid;
struct i2c_adapter;
+/**
+ * enum drm_bridge_notifier_event - DRM bridge events
+ */
+enum drm_bridge_notifier_event {
+ /**
+* @DRM_BRIDGE_NOTIFY_ADD: A bridge has just been added to the
+* global bridge list. See drm_bridge_add().
+*/
+ DRM_BRIDGE_NOTIFY_ADD,
+ /**
+* @DRM_BRIDGE_NOTIFY_REMOVE: A bridge is about to be removed from
+* the global bridge list. See drm_bridge_remove().
+*/
+ DRM_BRIDGE_NOTIFY_REMOVE,
+};
+
/**
* enum drm_bridge_attach_flags - Flags for _bridge_funcs.attach
*/
@@ -781,6 +797,9 @@ drm_priv_to_bridge(struct drm_private_obj *priv)
return container_of(priv, struct drm_bridge, base);
}
+int drm_bridge_notifier_register(struct notifier_block *nb);
+int drm_bridge_notifier_unregister(struct notifier_block *nb);
+
void drm_bridge_add(struct drm_bridge *bridge);
int devm_drm_bridge_add(struct device *dev, struct drm_bridge *bridge);
void drm_bridge_remove(struct drm_bridge *bridge);
--
2.34.1
[PATCH 0/4] drm: add support for hot-pluggable bridges
lly: at any given moment the add-on is either connected or disconencted -- it is never considered partially connected. After an add-on has been removed, an add-on of a different model can be connected, e.g. providing a different panel needing different timings. The technique to identify the model that gets connected is outside of the scope of this patch series, as described in "Development roadmap" below. Implementation -- In order to support the above use case while being reasonably generic and avoid unnecessary changes to the common DRM code, the implementation is based on the introduction of the "hotplug-bridge", a new bridge driver that represents the "hotplug connector" and should be positioned in the DRM pipeline exactly where the "hotplug connector" is. In this position the hotplug-bridge decouples the preceding and the following components so that each of them can be implemented normally, without having to even be aware of hot-plugging. The implementation is as transparent as possible in order to minimize the need of any changes to the design of other components: it is based on a few self-contained additions to drm_bridge and drm_encoder, and does not require any modification to other bridges. However the implementation has some tricky aspects that make it more complex than in an ideal design. See the last patch in the series for the details. Patch overview: * patch 1 adds device tree bindings for the "hotplug video connector" * patches 2 and 3 add some prerequisite new features to the DRM code * patch 4 adds the hotplug-bridge itself Development roadmap --- This series is a part of a larger work to support embedded devices having a hot-pluggable add-on. The overall work includes: 1. a mechanism to detect add-on insertion/removal, read the model ID and load a corresponding device tree overlay 2. fixes to existing bugs that get exposed when loading/unloading device tree overlays at runtime 3. allowing the tail of a DRM pipeline to be hot-pluggable [this series] All of the above are under development at Bootlin, and patches for item 2 are already under discussion on the relevant mailing-lists. Item 1 is a prerequisite for production usage of the hotplug-bridge, but even though it is working well enough in internal testing, it is not yet ready for sending patches for review. This patch series covering item 3 is being sent anyway in order to start discussion with the kernel community as early as possible, expecially the DRM community as this work is changing some of the assumptions behind the DRM architecture. Testing --- This series cannot be tested in public until the mechanism for add-on insertion and removal will be sent. It can however be tested by other means, even with a hardware that has no removable parts, "pretending" that one or more bridges can be removed: * remove and re-insert the driver module for the DRM bridge after the hotplug-bridge * unbind/bind the DRM bridge after the hotplug-bridge from its driver Thanks for you patience in reading this! Luca Co-developed-by: Paul Kocialkowski Signed-off-by: Luca Ceresoli --- Luca Ceresoli (3): dt-bindings: display: bridge: add the Hot-plug MIPI DSI connector drm/encoder: add drm_encoder_cleanup_from() drm/bridge: hotplug-bridge: add driver to support hot-pluggable DSI bridges Paul Kocialkowski (1): drm/bridge: add bridge notifier to be notified of bridge addition and removal .../bridge/hotplug-video-connector-dsi.yaml| 87 MAINTAINERS| 6 + drivers/gpu/drm/bridge/Kconfig | 15 + drivers/gpu/drm/bridge/Makefile| 1 + drivers/gpu/drm/bridge/hotplug-bridge.c| 561 + drivers/gpu/drm/drm_bridge.c | 35 ++ drivers/gpu/drm/drm_encoder.c | 21 + include/drm/drm_bridge.h | 19 + include/drm/drm_encoder.h | 1 + 9 files changed, 746 insertions(+) --- base-commit: 30b26f75c864d1da39fe5e8628f1cbc3702a9add change-id: 20240319-hotplug-drm-bridge-16b86e67fe92 Best regards, -- Luca Ceresoli
Re: [PATCH] fbcon: Increase maximum font width x height to 64 x 64
Hello Samuel, On Tue, 12 Mar 2024 22:39:02 +0100 Samuel Thibault wrote: > This remains relatively simple by just enlarging integers. > > It wouldn't be that simple to get to the console's 64x128 maximum, as it would > require 128b integers. > > Signed-off-by: Samuel Thibault > > Index: linux-6.4/drivers/video/fbdev/core/fbcon.c > === This patch is clearly not formatted according to the standard format and it does not apply with 'git am'. Using 'git format-patch' and 'git send-email' is *very* recommended as it will take care of all the formatting for you. Maintainers and anybody interested in your patch will be able to apply it easily. All the info you need are at https://docs.kernel.org/process/submitting-patches.html > --- linux-6.4.orig/drivers/video/fbdev/core/fbcon.c > +++ linux-6.4/drivers/video/fbdev/core/fbcon.c Apparently you are not using git to track your changes, so I recommend using it to have all the git utilities available. Best regards, Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH 1/1] drm/bridge: ti-sn65dsi83: Fix enable error path
Hello Alexander,
On Fri, 01 Mar 2024 11:45:27 +0100
Alexander Stein wrote:
> Hi Luca,
>
> Am Freitag, 1. März 2024, 11:10:59 CET schrieb Luca Ceresoli:
> > Hello Alexander,
> >
> > On Fri, 01 Mar 2024 10:57:37 +0100
> > Alexander Stein wrote:
> >
> > > Hi Luca,
> > >
> > > Am Freitag, 1. März 2024, 10:44:49 CET schrieb Luca Ceresoli:
> > > > Hello Alexander,
> > > >
> > > > On Thu, 29 Feb 2024 12:11:23 +0100
> > > > Alexander Stein wrote:
> > > >
> > > > > Hi Luca,
> > > > >
> > > > > Am Donnerstag, 29. Februar 2024, 10:47:23 CET schrieb Luca Ceresoli:
> > > > >
> > > > > > Hello Alexander,
> > > > > >
> > > > > > On Wed, 28 Feb 2024 09:15:46 +0100
> > > > > > Alexander Stein wrote:
> > > > > >
> > > > > >
> > > > > > [...]
> > > > > >
> > > > > > > Oh I mistook this DSI-LVDS bridge with the DSI-DP bridge on a
> > > > > > > different
> > > > > > > board, my bad. I hope I can provide some insights. My platform is
> > > > > > > imx8mm-tqma8mqml-mba8mx-lvds-tm070jvhg33.dtb.
> > > > > > > I can easily cause a PLL lock failure by reducing the delay for
> > > > > > > the
> > > > > > > enable-gpios 'gpio_delays'. This will result in a PLL lock faiure.
> > > > > > > On my platform the vcc-supply counters do look sane:
> > > > > > > > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> > > > > > > > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:0
> > > > > >
> > > > > > Interesting. Thanks for taking time to report your initial issue!
> > > > > >
> > > > > > > Once I remove the ti_sn65dsi83 module, the open_count decrements
> > > > > > > to 0 as
> > > > > > > well. Looks sane to me.
> > > > > > >
> > > > > > > If I revert commit c81cd8f7c774 ("Revert "drm/bridge:
> > > > > > > ti-sn65dsi83:
> > > > > > > Fix enable error path""), vcc-supply counters are:
> > > > > > > > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> > > > > > > > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:1
> > > > > > >
> > > > > > > So in my case the use_count does not decrease! If I remove the
> > > > > > > module
> > > > > > > ti_sn65dsi83, I get the WARN_ON (enable_count is still non-zero):
> > > > > > >
> > > > > > > > WARNING: CPU: 2 PID: 402 at drivers/regulator/core.c:2398
> > > > > > > > _regulator_put+0x15c/0x164
> > > > > > >
> > > > > > > This is on 6.8.0-rc6-next-20240228 with the following diff
> > > > > > > applied:
> > > > > > > --->8---
> > > > > > > diff --git a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > > > > > > b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > > > > > > index 427467df42bf..8461e1fd396f 100644
> > > > > > > --- a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > > > > > > +++ b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > > > > > > @@ -285,7 +285,7 @@ {
> > > > > > > dsi_lvds_bridge: bridge@2d {
> > > > > > > compatible = "ti,sn65dsi84";
> > > > > > > reg = <0x2d>;
> > > > > > > - enable-gpios = <_delays 0 13 0>;
> > > > > > > + enable-gpios = <_delays 0 0 0>;
> > > > > > > vcc-supply = <_sn65dsi83_1v8>;
> > > > > > > status = "disabled";
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > > > > > b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > > > > > index 4814b7b6d1fd..57a7ed13f996 100644
> > > > > > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > > > >
[PATCH] Revert "drm/bridge: ti-sn65dsi83: Fix enable error path"
This reverts commit 8a91b29f1f50ce7742cdbe5cf11d17f128511f3f.
The regulator_disable() added by the original commit solves one kind of
regulator imbalance but adds another one as it allows the regulator to be
disabled one more time than it is enabled in the following scenario:
1. Start video pipeline -> sn65dsi83_atomic_pre_enable -> regulator_enable
2. PLL lock fails -> regulator_disable
3. Stop video pipeline -> sn65dsi83_atomic_disable -> regulator_disable
The reason is clear from the code flow, which looks like this (after
removing unrelated code):
static void sn65dsi83_atomic_pre_enable()
{
regulator_enable(ctx->vcc);
if (PLL failed locking) {
regulator_disable(ctx->vcc); < added by patch being reverted
return;
}
}
static void sn65dsi83_atomic_disable()
{
regulator_disable(ctx->vcc);
}
The use case for introducing the additional regulator_disable() was
removing the module for debugging (see link below for the discussion). If
the module is removed after a .atomic_pre_enable, i.e. with an active
pipeline from the DRM point of view, .atomic_disable is not called and thus
the regulator would not be disabled.
According to the discussion however there is no actual use case for
removing the module with an active pipeline, except for
debugging/development.
On the other hand, the occurrence of a PLL lock failure is possible due to
any physical reason (e.g. a temporary hardware failure for electrical
reasons) so handling it gracefully should be supported. As there is no way
for .atomic[_pre]_enable to report an error to the core, the only clean way
to support it is calling regulator_disabled() only in .atomic_disable,
unconditionally, as it was before.
Link: https://lore.kernel.org/all/15244220.uLZWGnKmhe@steina-w/
Fixes: 8a91b29f1f50 ("drm/bridge: ti-sn65dsi83: Fix enable error path")
Signed-off-by: Luca Ceresoli
---
Many thanks to Alexander for the discussion.
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index e3501608aef9..12fb22d4cd23 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -499,7 +499,6 @@ printk(KERN_ERR "%s: LVDS in fallback (24/SPWG)\n",
__func__);
dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
/* On failure, disable PLL again and exit. */
regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
- regulator_disable(ctx->vcc);
return;
}
---
base-commit: a71e4adac20bfe852d269addfef340923ce23a4c
change-id: 20240306-ti-sn65dsi83-regulator-imbalance-10e217fd302c
Best regards,
--
Luca Ceresoli
Re: [PATCH 1/1] drm/bridge: ti-sn65dsi83: Fix enable error path
Hello Alexander,
On Fri, 01 Mar 2024 10:57:37 +0100
Alexander Stein wrote:
> Hi Luca,
>
> Am Freitag, 1. März 2024, 10:44:49 CET schrieb Luca Ceresoli:
> > Hello Alexander,
> >
> > On Thu, 29 Feb 2024 12:11:23 +0100
> > Alexander Stein wrote:
> >
> > > Hi Luca,
> > >
> > > Am Donnerstag, 29. Februar 2024, 10:47:23 CET schrieb Luca Ceresoli:
> > > > Hello Alexander,
> > > >
> > > > On Wed, 28 Feb 2024 09:15:46 +0100
> > > > Alexander Stein wrote:
> > > >
> > > >
> > > > [...]
> > > >
> > > > > Oh I mistook this DSI-LVDS bridge with the DSI-DP bridge on a
> > > > > different
> > > > > board, my bad. I hope I can provide some insights. My platform is
> > > > > imx8mm-tqma8mqml-mba8mx-lvds-tm070jvhg33.dtb.
> > > > > I can easily cause a PLL lock failure by reducing the delay for the
> > > > > enable-gpios 'gpio_delays'. This will result in a PLL lock faiure.
> > > > > On my platform the vcc-supply counters do look sane:
> > > > > > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> > > > > > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:0
> > > >
> > > > Interesting. Thanks for taking time to report your initial issue!
> > > >
> > > > > Once I remove the ti_sn65dsi83 module, the open_count decrements to 0
> > > > > as
> > > > > well. Looks sane to me.
> > > > >
> > > > > If I revert commit c81cd8f7c774 ("Revert "drm/bridge: ti-sn65dsi83:
> > > > > Fix enable error path""), vcc-supply counters are:
> > > > > > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> > > > > > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:1
> > > > >
> > > > > So in my case the use_count does not decrease! If I remove the module
> > > > > ti_sn65dsi83, I get the WARN_ON (enable_count is still non-zero):
> > > > > > WARNING: CPU: 2 PID: 402 at drivers/regulator/core.c:2398
> > > > > > _regulator_put+0x15c/0x164
> > > > >
> > > > > This is on 6.8.0-rc6-next-20240228 with the following diff applied:
> > > > >
> > > > > --->8---
> > > > > diff --git a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > > > > b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > > > > index 427467df42bf..8461e1fd396f 100644
> > > > > --- a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > > > > +++ b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > > > > @@ -285,7 +285,7 @@ {
> > > > > dsi_lvds_bridge: bridge@2d {
> > > > > compatible = "ti,sn65dsi84";
> > > > > reg = <0x2d>;
> > > > > - enable-gpios = <_delays 0 13 0>;
> > > > > + enable-gpios = <_delays 0 0 0>;
> > > > > vcc-supply = <_sn65dsi83_1v8>;
> > > > > status = "disabled";
> > > > >
> > > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > > > b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > > > index 4814b7b6d1fd..57a7ed13f996 100644
> > > > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > > > @@ -478,7 +478,6 @@ static void sn65dsi83_atomic_pre_enable(struct
> > > > > drm_bridge *bridge,
> > > > > dev_err(ctx->dev, "failed to lock PLL, ret=%i\n",
> > > > > ret);
> > > > > /* On failure, disable PLL again and exit. */
> > > > > regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> > > > > - regulator_disable(ctx->vcc);
> > > > > return;
> > > > > }
> > > > > --->8---
> > > > >
> > > > > So my patch indeed did fix an actual problem. On the other hand it
> > > > > seems
> > > > > sn65dsi83_atomic_disable is not called in my case for some reason.
> > > >
> > > > So you remove the module and atomic_disable is not called, after
> > > > hav
Re: [PATCH 1/1] drm/bridge: ti-sn65dsi83: Fix enable error path
Hello Alexander,
On Thu, 29 Feb 2024 12:11:23 +0100
Alexander Stein wrote:
> Hi Luca,
>
> Am Donnerstag, 29. Februar 2024, 10:47:23 CET schrieb Luca Ceresoli:
> > Hello Alexander,
> >
> > On Wed, 28 Feb 2024 09:15:46 +0100
> > Alexander Stein wrote:
> >
> >
> > [...]
> >
> > > Oh I mistook this DSI-LVDS bridge with the DSI-DP bridge on a different
> > > board, my bad. I hope I can provide some insights. My platform is
> > > imx8mm-tqma8mqml-mba8mx-lvds-tm070jvhg33.dtb.
> > > I can easily cause a PLL lock failure by reducing the delay for the
> > > enable-gpios 'gpio_delays'. This will result in a PLL lock faiure.
> > > On my platform the vcc-supply counters do look sane:
> > > > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> > > > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:0
> >
> > Interesting. Thanks for taking time to report your initial issue!
> >
> > > Once I remove the ti_sn65dsi83 module, the open_count decrements to 0 as
> > > well. Looks sane to me.
> > >
> > > If I revert commit c81cd8f7c774 ("Revert "drm/bridge: ti-sn65dsi83:
> > > Fix enable error path""), vcc-supply counters are:
> > > > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> > > > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:1
> > >
> > > So in my case the use_count does not decrease! If I remove the module
> > > ti_sn65dsi83, I get the WARN_ON (enable_count is still non-zero):
> > > > WARNING: CPU: 2 PID: 402 at drivers/regulator/core.c:2398
> > > > _regulator_put+0x15c/0x164
> > >
> > > This is on 6.8.0-rc6-next-20240228 with the following diff applied:
> > > --->8---
> > > diff --git a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > > b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > > index 427467df42bf..8461e1fd396f 100644
> > > --- a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> > > @@ -285,7 +285,7 @@ {
> > > dsi_lvds_bridge: bridge@2d {
> > > compatible = "ti,sn65dsi84";
> > > reg = <0x2d>;
> > > - enable-gpios = <_delays 0 13 0>;
> > > + enable-gpios = <_delays 0 0 0>;
> > > vcc-supply = <_sn65dsi83_1v8>;
> > > status = "disabled";
> > >
> > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > index 4814b7b6d1fd..57a7ed13f996 100644
> > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > @@ -478,7 +478,6 @@ static void sn65dsi83_atomic_pre_enable(struct
> > > drm_bridge *bridge,
> > > dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
> > > /* On failure, disable PLL again and exit. */
> > > regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> > > - regulator_disable(ctx->vcc);
> > > return;
> > > }
> > > --->8---
> > >
> > > So my patch indeed did fix an actual problem. On the other hand it seems
> > > sn65dsi83_atomic_disable is not called in my case for some reason.
> >
> > So you remove the module and atomic_disable is not called, after
> > having called atomic_pre_enable?
>
> Yes, that's the case.
Ah, it's quite obvious looking at the code: removing the module will
call sn65dsi83_remove()
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/ti-sn65dsi83.c#L729
which does just call drm_bridge_remove()
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_bridge.c#L243
which just removes the bridge from the list.
So maybe sn65dsi83_remove() should call regulator_disable() as a last
resort, but I'm not sure this is the correct solution and it would
involve some housekeeping to not disable the regulator more times than
it has been enabled.
What is the use case you have for removing the driver module?
I'm not implying removing the modules is wrong, but it definitely looks
like not supported / not working. I'm just trying to understand the big
picture.
Best regards,
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH 1/1] drm/bridge: ti-sn65dsi83: Fix enable error path
On Thu, 29 Feb 2024 11:48:27 +0100
Frieder Schrempf wrote:
> On 29.02.24 10:47, Luca Ceresoli wrote:
> > Hello Alexander,
> >
> > On Wed, 28 Feb 2024 09:15:46 +0100
> > Alexander Stein wrote:
> >
> >
> > [...]
> >
> >> Oh I mistook this DSI-LVDS bridge with the DSI-DP bridge on a different
> >> board, my bad. I hope I can provide some insights. My platform is
> >> imx8mm-tqma8mqml-mba8mx-lvds-tm070jvhg33.dtb.
> >> I can easily cause a PLL lock failure by reducing the delay for the
> >> enable-gpios 'gpio_delays'. This will result in a PLL lock faiure.
> >> On my platform the vcc-supply counters do look sane:
> >>> /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> >>> /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:0
> >
> > Interesting. Thanks for taking time to report your initial issue!
> >
> >> Once I remove the ti_sn65dsi83 module, the open_count decrements to 0 as
> >> well. Looks sane to me.
> >>
> >> If I revert commit c81cd8f7c774 ("Revert "drm/bridge: ti-sn65dsi83:
> >> Fix enable error path""), vcc-supply counters are:
> >>> /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> >>> /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:1
> >>
> >> So in my case the use_count does not decrease! If I remove the module
> >> ti_sn65dsi83, I get the WARN_ON (enable_count is still non-zero):
> >>> WARNING: CPU: 2 PID: 402 at drivers/regulator/core.c:2398
> >>> _regulator_put+0x15c/0x164
> >>
> >> This is on 6.8.0-rc6-next-20240228 with the following diff applied:
> >> --->8---
> >> diff --git a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> >> b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> >> index 427467df42bf..8461e1fd396f 100644
> >> --- a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> >> +++ b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> >> @@ -285,7 +285,7 @@ {
> >> dsi_lvds_bridge: bridge@2d {
> >> compatible = "ti,sn65dsi84";
> >> reg = <0x2d>;
> >> - enable-gpios = <_delays 0 13 0>;
> >> + enable-gpios = <_delays 0 0 0>;
> >> vcc-supply = <_sn65dsi83_1v8>;
> >> status = "disabled";
> >>
> >> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> >> b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> >> index 4814b7b6d1fd..57a7ed13f996 100644
> >> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> >> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> >> @@ -478,7 +478,6 @@ static void sn65dsi83_atomic_pre_enable(struct
> >> drm_bridge *bridge,
> >> dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
> >> /* On failure, disable PLL again and exit. */
> >> regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> >> - regulator_disable(ctx->vcc);
> >> return;
> >> }
> >> --->8---
> >>
> >> So my patch indeed did fix an actual problem. On the other hand it seems
> >> sn65dsi83_atomic_disable is not called in my case for some reason.
> >
> > So you remove the module and atomic_disable is not called, after
> > having called atomic_pre_enable?
> >
> > I'm very possibly missing something, but this looks like a bug in the
> > DRM bridge code at first sight.
>
> I'm just guessing, but could it be that this patch [1] would fix it?
>
> It looks like nobody cared to pick this up, although there are several
> reports for defects caused by [2] and this patch is supposed to fix them.
It looks like [1] (or the other patches mentioned by Michael in the same
thread?) should be applied indeed. I'm going to test those patches
ASAP, perhaps next week.
However I'm not sure this is related to the problem being discussed
here: the patches about pre_enable_prev_first are touching
atomic_pre_enable and atomic_pre_disable. Here Alexander reported when
removing the bridge atomic_disable is not called, and atomic_disable
is not affected by pre_enable_prev_first.
> [1] https://patchwork.freedesktop.org/patch/529288/
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=4fb912e5e19075874379cfcf074d90bd51ebf8ea
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH 1/1] drm/bridge: ti-sn65dsi83: Fix enable error path
Hello Alexander,
On Wed, 28 Feb 2024 09:15:46 +0100
Alexander Stein wrote:
[...]
> Oh I mistook this DSI-LVDS bridge with the DSI-DP bridge on a different
> board, my bad. I hope I can provide some insights. My platform is
> imx8mm-tqma8mqml-mba8mx-lvds-tm070jvhg33.dtb.
> I can easily cause a PLL lock failure by reducing the delay for the
> enable-gpios 'gpio_delays'. This will result in a PLL lock faiure.
> On my platform the vcc-supply counters do look sane:
> > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:0
Interesting. Thanks for taking time to report your initial issue!
> Once I remove the ti_sn65dsi83 module, the open_count decrements to 0 as
> well. Looks sane to me.
>
> If I revert commit c81cd8f7c774 ("Revert "drm/bridge: ti-sn65dsi83:
> Fix enable error path""), vcc-supply counters are:
> > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
> > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:1
>
> So in my case the use_count does not decrease! If I remove the module
> ti_sn65dsi83, I get the WARN_ON (enable_count is still non-zero):
> > WARNING: CPU: 2 PID: 402 at drivers/regulator/core.c:2398
> > _regulator_put+0x15c/0x164
>
> This is on 6.8.0-rc6-next-20240228 with the following diff applied:
> --->8---
> diff --git a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> index 427467df42bf..8461e1fd396f 100644
> --- a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> +++ b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
> @@ -285,7 +285,7 @@ {
> dsi_lvds_bridge: bridge@2d {
> compatible = "ti,sn65dsi84";
> reg = <0x2d>;
> - enable-gpios = <_delays 0 13 0>;
> + enable-gpios = <_delays 0 0 0>;
> vcc-supply = <_sn65dsi83_1v8>;
> status = "disabled";
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 4814b7b6d1fd..57a7ed13f996 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -478,7 +478,6 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge
> *bridge,
> dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
> /* On failure, disable PLL again and exit. */
> regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> - regulator_disable(ctx->vcc);
> return;
> }
> --->8---
>
> So my patch indeed did fix an actual problem. On the other hand it seems
> sn65dsi83_atomic_disable is not called in my case for some reason.
So you remove the module and atomic_disable is not called, after
having called atomic_pre_enable?
I'm very possibly missing something, but this looks like a bug in the
DRM bridge code at first sight.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH 1/1] drm/bridge: ti-sn65dsi83: Fix enable error path
Hi Alexander,
thanks for your feedback!
On Tue, 27 Feb 2024 13:05:46 +0100
Alexander Stein wrote:
> Hi Luca,
>
> Am Donnerstag, 22. Februar 2024, 16:36:37 CET schrieb Luca Ceresoli:
> > Hello Alexander,
> >
> > On Thu, 4 May 2023 08:53:16 +0200
> > Alexander Stein wrote:
> >
> > > If PLL locking failed, the regulator needs to be disabled again.
> > >
> > > Fixes: 5664e3c907e2 ("drm/bridge: ti-sn65dsi83: Add vcc supply regulator
> > > support")
> > > Signed-off-by: Alexander Stein
> > > ---
> > > drivers/gpu/drm/bridge/ti-sn65dsi83.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > index 75286c9afbb9..1f5c07989e2b 100644
> > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > @@ -478,6 +478,7 @@ static void sn65dsi83_atomic_enable(struct drm_bridge
> > > *bridge,
> > > dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
> > > /* On failure, disable PLL again and exit. */
> > > regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> > > + regulator_disable(ctx->vcc);
> > > return;
> > > }
> >
> > I'm reviving this thread as I've been investigating a bug that appears
> > related to this patch.
> >
> > Symptom: with a v6.8-rc5 kernel, if PLL fails locking, later on during
> > atomic disable I get:
> >
> > [ 41.065198] [ cut here ]
> > [ 41.069823] unbalanced disables for DOCK_SYS_1V8
> > [ 41.074482] WARNING: CPU: 0 PID: 58 at drivers/regulator/core.c:2999
> > _regulator_disable+0xf8/0x1d8
> > [ 41.083457] Modules linked in: smsc smsc95xx usbnet mii imx_cpufreq_dt
> > exc3000 imx8mm_thermal snd_soc_tlv320aic3x_spi snd_soc_tlv320aic3x_i2c
> > snd_soc_tlv320aic3x tmp103 snd_soc_simple_card snd_soc_simple_card_utils
> > fsl_ldb rtc_snvs snvs_pwrkey snd_soc_fsl_sai imx8mp_interconnect
> > snd_soc_fsl_utils imx_interconnect imx_pcm_dma rtc_rs5c372 ti_sn65dsi83
> > pwm_imx27 st_pressure_spi st_sensors_spi st_pressure_i2c st_pressure
> > st_sensors_i2c industrialio_triggered_buffer lm75 kfifo_buf st_sensors
> > opt3001 panel_simple etnaviv gpu_sched iio_hwmon governor_userspace imx_bus
> > imx8mp_hdmi_tx dw_hdmi drm_display_helper samsung_dsim imx_sdma imx_lcdif
> > drm_dma_helper imx8mp_hdmi_pvi drm_kms_helper drm
> > drm_panel_orientation_quirks fsl_imx8_ddr_perf caam error sbs_battery
> > pwm_bl backlight ltc2497 ltc2497_core crct10dif_ce
> > [ 41.157281] CPU: 0 PID: 58 Comm: kworker/0:2 Not tainted 6.8.0-rc5+ #7
> > [ 41.170339] Workqueue: events drm_mode_rmfb_work_fn [drm]
> > [ 41.175798] pstate: 6005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS
> > BTYPE=--)
> > [ 41.182762] pc : _regulator_disable+0xf8/0x1d8
> > [ 41.187209] lr : _regulator_disable+0xf8/0x1d8
> > [ 41.191654] sp : 800081aaba90
> > [ 41.194967] x29: 800081aaba90 x28: x27:
> > 02647e80
> > [ 41.202109] x26: 02d7a180 x25: 037858a0 x24:
> > 800079748ac8
> > [ 41.209250] x23: 02647ed8 x22: 0263f800 x21:
> > 0373d000
> > [ 41.216392] x20: 0373d000 x19: 01de6480 x18:
> > 0006
> > [ 41.223533] x17: x16: 1fffe03423e1 x15:
> > 800081aab520
> > [ 41.230674] x14: x13: 3856315f5359535f x12:
> > 4b434f4420726f66
> > [ 41.237815] x11: 2073656c62617369 x10: 8000814647a0 x9 :
> > 8000801b10e0
> > [ 41.244957] x8 : 8000814bc7a0 x7 : 00017fe8 x6 :
> > 8000814bc7a0
> > [ 41.252098] x5 : x4 : x3 :
> >
> > [ 41.259239] x2 : x1 : x0 :
> > 011b6600
> > [ 41.266380] Call trace:
> > [ 41.268826] _regulator_disable+0xf8/0x1d8
> > [ 41.272925] regulator_disable+0x4c/0x98
> > [ 41.276850] sn65dsi83_atomic_disable+0x70/0xc0 [ti_sn65dsi83]
> > [ 41.282692] drm_atomic_bridge_chain_disable+0x78/0x110 [drm]
> > [ 41.288481] disable_outputs+0x100/0x350 [drm_kms_helper]
> > [ 41.293902] drm_atomic_helper_commit_tail_rpm+0x2c/0xb0 [drm_kms_helper]
> > [ 41.300705] commit_tail+0xac/0x1a0 [drm_kms_helper]
> > [ 41.305685] drm_atomic_helper_commit
Re: [PATCH 1/1] drm/bridge: ti-sn65dsi83: Fix enable error path
or is disable twice, leading to "unbalanced disables". I initially removed the regulator_disable() line in sn65dsi83_atomic_pre_enable() locally and it worked fine. Then I did some git log and found you added this line on purpose (even though it was in sn65dsi83_atomic_enable() initially), so my question is whether you can explain exactly what was wrong before your patch. I have been working for a few weeks with the regulator_disable() line removed and found no issue. Best regards, Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH V8 10/12] drm/bridge: imx: add bridge wrapper driver for i.MX8MP DWC HDMI
On Sat, 3 Feb 2024 10:52:50 -0600 Adam Ford wrote: > From: Lucas Stach > > Add a simple wrapper driver for the DWC HDMI bridge driver that > implements the few bits that are necessary to abstract the i.MX8MP > SoC integration. > > Signed-off-by: Lucas Stach > Reviewed-by: Laurent Pinchart > Tested-by: Marek Vasut > Tested-by: Adam Ford #imx8mp-beacon > Tested-by: Richard Leitner > Tested-by: Frieder Schrempf > Tested-by: Luca Ceresoli > Signed-off-by: Adam Ford Reviewed-by: Luca Ceresoli -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH V8 09/12] dt-bindings: display: imx: add binding for i.MX8MP HDMI TX
On Sat, 3 Feb 2024 10:52:49 -0600
Adam Ford wrote:
> From: Lucas Stach
>
> The HDMI TX controller on the i.MX8MP SoC is a Synopsys designware IP
> core with a little bit of SoC integration around it.
>
> Signed-off-by: Lucas Stach
> Signed-off-by: Adam Ford
[...]
> +examples:
> + - |
> +#include
> +#include
> +#include
> +
> +hdmi@32fd8000 {
> +compatible = "fsl,imx8mp-hdmi-tx";
> +reg = <0x32fd8000 0x7eff>;
> +interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +clocks = < IMX8MP_CLK_HDMI_APB>,
> + < IMX8MP_CLK_HDMI_REF_266M>,
> + < IMX8MP_CLK_32K>,
> + <_tx_phy>;
> +clock-names = "iahb", "isfr", "cec", "pix";
> +power-domains = <_blk_ctrl IMX8MP_HDMIBLK_PD_HDMI_TX>;
> +reg-io-width = <1>;
> +ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> +
> + hdmi_tx_from_pvi: endpoint {
> + remote-endpoint = <_to_hdmi_tx>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + hdmi_tx_out: endpoint {
Two excess indenting spaces on this line (port@0 is correct).
Also, I think there should an empty line between properties and nodes.
With these fixed:
Reviewed-by: Luca Ceresoli
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH V8 07/12] dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI
Hi Adam, On Sat, 3 Feb 2024 10:52:47 -0600 Adam Ford wrote: > From: Lucas Stach > > Add binding for the i.MX8MP HDMI parallel video interface block. > > Signed-off-by: Lucas Stach > Reviewed-by: Laurent Pinchart > Reviewed-by: Conor Dooley > Signed-off-by: Adam Ford Reviewed-by: Luca Ceresoli -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH V8 02/12] phy: freescale: add Samsung HDMI PHY
On Sat, 3 Feb 2024 10:52:42 -0600
Adam Ford wrote:
> From: Lucas Stach
>
> This adds the driver for the Samsung HDMI PHY found on the
> i.MX8MP SoC.
>
> Signed-off-by: Lucas Stach
> Signed-off-by: Adam Ford
> Tested-by: Alexander Stein
[...]
> +#define PHY_REG_33 0x84
> +#define REG33_MODE_SET_DONE BIT(7)
> +#define REG33_FIX_DABIT(1)
> +
> +#define PHY_REG_34 0x88
> +#define REG34_PHY_READY BIT(7)
> +#define REG34_PLL_LOCK BIT(6)
> +#define REG34_PHY_CLK_READY BIT(5)
> +
> +
Nitpick: only one empty line here.
> +#define PHY_PLL_REGS_NUM 48
[...]
> +static int phy_clk_register(struct fsl_samsung_hdmi_phy *phy)
> +{
> + struct device *dev = phy->dev;
> + struct device_node *np = dev->of_node;
> + struct clk_init_data init;
> + const char *parent_name;
> + struct clk *phyclk;
> + int ret;
> +
> + parent_name = __clk_get_name(phy->refclk);
> +
> + init.parent_names = _name;
> + init.num_parents = 1;
> + init.flags = 0;
> + init.name = "hdmi_pclk";
> + init.ops = _clk_ops;
> +
> + phy->hw.init =
> +
> + phyclk = devm_clk_register(dev, >hw);
> + if (IS_ERR(phyclk))
> + return dev_err_probe(dev, PTR_ERR(phyclk),
> + "failed to register clock\n");
> +
> + ret = of_clk_add_provider(np, of_clk_src_simple_get, phyclk);
Ouch:
> This function is *deprecated*. Use of_clk_add_hw_provider() instead.
Appears as an easy replacement though.
Otherwise looks good.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH V8 01/12] dt-bindings: phy: add binding for the i.MX8MP HDMI PHY
On Sat, 3 Feb 2024 10:52:41 -0600 Adam Ford wrote: > From: Lucas Stach > > Add a DT binding for the HDMI PHY found on the i.MX8MP SoC. > > Signed-off-by: Lucas Stach > Signed-off-by: Adam Ford > Reviewed-by: Krzysztof Kozlowski Reviewed-by: Luca Ceresoli -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH V7 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface
On Sat, 6 Jan 2024 15:51:45 -0600 Adam Ford wrote: > From: Lucas Stach > > This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a > full timing generator and can switch between different video sources. On > the i.MX8MP however the only supported source is the LCDIF. The block > just needs to be powered up and told about the polarity of the video > sync signals to act in bypass mode. > > Signed-off-by: Lucas Stach > Reviewed-by: Luca Ceresoli (v2) > Tested-by: Marek Vasut (v1) > Tested-by: Luca Ceresoli (v2) > Tested-by: Richard Leitner (v2) > Tested-by: Frieder Schrempf (v2) > Reviewed-by: Laurent Pinchart (v3) > Reviewed-by: Luca Ceresoli > Tested-by: Luca Ceresoli > Tested-by: Fabio Estevam > Signed-off-by: Adam Ford On v6.8-rc1, custom hardware based on the Avnet i.MX8MP SMARC SoM. Tested-by: Luca Ceresoli I think you can remove the Tested-by I sent for previous versions. Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH V2 2/2] phy: freescale: add Samsung HDMI PHY
Hello Adam, On Sat, 6 Jan 2024 16:19:05 -0600 Adam Ford wrote: > From: Lucas Stach > > This adds the driver for the Samsung HDMI PHY found on the > i.MX8MP SoC. > > Signed-off-by: Lucas Stach > Signed-off-by: Adam Ford I had already tested the v2 from Lucas, however I also tested this version which works as well, on v6.8-rc1, custom hardware based on the Avnet i.MX8MP SMARC SoM. Tested-by: Luca Ceresoli Generally speaking, as there are several small patch series around which together implement HDMI on iMX8MP and similar, I think it would be much easier fore reviewing and testing if they were grouped into a unique series. Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v3 14/24] of: property: add of_graph_get_next_endpoint()
Hello Kuninori Morimoto, On Wed, 31 Jan 2024 05:06:36 + Kuninori Morimoto wrote: > To handle endpoint more intuitive, create of_graph_get_next_endpoint() > > of_graph_get_next_endpoint(port1, NULL); // A1 > of_graph_get_next_endpoint(port1, A1); // A2 > of_graph_get_next_endpoint(port1, A2); // NULL The idea looks good. My only concern is about reusing the of_graph_get_next_endpoint() name after having removed the old, different function having the same name. This can be confusing in the first place to who is used to the old function, and also to anybody rebasing their patches on top of a new kernel to find their code behaving differently. Also, as now we'd have two similar variants of this function, it would be good if each of them were having a name that clearly identifies in which way they differ from the other. So a better name for this function would probably be of_graph_get_next_port_endpoint() I guess, to clearly differentiate from of_graph_get_next_device_endpoint(). Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface
Hi, On Mon, 18 Dec 2023 04:36:55 +0200 Laurent Pinchart wrote: > On Fri, Dec 15, 2023 at 05:09:41PM -0300, Fabio Estevam wrote: > > On Fri, Dec 15, 2023 at 4:01 PM Adam Ford wrote: > > > > > Thanks for the list. I was able to successfully build the stable 6.6 > > > branch with those patches applied and I have the HDMI working. > > > Unfortunately, I get build errors on the linux-next, so it's going to > > > take me a little time to sort through all of this. > > > > If you need help to figure this problem out, please let me know. > > > > I haven't tried it against linux-next. > > > > > I am thinking that it would be better to consolidate all these > > > together into one series instead of piecemealing it. However, there > > > are some items that can be submitted right away without significantly > > > reworking them against linux-next. Do people have a preference? > > > > I think it makes sense to re-submit the "easy ones" right away. > > Agreed. The more we can merge quickly, the easier it will become to > rebase and upstream the rest. I agree as well, "release early, release often". These patches are around since a long time and lots of people are using them already, and tracking all of them from different threads is time-consuming. I will happily test them early as soon as they are sent. Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH] dt-bindings: display: remove backlight node from panel examples
The examples for these panel drivers have a backlight node in addition to
the actual panel node. However the exact backlight is outside the scope of
this binding and should be dropped from the example.
Link:
https://lore.kernel.org/linux-devicetree/20230724143152.ga3430423-r...@kernel.org/
Suggested-by: Rob Herring
Signed-off-by: Luca Ceresoli
---
Documentation/devicetree/bindings/display/ilitek,ili9486.yaml | 4
Documentation/devicetree/bindings/display/panel/ilitek,ili9163.yaml | 4
Documentation/devicetree/bindings/display/sitronix,st7735r.yaml | 5 -
3 files changed, 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
b/Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
index 1f8f2182e2f1..9cc1fd0751cd 100644
--- a/Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
+++ b/Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
@@ -50,10 +50,6 @@ examples:
- |
#include
-backlight: backlight {
-compatible = "gpio-backlight";
-gpios = < 22 GPIO_ACTIVE_HIGH>;
-};
spi {
#address-cells = <1>;
#size-cells = <0>;
diff --git
a/Documentation/devicetree/bindings/display/panel/ilitek,ili9163.yaml
b/Documentation/devicetree/bindings/display/panel/ilitek,ili9163.yaml
index 90e323e19edb..3cabbba86581 100644
--- a/Documentation/devicetree/bindings/display/panel/ilitek,ili9163.yaml
+++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9163.yaml
@@ -48,10 +48,6 @@ examples:
- |
#include
-backlight: backlight {
-compatible = "gpio-backlight";
-gpios = < 22 GPIO_ACTIVE_HIGH>;
-};
spi {
#address-cells = <1>;
#size-cells = <0>;
diff --git a/Documentation/devicetree/bindings/display/sitronix,st7735r.yaml
b/Documentation/devicetree/bindings/display/sitronix,st7735r.yaml
index 621f27148419..3b0ebc0db8e0 100644
--- a/Documentation/devicetree/bindings/display/sitronix,st7735r.yaml
+++ b/Documentation/devicetree/bindings/display/sitronix,st7735r.yaml
@@ -54,11 +54,6 @@ examples:
- |
#include
-backlight: backlight {
-compatible = "gpio-backlight";
-gpios = < 44 GPIO_ACTIVE_HIGH>;
-};
-
spi {
#address-cells = <1>;
#size-cells = <0>;
---
base-commit: 2430fa6470d5e76be39a3e0d6d01474234582f94
change-id: 20231008-dt-panel-example-no-backlight-cfdaeb861b97
Best regards,
--
Luca Ceresoli
Re: [PATCH v4 1/2] dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI
Hi Lucas,
On Thu, 28 Sep 2023 14:55:35 +0200
Lucas Stach wrote:
> Add binding for the i.MX8MP HDMI parallel video interface block.
>
> Signed-off-by: Lucas Stach
> Reviewed-by: Laurent Pinchart
> ---
> .../display/imx/fsl,imx8mp-hdmi-pvi.yaml | 83 +++
> 1 file changed, 83 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> b/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> new file mode 100644
> index ..df29006b4090
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/imx/fsl,imx8mp-hdmi-pvi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale i.MX8MP HDMI Parallel Video Interface
> +
> +maintainers:
> + - Lucas Stach
> +
> +description: |
> + The HDMI parallel video interface is a timing and sync generator block in
> the
> + i.MX8MP SoC, that sits between the video source and the HDMI TX controller.
> +
> +properties:
> + compatible:
> +const: fsl,imx8mp-hdmi-pvi
> +
> + reg:
> +maxItems: 1
> +
> + interrupts:
> +maxItems: 1
> +
> + power-domains:
> +maxItems: 1
> +
> + ports:
> +$ref: /schemas/graph.yaml#/properties/ports
> +
> +properties:
> + port@0:
> +$ref: /schemas/graph.yaml#/properties/port
> +description: Input from the LCDIF controller.
> +
> + port@1:
> +$ref: /schemas/graph.yaml#/properties/port
> +description: Output to the HDMI TX controller.
> +
> +required:
> + - port@0
> + - port@1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
Sure it is required? In the imx8mp.dtsi I have, which comes for a patch
you sent previously, there is no 'interrupts' property, and HDMI works.
> + - power-domains
> + - ports
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +#include
> +#include
> +
> +display-bridge@32fc4000 {
> +compatible = "fsl,imx8mp-hdmi-pvi";
> +reg = <0x32fc4000 0x40>;
The device has up to register 0x40, thus I guess the second value should
be 0x44 here. Or maybe 0x100, just to be comfortable. :)
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v4 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface
Hi Lucas, On Thu, 28 Sep 2023 14:55:36 +0200 Lucas Stach wrote: > This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a > full timing generator and can switch between different video sources. On > the i.MX8MP however the only supported source is the LCDIF. The block > just needs to be powered up and told about the polarity of the video > sync signals to act in bypass mode. > > Signed-off-by: Lucas Stach > Reviewed-by: Luca Ceresoli (v2) I was in Cc on your v3 but not on this v4. Maybe the " (v2)" on these lines confuses get_maintainers.pl? > Tested-by: Marek Vasut (v1) > Tested-by: Luca Ceresoli (v2) > Tested-by: Richard Leitner (v2) > Tested-by: Frieder Schrempf (v2) > Reviewed-by: Laurent Pinchart (v3) A changelog would be appreciated, especially as a long time has gone since I last looked at these patches. Confirmed for this v4: Reviewed-by: Luca Ceresoli [On Avnet MSC SM2-MB-EP1 based on the SMARC module] Tested-by: Luca Ceresoli Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2 4/4] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface
Hi Lucas,
On Fri, 16 Dec 2022 22:07:42 +0100
Lucas Stach wrote:
> This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a
> full timing generator and can switch between different video sources. On
> the i.MX8MP however the only supported source is the LCDIF. The block
> just needs to be powered up and told about the polarity of the video
> sync signals to act in bypass mode.
>
> Signed-off-by: Lucas Stach
> Tested-by: Marek Vasut
> ---
> drivers/gpu/drm/bridge/imx/Kconfig | 7 +
> drivers/gpu/drm/bridge/imx/Makefile | 1 +
> drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 202 +++
> 3 files changed, 210 insertions(+)
> create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
>
> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig
> b/drivers/gpu/drm/bridge/imx/Kconfig
> index d828d8bfd893..e6cc4000bccd 100644
> --- a/drivers/gpu/drm/bridge/imx/Kconfig
> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> @@ -53,4 +53,11 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
> Choose this to enable support for the internal HDMI encoder found
> on the i.MX8MP SoC.
>
> +config DRM_IMX8MP_HDMI_PVI
> + tristate "i.MX8MP HDMI PVI bridge support"
> + depends on OF
> + help
> + Choose this to enable support for the internal HDMI TX Parallel
> + Video Interface found on the i.MX8MP SoC.
> +
> endif # ARCH_MXC || COMPILE_TEST
> diff --git a/drivers/gpu/drm/bridge/imx/Makefile
> b/drivers/gpu/drm/bridge/imx/Makefile
> index 03b0074ae538..b0fd56550dad 100644
> --- a/drivers/gpu/drm/bridge/imx/Makefile
> +++ b/drivers/gpu/drm/bridge/imx/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK) += imx8qxp-pixel-link.o
> obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK_TO_DPI) += imx8qxp-pxl2dpi.o
>
> obj-$(CONFIG_DRM_IMX8MP_DW_HDMI_BRIDGE) += imx8mp-hdmi.o
> +obj-$(CONFIG_DRM_IMX8MP_HDMI_PVI) += imx8mp-hdmi-pvi.o
> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
> b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
> new file mode 100644
> index ..30d40c21dabb
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
> @@ -0,0 +1,202 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright (C) 2022 Pengutronix, Lucas Stach
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define HTX_PVI_CTL 0x0
Personally I would s/CTL/CTRL/, to be consistent with the manual and
thus more search-friendly.
> +#define PVI_CTL_OP_VSYNC_POLBIT(18)
> +#define PVI_CTL_OP_HSYNC_POLBIT(17)
> +#define PVI_CTL_OP_DE_POL BIT(16)
> +#define PVI_CTL_INP_VSYNC_POL BIT(14)
> +#define PVI_CTL_INP_HSYNC_POL BIT(13)
> +#define PVI_CTL_INP_DE_POL BIT(12)
> +#define PVI_CTL_INPUT_LCDIF BIT(2)
According to the reference manual there is actually a 2-bit field here:
HTX_PVI_MODE, using bits 2:1, and whose "LCDIF" value is 0b10. Thus
while it obviously won't change the resulting code, it seems more
correct to define this as (2 << 1).
> +static void imx8mp_hdmi_pvi_bridge_enable(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state)
> +{
> + struct drm_atomic_state *state = bridge_state->base.state;
> + struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge);
> + struct drm_connector_state *conn_state;
> + const struct drm_display_mode *mode;
> + struct drm_crtc_state *crtc_state;
> + struct drm_connector *connector;
> + u32 bus_flags, val;
> +
> + connector = drm_atomic_get_new_connector_for_encoder(state,
> bridge->encoder);
> + conn_state = drm_atomic_get_new_connector_state(state, connector);
> + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
> +
> + if (WARN_ON(pm_runtime_resume_and_get(pvi->dev)))
> + return;
> +
> + mode = _state->adjusted_mode;
> +
> + val = PVI_CTL_INPUT_LCDIF;
> +
> + if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> + val |= PVI_CTL_OP_VSYNC_POL | PVI_CTL_INP_VSYNC_POL;
> +
> + if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> + val |= PVI_CTL_OP_HSYNC_POL | PVI_CTL_INP_HSYNC_POL;
> +
> + if (pvi->next_bridge->timings)
> + bus_flags = pvi->next_bridge->timings->input_bus_flags;
> + else if (bridge_state)
> + bus_flags = bridge_state->input_bus_cfg.flags;
> +
> + if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
> + val |= PVI_CTL_OP_DE_POL | PVI_CTL_INP_DE_POL;
> +
> + writel(val, pvi->regs + HTX_PVI_CTL);
> +
Re: [PATCH v2 2/4] drm/bridge: imx: add bridge wrapper driver for i.MX8MP DWC HDMI
Hi Lucas, On Fri, 16 Dec 2022 22:07:40 +0100 Lucas Stach wrote: > Add a simple wrapper driver for the DWC HDMI bridge driver that > implements the few bits that are necessary to abstract the i.MX8MP > SoC integration. > > Signed-off-by: Lucas Stach > Reviewed-by: Laurent Pinchart > Tested-by: Marek Vasut I realized I had sent my Tested-by to v1 when v2 was already out. So, in case you still need some encouragement for keeping on with this series: [Tested on a custom board using modetest on v6.5-rc6] Tested-by: Luca Ceresoli -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH v2] drm/panel: simple: Fix Innolux G156HCE-L01 LVDS clock
This panel has been implemented in commit 225213f24c79 ("drm/panel-simple:
Add Innolux G156HCE-L01 panel entry") with a higher clock than the typical
one mentioned on the documentation to avoid flickering on the unit
tested. Testing on a different unit shows that the panel actually works
with the intended 70.93 MHz clock and even lower frequencies so the
flickering is likely caused either by a defective unit or by other
different components such as the bridge.
Fixes: eae7488814b5 ("drm/panel-simple: Add Innolux G156HCE-L01 panel entry")
Signed-off-by: Luca Ceresoli
Tested-by: Marek Vasut # MX8MM with LT9211
Reviewed-by: Marek Vasut
---
Changed in v2:
- fix commit hash in "Fixes:" line
- add tested/reviewed by Marek
---
drivers/gpu/drm/panel/panel-simple.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 701013b3ad13..a1bbc7f11e83 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2379,7 +2379,7 @@ static const struct panel_desc innolux_g121x1_l03 = {
};
static const struct display_timing innolux_g156hce_l01_timings = {
- .pixelclock = { 12000, 14400, 15000 },
+ .pixelclock = { 12000, 14186, 15000 },
.hactive = { 1920, 1920, 1920 },
.hfront_porch = { 80, 90, 100 },
.hback_porch = { 80, 90, 100 },
--
2.34.1
Re: [PATCH] drm/panel: simple: Fix Innolux G156HCE-L01 LVDS clock
Hello Neil,
On Mon, 14 Aug 2023 14:38:51 +0200
Neil Armstrong wrote:
> On 04/08/2023 17:10, Luca Ceresoli wrote:
> > This panel has been implemented in commit 225213f24c79 ("drm/panel-simple:
> > Add Innolux G156HCE-L01 panel entry") with a higher clock than the typical
> > one mentioned on the documentation to avoid flickering on the unit
> > tested. Testing on a different unit shows that the panel actually works
> > with the intended 70.93 MHz clock and even lower frequencies so the
> > flickering is likely caused either by a defective unit or by other
> > different components such as the bridge.
> >
> > Fixes: 225213f24c79 ("drm/panel-simple: Add Innolux G156HCE-L01 panel
> > entry")
>
> This is the wrong sha, it should be eae7488814b5
Ah, good catch, Thank you!
v2 on its way.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[PATCH v2 2/3] dt-bindings: display: panel: Add panels based on ILITEK ILI9806E
Add bindings for LCD panels based on the ILITEK ILI9806E RGB controller
connected over SPI and the "ShenZhen New Display Co NDS040480800-V3"
480x800 panel based on it.
Signed-off-by: Luca Ceresoli
---
Changes in v2:
- remove T: line form MAINTAINERS entry
- reference spi-peripheral-props.yaml
- add 'maxItems: 1' to reg
- use unevaluatedProperties
- remove out of scope backlight node
---
.../display/panel/ilitek,ili9806e.yaml| 63 +++
MAINTAINERS | 5 ++
2 files changed, 68 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
diff --git
a/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
b/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
new file mode 100644
index ..83d177be4e7d
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/ilitek,ili9806e.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ilitek ILI9806E display panels
+
+maintainers:
+ - Luca Ceresoli
+
+description:
+ This binding is for display panels using an Ilitek ILI9806E controller in
+ SPI mode.
+
+allOf:
+ - $ref: panel-common.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+items:
+ - enum:
+ # ShenZhen New Display Co 3.97" 480x800 RGB a-SI TFT LCD
+ - newdisplay,nds040480800-v3
+ - const: ilitek,ili9806e
+
+ reg:
+maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - port
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+display@0 {
+compatible = "newdisplay,nds040480800-v3", "ilitek,ili9806e";
+reg = <0>;
+spi-max-frequency = <100>;
+pinctrl-names = "default";
+pinctrl-0 = <_lcdgpios>;
+reset-gpios = < 26 GPIO_ACTIVE_LOW>;
+backlight = <>;
+
+port {
+ili9806e_in: endpoint {
+remote-endpoint = <_out>;
+};
+};
+};
+};
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 0f966f05fb0d..085e44a7b5e1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6516,6 +6516,11 @@ T: git git://anongit.freedesktop.org/drm/drm-misc
F: Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
F: drivers/gpu/drm/tiny/ili9486.c
+DRM DRIVER FOR ILITEK ILI9806E PANELS
+M: Luca Ceresoli
+S: Maintained
+F: Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
+
DRM DRIVER FOR JADARD JD9365DA-H3 MIPI-DSI LCD PANELS
M: Jagan Teki
S: Maintained
--
2.34.1
[PATCH v2 3/3] DRM: panel: add Ilitek ILI9806E driver
Add a driver for the ILITEK ILI9806E 480x864 RGB LCD controller connected over SPI, and implement the ShenZhen New Display Co NDS040480800-V3 480x800 panel. Signed-off-by: Luca Ceresoli --- Changes in v2: - add ILI9806E_P1_DISCTRL1 bit description --- MAINTAINERS | 1 + drivers/gpu/drm/panel/Kconfig | 13 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-ilitek-ili9806e.c | 385 ++ 4 files changed, 400 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9806e.c diff --git a/MAINTAINERS b/MAINTAINERS index 085e44a7b5e1..602dc866c2e3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6520,6 +6520,7 @@ DRM DRIVER FOR ILITEK ILI9806E PANELS M: Luca Ceresoli S: Maintained F: Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml +F: drivers/gpu/drm/panel/panel-ilitek-ili9806e.c DRM DRIVER FOR JADARD JD9365DA-H3 MIPI-DSI LCD PANELS M: Jagan Teki diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 203c0ef0bbfd..e3e89d86668a 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -194,6 +194,19 @@ config DRM_PANEL_ILITEK_ILI9341 QVGA (240x320) RGB panels. support serial & parallel rgb interface. +config DRM_PANEL_ILITEK_ILI9806E + tristate "Ilitek ILI9806E panel" + depends on OF + depends on BACKLIGHT_CLASS_DEVICE + select VIDEOMODE_HELPERS + select DRM_MIPI_DBI + help + Say Y here if you want to enable support for LCD panels connected + over SPI and based on the Ilitek ILI9806E controller. + + The ILI9806E is an LCD controller capable of driving 18-bit a-Si + TFT LCDs up to a resolution of 480x800. + config DRM_PANEL_ILITEK_ILI9881C tristate "Ilitek ILI9881C-based panels" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 30cf553c8d1d..f465140ae7df 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d obj-$(CONFIG_DRM_PANEL_HIMAX_HX8394) += panel-himax-hx8394.o obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += panel-ilitek-ili9341.o +obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9806E) += panel-ilitek-ili9806e.o obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o obj-$(CONFIG_DRM_PANEL_INNOLUX_EJ030NA) += panel-innolux-ej030na.o obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c new file mode 100644 index ..ffc60d924e63 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c @@ -0,0 +1,385 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for the Ilitek ILI9806E a-Si TFT LCD controller. + * + * Copyright (c) 2023 Delcon SRL + * Luca Ceresoli + */ + +#include +#include +#include + +#include +#include +#include +#include +#include + +#include + +#define ILI9806E_BUS_FORMATMEDIA_BUS_FMT_RGB666_1X18 + +// Page 1 registers +#define ILI9806E_P1_IFMODE10x08// Interface Mode Control 1 +#define IFMODE1_SEPT_SDIO BIT(3) // 1 = two data pins +#define IFMODE1_SDO_STATUS BIT(4) // 0 = SDO has output enable +#define ILI9806E_P1_DISCTRL1 0x20// Display Function Control 1 +#define DISCTRL1_SYNC_MODE BIT(0) // RGB interface mode: 0 = DE mode, 1 = SYNC mode +#define ILI9806E_P1_DISCTRL2 0x21// Display Function Control 2 +#define DISCTRL2_EPL BIT(0) // DE polarity (1 = active high) +#define DISCTRL2_DPL BIT(1) // PCLK polarity (1 = fetch on falling edge) +#define DISCTRL2_HSPL BIT(2) // HS polarity (1 = active high) +#define DISCTRL2_VSPL BIT(3) // VS polarity (1 = active high) +#define ILI9806E_P1_RESCTRL0x30// Resolution Control +#define RESCTRL_480x8640x0 +#define RESCTRL_480x8540x1 +#define RESCTRL_480x8000x2 +#define RESCTRL_480x6400x3 +#define RESCTRL_480x7200x4 +#define ILI9806E_P1_INVTR 0x31// Display Inversion Control +#define INVTR_NLA_COLUMN 0x0 +#define INVTR_NLA_1DOT 0x1 +#define INVTR_NLA_2DOT 0x2 +#define INVTR_NLA_3DOT 0x3 +#define INVTR_NLA_4DOT 0x4 +#define ILI9806E_P1_PWCTRL10x40// Power Control 1 +#define ILI9806E_P1_PWCTRL20x41// Power Control 2 +#define ILI9806E_P1_PWCTRL30x42// Power Control 3 +#define ILI9806E_P1_PWCTRL40x43// Power Control 4 +#define ILI9806E_P1_PWCTRL50x44// Power Control 5 +#define ILI9806E_P1_PWCTRL60x
[PATCH v2 1/3] dt-bindings: vendor-prefixes: add ShenZhen New Display Co.
ShenZhen New Display Co., Limited is the manufacturer of the NDS040480800-V3 LCD panel according the datasheet. Signed-off-by: Luca Ceresoli --- Changes in v2: none --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index af60bf1a6664..f73d6d4eabbe 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -929,6 +929,8 @@ patternProperties: description: Netronix, Inc. "^netxeon,.*": description: Shenzhen Netxeon Technology CO., LTD + "^newdisplay,.*": +description: ShenZhen New Display Co., Limited "^neweast,.*": description: Guangdong Neweast Optoelectronics CO., LTD "^newhaven,.*": -- 2.34.1
[PATCH] drm/panel: simple: Fix AUO G121EAN01 panel timings according to the docs
Commit 03e909acd95a ("drm/panel: simple: Add support for AUO G121EAN01.4
panel") added support for this panel model, but the timings it implements
are very different from what the datasheet describes. I checked both the
G121EAN01.0 datasheet from [0] and the G121EAN01.4 one from [1] and they
all have the same timings: for example the LVDS clock typical value is 74.4
MHz, not 66.7 MHz as implemented.
Replace the timings with the ones from the documentation. These timings
have been tested and the clock frequencies verified with an oscilloscope to
ensure they are correct.
Also use struct display_timing instead of struct drm_display_mode in order
to also specify the minimum and maximum values.
[0] https://embedded.avnet.com/product/g121ean01-0/
[1] https://embedded.avnet.com/product/g121ean01-4/
Fixes: 03e909acd95a ("drm/panel: simple: Add support for AUO G121EAN01.4 panel")
Signed-off-by: Luca Ceresoli
---
drivers/gpu/drm/panel/panel-simple.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 701013b3ad13..56854f78441e 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -999,21 +999,21 @@ static const struct panel_desc auo_g104sn02 = {
.connector_type = DRM_MODE_CONNECTOR_LVDS,
};
-static const struct drm_display_mode auo_g121ean01_mode = {
- .clock = 66700,
- .hdisplay = 1280,
- .hsync_start = 1280 + 58,
- .hsync_end = 1280 + 58 + 8,
- .htotal = 1280 + 58 + 8 + 70,
- .vdisplay = 800,
- .vsync_start = 800 + 6,
- .vsync_end = 800 + 6 + 4,
- .vtotal = 800 + 6 + 4 + 10,
+static const struct display_timing auo_g121ean01_timing = {
+ .pixelclock = { 6000, 7440, 9000 },
+ .hactive = { 1280, 1280, 1280 },
+ .hfront_porch = { 20, 50, 100 },
+ .hback_porch = { 20, 50, 100 },
+ .hsync_len = { 30, 100, 200 },
+ .vactive = { 800, 800, 800 },
+ .vfront_porch = { 2, 10, 25 },
+ .vback_porch = { 2, 10, 25 },
+ .vsync_len = { 4, 18, 50 },
};
static const struct panel_desc auo_g121ean01 = {
- .modes = _g121ean01_mode,
- .num_modes = 1,
+ .timings = _g121ean01_timing,
+ .num_timings = 1,
.bpc = 8,
.size = {
.width = 261,
--
2.34.1
[PATCH] drm/panel: simple: Fix Innolux G156HCE-L01 LVDS clock
This panel has been implemented in commit 225213f24c79 ("drm/panel-simple:
Add Innolux G156HCE-L01 panel entry") with a higher clock than the typical
one mentioned on the documentation to avoid flickering on the unit
tested. Testing on a different unit shows that the panel actually works
with the intended 70.93 MHz clock and even lower frequencies so the
flickering is likely caused either by a defective unit or by other
different components such as the bridge.
Fixes: 225213f24c79 ("drm/panel-simple: Add Innolux G156HCE-L01 panel entry")
Signed-off-by: Luca Ceresoli
---
drivers/gpu/drm/panel/panel-simple.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 56854f78441e..ec3a73bbfe30 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2379,7 +2379,7 @@ static const struct panel_desc innolux_g121x1_l03 = {
};
static const struct display_timing innolux_g156hce_l01_timings = {
- .pixelclock = { 12000, 14400, 15000 },
+ .pixelclock = { 12000, 14186, 15000 },
.hactive = { 1920, 1920, 1920 },
.hfront_porch = { 80, 90, 100 },
.hback_porch = { 80, 90, 100 },
--
2.34.1
Re: [PATCH 2/2] drm/panel-simple: Add Innolux G156HCE-L01 panel entry
Hi Marek, Neil, On Fri, 4 Aug 2023 10:19:12 +0200 Luca Ceresoli wrote: > Hi Marek, > > On Thu, 3 Aug 2023 19:10:35 +0200 > Marek Vasut wrote: > > > On 8/3/23 17:06, Luca Ceresoli wrote: > > > Hi Marek, > > > > > > On Thu, 3 Aug 2023 16:25:37 +0200 > > > Marek Vasut wrote: > > > > > >> On 8/3/23 16:23, Luca Ceresoli wrote: > > >>> Hi Marek, > > >> > > >> Hi, > > >> > > >>> On Mon, 31 Jul 2023 23:02:58 +0200 > > >>> Marek Vasut wrote: > > >>> > > >>>> Add support for Innolux G156HCE-L01 15.6" 1920x1080 24bpp > > >>>> dual-link LVDS TFT panel. Documentation is available at [1]. > > >>> > > >>> Interesting, I'm bringing up this exact panel right now and found your > > >>> patch. > > >>> > > >>>> The middle frequency is tuned slightly upward from 70.93 MHz > > >>>> to 72 MHz, otherwise the panel shows slight flicker. > > >>> > > >>> Using 70.93 MHz here does not show any flickering. I even tried going > > >>> in the opposite direction and set 70 MHz, and to use different > > >>> backlight settings, all without any flickering. > > >>> > > >>> Do you think you might be using a defective device? Would you have a > > >>> chance of testing another sample? > > >> > > >> I have literally one such display. > > >> > > >> Which SoC do you use (and if applicable, which bridge) ? > > > > > > The panel is driven by the DSI-2 output of a i.MX8MP through a TI > > > SN65DSI84 bridge. > > > > I use the LT9211 , so I wonder whether this might be another Lontium > > specific oddity. > > Or maybe not. After checking the LVDS clock with an oscilloscope I > discovered I was actually sending 77 MHz. After fixing it I found that > my panel (of which I only have one sample as well) does not display any > output with pixel clocks <= 75 MHz. It works with clocks >= 76 MHz. After checking lots of other details in my video setup and doing cleanups, I finally managed to have this panel working with the intended 70.93 MHz clock (and also lower clocks such as 70.00 MHz). Is it too late to change this patch? Or should I send a patch on top? Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH 2/2] drm/panel-simple: Add Innolux G156HCE-L01 panel entry
Hi Marek, On Thu, 3 Aug 2023 19:10:35 +0200 Marek Vasut wrote: > On 8/3/23 17:06, Luca Ceresoli wrote: > > Hi Marek, > > > > On Thu, 3 Aug 2023 16:25:37 +0200 > > Marek Vasut wrote: > > > >> On 8/3/23 16:23, Luca Ceresoli wrote: > >>> Hi Marek, > >> > >> Hi, > >> > >>> On Mon, 31 Jul 2023 23:02:58 +0200 > >>> Marek Vasut wrote: > >>> > >>>> Add support for Innolux G156HCE-L01 15.6" 1920x1080 24bpp > >>>> dual-link LVDS TFT panel. Documentation is available at [1]. > >>> > >>> Interesting, I'm bringing up this exact panel right now and found your > >>> patch. > >>> > >>>> The middle frequency is tuned slightly upward from 70.93 MHz > >>>> to 72 MHz, otherwise the panel shows slight flicker. > >>> > >>> Using 70.93 MHz here does not show any flickering. I even tried going > >>> in the opposite direction and set 70 MHz, and to use different > >>> backlight settings, all without any flickering. > >>> > >>> Do you think you might be using a defective device? Would you have a > >>> chance of testing another sample? > >> > >> I have literally one such display. > >> > >> Which SoC do you use (and if applicable, which bridge) ? > > > > The panel is driven by the DSI-2 output of a i.MX8MP through a TI > > SN65DSI84 bridge. > > I use the LT9211 , so I wonder whether this might be another Lontium > specific oddity. Or maybe not. After checking the LVDS clock with an oscilloscope I discovered I was actually sending 77 MHz. After fixing it I found that my panel (of which I only have one sample as well) does not display any output with pixel clocks <= 75 MHz. It works with clocks >= 76 MHz. I'm trying to get more info about this. Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH 2/2] drm/panel-simple: Add Innolux G156HCE-L01 panel entry
Hi Marek, On Thu, 3 Aug 2023 16:25:37 +0200 Marek Vasut wrote: > On 8/3/23 16:23, Luca Ceresoli wrote: > > Hi Marek, > > Hi, > > > On Mon, 31 Jul 2023 23:02:58 +0200 > > Marek Vasut wrote: > > > >> Add support for Innolux G156HCE-L01 15.6" 1920x1080 24bpp > >> dual-link LVDS TFT panel. Documentation is available at [1]. > > > > Interesting, I'm bringing up this exact panel right now and found your > > patch. > > > >> The middle frequency is tuned slightly upward from 70.93 MHz > >> to 72 MHz, otherwise the panel shows slight flicker. > > > > Using 70.93 MHz here does not show any flickering. I even tried going > > in the opposite direction and set 70 MHz, and to use different > > backlight settings, all without any flickering. > > > > Do you think you might be using a defective device? Would you have a > > chance of testing another sample? > > I have literally one such display. > > Which SoC do you use (and if applicable, which bridge) ? The panel is driven by the DSI-2 output of a i.MX8MP through a TI SN65DSI84 bridge. Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH 2/2] drm/panel-simple: Add Innolux G156HCE-L01 panel entry
Hi Marek,
On Mon, 31 Jul 2023 23:02:58 +0200
Marek Vasut wrote:
> Add support for Innolux G156HCE-L01 15.6" 1920x1080 24bpp
> dual-link LVDS TFT panel. Documentation is available at [1].
Interesting, I'm bringing up this exact panel right now and found your
patch.
> The middle frequency is tuned slightly upward from 70.93 MHz
> to 72 MHz, otherwise the panel shows slight flicker.
Using 70.93 MHz here does not show any flickering. I even tried going
in the opposite direction and set 70 MHz, and to use different
backlight settings, all without any flickering.
Do you think you might be using a defective device? Would you have a
chance of testing another sample?
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 396a22177c674..417dd69054fc1 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -2377,6 +2377,37 @@ static const struct panel_desc innolux_g121x1_l03 = {
> },
> };
>
> +static const struct display_timing innolux_g156hce_l01_timings = {
> + .pixelclock = { 12000, 14400, 15000 },
In case the 144 MHz clock is kept, I'd add a comment here to clarify
the reason for diverging from the datasheet without the need for git
log.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH 2/3] dt-bindings: display: panel: Add panels based on ILITEK ILI9806E
Hello Rob,
thanks for reviewing.
On Wed, 19 Jul 2023 13:02:54 -0600
Rob Herring wrote:
> On Wed, Jul 19, 2023 at 05:21:46PM +0200, Luca Ceresoli wrote:
> > Add bindings for LCD panels based on the ILITEK ILI9806E RGB controller
> > connected over SPI and the "ShenZhen New Display Co NDS040480800-V3"
> > 480x800 panel based on it.
> >
> > Signed-off-by: Luca Ceresoli
> > ---
> > .../display/panel/ilitek,ili9806e.yaml| 69 +++
> > MAINTAINERS | 6 ++
> > 2 files changed, 75 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> > b/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> > new file mode 100644
> > index ..42abc6923065
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> > @@ -0,0 +1,69 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/panel/ilitek,ili9806e.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Ilitek ILI9806E display panels
> > +
> > +maintainers:
> > + - Luca Ceresoli
> > +
> > +description:
> > + This binding is for display panels using an Ilitek ILI9806E controller in
> > + SPI mode.
> > +
> > +allOf:
> > + - $ref: panel-common.yaml#
>
> A SPI device should reference spi-peripheral-props.yaml as well.
>
> > +
> > +properties:
> > + compatible:
> > +items:
> > + - enum:
> > + # ShenZhen New Display Co 3.97" 480x800 RGB a-SI TFT LCD
> > + - newdisplay,nds040480800-v3
> > + - const: ilitek,ili9806e
> > +
> > + reg: true
>
> maxItems: 1
>
> > + spi-max-frequency: true
> > + reset-gpios: true
> > + backlight: true
> > + port: true
>
> Drop all these and ...
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - port
> > +
> > +additionalProperties: false
>
> ... use "unevaluatedProperties" instead.
>
> > +
> > +examples:
> > + - |
> > +#include
> > +
> > +backlight: backlight {
> > +compatible = "gpio-backlight";
> > +gpios = < 22 GPIO_ACTIVE_HIGH>;
> > +};
>
> The exact backlight is outside the scope of this binding and should be
> dropped from the example.
As this comes from copy-pasting from the bindings yaml for another
panel, would it be useful if I send a patch to remove it?
Requested changes queued for v2.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH 2/3] dt-bindings: display: panel: Add panels based on ILITEK ILI9806E
Hello Krzysztof, thanks for reviewing. On Wed, 19 Jul 2023 21:12:14 +0200 Krzysztof Kozlowski wrote: > On 19/07/2023 17:21, Luca Ceresoli wrote: > > Add bindings for LCD panels based on the ILITEK ILI9806E RGB controller > > connected over SPI and the "ShenZhen New Display Co NDS040480800-V3" > > 480x800 panel based on it. > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index aee340630eca..3c38699ee821 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -6515,6 +6515,12 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > > F: Documentation/devicetree/bindings/display/ilitek,ili9486.yaml > > F: drivers/gpu/drm/tiny/ili9486.c > > > > +DRM DRIVER FOR ILITEK ILI9806E PANELS > > +M: Luca Ceresoli > > +S: Maintained > > +T: git git://anongit.freedesktop.org/drm/drm-misc > > Nope, same for recent one-driver-subsystem. It's like a second try... > You do not have git tree for one driver. The git tree is for subsystem, > not driver. I see, no problem, I'm removing the T: line in v2. As this came from copy-pasting another panel driver, should the T: line be removed from every "DRM DRIVER FOR * PANELS" entry? I could send a patch to do that, if that makes sense. Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH 2/3] dt-bindings: display: panel: Add panels based on ILITEK ILI9806E
Hello Conor,
thanks for reviewing.
On Wed, 19 Jul 2023 17:44:45 +0100
Conor Dooley wrote:
> Hey Luca,
>
> On Wed, Jul 19, 2023 at 05:21:46PM +0200, Luca Ceresoli wrote:
> > Add bindings for LCD panels based on the ILITEK ILI9806E RGB controller
> > connected over SPI and the "ShenZhen New Display Co NDS040480800-V3"
> > 480x800 panel based on it.
> >
> > Signed-off-by: Luca Ceresoli
> > ---
> > .../display/panel/ilitek,ili9806e.yaml| 69 +++
> > MAINTAINERS | 6 ++
> > 2 files changed, 75 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> > b/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> > new file mode 100644
> > index ..42abc6923065
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
> > @@ -0,0 +1,69 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/panel/ilitek,ili9806e.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Ilitek ILI9806E display panels
> > +
> > +maintainers:
> > + - Luca Ceresoli
> > +
> > +description:
> > + This binding is for display panels using an Ilitek ILI9806E controller in
> > + SPI mode.
>
> I figure you explicitly mention SPI mode here because it also supports
> D{P,S}I?
Exactly, DSI is supported as well by the chip.
> > +allOf:
> > + - $ref: panel-common.yaml#
> > +
> > +properties:
> > + compatible:
> > +items:
> > + - enum:
> > + # ShenZhen New Display Co 3.97" 480x800 RGB a-SI TFT LCD
> > + - newdisplay,nds040480800-v3
> > + - const: ilitek,ili9806e
> > +
> > + reg: true
> > + spi-max-frequency: true
> > + reset-gpios: true
> > + backlight: true
> > + port: true
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - port
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > +#include
> > +
> > + backlight: backlight {
> > +compatible = "gpio-backlight";
> > +gpios = < 22 GPIO_ACTIVE_HIGH>;
> > +};
> > +spi {
>
> Just a nit, a blank line between properties please. Clearly no
> respinning needed for that...
I agree. This comes from copy-paste from another bindings file so I'm
probably sending a separate series to fix it and avoid the same issue
in the future.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[PATCH 3/3] DRM: panel: add Ilitek ILI9806E driver
Add a driver for the ILITEK ILI9806E 480x864 RGB LCD controller connected over SPI, and implement the ShenZhen New Display Co NDS040480800-V3 480x800 panel. Signed-off-by: Luca Ceresoli --- MAINTAINERS | 1 + drivers/gpu/drm/panel/Kconfig | 13 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-ilitek-ili9806e.c | 384 ++ 4 files changed, 399 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9806e.c diff --git a/MAINTAINERS b/MAINTAINERS index 3c38699ee821..4d657a049acc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6520,6 +6520,7 @@ M:Luca Ceresoli S: Maintained T: git git://anongit.freedesktop.org/drm/drm-misc F: Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml +F: drivers/gpu/drm/panel/panel-ilitek-ili9806e.c DRM DRIVER FOR JADARD JD9365DA-H3 MIPI-DSI LCD PANELS M: Jagan Teki diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 203c0ef0bbfd..e3e89d86668a 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -194,6 +194,19 @@ config DRM_PANEL_ILITEK_ILI9341 QVGA (240x320) RGB panels. support serial & parallel rgb interface. +config DRM_PANEL_ILITEK_ILI9806E + tristate "Ilitek ILI9806E panel" + depends on OF + depends on BACKLIGHT_CLASS_DEVICE + select VIDEOMODE_HELPERS + select DRM_MIPI_DBI + help + Say Y here if you want to enable support for LCD panels connected + over SPI and based on the Ilitek ILI9806E controller. + + The ILI9806E is an LCD controller capable of driving 18-bit a-Si + TFT LCDs up to a resolution of 480x800. + config DRM_PANEL_ILITEK_ILI9881C tristate "Ilitek ILI9881C-based panels" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 30cf553c8d1d..f465140ae7df 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d obj-$(CONFIG_DRM_PANEL_HIMAX_HX8394) += panel-himax-hx8394.o obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += panel-ilitek-ili9341.o +obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9806E) += panel-ilitek-ili9806e.o obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o obj-$(CONFIG_DRM_PANEL_INNOLUX_EJ030NA) += panel-innolux-ej030na.o obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c new file mode 100644 index ..57c12bff70f8 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9806e.c @@ -0,0 +1,384 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for the Ilitek ILI9806E a-Si TFT LCD controller. + * + * Copyright (c) 2023 Delcon SRL + * Luca Ceresoli + */ + +#include +#include +#include + +#include +#include +#include +#include +#include + +#include + +#define ILI9806E_BUS_FORMATMEDIA_BUS_FMT_RGB666_1X18 + +// Page 1 registers +#define ILI9806E_P1_IFMODE10x08// Interface Mode Control 1 +#define IFMODE1_SEPT_SDIO BIT(3) // 1 = two data pins +#define IFMODE1_SDO_STATUS BIT(4) // 0 = SDO has output enable +#define ILI9806E_P1_DISCTRL1 0x20// Display Function Control 1 +#define ILI9806E_P1_DISCTRL2 0x21// Display Function Control 2 +#define DISCTRL2_EPL BIT(0) // DE polarity (1 = active high) +#define DISCTRL2_DPL BIT(1) // PCLK polarity (1 = fetch on falling edge) +#define DISCTRL2_HSPL BIT(2) // HS polarity (1 = active high) +#define DISCTRL2_VSPL BIT(3) // VS polarity (1 = active high) +#define ILI9806E_P1_RESCTRL0x30// Resolution Control +#define RESCTRL_480x8640x0 +#define RESCTRL_480x8540x1 +#define RESCTRL_480x8000x2 +#define RESCTRL_480x6400x3 +#define RESCTRL_480x7200x4 +#define ILI9806E_P1_INVTR 0x31// Display Inversion Control +#define INVTR_NLA_COLUMN 0x0 +#define INVTR_NLA_1DOT 0x1 +#define INVTR_NLA_2DOT 0x2 +#define INVTR_NLA_3DOT 0x3 +#define INVTR_NLA_4DOT 0x4 +#define ILI9806E_P1_PWCTRL10x40// Power Control 1 +#define ILI9806E_P1_PWCTRL20x41// Power Control 2 +#define ILI9806E_P1_PWCTRL30x42// Power Control 3 +#define ILI9806E_P1_PWCTRL40x43// Power Control 4 +#define ILI9806E_P1_PWCTRL50x44// Power Control 5 +#define ILI9806E_P1_PWCTRL60x45// Power Control 6 +#define ILI9806E_P1_PWCTRL70x46// Power Control 7 +#define ILI9806E_P1_PWCTRL80x47//
[PATCH 1/3] dt-bindings: vendor-prefixes: add ShenZhen New Display Co.
ShenZhen New Display Co., Limited is the manufacturer of the NDS040480800-V3 LCD panel according the datasheet. Signed-off-by: Luca Ceresoli --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index af60bf1a6664..f73d6d4eabbe 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -929,6 +929,8 @@ patternProperties: description: Netronix, Inc. "^netxeon,.*": description: Shenzhen Netxeon Technology CO., LTD + "^newdisplay,.*": +description: ShenZhen New Display Co., Limited "^neweast,.*": description: Guangdong Neweast Optoelectronics CO., LTD "^newhaven,.*": -- 2.34.1
[PATCH 2/3] dt-bindings: display: panel: Add panels based on ILITEK ILI9806E
Add bindings for LCD panels based on the ILITEK ILI9806E RGB controller
connected over SPI and the "ShenZhen New Display Co NDS040480800-V3"
480x800 panel based on it.
Signed-off-by: Luca Ceresoli
---
.../display/panel/ilitek,ili9806e.yaml| 69 +++
MAINTAINERS | 6 ++
2 files changed, 75 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
diff --git
a/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
b/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
new file mode 100644
index ..42abc6923065
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/ilitek,ili9806e.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ilitek ILI9806E display panels
+
+maintainers:
+ - Luca Ceresoli
+
+description:
+ This binding is for display panels using an Ilitek ILI9806E controller in
+ SPI mode.
+
+allOf:
+ - $ref: panel-common.yaml#
+
+properties:
+ compatible:
+items:
+ - enum:
+ # ShenZhen New Display Co 3.97" 480x800 RGB a-SI TFT LCD
+ - newdisplay,nds040480800-v3
+ - const: ilitek,ili9806e
+
+ reg: true
+ spi-max-frequency: true
+ reset-gpios: true
+ backlight: true
+ port: true
+
+required:
+ - compatible
+ - reg
+ - port
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+
+backlight: backlight {
+compatible = "gpio-backlight";
+gpios = < 22 GPIO_ACTIVE_HIGH>;
+};
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+display@0 {
+compatible = "newdisplay,nds040480800-v3", "ilitek,ili9806e";
+reg = <0>;
+spi-max-frequency = <100>;
+pinctrl-names = "default";
+pinctrl-0 = <_lcdgpios>;
+reset-gpios = < 26 GPIO_ACTIVE_LOW>;
+backlight = <>;
+
+port {
+ili9806e_in: endpoint {
+remote-endpoint = <_out>;
+};
+};
+};
+};
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index aee340630eca..3c38699ee821 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6515,6 +6515,12 @@ T: git git://anongit.freedesktop.org/drm/drm-misc
F: Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
F: drivers/gpu/drm/tiny/ili9486.c
+DRM DRIVER FOR ILITEK ILI9806E PANELS
+M: Luca Ceresoli
+S: Maintained
+T: git git://anongit.freedesktop.org/drm/drm-misc
+F: Documentation/devicetree/bindings/display/panel/ilitek,ili9806e.yaml
+
DRM DRIVER FOR JADARD JD9365DA-H3 MIPI-DSI LCD PANELS
M: Jagan Teki
S: Maintained
--
2.34.1
[PATCH] drm/panel: db7430: remove unused variables
These variables are never referenced in the code.
Signed-off-by: Luca Ceresoli
---
drivers/gpu/drm/panel/panel-samsung-db7430.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-samsung-db7430.c
b/drivers/gpu/drm/panel/panel-samsung-db7430.c
index 117b26845083..14c6700e37b3 100644
--- a/drivers/gpu/drm/panel/panel-samsung-db7430.c
+++ b/drivers/gpu/drm/panel/panel-samsung-db7430.c
@@ -56,10 +56,6 @@ struct db7430 {
struct mipi_dbi dbi;
/** @panel: the DRM panel instance for this device */
struct drm_panel panel;
- /** @width: the width of this panel in mm */
- u32 width;
- /** @height: the height of this panel in mm */
- u32 height;
/** @reset: reset GPIO line */
struct gpio_desc *reset;
/** @regulators: VCCIO and VIO supply regulators */
--
2.34.1
Re: [PATCH v5 14/20] staging: media: tegra-video: move MIPI calibration calls from VI to CSI
Hi Hans,
On Fri, 14 Apr 2023 17:51:34 +0200
Hans Verkuil wrote:
> Hi Luca,
>
> I just encountered an error in this patch, so I have rejected the PR I made.
>
> See below for the details:
>
> On 07/04/2023 15:38, Luca Ceresoli wrote:
> > The CSI module does not handle all the MIPI lane calibration procedure,
> > leaving a small part of it to the VI module. In doing this,
> > tegra_channel_enable_stream() (vi.c) manipulates the private data of the
> > upstream subdev casting it to struct 'tegra_csi_channel', which will be
> > wrong after introducing a VIP (parallel video input) channel.
> >
> > This prevents adding support for the VIP module. It also breaks the
> > logical isolation between modules.
> >
> > Since the lane calibration requirement does not exist in the parallel input
> > module, moving the calibration function to a per-module op is not
> > optimal. Instead move the calibration procedure in the CSI module, together
> > with the rest of the calibration procedures. After this change,
> > tegra_channel_enable_stream() just calls v4l2_subdev_call() to ask for a
> > stream start/stop to the CSI module, which in turn knows all the
> > CSI-specific details to implement it.
> >
> > Signed-off-by: Luca Ceresoli
> > Reviewed-by: Dmitry Osipenko
> >
> > ---
> >
> > No changes in v5
> >
> > Changed in v4:
> > - Added review tags
> >
> > No changes in v3
> > No changes in v2
> > ---
> > drivers/staging/media/tegra-video/csi.c | 44
> > drivers/staging/media/tegra-video/vi.c | 54 ++---
> > 2 files changed, 48 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/staging/media/tegra-video/csi.c
> > b/drivers/staging/media/tegra-video/csi.c
> > index 9a03d5ccdf3c..b93fc879ef3a 100644
> > --- a/drivers/staging/media/tegra-video/csi.c
> > +++ b/drivers/staging/media/tegra-video/csi.c
> > @@ -328,12 +328,42 @@ static int tegra_csi_enable_stream(struct v4l2_subdev
> > *subdev)
> > }
> >
> > csi_chan->pg_mode = chan->pg_mode;
> > +
> > + /*
> > +* Tegra CSI receiver can detect the first LP to HS transition.
> > +* So, start the CSI stream-on prior to sensor stream-on and
> > +* vice-versa for stream-off.
> > +*/
> > ret = csi->ops->csi_start_streaming(csi_chan);
> > if (ret < 0)
> > goto finish_calibration;
> >
> > + if (csi_chan->mipi) {
> > + struct v4l2_subdev *src_subdev;
> > + /*
> > +* TRM has incorrectly documented to wait for done status from
> > +* calibration logic after CSI interface power on.
> > +* As per the design, calibration results are latched and
> > applied
> > +* to the pads only when the link is in LP11 state which will
> > happen
> > +* during the sensor stream-on.
> > +* CSI subdev stream-on triggers start of MIPI pads calibration.
> > +* Wait for calibration to finish here after sensor subdev
> > stream-on.
> > +*/
> > + src_subdev = tegra_channel_get_remote_source_subdev(chan);
> > + ret = v4l2_subdev_call(src_subdev, video, s_stream, true);
> > + err = tegra_mipi_finish_calibration(csi_chan->mipi);
> > +
> > + if (ret < 0 && ret != -ENOIOCTLCMD)
> > + goto disable_csi_stream;
>
> If there was an error from s_stream, then tegra_mipi_finish_calibration is
> called
> and it goes to disable_csi_stream...
>
> > +
> > + if (err < 0)
> > + dev_warn(csi->dev, "MIPI calibration failed: %d\n",
> > err);
> > + }
> > +
> > return 0;
> >
> > +disable_csi_stream:
> > + csi->ops->csi_stop_streaming(csi_chan);
> > finish_calibration:
> > if (csi_chan->mipi)
> > tegra_mipi_finish_calibration(csi_chan->mipi);
>
> ...but here tegra_mipi_finish_calibration() is called again, leading to an
> unlock
> imbalance.
Many thanks for your testing! Unfortunately I have no Tegra210 hardware
so this never happened here, but with your report the problem got
obvious and, luckily, the fix appeared to be just a oneliner.
v6 just sent! I'm wondering whether there is still hope to get this
6.4...
Best regards,
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[PATCH v6 20/20] staging: media: tegra-video: add support for Tegra20 parallel input
The VI peripheral of Tegra supports capturing from MIPI CSI-2 or parallel video (called VIP in the docs). The staging tegra-video driver currently implements MIPI CSI-2 video capture for Tegra210. Add support for parallel video capture (VIP) on Tegra20. With the generalizations added to the VI driver in previous commits, this is only a matter of adding the vip.c and tegra20.c implementations and registering them. Unfortunately there was no documentation available for the VI or VIP peripherals of Tegra20 (or any other Tegra chips). This implementation has been based entirely on the code from a vendor kernel based on Linux 3.1 and massively adapted to fit into the tegra-video driver. Parts of this code is definitely non-optimal to say the least (especially tegra20_vi_enable() and the single-frame capture logic), but it was impossible to improve it. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v6 Changed in v5: - Fixed small typo - Upgraded copyright year Changed in v4: - Added review tags Changed in v3 (suggested by Dmitry Osipenko): - merged the VIP patch and the Tegra20 patch to avoid chicken-egg problem due to the two modules depending on each other at build time - move tegra20_vip_soc to vip.c - remove channel@0 node from device tree parsing - remove unused variable Changed in v2: - fix tegra20_vi_enable() to clear bit when on==false - clamp width/height from set/try_fmt to avoid returning sizeimage=0 (fixes v4l2-compliance) --- drivers/staging/media/tegra-video/Makefile | 2 + drivers/staging/media/tegra-video/tegra20.c | 661 drivers/staging/media/tegra-video/vi.c | 3 + drivers/staging/media/tegra-video/vi.h | 3 + drivers/staging/media/tegra-video/video.c | 5 + drivers/staging/media/tegra-video/video.h | 1 + drivers/staging/media/tegra-video/vip.c | 290 + drivers/staging/media/tegra-video/vip.h | 68 ++ 8 files changed, 1033 insertions(+) create mode 100644 drivers/staging/media/tegra-video/tegra20.c create mode 100644 drivers/staging/media/tegra-video/vip.c create mode 100644 drivers/staging/media/tegra-video/vip.h diff --git a/drivers/staging/media/tegra-video/Makefile b/drivers/staging/media/tegra-video/Makefile index dfa2ef8f99ef..6c7552e05109 100644 --- a/drivers/staging/media/tegra-video/Makefile +++ b/drivers/staging/media/tegra-video/Makefile @@ -2,7 +2,9 @@ tegra-video-objs := \ video.o \ vi.o \ + vip.o \ csi.o +tegra-video-$(CONFIG_ARCH_TEGRA_2x_SOC) += tegra20.o tegra-video-$(CONFIG_ARCH_TEGRA_210_SOC) += tegra210.o obj-$(CONFIG_VIDEO_TEGRA) += tegra-video.o diff --git a/drivers/staging/media/tegra-video/tegra20.c b/drivers/staging/media/tegra-video/tegra20.c new file mode 100644 index ..c25286772603 --- /dev/null +++ b/drivers/staging/media/tegra-video/tegra20.c @@ -0,0 +1,661 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Tegra20-specific VI implementation + * + * Copyright (C) 2023 SKIDATA GmbH + * Author: Luca Ceresoli + */ + +/* + * This source file contains Tegra20 supported video formats, + * VI and VIP SoC specific data, operations and registers accessors. + */ + +#include +#include +#include +#include +#include +#include + +#include "vip.h" +#include "vi.h" + +#define TEGRA_VI_SYNCPT_WAIT_TIMEOUT msecs_to_jiffies(200) + +/* This are just good-sense numbers. The actual min/max is not documented. */ +#define TEGRA20_MIN_WIDTH 32U +#define TEGRA20_MIN_HEIGHT 32U +#define TEGRA20_MAX_WIDTH 2048U +#define TEGRA20_MAX_HEIGHT 2048U + +/* -- + * Registers + */ + +#define TEGRA_VI_CONT_SYNCPT_OUT_1 0x0060 +#define VI_CONT_SYNCPT_OUT_1_CONTINUOUS_SYNCPT BIT(8) +#define VI_CONT_SYNCPT_OUT_1_SYNCPT_IDX_SFT 0 + +#define TEGRA_VI_VI_INPUT_CONTROL 0x0088 +#define VI_INPUT_FIELD_DETECTBIT(27) +#define VI_INPUT_BT656 BIT(25) +#define VI_INPUT_YUV_INPUT_FORMAT_SFT8 /* bits [9:8] */ +#define VI_INPUT_YUV_INPUT_FORMAT_UYVY (0 << VI_INPUT_YUV_INPUT_FORMAT_SFT) +#define VI_INPUT_YUV_INPUT_FORMAT_VYUY (1 << VI_INPUT_YUV_INPUT_FORMAT_SFT) +#define VI_INPUT_YUV_INPUT_FORMAT_YUYV (2 << VI_INPUT_YUV_INPUT_FORMAT_SFT) +#define VI_INPUT_YUV_INPUT_FORMAT_YVYU (3 << VI_INPUT_YUV_INPUT_FORMAT_SFT) +#define VI_INPUT_INPUT_FORMAT_SFT2 /* bits [5:2] */ +#define VI_INPUT_INPUT_FORMAT_YUV422 (0 << VI_INPUT_INPUT_FORMAT_SFT) +#define VI_INPUT_VIP_INPUT_ENABLEBIT(1) + +#define TEGRA_VI_VI_CORE_CONTROL 0x008c +#define VI_VI_CORE_CONT
[PATCH v6 18/20] staging: media: tegra-video: add hooks for planar YUV and H/V flip
Tegra20 supports planar YUV422 capture, which can be implemented by writing
U and V base address registers in addition to the "main" base buffer
address register.
It also supports H and V flip, which among others requires to write the
start address (i.e. the 1st offset to write, at the end of the buffer or
line) in more registers for Y and, for planar formats, U and V.
Add minimal hooks in VI to allow per-SoC optional support to those
features:
- variables in struct tegra_vi for the U and V buffer base offsets
- variables in struct tegra_vi for the Y, U and V buffer start offsets
- an optional per-soc VI operation to compute those values on queue setup
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/vi.c | 4
drivers/staging/media/tegra-video/vi.h | 14 ++
2 files changed, 18 insertions(+)
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index b74cdd1d1c82..5ab24977ec46 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -102,6 +102,7 @@ tegra_get_format_by_fourcc(struct tegra_vi *vi, u32 fourcc)
/*
* videobuf2 queue operations
*/
+
static int tegra_channel_queue_setup(struct vb2_queue *vq,
unsigned int *nbuffers,
unsigned int *nplanes,
@@ -117,6 +118,9 @@ static int tegra_channel_queue_setup(struct vb2_queue *vq,
sizes[0] = chan->format.sizeimage;
alloc_devs[0] = chan->vi->dev;
+ if (chan->vi->ops->channel_queue_setup)
+ chan->vi->ops->channel_queue_setup(chan);
+
return 0;
}
diff --git a/drivers/staging/media/tegra-video/vi.h
b/drivers/staging/media/tegra-video/vi.h
index 02f940f2e2eb..cadf80b742a8 100644
--- a/drivers/staging/media/tegra-video/vi.h
+++ b/drivers/staging/media/tegra-video/vi.h
@@ -47,6 +47,7 @@ struct tegra_vi_channel;
* @channel_host1x_syncpt_free: free all synchronization points
* @vi_fmt_align: modify `pix` to fit the hardware alignment
* requirements and fill image geometry
+ * @channel_queue_setup: additional operations at the end of
vb2_ops::queue_setup
* @vi_start_streaming: starts media pipeline, subdevice streaming, sets up
* VI for capture and runs capture start and capture finish
* kthreads for capturing frames to buffer and returns them back.
@@ -58,6 +59,7 @@ struct tegra_vi_ops {
int (*channel_host1x_syncpt_init)(struct tegra_vi_channel *chan);
void (*channel_host1x_syncpt_free)(struct tegra_vi_channel *chan);
void (*vi_fmt_align)(struct v4l2_pix_format *pix, unsigned int bpp);
+ void (*channel_queue_setup)(struct tegra_vi_channel *chan);
int (*vi_start_streaming)(struct vb2_queue *vq, u32 count);
void (*vi_stop_streaming)(struct vb2_queue *vq);
};
@@ -148,6 +150,12 @@ struct tegra_vi {
* @queue: vb2 buffers queue
* @sequence: V4L2 buffers sequence number
*
+ * @addr_offset_u: U plane base address, relative to buffer base address (only
for planar)
+ * @addr_offset_v: V plane base address, relative to buffer base address (only
for planar)
+ * @start_offset: 1st Y byte to write, relative to buffer base address (for
H/V flip)
+ * @start_offset_u: 1st U byte to write, relative to buffer base address (for
H/V flip)
+ * @start_offset_v: 1st V byte to write, relative to buffer base address (for
H/V flip)
+ *
* @capture: list of queued buffers for capture
* @start_lock: protects the capture queued list
* @done: list of capture done queued buffers
@@ -189,6 +197,12 @@ struct tegra_vi_channel {
struct vb2_queue queue;
u32 sequence;
+ unsigned int addr_offset_u;
+ unsigned int addr_offset_v;
+ unsigned int start_offset;
+ unsigned int start_offset_u;
+ unsigned int start_offset_v;
+
struct list_head capture;
/* protects the capture queued list */
spinlock_t start_lock;
--
2.34.1
[PATCH v6 19/20] staging: media: tegra-video: add H/V flip controls
Tegra20 can do horizontal and vertical image flip, but Tegra210 cannot
(either the hardware, or this driver).
In preparation to adding Tegra20 support, add a flag in struct tegra_vi_soc
so the generic vi.c code knows whether the flip controls should be added or
not.
Also provide a generic implementation that simply sets two flags in the
channel struct. The Tegra20 implementation will enable flipping at stream
start based on those flags.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
Changed in v5:
- Fixed typo in comment
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/vi.c | 14 +-
drivers/staging/media/tegra-video/vi.h | 8
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index 5ab24977ec46..39e9df895ede 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -30,7 +30,7 @@
#include "vi.h"
#include "video.h"
-#define MAX_CID_CONTROLS 1
+#define MAX_CID_CONTROLS 3
/**
* struct tegra_vi_graph_entity - Entity in the video graph
@@ -910,6 +910,12 @@ static int vi_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_TEGRA_SYNCPT_TIMEOUT_RETRY:
chan->syncpt_timeout_retry = ctrl->val;
break;
+ case V4L2_CID_HFLIP:
+ chan->hflip = ctrl->val;
+ break;
+ case V4L2_CID_VFLIP:
+ chan->vflip = ctrl->val;
+ break;
default:
return -EINVAL;
}
@@ -981,6 +987,12 @@ static int tegra_channel_setup_ctrl_handler(struct
tegra_vi_channel *chan)
v4l2_ctrl_handler_free(>ctrl_handler);
return ret;
}
+
+ if (chan->vi->soc->has_h_v_flip) {
+ v4l2_ctrl_new_std(>ctrl_handler, _ctrl_ops,
V4L2_CID_HFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(>ctrl_handler, _ctrl_ops,
V4L2_CID_VFLIP, 0, 1, 1, 0);
+ }
+
#endif
/* setup the controls */
diff --git a/drivers/staging/media/tegra-video/vi.h
b/drivers/staging/media/tegra-video/vi.h
index cadf80b742a8..778c0ec475ab 100644
--- a/drivers/staging/media/tegra-video/vi.h
+++ b/drivers/staging/media/tegra-video/vi.h
@@ -74,6 +74,7 @@ struct tegra_vi_ops {
* @hw_revision: VI hw_revision
* @vi_max_channels: supported max streaming channels
* @vi_max_clk_hz: VI clock max frequency
+ * @has_h_v_flip: the chip can do H and V flip, and the driver implements it
*/
struct tegra_vi_soc {
const struct tegra_video_format *video_formats;
@@ -83,6 +84,7 @@ struct tegra_vi_soc {
u32 hw_revision;
unsigned int vi_max_channels;
unsigned int vi_max_clk_hz;
+ bool has_h_v_flip:1;
};
/**
@@ -172,6 +174,9 @@ struct tegra_vi {
* @tpg_fmts_bitmap: a bitmap for supported TPG formats
* @pg_mode: test pattern generator mode (disabled/direct/patch)
* @notifier: V4L2 asynchronous subdevs notifier
+ *
+ * @hflip: Horizontal flip is enabled
+ * @vflip: Vertical flip is enabled
*/
struct tegra_vi_channel {
struct list_head list;
@@ -222,6 +227,9 @@ struct tegra_vi_channel {
enum tegra_vi_pg_mode pg_mode;
struct v4l2_async_notifier notifier;
+
+ bool hflip:1;
+ bool vflip:1;
};
/**
--
2.34.1
[PATCH v6 17/20] staging: media: tegra-video: add syncpts for Tegra20 to struct tegra_vi
In preparation to implement Tegra20 parallel video capture, add a variable
to hold the required syncpt and document all the syncpt variables.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
No changes in v5
Changed in v4:
- Added review tags
Changed in v3:
- recycle mw_ack_sp[0] instead of adding out_sp
No changes in v2
---
drivers/staging/media/tegra-video/vi.h | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/tegra-video/vi.h
b/drivers/staging/media/tegra-video/vi.h
index dfb5870b1411..02f940f2e2eb 100644
--- a/drivers/staging/media/tegra-video/vi.h
+++ b/drivers/staging/media/tegra-video/vi.h
@@ -117,11 +117,13 @@ struct tegra_vi {
* @vi: Tegra video input device structure
* @frame_start_sp: host1x syncpoint pointer to synchronize programmed capture
* start condition with hardware frame start events through host1x
- * syncpoint counters.
+ * syncpoint counters. (Tegra210)
* @mw_ack_sp: host1x syncpoint pointer to synchronize programmed memory write
* ack trigger condition with hardware memory write done at end of
- * frame through host1x syncpoint counters.
+ * frame through host1x syncpoint counters (On Tegra20 used for the
+ * OUT_1 syncpt)
* @sp_incr_lock: protects cpu syncpoint increment.
+ * @next_out_sp_idx: next expected value for mw_ack_sp[0], i.e. OUT_1 (Tegra20)
*
* @kthread_start_capture: kthread to start capture of single frame when
* vb buffer is available. This thread programs VI CSI hardware
@@ -175,6 +177,7 @@ struct tegra_vi_channel {
struct host1x_syncpt *mw_ack_sp[GANG_PORTS_MAX];
/* protects the cpu syncpoint increment */
spinlock_t sp_incr_lock[GANG_PORTS_MAX];
+ u32 next_out_sp_idx;
struct task_struct *kthread_start_capture;
wait_queue_head_t start_wait;
--
2.34.1
[PATCH v6 16/20] staging: media: tegra-video: move syncpt init/free to a per-soc op
tegra_channel_host1x_syncpt_init() gets the host1x syncpts needed for the
Tegra210 implementation, and tegra_channel_host1x_syncpts_free() puts
them.
Tegra20 needs to get and put a different syncpt. In preparation for adding
Tegra20 support, move these functions to new ops in the soc-specific
`struct tegra_vi_ops` .
No functional changes.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/tegra210.c | 52
drivers/staging/media/tegra-video/vi.c | 52 ++--
drivers/staging/media/tegra-video/vi.h | 5 ++
3 files changed, 60 insertions(+), 49 deletions(-)
diff --git a/drivers/staging/media/tegra-video/tegra210.c
b/drivers/staging/media/tegra-video/tegra210.c
index b4fcd4e93b8c..da99f19a39e7 100644
--- a/drivers/staging/media/tegra-video/tegra210.c
+++ b/drivers/staging/media/tegra-video/tegra210.c
@@ -179,6 +179,56 @@ static u32 vi_csi_read(struct tegra_vi_channel *chan, u8
portno,
/*
* Tegra210 VI channel capture operations
*/
+
+static int tegra210_channel_host1x_syncpt_init(struct tegra_vi_channel *chan)
+{
+ struct tegra_vi *vi = chan->vi;
+ unsigned long flags = HOST1X_SYNCPT_CLIENT_MANAGED;
+ struct host1x_syncpt *fs_sp;
+ struct host1x_syncpt *mw_sp;
+ int ret, i;
+
+ for (i = 0; i < chan->numgangports; i++) {
+ fs_sp = host1x_syncpt_request(>client, flags);
+ if (!fs_sp) {
+ dev_err(vi->dev, "failed to request frame start
syncpoint\n");
+ ret = -ENOMEM;
+ goto free_syncpts;
+ }
+
+ mw_sp = host1x_syncpt_request(>client, flags);
+ if (!mw_sp) {
+ dev_err(vi->dev, "failed to request memory ack
syncpoint\n");
+ host1x_syncpt_put(fs_sp);
+ ret = -ENOMEM;
+ goto free_syncpts;
+ }
+
+ chan->frame_start_sp[i] = fs_sp;
+ chan->mw_ack_sp[i] = mw_sp;
+ spin_lock_init(>sp_incr_lock[i]);
+ }
+
+ return 0;
+
+free_syncpts:
+ for (i = 0; i < chan->numgangports; i++) {
+ host1x_syncpt_put(chan->mw_ack_sp[i]);
+ host1x_syncpt_put(chan->frame_start_sp[i]);
+ }
+ return ret;
+}
+
+static void tegra210_channel_host1x_syncpt_free(struct tegra_vi_channel *chan)
+{
+ int i;
+
+ for (i = 0; i < chan->numgangports; i++) {
+ host1x_syncpt_put(chan->mw_ack_sp[i]);
+ host1x_syncpt_put(chan->frame_start_sp[i]);
+ }
+}
+
static void tegra210_fmt_align(struct v4l2_pix_format *pix, unsigned int bpp)
{
unsigned int min_bpl;
@@ -753,6 +803,8 @@ static const struct tegra_video_format
tegra210_video_formats[] = {
/* Tegra210 VI operations */
static const struct tegra_vi_ops tegra210_vi_ops = {
+ .channel_host1x_syncpt_init = tegra210_channel_host1x_syncpt_init,
+ .channel_host1x_syncpt_free = tegra210_channel_host1x_syncpt_free,
.vi_fmt_align = tegra210_fmt_align,
.vi_start_streaming = tegra210_vi_start_streaming,
.vi_stop_streaming = tegra210_vi_stop_streaming,
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index 8df233049c81..b74cdd1d1c82 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -1062,21 +1062,11 @@ static int vi_fmts_bitmap_init(struct tegra_vi_channel
*chan)
return 0;
}
-static void tegra_channel_host1x_syncpts_free(struct tegra_vi_channel *chan)
-{
- int i;
-
- for (i = 0; i < chan->numgangports; i++) {
- host1x_syncpt_put(chan->mw_ack_sp[i]);
- host1x_syncpt_put(chan->frame_start_sp[i]);
- }
-}
-
static void tegra_channel_cleanup(struct tegra_vi_channel *chan)
{
v4l2_ctrl_handler_free(>ctrl_handler);
media_entity_cleanup(>video.entity);
- tegra_channel_host1x_syncpts_free(chan);
+ chan->vi->ops->channel_host1x_syncpt_free(chan);
mutex_destroy(>video_lock);
}
@@ -1094,42 +1084,6 @@ void tegra_channels_cleanup(struct tegra_vi *vi)
}
}
-static int tegra_channel_host1x_syncpt_init(struct tegra_vi_channel *chan)
-{
- struct tegra_vi *vi = chan->vi;
- unsigned long flags = HOST1X_SYNCPT_CLIENT_MANAGED;
- struct host1x_syncpt *fs_sp;
- struct host1x_syncpt *mw_sp;
- int ret, i;
-
- for (i = 0; i < chan->numgangports; i++) {
- fs_sp = host1x_syncpt_request(>client, flags);
- if (!fs_sp) {
- dev_err(vi->dev, "fa
[PATCH v6 15/20] staging: media: tegra-video: add a per-soc enable/disable op
The Tegra20 VI needs an additional operation to enable the VI, add an
operation for that.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/vi.c | 7 +++
drivers/staging/media/tegra-video/vi.h | 4
2 files changed, 11 insertions(+)
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index c76c2a404889..8df233049c81 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -1950,6 +1950,9 @@ static int tegra_vi_probe(struct platform_device *pdev)
vi->client.ops = _client_ops;
vi->client.dev = >dev;
+ if (vi->ops->vi_enable)
+ vi->ops->vi_enable(vi, true);
+
ret = host1x_client_register(>client);
if (ret < 0) {
dev_err(>dev,
@@ -1960,6 +1963,8 @@ static int tegra_vi_probe(struct platform_device *pdev)
return 0;
rpm_disable:
+ if (vi->ops->vi_enable)
+ vi->ops->vi_enable(vi, false);
pm_runtime_disable(>dev);
return ret;
}
@@ -1976,6 +1981,8 @@ static int tegra_vi_remove(struct platform_device *pdev)
return err;
}
+ if (vi->ops->vi_enable)
+ vi->ops->vi_enable(vi, false);
pm_runtime_disable(>dev);
return 0;
diff --git a/drivers/staging/media/tegra-video/vi.h
b/drivers/staging/media/tegra-video/vi.h
index b424c967c6f2..886b10e7d723 100644
--- a/drivers/staging/media/tegra-video/vi.h
+++ b/drivers/staging/media/tegra-video/vi.h
@@ -37,8 +37,11 @@ enum tegra_vi_pg_mode {
TEGRA_VI_PG_PATCH,
};
+struct tegra_vi;
+
/**
* struct tegra_vi_ops - Tegra VI operations
+ * @vi_enable: soc-specific operations needed to enable/disable the VI
peripheral
* @vi_fmt_align: modify `pix` to fit the hardware alignment
* requirements and fill image geometry
* @vi_start_streaming: starts media pipeline, subdevice streaming, sets up
@@ -48,6 +51,7 @@ enum tegra_vi_pg_mode {
* back any queued buffers.
*/
struct tegra_vi_ops {
+ int (*vi_enable)(struct tegra_vi *vi, bool on);
void (*vi_fmt_align)(struct v4l2_pix_format *pix, unsigned int bpp);
int (*vi_start_streaming)(struct vb2_queue *vq, u32 count);
void (*vi_stop_streaming)(struct vb2_queue *vq);
--
2.34.1
[PATCH v6 14/20] staging: media: tegra-video: move MIPI calibration calls from VI to CSI
The CSI module does not handle all the MIPI lane calibration procedure,
leaving a small part of it to the VI module. In doing this,
tegra_channel_enable_stream() (vi.c) manipulates the private data of the
upstream subdev casting it to struct 'tegra_csi_channel', which will be
wrong after introducing a VIP (parallel video input) channel.
This prevents adding support for the VIP module. It also breaks the
logical isolation between modules.
Since the lane calibration requirement does not exist in the parallel input
module, moving the calibration function to a per-module op is not
optimal. Instead move the calibration procedure in the CSI module, together
with the rest of the calibration procedures. After this change,
tegra_channel_enable_stream() just calls v4l2_subdev_call() to ask for a
stream start/stop to the CSI module, which in turn knows all the
CSI-specific details to implement it.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
Changed in v6:
- Fix unlock imbalance on Tegra210 (thanks Hans for reporting!)
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/csi.c | 44
drivers/staging/media/tegra-video/vi.c | 54 ++---
2 files changed, 48 insertions(+), 50 deletions(-)
diff --git a/drivers/staging/media/tegra-video/csi.c
b/drivers/staging/media/tegra-video/csi.c
index 9a03d5ccdf3c..91d9febb8fd8 100644
--- a/drivers/staging/media/tegra-video/csi.c
+++ b/drivers/staging/media/tegra-video/csi.c
@@ -328,12 +328,42 @@ static int tegra_csi_enable_stream(struct v4l2_subdev
*subdev)
}
csi_chan->pg_mode = chan->pg_mode;
+
+ /*
+* Tegra CSI receiver can detect the first LP to HS transition.
+* So, start the CSI stream-on prior to sensor stream-on and
+* vice-versa for stream-off.
+*/
ret = csi->ops->csi_start_streaming(csi_chan);
if (ret < 0)
goto finish_calibration;
+ if (csi_chan->mipi) {
+ struct v4l2_subdev *src_subdev;
+ /*
+* TRM has incorrectly documented to wait for done status from
+* calibration logic after CSI interface power on.
+* As per the design, calibration results are latched and
applied
+* to the pads only when the link is in LP11 state which will
happen
+* during the sensor stream-on.
+* CSI subdev stream-on triggers start of MIPI pads calibration.
+* Wait for calibration to finish here after sensor subdev
stream-on.
+*/
+ src_subdev = tegra_channel_get_remote_source_subdev(chan);
+ ret = v4l2_subdev_call(src_subdev, video, s_stream, true);
+
+ if (ret < 0 && ret != -ENOIOCTLCMD)
+ goto disable_csi_stream;
+
+ err = tegra_mipi_finish_calibration(csi_chan->mipi);
+ if (err < 0)
+ dev_warn(csi->dev, "MIPI calibration failed: %d\n",
err);
+ }
+
return 0;
+disable_csi_stream:
+ csi->ops->csi_stop_streaming(csi_chan);
finish_calibration:
if (csi_chan->mipi)
tegra_mipi_finish_calibration(csi_chan->mipi);
@@ -352,10 +382,24 @@ static int tegra_csi_enable_stream(struct v4l2_subdev
*subdev)
static int tegra_csi_disable_stream(struct v4l2_subdev *subdev)
{
+ struct tegra_vi_channel *chan = v4l2_get_subdev_hostdata(subdev);
struct tegra_csi_channel *csi_chan = to_csi_chan(subdev);
struct tegra_csi *csi = csi_chan->csi;
int err;
+ /*
+* Stream-off subdevices in reverse order to stream-on.
+* Remote source subdev in TPG mode is same as CSI subdev.
+*/
+ if (csi_chan->mipi) {
+ struct v4l2_subdev *src_subdev;
+
+ src_subdev = tegra_channel_get_remote_source_subdev(chan);
+ err = v4l2_subdev_call(src_subdev, video, s_stream, false);
+ if (err < 0 && err != -ENOIOCTLCMD)
+ dev_err_probe(csi->dev, err, "source subdev stream off
failed\n");
+ }
+
csi->ops->csi_stop_streaming(csi_chan);
if (csi_chan->mipi) {
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index b88532d8d2c9..c76c2a404889 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -197,49 +197,15 @@ tegra_channel_get_remote_source_subdev(struct
tegra_vi_channel *chan)
static int tegra_channel_enable_stream(struct tegra_vi_channel *chan)
{
- struct v4l2_subdev *csi_subdev, *src_subdev;
- struct tegra_csi_channel *csi_chan;
- int ret, err;
+ struct v4l2_subdev *subdev;
+ int ret;
- /*
-
[PATCH v6 13/20] staging: media: tegra-video: move default format to soc-specific data
The tegra_default_format in vi.c is specific to Tegra210 CSI.
In preparation for adding Tegra20 VIP support, move the default format to a
new field in the soc-specific `struct tegra_vi_soc`. Instead of an entire
format struct, only store a pointer to an item in the existing format
array.
No functional changes. The format pointed to is the same that used to be in
vi.c.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
Changed in v5:
- Minor update after removal of "staging: media: tegra-video: fix
.vidioc_enum_fmt_vid_cap to return all formats" patch
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/tegra210.c | 2 ++
drivers/staging/media/tegra-video/vi.c | 13 ++---
drivers/staging/media/tegra-video/vi.h | 2 ++
3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/media/tegra-video/tegra210.c
b/drivers/staging/media/tegra-video/tegra210.c
index d19ff6b49ae8..b4fcd4e93b8c 100644
--- a/drivers/staging/media/tegra-video/tegra210.c
+++ b/drivers/staging/media/tegra-video/tegra210.c
@@ -766,8 +766,10 @@ const struct tegra_vi_soc tegra210_vi_soc = {
.hw_revision = 3,
.vi_max_channels = 6,
#if IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG)
+ .default_video_format = _video_formats[0],
.vi_max_clk_hz = 49920,
#else
+ .default_video_format = _video_formats[4],
.vi_max_clk_hz = 99840,
#endif
};
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index 35d7cda1373f..b88532d8d2c9 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -45,15 +45,6 @@ struct tegra_vi_graph_entity {
struct v4l2_subdev *subdev;
};
-static const struct tegra_video_format tegra_default_format = {
- .img_dt = TEGRA_IMAGE_DT_RAW10,
- .bit_width = 10,
- .code = MEDIA_BUS_FMT_SRGGB10_1X10,
- .bpp = 2,
- .img_fmt = TEGRA_IMAGE_FORMAT_DEF,
- .fourcc = V4L2_PIX_FMT_SRGGB10,
-};
-
static inline struct tegra_vi *
host1x_client_to_vi(struct host1x_client *client)
{
@@ -1103,7 +1094,7 @@ static int vi_fmts_bitmap_init(struct tegra_vi_channel
*chan)
* there are no matched formats.
*/
if (!match_code) {
- match_code = tegra_default_format.code;
+ match_code = chan->vi->soc->default_video_format->code;
index = tegra_get_format_idx_by_code(chan->vi, match_code, 0);
if (WARN_ON(index < 0))
return -EINVAL;
@@ -1200,7 +1191,7 @@ static int tegra_channel_init(struct tegra_vi_channel
*chan)
init_waitqueue_head(>done_wait);
/* initialize the video format */
- chan->fmtinfo = _default_format;
+ chan->fmtinfo = chan->vi->soc->default_video_format;
chan->format.pixelformat = chan->fmtinfo->fourcc;
chan->format.colorspace = V4L2_COLORSPACE_SRGB;
chan->format.field = V4L2_FIELD_NONE;
diff --git a/drivers/staging/media/tegra-video/vi.h
b/drivers/staging/media/tegra-video/vi.h
index 213955c7545d..b424c967c6f2 100644
--- a/drivers/staging/media/tegra-video/vi.h
+++ b/drivers/staging/media/tegra-video/vi.h
@@ -58,6 +58,7 @@ struct tegra_vi_ops {
*
* @video_formats: supported video formats
* @nformats: total video formats
+ * @default_video_format: default video format (pointer to a @video_formats
item)
* @ops: vi operations
* @hw_revision: VI hw_revision
* @vi_max_channels: supported max streaming channels
@@ -66,6 +67,7 @@ struct tegra_vi_ops {
struct tegra_vi_soc {
const struct tegra_video_format *video_formats;
const unsigned int nformats;
+ const struct tegra_video_format *default_video_format;
const struct tegra_vi_ops *ops;
u32 hw_revision;
unsigned int vi_max_channels;
--
2.34.1
[PATCH v6 10/20] staging: media: tegra-video: remove unneeded include
There is only a pointer reference to struct tegra_vi in video.h, thus vi.h
is not needed.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/video.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/media/tegra-video/video.h
b/drivers/staging/media/tegra-video/video.h
index fadaf2189dc9..1e9be1474a9c 100644
--- a/drivers/staging/media/tegra-video/video.h
+++ b/drivers/staging/media/tegra-video/video.h
@@ -12,7 +12,6 @@
#include
#include "vi.h"
-#include "csi.h"
struct tegra_video_device {
struct v4l2_device v4l2_dev;
--
2.34.1
[PATCH v6 12/20] staging: media: tegra-video: move tegra_channel_fmt_align to a per-soc op
tegra_channel_fmt_align() takes care of the size constraints, alignment and
rounding requirements of the Tegra210 VI peripheral. Tegra20 has different
constraints.
In preparation for adding Tegra20 support, move this function to a new op
in the soc-specific `struct tegra_vi_ops` .
Also move to tegra210.c the T210-specific defines used in the moved code.
No functional changes.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/tegra210.c | 36 ++
drivers/staging/media/tegra-video/vi.c | 40 +++-
drivers/staging/media/tegra-video/vi.h | 9 ++---
3 files changed, 44 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/media/tegra-video/tegra210.c
b/drivers/staging/media/tegra-video/tegra210.c
index d58370a84737..d19ff6b49ae8 100644
--- a/drivers/staging/media/tegra-video/tegra210.c
+++ b/drivers/staging/media/tegra-video/tegra210.c
@@ -17,6 +17,13 @@
#include "csi.h"
#include "vi.h"
+#define TEGRA210_MIN_WIDTH 32U
+#define TEGRA210_MAX_WIDTH 32768U
+#define TEGRA210_MIN_HEIGHT32U
+#define TEGRA210_MAX_HEIGHT32768U
+
+#define SURFACE_ALIGN_BYTES64
+
#define TEGRA_VI_SYNCPT_WAIT_TIMEOUT msecs_to_jiffies(200)
/* Tegra210 VI registers */
@@ -172,6 +179,34 @@ static u32 vi_csi_read(struct tegra_vi_channel *chan, u8
portno,
/*
* Tegra210 VI channel capture operations
*/
+static void tegra210_fmt_align(struct v4l2_pix_format *pix, unsigned int bpp)
+{
+ unsigned int min_bpl;
+ unsigned int max_bpl;
+ unsigned int bpl;
+
+ /*
+* The transfer alignment requirements are expressed in bytes.
+* Clamp the requested width and height to the limits.
+*/
+ pix->width = clamp(pix->width, TEGRA210_MIN_WIDTH, TEGRA210_MAX_WIDTH);
+ pix->height = clamp(pix->height, TEGRA210_MIN_HEIGHT,
TEGRA210_MAX_HEIGHT);
+
+ /* Clamp the requested bytes per line value. If the maximum bytes per
+* line value is zero, the module doesn't support user configurable
+* line sizes. Override the requested value with the minimum in that
+* case.
+*/
+ min_bpl = pix->width * bpp;
+ max_bpl = rounddown(TEGRA210_MAX_WIDTH, SURFACE_ALIGN_BYTES);
+ bpl = roundup(pix->bytesperline, SURFACE_ALIGN_BYTES);
+
+ pix->bytesperline = clamp(bpl, min_bpl, max_bpl);
+ pix->sizeimage = pix->bytesperline * pix->height;
+ if (pix->pixelformat == V4L2_PIX_FMT_NV16)
+ pix->sizeimage *= 2;
+}
+
static int tegra_channel_capture_setup(struct tegra_vi_channel *chan,
u8 portno)
{
@@ -718,6 +753,7 @@ static const struct tegra_video_format
tegra210_video_formats[] = {
/* Tegra210 VI operations */
static const struct tegra_vi_ops tegra210_vi_ops = {
+ .vi_fmt_align = tegra210_fmt_align,
.vi_start_streaming = tegra210_vi_start_streaming,
.vi_stop_streaming = tegra210_vi_stop_streaming,
};
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index db98d06351b4..35d7cda1373f 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -473,36 +473,6 @@ static int tegra_channel_get_format(struct file *file,
void *fh,
return 0;
}
-static void tegra_channel_fmt_align(struct tegra_vi_channel *chan,
- struct v4l2_pix_format *pix,
- unsigned int bpp)
-{
- unsigned int min_bpl;
- unsigned int max_bpl;
- unsigned int bpl;
-
- /*
-* The transfer alignment requirements are expressed in bytes.
-* Clamp the requested width and height to the limits.
-*/
- pix->width = clamp(pix->width, TEGRA_MIN_WIDTH, TEGRA_MAX_WIDTH);
- pix->height = clamp(pix->height, TEGRA_MIN_HEIGHT, TEGRA_MAX_HEIGHT);
-
- /* Clamp the requested bytes per line value. If the maximum bytes per
-* line value is zero, the module doesn't support user configurable
-* line sizes. Override the requested value with the minimum in that
-* case.
-*/
- min_bpl = pix->width * bpp;
- max_bpl = rounddown(TEGRA_MAX_WIDTH, SURFACE_ALIGN_BYTES);
- bpl = roundup(pix->bytesperline, SURFACE_ALIGN_BYTES);
-
- pix->bytesperline = clamp(bpl, min_bpl, max_bpl);
- pix->sizeimage = pix->bytesperline * pix->height;
- if (pix->pixelformat == V4L2_PIX_FMT_NV16)
- pix->sizeimage *= 2;
-}
-
static int __tegra_channel_try_format(struct tegra_vi_channel *chan,
struct v4l2_pix_format *pix)
{
@@ -578,7 +548,7 @@ static int __tegra_channel_try_forma
[PATCH v6 11/20] staging: media: tegra-video: Kconfig: allow TPG only on Tegra210
We are about to add support for the Tegra20 parallel video capture, which has no TPG. In preparation for that, limit the VIDEO_TEGRA_TPG option to Tegra210 which is the only implementation currently provided by this driver. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v6 No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/media/tegra-video/Kconfig b/drivers/staging/media/tegra-video/Kconfig index df1b2cff2417..c53441822fdf 100644 --- a/drivers/staging/media/tegra-video/Kconfig +++ b/drivers/staging/media/tegra-video/Kconfig @@ -15,5 +15,6 @@ config VIDEO_TEGRA config VIDEO_TEGRA_TPG bool "NVIDIA Tegra VI driver TPG mode" depends on VIDEO_TEGRA + depends on ARCH_TEGRA_210_SOC help Say yes here to enable Tegra internal TPG mode -- 2.34.1
[PATCH v6 08/20] staging: media: tegra-video: move private struct declaration to C file
struct tegra_vi_graph_entity is an internal implementation detail of the VI
module. Move its declaration from vi.h to vi.c.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/vi.c | 13 +
drivers/staging/media/tegra-video/vi.h | 13 -
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index ce4ff4cbf587..db98d06351b4 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -32,6 +32,19 @@
#define MAX_CID_CONTROLS 1
+/**
+ * struct tegra_vi_graph_entity - Entity in the video graph
+ *
+ * @asd: subdev asynchronous registration information
+ * @entity: media entity from the corresponding V4L2 subdev
+ * @subdev: V4L2 subdev
+ */
+struct tegra_vi_graph_entity {
+ struct v4l2_async_subdev asd;
+ struct media_entity *entity;
+ struct v4l2_subdev *subdev;
+};
+
static const struct tegra_video_format tegra_default_format = {
.img_dt = TEGRA_IMAGE_DT_RAW10,
.bit_width = 10,
diff --git a/drivers/staging/media/tegra-video/vi.h
b/drivers/staging/media/tegra-video/vi.h
index 5396bf53ab75..9959cbe02ca0 100644
--- a/drivers/staging/media/tegra-video/vi.h
+++ b/drivers/staging/media/tegra-video/vi.h
@@ -98,19 +98,6 @@ struct tegra_vi {
struct list_head vi_chans;
};
-/**
- * struct tegra_vi_graph_entity - Entity in the video graph
- *
- * @asd: subdev asynchronous registration information
- * @entity: media entity from the corresponding V4L2 subdev
- * @subdev: V4L2 subdev
- */
-struct tegra_vi_graph_entity {
- struct v4l2_async_subdev asd;
- struct media_entity *entity;
- struct v4l2_subdev *subdev;
-};
-
/**
* struct tegra_vi_channel - Tegra video channel
*
--
2.34.1
[PATCH v6 09/20] staging: media: tegra-video: move tegra210_csi_soc to C file
This declaration is used only in csi.c, no need to export it elsewhere.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
No changes in v5
Changed in v4:
- Added review tags
This patch was added in v3.
---
drivers/staging/media/tegra-video/csi.c | 4
drivers/staging/media/tegra-video/csi.h | 4
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/tegra-video/csi.c
b/drivers/staging/media/tegra-video/csi.c
index 426e653bd55d..9a03d5ccdf3c 100644
--- a/drivers/staging/media/tegra-video/csi.c
+++ b/drivers/staging/media/tegra-video/csi.c
@@ -792,6 +792,10 @@ static int tegra_csi_remove(struct platform_device *pdev)
return 0;
}
+#if defined(CONFIG_ARCH_TEGRA_210_SOC)
+extern const struct tegra_csi_soc tegra210_csi_soc;
+#endif
+
static const struct of_device_id tegra_csi_of_id_table[] = {
#if defined(CONFIG_ARCH_TEGRA_210_SOC)
{ .compatible = "nvidia,tegra210-csi", .data = _csi_soc },
diff --git a/drivers/staging/media/tegra-video/csi.h
b/drivers/staging/media/tegra-video/csi.h
index 6960ea2e3d36..3e6e5ee1bb1e 100644
--- a/drivers/staging/media/tegra-video/csi.h
+++ b/drivers/staging/media/tegra-video/csi.h
@@ -151,10 +151,6 @@ struct tegra_csi {
struct list_head csi_chans;
};
-#if defined(CONFIG_ARCH_TEGRA_210_SOC)
-extern const struct tegra_csi_soc tegra210_csi_soc;
-#endif
-
void tegra_csi_error_recover(struct v4l2_subdev *subdev);
void tegra_csi_calc_settle_time(struct tegra_csi_channel *csi_chan,
u8 csi_port_num,
--
2.34.1
[PATCH v6 05/20] staging: media: tegra-video: fix typos in comment
Add "skip" in "so we can *skip* the current channel" or it doesn't make
sense.
Also add articles where appropriate to fix English grammar.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/vi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index 44c56dc85980..79dd02a1e29b 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -1859,10 +1859,10 @@ static int tegra_vi_graph_init(struct tegra_vi *vi)
* Walk the links to parse the full graph. Each channel will have
* one endpoint of the composite node. Start by parsing the
* composite node and parse the remote entities in turn.
-* Each channel will register v4l2 async notifier to make the graph
-* independent between the channels so we can the current channel
+* Each channel will register a v4l2 async notifier to make the graph
+* independent between the channels so we can skip the current channel
* in case of something wrong during graph parsing and continue with
-* next channels.
+* the next channels.
*/
list_for_each_entry(chan, >vi_chans, list) {
struct fwnode_handle *ep, *remote;
--
2.34.1
[PATCH v6 07/20] staging: media: tegra-video: slightly simplify cleanup on errors
of_node_put(node) does nothing if node == NULL, so it can be moved to the
cleanup section at the bottom.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/vi.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index 1c0424bf1ab0..ce4ff4cbf587 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -1352,7 +1352,7 @@ static int tegra_vi_channels_alloc(struct tegra_vi *vi)
struct device_node *node = vi->dev->of_node;
struct device_node *ep = NULL;
struct device_node *ports;
- struct device_node *port;
+ struct device_node *port = NULL;
unsigned int port_num;
struct device_node *parent;
struct v4l2_fwnode_endpoint v4l2_ep = { .bus_type = 0 };
@@ -1375,7 +1375,6 @@ static int tegra_vi_channels_alloc(struct tegra_vi *vi)
dev_err(vi->dev, "invalid port num %d for %pOF\n",
port_num, port);
ret = -EINVAL;
- of_node_put(port);
goto cleanup;
}
@@ -1398,13 +1397,12 @@ static int tegra_vi_channels_alloc(struct tegra_vi *vi)
lanes = v4l2_ep.bus.mipi_csi2.num_data_lanes;
ret = tegra_vi_channel_alloc(vi, port_num, port, lanes);
- if (ret < 0) {
- of_node_put(port);
+ if (ret < 0)
goto cleanup;
- }
}
cleanup:
+ of_node_put(port);
of_node_put(ports);
return ret;
}
--
2.34.1
[PATCH v6 06/20] staging: media: tegra-video: improve error messages
tegra_vi_channels_alloc() can primarily fail for two reasons:
1. "ports" node not found
2. port_num > vi->soc->vi_max_channels
Case 1 prints nothing, case 2 has a dev_err(). The caller [tegra_vi_init()]
has a generic dev_err() on any failure. This mean that in case 2 we print
two messages, and in case 1 we only print a generic message.
Remove the generic message and add a specific message when case 1 happens,
so that we always have one specific message without even increasing the
number of dev_dbg*() calls.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/vi.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index 79dd02a1e29b..1c0424bf1ab0 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -1361,7 +1361,7 @@ static int tegra_vi_channels_alloc(struct tegra_vi *vi)
ports = of_get_child_by_name(node, "ports");
if (!ports)
- return -ENODEV;
+ return dev_err_probe(vi->dev, -ENODEV, "%pOF: missing 'ports'
node\n", node);
for_each_child_of_node(ports, port) {
if (!of_node_name_eq(port, "port"))
@@ -1921,11 +1921,8 @@ static int tegra_vi_init(struct host1x_client *client)
ret = tegra_vi_tpg_channels_alloc(vi);
else
ret = tegra_vi_channels_alloc(vi);
- if (ret < 0) {
- dev_err(vi->dev,
- "failed to allocate vi channels: %d\n", ret);
+ if (ret < 0)
goto free_chans;
- }
ret = tegra_vi_channels_init(vi);
if (ret < 0)
--
2.34.1
[PATCH v6 03/20] staging: media: tegra-video: improve documentation of tegra_video_format fields
Some fields are irrelevant for Tegra20/VIP. Add a note to clarify that.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/vi.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/media/tegra-video/vi.h
b/drivers/staging/media/tegra-video/vi.h
index a68e2c02c7b0..5396bf53ab75 100644
--- a/drivers/staging/media/tegra-video/vi.h
+++ b/drivers/staging/media/tegra-video/vi.h
@@ -260,11 +260,11 @@ enum tegra_image_dt {
/**
* struct tegra_video_format - Tegra video format description
*
- * @img_dt: image data type
- * @bit_width: format width in bits per component
+ * @img_dt: MIPI CSI-2 data type (for CSI-2 only)
+ * @bit_width: format width in bits per component (for CSI/Tegra210 only)
* @code: media bus format code
* @bpp: bytes per pixel (when stored in memory)
- * @img_fmt: image format
+ * @img_fmt: image format (for CSI/Tegra210 only)
* @fourcc: V4L2 pixel format FCC identifier
*/
struct tegra_video_format {
--
2.34.1
[PATCH v6 04/20] staging: media: tegra-video: document tegra_channel_get_remote_source_subdev
Clarify what this function does.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v6
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/vi.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index 11dd142c98c5..44c56dc85980 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -164,6 +164,9 @@ tegra_channel_get_remote_csi_subdev(struct tegra_vi_channel
*chan)
return media_entity_to_v4l2_subdev(pad->entity);
}
+/*
+ * Walk up the chain until the initial source (e.g. image sensor)
+ */
struct v4l2_subdev *
tegra_channel_get_remote_source_subdev(struct tegra_vi_channel *chan)
{
--
2.34.1
[PATCH v6 02/20] dt-bindings: display: tegra: vi: add 'vip' property and example
The Tegra20 VI peripheral can receive parallel input from the VIP parallel
input module. Add it to the allowed properties and augment the existing
nvidia,tegra20-vi example to show a 'vip' property.
Signed-off-by: Luca Ceresoli
Reviewed-by: Rob Herring
---
No changes in v6
No changes in v5
Changed in RESEND,v4:
- Add Reviewed-by: Rob Herring
Changed in v4:
- complete the removal of 'channel@0'
Changed in v3 (suggested by Rob Herring):
- drop 'endpoint', unneeded as there's no extra properties in the
endpoints
Changed in v2 (suggested by Krzysztof Kozlowski):
- rename "i2c3" -> "ic2"
- add review tag
---
.../display/tegra/nvidia,tegra20-vi.yaml | 59 +++
MAINTAINERS | 1 +
2 files changed, 60 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml
b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml
index a42bf33d1e7d..2181855a0920 100644
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml
@@ -73,6 +73,18 @@ properties:
avdd-dsi-csi-supply:
description: DSI/CSI power supply. Must supply 1.2 V.
+ vip:
+$ref: /schemas/display/tegra/nvidia,tegra20-vip.yaml
+
+ ports:
+$ref: /schemas/graph.yaml#/properties/ports
+
+properties:
+ port@0:
+$ref: /schemas/graph.yaml#/properties/port
+description:
+ Input from the VIP (parallel input capture) module
+
patternProperties:
"^csi@[0-9a-f]+$":
type: object
@@ -108,6 +120,22 @@ examples:
#include
#include
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+camera@48 {
+compatible = "aptina,mt9v111";
+reg = <0x48>;
+clocks = <_clk>;
+
+port {
+mt9v111_out: endpoint {
+remote-endpoint = <_vip_in>;
+};
+};
+};
+};
+
vi@5408 {
compatible = "nvidia,tegra20-vi";
reg = <0x5408 0x0004>;
@@ -115,6 +143,37 @@ examples:
clocks = <_car TEGRA20_CLK_VI>;
resets = <_car 100>;
reset-names = "vi";
+
+vip {
+compatible = "nvidia,tegra20-vip";
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+port@0 {
+reg = <0>;
+vi_vip_in: endpoint {
+remote-endpoint = <_out>;
+};
+};
+port@1 {
+reg = <1>;
+vi_vip_out: endpoint {
+remote-endpoint = <_in>;
+};
+};
+};
+};
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+port@0 {
+reg = <0>;
+vi_in: endpoint {
+remote-endpoint = <_vip_out>;
+};
+};
+};
};
- |
diff --git a/MAINTAINERS b/MAINTAINERS
index 8d8b0aa8e009..90594131be4f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20615,6 +20615,7 @@ L: linux-me...@vger.kernel.org
L: linux-te...@vger.kernel.org
S: Maintained
F:
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
+F: Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml
F: Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
F: drivers/staging/media/tegra-video/
--
2.34.1
[PATCH v6 01/20] dt-bindings: display: tegra: add Tegra20 VIP
VIP is the parallel video capture component within the video input subsystem of Tegra20 (and other Tegra chips, apparently). Signed-off-by: Luca Ceresoli Reviewed-by: Krzysztof Kozlowski Reviewed-by: Dmitry Osipenko --- No changes in v6 No changes in v5 Changed in v4: - Added review tags - remove leftover lines after removal of 'channel@0' Changed in v3: - remove channel@0 node (Krzysztof, Rob, Dmitry) - add myself as a maintainer of the whole Tegra video driver (Dmitry) Changed in v2 (suggested by Krzysztof Kozlowski): - remove redundant "bindings" from subject line - remove $nodename - add channel@0 description - add reg: const: 0 --- .../display/tegra/nvidia,tegra20-vip.yaml | 41 +++ MAINTAINERS | 2 + 2 files changed, 43 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml new file mode 100644 index ..14294edb8d8c --- /dev/null +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/tegra/nvidia,tegra20-vip.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NVIDIA Tegra VIP (parallel video capture) controller + +maintainers: + - Luca Ceresoli + +properties: + compatible: +enum: + - nvidia,tegra20-vip + + ports: +$ref: /schemas/graph.yaml#/properties/ports + +properties: + port@0: +$ref: /schemas/graph.yaml#/properties/port +description: + Port receiving the video stream from the sensor + + port@1: +$ref: /schemas/graph.yaml#/properties/port +description: + Port sending the video stream to the VI + +required: + - port@0 + - port@1 + +unevaluatedProperties: false + +required: + - compatible + - ports + +# see nvidia,tegra20-vi.yaml for an example diff --git a/MAINTAINERS b/MAINTAINERS index 0e64787aace8..8d8b0aa8e009 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20610,10 +20610,12 @@ TEGRA VIDEO DRIVER M: Thierry Reding M: Jonathan Hunter M: Sowjanya Komatineni +M: Luca Ceresoli L: linux-me...@vger.kernel.org L: linux-te...@vger.kernel.org S: Maintained F: Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +F: Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml F: drivers/staging/media/tegra-video/ TEGRA XUSB PADCTL DRIVER -- 2.34.1
[PATCH v6 00/20] Add Tegra20 parallel video input capture
New in v6: a oneliner fix to patch 14 for an unlock imbalance in MIPI CSI calibration (Tegra210 only). Many thanks to Hans for testing and spotting this! Full details follow. Tegra20 and other Tegra SoCs have a video input (VI) peripheral that can receive from either MIPI CSI-2 or parallel video (called respectively "CSI" and "VIP" in the documentation). The kernel currently has a staging driver for Tegra210 CSI capture. This patch set adds support for Tegra20 VIP capture. Unfortunately I had no real documentation available to base this work on. I only had a working downstream 3.1 kernel, so I started with the driver found there and heavily reworked it to fit into the mainline tegra-video driver structure. The existing code appears written with the intent of being modular and allow adding new input mechanisms and new SoCs while keeping a unique VI core module. However its modularity and extensibility was not enough to add Tegra20 VIP support, so I added some hooks to turn hard-coded behaviour into per-SoC or per-bus customizable code. There are also a fix, some generic cleanups and DT bindings. Quick tour of the patches: * Device tree bindings 01. dt-bindings: display: tegra: add Tegra20 VIP 02. dt-bindings: display: tegra: vi: add 'vip' property and example * Minor improvements to logging, comments, cleanups 03. staging: media: tegra-video: improve documentation of tegra_video_format fields 04. staging: media: tegra-video: document tegra_channel_get_remote_source_subdev 05. staging: media: tegra-video: fix typos in comment 06. staging: media: tegra-video: improve error messages 07. staging: media: tegra-video: slightly simplify cleanup on errors 08. staging: media: tegra-video: move private struct declaration to C file 09. staging: media: tegra-video: move tegra210_csi_soc to C file 10. staging: media: tegra-video: remove unneeded include * Preparation to make the VI module generic enough to host Tegra20 and VIP 11. staging: media: tegra-video: Kconfig: allow TPG only on Tegra210 12. staging: media: tegra-video: move tegra_channel_fmt_align to a per-soc op 13. staging: media: tegra-video: move default format to soc-specific data 14. staging: media: tegra-video: move MIPI calibration calls from VI to CSI 15. staging: media: tegra-video: add a per-soc enable/disable op 16. staging: media: tegra-video: move syncpt init/free to a per-soc op 17. staging: media: tegra-video: add syncpts for Tegra20 to struct tegra_vi 18. staging: media: tegra-video: add hooks for planar YUV and H/V flip 19. staging: media: tegra-video: add H/V flip controls * Implementation of VIP and Tegra20 20. staging: media: tegra-video: add support for Tegra20 parallel input Enjoy! Changed in v5: - patch 14: fixed unlock imbalance on Tegra210 in MIPI CSI calibration Changed in v5: - removed patch 3 as requested by Hans Verkuil; now the driver is kept video-node-centric and the enum_format logic is unchanged - rebased on top of that - trivial fixes (typos) Changed in RESEND,v4: - add Rob's review tag on patch 2 Changed in v4: - fixed the leftovers after the removal of 'channel@0' in DT - added review tags by Dimtry Changed in v3: - removed the 'channel@0' node from the device tree representation of vip - squashed the last two patches (VIP + T20) into one - small cleanups - rebase on v6.2-rc1 Changed in v2: - improved dt-bindings patches based on reviews - removed patches 3 and 4 adding DT labels without a mainline user - two small fixes to the last patch [v5] https://lore.kernel.org/linux-tegra/20230407133852.2850145-1-luca.ceres...@bootlin.com/ [v4,resend] https://lore.kernel.org/linux-tegra/20230309144320.2937553-1-luca.ceres...@bootlin.com/ [v4] https://lore.kernel.org/linux-tegra/20230130141603.323221-1-luca.ceres...@bootlin.com/ [v3] https://lore.kernel.org/linux-media/20221229133205.981397-1-luca.ceres...@bootlin.com/ [v2] https://lore.kernel.org/linux-tegra/20221222100328.6e341874@booty/T/#t [v1] https://lore.kernel.org/linux-tegra/20221124155634.5bc2a423@booty/T/#t Luca Luca Ceresoli (20): dt-bindings: display: tegra: add Tegra20 VIP dt-bindings: display: tegra: vi: add 'vip' property and example staging: media: tegra-video: improve documentation of tegra_video_format fields staging: media: tegra-video: document tegra_channel_get_remote_source_subdev staging: media: tegra-video: fix typos in comment staging: media: tegra-video: improve error messages staging: media: tegra-video: slightly simplify cleanup on errors staging: media: tegra-video: move private struct declaration to C file staging: media: tegra-video: move tegra210_csi_soc to C file staging: media: tegra-video: remove unneeded include staging: media: tegra-video: Kconfig: allow TPG only on Tegra210 staging: media: tegra-video: move tegra_channel_fmt_align to a per-soc op staging: media: tegra-video: move default format
Re: [PATCH v5 00/20] Add Tegra20 parallel video input capture
Hello Hans, On Fri, 7 Apr 2023 15:38:32 +0200 Luca Ceresoli wrote: > New in v5: dropped the patch that was removing lots of the logic behind > enum_format, after discussion with Hans. The rest is unmodified except for > rebasing and fixing a couple typos in comments. > > Full details follow. > > Tegra20 and other Tegra SoCs have a video input (VI) peripheral that can > receive from either MIPI CSI-2 or parallel video (called respectively "CSI" > and "VIP" in the documentation). The kernel currently has a staging driver > for Tegra210 CSI capture. This patch set adds support for Tegra20 VIP > capture. > > Unfortunately I had no real documentation available to base this work on. > I only had a working downstream 3.1 kernel, so I started with the driver > found there and heavily reworked it to fit into the mainline tegra-video > driver structure. The existing code appears written with the intent of > being modular and allow adding new input mechanisms and new SoCs while > keeping a unique VI core module. However its modularity and extensibility > was not enough to add Tegra20 VIP support, so I added some hooks to turn > hard-coded behaviour into per-SoC or per-bus customizable code. There are > also a fix, some generic cleanups and DT bindings. > > Quick tour of the patches: > > * Device tree bindings > >01. dt-bindings: display: tegra: add Tegra20 VIP >02. dt-bindings: display: tegra: vi: add 'vip' property and example > > * Minor improvements to logging, comments, cleanups > >03. staging: media: tegra-video: improve documentation of > tegra_video_format fields >04. staging: media: tegra-video: document > tegra_channel_get_remote_source_subdev >05. staging: media: tegra-video: fix typos in comment >06. staging: media: tegra-video: improve error messages >07. staging: media: tegra-video: slightly simplify cleanup on errors >08. staging: media: tegra-video: move private struct declaration to C file >09. staging: media: tegra-video: move tegra210_csi_soc to C file >10. staging: media: tegra-video: remove unneeded include > > * Preparation to make the VI module generic enough to host Tegra20 and VIP > >11. staging: media: tegra-video: Kconfig: allow TPG only on Tegra210 >12. staging: media: tegra-video: move tegra_channel_fmt_align to a per-soc > op >13. staging: media: tegra-video: move default format to soc-specific data >14. staging: media: tegra-video: move MIPI calibration calls from VI to CSI >15. staging: media: tegra-video: add a per-soc enable/disable op >16. staging: media: tegra-video: move syncpt init/free to a per-soc op >17. staging: media: tegra-video: add syncpts for Tegra20 to struct tegra_vi >18. staging: media: tegra-video: add hooks for planar YUV and H/V flip >19. staging: media: tegra-video: add H/V flip controls > > * Implementation of VIP and Tegra20 > >20. staging: media: tegra-video: add support for Tegra20 parallel input > > Enjoy! > > Changed in v5: > - removed patch 3 as requested by Hans Verkuil; now the driver is kept > video-node-centric and the enum_format logic is unchanged > - rebased on top of that > - trivial fixes (typos) According to your review of v4, removing patch 3 was the only change required, and I didn't do anything else, and there have been no big changes since v1 anyway, so I was wondering whether this series has any hope to make it for 6.4... Best regards, Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH v5 20/20] staging: media: tegra-video: add support for Tegra20 parallel input
The VI peripheral of Tegra supports capturing from MIPI CSI-2 or parallel video (called VIP in the docs). The staging tegra-video driver currently implements MIPI CSI-2 video capture for Tegra210. Add support for parallel video capture (VIP) on Tegra20. With the generalizations added to the VI driver in previous commits, this is only a matter of adding the vip.c and tegra20.c implementations and registering them. Unfortunately there was no documentation available for the VI or VIP peripherals of Tegra20 (or any other Tegra chips). This implementation has been based entirely on the code from a vendor kernel based on Linux 3.1 and massively adapted to fit into the tegra-video driver. Parts of this code is definitely non-optimal to say the least (especially tegra20_vi_enable() and the single-frame capture logic), but it was impossible to improve it. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- Changed in v5: - Fixed small typo - Upgraded copyright year Changed in v4: - Added review tags Changed in v3 (suggested by Dmitry Osipenko): - merged the VIP patch and the Tegra20 patch to avoid chicken-egg problem due to the two modules depending on each other at build time - move tegra20_vip_soc to vip.c - remove channel@0 node from device tree parsing - remove unused variable Changed in v2: - fix tegra20_vi_enable() to clear bit when on==false - clamp width/height from set/try_fmt to avoid returning sizeimage=0 (fixes v4l2-compliance) --- drivers/staging/media/tegra-video/Makefile | 2 + drivers/staging/media/tegra-video/tegra20.c | 661 drivers/staging/media/tegra-video/vi.c | 3 + drivers/staging/media/tegra-video/vi.h | 3 + drivers/staging/media/tegra-video/video.c | 5 + drivers/staging/media/tegra-video/video.h | 1 + drivers/staging/media/tegra-video/vip.c | 290 + drivers/staging/media/tegra-video/vip.h | 68 ++ 8 files changed, 1033 insertions(+) create mode 100644 drivers/staging/media/tegra-video/tegra20.c create mode 100644 drivers/staging/media/tegra-video/vip.c create mode 100644 drivers/staging/media/tegra-video/vip.h diff --git a/drivers/staging/media/tegra-video/Makefile b/drivers/staging/media/tegra-video/Makefile index dfa2ef8f99ef..6c7552e05109 100644 --- a/drivers/staging/media/tegra-video/Makefile +++ b/drivers/staging/media/tegra-video/Makefile @@ -2,7 +2,9 @@ tegra-video-objs := \ video.o \ vi.o \ + vip.o \ csi.o +tegra-video-$(CONFIG_ARCH_TEGRA_2x_SOC) += tegra20.o tegra-video-$(CONFIG_ARCH_TEGRA_210_SOC) += tegra210.o obj-$(CONFIG_VIDEO_TEGRA) += tegra-video.o diff --git a/drivers/staging/media/tegra-video/tegra20.c b/drivers/staging/media/tegra-video/tegra20.c new file mode 100644 index ..c25286772603 --- /dev/null +++ b/drivers/staging/media/tegra-video/tegra20.c @@ -0,0 +1,661 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Tegra20-specific VI implementation + * + * Copyright (C) 2023 SKIDATA GmbH + * Author: Luca Ceresoli + */ + +/* + * This source file contains Tegra20 supported video formats, + * VI and VIP SoC specific data, operations and registers accessors. + */ + +#include +#include +#include +#include +#include +#include + +#include "vip.h" +#include "vi.h" + +#define TEGRA_VI_SYNCPT_WAIT_TIMEOUT msecs_to_jiffies(200) + +/* This are just good-sense numbers. The actual min/max is not documented. */ +#define TEGRA20_MIN_WIDTH 32U +#define TEGRA20_MIN_HEIGHT 32U +#define TEGRA20_MAX_WIDTH 2048U +#define TEGRA20_MAX_HEIGHT 2048U + +/* -- + * Registers + */ + +#define TEGRA_VI_CONT_SYNCPT_OUT_1 0x0060 +#define VI_CONT_SYNCPT_OUT_1_CONTINUOUS_SYNCPT BIT(8) +#define VI_CONT_SYNCPT_OUT_1_SYNCPT_IDX_SFT 0 + +#define TEGRA_VI_VI_INPUT_CONTROL 0x0088 +#define VI_INPUT_FIELD_DETECTBIT(27) +#define VI_INPUT_BT656 BIT(25) +#define VI_INPUT_YUV_INPUT_FORMAT_SFT8 /* bits [9:8] */ +#define VI_INPUT_YUV_INPUT_FORMAT_UYVY (0 << VI_INPUT_YUV_INPUT_FORMAT_SFT) +#define VI_INPUT_YUV_INPUT_FORMAT_VYUY (1 << VI_INPUT_YUV_INPUT_FORMAT_SFT) +#define VI_INPUT_YUV_INPUT_FORMAT_YUYV (2 << VI_INPUT_YUV_INPUT_FORMAT_SFT) +#define VI_INPUT_YUV_INPUT_FORMAT_YVYU (3 << VI_INPUT_YUV_INPUT_FORMAT_SFT) +#define VI_INPUT_INPUT_FORMAT_SFT2 /* bits [5:2] */ +#define VI_INPUT_INPUT_FORMAT_YUV422 (0 << VI_INPUT_INPUT_FORMAT_SFT) +#define VI_INPUT_VIP_INPUT_ENABLEBIT(1) + +#define TEGRA_VI_VI_CORE_CONTROL 0x008c +#define VI_VI_CORE_CONT
[PATCH v5 19/20] staging: media: tegra-video: add H/V flip controls
Tegra20 can do horizontal and vertical image flip, but Tegra210 cannot
(either the hardware, or this driver).
In preparation to adding Tegra20 support, add a flag in struct tegra_vi_soc
so the generic vi.c code knows whether the flip controls should be added or
not.
Also provide a generic implementation that simply sets two flags in the
channel struct. The Tegra20 implementation will enable flipping at stream
start based on those flags.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
Changed in v5:
- Fixed typo in comment
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/vi.c | 14 +-
drivers/staging/media/tegra-video/vi.h | 8
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index 5ab24977ec46..39e9df895ede 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -30,7 +30,7 @@
#include "vi.h"
#include "video.h"
-#define MAX_CID_CONTROLS 1
+#define MAX_CID_CONTROLS 3
/**
* struct tegra_vi_graph_entity - Entity in the video graph
@@ -910,6 +910,12 @@ static int vi_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_TEGRA_SYNCPT_TIMEOUT_RETRY:
chan->syncpt_timeout_retry = ctrl->val;
break;
+ case V4L2_CID_HFLIP:
+ chan->hflip = ctrl->val;
+ break;
+ case V4L2_CID_VFLIP:
+ chan->vflip = ctrl->val;
+ break;
default:
return -EINVAL;
}
@@ -981,6 +987,12 @@ static int tegra_channel_setup_ctrl_handler(struct
tegra_vi_channel *chan)
v4l2_ctrl_handler_free(>ctrl_handler);
return ret;
}
+
+ if (chan->vi->soc->has_h_v_flip) {
+ v4l2_ctrl_new_std(>ctrl_handler, _ctrl_ops,
V4L2_CID_HFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(>ctrl_handler, _ctrl_ops,
V4L2_CID_VFLIP, 0, 1, 1, 0);
+ }
+
#endif
/* setup the controls */
diff --git a/drivers/staging/media/tegra-video/vi.h
b/drivers/staging/media/tegra-video/vi.h
index cadf80b742a8..778c0ec475ab 100644
--- a/drivers/staging/media/tegra-video/vi.h
+++ b/drivers/staging/media/tegra-video/vi.h
@@ -74,6 +74,7 @@ struct tegra_vi_ops {
* @hw_revision: VI hw_revision
* @vi_max_channels: supported max streaming channels
* @vi_max_clk_hz: VI clock max frequency
+ * @has_h_v_flip: the chip can do H and V flip, and the driver implements it
*/
struct tegra_vi_soc {
const struct tegra_video_format *video_formats;
@@ -83,6 +84,7 @@ struct tegra_vi_soc {
u32 hw_revision;
unsigned int vi_max_channels;
unsigned int vi_max_clk_hz;
+ bool has_h_v_flip:1;
};
/**
@@ -172,6 +174,9 @@ struct tegra_vi {
* @tpg_fmts_bitmap: a bitmap for supported TPG formats
* @pg_mode: test pattern generator mode (disabled/direct/patch)
* @notifier: V4L2 asynchronous subdevs notifier
+ *
+ * @hflip: Horizontal flip is enabled
+ * @vflip: Vertical flip is enabled
*/
struct tegra_vi_channel {
struct list_head list;
@@ -222,6 +227,9 @@ struct tegra_vi_channel {
enum tegra_vi_pg_mode pg_mode;
struct v4l2_async_notifier notifier;
+
+ bool hflip:1;
+ bool vflip:1;
};
/**
--
2.34.1
[PATCH v5 18/20] staging: media: tegra-video: add hooks for planar YUV and H/V flip
Tegra20 supports planar YUV422 capture, which can be implemented by writing
U and V base address registers in addition to the "main" base buffer
address register.
It also supports H and V flip, which among others requires to write the
start address (i.e. the 1st offset to write, at the end of the buffer or
line) in more registers for Y and, for planar formats, U and V.
Add minimal hooks in VI to allow per-SoC optional support to those
features:
- variables in struct tegra_vi for the U and V buffer base offsets
- variables in struct tegra_vi for the Y, U and V buffer start offsets
- an optional per-soc VI operation to compute those values on queue setup
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/vi.c | 4
drivers/staging/media/tegra-video/vi.h | 14 ++
2 files changed, 18 insertions(+)
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index b74cdd1d1c82..5ab24977ec46 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -102,6 +102,7 @@ tegra_get_format_by_fourcc(struct tegra_vi *vi, u32 fourcc)
/*
* videobuf2 queue operations
*/
+
static int tegra_channel_queue_setup(struct vb2_queue *vq,
unsigned int *nbuffers,
unsigned int *nplanes,
@@ -117,6 +118,9 @@ static int tegra_channel_queue_setup(struct vb2_queue *vq,
sizes[0] = chan->format.sizeimage;
alloc_devs[0] = chan->vi->dev;
+ if (chan->vi->ops->channel_queue_setup)
+ chan->vi->ops->channel_queue_setup(chan);
+
return 0;
}
diff --git a/drivers/staging/media/tegra-video/vi.h
b/drivers/staging/media/tegra-video/vi.h
index 02f940f2e2eb..cadf80b742a8 100644
--- a/drivers/staging/media/tegra-video/vi.h
+++ b/drivers/staging/media/tegra-video/vi.h
@@ -47,6 +47,7 @@ struct tegra_vi_channel;
* @channel_host1x_syncpt_free: free all synchronization points
* @vi_fmt_align: modify `pix` to fit the hardware alignment
* requirements and fill image geometry
+ * @channel_queue_setup: additional operations at the end of
vb2_ops::queue_setup
* @vi_start_streaming: starts media pipeline, subdevice streaming, sets up
* VI for capture and runs capture start and capture finish
* kthreads for capturing frames to buffer and returns them back.
@@ -58,6 +59,7 @@ struct tegra_vi_ops {
int (*channel_host1x_syncpt_init)(struct tegra_vi_channel *chan);
void (*channel_host1x_syncpt_free)(struct tegra_vi_channel *chan);
void (*vi_fmt_align)(struct v4l2_pix_format *pix, unsigned int bpp);
+ void (*channel_queue_setup)(struct tegra_vi_channel *chan);
int (*vi_start_streaming)(struct vb2_queue *vq, u32 count);
void (*vi_stop_streaming)(struct vb2_queue *vq);
};
@@ -148,6 +150,12 @@ struct tegra_vi {
* @queue: vb2 buffers queue
* @sequence: V4L2 buffers sequence number
*
+ * @addr_offset_u: U plane base address, relative to buffer base address (only
for planar)
+ * @addr_offset_v: V plane base address, relative to buffer base address (only
for planar)
+ * @start_offset: 1st Y byte to write, relative to buffer base address (for
H/V flip)
+ * @start_offset_u: 1st U byte to write, relative to buffer base address (for
H/V flip)
+ * @start_offset_v: 1st V byte to write, relative to buffer base address (for
H/V flip)
+ *
* @capture: list of queued buffers for capture
* @start_lock: protects the capture queued list
* @done: list of capture done queued buffers
@@ -189,6 +197,12 @@ struct tegra_vi_channel {
struct vb2_queue queue;
u32 sequence;
+ unsigned int addr_offset_u;
+ unsigned int addr_offset_v;
+ unsigned int start_offset;
+ unsigned int start_offset_u;
+ unsigned int start_offset_v;
+
struct list_head capture;
/* protects the capture queued list */
spinlock_t start_lock;
--
2.34.1
[PATCH v5 17/20] staging: media: tegra-video: add syncpts for Tegra20 to struct tegra_vi
In preparation to implement Tegra20 parallel video capture, add a variable
to hold the required syncpt and document all the syncpt variables.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v5
Changed in v4:
- Added review tags
Changed in v3:
- recycle mw_ack_sp[0] instead of adding out_sp
No changes in v2
---
drivers/staging/media/tegra-video/vi.h | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/tegra-video/vi.h
b/drivers/staging/media/tegra-video/vi.h
index dfb5870b1411..02f940f2e2eb 100644
--- a/drivers/staging/media/tegra-video/vi.h
+++ b/drivers/staging/media/tegra-video/vi.h
@@ -117,11 +117,13 @@ struct tegra_vi {
* @vi: Tegra video input device structure
* @frame_start_sp: host1x syncpoint pointer to synchronize programmed capture
* start condition with hardware frame start events through host1x
- * syncpoint counters.
+ * syncpoint counters. (Tegra210)
* @mw_ack_sp: host1x syncpoint pointer to synchronize programmed memory write
* ack trigger condition with hardware memory write done at end of
- * frame through host1x syncpoint counters.
+ * frame through host1x syncpoint counters (On Tegra20 used for the
+ * OUT_1 syncpt)
* @sp_incr_lock: protects cpu syncpoint increment.
+ * @next_out_sp_idx: next expected value for mw_ack_sp[0], i.e. OUT_1 (Tegra20)
*
* @kthread_start_capture: kthread to start capture of single frame when
* vb buffer is available. This thread programs VI CSI hardware
@@ -175,6 +177,7 @@ struct tegra_vi_channel {
struct host1x_syncpt *mw_ack_sp[GANG_PORTS_MAX];
/* protects the cpu syncpoint increment */
spinlock_t sp_incr_lock[GANG_PORTS_MAX];
+ u32 next_out_sp_idx;
struct task_struct *kthread_start_capture;
wait_queue_head_t start_wait;
--
2.34.1
[PATCH v5 16/20] staging: media: tegra-video: move syncpt init/free to a per-soc op
tegra_channel_host1x_syncpt_init() gets the host1x syncpts needed for the
Tegra210 implementation, and tegra_channel_host1x_syncpts_free() puts
them.
Tegra20 needs to get and put a different syncpt. In preparation for adding
Tegra20 support, move these functions to new ops in the soc-specific
`struct tegra_vi_ops` .
No functional changes.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/tegra210.c | 52
drivers/staging/media/tegra-video/vi.c | 52 ++--
drivers/staging/media/tegra-video/vi.h | 5 ++
3 files changed, 60 insertions(+), 49 deletions(-)
diff --git a/drivers/staging/media/tegra-video/tegra210.c
b/drivers/staging/media/tegra-video/tegra210.c
index b4fcd4e93b8c..da99f19a39e7 100644
--- a/drivers/staging/media/tegra-video/tegra210.c
+++ b/drivers/staging/media/tegra-video/tegra210.c
@@ -179,6 +179,56 @@ static u32 vi_csi_read(struct tegra_vi_channel *chan, u8
portno,
/*
* Tegra210 VI channel capture operations
*/
+
+static int tegra210_channel_host1x_syncpt_init(struct tegra_vi_channel *chan)
+{
+ struct tegra_vi *vi = chan->vi;
+ unsigned long flags = HOST1X_SYNCPT_CLIENT_MANAGED;
+ struct host1x_syncpt *fs_sp;
+ struct host1x_syncpt *mw_sp;
+ int ret, i;
+
+ for (i = 0; i < chan->numgangports; i++) {
+ fs_sp = host1x_syncpt_request(>client, flags);
+ if (!fs_sp) {
+ dev_err(vi->dev, "failed to request frame start
syncpoint\n");
+ ret = -ENOMEM;
+ goto free_syncpts;
+ }
+
+ mw_sp = host1x_syncpt_request(>client, flags);
+ if (!mw_sp) {
+ dev_err(vi->dev, "failed to request memory ack
syncpoint\n");
+ host1x_syncpt_put(fs_sp);
+ ret = -ENOMEM;
+ goto free_syncpts;
+ }
+
+ chan->frame_start_sp[i] = fs_sp;
+ chan->mw_ack_sp[i] = mw_sp;
+ spin_lock_init(>sp_incr_lock[i]);
+ }
+
+ return 0;
+
+free_syncpts:
+ for (i = 0; i < chan->numgangports; i++) {
+ host1x_syncpt_put(chan->mw_ack_sp[i]);
+ host1x_syncpt_put(chan->frame_start_sp[i]);
+ }
+ return ret;
+}
+
+static void tegra210_channel_host1x_syncpt_free(struct tegra_vi_channel *chan)
+{
+ int i;
+
+ for (i = 0; i < chan->numgangports; i++) {
+ host1x_syncpt_put(chan->mw_ack_sp[i]);
+ host1x_syncpt_put(chan->frame_start_sp[i]);
+ }
+}
+
static void tegra210_fmt_align(struct v4l2_pix_format *pix, unsigned int bpp)
{
unsigned int min_bpl;
@@ -753,6 +803,8 @@ static const struct tegra_video_format
tegra210_video_formats[] = {
/* Tegra210 VI operations */
static const struct tegra_vi_ops tegra210_vi_ops = {
+ .channel_host1x_syncpt_init = tegra210_channel_host1x_syncpt_init,
+ .channel_host1x_syncpt_free = tegra210_channel_host1x_syncpt_free,
.vi_fmt_align = tegra210_fmt_align,
.vi_start_streaming = tegra210_vi_start_streaming,
.vi_stop_streaming = tegra210_vi_stop_streaming,
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index 8df233049c81..b74cdd1d1c82 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -1062,21 +1062,11 @@ static int vi_fmts_bitmap_init(struct tegra_vi_channel
*chan)
return 0;
}
-static void tegra_channel_host1x_syncpts_free(struct tegra_vi_channel *chan)
-{
- int i;
-
- for (i = 0; i < chan->numgangports; i++) {
- host1x_syncpt_put(chan->mw_ack_sp[i]);
- host1x_syncpt_put(chan->frame_start_sp[i]);
- }
-}
-
static void tegra_channel_cleanup(struct tegra_vi_channel *chan)
{
v4l2_ctrl_handler_free(>ctrl_handler);
media_entity_cleanup(>video.entity);
- tegra_channel_host1x_syncpts_free(chan);
+ chan->vi->ops->channel_host1x_syncpt_free(chan);
mutex_destroy(>video_lock);
}
@@ -1094,42 +1084,6 @@ void tegra_channels_cleanup(struct tegra_vi *vi)
}
}
-static int tegra_channel_host1x_syncpt_init(struct tegra_vi_channel *chan)
-{
- struct tegra_vi *vi = chan->vi;
- unsigned long flags = HOST1X_SYNCPT_CLIENT_MANAGED;
- struct host1x_syncpt *fs_sp;
- struct host1x_syncpt *mw_sp;
- int ret, i;
-
- for (i = 0; i < chan->numgangports; i++) {
- fs_sp = host1x_syncpt_request(>client, flags);
- if (!fs_sp) {
- dev_err(vi->dev, "failed to request frame start
syncpoint\n&qu
[PATCH v5 14/20] staging: media: tegra-video: move MIPI calibration calls from VI to CSI
The CSI module does not handle all the MIPI lane calibration procedure,
leaving a small part of it to the VI module. In doing this,
tegra_channel_enable_stream() (vi.c) manipulates the private data of the
upstream subdev casting it to struct 'tegra_csi_channel', which will be
wrong after introducing a VIP (parallel video input) channel.
This prevents adding support for the VIP module. It also breaks the
logical isolation between modules.
Since the lane calibration requirement does not exist in the parallel input
module, moving the calibration function to a per-module op is not
optimal. Instead move the calibration procedure in the CSI module, together
with the rest of the calibration procedures. After this change,
tegra_channel_enable_stream() just calls v4l2_subdev_call() to ask for a
stream start/stop to the CSI module, which in turn knows all the
CSI-specific details to implement it.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/csi.c | 44
drivers/staging/media/tegra-video/vi.c | 54 ++---
2 files changed, 48 insertions(+), 50 deletions(-)
diff --git a/drivers/staging/media/tegra-video/csi.c
b/drivers/staging/media/tegra-video/csi.c
index 9a03d5ccdf3c..b93fc879ef3a 100644
--- a/drivers/staging/media/tegra-video/csi.c
+++ b/drivers/staging/media/tegra-video/csi.c
@@ -328,12 +328,42 @@ static int tegra_csi_enable_stream(struct v4l2_subdev
*subdev)
}
csi_chan->pg_mode = chan->pg_mode;
+
+ /*
+* Tegra CSI receiver can detect the first LP to HS transition.
+* So, start the CSI stream-on prior to sensor stream-on and
+* vice-versa for stream-off.
+*/
ret = csi->ops->csi_start_streaming(csi_chan);
if (ret < 0)
goto finish_calibration;
+ if (csi_chan->mipi) {
+ struct v4l2_subdev *src_subdev;
+ /*
+* TRM has incorrectly documented to wait for done status from
+* calibration logic after CSI interface power on.
+* As per the design, calibration results are latched and
applied
+* to the pads only when the link is in LP11 state which will
happen
+* during the sensor stream-on.
+* CSI subdev stream-on triggers start of MIPI pads calibration.
+* Wait for calibration to finish here after sensor subdev
stream-on.
+*/
+ src_subdev = tegra_channel_get_remote_source_subdev(chan);
+ ret = v4l2_subdev_call(src_subdev, video, s_stream, true);
+ err = tegra_mipi_finish_calibration(csi_chan->mipi);
+
+ if (ret < 0 && ret != -ENOIOCTLCMD)
+ goto disable_csi_stream;
+
+ if (err < 0)
+ dev_warn(csi->dev, "MIPI calibration failed: %d\n",
err);
+ }
+
return 0;
+disable_csi_stream:
+ csi->ops->csi_stop_streaming(csi_chan);
finish_calibration:
if (csi_chan->mipi)
tegra_mipi_finish_calibration(csi_chan->mipi);
@@ -352,10 +382,24 @@ static int tegra_csi_enable_stream(struct v4l2_subdev
*subdev)
static int tegra_csi_disable_stream(struct v4l2_subdev *subdev)
{
+ struct tegra_vi_channel *chan = v4l2_get_subdev_hostdata(subdev);
struct tegra_csi_channel *csi_chan = to_csi_chan(subdev);
struct tegra_csi *csi = csi_chan->csi;
int err;
+ /*
+* Stream-off subdevices in reverse order to stream-on.
+* Remote source subdev in TPG mode is same as CSI subdev.
+*/
+ if (csi_chan->mipi) {
+ struct v4l2_subdev *src_subdev;
+
+ src_subdev = tegra_channel_get_remote_source_subdev(chan);
+ err = v4l2_subdev_call(src_subdev, video, s_stream, false);
+ if (err < 0 && err != -ENOIOCTLCMD)
+ dev_err_probe(csi->dev, err, "source subdev stream off
failed\n");
+ }
+
csi->ops->csi_stop_streaming(csi_chan);
if (csi_chan->mipi) {
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index b88532d8d2c9..c76c2a404889 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -197,49 +197,15 @@ tegra_channel_get_remote_source_subdev(struct
tegra_vi_channel *chan)
static int tegra_channel_enable_stream(struct tegra_vi_channel *chan)
{
- struct v4l2_subdev *csi_subdev, *src_subdev;
- struct tegra_csi_channel *csi_chan;
- int ret, err;
+ struct v4l2_subdev *subdev;
+ int ret;
- /*
-* Tegra CSI receiver can detect the first LP to HS transition.
-
[PATCH v5 15/20] staging: media: tegra-video: add a per-soc enable/disable op
The Tegra20 VI needs an additional operation to enable the VI, add an
operation for that.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/vi.c | 7 +++
drivers/staging/media/tegra-video/vi.h | 4
2 files changed, 11 insertions(+)
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index c76c2a404889..8df233049c81 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -1950,6 +1950,9 @@ static int tegra_vi_probe(struct platform_device *pdev)
vi->client.ops = _client_ops;
vi->client.dev = >dev;
+ if (vi->ops->vi_enable)
+ vi->ops->vi_enable(vi, true);
+
ret = host1x_client_register(>client);
if (ret < 0) {
dev_err(>dev,
@@ -1960,6 +1963,8 @@ static int tegra_vi_probe(struct platform_device *pdev)
return 0;
rpm_disable:
+ if (vi->ops->vi_enable)
+ vi->ops->vi_enable(vi, false);
pm_runtime_disable(>dev);
return ret;
}
@@ -1976,6 +1981,8 @@ static int tegra_vi_remove(struct platform_device *pdev)
return err;
}
+ if (vi->ops->vi_enable)
+ vi->ops->vi_enable(vi, false);
pm_runtime_disable(>dev);
return 0;
diff --git a/drivers/staging/media/tegra-video/vi.h
b/drivers/staging/media/tegra-video/vi.h
index b424c967c6f2..886b10e7d723 100644
--- a/drivers/staging/media/tegra-video/vi.h
+++ b/drivers/staging/media/tegra-video/vi.h
@@ -37,8 +37,11 @@ enum tegra_vi_pg_mode {
TEGRA_VI_PG_PATCH,
};
+struct tegra_vi;
+
/**
* struct tegra_vi_ops - Tegra VI operations
+ * @vi_enable: soc-specific operations needed to enable/disable the VI
peripheral
* @vi_fmt_align: modify `pix` to fit the hardware alignment
* requirements and fill image geometry
* @vi_start_streaming: starts media pipeline, subdevice streaming, sets up
@@ -48,6 +51,7 @@ enum tegra_vi_pg_mode {
* back any queued buffers.
*/
struct tegra_vi_ops {
+ int (*vi_enable)(struct tegra_vi *vi, bool on);
void (*vi_fmt_align)(struct v4l2_pix_format *pix, unsigned int bpp);
int (*vi_start_streaming)(struct vb2_queue *vq, u32 count);
void (*vi_stop_streaming)(struct vb2_queue *vq);
--
2.34.1
[PATCH v5 13/20] staging: media: tegra-video: move default format to soc-specific data
The tegra_default_format in vi.c is specific to Tegra210 CSI.
In preparation for adding Tegra20 VIP support, move the default format to a
new field in the soc-specific `struct tegra_vi_soc`. Instead of an entire
format struct, only store a pointer to an item in the existing format
array.
No functional changes. The format pointed to is the same that used to be in
vi.c.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
Changed in v5:
- Minor update after removal of "staging: media: tegra-video: fix
.vidioc_enum_fmt_vid_cap to return all formats" patch
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/tegra210.c | 2 ++
drivers/staging/media/tegra-video/vi.c | 13 ++---
drivers/staging/media/tegra-video/vi.h | 2 ++
3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/media/tegra-video/tegra210.c
b/drivers/staging/media/tegra-video/tegra210.c
index d19ff6b49ae8..b4fcd4e93b8c 100644
--- a/drivers/staging/media/tegra-video/tegra210.c
+++ b/drivers/staging/media/tegra-video/tegra210.c
@@ -766,8 +766,10 @@ const struct tegra_vi_soc tegra210_vi_soc = {
.hw_revision = 3,
.vi_max_channels = 6,
#if IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG)
+ .default_video_format = _video_formats[0],
.vi_max_clk_hz = 49920,
#else
+ .default_video_format = _video_formats[4],
.vi_max_clk_hz = 99840,
#endif
};
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index 35d7cda1373f..b88532d8d2c9 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -45,15 +45,6 @@ struct tegra_vi_graph_entity {
struct v4l2_subdev *subdev;
};
-static const struct tegra_video_format tegra_default_format = {
- .img_dt = TEGRA_IMAGE_DT_RAW10,
- .bit_width = 10,
- .code = MEDIA_BUS_FMT_SRGGB10_1X10,
- .bpp = 2,
- .img_fmt = TEGRA_IMAGE_FORMAT_DEF,
- .fourcc = V4L2_PIX_FMT_SRGGB10,
-};
-
static inline struct tegra_vi *
host1x_client_to_vi(struct host1x_client *client)
{
@@ -1103,7 +1094,7 @@ static int vi_fmts_bitmap_init(struct tegra_vi_channel
*chan)
* there are no matched formats.
*/
if (!match_code) {
- match_code = tegra_default_format.code;
+ match_code = chan->vi->soc->default_video_format->code;
index = tegra_get_format_idx_by_code(chan->vi, match_code, 0);
if (WARN_ON(index < 0))
return -EINVAL;
@@ -1200,7 +1191,7 @@ static int tegra_channel_init(struct tegra_vi_channel
*chan)
init_waitqueue_head(>done_wait);
/* initialize the video format */
- chan->fmtinfo = _default_format;
+ chan->fmtinfo = chan->vi->soc->default_video_format;
chan->format.pixelformat = chan->fmtinfo->fourcc;
chan->format.colorspace = V4L2_COLORSPACE_SRGB;
chan->format.field = V4L2_FIELD_NONE;
diff --git a/drivers/staging/media/tegra-video/vi.h
b/drivers/staging/media/tegra-video/vi.h
index 213955c7545d..b424c967c6f2 100644
--- a/drivers/staging/media/tegra-video/vi.h
+++ b/drivers/staging/media/tegra-video/vi.h
@@ -58,6 +58,7 @@ struct tegra_vi_ops {
*
* @video_formats: supported video formats
* @nformats: total video formats
+ * @default_video_format: default video format (pointer to a @video_formats
item)
* @ops: vi operations
* @hw_revision: VI hw_revision
* @vi_max_channels: supported max streaming channels
@@ -66,6 +67,7 @@ struct tegra_vi_ops {
struct tegra_vi_soc {
const struct tegra_video_format *video_formats;
const unsigned int nformats;
+ const struct tegra_video_format *default_video_format;
const struct tegra_vi_ops *ops;
u32 hw_revision;
unsigned int vi_max_channels;
--
2.34.1
[PATCH v5 12/20] staging: media: tegra-video: move tegra_channel_fmt_align to a per-soc op
tegra_channel_fmt_align() takes care of the size constraints, alignment and
rounding requirements of the Tegra210 VI peripheral. Tegra20 has different
constraints.
In preparation for adding Tegra20 support, move this function to a new op
in the soc-specific `struct tegra_vi_ops` .
Also move to tegra210.c the T210-specific defines used in the moved code.
No functional changes.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/tegra210.c | 36 ++
drivers/staging/media/tegra-video/vi.c | 40 +++-
drivers/staging/media/tegra-video/vi.h | 9 ++---
3 files changed, 44 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/media/tegra-video/tegra210.c
b/drivers/staging/media/tegra-video/tegra210.c
index d58370a84737..d19ff6b49ae8 100644
--- a/drivers/staging/media/tegra-video/tegra210.c
+++ b/drivers/staging/media/tegra-video/tegra210.c
@@ -17,6 +17,13 @@
#include "csi.h"
#include "vi.h"
+#define TEGRA210_MIN_WIDTH 32U
+#define TEGRA210_MAX_WIDTH 32768U
+#define TEGRA210_MIN_HEIGHT32U
+#define TEGRA210_MAX_HEIGHT32768U
+
+#define SURFACE_ALIGN_BYTES64
+
#define TEGRA_VI_SYNCPT_WAIT_TIMEOUT msecs_to_jiffies(200)
/* Tegra210 VI registers */
@@ -172,6 +179,34 @@ static u32 vi_csi_read(struct tegra_vi_channel *chan, u8
portno,
/*
* Tegra210 VI channel capture operations
*/
+static void tegra210_fmt_align(struct v4l2_pix_format *pix, unsigned int bpp)
+{
+ unsigned int min_bpl;
+ unsigned int max_bpl;
+ unsigned int bpl;
+
+ /*
+* The transfer alignment requirements are expressed in bytes.
+* Clamp the requested width and height to the limits.
+*/
+ pix->width = clamp(pix->width, TEGRA210_MIN_WIDTH, TEGRA210_MAX_WIDTH);
+ pix->height = clamp(pix->height, TEGRA210_MIN_HEIGHT,
TEGRA210_MAX_HEIGHT);
+
+ /* Clamp the requested bytes per line value. If the maximum bytes per
+* line value is zero, the module doesn't support user configurable
+* line sizes. Override the requested value with the minimum in that
+* case.
+*/
+ min_bpl = pix->width * bpp;
+ max_bpl = rounddown(TEGRA210_MAX_WIDTH, SURFACE_ALIGN_BYTES);
+ bpl = roundup(pix->bytesperline, SURFACE_ALIGN_BYTES);
+
+ pix->bytesperline = clamp(bpl, min_bpl, max_bpl);
+ pix->sizeimage = pix->bytesperline * pix->height;
+ if (pix->pixelformat == V4L2_PIX_FMT_NV16)
+ pix->sizeimage *= 2;
+}
+
static int tegra_channel_capture_setup(struct tegra_vi_channel *chan,
u8 portno)
{
@@ -718,6 +753,7 @@ static const struct tegra_video_format
tegra210_video_formats[] = {
/* Tegra210 VI operations */
static const struct tegra_vi_ops tegra210_vi_ops = {
+ .vi_fmt_align = tegra210_fmt_align,
.vi_start_streaming = tegra210_vi_start_streaming,
.vi_stop_streaming = tegra210_vi_stop_streaming,
};
diff --git a/drivers/staging/media/tegra-video/vi.c
b/drivers/staging/media/tegra-video/vi.c
index db98d06351b4..35d7cda1373f 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -473,36 +473,6 @@ static int tegra_channel_get_format(struct file *file,
void *fh,
return 0;
}
-static void tegra_channel_fmt_align(struct tegra_vi_channel *chan,
- struct v4l2_pix_format *pix,
- unsigned int bpp)
-{
- unsigned int min_bpl;
- unsigned int max_bpl;
- unsigned int bpl;
-
- /*
-* The transfer alignment requirements are expressed in bytes.
-* Clamp the requested width and height to the limits.
-*/
- pix->width = clamp(pix->width, TEGRA_MIN_WIDTH, TEGRA_MAX_WIDTH);
- pix->height = clamp(pix->height, TEGRA_MIN_HEIGHT, TEGRA_MAX_HEIGHT);
-
- /* Clamp the requested bytes per line value. If the maximum bytes per
-* line value is zero, the module doesn't support user configurable
-* line sizes. Override the requested value with the minimum in that
-* case.
-*/
- min_bpl = pix->width * bpp;
- max_bpl = rounddown(TEGRA_MAX_WIDTH, SURFACE_ALIGN_BYTES);
- bpl = roundup(pix->bytesperline, SURFACE_ALIGN_BYTES);
-
- pix->bytesperline = clamp(bpl, min_bpl, max_bpl);
- pix->sizeimage = pix->bytesperline * pix->height;
- if (pix->pixelformat == V4L2_PIX_FMT_NV16)
- pix->sizeimage *= 2;
-}
-
static int __tegra_channel_try_format(struct tegra_vi_channel *chan,
struct v4l2_pix_format *pix)
{
@@ -578,7 +548,7 @@ static int __tegra_channel_try_format(st
[PATCH v5 11/20] staging: media: tegra-video: Kconfig: allow TPG only on Tegra210
We are about to add support for the Tegra20 parallel video capture, which has no TPG. In preparation for that, limit the VIDEO_TEGRA_TPG option to Tegra210 which is the only implementation currently provided by this driver. Signed-off-by: Luca Ceresoli Reviewed-by: Dmitry Osipenko --- No changes in v5 Changed in v4: - Added review tags No changes in v3 No changes in v2 --- drivers/staging/media/tegra-video/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/media/tegra-video/Kconfig b/drivers/staging/media/tegra-video/Kconfig index df1b2cff2417..c53441822fdf 100644 --- a/drivers/staging/media/tegra-video/Kconfig +++ b/drivers/staging/media/tegra-video/Kconfig @@ -15,5 +15,6 @@ config VIDEO_TEGRA config VIDEO_TEGRA_TPG bool "NVIDIA Tegra VI driver TPG mode" depends on VIDEO_TEGRA + depends on ARCH_TEGRA_210_SOC help Say yes here to enable Tegra internal TPG mode -- 2.34.1
[PATCH v5 10/20] staging: media: tegra-video: remove unneeded include
There is only a pointer reference to struct tegra_vi in video.h, thus vi.h
is not needed.
Signed-off-by: Luca Ceresoli
Reviewed-by: Dmitry Osipenko
---
No changes in v5
Changed in v4:
- Added review tags
No changes in v3
No changes in v2
---
drivers/staging/media/tegra-video/video.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/media/tegra-video/video.h
b/drivers/staging/media/tegra-video/video.h
index fadaf2189dc9..1e9be1474a9c 100644
--- a/drivers/staging/media/tegra-video/video.h
+++ b/drivers/staging/media/tegra-video/video.h
@@ -12,7 +12,6 @@
#include
#include "vi.h"
-#include "csi.h"
struct tegra_video_device {
struct v4l2_device v4l2_dev;
--
2.34.1
