Re: [PATCH resend 11/15] drm/i915/dsi: Group MIPI_SEQ_BACKLIGHT_ON/OFF with panel_[en|dis]able_backlight

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 27-02-17 18:06, Bob Paauwe 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.

Ok, done for v2 of the patch-set I will go through your other comments
tomorrow and then send a v2.

Regards,

Hans


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.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 */






_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux