Re: [Intel-gfx] [PATCH v2 24/39] drm/i915: dvo_sil164.c: use SPDX header

[Joe Perches]

On Fri, 2022-07-15 at 17:35 -0400, Rodrigo Vivi wrote:
> On Wed, Jul 13, 2022 at 09:12:12AM +0100, Mauro Carvalho Chehab wrote:
> > This file is licensed with MIT license. Change its license text
> > to use SPDX.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> 
> Reviewed-by: Rodrigo Vivi 

Not exactly the MIT license as it's missing "or copyright holders"
> 
> > ---
> > 
> > To avoid mailbombing on a large number of people, only mailing lists were 
> > C/C on the cover.
> > See [PATCH v2 00/39] at: 
> > https://lore.kernel.org/all/cover.1657699522.git.mche...@kernel.org/
> > 
> >  drivers/gpu/drm/i915/display/dvo_sil164.c | 32 +--
> >  1 file changed, 6 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/dvo_sil164.c 
> > b/drivers/gpu/drm/i915/display/dvo_sil164.c
> > index 0dfa0a0209ff..12974f7c9dc1 100644
> > --- a/drivers/gpu/drm/i915/display/dvo_sil164.c
> > +++ b/drivers/gpu/drm/i915/display/dvo_sil164.c
> > @@ -1,30 +1,10 @@
> > -/**
> > +// SPDX-License-Identifier: MIT
> >  
> > -Copyright © 2006 Dave Airlie
> > -
> > -All Rights Reserved.
> > -
> > -Permission is hereby granted, free of charge, to any person obtaining a
> > -copy of this software and associated documentation files (the
> > -"Software"), to deal in the Software without restriction, including
> > -without limitation the rights to use, copy, modify, merge, publish,
> > -distribute, sub license, and/or sell copies of the Software, and to
> > -permit persons to whom the Software is furnished to do so, subject to
> > -the following conditions:
> > -
> > -The above copyright notice and this permission notice (including the
> > -next paragraph) shall be included in all copies or substantial portions
> > -of the Software.
> > -
> > -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> > -OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > -MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
> > -IN NO EVENT SHALL THE AUTHOR

Missing "Authors or copyright holders"

> > BE LIABLE FOR
> > -ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
> > -TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> > -SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> > -
> > -**/
> > +/*
> > + * Copyright © 2006 Dave Airlie
> > + *
> > + * All Rights Reserved.
> > + */
> >  
> >  #include "intel_display_types.h"
> >  #include "intel_dvo_dev.h"
> > -- 
> > 2.36.1
> > 

Re: [Intel-gfx] [PATCH] drm/i915: change node clearing from memset to initialization

[Joe Perches]

On Sat, 2022-04-16 at 13:48 -0700, Tom Rix wrote:
> On 4/16/22 11:33 AM, Joe Perches wrote:
> > On Sat, 2022-04-16 at 13:23 -0400, Tom Rix wrote:
> > > In insert_mappable_node(), the parameter node is
> > > cleared late in node's use with memset.
> > > insert_mappable_node() is a singleton, called only
> > > from i915_gem_gtt_prepare() which itself is only
> > > called by i915_gem_gtt_pread() and
> > > i915_gem_gtt_pwrite_fast() where the definition of
> > > node originates.
> > > 
> > > Instead of using memset, initialize node to 0 at it's
> > > definitions.
> > trivia: /it's/its/
> > 
> > Only reason _not_ to do this is memset is guaranteed to
> > zero any padding that might go to userspace.
> > 
> > But it doesn't seem there is any padding anyway nor is
> > the struct available to userspace.
> > 
> > So this seems fine though it might increase overall code
> > size a tiny bit.
> > 
> > I do have a caveat: see below:
> > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> > > b/drivers/gpu/drm/i915/i915_gem.c
> > []
> > > @@ -328,7 +327,6 @@ static struct i915_vma *i915_gem_gtt_prepare(struct 
> > > drm_i915_gem_object *obj,
> > >   goto err_ww;
> > >   } else if (!IS_ERR(vma)) {
> > >   node->start = i915_ggtt_offset(vma);
> > > - node->flags = 0;
> > Why is this unneeded?
> 
> node = {} initializes all of node's elements to 0, including this one.

true, but could the call to insert_mappable_node combined with the
retry goto in i915_gem_gtt_prepare set this to non-zero?

Re: [Intel-gfx] [PATCH] drm/i915: change node clearing from memset to initialization

[Joe Perches]

On Sat, 2022-04-16 at 13:23 -0400, Tom Rix wrote:
> In insert_mappable_node(), the parameter node is
> cleared late in node's use with memset.
> insert_mappable_node() is a singleton, called only
> from i915_gem_gtt_prepare() which itself is only
> called by i915_gem_gtt_pread() and
> i915_gem_gtt_pwrite_fast() where the definition of
> node originates.
> 
> Instead of using memset, initialize node to 0 at it's
> definitions.

trivia: /it's/its/

Only reason _not_ to do this is memset is guaranteed to
zero any padding that might go to userspace.

But it doesn't seem there is any padding anyway nor is
the struct available to userspace.

So this seems fine though it might increase overall code
size a tiny bit.

I do have a caveat: see below:

> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
[]
> @@ -328,7 +327,6 @@ static struct i915_vma *i915_gem_gtt_prepare(struct 
> drm_i915_gem_object *obj,
>   goto err_ww;
>   } else if (!IS_ERR(vma)) {
>   node->start = i915_ggtt_offset(vma);
> - node->flags = 0;

Why is this unneeded?

from: drm_mm_insert_node_in_range which can set node->flags

__set_bit(DRM_MM_NODE_ALLOCATED_BIT, >flags);

Re: [Intel-gfx] [PATCH 1/6] drivers: usb: remove usage of list iterator past the loop body

[Joe Perches]

On Mon, 2022-02-28 at 14:24 +0300, Dan Carpenter wrote:

> a multi-line indent gets curly braces for readability even though
> it's not required by C.  And then both sides would get curly braces.

That's more your personal preference than a coding style guideline.

Re: [Intel-gfx] [PATCH 1/3] lib/string_helpers: Consolidate yesno() implementation

[Joe Perches]

On Wed, 2022-01-19 at 16:00 -0500, Steven Rostedt wrote:
> On Wed, 19 Jan 2022 21:25:08 +0200
> Andy Shevchenko  wrote:
> 
> > > I say keep it one line!
> > > 
> > > Reviewed-by: Steven Rostedt (Google)   
> > 
> > I believe Sakari strongly follows the 80 rule, which means...
> 
> Checkpatch says "100" I think we need to simply update the docs (the
> documentation always lags the code ;-)

checkpatch doesn't say anything normally, it's a stupid script.
It just mindlessly bleats a message when a line exceeds 100 chars...

Just fyi: I think it's nicer on a single line too.

Re: [Intel-gfx] [PATCH 2/4] amdgpu_ucode: reduce number of pr_debug calls

[Joe Perches]

On Wed, 2021-09-29 at 19:44 -0600, Jim Cromie wrote:
> There are blocks of DRM_DEBUG calls, consolidate their args into
> single calls.  With dynamic-debug in use, each callsite consumes 56
> bytes of callsite data, and this patch removes about 65 calls, so
> it saves ~3.5kb.
> 
> no functional changes.

No functional change, but an output logging content change.

> RFC: this creates multi-line log messages, does that break any syslog
> conventions ?

It does change the output as each individual DRM_DEBUG is a call to
__drm_dbg which is effectively:

printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
   __builtin_return_address(0), );


> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
[]
> @@ -30,17 +30,26 @@
>  
> 
>  static void amdgpu_ucode_print_common_hdr(const struct 
> common_firmware_header *hdr)
>  {
> - DRM_DEBUG("size_bytes: %u\n", le32_to_cpu(hdr->size_bytes));
> - DRM_DEBUG("header_size_bytes: %u\n", 
> le32_to_cpu(hdr->header_size_bytes));
[]
> + DRM_DEBUG("size_bytes: %u\n"
> +   "header_size_bytes: %u\n"

etc...

Re: [Intel-gfx] [RFC PATCH v2 1/4] drm_print.h: rewrap __DRM_DEFINE_DBG_RATELIMITED macro

[Joe Perches]

On Sat, 2021-07-10 at 23:49 -0600, Jim Cromie wrote:
> whitespace only, to diff-minimize a later commit.
> no functional changes
[]
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
[]
> @@ -524,19 +524,24 @@ void __drm_err(const char *format, ...);
>  #define DRM_DEBUG_DP(fmt, ...)   
> \
>   __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
>  
> 
> -#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...)
> \
> -({   
> \
> - static DEFINE_RATELIMIT_STATE(rs_, DEFAULT_RATELIMIT_INTERVAL, 
> DEFAULT_RATELIMIT_BURST);\
> - const struct drm_device *drm_ = (drm);  
> \
> - 
> \
> - if (drm_debug_enabled(DRM_UT_ ## category) && __ratelimit(_))
> \
> - drm_dev_printk(drm_ ? drm_->dev : NULL, KERN_DEBUG, fmt, ## 
> __VA_ARGS__);   \
> +#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...)
> \
> +({   \
> + static DEFINE_RATELIMIT_STATE(rs_,  \
> +   DEFAULT_RATELIMIT_INTERVAL,   \
> +   DEFAULT_RATELIMIT_BURST); \
> + const struct drm_device *drm_ = (drm);  \
> + \
> + if (drm_debug_enabled(DRM_UT_ ## category)  \
> + && __ratelimit(_))   \

Though I don't really see the need for the change, the typical style
has the logical continuation at the end of the test.

if (drm_debug_enabled(DRM_UT_ ## category) &&   \
__ratelimit(_))  \


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH][next] drm/i915/gem: Fix fall-through warning for Clang

[Joe Perches]

On Mon, 2021-06-07 at 15:32 -0500, Gustavo A. R. Silva wrote:
> In preparation to enable -Wimplicit-fallthrough for Clang, fix a
> warning by explicitly adding a fallthrough; statement.
[]
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c 
> b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
[]
> @@ -62,6 +62,7 @@ static void try_to_writeback(struct drm_i915_gem_object 
> *obj,
>   switch (obj->mm.madv) {
>   case I915_MADV_DONTNEED:
>   i915_gem_object_truncate(obj);
> + fallthrough;
>   case __I915_MADV_PURGED:
>   return;
>   }

I think fallthrough to return is not particularly nice to follow.

This is the current function:

static void try_to_writeback(struct drm_i915_gem_object *obj,
 unsigned int flags)
{
switch (obj->mm.madv) {
case I915_MADV_DONTNEED:
i915_gem_object_truncate(obj);
case __I915_MADV_PURGED:
return;
}

if (flags & I915_SHRINK_WRITEBACK)
i915_gem_object_writeback(obj);
}

One of these might be more typical:

static void try_to_writeback(struct drm_i915_gem_object *obj,
 unsigned int flags)
{
switch (obj->mm.madv) {
case I915_MADV_DONTNEED:
i915_gem_object_truncate(obj);
break;
case __I915_MADV_PURGED:
break;
default:
if (flags & I915_SHRINK_WRITEBACK)
i915_gem_object_writeback(obj);
break;
}
}

or maybe:

static void try_to_writeback(struct drm_i915_gem_object *obj,
 unsigned int flags)
{
switch (obj->mm.madv) {
case I915_MADV_DONTNEED:
i915_gem_object_truncate(obj);
return;
case __I915_MADV_PURGED:
return;
}

if (flags & I915_SHRINK_WRITEBACK)
i915_gem_object_writeback(obj);
}


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/29] drm/i915: Avoid comma separated statements

[Joe Perches]

On Mon, 2020-08-24 at 21:56 -0700, Joe Perches wrote:
> Use semicolons and braces.

Ping?

> Signed-off-by: Joe Perches 
> ---
>  drivers/gpu/drm/i915/gt/gen8_ppgtt.c   | 8 +---
>  drivers/gpu/drm/i915/gt/intel_gt_requests.c| 6 --
>  drivers/gpu/drm/i915/gt/selftest_workarounds.c | 6 --
>  drivers/gpu/drm/i915/intel_runtime_pm.c| 6 --
>  4 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c 
> b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
> index 699125928272..114c13285ff1 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
> @@ -323,10 +323,12 @@ static int __gen8_ppgtt_alloc(struct i915_address_space 
> * const vm,
>   }
>  
> 
>   spin_lock(>lock);
> - if (likely(!pd->entry[idx]))
> + if (likely(!pd->entry[idx])) {
>   set_pd_entry(pd, idx, pt);
> - else
> - alloc = pt, pt = pd->entry[idx];
> + } else {
> + alloc = pt;
> + pt = pd->entry[idx];
> + }
>   }
>  
> 
>   if (lvl) {
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c 
> b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
> index 66fcbf9d0fdd..54408d0b5e6e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
> @@ -139,8 +139,10 @@ long intel_gt_retire_requests_timeout(struct intel_gt 
> *gt, long timeout)
>   LIST_HEAD(free);
>  
> 
>   interruptible = true;
> - if (unlikely(timeout < 0))
> - timeout = -timeout, interruptible = false;
> + if (unlikely(timeout < 0)) {
> + timeout = -timeout;
> + interruptible = false;
> + }
>  
> 
>   flush_submission(gt, timeout); /* kick the ksoftirqd tasklets */
>   spin_lock(>lock);
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c 
> b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index febc9e6692ba..3e4cbeed20bd 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -521,8 +521,10 @@ static int check_dirty_whitelist(struct intel_context 
> *ce)
>  
> 
>   srm = MI_STORE_REGISTER_MEM;
>   lrm = MI_LOAD_REGISTER_MEM;
> - if (INTEL_GEN(engine->i915) >= 8)
> - lrm++, srm++;
> + if (INTEL_GEN(engine->i915) >= 8) {
> + lrm++;
> + srm++;
> + }
>  
> 
>   pr_debug("%s: Writing garbage to %x\n",
>    engine->name, reg);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 153ca9e65382..f498f1c80755 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -201,8 +201,10 @@ __print_intel_runtime_pm_wakeref(struct drm_printer *p,
>   unsigned long rep;
>  
> 
>   rep = 1;
> - while (i + 1 < dbg->count && dbg->owners[i + 1] == stack)
> - rep++, i++;
> + while (i + 1 < dbg->count && dbg->owners[i + 1] == stack) {
> + rep++;
> + i++;
> + }
>   __print_depot_stack(stack, buf, PAGE_SIZE, 2);
>   drm_printf(p, "Wakeref x%lu taken at:\n%s", rep, buf);
>   }


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 000/141] Fix fall-through warnings for Clang

[Joe Perches]

On Tue, 2020-11-24 at 11:58 +1100, Finn Thain wrote:
> it's not for me to prove that such patches don't affect code 
> generation. That's for the patch author and (unfortunately) for reviewers.

Ideally, that proof would be provided by the compilation system itself
and not patch authors nor reviewers nor maintainers.

Unfortunately gcc does not guarantee repeatability or deterministic output.
To my knowledge, neither does clang.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 000/141] Fix fall-through warnings for Clang

[Joe Perches]

On Mon, 2020-11-23 at 07:58 -0800, James Bottomley wrote:
> We're also complaining about the inability to recruit maintainers:
> 
> https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/
> 
> And burn out:
> 
> http://antirez.com/news/129

https://www.wired.com/story/open-source-coders-few-tired/

> What I'm actually trying to articulate is a way of measuring value of
> the patch vs cost ... it has nothing really to do with who foots the
> actual bill.

It's unclear how to measure value in consistency.

But one way that costs can be reduced is by automation and _not_
involving maintainers when the patch itself is provably correct.

> One thesis I'm actually starting to formulate is that this continual
> devaluing of maintainers is why we have so much difficulty keeping and
> recruiting them.

The linux kernel has something like 1500 different maintainers listed
in the MAINTAINERS file.  That's not a trivial number.

$ git grep '^M:' MAINTAINERS | sort | uniq -c | wc -l
1543
$ git grep '^M:' MAINTAINERS| cut -f1 -d'<' | sort | uniq -c | wc -l
1446

I think the question you are asking is about trust and how it
effects development.

And back to that wired story, the actual number of what you might
be considering to be maintainers is likely less than 10% of the
listed numbers above.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC] MAINTAINERS tag for cleanup robot

[Joe Perches]

On Mon, 2020-11-23 at 09:33 +1100, Finn Thain wrote:
> On Sun, 22 Nov 2020, Joe Perches wrote:

> > But provably correct conversions IMO _should_ be done and IMO churn 
> > considerations should generally have less importance.
[]
> Moreover, the patch review workload for skilled humans is being generated 
> by the automation, which is completely backwards: the machine is supposed 
> to be helping.

Which is why the provably correct matters.

coccinelle transforms can be, but are not necessarily, provably correct.

The _show transforms done via the sysfs_emit_dev.cocci script are correct
as in commit aa838896d87a ("drivers core: Use sysfs_emit and sysfs_emit_at
for show(device *...) functions")

Worthwhile?  A different question, but I think yes as it reduces the
overall question space of the existing other sprintf overrun possibilities.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 000/141] Fix fall-through warnings for Clang

[Joe Perches]

On Sun, 2020-11-22 at 11:12 -0800, James Bottomley wrote:
> On Sun, 2020-11-22 at 10:25 -0800, Joe Perches wrote:
> > On Sun, 2020-11-22 at 10:21 -0800, James Bottomley wrote:
> > > Please tell me our reward for all this effort isn't a single
> > > missing error print.
> > 
> > There were quite literally dozens of logical defects found
> > by the fallthrough additions.  Very few were logging only.
> 
> So can you give us the best examples (or indeed all of them if someone
> is keeping score)?  hopefully this isn't a US election situation ...

Gustavo?  Are you running for congress now?

https://lwn.net/Articles/794944/


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 000/141] Fix fall-through warnings for Clang

[Joe Perches]

On Sun, 2020-11-22 at 10:21 -0800, James Bottomley wrote:
> Please tell me
> our reward for all this effort isn't a single missing error print.

There were quite literally dozens of logical defects found
by the fallthrough additions.  Very few were logging only.



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC] MAINTAINERS tag for cleanup robot

[Joe Perches]

On Sun, 2020-11-22 at 08:49 -0800, James Bottomley wrote:
> We can enforce sysfs_emit going forwards
> using tools like checkpatch

It's not really possible for checkpatch to find or warn about
sysfs uses of sprintf. checkpatch is really just a trivial
line-by-line parser and it has no concept of code intent.

It just can't warn on every use of the sprintf family.
There are just too many perfectly valid uses.

> but there's no benefit and a lot of harm to
> be done by trying to churn the entire tree

Single uses of sprintf for sysfs is not really any problem.

But likely there are still several possible overrun sprintf/snprintf
paths in sysfs.  Some of them are very obscure and unlikely to be
found by a robot as the logic for sysfs buf uses can be fairly twisty.

But provably correct conversions IMO _should_ be done and IMO churn
considerations should generally have less importance.



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC] MAINTAINERS tag for cleanup robot

[Joe Perches]

On Sun, 2020-11-22 at 08:33 -0800, Tom Rix wrote:
> On 11/21/20 9:10 AM, Joe Perches wrote:
> > On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
> > > A difficult part of automating commits is composing the subsystem
> > > preamble in the commit log.  For the ongoing effort of a fixer producing
> > > one or two fixes a release the use of 'treewide:' does not seem 
> > > appropriate.
> > > 
> > > It would be better if the normal prefix was used.  Unfortunately normal is
> > > not consistent across the tree.
> > > 
> > > So I am looking for comments for adding a new tag to the MAINTAINERS file
> > > 
> > >   D: Commit subsystem prefix
> > > 
> > > ex/ for FPGA DFL DRIVERS
> > > 
> > >   D: fpga: dfl:
> > I'm all for it.  Good luck with the effort.  It's not completely trivial.
> > 
> > From a decade ago:
> > 
> > https://lore.kernel.org/lkml/1289919077.28741.50.camel@Joe-Laptop/
> > 
> > (and that thread started with extra semicolon patches too)
> 
> Reading the history, how about this.
> 
> get_maintainer.pl outputs a single prefix, if multiple files have the
> same prefix it works, if they don't its an error.
> 
> Another script 'commit_one_file.sh' does the call to get_mainainter.pl
> to get the prefix and be called by run-clang-tools.py to get the fixer
> specific message.

It's not whether the script used is get_maintainer or any other script,
the question is really if the MAINTAINERS file is the appropriate place
to store per-subsystem patch specific prefixes.

It is.

Then the question should be how are the forms described and what is the
inheritance priority.  My preference would be to have a default of
inherit the parent base and add basename(subsystem dirname).

Commit history seems to have standardized on using colons as the separator
between the commit prefix and the subject.

A good mechanism to explore how various subsystems have uses prefixes in
the past might be something like:

$ git log --no-merges --pretty='%s' -  | \
  perl -n -e 'print substr($_, 0, rindex($_, ":") + 1) . "\n";' | \
  sort | uniq -c | sort -rn

Using 1 for commit_count and drivers/scsi for subsystem_path, the
top 40 entries are below:

About 1% don't have a colon, and there is no real consistency even
within individual drivers below scsi.  For instance, qla2xxx:

 1  814 scsi: qla2xxx:
 2  691 scsi: lpfc:
 3  389 scsi: hisi_sas:
 4  354 scsi: ufs:
 5  339 scsi:
 6  291 qla2xxx:
 7  256 scsi: megaraid_sas:
 8  249 scsi: mpt3sas:
 9  200 hpsa:
10  190 scsi: aacraid:
11  174 lpfc:
12  153 scsi: qedf:
13  144 scsi: smartpqi:
14  139 scsi: cxlflash:
15  122 scsi: core:
16  110 [SCSI] qla2xxx:
17  108 ncr5380:
18   98 scsi: hpsa:
19   97 
20   89 treewide:
21   88 mpt3sas:
22   86 scsi: libfc:
23   85 scsi: qedi:
24   84 scsi: be2iscsi:
25   81 [SCSI] qla4xxx:
26   81 hisi_sas:
27   81 block:
28   75 megaraid_sas:
29   71 scsi: sd:
30   69 [SCSI] hpsa:
31   68 cxlflash:
32   65 scsi: libsas:
33   65 scsi: fnic:
34   61 scsi: scsi_debug:
35   60 scsi: arcmsr:
36   57 be2iscsi:
37   53 atp870u:
38   51 scsi: bfa:
39   50 scsi: storvsc:
40   48 sd:


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC] MAINTAINERS tag for cleanup robot

[Joe Perches]

On Sat, 2020-11-21 at 09:18 -0800, James Bottomley wrote:
> On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
> > A difficult part of automating commits is composing the subsystem
> > preamble in the commit log.  For the ongoing effort of a fixer
> > producing one or two fixes a release the use of 'treewide:' does
> > not seem appropriate.
> > 
> > It would be better if the normal prefix was used.  Unfortunately
> > normal is not consistent across the tree.
> > 
> > D: Commit subsystem prefix
> > 
> > ex/ for FPGA DFL DRIVERS
> > 
> > D: fpga: dfl:
> 
> I've got to bet this is going to cause more issues than it solves. 
> SCSI uses scsi: : for drivers but not every driver has a
> MAINTAINERS entry.  We use either scsi: or scsi: core: for mid layer
> things, but we're not consistent.  Block uses blk-: for all
> of it's stuff but almost no s have a MAINTAINERS entry.  So
> the next thing you're going to cause is an explosion of suggested
> MAINTAINERs entries.

As well as some changes require simultaneous changes across
multiple subsystems.

> Has anyone actually complained about treewide:?

It depends on what you mean by treewide:

If a treewide: patch is applied by some "higher level" maintainer,
then generally, no.

If the treewide patch is also cc'd to many individual maintainers,
then yes, many many times.

Mostly because patches cause what is in their view churn or that
changes are not specific to their subsystem grounds.

The treewide patch is sometimes dropped, sometimes broken up and
generally not completely applied.

What would be useful in many cases like this is for a pre and post
application of the treewide patch to be compiled and the object
code verified for lack of any logic change.

Unfortunately, gcc does not guarantee deterministic compilation so
this isn't feasible with at least gcc.  Does clang guarantee this?

I'm not sure it's possible:
https://blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC] MAINTAINERS tag for cleanup robot

[Joe Perches]

On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
> A difficult part of automating commits is composing the subsystem
> preamble in the commit log.  For the ongoing effort of a fixer producing
> one or two fixes a release the use of 'treewide:' does not seem appropriate.
> 
> It would be better if the normal prefix was used.  Unfortunately normal is
> not consistent across the tree.
> 
> So I am looking for comments for adding a new tag to the MAINTAINERS file
> 
>   D: Commit subsystem prefix
> 
> ex/ for FPGA DFL DRIVERS
> 
>   D: fpga: dfl:

I'm all for it.  Good luck with the effort.  It's not completely trivial.

>From a decade ago:

https://lore.kernel.org/lkml/1289919077.28741.50.camel@Joe-Laptop/

(and that thread started with extra semicolon patches too)

> Continuing with cleaning up clang's -Wextra-semi-stmt

> diff --git a/Makefile b/Makefile
[]
> @@ -1567,20 +1567,21 @@ help:
>    echo  ''
>   @echo  'Static analysers:'
>   @echo  '  checkstack  - Generate a list of stack hogs'
>   @echo  '  versioncheck- Sanity check on version.h usage'
>   @echo  '  includecheck- Check for duplicate included header files'
>   @echo  '  export_report   - List the usages of all exported symbols'
>   @echo  '  headerdep   - Detect inclusion cycles in headers'
>   @echo  '  coccicheck  - Check with Coccinelle'
>   @echo  '  clang-analyzer  - Check with clang static analyzer'
>   @echo  '  clang-tidy  - Check with clang-tidy'
> + @echo  '  clang-tidy-fix  - Check and fix with clang-tidy'

A pity the ordering of the code below isn't the same as the above.

> -PHONY += clang-tidy clang-analyzer
> +PHONY += clang-tidy-fix clang-tidy clang-analyzer
[]
> -clang-tidy clang-analyzer: $(extmod-prefix)compile_commands.json
> +clang-tidy-fix clang-tidy clang-analyzer: 
> $(extmod-prefix)compile_commands.json
>   $(call cmd,clang_tools)
>  else
> -clang-tidy clang-analyzer:
> +clang-tidy-fix clang-tidy clang-analyzer:

[]

> diff --git a/scripts/clang-tools/run-clang-tools.py 
> b/scripts/clang-tools/run-clang-tools.py
[]
> @@ -22,43 +22,57 @@ def parse_arguments():
[]
>  parser.add_argument("type",
> -choices=["clang-tidy", "clang-analyzer"],
> +choices=["clang-tidy-fix", "clang-tidy", 
> "clang-analyzer"],

etc...

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 000/141] Fix fall-through warnings for Clang

[Joe Perches]

On Fri, 2020-11-20 at 12:21 -0600, Gustavo A. R. Silva wrote:
> Hi all,
> 
> This series aims to fix almost all remaining fall-through warnings in
> order to enable -Wimplicit-fallthrough for Clang.
> 
> In preparation to enable -Wimplicit-fallthrough for Clang, explicitly
> add multiple break/goto/return/fallthrough statements instead of just
> letting the code fall through to the next case.
> 
> Notice that in order to enable -Wimplicit-fallthrough for Clang, this
> change[1] is meant to be reverted at some point. So, this patch helps
> to move in that direction.

This was a bit hard to parse for a second or three.

Thanks Gustavo.

How was this change done?


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [trivial PATCH] treewide: Convert switch/case fallthrough; to break;

[Joe Perches]

On Thu, 2020-09-10 at 15:21 +0100, Robin Murphy wrote:
> On 2020-09-09 21:06, Joe Perches wrote:
> > fallthrough to a separate case/default label break; isn't very readable.
> > 
> > Convert pseudo-keyword fallthrough; statements to a simple break; when
> > the next label is case or default and the only statement in the next
> > label block is break;
> > 
> > Found using:
> > 
> > $ grep-2.5.4 -rP --include=*.[ch] -n 
> > "fallthrough;(\s*(case\s+\w+|default)\s*:\s*){1,7}break;" *
> > 
> > Miscellanea:
> > 
> > o Move or coalesce a couple label blocks above a default: block.
> > 
> > Signed-off-by: Joe Perches 
> > ---
> > 
> > Compiled allyesconfig x86-64 only.
> > A few files for other arches were not compiled.
> > 
> 
> [...]
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
> > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index c192544e874b..743db1abec40 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -3777,7 +3777,7 @@ static int arm_smmu_device_hw_probe(struct 
> > arm_smmu_device *smmu)
> > switch (FIELD_GET(IDR0_TTF, reg)) {
> > case IDR0_TTF_AARCH32_64:
> > smmu->ias = 40;
> > -   fallthrough;
> > +   break;
> > case IDR0_TTF_AARCH64:
> > break;
> > default:
> 
> I have to say I don't really agree with the readability argument for 
> this one - a fallthrough is semantically correct here, since the first 
> case is a superset of the second. It just happens that anything we would 
> do for the common subset is implicitly assumed (there are other 
> potential cases we simply haven't added support for at the moment), thus 
> the second case is currently empty.
> This change actively obfuscates that distinction.

Then perhaps comments should be added to usefully
describe the mechanisms.

case IDR0_TTF_AARCH32_64:
smmu->ias = 40;
fallthrough;/* and still do the 64 bit processing */
case IDR0_TTF_AARCH64:
/* Nothing specific yet */
break;

> Robin.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [trivial PATCH] treewide: Convert switch/case fallthrough; to break;

[Joe Perches]

On Wed, 2020-09-09 at 19:36 -0300, Jason Gunthorpe wrote:
> On Wed, Sep 09, 2020 at 01:06:39PM -0700, Joe Perches wrote:
> > fallthrough to a separate case/default label break; isn't very readable.
> > 
> > Convert pseudo-keyword fallthrough; statements to a simple break; when
> > the next label is case or default and the only statement in the next
> > label block is break;
> > 
> > Found using:
> > 
> > $ grep-2.5.4 -rP --include=*.[ch] -n 
> > "fallthrough;(\s*(case\s+\w+|default)\s*:\s*){1,7}break;" *
> > 
> > Miscellanea:
> > 
> > o Move or coalesce a couple label blocks above a default: block.
> > 
> > Signed-off-by: Joe Perches 
> > ---
> > 
> > Compiled allyesconfig x86-64 only.
> > A few files for other arches were not compiled.
> 
> IB part looks OK, I prefer it like this
> 
> You could do the same for continue as well, I saw a few of those..

I saw some continue uses as well but wasn't sure
and didn't look to see if the switch/case with
continue was in a for/while loop.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [trivial PATCH] treewide: Convert switch/case fallthrough; to break;

[Joe Perches]

fallthrough to a separate case/default label break; isn't very readable.

Convert pseudo-keyword fallthrough; statements to a simple break; when
the next label is case or default and the only statement in the next
label block is break;

Found using:

$ grep-2.5.4 -rP --include=*.[ch] -n 
"fallthrough;(\s*(case\s+\w+|default)\s*:\s*){1,7}break;" *

Miscellanea:

o Move or coalesce a couple label blocks above a default: block.

Signed-off-by: Joe Perches 
---

Compiled allyesconfig x86-64 only.
A few files for other arches were not compiled.

 arch/arm/mach-mmp/pm-pxa910.c |  2 +-
 arch/arm64/kvm/handle_exit.c  |  2 +-
 arch/mips/kernel/cpu-probe.c  |  2 +-
 arch/mips/math-emu/cp1emu.c   |  2 +-
 arch/s390/pci/pci.c   |  2 +-
 crypto/tcrypt.c   |  4 ++--
 drivers/ata/sata_mv.c |  2 +-
 drivers/atm/lanai.c   |  2 +-
 drivers/gpu/drm/i915/display/intel_sprite.c   |  2 +-
 drivers/gpu/drm/nouveau/nvkm/engine/disp/hdmi.c   |  2 +-
 drivers/hid/wacom_wac.c   |  2 +-
 drivers/i2c/busses/i2c-i801.c |  2 +-
 drivers/infiniband/ulp/rtrs/rtrs-clt.c| 14 +++---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c|  6 +++---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  2 +-
 drivers/irqchip/irq-vic.c |  4 ++--
 drivers/md/dm.c   |  2 +-
 drivers/media/dvb-frontends/drxd_hard.c   |  2 +-
 drivers/media/i2c/ov5640.c|  2 +-
 drivers/media/i2c/ov6650.c|  5 ++---
 drivers/media/i2c/smiapp/smiapp-core.c|  2 +-
 drivers/media/i2c/tvp5150.c   |  2 +-
 drivers/media/pci/ddbridge/ddbridge-core.c|  2 +-
 drivers/media/usb/cpia2/cpia2_core.c  |  2 +-
 drivers/mfd/iqs62x.c  |  3 +--
 drivers/mmc/host/atmel-mci.c  |  2 +-
 drivers/mtd/nand/raw/nandsim.c|  2 +-
 drivers/net/ethernet/intel/e1000e/phy.c   |  2 +-
 drivers/net/ethernet/intel/fm10k/fm10k_pf.c   |  2 +-
 drivers/net/ethernet/intel/i40e/i40e_adminq.c |  2 +-
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |  2 +-
 drivers/net/ethernet/intel/iavf/iavf_txrx.c   |  2 +-
 drivers/net/ethernet/intel/igb/e1000_phy.c|  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c|  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c|  2 +-
 drivers/net/ethernet/intel/ixgbevf/vf.c   |  2 +-
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c |  2 +-
 drivers/net/ethernet/qlogic/qed/qed_mcp.c |  2 +-
 drivers/net/ethernet/sfc/falcon/farch.c   |  2 +-
 drivers/net/ethernet/sfc/farch.c  |  2 +-
 drivers/net/phy/adin.c|  3 +--
 drivers/net/usb/pegasus.c |  4 ++--
 drivers/net/usb/usbnet.c  |  2 +-
 drivers/net/wireless/ath/ath5k/eeprom.c   |  2 +-
 drivers/net/wireless/mediatek/mt7601u/dma.c   |  8 
 drivers/nvme/host/core.c  | 12 ++--
 drivers/pcmcia/db1xxx_ss.c|  4 ++--
 drivers/power/supply/abx500_chargalg.c|  2 +-
 drivers/power/supply/charger-manager.c|  2 +-
 drivers/rtc/rtc-pcf85063.c|  2 +-
 drivers/s390/scsi/zfcp_fsf.c  |  2 +-
 drivers/scsi/aic7xxx/aic79xx_core.c   |  4 ++--
 drivers/scsi/aic94xx/aic94xx_tmf.c|  2 +-
 drivers/scsi/lpfc/lpfc_sli.c  |  2 +-
 drivers/scsi/smartpqi/smartpqi_init.c |  2 +-
 drivers/scsi/sr.c |  2 +-
 drivers/tty/serial/sunsu.c|  2 +-
 drivers/tty/serial/sunzilog.c |  2 +-
 drivers/tty/vt/vt_ioctl.c |  2 +-
 drivers/usb/dwc3/core.c   |  2 +-
 drivers/usb/gadget/legacy/inode.c |  2 +-
 drivers/usb/gadget/udc/pxa25x_udc.c   |  4 ++--
 drivers/usb/host/ohci-hcd.c   |  2 +-
 drivers/usb/isp1760/isp1760-hcd.c |  2 +-
 drivers/usb/musb/

[Intel-gfx] [PATCH 10/29] drm/i915: Avoid comma separated statements

[Joe Perches]

Use semicolons and braces.

Signed-off-by: Joe Perches 
---
 drivers/gpu/drm/i915/gt/gen8_ppgtt.c   | 8 +---
 drivers/gpu/drm/i915/gt/intel_gt_requests.c| 6 --
 drivers/gpu/drm/i915/gt/selftest_workarounds.c | 6 --
 drivers/gpu/drm/i915/intel_runtime_pm.c| 6 --
 4 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c 
b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
index 699125928272..114c13285ff1 100644
--- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
@@ -323,10 +323,12 @@ static int __gen8_ppgtt_alloc(struct i915_address_space * 
const vm,
}
 
spin_lock(>lock);
-   if (likely(!pd->entry[idx]))
+   if (likely(!pd->entry[idx])) {
set_pd_entry(pd, idx, pt);
-   else
-   alloc = pt, pt = pd->entry[idx];
+   } else {
+   alloc = pt;
+   pt = pd->entry[idx];
+   }
}
 
if (lvl) {
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c 
b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
index 66fcbf9d0fdd..54408d0b5e6e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
@@ -139,8 +139,10 @@ long intel_gt_retire_requests_timeout(struct intel_gt *gt, 
long timeout)
LIST_HEAD(free);
 
interruptible = true;
-   if (unlikely(timeout < 0))
-   timeout = -timeout, interruptible = false;
+   if (unlikely(timeout < 0)) {
+   timeout = -timeout;
+   interruptible = false;
+   }
 
flush_submission(gt, timeout); /* kick the ksoftirqd tasklets */
spin_lock(>lock);
diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c 
b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
index febc9e6692ba..3e4cbeed20bd 100644
--- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
@@ -521,8 +521,10 @@ static int check_dirty_whitelist(struct intel_context *ce)
 
srm = MI_STORE_REGISTER_MEM;
lrm = MI_LOAD_REGISTER_MEM;
-   if (INTEL_GEN(engine->i915) >= 8)
-   lrm++, srm++;
+   if (INTEL_GEN(engine->i915) >= 8) {
+   lrm++;
+   srm++;
+   }
 
pr_debug("%s: Writing garbage to %x\n",
 engine->name, reg);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 153ca9e65382..f498f1c80755 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -201,8 +201,10 @@ __print_intel_runtime_pm_wakeref(struct drm_printer *p,
unsigned long rep;
 
rep = 1;
-   while (i + 1 < dbg->count && dbg->owners[i + 1] == stack)
-   rep++, i++;
+   while (i + 1 < dbg->count && dbg->owners[i + 1] == stack) {
+   rep++;
+   i++;
+   }
__print_depot_stack(stack, buf, PAGE_SIZE, 2);
drm_printf(p, "Wakeref x%lu taken at:\n%s", rep, buf);
}
-- 
2.26.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 00/29] treewide: Convert comma separated statements

[Joe Perches]

There are many comma separated statements in the kernel.
See:https://lore.kernel.org/lkml/alpine.DEB.2.22.394.2008201856110.2524@hadrien/

Convert the comma separated statements that are in if/do/while blocks
to use braces and semicolons.

Many comma separated statements still exist but those are changes for
another day.

Joe Perches (29):
  coding-style.rst: Avoid comma statements
  alpha: Avoid comma separated statements
  ia64: Avoid comma separated statements
  sparc: Avoid comma separated statements
  ata: Avoid comma separated statements
  drbd: Avoid comma separated statements
  lp: Avoid comma separated statements
  dma-buf: Avoid comma separated statements
  drm/gma500: Avoid comma separated statements
  drm/i915: Avoid comma separated statements
  hwmon: (scmi-hwmon): Avoid comma separated statements
  Input: MT - Avoid comma separated statements
  bcache: Avoid comma separated statements
  media: Avoid comma separated statements
  mtd: Avoid comma separated statements
  8390: Avoid comma separated statements
  fs_enet: Avoid comma separated statements
  wan: sbni: Avoid comma separated statements
  s390/tty3270: Avoid comma separated statements
  scai/arm: Avoid comma separated statements
  media: atomisp: Avoid comma separated statements
  video: fbdev: Avoid comma separated statements
  fuse: Avoid comma separated statements
  reiserfs: Avoid comma separated statements
  lib/zlib: Avoid comma separated statements
  lib: zstd: Avoid comma separated statements
  ipv6: fib6: Avoid comma separated statements
  sunrpc: Avoid comma separated statements
  tools: Avoid comma separated statements

 Documentation/process/coding-style.rst|  17 +
 arch/alpha/kernel/pci_iommu.c |   8 +-
 arch/alpha/oprofile/op_model_ev4.c|  22 +-
 arch/alpha/oprofile/op_model_ev5.c|   8 +-
 arch/ia64/kernel/smpboot.c|   7 +-
 arch/sparc/kernel/smp_64.c|   7 +-
 drivers/ata/pata_icside.c |  21 +-
 drivers/block/drbd/drbd_receiver.c|   6 +-
 drivers/char/lp.c |   6 +-
 drivers/dma-buf/st-dma-fence.c|   7 +-
 drivers/gpu/drm/gma500/mdfld_intel_display.c  |  44 ++-
 drivers/gpu/drm/i915/gt/gen8_ppgtt.c  |   8 +-
 drivers/gpu/drm/i915/gt/intel_gt_requests.c   |   6 +-
 .../gpu/drm/i915/gt/selftest_workarounds.c|   6 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c   |   6 +-
 drivers/hwmon/scmi-hwmon.c|   6 +-
 drivers/input/input-mt.c  |  11 +-
 drivers/md/bcache/bset.c  |  12 +-
 drivers/md/bcache/sysfs.c |   6 +-
 drivers/media/i2c/msp3400-kthreads.c  |  12 +-
 drivers/media/pci/bt8xx/bttv-cards.c  |   6 +-
 drivers/media/pci/saa7134/saa7134-video.c |   7 +-
 drivers/mtd/devices/lart.c|  10 +-
 drivers/net/ethernet/8390/axnet_cs.c  |  19 +-
 drivers/net/ethernet/8390/lib8390.c   |  14 +-
 drivers/net/ethernet/8390/pcnet_cs.c  |   6 +-
 .../ethernet/freescale/fs_enet/fs_enet-main.c |  11 +-
 drivers/net/wan/sbni.c| 101 +++---
 drivers/s390/char/tty3270.c   |   6 +-
 drivers/scsi/arm/cumana_2.c   |  19 +-
 drivers/scsi/arm/eesox.c  |   9 +-
 drivers/scsi/arm/powertec.c   |   9 +-
 .../media/atomisp/pci/atomisp_subdev.c|   6 +-
 drivers/video/fbdev/tgafb.c   |  12 +-
 fs/fuse/dir.c |  24 +-
 fs/reiserfs/fix_node.c|  36 ++-
 lib/zlib_deflate/deftree.c|  49 ++-
 lib/zstd/compress.c   | 120 ---
 lib/zstd/fse_compress.c   |  24 +-
 lib/zstd/huf_compress.c   |   6 +-
 net/ipv6/ip6_fib.c|  12 +-
 net/sunrpc/sysctl.c   |   6 +-
 tools/lib/subcmd/help.c   |  10 +-
 tools/power/cpupower/utils/cpufreq-set.c  |  14 +-
 tools/testing/selftests/vm/gup_benchmark.c|  18 +-
 tools/testing/selftests/vm/userfaultfd.c  | 296 +++---
 46 files changed, 694 insertions(+), 382 deletions(-)

-- 
2.26.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gvt: Use ARRAY_SIZE instead of hardcoded size (rev2)

[Joe Perches]

On Tue, 2020-04-14 at 20:30 +, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/gvt: Use ARRAY_SIZE instead of hardcoded size (rev2)
> URL   : https://patchwork.freedesktop.org/series/75888/
> State : warning

This seems an odd message to receive as I was simply
making suggestions to the original submitter.

It's specifically _not_ signed-off.

> == Summary ==
> 
> $ dim checkpatch origin/drm-tip
> 7523f4bbc30d drm/i915/gvt: Use ARRAY_SIZE instead of hardcoded size
> -:16: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description 
> (prefer a maximum 75 chars per line)
> #16: 
> > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c 
> > b/drivers/gpu/drm/i915/gvt/vgpu.c
> 
> -:175: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)
> 
> total: 1 errors, 1 warnings, 0 checks, 127 lines checked
> 

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/gvt: Use ARRAY_SIZE instead of hardcoded size

[Joe Perches]

On Mon, 2020-04-13 at 22:32 +0800, Jason Yan wrote:
> Fix the following coccicheck warning:
> 
> drivers/gpu/drm/i915/gvt/vgpu.c:127:30-31: WARNING: Use ARRAY_SIZE
> 
> Signed-off-by: Jason Yan 
> ---
>  drivers/gpu/drm/i915/gvt/vgpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index 1d5ff88078bd..7d361623ff67 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -124,7 +124,7 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
>*/
>   low_avail = gvt_aperture_sz(gvt) - HOST_LOW_GM_SIZE;
>   high_avail = gvt_hidden_sz(gvt) - HOST_HIGH_GM_SIZE;
> - num_types = sizeof(vgpu_types) / sizeof(vgpu_types[0]);
> + num_types = ARRAY_SIZE(vgpu_types);
>  
>   gvt->types = kcalloc(num_types, sizeof(struct intel_vgpu_type),
>GFP_KERNEL);

It's probably better to remove num_types altogether and just
use ARRAY_SIZE in both places num_types is used.

Perhaps refactoring the function a bit more is also better.

Perhaps:

o Use ARRAY_SIZE
o Make vgpu_types static const to reduce data size and
  move the definition into the function where it's used
o Use temporaries to shorten the code indirections.

---
 drivers/gpu/drm/i915/gvt/vgpu.c | 92 +
 1 file changed, 47 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index 1d5ff8..e56f59d 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -77,26 +77,6 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu)
 #define VGPU_WEIGHT(vgpu_num)  \
(VGPU_MAX_WEIGHT / (vgpu_num))
 
-static struct {
-   unsigned int low_mm;
-   unsigned int high_mm;
-   unsigned int fence;
-
-   /* A vGPU with a weight of 8 will get twice as much GPU as a vGPU
-* with a weight of 4 on a contended host, different vGPU type has
-* different weight set. Legal weights range from 1 to 16.
-*/
-   unsigned int weight;
-   enum intel_vgpu_edid edid;
-   char *name;
-} vgpu_types[] = {
-/* Fixed vGPU type table */
-   { MB_TO_BYTES(64), MB_TO_BYTES(384), 4, VGPU_WEIGHT(8), 
GVT_EDID_1024_768, "8" },
-   { MB_TO_BYTES(128), MB_TO_BYTES(512), 4, VGPU_WEIGHT(4), 
GVT_EDID_1920_1200, "4" },
-   { MB_TO_BYTES(256), MB_TO_BYTES(1024), 4, VGPU_WEIGHT(2), 
GVT_EDID_1920_1200, "2" },
-   { MB_TO_BYTES(512), MB_TO_BYTES(2048), 4, VGPU_WEIGHT(1), 
GVT_EDID_1920_1200, "1" },
-};
-
 /**
  * intel_gvt_init_vgpu_types - initialize vGPU type list
  * @gvt : GVT device
@@ -106,9 +86,32 @@ static struct {
  */
 int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
 {
-   unsigned int num_types;
unsigned int i, low_avail, high_avail;
unsigned int min_low;
+   static const struct vgpu_types {
+   unsigned int low_mm;
+   unsigned int high_mm;
+   unsigned int fence;
+
+   /* A vGPU with a weight of 8 will get twice as much GPU
+* as a vGPU with a weight of 4 on a contended host,
+* different vGPU type has different weight set.
+* Legal weights range from 1 to 16.
+*/
+   unsigned int weight;
+   enum intel_vgpu_edid edid;
+   char *name;
+   } vgpu_types[] = {
+   /* Fixed vGPU type table */
+   { MB_TO_BYTES(64), MB_TO_BYTES(384), 4,
+ VGPU_WEIGHT(8), GVT_EDID_1024_768, "8" },
+   { MB_TO_BYTES(128), MB_TO_BYTES(512), 4,
+ VGPU_WEIGHT(4), GVT_EDID_1920_1200, "4" },
+   { MB_TO_BYTES(256), MB_TO_BYTES(1024), 4,
+ VGPU_WEIGHT(2), GVT_EDID_1920_1200, "2" },
+   { MB_TO_BYTES(512), MB_TO_BYTES(2048), 4,
+ VGPU_WEIGHT(1), GVT_EDID_1920_1200, "1" },
+   };
 
/* vGPU type name is defined as GVTg_Vx_y which contains
 * physical GPU generation type (e.g V4 as BDW server, V5 as
@@ -124,45 +127,44 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
 */
low_avail = gvt_aperture_sz(gvt) - HOST_LOW_GM_SIZE;
high_avail = gvt_hidden_sz(gvt) - HOST_HIGH_GM_SIZE;
-   num_types = sizeof(vgpu_types) / sizeof(vgpu_types[0]);
 
-   gvt->types = kcalloc(num_types, sizeof(struct intel_vgpu_type),
-GFP_KERNEL);
+   gvt->types = kcalloc(ARRAY_SIZE(vgpu_types),
+sizeof(struct intel_vgpu_type), GFP_KERNEL);
if (!gvt->types)
return -ENOMEM;
 
min_low = MB_TO_BYTES(32);
-   for (i = 0; i < num_types; ++i) {
-   if (low_avail / vgpu_types[i].low_mm == 0)
+   for (i = 0; i < ARRAY_SIZE(vgpu_types); i++) {
+   struct intel_vgpu_type *type = >types[i];
+   const struct vgpu_types 

[Intel-gfx] [PATCH -next 000/491] treewide: use fallthrough;

[Joe Perches]

There is a new fallthrough pseudo-keyword macro that can be used
to replace the various /* fallthrough */ style comments that are
used to indicate a case label code block is intended to fallthrough
to the next case label block.

See commit 294f69e662d1 ("compiler_attributes.h: Add 'fallthrough'
pseudo keyword for switch/case use")

These patches are intended to allow clang to detect missing
switch/case fallthrough uses.

Do a depth-first pass on the MAINTAINERS file and find the various
F: pattern files and convert the fallthrough comments to fallthrough;
for all files matched by all  F: patterns in in each section.

Done via the perl script below and the previously posted
cvt_fallthrough.pl script.

Link: 
https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/

These patches are based on next-20200310 and are available in

git://repo.or.cz/linux-2.6/trivial-mods.git in branch 20200310_fallthrough_2

$ cat commit_fallthrough.pl
#!/usr/bin/env perl

use sort 'stable';

#
# Reorder a sorted array so file entries are before directory entries
# depends on a trailing / for directories
# so:
#   foo/
#   foo/bar.c
# becomes
#   foo/bar.c
#   foo/
#
sub file_before_directory {
my ($array_ref) = (@_);

my $count = scalar(@$array_ref);

for (my $i = 1; $i < $count; $i++) {
if (substr(@$array_ref[$i - 1], -1) eq '/' &&
substr(@$array_ref[$i], 0, length(@$array_ref[$i - 1])) eq 
@$array_ref[$i - 1]) {
my $string = @$array_ref[$i - 1];
@$array_ref[$i - 1] = @$array_ref[$i];
@$array_ref[$i] = $string;
}
}
}

sub uniq {
my (@parms) = @_;

my %saw;
@parms = grep(!$saw{$_}++, @parms);

return @parms;
}

# Get all the F: file patterns in MAINTAINERS that could be a .[ch] file
my $maintainer_patterns = `grep -P '^F:\\s+' MAINTAINERS`;
my @patterns = split('\n', $maintainer_patterns);
s/^F:\s*// for @patterns;
@patterns = grep(!/^(?:Documentation|tools|scripts)\//, @patterns);
@patterns = grep(!/\.(?:dtsi?|rst|config)$/, @patterns);
@patterns = sort @patterns;
@patterns = sort { $b =~ tr/\//\// cmp $a =~ tr/\//\// } @patterns;
file_before_directory(\@patterns);

my %sections_done;

foreach my $pattern (@patterns) {

# Find the files the pattern matches
my $pattern_files = `git ls-files -- $pattern`;
my @new_patterns = split('\n', $pattern_files);
$pattern_files = join(' ', @new_patterns);
next if ($pattern_files =~ /^\s*$/);

# Find the section the first file matches
my $pattern_file = @new_patterns[0];
my $section_output = `./scripts/get_maintainer.pl --nogit --nogit-fallback 
--sections --pattern-depth=1 $pattern_file`;
my @section = split('\n', $section_output);
my $section_header = @section[0];

print("Section: <$section_header>\n");

# Skip the section if it's already done
print("Already done '$section_header'\n") if 
($sections_done{$section_header});
next if ($sections_done{$section_header}++);

# Find all the .[ch] files in all F: lines in that section
my @new_section;
foreach my $line (@section) {
last if ($line =~ /^\s*$/);
push(@new_section, $line);
}
@section = grep(/^F:/, @new_section);
s/^F:\s*// for @section;

@section = grep(!/^(?:Documentation|tools|scripts)\//, @section);
@section = grep(!/\.(?:dtsi?|rst|config)$/, @section);
@section = sort @section;
@section = uniq(@section);

my $section_files = join(' ', @section);

print("section_files: <$section_files>\n");

next if ($section_files =~ /^\s*$/);

my $cvt_files = `git ls-files -- $section_files`;
my @files = split('\n', $cvt_files);

@files = grep(!/^(?:Documentation|tools|scripts)\//, @files);
@files = grep(!/\.(?:dtsi?|rst|config)$/, @files);
@files = grep(/\.[ch]$/, @files);
@files = sort @files;
@files = uniq(@files);

$cvt_files = join(' ', @files);
print("files: <$cvt_files>\n");

next if (scalar(@files) < 1);

# Convert fallthroughs for all [.ch] files in the section
print("doing cvt_fallthrough.pl -- $cvt_files\n");

`cvt_fallthrough.pl -- $cvt_files`;

# If nothing changed, nothing to commit
`git diff-index --quiet HEAD --`;
next if (!$?);

# Commit the changes
my $fh;

open($fh, "+>", "cvt_fallthrough.commit_msg") or die "$0: can't create 
temporary file: $!\n";
print $fh <https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git@perches.com/
EOF
;
close $fh;

`git commit -s -a -F cvt_fallthrough.commit_msg`;
}

Joe Perches (491):
  MELLANOX ETHERNET INNOVA DRIVERS: Use fallthrough;
  MARVELL OCTEONTX2 RVU ADMIN FUNCTION DRIVER: Use fallthrough;
  MELLANOX MLX5 core VPI driver: Use fallthrough;
  PERFORMANCE EVENTS SUBSYSTEM: Use fallthrough;
  ARM/UNI

[Intel-gfx] [PATCH -next 026/491] INTEL GVT-g DRIVERS (Intel GPU Virtualization): Use fallthrough;

[Joe Perches]

Convert the various uses of fallthrough comments to fallthrough;

Done via script
Link: 
https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/

Signed-off-by: Joe Perches 
---
 drivers/gpu/drm/i915/gvt/handlers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gvt/handlers.c 
b/drivers/gpu/drm/i915/gvt/handlers.c
index 1793f69..0e792f9 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -1225,7 +1225,7 @@ static int handle_g2v_notification(struct intel_vgpu 
*vgpu, int notification)
switch (notification) {
case VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE:
root_entry_type = GTT_TYPE_PPGTT_ROOT_L3_ENTRY;
-   /* fall through */
+   fallthrough;
case VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE:
mm = intel_vgpu_get_ppgtt_mm(vgpu, root_entry_type, pdps);
return PTR_ERR_OR_ZERO(mm);
-- 
2.24.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] MAINTAINERS: adjust to reservation.h renaming

[Joe Perches]

On Fri, 2020-03-06 at 11:39 +0100, Daniel Vetter wrote:
> On Wed, Mar 04, 2020 at 01:08:32PM +0100, Christian König wrote:
> > Am 04.03.20 um 13:07 schrieb Lukas Bulwahn:
> > > Commit 52791eeec1d9 ("dma-buf: rename reservation_object to dma_resv")
> > > renamed include/linux/reservation.h to include/linux/dma-resv.h, but
> > > missed the reference in the MAINTAINERS entry.
> > > 
> > > Since then, ./scripts/get_maintainer.pl --self-test complains:
> > > 
> > >warning: no file matches F: include/linux/reservation.h
> > > 
> > > Adjust the DMA BUFFER SHARING FRAMEWORK entry in MAINTAINERS.
> > > 
> > > Co-developed-by: Sebastian Duda 
> > > Signed-off-by: Sebastian Duda 
> > > Signed-off-by: Lukas Bulwahn 
> > 
> > Reviewed-by: Christian König 
> 
> You'll push this too?
> -Daniel
> 
> > > ---
> > > Christian, please pick this patch.
> > > applies cleanly on current master and next-20200303
> > > 
> > >   MAINTAINERS | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 6158a143a13e..3d6cb2789c9e 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -5022,7 +5022,7 @@ L:  dri-de...@lists.freedesktop.org
> > >   L:  linaro-mm-...@lists.linaro.org (moderated for non-subscribers)
> > >   F:  drivers/dma-buf/
> > >   F:  include/linux/dma-buf*
> > > -F:   include/linux/reservation.h
> > > +F:   include/linux/dma-resv.h
> > >   F:  include/linux/*fence.h
> > >   F:  Documentation/driver-api/dma-buf.rst
> > >   K:  dma_(buf|fence|resv)

Slightly unrelated:

The K: entry matches a lot of other things
and may have a lot of false positive matches
like any variable named dma_buffer

This should also use (?:...) to avoid a perl
capture group.

Perhaps:

K:  '\bdma_(?:buf|fence|resv)\b'


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4] string-choice: add yesno(), onoff(), enableddisabled(), plural() helpers

[Joe Perches]

On Wed, 2019-10-23 at 15:56 -0700, Andrew Morton wrote:
> And doing this will cause additional savings: calling a single-arg
> out-of-line function generates less .text than calling yesno().

I get no change in size at all with any of
extern
static __always_inline
with either of bool or int argument.

gcc 8.3, defconfig with CONFIG_CHELSIO_T4


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] mmotm 2019-07-04-15-01 uploaded (gpu/drm/i915/oa/)

[Joe Perches]

On Thu, 2019-07-04 at 22:09 -0700, Andrew Morton wrote:
> diff(1) doesn't seem to know how to handle a zero-length file.
> 
> y:/home/akpm> mkdir foo
> y:/home/akpm> cd foo
> y:/home/akpm/foo> touch x
> y:/home/akpm/foo> diff -uN x y
> y:/home/akpm/foo> date > x
> y:/home/akpm/foo> diff -uN x y
> --- x   2019-07-04 21:58:37.815028211 -0700
> +++ y   1969-12-31 16:00:00.0 -0800
> @@ -1 +0,0 @@
> -Thu Jul  4 21:58:37 PDT 2019
> 
> So when comparing a zero-length file with a non-existent file, diff
> produces no output.

Why use the -N option ?

$ diff --help
[...]
  -N, --new-file  treat absent files as empty

otherwise

$ cd $(mktemp -d -p .)
$ touch x
$ diff -u x y
diff: y: No such file or directory


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH V2] include: linux: Regularise the use of FIELD_SIZEOF macro

[Joe Perches]

On Sat, 2019-06-29 at 17:25 +0300, Alexey Dobriyan wrote:
> On Tue, Jun 11, 2019 at 03:00:10PM -0600, Andreas Dilger wrote:
> > On Jun 11, 2019, at 2:48 PM, Andrew Morton  
> > wrote:
> > > On Wed, 12 Jun 2019 01:08:36 +0530 Shyam Saini 
> > >  wrote:
> > I did a check, and FIELD_SIZEOF() is used about 350x, while sizeof_field()
> > is about 30x, and SIZEOF_FIELD() is only about 5x.
> > 
> > That said, I'm much more in favour of "sizeof_field()" or "sizeof_member()"
> > than FIELD_SIZEOF().  Not only does that better match "offsetof()", with
> > which it is closely related, but is also closer to the original "sizeof()".
> > 
> > Since this is a rather trivial change, it can be split into a number of
> > patches to get approval/landing via subsystem maintainers, and there is no
> > huge urgency to remove the original macros until the users are gone.  It
> > would make sense to remove SIZEOF_FIELD() and sizeof_field() quickly so
> > they don't gain more users, and the remaining FIELD_SIZEOF() users can be
> > whittled away as the patches come through the maintainer trees.
> 
> The signature should be
> 
>   sizeof_member(T, m)
> 
> it is proper English,
> it is lowercase, so is easier to type,
> it uses standard term (member, not field),
> it blends in with standard "sizeof" operator,

yes please.

Also, a simple script conversion applied
immediately after an rc1 might be easiest
rather than individual patches.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 5/7] lib/hexdump.c: Allow multiple groups to be separated by lines '|'

[Joe Perches]

On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote:
> From: Alastair D'Silva 
> 
> With the wider display format, it can become hard to identify how many
> bytes into the line you are looking at.
> 
> The patch adds new flags to hex_dump_to_buffer() and print_hex_dump() to
> print vertical lines to separate every N groups of bytes.
> 
> eg.
> buf:: 454d414e 43415053|4e495f45 00584544  NAMESPAC|E_INDEX.
> buf:0010:  0002|   |
> 
> Signed-off-by: Alastair D'Silva 
> ---
>  include/linux/printk.h |  3 +++
>  lib/hexdump.c  | 59 --
>  2 files changed, 54 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/printk.h b/include/linux/printk.h
[]
> @@ -485,6 +485,9 @@ enum {
>  
>  #define HEXDUMP_ASCIIBIT(0)
>  #define HEXDUMP_SUPPRESS_REPEATEDBIT(1)
> +#define HEXDUMP_2_GRP_LINES  BIT(2)
> +#define HEXDUMP_4_GRP_LINES  BIT(3)
> +#define HEXDUMP_8_GRP_LINES  BIT(4)

These aren't really bits as only one value should be set
as 8 overrides 4 and 4 overrides 2.

I would also expect this to be a value of 2 in your above
example, rather than 8.  It's described as groups not bytes.

The example is showing a what would normally be a space ' '
separator as a vertical bar '|' every 2nd grouping.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags

[Joe Perches]

On Tue, 2019-06-25 at 15:06 +1000, Alastair D'Silva wrote:
> The change actions Jani's suggestion:
> https://lkml.org/lkml/2019/6/20/343

I suggest not changing any of the existing uses of
hex_dump_to_buffer and only use hex_dump_to_buffer_ext
when necessary for your extended use cases.



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags

[Joe Perches]

On Tue, 2019-06-25 at 15:06 +1000, Alastair D'Silva wrote:
> On Mon, 2019-06-24 at 22:01 -0700, Joe Perches wrote:
> > On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote:
> > > From: Alastair D'Silva 
> > > 
> > > In order to support additional features, rename hex_dump_to_buffer
> > > to
> > > hex_dump_to_buffer_ext, and replace the ascii bool parameter with
> > > flags.
> > []
> > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c
> > > b/drivers/gpu/drm/i915/intel_engine_cs.c
> > []
> > > @@ -1338,9 +1338,8 @@ static void hexdump(struct drm_printer *m,
> > > const void *buf, size_t len)
> > >   }
> > >  
> > >   WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos,
> > > - rowsize, sizeof(u32),
> > > - line, sizeof(line),
> > > - false) >=
> > > sizeof(line));
> > > + rowsize, sizeof(u32),
> > > line,
> > > + sizeof(line)) >=
> > > sizeof(line));
> > 
> > Huh?  Why do this?
[]
> The change actions Jani's suggestion:
> https://lkml.org/lkml/2019/6/20/343

I think you need to read this change again.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 0/7] Hexdump Enhancements

[Joe Perches]

On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote:
> From: Alastair D'Silva 
> 
> Apologies for the large CC list, it's a heads up for those responsible
> for subsystems where a prototype change in generic code causes a change
> in those subsystems.
[]
> The default behaviour of hexdump is unchanged, however, the prototype
> for hex_dump_to_buffer() has changed, and print_hex_dump() has been
> renamed to print_hex_dump_ext(), with a wrapper replacing it for
> compatibility with existing code, which would have been too invasive to
> change.

I believe this cover letter is misleading.

The point of the wrapper is to avoid unnecessary changes
in existing
code.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags

[Joe Perches]

On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote:
> From: Alastair D'Silva 
> 
> In order to support additional features, rename hex_dump_to_buffer to
> hex_dump_to_buffer_ext, and replace the ascii bool parameter with flags.
[]
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
> b/drivers/gpu/drm/i915/intel_engine_cs.c
[]
> @@ -1338,9 +1338,8 @@ static void hexdump(struct drm_printer *m, const void 
> *buf, size_t len)
>   }
>  
>   WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos,
> - rowsize, sizeof(u32),
> - line, sizeof(line),
> - false) >= sizeof(line));
> + rowsize, sizeof(u32), line,
> + sizeof(line)) >= sizeof(line));

Huh?  Why do this?

> diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c 
> b/drivers/isdn/hardware/mISDN/mISDNisar.c
[]
> @@ -70,8 +70,9 @@ send_mbox(struct isar_hw *isar, u8 his, u8 creg, u8 len, u8 
> *msg)
>   int l = 0;
>  
>   while (l < (int)len) {
> - hex_dump_to_buffer(msg + l, len - l, 32, 1,
> -isar->log, 256, 1);
> + hex_dump_to_buffer_ext(msg + l, len - l, 32, 1,
> +isar->log, 256,
> +HEXDUMP_ASCII);

Again, why do any of these?

The point of the wrapper is to avoid changing these.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 0/7] Hexdump Enhancements

[Joe Perches]

On Thu, 2019-06-20 at 11:14 +1000, Alastair D'Silva wrote:
> On Wed, 2019-06-19 at 17:35 -0700, Joe Perches wrote:
> > On Thu, 2019-06-20 at 09:15 +1000, Alastair D'Silva wrote:
> > > On Wed, 2019-06-19 at 09:31 -0700, Joe Perches wrote:
> > > > On Mon, 2019-06-17 at 12:04 +1000, Alastair D'Silva wrote:
> > > > > From: Alastair D'Silva 
> > > > > 
> > > > > Apologies for the large CC list, it's a heads up for those
> > > > > responsible
> > > > > for subsystems where a prototype change in generic code causes
> > > > > a
> > > > > change
> > > > > in those subsystems.
> > > > > 
> > > > > This series enhances hexdump.
> > > > 
> > > > Still not a fan of these patches.
> > > 
> > > I'm afraid there's not too much action I can take on that, I'm
> > > happy to
> > > address specific issues though.
> > > 
> > > > > These improve the readability of the dumped data in certain
> > > > > situations
> > > > > (eg. wide terminals are available, many lines of empty bytes
> > > > > exist,
> > > > > etc).
> > 
> > I think it's generally overkill for the desired uses.
> 
> I understand where you're coming from, however, these patches make it a
> lot easier to work with large chucks of binary data. I think it makes
> more sense to have these patches upstream, even though committed code
> may not necessarily have all the features enabled, as it means that
> devs won't have to apply out-of-tree patches during development to make
> larger dumps manageable.
> 
> > > > Changing hexdump's last argument from bool to int is odd.
> > > > 
> > > 
> > > Think of it as replacing a single boolean with many booleans.
> > 
> > I understand it.  It's odd.
> > 
> > I would rather not have a mixture of true, false, and apparently
> > random collections of bitfields like 0xd or 0b1011 or their
> > equivalent or'd defines.
> > 
> 
> Where's the mixture? What would you propose instead?

create a hex_dump_to_buffer_ext with a new argument
and a new static inline for the old hex_dump_to_buffer
without modifying the argument list that calls
hex_dump_to_buffer with whatever added argument content
you need.

Something like:

static inline
int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
   int groupsize, char *linebuf, size_t linebuflen,
   bool ascii)
{
return hex_dump_to_buffer_ext(buf, len, rowsize, groupsize,
  linebuf, linebuflen, ascii, 0);
}

and remove EXPORT_SYMBOL(hex_dump_to_buffer)
  


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 0/7] Hexdump Enhancements

[Joe Perches]

On Thu, 2019-06-20 at 09:15 +1000, Alastair D'Silva wrote:
> On Wed, 2019-06-19 at 09:31 -0700, Joe Perches wrote:
> > On Mon, 2019-06-17 at 12:04 +1000, Alastair D'Silva wrote:
> > > From: Alastair D'Silva 
> > > 
> > > Apologies for the large CC list, it's a heads up for those
> > > responsible
> > > for subsystems where a prototype change in generic code causes a
> > > change
> > > in those subsystems.
> > > 
> > > This series enhances hexdump.
> > 
> > Still not a fan of these patches.
> 
> I'm afraid there's not too much action I can take on that, I'm happy to
> address specific issues though.
> 
> > > These improve the readability of the dumped data in certain
> > > situations
> > > (eg. wide terminals are available, many lines of empty bytes exist,
> > > etc).

I think it's generally overkill for the desired uses.

> > Changing hexdump's last argument from bool to int is odd.
> > 
> 
> Think of it as replacing a single boolean with many booleans.

I understand it.  It's odd.

I would rather not have a mixture of true, false, and apparently
random collections of bitfields like 0xd or 0b1011 or their
equivalent or'd defines.


> There's only a handful of consumers, I don't think there is a value-add 
> in creating more wrappers vs updating the existing callers.

Perhaps more reason not to modify the existing api.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 0/7] Hexdump Enhancements

[Joe Perches]

On Mon, 2019-06-17 at 12:04 +1000, Alastair D'Silva wrote:
> From: Alastair D'Silva 
> 
> Apologies for the large CC list, it's a heads up for those responsible
> for subsystems where a prototype change in generic code causes a change
> in those subsystems.
> 
> This series enhances hexdump.

Still not a fan of these patches.

> These improve the readability of the dumped data in certain situations
> (eg. wide terminals are available, many lines of empty bytes exist, etc).

Changing hexdump's last argument from bool to int is odd.

Perhaps a new function should be added instead of changing
the existing hexdump.

> The default behaviour of hexdump is unchanged, however, the prototype
> for hex_dump_to_buffer() has changed, and print_hex_dump() has been
> renamed to print_hex_dump_ext(), with a wrapper replacing it for
> compatibility with existing code, which would have been too invasive to
> change.
> 
> Hexdump selftests have be run & confirmed passed.


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

[Joe Perches]

On Thu, 2018-07-12 at 07:54 -0600, Jens Axboe wrote:
> 
> Thanks for your invaluable and useful feedback, sharing your vast
> experience in patchsets with dependencies.

I've probably more experience sending patchsets
with dependencies across subsystems than anyone.

There is no single style that works and I've
probably tried them all.

It's actually a somewhat significant issue within
this community that could use some arbitration.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if

[Joe Perches]

On Wed, 2018-07-11 at 20:50 +0200, Daniel Vetter wrote:
> On Wed, Jul 11, 2018 at 8:30 PM, Jens Axboe  wrote:
> > On 7/11/18 10:45 AM, Tejun Heo wrote:
> > > On Wed, Jul 11, 2018 at 09:40:58AM -0700, Tejun Heo wrote:
> > > > On Mon, Jul 09, 2018 at 10:36:40AM +0200, Daniel Vetter wrote:
> > > > > Makes the macros resilient against if {} else {} blocks right
> > > > > afterwards.
> > > > > 
> > > > > Signed-off-by: Daniel Vetter 
> > > > > Cc: Tejun Heo 
> > > > > Cc: Jens Axboe 
> > > > > Cc: Shaohua Li 
> > > > > Cc: Kate Stewart 
> > > > > Cc: Greg Kroah-Hartman 
> > > > > Cc: Joseph Qi 
> > > > > Cc: Daniel Vetter 
> > > > > Cc: Arnd Bergmann 
> > > > 
> > > > Acked-by: Tejun Heo 
> > > > 
> > > > Jens, it'd probably be best to route this through block tree.
> > > 
> > > Oops, this requires an earlier patch to move the for_each_if def to a
> > > common header and should be routed together.
> > 
> > Yeah, this is a problem with the submission.
> > 
> > Always (ALWAYS) CC folks on at least the cover letter and generic
> > earlier patches. Getting just one patch sent like this is mostly
> > useless, and causes more harm than good.
> 
> Ime sending a patch with more than 20 or so recipients means it gets
> stuck everywhere in moderation queues. Or outright spam filters. I
> thought the correct way to do this is to cc: mailing lists (lkml has
> them all), but apparently that's not how it's done. Despite that all
> the patch series I get never have the cover letter addressed to me
> either.
> 
> So what's the magic way to make this possible?

Jens' advice is crap.

There is no generic way to make this possible.

BCC's don't work, series that touch multiple subsystems
get rejected when the recipient list is too large.

I think you did it correctly.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC][PATCH] Makefile: globally enable VLA warning

[Joe Perches]

On Tue, 2018-06-26 at 10:40 -0700, Kees Cook wrote:
> This is the patch I've got prepared now that fixes for all VLAs have been
> sent to maintainers (some are still under review/adjustment, but there
> aren't any unexplored cases left). My intention would be to have this land
> at the end of the next merge window after all the pending VLA patches
> have landed. I just wanted to get any feedback here, since it touches
> a couple areas in the process and I didn't want anyone to be surprised. :)
[]
> diff --git a/Makefile b/Makefile
[]
> @@ -778,6 +778,9 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) 
> -print-file-name=include)
>  # warn about C99 declaration after statement
>  KBUILD_CFLAGS += $(call cc-option,-Wdeclaration-after-statement,)
>  
> +# VLAs should not be used anywhere in the kernel
> +KBUILD_CFLAGS += $(call cc-option,-Wvla)

I'd probably spell out what a VLA is here.
# VLAs (Variable Length Arrays) should not be used anywhere in the kernel

Beyond that, seems sensible, thanks.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] gpu: Consistently use octal not symbolic permissions

[Joe Perches]

On Fri, 2018-05-25 at 09:41 +0300, Jani Nikula wrote:
> On Thu, 24 May 2018, Joe Perches <j...@perches.com> wrote:
> > There is currently a mixture of octal and symbolic permissions uses
> > in files in drivers/gpu/drm and one file in drivers/gpu.
> > 
> > There are ~270 existing octal uses and ~115 S_ uses.
> > 
> > Convert all the S_ symbolic permissions to their octal equivalents
> > as using octal and not symbolic permissions is preferred by many as more
> > readable.
> > 
> > see: https://lkml.org/lkml/2016/8/2/1945
> > 
> > Done with automated conversion via:
> > $ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace 
> > 
> > Miscellanea:
> > 
> > o Wrapped modified multi-line calls to a single line where appropriate
> > o Realign modified multi-line calls to open parenthesis
> > o drivers/gpu/drm/msm/adreno/a5xx_debugfs.c has a world-writeable
> >   debug permission for "reset" - perhaps that should be modified
> > Signed-off-by: Joe Perches <j...@perches.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c|  2 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 98 
> > +++---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   |  3 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  9 +-
> >  drivers/gpu/drm/armada/armada_debugfs.c|  4 +-
> >  drivers/gpu/drm/drm_debugfs.c  |  6 +-
> >  drivers/gpu/drm/drm_debugfs_crc.c  |  4 +-
> >  drivers/gpu/drm/drm_sysfs.c|  2 +-
> >  drivers/gpu/drm/i915/gvt/firmware.c|  2 +-
> >  drivers/gpu/drm/i915/i915_debugfs.c|  8 +-
> >  drivers/gpu/drm/i915/i915_perf.c   |  2 +-
> >  drivers/gpu/drm/i915/i915_sysfs.c  | 22 ++---
> >  drivers/gpu/drm/i915/intel_pipe_crc.c  |  2 +-
> 
> Please send at least i915 changes separately. There's zero reason to
> make our lives harder for this change.

The idea is to avoid unnecessary multiple patches for
individual trees.

But you could do that via something like:

$ git am --include='drivers/gpu/drm/i915/*' 

cheers, Joe

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for gpu: Consistently use octal not symbolic permissions

[Joe Perches]

On Thu, 2018-05-24 at 20:55 +, Patchwork wrote:
> == Series Details ==
> 
> Series: gpu: Consistently use octal not symbolic permissions
> URL   : https://patchwork.freedesktop.org/series/43729/
> State : warning

All of these are existing and expected.
Is there something else required?

> == Summary ==
> 
> $ dim checkpatch origin/drm-tip
> ce883dbd4f90 gpu: Consistently use octal not symbolic permissions
> -:18: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description 
> (prefer a maximum 75 chars per line)
> #18: 
> $ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace 
> 
> -:53: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'amdgpu_get_dpm_forced_performance_level' to 
> 'power_dpm_force_performance_level_show' 
> 'amdgpu_set_dpm_forced_performance_level' to 
> 'power_dpm_force_performance_level_store'
> #53: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:909:
> +static DEVICE_ATTR(power_dpm_force_performance_level, 0644,
> 
> -:85: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'amdgpu_get_pp_num_states' to 'pp_num_states_show'
> #85: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:912:
> +static DEVICE_ATTR(pp_num_states, 0444, amdgpu_get_pp_num_states, NULL);
> 
> -:86: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'amdgpu_get_pp_cur_state' to 'pp_cur_state_show'
> #86: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:913:
> +static DEVICE_ATTR(pp_cur_state, 0444, amdgpu_get_pp_cur_state, NULL);
> 
> -:87: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'amdgpu_get_pp_force_state' to 'pp_force_state_show' 
> 'amdgpu_set_pp_force_state' to 'pp_force_state_store'
> #87: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:914:
> +static DEVICE_ATTR(pp_force_state, 0644,
> 
> -:90: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'amdgpu_get_pp_table' to 'pp_table_show' 'amdgpu_set_pp_table' to 
> 'pp_table_store'
> #90: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:917:
> +static DEVICE_ATTR(pp_table, 0644,
> 
> -:93: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'amdgpu_get_pp_dpm_sclk' to 'pp_dpm_sclk_show' 'amdgpu_set_pp_dpm_sclk' to 
> 'pp_dpm_sclk_store'
> #93: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:920:
> +static DEVICE_ATTR(pp_dpm_sclk, 0644,
> 
> -:96: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'amdgpu_get_pp_dpm_mclk' to 'pp_dpm_mclk_show' 'amdgpu_set_pp_dpm_mclk' to 
> 'pp_dpm_mclk_store'
> #96: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:923:
> +static DEVICE_ATTR(pp_dpm_mclk, 0644,
> 
> -:99: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'amdgpu_get_pp_dpm_pcie' to 'pp_dpm_pcie_show' 'amdgpu_set_pp_dpm_pcie' to 
> 'pp_dpm_pcie_store'
> #99: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:926:
> +static DEVICE_ATTR(pp_dpm_pcie, 0644,
> 
> -:102: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'amdgpu_get_pp_sclk_od' to 'pp_sclk_od_show' 'amdgpu_set_pp_sclk_od' to 
> 'pp_sclk_od_store'
> #102: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:929:
> +static DEVICE_ATTR(pp_sclk_od, 0644,
> 
> -:105: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'amdgpu_get_pp_mclk_od' to 'pp_mclk_od_show' 'amdgpu_set_pp_mclk_od' to 
> 'pp_mclk_od_store'
> #105: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:932:
> +static DEVICE_ATTR(pp_mclk_od, 0644,
> 
> -:108: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'amdgpu_get_pp_power_profile_mode' to 'pp_power_profile_mode_show' 
> 'amdgpu_set_pp_power_profile_mode' to 'pp_power_profile_mode_store'
> #108: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:935:
> +static DEVICE_ATTR(pp_power_profile_mode, 0644,
> 
> -:111: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'amdgpu_get_pp_od_clk_voltage' to 'pp_od_clk_voltage_show' 
> 'amdgpu_set_pp_od_clk_voltage' to 'pp_od_clk_voltage_store'
> #111: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:938:
> +static DEVICE_ATTR(pp_od_clk_voltage, 0644,
> 
> -:141: WARNING:LONG_LINE: line over 100 characters
> #141: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:1327:
> +static SENSOR_DEVICE_ATTR(pwm1_enable, 0644, amdgpu_hwmon_get_pwm1_enable, 
> amdgpu_hwmon_set_pwm1_enable, 0);
> 
> -:152: WARNING:LONG_LINE: line over 100 characters
> #152: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:1338:
> +static SENSOR_DEVICE_ATTR(power1_cap, 0644, amdgpu_hwmon_show_power_cap, 
> amdgpu_hwmon_set_power_cap, 0);
> 
> -:362: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'show_rc6_ms' to 'rc6_residency_ms_show'
> #362: FILE: drivers/gpu/drm/i915/i915_sysfs.c:104:
> +static DEVICE_ATTR(rc6_residency_ms, 0444, show_rc6_ms, NULL);
> 
> -:363: WARNING:DEVICE_ATTR_FUNCTIONS: Consider renaming function(s) 
> 'show_rc6p_ms' to 'rc6p_residency_ms_show'
> #363: FILE: drivers/gpu/drm/i915/i915_sysfs.c:105:
> +static DEVICE_ATTR(rc6p_residency_ms, 0444, show_rc6p_ms, NULL);
> 
> -:364: WARNING:DEVICE_ATTR_FUNCTIONS: Consider 

[Intel-gfx] [PATCH] gpu: Consistently use octal not symbolic permissions

[Joe Perches]

There is currently a mixture of octal and symbolic permissions uses
in files in drivers/gpu/drm and one file in drivers/gpu.

There are ~270 existing octal uses and ~115 S_ uses.

Convert all the S_ symbolic permissions to their octal equivalents
as using octal and not symbolic permissions is preferred by many as more
readable.

see: https://lkml.org/lkml/2016/8/2/1945

Done with automated conversion via:
$ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace 

Miscellanea:

o Wrapped modified multi-line calls to a single line where appropriate
o Realign modified multi-line calls to open parenthesis
o drivers/gpu/drm/msm/adreno/a5xx_debugfs.c has a world-writeable
  debug permission for "reset" - perhaps that should be modified

Signed-off-by: Joe Perches <j...@perches.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 98 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  9 +-
 drivers/gpu/drm/armada/armada_debugfs.c|  4 +-
 drivers/gpu/drm/drm_debugfs.c  |  6 +-
 drivers/gpu/drm/drm_debugfs_crc.c  |  4 +-
 drivers/gpu/drm/drm_sysfs.c|  2 +-
 drivers/gpu/drm/i915/gvt/firmware.c|  2 +-
 drivers/gpu/drm/i915/i915_debugfs.c|  8 +-
 drivers/gpu/drm/i915/i915_perf.c   |  2 +-
 drivers/gpu/drm/i915/i915_sysfs.c  | 22 ++---
 drivers/gpu/drm/i915/intel_pipe_crc.c  |  2 +-
 drivers/gpu/drm/msm/adreno/a5xx_debugfs.c  |  5 +-
 drivers/gpu/drm/msm/msm_perf.c |  4 +-
 drivers/gpu/drm/msm/msm_rd.c   |  4 +-
 drivers/gpu/drm/nouveau/nouveau_debugfs.c  |  2 +-
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c| 11 ++-
 .../drm/omapdrm/displays/panel-sony-acx565akm.c|  6 +-
 .../drm/omapdrm/displays/panel-tpo-td043mtea1.c| 10 +--
 drivers/gpu/drm/radeon/radeon_pm.c | 26 +++---
 drivers/gpu/drm/radeon/radeon_ttm.c|  4 +-
 drivers/gpu/drm/sti/sti_drv.c  |  2 +-
 drivers/gpu/drm/tinydrm/mipi-dbi.c |  4 +-
 drivers/gpu/drm/ttm/ttm_bo.c   |  2 +-
 drivers/gpu/drm/ttm/ttm_memory.c   | 12 +--
 drivers/gpu/drm/ttm/ttm_page_alloc.c   |  6 +-
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c   |  6 +-
 drivers/gpu/drm/udl/udl_fb.c   |  4 +-
 drivers/gpu/host1x/debug.c | 12 +--
 30 files changed, 138 insertions(+), 146 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index f5fb93795a69..7b29febff511 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -830,7 +830,7 @@ int amdgpu_debugfs_regs_init(struct amdgpu_device *adev)
 
for (i = 0; i < ARRAY_SIZE(debugfs_regs); i++) {
ent = debugfs_create_file(debugfs_regs_names[i],
- S_IFREG | S_IRUGO, root,
+ S_IFREG | 0444, root,
  adev, debugfs_regs[i]);
if (IS_ERR(ent)) {
for (j = 0; j < i; j++) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index b455da487782..fa55d7e9e784 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -905,39 +905,39 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct 
device *dev,
return -EINVAL;
 }
 
-static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, 
amdgpu_set_dpm_state);
-static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(power_dpm_state, 0644, amdgpu_get_dpm_state, 
amdgpu_set_dpm_state);
+static DEVICE_ATTR(power_dpm_force_performance_level, 0644,
   amdgpu_get_dpm_forced_performance_level,
   amdgpu_set_dpm_forced_performance_level);
-static DEVICE_ATTR(pp_num_states, S_IRUGO, amdgpu_get_pp_num_states, NULL);
-static DEVICE_ATTR(pp_cur_state, S_IRUGO, amdgpu_get_pp_cur_state, NULL);
-static DEVICE_ATTR(pp_force_state, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_force_state,
-   amdgpu_set_pp_force_state);
-static DEVICE_ATTR(pp_table, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_table,
-   amdgpu_set_pp_table);
-static DEVICE_ATTR(pp_dpm_sclk, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_dpm_sclk,
-   amdgpu_set_pp_dpm_sclk);
-static DEVICE_ATTR(pp_dpm_mclk, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_dpm_mclk,
-   amdgpu_set_pp_dpm_mclk);
-static DEVICE_ATTR(pp_dpm_pcie, S_IRUGO | S_IWUSR,
-   

Re: [Intel-gfx] [PATCH] drm/i915/gvt: don't dereference 'workload' before null checking it

[Joe Perches]

On Wed, 2018-03-21 at 19:18 +, Colin Ian King wrote:
> On 21/03/18 19:09, Joe Perches wrote:
> > On Wed, 2018-03-21 at 19:06 +, Colin King wrote:
> > > From: Colin Ian King <colin.k...@canonical.com>
> > > 
> > > The pointer workload is dereferenced before it is null checked, hence
> > > there is a potential for a null pointer dereference on workload. Fix
> > > this by only dereferencing workload after it is null checked.
> > > 
> > > Detected by CoverityScan, CID#1466017 ("Dereference before null check")
> > 
> > Maybe true, but is it possible for workload to be null?
> > Maybe the null test should be removed instead.
> 
> From what I understand from the static analysis, there may be a
> potential for workload to be null, I couldn't rule it out so I went with
> the more paranoid stance of keeping the null check in.

workload cannot be null here.

Look at the uses of sr_oa_regs and see that
workload has already been dereferenced.

> > 
> > > Fixes: fa3dd623e559 ("drm/i915/gvt: keep oa config in shadow ctx")
> > > Signed-off-by: Colin Ian King <colin.k...@canonical.com>
> > > ---
> > >  drivers/gpu/drm/i915/gvt/scheduler.c | 10 +++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
> > > b/drivers/gpu/drm/i915/gvt/scheduler.c
> > > index 068126404151..f3010e365a48 100644
> > > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > > @@ -60,9 +60,9 @@ static void set_context_pdp_root_pointer(
> > >  static void sr_oa_regs(struct intel_vgpu_workload *workload,
> > >   u32 *reg_state, bool save)
> > >  {
> > > - struct drm_i915_private *dev_priv = workload->vgpu->gvt->dev_priv;
> > > - u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
> > > - u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
> > > + struct drm_i915_private *dev_priv;
> > > + u32 ctx_oactxctrl;
> > > + u32 ctx_flexeu0;
> > >   int i = 0;
> > >   u32 flex_mmio[] = {
> > >   i915_mmio_reg_offset(EU_PERF_CNTL0),
> > > @@ -77,6 +77,10 @@ static void sr_oa_regs(struct intel_vgpu_workload 
> > > *workload,
> > >   if (!workload || !reg_state || workload->ring_id != RCS)
> > >   return;
> > >  
> > > + dev_priv = workload->vgpu->gvt->dev_priv;
> > > + ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
> > > + ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
> > > +
> > >   if (save) {
> > >   workload->oactxctrl = reg_state[ctx_oactxctrl + 1];
> > >  
> 
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/gvt: don't dereference 'workload' before null checking it

[Joe Perches]

On Wed, 2018-03-21 at 19:06 +, Colin King wrote:
> From: Colin Ian King 
> 
> The pointer workload is dereferenced before it is null checked, hence
> there is a potential for a null pointer dereference on workload. Fix
> this by only dereferencing workload after it is null checked.
> 
> Detected by CoverityScan, CID#1466017 ("Dereference before null check")

Maybe true, but is it possible for workload to be null?
Maybe the null test should be removed instead.

> Fixes: fa3dd623e559 ("drm/i915/gvt: keep oa config in shadow ctx")
> Signed-off-by: Colin Ian King 
> ---
>  drivers/gpu/drm/i915/gvt/scheduler.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
> b/drivers/gpu/drm/i915/gvt/scheduler.c
> index 068126404151..f3010e365a48 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -60,9 +60,9 @@ static void set_context_pdp_root_pointer(
>  static void sr_oa_regs(struct intel_vgpu_workload *workload,
>   u32 *reg_state, bool save)
>  {
> - struct drm_i915_private *dev_priv = workload->vgpu->gvt->dev_priv;
> - u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
> - u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
> + struct drm_i915_private *dev_priv;
> + u32 ctx_oactxctrl;
> + u32 ctx_flexeu0;
>   int i = 0;
>   u32 flex_mmio[] = {
>   i915_mmio_reg_offset(EU_PERF_CNTL0),
> @@ -77,6 +77,10 @@ static void sr_oa_regs(struct intel_vgpu_workload 
> *workload,
>   if (!workload || !reg_state || workload->ring_id != RCS)
>   return;
>  
> + dev_priv = workload->vgpu->gvt->dev_priv;
> + ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
> + ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
> +
>   if (save) {
>   workload->oactxctrl = reg_state[ctx_oactxctrl + 1];
>  
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses

[Joe Perches]

On Fri, 2018-03-16 at 08:41 +0100, Daniel Vetter wrote:
> On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > arguments that can be removed by creating separate functins.
> > 
> > Create specific functions for these calls to reduce x86/64 defconfig
> > size by ~20k.
> > 
> > Modify the existing macros to use the specific calls.
> > 
> > new:
> > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > 1876562   44542 995 1922099  1d5433 (TOTALS)
> > 
> > old:
> > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > 1897565   44542 995 1943102  1da63e (TOTALS)
> > 
> > Miscellanea:
> > 
> > o intel_display requires a change to use the specific calls.
> > 
> > Signed-off-by: Joe Perches <j...@perches.com>
> 
> Impressed with the size of the bikeshed piled on top of this I decided to
> cut this all short by merging it.

Thanks.

There was a similar patch for the DRM_DEV_ macros
awhile ago that also reduced object code.

https://lkml.org/lkml/2017/9/25/247

Never applied.

Want a remerge resend?
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses

[Joe Perches]

On Thu, 2018-03-15 at 18:14 +0200, Ville Syrjälä wrote:
> > There's no trade-off in this patch for faster/larger.
> > This patch is simply smaller.  Smaller is better.
> 
> This feels a bit like saying pink is better than red because it's
> more pink.

Silly.  If you can't say smaller total object code that
performs the same task identically is better, I think
we can't discuss much of anything about code together.

Any printk related mechanism is not fast-path so any
icache dilution isn't an issue.

> That said, I'm not arguing against this patch as such. Making things
> smaller "just because" usually doesn't cause problems.

It seems more like you haven't read the patch.

>  But I was
> hoping that we might be after some more tangible gains here, and
> thus pointed out that there may be a better way to achieve even
> bigger gains.

Sure, it's just any such a discussion should not affect
this patch being applied.

This patch reduces the argument count of the drm_printk
(now drm_dbg) call and so is faster to execute even if
the emit test is internal to the drm_dbg function.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses

[Joe Perches]

On Thu, 2018-03-15 at 17:37 +0200, Ville Syrjälä wrote:
> On Thu, Mar 15, 2018 at 08:17:53AM -0700, Joe Perches wrote:
> > On Thu, 2018-03-15 at 17:05 +0200, Ville Syrjälä wrote:
> > > On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote:
> > > > Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> > > > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> > > > > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > > > > > arguments that can be removed by creating separate functins.
> > > > > > 
> > > > > > Create specific functions for these calls to reduce x86/64 defconfig
> > > > > > size by ~20k.
> > > > > > 
> > > > > > Modify the existing macros to use the specific calls.
> > > > > > 
> > > > > > new:
> > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > > 1876562   44542 995 1922099  1d5433 (TOTALS)
> > > > > > 
> > > > > > old:
> > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > > 1897565   44542 995 1943102  1da63e (TOTALS)
> > > > > > 
> > > > > > Miscellanea:
> > > > > > 
> > > > > > o intel_display requires a change to use the specific calls.
> > > > > 
> > > > > How much would we lose if we move the (drm_debug) outside the
> > > > > functions again?
> > 
> > again?
> 
> We used to do that. Someone changed it a while back, unintentially
> I believe.
> 
> > 
> > > > >  I'm somewhat concerned about all the function call
> > > > > overhead when debugs aren't even enabled.
> > 
> > Perhaps better to have compilation elimination
> > of the entire debug output instead.
> 
> That would require every bug reporter to recompile the kernel first.
> So this is not a solution we would ever seriously consider.
> 
> Not sure if it would be possible to use the alternatives thing to
> eliminate the function calls unless the user boots wih drm.debug!=0?
> 
> > 
> > I think you are discussing a different issue and
> > this discussion should not block this patch as
> > this patch has no impact other than code size
> > reduction.
> 
> But what is the goal of the code size reduction?

Smaller code.

> I assume the main
> goal is to make better use of the instruction cache to make the
> code faster. If there's a tradeoff between smaller and slightly
> faster vs. larger and a singificantly faster I tend to think we
> should go for the latter option.

There's no trade-off in this patch for faster/larger.
This patch is simply smaller.  Smaller is better.

Your faster/larger should be a different patch proposal.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses

[Joe Perches]

On Thu, 2018-03-15 at 17:05 +0200, Ville Syrjälä wrote:
> On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote:
> > Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> > > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > > > arguments that can be removed by creating separate functins.
> > > > 
> > > > Create specific functions for these calls to reduce x86/64 defconfig
> > > > size by ~20k.
> > > > 
> > > > Modify the existing macros to use the specific calls.
> > > > 
> > > > new:
> > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > 1876562   44542 995 1922099  1d5433 (TOTALS)
> > > > 
> > > > old:
> > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > 1897565   44542 995 1943102  1da63e (TOTALS)
> > > > 
> > > > Miscellanea:
> > > > 
> > > > o intel_display requires a change to use the specific calls.
> > > 
> > > How much would we lose if we move the (drm_debug) outside the
> > > functions again?

again?

> > >  I'm somewhat concerned about all the function call
> > > overhead when debugs aren't even enabled.

Perhaps better to have compilation elimination
of the entire debug output instead.

I think you are discussing a different issue and
this discussion should not block this patch as
this patch has no impact other than code size
reduction.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses

[Joe Perches]

On Thu, 2018-03-15 at 14:22 +0100, Maarten Lankhorst wrote:
> Op 13-03-18 om 23:02 schreef Joe Perches:
> > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > arguments that can be removed by creating separate functins.
> > 
> > Create specific functions for these calls to reduce x86/64 defconfig
> > size by ~20k.
> > 
> > Modify the existing macros to use the specific calls.
> > 
> > new:
> > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > 1876562   44542 995 1922099  1d5433 (TOTALS)
> > 
> > old:
> > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > 1897565   44542 995 1943102  1da63e (TOTALS)
[]
> I guess this adds up. Nice reduction. :)

Yup. 1% of all drm object code.

> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
[]
> >  
> > -   drm_printk(level, category, "mismatch in %s %pV", name, );
> > +   if (adjust)
> > +   drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, );
> > +   else
> > +   drm_err("mismatch in %s %pV", name, );
> 
> Could this use DRM_DEBUG_KMS/DRM_ERROR?
> 
> Rest looks good, so I can fix up if you want.

If want you change something like that, it should be separate patch.

btw: There was  separate patch that also reduced object size
of the drm_dev_printk calls several months ago.  Never applied.

https://lkml.org/lkml/2017/9/25/247

cheers, Joe

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses

[Joe Perches]

drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
arguments that can be removed by creating separate functins.

Create specific functions for these calls to reduce x86/64 defconfig
size by ~20k.

Modify the existing macros to use the specific calls.

new:
$ size -t drivers/gpu/drm/built-in.a | tail -1
1876562   44542 995 1922099  1d5433 (TOTALS)

old:
$ size -t drivers/gpu/drm/built-in.a | tail -1
1897565   44542 995 1943102  1da63e (TOTALS)

Miscellanea:

o intel_display requires a change to use the specific calls.

Signed-off-by: Joe Perches <j...@perches.com>
---
 drivers/gpu/drm/drm_print.c  | 28 +---
 drivers/gpu/drm/i915/intel_display.c | 15 ---
 include/drm/drm_print.h  | 27 ++-
 3 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 781518fd88e3..79abf6d5b4db 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -89,23 +89,37 @@ void drm_dev_printk(const struct device *dev, const char 
*level,
 }
 EXPORT_SYMBOL(drm_dev_printk);
 
-void drm_printk(const char *level, unsigned int category,
-   const char *format, ...)
+void drm_dbg(unsigned int category, const char *format, ...)
 {
struct va_format vaf;
va_list args;
 
-   if (category != DRM_UT_NONE && !(drm_debug & category))
+   if (!(drm_debug & category))
return;
 
va_start(args, format);
vaf.fmt = format;
vaf.va = 
 
-   printk("%s" "[" DRM_NAME ":%ps]%s %pV",
-  level, __builtin_return_address(0),
-  strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", );
+   printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
+  __builtin_return_address(0), );
+
+   va_end(args);
+}
+EXPORT_SYMBOL(drm_dbg);
+
+void drm_err(const char *format, ...)
+{
+   struct va_format vaf;
+   va_list args;
+
+   va_start(args, format);
+   vaf.fmt = format;
+   vaf.va = 
+
+   printk(KERN_ERR "[" DRM_NAME ":%ps] *ERROR* %pV",
+  __builtin_return_address(0), );
 
va_end(args);
 }
-EXPORT_SYMBOL(drm_printk);
+EXPORT_SYMBOL(drm_err);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 2933ad38094f..d8e522e3cd39 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11059,24 +11059,17 @@ intel_compare_link_m_n(const struct intel_link_m_n 
*m_n,
 static void __printf(3, 4)
 pipe_config_err(bool adjust, const char *name, const char *format, ...)
 {
-   char *level;
-   unsigned int category;
struct va_format vaf;
va_list args;
 
-   if (adjust) {
-   level = KERN_DEBUG;
-   category = DRM_UT_KMS;
-   } else {
-   level = KERN_ERR;
-   category = DRM_UT_NONE;
-   }
-
va_start(args, format);
vaf.fmt = format;
vaf.va = 
 
-   drm_printk(level, category, "mismatch in %s %pV", name, );
+   if (adjust)
+   drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, );
+   else
+   drm_err("mismatch in %s %pV", name, );
 
va_end(args);
 }
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 2a4a42e59a47..3a40c5a3a5fa 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -200,9 +200,10 @@ __printf(6, 7)
 void drm_dev_printk(const struct device *dev, const char *level,
unsigned int category, const char *function_name,
const char *prefix, const char *format, ...);
-__printf(3, 4)
-void drm_printk(const char *level, unsigned int category,
-   const char *format, ...);
+__printf(2, 3)
+void drm_dbg(unsigned int category, const char *format, ...);
+__printf(1, 2)
+void drm_err(const char *format, ...);
 
 /* Macros to make printk easier */
 
@@ -236,7 +237,7 @@ void drm_printk(const char *level, unsigned int category,
drm_dev_printk(dev, KERN_ERR, DRM_UT_NONE, __func__, " *ERROR*",\
   fmt, ##__VA_ARGS__)
 #define DRM_ERROR(fmt, ...)\
-   drm_printk(KERN_ERR, DRM_UT_NONE, fmt,  ##__VA_ARGS__)
+   drm_err(fmt, ##__VA_ARGS__)
 
 /**
  * Rate limited error output.  Like DRM_ERROR() but won't flood the log.
@@ -279,40 +280,40 @@ void drm_printk(const char *level, unsigned int category,
drm_dev_printk(dev, KERN_DEBUG, DRM_UT_CORE, __func__, "", fmt, \
   ##args)
 #define DRM_DEBUG(fmt, ...)\
-   drm_printk(KERN_DEBUG, DRM_UT_CORE, fmt, ##__VA_ARGS__)
+   drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
 
 #define DRM_DEV_DEBU

Re: [Intel-gfx] [-next PATCH 2/4] treewide: Use DEVICE_ATTR_RW

[Joe Perches]

On Wed, 2017-12-20 at 10:59 +0100, Greg Kroah-Hartman wrote:
> > > Why you didn't send that patch to the sysfs maintainer is a bit odd...  :)
> > 
> > So here's an opportunity for you:
> > 
> > The sysfs maintainer hasn't added include/linux/sysfs.h to
> > the list of maintained files...
> > 
> > DRIVER CORE, KOBJECTS, DEBUGFS AND SYSFS
> > M:  Greg Kroah-Hartman 
> > T:  git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
> > S:  Supported
> > F:  Documentation/kobject.txt
> > F:  drivers/base/
> > F:  fs/debugfs/
> > F:  fs/sysfs/
> > F:  include/linux/debugfs.h
> > F:  include/linux/kobj*
> > F:  lib/kobj*
> 
> Heh, good point, but using get_maintainer.pl does put me at the top of
> the list that you should be cc:ing:
> 
> $ ./scripts/get_maintainer.pl --file include/linux/sysfs.h
> Greg Kroah-Hartman  
> (commit_signer:3/3=100%,authored:2/3=67%,added_lines:7/8=88%)
> Kate Stewart  (commit_signer:1/3=33%)
> Thomas Gleixner  (commit_signer:1/3=33%)
> Philippe Ombredanne  (commit_signer:1/3=33%)
> Nick Desaulniers  
> (commit_signer:1/3=33%,authored:1/3=33%,added_lines:1/8=12%,removed_lines:1/1=100%)
> linux-ker...@vger.kernel.org (open list)

The script I use to send patches adds --nogit --nogit-fallback
to copy only listed maintainers because people that send cleanup
patches don't generally like to get random patches.

> > btw: there are many uses of a reversed declaration style of DEVICE_ATTR
> > 
> > Here's another thing that could be done for more DEVICE_ATTR_ uses.
> > 
> > ===
> > 
> > Some DEVICE_ATTR definitions use a reversed static function form from
> > the typical.  Convert them to use the more common macro form so it is
> > easier to grep for the style.
[]
> > $ git grep --name-only -w DEVICE_ATTR | \
> >   xargs perl -i dev_attr_rw_backwards.perl
> Ah, nice, I love perl :

That was a bad copy/paste of the script.

The actual script for RW is:

$ cat dev_attr_rw_backwards.perl
local $/;
while (<>) {
my $file = $_;
while ($file =~ m/\bDEVICE_ATTR\s*\(\s*(\w+)\s*,/g) {
my $var = $1;
if ($file =~ 
s/\bDEVICE_ATTR\s*\(\s*${var}\s*,\s*\(?(\s*S_IRUGO\s*\|\s*S_IWUSR|\s*S_IWUSR\s*\|\s*S_IRUGO\s*|\s*0644\s*)\)?\s*,\s*show_${var}\s*,\s*store_${var}\s*\)/DEVICE_ATTR_RW(${var})/g)
 {
$file =~ s/\bshow_${var}\b/${var}_show/g;
$file =~ s/\bstore_${var}\b/${var}_store/g;
}
}
print $file;
}

There are 3 different perl scripts for rw, ro, and wo.
and these scripts, because of function renaming and
possible reuse of the original function names by other
string concatenated macros, create some bad conversions
so they need some manual cleanups too.

Perhaps coccinelle could do a better job of it, but
likely string concatenation macro uses are going to
be hard to deal with in any case.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [-next PATCH 2/4] treewide: Use DEVICE_ATTR_RW

[Joe Perches]

On Wed, 2017-12-20 at 10:32 +0100, Greg Kroah-Hartman wrote:
> On Wed, Dec 20, 2017 at 01:24:44AM -0800, Joe Perches wrote:
> > On Wed, 2017-12-20 at 10:34 +0200, Jarkko Nikula wrote:
> > > On Tue, Dec 19, 2017 at 10:15:07AM -0800, Joe Perches wrote:
> > > > Convert DEVICE_ATTR uses to DEVICE_ATTR_RW where possible.
> > 
> > [] 
> > > > diff --git a/sound/soc/omap/mcbsp.c b/sound/soc/omap/mcbsp.c
> > 
> > []
> > > > @@ -854,7 +854,7 @@ static ssize_t dma_op_mode_store(struct device *dev,
> > > > return size;
> > > >  }
> > > >  
> > > > -static DEVICE_ATTR(dma_op_mode, 0644, dma_op_mode_show, 
> > > > dma_op_mode_store);
> > > > +static DEVICE_ATTR_RW(dma_op_mode);
> > > >  
> > > 
> > > While I can ack this part here if it helps generic cleanup effort I
> > > don't understart would it improve code readability in general? Mode 644
> > > is clear and don't need any grepping but for DEVICE_ATTR_RW() I had to go
> > > through all of these files in order to see what does it mean:
> 
> Yeah, 644 is "clear", but _RW() is even more clear.  Ideally I want to
> get rid of all of the "non-standard" users that set random modes of
> sysfs files, as we get it wrong too many times.  Using the "defaults" is
> much better.
> 
> > Are you suggesting that device.h (as that is where
> > DEVICE_ATTR and the other DEVICE_ATTR_ variants
> > are #defined) should have better comments for the
> > _ variants?
> > 
> > > DEVICE_ATTR_RW: include/linux/device.h
> > > __ATTR_RW: include/linux/sysfs.h
> > > S_IWUSR: include/uapi/linux/stat.h
> > > S_IRUGO: include/linux/stat.h
> > 
> > btw: patch 1/4 of the series does remove the uses of
> > S_ from these macros in sysfs.h and converts
> > them to simple octal instead.
> 
> Why you didn't send that patch to the sysfs maintainer is a bit odd...  :)

So here's an opportunity for you:

The sysfs maintainer hasn't added include/linux/sysfs.h to
the list of maintained files...

DRIVER CORE, KOBJECTS, DEBUGFS AND SYSFS
M:  Greg Kroah-Hartman <gre...@linuxfoundation.org>
T:  git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
S:  Supported
F:  Documentation/kobject.txt
F:  drivers/base/
F:  fs/debugfs/
F:  fs/sysfs/
F:  include/linux/debugfs.h
F:  include/linux/kobj*
F:  lib/kobj*

> I should be taking this whole series through my tree.  Joe, thanks for
> doing this in an automated way, I've been wanting to see this done for a
> long time.

btw: there are many uses of a reversed declaration style of DEVICE_ATTR

Here's another thing that could be done for more DEVICE_ATTR_ uses.

===

Some DEVICE_ATTR definitions use a reversed static function form from
the typical.  Convert them to use the more common macro form so it is
easier to grep for the style.

i.e.:
static ssize_t show_(...)
and
static ssize_t store_(...)

where the static function names in the DEVICE_ATTR_RW macro are reversed

static ssize_t _show(...)
and
static ssize_t _store(...)

Done with perl script

$ cat dev_attr_rw_backwards.perl
local $/;
while (<>) {
my $file = $_;
while ($file =~ m/\bDEVICE_ATTR\s*\(\s*(\w+)\s*,/g) {
my $var = $1;
if ($file =~ s/\bDEVICE_ATTR\s*\(\s*${var}\s*,\s*\(?(\s*S_IRUGO\s*\|\s*S
$file =~ s/\bshow_${var}\b/${var}_show/g;
$file =~ s/\bstore_${var}\b/${var}_store/g;
}
}
print $file;
}

$ git grep --name-only -w DEVICE_ATTR | \
  xargs perl -i dev_attr_rw_backwards.perl


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [-next PATCH 2/4] treewide: Use DEVICE_ATTR_RW

[Joe Perches]

On Wed, 2017-12-20 at 10:34 +0200, Jarkko Nikula wrote:
> On Tue, Dec 19, 2017 at 10:15:07AM -0800, Joe Perches wrote:
> > Convert DEVICE_ATTR uses to DEVICE_ATTR_RW where possible.
[] 
> > diff --git a/sound/soc/omap/mcbsp.c b/sound/soc/omap/mcbsp.c
[]
> > @@ -854,7 +854,7 @@ static ssize_t dma_op_mode_store(struct device *dev,
> > return size;
> >  }
> >  
> > -static DEVICE_ATTR(dma_op_mode, 0644, dma_op_mode_show, dma_op_mode_store);
> > +static DEVICE_ATTR_RW(dma_op_mode);
> >  
> 
> While I can ack this part here if it helps generic cleanup effort I
> don't understart would it improve code readability in general? Mode 644
> is clear and don't need any grepping but for DEVICE_ATTR_RW() I had to go
> through all of these files in order to see what does it mean:

Are you suggesting that device.h (as that is where
DEVICE_ATTR and the other DEVICE_ATTR_ variants
are #defined) should have better comments for the
_ variants?

> DEVICE_ATTR_RW: include/linux/device.h
> __ATTR_RW: include/linux/sysfs.h
> S_IWUSR: include/uapi/linux/stat.h
> S_IRUGO: include/linux/stat.h

btw: patch 1/4 of the series does remove the uses of
S_ from these macros in sysfs.h and converts
them to simple octal instead.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [-next PATCH 2/4] treewide: Use DEVICE_ATTR_RW

[Joe Perches]

Convert DEVICE_ATTR uses to DEVICE_ATTR_RW where possible.

Done with perl script:

$ git grep -w --name-only DEVICE_ATTR | \
  xargs perl -i -e 'local $/; while (<>) { 
s/\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?(\s*S_IRUGO\s*\|\s*S_IWUSR|\s*S_IWUSR\s*\|\s*S_IRUGO\s*|\s*0644\s*)\)?\s*,\s*\1_show\s*,\s*\1_store\s*\)/DEVICE_ATTR_RW(\1)/g;
 print;}'

Signed-off-by: Joe Perches <j...@perches.com>
---
 arch/s390/kernel/topology.c  |  3 +--
 arch/tile/kernel/sysfs.c |  2 +-
 drivers/gpu/drm/i915/i915_sysfs.c|  6 ++---
 drivers/platform/x86/compal-laptop.c | 18 +--
 drivers/s390/cio/device.c|  2 +-
 drivers/scsi/lpfc/lpfc_attr.c| 43 
 drivers/thermal/thermal_sysfs.c  |  9 
 drivers/tty/serial/sh-sci.c  |  2 +-
 drivers/usb/host/xhci-dbgcap.c   |  2 +-
 drivers/usb/phy/phy-tahvo.c  |  2 +-
 drivers/video/fbdev/auo_k190x.c  |  4 ++--
 drivers/video/fbdev/w100fb.c |  4 ++--
 lib/test_firmware.c  | 14 +---
 lib/test_kmod.c  | 14 +---
 sound/soc/omap/mcbsp.c   |  4 ++--
 15 files changed, 49 insertions(+), 80 deletions(-)

diff --git a/arch/s390/kernel/topology.c b/arch/s390/kernel/topology.c
index 4d5b65e527b5..4b6e0397f66d 100644
--- a/arch/s390/kernel/topology.c
+++ b/arch/s390/kernel/topology.c
@@ -404,8 +404,7 @@ static ssize_t dispatching_store(struct device *dev,
put_online_cpus();
return rc ? rc : count;
 }
-static DEVICE_ATTR(dispatching, 0644, dispatching_show,
-dispatching_store);
+static DEVICE_ATTR_RW(dispatching);
 
 static ssize_t cpu_polarization_show(struct device *dev,
 struct device_attribute *attr, char *buf)
diff --git a/arch/tile/kernel/sysfs.c b/arch/tile/kernel/sysfs.c
index 825867c53853..af5024f0fb5a 100644
--- a/arch/tile/kernel/sysfs.c
+++ b/arch/tile/kernel/sysfs.c
@@ -184,7 +184,7 @@ static ssize_t hv_stats_store(struct device *dev,
return n < 0 ? n : count;
 }
 
-static DEVICE_ATTR(hv_stats, 0644, hv_stats_show, hv_stats_store);
+static DEVICE_ATTR_RW(hv_stats);
 
 static int hv_stats_device_add(struct device *dev, struct subsys_interface 
*sif)
 {
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
b/drivers/gpu/drm/i915/i915_sysfs.c
index c74a20b80182..1d0ab8ff5915 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -447,9 +447,9 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 
 static DEVICE_ATTR(gt_act_freq_mhz, S_IRUGO, gt_act_freq_mhz_show, NULL);
 static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, NULL);
-static DEVICE_ATTR(gt_boost_freq_mhz, S_IRUGO | S_IWUSR, 
gt_boost_freq_mhz_show, gt_boost_freq_mhz_store);
-static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, 
gt_max_freq_mhz_store);
-static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, 
gt_min_freq_mhz_store);
+static DEVICE_ATTR_RW(gt_boost_freq_mhz);
+static DEVICE_ATTR_RW(gt_max_freq_mhz);
+static DEVICE_ATTR_RW(gt_min_freq_mhz);
 
 static DEVICE_ATTR(vlv_rpe_freq_mhz, S_IRUGO, vlv_rpe_freq_mhz_show, NULL);
 
diff --git a/drivers/platform/x86/compal-laptop.c 
b/drivers/platform/x86/compal-laptop.c
index 6bcb750e1865..4f9bc72f0584 100644
--- a/drivers/platform/x86/compal-laptop.c
+++ b/drivers/platform/x86/compal-laptop.c
@@ -679,18 +679,12 @@ static int bat_writeable_property(struct power_supply 
*psy,
 /* == */
 /* Driver Globals */
 /* == */
-static DEVICE_ATTR(wake_up_pme,
-   0644, wake_up_pme_show, wake_up_pme_store);
-static DEVICE_ATTR(wake_up_modem,
-   0644, wake_up_modem_show,   wake_up_modem_store);
-static DEVICE_ATTR(wake_up_lan,
-   0644, wake_up_lan_show, wake_up_lan_store);
-static DEVICE_ATTR(wake_up_wlan,
-   0644, wake_up_wlan_show,wake_up_wlan_store);
-static DEVICE_ATTR(wake_up_key,
-   0644, wake_up_key_show, wake_up_key_store);
-static DEVICE_ATTR(wake_up_mouse,
-   0644, wake_up_mouse_show,   wake_up_mouse_store);
+static DEVICE_ATTR_RW(wake_up_pme);
+static DEVICE_ATTR_RW(wake_up_modem);
+static DEVICE_ATTR_RW(wake_up_lan);
+static DEVICE_ATTR_RW(wake_up_wlan);
+static DEVICE_ATTR_RW(wake_up_key);
+static DEVICE_ATTR_RW(wake_up_mouse);
 
 static DEVICE_ATTR(fan1_input,  S_IRUGO, fan_show,  NULL);
 static DEVICE_ATTR(temp1_input, S_IRUGO, temp_cpu,  NULL);
diff --git a/drivers/s390/cio/device.c b/drivers/s390/cio/device.c
index 75a245f38e2e..6eefb67b31f3 100644
--- a/drivers/s390/cio/device.c
+++ b/drivers/s390/cio/device.c
@@ -600,7 +600,7 @@ static ssize_t vpm_show(struct device *dev, struct 
device_attribute *attr,
 static DEVICE_ATTR(devtype, 0444, devtype_show, NULL);
 static DEVICE_ATTR(cutype, 0444, cutype_show, NULL);
 static DEVICE_ATTR(modalias, 0444, moda

[Intel-gfx] [-next PATCH 0/4] sysfs and DEVICE_ATTR_

[Joe Perches]

Joe Perches (4):
  sysfs.h: Use octal permissions
  treewide: Use DEVICE_ATTR_RW
  treewide: Use DEVICE_ATTR_RO
  treewide: Use DEVICE_ATTR_WO

 arch/arm/mach-pxa/sharpsl_pm.c |  4 +-
 arch/s390/kernel/smp.c |  2 +-
 arch/s390/kernel/topology.c|  3 +-
 arch/sh/drivers/push-switch.c  |  2 +-
 arch/tile/kernel/sysfs.c   | 12 ++--
 arch/x86/kernel/cpu/microcode/core.c   |  2 +-
 drivers/acpi/device_sysfs.c|  6 +-
 drivers/char/ipmi/ipmi_msghandler.c| 17 +++---
 drivers/gpu/drm/i915/i915_sysfs.c  | 12 ++--
 drivers/input/touchscreen/elants_i2c.c |  2 +-
 drivers/net/ethernet/ibm/ibmvnic.c |  2 +-
 drivers/net/wimax/i2400m/sysfs.c   |  3 +-
 drivers/nvme/host/core.c   | 10 ++--
 drivers/platform/x86/compal-laptop.c   | 18 ++
 drivers/s390/cio/css.c |  8 +--
 drivers/s390/cio/device.c  | 10 ++--
 drivers/s390/crypto/ap_card.c  |  2 +-
 drivers/scsi/hpsa.c| 10 ++--
 drivers/scsi/lpfc/lpfc_attr.c  | 64 --
 .../staging/media/atomisp/pci/atomisp2/hmm/hmm.c   |  8 +--
 drivers/thermal/thermal_sysfs.c| 17 +++---
 drivers/tty/serial/sh-sci.c|  2 +-
 drivers/usb/host/xhci-dbgcap.c |  2 +-
 drivers/usb/phy/phy-tahvo.c|  2 +-
 drivers/video/fbdev/auo_k190x.c|  4 +-
 drivers/video/fbdev/w100fb.c   |  4 +-
 include/linux/sysfs.h  | 14 ++---
 lib/test_firmware.c| 14 ++---
 lib/test_kmod.c| 14 ++---
 sound/soc/omap/mcbsp.c |  4 +-
 sound/soc/soc-core.c   |  2 +-
 sound/soc/soc-dapm.c   |  2 +-
 32 files changed, 120 insertions(+), 158 deletions(-)

-- 
2.15.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [-next PATCH 3/4] treewide: Use DEVICE_ATTR_RO

[Joe Perches]

Convert DEVICE_ATTR uses to DEVICE_ATTR_RO where possible.

Done with perl script:

$ git grep -w --name-only DEVICE_ATTR | \
  xargs perl -i -e 'local $/; while (<>) { 
s/\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?(?:\s*S_IRUGO\s*|\s*0444\s*)\)?\s*,\s*\1_show\s*,\s*NULL\s*\)/DEVICE_ATTR_RO(\1)/g;
 print;}'

Signed-off-by: Joe Perches <j...@perches.com>
---
 arch/arm/mach-pxa/sharpsl_pm.c   |  4 ++--
 arch/sh/drivers/push-switch.c|  2 +-
 arch/tile/kernel/sysfs.c | 10 +-
 drivers/acpi/device_sysfs.c  |  6 +++---
 drivers/char/ipmi/ipmi_msghandler.c  | 17 -
 drivers/gpu/drm/i915/i915_sysfs.c|  6 +++---
 drivers/nvme/host/core.c | 10 +-
 drivers/s390/cio/css.c   |  8 
 drivers/s390/cio/device.c|  8 
 drivers/s390/crypto/ap_card.c|  2 +-
 drivers/scsi/hpsa.c  | 10 +-
 drivers/scsi/lpfc/lpfc_attr.c| 18 --
 drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c |  8 
 drivers/thermal/thermal_sysfs.c  |  6 +++---
 sound/soc/soc-core.c |  2 +-
 sound/soc/soc-dapm.c |  2 +-
 16 files changed, 58 insertions(+), 61 deletions(-)

diff --git a/arch/arm/mach-pxa/sharpsl_pm.c b/arch/arm/mach-pxa/sharpsl_pm.c
index 398ba9ba2632..ef9fd9b759cb 100644
--- a/arch/arm/mach-pxa/sharpsl_pm.c
+++ b/arch/arm/mach-pxa/sharpsl_pm.c
@@ -802,8 +802,8 @@ static ssize_t battery_voltage_show(struct device *dev, 
struct device_attribute
return sprintf(buf, "%d\n", sharpsl_pm.battstat.mainbat_voltage);
 }
 
-static DEVICE_ATTR(battery_percentage, 0444, battery_percentage_show, NULL);
-static DEVICE_ATTR(battery_voltage, 0444, battery_voltage_show, NULL);
+static DEVICE_ATTR_RO(battery_percentage);
+static DEVICE_ATTR_RO(battery_voltage);
 
 extern void (*apm_get_power_status)(struct apm_power_info *);
 
diff --git a/arch/sh/drivers/push-switch.c b/arch/sh/drivers/push-switch.c
index a17181160233..762bc5619910 100644
--- a/arch/sh/drivers/push-switch.c
+++ b/arch/sh/drivers/push-switch.c
@@ -24,7 +24,7 @@ static ssize_t switch_show(struct device *dev,
struct push_switch_platform_info *psw_info = dev->platform_data;
return sprintf(buf, "%s\n", psw_info->name);
 }
-static DEVICE_ATTR(switch, S_IRUGO, switch_show, NULL);
+static DEVICE_ATTR_RO(switch);
 
 static void switch_timer(struct timer_list *t)
 {
diff --git a/arch/tile/kernel/sysfs.c b/arch/tile/kernel/sysfs.c
index af5024f0fb5a..b09456a3d77a 100644
--- a/arch/tile/kernel/sysfs.c
+++ b/arch/tile/kernel/sysfs.c
@@ -38,7 +38,7 @@ static ssize_t chip_width_show(struct device *dev,
 {
return sprintf(page, "%u\n", smp_width);
 }
-static DEVICE_ATTR(chip_width, 0444, chip_width_show, NULL);
+static DEVICE_ATTR_RO(chip_width);
 
 static ssize_t chip_height_show(struct device *dev,
struct device_attribute *attr,
@@ -46,7 +46,7 @@ static ssize_t chip_height_show(struct device *dev,
 {
return sprintf(page, "%u\n", smp_height);
 }
-static DEVICE_ATTR(chip_height, 0444, chip_height_show, NULL);
+static DEVICE_ATTR_RO(chip_height);
 
 static ssize_t chip_serial_show(struct device *dev,
struct device_attribute *attr,
@@ -54,7 +54,7 @@ static ssize_t chip_serial_show(struct device *dev,
 {
return get_hv_confstr(page, HV_CONFSTR_CHIP_SERIAL_NUM);
 }
-static DEVICE_ATTR(chip_serial, 0444, chip_serial_show, NULL);
+static DEVICE_ATTR_RO(chip_serial);
 
 static ssize_t chip_revision_show(struct device *dev,
  struct device_attribute *attr,
@@ -62,7 +62,7 @@ static ssize_t chip_revision_show(struct device *dev,
 {
return get_hv_confstr(page, HV_CONFSTR_CHIP_REV);
 }
-static DEVICE_ATTR(chip_revision, 0444, chip_revision_show, NULL);
+static DEVICE_ATTR_RO(chip_revision);
 
 
 static ssize_t type_show(struct device *dev,
@@ -71,7 +71,7 @@ static ssize_t type_show(struct device *dev,
 {
return sprintf(page, "tilera\n");
 }
-static DEVICE_ATTR(type, 0444, type_show, NULL);
+static DEVICE_ATTR_RO(type);
 
 #define HV_CONF_ATTR(name, conf)   \
static ssize_t name ## _show(struct device *dev,\
diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index a041689e5701..545e91420cde 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -357,7 +357,7 @@ static ssize_t real_power_state_show(struct device *dev,
return sprintf(buf, "%s\n", acpi_power_state_string(state));
 }
 
-static DEVICE_ATTR(real_power_state, 0444, real_power_

Re: [Intel-gfx] drm/i915/gvt: Use common error handling code in shadow_workload_ring_buffer()

[Joe Perches]

On Tue, 2017-10-24 at 16:51 +0200, SF Markus Elfring wrote:
> Do you prefer to delegate the proposed software refactoring
> only to a corresponding optimiser?

yes.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] drm/i915/gvt: Use common error handling code in shadow_workload_ring_buffer()

[Joe Perches]

On Tue, 2017-10-24 at 17:26 +0300, Dan Carpenter wrote:
> The point of unwind code is to undo what was done earlier.  If a
> function allocates a list of things, using standard unwind style makes
> it simpler, safer and more readable.
> 
> This isn't the case here.  Instead of making the code more readable,
> we're making it more convoluted.  It's just that two out of three error
> messages happened to be the same and Markus wants to save a bit of
> memory by using the same string.  The memory savings is not so big that
> it's worth making the code less readable.

I agree with Dan.

It doesn't save any real memory either as the compiler/linker
reuses the repeated string.

It might, depending on the compiler, save a few bytes of
object code as the compiler may not optimize the repeated
call away though.  But a good compiler could do that too.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH][drm-next] drm/i915/gvt: ensure -ve return value is handled correctly

[Joe Perches]

On Wed, 2017-09-20 at 05:46 +0800, Zhenyu Wang wrote:
> On 2017.09.19 16:55:34 +0100, Colin King wrote:
> > From: Colin Ian King 
> > 
> > An earlier fix changed the return type from find_bb_size however the
> > integer return is being assigned to a unsigned int so the -ve error
> > check will never be detected. Make bb_size an int to fix this.
> > 
> > Detected by CoverityScan CID#1456886 ("Unsigned compared against 0")
> > 
> > Fixes: 1e3197d6ad73 ("drm/i915/gvt: Refine error handling for 
> > perform_bb_shadow")
> > Signed-off-by: Colin Ian King 
> > ---
> >  drivers/gpu/drm/i915/gvt/cmd_parser.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c 
> > b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > index 2c0ccbb817dc..f41cbf664b69 100644
> > --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > @@ -1628,7 +1628,7 @@ static int perform_bb_shadow(struct parser_exec_state 
> > *s)
> > struct intel_shadow_bb_entry *entry_obj;
> > struct intel_vgpu *vgpu = s->vgpu;
> > unsigned long gma = 0;
> > -   uint32_t bb_size;
> > +   int bb_size;
> > void *dst = NULL;
> > int ret = 0;
> >  
> 
> Applied this, thanks!

Is it possible for bb_size to be both >= 2g and valid?

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCHv4 3/3] drm/vgem: Enable dmabuf import interfaces

[Joe Perches]

On Thu, 2017-05-04 at 21:25 +0100, Chris Wilson wrote:
> On Thu, May 04, 2017 at 11:45:48AM -0700, Laura Abbott wrote:
> > 
> > Enable the GEM dma-buf import interfaces in addition to the export
> > interfaces. This lets vgem be used as a test source for other allocators
> > (e.g. Ion).
> > 
> > Reviewed-by: Chris Wilson 
> > Signed-off-by: Laura Abbott 
> > ---
> > v4: Use new drm_gem_prime_import_dev function
> > ---
> >  static const struct vm_operations_struct vgem_gem_vm_ops = {
> > @@ -114,12 +142,8 @@ static void vgem_postclose(struct drm_device *dev, 
> > struct drm_file *file)
> > kfree(vfile);
> >  }
> >  
> > -/* ioctls */
> > -
> > -static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> > - struct drm_file *file,
> > - unsigned int *handle,
> > - unsigned long size)
> > +static struct drm_vgem_gem_object *__vgem_gem_create(struct drm_device 
> > *dev,
> > +   unsigned long size)
> 
> I'm going to guess that doesn't line up anymore. If checkpatch isn't
> complaining, then sorry for the noise.

Because of the very long identifiers, perhaps a
nicer way to write this is like:

static struct drm_vgem_gem_object *
__vgen_gem_create(struct drm_device *dev, unsigned long size);

> > +static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> > + struct drm_file *file,
> > + unsigned int *handle,
> > + unsigned long size)
> 
> Ditto.

etc...

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/3] gpu: drm: drivers: Convert printk(KERN_ to pr_

[Joe Perches]

Use a more common logging style.

Miscellanea:

o Coalesce formats and realign arguments
o Neaten a few macros now using pr_

Signed-off-by: Joe Perches <j...@perches.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c   |  9 -
 drivers/gpu/drm/gma500/oaktrail_lvds.c| 18 +-
 drivers/gpu/drm/gma500/psb_drv.h  |  5 ++---
 drivers/gpu/drm/gma500/psb_intel_lvds.c   |  7 +++
 drivers/gpu/drm/i915/i915_sw_fence.c  |  8 
 drivers/gpu/drm/mgag200/mgag200_mode.c|  2 +-
 drivers/gpu/drm/msm/msm_drv.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_acpi.c|  7 ---
 drivers/gpu/drm/nouveau/nouveau_vga.c |  4 ++--
 drivers/gpu/drm/nouveau/nv50_display.c| 22 +++---
 drivers/gpu/drm/nouveau/nvkm/core/mm.c| 10 +-
 drivers/gpu/drm/omapdrm/dss/dsi.c | 17 -
 drivers/gpu/drm/omapdrm/dss/dss.c |  3 +--
 drivers/gpu/drm/omapdrm/dss/dss.h | 15 ++-
 drivers/gpu/drm/omapdrm/omap_gem.c|  5 ++---
 drivers/gpu/drm/r128/r128_cce.c   |  7 +++
 drivers/gpu/drm/ttm/ttm_bo.c  |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c |  6 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c |  3 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c  |  4 ++--
 20 files changed, 72 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c 
b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 5efdb7fbb7ee..e64960db3224 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -284,8 +284,7 @@ static bool cdv_intel_lvds_mode_fixup(struct drm_encoder 
*encoder,
head) {
if (tmp_encoder != encoder
&& tmp_encoder->crtc == encoder->crtc) {
-   printk(KERN_ERR "Can't enable LVDS and another "
-  "encoder on the same pipe\n");
+   pr_err("Can't enable LVDS and another encoder on the 
same pipe\n");
return false;
}
}
@@ -756,13 +755,13 @@ void cdv_intel_lvds_init(struct drm_device *dev,
 
 failed_find:
mutex_unlock(>mode_config.mutex);
-   printk(KERN_ERR "Failed find\n");
+   pr_err("Failed find\n");
psb_intel_i2c_destroy(gma_encoder->ddc_bus);
 failed_ddc:
-   printk(KERN_ERR "Failed DDC\n");
+   pr_err("Failed DDC\n");
psb_intel_i2c_destroy(gma_encoder->i2c_bus);
 failed_blc_i2c:
-   printk(KERN_ERR "Failed BLC\n");
+   pr_err("Failed BLC\n");
drm_encoder_cleanup(encoder);
drm_connector_cleanup(connector);
kfree(lvds_priv);
diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c 
b/drivers/gpu/drm/gma500/oaktrail_lvds.c
index f7038f12ac76..e6943fef0611 100644
--- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
+++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
@@ -255,15 +255,15 @@ static void oaktrail_lvds_get_configuration_mode(struct 
drm_device *dev,
((ti->vblank_hi << 8) | ti->vblank_lo);
mode->clock = ti->pixel_clock * 10;
 #if 0
-   printk(KERN_INFO "hdisplay is %d\n", mode->hdisplay);
-   printk(KERN_INFO "vdisplay is %d\n", mode->vdisplay);
-   printk(KERN_INFO "HSS is %d\n", mode->hsync_start);
-   printk(KERN_INFO "HSE is %d\n", mode->hsync_end);
-   printk(KERN_INFO "htotal is %d\n", mode->htotal);
-   printk(KERN_INFO "VSS is %d\n", mode->vsync_start);
-   printk(KERN_INFO "VSE is %d\n", mode->vsync_end);
-   printk(KERN_INFO "vtotal is %d\n", mode->vtotal);
-   printk(KERN_INFO "clock is %d\n", mode->clock);
+   pr_info("hdisplay is %d\n", mode->hdisplay);
+   pr_info("vdisplay is %d\n", mode->vdisplay);
+   pr_info("HSS is %d\n", mode->hsync_start);
+   pr_info("HSE is %d\n", mode->hsync_end);
+   pr_info("htotal is %d\n", mode->htotal);
+   pr_info("VSS is %d\n", mode->vsync_start);
+   pr_info("VSE is %d\n", mode->vsync_end);
+   pr_info("vtotal is %d\n", mode->vtotal);
+   pr_info("clock is %d\n", mode->clock);
 #endif
mode_dev->panel_fixed_mode = mode;
}
diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h
index 83e22fd4cfc0..83667087d6e5 100644
--- a/drivers/gpu/drm/gma500/psb_drv

[Intel-gfx] [PATCH 0/3] gpu: drm: Convert printk(KERN_ to pr_

[Joe Perches]

Broken up for Daniel Vetter

Joe Perches (3):
  gpu: drm: amd/radeon: Convert printk(KERN_ to pr_
  gpu: drm: core: Convert printk(KERN_ to pr_
  gpu: drm: drivers: Convert printk(KERN_ to pr_

 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_test.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/atom.c  | 44 -
 drivers/gpu/drm/amd/amdgpu/ci_dpm.c|  4 +-
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  |  8 ++--
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  |  8 ++--
 drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c |  4 +-
 drivers/gpu/drm/amd/include/amd_pcie_helpers.h |  4 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  2 +-
 drivers/gpu/drm/amd/powerplay/inc/pp_debug.h   |  2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c|  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c | 14 +++---
 .../gpu/drm/amd/powerplay/smumgr/polaris10_smc.c   |  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c   |  4 +-
 drivers/gpu/drm/drm_cache.c| 12 ++---
 drivers/gpu/drm/drm_edid.c |  4 +-
 drivers/gpu/drm/drm_ioc32.c|  3 +-
 drivers/gpu/drm/gma500/cdv_intel_lvds.c|  9 ++--
 drivers/gpu/drm/gma500/oaktrail_lvds.c | 18 +++
 drivers/gpu/drm/gma500/psb_drv.h   |  5 +-
 drivers/gpu/drm/gma500/psb_intel_lvds.c|  7 ++-
 drivers/gpu/drm/i915/i915_sw_fence.c   |  8 ++--
 drivers/gpu/drm/mgag200/mgag200_mode.c |  2 +-
 drivers/gpu/drm/msm/msm_drv.c  |  2 +-
 drivers/gpu/drm/nouveau/nouveau_acpi.c |  7 +--
 drivers/gpu/drm/nouveau/nouveau_vga.c  |  4 +-
 drivers/gpu/drm/nouveau/nv50_display.c | 22 -
 drivers/gpu/drm/nouveau/nvkm/core/mm.c | 10 ++--
 drivers/gpu/drm/omapdrm/dss/dsi.c  | 17 ---
 drivers/gpu/drm/omapdrm/dss/dss.c  |  3 +-
 drivers/gpu/drm/omapdrm/dss/dss.h  | 15 +++---
 drivers/gpu/drm/omapdrm/omap_gem.c |  5 +-
 drivers/gpu/drm/r128/r128_cce.c|  7 ++-
 drivers/gpu/drm/radeon/atom.c  | 46 --
 drivers/gpu/drm/radeon/cik.c   | 56 --
 drivers/gpu/drm/radeon/evergreen.c |  2 +-
 drivers/gpu/drm/radeon/evergreen_cs.c  |  7 ++-
 drivers/gpu/drm/radeon/ni.c| 22 +++--
 drivers/gpu/drm/radeon/r100.c  | 18 +++
 drivers/gpu/drm/radeon/r200.c  |  3 +-
 drivers/gpu/drm/radeon/r300.c  | 13 ++---
 drivers/gpu/drm/radeon/r420.c  |  9 ++--
 drivers/gpu/drm/radeon/r520.c  |  3 +-
 drivers/gpu/drm/radeon/r600.c  | 21 +++-
 drivers/gpu/drm/radeon/r600_cs.c   |  7 ++-
 drivers/gpu/drm/radeon/radeon.h|  3 +-
 drivers/gpu/drm/radeon/radeon_atpx_handler.c   |  4 +-
 drivers/gpu/drm/radeon/radeon_audio.c  |  4 +-
 drivers/gpu/drm/radeon/radeon_clocks.c |  2 +-
 drivers/gpu/drm/radeon/radeon_device.c |  8 ++--
 drivers/gpu/drm/radeon/radeon_fb.c |  3 +-
 drivers/gpu/drm/radeon/radeon_gem.c|  4 +-
 drivers/gpu/drm/radeon/radeon_test.c   |  6 +--
 drivers/gpu/drm/radeon/rs400.c |  4 +-
 drivers/gpu/drm/radeon/rs690.c |  3 +-
 drivers/gpu/drm/radeon/rv515.c |  9 ++--
 drivers/gpu/drm/radeon/si.c| 45 ++---
 drivers/gpu/drm/ttm/ttm_bo.c   |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c  |  6 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c  |  3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |  4 +-
 69 files changed, 253 insertions(+), 362 deletions(-)

-- 
2.10.0.rc2.1.g053435c

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/2] gpu: drm: Convert printk(KERN_ to pr_

[Joe Perches]

Use a more common logging style.

Miscellanea:

o Coalesce formats and realign arguments
o Neaten a few macros now using pr_

Signed-off-by: Joe Perches <j...@perches.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_test.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/atom.c  | 44 -
 drivers/gpu/drm/amd/amdgpu/ci_dpm.c|  4 +-
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  |  8 ++--
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  |  8 ++--
 drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c |  4 +-
 drivers/gpu/drm/amd/include/amd_pcie_helpers.h |  4 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  2 +-
 drivers/gpu/drm/amd/powerplay/inc/pp_debug.h   |  2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c|  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c | 14 +++---
 .../gpu/drm/amd/powerplay/smumgr/polaris10_smc.c   |  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c   |  4 +-
 drivers/gpu/drm/drm_cache.c| 12 ++---
 drivers/gpu/drm/drm_edid.c |  4 +-
 drivers/gpu/drm/drm_ioc32.c|  3 +-
 drivers/gpu/drm/gma500/cdv_intel_lvds.c|  9 ++--
 drivers/gpu/drm/gma500/oaktrail_lvds.c | 18 +++
 drivers/gpu/drm/gma500/psb_drv.h   |  5 +-
 drivers/gpu/drm/gma500/psb_intel_lvds.c|  7 ++-
 drivers/gpu/drm/i915/i915_sw_fence.c   |  8 ++--
 drivers/gpu/drm/mgag200/mgag200_mode.c |  2 +-
 drivers/gpu/drm/msm/msm_drv.c  |  2 +-
 drivers/gpu/drm/nouveau/nouveau_acpi.c |  7 +--
 drivers/gpu/drm/nouveau/nouveau_vga.c  |  4 +-
 drivers/gpu/drm/nouveau/nv50_display.c | 22 -
 drivers/gpu/drm/nouveau/nvkm/core/mm.c | 10 ++--
 drivers/gpu/drm/omapdrm/dss/dsi.c  | 17 ---
 drivers/gpu/drm/omapdrm/dss/dss.c  |  3 +-
 drivers/gpu/drm/omapdrm/dss/dss.h  | 15 +++---
 drivers/gpu/drm/omapdrm/omap_gem.c |  5 +-
 drivers/gpu/drm/r128/r128_cce.c|  7 ++-
 drivers/gpu/drm/radeon/atom.c  | 46 --
 drivers/gpu/drm/radeon/cik.c   | 56 --
 drivers/gpu/drm/radeon/evergreen.c |  2 +-
 drivers/gpu/drm/radeon/evergreen_cs.c  |  7 ++-
 drivers/gpu/drm/radeon/ni.c| 22 +++--
 drivers/gpu/drm/radeon/r100.c  | 18 +++
 drivers/gpu/drm/radeon/r200.c  |  3 +-
 drivers/gpu/drm/radeon/r300.c  | 13 ++---
 drivers/gpu/drm/radeon/r420.c  |  9 ++--
 drivers/gpu/drm/radeon/r520.c  |  3 +-
 drivers/gpu/drm/radeon/r600.c  | 21 +++-
 drivers/gpu/drm/radeon/r600_cs.c   |  7 ++-
 drivers/gpu/drm/radeon/radeon.h|  3 +-
 drivers/gpu/drm/radeon/radeon_atpx_handler.c   |  4 +-
 drivers/gpu/drm/radeon/radeon_audio.c  |  4 +-
 drivers/gpu/drm/radeon/radeon_clocks.c |  2 +-
 drivers/gpu/drm/radeon/radeon_device.c |  8 ++--
 drivers/gpu/drm/radeon/radeon_fb.c |  3 +-
 drivers/gpu/drm/radeon/radeon_gem.c|  4 +-
 drivers/gpu/drm/radeon/radeon_test.c   |  6 +--
 drivers/gpu/drm/radeon/rs400.c |  4 +-
 drivers/gpu/drm/radeon/rs690.c |  3 +-
 drivers/gpu/drm/radeon/rv515.c |  9 ++--
 drivers/gpu/drm/radeon/si.c| 45 ++---
 drivers/gpu/drm/ttm/ttm_bo.c   |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c  |  6 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c  |  3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |  4 +-
 69 files changed, 253 insertions(+), 362 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index c1b913541739..3f636632c289 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1515,7 +1515,8 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device *adev, u32 
index, u32 v);
  */
 #define RREG32(reg) amdgpu_mm_rreg(adev, (reg), false)
 #define RREG32_IDX(reg) amdgpu_mm_rreg(adev, (reg), true)
-#define DREG32(reg) printk(KERN_INFO "REGIS

[Intel-gfx] [PATCH 0/2] gpu: drm: Use pr_cont and neaten logging

[Joe Perches]

Joe Perches (2):
  drm: Use pr_cont where appropriate
  gpu: drm: Convert printk(KERN_ to pr_

 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c| 70 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_test.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/atom.c  | 44 ++
 drivers/gpu/drm/amd/amdgpu/ci_dpm.c|  4 +-
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  |  8 +--
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  |  8 +--
 drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c |  4 +-
 drivers/gpu/drm/amd/include/amd_pcie_helpers.h |  4 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  2 +-
 drivers/gpu/drm/amd/powerplay/inc/pp_debug.h   |  2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c|  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c | 14 ++---
 .../gpu/drm/amd/powerplay/smumgr/polaris10_smc.c   |  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c   |  4 +-
 drivers/gpu/drm/drm_cache.c| 12 ++--
 drivers/gpu/drm/drm_edid.c |  4 +-
 drivers/gpu/drm/drm_ioc32.c|  3 +-
 drivers/gpu/drm/gma500/cdv_intel_lvds.c|  9 ++-
 drivers/gpu/drm/gma500/oaktrail_lvds.c | 18 +++---
 drivers/gpu/drm/gma500/psb_drv.h   |  5 +-
 drivers/gpu/drm/gma500/psb_intel_lvds.c|  7 +--
 drivers/gpu/drm/i915/i915_sw_fence.c   |  8 +--
 drivers/gpu/drm/mgag200/mgag200_mode.c |  2 +-
 drivers/gpu/drm/msm/msm_drv.c  |  2 +-
 drivers/gpu/drm/nouveau/nouveau_acpi.c |  7 ++-
 drivers/gpu/drm/nouveau/nouveau_vga.c  |  4 +-
 drivers/gpu/drm/nouveau/nv50_display.c | 22 +++
 drivers/gpu/drm/nouveau/nvkm/core/mm.c | 10 +--
 drivers/gpu/drm/omapdrm/dss/dsi.c  | 17 +++---
 drivers/gpu/drm/omapdrm/dss/dss.c  |  3 +-
 drivers/gpu/drm/omapdrm/dss/dss.h  | 15 ++---
 drivers/gpu/drm/omapdrm/omap_gem.c |  5 +-
 drivers/gpu/drm/r128/r128_cce.c|  7 +--
 drivers/gpu/drm/radeon/atom.c  | 46 ++
 drivers/gpu/drm/radeon/cik.c   | 56 ++---
 drivers/gpu/drm/radeon/evergreen.c |  2 +-
 drivers/gpu/drm/radeon/evergreen_cs.c  |  7 +--
 drivers/gpu/drm/radeon/ni.c| 22 +++
 drivers/gpu/drm/radeon/r100.c  | 18 ++
 drivers/gpu/drm/radeon/r200.c  |  3 +-
 drivers/gpu/drm/radeon/r300.c  | 13 ++--
 drivers/gpu/drm/radeon/r420.c  |  9 +--
 drivers/gpu/drm/radeon/r520.c  |  3 +-
 drivers/gpu/drm/radeon/r600.c  | 21 +++
 drivers/gpu/drm/radeon/r600_cs.c   |  7 +--
 drivers/gpu/drm/radeon/r600_dpm.c  | 71 +++---
 drivers/gpu/drm/radeon/radeon.h|  3 +-
 drivers/gpu/drm/radeon/radeon_atpx_handler.c   |  4 +-
 drivers/gpu/drm/radeon/radeon_audio.c  |  4 +-
 drivers/gpu/drm/radeon/radeon_clocks.c |  2 +-
 drivers/gpu/drm/radeon/radeon_device.c |  8 +--
 drivers/gpu/drm/radeon/radeon_fb.c |  3 +-
 drivers/gpu/drm/radeon/radeon_gem.c|  4 +-
 drivers/gpu/drm/radeon/radeon_test.c   |  6 +-
 drivers/gpu/drm/radeon/rs400.c |  4 +-
 drivers/gpu/drm/radeon/rs690.c |  3 +-
 drivers/gpu/drm/radeon/rv515.c |  9 +--
 drivers/gpu/drm/radeon/si.c| 45 +-
 drivers/gpu/drm/ttm/ttm_bo.c   |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c  |  6 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c  |  3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |  4 +-
 71 files changed, 326 insertions(+), 430 deletions(-)

-- 
2.10.0.rc2.1.g053435c

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC PATCH] get_maintainer: Look for arbitrary letter prefixes in sections

[Joe Perches]

On Thu, 2016-11-03 at 10:07 +0100, Paul Bolle wrote:
> On Mon, 2016-10-24 at 11:05 -0700, Joe Perches wrote:
> > Jani Nikula proposes patches to add a few new letter prefixes
> > for "B:" bug reporting and "C:" maintainer chatting to the
> > various sections of MAINTAINERS.
> > 
> > Add a generic mechanism to get_maintainer.pl to find sections that
> > have any combination of "[A-Z]" letter prefix types in a section.
> > 
> > Signed-off-by: Joe Perches <j...@perches.com>
> 
> This patch made it into linux-next (ie, next-20161028).
> 
> > --- a/scripts/get_maintainer.pl
> > +++ b/scripts/get_maintainer.pl
> > @@ -271,7 +273,8 @@ $output_multiline = 0 if ($output_separator ne ", ");
> >  $output_rolestats = 1 if ($interactive);
> >  $output_roles = 1 if ($output_rolestats);
> >  
> > -if ($sections) {
> > +if ($sections || $letters ne "") {
> > +$sections = 1;
> 
> This triggers:
>     Unrecognized character \xA0; marked by <-- HERE after <-- HERE near 
> column 1 at ./scripts/get_maintainer.pl line 277.
> 
> Git blame shows:
>     git blame -L 277,+1 ./scripts/get_maintainer.pl
> b67071653d3fc (Joe Perches 2016-10-28 13:22:01 +1100 277) 
> $sections = 1;
> 
> (A0 seems to be the no break space. That character was inserted more
> often further down the patch.)
> 
> Anybody else seeing this?

Yes, it's been reported and should be fixed in -mm.
The fix should show up in -next in a little bit.

For now, try:
$ sed -i -e 's/\xA0/ /g' scripts/get_maintainer.pl

cheers, Joe

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] scripts/checkpatch: Check for Reviewed-by under --strict

[Joe Perches]

On Fri, 2016-10-28 at 13:49 +0100, Chris Wilson wrote:
> Some subsystem polices have a strict requirement that every patch must
> have at least one reviewer before being approved for upstream. Since
> encouraging review is good policy (great review is even better policy!)
> enforce checking for a Reviewed-by when checkpath is run with --strict
> (or with --review).

I rather dislike this as it imposes a rule outside
of what's documented in SubmittingPatches.

Ideally, please keep a private version of this.

And unless and until SubmittingPatches is updated,
please keep this separate from --strict.

> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Andy Whitcroft <a...@canonical.com>
> Cc: Joe Perches <j...@perches.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> ---
>  scripts/checkpatch.pl | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index a8368d1c4348..9eaa5a4fbbc0 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -21,6 +21,7 @@ use Getopt::Long qw(:config no_auto_abbrev);
>  my $quiet = 0;
>  my $tree = 1;
>  my $chk_signoff = 1;
> +my $chk_review = 0;
>  my $chk_patch = 1;
>  my $tst_only;
>  my $emacs = 0;
> @@ -69,6 +70,7 @@ Options:
>-q, --quietquiet
>--no-tree  run without a kernel tree
>--no-signoff   do not check for 'Signed-off-by' line
> +  --review   check for 'Reviewed-by' line
>--patchtreat FILE as patchfile (default)
>--emacsemacs compile window format
>--terseone line per report
> @@ -183,6 +185,7 @@ GetOptions(
>   'q|quiet+'  => \$quiet,
>   'tree!' => \$tree,
>   'signoff!'  => \$chk_signoff,
> + 'review!'   => \$chk_review,
>   'patch!'=> \$chk_patch,
>   'emacs!'=> \$emacs,
>   'terse!'=> \$terse,
> @@ -217,7 +220,7 @@ help(0) if ($help);
>  
>  list_types(0) if ($list_types);
>  
> -$fix = 1 if ($fix_inplace);
> +$chk_review = 1 if ($check); # --strict implies checking for Reviewed-by
>  $check_orig = $check;
>  
>  my $exit = 0;
> @@ -857,6 +860,7 @@ sub git_commit_info {
>  }
>  
>  $chk_signoff = 0 if ($file);
> +$chk_review = 0 if ($file);
>  
>  my @rawlines = ();
>  my @lines = ();
> @@ -2130,6 +2134,7 @@ sub process {
>  
>   our $clean = 1;
>   my $signoff = 0;
> + my $review = 0;
>   my $is_patch = 0;
>   my $in_header_lines = $file ? 0 : 1;
>   my $in_commit_log = 0;  #Scanning lines before patch
> @@ -2400,6 +2405,12 @@ sub process {
>   $in_commit_log = 0;
>   }
>  
> +# Check the patch for any review:
> + if ($line =~ /^\s*reviewed-by:/i) {
> + $review++;
> + $in_commit_log = 0;
> + }
> +
>  # Check if MAINTAINERS is being updated.  If so, there's probably no need to
>  # emit the "does MAINTAINERS need updating?" message on file add/move/delete
>   if ($line =~ /^\s*MAINTAINERS\s*\|/) {
> @@ -6204,6 +6215,10 @@ sub process {
>   ERROR("MISSING_SIGN_OFF",
> "Missing Signed-off-by: line(s)\n");
>   }
> + if ($is_patch && $has_commit_log && $chk_review && $review == 0) {
> + ERROR("MISSING_REVIEW",
> +   "Missing Reviewed-by: line(s)\n");
> + }
>  
>   print report_dump();
>   if ($summary && !($clean == 1 && $quiet == 1)) {
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm: Simplify logging macros, convert DRM_NOTE to DRM_NOTICE

[Joe Perches]

On Tue, 2016-09-27 at 17:36 +0100, Emil Velikov wrote:
> On 27 September 2016 at 17:04, Joe Perches <j...@perches.com> wrote:
> > On Tue, 2016-09-27 at 11:58 -0400, Sean Paul wrote:
> > > On Sun, Sep 25, 2016 at 10:18 PM, Joe Perches <j...@perches.com> wrote:
> > > > Use a bit more consistent style with kernel loglevels
> > > I'm not convinced this is worth doing if we're going to keep the
> > > WARN/WARNING discrepancy, and I don't think we should switch DRM_WARN
> > > to DRM_WARNING since it's so widely used.
> > There is no DRM_WARN inconsistency.
> DRM_WARN is to DRM_WARNING like DRM_INFO is to DRM_INFORMATION and
> DRM_NOTE is to DRM_NOTICE...

DRM_INFORMATION doesn't exist in the kernel tree.

> is what I'm thinking and seemingly so
> does Sean. Fwiw that part seem cosmetic/unrelated to the rest of the
> patch, so it might be worth keeping separate ?

To me, simplifying the macro means using the common kernel
macro forms.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm: Simplify logging macros, convert DRM_NOTE to DRM_NOTICE

[Joe Perches]

On Tue, 2016-09-27 at 17:36 +0100, Emil Velikov wrote:
> On 27 September 2016 at 17:04, Joe Perches <j...@perches.com> wrote:
> > On Tue, 2016-09-27 at 11:58 -0400, Sean Paul wrote:
> > > On Sun, Sep 25, 2016 at 10:18 PM, Joe Perches <j...@perches.com> wrote:
> > > > Use a bit more consistent style with kernel loglevels
> > > I'm not convinced this is worth doing if we're going to keep the
> > > WARN/WARNING discrepancy, and I don't think we should switch DRM_WARN
> > > to DRM_WARNING since it's so widely used.
> > There is no DRM_WARN inconsistency.
> DRM_WARN is to DRM_WARNING like DRM_INFO is to DRM_INFORMATION and
> DRM_NOTE is to DRM_NOTICE...

DRM_INFORMATION doesn't exist in the kernel tree.

> is what I'm thinking and seemingly so
> does Sean. Fwiw that part seem cosmetic/unrelated to the rest of the
> patch, so it might be worth keeping separate ?

To me, simplifying the macro means using the common kernel
macro forms.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm: Simplify logging macros, convert DRM_NOTE to DRM_NOTICE

[Joe Perches]

On Tue, 2016-09-27 at 11:58 -0400, Sean Paul wrote:
> > On Sun, Sep 25, 2016 at 10:18 PM, Joe Perches <j...@perches.com> wrote:
> > Use a bit more consistent style with kernel loglevels
> > I'm not convinced this is worth doing if we're going to keep the
> WARN/WARNING discrepancy, and I don't think we should switch DRM_WARN
> to DRM_WARNING since it's so widely used.

There is no DRM_WARN inconsistency.

What is used is pr_warn and dev_warn, not pr_warning and dev_warning

Well, there are still a few pr_warning uses, but those
will eventually be removed/converted.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/2] drm: Simplify logging macros, convert DRM_NOTE to DRM_NOTICE

[Joe Perches]

Use a bit more consistent style with kernel loglevels without
using macro argument concatenation.

Miscellanea:

o Single statement macros don't need do {} while (0)

Signed-off-by: Joe Perches <j...@perches.com>
---
 drivers/gpu/drm/i915/intel_guc_loader.c | 22 --
 include/drm/drmP.h  | 26 +-
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 6fd39efb7894..bc4f9895f356 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -566,7 +566,7 @@ int intel_guc_setup(struct drm_device *dev)
else if (err == 0)
DRM_INFO("GuC firmware load skipped\n");
else if (ret != -EIO)
-   DRM_NOTE("GuC firmware load failed: %d\n", err);
+   DRM_NOTICE("GuC firmware load failed: %d\n", err);
else
DRM_WARN("GuC firmware load failed: %d\n", err);
 
@@ -574,7 +574,7 @@ int intel_guc_setup(struct drm_device *dev)
if (fw_path == NULL)
DRM_INFO("GuC submission without firmware not 
supported\n");
if (ret == 0)
-   DRM_NOTE("Falling back from GuC submission to execlist 
mode\n");
+   DRM_NOTICE("Falling back from GuC submission to 
execlist mode\n");
else
DRM_ERROR("GuC init failed: %d\n", ret);
}
@@ -606,7 +606,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct 
intel_guc_fw *guc_fw)
 
/* Check the size of the blob before examining buffer contents */
if (fw->size < sizeof(struct guc_css_header)) {
-   DRM_NOTE("Firmware header is missing\n");
+   DRM_NOTICE("Firmware header is missing\n");
goto fail;
}
 
@@ -618,7 +618,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct 
intel_guc_fw *guc_fw)
css->key_size_dw - css->exponent_size_dw) * sizeof(u32);
 
if (guc_fw->header_size != sizeof(struct guc_css_header)) {
-   DRM_NOTE("CSS header definition mismatch\n");
+   DRM_NOTICE("CSS header definition mismatch\n");
goto fail;
}
 
@@ -628,7 +628,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct 
intel_guc_fw *guc_fw)
 
/* now RSA */
if (css->key_size_dw != UOS_RSA_SCRATCH_MAX_COUNT) {
-   DRM_NOTE("RSA key size is bad\n");
+   DRM_NOTICE("RSA key size is bad\n");
goto fail;
}
guc_fw->rsa_offset = guc_fw->ucode_offset + guc_fw->ucode_size;
@@ -637,14 +637,14 @@ static void guc_fw_fetch(struct drm_device *dev, struct 
intel_guc_fw *guc_fw)
/* At least, it should have header, uCode and RSA. Size of all three. */
size = guc_fw->header_size + guc_fw->ucode_size + guc_fw->rsa_size;
if (fw->size < size) {
-   DRM_NOTE("Missing firmware components\n");
+   DRM_NOTICE("Missing firmware components\n");
goto fail;
}
 
/* Header and uCode will be loaded to WOPCM. Size of the two. */
size = guc_fw->header_size + guc_fw->ucode_size;
if (size > guc_wopcm_size(to_i915(dev))) {
-   DRM_NOTE("Firmware is too large to fit in WOPCM\n");
+   DRM_NOTICE("Firmware is too large to fit in WOPCM\n");
goto fail;
}
 
@@ -659,9 +659,11 @@ static void guc_fw_fetch(struct drm_device *dev, struct 
intel_guc_fw *guc_fw)
 
if (guc_fw->guc_fw_major_found != guc_fw->guc_fw_major_wanted ||
guc_fw->guc_fw_minor_found < guc_fw->guc_fw_minor_wanted) {
-   DRM_NOTE("GuC firmware version %d.%d, required %d.%d\n",
-   guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found,
-   guc_fw->guc_fw_major_wanted, 
guc_fw->guc_fw_minor_wanted);
+   DRM_NOTICE("GuC firmware version %d.%d, required %d.%d\n",
+  guc_fw->guc_fw_major_found,
+  guc_fw->guc_fw_minor_found,
+  guc_fw->guc_fw_major_wanted,
+  guc_fw->guc_fw_minor_wanted);
err = -ENOEXEC;
goto fail;
}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index c53dc90942e0..95cd04aa9bf7 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -168,25 +168,25 @@ void drm_printk(const char *level, unsigned int category,
 /** \name Macros to make printk easier */
 /*@{*/
 
-#define _DRM_PRINTK(once, level, fmt, ...)   

[Intel-gfx] [PATCH 0/2] drm: Neaten and reduce object size

[Joe Perches]

Joe Perches (2):
  drm: Simplify logging macros, convert DRM_NOTE to DRM_NOTICE
  drm: Simplify drm_printk to reduce object size quite a bit

 drivers/gpu/drm/drm_drv.c   |  5 +--
 drivers/gpu/drm/i915/intel_guc_loader.c | 22 +++--
 include/drm/drmP.h  | 56 -
 3 files changed, 42 insertions(+), 41 deletions(-)

-- 
2.10.0.rc2.1.g053435c

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] MAINTAINERS: Add "B:" preferred bug reporting method

[Joe Perches]

On Tue, 2016-01-26 at 11:10 +0200, Jani Nikula wrote:
> Different subsystems and drivers have different preferred ways of
> receiving bug reports; mailing list or bugzillas at various
> locations. Add "B:" entry for specifying the preference to guide bug
> reporters at the right location.
> 
> Cc: Daniel Vetter <dan...@ffwll.ch>
> Cc: Joe Perches <j...@perches.com>
> Cc: Andrew Morton <a...@linux-foundation.org>
> Signed-off-by: Jani Nikula <jani.nik...@intel.com>
> 
> ---
> 
> v2: drop the B: entry for i915 due to bikeshedding, let's see if
> there's
> a chance this B: entry sticks in general
> ---
>  MAINTAINERS | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 30aca4aa5467..c9da3a2af398 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -75,6 +75,8 @@ Descriptions of section entries:
>   L: Mailing list that is relevant to this area
>   W: Web-page with status/info
>   Q: Patchwork web based patch tracking system site
> + B: Preferred method for reporting bugs; bug tracking system
> site
> +    or "List" for mailing list.
>   T: SCM tree type and location.
>      Type is one of: git, hg, quilt, stgit, topgit
>   S: Status, one of the following:

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] MAINTAINERS: Add "B:" preferred bug reporting method

[Joe Perches]

On Tue, 2016-01-26 at 11:10 +0200, Jani Nikula wrote:
> Different subsystems and drivers have different preferred ways of
> receiving bug reports; mailing list or bugzillas at various
> locations. Add "B:" entry for specifying the preference to guide bug
> reporters at the right location.
[]
> v2: drop the B: entry for i915 due to bikeshedding, let's see if there's
> a chance this B: entry sticks in general
[]
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 30aca4aa5467..c9da3a2af398 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -75,6 +75,8 @@ Descriptions of section entries:
>   L: Mailing list that is relevant to this area
>   W: Web-page with status/info
>   Q: Patchwork web based patch tracking system site
> + B: Preferred method for reporting bugs; bug tracking system site
> +    or "List" for mailing list.

I think that "List" is not a good idea.

Intermixing mailing list email addresses and URLs seems
odd and inconsistent.

Maybe there could be something like the "T:" and "S:"
entries where some additional qualifier is used for the
specific type of bug reporting.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] MAINTAINERS: Add "B:" preferred bug reporting method

[Joe Perches]

On Tue, 2016-01-26 at 15:33 +, Dave Gordon wrote:
> On 26/01/16 14:42, Joe Perches wrote:
> > On Tue, 2016-01-26 at 11:10 +0200, Jani Nikula wrote:
> > > Different subsystems and drivers have different preferred ways of
> > > receiving bug reports; mailing list or bugzillas at various
> > > locations. Add "B:" entry for specifying the preference to guide bug
> > > reporters at the right location.
> > []
> > > v2: drop the B: entry for i915 due to bikeshedding, let's see if there's
> > > a chance this B: entry sticks in general
> > []
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 30aca4aa5467..c9da3a2af398 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -75,6 +75,8 @@ Descriptions of section entries:
> > >   L: Mailing list that is relevant to this area
> > >   W: Web-page with status/info
> > >   Q: Patchwork web based patch tracking system site
> > > + B: Preferred method for reporting bugs; bug tracking system site
> > > +    or "List" for mailing list.
> > 
> > I think that "List" is not a good idea.
> > 
> > Intermixing mailing list email addresses and URLs seems
> > odd and inconsistent.
> > 
> > Maybe there could be something like the "T:" and "S:"
> > entries where some additional qualifier is used for the
> > specific type of bug reporting.
> 
> Make it a proper URI, allowing mailto:<listname@domain>
> or http://bugzilla.org/i915/ or anything else that people
> may come up with in future :)

Thanks Dave.  That makes some sense.

get_maintainer could then parse mailto: and http: schemes and
could do the right thing.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] MAINTAINERS: Add "B:" preferred bug reporting method

[Joe Perches]

On Fri, 2016-01-15 at 16:45 +0200, Jani Nikula wrote:
> Different subsystems and drivers have different preferred ways of
> receiving bug reports; mailing list or bugzillas at various
> locations. Add "B:" entry for specifying the preference to guide bug
> reporters at the right location.
[]
> diff --git a/MAINTAINERS b/MAINTAINERS
[]
> @@ -75,6 +75,8 @@ Descriptions of section entries:
>   L: Mailing list that is relevant to this area
>   W: Web-page with status/info
>   Q: Patchwork web based patch tracking system site
> + B: Preferred method for reporting bugs; bug tracking system site
> +    or "List" for mailing list.
>   T: SCM tree type and location.
>      Type is one of: git, hg, quilt, stgit, topgit
>   S: Status, one of the following:
> @@ -3655,6 +3657,7 @@ L:  intel-gfx@lists.freedesktop.org
>  L:   dri-de...@lists.freedesktop.org
>  W:   https://01.org/linuxgraphics/
>  Q:   http://patchwork.freedesktop.org/project/intel-gfx/
> +B:   
> https://bugs.freedesktop.org/enter_bug.cgi?product=DRI=DRM/Intel

This requires a LoginID & password

Maybe useful to show the open bugs too:
https://bugs.freedesktop.org/buglist.cgi?component=DRM%2FIntel=DRI=---

Maybe the get_maintainer script should be updated.
Something like: (untested)
---
 scripts/get_maintainer.pl | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 1873421..bbe5337 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -47,6 +47,7 @@ my $output_rolestats = 1;
 my $output_section_maxlen = 50;
 my $scm = 0;
 my $web = 0;
+my $bug = 0;
 my $subsystem = 0;
 my $status = 0;
 my $keywords = 1;
@@ -239,6 +240,7 @@ if (!GetOptions(
    'status!' => \$status,
    'scm!' => \$scm,
    'web!' => \$web,
+   'bug!' => \$bug,
    'pattern-depth=i' => \$pattern_depth,
    'k|keywords!' => \$keywords,
    'sections!' => \$sections,
@@ -276,12 +278,13 @@ if ($sections) {
 $status = 0;
 $subsystem = 0;
 $web = 0;
+$bug = 0;
 $keywords = 0;
 $interactive = 0;
 } else {
-my $selections = $email + $scm + $status + $subsystem + $web;
+my $selections = $email + $scm + $status + $subsystem + $web + $bug;
 if ($selections == 0) {
-   die "$P:  Missing required option: email, scm, status, subsystem or 
web\n";
+   die "$P:  Missing required option: email, scm, status, subsystem, web 
or bug\n";
 }
 }
 
@@ -505,6 +508,7 @@ my %hash_list_to;
 my @list_to = ();
 my @scm = ();
 my @web = ();
+my @bug = ();
 my @subsystem = ();
 my @status = ();
 my %deduplicate_name_hash = ();
@@ -537,6 +541,11 @@ if ($web) {
 output(@web);
 }
 
+if ($bug) {
+@bug = uniq(@bug);
+output(@bug);
+}
+
 exit($exit);
 
 sub ignore_email_address {
@@ -593,6 +602,7 @@ sub get_maintainers {
 @list_to = ();
 @scm = ();
 @web = ();
+@bug = ();
 @subsystem = ();
 @status = ();
 %deduplicate_name_hash = ();
@@ -802,6 +812,7 @@ MAINTAINER field selection options:
   --status => print status if any
   --subsystem => print subsystem name if any
   --web => print website(s) if any
+  --bug => print bug reporting mechanism(s) if any
 
 Output type options:
   --separator [, ] => separator for multiple entries on 1 line
@@ -1129,6 +1140,8 @@ sub add_categories {
    push(@scm, $pvalue);
    } elsif ($ptype eq "W") {
    push(@web, $pvalue);
+   } elsif ($ptype eq "B") {
+   push(@bug, $pvalue);
    } elsif ($ptype eq "S") {
    push(@status, $pvalue);
    }
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] checkpatch: Add spell checking of email subject line

[Joe Perches]

Only commit log and patch additions are checked for
typos and spelling errors currently.  Add a check
of the email subject line too.

Suggested-by: Jani Nikula jani.nik...@linux.intel.com
Signed-off-by: Joe Perches j...@perches.com
---
 scripts/checkpatch.pl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 421bbb4..c061a63 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2303,7 +2303,8 @@ sub process {
}
 
 # Check for various typo / spelling mistakes
-   if (defined($misspellings)  ($in_commit_log || $line =~ 
/^\+/)) {
+   if (defined($misspellings) 
+   ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) {
while ($rawline =~ 
/(?:^|[^a-z@])($misspellings)(?:$|[^a-z@])/gi) {
my $typo = $1;
my $typo_fix = $spelling_fix{lc($typo)};


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] checkpatch spell checking (was: Re: [PATCH] drm/i915: Reudce CHV DPLL min vco frequency to 4.8 GHz)

[Joe Perches]

On Thu, 2015-03-05 at 09:43 +0200, Jani Nikula wrote:
 On Wed, 04 Mar 2015, Ville Syrjälä ville.syrj...@linux.intel.com wrote:
  On Wed, Mar 04, 2015 at 08:11:38PM +0530, Purushothaman, Vijay A wrote:
  Minor nitpick: typo in patch title
 
  Dang. I already fixed a typo there before sending this out, but turns
  out I only managed to cchange it into a different typo :( Maybe I need
  to invest in a spell checker...
 
 These days checkpatch.pl does some of this for you (see
 scripts/spelling.txt). However your reudce isn't there, and, more
 importantly, AFAICT checkpatch.pl does not look at the patch subject
 anyway.
 
 Joe, Andy, hint, hint. ;)

Patches happily accepted, (hint back..:)

(adding Kees Cook too)

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] 3.19-rc1 errors when opening LID

[Joe Perches]

On Sat, 2014-12-27 at 21:22 +0100, Rafael J. Wysocki wrote:
[]
 +++ linux-pm/include/acpi/acpi_bus.h
 @@ -589,7 +589,8 @@ static inline u32 acpi_target_system_sta
  
  static inline bool acpi_device_power_manageable(struct acpi_device *adev)
  {
 - return adev-flags.power_manageable;
 + return adev-flags.power_manageable
 +  (adev-status.present || adev-status.functional);

Most code in the kernel has these logical
continuations at the end of the previous line.

 +++ linux-pm/drivers/acpi/device_pm.c
[]
 @@ -361,7 +362,7 @@ bool acpi_bus_power_manageable(acpi_hand
   int result;
  
   result = acpi_bus_get_device(handle, device);
 - return result ? false : device-flags.power_manageable;
 + return result ? false : acpi_device_power_manageable(device);

This might read better as:

if (acpi_bus_get_device(handle, device))
return false;

return acpi_device_power_manageable(device);


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/20] fix misspelling of current function in string

[Joe Perches]

On Sun, 2014-12-07 at 20:20 +0100, Julia Lawall wrote:
 These patches replace what appears to be a reference to the name of the
 current function but is misspelled in some way by either the name of the
 function itself, or by %s and then __func__ in an argument list.

At least a few of these seem to be function tracing
style uses that might as well be deleted instead.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 0/5] Convert last few uses of __FUNCTION__ to __func__

[Joe Perches]

Outside of staging, there aren't any more uses of __FUNCTION__ now...

Joe Perches (5):
  powerpc: Convert last uses of __FUNCTION__ to __func__
  x86: Convert last uses of __FUNCTION__ to __func__
  block: Convert last uses of __FUNCTION__ to __func__
  i915: Convert last uses of __FUNCTION__ to __func__
  slab: Convert last uses of __FUNCTION__ to __func__

 arch/powerpc/platforms/pseries/nvram.c   | 11 +--
 arch/x86/kernel/hpet.c   |  2 +-
 arch/x86/kernel/rtc.c|  4 ++--
 arch/x86/platform/intel-mid/intel_mid_vrtc.c |  2 +-
 drivers/block/drbd/drbd_int.h|  8 
 drivers/block/xen-blkfront.c |  4 ++--
 drivers/gpu/drm/i915/dvo_ns2501.c| 15 ++-
 mm/slab.h|  2 +-
 8 files changed, 22 insertions(+), 26 deletions(-)

-- 
1.8.1.2.459.gbcd45b4.dirty

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 4/5] i915: Convert last uses of __FUNCTION__ to __func__

[Joe Perches]

Just about all of these have been converted to __func__,
so convert the last uses.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/gpu/drm/i915/dvo_ns2501.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c 
b/drivers/gpu/drm/i915/dvo_ns2501.c
index 954acb2..e40cd26 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -234,7 +234,7 @@ static enum drm_mode_status ns2501_mode_valid(struct 
intel_dvo_device *dvo,
 {
DRM_DEBUG_KMS
(%s: is mode valid 
(hdisplay=%d,htotal=%d,vdisplay=%d,vtotal=%d)\n,
-__FUNCTION__, mode-hdisplay, mode-htotal, mode-vdisplay,
+__func__, mode-hdisplay, mode-htotal, mode-vdisplay,
 mode-vtotal);
 
/*
@@ -262,7 +262,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
 
DRM_DEBUG_KMS
(%s: set mode (hdisplay=%d,htotal=%d,vdisplay=%d,vtotal=%d).\n,
-__FUNCTION__, mode-hdisplay, mode-htotal, mode-vdisplay,
+__func__, mode-hdisplay, mode-htotal, mode-vdisplay,
 mode-vtotal);
 
/*
@@ -277,8 +277,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
if (mode-hdisplay == 800  mode-vdisplay == 600) {
/* mode 277 */
ns-reg_8_shadow = ~NS2501_8_BPAS;
-   DRM_DEBUG_KMS(%s: switching to 800x600\n,
- __FUNCTION__);
+   DRM_DEBUG_KMS(%s: switching to 800x600\n, __func__);
 
/*
 * No, I do not know where this data comes from.
@@ -341,8 +340,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
 
} else if (mode-hdisplay == 640  mode-vdisplay == 480) {
/* mode 274 */
-   DRM_DEBUG_KMS(%s: switching to 640x480\n,
- __FUNCTION__);
+   DRM_DEBUG_KMS(%s: switching to 640x480\n, __func__);
/*
 * No, I do not know where this data comes from.
 * It is just what the video bios left in the DVO, so
@@ -406,8 +404,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
 
} else if (mode-hdisplay == 1024  mode-vdisplay == 768) {
/* mode 280 */
-   DRM_DEBUG_KMS(%s: switching to 1024x768\n,
- __FUNCTION__);
+   DRM_DEBUG_KMS(%s: switching to 1024x768\n, __func__);
/*
 * This might or might not work, actually. I'm silently
 * assuming here that the native panel resolution is
@@ -459,7 +456,7 @@ static void ns2501_dpms(struct intel_dvo_device *dvo, bool 
enable)
unsigned char ch;
 
DRM_DEBUG_KMS(%s: Trying set the dpms of the DVO to %i\n,
- __FUNCTION__, enable);
+ __func__, enable);
 
ch = ns-reg_8_shadow;
 
-- 
1.8.1.2.459.gbcd45b4.dirty

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 0/6] treewide: kmem_cache_alloc GFP_ZERO cleanups

[Joe Perches]

Just a few cleanups to use zalloc style calls and reduce
the uses of __GFP_ZERO for kmem_cache_alloc[_node] uses.

Use the more kernel normal zalloc style.

Joe Perches (6):
  slab/block: Add and use kmem_cache_zalloc_node
  block: Convert kmem_cache_alloc(...GFP_ZERO) to kmem_cache_zalloc
  i915_gem: Convert kmem_cache_alloc(...GFP_ZERO) to kmem_cache_zalloc
  aio: Convert kmem_cache_alloc(...GFP_ZERO) to kmem_cache_zalloc
  ceph: Convert kmem_cache_alloc(...GFP_ZERO) to kmem_cache_zalloc
  f2fs: Convert kmem_cache_alloc(...GFP_ZERO) to kmem_cache_zalloc

 block/blk-core.c|  3 +--
 block/blk-integrity.c   |  3 +--
 block/blk-ioc.c |  6 ++
 block/cfq-iosched.c | 10 --
 drivers/gpu/drm/i915/i915_gem.c |  2 +-
 fs/aio.c|  2 +-
 fs/ceph/dir.c   |  2 +-
 fs/ceph/file.c  |  2 +-
 fs/f2fs/super.c |  2 +-
 include/linux/slab.h|  5 +
 10 files changed, 18 insertions(+), 19 deletions(-)

-- 
1.8.1.2.459.gbcd45b4.dirty

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/6] i915_gem: Convert kmem_cache_alloc(...GFP_ZERO) to kmem_cache_zalloc

[Joe Perches]

The helper exists, might as well use it instead of __GFP_ZERO.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/gpu/drm/i915/i915_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8a0eb96..c4acd96 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -214,7 +214,7 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void 
*data,
 void *i915_gem_object_alloc(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
-   return kmem_cache_alloc(dev_priv-slab, GFP_KERNEL | __GFP_ZERO);
+   return kmem_cache_zalloc(dev_priv-slab, GFP_KERNEL);
 }
 
 void i915_gem_object_free(struct drm_i915_gem_object *obj)
-- 
1.8.1.2.459.gbcd45b4.dirty

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [drm-intel:drm-intel-fixes 52/52] ERROR: Unrecognized email address: 'stable.]'

[Joe Perches]

On Wed, 2013-07-17 at 11:29 +0200, Daniel Vetter wrote:
 On Wed, Jul 17, 2013 at 11:10 AM, Fengguang Wu fengguang...@intel.com wrote:
  FYI, there are new warnings show up in
  tree:   git://people.freedesktop.org/~danvet/drm-intel.git drm-intel-fixes
  head:   71e4092e52499ec74bc1dec0f883b15f2c424ec5
  commit: 71e4092e52499ec74bc1dec0f883b15f2c424ec5 [52/52] drm/i915: fix 
  long-standing SNB regression in power consumption after resume v2
 
  scripts/checkpatch.pl 
  0001-drm-i915-fix-long-standing-SNB-regression-in-power-c.patch
 
  ERROR: Unrecognized email address: 'stable.]'
  #41:
  cc: stable.]
 
 Well, I've added that while applying the patch - I tend to smash
 maintainer notes into the sob section and word-wraping caused the cc:
 stable remark to be parsed. Is there an officially sanctioned way for
 such notes that appeases checkpatch? Adding lkml and checkpatch
 maintainer.
 -Daniel

(It would have been nice to get the content that failes
 instead of having to pull the tree)

Don't wrap text to start a line with cc:

Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Jesse Barnes jbar...@virtuousgeek.org
[danvet: Add note about v1 vs. v2 of this patch and use standard
layout for the commit citation. Also add the tested-bys from v1 and a
cc: stable.]
Cc: sta...@vger.kernel.org (Note: tiny conflict due to the addition of

You could have added something like:

[danvet: Add note about v1 vs. v2 of this patch and use standard
 layout for the commit citation.
 Also add the tested-bys from v1 and a cc: stable.]


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx