Re: [PATCH v8 3/3] drm/xe/soc_remapper: Add system controller config for SoC remapper

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Wed, Mar 25, 2026 at 08:02:21AM -0700, Dixit, Ashutosh wrote:
>On Tue, 23 Dec 2025 10:39:47 -0800, Umesh Nerlige Ramappa wrote:
>>
>> @@ -30,10 +36,17 @@ static void xe_soc_remapper_set_telem_region(struct xe_device *xe, u32 index)
>>   */
>>  int xe_soc_remapper_init(struct xe_device *xe)
>>  {
>> -	if (xe->info.has_soc_remapper_telem) {
>> +	bool has_soc_remapper = xe->info.has_soc_remapper_telem ||
>> +				xe->info.has_soc_remapper_sysctrl;
>> +
>> +	if (has_soc_remapper)
>>		spin_lock_init(&xe->soc_remapper.lock);
>> +
>> +	if (xe->info.has_soc_remapper_telem)
>>		xe->soc_remapper.set_telem_region = xe_soc_remapper_set_telem_region;
>> -	}
>> +
>> +	if (xe->info.has_soc_remapper_sysctrl)
>> +		xe->soc_remapper.set_sysctrl_region = xe_soc_remapper_set_sysctrl_region;
>
>Maybe these if () statements hould be moved inside these functions?
>Otherwise they will need to replicated in every place these functions are
>called?

The idea was for the caller to check if the funcs are available and then 
deal with the outcome. In sysctrl, the check was changed to assert due 
to some review comments. We are trying to change it back to a check and 
that should do for now. The telem caller is checking for it and bailing 
out as needed.

These function pointers are a bit off in terms of implementation. Matt 
Brost had suggested to clean them up and use vfuncs instead, but that 
refactor is still in the works.

Thanks,
Umesh

>
>We've already seen this problem in the sysctrl series.