Re: [RFC 3/7] drm/amdgpu: Create MQD for userspace queue

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 04/01/2023 15:35, Felix Kuehling wrote:
Am 2023-01-04 um 04:23 schrieb Shashank Sharma:

On 04/01/2023 10:17, Christian König wrote:
Am 04.01.23 um 10:13 schrieb Shashank Sharma:

On 04/01/2023 10:10, Christian König wrote:
Am 04.01.23 um 07:21 schrieb Yadav, Arvind:

On 1/4/2023 12:07 AM, Felix Kuehling wrote:
Am 2023-01-03 um 04:36 schrieb Shashank Sharma:
/*MQD struct for usermode Queue*/
+struct amdgpu_usermode_queue_mqd
This is specific to GC 11.  Every IP and version will have its own MQD format.  That should live in the IP specific code, not the generic code.  We already have the generic MQD parameters that we need from
the userq IOCTL.

Noted, we can separate out the generic parameters from gen specific parameter, and will try to wrap it around the generic structure.

- Shashank

Is there a reason why you can't use "struct v11_compute_mqd" from v11_structs.h?

Hi Felix,

Yes,  V11_compute_mqd does not have these below member which is needed for usermode queue.

    uint32_t shadow_base_lo; // offset: 0  (0x0)
    uint32_t shadow_base_hi; // offset: 1  (0x1)
    uint32_t gds_bkup_base_lo ; // offset: 2  (0x2)
    uint32_t gds_bkup_base_hi ; // offset: 3  (0x3)
    uint32_t fw_work_area_base_lo; // offset: 4  (0x4)
    uint32_t fw_work_area_base_hi; // offset: 5  (0x5)
    uint32_t shadow_initialized; // offset: 6  (0x6)
    uint32_t ib_vmid; // offset: 7  (0x7)

So we had to add new MQD structs.

Would it make more sense to update the existing MQD structures than adding new ones?

Imo, It might be a bit complicated in the bring-up state, but we can take a note of converting this structure into a union of two, or may be renaming it into a superset structure.

Union? Does that mean we have stuff which is individual for both versions of the struct?
So far it seems like Gfx MQD structure is a superset of two, but we have not compared them neck-to-neck yet, hence I feel like we can defer this task for sometime (but add into to-do list).

v11_gfx_mqd has these fields reserved. Updating the definition with the fields you need should not be a problem. v11_gfx_mqd is already used in gfx_v11_0.c.

The firmware shouldn't care much whether a queue is a kernel mode queue or a user mode queue. The MQD layout should be the same. So having two different structure definitions in two different places doesn't make sense. I don't think it's wise to leave this for cleanup later. That would only cause churn and ultimately more work than doing the right thing in the first place.

Regards,
  Felix

Hey Felix, noted, we will try to reuse this same structure.

- Shashank




BTW: Could we drop the "// offset:" stuff? This could cause problems with automated checkers.

Sure, we will do it.

- Shashank


Christian.


- Shashank

Regards,
Christian.


thanks

~arvind


Regards,
  Felix






[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux