[Intel-gfx] [v2 7/7] drm/i915: Backlight control using CRC PMIC based PWM driver

[Shobhit Kumar]

On Thu, Jun 25, 2015 at 6:17 PM, Ville Syrjälä
<ville.syrjala at 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 at 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 at linux.intel.com>
>> >> Cc: Linus Walleij <linus.walleij at linaro.org>
>> >> Cc: Alexandre Courbot <gnurou at gmail.com>
>> >> Cc: Thierry Reding <thierry.reding at gmail.com>
>> >> Signed-off-by: Shobhit Kumar <shobhit.kumar at 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