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.
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;
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.
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.
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;
-----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;
On 03/13/2013 07:14 PM, Inki Dae wrote:
-----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.
No, Once set GCF, it is set on end. Who can clear it?
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;
-----Original Message----- From: Joonyoung Shim [mailto:jy0922.shim@samsung.com] Sent: Wednesday, March 13, 2013 7:28 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 07:14 PM, Inki Dae wrote:
-----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.
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.
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.
Any other idea?
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;
On 03/13/2013 07:53 PM, Inki Dae wrote:
-----Original Message----- From: Joonyoung Shim [mailto:jy0922.shim@samsung.com] Sent: Wednesday, March 13, 2013 7:28 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 07:14 PM, Inki Dae wrote:
-----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 Choyj44.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.
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.
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.
Thanks.
Any other idea?
Thanks.
Signed-off-by: YoungJun Choyj44.cho@samsung.com Signed-off-by: Inki Daeinki.dae@samsung.com Signed-off-by: Kyungmin Parkkyungmin.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;
-----Original Message----- From: Joonyoung Shim [mailto:jy0922.shim@samsung.com] Sent: Thursday, March 14, 2013 11:30 AM 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 07:53 PM, Inki Dae wrote:
-----Original Message----- From: Joonyoung Shim [mailto:jy0922.shim@samsung.com] Sent: Wednesday, March 13, 2013 7:28 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 07:14 PM, Inki Dae wrote:
-----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 Choyj44.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.
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.
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 Choyj44.cho@samsung.com Signed-off-by: Inki Daeinki.dae@samsung.com Signed-off-by: Kyungmin Parkkyungmin.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 listor
* 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;
On Mar 14, 2013 1:59 PM, "Inki Dae" inki.dae@samsung.com wrote:
-----Original Message----- From: Joonyoung Shim [mailto:jy0922.shim@samsung.com] Sent: Thursday, March 14, 2013 11:30 AM 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 07:53 PM, Inki Dae wrote:
-----Original Message----- From: Joonyoung Shim [mailto:jy0922.shim@samsung.com] Sent: Wednesday, March 13, 2013 7:28 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 07:14 PM, Inki Dae wrote:
-----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 Choyj44.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.
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 Choyj44.cho@samsung.com > Signed-off-by: Inki Daeinki.dae@samsung.com > Signed-off-by: Kyungmin Parkkyungmin.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;
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
From: YoungJun Cho yj44.cho@samsung.com
This patch fixes G2D core malfunctioning 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.
Changelog v2: - Consider for interrupt setup to each command list and all command lists And correct typo.
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 | 20 +++++++++++++++----- 1 files changed, 15 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..1ff1144 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)
@@ -592,10 +592,6 @@ static void g2d_dma_start(struct g2d_data *g2d, pm_runtime_get_sync(g2d->dev); clk_enable(g2d->gate_clk);
- /* interrupt enable */ - writel_relaxed(G2D_INTEN_ACF | G2D_INTEN_UCF | G2D_INTEN_GCF, - g2d->regs + G2D_INTEN); - writel_relaxed(node->dma_addr, g2d->regs + G2D_DMA_SFR_BASE_ADDR); writel_relaxed(G2D_DMA_START, g2d->regs + G2D_DMA_COMMAND); } @@ -863,9 +859,23 @@ 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;
+ /* + * 'LIST_HOLD' command should be set to the DMA_HOLD_CMD_REG + * and GCF bit should be set to INTEN register if user wants + * G2D interrupt event once current command list execution is + * finished. + * Otherwise only ACF bit should be set to INTEN register so + * that one interrupt is occured after all command lists + * have been completed. + */ if (node->event) { + cmdlist->data[cmdlist->last++] = G2D_INTEN; + cmdlist->data[cmdlist->last++] = G2D_INTEN_ACF | G2D_INTEN_GCF; cmdlist->data[cmdlist->last++] = G2D_DMA_HOLD_CMD; cmdlist->data[cmdlist->last++] = G2D_LIST_HOLD; + } else { + cmdlist->data[cmdlist->last++] = G2D_INTEN; + cmdlist->data[cmdlist->last++] = G2D_INTEN_ACF; }
/* Check size of cmdlist: last 2 is about G2D_BITBLT_START */
dri-devel@lists.freedesktop.org
-
Inki Dae -
Joonyoung Shim -
YoungJun Cho