[Skiboot] [PATCH 4/5] NX: Add P9 NX support for gzip compression engine
Sukadev Bhattiprolu
sukadev at linux.vnet.ibm.com
Wed Feb 22 07:08:01 AEDT 2017
Haren Myneni [haren at linux.vnet.ibm.com] wrote:
>
> Power 9 introduces NX gzip compression engine. This patch adds gzip
> compression support for NX. Virtual Accelerator Switch (VAS) is used to
> access NX gzip engine and the channel setup be will done with receive
> FIFO. So RxFIFO address, logical partition ID (lpid), process ID (pid)
> and thread ID (tid) are used for this setup. P9 NX supports high and
> normal priority FIFOS. Skiboot configures User Mode Access Control
> (UMAC) registers with these values and also enables other registers to
> enable / disable the engine. Creates the following device-tree entries
> to provide RxFIFO address, lpid, pid and tid values so that kernel can
> drive P9 NX gzip engine.
>
> /ibm,nx-gzip-high : High priority gzip RxFIFO
> /ibm,nx-gzip-normal : Normal priority gzip RxFIFO
Are these under the xscom node? If so would be useful to mention that.
>
> Each RxFIFO node contains:
> rx-fifo-address : Address represents RxFIFO buffer
> lpid : Chip ID
> pid : gzip coprocessor type
> tid : Priority (either high or normal)
(same comment as in previous patch)
>
> Signed-off-by: Haren Myneni <haren at us.ibm.com>
> ---
> hw/Makefile.inc | 2 +-
> hw/nx-compress.c | 1 +
> hw/nx-gzip.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> include/nx.h | 1 +
> 4 files changed, 134 insertions(+), 1 deletions(-)
> create mode 100644 hw/nx-gzip.c
>
> diff --git a/hw/Makefile.inc b/hw/Makefile.inc
> index 97c909e..d4c4fcc 100644
> --- a/hw/Makefile.inc
> +++ b/hw/Makefile.inc
> @@ -2,7 +2,7 @@
> SUBDIRS += hw
> HW_OBJS = xscom.o chiptod.o gx.o cec.o lpc.o lpc-uart.o psi.o
> HW_OBJS += homer.o slw.o occ.o fsi-master.o centaur.o
> -HW_OBJS += nx.o nx-rng.o nx-crypto.o nx-compress.o nx-842.o
> +HW_OBJS += nx.o nx-rng.o nx-crypto.o nx-compress.o nx-842.o nx-gzip.o
> HW_OBJS += p7ioc.o p7ioc-inits.o p7ioc-phb.o
> HW_OBJS += phb3.o sfc-ctrl.o fake-rtc.o bt.o p8-i2c.o prd.o
> HW_OBJS += dts.o lpc-rtc.o npu.o npu-hw-procedures.o xive.o phb4.o
> diff --git a/hw/nx-compress.c b/hw/nx-compress.c
> index b4abe9e..235993c 100644
> --- a/hw/nx-compress.c
> +++ b/hw/nx-compress.c
> @@ -205,6 +205,7 @@ void nx_create_compress_node(struct dt_node *node)
> return;
>
> p9_nx_enable_842(node, gcid, pb_base);
> + p9_nx_enable_gzip(node, gcid, pb_base);
> } else
> nx_enable_842(node, gcid, pb_base);
> }
> diff --git a/hw/nx-gzip.c b/hw/nx-gzip.c
> new file mode 100644
> index 0000000..70cc387
> --- /dev/null
> +++ b/hw/nx-gzip.c
> @@ -0,0 +1,131 @@
> +/* Copyright 2016 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <skiboot.h>
> +#include <chip.h>
> +#include <xscom.h>
> +#include <io.h>
> +#include <cpu.h>
> +#include <nx.h>
> +
> +/* Configuration settings */
> +#define DMA_COMPRESS_PREFETCH (1) /* enable prefetching (on P9) */
> +#define DMA_DECOMPRESS_PREFETCH (1) /* enable prefetching (on P9) */
These values are used to set boolean fields right? Do we really need
these macros or can you simply use 'true' when setting them?
> +#define DMA_COMPRESS_MAX_RR (15) /* range 1-15 */
> +#define DMA_DECOMPRESS_MAX_RR (15) /* range 1-15 */
Can we move these macros to the header file where the other bit fields
of the register are defined? That way it will be easier to compare the
spec with the header.
> +#define DMA_SPBC (1) /* write SPBC in CPB */
> +#define EE (1) /* enable engine gzip */
Also a boolean values right? If so drop and just use 'true'?
> +
nit: line up the macro definitions if possible or introduce a blank line
in between.
> +static int nx_cfg_gzip_umac(struct dt_node *node, u32 gcid, u32 pb_base)
> +{
> + int rc;
> + u64 umac_bar, umac_ctrl, umac_notify;
> + struct dt_node *nx_node;
> +
> + nx_node = dt_new(node, "ibm,nx-gzip-high");
> + umac_bar = pb_base + NX_P9_GZIP_HIGH_PRI_RX_FIFO_BAR;
> + umac_ctrl = pb_base + NX_P9_GZIP_HIGH_PRI_RX_FIFO_CTRL;
> + umac_notify = pb_base + NX_P9_GZIP_HIGH_PRI_RX_FIFO_NOTIFY_MATCH;
nit: insert new line here for readability
> + rc = nx_set_rx_fifo(nx_node, gcid, umac_bar, umac_notify, umac_ctrl,
> + NX_CT_GZIP, RX_FIFO_HIGH_PRIORITY);
> + if (rc)
> + return rc;
> +
> + nx_node = dt_new(node, "ibm,nx-gzip-normal");
> + umac_bar = pb_base + NX_P9_GZIP_NORMAL_PRI_RX_FIFO_BAR;
> + umac_ctrl = pb_base + NX_P9_GZIP_NORMAL_PRI_RX_FIFO_CTRL;
> + umac_notify = pb_base + NX_P9_GZIP_NORMAL_PRI_RX_FIFO_NOTIFY_MATCH;
nit: insert new line here for readability
> + rc = nx_set_rx_fifo(nx_node, gcid, umac_bar, umac_notify, umac_ctrl,
> + NX_CT_GZIP, RX_FIFO_NORMAL_PRIORITY);
> +
> +
nit: extra new line.
> + return rc;
> +}
> +
> +static int nx_cfg_gzip_dma(u32 gcid, u64 xcfg)
> +{
> + u64 cfg;
> + int rc;
> +
> + rc = xscom_read(gcid, xcfg, &cfg);
> + if (rc)
> + return rc;
> +
> + cfg = SETFIELD(NX_P9_DMA_CFG_GZIP_COMPRESS_PREFETCH, cfg,
> + DMA_COMPRESS_PREFETCH);
> + cfg = SETFIELD(NX_P9_DMA_CFG_GZIP_DECOMPRESS_PREFETCH, cfg,
> + DMA_DECOMPRESS_PREFETCH);
Use 'true' instead of DMA_(DE)COMPRESS_PREFETCH ?
> +
> + cfg = SETFIELD(NX_DMA_CFG_GZIP_COMPRESS_MAX_RR, cfg,
> + DMA_COMPRESS_MAX_RR);
> + cfg = SETFIELD(NX_DMA_CFG_GZIP_DECOMPRESS_MAX_RR, cfg,
> + DMA_DECOMPRESS_MAX_RR);
> +
> + rc = xscom_write(gcid, xcfg, cfg);
> + if (rc)
> + prerror("NX%d: ERROR: DMA config failure %d\n", gcid, rc);
> + else
> + prlog(PR_DEBUG, "NX%d: DMA 0x%016lx\n", gcid,
> + (unsigned long)cfg);
> +
> + return rc;
> +}
> +
> +static int nx_cfg_gzip_ee(u32 gcid, u64 xcfg)
> +{
> + u64 cfg;
> + int rc;
> +
> + rc = xscom_read(gcid, xcfg, &cfg);
> + if (rc)
> + return rc;
> +
> + cfg = SETFIELD(NX_P9_EE_CFG_CH4, cfg, EE);
Use 'true' instead of EE?
> +
> + rc = xscom_write(gcid, xcfg, cfg);
> + if (rc)
> + prerror("NX%d: ERROR: Engine Enable failure %d\n", gcid, rc);
> + else
> + prlog(PR_DEBUG, "NX%d: Engine Enable 0x%016lx\n",
> + gcid, (unsigned long)cfg);
> +
> + return rc;
> +}
> +
> +void p9_nx_enable_gzip(struct dt_node *node, u32 gcid, u32 pb_base)
> +{
> + u64 cfg_dma, cfg_ee;
> + int rc;
> +
> + prlog(PR_INFO, "NX%d: gzip at 0x%x\n", gcid, pb_base);
> +
> + cfg_dma = pb_base + NX_P9_DMA_CFG;
> + cfg_ee = pb_base + NX_P9_EE_CFG;
> +
> + rc = nx_cfg_gzip_dma(gcid, cfg_dma);
> + if (rc)
> + return;
> +
> + rc = nx_cfg_gzip_ee(gcid, cfg_ee);
> + if (rc)
> + return;
> +
> + rc = nx_cfg_gzip_umac(node, gcid, pb_base);
> + if (rc)
> + return;
> +
> + prlog(PR_INFO, "NX%d: gzip Coprocessor Enabled\n", gcid);
> +}
> diff --git a/include/nx.h b/include/nx.h
> index 18fa91f..aedcc0e 100644
> --- a/include/nx.h
> +++ b/include/nx.h
> @@ -382,6 +382,7 @@ extern void nx_create_compress_node(struct dt_node *);
>
> extern void nx_enable_842(struct dt_node *, u32 gcid, u32 pb_base);
> extern void p9_nx_enable_842(struct dt_node *, u32 gcid, u32 pb_base);
> +extern void p9_nx_enable_gzip(struct dt_node *, u32 gcid, u32 pb_base);
>
> extern int nx_set_rx_fifo(struct dt_node *node, u32 gcid, u64 umac_bar,
> u64 umac_notify, u64 umac_ctrl, u32 ct_type,
> --
> 1.7.1
>
>
More information about the Skiboot
mailing list