Re: [PATCH 2/4] drm/xe/vf: Custom uC initialization

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 06.06.2024 13:37, Piotr Piórkowski wrote:
> Michal Wajdeczko <michal.wajdeczko@intel.com> wrote on wto [2024-cze-04 23:22:29 +0200]:
>> VF drivers can't modify WOPCM registers nor upload firmwares to
>> GuC, HuC or GSC.  Modify xe_uc initialization functions to skip
>> those steps if running in the VF mode.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_uc.c | 33 +++++++++++++++++++++++++++++++++
>>  1 file changed, 33 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_uc.c b/drivers/gpu/drm/xe/xe_uc.c
>> index 85808706d1c8..0f240534fb72 100644
>> --- a/drivers/gpu/drm/xe/xe_uc.c
>> +++ b/drivers/gpu/drm/xe/xe_uc.c
>> @@ -11,9 +11,11 @@
>>  #include "xe_gsc_proxy.h"
>>  #include "xe_gt.h"
>>  #include "xe_gt_printk.h"
>> +#include "xe_gt_sriov_vf.h"
>>  #include "xe_guc.h"
>>  #include "xe_guc_pc.h"
>>  #include "xe_huc.h"
>> +#include "xe_sriov.h"
>>  #include "xe_uc_fw.h"
>>  #include "xe_wopcm.h"
>>  
>> @@ -53,6 +55,9 @@ int xe_uc_init(struct xe_uc *uc)
>>  	if (!xe_device_uc_enabled(uc_to_xe(uc)))
>>  		return 0;
>>  
>> +	if (IS_SRIOV_VF(uc_to_xe(uc)))
>> +		return 0;
>> +
>>  	ret = xe_wopcm_init(&uc->wopcm);
>>  	if (ret)
>>  		goto err;
>> @@ -141,6 +146,31 @@ int xe_uc_init_hwconfig(struct xe_uc *uc)
>>  	return 0;
>>  }
>>  
>> +static int vf_uc_init_hw(struct xe_uc *uc)
>> +{
>> +	int err;
>> +
>> +	err = xe_uc_sanitize_reset(uc);
>> +	if (err)
>> +		return err;
>> +
>> +	err = xe_guc_enable_communication(&uc->guc);
>> +	if (err)
>> +		return err;
>> +
>> +	err = xe_gt_sriov_vf_connect(uc_to_gt(uc));
>> +	if (err)
>> +		return err;
>> +
>> +	uc->guc.submission_state.enabled = true;
>> +
>> +	err = xe_gt_record_default_lrcs(uc_to_gt(uc));
>> +	if (err)
>> +		return err;
>> +
>> +	return 0;
>> +}
>> +
>>  /*
>>   * Should be called during driver load, after every GT reset, and after every
>>   * suspend to reload / auth the firmwares.
>> @@ -153,6 +183,9 @@ int xe_uc_init_hw(struct xe_uc *uc)
>>  	if (!xe_device_uc_enabled(uc_to_xe(uc)))
>>  		return 0;
>>  
>> +	if (IS_SRIOV_VF(uc_to_xe(uc)))
>> +		return vf_uc_init_hw(uc);
>> +
>>  	ret = xe_huc_upload(&uc->huc);
>>  	if (ret)
>>  		return ret;
> 
> I would expand the commit description a bit, because you mention what we should
> not do on VF, but here we also add a new vf_uc_init_hw function.

will change commit message to:

"
VF drivers can't modify WOPCM registers nor upload firmwares to
GuC, HuC or GSC.  Modify xe_uc initialization functions to skip
those steps if running in the VF mode, or defer to new custom
helper function that would not include those steps.
"

> 
> If you expand the description then LGTM:
> Reviewed-by: Piotr Piórkowski <piotr.piorkowski@intel.com>

thanks!

> 
>> -- 
>> 2.43.0
>>
>