| Message ID | 1478865346-19043-3-git-send-email-yt.shen@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi YT,
I don't see a reason to handle device_data in such a generic way at
the generic mtk_ddp_comp layer.
The device data is very component specific, so just define different
structs for different comp types, ie:
struct mtk_disp_ovl_driver_data {
unsigned int reg_ovl_addr;
unsigned int fmt_rgb565;
unsigned int fmt_rgb888;
};
struct mtk_disp_rdma_driver_data {
unsigned int fifo_pseudo_size;
};
struct mtk_disp_color_driver_data {
unsigned int color_offset;
};
Then add typed pointers to the local structs that use them, for example:
struct mtk_disp_ovl {
struct mtk_ddp_comp ddp_comp;
struct drm_crtc *crtc;
const struct mtk_disp_ovl_driver_data *data;
};
And fetch the device specific driver data directly in .probe, as you
are already doing:
static int mtk_disp_ovl_probe(struct platform_device *pdev) {
...
priv->data = of_device_get_match_data(dev);
...
}
More comments in-line...
On Fri, Nov 11, 2016 at 7:55 PM, YT Shen <yt.shen@mediatek.com> wrote:
> There are some hardware settings changed, between MT8173 & MT2701:
> DISP_OVL address offset changed, color format definition changed.
> DISP_RDMA fifo size changed.
> DISP_COLOR offset changed.
> MIPI_TX pll setting changed.
> And add prefix for mtk_ddp_main & mtk_ddp_ext & mutex_mod.
Nit: I think it would make sense to combine this patch with
drm/mediatek: rename macros, add chip prefix
>
> Signed-off-by: YT Shen <yt.shen@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 27 ++++++++++++++++-----------
> drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 11 +++++++++--
> drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 11 +++++++----
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 27 +++++++++++++++++++++------
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 13 +++++++++++++
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 25 ++++++++++++++++++-------
> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 8 ++++++++
> drivers/gpu/drm/mediatek/mtk_mipi_tx.c | 24 +++++++++++++++++++++++-
> 8 files changed, 115 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 019b7ca..1139834 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -35,13 +35,10 @@
> #define DISP_REG_OVL_PITCH(n) (0x0044 + 0x20 * (n))
> #define DISP_REG_OVL_RDMA_CTRL(n) (0x00c0 + 0x20 * (n))
> #define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n))
> -#define DISP_REG_OVL_ADDR(n) (0x0f40 + 0x20 * (n))
Also, I would still use the "#define macros", for example
"DISP_REG_OVL_ADDR offsets, and use the named constant in the
driver_data:
#define DISP_REG_OVL_ADDR_MT8173 0x0f40
(and in a later patch:
#define DISP_REG_OVL_ADDR_MT2701 0x0040
)
Also, I would still use the macro rather than open coding the "0x20 *
(n)", and just pass 'ovl' to the overlay macros that depend on
hardware type.
Something like the following:
#define DISP_REG_OVL_ADDR(ovl, n) ((ovl)->data->ovl_addr + 0x20 * (n))
>
> #define OVL_RDMA_MEM_GMC 0x40402020
>
> #define OVL_CON_BYTE_SWAP BIT(24)
> -#define OVL_CON_CLRFMT_RGB565 (0 << 12)
> -#define OVL_CON_CLRFMT_RGB888 (1 << 12)
This seems like a really random and unnecessary hardware change.
Why chip designers, why!!?!?
For this one, it seems the polarity is either one way or the other, so
we can just use a bool to distinguish:
bool fmt_rgb565_is_0;
> +static const struct mtk_ddp_comp_driver_data mt8173_ovl_driver_data = {
> + .ovl = { DISP_REG_OVL_ADDR_MT8173, .fmt_rgb565_is_0 = true }
> +};
For use at runtime, the defines could become:
#define OVL_CON_CLRFMT_RGB565(ovl) ((ovl)->data->fmt_rgb565_is_0 ? 0
: OVL_CON_CLRFMT_RGB888)
#define OVL_CON_CLRFMT_RGB888(ovl) ((ovl)->data->fmt_rgb565_is_0 ?
OVL_CON_CLRFMT_RGB888 : 0)
> #define OVL_CON_CLRFMT_RGBA8888 (2 << 12)
> #define OVL_CON_CLRFMT_ARGB8888 (3 << 12)
> #define OVL_CON_AEN BIT(8)
> @@ -137,18 +134,18 @@ static void mtk_ovl_layer_off(struct mtk_ddp_comp *comp, unsigned int idx)
> writel(0x0, comp->regs + DISP_REG_OVL_RDMA_CTRL(idx));
> }
>
> -static unsigned int ovl_fmt_convert(unsigned int fmt)
> +static unsigned int ovl_fmt_convert(struct mtk_ddp_comp *comp, unsigned int fmt)
> {
> switch (fmt) {
> default:
> case DRM_FORMAT_RGB565:
> - return OVL_CON_CLRFMT_RGB565;
> + return comp->data->ovl.fmt_rgb565;
It will be nice to define a helper function for converting from the
generic 'mtk_ddp_comp' to the specific 'mtk_disp_ovl':
static inline struct mtk_disp_ovl *comp_to_ovl(struct mtk_ddp_comp *comp) {
return container_of(comp, struct mtk_disp_ovl, ddp_comp);
}
Then these could become:
return OVL_CON_CLRFMT_RGB565(comp_to_ovl(comp));
Or maybe cleaner, do the conversion once at the top of the function:
struct mtk_disp_ovl *ovl = comp_to_ovl(comp);
And then just:
return OVL_CON_CLRFMT_RGB565(ovl);
> case DRM_FORMAT_BGR565:
> - return OVL_CON_CLRFMT_RGB565 | OVL_CON_BYTE_SWAP;
> + return comp->data->ovl.fmt_rgb565 | OVL_CON_BYTE_SWAP;
> case DRM_FORMAT_RGB888:
> - return OVL_CON_CLRFMT_RGB888;
> + return comp->data->ovl.fmt_rgb888;
> case DRM_FORMAT_BGR888:
> - return OVL_CON_CLRFMT_RGB888 | OVL_CON_BYTE_SWAP;
> + return comp->data->ovl.fmt_rgb888 | OVL_CON_BYTE_SWAP;
> case DRM_FORMAT_RGBX8888:
> case DRM_FORMAT_RGBA8888:
> return OVL_CON_CLRFMT_ARGB8888;
[snip]
> diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> index 1c366f8..935a8ef 100644
> --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> @@ -16,6 +16,7 @@
> #include <linux/delay.h>
> #include <linux/io.h>
> #include <linux/module.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/phy/phy.h>
>
> @@ -87,6 +88,9 @@
>
> #define MIPITX_DSI_PLL_CON2 0x58
>
> +#define MIPITX_DSI_PLL_TOP 0x64
> +#define RG_DSI_MPPLL_PRESERVE (0xff << 8)
> +
> #define MIPITX_DSI_PLL_PWR 0x68
> #define RG_DSI_MPPLL_SDM_PWR_ON BIT(0)
> #define RG_DSI_MPPLL_SDM_ISO_EN BIT(1)
> @@ -123,10 +127,15 @@
> #define SW_LNT2_HSTX_PRE_OE BIT(24)
> #define SW_LNT2_HSTX_OE BIT(25)
>
> +struct mtk_mipitx_data {
> + const u32 data;
Use a better name, like "mppll_preserve".
Ok, that's it for now.
Actually, the patch set in general looks pretty good.
-Dan
Hi Daniel, On Fri, 2016-11-18 at 12:56 +0800, Daniel Kurtz wrote: > Hi YT, > > I don't see a reason to handle device_data in such a generic way at > the generic mtk_ddp_comp layer. > The device data is very component specific, so just define different > structs for different comp types, ie: > > struct mtk_disp_ovl_driver_data { > unsigned int reg_ovl_addr; > unsigned int fmt_rgb565; > unsigned int fmt_rgb888; > }; > > struct mtk_disp_rdma_driver_data { > unsigned int fifo_pseudo_size; > }; > > struct mtk_disp_color_driver_data { > unsigned int color_offset; > }; > > Then add typed pointers to the local structs that use them, for example: > > struct mtk_disp_ovl { > struct mtk_ddp_comp ddp_comp; > struct drm_crtc *crtc; > const struct mtk_disp_ovl_driver_data *data; > }; > > And fetch the device specific driver data directly in .probe, as you > are already doing: > > static int mtk_disp_ovl_probe(struct platform_device *pdev) { > ... > priv->data = of_device_get_match_data(dev); > ... > } These suggestions make code more readable. We will change ovl and rdma part, and keep mtk_disp_color_driver_data in its original place. Because ovl and rdma have its files, other modules share mtk_drm_ddp_comp.c. > > More comments in-line... > > On Fri, Nov 11, 2016 at 7:55 PM, YT Shen <yt.shen@mediatek.com> wrote: > > There are some hardware settings changed, between MT8173 & MT2701: > > DISP_OVL address offset changed, color format definition changed. > > DISP_RDMA fifo size changed. > > DISP_COLOR offset changed. > > MIPI_TX pll setting changed. > > And add prefix for mtk_ddp_main & mtk_ddp_ext & mutex_mod. > > Nit: I think it would make sense to combine this patch with > drm/mediatek: rename macros, add chip prefix Will do. > > > > > Signed-off-by: YT Shen <yt.shen@mediatek.com> > > --- > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 27 ++++++++++++++++----------- > > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 11 +++++++++-- > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 11 +++++++---- > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 27 +++++++++++++++++++++------ > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 13 +++++++++++++ > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 25 ++++++++++++++++++------- > > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 8 ++++++++ > > drivers/gpu/drm/mediatek/mtk_mipi_tx.c | 24 +++++++++++++++++++++++- > > 8 files changed, 115 insertions(+), 31 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > index 019b7ca..1139834 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > @@ -35,13 +35,10 @@ > > #define DISP_REG_OVL_PITCH(n) (0x0044 + 0x20 * (n)) > > #define DISP_REG_OVL_RDMA_CTRL(n) (0x00c0 + 0x20 * (n)) > > #define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n)) > > -#define DISP_REG_OVL_ADDR(n) (0x0f40 + 0x20 * (n)) > > Also, I would still use the "#define macros", for example > "DISP_REG_OVL_ADDR offsets, and use the named constant in the > driver_data: > > #define DISP_REG_OVL_ADDR_MT8173 0x0f40 > > (and in a later patch: > #define DISP_REG_OVL_ADDR_MT2701 0x0040 > ) > > Also, I would still use the macro rather than open coding the "0x20 * > (n)", and just pass 'ovl' to the overlay macros that depend on > hardware type. > Something like the following: > > #define DISP_REG_OVL_ADDR(ovl, n) ((ovl)->data->ovl_addr + 0x20 * (n)) Will use the "#define macros" here. > > > > > #define OVL_RDMA_MEM_GMC 0x40402020 > > > > #define OVL_CON_BYTE_SWAP BIT(24) > > -#define OVL_CON_CLRFMT_RGB565 (0 << 12) > > -#define OVL_CON_CLRFMT_RGB888 (1 << 12) > > This seems like a really random and unnecessary hardware change. > Why chip designers, why!!?!? There are many reasons for software bugs. Unnecessary hardware change should be one of them... > > For this one, it seems the polarity is either one way or the other, so > we can just use a bool to distinguish: > > bool fmt_rgb565_is_0; > > > +static const struct mtk_ddp_comp_driver_data mt8173_ovl_driver_data = { > > + .ovl = { DISP_REG_OVL_ADDR_MT8173, .fmt_rgb565_is_0 = true } > > +}; > > For use at runtime, the defines could become: > > #define OVL_CON_CLRFMT_RGB565(ovl) ((ovl)->data->fmt_rgb565_is_0 ? 0 > : OVL_CON_CLRFMT_RGB888) > #define OVL_CON_CLRFMT_RGB888(ovl) ((ovl)->data->fmt_rgb565_is_0 ? > OVL_CON_CLRFMT_RGB888 : 0) OK, will do. > > > #define OVL_CON_CLRFMT_RGBA8888 (2 << 12) > > #define OVL_CON_CLRFMT_ARGB8888 (3 << 12) > > #define OVL_CON_AEN BIT(8) > > @@ -137,18 +134,18 @@ static void mtk_ovl_layer_off(struct mtk_ddp_comp *comp, unsigned int idx) > > writel(0x0, comp->regs + DISP_REG_OVL_RDMA_CTRL(idx)); > > } > > > > -static unsigned int ovl_fmt_convert(unsigned int fmt) > > +static unsigned int ovl_fmt_convert(struct mtk_ddp_comp *comp, unsigned int fmt) > > { > > switch (fmt) { > > default: > > case DRM_FORMAT_RGB565: > > - return OVL_CON_CLRFMT_RGB565; > > + return comp->data->ovl.fmt_rgb565; > > It will be nice to define a helper function for converting from the > generic 'mtk_ddp_comp' to the specific 'mtk_disp_ovl': > > static inline struct mtk_disp_ovl *comp_to_ovl(struct mtk_ddp_comp *comp) { > return container_of(comp, struct mtk_disp_ovl, ddp_comp); > } > > Then these could become: > return OVL_CON_CLRFMT_RGB565(comp_to_ovl(comp)); > > Or maybe cleaner, do the conversion once at the top of the function: > struct mtk_disp_ovl *ovl = comp_to_ovl(comp); > > And then just: > return OVL_CON_CLRFMT_RGB565(ovl); > > Will use a helper function for the converting. > > case DRM_FORMAT_BGR565: > > - return OVL_CON_CLRFMT_RGB565 | OVL_CON_BYTE_SWAP; > > + return comp->data->ovl.fmt_rgb565 | OVL_CON_BYTE_SWAP; > > case DRM_FORMAT_RGB888: > > - return OVL_CON_CLRFMT_RGB888; > > + return comp->data->ovl.fmt_rgb888; > > case DRM_FORMAT_BGR888: > > - return OVL_CON_CLRFMT_RGB888 | OVL_CON_BYTE_SWAP; > > + return comp->data->ovl.fmt_rgb888 | OVL_CON_BYTE_SWAP; > > case DRM_FORMAT_RGBX8888: > > case DRM_FORMAT_RGBA8888: > > return OVL_CON_CLRFMT_ARGB8888; > > [snip] > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c > > index 1c366f8..935a8ef 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c > > +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c > > @@ -16,6 +16,7 @@ > > #include <linux/delay.h> > > #include <linux/io.h> > > #include <linux/module.h> > > +#include <linux/of_device.h> > > #include <linux/platform_device.h> > > #include <linux/phy/phy.h> > > > > @@ -87,6 +88,9 @@ > > > > #define MIPITX_DSI_PLL_CON2 0x58 > > > > +#define MIPITX_DSI_PLL_TOP 0x64 > > +#define RG_DSI_MPPLL_PRESERVE (0xff << 8) > > + > > #define MIPITX_DSI_PLL_PWR 0x68 > > #define RG_DSI_MPPLL_SDM_PWR_ON BIT(0) > > #define RG_DSI_MPPLL_SDM_ISO_EN BIT(1) > > @@ -123,10 +127,15 @@ > > #define SW_LNT2_HSTX_PRE_OE BIT(24) > > #define SW_LNT2_HSTX_OE BIT(25) > > > > +struct mtk_mipitx_data { > > + const u32 data; > > Use a better name, like "mppll_preserve". OK. Regards, yt.shen > > Ok, that's it for now. > Actually, the patch set in general looks pretty good. > > -Dan
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 019b7ca..1139834 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -35,13 +35,10 @@ #define DISP_REG_OVL_PITCH(n) (0x0044 + 0x20 * (n)) #define DISP_REG_OVL_RDMA_CTRL(n) (0x00c0 + 0x20 * (n)) #define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n)) -#define DISP_REG_OVL_ADDR(n) (0x0f40 + 0x20 * (n)) #define OVL_RDMA_MEM_GMC 0x40402020 #define OVL_CON_BYTE_SWAP BIT(24) -#define OVL_CON_CLRFMT_RGB565 (0 << 12) -#define OVL_CON_CLRFMT_RGB888 (1 << 12) #define OVL_CON_CLRFMT_RGBA8888 (2 << 12) #define OVL_CON_CLRFMT_ARGB8888 (3 << 12) #define OVL_CON_AEN BIT(8) @@ -137,18 +134,18 @@ static void mtk_ovl_layer_off(struct mtk_ddp_comp *comp, unsigned int idx) writel(0x0, comp->regs + DISP_REG_OVL_RDMA_CTRL(idx)); } -static unsigned int ovl_fmt_convert(unsigned int fmt) +static unsigned int ovl_fmt_convert(struct mtk_ddp_comp *comp, unsigned int fmt) { switch (fmt) { default: case DRM_FORMAT_RGB565: - return OVL_CON_CLRFMT_RGB565; + return comp->data->ovl.fmt_rgb565; case DRM_FORMAT_BGR565: - return OVL_CON_CLRFMT_RGB565 | OVL_CON_BYTE_SWAP; + return comp->data->ovl.fmt_rgb565 | OVL_CON_BYTE_SWAP; case DRM_FORMAT_RGB888: - return OVL_CON_CLRFMT_RGB888; + return comp->data->ovl.fmt_rgb888; case DRM_FORMAT_BGR888: - return OVL_CON_CLRFMT_RGB888 | OVL_CON_BYTE_SWAP; + return comp->data->ovl.fmt_rgb888 | OVL_CON_BYTE_SWAP; case DRM_FORMAT_RGBX8888: case DRM_FORMAT_RGBA8888: return OVL_CON_CLRFMT_ARGB8888; @@ -178,7 +175,7 @@ static void mtk_ovl_layer_config(struct mtk_ddp_comp *comp, unsigned int idx, if (!pending->enable) mtk_ovl_layer_off(comp, idx); - con = ovl_fmt_convert(fmt); + con = ovl_fmt_convert(comp, fmt); if (idx != 0) con |= OVL_CON_AEN | OVL_CON_ALPHA; @@ -186,7 +183,8 @@ static void mtk_ovl_layer_config(struct mtk_ddp_comp *comp, unsigned int idx, writel_relaxed(pitch, comp->regs + DISP_REG_OVL_PITCH(idx)); writel_relaxed(src_size, comp->regs + DISP_REG_OVL_SRC_SIZE(idx)); writel_relaxed(offset, comp->regs + DISP_REG_OVL_OFFSET(idx)); - writel_relaxed(addr, comp->regs + DISP_REG_OVL_ADDR(idx)); + writel_relaxed(addr, comp->regs + comp->data->ovl.addr_offset + + idx * 0x20); if (pending->enable) mtk_ovl_layer_on(comp, idx); @@ -270,6 +268,8 @@ static int mtk_disp_ovl_probe(struct platform_device *pdev) return ret; } + priv->ddp_comp.data = of_device_get_match_data(dev); + platform_set_drvdata(pdev, priv); ret = component_add(dev, &mtk_disp_ovl_component_ops); @@ -286,8 +286,13 @@ static int mtk_disp_ovl_remove(struct platform_device *pdev) return 0; } +static const struct mtk_ddp_comp_driver_data mt8173_ovl_driver_data = { + .ovl = {0x0f40, 0, 1 << 12} +}; + static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = { - { .compatible = "mediatek,mt8173-disp-ovl", }, + { .compatible = "mediatek,mt8173-disp-ovl", + .data = &mt8173_ovl_driver_data}, {}, }; MODULE_DEVICE_TABLE(of, mtk_disp_ovl_driver_dt_match); diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c index 0df05f9..b4225e2 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c @@ -123,7 +123,7 @@ static void mtk_rdma_config(struct mtk_ddp_comp *comp, unsigned int width, */ threshold = width * height * vrefresh * 4 * 7 / 1000000; reg = RDMA_FIFO_UNDERFLOW_EN | - RDMA_FIFO_PSEUDO_SIZE(SZ_8K) | + RDMA_FIFO_PSEUDO_SIZE(comp->data->rdma_fifo_pseudo_size) | RDMA_OUTPUT_VALID_FIFO_THRESHOLD(threshold); writel(reg, comp->regs + DISP_REG_RDMA_FIFO_CON); } @@ -208,6 +208,8 @@ static int mtk_disp_rdma_probe(struct platform_device *pdev) return ret; } + priv->ddp_comp.data = of_device_get_match_data(dev); + platform_set_drvdata(pdev, priv); ret = component_add(dev, &mtk_disp_rdma_component_ops); @@ -224,8 +226,13 @@ static int mtk_disp_rdma_remove(struct platform_device *pdev) return 0; } +static const struct mtk_ddp_comp_driver_data mt8173_rdma_driver_data = { + .rdma_fifo_pseudo_size = SZ_8K, +}; + static const struct of_device_id mtk_disp_rdma_driver_dt_match[] = { - { .compatible = "mediatek,mt8173-disp-rdma", }, + { .compatible = "mediatek,mt8173-disp-rdma", + .data = &mt8173_rdma_driver_data}, {}, }; MODULE_DEVICE_TABLE(of, mtk_disp_rdma_driver_dt_match); diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c index 2fc4321..8030769 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c @@ -77,9 +77,10 @@ struct mtk_ddp { struct clk *clk; void __iomem *regs; struct mtk_disp_mutex mutex[10]; + const unsigned int *mutex_mod; }; -static const unsigned int mutex_mod[DDP_COMPONENT_ID_MAX] = { +static const unsigned int mt8173_mutex_mod[DDP_COMPONENT_ID_MAX] = { [DDP_COMPONENT_AAL] = MT8173_MUTEX_MOD_DISP_AAL, [DDP_COMPONENT_COLOR0] = MT8173_MUTEX_MOD_DISP_COLOR0, [DDP_COMPONENT_COLOR1] = MT8173_MUTEX_MOD_DISP_COLOR1, @@ -247,7 +248,7 @@ void mtk_disp_mutex_add_comp(struct mtk_disp_mutex *mutex, break; default: reg = readl_relaxed(ddp->regs + DISP_REG_MUTEX_MOD(mutex->id)); - reg |= mutex_mod[id]; + reg |= ddp->mutex_mod[id]; writel_relaxed(reg, ddp->regs + DISP_REG_MUTEX_MOD(mutex->id)); return; } @@ -273,7 +274,7 @@ void mtk_disp_mutex_remove_comp(struct mtk_disp_mutex *mutex, break; default: reg = readl_relaxed(ddp->regs + DISP_REG_MUTEX_MOD(mutex->id)); - reg &= ~mutex_mod[id]; + reg &= ~(ddp->mutex_mod[id]); writel_relaxed(reg, ddp->regs + DISP_REG_MUTEX_MOD(mutex->id)); break; } @@ -326,6 +327,8 @@ static int mtk_ddp_probe(struct platform_device *pdev) return PTR_ERR(ddp->regs); } + ddp->mutex_mod = of_device_get_match_data(dev); + platform_set_drvdata(pdev, ddp); return 0; @@ -337,7 +340,7 @@ static int mtk_ddp_remove(struct platform_device *pdev) } static const struct of_device_id ddp_driver_dt_match[] = { - { .compatible = "mediatek,mt8173-disp-mutex" }, + { .compatible = "mediatek,mt8173-disp-mutex", .data = mt8173_mutex_mod}, {}, }; MODULE_DEVICE_TABLE(of, ddp_driver_dt_match); diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c index df33b3c..661a4a0 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c @@ -39,9 +39,8 @@ #define DISP_REG_UFO_START 0x0000 #define DISP_COLOR_CFG_MAIN 0x0400 -#define DISP_COLOR_START 0x0c00 -#define DISP_COLOR_WIDTH 0x0c50 -#define DISP_COLOR_HEIGHT 0x0c54 +#define DISP_COLOR_WIDTH 0x50 +#define DISP_COLOR_HEIGHT 0x54 #define DISP_AAL_EN 0x0000 #define DISP_AAL_SIZE 0x0030 @@ -107,15 +106,15 @@ static void mtk_color_config(struct mtk_ddp_comp *comp, unsigned int w, unsigned int h, unsigned int vrefresh, unsigned int bpc) { - writel(w, comp->regs + DISP_COLOR_WIDTH); - writel(h, comp->regs + DISP_COLOR_HEIGHT); + writel(w, comp->regs + comp->data->color_offset + DISP_COLOR_WIDTH); + writel(h, comp->regs + comp->data->color_offset + DISP_COLOR_HEIGHT); } static void mtk_color_start(struct mtk_ddp_comp *comp) { writel(COLOR_BYPASS_ALL | COLOR_SEQ_SEL, comp->regs + DISP_COLOR_CFG_MAIN); - writel(0x1, comp->regs + DISP_COLOR_START); + writel(0x1, comp->regs + comp->data->color_offset); } static void mtk_od_config(struct mtk_ddp_comp *comp, unsigned int w, @@ -264,6 +263,16 @@ struct mtk_ddp_comp_match { [DDP_COMPONENT_WDMA1] = { MTK_DISP_WDMA, 1, NULL }, }; +static const struct mtk_ddp_comp_driver_data mt8173_color_driver_data = { + .color_offset = 0x0c00, +}; + +static const struct of_device_id mtk_disp_color_driver_dt_match[] = { + { .compatible = "mediatek,mt8173-disp-color", + .data = &mt8173_color_driver_data}, + {}, +}; + int mtk_ddp_comp_get_id(struct device_node *node, enum mtk_ddp_comp_type comp_type) { @@ -286,6 +295,7 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node, enum mtk_ddp_comp_type type; struct device_node *larb_node; struct platform_device *larb_pdev; + const struct of_device_id *match; if (comp_id < 0 || comp_id >= DDP_COMPONENT_ID_MAX) return -EINVAL; @@ -310,6 +320,11 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node, type = mtk_ddp_matches[comp_id].type; + if (type == MTK_DISP_COLOR) { + match = of_match_node(mtk_disp_color_driver_dt_match, node); + comp->data = match->data; + } + /* Only DMA capable components need the LARB property */ comp->larb_dev = NULL; if (type != MTK_DISP_OVL && diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h index 22a33ee..2f6872a 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h @@ -78,6 +78,18 @@ struct mtk_ddp_comp_funcs { struct drm_crtc_state *state); }; +struct mtk_ddp_comp_driver_data { + union { + struct ovl { + unsigned int addr_offset; + unsigned int fmt_rgb565; + unsigned int fmt_rgb888; + } ovl; + unsigned int rdma_fifo_pseudo_size; + unsigned int color_offset; + }; +}; + struct mtk_ddp_comp { struct clk *clk; void __iomem *regs; @@ -85,6 +97,7 @@ struct mtk_ddp_comp { struct device *larb_dev; enum mtk_ddp_comp_id id; const struct mtk_ddp_comp_funcs *funcs; + const struct mtk_ddp_comp_driver_data *data; }; static inline void mtk_ddp_comp_config(struct mtk_ddp_comp *comp, diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index cf83f65..5f9b5e8 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -126,7 +126,7 @@ static int mtk_atomic_commit(struct drm_device *drm, .atomic_commit = mtk_atomic_commit, }; -static const enum mtk_ddp_comp_id mtk_ddp_main[] = { +static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = { DDP_COMPONENT_OVL0, DDP_COMPONENT_COLOR0, DDP_COMPONENT_AAL, @@ -137,7 +137,7 @@ static int mtk_atomic_commit(struct drm_device *drm, DDP_COMPONENT_PWM0, }; -static const enum mtk_ddp_comp_id mtk_ddp_ext[] = { +static const enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = { DDP_COMPONENT_OVL1, DDP_COMPONENT_COLOR1, DDP_COMPONENT_GAMMA, @@ -145,6 +145,13 @@ static int mtk_atomic_commit(struct drm_device *drm, DDP_COMPONENT_DPI0, }; +static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = { + .main_path = mt8173_mtk_ddp_main, + .main_len = ARRAY_SIZE(mt8173_mtk_ddp_main), + .ext_path = mt8173_mtk_ddp_ext, + .ext_len = ARRAY_SIZE(mt8173_mtk_ddp_ext), +}; + static int mtk_drm_kms_init(struct drm_device *drm) { struct mtk_drm_private *private = drm->dev_private; @@ -187,17 +194,19 @@ static int mtk_drm_kms_init(struct drm_device *drm) * and each statically assigned to a crtc: * OVL0 -> COLOR0 -> AAL -> OD -> RDMA0 -> UFOE -> DSI0 ... */ - ret = mtk_drm_crtc_create(drm, mtk_ddp_main, ARRAY_SIZE(mtk_ddp_main)); + ret = mtk_drm_crtc_create(drm, private->data->main_path, + private->data->main_len); if (ret < 0) goto err_component_unbind; /* ... and OVL1 -> COLOR1 -> GAMMA -> RDMA1 -> DPI0. */ - ret = mtk_drm_crtc_create(drm, mtk_ddp_ext, ARRAY_SIZE(mtk_ddp_ext)); + ret = mtk_drm_crtc_create(drm, private->data->ext_path, + private->data->ext_len); if (ret < 0) goto err_component_unbind; /* Use OVL device for all DMA memory allocations */ - np = private->comp_node[mtk_ddp_main[0]] ?: - private->comp_node[mtk_ddp_ext[0]]; + np = private->comp_node[private->data->main_path[0]] ?: + private->comp_node[private->data->ext_path[0]]; pdev = of_find_device_by_node(np); if (!pdev) { ret = -ENODEV; @@ -362,6 +371,7 @@ static int mtk_drm_probe(struct platform_device *pdev) mutex_init(&private->commit.lock); INIT_WORK(&private->commit.work, mtk_atomic_work); + private->data = of_device_get_match_data(dev); mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); private->config_regs = devm_ioremap_resource(dev, mem); @@ -512,7 +522,8 @@ static SIMPLE_DEV_PM_OPS(mtk_drm_pm_ops, mtk_drm_sys_suspend, mtk_drm_sys_resume); static const struct of_device_id mtk_drm_of_ids[] = { - { .compatible = "mediatek,mt8173-mmsys", }, + { .compatible = "mediatek,mt8173-mmsys", + .data = &mt8173_mmsys_driver_data}, { } }; diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h index aa93894..fa0b106 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h @@ -28,6 +28,13 @@ struct drm_property; struct regmap; +struct mtk_mmsys_driver_data { + const enum mtk_ddp_comp_id *main_path; + unsigned int main_len; + const enum mtk_ddp_comp_id *ext_path; + unsigned int ext_len; +}; + struct mtk_drm_private { struct drm_device *drm; struct device *dma_dev; @@ -40,6 +47,7 @@ struct mtk_drm_private { void __iomem *config_regs; struct device_node *comp_node[DDP_COMPONENT_ID_MAX]; struct mtk_ddp_comp *ddp_comp[DDP_COMPONENT_ID_MAX]; + const struct mtk_mmsys_driver_data *data; struct { struct drm_atomic_state *state; diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c index 1c366f8..935a8ef 100644 --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c @@ -16,6 +16,7 @@ #include <linux/delay.h> #include <linux/io.h> #include <linux/module.h> +#include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/phy/phy.h> @@ -87,6 +88,9 @@ #define MIPITX_DSI_PLL_CON2 0x58 +#define MIPITX_DSI_PLL_TOP 0x64 +#define RG_DSI_MPPLL_PRESERVE (0xff << 8) + #define MIPITX_DSI_PLL_PWR 0x68 #define RG_DSI_MPPLL_SDM_PWR_ON BIT(0) #define RG_DSI_MPPLL_SDM_ISO_EN BIT(1) @@ -123,10 +127,15 @@ #define SW_LNT2_HSTX_PRE_OE BIT(24) #define SW_LNT2_HSTX_OE BIT(25) +struct mtk_mipitx_data { + const u32 data; +}; + struct mtk_mipi_tx { struct device *dev; void __iomem *regs; unsigned int data_rate; + const struct mtk_mipitx_data *driver_data; struct clk_hw pll_hw; struct clk *pll; }; @@ -243,6 +252,10 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw) mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_DSI_PLL_CON1, RG_DSI_MPPLL_SDM_SSC_EN); + mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_TOP, + RG_DSI_MPPLL_PRESERVE, + mipi_tx->driver_data->data); + return 0; } @@ -255,6 +268,9 @@ static void mtk_mipi_tx_pll_unprepare(struct clk_hw *hw) mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_DSI_PLL_CON0, RG_DSI_MPPLL_PLL_EN); + mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_TOP, + RG_DSI_MPPLL_PRESERVE, 0); + mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_PWR, RG_DSI_MPPLL_SDM_ISO_EN | RG_DSI_MPPLL_SDM_PWR_ON, @@ -391,6 +407,7 @@ static int mtk_mipi_tx_probe(struct platform_device *pdev) if (!mipi_tx) return -ENOMEM; + mipi_tx->driver_data = of_device_get_match_data(dev); mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); mipi_tx->regs = devm_ioremap_resource(dev, mem); if (IS_ERR(mipi_tx->regs)) { @@ -448,8 +465,13 @@ static int mtk_mipi_tx_remove(struct platform_device *pdev) return 0; } +static const struct mtk_mipitx_data mt8173_mipitx_data = { + .data = (0 << 8) +}; + static const struct of_device_id mtk_mipi_tx_match[] = { - { .compatible = "mediatek,mt8173-mipi-tx", }, + { .compatible = "mediatek,mt8173-mipi-tx", + .data = &mt8173_mipitx_data }, {}, };
There are some hardware settings changed, between MT8173 & MT2701: DISP_OVL address offset changed, color format definition changed. DISP_RDMA fifo size changed. DISP_COLOR offset changed. MIPI_TX pll setting changed. And add prefix for mtk_ddp_main & mtk_ddp_ext & mutex_mod. Signed-off-by: YT Shen <yt.shen@mediatek.com> --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 27 ++++++++++++++++----------- drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 11 +++++++++-- drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 11 +++++++---- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 27 +++++++++++++++++++++------ drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 13 +++++++++++++ drivers/gpu/drm/mediatek/mtk_drm_drv.c | 25 ++++++++++++++++++------- drivers/gpu/drm/mediatek/mtk_drm_drv.h | 8 ++++++++ drivers/gpu/drm/mediatek/mtk_mipi_tx.c | 24 +++++++++++++++++++++++- 8 files changed, 115 insertions(+), 31 deletions(-)