| Message ID | 20171019171553.24056-4-suzuki.poulose@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 19, 2017 at 06:15:39PM +0100, Suzuki K Poulose wrote: > Right now we open code filling the trace buffer with synchronization > packets when the circular buffer wraps around in different drivers. > Move this to a common place. > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Cc: Mike Leach <mike.leach@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > drivers/hwtracing/coresight/coresight-etb10.c | 10 +++------ > drivers/hwtracing/coresight/coresight-priv.h | 8 ++++++++ > drivers/hwtracing/coresight/coresight-tmc-etf.c | 27 ++++++++----------------- > drivers/hwtracing/coresight/coresight-tmc-etr.c | 13 +----------- > 4 files changed, 20 insertions(+), 38 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c > index 56ecd7aff5eb..d7164ab8e229 100644 > --- a/drivers/hwtracing/coresight/coresight-etb10.c > +++ b/drivers/hwtracing/coresight/coresight-etb10.c > @@ -203,7 +203,6 @@ static void etb_dump_hw(struct etb_drvdata *drvdata) > bool lost = false; > int i; > u8 *buf_ptr; > - const u32 *barrier; > u32 read_data, depth; > u32 read_ptr, write_ptr; > u32 frame_off, frame_endoff; > @@ -234,19 +233,16 @@ static void etb_dump_hw(struct etb_drvdata *drvdata) > > depth = drvdata->buffer_depth; > buf_ptr = drvdata->buf; > - barrier = barrier_pkt; > for (i = 0; i < depth; i++) { > read_data = readl_relaxed(drvdata->base + > ETB_RAM_READ_DATA_REG); > - if (lost && *barrier) { > - read_data = *barrier; > - barrier++; > - } > - > *(u32 *)buf_ptr = read_data; > buf_ptr += 4; > } > > + if (lost) > + coresight_insert_barrier_packet(drvdata->buf); > + > if (frame_off) { > buf_ptr -= (frame_endoff * 4); > for (i = 0; i < frame_endoff; i++) { > diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h > index f1d0e21d8cab..d12f64928c00 100644 > --- a/drivers/hwtracing/coresight/coresight-priv.h > +++ b/drivers/hwtracing/coresight/coresight-priv.h > @@ -65,6 +65,7 @@ static DEVICE_ATTR_RO(name) > __coresight_simple_func(type, NULL, name, lo_off, hi_off) > > extern const u32 barrier_pkt[5]; > +#define CORESIGHT_BARRIER_PKT_SIZE (sizeof(barrier_pkt) - sizeof(u32)) When using a memcpy() there is no need to have a 0x0 at the end of the barrier_pkt array. A such I suggest you remove that and simply use sizeof() in coresight_insert_barrier_packet(). I'll review the rest of your patches tomorrow. > > enum etm_addr_type { > ETM_ADDR_TYPE_NONE, > @@ -98,6 +99,13 @@ struct cs_buffers { > void **data_pages; > }; > > +static inline void coresight_insert_barrier_packet(void *buf) > +{ > + if (buf) > + memcpy(buf, barrier_pkt, CORESIGHT_BARRIER_PKT_SIZE); > +} > + > + > static inline void CS_LOCK(void __iomem *addr) > { > do { > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c > index 0b6f1eb746de..d89bfb3042a2 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c > @@ -43,39 +43,28 @@ static void tmc_etb_enable_hw(struct tmc_drvdata *drvdata) > > static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata) > { > - bool lost = false; > char *bufp; > - const u32 *barrier; > - u32 read_data, status; > + u32 read_data, lost; > int i; > > - /* > - * Get a hold of the status register and see if a wrap around > - * has occurred. > - */ > - status = readl_relaxed(drvdata->base + TMC_STS); > - if (status & TMC_STS_FULL) > - lost = true; > - > + /* Check if the buffer was wrapped around. */ > + lost = readl_relaxed(drvdata->base + TMC_STS) & TMC_STS_FULL; > bufp = drvdata->buf; > drvdata->len = 0; > - barrier = barrier_pkt; > while (1) { > for (i = 0; i < drvdata->memwidth; i++) { > read_data = readl_relaxed(drvdata->base + TMC_RRD); > if (read_data == 0xFFFFFFFF) > - return; > - > - if (lost && *barrier) { > - read_data = *barrier; > - barrier++; > - } > - > + goto done; > memcpy(bufp, &read_data, 4); > bufp += 4; > drvdata->len += 4; > } > } > +done: > + if (lost) > + coresight_insert_barrier_packet(drvdata->buf); > + return; > } > > static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata) > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index 063f253f1c99..41535fa6b6cf 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -104,9 +104,7 @@ ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, > > static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata) > { > - const u32 *barrier; > u32 val; > - u32 *temp; > u64 rwp; > > rwp = tmc_read_rwp(drvdata); > @@ -119,16 +117,7 @@ static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata) > if (val & TMC_STS_FULL) { > drvdata->buf = drvdata->vaddr + rwp - drvdata->paddr; > drvdata->len = drvdata->size; > - > - barrier = barrier_pkt; > - temp = (u32 *)drvdata->buf; > - > - while (*barrier) { > - *temp = *barrier; > - temp++; > - barrier++; > - } > - > + coresight_insert_barrier_packet(drvdata->buf); > } else { > drvdata->buf = drvdata->vaddr; > drvdata->len = rwp - drvdata->paddr; > -- > 2.13.6 >
On 30/10/17 21:44, Mathieu Poirier wrote: > On Thu, Oct 19, 2017 at 06:15:39PM +0100, Suzuki K Poulose wrote: >> Right now we open code filling the trace buffer with synchronization >> packets when the circular buffer wraps around in different drivers. >> Move this to a common place. >> >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> Cc: Mike Leach <mike.leach@linaro.org> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >> --- >> drivers/hwtracing/coresight/coresight-etb10.c | 10 +++------ >> drivers/hwtracing/coresight/coresight-priv.h | 8 ++++++++ >> drivers/hwtracing/coresight/coresight-tmc-etf.c | 27 ++++++++----------------- >> drivers/hwtracing/coresight/coresight-tmc-etr.c | 13 +----------- >> 4 files changed, 20 insertions(+), 38 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c >> index 56ecd7aff5eb..d7164ab8e229 100644 >> --- a/drivers/hwtracing/coresight/coresight-etb10.c >> +++ b/drivers/hwtracing/coresight/coresight-etb10.c >> @@ -203,7 +203,6 @@ static void etb_dump_hw(struct etb_drvdata *drvdata) >> bool lost = false; >> int i; >> u8 *buf_ptr; >> - const u32 *barrier; >> u32 read_data, depth; >> u32 read_ptr, write_ptr; >> u32 frame_off, frame_endoff; >> @@ -234,19 +233,16 @@ static void etb_dump_hw(struct etb_drvdata *drvdata) >> >> depth = drvdata->buffer_depth; >> buf_ptr = drvdata->buf; >> - barrier = barrier_pkt; >> for (i = 0; i < depth; i++) { >> read_data = readl_relaxed(drvdata->base + >> ETB_RAM_READ_DATA_REG); >> - if (lost && *barrier) { >> - read_data = *barrier; >> - barrier++; >> - } >> - >> *(u32 *)buf_ptr = read_data; >> buf_ptr += 4; >> } >> >> + if (lost) >> + coresight_insert_barrier_packet(drvdata->buf); >> + >> if (frame_off) { >> buf_ptr -= (frame_endoff * 4); >> for (i = 0; i < frame_endoff; i++) { >> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h >> index f1d0e21d8cab..d12f64928c00 100644 >> --- a/drivers/hwtracing/coresight/coresight-priv.h >> +++ b/drivers/hwtracing/coresight/coresight-priv.h >> @@ -65,6 +65,7 @@ static DEVICE_ATTR_RO(name) >> __coresight_simple_func(type, NULL, name, lo_off, hi_off) >> >> extern const u32 barrier_pkt[5]; >> +#define CORESIGHT_BARRIER_PKT_SIZE (sizeof(barrier_pkt) - sizeof(u32)) > > When using a memcpy() there is no need to have a 0x0 at the end of the > barrier_pkt array. A such I suggest you remove that and simply use sizeof() > in coresight_insert_barrier_packet(). There is one place where can't simply do a memcpy(), in tmc_update_etf_buffer(), where we could potentially move over to the next PAGE while filling the barrier packets. This is why I didn't trim it off. However, I believe this shouldn't trigger as the trace data should always be aligned to the frame size of the TMC and the perf buffer size is page aligned. So, we should be able use memcpy() in that case too. I will fix it in the next version. Thanks Suzuki
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 56ecd7aff5eb..d7164ab8e229 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -203,7 +203,6 @@ static void etb_dump_hw(struct etb_drvdata *drvdata) bool lost = false; int i; u8 *buf_ptr; - const u32 *barrier; u32 read_data, depth; u32 read_ptr, write_ptr; u32 frame_off, frame_endoff; @@ -234,19 +233,16 @@ static void etb_dump_hw(struct etb_drvdata *drvdata) depth = drvdata->buffer_depth; buf_ptr = drvdata->buf; - barrier = barrier_pkt; for (i = 0; i < depth; i++) { read_data = readl_relaxed(drvdata->base + ETB_RAM_READ_DATA_REG); - if (lost && *barrier) { - read_data = *barrier; - barrier++; - } - *(u32 *)buf_ptr = read_data; buf_ptr += 4; } + if (lost) + coresight_insert_barrier_packet(drvdata->buf); + if (frame_off) { buf_ptr -= (frame_endoff * 4); for (i = 0; i < frame_endoff; i++) { diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index f1d0e21d8cab..d12f64928c00 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -65,6 +65,7 @@ static DEVICE_ATTR_RO(name) __coresight_simple_func(type, NULL, name, lo_off, hi_off) extern const u32 barrier_pkt[5]; +#define CORESIGHT_BARRIER_PKT_SIZE (sizeof(barrier_pkt) - sizeof(u32)) enum etm_addr_type { ETM_ADDR_TYPE_NONE, @@ -98,6 +99,13 @@ struct cs_buffers { void **data_pages; }; +static inline void coresight_insert_barrier_packet(void *buf) +{ + if (buf) + memcpy(buf, barrier_pkt, CORESIGHT_BARRIER_PKT_SIZE); +} + + static inline void CS_LOCK(void __iomem *addr) { do { diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 0b6f1eb746de..d89bfb3042a2 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -43,39 +43,28 @@ static void tmc_etb_enable_hw(struct tmc_drvdata *drvdata) static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata) { - bool lost = false; char *bufp; - const u32 *barrier; - u32 read_data, status; + u32 read_data, lost; int i; - /* - * Get a hold of the status register and see if a wrap around - * has occurred. - */ - status = readl_relaxed(drvdata->base + TMC_STS); - if (status & TMC_STS_FULL) - lost = true; - + /* Check if the buffer was wrapped around. */ + lost = readl_relaxed(drvdata->base + TMC_STS) & TMC_STS_FULL; bufp = drvdata->buf; drvdata->len = 0; - barrier = barrier_pkt; while (1) { for (i = 0; i < drvdata->memwidth; i++) { read_data = readl_relaxed(drvdata->base + TMC_RRD); if (read_data == 0xFFFFFFFF) - return; - - if (lost && *barrier) { - read_data = *barrier; - barrier++; - } - + goto done; memcpy(bufp, &read_data, 4); bufp += 4; drvdata->len += 4; } } +done: + if (lost) + coresight_insert_barrier_packet(drvdata->buf); + return; } static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata) diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 063f253f1c99..41535fa6b6cf 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -104,9 +104,7 @@ ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata) { - const u32 *barrier; u32 val; - u32 *temp; u64 rwp; rwp = tmc_read_rwp(drvdata); @@ -119,16 +117,7 @@ static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata) if (val & TMC_STS_FULL) { drvdata->buf = drvdata->vaddr + rwp - drvdata->paddr; drvdata->len = drvdata->size; - - barrier = barrier_pkt; - temp = (u32 *)drvdata->buf; - - while (*barrier) { - *temp = *barrier; - temp++; - barrier++; - } - + coresight_insert_barrier_packet(drvdata->buf); } else { drvdata->buf = drvdata->vaddr; drvdata->len = rwp - drvdata->paddr;
Right now we open code filling the trace buffer with synchronization packets when the circular buffer wraps around in different drivers. Move this to a common place. Cc: Mathieu Poirier <mathieu.poirier@linaro.org> Cc: Mike Leach <mike.leach@linaro.org> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> --- drivers/hwtracing/coresight/coresight-etb10.c | 10 +++------ drivers/hwtracing/coresight/coresight-priv.h | 8 ++++++++ drivers/hwtracing/coresight/coresight-tmc-etf.c | 27 ++++++++----------------- drivers/hwtracing/coresight/coresight-tmc-etr.c | 13 +----------- 4 files changed, 20 insertions(+), 38 deletions(-)