Re: [PATCH 061/207] drm/amdgpu/mes10.1: implement the ring functions of mes specific

[Alex Deucher]

On Tue, Jun 2, 2020 at 6:00 AM Christian König
 wrote:
>
> Am 01.06.20 um 20:00 schrieb Alex Deucher:
> > From: Jack Xiao 
> >
> > Implement mes ring functions and set up them.
> >
> > Signed-off-by: Jack Xiao 
> > Acked-by: Alex Deucher 
> > Reviewed-by: Hawking Zhang 
> > Signed-off-by: Alex Deucher 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/mes_v10_1.c | 43 ++
> >   1 file changed, 43 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c 
> > b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
> > index 4f7e345673ca..80f6812d8ecf 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
> > @@ -33,6 +33,47 @@ MODULE_FIRMWARE("amdgpu/navi10_mes.bin");
> >
> >   #define MES_EOP_SIZE   2048
> >
> > +static void mes_v10_1_ring_set_wptr(struct amdgpu_ring *ring)
> > +{
> > + struct amdgpu_device *adev = ring->adev;
> > +
> > + if (ring->use_doorbell) {
> > + atomic64_set((atomic64_t*)&adev->wb.wb[ring->wptr_offs],
> > +  ring->wptr);
>
> This atomic64_t type case still looks fishy to me. IIRC we agreed to not
> use them, don't we?

We use them for all the other ring code.  If we change it, we should
probably change it everywhere.  I don't think we ever agreed on a
replacement.

>
> > + WDOORBELL64(ring->doorbell_index, ring->wptr);
> > + } else {
> > + BUG();
>
> Do we really need the BUG() here and below?
>

We shouldn't ever actually hit these cases since the rings don't work
without doorbells.  Maybe it would be better to add a WARN_ON() if
someone tries to set ring->use_doorbell to false for MES.

Alex

> Christian.
>
> > + }
> > +}
> > +
> > +static u64 mes_v10_1_ring_get_rptr(struct amdgpu_ring *ring)
> > +{
> > + return ring->adev->wb.wb[ring->rptr_offs];
> > +}
> > +
> > +static u64 mes_v10_1_ring_get_wptr(struct amdgpu_ring *ring)
> > +{
> > + u64 wptr;
> > +
> > + if (ring->use_doorbell)
> > + wptr = atomic64_read((atomic64_t *)
> > +  &ring->adev->wb.wb[ring->wptr_offs]);
> > + else
> > + BUG();
> > + return wptr;
> > +}
> > +
> > +static const struct amdgpu_ring_funcs mes_v10_1_ring_funcs = {
> > + .type = AMDGPU_RING_TYPE_MES,
> > + .align_mask = 1,
> > + .nop = 0,
> > + .support_64bit_ptrs = true,
> > + .get_rptr = mes_v10_1_ring_get_rptr,
> > + .get_wptr = mes_v10_1_ring_get_wptr,
> > + .set_wptr = mes_v10_1_ring_set_wptr,
> > + .insert_nop = amdgpu_ring_insert_nop,
> > +};
> > +
> >   static int mes_v10_1_add_hw_queue(struct amdgpu_mes *mes,
> > struct mes_add_queue_input *input)
> >   {
> > @@ -315,6 +356,8 @@ static int mes_v10_1_ring_init(struct amdgpu_device 
> > *adev)
> >
> >   ring = &adev->mes.ring;
> >
> > + ring->funcs = &mes_v10_1_ring_funcs;
> > +
> >   ring->me = 3;
> >   ring->pipe = 0;
> >   ring->queue = 0;
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 061/207] drm/amdgpu/mes10.1: implement the ring functions of mes specific

[Christian König]

Am 01.06.20 um 20:00 schrieb Alex Deucher:

From: Jack Xiao 

Implement mes ring functions and set up them.

Signed-off-by: Jack Xiao 
Acked-by: Alex Deucher 
Reviewed-by: Hawking Zhang 
Signed-off-by: Alex Deucher 
---
  drivers/gpu/drm/amd/amdgpu/mes_v10_1.c | 43 ++
  1 file changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c 
b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
index 4f7e345673ca..80f6812d8ecf 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
@@ -33,6 +33,47 @@ MODULE_FIRMWARE("amdgpu/navi10_mes.bin");
  
  #define MES_EOP_SIZE   2048
  
+static void mes_v10_1_ring_set_wptr(struct amdgpu_ring *ring)

+{
+   struct amdgpu_device *adev = ring->adev;
+
+   if (ring->use_doorbell) {
+   atomic64_set((atomic64_t*)&adev->wb.wb[ring->wptr_offs],
+ring->wptr);


This atomic64_t type case still looks fishy to me. IIRC we agreed to not 
use them, don't we?



+   WDOORBELL64(ring->doorbell_index, ring->wptr);
+   } else {
+   BUG();


Do we really need the BUG() here and below?

Christian.


+   }
+}
+
+static u64 mes_v10_1_ring_get_rptr(struct amdgpu_ring *ring)
+{
+   return ring->adev->wb.wb[ring->rptr_offs];
+}
+
+static u64 mes_v10_1_ring_get_wptr(struct amdgpu_ring *ring)
+{
+   u64 wptr;
+
+   if (ring->use_doorbell)
+   wptr = atomic64_read((atomic64_t *)
+&ring->adev->wb.wb[ring->wptr_offs]);
+   else
+   BUG();
+   return wptr;
+}
+
+static const struct amdgpu_ring_funcs mes_v10_1_ring_funcs = {
+   .type = AMDGPU_RING_TYPE_MES,
+   .align_mask = 1,
+   .nop = 0,
+   .support_64bit_ptrs = true,
+   .get_rptr = mes_v10_1_ring_get_rptr,
+   .get_wptr = mes_v10_1_ring_get_wptr,
+   .set_wptr = mes_v10_1_ring_set_wptr,
+   .insert_nop = amdgpu_ring_insert_nop,
+};
+
  static int mes_v10_1_add_hw_queue(struct amdgpu_mes *mes,
  struct mes_add_queue_input *input)
  {
@@ -315,6 +356,8 @@ static int mes_v10_1_ring_init(struct amdgpu_device *adev)
  
  	ring = &adev->mes.ring;
  
+	ring->funcs = &mes_v10_1_ring_funcs;

+
ring->me = 3;
ring->pipe = 0;
ring->queue = 0;


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

[PATCH 061/207] drm/amdgpu/mes10.1: implement the ring functions of mes specific

[Alex Deucher]

From: Jack Xiao 

Implement mes ring functions and set up them.

Signed-off-by: Jack Xiao 
Acked-by: Alex Deucher 
Reviewed-by: Hawking Zhang 
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/mes_v10_1.c | 43 ++
 1 file changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c 
b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
index 4f7e345673ca..80f6812d8ecf 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
@@ -33,6 +33,47 @@ MODULE_FIRMWARE("amdgpu/navi10_mes.bin");
 
 #define MES_EOP_SIZE   2048
 
+static void mes_v10_1_ring_set_wptr(struct amdgpu_ring *ring)
+{
+   struct amdgpu_device *adev = ring->adev;
+
+   if (ring->use_doorbell) {
+   atomic64_set((atomic64_t*)&adev->wb.wb[ring->wptr_offs],
+ring->wptr);
+   WDOORBELL64(ring->doorbell_index, ring->wptr);
+   } else {
+   BUG();
+   }
+}
+
+static u64 mes_v10_1_ring_get_rptr(struct amdgpu_ring *ring)
+{
+   return ring->adev->wb.wb[ring->rptr_offs];
+}
+
+static u64 mes_v10_1_ring_get_wptr(struct amdgpu_ring *ring)
+{
+   u64 wptr;
+
+   if (ring->use_doorbell)
+   wptr = atomic64_read((atomic64_t *)
+&ring->adev->wb.wb[ring->wptr_offs]);
+   else
+   BUG();
+   return wptr;
+}
+
+static const struct amdgpu_ring_funcs mes_v10_1_ring_funcs = {
+   .type = AMDGPU_RING_TYPE_MES,
+   .align_mask = 1,
+   .nop = 0,
+   .support_64bit_ptrs = true,
+   .get_rptr = mes_v10_1_ring_get_rptr,
+   .get_wptr = mes_v10_1_ring_get_wptr,
+   .set_wptr = mes_v10_1_ring_set_wptr,
+   .insert_nop = amdgpu_ring_insert_nop,
+};
+
 static int mes_v10_1_add_hw_queue(struct amdgpu_mes *mes,
  struct mes_add_queue_input *input)
 {
@@ -315,6 +356,8 @@ static int mes_v10_1_ring_init(struct amdgpu_device *adev)
 
ring = &adev->mes.ring;
 
+   ring->funcs = &mes_v10_1_ring_funcs;
+
ring->me = 3;
ring->pipe = 0;
ring->queue = 0;
-- 
2.25.4

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