| Message ID | 1420206085-2913-5-git-send-email-shobhit.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 02 Jan 2015, Shobhit Kumar <shobhit.kumar@intel.com> 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@intel.com> > --- > 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. > > 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. > 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 (intel_dsi->dual_link) > intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C)); > -- > 1.9.1 >
On 1/9/2015 6:47 PM, Jani Nikula wrote: > On Fri, 02 Jan 2015, Shobhit Kumar <shobhit.kumar@intel.com> 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@intel.com> >> --- >> 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
On Mon, Jan 12, 2015 at 01:53:08PM +0530, Kumar, Shobhit wrote: > > > >> > >> 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 !! The answer is to use the component framework and delayed probing. Afaik there's unfortunately not yet any ready-made integration with drm panel/bridge yet since this is all very much in flux. -Daniel
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); } 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); + } + 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 (intel_dsi->dual_link) intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
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@intel.com> --- 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(+)