[Intel-gfx] [PATCH 1/1] drm/i915/display: convert dsc debugfs entry from output_bpp to input_bpc
Sharma, Swati2
swati2.sharma at intel.com
Fri Sep 2 19:22:15 UTC 2022
Thanks Jani for the review comments.
Have addressed in v2.
On 01-Sep-22 1:41 PM, Jani Nikula wrote:
> On Wed, 31 Aug 2022, Swati Sharma <swati2.sharma at intel.com> wrote:
>> With this patch, converting DSC debugfs entry from output_bpp to input_bpc.
>
> Please drop phrases like "With this patch", and just use imperative
> mood, "Convert DSC debugfs ...". Once it's committed and you look at git
> log, it's no longer a patch.
>
> The subject prefix could have "drm/i915/dsc".
>
>> Corresponding changes are done in i-g-t to validate DSC with different
>> input bpc supported per platform.
>
> The commit message should convey the "why". I guess the rationale is to
> be able to test input bpc? That should be the main thing, not the igt
> changes.
>
> Okay, let's say this and the igt changes get merged in lock step. We
> still have CI running new IGT on older kernels such as
> drm-intel-fixes. What happens?
With latest igt and old kernel, existing o/p bpp test will fail since
test won't be able to find
the debugfs entry and there is assert present in igt.
>
>>
>> Signed-off-by: Swati Sharma <swati2.sharma at intel.com>
>> ---
>> .../drm/i915/display/intel_display_debugfs.c | 26 +++++++++----------
>> .../drm/i915/display/intel_display_types.h | 2 +-
>> drivers/gpu/drm/i915/display/intel_dp.c | 23 +++++-----------
>> 3 files changed, 21 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> index 225b6bfc783c..23627ed3beb1 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> @@ -2138,7 +2138,7 @@ static const struct file_operations i915_dsc_fec_support_fops = {
>> .write = i915_dsc_fec_support_write
>> };
>>
>> -static int i915_dsc_bpp_show(struct seq_file *m, void *data)
>> +static int i915_dsc_bpc_show(struct seq_file *m, void *data)
>> {
>> struct drm_connector *connector = m->private;
>> struct drm_device *dev = connector->dev;
>> @@ -2161,14 +2161,14 @@ static int i915_dsc_bpp_show(struct seq_file *m, void *data)
>> }
>>
>> crtc_state = to_intel_crtc_state(crtc->state);
>> - seq_printf(m, "Compressed_BPP: %d\n", crtc_state->dsc.compressed_bpp);
>> + seq_printf(m, "Input_BPC: %d\n", crtc_state->dsc.config.bits_per_component);
>>
>> out: drm_modeset_unlock(&dev->mode_config.connection_mutex);
>>
>> return ret;
>> }
>>
>> -static ssize_t i915_dsc_bpp_write(struct file *file,
>> +static ssize_t i915_dsc_bpc_write(struct file *file,
>> const char __user *ubuf,
>> size_t len, loff_t *offp)
>> {
>> @@ -2176,33 +2176,33 @@ static ssize_t i915_dsc_bpp_write(struct file *file,
>> ((struct seq_file *)file->private_data)->private;
>> struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
>> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>> - int dsc_bpp = 0;
>> + int dsc_bpc = 0;
>> int ret;
>>
>> - ret = kstrtoint_from_user(ubuf, len, 0, &dsc_bpp);
>> + ret = kstrtoint_from_user(ubuf, len, 0, &dsc_bpc);
>> if (ret < 0)
>> return ret;
>>
>> - intel_dp->force_dsc_bpp = dsc_bpp;
>> + intel_dp->force_dsc_bpc = dsc_bpc;
>> *offp += len;
>>
>> return len;
>> }
>>
>> -static int i915_dsc_bpp_open(struct inode *inode,
>> +static int i915_dsc_bpc_open(struct inode *inode,
>> struct file *file)
>> {
>> - return single_open(file, i915_dsc_bpp_show,
>> + return single_open(file, i915_dsc_bpc_show,
>> inode->i_private);
>
> Bikeshed, while here, the modified lines could be reflowed to fit on one
> line.
>
>> }
>>
>> -static const struct file_operations i915_dsc_bpp_fops = {
>> +static const struct file_operations i915_dsc_bpc_fops = {
>> .owner = THIS_MODULE,
>> - .open = i915_dsc_bpp_open,
>> + .open = i915_dsc_bpc_open,
>> .read = seq_read,
>> .llseek = seq_lseek,
>> .release = single_release,
>> - .write = i915_dsc_bpp_write
>> + .write = i915_dsc_bpc_write
>> };
>>
>> /*
>> @@ -2272,8 +2272,8 @@ void intel_connector_debugfs_add(struct intel_connector *intel_connector)
>> debugfs_create_file("i915_dsc_fec_support", 0644, root,
>> connector, &i915_dsc_fec_support_fops);
>>
>> - debugfs_create_file("i915_dsc_bpp", 0644, root,
>> - connector, &i915_dsc_bpp_fops);
>> + debugfs_create_file("i915_dsc_bpc", 0644, root,
>> + connector, &i915_dsc_bpc_fops);
>> }
>>
>> if (connector->connector_type == DRM_MODE_CONNECTOR_DSI ||
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index 0da9b208d56e..dbda845030bf 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -1712,7 +1712,7 @@ struct intel_dp {
>>
>> /* Display stream compression testing */
>> bool force_dsc_en;
>> - int force_dsc_bpp;
>> + int force_dsc_bpc;
>>
>> bool hobl_failed;
>> bool hobl_active;
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 8d1559323412..0d75b00d3e5d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -1474,6 +1474,13 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>>
>> pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, conn_state->max_requested_bpc);
>>
>> + if (intel_dp->force_dsc_bpc) {
>> + pipe_bpp = intel_dp->force_dsc_bpc * 3;
>> + drm_dbg_kms(&dev_priv->drm,
>> + "Input DSC BPP forced to %d",
>> + pipe_bpp);
>
> Bikeshed, this would also fit on fewer lines.
>
> Other than the nitpicks, this seems to make everything simpler and
> exercise the regular code paths better also with forced dsc bpc.
>
> BR,
> Jani.
>
>> + }
>> +
>> /* Min Input BPC for ICL+ is 8 */
>> if (pipe_bpp < 8 * 3) {
>> drm_dbg_kms(&dev_priv->drm,
>> @@ -1525,22 +1532,6 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>> pipe_config->dsc.slice_count = dsc_dp_slice_count;
>> }
>>
>> - /* As of today we support DSC for only RGB */
>> - if (intel_dp->force_dsc_bpp) {
>> - if (intel_dp->force_dsc_bpp >= 8 &&
>> - intel_dp->force_dsc_bpp < pipe_bpp) {
>> - drm_dbg_kms(&dev_priv->drm,
>> - "DSC BPP forced to %d",
>> - intel_dp->force_dsc_bpp);
>> - pipe_config->dsc.compressed_bpp =
>> - intel_dp->force_dsc_bpp;
>> - } else {
>> - drm_dbg_kms(&dev_priv->drm,
>> - "Invalid DSC BPP %d",
>> - intel_dp->force_dsc_bpp);
>> - }
>> - }
>> -
>> /*
>> * VDSC engine operates at 1 Pixel per clock, so if peak pixel rate
>> * is greater than the maximum Cdclock and if slice count is even
>
--
~Swati Sharma
More information about the Intel-gfx
mailing list