| Message ID | 1418255597-4716-13-git-send-email-tprevite@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2014-12-10 21:53 GMT-02:00 Todd Previte <tprevite@gmail.com>: > Adds provisions in intel_dp_compute_config() to accommodate compliance > testing. Mostly this invovles circumventing the automatic link configuration > parameters and allowing the compliance code to set those parameters as > required by the tests. > > Signed-off-by: Todd Previte <tprevite@gmail.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 2a13124..4a55ca6 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1189,6 +1189,21 @@ intel_dp_compute_config(struct intel_encoder *encoder, > pipe_config->has_drrs = false; > pipe_config->has_audio = intel_dp->has_audio; > > + /* Compliance testing should skip most of this function */ > + if (!is_edp(intel_dp) && intel_dp->compliance_testing_active) { I couldn't find any patch on your series that flips intel_dp->compliance_testing_active to true, which is weird since it would prevent us from testing the code. Also, if we can make sure that we never set compliance_testing_active to true on eDP, we can remove the is_edp() check. > + bpp = intel_dp->compliance_config.bits_per_pixel; > + lane_count = intel_dp->compliance_config.lane_count; > + clock = intel_dp->compliance_config.link_rate >> 3; > + /* Assign here and skip at the end - ensures correct values */ > + intel_dp->link_bw = bws[clock]; > + intel_dp->lane_count = lane_count; > + pipe_config->pipe_bpp = bpp; > + pipe_config->port_clock = > + drm_dp_bw_code_to_link_rate(intel_dp->link_bw); > + > + goto compliance_exit; > + } > + > if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) { > intel_fixed_panel_mode(intel_connector->panel.fixed_mode, > adjusted_mode); > @@ -1275,6 +1290,7 @@ found: > DRM_DEBUG_KMS("DP link bw required %i available %i\n", > mode_rate, link_avail); > > +compliance_exit: Don't we need to move the color range adjustments to this point? > intel_link_compute_m_n(bpp, lane_count, > adjusted_mode->crtc_clock, > pipe_config->port_clock, > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 12/17/2014 09:04 AM, Paulo Zanoni wrote: > 2014-12-10 21:53 GMT-02:00 Todd Previte <tprevite@gmail.com>: >> Adds provisions in intel_dp_compute_config() to accommodate compliance >> testing. Mostly this invovles circumventing the automatic link configuration >> parameters and allowing the compliance code to set those parameters as >> required by the tests. >> >> Signed-off-by: Todd Previte <tprevite@gmail.com> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 2a13124..4a55ca6 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -1189,6 +1189,21 @@ intel_dp_compute_config(struct intel_encoder *encoder, >> pipe_config->has_drrs = false; >> pipe_config->has_audio = intel_dp->has_audio; >> >> + /* Compliance testing should skip most of this function */ >> + if (!is_edp(intel_dp) && intel_dp->compliance_testing_active) { > > I couldn't find any patch on your series that flips > intel_dp->compliance_testing_active to true, which is weird since it > would prevent us from testing the code. > > Also, if we can make sure that we never set compliance_testing_active > to true on eDP, we can remove the is_edp() check. Why would we not allow automation compliance testing on eDP? There are automation tests and fixtures from Unigraf and Agilent for eDP. -Clint > >> + bpp = intel_dp->compliance_config.bits_per_pixel; >> + lane_count = intel_dp->compliance_config.lane_count; >> + clock = intel_dp->compliance_config.link_rate >> 3; >> + /* Assign here and skip at the end - ensures correct values */ >> + intel_dp->link_bw = bws[clock]; >> + intel_dp->lane_count = lane_count; >> + pipe_config->pipe_bpp = bpp; >> + pipe_config->port_clock = >> + drm_dp_bw_code_to_link_rate(intel_dp->link_bw); >> + >> + goto compliance_exit; >> + } >> + >> if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) { >> intel_fixed_panel_mode(intel_connector->panel.fixed_mode, >> adjusted_mode); >> @@ -1275,6 +1290,7 @@ found: >> DRM_DEBUG_KMS("DP link bw required %i available %i\n", >> mode_rate, link_avail); >> >> +compliance_exit: > > Don't we need to move the color range adjustments to this point? > >> intel_link_compute_m_n(bpp, lane_count, >> adjusted_mode->crtc_clock, >> pipe_config->port_clock, >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > >
On 1/7/15 12:28 PM, Clint Taylor wrote: > On 12/17/2014 09:04 AM, Paulo Zanoni wrote: >> 2014-12-10 21:53 GMT-02:00 Todd Previte <tprevite@gmail.com>: >>> Adds provisions in intel_dp_compute_config() to accommodate compliance >>> testing. Mostly this invovles circumventing the automatic link >>> configuration >>> parameters and allowing the compliance code to set those parameters as >>> required by the tests. >>> >>> Signed-off-by: Todd Previte <tprevite@gmail.com> >>> --- >>> drivers/gpu/drm/i915/intel_dp.c | 16 ++++++++++++++++ >>> 1 file changed, 16 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c >>> b/drivers/gpu/drm/i915/intel_dp.c >>> index 2a13124..4a55ca6 100644 >>> --- a/drivers/gpu/drm/i915/intel_dp.c >>> +++ b/drivers/gpu/drm/i915/intel_dp.c >>> @@ -1189,6 +1189,21 @@ intel_dp_compute_config(struct intel_encoder >>> *encoder, >>> pipe_config->has_drrs = false; >>> pipe_config->has_audio = intel_dp->has_audio; >>> >>> + /* Compliance testing should skip most of this function */ >>> + if (!is_edp(intel_dp) && intel_dp->compliance_testing_active) { >> >> I couldn't find any patch on your series that flips >> intel_dp->compliance_testing_active to true, which is weird since it >> would prevent us from testing the code. >> >> Also, if we can make sure that we never set compliance_testing_active >> to true on eDP, we can remove the is_edp() check. > > Why would we not allow automation compliance testing on eDP? There are > automation tests and fixtures from Unigraf and Agilent for eDP. > > -Clint eDP has different testing requirements and is a completely different specification than the one for regular Displayport. This patch set is for external Displayport connections only in accordance with the Displayport Link CTS 1.2 Core rev 1.1a document. > >> >>> + bpp = intel_dp->compliance_config.bits_per_pixel; >>> + lane_count = intel_dp->compliance_config.lane_count; >>> + clock = intel_dp->compliance_config.link_rate >> 3; >>> + /* Assign here and skip at the end - ensures correct >>> values */ >>> + intel_dp->link_bw = bws[clock]; >>> + intel_dp->lane_count = lane_count; >>> + pipe_config->pipe_bpp = bpp; >>> + pipe_config->port_clock = >>> + drm_dp_bw_code_to_link_rate(intel_dp->link_bw); >>> + >>> + goto compliance_exit; >>> + } >>> + >>> if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) { >>> intel_fixed_panel_mode(intel_connector->panel.fixed_mode, >>> adjusted_mode); >>> @@ -1275,6 +1290,7 @@ found: >>> DRM_DEBUG_KMS("DP link bw required %i available %i\n", >>> mode_rate, link_avail); >>> >>> +compliance_exit: >> >> Don't we need to move the color range adjustments to this point? >> >>> intel_link_compute_m_n(bpp, lane_count, >>> adjusted_mode->crtc_clock, >>> pipe_config->port_clock, >>> -- >>> 1.9.1 >>> >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> >> >
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 2a13124..4a55ca6 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1189,6 +1189,21 @@ intel_dp_compute_config(struct intel_encoder *encoder, pipe_config->has_drrs = false; pipe_config->has_audio = intel_dp->has_audio; + /* Compliance testing should skip most of this function */ + if (!is_edp(intel_dp) && intel_dp->compliance_testing_active) { + bpp = intel_dp->compliance_config.bits_per_pixel; + lane_count = intel_dp->compliance_config.lane_count; + clock = intel_dp->compliance_config.link_rate >> 3; + /* Assign here and skip at the end - ensures correct values */ + intel_dp->link_bw = bws[clock]; + intel_dp->lane_count = lane_count; + pipe_config->pipe_bpp = bpp; + pipe_config->port_clock = + drm_dp_bw_code_to_link_rate(intel_dp->link_bw); + + goto compliance_exit; + } + if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) { intel_fixed_panel_mode(intel_connector->panel.fixed_mode, adjusted_mode); @@ -1275,6 +1290,7 @@ found: DRM_DEBUG_KMS("DP link bw required %i available %i\n", mode_rate, link_avail); +compliance_exit: intel_link_compute_m_n(bpp, lane_count, adjusted_mode->crtc_clock, pipe_config->port_clock,
Adds provisions in intel_dp_compute_config() to accommodate compliance testing. Mostly this invovles circumventing the automatic link configuration parameters and allowing the compliance code to set those parameters as required by the tests. Signed-off-by: Todd Previte <tprevite@gmail.com> --- drivers/gpu/drm/i915/intel_dp.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)