public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v3 1/2] drm: drm/i915: Add connector property to limit max bpc
@ 2018-08-25  1:02 Radhakrishna Sripada
  2018-08-25  1:02 ` [PATCH v3 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp Radhakrishna Sripada
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Radhakrishna Sripada @ 2018-08-25  1:02 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel, Kishore Kadiyala, Rodrigo Vivi

At times 12bpc HDMI cannot be driven due to faulty cables, dongles
level shifters etc. To workaround them we may need to drive the output
at a lower bpc. Currently the user space does not have a way to limit
the bpc. The default bpc to be programmed is decided by the driver and
is run against connector limitations.

Creating a new connector property "max bpc" in order to limit the bpc
with which the pixels are scanned out. xrandr can make use of this
connector property to make sure that bpc does not exceed the configured value.
This property can be used by userspace to set the bpc.

V2: Initialize max_bpc to satisfy kms_properties
V3: Move the property to drm_connector

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Kishore Kadiyala <kishore.kadiyala@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/drm_atomic.c        |  4 ++++
 drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
 drivers/gpu/drm/i915/intel_drv.h    |  2 ++
 drivers/gpu/drm/i915/intel_hdmi.c   | 11 +++++++++++
 drivers/gpu/drm/i915/intel_modes.c  | 20 ++++++++++++++++++++
 include/drm/drm_connector.h         |  6 ++++++
 include/drm/drm_mode_config.h       |  5 +++++
 7 files changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3eb061e11e2e..461dde0c2c10 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1416,6 +1416,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 
 		return set_out_fence_for_connector(state->state, connector,
 						   fence_ptr);
+	} else if (property == config->max_bpc_property) {
+		state->max_bpc = val;
 	} else if (connector->funcs->atomic_set_property) {
 		return connector->funcs->atomic_set_property(connector,
 				state, property, val);
@@ -1511,6 +1513,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = 0;
 	} else if (property == config->writeback_out_fence_ptr_property) {
 		*val = 0;
+	} else if (property == config->max_bpc_property) {
+		*val = state->max_bpc;
 	} else if (connector->funcs->atomic_get_property) {
 		return connector->funcs->atomic_get_property(connector,
 				state, property, val);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 38ce9a375ffb..82caac8d1432 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -638,6 +638,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->max_bpc !=
+			    new_connector_state->max_bpc)
+				new_crtc_state->connectors_changed = true;
 		}
 
 		if (funcs->atomic_check)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1b78de838c18..209eb1798238 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1862,6 +1862,8 @@ int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
 void intel_attach_force_audio_property(struct drm_connector *connector);
 void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
 void intel_attach_aspect_ratio_property(struct drm_connector *connector);
+void intel_attach_max_bpc_property(struct drm_connector *connector, int min, int
+				   max);
 
 
 /* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index a1799b5c12bb..82739f342246 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2097,11 +2097,22 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
 static void
 intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
 {
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
+
 	intel_attach_force_audio_property(connector);
 	intel_attach_broadcast_rgb_property(connector);
 	intel_attach_aspect_ratio_property(connector);
 	drm_connector_attach_content_type_property(connector);
 	connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+
+	if (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
+	    IS_CHERRYVIEW(dev_priv)) {
+		intel_attach_max_bpc_property(connector, 8, 10);
+		connector->state->max_bpc = 10;
+	} else if (INTEL_GEN(dev_priv) >= 5) {
+		intel_attach_max_bpc_property(connector, 8, 12);
+		connector->state->max_bpc = 12;
+	}
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
index ca44bf368e24..78f2ce92f194 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -133,3 +133,23 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector)
 			connector->dev->mode_config.aspect_ratio_property,
 			DRM_MODE_PICTURE_ASPECT_NONE);
 }
+
+void
+intel_attach_max_bpc_property(struct drm_connector *connector, int min, int
+			       max)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_property *prop;
+
+	prop = config->max_bpc_property;
+	if (prop == NULL) {
+		prop = drm_property_create_range(dev, 0, "max bpc", min, max);
+		if (prop == NULL)
+			return;
+
+		config->max_bpc_property = prop;
+	}
+
+	drm_object_attach_property(&connector->base, prop, max);
+}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 97ea41dc678f..d59b9e34ae80 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -460,6 +460,12 @@ struct drm_connector_state {
 	 * drm_writeback_signal_completion()
 	 */
 	struct drm_writeback_job *writeback_job;
+
+	/**
+	 * @max_bpc: Connector property to limit the maximum bit depth of
+	 * the pixels being scanned out.
+	 */
+	unsigned int max_bpc;
 };
 
 /**
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index a0b202e1d69a..b9cd7a73b244 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -562,6 +562,11 @@ struct drm_mode_config {
 	 */
 	struct drm_property *link_status_property;
 	/**
+	 * @max_bpc_property: Default connector property for the max bpc to be
+	 * driven out of the connector.
+	 */
+	struct drm_property *max_bpc_property;
+	/**
 	 * @plane_type_property: Default plane property to differentiate
 	 * CURSOR, PRIMARY and OVERLAY legacy uses of planes.
 	 */
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp
  2018-08-25  1:02 [PATCH v3 1/2] drm: drm/i915: Add connector property to limit max bpc Radhakrishna Sripada
@ 2018-08-25  1:02 ` Radhakrishna Sripada
  2018-08-25  1:09 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/2] drm: drm/i915: Add connector property to limit max bpc Patchwork
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Radhakrishna Sripada @ 2018-08-25  1:02 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel, Kishore Kadiyala, Rodrigo Vivi

From: "Sripada, Radhakrishna" <radhakrishna.sripada@intel.com>

Use the newly added "max bpc" connector property to limit pipe bpp.

V3: Use drm_connector_state to access the "max bpc" property

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Kishore Kadiyala <kishore.kadiyala@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 30fdfd1a3037..8506cd1634fb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10784,6 +10784,28 @@ connected_sink_compute_bpp(struct intel_connector *connector,
 	}
 }
 
+static void
+connected_sink_max_bpp(struct drm_connector_state *conn_state,
+			     struct intel_crtc_state *pipe_config)
+{
+	switch (conn_state->max_bpc) {
+	case 8:
+	case 9:
+		pipe_config->pipe_bpp = 8*3;
+		break;
+	case 10:
+	case 11:
+		pipe_config->pipe_bpp = 10*3;
+		break;
+	case 12:
+		pipe_config->pipe_bpp = 12*3;
+		break;
+	default:
+		break;
+	}
+	DRM_DEBUG_KMS("Limiting display bpp to %d\n", pipe_config->pipe_bpp);
+}
+
 static int
 compute_baseline_pipe_bpp(struct intel_crtc *crtc,
 			  struct intel_crtc_state *pipe_config)
@@ -10812,6 +10834,9 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
 		if (connector_state->crtc != &crtc->base)
 			continue;
 
+		if (connector_state->max_bpc)
+			connected_sink_max_bpp(connector_state, pipe_config);
+
 		connected_sink_compute_bpp(to_intel_connector(connector),
 					   pipe_config);
 	}
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/2] drm: drm/i915: Add connector property to limit max bpc
  2018-08-25  1:02 [PATCH v3 1/2] drm: drm/i915: Add connector property to limit max bpc Radhakrishna Sripada
  2018-08-25  1:02 ` [PATCH v3 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp Radhakrishna Sripada
@ 2018-08-25  1:09 ` Patchwork
  2018-08-25  1:29 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-08-25  1:09 UTC (permalink / raw)
  To: Radhakrishna Sripada; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v3,1/2] drm: drm/i915: Add connector property to limit max bpc
URL   : https://patchwork.freedesktop.org/series/48695/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
cf2501ba38fa drm: drm/i915: Add connector property to limit max bpc
-:17: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#17: 
connector property to make sure that bpc does not exceed the configured value.

-:125: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!prop"
#125: FILE: drivers/gpu/drm/i915/intel_modes.c:146:
+	if (prop == NULL) {

-:127: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!prop"
#127: FILE: drivers/gpu/drm/i915/intel_modes.c:148:
+		if (prop == NULL)

total: 0 errors, 1 warnings, 2 checks, 102 lines checked
7855b6acf894 drm/i915: Allow "max bpc" property to limit pipe_bpp
-:30: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#30: FILE: drivers/gpu/drm/i915/intel_display.c:10789:
+connected_sink_max_bpp(struct drm_connector_state *conn_state,
+			     struct intel_crtc_state *pipe_config)

-:35: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#35: FILE: drivers/gpu/drm/i915/intel_display.c:10794:
+		pipe_config->pipe_bpp = 8*3;
 		                         ^

-:39: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#39: FILE: drivers/gpu/drm/i915/intel_display.c:10798:
+		pipe_config->pipe_bpp = 10*3;
 		                          ^

-:42: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#42: FILE: drivers/gpu/drm/i915/intel_display.c:10801:
+		pipe_config->pipe_bpp = 12*3;
 		                          ^

total: 0 errors, 0 warnings, 4 checks, 37 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [v3,1/2] drm: drm/i915: Add connector property to limit max bpc
  2018-08-25  1:02 [PATCH v3 1/2] drm: drm/i915: Add connector property to limit max bpc Radhakrishna Sripada
  2018-08-25  1:02 ` [PATCH v3 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp Radhakrishna Sripada
  2018-08-25  1:09 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/2] drm: drm/i915: Add connector property to limit max bpc Patchwork
@ 2018-08-25  1:29 ` Patchwork
  2018-08-25  2:20 ` ✓ Fi.CI.IGT: " Patchwork
  2018-08-27 13:31 ` [PATCH v3 1/2] " Ville Syrjälä
  4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-08-25  1:29 UTC (permalink / raw)
  To: Radhakrishna Sripada; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v3,1/2] drm: drm/i915: Add connector property to limit max bpc
URL   : https://patchwork.freedesktop.org/series/48695/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4704 -> Patchwork_10015 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/48695/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10015:

  === IGT changes ===

    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-WARN -> DMESG-FAIL

    
== Known issues ==

  Here are the changes found in Patchwork_10015 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-kbl-guc:         PASS -> DMESG-FAIL (fdo#106947)

    igt@kms_chamelium@dp-edid-read:
      fi-kbl-7500u:       PASS -> FAIL (fdo#103841)

    igt@kms_frontbuffer_tracking@basic:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#107362)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#107362, fdo#103191)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
      fi-skl-guc:         FAIL (fdo#103191) -> PASS +1
      {fi-byt-clapper}:   FAIL (fdo#107362, fdo#103191) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-skl-6260u:       INCOMPLETE (fdo#104108) -> PASS

    {igt@pm_rpm@module-reload}:
      fi-cnl-psr:         WARN (fdo#107602) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602


== Participating hosts (54 -> 49) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4704 -> Patchwork_10015

  CI_DRM_4704: eb9a20b20d4790be1561235f45e209fba02dc6c0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10015: 7855b6acf8946464612d3eb13a6a65aade8612dd @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

7855b6acf894 drm/i915: Allow "max bpc" property to limit pipe_bpp
cf2501ba38fa drm: drm/i915: Add connector property to limit max bpc

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10015/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [v3,1/2] drm: drm/i915: Add connector property to limit max bpc
  2018-08-25  1:02 [PATCH v3 1/2] drm: drm/i915: Add connector property to limit max bpc Radhakrishna Sripada
                   ` (2 preceding siblings ...)
  2018-08-25  1:29 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-08-25  2:20 ` Patchwork
  2018-08-27 13:31 ` [PATCH v3 1/2] " Ville Syrjälä
  4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-08-25  2:20 UTC (permalink / raw)
  To: Radhakrishna Sripada; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v3,1/2] drm: drm/i915: Add connector property to limit max bpc
URL   : https://patchwork.freedesktop.org/series/48695/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4704_full -> Patchwork_10015_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

  Here are the changes found in Patchwork_10015_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ctx_isolation@bcs0-s3:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    igt@gem_exec_await@wide-contexts:
      shard-kbl:          PASS -> FAIL (fdo#105900)

    igt@gem_exec_big:
      shard-hsw:          PASS -> INCOMPLETE (fdo#103540)

    
    ==== Possible fixes ====

    igt@kms_cursor_legacy@cursor-vs-flip-toggle:
      shard-hsw:          FAIL (fdo#103355) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    igt@kms_vblank@pipe-a-ts-continuation-suspend:
      shard-hsw:          FAIL (fdo#104894) -> PASS

    
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#104894 https://bugs.freedesktop.org/show_bug.cgi?id=104894
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4704 -> Patchwork_10015

  CI_DRM_4704: eb9a20b20d4790be1561235f45e209fba02dc6c0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10015: 7855b6acf8946464612d3eb13a6a65aade8612dd @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10015/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] drm: drm/i915: Add connector property to limit max bpc
  2018-08-25  1:02 [PATCH v3 1/2] drm: drm/i915: Add connector property to limit max bpc Radhakrishna Sripada
                   ` (3 preceding siblings ...)
  2018-08-25  2:20 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-08-27 13:31 ` Ville Syrjälä
  2018-08-29 22:57   ` Radhakrishna Sripada
  4 siblings, 1 reply; 8+ messages in thread
From: Ville Syrjälä @ 2018-08-27 13:31 UTC (permalink / raw)
  To: Radhakrishna Sripada; +Cc: dri-devel, intel-gfx, Kishore Kadiyala, Rodrigo Vivi

On Fri, Aug 24, 2018 at 06:02:16PM -0700, Radhakrishna Sripada wrote:
> At times 12bpc HDMI cannot be driven due to faulty cables, dongles
> level shifters etc. To workaround them we may need to drive the output
> at a lower bpc. Currently the user space does not have a way to limit
> the bpc. The default bpc to be programmed is decided by the driver and
> is run against connector limitations.
> 
> Creating a new connector property "max bpc" in order to limit the bpc
> with which the pixels are scanned out. xrandr can make use of this
> connector property to make sure that bpc does not exceed the configured value.
> This property can be used by userspace to set the bpc.
> 
> V2: Initialize max_bpc to satisfy kms_properties
> V3: Move the property to drm_connector
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Kishore Kadiyala <kishore.kadiyala@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c        |  4 ++++
>  drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
>  drivers/gpu/drm/i915/intel_hdmi.c   | 11 +++++++++++
>  drivers/gpu/drm/i915/intel_modes.c  | 20 ++++++++++++++++++++

Pls move all the i915 stuff to the second patch.

>  include/drm/drm_connector.h         |  6 ++++++
>  include/drm/drm_mode_config.h       |  5 +++++
>  7 files changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3eb061e11e2e..461dde0c2c10 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1416,6 +1416,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  
>  		return set_out_fence_for_connector(state->state, connector,
>  						   fence_ptr);
> +	} else if (property == config->max_bpc_property) {
> +		state->max_bpc = val;
>  	} else if (connector->funcs->atomic_set_property) {
>  		return connector->funcs->atomic_set_property(connector,
>  				state, property, val);
> @@ -1511,6 +1513,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>  		*val = 0;
>  	} else if (property == config->writeback_out_fence_ptr_property) {
>  		*val = 0;
> +	} else if (property == config->max_bpc_property) {
> +		*val = state->max_bpc;
>  	} else if (connector->funcs->atomic_get_property) {
>  		return connector->funcs->atomic_get_property(connector,
>  				state, property, val);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 38ce9a375ffb..82caac8d1432 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -638,6 +638,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->max_bpc !=
> +			    new_connector_state->max_bpc)
> +				new_crtc_state->connectors_changed = true;
>  		}
>  
>  		if (funcs->atomic_check)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1b78de838c18..209eb1798238 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1862,6 +1862,8 @@ int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
>  void intel_attach_force_audio_property(struct drm_connector *connector);
>  void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
>  void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> +void intel_attach_max_bpc_property(struct drm_connector *connector, int min, int
> +				   max);
>  
>  
>  /* intel_overlay.c */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index a1799b5c12bb..82739f342246 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2097,11 +2097,22 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
>  static void
>  intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> +
>  	intel_attach_force_audio_property(connector);
>  	intel_attach_broadcast_rgb_property(connector);
>  	intel_attach_aspect_ratio_property(connector);
>  	drm_connector_attach_content_type_property(connector);
>  	connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> +
> +	if (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
> +	    IS_CHERRYVIEW(dev_priv)) {
> +		intel_attach_max_bpc_property(connector, 8, 10);
> +		connector->state->max_bpc = 10;

These platforms don't support HDMI deep color. The 10bpc stuff in
PIPECONF is for DP. Incidentally DP support seems to be missing from
this series.

> +	} else if (INTEL_GEN(dev_priv) >= 5) {
> +		intel_attach_max_bpc_property(connector, 8, 12);
> +		connector->state->max_bpc = 12;
> +	}
>  }
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> index ca44bf368e24..78f2ce92f194 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -133,3 +133,23 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector)
>  			connector->dev->mode_config.aspect_ratio_property,
>  			DRM_MODE_PICTURE_ASPECT_NONE);
>  }
> +
> +void
> +intel_attach_max_bpc_property(struct drm_connector *connector, int min, int
> +			       max)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +	struct drm_property *prop;
> +
> +	prop = config->max_bpc_property;
> +	if (prop == NULL) {
> +		prop = drm_property_create_range(dev, 0, "max bpc", min, max);
> +		if (prop == NULL)
> +			return;
> +
> +		config->max_bpc_property = prop;
> +	}
> +
> +	drm_object_attach_property(&connector->base, prop, max);

Might make sense to do 'connector->state->max_bpc = max' here.
Avoids having to sprinkle it in every driver.

> +}
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 97ea41dc678f..d59b9e34ae80 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -460,6 +460,12 @@ struct drm_connector_state {
>  	 * drm_writeback_signal_completion()
>  	 */
>  	struct drm_writeback_job *writeback_job;
> +
> +	/**
> +	 * @max_bpc: Connector property to limit the maximum bit depth of
> +	 * the pixels being scanned out.

The comment is misleading. It makes me think this has something to do
with the pixels we scan out from a framebuffer.

> +	 */
> +	unsigned int max_bpc;
>  };
>  
>  /**
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index a0b202e1d69a..b9cd7a73b244 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -562,6 +562,11 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *link_status_property;
>  	/**
> +	 * @max_bpc_property: Default connector property for the max bpc to be
> +	 * driven out of the connector.
> +	 */
> +	struct drm_property *max_bpc_property;
> +	/**
>  	 * @plane_type_property: Default plane property to differentiate
>  	 * CURSOR, PRIMARY and OVERLAY legacy uses of planes.
>  	 */
> -- 
> 2.9.3

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] drm: drm/i915: Add connector property to limit max bpc
  2018-08-27 13:31 ` [PATCH v3 1/2] " Ville Syrjälä
@ 2018-08-29 22:57   ` Radhakrishna Sripada
  2018-08-30 13:38     ` Ville Syrjälä
  0 siblings, 1 reply; 8+ messages in thread
From: Radhakrishna Sripada @ 2018-08-29 22:57 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: dri-devel, intel-gfx, Kishore Kadiyala, Rodrigo Vivi

On Mon, Aug 27, 2018 at 04:31:49PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 24, 2018 at 06:02:16PM -0700, Radhakrishna Sripada wrote:
> > At times 12bpc HDMI cannot be driven due to faulty cables, dongles
> > level shifters etc. To workaround them we may need to drive the output
> > at a lower bpc. Currently the user space does not have a way to limit
> > the bpc. The default bpc to be programmed is decided by the driver and
> > is run against connector limitations.
> > 
> > Creating a new connector property "max bpc" in order to limit the bpc
> > with which the pixels are scanned out. xrandr can make use of this
> > connector property to make sure that bpc does not exceed the configured value.
> > This property can be used by userspace to set the bpc.
> > 
> > V2: Initialize max_bpc to satisfy kms_properties
> > V3: Move the property to drm_connector
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Kishore Kadiyala <kishore.kadiyala@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > ---
> >  drivers/gpu/drm/drm_atomic.c        |  4 ++++
> >  drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
> >  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> >  drivers/gpu/drm/i915/intel_hdmi.c   | 11 +++++++++++
> >  drivers/gpu/drm/i915/intel_modes.c  | 20 ++++++++++++++++++++
> 
> Pls move all the i915 stuff to the second patch.
Sure will do it in the next rev.

> 
> >  include/drm/drm_connector.h         |  6 ++++++
> >  include/drm/drm_mode_config.h       |  5 +++++
> >  7 files changed, 52 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 3eb061e11e2e..461dde0c2c10 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1416,6 +1416,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> >  
> >  		return set_out_fence_for_connector(state->state, connector,
> >  						   fence_ptr);
> > +	} else if (property == config->max_bpc_property) {
> > +		state->max_bpc = val;
> >  	} else if (connector->funcs->atomic_set_property) {
> >  		return connector->funcs->atomic_set_property(connector,
> >  				state, property, val);
> > @@ -1511,6 +1513,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> >  		*val = 0;
> >  	} else if (property == config->writeback_out_fence_ptr_property) {
> >  		*val = 0;
> > +	} else if (property == config->max_bpc_property) {
> > +		*val = state->max_bpc;
> >  	} else if (connector->funcs->atomic_get_property) {
> >  		return connector->funcs->atomic_get_property(connector,
> >  				state, property, val);
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 38ce9a375ffb..82caac8d1432 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -638,6 +638,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->max_bpc !=
> > +			    new_connector_state->max_bpc)
> > +				new_crtc_state->connectors_changed = true;
> >  		}
> >  
> >  		if (funcs->atomic_check)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 1b78de838c18..209eb1798238 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1862,6 +1862,8 @@ int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
> >  void intel_attach_force_audio_property(struct drm_connector *connector);
> >  void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
> >  void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> > +void intel_attach_max_bpc_property(struct drm_connector *connector, int min, int
> > +				   max);
> >  
> >  
> >  /* intel_overlay.c */
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index a1799b5c12bb..82739f342246 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -2097,11 +2097,22 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
> >  static void
> >  intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
> >  {
> > +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > +
> >  	intel_attach_force_audio_property(connector);
> >  	intel_attach_broadcast_rgb_property(connector);
> >  	intel_attach_aspect_ratio_property(connector);
> >  	drm_connector_attach_content_type_property(connector);
> >  	connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> > +
> > +	if (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
> > +	    IS_CHERRYVIEW(dev_priv)) {
> > +		intel_attach_max_bpc_property(connector, 8, 10);
> > +		connector->state->max_bpc = 10;
> 
> These platforms don't support HDMI deep color. The 10bpc stuff in
> PIPECONF is for DP. Incidentally DP support seems to be missing from
> this series.
Limiting this to 8 and removing VLV/CHV to the other case as they support deep color.

> 
> > +	} else if (INTEL_GEN(dev_priv) >= 5) {
> > +		intel_attach_max_bpc_property(connector, 8, 12);
> > +		connector->state->max_bpc = 12;
> > +	}
> >  }
> >  
> >  /*
> > diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> > index ca44bf368e24..78f2ce92f194 100644
> > --- a/drivers/gpu/drm/i915/intel_modes.c
> > +++ b/drivers/gpu/drm/i915/intel_modes.c
> > @@ -133,3 +133,23 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector)
> >  			connector->dev->mode_config.aspect_ratio_property,
> >  			DRM_MODE_PICTURE_ASPECT_NONE);
> >  }
> > +
> > +void
> > +intel_attach_max_bpc_property(struct drm_connector *connector, int min, int
> > +			       max)
> > +{
> > +	struct drm_device *dev = connector->dev;
> > +	struct drm_mode_config *config = &dev->mode_config;
> > +	struct drm_property *prop;
> > +
> > +	prop = config->max_bpc_property;
> > +	if (prop == NULL) {
> > +		prop = drm_property_create_range(dev, 0, "max bpc", min, max);
> > +		if (prop == NULL)
> > +			return;
> > +
> > +		config->max_bpc_property = prop;
> > +	}
> > +
> > +	drm_object_attach_property(&connector->base, prop, max);
> 
> Might make sense to do 'connector->state->max_bpc = max' here.
> Avoids having to sprinkle it in every driver.
Sure will make this change in the next rev.

> 
> > +}
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 97ea41dc678f..d59b9e34ae80 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -460,6 +460,12 @@ struct drm_connector_state {
> >  	 * drm_writeback_signal_completion()
> >  	 */
> >  	struct drm_writeback_job *writeback_job;
> > +
> > +	/**
> > +	 * @max_bpc: Connector property to limit the maximum bit depth of
> > +	 * the pixels being scanned out.
> 
> The comment is misleading. It makes me think this has something to do
> with the pixels we scan out from a framebuffer.
Will modify this comment in the next rev.

Regards,
Radhakrishna(RK) Sripada
> 
> > +	 */
> > +	unsigned int max_bpc;
> >  };
> >  
> >  /**
> > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> > index a0b202e1d69a..b9cd7a73b244 100644
> > --- a/include/drm/drm_mode_config.h
> > +++ b/include/drm/drm_mode_config.h
> > @@ -562,6 +562,11 @@ struct drm_mode_config {
> >  	 */
> >  	struct drm_property *link_status_property;
> >  	/**
> > +	 * @max_bpc_property: Default connector property for the max bpc to be
> > +	 * driven out of the connector.
> > +	 */
> > +	struct drm_property *max_bpc_property;
> > +	/**
> >  	 * @plane_type_property: Default plane property to differentiate
> >  	 * CURSOR, PRIMARY and OVERLAY legacy uses of planes.
> >  	 */
> > -- 
> > 2.9.3
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] drm: drm/i915: Add connector property to limit max bpc
  2018-08-29 22:57   ` Radhakrishna Sripada
@ 2018-08-30 13:38     ` Ville Syrjälä
  0 siblings, 0 replies; 8+ messages in thread
From: Ville Syrjälä @ 2018-08-30 13:38 UTC (permalink / raw)
  To: Radhakrishna Sripada; +Cc: dri-devel, intel-gfx, Kishore Kadiyala, Rodrigo Vivi

On Wed, Aug 29, 2018 at 03:57:05PM -0700, Radhakrishna Sripada wrote:
> On Mon, Aug 27, 2018 at 04:31:49PM +0300, Ville Syrjälä wrote:
> > On Fri, Aug 24, 2018 at 06:02:16PM -0700, Radhakrishna Sripada wrote:
> > > At times 12bpc HDMI cannot be driven due to faulty cables, dongles
> > > level shifters etc. To workaround them we may need to drive the output
> > > at a lower bpc. Currently the user space does not have a way to limit
> > > the bpc. The default bpc to be programmed is decided by the driver and
> > > is run against connector limitations.
> > > 
> > > Creating a new connector property "max bpc" in order to limit the bpc
> > > with which the pixels are scanned out. xrandr can make use of this
> > > connector property to make sure that bpc does not exceed the configured value.
> > > This property can be used by userspace to set the bpc.
> > > 
> > > V2: Initialize max_bpc to satisfy kms_properties
> > > V3: Move the property to drm_connector
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Kishore Kadiyala <kishore.kadiyala@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_atomic.c        |  4 ++++
> > >  drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
> > >  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> > >  drivers/gpu/drm/i915/intel_hdmi.c   | 11 +++++++++++
> > >  drivers/gpu/drm/i915/intel_modes.c  | 20 ++++++++++++++++++++
> > 
> > Pls move all the i915 stuff to the second patch.
> Sure will do it in the next rev.
> 
> > 
> > >  include/drm/drm_connector.h         |  6 ++++++
> > >  include/drm/drm_mode_config.h       |  5 +++++
> > >  7 files changed, 52 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > index 3eb061e11e2e..461dde0c2c10 100644
> > > --- a/drivers/gpu/drm/drm_atomic.c
> > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > @@ -1416,6 +1416,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> > >  
> > >  		return set_out_fence_for_connector(state->state, connector,
> > >  						   fence_ptr);
> > > +	} else if (property == config->max_bpc_property) {
> > > +		state->max_bpc = val;
> > >  	} else if (connector->funcs->atomic_set_property) {
> > >  		return connector->funcs->atomic_set_property(connector,
> > >  				state, property, val);
> > > @@ -1511,6 +1513,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> > >  		*val = 0;
> > >  	} else if (property == config->writeback_out_fence_ptr_property) {
> > >  		*val = 0;
> > > +	} else if (property == config->max_bpc_property) {
> > > +		*val = state->max_bpc;
> > >  	} else if (connector->funcs->atomic_get_property) {
> > >  		return connector->funcs->atomic_get_property(connector,
> > >  				state, property, val);
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > index 38ce9a375ffb..82caac8d1432 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -638,6 +638,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->max_bpc !=
> > > +			    new_connector_state->max_bpc)
> > > +				new_crtc_state->connectors_changed = true;
> > >  		}
> > >  
> > >  		if (funcs->atomic_check)
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 1b78de838c18..209eb1798238 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1862,6 +1862,8 @@ int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
> > >  void intel_attach_force_audio_property(struct drm_connector *connector);
> > >  void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
> > >  void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> > > +void intel_attach_max_bpc_property(struct drm_connector *connector, int min, int
> > > +				   max);
> > >  
> > >  
> > >  /* intel_overlay.c */
> > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > > index a1799b5c12bb..82739f342246 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > @@ -2097,11 +2097,22 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
> > >  static void
> > >  intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
> > >  {
> > > +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > > +
> > >  	intel_attach_force_audio_property(connector);
> > >  	intel_attach_broadcast_rgb_property(connector);
> > >  	intel_attach_aspect_ratio_property(connector);
> > >  	drm_connector_attach_content_type_property(connector);
> > >  	connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> > > +
> > > +	if (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
> > > +	    IS_CHERRYVIEW(dev_priv)) {
> > > +		intel_attach_max_bpc_property(connector, 8, 10);
> > > +		connector->state->max_bpc = 10;
> > 
> > These platforms don't support HDMI deep color. The 10bpc stuff in
> > PIPECONF is for DP. Incidentally DP support seems to be missing from
> > this series.
> Limiting this to 8 and removing VLV/CHV to the other case as they support deep color.

They don't.

> 
> > 
> > > +	} else if (INTEL_GEN(dev_priv) >= 5) {

if (!HAS_GMCH_DISPLAY()) {
 ...

should be what we want here.


> > > +		intel_attach_max_bpc_property(connector, 8, 12);
> > > +		connector->state->max_bpc = 12;
> > > +	}
> > >  }
> > >  
> > >  /*
> > > diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> > > index ca44bf368e24..78f2ce92f194 100644
> > > --- a/drivers/gpu/drm/i915/intel_modes.c
> > > +++ b/drivers/gpu/drm/i915/intel_modes.c
> > > @@ -133,3 +133,23 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector)
> > >  			connector->dev->mode_config.aspect_ratio_property,
> > >  			DRM_MODE_PICTURE_ASPECT_NONE);
> > >  }
> > > +
> > > +void
> > > +intel_attach_max_bpc_property(struct drm_connector *connector, int min, int
> > > +			       max)
> > > +{
> > > +	struct drm_device *dev = connector->dev;
> > > +	struct drm_mode_config *config = &dev->mode_config;
> > > +	struct drm_property *prop;
> > > +
> > > +	prop = config->max_bpc_property;
> > > +	if (prop == NULL) {
> > > +		prop = drm_property_create_range(dev, 0, "max bpc", min, max);
> > > +		if (prop == NULL)
> > > +			return;
> > > +
> > > +		config->max_bpc_property = prop;
> > > +	}
> > > +
> > > +	drm_object_attach_property(&connector->base, prop, max);
> > 
> > Might make sense to do 'connector->state->max_bpc = max' here.
> > Avoids having to sprinkle it in every driver.
> Sure will make this change in the next rev.
> 
> > 
> > > +}
> > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > index 97ea41dc678f..d59b9e34ae80 100644
> > > --- a/include/drm/drm_connector.h
> > > +++ b/include/drm/drm_connector.h
> > > @@ -460,6 +460,12 @@ struct drm_connector_state {
> > >  	 * drm_writeback_signal_completion()
> > >  	 */
> > >  	struct drm_writeback_job *writeback_job;
> > > +
> > > +	/**
> > > +	 * @max_bpc: Connector property to limit the maximum bit depth of
> > > +	 * the pixels being scanned out.
> > 
> > The comment is misleading. It makes me think this has something to do
> > with the pixels we scan out from a framebuffer.
> Will modify this comment in the next rev.
> 
> Regards,
> Radhakrishna(RK) Sripada
> > 
> > > +	 */
> > > +	unsigned int max_bpc;
> > >  };
> > >  
> > >  /**
> > > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> > > index a0b202e1d69a..b9cd7a73b244 100644
> > > --- a/include/drm/drm_mode_config.h
> > > +++ b/include/drm/drm_mode_config.h
> > > @@ -562,6 +562,11 @@ struct drm_mode_config {
> > >  	 */
> > >  	struct drm_property *link_status_property;
> > >  	/**
> > > +	 * @max_bpc_property: Default connector property for the max bpc to be
> > > +	 * driven out of the connector.
> > > +	 */
> > > +	struct drm_property *max_bpc_property;
> > > +	/**
> > >  	 * @plane_type_property: Default plane property to differentiate
> > >  	 * CURSOR, PRIMARY and OVERLAY legacy uses of planes.
> > >  	 */
> > > -- 
> > > 2.9.3
> > 
> > -- 
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-08-30 13:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-25  1:02 [PATCH v3 1/2] drm: drm/i915: Add connector property to limit max bpc Radhakrishna Sripada
2018-08-25  1:02 ` [PATCH v3 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp Radhakrishna Sripada
2018-08-25  1:09 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/2] drm: drm/i915: Add connector property to limit max bpc Patchwork
2018-08-25  1:29 ` ✓ Fi.CI.BAT: success " Patchwork
2018-08-25  2:20 ` ✓ Fi.CI.IGT: " Patchwork
2018-08-27 13:31 ` [PATCH v3 1/2] " Ville Syrjälä
2018-08-29 22:57   ` Radhakrishna Sripada
2018-08-30 13:38     ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox