diff mbox

drm/i915: Fix build warning in debugfs

Message ID 1417483155-537-1-git-send-email-matthew.d.roper@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matt Roper Dec. 2, 2014, 1:19 a.m. UTC
i915_gem_obj_ggtt_pin() is marked as __must_check, but one callsite in
debugfs (i915_dump_lrc) was not checking the return value.  Add the
necessary check and make the debugfs node print something
semi-reasonable if we were to fail to pin.

I believe there's already in-progress discussion on the mailing list
about how this area of the code is going to be reworked in general.  But
in the meantime, this is just a quick fix to shut up the compiler
warning.

This warning was added in commit:

        commit dcb4c12a687710ab745c2cdee8298c3e97f6f707
        Author: Oscar Mateo <oscar.mateo@intel.com>
        Date:   Thu Nov 13 10:28:10 2014 +0000

            drm/i915/bdw: Pin the context backing objects to GGTT on-demand

Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Thomas Daniel <thomas.daniel@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Thomas Daniel Dec. 2, 2014, 9:43 a.m. UTC | #1
> -----Original Message-----
> From: Roper, Matthew D
> Sent: Tuesday, December 02, 2014 1:19 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Roper, Matthew D; Mateo Lozano, Oscar; Daniel, Thomas
> Subject: [PATCH] drm/i915: Fix build warning in debugfs
> 
> i915_gem_obj_ggtt_pin() is marked as __must_check, but one callsite in
> debugfs (i915_dump_lrc) was not checking the return value.  Add the
> necessary check and make the debugfs node print something semi-
> reasonable if we were to fail to pin.
> 
> I believe there's already in-progress discussion on the mailing list about how
> this area of the code is going to be reworked in general.  But in the
> meantime, this is just a quick fix to shut up the compiler warning.
I was actually going to post a new version of the fix patch yesterday for this, but during testing I discovered that execlists mode is broken at the moment so my attention went to that.

Agree that shutting up the compiler is good for the short term but this is Daniel's call.

Thomas


> This warning was added in commit:
> 
>         commit dcb4c12a687710ab745c2cdee8298c3e97f6f707
>         Author: Oscar Mateo <oscar.mateo@intel.com>
>         Date:   Thu Nov 13 10:28:10 2014 +0000
> 
>             drm/i915/bdw: Pin the context backing objects to GGTT on-demand
> 
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Thomas Daniel <thomas.daniel@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 7ea3843..9934ec9 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1801,10 +1801,15 @@ static int i915_dump_lrc(struct seq_file *m, void
> *unused)
>  			if (ctx_obj) {
>  				struct page *page;
>  				uint32_t *reg_state;
> -				int j;
> +				int ret, j;
> 
> -				i915_gem_obj_ggtt_pin(ctx_obj,
> +				ret = i915_gem_obj_ggtt_pin(ctx_obj,
>  						GEN8_LR_CONTEXT_ALIGN,
> 0);
> +				if (ret) {
> +					seq_printf(m, "CONTEXT: %s
> (unavail)\n",
> +						   ring->name);
> +					continue;
> +				}
> 
>  				page = i915_gem_object_get_page(ctx_obj,
> 1);
>  				reg_state = kmap_atomic(page);
> --
> 1.8.5.1
Shuang He Dec. 3, 2014, 12:48 a.m. UTC | #2
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              364/364              363/364
ILK                 -2              365/366              363/366
SNB                 -1              450/450              449/450
IVB                                  498/498              498/498
BYT                                  289/289              289/289
HSW                                  564/564              564/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_drm_import_export_prime      PASS(2, M25M23)      NO_RESULT(1, M23)
 ILK  igt_kms_flip_bcs-flip-vs-modeset-interruptible      DMESG_WARN(1, M26)PASS(6, M26)      DMESG_WARN(1, M26)
 ILK  igt_kms_flip_plain-flip-ts-check-interruptible      DMESG_WARN(2, M26)PASS(3, M26)      DMESG_WARN(1, M26)
*SNB  igt_kms_pipe_crc_basic_read-crc-pipe-A-frame-sequence      PASS(2, M35M22)      DMESG_WARN(1, M22)
Note: You need to pay more attention to line start with '*'
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7ea3843..9934ec9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1801,10 +1801,15 @@  static int i915_dump_lrc(struct seq_file *m, void *unused)
 			if (ctx_obj) {
 				struct page *page;
 				uint32_t *reg_state;
-				int j;
+				int ret, j;
 
-				i915_gem_obj_ggtt_pin(ctx_obj,
+				ret = i915_gem_obj_ggtt_pin(ctx_obj,
 						GEN8_LR_CONTEXT_ALIGN, 0);
+				if (ret) {
+					seq_printf(m, "CONTEXT: %s (unavail)\n",
+						   ring->name);
+					continue;
+				}
 
 				page = i915_gem_object_get_page(ctx_obj, 1);
 				reg_state = kmap_atomic(page);