Re: [PATCH 06/11] drm: create hdmi output property

2017-04-19 Thread Sharma, Shashank 

Hello Jose,

Sorry for delay in response, I was on vacation.
Please find my comments inline.

Regards
Shashank
On 4/12/2017 3:28 PM, Jose Abreu wrote:

Hi Shashank,


On 07-04-2017 17:39, Shashank Sharma wrote:

HDMI displays can support various output types, based on
the color space and subsampling type. The possible
outputs from a HDMI 2.0 monitor could be:
  - RGB
  - YCBCR 444
  - YCBCR 422
  - YCBCR 420

This patch adds a drm property, using which, a userspace
can specify its preference, for the HDMI output type.
The output type enums are similar to the mentioned outputs
above. To handle various subsampling of YCBCR output types,
this property allows two special cases:
  - DRM_HDMI_OUTPUT_YCBCR_HQ
This indicates preferred output should be YCBCR output, with highest
subsampling rate by the source/sink, which can be typically:
  - ycbcr444
  - ycbcr422
  - ycbcr420
  - DRM_HDMI_OUTPUT_YCBCR_LQ
This indicates preferred output should be YCBCR output, with lowest
subsampling rate supported by source/sink, which can be:
  - ycbcr420
  - ycbcr422
  - ycbcr444

Default value of the property is set to 0 = RGB, so no changes if you
dont set the property.

The HQ/LQ properties are a nice idea

Courtesy Ville :)

but where are you parsing
them (because in patch 07/11 you reject these properties)? Also,
you need to make sure that source and sink supports the color space.
Yes, and that's why its handling is kept in the core driver. If you see 
my I915 patches, I have parsed and handled the HQ and LQ properties in
intel_hdmi_compute_ycbcr_config function. I am rejecting the HQ/LQ in 
07/11, coz by the time we want to set AVI_IF the driver should have picked
the highest/lowest YCBCR mode, and now we should have a definite one out 
of 444,422 or 420.


Do you think it would be a good idea to find the HQ/LQ YCBCR output in 
DRM layer ? In that case it would be difficult if source supports these 
outputs


- Shashank

Best regards,
Jose Miguel Abreu


Cc: Ville Syrjala 
Cc: Jose Abreu 
Cc: Daniel Vetter 
Signed-off-by: Shashank Sharma 
---
  drivers/gpu/drm/drm_atomic.c|  2 ++
  drivers/gpu/drm/drm_atomic_helper.c |  4 
  drivers/gpu/drm/drm_connector.c | 31 +++
  include/drm/drm_connector.h | 14 ++
  include/drm/drm_mode_config.h   |  5 +
  5 files changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index f32506a..6ef34dc 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1123,6 +1123,8 @@ int drm_atomic_connector_set_property(struct 
drm_connector *connector,
 */
if (state->link_status != DRM_LINK_STATUS_GOOD)
state->link_status = val;
+   } else if (property == config->hdmi_output_property) {
+   state->hdmi_output = val;
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 8be9719..fcba3c0 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -567,6 +567,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
if (old_connector_state->link_status !=
new_connector_state->link_status)
new_crtc_state->connectors_changed = true;
+
+   if (old_connector_state->hdmi_output !=
+   new_connector_state->hdmi_output)
+   new_crtc_state->connectors_changed = true;
}
  
  		if (funcs->atomic_check)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 9f84761..10201b1 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -227,6 +227,11 @@ int drm_connector_init(struct drm_device *dev,
  config->edid_property,
  0);
  
+	if (connector_type != DRM_MODE_CONNECTOR_VIRTUAL)

+   drm_object_attach_property(>base,
+  config->hdmi_output_property,
+  0);
+
drm_object_attach_property(>base,
  config->dpms_property, 0);
  
@@ -617,6 +622,25 @@ static const struct drm_prop_enum_list drm_link_status_enum_list[] = {

  };
  DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
  
+static const struct drm_prop_enum_list drm_hdmi_output_enum_list[] = {

+   { DRM_HDMI_OUTPUT_DEFAULT_RGB, "output_rgb" },
+   { DRM_HDMI_OUTPUT_YCBCR422, "output_ycbcr422" },
+   {

Re: [PATCH 06/11] drm: create hdmi output property

2017-04-12 Thread Jose Abreu 
Hi Shashank,


On 07-04-2017 17:39, Shashank Sharma wrote:
> HDMI displays can support various output types, based on
> the color space and subsampling type. The possible
> outputs from a HDMI 2.0 monitor could be:
>  - RGB
>  - YCBCR 444
>  - YCBCR 422
>  - YCBCR 420
>
> This patch adds a drm property, using which, a userspace
> can specify its preference, for the HDMI output type.
> The output type enums are similar to the mentioned outputs
> above. To handle various subsampling of YCBCR output types,
> this property allows two special cases:
>  - DRM_HDMI_OUTPUT_YCBCR_HQ
> This indicates preferred output should be YCBCR output, with highest
> subsampling rate by the source/sink, which can be typically:
>  - ycbcr444
>  - ycbcr422
>  - ycbcr420
>  - DRM_HDMI_OUTPUT_YCBCR_LQ
> This indicates preferred output should be YCBCR output, with lowest
> subsampling rate supported by source/sink, which can be:
>  - ycbcr420
>  - ycbcr422
>  - ycbcr444
>
> Default value of the property is set to 0 = RGB, so no changes if you
> dont set the property.

The HQ/LQ properties are a nice idea but where are you parsing
them (because in patch 07/11 you reject these properties)? Also,
you need to make sure that source and sink supports the color space.

Best regards,
Jose Miguel Abreu

>
> Cc: Ville Syrjala 
> Cc: Jose Abreu 
> Cc: Daniel Vetter 
> Signed-off-by: Shashank Sharma 
> ---
>  drivers/gpu/drm/drm_atomic.c|  2 ++
>  drivers/gpu/drm/drm_atomic_helper.c |  4 
>  drivers/gpu/drm/drm_connector.c | 31 +++
>  include/drm/drm_connector.h | 14 ++
>  include/drm/drm_mode_config.h   |  5 +
>  5 files changed, 56 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index f32506a..6ef34dc 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1123,6 +1123,8 @@ int drm_atomic_connector_set_property(struct 
> drm_connector *connector,
>*/
>   if (state->link_status != DRM_LINK_STATUS_GOOD)
>   state->link_status = val;
> + } else if (property == config->hdmi_output_property) {
> + state->hdmi_output = val;
>   } else if (connector->funcs->atomic_set_property) {
>   return connector->funcs->atomic_set_property(connector,
>   state, property, val);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 8be9719..fcba3c0 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -567,6 +567,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>   if (old_connector_state->link_status !=
>   new_connector_state->link_status)
>   new_crtc_state->connectors_changed = true;
> +
> + if (old_connector_state->hdmi_output !=
> + new_connector_state->hdmi_output)
> + new_crtc_state->connectors_changed = true;
>   }
>  
>   if (funcs->atomic_check)
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 9f84761..10201b1 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -227,6 +227,11 @@ int drm_connector_init(struct drm_device *dev,
> config->edid_property,
> 0);
>  
> + if (connector_type != DRM_MODE_CONNECTOR_VIRTUAL)
> + drm_object_attach_property(>base,
> +config->hdmi_output_property,
> +0);
> +
>   drm_object_attach_property(>base,
> config->dpms_property, 0);
>  
> @@ -617,6 +622,25 @@ static const struct drm_prop_enum_list 
> drm_link_status_enum_list[] = {
>  };
>  DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
>  
> +static const struct drm_prop_enum_list drm_hdmi_output_enum_list[] = {
> + { DRM_HDMI_OUTPUT_DEFAULT_RGB, "output_rgb" },
> + { DRM_HDMI_OUTPUT_YCBCR422, "output_ycbcr422" },
> + { DRM_HDMI_OUTPUT_YCBCR444, "output_ycbcr444" },
> + { DRM_HDMI_OUTPUT_YCBCR420, "output_ycbcr420" },
> + { DRM_HDMI_OUTPUT_YCBCR_HQ, "output_ycbcr_high_subsampling" },
> + { DRM_HDMI_OUTPUT_YCBCR_LQ, "output_ycbcr_low_subsampling" },
> +};
> +
> +/**
> + * drm_get_hdmi_output_name - return a string for a given hdmi output enum
> + * @type: enum of output type
> + */
> +const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type)
> +{
> + return drm_hdmi_output_enum_list[type].name;
> +}
> +EXPORT_SYMBOL(drm_get_hdmi_output_name);
> +
>  /**
>   * drm_display_info_set_bus_formats -

Re: [Intel-gfx] [PATCH 06/11] drm: create hdmi output property

2017-04-08 Thread kbuild test robot 
Hi Shashank,

[auto build test WARNING on next-20170407]
[cannot apply to drm/drm-next drm-intel/for-linux-next tegra/for-next v4.9-rc8 
v4.9-rc7 v4.9-rc6 v4.11-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Shashank-Sharma/HDMI-YCBCR-output-handling-in-DRM-layer/20170408-190651
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick 
(https://www.imagemagick.org)
   arch/x86/include/asm/uaccess_32.h:1: warning: no structured comments found
   include/linux/init.h:1: warning: no structured comments found
   kernel/sched/core.c:2088: warning: No description found for parameter 'rf'
   kernel/sched/core.c:2088: warning: Excess function parameter 'cookie' 
description in 'try_to_wake_up_local'
   include/linux/kthread.h:26: warning: Excess function parameter '...' 
description in 'kthread_create'
   kernel/sys.c:1: warning: no structured comments found
   include/linux/device.h:969: warning: No description found for parameter 
'dma_ops'
   drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
   include/linux/iio/iio.h:597: warning: No description found for parameter 
'trig_readonly'
   include/linux/iio/trigger.h:151: warning: No description found for parameter 
'indio_dev'
   include/linux/iio/trigger.h:151: warning: No description found for parameter 
'trig'
   include/linux/device.h:970: warning: No description found for parameter 
'dma_ops'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'set_busid'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'irq_handler'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'irq_preinstall'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'irq_postinstall'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'irq_uninstall'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'debugfs_init'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'gem_open_object'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'gem_close_object'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'prime_handle_to_fd'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'prime_fd_to_handle'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'gem_prime_export'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'gem_prime_import'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'gem_prime_pin'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'gem_prime_unpin'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'gem_prime_res_obj'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'gem_prime_get_sg_table'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'gem_prime_import_sg_table'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'gem_prime_vmap'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'gem_prime_vunmap'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'gem_prime_mmap'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'gem_vm_ops'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'major'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'minor'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'patchlevel'
   include/drm/drm_drv.h:524: warning: No description found for parameter 'name'
   include/drm/drm_drv.h:524: warning: No description found for parameter 'desc'
   include/drm/drm_drv.h:524: warning: No description found for parameter 'date'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'driver_features'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'ioctls'
   include/drm/drm_drv.h:524: warning: No description found for parameter 
'num_ioctls'
   include/drm/drm_drv.h:524: warning: No description found for parameter 'fops'
   include/drm/drm_connector.h:142: warning: No description found for parameter 
'ycbcr420_dc_modes'
   include/drm/drm_connector.h:142: warning: No description found for parameter 
'ycbcr420_vcb_map'
>> include/drm/drm_connector.h:346: warning: No description found for parameter 
>> 'hdmi_output'
   include/drm/drm_color_mgmt.h:1: warning: no structured comments found
   drivers/gpu/drm/drm_plane_helper.c:403: warning: No description found for 
parameter 'ctx'
   drivers/gpu/drm/drm_plane_helper.c:404: warning: No description found for 
parameter 'ctx'
   drivers/gpu/drm/i915/intel_lpe_audio.c:343: warning:

[PATCH 06/11] drm: create hdmi output property

2017-04-07 Thread Shashank Sharma 
HDMI displays can support various output types, based on
the color space and subsampling type. The possible
outputs from a HDMI 2.0 monitor could be:
 - RGB
 - YCBCR 444
 - YCBCR 422
 - YCBCR 420

This patch adds a drm property, using which, a userspace
can specify its preference, for the HDMI output type.
The output type enums are similar to the mentioned outputs
above. To handle various subsampling of YCBCR output types,
this property allows two special cases:
 - DRM_HDMI_OUTPUT_YCBCR_HQ
This indicates preferred output should be YCBCR output, with highest
subsampling rate by the source/sink, which can be typically:
 - ycbcr444
 - ycbcr422
 - ycbcr420
 - DRM_HDMI_OUTPUT_YCBCR_LQ
This indicates preferred output should be YCBCR output, with lowest
subsampling rate supported by source/sink, which can be:
 - ycbcr420
 - ycbcr422
 - ycbcr444

Default value of the property is set to 0 = RGB, so no changes if you
dont set the property.

Cc: Ville Syrjala 
Cc: Jose Abreu 
Cc: Daniel Vetter 
Signed-off-by: Shashank Sharma 
---
 drivers/gpu/drm/drm_atomic.c|  2 ++
 drivers/gpu/drm/drm_atomic_helper.c |  4 
 drivers/gpu/drm/drm_connector.c | 31 +++
 include/drm/drm_connector.h | 14 ++
 include/drm/drm_mode_config.h   |  5 +
 5 files changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index f32506a..6ef34dc 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1123,6 +1123,8 @@ int drm_atomic_connector_set_property(struct 
drm_connector *connector,
 */
if (state->link_status != DRM_LINK_STATUS_GOOD)
state->link_status = val;
+   } else if (property == config->hdmi_output_property) {
+   state->hdmi_output = val;
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 8be9719..fcba3c0 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -567,6 +567,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
if (old_connector_state->link_status !=
new_connector_state->link_status)
new_crtc_state->connectors_changed = true;
+
+   if (old_connector_state->hdmi_output !=
+   new_connector_state->hdmi_output)
+   new_crtc_state->connectors_changed = true;
}
 
if (funcs->atomic_check)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 9f84761..10201b1 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -227,6 +227,11 @@ int drm_connector_init(struct drm_device *dev,
  config->edid_property,
  0);
 
+   if (connector_type != DRM_MODE_CONNECTOR_VIRTUAL)
+   drm_object_attach_property(>base,
+  config->hdmi_output_property,
+  0);
+
drm_object_attach_property(>base,
  config->dpms_property, 0);
 
@@ -617,6 +622,25 @@ static const struct drm_prop_enum_list 
drm_link_status_enum_list[] = {
 };
 DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
 
+static const struct drm_prop_enum_list drm_hdmi_output_enum_list[] = {
+   { DRM_HDMI_OUTPUT_DEFAULT_RGB, "output_rgb" },
+   { DRM_HDMI_OUTPUT_YCBCR422, "output_ycbcr422" },
+   { DRM_HDMI_OUTPUT_YCBCR444, "output_ycbcr444" },
+   { DRM_HDMI_OUTPUT_YCBCR420, "output_ycbcr420" },
+   { DRM_HDMI_OUTPUT_YCBCR_HQ, "output_ycbcr_high_subsampling" },
+   { DRM_HDMI_OUTPUT_YCBCR_LQ, "output_ycbcr_low_subsampling" },
+};
+
+/**
+ * drm_get_hdmi_output_name - return a string for a given hdmi output enum
+ * @type: enum of output type
+ */
+const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type)
+{
+   return drm_hdmi_output_enum_list[type].name;
+}
+EXPORT_SYMBOL(drm_get_hdmi_output_name);
+
 /**
  * drm_display_info_set_bus_formats - set the supported bus formats
  * @info: display info to store bus formats in
@@ -789,6 +813,13 @@ int drm_connector_create_standard_properties(struct 
drm_device *dev)
return -ENOMEM;
dev->mode_config.link_status_property = prop;
 
+   prop = drm_property_create_enum(dev, 0, "hdmi_output_format",
+   drm_hdmi_output_enum_list,
+