Op 29-06-16 om 17:36 schreef Jani Nikula:
> Previously we've just shoved the first eight devices in DIDL to CADL
> (list of active outputs). Some of the active outputs may have been left
> outside of CADL. The problem is, some BIOS implementations prevent
> laptop brightness hotkey propagation if the flat panel is not active.
>
> Now that we have connector to acpi device id mapping covered, we can
> update CADL based on which outputs are actually active.
>
> v3: actually git add the dev->dev_priv change.
>
> v4: update cadl in intel_shared_dpll_commit() if intel_state->modeset
> (Maarten)
>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Reviewed-and-tested-by: Peter Wu <peter@xxxxxxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/intel_display.c | 4 ++
> drivers/gpu/drm/i915/intel_opregion.c | 70 ++++++++++++++++++-----------------
> 3 files changed, 43 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 724d34b00196..64ab52529be8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3692,6 +3692,7 @@ extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
> extern int intel_opregion_notify_adapter(struct drm_i915_private *dev_priv,
> pci_power_t state);
> extern int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv);
> +extern void intel_opregion_update_cadl(struct drm_i915_private *dev_priv);
> #else
> static inline int intel_opregion_setup(struct drm_i915_private *dev) { return 0; }
> static inline void intel_opregion_register(struct drm_i915_private *dev_priv) { }
> @@ -3713,6 +3714,7 @@ static inline int intel_opregion_get_panel_type(struct drm_i915_private *dev)
> {
> return -ENODEV;
> }
> +static inline void intel_opregion_update_cadl(struct drm_i915_private *dev_priv) { }
> #endif
>
> /* intel_acpi.c */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d902a70edb84..4f404900f610 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13953,6 +13953,10 @@ static int intel_atomic_commit(struct drm_device *dev,
> dev_priv->wm.distrust_bios_wm = false;
> dev_priv->wm.skl_results = intel_state->wm_results;
> intel_shared_dpll_commit(state);
> +
> + if (intel_state->modeset)
> + intel_opregion_update_cadl(dev_priv);
> +
> intel_atomic_track_fbs(state);
>
> if (nonblock)
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index 632f0178c2b0..8b3f7e6ae4bb 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -642,24 +642,6 @@ static struct notifier_block intel_opregion_notifier = {
> * (version 3)
> */
>
> -static u32 get_did(struct intel_opregion *opregion, int i)
> -{
> - u32 did;
> -
> - if (i < ARRAY_SIZE(opregion->acpi->didl)) {
> - did = opregion->acpi->didl[i];
> - } else {
> - i -= ARRAY_SIZE(opregion->acpi->didl);
> -
> - if (WARN_ON(i >= ARRAY_SIZE(opregion->acpi->did2)))
> - return 0;
> -
> - did = opregion->acpi->did2[i];
> - }
> -
> - return did;
> -}
> -
> static void set_did(struct intel_opregion *opregion, int i, u32 val)
> {
> if (i < ARRAY_SIZE(opregion->acpi->didl)) {
> @@ -674,6 +656,14 @@ static void set_did(struct intel_opregion *opregion, int i, u32 val)
> }
> }
>
> +static void set_cad(struct intel_opregion *opregion, int i, u32 val)
> +{
> + if (WARN_ON(i >= ARRAY_SIZE(opregion->acpi->cadl)))
> + return;
> +
> + opregion->acpi->cadl[i] = val;
> +}
> +
> static u32 acpi_display_type(struct intel_connector *connector)
> {
> u32 display_type;
> @@ -759,22 +749,36 @@ static void intel_didl_outputs(struct drm_i915_private *dev_priv)
> set_did(opregion, i, 0);
> }
>
> -static void intel_setup_cadls(struct drm_i915_private *dev_priv)
> +/* Update CADL to reflect active outputs. */
> +void intel_opregion_update_cadl(struct drm_i915_private *dev_priv)
> {
> struct intel_opregion *opregion = &dev_priv->opregion;
> - int i = 0;
> - u32 disp_id;
> -
> - /* Initialize the CADL field by duplicating the DIDL values.
> - * Technically, this is not always correct as display outputs may exist,
> - * but not active. This initialization is necessary for some Clevo
> - * laptops that check this field before processing the brightness and
> - * display switching hotkeys. Just like DIDL, CADL is NULL-terminated if
> - * there are less than eight devices. */
> - do {
> - disp_id = get_did(opregion, i);
> - opregion->acpi->cadl[i] = disp_id;
> - } while (++i < 8 && disp_id != 0);
> + struct intel_crtc *crtc;
> + int i = 0, max_active = ARRAY_SIZE(opregion->acpi->cadl);
> +
> + for_each_intel_crtc(dev_priv->dev, crtc) {
> + struct intel_encoder *encoder;
> +
> + if (!crtc->active)
> + continue;
Argh, don't use crtc->active!
We have intel_state->active_crtcs + for_each_intel_crtc_mask for this, it should fix the issue Koenig mentioned.
If fixed,
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
