Re: [PATCH] drm/i915/BXT: Tolerance at BXT DSI pipe_config comparison

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Tuesday 29 March 2016 01:08 PM, Jani Nikula wrote:
On Mon, 28 Mar 2016, Ramalingam C <ramalingam.c@xxxxxxxxx> wrote:
On Monday 21 March 2016 07:13 PM, Jani Nikula wrote:
On Fri, 11 Mar 2016, Ramalingam C <ramalingam.c@xxxxxxxxx> wrote:
At BXT DSI, PIPE registers are inactive. So we can 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.
I think you should take the encoder specific callback parts from [1],
and instead of adding a new ->get_pipe_config hook, do them in
->get_hw_state, and add the rebased work from this patch on top.

BR,
Jani.

[1] http://patchwork.freedesktop.org/patch/msgid/1456771760-18823-1-git-send-email-ramalingam.c@xxxxxxxxx
Jani,

At present, encoder->get_hw_state is used for the getting the encoder's
activeness(dsi port is active or not) along with the pipe it is
connected to it.
By definition of that encoder function declaration, I dont think we
should use it for retrieving the pipe timing parameters from port.

I feel adding a new encoder function is making more sense, for handling
this special case of retrieving the timing parameters
from port register for BXT.

But If we dont want to use new encoder function I feel
encoder->get_config is the option. As this is supposed to get called after
display->get_pipe_config and also this function fills the
pipe_config->base.adjusted_mode.crtc_clock already.

Please tell me if you agree with the separate encoder function.
Please use encoder->get_config().

BR,
Jani.

Please find the change at https://lists.freedesktop.org/archives/intel-gfx/2016-March/090999.html

--Ram
--Ram


Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>

This change is required for
https://lists.freedesktop.org/archives/intel-gfx/2016-February/088653.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 7068dc1..46ce662 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12653,6 +12653,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 { \
@@ -12680,6 +12683,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(&current_config->name, \
   				    &pipe_config->name,\
@@ -12784,11 +12835,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);
@@ -12866,6 +12917,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_I_ALT
   #undef PIPE_CONF_CHECK_FLAGS
   #undef PIPE_CONF_CHECK_CLOCK_FUZZY

--
Thanks,
--Ram

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux