[Skiboot] [PATCH 4/5] NX: Add P9 NX support for gzip compression engine
Haren Myneni
haren at linux.vnet.ibm.com
Wed Feb 22 09:49:10 AEDT 2017
On 02/21/2017 12:08 PM, Sukadev Bhattiprolu wrote:
>
> 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
Will add /xscom@<xscom_addr>/nx@<nx_addr>
.
>
>>
>> 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?
As mention before, following the same syntax as exist in nx-842 code.
>
>> +#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.
Sure we can move some of these macros.
>
>> +#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