On Mar 14, 2013 1:59 PM, "Inki Dae" <<a href="mailto:inki.dae@samsung.com">inki.dae@samsung.com</a>> wrote: > > > > > -----Original Message----- > > From: Joonyoung Shim [mailto:<a href="mailto:jy0922.shim@samsung.com">jy0922.shim@samsung.com</a>] > > Sent: Thursday, March 14, 2013 11:30 AM > > To: Inki Dae > > Cc: <a href="mailto:airlied@linux.ie">airlied@linux.ie</a>; <a href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a>; > > <a href="mailto:kyungmin.park@samsung.com">kyungmin.park@samsung.com</a>; <a href="mailto:sw0312.kim@samsung.com">sw0312.kim@samsung.com</a>; 'YoungJun Cho' > > Subject: Re: [PATCH 3/7] drm/exynos: Fix G2D core mulfunctioning issue > > > > On 03/13/2013 07:53 PM, Inki Dae wrote: > > >> -----Original Message----- > > >> From: Joonyoung Shim [mailto:<a href="mailto:jy0922.shim@samsung.com">jy0922.shim@samsung.com</a>] > > >> Sent: Wednesday, March 13, 2013 7:28 PM > > >> To: Inki Dae > > >> <a href="mailto:Cc%3Aairlied@linux.ie">Cc:airlied@linux.ie</a>;<a href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a>; > > >> <a href="mailto:kyungmin.park@samsung.com">kyungmin.park@samsung.com</a>;<a href="mailto:sw0312.kim@samsung.com">sw0312.kim@samsung.com</a>; 'YoungJun Cho' > > >> Subject: Re: [PATCH 3/7] drm/exynos: Fix G2D core mulfunctioning issue > > >> > > >> On 03/13/2013 07:14 PM, Inki Dae wrote: > > >>>> -----Original Message----- > > >>>> From: Joonyoung Shim [mailto:<a href="mailto:jy0922.shim@samsung.com">jy0922.shim@samsung.com</a>] > > >>>> Sent: Wednesday, March 13, 2013 6:53 PM > > >>>> To: Inki Dae > > >>>> <a href="mailto:Cc%3Aairlied@linux.ie">Cc:airlied@linux.ie</a>;<a href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a>; > > >>>> <a href="mailto:kyungmin.park@samsung.com">kyungmin.park@samsung.com</a>;<a href="mailto:sw0312.kim@samsung.com">sw0312.kim@samsung.com</a>; 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<<a href="mailto:yj44.cho@samsung.com">yj44.cho@samsung.com</a>> > > >>>>> > > >>>>> 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. > > >> No, Once set GCF, it is set on end. Who can clear it? > > >> > > > Maybe you say that g2d_dma_start() is called by exec ioctl so the GCF > > bit > > > could be set by only last node. For this, We need to look into dma- > > driven > > > command processing. Assume that two more command lists exist and they > > are > > > executed at once. Then we CAN NOT GET each node while dma operation > > because > > > the command lists of each node are executed by dma at once. > > > > > > On other words, there is no way to enable GCF bit only in case that a > > > command list completion interrupt is needed because the need is from > > user > > > side. > > > > I requested to Youngjun Cho whether it is possible to work and to insert > > a command to set GCF bit of INTEN in command list and he said it is > > possiable and works because clears GCF bit in irq handler if command > > list completion interrupt occurs. > > > > Checked it out. It seems that the g2d dma could also access and control > other registers(not rendering relevant ones) in the command list. So we > could implement this approach that user can get event to each command list > completion more simply. > > With set_cmdlist ioctl with event, it makes GCF bit to be set to > node->cmdlist and only ACF bit without event. Afterwards, whenever each > command list is completed, G2D Core can aware of the interrupt source > enabled(GCF or ACF) so interrupt will occurs properly. > > Mr. Cho, please implement this approach and test it. > I tested it and checked working well. I'll send patch again. Thank you Best regards YJ > > > > So I think we need some policy for g2d driver. For example, if user > > wants to > > > get event to each node, all nodes from the user should be operated with > > GCF > > > bit otherwise without GCF bit. > > > > Actually now there is no way to inform completion of a set of command > > lists to user on async mode. So, i think completion event of a set of > > command lists needs also. > > > > I think it's enough to have only event to each command list because user > wants to do something only whenever one bitblt command is completed. Maybe > there is no use case to all command lists. Anyway, let's implement the above > approach first. :) > > > Thanks. > > > > > > > > Any other idea? > > > > > >>>> Thanks. > > >>>> > > >>>>> Signed-off-by: YoungJun Cho<<a href="mailto:yj44.cho@samsung.com">yj44.cho@samsung.com</a>> > > >>>>> Signed-off-by: Inki Dae<<a href="mailto:inki.dae@samsung.com">inki.dae@samsung.com</a>> > > >>>>> Signed-off-by: Kyungmin Park<<a href="mailto:kyungmin.park@samsung.com">kyungmin.park@samsung.com</a>> > > >>>>> --- > > >>>>> 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; > > _______________________________________________ > dri-devel mailing list > <a href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a>