Re: [PATCH v10 02/13] drm/mediatek: add *driver_data for different hardware settings

2016-12-05 Thread YT Shen 
Hi Daniel,

On Wed, 2016-11-30 at 14:42 +0800, Daniel Kurtz wrote:
> Hi YT,
> 
> 
> On Fri, Nov 25, 2016 at 6:34 PM, YT Shen  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.
> 
> Sorry, I have not had time to thoroughly review the new patch set, but
> one quick comment:
> 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h 
> > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > index 22a33ee..cfaa760 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > @@ -78,6 +78,10 @@ struct mtk_ddp_comp_funcs {
> >   struct drm_crtc_state *state);
> >  };
> >
> > +struct mtk_ddp_comp_driver_data {
> > +   unsigned int color_offset;
> > +};
> > +
> >  struct mtk_ddp_comp {
> > struct clk *clk;
> > void __iomem *regs;
> > @@ -85,6 +89,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;
> 
> I want to avoid adding this generic pointer here to all mtk_ddp_comp,
> since this is only used by the color component.
> Instead, define color specific types:
> 
> struct mtk_disp_color_data {
>unsigned int offset;
> };
> 
> struct mtk_disp_color {
>struct mtk_ddp_comp ddp_comp;
>const struct mtk_disp_color_data *data;
> };
> 
> Then, add another comp_type check and fetch the dev data in
> mtk_drm_probe()  or maybe mtk_ddp_comp_init().
> 
> -Dan
OK, we will remove the color data pointer from generic mtk_ddp_comp.
Thanks.

Regards,
yt.shen

Re: [PATCH v10 02/13] drm/mediatek: add *driver_data for different hardware settings

2016-11-29 Thread Daniel Kurtz 
Hi YT,


On Fri, Nov 25, 2016 at 6:34 PM, YT Shen  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.

Sorry, I have not had time to thoroughly review the new patch set, but
one quick comment:

> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h 
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index 22a33ee..cfaa760 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -78,6 +78,10 @@ struct mtk_ddp_comp_funcs {
>   struct drm_crtc_state *state);
>  };
>
> +struct mtk_ddp_comp_driver_data {
> +   unsigned int color_offset;
> +};
> +
>  struct mtk_ddp_comp {
> struct clk *clk;
> void __iomem *regs;
> @@ -85,6 +89,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;

I want to avoid adding this generic pointer here to all mtk_ddp_comp,
since this is only used by the color component.
Instead, define color specific types:

struct mtk_disp_color_data {
   unsigned int offset;
};

struct mtk_disp_color {
   struct mtk_ddp_comp ddp_comp;
   const struct mtk_disp_color_data *data;
};

Then, add another comp_type check and fetch the dev data in
mtk_drm_probe()  or maybe mtk_ddp_comp_init().

-Dan

[PATCH v10 02/13] drm/mediatek: add *driver_data for different hardware settings

2016-11-25 Thread YT Shen 
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 
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 41 -
 drivers/gpu/drm/mediatek/mtk_disp_rdma.c| 18 +++-
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c  | 71 +++--
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 29 +---
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  5 ++
 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, 160 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c 
b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 17971e4..678595c 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -35,18 +35,27 @@
 #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 DISP_REG_OVL_ADDR_MT8173   0x0f40
+#define DISP_REG_OVL_ADDR(ovl, n)  ((ovl)->data->addr + 0x20 * (n))
 
 #defineOVL_RDMA_MEM_GMC0x40402020
 
 #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_RGB (1 << 12)
 #define OVL_CON_CLRFMT_RGBA(2 << 12)
 #define OVL_CON_CLRFMT_ARGB(3 << 12)
+#define OVL_CON_CLRFMT_RGB565(ovl) ((ovl)->data->fmt_rgb565_is_0 ? \
+   0 : OVL_CON_CLRFMT_RGB)
+#define OVL_CON_CLRFMT_RGB888(ovl) ((ovl)->data->fmt_rgb565_is_0 ? \
+   OVL_CON_CLRFMT_RGB : 0)
 #defineOVL_CON_AEN BIT(8)
 #defineOVL_CON_ALPHA   0xff
 
+struct mtk_disp_ovl_data {
+   unsigned int addr;
+   bool fmt_rgb565_is_0;
+};
+
 /**
  * struct mtk_disp_ovl - DISP_OVL driver structure
  * @ddp_comp - structure containing type enum and hardware resources
@@ -55,6 +64,7 @@
 struct mtk_disp_ovl {
struct mtk_ddp_comp ddp_comp;
struct drm_crtc *crtc;
+   const struct mtk_disp_ovl_data  *data;
 };
 
 static inline struct mtk_disp_ovl *comp_to_ovl(struct mtk_ddp_comp *comp)
@@ -140,18 +150,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_disp_ovl *ovl, unsigned int fmt)
 {
switch (fmt) {
default:
case DRM_FORMAT_RGB565:
-   return OVL_CON_CLRFMT_RGB565;
+   return OVL_CON_CLRFMT_RGB565(ovl);
case DRM_FORMAT_BGR565:
-   return OVL_CON_CLRFMT_RGB565 | OVL_CON_BYTE_SWAP;
+   return OVL_CON_CLRFMT_RGB565(ovl) | OVL_CON_BYTE_SWAP;
case DRM_FORMAT_RGB888:
-   return OVL_CON_CLRFMT_RGB888;
+   return OVL_CON_CLRFMT_RGB888(ovl);
case DRM_FORMAT_BGR888:
-   return OVL_CON_CLRFMT_RGB888 | OVL_CON_BYTE_SWAP;
+   return OVL_CON_CLRFMT_RGB888(ovl) | OVL_CON_BYTE_SWAP;
case DRM_FORMAT_RGBX:
case DRM_FORMAT_RGBA:
return OVL_CON_CLRFMT_ARGB;
@@ -170,6 +180,7 @@ static unsigned int ovl_fmt_convert(unsigned int fmt)
 static void mtk_ovl_layer_config(struct mtk_ddp_comp *comp, unsigned int idx,
 struct mtk_plane_state *state)
 {
+   struct mtk_disp_ovl *ovl = comp_to_ovl(comp);
struct mtk_plane_pending_state *pending = &state->pending;
unsigned int addr = pending->addr;
unsigned int pitch = pending->pitch & 0x;
@@ -181,7 +192,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(ovl, fmt);
if (idx != 0)
con |= OVL_CON_AEN | OVL_CON_ALPHA;
 
@@ -189,7 +200,7 @@ 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 + DISP_REG_OVL_ADDR(ovl, idx));
 
if (pending->enable)