On Mar 14, 2013 1:59 PM, "Inki Dae" <inki.dae@xxxxxxxxxxx> wrote:
>
>
>
> > -----Original Message-----
> > From: Joonyoung Shim [mailto:jy0922.shim@xxxxxxxxxxx]
> > Sent: Thursday, March 14, 2013 11:30 AM
> > To: Inki Dae
> > Cc: airlied@xxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
> > kyungmin.park@xxxxxxxxxxx; sw0312.kim@xxxxxxxxxxx; '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:jy0922.shim@xxxxxxxxxxx]
> > >> Sent: Wednesday, March 13, 2013 7:28 PM
> > >> To: Inki Dae
> > >> Cc:airlied@xxxxxxxx;dri-devel@xxxxxxxxxxxxxxxxxxxxx;
> > >> kyungmin.park@xxxxxxxxxxx;sw0312.kim@xxxxxxxxxxx; '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:jy0922.shim@xxxxxxxxxxx]
> > >>>> Sent: Wednesday, March 13, 2013 6:53 PM
> > >>>> To: Inki Dae
> > >>>> Cc:airlied@xxxxxxxx;dri-devel@xxxxxxxxxxxxxxxxxxxxx;
> > >>>> kyungmin.park@xxxxxxxxxxx;sw0312.kim@xxxxxxxxxxx; 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@xxxxxxxxxxx>
> > >>>>>
> > >>>>> 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<yj44.cho@xxxxxxxxxxx>
> > >>>>> Signed-off-by: Inki Dae<inki.dae@xxxxxxxxxxx>
> > >>>>> Signed-off-by: Kyungmin Park<kyungmin.park@xxxxxxxxxxx>
> > >>>>> ---
> > >>>>> 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
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
