| Message ID | 1493394792-20743-4-git-send-email-philipp.tomsich@theobroma-systems.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Philipp, Dne petek, 28. april 2017 ob 17:53:10 CEST je Philipp Tomsich napisal(a): > This commit enables RK3399 support for HDMI through the following > changes: > - adds a driverdata structure to mirror some subtle version > differences between the RK3399 VOPs and those in the RK3288 > (e.g. the pin-polarity configuration) > - configures the VOP to output 32bpp for HDMI > - handles whether a VOP can output 10BIT data or not (i.e. RGBaaa vs. > RGB888) using the driverdata structure > > And as we touch this file anyway, we also increase the size of the > framebuffer to a slightly overzealous 4K2K@32bpp. > > Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> > --- > > arch/arm/include/asm/arch-rockchip/vop_rk3288.h | 11 ++ > drivers/video/rockchip/rk_vop.c | 180 > ++++++++++++++++++++---- 2 files changed, 161 insertions(+), 30 > deletions(-) > > diff --git a/arch/arm/include/asm/arch-rockchip/vop_rk3288.h > b/arch/arm/include/asm/arch-rockchip/vop_rk3288.h index 0ce3d67..bca6860 > 100644 > --- a/arch/arm/include/asm/arch-rockchip/vop_rk3288.h > +++ b/arch/arm/include/asm/arch-rockchip/vop_rk3288.h > @@ -196,9 +196,20 @@ enum vop_modes { > #define V_DSP_DEN_POL(x) (((x) & 1) << 6) > #define V_DSP_VSYNC_POL(x) (((x) & 1) << 5) > #define V_DSP_HSYNC_POL(x) (((x) & 1) << 4) > +#define V_DSP_PIN_POL(x) (((x) & 0xf) << 4) > #define V_DSP_OUT_MODE(x) ((x) & 0xf) > > /* VOP_DSP_CTRL1 */ > +#define V_RK3399_DSP_MIPI_POL(x) ((x) << 28) > +#define V_RK3399_DSP_EDP_POL(x) ((x) << 24) > +#define V_RK3399_DSP_HDMI_POL(x) ((x) << 20) > +#define V_RK3399_DSP_LVDS_POL(x) ((x) << 16) > + > +#define M_RK3399_DSP_MIPI_POL (V_RK3399_DSP_MIPI_POL(0xf)) > +#define M_RK3399_DSP_EDP_POL (V_RK3399_DSP_EDP_POL(0xf)) > +#define M_RK3399_DSP_HDMI_POL (V_RK3399_DSP_HDMI_POL(0xf)) > +#define M_RK3399_DSP_LVDS_POL (V_RK3399_DSP_LVDS_POL(0xf)) > + > #define M_DSP_LAYER3_SEL (3 << 14) > #define M_DSP_LAYER2_SEL (3 << 12) > #define M_DSP_LAYER1_SEL (3 << 10) > diff --git a/drivers/video/rockchip/rk_vop.c > b/drivers/video/rockchip/rk_vop.c index bc02f80..01b4b6a 100644 > --- a/drivers/video/rockchip/rk_vop.c > +++ b/drivers/video/rockchip/rk_vop.c > @@ -5,6 +5,8 @@ > * SPDX-License-Identifier: GPL-2.0+ > */ > > +#define DEBUG > + You probably forgot to remove that? Regards, Jernej > #include <common.h> > #include <clk.h> > #include <display.h> > @@ -28,11 +30,78 @@ > > DECLARE_GLOBAL_DATA_PTR; > > +enum vop_pol { > + HSYNC_POSITIVE = 0, > + VSYNC_POSITIVE = 1, > + DEN_NEGATIVE = 2, > + DCLK_INVERT = 3 > +}; > + > struct rk_vop_priv { > struct rk3288_vop *regs; > struct rk3288_grf *grf; > }; > > +enum vop_features { > + VOP_FEATURE_OUTPUT_10BIT = (1 << 0), > +}; > + > +struct rkvop_driverdata { > + /* configuration */ > + u32 features; > + /* block-specific setters/getters */ > + void (*set_pin_polarity)(struct udevice *, enum vop_modes, u32); > +}; > + > +static void rk3288_set_pin_polarity(struct udevice *dev, > + enum vop_modes mode, u32 polarity) > +{ > + struct rk_vop_priv *priv = dev_get_priv(dev); > + struct rk3288_vop *regs = priv->regs; > + > + /* The RK3328 VOP (v3.1) has its polarity configuration in ctrl0 */ > + clrsetbits_le32(®s->dsp_ctrl0, > + M_DSP_DCLK_POL | M_DSP_DEN_POL | > + M_DSP_VSYNC_POL | M_DSP_HSYNC_POL, > + V_DSP_PIN_POL(polarity)); > +} > + > +static void rk3399_set_pin_polarity(struct udevice *dev, > + enum vop_modes mode, u32 polarity) > +{ > + struct rk_vop_priv *priv = dev_get_priv(dev); > + struct rk3288_vop *regs = priv->regs; > + > + /* > + * The RK3399 VOPs (v3.5 and v3.6) require a per-mode setting of > + * the polarity configuration (in ctrl1). > + */ > + switch (mode) { > + case VOP_MODE_HDMI: > + clrsetbits_le32(®s->dsp_ctrl1, > + M_RK3399_DSP_HDMI_POL, > + V_RK3399_DSP_HDMI_POL(polarity)); > + break; > + > + case VOP_MODE_EDP: > + clrsetbits_le32(®s->dsp_ctrl1, > + M_RK3399_DSP_EDP_POL, > + V_RK3399_DSP_EDP_POL(polarity)); > + break; > + > + case VOP_MODE_MIPI: > + clrsetbits_le32(®s->dsp_ctrl1, > + M_RK3399_DSP_MIPI_POL, > + V_RK3399_DSP_MIPI_POL(polarity)); > + break; > + > + case VOP_MODE_LVDS: > + /* The RK3399 has neither parallel RGB nor LVDS output. */ > + default: > + debug("%s: unsupported output mode %x\n", __func__, mode); > + } > +} > + > void rkvop_enable(struct rk3288_vop *regs, ulong fbbase, > int fb_bits_per_pixel, const struct display_timing *edid) > { > @@ -89,50 +158,77 @@ void rkvop_enable(struct rk3288_vop *regs, ulong > fbbase, writel(0x01, ®s->reg_cfg_done); /* enable reg config */ > } > > -void rkvop_mode_set(struct rk3288_vop *regs, > - const struct display_timing *edid, enum vop_modes mode) > +static void rkvop_set_pin_polarity(struct udevice *dev, > + enum vop_modes mode, u32 polarity) > { > - u32 hactive = edid->hactive.typ; > - u32 vactive = edid->vactive.typ; > - u32 hsync_len = edid->hsync_len.typ; > - u32 hback_porch = edid->hback_porch.typ; > - u32 vsync_len = edid->vsync_len.typ; > - u32 vback_porch = edid->vback_porch.typ; > - u32 hfront_porch = edid->hfront_porch.typ; > - u32 vfront_porch = edid->vfront_porch.typ; > - uint flags; > - int mode_flags; > + struct rkvop_driverdata *ops = > + (struct rkvop_driverdata *)dev_get_driver_data(dev); > + > + if (ops->set_pin_polarity) > + ops->set_pin_polarity(dev, mode, polarity); > +} > + > +static void rkvop_enable_output(struct udevice *dev, enum vop_modes mode) > +{ > + struct rk_vop_priv *priv = dev_get_priv(dev); > + struct rk3288_vop *regs = priv->regs; > > switch (mode) { > case VOP_MODE_HDMI: > clrsetbits_le32(®s->sys_ctrl, M_ALL_OUT_EN, > V_HDMI_OUT_EN(1)); > break; > + > case VOP_MODE_EDP: > - default: > clrsetbits_le32(®s->sys_ctrl, M_ALL_OUT_EN, > V_EDP_OUT_EN(1)); > break; > + > case VOP_MODE_LVDS: > clrsetbits_le32(®s->sys_ctrl, M_ALL_OUT_EN, > V_RGB_OUT_EN(1)); > break; > + > + default: > + debug("%s: unsupported output mode %x\n", __func__, mode); > } > +} > > - if (mode == VOP_MODE_HDMI || mode == VOP_MODE_EDP) > - /* RGBaaa */ > - mode_flags = 15; > - else > - /* RGB888 */ > - mode_flags = 0; > +void rkvop_mode_set(struct udevice *dev, > + const struct display_timing *edid, enum vop_modes mode) > +{ > + struct rk_vop_priv *priv = dev_get_priv(dev); > + struct rk3288_vop *regs = priv->regs; > + struct rkvop_driverdata *data = > + (struct rkvop_driverdata *)dev_get_driver_data(dev); > + > + u32 hactive = edid->hactive.typ; > + u32 vactive = edid->vactive.typ; > + u32 hsync_len = edid->hsync_len.typ; > + u32 hback_porch = edid->hback_porch.typ; > + u32 vsync_len = edid->vsync_len.typ; > + u32 vback_porch = edid->vback_porch.typ; > + u32 hfront_porch = edid->hfront_porch.typ; > + u32 vfront_porch = edid->vfront_porch.typ; > + int mode_flags; > + u32 pin_polarity; > > - flags = V_DSP_OUT_MODE(mode_flags) | > - V_DSP_HSYNC_POL(!!(edid->flags & DISPLAY_FLAGS_HSYNC_HIGH)) | > - V_DSP_VSYNC_POL(!!(edid->flags & DISPLAY_FLAGS_VSYNC_HIGH)); > + pin_polarity = BIT(DCLK_INVERT); > + if (edid->flags & DISPLAY_FLAGS_HSYNC_HIGH) > + pin_polarity |= BIT(HSYNC_POSITIVE); > + if (edid->flags & DISPLAY_FLAGS_VSYNC_HIGH) > + pin_polarity |= BIT(VSYNC_POSITIVE); > > - clrsetbits_le32(®s->dsp_ctrl0, > - M_DSP_OUT_MODE | M_DSP_VSYNC_POL | M_DSP_HSYNC_POL, > - flags); > + rkvop_set_pin_polarity(dev, mode, pin_polarity); > + rkvop_enable_output(dev, mode); > + > + mode_flags = 0; /* RGB888 */ > + if ((data->features & VOP_FEATURE_OUTPUT_10BIT) && > + (mode == VOP_MODE_HDMI || mode == VOP_MODE_EDP)) > + mode_flags = 15; /* RGBaaa */ > + > + clrsetbits_le32(®s->dsp_ctrl0, M_DSP_OUT_MODE, > + V_DSP_OUT_MODE(mode_flags)); > > writel(V_HSYNC(hsync_len) | > V_HORPRD(hsync_len + hback_porch + hactive + hfront_porch), > @@ -249,8 +345,7 @@ int rk_display_init(struct udevice *dev, ulong fbbase, > return ret; > } > > - rkvop_mode_set(regs, &timing, vop_id); > - > + rkvop_mode_set(dev, &timing, vop_id); > rkvop_enable(regs, fbbase, 1 << l2bpp, &timing); > > ret = display_enable(disp, 1 << l2bpp, &timing); > @@ -341,19 +436,44 @@ static int rk_vop_bind(struct udevice *dev) > { > struct video_uc_platdata *plat = dev_get_uclass_platdata(dev); > > - plat->size = 1920 * 1080 * 2; > + /* > + * Let's allocate an excessively large framebuffer (i.e. the > + * maximum any of the supported VOP variants can output), so > + * we are prepared for the day when someone first tries to > + * drive a 4K2K HDMI signal from the bootloader. > + */ > + plat->size = 4096 * 2160 * 4; > > return 0; > } > > -static const struct video_ops rk_vop_ops = { > +struct rkvop_driverdata rk3288_driverdata = { > + .features = VOP_FEATURE_OUTPUT_10BIT, > + .set_pin_polarity = rk3288_set_pin_polarity, > +}; > + > +struct rkvop_driverdata rk3399_lit_driverdata = { > + .set_pin_polarity = rk3399_set_pin_polarity, > +}; > + > +struct rkvop_driverdata rk3399_big_driverdata = { > + .features = VOP_FEATURE_OUTPUT_10BIT, > + .set_pin_polarity = rk3399_set_pin_polarity, > }; > > static const struct udevice_id rk_vop_ids[] = { > - { .compatible = "rockchip,rk3288-vop" }, > + { .compatible = "rockchip,rk3288-vop", > + .data = (ulong)&rk3288_driverdata }, > + { .compatible = "rockchip,rk3399-vop-big", > + .data = (ulong)&rk3399_big_driverdata }, > + { .compatible = "rockchip,rk3399-vop-lit", > + .data = (ulong)&rk3399_lit_driverdata }, > { } > }; > > +static const struct video_ops rk_vop_ops = { > +}; > + > U_BOOT_DRIVER(rk_vop) = { > .name = "rk_vop", > .id = UCLASS_VIDEO, > -- > 1.9.1
Hi Philipp, On 28 April 2017 at 09:53, Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > This commit enables RK3399 support for HDMI through the following > changes: > - adds a driverdata structure to mirror some subtle version > differences between the RK3399 VOPs and those in the RK3288 > (e.g. the pin-polarity configuration) > - configures the VOP to output 32bpp for HDMI > - handles whether a VOP can output 10BIT data or not (i.e. RGBaaa vs. RGB888) > using the driverdata structure > > And as we touch this file anyway, we also increase the size of the > framebuffer to a slightly overzealous 4K2K@32bpp. > > Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> > --- > > arch/arm/include/asm/arch-rockchip/vop_rk3288.h | 11 ++ > drivers/video/rockchip/rk_vop.c | 180 ++++++++++++++++++++---- > 2 files changed, 161 insertions(+), 30 deletions(-) I'd like to somehow keep the SoC-specific code out of this driver. You've done a nice job separating it out, but I wonder if we can go a bit further. I'm thinking of perhaps having two vop drivers, one for rk3288 and one for rk3399. They can share most of the operations (e.g. bind()) which can stay in the existing rk_vop.c file. For probe() you can rename the existing probe() function to something like rockchip_vop_probe(), and pass it the device and a rkvop_driverdata *. Does that make sense? Then when we add more chips we'll have a small extra file with the SoC-specific functionality. Also please put the plat->size change in its own patch. Regards, Simon
Hi Simon, > On 30 Apr 2017, at 05:49, Simon Glass <sjg@chromium.org> wrote: > > Hi Philipp, > > On 28 April 2017 at 09:53, Philipp Tomsich > <philipp.tomsich@theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>> wrote: >> This commit enables RK3399 support for HDMI through the following >> changes: >> - adds a driverdata structure to mirror some subtle version >> differences between the RK3399 VOPs and those in the RK3288 >> (e.g. the pin-polarity configuration) >> - configures the VOP to output 32bpp for HDMI >> - handles whether a VOP can output 10BIT data or not (i.e. RGBaaa vs. RGB888) >> using the driverdata structure >> >> And as we touch this file anyway, we also increase the size of the >> framebuffer to a slightly overzealous 4K2K@32bpp. >> >> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> >> --- >> >> arch/arm/include/asm/arch-rockchip/vop_rk3288.h | 11 ++ >> drivers/video/rockchip/rk_vop.c | 180 ++++++++++++++++++++---- >> 2 files changed, 161 insertions(+), 30 deletions(-) > > I'd like to somehow keep the SoC-specific code out of this driver. > You've done a nice job separating it out, but I wonder if we can go a > bit further. > > I'm thinking of perhaps having two vop drivers, one for rk3288 and one > for rk3399. They can share most of the operations (e.g. bind()) which > can stay in the existing rk_vop.c file. For probe() you can rename the > existing probe() function to something like rockchip_vop_probe(), and > pass it the device and a rkvop_driverdata *. > > Does that make sense? Then when we add more chips we'll have a small > extra file with the SoC-specific functionality. I had considered (for any future work on this) to go into the opposite direction and to port drivers/gpu/drm/rockchip/rockchip_vop_reg.[ch] from Linux (and possibly even drivers/gpu/drm/rockchip/rockchip_drm_vop.c … those treat the various variants (3288,3328, 3366, 3368, 3399-lit, 3399-big) as a single hardware block that has a different version and sometimes has the capability of supporting optional features (e.g. 10bit RGB output) Instead of splitting things up it would thus put them into a single driver and then use driverdata to model all the differences. And to make things easier for the long-term maintenance (and avoid mistakes in the first place), I would rather try and reuse (large parts of) rockchip_vop_reg.[ch] verbatim in U-Boot. Keeping things aligned with the Linux driver would be my preferred long-term solution, if I can get a consensus for it… Cheers, Philipp.
Hi Philipp, On 30 April 2017 at 06:31, Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > Hi Simon, > > On 30 Apr 2017, at 05:49, Simon Glass <sjg@chromium.org> wrote: > > Hi Philipp, > > On 28 April 2017 at 09:53, Philipp Tomsich > <philipp.tomsich@theobroma-systems.com> wrote: > > This commit enables RK3399 support for HDMI through the following > changes: > - adds a driverdata structure to mirror some subtle version > differences between the RK3399 VOPs and those in the RK3288 > (e.g. the pin-polarity configuration) > - configures the VOP to output 32bpp for HDMI > - handles whether a VOP can output 10BIT data or not (i.e. RGBaaa vs. > RGB888) > using the driverdata structure > > And as we touch this file anyway, we also increase the size of the > framebuffer to a slightly overzealous 4K2K@32bpp. > > Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> > --- > > arch/arm/include/asm/arch-rockchip/vop_rk3288.h | 11 ++ > drivers/video/rockchip/rk_vop.c | 180 > ++++++++++++++++++++---- > 2 files changed, 161 insertions(+), 30 deletions(-) > > > I'd like to somehow keep the SoC-specific code out of this driver. > You've done a nice job separating it out, but I wonder if we can go a > bit further. > > I'm thinking of perhaps having two vop drivers, one for rk3288 and one > for rk3399. They can share most of the operations (e.g. bind()) which > can stay in the existing rk_vop.c file. For probe() you can rename the > existing probe() function to something like rockchip_vop_probe(), and > pass it the device and a rkvop_driverdata *. > > Does that make sense? Then when we add more chips we'll have a small > extra file with the SoC-specific functionality. > > > I had considered (for any future work on this) to go into the opposite > direction > and to port drivers/gpu/drm/rockchip/rockchip_vop_reg.[ch] from Linux (and > possibly even drivers/gpu/drm/rockchip/rockchip_drm_vop.c … those treat > the various variants (3288,3328, 3366, 3368, 3399-lit, 3399-big) as a > single > hardware block that has a different version and sometimes has the capability > of supporting optional features (e.g. 10bit RGB output) > > Instead of splitting things up it would thus put them into a single driver > and > then use driverdata to model all the differences. And to make things easier > for the long-term maintenance (and avoid mistakes in the first place), I > would > rather try and reuse (large parts of) rockchip_vop_reg.[ch] verbatim in > U-Boot. > > Keeping things aligned with the Linux driver would be my preferred long-term > solution, if I can get a consensus for it… I do understand this desire, but I worry that we will end up with monolithic drivers where adding video support will cost a lot in code space. Linux has essentially unlimited code space so the trade-offs are different. If you do end up porting these things more directly from Linux then I'd like to see how things can be broken up so that we can scale the code size functionality a bit. You are effectively creating a new API layer within the driver to handle hardware differences. I really like that approach, but I feel that putting all the implementations in this one file is unwieldy. Having said that, from the example you gave I doubt there is a very large difference in code size? Regards, Simon
> On 03 May 2017, at 05:00, Simon Glass <sjg@chromium.org> wrote: > > Hi Philipp, > > On 30 April 2017 at 06:31, Dr. Philipp Tomsich > <philipp.tomsich@theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>> wrote: >> Hi Simon, >> >> On 30 Apr 2017, at 05:49, Simon Glass <sjg@chromium.org> wrote: >> >> Hi Philipp, >> >> On 28 April 2017 at 09:53, Philipp Tomsich >> <philipp.tomsich@theobroma-systems.com> wrote: >> >> This commit enables RK3399 support for HDMI through the following >> changes: >> - adds a driverdata structure to mirror some subtle version >> differences between the RK3399 VOPs and those in the RK3288 >> (e.g. the pin-polarity configuration) >> - configures the VOP to output 32bpp for HDMI >> - handles whether a VOP can output 10BIT data or not (i.e. RGBaaa vs. >> RGB888) >> using the driverdata structure >> >> And as we touch this file anyway, we also increase the size of the >> framebuffer to a slightly overzealous 4K2K@32bpp. >> >> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> >> --- >> >> arch/arm/include/asm/arch-rockchip/vop_rk3288.h | 11 ++ >> drivers/video/rockchip/rk_vop.c | 180 >> ++++++++++++++++++++---- >> 2 files changed, 161 insertions(+), 30 deletions(-) >> >> >> I'd like to somehow keep the SoC-specific code out of this driver. >> You've done a nice job separating it out, but I wonder if we can go a >> bit further. >> >> I'm thinking of perhaps having two vop drivers, one for rk3288 and one >> for rk3399. They can share most of the operations (e.g. bind()) which >> can stay in the existing rk_vop.c file. For probe() you can rename the >> existing probe() function to something like rockchip_vop_probe(), and >> pass it the device and a rkvop_driverdata *. >> >> Does that make sense? Then when we add more chips we'll have a small >> extra file with the SoC-specific functionality. >> >> >> I had considered (for any future work on this) to go into the opposite >> direction >> and to port drivers/gpu/drm/rockchip/rockchip_vop_reg.[ch] from Linux (and >> possibly even drivers/gpu/drm/rockchip/rockchip_drm_vop.c … those treat >> the various variants (3288,3328, 3366, 3368, 3399-lit, 3399-big) as a >> single >> hardware block that has a different version and sometimes has the capability >> of supporting optional features (e.g. 10bit RGB output) >> >> Instead of splitting things up it would thus put them into a single driver >> and >> then use driverdata to model all the differences. And to make things easier >> for the long-term maintenance (and avoid mistakes in the first place), I >> would >> rather try and reuse (large parts of) rockchip_vop_reg.[ch] verbatim in >> U-Boot. >> >> Keeping things aligned with the Linux driver would be my preferred long-term >> solution, if I can get a consensus for it… > > I do understand this desire, but I worry that we will end up with > monolithic drivers where adding video support will cost a lot in code > space. Linux has essentially unlimited code space so the trade-offs > are different. > > If you do end up porting these things more directly from Linux then > I'd like to see how things can be broken up so that we can scale the > code size functionality a bit. > > You are effectively creating a new API layer within the driver to > handle hardware differences. I really like that approach, but I feel > that putting all the implementations in this one file is unwieldy. > > Having said that, from the example you gave I doubt there is a very > large difference in code size? In fact I started to look into the changes in more detail yesterday and already started to cut this up into individual drivers along your suggestion: on second look reusing the Linux code does not conflict the approach you suggested, as I can supply the register layout differences from the SoC-specific “mini-drivers”. Regards, Philipp.
diff --git a/arch/arm/include/asm/arch-rockchip/vop_rk3288.h b/arch/arm/include/asm/arch-rockchip/vop_rk3288.h index 0ce3d67..bca6860 100644 --- a/arch/arm/include/asm/arch-rockchip/vop_rk3288.h +++ b/arch/arm/include/asm/arch-rockchip/vop_rk3288.h @@ -196,9 +196,20 @@ enum vop_modes { #define V_DSP_DEN_POL(x) (((x) & 1) << 6) #define V_DSP_VSYNC_POL(x) (((x) & 1) << 5) #define V_DSP_HSYNC_POL(x) (((x) & 1) << 4) +#define V_DSP_PIN_POL(x) (((x) & 0xf) << 4) #define V_DSP_OUT_MODE(x) ((x) & 0xf) /* VOP_DSP_CTRL1 */ +#define V_RK3399_DSP_MIPI_POL(x) ((x) << 28) +#define V_RK3399_DSP_EDP_POL(x) ((x) << 24) +#define V_RK3399_DSP_HDMI_POL(x) ((x) << 20) +#define V_RK3399_DSP_LVDS_POL(x) ((x) << 16) + +#define M_RK3399_DSP_MIPI_POL (V_RK3399_DSP_MIPI_POL(0xf)) +#define M_RK3399_DSP_EDP_POL (V_RK3399_DSP_EDP_POL(0xf)) +#define M_RK3399_DSP_HDMI_POL (V_RK3399_DSP_HDMI_POL(0xf)) +#define M_RK3399_DSP_LVDS_POL (V_RK3399_DSP_LVDS_POL(0xf)) + #define M_DSP_LAYER3_SEL (3 << 14) #define M_DSP_LAYER2_SEL (3 << 12) #define M_DSP_LAYER1_SEL (3 << 10) diff --git a/drivers/video/rockchip/rk_vop.c b/drivers/video/rockchip/rk_vop.c index bc02f80..01b4b6a 100644 --- a/drivers/video/rockchip/rk_vop.c +++ b/drivers/video/rockchip/rk_vop.c @@ -5,6 +5,8 @@ * SPDX-License-Identifier: GPL-2.0+ */ +#define DEBUG + #include <common.h> #include <clk.h> #include <display.h> @@ -28,11 +30,78 @@ DECLARE_GLOBAL_DATA_PTR; +enum vop_pol { + HSYNC_POSITIVE = 0, + VSYNC_POSITIVE = 1, + DEN_NEGATIVE = 2, + DCLK_INVERT = 3 +}; + struct rk_vop_priv { struct rk3288_vop *regs; struct rk3288_grf *grf; }; +enum vop_features { + VOP_FEATURE_OUTPUT_10BIT = (1 << 0), +}; + +struct rkvop_driverdata { + /* configuration */ + u32 features; + /* block-specific setters/getters */ + void (*set_pin_polarity)(struct udevice *, enum vop_modes, u32); +}; + +static void rk3288_set_pin_polarity(struct udevice *dev, + enum vop_modes mode, u32 polarity) +{ + struct rk_vop_priv *priv = dev_get_priv(dev); + struct rk3288_vop *regs = priv->regs; + + /* The RK3328 VOP (v3.1) has its polarity configuration in ctrl0 */ + clrsetbits_le32(®s->dsp_ctrl0, + M_DSP_DCLK_POL | M_DSP_DEN_POL | + M_DSP_VSYNC_POL | M_DSP_HSYNC_POL, + V_DSP_PIN_POL(polarity)); +} + +static void rk3399_set_pin_polarity(struct udevice *dev, + enum vop_modes mode, u32 polarity) +{ + struct rk_vop_priv *priv = dev_get_priv(dev); + struct rk3288_vop *regs = priv->regs; + + /* + * The RK3399 VOPs (v3.5 and v3.6) require a per-mode setting of + * the polarity configuration (in ctrl1). + */ + switch (mode) { + case VOP_MODE_HDMI: + clrsetbits_le32(®s->dsp_ctrl1, + M_RK3399_DSP_HDMI_POL, + V_RK3399_DSP_HDMI_POL(polarity)); + break; + + case VOP_MODE_EDP: + clrsetbits_le32(®s->dsp_ctrl1, + M_RK3399_DSP_EDP_POL, + V_RK3399_DSP_EDP_POL(polarity)); + break; + + case VOP_MODE_MIPI: + clrsetbits_le32(®s->dsp_ctrl1, + M_RK3399_DSP_MIPI_POL, + V_RK3399_DSP_MIPI_POL(polarity)); + break; + + case VOP_MODE_LVDS: + /* The RK3399 has neither parallel RGB nor LVDS output. */ + default: + debug("%s: unsupported output mode %x\n", __func__, mode); + } +} + void rkvop_enable(struct rk3288_vop *regs, ulong fbbase, int fb_bits_per_pixel, const struct display_timing *edid) { @@ -89,50 +158,77 @@ void rkvop_enable(struct rk3288_vop *regs, ulong fbbase, writel(0x01, ®s->reg_cfg_done); /* enable reg config */ } -void rkvop_mode_set(struct rk3288_vop *regs, - const struct display_timing *edid, enum vop_modes mode) +static void rkvop_set_pin_polarity(struct udevice *dev, + enum vop_modes mode, u32 polarity) { - u32 hactive = edid->hactive.typ; - u32 vactive = edid->vactive.typ; - u32 hsync_len = edid->hsync_len.typ; - u32 hback_porch = edid->hback_porch.typ; - u32 vsync_len = edid->vsync_len.typ; - u32 vback_porch = edid->vback_porch.typ; - u32 hfront_porch = edid->hfront_porch.typ; - u32 vfront_porch = edid->vfront_porch.typ; - uint flags; - int mode_flags; + struct rkvop_driverdata *ops = + (struct rkvop_driverdata *)dev_get_driver_data(dev); + + if (ops->set_pin_polarity) + ops->set_pin_polarity(dev, mode, polarity); +} + +static void rkvop_enable_output(struct udevice *dev, enum vop_modes mode) +{ + struct rk_vop_priv *priv = dev_get_priv(dev); + struct rk3288_vop *regs = priv->regs; switch (mode) { case VOP_MODE_HDMI: clrsetbits_le32(®s->sys_ctrl, M_ALL_OUT_EN, V_HDMI_OUT_EN(1)); break; + case VOP_MODE_EDP: - default: clrsetbits_le32(®s->sys_ctrl, M_ALL_OUT_EN, V_EDP_OUT_EN(1)); break; + case VOP_MODE_LVDS: clrsetbits_le32(®s->sys_ctrl, M_ALL_OUT_EN, V_RGB_OUT_EN(1)); break; + + default: + debug("%s: unsupported output mode %x\n", __func__, mode); } +} - if (mode == VOP_MODE_HDMI || mode == VOP_MODE_EDP) - /* RGBaaa */ - mode_flags = 15; - else - /* RGB888 */ - mode_flags = 0; +void rkvop_mode_set(struct udevice *dev, + const struct display_timing *edid, enum vop_modes mode) +{ + struct rk_vop_priv *priv = dev_get_priv(dev); + struct rk3288_vop *regs = priv->regs; + struct rkvop_driverdata *data = + (struct rkvop_driverdata *)dev_get_driver_data(dev); + + u32 hactive = edid->hactive.typ; + u32 vactive = edid->vactive.typ; + u32 hsync_len = edid->hsync_len.typ; + u32 hback_porch = edid->hback_porch.typ; + u32 vsync_len = edid->vsync_len.typ; + u32 vback_porch = edid->vback_porch.typ; + u32 hfront_porch = edid->hfront_porch.typ; + u32 vfront_porch = edid->vfront_porch.typ; + int mode_flags; + u32 pin_polarity; - flags = V_DSP_OUT_MODE(mode_flags) | - V_DSP_HSYNC_POL(!!(edid->flags & DISPLAY_FLAGS_HSYNC_HIGH)) | - V_DSP_VSYNC_POL(!!(edid->flags & DISPLAY_FLAGS_VSYNC_HIGH)); + pin_polarity = BIT(DCLK_INVERT); + if (edid->flags & DISPLAY_FLAGS_HSYNC_HIGH) + pin_polarity |= BIT(HSYNC_POSITIVE); + if (edid->flags & DISPLAY_FLAGS_VSYNC_HIGH) + pin_polarity |= BIT(VSYNC_POSITIVE); - clrsetbits_le32(®s->dsp_ctrl0, - M_DSP_OUT_MODE | M_DSP_VSYNC_POL | M_DSP_HSYNC_POL, - flags); + rkvop_set_pin_polarity(dev, mode, pin_polarity); + rkvop_enable_output(dev, mode); + + mode_flags = 0; /* RGB888 */ + if ((data->features & VOP_FEATURE_OUTPUT_10BIT) && + (mode == VOP_MODE_HDMI || mode == VOP_MODE_EDP)) + mode_flags = 15; /* RGBaaa */ + + clrsetbits_le32(®s->dsp_ctrl0, M_DSP_OUT_MODE, + V_DSP_OUT_MODE(mode_flags)); writel(V_HSYNC(hsync_len) | V_HORPRD(hsync_len + hback_porch + hactive + hfront_porch), @@ -249,8 +345,7 @@ int rk_display_init(struct udevice *dev, ulong fbbase, return ret; } - rkvop_mode_set(regs, &timing, vop_id); - + rkvop_mode_set(dev, &timing, vop_id); rkvop_enable(regs, fbbase, 1 << l2bpp, &timing); ret = display_enable(disp, 1 << l2bpp, &timing); @@ -341,19 +436,44 @@ static int rk_vop_bind(struct udevice *dev) { struct video_uc_platdata *plat = dev_get_uclass_platdata(dev); - plat->size = 1920 * 1080 * 2; + /* + * Let's allocate an excessively large framebuffer (i.e. the + * maximum any of the supported VOP variants can output), so + * we are prepared for the day when someone first tries to + * drive a 4K2K HDMI signal from the bootloader. + */ + plat->size = 4096 * 2160 * 4; return 0; } -static const struct video_ops rk_vop_ops = { +struct rkvop_driverdata rk3288_driverdata = { + .features = VOP_FEATURE_OUTPUT_10BIT, + .set_pin_polarity = rk3288_set_pin_polarity, +}; + +struct rkvop_driverdata rk3399_lit_driverdata = { + .set_pin_polarity = rk3399_set_pin_polarity, +}; + +struct rkvop_driverdata rk3399_big_driverdata = { + .features = VOP_FEATURE_OUTPUT_10BIT, + .set_pin_polarity = rk3399_set_pin_polarity, }; static const struct udevice_id rk_vop_ids[] = { - { .compatible = "rockchip,rk3288-vop" }, + { .compatible = "rockchip,rk3288-vop", + .data = (ulong)&rk3288_driverdata }, + { .compatible = "rockchip,rk3399-vop-big", + .data = (ulong)&rk3399_big_driverdata }, + { .compatible = "rockchip,rk3399-vop-lit", + .data = (ulong)&rk3399_lit_driverdata }, { } }; +static const struct video_ops rk_vop_ops = { +}; + U_BOOT_DRIVER(rk_vop) = { .name = "rk_vop", .id = UCLASS_VIDEO,
This commit enables RK3399 support for HDMI through the following changes: - adds a driverdata structure to mirror some subtle version differences between the RK3399 VOPs and those in the RK3288 (e.g. the pin-polarity configuration) - configures the VOP to output 32bpp for HDMI - handles whether a VOP can output 10BIT data or not (i.e. RGBaaa vs. RGB888) using the driverdata structure And as we touch this file anyway, we also increase the size of the framebuffer to a slightly overzealous 4K2K@32bpp. Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> --- arch/arm/include/asm/arch-rockchip/vop_rk3288.h | 11 ++ drivers/video/rockchip/rk_vop.c | 180 ++++++++++++++++++++---- 2 files changed, 161 insertions(+), 30 deletions(-)