On Fri, 3 Aug 2012 10:05:41 +0300
Jani Nikula <jani.nikula at linux.intel.com> wrote:
> On Thu, 02 Aug 2012, Ben Widawsky <ben at bwidawsk.net> wrote:
> > On 2012-08-02 05:07, Vijay Purushothaman wrote:
> >> In Valleyview the DPLL and lane control registers are accessible
> >> only through side band fabric called DPIO. Added two tools to read
> >> and write
> >> registers residing in this space.
> >
> > Could I convince you to use the centralized read/write mmio
> > functions? Otherwise, everything seems fine to me here.
>
> The introduced intel_dpio_read.c and intel_dpio_write.c files are
> almost the same. They could share most of the code instead of
> duplication, IMO.
>
> BR,
> Jani.
>
I would still prefer separate dpio read and write functions
more along the lines of intel_reg_read and intel_reg_write. I will move
these functions to intel_reg_read.c and intel_reg_write.c based on
Ben's feedback.
Thanks,
Vijay
>
> >
> >>
> >> Signed-off-by: Vijay Purushothaman
> >> <vijay.a.purushothaman at intel.com> ---
> >> tools/Makefile.am | 2 +
> >> tools/intel_dpio_read.c | 105
> >> ++++++++++++++++++++++++++++++++++++++++++++++
> >> tools/intel_dpio_write.c | 103
> >> +++++++++++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 210 insertions(+)
> >> create mode 100644 tools/intel_dpio_read.c
> >> create mode 100644 tools/intel_dpio_write.c
> >>
> >> diff --git a/tools/Makefile.am b/tools/Makefile.am
> >> index d461f38..71fb087 100644
> >> --- a/tools/Makefile.am
> >> +++ b/tools/Makefile.am
> >> @@ -15,6 +15,8 @@ bin_PROGRAMS = \
> >> intel_reg_write \
> >> intel_reg_read \
> >> intel_forcewaked \
> >> + intel_dpio_read \
> >> + intel_dpio_write \
> >> intel_l3_parity
> >>
> >> noinst_PROGRAMS = \
> >> diff --git a/tools/intel_dpio_read.c b/tools/intel_dpio_read.c
> >> new file mode 100644
> >> index 0000000..8b924fd
> >> --- /dev/null
> >> +++ b/tools/intel_dpio_read.c
> >> @@ -0,0 +1,105 @@
> >> +/*
> >> + * Copyright ? 2012 Intel Corporation
> >> + *
> >> + * Permission is hereby granted, free of charge, to any person
> >> obtaining a
> >> + * copy of this software and associated documentation files (the
> >> "Software"),
> >> + * to deal in the Software without restriction, including without
> >> limitation
> >> + * the rights to use, copy, modify, merge, publish, distribute,
> >> sublicense,
> >> + * and/or sell copies of the Software, and to permit persons to
> >> whom the
> >> + * Software is furnished to do so, subject to the following
> >> conditions:
> >> + *
> >> + * The above copyright notice and this permission notice
> >> (including the next
> >> + * paragraph) shall be included in all copies or substantial
> >> portions of the
> >> + * Software.
> >> + *
> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >> EXPRESS OR
> >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> >> MERCHANTABILITY,
> >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> >> EVENT SHALL
> >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> >> DAMAGES OR OTHER
> >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> >> OTHERWISE, ARISING
> >> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >> OTHER
> >> + * DEALINGS IN THE SOFTWARE.
> >> + *
> >> + * Authors:
> >> + * Vijay Purushothaman
> >> <vijay.a.purushothaman at intel.com>
> >> + *
> >> + */
> >> +
> >> +#include <unistd.h>
> >> +#include <stdlib.h>
> >> +#include <stdio.h>
> >> +#include <err.h>
> >> +#include <string.h>
> >> +#include "intel_gpu_tools.h"
> >> +
> >> +#define VLV_DISPLAY_BASE 0x180000
> >> +#define DPIO_PKT 0x2100
> >> +#define DPIO_RID (0 << 24)
> >> +#define DPIO_OP_WRITE (1 << 16)
> >> +#define DPIO_OP_READ (0 << 16)
> >> +#define DPIO_PORTID (0x12 << 8)
> >> +#define DPIO_BYTE (0xf << 4)
> >> +#define DPIO_BUSY (1 << 0)
> >> +#define DPIO_DATA 0x2104
> >> +#define DPIO_REG 0x2108
> >> +
> >> +static uint32_t vlv_display_reg_read(uint32_t reg)
> >> +{
> >> + reg += VLV_DISPLAY_BASE;
> >> + return (*(volatile uint32_t *)((volatile char*)mmio +
> >> reg)); +}
> >> +
> >> +static void vlv_display_reg_write(uint32_t reg, uint32_t val)
> >> +{
> >> + volatile uint32_t *ptr;
> >> +
> >> + reg += VLV_DISPLAY_BASE;
> >> + ptr = (volatile uint32_t *)((volatile char *) mmio + reg);
> >> + *ptr = val;
> >> +}
> >> +
> >> +static void usage(char *cmdname)
> >> +{
> >> + printf("Warning : This program will work only on
> >> Valleyview\n");
> >> + printf("Usage: %s [addr]\n", cmdname);
> >> + printf("\t addr : in 0xXXXX format\n");
> >> +}
> >> +
> >> +int main(int argc, char** argv)
> >> +{
> >> + int ret = 0;
> >> + uint32_t reg, val;
> >> + char *cmdname = strdup(argv[0]);
> >> +
> >> + if (argc != 2) {
> >> + usage(cmdname);
> >> + ret = 1;
> >> + goto out;
> >> + }
> >> +
> >> + sscanf(argv[1], "0x%x", ®);
> >> +
> >> + intel_register_access_init(intel_get_pci_device(), 0);
> >> +
> >> + /* Check whether the side band fabric is ready to accept
> >> commands */
> >> + do {
> >> + usleep(1);
> >> + } while (vlv_display_reg_read(DPIO_PKT) & DPIO_BUSY);
> >> +
> >> + vlv_display_reg_write(DPIO_REG, reg);
> >> + vlv_display_reg_write(DPIO_PKT, DPIO_RID | DPIO_OP_READ |
> >> DPIO_PORTID |
> >> +
> >> DPIO_BYTE);
> >> + do {
> >> + usleep(1);
> >> + } while (vlv_display_reg_read(DPIO_PKT) & DPIO_BUSY);
> >> +
> >> + val = vlv_display_reg_read(DPIO_DATA);
> >> +
> >> + printf("Read DPIO register: 0x%x - Value : 0x%x\n", reg,
> >> val); +
> >> + intel_register_access_fini();
> >> +
> >> +out:
> >> + free(cmdname);
> >> + return ret;
> >> +}
> >> diff --git a/tools/intel_dpio_write.c b/tools/intel_dpio_write.c
> >> new file mode 100644
> >> index 0000000..96b72dd
> >> --- /dev/null
> >> +++ b/tools/intel_dpio_write.c
> >> @@ -0,0 +1,103 @@
> >> +/*
> >> + * Copyright ? 2012 Intel Corporation
> >> + *
> >> + * Permission is hereby granted, free of charge, to any person
> >> obtaining a
> >> + * copy of this software and associated documentation files (the
> >> "Software"),
> >> + * to deal in the Software without restriction, including without
> >> limitation
> >> + * the rights to use, copy, modify, merge, publish, distribute,
> >> sublicense,
> >> + * and/or sell copies of the Software, and to permit persons to
> >> whom the
> >> + * Software is furnished to do so, subject to the following
> >> conditions:
> >> + *
> >> + * The above copyright notice and this permission notice
> >> (including the next
> >> + * paragraph) shall be included in all copies or substantial
> >> portions of the
> >> + * Software.
> >> + *
> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >> EXPRESS OR
> >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> >> MERCHANTABILITY,
> >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> >> EVENT SHALL
> >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> >> DAMAGES OR OTHER
> >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> >> OTHERWISE, ARISING
> >> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >> OTHER
> >> + * DEALINGS IN THE SOFTWARE.
> >> + *
> >> + * Authors:
> >> + * Vijay Purushothaman
> >> <vijay.a.purushothaman at intel.com>
> >> + *
> >> + */
> >> +
> >> +#include <unistd.h>
> >> +#include <stdlib.h>
> >> +#include <stdio.h>
> >> +#include <err.h>
> >> +#include <string.h>
> >> +#include "intel_gpu_tools.h"
> >> +
> >> +#define VLV_DISPLAY_BASE 0x180000
> >> +#define DPIO_PKT 0x2100
> >> +#define DPIO_RID (0 << 24)
> >> +#define DPIO_OP_WRITE (1 << 16)
> >> +#define DPIO_OP_READ (0 << 16)
> >> +#define DPIO_PORTID (0x12 << 8)
> >> +#define DPIO_BYTE (0xf << 4)
> >> +#define DPIO_BUSY (1 << 0)
> >> +#define DPIO_DATA 0x2104
> >> +#define DPIO_REG 0x2108
> >> +
> >> +static uint32_t vlv_display_reg_read(uint32_t reg)
> >> +{
> >> + reg += VLV_DISPLAY_BASE;
> >> + return (*(volatile uint32_t *)((volatile char*)mmio +
> >> reg)); +}
> >> +
> >> +static void vlv_display_reg_write(uint32_t reg, uint32_t val)
> >> +{
> >> + volatile uint32_t *ptr;
> >> +
> >> + reg += VLV_DISPLAY_BASE;
> >> + ptr = (volatile uint32_t *)((volatile char *) mmio + reg);
> >> + *ptr = val;
> >> +}
> >> +
> >> +static void usage(char *cmdname)
> >> +{
> >> + printf("Warning : This program will work only on
> >> Valleyview\n");
> >> + printf("Usage: %s [addr] [val]\n", cmdname);
> >> + printf("\t addr : in 0xXXXX format\n");
> >> +}
> >> +
> >> +int main(int argc, char** argv)
> >> +{
> >> + int ret = 0;
> >> + uint32_t reg, val;
> >> + char *cmdname = strdup(argv[0]);
> >> +
> >> + if (argc != 3) {
> >> + usage(cmdname);
> >> + ret = 1;
> >> + goto out;
> >> + }
> >> +
> >> + sscanf(argv[1], "0x%x", ®);
> >> + sscanf(argv[2], "0x%x", &val);
> >> +
> >> + intel_register_access_init(intel_get_pci_device(), 0);
> >> +
> >> + /* Check whether the side band fabric is ready to accept
> >> commands */
> >> + do {
> >> + usleep(1);
> >> + } while (vlv_display_reg_read(DPIO_PKT) & DPIO_BUSY);
> >> +
> >> + vlv_display_reg_write(DPIO_DATA, val);
> >> + vlv_display_reg_write(DPIO_REG, reg);
> >> + vlv_display_reg_write(DPIO_PKT, DPIO_RID | DPIO_OP_WRITE
> >> | DPIO_PORTID |
> >> +
> >> DPIO_BYTE);
> >> + do {
> >> + usleep(1);
> >> + } while (vlv_display_reg_read(DPIO_PKT) & DPIO_BUSY);
> >> +
> >> + intel_register_access_fini();
> >> +
> >> +out:
> >> + free(cmdname);
> >> + return ret;
> >> +}
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
