| Message ID | 1434970465-12687-8-git-send-email-shobhit.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jun 22, 2015 at 04:24:25PM +0530, Shobhit Kumar wrote: > Use the CRC PWM device in intel_panel.c and add new MIPI backlight > specififc callbacks > > v2: Modify to use pwm_config callback > v3: Addressed Jani's comments > - Renamed all function as pwm_* instead of vlv_* > - Call intel_panel_actually_set_backlight in enable function > - Return -ENODEV in case pwm_get fails > - in case pwm_config error return error cdoe from pwm_config > - Cleanup pwm in intel_panel_destroy_backlight > > CC: Samuel Ortiz <sameo@linux.intel.com> > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Alexandre Courbot <gnurou@gmail.com> > Cc: Thierry Reding <thierry.reding@gmail.com> > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> > --- > drivers/gpu/drm/i915/intel_drv.h | 4 ++ > drivers/gpu/drm/i915/intel_dsi.c | 6 +++ > drivers/gpu/drm/i915/intel_panel.c | 95 ++++++++++++++++++++++++++++++++++++-- > 3 files changed, 100 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 2afb31a..561c17f 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -182,6 +182,10 @@ struct intel_panel { > bool enabled; > bool combination_mode; /* gen 2/4 only */ > bool active_low_pwm; > + > + /* PWM chip */ > + struct pwm_device *pwm; > + > struct backlight_device *device; > } backlight; > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index c4db74a..be8722c 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -402,6 +402,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder) > > intel_dsi_port_enable(encoder); > } > + > + intel_panel_enable_backlight(intel_dsi->attached_connector); > } > > static void intel_dsi_pre_enable(struct intel_encoder *encoder) > @@ -466,6 +468,8 @@ static void intel_dsi_pre_disable(struct intel_encoder *encoder) > > DRM_DEBUG_KMS("\n"); > > + intel_panel_disable_backlight(intel_dsi->attached_connector); > + > if (is_vid_mode(intel_dsi)) { > /* Send Shutdown command to the panel in LP mode */ > for_each_dsi_port(port, intel_dsi->ports) > @@ -1132,6 +1136,8 @@ void intel_dsi_init(struct drm_device *dev) > } > > intel_panel_init(&intel_connector->panel, fixed_mode, NULL); > + intel_panel_setup_backlight(connector, > + (intel_encoder->crtc_mask = (1 << PIPE_A)) ? PIPE_A : PIPE_B); ^ Whoops. But since the PWM backlight doesn't need the initial pipe for anything you can actually just pass INVALID_PIPE here. > > return; > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index 7d83527..2aa30db 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -32,8 +32,12 @@ > > #include <linux/kernel.h> > #include <linux/moduleparam.h> > +#include <linux/pwm.h> > #include "intel_drv.h" > > +#define CRC_PMIC_PWM_PERIOD_NS 21333 > +#define CRC_PMIC_PWM_STEPS 255 This define appears to be unused. > + > void > intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode, > struct drm_display_mode *adjusted_mode) > @@ -544,6 +548,15 @@ static u32 bxt_get_backlight(struct intel_connector *connector) > return I915_READ(BXT_BLC_PWM_DUTY1); > } > > +static u32 pwm_get_backlight(struct intel_connector *connector) > +{ > + struct intel_panel *panel = &connector->panel; > + int duty_ns; > + > + duty_ns = pwm_get_duty_cycle(panel->backlight.pwm); > + return DIV_ROUND_UP(duty_ns * 100, CRC_PMIC_PWM_PERIOD_NS); > +} > + > static u32 intel_panel_get_backlight(struct intel_connector *connector) > { > struct drm_device *dev = connector->base.dev; > @@ -632,6 +645,14 @@ static void bxt_set_backlight(struct intel_connector *connector, u32 level) > I915_WRITE(BXT_BLC_PWM_DUTY1, level); > } > > +static void pwm_set_backlight(struct intel_connector *connector, u32 level) > +{ > + struct intel_panel *panel = &connector->panel; > + int duty_ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100); > + > + pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS); > +} > + > static void > intel_panel_actually_set_backlight(struct intel_connector *connector, u32 level) > { > @@ -769,6 +790,16 @@ static void bxt_disable_backlight(struct intel_connector *connector) > I915_WRITE(BXT_BLC_PWM_CTL1, tmp & ~BXT_BLC_PWM_ENABLE); > } > > +static void pwm_disable_backlight(struct intel_connector *connector) > +{ > + struct intel_panel *panel = &connector->panel; > + > + /* Disable the backlight */ > + pwm_config(panel->backlight.pwm, 0, CRC_PMIC_PWM_PERIOD_NS); > + usleep_range(2000, 3000); > + pwm_disable(panel->backlight.pwm); > +} > + > void intel_panel_disable_backlight(struct intel_connector *connector) > { > struct drm_device *dev = connector->base.dev; > @@ -1002,6 +1033,14 @@ static void bxt_enable_backlight(struct intel_connector *connector) > I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl | BXT_BLC_PWM_ENABLE); > } > > +static void pwm_enable_backlight(struct intel_connector *connector) > +{ > + struct intel_panel *panel = &connector->panel; > + > + pwm_enable(panel->backlight.pwm); > + intel_panel_actually_set_backlight(connector, panel->backlight.level); > +} > + > void intel_panel_enable_backlight(struct intel_connector *connector) > { > struct drm_device *dev = connector->base.dev; > @@ -1378,6 +1417,40 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused) > return 0; > } > > +static int pwm_setup_backlight(struct intel_connector *connector, > + enum pipe pipe) > +{ > + struct drm_device *dev = connector->base.dev; > + struct intel_panel *panel = &connector->panel; > + int retval = -1; No need to initialize it. > + > + /* Get the PWM chip for backlight control */ > + panel->backlight.pwm = pwm_get(dev->dev, "pwm_backlight"); > + if (IS_ERR(panel->backlight.pwm)) { > + DRM_ERROR("Failed to own the pwm chip\n"); > + panel->backlight.pwm = NULL; > + return -ENODEV; > + } > + > + retval = pwm_config(panel->backlight.pwm, CRC_PMIC_PWM_PERIOD_NS, > + CRC_PMIC_PWM_PERIOD_NS); > + if (retval < 0) { > + DRM_ERROR("Failed to configure the pwm chip\n"); > + pwm_put(panel->backlight.pwm); > + panel->backlight.pwm = NULL; > + return retval; > + } > + > + panel->backlight.min = 0; /* 0% */ > + panel->backlight.max = 100; /* 100% */ > + panel->backlight.level = DIV_ROUND_UP( > + pwm_get_duty_cycle(panel->backlight.pwm) * 100, > + CRC_PMIC_PWM_PERIOD_NS); > + panel->backlight.enabled = panel->backlight.level != 0; > + > + return 0; > +} > + > int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe) > { > struct drm_device *dev = connector->dev; > @@ -1421,6 +1494,10 @@ void intel_panel_destroy_backlight(struct drm_connector *connector) > struct intel_connector *intel_connector = to_intel_connector(connector); > struct intel_panel *panel = &intel_connector->panel; > > + /* dispose of the pwm */ > + if (panel->backlight.pwm) > + pwm_put(panel->backlight.pwm); > + > panel->backlight.present = false; > } > > @@ -1448,11 +1525,19 @@ void intel_panel_init_backlight_funcs(struct drm_device *dev) > dev_priv->display.set_backlight = pch_set_backlight; > dev_priv->display.get_backlight = pch_get_backlight; > } else if (IS_VALLEYVIEW(dev)) { > - dev_priv->display.setup_backlight = vlv_setup_backlight; > - dev_priv->display.enable_backlight = vlv_enable_backlight; > - dev_priv->display.disable_backlight = vlv_disable_backlight; > - dev_priv->display.set_backlight = vlv_set_backlight; > - dev_priv->display.get_backlight = vlv_get_backlight; > + if (dev_priv->vbt.has_mipi) { Do all VLV DSI desins use the PMIC for backlight, or is there something more specific in VBT we could look at? And what about CHT? Othwerwise things seem reasonable, so with the the intel_panel_setup_backlight() pipe thing fixed this patch is Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> I also gave the entire series a go on my FFRD8 and it appears to work just fine, so you can also add Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> to all the patches if you want. > + dev_priv->display.setup_backlight = pwm_setup_backlight; > + dev_priv->display.enable_backlight = pwm_enable_backlight; > + dev_priv->display.disable_backlight = pwm_disable_backlight; > + dev_priv->display.set_backlight = pwm_set_backlight; > + dev_priv->display.get_backlight = pwm_get_backlight; > + } else { > + dev_priv->display.setup_backlight = vlv_setup_backlight; > + dev_priv->display.enable_backlight = vlv_enable_backlight; > + dev_priv->display.disable_backlight = vlv_disable_backlight; > + dev_priv->display.set_backlight = vlv_set_backlight; > + dev_priv->display.get_backlight = vlv_get_backlight; > + } > } else if (IS_GEN4(dev)) { > dev_priv->display.setup_backlight = i965_setup_backlight; > dev_priv->display.enable_backlight = i965_enable_backlight; > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Jun 25, 2015 at 2:18 PM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Mon, Jun 22, 2015 at 04:24:25PM +0530, Shobhit Kumar wrote: >> Use the CRC PWM device in intel_panel.c and add new MIPI backlight >> specififc callbacks >> >> v2: Modify to use pwm_config callback >> v3: Addressed Jani's comments >> - Renamed all function as pwm_* instead of vlv_* >> - Call intel_panel_actually_set_backlight in enable function >> - Return -ENODEV in case pwm_get fails >> - in case pwm_config error return error cdoe from pwm_config >> - Cleanup pwm in intel_panel_destroy_backlight >> >> CC: Samuel Ortiz <sameo@linux.intel.com> >> Cc: Linus Walleij <linus.walleij@linaro.org> >> Cc: Alexandre Courbot <gnurou@gmail.com> >> Cc: Thierry Reding <thierry.reding@gmail.com> >> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> >> --- >> drivers/gpu/drm/i915/intel_drv.h | 4 ++ >> drivers/gpu/drm/i915/intel_dsi.c | 6 +++ >> drivers/gpu/drm/i915/intel_panel.c | 95 ++++++++++++++++++++++++++++++++++++-- >> 3 files changed, 100 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index 2afb31a..561c17f 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -182,6 +182,10 @@ struct intel_panel { >> bool enabled; >> bool combination_mode; /* gen 2/4 only */ >> bool active_low_pwm; >> + >> + /* PWM chip */ >> + struct pwm_device *pwm; >> + >> struct backlight_device *device; >> } backlight; >> >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c >> index c4db74a..be8722c 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> @@ -402,6 +402,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder) >> >> intel_dsi_port_enable(encoder); >> } >> + >> + intel_panel_enable_backlight(intel_dsi->attached_connector); >> } >> >> static void intel_dsi_pre_enable(struct intel_encoder *encoder) >> @@ -466,6 +468,8 @@ static void intel_dsi_pre_disable(struct intel_encoder *encoder) >> >> DRM_DEBUG_KMS("\n"); >> >> + intel_panel_disable_backlight(intel_dsi->attached_connector); >> + >> if (is_vid_mode(intel_dsi)) { >> /* Send Shutdown command to the panel in LP mode */ >> for_each_dsi_port(port, intel_dsi->ports) >> @@ -1132,6 +1136,8 @@ void intel_dsi_init(struct drm_device *dev) >> } >> >> intel_panel_init(&intel_connector->panel, fixed_mode, NULL); >> + intel_panel_setup_backlight(connector, >> + (intel_encoder->crtc_mask = (1 << PIPE_A)) ? PIPE_A : PIPE_B); > ^ > > Whoops. But since the PWM backlight doesn't need the initial pipe for > anything you can actually just pass INVALID_PIPE here. > You are right, its unused, but I thought passing right value still made sense. Otherwise it makes it look like I am setting up back-light for invalid pipe, when the real meaning is something like DONTCARE_PIPE >> >> return; >> >> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c >> index 7d83527..2aa30db 100644 >> --- a/drivers/gpu/drm/i915/intel_panel.c >> +++ b/drivers/gpu/drm/i915/intel_panel.c >> @@ -32,8 +32,12 @@ >> >> #include <linux/kernel.h> >> #include <linux/moduleparam.h> >> +#include <linux/pwm.h> >> #include "intel_drv.h" >> >> +#define CRC_PMIC_PWM_PERIOD_NS 21333 >> +#define CRC_PMIC_PWM_STEPS 255 > > This define appears to be unused. > Yeah, missed removing it. >> + >> void >> intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode, >> struct drm_display_mode *adjusted_mode) >> @@ -544,6 +548,15 @@ static u32 bxt_get_backlight(struct intel_connector *connector) >> return I915_READ(BXT_BLC_PWM_DUTY1); >> } >> >> +static u32 pwm_get_backlight(struct intel_connector *connector) >> +{ >> + struct intel_panel *panel = &connector->panel; >> + int duty_ns; >> + >> + duty_ns = pwm_get_duty_cycle(panel->backlight.pwm); >> + return DIV_ROUND_UP(duty_ns * 100, CRC_PMIC_PWM_PERIOD_NS); >> +} >> + >> static u32 intel_panel_get_backlight(struct intel_connector *connector) >> { >> struct drm_device *dev = connector->base.dev; >> @@ -632,6 +645,14 @@ static void bxt_set_backlight(struct intel_connector *connector, u32 level) >> I915_WRITE(BXT_BLC_PWM_DUTY1, level); >> } >> >> +static void pwm_set_backlight(struct intel_connector *connector, u32 level) >> +{ >> + struct intel_panel *panel = &connector->panel; >> + int duty_ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100); >> + >> + pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS); >> +} >> + >> static void >> intel_panel_actually_set_backlight(struct intel_connector *connector, u32 level) >> { >> @@ -769,6 +790,16 @@ static void bxt_disable_backlight(struct intel_connector *connector) >> I915_WRITE(BXT_BLC_PWM_CTL1, tmp & ~BXT_BLC_PWM_ENABLE); >> } >> >> +static void pwm_disable_backlight(struct intel_connector *connector) >> +{ >> + struct intel_panel *panel = &connector->panel; >> + >> + /* Disable the backlight */ >> + pwm_config(panel->backlight.pwm, 0, CRC_PMIC_PWM_PERIOD_NS); >> + usleep_range(2000, 3000); >> + pwm_disable(panel->backlight.pwm); >> +} >> + >> void intel_panel_disable_backlight(struct intel_connector *connector) >> { >> struct drm_device *dev = connector->base.dev; >> @@ -1002,6 +1033,14 @@ static void bxt_enable_backlight(struct intel_connector *connector) >> I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl | BXT_BLC_PWM_ENABLE); >> } >> >> +static void pwm_enable_backlight(struct intel_connector *connector) >> +{ >> + struct intel_panel *panel = &connector->panel; >> + >> + pwm_enable(panel->backlight.pwm); >> + intel_panel_actually_set_backlight(connector, panel->backlight.level); >> +} >> + >> void intel_panel_enable_backlight(struct intel_connector *connector) >> { >> struct drm_device *dev = connector->base.dev; >> @@ -1378,6 +1417,40 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused) >> return 0; >> } >> >> +static int pwm_setup_backlight(struct intel_connector *connector, >> + enum pipe pipe) >> +{ >> + struct drm_device *dev = connector->base.dev; >> + struct intel_panel *panel = &connector->panel; >> + int retval = -1; > > No need to initialize it. > >> + >> + /* Get the PWM chip for backlight control */ >> + panel->backlight.pwm = pwm_get(dev->dev, "pwm_backlight"); >> + if (IS_ERR(panel->backlight.pwm)) { >> + DRM_ERROR("Failed to own the pwm chip\n"); >> + panel->backlight.pwm = NULL; >> + return -ENODEV; >> + } >> + >> + retval = pwm_config(panel->backlight.pwm, CRC_PMIC_PWM_PERIOD_NS, >> + CRC_PMIC_PWM_PERIOD_NS); >> + if (retval < 0) { >> + DRM_ERROR("Failed to configure the pwm chip\n"); >> + pwm_put(panel->backlight.pwm); >> + panel->backlight.pwm = NULL; >> + return retval; >> + } >> + >> + panel->backlight.min = 0; /* 0% */ >> + panel->backlight.max = 100; /* 100% */ >> + panel->backlight.level = DIV_ROUND_UP( >> + pwm_get_duty_cycle(panel->backlight.pwm) * 100, >> + CRC_PMIC_PWM_PERIOD_NS); >> + panel->backlight.enabled = panel->backlight.level != 0; >> + >> + return 0; >> +} >> + >> int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe) >> { >> struct drm_device *dev = connector->dev; >> @@ -1421,6 +1494,10 @@ void intel_panel_destroy_backlight(struct drm_connector *connector) >> struct intel_connector *intel_connector = to_intel_connector(connector); >> struct intel_panel *panel = &intel_connector->panel; >> >> + /* dispose of the pwm */ >> + if (panel->backlight.pwm) >> + pwm_put(panel->backlight.pwm); >> + >> panel->backlight.present = false; >> } >> >> @@ -1448,11 +1525,19 @@ void intel_panel_init_backlight_funcs(struct drm_device *dev) >> dev_priv->display.set_backlight = pch_set_backlight; >> dev_priv->display.get_backlight = pch_get_backlight; >> } else if (IS_VALLEYVIEW(dev)) { >> - dev_priv->display.setup_backlight = vlv_setup_backlight; >> - dev_priv->display.enable_backlight = vlv_enable_backlight; >> - dev_priv->display.disable_backlight = vlv_disable_backlight; >> - dev_priv->display.set_backlight = vlv_set_backlight; >> - dev_priv->display.get_backlight = vlv_get_backlight; >> + if (dev_priv->vbt.has_mipi) { > > Do all VLV DSI desins use the PMIC for backlight, or is there > something more specific in VBT we could look at? > No, VLV designs can actually also use LPSS PWM as well as DISPLAY_PWM. Infact we had a case where even for eDP, customer design used LPSS. So this flag in VBT for mipi_config block "pwm_blc" indicate the same for now. But today this is only PMIC Vs LPSS. There is another update pending where we have full flexibility in VBT to define PMIC or LPSS or DISPLAY_PWM to handle the eDP case that I mentioned above. > And what about CHT? Its the same as I described above. But then beyond CHT, there is effort to unify this all as only one SoC PWM. > > Othwerwise things seem reasonable, so with the the > intel_panel_setup_backlight() pipe thing fixed this patch is > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > I also gave the entire series a go on my FFRD8 and it appears to work > just fine, so you can also add > Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > to all the patches if you want. Thanks a lot for testing it out. Regards Shobhit > >> + dev_priv->display.setup_backlight = pwm_setup_backlight; >> + dev_priv->display.enable_backlight = pwm_enable_backlight; >> + dev_priv->display.disable_backlight = pwm_disable_backlight; >> + dev_priv->display.set_backlight = pwm_set_backlight; >> + dev_priv->display.get_backlight = pwm_get_backlight; >> + } else { >> + dev_priv->display.setup_backlight = vlv_setup_backlight; >> + dev_priv->display.enable_backlight = vlv_enable_backlight; >> + dev_priv->display.disable_backlight = vlv_disable_backlight; >> + dev_priv->display.set_backlight = vlv_set_backlight; >> + dev_priv->display.get_backlight = vlv_get_backlight; >> + } >> } else if (IS_GEN4(dev)) { >> dev_priv->display.setup_backlight = i965_setup_backlight; >> dev_priv->display.enable_backlight = i965_enable_backlight; >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Jun 25, 2015 at 05:38:50PM +0530, Shobhit Kumar wrote: > On Thu, Jun 25, 2015 at 2:18 PM, Ville Syrjälä > <ville.syrjala@linux.intel.com> wrote: > > On Mon, Jun 22, 2015 at 04:24:25PM +0530, Shobhit Kumar wrote: > >> Use the CRC PWM device in intel_panel.c and add new MIPI backlight > >> specififc callbacks > >> > >> v2: Modify to use pwm_config callback > >> v3: Addressed Jani's comments > >> - Renamed all function as pwm_* instead of vlv_* > >> - Call intel_panel_actually_set_backlight in enable function > >> - Return -ENODEV in case pwm_get fails > >> - in case pwm_config error return error cdoe from pwm_config > >> - Cleanup pwm in intel_panel_destroy_backlight > >> > >> CC: Samuel Ortiz <sameo@linux.intel.com> > >> Cc: Linus Walleij <linus.walleij@linaro.org> > >> Cc: Alexandre Courbot <gnurou@gmail.com> > >> Cc: Thierry Reding <thierry.reding@gmail.com> > >> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> > >> --- > >> drivers/gpu/drm/i915/intel_drv.h | 4 ++ > >> drivers/gpu/drm/i915/intel_dsi.c | 6 +++ > >> drivers/gpu/drm/i915/intel_panel.c | 95 ++++++++++++++++++++++++++++++++++++-- > >> 3 files changed, 100 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > >> index 2afb31a..561c17f 100644 > >> --- a/drivers/gpu/drm/i915/intel_drv.h > >> +++ b/drivers/gpu/drm/i915/intel_drv.h > >> @@ -182,6 +182,10 @@ struct intel_panel { > >> bool enabled; > >> bool combination_mode; /* gen 2/4 only */ > >> bool active_low_pwm; > >> + > >> + /* PWM chip */ > >> + struct pwm_device *pwm; > >> + > >> struct backlight_device *device; > >> } backlight; > >> > >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > >> index c4db74a..be8722c 100644 > >> --- a/drivers/gpu/drm/i915/intel_dsi.c > >> +++ b/drivers/gpu/drm/i915/intel_dsi.c > >> @@ -402,6 +402,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder) > >> > >> intel_dsi_port_enable(encoder); > >> } > >> + > >> + intel_panel_enable_backlight(intel_dsi->attached_connector); > >> } > >> > >> static void intel_dsi_pre_enable(struct intel_encoder *encoder) > >> @@ -466,6 +468,8 @@ static void intel_dsi_pre_disable(struct intel_encoder *encoder) > >> > >> DRM_DEBUG_KMS("\n"); > >> > >> + intel_panel_disable_backlight(intel_dsi->attached_connector); > >> + > >> if (is_vid_mode(intel_dsi)) { > >> /* Send Shutdown command to the panel in LP mode */ > >> for_each_dsi_port(port, intel_dsi->ports) > >> @@ -1132,6 +1136,8 @@ void intel_dsi_init(struct drm_device *dev) > >> } > >> > >> intel_panel_init(&intel_connector->panel, fixed_mode, NULL); > >> + intel_panel_setup_backlight(connector, > >> + (intel_encoder->crtc_mask = (1 << PIPE_A)) ? PIPE_A : PIPE_B); > > ^ > > > > Whoops. But since the PWM backlight doesn't need the initial pipe for > > anything you can actually just pass INVALID_PIPE here. > > > > You are right, its unused, but I thought passing right value still > made sense. Otherwise it makes it look like I am setting up back-light > for invalid pipe, when the real meaning is something like > DONTCARE_PIPE Well it's not really about the pipe. It's about which set of BLC registers we're supoosed to use when using the BLC built into the display engine. And that's only done so that we take over the hardware state correctly. So INVALID_PIPE is just fine in this case since the backlight control has nothing to do with the pipe.
On Thu, Jun 25, 2015 at 6:17 PM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Thu, Jun 25, 2015 at 05:38:50PM +0530, Shobhit Kumar wrote: >> On Thu, Jun 25, 2015 at 2:18 PM, Ville Syrjälä >> <ville.syrjala@linux.intel.com> wrote: >> > On Mon, Jun 22, 2015 at 04:24:25PM +0530, Shobhit Kumar wrote: >> >> Use the CRC PWM device in intel_panel.c and add new MIPI backlight >> >> specififc callbacks >> >> >> >> v2: Modify to use pwm_config callback >> >> v3: Addressed Jani's comments >> >> - Renamed all function as pwm_* instead of vlv_* >> >> - Call intel_panel_actually_set_backlight in enable function >> >> - Return -ENODEV in case pwm_get fails >> >> - in case pwm_config error return error cdoe from pwm_config >> >> - Cleanup pwm in intel_panel_destroy_backlight >> >> >> >> CC: Samuel Ortiz <sameo@linux.intel.com> >> >> Cc: Linus Walleij <linus.walleij@linaro.org> >> >> Cc: Alexandre Courbot <gnurou@gmail.com> >> >> Cc: Thierry Reding <thierry.reding@gmail.com> >> >> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> >> >> --- >> >> drivers/gpu/drm/i915/intel_drv.h | 4 ++ >> >> drivers/gpu/drm/i915/intel_dsi.c | 6 +++ >> >> drivers/gpu/drm/i915/intel_panel.c | 95 ++++++++++++++++++++++++++++++++++++-- >> >> 3 files changed, 100 insertions(+), 5 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> >> index 2afb31a..561c17f 100644 >> >> --- a/drivers/gpu/drm/i915/intel_drv.h >> >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> >> @@ -182,6 +182,10 @@ struct intel_panel { >> >> bool enabled; >> >> bool combination_mode; /* gen 2/4 only */ >> >> bool active_low_pwm; >> >> + >> >> + /* PWM chip */ >> >> + struct pwm_device *pwm; >> >> + >> >> struct backlight_device *device; >> >> } backlight; >> >> >> >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c >> >> index c4db74a..be8722c 100644 >> >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> >> @@ -402,6 +402,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder) >> >> >> >> intel_dsi_port_enable(encoder); >> >> } >> >> + >> >> + intel_panel_enable_backlight(intel_dsi->attached_connector); >> >> } >> >> >> >> static void intel_dsi_pre_enable(struct intel_encoder *encoder) >> >> @@ -466,6 +468,8 @@ static void intel_dsi_pre_disable(struct intel_encoder *encoder) >> >> >> >> DRM_DEBUG_KMS("\n"); >> >> >> >> + intel_panel_disable_backlight(intel_dsi->attached_connector); >> >> + >> >> if (is_vid_mode(intel_dsi)) { >> >> /* Send Shutdown command to the panel in LP mode */ >> >> for_each_dsi_port(port, intel_dsi->ports) >> >> @@ -1132,6 +1136,8 @@ void intel_dsi_init(struct drm_device *dev) >> >> } >> >> >> >> intel_panel_init(&intel_connector->panel, fixed_mode, NULL); >> >> + intel_panel_setup_backlight(connector, >> >> + (intel_encoder->crtc_mask = (1 << PIPE_A)) ? PIPE_A : PIPE_B); >> > ^ >> > >> > Whoops. But since the PWM backlight doesn't need the initial pipe for >> > anything you can actually just pass INVALID_PIPE here. >> > >> >> You are right, its unused, but I thought passing right value still >> made sense. Otherwise it makes it look like I am setting up back-light >> for invalid pipe, when the real meaning is something like >> DONTCARE_PIPE > > Well it's not really about the pipe. It's about which set of BLC > registers we're supoosed to use when using the BLC built into the > display engine. And that's only done so that we take over the > hardware state correctly. So INVALID_PIPE is just fine in this case > since the backlight control has nothing to do with the pipe. > Ok, will update. Regards Shobhit
On Thu, 25 Jun 2015, Shobhit Kumar <kumar@shobhit.info> wrote: > On Thu, Jun 25, 2015 at 2:18 PM, Ville Syrjälä > <ville.syrjala@linux.intel.com> wrote: >> On Mon, Jun 22, 2015 at 04:24:25PM +0530, Shobhit Kumar wrote: >>> Use the CRC PWM device in intel_panel.c and add new MIPI backlight >>> specififc callbacks >>> >>> v2: Modify to use pwm_config callback >>> v3: Addressed Jani's comments >>> - Renamed all function as pwm_* instead of vlv_* >>> - Call intel_panel_actually_set_backlight in enable function >>> - Return -ENODEV in case pwm_get fails >>> - in case pwm_config error return error cdoe from pwm_config >>> - Cleanup pwm in intel_panel_destroy_backlight >>> >>> CC: Samuel Ortiz <sameo@linux.intel.com> >>> Cc: Linus Walleij <linus.walleij@linaro.org> >>> Cc: Alexandre Courbot <gnurou@gmail.com> >>> Cc: Thierry Reding <thierry.reding@gmail.com> >>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_drv.h | 4 ++ >>> drivers/gpu/drm/i915/intel_dsi.c | 6 +++ >>> drivers/gpu/drm/i915/intel_panel.c | 95 ++++++++++++++++++++++++++++++++++++-- >>> 3 files changed, 100 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >>> index 2afb31a..561c17f 100644 >>> --- a/drivers/gpu/drm/i915/intel_drv.h >>> +++ b/drivers/gpu/drm/i915/intel_drv.h >>> @@ -182,6 +182,10 @@ struct intel_panel { >>> bool enabled; >>> bool combination_mode; /* gen 2/4 only */ >>> bool active_low_pwm; >>> + >>> + /* PWM chip */ >>> + struct pwm_device *pwm; >>> + >>> struct backlight_device *device; >>> } backlight; >>> >>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c >>> index c4db74a..be8722c 100644 >>> --- a/drivers/gpu/drm/i915/intel_dsi.c >>> +++ b/drivers/gpu/drm/i915/intel_dsi.c >>> @@ -402,6 +402,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder) >>> >>> intel_dsi_port_enable(encoder); >>> } >>> + >>> + intel_panel_enable_backlight(intel_dsi->attached_connector); >>> } >>> >>> static void intel_dsi_pre_enable(struct intel_encoder *encoder) >>> @@ -466,6 +468,8 @@ static void intel_dsi_pre_disable(struct intel_encoder *encoder) >>> >>> DRM_DEBUG_KMS("\n"); >>> >>> + intel_panel_disable_backlight(intel_dsi->attached_connector); >>> + >>> if (is_vid_mode(intel_dsi)) { >>> /* Send Shutdown command to the panel in LP mode */ >>> for_each_dsi_port(port, intel_dsi->ports) >>> @@ -1132,6 +1136,8 @@ void intel_dsi_init(struct drm_device *dev) >>> } >>> >>> intel_panel_init(&intel_connector->panel, fixed_mode, NULL); >>> + intel_panel_setup_backlight(connector, >>> + (intel_encoder->crtc_mask = (1 << PIPE_A)) ? PIPE_A : PIPE_B); >> ^ >> >> Whoops. But since the PWM backlight doesn't need the initial pipe for >> anything you can actually just pass INVALID_PIPE here. >> > > You are right, its unused, but I thought passing right value still > made sense. Otherwise it makes it look like I am setting up back-light > for invalid pipe, when the real meaning is something like > DONTCARE_PIPE > >>> >>> return; >>> >>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c >>> index 7d83527..2aa30db 100644 >>> --- a/drivers/gpu/drm/i915/intel_panel.c >>> +++ b/drivers/gpu/drm/i915/intel_panel.c >>> @@ -32,8 +32,12 @@ >>> >>> #include <linux/kernel.h> >>> #include <linux/moduleparam.h> >>> +#include <linux/pwm.h> >>> #include "intel_drv.h" >>> >>> +#define CRC_PMIC_PWM_PERIOD_NS 21333 >>> +#define CRC_PMIC_PWM_STEPS 255 >> >> This define appears to be unused. >> > > Yeah, missed removing it. > >>> + >>> void >>> intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode, >>> struct drm_display_mode *adjusted_mode) >>> @@ -544,6 +548,15 @@ static u32 bxt_get_backlight(struct intel_connector *connector) >>> return I915_READ(BXT_BLC_PWM_DUTY1); >>> } >>> >>> +static u32 pwm_get_backlight(struct intel_connector *connector) >>> +{ >>> + struct intel_panel *panel = &connector->panel; >>> + int duty_ns; >>> + >>> + duty_ns = pwm_get_duty_cycle(panel->backlight.pwm); >>> + return DIV_ROUND_UP(duty_ns * 100, CRC_PMIC_PWM_PERIOD_NS); >>> +} >>> + >>> static u32 intel_panel_get_backlight(struct intel_connector *connector) >>> { >>> struct drm_device *dev = connector->base.dev; >>> @@ -632,6 +645,14 @@ static void bxt_set_backlight(struct intel_connector *connector, u32 level) >>> I915_WRITE(BXT_BLC_PWM_DUTY1, level); >>> } >>> >>> +static void pwm_set_backlight(struct intel_connector *connector, u32 level) >>> +{ >>> + struct intel_panel *panel = &connector->panel; >>> + int duty_ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100); >>> + >>> + pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS); >>> +} >>> + >>> static void >>> intel_panel_actually_set_backlight(struct intel_connector *connector, u32 level) >>> { >>> @@ -769,6 +790,16 @@ static void bxt_disable_backlight(struct intel_connector *connector) >>> I915_WRITE(BXT_BLC_PWM_CTL1, tmp & ~BXT_BLC_PWM_ENABLE); >>> } >>> >>> +static void pwm_disable_backlight(struct intel_connector *connector) >>> +{ >>> + struct intel_panel *panel = &connector->panel; >>> + >>> + /* Disable the backlight */ >>> + pwm_config(panel->backlight.pwm, 0, CRC_PMIC_PWM_PERIOD_NS); >>> + usleep_range(2000, 3000); >>> + pwm_disable(panel->backlight.pwm); >>> +} >>> + >>> void intel_panel_disable_backlight(struct intel_connector *connector) >>> { >>> struct drm_device *dev = connector->base.dev; >>> @@ -1002,6 +1033,14 @@ static void bxt_enable_backlight(struct intel_connector *connector) >>> I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl | BXT_BLC_PWM_ENABLE); >>> } >>> >>> +static void pwm_enable_backlight(struct intel_connector *connector) >>> +{ >>> + struct intel_panel *panel = &connector->panel; >>> + >>> + pwm_enable(panel->backlight.pwm); >>> + intel_panel_actually_set_backlight(connector, panel->backlight.level); >>> +} >>> + >>> void intel_panel_enable_backlight(struct intel_connector *connector) >>> { >>> struct drm_device *dev = connector->base.dev; >>> @@ -1378,6 +1417,40 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused) >>> return 0; >>> } >>> >>> +static int pwm_setup_backlight(struct intel_connector *connector, >>> + enum pipe pipe) >>> +{ >>> + struct drm_device *dev = connector->base.dev; >>> + struct intel_panel *panel = &connector->panel; >>> + int retval = -1; >> >> No need to initialize it. >> >>> + >>> + /* Get the PWM chip for backlight control */ >>> + panel->backlight.pwm = pwm_get(dev->dev, "pwm_backlight"); >>> + if (IS_ERR(panel->backlight.pwm)) { >>> + DRM_ERROR("Failed to own the pwm chip\n"); >>> + panel->backlight.pwm = NULL; >>> + return -ENODEV; >>> + } >>> + >>> + retval = pwm_config(panel->backlight.pwm, CRC_PMIC_PWM_PERIOD_NS, >>> + CRC_PMIC_PWM_PERIOD_NS); >>> + if (retval < 0) { >>> + DRM_ERROR("Failed to configure the pwm chip\n"); >>> + pwm_put(panel->backlight.pwm); >>> + panel->backlight.pwm = NULL; >>> + return retval; >>> + } >>> + >>> + panel->backlight.min = 0; /* 0% */ >>> + panel->backlight.max = 100; /* 100% */ >>> + panel->backlight.level = DIV_ROUND_UP( >>> + pwm_get_duty_cycle(panel->backlight.pwm) * 100, >>> + CRC_PMIC_PWM_PERIOD_NS); >>> + panel->backlight.enabled = panel->backlight.level != 0; >>> + >>> + return 0; >>> +} >>> + >>> int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe) >>> { >>> struct drm_device *dev = connector->dev; >>> @@ -1421,6 +1494,10 @@ void intel_panel_destroy_backlight(struct drm_connector *connector) >>> struct intel_connector *intel_connector = to_intel_connector(connector); >>> struct intel_panel *panel = &intel_connector->panel; >>> >>> + /* dispose of the pwm */ >>> + if (panel->backlight.pwm) >>> + pwm_put(panel->backlight.pwm); >>> + >>> panel->backlight.present = false; >>> } >>> >>> @@ -1448,11 +1525,19 @@ void intel_panel_init_backlight_funcs(struct drm_device *dev) >>> dev_priv->display.set_backlight = pch_set_backlight; >>> dev_priv->display.get_backlight = pch_get_backlight; >>> } else if (IS_VALLEYVIEW(dev)) { >>> - dev_priv->display.setup_backlight = vlv_setup_backlight; >>> - dev_priv->display.enable_backlight = vlv_enable_backlight; >>> - dev_priv->display.disable_backlight = vlv_disable_backlight; >>> - dev_priv->display.set_backlight = vlv_set_backlight; >>> - dev_priv->display.get_backlight = vlv_get_backlight; >>> + if (dev_priv->vbt.has_mipi) { >> >> Do all VLV DSI desins use the PMIC for backlight, or is there >> something more specific in VBT we could look at? >> > > No, VLV designs can actually also use LPSS PWM as well as DISPLAY_PWM. > Infact we had a case where even for eDP, customer design used LPSS. So > this flag in VBT for mipi_config block "pwm_blc" indicate the same for > now. But today this is only PMIC Vs LPSS. There is another update > pending where we have full flexibility in VBT to define PMIC or LPSS > or DISPLAY_PWM to handle the eDP case that I mentioned above. Side note, we may need to move the hooks from dev_priv->display to connector->panel in the future. BR, Jani. > >> And what about CHT? > > Its the same as I described above. But then beyond CHT, there is > effort to unify this all as only one SoC PWM. > >> >> Othwerwise things seem reasonable, so with the the >> intel_panel_setup_backlight() pipe thing fixed this patch is >> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >> I also gave the entire series a go on my FFRD8 and it appears to work >> just fine, so you can also add >> Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >> to all the patches if you want. > > Thanks a lot for testing it out. > > Regards > Shobhit > >> >>> + dev_priv->display.setup_backlight = pwm_setup_backlight; >>> + dev_priv->display.enable_backlight = pwm_enable_backlight; >>> + dev_priv->display.disable_backlight = pwm_disable_backlight; >>> + dev_priv->display.set_backlight = pwm_set_backlight; >>> + dev_priv->display.get_backlight = pwm_get_backlight; >>> + } else { >>> + dev_priv->display.setup_backlight = vlv_setup_backlight; >>> + dev_priv->display.enable_backlight = vlv_enable_backlight; >>> + dev_priv->display.disable_backlight = vlv_disable_backlight; >>> + dev_priv->display.set_backlight = vlv_set_backlight; >>> + dev_priv->display.get_backlight = vlv_get_backlight; >>> + } >>> } else if (IS_GEN4(dev)) { >>> dev_priv->display.setup_backlight = i965_setup_backlight; >>> dev_priv->display.enable_backlight = i965_enable_backlight; >>> -- >>> 1.9.1 >>> >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> -- >> Ville Syrjälä >> Intel OTC >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 2afb31a..561c17f 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -182,6 +182,10 @@ struct intel_panel { bool enabled; bool combination_mode; /* gen 2/4 only */ bool active_low_pwm; + + /* PWM chip */ + struct pwm_device *pwm; + struct backlight_device *device; } backlight; diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index c4db74a..be8722c 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -402,6 +402,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder) intel_dsi_port_enable(encoder); } + + intel_panel_enable_backlight(intel_dsi->attached_connector); } static void intel_dsi_pre_enable(struct intel_encoder *encoder) @@ -466,6 +468,8 @@ static void intel_dsi_pre_disable(struct intel_encoder *encoder) DRM_DEBUG_KMS("\n"); + intel_panel_disable_backlight(intel_dsi->attached_connector); + if (is_vid_mode(intel_dsi)) { /* Send Shutdown command to the panel in LP mode */ for_each_dsi_port(port, intel_dsi->ports) @@ -1132,6 +1136,8 @@ void intel_dsi_init(struct drm_device *dev) } intel_panel_init(&intel_connector->panel, fixed_mode, NULL); + intel_panel_setup_backlight(connector, + (intel_encoder->crtc_mask = (1 << PIPE_A)) ? PIPE_A : PIPE_B); return; diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 7d83527..2aa30db 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -32,8 +32,12 @@ #include <linux/kernel.h> #include <linux/moduleparam.h> +#include <linux/pwm.h> #include "intel_drv.h" +#define CRC_PMIC_PWM_PERIOD_NS 21333 +#define CRC_PMIC_PWM_STEPS 255 + void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode, struct drm_display_mode *adjusted_mode) @@ -544,6 +548,15 @@ static u32 bxt_get_backlight(struct intel_connector *connector) return I915_READ(BXT_BLC_PWM_DUTY1); } +static u32 pwm_get_backlight(struct intel_connector *connector) +{ + struct intel_panel *panel = &connector->panel; + int duty_ns; + + duty_ns = pwm_get_duty_cycle(panel->backlight.pwm); + return DIV_ROUND_UP(duty_ns * 100, CRC_PMIC_PWM_PERIOD_NS); +} + static u32 intel_panel_get_backlight(struct intel_connector *connector) { struct drm_device *dev = connector->base.dev; @@ -632,6 +645,14 @@ static void bxt_set_backlight(struct intel_connector *connector, u32 level) I915_WRITE(BXT_BLC_PWM_DUTY1, level); } +static void pwm_set_backlight(struct intel_connector *connector, u32 level) +{ + struct intel_panel *panel = &connector->panel; + int duty_ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100); + + pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS); +} + static void intel_panel_actually_set_backlight(struct intel_connector *connector, u32 level) { @@ -769,6 +790,16 @@ static void bxt_disable_backlight(struct intel_connector *connector) I915_WRITE(BXT_BLC_PWM_CTL1, tmp & ~BXT_BLC_PWM_ENABLE); } +static void pwm_disable_backlight(struct intel_connector *connector) +{ + struct intel_panel *panel = &connector->panel; + + /* Disable the backlight */ + pwm_config(panel->backlight.pwm, 0, CRC_PMIC_PWM_PERIOD_NS); + usleep_range(2000, 3000); + pwm_disable(panel->backlight.pwm); +} + void intel_panel_disable_backlight(struct intel_connector *connector) { struct drm_device *dev = connector->base.dev; @@ -1002,6 +1033,14 @@ static void bxt_enable_backlight(struct intel_connector *connector) I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl | BXT_BLC_PWM_ENABLE); } +static void pwm_enable_backlight(struct intel_connector *connector) +{ + struct intel_panel *panel = &connector->panel; + + pwm_enable(panel->backlight.pwm); + intel_panel_actually_set_backlight(connector, panel->backlight.level); +} + void intel_panel_enable_backlight(struct intel_connector *connector) { struct drm_device *dev = connector->base.dev; @@ -1378,6 +1417,40 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused) return 0; } +static int pwm_setup_backlight(struct intel_connector *connector, + enum pipe pipe) +{ + struct drm_device *dev = connector->base.dev; + struct intel_panel *panel = &connector->panel; + int retval = -1; + + /* Get the PWM chip for backlight control */ + panel->backlight.pwm = pwm_get(dev->dev, "pwm_backlight"); + if (IS_ERR(panel->backlight.pwm)) { + DRM_ERROR("Failed to own the pwm chip\n"); + panel->backlight.pwm = NULL; + return -ENODEV; + } + + retval = pwm_config(panel->backlight.pwm, CRC_PMIC_PWM_PERIOD_NS, + CRC_PMIC_PWM_PERIOD_NS); + if (retval < 0) { + DRM_ERROR("Failed to configure the pwm chip\n"); + pwm_put(panel->backlight.pwm); + panel->backlight.pwm = NULL; + return retval; + } + + panel->backlight.min = 0; /* 0% */ + panel->backlight.max = 100; /* 100% */ + panel->backlight.level = DIV_ROUND_UP( + pwm_get_duty_cycle(panel->backlight.pwm) * 100, + CRC_PMIC_PWM_PERIOD_NS); + panel->backlight.enabled = panel->backlight.level != 0; + + return 0; +} + int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe) { struct drm_device *dev = connector->dev; @@ -1421,6 +1494,10 @@ void intel_panel_destroy_backlight(struct drm_connector *connector) struct intel_connector *intel_connector = to_intel_connector(connector); struct intel_panel *panel = &intel_connector->panel; + /* dispose of the pwm */ + if (panel->backlight.pwm) + pwm_put(panel->backlight.pwm); + panel->backlight.present = false; } @@ -1448,11 +1525,19 @@ void intel_panel_init_backlight_funcs(struct drm_device *dev) dev_priv->display.set_backlight = pch_set_backlight; dev_priv->display.get_backlight = pch_get_backlight; } else if (IS_VALLEYVIEW(dev)) { - dev_priv->display.setup_backlight = vlv_setup_backlight; - dev_priv->display.enable_backlight = vlv_enable_backlight; - dev_priv->display.disable_backlight = vlv_disable_backlight; - dev_priv->display.set_backlight = vlv_set_backlight; - dev_priv->display.get_backlight = vlv_get_backlight; + if (dev_priv->vbt.has_mipi) { + dev_priv->display.setup_backlight = pwm_setup_backlight; + dev_priv->display.enable_backlight = pwm_enable_backlight; + dev_priv->display.disable_backlight = pwm_disable_backlight; + dev_priv->display.set_backlight = pwm_set_backlight; + dev_priv->display.get_backlight = pwm_get_backlight; + } else { + dev_priv->display.setup_backlight = vlv_setup_backlight; + dev_priv->display.enable_backlight = vlv_enable_backlight; + dev_priv->display.disable_backlight = vlv_disable_backlight; + dev_priv->display.set_backlight = vlv_set_backlight; + dev_priv->display.get_backlight = vlv_get_backlight; + } } else if (IS_GEN4(dev)) { dev_priv->display.setup_backlight = i965_setup_backlight; dev_priv->display.enable_backlight = i965_enable_backlight;
Use the CRC PWM device in intel_panel.c and add new MIPI backlight specififc callbacks v2: Modify to use pwm_config callback v3: Addressed Jani's comments - Renamed all function as pwm_* instead of vlv_* - Call intel_panel_actually_set_backlight in enable function - Return -ENODEV in case pwm_get fails - in case pwm_config error return error cdoe from pwm_config - Cleanup pwm in intel_panel_destroy_backlight CC: Samuel Ortiz <sameo@linux.intel.com> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Alexandre Courbot <gnurou@gmail.com> Cc: Thierry Reding <thierry.reding@gmail.com> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> --- drivers/gpu/drm/i915/intel_drv.h | 4 ++ drivers/gpu/drm/i915/intel_dsi.c | 6 +++ drivers/gpu/drm/i915/intel_panel.c | 95 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 100 insertions(+), 5 deletions(-)