On Wed, 15 Feb 2023, "Dixit, Ashutosh" <ashutosh.dixit@xxxxxxxxx> wrote:
> On Tue, 14 Feb 2023 16:54:14 -0800, Umesh Nerlige Ramappa wrote:
>>
>> Check for return value from i915_perf_init and fail driver init if perf
>> init fails.
>
> Guess we'll start returning anything other than zero later:
And before doing so, this change is useless, and IMO should not be
merged. If it can't or doesn't fail, it should be void. The probe is
complicated enough as is, let's not complicate it unless there's a
reason for it.
Moreover, we'll probably want to consider what parts of the driver probe
are allowed to fail, and the driver can still function. I'm pretty sure
most people prefer a working screen without OA over failing probe.
BR,
Jani.
>
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx>
>
>>
>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/i915_driver.c | 4 +++-
>> drivers/gpu/drm/i915/i915_perf.c | 4 +++-
>> drivers/gpu/drm/i915/i915_perf.h | 2 +-
>> 3 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>> index 0c0ae3eabb4b..998ca41c9713 100644
>> --- a/drivers/gpu/drm/i915/i915_driver.c
>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>> @@ -477,7 +477,9 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>> if (ret)
>> return ret;
>>
>> - i915_perf_init(dev_priv);
>> + ret = i915_perf_init(dev_priv);
>> + if (ret)
>> + return ret;
>>
>> ret = i915_ggtt_probe_hw(dev_priv);
>> if (ret)
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index 0b2097ad000e..e134523576f8 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -4845,7 +4845,7 @@ static void i915_perf_init_info(struct drm_i915_private *i915)
>> * Note: i915-perf initialization is split into an 'init' and 'register'
>> * phase with the i915_perf_register() exposing state to userspace.
>> */
>> -void i915_perf_init(struct drm_i915_private *i915)
>> +int i915_perf_init(struct drm_i915_private *i915)
>> {
>> struct i915_perf *perf = &i915->perf;
>>
>> @@ -4962,6 +4962,8 @@ void i915_perf_init(struct drm_i915_private *i915)
>>
>> oa_init_supported_formats(perf);
>> }
>> +
>> + return 0;
>> }
>>
>> static int destroy_config(int id, void *p, void *data)
>> diff --git a/drivers/gpu/drm/i915/i915_perf.h b/drivers/gpu/drm/i915/i915_perf.h
>> index f96e09a4af04..253637651d5e 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.h
>> +++ b/drivers/gpu/drm/i915/i915_perf.h
>> @@ -18,7 +18,7 @@ struct i915_oa_config;
>> struct intel_context;
>> struct intel_engine_cs;
>>
>> -void i915_perf_init(struct drm_i915_private *i915);
>> +int i915_perf_init(struct drm_i915_private *i915);
>> void i915_perf_fini(struct drm_i915_private *i915);
>> void i915_perf_register(struct drm_i915_private *i915);
>> void i915_perf_unregister(struct drm_i915_private *i915);
>> --
>> 2.36.1
>>
--
Jani Nikula, Intel Open Source Graphics Center
