| Message ID | 1418255597-4716-5-git-send-email-tprevite@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 11 Dec 2014, Todd Previte <tprevite@gmail.com> wrote: > This patch was part of "[PATCH 05/10] drm/i915: Add debugfs interface for > Displayport debug and compliance testing". That patch has been split into > smaller patches for ease of review and integration. > > This patch contains the definitions/declarations for some of the constants > and data structures added to support debugfs output for Displayport compliance > testing. > > Signed-off-by: Todd Previte <tprevite@gmail.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 41 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 479e0c1..65b4f5e 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -33,6 +33,7 @@ > #include <linux/slab.h> > #include <linux/export.h> > #include <linux/list_sort.h> > +#include <linux/string.h> > #include <asm/msr-index.h> > #include <drm/drmP.h> > #include "intel_drv.h" > @@ -51,6 +52,46 @@ static const char *yesno(int v) > return v ? "yes" : "no"; > } > > +#define DP_PARAMETER_COUNT 8 > +#define MAX_DP_CONFIG_LINE_COUNT 64 > + > +enum dp_config_param { > + DP_CONFIG_PARAM_INVALID = -1, > + DP_CONFIG_PARAM_CONNECTOR = 0, > + DP_CONFIG_PARAM_LINK_RATE, > + DP_CONFIG_PARAM_LANE_COUNT, > + DP_CONFIG_PARAM_VOLTAGE_SWING, > + DP_CONFIG_PARAM_PREEMPHASIS, > + DP_CONFIG_PARAM_HRES, > + DP_CONFIG_PARAM_VRES, > + DP_CONFIG_PARAM_BPP If you start the enum with DP_CONFIG_PARAM_CONNECTOR, you can add DP_PARAMETER_COUNT here which will automatically be correct, and then add DP_CONFIG_PARAM_INVALID = -1 after that. > +}; > + > +struct dp_config { > + enum dp_config_param type; > + unsigned long value; > +}; > + > +const char *dp_conf_tokens[DP_PARAMETER_COUNT] = { > + "Connector", > + "Link Rate", > + "Lane Count", > + "Voltage Swing Level", > + "Preemphasis Level", > + "Horizontal Resolution", > + "Vertical Resolution", > + "Bits Per Pixel" It's customary to initialize these with designated initializers so that the order here is not tied to the enum. I.e. [DP_CONFIG_PARAM_CONNECTOR] = "Connector", > +}; > +/* Strings for writing out dp_configs */ > +#define DP_CONF_STR_CONNECTOR "Connector: %s\n" > +#define DP_CONF_STR_LINKRATE "Link Rate: %02x\n" > +#define DP_CONF_STR_LANES "Lane Count: %u\n" > +#define DP_CONF_STR_VSWING "Voltage Swing Level: %u\n" > +#define DP_CONF_STR_PREEMP "Preemphasis Level: %u\n" > +#define DP_CONF_STR_HRES "Horizontal Resolution: %d\n" > +#define DP_CONF_STR_VRES "Vertical Resolution: %d\n" > +#define DP_CONF_STR_BPP "Bits Per Pixel: %u\n" I really don't like format strings as defines, because it moves the format string away from the parameters that need to match the format. A better option might be to have helper wrappers for seq_printf that take the required parameters. BR, Jani. > + > /* As the drm_debugfs_init() routines are called before dev->dev_private is > * allocated we need to hook into the minor for release. */ > static int > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 12/15/14 6:11 AM, Jani Nikula wrote: > On Thu, 11 Dec 2014, Todd Previte<tprevite@gmail.com> wrote: >> This patch was part of "[PATCH 05/10] drm/i915: Add debugfs interface for >> Displayport debug and compliance testing". That patch has been split into >> smaller patches for ease of review and integration. >> >> This patch contains the definitions/declarations for some of the constants >> and data structures added to support debugfs output for Displayport compliance >> testing. >> >> Signed-off-by: Todd Previte<tprevite@gmail.com> >> --- >> drivers/gpu/drm/i915/i915_debugfs.c | 41 +++++++++++++++++++++++++++++++++++++ >> 1 file changed, 41 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c >> index 479e0c1..65b4f5e 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -33,6 +33,7 @@ >> #include <linux/slab.h> >> #include <linux/export.h> >> #include <linux/list_sort.h> >> +#include <linux/string.h> >> #include <asm/msr-index.h> >> #include <drm/drmP.h> >> #include "intel_drv.h" >> @@ -51,6 +52,46 @@ static const char *yesno(int v) >> return v ? "yes" : "no"; >> } >> >> +#define DP_PARAMETER_COUNT 8 >> +#define MAX_DP_CONFIG_LINE_COUNT 64 >> + >> +enum dp_config_param { >> + DP_CONFIG_PARAM_INVALID = -1, >> + DP_CONFIG_PARAM_CONNECTOR = 0, >> + DP_CONFIG_PARAM_LINK_RATE, >> + DP_CONFIG_PARAM_LANE_COUNT, >> + DP_CONFIG_PARAM_VOLTAGE_SWING, >> + DP_CONFIG_PARAM_PREEMPHASIS, >> + DP_CONFIG_PARAM_HRES, >> + DP_CONFIG_PARAM_VRES, >> + DP_CONFIG_PARAM_BPP > If you start the enum with DP_CONFIG_PARAM_CONNECTOR, you can add > DP_PARAMETER_COUNT here which will automatically be correct, and then > add DP_CONFIG_PARAM_INVALID = -1 after that. Good point. That change has been integrated into V3. >> +}; >> + >> +struct dp_config { >> + enum dp_config_param type; >> + unsigned long value; >> +}; >> + >> +const char *dp_conf_tokens[DP_PARAMETER_COUNT] = { >> + "Connector", >> + "Link Rate", >> + "Lane Count", >> + "Voltage Swing Level", >> + "Preemphasis Level", >> + "Horizontal Resolution", >> + "Vertical Resolution", >> + "Bits Per Pixel" > It's customary to initialize these with designated initializers so that > the order here is not tied to the enum. I.e. > > [DP_CONFIG_PARAM_CONNECTOR] = "Connector", Makes sense. Fixed for V3. >> +}; >> +/* Strings for writing out dp_configs */ >> +#define DP_CONF_STR_CONNECTOR "Connector: %s\n" >> +#define DP_CONF_STR_LINKRATE "Link Rate: %02x\n" >> +#define DP_CONF_STR_LANES "Lane Count: %u\n" >> +#define DP_CONF_STR_VSWING "Voltage Swing Level: %u\n" >> +#define DP_CONF_STR_PREEMP "Preemphasis Level: %u\n" >> +#define DP_CONF_STR_HRES "Horizontal Resolution: %d\n" >> +#define DP_CONF_STR_VRES "Vertical Resolution: %d\n" >> +#define DP_CONF_STR_BPP "Bits Per Pixel: %u\n" > I really don't like format strings as defines, because it moves the > format string away from the parameters that need to match the format. > > A better option might be to have helper wrappers for seq_printf that > take the required parameters. > > BR, > Jani. Fair enough. I added a function to wrap the seq_printf function and one to initialize the tokens instead of having them are straight #defines. Other patches further on in the sequence will need to be adjusted to accommodate these changes. >> + >> /* As the drm_debugfs_init() routines are called before dev->dev_private is >> * allocated we need to hook into the minor for release. */ >> static int >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 479e0c1..65b4f5e 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -33,6 +33,7 @@ #include <linux/slab.h> #include <linux/export.h> #include <linux/list_sort.h> +#include <linux/string.h> #include <asm/msr-index.h> #include <drm/drmP.h> #include "intel_drv.h" @@ -51,6 +52,46 @@ static const char *yesno(int v) return v ? "yes" : "no"; } +#define DP_PARAMETER_COUNT 8 +#define MAX_DP_CONFIG_LINE_COUNT 64 + +enum dp_config_param { + DP_CONFIG_PARAM_INVALID = -1, + DP_CONFIG_PARAM_CONNECTOR = 0, + DP_CONFIG_PARAM_LINK_RATE, + DP_CONFIG_PARAM_LANE_COUNT, + DP_CONFIG_PARAM_VOLTAGE_SWING, + DP_CONFIG_PARAM_PREEMPHASIS, + DP_CONFIG_PARAM_HRES, + DP_CONFIG_PARAM_VRES, + DP_CONFIG_PARAM_BPP +}; + +struct dp_config { + enum dp_config_param type; + unsigned long value; +}; + +const char *dp_conf_tokens[DP_PARAMETER_COUNT] = { + "Connector", + "Link Rate", + "Lane Count", + "Voltage Swing Level", + "Preemphasis Level", + "Horizontal Resolution", + "Vertical Resolution", + "Bits Per Pixel" +}; +/* Strings for writing out dp_configs */ +#define DP_CONF_STR_CONNECTOR "Connector: %s\n" +#define DP_CONF_STR_LINKRATE "Link Rate: %02x\n" +#define DP_CONF_STR_LANES "Lane Count: %u\n" +#define DP_CONF_STR_VSWING "Voltage Swing Level: %u\n" +#define DP_CONF_STR_PREEMP "Preemphasis Level: %u\n" +#define DP_CONF_STR_HRES "Horizontal Resolution: %d\n" +#define DP_CONF_STR_VRES "Vertical Resolution: %d\n" +#define DP_CONF_STR_BPP "Bits Per Pixel: %u\n" + /* As the drm_debugfs_init() routines are called before dev->dev_private is * allocated we need to hook into the minor for release. */ static int
This patch was part of "[PATCH 05/10] drm/i915: Add debugfs interface for Displayport debug and compliance testing". That patch has been split into smaller patches for ease of review and integration. This patch contains the definitions/declarations for some of the constants and data structures added to support debugfs output for Displayport compliance testing. Signed-off-by: Todd Previte <tprevite@gmail.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 41 +++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)