[Mesa-dev] [PATCH] i965: Make intel_get_param return an int

[Connor Abbott]

On Tue, Apr 12, 2016 at 11:39 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> On Apr 11, 2016 9:49 AM, "Ben Widawsky" <ben at bwidawsk.net> wrote:
>>
>> This will fix the spurious error message: "Failed to query GPU
>> properties."
>> that was unintentionally added in cc01b63d730.
>>
>> This patch changes the function to return an int so that the caller is
>> able to
>> do stuff based on the return value.
>>
>> The equivalent of this patch was in the original series that fixed up the
>> warning, but I dropped it at the last moment. It is required to make the
>> desired
>> behavior of not warning when trying to query GPU properties from the
>> kernel
>> unless there is something the user can do about it.
>>
>> NOTE: Broadwell appears to actually have some issue where the kernel
>> returns
>> ENODEV when it shouldn't be. I will investigate this separately.
>>
>> Reported-by: Chris Forbes <chrisf at ijw.co.nz>
>> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
>> ---
>>  src/mesa/drivers/dri/i965/intel_screen.c | 13 ++++++-------
>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
>> b/src/mesa/drivers/dri/i965/intel_screen.c
>> index 03e6852..d1e8b68 100644
>> --- a/src/mesa/drivers/dri/i965/intel_screen.c
>> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
>> @@ -932,7 +932,7 @@ static const __DRIextension
>> *intelRobustScreenExtensions[] = {
>>      NULL
>>  };
>>
>> -static bool
>> +static int
>>  intel_get_param(__DRIscreen *psp, int param, int *value)
>>  {
>>     int ret;
>> @@ -946,17 +946,16 @@ intel_get_param(__DRIscreen *psp, int param, int
>> *value)
>>     if (ret) {
>>        if (ret != -EINVAL)
>>          _mesa_warning(NULL, "drm_i915_getparam: %d", ret);
>> -      return false;
>>     }
>>
>> -   return true;
>> +   return ret;
>>  }
>>
>>  static bool
>>  intel_get_boolean(__DRIscreen *psp, int param)
>>  {
>>     int value = 0;
>> -   return intel_get_param(psp, param, &value) && value;
>> +   return (intel_get_param(psp, param, &value) == 0) && value;
>>  }
>>
>>  static void
>> @@ -1089,12 +1088,12 @@ intel_detect_sseu(struct intel_screen
>> *intelScreen)
>>
>>     ret = intel_get_param(intelScreen->driScrnPriv,
>> I915_PARAM_SUBSLICE_TOTAL,
>>                           &intelScreen->subslice_total);
>> -   if (ret != -EINVAL)
>> +   if (ret < 0 && ret != -EINVAL)
>>        goto err_out;
>>
>>     ret = intel_get_param(intelScreen->driScrnPriv,
>>                           I915_PARAM_EU_TOTAL, &intelScreen->eu_total);
>> -   if (ret != -EINVAL)
>> +   if (ret < 0 && ret != -EINVAL)
>>        goto err_out;
>>
>>     /* Without this information, we cannot get the right Braswell
>> brandstrings,
>> @@ -1110,7 +1109,7 @@ intel_detect_sseu(struct intel_screen *intelScreen)
>>  err_out:
>>     intelScreen->subslice_total = -1;
>>     intelScreen->eu_total = -1;
>> -   _mesa_warning(NULL, "Failed to query GPU properties.\n");
>> +   _mesa_warning(NULL, "Failed to query GPU properties (%d).\n", ret);
>
> Isn't there some magic printf thing that will turn an error code into a
> useful string?  I know %m works for errno but is there one you can use when
> the function returns the error?

Maybe strerror(3) + %s?

>
> Either way,
>
> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
>
>>  }
>>
>>  static bool
>> --
>> 2.8.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>