| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <783a13d05378fb29738933572b80ca15b375c565.camel@baylibre.com>
Date: Tue, 09 Apr 2019 10:43:20 +0200
From: Jerome Brunet <jbrunet@...libre.com>
To: Neil Armstrong <narmstrong@...libre.com>,
dri-devel@...ts.freedesktop.org
Cc: linux-amlogic@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 07/11] drm/meson: Add G12A support for plane handling in
CRTC driver
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@...libre.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
> + 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
> + (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 ?
> + 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
> 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;
Powered by blists - more mailing lists