Hi Vladimir,
[Added Andy Yan to Cc:, because imx-hdmi->dw-hdmi]
Am Montag, den 01.12.2014, 17:39 +0200 schrieb Vladimir Zapolskiy:
> On 01.12.2014 17:11, Philipp Zabel wrote:
> > Am Montag, den 01.12.2014, 16:54 +0200 schrieb Vladimir Zapolskiy:
> >> Hi Philipp and Shawn,
> >>
> >> On 15.11.2014 19:49, Vladimir Zapolskiy wrote:
> >>> Provide information about how to bind internal iMX6Q/DL HDMI DDC I2C
> >>> master controller. The property is set as optional one, because iMX6
> >>> HDMI DDC bus may be represented by one of general purpose I2C busses
> >>> found on SoC.
> >>>
> >>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@xxxxxxxxxx>
> >>> Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx>
> >>> Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> >>> Cc: Shawn Guo <shawn.guo@xxxxxxxxxx>
> >>> Cc: devicetree@xxxxxxxxxxxxxxx
> >>> Cc: linux-media@xxxxxxxxxxxxxxx
> >>> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> >>> Cc: linux-i2c@xxxxxxxxxxxxxxx
> >>> ---
> >>> Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt | 10 +++++++++-
> >>> 1 file changed, 9 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> >>> index 1b756cf..43c8924 100644
> >>> --- a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> >>> +++ b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> >>> @@ -10,6 +10,8 @@ Required properties:
> >>> - #address-cells : should be <1>
> >>> - #size-cells : should be <0>
> >>> - compatible : should be "fsl,imx6q-hdmi" or "fsl,imx6dl-hdmi".
> >>> + If internal HDMI DDC I2C master controller is supposed to be used,
> >>> + then "simple-bus" should be added to compatible value.
> >>> - gpr : should be <&gpr>.
> >>> The phandle points to the iomuxc-gpr region containing the HDMI
> >>> multiplexer control register.
> >>> @@ -22,6 +24,7 @@ Required properties:
> >>>
> >>> Optional properties:
> >>> - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
> >>> + - ddc: internal HDMI DDC I2C master controller
> >>>
> >>> example:
> >>>
> >>> @@ -32,7 +35,7 @@ example:
> >>> hdmi: hdmi@0120000 {
> >>> #address-cells = <1>;
> >>> #size-cells = <0>;
> >>> - compatible = "fsl,imx6q-hdmi";
> >>> + compatible = "fsl,imx6q-hdmi", "simple-bus";
> >>> reg = <0x00120000 0x9000>;
> >>> interrupts = <0 115 0x04>;
> >>> gpr = <&gpr>;
> >>> @@ -40,6 +43,11 @@ example:
> >>> clock-names = "iahb", "isfr";
> >>> ddc-i2c-bus = <&i2c2>;
> >>>
> >>> + hdmi_ddc: ddc {
> >>> + compatible = "fsl,imx6q-hdmi-ddc";
> >>> + status = "disabled";
> >>> + };
> >>> +
> >>> port@0 {
> >>> reg = <0>;
> >>>
> >>>
> >>
> >> knowing in advance that I2C framework lacks a graceful support of non
> >> fully compliant I2C devices, do you have any objections to the proposed
> >> iMX HDMI DTS change?
> >
> > I'm not sure about this. Have you seen "drm: Decouple EDID parsing from
> > I2C adapter"? I feel like in the absence of a ddc-i2c-bus property the
> > imx-hdmi/dw-hdmi driver should try to use the internal HDMI DDC I2C
> > master controller, bypassing the I2C framework altogether.
> >
> My idea is exactly not to bypass the I2C framework, briefly the
> rationale is that
> * it allows to reuse I2C UAPI/tools naturally applied to the internal
> iMX HDMI DDC bus,
> * it allows to use iMX HDMI DDC bus as an additional feature-limited I2C
> bus on SoC (who knows, I absolutely won't be surprised, if anyone needs
> it on practice),
> * if an HDMI controller supports an external I2C bus, the integration
> with HDMI DDC bus driver based on I2C framework is seamless.
>
> However I agree that the selected approach may look odd, the question is
> if the oddness comes from the technical side or from the fact that
> nobody has done it before this way.
>
> I'm open to any critique, if the proposal of creating an I2C bus from
> HDMI DDC bus is lame, then I suppose the shared iMX HDMI DDC bus driver
> should be converted to something formless and internally used by
> imx-hdmi. The negative side-effects of such a change from my point of
> view are
> * more or less natural modularity is lost,
> * a number of I2C framework API/functions should be copy-pasted to the
> updated HDMI DDC bus driver to support a subset of I2C read/write
> transactions.
If Wolfram is happy to accomodate such feature limited, 'I2C master'
devices in i2c/drivers/busses in principle, I won't disagree.
But then it should be abstracted properly. The dw-hdmi-tx core on i.MX6
has the DDC I2C master register space at 0x7e00 - 0x7e12. What are the
offsets on the Rockchip version? If the "simple-bus" compatible is to be
set on the hdmi driver, the ddc driver should do its own register
access, and therefore needs a reg property. I suspect for the ddc-i2cm
we should get away with a common compatible like "snps,dw-hdmi-i2c".
hdmi: hdmi@120000 {
/* ... */
compatible = "fsl,imx6q-hdmi", "snps,dw-hdmi";
ddc-i2c-bus = <&hdmi_ddc>;
hdmi_ddc: i2c@127e00 {
compatible = "snps,dw-hdmi-i2c";
reg = <0x1207e00 0x13>
};
/* could add phy-i2cm, cec, ... here */
};
Also there's an i2c bus for communication with the phy at 0x3020 -
0x3032, should that be handled in a similar way, then?
Do we need to make the hdmi driver a proper interrupt controller? At
least the two i2c masters have two interrupts each, "done" and "error".
regards
Philipp
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
