[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <56eec57c795bdea59baac3ad3d25e015d4c596c3.camel@mediatek.com>
Date: Fri, 16 Feb 2024 02:16:58 +0000
From: Shawn Sung (宋孝謙) <Shawn.Sung@...iatek.com>
To: "angelogioacchino.delregno@...labora.com"
<angelogioacchino.delregno@...labora.com>, "chunkuang.hu@...nel.org"
<chunkuang.hu@...nel.org>
CC: "linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Bibby Hsieh (謝濟遠) <Bibby.Hsieh@...iatek.com>,
"jason-ch.chen@...iatek.corp-partner.google.com"
<jason-ch.chen@...iatek.corp-partner.google.com>,
Nancy Lin (林欣螢) <Nancy.Lin@...iatek.com>,
"daniel@...ll.ch" <daniel@...ll.ch>, "p.zabel@...gutronix.de"
<p.zabel@...gutronix.de>, CK Hu (胡俊光)
<ck.hu@...iatek.com>, "seanpaul@...omium.org" <seanpaul@...omium.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"airlied@...il.com" <airlied@...il.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "matthias.bgg@...il.com"
<matthias.bgg@...il.com>, "fshao@...omium.org" <fshao@...omium.org>
Subject: Re: [PATCH v5 10/13] drm/mediatek: Support CRC in display driver
Hi Angelo,
On Thu, 2024-02-15 at 12:06 +0100, AngeloGioacchino Del Regno wrote:
> Il 15/02/24 11:11, Hsiao Chien Sung ha scritto:
> > Register CRC related function pointers to support
> > CRC retrieval.
> >
> > Signed-off-by: Hsiao Chien Sung <shawn.sung@...iatek.com>
> > ---
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 239
> > ++++++++++++++++++++
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 39 ++++
> > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 3 +
> > 3 files changed, 281 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index 14cf75fa217f9..6cb1ed419dee7 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -68,6 +68,9 @@ struct mtk_drm_crtc {
> > /* lock for display hardware access */
> > struct mutex hw_lock;
> > bool config_updating;
> > +
> > + struct mtk_ddp_comp *crc_provider;
> > + unsigned int frames;
> > };
> >
> > struct mtk_crtc_state {
> > @@ -635,6 +638,14 @@ static void mtk_crtc_ddp_irq(void *data)
> > struct drm_crtc *crtc = data;
> > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> > struct mtk_drm_private *priv = crtc->dev->dev_private;
> > + struct mtk_ddp_comp *comp = mtk_crtc->crc_provider;
> > +
> > + /*
> > + * crc providers should make sure the crc is always correct
> > + * by resetting it in .crc_read()
> > + */
> > + if (crtc->crc.opened)
> > + comp->funcs->crc_read(comp->dev);
> >
> > #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> > if (!priv->data->shadow_register && !mtk_crtc-
> > >cmdq_client.chan)
> > @@ -646,6 +657,24 @@ static void mtk_crtc_ddp_irq(void *data)
> > if (!priv->data->shadow_register)
> > mtk_crtc_ddp_config(crtc, NULL);
> > #endif
> > +
> > + /*
> > + * drm_crtc_add_crc_entry() could take more than 50ms to finish
> > + * put it at the end of the isr
> > + */
> > + if (crtc->crc.opened) {
> > + /*
> > + * skip the first crc because the first frame is
> > configured by
> > + * mtk_crtc_ddp_hw_init() when atomic enable
> > + */
> > + if (++mtk_crtc->frames > 1) {
> > + drm_crtc_add_crc_entry(crtc, true,
> > + drm_crtc_vblank_count(cr
> > tc),
> > + comp->funcs-
> > >crc_entry(comp->dev));
> > + }
> > + } else {
> > + mtk_crtc->frames = 0;
> > + }
> > mtk_drm_finish_page_flip(mtk_crtc);
> > }
> >
> > @@ -704,6 +733,40 @@ static void mtk_drm_crtc_update_output(struct
> > drm_crtc *crtc,
> > }
> > }
> >
> > +static int mtk_drm_crtc_set_crc_source(struct drm_crtc *crtc,
> > const char *src)
> > +{
> > + if (src && strcmp(src, "auto") != 0) {
> > + DRM_ERROR("%s(crtc-%d): unknown source '%s'\n",
> > + __func__, drm_crtc_index(crtc), src);
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> > +
> > +static int mtk_drm_crtc_verify_crc_source(struct drm_crtc *crtc,
> > + const char *src,
> > + size_t *cnt)
> > +{
> > + struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> > + struct mtk_ddp_comp *comp = mtk_crtc->crc_provider;
> > +
> > + if (!comp) {
> > + DRM_ERROR("%s(crtc-%d): no crc provider\n",
> > + __func__, drm_crtc_index(crtc));
> > + return -ENOENT;
> > + }
> > +
> > + if (src && strcmp(src, "auto") != 0) {
> > + DRM_ERROR("%s(crtc-%d): unknown source '%s'\n",
> > + __func__, drm_crtc_index(crtc), src);
> > + return -EINVAL;
> > + }
> > +
> > + *cnt = comp->funcs->crc_cnt(comp->dev);
> > +
> > + return 0;
> > +}
> > +
> > int mtk_drm_crtc_plane_check(struct drm_crtc *crtc, struct
> > drm_plane *plane,
> > struct mtk_plane_state *state)
> > {
> > @@ -841,6 +904,8 @@ static const struct drm_crtc_funcs
> > mtk_crtc_funcs = {
> > .atomic_destroy_state = mtk_drm_crtc_destroy_state,
> > .enable_vblank = mtk_drm_crtc_enable_vblank,
> > .disable_vblank = mtk_drm_crtc_disable_vblank,
> > + .set_crc_source = mtk_drm_crtc_set_crc_source,
> > + .verify_crc_source = mtk_drm_crtc_verify_crc_source,
> > };
> >
> > static const struct drm_crtc_helper_funcs mtk_crtc_helper_funcs =
> > {
> > @@ -1033,6 +1098,11 @@ int mtk_drm_crtc_create(struct drm_device
> > *drm_dev,
> >
> > if (comp->funcs->ctm_set)
> > has_ctm = true;
> > +
> > + if (comp->funcs->crc_cnt &&
> > + comp->funcs->crc_entry &&
> > + comp->funcs->crc_read)
> > + mtk_crtc->crc_provider = comp;
> > }
> >
> > mtk_ddp_comp_register_vblank_cb(comp, mtk_crtc_ddp_irq,
> > @@ -1137,3 +1207,172 @@ int mtk_drm_crtc_create(struct drm_device
> > *drm_dev,
> >
> > return 0;
> > }
> > +
> > +void mtk_drm_crc_init(struct mtk_drm_crc *crc,
> > + const u32 *crc_offset_table, size_t crc_count,
> > + u32 reset_offset, u32 reset_mask)
> > +{
> > + crc->ofs = crc_offset_table;
> > + crc->cnt = crc_count;
> > + crc->rst_ofs = reset_offset;
> > + crc->rst_msk = reset_mask;
> > + crc->va = kcalloc(crc->cnt, sizeof(*crc->va), GFP_KERNEL);
> > + if (!crc->va) {
> > + DRM_ERROR("failed to allocate memory for crc\n");
> > + crc->cnt = 0;
> > + }
> > +}
> > +
> > +void mtk_drm_crc_read(struct mtk_drm_crc *crc, void __iomem *reg)
> > +{
>
> u32 n; goes here
>
> > + if (!crc->cnt || !crc->ofs || !crc->va)
> > + return;
> > +
> > +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> > + /* sync to see the most up-to-date copy of the DMA buffer */
> > + dma_sync_single_for_cpu(crc->cmdq_client.chan->mbox->dev,
> > + crc->pa, crc->cnt * sizeof(*crc->va),
> > + DMA_FROM_DEVICE);
> > +#else
> > + /* read crc with cpu for the platforms without cmdq */
> > + {
>
> then you don't need the braces.
>
> Regards,
> Angelo
>
The variable n is placed within the braces because when CONFIG_MTK_CMDQ
is defined, the variable is not being used and causes a build error.
This is a workaround in order to have only one #if macro in that
function for a more clean code.
Thanks,
Shawn
Powered by blists - more mailing lists