[PATCH 2/2] drm/amdkfd: Decode bit fields in 'cik_ih_ring_entry' directly

[StDenis, Tom]

Better would be to add them to a bitmask header and then use REG_GET_FIELD() so it's nice and clean looking.


Tom


________________________________
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of Christian König <deathsimple at vodafone.de>
Sent: Tuesday, September 27, 2016 08:52
To: Edward O'Callaghan; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdkfd: Decode bit fields in 'cik_ih_ring_entry' directly

NAK, don't use bitfields to decode hardware values. They aren't portable.

Regards,
Christian.

Am 27.09.2016 um 14:47 schrieb Edward O'Callaghan:
> Signed-off-by: Edward O'Callaghan <funfunctor at folklore1984.net>
> ---
>   drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c | 15 +++++----------
>   drivers/gpu/drm/amd/amdkfd/cik_int.h             | 20 ++++++++++++++++----
>   2 files changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c b/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c
> index 211fc48..1c47b9e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c
> +++ b/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c
> @@ -27,14 +27,12 @@
>   static bool cik_event_interrupt_isr(struct kfd_dev *dev,
>                                        const uint32_t *ih_ring_entry)
>   {
> -     unsigned int pasid;
>        const struct cik_ih_ring_entry *ihre =
>                        (const struct cik_ih_ring_entry *)ih_ring_entry;
>
> -     pasid = (ihre->ring_id & 0xffff0000) >> 16;
>
>        /* Do not process in ISR, just request it to be forwarded to WQ. */
> -     return (pasid != 0) &&
> +     return (ihre->pasid != 0) &&
>                (ihre->source_id == CIK_INTSRC_CP_END_OF_PIPE ||
>                ihre->source_id == CIK_INTSRC_SQ_INTERRUPT_MSG ||
>                ihre->source_id == CIK_INTSRC_CP_BAD_OPCODE);
> @@ -43,21 +41,18 @@ static bool cik_event_interrupt_isr(struct kfd_dev *dev,
>   static void cik_event_interrupt_wq(struct kfd_dev *dev,
>                                        const uint32_t *ih_ring_entry)
>   {
> -     unsigned int pasid;
>        const struct cik_ih_ring_entry *ihre =
>                        (const struct cik_ih_ring_entry *)ih_ring_entry;
>
> -     pasid = (ihre->ring_id & 0xffff0000) >> 16;
> -
> -     if (pasid == 0)
> +     if (ihre->pasid == 0)
>                return;
>
>        if (ihre->source_id == CIK_INTSRC_CP_END_OF_PIPE)
> -             kfd_signal_event_interrupt(pasid, 0, 0);
> +             kfd_signal_event_interrupt(ihre->pasid, 0, 0);
>        else if (ihre->source_id == CIK_INTSRC_SQ_INTERRUPT_MSG)
> -             kfd_signal_event_interrupt(pasid, ihre->data & 0xFF, 8);
> +             kfd_signal_event_interrupt(ihre->pasid, ihre->data & 0xFF, 8);
>        else if (ihre->source_id == CIK_INTSRC_CP_BAD_OPCODE)
> -             kfd_signal_hw_exception_event(pasid);
> +             kfd_signal_hw_exception_event(ihre->pasid);
>   }
>
>   const struct kfd_event_interrupt_class event_interrupt_class_cik = {
> diff --git a/drivers/gpu/drm/amd/amdkfd/cik_int.h b/drivers/gpu/drm/amd/amdkfd/cik_int.h
> index 79a16d2..27d1ede 100644
> --- a/drivers/gpu/drm/amd/amdkfd/cik_int.h
> +++ b/drivers/gpu/drm/amd/amdkfd/cik_int.h
> @@ -26,10 +26,22 @@
>   #include <linux/types.h>
>
>   struct cik_ih_ring_entry {
> -     uint32_t source_id;
> -     uint32_t data;
> -     uint32_t ring_id;
> -     uint32_t reserved;
> +     uint32_t source_id:8;
> +     uint32_t reserved1:8;
> +     uint32_t reserved2:16;
> +
> +     uint32_t data:28;
> +     uint32_t reserved3:4;
> +
> +     /* pipeid, meid and unused3 are officially called RINGID,
> +      * but for our purposes, they always decode into pipe and ME. */
> +     uint32_t pipeid:2;
> +     uint32_t meid:2;
> +     uint32_t reserved4:4;
> +     uint32_t vmid:8;
> +     uint32_t pasid:16;
> +
> +     uint32_t reserved5;
>   };
>
>   #define CIK_INTSRC_DEQUEUE_COMPLETE 0xC6


_______________________________________________
amd-gfx mailing list
amd-gfx at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - lists.freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
lists.freedesktop.org
To see the collection of prior postings to the list, visit the amd-gfx Archives. Using amd-gfx: To post a message to all the list members, send email ...



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20160927/82022896/attachment.html>