[PATCH 1/5] drm/amdgpu: add system interrupt register offset header

[PATCH 1/5] drm/amdgpu: add system interrupt register offset header

[boyuan.zhang-5C7GfCeVMHo]

From: Boyuan Zhang <boyuan.zhang@amd.com>

Add new register offset for enabling system interrupt.

Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
---
 drivers/gpu/drm/amd/include/asic_reg/vcn/vcn_1_0_offset.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/include/asic_reg/vcn/vcn_1_0_offset.h b/drivers/gpu/drm/amd/include/asic_reg/vcn/vcn_1_0_offset.h
index fe0cbaa..216a401 100644
--- a/drivers/gpu/drm/amd/include/asic_reg/vcn/vcn_1_0_offset.h
+++ b/drivers/gpu/drm/amd/include/asic_reg/vcn/vcn_1_0_offset.h
@@ -307,6 +307,8 @@
 #define mmUVD_LMI_CTRL2_BASE_IDX                                                                       1
 #define mmUVD_MASTINT_EN                                                                               0x0540
 #define mmUVD_MASTINT_EN_BASE_IDX                                                                      1
+#define mmUVD_SYS_INT_EN                                                                               0x0541
+#define mmUVD_SYS_INT_EN_BASE_IDX                                                                      1
 #define mmJPEG_CGC_CTRL                                                                                0x0565
 #define mmJPEG_CGC_CTRL_BASE_IDX                                                                       1
 #define mmUVD_LMI_CTRL                                                                                 0x0566
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/5] drm/amdgpu: add system interrupt mask for jrbc

[boyuan.zhang-5C7GfCeVMHo]

From: Boyuan Zhang <boyuan.zhang@amd.com>

Add new mask for enabling system interrupt for jrbc.

Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
---
 drivers/gpu/drm/amd/include/asic_reg/vcn/vcn_1_0_sh_mask.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/include/asic_reg/vcn/vcn_1_0_sh_mask.h b/drivers/gpu/drm/amd/include/asic_reg/vcn/vcn_1_0_sh_mask.h
index d6ba269..124383d 100644
--- a/drivers/gpu/drm/amd/include/asic_reg/vcn/vcn_1_0_sh_mask.h
+++ b/drivers/gpu/drm/amd/include/asic_reg/vcn/vcn_1_0_sh_mask.h
@@ -982,6 +982,8 @@
 #define UVD_MASTINT_EN__VCPU_EN_MASK                                                                          0x00000002L
 #define UVD_MASTINT_EN__SYS_EN_MASK                                                                           0x00000004L
 #define UVD_MASTINT_EN__INT_OVERRUN_MASK                                                                      0x007FFFF0L
+//UVD_SYS_INT_EN
+#define UVD_SYS_INT_EN__UVD_JRBC_EN_MASK                                                                      0x00000010L
 //JPEG_CGC_CTRL
 #define JPEG_CGC_CTRL__DYN_CLOCK_MODE__SHIFT                                                                  0x0
 #define JPEG_CGC_CTRL__JPEG2_MODE__SHIFT                                                                      0x1
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/5] drm/amdgpu: enable system interrupt for jrbc

[boyuan.zhang-5C7GfCeVMHo]

From: Boyuan Zhang <boyuan.zhang@amd.com>

Enable system interrupt for jrbc during engine starting time.

Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index 4fccb21..22c1588 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -595,6 +595,7 @@ static int vcn_v1_0_start(struct amdgpu_device *adev)
 	struct amdgpu_ring *ring = &adev->vcn.ring_dec;
 	uint32_t rb_bufsz, tmp;
 	uint32_t lmi_swap_cntl;
+	uint32_t reg_temp;
 	int i, j, r;
 
 	/* disable byte swapping */
@@ -700,6 +701,11 @@ static int vcn_v1_0_start(struct amdgpu_device *adev)
 		(UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK),
 		~(UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK));
 
+	/* enable system interrupt for JRBC*/
+	reg_temp = RREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN));
+	reg_temp |= UVD_SYS_INT_EN__UVD_JRBC_EN_MASK;
+	WREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN), reg_temp);
+
 	/* clear the bit 4 of VCN_STATUS */
 	WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_STATUS), 0,
 			~(2 << UVD_STATUS__VCPU_REPORT__SHIFT));
@@ -1754,7 +1760,7 @@ static const struct amdgpu_irq_src_funcs vcn_v1_0_irq_funcs = {
 
 static void vcn_v1_0_set_irq_funcs(struct amdgpu_device *adev)
 {
-	adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 1;
+	adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 2;
 	adev->vcn.irq.funcs = &vcn_v1_0_irq_funcs;
 }
 
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/5] drm/amdgpu: add emit trap for vcn jpeg

[boyuan.zhang-5C7GfCeVMHo]

From: Boyuan Zhang <boyuan.zhang@amd.com>

Add emit trap command in jpeg emit fence call.

Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index 22c1588..745c9ae 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -1317,6 +1317,10 @@ static void vcn_v1_0_jpeg_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u6
 	amdgpu_ring_write(ring,
 		PACKETJ(0, 0, 0, PACKETJ_TYPE0));
 	amdgpu_ring_write(ring, 0x1);
+
+	/* emit trap */
+	amdgpu_ring_write(ring, PACKETJ(0, 0, 0, PACKETJ_TYPE7));
+	amdgpu_ring_write(ring, 0);
 }
 
 /**
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 5/5] drm/amdgpu: fix emit frame size and comments for jpeg

[boyuan.zhang-5C7GfCeVMHo]

From: Boyuan Zhang <boyuan.zhang@amd.com>

Fix vcn jpeg ring emit fence size in dword, and fix the naming in comments.

Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index 745c9ae..79fc76c 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -1715,10 +1715,10 @@ static const struct amdgpu_ring_funcs vcn_v1_0_jpeg_ring_vm_funcs = {
 		6 + 6 + /* hdp invalidate / flush */
 		SOC15_FLUSH_GPU_TLB_NUM_WREG * 6 +
 		SOC15_FLUSH_GPU_TLB_NUM_REG_WAIT * 8 +
-		8 + /* vcn_v1_0_dec_ring_emit_vm_flush */
-		14 + 14 + /* vcn_v1_0_dec_ring_emit_fence x2 vm fence */
+		8 + /* vcn_v1_0_jpeg_ring_emit_vm_flush */
+		26 + 26 + /* vcn_v1_0_jpeg_ring_emit_fence x2 vm fence */
 		6,
-	.emit_ib_size = 22, /* vcn_v1_0_dec_ring_emit_ib */
+	.emit_ib_size = 22, /* vcn_v1_0_jpeg_ring_emit_ib */
 	.emit_ib = vcn_v1_0_jpeg_ring_emit_ib,
 	.emit_fence = vcn_v1_0_jpeg_ring_emit_fence,
 	.emit_vm_flush = vcn_v1_0_jpeg_ring_emit_vm_flush,
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: enable system interrupt for jrbc

[Leo Liu]

On 07/18/2018 04:39 PM, boyuan.zhang@amd.com wrote:
> From: Boyuan Zhang <boyuan.zhang@amd.com>
>
> Enable system interrupt for jrbc during engine starting time.
>
> Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index 4fccb21..22c1588 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -595,6 +595,7 @@ static int vcn_v1_0_start(struct amdgpu_device *adev)
>   	struct amdgpu_ring *ring = &adev->vcn.ring_dec;
>   	uint32_t rb_bufsz, tmp;
>   	uint32_t lmi_swap_cntl;
> +	uint32_t reg_temp;
>   	int i, j, r;
>   
>   	/* disable byte swapping */
> @@ -700,6 +701,11 @@ static int vcn_v1_0_start(struct amdgpu_device *adev)
>   		(UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK),
>   		~(UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK));
>   
> +	/* enable system interrupt for JRBC*/
> +	reg_temp = RREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN));
> +	reg_temp |= UVD_SYS_INT_EN__UVD_JRBC_EN_MASK;
> +	WREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN), reg_temp);

Here you could use below instead.
WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN), UVD_SYS_INT_EN__UVD_JRBC_EN_MASK,
	~UVD_SYS_INT_EN__UVD_JRBC_EN_MASK);

Ether way, the whole series are
Acked-by: Leo Liu <leo.liu@amd.com>



> +
>   	/* clear the bit 4 of VCN_STATUS */
>   	WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_STATUS), 0,
>   			~(2 << UVD_STATUS__VCPU_REPORT__SHIFT));
> @@ -1754,7 +1760,7 @@ static const struct amdgpu_irq_src_funcs vcn_v1_0_irq_funcs = {
>   
>   static void vcn_v1_0_set_irq_funcs(struct amdgpu_device *adev)
>   {
> -	adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 1;
> +	adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 2;
>   	adev->vcn.irq.funcs = &vcn_v1_0_irq_funcs;
>   }
>   

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: enable system interrupt for jrbc

[Zhang, Boyuan]

[-- Attachment #1.1: Type: text/plain, Size: 2559 bytes --]

Yes, agree! It's better to use that existing function. Will change it accordingly.


Thanks,

Boyuan

________________________________
From: Liu, Leo
Sent: July 19, 2018 2:13:50 PM
To: Zhang, Boyuan; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 3/5] drm/amdgpu: enable system interrupt for jrbc



On 07/18/2018 04:39 PM, boyuan.zhang-5C7GfCeVMHo@public.gmane.org wrote:
> From: Boyuan Zhang <boyuan.zhang-5C7GfCeVMHo@public.gmane.org>
>
> Enable system interrupt for jrbc during engine starting time.
>
> Signed-off-by: Boyuan Zhang <boyuan.zhang-5C7GfCeVMHo@public.gmane.org>
> ---
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index 4fccb21..22c1588 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -595,6 +595,7 @@ static int vcn_v1_0_start(struct amdgpu_device *adev)
>        struct amdgpu_ring *ring = &adev->vcn.ring_dec;
>        uint32_t rb_bufsz, tmp;
>        uint32_t lmi_swap_cntl;
> +     uint32_t reg_temp;
>        int i, j, r;
>
>        /* disable byte swapping */
> @@ -700,6 +701,11 @@ static int vcn_v1_0_start(struct amdgpu_device *adev)
>                (UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK),
>                ~(UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK));
>
> +     /* enable system interrupt for JRBC*/
> +     reg_temp = RREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN));
> +     reg_temp |= UVD_SYS_INT_EN__UVD_JRBC_EN_MASK;
> +     WREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN), reg_temp);

Here you could use below instead.
WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN), UVD_SYS_INT_EN__UVD_JRBC_EN_MASK,
        ~UVD_SYS_INT_EN__UVD_JRBC_EN_MASK);

Ether way, the whole series are
Acked-by: Leo Liu <leo.liu-5C7GfCeVMHo@public.gmane.org>



> +
>        /* clear the bit 4 of VCN_STATUS */
>        WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_STATUS), 0,
>                        ~(2 << UVD_STATUS__VCPU_REPORT__SHIFT));
> @@ -1754,7 +1760,7 @@ static const struct amdgpu_irq_src_funcs vcn_v1_0_irq_funcs = {
>
>   static void vcn_v1_0_set_irq_funcs(struct amdgpu_device *adev)
>   {
> -     adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 1;
> +     adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 2;
>        adev->vcn.irq.funcs = &vcn_v1_0_irq_funcs;
>   }
>


[-- Attachment #1.2: Type: text/html, Size: 4738 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: enable system interrupt for jrbc

[Alex Deucher]

On Wed, Jul 18, 2018 at 4:39 PM,  <boyuan.zhang@amd.com> wrote:
> From: Boyuan Zhang <boyuan.zhang@amd.com>
>
> Enable system interrupt for jrbc during engine starting time.
>
> Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index 4fccb21..22c1588 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -595,6 +595,7 @@ static int vcn_v1_0_start(struct amdgpu_device *adev)
>         struct amdgpu_ring *ring = &adev->vcn.ring_dec;
>         uint32_t rb_bufsz, tmp;
>         uint32_t lmi_swap_cntl;
> +       uint32_t reg_temp;
>         int i, j, r;
>
>         /* disable byte swapping */
> @@ -700,6 +701,11 @@ static int vcn_v1_0_start(struct amdgpu_device *adev)
>                 (UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK),
>                 ~(UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK));
>
> +       /* enable system interrupt for JRBC*/
> +       reg_temp = RREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN));
> +       reg_temp |= UVD_SYS_INT_EN__UVD_JRBC_EN_MASK;
> +       WREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN), reg_temp);
> +

Shouldn't we move the setting of these interrupts into
vcn_v1_0_set_interrupt_state()? Same for the mastint.  that way they
will get enabled/disabled as part of the fence driver sequence I
think.  Or do they need to happen in a specific sequence?

Alex

>         /* clear the bit 4 of VCN_STATUS */
>         WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_STATUS), 0,
>                         ~(2 << UVD_STATUS__VCPU_REPORT__SHIFT));
> @@ -1754,7 +1760,7 @@ static const struct amdgpu_irq_src_funcs vcn_v1_0_irq_funcs = {
>
>  static void vcn_v1_0_set_irq_funcs(struct amdgpu_device *adev)
>  {
> -       adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 1;
> +       adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 2;
>         adev->vcn.irq.funcs = &vcn_v1_0_irq_funcs;
>  }
>
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: enable system interrupt for jrbc

[Boyuan Zhang]

On 2018-07-19 02:51 PM, Alex Deucher wrote:
> On Wed, Jul 18, 2018 at 4:39 PM,  <boyuan.zhang@amd.com> wrote:
>> From: Boyuan Zhang <boyuan.zhang@amd.com>
>>
>> Enable system interrupt for jrbc during engine starting time.
>>
>> Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> index 4fccb21..22c1588 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> @@ -595,6 +595,7 @@ static int vcn_v1_0_start(struct amdgpu_device *adev)
>>          struct amdgpu_ring *ring = &adev->vcn.ring_dec;
>>          uint32_t rb_bufsz, tmp;
>>          uint32_t lmi_swap_cntl;
>> +       uint32_t reg_temp;
>>          int i, j, r;
>>
>>          /* disable byte swapping */
>> @@ -700,6 +701,11 @@ static int vcn_v1_0_start(struct amdgpu_device *adev)
>>                  (UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK),
>>                  ~(UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK));
>>
>> +       /* enable system interrupt for JRBC*/
>> +       reg_temp = RREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN));
>> +       reg_temp |= UVD_SYS_INT_EN__UVD_JRBC_EN_MASK;
>> +       WREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN), reg_temp);
>> +
> Shouldn't we move the setting of these interrupts into
> vcn_v1_0_set_interrupt_state()? Same for the mastint.  that way they
> will get enabled/disabled as part of the fence driver sequence I
> think.  Or do they need to happen in a specific sequence?
>
> Alex

Hmm... at least for this JPEG specific case, interrupt won't be raised 
during those times that we don't care about the interrupt. This is not 
like other system component where interrupt might still be raised even 
if we don't care about it. So my feeling is that whether we disable it 
at the beginning and enable it later on, or we just enable it at the 
beginning doesn't really matter in the practical sense.

Regards,
Boyuan

>
>>          /* clear the bit 4 of VCN_STATUS */
>>          WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_STATUS), 0,
>>                          ~(2 << UVD_STATUS__VCPU_REPORT__SHIFT));
>> @@ -1754,7 +1760,7 @@ static const struct amdgpu_irq_src_funcs vcn_v1_0_irq_funcs = {
>>
>>   static void vcn_v1_0_set_irq_funcs(struct amdgpu_device *adev)
>>   {
>> -       adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 1;
>> +       adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 2;
>>          adev->vcn.irq.funcs = &vcn_v1_0_irq_funcs;
>>   }
>>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: enable system interrupt for jrbc

[Christian König]

Am 23.07.2018 um 21:53 schrieb Boyuan Zhang:
>
>
> On 2018-07-19 02:51 PM, Alex Deucher wrote:
>> On Wed, Jul 18, 2018 at 4:39 PM, <boyuan.zhang@amd.com> wrote:
>>> From: Boyuan Zhang <boyuan.zhang@amd.com>
>>>
>>> Enable system interrupt for jrbc during engine starting time.
>>>
>>> Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 8 +++++++-
>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c 
>>> b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>> index 4fccb21..22c1588 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>> @@ -595,6 +595,7 @@ static int vcn_v1_0_start(struct amdgpu_device 
>>> *adev)
>>>          struct amdgpu_ring *ring = &adev->vcn.ring_dec;
>>>          uint32_t rb_bufsz, tmp;
>>>          uint32_t lmi_swap_cntl;
>>> +       uint32_t reg_temp;
>>>          int i, j, r;
>>>
>>>          /* disable byte swapping */
>>> @@ -700,6 +701,11 @@ static int vcn_v1_0_start(struct amdgpu_device 
>>> *adev)
>>> (UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK),
>>> ~(UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK));
>>>
>>> +       /* enable system interrupt for JRBC*/
>>> +       reg_temp = RREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN));
>>> +       reg_temp |= UVD_SYS_INT_EN__UVD_JRBC_EN_MASK;
>>> +       WREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN), reg_temp);
>>> +
>> Shouldn't we move the setting of these interrupts into
>> vcn_v1_0_set_interrupt_state()? Same for the mastint.  that way they
>> will get enabled/disabled as part of the fence driver sequence I
>> think.  Or do they need to happen in a specific sequence?
>>
>> Alex
>
> Hmm... at least for this JPEG specific case, interrupt won't be raised 
> during those times that we don't care about the interrupt. This is not 
> like other system component where interrupt might still be raised even 
> if we don't care about it. So my feeling is that whether we disable it 
> at the beginning and enable it later on, or we just enable it at the 
> beginning doesn't really matter in the practical sense.

I agree with Alex here. While we currently don't use that much we would 
still like to be able to control interrupts and not just silently enable 
them all the time.

Regards,
Christian.

>
> Regards,
> Boyuan
>
>>
>>>          /* clear the bit 4 of VCN_STATUS */
>>>          WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_STATUS), 0,
>>>                          ~(2 << UVD_STATUS__VCPU_REPORT__SHIFT));
>>> @@ -1754,7 +1760,7 @@ static const struct amdgpu_irq_src_funcs 
>>> vcn_v1_0_irq_funcs = {
>>>
>>>   static void vcn_v1_0_set_irq_funcs(struct amdgpu_device *adev)
>>>   {
>>> -       adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 1;
>>> +       adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 2;
>>>          adev->vcn.irq.funcs = &vcn_v1_0_irq_funcs;
>>>   }
>>>
>>> -- 
>>> 2.7.4
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: enable system interrupt for jrbc

[Boyuan Zhang]

On 2018-07-24 08:50 AM, Christian König wrote:
> Am 23.07.2018 um 21:53 schrieb Boyuan Zhang:
>>
>>
>> On 2018-07-19 02:51 PM, Alex Deucher wrote:
>>> On Wed, Jul 18, 2018 at 4:39 PM, <boyuan.zhang@amd.com> wrote:
>>>> From: Boyuan Zhang <boyuan.zhang@amd.com>
>>>>
>>>> Enable system interrupt for jrbc during engine starting time.
>>>>
>>>> Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 8 +++++++-
>>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>> index 4fccb21..22c1588 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>> @@ -595,6 +595,7 @@ static int vcn_v1_0_start(struct amdgpu_device 
>>>> *adev)
>>>>          struct amdgpu_ring *ring = &adev->vcn.ring_dec;
>>>>          uint32_t rb_bufsz, tmp;
>>>>          uint32_t lmi_swap_cntl;
>>>> +       uint32_t reg_temp;
>>>>          int i, j, r;
>>>>
>>>>          /* disable byte swapping */
>>>> @@ -700,6 +701,11 @@ static int vcn_v1_0_start(struct amdgpu_device 
>>>> *adev)
>>>> (UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK),
>>>> ~(UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK));
>>>>
>>>> +       /* enable system interrupt for JRBC*/
>>>> +       reg_temp = RREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN));
>>>> +       reg_temp |= UVD_SYS_INT_EN__UVD_JRBC_EN_MASK;
>>>> +       WREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN), reg_temp);
>>>> +
>>> Shouldn't we move the setting of these interrupts into
>>> vcn_v1_0_set_interrupt_state()? Same for the mastint.  that way they
>>> will get enabled/disabled as part of the fence driver sequence I
>>> think.  Or do they need to happen in a specific sequence?
>>>
>>> Alex
>>
>> Hmm... at least for this JPEG specific case, interrupt won't be 
>> raised during those times that we don't care about the interrupt. 
>> This is not like other system component where interrupt might still 
>> be raised even if we don't care about it. So my feeling is that 
>> whether we disable it at the beginning and enable it later on, or we 
>> just enable it at the beginning doesn't really matter in the 
>> practical sense.
>
> I agree with Alex here. While we currently don't use that much we 
> would still like to be able to control interrupts and not just 
> silently enable them all the time.
>
> Regards,
> Christian.

Yes, I agree with you and Alex in terms of controlling interrupts. I did 
a quick try from my side yesterday, it seems that 
"vcn_v1_0_set_interrupt_state()" is not being called in fence driver 
sequence, and not only for jpeg but for vcn in general. I saw that the 
current "vcn_v1_0_set_interrupt_state()" just returns 0, is there any 
reason that we didn't use this function before? Seems like this is on 
purpose to me. Any information regarding to this?

Regards,
Boyuan

>
>>
>> Regards,
>> Boyuan
>>
>>>
>>>>          /* clear the bit 4 of VCN_STATUS */
>>>>          WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_STATUS), 0,
>>>>                          ~(2 << UVD_STATUS__VCPU_REPORT__SHIFT));
>>>> @@ -1754,7 +1760,7 @@ static const struct amdgpu_irq_src_funcs 
>>>> vcn_v1_0_irq_funcs = {
>>>>
>>>>   static void vcn_v1_0_set_irq_funcs(struct amdgpu_device *adev)
>>>>   {
>>>> -       adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 1;
>>>> +       adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 2;
>>>>          adev->vcn.irq.funcs = &vcn_v1_0_irq_funcs;
>>>>   }
>>>>
>>>> -- 
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: enable system interrupt for jrbc

[Alex Deucher]

On Wed, Jul 25, 2018 at 10:01 AM, Boyuan Zhang <boyzhang@amd.com> wrote:
>
>
> On 2018-07-24 08:50 AM, Christian König wrote:
>>
>> Am 23.07.2018 um 21:53 schrieb Boyuan Zhang:
>>>
>>>
>>>
>>> On 2018-07-19 02:51 PM, Alex Deucher wrote:
>>>>
>>>> On Wed, Jul 18, 2018 at 4:39 PM, <boyuan.zhang@amd.com> wrote:
>>>>>
>>>>> From: Boyuan Zhang <boyuan.zhang@amd.com>
>>>>>
>>>>> Enable system interrupt for jrbc during engine starting time.
>>>>>
>>>>> Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 8 +++++++-
>>>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>>> index 4fccb21..22c1588 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>>> @@ -595,6 +595,7 @@ static int vcn_v1_0_start(struct amdgpu_device
>>>>> *adev)
>>>>>          struct amdgpu_ring *ring = &adev->vcn.ring_dec;
>>>>>          uint32_t rb_bufsz, tmp;
>>>>>          uint32_t lmi_swap_cntl;
>>>>> +       uint32_t reg_temp;
>>>>>          int i, j, r;
>>>>>
>>>>>          /* disable byte swapping */
>>>>> @@ -700,6 +701,11 @@ static int vcn_v1_0_start(struct amdgpu_device
>>>>> *adev)
>>>>> (UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK),
>>>>> ~(UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK));
>>>>>
>>>>> +       /* enable system interrupt for JRBC*/
>>>>> +       reg_temp = RREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN));
>>>>> +       reg_temp |= UVD_SYS_INT_EN__UVD_JRBC_EN_MASK;
>>>>> +       WREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN), reg_temp);
>>>>> +
>>>>
>>>> Shouldn't we move the setting of these interrupts into
>>>> vcn_v1_0_set_interrupt_state()? Same for the mastint.  that way they
>>>> will get enabled/disabled as part of the fence driver sequence I
>>>> think.  Or do they need to happen in a specific sequence?
>>>>
>>>> Alex
>>>
>>>
>>> Hmm... at least for this JPEG specific case, interrupt won't be raised
>>> during those times that we don't care about the interrupt. This is not like
>>> other system component where interrupt might still be raised even if we
>>> don't care about it. So my feeling is that whether we disable it at the
>>> beginning and enable it later on, or we just enable it at the beginning
>>> doesn't really matter in the practical sense.
>>
>>
>> I agree with Alex here. While we currently don't use that much we would
>> still like to be able to control interrupts and not just silently enable
>> them all the time.
>>
>> Regards,
>> Christian.
>
>
> Yes, I agree with you and Alex in terms of controlling interrupts. I did a
> quick try from my side yesterday, it seems that
> "vcn_v1_0_set_interrupt_state()" is not being called in fence driver
> sequence, and not only for jpeg but for vcn in general. I saw that the
> current "vcn_v1_0_set_interrupt_state()" just returns 0, is there any reason
> that we didn't use this function before? Seems like this is on purpose to
> me. Any information regarding to this?

IIRC, on older versions of UVD there was no control bit for the UVD
fence interrupt so it was always enabled.   Also, you may want to add
an irq type so that you can pass that to amdgpu_ring_init() so we can
enable/disable each irq source independently.

Alex

>
> Regards,
> Boyuan
>
>
>>
>>>
>>> Regards,
>>> Boyuan
>>>
>>>>
>>>>>          /* clear the bit 4 of VCN_STATUS */
>>>>>          WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_STATUS), 0,
>>>>>                          ~(2 << UVD_STATUS__VCPU_REPORT__SHIFT));
>>>>> @@ -1754,7 +1760,7 @@ static const struct amdgpu_irq_src_funcs
>>>>> vcn_v1_0_irq_funcs = {
>>>>>
>>>>>   static void vcn_v1_0_set_irq_funcs(struct amdgpu_device *adev)
>>>>>   {
>>>>> -       adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 1;
>>>>> +       adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 2;
>>>>>          adev->vcn.irq.funcs = &vcn_v1_0_irq_funcs;
>>>>>   }
>>>>>
>>>>> --
>>>>> 2.7.4
>>>>>
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: enable system interrupt for jrbc

[Boyuan Zhang]

On 2018-07-25 11:05 AM, Alex Deucher wrote:
> On Wed, Jul 25, 2018 at 10:01 AM, Boyuan Zhang <boyzhang@amd.com> wrote:
>>
>> On 2018-07-24 08:50 AM, Christian König wrote:
>>> Am 23.07.2018 um 21:53 schrieb Boyuan Zhang:
>>>>
>>>>
>>>> On 2018-07-19 02:51 PM, Alex Deucher wrote:
>>>>> On Wed, Jul 18, 2018 at 4:39 PM, <boyuan.zhang@amd.com> wrote:
>>>>>> From: Boyuan Zhang <boyuan.zhang@amd.com>
>>>>>>
>>>>>> Enable system interrupt for jrbc during engine starting time.
>>>>>>
>>>>>> Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
>>>>>> ---
>>>>>>    drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 8 +++++++-
>>>>>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>>>> index 4fccb21..22c1588 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>>>> @@ -595,6 +595,7 @@ static int vcn_v1_0_start(struct amdgpu_device
>>>>>> *adev)
>>>>>>           struct amdgpu_ring *ring = &adev->vcn.ring_dec;
>>>>>>           uint32_t rb_bufsz, tmp;
>>>>>>           uint32_t lmi_swap_cntl;
>>>>>> +       uint32_t reg_temp;
>>>>>>           int i, j, r;
>>>>>>
>>>>>>           /* disable byte swapping */
>>>>>> @@ -700,6 +701,11 @@ static int vcn_v1_0_start(struct amdgpu_device
>>>>>> *adev)
>>>>>> (UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK),
>>>>>> ~(UVD_MASTINT_EN__VCPU_EN_MASK|UVD_MASTINT_EN__SYS_EN_MASK));
>>>>>>
>>>>>> +       /* enable system interrupt for JRBC*/
>>>>>> +       reg_temp = RREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN));
>>>>>> +       reg_temp |= UVD_SYS_INT_EN__UVD_JRBC_EN_MASK;
>>>>>> +       WREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SYS_INT_EN), reg_temp);
>>>>>> +
>>>>> Shouldn't we move the setting of these interrupts into
>>>>> vcn_v1_0_set_interrupt_state()? Same for the mastint.  that way they
>>>>> will get enabled/disabled as part of the fence driver sequence I
>>>>> think.  Or do they need to happen in a specific sequence?
>>>>>
>>>>> Alex
>>>>
>>>> Hmm... at least for this JPEG specific case, interrupt won't be raised
>>>> during those times that we don't care about the interrupt. This is not like
>>>> other system component where interrupt might still be raised even if we
>>>> don't care about it. So my feeling is that whether we disable it at the
>>>> beginning and enable it later on, or we just enable it at the beginning
>>>> doesn't really matter in the practical sense.
>>>
>>> I agree with Alex here. While we currently don't use that much we would
>>> still like to be able to control interrupts and not just silently enable
>>> them all the time.
>>>
>>> Regards,
>>> Christian.
>>
>> Yes, I agree with you and Alex in terms of controlling interrupts. I did a
>> quick try from my side yesterday, it seems that
>> "vcn_v1_0_set_interrupt_state()" is not being called in fence driver
>> sequence, and not only for jpeg but for vcn in general. I saw that the
>> current "vcn_v1_0_set_interrupt_state()" just returns 0, is there any reason
>> that we didn't use this function before? Seems like this is on purpose to
>> me. Any information regarding to this?
> IIRC, on older versions of UVD there was no control bit for the UVD
> fence interrupt so it was always enabled.   Also, you may want to add
> an irq type so that you can pass that to amdgpu_ring_init() so we can
> enable/disable each irq source independently.
>
> Alex

As discussed, the codes will be leave as it for now. Will add TODO 
comment for future cleanup, and do more test on iqr_put/iqr_get to see 
the possibility of moving both master interrupt and system interrupt to 
set_interrupt call.

Thanks,
Boyuan

>
>> Regards,
>> Boyuan
>>
>>
>>>> Regards,
>>>> Boyuan
>>>>
>>>>>>           /* clear the bit 4 of VCN_STATUS */
>>>>>>           WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_STATUS), 0,
>>>>>>                           ~(2 << UVD_STATUS__VCPU_REPORT__SHIFT));
>>>>>> @@ -1754,7 +1760,7 @@ static const struct amdgpu_irq_src_funcs
>>>>>> vcn_v1_0_irq_funcs = {
>>>>>>
>>>>>>    static void vcn_v1_0_set_irq_funcs(struct amdgpu_device *adev)
>>>>>>    {
>>>>>> -       adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 1;
>>>>>> +       adev->vcn.irq.num_types = adev->vcn.num_enc_rings + 2;
>>>>>>           adev->vcn.irq.funcs = &vcn_v1_0_irq_funcs;
>>>>>>    }
>>>>>>
>>>>>> --
>>>>>> 2.7.4
>>>>>>
>>>>>> _______________________________________________
>>>>>> amd-gfx mailing list
>>>>>> amd-gfx@lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx