Re: [Intel-gfx] [PATCH] i915/gem: Force HW tracking to exit PSR
-Original Message- From: Chris Wilson Sent: Friday, August 7, 2020 5:30 PM To: Singh, Gaurav K ; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] i915/gem: Force HW tracking to exit PSR Quoting Gaurav K Singh (2020-08-07 12:56:33) > Instead of calling i915_gem_object_invalidate_frontbuffer(), > call i915_gem_object_flush_frontbuffer() which will eventually call > psr_force_hw_tracking_exit(). This will force HW tracking to exit PSR > instead of disabling and re-enabling. set-domain is before the frontbuffer is dirtied. This is meant to be followed by either sw_finish (primarily for direct CPU access to the frontbuffer) or the more modern approach of marking the framebuffer as dirty (DRM_IOCTL_MODE_DIRTYFB). If you following the API, then we have a problem. But it sounds like it could be circumventing an important step. -Chris Hi Chris, Thanks for your comments. I was following the API and this issue is seen on various kind of PSR1 & PSR2 panels on older Gen and as well as on new Gen with older and latest drm-tip kernel. With this patch, the issue was resolved on all kinds of PSR panels across platforms. Can you please suggest a way to do it in kernel, not sure whether I have access to Application code. With regards, Gaurav ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix S0ix/S3 suspend stress issue
-Original Message-
From: Souza, Jose
Sent: Wednesday, September 18, 2019 11:14 PM
To: Singh, Gaurav K ; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix S0ix/S3 suspend stress issue
On Wed, 2019-09-18 at 11:23 +0530, Gaurav K Singh wrote:
> During S0ix/S3 suspend stress test on Cometlake chromebook, after few
> iterations we are seeing failure wrt PSR link CRC Error and stress
> test stops. This S0ix test is failing only when there is a CRC
> mismatch. In case of CRC mismatch, panel generates IRQ_HD and whenever
> there is CRC mismatch, we are disabling PSR2 in driver.
>
> By not disabling PSR2 in driver and only by writing 1 to clear sticky
> bit 0 in DPCD 0x2006 in panel,issue goes away.
> Completed 2500 S0ix/S3 test cycles on multiple CML chromebooks.
>
> As per EDP spec for CRC mismatch, nothing has been mentioned
> explicitly for Source device, only by writing 1 to clear sticky bit 0
> in DPCD 0x2006 in sink is mentioned.
>
> Signed-off-by: Gaurav K Singh
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 6 +-
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index b3c7eef53bf3..502e29dbbea9 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1325,15 +1325,11 @@ void intel_psr_short_pulse(struct intel_dp
> *intel_dp)
> if (val & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
> DRM_DEBUG_KMS("PSR VSC SDP uncorrectable error, disabling
> PSR\n");
> if (val & DP_PSR_LINK_CRC_ERROR)
> - DRM_ERROR("PSR Link CRC error, disabling PSR\n");
> + DRM_DEBUG_KMS("PSR Link CRC error, clearing PSR error
> status DPCD\n");
>
> if (val & ~errors)
> DRM_ERROR("PSR_ERROR_STATUS unhandled errors %x\n",
> val & ~errors);
> - if (val & errors) {
> - intel_psr_disable_locked(intel_dp);
> - psr->sink_not_reliable = true;
> - }
With this change you are breaking CRC and all the other error handling.
If CRC did not match, it means the image that was received by sink do not match
the expected, it could happen because of problems on sink, source or flat cable.
It is better have perfect frames than save power, so this is not acceptable.
Thanks Jose for your comments. There is issue with my thunderbird, using
outlook for the reply.
We do not see any such CRC issue on Innolux PSR2 panel but seeing only on AUO
PSR2 panel. Now since this issue can happen because of problems on sink, source
or flat cable, I see this as expected behavior from driver side. Please
comment.
With regards,
Gaurav
> /* clear status register */
> drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, val);
> exit:
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 06/25] drm/i915/dp: Validate modes using max Output BPP and slice count when DSC supported
On 9/12/2018 6:25 AM, Manasi Navare wrote:
When DSC is supported we need to validate the modes based on the
maximum supported compressed BPP and maximum supported slice count.
This allows us to allow the modes with pixel clock greater than the
available link BW as long as it meets the compressed BPP
and slice count requirements.
v3:
* Use the macro for dsc sink support (Jani N)
v2:
* Properly comment why we are right shifting the bpp value (Anusha)
Cc: Gaurav K Singh
Cc: Jani Nikula
Cc: Ville Syrjala
Cc: Anusha Srivatsa
Signed-off-by: Manasi Navare
Reviewed-by: Anusha Srivatsa
---
drivers/gpu/drm/i915/intel_dp.c | 31 ++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 719c2e426c28..63b7efa10a0f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -605,9 +605,12 @@ intel_dp_mode_valid(struct drm_connector *connector,
struct intel_dp *intel_dp = intel_attached_dp(connector);
struct intel_connector *intel_connector = to_intel_connector(connector);
struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
+ struct drm_i915_private *dev_priv = to_i915(connector->dev);
int target_clock = mode->clock;
int max_rate, mode_rate, max_lanes, max_link_clock;
int max_dotclk;
+ u16 dsc_max_output_bpp = 0;
+ u8 dsc_slice_count = 0;
if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
return MODE_NO_DBLESCAN;
@@ -630,7 +633,33 @@ intel_dp_mode_valid(struct drm_connector *connector,
max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
mode_rate = intel_dp_link_required(target_clock, 18);
- if (mode_rate > max_rate || target_clock > max_dotclk)
+ /*
+* Output bpp is stored in 6.4 format so right shift by 4 to get the
+* integer value since we support only integer values of bpp.
+*/
+ if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) &&
+ drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) {
+ if (intel_dp_is_edp(intel_dp)) {
+ dsc_max_output_bpp =
+ drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd)
>> 4;
+ dsc_slice_count =
+
drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
+ true);
+ } else {
+ dsc_max_output_bpp =
+ intel_dp_dsc_get_output_bpp(max_link_clock,
+ max_lanes,
+ target_clock,
+ mode->hdisplay) >>
4;
+ dsc_slice_count =
+ intel_dp_dsc_get_slice_count(intel_dp,
+target_clock,
+mode->hdisplay);
+ }
+ }
+
+ if ((mode_rate > max_rate && !(dsc_max_output_bpp && dsc_slice_count))
||
+ target_clock > max_dotclk)
return MODE_CLOCK_HIGH;
if (mode->clock < 1)
This patch looks good to me.
Reviewed-by: Gaurav K Singh
With regards,
Gaurav
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 05/25] drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC
On 9/12/2018 6:25 AM, Manasi Navare wrote:
This patch adds helpers for calculating the maximum compressed BPP
supported with small joiner.
This also adds a helper for calculating the slice count in case
of small joiner.
These are inside intel_dp since they take into account hardware
limitations.
v6:
* Take mode_clock and mode_hdisplay as input arguments
so that this can be called in intel_dp_mode_valid (Manasi)
v5:
* Get the max slice width from DPCD
* Check against Min_Slice_width of 2560 (Anusha)
v4:
* #defines for PPR in slice count helper (Gaurav)
v3:
* Simply logic for bpp (DK)
* Limit the valid slice count by max supported by Sink (Manasi)
v2:
* Change the small joiner RAM buffer constant as bspec changed (Manasi)
* rename it as SMALL_JOINER since we are not enabling big joiner yet (Anusha)
Cc: Gaurav K Singh
Cc: Jani Nikula
Cc: Ville Syrjala
Cc: Anusha Srivatsa
Cc: Dhinakaran Pandiyan
Signed-off-by: Manasi Navare
Reviewed-by: Anusha Srivatsa
---
drivers/gpu/drm/i915/intel_dp.c | 104 +++
drivers/gpu/drm/i915/intel_drv.h | 4 ++
2 files changed, 108 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1dfcceb55182..719c2e426c28 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -45,6 +45,17 @@
#define DP_DPRX_ESI_LEN 14
+/* DP DSC small joiner has 2 FIFOs each of 640 x 6 bytes */
+#define DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER 61440
+
+/* DP DSC throughput values used for slice count calculations KPixels/s */
+#define DP_DSC_PEAK_PIXEL_RATE 272
+#define DP_DSC_MAX_ENC_THROUGHPUT_034
+#define DP_DSC_MAX_ENC_THROUGHPUT_140
+
+/* DP DSC FEC Overhead factor = (100 - 2.4)/100 */
This comment is misleading to get the value of 976
+#define DP_DSC_FEC_OVERHEAD_FACTOR 976
+
/* Compliance test status bits */
#define INTEL_DP_RESOLUTION_SHIFT_MASK0
#define INTEL_DP_RESOLUTION_PREFERRED (1 << INTEL_DP_RESOLUTION_SHIFT_MASK)
@@ -93,6 +104,14 @@ static const struct dp_link_dpll chv_dpll[] = {
{ .p1 = 4, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } },
};
+/* Constants for DP DSC configurations */
+static const u8 valid_dsc_bpp[] = {6, 8, 10, 12, 15};
+
+/* With Single pipe configuration, HW is capable of supporting maximum
+ * of 4 slices per line.
+ */
+static const u8 valid_dsc_slicecount[] = {1, 2, 4};
+
/**
* intel_dp_is_edp - is the given port attached to an eDP panel (either CPU
or PCH)
* @intel_dp: DP struct
@@ -4080,6 +4099,91 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8
*sink_irq_vector)
DP_DPRX_ESI_LEN;
}
+uint16_t intel_dp_dsc_get_output_bpp(int link_clock, uint8_t lane_count,
+int mode_clock, int mode_hdisplay)
Can we use u16 here. I know that for functions defined earlier in this
file, we have used uint16_t. But since we are adding new functions, we
can follow u16 or u8 accordingly.
+{
+ u16 bits_per_pixel, max_bpp_small_joiner_ram;
+ int i;
+
+ /*
+* Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
+* (LinkSymbolClock)* 8 * ((100-FECOverhead)/100)*(TimeSlotsPerMTP)
+* FECOverhead = 2.4%, for SST -> TimeSlotsPerMTP is 1,
+* for MST -> TimeSlotsPerMTP has to be calculated
+*/
+ bits_per_pixel = (link_clock * lane_count * 8 *
+ DP_DSC_FEC_OVERHEAD_FACTOR) /
+ mode_clock;
+
+ /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
+ max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER /
+ mode_hdisplay;
+
+ /*
+* Greatest allowed DSC BPP = MIN (output BPP from avaialble Link BW
+* check, output bpp from small joiner RAM check)
+*/
+ bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
+
+ /* Error out if the max bpp is less than smallest allowed valid bpp */
+ if (bits_per_pixel < valid_dsc_bpp[0]) {
+ DRM_DEBUG_KMS("Unsupported BPP %d\n", bits_per_pixel);
+ return 0;
+ }
+
+ /* Find the nearest match in the array of known BPPs from VESA */
+ for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
+ if (bits_per_pixel < valid_dsc_bpp[i + 1])
+ break;
+ }
+ bits_per_pixel = valid_dsc_bpp[i];
+
+ /*
+* Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
+* fractional part is 0
+*/
+ return bits_per_pixel << 4;
+}
+
+uint8_t intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
+int mode_clock,
+int mode_hdisplay)
Same comment as above.
+{
+ u8 min_slice_count, i;
+ int max_slice_width;
+
+ if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
+
Re: [Intel-gfx] [PATCH v4 05/25] drm/i915/dp: Add helpers for Compressed BPP and Slice Count for DSC
On 9/12/2018 6:25 AM, Manasi Navare wrote:
This patch adds helpers for calculating the maximum compressed BPP
supported with small joiner.
This also adds a helper for calculating the slice count in case
of small joiner.
These are inside intel_dp since they take into account hardware
limitations.
v6:
* Take mode_clock and mode_hdisplay as input arguments
so that this can be called in intel_dp_mode_valid (Manasi)
v5:
* Get the max slice width from DPCD
* Check against Min_Slice_width of 2560 (Anusha)
v4:
* #defines for PPR in slice count helper (Gaurav)
v3:
* Simply logic for bpp (DK)
* Limit the valid slice count by max supported by Sink (Manasi)
v2:
* Change the small joiner RAM buffer constant as bspec changed (Manasi)
* rename it as SMALL_JOINER since we are not enabling big joiner yet (Anusha)
Cc: Gaurav K Singh
Cc: Jani Nikula
Cc: Ville Syrjala
Cc: Anusha Srivatsa
Cc: Dhinakaran Pandiyan
Signed-off-by: Manasi Navare
Reviewed-by: Anusha Srivatsa
---
drivers/gpu/drm/i915/intel_dp.c | 104 +++
drivers/gpu/drm/i915/intel_drv.h | 4 ++
2 files changed, 108 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1dfcceb55182..719c2e426c28 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -45,6 +45,17 @@
#define DP_DPRX_ESI_LEN 14
+/* DP DSC small joiner has 2 FIFOs each of 640 x 6 bytes */
+#define DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER 61440
+
+/* DP DSC throughput values used for slice count calculations KPixels/s */
+#define DP_DSC_PEAK_PIXEL_RATE 272
+#define DP_DSC_MAX_ENC_THROUGHPUT_034
+#define DP_DSC_MAX_ENC_THROUGHPUT_140
+
+/* DP DSC FEC Overhead factor = (100 - 2.4)/100 */
This comment is misleading to get the value of 976
+#define DP_DSC_FEC_OVERHEAD_FACTOR 976
+
/* Compliance test status bits */
#define INTEL_DP_RESOLUTION_SHIFT_MASK0
#define INTEL_DP_RESOLUTION_PREFERRED (1 << INTEL_DP_RESOLUTION_SHIFT_MASK)
@@ -93,6 +104,14 @@ static const struct dp_link_dpll chv_dpll[] = {
{ .p1 = 4, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } },
};
+/* Constants for DP DSC configurations */
+static const u8 valid_dsc_bpp[] = {6, 8, 10, 12, 15};
+
+/* With Single pipe configuration, HW is capable of supporting maximum
+ * of 4 slices per line.
+ */
+static const u8 valid_dsc_slicecount[] = {1, 2, 4};
+
/**
* intel_dp_is_edp - is the given port attached to an eDP panel (either CPU
or PCH)
* @intel_dp: DP struct
@@ -4080,6 +4099,91 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8
*sink_irq_vector)
DP_DPRX_ESI_LEN;
}
+uint16_t intel_dp_dsc_get_output_bpp(int link_clock, uint8_t lane_count,
+int mode_clock, int mode_hdisplay)
For all the new functions being added, can we use u16 instead of
uint16_t. I know for the fact that in this file, uint16_t has been used
in the functions which were defined earlier. But since we are adding new
functions, we can use u16 or u8 accordingly.
+{
+ u16 bits_per_pixel, max_bpp_small_joiner_ram;
+ int i;
+
+ /*
+* Available Link Bandwidth(Kbits/sec) = (NumberOfLanes)*
+* (LinkSymbolClock)* 8 * ((100-FECOverhead)/100)*(TimeSlotsPerMTP)
+* FECOverhead = 2.4%, for SST -> TimeSlotsPerMTP is 1,
+* for MST -> TimeSlotsPerMTP has to be calculated
+*/
+ bits_per_pixel = (link_clock * lane_count * 8 *
+ DP_DSC_FEC_OVERHEAD_FACTOR) /
+ mode_clock;
+
+ /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
+ max_bpp_small_joiner_ram = DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER /
+ mode_hdisplay;
+
+ /*
+* Greatest allowed DSC BPP = MIN (output BPP from avaialble Link BW
+* check, output bpp from small joiner RAM check)
+*/
+ bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
+
+ /* Error out if the max bpp is less than smallest allowed valid bpp */
+ if (bits_per_pixel < valid_dsc_bpp[0]) {
+ DRM_DEBUG_KMS("Unsupported BPP %d\n", bits_per_pixel);
+ return 0;
+ }
+
+ /* Find the nearest match in the array of known BPPs from VESA */
+ for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
+ if (bits_per_pixel < valid_dsc_bpp[i + 1])
+ break;
+ }
+ bits_per_pixel = valid_dsc_bpp[i];
+
+ /*
+* Compressed BPP in U6.4 format so multiply by 16, for Gen 11,
+* fractional part is 0
+*/
+ return bits_per_pixel << 4;
+}
+
+uint8_t intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
+int mode_clock,
+int mode_hdisplay)
Same comment as above.
+{
+ u8 min_slice_count, i;
+
Re: [Intel-gfx] [PATCH v4 04/25] drm/dp: DRM DP helper/macros to get DP sink DSC parameters
On 9/12/2018 6:25 AM, Manasi Navare wrote:
This patch adds inline functions and helpers for obtaining
DP sink's supported DSC parameters like DSC sink support,
eDP compressed BPP supported, maximum slice count supported
by the sink devices, DSC line buffer bit depth supported on DP sink,
DSC sink maximum color depth by parsing corresponding DPCD registers.
v4:
* Add helper to give line buf bit depth (Manasi)
* Correct the bit masking in color depth helper (manasi)
v3:
* Use SLICE_CAP_2 for DP (Anusha)
v2:
* Add DSC sink support macro (Jani N)
Cc: Gaurav K Singh
Cc: dri-de...@lists.freedesktop.org
Cc: Jani Nikula
Cc: Ville Syrjala
Cc: Anusha Srivatsa
Signed-off-by: Manasi Navare
Reviewed-by: Anusha Srivatsa
This patch looks good to me.
Reviewed-by: Gaurav K Singh
---
drivers/gpu/drm/drm_dp_helper.c | 90 +
include/drm/drm_dp_helper.h | 30 +++
2 files changed, 120 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 8c6b9fd89f8a..5d5879f115ce 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1337,3 +1337,93 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct
drm_dp_desc *desc,
return 0;
}
EXPORT_SYMBOL(drm_dp_read_desc);
+
+/**
+ * DRM DP Helpers for DSC
+ */
+u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
+ bool is_edp)
+{
+ u8 slice_cap1 = dsc_dpcd[DP_DSC_SLICE_CAP_1 - DP_DSC_SUPPORT];
+
+ if (is_edp) {
+ /* For eDP, register DSC_SLICE_CAPABILITIES_1 gives slice count
*/
+ if (slice_cap1 & DP_DSC_4_PER_DP_DSC_SINK)
+ return 4;
+ if (slice_cap1 & DP_DSC_2_PER_DP_DSC_SINK)
+ return 2;
+ if (slice_cap1 & DP_DSC_1_PER_DP_DSC_SINK)
+ return 1;
+ } else {
+ /* For DP, use values from DSC_SLICE_CAP_1 and DSC_SLICE_CAP2 */
+ u8 slice_cap2 = dsc_dpcd[DP_DSC_SLICE_CAP_2 - DP_DSC_SUPPORT];
+
+ if (slice_cap2 & DP_DSC_24_PER_DP_DSC_SINK)
+ return 24;
+ if (slice_cap2 & DP_DSC_20_PER_DP_DSC_SINK)
+ return 20;
+ if (slice_cap2 & DP_DSC_16_PER_DP_DSC_SINK)
+ return 16;
+ if (slice_cap1 & DP_DSC_12_PER_DP_DSC_SINK)
+ return 12;
+ if (slice_cap1 & DP_DSC_10_PER_DP_DSC_SINK)
+ return 10;
+ if (slice_cap1 & DP_DSC_8_PER_DP_DSC_SINK)
+ return 8;
+ if (slice_cap1 & DP_DSC_6_PER_DP_DSC_SINK)
+ return 6;
+ if (slice_cap1 & DP_DSC_4_PER_DP_DSC_SINK)
+ return 4;
+ if (slice_cap1 & DP_DSC_2_PER_DP_DSC_SINK)
+ return 2;
+ if (slice_cap1 & DP_DSC_1_PER_DP_DSC_SINK)
+ return 1;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_dsc_sink_max_slice_count);
+
+u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
+{
+ u8 line_buf_depth = dsc_dpcd[DP_DSC_LINE_BUF_BIT_DEPTH -
DP_DSC_SUPPORT];
+
+ switch (line_buf_depth & DP_DSC_LINE_BUF_BIT_DEPTH_MASK) {
+ case DP_DSC_LINE_BUF_BIT_DEPTH_9:
+ return 9;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_10:
+ return 10;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_11:
+ return 11;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_12:
+ return 12;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_13:
+ return 13;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_14:
+ return 14;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_15:
+ return 15;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_16:
+ return 16;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_8:
+ return 8;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
+
+u8 drm_dp_dsc_sink_max_color_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
+{
+ u8 color_depth = dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP - DP_DSC_SUPPORT];
+
+ if (color_depth & DP_DSC_12_BPC)
+ return 12;
+ if (color_depth & DP_DSC_10_BPC)
+ return 10;
+ if (color_depth & DP_DSC_8_BPC)
+ return 8;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_dsc_sink_max_color_depth);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 7f6237cad10d..ce6297908fd6 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1065,6 +1065,36 @@ drm_dp_is_branch(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
return dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT;
}
+/* DP/eDP DSC support */
+u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
+
Re: [Intel-gfx] [PATCH v4 02/25] drm/dp: Add DP DSC DPCD receiver capability size define and missing SHIFT
On 9/12/2018 6:25 AM, Manasi Navare wrote: This patch defines the DP DSC receiver capability size that gives total number of DP DSC DPCD registers. This also adds a missing #defines for DP DSC support missed in the commit id (ab6a46ea6842ce "Add DPCD definitions for DP 1.4 DSC feature") v3: * MIN_SLICE_WIDTH = 2560 (Anusha) * Define DP_DSC_SLICE_WIDTH_MULTIPLIER = 320 v2: * Add SHIFT define and DECOMPRESSION_EN define missed in prev patch Cc: dri-de...@lists.freedesktop.org Cc: Jani Nikula Cc: Ville Syrjala Cc: Anusha Srivatsa Cc: Gaurav K Singh Signed-off-by: Manasi Navare Reviewed-by: Anusha Srivatsa The patch looks good to me. Reviewed-by: Gaurav K Singh --- include/drm/drm_dp_helper.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 698082a02b97..7f6237cad10d 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -231,6 +231,8 @@ #define DP_DSC_MAX_BITS_PER_PIXEL_LOW 0x067 /* eDP 1.4 */ #define DP_DSC_MAX_BITS_PER_PIXEL_HI0x068 /* eDP 1.4 */ +# define DP_DSC_MAX_BITS_PER_PIXEL_HI_MASK (0x3 << 0) +# define DP_DSC_MAX_BITS_PER_PIXEL_HI_SHIFT 8 #define DP_DSC_DEC_COLOR_FORMAT_CAP 0x069 # define DP_DSC_RGB (1 << 0) @@ -279,6 +281,8 @@ # define DP_DSC_THROUGHPUT_MODE_1_1000 (14 << 4) #define DP_DSC_MAX_SLICE_WIDTH 0x06C +#define DP_DSC_MIN_SLICE_WIDTH_VALUE2560 +#define DP_DSC_SLICE_WIDTH_MULTIPLIER 320 #define DP_DSC_SLICE_CAP_2 0x06D # define DP_DSC_16_PER_DP_DSC_SINK (1 << 0) @@ -477,6 +481,7 @@ # define DP_AUX_FRAME_SYNC_VALID (1 << 0) #define DP_DSC_ENABLE 0x160 /* DP 1.4 */ +# define DP_DECOMPRESSION_EN(1 << 0) #define DP_PSR_EN_CFG 0x170 /* XXX 1.2? */ # define DP_PSR_ENABLE(1 << 0) @@ -963,6 +968,7 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI #define DP_BRANCH_OUI_HEADER_SIZE 0xc #define DP_RECEIVER_CAP_SIZE 0xf +#define DP_DSC_RECEIVER_CAP_SIZE0xf #define EDP_PSR_RECEIVER_CAP_SIZE 2 #define EDP_DISPLAY_CTL_CAP_SIZE 3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 02/23] drm/i915/dp: Cache the DP/eDP DSC DPCD register set on Hotplug/eDP Init
On 8/1/2018 2:36 AM, Manasi Navare wrote:
DSC is supported on eDP starting GEN 10 display and on DP starting
GEN 11.
This patch implements the discovery phase of DSC. On hotplug,
source reads the DSC DPCD register set (0x00060 - 0x006F) to
Please correct it to 0x0006F in order to match the spec
read the decompression capabilities of the sink device.
This entire block of registers is cached in intel_dp so that
capability information can be used during DSC configuration
phase during compute_config phase of the modeset.
For eDP, this caching happens during the eDP initialization.
This caching is done only for eDP and DP rev >= 1.4
v5:
* Fix the block comment (Gaurav)
* Use DRM_ERROR for dpcd_read fail (Gaurav,Anusha)
v4:
* Cache these only for Gen >= 11
v3:
* Remove the dsc_sink_support field in intel_dp (Jani N)
v2:
* Clear the cached registers on hotplug always (Jani N)
* Combine the eDP and DP caching in same function (Jani N)
Cc: Jani Nikula
Cc: Ville Syrjala
Cc: Daniel Vetter
Cc: Anusha Srivatsa
Cc: Gaurav K Singh
Signed-off-by: Manasi Navare
Except the above comment, this patch looks good to me..
Reviewed-by: Gaurav K Singh
---
drivers/gpu/drm/i915/intel_dp.c | 32
drivers/gpu/drm/i915/intel_drv.h | 1 +
2 files changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8e0e14b..afa4e2d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3877,6 +3877,29 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
return intel_dp->dpcd[DP_DPCD_REV] != 0;
}
+static void intel_dp_get_dsc_sink_cap(struct intel_dp *intel_dp)
+{
+ /*
+*Clear the cached register set to avoid using stale values
+* for the sinks that do not support DSC.
+*/
+ memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
+
+ /* Cache the DSC DPCD if eDP or DP rev >= 1.4 */
+ if (intel_dp->dpcd[DP_DPCD_REV] >= 0x14 ||
+ intel_dp->edp_dpcd[0] >= DP_EDP_14) {
+ if (drm_dp_dpcd_read(&intel_dp->aux, DP_DSC_SUPPORT,
+intel_dp->dsc_dpcd,
+sizeof(intel_dp->dsc_dpcd)) < 0)
+ DRM_ERROR("Failed to read DPCD register 0x%x\n",
+ DP_DSC_SUPPORT);
+
+ DRM_DEBUG_KMS("DSC DPCD: %*ph\n",
+ (int)sizeof(intel_dp->dsc_dpcd),
+ intel_dp->dsc_dpcd);
+ }
+}
+
static bool
intel_edp_init_dpcd(struct intel_dp *intel_dp)
{
@@ -3953,6 +3976,10 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
intel_dp_set_common_rates(intel_dp);
+ /* Read the eDP DSC DPCD registers */
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+ intel_dp_get_dsc_sink_cap(intel_dp);
+
return true;
}
@@ -4944,6 +4971,7 @@ intel_dp_long_pulse(struct intel_connector *connector)
if (status == connector_status_disconnected) {
memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
+ memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
if (intel_dp->is_mst) {
DRM_DEBUG_KMS("MST device may have disappeared %d vs
%d\n",
@@ -4969,6 +4997,10 @@ intel_dp_long_pulse(struct intel_connector *connector)
intel_dp_print_rates(intel_dp);
+ /* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
+ if (INTEL_GEN(dev_priv) >= 11)
+ intel_dp_get_dsc_sink_cap(intel_dp);
+
drm_dp_read_desc(&intel_dp->aux, &intel_dp->desc,
drm_dp_is_branch(intel_dp->dpcd));
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 99a5f5b..29abe7a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1070,6 +1070,7 @@ struct intel_dp {
uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
uint8_t edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
+ u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE];
/* source rates */
int num_source_rates;
const int *source_rates;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/audio: Fix audio detection issue on GLK
On 4/17/2018 11:58 PM, Du,Wenkai wrote: On 4/17/2018 11:22 AM, Gaurav K Singh wrote: On Geminilake, sometimes audio card is not getting detected after reboot. This is a spurious issue happening on Geminilake. HW codec and HD audio controller link was going out of sync for which there was a fix in i915 driver but was not getting invoked for GLK. Extending this fix to GLK as well. Tested by Du,Wenkai on GLK board. Bspec: 21829 v2: Instead of checking GEN9_BC, BXT and GLK macros, use IS_GEN9 macro (Jani N) Signed-off-by: Gaurav K Singh Reviewed-by: Abhay Kumar Reviewed-by: Wenkai Du Tested-by: Wenkai Du Regards, Wenkai Hi Jani, Could you please help in merging this patch to unblock audio. With regards, Gaurav --- drivers/gpu/drm/i915/intel_audio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 656f6c931341..3ea566f99450 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -729,7 +729,7 @@ static void i915_audio_component_codec_wake_override(struct device *kdev, struct drm_i915_private *dev_priv = kdev_to_i915(kdev); u32 tmp; - if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv)) + if (!IS_GEN9(dev_priv)) return; i915_audio_component_get_power(kdev); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/audio: Fix audio detection issue on GLK
On 4/10/2018 4:32 PM, Jani Nikula wrote: On Mon, 09 Apr 2018, "Kumar, Abhay" wrote: On 4/9/2018 4:20 PM, Pandiyan, Dhinakaran wrote: On Mon, 2018-04-09 at 12:18 -0700, Kumar, Abhay wrote: On 4/9/2018 12:10 PM, Rodrigo Vivi wrote: On Mon, Apr 09, 2018 at 05:07:31PM +0300, Jani Nikula wrote: On Sun, 08 Apr 2018, Gaurav K Singh wrote: On Geminilake, sometimes audio card is not getting detected after reboot. This is a spurious issue happening on Geminilake. HW codec and HD audio controller link was going out of sync for which there was a fix in i915 driver but was not getting invoked for GLK. Extending this fix to GLK as well. Tested by Du,Wenkai on GLK board. Signed-off-by: Gaurav K Singh --- drivers/gpu/drm/i915/intel_audio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 656f6c931341..73b1e0b96f88 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -729,7 +729,8 @@ static void i915_audio_component_codec_wake_override(struct device *kdev, struct drm_i915_private *dev_priv = kdev_to_i915(kdev); u32 tmp; - if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv)) + if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv) && + !IS_GEMINILAKE(dev_priv)) That could be written as if (!IS_GEN9_BC(dev_priv) && !IS_GEN9_LP(dev_priv)) which in turn could just be written as if (!IS_GEN9(dev_priv)) ...but since GLK has gen 10 display, so I'm wondering if the same issue will be present in gen 10 too, and whether this should just become if (INTEL_GEN(dev_priv) < 9) +1. I opened here to exactly add same comment. I am checking with DINQ and without this patch for GLK it can enumerate HDA codec. Ofcourse after cdclk fix. How about the other way around? i.e., does codec enumeration work this patch but without the cdclk change? Nop. with DINQ we need to have cdclk change to make Codec detection work. With or without this patch. Basically what you're saying is that this patch is not needed? Gaurav, can you check with the CDCLK patch [1] if that fixes the issue for you? BR, Jani. Jani, Our team tried my patch (not including Abhay's patch) which fixed the issue. But will also try Abhay's patch(this time without my patch), Will update accordingly. With regards, Gaurav PS. The CDCLK patch is not enough to fix the issue completely (probe without display outputs will still choose a low CDCLK) but that's work in progress. [1] http://patchwork.freedesktop.org/patch/msgid/1508968932-32208-1-git-send-email-abhay.ku...@intel.com BR, Jani. return; i915_audio_component_get_power(kdev); -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
On 3/5/2018 11:51 PM, Pandiyan, Dhinakaran wrote: On Thu, 2018-01-04 at 00:48 +0530, Gaurav K Singh wrote: From: Gaurav Singh On Apollolake, with stress test warm reboot, audio card was not getting enumerated after reboot. This was a spurious issue happening on Apollolake. HW codec and HD audio controller link was going out of sync for which there was a fix in i915 driver but was not getting invoked for BXT. Extending this fix to BXT as well. Tested on apollolake chromebook by stress test warm reboot with 2500 iterations. Signed-off-by: Gaurav K Singh Might be worth adding Bspec: 21829 to the commit message. Reviewed-by: Dhinakaran Pandiyan Please rebase and send this patch to the list to CI it. Hi DK, Sure, i have added Bspec index and sent the updated patch. https://patchwork.freedesktop.org/patch/215067/ With regards, Gaurav --- drivers/gpu/drm/i915/intel_audio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index f1502a0188eb..c71c04e1c3f6 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -729,7 +729,7 @@ static void i915_audio_component_codec_wake_override(struct device *kdev, struct drm_i915_private *dev_priv = kdev_to_i915(kdev); u32 tmp; - if (!IS_GEN9_BC(dev_priv)) + if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv)) return; i915_audio_component_get_power(kdev); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
On 3/5/2018 12:13 PM, Mullah, Abid A wrote:
Did u check the fix on bxt and if u didnt see any issue then u can go ahead. I
will update bspec tomorrow
Sent from my iPhone
Tested on apollolake chromebook by stress test warm reboot with 2500
iterations and did not see any issue.
With regards,
Gaurav
On Mar 4, 2018, at 10:09 PM, Singh, Gaurav K wrote:
On 2/27/2018 11:00 PM, Runyan, Arthur J wrote:
Ok, please update the workaround page to show all the impacted projects
https://gfxspecs.intel.com/Predator/Home/Index/21829
Hi,
Should we wait for Bspec to get updated or we can go ahead with the merge.
Please advise.
With regards,
Gaurav
-Original Message-
From: Mullah, Abid A
Sent: Tuesday, 27 February, 2018 8:52 AM
To: Runyan, Arthur J ; Pandiyan, Dhinakaran
; Singh, Gaurav K
; Neelagandan, Harigaran
Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo ;
Nikula, Jani
Subject: RE: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
Yes. It will be needed for BXT also.
-Original Message-
From: Runyan, Arthur J
Sent: Tuesday, February 27, 2018 8:49 AM
To: Pandiyan, Dhinakaran ; Singh, Gaurav
K ; Mullah, Abid A ;
Neelagandan, Harigaran
Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo ;
Nikula, Jani
Subject: RE: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
Abid or Hari, please check. There was a workaround to set
AUD_CHICKENBIT_REG bit 15 (Codec Wake overwrite to DACFEUNIT) on SKL
and KBL. Does it apply to BXT also?
-Original Message-
From: Pandiyan, Dhinakaran
Sent: Monday, 26 February, 2018 6:04 PM
To: Runyan, Arthur J ; Singh, Gaurav K
Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo
; Nikula, Jani
Subject: RE: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
-Original Message-
From: Runyan, Arthur J
Sent: Tuesday, January 9, 2018 11:55 AM
To: Pandiyan, Dhinakaran ; Singh,
Gaurav K
Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo
Subject: RE: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
Sorry, I've been out. I'm checking on this.
Hi Art,
Is AUD_CHICKENBIT_REG:15 needed for BXT to fix code enumeration
issues?
-DK
-Original Message-
From: Pandiyan, Dhinakaran
Sent: Thursday, 4 January, 2018 2:00 PM
To: Singh, Gaurav K
Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo
; subransu.s.pru...@intel.com; Runyan,
Arthur J
Subject: Re: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
+Art
On Thu, 2018-01-04 at 22:13 +0530, Singh, Gaurav K wrote:
On 1/4/2018 2:48 AM, Rodrigo Vivi wrote:
On Wed, Jan 03, 2018 at 08:31:10PM +, Pandiyan, Dhinakaran
wrote:
On Thu, 2018-01-04 at 00:48 +0530, Gaurav K Singh wrote:
From: Gaurav Singh
On Apollolake, with stress test warm reboot, audio card was
not getting enumerated after reboot. This was a
The problem looks similar to
https://lists.freedesktop.org/archives/intel-gfx/2017-October/1
4449
5.html
although the proposed solutions are vastly different. I have
Cc'd some more people.
spurious issue happening on Apollolake. HW codec and HD audio
controller link was going out of sync for which there was a
fix in
i915 driver but was not getting invoked for BXT. Extending
this fix to BXT as well.
Tested on apollolake chromebook by stress test warm reboot
with
2500 iterations.
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/intel_audio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_audio.c
b/drivers/gpu/drm/i915/intel_audio.c
index f1502a0188eb..c71c04e1c3f6 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -729,7 +729,7 @@ static void
i915_audio_component_codec_wake_override(struct device *kdev,
struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
u32 tmp;
-if (!IS_GEN9_BC(dev_priv))
+if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv))
IS_GEN9()? GLK might need this too.
I think this is applicable for all Gen9 platforms.
if GLK need there is the possibility of CNL also needing it...
So not sure where to stop.
Also looking to the original patch that introduced this
function, commit '632f3ab95fe2 ("drm/i915/audio: add codec
wakeup override enabled/disable callback")'
it tells that the reason was:
"In SKL, HDMI/DP codec and PCH HD Audio Controller are in
different p$ wells, so it's necessary to reset display audio
codecs when power we$ otherwise display audio codecs will
disappear when resume from low p$ state."
Is this the case here on BXT?
Yes, its the same case with BXT.
Another interesting thing I noticed on Spec when searching for
this bit was that this bit is related to an workaround on SKL/KBL/CFL...
no mention to BXT.
"This workaround is needed for an HW issue in SKL and KBL in
which HW codec and HD audio controller link was going out of sync."
Yes, in Bspec it has been mentioned only for SKL and KBL. But
without this fix, sound card wa
Re: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
On 2/27/2018 11:00 PM, Runyan, Arthur J wrote:
Ok, please update the workaround page to show all the impacted projects
https://gfxspecs.intel.com/Predator/Home/Index/21829
Hi,
Should we wait for Bspec to get updated or we can go ahead with the
merge. Please advise.
With regards,
Gaurav
-Original Message-
From: Mullah, Abid A
Sent: Tuesday, 27 February, 2018 8:52 AM
To: Runyan, Arthur J ; Pandiyan, Dhinakaran
; Singh, Gaurav K
; Neelagandan, Harigaran
Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo ;
Nikula, Jani
Subject: RE: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
Yes. It will be needed for BXT also.
-Original Message-
From: Runyan, Arthur J
Sent: Tuesday, February 27, 2018 8:49 AM
To: Pandiyan, Dhinakaran ; Singh, Gaurav
K ; Mullah, Abid A ;
Neelagandan, Harigaran
Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo ;
Nikula, Jani
Subject: RE: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
Abid or Hari, please check. There was a workaround to set
AUD_CHICKENBIT_REG bit 15 (Codec Wake overwrite to DACFEUNIT) on SKL
and KBL. Does it apply to BXT also?
-Original Message-
From: Pandiyan, Dhinakaran
Sent: Monday, 26 February, 2018 6:04 PM
To: Runyan, Arthur J ; Singh, Gaurav K
Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo
; Nikula, Jani
Subject: RE: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
-Original Message-
From: Runyan, Arthur J
Sent: Tuesday, January 9, 2018 11:55 AM
To: Pandiyan, Dhinakaran ; Singh,
Gaurav K
Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo
Subject: RE: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
Sorry, I've been out. I'm checking on this.
Hi Art,
Is AUD_CHICKENBIT_REG:15 needed for BXT to fix code enumeration
issues?
-DK
-Original Message-
From: Pandiyan, Dhinakaran
Sent: Thursday, 4 January, 2018 2:00 PM
To: Singh, Gaurav K
Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo
; subransu.s.pru...@intel.com; Runyan,
Arthur J
Subject: Re: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
+Art
On Thu, 2018-01-04 at 22:13 +0530, Singh, Gaurav K wrote:
On 1/4/2018 2:48 AM, Rodrigo Vivi wrote:
On Wed, Jan 03, 2018 at 08:31:10PM +, Pandiyan, Dhinakaran
wrote:
On Thu, 2018-01-04 at 00:48 +0530, Gaurav K Singh wrote:
From: Gaurav Singh
On Apollolake, with stress test warm reboot, audio card was
not getting enumerated after reboot. This was a
The problem looks similar to
https://lists.freedesktop.org/archives/intel-gfx/2017-October/1
4449
5.html
although the proposed solutions are vastly different. I have
Cc'd some more people.
spurious issue happening on Apollolake. HW codec and HD audio
controller link was going out of sync for which there was a
fix in
i915 driver but was not getting invoked for BXT. Extending
this fix to BXT as well.
Tested on apollolake chromebook by stress test warm reboot
with
2500 iterations.
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/intel_audio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_audio.c
b/drivers/gpu/drm/i915/intel_audio.c
index f1502a0188eb..c71c04e1c3f6 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -729,7 +729,7 @@ static void
i915_audio_component_codec_wake_override(struct device *kdev,
struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
u32 tmp;
- if (!IS_GEN9_BC(dev_priv))
+ if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv))
IS_GEN9()? GLK might need this too.
I think this is applicable for all Gen9 platforms.
if GLK need there is the possibility of CNL also needing it...
So not sure where to stop.
Also looking to the original patch that introduced this
function, commit '632f3ab95fe2 ("drm/i915/audio: add codec
wakeup override enabled/disable callback")'
it tells that the reason was:
"In SKL, HDMI/DP codec and PCH HD Audio Controller are in
different p$ wells, so it's necessary to reset display audio
codecs when power we$ otherwise display audio codecs will
disappear when resume from low p$ state."
Is this the case here on BXT?
Yes, its the same case with BXT.
Another interesting thing I noticed on Spec when searching for
this bit was that this bit is related to an workaround on SKL/KBL/CFL...
no mention to BXT.
"This workaround is needed for an HW issue in SKL and KBL in
which HW codec and HD audio controller link was going out of sync."
Yes, in Bspec it has been mentioned only for SKL and KBL. But
without this fix, sound card was not getting enumerated for BXT.
Art,
Can you please help us here? To summarize, the question is what
platforms
need
the AUD_CHICKENBIT_REG:15 bit to be set to avoid code enumeration
failures?
-DK
Thanks,
Rodrigo.
return;
i915_audio_component_get_power(kdev);
Re: [Intel-gfx] [PATCH 01/10] drm: i915: Defining Compression Capabilities
On 2/24/2018 5:24 AM, Manasi Navare wrote:
On Fri, Feb 23, 2018 at 09:25:44PM +0530, Gaurav K Singh wrote:
For Vesa Display Stream compression, defining structures for
compression capabilities to be stored in encoder.
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/i915_drv.h | 125 +++
drivers/gpu/drm/i915/intel_drv.h | 62 +++
include/drm/drm_dp_helper.h | 1 +
3 files changed, 188 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0d8cb74e7d02..4b1c323c0925 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -780,6 +780,131 @@ struct i915_psr {
void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
};
+/* DSC Configuration structure */
+#define NUM_BUF_RANGES 15
+
+/* Configuration for a single Rate Control model range */
+struct rc_range_parameters {
+ /* Min Quantization Parameters allowed for this range */
+ unsigned long range_min_qp;
Its only a 5 bit value, so uint_8 should be good, why have a unsigned long.
Same for max_qp which is 5 bits and bpg_offset which is 6 bits.
Please consider these for the parameters below.
My bad, will take care in next patchset.
+ /* Max Quantization Parameters allowed for this range */
+ unsigned long range_max_qp;
+ /* Bits/group offset to apply to target for this group */
+ unsigned long range_bpg_offset;
+};
+
+struct vdsc_config {
+ /* Bits / component for previous reconstructed line buffer */
+ unsigned long line_buf_depth;
+ /*
+* Rate control buffer size (in bits); not in PPS,
+* used only in C model for checking overflow
+*/
+ unsigned long rc_bits;
+ /* Bits per component to code (must be 8, 10, or 12) */
+ unsigned long bits_per_component;
+ /*
+* Flag indicating to do RGB - YCoCg conversion
+* and back (should be 1 for RGB input)
+*/
+ bool convert_rgb;
+ unsigned long slice_count;
For eDP, it can be max 4, why unsigned long?
Will take care.
+ /* Slice Width */
+ unsigned long slice_width;
+ /* Slice Height */
+ unsigned long slice_height;
+ /*
+* 4:2:2 enable mode (from PPS, 4:2:2 conversion happens
+* outside of DSC encode/decode algorithm)
+*/
+ bool enable422;
+ /* Picture Width */
+ unsigned long pic_width;
+ /* Picture Height */
+ unsigned long pic_height;
+ /* Offset to bits/group used by RC to determine QP adjustment */
+ unsigned long rc_tgt_offset_high;
+ /* Offset to bits/group used by RC to determine QP adjustment */
+ unsigned long rc_tgt_offset_low;
+ /* Bits/pixel target << 4 (ie., 4 fractional bits) */
+ unsigned long bits_per_pixel;
+ /*
+* Factor to determine if an edge is present based
+* on the bits produced
+*/
+ unsigned long rc_edge_factor;
+ /* Slow down incrementing once the range reaches this value */
+ unsigned long rc_quant_incr_limit1;
+ /* Slow down incrementing once the range reaches this value */
+ unsigned long rc_quant_incr_limit0;
+ /* Number of pixels to delay the initial transmission */
+ unsigned long initial_xmit_delay;
+ /* Number of pixels to delay the VLD on the decoder,not including SSM */
+ unsigned long initial_dec_delay;
+ /* Block prediction range (in pixels) */
+ bool block_pred_enable;
+ /* Bits/group offset to use for first line of the slice */
+ unsigned long first_line_bpg_Ofs;
+ /* Value to use for RC model offset at slice start */
+ unsigned long initial_offset;
+ /* X position in the picture of top-left corner of slice */
+ unsigned long x_start;
+ /* Y position in the picture of top-left corner of slice */
+ unsigned long y_start;
+ /* Thresholds defining each of the buffer ranges */
+ unsigned long rc_buf_thresh[NUM_BUF_RANGES - 1];
+ /* Parameters for each of the RC ranges */
+ struct rc_range_parameters rc_range_params[NUM_BUF_RANGES];
+ /* Total size of RC model */
+ unsigned long rc_model_size;
+ /* Minimum QP where flatness information is sent */
+ unsigned long flatness_minQp;
+ /* Maximum QP where flatness information is sent */
+ unsigned long flatness_maxQp;
+ /*
+* MAX-MIN for all components is required to
+* be <= this value for flatness to be used
+*/
+ unsigned long flatness_det_thresh;
+ /* Initial value for scale factor */
+ unsigned long initial_scale_value;
+ /* Decrement scale factor every scale_decrement_interval groups */
+ unsigned long scale_decrement_interval;
+ /* Increment scale factor every scale_increment_interval groups */
+ unsigned long sca
Re: [Intel-gfx] [PATCH 00/10] Enabling VDSC in i915 driver for GLK
On 2/24/2018 4:23 AM, Manasi Navare wrote: Thanks for the patches. I am working on the DSC support on i915 for eDP/DP as well. Looking at the patches below, this is specific to VDSC enabling for eDP panels and not for the external DP. So please mention that specifically in the cover letter as well. Since most of the VDSC functionality will be same across EDP and DP, i mentioned DP in generic terms. But no worries, i will mention EDP explicitly in the next patch set while fixing the review comments. On Fri, Feb 23, 2018 at 09:25:43PM +0530, Gaurav K Singh wrote: Display manufacturers are turning to higher-resolution displays to differentiate their products. The increased pixel counts have required increased bandwidth over the links that drive these displays. However, advances in physical layer technology have not kept up with the increases in pixel counts. These factors have created a need for compression on display links. The Video Electronics Standards Association(VESA),in liaison with the MIPI Alliance, has developed an industry standard Display Stream Compression(DSC) for interoperable, visually lossless compression over display links. These patches enable VDSC in i915 gfx driver for Gen9,Gen10 platforms Please specify that this enables VDSC for eDp in i915 gfx driver. Sure, will take care. and provide basic code for future platforms. Testing: Did testing on GLK RVP. By default GLK RVP has non-DSC EDP panel, there was no regression with these patches. BA Chrome Team (OTC) do not have EDP panel which supports DSC. Trying to arrrage DSC EDP panel from other teams in BA, hopeful to get it in few weeks. I do have a DSC eDP panel here in Oregon and can volunteer for testing your patches with that on GLK RVP. Manasi I am hopeful to get the panel sometime late next week. If i am not able to get it, I would surely take your help to test my patches. Dropping the patches to get the review started. Gaurav K Singh (10): drm: i915: Defining Compression Capabilities drm: i915: Get DSC capability from DP sink drm: i915: Enable/Disable DSC in DP sink drm: i915: Compute RC & DSC parameters drm: i915: Define Picture Parameter Set drm/i915: Populate PPS Secondary Data Pkt for Sink drm: i915: Define VDSC regs and DSC params drm: i915: Enable VDSC in Source drm: i915: Disable VDSC from Source drm/i915: Encoder enable/disable seq wrt DSC drivers/gpu/drm/i915/Makefile|1 + drivers/gpu/drm/i915/i915_drv.h | 589 drivers/gpu/drm/i915/i915_reg.h | 451 drivers/gpu/drm/i915/intel_ddi.c |4 + drivers/gpu/drm/i915/intel_display.c | 20 + drivers/gpu/drm/i915/intel_dp.c | 182 + drivers/gpu/drm/i915/intel_drv.h | 64 ++ drivers/gpu/drm/i915/intel_vdsc.c| 1243 ++ include/drm/drm_dp_helper.h |3 + 9 files changed, 2557 insertions(+) create mode 100644 drivers/gpu/drm/i915/intel_vdsc.c -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: i915: Fix audio issue on BXT
On 1/4/2018 2:48 AM, Rodrigo Vivi wrote:
On Wed, Jan 03, 2018 at 08:31:10PM +, Pandiyan, Dhinakaran wrote:
On Thu, 2018-01-04 at 00:48 +0530, Gaurav K Singh wrote:
From: Gaurav Singh
On Apollolake, with stress test warm reboot, audio card
was not getting enumerated after reboot. This was a
The problem looks similar to
https://lists.freedesktop.org/archives/intel-gfx/2017-October/144495.html
although the proposed solutions are vastly different. I have Cc'd some
more people.
spurious issue happening on Apollolake. HW codec and
HD audio controller link was going out of sync for which
there was a fix in i915 driver but was not getting invoked
for BXT. Extending this fix to BXT as well.
Tested on apollolake chromebook by stress test warm reboot
with 2500 iterations.
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/intel_audio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_audio.c
b/drivers/gpu/drm/i915/intel_audio.c
index f1502a0188eb..c71c04e1c3f6 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -729,7 +729,7 @@ static void i915_audio_component_codec_wake_override(struct
device *kdev,
struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
u32 tmp;
- if (!IS_GEN9_BC(dev_priv))
+ if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv))
IS_GEN9()? GLK might need this too.
I think this is applicable for all Gen9 platforms.
if GLK need there is the possibility of CNL also needing it...
So not sure where to stop.
Also looking to the original patch that introduced this function,
commit '632f3ab95fe2 ("drm/i915/audio: add codec wakeup override
enabled/disable callback")'
it tells that the reason was:
"In SKL, HDMI/DP codec and PCH HD Audio Controller are in different p$
wells, so it's necessary to reset display audio codecs when power we$
otherwise display audio codecs will disappear when resume from low p$
state."
Is this the case here on BXT?
Yes, its the same case with BXT.
Another interesting thing I noticed on Spec when searching for this bit
was that this bit is related to an workaround on SKL/KBL/CFL... no mention
to BXT.
"This workaround is needed for an HW issue in SKL and KBL in which HW codec
and HD audio controller link was going out of sync."
Yes, in Bspec it has been mentioned only for SKL and KBL. But without
this fix, sound card was not getting enumerated for BXT.
Thanks,
Rodrigo.
return;
i915_audio_component_get_power(kdev);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/4] DSI Dual link enabling on BXT
On 9/16/2015 2:48 PM, Gaurav K Singh wrote: Hi, These patches enable DSI dual link mode on BXT boards. These set of patches build on top of the floated DSI Video mode patches on BXT (Uma's patches). Regards Gaurav Gaurav K Singh (4): drm/i915: Enable dual link mode in BXT drm/i915: Use adjusted mode clk for calculating DSI clk drm/i915: Execute RESET sequence before device ready drm/i915: Program vactive & hactive display size for both ports drivers/gpu/drm/i915/i915_reg.h|7 +++--- drivers/gpu/drm/i915/intel_display.c | 37 drivers/gpu/drm/i915/intel_dsi.c | 11 ++--- drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 15 +++ drivers/gpu/drm/i915/intel_dsi_pll.c |4 ++- include/drm/drm_panel.h|9 +++ 6 files changed, 76 insertions(+), 7 deletions(-) Hi All, Did anyone get a chance to review this patchset. With regards, Gaurav ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC CABC PATCH v2 3/3] drm/i915: CABC support for backlight control
On 7/30/2015 4:18 PM, Singh, Gaurav K wrote:
On 7/24/2015 5:54 PM, Deepak M wrote:
In CABC (Content Adaptive Brightness Control) content grey level
scale can be increased while simultaneously decreasing
brightness of the backlight to achieve same perceived brightness.
The CABC is not standardized and panel vendors are free to follow
their implementation. The CABC implementaion here assumes that the
panels use standard SW register for control.
In this design there will be no PWM signal from the SoC and DCS
commands are sent to enable and control the backlight brightness.
v2:
- Created a new backlight driver for cabc, which will be registered
only when it cabc is supported by panel.
(Addressed comment from Daniel Vetter)
Signed-off-by: Deepak M
---
drivers/gpu/drm/i915/Makefile |1 +
drivers/gpu/drm/i915/intel_drv.h |3 +-
drivers/gpu/drm/i915/intel_dsi.c | 13 ++
drivers/gpu/drm/i915/intel_dsi_cabc.c | 349
+
drivers/gpu/drm/i915/intel_dsi_cabc.h | 45 +
drivers/gpu/drm/i915/intel_panel.c| 22 ++-
include/video/mipi_display.h |8 +
7 files changed, 436 insertions(+), 5 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_dsi_cabc.c
create mode 100644 drivers/gpu/drm/i915/intel_dsi_cabc.h
diff --git a/drivers/gpu/drm/i915/Makefile
b/drivers/gpu/drm/i915/Makefile
index de21965..a73953c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -75,6 +75,7 @@ i915-y += dvo_ch7017.o \
intel_dp_mst.o \
intel_dsi.o \
intel_dsi_pll.o \
+ intel_dsi_cabc.o \
intel_dsi_panel_vbt.o \
intel_dvo.o \
intel_hdmi.o \
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 3f0a890..9f806dd5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1319,7 +1319,8 @@ extern struct drm_display_mode
*intel_find_panel_downclock(
struct drm_connector *connector);
void intel_backlight_register(struct drm_device *dev);
void intel_backlight_unregister(struct drm_device *dev);
-
+extern int cabc_backlight_device_register(struct intel_connector
*connector);
+extern void cabc_backlight_device_unregister(struct intel_connector
*connector);
/* intel_psr.c */
void intel_psr_enable(struct intel_dp *intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c
b/drivers/gpu/drm/i915/intel_dsi.c
index 98998e9..8f006f2 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -34,6 +34,7 @@
#include "i915_drv.h"
#include "intel_drv.h"
#include "intel_dsi.h"
+#include "intel_dsi_cabc.h"
static const struct {
u16 panel_id;
@@ -376,6 +377,7 @@ static void intel_dsi_enable(struct intel_encoder
*encoder)
struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+struct intel_connector *connector = intel_dsi->attached_connector;
enum port port;
DRM_DEBUG_KMS("\n");
@@ -396,6 +398,9 @@ static void intel_dsi_enable(struct intel_encoder
*encoder)
intel_dsi_port_enable(encoder);
}
+
+if (dev_priv->vbt.dsi.config->cabc_supported)
+cabc_enable_backlight(connector);
}
static void intel_dsi_pre_enable(struct intel_encoder *encoder)
@@ -469,11 +474,15 @@ static void intel_dsi_disable(struct
intel_encoder *encoder)
struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+struct intel_connector *connector = intel_dsi->attached_connector;
enum port port;
u32 temp;
DRM_DEBUG_KMS("\n");
+if (dev_priv->vbt.dsi.config->cabc_supported)
+cabc_disable_backlight(connector);
+
if (is_vid_mode(intel_dsi)) {
for_each_dsi_port(port, intel_dsi->ports)
wait_for_dsi_fifo_empty(intel_dsi, port);
@@ -1099,6 +1108,10 @@ void intel_dsi_init(struct drm_device *dev)
intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
+if (dev_priv->vbt.dsi.config->cabc_supported)
+cabc_setup_backlight(connector,
+intel_encoder->crtc_mask ==
+(1 << PIPE_A) ? PIPE_A : PIPE_B);
return;
err:
diff --git a/drivers/gpu/drm/i915/intel_dsi_cabc.c
b/drivers/gpu/drm/i915/intel_dsi_cabc.c
new file mode 100644
index 000..2a78326
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_dsi_cabc.c
@@ -0,0 +1,349 @@
+/*
+ * Copyright © 2006-2010 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person
obtaining a
+ * copy of this software and associated documentation files (the
"
Re: [Intel-gfx] [RFC CABC PATCH v2 3/3] drm/i915: CABC support for backlight control
On 7/24/2015 5:54 PM, Deepak M wrote:
In CABC (Content Adaptive Brightness Control) content grey level
scale can be increased while simultaneously decreasing
brightness of the backlight to achieve same perceived brightness.
The CABC is not standardized and panel vendors are free to follow
their implementation. The CABC implementaion here assumes that the
panels use standard SW register for control.
In this design there will be no PWM signal from the SoC and DCS
commands are sent to enable and control the backlight brightness.
v2:
- Created a new backlight driver for cabc, which will be registered
only when it cabc is supported by panel.
(Addressed comment from Daniel Vetter)
Signed-off-by: Deepak M
---
drivers/gpu/drm/i915/Makefile |1 +
drivers/gpu/drm/i915/intel_drv.h |3 +-
drivers/gpu/drm/i915/intel_dsi.c | 13 ++
drivers/gpu/drm/i915/intel_dsi_cabc.c | 349 +
drivers/gpu/drm/i915/intel_dsi_cabc.h | 45 +
drivers/gpu/drm/i915/intel_panel.c| 22 ++-
include/video/mipi_display.h |8 +
7 files changed, 436 insertions(+), 5 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_dsi_cabc.c
create mode 100644 drivers/gpu/drm/i915/intel_dsi_cabc.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index de21965..a73953c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -75,6 +75,7 @@ i915-y += dvo_ch7017.o \
intel_dp_mst.o \
intel_dsi.o \
intel_dsi_pll.o \
+ intel_dsi_cabc.o \
intel_dsi_panel_vbt.o \
intel_dvo.o \
intel_hdmi.o \
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3f0a890..9f806dd5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1319,7 +1319,8 @@ extern struct drm_display_mode
*intel_find_panel_downclock(
struct drm_connector *connector);
void intel_backlight_register(struct drm_device *dev);
void intel_backlight_unregister(struct drm_device *dev);
-
+extern int cabc_backlight_device_register(struct intel_connector *connector);
+extern void cabc_backlight_device_unregister(struct intel_connector
*connector);
/* intel_psr.c */
void intel_psr_enable(struct intel_dp *intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 98998e9..8f006f2 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -34,6 +34,7 @@
#include "i915_drv.h"
#include "intel_drv.h"
#include "intel_dsi.h"
+#include "intel_dsi_cabc.h"
static const struct {
u16 panel_id;
@@ -376,6 +377,7 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ struct intel_connector *connector = intel_dsi->attached_connector;
enum port port;
DRM_DEBUG_KMS("\n");
@@ -396,6 +398,9 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
intel_dsi_port_enable(encoder);
}
+
+ if (dev_priv->vbt.dsi.config->cabc_supported)
+ cabc_enable_backlight(connector);
}
static void intel_dsi_pre_enable(struct intel_encoder *encoder)
@@ -469,11 +474,15 @@ static void intel_dsi_disable(struct intel_encoder
*encoder)
struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ struct intel_connector *connector = intel_dsi->attached_connector;
enum port port;
u32 temp;
DRM_DEBUG_KMS("\n");
+ if (dev_priv->vbt.dsi.config->cabc_supported)
+ cabc_disable_backlight(connector);
+
if (is_vid_mode(intel_dsi)) {
for_each_dsi_port(port, intel_dsi->ports)
wait_for_dsi_fifo_empty(intel_dsi, port);
@@ -1099,6 +1108,10 @@ void intel_dsi_init(struct drm_device *dev)
intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
+ if (dev_priv->vbt.dsi.config->cabc_supported)
+ cabc_setup_backlight(connector,
+ intel_encoder->crtc_mask ==
+ (1 << PIPE_A) ? PIPE_A : PIPE_B);
return;
err:
diff --git a/drivers/gpu/drm/i915/intel_dsi_cabc.c
b/drivers/gpu/drm/i915/intel_dsi_cabc.c
new file mode 100644
index 000..2a78326
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_dsi_cabc.c
@@ -0,0 +1,349 @@
+/*
+ * Copyright © 2006-2010 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without li
Re: [Intel-gfx] {Intel-gfx] [RFC 01/14] drm/i915: allocate gem memory for mipi dbi cmd buffer
On 6/19/2015 3:32 AM, Gaurav K Singh wrote:
Allocate gem memory for MIPI DBI command buffer. This memory
will be used when sending command via DBI interface.
v2: lock mutex before gem object unreference and later set gem obj ptr to NULL
(Gaurav)
Signed-off-by: Yogesh Mohan Marimuthu
Signed-off-by: Gaurav K Singh
Signed-off-by: Shobhit Kumar
---
drivers/gpu/drm/i915/intel_dsi.c | 40 ++
drivers/gpu/drm/i915/intel_dsi.h |4
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 98998e9..011fef2 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -407,9 +407,35 @@ static void intel_dsi_pre_enable(struct intel_encoder
*encoder)
enum pipe pipe = intel_crtc->pipe;
enum port port;
u32 tmp;
+ int ret;
DRM_DEBUG_KMS("\n");
+ if (!intel_dsi->gem_obj && is_cmd_mode(intel_dsi)) {
+ intel_dsi->gem_obj = i915_gem_alloc_object(dev, 4096);
+ if (!intel_dsi->gem_obj) {
+ DRM_ERROR("Failed to allocate seqno page\n");
+ return;
+ }
+
+ ret = i915_gem_object_set_cache_level(intel_dsi->gem_obj,
+ I915_CACHE_LLC);
+ if (ret)
+ goto err_unref;
+
+ ret = i915_gem_obj_ggtt_pin(intel_dsi->gem_obj, 4096, 0);
+ if (ret) {
+err_unref:
+ drm_gem_object_unreference(&intel_dsi->gem_obj->base);
+ return;
+ }
+
+ intel_dsi->cmd_buff =
+ kmap(sg_page(intel_dsi->gem_obj->pages->sgl));
+ intel_dsi->cmd_buff_phy_addr = page_to_phys(
+ sg_page(intel_dsi->gem_obj->pages->sgl));
+ }
+
/* Disable DPOunit clock gating, can stall pipe
* and we need DPLL REFA always enabled */
tmp = I915_READ(DPLL(pipe));
@@ -555,6 +581,7 @@ static void intel_dsi_post_disable(struct intel_encoder
*encoder)
{
struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ struct drm_device *dev = encoder->base.dev;
u32 val;
DRM_DEBUG_KMS("\n");
@@ -571,6 +598,15 @@ static void intel_dsi_post_disable(struct intel_encoder
*encoder)
msleep(intel_dsi->panel_off_delay);
msleep(intel_dsi->panel_pwr_cycle_delay);
+
+ if (intel_dsi->gem_obj) {
+ kunmap(intel_dsi->cmd_buff);
+ i915_gem_object_ggtt_unpin(intel_dsi->gem_obj);
+ mutex_lock(&dev->struct_mutex);
+ drm_gem_object_unreference(&intel_dsi->gem_obj->base);
+ mutex_unlock(&dev->struct_mutex);
+ }
+ intel_dsi->gem_obj = NULL;
}
static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
@@ -1042,6 +1078,10 @@ void intel_dsi_init(struct drm_device *dev)
intel_dsi->ports = (1 << PORT_C);
}
+ intel_dsi->cmd_buff = NULL;
+ intel_dsi->cmd_buff_phy_addr = 0;
+ intel_dsi->gem_obj = NULL;
+
/* Create a DSI host (and a device) for each port. */
for_each_dsi_port(port, intel_dsi->ports) {
struct intel_dsi_host *host;
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 2784ac4..36ca3cc 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -44,6 +44,10 @@ struct intel_dsi {
struct intel_connector *attached_connector;
+ struct drm_i915_gem_object *gem_obj;
+ void *cmd_buff;
+ dma_addr_t cmd_buff_phy_addr;
+
/* bit mask of ports being driven */
u16 ports;
Corrected the initial patch. Working on the dma_alloc_coherent patch ,
will update soon.
With regards,
Gaurav
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 01/14] drm/i915: allocate gem memory for mipi dbi cmd buffer
On 6/15/2015 4:00 PM, Daniel Vetter wrote: On Mon, Jun 01, 2015 at 02:03:15PM +0300, Ville Syrjälä wrote: On Fri, May 29, 2015 at 07:10:53PM +0200, Daniel Vetter wrote: On Fri, May 29, 2015 at 01:59:01PM +0300, Ville Syrjälä wrote: On Fri, May 29, 2015 at 04:06:53PM +0530, Gaurav K Singh wrote: Allocate gem memory for MIPI DBI command buffer. This memory will be used when sending command via DBI interface. Why would you allocate this via gem? AFAICS you only feed the bus address to the hardware. Using the dma-api would seem like the right choice here, but I'm not sure how to deal with the dma mask. Yeah dma_alloc_coherent is what you want here. The mask can be ignored, it should be suitable already. Umm, this thing seems to limited to 32bit addresses. And we set the mask to 39 or 40 bits depending on the gen. Well that's what we have dma_set_coherent_mask for, hooray. Not the first one, see the other hacks with comments in i915_driver_load. -Daniel checking on the usage of dma_alloc_coherent. Will come back on this. With regards, Gaurav ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 11/14] drm/i915: Enable MIPI display self refresh mode
On 6/15/2015 4:03 PM, Daniel Vetter wrote:
On Sat, Jun 13, 2015 at 12:24:57PM +0530, Mohan Marimuthu, Yogesh wrote:
On 5/29/2015 10:51 PM, Daniel Vetter wrote:
On Fri, May 29, 2015 at 04:07:03PM +0530, Gaurav K Singh wrote:
During enable sequence for MIPI encoder in command mode, enable
MIPI display self-refresh mode bit in Pipe Ctrl reg.
Signed-off-by: Gaurav K Singh
Signed-off-by: Yogesh Mohan Marimuthu
Signed-off-by: Shobhit Kumar
---
drivers/gpu/drm/i915/intel_display.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index cab2ac8..fc84313 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -44,6 +44,7 @@
#include
#include
#include
+#include "intel_dsi.h"
/* Primary plane formats supported by all gen */
#define COMMON_PRIMARY_FORMATS \
@@ -2110,6 +2111,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_encoder *encoder;
+ struct intel_dsi *intel_dsi;
enum pipe pipe = crtc->pipe;
enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
pipe);
@@ -2154,6 +2157,18 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
return;
}
+ for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+ if (encoder->type == INTEL_OUTPUT_DSI) {
+ intel_dsi = enc_to_intel_dsi(&encoder->base);
+ if (intel_dsi && (intel_dsi->operation_mode ==
+ INTEL_DSI_COMMAND_MODE)) {
+ val = val | PIPECONF_MIPI_DSR_ENABLE;
+ I915_WRITE(reg, val);
+ }
+ break;
+ }
+ }
When we have these kind of encoder/crtc state depencies we resolve them by
adding a bit of state to intel_crtc_state which is set as needed in the
encoder's compute_config callback. Then all you need here is
if (intel_state->dsi_self_refresh)
val |= PIPECONF_MIPI_DSR_ENABLE;
Also is that additional write really required?
-Daniel
Yes additional write is required. MIPI_DSR_ENABLE has to be written first
then followed
by pipe enable. if MIPI_DSR_ENABLE and pipe enable is done in same write,
observed
that the image from pipe is not sent to panel when issued mem write command.
Having a state variable instead of looping through the encoders definitely
looks good.
Need to find a place to update the state variable. I will get back on this.
Like I said such state is precomputed in the encoder callbacks, in this
case intel_dsi_compute_config.
Cheers, Daniel
Agree with you daniel regarding state flag. Updated patch is ready, will
upload shortly.
Regarding additional write, as Yogesh confirmed, both the writes are
required.
With regards,
Gaurav
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 07/14] drm/i915: Disable MIPI display self refresh mode
On 5/29/2015 10:50 PM, Daniel Vetter wrote:
On Fri, May 29, 2015 at 07:16:36PM +0200, Daniel Vetter wrote:
On Fri, May 29, 2015 at 04:06:59PM +0530, Gaurav K Singh wrote:
During disable sequence for MIPI encoder in command mode, disable
MIPI display self-refresh mode bit in Pipe Ctrl reg.
Signed-off-by: Gaurav K Singh
Signed-off-by: Yogesh Mohan Marimuthu
Signed-off-by: Shobhit Kumar
---
drivers/gpu/drm/i915/intel_display.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 895d7c7..cab2ac8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2171,6 +2171,9 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
static void intel_disable_pipe(struct intel_crtc *crtc)
{
struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+ struct intel_encoder *encoder;
+ struct intel_dsi *intel_dsi;
+ struct drm_device *dev = crtc->base.dev;
enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
enum pipe pipe = crtc->pipe;
int reg;
@@ -2189,6 +2192,16 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
if ((val & PIPECONF_ENABLE) == 0)
return;
+ for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+ if (encoder->type == INTEL_OUTPUT_DSI) {
+ intel_dsi = enc_to_intel_dsi(&encoder->base);
+ if (intel_dsi && (intel_dsi->operation_mode ==
+ INTEL_DSI_COMMAND_MODE))
+ val = val & ~PIPECONF_MIPI_DSR_ENABLE;
+ break;
+ }
+ }
This must be moved into a suitable encoder callback. Yes the ddi code is
full of cases where encoder stuff is done from the generic crtc code. But
we now have 2 completely different kinds of ports on bxt (ddi and dsi),
and we need to get some solid structure into the code again.
Ah I missed that this is a pipeconf thing. Unconditionally clearing this
bit will achieve the same, without the need to loop over connectors.
-Daniel
Agree with you. Instead of loop over the encoders, will set up the state
flag and will use it here.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 06/14] drm/i915: Disable vlank interrupt for disabling MIPI cmd mode
On 5/29/2015 10:53 PM, Daniel Vetter wrote:
On Fri, May 29, 2015 at 07:14:43PM +0200, Daniel Vetter wrote:
On Fri, May 29, 2015 at 04:06:58PM +0530, Gaurav K Singh wrote:
vblank interrupt should be disabled before starting the disable
sequence for MIPI command mode. Otherwise when pipe is disabled
TE interurpt will be still handled and one memory write command
will be sent with pipe disabled. This makes the pipe hw to get
stuck and it doesn't recover in the next enable sequence causing
display blank out.
Signed-off-by: Yogesh Mohan Marimuthu
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/intel_dsi.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 04d8ce0..aeea289 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -513,11 +513,25 @@ static void intel_dsi_enable_nop(struct intel_encoder
*encoder)
static void intel_dsi_pre_disable(struct intel_encoder *encoder)
{
+ struct drm_device *dev = encoder->base.dev;
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+ int pipe = intel_crtc->pipe;
enum port port;
DRM_DEBUG_KMS("\n");
+ if (is_cmd_mode(intel_dsi)) {
+ dev->driver->disable_vblank(dev, pipe);
+
+ /*
+* Make sure that the last frame is sent otherwise pipe can get
+* stuck. Currently providing delay time for ~2 vblanks
+* assuming 60fps.
+*/
+ mdelay(40);
+ }
Nope. You need to move around the drm_vblank_off suitably, only that
function correctly handles all the book-keeping around vblank interrupts.
If this doesn't work out because of ordering we need to dig into this and
figure out something. Worst case we need to push the drm_vblank_off call
into encoder callbacks for everyone. That's something we already discussed
but then decided against.
I seem to be blind, but where exactly is that vblank-driven upload code?
-Daniel
Hi Daniel,
dev->driver->disable_vblank calls valleyview_disable_vblank which already
exists. But I did check with drm_vblank_off, it seems to work fine.
I will float the updated patch shortly.
With regards,
Gaurav
-Daniel
+
if (is_vid_mode(intel_dsi)) {
/* Send Shutdown command to the panel in LP mode */
for_each_dsi_port(port, intel_dsi->ports)
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Changes required to enable DSI Video Mode on CHT
On 12/12/2014 1:03 PM, Singh, Gaurav K wrote:
On 12/10/2014 7:38 PM, Gaurav K Singh wrote:
For CHT changes are required for calculating the correct m,n & p with
minimal error +/- for the required DSI clock, so that the correct
dividor
& ctrl values are written in cck regs for DSI. This patch has been
tested
on CHT RVP with 1200 x 1920 panel.
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/intel_dsi_pll.c | 43
++
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c
b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 8957f10..9236b66 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -162,7 +162,8 @@ static u32 dsi_clk_from_pclk(u32 pclk, int
pixel_format, int lane_count)
#endif
-static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp *dsi_mnp)
+static int dsi_calc_mnp(struct drm_i915_private *dev_priv,
+u32 dsi_clk, struct dsi_mnp *dsi_mnp)
{
u32 m, n, p;
u32 ref_clk;
@@ -173,6 +174,10 @@ static int dsi_calc_mnp(u32 dsi_clk, struct
dsi_mnp *dsi_mnp)
u32 calc_m;
u32 calc_p;
u32 m_seed;
+u32 m_start;
+u32 m_limit;
+u32 n_limit;
+u32 p_limit;
/* dsi_clk is expected in KHZ */
if (dsi_clk < 30 || dsi_clk > 115) {
@@ -180,18 +185,33 @@ static int dsi_calc_mnp(u32 dsi_clk, struct
dsi_mnp *dsi_mnp)
return -ECHRNG;
}
-ref_clk = 25000;
+if (IS_CHERRYVIEW(dev_priv->dev)) {
+ref_clk = 10;
+m_start = 70;
+m_limit = 96;
+n_limit = 4;
+p_limit = 6;
+} else if (IS_VALLEYVIEW(dev_priv->dev)) {
+ref_clk = 25000;
+m_start = 62;
+m_limit = 92;
+n_limit = 1;
+p_limit = 6;
+} else {
+DRM_ERROR("Unsupported device\n");
+return -ENODEV;
+}
target_dsi_clk = dsi_clk;
error = 0x;
tmp_error = 0x;
calc_m = 0;
calc_p = 0;
-for (m = 62; m <= 92; m++) {
-for (p = 2; p <= 6; p++) {
+for (m = m_start; m <= m_limit; m++) {
+for (p = 2; p <= p_limit; p++) {
/* Find the optimal m and p divisors
with minimal error +/- the required clock */
-calc_dsi_clk = (m * ref_clk) / p;
+calc_dsi_clk = (m * ref_clk) / (p * n_limit);
if (calc_dsi_clk == target_dsi_clk) {
calc_m = m;
calc_p = p;
@@ -212,11 +232,14 @@ static int dsi_calc_mnp(u32 dsi_clk, struct
dsi_mnp *dsi_mnp)
}
m_seed = lfsr_converts[calc_m - 62];
-n = 1;
+n = n_limit;
dsi_mnp->dsi_pll_ctrl = 1 << (DSI_PLL_P1_POST_DIV_SHIFT +
calc_p - 2);
-dsi_mnp->dsi_pll_div = (n - 1) << DSI_PLL_N1_DIV_SHIFT |
-m_seed << DSI_PLL_M1_DIV_SHIFT;
-
+if (IS_CHERRYVIEW(dev_priv->dev))
+dsi_mnp->dsi_pll_div = (n/2) << DSI_PLL_N1_DIV_SHIFT |
+m_seed << DSI_PLL_M1_DIV_SHIFT;
+else
+dsi_mnp->dsi_pll_div = (n - 1) << DSI_PLL_N1_DIV_SHIFT |
+m_seed << DSI_PLL_M1_DIV_SHIFT;
return 0;
}
@@ -235,7 +258,7 @@ static void vlv_configure_dsi_pll(struct
intel_encoder *encoder)
dsi_clk = dsi_clk_from_pclk(intel_dsi->pclk,
intel_dsi->pixel_format,
intel_dsi->lane_count);
-ret = dsi_calc_mnp(dsi_clk, &dsi_mnp);
+ret = dsi_calc_mnp(dev_priv, dsi_clk, &dsi_mnp);
if (ret) {
DRM_DEBUG_KMS("dsi_calc_mnp failed\n");
return;
Hi Jani,
Could you please review this patch?
With regards,
Gaurav
Hi Jani,
Could you please review this patch?
With regards,
Gaurav
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Changes required to enable DSI Video Mode on CHT
On 12/10/2014 7:38 PM, Gaurav K Singh wrote:
For CHT changes are required for calculating the correct m,n & p with
minimal error +/- for the required DSI clock, so that the correct dividor
& ctrl values are written in cck regs for DSI. This patch has been tested
on CHT RVP with 1200 x 1920 panel.
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/intel_dsi_pll.c | 43 ++
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c
b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 8957f10..9236b66 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -162,7 +162,8 @@ static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format,
int lane_count)
#endif
-static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp *dsi_mnp)
+static int dsi_calc_mnp(struct drm_i915_private *dev_priv,
+ u32 dsi_clk, struct dsi_mnp *dsi_mnp)
{
u32 m, n, p;
u32 ref_clk;
@@ -173,6 +174,10 @@ static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp
*dsi_mnp)
u32 calc_m;
u32 calc_p;
u32 m_seed;
+ u32 m_start;
+ u32 m_limit;
+ u32 n_limit;
+ u32 p_limit;
/* dsi_clk is expected in KHZ */
if (dsi_clk < 30 || dsi_clk > 115) {
@@ -180,18 +185,33 @@ static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp
*dsi_mnp)
return -ECHRNG;
}
- ref_clk = 25000;
+ if (IS_CHERRYVIEW(dev_priv->dev)) {
+ ref_clk = 10;
+ m_start = 70;
+ m_limit = 96;
+ n_limit = 4;
+ p_limit = 6;
+ } else if (IS_VALLEYVIEW(dev_priv->dev)) {
+ ref_clk = 25000;
+ m_start = 62;
+ m_limit = 92;
+ n_limit = 1;
+ p_limit = 6;
+ } else {
+ DRM_ERROR("Unsupported device\n");
+ return -ENODEV;
+ }
target_dsi_clk = dsi_clk;
error = 0x;
tmp_error = 0x;
calc_m = 0;
calc_p = 0;
- for (m = 62; m <= 92; m++) {
- for (p = 2; p <= 6; p++) {
+ for (m = m_start; m <= m_limit; m++) {
+ for (p = 2; p <= p_limit; p++) {
/* Find the optimal m and p divisors
with minimal error +/- the required clock */
- calc_dsi_clk = (m * ref_clk) / p;
+ calc_dsi_clk = (m * ref_clk) / (p * n_limit);
if (calc_dsi_clk == target_dsi_clk) {
calc_m = m;
calc_p = p;
@@ -212,11 +232,14 @@ static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp
*dsi_mnp)
}
m_seed = lfsr_converts[calc_m - 62];
- n = 1;
+ n = n_limit;
dsi_mnp->dsi_pll_ctrl = 1 << (DSI_PLL_P1_POST_DIV_SHIFT + calc_p - 2);
- dsi_mnp->dsi_pll_div = (n - 1) << DSI_PLL_N1_DIV_SHIFT |
- m_seed << DSI_PLL_M1_DIV_SHIFT;
-
+ if (IS_CHERRYVIEW(dev_priv->dev))
+ dsi_mnp->dsi_pll_div = (n/2) << DSI_PLL_N1_DIV_SHIFT |
+ m_seed << DSI_PLL_M1_DIV_SHIFT;
+ else
+ dsi_mnp->dsi_pll_div = (n - 1) << DSI_PLL_N1_DIV_SHIFT |
+ m_seed << DSI_PLL_M1_DIV_SHIFT;
return 0;
}
@@ -235,7 +258,7 @@ static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
dsi_clk = dsi_clk_from_pclk(intel_dsi->pclk, intel_dsi->pixel_format,
intel_dsi->lane_count);
- ret = dsi_calc_mnp(dsi_clk, &dsi_mnp);
+ ret = dsi_calc_mnp(dev_priv, dsi_clk, &dsi_mnp);
if (ret) {
DRM_DEBUG_KMS("dsi_calc_mnp failed\n");
return;
Hi Jani,
Could you please review this patch?
With regards,
Gaurav
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: DSI sequence related changes for DSI Port C
On 12/10/2014 2:50 PM, Daniel Vetter wrote: On Tue, Dec 09, 2014 at 12:30:49PM +0200, Jani Nikula wrote: On Tue, 09 Dec 2014, "Singh, Gaurav K" wrote: On 12/7/2014 4:13 PM, Gaurav K Singh wrote: For DSI Port A & C, the seq_port value has been set to 0 now in VBT Now the sequence of DSI single link on Port A and Port C will based on the DVO port from VBT block 2. Signed-off-by: Gaurav K Singh --- drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c index f8c2269..e7e2e52 100644 --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c @@ -110,7 +110,14 @@ static u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, u8 *data) vc = (byte >> MIPI_VIRTUAL_CHANNEL_SHIFT) & 0x3; seq_port = (byte >> MIPI_PORT_SHIFT) & 0x3; - port = intel_dsi_seq_port_to_port(seq_port); + /* For DSI Port A & C, the seq_port value has been set to 0 now in VBT +* Now the sequence of DSI single link on Port A and Port C will based +* on the DVO port from VBT block 2. +*/ + if (intel_dsi->ports == (1 << PORT_C)) + port = PORT_C; + else + port = intel_dsi_seq_port_to_port(seq_port); /* LP or HS mode */ intel_dsi->hs = mode; Jani, Need your reviewed-by on this patch too. Okay, I was confused because there were actually five patches in this four patch series! ;) The *code* is Reviewed-by: Jani Nikula because I understand it, but frankly both the commit message and the comment confuse me more. Hm, do you have suggestions for a better commit message? Should we just drop the comment. I agree that the talk about VBT is really confusing and smells like leftovers from other stuff. I'll wait with this one until this is resolved. Accurate and clear commit messages are important. -Daniel Jani, How is the below commit message? From now on for both DSI Ports A & C, the seq_port value has been set to 0. seq_port value is parsed from Sequence block#53 of VBT.So, for packets that needs to be read/write for DSI single link on Port A and Port C will based on the DVO port from VBT block 2. With regards, Gaurav ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: DSI sequence related changes for DSI Port C
On 12/9/2014 4:00 PM, Jani Nikula wrote: On Tue, 09 Dec 2014, "Singh, Gaurav K" wrote: On 12/7/2014 4:13 PM, Gaurav K Singh wrote: For DSI Port A & C, the seq_port value has been set to 0 now in VBT Now the sequence of DSI single link on Port A and Port C will based on the DVO port from VBT block 2. Signed-off-by: Gaurav K Singh --- drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c index f8c2269..e7e2e52 100644 --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c @@ -110,7 +110,14 @@ static u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, u8 *data) vc = (byte >> MIPI_VIRTUAL_CHANNEL_SHIFT) & 0x3; seq_port = (byte >> MIPI_PORT_SHIFT) & 0x3; - port = intel_dsi_seq_port_to_port(seq_port); + /* For DSI Port A & C, the seq_port value has been set to 0 now in VBT +* Now the sequence of DSI single link on Port A and Port C will based +* on the DVO port from VBT block 2. +*/ + if (intel_dsi->ports == (1 << PORT_C)) + port = PORT_C; + else + port = intel_dsi_seq_port_to_port(seq_port); /* LP or HS mode */ intel_dsi->hs = mode; Jani, Need your reviewed-by on this patch too. Okay, I was confused because there were actually five patches in this four patch series! ;) The *code* is Reviewed-by: Jani Nikula because I understand it, but frankly both the commit message and the comment confuse me more. Thanks Jani. I uploaded the next version of my 1/4 patch over my message id only, but sorry for the confusion. Daniel, Could you please merge this patch too, got reviewed-by from Jani. Thanks. With regards, Gaurav With regards, Gaurav ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: DSI sequence related changes for DSI Port C
On 12/7/2014 4:13 PM, Gaurav K Singh wrote: For DSI Port A & C, the seq_port value has been set to 0 now in VBT Now the sequence of DSI single link on Port A and Port C will based on the DVO port from VBT block 2. Signed-off-by: Gaurav K Singh --- drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c index f8c2269..e7e2e52 100644 --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c @@ -110,7 +110,14 @@ static u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, u8 *data) vc = (byte >> MIPI_VIRTUAL_CHANNEL_SHIFT) & 0x3; seq_port = (byte >> MIPI_PORT_SHIFT) & 0x3; - port = intel_dsi_seq_port_to_port(seq_port); + /* For DSI Port A & C, the seq_port value has been set to 0 now in VBT +* Now the sequence of DSI single link on Port A and Port C will based +* on the DVO port from VBT block 2. +*/ + if (intel_dsi->ports == (1 << PORT_C)) + port = PORT_C; + else + port = intel_dsi_seq_port_to_port(seq_port); /* LP or HS mode */ intel_dsi->hs = mode; Jani, Need your reviewed-by on this patch too. With regards, Gaurav ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use DSI Pll1 for enabling MIPI DSI on Port C
On 12/8/2014 5:03 PM, Jani Nikula wrote:
On Sun, 07 Dec 2014, Gaurav K Singh wrote:
DSI Pll1 is used for enabling DSI on Port C.
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/intel_dsi_pll.c |7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c
b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 8957f10..9b7f6a5 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -241,9 +241,12 @@ static void vlv_configure_dsi_pll(struct intel_encoder
*encoder)
return;
}
- dsi_mnp.dsi_pll_ctrl |= DSI_PLL_CLK_GATE_DSI0_DSIPLL;
+ if ((intel_dsi->ports == ((1 << PORT_A) | (1 << PORT_C))) ||
+ (intel_dsi->ports == (1 << PORT_A)))
+ dsi_mnp.dsi_pll_ctrl |= DSI_PLL_CLK_GATE_DSI0_DSIPLL;
- if (intel_dsi->dual_link)
+ if ((intel_dsi->ports == ((1 << PORT_A) | (1 << PORT_C))) ||
+ (intel_dsi->ports == (1 << PORT_C)))
dsi_mnp.dsi_pll_ctrl |= DSI_PLL_CLK_GATE_DSI1_DSIPLL;
In other words, patches 1 and 2 can be squashed into one that has:
if (intel_dsi->ports & (1 << PORT_A))
dsi_mnp.dsi_pll_ctrl |= DSI_PLL_CLK_GATE_DSI0_DSIPLL;
if (intel_dsi->ports & (1 << PORT_C))
dsi_mnp.dsi_pll_ctrl |= DSI_PLL_CLK_GATE_DSI1_DSIPLL;
Right?
BR,
Jani.
Jani,
Yes, uploaded the new patch addressing your comments.
With regards,
Gaurav
DRM_DEBUG_KMS("dsi pll div %08x, ctrl %08x\n",
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Get HW state changes required for DSI port C
On 12/8/2014 5:07 PM, Jani Nikula wrote:
On Sun, 07 Dec 2014, Gaurav K Singh wrote:
Due to some hardware limitations, MIPI Port C DPI Enable bit
does not get set. To check whether DSI Port C was enabled in BIOS,
check the Pipe B enable bit for DSI Port C. In hardware, DSI Port C
is linked with Pipe B.
"due to some hardware limitations" is awfully vague. The same code will
be used on many platforms; I doubt this applies to all of them. Is there
a workaround name for this?
BR,
Jani.
Hi Jani,
This is currently only for BYT, I missed to add in this patch commit header.
Software workaround for getting the HW status of DSI Port C on BYT, will
it be fine if I include this in the commit header.
With regards,
Gaurav
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/intel_dsi.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 215d004..0334c4d 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -398,8 +398,9 @@ static bool intel_dsi_get_hw_state(struct intel_encoder
*encoder,
enum pipe *pipe)
{
struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
enum intel_display_power_domain power_domain;
- u32 port_ctl, func;
+ u32 dsi_status, func;
enum port port;
DRM_DEBUG_KMS("\n");
@@ -409,13 +410,23 @@ static bool intel_dsi_get_hw_state(struct intel_encoder
*encoder,
return false;
/* XXX: this only works for one DSI output */
- for_each_dsi_port(port, (1 << PORT_A) | (1 << PORT_C)) {
- port_ctl = I915_READ(MIPI_PORT_CTRL(port));
+ for_each_dsi_port(port, intel_dsi->ports) {
func = I915_READ(MIPI_DSI_FUNC_PRG(port));
- if ((port_ctl & DPI_ENABLE) || (func & CMD_MODE_DATA_WIDTH_MASK)) {
+ /* Due to some hardware limitations, MIPI Port C DPI Enable
+* bit does not get set. To check whether DSI Port C was
+* enabled in BIOS, check the Pipe B enable bit
+*/
+ if (port == PORT_C)
+ dsi_status = I915_READ(PIPECONF(PIPE_B)) &
+ PIPECONF_ENABLE;
+ else
+ dsi_status = I915_READ(MIPI_PORT_CTRL(port)) &
+ DPI_ENABLE;
+
+ if (dsi_status || (func & CMD_MODE_DATA_WIDTH_MASK)) {
if (I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY) {
- *pipe = port == PORT_A ? PIPE_A : PIPE_C;
+ *pipe = port == PORT_A ? PIPE_A : PIPE_B;
return true;
}
}
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/10] drm/i915: Pixel Clock changes for DSI dual link
On 12/5/2014 11:18 PM, Siluvery, Arun wrote:
On 05/12/2014 17:36, Jani Nikula wrote:
On Fri, 05 Dec 2014, "Siluvery, Arun"
wrote:
On 05/12/2014 16:33, Singh, Gaurav K wrote:
On 12/4/2014 2:57 PM, Jani Nikula wrote:
On Thu, 04 Dec 2014, Gaurav K Singh wrote:
For dual link MIPI Panels, each port needs half of pixel clock.
Pixel overlap
can be enabled if needed by panel, then in that case, pixel clock
will be
increased for extra pixels.
just a question, why do we need pixel overlap?
I couldn't find more details from spec other than that when overlap is
set some extra pixels are sent.
From the host perspective a dual link (or dual channel) DSI device is
two independent peripheral devices. On the peripheral side the display
has to combine the input from the two links (which may be two
independent DSI blocks on the peripheral as well) into one contiguous
display. I don't know the details, but I'm guessing pixel overlap just
makes it easier for the peripheral implementation to get it all
together.
Thank you for the details.
I am just wondering how few extra pixels help on the display side
unless they are fixed values which act like some kind of markers to
synchronize between two halves.
regards
Arun
Pixel overlap count are fixed values only and it can have values of 0, 1
or 2. These values will be defined for the particular MIPI DSI panel specs.
These values are programmed in VBT which driver parses and uses it.
With regards,
Gaurav
BR,
Jani.
regards
Arun
v2 : Address review comments by Jani
- Removed the bit mask used for ->dual_link
- Used DSI instead of MIPI for #define variables
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/i915_reg.h|4
drivers/gpu/drm/i915/intel_bios.h |3 ++-
drivers/gpu/drm/i915/intel_dsi.c |8
drivers/gpu/drm/i915/intel_dsi.h |6 ++
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 21
+
5 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index c981f5d..87149ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6029,6 +6029,10 @@ enum punit_power_well {
#define GEN8_PMINTR_REDIRECT_TO_NON_DISP (1<<31)
#define VLV_PWRDWNUPCTL0xA294
+#define VLV_CHICKEN_30x7040C
+#define PIXEL_OVERLAP_CNT_MASK(3 << 30)
+#define PIXEL_OVERLAP_CNT_SHIFT30
I didn't find this register, but does it not need + VLV_DISPLAY_BASE?
Given that I can't find the register my review is pretty shallow,
but I
don't spot anything obviously wrong either. With these caveats,
Reviewed-by: Jani Nikula
This reg is available in BSpec though the bit definitions have not
been updated in the BSpec. Also, it was communicated by the BIOS
team.
+
#define GEN6_PMISR0x44020
#define GEN6_PMIMR0x44024 /* rps_lock */
#define GEN6_PMIIR0x44028
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index de01167..a6a8710 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -818,7 +818,8 @@ struct mipi_config {
#define DUAL_LINK_PIXEL_ALT2
u16 dual_link:2;
u16 lane_cnt:2;
-u16 rsvd3:12;
+u16 pixel_overlap:3;
+u16 rsvd3:9;
u16 rsvd4;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c
b/drivers/gpu/drm/i915/intel_dsi.c
index dbe52e9..4e18abd 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -111,6 +111,14 @@ static void intel_dsi_port_enable(struct
intel_encoder *encoder)
enum port port;
u32 temp;
+if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
+temp = I915_READ(VLV_CHICKEN_3);
+temp &= ~PIXEL_OVERLAP_CNT_MASK |
+intel_dsi->pixel_overlap <<
+PIXEL_OVERLAP_CNT_SHIFT;
+I915_WRITE(VLV_CHICKEN_3, temp);
+}
+
for_each_dsi_port(port, intel_dsi->ports) {
temp = I915_READ(MIPI_PORT_CTRL(port));
diff --git a/drivers/gpu/drm/i915/intel_dsi.h
b/drivers/gpu/drm/i915/intel_dsi.h
index f2cc2fc..8fe2064 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -28,6 +28,11 @@
#include
#include "intel_drv.h"
+/* Dual Link support */
+#define DSI_DUAL_LINK_NONE0
+#define DSI_DUAL_LINK_FRONT_BACK1
+#define DSI_DUAL_LINK_PIXEL_ALT2
+
struct intel_dsi_device {
unsigned int panel_id;
const char *name;
@@ -105,6 +110,7 @@ struct intel_dsi {
u8 escape_clk_div;
u8 dual_link;
+u8 pixel_overlap;
u32 port_bits;
u32 bw_timer;
u32 dphy_reg;
diff --git a/drivers/gpu/drm/i915/intel_d
Re: [Intel-gfx] [PATCH 02/10] drm/i915: Added port as parameter to the functions which does read/write of DSI Controller
On 12/5/2014 8:08 PM, Daniel Vetter wrote: On Fri, Dec 05, 2014 at 06:20:43PM +0530, Singh, Gaurav K wrote: On 12/4/2014 4:52 PM, Daniel Vetter wrote: On Thu, Dec 04, 2014 at 11:14:01AM +0200, Jani Nikula wrote: On Thu, 04 Dec 2014, Gaurav K Singh wrote: This patch is in preparation of DSI dual link panels. For dual link panels, few packets needs to be sent to Port A or Port C or both. Based on the portno from MIPI Sequence Block#53, these sequences needs to be sent accordingly. v2: Addressed review comments by Jani - port variables named properly Signed-off-by: Gaurav K Singh Reviewed-by: Jani Nikula Merged the first two patches, thanks. -Daniel Hi Daniel, Thanks. I have addressed Jani's review comments too for the rest of the patches. Well looking through the resend patches your changes look a bit minimal. Which might be the right thing to do, but review is also about communication and sharing expert knowledge. So please follow up to the questions from Jani where you didn't adjust the code. And if there's any follow-up patches from those discussions please submit them. Another part: Your editor doesn't seem to align continuation lines quite how we usually do that. checkpatch --strict will catch that. Two big rules: - function paramater lists should be aligned to the opening ( on the next line. - Continuations of boolean logic checks (e.g. in if checks) same, with the special rule that && or || should be on the previous line as the last thing. - continuation of any other long lines should be indented 1 or two spaces (just to make sure it sticks out without looking jarring). Especially the first two help readability a lot of you're used to them, so please do a follow-up patch to rectify this in intel_dsi*.c. Thanks, Daniel Done. Fixed the style related issues and comments also added. With regards, Gaurav ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/10] drm/i915: Pixel Clock changes for DSI dual link
On 12/5/2014 10:24 PM, Siluvery, Arun wrote:
On 05/12/2014 16:33, Singh, Gaurav K wrote:
On 12/4/2014 2:57 PM, Jani Nikula wrote:
On Thu, 04 Dec 2014, Gaurav K Singh wrote:
For dual link MIPI Panels, each port needs half of pixel clock.
Pixel overlap
can be enabled if needed by panel, then in that case, pixel clock
will be
increased for extra pixels.
just a question, why do we need pixel overlap?
I couldn't find more details from spec other than that when overlap is
set some extra pixels are sent.
regards
Arun
In Front-Back video mode for MIPI Dual Link the first half of columns of
pixel is always transmitted by MIPI Port A and second half in MIPI Port C.
Pixel Overlap Count sets the number of Pixels to be overlapped per half
of Scanline in Front-Back video mode. In dual link front-back mode,
pixel overlap count needs to be added to HActive to account for the
additional pixels added due to overlap.
v2 : Address review comments by Jani
- Removed the bit mask used for ->dual_link
- Used DSI instead of MIPI for #define variables
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/i915_reg.h|4
drivers/gpu/drm/i915/intel_bios.h |3 ++-
drivers/gpu/drm/i915/intel_dsi.c |8
drivers/gpu/drm/i915/intel_dsi.h |6 ++
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 21
+
5 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index c981f5d..87149ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6029,6 +6029,10 @@ enum punit_power_well {
#define GEN8_PMINTR_REDIRECT_TO_NON_DISP(1<<31)
#define VLV_PWRDWNUPCTL0xA294
+#define VLV_CHICKEN_30x7040C
+#define PIXEL_OVERLAP_CNT_MASK(3 << 30)
+#define PIXEL_OVERLAP_CNT_SHIFT30
I didn't find this register, but does it not need + VLV_DISPLAY_BASE?
Given that I can't find the register my review is pretty shallow, but I
don't spot anything obviously wrong either. With these caveats,
Reviewed-by: Jani Nikula
This reg is available in BSpec though the bit definitions have not
been updated in the BSpec. Also, it was communicated by the BIOS team.
+
#define GEN6_PMISR0x44020
#define GEN6_PMIMR0x44024 /* rps_lock */
#define GEN6_PMIIR0x44028
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index de01167..a6a8710 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -818,7 +818,8 @@ struct mipi_config {
#define DUAL_LINK_PIXEL_ALT2
u16 dual_link:2;
u16 lane_cnt:2;
-u16 rsvd3:12;
+u16 pixel_overlap:3;
+u16 rsvd3:9;
u16 rsvd4;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c
b/drivers/gpu/drm/i915/intel_dsi.c
index dbe52e9..4e18abd 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -111,6 +111,14 @@ static void intel_dsi_port_enable(struct
intel_encoder *encoder)
enum port port;
u32 temp;
+if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
+temp = I915_READ(VLV_CHICKEN_3);
+temp &= ~PIXEL_OVERLAP_CNT_MASK |
+intel_dsi->pixel_overlap <<
+PIXEL_OVERLAP_CNT_SHIFT;
+I915_WRITE(VLV_CHICKEN_3, temp);
+}
+
for_each_dsi_port(port, intel_dsi->ports) {
temp = I915_READ(MIPI_PORT_CTRL(port));
diff --git a/drivers/gpu/drm/i915/intel_dsi.h
b/drivers/gpu/drm/i915/intel_dsi.h
index f2cc2fc..8fe2064 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -28,6 +28,11 @@
#include
#include "intel_drv.h"
+/* Dual Link support */
+#define DSI_DUAL_LINK_NONE0
+#define DSI_DUAL_LINK_FRONT_BACK1
+#define DSI_DUAL_LINK_PIXEL_ALT2
+
struct intel_dsi_device {
unsigned int panel_id;
const char *name;
@@ -105,6 +110,7 @@ struct intel_dsi {
u8 escape_clk_div;
u8 dual_link;
+u8 pixel_overlap;
u32 port_bits;
u32 bw_timer;
u32 dphy_reg;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index f60146f..f8c2269 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -288,6 +288,7 @@ static bool generic_init(struct
intel_dsi_device *dsi)
intel_dsi->lane_count = mipi_config->lane_cnt + 1;
intel_dsi->pixel_format =
mipi_config->videomode_color_format << 7;
intel_dsi->dual_link = mipi_config->dual_link;
+intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
if (intel_dsi->dual_link)
intel_dsi->ports = ((1 <
Re: [Intel-gfx] [PATCH 04/10] drm/i915: Pixel Clock changes for DSI dual link
On 12/4/2014 2:57 PM, Jani Nikula wrote:
On Thu, 04 Dec 2014, Gaurav K Singh wrote:
For dual link MIPI Panels, each port needs half of pixel clock. Pixel overlap
can be enabled if needed by panel, then in that case, pixel clock will be
increased for extra pixels.
v2 : Address review comments by Jani
- Removed the bit mask used for ->dual_link
- Used DSI instead of MIPI for #define variables
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/i915_reg.h|4
drivers/gpu/drm/i915/intel_bios.h |3 ++-
drivers/gpu/drm/i915/intel_dsi.c |8
drivers/gpu/drm/i915/intel_dsi.h |6 ++
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 21 +
5 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c981f5d..87149ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6029,6 +6029,10 @@ enum punit_power_well {
#define GEN8_PMINTR_REDIRECT_TO_NON_DISP (1<<31)
#define VLV_PWRDWNUPCTL 0xA294
+#define VLV_CHICKEN_30x7040C
+#define PIXEL_OVERLAP_CNT_MASK(3 << 30)
+#define PIXEL_OVERLAP_CNT_SHIFT 30
I didn't find this register, but does it not need + VLV_DISPLAY_BASE?
Given that I can't find the register my review is pretty shallow, but I
don't spot anything obviously wrong either. With these caveats,
Reviewed-by: Jani Nikula
This reg is available in BSpec though the bit definitions have not been updated
in the BSpec. Also, it was communicated by the BIOS team.
+
#define GEN6_PMISR0x44020
#define GEN6_PMIMR0x44024 /* rps_lock */
#define GEN6_PMIIR0x44028
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index de01167..a6a8710 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -818,7 +818,8 @@ struct mipi_config {
#define DUAL_LINK_PIXEL_ALT 2
u16 dual_link:2;
u16 lane_cnt:2;
- u16 rsvd3:12;
+ u16 pixel_overlap:3;
+ u16 rsvd3:9;
u16 rsvd4;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index dbe52e9..4e18abd 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -111,6 +111,14 @@ static void intel_dsi_port_enable(struct intel_encoder
*encoder)
enum port port;
u32 temp;
+ if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
+ temp = I915_READ(VLV_CHICKEN_3);
+ temp &= ~PIXEL_OVERLAP_CNT_MASK |
+ intel_dsi->pixel_overlap <<
+ PIXEL_OVERLAP_CNT_SHIFT;
+ I915_WRITE(VLV_CHICKEN_3, temp);
+ }
+
for_each_dsi_port(port, intel_dsi->ports) {
temp = I915_READ(MIPI_PORT_CTRL(port));
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index f2cc2fc..8fe2064 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -28,6 +28,11 @@
#include
#include "intel_drv.h"
+/* Dual Link support */
+#define DSI_DUAL_LINK_NONE 0
+#define DSI_DUAL_LINK_FRONT_BACK 1
+#define DSI_DUAL_LINK_PIXEL_ALT2
+
struct intel_dsi_device {
unsigned int panel_id;
const char *name;
@@ -105,6 +110,7 @@ struct intel_dsi {
u8 escape_clk_div;
u8 dual_link;
+ u8 pixel_overlap;
u32 port_bits;
u32 bw_timer;
u32 dphy_reg;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index f60146f..f8c2269 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -288,6 +288,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
intel_dsi->lane_count = mipi_config->lane_cnt + 1;
intel_dsi->pixel_format = mipi_config->videomode_color_format << 7;
intel_dsi->dual_link = mipi_config->dual_link;
+ intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
if (intel_dsi->dual_link)
intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
@@ -310,6 +311,20 @@ static bool generic_init(struct intel_dsi_device *dsi)
pclk = mode->clock;
+ /* In dual link mode each port needs half of pixel clock */
+ if (intel_dsi->dual_link) {
+ pclk = pclk / 2;
+
+ /* we can enable pixel_overlap if needed by panel. In this
+* case we need to increase the pixelclock for extra pixels
+*/
+ if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
+ pclk += DIV_ROUND_UP(mode->vtotal *
+ intel
Re: [Intel-gfx] [PATCH 02/10] drm/i915: Added port as parameter to the functions which does read/write of DSI Controller
On 12/4/2014 4:52 PM, Daniel Vetter wrote: On Thu, Dec 04, 2014 at 11:14:01AM +0200, Jani Nikula wrote: On Thu, 04 Dec 2014, Gaurav K Singh wrote: This patch is in preparation of DSI dual link panels. For dual link panels, few packets needs to be sent to Port A or Port C or both. Based on the portno from MIPI Sequence Block#53, these sequences needs to be sent accordingly. v2: Addressed review comments by Jani - port variables named properly Signed-off-by: Gaurav K Singh Reviewed-by: Jani Nikula Merged the first two patches, thanks. -Daniel Hi Daniel, Thanks. I have addressed Jani's review comments too for the rest of the patches. With regards, Gaurav ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/9] BYT DSI Dual Link Support
On 12/1/2014 7:17 PM, Jani Nikula wrote: On Sat, 29 Nov 2014, Gaurav K Singh wrote: Hi, These set of patches build on top of the existing DSI Video mode support to enable dual link MIPI panels with high resolutions. These patches have been tested on a 25x16 panel and works well. Good job, it's starting to look good. Please go ahead and start addressing the review comments I made. I've glanced through all the patches, but I'll still have a more in-depth look. Thanks, Jani. Hi Jani, Thanks for the review comments. I have addressed all your review comments and have uploaded the new patch set. Please review. With regards, Gaurav v2: Commit message added to all patches. All review comments of Jani, Nikula have been addressed in the second version of patches. v3: for_each_dsi_port macro used instead of for loop for dual link support Regards Gaurav Gaurav K Singh (9): drm/i915: New functions added for enabling & disabling MIPI Port Ctrl reg drm/i915: Added port as parameter to the functions which does read/write of DSI Controller drm/i915: Add support for port enable/disable for dual link configuration drm/i915: Pixel Clock changes for DSI dual link drm/i915: Dual link needs Shutdown and Turn on packet for both ports drm/i915: Enable DSI PLL for both DSI0 and DSI1 in case of dual link drm/i915: MIPI Timings related changes for dual link drm/i915: Update the DSI disable path to support dual link panel disabling drm/i915: Update the DSI enable path to support dual link panel enabling drivers/gpu/drm/i915/i915_reg.h|5 + drivers/gpu/drm/i915/intel_bios.h |3 +- drivers/gpu/drm/i915/intel_dsi.c | 415 +--- drivers/gpu/drm/i915/intel_dsi.h |7 + drivers/gpu/drm/i915/intel_dsi_cmd.c | 101 --- drivers/gpu/drm/i915/intel_dsi_cmd.h | 46 +-- drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 49 +++- drivers/gpu/drm/i915/intel_dsi_pll.c |9 +- 8 files changed, 383 insertions(+), 252 deletions(-) -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: Add support for port enable/disable for dual link configuration
On 12/1/2014 7:41 PM, Jani Nikula wrote:
On Mon, 01 Dec 2014, Jani Nikula wrote:
On Sat, 29 Nov 2014, Gaurav K Singh wrote:
For Dual Link MIPI Panels, both Port A and Port C should be enabled
during the MIPI encoder enabling sequence. Similarly, during the
disabling sequence, both ports needs to be disabled.
v2: Used for_each_dsi_port macro instead of for loop
Signed-off-by: Gaurav K Singh
Signed-off-by: Shobhit Kumar
---
drivers/gpu/drm/i915/i915_reg.h|1 +
drivers/gpu/drm/i915/intel_dsi.c | 39 +++-
drivers/gpu/drm/i915/intel_dsi.h |1 +
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |7 +
4 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc03fac..c981f5d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6664,6 +6664,7 @@ enum punit_power_well {
#define DPI_ENABLE (1 << 31) /* A + C */
#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT27
#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
+#define DUAL_LINK_MODE_SHIFT 26
#define DUAL_LINK_MODE_MASK (1 << 26)
#define DUAL_LINK_MODE_FRONT_BACK(0 << 26)
#define DUAL_LINK_MODE_PIXEL_ALTERNATIVE (1 << 26)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 693736b..1163a5b 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -108,28 +108,43 @@ static void intel_dsi_port_enable(struct intel_encoder
*encoder)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
- enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
+ enum port port;
u32 temp;
- /* assert ip_tg_enable signal */
- temp = I915_READ(MIPI_PORT_CTRL(port)) & ~LANE_CONFIGURATION_MASK;
- temp = temp | intel_dsi->port_bits;
- I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
- POSTING_READ(MIPI_PORT_CTRL(port));
+ for_each_dsi_port(port, intel_dsi->ports) {
+ temp = I915_READ(MIPI_PORT_CTRL(port));
+
+ if (intel_dsi->dual_link) {
+ if (port == PORT_A)
+ intel_dsi->port_bits |= intel_crtc->pipe ?
+ LANE_CONFIGURATION_DUAL_LINK_B :
+ LANE_CONFIGURATION_DUAL_LINK_A;
+ else
+ intel_dsi->port_bits = 0;
It feels wrong to clobber intel_dsi->port_bits here; the old code didn't
do that either. I think you should either set port_bits somewhere else
(or remove it altogether), and then modify temp depending on port.
Side note, it seems to me intel_dsi->dual_link is becoming redundant, as
intel_dsi->ports will have more than one bit set in the dual link
case. This can be a future cleanup though.
Okay, so it's becoming redundant for checking whether we are using dual
link or not, but it still has it's place for indicating which dual link
mode to use.
BR,
Jani.
Hi Jani,
I preferred if (intel_dsi->dual_link) than if (intel_dsi->ports ==((1 <<
PORT_A) | (1 << PORT_C))), as it looks more cleaner than the second one.
Regarding port_bits, in the old code for single link DSI panels, this
variable was always 0, and it did not had any use. But with dual link
configuration, there are few more register bits which we need to
configure, since the function name was intel_dsi_port_enable, i thought
of setting the corresponding mipi port_bits variable in this function
itself.
With regards,
Gaurav
BR,
Jani.
+ } else
+ temp &= ~LANE_CONFIGURATION_MASK;
+
+ /* assert ip_tg_enable signal */
+ temp = temp | intel_dsi->port_bits;
+ I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
+ POSTING_READ(MIPI_PORT_CTRL(port));
+ }
}
static void intel_dsi_port_disable(struct intel_encoder *encoder)
{
struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
- enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ enum port port;
u32 temp;
- /* de-assert ip_tg_enable signal */
- temp = I915_READ(MIPI_PORT_CTRL(port));
- I915_WRITE(MIPI_PORT_CTRL(port), temp & ~DPI_ENABLE);
- POSTING_READ(MIPI_PORT_CTRL(port));
+ for_each_dsi_port(port, intel_dsi->ports) {
+ /* de-assert ip_tg_enable signal */
+ te
Re: [Intel-gfx] [PATCH 6/9] drm/i915: Enable DSI PLL for both DSI0 and DSI1 in case of dual link
On 12/1/2014 6:57 PM, Jani Nikula wrote:
On Sat, 29 Nov 2014, Gaurav K Singh wrote:
For Dual link MIPI Panels, dsipll clock for both DSI0 and DSI1 needs to be
enabled.
v2: Address review comments by Jani
- Added wait time for PLL to be locked.
Signed-off-by: Gaurav K Singh
Signed-off-by: Shobhit Kumar
---
drivers/gpu/drm/i915/intel_dsi_pll.c |9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c
b/drivers/gpu/drm/i915/intel_dsi_pll.c
index fa7a6ca..93d8e9a 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -243,6 +243,9 @@ static void vlv_configure_dsi_pll(struct intel_encoder
*encoder)
dsi_mnp.dsi_pll_ctrl |= DSI_PLL_CLK_GATE_DSI0_DSIPLL;
+ if (intel_dsi->dual_link)
+ dsi_mnp.dsi_pll_ctrl |= DSI_PLL_CLK_GATE_DSI1_DSIPLL;
+
DRM_DEBUG_KMS("dsi pll div %08x, ctrl %08x\n",
dsi_mnp.dsi_pll_div, dsi_mnp.dsi_pll_ctrl);
@@ -269,12 +272,12 @@ void vlv_enable_dsi_pll(struct intel_encoder *encoder)
tmp |= DSI_PLL_VCO_EN;
vlv_cck_write(dev_priv, CCK_REG_DSI_PLL_CONTROL, tmp);
- mutex_unlock(&dev_priv->dpio_lock);
-
- if (wait_for(I915_READ(PIPECONF(PIPE_A)) & PIPECONF_DSI_PLL_LOCKED,
20)) {
+ if (wait_for(vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL) &
+ DSI_PLL_LOCK, 20)) {
DRM_ERROR("DSI PLL lock failed\n");
return;
}
+ mutex_unlock(&dev_priv->dpio_lock);
This hunk seems to be an unrelated change, I think it should be a
separate patch.
BR,
Jani.
Sure, will put this change as a separate patch.
DRM_DEBUG_KMS("DSI PLL locked\n");
}
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] BYT DSI Dual Link Support
On 11/24/2014 2:31 PM, Jani Nikula wrote: On Mon, 24 Nov 2014, "Singh, Gaurav K" wrote: Hi Jani, Thanks for the review comments. Regarding the first 2 patches, I was doing almost the same thing in my 3rd and 4th patch. But your patches are more generic. Regarding the 3rd patch, I have a comment: Since in case of dual link panels, few panels may require sequence to be sent only on Port A or Port C or both. In that case, for_each_dsi_port(port, intel_dsi->ports) will cause it to be sent to both ports. To resolve this, in the earlier patches, intel_dsi->port was used which gets calculated to either 0 or 1 in mipi_exec_send_packet(). This value of 0 or 1 is dependent on sequence block#53. From now on as we will be using the _PORT3() macro for using proper MIPI regs, then for this scenario, we may need to have some workaround/hardcode type of code again. May I know your suggestion on this? Perhaps patch 3/3 was a bad place for the for_each_dsi_port example - it should have been put into intel_dsi.c. Maybe you'll need to add the port as parameter to the functions in intel_dsi_cmd.c, and let the caller decide which port should be used? I want to avoid any platform/panel specific special casing in intel_dsi.c and intel_dsi_cmd.c. I think the general case for for_each_dsi_port is in intel_dsi.c anyway. BR, Jani. Jani, I have taken care of this in the next version of dual link patches. PATCH 3/3 need not be merged as it has already been taken care as part of dual link implementation. Once your first two patches gets merged in drm-intel-nightly branch, I can push the next version of dual link patches for review. With regards, Gaurav With regards, Gaurav On 11/14/2014 8:24 PM, Jani Nikula wrote: Hi Shobhit and Gaurav - I've been pondering this whole MIPI DSI pipes vs. ports thing and discussing with Ville and others. Rather than try and fail in explaining the ideas, here are some concrete patches to describe what I'd like to be done first. The most important thing is that we don't confuse the pipes and ports. Getting confused was easy with the pipe B mapping to port C, and the register defines being very confused/confusing about it. These patches attempt to fix that. Before adding dual link support, there's a simple function mapping the pipe to port. Next up is expanding that to handle multiple ports driven from one pipe. That's handled by adding intel_dsi->ports bitmap that has the bit set for each port that is to be driven. I've added the bitmap and some helpers to iterate over the configured ports, but there's no actual support for doing the configuration. I'm hoping you could take over from here. There's a sample patch about the usage. I'm sorry it's taken me so long to reply. With the new stuff coming in, I really think it's important to get the foundation right first. Especially because I'm to blame for getting some of the port/pipe stuff confused in the first place... BR, Jani. Jani Nikula (3): drm/i915/dsi: clean up MIPI DSI pipe vs. port usage drm/i915/dsi: add ports to intel_dsi to describe the ports being driven drm/i915/dsi: an example how to handle dual link for each port drivers/gpu/drm/i915/i915_reg.h | 303 ++- drivers/gpu/drm/i915/intel_dsi.c | 151 - drivers/gpu/drm/i915/intel_dsi.h | 19 +++ drivers/gpu/drm/i915/intel_dsi_cmd.c | 76 - 4 files changed, 290 insertions(+), 259 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/dsi: clean up MIPI DSI pipe vs. port usage
On 11/14/2014 8:24 PM, Jani Nikula wrote:
MIPI DSI works on ports A and C, which map to pipes A and B,
respectively. Things are going to get more complicated with the
introduction of dual link DSI support, so clean up the register defines
and code to match reality.
Signed-off-by: Jani Nikula
Reviewed-by: Gaurav K Singh
---
drivers/gpu/drm/i915/i915_reg.h | 303 ++-
drivers/gpu/drm/i915/intel_dsi.c | 148 -
drivers/gpu/drm/i915/intel_dsi.h | 16 ++
drivers/gpu/drm/i915/intel_dsi_cmd.c | 64
4 files changed, 277 insertions(+), 254 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a143127eb451..250043f3f22e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -31,6 +31,8 @@
#define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
#define _PIPE3(pipe, a, b, c) ((pipe) == PIPE_A ? (a) : \
(pipe) == PIPE_B ? (b) : (c))
+#define _PORT3(port, a, b, c) ((port) == PORT_A ? (a) : \
+ (port) == PORT_B ? (b) : (c))
#define _MASKED_BIT_ENABLE(a) (((a) << 16) | (a))
#define _MASKED_BIT_DISABLE(a) ((a) << 16)
@@ -1492,7 +1494,7 @@ enum punit_power_well {
#define I915_ISP_INTERRUPT(1<<22)
#define I915_LPE_PIPE_B_INTERRUPT (1<<21)
#define I915_LPE_PIPE_A_INTERRUPT (1<<20)
-#define I915_MIPIB_INTERRUPT (1<<19)
+#define I915_MIPIC_INTERRUPT (1<<19)
#define I915_MIPIA_INTERRUPT (1<<18)
#define I915_PIPE_CONTROL_NOTIFY_INTERRUPT(1<<18)
#define I915_DISPLAY_PORT_INTERRUPT (1<<17)
@@ -6612,29 +6614,30 @@ enum punit_power_well {
#define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME,
_PIPE_B_CSC_POSTOFF_ME)
#define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO,
_PIPE_B_CSC_POSTOFF_LO)
-/* VLV MIPI registers */
+/* MIPI DSI registers */
+
+#define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */
#define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
-#define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
-#define MIPI_PORT_CTRL(tc) _TRANSCODER(tc, _MIPIA_PORT_CTRL, \
- _MIPIB_PORT_CTRL)
-#define DPI_ENABLE(1 << 31) /* A + B */
+#define _MIPIC_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
+#define MIPI_PORT_CTRL(port) _MIPI_PORT(port, _MIPIA_PORT_CTRL,
_MIPIC_PORT_CTRL)
+#define DPI_ENABLE(1 << 31) /* A + C */
#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT27
#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
#define DUAL_LINK_MODE_MASK (1 << 26)
#define DUAL_LINK_MODE_FRONT_BACK(0 << 26)
#define DUAL_LINK_MODE_PIXEL_ALTERNATIVE (1 << 26)
-#define DITHERING_ENABLE (1 << 25) /* A + B */
+#define DITHERING_ENABLE (1 << 25) /* A + C */
#define FLOPPED_HSTX (1 << 23)
#define DE_INVERT(1 << 19) /* XXX */
#define MIPIA_FLISDSI_DELAY_COUNT_SHIFT 18
#define MIPIA_FLISDSI_DELAY_COUNT_MASK (0xf << 18)
#define AFE_LATCHOUT (1 << 17)
#define LP_OUTPUT_HOLD (1 << 16)
-#define MIPIB_FLISDSI_DELAY_COUNT_HIGH_SHIFT 15
-#define MIPIB_FLISDSI_DELAY_COUNT_HIGH_MASK (1 << 15)
-#define MIPIB_MIPI4DPHY_DELAY_COUNT_SHIFT 11
-#define MIPIB_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 11)
+#define MIPIC_FLISDSI_DELAY_COUNT_HIGH_SHIFT 15
+#define MIPIC_FLISDSI_DELAY_COUNT_HIGH_MASK (1 << 15)
+#define MIPIC_MIPI4DPHY_DELAY_COUNT_SHIFT 11
+#define MIPIC_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 11)
#define CSB_SHIFT9
#define CSB_MASK (3 << 9)
#define CSB_20MHZ(0 << 9)
@@ -6643,10 +6646,10 @@ enum punit_power_well {
#define BANDGAP_MASK (1 << 8)
#define BANDGAP_PNW_CIRCUIT (0 << 8)
#define BANDGAP_LNC_CIRCUIT (1 << 8)
-#define MIPIB_FLISDSI_DELAY_COUNT_LOW_SHIFT 5
-#define MIPIB_FLISDSI_DELAY_COUNT_LOW_MASK(7 << 5)
-#define TEARING_EFFECT_DELAY (1 << 4) /* A + B */
-#define TEARING_EFFECT_SHIFT 2 /* A + B */
+#define MIPIC_FLISDSI_DELAY_COUNT_LOW_SHIFT 5
+#define MIPIC_FLISDSI_DELAY_COUNT_LOW_MASK
Re: [Intel-gfx] [PATCH 2/3] drm/i915/dsi: add ports to intel_dsi to describe the ports being driven
On 11/14/2014 8:24 PM, Jani Nikula wrote:
Later on this can include multiple ports (e.g. (1 << PORT_A) | (1 <<
PORT_C)) to describe dual link DSI.
Signed-off-by: Jani Nikula
Reviewed-by: Gaurav K Singh
---
drivers/gpu/drm/i915/intel_dsi.c | 7 +--
drivers/gpu/drm/i915/intel_dsi.h | 3 +++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 35842a6687d8..259cb4ab2067 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -749,10 +749,13 @@ void intel_dsi_init(struct drm_device *dev)
intel_connector->unregister = intel_connector_unregister;
/* Pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI port C */
- if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA)
+ if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) {
intel_encoder->crtc_mask = (1 << PIPE_A);
- else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC)
+ intel_dsi->ports = (1 << PORT_A);
+ } else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC) {
intel_encoder->crtc_mask = (1 << PIPE_B);
+ intel_dsi->ports = (1 << PORT_C);
+ }
for (i = 0; i < ARRAY_SIZE(intel_dsi_devices); i++) {
dsi = &intel_dsi_devices[i];
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 97a6f621774f..7f5d0280b9d7 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -78,6 +78,9 @@ struct intel_dsi {
struct intel_connector *attached_connector;
+ /* bit mask of ports being driven */
+ u16 ports;
+
/* if true, use HS mode, otherwise LP */
bool hs;
This patch looks good.
With regards,
Gaurav
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/dsi: clean up MIPI DSI pipe vs. port usage
On 11/26/2014 11:38 PM, Daniel Vetter wrote: On Wed, Nov 26, 2014 at 10:50:46PM +0530, Singh, Gaurav K wrote: This patch has some style issues. Please address them. Please be more specific, that's rather non-actionable review. Also general rule of thumb is that if it doesn't look offensive and the patch is otherwise good it still deserves an r-b. I can easily fix up small things while applying (and do that all the time). -Daniel checkpatch.pl is throwing few warnings and one error which are mainly because of lines more than 80 characters. The patch is otherwise looking good. I will add Reviewed-by field in the patch. with regards, Gaurav ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] BYT DSI Dual Link Support
On 11/24/2014 2:31 PM, Jani Nikula wrote: On Mon, 24 Nov 2014, "Singh, Gaurav K" wrote: Hi Jani, Thanks for the review comments. Regarding the first 2 patches, I was doing almost the same thing in my 3rd and 4th patch. But your patches are more generic. Regarding the 3rd patch, I have a comment: Since in case of dual link panels, few panels may require sequence to be sent only on Port A or Port C or both. In that case, for_each_dsi_port(port, intel_dsi->ports) will cause it to be sent to both ports. To resolve this, in the earlier patches, intel_dsi->port was used which gets calculated to either 0 or 1 in mipi_exec_send_packet(). This value of 0 or 1 is dependent on sequence block#53. From now on as we will be using the _PORT3() macro for using proper MIPI regs, then for this scenario, we may need to have some workaround/hardcode type of code again. May I know your suggestion on this? Perhaps patch 3/3 was a bad place for the for_each_dsi_port example - it should have been put into intel_dsi.c. Maybe you'll need to add the port as parameter to the functions in intel_dsi_cmd.c, and let the caller decide which port should be used? I want to avoid any platform/panel specific special casing in intel_dsi.c and intel_dsi_cmd.c. I think the general case for for_each_dsi_port is in intel_dsi.c anyway. BR, Jani. Hi Jani, My patches are ready on top of your first 2 patches. Once your patches are merged, I will push my whole set of patches for review. With regards, Gaurav With regards, Gaurav On 11/14/2014 8:24 PM, Jani Nikula wrote: Hi Shobhit and Gaurav - I've been pondering this whole MIPI DSI pipes vs. ports thing and discussing with Ville and others. Rather than try and fail in explaining the ideas, here are some concrete patches to describe what I'd like to be done first. The most important thing is that we don't confuse the pipes and ports. Getting confused was easy with the pipe B mapping to port C, and the register defines being very confused/confusing about it. These patches attempt to fix that. Before adding dual link support, there's a simple function mapping the pipe to port. Next up is expanding that to handle multiple ports driven from one pipe. That's handled by adding intel_dsi->ports bitmap that has the bit set for each port that is to be driven. I've added the bitmap and some helpers to iterate over the configured ports, but there's no actual support for doing the configuration. I'm hoping you could take over from here. There's a sample patch about the usage. I'm sorry it's taken me so long to reply. With the new stuff coming in, I really think it's important to get the foundation right first. Especially because I'm to blame for getting some of the port/pipe stuff confused in the first place... BR, Jani. Jani Nikula (3): drm/i915/dsi: clean up MIPI DSI pipe vs. port usage drm/i915/dsi: add ports to intel_dsi to describe the ports being driven drm/i915/dsi: an example how to handle dual link for each port drivers/gpu/drm/i915/i915_reg.h | 303 ++- drivers/gpu/drm/i915/intel_dsi.c | 151 - drivers/gpu/drm/i915/intel_dsi.h | 19 +++ drivers/gpu/drm/i915/intel_dsi_cmd.c | 76 - 4 files changed, 290 insertions(+), 259 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/dsi: clean up MIPI DSI pipe vs. port usage
On 11/14/2014 8:24 PM, Jani Nikula wrote:
MIPI DSI works on ports A and C, which map to pipes A and B,
respectively. Things are going to get more complicated with the
introduction of dual link DSI support, so clean up the register defines
and code to match reality.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_reg.h | 303 ++-
drivers/gpu/drm/i915/intel_dsi.c | 148 -
drivers/gpu/drm/i915/intel_dsi.h | 16 ++
drivers/gpu/drm/i915/intel_dsi_cmd.c | 64
4 files changed, 277 insertions(+), 254 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a143127eb451..250043f3f22e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -31,6 +31,8 @@
#define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
#define _PIPE3(pipe, a, b, c) ((pipe) == PIPE_A ? (a) : \
(pipe) == PIPE_B ? (b) : (c))
+#define _PORT3(port, a, b, c) ((port) == PORT_A ? (a) : \
+ (port) == PORT_B ? (b) : (c))
#define _MASKED_BIT_ENABLE(a) (((a) << 16) | (a))
#define _MASKED_BIT_DISABLE(a) ((a) << 16)
@@ -1492,7 +1494,7 @@ enum punit_power_well {
#define I915_ISP_INTERRUPT(1<<22)
#define I915_LPE_PIPE_B_INTERRUPT (1<<21)
#define I915_LPE_PIPE_A_INTERRUPT (1<<20)
-#define I915_MIPIB_INTERRUPT (1<<19)
+#define I915_MIPIC_INTERRUPT (1<<19)
#define I915_MIPIA_INTERRUPT (1<<18)
#define I915_PIPE_CONTROL_NOTIFY_INTERRUPT(1<<18)
#define I915_DISPLAY_PORT_INTERRUPT (1<<17)
@@ -6612,29 +6614,30 @@ enum punit_power_well {
#define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME,
_PIPE_B_CSC_POSTOFF_ME)
#define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO,
_PIPE_B_CSC_POSTOFF_LO)
-/* VLV MIPI registers */
+/* MIPI DSI registers */
+
+#define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */
#define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
-#define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
-#define MIPI_PORT_CTRL(tc) _TRANSCODER(tc, _MIPIA_PORT_CTRL, \
- _MIPIB_PORT_CTRL)
-#define DPI_ENABLE(1 << 31) /* A + B */
+#define _MIPIC_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
+#define MIPI_PORT_CTRL(port) _MIPI_PORT(port, _MIPIA_PORT_CTRL,
_MIPIC_PORT_CTRL)
+#define DPI_ENABLE(1 << 31) /* A + C */
#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT27
#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
#define DUAL_LINK_MODE_MASK (1 << 26)
#define DUAL_LINK_MODE_FRONT_BACK(0 << 26)
#define DUAL_LINK_MODE_PIXEL_ALTERNATIVE (1 << 26)
-#define DITHERING_ENABLE (1 << 25) /* A + B */
+#define DITHERING_ENABLE (1 << 25) /* A + C */
#define FLOPPED_HSTX (1 << 23)
#define DE_INVERT(1 << 19) /* XXX */
#define MIPIA_FLISDSI_DELAY_COUNT_SHIFT 18
#define MIPIA_FLISDSI_DELAY_COUNT_MASK (0xf << 18)
#define AFE_LATCHOUT (1 << 17)
#define LP_OUTPUT_HOLD (1 << 16)
-#define MIPIB_FLISDSI_DELAY_COUNT_HIGH_SHIFT 15
-#define MIPIB_FLISDSI_DELAY_COUNT_HIGH_MASK (1 << 15)
-#define MIPIB_MIPI4DPHY_DELAY_COUNT_SHIFT 11
-#define MIPIB_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 11)
+#define MIPIC_FLISDSI_DELAY_COUNT_HIGH_SHIFT 15
+#define MIPIC_FLISDSI_DELAY_COUNT_HIGH_MASK (1 << 15)
+#define MIPIC_MIPI4DPHY_DELAY_COUNT_SHIFT 11
+#define MIPIC_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 11)
#define CSB_SHIFT9
#define CSB_MASK (3 << 9)
#define CSB_20MHZ(0 << 9)
@@ -6643,10 +6646,10 @@ enum punit_power_well {
#define BANDGAP_MASK (1 << 8)
#define BANDGAP_PNW_CIRCUIT (0 << 8)
#define BANDGAP_LNC_CIRCUIT (1 << 8)
-#define MIPIB_FLISDSI_DELAY_COUNT_LOW_SHIFT 5
-#define MIPIB_FLISDSI_DELAY_COUNT_LOW_MASK(7 << 5)
-#define TEARING_EFFECT_DELAY (1 << 4) /* A + B */
-#define TEARING_EFFECT_SHIFT 2 /* A + B */
+#define MIPIC_FLISDSI_DELAY_COUNT_LOW_SHIFT 5
+#define MIPIC_FLISDSI_DELAY_COUNT_LOW_MASK(7 << 5)
+#define TEARING_EFFE
Re: [Intel-gfx] [PATCH 0/3] BYT DSI Dual Link Support
Hi Jani, Thanks for the review comments. Regarding the first 2 patches, I was doing almost the same thing in my 3rd and 4th patch. But your patches are more generic. Regarding the 3rd patch, I have a comment: Since in case of dual link panels, few panels may require sequence to be sent only on Port A or Port C or both. In that case, for_each_dsi_port(port, intel_dsi->ports) will cause it to be sent to both ports. To resolve this, in the earlier patches, intel_dsi->port was used which gets calculated to either 0 or 1 in mipi_exec_send_packet(). This value of 0 or 1 is dependent on sequence block#53. From now on as we will be using the _PORT3() macro for using proper MIPI regs, then for this scenario, we may need to have some workaround/hardcode type of code again. May I know your suggestion on this? With regards, Gaurav On 11/14/2014 8:24 PM, Jani Nikula wrote: Hi Shobhit and Gaurav - I've been pondering this whole MIPI DSI pipes vs. ports thing and discussing with Ville and others. Rather than try and fail in explaining the ideas, here are some concrete patches to describe what I'd like to be done first. The most important thing is that we don't confuse the pipes and ports. Getting confused was easy with the pipe B mapping to port C, and the register defines being very confused/confusing about it. These patches attempt to fix that. Before adding dual link support, there's a simple function mapping the pipe to port. Next up is expanding that to handle multiple ports driven from one pipe. That's handled by adding intel_dsi->ports bitmap that has the bit set for each port that is to be driven. I've added the bitmap and some helpers to iterate over the configured ports, but there's no actual support for doing the configuration. I'm hoping you could take over from here. There's a sample patch about the usage. I'm sorry it's taken me so long to reply. With the new stuff coming in, I really think it's important to get the foundation right first. Especially because I'm to blame for getting some of the port/pipe stuff confused in the first place... BR, Jani. Jani Nikula (3): drm/i915/dsi: clean up MIPI DSI pipe vs. port usage drm/i915/dsi: add ports to intel_dsi to describe the ports being driven drm/i915/dsi: an example how to handle dual link for each port drivers/gpu/drm/i915/i915_reg.h | 303 ++- drivers/gpu/drm/i915/intel_dsi.c | 151 - drivers/gpu/drm/i915/intel_dsi.h | 19 +++ drivers/gpu/drm/i915/intel_dsi_cmd.c | 76 - 4 files changed, 290 insertions(+), 259 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: MIPI Port Ctrl related changes for dual link configuration
On 9/24/2014 2:57 PM, Jani Nikula wrote:
On Wed, 24 Sep 2014, Gaurav K Singh wrote:
Signed-off-by: Gaurav K Singh
Signed-off-by: Shobhit Kumar
---
drivers/gpu/drm/i915/i915_reg.h|1 +
drivers/gpu/drm/i915/intel_dsi.c | 53 ++--
drivers/gpu/drm/i915/intel_dsi.h |1 +
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |1 +
4 files changed, 45 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ad8179b..922d807 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6215,6 +6215,7 @@ enum punit_power_well {
#define DPI_ENABLE (1 << 31) /* A + B */
#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT27
#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
+#define DUAL_LINK_MODE_SHIFT 26
#define DUAL_LINK_MODE_MASK (1 << 26)
#define DUAL_LINK_MODE_FRONT_BACK(0 << 26)
#define DUAL_LINK_MODE_PIXEL_ALTERNATIVE (1 << 26)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index e456ca9..3b1890e 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -109,13 +109,31 @@ static void intel_dsi_port_enable(struct intel_encoder
*encoder)
struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
enum pipe pipe = intel_crtc->pipe;
- u32 temp;
-
- /* assert ip_tg_enable signal */
- temp = I915_READ(MIPI_PORT_CTRL(pipe)) & ~LANE_CONFIGURATION_MASK;
- temp = temp | intel_dsi->port_bits;
- I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE);
- POSTING_READ(MIPI_PORT_CTRL(pipe));
+ u32 temp, port_control = 0;
+
+ if (intel_dsi->dual_link) {
+ port_control = (intel_dsi->dual_link - 1)
+ << DUAL_LINK_MODE_SHIFT;
+ port_control |= pipe ? LANE_CONFIGURATION_DUAL_LINK_B :
+ LANE_CONFIGURATION_DUAL_LINK_A;
+ /*For Port A */
+ temp = I915_READ(MIPI_PORT_CTRL(0));
+ temp = temp | port_control;
+ I915_WRITE(MIPI_PORT_CTRL(0), temp | DPI_ENABLE);
+ POSTING_READ(MIPI_PORT_CTRL(0));
+
+ /* For Port C */
+ temp = I915_READ(MIPI_PORT_CTRL(1));
+ I915_WRITE(MIPI_PORT_CTRL(1), temp | DPI_ENABLE);
+ POSTING_READ(MIPI_PORT_CTRL(1));
This calls for a cleanup in i915_reg.h for per port vs. per transcoder
registers. MIPI_PORT_CTRL(1) uses _TRANSCODER macro. We also have enum
port with PORT_C == 2. This gets confusing.
Are you suggesting to use _PORT macro instead of _TRANSCODER macro?
+ } else {
+ /* assert ip_tg_enable signal */
+ temp = I915_READ(MIPI_PORT_CTRL(pipe)) &
+ ~LANE_CONFIGURATION_MASK;
+ temp = temp | intel_dsi->port_bits;
+ I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE);
+ POSTING_READ(MIPI_PORT_CTRL(pipe));
+ }
}
static void intel_dsi_port_disable(struct intel_encoder *encoder)
@@ -123,13 +141,26 @@ static void intel_dsi_port_disable(struct intel_encoder
*encoder)
struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
enum pipe pipe = intel_crtc->pipe;
u32 temp;
- /* de-assert ip_tg_enable signal */
- temp = I915_READ(MIPI_PORT_CTRL(pipe));
- I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
- POSTING_READ(MIPI_PORT_CTRL(pipe));
+ if (intel_dsi->dual_link) {
+ /*For Port A */
+ temp = I915_READ(MIPI_PORT_CTRL(0));
+ I915_WRITE(MIPI_PORT_CTRL(0), temp & ~DPI_ENABLE);
+ POSTING_READ(MIPI_PORT_CTRL(0));
+
+ /* For Port C */
+ temp = I915_READ(MIPI_PORT_CTRL(1));
+ I915_WRITE(MIPI_PORT_CTRL(1), temp & ~DPI_ENABLE);
+ POSTING_READ(MIPI_PORT_CTRL(1));
+ } else {
+ /* de-assert ip_tg_enable signal */
+ temp = I915_READ(MIPI_PORT_CTRL(pipe));
+ I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
+ POSTING_READ(MIPI_PORT_CTRL(pipe));
+ }
}
static void intel_dsi_device_ready(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 587e71f..950ab41 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -101,6 +101,7 @@ struct intel_dsi
