Re: [PATCH 12/17] drm/i915: Update intel_dp_compute_config() to handle compliance test requests

Todd Previte
2015-02-18T16:59:14Z

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
>>
>>
>>