[Intel-gfx] [PATCH v5 1/9] drm/i915: Fix cap check for intel_dp_aux_backlight driver

[Pandiyan, Dhinakaran]

On Mon, 2017-05-08 at 11:06 -0700, Puthikorn Voravootivat wrote:
> I added the option to choose to prioritized AUX or PWM before in last
> version of this patch set.
> But comment from Jani said that it is better to separate the patch.
> The option to prioritized the PWM in now in patch #4

You are still modifying priorities in this patch by removing the check
for !(_PIN_ENABLE_CAP || _BRIGHTNESS_PWM_PIN_CAP)

I can see why it made sense to split the previous version, but shouldn't
this patch be just about fixing the CAPS check for the existing design.

i.e.,
+(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) {


You can then make the PWM PIN/AUX preference changes in patch 4.


Jani,
please correct me if I'm wrong here.

-DK

> 
> 
> On Mon, May 8, 2017 at 10:55 AM, Pandiyan, Dhinakaran
> <dhinakaran.pandiyan at intel.com> wrote:
>         On Wed, 2017-05-03 at 17:28 -0700, Puthikorn Voravootivat
>         wrote:
>         > intel_dp_aux_backlight driver should check for the
>         > DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP before enable the
>         driver.
>         >
>         > Signed-off-by: Puthikorn Voravootivat <puthik at chromium.org>
>         > ---
>         >  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 ++---
>         >  1 file changed, 2 insertions(+), 3 deletions(-)
>         >
>         > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>         b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>         > index 6532e226db29..24a905d1a74b 100644
>         > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>         > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>         > @@ -142,10 +142,9 @@
>         intel_dp_aux_display_control_capable(struct intel_connector
>         *connector)
>         >       /* Check the  eDP Display control capabilities
>         registers to determine if
>         >        * the panel can support backlight control over the
>         aux channel
>         >        */
>         > -     if (intel_dp->edp_dpcd[1] &
>         DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP &&
>         > +     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[1] &
>         DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) ||
>         > -           (intel_dp->edp_dpcd[2] &
>         DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) {
>         
>         ^Were these two changes intended? The patch claims an
>         additional check
>         for DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP is being added but
>         also
>         makes this unrelated change. Aren't you changing how the
>         driver
>         prioritizes AUX v/s PWM brightness control by removing these
>         lines?
>         
>         -DK
>         
>         
>         > +         (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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx