Re: [PATCH 3/3] drm/bridge: chrontel-ch7033: Add a new driver
Hello Laurent,
On Wed, 2020-01-08 at 17:20 +0200, Laurent Pinchart wrote:
> Hi Lubomir,
>
> Thank you for the patch.
>
> On Fri, Dec 20, 2019 at 08:49:14AM +0100, Lubomir Rintel wrote:
> > This is a driver for video encoder with VGA and DVI/HDMI outputs.
> >
> > There is no documentation for the chip -- the operation was guessed from
> > what was sniffed on a Dell Wyse 3020 ThinOS terminal, the register names
> > come from the ch7035 driver in Mediatek's GPL code dump.
> >
> > Only bare minimum is implemented -- no fancy stuff, such as scaling. That
> > would only worsen our misery. We don't load the firmware and we don't need
> > to even bother enabling the MCU. There are probably no distributable
> > firmware images anyway.
> >
> > Just like the tda998x driver, this one uses the component framework and
> > adds an encoder on component bind, so that it works with the Armada DRM
> > driver.
>
> Any chance the Armada DRM driver could use of_drm_find_bridge() to avoid
> having to use the component framework everywhere ?
Thanks for the response.
I've brought this up previously and the Armada DRM maintainer pointed
out that drm_bridge_remove() is essentialy a surprise removal [1].
Unlike the component unbind, it doesn't let the crtc driver know that
the bridge is gone. Understandably, he wasn't too fond of the idea of
losing the ability to unload the bridge driver and not much has changed
since.
[1] https://www.spinics.net/lists/dri-devel/msg201927.html
This shouldn't be too much of a problem (other than extra LOC in the
driver), because the bridge is registered outside the component bind
path, therefore the drivers that choose not to use the component
framework can use just the bridge alone just fine.
Thank you
Lubo
> > Tested with a handful of monitors ranging from 1024x768@75 to 1400x1050@60,
> > with VGA as well as DVI.
> >
> > Signed-off-by: Lubomir Rintel
> > ---
> > drivers/gpu/drm/bridge/Kconfig | 10 +
> > drivers/gpu/drm/bridge/Makefile | 1 +
> > drivers/gpu/drm/bridge/chrontel-ch7033.c | 722 +++
> > 3 files changed, 733 insertions(+)
> > create mode 100644 drivers/gpu/drm/bridge/chrontel-ch7033.c
> >
> > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> > index 34362976cd6fd..9456ea968c5b7 100644
> > --- a/drivers/gpu/drm/bridge/Kconfig
> > +++ b/drivers/gpu/drm/bridge/Kconfig
> > @@ -37,6 +37,16 @@ config DRM_CDNS_DSI
> > Support Cadence DPI to DSI bridge. This is an internal
> > bridge and is meant to be directly embedded in a SoC.
> >
> > +config DRM_CHRONTEL_CH7033
> > + tristate "Chrontel CH7033 Video Encoder"
> > + depends on OF
> > + select DRM_KMS_HELPER
> > + help
> > + Enable support for the Chrontel CH7033 VGA/DVI/HDMI Encoder, as
> > + found in the Dell Wyse 3020 thin client.
> > +
> > + If in doubt, say "N".
> > +
> > config DRM_DUMB_VGA_DAC
> > tristate "Dumb VGA DAC Bridge support"
> > depends on OF
> > diff --git a/drivers/gpu/drm/bridge/Makefile
> > b/drivers/gpu/drm/bridge/Makefile
> > index 4934fcf5a6f82..74a9ab2f17468 100644
> > --- a/drivers/gpu/drm/bridge/Makefile
> > +++ b/drivers/gpu/drm/bridge/Makefile
> > @@ -1,6 +1,7 @@
> > # SPDX-License-Identifier: GPL-2.0
> > obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> > obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
> > +obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
> > obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
> > obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
> > obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) +=
> > megachips-stdp-ge-b850v3-fw.o
> > diff --git a/drivers/gpu/drm/bridge/chrontel-ch7033.c
> > b/drivers/gpu/drm/bridge/chrontel-ch7033.c
> > new file mode 100644
> > index 0..a3b63984226a4
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/chrontel-ch7033.c
> > @@ -0,0 +1,722 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Chrontel CH7033 Video Encoder Driver
> > + *
> > + * Copyright (C) 2019 Lubomir Rintel
> > + */
> > +
> > +#include
> > +#include
> > +#include
>
> Could you please sort these alphabetically ?
>
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +/* Page 0, Register 0x07 */
> > +enum {
> > + DRI_PD = BIT(3),
> > + IO_PD = BIT(5),
> > +};
> > +
> > +/* Page 0, Register 0x08 */
> > +enum {
> > + DRI_PDDRI = GENMASK(7, 4),
> > + PDDAC = GENMASK(3, 1),
> > + PANEN = BIT(0),
> > +};
> > +
> > +/* Page 0, Register 0x09 */
> > +enum {
> > + DPD = BIT(7),
> > + GCKOFF = BIT(6),
> > + TV_BP = BIT(5),
> > + SCLPD = BIT(4),
> > + SDPD= BIT(3),
> > + VGA_PD = BIT(2),
> > + HDBKPD = BIT(1),
> > + HDMI_PD = BIT(0),
> > +};
> > +
> > +/* Page 0, Register 0x0a */
> > +enum {
> > + MEMINIT = BIT(7),
>
Re: [PATCH 3/3] drm/bridge: chrontel-ch7033: Add a new driver
Hi Lubomir,
Thank you for the patch.
On Fri, Dec 20, 2019 at 08:49:14AM +0100, Lubomir Rintel wrote:
> This is a driver for video encoder with VGA and DVI/HDMI outputs.
>
> There is no documentation for the chip -- the operation was guessed from
> what was sniffed on a Dell Wyse 3020 ThinOS terminal, the register names
> come from the ch7035 driver in Mediatek's GPL code dump.
>
> Only bare minimum is implemented -- no fancy stuff, such as scaling. That
> would only worsen our misery. We don't load the firmware and we don't need
> to even bother enabling the MCU. There are probably no distributable
> firmware images anyway.
>
> Just like the tda998x driver, this one uses the component framework and
> adds an encoder on component bind, so that it works with the Armada DRM
> driver.
Any chance the Armada DRM driver could use of_drm_find_bridge() to avoid
having to use the component framework everywhere ?
> Tested with a handful of monitors ranging from 1024x768@75 to 1400x1050@60,
> with VGA as well as DVI.
>
> Signed-off-by: Lubomir Rintel
> ---
> drivers/gpu/drm/bridge/Kconfig | 10 +
> drivers/gpu/drm/bridge/Makefile | 1 +
> drivers/gpu/drm/bridge/chrontel-ch7033.c | 722 +++
> 3 files changed, 733 insertions(+)
> create mode 100644 drivers/gpu/drm/bridge/chrontel-ch7033.c
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 34362976cd6fd..9456ea968c5b7 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -37,6 +37,16 @@ config DRM_CDNS_DSI
> Support Cadence DPI to DSI bridge. This is an internal
> bridge and is meant to be directly embedded in a SoC.
>
> +config DRM_CHRONTEL_CH7033
> + tristate "Chrontel CH7033 Video Encoder"
> + depends on OF
> + select DRM_KMS_HELPER
> + help
> + Enable support for the Chrontel CH7033 VGA/DVI/HDMI Encoder, as
> + found in the Dell Wyse 3020 thin client.
> +
> + If in doubt, say "N".
> +
> config DRM_DUMB_VGA_DAC
> tristate "Dumb VGA DAC Bridge support"
> depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 4934fcf5a6f82..74a9ab2f17468 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
> +obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
> obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
> obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
> obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) +=
> megachips-stdp-ge-b850v3-fw.o
> diff --git a/drivers/gpu/drm/bridge/chrontel-ch7033.c
> b/drivers/gpu/drm/bridge/chrontel-ch7033.c
> new file mode 100644
> index 0..a3b63984226a4
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/chrontel-ch7033.c
> @@ -0,0 +1,722 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Chrontel CH7033 Video Encoder Driver
> + *
> + * Copyright (C) 2019 Lubomir Rintel
> + */
> +
> +#include
> +#include
> +#include
Could you please sort these alphabetically ?
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/* Page 0, Register 0x07 */
> +enum {
> + DRI_PD = BIT(3),
> + IO_PD = BIT(5),
> +};
> +
> +/* Page 0, Register 0x08 */
> +enum {
> + DRI_PDDRI = GENMASK(7, 4),
> + PDDAC = GENMASK(3, 1),
> + PANEN = BIT(0),
> +};
> +
> +/* Page 0, Register 0x09 */
> +enum {
> + DPD = BIT(7),
> + GCKOFF = BIT(6),
> + TV_BP = BIT(5),
> + SCLPD = BIT(4),
> + SDPD= BIT(3),
> + VGA_PD = BIT(2),
> + HDBKPD = BIT(1),
> + HDMI_PD = BIT(0),
> +};
> +
> +/* Page 0, Register 0x0a */
> +enum {
> + MEMINIT = BIT(7),
> + MEMIDLE = BIT(6),
> + MEMPD = BIT(5),
> + STOP= BIT(4),
> + LVDS_PD = BIT(3),
> + HD_DVIB = BIT(2),
> + HDCP_PD = BIT(1),
> + MCU_PD = BIT(0),
> +};
> +
> +/* Page 0, Register 0x18 */
> +enum {
> + IDF = GENMASK(7, 4),
> + INTEN = BIT(3),
> + SWAP= GENMASK(2, 0),
> +};
> +
> +enum {
> + BYTE_SWAP_RGB = 0,
> + BYTE_SWAP_RBG = 1,
> + BYTE_SWAP_GRB = 2,
> + BYTE_SWAP_GBR = 3,
> + BYTE_SWAP_BRG = 4,
> + BYTE_SWAP_BGR = 5,
> +};
> +
> +/* Page 0, Register 0x19 */
> +enum {
> + HPO_I = BIT(5),
> + VPO_I = BIT(4),
> + DEPO_I = BIT(3),
> + CRYS_EN = BIT(2),
> + GCLKFREQ= GENMASK(2, 0),
> +};
> +
> +/* Page 0, Register 0x2e */
> +enum {
> + HFLIP = BIT(7),
> + VFLIP = BIT(6),
> + DEPO_O = BIT(5),
> +
[PATCH 3/3] drm/bridge: chrontel-ch7033: Add a new driver
This is a driver for video encoder with VGA and DVI/HDMI outputs.
There is no documentation for the chip -- the operation was guessed from
what was sniffed on a Dell Wyse 3020 ThinOS terminal, the register names
come from the ch7035 driver in Mediatek's GPL code dump.
Only bare minimum is implemented -- no fancy stuff, such as scaling. That
would only worsen our misery. We don't load the firmware and we don't need
to even bother enabling the MCU. There are probably no distributable
firmware images anyway.
Just like the tda998x driver, this one uses the component framework and
adds an encoder on component bind, so that it works with the Armada DRM
driver.
Tested with a handful of monitors ranging from 1024x768@75 to 1400x1050@60,
with VGA as well as DVI.
Signed-off-by: Lubomir Rintel
---
drivers/gpu/drm/bridge/Kconfig | 10 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/chrontel-ch7033.c | 722 +++
3 files changed, 733 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/chrontel-ch7033.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 34362976cd6fd..9456ea968c5b7 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -37,6 +37,16 @@ config DRM_CDNS_DSI
Support Cadence DPI to DSI bridge. This is an internal
bridge and is meant to be directly embedded in a SoC.
+config DRM_CHRONTEL_CH7033
+ tristate "Chrontel CH7033 Video Encoder"
+ depends on OF
+ select DRM_KMS_HELPER
+ help
+ Enable support for the Chrontel CH7033 VGA/DVI/HDMI Encoder, as
+ found in the Dell Wyse 3020 thin client.
+
+ If in doubt, say "N".
+
config DRM_DUMB_VGA_DAC
tristate "Dumb VGA DAC Bridge support"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 4934fcf5a6f82..74a9ab2f17468 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
+obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) +=
megachips-stdp-ge-b850v3-fw.o
diff --git a/drivers/gpu/drm/bridge/chrontel-ch7033.c
b/drivers/gpu/drm/bridge/chrontel-ch7033.c
new file mode 100644
index 0..a3b63984226a4
--- /dev/null
+++ b/drivers/gpu/drm/bridge/chrontel-ch7033.c
@@ -0,0 +1,722 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Chrontel CH7033 Video Encoder Driver
+ *
+ * Copyright (C) 2019 Lubomir Rintel
+ */
+
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* Page 0, Register 0x07 */
+enum {
+ DRI_PD = BIT(3),
+ IO_PD = BIT(5),
+};
+
+/* Page 0, Register 0x08 */
+enum {
+ DRI_PDDRI = GENMASK(7, 4),
+ PDDAC = GENMASK(3, 1),
+ PANEN = BIT(0),
+};
+
+/* Page 0, Register 0x09 */
+enum {
+ DPD = BIT(7),
+ GCKOFF = BIT(6),
+ TV_BP = BIT(5),
+ SCLPD = BIT(4),
+ SDPD= BIT(3),
+ VGA_PD = BIT(2),
+ HDBKPD = BIT(1),
+ HDMI_PD = BIT(0),
+};
+
+/* Page 0, Register 0x0a */
+enum {
+ MEMINIT = BIT(7),
+ MEMIDLE = BIT(6),
+ MEMPD = BIT(5),
+ STOP= BIT(4),
+ LVDS_PD = BIT(3),
+ HD_DVIB = BIT(2),
+ HDCP_PD = BIT(1),
+ MCU_PD = BIT(0),
+};
+
+/* Page 0, Register 0x18 */
+enum {
+ IDF = GENMASK(7, 4),
+ INTEN = BIT(3),
+ SWAP= GENMASK(2, 0),
+};
+
+enum {
+ BYTE_SWAP_RGB = 0,
+ BYTE_SWAP_RBG = 1,
+ BYTE_SWAP_GRB = 2,
+ BYTE_SWAP_GBR = 3,
+ BYTE_SWAP_BRG = 4,
+ BYTE_SWAP_BGR = 5,
+};
+
+/* Page 0, Register 0x19 */
+enum {
+ HPO_I = BIT(5),
+ VPO_I = BIT(4),
+ DEPO_I = BIT(3),
+ CRYS_EN = BIT(2),
+ GCLKFREQ= GENMASK(2, 0),
+};
+
+/* Page 0, Register 0x2e */
+enum {
+ HFLIP = BIT(7),
+ VFLIP = BIT(6),
+ DEPO_O = BIT(5),
+ HPO_O = BIT(4),
+ VPO_O = BIT(3),
+ TE = GENMASK(2, 0),
+};
+
+/* Page 0, Register 0x2b */
+enum {
+ SWAPS = GENMASK(7, 4),
+ VFMT= GENMASK(3, 0),
+};
+
+/* Page 0, Register 0x54 */
+enum {
+ COMP_BP = BIT(7),
+ DAC_EN_T= BIT(6),
+ HWO_HDMI_HI = GENMASK(5, 3),
+ HOO_HDMI_HI = GENMASK(2, 0),
+};
+
+/* Page 0, Register 0x57 */
+enum {
+ FLDSEN
