[PATCH] drm/amd/display: Add pixel encoding info to debugfs

[PATCH] drm/amd/display: Add pixel encoding info to debugfs

[Rino Andre Johnsen]

[Why]
For debugging and testing purposes.

[How]
Create amdgpu_current_pixelencoding debugfs entry.
Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding

Signed-off-by: Rino Andre Johnsen <rinoandrejohnsen@gmail.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 27d5c6077630..d275e5522335 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct seq_file *m, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
 
+/*
+ * Returns the current pixelencoding for the crtc.
+ * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
+ */
+static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
+{
+	struct drm_crtc *crtc = m->private;
+	struct drm_device *dev = crtc->dev;
+	struct dm_crtc_state *dm_crtc_state = NULL;
+	int res = -ENODEV;
+
+	mutex_lock(&dev->mode_config.mutex);
+	drm_modeset_lock(&crtc->mutex, NULL);
+	if (crtc->state == NULL)
+		goto unlock;
+
+	dm_crtc_state = to_dm_crtc_state(crtc->state);
+	if (dm_crtc_state->stream == NULL)
+		goto unlock;
+
+	switch (dm_crtc_state->stream->timing.pixel_encoding) {
+	case PIXEL_ENCODING_RGB:
+		seq_puts(m, "RGB");
+		break;
+	case PIXEL_ENCODING_YCBCR422:
+		seq_puts(m, "YCBCR422");
+		break;
+	case PIXEL_ENCODING_YCBCR444:
+		seq_puts(m, "YCBCR444");
+		break;
+	case PIXEL_ENCODING_YCBCR420:
+		seq_puts(m, "YCBCR420");
+		break;
+	default:
+		goto unlock;
+	}
+	res = 0;
+
+unlock:
+	drm_modeset_unlock(&crtc->mutex);
+	mutex_unlock(&dev->mode_config.mutex);
+
+	return res;
+}
+DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
 
 /*
  * Example usage:
@@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
 			    crtc, &amdgpu_current_bpc_fops);
 	debugfs_create_file("amdgpu_current_colorspace", 0644, crtc->debugfs_entry,
 			    crtc, &amdgpu_current_colorspace_fops);
+	debugfs_create_file("amdgpu_current_pixelencoding", 0644, crtc->debugfs_entry,
+			    crtc, &amdgpu_current_pixelencoding_fops);
 }
 
 /*
-- 
2.45.1

Re: [PATCH] drm/amd/display: Add pixel encoding info to debugfs

[Mario Limonciello]

On 5/20/2024 16:07, Rino Andre Johnsen wrote:
> [Why]
> For debugging and testing purposes.
> 
> [How]
> Create amdgpu_current_pixelencoding debugfs entry.
> Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding
> 
> Signed-off-by: Rino Andre Johnsen <rinoandrejohnsen@gmail.com>
> ---
>   .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++++++++++++++++++
>   1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> index 27d5c6077630..d275e5522335 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> @@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct seq_file *m, void *data)
>   }
>   DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
>   
> +/*
> + * Returns the current pixelencoding for the crtc.
> + * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
> + */
> +static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
> +{
> +	struct drm_crtc *crtc = m->private;
> +	struct drm_device *dev = crtc->dev;
> +	struct dm_crtc_state *dm_crtc_state = NULL;

There is no need to initialize dm_crtc_state to NULL.  You set it before 
its first use.

> +	int res = -ENODEV;
> +
> +	mutex_lock(&dev->mode_config.mutex);
> +	drm_modeset_lock(&crtc->mutex, NULL);
> +	if (crtc->state == NULL)
> +		goto unlock;
> +
> +	dm_crtc_state = to_dm_crtc_state(crtc->state);
> +	if (dm_crtc_state->stream == NULL)
> +		goto unlock;
> +
> +	switch (dm_crtc_state->stream->timing.pixel_encoding) {
> +	case PIXEL_ENCODING_RGB:
> +		seq_puts(m, "RGB");
> +		break;
> +	case PIXEL_ENCODING_YCBCR422:
> +		seq_puts(m, "YCBCR422");
> +		break;
> +	case PIXEL_ENCODING_YCBCR444:
> +		seq_puts(m, "YCBCR444");
> +		break;
> +	case PIXEL_ENCODING_YCBCR420:
> +		seq_puts(m, "YCBCR420");
> +		break;
> +	default:
> +		goto unlock;
> +	}
> +	res = 0;
> +
> +unlock:
> +	drm_modeset_unlock(&crtc->mutex);
> +	mutex_unlock(&dev->mode_config.mutex);
> +
> +	return res;
> +}
> +DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
>   
>   /*
>    * Example usage:
> @@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
>   			    crtc, &amdgpu_current_bpc_fops);
>   	debugfs_create_file("amdgpu_current_colorspace", 0644, crtc->debugfs_entry,
>   			    crtc, &amdgpu_current_colorspace_fops);
> +	debugfs_create_file("amdgpu_current_pixelencoding", 0644, crtc->debugfs_entry,
> +			    crtc, &amdgpu_current_pixelencoding_fops);
>   }
>   
>   /*

[PATCH v2] drm/amd/display: Add pixel encoding info to debugfs

[Rino Andre Johnsen]

[Why]
For debugging and testing purposes.

[How]
Create amdgpu_current_pixelencoding debugfs entry.
Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding

Signed-off-by: Rino Andre Johnsen <rinoandrejohnsen@gmail.com>
---

Changes in v2:
1. Do not initialize dm_crtc_state to NULL.
---
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 27d5c6077630..4254d4a4b56b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct seq_file *m, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
 
+/*
+ * Returns the current pixelencoding for the crtc.
+ * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
+ */
+static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
+{
+	struct drm_crtc *crtc = m->private;
+	struct drm_device *dev = crtc->dev;
+	struct dm_crtc_state *dm_crtc_state;
+	int res = -ENODEV;
+
+	mutex_lock(&dev->mode_config.mutex);
+	drm_modeset_lock(&crtc->mutex, NULL);
+	if (crtc->state == NULL)
+		goto unlock;
+
+	dm_crtc_state = to_dm_crtc_state(crtc->state);
+	if (dm_crtc_state->stream == NULL)
+		goto unlock;
+
+	switch (dm_crtc_state->stream->timing.pixel_encoding) {
+	case PIXEL_ENCODING_RGB:
+		seq_puts(m, "RGB");
+		break;
+	case PIXEL_ENCODING_YCBCR422:
+		seq_puts(m, "YCBCR422");
+		break;
+	case PIXEL_ENCODING_YCBCR444:
+		seq_puts(m, "YCBCR444");
+		break;
+	case PIXEL_ENCODING_YCBCR420:
+		seq_puts(m, "YCBCR420");
+		break;
+	default:
+		goto unlock;
+	}
+	res = 0;
+
+unlock:
+	drm_modeset_unlock(&crtc->mutex);
+	mutex_unlock(&dev->mode_config.mutex);
+
+	return res;
+}
+DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
 
 /*
  * Example usage:
@@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
 			    crtc, &amdgpu_current_bpc_fops);
 	debugfs_create_file("amdgpu_current_colorspace", 0644, crtc->debugfs_entry,
 			    crtc, &amdgpu_current_colorspace_fops);
+	debugfs_create_file("amdgpu_current_pixelencoding", 0644, crtc->debugfs_entry,
+			    crtc, &amdgpu_current_pixelencoding_fops);
 }
 
 /*
-- 
2.45.1

Re: [PATCH v2] drm/amd/display: Add pixel encoding info to debugfs

[Christian König]

Am 21.05.24 um 07:11 schrieb Rino Andre Johnsen:
> [Why]
> For debugging and testing purposes.
>
> [How]
> Create amdgpu_current_pixelencoding debugfs entry.
> Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding

Why isn't that available as standard DRM CRTC property in either sysfs 
or debugfs?

I think the format specifiers should already be available somewhere there.

Regards,
Christian.

>
> Signed-off-by: Rino Andre Johnsen <rinoandrejohnsen@gmail.com>
> ---
>
> Changes in v2:
> 1. Do not initialize dm_crtc_state to NULL.
> ---
>   .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++++++++++++++++++
>   1 file changed, 47 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> index 27d5c6077630..4254d4a4b56b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> @@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct seq_file *m, void *data)
>   }
>   DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
>   
> +/*
> + * Returns the current pixelencoding for the crtc.
> + * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
> + */
> +static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
> +{
> +	struct drm_crtc *crtc = m->private;
> +	struct drm_device *dev = crtc->dev;
> +	struct dm_crtc_state *dm_crtc_state;
> +	int res = -ENODEV;
> +
> +	mutex_lock(&dev->mode_config.mutex);
> +	drm_modeset_lock(&crtc->mutex, NULL);
> +	if (crtc->state == NULL)
> +		goto unlock;
> +
> +	dm_crtc_state = to_dm_crtc_state(crtc->state);
> +	if (dm_crtc_state->stream == NULL)
> +		goto unlock;
> +
> +	switch (dm_crtc_state->stream->timing.pixel_encoding) {
> +	case PIXEL_ENCODING_RGB:
> +		seq_puts(m, "RGB");
> +		break;
> +	case PIXEL_ENCODING_YCBCR422:
> +		seq_puts(m, "YCBCR422");
> +		break;
> +	case PIXEL_ENCODING_YCBCR444:
> +		seq_puts(m, "YCBCR444");
> +		break;
> +	case PIXEL_ENCODING_YCBCR420:
> +		seq_puts(m, "YCBCR420");
> +		break;
> +	default:
> +		goto unlock;
> +	}
> +	res = 0;
> +
> +unlock:
> +	drm_modeset_unlock(&crtc->mutex);
> +	mutex_unlock(&dev->mode_config.mutex);
> +
> +	return res;
> +}
> +DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
>   
>   /*
>    * Example usage:
> @@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
>   			    crtc, &amdgpu_current_bpc_fops);
>   	debugfs_create_file("amdgpu_current_colorspace", 0644, crtc->debugfs_entry,
>   			    crtc, &amdgpu_current_colorspace_fops);
> +	debugfs_create_file("amdgpu_current_pixelencoding", 0644, crtc->debugfs_entry,
> +			    crtc, &amdgpu_current_pixelencoding_fops);
>   }
>   
>   /*

Re: [PATCH v2] drm/amd/display: Add pixel encoding info to debugfs

[Rino André Johnsen]

What is already there in debugfs is 'bpc' and 'colorspace', but not
the pixel encoding/format.
I have searched high and low for that to be able to verify that my
monitor and computer are using my preferred combination of all those
three values.

I do think it should be available as a standard DRM CRTC property, but
for the time being, I figured that a simple debugfs property would be
sufficient for time being.

Rino


On Tue, May 21, 2024 at 9:04 PM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 21.05.24 um 07:11 schrieb Rino Andre Johnsen:
> > [Why]
> > For debugging and testing purposes.
> >
> > [How]
> > Create amdgpu_current_pixelencoding debugfs entry.
> > Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding
>
> Why isn't that available as standard DRM CRTC property in either sysfs
> or debugfs?
>
> I think the format specifiers should already be available somewhere there.
>
> Regards,
> Christian.
>
> >
> > Signed-off-by: Rino Andre Johnsen <rinoandrejohnsen@gmail.com>
> > ---
> >
> > Changes in v2:
> > 1. Do not initialize dm_crtc_state to NULL.
> > ---
> >   .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++++++++++++++++++
> >   1 file changed, 47 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> > index 27d5c6077630..4254d4a4b56b 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> > @@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct seq_file *m, void *data)
> >   }
> >   DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
> >
> > +/*
> > + * Returns the current pixelencoding for the crtc.
> > + * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
> > + */
> > +static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
> > +{
> > +     struct drm_crtc *crtc = m->private;
> > +     struct drm_device *dev = crtc->dev;
> > +     struct dm_crtc_state *dm_crtc_state;
> > +     int res = -ENODEV;
> > +
> > +     mutex_lock(&dev->mode_config.mutex);
> > +     drm_modeset_lock(&crtc->mutex, NULL);
> > +     if (crtc->state == NULL)
> > +             goto unlock;
> > +
> > +     dm_crtc_state = to_dm_crtc_state(crtc->state);
> > +     if (dm_crtc_state->stream == NULL)
> > +             goto unlock;
> > +
> > +     switch (dm_crtc_state->stream->timing.pixel_encoding) {
> > +     case PIXEL_ENCODING_RGB:
> > +             seq_puts(m, "RGB");
> > +             break;
> > +     case PIXEL_ENCODING_YCBCR422:
> > +             seq_puts(m, "YCBCR422");
> > +             break;
> > +     case PIXEL_ENCODING_YCBCR444:
> > +             seq_puts(m, "YCBCR444");
> > +             break;
> > +     case PIXEL_ENCODING_YCBCR420:
> > +             seq_puts(m, "YCBCR420");
> > +             break;
> > +     default:
> > +             goto unlock;
> > +     }
> > +     res = 0;
> > +
> > +unlock:
> > +     drm_modeset_unlock(&crtc->mutex);
> > +     mutex_unlock(&dev->mode_config.mutex);
> > +
> > +     return res;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
> >
> >   /*
> >    * Example usage:
> > @@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
> >                           crtc, &amdgpu_current_bpc_fops);
> >       debugfs_create_file("amdgpu_current_colorspace", 0644, crtc->debugfs_entry,
> >                           crtc, &amdgpu_current_colorspace_fops);
> > +     debugfs_create_file("amdgpu_current_pixelencoding", 0644, crtc->debugfs_entry,
> > +                         crtc, &amdgpu_current_pixelencoding_fops);
> >   }
> >
> >   /*
>

Re: [PATCH v2] drm/amd/display: Add pixel encoding info to debugfs

[Mario Limonciello]

On 5/21/2024 15:06, Rino André Johnsen wrote:
> What is already there in debugfs is 'bpc' and 'colorspace', but not
> the pixel encoding/format.
> I have searched high and low for that to be able to verify that my
> monitor and computer are using my preferred combination of all those
> three values.
> 
> I do think it should be available as a standard DRM CRTC property, but
> for the time being, I figured that a simple debugfs property would be
> sufficient for time being.
> 

It's just about as much work either way to populate it though, why do it 
twice instead of just doing it right the first time?

> Rino
> 
> 
> On Tue, May 21, 2024 at 9:04 PM Christian König
> <christian.koenig@amd.com> wrote:
>>
>> Am 21.05.24 um 07:11 schrieb Rino Andre Johnsen:
>>> [Why]
>>> For debugging and testing purposes.
>>>
>>> [How]
>>> Create amdgpu_current_pixelencoding debugfs entry.
>>> Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding
>>
>> Why isn't that available as standard DRM CRTC property in either sysfs
>> or debugfs?
>>
>> I think the format specifiers should already be available somewhere there.
>>
>> Regards,
>> Christian.
>>
>>>
>>> Signed-off-by: Rino Andre Johnsen <rinoandrejohnsen@gmail.com>
>>> ---
>>>
>>> Changes in v2:
>>> 1. Do not initialize dm_crtc_state to NULL.
>>> ---
>>>    .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++++++++++++++++++
>>>    1 file changed, 47 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>>> index 27d5c6077630..4254d4a4b56b 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>>> @@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct seq_file *m, void *data)
>>>    }
>>>    DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
>>>
>>> +/*
>>> + * Returns the current pixelencoding for the crtc.
>>> + * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
>>> + */
>>> +static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
>>> +{
>>> +     struct drm_crtc *crtc = m->private;
>>> +     struct drm_device *dev = crtc->dev;
>>> +     struct dm_crtc_state *dm_crtc_state;
>>> +     int res = -ENODEV;
>>> +
>>> +     mutex_lock(&dev->mode_config.mutex);
>>> +     drm_modeset_lock(&crtc->mutex, NULL);
>>> +     if (crtc->state == NULL)
>>> +             goto unlock;
>>> +
>>> +     dm_crtc_state = to_dm_crtc_state(crtc->state);
>>> +     if (dm_crtc_state->stream == NULL)
>>> +             goto unlock;
>>> +
>>> +     switch (dm_crtc_state->stream->timing.pixel_encoding) {
>>> +     case PIXEL_ENCODING_RGB:
>>> +             seq_puts(m, "RGB");
>>> +             break;
>>> +     case PIXEL_ENCODING_YCBCR422:
>>> +             seq_puts(m, "YCBCR422");
>>> +             break;
>>> +     case PIXEL_ENCODING_YCBCR444:
>>> +             seq_puts(m, "YCBCR444");
>>> +             break;
>>> +     case PIXEL_ENCODING_YCBCR420:
>>> +             seq_puts(m, "YCBCR420");
>>> +             break;
>>> +     default:
>>> +             goto unlock;
>>> +     }
>>> +     res = 0;
>>> +
>>> +unlock:
>>> +     drm_modeset_unlock(&crtc->mutex);
>>> +     mutex_unlock(&dev->mode_config.mutex);
>>> +
>>> +     return res;
>>> +}
>>> +DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
>>>
>>>    /*
>>>     * Example usage:
>>> @@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
>>>                            crtc, &amdgpu_current_bpc_fops);
>>>        debugfs_create_file("amdgpu_current_colorspace", 0644, crtc->debugfs_entry,
>>>                            crtc, &amdgpu_current_colorspace_fops);
>>> +     debugfs_create_file("amdgpu_current_pixelencoding", 0644, crtc->debugfs_entry,
>>> +                         crtc, &amdgpu_current_pixelencoding_fops);
>>>    }
>>>
>>>    /*
>>

Re: [PATCH v2] drm/amd/display: Add pixel encoding info to debugfs

[Rino André Johnsen]

To be perfectly honest with you, I haven't given that much though. I
used the 'bpc' and 'colorspace' property in debugfs, since I could not
find that information anywhere else. And since I also needed to verify
the pixel encoding being used, I added it where those other values
were. That made for a simple and easy addition for this property.

If you want me to do this differently, let me know. And please point
me to the standardized DRM property where I should expose the values.

Rino

On Tue, May 21, 2024 at 10:55 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> On 5/21/2024 15:06, Rino André Johnsen wrote:
> > What is already there in debugfs is 'bpc' and 'colorspace', but not
> > the pixel encoding/format.
> > I have searched high and low for that to be able to verify that my
> > monitor and computer are using my preferred combination of all those
> > three values.
> >
> > I do think it should be available as a standard DRM CRTC property, but
> > for the time being, I figured that a simple debugfs property would be
> > sufficient for time being.
> >
>
> It's just about as much work either way to populate it though, why do it
> twice instead of just doing it right the first time?
>
> > Rino
> >
> >
> > On Tue, May 21, 2024 at 9:04 PM Christian König
> > <christian.koenig@amd.com> wrote:
> >>
> >> Am 21.05.24 um 07:11 schrieb Rino Andre Johnsen:
> >>> [Why]
> >>> For debugging and testing purposes.
> >>>
> >>> [How]
> >>> Create amdgpu_current_pixelencoding debugfs entry.
> >>> Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding
> >>
> >> Why isn't that available as standard DRM CRTC property in either sysfs
> >> or debugfs?
> >>
> >> I think the format specifiers should already be available somewhere there.
> >>
> >> Regards,
> >> Christian.
> >>
> >>>
> >>> Signed-off-by: Rino Andre Johnsen <rinoandrejohnsen@gmail.com>
> >>> ---
> >>>
> >>> Changes in v2:
> >>> 1. Do not initialize dm_crtc_state to NULL.
> >>> ---
> >>>    .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++++++++++++++++++
> >>>    1 file changed, 47 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> >>> index 27d5c6077630..4254d4a4b56b 100644
> >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> >>> @@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct seq_file *m, void *data)
> >>>    }
> >>>    DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
> >>>
> >>> +/*
> >>> + * Returns the current pixelencoding for the crtc.
> >>> + * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
> >>> + */
> >>> +static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
> >>> +{
> >>> +     struct drm_crtc *crtc = m->private;
> >>> +     struct drm_device *dev = crtc->dev;
> >>> +     struct dm_crtc_state *dm_crtc_state;
> >>> +     int res = -ENODEV;
> >>> +
> >>> +     mutex_lock(&dev->mode_config.mutex);
> >>> +     drm_modeset_lock(&crtc->mutex, NULL);
> >>> +     if (crtc->state == NULL)
> >>> +             goto unlock;
> >>> +
> >>> +     dm_crtc_state = to_dm_crtc_state(crtc->state);
> >>> +     if (dm_crtc_state->stream == NULL)
> >>> +             goto unlock;
> >>> +
> >>> +     switch (dm_crtc_state->stream->timing.pixel_encoding) {
> >>> +     case PIXEL_ENCODING_RGB:
> >>> +             seq_puts(m, "RGB");
> >>> +             break;
> >>> +     case PIXEL_ENCODING_YCBCR422:
> >>> +             seq_puts(m, "YCBCR422");
> >>> +             break;
> >>> +     case PIXEL_ENCODING_YCBCR444:
> >>> +             seq_puts(m, "YCBCR444");
> >>> +             break;
> >>> +     case PIXEL_ENCODING_YCBCR420:
> >>> +             seq_puts(m, "YCBCR420");
> >>> +             break;
> >>> +     default:
> >>> +             goto unlock;
> >>> +     }
> >>> +     res = 0;
> >>> +
> >>> +unlock:
> >>> +     drm_modeset_unlock(&crtc->mutex);
> >>> +     mutex_unlock(&dev->mode_config.mutex);
> >>> +
> >>> +     return res;
> >>> +}
> >>> +DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
> >>>
> >>>    /*
> >>>     * Example usage:
> >>> @@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
> >>>                            crtc, &amdgpu_current_bpc_fops);
> >>>        debugfs_create_file("amdgpu_current_colorspace", 0644, crtc->debugfs_entry,
> >>>                            crtc, &amdgpu_current_colorspace_fops);
> >>> +     debugfs_create_file("amdgpu_current_pixelencoding", 0644, crtc->debugfs_entry,
> >>> +                         crtc, &amdgpu_current_pixelencoding_fops);
> >>>    }
> >>>
> >>>    /*
> >>
>

Re: [PATCH v2] drm/amd/display: Add pixel encoding info to debugfs

[Mario Limonciello]

+ Simon

On 5/22/2024 05:07, Rino André Johnsen wrote:
> To be perfectly honest with you, I haven't given that much though. I
> used the 'bpc' and 'colorspace' property in debugfs, since I could not
> find that information anywhere else. And since I also needed to verify
> the pixel encoding being used, I added it where those other values
> were. That made for a simple and easy addition for this property.
> 
> If you want me to do this differently, let me know. And please point
> me to the standardized DRM property where I should expose the values.

Here's a pointer to where the colorspace property is created:

https://github.com/torvalds/linux/blob/v6.9/drivers/gpu/drm/drm_connector.c#L2147

I would expect you can make another property for the information you're 
looking for and then can get it from userspace using standard property
APIs.

> 
> Rino
> 
> On Tue, May 21, 2024 at 10:55 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>>
>> On 5/21/2024 15:06, Rino André Johnsen wrote:
>>> What is already there in debugfs is 'bpc' and 'colorspace', but not
>>> the pixel encoding/format.
>>> I have searched high and low for that to be able to verify that my
>>> monitor and computer are using my preferred combination of all those
>>> three values.
>>>
>>> I do think it should be available as a standard DRM CRTC property, but
>>> for the time being, I figured that a simple debugfs property would be
>>> sufficient for time being.
>>>
>>
>> It's just about as much work either way to populate it though, why do it
>> twice instead of just doing it right the first time?
>>
>>> Rino
>>>
>>>
>>> On Tue, May 21, 2024 at 9:04 PM Christian König
>>> <christian.koenig@amd.com> wrote:
>>>>
>>>> Am 21.05.24 um 07:11 schrieb Rino Andre Johnsen:
>>>>> [Why]
>>>>> For debugging and testing purposes.
>>>>>
>>>>> [How]
>>>>> Create amdgpu_current_pixelencoding debugfs entry.
>>>>> Usage: cat /sys/kernel/debug/dri/1/crtc-0/amdgpu_current_pixelencoding
>>>>
>>>> Why isn't that available as standard DRM CRTC property in either sysfs
>>>> or debugfs?
>>>>
>>>> I think the format specifiers should already be available somewhere there.
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> Signed-off-by: Rino Andre Johnsen <rinoandrejohnsen@gmail.com>
>>>>> ---
>>>>>
>>>>> Changes in v2:
>>>>> 1. Do not initialize dm_crtc_state to NULL.
>>>>> ---
>>>>>     .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 47 +++++++++++++++++++
>>>>>     1 file changed, 47 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>>>>> index 27d5c6077630..4254d4a4b56b 100644
>>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>>>>> @@ -1160,6 +1160,51 @@ static int amdgpu_current_colorspace_show(struct seq_file *m, void *data)
>>>>>     }
>>>>>     DEFINE_SHOW_ATTRIBUTE(amdgpu_current_colorspace);
>>>>>
>>>>> +/*
>>>>> + * Returns the current pixelencoding for the crtc.
>>>>> + * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/amdgpu_current_pixelencoding
>>>>> + */
>>>>> +static int amdgpu_current_pixelencoding_show(struct seq_file *m, void *data)
>>>>> +{
>>>>> +     struct drm_crtc *crtc = m->private;
>>>>> +     struct drm_device *dev = crtc->dev;
>>>>> +     struct dm_crtc_state *dm_crtc_state;
>>>>> +     int res = -ENODEV;
>>>>> +
>>>>> +     mutex_lock(&dev->mode_config.mutex);
>>>>> +     drm_modeset_lock(&crtc->mutex, NULL);
>>>>> +     if (crtc->state == NULL)
>>>>> +             goto unlock;
>>>>> +
>>>>> +     dm_crtc_state = to_dm_crtc_state(crtc->state);
>>>>> +     if (dm_crtc_state->stream == NULL)
>>>>> +             goto unlock;
>>>>> +
>>>>> +     switch (dm_crtc_state->stream->timing.pixel_encoding) {
>>>>> +     case PIXEL_ENCODING_RGB:
>>>>> +             seq_puts(m, "RGB");
>>>>> +             break;
>>>>> +     case PIXEL_ENCODING_YCBCR422:
>>>>> +             seq_puts(m, "YCBCR422");
>>>>> +             break;
>>>>> +     case PIXEL_ENCODING_YCBCR444:
>>>>> +             seq_puts(m, "YCBCR444");
>>>>> +             break;
>>>>> +     case PIXEL_ENCODING_YCBCR420:
>>>>> +             seq_puts(m, "YCBCR420");
>>>>> +             break;
>>>>> +     default:
>>>>> +             goto unlock;
>>>>> +     }
>>>>> +     res = 0;
>>>>> +
>>>>> +unlock:
>>>>> +     drm_modeset_unlock(&crtc->mutex);
>>>>> +     mutex_unlock(&dev->mode_config.mutex);
>>>>> +
>>>>> +     return res;
>>>>> +}
>>>>> +DEFINE_SHOW_ATTRIBUTE(amdgpu_current_pixelencoding);
>>>>>
>>>>>     /*
>>>>>      * Example usage:
>>>>> @@ -3688,6 +3733,8 @@ void crtc_debugfs_init(struct drm_crtc *crtc)
>>>>>                             crtc, &amdgpu_current_bpc_fops);
>>>>>         debugfs_create_file("amdgpu_current_colorspace", 0644, crtc->debugfs_entry,
>>>>>                             crtc, &amdgpu_current_colorspace_fops);
>>>>> +     debugfs_create_file("amdgpu_current_pixelencoding", 0644, crtc->debugfs_entry,
>>>>> +                         crtc, &amdgpu_current_pixelencoding_fops);
>>>>>     }
>>>>>
>>>>>     /*
>>>>
>>

Re: [PATCH v2] drm/amd/display: Add pixel encoding info to debugfs

[Simon Ser]

On Wednesday, May 22nd, 2024 at 15:36, Mario Limonciello <mario.limonciello@amd.com> wrote:

> > To be perfectly honest with you, I haven't given that much though. I
> > used the 'bpc' and 'colorspace' property in debugfs, since I could not
> > find that information anywhere else. And since I also needed to verify
> > the pixel encoding being used, I added it where those other values
> > were. That made for a simple and easy addition for this property.
> > 
> > If you want me to do this differently, let me know. And please point
> > me to the standardized DRM property where I should expose the values.

FWIW, there is a patch from Andri to add a similar (?) property:
https://lore.kernel.org/dri-devel/20240115160554.720247-1-andri@yngvason.is/

The patch also allows user-space to set the "pixel encoding".

Re: [PATCH v2] drm/amd/display: Add pixel encoding info to debugfs

[Rino André Johnsen]

Looked through the patch series from Andri Yngvason and that does
exactly what I wanted in the first place. I think that the patch
series should be encouraged to be merged in as fast as possible.

For the patch I have submitted, it stands on its own, since the patch
series from Andri Yngvason does not include anything in the debugfs.
This means whenever or not the patch series gets merged, at least
those with an AMD gpu can figure out which pixel encoding that is
used.

Rino

On Wed, May 22, 2024 at 3:40 PM Simon Ser <contact@emersion.fr> wrote:
>
> On Wednesday, May 22nd, 2024 at 15:36, Mario Limonciello <mario.limonciello@amd.com> wrote:
>
> > > To be perfectly honest with you, I haven't given that much though. I
> > > used the 'bpc' and 'colorspace' property in debugfs, since I could not
> > > find that information anywhere else. And since I also needed to verify
> > > the pixel encoding being used, I added it where those other values
> > > were. That made for a simple and easy addition for this property.
> > >
> > > If you want me to do this differently, let me know. And please point
> > > me to the standardized DRM property where I should expose the values.
>
> FWIW, there is a patch from Andri to add a similar (?) property:
> https://lore.kernel.org/dri-devel/20240115160554.720247-1-andri@yngvason.is/
>
> The patch also allows user-space to set the "pixel encoding".