RE: [PATCH 3/7] drm/exynos: Fix G2D core mulfunctioning issue

[Inki Dae <inki.dae@samsung.com>]

> -----Original Message-----
> From: Joonyoung Shim [mailto:jy0922.shim@samsung.com]
> Sent: Wednesday, March 13, 2013 6:53 PM
> To: Inki Dae
> Cc: airlied@linux.ie; dri-devel@lists.freedesktop.org;
> kyungmin.park@samsung.com; sw0312.kim@samsung.com; YoungJun Cho
> Subject: Re: [PATCH 3/7] drm/exynos: Fix G2D core mulfunctioning issue
> 
> On 03/13/2013 06:04 PM, Inki Dae wrote:
> > From: YoungJun Cho <yj44.cho@samsung.com>
> >
> > This patch fixes G2D core mulfunctioning issue once g2d dma is started.
> > Without 'DMA_HOLD_CMD_REG' register setting, there is only one interrupt
> > after the execution to all command lists have been completed. And that
> > induces watchdog. So this patch sets 'LIST_HOLD' command to the register
> > so that command execution interrupt can be occured whenever each command
> > list execution is finished.
> 
> No, this problem occurs as GCF bit of INTEN_REG register is enabled
> always. If wants to raise interrupt immediately after a command list
> finished, GCF bit should be enabled, and it also needs to enable of LIST
> Hold bit of DMA_HOLD_CMD_REG register. If one of the two is missed out,
> g2d hardware will not work normally sometimes.

Right, these two things(LIST HOLD command and GCF interrupt enabling) should
be pair.

> 
> This patch is just workaround and it can happen performance issue
> because g2d hardware stops a moment whenever a command list finished.
> 
> So, we need the way which enable GCF bit only when a command list
> completion interrupt needs.
> 

Agree. How about this? If node->event isn't NULL then set GCF to INTEN
register in g2d_dma_start(). For this way, I already mentioned through
internal email thread.

> Thanks.
> 
> > Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> > Signed-off-by: Inki Dae <inki.dae@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >   drivers/gpu/drm/exynos/exynos_drm_g2d.c |   13 ++++++++-----
> >   1 files changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> > index 095520f..91bc4cc 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> > @@ -82,7 +82,7 @@
> >   #define G2D_DMA_LIST_DONE_COUNT_OFFSET	17
> >
> >   /* G2D_DMA_HOLD_CMD */
> > -#define G2D_USET_HOLD			(1 << 2)
> > +#define G2D_USER_HOLD			(1 << 2)
> >   #define G2D_LIST_HOLD			(1 << 1)
> >   #define G2D_BITBLT_HOLD			(1 << 0)
> >
> > @@ -863,10 +863,13 @@ int exynos_g2d_set_cmdlist_ioctl(struct drm_device
> *drm_dev, void *data,
> >   	cmdlist->data[cmdlist->last++] = G2D_SRC_BASE_ADDR;
> >   	cmdlist->data[cmdlist->last++] = 0;
> >
> > -	if (node->event) {
> > -		cmdlist->data[cmdlist->last++] = G2D_DMA_HOLD_CMD;
> > -		cmdlist->data[cmdlist->last++] = G2D_LIST_HOLD;
> > -	}
> > +	/*
> > +	 * 'LIST_HOLD' command should be set to the DMA_HOLD_CMD_REG
> > +	 * if user wants G2D interrupt event once each command list or
> > +	 * BitBLT command execution is finished.
> > +	 */
> > +	cmdlist->data[cmdlist->last++] = G2D_DMA_HOLD_CMD;
> > +	cmdlist->data[cmdlist->last++] = G2D_LIST_HOLD;
> >
> >   	/* Check size of cmdlist: last 2 is about G2D_BITBLT_START */
> >   	size = cmdlist->last + req->cmd_nr * 2 + req->cmd_buf_nr * 2 + 2;