[PATCH v5] drm/mediatek: Add support for AR30 and BA30

Chun-Kuang Hu chunkuang.hu at kernel.org
Wed Feb 1 07:30:02 PST 2023


Hi, Justin:

Justin Green <greenjustin at chromium.org> 於 2023年1月31日 週二 下午11:04寫道:
>
> Hi Chun-Kuang,
> Thanks for the review! Will try to have a new patch out later today.
>
> Re MT8195 RDMA: Yes, the MT8195 RDMA has a 10 bit mode, but I left
> this unimplemented because I have no means of testing it
> unfortunately; ChromeOS does not use this hardware.
>
> I also wanted to mention, I think this patch might conflict with the
> in-flight ovl-adaptor patch. Is there a protocol for a situation like
> this?

If I prefer the order of the two series, I would told you. But I have
no preference of these two series, so you may decide these two serious
are independent or dependent. If independent, I would apply the one
which is ready first, so the other one should rebase. If dependent,
you should discuss with Nancy the order of these two serious, the
second series should rebase on first series.

Regards,
Chun-Kuang.

>
> Regards,
> Justin
>
> On Mon, Jan 30, 2023 at 7:28 PM Chun-Kuang Hu <chunkuang.hu at kernel.org> wrote:
> >
> > Hi, Justin:
> >
> > Justin Green <greenjustin at chromium.org> 於 2023年1月31日 週二 上午4:36寫道:
> > >
> > > Add support for AR30 and BA30 pixel formats to the Mediatek DRM driver.
> > >
> > > Tested using "modetest -P" on an MT8195.
> > >
> > > Signed-off-by: Justin Green <greenjustin at chromium.org>
> > > ---
> > > v2:
> > >  * Rebase and resolve merge conflicts with the AFBC patch.
> > > v3:
> > >  * Moved 10-bit support detection to mtk_disk_ovl.c
> > > v4:
> > >  * Moved formats to mtk_disp_ovl.c and mtk_disp_rdma.c
> > > v5:
> > >  * Minor style adjustments per checkpatch.pl
> > >
> > >  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |  4 ++
> > >  drivers/gpu/drm/mediatek/mtk_disp_ovl.c     | 75 +++++++++++++++++++++
> > >  drivers/gpu/drm/mediatek/mtk_disp_rdma.c    | 27 ++++++++
> > >  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |  4 +-
> > >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  4 ++
> > >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 20 ++++++
> > >  drivers/gpu/drm/mediatek/mtk_drm_plane.c    | 28 +++-----
> > >  drivers/gpu/drm/mediatek/mtk_drm_plane.h    |  3 +-
> > >  8 files changed, 146 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > index 33e61a136bbc..6ad22ce75b81 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > @@ -96,6 +96,8 @@ void mtk_ovl_register_vblank_cb(struct device *dev,
> > >  void mtk_ovl_unregister_vblank_cb(struct device *dev);
> > >  void mtk_ovl_enable_vblank(struct device *dev);
> > >  void mtk_ovl_disable_vblank(struct device *dev);
> > > +const u32 *mtk_ovl_get_formats(struct device *dev);
> > > +size_t mtk_ovl_get_num_formats(struct device *dev);
> > >
> > >  void mtk_rdma_bypass_shadow(struct device *dev);
> > >  int mtk_rdma_clk_enable(struct device *dev);
> > > @@ -122,4 +124,6 @@ void mtk_mdp_rdma_start(struct device *dev, struct cmdq_pkt *cmdq_pkt);
> > >  void mtk_mdp_rdma_stop(struct device *dev, struct cmdq_pkt *cmdq_pkt);
> > >  void mtk_mdp_rdma_config(struct device *dev, struct mtk_mdp_rdma_cfg *cfg,
> > >                          struct cmdq_pkt *cmdq_pkt);
> > > +const u32 *mtk_rdma_get_formats(struct device *dev);
> > > +size_t mtk_rdma_get_num_formats(struct device *dev);
> > >  #endif
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > index 84daeaffab6a..1db70a77560f 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > @@ -41,6 +41,7 @@
> > >  #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_MT2701               0x0040
> > > +#define DISP_REG_OVL_CLRFMT_EXT                        0x02D0
> > >  #define DISP_REG_OVL_ADDR_MT8173               0x0f40
> > >  #define DISP_REG_OVL_ADDR(ovl, n)              ((ovl)->data->addr + 0x20 * (n))
> > >  #define DISP_REG_OVL_HDR_ADDR(ovl, n)          ((ovl)->data->addr + 0x20 * (n) + 0x04)
> > > @@ -61,11 +62,45 @@
> > >                                         0 : OVL_CON_CLRFMT_RGB)
> > >  #define OVL_CON_CLRFMT_RGB888(ovl)     ((ovl)->data->fmt_rgb565_is_0 ? \
> > >                                         OVL_CON_CLRFMT_RGB : 0)
> > > +#define OVL_CON_CLRFMT_BIT_DEPTH_MASK(ovl)     (0xFF << 4 * (ovl))
> > > +#define OVL_CON_CLRFMT_BIT_DEPTH(depth, ovl)   (depth << 4 * (ovl))
> > > +#define OVL_CON_CLRFMT_8_BIT                   0x00
> > > +#define OVL_CON_CLRFMT_10_BIT                  0x01
> > >  #define        OVL_CON_AEN             BIT(8)
> > >  #define        OVL_CON_ALPHA           0xff
> > >  #define        OVL_CON_VIRT_FLIP       BIT(9)
> > >  #define        OVL_CON_HORZ_FLIP       BIT(10)
> > >
> > > +static const u32 formats_mt8173[] = {
> > > +       DRM_FORMAT_XRGB8888,
> > > +       DRM_FORMAT_ARGB8888,
> > > +       DRM_FORMAT_BGRX8888,
> > > +       DRM_FORMAT_BGRA8888,
> > > +       DRM_FORMAT_ABGR8888,
> > > +       DRM_FORMAT_XBGR8888,
> > > +       DRM_FORMAT_RGB888,
> > > +       DRM_FORMAT_BGR888,
> > > +       DRM_FORMAT_RGB565,
> > > +       DRM_FORMAT_UYVY,
> > > +       DRM_FORMAT_YUYV,
> > > +};
> > > +
> > > +static const u32 formats_mt8195[] = {
> > > +       DRM_FORMAT_XRGB8888,
> > > +       DRM_FORMAT_ARGB8888,
> > > +       DRM_FORMAT_ARGB2101010,
> > > +       DRM_FORMAT_BGRX8888,
> > > +       DRM_FORMAT_BGRA8888,
> > > +       DRM_FORMAT_BGRA1010102,
> > > +       DRM_FORMAT_ABGR8888,
> > > +       DRM_FORMAT_XBGR8888,
> > > +       DRM_FORMAT_RGB888,
> > > +       DRM_FORMAT_BGR888,
> > > +       DRM_FORMAT_RGB565,
> > > +       DRM_FORMAT_UYVY,
> > > +       DRM_FORMAT_YUYV,
> > > +};
> > > +
> > >  struct mtk_disp_ovl_data {
> > >         unsigned int addr;
> > >         unsigned int gmc_bits;
> > > @@ -73,6 +108,7 @@ struct mtk_disp_ovl_data {
> > >         bool fmt_rgb565_is_0;
> > >         bool smi_id_en;
> > >         bool supports_afbc;
> > > +       bool supports_10bit;
> >
> > For different SoC, the supported formats may be numerous combination,
> > so I would like to place the formats directly in private date. That
> > means
> >
> > const u32 formats;
> > size_t num_formats
> >
> > >  };
> > >
> > >  /*
> > > @@ -188,6 +224,26 @@ static void mtk_ovl_set_afbc(struct mtk_disp_ovl *ovl, struct cmdq_pkt *cmdq_pkt
> > >                            DISP_REG_OVL_DATAPATH_CON, OVL_LAYER_AFBC_EN(idx));
> > >  }
> > >
> > > +static void mtk_ovl_set_bit_depth(struct device *dev, int idx, u32 format,
> > > +                                 struct cmdq_pkt *cmdq_pkt)
> > > +{
> > > +       struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
> > > +       unsigned int reg;
> > > +       unsigned int bit_depth = OVL_CON_CLRFMT_8_BIT;
> > > +
> > > +       reg = readl(ovl->regs + DISP_REG_OVL_CLRFMT_EXT);
> > > +       reg &= ~OVL_CON_CLRFMT_BIT_DEPTH_MASK(idx);
> > > +
> > > +       if (format == DRM_FORMAT_RGBA1010102 ||
> > > +           format == DRM_FORMAT_BGRA1010102 ||
> > > +           format == DRM_FORMAT_ARGB2101010)
> > > +               bit_depth = OVL_CON_CLRFMT_10_BIT;
> > > +
> > > +       reg |= OVL_CON_CLRFMT_BIT_DEPTH(bit_depth, idx);
> > > +
> > > +       mtk_ddp_write(cmdq_pkt, reg, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_CLRFMT_EXT);
> > > +}
> > > +
> > >  void mtk_ovl_config(struct device *dev, unsigned int w,
> > >                     unsigned int h, unsigned int vrefresh,
> > >                     unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
> > > @@ -216,6 +272,21 @@ unsigned int mtk_ovl_supported_rotations(struct device *dev)
> > >                DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y;
> > >  }
> > >
> > > +const u32 *mtk_ovl_get_formats(struct device *dev)
> > > +{
> > > +       struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
> > > +
> > > +       return ovl->data->supports_10bit ? formats_mt8195 : formats_mt8173;
> > > +}
> > > +
> > > +size_t mtk_ovl_get_num_formats(struct device *dev)
> > > +{
> > > +       struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
> > > +
> > > +       return ovl->data->supports_10bit ? ARRAY_SIZE(formats_mt8195)
> > > +               : ARRAY_SIZE(formats_mt8173);
> > > +}
> > > +
> > >  int mtk_ovl_layer_check(struct device *dev, unsigned int idx,
> > >                         struct mtk_plane_state *mtk_state)
> > >  {
> > > @@ -302,9 +373,11 @@ static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt)
> > >                 return OVL_CON_CLRFMT_ARGB8888;
> > >         case DRM_FORMAT_BGRX8888:
> > >         case DRM_FORMAT_BGRA8888:
> > > +       case DRM_FORMAT_BGRA1010102:
> > >                 return OVL_CON_CLRFMT_ARGB8888 | OVL_CON_BYTE_SWAP;
> > >         case DRM_FORMAT_XRGB8888:
> > >         case DRM_FORMAT_ARGB8888:
> > > +       case DRM_FORMAT_ARGB2101010:
> > >                 return OVL_CON_CLRFMT_RGBA8888;
> > >         case DRM_FORMAT_XBGR8888:
> > >         case DRM_FORMAT_ABGR8888:
> > > @@ -388,6 +461,7 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
> > >                                       &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_PITCH_MSB(idx));
> > >         }
> > >
> > > +       mtk_ovl_set_bit_depth(dev, idx, fmt, cmdq_pkt);
> > >         mtk_ovl_layer_on(dev, idx, cmdq_pkt);
> > >  }
> > >
> > > @@ -541,6 +615,7 @@ static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
> > >         .fmt_rgb565_is_0 = true,
> > >         .smi_id_en = true,
> > >         .supports_afbc = true,
> > > +       .supports_10bit = true,
> > >  };
> > >
> > >  static const struct of_device_id 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 0ec2e4049e07..7937f6fef3b4 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > > @@ -54,8 +54,23 @@
> > >
> > >  #define RDMA_MEM_GMC                           0x40402020
> > >
> > > +static const u32 formats_mt8173[] = {
> > > +       DRM_FORMAT_XRGB8888,
> > > +       DRM_FORMAT_ARGB8888,
> > > +       DRM_FORMAT_BGRX8888,
> > > +       DRM_FORMAT_BGRA8888,
> > > +       DRM_FORMAT_ABGR8888,
> > > +       DRM_FORMAT_XBGR8888,
> > > +       DRM_FORMAT_RGB888,
> > > +       DRM_FORMAT_BGR888,
> > > +       DRM_FORMAT_RGB565,
> > > +       DRM_FORMAT_UYVY,
> > > +       DRM_FORMAT_YUYV,
> > > +};
> > > +
> > >  struct mtk_disp_rdma_data {
> > >         unsigned int fifo_size;
> > > +       bool supports_10bit;
> > >  };
> > >
> > >  /*
> > > @@ -262,6 +277,17 @@ void mtk_rdma_layer_config(struct device *dev, unsigned int idx,
> > >
> > >  }
> > >
> > > +// TODO: Add support for 10 bit mode.
> >
> > /* ... */
> >
> > Does mt8195 rdma support 10 bits mode? If yes, why not implement it in
> > this patch? If no, I think it's not necessary to comment this.
> >
> > > +const u32 *mtk_rdma_get_formats(struct device *dev)
> > > +{
> > > +       return formats_mt8173;
> > > +}
> > > +
> > > +size_t mtk_rdma_get_num_formats(struct device *dev)
> > > +{
> > > +       return ARRAY_SIZE(formats_mt8173);
> > > +}
> > > +
> > >  static int mtk_disp_rdma_bind(struct device *dev, struct device *master,
> > >                               void *data)
> > >  {
> > > @@ -372,6 +398,7 @@ static const struct mtk_disp_rdma_data mt8183_rdma_driver_data = {
> > >
> > >  static const struct mtk_disp_rdma_data mt8195_rdma_driver_data = {
> > >         .fifo_size = 1920,
> > > +       .supports_10bit = true,
> > >  };
> > >
> > >  static const struct of_device_id mtk_disp_rdma_driver_dt_match[] = {
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > > index 112615817dcb..2ba01c484228 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > > @@ -856,7 +856,9 @@ static int mtk_drm_crtc_init_comp_planes(struct drm_device *drm_dev,
> > >                                 BIT(pipe),
> > >                                 mtk_drm_crtc_plane_type(mtk_crtc->layer_nr,
> > >                                                         num_planes),
> > > -                               mtk_ddp_comp_supported_rotations(comp));
> > > +                               mtk_ddp_comp_supported_rotations(comp),
> > > +                               mtk_ddp_comp_get_formats(comp),
> > > +                               mtk_ddp_comp_get_num_formats(comp));
> > >                 if (ret)
> > >                         return ret;
> > >
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > > index 6b6d5335c834..4a4c1928f83d 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > > @@ -359,6 +359,8 @@ static const struct mtk_ddp_comp_funcs ddp_ovl = {
> > >         .layer_config = mtk_ovl_layer_config,
> > >         .bgclr_in_on = mtk_ovl_bgclr_in_on,
> > >         .bgclr_in_off = mtk_ovl_bgclr_in_off,
> > > +       .get_formats = mtk_ovl_get_formats,
> > > +       .get_num_formats = mtk_ovl_get_num_formats,
> > >  };
> > >
> > >  static const struct mtk_ddp_comp_funcs ddp_postmask = {
> > > @@ -381,6 +383,8 @@ static const struct mtk_ddp_comp_funcs ddp_rdma = {
> > >         .disable_vblank = mtk_rdma_disable_vblank,
> > >         .layer_nr = mtk_rdma_layer_nr,
> > >         .layer_config = mtk_rdma_layer_config,
> > > +       .get_formats = mtk_rdma_get_formats,
> > > +       .get_num_formats = mtk_rdma_get_num_formats,
> > >  };
> > >
> > >  static const struct mtk_ddp_comp_funcs ddp_ufoe = {
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > > index 2d0052c23dcb..782c4a44f675 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > > @@ -71,6 +71,8 @@ struct mtk_ddp_comp_funcs {
> > >         void (*bgclr_in_off)(struct device *dev);
> > >         void (*ctm_set)(struct device *dev,
> > >                         struct drm_crtc_state *state);
> > > +       const u32 *(*get_formats)(struct device *dev);
> > > +       size_t (*get_num_formats)(struct device *dev);
> >
> > I think you have do two things in this patch. One is adding ddp
> > component interface to get formats. Another one is mt8195 ovl support
> > 10 bits format. So break this patch into two patches.
> >
> > Regards,
> > Chun-Kuang.
> >
> > >  };
> > >
> > >  struct mtk_ddp_comp {
> > > @@ -151,6 +153,24 @@ unsigned int mtk_ddp_comp_supported_rotations(struct mtk_ddp_comp *comp)
> > >         return 0;
> > >  }
> > >
> > > +static inline
> > > +const u32 *mtk_ddp_comp_get_formats(struct mtk_ddp_comp *comp)
> > > +{
> > > +       if (comp->funcs && comp->funcs->get_formats)
> > > +               return comp->funcs->get_formats(comp->dev);
> > > +
> > > +       return NULL;
> > > +}
> > > +
> > > +static inline
> > > +size_t mtk_ddp_comp_get_num_formats(struct mtk_ddp_comp *comp)
> > > +{
> > > +       if (comp->funcs && comp->funcs->get_num_formats)
> > > +               return comp->funcs->get_num_formats(comp->dev);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  static inline unsigned int mtk_ddp_comp_layer_nr(struct mtk_ddp_comp *comp)
> > >  {
> > >         if (comp->funcs && comp->funcs->layer_nr)
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> > > index d54fbf34b000..03e63c29111e 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> > > @@ -19,20 +19,6 @@
> > >  #include "mtk_drm_gem.h"
> > >  #include "mtk_drm_plane.h"
> > >
> > > -static const u32 formats[] = {
> > > -       DRM_FORMAT_XRGB8888,
> > > -       DRM_FORMAT_ARGB8888,
> > > -       DRM_FORMAT_BGRX8888,
> > > -       DRM_FORMAT_BGRA8888,
> > > -       DRM_FORMAT_ABGR8888,
> > > -       DRM_FORMAT_XBGR8888,
> > > -       DRM_FORMAT_RGB888,
> > > -       DRM_FORMAT_BGR888,
> > > -       DRM_FORMAT_RGB565,
> > > -       DRM_FORMAT_UYVY,
> > > -       DRM_FORMAT_YUYV,
> > > -};
> > > -
> > >  static const u64 modifiers[] = {
> > >         DRM_FORMAT_MOD_LINEAR,
> > >         DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 |
> > > @@ -315,13 +301,21 @@ static const struct drm_plane_helper_funcs mtk_plane_helper_funcs = {
> > >
> > >  int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane,
> > >                    unsigned long possible_crtcs, enum drm_plane_type type,
> > > -                  unsigned int supported_rotations)
> > > +                  unsigned int supported_rotations, const u32 *formats,
> > > +                  size_t num_formats)
> > >  {
> > >         int err;
> > >
> > > +       if (!formats || !num_formats) {
> > > +               DRM_ERROR("no pixel formats for plane\n");
> > > +               return -EINVAL;
> > > +       }
> > > +
> > >         err = drm_universal_plane_init(dev, plane, possible_crtcs,
> > > -                                      &mtk_plane_funcs, formats,
> > > -                                      ARRAY_SIZE(formats), modifiers, type, NULL);
> > > +                                      &mtk_plane_funcs,
> > > +                                      formats, num_formats,
> > > +                                      modifiers, type, NULL);
> > > +
> > >         if (err) {
> > >                 DRM_ERROR("failed to initialize plane\n");
> > >                 return err;
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
> > > index 8f39011cdbfc..99aff7da0831 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
> > > @@ -48,6 +48,7 @@ to_mtk_plane_state(struct drm_plane_state *state)
> > >
> > >  int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane,
> > >                    unsigned long possible_crtcs, enum drm_plane_type type,
> > > -                  unsigned int supported_rotations);
> > > +                  unsigned int supported_rotations, const u32 *formats,
> > > +                  size_t num_formats);
> > >
> > >  #endif
> > > --
> > > 2.39.1.456.gfc5497dd1b-goog
> > >



More information about the Linux-mediatek mailing list