RE: [PATCH v9 00/13] arch/resctrl: AMD QoS support

2018-12-26 Thread Moger, Babu
Hi Jan,

> -Original Message-
> From: Jan Engelhardt 
> Sent: Sunday, December 23, 2018 12:27 PM
> To: Moger, Babu 
> Cc: t...@linutronix.de; mi...@redhat.com; b...@alien8.de; cor...@lwn.net;
> fenghua...@intel.com; reinette.cha...@intel.com; pet...@infradead.org;
> gre...@linuxfoundation.org; da...@davemloft.net; akpm@linux-
> foundation.org; h...@zytor.com; x...@kernel.org;
> mchehab+sams...@kernel.org; a...@arndb.de;
> kstew...@linuxfoundation.org; pombreda...@nexb.com;
> raf...@kernel.org; kirill.shute...@linux.intel.com; tony.l...@intel.com;
> qianyue...@alibaba-inc.com; xiaochen.s...@intel.com;
> pbonz...@redhat.com; Singh, Brijesh ; Hurwitz,
> Sherry ; dw...@infradead.org; Lendacky,
> Thomas ; l...@kernel.org; j...@8bytes.org;
> ja...@google.com; vkuzn...@redhat.com; r...@alum.mit.edu;
> jpoim...@redhat.com; linux-ker...@vger.kernel.org; linux-
> d...@vger.kernel.org
> Subject: Re: [PATCH v9 00/13] arch/resctrl: AMD QoS support
> 
> 
> On Nov 21 2018 20:28:23, Moger, Babu wrote:
> >
> >This series adds support for AMD64 architectural extensions for
> >Platform Quality of Service.
> 
> The term "QoS" is already used for net/sched/. It will be bad naming to
> have QoS - and then an AMD QoS / Platform QoS as well. Preexisting,
> decade-old howtos may speak of enabling "QoS", and with the ambiguity,
> the unsuspecting kernel builder apprentice may enable the wrong thing in
> his config.
> 
> So either the QoS from networking gets correspondingly renamed ("packet
> QoS"?.. or something), or else AMD QoS should be changed.

The QoS name is not visible to the user. This feature is referred as QoS
in AMD documents. But for the kernel user it is called as RESCTRL which we
agreed upon in our earlier discussions. So there is no ambiguity  or
confusion here. Let me know if you have any questions.




Re: [PATCH v3 2/9] dt/bindings: drm/komeda: Add DT bindings for ARM display processor D71

2018-12-26 Thread james qian wang (Arm Technology China)
On Mon, Dec 24, 2018 at 08:00:51PM +0800, Liviu Dudau wrote:
> On Fri, Dec 21, 2018 at 09:59:12AM +, james qian wang (Arm Technology 
> China) wrote:
> > Add DT bindings documentation for the ARM display processor D71 and later
> > IPs.
> > 
> > Signed-off-by: James (Qian) Wang 
> > 
> > Changes in v3:
> > - Deleted unnecessary property: interrupt-names.
> > - Dropped 'ports' and moving 'port' up a level.
> > ---
> >  .../bindings/display/arm/arm,komeda.txt   | 79 +++
> >  1 file changed, 79 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/display/arm/arm,komeda.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/display/arm/arm,komeda.txt 
> > b/Documentation/devicetree/bindings/display/arm/arm,komeda.txt
> > new file mode 100644
> > index ..b4e450243c7d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/arm/arm,komeda.txt
> > @@ -0,0 +1,79 @@
> > +Device Tree bindings for ARM Komeda display driver
> > +
> > +Required properties:
> > +- compatible: Should be "arm,mali-d71"
> > +- reg: Physical base address and length of the registers in the system
> > +- interrupts: the interrupt line number of the device in the system
> > +- clocks: A list of phandle + clock-specifier pairs, one for each entry
> > +in 'clock-names'
> > +- clock-names: A list of clock names. It should contain:
> > +  - "mclk": for the main processor clock
> > +  - "pclk": for the APB interface clock
> > +- #address-cells: Must be 1
> > +- #size-cells: Must be 0
> > +
> > +Required properties for sub-node: pipeline@nq
> > +Each device contains one or two pipeline sub-nodes (at least one), each
> > +pipeline node should provide properties:
> > +- reg: Zero-indexed identifier for the pipeline
> > +- clocks: A list of phandle + clock-specifier pairs, one for each entry
> > +in 'clock-names'
> > +- clock-names: should contain:
> > +  - "pxclk": pixel clock
> > +  - "aclk": AXI interface clock
> > +
> > +- port: each pipeline connect to an encoder input port. The connection is
> > +modeled using the OF graph bindings specified in
> > +Documentation/devicetree/bindings/graph.txt
> > +
> > +Optional properties:
> > +  - memory-region: phandle to a node describing memory (see
> > +Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt)
> > +to be used for the framebuffer; if not present, the framebuffer may
> > +be located anywhere in memory.
> > +
> > +Example:
> > +/ {
> > +   ...
> > +
> > +   dp0: display@c0 {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   compatible = "arm,mali-d71";
> > +   reg = <0xc0 0x2>;
> > +   interrupts = <0 168 4>;
> > +   clocks = <&dpu_mclk>, <&dpu_aclk>;
> > +   clock-names = "mclk", "pclk";
> > +
> > +   dp0_pipe0: pipeline@0 {
> > +   clocks = <&fpgaosc2>, <&dpu_aclk>;
> > +   clock-names = "pxclk", "aclk";
> > +   reg = <0>;
> > +
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> 
> These are undocumented and not necessary anyway, as the pipelines will
> inherit display's attributes.

thank you, will fix it in the next version.

> > +
> > +   port@0 {
> > +   dp0_pipe0_out: endpoint {
> > +   remote-endpoint = <&db_dvi0_in>;
> > +   };
> > +   };
> > +   };
> > +
> > +   dp0_pipe1: pipeline@1 {
> > +   clocks = <&fpgaosc2>, <&dpu_aclk>;
> > +   clock-names = "pxclk", "aclk";
> > +   reg = <1>;
> > +
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> 
> same here.

OK.

> 
> > +
> > +   port@0 {
> > +   dp0_pipe1_out: endpoint {
> > +   remote-endpoint = <&db_dvi1_in>;
> > +   };
> > +   };
> > +   };
> > +   };
> > +   ...
> > +};
> > -- 
> > 2.17.1
> > 
> 
> With these changes:
> 
> Reviewed-by: Liviu Dudau 
> 
> Best regards,
> Liviu
> 
> 
> -- 
> 
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---
> ¯\_(ツ)_/¯


Re: [PATCH v3 1/9] drm/komeda: komeda_dev/pipeline/component definition and initialzation

2018-12-26 Thread james qian wang (Arm Technology China)
On Mon, Dec 24, 2018 at 07:57:41PM +0800, Liviu Dudau wrote:
> On Fri, Dec 21, 2018 at 09:58:55AM +, james qian wang (Arm Technology 
> China) wrote:
> > 1. Added a brief definition of komeda_dev/pipeline/component, this change
> >didn't add the detailed component features and capabilities, which will
> >be added in the following changes.
> > 2. Corresponding resources discovery and initialzation functions.
> > 
> > Signed-off-by: James (Qian) Wang 
> > 
> > Changes in v3:
> > - Fixed style problem found by checkpatch.pl --strict.
> > 
> > Changes in v2:
> > - Unified abbreviation of "pipeline" to "pipe".
> > ---
> >  drivers/gpu/drm/arm/Kconfig   |   2 +
> >  drivers/gpu/drm/arm/Makefile  |   1 +
> >  drivers/gpu/drm/arm/display/Kbuild|   3 +
> >  drivers/gpu/drm/arm/display/Kconfig   |  14 +
> >  .../drm/arm/display/include/malidp_product.h  |  23 ++
> >  .../drm/arm/display/include/malidp_utils.h|  16 +
> >  drivers/gpu/drm/arm/display/komeda/Makefile   |  11 +
> >  .../gpu/drm/arm/display/komeda/komeda_dev.c   | 117 ++
> >  .../gpu/drm/arm/display/komeda/komeda_dev.h   |  98 +
> >  .../drm/arm/display/komeda/komeda_pipeline.c  | 198 ++
> >  .../drm/arm/display/komeda/komeda_pipeline.h  | 350 ++
> >  11 files changed, 833 insertions(+)
> >  create mode 100644 drivers/gpu/drm/arm/display/Kbuild
> >  create mode 100644 drivers/gpu/drm/arm/display/Kconfig
> >  create mode 100644 drivers/gpu/drm/arm/display/include/malidp_product.h
> >  create mode 100644 drivers/gpu/drm/arm/display/include/malidp_utils.h
> >  create mode 100644 drivers/gpu/drm/arm/display/komeda/Makefile
> >  create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> >  create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> >  create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
> >  create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > 
> > diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
> > index f9f7761cb2f4..a204103b3efb 100644
> > --- a/drivers/gpu/drm/arm/Kconfig
> > +++ b/drivers/gpu/drm/arm/Kconfig
> > @@ -37,4 +37,6 @@ config DRM_MALI_DISPLAY
> >  
> >   If compiled as a module it will be called mali-dp.
> >  
> > +source "drivers/gpu/drm/arm/display/Kconfig"
> > +
> >  endmenu
> > diff --git a/drivers/gpu/drm/arm/Makefile b/drivers/gpu/drm/arm/Makefile
> > index 3bf31d1a4722..120bef801fcf 100644
> > --- a/drivers/gpu/drm/arm/Makefile
> > +++ b/drivers/gpu/drm/arm/Makefile
> > @@ -3,3 +3,4 @@ obj-$(CONFIG_DRM_HDLCD) += hdlcd.o
> >  mali-dp-y := malidp_drv.o malidp_hw.o malidp_planes.o malidp_crtc.o
> >  mali-dp-y += malidp_mw.o
> >  obj-$(CONFIG_DRM_MALI_DISPLAY) += mali-dp.o
> > +obj-$(CONFIG_DRM_KOMEDA) += display/
> > diff --git a/drivers/gpu/drm/arm/display/Kbuild 
> > b/drivers/gpu/drm/arm/display/Kbuild
> > new file mode 100644
> > index ..382f1ca831e4
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/display/Kbuild
> > @@ -0,0 +1,3 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +obj-$(CONFIG_DRM_KOMEDA) += komeda/
> > diff --git a/drivers/gpu/drm/arm/display/Kconfig 
> > b/drivers/gpu/drm/arm/display/Kconfig
> > new file mode 100644
> > index ..cec0639e3aa1
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/display/Kconfig
> > @@ -0,0 +1,14 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +config DRM_KOMEDA
> > +   tristate "ARM Komeda display driver"
> > +   depends on DRM && OF
> > +   depends on COMMON_CLK
> > +   select DRM_KMS_HELPER
> > +   select DRM_KMS_CMA_HELPER
> > +   select DRM_GEM_CMA_HELPER
> > +   select VIDEOMODE_HELPERS
> > +   help
> > + Choose this option if you want to compile the ARM Komeda display
> > + Processor driver. It supports the D71 variants of the hardware.
> > +
> > + If compiled as a module it will be called komeda.
> > diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h 
> > b/drivers/gpu/drm/arm/display/include/malidp_product.h
> > new file mode 100644
> > index ..b35fc5db866b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/display/include/malidp_product.h
> > @@ -0,0 +1,23 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * (C) COPYRIGHT 2018 ARM Limited. All rights reserved.
> > + * Author: James.Qian.Wang 
> > + *
> > + */
> > +#ifndef _MALIDP_PRODUCT_H_
> > +#define _MALIDP_PRODUCT_H_
> > +
> > +/* Product identification */
> > +#define MALIDP_CORE_ID(__product, __major, __minor, __status) \
> > +   __product) & 0x) << 16) | (((__major) & 0xF) << 12) | \
> > +   (((__minor) & 0xF) << 8) | ((__status) & 0xFF))
> > +
> > +#define MALIDP_CORE_ID_PRODUCT_ID(__core_id) ((__u32)(__core_id) >> 16)
> > +#define MALIDP_CORE_ID_MAJOR(__core_id)  (((__u32)(__core_id) >> 12) & 
> > 0xF)
> > +#define MALIDP_CORE_ID_MINOR(__core_id)  (((__u32)(__core_id) >> 8) & 
> > 0xF)
> > +#define MALIDP_CORE_ID_STATUS(__core_id

Re: [PATCH v3 7/9] drm/komeda: Attach komeda_dev to DRM-KMS

2018-12-26 Thread james qian wang (Arm Technology China)
On Mon, Dec 24, 2018 at 08:32:14PM +0800, Liviu Dudau wrote:
> On Fri, Dec 21, 2018 at 10:00:33AM +, james qian wang (Arm Technology 
> China) wrote:
> > Add komeda_kms abstracton to attach komeda_dev to DRM-KMS
> >   CRTC: according to the komeda_pipeline
> >   PLANE: according to komeda_layer (layer input pipeline)
> >   PRIVATE_OBJS: komeda_pipeline/component all will be treat as private_objs
> > 
> > komeda_kms is for connecting DRM-KMS and komeda_dev, like reporting the
> > kms object properties according to the komeda_dev, and pass/convert KMS's
> > requirement to komeda_dev.
> > 
> > Changes in v3:
> > - Fixed style problem found by checkpatch.pl --strict.
> > 
> > Changes in v2:
> > - Unified abbreviation of "pipeline" to "pipe".
> > 
> > Signed-off-by: James (Qian) Wang 
> > ---
> >  drivers/gpu/drm/arm/display/komeda/Makefile   |   6 +-
> >  .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 106 +++
> >  .../gpu/drm/arm/display/komeda/komeda_drv.c   |  19 +-
> >  .../gpu/drm/arm/display/komeda/komeda_kms.c   | 169 ++
> >  .../gpu/drm/arm/display/komeda/komeda_kms.h   | 113 
> >  .../drm/arm/display/komeda/komeda_pipeline.h  |   3 +
> >  .../gpu/drm/arm/display/komeda/komeda_plane.c | 109 +++
> >  .../arm/display/komeda/komeda_private_obj.c   |  88 +
> >  8 files changed, 608 insertions(+), 5 deletions(-)
> >  create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> >  create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> >  create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> >  create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> >  create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c
> > 
> > diff --git a/drivers/gpu/drm/arm/display/komeda/Makefile 
> > b/drivers/gpu/drm/arm/display/komeda/Makefile
> > index 25beae900ed2..1b875e5dc0f6 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/Makefile
> > +++ b/drivers/gpu/drm/arm/display/komeda/Makefile
> > @@ -9,7 +9,11 @@ komeda-y := \
> > komeda_dev.o \
> > komeda_format_caps.o \
> > komeda_pipeline.o \
> > -   komeda_framebuffer.o
> > +   komeda_framebuffer.o \
> > +   komeda_kms.o \
> > +   komeda_crtc.o \
> > +   komeda_plane.o \
> > +   komeda_private_obj.o
> >  
> >  komeda-y += \
> > d71/d71_dev.o
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c 
> > b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > new file mode 100644
> > index ..5bb5a55f6b31
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > @@ -0,0 +1,106 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * (C) COPYRIGHT 2018 ARM Limited. All rights reserved.
> > + * Author: James.Qian.Wang 
> > + *
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include "komeda_dev.h"
> > +#include "komeda_kms.h"
> > +
> > +struct drm_crtc_helper_funcs komeda_crtc_helper_funcs = {
> > +};
> > +
> > +static const struct drm_crtc_funcs komeda_crtc_funcs = {
> > +};
> > +
> > +int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
> > +  struct komeda_dev *mdev)
> > +{
> > +   struct komeda_crtc *crtc;
> > +   struct komeda_pipeline *master;
> > +   char str[16];
> > +   int i;
> > +
> > +   kms->n_crtcs = 0;
> > +
> > +   for (i = 0; i < mdev->n_pipelines; i++) {
> > +   crtc = &kms->crtcs[kms->n_crtcs];
> > +   master = mdev->pipelines[i];
> > +
> > +   crtc->master = master;
> > +   crtc->slave  = NULL;
> > +
> > +   if (crtc->slave)
> > +   sprintf(str, "pipe-%d", crtc->slave->id);
> > +   else
> > +   sprintf(str, "None");
> > +
> > +   DRM_INFO("crtc%d: master(pipe-%d) slave(%s) output: %s.\n",
> > +kms->n_crtcs, master->id, str,
> > +master->of_output_dev ?
> > +master->of_output_dev->full_name : "None");
> > +
> > +   kms->n_crtcs++;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +static struct drm_plane *
> > +get_crtc_primary(struct komeda_kms_dev *kms, struct komeda_crtc *crtc)
> > +{
> > +   struct komeda_plane *kplane;
> > +   struct drm_plane *plane;
> > +
> > +   drm_for_each_plane(plane, &kms->base) {
> > +   if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> > +   continue;
> > +
> > +   kplane = to_kplane(plane);
> > +   /* only master can be primary */
> > +   if (kplane->layer->base.pipeline == crtc->master)
> > +   return plane;
> > +   }
> > +
> > +   return NULL;
> > +}
> > +
> > +static int komeda_crtc_add(struct komeda_kms_dev *kms,
> > +  struct komeda_crtc *kcrtc)
> > +{
> > +   struct drm_crtc *crtc = &kcrtc->base;
> > +   int err;
> > +
> > +   err = drm_crtc_init_with_planes(&kms->base, crtc,
> > +   g