Am 2021-10-15 um 4:48 a.m. schrieb Lang Yu:
> +enum queue_update_flag {
> + UPDATE_FLAG_PROPERTITY = 0,
> + UPDATE_FLAG_CU_MASK,
> +};
> +
> +struct queue_update_info {
> + union {
> + struct queue_properties properties;
> + struct {
> + uint32_t count; /* Must be a multiple of 32 */
> + uint32_t *ptr;
> + } cu_mask;
> + };
> +
> + enum queue_update_flag update_flag;
> +};
> +
This doesn't make sense to me. As I understand it, queue_update_info is
for information that is not stored in queue_properties but only in the
MQDs. Therefore, it should not include the queue_properties.
All the low level functions in the MQD managers get both the
queue_properties and the queue_update_info. So trying to wrap both in
the same union doesn't make sense there either.
I think you only need this because you tried to generalize
pqm_update_queue to handle both updates to queue_properties and CU mask
updates with a single argument. IMO this does not make the interface any
clearer. I think it would be more straight-forward to keep a separate
pqm_set_cu_mask function that takes a queue_update_info parameter. If
you're looking for more generic names, I suggest the following:
* Rename pqm_update_queue to pqm_update_queue_properties
* Rename struct queue_update_info to struct mqd_update_info
* Rename pqm_set_cu_mask to pqm_update_mqd. For now this is only used
for CU mask (the union has only one struct member for now). It may
be used for other MQD properties that don't need to be stored in
queue_properties in the future
Regards,
Felix
