Re: [PATCH v17 22/23] x86/sgx: SGX documentation
On Sun, Dec 02, 2018 at 07:28:55PM -0800, Randy Dunlap wrote: > Hi, > I have more editing comments below. > > > On 11/15/18 5:01 PM, Jarkko Sakkinen wrote: > > Documentation of the features of the Software Guard eXtensions used > > by the Linux kernel and basic design choices for the core and driver > > and functionality. > > > > Signed-off-by: Jarkko Sakkinen > > Co-developed-by: Sean Christopherson > > Signed-off-by: Sean Christopherson > > --- > > Documentation/index.rst | 1 + > > Documentation/x86/index.rst | 8 ++ > > Documentation/x86/intel_sgx.rst | 233 > > 3 files changed, 242 insertions(+) > > create mode 100644 Documentation/x86/index.rst > > create mode 100644 Documentation/x86/intel_sgx.rst > > > diff --git a/Documentation/x86/intel_sgx.rst > > b/Documentation/x86/intel_sgx.rst > > new file mode 100644 > > index ..f51b43f9e125 > > --- /dev/null > > +++ b/Documentation/x86/intel_sgx.rst > > @@ -0,0 +1,233 @@ > > +=== > > +Intel(R) SGX driver > > +=== > > + > > +Introduction > > + > > + > > +Intel(R) SGX is a set of CPU instructions that can be used by applications > > to > > +set aside private regions of code and data. The code outside the enclave is > > +disallowed to access the memory inside the enclave by the CPU access > > control. > > +In a way you can think that SGX provides inverted sandbox. It protects the > >provides an inverted sandbox. > > > +application from a malicious host. > > + > > +You can tell if your CPU supports SGX by looking into ``/proc/cpuinfo``: > > + > > + ``cat /proc/cpuinfo | grep sgx`` > > + > > +Overview of SGX > > +=== > > + > > +SGX has a set of data structures to maintain information about the > > enclaves and > > +their security properties. BIOS reserves a fixed size region of physical > > memory > > +for these structures by setting Processor Reserved Memory Range Registers > > +(PRMRR). > > + > > +This memory range is protected from outside access by the CPU and all the > > data > > +coming in and out of the CPU package is encrypted by a key that is > > generated for > > +each boot cycle. > > + > > +Enclaves execute in ring-3 in a special enclave submode using pages from > > the > >ring 3 > > > +reserved memory range. A fixed logical address range for the enclave is > > reserved > > +by ENCLS(ECREATE), a leaf instruction used to create enclaves. It is > > referred in > > > referred to in > > > +the documentation commonly as the ELRANGE. > > + > > +Every memory access to the ELRANGE is asserted by the CPU. If the CPU is > > not > > +executing in the enclave mode inside the enclave, #GP is raised. On the > > other > > +hand, enclave code can make memory accesses both inside and outside of the > > +ELRANGE. > > + > > +Enclave can only execute code inside the ELRANGE. Instructions that may > > cause > >An enclave can only > > > +VMEXIT, IO instructions and instructions that require a privilege change > > are > > +prohibited inside the enclave. Interrupts and exceptions always cause > > enclave > > always cause an > enclave > > > +to exit and jump to an address outside the enclave given when the enclave > > is > > +entered by using the leaf instruction ENCLS(EENTER). > > + > > +Protected memory > > + > > + > > +Enclave Page Cache (EPC) > > +Physical pages used with enclaves that are protected by the CPU from > > +unauthorized access. > > + > > +Enclave Page Cache Map (EPCM) > > +A database that describes the properties and state of the pages e.g. > > their > > +permissions or to which enclave they belong to. > > Drop one of those "to" words (either one). > > > + > > +Memory Encryption Engine (MEE) integrity tree > > +Autonomously updated integrity tree. The root of the tree located in > > on-die > > +SRAM. > > + > > +EPC data types > > +-- > > + > > +SGX Enclave Control Structure (SECS) > > +Describes the global properties of an enclave. Will not be mapped to > > the > > +ELRANGE. > > + > > +Regular (REG) > > +These pages contain code and data. > > + > > +Thread Control Structure (TCS) > > +The pages that define the entry points inside an enclave. An enclave > > can > > +only be entered through these entry points and each can host a single > > +hardware thread at a time. > > + > > +Version Array (VA) > > + The pages contain 64-bit version numbers for pages that have been > > swapped > > + outside the enclave. Each page has the capacity of 512 version numbers. > > + > > +Launch control > > +-- > > + > > +To launch an enclave, two structures must be provided for ENCLS(EINIT): > > + > > +1. **SIGSTRUCT:** signed measuremen
Re: [PATCH v4 1/2] Documentation/admin-guide: introduce perf-security.rst file
Hi Peter, On 27.11.2018 22:13, Alexey Budankov wrote: > On 27.11.2018 21:11, Jonathan Corbet wrote: >> On Tue, 27 Nov 2018 11:15:37 +0300 >> Alexey Budankov wrote: >> >>> +To perform security checks, the Linux implementation splits processes into >>> two >>> +categories [6]_ : a) privileged processes (whose effective user ID is 0, >>> referred >>> +to as superuser or root), and b) unprivileged processes (whose effective >>> UID is >>> +nonzero). Privileged processes bypass all kernel security permission >>> checks so >>> +perf_events performance monitoring is fully available to privileged >>> processes >>> +without access, scope and resource restrictions. >>> + >>> +Unprivileged processes are subject to a full security permission check >>> based on >>> +the process's credentials [5]_ (usually: effective UID, effective GID, and >>> +supplementary group list). >>> + >>> +Linux divides the privileges traditionally associated with superuser into >>> +distinct units, known as capabilities [6]_ , which can be independently >>> enabled >>> +and disabled on per-thread basis for processes and files of unprivileged >>> users. >>> + >>> +Unprivileged processes with enabled CAP_SYS_ADMIN capability are treated as >>> +privileged processes with respect to perf_events performance monitoring and >>> +bypass *scope* permissions checks in the kernel. >>> + >>> +Unprivileged processes using perf_events system call API is also subject >>> for >>> +PTRACE_MODE_READ_REALCREDS ptrace access mode check [7]_ , whose outcome >>> +determines whether monitoring is permitted. So unprivileged processes >>> provided >>> +with CAP_SYS_PTRACE capability are effectively permitted to pass the check. >> >> It's good to have more information here. I could certainly quibble >> further with things - a process with CAP_SYS_ADMIN is not "unprivileged"! >> - but I don't want to stand in the way of this any further. I *would* >> still like to see an ack from the perf world, though. > > Thanks for meaningful comments! Looking forward to ack from perf world. May I ask you to review v4 of the patches? Your comments on v1 have been addressed in there. Thanks, Alexey > >> >> With regard to Kees's comment on merging the two patches; I would probably >> add the new document to index.rst in the same patch, but it doesn't matter >> that much. Not worth redoing the patch just for that. > > Thanks, > Alexey > >> >> jon >> >
Re: [PATCH v3] code-of-conduct: Remove explicit list of discrimination factors
On Sun, Dec 02, 2018 at 10:32:57AM +0100, Geert Uytterhoeven wrote: > Providing an explicit list of discrimination factors may give the false > impression that discrimination based on other unlisted factors would be > allowed. > > Furthermore, this list is already overly long, polarizing, > politically-laden, and reinstating the concept of human races. > None of these is related to the goals of the Linux kernel project. > > Avoid any ambiguity or political undertone by removing the list, to > ensure "a harassment-free experience for everyone", period. I understand the reason you and others are proposing this change, however for now, let us stick with the text that we have. As Linus and I said just over a month ago, let's sit with the text we have until something comes up that requires a change to happen. Also, I recommend you work with the upstream developers of this text to see if they agree with your changes here. If they do, and update their version, I will be glad to revisit this text at that time. But everyone, please note that there are very specific reasons for listing things like this. You might not agree with those reasons, but many other people do so each "camp" can not be happy in the end. As we did have 3 years without such a list of factors, perhaps it is time to have 3 years with the list to provide a fair balance. :) So I'm not going to accept this patch at this point in time, sorry. thanks, greg k-h
Re: [PATCH 0/2] docs: memory-hotplug: add details about locking internals
On 12.10.18 19:21, Jonathan Corbet wrote: > On Thu, 11 Oct 2018 07:58:15 +0300 > Mike Rapoport wrote: > >> As discussed at [1], the latest updates to memory hotplug documentation are >> causing a conflict between docs and mmotm trees. >> These patches resolve the conflict. >> >> [1] https://lkml.org/lkml/2018/10/8/227 >> >> David Hildenbrand (1): >> docs/core-api: memory-hotplug: add some details about locking internals >> >> Mike Rapoport (1): >> docs/core-api: rename memory-hotplug-notifier to memory-hotplug > > I've applied the pair, thanks. > > jon > Looking at linux-next, we now have duplicate documentation: $ git grep mem_hotplug_lock Documentation/admin-guide/mm/memory-hotplug.rst: mem_hotplug_lock Documentation/admin-guide/mm/memory-hotplug.rst:- add_memory_resource() will first take the mem_hotplug_lock, followed by Documentation/admin-guide/mm/memory-hotplug.rst:heterogeneous/device memory, we should always hold the mem_hotplug_lock in Documentation/admin-guide/mm/memory-hotplug.rst:In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read Documentation/core-api/memory-hotplug.rst: mem_hotplug_lock Documentation/core-api/memory-hotplug.rst:- add_memory_resource() will first take the mem_hotplug_lock, followed by Documentation/core-api/memory-hotplug.rst:heterogeneous/device memory, we should always hold the mem_hotplug_lock in Documentation/core-api/memory-hotplug.rst:In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read It really only should go to Documentation/core-api/memory-hotplug.rst Should I send a patch or who can fix that up? -- Thanks, David / dhildenb
[RFC AFBC 01/12] drm/fourcc: Add AFBC yuv fourccs for Mali
From: Brian Starkey As we look to enable AFBC using DRM format modifiers, we run into problems which we've historically handled via vendor-private details (i.e. gralloc, on Android). AFBC (as an encoding) is fully flexible, and for example YUV data can be encoded into 1, 2 or 3 encoded "planes", much like the linear equivalents. Component order is also meaningful, as AFBC doesn't necessarily care about what each "channel" of the data it encodes contains. Therefore ABGR and RGBA can be encoded in AFBC with different representations. Similarly, 'X' components may be encoded into AFBC streams in cases where a decoder expects to decode a 4th component. In addition, AFBC is a licensable IP, meaning that to support the ecosystem we need to ensure that _all_ AFBC users are able to describe the encodings that they need. This is much better achieved by preserving meaning in the fourcc codes when they are combined with an AFBC modifier. In essence, we want to use the modifier to describe the parameters of the AFBC encode/decode, and use the fourcc code to describe the data being encoded/decoded. To do anything different would be to introduce redundancy - we would need to duplicate in the modifier information which is _already_ conveyed clearly and non-ambigiously by a fourcc code. I hope that for RGB this is non-controversial. (BGRA + MODIFIER_AFBC) is a different format from (RGBA + MODIFIER_AFBC). Possibly more controversial is that (XBGR + MODIFIER_AFBC) is different from (BGR888 + MODIFIER_AFBC). I understand that in some schemes it is not the case - but in AFBC it is so. Where we run into problems is where there are not already fourcc codes which represent the data which the AFBC encoder/decoder is processing. To that end, we want to introduce new fourcc codes to describe the data being encoded/decoded, in the places where none of the existing fourcc codes are applicable. Where we don't support an equivalent non-compressed layout, or where no "obvious" linear layout exists, we are proposing adding fourcc codes which have no associated linear layout - because any layout we proposed would be completely arbitrary. Some formats are following the naming conventions from [2]. The summary of the new formats is: DRM_FORMAT_VUY888 - Packed 8-bit YUV 444. Y followed by U then V. DRM_FORMAT_VUY101010 - Packed 10-bit YUV 444. Y followed by U then V. No defined linear encoding. DRM_FORMAT_Y210 - Packed 10-bit YUV 422. Y followed by U (then Y) then V. 10-bit samples in 16-bit words. DRM_FORMAT_Y410 - Packed 10-bit YUV 444, with 2-bit alpha. DRM_FORMAT_P210 - Semi-planar 10-bit YUV 422. Y plane, followed by interleaved U-then-V plane. 10-bit samples in 16-bit words. DRM_FORMAT_YUV420_8BIT - Packed 8-bit YUV 420. Y followed by U then V. No defined linear encoding DRM_FORMAT_YUV420_10BIT - Packed 10-bit YUV 420. Y followed by U then V. No defined linear encoding Please also note that in the absence of AFBC, we would still need to add Y410, Y210 and P210. Full rationale follows: YUV 444 8-bit, 1-plane -- The currently defined AYUV format encodes a 4th alpha component, which makes it unsuitable for representing a 3-component YUV 444 AFBC stream. The proposed[1] XYUV format which is supported by Mali-DP in linear layout is also unsuitable, because the component order is the opposite of the AFBC version, and it encodes a 4th 'X' component. DRM_FORMAT_VUY888 is the "obvious" format for a 3-component, packed, YUV 444 8-bit format, with the component order which our HW expects to encode/decode. It conforms to the same naming convention as the existing packed YUV 444 format. The naming here is meant to be consistent with DRM_FORMAT_AYUV and DRM_FORMAT_XYUV[1] YUV 444 10-bit, 1-plane --- There is no currently-defined YUV 444 10-bit format in drm_fourcc.h, irrespective of number of planes. The proposed[1] XVYU2101010 format which is supported by Mali-DP in linear layout uses the wrong component order, and also encodes a 4th 'X' component, which doesn't match the AFBC version of YUV 444 10-bit which we support. DRM_FORMAT_Y410 is the same layout as XVYU2101010, but with 2 bits of alpha. This format is supported with linear layout by Mali GPUs. The naming follows[2]. There is no "obvious" linear encoding for a 3-component 10:10:10 packed format, and so DRM_FORMAT_VUY101010 defines a component order, but not a bit encoding. Again, the naming is meant to be consistent with DRM_FORMAT_AYUV. YUV 422 8-bit, 1-plane -- The existing DRM_FORMAT_YUYV (and the other component orders) are single-planar YUV 422 8-bit formats. Following the convention of the component orders of the RGB formats, YUYV has the correct component order for our AFBC encoding (Y followed by U followed by V)
[RFC AFBC 11/12] drm/arm/malidp:- Disregard the pitch alignment constraint for AFBC framebuffer.
Considering the fact that some of the AFBC specific pixel formats are expressed
in bits per pixel (ie bpp which is not byte aligned), the pitch (ie width * bpp)
is not guaranteed to be aligned to burst size (ie 8 or 16 bytes).
For example, DRM_FORMAT_VUY101010 is 30 bits per pixel. For a framebuffer of
width 32 pixels, the pitch will be 120 bytes which is not aligned to burst size
(ie 16 bytes) for DP650.
Signed-off-by: Ayan Kumar halder
---
drivers/gpu/drm/arm/malidp_planes.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c
b/drivers/gpu/drm/arm/malidp_planes.c
index d0a00ee..eec0442 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -529,8 +529,8 @@ static int malidp_de_plane_check(struct drm_plane *plane,
for (i = 0; i < ms->n_planes; i++) {
u8 alignment = malidp_hw_get_pitch_align(mp->hwdev, rotated);
- if ((fb->pitches[i] * drm_format_info_block_height(fb->format,
i))
- & (alignment - 1)) {
+ if (((fb->pitches[i] * drm_format_info_block_height(fb->format,
i))
+ & (alignment - 1)) && !(fb->modifier)) {
DRM_DEBUG_KMS("Invalid pitch %u for plane %d\n",
fb->pitches[i], i);
return -EINVAL;
--
2.7.4
[RFC v3 AFBC 12/12] drm/arm/malidp: Added support for AFBC modifiers for all layers except DE_SMART
The list of modifiers to be supported for each plane has been dynamically
generated
from 'malidp_format_modifiers[]' and 'malidp_hw_regmap->features'.
Changes from v1:-
1. Replaced DRM_ERROR() with DRM_DEBUG_KMS() in malidp_format_mod_supported()
to report unsupported modifiers.
Changes from v2:-
1. Removed malidp_format_mod_supported() from the current patch. This has been
added
in "PATCH 7/12"
2. Dynamically generate the list of modifiers (to be supported for each plane)
from
'malidp_format_modifiers' and features.
Signed-off-by: Ayan Kumar halder
---
drivers/gpu/drm/arm/malidp_drv.c| 1 +
drivers/gpu/drm/arm/malidp_planes.c | 28 ++--
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index b2b97db..be45703 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -388,6 +388,7 @@ static int malidp_init(struct drm_device *drm)
drm->mode_config.max_height = hwdev->max_line_size;
drm->mode_config.funcs = &malidp_mode_config_funcs;
drm->mode_config.helper_private = &malidp_mode_config_helpers;
+ drm->mode_config.allow_fb_modifiers = true;
ret = malidp_crtc_init(drm);
if (ret)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c
b/drivers/gpu/drm/arm/malidp_planes.c
index eec0442..01037d0 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -934,6 +934,25 @@ int malidp_de_planes_init(struct drm_device *drm)
BIT(DRM_MODE_BLEND_COVERAGE);
u32 *formats;
int ret, i, j, n;
+ u64 supported_modifiers[MODIFIERS_COUNT_MAX];
+ const u64 *modifiers;
+
+ modifiers = malidp_format_modifiers;
+
+ if (!(map->features & MALIDP_DEVICE_AFBC_SUPPORT_SPLIT)) {
+ /*
+* Since our hardware does not support SPLIT, so build the list
of
+* supported modifiers excluding SPLIT ones.
+*/
+ while (*modifiers != DRM_FORMAT_MOD_INVALID) {
+ if (!(*modifiers & AFBC_SPLIT))
+ supported_modifiers[j++] = *modifiers;
+
+ modifiers++;
+ }
+ supported_modifiers[j++] = DRM_FORMAT_MOD_INVALID;
+ modifiers = supported_modifiers;
+ }
formats = kcalloc(map->n_pixel_formats, sizeof(*formats), GFP_KERNEL);
if (!formats) {
@@ -958,9 +977,14 @@ int malidp_de_planes_init(struct drm_device *drm)
plane_type = (i == 0) ? DRM_PLANE_TYPE_PRIMARY :
DRM_PLANE_TYPE_OVERLAY;
+
+ /*
+* All the layers except smart layer supports AFBC modifiers.
+*/
ret = drm_universal_plane_init(drm, &plane->base, crtcs,
- &malidp_de_plane_funcs, formats,
- n, NULL, plane_type, NULL);
+ &malidp_de_plane_funcs, formats, n,
+ (id == DE_SMART) ? NULL : modifiers,
plane_type, NULL);
+
if (ret < 0)
goto cleanup;
--
2.7.4
[RFC AFBC 05/12] drm/arm/malidp:- Define a common list of AFBC format modifiers supported for DP500, DP550 and DP650
We need to define a common list of format modifiers supported by each of the
Mali
display processors. The difference between DP500 from DP550/650 is that DP500
does not support block split mode (ie AFBC_FORMAT_MOD_SPLIT) and DP550 supports
YUV420 with split mode. We noted these special cases by defining
MALIDP_DEVICE_AFBC_SUPPORT_SPLIT
and AFBC_SUPPORT_SPLIT_WITH_YUV_420_10 for malidp_hw_regmap.features
Also we have defined a set of meaningful macros to shorten the modifier names
Signed-off-by: Ayan Kumar halder
Change-Id: I09fba2032a7474e6ce45af230e0ed18fc1f4c1df
---
drivers/gpu/drm/arm/malidp_drv.c | 8
drivers/gpu/drm/arm/malidp_hw.c | 30 --
drivers/gpu/drm/arm/malidp_hw.h | 20 +++-
3 files changed, 47 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 505f316..b8db92f 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -293,8 +293,8 @@ malidp_verify_afbc_framebuffer_caps(struct drm_device *dev,
return false;
}
- switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
- case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
+ switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
+ case AFBC_SIZE_16X16:
if ((mode_cmd->width % 16) || (mode_cmd->height % 16)) {
DRM_DEBUG_KMS("AFBC buffers must be aligned to 16
pixels\n");
return false;
@@ -319,8 +319,8 @@ malidp_verify_afbc_framebuffer_size(struct drm_device *dev,
u32 afbc_superblock_size = 0, afbc_superblock_height = 0;
u32 afbc_superblock_width = 0, afbc_size = 0;
- switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
- case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
+ switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
+ case AFBC_SIZE_16X16:
afbc_superblock_height = 16;
afbc_superblock_width = 16;
break;
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 87b7b12..55d379b 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -137,6 +137,32 @@ static const struct malidp_layer malidp650_layers[] = {
ROTATE_NONE, 0 },
};
+const u64 malidp_format_modifiers[] = {
+ /* All RGB formats (except XRGB, RGBX, XBGR, BGRX) */
+ DRM_FORMAT_MOD_ARM_AFBC(AFBC_SIZE_16X16 | AFBC_YTR | AFBC_SPARSE),
+ DRM_FORMAT_MOD_ARM_AFBC(AFBC_SIZE_16X16 | AFBC_YTR),
+
+ /* All RGB formats > 16bpp (except XRGB, RGBX, XBGR, BGRX) */
+ DRM_FORMAT_MOD_ARM_AFBC(AFBC_SIZE_16X16 | AFBC_YTR | AFBC_SPARSE |
AFBC_SPLIT),
+
+ /* All 8 or 10 bit YUV 444 formats. */
+ /* In DP550, 10 bit YUV 420 format also supported */
+ DRM_FORMAT_MOD_ARM_AFBC(AFBC_SIZE_16X16 | AFBC_SPARSE | AFBC_SPLIT),
+
+ /* YUV 420, 422 P1 8 bit and YUV 444 8 bit/10 bit formats */
+ DRM_FORMAT_MOD_ARM_AFBC(AFBC_SIZE_16X16 | AFBC_SPARSE),
+ DRM_FORMAT_MOD_ARM_AFBC(AFBC_SIZE_16X16),
+
+ /* YUV 420, 422 P1 8, 10 bit formats */
+ DRM_FORMAT_MOD_ARM_AFBC(AFBC_SIZE_16X16 | AFBC_CBR | AFBC_SPARSE),
+ DRM_FORMAT_MOD_ARM_AFBC(AFBC_SIZE_16X16 | AFBC_CBR),
+
+ /* All formats */
+ DRM_FORMAT_MOD_LINEAR,
+
+ DRM_FORMAT_MOD_INVALID
+};
+
#define SE_N_SCALING_COEFFS96
static const u16 dp500_se_scaling_coeffs[][SE_N_SCALING_COEFFS] = {
[MALIDP_UPSCALING_COEFFS - 1] = {
@@ -841,7 +867,7 @@ const struct malidp_hw malidp_device[MALIDP_MAX_DEVICES] = {
.se_base = MALIDP550_SE_BASE,
.dc_base = MALIDP550_DC_BASE,
.out_depth_base = MALIDP550_DE_OUTPUT_DEPTH,
- .features = MALIDP_REGMAP_HAS_CLEARIRQ,
+ .features = MALIDP_REGMAP_HAS_CLEARIRQ |
MALIDP_DEVICE_AFBC_SUPPORT_SPLIT | AFBC_SUPPORT_SPLIT_WITH_YUV_420_10,
.n_layers = ARRAY_SIZE(malidp550_layers),
.layers = malidp550_layers,
.de_irq_map = {
@@ -887,7 +913,7 @@ const struct malidp_hw malidp_device[MALIDP_MAX_DEVICES] = {
.se_base = MALIDP550_SE_BASE,
.dc_base = MALIDP550_DC_BASE,
.out_depth_base = MALIDP550_DE_OUTPUT_DEPTH,
- .features = MALIDP_REGMAP_HAS_CLEARIRQ,
+ .features = MALIDP_REGMAP_HAS_CLEARIRQ |
MALIDP_DEVICE_AFBC_SUPPORT_SPLIT,
.n_layers = ARRAY_SIZE(malidp650_layers),
.layers = malidp650_layers,
.de_irq_map = {
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 651558f..27b907f 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -95,7 +95,9 @@ struct malidp_se_config {
};
/* regm
[RFC AFBC 09/12] drm/arm/malidp:- Writeback framebuffer does not support any modifiers
In malidp, the writeback pipeline does not support writing crtc output
to a framebuffer with modifiers ie the memory writeback content is
devoid of any compression or tiling, etc.
So we have added a commit check in memory writeback encoder helper function
to validate if the framebuffer has any modifier and if so, return EINVAL.
Signed-off-by: Ayan Kumar halder
---
drivers/gpu/drm/arm/malidp_mw.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
index 0484744..90c964a 100644
--- a/drivers/gpu/drm/arm/malidp_mw.c
+++ b/drivers/gpu/drm/arm/malidp_mw.c
@@ -141,6 +141,11 @@ malidp_mw_encoder_atomic_check(struct drm_encoder *encoder,
return -EINVAL;
}
+ if (fb->modifier) {
+ DRM_DEBUG_KMS("Writeback framebuffer does not support
modifiers\n");
+ return -EINVAL;
+ }
+
mw_state->format =
malidp_hw_get_format_id(&malidp->dev->hw->map, SE_MEMWRITE,
fb->format->format, !!fb->modifier);
--
2.7.4
[RFC AFBC 08/12] drm/arm/malidp: Specified the rotation memory requirements for AFBC YUV formats
The newly supported AFBC YUV formats have the following rotation memory
constraints (in DP550/DP650).
1. DRM_FORMAT_VUY888/DRM_FORMAT_VUY101010 :- It can rotate upto 8
horizontal lines in the AFBC output buffer.
2. DRM_FORMAT_YUV420_8BIT :- It can rotate upto 16 horizontal lines
in the AFBC output buffer.
Also some of the pixel formats are specified in bits per pixel (rather
than bytes per pixel), so the calculation needs to take note of this.
Besides there are some difference between DP550 and DP650 and these are
as follows:-
1. DRM_FORMAT_X0L2 (in uncompressed format) does not support rotation in DP550.
For DP650, it can rotate upto 16 horizontal lines in the AFBC output buffer,
whereas in DP550 (with AFBC), it can rotate upto 8 horizontal lines.
2. DRM_FORMAT_YUV420_10BIT :- It can rotate upto 8 horizontal lines in
dp550 and 16 horizontal lines in DP650.
Signed-off-by: Ayan Kumar halder
---
drivers/gpu/drm/arm/malidp_hw.c | 101
drivers/gpu/drm/arm/malidp_hw.h | 5 +-
drivers/gpu/drm/arm/malidp_planes.c | 3 +-
3 files changed, 98 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 4a774be..d9866a8 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -375,14 +375,39 @@ static void malidp500_modeset(struct malidp_hw_device
*hwdev, struct videomode *
malidp_hw_clearbits(hwdev, MALIDP_DISP_FUNC_ILACED,
MALIDP_DE_DISPLAY_FUNC);
}
-static int malidp500_rotmem_required(struct malidp_hw_device *hwdev, u16 w,
u16 h, u32 fmt)
+int malidp_format_get_bpp(u32 fmt)
+{
+ int bpp = drm_format_plane_cpp(fmt, 0) * 8;
+
+ if (bpp == 0) {
+ switch (fmt) {
+ case DRM_FORMAT_VUY101010:
+ bpp = 30;
+ case DRM_FORMAT_YUV420_10BIT:
+ bpp = 15;
+ break;
+ case DRM_FORMAT_YUV420_8BIT:
+ bpp = 12;
+ break;
+ default:
+ bpp = 0;
+ }
+ }
+
+ return bpp;
+}
+
+static int malidp500_rotmem_required(struct malidp_hw_device *hwdev, u16 w,
+u16 h, u32 fmt, bool has_modifier)
{
/*
* Each layer needs enough rotation memory to fit 8 lines
* worth of pixel data. Required size is then:
*size = rotated_width * (bpp / 8) * 8;
*/
- return w * drm_format_plane_cpp(fmt, 0) * 8;
+ int bpp = malidp_format_get_bpp(fmt);
+
+ return w * bpp;
}
static void malidp500_se_write_pp_coefftab(struct malidp_hw_device *hwdev,
@@ -660,9 +685,9 @@ static void malidp550_modeset(struct malidp_hw_device
*hwdev, struct videomode *
malidp_hw_clearbits(hwdev, MALIDP_DISP_FUNC_ILACED,
MALIDP_DE_DISPLAY_FUNC);
}
-static int malidp550_rotmem_required(struct malidp_hw_device *hwdev, u16 w,
u16 h, u32 fmt)
+static int malidpx50_get_bytes_per_column(u32 fmt)
{
- u32 bytes_per_col;
+ u32 bytes_per_column;
switch (fmt) {
/* 8 lines at 4 bytes per pixel */
@@ -688,19 +713,77 @@ static int malidp550_rotmem_required(struct
malidp_hw_device *hwdev, u16 w, u16
case DRM_FORMAT_UYVY:
case DRM_FORMAT_YUYV:
case DRM_FORMAT_X0L0:
- case DRM_FORMAT_X0L2:
- bytes_per_col = 32;
+ bytes_per_column = 32;
break;
/* 16 lines at 1.5 bytes per pixel */
case DRM_FORMAT_NV12:
case DRM_FORMAT_YUV420:
- bytes_per_col = 24;
+ /* 8 lines at 3 bytes per pixel */
+ case DRM_FORMAT_VUY888:
+ /* 16 lines at 12 bits per pixel */
+ case DRM_FORMAT_YUV420_8BIT:
+ /* 8 lines at 3 bytes per pixel */
+ case DRM_FORMAT_P010:
+ bytes_per_column = 24;
+ break;
+ /* 8 lines at 30 bits per pixel */
+ case DRM_FORMAT_VUY101010:
+ /* 16 lines at 15 bits per pixel */
+ case DRM_FORMAT_YUV420_10BIT:
+ bytes_per_column = 30;
break;
default:
return -EINVAL;
}
- return w * bytes_per_col;
+ return bytes_per_column;
+}
+
+static int malidp550_rotmem_required(struct malidp_hw_device *hwdev, u16 w,
+u16 h, u32 fmt, bool has_modifier)
+{
+ int bytes_per_column = 0;
+
+ switch (fmt) {
+ /* 8 lines at 15 bits per pixel */
+ case DRM_FORMAT_YUV420_10BIT:
+ bytes_per_column = 15;
+ break;
+ /* Uncompressed YUV 420 10 bit single plane cannot be rotated */
+ case DRM_FORMAT_X0L2:
+ if (has_modifier)
+ bytes_per_column = 8;
+ else
+ return -EINVAL;
+ break;
+ default:
+ bytes_per_column = malidpx50
[RFC AFBC 10/12] drm/arm/malidp:- Use the newly introduced malidp_format_get_bpp() instead of relying on cpp for calculating framebuffer size
Formats like DRM_FORMAT_VUY101010, DRM_FORMAT_YUV420_8BIT and
DRM_FORMAT_YUV420_10BIT are expressed in bits per pixel as they have a non
integer value of cpp (thus denoted as '0' in drm_format_info[]). Therefore,
the calculation of AFBC framebuffer size needs to use malidp_format_get_bpp().
Signed-off-by: Ayan Kumar halder
---
drivers/gpu/drm/arm/malidp_drv.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 2f0b553..b2b97db 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -297,6 +297,7 @@ malidp_verify_afbc_framebuffer_size(struct drm_device *dev,
struct drm_gem_object *objs = NULL;
u32 afbc_superblock_size = 0, afbc_superblock_height = 0;
u32 afbc_superblock_width = 0, afbc_size = 0;
+ int bpp = 0;
switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
case AFBC_SIZE_16X16:
@@ -313,15 +314,17 @@ malidp_verify_afbc_framebuffer_size(struct drm_device
*dev,
n_superblocks = (mode_cmd->width / afbc_superblock_width) *
(mode_cmd->height / afbc_superblock_height);
- afbc_superblock_size = info->cpp[0] * afbc_superblock_width *
- afbc_superblock_height;
+ bpp = malidp_format_get_bpp(info->format);
+
+ afbc_superblock_size = (bpp * afbc_superblock_width *
afbc_superblock_height) / BITS_PER_BYTE;
afbc_size = ALIGN(n_superblocks * AFBC_HEADER_SIZE,
AFBC_SUPERBLK_ALIGNMENT);
afbc_size += n_superblocks * ALIGN(afbc_superblock_size,
AFBC_SUPERBLK_ALIGNMENT);
- if (mode_cmd->width * info->cpp[0] != mode_cmd->pitches[0]) {
- DRM_DEBUG_KMS("Invalid value of pitch (=%u) should be same as
width (=%u) * cpp (=%u)\n",
- mode_cmd->pitches[0], mode_cmd->width,
info->cpp[0]);
+ if ((mode_cmd->width * bpp) != (mode_cmd->pitches[0] * BITS_PER_BYTE)) {
+ DRM_DEBUG_KMS("Invalid value of (pitch * BITS_PER_BYTE) (=%u) "
+ "should be same as width (=%u) * bpp (=%u)\n",
+ (mode_cmd->pitches[0] * BITS_PER_BYTE),
mode_cmd->width, bpp);
return false;
}
--
2.7.4
[RFC AFBC 06/12] drm/arm/malidp:- Added support for new YUV formats for DP500, DP550 and DP650
We have added some new formats to be supported on DP500/DP550/DP650.
Signed-off-by: Ayan Kumar Halder
Depends on :- https://patchwork.kernel.org/patch/10460063/
---
drivers/gpu/drm/arm/malidp_hw.c | 22 +-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 55d379b..25ac5890 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -49,6 +49,12 @@ static const struct malidp_format_id malidp500_de_formats[]
= {
{ DRM_FORMAT_YUYV, DE_VIDEO1, 13 },
{ DRM_FORMAT_NV12, DE_VIDEO1 | SE_MEMWRITE, 14 },
{ DRM_FORMAT_YUV420, DE_VIDEO1, 15 },
+ { DRM_FORMAT_XYUV, DE_VIDEO1, 16 },
+ /* These are supported with AFBC only */
+ { DRM_FORMAT_YUV420_8BIT, DE_VIDEO1, 14 },
+ { DRM_FORMAT_VUY888, DE_VIDEO1, 16 },
+ { DRM_FORMAT_VUY101010, DE_VIDEO1, 17 },
+ { DRM_FORMAT_YUV420_10BIT, DE_VIDEO1, 18 }
};
#define MALIDP_ID(__group, __format) \
@@ -74,11 +80,25 @@ static const struct malidp_format_id malidp500_de_formats[]
= {
{ DRM_FORMAT_ABGR1555, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2,
MALIDP_ID(4, 1) }, \
{ DRM_FORMAT_RGB565, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(4,
2) }, \
{ DRM_FORMAT_BGR565, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(4,
3) }, \
+ /* This is only supported with linear modifier */ \
+ { DRM_FORMAT_XYUV, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(5, 0) },\
+ /* This is only supported with AFBC modifier */ \
+ { DRM_FORMAT_VUY888, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(5, 0) }, \
{ DRM_FORMAT_YUYV, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(5, 2) },\
+ /* This is only supported with linear modifier */ \
{ DRM_FORMAT_UYVY, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(5, 3) },\
{ DRM_FORMAT_NV12, DE_VIDEO1 | DE_VIDEO2 | SE_MEMWRITE, MALIDP_ID(5, 6)
}, \
+ /* This is only supported with AFBC modifier */ \
+ { DRM_FORMAT_YUV420_8BIT, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(5, 6) }, \
{ DRM_FORMAT_YUV420, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(5, 7) }, \
- { DRM_FORMAT_X0L2, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(6, 6)}
+ /* This is only supported with linear modifier */ \
+ { DRM_FORMAT_XVYU2101010, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(6, 0)}, \
+ /* This is only supported with AFBC modifier */ \
+ { DRM_FORMAT_VUY101010, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(6, 0)}, \
+ { DRM_FORMAT_X0L2, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(6, 6)}, \
+ /* This is only supported with AFBC modifier */ \
+ { DRM_FORMAT_YUV420_10BIT, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(6, 7)}, \
+ { DRM_FORMAT_P010, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(6, 7)}
static const struct malidp_format_id malidp550_de_formats[] = {
MALIDP_COMMON_FORMATS,
--
2.7.4
[RFC AFBC 02/12] drm: Added a new format DRM_FORMAT_XVYU2101010
We have added a new format ie DRM_FORMAT_XVYU2101010 which is supported
by mali display driver.
Signed-off-by: Ayan Kumar halder
---
drivers/gpu/drm/drm_fourcc.c | 1 +
include/uapi/drm/drm_fourcc.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 6b7a62e..d31e1ae 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -229,6 +229,7 @@ const struct drm_format_info *__drm_format_info(u32 format)
{ .format = DRM_FORMAT_VUY888, .depth = 0,
.num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
{ .format = DRM_FORMAT_Y410,.depth = 0,
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true,
.is_yuv = true },
{ .format = DRM_FORMAT_AYUV,.depth = 0,
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true,
.is_yuv = true },
+ { .format = DRM_FORMAT_XVYU2101010, .depth = 0,
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
{ .format = DRM_FORMAT_P010,.depth = 0,
.num_planes = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2, .is_yuv = true },
{ .format = DRM_FORMAT_P012,.depth = 0,
.num_planes = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2, .is_yuv = true },
{ .format = DRM_FORMAT_P016,.depth = 0,
.num_planes = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2, .is_yuv = true },
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 71b2bc7..75c4b5a 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -153,6 +153,7 @@ extern "C" {
#define DRM_FORMAT_AYUVfourcc_code('A', 'Y', 'U', 'V') /*
[31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
#define DRM_FORMAT_XYUVfourcc_code('X', 'Y', 'U', 'V') /* [31:0]
X:Y:Cb:Cr 8:8:8:8 little endian */
+#define DRM_FORMAT_XVYU2101010 fourcc_code('X', 'V', '3', '0') /* [31:0]
X:Cr:Y:Cb 2:10:10:10 little endian */
#define DRM_FORMAT_VUY888 fourcc_code('V', 'U', '2', '4') /* [23:0]
Cr:Cb:Y 8:8:8 little endian */
#define DRM_FORMAT_Y410fourcc_code('Y', '4', '1', '0') /*
[31:0] A:Cr:Y:Cb 2:10:10:10 little endian */
#define DRM_FORMAT_VUY101010 fourcc_code('V', 'U', '3', '0') /* Y followed
by U then V, 10:10:10. Non-linear modifier only */
--
2.7.4
[RFC v3 AFBC 04/12] drm/arm/malidp: Set the AFBC register bits if the framebuffer has AFBC modifier
Added the AFBC decoder registers for DP500 , DP550 and DP650.
These registers control the processing of AFBC buffers. It controls various
features like AFBC decoder enable, lossless transformation and block split
as well as setting of the left, right, top and bottom cropping of AFBC buffers
(in number of pixels).
All the layers (except DE_SMART) support framebuffers with AFBC modifiers.
One needs to set the pixel values of the top, left, bottom and right cropping
for the AFBC framebuffer.
Cropping an AFBC framebuffer is controlled by the AFBC crop registers.
In that case, the layer input size registers should be configured with
framebuffer's dimensions and not with drm_plane_state source width/height
values (which is used for non AFBC framebuffer to denote cropping).
Changes from v1:
- Removed the "if (fb->modifier)" check from malidp_de_plane_update()
and added it in malidp_de_set_plane_afbc(). This will consolidate all the
AFBC specific register configurations in a single function ie
malidp_de_set_plane_afbc().
Changes from v2:
- For AFBC framebuffer, layer input size register should be set to
framebuffer's
width and height
Signed-off-by: Ayan Kumar Halder
---
drivers/gpu/drm/arm/malidp_hw.c | 25 +
drivers/gpu/drm/arm/malidp_hw.h | 2 +
drivers/gpu/drm/arm/malidp_planes.c | 109 +++-
drivers/gpu/drm/arm/malidp_regs.h | 20 +++
4 files changed, 130 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index b9bed11..87b7b12 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -94,11 +94,12 @@ static const struct malidp_layer malidp500_layers[] = {
* yuv2rgb matrix offset, mmu control register offset,
rotation_features
*/
{ DE_VIDEO1, MALIDP500_DE_LV_BASE, MALIDP500_DE_LV_PTR_BASE,
- MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, 0, ROTATE_ANY },
+ MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, 0, ROTATE_ANY,
+ MALIDP500_DE_LV_AD_CTRL },
{ DE_GRAPHICS1, MALIDP500_DE_LG1_BASE, MALIDP500_DE_LG1_PTR_BASE,
- MALIDP_DE_LG_STRIDE, 0, 0, ROTATE_ANY },
+ MALIDP_DE_LG_STRIDE, 0, 0, ROTATE_ANY, MALIDP500_DE_LG1_AD_CTRL
},
{ DE_GRAPHICS2, MALIDP500_DE_LG2_BASE, MALIDP500_DE_LG2_PTR_BASE,
- MALIDP_DE_LG_STRIDE, 0, 0, ROTATE_ANY },
+ MALIDP_DE_LG_STRIDE, 0, 0, ROTATE_ANY, MALIDP500_DE_LG2_AD_CTRL
},
};
static const struct malidp_layer malidp550_layers[] = {
@@ -106,13 +107,15 @@ static const struct malidp_layer malidp550_layers[] = {
* yuv2rgb matrix offset, mmu control register offset,
rotation_features
*/
{ DE_VIDEO1, MALIDP550_DE_LV1_BASE, MALIDP550_DE_LV1_PTR_BASE,
- MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, 0, ROTATE_ANY },
+ MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, 0, ROTATE_ANY,
+ MALIDP550_DE_LV1_AD_CTRL },
{ DE_GRAPHICS1, MALIDP550_DE_LG_BASE, MALIDP550_DE_LG_PTR_BASE,
- MALIDP_DE_LG_STRIDE, 0, 0, ROTATE_ANY },
+ MALIDP_DE_LG_STRIDE, 0, 0, ROTATE_ANY, MALIDP550_DE_LG_AD_CTRL
},
{ DE_VIDEO2, MALIDP550_DE_LV2_BASE, MALIDP550_DE_LV2_PTR_BASE,
- MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, 0, ROTATE_ANY },
+ MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, 0, ROTATE_ANY,
+ MALIDP550_DE_LV2_AD_CTRL },
{ DE_SMART, MALIDP550_DE_LS_BASE, MALIDP550_DE_LS_PTR_BASE,
- MALIDP550_DE_LS_R1_STRIDE, 0, 0, ROTATE_NONE },
+ MALIDP550_DE_LS_R1_STRIDE, 0, 0, ROTATE_NONE, 0 },
};
static const struct malidp_layer malidp650_layers[] = {
@@ -122,16 +125,16 @@ static const struct malidp_layer malidp650_layers[] = {
*/
{ DE_VIDEO1, MALIDP550_DE_LV1_BASE, MALIDP550_DE_LV1_PTR_BASE,
MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB,
- MALIDP650_DE_LV_MMU_CTRL, ROTATE_ANY },
+ MALIDP650_DE_LV_MMU_CTRL, ROTATE_ANY, MALIDP550_DE_LV1_AD_CTRL
},
{ DE_GRAPHICS1, MALIDP550_DE_LG_BASE, MALIDP550_DE_LG_PTR_BASE,
MALIDP_DE_LG_STRIDE, 0, MALIDP650_DE_LG_MMU_CTRL,
- ROTATE_COMPRESSED },
+ ROTATE_COMPRESSED, MALIDP550_DE_LG_AD_CTRL },
{ DE_VIDEO2, MALIDP550_DE_LV2_BASE, MALIDP550_DE_LV2_PTR_BASE,
MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB,
- MALIDP650_DE_LV_MMU_CTRL, ROTATE_ANY },
+ MALIDP650_DE_LV_MMU_CTRL, ROTATE_ANY, MALIDP550_DE_LV2_AD_CTRL
},
{ DE_SMART, MALIDP550_DE_LS_BASE, MALIDP550_DE_LS_PTR_BASE,
MALIDP550_DE_LS_R1_STRIDE, 0, MALIDP650_DE_LS_MMU_CTRL,
- ROTATE_NONE },
+ ROTATE_NONE, 0 },
};
#define SE_N_SCALING_COEFFS96
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 40155e2..651558f 100644
---
[RFC AFBC v2 00/12] Add support for Arm Framebuffer Compression(AFBC) in mali display driver
This patchset aims to add support for AFBC in mali display driver with the help of format modifiers. AFBC modifiers adds some constraints to framebuffer size, alignment, pitch, formats, etc In the previous patchset ie https://lists.freedesktop.org/archives/dri-devel/2018-June/180124.html I had illustrated how to add support for one format (ie BGR888) with one valid combination of AFBC modifiers ie AFBC_SIZE_16X16 | AFBC_YTR | AFBC_SPARSE Changes from v1:- In this current patchset, I have enabled the support for all AFBC modifiers (which are supported on DP500, DP550 and DP650) with all the pixel formats. Also, we have introduced some new pixel formats which are supported with AFBC modifiers only as well some other pixel formats which are supported with LINEAR only. Please feel free to let me know your thoughts. Depends on :- https://patchwork.kernel.org/patch/10460063/ Ayan Kumar Halder (10): drm: Added a new format DRM_FORMAT_XVYU2101010 drm/arm/malidp: Set the AFBC register bits if the framebuffer has AFBC modifier drm/arm/malidp:- Define a common list of AFBC format modifiers supported for DP500, DP550 and DP650 drm/arm/malidp:- Added support for new YUV formats for DP500, DP550 and DP650 drm/arm/malidp: Define the constraints on each supported drm_fourcc format for the AFBC modifiers. drm/arm/malidp: Specified the rotation memory requirements for AFBC YUV formats drm/arm/malidp:- Writeback framebuffer does not support any modifiers drm/arm/malidp:- Use the newly introduced malidp_format_get_bpp() instead of relying on cpp for calculating framebuffer size drm/arm/malidp:- Disregard the pitch alignment constraint for AFBC framebuffer. drm/arm/malidp: Added support for AFBC modifiers for all layers except DE_SMART Brian Starkey (2): drm/fourcc: Add AFBC yuv fourccs for Mali drm/afbc: Add AFBC modifier usage documentation Documentation/gpu/afbc.rst | 233 +++ Documentation/gpu/drivers.rst | 1 + MAINTAINERS | 1 + drivers/gpu/drm/arm/malidp_drv.c| 45 ++ drivers/gpu/drm/arm/malidp_drv.h| 6 + drivers/gpu/drm/arm/malidp_hw.c | 245 drivers/gpu/drm/arm/malidp_hw.h | 30 +++- drivers/gpu/drm/arm/malidp_mw.c | 7 +- drivers/gpu/drm/arm/malidp_planes.c | 268 +--- drivers/gpu/drm/arm/malidp_regs.h | 20 +++ drivers/gpu/drm/drm_fourcc.c| 11 ++ include/uapi/drm/drm_fourcc.h | 20 ++- 12 files changed, 799 insertions(+), 88 deletions(-) create mode 100644 Documentation/gpu/afbc.rst -- 2.7.4
[RFC AFBC 03/12] drm/afbc: Add AFBC modifier usage documentation
From: Brian Starkey AFBC is a flexible, proprietary, lossless compression protocol and format, with a number of defined DRM format modifiers. To facilitate consistency and compatibility between different AFBC producers and consumers, document the expectations for usage of the AFBC DRM format modifiers in a new .rst chapter. Signed-off-by: Brian Starkey Reviewed-by: Liviu Dudau --- Documentation/gpu/afbc.rst| 233 ++ Documentation/gpu/drivers.rst | 1 + MAINTAINERS | 1 + include/uapi/drm/drm_fourcc.h | 3 + 4 files changed, 238 insertions(+) create mode 100644 Documentation/gpu/afbc.rst diff --git a/Documentation/gpu/afbc.rst b/Documentation/gpu/afbc.rst new file mode 100644 index 000..922d955 --- /dev/null +++ b/Documentation/gpu/afbc.rst @@ -0,0 +1,233 @@ +=== + Arm Framebuffer Compression (AFBC) +=== + +AFBC is a proprietary lossless image compression protocol and format. +It provides fine-grained random access and minimizes the amount of +data transferred between IP blocks. + +AFBC can be enabled on drivers which support it via use of the AFBC +format modifiers defined in drm_fourcc.h. See DRM_FORMAT_MOD_ARM_AFBC(*). + +All users of the AFBC modifiers must follow the usage guidelines laid +out in this document, to ensure compatibility across different AFBC +producers and consumers. + +Components and Ordering +=== + +AFBC streams can contain several components - where a component +corresponds to a color channel (i.e. R, G, B, X, A, Y, Cb, Cr). +The assignment of input/output color channels must be consistent +between the encoder and the decoder for correct operation, otherwise +the consumer will interpret the decoded data incorrectly. + +Furthermore, when the lossless colorspace transform is used +(AFBC_FORMAT_MOD_YTR, which should be enabled for RGB buffers for +maximum compression efficiency), the component order must be: + + * Component 0: R + * Component 1: G + * Component 2: B + +The component ordering is communicated via the fourcc code in the +fourcc:modifier pair. In general, component '0' is considered to +reside in the least-significant bits of the corresponding linear +format. For example, COMP(bits): + + * DRM_FORMAT_ABGR + + * Component 0: R(8) + * Component 1: G(8) + * Component 2: B(8) + * Component 3: A(8) + + * DRM_FORMAT_BGR888 + + * Component 0: R(8) + * Component 1: G(8) + * Component 2: B(8) + + * DRM_FORMAT_YUYV + + * Component 0: Y(8) + * Component 1: Cb(8, 2x1 subsampled) + * Component 2: Cr(8, 2x1 subsampled) + +In AFBC, 'X' components are not treated any differently from any other +component. Therefore, an AFBC buffer with fourcc DRM_FORMAT_XBGR +encodes with 4 components, like so: + + * DRM_FORMAT_XBGR + + * Component 0: R(8) + * Component 1: G(8) + * Component 2: B(8) + * Component 3: X(8) + +Please note, however, that the inclusion of a "wasted" 'X' channel is +bad for compression efficiency, and so it's recommended to avoid +formats containing 'X' bits. If a fourth component is +required/expected by the encoder/decoder, then it is recommended to +instead use an equivalent format with alpha, setting all alpha bits to +'1'. If there is no requirement for a fourth component, then a format +which doesn't include alpha can be used, e.g. DRM_FORMAT_BGR888. + +Number of Planes + + +Formats which are typically multi-planar in linear layouts (e.g. YUV +420), can be encoded into one, or multiple, AFBC planes. As with +component order, the encoder and decoder must agree about the number +of planes in order to correctly decode the buffer. The fourcc code is +used to determine the number of encoded planes in an AFBC buffer, +matching the number of planes for the linear (unmodified) format. +Within each plane, the component ordering also follows the fourcc +code: + +For example: + + * DRM_FORMAT_YUYV: nplanes = 1 + + * Plane 0: + + * Component 0: Y(8) + * Component 1: Cb(8, 2x1 subsampled) + * Component 2: Cr(8, 2x1 subsampled) + + * DRM_FORMAT_NV12: nplanes = 2 + + * Plane 0: + + * Component 0: Y(8) + + * Plane 1: + + * Component 0: Cb(8, 2x1 subsampled) + * Component 1: Cr(8, 2x1 subsampled) + +Cross-device interoperability += + +For maximum compatibility across devices, the table below defines +canonical formats for use between AFBC-enabled devices. Formats which +are listed here must be used exactly as specified when using the AFBC +modifiers. Formats which are not listed should be avoided. + +.. flat-table:: AFBC formats + + * - Fourcc code + - Description + - Planes/Components + + * - DRM_FORMAT_ABGR2101010 + - 10-bit per component RGB, with 2-bit alpha + - Plane 0: 4 components + * Component 0: R(10) + * Component 1: G(10) + * Component 2: B(10) +
[RFC AFBC 07/12] drm/arm/malidp: Define the constraints on each supported drm_fourcc format for the AFBC modifiers.
The constraints are as follows (for Mali-DP 500, 550, 650) :-
1. AFBC is not supported for the formats defined in
malidp_hw_format_is_linear_only()
2. Some of the formats are supported only with AFBC modifiers. Thus we have
introduced a new function 'malidp_hw_format_is_afbc_only()' which verifies the
same.
3. AFBC_FORMAT_MOD_YTR needs to be provided for any RGB format.
4. Formats <= 16bpp cannot support AFBC_FORMAT_MOD_SPLIT.
5. CBR should not be set for non-subsampled formats.
6. SMART layer does not support framebuffer with AFBC modifiers.
Return -EINVAL for such a scenario.
7. AFBC_FORMAT_MOD_YTR is not supported for any YUV formats.
8. Formats which are subsampled cannot support AFBC_FORMAT_MOD_SPLIT. However in
DP550, YUV_420_10BIT is supported with AFBC_FORMAT_MOD_SPLIT. This feature has
been identified with MALIDP_DEVICE_AFBC_YUV_420_10_SUPPORT_SPLIT.
9. In DP550 and DP650, for YUYV, the hardware supports different format-ids to
be used with and without AFBC modifier. We have used the feature
'MALIDP_DEVICE_AFBC_YUYV_USE_422_P2' to identify this characteristic.
Signed-off-by: Ayan Kumar halder
---
drivers/gpu/drm/arm/malidp_drv.c| 23 +--
drivers/gpu/drm/arm/malidp_drv.h| 6 ++
drivers/gpu/drm/arm/malidp_hw.c | 71 +++--
drivers/gpu/drm/arm/malidp_hw.h | 5 +-
drivers/gpu/drm/arm/malidp_mw.c | 2 +-
drivers/gpu/drm/arm/malidp_planes.c | 124 +++-
6 files changed, 199 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index b8db92f..2f0b553 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -264,29 +264,8 @@ static bool
malidp_verify_afbc_framebuffer_caps(struct drm_device *dev,
const struct drm_mode_fb_cmd2 *mode_cmd)
{
- const struct drm_format_info *info;
-
- if ((mode_cmd->modifier[0] >> 56) != DRM_FORMAT_MOD_VENDOR_ARM) {
- DRM_DEBUG_KMS("Unknown modifier (not Arm)\n");
+ if (malidp_format_mod_supported(dev, mode_cmd->pixel_format,
mode_cmd->modifier[0]) == false)
return false;
- }
-
- if (mode_cmd->modifier[0] &
- ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_MOD_VALID_BITS)) {
- DRM_DEBUG_KMS("Unsupported modifiers\n");
- return false;
- }
-
- info = drm_get_format_info(dev, mode_cmd);
- if (!info) {
- DRM_DEBUG_KMS("Unable to get the format information\n");
- return false;
- }
-
- if (info->num_planes != 1) {
- DRM_DEBUG_KMS("AFBC buffers expect one plane\n");
- return false;
- }
if (mode_cmd->offsets[0] != 0) {
DRM_DEBUG_KMS("AFBC buffers' plane offset should be 0\n");
diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
index b76c86f..019a682 100644
--- a/drivers/gpu/drm/arm/malidp_drv.h
+++ b/drivers/gpu/drm/arm/malidp_drv.h
@@ -90,6 +90,12 @@ struct malidp_crtc_state {
int malidp_de_planes_init(struct drm_device *drm);
int malidp_crtc_init(struct drm_device *drm);
+bool malidp_hw_format_is_linear_only(u32 format);
+bool malidp_hw_format_is_afbc_only(u32 format);
+
+bool malidp_format_mod_supported(struct drm_device *drm,
+u32 format, u64 modifier);
+
#ifdef CONFIG_DEBUG_FS
void malidp_error(struct malidp_drm *malidp,
struct malidp_error_stats *error_stats, u32 status,
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 25ac5890..4a774be 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -60,6 +60,8 @@ static const struct malidp_format_id malidp500_de_formats[] =
{
#define MALIDP_ID(__group, __format) \
__group) & 0x7) << 3) | ((__format) & 0x7))
+#define AFBC_YUV_422_FORMAT_ID MALIDP_ID(5, 1)
+
#define MALIDP_COMMON_FORMATS \
/*fourcc, layers supporting the format, internal id */ \
{ DRM_FORMAT_ARGB2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 |
SE_MEMWRITE, MALIDP_ID(0, 0) }, \
@@ -887,7 +889,10 @@ const struct malidp_hw malidp_device[MALIDP_MAX_DEVICES] =
{
.se_base = MALIDP550_SE_BASE,
.dc_base = MALIDP550_DC_BASE,
.out_depth_base = MALIDP550_DE_OUTPUT_DEPTH,
- .features = MALIDP_REGMAP_HAS_CLEARIRQ |
MALIDP_DEVICE_AFBC_SUPPORT_SPLIT | AFBC_SUPPORT_SPLIT_WITH_YUV_420_10,
+ .features = MALIDP_REGMAP_HAS_CLEARIRQ |
+ MALIDP_DEVICE_AFBC_SUPPORT_SPLIT |
+ MALIDP_DEVICE_AFBC_YUV_420_10_SUPPORT_SPLIT
|
+ MALIDP_DEVICE_AFBC_YUYV_USE_422_P2,
.n_layers = ARRAY_SIZE(malidp550_layers),
.layers = malidp550_layers,
Re: [PATCH v3] code-of-conduct: Remove explicit list of discrimination factors
Hi Greg,
On Mon, Dec 3, 2018 at 12:05 PM Greg Kroah-Hartman
wrote:
> On Sun, Dec 02, 2018 at 10:32:57AM +0100, Geert Uytterhoeven wrote:
> > Providing an explicit list of discrimination factors may give the false
> > impression that discrimination based on other unlisted factors would be
> > allowed.
> >
> > Furthermore, this list is already overly long, polarizing,
> > politically-laden, and reinstating the concept of human races.
> > None of these is related to the goals of the Linux kernel project.
> >
> > Avoid any ambiguity or political undertone by removing the list, to
> > ensure "a harassment-free experience for everyone", period.
>
> I understand the reason you and others are proposing this change,
> however for now, let us stick with the text that we have. As Linus and
> I said just over a month ago, let's sit with the text we have until
> something comes up that requires a change to happen.
>
> Also, I recommend you work with the upstream developers of this text to
> see if they agree with your changes here. If they do, and update their
> version, I will be glad to revisit this text at that time.
I did, cfr.
https://github.com/ContributorCovenant/contributor_covenant/issues/610
The official response was:
"I'm not going to make this change to the Contributor Covenant itself,
since I believe that explicitly listing examples of protected classes is
important. However, any adopting project is free to modify the document
according to the license."
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v8 1/2] perf, uncore: Adding documentation for ThunderX2 pmu uncore driver
On Thu, Nov 22, 2018 at 03:04:31AM +, Kulkarni, Ganapatrao wrote: > The SoC has PMU support in its L3 cache controller (L3C) and in the > DDR4 Memory Controller (DMC). > > Signed-off-by: Ganapatrao Kulkarni > --- > Documentation/perf/thunderx2-pmu.txt | 106 +++ > 1 file changed, 106 insertions(+) > create mode 100644 Documentation/perf/thunderx2-pmu.txt Thanks for writing the documentation, although I think it needs a bit of help before we can merge it. > diff --git a/Documentation/perf/thunderx2-pmu.txt > b/Documentation/perf/thunderx2-pmu.txt > new file mode 100644 > index ..9f5dd7459e68 > --- /dev/null > +++ b/Documentation/perf/thunderx2-pmu.txt > @@ -0,0 +1,106 @@ > + > +Cavium ThunderX2 SoC Performance Monitoring Unit (PMU UNCORE) > +== > + > +ThunderX2 SoC PMU consists of independent system wide per Socket PMUs such > +as Level 3 Cache(L3C) and DDR4 Memory Controller(DMC). Please add some punctuation here. > + > +DMC has 8 interleave channels and L3C has 16 interleave tiles. Events are *The* DMC and *the* L3C > +sampled for default channel(i.e channel 0) and prorated to total number of I'm not sure I understand this; are you saying it's not possible to sample channels other than channel 0? > +channels/tiles. > + > +DMC and L3C, Each PMU supports up to 4 counters. Counters are independently The start of this sentence makes no sense and you've got a capital "Each". > +programmable and can be started and stopped individually. Each counter can > +be set to sample specific perf events. Counters are 32 bit and do not support > +overflow interrupt; they are sampled at every 2 seconds. I think this is unfortunate wording, because actually you don't support what perf calls "sampling" at all. > + > +PMU UNCORE (perf) driver: > + > +The thunderx2-pmu driver registers several perf PMUs for DMC and L3C devices. I think the driver name uses an underscore instead of a hyphen. > +Each of the PMUs provides description of its available events > +and configuration options in sysfs. > + see /sys/devices/uncore_ > + > +S is socket id. *the* socket id > +Each PMU can be used to sample up to 4 events simultaneously. > + > +The "format" directory describes format of the config (event ID). > +The "events" directory provides configuration templates for all > +supported event types that can be used with perf tool. You can drop this bit, since it's not specific to your PMU and is actually describing the perf ABI via sysfs. If we want to document that someplace, it should be in a separate file. > + > +For example, "uncore_dmc_0/cnt_cycles/" is an > +equivalent of "uncore_dmc_0/config=0x1/". Why is this helpful? > + > +Each perf driver also provides a "cpumask" sysfs attribute, which contains a > +single CPU ID of the processor which is likely to be used to handle all the > +PMU events. It will be the first online CPU from the NUMA node of the PMU > device. Again, I don't think this really belongs in here. > + > +Example for perf tool use: > + > +perf stat -a -e uncore_dmc_0/cnt_cycles/ sleep 1 > + > +perf stat -a -e \ > +uncore_dmc_0/cnt_cycles/,\ > +uncore_dmc_0/data_transfers/,\ > +uncore_dmc_0/read_txns/,\ > +uncore_dmc_0/write_txns/ sleep 1 > + > +perf stat -a -e \ > +uncore_l3c_0/read_request/,\ > +uncore_l3c_0/read_hit/,\ > +uncore_l3c_0/inv_request/,\ > +uncore_l3c_0/inv_hit/ sleep 1 > + > +The driver does not support sampling, therefore "perf record" will > +not work. Per-task (without "-a") perf sessions are not supported. What do you mean by "not supported"? If I invoke perf as: # ./perf stat -e uncore_dmc_0/cnt_cycles/ -- ls then I get results back. > + > +L3C events: > + > + > +read_request: > + Number of Read requests received by the L3 Cache. > + This include Read as well as Read Exclusives. > + > +read_hit: > + Number of Read requests received by the L3 cache that were hit > + in the L3 (Data provided form the L3) > + > +writeback_request: > + Number of Write Backs received by the L3 Cache. These are basically > + the L2 Evicts and writes from the PCIe Write Cache. > + > +inv_nwrite_request: > + This is the Number of Invalidate and Write received by the L3 Cache. > + Also Writes from IO that did not go through the PCIe Write Cache. > + > +inv_nwrite_hit > + This is the Number of Invalidate and Write received by the L3 Cache > + That were a hit in the L3 Cache. > + > +inv_request: > + Number of Invalidate request received by the L3 Cache. > + > +inv_hit: > + Number of Invalidate request received by the L3 Cache that were a > + hit in L3. > + > +evict_request: > + Number of Evicts that the L3 generated. Wouldn't this be better off in the perf tools sources, as part of the JSON events file for your PMU? > + > +NOTE: > +1. Granularity of all these events counter value is cache line length(64 > Bytes)
Re: [PATCH v8 2/2] ThunderX2, perf : Add Cavium ThunderX2 SoC UNCORE PMU driver
On Thu, Nov 22, 2018 at 03:04:35AM +, Kulkarni, Ganapatrao wrote:
> This patch adds a perf driver for the PMU UNCORE devices DDR4 Memory
> Controller(DMC) and Level 3 Cache(L3C). Each PMU supports up to 4
> counters. All counters lack overflow interrupt and are
> sampled periodically.
>
> Signed-off-by: Ganapatrao Kulkarni
> ---
> drivers/perf/Kconfig | 9 +
> drivers/perf/Makefile| 1 +
> drivers/perf/thunderx2_pmu.c | 869 +++
> include/linux/cpuhotplug.h | 1 +
> 4 files changed, 880 insertions(+)
> create mode 100644 drivers/perf/thunderx2_pmu.c
>
> diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> index 08ebaf7cca8b..af9bc178495d 100644
> --- a/drivers/perf/Kconfig
> +++ b/drivers/perf/Kconfig
> @@ -87,6 +87,15 @@ config QCOM_L3_PMU
> Adds the L3 cache PMU into the perf events subsystem for
> monitoring L3 cache events.
>
> +config THUNDERX2_PMU
> + tristate "Cavium ThunderX2 SoC PMU UNCORE"
> + depends on ARCH_THUNDER2 && ARM64 && ACPI && NUMA
> + default m
> + help
> +Provides support for ThunderX2 UNCORE events.
> +The SoC has PMU support in its L3 cache controller (L3C) and
> +in the DDR4 Memory Controller (DMC).
> +
> config XGENE_PMU
> depends on ARCH_XGENE
> bool "APM X-Gene SoC PMU"
> diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
> index b3902bd37d53..909f27fd9db3 100644
> --- a/drivers/perf/Makefile
> +++ b/drivers/perf/Makefile
> @@ -7,5 +7,6 @@ obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
> obj-$(CONFIG_HISI_PMU) += hisilicon/
> obj-$(CONFIG_QCOM_L2_PMU)+= qcom_l2_pmu.o
> obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
> +obj-$(CONFIG_THUNDERX2_PMU) += thunderx2_pmu.o
> obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
> obj-$(CONFIG_ARM_SPE_PMU) += arm_spe_pmu.o
> diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
> new file mode 100644
> index ..e6509ba868ab
> --- /dev/null
> +++ b/drivers/perf/thunderx2_pmu.c
> @@ -0,0 +1,869 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CAVIUM THUNDERX2 SoC PMU UNCORE
> + * Copyright (C) 2018 Cavium Inc.
> + * Author: Ganapatrao Kulkarni
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +/* Each ThunderX2(TX2) Socket has a L3C and DMC UNCORE PMU device.
> + * Each UNCORE PMU device consists of 4 independent programmable counters.
> + * Counters are 32 bit and do not support overflow interrupt,
> + * they need to be sampled before overflow(i.e, at every 2 seconds).
> + */
> +
> +#define TX2_PMU_MAX_COUNTERS 4
> +#define TX2_PMU_DMC_CHANNELS 8
> +#define TX2_PMU_L3_TILES 16
> +
> +#define TX2_PMU_HRTIMER_INTERVAL (2 * NSEC_PER_SEC)
> +#define GET_EVENTID(ev) ((ev->hw.config) & 0x1ff)
I think this should be 0x1f.
> +#define GET_COUNTERID(ev)((ev->hw.idx) & 0x3)
> + /* 1 byte per counter(4 counters).
> + * Event id is encoded in bits [5:1] of a byte,
> + */
> +#define DMC_EVENT_CFG(idx, val) ((val) << (((idx) * 8) + 1))
> +
> +#define L3C_COUNTER_CTL 0xA8
> +#define L3C_COUNTER_DATA 0xAC
> +#define DMC_COUNTER_CTL 0x234
> +#define DMC_COUNTER_DATA 0x240
> +
> +/* L3C event IDs */
> +#define L3_EVENT_READ_REQ0xD
> +#define L3_EVENT_WRITEBACK_REQ 0xE
> +#define L3_EVENT_INV_N_WRITE_REQ 0xF
> +#define L3_EVENT_INV_REQ 0x10
> +#define L3_EVENT_EVICT_REQ 0x13
> +#define L3_EVENT_INV_N_WRITE_HIT 0x14
> +#define L3_EVENT_INV_HIT 0x15
> +#define L3_EVENT_READ_HIT0x17
> +#define L3_EVENT_MAX 0x18
> +
> +/* DMC event IDs */
> +#define DMC_EVENT_COUNT_CYCLES 0x1
> +#define DMC_EVENT_WRITE_TXNS 0xB
> +#define DMC_EVENT_DATA_TRANSFERS 0xD
> +#define DMC_EVENT_READ_TXNS 0xF
> +#define DMC_EVENT_MAX0x10
> +
> +enum tx2_uncore_type {
> + PMU_TYPE_L3C,
> + PMU_TYPE_DMC,
> + PMU_TYPE_INVALID,
> +};
> +
> +/*
> + * pmu on each socket has 2 uncore devices(dmc and l3c),
> + * each device has 4 counters.
> + */
> +struct tx2_uncore_pmu {
> + struct hlist_node hpnode;
> + struct list_head entry;
> + struct pmu pmu;
> + char *name;
> + int node;
> + int cpu;
> + u32max_counters;
> + u32prorate_factor;
> + u32max_events;
> + u64 hrtimer_interval;
> + void __iomem *base;
> + DECLARE_BITMAP(active_counters, TX2_PMU_MAX_COUNTERS);
> + struct perf_event *events[TX2_PMU_MAX_COUNTERS];
> + struct device *dev;
> + struct hrtimer hrtimer;
> + const struct attribute_group **attr_groups;
> + enum tx2_uncore_type type;
> + void(*init_cntr_base)(struct perf_event *event,
> + struct tx2_uncore_pmu *tx2_pmu);
> + void(*stop_event)(struct perf_event *event
Re: [PATCH v3] code-of-conduct: Remove explicit list of discrimination factors
On Mon 2018-12-03 12:05:06, Greg Kroah-Hartman wrote: > On Sun, Dec 02, 2018 at 10:32:57AM +0100, Geert Uytterhoeven wrote: > > Providing an explicit list of discrimination factors may give the false > > impression that discrimination based on other unlisted factors would be > > allowed. > > > > Furthermore, this list is already overly long, polarizing, > > politically-laden, and reinstating the concept of human races. > > None of these is related to the goals of the Linux kernel project. > > > > Avoid any ambiguity or political undertone by removing the list, to > > ensure "a harassment-free experience for everyone", period. > > I understand the reason you and others are proposing this change, > however for now, let us stick with the text that we have. As Linus and > I said just over a month ago, let's sit with the text we have until > something comes up that requires a change to happen. > > Also, I recommend you work with the upstream developers of this text to > see if they agree with your changes here. If they do, and update their > version, I will be glad to revisit this text at that time. > But everyone, please note that there are very specific reasons for > listing things like this. You might not agree with those reasons, So what are the reasons? You marked yourself as a maintainer (and I do not think community agreement exists that you should be maintaining this), yet you refuse to take changes, pointing that there's "upstream". > many other people do so each "camp" can not be happy in the end. As we > did have 3 years without such a list of factors, perhaps it is time to > have 3 years with the list to provide a fair balance. :) What kind of argumentation faul is this? Aha. https://en.wikipedia.org/wiki/Argument_to_moderation > So I'm not going to accept this patch at this point in time, sorry. Linus, I don't think Greg is doing good job maintaining this. Can you take the patch? (Or explain what is going on here, because I don't think public has full story). Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH v4 1/8] clk: clkdev/of_clk - add managed lookup and provider registrations
Hello Stephen & All,
I created v5 where I fixed obvious issues. I'll send it in few minutes.
Please note following topics:
On Fri, Nov 30, 2018 at 12:50:22PM +0200, Matti Vaittinen wrote:
>
> On Fri, Nov 30, 2018 at 12:54:10AM -0800, Stephen Boyd wrote:
> > Quoting Matti Vaittinen (2018-11-13 03:55:58)
> > >
> > > -int devm_of_clk_add_hw_provider(struct device *dev,
> > > +static int __devm_of_clk_add_hw_provider(struct device *dev,
> > > struct clk_hw *(*get)(struct of_phandle_args
> > > *clkspec,
> > > void *data),
> > > - void *data)
> > > + struct device_node *of_node, void *data)
> > > {
> > > - struct device_node **ptr, *np;
> > > + struct device_node **ptr;
> > > int ret;
> > >
> > > ptr = devres_alloc(devm_of_clk_release_provider, sizeof(*ptr),
> > > @@ -3906,10 +3906,9 @@ int devm_of_clk_add_hw_provider(struct device *dev,
> > > if (!ptr)
> > > return -ENOMEM;
> > >
> > > - np = dev->of_node;
> > > - ret = of_clk_add_hw_provider(np, get, data);
> > > + *ptr = of_node;
> > > + ret = of_clk_add_hw_provider(of_node, get, data);
> > > if (!ret) {
> > > - *ptr = np;
> >
> > Why is this moved outside of the if condition?
> I completely removed the local variable np and just unconditionally set
> the allocated devres to point at the node (if allocation succeeded). We
> could of course only do this if the provider registration succeeded and
> save one assignment - but I guess I intended to remove the curly braces
> and thus decided to go for one liner after if. But apparently I didn't
> remove the braces O_o. Well, I can put the assignment inside the
> condition if you prefer that.
>
> > In fact, why isn't just
> > the first line in this hunk deleted and passed to this function as
> > struct device_node *np?
>
> I am sorry but I don't quite follow your suggestion here. Do you mean we
> could just pass the struct device_node *np in devres_add()? I thought
> the pointer passed to devress_add() should be allocated using
> devres_alloc. Can you please elaborate what you mean?
I could not really spot what to fix in patched code (see below).
static int __devm_of_clk_add_hw_provider(struct device *dev,
struct clk_hw *(*get)(struct of_phandle_args *clkspec,
void *data),
struct device_node *of_node, void *data)
{
struct device_node **ptr;
int ret;
ptr = devres_alloc(devm_of_clk_release_provider, sizeof(*ptr),
GFP_KERNEL);
if (!ptr)
return -ENOMEM;
*ptr = of_node;
ret = of_clk_add_hw_provider(of_node, get, data);
if (!ret)
devres_add(dev, ptr);
else
devres_free(ptr);
return ret;
}
As far as I understand we need to allocate the ptr using devres_alloc.
We also need to pass this ptr to of_clk_add_hw_provider - and we must
assign our node to the *ptr. (I removed the extra braces - this change
is laso included in v5 but I don't see how we should improve). Can you
please explain me if you still wish to me change this further?
> > > +int devm_of_clk_add_parent_hw_provider(struct device *dev,
> > > + struct clk_hw *(*get)(struct of_phandle_args
> > > *clkspec,
> > > + void *data),
> > > + void *data)
> > > +{
> > > + return __devm_of_clk_add_hw_provider(dev, get,
> > > dev->parent->of_node,
> >
> > I'm wondering if we can somehow auto-detect this in
> > devm_of_clk_add_hw_provider() by looking for #clock-cells in the node.
> > If it isn't there, then we go to the parent node and look for a
> > #clock-cells property there in the DT node for that device. Does that
> > make sense? Then there isn't any new API and we can attach the lifetime
> > of the devm registration to the presence of the property indicating this
> > is a clk controller or not.
>
> Huh. I don't know why but building this kind of logic in core is a bit
> scary to me. I guess I can try implementing something like this - but I
> am not really a fan of this. (Accidentally) omit the #clock-cells from
> node and we go to parent node - I am a novice on this area but this
> sounds like a potential hazard to me. I believe the driver should know
> if it's properties should be in own or parent node - and if they are
> not, then there should be no guessing but error. The lifetime is topic
> where I would like to get information from you who know the kernel
> better than I do =) But I guess the parent node is there at least as
> long as the child device is alive. So for me the life time of
> get-callback is more crucial - but as I said, I don't understand the
> kernel in details so you probably know it better
[PATCH v5 0/9] clk: clkdev: managed clk lookup and provider registrations
Series add bd71837/bd71837 PMIC clock support + managed interfaces Few clk drivers appear to be leaking clkdev lookup registrations at driver remove. The patch series adds devm versions of lookup registrations and cleans up few drivers. Driver clean-up patches have not been tested as I lack the HW. All testing and comments if driver/device removal is even possible for changed drivers is highly appreciated. If removal is not possible I will gladly drop the patches from series - although leaking lookups may serve as bad example for new developers =) Patch 8 adds support for clock gate in ROHM bd71837 and bd71847 PMICs. This change is included in the series because it depends on new managed interfaces introduced in this series. bd718x7 driver and devm interfaces are tested on BeagleBoneBlack and bd71837 break-out board. Clk area register interface of bd71847 is identical to bd71837. Changed drivers are: clk-max77686, clk-st, clk-hi655x, rk808, clk-twl6040 and apcs-msm8916. New driver is clk-bd718x7 This series has been discussed for a while now. For those who want to see whole discussion: The bd71837 driver was originally proposed here https://lore.kernel.org/lkml/d99c8762b0fbbcb18ec4f4d104191364c0ea798c.1528117485.git.matti.vaitti...@fi.rohmeurope.com/ clk portion was separated from that series and devm variants were proposed here https://lore.kernel.org/linux-clk/cover.1535630942.git.matti.vaitti...@fi.rohmeurope.com/ Cleanup to other drivers was initiated in this series while waiting for MFD portions of bd718x7 to be applied. And now when MFD dependencies are in-tree the patch version (4) combined bd718x7 driver back to this series. Changelog (for this series) v5 - Split v4 patch 1. Place clkdev stuff to patch 1 and clk provider to patch 2 - Remove devm_clk_release_clkdev from devres.txt - Added kerneldoc for managed provider registrations. - Cleaned styling issues. Changelog (for this series) v4 - Add support for ROHM bd718x7 PMIC clock gate. Included in this patch series because it depends on managed interfaces added in patch 1. Changelog (for this series) v3 Address issues spotted by Krzysztof Kozlowski - Drop patch 3 for clk-s3c2410-dclk as this device can never be removed - Fix indentiation for clk-max77686 - Rest of the patches are unchanged. Changelog (for this series) v2 Issue spotted by 0-Day test suite - Add a stub function 'devm_of_clk_add_parent_hw_provider' for no OF config. - patches 2-8 are unchanged. This patch series is based on clk-next --- Matti Vaittinen (9): clkdev: add managed clkdev lookup registration clk: of_clk - add managed provider registrations clk: clk-max77686: Clean clkdev lookup leak and use devm clk: clk-st: avoid clkdev lookup leak at remove clk: clk-hi655x: Free of_provider at remove clk: rk808: use managed version of of_provider registration clk: clk-twl6040: Free of_provider at remove clk: apcs-msm8916: simplify probe cleanup by using devm clk: bd718x7: Initial support for ROHM bd71837/bd71847 PMIC clock Documentation/driver-model/devres.txt | 2 + drivers/clk/Kconfig | 7 ++ drivers/clk/Makefile | 1 + drivers/clk/clk-bd718x7.c | 131 ++ drivers/clk/clk-hi655x.c | 4 +- drivers/clk/clk-max77686.c| 29 ++-- drivers/clk/clk-rk808.c | 15 +--- drivers/clk/clk-twl6040.c | 5 +- drivers/clk/clk.c | 65 ++--- drivers/clk/clkdev.c | 122 --- drivers/clk/qcom/apcs-msm8916.c | 5 +- drivers/clk/x86/clk-st.c | 3 +- include/linux/clk-provider.h | 11 +++ include/linux/clkdev.h| 4 ++ 14 files changed, 325 insertions(+), 79 deletions(-) create mode 100644 drivers/clk/clk-bd718x7.c -- 2.14.3 -- Matti Vaittinen ROHM Semiconductors ~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
[PATCH v5 2/9] clk: of_clk - add managed provider registrations
With MFD devices the clk properties may be contained in MFD (parent) DT
node. Current devm_of_clk_add_hw_provider assumes the clk is bound to MFD
subdevice not to MFD device (parent). Add
devm_of_clk_add_hw_provider_parent to tackle this issue.
Signed-off-by: Matti Vaittinen
---
Documentation/driver-model/devres.txt | 1 +
drivers/clk/clk.c | 65 ++-
include/linux/clk-provider.h | 11 ++
3 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/Documentation/driver-model/devres.txt
b/Documentation/driver-model/devres.txt
index dbf14326243b..97c9c575b2af 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -238,6 +238,7 @@ CLOCK
devm_clk_put()
devm_clk_hw_register()
devm_of_clk_add_hw_provider()
+ devm_of_clk_add_parent_hw_provider()
devm_clk_hw_register_clkdev()
DMA
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index af011974d4ec..30beb60bd8f9 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3893,12 +3893,12 @@ static void devm_of_clk_release_provider(struct device
*dev, void *res)
of_clk_del_provider(*(struct device_node **)res);
}
-int devm_of_clk_add_hw_provider(struct device *dev,
+static int __devm_of_clk_add_hw_provider(struct device *dev,
struct clk_hw *(*get)(struct of_phandle_args *clkspec,
void *data),
- void *data)
+ struct device_node *of_node, void *data)
{
- struct device_node **ptr, *np;
+ struct device_node **ptr;
int ret;
ptr = devres_alloc(devm_of_clk_release_provider, sizeof(*ptr),
@@ -3906,19 +3906,62 @@ int devm_of_clk_add_hw_provider(struct device *dev,
if (!ptr)
return -ENOMEM;
- np = dev->of_node;
- ret = of_clk_add_hw_provider(np, get, data);
- if (!ret) {
- *ptr = np;
+ *ptr = of_node;
+ ret = of_clk_add_hw_provider(of_node, get, data);
+ if (!ret)
devres_add(dev, ptr);
- } else {
+ else
devres_free(ptr);
- }
return ret;
}
+
+/**
+ * devm_of_clk_add_hw_provider() - Managed clk provider node registration
+ * @dev: Device acting as the clock provider. Used for DT node and lifetime.
+ * @get: callback for decoding clk_hw
+ * @data: context pointer for @get callback.
+ *
+ * Returns 0 on success or an errno on failure.
+ *
+ * Registers clock provider for given device's node. Provider is automatically
+ * released at device exit.
+ */
+int devm_of_clk_add_hw_provider(struct device *dev,
+ struct clk_hw *(*get)(struct of_phandle_args *clkspec,
+ void *data),
+ void *data)
+{
+ return __devm_of_clk_add_hw_provider(dev, get, dev->of_node, data);
+}
EXPORT_SYMBOL_GPL(devm_of_clk_add_hw_provider);
+/**
+ * devm_of_clk_add_parent_hw_provider() - Managed clk provider node
registration
+ * @dev: Device acting as the clock provider. Provider's DT node is parent
node.
+ * @get: callback for decoding clk_hw
+ * @data: context pointer for @get callback.
+ *
+ * Returns 0 on success or an errno on failure.
+ *
+ * Registers clock provider for given device's parent node. Usable in cases
+ * where it really is the parent node that contains the provider information.
+ * Typical use-cases are MFD devices where the MFD sub-device is handling
+ * actual clock HW but the MFD node (parent) is containing the clock
+ * information.
+ *
+ * Provider is automatically released at device exit.
+ */
+int devm_of_clk_add_parent_hw_provider(struct device *dev,
+ struct clk_hw *(*get)(struct of_phandle_args *clkspec,
+ void *data),
+ void *data)
+{
+ return __devm_of_clk_add_hw_provider(dev, get, dev->parent->of_node,
+data);
+}
+EXPORT_SYMBOL_GPL(devm_of_clk_add_parent_hw_provider);
+
/**
* of_clk_del_provider() - Remove a previously registered clock provider
* @np: Device node pointer associated with clock provider
@@ -3950,6 +3993,10 @@ static int devm_clk_provider_match(struct device *dev,
void *res, void *data)
return *np == data;
}
+/**
+ * devm_of_clk_del_provider() - Remove clock provider registered using devm
+ * @dev: Device to whose lifetime the clock provider was bound
+ */
void devm_of_clk_del_provider(struct device *dev)
{
int ret;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 60c51871b04b..a6663f084cf1 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -916,6 +916,10 @@ int devm_of_clk_add_hw_provider(struct device *dev,
struct clk_hw *(*get)(struct of_phandle_args
*clkspec,
[PATCH v5 1/9] clkdev: add managed clkdev lookup registration
Clkdev registration lacks of managed registration functions and it
seems few drivers do not drop clkdev lookups at exit. Add
devm_clk_hw_register_clkdev and devm_clk_release_clkdev to ease lookup
releasing at exit.
Signed-off-by: Matti Vaittinen
---
Documentation/driver-model/devres.txt | 1 +
drivers/clk/clkdev.c | 122 ++
include/linux/clkdev.h| 4 ++
3 files changed, 101 insertions(+), 26 deletions(-)
diff --git a/Documentation/driver-model/devres.txt
b/Documentation/driver-model/devres.txt
index 43681ca0837f..dbf14326243b 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -238,6 +238,7 @@ CLOCK
devm_clk_put()
devm_clk_hw_register()
devm_of_clk_add_hw_provider()
+ devm_clk_hw_register_clkdev()
DMA
dmaenginem_async_device_register()
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 9ab3db8b3988..68a1e55a60d2 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -401,6 +401,25 @@ static struct clk_lookup *__clk_register_clkdev(struct
clk_hw *hw,
return cl;
}
+static int do_clk_register_clkdev(struct clk_hw *hw,
+ struct clk_lookup **cl, const char *con_id, const char *dev_id)
+{
+
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+ /*
+* Since dev_id can be NULL, and NULL is handled specially, we must
+* pass it as either a NULL format string, or with "%s".
+*/
+ if (dev_id)
+ *cl = __clk_register_clkdev(hw, con_id, "%s",
+ dev_id);
+ else
+ *cl = __clk_register_clkdev(hw, con_id, NULL);
+
+ return *cl ? 0 : -ENOMEM;
+}
+
/**
* clk_register_clkdev - register one clock lookup for a struct clk
* @clk: struct clk to associate with all clk_lookups
@@ -420,20 +439,10 @@ int clk_register_clkdev(struct clk *clk, const char
*con_id,
{
struct clk_lookup *cl;
- if (IS_ERR(clk))
- return PTR_ERR(clk);
-
- /*
-* Since dev_id can be NULL, and NULL is handled specially, we must
-* pass it as either a NULL format string, or with "%s".
-*/
- if (dev_id)
- cl = __clk_register_clkdev(__clk_get_hw(clk), con_id, "%s",
- dev_id);
- else
- cl = __clk_register_clkdev(__clk_get_hw(clk), con_id, NULL);
-
- return cl ? 0 : -ENOMEM;
+ if (!IS_ERR(clk))
+ return do_clk_register_clkdev(__clk_get_hw(clk), &cl, con_id,
+ dev_id);
+ return PTR_ERR(clk);
}
EXPORT_SYMBOL(clk_register_clkdev);
@@ -456,18 +465,79 @@ int clk_hw_register_clkdev(struct clk_hw *hw, const char
*con_id,
{
struct clk_lookup *cl;
- if (IS_ERR(hw))
- return PTR_ERR(hw);
+ return do_clk_register_clkdev(hw, &cl, con_id, dev_id);
+}
+EXPORT_SYMBOL(clk_hw_register_clkdev);
- /*
-* Since dev_id can be NULL, and NULL is handled specially, we must
-* pass it as either a NULL format string, or with "%s".
-*/
- if (dev_id)
- cl = __clk_register_clkdev(hw, con_id, "%s", dev_id);
- else
- cl = __clk_register_clkdev(hw, con_id, NULL);
+static void devm_clkdev_release(struct device *dev, void *res)
+{
+ clkdev_drop(*(struct clk_lookup **)res);
+}
- return cl ? 0 : -ENOMEM;
+static int devm_clk_match_clkdev(struct device *dev, void *res, void *data)
+{
+ struct clk_lookup **l = res;
+
+ if (!l || !*l) {
+ WARN_ON(!l || !*l);
+ return 0;
+ }
+ return *l == data;
}
-EXPORT_SYMBOL(clk_hw_register_clkdev);
+
+/**
+ * devm_clk_release_clkdev - Resource managed clkdev lookup release
+ * @dev: device this lookup is bound
+ * @con_id: connection ID string on device
+ * @dev_id: format string describing device name
+ *
+ * Drop the clkdev lookup created with devm_clk_hw_register_clkdev.
+ * Normally this function will not need to be called and the resource
+ * management code will ensure that the resource is freed.
+ */
+void devm_clk_release_clkdev(struct device *dev, const char *con_id,
+const char *dev_id)
+{
+ struct clk_lookup *cl;
+ int rval;
+
+ cl = clk_find(dev_id, con_id);
+ WARN_ON(!cl);
+ rval = devres_release(dev, devm_clkdev_release,
+ devm_clk_match_clkdev, cl);
+ WARN_ON(rval);
+}
+EXPORT_SYMBOL(devm_clk_release_clkdev);
+
+/**
+ * devm_clk_hw_register_clkdev - managed clk lookup registration for clk_hw
+ * @dev: device this lookup is bound
+ * @hw: struct clk_hw to associate with all clk_lookups
+ * @con_id: connection ID string on device
+ * @dev_id: format string describing device name
+ *
+ * con_id or dev_id may be NULL as a wildcard, just as in the rest of
+ * clkdev.
+ *
+ * To m
[PATCH v5 3/9] clk: clk-max77686: Clean clkdev lookup leak and use devm
clk-max77686 never clean clkdev lookup at remove. This can cause
oops if clk-max77686 is removed and inserted again. Fix leak by
using new devm clkdev lookup registration. Simplify also error
path by using new devm_of_clk_add_parent_hw_provider.
Signed-off-by: Matti Vaittinen
Reviewed-by: Krzysztof Kozlowski
---
drivers/clk/clk-max77686.c | 29 +++--
1 file changed, 7 insertions(+), 22 deletions(-)
diff --git a/drivers/clk/clk-max77686.c b/drivers/clk/clk-max77686.c
index 22c937644c93..e65925d5e412 100644
--- a/drivers/clk/clk-max77686.c
+++ b/drivers/clk/clk-max77686.c
@@ -235,8 +235,9 @@ static int max77686_clk_probe(struct platform_device *pdev)
return ret;
}
- ret = clk_hw_register_clkdev(&max_clk_data->hw,
-max_clk_data->clk_idata.name,
NULL);
+ ret = devm_clk_hw_register_clkdev(dev, &max_clk_data->hw,
+ max_clk_data->clk_idata.name,
+ NULL);
if (ret < 0) {
dev_err(dev, "Failed to clkdev register: %d\n", ret);
return ret;
@@ -244,8 +245,9 @@ static int max77686_clk_probe(struct platform_device *pdev)
}
if (parent->of_node) {
- ret = of_clk_add_hw_provider(parent->of_node,
of_clk_max77686_get,
-drv_data);
+ ret = devm_of_clk_add_parent_hw_provider(dev,
+of_clk_max77686_get,
+drv_data);
if (ret < 0) {
dev_err(dev, "Failed to register OF clock provider:
%d\n",
@@ -261,27 +263,11 @@ static int max77686_clk_probe(struct platform_device
*pdev)
1 << MAX77802_CLOCK_LOW_JITTER_SHIFT);
if (ret < 0) {
dev_err(dev, "Failed to config low-jitter: %d\n", ret);
- goto remove_of_clk_provider;
+ return ret;
}
}
return 0;
-
-remove_of_clk_provider:
- if (parent->of_node)
- of_clk_del_provider(parent->of_node);
-
- return ret;
-}
-
-static int max77686_clk_remove(struct platform_device *pdev)
-{
- struct device *parent = pdev->dev.parent;
-
- if (parent->of_node)
- of_clk_del_provider(parent->of_node);
-
- return 0;
}
static const struct platform_device_id max77686_clk_id[] = {
@@ -297,7 +283,6 @@ static struct platform_driver max77686_clk_driver = {
.name = "max77686-clk",
},
.probe = max77686_clk_probe,
- .remove = max77686_clk_remove,
.id_table = max77686_clk_id,
};
--
2.14.3
--
Matti Vaittinen
ROHM Semiconductors
~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
Re: [PATCH v5 0/9] clk: clkdev: managed clk lookup and provider registrations
> Series add bd71837/bd71837 PMIC clock support + managed interfaces [...] > Matti Vaittinen (9): > clkdev: add managed clkdev lookup registration > clk: of_clk - add managed provider registrations > clk: clk-max77686: Clean clkdev lookup leak and use devm > clk: clk-st: avoid clkdev lookup leak at remove > clk: clk-hi655x: Free of_provider at remove > clk: rk808: use managed version of of_provider registration > clk: clk-twl6040: Free of_provider at remove > clk: apcs-msm8916: simplify probe cleanup by using devm > clk: bd718x7: Initial support for ROHM bd71837/bd71847 PMIC clock > > Documentation/driver-model/devres.txt | 2 + > drivers/clk/Kconfig | 7 ++ > drivers/clk/Makefile | 1 + > drivers/clk/clk-bd718x7.c | 131 > ++ > drivers/clk/clk-hi655x.c | 4 +- > drivers/clk/clk-max77686.c| 29 ++-- > drivers/clk/clk-rk808.c | 15 +--- > drivers/clk/clk-twl6040.c | 5 +- > drivers/clk/clk.c | 65 ++--- > drivers/clk/clkdev.c | 122 --- > drivers/clk/qcom/apcs-msm8916.c | 5 +- > drivers/clk/x86/clk-st.c | 3 +- > include/linux/clk-provider.h | 11 +++ > include/linux/clkdev.h| 4 ++ > 14 files changed, 325 insertions(+), 79 deletions(-) > create mode 100644 drivers/clk/clk-bd718x7.c You've send me a cover letter and no patches. I suggest that I should not be on the addressee list at all for this. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
[PATCH v5 4/9] clk: clk-st: avoid clkdev lookup leak at remove
Use devm based clkdev lookup registration to avoid leaking lookup structures. Signed-off-by: Matti Vaittinen --- drivers/clk/x86/clk-st.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/clk/x86/clk-st.c b/drivers/clk/x86/clk-st.c index 3a0996f2d556..25d4b97aff9b 100644 --- a/drivers/clk/x86/clk-st.c +++ b/drivers/clk/x86/clk-st.c @@ -52,7 +52,8 @@ static int st_clk_probe(struct platform_device *pdev) 0, st_data->base + MISCCLKCNTL1, OSCCLKENB, CLK_GATE_SET_TO_DISABLE, NULL); - clk_hw_register_clkdev(hws[ST_CLK_GATE], "oscout1", NULL); + devm_clk_hw_register_clkdev(&pdev->dev, hws[ST_CLK_GATE], "oscout1", + NULL); return 0; } -- 2.14.3 -- Matti Vaittinen ROHM Semiconductors ~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
[PATCH v5 6/9] clk: rk808: use managed version of of_provider registration
Simplify clean-up for rk808 by using managed version of of_provider
registration.
Signed-off-by: Matti Vaittinen
---
drivers/clk/clk-rk808.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/clk/clk-rk808.c b/drivers/clk/clk-rk808.c
index 6461f2820a5b..177340edaae5 100644
--- a/drivers/clk/clk-rk808.c
+++ b/drivers/clk/clk-rk808.c
@@ -138,23 +138,12 @@ static int rk808_clkout_probe(struct platform_device
*pdev)
if (ret)
return ret;
- return of_clk_add_hw_provider(node, of_clk_rk808_get, rk808_clkout);
-}
-
-static int rk808_clkout_remove(struct platform_device *pdev)
-{
- struct rk808 *rk808 = dev_get_drvdata(pdev->dev.parent);
- struct i2c_client *client = rk808->i2c;
- struct device_node *node = client->dev.of_node;
-
- of_clk_del_provider(node);
-
- return 0;
+ return devm_of_clk_add_parent_hw_provider(&pdev->dev,
+ of_clk_rk808_get, rk808_clkout);
}
static struct platform_driver rk808_clkout_driver = {
.probe = rk808_clkout_probe,
- .remove = rk808_clkout_remove,
.driver = {
.name = "rk808-clkout",
},
--
2.14.3
--
Matti Vaittinen
ROHM Semiconductors
~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
[PATCH v5 5/9] clk: clk-hi655x: Free of_provider at remove
use devm variant for of_provider registration so provider is freed
at exit.
Signed-off-by: Matti Vaittinen
---
drivers/clk/clk-hi655x.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk-hi655x.c b/drivers/clk/clk-hi655x.c
index 403a0188634a..394d0109104d 100644
--- a/drivers/clk/clk-hi655x.c
+++ b/drivers/clk/clk-hi655x.c
@@ -107,8 +107,8 @@ static int hi655x_clk_probe(struct platform_device *pdev)
if (ret)
return ret;
- return of_clk_add_hw_provider(parent->of_node, of_clk_hw_simple_get,
-&hi655x_clk->clk_hw);
+ return devm_of_clk_add_parent_hw_provider(&pdev->dev,
+ of_clk_hw_simple_get, &hi655x_clk->clk_hw);
}
static struct platform_driver hi655x_clk_driver = {
--
2.14.3
--
Matti Vaittinen
ROHM Semiconductors
~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
[PATCH v5 7/9] clk: clk-twl6040: Free of_provider at remove
use devm variant for of_provider registration so provider is freed
at exit.
Signed-off-by: Matti Vaittinen
Acked-by: Peter Ujfalusi
---
drivers/clk/clk-twl6040.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/clk-twl6040.c b/drivers/clk/clk-twl6040.c
index 25dfe050ae9f..e9da09453eb2 100644
--- a/drivers/clk/clk-twl6040.c
+++ b/drivers/clk/clk-twl6040.c
@@ -108,9 +108,8 @@ static int twl6040_pdmclk_probe(struct platform_device
*pdev)
platform_set_drvdata(pdev, clkdata);
- return of_clk_add_hw_provider(pdev->dev.parent->of_node,
- of_clk_hw_simple_get,
- &clkdata->pdmclk_hw);
+ return devm_of_clk_add_parent_hw_provider(&pdev->dev,
+ of_clk_hw_simple_get, &clkdata->pdmclk_hw);
}
static struct platform_driver twl6040_pdmclk_driver = {
--
2.14.3
--
Matti Vaittinen
ROHM Semiconductors
~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
[PATCH v5 8/9] clk: apcs-msm8916: simplify probe cleanup by using devm
use devm variant for of_provider registration.
Signed-off-by: Matti Vaittinen
---
drivers/clk/qcom/apcs-msm8916.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/qcom/apcs-msm8916.c b/drivers/clk/qcom/apcs-msm8916.c
index b1cc8dbcd327..f4e0c136ab1a 100644
--- a/drivers/clk/qcom/apcs-msm8916.c
+++ b/drivers/clk/qcom/apcs-msm8916.c
@@ -96,8 +96,8 @@ static int qcom_apcs_msm8916_clk_probe(struct platform_device
*pdev)
goto err;
}
- ret = of_clk_add_hw_provider(parent->of_node, of_clk_hw_simple_get,
-&a53cc->clkr.hw);
+ ret = devm_of_clk_add_parent_hw_provider(dev, of_clk_hw_simple_get,
+&a53cc->clkr.hw);
if (ret) {
dev_err(dev, "failed to add clock provider: %d\n", ret);
goto err;
@@ -118,7 +118,6 @@ static int qcom_apcs_msm8916_clk_remove(struct
platform_device *pdev)
struct device *parent = pdev->dev.parent;
clk_notifier_unregister(a53cc->pclk, &a53cc->clk_nb);
- of_clk_del_provider(parent->of_node);
return 0;
}
--
2.14.3
--
Matti Vaittinen
ROHM Semiconductors
~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
[PATCH v5 9/9] clk: bd718x7: Initial support for ROHM bd71837/bd71847 PMIC clock
ROHM bd71837 and bd71847 contain 32768Hz clock gate. Support the clock
using generic clock framework. Note, only bd71837 is tested but bd71847
should be identical what comes to clk parts.
Signed-off-by: Matti Vaittinen
---
drivers/clk/Kconfig | 7 +++
drivers/clk/Makefile | 1 +
drivers/clk/clk-bd718x7.c | 131 ++
3 files changed, 139 insertions(+)
create mode 100644 drivers/clk/clk-bd718x7.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 81cdb4eaca07..2dc12bf75b1b 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -283,6 +283,13 @@ config COMMON_CLK_STM32H7
---help---
Support for stm32h7 SoC family clocks
+config COMMON_CLK_BD718XX
+ tristate "Clock driver for ROHM BD718x7 PMIC"
+ depends on MFD_ROHM_BD718XX
+ help
+ This driver supports ROHM BD71837 and ROHM BD71847
+ PMICs clock gates.
+
source "drivers/clk/actions/Kconfig"
source "drivers/clk/bcm/Kconfig"
source "drivers/clk/hisilicon/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 72be7a38cff1..a47430b387db 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -21,6 +21,7 @@ endif
obj-$(CONFIG_MACH_ASM9260) += clk-asm9260.o
obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN)+= clk-axi-clkgen.o
obj-$(CONFIG_ARCH_AXXIA) += clk-axm5516.o
+obj-$(CONFIG_COMMON_CLK_BD718XX) += clk-bd718x7.o
obj-$(CONFIG_COMMON_CLK_CDCE706) += clk-cdce706.o
obj-$(CONFIG_COMMON_CLK_CDCE925) += clk-cdce925.o
obj-$(CONFIG_ARCH_CLPS711X)+= clk-clps711x.o
diff --git a/drivers/clk/clk-bd718x7.c b/drivers/clk/clk-bd718x7.c
new file mode 100644
index ..df5f1068ce8e
--- /dev/null
+++ b/drivers/clk/clk-bd718x7.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 ROHM Semiconductors
+// bd71837.c -- ROHM BD71837MWV clock driver
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct bd718xx_clk {
+ struct clk_hw hw;
+ u8 reg;
+ u8 mask;
+ struct platform_device *pdev;
+ struct bd718xx *mfd;
+};
+
+static int bd71837_clk_set(struct clk_hw *hw, int status)
+{
+ struct bd718xx_clk *c = container_of(hw, struct bd718xx_clk, hw);
+
+ return regmap_update_bits(c->mfd->regmap, c->reg, c->mask, status);
+}
+
+static void bd71837_clk_disable(struct clk_hw *hw)
+{
+ int rv;
+ struct bd718xx_clk *c = container_of(hw, struct bd718xx_clk, hw);
+
+ rv = bd71837_clk_set(hw, 0);
+ if (rv)
+ dev_dbg(&c->pdev->dev, "Failed to disable 32K clk (%d)\n", rv);
+}
+
+static int bd71837_clk_enable(struct clk_hw *hw)
+{
+ return bd71837_clk_set(hw, 1);
+}
+
+static int bd71837_clk_is_enabled(struct clk_hw *hw)
+{
+ int enabled;
+ int rval;
+ struct bd718xx_clk *c = container_of(hw, struct bd718xx_clk, hw);
+
+ rval = regmap_read(c->mfd->regmap, c->reg, &enabled);
+
+ if (rval)
+ return rval;
+
+ return enabled & c->mask;
+}
+
+static const struct clk_ops bd71837_clk_ops = {
+ .prepare = &bd71837_clk_enable,
+ .unprepare = &bd71837_clk_disable,
+ .is_prepared = &bd71837_clk_is_enabled,
+};
+
+static int bd71837_clk_probe(struct platform_device *pdev)
+{
+ struct bd718xx_clk *c;
+ int rval = -ENOMEM;
+ const char *parent_clk;
+ struct device *parent = pdev->dev.parent;
+ struct bd718xx *mfd = dev_get_drvdata(parent);
+ struct clk_init_data init = {
+ .name = "bd718xx-32k-out",
+ .ops = &bd71837_clk_ops,
+ };
+
+ c = devm_kzalloc(&pdev->dev, sizeof(*c), GFP_KERNEL);
+ if (!c)
+ return -ENOMEM;
+
+ init.num_parents = 1;
+ parent_clk = of_clk_get_parent_name(parent->of_node, 0);
+
+ init.parent_names = &parent_clk;
+ if (!parent_clk) {
+ dev_err(&pdev->dev, "No parent clk found\n");
+ return -EINVAL;
+ }
+
+ c->reg = BD718XX_REG_OUT32K;
+ c->mask = BD718XX_OUT32K_EN;
+ c->mfd = mfd;
+ c->pdev = pdev;
+ c->hw.init = &init;
+
+ of_property_read_string_index(parent->of_node,
+ "clock-output-names", 0, &init.name);
+
+ rval = devm_clk_hw_register(&pdev->dev, &c->hw);
+ if (!rval) {
+ rval = devm_clk_hw_register_clkdev(&pdev->dev,
+ &c->hw, init.name, NULL);
+ if (rval)
+ dev_warn(&pdev->dev, "Failed to register clkdev\n");
+ if (parent->of_node) {
+ rval = devm_of_clk_add_parent_hw_provider(&pdev->dev,
+of_clk_hw_simple_get, &c->hw);
+ if (rval)
+ dev_err(&pdev->dev,
+
[PATCH] cpuidle: Add 'high' and 'low' idle state metrics
From: Rafael J. Wysocki
Add two new metrics for CPU idle states, "high" and "low", to count
the number of times the given state had been asked for (or entered
from the kernel's perspective), but the observed idle duration turned
out to be too high or too low for it (respectively).
These mertics help to estimat the quality of the CPU idle governor
in use.
Signed-off-by: Rafael J. Wysocki
---
On top of https://patchwork.kernel.org/patch/10705317/
---
Documentation/ABI/testing/sysfs-devices-system-cpu |7 +
Documentation/admin-guide/pm/cpuidle.rst |8 +
drivers/cpuidle/cpuidle.c | 29 +
drivers/cpuidle/sysfs.c|6
include/linux/cpuidle.h|2 +
5 files changed, 52 insertions(+)
Index: linux-pm/drivers/cpuidle/cpuidle.c
===
--- linux-pm.orig/drivers/cpuidle/cpuidle.c
+++ linux-pm/drivers/cpuidle/cpuidle.c
@@ -248,6 +248,8 @@ int cpuidle_enter_state(struct cpuidle_d
local_irq_enable();
if (entered_state >= 0) {
+ int i;
+
/*
* Update cpuidle counters
* This can be moved to within driver enter routine,
@@ -260,6 +262,33 @@ int cpuidle_enter_state(struct cpuidle_d
dev->last_residency = (int)diff;
dev->states_usage[entered_state].time += dev->last_residency;
dev->states_usage[entered_state].usage++;
+
+ if (diff < drv->states[entered_state].target_residency) {
+ for (i = entered_state - 1; i >= 0; i--) {
+ if (drv->states[i].disabled ||
+ dev->states_usage[i].disable)
+ continue;
+
+ /* Shallower states are enabled, so update. */
+ dev->states_usage[entered_state].high++;
+ break;
+ }
+ } else {
+ for (i = entered_state + 1; i < drv->state_count; i++) {
+ if (drv->states[i].disabled ||
+ dev->states_usage[i].disable)
+ continue;
+
+ /*
+* Update if a deeper state would have been a
+* better match for the observed idle duration.
+*/
+ if (diff >= drv->states[i].target_residency)
+ dev->states_usage[entered_state].low++;
+
+ break;
+ }
+ }
} else {
dev->last_residency = 0;
}
Index: linux-pm/include/linux/cpuidle.h
===
--- linux-pm.orig/include/linux/cpuidle.h
+++ linux-pm/include/linux/cpuidle.h
@@ -33,6 +33,8 @@ struct cpuidle_state_usage {
unsigned long long disable;
unsigned long long usage;
unsigned long long time; /* in US */
+ unsigned long long high; /* Number of times it's been too deep */
+ unsigned long long low; /* Number of times it's been too shallow
*/
#ifdef CONFIG_SUSPEND
unsigned long long s2idle_usage;
unsigned long long s2idle_time; /* in US */
Index: linux-pm/drivers/cpuidle/sysfs.c
===
--- linux-pm.orig/drivers/cpuidle/sysfs.c
+++ linux-pm/drivers/cpuidle/sysfs.c
@@ -301,6 +301,8 @@ define_show_state_str_function(name)
define_show_state_str_function(desc)
define_show_state_ull_function(disable)
define_store_state_ull_function(disable)
+define_show_state_ull_function(high)
+define_show_state_ull_function(low)
define_one_state_ro(name, show_state_name);
define_one_state_ro(desc, show_state_desc);
@@ -310,6 +312,8 @@ define_one_state_ro(power, show_state_po
define_one_state_ro(usage, show_state_usage);
define_one_state_ro(time, show_state_time);
define_one_state_rw(disable, show_state_disable, store_state_disable);
+define_one_state_ro(high, show_state_high);
+define_one_state_ro(low, show_state_low);
static struct attribute *cpuidle_state_default_attrs[] = {
&attr_name.attr,
@@ -320,6 +324,8 @@ static struct attribute *cpuidle_state_d
&attr_usage.attr,
&attr_time.attr,
&attr_disable.attr,
+ &attr_high.attr,
+ &attr_low.attr,
NULL
};
Index: linux-pm/Documentation/admin-guide/pm/cpuidle.rst
===
--- linux-pm.orig/Documentation/admin-guide/pm/cpuidle.rst
+++ linux-pm/Documentation/admin-guide/pm/cpuidle.rst
@@ -404,9 +404,17 @@ object corresponding
[PATCH] cpuidle: poll_state: Disregard disable idle states
From: Rafael J. Wysocki
When computing the limit of time to spend in the loop in poll_idle(),
use the target residency of the first enabled idle state deeper than
state 0 instead of always using the target residency of state 1.
This helps when state 1 is disabled for diagnostics, for instance.
Signed-off-by: Rafael J. Wysocki
---
drivers/cpuidle/poll_state.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
Index: linux-pm/drivers/cpuidle/poll_state.c
===
--- linux-pm.orig/drivers/cpuidle/poll_state.c
+++ linux-pm/drivers/cpuidle/poll_state.c
@@ -20,8 +20,17 @@ static int __cpuidle poll_idle(struct cp
local_irq_enable();
if (!current_set_polling_and_test()) {
- u64 limit = (u64)drv->states[1].target_residency *
NSEC_PER_USEC;
unsigned int loop_count = 0;
+ u64 limit = TICK_USEC;
+ int i;
+
+ for (i = 1; i < drv->state_count; i++) {
+ if (drv->states[i].disabled ||
dev->states_usage[i].disable)
+ continue;
+
+ limit = (u64)drv->states[i].target_residency *
NSEC_PER_USEC;
+ break;
+ }
while (!need_resched()) {
cpu_relax();
Re: [PATCH v5 0/9] clk: clkdev: managed clk lookup and provider registrations
Hi de Ho Lee, On Mon, Dec 03, 2018 at 12:19:51PM +, Lee Jones wrote: > > Series add bd71837/bd71837 PMIC clock support + managed interfaces > > You've send me a cover letter and no patches. I am afraid the patches may be still on their way... I added recipients to git format-patch via command-line so you are probably included also in patch mails :( > > I suggest that I should not be on the addressee list at all for this. I'm terribly sorry for spamming! I must've done some mistake with the get_maintainer.pl as I copied the recipients from that output. I may have accidentally included some old patch to maintainer query. > > -- > Lee Jones [李琼斯] > Linaro Services Technical Lead > Linaro.org │ Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog -- Matti Vaittinen ROHM Semiconductors ~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
Re: [PATCH v5 0/9] clk: clkdev: managed clk lookup and provider registrations
On Mon, 03 Dec 2018, Matti Vaittinen wrote: > > Hi de Ho Lee, > > On Mon, Dec 03, 2018 at 12:19:51PM +, Lee Jones wrote: > > > Series add bd71837/bd71837 PMIC clock support + managed interfaces > > > > You've send me a cover letter and no patches. > > > I am afraid the patches may be still on their way... I added recipients > to git format-patch via command-line so you are probably included also > in patch mails :( Ah I see. Yes, I see that they have started to come through. > > I suggest that I should not be on the addressee list at all for this. > > I'm terribly sorry for spamming! I must've done some mistake with the > get_maintainer.pl as I copied the recipients from that output. I may > have accidentally included some old patch to maintainer query. No problem. If you could remove me from future mails though, that would be appreciated. The torrent it fierce enough without receiving correspondence that is not relevant to me. Thanks. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH v3] code-of-conduct: Remove explicit list of discrimination factors
On Mon, Dec 03, 2018 at 12:53:00PM +0100, Geert Uytterhoeven wrote: > Hi Greg, > > On Mon, Dec 3, 2018 at 12:05 PM Greg Kroah-Hartman > wrote: > > On Sun, Dec 02, 2018 at 10:32:57AM +0100, Geert Uytterhoeven wrote: > > > Providing an explicit list of discrimination factors may give the false > > > impression that discrimination based on other unlisted factors would be > > > allowed. > > > > > > Furthermore, this list is already overly long, polarizing, > > > politically-laden, and reinstating the concept of human races. > > > None of these is related to the goals of the Linux kernel project. > > > > > > Avoid any ambiguity or political undertone by removing the list, to > > > ensure "a harassment-free experience for everyone", period. > > > > I understand the reason you and others are proposing this change, > > however for now, let us stick with the text that we have. As Linus and > > I said just over a month ago, let's sit with the text we have until > > something comes up that requires a change to happen. > > > > Also, I recommend you work with the upstream developers of this text to > > see if they agree with your changes here. If they do, and update their > > version, I will be glad to revisit this text at that time. > > I did, cfr. > https://github.com/ContributorCovenant/contributor_covenant/issues/610 > > The official response was: > >"I'm not going to make this change to the Contributor Covenant itself, > since I believe that explicitly listing examples of protected classes is > important. However, any adopting project is free to modify the document > according to the license." I figured. As I said, some people feel that the list is good to have, if not essential. So let's stick with it for now. thanks, greg k-h
Re: [PATCH v4 4/7] cgroup: cgroup v2 freezer
To be honest, I fail to understand this patch. At least after a quick glance,
I will try to read it again tomorrow but so far I do not even understand the
desired semantics wrt signals/ptrace.
On 11/30, Roman Gushchin wrote:
>
> @@ -368,6 +369,8 @@ static inline int signal_pending_state(long state, struct
> task_struct *p)
> return 0;
> if (!signal_pending(p))
> return 0;
> + if (unlikely(cgroup_task_frozen(p)))
> + return __fatal_signal_pending(p);
Oh, this is not nice. And doesn't look right.
> +/*
> + * Entry path into frozen state.
> + * If the task was not frozen before, counters are updated and the cgroup
> state
> + * is revisited. Otherwise, the task is put into the TASK_KILLABLE sleep.
> + */
> +void cgroup_enter_frozen(void)
> +{
> + if (!current->frozen) {
> + struct cgroup *cgrp;
> +
> + spin_lock_irq(&css_set_lock);
> + current->frozen = true;
> + cgrp = task_dfl_cgroup(current);
> + cgrp->freezer.nr_frozen_tasks++;
> + WARN_ON_ONCE(cgrp->freezer.nr_frozen_tasks >
> + cgrp->freezer.nr_tasks_to_freeze);
> + cgroup_update_frozen(cgrp, true);
> + spin_unlock_irq(&css_set_lock);
> + }
> +
> + __set_current_state(TASK_INTERRUPTIBLE);
> + schedule();
The comment above says TASK_KILLABLE, very confusing.
Probably this pairs with the change in signal_pending_state() above. So this
schedule() should actually "work" in that it won't return if signal_pending().
But this can't protect from another signal_wake_up(). Yes, iiuc in this case
cgroup_enter_frozen() will be called again "soon" but this all looks strange.
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -410,6 +410,13 @@ static int ptrace_attach(struct task_struct *task, long
> request,
>
> spin_lock(&task->sighand->siglock);
>
> + /*
> + * If the process is frozen, let's wake it up to give it a chance
> + * to enter the ptrace trap.
> + */
> + if (cgroup_task_frozen(task))
> + wake_up_process(task);
And why this can't race with cgroup_enter_frozen() ?
Or think of PTRACE_INTERRUPT. It can race with cgroup_enter_frozen() too, the
tracee can miss this request because of that change in signal_pending_state().
> static void do_jobctl_trap(void)
> {
> + struct sighand_struct *sighand = current->sighand;
> struct signal_struct *signal = current->signal;
> int signr = current->jobctl & JOBCTL_STOP_SIGMASK;
>
> - if (current->ptrace & PT_SEIZED) {
> - if (!signal->group_stop_count &&
> - !(signal->flags & SIGNAL_STOP_STOPPED))
> - signr = SIGTRAP;
> - WARN_ON_ONCE(!signr);
> - ptrace_do_notify(signr, signr | (PTRACE_EVENT_STOP << 8),
> - CLD_STOPPED);
> - } else {
> - WARN_ON_ONCE(!signr);
> - ptrace_stop(signr, CLD_STOPPED, 0, NULL);
> - current->exit_code = 0;
> + if (current->jobctl & (JOBCTL_TRAP_STOP | JOBCTL_TRAP_NOTIFY)) {
> + if (current->ptrace & PT_SEIZED) {
> + if (!signal->group_stop_count &&
> + !(signal->flags & SIGNAL_STOP_STOPPED))
> + signr = SIGTRAP;
> + WARN_ON_ONCE(!signr);
> + ptrace_do_notify(signr,
> + signr | (PTRACE_EVENT_STOP << 8),
> + CLD_STOPPED);
> + } else {
> + WARN_ON_ONCE(!signr);
> + ptrace_stop(signr, CLD_STOPPED, 0, NULL);
> + current->exit_code = 0;
> + }
> + } else if (current->jobctl & JOBCTL_TRAP_FREEZE) {
> + /*
> + * Enter the frozen state, unless the task is about to exit.
> + */
> + if (fatal_signal_pending(current)) {
> + current->jobctl &= ~JOBCTL_TRAP_FREEZE;
> + } else {
> + spin_unlock_irq(&sighand->siglock);
> + cgroup_enter_frozen();
> + spin_lock_irq(&sighand->siglock);
> + }
> }
> }
>
> @@ -2401,12 +2420,23 @@ bool get_signal(struct ksignal *ksig)
> do_signal_stop(0))
> goto relock;
>
> - if (unlikely(current->jobctl & JOBCTL_TRAP_MASK)) {
> + if (unlikely(current->jobctl &
> + (JOBCTL_TRAP_MASK | JOBCTL_TRAP_FREEZE))) {
> do_jobctl_trap();
Cosmetic nit, but can't you add another helper? To me something like
if (JOBCTL_TRAP_MASK)
do_jobctl_trap();
else if (JOBCTL_TRAP_FREEZE)
do_jobctl_freeze();
will look more clean, but I won't insist.
> + /*
> +
Re: [PATCH v4 4/7] cgroup: cgroup v2 freezer
On 11/30, Roman Gushchin wrote:
>
> +void cgroup_enter_frozen(void)
> +{
> + if (!current->frozen) {
> + struct cgroup *cgrp;
> +
> + spin_lock_irq(&css_set_lock);
> + current->frozen = true;
> + cgrp = task_dfl_cgroup(current);
> + cgrp->freezer.nr_frozen_tasks++;
> + WARN_ON_ONCE(cgrp->freezer.nr_frozen_tasks >
> + cgrp->freezer.nr_tasks_to_freeze);
> + cgroup_update_frozen(cgrp, true);
> + spin_unlock_irq(&css_set_lock);
> + }
> +
> + __set_current_state(TASK_INTERRUPTIBLE);
> + schedule();
> + __set_current_state(TASK_RUNNING);
forgot to mention, schedule() always returns in TASK_RUNNING, so the 2nd
__set_current_state() is pointless.
Oleg.
Re: [PATCH 1/2 v2] kdump: add the vmcoreinfo documentation
Add some more Ccs. On Sun, Dec 02, 2018 at 11:08:38AM +0800, Lianbo Jiang wrote: > This document lists some variables that export to vmcoreinfo, and briefly > describles what these variables indicate. It should be instructive for > many people who do not know the vmcoreinfo, and it also normalizes the > exported variable as a standard ABI between kernel and use-space. Yeah, I'm not sure about it being an ABI. Apparently, it is considered too tightly coupled to the kernel for it to be an ABI. Regardless, thanks for doing that. > Suggested-by: Borislav Petkov > Signed-off-by: Lianbo Jiang > --- > Documentation/kdump/vmcoreinfo.txt | 400 + > 1 file changed, 400 insertions(+) > create mode 100644 Documentation/kdump/vmcoreinfo.txt > > diff --git a/Documentation/kdump/vmcoreinfo.txt > b/Documentation/kdump/vmcoreinfo.txt Aren't we adding new docs in rst format only or what is the logic there? Jon? > new file mode 100644 > index ..c6759be14af7 > --- /dev/null > +++ b/Documentation/kdump/vmcoreinfo.txt > @@ -0,0 +1,400 @@ > + > + Documentation for Vmcoreinfo > + > + > +=== > +What is the vmcoreinfo? > +=== > +The vmcoreinfo contains the first kernel's various information, for The first sentence here should be explaining what VMCOREINFO is: it is an ELF PT_NOTE section. So that people can go, oh ok, it is a special ELF section, when reading. Then, MAKEDUMPFILE(8) spells VMCOREINFO in all caps and I think we should do that too here, for ease of recognition. Btw, do we have a makedumpfile switch or a tool/script which dumps VMCOREINFO contents in human-readable form? Maybe something nicer than: $ hexdump -C /proc/kcore > +example, structure size, page size, symbol values and field offset, > +etc. These data are encapsulated into an elf format, and these data > +will also help user-space tools(e.g. makedumpfile, crash) analyze the > +first kernel's memory usage. > + > + > +Common variables > + > + > +init_uts_ns.name.release > + > +The number of OS release. > + > +PAGE_SIZE > += > +The size of a page. It is usually 4k bytes. > + > +init_uts_ns > +=== > +This is the UTS namespace, which is used to isolate two specific elements > +of the system that relate to the uname system call. The UTS namespace is > +named after the data structure used to store information returned by the > +uname system call. Those non-obvious exports should also have a short explanation why they're part of VMCOREINFO. > + > +node_online_map > +=== > +It is a macro definition, actually it is an arrary node_states[N_ONLINE], > +and it represents the set of online node in a system, one bit position > +per node number. Ditto. So yeah, people can find out what those things are but I think it is more important to state here *why* they're part of VMCOREINFO and how they're used and why they're exported. Who knows, some might turn out to be not needed anymore. :) > + > +swapper_pg_dir > += > +It is always an array, it gerenally stands for the pgd for the kernel. > +When mmu is enabled in config file, the 'swapper_pg_dir' is valid. > + > +_stext > +== > +It is an assemble directive that defines the beginning of the text section. That's an assembly symbol. > +In gerenal, the '_stext' indicates the kernel start address. > + > +vmap_area_list > +== > +It stores the virtual area list, makedumpfile can get the vmalloc start > +value according to this variable. "... from this variable." > + > +mem_map > +=== > +Physical addresses are translated to struct pages by treating them as an > +index into the mem_map array. Shifting a physical address PAGE_SHIFT bits > +to the right will treat it as a PFN from physical address 0, which is also > +an index within the mem_map array. > + > +In a word, it can map the address to struct page. "In short, ... " > + > +contig_page_data > + > +Makedumpfile can get the pglist_data structure according to this symbol Please look up in the dictionary what "according" means. Using it in this context is at least weird. > +'contig_page_data'. The pglist_data structure is used to describe the > +memory layout. > + > +mem_section|(mem_section, NR_SECTION_ROOTS)|(mem_section, section_mem_map) > +== > +Export the address of 'mem_section' array, and it's length, structure size, > +and the 'section_mem_map' offset. > + > +It exists in the sparse memory mapping model, and it is also somewhat > +similar to the mem_map variable, both of them will help to translate > +the address. > + > +page > + > +The size of a 'page' structure. > + > +pglist_data > +=== > +The size of a 'pglist_dat
Re: [PATCH v4 4/7] cgroup: cgroup v2 freezer
On 11/30, Roman Gushchin wrote:
>
> void recalc_sigpending(void)
> {
> if (!recalc_sigpending_tsk(current) && !freezing(current) &&
> - !klp_patch_pending(current))
> + !klp_patch_pending(current) && !cgroup_task_frozen(current))
I must have missed something... but shouldn't it check JOBCTL_TRAP_FREEZE
rather than ->frozen?
Oleg.
Re: [PATCH v4 3/7] cgroup: protect cgroup->nr_(dying_)descendants by css_set_lock
On Fri, Nov 30, 2018 at 03:47:41PM -0800, Roman Gushchin wrote: > + * nr_descendants and nr_dying_descendants are protected > + * by cgroup_mutex and css_set_lock. Can you be a bit more specific - hold both for writes, either for reads. Thanks. -- tejun
Re: [PATCH v3] code-of-conduct: Remove explicit list of discrimination factors
On Mon, Dec 3, 2018 at 4:15 AM Pavel Machek wrote: > > Linus, I don't think Greg is doing good job maintaining this. Can you > take the patch? (Or explain what is going on here, because I don't > think public has full story). I think Greg is doing a fine job, and I personally will not take patches to the CoC without *much* stronger reasons than just some random opinion. This is an area where everybody has opinions, and there is no inherent technical agreement (ie no "look, numbers: this makes code 5% faster"), so my policy wrt the CoC is that changes to it have to have very concrete reasons for them. IOW, actual problems caused by real issues, not "in my opinion". Honestly, the list of discrimination factors looks fine to me, and if it is shown to be insufficient or problematic, there's the documented link to the Code of Conduct Committee. That sounds to me like the right way to bring up problematic behavior - and if the review then shows that "yes, the CoC should cover this too", then at that point we have a real and present example and reason to modify the CoC. Linus
Re: [PATCH v4 3/7] cgroup: protect cgroup->nr_(dying_)descendants by css_set_lock
On Mon, Dec 03, 2018 at 08:17:06AM -0800, Tejun Heo wrote: > On Fri, Nov 30, 2018 at 03:47:41PM -0800, Roman Gushchin wrote: > > +* nr_descendants and nr_dying_descendants are protected > > +* by cgroup_mutex and css_set_lock. > > Can you be a bit more specific - hold both for writes, either for > reads. Sure. Thanks!
Re: [PATCH v5 1/9] clkdev: add managed clkdev lookup registration
Hi,
On Mon, Dec 03, 2018 at 02:17:56PM +0200, Matti Vaittinen wrote:
> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index 9ab3db8b3988..68a1e55a60d2 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -401,6 +401,25 @@ static struct clk_lookup *__clk_register_clkdev(struct
> clk_hw *hw,
> return cl;
> }
>
> +static int do_clk_register_clkdev(struct clk_hw *hw,
> + struct clk_lookup **cl, const char *con_id, const char *dev_id)
> +{
> +
No need for this blank line.
> + if (IS_ERR(hw))
> + return PTR_ERR(hw);
> + /*
> + * Since dev_id can be NULL, and NULL is handled specially, we must
> + * pass it as either a NULL format string, or with "%s".
> + */
> + if (dev_id)
> + *cl = __clk_register_clkdev(hw, con_id, "%s",
> +dev_id);
Please move "dev_id);" onto the line above.
> @@ -420,20 +439,10 @@ int clk_register_clkdev(struct clk *clk, const char
> *con_id,
> {
> struct clk_lookup *cl;
>
> - if (IS_ERR(clk))
> - return PTR_ERR(clk);
> -
> - /*
> - * Since dev_id can be NULL, and NULL is handled specially, we must
> - * pass it as either a NULL format string, or with "%s".
> - */
> - if (dev_id)
> - cl = __clk_register_clkdev(__clk_get_hw(clk), con_id, "%s",
> -dev_id);
> - else
> - cl = __clk_register_clkdev(__clk_get_hw(clk), con_id, NULL);
> -
> - return cl ? 0 : -ENOMEM;
> + if (!IS_ERR(clk))
> + return do_clk_register_clkdev(__clk_get_hw(clk), &cl, con_id,
> + dev_id);
> + return PTR_ERR(clk);
Please keep this as:
if (IS_ERR(clk))
return PTR_ERR(clk);
return do_clk_register_clkdev(__clk_get_hw(clk), &cl, con_id,
dev_id);
rather than inverting it - it makes it more consistent with code elsewhere.
> +static int devm_clk_match_clkdev(struct device *dev, void *res, void *data)
> +{
> + struct clk_lookup **l = res;
> +
> + if (!l || !*l) {
> + WARN_ON(!l || !*l);
How can "l" be NULL here? How can *l be NULL?
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
[PATCH v2 01/34] kbuild: Add support for DT binding schema checks
This adds the build infrastructure for checking DT binding schema documents and validating dts files using the binding schema. Check DT binding schema documents: make dt_binding_check Build dts files and check using DT binding schema: make dtbs_check Optionally, DT_SCHEMA_FILES can passed in with a schema file(s) to use for validation. This makes it easier to find and fix errors generated by a specific schema. Currently, the validation targets are separate from a normal build to avoid a hard dependency on the external DT schema project and because there are lots of warnings generated. Cc: Jonathan Corbet Cc: Mark Rutland Cc: Masahiro Yamada Cc: Michal Marek Cc: linux-doc@vger.kernel.org Cc: devicet...@vger.kernel.org Cc: linux-kbu...@vger.kernel.org Signed-off-by: Rob Herring --- .gitignore | 1 + Documentation/Makefile | 2 +- Documentation/devicetree/bindings/.gitignore | 1 + Documentation/devicetree/bindings/Makefile | 33 Makefile | 11 +-- scripts/Makefile.lib | 24 -- 6 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/.gitignore create mode 100644 Documentation/devicetree/bindings/Makefile diff --git a/.gitignore b/.gitignore index 97ba6b79834c..a20ac26aa2f5 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ *.bin *.bz2 *.c.[012]*.* +*.dt.yaml *.dtb *.dtb.S *.dwo diff --git a/Documentation/Makefile b/Documentation/Makefile index 2ca77ad0f238..9786957c6a35 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -2,7 +2,7 @@ # Makefile for Sphinx documentation # -subdir-y := +subdir-y := devicetree/bindings/ # You can set these variables from the command line. SPHINXBUILD = sphinx-build diff --git a/Documentation/devicetree/bindings/.gitignore b/Documentation/devicetree/bindings/.gitignore new file mode 100644 index ..d9194c02dd08 --- /dev/null +++ b/Documentation/devicetree/bindings/.gitignore @@ -0,0 +1 @@ +*.example.dts diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile new file mode 100644 index ..ee0110dd8131 --- /dev/null +++ b/Documentation/devicetree/bindings/Makefile @@ -0,0 +1,33 @@ +# SPDX-License-Identifier: GPL-2.0 +DT_DOC_CHECKER ?= dt-doc-validate +DT_EXTRACT_EX ?= dt-extract-example +DT_MK_SCHEMA ?= dt-mk-schema +DT_MK_SCHEMA_FLAGS := $(if $(DT_SCHEMA_FILES), -u) + +quiet_cmd_chk_binding = CHKDT $< + cmd_chk_binding = (set -e; \ + $(DT_DOC_CHECKER) $< ; \ + mkdir -p $(dir $@) ; \ + $(DT_EXTRACT_EX) $< > $@ ) + +$(obj)/%.example.dts: $(src)/%.yaml FORCE + $(call if_changed,chk_binding) + +DT_TMP_SCHEMA := .schema.yaml.tmp +extra-y += $(DT_TMP_SCHEMA) + +quiet_cmd_mk_schema = SCHEMA $@ + cmd_mk_schema = mkdir -p $(obj); \ + rm -f $@; \ + $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $< + +DT_DOCS = $(shell cd $(srctree)/$(src) && find * -name '*.yaml') +DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(DT_DOCS)) + +extra-y += $(patsubst $(src)/%.yaml,%.example.dts, $(DT_SCHEMA_FILES)) +extra-y += $(patsubst $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES)) + +$(obj)/$(DT_TMP_SCHEMA): $(addprefix $(obj)/,$(patsubst $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES))) + +$(obj)/$(DT_TMP_SCHEMA): $(addprefix $(srctree)/, $(DT_SCHEMA_FILES)) FORCE + $(call if_changed,mk_schema) diff --git a/Makefile b/Makefile index 2f36db897895..ff59adf43300 100644 --- a/Makefile +++ b/Makefile @@ -1232,10 +1232,13 @@ ifneq ($(dtstree),) %.dtb: prepare3 scripts_dtc $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ -PHONY += dtbs dtbs_install +PHONY += dtbs dtbs_install dt_binding_check dtbs: prepare3 scripts_dtc $(Q)$(MAKE) $(build)=$(dtstree) +dtbs_check: prepare3 dt_binding_check + $(Q)$(MAKE) $(build)=$(dtstree) CHECK_DTBS=1 + dtbs_install: $(Q)$(MAKE) $(dtbinst)=$(dtstree) @@ -1249,6 +1252,9 @@ PHONY += scripts_dtc scripts_dtc: scripts_basic $(Q)$(MAKE) $(build)=scripts/dtc +dt_binding_check: scripts_dtc + $(Q)$(MAKE) $(build)=Documentation/devicetree/bindings + # --- # Modules @@ -1611,7 +1617,8 @@ clean: $(clean-dirs) $(call cmd,rmfiles) @find $(if $(KBUILD_EXTMOD), $(KBUILD_EXTMOD), .) $(RCS_FIND_IGNORE) \ \( -name '*.[aios]' -o -name '*.ko' -o -name '.*.cmd' \ - -o -name '*.ko.*' -o -name '*.dtb' -o -name '*.dtb.S' \ + -o -name '*.ko.*' \ + -o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \ -o -name '*.dwo' -o -name '*.lst' \ -o -name '*.su' \ -o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.
Re: [PATCH v4 1/8] clk: clkdev/of_clk - add managed lookup and provider registrations
Quoting Matti Vaittinen (2018-11-30 02:50:22)
> Hello Stephen,
>
> Thanks a bunch for taking the time and reviewing this!
>
> On Fri, Nov 30, 2018 at 12:54:10AM -0800, Stephen Boyd wrote:
> > Quoting Matti Vaittinen (2018-11-13 03:55:58)
> > > With MFD devices the clk properties may be contained in MFD (parent) DT
> > > node. Current devm_of_clk_add_hw_provider assumes the clk is bound to MFD
> > > subdevice not to MFD device (parent). Add
> > > devm_of_clk_add_hw_provider_parent to tackle this issue.
> > >
> > > Also clkdev registration lacks of managed registration functions and it
> > > seems few drivers do not drop clkdev lookups at exit. Add
> > > devm_clk_hw_register_clkdev and devm_clk_release_clkdev to ease lookup
> > > releasing at exit.
> >
> > Please split this into clkdev and non-clkdev devm functionality.
> Allright. I'll split this to two patches.
>
> > > --- a/Documentation/driver-model/devres.txt
> > > +++ b/Documentation/driver-model/devres.txt
> > > @@ -238,6 +238,9 @@ CLOCK
> > >devm_clk_put()
> > >devm_clk_hw_register()
> > >devm_of_clk_add_hw_provider()
> > > + devm_of_clk_add_parent_hw_provider()
> > > + devm_clk_hw_register_clkdev()
> > > + devm_clk_release_clkdev()
> >
> > The 'release' or non-common functions shouldn't be documented here.
> So I will drop the line mentioning devm_clk_release_clkdev()
>
> > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > > index af011974d4ec..9bb921eb90f6 100644
> > > --- a/drivers/clk/clk.c
> > > +++ b/drivers/clk/clk.c
> > > @@ -3893,12 +3893,12 @@ static void devm_of_clk_release_provider(struct
> > > device *dev, void *res)
> > > of_clk_del_provider(*(struct device_node **)res);
> > > }
> > >
> > > -int devm_of_clk_add_hw_provider(struct device *dev,
> > > +static int __devm_of_clk_add_hw_provider(struct device *dev,
> > > struct clk_hw *(*get)(struct of_phandle_args
> > > *clkspec,
> > > void *data),
> > > - void *data)
> > > + struct device_node *of_node, void *data)
> > > {
> > > - struct device_node **ptr, *np;
> > > + struct device_node **ptr;
> > > int ret;
> > >
> > > ptr = devres_alloc(devm_of_clk_release_provider, sizeof(*ptr),
> > > @@ -3906,10 +3906,9 @@ int devm_of_clk_add_hw_provider(struct device *dev,
> > > if (!ptr)
> > > return -ENOMEM;
> > >
> > > - np = dev->of_node;
> > > - ret = of_clk_add_hw_provider(np, get, data);
> > > + *ptr = of_node;
> > > + ret = of_clk_add_hw_provider(of_node, get, data);
> > > if (!ret) {
> > > - *ptr = np;
> >
> > Why is this moved outside of the if condition?
> I completely removed the local variable np and just unconditionally set
> the allocated devres to point at the node (if allocation succeeded). We
> could of course only do this if the provider registration succeeded and
> save one assignment - but I guess I intended to remove the curly braces
> and thus decided to go for one liner after if. But apparently I didn't
> remove the braces O_o. Well, I can put the assignment inside the
> condition if you prefer that.
>
> > In fact, why isn't just
> > the first line in this hunk deleted and passed to this function as
> > struct device_node *np?
>
> I am sorry but I don't quite follow your suggestion here. Do you mean we
> could just pass the struct device_node *np in devres_add()? I thought
> the pointer passed to devress_add() should be allocated using
> devres_alloc. Can you please elaborate what you mean?
I'm just trying to reduce the diff in the patch.
>
> >
> > > devres_add(dev, ptr);
> > > } else {
> > > devres_free(ptr);
[..]
> >
> > > +int devm_of_clk_add_hw_provider(struct device *dev,
> > > + struct clk_hw *(*get)(struct of_phandle_args
> > > *clkspec,
> > > + void *data),
> > > + void *data)
> > > +{
> > > + return __devm_of_clk_add_hw_provider(dev, get, dev->of_node,
> > > data);
> > > +}
> > > EXPORT_SYMBOL_GPL(devm_of_clk_add_hw_provider);
> > >
> > > +int devm_of_clk_add_parent_hw_provider(struct device *dev,
> > > + struct clk_hw *(*get)(struct of_phandle_args
> > > *clkspec,
> > > + void *data),
> > > + void *data)
> > > +{
> > > + return __devm_of_clk_add_hw_provider(dev, get,
> > > dev->parent->of_node,
> >
> > I'm wondering if we can somehow auto-detect this in
> > devm_of_clk_add_hw_provider() by looking for #clock-cells in the node.
> > If it isn't there, then we go to the parent node and look for a
> > #clock-cells property there in the DT node for that device. Does that
> > make sense? Then there isn't any new API and we can attach the lifetime
> > of the devm registration to th
Re: [PATCH v4 4/7] cgroup: cgroup v2 freezer
On Mon, Dec 03, 2018 at 03:47:18PM +0100, Oleg Nesterov wrote:
> To be honest, I fail to understand this patch. At least after a quick glance,
> I will try to read it again tomorrow but so far I do not even understand the
> desired semantics wrt signals/ptrace.
>
> On 11/30, Roman Gushchin wrote:
> >
> > @@ -368,6 +369,8 @@ static inline int signal_pending_state(long state,
> > struct task_struct *p)
> > return 0;
> > if (!signal_pending(p))
> > return 0;
> > + if (unlikely(cgroup_task_frozen(p)))
> > + return __fatal_signal_pending(p);
>
> Oh, this is not nice. And doesn't look right.
>
> > +/*
> > + * Entry path into frozen state.
> > + * If the task was not frozen before, counters are updated and the cgroup
> > state
> > + * is revisited. Otherwise, the task is put into the TASK_KILLABLE sleep.
> > + */
> > +void cgroup_enter_frozen(void)
> > +{
> > + if (!current->frozen) {
> > + struct cgroup *cgrp;
> > +
> > + spin_lock_irq(&css_set_lock);
> > + current->frozen = true;
> > + cgrp = task_dfl_cgroup(current);
> > + cgrp->freezer.nr_frozen_tasks++;
> > + WARN_ON_ONCE(cgrp->freezer.nr_frozen_tasks >
> > +cgrp->freezer.nr_tasks_to_freeze);
> > + cgroup_update_frozen(cgrp, true);
> > + spin_unlock_irq(&css_set_lock);
> > + }
> > +
> > + __set_current_state(TASK_INTERRUPTIBLE);
> > + schedule();
>
> The comment above says TASK_KILLABLE, very confusing.
Sorry, it's a leftover from one of the previous versions. Fixed.
>
> Probably this pairs with the change in signal_pending_state() above. So this
> schedule() should actually "work" in that it won't return if signal_pending().
>
> But this can't protect from another signal_wake_up(). Yes, iiuc in this case
> cgroup_enter_frozen() will be called again "soon" but this all looks strange.
So, the idea here is to make ptrace traps and fatal signals working, but
non-fatal shouldn't be delivered.
As soon as the frozen task is looping in the signal delivery loop, it's fine,
it's not going anywhere.
Without the change above the task is getting out of schedule() immediately,
if any signal is pending (including non-fatals). So it becomes a busy-loop.
>
> > --- a/kernel/ptrace.c
> > +++ b/kernel/ptrace.c
> > @@ -410,6 +410,13 @@ static int ptrace_attach(struct task_struct *task,
> > long request,
> >
> > spin_lock(&task->sighand->siglock);
> >
> > + /*
> > +* If the process is frozen, let's wake it up to give it a chance
> > +* to enter the ptrace trap.
> > +*/
> > + if (cgroup_task_frozen(task))
> > + wake_up_process(task);
>
> And why this can't race with cgroup_enter_frozen() ?
>
> Or think of PTRACE_INTERRUPT. It can race with cgroup_enter_frozen() too, the
> tracee can miss this request because of that change in signal_pending_state().
It's a good point. So I need an additional synchronization around
checking/setting the JOBCTL_TRAP_FREEZE?
>
>
> > static void do_jobctl_trap(void)
> > {
> > + struct sighand_struct *sighand = current->sighand;
> > struct signal_struct *signal = current->signal;
> > int signr = current->jobctl & JOBCTL_STOP_SIGMASK;
> >
> > - if (current->ptrace & PT_SEIZED) {
> > - if (!signal->group_stop_count &&
> > - !(signal->flags & SIGNAL_STOP_STOPPED))
> > - signr = SIGTRAP;
> > - WARN_ON_ONCE(!signr);
> > - ptrace_do_notify(signr, signr | (PTRACE_EVENT_STOP << 8),
> > -CLD_STOPPED);
> > - } else {
> > - WARN_ON_ONCE(!signr);
> > - ptrace_stop(signr, CLD_STOPPED, 0, NULL);
> > - current->exit_code = 0;
> > + if (current->jobctl & (JOBCTL_TRAP_STOP | JOBCTL_TRAP_NOTIFY)) {
> > + if (current->ptrace & PT_SEIZED) {
> > + if (!signal->group_stop_count &&
> > + !(signal->flags & SIGNAL_STOP_STOPPED))
> > + signr = SIGTRAP;
> > + WARN_ON_ONCE(!signr);
> > + ptrace_do_notify(signr,
> > +signr | (PTRACE_EVENT_STOP << 8),
> > +CLD_STOPPED);
> > + } else {
> > + WARN_ON_ONCE(!signr);
> > + ptrace_stop(signr, CLD_STOPPED, 0, NULL);
> > + current->exit_code = 0;
> > + }
> > + } else if (current->jobctl & JOBCTL_TRAP_FREEZE) {
> > + /*
> > +* Enter the frozen state, unless the task is about to exit.
> > +*/
> > + if (fatal_signal_pending(current)) {
> > + current->jobctl &= ~JOBCTL_TRAP_FREEZE;
> > + } else {
> > + spin_unlock_irq(&sighand->siglock);
> > + cgroup_enter_frozen();
> > + spin_lock_irq(&sighand->siglock);
> > + }
> >
Re: [PATCH] i3c: master: dw: split dw-i3c-master.c into master and bus specific parts
Hi Boris,
Sorry for the delayed response.
On 27/11/18 12:33, Boris Brezillon wrote:
Hi Vitor,
On Tue, 27 Nov 2018 11:50:53 +
vitor wrote:
Sorry for that and don't take me wrong... maybe I should rise this
question early but this only came up now when I started splitting and
thinking where to put what is for master for slave, what is common and
the thing of putting everything of controller in a folder.
So you have such a dual-role controller?
Yes, we already talked about secondary master support.
There's a difference between a secondary master that waits for its time
to become the currrent master, and a secondary master that provides I3C
device features when it's acting as a slave (sensor, GPIO
controller, ...). So far we focused on supporting the former. If
there's a need for the latter, then we should start thinking about the
slave framework...
What I call a slave controller would be something that lets you reply to
SDR/DDR transactions or fill a generic regmap that would be exposed to
other masters on the bus. This way we could implement generic slave
drivers in Linux (the same way we have gadget drivers). Anything else
is likely to be to specific to be exposed as a generic framework.
I would bet to do something like in i2c, we don't need the same level of
complexity found in USB.
Can you detail a bit more what you have in mind? I don't think we can
do like I2C, simply because we need to expose a valid DCR +
manuf-ID/PID so that other masters can bind the device to the
appropriate driver on their side. Plus, if we're about to expose
generic profiles, we likely don't want each I3C slave controller driver
to do that on its own.
I think this should be discuss in another thread. Do you agree?
Taking the USB as exemple do you prefer a dwc folder on i3c root?
Hm, not sure I like this idea either. So I see 2 options:
1/ put all controller drivers (both master and slave ones) in a common
directory (drivers/i3c/controllers) as you suggest, and prefix them
correctly (i3c-master-.c, i3c-slave-.c and i3c-dual-.c)
I agree with the controller folder but not with prefix. Please check
what is already in the kernel.
If we mix everything in the same subdir, I'd like to have an easy way
to quickly identify those that are slave controllers and those that are
master controllers. For the dual-role thing, maybe we can consider them
as master (ones with advances slave features).
Would you be okay with drivers/i3c/controllers/{designware,dw}/..., so
that you can have all designware drivers (for both slave and master
blocks) in the same dir?
Yes, that was what I trying to tell you. For me this might be the best
option.
I would like to avoid having dual role i3c driver in a master folder.
For those that are placed directly under drivers/i3c/controllers/...
(because they only have one .c file), I'd like to keep a standard
prefix.
I don't disagree, and for those that have more than one file they should
be in a folder, right?
What prefix do you have in mind for those files inside a folder?
To be clear, the subsystem is nice and I working with daily. As I said
this is something that I dealing now and I'm telling what I think that
is not correct.
Come on! All I've seen so far are complaints on tiny details, it
definitely doesn't prevent you from adding new features.
Regards,
Boris
No, it's not. But as you can see to slipt the driver in parts this
subject has some relevance.
Best regards,
Vitor Soares
[RFC] net: documentation: build a directory structure for drivers
Documentation/networking/ is full of cryptically named files with
driver documentation. This makes finding interesting information
at a glance really hard. Move all those files into a directory
called device_drivers (since not all drivers are for device) and
fix up references.
RFC v0.1 -> RFC v1:
- also add .txt suffix to the files which are missing it (Quentin)
Signed-off-by: Jakub Kicinski
Reviewed-by: Quentin Monnet
---
I'm posting as an RFC to see what the general feeling is about
such a patch. I can split into per-driver changes later if that's
the best way forward.
---
.../devicetree/bindings/misc/fsl,qoriq-mc.txt | 2 +-
.../{ => device_drivers/3com}/3c509.txt | 0
.../{ => device_drivers/3com}/vortex.txt | 2 +-
.../{ => device_drivers/amazon}/ena.txt | 0
.../{ => device_drivers/chelsio}/cxgb.txt | 0
.../{ => device_drivers/cirrus}/cs89x0.txt| 0
.../{ => device_drivers/davicom}/dm9000.txt | 0
.../{ => device_drivers/dec}/de4x5.txt| 0
.../{ => device_drivers/dec}/dmfe.txt | 0
.../{ => device_drivers/dlink}/dl2k.txt | 0
.../{ => device_drivers/freescale}/dpaa.txt | 0
.../freescale}/dpaa2/dpio-driver.rst | 4 +-
.../freescale}/dpaa2/ethernet-driver.rst | 2 +-
.../freescale}/dpaa2/index.rst| 0
.../freescale}/dpaa2/overview.rst | 0
.../freescale}/gianfar.txt| 0
.../{ => device_drivers/intel}/e100.rst | 0
.../{ => device_drivers/intel}/e1000.rst | 0
.../{ => device_drivers/intel}/e1000e.rst | 0
.../{ => device_drivers/intel}/fm10k.rst | 0
.../{ => device_drivers/intel}/i40e.rst | 0
.../{ => device_drivers/intel}/iavf.rst | 0
.../{ => device_drivers/intel}/ice.rst| 0
.../{ => device_drivers/intel}/igb.rst| 0
.../{ => device_drivers/intel}/igbvf.rst | 0
.../intel/ipw2100.txt}| 0
.../intel/ipw2200.txt}| 0
.../{ => device_drivers/intel}/ixgb.rst | 0
.../{ => device_drivers/intel}/ixgbe.rst | 0
.../{ => device_drivers/intel}/ixgbevf.rst| 0
.../{ => device_drivers/microsoft}/netvsc.txt | 0
.../{ => device_drivers/neterion}/s2io.txt| 0
.../{ => device_drivers/neterion}/vxge.txt| 0
.../qlogic}/LICENSE.qla3xxx | 0
.../qlogic}/LICENSE.qlcnic| 0
.../{ => device_drivers/qlogic}/LICENSE.qlge | 0
.../{ => device_drivers/qualcomm}/rmnet.txt | 0
.../sb1000.txt} | 0
.../{ => device_drivers/smsc}/smc9.txt| 0
.../{ => device_drivers/stmicro}/stmmac.txt | 0
.../ti/cpsw.txt} | 0
.../{ => device_drivers/ti}/tlan.txt | 0
.../toshiba}/spider_net.txt | 0
MAINTAINERS | 48 +--
drivers/net/Kconfig | 8 ++--
drivers/net/ethernet/3com/3c59x.c | 4 +-
drivers/net/ethernet/3com/Kconfig | 5 +-
drivers/net/ethernet/chelsio/Kconfig | 3 +-
drivers/net/ethernet/cirrus/Kconfig | 2 +-
drivers/net/ethernet/dec/tulip/Kconfig| 4 +-
drivers/net/ethernet/dlink/dl2k.c | 2 +-
drivers/net/ethernet/intel/Kconfig| 24 +-
drivers/net/ethernet/neterion/Kconfig | 4 +-
drivers/net/ethernet/smsc/Kconfig | 4 +-
drivers/net/ethernet/ti/Kconfig | 3 +-
drivers/net/ethernet/ti/tlan.c| 4 +-
drivers/net/wireless/intel/ipw2x00/Kconfig| 10 ++--
drivers/net/wireless/intel/ipw2x00/ipw2100.c | 2 +-
58 files changed, 71 insertions(+), 66 deletions(-)
rename Documentation/networking/{ => device_drivers/3com}/3c509.txt (100%)
rename Documentation/networking/{ => device_drivers/3com}/vortex.txt (99%)
rename Documentation/networking/{ => device_drivers/amazon}/ena.txt (100%)
rename Documentation/networking/{ => device_drivers/chelsio}/cxgb.txt (100%)
rename Documentation/networking/{ => device_drivers/cirrus}/cs89x0.txt (100%)
rename Documentation/networking/{ => device_drivers/davicom}/dm9000.txt (100%)
rename Documentation/networking/{ => device_drivers/dec}/de4x5.txt (100%)
rename Documentation/networking/{ => device_drivers/dec}/dmfe.txt (100%)
rename Documentation/networking/{ => device_drivers/dlink}/dl2k.txt (100%)
rename Documentation/networking/{ => device_drivers/freescale}/dpaa.txt (100%)
rename Documentation/networking/{ =>
device_drivers/freescale}/dpaa2/dpio-driver.rst (97%)
rename Documentation/networking/{ =>
device_drivers/freescale}/dpaa2/ethernet-driver.rst (98%)
rename Documentation/networking/{ => device_drivers/freescale}/dpaa2/index.rst
(100%)
rename Documentation/networking/{ =>
device_drivers/freescale}/dpaa2/overview.rst (100%)
rename Documentation/networking/{ => device_drivers/freescale}/gianfar.txt
(100%)
rename Documentation/networkin
Re: [RFC] net: documentation: build a directory structure for drivers
On 12/3/18 6:43 PM, Jakub Kicinski wrote: > Documentation/networking/ is full of cryptically named files with > driver documentation. This makes finding interesting information > at a glance really hard. Move all those files into a directory > called device_drivers (since not all drivers are for device) and > fix up references. > > RFC v0.1 -> RFC v1: > - also add .txt suffix to the files which are missing it (Quentin) > > Signed-off-by: Jakub Kicinski > Reviewed-by: Quentin Monnet > --- > I'm posting as an RFC to see what the general feeling is about > such a patch. I can split into per-driver changes later if that's > the best way forward. LGTM. Thanks for taking the initiative on this. Acked-by: David Ahern
Re: [PATCH v8 1/2] perf, uncore: Adding documentation for ThunderX2 pmu uncore driver
Hi Will, On Mon, Dec 3, 2018 at 5:39 PM Will Deacon wrote: > > On Thu, Nov 22, 2018 at 03:04:31AM +, Kulkarni, Ganapatrao wrote: > > The SoC has PMU support in its L3 cache controller (L3C) and in the > > DDR4 Memory Controller (DMC). > > > > Signed-off-by: Ganapatrao Kulkarni > > --- > > Documentation/perf/thunderx2-pmu.txt | 106 +++ > > 1 file changed, 106 insertions(+) > > create mode 100644 Documentation/perf/thunderx2-pmu.txt > > Thanks for writing the documentation, although I think it needs a bit of > help before we can merge it. sure will send next version ASAP. > > > diff --git a/Documentation/perf/thunderx2-pmu.txt > > b/Documentation/perf/thunderx2-pmu.txt > > new file mode 100644 > > index ..9f5dd7459e68 > > --- /dev/null > > +++ b/Documentation/perf/thunderx2-pmu.txt > > @@ -0,0 +1,106 @@ > > + > > +Cavium ThunderX2 SoC Performance Monitoring Unit (PMU UNCORE) > > +== > > + > > +ThunderX2 SoC PMU consists of independent system wide per Socket PMUs such > > +as Level 3 Cache(L3C) and DDR4 Memory Controller(DMC). > > Please add some punctuation here. Thanks will do. > > > + > > +DMC has 8 interleave channels and L3C has 16 interleave tiles. Events are > > *The* DMC and *the* L3C ok > > > > +sampled for default channel(i.e channel 0) and prorated to total number of > > I'm not sure I understand this; are you saying it's not possible to sample > channels other than channel 0? yes, sampling channel zero, since channels are interleave, multiplying by number of channels will give fair data. Removed per channel sample since it was involved SMC calls. > > > +channels/tiles. > > + > > +DMC and L3C, Each PMU supports up to 4 counters. Counters are independently > > The start of this sentence makes no sense and you've got a capital "Each". > > > +programmable and can be started and stopped individually. Each counter can > > +be set to sample specific perf events. Counters are 32 bit and do not > > support > > +overflow interrupt; they are sampled at every 2 seconds. > > I think this is unfortunate wording, because actually you don't support what > perf calls "sampling" at all. ok, let me rephrase it. > > > + > > +PMU UNCORE (perf) driver: > > + > > +The thunderx2-pmu driver registers several perf PMUs for DMC and L3C > > devices. > > I think the driver name uses an underscore instead of a hyphen. thanks. > > > +Each of the PMUs provides description of its available events > > +and configuration options in sysfs. > > + see /sys/devices/uncore_ > > + > > +S is socket id. > > *the* socket id ok > > > +Each PMU can be used to sample up to 4 events simultaneously. > > + > > +The "format" directory describes format of the config (event ID). > > +The "events" directory provides configuration templates for all > > +supported event types that can be used with perf tool. > > You can drop this bit, since it's not specific to your PMU and is actually > describing the perf ABI via sysfs. If we want to document that someplace, it > should be in a separate file. ok , let me drop sysfs part. > > > + > > +For example, "uncore_dmc_0/cnt_cycles/" is an > > +equivalent of "uncore_dmc_0/config=0x1/". > > Why is this helpful? ok let me drop second line. > > > + > > +Each perf driver also provides a "cpumask" sysfs attribute, which contains > > a > > +single CPU ID of the processor which is likely to be used to handle all the > > +PMU events. It will be the first online CPU from the NUMA node of the PMU > > device. > > Again, I don't think this really belongs in here. ok. > > > + > > +Example for perf tool use: > > + > > +perf stat -a -e uncore_dmc_0/cnt_cycles/ sleep 1 > > + > > +perf stat -a -e \ > > +uncore_dmc_0/cnt_cycles/,\ > > +uncore_dmc_0/data_transfers/,\ > > +uncore_dmc_0/read_txns/,\ > > +uncore_dmc_0/write_txns/ sleep 1 > > + > > +perf stat -a -e \ > > +uncore_l3c_0/read_request/,\ > > +uncore_l3c_0/read_hit/,\ > > +uncore_l3c_0/inv_request/,\ > > +uncore_l3c_0/inv_hit/ sleep 1 > > + > > +The driver does not support sampling, therefore "perf record" will > > +not work. Per-task (without "-a") perf sessions are not supported. > > What do you mean by "not supported"? If I invoke perf as: I mean --per-core option, needs rephrasing. > > # ./perf stat -e uncore_dmc_0/cnt_cycles/ -- ls > > then I get results back. > > > + > > +L3C events: > > + > > + > > +read_request: > > + Number of Read requests received by the L3 Cache. > > + This include Read as well as Read Exclusives. > > + > > +read_hit: > > + Number of Read requests received by the L3 cache that were hit > > + in the L3 (Data provided form the L3) > > + > > +writeback_request: > > + Number of Write Backs received by the L3 Cache. These are basically > > + the L2 Evicts and writes from the PCIe Write Cache. > > + > > +inv_nwrite_request: > > + This is the Number of Invalidate and Write
Re: [PATCH v8 2/2] ThunderX2, perf : Add Cavium ThunderX2 SoC UNCORE PMU driver
On Mon, Dec 3, 2018 at 5:41 PM Will Deacon wrote:
>
> On Thu, Nov 22, 2018 at 03:04:35AM +, Kulkarni, Ganapatrao wrote:
> > This patch adds a perf driver for the PMU UNCORE devices DDR4 Memory
> > Controller(DMC) and Level 3 Cache(L3C). Each PMU supports up to 4
> > counters. All counters lack overflow interrupt and are
> > sampled periodically.
> >
> > Signed-off-by: Ganapatrao Kulkarni
> > ---
> > drivers/perf/Kconfig | 9 +
> > drivers/perf/Makefile| 1 +
> > drivers/perf/thunderx2_pmu.c | 869 +++
> > include/linux/cpuhotplug.h | 1 +
> > 4 files changed, 880 insertions(+)
> > create mode 100644 drivers/perf/thunderx2_pmu.c
> >
> > diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> > index 08ebaf7cca8b..af9bc178495d 100644
> > --- a/drivers/perf/Kconfig
> > +++ b/drivers/perf/Kconfig
> > @@ -87,6 +87,15 @@ config QCOM_L3_PMU
> > Adds the L3 cache PMU into the perf events subsystem for
> > monitoring L3 cache events.
> >
> > +config THUNDERX2_PMU
> > + tristate "Cavium ThunderX2 SoC PMU UNCORE"
> > + depends on ARCH_THUNDER2 && ARM64 && ACPI && NUMA
> > + default m
> > + help
> > +Provides support for ThunderX2 UNCORE events.
> > +The SoC has PMU support in its L3 cache controller (L3C) and
> > +in the DDR4 Memory Controller (DMC).
> > +
> > config XGENE_PMU
> > depends on ARCH_XGENE
> > bool "APM X-Gene SoC PMU"
> > diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
> > index b3902bd37d53..909f27fd9db3 100644
> > --- a/drivers/perf/Makefile
> > +++ b/drivers/perf/Makefile
> > @@ -7,5 +7,6 @@ obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
> > obj-$(CONFIG_HISI_PMU) += hisilicon/
> > obj-$(CONFIG_QCOM_L2_PMU)+= qcom_l2_pmu.o
> > obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
> > +obj-$(CONFIG_THUNDERX2_PMU) += thunderx2_pmu.o
> > obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
> > obj-$(CONFIG_ARM_SPE_PMU) += arm_spe_pmu.o
> > diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
> > new file mode 100644
> > index ..e6509ba868ab
> > --- /dev/null
> > +++ b/drivers/perf/thunderx2_pmu.c
> > @@ -0,0 +1,869 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * CAVIUM THUNDERX2 SoC PMU UNCORE
> > + * Copyright (C) 2018 Cavium Inc.
> > + * Author: Ganapatrao Kulkarni
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +/* Each ThunderX2(TX2) Socket has a L3C and DMC UNCORE PMU device.
> > + * Each UNCORE PMU device consists of 4 independent programmable counters.
> > + * Counters are 32 bit and do not support overflow interrupt,
> > + * they need to be sampled before overflow(i.e, at every 2 seconds).
> > + */
> > +
> > +#define TX2_PMU_MAX_COUNTERS 4
> > +#define TX2_PMU_DMC_CHANNELS 8
> > +#define TX2_PMU_L3_TILES 16
> > +
> > +#define TX2_PMU_HRTIMER_INTERVAL (2 * NSEC_PER_SEC)
> > +#define GET_EVENTID(ev) ((ev->hw.config) & 0x1ff)
>
> I think this should be 0x1f.
yes it should be, i will update it.
>
> > +#define GET_COUNTERID(ev)((ev->hw.idx) & 0x3)
> > + /* 1 byte per counter(4 counters).
> > + * Event id is encoded in bits [5:1] of a byte,
> > + */
> > +#define DMC_EVENT_CFG(idx, val) ((val) << (((idx) * 8) + 1))
> > +
> > +#define L3C_COUNTER_CTL 0xA8
> > +#define L3C_COUNTER_DATA 0xAC
> > +#define DMC_COUNTER_CTL 0x234
> > +#define DMC_COUNTER_DATA 0x240
> > +
> > +/* L3C event IDs */
> > +#define L3_EVENT_READ_REQ0xD
> > +#define L3_EVENT_WRITEBACK_REQ 0xE
> > +#define L3_EVENT_INV_N_WRITE_REQ 0xF
> > +#define L3_EVENT_INV_REQ 0x10
> > +#define L3_EVENT_EVICT_REQ 0x13
> > +#define L3_EVENT_INV_N_WRITE_HIT 0x14
> > +#define L3_EVENT_INV_HIT 0x15
> > +#define L3_EVENT_READ_HIT0x17
> > +#define L3_EVENT_MAX 0x18
> > +
> > +/* DMC event IDs */
> > +#define DMC_EVENT_COUNT_CYCLES 0x1
> > +#define DMC_EVENT_WRITE_TXNS 0xB
> > +#define DMC_EVENT_DATA_TRANSFERS 0xD
> > +#define DMC_EVENT_READ_TXNS 0xF
> > +#define DMC_EVENT_MAX0x10
> > +
> > +enum tx2_uncore_type {
> > + PMU_TYPE_L3C,
> > + PMU_TYPE_DMC,
> > + PMU_TYPE_INVALID,
> > +};
> > +
> > +/*
> > + * pmu on each socket has 2 uncore devices(dmc and l3c),
> > + * each device has 4 counters.
> > + */
> > +struct tx2_uncore_pmu {
> > + struct hlist_node hpnode;
> > + struct list_head entry;
> > + struct pmu pmu;
> > + char *name;
> > + int node;
> > + int cpu;
> > + u32max_counters;
> > + u32prorate_factor;
> > + u32max_events;
> > + u64 hrtimer_interval;
> > + void __iomem *base;
> > + DECLARE_BITMAP(active_counters, TX2_PMU_MAX_COUNTERS);
> > + struct perf_event *
Re: [PATCH v4 1/8] clk: clkdev/of_clk - add managed lookup and provider registrations
Hello Again Stephen,
I did already send v5 prior to your reply but I will create v6 today
based on this discussion.
On Mon, Dec 03, 2018 at 03:35:10PM -0800, Stephen Boyd wrote:
> Quoting Matti Vaittinen (2018-11-30 02:50:22)
> > Hello Stephen,
> >
> > Thanks a bunch for taking the time and reviewing this!
> >
> > On Fri, Nov 30, 2018 at 12:54:10AM -0800, Stephen Boyd wrote:
> > > Quoting Matti Vaittinen (2018-11-13 03:55:58)
> > > > -int devm_of_clk_add_hw_provider(struct device *dev,
> > > > +static int __devm_of_clk_add_hw_provider(struct device *dev,
> > > > struct clk_hw *(*get)(struct of_phandle_args
> > > > *clkspec,
> > > > void *data),
> > > > - void *data)
> > > > + struct device_node *of_node, void *data)
> > > > {
> > > > - struct device_node **ptr, *np;
> > > > + struct device_node **ptr;
> > > > int ret;
> > > >
> > > > ptr = devres_alloc(devm_of_clk_release_provider, sizeof(*ptr),
> > > > @@ -3906,10 +3906,9 @@ int devm_of_clk_add_hw_provider(struct device
> > > > *dev,
> > > > if (!ptr)
> > > > return -ENOMEM;
> > > >
> > > > - np = dev->of_node;
> > > > - ret = of_clk_add_hw_provider(np, get, data);
> > > > + *ptr = of_node;
> > > > + ret = of_clk_add_hw_provider(of_node, get, data);
> > > > if (!ret) {
> > > > - *ptr = np;
> > >
> > > Why is this moved outside of the if condition?
> > I completely removed the local variable np and just unconditionally set
> > the allocated devres to point at the node (if allocation succeeded). We
> > could of course only do this if the provider registration succeeded and
> > save one assignment - but I guess I intended to remove the curly braces
> > and thus decided to go for one liner after if. But apparently I didn't
> > remove the braces O_o. Well, I can put the assignment inside the
> > condition if you prefer that.
> >
> > > In fact, why isn't just
> > > the first line in this hunk deleted and passed to this function as
> > > struct device_node *np?
> >
> > I am sorry but I don't quite follow your suggestion here. Do you mean we
> > could just pass the struct device_node *np in devres_add()? I thought
> > the pointer passed to devress_add() should be allocated using
> > devres_alloc. Can you please elaborate what you mean?
>
> I'm just trying to reduce the diff in the patch.
Oh, right. I will see how renaming the argument to np would impact to
patch size. iActually, I never consider the patch size at all - I have
only been concentrating on how the resulting file looks like. It didn't
ever cross my mind that patch size matters. But I guess the size of
chanes is really meaningfull when the amount of changes is large.
> > > > devres_add(dev, ptr);
> > > > } else {
> > > > devres_free(ptr);
> [..]
> > >
> > > > +int devm_of_clk_add_hw_provider(struct device *dev,
> > > > + struct clk_hw *(*get)(struct of_phandle_args
> > > > *clkspec,
> > > > + void *data),
> > > > + void *data)
> > > > +{
> > > > + return __devm_of_clk_add_hw_provider(dev, get, dev->of_node,
> > > > data);
> > > > +}
> > > > EXPORT_SYMBOL_GPL(devm_of_clk_add_hw_provider);
> > > >
> > > > +int devm_of_clk_add_parent_hw_provider(struct device *dev,
> > > > + struct clk_hw *(*get)(struct of_phandle_args
> > > > *clkspec,
> > > > + void *data),
> > > > + void *data)
> > > > +{
> > > > + return __devm_of_clk_add_hw_provider(dev, get,
> > > > dev->parent->of_node,
> > >
> > > I'm wondering if we can somehow auto-detect this in
> > > devm_of_clk_add_hw_provider() by looking for #clock-cells in the node.
> > > If it isn't there, then we go to the parent node and look for a
> > > #clock-cells property there in the DT node for that device. Does that
> > > make sense? Then there isn't any new API and we can attach the lifetime
> > > of the devm registration to the presence of the property indicating this
> > > is a clk controller or not.
> >
> > Huh. I don't know why but building this kind of logic in core is a bit
> > scary to me. I guess I can try implementing something like this - but I
> > am not really a fan of this. (Accidentally) omit the #clock-cells from
> > node and we go to parent node - I am a novice on this area but this
> > sounds like a potential hazard to me. I believe the driver should know
> > if it's properties should be in own or parent node - and if they are
> > not, then there should be no guessing but error. The lifetime is topic
> > where I would like to get information from you who know the kernel
> > better than I do =) But I guess the parent node is there at least as
> > long as the child device is alive. So for
Brauchen Sie einen Kredit?
-- Brauchen Sie einen Kredit?
RE,
-- Hello, First of all i will like to apologies for my manner of communication because you do not know me personally, its due to the fact that i have a very important proposal for you.
