Messages in this thread | | | Subject | Re: [PATCH 07/11] drm/meson: Add G12A support for plane handling in CRTC driver | From | Neil Armstrong <> | Date | Tue, 9 Apr 2019 11:17:30 +0200 |
| |
On 09/04/2019 10:43, Jerome Brunet wrote: > On Mon, 2019-03-25 at 15:18 +0100, Neil Armstrong wrote: >> This patch adds support for the new OSD+VD Plane blending module >> in the CRTC code by adding the G12A code to manage the blending >> module and setting the right OSD1 & VD1 plane registers. >> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> >> --- >> drivers/gpu/drm/meson/meson_crtc.c | 269 +++++++++++++++++++++++------ >> drivers/gpu/drm/meson/meson_drv.h | 4 + >> 2 files changed, 221 insertions(+), 52 deletions(-) >> >> diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c >> index 6d9311e254ef..5579f8ac3e3f 100644 >> --- a/drivers/gpu/drm/meson/meson_crtc.c >> +++ b/drivers/gpu/drm/meson/meson_crtc.c >> @@ -39,12 +39,17 @@ >> #include "meson_viu.h" >> #include "meson_registers.h" >> >> +#define MESON_G12A_VIU_OFFSET 0x5ec0 >> + >> /* CRTC definition */ >> >> struct meson_crtc { >> struct drm_crtc base; >> struct drm_pending_vblank_event *event; >> struct meson_drm *priv; >> + void (*enable_osd1)(struct meson_drm *priv); >> + void (*enable_vd1)(struct meson_drm *priv); >> + unsigned int viu_offset; >> }; >> #define to_meson_crtc(x) container_of(x, struct meson_crtc, base) >> >> @@ -80,6 +85,44 @@ static const struct drm_crtc_funcs meson_crtc_funcs = { >> >> }; >> >> +static void meson_g12a_crtc_atomic_enable(struct drm_crtc *crtc, >> + struct drm_crtc_state *old_state) >> +{ >> + struct meson_crtc *meson_crtc = to_meson_crtc(crtc); >> + struct drm_crtc_state *crtc_state = crtc->state; >> + struct meson_drm *priv = meson_crtc->priv; >> + >> + DRM_DEBUG_DRIVER("\n"); >> + >> + if (!crtc_state) { >> + DRM_ERROR("Invalid crtc_state\n"); >> + return; >> + } >> + >> + /* VD1 Preblend vertical start/end */ >> + writel(FIELD_PREP(GENMASK(11, 0), 2303), > > Could could define this mask somewhere > I don't know if the 2303 can be explained. I'd nice if it could
It's a width, I'll add a comment.
> >> + priv->io_base + _REG(VPP_PREBLEND_VD1_V_START_END)); >> + >> + /* Setup Blender */ >> + writel(crtc_state->mode.hdisplay | >> + crtc_state->mode.vdisplay << 16, >> + priv->io_base + _REG(VPP_POSTBLEND_H_SIZE)); >> + >> + writel_relaxed(0 << 16 | > > this 16 shift seems to be used for hdisplay or something. Could > define/document it a bit
Ack
> >> + (crtc_state->mode.hdisplay - 1), >> + priv->io_base + _REG(VPP_OSD1_BLD_H_SCOPE)); >> + writel_relaxed(0 << 16 | >> + (crtc_state->mode.vdisplay - 1), >> + priv->io_base + _REG(VPP_OSD1_BLD_V_SCOPE)); >> + writel_relaxed(crtc_state->mode.hdisplay << 16 | >> + crtc_state->mode.vdisplay, >> + priv->io_base + _REG(VPP_OUT_H_V_SIZE)); >> + >> + drm_crtc_vblank_on(crtc); >> + >> + priv->viu.osd1_enabled = true; >> +} >> + >> static void meson_crtc_atomic_enable(struct drm_crtc *crtc, >> struct drm_crtc_state *old_state) >> { >> @@ -110,6 +153,31 @@ static void meson_crtc_atomic_enable(struct drm_crtc *crtc, >> priv->viu.osd1_enabled = true; >> } >> >> +static void meson_g12a_crtc_atomic_disable(struct drm_crtc *crtc, >> + struct drm_crtc_state *old_state) >> +{ >> + struct meson_crtc *meson_crtc = to_meson_crtc(crtc); >> + struct meson_drm *priv = meson_crtc->priv; >> + >> + DRM_DEBUG_DRIVER("\n"); >> + >> + drm_crtc_vblank_off(crtc); >> + >> + priv->viu.osd1_enabled = false; >> + priv->viu.osd1_commit = false; >> + >> + priv->viu.vd1_enabled = false; >> + priv->viu.vd1_commit = false; >> + >> + if (crtc->state->event && !crtc->state->active) { >> + spin_lock_irq(&crtc->dev->event_lock); >> + drm_crtc_send_vblank_event(crtc, crtc->state->event); >> + spin_unlock_irq(&crtc->dev->event_lock); >> + >> + crtc->state->event = NULL; >> + } >> +} >> + >> static void meson_crtc_atomic_disable(struct drm_crtc *crtc, >> struct drm_crtc_state *old_state) >> { >> @@ -173,6 +241,53 @@ static const struct drm_crtc_helper_funcs meson_crtc_helper_funcs = { >> .atomic_disable = meson_crtc_atomic_disable, >> }; >> >> +static const struct drm_crtc_helper_funcs meson_g12a_crtc_helper_funcs = { >> + .atomic_begin = meson_crtc_atomic_begin, >> + .atomic_flush = meson_crtc_atomic_flush, >> + .atomic_enable = meson_g12a_crtc_atomic_enable, >> + .atomic_disable = meson_g12a_crtc_atomic_disable, >> +}; >> + >> +static void meson_crtc_enable_osd1(struct meson_drm *priv) >> +{ >> + writel_bits_relaxed(VPP_OSD1_POSTBLEND, VPP_OSD1_POSTBLEND, >> + priv->io_base + _REG(VPP_MISC)); >> +} >> + >> +static void meson_g12a_crtc_enable_osd1(struct meson_drm *priv) >> +{ >> + writel_relaxed(priv->viu.osd_blend_din0_scope_h, >> + priv->io_base + >> + _REG(VIU_OSD_BLEND_DIN0_SCOPE_H)); >> + writel_relaxed(priv->viu.osd_blend_din0_scope_v, >> + priv->io_base + >> + _REG(VIU_OSD_BLEND_DIN0_SCOPE_V)); >> + writel_relaxed(priv->viu.osb_blend0_size, >> + priv->io_base + >> + _REG(VIU_OSD_BLEND_BLEND0_SIZE)); >> + writel_relaxed(priv->viu.osb_blend1_size, >> + priv->io_base + >> + _REG(VIU_OSD_BLEND_BLEND1_SIZE)); >> +} >> + >> +static void meson_crtc_enable_vd1(struct meson_drm *priv) >> +{ >> + writel_bits_relaxed(VPP_VD1_PREBLEND | VPP_VD1_POSTBLEND | >> + VPP_COLOR_MNG_ENABLE, >> + VPP_VD1_PREBLEND | VPP_VD1_POSTBLEND | >> + VPP_COLOR_MNG_ENABLE, >> + priv->io_base + _REG(VPP_MISC)); >> +} >> + >> +static void meson_g12a_crtc_enable_vd1(struct meson_drm *priv) >> +{ >> + writel_relaxed(((1 << 16) | /* post bld premult*/ >> + (1 << 8) | /* post src */ >> + (1 << 4) | /* pre bld premult*/ >> + (1 << 0)), > > Could you use the BIT() macro here and possibly some defines ?
Ack
> >> + priv->io_base + _REG(VD1_BLEND_SRC_CTRL)); >> +} >> + >> void meson_crtc_irq(struct meson_drm *priv) >> { >> struct meson_crtc *meson_crtc = to_meson_crtc(priv->crtc); >> @@ -219,8 +334,8 @@ void meson_crtc_irq(struct meson_drm *priv) >> MESON_CANVAS_BLKMODE_LINEAR, 0); >> >> /* Enable OSD1 */ >> - writel_bits_relaxed(VPP_OSD1_POSTBLEND, VPP_OSD1_POSTBLEND, >> - priv->io_base + _REG(VPP_MISC)); >> + if (meson_crtc->enable_osd1) >> + meson_crtc->enable_osd1(priv); >> >> priv->viu.osd1_commit = false; >> } >> @@ -261,89 +376,133 @@ void meson_crtc_irq(struct meson_drm *priv) >> }; >> >> writel_relaxed(priv->viu.vd1_if0_gen_reg, >> - priv->io_base + _REG(VD1_IF0_GEN_REG)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_GEN_REG)); > > Could you make a local variable for priv->io_base + meson_crtc->viu_offset ? > It would be a little easier to read
Indeed
> >> writel_relaxed(priv->viu.vd1_if0_gen_reg, >> - priv->io_base + _REG(VD2_IF0_GEN_REG)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_GEN_REG)); >> writel_relaxed(priv->viu.vd1_if0_gen_reg2, >> - priv->io_base + _REG(VD1_IF0_GEN_REG2)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_GEN_REG2)); >> writel_relaxed(priv->viu.viu_vd1_fmt_ctrl, >> - priv->io_base + _REG(VIU_VD1_FMT_CTRL)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VIU_VD1_FMT_CTRL)); >> writel_relaxed(priv->viu.viu_vd1_fmt_ctrl, >> - priv->io_base + _REG(VIU_VD2_FMT_CTRL)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VIU_VD2_FMT_CTRL)); >> writel_relaxed(priv->viu.viu_vd1_fmt_w, >> - priv->io_base + _REG(VIU_VD1_FMT_W)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VIU_VD1_FMT_W)); >> writel_relaxed(priv->viu.viu_vd1_fmt_w, >> - priv->io_base + _REG(VIU_VD2_FMT_W)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VIU_VD2_FMT_W)); >> writel_relaxed(priv->viu.vd1_if0_canvas0, >> - priv->io_base + _REG(VD1_IF0_CANVAS0)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_CANVAS0)); >> writel_relaxed(priv->viu.vd1_if0_canvas0, >> - priv->io_base + _REG(VD1_IF0_CANVAS1)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_CANVAS1)); >> writel_relaxed(priv->viu.vd1_if0_canvas0, >> - priv->io_base + _REG(VD2_IF0_CANVAS0)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_CANVAS0)); >> writel_relaxed(priv->viu.vd1_if0_canvas0, >> - priv->io_base + _REG(VD2_IF0_CANVAS1)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_CANVAS1)); >> writel_relaxed(priv->viu.vd1_if0_luma_x0, >> - priv->io_base + _REG(VD1_IF0_LUMA_X0)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_LUMA_X0)); >> writel_relaxed(priv->viu.vd1_if0_luma_x0, >> - priv->io_base + _REG(VD1_IF0_LUMA_X1)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_LUMA_X1)); >> writel_relaxed(priv->viu.vd1_if0_luma_x0, >> - priv->io_base + _REG(VD2_IF0_LUMA_X0)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_LUMA_X0)); >> writel_relaxed(priv->viu.vd1_if0_luma_x0, >> - priv->io_base + _REG(VD2_IF0_LUMA_X1)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_LUMA_X1)); >> writel_relaxed(priv->viu.vd1_if0_luma_y0, >> - priv->io_base + _REG(VD1_IF0_LUMA_Y0)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_LUMA_Y0)); >> writel_relaxed(priv->viu.vd1_if0_luma_y0, >> - priv->io_base + _REG(VD1_IF0_LUMA_Y1)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_LUMA_Y1)); >> writel_relaxed(priv->viu.vd1_if0_luma_y0, >> - priv->io_base + _REG(VD2_IF0_LUMA_Y0)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_LUMA_Y0)); >> writel_relaxed(priv->viu.vd1_if0_luma_y0, >> - priv->io_base + _REG(VD2_IF0_LUMA_Y1)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_LUMA_Y1)); >> writel_relaxed(priv->viu.vd1_if0_chroma_x0, >> - priv->io_base + _REG(VD1_IF0_CHROMA_X0)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_CHROMA_X0)); >> writel_relaxed(priv->viu.vd1_if0_chroma_x0, >> - priv->io_base + _REG(VD1_IF0_CHROMA_X1)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_CHROMA_X1)); >> writel_relaxed(priv->viu.vd1_if0_chroma_x0, >> - priv->io_base + _REG(VD2_IF0_CHROMA_X0)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_CHROMA_X0)); >> writel_relaxed(priv->viu.vd1_if0_chroma_x0, >> - priv->io_base + _REG(VD2_IF0_CHROMA_X1)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_CHROMA_X1)); >> writel_relaxed(priv->viu.vd1_if0_chroma_y0, >> - priv->io_base + _REG(VD1_IF0_CHROMA_Y0)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_CHROMA_Y0)); >> writel_relaxed(priv->viu.vd1_if0_chroma_y0, >> - priv->io_base + _REG(VD1_IF0_CHROMA_Y1)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_CHROMA_Y1)); >> writel_relaxed(priv->viu.vd1_if0_chroma_y0, >> - priv->io_base + _REG(VD2_IF0_CHROMA_Y0)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_CHROMA_Y0)); >> writel_relaxed(priv->viu.vd1_if0_chroma_y0, >> - priv->io_base + _REG(VD2_IF0_CHROMA_Y1)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_CHROMA_Y1)); >> writel_relaxed(priv->viu.vd1_if0_repeat_loop, >> - priv->io_base + _REG(VD1_IF0_RPT_LOOP)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_RPT_LOOP)); >> writel_relaxed(priv->viu.vd1_if0_repeat_loop, >> - priv->io_base + _REG(VD2_IF0_RPT_LOOP)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_RPT_LOOP)); >> writel_relaxed(priv->viu.vd1_if0_luma0_rpt_pat, >> - priv->io_base + _REG(VD1_IF0_LUMA0_RPT_PAT)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_LUMA0_RPT_PAT)); >> writel_relaxed(priv->viu.vd1_if0_luma0_rpt_pat, >> - priv->io_base + _REG(VD2_IF0_LUMA0_RPT_PAT)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_LUMA0_RPT_PAT)); >> writel_relaxed(priv->viu.vd1_if0_luma0_rpt_pat, >> - priv->io_base + _REG(VD1_IF0_LUMA1_RPT_PAT)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_LUMA1_RPT_PAT)); >> writel_relaxed(priv->viu.vd1_if0_luma0_rpt_pat, >> - priv->io_base + _REG(VD2_IF0_LUMA1_RPT_PAT)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_LUMA1_RPT_PAT)); >> writel_relaxed(priv->viu.vd1_if0_chroma0_rpt_pat, >> - priv->io_base + _REG(VD1_IF0_CHROMA0_RPT_PAT)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_CHROMA0_RPT_PAT)); >> writel_relaxed(priv->viu.vd1_if0_chroma0_rpt_pat, >> - priv->io_base + _REG(VD2_IF0_CHROMA0_RPT_PAT)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_CHROMA0_RPT_PAT)); >> writel_relaxed(priv->viu.vd1_if0_chroma0_rpt_pat, >> - priv->io_base + _REG(VD1_IF0_CHROMA1_RPT_PAT)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_CHROMA1_RPT_PAT)); >> writel_relaxed(priv->viu.vd1_if0_chroma0_rpt_pat, >> - priv->io_base + _REG(VD2_IF0_CHROMA1_RPT_PAT)); >> - writel_relaxed(0, priv->io_base + _REG(VD1_IF0_LUMA_PSEL)); >> - writel_relaxed(0, priv->io_base + _REG(VD1_IF0_CHROMA_PSEL)); >> - writel_relaxed(0, priv->io_base + _REG(VD2_IF0_LUMA_PSEL)); >> - writel_relaxed(0, priv->io_base + _REG(VD2_IF0_CHROMA_PSEL)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_CHROMA1_RPT_PAT)); >> + writel_relaxed(0, priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_LUMA_PSEL)); >> + writel_relaxed(0, priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_CHROMA_PSEL)); >> + writel_relaxed(0, priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_LUMA_PSEL)); >> + writel_relaxed(0, priv->io_base + meson_crtc->viu_offset + >> + _REG(VD2_IF0_CHROMA_PSEL)); >> writel_relaxed(priv->viu.vd1_range_map_y, >> - priv->io_base + _REG(VD1_IF0_RANGE_MAP_Y)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_RANGE_MAP_Y)); >> writel_relaxed(priv->viu.vd1_range_map_cb, >> - priv->io_base + _REG(VD1_IF0_RANGE_MAP_CB)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_RANGE_MAP_CB)); >> writel_relaxed(priv->viu.vd1_range_map_cr, >> - priv->io_base + _REG(VD1_IF0_RANGE_MAP_CR)); >> + priv->io_base + meson_crtc->viu_offset + >> + _REG(VD1_IF0_RANGE_MAP_CR)); >> writel_relaxed(0x78404, >> priv->io_base + _REG(VPP_SC_MISC)); >> writel_relaxed(priv->viu.vpp_pic_in_height, >> @@ -389,11 +548,8 @@ void meson_crtc_irq(struct meson_drm *priv) >> writel_relaxed(0x42, priv->io_base + _REG(VPP_SCALE_COEF_IDX)); >> >> /* Enable VD1 */ >> - writel_bits_relaxed(VPP_VD1_PREBLEND | VPP_VD1_POSTBLEND | >> - VPP_COLOR_MNG_ENABLE, >> - VPP_VD1_PREBLEND | VPP_VD1_POSTBLEND | >> - VPP_COLOR_MNG_ENABLE, >> - priv->io_base + _REG(VPP_MISC)); >> + if (meson_crtc->enable_vd1) >> + meson_crtc->enable_vd1(priv); >> >> priv->viu.vd1_commit = false; >> } >> @@ -430,7 +586,16 @@ int meson_crtc_create(struct meson_drm *priv) >> return ret; >> } >> >> - drm_crtc_helper_add(crtc, &meson_crtc_helper_funcs); >> + if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) { >> + meson_crtc->enable_osd1 = meson_g12a_crtc_enable_osd1; >> + meson_crtc->enable_vd1 = meson_g12a_crtc_enable_vd1; >> + meson_crtc->viu_offset = MESON_G12A_VIU_OFFSET; >> + drm_crtc_helper_add(crtc, &meson_g12a_crtc_helper_funcs); >> + } else { >> + meson_crtc->enable_osd1 = meson_crtc_enable_osd1; >> + meson_crtc->enable_vd1 = meson_crtc_enable_vd1; >> + drm_crtc_helper_add(crtc, &meson_crtc_helper_funcs); >> + } >> >> priv->crtc = crtc; >> >> diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h >> index 214a7cb18ce2..9614baa836b9 100644 >> --- a/drivers/gpu/drm/meson/meson_drv.h >> +++ b/drivers/gpu/drm/meson/meson_drv.h >> @@ -62,6 +62,10 @@ struct meson_drm { >> uint32_t osd_sc_h_phase_step; >> uint32_t osd_sc_h_ctrl0; >> uint32_t osd_sc_v_ctrl0; >> + uint32_t osd_blend_din0_scope_h; >> + uint32_t osd_blend_din0_scope_v; >> + uint32_t osb_blend0_size; >> + uint32_t osb_blend1_size; >> >> bool vd1_enabled; >> bool vd1_commit; > >
Will fix in a follow-up patch.
Neil
| |