[PATCH 1/9] drm/i915: Replace ten seq_puts() calls by seq_putc()

[PATCH 1/9] drm/i915: Replace ten seq_puts() calls by seq_putc()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 11:04:45 +0200

Some single characters should be put into a sequence.
Thus use the corresponding function "seq_putc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d689e511744e..f2bda699749a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -190,7 +190,7 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
 			seq_printf(m, " , fence: %d%s",
 				   vma->fence->id,
 				   i915_gem_active_isset(&vma->last_fence) ? "*" : "");
-		seq_puts(m, ")");
+		seq_putc(m, ')');
 	}
 	if (obj->stolen)
 		seq_printf(m, " (stolen: %08llx)", obj->stolen->start);
@@ -2689,7 +2689,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
 			    (stat[pipe] == VLV_EDP_PSR_ACTIVE_SF_UPDATE))
 				seq_printf(m, " pipe %c", pipe_name(pipe));
 		}
-	seq_puts(m, "\n");
+	seq_putc(m, '\n');
 
 	/*
 	 * VLV/CHV PSR has no kind of performance counter
@@ -3176,7 +3176,7 @@ static void intel_scaler_info(struct seq_file *m, struct intel_crtc *intel_crtc)
 			seq_printf(m, ", scalers[%d]: use=%s, mode=%x",
 				   i, yesno(sc->in_use), sc->mode);
 		}
-		seq_puts(m, "\n");
+		seq_putc(m, '\n');
 	} else {
 		seq_puts(m, "\tNo scalers available on this platform\n");
 	}
@@ -3384,8 +3384,7 @@ static int i915_engine_info(struct seq_file *m, void *unused)
 				   w->tsk->comm, w->tsk->pid, w->seqno);
 		}
 		spin_unlock_irq(&b->rb_lock);
-
-		seq_puts(m, "\n");
+		seq_putc(m, '\n');
 	}
 
 	intel_runtime_pm_put(dev_priv);
@@ -3629,7 +3628,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
 		/* DRRS not supported. Print the VBT parameter*/
 		seq_puts(m, "\tDRRS Supported : No");
 	}
-	seq_puts(m, "\n");
+	seq_putc(m, '\n');
 }
 
 static int i915_drrs_status(struct seq_file *m, void *unused)
@@ -3764,12 +3763,11 @@ static int i915_displayport_test_active_show(struct seq_file *m, void *data)
 		if (connector->status == connector_status_connected &&
 		    connector->encoder != NULL) {
 			intel_dp = enc_to_intel_dp(connector->encoder);
-			if (intel_dp->compliance.test_active)
-				seq_puts(m, "1");
-			else
-				seq_puts(m, "0");
-		} else
-			seq_puts(m, "0");
+			seq_putc(m,
+				 intel_dp->compliance.test_active ? '1' : '0');
+		} else {
+			seq_putc(m, '0');
+		}
 	}
 	drm_connector_list_iter_end(&conn_iter);
 
@@ -3823,8 +3821,9 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
 				seq_printf(m, "bpc: %u\n",
 					   intel_dp->compliance.test_data.bpc);
 			}
-		} else
-			seq_puts(m, "0");
+		} else {
+			seq_putc(m, '0');
+		}
 	}
 	drm_connector_list_iter_end(&conn_iter);
 
@@ -3864,8 +3863,9 @@ static int i915_displayport_test_type_show(struct seq_file *m, void *data)
 		    connector->encoder != NULL) {
 			intel_dp = enc_to_intel_dp(connector->encoder);
 			seq_printf(m, "%02lx", intel_dp->compliance.test_type);
-		} else
-			seq_puts(m, "0");
+		} else {
+			seq_putc(m, '0');
+		}
 	}
 	drm_connector_list_iter_end(&conn_iter);
 
-- 
2.12.2

[PATCH 2/9] drm/i915: Combine five seq_printf() calls in i915_display_info()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 13:17:10 +0200

Some text was put into a sequence by separate function calls.
Print the same data by two single function calls instead.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f2bda699749a..4adf96be9146 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3191,8 +3191,7 @@ static int i915_display_info(struct seq_file *m, void *unused)
 	struct drm_connector_list_iter conn_iter;
 
 	intel_runtime_pm_get(dev_priv);
-	seq_printf(m, "CRTC info\n");
-	seq_printf(m, "---------\n");
+	seq_puts(m, "CRTC info\n---------\n");
 	for_each_intel_crtc(dev, crtc) {
 		bool active;
 		struct intel_crtc_state *pipe_config;
@@ -3226,9 +3225,7 @@ static int i915_display_info(struct seq_file *m, void *unused)
 		drm_modeset_unlock(&crtc->base.mutex);
 	}
 
-	seq_printf(m, "\n");
-	seq_printf(m, "Connector info\n");
-	seq_printf(m, "--------------\n");
+	seq_puts(m, "\nConnector info\n--------------\n");
 	mutex_lock(&dev->mode_config.mutex);
 	drm_connector_list_iter_begin(dev, &conn_iter);
 	drm_for_each_connector_iter(connector, &conn_iter)
-- 
2.12.2

[PATCH 3/9] drm/i915: Replace 14 seq_printf() calls by seq_puts()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 13:20:47 +0200

Some strings which did not contain data format specifications should be put
into a sequence. Thus use the corresponding function "seq_puts".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 4adf96be9146..296108464f2b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -149,7 +149,7 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
 	}
 	seq_printf(m, " (pinned x %d)", pin_count);
 	if (obj->pin_display)
-		seq_printf(m, " (display)");
+		seq_puts(m, " (display)");
 	list_for_each_entry(vma, &obj->vma_list, obj_link) {
 		if (!drm_mm_node_allocated(&vma->node))
 			continue;
@@ -581,8 +581,10 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 					   intel_engine_last_submit(engine),
 					   intel_engine_get_seqno(engine),
 					   i915_gem_request_completed(work->flip_queued_req));
-			} else
-				seq_printf(m, "Flip not associated with any ring\n");
+			} else {
+				seq_puts(m,
+					 "Flip not associated with any ring\n");
+			}
 			seq_printf(m, "Flip queued on frame %d, (was ready on frame %d), now %d\n",
 				   work->flip_queued_vblank,
 				   work->flip_ready_vblank,
@@ -2048,7 +2050,7 @@ static int i915_dump_lrc(struct seq_file *m, void *unused)
 	int ret;
 
 	if (!i915.enable_execlists) {
-		seq_printf(m, "Logical Ring Contexts are disabled\n");
+		seq_puts(m, "Logical Ring Contexts are disabled\n");
 		return 0;
 	}
 
@@ -2402,7 +2404,7 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
 	if (!HAS_GUC_UCODE(dev_priv))
 		return 0;
 
-	seq_printf(m, "GuC firmware status:\n");
+	seq_puts(m, "GuC firmware status:\n");
 	seq_printf(m, "\tpath: %s\n",
 		guc_fw->path);
 	seq_printf(m, "\tfetch: %s\n",
@@ -2510,7 +2512,7 @@ static int i915_guc_info(struct seq_file *m, void *data)
 		return 0;
 	}
 
-	seq_printf(m, "Doorbell map:\n");
+	seq_puts(m, "Doorbell map:\n");
 	seq_printf(m, "\t%*pb\n", GUC_NUM_DOORBELLS, guc->doorbell_bitmap);
 	seq_printf(m, "Doorbell next cacheline: 0x%x\n\n", guc->db_cacheline);
 
@@ -2521,7 +2523,7 @@ static int i915_guc_info(struct seq_file *m, void *data)
 	seq_printf(m, "GuC last action error code: %d\n", guc->action_err);
 
 	total = 0;
-	seq_printf(m, "\nGuC submissions:\n");
+	seq_puts(m, "\nGuC submissions:\n");
 	for_each_engine(engine, dev_priv, id) {
 		u64 submissions = guc->submissions[id];
 		total += submissions;
@@ -2795,7 +2797,7 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused)
 	seq_printf(m, "Usage count: %d\n",
 		   atomic_read(&dev_priv->drm.dev->power.usage_count));
 #else
-	seq_printf(m, "Device Power Management (CONFIG_PM) disabled\n");
+	seq_puts(m, "Device Power Management (CONFIG_PM) disabled\n");
 #endif
 	seq_printf(m, "PCI device power state: %s [%d]\n",
 		   pci_power_name(pdev->current_state),
@@ -2914,7 +2916,7 @@ static void intel_encoder_info(struct seq_file *m,
 			   drm_get_connector_status_name(connector->status));
 		if (connector->status == connector_status_connected) {
 			struct drm_display_mode *mode = &crtc->mode;
-			seq_printf(m, ", mode:\n");
+			seq_puts(m, ", mode:\n");
 			intel_seq_print_mode(m, 2, mode);
 		} else {
 			seq_putc(m, '\n');
@@ -2945,7 +2947,7 @@ static void intel_panel_info(struct seq_file *m, struct intel_panel *panel)
 {
 	struct drm_display_mode *mode = panel->fixed_mode;
 
-	seq_printf(m, "\tfixed mode:\n");
+	seq_puts(m, "\tfixed mode:\n");
 	intel_seq_print_mode(m, 2, mode);
 }
 
@@ -3038,7 +3040,7 @@ static void intel_connector_info(struct seq_file *m,
 		break;
 	}
 
-	seq_printf(m, "\tmodes:\n");
+	seq_puts(m, "\tmodes:\n");
 	list_for_each_entry(mode, &connector->modes, head)
 		intel_seq_print_mode(m, 2, mode);
 }
@@ -3266,9 +3268,7 @@ static int i915_engine_info(struct seq_file *m, void *unused)
 			   engine->timeline->inflight_seqnos);
 
 		rcu_read_lock();
-
-		seq_printf(m, "\tRequests:\n");
-
+		seq_puts(m, "\tRequests:\n");
 		rq = list_first_entry(&engine->timeline->requests,
 				      struct drm_i915_gem_request, link);
 		if (&rq->link != &engine->timeline->requests)
@@ -3346,7 +3346,7 @@ static int i915_engine_info(struct seq_file *m, void *unused)
 					   engine->execlist_port[0].count);
 				print_request(m, rq, "rq: ");
 			} else {
-				seq_printf(m, "\t\tELSP[0] idle\n");
+				seq_puts(m, "\t\tELSP[0] idle\n");
 			}
 			rq = READ_ONCE(engine->execlist_port[1].request);
 			if (rq) {
@@ -3354,7 +3354,7 @@ static int i915_engine_info(struct seq_file *m, void *unused)
 					   engine->execlist_port[1].count);
 				print_request(m, rq, "rq: ");
 			} else {
-				seq_printf(m, "\t\tELSP[1] idle\n");
+				seq_puts(m, "\t\tELSP[1] idle\n");
 			}
 			rcu_read_unlock();
 
@@ -3465,7 +3465,7 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
 		seq_printf(m, "DPLL%i: %s, id: %i\n", i, pll->name, pll->id);
 		seq_printf(m, " crtc_mask: 0x%08x, active: 0x%x, on: %s\n",
 			   pll->state.crtc_mask, pll->active_mask, yesno(pll->on));
-		seq_printf(m, " tracked hardware state:\n");
+		seq_puts(m, " tracked hardware state:\n");
 		seq_printf(m, " dpll:    0x%08x\n", pll->state.hw_state.dpll);
 		seq_printf(m, " dpll_md: 0x%08x\n",
 			   pll->state.hw_state.dpll_md);
-- 
2.12.2

Re: [PATCH 3/9] drm/i915: Replace 14 seq_printf() calls by seq_puts()

[Chris Wilson]

On Thu, May 04, 2017 at 06:54:16PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 4 May 2017 13:20:47 +0200
> 
> Some strings which did not contain data format specifications should be put
> into a sequence. Thus use the corresponding function "seq_puts".

debugfs / seq_file is not performance critical. Familiar idiomatic code is
much preferred over continually switching between seq_printf and seq_puts.

And don't even start on converting seq_printf / seq_puts to seq_putc...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 3/9] drm/i915: Replace 14 seq_printf() calls by seq_puts()

[Jani Nikula]

On Thu, 04 May 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, May 04, 2017 at 06:54:16PM +0200, SF Markus Elfring wrote:
>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Thu, 4 May 2017 13:20:47 +0200
>> 
>> Some strings which did not contain data format specifications should be put
>> into a sequence. Thus use the corresponding function "seq_puts".
>
> debugfs / seq_file is not performance critical. Familiar idiomatic code is
> much preferred over continually switching between seq_printf and seq_puts.
>
> And don't even start on converting seq_printf / seq_puts to seq_putc...

Agreed. I don't want any of the seq_* changes in this series.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH 4/9] drm/i915: Delete unnecessary braces in three functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 13:40:53 +0200

Do not use curly brackets at some source code places
where a single statement should be sufficient.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 296108464f2b..bf9a2e8d8c16 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -565,13 +565,13 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 			u32 addr;
 
 			pending = atomic_read(&work->pending);
-			if (pending) {
+			if (pending)
 				seq_printf(m, "Flip ioctl preparing on pipe %c (plane %c)\n",
 					   pipe, plane);
-			} else {
+			else
 				seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n",
 					   pipe, plane);
-			}
+
 			if (work->flip_queued_req) {
 				struct intel_engine_cs *engine = work->flip_queued_req->engine;
 
@@ -3130,13 +3130,11 @@ static void intel_plane_info(struct seq_file *m, struct intel_crtc *intel_crtc)
 		}
 
 		state = plane->state;
-
-		if (state->fb) {
+		if (state->fb)
 			drm_get_format_name(state->fb->format->format,
 					    &format_name);
-		} else {
+		else
 			sprintf(format_name.str, "N/A");
-		}
 
 		seq_printf(m, "\t--Plane id %d: type=%s, crtc_pos=%4dx%4d, crtc_size=%4dx%4d, src_pos=%d.%04ux%d.%04u, src_size=%d.%04ux%d.%04u, format=%s, rotation=%s\n",
 			   plane->base.id,
@@ -4636,13 +4634,12 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
 
 	intel_runtime_pm_get(dev_priv);
 
-	if (IS_CHERRYVIEW(dev_priv)) {
+	if (IS_CHERRYVIEW(dev_priv))
 		cherryview_sseu_device_status(dev_priv, &sseu);
-	} else if (IS_BROADWELL(dev_priv)) {
+	else if (IS_BROADWELL(dev_priv))
 		broadwell_sseu_device_status(dev_priv, &sseu);
-	} else if (INTEL_GEN(dev_priv) >= 9) {
+	else if (INTEL_GEN(dev_priv) >= 9)
 		gen9_sseu_device_status(dev_priv, &sseu);
-	}
 
 	intel_runtime_pm_put(dev_priv);
 
-- 
2.12.2

Re: [PATCH 4/9] drm/i915: Delete unnecessary braces in three functions

[Jani Nikula]

On Thu, 04 May 2017, SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 4 May 2017 13:40:53 +0200
>
> Do not use curly brackets at some source code places
> where a single statement should be sufficient.

We only tend to do this kind of changes when we're changing the
surrounding code anyway. I'm sure there are plenty of places where you
could add or remove braces, but it's not productive to go around
changing just them.

BR,
Jani.


>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 296108464f2b..bf9a2e8d8c16 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -565,13 +565,13 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
>  			u32 addr;
>  
>  			pending = atomic_read(&work->pending);
> -			if (pending) {
> +			if (pending)
>  				seq_printf(m, "Flip ioctl preparing on pipe %c (plane %c)\n",
>  					   pipe, plane);
> -			} else {
> +			else
>  				seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n",
>  					   pipe, plane);
> -			}
> +
>  			if (work->flip_queued_req) {
>  				struct intel_engine_cs *engine = work->flip_queued_req->engine;
>  
> @@ -3130,13 +3130,11 @@ static void intel_plane_info(struct seq_file *m, struct intel_crtc *intel_crtc)
>  		}
>  
>  		state = plane->state;
> -
> -		if (state->fb) {
> +		if (state->fb)
>  			drm_get_format_name(state->fb->format->format,
>  					    &format_name);
> -		} else {
> +		else
>  			sprintf(format_name.str, "N/A");
> -		}
>  
>  		seq_printf(m, "\t--Plane id %d: type=%s, crtc_pos=%4dx%4d, crtc_size=%4dx%4d, src_pos=%d.%04ux%d.%04u, src_size=%d.%04ux%d.%04u, format=%s, rotation=%s\n",
>  			   plane->base.id,
> @@ -4636,13 +4634,12 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
>  
>  	intel_runtime_pm_get(dev_priv);
>  
> -	if (IS_CHERRYVIEW(dev_priv)) {
> +	if (IS_CHERRYVIEW(dev_priv))
>  		cherryview_sseu_device_status(dev_priv, &sseu);
> -	} else if (IS_BROADWELL(dev_priv)) {
> +	else if (IS_BROADWELL(dev_priv))
>  		broadwell_sseu_device_status(dev_priv, &sseu);
> -	} else if (INTEL_GEN(dev_priv) >= 9) {
> +	else if (INTEL_GEN(dev_priv) >= 9)
>  		gen9_sseu_device_status(dev_priv, &sseu);
> -	}
>  
>  	intel_runtime_pm_put(dev_priv);

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH 5/9] drm/i915: Adjust seven checks for null pointers

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 13:52:19 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written …

Thus fix affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index bf9a2e8d8c16..d9c699d7245e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -242,7 +242,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
 		if (count == total)
 			break;
 
-		if (obj->stolen == NULL)
+		if (!obj->stolen)
 			continue;
 
 		objects[count++] = obj;
@@ -254,7 +254,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
 		if (count == total)
 			break;
 
-		if (obj->stolen == NULL)
+		if (!obj->stolen)
 			continue;
 
 		objects[count++] = obj;
@@ -557,7 +557,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 
 		spin_lock_irq(&dev->event_lock);
 		work = crtc->flip_work;
-		if (work == NULL) {
+		if (!work) {
 			seq_printf(m, "No flip due on pipe %c (plane %c)\n",
 				   pipe, plane);
 		} else {
@@ -3717,7 +3717,7 @@ static ssize_t i915_displayport_test_active_write(struct file *file,
 			continue;
 
 		if (connector->status == connector_status_connected &&
-		    connector->encoder != NULL) {
+		    connector->encoder) {
 			intel_dp = enc_to_intel_dp(connector->encoder);
 			status = kstrtoint(input_buffer, 10, &val);
 			if (status < 0)
@@ -3756,7 +3756,7 @@ static int i915_displayport_test_active_show(struct seq_file *m, void *data)
 			continue;
 
 		if (connector->status == connector_status_connected &&
-		    connector->encoder != NULL) {
+		    connector->encoder) {
 			intel_dp = enc_to_intel_dp(connector->encoder);
 			seq_putc(m,
 				 intel_dp->compliance.test_active ? '1' : '0');
@@ -3801,7 +3801,7 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
 			continue;
 
 		if (connector->status == connector_status_connected &&
-		    connector->encoder != NULL) {
+		    connector->encoder) {
 			intel_dp = enc_to_intel_dp(connector->encoder);
 			if (intel_dp->compliance.test_type ==
 			    DP_TEST_LINK_EDID_READ)
@@ -3855,7 +3855,7 @@ static int i915_displayport_test_type_show(struct seq_file *m, void *data)
 			continue;
 
 		if (connector->status == connector_status_connected &&
-		    connector->encoder != NULL) {
+		    connector->encoder) {
 			intel_dp = enc_to_intel_dp(connector->encoder);
 			seq_printf(m, "%02lx", intel_dp->compliance.test_type);
 		} else {
-- 
2.12.2

Re: [PATCH 5/9] drm/i915: Adjust seven checks for null pointers

[Jani Nikula]

On Thu, 04 May 2017, SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 4 May 2017 13:52:19 +0200
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> The script “checkpatch.pl” pointed information out like the following.
>
> Comparison to NULL could be written …

Could be written one way or the other. We have and accept
both. Sometimes explicit comparison with NULL is preferred, depending on
judgement, not based on what a tool says.

BR,
Jani.


>
> Thus fix affected source code places.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index bf9a2e8d8c16..d9c699d7245e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -242,7 +242,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
>  		if (count == total)
>  			break;
>  
> -		if (obj->stolen == NULL)
> +		if (!obj->stolen)
>  			continue;
>  
>  		objects[count++] = obj;
> @@ -254,7 +254,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
>  		if (count == total)
>  			break;
>  
> -		if (obj->stolen == NULL)
> +		if (!obj->stolen)
>  			continue;
>  
>  		objects[count++] = obj;
> @@ -557,7 +557,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
>  
>  		spin_lock_irq(&dev->event_lock);
>  		work = crtc->flip_work;
> -		if (work == NULL) {
> +		if (!work) {
>  			seq_printf(m, "No flip due on pipe %c (plane %c)\n",
>  				   pipe, plane);
>  		} else {
> @@ -3717,7 +3717,7 @@ static ssize_t i915_displayport_test_active_write(struct file *file,
>  			continue;
>  
>  		if (connector->status == connector_status_connected &&
> -		    connector->encoder != NULL) {
> +		    connector->encoder) {
>  			intel_dp = enc_to_intel_dp(connector->encoder);
>  			status = kstrtoint(input_buffer, 10, &val);
>  			if (status < 0)
> @@ -3756,7 +3756,7 @@ static int i915_displayport_test_active_show(struct seq_file *m, void *data)
>  			continue;
>  
>  		if (connector->status == connector_status_connected &&
> -		    connector->encoder != NULL) {
> +		    connector->encoder) {
>  			intel_dp = enc_to_intel_dp(connector->encoder);
>  			seq_putc(m,
>  				 intel_dp->compliance.test_active ? '1' : '0');
> @@ -3801,7 +3801,7 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
>  			continue;
>  
>  		if (connector->status == connector_status_connected &&
> -		    connector->encoder != NULL) {
> +		    connector->encoder) {
>  			intel_dp = enc_to_intel_dp(connector->encoder);
>  			if (intel_dp->compliance.test_type ==
>  			    DP_TEST_LINK_EDID_READ)
> @@ -3855,7 +3855,7 @@ static int i915_displayport_test_type_show(struct seq_file *m, void *data)
>  			continue;
>  
>  		if (connector->status == connector_status_connected &&
> -		    connector->encoder != NULL) {
> +		    connector->encoder) {
>  			intel_dp = enc_to_intel_dp(connector->encoder);
>  			seq_printf(m, "%02lx", intel_dp->compliance.test_type);
>  		} else {

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH 6/9] drm/i915: Add spaces for better code readability

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 14:04:38 +0200

Use space characters at some source code places according to
the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d9c699d7245e..6f3119d40c50 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2358,7 +2358,7 @@ static int i915_llc(struct seq_file *m, void *data)
 
 	seq_printf(m, "LLC: %s\n", yesno(HAS_LLC(dev_priv)));
 	seq_printf(m, "%s: %lluMB\n", edram ? "eDRAM" : "eLLC",
-		   intel_uncore_edram_size(dev_priv)/1024/1024);
+		   intel_uncore_edram_size(dev_priv) / 1024 / 1024);
 
 	return 0;
 }
@@ -4502,7 +4502,7 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
 {
 	int s_max = 3, ss_max = 4;
 	int s, ss;
-	u32 s_reg[s_max], eu_reg[2*s_max], eu_mask[2];
+	u32 s_reg[s_max], eu_reg[2 * s_max], eu_mask[2];
 
 	/* BXT has a single slice and at most 3 subslices. */
 	if (IS_GEN9_LP(dev_priv)) {
@@ -4512,8 +4512,8 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
 
 	for (s = 0; s < s_max; s++) {
 		s_reg[s] = I915_READ(GEN9_SLICE_PGCTL_ACK(s));
-		eu_reg[2*s] = I915_READ(GEN9_SS01_EU_PGCTL_ACK(s));
-		eu_reg[2*s + 1] = I915_READ(GEN9_SS23_EU_PGCTL_ACK(s));
+		eu_reg[2 * s] = I915_READ(GEN9_SS01_EU_PGCTL_ACK(s));
+		eu_reg[2 * s + 1] = I915_READ(GEN9_SS23_EU_PGCTL_ACK(s));
 	}
 
 	eu_mask[0] = GEN9_PGCTL_SSA_EU08_ACK |
@@ -4547,8 +4547,8 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
 				sseu->subslice_mask |= BIT(ss);
 			}
 
-			eu_cnt = 2 * hweight32(eu_reg[2*s + ss/2] &
-					       eu_mask[ss%2]);
+			eu_cnt = 2 * hweight32(eu_reg[2 * s + ss / 2] &
+					       eu_mask[ss % 2]);
 			sseu->eu_total += eu_cnt;
 			sseu->eu_per_subslice = max_t(unsigned int,
 						      sseu->eu_per_subslice,
-- 
2.12.2

Re: [PATCH 6/9] drm/i915: Add spaces for better code readability

[Jani Nikula]

On Thu, 04 May 2017, SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 4 May 2017 14:04:38 +0200
>
> Use space characters at some source code places according to
> the Linux coding style convention.

LGTM. Frankly the only concern I have with accepting this patch is that
it encourages you and others to submit more patches like
this. Generally, we do this kind of changes only when touching the
nearby code for some real changes.

BR,
Jani.

>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index d9c699d7245e..6f3119d40c50 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2358,7 +2358,7 @@ static int i915_llc(struct seq_file *m, void *data)
>  
>  	seq_printf(m, "LLC: %s\n", yesno(HAS_LLC(dev_priv)));
>  	seq_printf(m, "%s: %lluMB\n", edram ? "eDRAM" : "eLLC",
> -		   intel_uncore_edram_size(dev_priv)/1024/1024);
> +		   intel_uncore_edram_size(dev_priv) / 1024 / 1024);
>  
>  	return 0;
>  }
> @@ -4502,7 +4502,7 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
>  {
>  	int s_max = 3, ss_max = 4;
>  	int s, ss;
> -	u32 s_reg[s_max], eu_reg[2*s_max], eu_mask[2];
> +	u32 s_reg[s_max], eu_reg[2 * s_max], eu_mask[2];
>  
>  	/* BXT has a single slice and at most 3 subslices. */
>  	if (IS_GEN9_LP(dev_priv)) {
> @@ -4512,8 +4512,8 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
>  
>  	for (s = 0; s < s_max; s++) {
>  		s_reg[s] = I915_READ(GEN9_SLICE_PGCTL_ACK(s));
> -		eu_reg[2*s] = I915_READ(GEN9_SS01_EU_PGCTL_ACK(s));
> -		eu_reg[2*s + 1] = I915_READ(GEN9_SS23_EU_PGCTL_ACK(s));
> +		eu_reg[2 * s] = I915_READ(GEN9_SS01_EU_PGCTL_ACK(s));
> +		eu_reg[2 * s + 1] = I915_READ(GEN9_SS23_EU_PGCTL_ACK(s));
>  	}
>  
>  	eu_mask[0] = GEN9_PGCTL_SSA_EU08_ACK |
> @@ -4547,8 +4547,8 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
>  				sseu->subslice_mask |= BIT(ss);
>  			}
>  
> -			eu_cnt = 2 * hweight32(eu_reg[2*s + ss/2] &
> -					       eu_mask[ss%2]);
> +			eu_cnt = 2 * hweight32(eu_reg[2 * s + ss / 2] &
> +					       eu_mask[ss % 2]);
>  			sseu->eu_total += eu_cnt;
>  			sseu->eu_per_subslice = max_t(unsigned int,
>  						      sseu->eu_per_subslice,

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH 7/9] drm/i915: Combine substrings for a message in gen6_drpc_info()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 14:15:00 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: quoted string split across lines

Thus fix the affected source code place.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 6f3119d40c50..dbd52ea89fb4 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1529,8 +1529,8 @@ static int gen6_drpc_info(struct seq_file *m)
 
 	forcewake_count = READ_ONCE(dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count);
 	if (forcewake_count) {
-		seq_puts(m, "RC information inaccurate because somebody "
-			    "holds a forcewake reference \n");
+		seq_puts(m,
+			 "RC information inaccurate because somebody holds a forcewake reference.\n");
 	} else {
 		/* NB: we cannot use forcewake, else we read the wrong values */
 		while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_ACK) & 1))
-- 
2.12.2

Re: [PATCH 7/9] drm/i915: Combine substrings for a message in gen6_drpc_info()

[Chris Wilson]

On Thu, May 04, 2017 at 06:59:23PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 4 May 2017 14:15:00 +0200
> 
> The script "checkpatch.pl" pointed information out like the following.
> 
> WARNING: quoted string split across lines
> 
> Thus fix the affected source code place.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 6f3119d40c50..dbd52ea89fb4 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1529,8 +1529,8 @@ static int gen6_drpc_info(struct seq_file *m)
>  
>  	forcewake_count = READ_ONCE(dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count);
>  	if (forcewake_count) {
> -		seq_puts(m, "RC information inaccurate because somebody "
> -			    "holds a forcewake reference \n");
> +		seq_puts(m,
> +			 "RC information inaccurate because somebody holds a forcewake reference.\n");

And now you break the 80col rule. Blind adherence to checkpatch is
impossible.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 7/9] drm/i915: Combine substrings for a message in gen6_drpc_info()

[SF Markus Elfring]

>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1529,8 +1529,8 @@ static int gen6_drpc_info(struct seq_file *m)
>>  
>>  	forcewake_count = READ_ONCE(dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count);
>>  	if (forcewake_count) {
>> -		seq_puts(m, "RC information inaccurate because somebody "
>> -			    "holds a forcewake reference \n");
>> +		seq_puts(m,
>> +			 "RC information inaccurate because somebody holds a forcewake reference.\n");
> 
> And now you break the 80col rule. Blind adherence to checkpatch is impossible.

Have you got any other coding style preferences around the grepping
of longer message strings from such source code?

Regards,
Markus

Re: [PATCH 7/9] drm/i915: Combine substrings for a message in gen6_drpc_info()

[Chris Wilson]

On Thu, May 04, 2017 at 10:48:10PM +0200, SF Markus Elfring wrote:
> >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >> @@ -1529,8 +1529,8 @@ static int gen6_drpc_info(struct seq_file *m)
> >>  
> >>  	forcewake_count = READ_ONCE(dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count);
> >>  	if (forcewake_count) {
> >> -		seq_puts(m, "RC information inaccurate because somebody "
> >> -			    "holds a forcewake reference \n");
> >> +		seq_puts(m,
> >> +			 "RC information inaccurate because somebody holds a forcewake reference.\n");
> > 
> > And now you break the 80col rule. Blind adherence to checkpatch is impossible.
> 
> Have you got any other coding style preferences around the grepping
> of longer message strings from such source code?

I personally use long strings (because they are less hassle to write),
except when they are ridiculously long. But checkpatch complains either
way, so checkpatch itself is not a reason to make a change.

Certainly grepping for a complete seq_printf() is unlikely (i.e. you had
to open the debugfs file to see it, so you must already know where to
look in the code).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 7/9] drm/i915: Combine substrings for a message in gen6_drpc_info()

[Dan Carpenter]

On Thu, May 04, 2017 at 09:12:32PM +0100, Chris Wilson wrote:
> On Thu, May 04, 2017 at 06:59:23PM +0200, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Thu, 4 May 2017 14:15:00 +0200
> > 
> > The script "checkpatch.pl" pointed information out like the following.
> > 
> > WARNING: quoted string split across lines
> > 
> > Thus fix the affected source code place.
> > 
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 6f3119d40c50..dbd52ea89fb4 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -1529,8 +1529,8 @@ static int gen6_drpc_info(struct seq_file *m)
> >  
> >  	forcewake_count = READ_ONCE(dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count);
> >  	if (forcewake_count) {
> > -		seq_puts(m, "RC information inaccurate because somebody "
> > -			    "holds a forcewake reference \n");
> > +		seq_puts(m,
> > +			 "RC information inaccurate because somebody holds a forcewake reference.\n");
> 
> And now you break the 80col rule. Blind adherence to checkpatch is
> impossible.
> -Chris

No.  Checkpatch allows you to go over 80 characters to avoid splitting a
string.

regards,
dan carpenter

[PATCH 8/9] drm/i915: Replace a seq_puts() call by seq_putc() in two functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 14:23:32 +0200

Two single characters (line breaks) should be put into a sequence.
Thus use the corresponding function "seq_putc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 2aa6b97fd22f..9f64dc3f2d05 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1254,7 +1254,7 @@ static void gen8_dump_pdp(struct i915_hw_ppgtt *ppgtt,
 					else
 						seq_puts(m, "  SCRATCH ");
 				}
-				seq_puts(m, "\n");
+				seq_putc(m, '\n');
 			}
 			kunmap_atomic(pt_vaddr);
 		}
@@ -1437,7 +1437,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 				else
 					seq_puts(m, "  SCRATCH ");
 			}
-			seq_puts(m, "\n");
+			seq_putc(m, '\n');
 		}
 		kunmap_atomic(pt_vaddr);
 	}
-- 
2.12.2

[PATCH 9/9] drm/i915: Combine substrings for two messages in i915_ggtt_probe_hw()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 14:30:37 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: quoted string split across lines

Thus fix the affected source code place.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 9f64dc3f2d05..508431f42b65 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2905,16 +2905,14 @@ int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv)
 	}
 
 	if ((ggtt->base.total - 1) >> 32) {
-		DRM_ERROR("We never expected a Global GTT with more than 32bits"
-			  " of address space! Found %lldM!\n",
+		DRM_ERROR("We never expected a Global GTT with more than 32bits of address space! Found %lldM!\n",
 			  ggtt->base.total >> 20);
 		ggtt->base.total = 1ULL << 32;
 		ggtt->mappable_end = min(ggtt->mappable_end, ggtt->base.total);
 	}
 
 	if (ggtt->mappable_end > ggtt->base.total) {
-		DRM_ERROR("mappable aperture extends past end of GGTT,"
-			  " aperture=%llx, total=%llx\n",
+		DRM_ERROR("mappable aperture extends past end of GGTT, aperture=%llx, total=%llx\n",
 			  ggtt->mappable_end, ggtt->base.total);
 		ggtt->mappable_end = ggtt->base.total;
 	}
-- 
2.12.2