On Thu, 19 Feb 2015, Todd Previte <tprevite@xxxxxxxxx> wrote: > This patch is the amalgamation of 7 patches from the V2 series. These > patches all involve the implementation of the debugfs mechanism for > handling Displayport compliance testing. The following are the commit > messages from those 7 patches (included here to try and preserve the > patch set history): I think there should be per connector debugfs files for this, instead of doing it for all DP connectors. See my dpcd debugfs patch [1] for an example. I wish that got reviewed and merged... BR, Jani. [1] http://mid.gmane.org/1424867645-21608-1-git-send-email-jani.nikula@xxxxxxxxx > > ------------------------------------------------------------------------------- > [PATCH 04/17] drm/i915: Add debugfs information for Displayport compliance > testing > > Originally, this patch was part of "[PATCH 05/10] drm/i915: Add debugfs > interface for Displayport debug and compliance testing". It contained > definitions/declarations for some of the constants and data structures > added to support debugfs output for Displayport compliance > testing. > ------------------------------------------------------------------------------- > [PATCH 05/17] drm/i915: Add file ops for Displayport configuration in debugfs > > This patch was also part of "[PATCH 05/10] drm/i915: Add debugfs interface > for Displayport debug and compliance testing". This one added file operations > structures for Displayport configuration and the declarations for open() and > write() in the file ops structure. > ------------------------------------------------------------------------------- > [PATCH 06/17] drm/i915: Add support functions in debugfs for handling > Displayport compliance configuration > > This patch was previously part of "[PATCH 05/10] drm/i915: Add debugfs interface > for Displayport debug and compliance testing". It added two support functions > for handling Displayport configuration parameters that are used for compliance > testing. > ------------------------------------------------------------------------------- > [PATCH 07/17] drm/i915: Add and implement the debugfs 'show' functions for > Displayport compliance > > This patch was previously part of "[PATCH 05/10] drm/i915: Add debugfs > interface for Displayport debug and compliance testing". This patch implemented > the show() function. > ------------------------------------------------------------------------------- > [PATCH 08/17] drm/i915: Add Displayport link configuration structure > > This patch was previously part of "[PATCH 07/10] drm/i915: Add structures for > Displayport compliance testing parameters". It added a struct to maintain link > configuration data. These were meant purely for Displayport compliance use. > ------------------------------------------------------------------------------- > [PATCH 09/17] drm/i915: Add config parsing utilities in debugfs for Displayport > compliance > > This patch was previously part of "[PATCH 05/10] drm/i915: Add debugfs > interface for Displayport debug and compliance testing". This patch adds two > functions to handle parsing of Displayport configuration information as it > formatted in the debugfs file. It is used to process incoming configuration > changes from the userspace compliance application during testing. > > ------------------------------------------------------------------------------- > [PATCH 10/17] drm/i915: Implement the 'open' and 'write' debugfs functions for > Displayport compliance > > This patch is a combination of sections out of the following two previous > patches: > [PATCH 05/10] drm/i915: Add debugfs interface for Displayport debug and > compliance testing > [PATCH 07/10] drm/i915: Add structures for Displayport compliance testing > parameters > > This patch implements the debugfs functions for 'open' and 'write' as they > are specified in the file ops structure. The 'open' function outputs > Displayport data to the appropriate debugfs file while the 'write' function > handles parsing of user data written to that same file. > ------------------------------------------------------------------------------- > > The nature and scope of the contents of this patch make breaking it into > smaller chunks difficult. With everthing contained in a single patch, the > code is easier to review and understand. > > That said, this monster of a patch does quite a few things. First, it adds > some necessary data structures for performing compliance testing operations > on the kernel side. Specifically, it adds link configuration parameters and > string tokens for the debugfs files. There are multiple utility functions > added here as well in support of the debugfs system for handling compliance > testing. Functions for printing strings to the files with the correct format, > parsing tokens, tokenizing strings and doing the grunt work for writing the > data to the debugfs files. > > This patch also adds the basic debugfs structures to build the file for the > link configuration output as well as the file operations structures to handle > the open and show functions. > > V2: > - N/A > V3: > - Merged patches 4-10 into a single patch per review feedback > - Added previous commit messages from the other commits integrated > here to preserve the integrity of the patch sequence > - Adjusted the config_ctl_write function to only check the drm_connector > struct for a) type == Displayport and b) status == active. This is > to eliminate patches 14 and 15 from V2, which become unnecessary > in light of this change. > - Fixed checkpatch.pl errors > > Signed-off-by: Todd Previte <tprevite@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 471 ++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 13 + > 2 files changed, 484 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index b315f01..b77574e 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,121 @@ static const char *yesno(int v) > return v ? "yes" : "no"; > } > > +#define MAX_DP_CONFIG_LINE_COUNT 64 > + > +enum dp_config_param { > + DP_CONFIG_PARAM_CONNECTOR = 0, > + DP_CONFIG_PARAM_CONNECTOR_ID, > + 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, > + DP_PARAMETER_COUNT, > + DP_CONFIG_PARAM_INVALID = -1 > +}; > + > +struct dp_config { > + enum dp_config_param type; > + unsigned long value; > +}; > + > +static char *dp_get_config_str(int for_token) > +{ > + char *s; > + > + switch (for_token) { > + case DP_CONFIG_PARAM_CONNECTOR: > + s = "Connector"; > + break; > + case DP_CONFIG_PARAM_CONNECTOR_ID: > + s = "Connector ID"; > + break; > + case DP_CONFIG_PARAM_LINK_RATE: > + s = "Link Rate"; > + break; > + case DP_CONFIG_PARAM_LANE_COUNT: > + s = "Lane Count"; > + break; > + case DP_CONFIG_PARAM_VOLTAGE_SWING: > + s = "Voltage Swing Level"; > + break; > + case DP_CONFIG_PARAM_PREEMPHASIS: > + s = "Preemphasis Level"; > + break; > + case DP_CONFIG_PARAM_HRES: > + s = "Horizontal Resolution"; > + break; > + case DP_CONFIG_PARAM_VRES: > + s = "Vertical Resolution"; > + break; > + case DP_CONFIG_PARAM_BPP: > + s = "Bits Per Pixel"; > + break; > + default: > + s = ""; > + break; > + } > + > + return s; > +} > + > +static void dp_print_string(struct seq_file *m, > + enum dp_config_param for_param, > + void *data) > +{ > + char *format_string; > + unsigned long output_value; > + > + switch (for_param) { > + case DP_CONFIG_PARAM_CONNECTOR: > + /* Special case for the string parameter */ > + format_string = "Connector: %s\n"; > + seq_printf(m, format_string, (char *) data); > + break; > + case DP_CONFIG_PARAM_CONNECTOR_ID: > + format_string = "Connector ID: %u\n"; > + output_value = *(unsigned int *) data; > + break; > + case DP_CONFIG_PARAM_LINK_RATE: > + format_string = "Link Rate: %02x\n"; > + output_value = *(unsigned char *) data; > + break; > + case DP_CONFIG_PARAM_LANE_COUNT: > + format_string = "Lane Count: %u\n"; > + output_value = *(unsigned int *) data; > + break; > + case DP_CONFIG_PARAM_VOLTAGE_SWING: > + format_string = "Voltage Swing Level: %u\n"; > + output_value = *(unsigned char *) data; > + break; > + case DP_CONFIG_PARAM_PREEMPHASIS: > + format_string = "Preemphasis Level: %u\n"; > + output_value = *(unsigned char *) data; > + break; > + case DP_CONFIG_PARAM_HRES: > + format_string = "Horizontal Resolution: %d\n"; > + output_value = *(int *) data; > + break; > + case DP_CONFIG_PARAM_VRES: > + format_string = "Vertical Resolution: %d\n"; > + output_value = *(int *) data; > + break; > + case DP_CONFIG_PARAM_BPP: > + format_string = "Bits Per Pixel: %u\n"; > + output_value = *(unsigned char *) data; > + break; > + default: > + format_string = NULL; > + break; > + } > + > + if (format_string != NULL && for_param != DP_CONFIG_PARAM_CONNECTOR) > + seq_printf(m, format_string, output_value); > +} > + > /* 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 > @@ -3701,6 +3817,360 @@ static const struct file_operations i915_display_crc_ctl_fops = { > .write = display_crc_ctl_write > }; > > +static void dp_show_config(struct seq_file *m, > + struct drm_connector *connector) > +{ > + struct intel_dp *intel_dp; > + struct intel_crtc *crtc; > + struct intel_connector *intel_connector = to_intel_connector(connector); > + struct intel_encoder *intel_encoder = intel_connector->encoder; > + int pipe_bpp, hres, vres; > + uint8_t vs[4], pe[4]; > + struct drm_display_mode *mode; > + int i = 0; > + > + if (!intel_encoder) > + return; > + > + if (intel_encoder->type != INTEL_OUTPUT_DISPLAYPORT) > + return; > + > + intel_dp = enc_to_intel_dp(&intel_encoder->base); > + > + for (i = 0; i < intel_dp->lane_count; i++) { > + vs[i] = intel_dp->train_set[i] > + & DP_TRAIN_VOLTAGE_SWING_MASK; > + pe[i] = (intel_dp->train_set[i] > + & DP_TRAIN_PRE_EMPHASIS_MASK) >> > + DP_TRAIN_PRE_EMPHASIS_SHIFT; > + } > + > + /* Output mode info only if a valid CRTC exists */ > + if (intel_encoder->base.crtc) { > + crtc = to_intel_crtc(intel_encoder->base.crtc); > + pipe_bpp = crtc->config->pipe_bpp; > + mode = &crtc->config->base.adjusted_mode; > + hres = mode->hdisplay; > + vres = mode->vdisplay; > + } else { > + pipe_bpp = 0; > + hres = vres = 0; > + } > + > + dp_print_string(m, > + DP_CONFIG_PARAM_CONNECTOR, > + connector->name); > + dp_print_string(m, > + DP_CONFIG_PARAM_CONNECTOR_ID, > + &connector->base.id); > + dp_print_string(m, > + DP_CONFIG_PARAM_LINK_RATE, > + &intel_dp->link_bw); > + dp_print_string(m, > + DP_CONFIG_PARAM_LANE_COUNT, > + &intel_dp->lane_count); > + dp_print_string(m, > + DP_CONFIG_PARAM_VOLTAGE_SWING, > + &vs[0]); > + dp_print_string(m, > + DP_CONFIG_PARAM_PREEMPHASIS, > + &pe[0]); > + dp_print_string(m, > + DP_CONFIG_PARAM_HRES, > + &hres); > + dp_print_string(m, > + DP_CONFIG_PARAM_VRES, > + &vres); > + dp_print_string(m, > + DP_CONFIG_PARAM_BPP, > + &pipe_bpp); > +} > + > +enum dp_config_param dp_get_param_type(char *input_string) > +{ > + enum dp_config_param param_type = DP_CONFIG_PARAM_INVALID; > + char *s; > + int i = 0; > + > + for (i = 0; i < DP_PARAMETER_COUNT; i++) { > + s = dp_get_config_str(i); > + if (strncmp(input_string, s, > + strlen(s)) == 0) { > + param_type = (enum dp_config_param) i; > + break; > + } > + } > + return param_type; > +} > + > +int dp_get_param_value(enum dp_config_param param, > + char *input_string, > + void *output_value) > +{ > + int status = 0; > + unsigned int *out_ptr = (unsigned int *)output_value; > + char *index = input_string; > + > + switch (param) { > + case DP_CONFIG_PARAM_CONNECTOR: > + output_value = (char *)index; > + break; > + case DP_CONFIG_PARAM_LINK_RATE: > + case DP_CONFIG_PARAM_LANE_COUNT: > + case DP_CONFIG_PARAM_VOLTAGE_SWING: > + case DP_CONFIG_PARAM_PREEMPHASIS: > + status = kstrtol(index, 16, (long *)out_ptr); > + break; > + case DP_CONFIG_PARAM_HRES: > + case DP_CONFIG_PARAM_VRES: > + case DP_CONFIG_PARAM_BPP: > + status = kstrtol(index, 10, (long *)out_ptr); > + break; > + default: > + /* Unhandled case */ > + status = -EINVAL; > + *out_ptr = 0; > + break; > + } > + > + return status; > +} > + > +int dp_tokenize_config(char *input_buffer, char *output[]) > +{ > + char *base = input_buffer, *index, *end; > + int line_count = 0; > + int i = 0, len = 0; > + int done = 0; > + > + if (!input_buffer) > + return 0; > + > + while (!done) { > + index = strpbrk(base, ":"); > + if (index) { > + len = index - base; > + *index = '\0'; > + index++; > + /* Save the type string */ > + output[i] = base; > + i++; > + line_count++; > + end = strpbrk(index, "\n\0"); > + if (end) { > + *end = '\0'; > + /* Eat up whitespace */ > + while (*index <= 0x20) > + index++; > + output[i] = index; > + i++; > + line_count++; > + } else > + done = 1; > + /* Move to the next section of the string */ > + base = end + 1; > + } else > + done = 1; > + } > + return line_count; > +} > + > +static int dp_parse_config(char *input_buffer, > + ssize_t buffer_size, > + struct intel_dp *intel_dp) > +{ > + int status = 0; > + char *lines[MAX_DP_CONFIG_LINE_COUNT]; > + int i = 0; > + struct dp_config parms[DP_PARAMETER_COUNT]; > + int line_count = 0; > + char *buffer = input_buffer; > + enum dp_config_param parm_type; > + unsigned long parm_value; > + > + line_count = dp_tokenize_config(buffer, lines); > + > + if (line_count == 0) { > + DRM_DEBUG_DRIVER("No lines to process\n"); > + return 0; > + } > + > + for (i = 0; i < line_count; i += 2) { > + parm_type = dp_get_param_type(lines[i]); > + if (parm_type != DP_CONFIG_PARAM_INVALID) { > + status = dp_get_param_value(parm_type, > + lines[i+1], > + &parm_value); > + if (status == 0) { > + parms[parm_type].type = parm_type; > + parms[parm_type].value = parm_value; > + } > + } > + } > + > + if (parms[DP_CONFIG_PARAM_LINK_RATE].value == 0x06 || > + parms[DP_CONFIG_PARAM_LINK_RATE].value == 0x0a || > + parms[DP_CONFIG_PARAM_LINK_RATE].value == 0x14) { > + intel_dp->compliance_config.link_rate = > + parms[DP_CONFIG_PARAM_LINK_RATE].value; > + } else > + return -EINVAL; > + > + if (parms[DP_CONFIG_PARAM_LANE_COUNT].value == 0x01 || > + parms[DP_CONFIG_PARAM_LANE_COUNT].value == 0x02 || > + parms[DP_CONFIG_PARAM_LANE_COUNT].value == 0x04) { > + intel_dp->compliance_config.lane_count = > + parms[DP_CONFIG_PARAM_LANE_COUNT].value; > + } else > + return -EINVAL; > + > + if (parms[DP_CONFIG_PARAM_VOLTAGE_SWING].value <= 0x03 && > + parms[DP_CONFIG_PARAM_VOLTAGE_SWING].value >= 0x00) { > + intel_dp->compliance_config.vswing_level = > + parms[DP_CONFIG_PARAM_VOLTAGE_SWING].value; > + } else > + return -EINVAL; > + > + if (parms[DP_CONFIG_PARAM_PREEMPHASIS].value <= 0x03 && > + parms[DP_CONFIG_PARAM_PREEMPHASIS].value >= 0x00) { > + intel_dp->compliance_config.preemp_level = > + parms[DP_CONFIG_PARAM_PREEMPHASIS].value; > + } else > + return -EINVAL; > + > + if (parms[DP_CONFIG_PARAM_BPP].value == 18 || > + parms[DP_CONFIG_PARAM_BPP].value == 24 || > + parms[DP_CONFIG_PARAM_BPP].value == 30 || > + parms[DP_CONFIG_PARAM_BPP].value == 36) { > + intel_dp->compliance_config.bits_per_pixel = > + parms[DP_CONFIG_PARAM_PREEMPHASIS].value; > + } else > + return -EINVAL; > + > + if (parms[DP_CONFIG_PARAM_HRES].value > 0 && > + parms[DP_CONFIG_PARAM_HRES].value <= 8192) { > + intel_dp->compliance_config.hres = > + parms[DP_CONFIG_PARAM_HRES].value; > + } else > + return -EINVAL; > + > + if (parms[DP_CONFIG_PARAM_VRES].value > 0 && > + parms[DP_CONFIG_PARAM_VRES].value <= 8192) { > + intel_dp->compliance_config.vres = > + parms[DP_CONFIG_PARAM_VRES].value; > + } else > + return -EINVAL; > + > + return status; > +} > + > +static int i915_displayport_config_ctl_show(struct seq_file *m, void *data) > +{ > + struct drm_device *dev = m->private; > + struct drm_connector *connector; > + struct list_head *connector_list = &dev->mode_config.connector_list; > + > + if (!dev) > + return -ENODEV; > + > + list_for_each_entry(connector, connector_list, head) { > + > + if (connector->connector_type != > + DRM_MODE_CONNECTOR_DisplayPort) > + continue; > + > + if (connector->status == connector_status_connected) { > + dp_show_config(m, connector); > + } else { > + dp_print_string(m, > + DP_CONFIG_PARAM_CONNECTOR, > + connector->name); > + dp_print_string(m, > + DP_CONFIG_PARAM_CONNECTOR_ID, > + &connector->base.id); > + seq_puts(m, "disconnected\n"); > + } > + } > + > + return 0; > +} > + > +static int i915_displayport_config_ctl_open(struct inode *inode, > + struct file *file) > +{ > + struct drm_device *dev = inode->i_private; > + > + return single_open(file, i915_displayport_config_ctl_show, dev); > +} > + > +static ssize_t i915_displayport_config_ctl_write(struct file *file, > + const char __user *ubuf, > + size_t len, loff_t *offp) > +{ > + char *input_buffer; > + int status = 0; > + struct seq_file *m; > + struct drm_device *dev; > + struct drm_connector *connector; > + struct list_head *connector_list; > + struct intel_dp *intel_dp; > + > + m = file->private_data; > + if (!m) { > + status = -ENODEV; > + return status; > + } > + dev = m->private; > + > + if (!dev) { > + status = -ENODEV; > + return status; > + } > + connector_list = &dev->mode_config.connector_list; > + > + if (len == 0) > + return 0; > + > + input_buffer = kmalloc(len + 1, GFP_KERNEL); > + if (!input_buffer) > + return -ENOMEM; > + > + if (copy_from_user(input_buffer, ubuf, len)) { > + status = -EFAULT; > + goto out; > + } > + > + input_buffer[len] = '\0'; > + DRM_DEBUG_DRIVER("Copied %d bytes from user\n", (unsigned int)len); > + > + list_for_each_entry(connector, connector_list, head) { > + if (connector->connector_type == > + DRM_MODE_CONNECTOR_DisplayPort && > + connector->status == connector_status_connected) { > + intel_dp = enc_to_intel_dp(connector->encoder); > + status = dp_parse_config(input_buffer, len, intel_dp); > + if (status < 0) > + goto out; > + } > + } > +out: > + kfree(input_buffer); > + if (status < 0) > + return status; > + > + *offp += len; > + return len; > +} > + > +static const struct file_operations i915_displayport_config_ctl_fops = { > + .owner = THIS_MODULE, > + .open = i915_displayport_config_ctl_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > + .write = i915_displayport_config_ctl_write > +}; > + > static void wm_latency_show(struct seq_file *m, const uint16_t wm[8]) > { > struct drm_device *dev = m->private; > @@ -4450,6 +4920,7 @@ static const struct i915_debugfs_files { > {"i915_spr_wm_latency", &i915_spr_wm_latency_fops}, > {"i915_cur_wm_latency", &i915_cur_wm_latency_fops}, > {"i915_fbc_false_color", &i915_fbc_fc_fops}, > + {"i915_displayport_config_ctl", &i915_displayport_config_ctl_fops} > }; > > void intel_display_crc_init(struct drm_device *dev) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 74b30c1..c813d3c 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -592,6 +592,18 @@ struct intel_hdmi { > struct intel_dp_mst_encoder; > #define DP_MAX_DOWNSTREAM_PORTS 0x10 > > +struct intel_dp_link_config { > + uint8_t lane_count; > + uint8_t link_rate; > + uint8_t vswing_level; > + uint8_t preemp_level; > + uint32_t hres; > + uint32_t vres; > + uint8_t bits_per_pixel; > + struct drm_display_mode *compliance_mode; > + char connector_name[12]; > +}; > + > struct intel_dp { > uint32_t output_reg; > uint32_t aux_ch_ctl_reg; > @@ -651,6 +663,7 @@ struct intel_dp { > /* Displayport compliance testing */ > unsigned long compliance_test_data; > bool compliance_testing_active; > + struct intel_dp_link_config compliance_config; > }; > > struct intel_digital_port { > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx