All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Allen Ballway <ballway@chromium.org>, ballway@chromium.org
Cc: linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org,
	dri-devel@lists.freedesktop.org, josh@joshtriplett.org
Subject: Re: [Intel-gfx] [PATCH v3 RESEND] drm/i915/quirk: Add quirk for devices that cannot be dimmed
Date: Mon, 11 Sep 2023 20:29:35 +0300	[thread overview]
Message-ID: <87fs3kehz4.fsf@intel.com> (raw)
In-Reply-To: <20230808173957.2017765-1-ballway@chromium.org>

On Tue, 08 Aug 2023, Allen Ballway <ballway@chromium.org> wrote:
> Cybernet T10C cannot be dimmed without the backlight strobing. Create a
> new quirk to lock the minimum brightness to the highest supported value.
> This aligns the device with its behavior on Windows, which will not
> lower the brightness below maximum.

Noting here for the benefit of others, it's possible to make the
brightness work [1], now we "just" need to figure out how to do that
nicely. So we should drop this patch.

BR,
Jani.


[1] https://gitlab.freedesktop.org/drm/intel/-/issues/8187#note_2072633

>
> Signed-off-by: Allen Ballway <ballway@chromium.org>
> ---
> V2 -> V3: Fix typo.
> V1 -> V2: Fix style issue.
>
> .../gpu/drm/i915/display/intel_backlight.c    |  5 ++++
>  drivers/gpu/drm/i915/display/intel_quirks.c   | 27 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_quirks.h   |  1 +
>  3 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
> index 2e8f17c045222..f015563d3ebd5 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
> @@ -1192,6 +1192,11 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector)
>
>  	drm_WARN_ON(&i915->drm, panel->backlight.pwm_level_max == 0);
>
> +	if (intel_has_quirk(i915, QUIRK_NO_DIM)) {
> +		/* Cannot dim backlight, set minimum to highest value */
> +		return panel->backlight.pwm_level_max - 1;
> +	}
> +
>  	/*
>  	 * XXX: If the vbt value is 255, it makes min equal to max, which leads
>  	 * to problems. There are such machines out there. Either our
> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c
> index a280448df771a..910c95840a539 100644
> --- a/drivers/gpu/drm/i915/display/intel_quirks.c
> +++ b/drivers/gpu/drm/i915/display/intel_quirks.c
> @@ -65,6 +65,12 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915)
>  	drm_info(&i915->drm, "Applying no pps backlight power quirk\n");
>  }
>
> +static void quirk_no_dim(struct drm_i915_private *i915)
> +{
> +	intel_set_quirk(i915, QUIRK_NO_DIM);
> +	drm_info(&i915->drm, "Applying no dim quirk\n");
> +}
> +
>  struct intel_quirk {
>  	int device;
>  	int subsystem_vendor;
> @@ -90,6 +96,12 @@ static int intel_dmi_no_pps_backlight(const struct dmi_system_id *id)
>  	return 1;
>  }
>
> +static int intel_dmi_no_dim(const struct dmi_system_id *id)
> +{
> +	DRM_INFO("No dimming allowed on %s\n", id->ident);
> +	return 1;
> +}
> +
>  static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>  	{
>  		.dmi_id_list = &(const struct dmi_system_id[]) {
> @@ -136,6 +148,20 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>  		},
>  		.hook = quirk_no_pps_backlight_power_hook,
>  	},
> +	{
> +		.dmi_id_list = &(const struct dmi_system_id[]) {
> +			{
> +				.callback = intel_dmi_no_dim,
> +				.ident = "Cybernet T10C Tablet",
> +				.matches = {DMI_EXACT_MATCH(DMI_BOARD_VENDOR,
> +							    "Cybernet Manufacturing Inc."),
> +					    DMI_EXACT_MATCH(DMI_BOARD_NAME, "T10C Tablet"),
> +				},
> +			},
> +			{ }
> +		},
> +		.hook = quirk_no_dim,
> +	},
>  };
>
>  static struct intel_quirk intel_quirks[] = {
> @@ -218,6 +244,7 @@ void intel_init_quirks(struct drm_i915_private *i915)
>  		     q->subsystem_device == PCI_ANY_ID))
>  			q->hook(i915);
>  	}
> +
>  	for (i = 0; i < ARRAY_SIZE(intel_dmi_quirks); i++) {
>  		if (dmi_check_system(*intel_dmi_quirks[i].dmi_id_list) != 0)
>  			intel_dmi_quirks[i].hook(i915);
> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.h b/drivers/gpu/drm/i915/display/intel_quirks.h
> index 10a4d163149fd..b41c7bbf0a5e3 100644
> --- a/drivers/gpu/drm/i915/display/intel_quirks.h
> +++ b/drivers/gpu/drm/i915/display/intel_quirks.h
> @@ -17,6 +17,7 @@ enum intel_quirk_id {
>  	QUIRK_INVERT_BRIGHTNESS,
>  	QUIRK_LVDS_SSC_DISABLE,
>  	QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK,
> +	QUIRK_NO_DIM,
>  };
>
>  void intel_init_quirks(struct drm_i915_private *i915);
> --
> 2.41.0.255.g8b1d071c50-goog
>

-- 
Jani Nikula, Intel Open Source Graphics Center

WARNING: multiple messages have this Message-ID (diff)
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Allen Ballway <ballway@chromium.org>, ballway@chromium.org
Cc: linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org,
	arun.r.murthy@intel.com, dri-devel@lists.freedesktop.org,
	josh@joshtriplett.org
Subject: Re: [PATCH v3 RESEND] drm/i915/quirk: Add quirk for devices that cannot be dimmed
Date: Mon, 11 Sep 2023 20:29:35 +0300	[thread overview]
Message-ID: <87fs3kehz4.fsf@intel.com> (raw)
In-Reply-To: <20230808173957.2017765-1-ballway@chromium.org>

On Tue, 08 Aug 2023, Allen Ballway <ballway@chromium.org> wrote:
> Cybernet T10C cannot be dimmed without the backlight strobing. Create a
> new quirk to lock the minimum brightness to the highest supported value.
> This aligns the device with its behavior on Windows, which will not
> lower the brightness below maximum.

Noting here for the benefit of others, it's possible to make the
brightness work [1], now we "just" need to figure out how to do that
nicely. So we should drop this patch.

BR,
Jani.


[1] https://gitlab.freedesktop.org/drm/intel/-/issues/8187#note_2072633

>
> Signed-off-by: Allen Ballway <ballway@chromium.org>
> ---
> V2 -> V3: Fix typo.
> V1 -> V2: Fix style issue.
>
> .../gpu/drm/i915/display/intel_backlight.c    |  5 ++++
>  drivers/gpu/drm/i915/display/intel_quirks.c   | 27 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_quirks.h   |  1 +
>  3 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
> index 2e8f17c045222..f015563d3ebd5 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
> @@ -1192,6 +1192,11 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector)
>
>  	drm_WARN_ON(&i915->drm, panel->backlight.pwm_level_max == 0);
>
> +	if (intel_has_quirk(i915, QUIRK_NO_DIM)) {
> +		/* Cannot dim backlight, set minimum to highest value */
> +		return panel->backlight.pwm_level_max - 1;
> +	}
> +
>  	/*
>  	 * XXX: If the vbt value is 255, it makes min equal to max, which leads
>  	 * to problems. There are such machines out there. Either our
> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c
> index a280448df771a..910c95840a539 100644
> --- a/drivers/gpu/drm/i915/display/intel_quirks.c
> +++ b/drivers/gpu/drm/i915/display/intel_quirks.c
> @@ -65,6 +65,12 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915)
>  	drm_info(&i915->drm, "Applying no pps backlight power quirk\n");
>  }
>
> +static void quirk_no_dim(struct drm_i915_private *i915)
> +{
> +	intel_set_quirk(i915, QUIRK_NO_DIM);
> +	drm_info(&i915->drm, "Applying no dim quirk\n");
> +}
> +
>  struct intel_quirk {
>  	int device;
>  	int subsystem_vendor;
> @@ -90,6 +96,12 @@ static int intel_dmi_no_pps_backlight(const struct dmi_system_id *id)
>  	return 1;
>  }
>
> +static int intel_dmi_no_dim(const struct dmi_system_id *id)
> +{
> +	DRM_INFO("No dimming allowed on %s\n", id->ident);
> +	return 1;
> +}
> +
>  static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>  	{
>  		.dmi_id_list = &(const struct dmi_system_id[]) {
> @@ -136,6 +148,20 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>  		},
>  		.hook = quirk_no_pps_backlight_power_hook,
>  	},
> +	{
> +		.dmi_id_list = &(const struct dmi_system_id[]) {
> +			{
> +				.callback = intel_dmi_no_dim,
> +				.ident = "Cybernet T10C Tablet",
> +				.matches = {DMI_EXACT_MATCH(DMI_BOARD_VENDOR,
> +							    "Cybernet Manufacturing Inc."),
> +					    DMI_EXACT_MATCH(DMI_BOARD_NAME, "T10C Tablet"),
> +				},
> +			},
> +			{ }
> +		},
> +		.hook = quirk_no_dim,
> +	},
>  };
>
>  static struct intel_quirk intel_quirks[] = {
> @@ -218,6 +244,7 @@ void intel_init_quirks(struct drm_i915_private *i915)
>  		     q->subsystem_device == PCI_ANY_ID))
>  			q->hook(i915);
>  	}
> +
>  	for (i = 0; i < ARRAY_SIZE(intel_dmi_quirks); i++) {
>  		if (dmi_check_system(*intel_dmi_quirks[i].dmi_id_list) != 0)
>  			intel_dmi_quirks[i].hook(i915);
> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.h b/drivers/gpu/drm/i915/display/intel_quirks.h
> index 10a4d163149fd..b41c7bbf0a5e3 100644
> --- a/drivers/gpu/drm/i915/display/intel_quirks.h
> +++ b/drivers/gpu/drm/i915/display/intel_quirks.h
> @@ -17,6 +17,7 @@ enum intel_quirk_id {
>  	QUIRK_INVERT_BRIGHTNESS,
>  	QUIRK_LVDS_SSC_DISABLE,
>  	QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK,
> +	QUIRK_NO_DIM,
>  };
>
>  void intel_init_quirks(struct drm_i915_private *i915);
> --
> 2.41.0.255.g8b1d071c50-goog
>

-- 
Jani Nikula, Intel Open Source Graphics Center

WARNING: multiple messages have this Message-ID (diff)
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Allen Ballway <ballway@chromium.org>, ballway@chromium.org
Cc: arun.r.murthy@intel.com, dri-devel@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org, josh@joshtriplett.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 RESEND] drm/i915/quirk: Add quirk for devices that cannot be dimmed
Date: Mon, 11 Sep 2023 20:29:35 +0300	[thread overview]
Message-ID: <87fs3kehz4.fsf@intel.com> (raw)
In-Reply-To: <20230808173957.2017765-1-ballway@chromium.org>

On Tue, 08 Aug 2023, Allen Ballway <ballway@chromium.org> wrote:
> Cybernet T10C cannot be dimmed without the backlight strobing. Create a
> new quirk to lock the minimum brightness to the highest supported value.
> This aligns the device with its behavior on Windows, which will not
> lower the brightness below maximum.

Noting here for the benefit of others, it's possible to make the
brightness work [1], now we "just" need to figure out how to do that
nicely. So we should drop this patch.

BR,
Jani.


[1] https://gitlab.freedesktop.org/drm/intel/-/issues/8187#note_2072633

>
> Signed-off-by: Allen Ballway <ballway@chromium.org>
> ---
> V2 -> V3: Fix typo.
> V1 -> V2: Fix style issue.
>
> .../gpu/drm/i915/display/intel_backlight.c    |  5 ++++
>  drivers/gpu/drm/i915/display/intel_quirks.c   | 27 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_quirks.h   |  1 +
>  3 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
> index 2e8f17c045222..f015563d3ebd5 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
> @@ -1192,6 +1192,11 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector)
>
>  	drm_WARN_ON(&i915->drm, panel->backlight.pwm_level_max == 0);
>
> +	if (intel_has_quirk(i915, QUIRK_NO_DIM)) {
> +		/* Cannot dim backlight, set minimum to highest value */
> +		return panel->backlight.pwm_level_max - 1;
> +	}
> +
>  	/*
>  	 * XXX: If the vbt value is 255, it makes min equal to max, which leads
>  	 * to problems. There are such machines out there. Either our
> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c
> index a280448df771a..910c95840a539 100644
> --- a/drivers/gpu/drm/i915/display/intel_quirks.c
> +++ b/drivers/gpu/drm/i915/display/intel_quirks.c
> @@ -65,6 +65,12 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915)
>  	drm_info(&i915->drm, "Applying no pps backlight power quirk\n");
>  }
>
> +static void quirk_no_dim(struct drm_i915_private *i915)
> +{
> +	intel_set_quirk(i915, QUIRK_NO_DIM);
> +	drm_info(&i915->drm, "Applying no dim quirk\n");
> +}
> +
>  struct intel_quirk {
>  	int device;
>  	int subsystem_vendor;
> @@ -90,6 +96,12 @@ static int intel_dmi_no_pps_backlight(const struct dmi_system_id *id)
>  	return 1;
>  }
>
> +static int intel_dmi_no_dim(const struct dmi_system_id *id)
> +{
> +	DRM_INFO("No dimming allowed on %s\n", id->ident);
> +	return 1;
> +}
> +
>  static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>  	{
>  		.dmi_id_list = &(const struct dmi_system_id[]) {
> @@ -136,6 +148,20 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>  		},
>  		.hook = quirk_no_pps_backlight_power_hook,
>  	},
> +	{
> +		.dmi_id_list = &(const struct dmi_system_id[]) {
> +			{
> +				.callback = intel_dmi_no_dim,
> +				.ident = "Cybernet T10C Tablet",
> +				.matches = {DMI_EXACT_MATCH(DMI_BOARD_VENDOR,
> +							    "Cybernet Manufacturing Inc."),
> +					    DMI_EXACT_MATCH(DMI_BOARD_NAME, "T10C Tablet"),
> +				},
> +			},
> +			{ }
> +		},
> +		.hook = quirk_no_dim,
> +	},
>  };
>
>  static struct intel_quirk intel_quirks[] = {
> @@ -218,6 +244,7 @@ void intel_init_quirks(struct drm_i915_private *i915)
>  		     q->subsystem_device == PCI_ANY_ID))
>  			q->hook(i915);
>  	}
> +
>  	for (i = 0; i < ARRAY_SIZE(intel_dmi_quirks); i++) {
>  		if (dmi_check_system(*intel_dmi_quirks[i].dmi_id_list) != 0)
>  			intel_dmi_quirks[i].hook(i915);
> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.h b/drivers/gpu/drm/i915/display/intel_quirks.h
> index 10a4d163149fd..b41c7bbf0a5e3 100644
> --- a/drivers/gpu/drm/i915/display/intel_quirks.h
> +++ b/drivers/gpu/drm/i915/display/intel_quirks.h
> @@ -17,6 +17,7 @@ enum intel_quirk_id {
>  	QUIRK_INVERT_BRIGHTNESS,
>  	QUIRK_LVDS_SSC_DISABLE,
>  	QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK,
> +	QUIRK_NO_DIM,
>  };
>
>  void intel_init_quirks(struct drm_i915_private *i915);
> --
> 2.41.0.255.g8b1d071c50-goog
>

-- 
Jani Nikula, Intel Open Source Graphics Center

  reply	other threads:[~2023-09-11 17:29 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-06 21:25 [Intel-gfx] [PATCH] drm/i915/quirk: Add quirk for devices that cannot be dimmed Allen Ballway
2023-06-06 21:25 ` Allen Ballway
2023-06-06 21:25 ` Allen Ballway
2023-06-07  0:32 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2023-06-07  0:37 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-06-07 16:40 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2023-06-22 17:28 ` [Intel-gfx] [PATCH v2] " Allen Ballway
2023-06-22 17:28   ` Allen Ballway
2023-06-22 17:28   ` Allen Ballway
2023-06-22 17:51   ` [Intel-gfx] " Josh Triplett
2023-06-22 17:51     ` Josh Triplett
2023-06-22 17:51     ` Josh Triplett
2023-06-29 17:21     ` [Intel-gfx] [PATCH] " Allen Ballway
2023-06-29 17:21       ` Allen Ballway
2023-06-29 17:21       ` Allen Ballway
2023-08-08 17:39       ` [Intel-gfx] [PATCH v3 RESEND] " Allen Ballway
2023-08-08 17:39         ` Allen Ballway
2023-08-08 17:39         ` Allen Ballway
2023-09-11 17:29         ` Jani Nikula [this message]
2023-09-11 17:29           ` Jani Nikula
2023-09-11 17:29           ` Jani Nikula
2023-06-22 18:43 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/quirk: Add quirk for devices that cannot be dimmed (rev2) Patchwork
2023-06-23  8:12 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2023-06-29 19:53 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/quirk: Add quirk for devices that cannot be dimmed (rev3) Patchwork
2023-08-08 19:43 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/quirk: Add quirk for devices that cannot be dimmed (rev4) Patchwork
2023-08-08 19:43 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-08-08 19:54 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-08-09  6:26 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

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=87fs3kehz4.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=ballway@chromium.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    /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.