[Intel-gfx] [PATCH 2/2] drm/i915/BXT: Tolerance at BXT DSI pipe_config comparison
Ramalingam C
ramalingam.c at intel.com
Wed Mar 30 14:19:40 UTC 2016
On Wednesday 30 March 2016 05:02 PM, Daniel Vetter wrote:
> On Tue, Mar 29, 2016 at 11:04:51PM +0530, Ramalingam C wrote:
>> At BXT DSI, PIPE registers are inactive. So we can't get the
>> PIPE's mode parameters from them. The possible option is
>> retriving them from the PORT registers. But mode timing
>> parameters are progammed to port registers interms of byteclocks.
>>
>> The formula used to convert the pixels interms of byteclk is
>> DIV_ROUND_UP(DIV_ROUND_UP(pixels * bpp * burst_mode_ratio,
>> 8 * 100), lane_count);
>>
>> So we retrieve them, interms of pixels as
>> DIV_ROUND_UP((clk_hs * lane_count * 8 * 100),
>> (bpp * burst_mode_ratio));
>>
>> Due to the multiple DIV_ROUND_UP in both formulas we get the worst
>> case delta in the retrieved PIPE's timing parameter as below
>> DIV_ROUND_UP((8 * intel_dsi->lane_count * 100),
>> (dsi_pixel_format_bpp(intel_dsi->pixel_format) *
>> intel_dsi->burst_mode_ratio)))
>>
>> This converson of byteclk to pixel is required for hsync, hfp and hbp.
>> Which intern impacts horrizontal timing parameters. At worst case to
>> get htotal all there parameters are added with hactive.
>> Hence delta will be 3 times of above formula. Hence this value is
>> considered as tolerance for pipe_config comparison, in case of BXT DSI.
>>
>> Signed-off-by: Ramalingam C <ramalingam.c at intel.com>
> This is the wrong way round imo, better would be to adjust the adjusted
> mode in the bxt dsi compute_config function to match the hw granularity.
> Stuff _really_ should match exactly, the fuzzy clock matching is mostly
> because our clock cod is a mess, and we can't/don't properly
> forward-compuate the actual clock timings we program into the hardware.
> -Daniel
Daniel, I got your point. But the problem will be that difficulty(even
if possible) in adjusting the adjusted mode parameters.
Reason is we are not programing the mode parameter as such. We will
derive the hfp, hsync and hbp from
hsync_start, hsync_end, hdisplay and htotal. These will be
adjusted(divided by 2) for dual link scenario.
And then resultant will go into the conversion as mentioned in the
commit message (two DIV_ROUND_UP onwards
and one DIV_ROUND_UP backwards). For this we have to make the parameter
divisible by three different factors.
So IMHO, even if this is possible, it will look more messy.
Predicting the max error and tolerating it in pipe_config_compare will
be the straight forward and more reasonable.
Please let me know if i can go ahead in this approach.
>
>> ---
>> Reviewed at https://lists.freedesktop.org/archives/intel-gfx/2016-March/089548.html
>>
>> drivers/gpu/drm/i915/intel_display.c | 62 +++++++++++++++++++++++++++++++---
>> 1 file changed, 57 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index c0627d6..282f036 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -12557,6 +12557,9 @@ intel_pipe_config_compare(struct drm_device *dev,
>> bool adjust)
>> {
>> bool ret = true;
>> + struct intel_crtc *crtc = to_intel_crtc(current_config->base.crtc);
>> + struct intel_encoder *intel_encoder;
>> + struct intel_dsi *intel_dsi = NULL;
>>
>> #define INTEL_ERR_OR_DBG_KMS(fmt, ...) \
>> do { \
>> @@ -12593,6 +12596,54 @@ intel_pipe_config_compare(struct drm_device *dev,
>> ret = false; \
>> }
>>
>> +/*
>> + * In case of BXT DSI, HW pipe_config will be retrieved from the port's timing
>> + * configuration. This retrival includes some errors due to the DIV_ROUND_UP.
>> + * So we are considering the max possible error at the comparison.
>> + */
>> +/*
>> + * htotal = hactive + hfp + hsync + hbp. Here last three lements might have
>> + * the converson error, hence we consider the 3 times of error as tolerance.
>> + */
>> +
>> +#define MAX_BXT_DSI_TIMING_RETRIVAL_ERR \
>> + (intel_dsi == NULL ? 0 : \
>> + DIV_ROUND_UP((3 * 8 * intel_dsi->lane_count * 100), \
>> + (dsi_pixel_format_bpp(intel_dsi->pixel_format) * \
>> + intel_dsi->burst_mode_ratio)))
>> +
>> +#define BXT_DSI_PIPE_CONF_CHECK_I_RANGE(name) { \
>> + for_each_encoder_on_crtc(dev, &crtc->base, \
>> + intel_encoder) { \
>> + if (intel_encoder->type == INTEL_OUTPUT_DSI) { \
>> + intel_dsi = enc_to_intel_dsi(&intel_encoder->base); \
>> + } \
>> + } \
>> + if (!(current_config->name < pipe_config->name && \
>> + current_config->name >= (pipe_config->name - \
>> + MAX_BXT_DSI_TIMING_RETRIVAL_ERR))) { \
>> + INTEL_ERR_OR_DBG_KMS("mismatch in " #name " " \
>> + "(expected %i, found %i(Err tolerance considered))\n", \
>> + current_config->name, \
>> + pipe_config->name); \
>> + ret = false; \
>> + } \
>> +}
>> +
>> +#define PIPE_CONF_CHECK_I_RANGE(name) { \
>> + if (current_config->name != pipe_config->name) { \
>> + if (IS_BROXTON(dev) && crtc->config->has_dsi_encoder) { \
>> + BXT_DSI_PIPE_CONF_CHECK_I_RANGE(name) \
>> + } else { \
>> + INTEL_ERR_OR_DBG_KMS("mismatch in " #name " " \
>> + "(expected %i, found %i)\n", \
>> + current_config->name, \
>> + pipe_config->name); \
>> + ret = false; \
>> + } \
>> + } \
>> +}
>> +
>> #define PIPE_CONF_CHECK_M_N(name) \
>> if (!intel_compare_link_m_n(¤t_config->name, \
>> &pipe_config->name,\
>> @@ -12697,11 +12748,11 @@ intel_pipe_config_compare(struct drm_device *dev,
>> PIPE_CONF_CHECK_I(has_dsi_encoder);
>>
>> PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hdisplay);
>> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_htotal);
>> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hblank_start);
>> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hblank_end);
>> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hsync_start);
>> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hsync_end);
>> + PIPE_CONF_CHECK_I_RANGE(base.adjusted_mode.crtc_htotal);
>> + PIPE_CONF_CHECK_I_RANGE(base.adjusted_mode.crtc_hblank_start);
>> + PIPE_CONF_CHECK_I_RANGE(base.adjusted_mode.crtc_hblank_end);
>> + PIPE_CONF_CHECK_I_RANGE(base.adjusted_mode.crtc_hsync_start);
>> + PIPE_CONF_CHECK_I_RANGE(base.adjusted_mode.crtc_hsync_end);
>>
>> PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vdisplay);
>> PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vtotal);
>> @@ -12779,6 +12830,7 @@ intel_pipe_config_compare(struct drm_device *dev,
>>
>> #undef PIPE_CONF_CHECK_X
>> #undef PIPE_CONF_CHECK_I
>> +#undef PIPE_CONF_CHECK_I_RANGE
>> #undef PIPE_CONF_CHECK_P
>> #undef PIPE_CONF_CHECK_I_ALT
>> #undef PIPE_CONF_CHECK_FLAGS
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Thanks,
--Ram
More information about the Intel-gfx
mailing list