Re: [PATCH v3 2/8] drm/i915/selftests: Use drm_* prints in live selftests

[Andi Shyti <andi.shyti@kernel.org>]

Hi Krzysztof,

...

>  static int intel_fw_table_check(const struct intel_forcewake_range *ranges,
>  				unsigned int num_ranges,
> -				bool is_watertight)
> +				bool is_watertight,
> +				const struct drm_device *drm)
>  {
>  	unsigned int i;
>  	s32 prev;
> @@ -36,22 +39,22 @@ static int intel_fw_table_check(const struct intel_forcewake_range *ranges,
>  	for (i = 0, prev = -1; i < num_ranges; i++, ranges++) {
>  		/* Check that the table is watertight */
>  		if (is_watertight && (prev + 1) != (s32)ranges->start) {
> -			pr_err("%s: entry[%d]:(%x, %x) is not watertight to previous (%x)\n",
> -			       __func__, i, ranges->start, ranges->end, prev);
> +			drm_err(drm, "%s: entry[%d]:(%x, %x) is not watertight to previous (%x)\n",
> +				__func__, i, ranges->start, ranges->end, prev);
>  			return -EINVAL;
>  		}
>  
>  		/* Check that the table never goes backwards */
>  		if (prev >= (s32)ranges->start) {
> -			pr_err("%s: entry[%d]:(%x, %x) is less than the previous (%x)\n",
> -			       __func__, i, ranges->start, ranges->end, prev);
> +			drm_err(drm, "%s: entry[%d]:(%x, %x) is less than the previous (%x)\n",
> +				__func__, i, ranges->start, ranges->end, prev);
>  			return -EINVAL;
>  		}
>  
>  		/* Check that the entry is valid */
>  		if (ranges->start >= ranges->end) {
> -			pr_err("%s: entry[%d]:(%x, %x) has negative length\n",
> -			       __func__, i, ranges->start, ranges->end);
> +			drm_err(drm, "%s: entry[%d]:(%x, %x) has negative length\n",
> +				__func__, i, ranges->start, ranges->end);
>  			return -EINVAL;
>  		}
>  
> @@ -126,7 +129,8 @@ int intel_uncore_mock_selftests(void)
>  	for (i = 0; i < ARRAY_SIZE(fw); i++) {
>  		err = intel_fw_table_check(fw[i].ranges,
>  					   fw[i].num_ranges,
> -					   fw[i].is_watertight);
> +					   fw[i].is_watertight,
> +					   NULL);

arrghh! Here we are again at the drm_err(NULL,...) case.

You could eventually do in intel_fw_table_check()

	if (drm)
		drm_err(drm, ...);
	else
		pr_err(...);

but I think it's not worth the effort, just leave pr_err().
Besides, passing the drm device is quite out of the scope of the
function itself, it's just odd!

Andi

>  		if (err)
>  			return err;
>  	}