Re: [RFC v2 4/4] drm/i915: Enable DSI panel enable/disable based on PMIC

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

 



On 1/9/2015 6:47 PM, Jani Nikula wrote:
On Fri, 02 Jan 2015, Shobhit Kumar <shobhit.kumar@xxxxxxxxx> wrote:
This allows for proper PPS during enable/disable of BYT-T platforms
where these signals are routed through PMIC. Needs DRM_PANEL to be
selected by default as well

Signed-off-by: Shobhit Kumar <shobhit.kumar@xxxxxxxxx>
---
  drivers/gpu/drm/i915/Kconfig               |  1 +
  drivers/gpu/drm/i915/intel_dsi.c           | 16 ++++++++++++++++
  drivers/gpu/drm/i915/intel_dsi.h           |  6 ++++++
  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |  1 +
  4 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 4e39ab3..3210dbb 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -18,6 +18,7 @@ config DRM_I915
  	select INPUT if ACPI
  	select ACPI_VIDEO if ACPI
  	select ACPI_BUTTON if ACPI
+	select DRM_PANEL
  	help
  	  Choose this option if you have a system that has "Intel Graphics
  	  Media Accelerator" or "HD Graphics" integrated graphics,
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 42b6d6f..431e7cb 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -26,6 +26,7 @@
  #include <drm/drmP.h>
  #include <drm/drm_crtc.h>
  #include <drm/drm_edid.h>
+#include <drm/drm_panel.h>
  #include <drm/i915_drm.h>
  #include <linux/slab.h>
  #include "i915_drv.h"
@@ -230,6 +231,8 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)

  	DRM_DEBUG_KMS("\n");

+	drm_panel_enable(intel_dsi->panel);
+
  	/* Disable DPOunit clock gating, can stall pipe
  	 * and we need DPLL REFA always enabled */
  	tmp = I915_READ(DPLL(pipe));
@@ -392,6 +395,8 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)

  	msleep(intel_dsi->panel_off_delay);
  	msleep(intel_dsi->panel_pwr_cycle_delay);
+
+	drm_panel_disable(intel_dsi->panel);
  }

As I explained in a private mail, I intend to convert all of our panel
driver callbacks to the drm_panel model. So we'd have two sets of
drm_panel_* hooks sprinkled here, one for handling the panel power and
the other for the generic vbt panel driver. An alternative would be to
have the vbt panel driver handle this internally, but I don't really
know which one is better. In any case this has a fairly small footprint
so it's easy to change one way or another.

PMIC based control is something unique for current platforms (BYT/CHT) and all future platforms will go with SoC based control. Given that, I think we should not put this is VBT panel driver and keep separate.



  static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
@@ -896,6 +901,17 @@ void intel_dsi_init(struct drm_device *dev)
  	fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
  	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);

+	/* Initialize the PMIC based drm_panel if available on the platform */
+	if (intel_dsi->pps_blc == PPS_BLC_PMIC) {
+		intel_dsi->panel = name_drm_find_panel("crystal_cove_panel");
+		if (!intel_dsi->panel) {
+			DRM_ERROR("PMIC Panel control will not work !!\n");
+			return;
+		}
+
+		drm_panel_attach(intel_dsi->panel, connector);
+	}
+

I think there's an init order problem here. If the panel driver hasn't
been registered yet this will fail. I don't know what the answer to that
should be.

Agree and for my testing I made both PMIC and This panel driver as in-built and i915 as module. I was hoping for some answer !!


  	return;

  err:
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 8fe2064..4a9242d 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -33,6 +33,9 @@
  #define DSI_DUAL_LINK_FRONT_BACK	1
  #define DSI_DUAL_LINK_PIXEL_ALT		2

+#define PPS_BLC_PMIC	0
+#define PPS_BLC_SOC	1
+
  struct intel_dsi_device {
  	unsigned int panel_id;
  	const char *name;
@@ -83,6 +86,8 @@ struct intel_dsi {

  	struct intel_connector *attached_connector;

+	struct drm_panel *panel;
+
  	/* bit mask of ports being driven */
  	u16 ports;

@@ -116,6 +121,7 @@ struct intel_dsi {
  	u32 dphy_reg;
  	u32 video_frmt_cfg_bits;
  	u16 lp_byte_clk;
+	u8 pps_blc;

  	/* timeouts in byte clocks */
  	u16 lp_rx_timeout;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 5493aef..0612d33 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -297,6 +297,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
  	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;
+	intel_dsi->pps_blc = mipi_config->pwm_blc;

If the drm_panel for crystal cove is going to be like in this patch, I
think this part belongs in intel_dsi.c.

If you are suggesting check mipi_config->pwm_blc directly in intel_dsi, that can be done. Was just abstracting all VBT fields in generic VBT based panel driver

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




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