[PATCH v2] drm/amdkfd: Preserve cp_hqd_pq_control on update_mqd

Jay Cornwall jay.cornwall at amd.com
Wed Feb 19 16:46:47 UTC 2025


On 2/18/2025 17:13, David Yat Sin wrote:

> @@ -107,6 +107,7 @@ static void init_mqd(struct mqd_manager *mm, void **mqd,
>   	m->cp_hqd_persistent_state = CP_HQD_PERSISTENT_STATE__PRELOAD_REQ_MASK |
>   			0x53 << CP_HQD_PERSISTENT_STATE__PRELOAD_SIZE__SHIFT;
>   
> +	m->cp_hqd_pq_control = 5 << CP_HQD_PQ_CONTROL__RPTR_BLOCK_SIZE__SHIFT;
>   	m->cp_mqd_control = 1 << CP_MQD_CONTROL__PRIV_STATE__SHIFT;
>   
>   	m->cp_mqd_base_addr_lo        = lower_32_bits(addr);
> @@ -167,7 +168,7 @@ static void update_mqd(struct mqd_manager *mm, void *mqd,
>   
>   	m = get_mqd(mqd);
>   
> -	m->cp_hqd_pq_control = 5 << CP_HQD_PQ_CONTROL__RPTR_BLOCK_SIZE__SHIFT;
> +	m->cp_hqd_pq_control &= ~CP_HQD_PQ_CONTROL__QUEUE_SIZE_MASK;
>   	m->cp_hqd_pq_control |=
>   			ffs(q->queue_size / sizeof(unsigned int)) - 1 - 1;
>   	m->cp_hqd_pq_control |= CP_HQD_PQ_CONTROL__UNORD_DISPATCH_MASK;

Looks functionally OK but I'd also move the line above into init_mqd for 
consistency. I think the intent of update_mqd is to change fields which 
depend on the queue_properties input, with constant fields set in init_mqd.

> @@ -121,8 +121,10 @@ static void init_mqd(struct mqd_manager *mm, void **mqd,
>   	m->cp_hqd_persistent_state = CP_HQD_PERSISTENT_STATE__PRELOAD_REQ_MASK |
>   			0x55 << CP_HQD_PERSISTENT_STATE__PRELOAD_SIZE__SHIFT;
>   
> +	m->cp_hqd_pq_control = 5 << CP_HQD_PQ_CONTROL__RPTR_BLOCK_SIZE__SHIFT;
>   	m->cp_mqd_control = 1 << CP_MQD_CONTROL__PRIV_STATE__SHIFT;
>   
> +

Remove whitespace change above.

With those fixed:

Reviewed-by: Jay Cornwall <jay.cornwall at amd.com>


More information about the amd-gfx mailing list