On Sat, 25 Feb 2017 11:37:50 +0100
Hans de Goede <hdego...@redhat.com> wrote:
> Hi,
>
> On 24-02-17 18:00, Bob Paauwe wrote:
> > On Mon, 20 Feb 2017 15:08:41 +0100
> > Hans de Goede <hdego...@redhat.com> wrote:
> >
> >> Execute the MIPI_SEQ_BACKLIGHT_ON/OFF VBT sequences at the same time as
> >> we call intel_panel_enable_backlight() / intel_panel_disable_backlight().
> >>
> >> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> >
> > I'm not sure that the added comments are necessary. Maybe if there was
> > some explanation for why we're using two different mechanisms to
> > enable/disable backlight instead.
>
> That assumes I know why there are 2 mechanisms of I have to guess it
> is because on some boards only one of the mechanisms works, but that
> is just a guess. Just calling the VBT sequence should be enough on
> (most?) boards.
>
> Regards,
>
> Hans
I don't think I can describe why we have two mechanisms so just
dropping the comment would be my preference at this point.
It may be that calling the intel_panel_enable_backlight() will make
sure the sysfs entries are correct while the VBT sequence simply
modifies the hardware.
Possibly a good solution would be to enhance the code that executes the
VBT sequence to also call the intel_panel_enable_backlight(), but
that's not something to change here.
>
>
> >
> > Reviewed-by: Bob Paauwe <bob.j.paa...@intel.com>
> >
> >> ---
> >> drivers/gpu/drm/i915/intel_dsi.c | 6 ++++--
> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> >> b/drivers/gpu/drm/i915/intel_dsi.c
> >> index 8408f59..a8d0948 100644
> >> --- a/drivers/gpu/drm/i915/intel_dsi.c
> >> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> >> @@ -687,12 +687,13 @@ static void intel_dsi_pre_enable(struct
> >> intel_encoder *encoder,
> >> msleep(100);
> >>
> >> intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
> >> - intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
> >>
> >> intel_dsi_port_enable(encoder);
> >> }
> >>
> >> + /* Enable backlight, both pwm and VBT */
> >> intel_panel_enable_backlight() (intel_dsi->attached_connector);
> >> + intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
> >> }
> >>
> >> static void intel_dsi_enable_nop(struct intel_encoder *encoder,
> >> @@ -718,6 +719,8 @@ static void intel_dsi_pre_disable(struct intel_encoder
> >> *encoder,
> >>
> >> DRM_DEBUG_KMS("\n");
> >>
> >> + /* Disable backlight, both VBT and pwm */
> >> + intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_OFF);
> >> intel_panel_disable_backlight(intel_dsi->attached_connector);
> >>
> >> /*
> >> @@ -762,7 +765,6 @@ static void intel_dsi_post_disable(struct
> >> intel_encoder *encoder,
> >> * if disable packets are sent before sending shutdown packet then in
> >> * some next enable sequence send turn on packet error is observed
> >> */
> >> - intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_OFF);
> >> intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_OFF);
> >>
> >> /* Transition to LP-00 */
> >
> >
> >
--
--
Bob Paauwe
bob.j.paa...@intel.com
IOTG / PED Software Organization
Intel Corp. Folsom, CA
(916) 356-6193
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
