Hi Krzysztof,
On 2026-08-28 at 10:03:33 +0000, Krzysztof Karas wrote:
> Move away from generic pr prints to device context aware drm
> versions.
>
> Assisted-by: GitHub-Copilot:claude-opus-5
> Signed-off-by: Krzysztof Karas <[email protected]>
> ---
> .../gpu/drm/i915/selftests/i915_gem_evict.c | 75 +++--
> i915_request_put(last);
<snip>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> index e6bd06774816..0c855f00c3ae 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> @@ -25,6 +25,8 @@
> #include <linux/list_sort.h>
> #include <linux/prime_numbers.h>
>
> +#include <drm/drm_print.h>
> +
> #include "gem/i915_gem_context.h"
> #include "gem/i915_gem_internal.h"
> #include "gem/i915_gem_lmem.h"
> @@ -34,6 +36,7 @@
> #include "gt/intel_gpu_commands.h"
> #include "gt/intel_gtt.h"
>
> +#include "../i915_drv.h"
Is the ../ really necessary here? It builds for me if i just switch to
#include "i915_drv.h", other includes treat driver/gpu/drm/i915 as the
working dir. I've seen both styles used in the selftests but it just
looks a bit out of place here.
<snip>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_perf.c
> b/drivers/gpu/drm/i915/selftests/i915_perf.c
> index e9469e27f42a..d2a1a901d47b 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_perf.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_perf.c
> @@ -14,6 +14,8 @@
> #include "igt_flush_test.h"
> #include "lib_sw_fence.h"
>
> +#include <drm/drm_print.h>
> +
Giga nit, but should this be before the "" includes?
<snip>
> diff --git a/drivers/gpu/drm/i915/selftests/intel_uncore.c
> b/drivers/gpu/drm/i915/selftests/intel_uncore.c
> index 514d2200751b..1f58024150fa 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_uncore.c
> @@ -26,9 +26,12 @@
>
> #include "gt/intel_gt.h"
>
> +#include <drm/drm_print.h>
> +
Should this be before the "" includes?
<not snipping here, same file below>
> 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);
I'll join Andi here in that this looks a bit weird. Especially since the
next function in the file intel_shadow_table_check() keeps using
pr_err(). Either convert that too or don't do either.
<snippnig the rest>
Thanks
Krzysztof
