On Mon, 27 Feb 2017, Bob Paauwe <bob.j.paauwe@xxxxxxxxx> wrote:
> On Sat, 25 Feb 2017 11:37:50 +0100
> Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
>> Hi,
>>
>> On 24-02-17 18:00, Bob Paauwe wrote:
>> > On Mon, 20 Feb 2017 15:08:41 +0100
>> > Hans de Goede <hdegoede@xxxxxxxxxx> 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 <hdegoede@xxxxxxxxxx>
>> >
>> > 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.
I think they're mostly alternatives, and only one or the other gets
used, but since the VBT sequence is rather generic, I think it's
possible one controls the SoC side and the other the panel side.
> 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.
Please leave it as it is.
BR,
Jani.
>
>>
>>
>> >
>> > Reviewed-by: Bob Paauwe <bob.j.paauwe@xxxxxxxxx>
>> >
>> >> ---
>> >> 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 */
>> >
>> >
>> >
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
