On 1/4/2023 8:05 PM, 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.
Thankyou for your comment. We will update as per your suggestions.
~arvind
Regards,
Felix
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
