Re: [Intel-xe] [PATCH 1/3] drm/xe: Add a couple of pcode helpers

["Sundaresan, Sujaritha" <sujaritha.sundaresan@intel.com>]

On 9/3/2023 7:00 PM, Sundaresan, Sujaritha wrote:
>
> On 9/2/2023 2:04 AM, Rodrigo Vivi wrote:
>> On Fri, Sep 01, 2023 at 05:45:43PM +0530, Sujaritha Sundaresan wrote:
>>> Some pcode commands take additional sub-commands and parameters. Add a
>>> couple of helpers to help formatting these commands to improve code
>>> readability.
>>>
>>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>>> ---
>>>   drivers/gpu/drm/xe/xe_pcode.c | 28 ++++++++++++++++++++++++++++
>>>   drivers/gpu/drm/xe/xe_pcode.h |  3 +++
>>>   2 files changed, 31 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_pcode.c 
>>> b/drivers/gpu/drm/xe/xe_pcode.c
>>> index 7f1bf2297f51..e45169f47500 100644
>>> --- a/drivers/gpu/drm/xe/xe_pcode.c
>>> +++ b/drivers/gpu/drm/xe/xe_pcode.c
>>> @@ -104,6 +104,34 @@ int xe_pcode_read(struct xe_gt *gt, u32 mbox, 
>>> u32 *val, u32 *val1)
>>>       return err;
>>>   }
>> a doc would be required...
>>
>>> +int xe_pcode_read_p(struct xe_gt *gt, u32 mbcmd, u32 p1, u32 p2, 
>>> u32 *val)
>> a better name would be nice....
>>
>>> +{
>>> +    u32 mbox;
>>> +    int err;
>>> +
>>> +    mbox = REG_FIELD_PREP(PCODE_MB_COMMAND, mbcmd)
>>> +        | REG_FIELD_PREP(PCODE_MB_PARAM1, p1)
>>> +        | REG_FIELD_PREP(PCODE_MB_PARAM2, p2);
>>> +
>>> +    err = xe_pcode_read(gt, mbox, val, NULL);
>> but why not simply modifying the existent one to accept 2 params?
>>
>> int xe_pcode_read(struct xe_gt *gt, u32 mbox_param1, u32 mbox_param2,
>>                u32 *val, u32 *val1)
>>
>> and the equivalent write...
>>
>> oh, and while doing that, could you please add the missing documentation
>> to these 2 functions?
>>
>> Thanks,
>> Rodrigo.
>
> Sure that would work. Will add the docs as well.
>
> Thanks,
>
> Suja

Hi Rodrigo,

Another question,

I can change the existing pcode_read function, but would it be better to 
have a separate new write equivalent ?

>
>>
>>> +
>>> +    return err;
>>> +}
>>> +
>>> +int xe_pcode_write_p(struct xe_gt *gt, u32 mbcmd, u32 p1, u32 p2, 
>>> u32 val)
>>> +{
>>> +    u32 mbox;
>>> +    int err;
>>> +
>>> +    mbox = REG_FIELD_PREP(PCODE_MB_COMMAND, mbcmd)
>>> +        | REG_FIELD_PREP(PCODE_MB_PARAM1, p1)
>>> +        | REG_FIELD_PREP(PCODE_MB_PARAM2, p2);
>>> +
>>> +    err = xe_pcode_write(gt, mbox, val);
>>> +
>>> +    return err;
>>> +}
>>> +
>>>   static int xe_pcode_try_request(struct xe_gt *gt, u32 mbox,
>>>                   u32 request, u32 reply_mask, u32 reply,
>>>                   u32 *status, bool atomic, int timeout_us)
>>> diff --git a/drivers/gpu/drm/xe/xe_pcode.h 
>>> b/drivers/gpu/drm/xe/xe_pcode.h
>>> index 3b4aa8c1a3ba..8d4103afd7e0 100644
>>> --- a/drivers/gpu/drm/xe/xe_pcode.h
>>> +++ b/drivers/gpu/drm/xe/xe_pcode.h
>>> @@ -19,6 +19,9 @@ int xe_pcode_write_timeout(struct xe_gt *gt, u32 
>>> mbox, u32 val,
>>>   #define xe_pcode_write(gt, mbox, val) \
>>>       xe_pcode_write_timeout(gt, mbox, val, 1)
>>>   +int xe_pcode_read_p(struct xe_gt *gt, u32 mbcmd, u32 p1, u32 p2, 
>>> u32 *val);
>>> +int xe_pcode_write_p(struct xe_gt *gt, u32 mbcmd, u32 p1, u32 p2, 
>>> u32 val);
>>> +
>>>   int xe_pcode_request(struct xe_gt *gt, u32 mbox, u32 request,
>>>                u32 reply_mask, u32 reply, int timeout_ms);
>>>   --
>>> 2.25.1
>>>