Re: [PATCH v2] drm/i915: Move LRC register offsets to a header file

Michel Thierry
2018-01-22T21:49:20Z

On 22/01/18 13:28, Michal Wajdeczko wrote:
> On Mon, 22 Jan 2018 21:56:36 +0100, Lucas De Marchi
> <lucas.demarchi@intel.com> wrote:
>
>> On Mon, Jan 22, 2018 at 12:32:57PM -0800, Michel Thierry wrote:
>>> Newer platforms may have subtle offset changes, which will increase the
>>> number of defines, so it is probably better to start moving them to its
>>> own header file. Also move the macros used while setting the reg state.
>>>
>>> v2: Rename to intel_lrc_reg.h, to be consistent with i915_reg.h and
>>> intel_guc_reg.h (Chris)
>>>
>>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>
>> [ ... ]
>>
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc_reg.h
>>> b/drivers/gpu/drm/i915/intel_lrc_reg.h
>>> new file mode 100644
>>> index 000000000000..f50d63cb4b66
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/intel_lrc_reg.h
>>> @@ -0,0 +1,78 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*
>>> + * Copyright © 2018 Intel Corporation
>                    ^^^^
> hmm, maybe years should be "2014-2018"
>

2014 because that's when these #define were originally added?

>>> + *
>>> + * 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,
>>> sublicense,
>>> + * 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 NONINFRINGEMENT.  IN NO
>>> EVENT SHALL
>>> + * THE 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.
>>> + */
>>
>> Looking at other files added after the SPDX change, it doesn't look
>> like we should duplicate the information about license. So in this case
>> AFAIU it should contain only the SPDX tag and the Copyright, but not
>> license text. See
>>
>> git log --grep "Remove redundant license text"
>>
>>
>> Lucas De Marchi
>>
>
> and by looking at other examples I think best practice is to put this tag
> right under a copyright line:
>
> /*
>  * Copyright © 2014-2018 Intel Corporation
>  *
>  * SPDX-License-Identifier: MIT
>  */
>

Best practice, but not the most common:

$ git grep " \* SPDX-License-Identifier:" |wc -l
94

$ git grep "/\* SPDX-License-Identifier:" |wc -l
7822

Anyway it looks ok to me, objections?