| Message ID | 20170504002836.120988-4-puthik@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2017-05-03 at 17:28 -0700, Puthikorn Voravootivat wrote: > There are some panel that > (1) does not support display backlight enable via AUX > (2) support display backlight adjustment via AUX > (3) support display backlight enable via eDP BL_ENABLE pin > > The current driver required that (1) must be support to enable (2). > This patch drops that requirement. > > Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> > --- > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > index ad8560c5f689..5b83c9737644 100644 > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable) > { > uint8_t reg_val = 0; > > + /* Early return when display use other mechanism to enable backlight. */ > + if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) > + return; > + How is backlight enabled in this case? -DK > if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER, > ®_val) < 0) { > DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", > @@ -164,7 +168,6 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector) > * the panel can support backlight control over the aux channel > */ > if ((intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP) && > - (intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && > (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) { > DRM_DEBUG_KMS("AUX Backlight Control Supported!\n"); > return true;
> How is backlight enabled in this case? Using eDP BL_ENABLE pin On Sat, May 6, 2017 at 1:59 AM, Pandiyan, Dhinakaran < dhinakaran.pandiyan@intel.com> wrote: > On Wed, 2017-05-03 at 17:28 -0700, Puthikorn Voravootivat wrote: > > There are some panel that > > (1) does not support display backlight enable via AUX > > (2) support display backlight adjustment via AUX > > (3) support display backlight enable via eDP BL_ENABLE pin > > > > The current driver required that (1) must be support to enable (2). > > This patch drops that requirement. > > > > Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> > > --- > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > index ad8560c5f689..5b83c9737644 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp > *intel_dp, bool enable) > > { > > uint8_t reg_val = 0; > > > > + /* Early return when display use other mechanism to enable > backlight. */ > > + if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) > > + return; > > + > > How is backlight enabled in this case? > > -DK > > > if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_ > REGISTER, > > ®_val) < 0) { > > DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", > > @@ -164,7 +168,6 @@ intel_dp_aux_display_control_capable(struct > intel_connector *connector) > > * the panel can support backlight control over the aux channel > > */ > > if ((intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP) > && > > - (intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && > > (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) > { > > DRM_DEBUG_KMS("AUX Backlight Control Supported!\n"); > > return true; > >
On Tue, 2017-05-09 at 16:39 -0700, Puthikorn Voravootivat wrote: > > How is backlight enabled in this case? > Using eDP BL_ENABLE pin Sure, but I am not seeing how this falls back to one of the [bxt,lpt]_enable_backlight() functions in intel_panel.c. Apologies if I am missing something very obvious. If intel_dp_aux_init_backlight_funcs() returned -ENODEV, then one of the platform specific PWM enable callbacks would be called. But in this case, dp_aux_enable_backlight() just returns without doing anything. -DK > > > On Sat, May 6, 2017 at 1:59 AM, Pandiyan, Dhinakaran > <dhinakaran.pandiyan@intel.com> wrote: > On Wed, 2017-05-03 at 17:28 -0700, Puthikorn Voravootivat > wrote: > > There are some panel that > > (1) does not support display backlight enable via AUX > > (2) support display backlight adjustment via AUX > > (3) support display backlight enable via eDP BL_ENABLE pin > > > > The current driver required that (1) must be support to > enable (2). > > This patch drops that requirement. > > > > Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> > > --- > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > index ad8560c5f689..5b83c9737644 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > @@ -28,6 +28,10 @@ static void > set_aux_backlight_enable(struct intel_dp *intel_dp, bool > enable) > > { > > uint8_t reg_val = 0; > > > > + /* Early return when display use other mechanism to > enable backlight. */ > > + if (!(intel_dp->edp_dpcd[1] & > DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) > > + return; > > + > > How is backlight enabled in this case? > > -DK > > > if (drm_dp_dpcd_readb(&intel_dp->aux, > DP_EDP_DISPLAY_CONTROL_REGISTER, > > ®_val) < 0) { > > DRM_DEBUG_KMS("Failed to read DPCD register 0x > %x\n", > > @@ -164,7 +168,6 @@ > intel_dp_aux_display_control_capable(struct intel_connector > *connector) > > * the panel can support backlight control over the > aux channel > > */ > > if ((intel_dp->edp_dpcd[1] & > DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP) && > > - (intel_dp->edp_dpcd[1] & > DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && > > (intel_dp->edp_dpcd[2] & > DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) { > > DRM_DEBUG_KMS("AUX Backlight Control > Supported!\n"); > > return true; > > > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 10 May 2017, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote: > On Tue, 2017-05-09 at 16:39 -0700, Puthikorn Voravootivat wrote: >> > How is backlight enabled in this case? >> Using eDP BL_ENABLE pin > > > Sure, but I am not seeing how this falls back to one of the > [bxt,lpt]_enable_backlight() functions in intel_panel.c. Apologies if I > am missing something very obvious. > > If intel_dp_aux_init_backlight_funcs() returned -ENODEV, then one of the > platform specific PWM enable callbacks would be called. But in this > case, dp_aux_enable_backlight() just returns without doing anything. The eDP BL_ENABLE pin (in the physical eDP connector) is controlled by the panel power sequencer, independent from the PWM control. See intel_edp_backlight_* functions in intel_dp.c. BR, Jani. > > > -DK >> >> >> On Sat, May 6, 2017 at 1:59 AM, Pandiyan, Dhinakaran >> <dhinakaran.pandiyan@intel.com> wrote: >> On Wed, 2017-05-03 at 17:28 -0700, Puthikorn Voravootivat >> wrote: >> > There are some panel that >> > (1) does not support display backlight enable via AUX >> > (2) support display backlight adjustment via AUX >> > (3) support display backlight enable via eDP BL_ENABLE pin >> > >> > The current driver required that (1) must be support to >> enable (2). >> > This patch drops that requirement. >> > >> > Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> >> > --- >> > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 ++++- >> > 1 file changed, 4 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c >> b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c >> > index ad8560c5f689..5b83c9737644 100644 >> > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c >> > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c >> > @@ -28,6 +28,10 @@ static void >> set_aux_backlight_enable(struct intel_dp *intel_dp, bool >> enable) >> > { >> > uint8_t reg_val = 0; >> > >> > + /* Early return when display use other mechanism to >> enable backlight. */ >> > + if (!(intel_dp->edp_dpcd[1] & >> DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) >> > + return; >> > + >> >> How is backlight enabled in this case? >> >> -DK >> >> > if (drm_dp_dpcd_readb(&intel_dp->aux, >> DP_EDP_DISPLAY_CONTROL_REGISTER, >> > ®_val) < 0) { >> > DRM_DEBUG_KMS("Failed to read DPCD register 0x >> %x\n", >> > @@ -164,7 +168,6 @@ >> intel_dp_aux_display_control_capable(struct intel_connector >> *connector) >> > * the panel can support backlight control over the >> aux channel >> > */ >> > if ((intel_dp->edp_dpcd[1] & >> DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP) && >> > - (intel_dp->edp_dpcd[1] & >> DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && >> > (intel_dp->edp_dpcd[2] & >> DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) { >> > DRM_DEBUG_KMS("AUX Backlight Control >> Supported!\n"); >> > return true; >> >> >> >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Wed, 2017-05-10 at 13:05 +0300, Jani Nikula wrote: > On Wed, 10 May 2017, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote: > > On Tue, 2017-05-09 at 16:39 -0700, Puthikorn Voravootivat wrote: > >> > How is backlight enabled in this case? > >> Using eDP BL_ENABLE pin > > > > > > Sure, but I am not seeing how this falls back to one of the > > [bxt,lpt]_enable_backlight() functions in intel_panel.c. Apologies if I > > am missing something very obvious. > > > > If intel_dp_aux_init_backlight_funcs() returned -ENODEV, then one of the > > platform specific PWM enable callbacks would be called. But in this > > case, dp_aux_enable_backlight() just returns without doing anything. > > The eDP BL_ENABLE pin (in the physical eDP connector) is controlled by > the panel power sequencer, independent from the PWM control. See > intel_edp_backlight_* functions in intel_dp.c. > > BR, > Jani. > > Ah! Thanks, that makes sense. The naming is a bit confusing though. The _enable_backlight() functions in intel_panel.c don't enable backlight but enable PWM. > > > > > > -DK > >> > >> > >> On Sat, May 6, 2017 at 1:59 AM, Pandiyan, Dhinakaran > >> <dhinakaran.pandiyan@intel.com> wrote: > >> On Wed, 2017-05-03 at 17:28 -0700, Puthikorn Voravootivat > >> wrote: > >> > There are some panel that > >> > (1) does not support display backlight enable via AUX > >> > (2) support display backlight adjustment via AUX > >> > (3) support display backlight enable via eDP BL_ENABLE pin > >> > > >> > The current driver required that (1) must be support to > >> enable (2). > >> > This patch drops that requirement. > >> > > >> > Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> > >> > --- > >> > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 ++++- > >> > 1 file changed, 4 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > >> b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > >> > index ad8560c5f689..5b83c9737644 100644 > >> > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > >> > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > >> > @@ -28,6 +28,10 @@ static void > >> set_aux_backlight_enable(struct intel_dp *intel_dp, bool > >> enable) > >> > { > >> > uint8_t reg_val = 0; > >> > > >> > + /* Early return when display use other mechanism to > >> enable backlight. */ > >> > + if (!(intel_dp->edp_dpcd[1] & > >> DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) > >> > + return; > >> > + > >> > >> How is backlight enabled in this case? > >> > >> -DK > >> > >> > if (drm_dp_dpcd_readb(&intel_dp->aux, > >> DP_EDP_DISPLAY_CONTROL_REGISTER, > >> > ®_val) < 0) { > >> > DRM_DEBUG_KMS("Failed to read DPCD register 0x > >> %x\n", > >> > @@ -164,7 +168,6 @@ > >> intel_dp_aux_display_control_capable(struct intel_connector > >> *connector) > >> > * the panel can support backlight control over the > >> aux channel > >> > */ > >> > if ((intel_dp->edp_dpcd[1] & > >> DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP) && > >> > - (intel_dp->edp_dpcd[1] & > >> DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && > >> > (intel_dp->edp_dpcd[2] & > >> DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) { > >> > DRM_DEBUG_KMS("AUX Backlight Control > >> Supported!\n"); > >> > return true; > >> > >> > >> > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > >
diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index ad8560c5f689..5b83c9737644 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable) { uint8_t reg_val = 0; + /* Early return when display use other mechanism to enable backlight. */ + if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) + return; + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER, ®_val) < 0) { DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", @@ -164,7 +168,6 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector) * the panel can support backlight control over the aux channel */ if ((intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP) && - (intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) { DRM_DEBUG_KMS("AUX Backlight Control Supported!\n"); return true;
There are some panel that (1) does not support display backlight enable via AUX (2) support display backlight adjustment via AUX (3) support display backlight enable via eDP BL_ENABLE pin The current driver required that (1) must be support to enable (2). This patch drops that requirement. Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)