All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matt Roper <matthew.d.roper@intel.com>
To: "José Roberto de Souza" <jose.souza@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 1/3] drm/i915/vbt: Fix backlight parsing for VBT 234+
Date: Wed, 7 Oct 2020 15:45:39 -0700	[thread overview]
Message-ID: <20201007224539.GL2863995@mdroper-desk1.amr.corp.intel.com> (raw)
In-Reply-To: <20200929223419.146925-1-jose.souza@intel.com>

On Tue, Sep 29, 2020 at 03:34:17PM -0700, José Roberto de Souza wrote:
> Child min_brightness is obsolete from VBT 234+, instead the new
> min_brightness field in the main structure should be used.
> 
> This new field is 16 bits wide, so backlight_precision_bits is needed
> to check if value needs to be scaled down but it is only available in
> VBT 236+ so working around it by using the also new backlight_level
> in the main struct.
> 
> v2:
> - missed that backlight_data->level is also obsolete
> 
> BSpec: 20149
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c     | 30 +++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_vbt_defs.h | 12 ++++++--
>  2 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 4716484af62d..58e5657a77bb 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -425,6 +425,7 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv,
>  	const struct bdb_lfp_backlight_data *backlight_data;
>  	const struct lfp_backlight_data_entry *entry;
>  	int panel_type = dev_priv->vbt.panel_type;
> +	u16 level;
>  
>  	backlight_data = find_section(bdb, BDB_LVDS_BACKLIGHT);
>  	if (!backlight_data)
> @@ -459,14 +460,39 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv,
>  
>  	dev_priv->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz;
>  	dev_priv->vbt.backlight.active_low_pwm = entry->active_low_pwm;
> -	dev_priv->vbt.backlight.min_brightness = entry->min_brightness;
> +
> +	if (bdb->version >= 234) {
> +		bool scale = false;
> +		u16 min_level;
> +
> +		level = backlight_data->backlight_level[panel_type].level;
> +		min_level = backlight_data->backlight_min_level[panel_type].level;
> +
> +		if (bdb->version >= 236)
> +			scale = backlight_data->backlight_precision_bits[panel_type] == 16;
> +		else
> +			scale = level > 255;

I'm not sure I follow the 'else' arm here.  On version 234/235 we'd have
16-bit level values.  In the absence of any other precision information
wouldn't we assume that all the bits are used and that we have a full
16-bit precision?  If the level is < 256 (or for that matter if we have
any value where level & 0xFF is non-zero) wouldn't that definitely mean
that there are 16-bits of precision since otherwise those low bits would
have to be 0's?

> +
> +		if (scale)
> +			min_level = min_level / 255;
> +
> +		if (min_level > 255) {
> +			drm_warn(&dev_priv->drm, "Backlight min level > 255\n");
> +			level = 255;
> +		}
> +		dev_priv->vbt.backlight.min_brightness = min_level;
> +	} else {
> +		level = backlight_data->level[panel_type];
> +		dev_priv->vbt.backlight.min_brightness = entry->min_brightness;
> +	}
> +
>  	drm_dbg_kms(&dev_priv->drm,
>  		    "VBT backlight PWM modulation frequency %u Hz, "
>  		    "active %s, min brightness %u, level %u, controller %u\n",
>  		    dev_priv->vbt.backlight.pwm_freq_hz,
>  		    dev_priv->vbt.backlight.active_low_pwm ? "low" : "high",
>  		    dev_priv->vbt.backlight.min_brightness,
> -		    backlight_data->level[panel_type],
> +		    level,
>  		    dev_priv->vbt.backlight.controller);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index 54bcc6a6947c..b4742c4fde97 100644
> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> @@ -782,7 +782,7 @@ struct lfp_backlight_data_entry {
>  	u8 active_low_pwm:1;
>  	u8 obsolete1:5;
>  	u16 pwm_freq_hz;
> -	u8 min_brightness;
> +	u8 min_brightness; /* Obsolete from 234+ */
>  	u8 obsolete2;
>  	u8 obsolete3;
>  } __packed;
> @@ -792,11 +792,19 @@ struct lfp_backlight_control_method {
>  	u8 controller:4;
>  } __packed;
>  
> +struct lfp_backlight_level {
> +	u32 level : 16;
> +	u32 reserved : 16;
> +} __packed;
> +
>  struct bdb_lfp_backlight_data {
>  	u8 entry_size;
>  	struct lfp_backlight_data_entry data[16];
> -	u8 level[16];
> +	u8 level[16]; /* Obsolete from 234+ */
>  	struct lfp_backlight_control_method backlight_control[16];
> +	struct lfp_backlight_level backlight_level[16];		/* 234+ */
> +	struct lfp_backlight_level backlight_min_level[16];	/* 234+ */

Technically these two are described as "brightness level" rather than
"backlight level" in the spec.  Matching the spec's terminology might
make this slightly easier to follow when people look at it in the
future, but up to you.


Matt

> +	u8 backlight_precision_bits[16];					/* 236+ */
>  } __packed;
>  
>  /*
> -- 
> 2.28.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2020-10-07 22:45 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-29 22:34 [Intel-gfx] [PATCH v2 1/3] drm/i915/vbt: Fix backlight parsing for VBT 234+ José Roberto de Souza
2020-09-29 22:34 ` [Intel-gfx] [PATCH v2 2/3] drm/i915/vbt: Update the version and expected size of BDB_GENERAL_DEFINITIONS map José Roberto de Souza
2020-10-07 22:49   ` Matt Roper
2020-09-29 22:34 ` [Intel-gfx] [PATCH v2 3/3] drm/i915/vbt: Add VRR VBT toggle José Roberto de Souza
2020-10-07 22:53   ` Matt Roper
2020-09-29 23:09 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915/vbt: Fix backlight parsing for VBT 234+ Patchwork
2020-09-30 13:03 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2020-10-06  9:04 ` [Intel-gfx] [PATCH v2 1/3] " Jani Nikula
2020-10-06 17:10   ` Souza, Jose
2020-10-07 22:45 ` Matt Roper [this message]
2020-10-08  0:33   ` Souza, Jose
2020-10-08  3:28     ` Matt Roper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201007224539.GL2863995@mdroper-desk1.amr.corp.intel.com \
    --to=matthew.d.roper@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jose.souza@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.