| Message ID | 1479850766-32748-4-git-send-email-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 22 Nov 2016, Manasi Navare <manasi.d.navare@intel.com> wrote: > From: Jim Bride <jim.bride@linux.intel.com> > > For DP compliance we need to be able to control the output color > type for the pipe associated with the DP port. When a specific DP > compliance test is requested with specific BPC value, we read the BPC > value from the DPCD register and store it in the intel_dp structure. > If this BPC value in intel_dp structure has a non-zero value > and we're on a display port connector, then we use the value to > calculate the bpp for the pipe. For cases where we are > not on DP or there has not been an overridden value then we behave > as normal. > > Signed-off-by: Jim Bride <jim.bride@linux.intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 4 +++- > drivers/gpu/drm/i915/intel_dp.c | 10 ++++++++++ > drivers/gpu/drm/i915/intel_dp_mst.c | 11 +++++++++-- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 4 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index b7a7ed8..bb1cca2 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12694,11 +12694,13 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev) > > /* Clamp display bpp to EDID value */ > for_each_connector_in_state(state, connector, connector_state, i) { > + Unrelated change, and one we don't want. > if (connector_state->crtc != &crtc->base) > continue; > > connected_sink_compute_bpp(to_intel_connector(connector), > - pipe_config); > + pipe_config); > + Unrelated change, and one we don't want. > } > > return bpp; > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 6693918..f93e130 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1549,6 +1549,13 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp, > if (bpc > 0) > bpp = min(bpp, 3*bpc); > > + /* For DP Compliance we override the computed bpp for the pipe */ > + if (intel_dp->compliance_force_bpc != 0) { You don't actually *set* compliance_force_bpc in this patch, making all of this a complicated nop. We don't want this kind of changes, because if this regresses, it'll regress in the patch actually *using* the code, i.e. the patch setting compliance_force_bpc, not this one. The rationale is the same as for adding unused functions as prep patches. > + pipe_config->pipe_bpp = intel_dp->compliance_force_bpc*3; How about making that compliance_force_bpp and setting it to * 3 directly in one place instead of sprinkling the * 3 all over the place? > + DRM_DEBUG_KMS("Setting pipe_bpp to %d\n", > + pipe_config->pipe_bpp); > + } > + > return bpp; > } > > @@ -1629,6 +1636,7 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp, > /* Walk through all bpp values. Luckily they're all nicely spaced with 2 > * bpc in between. */ > bpp = intel_dp_compute_bpp(intel_dp, pipe_config); > + Unrelated change. Please pay attention to these. > if (is_edp(intel_dp)) { > > /* Get bpp from vbt only for panels that dont have bpp in edid */ > @@ -4109,6 +4117,7 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) > intel_dp->compliance_test_data = 0; > intel_dp->compliance_test_lane_count = 0; > intel_dp->compliance_test_link_rate = 0; > + intel_dp->compliance_force_bpc = 0; Again, benefits from having a sub struct. > > /* > * Now read the DPCD to see if it's actually running > @@ -4434,6 +4443,7 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > intel_dp->compliance_test_active = 0; > intel_dp->compliance_test_type = 0; > intel_dp->compliance_test_data = 0; > + intel_dp->compliance_force_bpc = 0; > intel_dp->compliance_test_lane_count = 0; > intel_dp->compliance_test_link_rate = 0; > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c > index b029d10..940f173 100644 > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > @@ -45,6 +45,14 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, > > pipe_config->has_pch_encoder = false; > bpp = 24; > + /* For DP Compliance we need to ensurethat we can override > + * the computed bpp for the pipe > + */ Unnecessary comment. > + if (intel_dp->compliance_force_bpc) { > + bpp = intel_dp->compliance_force_bpc * 3; > + DRM_DEBUG_KMS("Setting pipe bpp to %d\n", > + bpp); > + } > /* > * for MST we always configure max link bw - the spec doesn't > * seem to suggest we should do otherwise. > @@ -52,8 +60,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, > lane_count = drm_dp_max_lane_count(intel_dp->dpcd); > > pipe_config->lane_count = lane_count; > - > - pipe_config->pipe_bpp = 24; > + pipe_config->pipe_bpp = bpp; > pipe_config->port_clock = intel_dp_max_link_rate(intel_dp); > > state = pipe_config->base.state; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 1e88288..3eb428e 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -960,6 +960,7 @@ struct intel_dp { > bool compliance_test_active; > u8 compliance_test_lane_count; > u8 compliance_test_link_rate; > + unsigned long compliance_force_bpc; /* 0 for default or bpc to use */ unsigned long? How about plain int? > }; > > struct intel_lspcon {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b7a7ed8..bb1cca2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12694,11 +12694,13 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev) /* Clamp display bpp to EDID value */ for_each_connector_in_state(state, connector, connector_state, i) { + if (connector_state->crtc != &crtc->base) continue; connected_sink_compute_bpp(to_intel_connector(connector), - pipe_config); + pipe_config); + } return bpp; diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 6693918..f93e130 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1549,6 +1549,13 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp, if (bpc > 0) bpp = min(bpp, 3*bpc); + /* For DP Compliance we override the computed bpp for the pipe */ + if (intel_dp->compliance_force_bpc != 0) { + pipe_config->pipe_bpp = intel_dp->compliance_force_bpc*3; + DRM_DEBUG_KMS("Setting pipe_bpp to %d\n", + pipe_config->pipe_bpp); + } + return bpp; } @@ -1629,6 +1636,7 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp, /* Walk through all bpp values. Luckily they're all nicely spaced with 2 * bpc in between. */ bpp = intel_dp_compute_bpp(intel_dp, pipe_config); + if (is_edp(intel_dp)) { /* Get bpp from vbt only for panels that dont have bpp in edid */ @@ -4109,6 +4117,7 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) intel_dp->compliance_test_data = 0; intel_dp->compliance_test_lane_count = 0; intel_dp->compliance_test_link_rate = 0; + intel_dp->compliance_force_bpc = 0; /* * Now read the DPCD to see if it's actually running @@ -4434,6 +4443,7 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, intel_dp->compliance_test_active = 0; intel_dp->compliance_test_type = 0; intel_dp->compliance_test_data = 0; + intel_dp->compliance_force_bpc = 0; intel_dp->compliance_test_lane_count = 0; intel_dp->compliance_test_link_rate = 0; diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index b029d10..940f173 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -45,6 +45,14 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, pipe_config->has_pch_encoder = false; bpp = 24; + /* For DP Compliance we need to ensurethat we can override + * the computed bpp for the pipe + */ + if (intel_dp->compliance_force_bpc) { + bpp = intel_dp->compliance_force_bpc * 3; + DRM_DEBUG_KMS("Setting pipe bpp to %d\n", + bpp); + } /* * for MST we always configure max link bw - the spec doesn't * seem to suggest we should do otherwise. @@ -52,8 +60,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, lane_count = drm_dp_max_lane_count(intel_dp->dpcd); pipe_config->lane_count = lane_count; - - pipe_config->pipe_bpp = 24; + pipe_config->pipe_bpp = bpp; pipe_config->port_clock = intel_dp_max_link_rate(intel_dp); state = pipe_config->base.state; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1e88288..3eb428e 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -960,6 +960,7 @@ struct intel_dp { bool compliance_test_active; u8 compliance_test_lane_count; u8 compliance_test_link_rate; + unsigned long compliance_force_bpc; /* 0 for default or bpc to use */ }; struct intel_lspcon {