| Message ID | 20170504002836.120988-2-puthik@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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; > }
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 On Mon, May 8, 2017 at 10:55 AM, Pandiyan, Dhinakaran < dhinakaran.pandiyan@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@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; > > } > >
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@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@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@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 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))) { + (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) { DRM_DEBUG_KMS("AUX Backlight Control Supported!\n"); return true; }
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@chromium.org> --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)