[PATCH] drm/i915/dsc: Add Per connector debugfs node for DSC BPP enable

Srivatsa, Anusha anusha.srivatsa at intel.com
Wed Jun 19 00:13:01 UTC 2019



>-----Original Message-----
>From: Navare, Manasi D
>Sent: Thursday, June 13, 2019 4:13 PM
>To: Srivatsa, Anusha <anusha.srivatsa at intel.com>
>Cc: intel-gfx-trybot at lists.freedesktop.org
>Subject: Re: [PATCH] drm/i915/dsc: Add Per connector debugfs node for DSC BPP
>enable
>
>On Thu, May 30, 2019 at 01:37:52PM -0700, Anusha wrote:
>> From: Anusha Srivatsa <anusha.srivatsa at intel.com>
>>
>> DSC can be supported per DP connector. This patch creates a per
>> connector debugfs node to expose the Input and Compressed BPP.
>>
>> The same node can be used from userspace to force DSC to a certain
>> BPP.
>>
>> force_dsc_bpp is written through this debugfs node to force DSC BPP to
>> all accepted values
>>
>> Cc: Manasi Navare <manasi.d.navare at intel.com>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c | 102 +++++++++++++++++++++++++++-
>>  drivers/gpu/drm/i915/intel_dp.c     |   4 ++
>>  drivers/gpu/drm/i915/intel_drv.h    |   1 +
>>  3 files changed, 106 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 74afdeff2245..d36ebb5c957a 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -4900,6 +4900,98 @@ static const struct file_operations
>i915_dsc_fec_support_fops = {
>>  	.write = i915_dsc_fec_support_write
>>  };
>>
>> +static int i915_dsc_bpp_slice_support_show(struct seq_file *m, void
>> +*data) {
>> +	struct drm_connector *connector = m->private;
>> +	struct drm_device *dev = connector->dev;
>> +	struct drm_crtc *crtc;
>> +	struct intel_dp *intel_dp;
>> +	struct drm_modeset_acquire_ctx ctx;
>> +	struct intel_crtc_state *crtc_state = NULL;
>> +	int ret = 0;
>> +	bool try_again = false;
>> +
>> +	drm_modeset_acquire_init(&ctx,
>DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
>> +
>> +	do {
>> +		try_again = false;
>> +		ret = drm_modeset_lock(&dev-
>>mode_config.connection_mutex,
>> +				       &ctx);
>> +		if (ret) {
>> +			ret = -EINTR;
>> +			break;
>> +		}
>> +		crtc = connector->state->crtc;
>> +		if (connector->status != connector_status_connected || !crtc) {
>> +			ret = -ENODEV;
>> +			break;
>> +		}
>> +		ret = drm_modeset_lock(&crtc->mutex, &ctx);
>> +		if (ret == -EDEADLK) {
>> +			ret = drm_modeset_backoff(&ctx);
>> +			if (!ret) {
>> +				try_again = true;
>> +				continue;
>> +			}
>> +			break;
>> +		} else if (ret) {
>> +			break;
>> +		}
>> +		intel_dp =
>enc_to_intel_dp(&intel_attached_encoder(connector)->base);
>> +		crtc_state = to_intel_crtc_state(crtc->state);
>> +		seq_printf(m, "Input_BPP: %d\n", crtc_state->pipe_bpp);
>> +		seq_printf(m, "Compressed_BPP: %d\n",
>> +crtc_state->dsc_params.compressed_bpp);
>
>You will need to expose even the max dsc bpp supported for eDP here and use
>that as upper bound for the dsc bpp array in the IGT for eDP since in case of eDP
>we obtain the max bpp from the dpcd unlike for DP

Yes. Makes sense.

>> +	} while (try_again);
>> +
>> +	drm_modeset_drop_locks(&ctx);
>> +	drm_modeset_acquire_fini(&ctx);
>> +
>> +	return ret;
>> +}
>> +
>> +static ssize_t i915_dsc_bpp_slice_support_write(struct file *file,
>> +						const char __user *ubuf,
>> +						size_t len, loff_t *offp)
>> +{
>> +	int dsc_bpp = 0;
>> +	int ret;
>> +	struct drm_connector *connector =
>> +		((struct seq_file *)file->private_data)->private;
>> +	struct intel_encoder *encoder = intel_attached_encoder(connector);
>> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>> +
>> +	if (len == 0)
>> +		return 0;
>> +
>> +	DRM_DEBUG_DRIVER("Copied %zu bytes from user to force BPP\n",
>> +			 len);
>> +
>> +	ret = kstrtoint_from_user(ubuf, len, 0, &dsc_bpp);
>> +
>> +	intel_dp->force_dsc_bpp = dsc_bpp;
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	*offp += len;
>> +	return len;
>> +}
>> +
>> +static int i915_dsc_bpp_slice_support_open(struct inode *inode,
>> +					   struct file *file)
>> +{
>> +	return single_open(file, i915_dsc_bpp_slice_support_show,
>> +			   inode->i_private);
>> +}
>> +
>> +static const struct file_operations i915_dsc_bpp_slice_support_fops = {
>> +	.owner = THIS_MODULE,
>> +	.open = i915_dsc_bpp_slice_support_open,
>> +	.read = seq_read,
>> +	.llseek = seq_lseek,
>> +	.release = single_release,
>> +	.write = i915_dsc_bpp_slice_support_write };
>> +
>>  /**
>>   * i915_debugfs_connector_add - add i915 specific connector debugfs files
>>   * @connector: pointer to a registered drm_connector @@ -4939,9
>> +5031,17 @@ int i915_debugfs_connector_add(struct drm_connector
>> *connector)
>>
>>  	if (INTEL_GEN(dev_priv) >= 10 &&
>>  	    (connector->connector_type ==
>DRM_MODE_CONNECTOR_DisplayPort ||
>> -	     connector->connector_type == DRM_MODE_CONNECTOR_eDP))
>> +	     connector->connector_type == DRM_MODE_CONNECTOR_eDP)) {
>>  		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,
>>  				    connector, &i915_dsc_fec_support_fops);
>> +	}
>> +
>> +	if (INTEL_GEN(dev_priv) >= 10 &&
>> +	    (connector->connector_type ==
>DRM_MODE_CONNECTOR_DisplayPort ||
>> +	     connector->connector_type == DRM_MODE_CONNECTOR_eDP)) {
>> +		debugfs_create_file("i915_dsc_bpp_slice_support", S_IRUGO,
>root,
>> +				    connector,
>&i915_dsc_bpp_slice_support_fops);
>> +	}
>>
>>  	return 0;
>>  }
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> b/drivers/gpu/drm/i915/intel_dp.c index 24b56b2a76c8..92b050e5013a
>> 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -2003,6 +2003,10 @@ static int intel_dp_dsc_compute_config(struct
>intel_dp *intel_dp,
>>  	}
>>
>>  	pipe_config->dsc_params.compression_enable = true;
>> +	if (intel_dp->force_dsc_bpp) {
>> +		DRM_ERROR("DSC BPC forced to %d", intel_dp->force_dsc_bpp);
>> +		pipe_config->dsc_params.compressed_bpp = intel_dp-
>>force_dsc_bpp;
>> +	}
>
>This forcing should happen where we get compressed_bpp from
>intel_dp_dsc_get_output_bpp because then we compute dsc params based on
>thsi bpp value and then set compression_enable to true at the end.
	
Good point.

>Also i think it only makes sense to run this bpp test for eDP because in case of dp
>we compute the optimum output BPP based on lot of parameters like the small
>joiner ram limits, link BW etc.

Hmm... true. 
Will make the suitable changes.
Thanks a lot for the reviews.

Anusha 

>Manasi
>
>>  	DRM_DEBUG_KMS("DP DSC computed with Input Bpp = %d "
>>  		      "Compressed Bpp = %d Slice Count = %d\n",
>>  		      pipe_config->pipe_bpp,
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index 0dcc03592d6e..06e7ff2e61b0 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1199,6 +1199,7 @@ struct intel_dp {
>>
>>  	/* Display stream compression testing */
>>  	bool force_dsc_en;
>> +	int force_dsc_bpp;
>>  };
>>
>>  enum lspcon_vendor {
>> --
>> 2.17.1
>>


More information about the Intel-gfx-trybot mailing list